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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ltb3ywaz2req4yqdqmtq4ejbzh4esjszbx4x6ab3k5zmqxhdpg@qqjetty6fs3q>
Date: Tue, 24 Dec 2024 09:32:38 +0100
From: Andi Shyti <andi.shyti@...nel.org>
To: Carlos Song <carlos.song@....com>
Cc: "o.rempel@...gutronix.de" <o.rempel@...gutronix.de>, 
	"kernel@...gutronix.de" <kernel@...gutronix.de>, "shawnguo@...nel.org" <shawnguo@...nel.org>, 
	"s.hauer@...gutronix.de" <s.hauer@...gutronix.de>, "festevam@...il.com" <festevam@...il.com>, 
	"linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>, "imx@...ts.linux.dev" <imx@...ts.linux.dev>, 
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, 
	Clark Wang <xiaoning.wang@....com>, Ahmad Fatoum <a.fatoum@...gutronix.de>
Subject: Re: [PATCH v6] i2c: imx: support DMA defer probing

> > > @@ -1802,6 +1803,18 @@ static int i2c_imx_probe(struct platform_device
> > *pdev)
> > >       if (ret == -EPROBE_DEFER)
> > >               goto clk_notifier_unregister;
> > >
> > > +     /* As we can always fall back to PIO, let's ignore the error setting up
> > 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(&pdev->dev, "Failed to setup DMA (%pe),
> > only use PIO mode\n",
> > > +                             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