Thread (6 messages) 6 messages, 2 authors, 2024-12-25

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

From: Andi Shyti <andi.shyti@kernel.org>
Date: 2024-12-24 08:32:42
Also in: imx, linux-i2c, lkml

quoted
quoted
@@ -1802,6 +1803,18 @@ static int i2c_imx_probe(struct platform_device
*pdev)
quoted
      if (ret == -EPROBE_DEFER)
              goto clk_notifier_unregister;

+     /* As we can always fall back to PIO, let's ignore the error setting up
DMA. */
quoted
+     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 (%pe),
only use PIO mode\n",
quoted
+                             ERR_PTR(ret));
My question here is not just about the use of dev_err vs dev_err_probe, but why
don't we exit the probe if we get an error.

We should use PIO only in case of ENODEV, in all the other cases I think we
should just leave. E.g. why don't we exit if we meet ret == -ENOMEM?
Hi, Andi

Thank you! From my point, I2C is critical bus so it should be available as much as possible.
-ENOMEM or other unknown errors all are from i2c_imx_dma_request(). So error happened in enable DMA mode process.
OK, makes sense, it's the idea of "let things fail on their own,
I'll move forward as much as I can"; we need to be aware of
the choice. Please add a comment above.

But then it's not an error, but a warning. With errors we bail
out, with warnings we tell users that something went wrong.

Sorry for keeping you on this point for so long, but do you mind
swapping this dev_err in dev_warn, with a comment explaining the
reason we decided not to leave?

Thanks,
Andi
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help