| Message ID | 20240705211452.1157967-2-u.kleine-koenig@baylibre.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | pwm: Make info in traces about affected pwm more useful | expand |
Hello, On Fri, Jul 05, 2024 at 11:14:51PM +0200, Uwe Kleine-König wrote: > The hashed pointer isn't useful to identify the pwm device. Instead > store and emit chipid and hwpwm. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com> > --- > include/trace/events/pwm.h | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/include/trace/events/pwm.h b/include/trace/events/pwm.h > index 12b35e4ff917..8022701c446d 100644 > --- a/include/trace/events/pwm.h > +++ b/include/trace/events/pwm.h > @@ -15,7 +15,8 @@ DECLARE_EVENT_CLASS(pwm, > TP_ARGS(pwm, state, err), > > TP_STRUCT__entry( > - __field(struct pwm_device *, pwm) > + __field(unsigned int, chipid) > + __field(unsigned int, hwpwm) > __field(u64, period) > __field(u64, duty_cycle) > __field(enum pwm_polarity, polarity) > @@ -24,7 +25,8 @@ DECLARE_EVENT_CLASS(pwm, > ), > > TP_fast_assign( > - __entry->pwm = pwm; > + __entry->chipid = pwm->chip->id; > + __entry->hwpwm = pwm->hwpwm; > __entry->period = state->period; > __entry->duty_cycle = state->duty_cycle; > __entry->polarity = state->polarity; > @@ -32,8 +34,8 @@ DECLARE_EVENT_CLASS(pwm, > __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("pwmchip%u.%u: period=%llu duty_cycle=%llu polarity=%d enabled=%d err=%d", > + __entry->chipid, __entry->hwpwm, __entry->period, __entry->duty_cycle, > __entry->polarity, __entry->enabled, __entry->err) > > ); I think the patch is obvious enough to be ok even without the tracing maintainer's blessing. I applied it to https://git.kernel.org/pub/scm/linux/kernel/git/ukleinek/linux.git pwm/for-next . Best regards Uwe
On Tue, 30 Jul 2024 09:22:53 +0200 Uwe Kleine-König <u.kleine-koenig@baylibre.com> wrote: > I think the patch is obvious enough to be ok even without the tracing > maintainer's blessing. I applied it to > https://git.kernel.org/pub/scm/linux/kernel/git/ukleinek/linux.git pwm/for-next > . No problem. Thanks for the Cc. I don't always reply if the patch is pretty straight forward. But I do look for things like holes in the TP_struct() portion, that would waste ring buffer space. As well as uses of dereferencing pointers. -- Steve
On Tue, Jul 30, 2024 at 10:39:10AM -0400, Steven Rostedt wrote: > On Tue, 30 Jul 2024 09:22:53 +0200 > Uwe Kleine-König <u.kleine-koenig@baylibre.com> wrote: > > > I think the patch is obvious enough to be ok even without the tracing > > maintainer's blessing. I applied it to > > https://git.kernel.org/pub/scm/linux/kernel/git/ukleinek/linux.git pwm/for-next > > . > > No problem. Thanks for the Cc. I don't always reply if the patch is pretty > straight forward. But I do look for things like holes in the TP_struct() > portion, that would waste ring buffer space. As well as uses of > dereferencing pointers. Looking at https://lore.kernel.org/linux-pwm/7b9c9ee490df1df1de3bbfafd501f45c6cb2ec4c.1722261050.git.u.kleine-koenig@baylibre.com is this also in the "straight forward" category? If not, some feedback would be appreciated there. Thanks Uwe
diff --git a/include/trace/events/pwm.h b/include/trace/events/pwm.h index 12b35e4ff917..8022701c446d 100644 --- a/include/trace/events/pwm.h +++ b/include/trace/events/pwm.h @@ -15,7 +15,8 @@ DECLARE_EVENT_CLASS(pwm, TP_ARGS(pwm, state, err), TP_STRUCT__entry( - __field(struct pwm_device *, pwm) + __field(unsigned int, chipid) + __field(unsigned int, hwpwm) __field(u64, period) __field(u64, duty_cycle) __field(enum pwm_polarity, polarity) @@ -24,7 +25,8 @@ DECLARE_EVENT_CLASS(pwm, ), TP_fast_assign( - __entry->pwm = pwm; + __entry->chipid = pwm->chip->id; + __entry->hwpwm = pwm->hwpwm; __entry->period = state->period; __entry->duty_cycle = state->duty_cycle; __entry->polarity = state->polarity; @@ -32,8 +34,8 @@ DECLARE_EVENT_CLASS(pwm, __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("pwmchip%u.%u: period=%llu duty_cycle=%llu polarity=%d enabled=%d err=%d", + __entry->chipid, __entry->hwpwm, __entry->period, __entry->duty_cycle, __entry->polarity, __entry->enabled, __entry->err) );
The hashed pointer isn't useful to identify the pwm device. Instead store and emit chipid and hwpwm. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com> --- include/trace/events/pwm.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) base-commit: 6ba37c70795abf1d59976b3a49acafac14b72a4f