| Message ID | 20190725234032.21152-38-pierre-louis.bossart@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | soundwire: updates for 5.4 | expand |
On Thu, Jul 25, 2019 at 06:40:29PM -0500, Pierre-Louis Bossart wrote: > This is to kick devices into reset and see what software does > > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> > --- > drivers/soundwire/cadence_master.c | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > > diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c > index fa7230b0f200..53278aa2436f 100644 > --- a/drivers/soundwire/cadence_master.c > +++ b/drivers/soundwire/cadence_master.c > @@ -331,6 +331,25 @@ static const struct file_operations cdns_reg_fops = { > .llseek = default_llseek, > }; > > +static int cdns_hw_reset(void *data, u64 value) > +{ > + struct sdw_cdns *cdns = data; > + int ret; > + > + if (value != 1) > + return 0; > + > + dev_info(cdns->dev, "starting link hw_reset\n"); > + > + ret = sdw_cdns_exit_reset(cdns); > + > + dev_info(cdns->dev, "link hw_reset done\n"); Do not be noisy for when things always go right. This looks like debuggging code, please remove. > + > + 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 > @@ -339,6 +358,9 @@ static const struct file_operations cdns_reg_fops = { > 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_unsafe("cdns-hw-reset", 0200, root, cdns, > + &cdns_hw_reset_fops); Why unsafe? thanks, greg k-h
Thanks for the review Greg. >> +static int cdns_hw_reset(void *data, u64 value) >> +{ >> + struct sdw_cdns *cdns = data; >> + int ret; >> + >> + if (value != 1) >> + return 0; >> + >> + dev_info(cdns->dev, "starting link hw_reset\n"); >> + >> + ret = sdw_cdns_exit_reset(cdns); >> + >> + dev_info(cdns->dev, "link hw_reset done\n"); > > Do not be noisy for when things always go right. This looks like > debuggging code, please remove. yes, missed this in the cleanup, will remove. >> +DEFINE_DEBUGFS_ATTRIBUTE(cdns_hw_reset_fops, NULL, cdns_hw_reset, "%llu\n"); >> + >> /** >> * sdw_cdns_debugfs_init() - Cadence debugfs init >> * @cdns: Cadence instance >> @@ -339,6 +358,9 @@ static const struct file_operations cdns_reg_fops = { >> 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_unsafe("cdns-hw-reset", 0200, root, cdns, >> + &cdns_hw_reset_fops); > > Why unsafe? Dunno. I followed the documentation and my take-away, along with a number of examples, was to use _unsafe. I really have no idea if this is correct or not, I can remove this qualifier if that's not needed.
On Thu, Jul 25, 2019 at 06:40:29PM -0500, Pierre-Louis Bossart wrote: > This is to kick devices into reset and see what software does > > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> > --- > drivers/soundwire/cadence_master.c | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > > diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c > index fa7230b0f200..53278aa2436f 100644 > --- a/drivers/soundwire/cadence_master.c > +++ b/drivers/soundwire/cadence_master.c > @@ -331,6 +331,25 @@ static const struct file_operations cdns_reg_fops = { > .llseek = default_llseek, > }; > > +static int cdns_hw_reset(void *data, u64 value) > +{ > + struct sdw_cdns *cdns = data; > + int ret; > + > + if (value != 1) > + return 0; > + > + dev_info(cdns->dev, "starting link hw_reset\n"); > + > + ret = sdw_cdns_exit_reset(cdns); > + > + dev_info(cdns->dev, "link hw_reset done\n"); Both really should be dev_info()? Maybe at least one of them can be dev_dbg()? Thanks Guennadi > + > + 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 > @@ -339,6 +358,9 @@ static const struct file_operations cdns_reg_fops = { > 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_unsafe("cdns-hw-reset", 0200, root, cdns, > + &cdns_hw_reset_fops); > } > EXPORT_SYMBOL_GPL(sdw_cdns_debugfs_init); > > -- > 2.20.1 > > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>> +static int cdns_hw_reset(void *data, u64 value) >> +{ >> + struct sdw_cdns *cdns = data; >> + int ret; >> + >> + if (value != 1) >> + return 0; >> + >> + dev_info(cdns->dev, "starting link hw_reset\n"); >> + >> + ret = sdw_cdns_exit_reset(cdns); >> + >> + dev_info(cdns->dev, "link hw_reset done\n"); > > Both really should be dev_info()? Maybe at least one of them can be dev_dbg()? I have to walk back on what I explained to Greg. The idea was to have a dmesg trace when this function as called when the user plays with debugfs, otherwise the dmesg log is difficult to interpret (devices can go off the bus on their own). I'll keep the first one only and demote it to dev_dbg.
diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index fa7230b0f200..53278aa2436f 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -331,6 +331,25 @@ static const struct file_operations cdns_reg_fops = { .llseek = default_llseek, }; +static int cdns_hw_reset(void *data, u64 value) +{ + struct sdw_cdns *cdns = data; + int ret; + + if (value != 1) + return 0; + + dev_info(cdns->dev, "starting link hw_reset\n"); + + ret = sdw_cdns_exit_reset(cdns); + + dev_info(cdns->dev, "link hw_reset done\n"); + + 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 @@ -339,6 +358,9 @@ static const struct file_operations cdns_reg_fops = { 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_unsafe("cdns-hw-reset", 0200, root, cdns, + &cdns_hw_reset_fops); } EXPORT_SYMBOL_GPL(sdw_cdns_debugfs_init);
This is to kick devices into reset and see what software does Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> --- drivers/soundwire/cadence_master.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)