diff mbox series

arm64: dts: imx8mm-evk: add pwm1/backlight support

Message ID 20220413102052.20207-1-tommaso.merciai@amarulasolutions.com (mailing list archive)
State New, archived
Headers show
Series arm64: dts: imx8mm-evk: add pwm1/backlight support | expand

Commit Message

Tommaso Merciai April 13, 2022, 10:20 a.m. UTC 
Add pwm1/backlight support nodes for imx8mm_evk board. Align with u-boot
dts

References:
 - https://patchwork.ozlabs.org/project/uboot/patch/20220326111911.13720-9-tommaso.merciai@amarulasolutions.com/

Signed-off-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
---
 arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)

Comments

Krzysztof Kozlowski April 13, 2022, 10:28 a.m. UTC | #1 
On 13/04/2022 12:20, Tommaso Merciai wrote:
> Add pwm1/backlight support nodes for imx8mm_evk board. Align with u-boot
> dts
> 
> References:
>  - https://patchwork.ozlabs.org/project/uboot/patch/20220326111911.13720-9-tommaso.merciai@amarulasolutions.com/
> 
> Signed-off-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
> ---
>  arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi | 21 +++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi
> index 6d67df7692f1..55566708f667 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi
> @@ -59,6 +59,15 @@ reg_usdhc2_vmmc: regulator-usdhc2 {
>  		enable-active-high;
>  	};
>  
> +	backlight: backlight {
> +		status = "disabled";

Why disabled?

> +		compatible = "pwm-backlight";
> +		pwms = <&pwm1 0 5000000>;
> +		brightness-levels = <0 255>;
> +		num-interpolated-steps = <255>;
> +		default-brightness-level = <250>;
> +	};
> +
>  	ir-receiver {
>  		compatible = "gpio-ir-receiver";
>  		gpios = <&gpio1 13 GPIO_ACTIVE_LOW>;
> @@ -395,6 +404,12 @@ &wdog1 {
>  	status = "okay";
>  };
>  
> +&pwm1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_backlight>;
> +	status = "disabled";

Same here.


Best regards,
Krzysztof
Tommaso Merciai April 13, 2022, 11:58 a.m. UTC | #2 
On Wed, Apr 13, 2022 at 12:28:00PM +0200, Krzysztof Kozlowski wrote:
> On 13/04/2022 12:20, Tommaso Merciai wrote:
> > Add pwm1/backlight support nodes for imx8mm_evk board. Align with u-boot
> > dts
> > 
> > References:
> >  - https://patchwork.ozlabs.org/project/uboot/patch/20220326111911.13720-9-tommaso.merciai@amarulasolutions.com/
> > 
> > Signed-off-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
> > ---
> >  arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi | 21 +++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi
> > index 6d67df7692f1..55566708f667 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi
> > @@ -59,6 +59,15 @@ reg_usdhc2_vmmc: regulator-usdhc2 {
> >  		enable-active-high;
> >  	};
> >  
> > +	backlight: backlight {
> > +		status = "disabled";
> 
> Why disabled?
> 
> > +		compatible = "pwm-backlight";
> > +		pwms = <&pwm1 0 5000000>;
> > +		brightness-levels = <0 255>;
> > +		num-interpolated-steps = <255>;
> > +		default-brightness-level = <250>;
> > +	};
> > +
> >  	ir-receiver {
> >  		compatible = "gpio-ir-receiver";
> >  		gpios = <&gpio1 13 GPIO_ACTIVE_LOW>;
> > @@ -395,6 +404,12 @@ &wdog1 {
> >  	status = "okay";
> >  };
> >  
> > +&pwm1 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&pinctrl_backlight>;
> > +	status = "disabled";
> 
> Same here.
> 
> 
> Best regards,
> Krzysztof

Hi Krzysztof,
I think is better to keep disable into .dtsi and enable it at .dts
level.
What do you think about?

Regards,
Tommaso
Krzysztof Kozlowski April 13, 2022, 12:27 p.m. UTC | #3 
On 13/04/2022 13:58, Tommaso Merciai wrote:
>>> +	backlight: backlight {
>>> +		status = "disabled";
>>
>> Why disabled?
>>
>>> +		compatible = "pwm-backlight";
>>> +		pwms = <&pwm1 0 5000000>;
>>> +		brightness-levels = <0 255>;
>>> +		num-interpolated-steps = <255>;
>>> +		default-brightness-level = <250>;
>>> +	};
>>> +
>>>  	ir-receiver {
>>>  		compatible = "gpio-ir-receiver";
>>>  		gpios = <&gpio1 13 GPIO_ACTIVE_LOW>;
>>> @@ -395,6 +404,12 @@ &wdog1 {
>>>  	status = "okay";
>>>  };
>>>  
>>> +&pwm1 {
>>> +	pinctrl-names = "default";
>>> +	pinctrl-0 = <&pinctrl_backlight>;
>>> +	status = "disabled";
>>
>> Same here.
>>
>>
>> Best regards,
>> Krzysztof
> 
> Hi Krzysztof,
> I think is better to keep disable into .dtsi and enable it at .dts
> level.
> What do you think about?

Why better? This is already board DTSI, not a SoC DTSI. All boards using
it are supposed to have it available, aren't they?

Usually nodes should be disabled in a DTSI if they need some resources
not available in that DTSI. For example if they need some supply. It's
not the case here, right?


Best regards,
Krzysztof
Tommaso Merciai April 13, 2022, 12:40 p.m. UTC | #4 
On Wed, Apr 13, 2022 at 02:27:00PM +0200, Krzysztof Kozlowski wrote:
> On 13/04/2022 13:58, Tommaso Merciai wrote:
> >>> +	backlight: backlight {
> >>> +		status = "disabled";
> >>
> >> Why disabled?
> >>
> >>> +		compatible = "pwm-backlight";
> >>> +		pwms = <&pwm1 0 5000000>;
> >>> +		brightness-levels = <0 255>;
> >>> +		num-interpolated-steps = <255>;
> >>> +		default-brightness-level = <250>;
> >>> +	};
> >>> +
> >>>  	ir-receiver {
> >>>  		compatible = "gpio-ir-receiver";
> >>>  		gpios = <&gpio1 13 GPIO_ACTIVE_LOW>;
> >>> @@ -395,6 +404,12 @@ &wdog1 {
> >>>  	status = "okay";
> >>>  };
> >>>  
> >>> +&pwm1 {
> >>> +	pinctrl-names = "default";
> >>> +	pinctrl-0 = <&pinctrl_backlight>;
> >>> +	status = "disabled";
> >>
> >> Same here.
> >>
> >>
> >> Best regards,
> >> Krzysztof
> > 
> > Hi Krzysztof,
> > I think is better to keep disable into .dtsi and enable it at .dts
> > level.
> > What do you think about?
> 
> Why better? This is already board DTSI, not a SoC DTSI. All boards using
> it are supposed to have it available, aren't they?
> 
> Usually nodes should be disabled in a DTSI if they need some resources
> not available in that DTSI. For example if they need some supply. It's
> not the case here, right?

Hi Krzysztof,
Yes, right I check both schematics (DSI_BL_PWM). We can set PWM enable.
I'll update status in v2.

> 
> 
> Best regards,
> Krzysztof

Regards,
Tommaso
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi
index 6d67df7692f1..55566708f667 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi
@@ -59,6 +59,15 @@  reg_usdhc2_vmmc: regulator-usdhc2 {
 		enable-active-high;
 	};
 
+	backlight: backlight {
+		status = "disabled";
+		compatible = "pwm-backlight";
+		pwms = <&pwm1 0 5000000>;
+		brightness-levels = <0 255>;
+		num-interpolated-steps = <255>;
+		default-brightness-level = <250>;
+	};
+
 	ir-receiver {
 		compatible = "gpio-ir-receiver";
 		gpios = <&gpio1 13 GPIO_ACTIVE_LOW>;
@@ -395,6 +404,12 @@  &wdog1 {
 	status = "okay";
 };
 
+&pwm1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_backlight>;
+	status = "disabled";
+};
+
 &iomuxc {
 	pinctrl_fec1: fec1grp {
 		fsl,pins = <
@@ -549,4 +564,10 @@  pinctrl_wdog: wdoggrp {
 			MX8MM_IOMUXC_GPIO1_IO02_WDOG1_WDOG_B	0x166
 		>;
 	};
+
+	pinctrl_backlight: backlightgrp {
+		fsl,pins = <
+			MX8MM_IOMUXC_GPIO1_IO01_PWM1_OUT	0x06
+		>;
+	};
 };