Re: [PATCH v2 4/8] arm64: dts: r8a7796: salvator-x: Add panel backlight support

[Simon Horman <horms@verge.net.au>]

On Sat, May 13, 2017 at 09:56:12PM +0300, Laurent Pinchart wrote:
> Hi Simon,
> 
> I'm afraid you added nodes in the wrong places when applying the patch to 
> salvator-x.dtsi :-( Please see below.
> 
> On Thursday 27 Apr 2017 17:43:56 Laurent Pinchart wrote:
> > Hi Ulrich,
> > 
> > Thank you for the patch.
> > 
> > On Thursday 27 Apr 2017 16:37:39 Ulrich Hecht wrote:
> > > The panel backlight is controlled through a GPIO and a PWM channel.
> > > 
> > > Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> > 
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > > ---
> > > 
> > >  arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts | 22 ++++++++++++++++
> > >  1 file changed, 22 insertions(+)
> > > 
> > > diff --git a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
> > > b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts index
> > > 14d9e51..abda84c
> > > 100644
> > > --- a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
> > > +++ b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
> > > @@ -133,6 +133,11 @@
> > >  		function = "i2c2";
> > >  	};
> > > 
> > > +	pwm1_pins: pwm {
> > > +		groups = "pwm1_a";
> > > +		function = "pwm1";
> > > +	};
> > > +
> 
> This is alphabetically ordered here and isn't anymore in your devel branch.

Thanks, I  will move it from between scif_clk_pins and sdhi0_pins to
between i2c2_pins and scif1_pins.

> > >  	sdhi0_pins: sd0 {
> > >  		groups = "sdhi0_data4", "sdhi0_ctrl";
> > >  		function = "sdhi0";
> > > @@ -183,6 +188,16 @@
> > >  		interrupt-parent = <&gpio2>;
> > >  		interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
> > >  	};
> > > +
> > > +	backlight: backlight {
> > > +		compatible = "pwm-backlight";
> > > +		pwms = <&pwm1 0 50000>;
> > > +
> > > +		brightness-levels = <256 128 64 16 8 4 0>;
> > > +		default-brightness-level = <6>;
> > > +
> > > +		enable-gpios = <&gpio6 7 GPIO_ACTIVE_HIGH>;
> > > +	};
> 
> The backlight node needs to be added as a child of the root node, while you've 
> added it to the DU node in your devel branch.

Thanks. I have moved it to between audio_clkout and reg_1p8v in
the root node.

> > >  };
> > >  
> > >  &extal_clk {
> > > @@ -235,6 +250,13 @@
> > >  	status = "okay";
> > >  };
> > > 
> > > +&pwm1 {
> > > +	pinctrl-0 = <&pwm1_pins>;
> > > +	pinctrl-names = "default";
> > > +
> > > +	status = "okay";
> > > +};
> > > +
> 
> This is alphabetically ordered here and isn't anymore in your devel branch.

Thanks, I have moved it from between rcar_sound and scif1 to
between pfc and rcar_sound. 

> Can you rebase your devel branch to fix this ?

Sure. I have the following in my local tree which I plan to push
a later today.


From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

[PATCH] arm64: dts: salvator-x: Add panel backlight support

The panel backlight is controlled through a GPIO and a PWM channel.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
[simon: apply to salvator-x.dtsi instead of r8a7795-salvator-x.dts]
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
 arch/arm64/boot/dts/renesas/salvator-x.dtsi | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/salvator-x.dtsi b/arch/arm64/boot/dts/renesas/salvator-x.dtsi
index d5eb022d247c..7240bcd75918 100644
--- a/arch/arm64/boot/dts/renesas/salvator-x.dtsi
+++ b/arch/arm64/boot/dts/renesas/salvator-x.dtsi
@@ -58,6 +58,16 @@
 		clock-frequency = <11289600>;
 	};
 
+	backlight: backlight {
+		compatible = "pwm-backlight";
+		pwms = <&pwm1 0 50000>;
+
+		brightness-levels = <256 128 64 16 8 4 0>;
+		default-brightness-level = <6>;
+
+		enable-gpios = <&gpio6 7 GPIO_ACTIVE_HIGH>;
+	};
+
 	reg_1p8v: regulator0 {
 		compatible = "regulator-fixed";
 		regulator-name = "fixed-1.8V";
@@ -358,6 +368,11 @@
 		function = "i2c2";
 	};
 
+	pwm1_pins: pwm {
+		groups = "pwm1_a";
+		function = "pwm1";
+	};
+
 	scif1_pins: scif1 {
 		groups = "scif1_data_a", "scif1_ctrl";
 		function = "scif1";
@@ -443,6 +458,13 @@
 	};
 };
 
+&pwm1 {
+	pinctrl-0 = <&pwm1_pins>;
+	pinctrl-names = "default";
+
+	status = "okay";
+};
+
 &rcar_sound {
 	pinctrl-0 = <&sound_pins &sound_clk_pins>;
 	pinctrl-names = "default";
-- 
2.1.4