| Message ID | 20200311184128.4212-5-pierre-louis.bossart@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | SoundWire: cadence: add clock stop and fix programming sequences | expand |
On 11-03-20, 13:41, Pierre-Louis Bossart wrote: > config_update() may time out or cannot be use in ClockStopMode > > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> > --- > drivers/soundwire/cadence_master.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c > index 71cba2585151..4089c271305a 100644 > --- a/drivers/soundwire/cadence_master.c > +++ b/drivers/soundwire/cadence_master.c > @@ -239,6 +239,11 @@ static int cdns_config_update(struct sdw_cdns *cdns) > { > int ret; > > + if (sdw_cdns_is_clock_stop(cdns)) { > + dev_err(cdns->dev, "Cannot program MCP_CONFIG_UPDATE in ClockStopMode\n"); This looks fine but duplicates the log, so maybe you can remove from here or preceding patch... Or use single line as I suggested in that patch and keep this as is.
On 3/13/20 7:08 AM, Vinod Koul wrote: > On 11-03-20, 13:41, Pierre-Louis Bossart wrote: >> config_update() may time out or cannot be use in ClockStopMode >> >> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> >> --- >> drivers/soundwire/cadence_master.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c >> index 71cba2585151..4089c271305a 100644 >> --- a/drivers/soundwire/cadence_master.c >> +++ b/drivers/soundwire/cadence_master.c >> @@ -239,6 +239,11 @@ static int cdns_config_update(struct sdw_cdns *cdns) >> { >> int ret; >> >> + if (sdw_cdns_is_clock_stop(cdns)) { >> + dev_err(cdns->dev, "Cannot program MCP_CONFIG_UPDATE in ClockStopMode\n"); > > This looks fine but duplicates the log, so maybe you can remove from > here or preceding patch... Or use single line as I suggested in that > patch and keep this as is. It doesn't duplicate it. In Patch1 it's a dev_dbg log that's useful for integration, and that function is used in other places (see the soundwire:intel series). In this patch this is a clear error leading to a master hang.
diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c index 71cba2585151..4089c271305a 100644 --- a/drivers/soundwire/cadence_master.c +++ b/drivers/soundwire/cadence_master.c @@ -239,6 +239,11 @@ static int cdns_config_update(struct sdw_cdns *cdns) { int ret; + if (sdw_cdns_is_clock_stop(cdns)) { + dev_err(cdns->dev, "Cannot program MCP_CONFIG_UPDATE in ClockStopMode\n"); + return -EINVAL; + } + ret = cdns_clear_bit(cdns, CDNS_MCP_CONFIG_UPDATE, CDNS_MCP_CONFIG_UPDATE_BIT); if (ret < 0)
config_update() may time out or cannot be use in ClockStopMode Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> --- drivers/soundwire/cadence_master.c | 5 +++++ 1 file changed, 5 insertions(+)