Re: [PATCH] dts: nxp: mxs: Add descriptions for imx287 based btt3-[012] devices

[Lukasz Majewski <lukma@denx.de>]

[-- Attachment #1: Type: text/plain, Size: 3846 bytes --]

Hi Rob,

> On Wed, Aug 28, 2024 at 9:12 AM Lukasz Majewski <lukma@denx.de> wrote:
> >
> > Hi Rob,
> >  
> > > On Wed, 28 Aug 2024 13:21:39 +0200, Lukasz Majewski wrote:  
> > > > The btt3 device' HW revisions from 0 to 2 use imx287 SoC and
> > > > are to some extend similar to already upstreamed XEA devices,
> > > > hence are using common imx28-lwe.dtsi file.
> > > >
> > > > New, btt3.dtsi has been added to embrace common DTS properties
> > > > for different HW revisions for this device.
> > > >
> > > > As a result - changes introduced in btt3-[012].dts are minimal.
> > > >
> > > > Signed-off-by: Lukasz Majewski <lukma@denx.de>
> > > > ---
> > > >  arch/arm/boot/dts/nxp/mxs/Makefile   |   3 +
> > > >  arch/arm/boot/dts/nxp/mxs/btt3-0.dts |  12 +
> > > >  arch/arm/boot/dts/nxp/mxs/btt3-1.dts |   8 +
> > > >  arch/arm/boot/dts/nxp/mxs/btt3-2.dts |  12 +
> > > >  arch/arm/boot/dts/nxp/mxs/btt3.dtsi  | 320
> > > > +++++++++++++++++++++++++++ 5 files changed, 355 insertions(+)
> > > >  create mode 100644 arch/arm/boot/dts/nxp/mxs/btt3-0.dts
> > > >  create mode 100644 arch/arm/boot/dts/nxp/mxs/btt3-1.dts
> > > >  create mode 100644 arch/arm/boot/dts/nxp/mxs/btt3-2.dts
> > > >  create mode 100644 arch/arm/boot/dts/nxp/mxs/btt3.dtsi
> > > >  
> > >
> > >
> > > My bot found new DTB warnings on the .dts files added or changed
> > > in this series.
> > >
> > > Some warnings may be from an existing SoC .dtsi. Or perhaps the
> > > warnings are fixed by another series. Ultimately, it is up to the
> > > platform maintainer whether these warnings are acceptable or not.
> > > No need to reply unless the platform maintainer has comments.
> > >  
> >
> > There a few things to clarify after:
> > CROSS_COMPILE="arm-none-linux-gnueabihf-"
> > ARCH=arm make -j4 CHECK_DTBS=y nxp/mxs/btt3-0.dtb
> >
> > 1. mxs-spi.yaml vs mxs-mmc.yaml -> both refer to the same set of HW
> > - as those IP blocks can work as both mmc and SPI.  
> 
> Sounds like there should be 1 binding then.

I also think so - one binding with two compatibles.

> 
> >
> > 2. For the sound codec:
> >  codec@1a: compatible: ['wlf,wm8940', 'wlf,wm8974'] is too long
> >
> > This is a bit problematic as well - as WM8974 doesn't have ANY ID
> > register, so it needs to be probed manually and decided which module
> > with sound codec needs to be inserted. The WM8940 is the pin
> > compatible replacement.  
> 
> So if the OS has 2 drivers for these 2 devices, which one does it
> pick?

(For now) user space assess if the correct codec is inserted as a
module.

> The answer for Linux is it is undefined. The expectation is if
> there are multiple options, the firmware figures out which one is
> present and adjusts the DT.

So then, only the newest one would be used, and then I would move
detection to u-boot and use overlays.

> 
> > 3. The simple-bus.yaml seems to be missing in the Linux kernel tree
> > - problem is that imx28 has 'apbx' bus.  
> 
> simple-bus.yaml lives in dtschema repo.
> 

Yes, I've just noticed that.

> Node names should be generic, not named for the implementation/type.
> The binding is fairly flexible in allowing ".*-bus".

Fabio has just prepared and shared proper patch.

> 
> > 4. The fsl,lcdif.yaml doesn't allow the situation when one specify
> > by hand (in DTS) display parameters (as different ones can be used
> > by the manufacturing site).  
> 
> Shrug. At the end of the day, the binding and dts have to match. One
> (or both) has to be adjusted.

Ok.

> 
> Rob


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]