linux-kernel - RE: [EXT] regression due to soc_device_match not handling defer (Was: [PATCH v4 4/4] soc: imx8m: change to use 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: <DB6PR0402MB27607D28703D8C3E614E2F2A884D9@DB6PR0402MB2760.eurprd04.prod.outlook.com>
Date:   Thu, 15 Apr 2021 01:33:02 +0000
From:   Peng Fan <peng.fan@....com>
To:     Dominique MARTINET <dominique.martinet@...ark-techno.com>,
        "Alice Guo (OSS)" <alice.guo@....nxp.com>
CC:     Shawn Guo <shawnguo@...nel.org>,
        Krzysztof Kozlowski <krzk@...nel.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        dl-linux-imx <linux-imx@....com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [EXT] regression due to soc_device_match not handling defer (Was:
 [PATCH v4 4/4] soc: imx8m: change to use platform driver)

> (Was: [PATCH v4 4/4] soc: imx8m: change to use platform driver)
> 
> Alice Guo (OSS) wrote on Tue, Mar 30, 2021 at 02:41:23AM +0000:
> > Thanks for reporting this issue, I'll check and add a fix to handle defer probe.
> 
> I haven't seen any follow up on this, have you had a chance to take a look?

We are trying to find a proper solution for this.

The proper method might be make soc_device_match return probe defer,
and take early soc attr into consideration, but I am not sure this would win
maintainer's vote.

> If this won't make it for 5.12 (in a couple of week probably?) would it make
> sense to revert 7d981405d0fd ("soc: imx8m: change to use platform
> driver") for now?

Please no. We are targeting android GKI, make driver as modules.
And reverting to original method will also break kexec.

I am on IRC #linux-imx, we could take more if you would like to.

Thanks,
Peng.

> 
> 
> 
> While looking at the code earlier I also have an unrelated, late-review on the
> patch itself:
> 
> > +static u32 __init imx8mq_soc_revision(struct device *dev)
> > [...]
> >  @@ -191,8 +223,16 @@ static int __init imx8_soc_init(void)
> >         data = id->data;
> >         if (data) {
> >                 soc_dev_attr->soc_id = data->name;
> > -               if (data->soc_revision)
> > -                       soc_rev = data->soc_revision();
> > +               if (data->soc_revision) {
> > +                       if (pdev) {
> > +                               soc_rev =
> data->soc_revision(&pdev->dev);
> > +                               ret = soc_rev;
> > +                               if (ret < 0)
> 
> I appreciate current soc_revision are "small enough" (looking at
> include/soc/imx/revision.h we're talking < 256) so this actually works, but
> would it make sense to either make soc_rev signed, or to have
> soc_revision() return a s64, or have the revision filled in another *u32
> argument to make sure the error is an error and not just a large rev?
> 
> This is most definitely fine for now but that kind of code patterns can lead to
> weird errors down the road.
> 
> Thanks,
> --
> Dominique