Re: [PATCH 03/11] arm64: dts: renesas: hihope-common: Separate out Rev.2.0 specific into hihope-common-rev2.dtsi file

[Geert Uytterhoeven]

Hi Prabhakar,

On Tue, Jun 23, 2020 at 9:51 AM Lad, Prabhakar
<prabhakar.csengg@xxxxxxxxx> wrote:
> On Mon, Jun 8, 2020 at 3:59 PM Lad, Prabhakar
> <prabhakar.csengg@xxxxxxxxx> wrote:
> > On Mon, Jun 8, 2020 at 3:47 PM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
> > > On Sun, Jun 7, 2020 at 8:41 PM Lad Prabhakar
> > > <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> wrote:
> > > > Separate out Rev.2.0 specific hardware changes into
> > > > hihope-common-rev2.dtsi file so that hihope-common.dtsi can be used
> > > > by all the variants for RZ/G2M[N] boards.
> > > >
> > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> > > > Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@xxxxxxxxxxxxxx>

> > > > @@ -0,0 +1,101 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * Device Tree Source for the HiHope RZ/G2[MN] main board Rev.2.0 common
> > > > + * parts
> > > > + *
> > > > + * Copyright (C) 2020 Renesas Electronics Corp.
> > > > + */
> > > > +
> > > > +#include <dt-bindings/gpio/gpio.h>
> > > > +
> > > > +/ {
> > > > +       leds {
> > > > +               compatible = "gpio-leds";
> > > > +
> > > > +               bt_active_led {
> > > > +                       label = "blue:bt";
> > > > +                       gpios = <&gpio7  0 GPIO_ACTIVE_HIGH>;
> > > > +                       linux,default-trigger = "hci0-power";
> > > > +                       default-state = "off";
> > > > +               };
> > > > +
> > > > +               led0 {
> > > > +                       gpios = <&gpio6 11 GPIO_ACTIVE_HIGH>;
> > > > +               };
> > > > +
> > > > +               led1 {
> > > > +                       gpios = <&gpio6 12 GPIO_ACTIVE_HIGH>;
> > > > +               };
> > > > +
> > > > +               led2 {
> > > > +                       gpios = <&gpio6 13 GPIO_ACTIVE_HIGH>;
> > > > +               };
> > > > +
> > > > +               led3 {
> > > > +                       gpios = <&gpio0  0 GPIO_ACTIVE_HIGH>;
> > > > +               };
> > >
> > > led1, led2, and led3 are present on both, so I'd keep them in
> > > hihope-common.dtsi.
> > >
> > The leds defined in hihope-common-rev4.dtsi are as per the label names
> > on the schematics/board so that it's easier to identify the LED's by
> > name.
> >
> I was waiting on the above to be confirmed.

I can confirm the naming of the LEDs on the rev4 board.
However, following the same reasoning, the rev2 LEDs should be renamed
led2201, led2202, led2203, and led2402 ;-)
Does anyone rely on the names?  If not, it may make sense to use the
rev4 names for both, in the common file?

Not even considering the switches...
Seems they forgot to rename switches SW220[123] when renaming LED220[123].
Worse, on rev2, you have SW220_2_/LED220_1_ sharing a GPIO, and
SW220_1_/LED220_2_ sharing another one.

And on rev4, GP6_11/GP_LED/TSW_0_ is driving LED_4_ and SW220_2_?

Conclusion: I don't care how you name them ;-)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds