Re: [PATCH v2 5/6] ARM: dts: sun8i: s3: Add dts for the Elimo Initium SBC

[Matteo Scordino]

-- 
Matteo Scordino / Embedded Software Consultant
Mobile: +44 (0)7463701446

On Mon, 2020-11-02 at 11:06 +0100, Maxime Ripard wrote:
> On Fri, Oct 30, 2020 at 11:43:24PM +0000, Matteo Scordino wrote:
> > The Elimo Engineering Initium is an Open Source Hardware Single Board
> > Computer based on the Elimo Impetus SoM.
> > 
> > It is meant as the first development platform for the Impetus, providing
> > convenient access to the peripherals on the Impetus.
> > 
> > It provides:
> > USB-C power input
> > UART-to-USB bridge on the USB-C connector, connected to UART1
> > USB-A connector for USB2.0 (Host, Device, OTG)
> > Audio Line In/Out
> > Pin header to access all signals on the M2 connector of the SoM
> > 
> > Signed-off-by: Matteo Scordino <matteo.scordino@xxxxxxxxx>
> > ---
> >  arch/arm/boot/dts/Makefile                   |  1 +
> >  arch/arm/boot/dts/sun8i-s3-elimo-initium.dts | 28 ++++++++++++++++++++
> >  2 files changed, 29 insertions(+)
> >  create mode 100644 arch/arm/boot/dts/sun8i-s3-elimo-initium.dts
> > 
> > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> > index 4f0adfead547..dcfb8d39c267 100644
> > --- a/arch/arm/boot/dts/Makefile
> > +++ b/arch/arm/boot/dts/Makefile
> > @@ -1210,6 +1210,7 @@ dtb-$(CONFIG_MACH_SUN8I) += \
> >  	sun8i-r40-bananapi-m2-ultra.dtb \
> >  	sun8i-s3-lichee-zero-plus.dtb \
> >  	sun8i-s3-pinecube.dtb \
> > +	sun8i-s3-elimo-initium.dtb \
> 
> This should be sorted as well

Ouch. Will do.

> 
> >  	sun8i-t3-cqa3t-bv3.dtb \
> >  	sun8i-v3s-licheepi-zero.dtb \
> >  	sun8i-v3s-licheepi-zero-dock.dtb \
> > diff --git a/arch/arm/boot/dts/sun8i-s3-elimo-initium.dts b/arch/arm/boot/dts/sun8i-s3-elimo-initium.dts
> > new file mode 100644
> > index 000000000000..7677ddc07bf9
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/sun8i-s3-elimo-initium.dts
> > @@ -0,0 +1,28 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Copyright (C) 2020 Matteo Scordino <matteo@xxxxxxxx>
> > + */
> > +
> > +/dts-v1/;
> > +#include "sun8i-s3-elimo-impetus.dtsi"
> > +
> > +/ {
> > +	model = "Elimo Initium";
> > +	compatible = "elimo,initium", "elimo,impetus", "sochip,s3",
> > +    "allwinner,sun8i-v3";
> 
> You should indent that properly

Double ouch. 
However, how is a multiline dt property supposed to be indented? 
I have tried a few combinations but nothing seemed to make checkpatch.pl happy,
except leaving no space at all at the beginning of the line. But I think makes it
quite unreadable?

> > +	aliases {
> > +		serial0 = &uart0;
> > +		serial1 = &uart1;
> > +	};
> > +
> > +	chosen {
> > +		stdout-path = "serial0:115200n8";
> > +	};
> > +};
> 
> It's already in the DTSI, there's no need to duplicate it here

After your question about the UART1 in the Impetus dtsi, I think it makes more
sense to remove it from the dtsi, since UART1 is directly used in this board (for
an FTDI chip) but not necessarily used in the "bare" Impetus SoM.

> Maxime

Attachment: signature.asc
Description: This is a digitally signed message part