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: <ac403563-34ee-4ba1-b658-06f8df0f5f90@denx.de>
Date: Thu, 26 Sep 2024 23:26:45 +0200
From: Marek Vasut <marex@...x.de>
To: Saravana Kannan <saravanak@...gle.com>, Arnd Bergmann <arnd@...db.de>
Cc: 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 9/26/24 7:36 PM, Saravana Kannan wrote:
> 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.
Fixed in V3.