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
