Re: [PATCH 09/13] soundwire: intel: add CLK_STOP_BUS_RESET support

[Vinod Koul <vkoul@kernel.org>]

On 17-08-20, 09:30, Pierre-Louis Bossart wrote:
> 
> 
> 
> > > +	} else if (clock_stop_quirks & SDW_INTEL_CLK_STOP_BUS_RESET) {
> > > +		ret = sdw_cdns_clock_stop(cdns, true);
> > > +		if (ret < 0) {
> > > +			dev_err(dev, "cannot enable clock stop on suspend\n");
> > > +			return ret;
> > > +		}
> > > +
> > > +		ret = sdw_cdns_enable_interrupt(cdns, false);
> > > +		if (ret < 0) {
> > > +			dev_err(dev, "cannot disable interrupts on suspend\n");
> > > +			return ret;
> > > +		}
> > > +
> > > +		ret = intel_link_power_down(sdw);
> > > +		if (ret) {
> > > +			dev_err(dev, "Link power down failed: %d", ret);
> > > +			return ret;
> > > +		}
> > 
> > no cleanup on all the error cases here?
> 
> See above the 'else if' test, the clock stop on suspend will be followed by
> a bus reset on resume. this is essentially a complete bus restart.

ok

> The only open here is whether we should actually return an error while
> suspending, or just log the error and squelch it. We decided to return the
> status so that the pm_runtime suspend does not proceed: the state remains
> active which is easier to detect than a single line in a dmesg log.

right, returning makes sense and is done correctly above

-- 
~Vinod