Hi Shuah,
Em Mon, 11 Aug 2014 21:13:55 -0600
Shuah Khan <shuah.kh@xxxxxxxxxxx> escreveu:
> au0828 doesn't resume correctly and TV tuning fails with
> xc_set_signal_source(0) failed message. Change au0828 dvb
> suspend and resume interfaces to suspend and resume frontend
> during suspend and resume respectively. au0828_dvb_suspend()
> calls dvb_frontend_suspend() which in turn invokes tuner ops
> sleep followed by fe ops sleep. au0828_dvb_resume() calls
> dvb_frontend_resume() which in turn calls fe ops ini follwed
> by tuner ops ini before waking up the frontend. With this change
> HVR950Q suspend and resume work when system gets suspended when
> digital function is tuned to a channel and with active TV stream,
> and after resume it went right back to active TV stream.
>
> Signed-off-by: Shuah Khan <shuah.kh@xxxxxxxxxxx>
> ---
> drivers/media/usb/au0828/au0828-dvb.c | 37 ++++++++++++++-------------------
> 1 file changed, 16 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/media/usb/au0828/au0828-dvb.c b/drivers/media/usb/au0828/au0828-dvb.c
> index 821f86e..50e7c82 100644
> --- a/drivers/media/usb/au0828/au0828-dvb.c
> +++ b/drivers/media/usb/au0828/au0828-dvb.c
> @@ -619,35 +619,30 @@ int au0828_dvb_register(struct au0828_dev *dev)
>
> void au0828_dvb_suspend(struct au0828_dev *dev)
> {
> - struct au0828_dvb *dvb = &dev->dvb;
> -
> - if (dvb->frontend && dev->urb_streaming) {
> - pr_info("stopping DVB\n");
> + struct au0828_dvb *dvb;
> + int rc;
>
> - cancel_work_sync(&dev->restart_streaming);
The restart streaming delayed work still needs to be canceled if
active. This work can be active while DVB is streaming, to workaround
on a chipset bug.
The best is to first call cancel the pending URBs and stop_urb_transfer(),
in order to avoid race conditions.
> + if (dev == NULL)
> + return;
In this specific device driver, I think that this condition will never
happen:
$ git grep "dev = NULL" drivers/media/usb/au0828/au0828*
drivers/media/usb/au0828/au0828-core.c: dev->usbdev = NULL;
>
> - /* Stop transport */
> - mutex_lock(&dvb->lock);
> - stop_urb_transfer(dev);
My understanding is that we still need to cancel the pending URBs before
suspending, in order to stop the DMA engine.
> - au0828_stop_transport(dev, 1);
> - mutex_unlock(&dvb->lock);
> - dev->need_urb_start = 1;
> + dvb = &dev->dvb;
> + if (dvb->frontend) {
> + rc = dvb_frontend_suspend(dvb->frontend);
> + pr_info("au0828_dvb_suspend(): Suspending DVB fe %d\n", rc);
> }
> }
>
> void au0828_dvb_resume(struct au0828_dev *dev)
> {
> - struct au0828_dvb *dvb = &dev->dvb;
> -
> - if (dvb->frontend && dev->need_urb_start) {
> - pr_info("resuming DVB\n");
> + struct au0828_dvb *dvb;
> + int rc;
>
> - au0828_set_frontend(dvb->frontend);
> + if (dev == NULL)
> + return;
>
> - /* Start transport */
> - mutex_lock(&dvb->lock);
> - au0828_start_transport(dev);
> - start_urb_transfer(dev);
> - mutex_unlock(&dvb->lock);
As we need to stop pending URB transfers and stop the DMA engine,
we still need the above code. We don't need to restart a pending
work, because what the pending work would be doing is to stop and
restart the transfer, and the suspend/resume code should do it already.
> + dvb = &dev->dvb;
> + if (dvb->frontend) {
> + rc = dvb_frontend_resume(dvb->frontend);
> + pr_info("au0828_dvb_resume(): Resuming DVB fe %d\n", rc);
> }
> }
Regards,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
