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, );
>
> - 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, );
>
> - 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
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,
);
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, );
- 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, );
- 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(const
