[PATCH] soc: imx: check ls1021a

[Peng Fan]

> Subject: Re: [PATCH] soc: imx: check ls1021a
> 
> On Mon, Jul 20, 2020 at 8:30 AM Peng Fan <peng.fan at nxp.com> wrote:
> >
> > Hi Arnd,
> >
> > > Subject: Re: [PATCH] soc: imx: check ls1021a
> > >
> > > On Thu, Jul 9, 2020 at 10:31 AM <peng.fan at nxp.com> wrote:
> > > >
> > > > From: Peng Fan <peng.fan at nxp.com>
> > > >
> > > > fsl,ls1021a is a mach under arch/arm/mach-imx/, however it could
> > > > not use the soc driver which will break caam on ls1021a platform.
> > > >
> > > > So directly return if it is compatible with fsl,ls1021a.
> > > >
> > > > Fixes: 52102a3ba6a61 ("soc: imx: move cpu code to
> > > > drivers/soc/imx")
> > > > Signed-off-by: Peng Fan <peng.fan at nxp.com>
> > >
> > > I just forwarded this patch to Linus as part of the v5.8 fixes.
> > > The patch goes in the right direction, but as far as I can tell, the
> > > code is still wrong and needs to be fixed. Can you create another patch on
> top?
> > >
> > > The problem is that loading this driver on *anything* other than an
> > > i.MX machine will cause unexpected results, and it first has to
> > > check that it is running on a compatible machine to start with!
> > >
> > > In a distro kernel, this driver is always built-in at the moment if
> > > CONFIG_ARCH_MXC is enabled, regardless of what other platforms are
> > > enabled in addition, and what machine we end up running on.
> >
> > So I could use CONFIG_SOC_IMX[x] to build the file? Is this ok for you?
> 
> Certainly not, that would not address the problem at all.
> 
> You really have to replace the compile-time decision with a runtime decision.
> Running hardware specific code as the result of an initcall or module_init() is
> never correct.
> 
> The usual way this is handled is to register a platform_driver instance that
> binds to a set of DT compatible strings, and then only do hardware specific
> actions in the probe function that is called if as compatible device is actually
> present.
> 
> If you cannot do that here (e.g. because you need the soc_device to be
> present very early), then you have to manually check for some DT node and
> property to determine whether you run on an i.MX machine and do the silent
> 'return 0' if not.
> This is roughly what you did when you checked for one specific non-i.MX
> machine, but since you cannot provide an exhaustive list of all non-i.MX
> machines (a denylist) you have to provide some for of allowlist.

Is the following diff ok for you?
Actually all i.MX SoCs will set __mxc_cpu_type. If it is not a valid
one, just return.

diff --git a/drivers/soc/imx/soc-imx.c b/drivers/soc/imx/soc-imx.c
index 01bfea1cb64a..e55225a85592 100644
--- a/drivers/soc/imx/soc-imx.c
+++ b/drivers/soc/imx/soc-imx.c
@@ -33,9 +33,6 @@ static int __init imx_soc_device_init(void)
        u32 val;
        int ret;

-       if (of_machine_is_compatible("fsl,ls1021a"))
-               return 0;
-
        soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
        if (!soc_dev_attr)
                return -ENOMEM;
@@ -131,6 +128,8 @@ static int __init imx_soc_device_init(void)
                break;
        default:
                soc_id = "Unknown";
+               ret = 0;
+               goto free_soc;
        }
        soc_dev_attr->soc_id = soc_id;

Thanks,
Peng.

> 
>      Arnd