Message ID | 7f7ce46efc76e02a472ed52ab1fc06c3575a14e8.1720435656.git.u.kleine-koenig@baylibre.com |
---|---|
State | Superseded |
Headers | show |
Series | pwm: New abstraction and userspace API | expand |
Missing a sign-off? On 2024-07-08 6:52 a.m., Uwe Kleine-König wrote: > --- > drivers/pwm/core.c | 68 ++++++++++++++++--- > include/trace/events/pwm.h | 134 ++++++++++++++++++++++++++++++++++--- > 2 files changed, 183 insertions(+), 19 deletions(-) > > diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c > index d64c033c4cb2..a2320ae77220 100644 > --- a/drivers/pwm/core.c > +++ b/drivers/pwm/core.c > @@ -138,6 +138,52 @@ static int pwm_check_rounding(const struct pwm_waveform *wf, > return 0; > } > > +static int pwm_round_waveform_tohw(struct pwm_chip *chip, struct pwm_device *pwm, > + const struct pwm_waveform *wf, void *wfhw) > +{ > + const struct pwm_ops *ops = chip->ops; > + int ret; > + > + ret = ops->round_waveform_tohw(chip, pwm, wf, wfhw); > + trace_pwm_round_waveform_tohw(pwm, wf, wfhw, ret); > + > + return ret; > +} > + > +static int pwm_round_waveform_fromhw(struct pwm_chip *chip, struct pwm_device *pwm, > + const void *wfhw, struct pwm_waveform *wf) > +{ > + const struct pwm_ops *ops = chip->ops; > + int ret; > + > + ret = ops->round_waveform_fromhw(chip, pwm, wfhw, wf); > + trace_pwm_round_waveform_fromhw(pwm, wfhw, wf, ret); > + > + return ret; > +} > + > +static int pwm_read_waveform(struct pwm_chip *chip, struct pwm_device *pwm, void *wfhw) > +{ > + const struct pwm_ops *ops = chip->ops; > + int ret; > + > + ret = ops->read_waveform(chip, pwm, wfhw); > + trace_pwm_read_waveform(pwm, wfhw, ret); > + > + return ret; > +} > + > +static int pwm_write_waveform(struct pwm_chip *chip, struct pwm_device *pwm, const void *wfhw) > +{ > + const struct pwm_ops *ops = chip->ops; > + int ret; > + > + ret = ops->write_waveform(chip, pwm, wfhw); > + trace_pwm_write_waveform(pwm, wfhw, ret); > + > + return ret; > +} > + > #define WFHWSIZE 20 > > static int pwm_get_waveform(struct pwm_device *pwm, > @@ -155,11 +201,11 @@ static int pwm_get_waveform(struct pwm_device *pwm, > if (!chip->operational) > return -ENODEV; > > - err = ops->read_waveform(chip, pwm, &wfhw); > + err = pwm_read_waveform(chip, pwm, &wfhw); > if (err) > return err; > > - return ops->round_waveform_fromhw(chip, pwm, &wfhw, wf); > + return pwm_round_waveform_fromhw(chip, pwm, &wfhw, wf); > } > > /* Called with the pwmchip lock held */ > @@ -180,12 +226,12 @@ static int __pwm_set_waveform(struct pwm_device *pwm, > wf->duty_offset >= wf->period_length)) > return -EINVAL; > > - err = ops->round_waveform_tohw(chip, pwm, wf, &wfhw); > + err = pwm_round_waveform_tohw(chip, pwm, wf, &wfhw); > if (err) > return err; > > if ((IS_ENABLED(CONFIG_PWM_DEBUG) || exact) && wf->period_length) { > - err = ops->round_waveform_fromhw(chip, pwm, &wfhw, &wf_rounded); > + err = pwm_round_waveform_fromhw(chip, pwm, &wfhw, &wf_rounded); > if (err) > return err; > > @@ -203,7 +249,7 @@ static int __pwm_set_waveform(struct pwm_device *pwm, > } > } > > - err = ops->write_waveform(chip, pwm, &wfhw); > + err = pwm_write_waveform(chip, pwm, &wfhw); > if (err) > return err; > > @@ -213,12 +259,12 @@ static int __pwm_set_waveform(struct pwm_device *pwm, > if (IS_ENABLED(CONFIG_PWM_DEBUG) && ops->read_waveform && wf->period_length) { > struct pwm_waveform wf_set; > > - err = ops->read_waveform(chip, pwm, &wfhw); > + err = pwm_read_waveform(chip, pwm, &wfhw); > if (err) > /* maybe ignore? */ > return err; > > - err = ops->round_waveform_fromhw(chip, pwm, &wfhw, &wf_set); > + err = pwm_round_waveform_fromhw(chip, pwm, &wfhw, &wf_set); > if (err) > /* maybe ignore? */ > return err; > @@ -443,11 +489,11 @@ static int __pwm_apply(struct pwm_device *pwm, const struct pwm_state *state) > * high. > */ > > - err = ops->round_waveform_tohw(chip, pwm, &wf, &wfhw); > + err = pwm_round_waveform_tohw(chip, pwm, &wf, &wfhw); > if (err) > return err; > > - err = ops->write_waveform(chip, pwm, &wfhw); > + err = pwm_write_waveform(chip, pwm, &wfhw); > if (err) > return err; > > @@ -548,11 +594,11 @@ static int pwm_get_state_hw(struct pwm_device *pwm, struct pwm_state *state) > > scoped_guard(pwmchip, chip) { > > - ret = ops->read_waveform(chip, pwm, &wfhw); > + ret = pwm_read_waveform(chip, pwm, &wfhw); > if (ret) > return ret; > > - ret = ops->round_waveform_fromhw(chip, pwm, &wfhw, &wf); > + ret = pwm_round_waveform_fromhw(chip, pwm, &wfhw, &wf); > if (ret) > return ret; > } > diff --git a/include/trace/events/pwm.h b/include/trace/events/pwm.h > index 8022701c446d..3b5b20d2aff0 100644 > --- a/include/trace/events/pwm.h > +++ b/include/trace/events/pwm.h > @@ -8,15 +8,135 @@ > #include <linux/pwm.h> > #include <linux/tracepoint.h> > > +#define TP_PROTO_pwm(args...) \ > + TP_PROTO(struct pwm_device *pwm, args) > + > +#define TP_ARGS_pwm(args...) \ > + TP_ARGS(pwm, args) > + > +#define TP_STRUCT__entry_pwm(args...) \ > + TP_STRUCT__entry( \ > + __field(unsigned int, chipid) \ > + __field(unsigned int, hwpwm) \ > + args) > + > +#define TP_fast_assign_pwm(args...) \ > + TP_fast_assign( \ > + __entry->chipid = pwm->chip->id; \ > + __entry->hwpwm = pwm->hwpwm; \ > + args) > + > +#define TP_printk_pwm(fmt, args...) \ > + TP_printk("pwmchip%u.%u: " fmt, __entry->chipid, __entry->hwpwm, args) > + > +#define __field_pwmwf(wf) \ > + __field(u64, wf ## _period_length) \ > + __field(u64, wf ## _duty_length) \ > + __field(u64, wf ## _duty_offset) \ > + > +#define fast_assign_pwmwf(wf) \ > + __entry->wf ## _period_length = wf->period_length; \ > + __entry->wf ## _duty_length = wf->duty_length; \ > + __entry->wf ## _duty_offset = wf->duty_offset > + > +#define printk_pwmwf_format(wf) \ > + "%lld/%lld [+%lld]" > + > +#define printk_pwmwf_formatargs(wf) \ > + __entry->wf ## _duty_length, __entry->wf ## _period_length, __entry->wf ## _duty_offset > + > +TRACE_EVENT(pwm_round_waveform_tohw, > + > + TP_PROTO_pwm(const struct pwm_waveform *wf, void *wfhw, int err), > + > + TP_ARGS_pwm(wf, wfhw, err), > + > + TP_STRUCT__entry_pwm( > + __field_pwmwf(wf) > + __field(void *, wfhw) > + __field(int, err) > + ), > + > + TP_fast_assign_pwm( > + fast_assign_pwmwf(wf); > + __entry->wfhw = wfhw; > + __entry->err = err; > + ), > + > + TP_printk_pwm(printk_pwmwf_format(wf) " > %p err=%d", > + printk_pwmwf_formatargs(wf), __entry->wfhw, __entry->err) > +); > + > +TRACE_EVENT(pwm_round_waveform_fromhw, > + > + TP_PROTO_pwm(const void *wfhw, struct pwm_waveform *wf, int err), > + > + TP_ARGS_pwm(wfhw, wf, err), > + > + TP_STRUCT__entry_pwm( > + __field(const void *, wfhw) > + __field_pwmwf(wf) > + __field(int, err) > + ), > + > + TP_fast_assign_pwm( > + __entry->wfhw = wfhw; > + fast_assign_pwmwf(wf); > + __entry->err = err; > + ), > + > + TP_printk_pwm("%p > " printk_pwmwf_format(wf) " err=%d", > + __entry->wfhw, printk_pwmwf_formatargs(wf), __entry->err) > +); > + > +TRACE_EVENT(pwm_read_waveform, > + > + TP_PROTO_pwm(void *wfhw, int err), > + > + TP_ARGS_pwm(wfhw, err), > + > + TP_STRUCT__entry_pwm( > + __field(void *, wfhw) > + __field(int, err) > + ), > + > + TP_fast_assign_pwm( > + __entry->wfhw = wfhw; > + __entry->err = err; > + ), > + > + TP_printk_pwm("%p err=%d", > + __entry->wfhw, __entry->err) > +); > + > +TRACE_EVENT(pwm_write_waveform, > + > + TP_PROTO_pwm(const void *wfhw, int err), > + > + TP_ARGS_pwm(wfhw, err), > + > + TP_STRUCT__entry_pwm( > + __field(const void *, wfhw) > + __field(int, err) > + ), > + > + TP_fast_assign_pwm( > + __entry->wfhw = wfhw; > + __entry->err = err; > + ), > + > + TP_printk_pwm("%p err=%d", > + __entry->wfhw, __entry->err) > +); > + > + > DECLARE_EVENT_CLASS(pwm, > > TP_PROTO(struct pwm_device *pwm, const struct pwm_state *state, int err), > > TP_ARGS(pwm, state, err), > > - TP_STRUCT__entry( > - __field(unsigned int, chipid) > - __field(unsigned int, hwpwm) > + TP_STRUCT__entry_pwm( > __field(u64, period) > __field(u64, duty_cycle) > __field(enum pwm_polarity, polarity) > @@ -24,9 +144,7 @@ DECLARE_EVENT_CLASS(pwm, > __field(int, err) > ), > > - TP_fast_assign( > - __entry->chipid = pwm->chip->id; > - __entry->hwpwm = pwm->hwpwm; > + TP_fast_assign_pwm( > __entry->period = state->period; > __entry->duty_cycle = state->duty_cycle; > __entry->polarity = state->polarity; > @@ -34,8 +152,8 @@ DECLARE_EVENT_CLASS(pwm, > __entry->err = err; > ), > > - TP_printk("pwmchip%u.%u: period=%llu duty_cycle=%llu polarity=%d enabled=%d err=%d", > - __entry->chipid, __entry->hwpwm, __entry->period, __entry->duty_cycle, > + TP_printk_pwm("period=%llu duty_cycle=%llu polarity=%d enabled=%d err=%d", > + __entry->period, __entry->duty_cycle, > __entry->polarity, __entry->enabled, __entry->err) > > );
On Mon, Jul 08, 2024 at 02:14:09PM -0400, Trevor Gamblin wrote:
> Missing a sign-off?
Yes, and a commit log.
Thanks for your review!
Best regards
Uwe
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c index d64c033c4cb2..a2320ae77220 100644 --- a/drivers/pwm/core.c +++ b/drivers/pwm/core.c @@ -138,6 +138,52 @@ static int pwm_check_rounding(const struct pwm_waveform *wf, return 0; } +static int pwm_round_waveform_tohw(struct pwm_chip *chip, struct pwm_device *pwm, + const struct pwm_waveform *wf, void *wfhw) +{ + const struct pwm_ops *ops = chip->ops; + int ret; + + ret = ops->round_waveform_tohw(chip, pwm, wf, wfhw); + trace_pwm_round_waveform_tohw(pwm, wf, wfhw, ret); + + return ret; +} + +static int pwm_round_waveform_fromhw(struct pwm_chip *chip, struct pwm_device *pwm, + const void *wfhw, struct pwm_waveform *wf) +{ + const struct pwm_ops *ops = chip->ops; + int ret; + + ret = ops->round_waveform_fromhw(chip, pwm, wfhw, wf); + trace_pwm_round_waveform_fromhw(pwm, wfhw, wf, ret); + + return ret; +} + +static int pwm_read_waveform(struct pwm_chip *chip, struct pwm_device *pwm, void *wfhw) +{ + const struct pwm_ops *ops = chip->ops; + int ret; + + ret = ops->read_waveform(chip, pwm, wfhw); + trace_pwm_read_waveform(pwm, wfhw, ret); + + return ret; +} + +static int pwm_write_waveform(struct pwm_chip *chip, struct pwm_device *pwm, const void *wfhw) +{ + const struct pwm_ops *ops = chip->ops; + int ret; + + ret = ops->write_waveform(chip, pwm, wfhw); + trace_pwm_write_waveform(pwm, wfhw, ret); + + return ret; +} + #define WFHWSIZE 20 static int pwm_get_waveform(struct pwm_device *pwm, @@ -155,11 +201,11 @@ static int pwm_get_waveform(struct pwm_device *pwm, if (!chip->operational) return -ENODEV; - err = ops->read_waveform(chip, pwm, &wfhw); + err = pwm_read_waveform(chip, pwm, &wfhw); if (err) return err; - return ops->round_waveform_fromhw(chip, pwm, &wfhw, wf); + return pwm_round_waveform_fromhw(chip, pwm, &wfhw, wf); } /* Called with the pwmchip lock held */ @@ -180,12 +226,12 @@ static int __pwm_set_waveform(struct pwm_device *pwm, wf->duty_offset >= wf->period_length)) return -EINVAL; - err = ops->round_waveform_tohw(chip, pwm, wf, &wfhw); + err = pwm_round_waveform_tohw(chip, pwm, wf, &wfhw); if (err) return err; if ((IS_ENABLED(CONFIG_PWM_DEBUG) || exact) && wf->period_length) { - err = ops->round_waveform_fromhw(chip, pwm, &wfhw, &wf_rounded); + err = pwm_round_waveform_fromhw(chip, pwm, &wfhw, &wf_rounded); if (err) return err; @@ -203,7 +249,7 @@ static int __pwm_set_waveform(struct pwm_device *pwm, } } - err = ops->write_waveform(chip, pwm, &wfhw); + err = pwm_write_waveform(chip, pwm, &wfhw); if (err) return err; @@ -213,12 +259,12 @@ static int __pwm_set_waveform(struct pwm_device *pwm, if (IS_ENABLED(CONFIG_PWM_DEBUG) && ops->read_waveform && wf->period_length) { struct pwm_waveform wf_set; - err = ops->read_waveform(chip, pwm, &wfhw); + err = pwm_read_waveform(chip, pwm, &wfhw); if (err) /* maybe ignore? */ return err; - err = ops->round_waveform_fromhw(chip, pwm, &wfhw, &wf_set); + err = pwm_round_waveform_fromhw(chip, pwm, &wfhw, &wf_set); if (err) /* maybe ignore? */ return err; @@ -443,11 +489,11 @@ static int __pwm_apply(struct pwm_device *pwm, const struct pwm_state *state) * high. */ - err = ops->round_waveform_tohw(chip, pwm, &wf, &wfhw); + err = pwm_round_waveform_tohw(chip, pwm, &wf, &wfhw); if (err) return err; - err = ops->write_waveform(chip, pwm, &wfhw); + err = pwm_write_waveform(chip, pwm, &wfhw); if (err) return err; @@ -548,11 +594,11 @@ static int pwm_get_state_hw(struct pwm_device *pwm, struct pwm_state *state) scoped_guard(pwmchip, chip) { - ret = ops->read_waveform(chip, pwm, &wfhw); + ret = pwm_read_waveform(chip, pwm, &wfhw); if (ret) return ret; - ret = ops->round_waveform_fromhw(chip, pwm, &wfhw, &wf); + ret = pwm_round_waveform_fromhw(chip, pwm, &wfhw, &wf); if (ret) return ret; } diff --git a/include/trace/events/pwm.h b/include/trace/events/pwm.h index 8022701c446d..3b5b20d2aff0 100644 --- a/include/trace/events/pwm.h +++ b/include/trace/events/pwm.h @@ -8,15 +8,135 @@ #include <linux/pwm.h> #include <linux/tracepoint.h> +#define TP_PROTO_pwm(args...) \ + TP_PROTO(struct pwm_device *pwm, args) + +#define TP_ARGS_pwm(args...) \ + TP_ARGS(pwm, args) + +#define TP_STRUCT__entry_pwm(args...) \ + TP_STRUCT__entry( \ + __field(unsigned int, chipid) \ + __field(unsigned int, hwpwm) \ + args) + +#define TP_fast_assign_pwm(args...) \ + TP_fast_assign( \ + __entry->chipid = pwm->chip->id; \ + __entry->hwpwm = pwm->hwpwm; \ + args) + +#define TP_printk_pwm(fmt, args...) \ + TP_printk("pwmchip%u.%u: " fmt, __entry->chipid, __entry->hwpwm, args) + +#define __field_pwmwf(wf) \ + __field(u64, wf ## _period_length) \ + __field(u64, wf ## _duty_length) \ + __field(u64, wf ## _duty_offset) \ + +#define fast_assign_pwmwf(wf) \ + __entry->wf ## _period_length = wf->period_length; \ + __entry->wf ## _duty_length = wf->duty_length; \ + __entry->wf ## _duty_offset = wf->duty_offset + +#define printk_pwmwf_format(wf) \ + "%lld/%lld [+%lld]" + +#define printk_pwmwf_formatargs(wf) \ + __entry->wf ## _duty_length, __entry->wf ## _period_length, __entry->wf ## _duty_offset + +TRACE_EVENT(pwm_round_waveform_tohw, + + TP_PROTO_pwm(const struct pwm_waveform *wf, void *wfhw, int err), + + TP_ARGS_pwm(wf, wfhw, err), + + TP_STRUCT__entry_pwm( + __field_pwmwf(wf) + __field(void *, wfhw) + __field(int, err) + ), + + TP_fast_assign_pwm( + fast_assign_pwmwf(wf); + __entry->wfhw = wfhw; + __entry->err = err; + ), + + TP_printk_pwm(printk_pwmwf_format(wf) " > %p err=%d", + printk_pwmwf_formatargs(wf), __entry->wfhw, __entry->err) +); + +TRACE_EVENT(pwm_round_waveform_fromhw, + + TP_PROTO_pwm(const void *wfhw, struct pwm_waveform *wf, int err), + + TP_ARGS_pwm(wfhw, wf, err), + + TP_STRUCT__entry_pwm( + __field(const void *, wfhw) + __field_pwmwf(wf) + __field(int, err) + ), + + TP_fast_assign_pwm( + __entry->wfhw = wfhw; + fast_assign_pwmwf(wf); + __entry->err = err; + ), + + TP_printk_pwm("%p > " printk_pwmwf_format(wf) " err=%d", + __entry->wfhw, printk_pwmwf_formatargs(wf), __entry->err) +); + +TRACE_EVENT(pwm_read_waveform, + + TP_PROTO_pwm(void *wfhw, int err), + + TP_ARGS_pwm(wfhw, err), + + TP_STRUCT__entry_pwm( + __field(void *, wfhw) + __field(int, err) + ), + + TP_fast_assign_pwm( + __entry->wfhw = wfhw; + __entry->err = err; + ), + + TP_printk_pwm("%p err=%d", + __entry->wfhw, __entry->err) +); + +TRACE_EVENT(pwm_write_waveform, + + TP_PROTO_pwm(const void *wfhw, int err), + + TP_ARGS_pwm(wfhw, err), + + TP_STRUCT__entry_pwm( + __field(const void *, wfhw) + __field(int, err) + ), + + TP_fast_assign_pwm( + __entry->wfhw = wfhw; + __entry->err = err; + ), + + TP_printk_pwm("%p err=%d", + __entry->wfhw, __entry->err) +); + + DECLARE_EVENT_CLASS(pwm, TP_PROTO(struct pwm_device *pwm, const struct pwm_state *state, int err), TP_ARGS(pwm, state, err), - TP_STRUCT__entry( - __field(unsigned int, chipid) - __field(unsigned int, hwpwm) + TP_STRUCT__entry_pwm( __field(u64, period) __field(u64, duty_cycle) __field(enum pwm_polarity, polarity) @@ -24,9 +144,7 @@ DECLARE_EVENT_CLASS(pwm, __field(int, err) ), - TP_fast_assign( - __entry->chipid = pwm->chip->id; - __entry->hwpwm = pwm->hwpwm; + TP_fast_assign_pwm( __entry->period = state->period; __entry->duty_cycle = state->duty_cycle; __entry->polarity = state->polarity; @@ -34,8 +152,8 @@ DECLARE_EVENT_CLASS(pwm, __entry->err = err; ), - TP_printk("pwmchip%u.%u: period=%llu duty_cycle=%llu polarity=%d enabled=%d err=%d", - __entry->chipid, __entry->hwpwm, __entry->period, __entry->duty_cycle, + TP_printk_pwm("period=%llu duty_cycle=%llu polarity=%d enabled=%d err=%d", + __entry->period, __entry->duty_cycle, __entry->polarity, __entry->enabled, __entry->err) );