[PATCH v2 1/1] arm64: dts: lx2160a: Fix the compatible string of LX2160A UART

[Li Yang]

On Tue, Aug 10, 2021 at 1:28 AM Leo Li <leoyang.li at nxp.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Leizhen (ThunderTown) <thunder.leizhen at huawei.com>
> > Sent: Monday, August 9, 2021 8:28 PM
> > To: Leo Li <leoyang.li at nxp.com>; Shawn Guo <shawnguo at kernel.org>; Rob
> > Herring <robh+dt at kernel.org>; Mark Kettenis <mark.kettenis at xs4all.nl>;
> > devicetree <devicetree at vger.kernel.org>; linux-arm-kernel <linux-arm-
> > kernel at lists.infradead.org>; linux-kernel <linux-kernel at vger.kernel.org>
> > Subject: Re: [PATCH v2 1/1] arm64: dts: lx2160a: Fix the compatible string of
> > LX2160A UART
> >
> >
> >
> > On 2021/8/10 6:52, Leo Li wrote:
> > >
> > >
> > >> -----Original Message-----
> > >> From: Zhen Lei <thunder.leizhen at huawei.com>
> > >> Sent: Tuesday, June 15, 2021 8:16 AM
> > >> To: Shawn Guo <shawnguo at kernel.org>; Leo Li <leoyang.li at nxp.com>;
> > Rob
> > >> Herring <robh+dt at kernel.org>; Mark Kettenis
> > >> <mark.kettenis at xs4all.nl>; devicetree <devicetree at vger.kernel.org>;
> > >> linux-arm-kernel <linux-arm- kernel at lists.infradead.org>;
> > >> linux-kernel <linux-kernel at vger.kernel.org>
> > >> Cc: Zhen Lei <thunder.leizhen at huawei.com>
> > >> Subject: [PATCH v2 1/1] arm64: dts: lx2160a: Fix the compatible
> > >> string of LX2160A UART
> > >>
> > >> Mark Kettenis told us that:
> > >> According to the NXP documentation, the LX2160A has a real PL011 UART.
> > >>
> > >> Therefore, rewrite it to the compatible string of pl011. The property
> > >> "current- speed" specific to "arm,sbsa-uart" is also deleted.
> > >
> > > Sorry that I missed the discussion on the v1.  But looks like this change
> > breaks the LX2160 boot.  The AMBA matching doesn't seem to work.  And
> > the console is not registered correctly.
> >
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.k
> > ernel.org%2Flinux-arm-kernel%2Fcba3a29f-92b5-072a-9a27-
> > 60240f072dad%40huawei.com%2F&data=04%7C01%7Cleoyang.li%40nx
> > p.com%7C9986b52f71724d7f6ae108d95b9e1d9b%7C686ea1d3bc2b4c6fa92cd
> > 99c5c301635%7C0%7C0%7C637641556923909225%7CUnknown%7CTWFpbGZ
> > sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6M
> > n0%3D%7C1000&sdata=wReesSfMj1hV2iPTN0%2F%2B%2Fb%2BKJH8xF
> > LLcDgOMfjx731I%3D&reserved=0
> >
> > Maybe we should fall back to v1.
>
> I didn't look into the problem in detail.  Probably it is because of lacking the clock properties needed by the AMBA bus?
>

After reading through the history of sbsa-uart,  I think that I would
prefer us to fall back to v1 to be compatible with sbsa-uart rather
than changing it to be compatible with pl011.  Although the hardware
is actually compatible with the pl011, it would be better for us to
follow the newer SBSA standard to leave advanced configuration to
firmware and just expose the simple standard interface to OS.

Regards,
Leo

> >
> > >
> > > [    0.639055] OF: amba_device_add() failed (-2) for /soc/serial at 21c0000
> > > [    0.645612] OF: amba_device_add() failed (-2) for /soc/serial at 21d0000
> > >
> > >>
> > >> Suggested-by: Shawn Guo <shawnguo at kernel.org>
> > >> Suggested-by: Mark Kettenis <mark.kettenis at xs4all.nl>
> > >> Signed-off-by: Zhen Lei <thunder.leizhen at huawei.com>
> > >> ---
> > >>  arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi | 12 ++++--------
> > >>  1 file changed, 4 insertions(+), 8 deletions(-)
> > >>
> > >> diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > >> b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > >> index c4b1a59ba424..d2e6f7285674 100644
> > >> --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > >> +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > >> @@ -920,34 +920,30 @@ QORIQ_CLK_PLL_DIV(8)>,
> > >>            };
> > >>
> > >>            uart0: serial at 21c0000 {
> > >> -                  compatible = "arm,sbsa-uart","arm,pl011";
> > >> +                  compatible = "arm,pl011", "arm,primecell";
> > >>                    reg = <0x0 0x21c0000 0x0 0x1000>;
> > >>                    interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
> > >> -                  current-speed = <115200>;
> > >>                    status = "disabled";
> > >>            };
> > >>
> > >>            uart1: serial at 21d0000 {
> > >> -                  compatible = "arm,sbsa-uart","arm,pl011";
> > >> +                  compatible = "arm,pl011", "arm,primecell";
> > >>                    reg = <0x0 0x21d0000 0x0 0x1000>;
> > >>                    interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
> > >> -                  current-speed = <115200>;
> > >>                    status = "disabled";
> > >>            };
> > >>
> > >>            uart2: serial at 21e0000 {
> > >> -                  compatible = "arm,sbsa-uart","arm,pl011";
> > >> +                  compatible = "arm,pl011", "arm,primecell";
> > >>                    reg = <0x0 0x21e0000 0x0 0x1000>;
> > >>                    interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
> > >> -                  current-speed = <115200>;
> > >>                    status = "disabled";
> > >>            };
> > >>
> > >>            uart3: serial at 21f0000 {
> > >> -                  compatible = "arm,sbsa-uart","arm,pl011";
> > >> +                  compatible = "arm,pl011", "arm,primecell";
> > >>                    reg = <0x0 0x21f0000 0x0 0x1000>;
> > >>                    interrupts = <GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH>;
> > >> -                  current-speed = <115200>;
> > >>                    status = "disabled";
> > >>            };
> > >>
> > >> --
> > >> 2.25.1
> > >>
> > >
> > > .
> > >