Hi, Krzysztof.

Thanks for the feedback.

-----Original Message-----
From: Krzysztof Kozlowski <krzk@kernel.org>
Sent: Friday, August 29, 2025 11:06 PM
To: Nas Chung <nas.chung@chipsnmedia.com>; mchehab@kernel.org;
hverkuil@xs4all.nl; robh@kernel.org; krzk+dt@kernel.org;
conor+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de
Cc: linux-media@vger.kernel.org; devicetree@vger.kernel.org; linux-
kernel@vger.kernel.org; linux-imx@nxp.com; linux-arm-
kernel@lists.infradead.org; jackson.lee [off-list ref];
lafley.kim [off-list ref]; Ming Qian [off-list ref]
Subject: Re: [PATCH v3 8/9] media: chips-media: wave6: Add Wave6 control
driver

On 29/08/2025 10:46, Nas Chung wrote:

quoted

+
+static void wave6_vpu_load_firmware(const struct firmware *fw, void

*context)

quoted

+{
+	struct wave6_vpu_device *vpu = context;
+
+	guard(mutex)(&vpu->lock);

Why? How could this be called in parallel, before the probe?

This callback is called asynchronously via firmware_request_nowait_nowarn().
In practice, I observed a case where the callback was executing
while the device's release() function was being called.

quoted

+
+	if (!fw || !fw->data) {
+		dev_err(vpu->dev, "No firmware.\n");
+		return;
+	}
+
+	if (!vpu->fw_available)
+		goto exit;
+
+	if (fw->size + W6_EXTRA_CODE_BUF_SIZE >

wave6_vpu_get_code_buf_size(vpu)) {

quoted

+		dev_err(vpu->dev, "firmware size (%ld > %zd) is too big\n",
+			fw->size, vpu->code_buf.size);
+		vpu->fw_available = false;
+		goto exit;
+	}
+
+	memcpy(vpu->code_buf.vaddr, fw->data, fw->size);
+
+	vpu->get_vpu = wave6_vpu_get;
+	vpu->put_vpu = wave6_vpu_put;
+	vpu->req_work_buffer = wave6_vpu_require_work_buffer;
+	of_platform_populate(vpu->dev->of_node, NULL, NULL, vpu->dev);
+
+exit:
+	release_firmware(fw);
+}
+
+static int wave6_vpu_probe(struct platform_device *pdev)
+{
+	struct device_node *np;
+	struct wave6_vpu_device *vpu;
+	const struct wave6_vpu_resource *res;
+	int ret;
+
+	ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
+	if (ret < 0) {
+		dev_err(&pdev->dev, "dma_set_mask_and_coherent failed: %d\n",

ret);

quoted

+		return ret;
+	}
+
+	res = of_device_get_match_data(&pdev->dev);
+	if (!res)
+		return -ENODEV;
+
+	vpu = devm_kzalloc(&pdev->dev, sizeof(*vpu), GFP_KERNEL);
+	if (!vpu)
+		return -ENOMEM;
+
+	ret = devm_mutex_init(&pdev->dev, &vpu->lock);
+	if (ret)
+		return ret;
+
+	atomic_set(&vpu->core_count, 0);
+	INIT_LIST_HEAD(&vpu->work_buffers);
+	dev_set_drvdata(&pdev->dev, vpu);
+	vpu->dev = &pdev->dev;
+	vpu->res = res;
+	vpu->reg_base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(vpu->reg_base))
+		return PTR_ERR(vpu->reg_base);
+
+	ret = devm_clk_bulk_get_all(&pdev->dev, &vpu->clks);
+	if (ret < 0) {
+		dev_warn(&pdev->dev, "unable to get clocks: %d\n", ret);

You need to handle deferred probe.

Got it, I will address this in v4 using dev_err_probe().

Thanks,
Nas.

quoted

+		ret = 0;
+	}
+	vpu->num_clks = ret;

Best regards,
Krzysztof