Linux-Renesas-SoC Archive on lore.kernel.org
 help / Atom feed
* [PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering
@ 2019-01-09 14:00 marek.vasut
  2019-01-09 15:26 ` Geert Uytterhoeven
  2019-01-10 12:59 ` Laurent Pinchart
  0 siblings, 2 replies; 8+ messages in thread
From: marek.vasut @ 2019-01-09 14:00 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Marek Vasut, Laurent Pinchart, Simon Horman, Wolfram Sang,
	linux-renesas-soc

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering
  2019-01-09 14:00 [PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering marek.vasut
@ 2019-01-09 15:26 ` Geert Uytterhoeven
  2019-01-09 16:58   ` Simon Horman
  2019-01-10 12:59 ` Laurent Pinchart
  1 sibling, 1 reply; 8+ messages in thread
From: Geert Uytterhoeven @ 2019-01-09 15:26 UTC (permalink / raw)
  To: Marek Vasut
  Cc: Linux ARM, Linux-Renesas, Simon Horman, Laurent Pinchart,
	Wolfram Sang, Marek Vasut

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering
  2019-01-09 15:26 ` Geert Uytterhoeven
@ 2019-01-09 16:58   ` Simon Horman
  2019-01-10 10:02     ` Simon Horman
  2019-01-10 12:59     ` Laurent Pinchart
  0 siblings, 2 replies; 8+ messages in thread
From: Simon Horman @ 2019-01-09 16:58 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Marek Vasut, Linux ARM, Linux-Renesas, Laurent Pinchart,
	Wolfram Sang, Marek Vasut

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.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering
  2019-01-09 16:58   ` Simon Horman
@ 2019-01-10 10:02     ` Simon Horman
  2019-01-10 13:40       ` Marek Vasut
  2019-01-10 12:59     ` Laurent Pinchart
  1 sibling, 1 reply; 8+ messages in thread
From: Simon Horman @ 2019-01-10 10:02 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Marek Vasut, Linux ARM, Linux-Renesas, Laurent Pinchart,
	Wolfram Sang, Marek Vasut

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering
  2019-01-09 14:00 [PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering marek.vasut
  2019-01-09 15:26 ` Geert Uytterhoeven
@ 2019-01-10 12:59 ` Laurent Pinchart
  1 sibling, 0 replies; 8+ messages in thread
From: Laurent Pinchart @ 2019-01-10 12:59 UTC (permalink / raw)
  To: marek.vasut
  Cc: linux-arm-kernel, Marek Vasut, Laurent Pinchart, Simon Horman,
	Wolfram Sang, linux-renesas-soc

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




^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering
  2019-01-09 16:58   ` Simon Horman
  2019-01-10 10:02     ` Simon Horman
@ 2019-01-10 12:59     ` Laurent Pinchart
  2019-01-10 13:40       ` Marek Vasut
  1 sibling, 1 reply; 8+ messages in thread
From: Laurent Pinchart @ 2019-01-10 12:59 UTC (permalink / raw)
  To: Simon Horman
  Cc: Geert Uytterhoeven, Marek Vasut, Linux ARM, Linux-Renesas,
	Laurent Pinchart, Wolfram Sang, Marek Vasut

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




^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering
  2019-01-10 10:02     ` Simon Horman
@ 2019-01-10 13:40       ` Marek Vasut
  0 siblings, 0 replies; 8+ messages in thread
From: Marek Vasut @ 2019-01-10 13:40 UTC (permalink / raw)
  To: Simon Horman, Geert Uytterhoeven
  Cc: Linux ARM, Linux-Renesas, Laurent Pinchart, Wolfram Sang, 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering
  2019-01-10 12:59     ` Laurent Pinchart
@ 2019-01-10 13:40       ` Marek Vasut
  0 siblings, 0 replies; 8+ messages in thread
From: Marek Vasut @ 2019-01-10 13:40 UTC (permalink / raw)
  To: Laurent Pinchart, Simon Horman
  Cc: Geert Uytterhoeven, Linux ARM, Linux-Renesas, Laurent Pinchart,
	Wolfram Sang, 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, back to index

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-09 14:00 [PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering marek.vasut
2019-01-09 15:26 ` Geert Uytterhoeven
2019-01-09 16:58   ` Simon Horman
2019-01-10 10:02     ` Simon Horman
2019-01-10 13:40       ` Marek Vasut
2019-01-10 12:59     ` Laurent Pinchart
2019-01-10 13:40       ` Marek Vasut
2019-01-10 12:59 ` Laurent Pinchart

Linux-Renesas-SoC Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-renesas-soc/0 linux-renesas-soc/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-renesas-soc linux-renesas-soc/ https://lore.kernel.org/linux-renesas-soc \
		linux-renesas-soc@vger.kernel.org linux-renesas-soc@archiver.kernel.org
	public-inbox-index linux-renesas-soc


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-renesas-soc


AGPL code for this site: git clone https://public-inbox.org/ public-inbox