[PATCH 1/5] em28xx: fix indenting in em28xx_usb_probe()

[PATCH 1/5] em28xx: fix indenting in em28xx_usb_probe()

[Frank Schäfer]

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-cards.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
index 50aa5a5..ff19833 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -3416,15 +3416,14 @@ static int em28xx_usb_probe(struct usb_interface *interface,
 
 	/* Select USB transfer types to use */
 	if (has_video) {
-	    if (!dev->analog_ep_isoc || (try_bulk && dev->analog_ep_bulk))
-		dev->analog_xfer_bulk = 1;
-	    em28xx_info("analog set to %s mode.\n",
-			dev->analog_xfer_bulk ? "bulk" : "isoc");
+		if (!dev->analog_ep_isoc || (try_bulk && dev->analog_ep_bulk))
+			dev->analog_xfer_bulk = 1;
+		em28xx_info("analog set to %s mode.\n",
+			    dev->analog_xfer_bulk ? "bulk" : "isoc");
 	}
 	if (has_dvb) {
-	    if (!dev->dvb_ep_isoc || (try_bulk && dev->dvb_ep_bulk))
-		dev->dvb_xfer_bulk = 1;
-
+		if (!dev->dvb_ep_isoc || (try_bulk && dev->dvb_ep_bulk))
+			dev->dvb_xfer_bulk = 1;
 		em28xx_info("dvb set to %s mode.\n",
 			    dev->dvb_xfer_bulk ? "bulk" : "isoc");
 	}
-- 
1.8.4.5

[PATCH 2/5] em28xx: fix i2c_set_adapdata() call in em28xx_i2c_register()

[Frank Schäfer]

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-i2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c
index ba6433c..04e8577 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -939,7 +939,7 @@ int em28xx_i2c_register(struct em28xx *dev, unsigned bus,
 	dev->i2c_bus[bus].algo_type = algo_type;
 	dev->i2c_bus[bus].dev = dev;
 	dev->i2c_adap[bus].algo_data = &dev->i2c_bus[bus];
-	i2c_set_adapdata(&dev->i2c_adap[bus], &dev->v4l2_dev);
+	i2c_set_adapdata(&dev->i2c_adap[bus], dev);
 
 	retval = i2c_add_adapter(&dev->i2c_adap[bus]);
 	if (retval < 0) {
-- 
1.8.4.5

Re: [PATCH 2/5] em28xx: fix i2c_set_adapdata() call in em28xx_i2c_register()

[Hans Verkuil]

Hi Frank,

I've got a comment about this patch:

On 03/22/2014 02:01 PM, Frank Schäfer wrote:
> Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
> ---
>  drivers/media/usb/em28xx/em28xx-i2c.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c
> index ba6433c..04e8577 100644
> --- a/drivers/media/usb/em28xx/em28xx-i2c.c
> +++ b/drivers/media/usb/em28xx/em28xx-i2c.c
> @@ -939,7 +939,7 @@ int em28xx_i2c_register(struct em28xx *dev, unsigned bus,
>  	dev->i2c_bus[bus].algo_type = algo_type;
>  	dev->i2c_bus[bus].dev = dev;
>  	dev->i2c_adap[bus].algo_data = &dev->i2c_bus[bus];
> -	i2c_set_adapdata(&dev->i2c_adap[bus], &dev->v4l2_dev);
> +	i2c_set_adapdata(&dev->i2c_adap[bus], dev);

As far as I can see nobody is calling i2c_get_adapdata. Should this line be removed
altogether?

If it is used somewhere, can you point me that?

I'm taking the other patches from this series (using the v2 version of patch 4/5) since
those look fine.

Regards,

	Hans

>  
>  	retval = i2c_add_adapter(&dev->i2c_adap[bus]);
>  	if (retval < 0) {
> 

Re: [PATCH 2/5] em28xx: fix i2c_set_adapdata() call in em28xx_i2c_register()

[Frank Schäfer]

Hi Hans,

Am 09.05.2014 10:40, schrieb Hans Verkuil:
> Hi Frank,
>
> I've got a comment about this patch:
>
> On 03/22/2014 02:01 PM, Frank Schäfer wrote:
>> Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
>> ---
>>  drivers/media/usb/em28xx/em28xx-i2c.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c
>> index ba6433c..04e8577 100644
>> --- a/drivers/media/usb/em28xx/em28xx-i2c.c
>> +++ b/drivers/media/usb/em28xx/em28xx-i2c.c
>> @@ -939,7 +939,7 @@ int em28xx_i2c_register(struct em28xx *dev, unsigned bus,
>>  	dev->i2c_bus[bus].algo_type = algo_type;
>>  	dev->i2c_bus[bus].dev = dev;
>>  	dev->i2c_adap[bus].algo_data = &dev->i2c_bus[bus];
>> -	i2c_set_adapdata(&dev->i2c_adap[bus], &dev->v4l2_dev);
>> +	i2c_set_adapdata(&dev->i2c_adap[bus], dev);
> As far as I can see nobody is calling i2c_get_adapdata. Should this line be removed
> altogether?
>
> If it is used somewhere, can you point me that?
Good catch.
Indeed, nobody is using it anymore so it can removed instead.
Drop this patch, I will send a new one in a minute.

> I'm taking the other patches from this series (using the v2 version of patch 4/5) since
> those look fine.
Thanks !

Regards,
Frank


>
> Regards,
>
> 	Hans
>
>>  
>>  	retval = i2c_add_adapter(&dev->i2c_adap[bus]);
>>  	if (retval < 0) {
>>

[PATCH 3/5] em28xx: remove some unused fields from struct em28xx

[Frank Schäfer]

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx.h | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index 2051fc9..e95f4eb 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -618,7 +618,6 @@ struct em28xx {
 	unsigned hscale;	/* horizontal scale factor (see datasheet) */
 	unsigned vscale;	/* vertical scale factor (see datasheet) */
 	int interlaced;		/* 1=interlace fileds, 0=just top fileds */
-	unsigned int video_bytesread;	/* Number of bytes read */
 
 	unsigned long hash;	/* eeprom hash - for boards with generic ID */
 	unsigned long i2c_hash;	/* i2c devicelist hash -
@@ -638,8 +637,6 @@ struct em28xx {
 	/* locks */
 	struct mutex lock;
 	struct mutex ctrl_urb_lock;	/* protects urb_buf */
-	/* spinlock_t queue_lock; */
-	struct list_head inqueue, outqueue;
 	struct video_device *vbi_dev;
 	struct video_device *radio_dev;
 
@@ -663,7 +660,6 @@ struct em28xx {
 	spinlock_t slock;
 
 	unsigned int field_count;
-	unsigned int vbi_field_count;
 
 	/* usb transfer */
 	struct usb_device *udev;	/* the usb device */
-- 
1.8.4.5

[PATCH 4/5] em28xx: remove function em28xx_compression_disable() and its call

[Frank Schäfer]

em28xx_compression_disable() is a single line function which is called only one
time and this call also isn't needed.
Register 0x26 is always configured as part of the scaler configuration, which
in turn is always done when the resolution changes. And the initial resolution
setting is applied at first device open.

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-video.c | 1 -
 drivers/media/usb/em28xx/em28xx.h       | 6 ------
 2 files changed, 7 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 0856e5d..e15ac75 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -2376,7 +2376,6 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 			 (EM28XX_XCLK_AUDIO_UNMUTE | val));
 
 	em28xx_set_outfmt(dev);
-	em28xx_compression_disable(dev);
 
 	/* Add image controls */
 	/* NOTE: at this point, the subdevices are already registered, so bridge
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index e95f4eb..dd6190c 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -793,12 +793,6 @@ int em28xx_init_camera(struct em28xx *dev);
 	printk(KERN_WARNING "%s: "fmt,\
 			dev->name , ##arg); } while (0)
 
-static inline int em28xx_compression_disable(struct em28xx *dev)
-{
-	/* side effect of disabling scaler and mixer */
-	return em28xx_write_reg(dev, EM28XX_R26_COMPR, 0x00);
-}
-
 /*FIXME: maxw should be dependent of alt mode */
 static inline unsigned int norm_maxw(struct em28xx *dev)
 {
-- 
1.8.4.5

Re: [PATCH 4/5] em28xx: remove function em28xx_compression_disable() and its call

[Frank Schäfer]

Am 22.03.2014 14:01, schrieb Frank Schäfer:
> em28xx_compression_disable() is a single line function which is called only one
> time and this call also isn't needed.
> Register 0x26 is always configured as part of the scaler configuration, which
> in turn is always done when the resolution changes. And the initial resolution
> setting is applied at first device open.
>
> Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
> ---
>  drivers/media/usb/em28xx/em28xx-video.c | 1 -
>  drivers/media/usb/em28xx/em28xx.h       | 6 ------
>  2 files changed, 7 deletions(-)
>
> diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
> index 0856e5d..e15ac75 100644
> --- a/drivers/media/usb/em28xx/em28xx-video.c
> +++ b/drivers/media/usb/em28xx/em28xx-video.c
> @@ -2376,7 +2376,6 @@ static int em28xx_v4l2_init(struct em28xx *dev)
>  			 (EM28XX_XCLK_AUDIO_UNMUTE | val));
>  
>  	em28xx_set_outfmt(dev);
> -	em28xx_compression_disable(dev);
>  
>  	/* Add image controls */
>  	/* NOTE: at this point, the subdevices are already registered, so bridge
> diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
> index e95f4eb..dd6190c 100644
> --- a/drivers/media/usb/em28xx/em28xx.h
> +++ b/drivers/media/usb/em28xx/em28xx.h
> @@ -793,12 +793,6 @@ int em28xx_init_camera(struct em28xx *dev);
>  	printk(KERN_WARNING "%s: "fmt,\
>  			dev->name , ##arg); } while (0)
>  
> -static inline int em28xx_compression_disable(struct em28xx *dev)
> -{
> -	/* side effect of disabling scaler and mixer */
> -	return em28xx_write_reg(dev, EM28XX_R26_COMPR, 0x00);
> -}
> -
>  /*FIXME: maxw should be dependent of alt mode */
>  static inline unsigned int norm_maxw(struct em28xx *dev)
>  {

Please disregard this patch, I will send an updated version in a minute.

[PATCH 5/5] em28xx: move norm_maxw() and norm_maxh() from em28xx.h to em28xx-video.c

[Frank Schäfer]

Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
 drivers/media/usb/em28xx/em28xx-video.c | 23 +++++++++++++++++++++++
 drivers/media/usb/em28xx/em28xx.h       | 22 ----------------------
 2 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index e15ac75..ac499ae 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -141,6 +141,29 @@ static struct em28xx_fmt format[] = {
 	},
 };
 
+/*FIXME: maxw should be dependent of alt mode */
+static inline unsigned int norm_maxw(struct em28xx *dev)
+{
+	if (dev->board.is_webcam)
+		return dev->sensor_xres;
+
+	if (dev->board.max_range_640_480)
+		return 640;
+
+	return 720;
+}
+
+static inline unsigned int norm_maxh(struct em28xx *dev)
+{
+	if (dev->board.is_webcam)
+		return dev->sensor_yres;
+
+	if (dev->board.max_range_640_480)
+		return 480;
+
+	return (dev->norm & V4L2_STD_625_50) ? 576 : 480;
+}
+
 static int em28xx_vbi_supported(struct em28xx *dev)
 {
 	/* Modprobe option to manually disable */
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index dd6190c..4beb1fa 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -793,26 +793,4 @@ int em28xx_init_camera(struct em28xx *dev);
 	printk(KERN_WARNING "%s: "fmt,\
 			dev->name , ##arg); } while (0)
 
-/*FIXME: maxw should be dependent of alt mode */
-static inline unsigned int norm_maxw(struct em28xx *dev)
-{
-	if (dev->board.is_webcam)
-		return dev->sensor_xres;
-
-	if (dev->board.max_range_640_480)
-		return 640;
-
-	return 720;
-}
-
-static inline unsigned int norm_maxh(struct em28xx *dev)
-{
-	if (dev->board.is_webcam)
-		return dev->sensor_yres;
-
-	if (dev->board.max_range_640_480)
-		return 480;
-
-	return (dev->norm & V4L2_STD_625_50) ? 576 : 480;
-}
 #endif
-- 
1.8.4.5