* [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 ` [PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering Laurent Pinchart 0 siblings, 2 replies; 9+ 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 related [flat|nested] 9+ 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 ` [PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering Laurent Pinchart 1 sibling, 1 reply; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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 2019-07-31 7:56 ` DTC check_duplicate_node_names (was: Re: [PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering) Geert Uytterhoeven 0 siblings, 1 reply; 9+ 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] 9+ messages in thread
* DTC check_duplicate_node_names (was: Re: [PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering) 2019-01-10 13:40 ` Marek Vasut @ 2019-07-31 7:56 ` Geert Uytterhoeven 0 siblings, 0 replies; 9+ messages in thread From: Geert Uytterhoeven @ 2019-07-31 7:56 UTC (permalink / raw) To: Marek Vasut Cc: Laurent Pinchart, Simon Horman, Linux ARM, Linux-Renesas, Laurent Pinchart, Wolfram Sang, Marek Vasut, Rob Herring, Frank Rowand, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS 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 ^ permalink raw reply [flat|nested] 9+ 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; 9+ 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] 9+ messages in thread
end of thread, other threads:[~2019-07-31 7:56 UTC | newest] Thread overview: 9+ 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-07-31 7:56 ` DTC check_duplicate_node_names (was: Re: [PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering) Geert Uytterhoeven 2019-01-10 12:59 ` [PATCH] arm64: dts: renesas: r8a77990: ebisu: Fix backlight regulator numbering Laurent Pinchart
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).