em28xx: fix indenting in em28xx_usb_probe()

PATCH In review Frank Schäfer 2 participants 8 messages Mar 22, 2014 – May 11, 2014

Subsystems (2)

Media (V4L/DVB) EM28XX VIDEO4LINUX driver

lore

Share & tools

lore

Frank Schäfer

author

2014-03-22 13:00 UTC #0 lore raw

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

Signed-off-by Frank Schäfer <[email protected]>

---
 drivers/media/usb/em28xx/em28xx-cards.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

drivers/media/usb/em28xx/em28xx-cards.c +6 -7

index 50aa5a5..ff19833 100644
@@ -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");
 	}

Signature · 1 line

1.8.4.5

Frank Schäfer

author

2014-03-22 13:00 UTC #1 lore raw

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

Signed-off-by Frank Schäfer <[email protected]>

---
 drivers/media/usb/em28xx/em28xx-i2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/media/usb/em28xx/em28xx-i2c.c +1 -1

index ba6433c..04e8577 100644
@@ -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) {

Signature · 1 line

1.8.4.5

Hans Verkuil

2014-05-09 08:41 UTC #2 lore raw

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

Hi Frank,

I've got a comment about this patch:

Quoted 10 lines · Frank Schäfer

Signed-off-by Frank Schäfer <[email protected]>
---
 drivers/media/usb/em28xx/em28xx-i2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/media/usb/em28xx/em28xx-i2c.c +0 -0
index ba6433c..04e8577 100644

drivers/media/usb/em28xx/em28xx-i2c.c int em28xx_i2c_register(struct em28xx *dev, unsigned bus, · line 942

Quoted 5 lines

drivers/media/usb/em28xx/em28xx-i2c.c +1 -1

@@ -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

Quoted 4 lines

drivers/media/usb/em28xx/em28xx-i2c.c +0 -0

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

Frank Schäfer

author

2014-05-11 20:25 UTC #3 lore raw

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

Hi Hans,

Am 09.05.2014 10:40, schrieb Hans Verkuil:

Quoted 23 lines

Hi Frank,
I've got a comment about this patch:
Quoted 10 lines · Frank Schäfer
Signed-off-by Frank Schäfer <[email protected]>
---
 drivers/media/usb/em28xx/em28xx-i2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/media/usb/em28xx/em28xx-i2c.c +0 -0
index ba6433c..04e8577 100644
drivers/media/usb/em28xx/em28xx-i2c.c int em28xx_i2c_register(struct em28xx *dev, unsigned bus, · line 942
Quoted 5 lines
drivers/media/usb/em28xx/em28xx-i2c.c +1 -1
@@ -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?

Quoted 1 line

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.

Quoted 2 lines

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

Thanks !

Regards,
Frank

Quoted 9 lines

Regards,
Hans
Quoted 4 lines
drivers/media/usb/em28xx/em28xx-i2c.c +0 -0
 	retval = i2c_add_adapter(&dev->i2c_adap[bus]);
 	if (retval < 0) {
 

Frank Schäfer

author

2014-03-22 13:00 UTC #4 lore raw

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

In reply to Frank Schäfer

Signed-off-by Frank Schäfer <[email protected]>

---
 drivers/media/usb/em28xx/em28xx.h | 4 ----
 1 file changed, 4 deletions(-)

drivers/media/usb/em28xx/em28xx.h +0 -4

index 2051fc9..e95f4eb 100644
@@ -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 */

Signature · 1 line

1.8.4.5

Frank Schäfer

author

2014-03-22 13:00 UTC #5 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/media/usb/em28xx/em28xx-video.c | 1 -
 drivers/media/usb/em28xx/em28xx.h       | 6 ------
 2 files changed, 7 deletions(-)

drivers/media/usb/em28xx/em28xx-video.c +0 -1

index 0856e5d..e15ac75 100644
@@ -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

drivers/media/usb/em28xx/em28xx.h +0 -6

index e95f4eb..dd6190c 100644
@@ -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)
 {

Signature · 1 line

1.8.4.5

Frank Schäfer

author

2014-03-24 19:26 UTC #6 lore raw

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

Am 22.03.2014 14:01, schrieb Frank Schäfer:

Quoted 31 lines

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 <[email protected]>
---
 drivers/media/usb/em28xx/em28xx-video.c | 1 -
 drivers/media/usb/em28xx/em28xx.h       | 6 ------
 2 files changed, 7 deletions(-)
drivers/media/usb/em28xx/em28xx-video.c +0 -1
index 0856e5d..e15ac75 100644
@@ -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
drivers/media/usb/em28xx/em28xx.h +0 -0
index e95f4eb..dd6190c 100644
@@ -793,12 +793,6 @@ int em28xx_init_camera(struct em28xx *dev);
 	printk(KERN_WARNING "%s: "fmt,\
 			dev->name , ##arg); } while (0)

drivers/media/usb/em28xx/em28xx.h int em28xx_init_camera(struct em28xx *dev); · line 798

Quoted 10 lines

drivers/media/usb/em28xx/em28xx.h +0 -6

 
-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.

Frank Schäfer

author

2014-03-22 13:00 UTC #7 lore raw

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

In reply to Frank Schäfer

Signed-off-by Frank Schäfer <[email protected]>

---
 drivers/media/usb/em28xx/em28xx-video.c | 23 +++++++++++++++++++++++
 drivers/media/usb/em28xx/em28xx.h       | 22 ----------------------
 2 files changed, 23 insertions(+), 22 deletions(-)

drivers/media/usb/em28xx/em28xx-video.c +23 -0

index e15ac75..ac499ae 100644
@@ -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 */

drivers/media/usb/em28xx/em28xx.h +0 ~~-22~~

index dd6190c..4beb1fa 100644
@@ -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

Signature · 1 line

1.8.4.5