Re: [PATCH v2 1/1] arm: dts: sunxi: Add ICnova A20 ADB4006 board support

[Ludwig Kormann]

Hi,

thanks for your review.

Am 19.04.23 um 15:05 schrieb Krzysztof Kozlowski:
> On 19/04/2023 14:12, Ludwig Kormann wrote:
> > Add board support for ICnova A20 SomPi compute module on
> > ICnova ADB4006 development board.
> >
> > Specification:
> > SoM
> > - Processor: Allwinner A20 Cortex-A7 Dual Core at 1GHz
> > - 512MB DDR3 RAM
> > - Fast Ethernet (Phy: Realtek RTL8201CP)
> > ADB4006
> > - I2C
> > - 2x USB 2.0
> > - 1x Fast Ethernet port
> > - 1x SATA
> > - 2x buttons (PWRON, Boot)
> > - 2x LEDS
> > - serial console
> > - HDMI
> > - µSD-Card slot
> > - Audio Line-In / Line-Out
> > - GPIO pinheaders
> >
> > https://wiki.in-circuit.de/index.php5?title=ICnova_ADB4006
> > https://wiki.in-circuit.de/index.php5?title=ICnova_A20_SODIMM
> >
> > ---
> >
> > changes in v2:
> > - use short licensing header
> > - remove deprecated elements from led nodes
> > - disable csi power supply
> > - add missing pins in usbphy node
> > - split dts into SoM dtsi and carrier board dts
> >
> > v1 of this patch was sent to the uboot mailing list [1].
> >
> > [1] https://lists.denx.de/pipermail/u-boot/2023-April/514605.html
> >
> > Signed-off-by: Ludwig Kormann <ludwig.kormann@xxxxxxxxxxxxx>
> > ---
> >   .../devicetree/bindings/arm/sunxi.yaml        |   6 +
> Bindings are always separate patches. checkpatch did not complain?

I just ran checkpatch.pl, you're right, it does complain. I will move 
the bindings to a seperate patch.

> > arch/arm/boot/dts/Makefile                    |   1 +
> >   .../boot/dts/sun7i-a20-icnova-a20-adb4006.dts | 137 ++++++++++++++++++
> >   arch/arm/boot/dts/sun7i-a20-icnova-a20.dtsi   |  63 ++++++++
> >   4 files changed, 207 insertions(+)
> >   create mode 100644 arch/arm/boot/dts/sun7i-a20-icnova-a20-adb4006.dts
> >   create mode 100644 arch/arm/boot/dts/sun7i-a20-icnova-a20.dtsi
> >
> > diff --git a/Documentation/devicetree/bindings/arm/sunxi.yaml 
> > b/Documentation/devicetree/bindings/arm/sunxi.yaml
> > index 013821f4a7b8..12f0c236f17b 100644
> > --- a/Documentation/devicetree/bindings/arm/sunxi.yaml
> > +++ b/Documentation/devicetree/bindings/arm/sunxi.yaml
> > @@ -305,6 +305,12 @@ properties:
> >             - const: allwinner,i12-tvbox
> >             - const: allwinner,sun7i-a20
> >   +      - description: ICNova A20 ADB4006
> > +        items:
> > +          - const: incircuit,icnova-a20-adb4006
> > +          - const: incircuit,icnova-a20
> > +          - const: allwinner,sun7i-a20
> > +
> >         - description: ICNova A20 SWAC
> >           items:
> >             - const: incircuit,icnova-a20-swac
> > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> > index 3cc32722c394..b6b408417261 100644
> > --- a/arch/arm/boot/dts/Makefile
> > +++ b/arch/arm/boot/dts/Makefile
> > @@ -1321,6 +1321,7 @@ dtb-$(CONFIG_MACH_SUN7I) += \
> >       sun7i-a20-hummingbird.dtb \
> >       sun7i-a20-itead-ibox.dtb \
> >       sun7i-a20-i12-tvbox.dtb \
> > +    sun7i-a20-icnova-a20-adb4006.dtb \
> >       sun7i-a20-icnova-swac.dtb \
> >       sun7i-a20-lamobo-r1.dtb \
> >       sun7i-a20-linutronix-testbox-v2.dtb \
> > diff --git a/arch/arm/boot/dts/sun7i-a20-icnova-a20-adb4006.dts 
> > b/arch/arm/boot/dts/sun7i-a20-icnova-a20-adb4006.dts
> > new file mode 100644
> > index 000000000000..c1606c085e4e
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/sun7i-a20-icnova-a20-adb4006.dts
> > @@ -0,0 +1,137 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> Unusual license. Are you sure you are ok with GPLv5.0?

Thanks for the hint. I will remove the '+' and update the licensing to 
"GPL-2.0 OR MIT".

> Also, at the end of your files - drop stray blank lines.

I will remove them.


I will implement the changes and provide patch series v3.

kind regards,
Ludwig

> Best regards,
> Krzysztof