[PATCH v2 2/6] dt: lpc32xx: pwm: update documentation of LPC32xx PWM device

[PATCH v2 2/6] dt: lpc32xx: pwm: update documentation of LPC32xx PWM device

[Vladimir Zapolskiy]

NXP LPC32xx SoC has two separate PWM controller devices, update device
tree binding documentation to reflect this fact.

The change makes previous PWM device node description incompatible
with the new version.

Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
---
Changes from v1 to v2:
- removed '0x' from a device node address and use lower case hex chars.

 Documentation/devicetree/bindings/pwm/lpc32xx-pwm.txt | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pwm/lpc32xx-pwm.txt b/Documentation/devicetree/bindings/pwm/lpc32xx-pwm.txt
index 1ab1abc..74b5bc5 100644
--- a/Documentation/devicetree/bindings/pwm/lpc32xx-pwm.txt
+++ b/Documentation/devicetree/bindings/pwm/lpc32xx-pwm.txt
@@ -8,5 +8,10 @@ Examples:
 
 pwm@4005c000 {
 	compatible = "nxp,lpc3220-pwm";
-	reg = <0x4005c000 0x8>;
+	reg = <0x4005c000 0x4>;
+};
+
+pwm@4005c004 {
+	compatible = "nxp,lpc3220-pwm";
+	reg = <0x4005c004 0x4>;
 };
-- 
2.1.4

[PATCH v2 0/6] pwm: lpc32xx: fixes in the LPC32xx PWM driver

[Vladimir Zapolskiy]

The changeset fixes a number of issues within current implementation
of LPC32xx PWM controller driver, namely
- the SoC has two independent PWM controllers with one channel each,
- runtime warnings from common clock framework,
- overflow in duty cycle calculation may result in disabled PWM,
- unsupported period values are not accepted.

Correction of PWM channels requires a correspondent change in
arch/arm/boot/dts/lpc32xx.dtsi, it has been already sent for review.

Changes from v1 to v2:
- corrected style of examples in documentation,
- improved commit messages

Version v1 can be found here:
  http://comments.gmane.org/gmane.linux.pwm/2828
  http://www.spinics.net/lists/devicetree/msg105398.html

Vladimir Zapolskiy (6):
  dt: lpc32xx: pwm: correct LPC32xx PWM device node example
  dt: lpc32xx: pwm: update documentation of LPC32xx PWM device
  pwm: lpc32xx: correct number of PWM channels from 2 to 1
  pwm: lpc32xx: fix warnings from common clock framework
  pwm: lpc32xx: fix and simplify duty cycle and period calculations
  pwm: lpc32xx: return ERANGE, if requested period is not supported

 .../devicetree/bindings/pwm/lpc32xx-pwm.txt        |  9 +++-
 drivers/pwm/pwm-lpc32xx.c                          | 59 ++++++++--------------
 2 files changed, 29 insertions(+), 39 deletions(-)

-- 
2.1.4

[PATCH v2 1/6] dt: lpc32xx: pwm: correct LPC32xx PWM device node example

[Vladimir Zapolskiy]

The change removes '0x' from a device node address and uses lower case
hex chars.

Signed-off-by: Vladimir Zapolskiy <vz-ChpfBGZJDbMAvxtiuMwx3w@public.gmane.org>
---
Changes from v1 to v2:
- new change

 Documentation/devicetree/bindings/pwm/lpc32xx-pwm.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/pwm/lpc32xx-pwm.txt b/Documentation/devicetree/bindings/pwm/lpc32xx-pwm.txt
index cfe1db3..1ab1abc 100644
--- a/Documentation/devicetree/bindings/pwm/lpc32xx-pwm.txt
+++ b/Documentation/devicetree/bindings/pwm/lpc32xx-pwm.txt
@@ -6,7 +6,7 @@ Required properties:
 
 Examples:
 
-pwm@0x4005C000 {
+pwm@4005c000 {
 	compatible = "nxp,lpc3220-pwm";
-	reg = <0x4005C000 0x8>;
+	reg = <0x4005c000 0x8>;
 };
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/6] dt: lpc32xx: pwm: update documentation of LPC32xx PWM device

[Vladimir Zapolskiy]

NXP LPC32xx SoC has two separate PWM controller devices, update device
tree binding documentation to reflect this fact.

The change makes previous PWM device node description incompatible
with the new version.

Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
---
Changes from v1 to v2:
- removed '0x' from a device node address and use lower case hex chars.

 Documentation/devicetree/bindings/pwm/lpc32xx-pwm.txt | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pwm/lpc32xx-pwm.txt b/Documentation/devicetree/bindings/pwm/lpc32xx-pwm.txt
index 1ab1abc..74b5bc5 100644
--- a/Documentation/devicetree/bindings/pwm/lpc32xx-pwm.txt
+++ b/Documentation/devicetree/bindings/pwm/lpc32xx-pwm.txt
@@ -8,5 +8,10 @@ Examples:
 
 pwm@4005c000 {
 	compatible = "nxp,lpc3220-pwm";
-	reg = <0x4005c000 0x8>;
+	reg = <0x4005c000 0x4>;
+};
+
+pwm@4005c004 {
+	compatible = "nxp,lpc3220-pwm";
+	reg = <0x4005c004 0x4>;
 };
-- 
2.1.4

[PATCH v2 3/6] pwm: lpc32xx: correct number of PWM channels from 2 to 1

[Vladimir Zapolskiy]

LPC32xx SoC has two independent PWM controllers, they have different
clock parents, clock gates and even slightly different controls, and
each of these two PWM controllers has one output channel. Due to
almost similar controls arranged in a row it is incorrectly set that
there is one PWM controller with two channels, fix this problem, which
at the moment prevents separate configuration of different clock
parents and gates for both PWM controllers.

The change makes previous PWM device node description incompatible
with this update.

Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
---
Changes from v1 to v2:
- improved commit message

 drivers/pwm/pwm-lpc32xx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pwm/pwm-lpc32xx.c b/drivers/pwm/pwm-lpc32xx.c
index 9fde60c..ce8ab20 100644
--- a/drivers/pwm/pwm-lpc32xx.c
+++ b/drivers/pwm/pwm-lpc32xx.c
@@ -134,7 +134,7 @@ static int lpc32xx_pwm_probe(struct platform_device *pdev)
 
 	lpc32xx->chip.dev = &pdev->dev;
 	lpc32xx->chip.ops = &lpc32xx_pwm_ops;
-	lpc32xx->chip.npwm = 2;
+	lpc32xx->chip.npwm = 1;
 	lpc32xx->chip.base = -1;
 
 	ret = pwmchip_add(&lpc32xx->chip);
-- 
2.1.4

[PATCH v2 4/6] pwm: lpc32xx: make device usable with common clock framework

[Vladimir Zapolskiy]

As a preparatory change for switching LPC32xx mach support to common
clock framework fix clk_enable/clk_disable calls without matching
clk_prepare/clk_unprepare.

The driver can not be used on a platform with common clock framework
until clk_prepare/clk_unprepare calls are added, otherwise clk_enable
calls will fail and a WARN is generated:

    # echo 1 > /sys/bus/platform/drivers/lpc32xx-pwm/4005c000.pwm/pwm/pwmchip0/pwm0/enable
    ------------[ cut here ]------------
    WARNING: CPU: 0 PID: 701 at drivers/clk/clk.c:727 clk_core_enable+0x2c/0xa4()
    Modules linked in: sc16is7xx
    CPU: 0 PID: 701 Comm: sh Tainted: G        W       4.3.0-rc2+ #171
    Hardware name: LPC32XX SoC (Flattened Device Tree)
    Backtrace:
    [<>] (dump_backtrace) from [<>] (show_stack+0x18/0x1c)
    [<>] (show_stack) from [<>] (dump_stack+0x20/0x28)
    [<>] (dump_stack) from [<>] (warn_slowpath_common+0x90/0xb8)
    [<>] (warn_slowpath_common) from [<>] (warn_slowpath_null+0x24/0x2c)
    [<>] (warn_slowpath_null) from [<>] (clk_core_enable+0x2c/0xa4)
    [<>] (clk_core_enable) from [<>] (clk_enable+0x24/0x38)
    [<>] (clk_enable) from [<>] (lpc32xx_pwm_enable+0x1c/0x40)
    [<>] (lpc32xx_pwm_enable) from [<>] (pwm_enable+0x48/0x5c)
    [<>] (pwm_enable) from [<>] (pwm_enable_store+0x5c/0x78)
    [<>] (pwm_enable_store) from [<>] (dev_attr_store+0x20/0x2c)
    [<>] (dev_attr_store) from [<>] (sysfs_kf_write+0x44/0x50)
    [<>] (sysfs_kf_write) from [<>] (kernfs_fop_write+0x134/0x194)
    [<>] (kernfs_fop_write) from [<>] (__vfs_write+0x34/0xdc)
    [<>] (__vfs_write) from [<>] (vfs_write+0xb8/0x140)
    [<>] (vfs_write) from [<>] (SyS_write+0x50/0x90)
    [<>] (SyS_write) from [<>] (ret_fast_syscall+0x0/0x38)

Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
---
Changes from v1 to v2:
- improved commit message

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

diff --git a/drivers/pwm/pwm-lpc32xx.c b/drivers/pwm/pwm-lpc32xx.c
index ce8ab20..63468a8 100644
--- a/drivers/pwm/pwm-lpc32xx.c
+++ b/drivers/pwm/pwm-lpc32xx.c
@@ -83,7 +83,7 @@ static int lpc32xx_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 	u32 val;
 	int ret;
 
-	ret = clk_enable(lpc32xx->clk);
+	ret = clk_prepare_enable(lpc32xx->clk);
 	if (ret)
 		return ret;
 
@@ -103,7 +103,7 @@ static void lpc32xx_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
 	val &= ~PWM_ENABLE;
 	writel(val, lpc32xx->base + (pwm->hwpwm << 2));
 
-	clk_disable(lpc32xx->clk);
+	clk_disable_unprepare(lpc32xx->clk);
 }
 
 static const struct pwm_ops lpc32xx_pwm_ops = {
-- 
2.1.4

[PATCH v2 5/6] pwm: lpc32xx: fix and simplify duty cycle and period calculations

[Vladimir Zapolskiy]

The change fixes a problem, if duty_ns is too small in comparison
to period_ns (as a valid corner case duty_ns is 0 ns), then due to
PWM_DUTY() macro applied on a value the result is overflowed over 8
bits, and instead of the highest bitfield duty cycle value 0xff the
invalid duty cycle bitfield value 0x00 is written.

For reference the LPC32xx spec defines PWMx_DUTY bitfield description
is this way and it seems to be correct:

 [Low]/[High] = [PWM_DUTY]/[256-PWM_DUTY], where 0 < PWM_DUTY <= 255.

In addition according to my oscilloscope measurements LPC32xx PWM is
"tristate" in sense that it produces a wave with floating min/max
voltage levels for different duty cycle values, for corner cases:

  PWM_DUTY == 0x01 => signal is in range from -1.05v to 0v
  ....
  PWM_DUTY == 0x80 => signal is in range from -0.75v to +0.75v
  ....
  PWM_DUTY == 0xff => signal is in range from 0v to +1.05v

  PWM_DUTY == 0x00 => signal is around 0v, PWM is off

Due to this peculiarity on very long period ranges (less than 1KHz)
and odd pre-divider values PWM generated wave does not remind a
clock shape signal, but rather a heartbit shape signal with positive
and negative peaks, so I would recommend to use high-speed HCLK clock
as a PWM parent clock and avoid using RTC clock as a parent.

The change corrects PWM output in corner cases and prevents any
possible overflows in calculation of values for PWM_DUTY and
PWM_RELOADV bitfields, thus helper macro definitions may be removed.

Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
---
Changes from v1 to v2:
- none

 drivers/pwm/pwm-lpc32xx.c | 53 +++++++++++++++++------------------------------
 1 file changed, 19 insertions(+), 34 deletions(-)

diff --git a/drivers/pwm/pwm-lpc32xx.c b/drivers/pwm/pwm-lpc32xx.c
index 63468a8..294a68f 100644
--- a/drivers/pwm/pwm-lpc32xx.c
+++ b/drivers/pwm/pwm-lpc32xx.c
@@ -24,9 +24,7 @@ struct lpc32xx_pwm_chip {
 	void __iomem *base;
 };
 
-#define PWM_ENABLE	(1 << 31)
-#define PWM_RELOADV(x)	(((x) & 0xFF) << 8)
-#define PWM_DUTY(x)	((x) & 0xFF)
+#define PWM_ENABLE	BIT(31)
 
 #define to_lpc32xx_pwm_chip(_chip) \
 	container_of(_chip, struct lpc32xx_pwm_chip, chip)
@@ -38,40 +36,27 @@ static int lpc32xx_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	unsigned long long c;
 	int period_cycles, duty_cycles;
 	u32 val;
-
-	c = clk_get_rate(lpc32xx->clk) / 256;
-	c = c * period_ns;
-	do_div(c, NSEC_PER_SEC);
-
-	/* Handle high and low extremes */
-	if (c == 0)
-		c = 1;
-	if (c > 255)
-		c = 0; /* 0 set division by 256 */
-	period_cycles = c;
-
-	/* The duty-cycle value is as follows:
-	 *
-	 *  DUTY-CYCLE     HIGH LEVEL
-	 *      1            99.9%
-	 *      25           90.0%
-	 *      128          50.0%
-	 *      220          10.0%
-	 *      255           0.1%
-	 *      0             0.0%
-	 *
-	 * In other words, the register value is duty-cycle % 256 with
-	 * duty-cycle in the range 1-256.
-	 */
-	c = 256 * duty_ns;
-	do_div(c, period_ns);
-	if (c > 255)
-		c = 255;
-	duty_cycles = 256 - c;
+	c = clk_get_rate(lpc32xx->clk);
+
+	/* The highest acceptable divisor is 256, which is represented by 0 */
+	period_cycles = div64_u64(c * period_ns,
+			       (unsigned long long)NSEC_PER_SEC * 256);
+	if (!period_cycles)
+		period_cycles = 1;
+	if (period_cycles > 255)
+		period_cycles = 0;
+
+	/* Compute 256 x #duty/period value and care for corner cases */
+	duty_cycles = div64_u64((unsigned long long)(period_ns - duty_ns) * 256,
+				period_ns);
+	if (!duty_cycles)
+		duty_cycles = 1;
+	if (duty_cycles > 255)
+		duty_cycles = 255;
 
 	val = readl(lpc32xx->base + (pwm->hwpwm << 2));
 	val &= ~0xFFFF;
-	val |= PWM_RELOADV(period_cycles) | PWM_DUTY(duty_cycles);
+	val |= (period_cycles << 8) | duty_cycles;
 	writel(val, lpc32xx->base + (pwm->hwpwm << 2));
 
 	return 0;
-- 
2.1.4

[PATCH v2 6/6] pwm: lpc32xx: return ERANGE, if requested period is not supported

[Vladimir Zapolskiy]

Instead of silent acceptance of unsupported requested configuration
for PWM period and setting the boundary supported value, return
-ERANGE to a caller.

Duty period value equal to 0 or period is still accepted to allow
configuration by PWM sysfs interface, when it is set to 0 by default.

For reference this is a list of restrictions on period_ns == 1/freq:

  | PWM parent clock | parent clock divisor | max freq | min freq |
  +------------------+----------------------+----------+----------+
  |   HCLK == 13 MHz |      1 (min)         | 50.7 KHz | 198.3 Hz |
  |   HCLK == 13 MHz |     15 (max)         | 3.38 KHz | 13.22 Hz |
  |  RTC == 32.7 KHz |      1 (min)         |   128 Hz |   0.5 Hz |
  |  RTC == 32.7 KHz |     15 (max)         | 8.533 Hz | 0.033 Hz |

Note that PWM sysfs interface does not support setting of period more
than NSEC_PER_SEC / MAX_INT32 ~ 2 seconds, however this PWM controller
supports a period up to 30 seconds.

Signed-off-by: Vladimir Zapolskiy <vz-ChpfBGZJDbMAvxtiuMwx3w@public.gmane.org>
---
Changes from v1 to v2:
- none

 drivers/pwm/pwm-lpc32xx.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/pwm/pwm-lpc32xx.c b/drivers/pwm/pwm-lpc32xx.c
index 294a68f..4d470c1 100644
--- a/drivers/pwm/pwm-lpc32xx.c
+++ b/drivers/pwm/pwm-lpc32xx.c
@@ -41,9 +41,9 @@ static int lpc32xx_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	/* The highest acceptable divisor is 256, which is represented by 0 */
 	period_cycles = div64_u64(c * period_ns,
 			       (unsigned long long)NSEC_PER_SEC * 256);
-	if (!period_cycles)
-		period_cycles = 1;
-	if (period_cycles > 255)
+	if (!period_cycles || period_cycles > 256)
+		return -ERANGE;
+	if (period_cycles == 256)
 		period_cycles = 0;
 
 	/* Compute 256 x #duty/period value and care for corner cases */
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/6] dt: lpc32xx: pwm: correct LPC32xx PWM device node example

[Rob Herring]

On Sun, Dec 06, 2015 at 01:31:57PM +0200, Vladimir Zapolskiy wrote:
> The change removes '0x' from a device node address and uses lower case
> hex chars.
> 
> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>

Acked-by: Rob Herring <robh@kernel.org>

> ---
> Changes from v1 to v2:
> - new change
> 
>  Documentation/devicetree/bindings/pwm/lpc32xx-pwm.txt | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pwm/lpc32xx-pwm.txt b/Documentation/devicetree/bindings/pwm/lpc32xx-pwm.txt
> index cfe1db3..1ab1abc 100644
> --- a/Documentation/devicetree/bindings/pwm/lpc32xx-pwm.txt
> +++ b/Documentation/devicetree/bindings/pwm/lpc32xx-pwm.txt
> @@ -6,7 +6,7 @@ Required properties:
>  
>  Examples:
>  
> -pwm@0x4005C000 {
> +pwm@4005c000 {
>  	compatible = "nxp,lpc3220-pwm";
> -	reg = <0x4005C000 0x8>;
> +	reg = <0x4005c000 0x8>;
>  };
> -- 
> 2.1.4
> 

Re: [PATCH v2 2/6] dt: lpc32xx: pwm: update documentation of LPC32xx PWM device

[Rob Herring]

On Sun, Dec 06, 2015 at 01:31:58PM +0200, Vladimir Zapolskiy wrote:
> NXP LPC32xx SoC has two separate PWM controller devices, update device
> tree binding documentation to reflect this fact.
> 
> The change makes previous PWM device node description incompatible
> with the new version.
> 
> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>

Acked-by: Rob Herring <robh@kernel.org>

> ---
> Changes from v1 to v2:
> - removed '0x' from a device node address and use lower case hex chars.
> 
>  Documentation/devicetree/bindings/pwm/lpc32xx-pwm.txt | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/pwm/lpc32xx-pwm.txt b/Documentation/devicetree/bindings/pwm/lpc32xx-pwm.txt
> index 1ab1abc..74b5bc5 100644
> --- a/Documentation/devicetree/bindings/pwm/lpc32xx-pwm.txt
> +++ b/Documentation/devicetree/bindings/pwm/lpc32xx-pwm.txt
> @@ -8,5 +8,10 @@ Examples:
>  
>  pwm@4005c000 {
>  	compatible = "nxp,lpc3220-pwm";
> -	reg = <0x4005c000 0x8>;
> +	reg = <0x4005c000 0x4>;
> +};
> +
> +pwm@4005c004 {
> +	compatible = "nxp,lpc3220-pwm";
> +	reg = <0x4005c004 0x4>;
>  };
> -- 
> 2.1.4
> 

Re: [PATCH v2 0/6] pwm: lpc32xx: fixes in the LPC32xx PWM driver

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1500 bytes --]

On Sun, Dec 06, 2015 at 01:31:56PM +0200, Vladimir Zapolskiy wrote:
> The changeset fixes a number of issues within current implementation
> of LPC32xx PWM controller driver, namely
> - the SoC has two independent PWM controllers with one channel each,
> - runtime warnings from common clock framework,
> - overflow in duty cycle calculation may result in disabled PWM,
> - unsupported period values are not accepted.
> 
> Correction of PWM channels requires a correspondent change in
> arch/arm/boot/dts/lpc32xx.dtsi, it has been already sent for review.
> 
> Changes from v1 to v2:
> - corrected style of examples in documentation,
> - improved commit messages
> 
> Version v1 can be found here:
>   http://comments.gmane.org/gmane.linux.pwm/2828
>   http://www.spinics.net/lists/devicetree/msg105398.html
> 
> Vladimir Zapolskiy (6):
>   dt: lpc32xx: pwm: correct LPC32xx PWM device node example
>   dt: lpc32xx: pwm: update documentation of LPC32xx PWM device
>   pwm: lpc32xx: correct number of PWM channels from 2 to 1
>   pwm: lpc32xx: fix warnings from common clock framework
>   pwm: lpc32xx: fix and simplify duty cycle and period calculations
>   pwm: lpc32xx: return ERANGE, if requested period is not supported
> 
>  .../devicetree/bindings/pwm/lpc32xx-pwm.txt        |  9 +++-
>  drivers/pwm/pwm-lpc32xx.c                          | 59 ++++++++--------------
>  2 files changed, 29 insertions(+), 39 deletions(-)

All patches applied, thanks.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]