Message-ID: <CAGETcx_ODj-FqhH7stxu27PpGNg2LiFwgC=1piLaBjDV4OagLA@mail.gmail.com>
Date: Wed, 25 Sep 2024 11:48:12 -0700
From: Saravana Kannan <saravanak@...gle.com>
To: Marek Vasut <marex@...x.de>
Cc: Arnd Bergmann <arnd@...db.de>, linux-arm-kernel@...ts.infradead.org, 
	kernel@...electronics.com, 
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>, 
	Fabio Estevam <festevam@...il.com>, Jeff Johnson <quic_jjohnson@...cinc.com>, 
	Neil Armstrong <neil.armstrong@...aro.org>, Pengutronix Kernel Team <kernel@...gutronix.de>, 
	Sascha Hauer <s.hauer@...gutronix.de>, Shawn Guo <shawnguo@...nel.org>, imx@...ts.linux.dev, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] soc: imx8m: Probe the SoC driver late

On Wed, Sep 25, 2024 at 10:07 AM Marek Vasut <marex@...x.de> wrote:
>
> On 9/25/24 6:31 PM, Arnd Bergmann wrote:
> > On Wed, Sep 25, 2024, at 16:09, Marek Vasut wrote:
> >> On 9/25/24 6:04 PM, Arnd Bergmann wrote:
> >>> On Wed, Sep 25, 2024, at 16:00, Marek Vasut wrote:
> >>>> With driver_async_probe=* on kernel command line, the following trace is
> >>>> produced because on i.MX8M Plus hardware because the soc-imx8m.c driver
> >>>> calls of_clk_get_by_name() which returns -EPROBE_DEFER because the clock
> >>>> driver is not yet probed. This was not detected during regular testing
> >>>> without driver_async_probe.
> >>>>
> >>>> Attempt to fix it by probing the SoC driver late, but I don't think that
> >>>> is the correct approach here.
> >>>
> >>> I think the correct fix would be to propagate the -EPROBE_DEFER
> >>> and return that from imx8_soc_init(), so it gets retried again
> >>> after the clock driver.
> >> I already tried that, but if I return -EPROBE_DEFER from
> >> device_initcall, it doesn't get retriggered . I suspect EPROBE_DEFER
> >> works only for proper drivers ?
> >
> > Right, of course. And unfortunately it can't just register to
> > the fsl,imx8mm-anatop/fsl,imx8mm-ocotp/... nodes because they
> > all have a driver already.

Arnd,

Can't we change this to add a platform device and a platform driver in
the initcall? And then the driver can return -EPROBE_DEFER if it can't
get the clock yet?

> >
> > On the other hand, making it a late_initcall() defeats the
> > purpose of the driver because then it can't be used by other
> > drivers with soc_device_match(), resulting in incorrect
> > behavior when another driver relies on this to enable
> > a chip revision specific workaround.

We could have soc_device_match() return -EPROBE_DEFER if no soc device
has been registered yet.

For cases where it's already working without any changes, we shouldn't
see any new -EPROBE_DEFER return values. But for cases like what Marek
is trying to do, it should work properly. He might have to fix bad
driver code where they remap the error instead of returning it as is.

On a tangential note, the soc framework seems to be yet another
framework violating the bus vs class thing. If it's a bus, then you
need to have a probe. Otherwise, just make it a class. Might be too
much to fix at this point, but might be good to keep this in mind if
you plan to write more frameworks or redo soc framework at some point
:)

See Slide 20:
https://lpc.events/event/18/contributions/1734/

> I know, I am unsure how to proceed with this. Convert this to
> platform_driver or some such and probe it early maybe ?

Marek,

Thanks for trying out drive_async_probe=* and trying to fix boards/drivers.

The issue you are facing and the proper way to handle it was covered
in my talk at LPC 2024 in Slide 18:
https://lpc.events/event/18/contributions/1734/

All the benefits of fw_devlink are only present for drivers that use a
device-driver model. And yes, in this case we should convert this
driver into a platform device/driver if it's possible.

Thanks,
Saravana

Powered by blists - more mailing lists