V4L/DVB (8914): cx18: Throttle mmio to/from the CX23418 so boards work in older systems

cx18: Throttle mmio to/from the CX23418 so boards work in older systems.  The
CX23418 couldn't reliably handle mmio at the rate at which the cx18 driver was
attempting to access the chip.  The PCI bridge arrangements and settings on
modern motherboards still allowed the CX23418 to work OK, but it didn't work
well on many older motherboards: mysterious I2C errors, firmware loading errors,
etc.  This patch adds a throttle to *all* mmio access to the CX23418.  It
defaults to a delay of 31 ns, but is adjustable by the mmio_ndelay module parm.
My HVR-1600 and Raptor PAL/SECAM card now function for analog capture on a
motherboard with an Intel 82810E Northbridge and 82801AA Southbridge.
Signed-off-by: NAndy Walls <awalls@radix.net>
Signed-off-by: NMauro Carvalho Chehab <mchehab@redhat.com>

drivers/media/video/cx18/cx18-driver.c

@@ -4,6 +4,7 @@
  *  Derived from ivtv-driver.c
  *
  *  Copyright (C) 2007  Hans Verkuil <hverkuil@xs4all.nl>
+ *  Copyright (C) 2008  Andy Walls <awalls@radix.net>
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -74,10 +75,14 @@ static int radio[CX18_MAX_CARDS] = { -1, -1, -1, -1, -1, -1, -1, -1,
 				     -1, -1, -1, -1, -1, -1, -1, -1,
 				     -1, -1, -1, -1, -1, -1, -1, -1,
 				     -1, -1, -1, -1, -1, -1, -1, -1 };
-
+static int mmio_ndelay[CX18_MAX_CARDS] = { -1, -1, -1, -1, -1, -1, -1, -1,
+					   -1, -1, -1, -1, -1, -1, -1, -1,
+					   -1, -1, -1, -1, -1, -1, -1, -1,
+					   -1, -1, -1, -1, -1, -1, -1, -1 };
 static unsigned cardtype_c = 1;
 static unsigned tuner_c = 1;
 static unsigned radio_c = 1;
+static int mmio_ndelay_c = 1;
 static char pal[] = "--";
 static char secam[] = "--";
 static char ntsc[] = "-";
@@ -96,6 +101,7 @@ int cx18_debug;
 module_param_array(tuner, int, &tuner_c, 0644);
 module_param_array(radio, bool, &radio_c, 0644);
 module_param_array(cardtype, int, &cardtype_c, 0644);
+module_param_array(mmio_ndelay, int, &mmio_ndelay_c, 0644);
 module_param_string(pal, pal, sizeof(pal), 0644);
 module_param_string(secam, secam, sizeof(secam), 0644);
 module_param_string(ntsc, ntsc, sizeof(ntsc), 0644);
@@ -141,6 +147,11 @@ MODULE_PARM_DESC(debug,
 MODULE_PARM_DESC(cx18_pci_latency,
 		 "Change the PCI latency to 64 if lower: 0 = No, 1 = Yes,\n"
 		 "\t\t\tDefault: Yes");
+MODULE_PARM_DESC(mmio_ndelay,
+		 "Delay (ns) for each CX23418 memory mapped IO access.\n"
+		 "\t\t\tTry larger values that are close to a multiple of the\n"
+		 "\t\t\tPCI clock period, 30.3 ns, if your card doesn't work.\n"
+		 "\t\t\tDefault: " __stringify(CX18_DEFAULT_MMIO_NDELAY));
 MODULE_PARM_DESC(enc_mpg_buffers,
 		 "Encoder MPG Buffers (in MB)\n"
 		 "\t\t\tDefault: " __stringify(CX18_DEFAULT_ENC_MPG_BUFFERS));
@@ -357,6 +368,11 @@ static void cx18_process_options(struct cx18 *cx)
 	cx->options.tuner = tuner[cx->num];
 	cx->options.radio = radio[cx->num];
 
+	if (mmio_ndelay[cx->num] < 0)
+		cx->options.mmio_ndelay = CX18_DEFAULT_MMIO_NDELAY;
+	else
+		cx->options.mmio_ndelay = mmio_ndelay[cx->num];
+
 	cx->std = cx18_parse_std(cx);
 	if (cx->options.cardtype == -1) {
 		CX18_INFO("Ignore card\n");

drivers/media/video/cx18/cx18-driver.h

@@ -63,6 +63,9 @@
 #  error "This driver requires kernel PCI support."
 #endif
 
+/* Default delay to throttle mmio access to the CX23418 so it doesn't choke */
+#define CX18_DEFAULT_MMIO_NDELAY 31 /* 30.3 ns = 1 PCI clock(s) / 33 MHz */
+
 #define CX18_MEM_OFFSET	0x00000000
 #define CX18_MEM_SIZE	0x04000000
 #define CX18_REG_OFFSET	0x02000000
@@ -176,6 +179,7 @@ struct cx18_options {
 	int cardtype;		/* force card type on load */
 	int tuner;		/* set tuner on load */
 	int radio;		/* enable/disable radio */
+	unsigned long mmio_ndelay; /* delay in ns after every PCI mmio access */
 };
 
 /* per-buffer bit flags */

drivers/media/video/cx18/cx18-io.c

@@ -21,76 +21,69 @@
  */
 
 #include "cx18-driver.h"
+#include "cx18-io.h"
 #include "cx18-irq.h"
 
-void cx18_raw_writel(struct cx18 *cx, u32 val, void __iomem *addr)
-{
-	__raw_writel(val, addr);
-}
-
-u32 cx18_raw_readl(struct cx18 *cx, const void __iomem *addr)
-{
-	return __raw_readl(addr);
-}
-
-u32 cx18_write_sync(struct cx18 *cx, u32 val, void __iomem *addr)
-{
-	writel(val, addr);
-	return readl(addr);
-}
-
-void cx18_writel(struct cx18 *cx, u32 val, void __iomem *addr)
-{
-	writel(val, addr);
-}
-
-u32 cx18_readl(struct cx18 *cx, const void __iomem *addr)
-{
-	return readl(addr);
-}
-
-
-/* Access "register" region of CX23418 memory mapped I/O */
-u32 cx18_read_reg(struct cx18 *cx, u32 reg)
-{
-	return readl(cx->reg_mem + reg);
-}
-
-void cx18_write_reg(struct cx18 *cx, u32 val, u32 reg)
-{
-	writel(val, cx->reg_mem + reg);
-}
-
-u32 cx18_write_reg_sync(struct cx18 *cx, u32 val, u32 reg)
-{
-	return cx18_write_sync(cx, val, cx->reg_mem + reg);
-}
-
-/* Access "encoder memory" region of CX23418 memory mapped I/O */
-u32 cx18_read_enc(struct cx18 *cx, u32 addr)
-{
-	return readl(cx->enc_mem + addr);
-}
-
-void cx18_write_enc(struct cx18 *cx, u32 val, u32 addr)
-{
-	writel(val, cx->enc_mem + addr);
-}
-
-u32 cx18_write_enc_sync(struct cx18 *cx, u32 val, u32 addr)
-{
-	return cx18_write_sync(cx, val, cx->enc_mem + addr);
-}
-
 void cx18_memcpy_fromio(struct cx18 *cx, void *to,
 			const void __iomem *from, unsigned int len)
 {
-	memcpy_fromio(to, from, len);
+	/* Align reads on the CX23418's addresses */
+	if ((len > 0) && ((unsigned)from & 1)) {
+		*((u8 *)to) = cx18_readb(cx, from);
+		len--;
+		to++;
+		from++;
+	}
+	if ((len > 1) && ((unsigned)from & 2)) {
+		*((u16 *)to) = cx18_raw_readw(cx, from);
+		len -= 2;
+		to += 2;
+		from += 2;
+	}
+	while (len > 3) {
+		*((u32 *)to) = cx18_raw_readl(cx, from);
+		len -= 4;
+		to += 4;
+		from += 4;
+	}
+	if (len > 1) {
+		*((u16 *)to) = cx18_raw_readw(cx, from);
+		len -= 2;
+		to += 2;
+		from += 2;
+	}
+	if (len > 0)
+		*((u8 *)to) = cx18_readb(cx, from);
 }
 
 void cx18_memset_io(struct cx18 *cx, void __iomem *addr, int val, size_t count)
 {
-	memset_io(addr, val, count);
+	u16 val2 = val | (val << 8);
+	u32 val4 = val2 | (val2 << 16);
+
+	/* Align writes on the CX23418's addresses */
+	if ((count > 0) && ((unsigned)addr & 1)) {
+		cx18_writeb(cx, (u8) val, addr);
+		count--;
+		addr++;
+	}
+	if ((count > 1) && ((unsigned)addr & 2)) {
+		cx18_writew(cx, val2, addr);
+		count -= 2;
+		addr += 2;
+	}
+	while (count > 3) {
+		cx18_writel(cx, val4, addr);
+		count -= 4;
+		addr += 4;
+	}
+	if (count > 1) {
+		cx18_writew(cx, val2, addr);
+		count -= 2;
+		addr += 2;
+	}
+	if (count > 0)
+		cx18_writeb(cx, (u8) val, addr);
 }
 
 void cx18_sw1_irq_enable(struct cx18 *cx, u32 val)

drivers/media/video/cx18/cx18-io.h

@@ -25,38 +25,109 @@
 
 #include "cx18-driver.h"
 
-/* This is a PCI post thing, where if the pci register is not read, then
-   the write doesn't always take effect right away. By reading back the
-   register any pending PCI writes will be performed (in order), and so
-   you can be sure that the writes are guaranteed to be done.
+static inline void cx18_io_delay(struct cx18 *cx)
+{
+	if (cx->options.mmio_ndelay)
+		ndelay(cx->options.mmio_ndelay);
+}
 
-   Rarely needed, only in some timing sensitive cases.
-   Apparently if this is not done some motherboards seem
-   to kill the firmware and get into the broken state until computer is
-   rebooted. */
-u32 cx18_write_sync(struct cx18 *cx, u32 val, void __iomem *addr);
+/* Non byteswapping memory mapped IO */
+static inline void cx18_raw_writel(struct cx18 *cx, u32 val, void __iomem *addr)
+{
+	__raw_writel(val, addr);
+	cx18_io_delay(cx);
+}
 
-void cx18_writel(struct cx18 *cx, u32 val, void __iomem *addr);
-u32 cx18_readl(struct cx18 *cx, const void __iomem *addr);
+static inline u32 cx18_raw_readl(struct cx18 *cx, const void __iomem *addr)
+{
+	u32 ret = __raw_readl(addr);
+	cx18_io_delay(cx);
+	return ret;
+}
 
-/* No endiannes conversion calls */
-void cx18_raw_writel(struct cx18 *cx, u32 val, void __iomem *addr);
-u32 cx18_raw_readl(struct cx18 *cx, const void __iomem *addr);
+static inline u16 cx18_raw_readw(struct cx18 *cx, const void __iomem *addr)
+{
+	u16 ret = __raw_readw(addr);
+	cx18_io_delay(cx);
+	return ret;
+}
 
-/* Access "register" region of CX23418 memory mapped I/O */
-u32 cx18_read_reg(struct cx18 *cx, u32 reg);
-void cx18_write_reg(struct cx18 *cx, u32 val, u32 reg);
-u32 cx18_write_reg_sync(struct cx18 *cx, u32 val, u32 reg);
+/* Normal memory mapped IO */
+static inline void cx18_writel(struct cx18 *cx, u32 val, void __iomem *addr)
+{
+	writel(val, addr);
+	cx18_io_delay(cx);
+}
 
-/* Access "encoder memory" region of CX23418 memory mapped I/O */
-u32 cx18_read_enc(struct cx18 *cx, u32 addr);
-void cx18_write_enc(struct cx18 *cx, u32 val, u32 addr);
-u32 cx18_write_enc_sync(struct cx18 *cx, u32 val, u32 addr);
+static inline void cx18_writew(struct cx18 *cx, u16 val, void __iomem *addr)
+{
+	writew(val, addr);
+	cx18_io_delay(cx);
+}
+
+static inline void cx18_writeb(struct cx18 *cx, u8 val, void __iomem *addr)
+{
+	writeb(val, addr);
+	cx18_io_delay(cx);
+}
+
+static inline u32 cx18_readl(struct cx18 *cx, const void __iomem *addr)
+{
+	u32 ret = readl(addr);
+	cx18_io_delay(cx);
+	return ret;
+}
+
+static inline u8 cx18_readb(struct cx18 *cx, const void __iomem *addr)
+{
+	u8 ret = readb(addr);
+	cx18_io_delay(cx);
+	return ret;
+}
+
+static inline u32 cx18_write_sync(struct cx18 *cx, u32 val, void __iomem *addr)
+{
+	cx18_writel(cx, val, addr);
+	return cx18_readl(cx, addr);
+}
 
 void cx18_memcpy_fromio(struct cx18 *cx, void *to,
 			const void __iomem *from, unsigned int len);
 void cx18_memset_io(struct cx18 *cx, void __iomem *addr, int val, size_t count);
 
+/* Access "register" region of CX23418 memory mapped I/O */
+static inline void cx18_write_reg(struct cx18 *cx, u32 val, u32 reg)
+{
+	cx18_writel(cx, val, cx->reg_mem + reg);
+}
+
+static inline u32 cx18_read_reg(struct cx18 *cx, u32 reg)
+{
+	return cx18_readl(cx, cx->reg_mem + reg);
+}
+
+static inline u32 cx18_write_reg_sync(struct cx18 *cx, u32 val, u32 reg)
+{
+	return cx18_write_sync(cx, val, cx->reg_mem + reg);
+}
+
+/* Access "encoder memory" region of CX23418 memory mapped I/O */
+static inline void cx18_write_enc(struct cx18 *cx, u32 val, u32 addr)
+{
+	cx18_writel(cx, val, cx->enc_mem + addr);
+}
+
+static inline u32 cx18_read_enc(struct cx18 *cx, u32 addr)
+{
+	return cx18_readl(cx, cx->enc_mem + addr);
+}
+
+static inline u32 cx18_write_enc_sync(struct cx18 *cx, u32 val, u32 addr)
+{
+	return cx18_write_sync(cx, val, cx->enc_mem + addr);
+}
+
+
 void cx18_sw1_irq_enable(struct cx18 *cx, u32 val);
 void cx18_sw1_irq_disable(struct cx18 *cx, u32 val);
 void cx18_sw2_irq_enable(struct cx18 *cx, u32 val);

drivers/media/video/cx18/cx18-queue.c

@@ -170,7 +170,6 @@ int cx18_stream_alloc(struct cx18_stream *s)
 		}
 		buf->id = cx->buffer_id++;
 		INIT_LIST_HEAD(&buf->list);
-		/* FIXME - check for mmio */
 		buf->dma_handle = pci_map_single(s->cx->dev,
 				buf->buf, s->buf_size, s->dma);
 		cx18_buf_sync_for_cpu(s, buf);
@@ -194,7 +193,6 @@ void cx18_stream_free(struct cx18_stream *s)
 
 	/* empty q_free */
 	while ((buf = cx18_dequeue(s, &s->q_free))) {
-		/* FIXME - check for mmio */
 		pci_unmap_single(s->cx->dev, buf->dma_handle,
 				s->buf_size, s->dma);
 		kfree(buf->buf);

drivers/media/video/cx18/cx18-queue.h

@@ -28,7 +28,6 @@
 static inline void cx18_buf_sync_for_cpu(struct cx18_stream *s,
 	struct cx18_buffer *buf)
 {
-	/* FIXME check IO transfers */
 	pci_dma_sync_single_for_cpu(s->cx->dev, buf->dma_handle,
 				s->buf_size, s->dma);
 }
@@ -36,7 +35,6 @@ static inline void cx18_buf_sync_for_cpu(struct cx18_stream *s,
 static inline void cx18_buf_sync_for_device(struct cx18_stream *s,
 	struct cx18_buffer *buf)
 {
-	/* FIXME check IO transfers */
 	pci_dma_sync_single_for_device(s->cx->dev, buf->dma_handle,
 				s->buf_size, s->dma);
 }