RE: [PATCH] soc: imx: check ls1021a

From: Peng Fan <peng.fan@nxp.com>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Aisheng Dong <aisheng.dong@nxp.com>,
	Horia Geanta <horia.geanta@nxp.com>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	dl-linux-imx <linux-imx@nxp.com>,
	Sascha Hauer <kernel@pengutronix.de>,
	Leonard Crestez <leonard.crestez@nxp.com>,
	Shawn Guo <shawnguo@kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>
Subject: RE: [PATCH] soc: imx: check ls1021a
Date: Mon, 20 Jul 2020 07:05:05 +0000	[thread overview]
Message-ID: <DB6PR0402MB2760D45E364C89E994B84DB4887B0@DB6PR0402MB2760.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <CAK8P3a0JSZacnSQgJ95Za1MNA-AB0cxCd=vSEf4KY+Mw=94vdw@mail.gmail.com>

> Subject: Re: [PATCH] soc: imx: check ls1021a
> 
> On Mon, Jul 20, 2020 at 8:30 AM Peng Fan <peng.fan@nxp.com> wrote:
> >
> > Hi Arnd,
> >
> > > Subject: Re: [PATCH] soc: imx: check ls1021a
> > >
> > > On Thu, Jul 9, 2020 at 10:31 AM <peng.fan@nxp.com> wrote:
> > > >
> > > > From: Peng Fan <peng.fan@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@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.

Ok, then we have same issue in soc-imx8m.c soc-imx-scu.c I think.
soc-imx-scu.c use platform driver, but it not use DT.

> 
> 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.

Got it.

> 
> 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.

Thanks for the tips, I'll check to see which could be used here.

Thanks,
Peng.

> 
>      Arnd
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel