This series extends the PWM subsystem to support the duty_offset feature
found on some PWM devices. It includes a patch to enable this feature
for the axi-pwmgen driver, which can also serve as an example of how to
implement it for other devices.
The series was tested on actual hardware using a Zedboard. An
oscilloscope was used to validate that the generated PWM signals matched
the requested ones. The libpwm [1] tool was also used for testing the
char device functionality.
Note that in addition to the other patches in this series, the changes
to the axi-pwmgen driver rely on [2] and [3], which haven't been picked
up yet.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/ukleinek/libpwm.git/
[2] https://lore.kernel.org/linux-pwm/[email protected]/
[3] https://lore.kernel.org/linux-pwm/[email protected]/
---
v2 changes:
* Address feedback for driver in v1:
* Remove supports_offset flag in pwm_chip struct, and references to it
in the axi-pwmgen driver patch
* Drop pwm_config_full patch entirely
* Don't return EINVAL when state->duty_offset + state->duty_cycle >
state->period in __pwm_apply(), since this is valid as long as
neither is greater than state->period on its own
* Add a check to disallow setting the PWM signal as inverse and a
nonzero duty_offset at the same time in __pwm_apply(), with a
comment explaining why
Link to v1 (RFC): https://lore.kernel.org/linux-pwm/[email protected]/
Trevor Gamblin (2):
pwm: add duty offset support
pwm: axi-pwmgen: add duty offset support
drivers/pwm/core.c | 82 +++++++++++++++++++++++++++++++++---
drivers/pwm/pwm-axi-pwmgen.c | 34 +++++++++++----
include/linux/pwm.h | 15 +++++++
include/trace/events/pwm.h | 6 ++-
4 files changed, 121 insertions(+), 16 deletions(-)
--
2.44.0
Enable duty_offset feature now that it is supported in the pwm
subsystem. Related macros and struct fields related to duty_offset are
renamed to be consistent.
Signed-off-by: Trevor Gamblin <[email protected]>
---
v2 changes:
* Address feedback for driver in v1:
* Remove line setting supports_offset flag in pwm_chip, since that has
been removed from the struct in core.c.
---
drivers/pwm/pwm-axi-pwmgen.c | 34 ++++++++++++++++++++++++++--------
1 file changed, 26 insertions(+), 8 deletions(-)
diff --git a/drivers/pwm/pwm-axi-pwmgen.c b/drivers/pwm/pwm-axi-pwmgen.c
index 539625c404ac..25a083003432 100644
--- a/drivers/pwm/pwm-axi-pwmgen.c
+++ b/drivers/pwm/pwm-axi-pwmgen.c
@@ -6,9 +6,9 @@
* Copyright 2024 Baylibre SAS
*
* Limitations:
- * - The writes to registers for period and duty are shadowed until
- * LOAD_CONFIG is written to AXI_PWMGEN_REG_CONFIG at the end of the
- * current period.
+ * - The writes to registers for period, duty, and duty_offset are
+ * shadowed until LOAD_CONFIG is written to AXI_PWMGEN_REG_CONFIG at
+ * the end of the current period.
* - Writing LOAD_CONFIG also has the effect of re-synchronizing all
* enabled channels, which could cause glitching on other channels. It
* is therefore expected that channels are assigned harmonic periods
@@ -34,7 +34,7 @@
#define AXI_PWMGEN_REG_NPWM 0x14
#define AXI_PWMGEN_CHX_PERIOD(v, ch) ((v)->period_base + (v)->ch_step * (ch))
#define AXI_PWMGEN_CHX_DUTY(v, ch) ((v)->duty_base + (v)->ch_step * (ch))
-#define AXI_PWMGEN_CHX_OFFSET(v, ch) ((v)->offset_base + (v)->ch_step * (ch))
+#define AXI_PWMGEN_CHX_DUTY_OFFSET(v, ch) ((v)->duty_offset_base + (v)->ch_step * (ch))
#define AXI_PWMGEN_REG_CORE_MAGIC_VAL 0x601A3471 /* Identification number to test during setup */
#define AXI_PWMGEN_LOAD_CONFIG BIT(1)
#define AXI_PWMGEN_RESET BIT(0)
@@ -42,7 +42,7 @@
struct axi_pwm_variant {
u8 period_base;
u8 duty_base;
- u8 offset_base;
+ u8 duty_offset_base;
u8 major_version;
u8 ch_step;
};
@@ -62,7 +62,7 @@ static const struct regmap_config axi_pwmgen_regmap_config = {
static const struct axi_pwm_variant pwmgen_1_00_variant = {
.period_base = 0x40,
.duty_base = 0x44,
- .offset_base = 0x48,
+ .duty_offset_base = 0x48,
.major_version = 1,
.ch_step = 12,
};
@@ -70,7 +70,7 @@ static const struct axi_pwm_variant pwmgen_1_00_variant = {
static const struct axi_pwm_variant pwmgen_2_00_variant = {
.period_base = 0x40,
.duty_base = 0x80,
- .offset_base = 0xC0,
+ .duty_offset_base = 0xC0,
.major_version = 2,
.ch_step = 4,
};
@@ -83,7 +83,7 @@ static int axi_pwmgen_apply(struct pwm_chip *chip, struct pwm_device *pwm,
unsigned int ch = pwm->hwpwm;
struct regmap *regmap = ddata->regmap;
const struct axi_pwm_variant *variant = ddata->variant;
- u64 period_cnt, duty_cnt;
+ u64 period_cnt, duty_cnt, duty_offset_cnt;
int ret;
if (state->polarity != PWM_POLARITY_NORMAL)
@@ -108,6 +108,14 @@ static int axi_pwmgen_apply(struct pwm_chip *chip, struct pwm_device *pwm,
ret = regmap_write(regmap, AXI_PWMGEN_CHX_DUTY(variant, ch), duty_cnt);
if (ret)
return ret;
+
+ duty_offset_cnt = mul_u64_u64_div_u64(state->duty_offset, ddata->clk_rate_hz, NSEC_PER_SEC);
+ if (duty_offset_cnt > UINT_MAX)
+ duty_offset_cnt = UINT_MAX;
+
+ ret = regmap_write(regmap, AXI_PWMGEN_CHX_DUTY_OFFSET(variant, ch), duty_offset_cnt);
+ if (ret)
+ return ret;
} else {
ret = regmap_write(regmap, AXI_PWMGEN_CHX_PERIOD(variant, ch), 0);
if (ret)
@@ -116,6 +124,10 @@ static int axi_pwmgen_apply(struct pwm_chip *chip, struct pwm_device *pwm,
ret = regmap_write(regmap, AXI_PWMGEN_CHX_DUTY(variant, ch), 0);
if (ret)
return ret;
+
+ ret = regmap_write(regmap, AXI_PWMGEN_CHX_DUTY_OFFSET(variant, ch), 0);
+ if (ret)
+ return ret;
}
return regmap_write(regmap, AXI_PWMGEN_REG_CONFIG, AXI_PWMGEN_LOAD_CONFIG);
@@ -145,6 +157,12 @@ static int axi_pwmgen_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
state->duty_cycle = DIV_ROUND_UP_ULL((u64)cnt * NSEC_PER_SEC, ddata->clk_rate_hz);
+ ret = regmap_read(regmap, AXI_PWMGEN_CHX_DUTY_OFFSET(variant, ch), &cnt);
+ if (ret)
+ return ret;
+
+ state->duty_offset = DIV_ROUND_UP_ULL((u64)cnt * NSEC_PER_SEC, ddata->clk_rate_hz);
+
state->polarity = PWM_POLARITY_NORMAL;
return 0;
--
2.44.0
Some PWM chips support a "phase" or "duty_offset" feature. This patch
continues adding support for configuring this property in the PWM
subsystem.
Functions duty_offset_show(), duty_offset_store(), and
pwm_get_duty_offset() are added to match what exists for duty_cycle.
Handle duty_offset in the new pwmchip char device logic.
Add a check to disallow applying a state with both inversed polarity and
a nonzero duty_offset.
Also add duty_offset to TP_printk in include/trace/events/pwm.h so that
it is reported with other properties when using the event tracing pipe
for debug.
Signed-off-by: Trevor Gamblin <[email protected]>
---
v2 changes:
* Address feedback in v1:
* Remove supports_offset flag in pwm_chip struct
* Don't return EINVAL when state->duty_offset + state->duty_cycle >
state->period in __pwm_apply(), since this is valid as long as
neither is greater than state->period on its own
* Add a check to disallow setting the PWM signal as inverse and a
nonzero duty_offset at the same time in __pwm_apply(), with a
comment explaining why
---
drivers/pwm/core.c | 82 +++++++++++++++++++++++++++++++++++---
include/linux/pwm.h | 15 +++++++
include/trace/events/pwm.h | 6 ++-
3 files changed, 95 insertions(+), 8 deletions(-)
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 2745941a008b..0d12cce67be7 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -80,6 +80,7 @@ static void pwm_apply_debug(struct pwm_device *pwm,
*/
if (s1.enabled && s1.polarity != state->polarity) {
s2.polarity = state->polarity;
+ s2.duty_offset = s1.duty_cycle;
s2.duty_cycle = s1.period - s1.duty_cycle;
s2.period = s1.period;
s2.enabled = s1.enabled;
@@ -121,6 +122,23 @@ static void pwm_apply_debug(struct pwm_device *pwm,
state->duty_cycle, state->period,
s2.duty_cycle, s2.period);
+ if (state->enabled &&
+ last->polarity == state->polarity &&
+ last->period == s2.period &&
+ last->duty_offset > s2.duty_offset &&
+ last->duty_offset <= state->duty_offset)
+ dev_warn(pwmchip_parent(chip),
+ ".apply didn't pick the best available duty offset (requested: %llu/%llu, applied: %llu/%llu, possible: %llu/%llu)\n",
+ state->duty_offset, state->period,
+ s2.duty_offset, s2.period,
+ last->duty_offset, last->period);
+
+ if (state->enabled && state->duty_offset < s2.duty_offset)
+ dev_warn(pwmchip_parent(chip),
+ ".apply is supposed to round down duty_offset (requested: %llu/%llu, applied: %llu/%llu)\n",
+ state->duty_offset, state->period,
+ s2.duty_offset, s2.period);
+
if (!state->enabled && s2.enabled && s2.duty_cycle > 0)
dev_warn(pwmchip_parent(chip),
"requested disabled, but yielded enabled with duty > 0\n");
@@ -144,12 +162,13 @@ static void pwm_apply_debug(struct pwm_device *pwm,
if (s1.enabled != last->enabled ||
s1.polarity != last->polarity ||
(s1.enabled && s1.period != last->period) ||
+ (s1.enabled && s1.duty_offset != last->duty_offset) ||
(s1.enabled && s1.duty_cycle != last->duty_cycle)) {
dev_err(pwmchip_parent(chip),
- ".apply is not idempotent (ena=%d pol=%d %llu/%llu) -> (ena=%d pol=%d %llu/%llu)\n",
+ ".apply is not idempotent (ena=%d pol=%d %llu/%llu/%llu) -> (ena=%d pol=%d %llu/%llu/%llu)\n",
s1.enabled, s1.polarity, s1.duty_cycle, s1.period,
- last->enabled, last->polarity, last->duty_cycle,
- last->period);
+ s1.duty_offset, last->enabled, last->polarity,
+ last->duty_cycle, last->period, last->duty_offset);
}
}
@@ -164,13 +183,24 @@ static int __pwm_apply(struct pwm_device *pwm, const struct pwm_state *state)
int err;
if (!pwm || !state || !state->period ||
- state->duty_cycle > state->period)
+ state->duty_cycle > state->period ||
+ state->duty_offset > state->period)
return -EINVAL;
chip = pwm->chip;
+ /*
+ * There is no need to set duty_offset with inverse polarity,
+ * since signals with duty_offset values greater than 0.5 *
+ * period can equivalently be represented by an inverted signal
+ * without offset.
+ */
+ if (state->polarity == PWM_POLARITY_INVERSED && state->duty_offset)
+ return -EINVAL;
+
if (state->period == pwm->state.period &&
state->duty_cycle == pwm->state.duty_cycle &&
+ state->duty_offset == pwm->state.duty_offset &&
state->polarity == pwm->state.polarity &&
state->enabled == pwm->state.enabled &&
state->usage_power == pwm->state.usage_power)
@@ -292,10 +322,11 @@ int pwm_adjust_config(struct pwm_device *pwm)
* been configured.
*
* In either case, we setup the new period and polarity, and assign a
- * duty cycle of 0.
+ * duty cycle and offset of 0.
*/
if (!state.period) {
state.duty_cycle = 0;
+ state.duty_offset = 0;
state.period = pargs.period;
state.polarity = pargs.polarity;
@@ -617,6 +648,41 @@ static ssize_t duty_cycle_store(struct device *pwm_dev,
return ret ? : size;
}
+static ssize_t duty_offset_show(struct device *pwm_dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ const struct pwm_device *pwm = pwm_from_dev(pwm_dev);
+ struct pwm_state state;
+
+ pwm_get_state(pwm, &state);
+
+ return sysfs_emit(buf, "%llu\n", state.duty_offset);
+}
+
+static ssize_t duty_offset_store(struct device *pwm_dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ struct pwm_export *export = pwmexport_from_dev(pwm_dev);
+ struct pwm_device *pwm = export->pwm;
+ struct pwm_state state;
+ u64 val;
+ int ret;
+
+ ret = kstrtou64(buf, 0, &val);
+ if (ret)
+ return ret;
+
+ mutex_lock(&export->lock);
+ pwm_get_state(pwm, &state);
+ state.duty_offset = val;
+ ret = pwm_apply_might_sleep(pwm, &state);
+ mutex_unlock(&export->lock);
+
+ return ret ? : size;
+}
+
static ssize_t enable_show(struct device *pwm_dev,
struct device_attribute *attr,
char *buf)
@@ -731,6 +797,7 @@ static ssize_t capture_show(struct device *pwm_dev,
static DEVICE_ATTR_RW(period);
static DEVICE_ATTR_RW(duty_cycle);
+static DEVICE_ATTR_RW(duty_offset);
static DEVICE_ATTR_RW(enable);
static DEVICE_ATTR_RW(polarity);
static DEVICE_ATTR_RO(capture);
@@ -738,6 +805,7 @@ static DEVICE_ATTR_RO(capture);
static struct attribute *pwm_attrs[] = {
&dev_attr_period.attr,
&dev_attr_duty_cycle.attr,
+ &dev_attr_duty_offset.attr,
&dev_attr_enable.attr,
&dev_attr_polarity.attr,
&dev_attr_capture.attr,
@@ -1290,7 +1358,7 @@ static long pwm_cdev_ioctl(struct file *file, unsigned int cmd, unsigned long ar
if (state.enabled) {
cstate.period = state.period;
if (state.polarity == PWM_POLARITY_NORMAL) {
- cstate.duty_offset = 0;
+ cstate.duty_offset = state.duty_offset;
cstate.duty_cycle = state.duty_cycle;
} else {
cstate.duty_offset = state.duty_cycle;
@@ -1356,6 +1424,7 @@ static long pwm_cdev_ioctl(struct file *file, unsigned int cmd, unsigned long ar
state.period = cstate.period;
state.polarity = PWM_POLARITY_NORMAL;
state.duty_cycle = cstate.duty_cycle;
+ state.duty_offset = cstate.duty_offset;
} else {
state.enabled = false;
}
@@ -1991,6 +2060,7 @@ static void pwm_dbg_show(struct pwm_chip *chip, struct seq_file *s)
seq_printf(s, " period: %llu ns", state.period);
seq_printf(s, " duty: %llu ns", state.duty_cycle);
+ seq_printf(s, " duty_offset: %llu ns", state.duty_offset);
seq_printf(s, " polarity: %s",
state.polarity ? "inverse" : "normal");
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index a58db7011807..5a93212c58eb 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -51,6 +51,7 @@ enum {
* struct pwm_state - state of a PWM channel
* @period: PWM period (in nanoseconds)
* @duty_cycle: PWM duty cycle (in nanoseconds)
+ * @duty_offset: PWM duty offset (in nanoseconds)
* @polarity: PWM polarity
* @enabled: PWM enabled status
* @usage_power: If set, the PWM driver is only required to maintain the power
@@ -61,6 +62,7 @@ enum {
struct pwm_state {
u64 period;
u64 duty_cycle;
+ u64 duty_offset;
enum pwm_polarity polarity;
bool enabled;
bool usage_power;
@@ -130,6 +132,15 @@ static inline u64 pwm_get_duty_cycle(const struct pwm_device *pwm)
return state.duty_cycle;
}
+static inline u64 pwm_get_duty_offset(const struct pwm_device *pwm)
+{
+ struct pwm_state state;
+
+ pwm_get_state(pwm, &state);
+
+ return state.duty_offset;
+}
+
static inline enum pwm_polarity pwm_get_polarity(const struct pwm_device *pwm)
{
struct pwm_state state;
@@ -161,6 +172,9 @@ static inline void pwm_get_args(const struct pwm_device *pwm,
* ->duty_cycle value exceed the pwm_args->period one, which would trigger
* an error if the user calls pwm_apply_might_sleep() without adjusting ->duty_cycle
* first.
+ *
+ * ->duty_offset is likewise set to zero to avoid inconsistent default
+ * states.
*/
static inline void pwm_init_state(const struct pwm_device *pwm,
struct pwm_state *state)
@@ -176,6 +190,7 @@ static inline void pwm_init_state(const struct pwm_device *pwm,
state->period = args.period;
state->polarity = args.polarity;
state->duty_cycle = 0;
+ state->duty_offset = 0;
state->usage_power = false;
}
diff --git a/include/trace/events/pwm.h b/include/trace/events/pwm.h
index 12b35e4ff917..2d25ac5de816 100644
--- a/include/trace/events/pwm.h
+++ b/include/trace/events/pwm.h
@@ -18,6 +18,7 @@ DECLARE_EVENT_CLASS(pwm,
__field(struct pwm_device *, pwm)
__field(u64, period)
__field(u64, duty_cycle)
+ __field(u64, duty_offset)
__field(enum pwm_polarity, polarity)
__field(bool, enabled)
__field(int, err)
@@ -27,13 +28,14 @@ DECLARE_EVENT_CLASS(pwm,
__entry->pwm = pwm;
__entry->period = state->period;
__entry->duty_cycle = state->duty_cycle;
+ __entry->duty_offset = state->duty_offset;
__entry->polarity = state->polarity;
__entry->enabled = state->enabled;
__entry->err = err;
),
- TP_printk("%p: period=%llu duty_cycle=%llu polarity=%d enabled=%d err=%d",
- __entry->pwm, __entry->period, __entry->duty_cycle,
+ TP_printk("%p: period=%llu duty_cycle=%llu duty_offset=%llu polarity=%d enabled=%d err=%d",
+ __entry->pwm, __entry->period, __entry->duty_cycle, __entry->duty_offset,
__entry->polarity, __entry->enabled, __entry->err)
);
--
2.44.0