On Wed, Oct 26, 2016 at 06:55:22AM +0000, M.H. Lian wrote:
> Hi Robin,
>
> Please see my comments inline.
>
> Thanks,
> Minghuan
>
> > -----Original Message-----
> > From: Robin Murphy [mailto:robin.mur...@arm.com]
> > Sent: Tuesday, October 25, 2016 9:01 PM
> > To: M.H. Lian <minghuan.l...@nxp.com>; linux-arm-
> > ker...@lists.infradead.org; linux-kernel@vger.kernel.org;
> > devicet...@vger.kernel.org
> > Cc: Marc Zyngier <marc.zyng...@arm.com>; Stuart Yoder
> > <stuart.yo...@nxp.com>; Leo Li <leoyang...@nxp.com>; Scott Wood
> > <scott.w...@nxp.com>; Shawn Guo <shawn...@kernel.org>; Mingkai Hu
> > <mingkai...@nxp.com>
> > Subject: Re: [PATCH 1/6] dt/bindings: adjust bindings for Layerscape SCFG
> > MSI
> >
> > On 25/10/16 13:35, Minghuan Lian wrote:
> > > 1. The different version of a SoC may have different MSI
> > > implementation. But compatible "fsl,<soc-name>-msi" can not describe
> > > the SoC version.
> >
> > Can't it?
> >
> > compatible = "fsl-ls1043a-rev11-msi";
> >
> > Oh, I guess it can!
> >
> > Joking aside, if there are multiple versions of a piece of hardware which
> > require *different* treatment by drivers, then it is obviously wrong to use
> > the same compatible string because *they are not compatible*.
> >
> [Minghuan Lian] Yes, but Rev1.0 and Rev1.1 SoC will use the same dts files.
> We cannot create different dts files for each revision of the same kind of
> SoC.
... why?
The DT should describe the hardware; if hardware differs then it should
have a different DT.
> It means that there are different variants in the different versions
> of the same SoC that will use the same compatible string.
Why can you not add a string for each variant, in addition to the SoC
string? We do that elsewhere.
> So I have to use SoC match interface to get the versions.
>
> I'm too radical. I do not want to first check SoC family via
> compatible string and then check revision via SoC match or SVR.
You can have *both* in the the compatible string list, e.g. at the top
level:
compatible = "vendor,soc-rev", "vendor-soc";
For devices which differ, this can be encoded similarly in the device
compatible string list.
> I selected the "SoC match" like the following to get the related information
> at only one place.
>
> static struct soc_device_attribute soc_msi_matches[] = {
> { .family = "QorIQ LS1021A",
> .data = &ls1021_msi_cfg },
> { .family = "QorIQ LS1012A",
> .data = &ls1021_msi_cfg },
> { .family = "QorIQ LS1043A", .revision = "1.0",
> .data = &ls1021_msi_cfg },
> { .family = "QorIQ LS1043A", .revision = "1.1",
> .data = &ls1043_rev11_msi_cfg },
> { .family = "QorIQ LS1046A",
> .data = &ls1046_msi_cfg },
> { },
> };
>
> I will remain the SoC related compatible and try to describe the
> difference via some kind of the property.
As commented on the driver side, this should be described with DT
properties on the devices.
Thanks,
Mark.
