[PATCH] soc: imx: check ls1021a

[Arnd Bergmann]

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.

     Arnd