On 21-10-19, 05:20, Pierre-Louis Bossart wrote:
> On 10/20/19 11:04 PM, Vinod Koul wrote:
> > On 16-09-19, 14:09, Pierre-Louis Bossart wrote:
> > > Provide debugfs capability to kick link and devices into hard-reset
> > > (as defined by MIPI). This capability is really useful when some
> > > devices are no longer responsive and/or to check the software handling
> > > of resynchronization.
> > >
> > > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
> > > ---
> > > drivers/soundwire/cadence_master.c | 20 ++++++++++++++++++++
> > > 1 file changed, 20 insertions(+)
> > >
> > > diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c
> > > index e3d06330d125..5f900cf2acb9 100644
> > > --- a/drivers/soundwire/cadence_master.c
> > > +++ b/drivers/soundwire/cadence_master.c
> > > @@ -340,6 +340,23 @@ static int cdns_reg_show(struct seq_file *s, void *data)
> > > }
> > > DEFINE_SHOW_ATTRIBUTE(cdns_reg);
> > > +static int cdns_hw_reset(void *data, u64 value)
> > > +{
> > > + struct sdw_cdns *cdns = data;
> > > + int ret;
> > > +
> > > + if (value != 1)
> > > + return -EINVAL;
> > > +
> > > + ret = sdw_cdns_exit_reset(cdns);
> >
> > So we are performing reset of the device behind the kernel, so I think
> > it makes sense to mark the kernel as tainted.
>
> This is a bit ironic. This reset is the only way to prove that the
> enumeration is done right and that all the subsystem fully recovers, and yet
> we'd mark the kernel as 'untrustworthy' and all bug reports would be
> ignored.
Nope you dont expect this would be done on a production system and for
you own testing you can choose not to ignore the reports!
> I don't mind doing this but we'd have to agree on a code. The only one I see
> relevant is "taint requested by userspace application", which is not exactly
> super useful.
But it does tell you that userspace did something so watch out!
> > > + dev_dbg(cdns->dev, "link hw_reset done: %d\n", ret);
> > > +
> > > + return ret;
> >
> > We may want to get rid of the debug and do:
> > return sdw_cdns_exit_reset();
>
> this debug line is useful, it marks the start of the reset sequence and
> that's valuable information. Remove it and you've lost the ability to
> analyze the dmesg logs. It's even more useful if we mark the kernel as
> tainted as you suggested above.
ok
>
> >
> > > +}
> > > +
> > > +DEFINE_DEBUGFS_ATTRIBUTE(cdns_hw_reset_fops, NULL, cdns_hw_reset, "%llu\n");
> > > +
> > > /**
> > > * sdw_cdns_debugfs_init() - Cadence debugfs init
> > > * @cdns: Cadence instance
> > > @@ -348,6 +365,9 @@ DEFINE_SHOW_ATTRIBUTE(cdns_reg);
> > > void sdw_cdns_debugfs_init(struct sdw_cdns *cdns, struct dentry *root)
> > > {
> > > debugfs_create_file("cdns-registers", 0400, root, cdns, &cdns_reg_fops);
> > > +
> > > + debugfs_create_file("cdns-hw-reset", 0200, root, cdns,
> > > + &cdns_hw_reset_fops);
> > > }
> > > EXPORT_SYMBOL_GPL(sdw_cdns_debugfs_init);
> > > --
> > > 2.20.1
> >
--
~Vinod
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
