public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] media: radio: Critical v4l2 registration bugfix for si470x over i2c
@ 2018-01-18 20:01 Douglas Fischer
  0 siblings, 0 replies; 3+ messages in thread
From: Douglas Fischer @ 2018-01-18 20:01 UTC (permalink / raw)
  To: linux-media

Added the call to v4l2_device_register() required to add a new radio
device. Without this patch, it is impossible for the driver to load.
This does not affect USB devices.

Signed-off-by: Douglas Fischer <fischerdouglasc@gmail.com>
---

diff -uprN linux.orig/drivers/media/radio/si470x/radio-si470x-i2c.c
linux/drivers/media/radio/si470x/radio-si470x-i2c.c ---
linux.orig/drivers/media/radio/si470x/radio-si470x-i2c.c
2018-01-15 21:58:10.675620432 -0500 +++
linux/drivers/media/radio/si470x/radio-si470x-i2c.c	2018-01-16
17:08:02.929734342 -0500 @@ -43,7 +43,6 @@ static const struct
i2c_device_id si470x MODULE_DEVICE_TABLE(i2c, si470x_i2c_id); 
-
 /**************************************************************************
  * Module Parameters
  **************************************************************************/
@@ -362,8 +361,29 @@ static int si470x_i2c_probe(struct i2c_c
 	mutex_init(&radio->lock);
 	init_completion(&radio->completion);
 
+	retval = v4l2_device_register(&client->dev, &radio->v4l2_dev);
+	if (retval < 0) {
+		dev_err(&client->dev, "couldn't register
v4l2_device\n");
+		goto err_initial;
+	}
+
+	v4l2_ctrl_handler_init(&radio->hdl, 2);
+	v4l2_ctrl_new_std(&radio->hdl, &si470x_ctrl_ops,
+			V4L2_CID_AUDIO_MUTE, 0, 1, 1, 1);
+	v4l2_ctrl_new_std(&radio->hdl, &si470x_ctrl_ops,
+			V4L2_CID_AUDIO_VOLUME, 0, 15, 1, 15);
+	if (radio->hdl.error) {
+		retval = radio->hdl.error;
+		dev_err(&client->dev, "couldn't register control\n");
+		goto err_dev;
+	}
+
 	/* video device initialization */
 	radio->videodev = si470x_viddev_template;
+	radio->videodev.ctrl_handler =
&radio->hdl;				// no?
+	radio->videodev.lock =
&radio->lock;					// no?
+	radio->videodev.v4l2_dev = &radio->v4l2_dev;
+	radio->videodev.release = video_device_release_empty;
 	video_set_drvdata(&radio->videodev, radio);
 
 	/* power up : need 110ms */
@@ -435,6 +455,8 @@ static int si470x_i2c_probe(struct i2c_c
 	return 0;
 err_all:
 	free_irq(client->irq, radio);
+err_dev:
+	v4l2_device_unregister(&radio->v4l2_dev);
 err_rds:
 	kfree(radio->buffer);
 err_radio:

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH] media: radio: Critical v4l2 registration bugfix for si470x over i2c
@ 2018-01-20 19:19 Douglas Fischer
  2018-02-15 14:31 ` Hans Verkuil
  0 siblings, 1 reply; 3+ messages in thread
From: Douglas Fischer @ 2018-01-20 19:19 UTC (permalink / raw)
  To: hverkuil, linux-media

Added the call to v4l2_device_register() required to add a new radio
device. Without this patch, it is impossible for the driver to load.
This does not affect USB devices.

Signed-off-by: Douglas Fischer <fischerdouglasc@gmail.com>
---

diff -uprN linux.orig/drivers/media/radio/si470x/radio-si470x-i2c.c
linux/drivers/media/radio/si470x/radio-si470x-i2c.c ---
linux.orig/drivers/media/radio/si470x/radio-si470x-i2c.c
2018-01-15 21:58:10.675620432 -0500 +++
linux/drivers/media/radio/si470x/radio-si470x-i2c.c	2018-01-16
17:08:02.929734342 -0500 @@ -43,7 +43,6 @@ static const struct
i2c_device_id si470x MODULE_DEVICE_TABLE(i2c, si470x_i2c_id); 
-
 /**************************************************************************
  * Module Parameters
  **************************************************************************/
@@ -362,8 +361,29 @@ static int si470x_i2c_probe(struct i2c_c
 	mutex_init(&radio->lock);
 	init_completion(&radio->completion);
 
+	retval = v4l2_device_register(&client->dev, &radio->v4l2_dev);
+	if (retval < 0) {
+		dev_err(&client->dev, "couldn't register
v4l2_device\n");
+		goto err_initial;
+	}
+
+	v4l2_ctrl_handler_init(&radio->hdl, 2);
+	v4l2_ctrl_new_std(&radio->hdl, &si470x_ctrl_ops,
+			V4L2_CID_AUDIO_MUTE, 0, 1, 1, 1);
+	v4l2_ctrl_new_std(&radio->hdl, &si470x_ctrl_ops,
+			V4L2_CID_AUDIO_VOLUME, 0, 15, 1, 15);
+	if (radio->hdl.error) {
+		retval = radio->hdl.error;
+		dev_err(&client->dev, "couldn't register control\n");
+		goto err_dev;
+	}
+
 	/* video device initialization */
 	radio->videodev = si470x_viddev_template;
+	radio->videodev.ctrl_handler =
&radio->hdl;				// no?
+	radio->videodev.lock =
&radio->lock;					// no?
+	radio->videodev.v4l2_dev = &radio->v4l2_dev;
+	radio->videodev.release = video_device_release_empty;
 	video_set_drvdata(&radio->videodev, radio);
 
 	/* power up : need 110ms */
@@ -435,6 +455,8 @@ static int si470x_i2c_probe(struct i2c_c
 	return 0;
 err_all:
 	free_irq(client->irq, radio);
+err_dev:
+	v4l2_device_unregister(&radio->v4l2_dev);
 err_rds:
 	kfree(radio->buffer);
 err_radio:

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] media: radio: Critical v4l2 registration bugfix for si470x over i2c
  2018-01-20 19:19 Douglas Fischer
@ 2018-02-15 14:31 ` Hans Verkuil
  0 siblings, 0 replies; 3+ messages in thread
From: Hans Verkuil @ 2018-02-15 14:31 UTC (permalink / raw)
  To: Douglas Fischer, linux-media

Hi Douglas,

On 20/01/18 20:19, Douglas Fischer wrote:
> Added the call to v4l2_device_register() required to add a new radio
> device. Without this patch, it is impossible for the driver to load.
> This does not affect USB devices.
> 
> Signed-off-by: Douglas Fischer <fischerdouglasc@gmail.com>
> ---
> 
> diff -uprN linux.orig/drivers/media/radio/si470x/radio-si470x-i2c.c
> linux/drivers/media/radio/si470x/radio-si470x-i2c.c ---
> linux.orig/drivers/media/radio/si470x/radio-si470x-i2c.c
> 2018-01-15 21:58:10.675620432 -0500 +++
> linux/drivers/media/radio/si470x/radio-si470x-i2c.c	2018-01-16
> 17:08:02.929734342 -0500 @@ -43,7 +43,6 @@ static const struct
> i2c_device_id si470x MODULE_DEVICE_TABLE(i2c, si470x_i2c_id); 
> -
>  /**************************************************************************
>   * Module Parameters
>   **************************************************************************/
> @@ -362,8 +361,29 @@ static int si470x_i2c_probe(struct i2c_c
>  	mutex_init(&radio->lock);
>  	init_completion(&radio->completion);
>  
> +	retval = v4l2_device_register(&client->dev, &radio->v4l2_dev);
> +	if (retval < 0) {
> +		dev_err(&client->dev, "couldn't register
> v4l2_device\n");
> +		goto err_initial;
> +	}
> +
> +	v4l2_ctrl_handler_init(&radio->hdl, 2);
> +	v4l2_ctrl_new_std(&radio->hdl, &si470x_ctrl_ops,
> +			V4L2_CID_AUDIO_MUTE, 0, 1, 1, 1);
> +	v4l2_ctrl_new_std(&radio->hdl, &si470x_ctrl_ops,
> +			V4L2_CID_AUDIO_VOLUME, 0, 15, 1, 15);
> +	if (radio->hdl.error) {
> +		retval = radio->hdl.error;
> +		dev_err(&client->dev, "couldn't register control\n");
> +		goto err_dev;
> +	}

You don't explain in the commit log why this is added as well.

> +
>  	/* video device initialization */
>  	radio->videodev = si470x_viddev_template;
> +	radio->videodev.ctrl_handler =
> &radio->hdl;				// no?
> +	radio->videodev.lock =
> &radio->lock;					// no?

'no?' what?

> +	radio->videodev.v4l2_dev = &radio->v4l2_dev;
> +	radio->videodev.release = video_device_release_empty;
>  	video_set_drvdata(&radio->videodev, radio);
>  
>  	/* power up : need 110ms */
> @@ -435,6 +455,8 @@ static int si470x_i2c_probe(struct i2c_c
>  	return 0;
>  err_all:
>  	free_irq(client->irq, radio);
> +err_dev:
> +	v4l2_device_unregister(&radio->v4l2_dev);

This can't be right. radio->buffer is allocated after the v4l2_device, so the
kfree for that should come before err_dev.

Please double check the cleanup order.

>  err_rds:
>  	kfree(radio->buffer);
>  err_radio:
> 

Regards,

	Hans

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-02-15 14:31 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-01-18 20:01 [PATCH] media: radio: Critical v4l2 registration bugfix for si470x over i2c Douglas Fischer
  -- strict thread matches above, loose matches on Subject: below --
2018-01-20 19:19 Douglas Fischer
2018-02-15 14:31 ` Hans Verkuil

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox