| Message ID | 20190813213227.5163-3-pierre-louis.bossart@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | soundwire: inits and PM additions for 5.4 | expand |
On 13-08-19, 16:32, 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@linux.intel.com> > --- > 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 046622e4b264..bd58d80ff636 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 0; Should this not be EINVAL to indicate invalid value passed? > + > + ret = sdw_cdns_exit_reset(cdns); > + > + dev_dbg(cdns->dev, "link hw_reset done: %d\n", ret); > + > + return ret; > +} > + > +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
On 9/4/19 2:13 AM, Vinod Koul wrote: > On 13-08-19, 16:32, 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@linux.intel.com> >> --- >> 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 046622e4b264..bd58d80ff636 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 0; > > Should this not be EINVAL to indicate invalid value passed? Maybe. I must admit I don't know what -EINVAL would do, this is used for debugfs so it's not clear to me if the user will see a difference? > >> + >> + ret = sdw_cdns_exit_reset(cdns); >> + >> + dev_dbg(cdns->dev, "link hw_reset done: %d\n", ret); >> + >> + return ret; >> +} >> + >> +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 >
On 04-09-19, 08:18, Pierre-Louis Bossart wrote: > On 9/4/19 2:13 AM, Vinod Koul wrote: > > On 13-08-19, 16:32, 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@linux.intel.com> > > > --- > > > 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 046622e4b264..bd58d80ff636 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 0; > > > > Should this not be EINVAL to indicate invalid value passed? > > Maybe. I must admit I don't know what -EINVAL would do, this is used for > debugfs so it's not clear to me if the user will see a difference? Well user should see "write error: Invalid argument" when he writes anything other than valid values :)
On 9/4/19 11:49 AM, Vinod Koul wrote: > On 04-09-19, 08:18, Pierre-Louis Bossart wrote: >> On 9/4/19 2:13 AM, Vinod Koul wrote: >>> On 13-08-19, 16:32, 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@linux.intel.com> >>>> --- >>>> 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 046622e4b264..bd58d80ff636 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 0; >>> >>> Should this not be EINVAL to indicate invalid value passed? >> >> Maybe. I must admit I don't know what -EINVAL would do, this is used for >> debugfs so it's not clear to me if the user will see a difference? > > Well user should see "write error: Invalid argument" when he writes > anything other than valid values :) ok then, will do.
diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index 046622e4b264..bd58d80ff636 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 0; + + ret = sdw_cdns_exit_reset(cdns); + + dev_dbg(cdns->dev, "link hw_reset done: %d\n", ret); + + return ret; +} + +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);
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@linux.intel.com> --- drivers/soundwire/cadence_master.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)