Re: [PATCH 3/3] Input: applespi: Add trace_event module param for early tracing.

[Life is hard, and then you die]

  Hi Dmitry,

On Wed, Feb 17, 2021 at 01:06:27PM -0800, Dmitry Torokhov wrote:
> On Wed, Feb 17, 2021 at 12:52:57PM -0800, Life is hard, and then you die wrote:
> > 
> > On Wed, Feb 17, 2021 at 12:26:18PM -0800, Dmitry Torokhov wrote:
> > > 
> > > On Wed, Feb 17, 2021 at 11:07:18AM -0800, Ronald Tschalär wrote:
> > > > The problem is that tracing can't be set via sysfs until the module is
> > > > loaded, at which point the keyboard and trackpad initialization commands
> > > > have already been run and hence tracing can't be used to debug problems
> > > > here.
> > > > 
> > > > Adding this param allows tracing to be enabled for messages sent and
> > > > received during module probing. It takes comma-separated list of events,
> > > > e.g.
> > > > 
> > > >   trace_event=applespi_tp_ini_cmd,applespi_bad_crc
> > > 
> > > You can unbind and rebind a device to a driver via sysfs as many times
> > > as needed (see bind and unbind driver sysfs attributes), so I believe

Ok yes, that works well, except for the boot-debug scenario.

> > Hmm, I'm going to have to play with that a bit, but one place it still
> > won't help I think is something we ran into in practise: init was
> > failing during boot, but was successfull later on.
> 
> Maybe compiling module as a built-in and then using kernel command line
> option to initiate the trace would work?

My personal issue with this is the fact that most folks reporting
issues are running their distro's standard kernel, which invariably
has this (and most others) compiled as a loadable module; and asking
folks to rebuild their kernel is actually quite a hurdle for them, in
particular compared to asking them to just add some boot params or
manipulating some sysfs entries. So I prefer to try to provide easy
ways for folks to be able to generate and report info back that work
and are enabled out-of-the-box on most distros.

> If this facility is really needed, it would be beneficial for other
> modules as well, and thus better implemented in the module loading code
> to activate desired tracing after loading the module but before invoking
> module init code.

I don't know if it rises to the level of "really needed" - I certainly
needed something like this to debug an issue and hence the module
param. And I figured if somebody adds/debugs additional init commands
they could find it useful too. But this may not be commonly needed
after all, or folks are using some other solution.

If there's interest, I might be able to take a stab a this in the near
future, but not sure.


  Cheers,

  Ronald