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

From: Vinod Koul
Date: Tue Aug 18 2020 - 02:27:41 EST


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