Hello Carlos,
On 20.12.24 07:58, Carlos Song wrote:
>
>
>> -----Original Message-----
>> From: Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx>
>> Sent: Friday, December 20, 2024 2:13 PM
>> To: Carlos Song <carlos.song@xxxxxxx>
>> Cc: Andi Shyti <andi.shyti@xxxxxxxxxx>; Frank Li <frank.li@xxxxxxx>;
>> kernel@xxxxxxxxxxxxxx; shawnguo@xxxxxxxxxx; s.hauer@xxxxxxxxxxxxxx;
>> festevam@xxxxxxxxx; linux-i2c@xxxxxxxxxxxxxxx; imx@xxxxxxxxxxxxxxx;
>> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Clark Wang
>> <xiaoning.wang@xxxxxxx>; Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx>
>> Subject: [EXT] Re: [PATCH v5] i2c: imx: support DMA defer probing
>>
>> Caution: This is an external email. Please take care when clicking links or
>> opening attachments. When in doubt, report the message using the 'Report this
>> email' button
>>
>>
>> On Fri, Dec 20, 2024 at 05:59:38AM +0000, Carlos Song wrote:
>>>>> So we make this logic. Anyway we let the I2C controller registered
>>>>> whether
>>>> DMA is available or not(except defer probe).
>>>>> Ignoring ENODEV and EPROBE_DEFER makes it looks like nothing
>>>>> happened if
>>>> DMA is defer probed or not enabled(This is an expected).
>>>>> However we still need i2c DMA status is known when meet an
>>>>> unexpected
>>>> error, so we use dev_err_probe() to print error.
>>>>
>>>> Why dev_err_probe() instead of dev_err()?
>>>>
>>> Hi,
>>> In patch V2 discussion, Marc suggested just return dev_err_probe(),
>>> but I don't accept it so I choose to use dev_err_probe() to print error in V3.[1]
>> In this case, the two APIs have the same function, do you mean dev_err() is more
>> suitable?
>>
>> Yes, dev_err_probe() should be used in combination with return. For
>> example:
>> return dev_err_probe(...);
>>
>> It will pass the return value on exit of the function and optionally print of the
>> error message if it is not EPROBE_DEFER. Practically it replace commonly used
>> coding pattern:
>> if (ret == -EPROBE_DEFER) {
>> return ret;
>> } else if (ret) {
>> dev_err(..);
>> return ret;
>> }
>>
> Hi,
>
> Get your good point. I will change my code in V6:
> + ret = i2c_imx_dma_request(i2c_imx, phy_addr);
> + if (ret) {
> + if (ret == -EPROBE_DEFER)
> + goto clk_notifier_unregister;
> + else if (ret == -ENODEV)
> + dev_dbg(&pdev->dev, "Only use PIO mode\n");
> + else
> + dev_err(&pdev->dev, "Failed to setup DMA, only use PIO mode\n");
> + }
>
> I think this is what you want to see, right?
This loses the information why the error happens (ret). Using dev_err_probe
even if no probe deferral is expected in that branch is perfectly fine
and the kernel-doc even points it out:
Using this helper in your probe function is totally fine even if @err
is known to never be -EPROBE_DEFER.
Cheers,
Ahmad
>
>> --
>> Pengutronix e.K. |
>> |
>> Steuerwalder Str. 21 |
>> http://www.pen/
>> gutronix.de%2F&data=05%7C02%7Ccarlos.song%40nxp.com%7C2950266755a
>> 241c00a9208dd20bd5cf2%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0
>> %7C638702719862691439%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGki
>> OnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ
>> %3D%3D%7C0%7C%7C%7C&sdata=aIuzJP0v5C6HzOCGnCHobK9Llml3thHclTwu
>> CjD13IM%3D&reserved=0 |
>> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0
>> |
>> Amtsgericht Hildesheim, HRA 2686 | Fax:
>> +49-5121-206917-5555 |
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
