* [PATCH v1 0/2] pwm: Add count to sysfs for Intel PWM driver @ 2022-01-03 8:16 vishakha.joshi 2022-01-03 8:16 ` [PATCH v1 1/2] " vishakha.joshi 2022-01-03 8:16 ` [PATCH v1 2/2] pwm: Update the REPEAT_COUNT value vishakha.joshi 0 siblings, 2 replies; 6+ messages in thread From: vishakha.joshi @ 2022-01-03 8:16 UTC (permalink / raw) To: thierry.reding Cc: u.kleine-koenig, lee.jones, linux-pwm, linux-kernel, andriy.shevchenko, jarkko.nikula, vijayakannan.ayyathurai, bala.senthil, tamal.saha, lakshmi.bai.raja.subramanian, vishakha.joshi From: Vishakha Joshi <vishakha.joshi@intel.com> Hi, The patch 1 adds the count for PWM waveform to sysfs interface. The patch 2 updates the count in the KeemBay PWM driver. Please help to review these patches. Thanks, Vishakha Joshi Vishakha Joshi (2): pwm: Add count to sysfs for Intel PWM driver pwm: Update the REPEAT_COUNT value Documentation/ABI/testing/sysfs-class-pwm | 8 ++++++ drivers/pwm/pwm-keembay.c | 9 ++++-- drivers/pwm/sysfs.c | 34 +++++++++++++++++++++++ include/linux/pwm.h | 2 ++ 4 files changed, 50 insertions(+), 3 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v1 1/2] pwm: Add count to sysfs for Intel PWM driver 2022-01-03 8:16 [PATCH v1 0/2] pwm: Add count to sysfs for Intel PWM driver vishakha.joshi @ 2022-01-03 8:16 ` vishakha.joshi 2022-01-03 12:14 ` Uwe Kleine-König 2022-01-03 8:16 ` [PATCH v1 2/2] pwm: Update the REPEAT_COUNT value vishakha.joshi 1 sibling, 1 reply; 6+ messages in thread From: vishakha.joshi @ 2022-01-03 8:16 UTC (permalink / raw) To: thierry.reding Cc: u.kleine-koenig, lee.jones, linux-pwm, linux-kernel, andriy.shevchenko, jarkko.nikula, vijayakannan.ayyathurai, bala.senthil, tamal.saha, lakshmi.bai.raja.subramanian, vishakha.joshi From: Vishakha Joshi <vishakha.joshi@intel.com> The number of cycles in the PWM waveform is generated according to the count value configured in the sysfs interface. This helps to control the duration of the PWM waveform in the KeemBay SoC. In case of default count value which is zero, the PWM waveform repeats indefinitely. Signed-off-by: Vishakha Joshi <vishakha.joshi@intel.com> --- Documentation/ABI/testing/sysfs-class-pwm | 8 ++++++ drivers/pwm/sysfs.c | 34 +++++++++++++++++++++++ include/linux/pwm.h | 2 ++ 3 files changed, 44 insertions(+) diff --git a/Documentation/ABI/testing/sysfs-class-pwm b/Documentation/ABI/testing/sysfs-class-pwm index 3d65285bcd5f..dde57b5a359f 100644 --- a/Documentation/ABI/testing/sysfs-class-pwm +++ b/Documentation/ABI/testing/sysfs-class-pwm @@ -86,3 +86,11 @@ Description: Capture information about a PWM signal. The output format is a pair unsigned integers (period and duty cycle), separated by a single space. + +What: /sys/class/pwm/pwmchip<N>/pwmX/count +Date: December 2021 +KernelVersion: 5.17 +Contact: Vishakha Joshi <vishakha.joshi@intel.com> +Description: + The PWM repeat count of the number of cycles in the waveform. + The default value for the count is zero with infinite repetition. diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c index 9903c3a7eced..4d8fbd134f1d 100644 --- a/drivers/pwm/sysfs.c +++ b/drivers/pwm/sysfs.c @@ -215,11 +215,44 @@ static ssize_t capture_show(struct device *child, return sprintf(buf, "%u %u\n", result.period, result.duty_cycle); } +static ssize_t count_store(struct device *child, struct device_attribute *attr, const char *buf, + size_t size) +{ + struct pwm_export *export = child_to_pwm_export(child); + struct pwm_device *pwm = export->pwm; + struct pwm_state state; + unsigned int count; + int ret; + + ret = kstrtouint(buf, 0, &count); + if (ret) + return ret; + + mutex_lock(&export->lock); + pwm_get_state(pwm, &state); + state.count = count; + ret = pwm_apply_state(pwm, &state); + mutex_unlock(&export->lock); + + return ret ?: size; +} + +static ssize_t count_show(struct device *child, struct device_attribute *attr, char *buf) +{ + const struct pwm_device *pwm = child_to_pwm_device(child); + struct pwm_state state; + + pwm_get_state(pwm, &state); + + return sysfs_emit(buf, "%d\n", state.count); +} + static DEVICE_ATTR_RW(period); static DEVICE_ATTR_RW(duty_cycle); static DEVICE_ATTR_RW(enable); static DEVICE_ATTR_RW(polarity); static DEVICE_ATTR_RO(capture); +static DEVICE_ATTR_RW(count); static struct attribute *pwm_attrs[] = { &dev_attr_period.attr, @@ -227,6 +260,7 @@ static struct attribute *pwm_attrs[] = { &dev_attr_enable.attr, &dev_attr_polarity.attr, &dev_attr_capture.attr, + &dev_attr_count.attr, NULL }; ATTRIBUTE_GROUPS(pwm); diff --git a/include/linux/pwm.h b/include/linux/pwm.h index e6dac95e4960..dc0612867e65 100644 --- a/include/linux/pwm.h +++ b/include/linux/pwm.h @@ -52,6 +52,7 @@ enum { * struct pwm_state - state of a PWM channel * @period: PWM period (in nanoseconds) * @duty_cycle: PWM duty cycle (in nanoseconds) + * @count: PWM repeat count * @polarity: PWM polarity * @enabled: PWM enabled status * @usage_power: If set, the PWM driver is only required to maintain the power @@ -62,6 +63,7 @@ enum { struct pwm_state { u64 period; u64 duty_cycle; + u32 count; enum pwm_polarity polarity; bool enabled; bool usage_power; -- 2.17.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v1 1/2] pwm: Add count to sysfs for Intel PWM driver 2022-01-03 8:16 ` [PATCH v1 1/2] " vishakha.joshi @ 2022-01-03 12:14 ` Uwe Kleine-König 2022-01-04 13:58 ` Andy Shevchenko 0 siblings, 1 reply; 6+ messages in thread From: Uwe Kleine-König @ 2022-01-03 12:14 UTC (permalink / raw) To: vishakha.joshi Cc: thierry.reding, lee.jones, linux-pwm, linux-kernel, andriy.shevchenko, jarkko.nikula, vijayakannan.ayyathurai, bala.senthil, tamal.saha, lakshmi.bai.raja.subramanian [-- Attachment #1: Type: text/plain, Size: 4415 bytes --] Hello, On Mon, Jan 03, 2022 at 01:46:09PM +0530, vishakha.joshi@intel.com wrote: > From: Vishakha Joshi <vishakha.joshi@intel.com> > > The number of cycles in the PWM waveform is generated according to the > count value configured in the sysfs interface. > This helps to control the duration of the PWM waveform in the KeemBay > SoC. > In case of default count value which is zero, the PWM waveform repeats > indefinitely. > > Signed-off-by: Vishakha Joshi <vishakha.joshi@intel.com> > --- > Documentation/ABI/testing/sysfs-class-pwm | 8 ++++++ > drivers/pwm/sysfs.c | 34 +++++++++++++++++++++++ > include/linux/pwm.h | 2 ++ > 3 files changed, 44 insertions(+) > > diff --git a/Documentation/ABI/testing/sysfs-class-pwm b/Documentation/ABI/testing/sysfs-class-pwm > index 3d65285bcd5f..dde57b5a359f 100644 > --- a/Documentation/ABI/testing/sysfs-class-pwm > +++ b/Documentation/ABI/testing/sysfs-class-pwm > @@ -86,3 +86,11 @@ Description: > Capture information about a PWM signal. The output format is a > pair unsigned integers (period and duty cycle), separated by a > single space. > + > +What: /sys/class/pwm/pwmchip<N>/pwmX/count > +Date: December 2021 > +KernelVersion: 5.17 > +Contact: Vishakha Joshi <vishakha.joshi@intel.com> > +Description: > + The PWM repeat count of the number of cycles in the waveform. > + The default value for the count is zero with infinite repetition. > diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c > index 9903c3a7eced..4d8fbd134f1d 100644 > --- a/drivers/pwm/sysfs.c > +++ b/drivers/pwm/sysfs.c > @@ -215,11 +215,44 @@ static ssize_t capture_show(struct device *child, > return sprintf(buf, "%u %u\n", result.period, result.duty_cycle); > } > > +static ssize_t count_store(struct device *child, struct device_attribute *attr, const char *buf, > + size_t size) > +{ > + struct pwm_export *export = child_to_pwm_export(child); > + struct pwm_device *pwm = export->pwm; > + struct pwm_state state; > + unsigned int count; > + int ret; > + > + ret = kstrtouint(buf, 0, &count); > + if (ret) > + return ret; > + > + mutex_lock(&export->lock); > + pwm_get_state(pwm, &state); > + state.count = count; > + ret = pwm_apply_state(pwm, &state); > + mutex_unlock(&export->lock); > + > + return ret ?: size; > +} > + > +static ssize_t count_show(struct device *child, struct device_attribute *attr, char *buf) > +{ > + const struct pwm_device *pwm = child_to_pwm_device(child); > + struct pwm_state state; > + > + pwm_get_state(pwm, &state); > + > + return sysfs_emit(buf, "%d\n", state.count); > +} > + > static DEVICE_ATTR_RW(period); > static DEVICE_ATTR_RW(duty_cycle); > static DEVICE_ATTR_RW(enable); > static DEVICE_ATTR_RW(polarity); > static DEVICE_ATTR_RO(capture); > +static DEVICE_ATTR_RW(count); > > static struct attribute *pwm_attrs[] = { > &dev_attr_period.attr, > @@ -227,6 +260,7 @@ static struct attribute *pwm_attrs[] = { > &dev_attr_enable.attr, > &dev_attr_polarity.attr, > &dev_attr_capture.attr, > + &dev_attr_count.attr, > NULL > }; > ATTRIBUTE_GROUPS(pwm); > diff --git a/include/linux/pwm.h b/include/linux/pwm.h > index e6dac95e4960..dc0612867e65 100644 > --- a/include/linux/pwm.h > +++ b/include/linux/pwm.h > @@ -52,6 +52,7 @@ enum { > * struct pwm_state - state of a PWM channel > * @period: PWM period (in nanoseconds) > * @duty_cycle: PWM duty cycle (in nanoseconds) > + * @count: PWM repeat count > * @polarity: PWM polarity > * @enabled: PWM enabled status > * @usage_power: If set, the PWM driver is only required to maintain the power > @@ -62,6 +63,7 @@ enum { > struct pwm_state { > u64 period; > u64 duty_cycle; > + u32 count; > enum pwm_polarity polarity; > bool enabled; > bool usage_power; This needs more documentation about what the semantic should be. E.g. what should .get_state return? Also I doubt this is a good idea given that most controllers cannot implement it. If we really want to support a count, I request that all drivers that don't support it get updated to refuse a request with count != 0. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ | [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1 1/2] pwm: Add count to sysfs for Intel PWM driver 2022-01-03 12:14 ` Uwe Kleine-König @ 2022-01-04 13:58 ` Andy Shevchenko 2022-01-13 14:11 ` Uwe Kleine-König 0 siblings, 1 reply; 6+ messages in thread From: Andy Shevchenko @ 2022-01-04 13:58 UTC (permalink / raw) To: Uwe Kleine-König Cc: vishakha.joshi, thierry.reding, lee.jones, linux-pwm, linux-kernel, jarkko.nikula, vijayakannan.ayyathurai, bala.senthil, tamal.saha, lakshmi.bai.raja.subramanian On Mon, Jan 03, 2022 at 01:14:54PM +0100, Uwe Kleine-König wrote: > On Mon, Jan 03, 2022 at 01:46:09PM +0530, vishakha.joshi@intel.com wrote: > > From: Vishakha Joshi <vishakha.joshi@intel.com> ... > > struct pwm_state { > > u64 period; > > u64 duty_cycle; > > + u32 count; > > enum pwm_polarity polarity; > > bool enabled; > > bool usage_power; > > This needs more documentation about what the semantic should be. E.g. > what should .get_state return? Good point. I guess ideally we should return the amount of the periods we still need to produce, but I'm not sure if it can be done cleanly. It would probably require a timer to be run in parallel, hence almost full software implementation of the feature. (This answers the second concern) > Also I doubt this is a good idea given that most controllers cannot > implement it. > > If we really want to support a count, I request that all drivers that > don't support it get updated to refuse a request with count != 0. Hmm... Not sure it worth it, perhaps taking into account above the -1 (in unsigned type) returned on ->get_state() can suffice as not supporting feature? -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1 1/2] pwm: Add count to sysfs for Intel PWM driver 2022-01-04 13:58 ` Andy Shevchenko @ 2022-01-13 14:11 ` Uwe Kleine-König 0 siblings, 0 replies; 6+ messages in thread From: Uwe Kleine-König @ 2022-01-13 14:11 UTC (permalink / raw) To: Andy Shevchenko Cc: vishakha.joshi, thierry.reding, lee.jones, linux-pwm, linux-kernel, jarkko.nikula, vijayakannan.ayyathurai, bala.senthil, tamal.saha, lakshmi.bai.raja.subramanian [-- Attachment #1: Type: text/plain, Size: 1064 bytes --] Hello, On Tue, Jan 04, 2022 at 03:58:55PM +0200, Andy Shevchenko wrote: > On Mon, Jan 03, 2022 at 01:14:54PM +0100, Uwe Kleine-König wrote: > > If we really want to support a count, I request that all drivers that > > don't support it get updated to refuse a request with count != 0. > > Hmm... Not sure it worth it, perhaps taking into account above the -1 > (in unsigned type) returned on ->get_state() can suffice as not supporting > feature? In my eyes that's a bad idea. You have to touch most drivers anyhow to set the -1. So the outcome is the worst possible combination: Many changes and still much implicit logic distributed between the drivers and the core about what is supported and what not. (Or you have to initialize .count = -1 before calling the get_state callback. Then you get rid of "have to touch most drivers". But that's still ugly IMO.) Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ | [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v1 2/2] pwm: Update the REPEAT_COUNT value 2022-01-03 8:16 [PATCH v1 0/2] pwm: Add count to sysfs for Intel PWM driver vishakha.joshi 2022-01-03 8:16 ` [PATCH v1 1/2] " vishakha.joshi @ 2022-01-03 8:16 ` vishakha.joshi 1 sibling, 0 replies; 6+ messages in thread From: vishakha.joshi @ 2022-01-03 8:16 UTC (permalink / raw) To: thierry.reding Cc: u.kleine-koenig, lee.jones, linux-pwm, linux-kernel, andriy.shevchenko, jarkko.nikula, vijayakannan.ayyathurai, bala.senthil, tamal.saha, lakshmi.bai.raja.subramanian, vishakha.joshi From: Vishakha Joshi <vishakha.joshi@intel.com> Update the count value in the PWM_LEADIN register. Signed-off-by: Vishakha Joshi <vishakha.joshi@intel.com> --- drivers/pwm/pwm-keembay.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/pwm/pwm-keembay.c b/drivers/pwm/pwm-keembay.c index 733811b05721..4494e54e23b8 100644 --- a/drivers/pwm/pwm-keembay.c +++ b/drivers/pwm/pwm-keembay.c @@ -128,11 +128,14 @@ static int keembay_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, if (state->polarity != PWM_POLARITY_NORMAL) return -EINVAL; + if (state->count > KMB_PWM_COUNT_MAX) + return -EINVAL; + /* - * Configure the pwm repeat count as infinite at (15:0) and leadin - * low time as 0 at (30:16), which is in terms of clock cycles. + * Configure the PWM repeat count at (15:0) and LEADIN low time as 0 at + * (30:16), which is in terms of clock cycles. */ - keembay_pwm_update_bits(priv, KMB_PWM_LEADIN_MASK, 0, + keembay_pwm_update_bits(priv, KMB_PWM_LEADIN_MASK, state->count, KMB_PWM_LEADIN_OFFSET(pwm->hwpwm)); keembay_pwm_get_state(chip, pwm, ¤t_state); -- 2.17.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-01-13 14:11 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-01-03 8:16 [PATCH v1 0/2] pwm: Add count to sysfs for Intel PWM driver vishakha.joshi 2022-01-03 8:16 ` [PATCH v1 1/2] " vishakha.joshi 2022-01-03 12:14 ` Uwe Kleine-König 2022-01-04 13:58 ` Andy Shevchenko 2022-01-13 14:11 ` Uwe Kleine-König 2022-01-03 8:16 ` [PATCH v1 2/2] pwm: Update the REPEAT_COUNT value vishakha.joshi
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.