mbox series

[v2,0/4] Add trace events for SD registers.

Message ID 20190416183257.247902-1-rrangel@chromium.org (mailing list archive)
Headers show
Series Add trace events for SD registers. | expand

Message

Raul Rangel April 16, 2019, 6:32 p.m. UTC
I am not able to make a single event class for all these registers. They
all have different struct sizes and different printf formats.

Thanks for the reviews!

Changes in v2:
- Made trace_sd_scr print out flags.
- Add BUILD_BUG_ON to make sure tracing stays in sync with structs.
- memcpy using sizeof(__entry->raw)

Raul E Rangel (4):
  mmc: core: Add trace event for SD SCR response
  mmc: core: Add trace event for SD SSR response
  mmc: core: Add trace event for SD OCR response
  mmc: core: Add trace event for CSD response

 drivers/mmc/core/mmc.c     |   4 +
 drivers/mmc/core/sd.c      |  10 ++
 drivers/mmc/core/sd_ops.c  |   6 ++
 include/trace/events/mmc.h | 204 +++++++++++++++++++++++++++++++++++++
 4 files changed, 224 insertions(+)

Comments

Ulf Hansson April 23, 2019, 6:29 a.m. UTC | #1
On Tue, 16 Apr 2019 at 20:33, Raul E Rangel <rrangel@chromium.org> wrote:
>
> I am not able to make a single event class for all these registers. They
> all have different struct sizes and different printf formats.
>
> Thanks for the reviews!
>
> Changes in v2:
> - Made trace_sd_scr print out flags.
> - Add BUILD_BUG_ON to make sure tracing stays in sync with structs.
> - memcpy using sizeof(__entry->raw)
>
> Raul E Rangel (4):
>   mmc: core: Add trace event for SD SCR response
>   mmc: core: Add trace event for SD SSR response
>   mmc: core: Add trace event for SD OCR response
>   mmc: core: Add trace event for CSD response
>
>  drivers/mmc/core/mmc.c     |   4 +
>  drivers/mmc/core/sd.c      |  10 ++
>  drivers/mmc/core/sd_ops.c  |   6 ++
>  include/trace/events/mmc.h | 204 +++++++++++++++++++++++++++++++++++++
>  4 files changed, 224 insertions(+)
>
> --
> 2.21.0.392.gf8f6787159e-goog
>

Why do you need this? We already have these card registers reflected
though sysfs files, isn't that sufficient?

Kind regards
Uffe
Raul Rangel April 24, 2019, 3:31 p.m. UTC | #2
On Tue, Apr 23, 2019 at 08:29:15AM +0200, Ulf Hansson wrote:
> On Tue, 16 Apr 2019 at 20:33, Raul E Rangel <rrangel@chromium.org> wrote:
> >
> > I am not able to make a single event class for all these registers. They
> > all have different struct sizes and different printf formats.
> >
> > Thanks for the reviews!
> >
> > Changes in v2:
> > - Made trace_sd_scr print out flags.
> > - Add BUILD_BUG_ON to make sure tracing stays in sync with structs.
> > - memcpy using sizeof(__entry->raw)
> >
> > Raul E Rangel (4):
> >   mmc: core: Add trace event for SD SCR response
> >   mmc: core: Add trace event for SD SSR response
> >   mmc: core: Add trace event for SD OCR response
> >   mmc: core: Add trace event for CSD response
> >
> >  drivers/mmc/core/mmc.c     |   4 +
> >  drivers/mmc/core/sd.c      |  10 ++
> >  drivers/mmc/core/sd_ops.c  |   6 ++
> >  include/trace/events/mmc.h | 204 +++++++++++++++++++++++++++++++++++++
> >  4 files changed, 224 insertions(+)
> >
> > --
> > 2.21.0.392.gf8f6787159e-goog
> >
> 
> Why do you need this? We already have these card registers reflected
> though sysfs files, isn't that sufficient?
> 
I was not actually aware that the registers were exposed via sysfs. I
was debugging a problem where the host controller was returning all
zeros when reading from the card. I wasn't aware that it was returning
all zeros until I added tracing. It made it quite easy to diagnose the
problem by just diffing the two traces.

Also if the card fails to mount because the data is invalid, the
registers would not be exposed via sysfs. So tracing makes gives us an
easy way to debug these types of failures.

Thanks,
Raul
> Kind regards
> Uffe
Ulf Hansson April 29, 2019, 9:57 a.m. UTC | #3
On Wed, 24 Apr 2019 at 17:31, Raul Rangel <rrangel@chromium.org> wrote:
>
> On Tue, Apr 23, 2019 at 08:29:15AM +0200, Ulf Hansson wrote:
> > On Tue, 16 Apr 2019 at 20:33, Raul E Rangel <rrangel@chromium.org> wrote:
> > >
> > > I am not able to make a single event class for all these registers. They
> > > all have different struct sizes and different printf formats.
> > >
> > > Thanks for the reviews!
> > >
> > > Changes in v2:
> > > - Made trace_sd_scr print out flags.
> > > - Add BUILD_BUG_ON to make sure tracing stays in sync with structs.
> > > - memcpy using sizeof(__entry->raw)
> > >
> > > Raul E Rangel (4):
> > >   mmc: core: Add trace event for SD SCR response
> > >   mmc: core: Add trace event for SD SSR response
> > >   mmc: core: Add trace event for SD OCR response
> > >   mmc: core: Add trace event for CSD response
> > >
> > >  drivers/mmc/core/mmc.c     |   4 +
> > >  drivers/mmc/core/sd.c      |  10 ++
> > >  drivers/mmc/core/sd_ops.c  |   6 ++
> > >  include/trace/events/mmc.h | 204 +++++++++++++++++++++++++++++++++++++
> > >  4 files changed, 224 insertions(+)
> > >
> > > --
> > > 2.21.0.392.gf8f6787159e-goog
> > >
> >
> > Why do you need this? We already have these card registers reflected
> > though sysfs files, isn't that sufficient?
> >
> I was not actually aware that the registers were exposed via sysfs. I
> was debugging a problem where the host controller was returning all
> zeros when reading from the card. I wasn't aware that it was returning
> all zeros until I added tracing. It made it quite easy to diagnose the
> problem by just diffing the two traces.

This sounds like a quite an unusual problem, and I don't think having
the buffers printed via tracing is worth it.

Moreover, we already have tracing per command/request (but don't print
the buffers), that should cover most of error cases during init, don't
you think?

[...]

Kind regards
Uffe