On 13-03-20, 12:07, Pierre-Louis Bossart wrote: > > > On 3/13/20 7:21 AM, Vinod Koul wrote: > > On 11-03-20, 13:41, Pierre-Louis Bossart wrote: > > > > > @@ -225,12 +225,30 @@ static int cdns_clear_bit(struct sdw_cdns *cdns, int offset, u32 value) > > > return 0; > > > timeout--; > > > - udelay(50); > > > + usleep_range(50, 100); > > > > this seems okay change, but unrelated to this patch > > ok. It doesn't really matter anyway, this function is removed in Patch 8 Ok pls drop from here. > > > +int sdw_cdns_clock_stop(struct sdw_cdns *cdns, bool block_wake) > > > +{ > > > + bool slave_present = false; > > > + struct sdw_slave *slave; > > > + u32 status; > > > + int ret; > > > + > > > + /* Check suspend status */ > > > + status = cdns_readl(cdns, CDNS_MCP_STAT); > > > + if (status & CDNS_MCP_STAT_CLK_STOP) { > > > + dev_dbg(cdns->dev, "Clock is already stopped\n"); > > > + return 1; > > > > return of 1..? Does that indicate success/fail..? > > success. I guess it could be moved as 0. That would be better. We use 0 for success everywhere and -ve error codes. -- ~Vinod