[1/4] arm64: dts: beacon-renesom: Fix gpio expander reference

  The board used to originally introduce the Beacon Embedded
RZ/G2[M/N/H] boards had a GPIO expander with address 20, but
this was change when the final board went to production.

The production boards changed both the part itself and
the address.  With the incorrect address, the LCD cannot
come up.  If the LCD fails, the rcar-du driver fails to come up,
and that also breaks HDMI.

Pre-release board were not shipped to the general public, so it
should be safe to push this as a fix.  Anyone with a production
board would have video fail due to this GPIO expander change.

Fixes: a1d8a344f1ca ("arm64: dts: renesas: Introduce r8a774a1-beacon-rzg2m-kit")
Signed-off-by: Adam Ford <aford173@gmail.com>
  

On Wed, Jan 4, 2023 at 8:12 AM Adam Ford <aford173@gmail.com> wrote:
>
> The board used to originally introduce the Beacon Embedded
> RZ/G2[M/N/H] boards had a GPIO expander with address 20, but
> this was change when the final board went to production.
>
> The production boards changed both the part itself and
> the address.  With the incorrect address, the LCD cannot
> come up.  If the LCD fails, the rcar-du driver fails to come up,
> and that also breaks HDMI.
>
> Pre-release board were not shipped to the general public, so it
> should be safe to push this as a fix.  Anyone with a production
> board would have video fail due to this GPIO expander change.
>
Geert,

Is this patch OK as-is? This is more critical to me than the other
patches in the series.

adam

> Fixes: a1d8a344f1ca ("arm64: dts: renesas: Introduce r8a774a1-beacon-rzg2m-kit")
> Signed-off-by: Adam Ford <aford173@gmail.com>
>
> diff --git a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> index 8166e3c1ff4e..8b6fe235a8f0 100644
> --- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> @@ -437,20 +437,6 @@ wm8962_endpoint: endpoint {
>                 };
>         };
>
> -       /* 0 - lcd_reset */
> -       /* 1 - lcd_pwr */
> -       /* 2 - lcd_select */
> -       /* 3 - backlight-enable */
> -       /* 4 - Touch_shdwn */
> -       /* 5 - LCD_H_pol */
> -       /* 6 - lcd_V_pol */
> -       gpio_exp1: gpio@20 {
> -               compatible = "onnn,pca9654";
> -               reg = <0x20>;
> -               gpio-controller;
> -               #gpio-cells = <2>;
> -       };
> -
>         touchscreen@26 {
>                 compatible = "ilitek,ili2117";
>                 reg = <0x26>;
> @@ -482,6 +468,21 @@ hd3ss3220_out_ep: endpoint {
>                         };
>                 };
>         };
> +
> +       gpio_exp1: gpio@70 {
> +               compatible = "onnn,pca9654";
> +               reg = <0x70>;
> +               gpio-controller;
> +               #gpio-cells = <2>;
> +               gpio-line-names =
> +                       "lcd_reset",
> +                       "lcd_pwr",
> +                       "lcd_select",
> +                       "backlight-enable",
> +                       "Touch_shdwn",
> +                       "LCD_H_pol",
> +                       "lcd_V_pol";
> +       };
>  };
>
>  &lvds0 {
> --
> 2.34.1
>
  

Hi Adam,

On Wed, Jan 4, 2023 at 3:12 PM Adam Ford <aford173@gmail.com> wrote:
> The board used to originally introduce the Beacon Embedded
> RZ/G2[M/N/H] boards had a GPIO expander with address 20, but
> this was change when the final board went to production.
>
> The production boards changed both the part itself and
> the address.  With the incorrect address, the LCD cannot
> come up.  If the LCD fails, the rcar-du driver fails to come up,
> and that also breaks HDMI.
>
> Pre-release board were not shipped to the general public, so it
> should be safe to push this as a fix.  Anyone with a production
> board would have video fail due to this GPIO expander change.
>
> Fixes: a1d8a344f1ca ("arm64: dts: renesas: Introduce r8a774a1-beacon-rzg2m-kit")
> Signed-off-by: Adam Ford <aford173@gmail.com>

Thanks for your patch!

> --- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> @@ -437,20 +437,6 @@ wm8962_endpoint: endpoint {
>                 };
>         };
>
> -       /* 0 - lcd_reset */
> -       /* 1 - lcd_pwr */
> -       /* 2 - lcd_select */
> -       /* 3 - backlight-enable */
> -       /* 4 - Touch_shdwn */
> -       /* 5 - LCD_H_pol */
> -       /* 6 - lcd_V_pol */
> -       gpio_exp1: gpio@20 {
> -               compatible = "onnn,pca9654";
> -               reg = <0x20>;
> -               gpio-controller;
> -               #gpio-cells = <2>;
> -       };
> -
>         touchscreen@26 {
>                 compatible = "ilitek,ili2117";
>                 reg = <0x26>;
> @@ -482,6 +468,21 @@ hd3ss3220_out_ep: endpoint {
>                         };
>                 };
>         };
> +
> +       gpio_exp1: gpio@70 {
> +               compatible = "onnn,pca9654";

According to the patch description, the actual part was changed, too?

> +               reg = <0x70>;
> +               gpio-controller;
> +               #gpio-cells = <2>;
> +               gpio-line-names =
> +                       "lcd_reset",
> +                       "lcd_pwr",
> +                       "lcd_select",
> +                       "backlight-enable",
> +                       "Touch_shdwn",
> +                       "LCD_H_pol",
> +                       "lcd_V_pol";
> +       };
>  };

The rest LGTM.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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
  

On Wed, Jan 11, 2023 at 10:29 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Hi Adam,
>
> On Wed, Jan 4, 2023 at 3:12 PM Adam Ford <aford173@gmail.com> wrote:
> > The board used to originally introduce the Beacon Embedded
> > RZ/G2[M/N/H] boards had a GPIO expander with address 20, but
> > this was change when the final board went to production.
> >
> > The production boards changed both the part itself and
> > the address.  With the incorrect address, the LCD cannot
> > come up.  If the LCD fails, the rcar-du driver fails to come up,
> > and that also breaks HDMI.
> >
> > Pre-release board were not shipped to the general public, so it
> > should be safe to push this as a fix.  Anyone with a production
> > board would have video fail due to this GPIO expander change.
> >
> > Fixes: a1d8a344f1ca ("arm64: dts: renesas: Introduce r8a774a1-beacon-rzg2m-kit")
> > Signed-off-by: Adam Ford <aford173@gmail.com>
>
> Thanks for your patch!
>
> > --- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > @@ -437,20 +437,6 @@ wm8962_endpoint: endpoint {
> >                 };
> >         };
> >
> > -       /* 0 - lcd_reset */
> > -       /* 1 - lcd_pwr */
> > -       /* 2 - lcd_select */
> > -       /* 3 - backlight-enable */
> > -       /* 4 - Touch_shdwn */
> > -       /* 5 - LCD_H_pol */
> > -       /* 6 - lcd_V_pol */
> > -       gpio_exp1: gpio@20 {
> > -               compatible = "onnn,pca9654";
> > -               reg = <0x20>;
> > -               gpio-controller;
> > -               #gpio-cells = <2>;
> > -       };
> > -
> >         touchscreen@26 {
> >                 compatible = "ilitek,ili2117";
> >                 reg = <0x26>;
> > @@ -482,6 +468,21 @@ hd3ss3220_out_ep: endpoint {
> >                         };
> >                 };
> >         };
> > +
> > +       gpio_exp1: gpio@70 {
> > +               compatible = "onnn,pca9654";
>
> According to the patch description, the actual part was changed, too?

humm.. I was under the impression it was after talking with one of the
hardware developers, but clearly it's using the same compatible name.
I will investigate this further.  One way or the other, I'll redo the
commit message.

adam
>
> > +               reg = <0x70>;
> > +               gpio-controller;
> > +               #gpio-cells = <2>;
> > +               gpio-line-names =
> > +                       "lcd_reset",
> > +                       "lcd_pwr",
> > +                       "lcd_select",
> > +                       "backlight-enable",
> > +                       "Touch_shdwn",
> > +                       "LCD_H_pol",
> > +                       "lcd_V_pol";
> > +       };
> >  };
>
> The rest LGTM.
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> 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
  

On Wed, Jan 11, 2023 at 11:05 AM Adam Ford <aford173@gmail.com> wrote:
>
> On Wed, Jan 11, 2023 at 10:29 AM Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> >
> > Hi Adam,
> >
> > On Wed, Jan 4, 2023 at 3:12 PM Adam Ford <aford173@gmail.com> wrote:
> > > The board used to originally introduce the Beacon Embedded
> > > RZ/G2[M/N/H] boards had a GPIO expander with address 20, but
> > > this was change when the final board went to production.
> > >
> > > The production boards changed both the part itself and
> > > the address.  With the incorrect address, the LCD cannot
> > > come up.  If the LCD fails, the rcar-du driver fails to come up,
> > > and that also breaks HDMI.
> > >
> > > Pre-release board were not shipped to the general public, so it
> > > should be safe to push this as a fix.  Anyone with a production
> > > board would have video fail due to this GPIO expander change.
> > >
> > > Fixes: a1d8a344f1ca ("arm64: dts: renesas: Introduce r8a774a1-beacon-rzg2m-kit")
> > > Signed-off-by: Adam Ford <aford173@gmail.com>
> >
> > Thanks for your patch!
> >
> > > --- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > @@ -437,20 +437,6 @@ wm8962_endpoint: endpoint {
> > >                 };
> > >         };
> > >
> > > -       /* 0 - lcd_reset */
> > > -       /* 1 - lcd_pwr */
> > > -       /* 2 - lcd_select */
> > > -       /* 3 - backlight-enable */
> > > -       /* 4 - Touch_shdwn */
> > > -       /* 5 - LCD_H_pol */
> > > -       /* 6 - lcd_V_pol */
> > > -       gpio_exp1: gpio@20 {
> > > -               compatible = "onnn,pca9654";
> > > -               reg = <0x20>;
> > > -               gpio-controller;
> > > -               #gpio-cells = <2>;
> > > -       };
> > > -
> > >         touchscreen@26 {
> > >                 compatible = "ilitek,ili2117";
> > >                 reg = <0x26>;
> > > @@ -482,6 +468,21 @@ hd3ss3220_out_ep: endpoint {
> > >                         };
> > >                 };
> > >         };
> > > +
> > > +       gpio_exp1: gpio@70 {
> > > +               compatible = "onnn,pca9654";
> >
> > According to the patch description, the actual part was changed, too?

After a bit of some investigation, the part was changed to
nxp,pca9538.  It appears to be the same driver and the same ".data"
structure is used.  This probably explains why I didn't see any issues
when I was testing this.  Unfortunately, the hardware engineer who
made this hardware change is not around to ask why the change was
made.    I'll do a V2 since, I don't see this in renesas-devel.

adam
>
> humm.. I was under the impression it was after talking with one of the
> hardware developers, but clearly it's using the same compatible name.
> I will investigate this further.  One way or the other, I'll redo the
> commit message.
>
> adam
> >
> > > +               reg = <0x70>;
> > > +               gpio-controller;
> > > +               #gpio-cells = <2>;
> > > +               gpio-line-names =
> > > +                       "lcd_reset",
> > > +                       "lcd_pwr",
> > > +                       "lcd_select",
> > > +                       "backlight-enable",
> > > +                       "Touch_shdwn",
> > > +                       "LCD_H_pol",
> > > +                       "lcd_V_pol";
> > > +       };
> > >  };
> >
> > The rest LGTM.
> >
> > Gr{oetje,eeting}s,
> >
> >                         Geert
> >
> > --
> > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> >
> > 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
  

Hi Adam,

On Fri, Jan 13, 2023 at 2:17 PM Adam Ford <aford173@gmail.com> wrote:
> On Wed, Jan 11, 2023 at 11:05 AM Adam Ford <aford173@gmail.com> wrote:
> > On Wed, Jan 11, 2023 at 10:29 AM Geert Uytterhoeven
> > <geert@linux-m68k.org> wrote:
> > > On Wed, Jan 4, 2023 at 3:12 PM Adam Ford <aford173@gmail.com> wrote:
> > > > The board used to originally introduce the Beacon Embedded
> > > > RZ/G2[M/N/H] boards had a GPIO expander with address 20, but
> > > > this was change when the final board went to production.
> > > >
> > > > The production boards changed both the part itself and
> > > > the address.  With the incorrect address, the LCD cannot
> > > > come up.  If the LCD fails, the rcar-du driver fails to come up,
> > > > and that also breaks HDMI.
> > > >
> > > > Pre-release board were not shipped to the general public, so it
> > > > should be safe to push this as a fix.  Anyone with a production
> > > > board would have video fail due to this GPIO expander change.
> > > >
> > > > Fixes: a1d8a344f1ca ("arm64: dts: renesas: Introduce r8a774a1-beacon-rzg2m-kit")
> > > > Signed-off-by: Adam Ford <aford173@gmail.com>

> > > > --- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > > +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi
> > > > @@ -437,20 +437,6 @@ wm8962_endpoint: endpoint {
> > > >                 };
> > > >         };
> > > >
> > > > -       /* 0 - lcd_reset */
> > > > -       /* 1 - lcd_pwr */
> > > > -       /* 2 - lcd_select */
> > > > -       /* 3 - backlight-enable */
> > > > -       /* 4 - Touch_shdwn */
> > > > -       /* 5 - LCD_H_pol */
> > > > -       /* 6 - lcd_V_pol */
> > > > -       gpio_exp1: gpio@20 {
> > > > -               compatible = "onnn,pca9654";
> > > > -               reg = <0x20>;
> > > > -               gpio-controller;
> > > > -               #gpio-cells = <2>;
> > > > -       };
> > > > -
> > > >         touchscreen@26 {
> > > >                 compatible = "ilitek,ili2117";
> > > >                 reg = <0x26>;
> > > > @@ -482,6 +468,21 @@ hd3ss3220_out_ep: endpoint {
> > > >                         };
> > > >                 };
> > > >         };
> > > > +
> > > > +       gpio_exp1: gpio@70 {
> > > > +               compatible = "onnn,pca9654";
> > >
> > > According to the patch description, the actual part was changed, too?
>
> After a bit of some investigation, the part was changed to
> nxp,pca9538.  It appears to be the same driver and the same ".data"
> structure is used.  This probably explains why I didn't see any issues
> when I was testing this.  Unfortunately, the hardware engineer who
> made this hardware change is not around to ask why the change was
> made.

Yeah, there are lots of compatible parts.

> I'll do a V2 since, I don't see this in renesas-devel.

Correct, I was waiting for clarification.
Looking forward to v2...

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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