[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<SL2P216MB1246A048955A09A5A4D15CA3FB07A@SL2P216MB1246.KORP216.PROD.OUTLOOK.COM>
Date: Mon, 1 Sep 2025 08:13:44 +0000
From: Nas Chung <nas.chung@...psnmedia.com>
To: Krzysztof Kozlowski <krzk@...nel.org>, "mchehab@...nel.org"
<mchehab@...nel.org>, "hverkuil@...all.nl" <hverkuil@...all.nl>,
"robh@...nel.org" <robh@...nel.org>, "krzk+dt@...nel.org"
<krzk+dt@...nel.org>, "conor+dt@...nel.org" <conor+dt@...nel.org>,
"shawnguo@...nel.org" <shawnguo@...nel.org>, "s.hauer@...gutronix.de"
<s.hauer@...gutronix.de>
CC: "linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-imx@....com" <linux-imx@....com>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, jackson.lee
<jackson.lee@...psnmedia.com>, lafley.kim <lafley.kim@...psnmedia.com>, Ming
Qian <ming.qian@....nxp.com>
Subject: RE: [PATCH v3 8/9] media: chips-media: wave6: Add Wave6 control
driver
Hi, Krzysztof.
Thanks for the feedback.
>-----Original Message-----
>From: Krzysztof Kozlowski <krzk@...nel.org>
>Sent: Friday, August 29, 2025 11:06 PM
>To: Nas Chung <nas.chung@...psnmedia.com>; mchehab@...nel.org;
>hverkuil@...all.nl; robh@...nel.org; krzk+dt@...nel.org;
>conor+dt@...nel.org; shawnguo@...nel.org; s.hauer@...gutronix.de
>Cc: linux-media@...r.kernel.org; devicetree@...r.kernel.org; linux-
>kernel@...r.kernel.org; linux-imx@....com; linux-arm-
>kernel@...ts.infradead.org; jackson.lee <jackson.lee@...psnmedia.com>;
>lafley.kim <lafley.kim@...psnmedia.com>; Ming Qian <ming.qian@....nxp.com>
>Subject: Re: [PATCH v3 8/9] media: chips-media: wave6: Add Wave6 control
>driver
>
>On 29/08/2025 10:46, Nas Chung wrote:
>> +
>> +static void wave6_vpu_load_firmware(const struct firmware *fw, void
>*context)
>> +{
>> + 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.
>
>> +
>> + 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)) {
>> + 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);
>> + 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.
>
>> + ret = 0;
>> + }
>> + vpu->num_clks = ret;
>Best regards,
>Krzysztof
Powered by blists - more mailing lists