[PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering

[PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering

[marek.vasut]

From: Marek Vasut <marek.vasut+renesas@gmail.com>

There are two regulator1 nodes in the Ebisu DTS right now, one 3.3V for
the eMMC and one 12V for the backlight. This causes one to be overwritten
by the other, ultimatelly resulting in inoperable eMMC, which depends on
the former. Fix this by renumbering the backlight regulator to regulator2.

Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Cc: Simon Horman <horms+renesas@verge.net.au>
Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: linux-renesas-soc@vger.kernel.org
Reported-by: Simon Horman <horms+renesas@verge.net.au>
Fixes: 9d16c4a10e07 ("arm64: dts: renesas: r8a77990: ebisu: Add backlight")
---
 arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
index e0590c0af4c4..89383aa35d65 100644
--- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
@@ -191,7 +191,7 @@
 		clock-frequency = <24576000>;
 	};
 
-	reg_12p0v: regulator1 {
+	reg_12p0v: regulator2 {
 		compatible = "regulator-fixed";
 		regulator-name = "D12.0V";
 		regulator-min-microvolt = <12000000>;
-- 
2.19.2

[PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering

[marek.vasut]

From: Marek Vasut <marek.vasut+renesas@gmail.com>

There are two regulator1 nodes in the Ebisu DTS right now, one 3.3V for
the eMMC and one 12V for the backlight. This causes one to be overwritten
by the other, ultimatelly resulting in inoperable eMMC, which depends on
the former. Fix this by renumbering the backlight regulator to regulator2.

Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Cc: Simon Horman <horms+renesas@verge.net.au>
Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: linux-renesas-soc@vger.kernel.org
Reported-by: Simon Horman <horms+renesas@verge.net.au>
Fixes: 9d16c4a10e07 ("arm64: dts: renesas: r8a77990: ebisu: Add backlight")
---
 arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
index e0590c0af4c4..89383aa35d65 100644
--- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
@@ -191,7 +191,7 @@
 		clock-frequency = <24576000>;
 	};
 
-	reg_12p0v: regulator1 {
+	reg_12p0v: regulator2 {
 		compatible = "regulator-fixed";
 		regulator-name = "D12.0V";
 		regulator-min-microvolt = <12000000>;
-- 
2.19.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering

[Geert Uytterhoeven]

On Wed, Jan 9, 2019 at 3:01 PM <marek.vasut@gmail.com> wrote:
> From: Marek Vasut <marek.vasut+renesas@gmail.com>
>
> There are two regulator1 nodes in the Ebisu DTS right now, one 3.3V for
> the eMMC and one 12V for the backlight. This causes one to be overwritten
> by the other, ultimatelly resulting in inoperable eMMC, which depends on
> the former. Fix this by renumbering the backlight regulator to regulator2.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Cc: Simon Horman <horms+renesas@verge.net.au>
> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Cc: linux-renesas-soc@vger.kernel.org
> Reported-by: Simon Horman <horms+renesas@verge.net.au>
> Fixes: 9d16c4a10e07 ("arm64: dts: renesas: r8a77990: ebisu: Add backlight")

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

> --- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> @@ -191,7 +191,7 @@
>                 clock-frequency = <24576000>;
>         };
>
> -       reg_12p0v: regulator1 {
> +       reg_12p0v: regulator2 {
>                 compatible = "regulator-fixed";
>                 regulator-name = "D12.0V";
>                 regulator-min-microvolt = <12000000>;

Perhaps the node name should get a more descriptive suffix
(e.g. "regulator-12p0v"), like is already done for some of the other
regulators?

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

Re: [PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering

[Geert Uytterhoeven]

On Wed, Jan 9, 2019 at 3:01 PM <marek.vasut@gmail.com> wrote:
> From: Marek Vasut <marek.vasut+renesas@gmail.com>
>
> There are two regulator1 nodes in the Ebisu DTS right now, one 3.3V for
> the eMMC and one 12V for the backlight. This causes one to be overwritten
> by the other, ultimatelly resulting in inoperable eMMC, which depends on
> the former. Fix this by renumbering the backlight regulator to regulator2.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Cc: Simon Horman <horms+renesas@verge.net.au>
> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Cc: linux-renesas-soc@vger.kernel.org
> Reported-by: Simon Horman <horms+renesas@verge.net.au>
> Fixes: 9d16c4a10e07 ("arm64: dts: renesas: r8a77990: ebisu: Add backlight")

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

> --- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> @@ -191,7 +191,7 @@
>                 clock-frequency = <24576000>;
>         };
>
> -       reg_12p0v: regulator1 {
> +       reg_12p0v: regulator2 {
>                 compatible = "regulator-fixed";
>                 regulator-name = "D12.0V";
>                 regulator-min-microvolt = <12000000>;

Perhaps the node name should get a more descriptive suffix
(e.g. "regulator-12p0v"), like is already done for some of the other
regulators?

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering

[Simon Horman]

On Wed, Jan 09, 2019 at 04:26:25PM +0100, Geert Uytterhoeven wrote:
> On Wed, Jan 9, 2019 at 3:01 PM <marek.vasut@gmail.com> wrote:
> > From: Marek Vasut <marek.vasut+renesas@gmail.com>
> >
> > There are two regulator1 nodes in the Ebisu DTS right now, one 3.3V for
> > the eMMC and one 12V for the backlight. This causes one to be overwritten
> > by the other, ultimatelly resulting in inoperable eMMC, which depends on
> > the former. Fix this by renumbering the backlight regulator to regulator2.
> >
> > Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> > Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > Cc: Simon Horman <horms+renesas@verge.net.au>
> > Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > Cc: linux-renesas-soc@vger.kernel.org
> > Reported-by: Simon Horman <horms+renesas@verge.net.au>
> > Fixes: 9d16c4a10e07 ("arm64: dts: renesas: r8a77990: ebisu: Add backlight")
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> > --- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> > +++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> > @@ -191,7 +191,7 @@
> >                 clock-frequency = <24576000>;
> >         };
> >
> > -       reg_12p0v: regulator1 {
> > +       reg_12p0v: regulator2 {
> >                 compatible = "regulator-fixed";
> >                 regulator-name = "D12.0V";
> >                 regulator-min-microvolt = <12000000>;
> 
> Perhaps the node name should get a more descriptive suffix
> (e.g. "regulator-12p0v"), like is already done for some of the other
> regulators?

I think I would prefer that addressed in a follow-up patch.

Re: [PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering

[Simon Horman]

On Wed, Jan 09, 2019 at 04:26:25PM +0100, Geert Uytterhoeven wrote:
> On Wed, Jan 9, 2019 at 3:01 PM <marek.vasut@gmail.com> wrote:
> > From: Marek Vasut <marek.vasut+renesas@gmail.com>
> >
> > There are two regulator1 nodes in the Ebisu DTS right now, one 3.3V for
> > the eMMC and one 12V for the backlight. This causes one to be overwritten
> > by the other, ultimatelly resulting in inoperable eMMC, which depends on
> > the former. Fix this by renumbering the backlight regulator to regulator2.
> >
> > Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> > Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > Cc: Simon Horman <horms+renesas@verge.net.au>
> > Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > Cc: linux-renesas-soc@vger.kernel.org
> > Reported-by: Simon Horman <horms+renesas@verge.net.au>
> > Fixes: 9d16c4a10e07 ("arm64: dts: renesas: r8a77990: ebisu: Add backlight")
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> > --- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> > +++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> > @@ -191,7 +191,7 @@
> >                 clock-frequency = <24576000>;
> >         };
> >
> > -       reg_12p0v: regulator1 {
> > +       reg_12p0v: regulator2 {
> >                 compatible = "regulator-fixed";
> >                 regulator-name = "D12.0V";
> >                 regulator-min-microvolt = <12000000>;
> 
> Perhaps the node name should get a more descriptive suffix
> (e.g. "regulator-12p0v"), like is already done for some of the other
> regulators?

I think I would prefer that addressed in a follow-up patch.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering

[Simon Horman]

On Wed, Jan 09, 2019 at 05:58:23PM +0100, Simon Horman wrote:
> On Wed, Jan 09, 2019 at 04:26:25PM +0100, Geert Uytterhoeven wrote:
> > On Wed, Jan 9, 2019 at 3:01 PM <marek.vasut@gmail.com> wrote:
> > > From: Marek Vasut <marek.vasut+renesas@gmail.com>
> > >
> > > There are two regulator1 nodes in the Ebisu DTS right now, one 3.3V for
> > > the eMMC and one 12V for the backlight. This causes one to be overwritten
> > > by the other, ultimatelly resulting in inoperable eMMC, which depends on
> > > the former. Fix this by renumbering the backlight regulator to regulator2.
> > >
> > > Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> > > Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > > Cc: Simon Horman <horms+renesas@verge.net.au>
> > > Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > > Cc: linux-renesas-soc@vger.kernel.org
> > > Reported-by: Simon Horman <horms+renesas@verge.net.au>
> > > Fixes: 9d16c4a10e07 ("arm64: dts: renesas: r8a77990: ebisu: Add backlight")
> > 
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > 
> > > --- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> > > +++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> > > @@ -191,7 +191,7 @@
> > >                 clock-frequency = <24576000>;
> > >         };
> > >
> > > -       reg_12p0v: regulator1 {
> > > +       reg_12p0v: regulator2 {
> > >                 compatible = "regulator-fixed";
> > >                 regulator-name = "D12.0V";
> > >                 regulator-min-microvolt = <12000000>;
> > 
> > Perhaps the node name should get a more descriptive suffix
> > (e.g. "regulator-12p0v"), like is already done for some of the other
> > regulators?
> 
> I think I would prefer that addressed in a follow-up patch.

Thanks for this patch.

I have now tested it and it looks good to me.
I can now access eMMC as a block device.

Tested-by: Simon Horman <horms+renesas@verge.net.au>

I plan to apply this for v5.1 as the problem appears to
be introduced in a patch queued-up for v5.1.



# dmesg | egrep "(ee160000.sd|mmc0|mmcblk0|backlight)"
[    0.893760] pwm-backlight backlight: Linked as a consumer to regulator.3
[    2.901953] renesas_sdhi_internal_dmac ee160000.sd: Linked as a consumer to regulator.2
[    2.910262] renesas_sdhi_internal_dmac ee160000.sd: Linked as a consumer to regulator.1
[    2.967591] renesas_sdhi_internal_dmac ee160000.sd: mmc0 base at 0xee160000 max clock rate 200 MHz
[    3.049943] mmc0: new HS200 MMC card at address 0001
[    3.055843] mmcblk0: mmc0:0001 BGSD3R 29.1 GiB 
[    3.064414] mmcblk0boot0: mmc0:0001 BGSD3R partition 1 16.0 MiB
[    3.074888] mmcblk0boot1: mmc0:0001 BGSD3R partition 2 16.0 MiB
[    3.081522] mmcblk0rpmb: mmc0:0001 BGSD3R partition 3 4.00 MiB, chardev (243:0)

# dd if=/dev/mmcblk0 of=/dev/null bs=1M count=512 iflag=direct
512+0 records in
512+0 records out
536870912 bytes (537 MB) copied, 14.7343 s, 36.4 MB/s
# dd if=/dev/mmcblk0 of=/dev/null bs=1M count=512 iflag=direct
512+0 records in
512+0 records out
536870912 bytes (537 MB) copied, 8.46809 s, 63.4 MB/s
# dd if=/dev/mmcblk0 of=/dev/null bs=1M count=512 iflag=direct
512+0 records in
512+0 records out
536870912 bytes (537 MB) copied, 14.6945 s, 36.5 MB/s

# dd of=/dev/mmcblk0 if=/dev/zero bs=1M count=512 oflag=direct
512+0 records in
512+0 records out
536870912 bytes (537 MB) copied, 19.1677 s, 28.0 MB/s
# dd of=/dev/mmcblk0 if=/dev/zero bs=1M count=512 oflag=direct
512+0 records in
512+0 records out
536870912 bytes (537 MB) copied, 21.3139 s, 25.2 MB/s
# dd of=/dev/mmcblk0 if=/dev/zero bs=1M count=512 oflag=direct
512+0 records in
512+0 records out
536870912 bytes (537 MB) copied, 19.9205 s, 27.0 MB/s

Re: [PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering

[Simon Horman]

On Wed, Jan 09, 2019 at 05:58:23PM +0100, Simon Horman wrote:
> On Wed, Jan 09, 2019 at 04:26:25PM +0100, Geert Uytterhoeven wrote:
> > On Wed, Jan 9, 2019 at 3:01 PM <marek.vasut@gmail.com> wrote:
> > > From: Marek Vasut <marek.vasut+renesas@gmail.com>
> > >
> > > There are two regulator1 nodes in the Ebisu DTS right now, one 3.3V for
> > > the eMMC and one 12V for the backlight. This causes one to be overwritten
> > > by the other, ultimatelly resulting in inoperable eMMC, which depends on
> > > the former. Fix this by renumbering the backlight regulator to regulator2.
> > >
> > > Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> > > Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > > Cc: Simon Horman <horms+renesas@verge.net.au>
> > > Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > > Cc: linux-renesas-soc@vger.kernel.org
> > > Reported-by: Simon Horman <horms+renesas@verge.net.au>
> > > Fixes: 9d16c4a10e07 ("arm64: dts: renesas: r8a77990: ebisu: Add backlight")
> > 
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > 
> > > --- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> > > +++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> > > @@ -191,7 +191,7 @@
> > >                 clock-frequency = <24576000>;
> > >         };
> > >
> > > -       reg_12p0v: regulator1 {
> > > +       reg_12p0v: regulator2 {
> > >                 compatible = "regulator-fixed";
> > >                 regulator-name = "D12.0V";
> > >                 regulator-min-microvolt = <12000000>;
> > 
> > Perhaps the node name should get a more descriptive suffix
> > (e.g. "regulator-12p0v"), like is already done for some of the other
> > regulators?
> 
> I think I would prefer that addressed in a follow-up patch.

Thanks for this patch.

I have now tested it and it looks good to me.
I can now access eMMC as a block device.

Tested-by: Simon Horman <horms+renesas@verge.net.au>

I plan to apply this for v5.1 as the problem appears to
be introduced in a patch queued-up for v5.1.



# dmesg | egrep "(ee160000.sd|mmc0|mmcblk0|backlight)"
[    0.893760] pwm-backlight backlight: Linked as a consumer to regulator.3
[    2.901953] renesas_sdhi_internal_dmac ee160000.sd: Linked as a consumer to regulator.2
[    2.910262] renesas_sdhi_internal_dmac ee160000.sd: Linked as a consumer to regulator.1
[    2.967591] renesas_sdhi_internal_dmac ee160000.sd: mmc0 base at 0xee160000 max clock rate 200 MHz
[    3.049943] mmc0: new HS200 MMC card at address 0001
[    3.055843] mmcblk0: mmc0:0001 BGSD3R 29.1 GiB 
[    3.064414] mmcblk0boot0: mmc0:0001 BGSD3R partition 1 16.0 MiB
[    3.074888] mmcblk0boot1: mmc0:0001 BGSD3R partition 2 16.0 MiB
[    3.081522] mmcblk0rpmb: mmc0:0001 BGSD3R partition 3 4.00 MiB, chardev (243:0)

# dd if=/dev/mmcblk0 of=/dev/null bs=1M count=512 iflag=direct
512+0 records in
512+0 records out
536870912 bytes (537 MB) copied, 14.7343 s, 36.4 MB/s
# dd if=/dev/mmcblk0 of=/dev/null bs=1M count=512 iflag=direct
512+0 records in
512+0 records out
536870912 bytes (537 MB) copied, 8.46809 s, 63.4 MB/s
# dd if=/dev/mmcblk0 of=/dev/null bs=1M count=512 iflag=direct
512+0 records in
512+0 records out
536870912 bytes (537 MB) copied, 14.6945 s, 36.5 MB/s

# dd of=/dev/mmcblk0 if=/dev/zero bs=1M count=512 oflag=direct
512+0 records in
512+0 records out
536870912 bytes (537 MB) copied, 19.1677 s, 28.0 MB/s
# dd of=/dev/mmcblk0 if=/dev/zero bs=1M count=512 oflag=direct
512+0 records in
512+0 records out
536870912 bytes (537 MB) copied, 21.3139 s, 25.2 MB/s
# dd of=/dev/mmcblk0 if=/dev/zero bs=1M count=512 oflag=direct
512+0 records in
512+0 records out
536870912 bytes (537 MB) copied, 19.9205 s, 27.0 MB/s

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering

[Laurent Pinchart]

Hi Marek,

Thank you for the patch.

On Wednesday, 9 January 2019 16:00:45 EET marek.vasut@gmail.com wrote:
> From: Marek Vasut <marek.vasut+renesas@gmail.com>
> 
> There are two regulator1 nodes in the Ebisu DTS right now, one 3.3V for
> the eMMC and one 12V for the backlight. This causes one to be overwritten
> by the other, ultimatelly resulting in inoperable eMMC, which depends on
> the former. Fix this by renumbering the backlight regulator to regulator2.
> 
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Cc: Simon Horman <horms+renesas@verge.net.au>
> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Cc: linux-renesas-soc@vger.kernel.org
> Reported-by: Simon Horman <horms+renesas@verge.net.au>
> Fixes: 9d16c4a10e07 ("arm64: dts: renesas: r8a77990: ebisu: Add backlight")

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Sorry for breaking this in the first place.

> ---
>  arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts index
> e0590c0af4c4..89383aa35d65 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> @@ -191,7 +191,7 @@
>  		clock-frequency = <24576000>;
>  	};
> 
> -	reg_12p0v: regulator1 {
> +	reg_12p0v: regulator2 {
>  		compatible = "regulator-fixed";
>  		regulator-name = "D12.0V";
>  		regulator-min-microvolt = <12000000>;

-- 
Regards,

Laurent Pinchart

Re: [PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering

[Laurent Pinchart]

Hi Marek,

Thank you for the patch.

On Wednesday, 9 January 2019 16:00:45 EET marek.vasut@gmail.com wrote:
> From: Marek Vasut <marek.vasut+renesas@gmail.com>
> 
> There are two regulator1 nodes in the Ebisu DTS right now, one 3.3V for
> the eMMC and one 12V for the backlight. This causes one to be overwritten
> by the other, ultimatelly resulting in inoperable eMMC, which depends on
> the former. Fix this by renumbering the backlight regulator to regulator2.
> 
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Cc: Simon Horman <horms+renesas@verge.net.au>
> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Cc: linux-renesas-soc@vger.kernel.org
> Reported-by: Simon Horman <horms+renesas@verge.net.au>
> Fixes: 9d16c4a10e07 ("arm64: dts: renesas: r8a77990: ebisu: Add backlight")

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Sorry for breaking this in the first place.

> ---
>  arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts index
> e0590c0af4c4..89383aa35d65 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> @@ -191,7 +191,7 @@
>  		clock-frequency = <24576000>;
>  	};
> 
> -	reg_12p0v: regulator1 {
> +	reg_12p0v: regulator2 {
>  		compatible = "regulator-fixed";
>  		regulator-name = "D12.0V";
>  		regulator-min-microvolt = <12000000>;

-- 
Regards,

Laurent Pinchart




_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering

[Laurent Pinchart]

On Wednesday, 9 January 2019 18:58:23 EET Simon Horman wrote:
> On Wed, Jan 09, 2019 at 04:26:25PM +0100, Geert Uytterhoeven wrote:
> > On Wed, Jan 9, 2019 at 3:01 PM <marek.vasut@gmail.com> wrote:
> > > From: Marek Vasut <marek.vasut+renesas@gmail.com>
> > > 
> > > There are two regulator1 nodes in the Ebisu DTS right now, one 3.3V for
> > > the eMMC and one 12V for the backlight. This causes one to be
> > > overwritten
> > > by the other, ultimatelly resulting in inoperable eMMC, which depends on
> > > the former. Fix this by renumbering the backlight regulator to
> > > regulator2.
> > > 
> > > Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> > > Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > > Cc: Simon Horman <horms+renesas@verge.net.au>
> > > Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > > Cc: linux-renesas-soc@vger.kernel.org
> > > Reported-by: Simon Horman <horms+renesas@verge.net.au>
> > > Fixes: 9d16c4a10e07 ("arm64: dts: renesas: r8a77990: ebisu: Add
> > > backlight")
> > 
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > 
> > > --- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> > > +++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> > > @@ -191,7 +191,7 @@
> > > 
> > >                 clock-frequency = <24576000>;
> > >         
> > >         };
> > > 
> > > -       reg_12p0v: regulator1 {
> > > +       reg_12p0v: regulator2 {
> > > 
> > >                 compatible = "regulator-fixed";
> > >                 regulator-name = "D12.0V";
> > >                 regulator-min-microvolt = <12000000>;
> > 
> > Perhaps the node name should get a more descriptive suffix
> > (e.g. "regulator-12p0v"), like is already done for some of the other
> > regulators?
> 
> I think I would prefer that addressed in a follow-up patch.

Agreed, but it would still be a very good idea. I think we need to standardize 
names for regulators, otherwise this is bound to happen again in the future.

-- 
Regards,

Laurent Pinchart

Re: [PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering

[Laurent Pinchart]

On Wednesday, 9 January 2019 18:58:23 EET Simon Horman wrote:
> On Wed, Jan 09, 2019 at 04:26:25PM +0100, Geert Uytterhoeven wrote:
> > On Wed, Jan 9, 2019 at 3:01 PM <marek.vasut@gmail.com> wrote:
> > > From: Marek Vasut <marek.vasut+renesas@gmail.com>
> > > 
> > > There are two regulator1 nodes in the Ebisu DTS right now, one 3.3V for
> > > the eMMC and one 12V for the backlight. This causes one to be
> > > overwritten
> > > by the other, ultimatelly resulting in inoperable eMMC, which depends on
> > > the former. Fix this by renumbering the backlight regulator to
> > > regulator2.
> > > 
> > > Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> > > Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > > Cc: Simon Horman <horms+renesas@verge.net.au>
> > > Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > > Cc: linux-renesas-soc@vger.kernel.org
> > > Reported-by: Simon Horman <horms+renesas@verge.net.au>
> > > Fixes: 9d16c4a10e07 ("arm64: dts: renesas: r8a77990: ebisu: Add
> > > backlight")
> > 
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > 
> > > --- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> > > +++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> > > @@ -191,7 +191,7 @@
> > > 
> > >                 clock-frequency = <24576000>;
> > >         
> > >         };
> > > 
> > > -       reg_12p0v: regulator1 {
> > > +       reg_12p0v: regulator2 {
> > > 
> > >                 compatible = "regulator-fixed";
> > >                 regulator-name = "D12.0V";
> > >                 regulator-min-microvolt = <12000000>;
> > 
> > Perhaps the node name should get a more descriptive suffix
> > (e.g. "regulator-12p0v"), like is already done for some of the other
> > regulators?
> 
> I think I would prefer that addressed in a follow-up patch.

Agreed, but it would still be a very good idea. I think we need to standardize 
names for regulators, otherwise this is bound to happen again in the future.

-- 
Regards,

Laurent Pinchart




_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering

[Marek Vasut]

On 1/10/19 11:02 AM, Simon Horman wrote:
> On Wed, Jan 09, 2019 at 05:58:23PM +0100, Simon Horman wrote:
>> On Wed, Jan 09, 2019 at 04:26:25PM +0100, Geert Uytterhoeven wrote:
>>> On Wed, Jan 9, 2019 at 3:01 PM <marek.vasut@gmail.com> wrote:
>>>> From: Marek Vasut <marek.vasut+renesas@gmail.com>
>>>>
>>>> There are two regulator1 nodes in the Ebisu DTS right now, one 3.3V for
>>>> the eMMC and one 12V for the backlight. This causes one to be overwritten
>>>> by the other, ultimatelly resulting in inoperable eMMC, which depends on
>>>> the former. Fix this by renumbering the backlight regulator to regulator2.
>>>>
>>>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
>>>> Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>>>> Cc: Simon Horman <horms+renesas@verge.net.au>
>>>> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
>>>> Cc: linux-renesas-soc@vger.kernel.org
>>>> Reported-by: Simon Horman <horms+renesas@verge.net.au>
>>>> Fixes: 9d16c4a10e07 ("arm64: dts: renesas: r8a77990: ebisu: Add backlight")
>>>
>>> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>>
>>>> --- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
>>>> +++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
>>>> @@ -191,7 +191,7 @@
>>>>                 clock-frequency = <24576000>;
>>>>         };
>>>>
>>>> -       reg_12p0v: regulator1 {
>>>> +       reg_12p0v: regulator2 {
>>>>                 compatible = "regulator-fixed";
>>>>                 regulator-name = "D12.0V";
>>>>                 regulator-min-microvolt = <12000000>;
>>>
>>> Perhaps the node name should get a more descriptive suffix
>>> (e.g. "regulator-12p0v"), like is already done for some of the other
>>> regulators?
>>
>> I think I would prefer that addressed in a follow-up patch.
> 
> Thanks for this patch.
> 
> I have now tested it and it looks good to me.
> I can now access eMMC as a block device.
> 
> Tested-by: Simon Horman <horms+renesas@verge.net.au>
> 
> I plan to apply this for v5.1 as the problem appears to
> be introduced in a patch queued-up for v5.1.

Thanks

> # dmesg | egrep "(ee160000.sd|mmc0|mmcblk0|backlight)"
> [    0.893760] pwm-backlight backlight: Linked as a consumer to regulator.3
> [    2.901953] renesas_sdhi_internal_dmac ee160000.sd: Linked as a consumer to regulator.2
> [    2.910262] renesas_sdhi_internal_dmac ee160000.sd: Linked as a consumer to regulator.1
> [    2.967591] renesas_sdhi_internal_dmac ee160000.sd: mmc0 base at 0xee160000 max clock rate 200 MHz
> [    3.049943] mmc0: new HS200 MMC card at address 0001
> [    3.055843] mmcblk0: mmc0:0001 BGSD3R 29.1 GiB 
> [    3.064414] mmcblk0boot0: mmc0:0001 BGSD3R partition 1 16.0 MiB
> [    3.074888] mmcblk0boot1: mmc0:0001 BGSD3R partition 2 16.0 MiB
> [    3.081522] mmcblk0rpmb: mmc0:0001 BGSD3R partition 3 4.00 MiB, chardev (243:0)
> 
> # dd if=/dev/mmcblk0 of=/dev/null bs=1M count=512 iflag=direct
> 512+0 records in
> 512+0 records out
> 536870912 bytes (537 MB) copied, 14.7343 s, 36.4 MB/s
> # dd if=/dev/mmcblk0 of=/dev/null bs=1M count=512 iflag=direct
> 512+0 records in
> 512+0 records out
> 536870912 bytes (537 MB) copied, 8.46809 s, 63.4 MB/s
> # dd if=/dev/mmcblk0 of=/dev/null bs=1M count=512 iflag=direct
> 512+0 records in
> 512+0 records out
> 536870912 bytes (537 MB) copied, 14.6945 s, 36.5 MB/s

Seems a bit slow to me for an HS200 card :-)
Update your ATF, it has QoS updates for SDHI.

-- 
Best regards,
Marek Vasut

Re: [PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering

[Marek Vasut]

On 1/10/19 11:02 AM, Simon Horman wrote:
> On Wed, Jan 09, 2019 at 05:58:23PM +0100, Simon Horman wrote:
>> On Wed, Jan 09, 2019 at 04:26:25PM +0100, Geert Uytterhoeven wrote:
>>> On Wed, Jan 9, 2019 at 3:01 PM <marek.vasut@gmail.com> wrote:
>>>> From: Marek Vasut <marek.vasut+renesas@gmail.com>
>>>>
>>>> There are two regulator1 nodes in the Ebisu DTS right now, one 3.3V for
>>>> the eMMC and one 12V for the backlight. This causes one to be overwritten
>>>> by the other, ultimatelly resulting in inoperable eMMC, which depends on
>>>> the former. Fix this by renumbering the backlight regulator to regulator2.
>>>>
>>>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
>>>> Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>>>> Cc: Simon Horman <horms+renesas@verge.net.au>
>>>> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
>>>> Cc: linux-renesas-soc@vger.kernel.org
>>>> Reported-by: Simon Horman <horms+renesas@verge.net.au>
>>>> Fixes: 9d16c4a10e07 ("arm64: dts: renesas: r8a77990: ebisu: Add backlight")
>>>
>>> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>>
>>>> --- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
>>>> +++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
>>>> @@ -191,7 +191,7 @@
>>>>                 clock-frequency = <24576000>;
>>>>         };
>>>>
>>>> -       reg_12p0v: regulator1 {
>>>> +       reg_12p0v: regulator2 {
>>>>                 compatible = "regulator-fixed";
>>>>                 regulator-name = "D12.0V";
>>>>                 regulator-min-microvolt = <12000000>;
>>>
>>> Perhaps the node name should get a more descriptive suffix
>>> (e.g. "regulator-12p0v"), like is already done for some of the other
>>> regulators?
>>
>> I think I would prefer that addressed in a follow-up patch.
> 
> Thanks for this patch.
> 
> I have now tested it and it looks good to me.
> I can now access eMMC as a block device.
> 
> Tested-by: Simon Horman <horms+renesas@verge.net.au>
> 
> I plan to apply this for v5.1 as the problem appears to
> be introduced in a patch queued-up for v5.1.

Thanks

> # dmesg | egrep "(ee160000.sd|mmc0|mmcblk0|backlight)"
> [    0.893760] pwm-backlight backlight: Linked as a consumer to regulator.3
> [    2.901953] renesas_sdhi_internal_dmac ee160000.sd: Linked as a consumer to regulator.2
> [    2.910262] renesas_sdhi_internal_dmac ee160000.sd: Linked as a consumer to regulator.1
> [    2.967591] renesas_sdhi_internal_dmac ee160000.sd: mmc0 base at 0xee160000 max clock rate 200 MHz
> [    3.049943] mmc0: new HS200 MMC card at address 0001
> [    3.055843] mmcblk0: mmc0:0001 BGSD3R 29.1 GiB 
> [    3.064414] mmcblk0boot0: mmc0:0001 BGSD3R partition 1 16.0 MiB
> [    3.074888] mmcblk0boot1: mmc0:0001 BGSD3R partition 2 16.0 MiB
> [    3.081522] mmcblk0rpmb: mmc0:0001 BGSD3R partition 3 4.00 MiB, chardev (243:0)
> 
> # dd if=/dev/mmcblk0 of=/dev/null bs=1M count=512 iflag=direct
> 512+0 records in
> 512+0 records out
> 536870912 bytes (537 MB) copied, 14.7343 s, 36.4 MB/s
> # dd if=/dev/mmcblk0 of=/dev/null bs=1M count=512 iflag=direct
> 512+0 records in
> 512+0 records out
> 536870912 bytes (537 MB) copied, 8.46809 s, 63.4 MB/s
> # dd if=/dev/mmcblk0 of=/dev/null bs=1M count=512 iflag=direct
> 512+0 records in
> 512+0 records out
> 536870912 bytes (537 MB) copied, 14.6945 s, 36.5 MB/s

Seems a bit slow to me for an HS200 card :-)
Update your ATF, it has QoS updates for SDHI.

-- 
Best regards,
Marek Vasut

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering

[Marek Vasut]

On 1/10/19 1:59 PM, Laurent Pinchart wrote:
> On Wednesday, 9 January 2019 18:58:23 EET Simon Horman wrote:
>> On Wed, Jan 09, 2019 at 04:26:25PM +0100, Geert Uytterhoeven wrote:
>>> On Wed, Jan 9, 2019 at 3:01 PM <marek.vasut@gmail.com> wrote:
>>>> From: Marek Vasut <marek.vasut+renesas@gmail.com>
>>>>
>>>> There are two regulator1 nodes in the Ebisu DTS right now, one 3.3V for
>>>> the eMMC and one 12V for the backlight. This causes one to be
>>>> overwritten
>>>> by the other, ultimatelly resulting in inoperable eMMC, which depends on
>>>> the former. Fix this by renumbering the backlight regulator to
>>>> regulator2.
>>>>
>>>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
>>>> Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>>>> Cc: Simon Horman <horms+renesas@verge.net.au>
>>>> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
>>>> Cc: linux-renesas-soc@vger.kernel.org
>>>> Reported-by: Simon Horman <horms+renesas@verge.net.au>
>>>> Fixes: 9d16c4a10e07 ("arm64: dts: renesas: r8a77990: ebisu: Add
>>>> backlight")
>>>
>>> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>>
>>>> --- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
>>>> +++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
>>>> @@ -191,7 +191,7 @@
>>>>
>>>>                 clock-frequency = <24576000>;
>>>>         
>>>>         };
>>>>
>>>> -       reg_12p0v: regulator1 {
>>>> +       reg_12p0v: regulator2 {
>>>>
>>>>                 compatible = "regulator-fixed";
>>>>                 regulator-name = "D12.0V";
>>>>                 regulator-min-microvolt = <12000000>;
>>>
>>> Perhaps the node name should get a more descriptive suffix
>>> (e.g. "regulator-12p0v"), like is already done for some of the other
>>> regulators?
>>
>> I think I would prefer that addressed in a follow-up patch.
> 
> Agreed, but it would still be a very good idea. I think we need to standardize 
> names for regulators, otherwise this is bound to happen again in the future.

Isn't the YAML DT schema validator supposed to catch those problems ?
I'd even expect DTC to be able to catch such duplicate nodes and warn
about them.

-- 
Best regards,
Marek Vasut

Re: [PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering

[Marek Vasut]

On 1/10/19 1:59 PM, Laurent Pinchart wrote:
> On Wednesday, 9 January 2019 18:58:23 EET Simon Horman wrote:
>> On Wed, Jan 09, 2019 at 04:26:25PM +0100, Geert Uytterhoeven wrote:
>>> On Wed, Jan 9, 2019 at 3:01 PM <marek.vasut@gmail.com> wrote:
>>>> From: Marek Vasut <marek.vasut+renesas@gmail.com>
>>>>
>>>> There are two regulator1 nodes in the Ebisu DTS right now, one 3.3V for
>>>> the eMMC and one 12V for the backlight. This causes one to be
>>>> overwritten
>>>> by the other, ultimatelly resulting in inoperable eMMC, which depends on
>>>> the former. Fix this by renumbering the backlight regulator to
>>>> regulator2.
>>>>
>>>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
>>>> Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>>>> Cc: Simon Horman <horms+renesas@verge.net.au>
>>>> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
>>>> Cc: linux-renesas-soc@vger.kernel.org
>>>> Reported-by: Simon Horman <horms+renesas@verge.net.au>
>>>> Fixes: 9d16c4a10e07 ("arm64: dts: renesas: r8a77990: ebisu: Add
>>>> backlight")
>>>
>>> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>>
>>>> --- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
>>>> +++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
>>>> @@ -191,7 +191,7 @@
>>>>
>>>>                 clock-frequency = <24576000>;
>>>>         
>>>>         };
>>>>
>>>> -       reg_12p0v: regulator1 {
>>>> +       reg_12p0v: regulator2 {
>>>>
>>>>                 compatible = "regulator-fixed";
>>>>                 regulator-name = "D12.0V";
>>>>                 regulator-min-microvolt = <12000000>;
>>>
>>> Perhaps the node name should get a more descriptive suffix
>>> (e.g. "regulator-12p0v"), like is already done for some of the other
>>> regulators?
>>
>> I think I would prefer that addressed in a follow-up patch.
> 
> Agreed, but it would still be a very good idea. I think we need to standardize 
> names for regulators, otherwise this is bound to happen again in the future.

Isn't the YAML DT schema validator supposed to catch those problems ?
I'd even expect DTC to be able to catch such duplicate nodes and warn
about them.

-- 
Best regards,
Marek Vasut

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

DTC check_duplicate_node_names (was: Re: [PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering)

[Geert Uytterhoeven]

Hi Marek,

Bringing this to the attention of the DTC people...

On Thu, Jan 10, 2019 at 3:38 PM Marek Vasut <marek.vasut@gmail.com> wrote:
> On 1/10/19 1:59 PM, Laurent Pinchart wrote:
> > On Wednesday, 9 January 2019 18:58:23 EET Simon Horman wrote:
> >> On Wed, Jan 09, 2019 at 04:26:25PM +0100, Geert Uytterhoeven wrote:
> >>> On Wed, Jan 9, 2019 at 3:01 PM <marek.vasut@gmail.com> wrote:
> >>>> From: Marek Vasut <marek.vasut+renesas@gmail.com>
> >>>>
> >>>> There are two regulator1 nodes in the Ebisu DTS right now, one 3.3V for
> >>>> the eMMC and one 12V for the backlight. This causes one to be
> >>>> overwritten
> >>>> by the other, ultimatelly resulting in inoperable eMMC, which depends on
> >>>> the former. Fix this by renumbering the backlight regulator to
> >>>> regulator2.
> >>>>
> >>>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> >>>> Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> >>>> Cc: Simon Horman <horms+renesas@verge.net.au>
> >>>> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> >>>> Cc: linux-renesas-soc@vger.kernel.org
> >>>> Reported-by: Simon Horman <horms+renesas@verge.net.au>
> >>>> Fixes: 9d16c4a10e07 ("arm64: dts: renesas: r8a77990: ebisu: Add
> >>>> backlight")
> >>>
> >>> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >>>
> >>>> --- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> >>>> +++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> >>>> @@ -191,7 +191,7 @@
> >>>>
> >>>>                 clock-frequency = <24576000>;
> >>>>
> >>>>         };
> >>>>
> >>>> -       reg_12p0v: regulator1 {
> >>>> +       reg_12p0v: regulator2 {
> >>>>
> >>>>                 compatible = "regulator-fixed";
> >>>>                 regulator-name = "D12.0V";
> >>>>                 regulator-min-microvolt = <12000000>;
> >>>
> >>> Perhaps the node name should get a more descriptive suffix
> >>> (e.g. "regulator-12p0v"), like is already done for some of the other
> >>> regulators?
> >>
> >> I think I would prefer that addressed in a follow-up patch.
> >
> > Agreed, but it would still be a very good idea. I think we need to standardize
> > names for regulators, otherwise this is bound to happen again in the future.

And so it did (patch sent for the same bug in r8a77995-draak.dts).

> Isn't the YAML DT schema validator supposed to catch those problems ?
> I'd even expect DTC to be able to catch such duplicate nodes and warn
> about them.

DTC indeed has check_duplicate_node_names.
However, it only works for the base DTS, not for any later modifications in
the board DTS.

I.e. the original dup-nodename.dts in the DTC testsuite triggers an error,
but the modified version below doesn't.

--- a/tests/dup-nodename.dts
+++ b/tests/dup-nodename.dts
@@ -1,8 +1,11 @@
 /dts-v1/;

+/ {
+};
+
 / {
        node {
        };
        node {
        };
 };

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

DTC check_duplicate_node_names (was: Re: [PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering)

[Geert Uytterhoeven]

Hi Marek,

Bringing this to the attention of the DTC people...

On Thu, Jan 10, 2019 at 3:38 PM Marek Vasut <marek.vasut@gmail.com> wrote:
> On 1/10/19 1:59 PM, Laurent Pinchart wrote:
> > On Wednesday, 9 January 2019 18:58:23 EET Simon Horman wrote:
> >> On Wed, Jan 09, 2019 at 04:26:25PM +0100, Geert Uytterhoeven wrote:
> >>> On Wed, Jan 9, 2019 at 3:01 PM <marek.vasut@gmail.com> wrote:
> >>>> From: Marek Vasut <marek.vasut+renesas@gmail.com>
> >>>>
> >>>> There are two regulator1 nodes in the Ebisu DTS right now, one 3.3V for
> >>>> the eMMC and one 12V for the backlight. This causes one to be
> >>>> overwritten
> >>>> by the other, ultimatelly resulting in inoperable eMMC, which depends on
> >>>> the former. Fix this by renumbering the backlight regulator to
> >>>> regulator2.
> >>>>
> >>>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> >>>> Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> >>>> Cc: Simon Horman <horms+renesas@verge.net.au>
> >>>> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> >>>> Cc: linux-renesas-soc@vger.kernel.org
> >>>> Reported-by: Simon Horman <horms+renesas@verge.net.au>
> >>>> Fixes: 9d16c4a10e07 ("arm64: dts: renesas: r8a77990: ebisu: Add
> >>>> backlight")
> >>>
> >>> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >>>
> >>>> --- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> >>>> +++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> >>>> @@ -191,7 +191,7 @@
> >>>>
> >>>>                 clock-frequency = <24576000>;
> >>>>
> >>>>         };
> >>>>
> >>>> -       reg_12p0v: regulator1 {
> >>>> +       reg_12p0v: regulator2 {
> >>>>
> >>>>                 compatible = "regulator-fixed";
> >>>>                 regulator-name = "D12.0V";
> >>>>                 regulator-min-microvolt = <12000000>;
> >>>
> >>> Perhaps the node name should get a more descriptive suffix
> >>> (e.g. "regulator-12p0v"), like is already done for some of the other
> >>> regulators?
> >>
> >> I think I would prefer that addressed in a follow-up patch.
> >
> > Agreed, but it would still be a very good idea. I think we need to standardize
> > names for regulators, otherwise this is bound to happen again in the future.

And so it did (patch sent for the same bug in r8a77995-draak.dts).

> Isn't the YAML DT schema validator supposed to catch those problems ?
> I'd even expect DTC to be able to catch such duplicate nodes and warn
> about them.

DTC indeed has check_duplicate_node_names.
However, it only works for the base DTS, not for any later modifications in
the board DTS.

I.e. the original dup-nodename.dts in the DTC testsuite triggers an error,
but the modified version below doesn't.

--- a/tests/dup-nodename.dts
+++ b/tests/dup-nodename.dts
@@ -1,8 +1,11 @@
 /dts-v1/;

+/ {
+};
+
 / {
        node {
        };
        node {
        };
 };

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

DTC check_duplicate_node_names (was: Re: [PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering)

[Geert Uytterhoeven]

Hi Marek,

Bringing this to the attention of the DTC people...

On Thu, Jan 10, 2019 at 3:38 PM Marek Vasut <marek.vasut@gmail.com> wrote:
> On 1/10/19 1:59 PM, Laurent Pinchart wrote:
> > On Wednesday, 9 January 2019 18:58:23 EET Simon Horman wrote:
> >> On Wed, Jan 09, 2019 at 04:26:25PM +0100, Geert Uytterhoeven wrote:
> >>> On Wed, Jan 9, 2019 at 3:01 PM <marek.vasut@gmail.com> wrote:
> >>>> From: Marek Vasut <marek.vasut+renesas@gmail.com>
> >>>>
> >>>> There are two regulator1 nodes in the Ebisu DTS right now, one 3.3V for
> >>>> the eMMC and one 12V for the backlight. This causes one to be
> >>>> overwritten
> >>>> by the other, ultimatelly resulting in inoperable eMMC, which depends on
> >>>> the former. Fix this by renumbering the backlight regulator to
> >>>> regulator2.
> >>>>
> >>>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> >>>> Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> >>>> Cc: Simon Horman <horms+renesas@verge.net.au>
> >>>> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
> >>>> Cc: linux-renesas-soc@vger.kernel.org
> >>>> Reported-by: Simon Horman <horms+renesas@verge.net.au>
> >>>> Fixes: 9d16c4a10e07 ("arm64: dts: renesas: r8a77990: ebisu: Add
> >>>> backlight")
> >>>
> >>> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >>>
> >>>> --- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> >>>> +++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
> >>>> @@ -191,7 +191,7 @@
> >>>>
> >>>>                 clock-frequency = <24576000>;
> >>>>
> >>>>         };
> >>>>
> >>>> -       reg_12p0v: regulator1 {
> >>>> +       reg_12p0v: regulator2 {
> >>>>
> >>>>                 compatible = "regulator-fixed";
> >>>>                 regulator-name = "D12.0V";
> >>>>                 regulator-min-microvolt = <12000000>;
> >>>
> >>> Perhaps the node name should get a more descriptive suffix
> >>> (e.g. "regulator-12p0v"), like is already done for some of the other
> >>> regulators?
> >>
> >> I think I would prefer that addressed in a follow-up patch.
> >
> > Agreed, but it would still be a very good idea. I think we need to standardize
> > names for regulators, otherwise this is bound to happen again in the future.

And so it did (patch sent for the same bug in r8a77995-draak.dts).

> Isn't the YAML DT schema validator supposed to catch those problems ?
> I'd even expect DTC to be able to catch such duplicate nodes and warn
> about them.

DTC indeed has check_duplicate_node_names.
However, it only works for the base DTS, not for any later modifications in
the board DTS.

I.e. the original dup-nodename.dts in the DTC testsuite triggers an error,
but the modified version below doesn't.

--- a/tests/dup-nodename.dts
+++ b/tests/dup-nodename.dts
@@ -1,8 +1,11 @@
 /dts-v1/;

+/ {
+};
+
 / {
        node {
        };
        node {
        };
 };

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel