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(-)
--
2.9.2
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(-)
diff --git a/Documentation/devicetree/bindings/pwm/imx-pwm.txt b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
index e00c2e9..c61bdf8 100644
--- a/Documentation/devicetree/bindings/pwm/imx-pwm.txt
+++ b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
@@ -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@53fb4000 {
- #pwm-cells = <2>;
+ #pwm-cells = <3>;
compatible = "fsl,imx53-pwm", "fsl,imx27-pwm";
reg = <0x53fb4000 0x4000>;
clocks = <&clks IMX5_CLK_PWM1_IPG_GATE>,
diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
index d600fd5..c37d223 100644
--- a/drivers/pwm/pwm-imx.c
+++ b/drivers/pwm/pwm-imx.c
@@ -38,6 +38,7 @@
#define MX3_PWMCR_DOZEEN (1 << 24)
#define MX3_PWMCR_WAITEN (1 << 23)
#define MX3_PWMCR_DBGEN (1 << 22)
+#define MX3_PWMCR_POUTC (1 << 18)
#define MX3_PWMCR_CLKSRC_IPG_HIGH (2 << 16)
#define MX3_PWMCR_CLKSRC_IPG (1 << 16)
#define MX3_PWMCR_SWR (1 << 3)
@@ -180,6 +181,9 @@ static int imx_pwm_config_v2(struct pwm_chip *chip,
if (enable)
cr |= MX3_PWMCR_EN;
+ if (pwm->args.polarity == PWM_POLARITY_INVERSED)
+ cr |= MX3_PWMCR_POUTC;
+
writel(cr, imx->mmio_base + MX3_PWMCR);
return 0;
@@ -240,27 +244,62 @@ static void imx_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
clk_disable_unprepare(imx->clk_per);
}
-static struct pwm_ops imx_pwm_ops = {
+static int imx_pwm_set_polarity(struct pwm_chip *chip, struct pwm_device *pwm,
+ enum pwm_polarity polarity)
+{
+ struct imx_chip *imx = to_imx_chip(chip);
+ u32 val;
+
+ if (polarity == pwm->args.polarity)
+ return 0;
+
+ val = readl(imx->mmio_base + MX3_PWMCR);
+
+ if (polarity == PWM_POLARITY_INVERSED)
+ val |= MX3_PWMCR_POUTC;
+ else
+ val &= ~MX3_PWMCR_POUTC;
+
+ writel(val, imx->mmio_base + MX3_PWMCR);
+
+ dev_dbg(imx->chip.dev, "%s: polarity set to %s\n", __func__,
+ polarity == PWM_POLARITY_INVERSED ? "inverted" : "normal");
+
+ return 0;
+}
+
+static struct pwm_ops imx_pwm_ops_v1 = {
.enable = imx_pwm_enable,
.disable = imx_pwm_disable,
.config = imx_pwm_config,
.owner = THIS_MODULE,
};
+static struct pwm_ops imx_pwm_ops_v2 = {
+ .enable = imx_pwm_enable,
+ .disable = imx_pwm_disable,
+ .set_polarity = imx_pwm_set_polarity,
+ .config = imx_pwm_config,
+ .owner = THIS_MODULE,
+};
+
struct imx_pwm_data {
int (*config)(struct pwm_chip *chip,
struct pwm_device *pwm, int duty_ns, int period_ns);
void (*set_enable)(struct pwm_chip *chip, bool enable);
+ struct pwm_ops *pwm_ops;
};
static struct imx_pwm_data imx_pwm_data_v1 = {
.config = imx_pwm_config_v1,
.set_enable = imx_pwm_set_enable_v1,
+ .pwm_ops = &imx_pwm_ops_v1,
};
static struct imx_pwm_data imx_pwm_data_v2 = {
.config = imx_pwm_config_v2,
.set_enable = imx_pwm_set_enable_v2,
+ .pwm_ops = &imx_pwm_ops_v2,
};
static const struct of_device_id imx_pwm_dt_ids[] = {
@@ -282,6 +321,8 @@ static int imx_pwm_probe(struct platform_device *pdev)
if (!of_id)
return -ENODEV;
+ data = of_id->data;
+
imx = devm_kzalloc(&pdev->dev, sizeof(*imx), GFP_KERNEL);
if (imx == NULL)
return -ENOMEM;
@@ -300,18 +341,22 @@ static int imx_pwm_probe(struct platform_device *pdev)
return PTR_ERR(imx->clk_ipg);
}
- imx->chip.ops = &imx_pwm_ops;
+ imx->chip.ops = data->pwm_ops;
imx->chip.dev = &pdev->dev;
imx->chip.base = -1;
imx->chip.npwm = 1;
imx->chip.can_sleep = true;
+ if (data->pwm_ops->set_polarity) {
+ dev_dbg(&pdev->dev, "PWM supports output inversion\n");
+ imx->chip.of_xlate = of_pwm_xlate_with_flags;
+ imx->chip.of_pwm_n_cells = 3;
+ }
r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
imx->mmio_base = devm_ioremap_resource(&pdev->dev, r);
if (IS_ERR(imx->mmio_base))
return PTR_ERR(imx->mmio_base);
- data = of_id->data;
imx->config = data->config;
imx->set_enable = data->set_enable;
--
2.9.2
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(-)
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 195373e..aae8db3 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -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;
--
2.9.2
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(-)
diff --git a/arch/arm/boot/dts/imx7-colibri.dtsi b/arch/arm/boot/dts/imx7-colibri.dtsi
index a9cc657..2af5e3e 100644
--- a/arch/arm/boot/dts/imx7-colibri.dtsi
+++ b/arch/arm/boot/dts/imx7-colibri.dtsi
@@ -43,7 +43,7 @@
/ {
bl: backlight {
compatible = "pwm-backlight";
- pwms = <&pwm1 0 5000000>;
+ pwms = <&pwm1 0 5000000 0>;
};
reg_module_3v3: regulator-module-3v3 {
--
2.9.2
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(-)
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 0dbd29e..195373e 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -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;
}
--
2.9.2
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(-)
diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index 0d7d5ac..8d1d471 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -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";
};
--
2.9.2
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(-)
diff --git a/arch/arm/boot/dts/imx7-colibri.dtsi b/arch/arm/boot/dts/imx7-colibri.dtsi
index 2af5e3e..ce5edb5 100644
--- a/arch/arm/boot/dts/imx7-colibri.dtsi
+++ b/arch/arm/boot/dts/imx7-colibri.dtsi
@@ -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
--
2.9.2
Dear Bhuvanchandra,
Thank you for your effort to send those patches to ML.
> 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
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
>
> 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(-)
>
Hi Lukasz,
On 2016-10-04 00:48, Lukasz Majewski wrote:
> Dear Bhuvanchandra,
>
> Thank you for your effort to send those patches to ML.
>
>> 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
>
> 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>;
>
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>;
> 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
>
> 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
>
>>
>> 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(-)
>>
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(-)
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 1f62668..6cd6004 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -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;
}
--
2.1.4
Hi Stefan,
> Hi Lukasz,
>
> On 2016-10-04 00:48, Lukasz Majewski wrote:
> > Dear Bhuvanchandra,
> >
> > Thank you for your effort to send those patches to ML.
> >
> >> 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
> >
> > 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>;
> >
>
> 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";
};
>
>
> > 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.
The problem is with the Bhuvanchandra original patch.
I will point it out when replying to original patch.
Thanks for support,
Łukasz Majewski
>
> --
> Stefan
>
> >
> > 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
> >
> >>
> >> 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(-)
> >>
Hi Bhuvanchandra,
> 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(-)
>
> diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
> index 195373e..aae8db3 100644
> --- a/drivers/pwm/core.c
> +++ b/drivers/pwm/core.c
> @@ -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);
^^^^^^^^^^^^^^^^
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
> + 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;
Hi Lukasz,
On 10/06/16 12:06, Lukasz Majewski wrote:
> Hi Bhuvanchandra,
>
>> 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(-)
>>
>> diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
>> index 195373e..aae8db3 100644
>> --- a/drivers/pwm/core.c
>> +++ b/drivers/pwm/core.c
>> @@ -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);
> ^^^^^^^^^^^^^^^^
> 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.
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
>
> 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
>
>> + 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;