Re: [PATCH] dma: cpp41: Fix handling of error path

[Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>]

* Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> [161116 06:55]:
> * Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org> [161115 22:25]:
> > On Wednesday 16 November 2016 02:28 AM, Tony Lindgren wrote:
> > > * Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org> [161115 00:35]:
> > >> If pm_runtime_get_sync() fails due to callback error, then
> > >> rpm_callback() sets dev.power.runtime_error to an error value which gets
> > >> cleared by an explicit call to pm_runtime_set_suspended().
> > >>
> > >> This will tell the framework that the status of device is suspended.
> > >> Else, the failure will be sticky and on subsequent attempts,
> > >> rpm_resume() will keep returning early instead of trying to resume the
> > >> device again.
> > >>
> > >> This is as far as I can gather from code. So, I believe the recovery
> > >> path should be:
> > >>
> > >> 	if (error < 0) {
> > >> 		pm_runtime_set_suspended(cdd->ddev.dev);
> > >> 		pm_runtime_put_noidle(cdd->ddev.dev);
> > >>
> > >> 		...
> > > 
> > > Well we should test this :)
> > 
> > Yes, right! Was this patch created to fix an error in practice or just
> > based on code review?
> 
> Based on code review for related musb fixes. I'll test the above today
> at some point.

OK so adding pm_runtime_set_suspended() allows retries, but it also seems
dangerous as it clears dev.power.runtime_error. I think we're better of
not having cppi41 work at all on errors which now happens. Otherwise some
errors could just get ignored and we may even risk corrupting data.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html