Re: [PATCH v5] i2c: imx: support DMA defer probing

Carlos Song
2024-12-20T02:45:13Z

> -----Original Message-----
> From: Oleksij Rempel <o.rempel@pengutronix.de>
> Sent: Thursday, December 19, 2024 8:23 PM
> To: Andi Shyti <andi.shyti@kernel.org>
> Cc: Carlos Song <carlos.song@nxp.com>; Frank Li <frank.li@nxp.com>;
> kernel@pengutronix.de; shawnguo@kernel.org; s.hauer@pengutronix.de;
> festevam@gmail.com; linux-i2c@vger.kernel.org; imx@lists.linux.dev;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Clark Wang
> <xiaoning.wang@nxp.com>; Ahmad Fatoum <a.fatoum@pengutronix.de>
> 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 Thu, Dec 19, 2024 at 01:02:29PM +0100, Andi Shyti wrote:
> > Hi Carlos,
> >
> > > + /*
> > > + * Init DMA config if supported, -ENODEV means DMA not enabled at
> > > + * this platform, that is not a real error, so just remind "only
> > > + * PIO mode is used". If DMA is enabled, but meet error when request
> > > + * DMA channel, error should be showed in probe error log. PIO mode
> > > + * should be available regardless of DMA.
> > > + */
> > > + 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_probe(&pdev->dev, ret, "Failed to setup
> > > + DMA, only use PIO mode\n");
> >
> > Just for understanding, should we quit in this last case, as well?
> >
> > Before we were ignoring ENODEV and EPROBE_DEFER, but now you are
> > making it clear that other failures like ENOMEM might happen.
>
> Good point, dev_err_probe() would not print an error in case of EPROBE_DEFER,
> but in this case we should only print error and continue with PIO.
>
Hi,

Thank you all very much! As I comment at previous mail:
DMA mode should be optional for i2c-imx, because i2c-imx can accept DMA mode not enabled, because it still can work in CPU mode.[1]
[1]https://lore.kernel.org/imx/AM0PR0402MB39374E34FD6133B5E3D414D7E82F2@AM0PR0402MB3937.eurprd04.prod.outlook.com/

Also we don't want to annoy current user without DMA[2]
[2] https://lore.kernel.org/imx/20241127-analytic-azure-hamster-727fd8-mkl@pengutronix.de/

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.

> --
> Pengutronix e.K. |
> |
> Steuerwalder Str. 21 |
> http://www.pen/
> gutronix.de%2F&data=05%7C02%7Ccarlos.song%40nxp.com%7Cf0f2b14d05e4
> 4bb9581408dd2027ef82%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%
> 7C638702078063818207%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiO
> nRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%
> 3D%3D%7C0%7C%7C%7C&sdata=q355QWfiCHuoREtM4pkuovpn%2F6MeXynD
> hrpSEpWeCXk%3D&reserved=0 |
> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0
> |
> Amtsgericht Hildesheim, HRA 2686 | Fax:
> +49-5121-206917-5555 |