Support PWM polarity control

PATCH Superseded Bhuvanchandra DV 3 participants 17 messages Oct 1, 2016 – Oct 7, 2016

Subsystems (3)

Device tree bindings Arm i.MX and Layerscape PWM

lore

Share & tools

lore

Bhuvanchandra DV

author

2016-10-01 10:12 UTC #0 lore raw

v2 cover Support PWM polarity control

Changes since v2:

- Picked the stalled patchset[1] from Lothar Wassmann which adds the basic support for polarity control on imx-pwm driver and adds backward compatibility support for devices which does not have polarity control feature.

Changes since Lothars v6:

- Squash Lukasz patch[2].

[1] http://thread.gmane.org/gmane.linux.pwm/1621
[2] https://www.spinics.net/lists/arm-kernel/msg530818.html

Bhuvanchandra DV (3):
arm: dts: imx7: Update #pwm-cells for PWM polarity control
arm: dts: imx7-colibri: Use pwm polarity control
arm: dts: imx7-colibri: Use enable-gpios for BL_ON

Lothar Wassmann (3):
pwm: print error messages with pr_err() instead of pr_debug()
pwm: core: make the PWM_POLARITY flag in DTB optional
pwm: imx: support output polarity inversion

 Documentation/devicetree/bindings/pwm/imx-pwm.txt |  6 +--
 arch/arm/boot/dts/imx7-colibri.dtsi               | 12 +++++-
 arch/arm/boot/dts/imx7s.dtsi                      |  8 ++--
 drivers/pwm/core.c                                | 31 ++++++++------
 drivers/pwm/pwm-imx.c                             | 51 +++++++++++++++++++++--
 5 files changed, 83 insertions(+), 25 deletions(-)

Signature · 1 line

2.9.2

Bhuvanchandra DV

author

2016-10-01 10:12 UTC #1 lore raw

PATCH v2 1/6 pwm: print error messages with pr_err() instead of pr_debug()

From: Lothar Wassmann <[email protected]>

Make the messages that are printed in case of fatal errors actually visible to the user without having to recompile the driver with debugging enabled.

Signed-off-by Lothar Wa?mann <[email protected]>

Signed-off-by Bhuvanchandra DV <[email protected]>

---
 drivers/pwm/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/pwm/core.c +2 -2

index 0dbd29e..195373e 100644
@@ -661,13 +661,13 @@ struct pwm_device *of_pwm_get(struct device_node *np, const char *con_id)
 	err = of_parse_phandle_with_args(np, "pwms", "#pwm-cells", index,
 					 &args);
 	if (err) {
-		pr_debug("%s(): can't parse \"pwms\" property\n", __func__);
+		pr_err("%s(): can't parse \"pwms\" property\n", __func__);
 		return ERR_PTR(err);
 	}
 
 	pc = of_node_to_pwmchip(args.np);
 	if (IS_ERR(pc)) {
-		pr_debug("%s(): PWM chip not found\n", __func__);
+		pr_err("%s(): PWM chip not found\n", __func__);
 		pwm = ERR_CAST(pc);
 		goto put;
 	}

Signature · 1 line

2.9.2

Bhuvanchandra DV

author

2016-10-01 10:12 UTC #2 lore raw

PATCH v2 2/6 pwm: core: make the PWM_POLARITY flag in DTB optional

In reply to Bhuvanchandra DV

From: Lothar Wassmann <[email protected]>

Change the pwm chip driver registration, so that a chip driver that supports polarity inversion can still be used with DTBs that don't provide the 'PWM_POLARITY' flag.

This is done to provide polarity inversion support for the pwm-imx driver without having to modify all existing DTS files.

Signed-off-by Lothar Wassmann <[email protected]>

Signed-off-by Bhuvanchandra DV <[email protected]>

---
 drivers/pwm/core.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

drivers/pwm/core.c +16 ~~-11~~

index 195373e..aae8db3 100644
@@ -137,9 +137,14 @@ of_pwm_xlate_with_flags(struct pwm_chip *pc, const struct of_phandle_args *args)
 {
 	struct pwm_device *pwm;
 
+	/* check, whether the driver supports a third cell for flags */
 	if (pc->of_pwm_n_cells < 3)
 		return ERR_PTR(-EINVAL);
 
+	/* flags in the third cell are optional */
+	if (args->args_count < 2)
+		return ERR_PTR(-EINVAL);
+
 	if (args->args[0] >= pc->npwm)
 		return ERR_PTR(-EINVAL);

@@ -149,10 +154,12 @@ of_pwm_xlate_with_flags(struct pwm_chip *pc, const struct of_phandle_args *args)
 
 	pwm->args.period = args->args[1];
 
-	if (args->args[2] & PWM_POLARITY_INVERTED)
-		pwm->args.polarity = PWM_POLARITY_INVERSED;
-	else
-		pwm->args.polarity = PWM_POLARITY_NORMAL;
+	if (args->args_count > 2) {
+		if (args->args[2] & PWM_POLARITY_INVERTED)
+			pwm_set_polarity(pwm, PWM_POLARITY_INVERSED);
+		else
+			pwm_set_polarity(pwm, PWM_POLARITY_NORMAL);
+	}
 
 	return pwm;
 }

@@ -163,9 +170,14 @@ of_pwm_simple_xlate(struct pwm_chip *pc, const struct of_phandle_args *args)
 {
 	struct pwm_device *pwm;
 
+	/* sanity check driver support */
 	if (pc->of_pwm_n_cells < 2)
 		return ERR_PTR(-EINVAL);
 
+	/* all cells are required */
+	if (args->args_count != pc->of_pwm_n_cells)
+		return ERR_PTR(-EINVAL);
+
 	if (args->args[0] >= pc->npwm)
 		return ERR_PTR(-EINVAL);

@@ -672,13 +684,6 @@ struct pwm_device *of_pwm_get(struct device_node *np, const char *con_id)
 		goto put;
 	}
 
-	if (args.args_count != pc->of_pwm_n_cells) {
-		pr_debug("%s: wrong #pwm-cells for %s\n", np->full_name,
-			 args.np->full_name);
-		pwm = ERR_PTR(-EINVAL);
-		goto put;
-	}
-
 	pwm = pc->of_xlate(pc, &args);
 	if (IS_ERR(pwm))
 		goto put;

Signature · 1 line

2.9.2

Lukasz Majewski

2016-10-06 06:36 UTC #3 lore raw

Re: [PATCH v2 2/6] pwm: core: make the PWM_POLARITY flag in DTB optional

Hi Bhuvanchandra,

Quoted 36 lines

From: Lothar Wassmann <[email protected]>
Change the pwm chip driver registration, so that a chip driver that supports polarity inversion can still be used with DTBs that don't provide the 'PWM_POLARITY' flag.
This is done to provide polarity inversion support for the pwm-imx driver without having to modify all existing DTS files.
Signed-off-by Lothar Wassmann <[email protected]>
Signed-off-by Bhuvanchandra DV <[email protected]>
Suggested-by Thierry Reding <[email protected]>
---
 drivers/pwm/core.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)
drivers/pwm/core.c +5 -0
index 195373e..aae8db3 100644
@@ -137,9 +137,14 @@ of_pwm_xlate_with_flags(struct pwm_chip *pc,
 const struct of_phandle_args *args) {
 	struct pwm_device *pwm;
 
+	/* check, whether the driver supports a third cell for flags
 */ if (pc->of_pwm_n_cells < 3)
 		return ERR_PTR(-EINVAL);
 
+	/* flags in the third cell are optional */
+	if (args->args_count < 2)
+		return ERR_PTR(-EINVAL);
+
 	if (args->args[0] >= pc->npwm)
 		return ERR_PTR(-EINVAL);
 

drivers/pwm/core.c of_pwm_xlate_with_flags(struct pwm_chip *pc, · line 159

Quoted 10 lines

drivers/pwm/core.c +3 -4

@@ -149,10 +154,12 @@ of_pwm_xlate_with_flags(struct pwm_chip *pc,
 const struct of_phandle_args *args) 
 	pwm->args.period = args->args[1];
 
-	if (args->args[2] & PWM_POLARITY_INVERTED)
-		pwm->args.polarity = PWM_POLARITY_INVERSED;
-	else
-		pwm->args.polarity = PWM_POLARITY_NORMAL;
+	if (args->args_count > 2) {
+		if (args->args[2] & PWM_POLARITY_INVERTED)
+			pwm_set_polarity(pwm, PWM_POLARITY_INVERSED);

^^^^^^^^^^^^^^^^
here we should set pwm->args.polarity, since the pwm_set_polarity() calls pwm_apply_state() which requires duty_cycle and period to be set.

In this particular moment it is not yet set and polarity is not properly configured.

Patch fixing this will be sent as a reply to this e-mail. Please just squash it and test on your platform.

Best regards,
?ukasz Majewski

Quoted 35 lines

drivers/pwm/core.c +8 -7

+		else
+			pwm_set_polarity(pwm, PWM_POLARITY_NORMAL);
+	}
 
 	return pwm;
 }

@@ -163,9 +170,14 @@ of_pwm_simple_xlate(struct pwm_chip *pc, const
 struct of_phandle_args *args) {
 	struct pwm_device *pwm;
 
+	/* sanity check driver support */
 	if (pc->of_pwm_n_cells < 2)
 		return ERR_PTR(-EINVAL);
 
+	/* all cells are required */
+	if (args->args_count != pc->of_pwm_n_cells)
+		return ERR_PTR(-EINVAL);
+
 	if (args->args[0] >= pc->npwm)
 		return ERR_PTR(-EINVAL);

@@ -672,13 +684,6 @@ struct pwm_device *of_pwm_get(struct device_node
 *np, const char *con_id) goto put;
 	}
 
-	if (args.args_count != pc->of_pwm_n_cells) {
-		pr_debug("%s: wrong #pwm-cells for %s\n",
 np->full_name,
-			 args.np->full_name);
-		pwm = ERR_PTR(-EINVAL);
-		goto put;
-	}
-
 	pwm = pc->of_xlate(pc, &args);
 	if (IS_ERR(pwm))

Quoted 1 line

goto put;

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161006/743020b3/attachment.sig>

Bhuvanchandra DV

author

2016-10-07 04:49 UTC #4 lore raw

Re: [PATCH v2 2/6] pwm: core: make the PWM_POLARITY flag in DTB optional

Hi Lukasz,

Quoted 53 lines · Lukasz Majewski

Hi Bhuvanchandra,
Quoted 36 lines
From: Lothar Wassmann <[email protected]>
Change the pwm chip driver registration, so that a chip driver that supports polarity inversion can still be used with DTBs that don't provide the 'PWM_POLARITY' flag.
This is done to provide polarity inversion support for the pwm-imx driver without having to modify all existing DTS files.
Signed-off-by Lothar Wassmann <[email protected]>
Signed-off-by Bhuvanchandra DV <[email protected]>
Suggested-by Thierry Reding <[email protected]>
---
  drivers/pwm/core.c | 27 ++++++++++++++++-----------
  1 file changed, 16 insertions(+), 11 deletions(-)
drivers/pwm/core.c +5 -0
index 195373e..aae8db3 100644
@@ -137,9 +137,14 @@ of_pwm_xlate_with_flags(struct pwm_chip *pc,
 const struct of_phandle_args *args) {
  	struct pwm_device *pwm;
 
+	/* check, whether the driver supports a third cell for flags
 */ if (pc->of_pwm_n_cells < 3)
  		return ERR_PTR(-EINVAL);
 
+	/* flags in the third cell are optional */
+	if (args->args_count < 2)
+		return ERR_PTR(-EINVAL);
+
  	if (args->args[0] >= pc->npwm)
  		return ERR_PTR(-EINVAL);
 
drivers/pwm/core.c of_pwm_xlate_with_flags(struct pwm_chip *pc, · line 159
Quoted 10 lines
drivers/pwm/core.c +3 -4
@@ -149,10 +154,12 @@ of_pwm_xlate_with_flags(struct pwm_chip *pc,
 const struct of_phandle_args *args)
  	pwm->args.period = args->args[1];
 
-	if (args->args[2] & PWM_POLARITY_INVERTED)
-		pwm->args.polarity = PWM_POLARITY_INVERSED;
-	else
-		pwm->args.polarity = PWM_POLARITY_NORMAL;
+	if (args->args_count > 2) {
+		if (args->args[2] & PWM_POLARITY_INVERTED)
+			pwm_set_polarity(pwm, PWM_POLARITY_INVERSED);
^^^^^^^^^^^^^^^^
here we should set pwm->args.polarity, since the pwm_set_polarity() calls pwm_apply_state() which requires duty_cycle and period to be set.

Quoted 2 lines

In this particular moment it is not yet set and polarity is not properly configured.

Agreed. Will do a clean v3 patchset along with the patch you sent (pwm: core: Use pwm->args.polarity to setup PWM_POLARITY_INVERSED).

--
Bhuvan

Quoted 43 lines

Patch fixing this will be sent as a reply to this e-mail. Please just squash it and test on your platform.

Best regards,
?ukasz Majewski

Quoted 36 lines

drivers/pwm/core.c +8 -7

+		else
+			pwm_set_polarity(pwm, PWM_POLARITY_NORMAL);
+	}
 
  	return pwm;
  }

@@ -163,9 +170,14 @@ of_pwm_simple_xlate(struct pwm_chip *pc, const
 struct of_phandle_args *args) {
  	struct pwm_device *pwm;
 
+	/* sanity check driver support */
  	if (pc->of_pwm_n_cells < 2)
  		return ERR_PTR(-EINVAL);
 
+	/* all cells are required */
+	if (args->args_count != pc->of_pwm_n_cells)
+		return ERR_PTR(-EINVAL);
+
  	if (args->args[0] >= pc->npwm)
  		return ERR_PTR(-EINVAL);

@@ -672,13 +684,6 @@ struct pwm_device *of_pwm_get(struct device_node
 *np, const char *con_id) goto put;
  	}
 
-	if (args.args_count != pc->of_pwm_n_cells) {
-		pr_debug("%s: wrong #pwm-cells for %s\n",
 np->full_name,
-			 args.np->full_name);
-		pwm = ERR_PTR(-EINVAL);
-		goto put;
-	}
-
  	pwm = pc->of_xlate(pc, &args);
  	if (IS_ERR(pwm))

goto put;

Lukasz Majewski

2016-10-06 06:49 UTC #5 lore raw

PATCH 1/1 pwm: core: Use pwm->args.polarity to setup PWM_POLARITY_INVERSED

In reply to Bhuvanchandra DV

The pwm_set_polarity() function finally calls pwm_apply_state(), which in turn requires state->duty_cycle and state->period properly set.

In the moment when polarity is set, the PWM is disabled and not configured. For that reason both above variables are set to 0 and the polarity is not set.

To be sure that polarity is setup, one needs to set pwm->args.polarity, which controls MX3_PWMCR_POUTC bit setting at imx_pwm_config_v2().

Signed-off-by Lukasz Majewski <[email protected]>

---
This patch should be applied on top of:

"[v2,2/6] pwm: core: make the PWM_POLARITY flag in DTB optional"

http://patchwork.ozlabs.org/patch/677330/

---
 drivers/pwm/core.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

drivers/pwm/core.c +3 -5

index 1f62668..6cd6004 100644
@@ -153,13 +153,11 @@ of_pwm_xlate_with_flags(struct pwm_chip *pc, const struct of_phandle_args *args)
 		return pwm;
 
 	pwm->args.period = args->args[1];
+	pwm->args.polarity = PWM_POLARITY_NORMAL;
 
-	if (args->args_count > 2) {
+	if (args->args_count > 2)
 		if (args->args[2] & PWM_POLARITY_INVERTED)
-			pwm_set_polarity(pwm, PWM_POLARITY_INVERSED);
-		else
-			pwm_set_polarity(pwm, PWM_POLARITY_NORMAL);
-	}
+			pwm->args.polarity = PWM_POLARITY_INVERSED;
 
 	return pwm;
 }

Signature · 1 line

2.1.4

Bhuvanchandra DV

author

2016-10-01 10:12 UTC #6 lore raw

PATCH v2 3/6 pwm: imx: support output polarity inversion

In reply to Bhuvanchandra DV

From: Lothar Wassmann <[email protected]>

The i.MX pwm unit on i.MX27 and newer SoCs provides a configurable output polarity. This patch adds support to utilize this feature where available.

Signed-off-by Lothar Wa?mann <[email protected]>

Signed-off-by Lukasz Majewski <[email protected]>

Signed-off-by Bhuvanchandra DV <[email protected]>

Acked-by Shawn Guo <[email protected]>

Reviewed-by Sascha Hauer <[email protected]>

---
 Documentation/devicetree/bindings/pwm/imx-pwm.txt |  6 +--
 drivers/pwm/pwm-imx.c                             | 51 +++++++++++++++++++++--
 2 files changed, 51 insertions(+), 6 deletions(-)

Documentation/devicetree/bindings/pwm/imx-pwm.txt +3 -3

index e00c2e9..c61bdf8 100644
@@ -6,8 +6,8 @@ Required properties:
   - "fsl,imx1-pwm" for PWM compatible with the one integrated on i.MX1
   - "fsl,imx27-pwm" for PWM compatible with the one integrated on i.MX27
 - reg: physical base address and length of the controller's registers
-- #pwm-cells: should be 2. See pwm.txt in this directory for a description of
-  the cells format.
+- #pwm-cells: 2 for i.MX1 and 3 for i.MX27 and newer SoCs. See pwm.txt
+  in this directory for a description of the cells format.
 - clocks : Clock specifiers for both ipg and per clocks.
 - clock-names : Clock names should include both "ipg" and "per"
 See the clock consumer binding,

@@ -17,7 +17,7 @@ See the clock consumer binding,
 Example:
 
 pwm1: pwm at 53fb4000 {
-	#pwm-cells = <2>;
+	#pwm-cells = <3>;
 	compatible = "fsl,imx53-pwm", "fsl,imx27-pwm";
 	reg = <0x53fb4000 0x4000>;
 	clocks = <&clks IMX5_CLK_PWM1_IPG_GATE>,

drivers/pwm/pwm-imx.c +48 -3

111 lines not on this page · raw message

Signature · 1 line

2.9.2

Bhuvanchandra DV

author

2016-10-01 10:12 UTC #7 lore raw

PATCH v2 4/6 arm: dts: imx7: Update #pwm-cells for PWM polarity control

In reply to Bhuvanchandra DV

Update #pwm-cells to 3 in order to support PWM signal polarity control.

Signed-off-by Bhuvanchandra DV <[email protected]>

Acked-by Rob Herring <[email protected]>

---
 arch/arm/boot/dts/imx7s.dtsi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

arch/arm/boot/dts/imx7s.dtsi +4 -4

index 0d7d5ac..8d1d471 100644
@@ -601,7 +601,7 @@
 				clocks = <&clks IMX7D_PWM1_ROOT_CLK>,
 					 <&clks IMX7D_PWM1_ROOT_CLK>;
 				clock-names = "ipg", "per";
-				#pwm-cells = <2>;
+				#pwm-cells = <3>;
 				status = "disabled";
 			};

@@ -612,7 +612,7 @@
 				clocks = <&clks IMX7D_PWM2_ROOT_CLK>,
 					 <&clks IMX7D_PWM2_ROOT_CLK>;
 				clock-names = "ipg", "per";
-				#pwm-cells = <2>;
+				#pwm-cells = <3>;
 				status = "disabled";
 			};

@@ -623,7 +623,7 @@
 				clocks = <&clks IMX7D_PWM3_ROOT_CLK>,
 					 <&clks IMX7D_PWM3_ROOT_CLK>;
 				clock-names = "ipg", "per";
-				#pwm-cells = <2>;
+				#pwm-cells = <3>;
 				status = "disabled";
 			};

@@ -634,7 +634,7 @@
 				clocks = <&clks IMX7D_PWM4_ROOT_CLK>,
 					 <&clks IMX7D_PWM4_ROOT_CLK>;
 				clock-names = "ipg", "per";
-				#pwm-cells = <2>;
+				#pwm-cells = <3>;
 				status = "disabled";
 			};

Signature · 1 line

2.9.2

Bhuvanchandra DV

author

2016-10-01 10:12 UTC #8 lore raw

PATCH v2 5/6 arm: dts: imx7-colibri: Use pwm polarity control

In reply to Bhuvanchandra DV

Configure PWM polarity control.

Signed-off-by Bhuvanchandra DV <[email protected]>

---
 arch/arm/boot/dts/imx7-colibri.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/arm/boot/dts/imx7-colibri.dtsi +1 -1

index a9cc657..2af5e3e 100644
@@ -43,7 +43,7 @@
 / {
 	bl: backlight {
 		compatible = "pwm-backlight";
-		pwms = <&pwm1 0 5000000>;
+		pwms = <&pwm1 0 5000000 0>;
 	};
 
 	reg_module_3v3: regulator-module-3v3 {

Signature · 1 line

2.9.2

Lukasz Majewski

2016-10-06 06:40 UTC #9 lore raw

PATCH v2 5/6 arm: dts: imx7-colibri: Use pwm polarity control

Quoted 12 lines · On Sat, 1 Oct 2016 15:42:34 +0530 Bhuvanchandra DV…

Configure PWM polarity control.
Signed-off-by Bhuvanchandra DV <[email protected]>
---
 arch/arm/boot/dts/imx7-colibri.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
(quoted diff) +0 -0
b/arch/arm/boot/dts/imx7-colibri.dtsi index a9cc657..2af5e3e 100644

arch/arm/boot/dts/imx7-colibri.dtsi line 46

Quoted 5 lines

arch/arm/boot/dts/imx7-colibri.dtsi +1 -1

@@ -43,7 +43,7 @@
 / {
 	bl: backlight {
 		compatible = "pwm-backlight";
-		pwms = <&pwm1 0 5000000>;
+		pwms = <&pwm1 0 5000000 0>;

My recommendation would be to add:
#include <dt-bindings/pwm/pwm.h>

and then define pwms as:

pwms = <&pwm1 0 5000000 PWM_POLARITY_NORMAL>;

It would be more readable

Best regards,
?ukasz Majewski

arch/arm/boot/dts/imx7-colibri.dtsi line 49

Quoted 3 lines

arch/arm/boot/dts/imx7-colibri.dtsi +0 -0
 	};
 
 	reg_module_3v3: regulator-module-3v3 {

Bhuvanchandra DV

author

2016-10-07 04:49 UTC #10 lore raw

PATCH v2 5/6 arm: dts: imx7-colibri: Use pwm polarity control

Quoted 23 lines · Lukasz Majewski

Quoted 12 lines · On Sat, 1 Oct 2016 15:42:34 +0530 Bhuvanchandra DV…
Configure PWM polarity control.
Signed-off-by Bhuvanchandra DV <[email protected]>
---
  arch/arm/boot/dts/imx7-colibri.dtsi | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
(quoted diff) +0 -0
b/arch/arm/boot/dts/imx7-colibri.dtsi index a9cc657..2af5e3e 100644
arch/arm/boot/dts/imx7-colibri.dtsi line 46
Quoted 5 lines
arch/arm/boot/dts/imx7-colibri.dtsi +1 -1
@@ -43,7 +43,7 @@
  / {
  	bl: backlight {
  		compatible = "pwm-backlight";
-		pwms = <&pwm1 0 5000000>;
+		pwms = <&pwm1 0 5000000 0>;
My recommendation would be to add:
#include <dt-bindings/pwm/pwm.h>

Quoted 5 lines

and then define pwms as:
pwms = <&pwm1 0 5000000 PWM_POLARITY_NORMAL>;
It would be more readable

Ok, will add that.

--
Bhuvan

Quoted 7 lines

Best regards,
?ukasz Majewski
Quoted 3 lines
arch/arm/boot/dts/imx7-colibri.dtsi +0 -0
  	};
 
  	reg_module_3v3: regulator-module-3v3 {

Bhuvanchandra DV

author

2016-10-07 06:08 UTC #11 lore raw

PATCH v2 5/6 arm: dts: imx7-colibri: Use pwm polarity control

In reply to Lukasz Majewski

Quoted 23 lines · Lukasz Majewski

Quoted 12 lines · On Sat, 1 Oct 2016 15:42:34 +0530 Bhuvanchandra DV…
Configure PWM polarity control.
Signed-off-by Bhuvanchandra DV <[email protected]>
---
  arch/arm/boot/dts/imx7-colibri.dtsi | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
(quoted diff) +0 -0
b/arch/arm/boot/dts/imx7-colibri.dtsi index a9cc657..2af5e3e 100644
arch/arm/boot/dts/imx7-colibri.dtsi line 46
Quoted 5 lines
arch/arm/boot/dts/imx7-colibri.dtsi +1 -1
@@ -43,7 +43,7 @@
  / {
  	bl: backlight {
  		compatible = "pwm-backlight";
-		pwms = <&pwm1 0 5000000>;
+		pwms = <&pwm1 0 5000000 0>;
My recommendation would be to add:
#include <dt-bindings/pwm/pwm.h>

Quoted 3 lines

and then define pwms as:
pwms = <&pwm1 0 5000000 PWM_POLARITY_NORMAL>;

Addendum:
There is no "PWM_POLARITY_NORMAL" defined in "#include <dt-bindings/pwm/pwm.h>" I guess it is helpful when polarity inversion is enabled.

--
Bhuvan

Quoted 9 lines

It would be more readable
Best regards,
?ukasz Majewski
Quoted 3 lines
arch/arm/boot/dts/imx7-colibri.dtsi +0 -0
  	};
 
  	reg_module_3v3: regulator-module-3v3 {

Lukasz Majewski

2016-10-07 06:41 UTC #12 lore raw

PATCH v2 5/6 arm: dts: imx7-colibri: Use pwm polarity control

Quoted 25 lines · On Fri, 7 Oct 2016 11:38:52 +0530 Bhuvanchandra DV…

Quoted 23 lines · Lukasz Majewski
Quoted 12 lines · On Sat, 1 Oct 2016 15:42:34 +0530 Bhuvanchandra DV…
Configure PWM polarity control.
Signed-off-by Bhuvanchandra DV <[email protected]>
---
  arch/arm/boot/dts/imx7-colibri.dtsi | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
(quoted diff) +0 -0
b/arch/arm/boot/dts/imx7-colibri.dtsi index a9cc657..2af5e3e 100644
arch/arm/boot/dts/imx7-colibri.dtsi line 46
Quoted 5 lines
arch/arm/boot/dts/imx7-colibri.dtsi +1 -1
@@ -43,7 +43,7 @@
  / {
  	bl: backlight {
  		compatible = "pwm-backlight";
-		pwms = <&pwm1 0 5000000>;
+		pwms = <&pwm1 0 5000000 0>;
My recommendation would be to add:
#include <dt-bindings/pwm/pwm.h>

Quoted 8 lines

Quoted 3 lines
and then define pwms as:
pwms = <&pwm1 0 5000000 PWM_POLARITY_NORMAL>;
Addendum:
There is no "PWM_POLARITY_NORMAL" defined in "#include <dt-bindings/pwm/pwm.h>" I guess it is helpful when polarity inversion is enabled.

Yes, you are right :-), the normal polarity is assumed to be the default.

There is only defined flag for inversion.

Sorry for the noise.

Best regards,
?ukasz Majewski

Quoted 14 lines

--
Bhuvan
Quoted 9 lines
It would be more readable
Best regards,
?ukasz Majewski
Quoted 3 lines
arch/arm/boot/dts/imx7-colibri.dtsi +0 -0
  	};
 
  	reg_module_3v3: regulator-module-3v3 {

Bhuvanchandra DV

author

2016-10-01 10:12 UTC #13 lore raw

PATCH v2 6/6 arm: dts: imx7-colibri: Use enable-gpios for BL_ON

In reply to Bhuvanchandra DV

Use pwm-backlight driver 'enable-gpios' property for backlight on/off control.

Signed-off-by Bhuvanchandra DV <[email protected]>

---
 arch/arm/boot/dts/imx7-colibri.dtsi | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

arch/arm/boot/dts/imx7-colibri.dtsi +9 -1

index 2af5e3e..ce5edb5 100644
@@ -43,7 +43,10 @@
 / {
 	bl: backlight {
 		compatible = "pwm-backlight";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_gpio_bl_on>;
 		pwms = <&pwm1 0 5000000 0>;
+		enable-gpios = <&gpio5 1 GPIO_ACTIVE_HIGH>;
 	};
 
 	reg_module_3v3: regulator-module-3v3 {

@@ -356,7 +359,6 @@
 		fsl,pins = <
 			MX7D_PAD_ECSPI2_SS0__GPIO4_IO23		0x14 /* SODIMM 65 */
 			MX7D_PAD_SD1_CD_B__GPIO5_IO0		0x14 /* SODIMM 69 */
-			MX7D_PAD_SD1_WP__GPIO5_IO1		0x14 /* SODIMM 71 */
 			MX7D_PAD_I2C4_SDA__GPIO4_IO15		0x14 /* SODIMM 75 */
 			MX7D_PAD_ECSPI1_MISO__GPIO4_IO18	0x14 /* SODIMM 79 */
 			MX7D_PAD_I2C3_SCL__GPIO4_IO12		0x14 /* SODIMM 81 */

@@ -388,6 +390,12 @@
 		>;
 	};
 
+		pinctrl_gpio_bl_on: gpio-bl-on {
+			fsl,pins = <
+				MX7D_PAD_SD1_WP__GPIO5_IO1		0x14
+			>;
+		};
+
 	pinctrl_i2c1_int: i2c1-int-grp { /* PMIC / TOUCH */
 		fsl,pins = <
 			MX7D_PAD_GPIO1_IO13__GPIO1_IO13	0x79

Signature · 1 line

2.9.2

Lukasz Majewski

2016-10-04 07:48 UTC #14 lore raw

In reply to Bhuvanchandra DV

Dear Bhuvanchandra,

Thank you for your effort to send those patches to ML.

Quoted 19 lines

Changes since v2:
- Picked the stalled patchset[1] from Lothar Wassmann which adds the basic support for polarity control on imx-pwm driver and adds backward compatibility support for devices which does not have polarity control feature.
Changes since Lothars v6:
- Squash Lukasz patch[2].
[1] http://thread.gmane.org/gmane.linux.pwm/1621
[2] https://www.spinics.net/lists/arm-kernel/msg530818.html
Bhuvanchandra DV (3):
arm: dts: imx7: Update #pwm-cells for PWM polarity control
arm: dts: imx7-colibri: Use pwm polarity control
arm: dts: imx7-colibri: Use enable-gpios for BL_ON

Quoted 4 lines

Lothar Wassmann (3):
pwm: print error messages with pr_err() instead of pr_debug()
pwm: core: make the PWM_POLARITY flag in DTB optional
pwm: imx: support output polarity inversion

For some reason this patchset works differently than the one developed by Lothar.

The difference is with the brightness level control.

My brightness definition in DTS:

           pwms = <&pwm2 0 5000000 PWM_POLARITY_INVERTED>;

brightness-levels = < 0 1 2 3 4 5 6 7 8 9

					 ..  ............
				    250 251 252 253 254 255>;
		default-brightness-level = <50>;
		enable-gpios = <&gpio1 2 GPIO_ACTIVE_LOW>;

When I go to the backlight sysfs entry:

cd /sys/devices/soc0/backlight/backlight/backlight

It seems like the brightness level control is inverted - i.e. 'echo 20 > brightness" makes picture on the screen very bright, and 'echo 200 > brightness' makes the picture diminish.

With my "internal" patches the situation is opposite (and I've checked it with my HW connections).

Could you check on your setup if similar situation takes place? I mean if the brightness control works as expected?

Thanks in advance,
?ukasz Majewski

Quoted 9 lines

 Documentation/devicetree/bindings/pwm/imx-pwm.txt |  6 +--
 arch/arm/boot/dts/imx7-colibri.dtsi               | 12 +++++-
 arch/arm/boot/dts/imx7s.dtsi                      |  8 ++--
 drivers/pwm/core.c                                | 31 ++++++++------
 drivers/pwm/pwm-imx.c                             | 51

+++++++++++++++++++++-- 5 files changed, 83 insertions(+), 25 deletions(-)

Stefan Agner

2016-10-05 16:50 UTC #15 lore raw

Hi Lukasz,

Quoted 35 lines · Lukasz Majewski

Dear Bhuvanchandra,
Thank you for your effort to send those patches to ML.
Quoted 19 lines
Changes since v2:
- Picked the stalled patchset[1] from Lothar Wassmann which adds the basic support for polarity control on imx-pwm driver and adds backward compatibility support for devices which does not have polarity control feature.
Changes since Lothars v6:
- Squash Lukasz patch[2].
[1] http://thread.gmane.org/gmane.linux.pwm/1621
[2] https://www.spinics.net/lists/arm-kernel/msg530818.html
Bhuvanchandra DV (3):
arm: dts: imx7: Update #pwm-cells for PWM polarity control
arm: dts: imx7-colibri: Use pwm polarity control
arm: dts: imx7-colibri: Use enable-gpios for BL_ON
Quoted 4 lines
Lothar Wassmann (3):
pwm: print error messages with pr_err() instead of pr_debug()
pwm: core: make the PWM_POLARITY flag in DTB optional
pwm: imx: support output polarity inversion
For some reason this patchset works differently than the one developed by Lothar.
The difference is with the brightness level control.
My brightness definition in DTS:

Quoted 9 lines

           pwms = <&pwm2 0 5000000 PWM_POLARITY_INVERTED>;

brightness-levels = < 0 1 2 3 4 5 6 7 8 9

					 ..  ............
				    250 251 252 253 254 255>;
		default-brightness-level = <50>;
		enable-gpios = <&gpio1 2 GPIO_ACTIVE_LOW>;

If you are using something else than i.MX 7 you also want to update the SoC level device tree, specifically change the pwm-cells property:
#pwm-cells = <3>;

Quoted 10 lines

When I go to the backlight sysfs entry:
cd /sys/devices/soc0/backlight/backlight/backlight
It seems like the brightness level control is inverted - i.e. 'echo 20 > brightness" makes picture on the screen very bright, and 'echo 200 > brightness' makes the picture diminish.
With my "internal" patches the situation is opposite (and I've checked it with my HW connections).

Just to check whether the driver actually applies the polarity you can add a #define DEBUG at the top of the driver (drivers/pwm/pwm-imx.c) and pass ignore_loglevel as kernel command line. This should give you "PWM supports output inversion" at startup and a "... polarity set to .." message whenever the polarity is set.

--
Stefan

Quoted 16 lines

Could you check on your setup if similar situation takes place? I mean if the brightness control works as expected?
Thanks in advance,
?ukasz Majewski
Quoted 9 lines
 Documentation/devicetree/bindings/pwm/imx-pwm.txt |  6 +--
 arch/arm/boot/dts/imx7-colibri.dtsi               | 12 +++++-
 arch/arm/boot/dts/imx7s.dtsi                      |  8 ++--
 drivers/pwm/core.c                                | 31 ++++++++------
 drivers/pwm/pwm-imx.c                             | 51
+++++++++++++++++++++-- 5 files changed, 83 insertions(+), 25 deletions(-)

Lukasz Majewski

2016-10-06 06:26 UTC #16 lore raw

Hi Stefan,

Quoted 49 lines

Hi Lukasz,
Quoted 37 lines · Lukasz Majewski
Dear Bhuvanchandra,
Thank you for your effort to send those patches to ML.
Quoted 19 lines
Changes since v2:
- Picked the stalled patchset[1] from Lothar Wassmann which adds the basic support for polarity control on imx-pwm driver and adds backward compatibility support for devices which does not have polarity control feature.
Changes since Lothars v6:
- Squash Lukasz patch[2].
[1] http://thread.gmane.org/gmane.linux.pwm/1621
[2] https://www.spinics.net/lists/arm-kernel/msg530818.html
Bhuvanchandra DV (3):
arm: dts: imx7: Update #pwm-cells for PWM polarity control
arm: dts: imx7-colibri: Use pwm polarity control
arm: dts: imx7-colibri: Use enable-gpios for BL_ON
Quoted 4 lines
Lothar Wassmann (3):
pwm: print error messages with pr_err() instead of pr_debug()
pwm: core: make the PWM_POLARITY flag in DTB optional
pwm: imx: support output polarity inversion
For some reason this patchset works differently than the one developed by Lothar.
The difference is with the brightness level control.
My brightness definition in DTS:
           pwms = <&pwm2 0 5000000 PWM_POLARITY_INVERTED>;
Quoted 8 lines
brightness-levels = < 0 1 2 3 4 5 6
7 8 9
					 ..  ............
				    250 251 252 253 254 255>;
		default-brightness-level = <50>;
		enable-gpios = <&gpio1 2 GPIO_ACTIVE_LOW>;

Quoted 3 lines

If you are using something else than i.MX 7 you also want to update the SoC level device tree, specifically change the pwm-cells property:
#pwm-cells = <3>;

Good point. However, it is declared elsewhere (with pwm2 node)

&pwm2 {
	#pwm-cells = <3>;
	pinctrl-names = "default";
	pinctrl-0 = <&pinctrl_pwm2>;
	status = "okay";
};

Quoted 10 lines

Quoted 10 lines
When I go to the backlight sysfs entry:
cd /sys/devices/soc0/backlight/backlight/backlight
It seems like the brightness level control is inverted - i.e. 'echo 20 > brightness" makes picture on the screen very bright, and 'echo 200 > brightness' makes the picture diminish.

Quoted 8 lines

Quoted 2 lines
With my "internal" patches the situation is opposite (and I've checked it with my HW connections).
Just to check whether the driver actually applies the polarity you can add a #define DEBUG at the top of the driver (drivers/pwm/pwm-imx.c) and pass ignore_loglevel as kernel command line. This should give you "PWM supports output inversion" at startup and a "... polarity set to .." message whenever the polarity is set.

The problem is with the Bhuvanchandra original patch.

I will point it out when replying to original patch.

Thanks for support,

?ukasz Majewski

Quoted 20 lines

--
Stefan
Quoted 16 lines
Could you check on your setup if similar situation takes place? I mean if the brightness control works as expected?
Thanks in advance,
?ukasz Majewski
Quoted 9 lines
 Documentation/devicetree/bindings/pwm/imx-pwm.txt |  6 +--
 arch/arm/boot/dts/imx7-colibri.dtsi               | 12 +++++-
 arch/arm/boot/dts/imx7s.dtsi                      |  8 ++--
 drivers/pwm/core.c                                | 31
++++++++------ drivers/pwm/pwm-imx.c |
51 +++++++++++++++++++++-- 5 files changed, 83 insertions(+), 25 deletions(-)