Re: [PATCH 2/2] pwm: core: Add option to config PWM duty/period with u64 data length
Hi Guru, Thank you for the patch! Yet something to improve: [auto build test ERROR on linus/master] [cannot apply to v5.3 next-20190915] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Guru-Das-Srinagesh/pwm-Add-different-PWM-output-types-support/20190916-151008 config: arm-multi_v4t_defconfig (attached as .config) compiler: arm-linux-gnueabi-gcc (GCC) 7.4.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=7.4.0 make.cross ARCH=arm If you fix the issue, kindly add following tag Reported-by: kbuild test robot All errors (new ones prefixed by >>): drivers/pwm/pwm-clps711x.o: In function `clps711x_pwm_config': >> pwm-clps711x.c:(.text+0x64): undefined reference to `__aeabi_uldivmod' drivers/pwm/pwm-clps711x.o: In function `clps711x_pwm_enable': pwm-clps711x.c:(.text+0x168): undefined reference to `__aeabi_uldivmod' drivers/video/backlight/pwm_bl.o: In function `pwm_backlight_probe': >> pwm_bl.c:(.text+0x5fc): undefined reference to `__aeabi_uldivmod' --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [PATCH 2/2] pwm: core: Add option to config PWM duty/period with u64 data length
On Fri, Sep 13, 2019 at 03:57:44PM -0700, Guru Das Srinagesh 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. > > Change-Id: Ic8722088510d447fc939ab6a5014711aef1b832f > Signed-off-by: Fenglin Wu > Signed-off-by: Guru Das Srinagesh > --- > drivers/pwm/core.c | 20 ++--- > drivers/pwm/sysfs.c | 6 +-- > include/linux/pwm.h | 115 > > 3 files changed, 126 insertions(+), 15 deletions(-) pwm_ops->config() is considered legacy API and only remains for drivers that haven't been converted to the atomic API yet. If you want to extend the range for period and duty cycle, please add that to the atomic API, which is pwm_ops->apply() and struct pwm_state. Thierry > > diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c > index 960a451..02ad16b 100644 > --- a/drivers/pwm/core.c > +++ b/drivers/pwm/core.c > @@ -526,9 +526,19 @@ int pwm_apply_state(struct pwm_device *pwm, struct > pwm_state *state) > > if (state->period != pwm->state.period || > state->duty_cycle != pwm->state.duty_cycle) { > - err = pwm->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; > > @@ -1181,8 +1191,8 @@ static void pwm_dbg_show(struct pwm_chip *chip, struct > seq_file *s) > if (state.enabled) > seq_puts(s, " enabled"); > > - seq_printf(s, " period: %u ns", state.period); > - seq_printf(s, " duty: %u ns", state.duty_cycle); > + seq_printf(s, " period: %llu ns", state.period); > + seq_printf(s, " duty: %llu ns", state.duty_cycle); > seq_printf(s, " polarity: %s", > state.polarity ? "inverse" : "normal"); > > diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c > index ab703f2..ef78c40 100644 > --- a/drivers/pwm/sysfs.c > +++ b/drivers/pwm/sysfs.c > @@ -42,7 +42,7 @@ static ssize_t period_show(struct device *child, > > pwm_get_state(pwm, &state); > > - return sprintf(buf, "%u\n", state.period); > + return sprintf(buf, "%llu\n", state.period); > } > > static ssize_t period_store(struct device *child, > @@ -77,7 +77,7 @@ static ssize_t duty_cycle_show(struct device *child, > > pwm_get_state(pwm, &state); > > - return sprintf(buf, "%u\n", state.duty_cycle); > + return sprintf(buf, "%llu\n", state.duty_cycle); > } > > static ssize_t duty_cycle_store(struct device *child, > @@ -212,7 +212,7 @@ static ssize_t capture_show(struct device *child, > if (ret) > return ret; > > - return sprintf(buf, "%u %u\n", result.period, result.duty_cycle); > + return sprintf(buf, "%llu %llu\n", result.period, result.duty_cycle); > } > > static ssize_t output_type_show(struct device *child, > diff --git a/include/linux/pwm.h b/include/linux/pwm.h > index 416f08e..d714385 100644 > --- a/include/linux/pwm.h > +++ b/include/linux/pwm.h > @@ -39,7 +39,7 @@ enum pwm_polarity { > * current PWM hardware state. > */ > struct pwm_args { > - unsigned int period; > + u64 period; > enum pwm_polarity polarity; > }; > > @@ -66,9 +66,9 @@ enum pwm_output_type { > * @cycles_per_duty: number of PWM period cycles an entry stays at > */ > struct pwm_output_pattern { > - unsigned int *duty_pattern; > + u64 *duty_pattern; > unsigned int num_entries; > - unsigned int cycles_per_duty; > + u64 cycles_per_duty; > }; > > /* > @@ -79,8 +79,8 @@ struct pwm_output_pattern { > * @enabled: PWM enabled status > */ > struct pwm_state { > - unsigned int period; > - unsigned int duty_cycle; > + u64 period; >
Re: [PATCH 2/2] pwm: core: Add option to config PWM duty/period with u64 data length
Hi Guru, Thank you for the patch! Yet something to improve: [auto build test ERROR on linus/master] [cannot apply to v5.3 next-20190915] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Guru-Das-Srinagesh/pwm-Add-different-PWM-output-types-support/20190916-151008 config: i386-randconfig-h004-201937 (attached as .config) compiler: gcc-7 (Debian 7.4.0-11) 7.4.0 reproduce: # save the attached .config to linux build tree make ARCH=i386 If you fix the issue, kindly add following tag Reported-by: kbuild test robot All errors (new ones prefixed by >>): ld: drivers/hwmon/pwm-fan.o: in function `pwm_fan_resume': >> drivers/hwmon/pwm-fan.c:440: undefined reference to `__udivdi3' vim +440 drivers/hwmon/pwm-fan.c d82d57767c8598 Kamil Debski2014-07-16 420 d82d57767c8598 Kamil Debski2014-07-16 421 static int pwm_fan_resume(struct device *dev) d82d57767c8598 Kamil Debski2014-07-16 422 { d82d57767c8598 Kamil Debski2014-07-16 423 struct pwm_fan_ctx *ctx = dev_get_drvdata(dev); 2289711c9d4d58 Boris Brezillon 2016-04-14 424 struct pwm_args pargs; 48b9d5b4f40825 Kamil Debski2014-11-03 425 unsigned long duty; 48b9d5b4f40825 Kamil Debski2014-11-03 426 int ret; d82d57767c8598 Kamil Debski2014-07-16 427 b57e1d42939721 Stefan Wahren 2019-02-22 428 if (ctx->reg_en) { b57e1d42939721 Stefan Wahren 2019-02-22 429 ret = regulator_enable(ctx->reg_en); b57e1d42939721 Stefan Wahren 2019-02-22 430 if (ret) { b57e1d42939721 Stefan Wahren 2019-02-22 431 dev_err(dev, "Failed to enable fan supply: %d\n", ret); b57e1d42939721 Stefan Wahren 2019-02-22 432 return ret; b57e1d42939721 Stefan Wahren 2019-02-22 433 } b57e1d42939721 Stefan Wahren 2019-02-22 434 } b57e1d42939721 Stefan Wahren 2019-02-22 435 48b9d5b4f40825 Kamil Debski2014-11-03 436 if (ctx->pwm_value == 0) d82d57767c8598 Kamil Debski2014-07-16 437 return 0; 48b9d5b4f40825 Kamil Debski2014-11-03 438 2289711c9d4d58 Boris Brezillon 2016-04-14 439 pwm_get_args(ctx->pwm, &pargs); 2289711c9d4d58 Boris Brezillon 2016-04-14 @440 duty = DIV_ROUND_UP(ctx->pwm_value * (pargs.period - 1), MAX_PWM); 2289711c9d4d58 Boris Brezillon 2016-04-14 441 ret = pwm_config(ctx->pwm, duty, pargs.period); 48b9d5b4f40825 Kamil Debski2014-11-03 442 if (ret) 48b9d5b4f40825 Kamil Debski2014-11-03 443 return ret; 48b9d5b4f40825 Kamil Debski2014-11-03 444 return pwm_enable(ctx->pwm); d82d57767c8598 Kamil Debski2014-07-16 445 } d82d57767c8598 Kamil Debski2014-07-16 446 #endif d82d57767c8598 Kamil Debski2014-07-16 447 :: The code at line 440 was first introduced by commit :: 2289711c9d4d588954ff86a06685f1579bf6c446 hwmon: pwm-fan: Use pwm_get_args() where appropriate :: TO: Boris Brezillon :: CC: Thierry Reding --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
[PATCH 2/2] 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. Change-Id: Ic8722088510d447fc939ab6a5014711aef1b832f Signed-off-by: Fenglin Wu Signed-off-by: Guru Das Srinagesh --- drivers/pwm/core.c | 20 ++--- drivers/pwm/sysfs.c | 6 +-- include/linux/pwm.h | 115 3 files changed, 126 insertions(+), 15 deletions(-) diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c index 960a451..02ad16b 100644 --- a/drivers/pwm/core.c +++ b/drivers/pwm/core.c @@ -526,9 +526,19 @@ int pwm_apply_state(struct pwm_device *pwm, struct pwm_state *state) if (state->period != pwm->state.period || state->duty_cycle != pwm->state.duty_cycle) { - err = pwm->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; @@ -1181,8 +1191,8 @@ static void pwm_dbg_show(struct pwm_chip *chip, struct seq_file *s) if (state.enabled) seq_puts(s, " enabled"); - seq_printf(s, " period: %u ns", state.period); - seq_printf(s, " duty: %u ns", state.duty_cycle); + seq_printf(s, " period: %llu ns", state.period); + seq_printf(s, " duty: %llu ns", state.duty_cycle); seq_printf(s, " polarity: %s", state.polarity ? "inverse" : "normal"); diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c index ab703f2..ef78c40 100644 --- a/drivers/pwm/sysfs.c +++ b/drivers/pwm/sysfs.c @@ -42,7 +42,7 @@ static ssize_t period_show(struct device *child, pwm_get_state(pwm, &state); - return sprintf(buf, "%u\n", state.period); + return sprintf(buf, "%llu\n", state.period); } static ssize_t period_store(struct device *child, @@ -77,7 +77,7 @@ static ssize_t duty_cycle_show(struct device *child, pwm_get_state(pwm, &state); - return sprintf(buf, "%u\n", state.duty_cycle); + return sprintf(buf, "%llu\n", state.duty_cycle); } static ssize_t duty_cycle_store(struct device *child, @@ -212,7 +212,7 @@ static ssize_t capture_show(struct device *child, if (ret) return ret; - return sprintf(buf, "%u %u\n", result.period, result.duty_cycle); + return sprintf(buf, "%llu %llu\n", result.period, result.duty_cycle); } static ssize_t output_type_show(struct device *child, diff --git a/include/linux/pwm.h b/include/linux/pwm.h index 416f08e..d714385 100644 --- a/include/linux/pwm.h +++ b/include/linux/pwm.h @@ -39,7 +39,7 @@ enum pwm_polarity { * current PWM hardware state. */ struct pwm_args { - unsigned int period; + u64 period; enum pwm_polarity polarity; }; @@ -66,9 +66,9 @@ enum pwm_output_type { * @cycles_per_duty: number of PWM period cycles an entry stays at */ struct pwm_output_pattern { - unsigned int *duty_pattern; + u64 *duty_pattern; unsigned int num_entries; - unsigned int cycles_per_duty; + u64 cycles_per_duty; }; /* @@ -79,8 +79,8 @@ struct pwm_output_pattern { * @enabled: PWM enabled status */ struct pwm_state { - unsigned int period; - unsigned int duty_cycle; + u64 period; + u64 duty_cycle; enum pwm_polarity polarity; enum pwm_output_type output_type; struct pwm_output_pattern *output_pattern; @@ -136,12 +136,30 @@ static inline void pwm_set_period(struct pwm_device *pwm, unsigned int period) pwm->state.period = period; } +static inline void pwm_set_period_extend(struct pwm_device *pwm, u64 period) +{ + if (pwm) + pwm->state.period = period; +} + static inline unsigned int pwm_get_period