[PATCH 00/10] Sparse fixes

[PATCH 00/10] Sparse fixes

[Hans Verkuil]

This patch series fixes almost all remaining sparse warnings. The only one
still there is this one in this platform driver:

.../platform/timblogiw.c:562:22: warning: context imbalance in 'buffer_queue' - unexpected unlock

Note that due to a bug in the daily build process not all media drivers were
checked with sparse. Now that I've fixed the build script I got a bunch of new
sparse warnings and one error. Those new sparse messages are addressed by
patches 6-10. Several of those fix real bugs too.

Regards,

	Hans

[PATCH 01/10] av7110: fix sparse warning

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

drivers/media/pci/ttpci/av7110.c:1226:15: warning: memset with byte count of 192512

Instead of memsetting this in one go, loop over each line and memset each line
separately.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/pci/ttpci/av7110.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/media/pci/ttpci/av7110.c b/drivers/media/pci/ttpci/av7110.c
index c1f0617..45199a1 100644
--- a/drivers/media/pci/ttpci/av7110.c
+++ b/drivers/media/pci/ttpci/av7110.c
@@ -1219,11 +1219,14 @@ static int stop_ts_capture(struct av7110 *budget)
 
 static int start_ts_capture(struct av7110 *budget)
 {
+	unsigned y;
+
 	dprintk(2, "budget: %p\n", budget);
 
 	if (budget->feeding1)
 		return ++budget->feeding1;
-	memset(budget->grabbing, 0x00, TS_BUFLEN);
+	for (y = 0; y < TS_HEIGHT; y++)
+		memset(budget->grabbing + y * TS_WIDTH, 0x00, TS_WIDTH);
 	budget->ttbp = 0;
 	SAA7146_ISR_CLEAR(budget->dev, MASK_10);  /* VPE */
 	SAA7146_IER_ENABLE(budget->dev, MASK_10); /* VPE */
-- 
2.1.3

[PATCH 02/10] budget-core: fix sparse warnings

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Fixes these sparse warnings.

drivers/media/pci/ttpci/budget-core.c:250:17: warning: context imbalance in 'ttpci_budget_debiread' - different lock contexts for basic block
drivers/media/pci/ttpci/budget-core.c:289:17: warning: context imbalance in 'ttpci_budget_debiwrite' - different lock contexts for basic block

To be honest, the new code does look better than the old.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/pci/ttpci/budget-core.c | 89 +++++++++++++++++++----------------
 1 file changed, 49 insertions(+), 40 deletions(-)

diff --git a/drivers/media/pci/ttpci/budget-core.c b/drivers/media/pci/ttpci/budget-core.c
index 37d02fe..23e0549 100644
--- a/drivers/media/pci/ttpci/budget-core.c
+++ b/drivers/media/pci/ttpci/budget-core.c
@@ -231,63 +231,59 @@ static void vpeirq(unsigned long data)
 }
 
 
-int ttpci_budget_debiread(struct budget *budget, u32 config, int addr, int count,
-			  int uselocks, int nobusyloop)
+static int ttpci_budget_debiread_nolock(struct budget *budget, u32 config,
+		int addr, int count, int nobusyloop)
 {
 	struct saa7146_dev *saa = budget->dev;
-	int result = 0;
-	unsigned long flags = 0;
-
-	if (count > 4 || count <= 0)
-		return 0;
-
-	if (uselocks)
-		spin_lock_irqsave(&budget->debilock, flags);
+	int result;
 
-	if ((result = saa7146_wait_for_debi_done(saa, nobusyloop)) < 0) {
-		if (uselocks)
-			spin_unlock_irqrestore(&budget->debilock, flags);
+	result = saa7146_wait_for_debi_done(saa, nobusyloop);
+	if (result < 0)
 		return result;
-	}
 
 	saa7146_write(saa, DEBI_COMMAND, (count << 17) | 0x10000 | (addr & 0xffff));
 	saa7146_write(saa, DEBI_CONFIG, config);
 	saa7146_write(saa, DEBI_PAGE, 0);
 	saa7146_write(saa, MC2, (2 << 16) | 2);
 
-	if ((result = saa7146_wait_for_debi_done(saa, nobusyloop)) < 0) {
-		if (uselocks)
-			spin_unlock_irqrestore(&budget->debilock, flags);
+	result = saa7146_wait_for_debi_done(saa, nobusyloop);
+	if (result < 0)
 		return result;
-	}
 
 	result = saa7146_read(saa, DEBI_AD);
 	result &= (0xffffffffUL >> ((4 - count) * 8));
-
-	if (uselocks)
-		spin_unlock_irqrestore(&budget->debilock, flags);
-
 	return result;
 }
 
-int ttpci_budget_debiwrite(struct budget *budget, u32 config, int addr,
-			   int count, u32 value, int uselocks, int nobusyloop)
+int ttpci_budget_debiread(struct budget *budget, u32 config, int addr, int count,
+			  int uselocks, int nobusyloop)
 {
-	struct saa7146_dev *saa = budget->dev;
-	unsigned long flags = 0;
-	int result;
-
 	if (count > 4 || count <= 0)
 		return 0;
 
-	if (uselocks)
-		spin_lock_irqsave(&budget->debilock, flags);
+	if (uselocks) {
+		unsigned long flags;
+		int result;
 
-	if ((result = saa7146_wait_for_debi_done(saa, nobusyloop)) < 0) {
-		if (uselocks)
-			spin_unlock_irqrestore(&budget->debilock, flags);
+		spin_lock_irqsave(&budget->debilock, flags);
+		result = ttpci_budget_debiread_nolock(budget, config, addr,
+						      count, nobusyloop);
+		spin_unlock_irqrestore(&budget->debilock, flags);
 		return result;
 	}
+	return ttpci_budget_debiread_nolock(budget, config, addr,
+					    count, nobusyloop);
+}
+
+static int ttpci_budget_debiwrite_nolock(struct budget *budget, u32 config,
+		int addr, int count, u32 value, int nobusyloop)
+{
+	struct saa7146_dev *saa = budget->dev;
+	int result;
+
+	result = saa7146_wait_for_debi_done(saa, nobusyloop);
+	if (result < 0)
+		return result;
 
 	saa7146_write(saa, DEBI_COMMAND, (count << 17) | 0x00000 | (addr & 0xffff));
 	saa7146_write(saa, DEBI_CONFIG, config);
@@ -295,15 +291,28 @@ int ttpci_budget_debiwrite(struct budget *budget, u32 config, int addr,
 	saa7146_write(saa, DEBI_AD, value);
 	saa7146_write(saa, MC2, (2 << 16) | 2);
 
-	if ((result = saa7146_wait_for_debi_done(saa, nobusyloop)) < 0) {
-		if (uselocks)
-			spin_unlock_irqrestore(&budget->debilock, flags);
-		return result;
-	}
+	result = saa7146_wait_for_debi_done(saa, nobusyloop);
+	return result < 0 ? result : 0;
+}
 
-	if (uselocks)
+int ttpci_budget_debiwrite(struct budget *budget, u32 config, int addr,
+			   int count, u32 value, int uselocks, int nobusyloop)
+{
+	if (count > 4 || count <= 0)
+		return 0;
+
+	if (uselocks) {
+		unsigned long flags;
+		int result;
+
+		spin_lock_irqsave(&budget->debilock, flags);
+		result = ttpci_budget_debiwrite_nolock(budget, config, addr,
+						count, value, nobusyloop);
 		spin_unlock_irqrestore(&budget->debilock, flags);
-	return 0;
+		return result;
+	}
+	return ttpci_budget_debiwrite_nolock(budget, config, addr,
+					     count, value, nobusyloop);
 }
 
 
-- 
2.1.3

[PATCH 03/10] ivtv: fix sparse warning

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Fix this warning:

drivers/media/pci/ivtv/ivtv-irq.c:418:9: warning: context imbalance in 'ivtv_dma_stream_dec_prepare' - different lock contexts for basic block

sparse didn't quite understand the locking scheme, so rewrite it to keep
sparse happy.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Andy Walls <awalls@md.metrocast.net>
---
 drivers/media/pci/ivtv/ivtv-irq.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/media/pci/ivtv/ivtv-irq.c b/drivers/media/pci/ivtv/ivtv-irq.c
index ab6d5d2..e7d7017 100644
--- a/drivers/media/pci/ivtv/ivtv-irq.c
+++ b/drivers/media/pci/ivtv/ivtv-irq.c
@@ -357,7 +357,6 @@ void ivtv_dma_stream_dec_prepare(struct ivtv_stream *s, u32 offset, int lock)
 	u32 uv_offset = offset + IVTV_YUV_BUFFER_UV_OFFSET;
 	int y_done = 0;
 	int bytes_written = 0;
-	unsigned long flags = 0;
 	int idx = 0;
 
 	IVTV_DEBUG_HI_DMA("DEC PREPARE DMA %s: %08x %08x\n", s->name, s->q_predma.bytesused, offset);
@@ -407,16 +406,21 @@ void ivtv_dma_stream_dec_prepare(struct ivtv_stream *s, u32 offset, int lock)
 
 	/* Sync Hardware SG List of buffers */
 	ivtv_stream_sync_for_device(s);
-	if (lock)
+	if (lock) {
+		unsigned long flags = 0;
+
 		spin_lock_irqsave(&itv->dma_reg_lock, flags);
-	if (!test_bit(IVTV_F_I_DMA, &itv->i_flags)) {
-		ivtv_dma_dec_start(s);
-	}
-	else {
-		set_bit(IVTV_F_S_DMA_PENDING, &s->s_flags);
-	}
-	if (lock)
+		if (!test_bit(IVTV_F_I_DMA, &itv->i_flags))
+			ivtv_dma_dec_start(s);
+		else
+			set_bit(IVTV_F_S_DMA_PENDING, &s->s_flags);
 		spin_unlock_irqrestore(&itv->dma_reg_lock, flags);
+	} else {
+		if (!test_bit(IVTV_F_I_DMA, &itv->i_flags))
+			ivtv_dma_dec_start(s);
+		else
+			set_bit(IVTV_F_S_DMA_PENDING, &s->s_flags);
+	}
 }
 
 static void ivtv_dma_enc_start_xfer(struct ivtv_stream *s)
-- 
2.1.3

[PATCH 04/10] videobuf2-vmalloc: fix sparse warning

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Fix this warning:

drivers/media/v4l2-core/videobuf2-vmalloc.c:98:28: warning: incorrect type in assignment (different address spaces)
drivers/media/v4l2-core/videobuf2-vmalloc.c:158:28: warning: incorrect type in argument 1 (different address spaces)

The warning is correct, but we have no other choice here to forcibly cast.
At least it is now explicit that such a cast is needed.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/v4l2-core/videobuf2-vmalloc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-vmalloc.c b/drivers/media/v4l2-core/videobuf2-vmalloc.c
index fba944e..7f6d41b 100644
--- a/drivers/media/v4l2-core/videobuf2-vmalloc.c
+++ b/drivers/media/v4l2-core/videobuf2-vmalloc.c
@@ -95,7 +95,7 @@ static void *vb2_vmalloc_get_userptr(void *alloc_ctx, unsigned long vaddr,
 		if (vb2_get_contig_userptr(vaddr, size, &vma, &physp))
 			goto fail_pages_array_alloc;
 		buf->vma = vma;
-		buf->vaddr = ioremap_nocache(physp, size);
+		buf->vaddr = (__force void *)ioremap_nocache(physp, size);
 		if (!buf->vaddr)
 			goto fail_pages_array_alloc;
 	} else {
@@ -155,7 +155,7 @@ static void vb2_vmalloc_put_userptr(void *buf_priv)
 		kfree(buf->pages);
 	} else {
 		vb2_put_vma(buf->vma);
-		iounmap(buf->vaddr);
+		iounmap((__force void __iomem *)buf->vaddr);
 	}
 	kfree(buf);
 }
-- 
2.1.3

[PATCH 05/10] media-entity: fix sparse warnings

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

drivers/media/media-entity.c:238:17: warning: Variable length array is used.
drivers/media/media-entity.c:239:17: warning: Variable length array is used.

Replace variable length by MEDIA_ENTITY_ENUM_MAX_ID.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/media-entity.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index 4d8e01c..dcf322b 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -235,8 +235,8 @@ __must_check int media_entity_pipeline_start(struct media_entity *entity,
 	media_entity_graph_walk_start(&graph, entity);
 
 	while ((entity = media_entity_graph_walk_next(&graph))) {
-		DECLARE_BITMAP(active, entity->num_pads);
-		DECLARE_BITMAP(has_no_links, entity->num_pads);
+		DECLARE_BITMAP(active, MEDIA_ENTITY_ENUM_MAX_ID);
+		DECLARE_BITMAP(has_no_links, MEDIA_ENTITY_ENUM_MAX_ID);
 		unsigned int i;
 
 		entity->stream_count++;
-- 
2.1.3

Re: [PATCH 05/10] media-entity: fix sparse warnings

[Sakari Ailus]

Hi Hans,

On Sat, Dec 13, 2014 at 12:52:55PM +0100, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> drivers/media/media-entity.c:238:17: warning: Variable length array is used.
> drivers/media/media-entity.c:239:17: warning: Variable length array is used.
> 
> Replace variable length by MEDIA_ENTITY_ENUM_MAX_ID.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/media/media-entity.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index 4d8e01c..dcf322b 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -235,8 +235,8 @@ __must_check int media_entity_pipeline_start(struct media_entity *entity,
>  	media_entity_graph_walk_start(&graph, entity);
>  
>  	while ((entity = media_entity_graph_walk_next(&graph))) {
> -		DECLARE_BITMAP(active, entity->num_pads);
> -		DECLARE_BITMAP(has_no_links, entity->num_pads);
> +		DECLARE_BITMAP(active, MEDIA_ENTITY_ENUM_MAX_ID);
> +		DECLARE_BITMAP(has_no_links, MEDIA_ENTITY_ENUM_MAX_ID);
>  		unsigned int i;
>  
>  		entity->stream_count++;

NAK.

MEDIA_ENTITY_ENUM_MAX_ID is the number of *entities* the graph parsing code
can handle, whereas the bitmap has to be large enough for the *pads* in an
entity.

There is no guarantee that the number of pads would be less than
MEDIA_ENTITY_ENUM_MAX_ID, which could lead to memory corruption. Also
MEDIA_ENTITY_ENUM_MAX_ID is probably about ten times as large as there
usually are pads.

What's wrong with using variable length arrays in general? Of course, there
needs to be a guarantee that the length is decently small, which is the case
in here.

We could define an upper limit for the number of pads, and check that the
number of pads actually is not greater than this.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

[PATCH 06/10] hd29l2: fix sparse error and warnings

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

drivers/media/dvb-frontends/hd29l2.c:29:18: warning: Variable length array is used.
drivers/media/dvb-frontends/hd29l2.c:34:32: error: cannot size expression
drivers/media/dvb-frontends/hd29l2.c:125:5: warning: symbol 'hd29l2_rd_reg_mask' was not declared. Should it be static?

Variable length arrays are frowned upon, so replace with a fixed length and check
that there won't be a buffer overrun.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Antti Palosaari <crope@iki.fi>
---
 drivers/media/dvb-frontends/hd29l2.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/media/dvb-frontends/hd29l2.c b/drivers/media/dvb-frontends/hd29l2.c
index d7b9d54..67c8e6d 100644
--- a/drivers/media/dvb-frontends/hd29l2.c
+++ b/drivers/media/dvb-frontends/hd29l2.c
@@ -22,20 +22,24 @@
 
 #include "hd29l2_priv.h"
 
+#define HD29L2_MAX_LEN (3)
+
 /* write multiple registers */
 static int hd29l2_wr_regs(struct hd29l2_priv *priv, u8 reg, u8 *val, int len)
 {
 	int ret;
-	u8 buf[2 + len];
+	u8 buf[2 + HD29L2_MAX_LEN];
 	struct i2c_msg msg[1] = {
 		{
 			.addr = priv->cfg.i2c_addr,
 			.flags = 0,
-			.len = sizeof(buf),
+			.len = 2 + len,
 			.buf = buf,
 		}
 	};
 
+	if (len > HD29L2_MAX_LEN)
+		return -EINVAL;
 	buf[0] = 0x00;
 	buf[1] = reg;
 	memcpy(&buf[2], val, len);
@@ -118,7 +122,7 @@ static int hd29l2_wr_reg_mask(struct hd29l2_priv *priv, u8 reg, u8 val, u8 mask)
 }
 
 /* read single register with mask */
-int hd29l2_rd_reg_mask(struct hd29l2_priv *priv, u8 reg, u8 *val, u8 mask)
+static int hd29l2_rd_reg_mask(struct hd29l2_priv *priv, u8 reg, u8 *val, u8 mask)
 {
 	int ret, i;
 	u8 tmp;
-- 
2.1.3

Re: [PATCH 06/10] hd29l2: fix sparse error and warnings

[Antti Palosaari]

On 12/13/2014 01:52 PM, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> drivers/media/dvb-frontends/hd29l2.c:29:18: warning: Variable length array is used.
> drivers/media/dvb-frontends/hd29l2.c:34:32: error: cannot size expression
> drivers/media/dvb-frontends/hd29l2.c:125:5: warning: symbol 'hd29l2_rd_reg_mask' was not declared. Should it be static?
>
> Variable length arrays are frowned upon, so replace with a fixed length and check
> that there won't be a buffer overrun.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Cc: Antti Palosaari <crope@iki.fi>

Reviewed-by: Antti Palosaari <crope@iki.fi>

Antti

-- 
http://palosaari.fi/

[PATCH 07/10] m5mols: fix sparse warnings

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

drivers/media/i2c/m5mols/m5mols_core.c:128:24: warning: cast to restricted __be16
drivers/media/i2c/m5mols/m5mols_core.c:128:24: warning: cast to restricted __be16
drivers/media/i2c/m5mols/m5mols_core.c:128:24: warning: cast to restricted __be16
drivers/media/i2c/m5mols/m5mols_core.c:128:24: warning: cast to restricted __be16
drivers/media/i2c/m5mols/m5mols_core.c:130:24: warning: cast to restricted __be32
drivers/media/i2c/m5mols/m5mols_core.c:130:24: warning: cast to restricted __be32
drivers/media/i2c/m5mols/m5mols_core.c:130:24: warning: cast to restricted __be32
drivers/media/i2c/m5mols/m5mols_core.c:130:24: warning: cast to restricted __be32
drivers/media/i2c/m5mols/m5mols_core.c:130:24: warning: cast to restricted __be32
drivers/media/i2c/m5mols/m5mols_core.c:130:24: warning: cast to restricted __be32
drivers/media/i2c/m5mols/m5mols_core.c:457:19: warning: cast to restricted __be16
drivers/media/i2c/m5mols/m5mols_core.c:457:19: warning: cast to restricted __be16
drivers/media/i2c/m5mols/m5mols_core.c:457:19: warning: cast to restricted __be16
drivers/media/i2c/m5mols/m5mols_core.c:457:19: warning: cast to restricted __be16
drivers/media/i2c/m5mols/m5mols_core.c:458:19: warning: cast to restricted __be16
drivers/media/i2c/m5mols/m5mols_core.c:458:19: warning: cast to restricted __be16
drivers/media/i2c/m5mols/m5mols_core.c:458:19: warning: cast to restricted __be16
drivers/media/i2c/m5mols/m5mols_core.c:458:19: warning: cast to restricted __be16
drivers/media/i2c/m5mols/m5mols_core.c:459:22: warning: cast to restricted __be16
drivers/media/i2c/m5mols/m5mols_core.c:459:22: warning: cast to restricted __be16
drivers/media/i2c/m5mols/m5mols_core.c:459:22: warning: cast to restricted __be16
drivers/media/i2c/m5mols/m5mols_core.c:459:22: warning: cast to restricted __be16
drivers/media/i2c/m5mols/m5mols_core.c:460:20: warning: cast to restricted __be16
drivers/media/i2c/m5mols/m5mols_core.c:460:20: warning: cast to restricted __be16
drivers/media/i2c/m5mols/m5mols_core.c:460:20: warning: cast to restricted __be16
drivers/media/i2c/m5mols/m5mols_core.c:460:20: warning: cast to restricted __be16

The be16_to_cpu conversions in m5mols_get_version() are not needed since the
data is already using cpu endianness. This was never noticed since these
version fields are never used.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 drivers/media/i2c/m5mols/m5mols_core.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/media/i2c/m5mols/m5mols_core.c b/drivers/media/i2c/m5mols/m5mols_core.c
index 2820f7c..6ed16e5 100644
--- a/drivers/media/i2c/m5mols/m5mols_core.c
+++ b/drivers/media/i2c/m5mols/m5mols_core.c
@@ -125,9 +125,9 @@ static u32 m5mols_swap_byte(u8 *data, u8 length)
 	if (length == 1)
 		return *data;
 	else if (length == 2)
-		return be16_to_cpu(*((u16 *)data));
+		return be16_to_cpu(*((__be16 *)data));
 	else
-		return be32_to_cpu(*((u32 *)data));
+		return be32_to_cpu(*((__be32 *)data));
 }
 
 /**
@@ -454,11 +454,6 @@ static int m5mols_get_version(struct v4l2_subdev *sd)
 			return ret;
 	}
 
-	ver->fw = be16_to_cpu(ver->fw);
-	ver->hw = be16_to_cpu(ver->hw);
-	ver->param = be16_to_cpu(ver->param);
-	ver->awb = be16_to_cpu(ver->awb);
-
 	v4l2_info(sd, "Manufacturer\t[%s]\n",
 			is_manufacturer(info, REG_SAMSUNG_ELECTRO) ?
 			"Samsung Electro-Machanics" :
-- 
2.1.3

Re: [PATCH 07/10] m5mols: fix sparse warnings

[Sylwester Nawrocki]

On 13/12/14 12:52, Hans Verkuil wrote:
> The be16_to_cpu conversions in m5mols_get_version() are not needed since the
> data is already using cpu endianness. This was never noticed since these
> version fields are never used.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Acked-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

[PATCH 08/10] s5k4ecgx: fix sparse warnings

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

drivers/media/i2c/s5k4ecgx.c:223:16: warning: cast to restricted __be16
drivers/media/i2c/s5k4ecgx.c:223:16: warning: cast to restricted __be16
drivers/media/i2c/s5k4ecgx.c:223:16: warning: cast to restricted __be16
drivers/media/i2c/s5k4ecgx.c:223:16: warning: cast to restricted __be16
drivers/media/i2c/s5k4ecgx.c:344:20: warning: cast to restricted __le32
drivers/media/i2c/s5k4ecgx.c:354:20: warning: cast to restricted __le32
drivers/media/i2c/s5k4ecgx.c:364:24: warning: cast to restricted __le32
drivers/media/i2c/s5k4ecgx.c:366:23: warning: cast to restricted __le16

The get_unaligned_le*() functions return the value using cpu endianness,
so calling le*_to_cpu is wrong.

It hasn't been not noticed because this code has only been run on little
endian systems, so le*_to_cpu doesn't do anything.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 drivers/media/i2c/s5k4ecgx.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/media/i2c/s5k4ecgx.c b/drivers/media/i2c/s5k4ecgx.c
index d1c50c9..7007131 100644
--- a/drivers/media/i2c/s5k4ecgx.c
+++ b/drivers/media/i2c/s5k4ecgx.c
@@ -220,7 +220,7 @@ static int s5k4ecgx_i2c_read(struct i2c_client *client, u16 addr, u16 *val)
 	msg[1].buf = rbuf;
 
 	ret = i2c_transfer(client->adapter, msg, 2);
-	*val = be16_to_cpu(*((u16 *)rbuf));
+	*val = be16_to_cpu(*((__be16 *)rbuf));
 
 	v4l2_dbg(4, debug, client, "i2c_read: 0x%04X : 0x%04x\n", addr, *val);
 
@@ -341,7 +341,7 @@ static int s5k4ecgx_load_firmware(struct v4l2_subdev *sd)
 		v4l2_err(sd, "Failed to read firmware %s\n", S5K4ECGX_FIRMWARE);
 		return err;
 	}
-	regs_num = le32_to_cpu(get_unaligned_le32(fw->data));
+	regs_num = get_unaligned_le32(fw->data);
 
 	v4l2_dbg(3, debug, sd, "FW: %s size %zu register sets %d\n",
 		 S5K4ECGX_FIRMWARE, fw->size, regs_num);
@@ -351,8 +351,7 @@ static int s5k4ecgx_load_firmware(struct v4l2_subdev *sd)
 		err = -EINVAL;
 		goto fw_out;
 	}
-	crc_file = le32_to_cpu(get_unaligned_le32(fw->data +
-						  regs_num * FW_RECORD_SIZE));
+	crc_file = get_unaligned_le32(fw->data + regs_num * FW_RECORD_SIZE);
 	crc = crc32_le(~0, fw->data, regs_num * FW_RECORD_SIZE);
 	if (crc != crc_file) {
 		v4l2_err(sd, "FW: invalid crc (%#x:%#x)\n", crc, crc_file);
@@ -361,9 +360,9 @@ static int s5k4ecgx_load_firmware(struct v4l2_subdev *sd)
 	}
 	ptr = fw->data + FW_RECORD_SIZE;
 	for (i = 1; i < regs_num; i++) {
-		addr = le32_to_cpu(get_unaligned_le32(ptr));
+		addr = get_unaligned_le32(ptr);
 		ptr += sizeof(u32);
-		val = le16_to_cpu(get_unaligned_le16(ptr));
+		val = get_unaligned_le16(ptr);
 		ptr += sizeof(u16);
 		if (addr - addr_inc != 2)
 			err = s5k4ecgx_write(client, addr, val);
-- 
2.1.3

Re: [PATCH 08/10] s5k4ecgx: fix sparse warnings

[Sylwester Nawrocki]

On 13/12/14 12:52, Hans Verkuil wrote:
> The get_unaligned_le*() functions return the value using cpu endianness,
> so calling le*_to_cpu is wrong.
> 
> It hasn't been not noticed because this code has only been run on little
> endian systems, so le*_to_cpu doesn't do anything.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>

Acked-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

[PATCH 09/10] s5k6aa: fix sparse warnings

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

drivers/media/i2c/s5k6aa.c:351:16: warning: cast to restricted __be16
drivers/media/i2c/s5k6aa.c:351:16: warning: cast to restricted __be16
drivers/media/i2c/s5k6aa.c:351:16: warning: cast to restricted __be16
drivers/media/i2c/s5k6aa.c:351:16: warning: cast to restricted __be16

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 drivers/media/i2c/s5k6aa.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/s5k6aa.c b/drivers/media/i2c/s5k6aa.c
index 2851581..c98dec3 100644
--- a/drivers/media/i2c/s5k6aa.c
+++ b/drivers/media/i2c/s5k6aa.c
@@ -348,7 +348,7 @@ static int s5k6aa_i2c_read(struct i2c_client *client, u16 addr, u16 *val)
 	msg[1].buf = rbuf;
 
 	ret = i2c_transfer(client->adapter, msg, 2);
-	*val = be16_to_cpu(*((u16 *)rbuf));
+	*val = be16_to_cpu(*((__be16 *)rbuf));
 
 	v4l2_dbg(3, debug, client, "i2c_read: 0x%04X : 0x%04x\n", addr, *val);
 
-- 
2.1.3

Re: [PATCH 09/10] s5k6aa: fix sparse warnings

[Sylwester Nawrocki]

On 13/12/14 12:52, Hans Verkuil wrote:
> drivers/media/i2c/s5k6aa.c:351:16: warning: cast to restricted __be16
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Acked-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

[PATCH 10/10] s5k5baf: fix sparse warnings

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

drivers/media/i2c/s5k5baf.c:1796:33: warning: duplicate const
drivers/media/i2c/s5k5baf.c:379:24: warning: cast to restricted __le16
drivers/media/i2c/s5k5baf.c:437:11: warning: incorrect type in assignment (different base types)
drivers/media/i2c/s5k5baf.c:445:16: warning: incorrect type in return expression (different base types)

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 drivers/media/i2c/s5k5baf.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/media/i2c/s5k5baf.c b/drivers/media/i2c/s5k5baf.c
index 60a74d8..a3d7d03 100644
--- a/drivers/media/i2c/s5k5baf.c
+++ b/drivers/media/i2c/s5k5baf.c
@@ -353,7 +353,7 @@ static struct v4l2_rect s5k5baf_cis_rect = {
  *
  */
 static int s5k5baf_fw_parse(struct device *dev, struct s5k5baf_fw **fw,
-			    size_t count, const u16 *data)
+			    size_t count, const __le16 *data)
 {
 	struct s5k5baf_fw *f;
 	u16 *d, i, *end;
@@ -421,6 +421,7 @@ static u16 s5k5baf_i2c_read(struct s5k5baf *state, u16 addr)
 {
 	struct i2c_client *c = v4l2_get_subdevdata(&state->sd);
 	__be16 w, r;
+	u16 res;
 	struct i2c_msg msg[] = {
 		{ .addr = c->addr, .flags = 0,
 		  .len = 2, .buf = (u8 *)&w },
@@ -434,15 +435,15 @@ static u16 s5k5baf_i2c_read(struct s5k5baf *state, u16 addr)
 
 	w = cpu_to_be16(addr);
 	ret = i2c_transfer(c->adapter, msg, 2);
-	r = be16_to_cpu(r);
+	res = be16_to_cpu(r);
 
-	v4l2_dbg(3, debug, c, "i2c_read: 0x%04x : 0x%04x\n", addr, r);
+	v4l2_dbg(3, debug, c, "i2c_read: 0x%04x : 0x%04x\n", addr, res);
 
 	if (ret != 2) {
 		v4l2_err(c, "i2c_read: error during transfer (%d)\n", ret);
 		state->error = ret;
 	}
-	return r;
+	return res;
 }
 
 static void s5k5baf_i2c_write(struct s5k5baf *state, u16 addr, u16 val)
@@ -1037,7 +1038,7 @@ static int s5k5baf_load_setfile(struct s5k5baf *state)
 	}
 
 	ret = s5k5baf_fw_parse(&c->dev, &state->fw, fw->size / 2,
-			       (u16 *)fw->data);
+			       (__le16 *)fw->data);
 
 	release_firmware(fw);
 
@@ -1793,7 +1794,7 @@ static const struct v4l2_subdev_ops s5k5baf_subdev_ops = {
 
 static int s5k5baf_configure_gpios(struct s5k5baf *state)
 {
-	static const char const *name[] = { "S5K5BAF_STBY", "S5K5BAF_RST" };
+	static const char * const name[] = { "S5K5BAF_STBY", "S5K5BAF_RST" };
 	struct i2c_client *c = v4l2_get_subdevdata(&state->sd);
 	struct s5k5baf_gpio *g = state->gpios;
 	int ret, i;
-- 
2.1.3

Re: [PATCH 10/10] s5k5baf: fix sparse warnings

[Sylwester Nawrocki]

On 13/12/14 12:53, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> drivers/media/i2c/s5k5baf.c:1796:33: warning: duplicate const
> drivers/media/i2c/s5k5baf.c:379:24: warning: cast to restricted __le16
> drivers/media/i2c/s5k5baf.c:437:11: warning: incorrect type in assignment (different base types)
> drivers/media/i2c/s5k5baf.c:445:16: warning: incorrect type in return expression (different base types)
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Acked-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

> ---
>  drivers/media/i2c/s5k5baf.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/i2c/s5k5baf.c b/drivers/media/i2c/s5k5baf.c
> index 60a74d8..a3d7d03 100644
> --- a/drivers/media/i2c/s5k5baf.c
> +++ b/drivers/media/i2c/s5k5baf.c
> @@ -353,7 +353,7 @@ static struct v4l2_rect s5k5baf_cis_rect = {
>   *
>   */
>  static int s5k5baf_fw_parse(struct device *dev, struct s5k5baf_fw **fw,
> -			    size_t count, const u16 *data)
> +			    size_t count, const __le16 *data)
>  {
>  	struct s5k5baf_fw *f;
>  	u16 *d, i, *end;
> @@ -421,6 +421,7 @@ static u16 s5k5baf_i2c_read(struct s5k5baf *state, u16 addr)
>  {
>  	struct i2c_client *c = v4l2_get_subdevdata(&state->sd);
>  	__be16 w, r;
> +	u16 res;
>  	struct i2c_msg msg[] = {
>  		{ .addr = c->addr, .flags = 0,
>  		  .len = 2, .buf = (u8 *)&w },
> @@ -434,15 +435,15 @@ static u16 s5k5baf_i2c_read(struct s5k5baf *state, u16 addr)
>  
>  	w = cpu_to_be16(addr);
>  	ret = i2c_transfer(c->adapter, msg, 2);
> -	r = be16_to_cpu(r);
> +	res = be16_to_cpu(r);
>  
> -	v4l2_dbg(3, debug, c, "i2c_read: 0x%04x : 0x%04x\n", addr, r);
> +	v4l2_dbg(3, debug, c, "i2c_read: 0x%04x : 0x%04x\n", addr, res);
>  
>  	if (ret != 2) {
>  		v4l2_err(c, "i2c_read: error during transfer (%d)\n", ret);
>  		state->error = ret;
>  	}
> -	return r;
> +	return res;
>  }
>  
>  static void s5k5baf_i2c_write(struct s5k5baf *state, u16 addr, u16 val)
> @@ -1037,7 +1038,7 @@ static int s5k5baf_load_setfile(struct s5k5baf *state)
>  	}
>  
>  	ret = s5k5baf_fw_parse(&c->dev, &state->fw, fw->size / 2,
> -			       (u16 *)fw->data);
> +			       (__le16 *)fw->data);
>  
>  	release_firmware(fw);
>  
> @@ -1793,7 +1794,7 @@ static const struct v4l2_subdev_ops s5k5baf_subdev_ops = {
>  
>  static int s5k5baf_configure_gpios(struct s5k5baf *state)
>  {
> -	static const char const *name[] = { "S5K5BAF_STBY", "S5K5BAF_RST" };
> +	static const char * const name[] = { "S5K5BAF_STBY", "S5K5BAF_RST" };
>  	struct i2c_client *c = v4l2_get_subdevdata(&state->sd);
>  	struct s5k5baf_gpio *g = state->gpios;
>  	int ret, i;