From: Hans Verkuil <hans.verkuil@xxxxxxxxx>
The radio-si470x-usb driver supported both autosuspend and it stopped the
radio the moment the last user of the radio device closed it. However, that
was very confusing since if you play the audio from the device (e.g. through
arecord -D ... | aplay) then no sound would play unless you had the radio
device open at the same time, even though there is no need to do anything
with that node.
On the other hand, the actual suspend/resume functions didn't do anything,
which would fail if you *did* have the radio node open at that time.
So:
- remove autosuspend (bad idea in general for USB radio devices)
- move the start/stop out of the open/release functions into the resume/suspend
functions.
Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
---
drivers/media/radio/si470x/radio-si470x-common.c | 1 -
drivers/media/radio/si470x/radio-si470x-usb.c | 149 ++++++++++------------
2 files changed, 70 insertions(+), 80 deletions(-)
diff --git a/drivers/media/radio/si470x/radio-si470x-common.c b/drivers/media/radio/si470x/radio-si470x-common.c
index b9a44d4..969cf49 100644
--- a/drivers/media/radio/si470x/radio-si470x-common.c
+++ b/drivers/media/radio/si470x/radio-si470x-common.c
@@ -570,7 +570,6 @@ static int si470x_s_ctrl(struct v4l2_ctrl *ctrl)
else
radio->registers[POWERCFG] |= POWERCFG_DMUTE;
return si470x_set_register(radio, POWERCFG);
- break;
default:
return -EINVAL;
}
diff --git a/drivers/media/radio/si470x/radio-si470x-usb.c b/drivers/media/radio/si470x/radio-si470x-usb.c
index f133c3d..e9f6387 100644
--- a/drivers/media/radio/si470x/radio-si470x-usb.c
+++ b/drivers/media/radio/si470x/radio-si470x-usb.c
@@ -481,91 +481,20 @@ resubmit:
}
-
-/**************************************************************************
- * File Operations Interface
- **************************************************************************/
-
-/*
- * si470x_fops_open - file open
- */
int si470x_fops_open(struct file *file)
{
- struct si470x_device *radio = video_drvdata(file);
- int retval = v4l2_fh_open(file);
-
- if (retval)
- return retval;
-
- retval = usb_autopm_get_interface(radio->intf);
- if (retval < 0)
- goto done;
-
- if (v4l2_fh_is_singular_file(file)) {
- /* start radio */
- retval = si470x_start(radio);
- if (retval < 0) {
- usb_autopm_put_interface(radio->intf);
- goto done;
- }
-
- /* initialize interrupt urb */
- usb_fill_int_urb(radio->int_in_urb, radio->usbdev,
- usb_rcvintpipe(radio->usbdev,
- radio->int_in_endpoint->bEndpointAddress),
- radio->int_in_buffer,
- le16_to_cpu(radio->int_in_endpoint->wMaxPacketSize),
- si470x_int_in_callback,
- radio,
- radio->int_in_endpoint->bInterval);
-
- radio->int_in_running = 1;
- mb();
-
- retval = usb_submit_urb(radio->int_in_urb, GFP_KERNEL);
- if (retval) {
- dev_info(&radio->intf->dev,
- "submitting int urb failed (%d)\n", retval);
- radio->int_in_running = 0;
- usb_autopm_put_interface(radio->intf);
- }
- }
-
-done:
- if (retval)
- v4l2_fh_release(file);
- return retval;
+ return v4l2_fh_open(file);
}
-
-/*
- * si470x_fops_release - file release
- */
int si470x_fops_release(struct file *file)
{
- struct si470x_device *radio = video_drvdata(file);
-
- if (v4l2_fh_is_singular_file(file)) {
- /* shutdown interrupt handler */
- if (radio->int_in_running) {
- radio->int_in_running = 0;
- if (radio->int_in_urb)
- usb_kill_urb(radio->int_in_urb);
- }
-
- /* cancel read processes */
- wake_up_interruptible(&radio->read_queue);
-
- /* stop radio */
- si470x_stop(radio);
- usb_autopm_put_interface(radio->intf);
- }
return v4l2_fh_release(file);
}
-static void si470x_usb_release(struct video_device *vdev)
+static void si470x_usb_release(struct v4l2_device *v4l2_dev)
{
- struct si470x_device *radio = video_get_drvdata(vdev);
+ struct si470x_device *radio =
+ container_of(v4l2_dev, struct si470x_device, v4l2_dev);
usb_free_urb(radio->int_in_urb);
v4l2_ctrl_handler_free(&radio->hdl);
@@ -599,6 +528,38 @@ int si470x_vidioc_querycap(struct file *file, void *priv,
}
+static int si470x_start_usb(struct si470x_device *radio)
+{
+ int retval;
+
+ /* start radio */
+ retval = si470x_start(radio);
+ if (retval < 0)
+ return retval;
+
+ v4l2_ctrl_handler_setup(&radio->hdl);
+
+ /* initialize interrupt urb */
+ usb_fill_int_urb(radio->int_in_urb, radio->usbdev,
+ usb_rcvintpipe(radio->usbdev,
+ radio->int_in_endpoint->bEndpointAddress),
+ radio->int_in_buffer,
+ le16_to_cpu(radio->int_in_endpoint->wMaxPacketSize),
+ si470x_int_in_callback,
+ radio,
+ radio->int_in_endpoint->bInterval);
+
+ radio->int_in_running = 1;
+ mb();
+
+ retval = usb_submit_urb(radio->int_in_urb, GFP_KERNEL);
+ if (retval) {
+ dev_info(&radio->intf->dev,
+ "submitting int urb failed (%d)\n", retval);
+ radio->int_in_running = 0;
+ }
+ return retval;
+}
/**************************************************************************
* USB Interface
@@ -658,6 +619,7 @@ static int si470x_usb_driver_probe(struct usb_interface *intf,
goto err_intbuffer;
}
+ radio->v4l2_dev.release = si470x_usb_release;
retval = v4l2_device_register(&intf->dev, &radio->v4l2_dev);
if (retval < 0) {
dev_err(&intf->dev, "couldn't register v4l2_device\n");
@@ -678,7 +640,7 @@ static int si470x_usb_driver_probe(struct usb_interface *intf,
radio->videodev.ctrl_handler = &radio->hdl;
radio->videodev.lock = &radio->lock;
radio->videodev.v4l2_dev = &radio->v4l2_dev;
- radio->videodev.release = si470x_usb_release;
+ radio->videodev.release = video_device_release_empty;
set_bit(V4L2_FL_USE_FH_PRIO, &radio->videodev.flags);
video_set_drvdata(&radio->videodev, radio);
@@ -754,11 +716,16 @@ static int si470x_usb_driver_probe(struct usb_interface *intf,
init_waitqueue_head(&radio->read_queue);
usb_set_intfdata(intf, radio);
+ /* start radio */
+ retval = si470x_start_usb(radio);
+ if (retval < 0)
+ goto err_all;
+
/* register video device */
retval = video_register_device(&radio->videodev, VFL_TYPE_RADIO,
radio_nr);
if (retval) {
- dev_warn(&intf->dev, "Could not register video device\n");
+ dev_err(&intf->dev, "Could not register video device\n");
goto err_all;
}
@@ -786,8 +753,22 @@ err_initial:
static int si470x_usb_driver_suspend(struct usb_interface *intf,
pm_message_t message)
{
+ struct si470x_device *radio = usb_get_intfdata(intf);
+
dev_info(&intf->dev, "suspending now...\n");
+ /* shutdown interrupt handler */
+ if (radio->int_in_running) {
+ radio->int_in_running = 0;
+ if (radio->int_in_urb)
+ usb_kill_urb(radio->int_in_urb);
+ }
+
+ /* cancel read processes */
+ wake_up_interruptible(&radio->read_queue);
+
+ /* stop radio */
+ si470x_stop(radio);
return 0;
}
@@ -797,9 +778,12 @@ static int si470x_usb_driver_suspend(struct usb_interface *intf,
*/
static int si470x_usb_driver_resume(struct usb_interface *intf)
{
+ struct si470x_device *radio = usb_get_intfdata(intf);
+
dev_info(&intf->dev, "resuming now...\n");
- return 0;
+ /* start radio */
+ return si470x_start_usb(radio);
}
@@ -815,11 +799,18 @@ static void si470x_usb_driver_disconnect(struct usb_interface *intf)
video_unregister_device(&radio->videodev);
usb_set_intfdata(intf, NULL);
mutex_unlock(&radio->lock);
+ v4l2_device_put(&radio->v4l2_dev);
}
/*
* si470x_usb_driver - usb driver interface
+ *
+ * A note on suspend/resume: this driver had only empty suspend/resume
+ * functions, and when I tried to test suspend/resume it always disconnected
+ * instead of resuming (using my ADS InstantFM stick). So I've decided to
+ * remove these callbacks until someone else with better hardware can
+ * implement and test this.
*/
static struct usb_driver si470x_usb_driver = {
.name = DRIVER_NAME,
@@ -827,8 +818,8 @@ static struct usb_driver si470x_usb_driver = {
.disconnect = si470x_usb_driver_disconnect,
.suspend = si470x_usb_driver_suspend,
.resume = si470x_usb_driver_resume,
+ .reset_resume = si470x_usb_driver_resume,
.id_table = si470x_usb_driver_id_table,
- .supports_autosuspend = 1,
};
module_usb_driver(si470x_usb_driver);
--
1.7.10
--
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
