Re: [PATCH RFC 2/6] pwm: core: Add option to config PWM duty/period with u64 data length
Hello Martin, On Mon, Jul 27, 2020 at 09:58:01AM +0200, Martin Botka wrote: > > I hit "reply-to-all" and the mail only was sent to you because you wrote > > to only me. > > Yes my reply was only to you. But your original message was sent only to me > too. > So when i clicked reply to all it was only you as you sent it only to me. Oh indeed. Bummer, and I was so sure to always reply to all :-| Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
Re: [PATCH RFC 2/6] pwm: core: Add option to config PWM duty/period with u64 data length
Hello, > I hit "reply-to-all" and the mail only was sent to you because you wrote > to only me. Yes my reply was only to you. But your original message was sent only to me too. So when i clicked reply to all it was only you as you sent it only to me. > Also threading is somehow strange because your reply to my mail Yes Gmail would not allow me to reply to your message and also send it to everyone so i had to reply to Andy's email which is why the threading is broken there. Sorry for that. > So I assume all the strange things happened on your side until proved > otherwise. :-) I think i just proved otherwise :) Best Regards, Martin
Re: [PATCH RFC 2/6] pwm: core: Add option to config PWM duty/period with u64 data length
Hello Martin, On Mon, Jul 27, 2020 at 09:29:19AM +0200, Martin Botka wrote: > On Sat, Jul 25, 2020 at 09:12:23PM +0200, Martin Botka wrote: > >> > Note there is already a series that changes these values to u64. See > >> > a9d887dc1c60ed67f2271d66560cdcf864c4a578 in linux-next. > >> > >> Amazing. But isn't there the same issue with it as this one where this > >> would fail to build on 32 bit architecture? > > > > In theory all these cases are coped for. I didn't see any problems yet, > > so I still assume also the 32 bit archs are fine. > > OK then all is fine. I will drop the patch in V2. > > Also Uwe i just realized that you sent the original message and also > this reply only to me and not to anyone else. > Could you please send the messages also to everyone else ? I hit "reply-to-all" and the mail only was sent to you because you wrote to only me. Also threading is somehow strange because your reply to my mail (with Message-Id: 20200727070411.ovkuwm76vuw3h...@pengutronix.de ) has In-Reply-To: . So I assume all the strange things happened on your side until proved otherwise. :-) Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
Re: [PATCH RFC 2/6] pwm: core: Add option to config PWM duty/period with u64 data length
> Could you please send the messages also to everyone else ? Next time of course.
Re: [PATCH RFC 2/6] pwm: core: Add option to config PWM duty/period with u64 data length
Hello Uwe, On Sat, Jul 25, 2020 at 09:12:23PM +0200, Martin Botka wrote: >> > Note there is already a series that changes these values to u64. See >> > a9d887dc1c60ed67f2271d66560cdcf864c4a578 in linux-next. >> >> Amazing. But isn't there the same issue with it as this one where this >> would fail to build on 32 bit architecture? > > In theory all these cases are coped for. I didn't see any problems yet, > so I still assume also the 32 bit archs are fine. OK then all is fine. I will drop the patch in V2. Also Uwe i just realized that you sent the original message and also this reply only to me and not to anyone else. Could you please send the messages also to everyone else ? Thank you. Best regards, Martin
Re: [PATCH RFC 2/6] pwm: core: Add option to config PWM duty/period with u64 data length
> And all divisions go mad on 32-bit CPU, right? > Please, if you thought about it carefully, update a commit message to > clarify that. Hello, This patch will be dropped in V2 since another series already made these u64. See a9d887dc1c60ed67f2271d66560cdcf864c4a578 in linux-next. I have not tested compiling that commit in linux-next on 32 bit arch but if it fails i can replace this commit with fix for that. Also I'm not the author of this commit. Konrad Dybcio fast forwarded it to 5.8 from 4.14. Fenglin Wu is the author and also created that commit message. Thank you. Best regards Martin
Re: [PATCH RFC 2/6] pwm: core: Add option to config PWM duty/period with u64 data length
On Sat, Jul 25, 2020 at 12:40 AM Martin Botka wrote: > > From: Fenglin Wu > > Currently, PWM core driver provides interfaces for configuring PWM > period and duty length in nanoseconds with an integer data type, so > the max period can be only set to ~2.147 seconds. Add interfaces which > can set PWM period and duty with u64 data type to remove this > limitation. And all divisions go mad on 32-bit CPU, right? Please, if you thought about it carefully, update a commit message to clarify that. -- With Best Regards, Andy Shevchenko
Re: [PATCH RFC 2/6] pwm: core: Add option to config PWM duty/period with u64 data length
> +#include > - gain_q23 = (gain_q23 * dmic->boost_gain) / 100; > + gain_q23 = div_u64(gain_q23 * dmic->boost_gain, 100); Ok so using a macro. Thank you.
Re: [PATCH RFC 2/6] pwm: core: Add option to config PWM duty/period with u64 data length
Hi! > As can be seen this divides llu by llu in few warnings and error. > At the time of sending i didn't realize it but this fails on 32 bit > architectures. > > So i would like to ask how would you like this fixed ? > Using macro or some other way ? +#include - gain_q23 = (gain_q23 * dmic->boost_gain) / 100; + gain_q23 = div_u64(gain_q23 * dmic->boost_gain, 100); Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: PGP signature
Re: [PATCH RFC 2/6] pwm: core: Add option to config PWM duty/period with u64 data length
Hello, As can be seen this divides llu by llu in few warnings and error. At the time of sending i didn't realize it but this fails on 32 bit architectures. So i would like to ask how would you like this fixed ? Using macro or some other way ? Thank you. Best regards, Martin
[PATCH RFC 2/6] pwm: core: Add option to config PWM duty/period with u64 data length
From: Fenglin Wu
Currently, PWM core driver provides interfaces for configuring PWM
period and duty length in nanoseconds with an integer data type, so
the max period can be only set to ~2.147 seconds. Add interfaces which
can set PWM period and duty with u64 data type to remove this
limitation.
Signed-off-by: Fenglin Wu
[konradyb...@gmail.com: Fast-forward from kernel 4.14 to 5.8]
Signed-off-by: Konrad Dybcio
Signed-off-by: Martin Botka
---
drivers/pwm/core.c | 30 +++--
drivers/pwm/sysfs.c | 6 ++--
include/linux/pwm.h | 79 +
3 files changed, 95 insertions(+), 20 deletions(-)
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index f3aa44106962..82411e3ccbbb 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -511,12 +511,12 @@ static void pwm_apply_state_debug(struct pwm_device *pwm,
last->period > s2.period &&
last->period <= state->period)
dev_warn(chip->dev,
-".apply didn't pick the best available period
(requested: %u, applied: %u, possible: %u)\n",
+".apply didn't pick the best available period
(requested: %llu, applied: %llu, possible: %llu)\n",
state->period, s2.period, last->period);
if (state->enabled && state->period < s2.period)
dev_warn(chip->dev,
-".apply is supposed to round down period (requested:
%u, applied: %u)\n",
+".apply is supposed to round down period (requested:
%llu, applied: %llu)\n",
state->period, s2.period);
if (state->enabled &&
@@ -525,14 +525,14 @@ static void pwm_apply_state_debug(struct pwm_device *pwm,
last->duty_cycle > s2.duty_cycle &&
last->duty_cycle <= state->duty_cycle)
dev_warn(chip->dev,
-".apply didn't pick the best available duty cycle
(requested: %u/%u, applied: %u/%u, possible: %u/%u)\n",
+".apply didn't pick the best available duty cycle
(requested: %llu/%llu, applied: %llu/%llu, possible: %llu/%llu)\n",
state->duty_cycle, state->period,
s2.duty_cycle, s2.period,
last->duty_cycle, last->period);
if (state->enabled && state->duty_cycle < s2.duty_cycle)
dev_warn(chip->dev,
-".apply is supposed to round down duty_cycle
(requested: %u/%u, applied: %u/%u)\n",
+".apply is supposed to round down duty_cycle
(requested: %llu/%llu, applied: %llu/%llu)\n",
state->duty_cycle, state->period,
s2.duty_cycle, s2.period);
@@ -559,7 +559,7 @@ static void pwm_apply_state_debug(struct pwm_device *pwm,
(s1.enabled && s1.period != last->period) ||
(s1.enabled && s1.duty_cycle != last->duty_cycle)) {
dev_err(chip->dev,
- ".apply is not idempotent (ena=%d pol=%d %u/%u) ->
(ena=%d pol=%d %u/%u)\n",
+ ".apply is not idempotent (ena=%d pol=%d %llu/%llu) ->
(ena=%d pol=%d %llu/%llu)\n",
s1.enabled, s1.polarity, s1.duty_cycle, s1.period,
last->enabled, last->polarity, last->duty_cycle,
last->period);
@@ -655,9 +655,19 @@ int pwm_apply_state(struct pwm_device *pwm, const struct
pwm_state *state)
if (state->period != pwm->state.period ||
state->duty_cycle != pwm->state.duty_cycle) {
- err = chip->ops->config(pwm->chip, pwm,
- state->duty_cycle,
- state->period);
+ if (pwm->chip->ops->config_extend) {
+ err =
pwm->chip->ops->config_extend(pwm->chip,
+ pwm,
state->duty_cycle,
+ state->period);
+ } else {
+ if (state->period > UINT_MAX)
+ pr_warn("period %llu duty_cycle %llu
will be truncated\n",
+ state->period,
+
state->duty_cycle);
+ err = pwm->chip->ops->config(pwm->chip, pwm,
+
state->duty_cycle,
+ state->period);
+ }
if (err)
return err;
@@ -1310,8 +1320,8 @@ static void pwm_dbg_show(struct pwm_chip *chip, struct
seq_file *s)
if (state.enabled)
