linux-kernel - Re: [PATCH v2] soc: imx8m: Probe the SoC driver as platform driver

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGETcx8opbQ=+dBkSWwfCoqGsvUBftJm7VaGULZhC5DNyuktJQ@mail.gmail.com>
Date: Thu, 26 Sep 2024 10:36:16 -0700
From: Saravana Kannan <saravanak@...gle.com>
To: Arnd Bergmann <arnd@...db.de>
Cc: Marek Vasut <marex@...x.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 v2] soc: imx8m: Probe the SoC driver as platform driver

On Wed, Sep 25, 2024 at 11:28 PM Arnd Bergmann <arnd@...db.de> wrote:
>
> On Wed, Sep 25, 2024, at 22:04, 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.
> >
> > Convert the SoC code to platform driver and instantiate a platform device
> > in its current device_initcall() to probe the platform driver. Rework
> > .soc_revision callback to always return valid error code and return SoC
> > revision via parameter. This way, if anything in the .soc_revision callback
> > return -EPROBE_DEFER, it gets propagated to .probe and the .probe will get
> > retried later.
>
> Thanks for the new version, that was quick!
>
> > +static struct platform_driver imx8m_soc_driver = {
> > +     .probe = imx8m_soc_probe,
> > +     .driver = {
> > +             .name = "imx8m-soc",
> > +     },
> > +};
> > +module_platform_driver(imx8m_soc_driver);
> > +
> > +static int __init imx8_soc_init(void)
> > +{
> > +     struct platform_device *pdev;
> > +
> > +     pdev = platform_device_register_simple("imx8m-soc", -1, NULL, 0);
> > +
> > +     return IS_ERR(pdev) ? PTR_ERR(pdev) : 0;
> > +}
> >  device_initcall(imx8_soc_init);
>
> Did you run into problems with the method I suggested first?
>
> I don't like the way that this version still registers both the
> device and driver regardless of the hardware it runs on, I'd
> prefer to leave the platform check in the initcall and
> only register them if we are actually on an imx8 machine.
>
> Having two initcalls also makes it impossible to build this
> as a loadable module, which is why I suggested
> platform_create_bundle().

That particular helper doesn't seem to like probe deferral because it
allows for some of the code to be in init sections. So, it gets
dropped after module load is complete. So, Marek will still have to
register the device and driver separately. But I agree that it might
be better to NOT register the device and driver if the basic
conditions aren't met.

-Saravana

> I think you can keep the
> of_device_id lookup and pass the imx8_soc_data pointer
> as the platform_data to platform_create_bundle.
>
>     Arnd