[PATCH] arm: dts: imx6qdl-vicut1.dtsi: Add backlight-isb node

David Jander posted 1 patch 3 years, 8 months ago
There is a newer version of this series
arch/arm/boot/dts/imx6qdl-vicut1.dtsi | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
[PATCH] arm: dts: imx6qdl-vicut1.dtsi: Add backlight-isb node
Posted by David Jander 3 years, 8 months ago
On some older hardware models this is needed. It will do no harm if used
with newer models though.

Signed-off-by: David Jander <david@protonic.nl>
---
 arch/arm/boot/dts/imx6qdl-vicut1.dtsi | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/arm/boot/dts/imx6qdl-vicut1.dtsi b/arch/arm/boot/dts/imx6qdl-vicut1.dtsi
index a1676b5d2980..08d425020088 100644
--- a/arch/arm/boot/dts/imx6qdl-vicut1.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-vicut1.dtsi
@@ -37,6 +37,16 @@ backlight_led: backlight_led {
 		power-supply = <&reg_3v3>;
 	};
 
+	/* only for backwards compatibility with old HW */
+	backlight_isb: backlight_isb {
+		compatible = "pwm-backlight";
+		pwms = <&pwm2 0 5000000 0>;
+		brightness-levels = <0 8 48 255>;
+		num-interpolated-steps = <5>;
+		default-brightness-level = <0>;
+		power-supply = <&reg_3v3>;
+	};
+
 	connector {
 		compatible = "composite-video-connector";
 		label = "Composite0";
@@ -370,6 +380,12 @@ &pwm1 {
 	status = "okay";
 };
 
+&pwm2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_pwm2>;
+	status = "okay";
+};
+
 &pwm3 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_pwm3>;
@@ -601,6 +617,12 @@ MX6QDL_PAD_DISP0_DAT8__PWM1_OUT			0x1b0b0
 		>;
 	};
 
+	pinctrl_pwm2: pwm2grp {
+		fsl,pins = <
+			MX6QDL_PAD_DISP0_DAT9__PWM2_OUT			0x1b0b0
+		>;
+	};
+
 	pinctrl_pwm3: pwm3grp {
 		fsl,pins = <
 			MX6QDL_PAD_SD4_DAT1__PWM3_OUT			0x1b0b0
-- 
2.32.0
Re: [PATCH] arm: dts: imx6qdl-vicut1.dtsi: Add backlight-isb node
Posted by Krzysztof Kozlowski 3 years, 8 months ago
On 25/07/2022 14:48, David Jander wrote:
> On some older hardware models this is needed. It will do no harm if used
> with newer models though.
> 
> Signed-off-by: David Jander <david@protonic.nl>
> ---
>  arch/arm/boot/dts/imx6qdl-vicut1.dtsi | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx6qdl-vicut1.dtsi b/arch/arm/boot/dts/imx6qdl-vicut1.dtsi
> index a1676b5d2980..08d425020088 100644
> --- a/arch/arm/boot/dts/imx6qdl-vicut1.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-vicut1.dtsi
> @@ -37,6 +37,16 @@ backlight_led: backlight_led {
>  		power-supply = <&reg_3v3>;
>  	};
>  
> +	/* only for backwards compatibility with old HW */
> +	backlight_isb: backlight_isb {

Node name: backlight-isb

I know that other node had underscore, but these are not correct for names.


Best regards,
Krzysztof
Re: [PATCH] arm: dts: imx6qdl-vicut1.dtsi: Add backlight-isb node
Posted by David Jander 3 years, 8 months ago
On Mon, 25 Jul 2022 22:33:12 +0200
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> On 25/07/2022 14:48, David Jander wrote:
> > On some older hardware models this is needed. It will do no harm if used
> > with newer models though.
> > 
> > Signed-off-by: David Jander <david@protonic.nl>
> > ---
> >  arch/arm/boot/dts/imx6qdl-vicut1.dtsi | 22 ++++++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/imx6qdl-vicut1.dtsi b/arch/arm/boot/dts/imx6qdl-vicut1.dtsi
> > index a1676b5d2980..08d425020088 100644
> > --- a/arch/arm/boot/dts/imx6qdl-vicut1.dtsi
> > +++ b/arch/arm/boot/dts/imx6qdl-vicut1.dtsi
> > @@ -37,6 +37,16 @@ backlight_led: backlight_led {
> >  		power-supply = <&reg_3v3>;
> >  	};
> >  
> > +	/* only for backwards compatibility with old HW */
> > +	backlight_isb: backlight_isb {  
> 
> Node name: backlight-isb
> 
> I know that other node had underscore, but these are not correct for names.

I know the names are not correct, but they are in the style of existing node
names in the same file. Do you want me to change them all, or just this one?
This change would break existing user-space software though, which is something
I prefer not to do. Unfortunately both names are relied upon by user-space
software for almost 10 years now, and changing them will force me to maintain
off-tree patches that revert these changes.
I understand the reason for the change though, and if you insist I will do it.

Best regards,

-- 
David Jander
Re: [PATCH] arm: dts: imx6qdl-vicut1.dtsi: Add backlight-isb node
Posted by Krzysztof Kozlowski 3 years, 8 months ago
On 26/07/2022 08:30, David Jander wrote:
> On Mon, 25 Jul 2022 22:33:12 +0200
> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> 
>> On 25/07/2022 14:48, David Jander wrote:
>>> On some older hardware models this is needed. It will do no harm if used
>>> with newer models though.
>>>
>>> Signed-off-by: David Jander <david@protonic.nl>
>>> ---
>>>  arch/arm/boot/dts/imx6qdl-vicut1.dtsi | 22 ++++++++++++++++++++++
>>>  1 file changed, 22 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/imx6qdl-vicut1.dtsi b/arch/arm/boot/dts/imx6qdl-vicut1.dtsi
>>> index a1676b5d2980..08d425020088 100644
>>> --- a/arch/arm/boot/dts/imx6qdl-vicut1.dtsi
>>> +++ b/arch/arm/boot/dts/imx6qdl-vicut1.dtsi
>>> @@ -37,6 +37,16 @@ backlight_led: backlight_led {
>>>  		power-supply = <&reg_3v3>;
>>>  	};
>>>  
>>> +	/* only for backwards compatibility with old HW */
>>> +	backlight_isb: backlight_isb {  
>>
>> Node name: backlight-isb
>>
>> I know that other node had underscore, but these are not correct for names.
> 
> I know the names are not correct, but they are in the style of existing node
> names in the same file. Do you want me to change them all, or just this one?
> This change would break existing user-space software though, which is something
> I prefer not to do. Unfortunately both names are relied upon by user-space
> software for almost 10 years now, and changing them will force me to maintain
> off-tree patches that revert these changes.
> I understand the reason for the change though, and if you insist I will do it.

Node names are not ABI, thus any user-space depending on them does it
wrong. Could you link/name which user-space depends on it?

I don't understand why new node is also already a dependency. It's a new
node, there cannot be anything which depends on it before it is being
applied in upstream.


Best regards,
Krzysztof
Re: [PATCH] arm: dts: imx6qdl-vicut1.dtsi: Add backlight-isb node
Posted by David Jander 3 years, 8 months ago
On Tue, 26 Jul 2022 09:01:47 +0200
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> On 26/07/2022 08:30, David Jander wrote:
> > On Mon, 25 Jul 2022 22:33:12 +0200
> > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> >   
> >> On 25/07/2022 14:48, David Jander wrote:  
> >>> On some older hardware models this is needed. It will do no harm if used
> >>> with newer models though.
> >>>
> >>> Signed-off-by: David Jander <david@protonic.nl>
> >>> ---
> >>>  arch/arm/boot/dts/imx6qdl-vicut1.dtsi | 22 ++++++++++++++++++++++
> >>>  1 file changed, 22 insertions(+)
> >>>
> >>> diff --git a/arch/arm/boot/dts/imx6qdl-vicut1.dtsi b/arch/arm/boot/dts/imx6qdl-vicut1.dtsi
> >>> index a1676b5d2980..08d425020088 100644
> >>> --- a/arch/arm/boot/dts/imx6qdl-vicut1.dtsi
> >>> +++ b/arch/arm/boot/dts/imx6qdl-vicut1.dtsi
> >>> @@ -37,6 +37,16 @@ backlight_led: backlight_led {
> >>>  		power-supply = <&reg_3v3>;
> >>>  	};
> >>>  
> >>> +	/* only for backwards compatibility with old HW */
> >>> +	backlight_isb: backlight_isb {    
> >>
> >> Node name: backlight-isb
> >>
> >> I know that other node had underscore, but these are not correct for names.  
> > 
> > I know the names are not correct, but they are in the style of existing node
> > names in the same file. Do you want me to change them all, or just this one?
> > This change would break existing user-space software though, which is something
> > I prefer not to do. Unfortunately both names are relied upon by user-space
> > software for almost 10 years now, and changing them will force me to maintain
> > off-tree patches that revert these changes.
> > I understand the reason for the change though, and if you insist I will do it.  
> 
> Node names are not ABI, thus any user-space depending on them does it
> wrong. Could you link/name which user-space depends on it?
>
> I don't understand why new node is also already a dependency. It's a new
> node, there cannot be anything which depends on it before it is being
> applied in upstream.

Never mind. I already sent out v2 which fixes both node names.
Thanks.

Best regards,

-- 
David Jander