FAILED: patch "[PATCH] pwm: jz4740: Enhance precision in calculation of duty cycle" failed to apply to 5.4-stable tree

PATCH [email protected] 3 participants 4 messages Jun 19, 2020 – Jun 23, 2020

Subsystems (3)

Stable branch PWM Ingenic JZ47xx SoCs

lore

Share & tools

lore

[email protected]

author maintainer

2020-06-19 13:45 UTC #0 lore raw

The patch below does not apply to the 5.4-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 9017dc4fbd59c09463019ce494cfe36d654495a8 Mon Sep 17 00:00:00 2001
From: Paul Cercueil <[email protected]>
Date: Wed, 27 May 2020 13:52:23 +0200
Subject: [PATCH] pwm: jz4740: Enhance precision in calculation of duty cycle
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Calculating the hardware value for the duty from the hardware value of the period resulted in a precision loss versus calculating it from the clock rate directly.

(Also remove a cast that doesn't really need to be here)

Fixes f6b8a5700057 ("pwm: Add Ingenic JZ4740 support")

Cc <[email protected]>

Reviewed-by Uwe Kleine-König <[email protected]>

Signed-off-by Paul Cercueil <[email protected]>

Signed-off-by Thierry Reding <[email protected]>

drivers/pwm/pwm-jz4740.c +3 -3

index 3cd5c054ad9a..4fe9d99ac9a9 100644
@@ -158,11 +158,11 @@ static int jz4740_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	/* Calculate period value */
 	tmp = (unsigned long long)rate * state->period;
 	do_div(tmp, NSEC_PER_SEC);
-	period = (unsigned long)tmp;
+	period = tmp;
 
 	/* Calculate duty value */
-	tmp = (unsigned long long)period * state->duty_cycle;
-	do_div(tmp, state->period);
+	tmp = (unsigned long long)rate * state->duty_cycle;
+	do_div(tmp, NSEC_PER_SEC);
 	duty = period - tmp;
 
 	if (duty >= period)

Sasha Levin

maintainer

2020-06-23 00:40 UTC #1 lore raw

Quoted 24 lines · On Fri, Jun 19, 2020 at 03:45:07PM +0200, [email protected] wrote:

The patch below does not apply to the 5.4-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 9017dc4fbd59c09463019ce494cfe36d654495a8 Mon Sep 17 00:00:00 2001
From: Paul Cercueil <[email protected]>
Date: Wed, 27 May 2020 13:52:23 +0200
Subject: [PATCH] pwm: jz4740: Enhance precision in calculation of duty cycle
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Calculating the hardware value for the duty from the hardware value of the period resulted in a precision loss versus calculating it from the clock rate directly.

Quoted 8 lines

(Also remove a cast that doesn't really need to be here)
Fixes f6b8a5700057 ("pwm: Add Ingenic JZ4740 support")
Cc <[email protected]>
Suggested-by Uwe Kleine-König <[email protected]>
Reviewed-by Uwe Kleine-König <[email protected]>
Signed-off-by Paul Cercueil <[email protected]>
Signed-off-by Thierry Reding <[email protected]>

I suspect that the fixes tag should have been pointing to ce1f9cece057 ("pwm: jz4740: Use clocks from TCU driver") instead.

Signature · 2 lines

Thanks,
Sasha

Uwe Kleine-König

Reviewed-by

2020-06-23 07:25 UTC #2 lore raw

Quoted 6 lines · Sasha Levin

Quoted 5 lines · On Fri, Jun 19, 2020 at 03:45:07PM +0200, [email protected] wrote:
The patch below does not apply to the 5.4-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

See below for a backport.

Quoted 27 lines

Quoted 18 lines
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 9017dc4fbd59c09463019ce494cfe36d654495a8 Mon Sep 17 00:00:00 2001
From: Paul Cercueil <[email protected]>
Date: Wed, 27 May 2020 13:52:23 +0200
Subject: [PATCH] pwm: jz4740: Enhance precision in calculation of duty cycle
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Calculating the hardware value for the duty from the hardware value of the period resulted in a precision loss versus calculating it from the clock rate directly.
Quoted 8 lines
(Also remove a cast that doesn't really need to be here)
Fixes f6b8a5700057 ("pwm: Add Ingenic JZ4740 support")
Cc <[email protected]>
Suggested-by Uwe Kleine-König <[email protected]>
Reviewed-by Uwe Kleine-König <[email protected]>
Signed-off-by Paul Cercueil <[email protected]>
Signed-off-by Thierry Reding <[email protected]>

Quoted 2 lines

I suspect that the fixes tag should have been pointing to ce1f9cece057 ("pwm: jz4740: Use clocks from TCU driver") instead.

No, f6b8a5700057 is right. The cast that was dropped isn't there, but the suboptimal calculation is.

The backport on top of 5.4.y looks as follows:

From b39d3d4c6ba4b7ba8b97a0f7e650924920e4d95c Mon Sep 17 00:00:00 2001
From: Paul Cercueil <[email protected]>
Date: Wed, 27 May 2020 13:52:23 +0200
Subject: [PATCH] pwm: jz4740: Enhance precision in calculation of duty cycle
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

commit 9017dc4fbd59c09463019ce494cfe36d654495a8 upstream.

Calculating the hardware value for the duty from the hardware value of the period resulted in a precision loss versus calculating it from the clock rate directly.

Fixes f6b8a5700057 ("pwm: Add Ingenic JZ4740 support")

Cc <[email protected]>

Reviewed-by Uwe Kleine-König <[email protected]>

Signed-off-by Paul Cercueil <[email protected]>

Signed-off-by Thierry Reding <[email protected]>

[ukl: backport to v5.4.y and adapt commit log accordingly]

Signed-off-by Uwe Kleine-König <[email protected]>

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

drivers/pwm/pwm-jz4740.c +2 -2

index 9d78cc21cb12..d0f5c69930d0 100644
@@ -108,8 +108,8 @@ static int jz4740_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	if (prescaler == 6)
 		return -EINVAL;
 
-	tmp = (unsigned long long)period * state->duty_cycle;
-	do_div(tmp, state->period);
+	tmp = (unsigned long long)rate * state->duty_cycle;
+	do_div(tmp, NSEC_PER_SEC);
 	duty = period - tmp;
 
 	if (duty >= period)

Signature · 2 lines

Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Greg KH

author maintainer

2020-06-23 12:21 UTC #3 lore raw

Quoted 77 lines · Uwe Kleine-König

Quoted 6 lines · Sasha Levin
Quoted 5 lines · On Fri, Jun 19, 2020 at 03:45:07PM +0200, [email protected] wrote:
The patch below does not apply to the 5.4-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
See below for a backport.
Quoted 27 lines
Quoted 18 lines
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 9017dc4fbd59c09463019ce494cfe36d654495a8 Mon Sep 17 00:00:00 2001
From: Paul Cercueil <[email protected]>
Date: Wed, 27 May 2020 13:52:23 +0200
Subject: [PATCH] pwm: jz4740: Enhance precision in calculation of duty cycle
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Calculating the hardware value for the duty from the hardware value of the period resulted in a precision loss versus calculating it from the clock rate directly.
Quoted 8 lines
(Also remove a cast that doesn't really need to be here)
Fixes f6b8a5700057 ("pwm: Add Ingenic JZ4740 support")
Cc <[email protected]>
Suggested-by Uwe Kleine-König <[email protected]>
Reviewed-by Uwe Kleine-König <[email protected]>
Signed-off-by Paul Cercueil <[email protected]>
Signed-off-by Thierry Reding <[email protected]>
Quoted 2 lines
I suspect that the fixes tag should have been pointing to ce1f9cece057 ("pwm: jz4740: Use clocks from TCU driver") instead.
No, f6b8a5700057 is right. The cast that was dropped isn't there, but the suboptimal calculation is.
The backport on top of 5.4.y looks as follows:
From b39d3d4c6ba4b7ba8b97a0f7e650924920e4d95c Mon Sep 17 00:00:00 2001
From: Paul Cercueil <[email protected]>
Date: Wed, 27 May 2020 13:52:23 +0200
Subject: [PATCH] pwm: jz4740: Enhance precision in calculation of duty cycle
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
commit 9017dc4fbd59c09463019ce494cfe36d654495a8 upstream.
Calculating the hardware value for the duty from the hardware value of the period resulted in a precision loss versus calculating it from the clock rate directly.
Fixes f6b8a5700057 ("pwm: Add Ingenic JZ4740 support")
Cc <[email protected]>
Suggested-by Uwe Kleine-König <[email protected]>
Reviewed-by Uwe Kleine-König <[email protected]>
Signed-off-by Paul Cercueil <[email protected]>
Signed-off-by Thierry Reding <[email protected]>
[ukl: backport to v5.4.y and adapt commit log accordingly]
Signed-off-by Uwe Kleine-König <[email protected]>
---
 drivers/pwm/pwm-jz4740.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/pwm/pwm-jz4740.c +0 -0
index 9d78cc21cb12..d0f5c69930d0 100644

drivers/pwm/pwm-jz4740.c static int jz4740_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, · line 115

Quoted 10 lines

drivers/pwm/pwm-jz4740.c +2 -2

@@ -108,8 +108,8 @@ static int jz4740_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	if (prescaler == 6)
 		return -EINVAL;
 
-	tmp = (unsigned long long)period * state->duty_cycle;
-	do_div(tmp, state->period);
+	tmp = (unsigned long long)rate * state->duty_cycle;
+	do_div(tmp, NSEC_PER_SEC);
 	duty = period - tmp;
 
 	if (duty >= period)

Thanks, now queued up.

greg k-h