Re: [PATCH] ARM: dts: vf610: Add ZII SPB4 board

[Andrey Smirnov]

On Tue, Mar 12, 2019 at 6:20 AM Fabio Estevam <festevam@xxxxxxxxx> wrote:
>
> Hi Andrey,
>
> On Mon, Mar 11, 2019 at 3:49 PM Andrey Smirnov <andrew.smirnov@xxxxxxxxx> wrote:
>
> > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> > index f4f5aeaf3298..035ad9fc49f3 100644
> > --- a/arch/arm/boot/dts/Makefile
> > +++ b/arch/arm/boot/dts/Makefile
> > @@ -607,7 +607,8 @@ dtb-$(CONFIG_SOC_VF610) += \
> >         vf610-zii-dev-rev-c.dtb \
> >         vf610-zii-scu4-aib.dtb \
> >         vf610-zii-ssmb-dtu.dtb \
> > -       vf610-zii-ssmb-spu3.dtb
> > +       vf610-zii-ssmb-spu3.dtb \
> > +       vf610-zii-spb4.dtb
>
> Please keep it in alphabetical order.
>

OK, will do.

> > +       gpio-leds {
> > +               compatible = "gpio-leds";
> > +               pinctrl-0 = <&pinctrl_leds_debug>;
> > +               pinctrl-names = "default";
> > +
> > +               led-debug {
> > +                       label = "zii:green:debug1";
> > +                       gpios = <&gpio2 18 GPIO_ACTIVE_HIGH>;
> > +                       linux,default-trigger = "heartbeat";
> > +                       max-brightness = <1>;
>
> This is an invalid property for gpio-leds. It is used for leds pwm
> only. Please remove it.
>

OK, will do.

> > +               };
> > +       };
> > +
> > +       reg_vcc_3v3_mcu: regulator {
>
> reg_vcc_3v3_mcu: regulator-vcc-3v3-mcu  {
>

Will do.

> > +&dspi1 {
> > +       bus-num = <1>;
> > +       pinctrl-names = "default";
> > +       pinctrl-0 = <&pinctrl_dspi1>;
> > +       status = "okay";
> > +
> > +       m25p128@0 {
>
> Node names should be generic:
>
> spi-nor@0
>

OK, will do. Although, "flash" seem to be more popular in ZII files
(used by RDU1 and RDU2), so I am inclined to use that instead, if you
don't mind.

> > +&i2c0 {
> > +       clock-frequency = <100000>;
> > +       pinctrl-names = "default";
> > +       pinctrl-0 = <&pinctrl_i2c0>;
> > +       status = "okay";
> > +
> > +       gpio6: pca9505@22 {
>
> io-expander@22
>

Will do.

> > +               compatible = "nxp,pca9554";
> > +               reg = <0x22>;
> > +               gpio-controller;
> > +               #gpio-cells = <2>;
> > +       };
> > +
> > +       at24c04@50 {
>
> eeprom@50
>

Will do.

> > +               compatible = "atmel,24c04";
> > +               reg = <0x50>;
> > +               label = "nameplate";
> > +       };
> > +
> > +       at24c04@52 {
>
> eeprom@52


Will do.

I am sure you've noticed that already, but just in case you didn't,
all of the non-generic names actually came from DTS for SPU3, so it
looks like we missed that when that file was upstreamed.

Thanks,
Andrey Smirnov