RE: [PATCH] arm64: dts: renesas: rzv2mevk2: Fix eMMC/SDHI pinctrl names

[Fabrizio Castro]

Hi Geert,


Thanks for your reply.

> From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Subject: Re: [PATCH] arm64: dts: renesas: rzv2mevk2: Fix eMMC/SDHI
> pinctrl names
> 
> Hi Fabrizio,
> 
> CC pinctrl
> 
> Thanks for your patch!
> 
> On Fri, Jun 23, 2023 at 10:01 AM Fabrizio Castro
> <fabrizio.castro.jz@xxxxxxxxxxx> wrote:
> > The original commit uses the same names ("data" and "ctrl")
> > for the subnodes of pinctrl definitions for both eMMC and
> > SDHI0 (emmc_pins, sdhi0_pins, and sdhi0_pins_uhs) leading to
> 
> That should be fine, as the parents of these subnodes do have
> different
> names?

That's what I originally thought as well.

> 
> > the below issue:
> >
> > pinctrl-rzv2m b6250000.pinctrl: pin P8_2 already requested by
> 85000000.mmc; cannot claim for 85020000.mmc
> > pinctrl-rzv2m b6250000.pinctrl: pin-130 (85020000.mmc) status -22
> > renesas_sdhi_internal_dmac 85020000.mmc: Error applying setting,
> reverse things back
> 
> To me, that sounds like a bug in the pinctrl core.
> Or am I missing something?

I am not sure if this behaviour is intended or not, I'll probably
have a deeper look into it later on if I have time, but right now
(with the current state of things) this patch is necessary.
It doesn't hurt to have unique names for subnodes, and looking
through Renesas device trees, the names for the subnodes are
unique for the other platforms.

By the way, I didn't see the problem when I tested the original
patch (I still have the logs of the original testing, that was based
6.2.0-rc3+, and all was working well), therefore it'd be good to look
further into this later on, for future reference.

Cheers,
Fab

> 
> > This commit fixes the problem by making the names for the
> > pinctrl subnodes of eMMC and SDHI0 unique.
> >
> > Fixes: b6c0be722b0c ("arm64: dts: renesas: rzv2mevk2: Add uSD card
> and eMMC support")
> > Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@xxxxxxxxxxx>
> > ---
> >  .../arm64/boot/dts/renesas/r9a09g011-v2mevk2.dts | 16 ++++++++-----
> ---
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/renesas/r9a09g011-v2mevk2.dts
> b/arch/arm64/boot/dts/renesas/r9a09g011-v2mevk2.dts
> > index 39fe3f94991e..11c5cffea5a5 100644
> > --- a/arch/arm64/boot/dts/renesas/r9a09g011-v2mevk2.dts
> > +++ b/arch/arm64/boot/dts/renesas/r9a09g011-v2mevk2.dts
> > @@ -167,7 +167,7 @@ &i2c2 {
> >
> >  &pinctrl {
> >         emmc_pins: emmc {
> > -               data {
> > +               emmc_data {
> >                         pinmux = <RZV2M_PORT_PINMUX(0, 0, 2)>, /*
> MMDAT0 */
> >                                  <RZV2M_PORT_PINMUX(0, 1, 2)>, /*
> MMDAT1 */
> >                                  <RZV2M_PORT_PINMUX(0, 2, 2)>, /*
> MMDAT2 */
> > @@ -179,7 +179,7 @@ data {
> >                         power-source = <1800>;
> >                 };
> >
> > -               ctrl {
> > +               emmc_ctrl {
> >                         pinmux = <RZV2M_PORT_PINMUX(0, 10, 2)>, /*
> MMCMD */
> >                                  <RZV2M_PORT_PINMUX(0, 11, 2)>; /*
> MMCLK */
> >                         power-source = <1800>;
> > @@ -197,7 +197,7 @@ i2c2_pins: i2c2 {
> >         };
> >
> >         sdhi0_pins: sd0 {
> > -               data {
> > +               sd0_data {
> >                         pinmux = <RZV2M_PORT_PINMUX(8, 2, 1)>, /*
> SD0DAT0 */
> >                                  <RZV2M_PORT_PINMUX(8, 3, 1)>, /*
> SD0DAT1 */
> >                                  <RZV2M_PORT_PINMUX(8, 4, 1)>, /*
> SD0DAT2 */
> > @@ -205,20 +205,20 @@ data {
> >                         power-source = <3300>;
> >                 };
> >
> > -               ctrl {
> > +               sd0_ctrl {
> >                         pinmux = <RZV2M_PORT_PINMUX(8, 0, 1)>, /*
> SD0CMD */
> >                                  <RZV2M_PORT_PINMUX(8, 1, 1)>; /*
> SD0CLK */
> >                         power-source = <3300>;
> >                 };
> >
> > -               cd {
> > +               sd0_cd {
> >                         pinmux = <RZV2M_PORT_PINMUX(8, 7, 1)>; /*
> SD0CD */
> >                         power-source = <3300>;
> >                 };
> >         };
> >
> >         sdhi0_pins_uhs: sd0-uhs {
> > -               data {
> > +               sd0_uhs_data {
> >                         pinmux = <RZV2M_PORT_PINMUX(8, 2, 1)>, /*
> SD0DAT0 */
> >                                  <RZV2M_PORT_PINMUX(8, 3, 1)>, /*
> SD0DAT1 */
> >                                  <RZV2M_PORT_PINMUX(8, 4, 1)>, /*
> SD0DAT2 */
> > @@ -226,13 +226,13 @@ data {
> >                         power-source = <1800>;
> >                 };
> >
> > -               ctrl {
> > +               sd0_uhs_ctrl {
> >                         pinmux = <RZV2M_PORT_PINMUX(8, 0, 1)>, /*
> SD0CMD */
> >                                  <RZV2M_PORT_PINMUX(8, 1, 1)>; /*
> SD0CLK */
> >                         power-source = <1800>;
> >                 };
> >
> > -               cd {
> > +               sd0_uhs_cd {
> >                         pinmux = <RZV2M_PORT_PINMUX(8, 7, 1)>; /*
> SD0CD */
> >                         power-source = <1800>;
> >                 };
> 
> 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