[PATCH 1/6] [media] mantis: check for errors on readl inside loop

[PATCH 1/6] [media] mantis: check for errors on readl inside loop

[Mauro Carvalho Chehab]

As warned by smatch:
	drivers/media/pci/mantis/mantis_uart.c:105 mantis_uart_work() warn: this loop depends on readl() succeeding

If readl() fails, this could lead into an endless loop. Avoid that.
We might instead add some timeout logic, but it readl() is
failing, then something really wrong is happening.

While here, remove two defines that are only used once.

Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
---
 drivers/media/pci/mantis/mantis_common.h | 7 ++-----
 drivers/media/pci/mantis/mantis_uart.c   | 4 ++--
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/media/pci/mantis/mantis_common.h b/drivers/media/pci/mantis/mantis_common.h
index d48778a366a9..8348c5de3d18 100644
--- a/drivers/media/pci/mantis/mantis_common.h
+++ b/drivers/media/pci/mantis/mantis_common.h
@@ -54,11 +54,8 @@
 	}												\
 } while(0)
 
-#define mwrite(dat, addr)	writel((dat), addr)
-#define mread(addr)		readl(addr)
-
-#define mmwrite(dat, addr)	mwrite((dat), (mantis->mmio + (addr)))
-#define mmread(addr)		mread(mantis->mmio + (addr))
+#define mmwrite(dat, addr)	writel((dat), (mantis->mmio + (addr)))
+#define mmread(addr)		readl(mantis->mmio + (addr))
 
 #define MANTIS_TS_188		0
 #define MANTIS_TS_204		1
diff --git a/drivers/media/pci/mantis/mantis_uart.c b/drivers/media/pci/mantis/mantis_uart.c
index f1c96aec8c7b..95ccc34be9fd 100644
--- a/drivers/media/pci/mantis/mantis_uart.c
+++ b/drivers/media/pci/mantis/mantis_uart.c
@@ -91,7 +91,7 @@ static void mantis_uart_read(struct mantis_pci *mantis)
 static void mantis_uart_work(struct work_struct *work)
 {
 	struct mantis_pci *mantis = container_of(work, struct mantis_pci, uart_work);
-	u32 stat;
+	int stat;
 
 	stat = mmread(MANTIS_UART_STAT);
 
@@ -102,7 +102,7 @@ static void mantis_uart_work(struct work_struct *work)
 	 * MANTIS_UART_RXFIFO_DATA is only set if at least
 	 * config->bytes + 1 bytes are in the FIFO.
 	 */
-	while (stat & MANTIS_UART_RXFIFO_DATA) {
+	while ((stat >= 0) && (stat & MANTIS_UART_RXFIFO_DATA)) {
 		mantis_uart_read(mantis);
 		stat = mmread(MANTIS_UART_STAT);
 	}
-- 
2.5.0

[PATCH 2/6] [media] ddcore: avoid endless loop if readl() fails

[Mauro Carvalho Chehab]

As warned by smatch:
	drivers/media/pci/ddbridge/ddbridge-core.c:1351 flashio() warn: this loop depends on readl() succeeding
	drivers/media/pci/ddbridge/ddbridge-core.c:1371 flashio() warn: this loop depends on readl() succeeding

Let the loop be interrupted too if something really bad happens
and readl() fails.

Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
---
 drivers/media/pci/ddbridge/ddbridge-core.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c b/drivers/media/pci/ddbridge/ddbridge-core.c
index 6e995ef8c37e..4b85dd0cac3f 100644
--- a/drivers/media/pci/ddbridge/ddbridge-core.c
+++ b/drivers/media/pci/ddbridge/ddbridge-core.c
@@ -1339,6 +1339,7 @@ static irqreturn_t irq_handler(int irq, void *dev_id)
 static int flashio(struct ddb *dev, u8 *wbuf, u32 wlen, u8 *rbuf, u32 rlen)
 {
 	u32 data, shift;
+	int val;
 
 	if (wlen > 4)
 		ddbwritel(1, SPI_CONTROL);
@@ -1348,8 +1349,9 @@ static int flashio(struct ddb *dev, u8 *wbuf, u32 wlen, u8 *rbuf, u32 rlen)
 		wbuf += 4;
 		wlen -= 4;
 		ddbwritel(data, SPI_DATA);
-		while (ddbreadl(SPI_CONTROL) & 0x0004)
-			;
+		do {
+			val = ddbreadl(SPI_CONTROL);
+		} while (val >= 0 && val & 0x0004);
 	}
 
 	if (rlen)
@@ -1368,8 +1370,9 @@ static int flashio(struct ddb *dev, u8 *wbuf, u32 wlen, u8 *rbuf, u32 rlen)
 	if (shift)
 		data <<= shift;
 	ddbwritel(data, SPI_DATA);
-	while (ddbreadl(SPI_CONTROL) & 0x0004)
-		;
+	do {
+		val = ddbreadl(SPI_CONTROL);
+	} while (val >= 0 && val & 0x0004);
 
 	if (!rlen) {
 		ddbwritel(0, SPI_CONTROL);
@@ -1380,8 +1383,9 @@ static int flashio(struct ddb *dev, u8 *wbuf, u32 wlen, u8 *rbuf, u32 rlen)
 
 	while (rlen > 4) {
 		ddbwritel(0xffffffff, SPI_DATA);
-		while (ddbreadl(SPI_CONTROL) & 0x0004)
-			;
+		do {
+			val = ddbreadl(SPI_CONTROL);
+		} while (val >= 0 && val & 0x0004);
 		data = ddbreadl(SPI_DATA);
 		*(u32 *) rbuf = swab32(data);
 		rbuf += 4;
@@ -1389,8 +1393,9 @@ static int flashio(struct ddb *dev, u8 *wbuf, u32 wlen, u8 *rbuf, u32 rlen)
 	}
 	ddbwritel(0x0003 | ((rlen << (8 + 3)) & 0x1F00), SPI_CONTROL);
 	ddbwritel(0xffffffff, SPI_DATA);
-	while (ddbreadl(SPI_CONTROL) & 0x0004)
-		;
+	do {
+		val = ddbreadl(SPI_CONTROL);
+	} while (val >= 0 && val & 0x0004);
 
 	data = ddbreadl(SPI_DATA);
 	ddbwritel(0, SPI_CONTROL);
-- 
2.5.0

[PATCH 3/6] [media] mceusb: use %*ph for small buffer dumps

[Mauro Carvalho Chehab]

It makes the printk cleaner. As a side efect, it also fixes those smatch
warnings:
	drivers/media/rc/mceusb.c:590 mceusb_dev_printdata() warn: argument 6 to %02x specifier has type 'char'
	drivers/media/rc/mceusb.c:590 mceusb_dev_printdata() warn: argument 7 to %02x specifier has type 'char'

Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
---
 drivers/media/rc/mceusb.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c
index 2cdb740cde48..35155ae500c7 100644
--- a/drivers/media/rc/mceusb.c
+++ b/drivers/media/rc/mceusb.c
@@ -587,9 +587,8 @@ static void mceusb_dev_printdata(struct mceusb_dev *ir, char *buf,
 			if (len == 2)
 				dev_dbg(dev, "Get hw/sw rev?");
 			else
-				dev_dbg(dev, "hw/sw rev 0x%02x 0x%02x 0x%02x 0x%02x",
-					 data1, data2,
-					 buf[start + 4], buf[start + 5]);
+				dev_dbg(dev, "hw/sw rev %*ph",
+					4, &buf[start + 2]);
 			break;
 		case MCE_CMD_RESUME:
 			dev_dbg(dev, "Device resume requested");
-- 
2.5.0

[PATCH 4/6] [media] st-rc: prevent a endless loop

[Mauro Carvalho Chehab]

As warned by smatch:
	drivers/media/rc/st_rc.c:110 st_rc_rx_interrupt() warn: this loop depends on readl() succeeding

as readl() might fail, with likely means some unrecovered error,
let's loop only if it succeeds.

Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
---
 drivers/media/rc/st_rc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/rc/st_rc.c b/drivers/media/rc/st_rc.c
index 1fa0c9d1c508..151bfe2aea55 100644
--- a/drivers/media/rc/st_rc.c
+++ b/drivers/media/rc/st_rc.c
@@ -99,7 +99,7 @@ static irqreturn_t st_rc_rx_interrupt(int irq, void *data)
 	unsigned int symbol, mark = 0;
 	struct st_rc_device *dev = data;
 	int last_symbol = 0;
-	u32 status;
+	int status;
 	DEFINE_IR_RAW_EVENT(ev);
 
 	if (dev->irq_wake)
@@ -107,7 +107,7 @@ static irqreturn_t st_rc_rx_interrupt(int irq, void *data)
 
 	status  = readl(dev->rx_base + IRB_RX_STATUS);
 
-	while (status & (IRB_FIFO_NOT_EMPTY | IRB_OVERFLOW)) {
+	while (status > 0 && (status & (IRB_FIFO_NOT_EMPTY | IRB_OVERFLOW))) {
 		u32 int_status = readl(dev->rx_base + IRB_RX_INT_STATUS);
 		if (unlikely(int_status & IRB_RX_OVERRUN_INT)) {
 			/* discard the entire collection in case of errors!  */
-- 
2.5.0

[PATCH 5/6] [media] touptek: don't DMA at the stack

[Mauro Carvalho Chehab]

As warned by smatch:
	drivers/media/usb/gspca/touptek.c:220 reg_w() error: doing dma on the stack (buff)
	drivers/media/usb/gspca/touptek.c:458 configure() error: doing dma on the stack (buff)

This can fail, as the stack may not be in a memory that would
allod DMA. So, use the usb_buf instead.

Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
---
 drivers/media/usb/gspca/touptek.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/gspca/touptek.c b/drivers/media/usb/gspca/touptek.c
index 7bac6bc96063..8063b8e45ee5 100644
--- a/drivers/media/usb/gspca/touptek.c
+++ b/drivers/media/usb/gspca/touptek.c
@@ -211,7 +211,7 @@ static int val_reply(struct gspca_dev *gspca_dev, const char *reply, int rc)
 
 static void reg_w(struct gspca_dev *gspca_dev, u16 value, u16 index)
 {
-	char buff[1];
+	char *buff = gspca_dev->usb_buf;
 	int rc;
 
 	PDEBUG(D_USBO,
@@ -438,7 +438,7 @@ static void configure_encrypted(struct gspca_dev *gspca_dev)
 static int configure(struct gspca_dev *gspca_dev)
 {
 	int rc;
-	uint8_t buff[4];
+	char *buff = gspca_dev->usb_buf;
 
 	PDEBUG(D_STREAM, "configure()\n");
 
-- 
2.5.0

[PATCH 6/6] [media] touptek: cast char types on %x printk

[Mauro Carvalho Chehab]

This fixes those two smatch warnings:
	drivers/media/usb/gspca/touptek.c:206 val_reply() warn: argument 3 to %02x specifier has type 'char'
	drivers/media/usb/gspca/touptek.c:222 reg_w() warn: argument 4 to %02x specifier has type 'char'

Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
---
 drivers/media/usb/gspca/touptek.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/gspca/touptek.c b/drivers/media/usb/gspca/touptek.c
index 8063b8e45ee5..b8af4370d27c 100644
--- a/drivers/media/usb/gspca/touptek.c
+++ b/drivers/media/usb/gspca/touptek.c
@@ -203,7 +203,7 @@ static int val_reply(struct gspca_dev *gspca_dev, const char *reply, int rc)
 		return -EIO;
 	}
 	if (reply[0] != 0x08) {
-		PERR("Bad reply 0x%02X", reply[0]);
+		PERR("Bad reply 0x%02x", (int)reply[0]);
 		return -EIO;
 	}
 	return 0;
@@ -219,7 +219,7 @@ static void reg_w(struct gspca_dev *gspca_dev, u16 value, u16 index)
 		value, index);
 	rc = usb_control_msg(gspca_dev->dev, usb_rcvctrlpipe(gspca_dev->dev, 0),
 		0x0B, 0xC0, value, index, buff, 1, 500);
-	PDEBUG(D_USBO, "rc=%d, ret={0x%02X}", rc, buff[0]);
+	PDEBUG(D_USBO, "rc=%d, ret={0x%02x}", rc, (int)buff[0]);
 	if (rc < 0) {
 		PERR("Failed reg_w(0x0B, 0xC0, 0x%04X, 0x%04X) w/ rc %d\n",
 			value, index, rc);
-- 
2.5.0

Re: [PATCH 4/6] [media] st-rc: prevent a endless loop

[Patrice Chotard]

Hi Mauro

On 03/06/2016 02:39 PM, Mauro Carvalho Chehab wrote:
> As warned by smatch:
> 	drivers/media/rc/st_rc.c:110 st_rc_rx_interrupt() warn: this loop depends on readl() succeeding
>
> as readl() might fail, with likely means some unrecovered error,
> let's loop only if it succeeds.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> ---
>   drivers/media/rc/st_rc.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/rc/st_rc.c b/drivers/media/rc/st_rc.c
> index 1fa0c9d1c508..151bfe2aea55 100644
> --- a/drivers/media/rc/st_rc.c
> +++ b/drivers/media/rc/st_rc.c
> @@ -99,7 +99,7 @@ static irqreturn_t st_rc_rx_interrupt(int irq, void *data)
>   	unsigned int symbol, mark = 0;
>   	struct st_rc_device *dev = data;
>   	int last_symbol = 0;
> -	u32 status;
> +	int status;
>   	DEFINE_IR_RAW_EVENT(ev);
>   
>   	if (dev->irq_wake)
> @@ -107,7 +107,7 @@ static irqreturn_t st_rc_rx_interrupt(int irq, void *data)
>   
>   	status  = readl(dev->rx_base + IRB_RX_STATUS);
>   
> -	while (status & (IRB_FIFO_NOT_EMPTY | IRB_OVERFLOW)) {
> +	while (status > 0 && (status & (IRB_FIFO_NOT_EMPTY | IRB_OVERFLOW))) {
>   		u32 int_status = readl(dev->rx_base + IRB_RX_INT_STATUS);
>   		if (unlikely(int_status & IRB_RX_OVERRUN_INT)) {
>   			/* discard the entire collection in case of errors!  */

Acked-by: Patrice Chotard <patrice.chotard@st.com>