[PATCH v7 0/5] OMAP: McBSP: Use register cache

[PATCH v7 0/5] OMAP: McBSP: Use register cache

[Janusz Krzysztofik]

Change the way McBSP registers are maintained: store values written to the
device in a cache in order to make use of those cached values when
convenient.

This could help for developing the McBSP context save/restore features, as
well as solve the problem of possible register corruption, experienced on
OMAP1510 based Amstrad Delta board at least.

Series created against linux-omap for-next, commit
82f1d8f22f2c65e70206e40a6f17688bf64a892c.
All patches tested on OMAP1510 based Amstrad Delta and compile-tested using
omap_3430sdp_defconfig at least.

Janusz Krzysztofik (5):
	OMAP: McBSP: Use macros for all register read/write operations
	OMAP: McBSP: Modify macros/functions API for easy cache access
	OMAP: McBSP: Introduce caching in register write operations
	OMAP: McBSP: Use cache when modifying individual register bits
	OMAP: McBSP: Split and move read/write functions to mach-omap1/2

 arch/arm/mach-omap1/mcbsp.c             |   28 +
 arch/arm/mach-omap2/mcbsp.c             |   42 ++
 arch/arm/plat-omap/include/plat/mcbsp.h |    6
 arch/arm/plat-omap/mcbsp.c              |  470 +++++++++++++++-----------------
 4 files changed, 292 insertions(+), 254 deletions(-)

Thanks,
Janusz

[PATCH v7 1/5] OMAP: McBSP: Use macros for all register read/write operations

[Janusz Krzysztofik]

There are several places where readw()/writew() functions are used instead of
OMAP_MCBSP_READ()/WRITE() macros for manipulating McBSP registers. Replace
them with macros to ensure consistent behaviour after caching is introduced.

Created against linux-omap for-next,
commit 82f1d8f22f2c65e70206e40a6f17688bf64a892c.

Tested on OMAP1510 based Amstrad Delta.
Compile-tested with omap_3430sdp_defconfig.

Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>

---
No functional changes since v3.

 arch/arm/plat-omap/mcbsp.c |   44 ++++++++++++++++++++++----------------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff -upr git.orig/arch/arm/plat-omap/mcbsp.c git/arch/arm/plat-omap/mcbsp.c
--- git.orig/arch/arm/plat-omap/mcbsp.c	2009-12-02 15:48:52.000000000 +0100
+++ git/arch/arm/plat-omap/mcbsp.c	2009-12-09 15:10:56.000000000 +0100
@@ -622,26 +622,26 @@ int omap_mcbsp_pollwrite(unsigned int id
 	mcbsp = id_to_mcbsp_ptr(id);
 	base = mcbsp->io_base;
 
-	writew(buf, base + OMAP_MCBSP_REG_DXR1);
+	OMAP_MCBSP_WRITE(base, DXR1, buf);
 	/* if frame sync error - clear the error */
-	if (readw(base + OMAP_MCBSP_REG_SPCR2) & XSYNC_ERR) {
+	if (OMAP_MCBSP_READ(base, SPCR2) & XSYNC_ERR) {
 		/* clear error */
-		writew(readw(base + OMAP_MCBSP_REG_SPCR2) & (~XSYNC_ERR),
-		       base + OMAP_MCBSP_REG_SPCR2);
+		OMAP_MCBSP_WRITE(base, SPCR2,
+				OMAP_MCBSP_READ(base, SPCR2) & (~XSYNC_ERR));
 		/* resend */
 		return -1;
 	} else {
 		/* wait for transmit confirmation */
 		int attemps = 0;
-		while (!(readw(base + OMAP_MCBSP_REG_SPCR2) & XRDY)) {
+		while (!(OMAP_MCBSP_READ(base, SPCR2) & XRDY)) {
 			if (attemps++ > 1000) {
-				writew(readw(base + OMAP_MCBSP_REG_SPCR2) &
-				       (~XRST),
-				       base + OMAP_MCBSP_REG_SPCR2);
+				OMAP_MCBSP_WRITE(base, SPCR2,
+						OMAP_MCBSP_READ(base, SPCR2) &
+						(~XRST));
 				udelay(10);
-				writew(readw(base + OMAP_MCBSP_REG_SPCR2) |
-				       (XRST),
-				       base + OMAP_MCBSP_REG_SPCR2);
+				OMAP_MCBSP_WRITE(base, SPCR2,
+						OMAP_MCBSP_READ(base, SPCR2) |
+						(XRST));
 				udelay(10);
 				dev_err(mcbsp->dev, "Could not write to"
 					" McBSP%d Register\n", mcbsp->id);
@@ -667,24 +667,24 @@ int omap_mcbsp_pollread(unsigned int id,
 
 	base = mcbsp->io_base;
 	/* if frame sync error - clear the error */
-	if (readw(base + OMAP_MCBSP_REG_SPCR1) & RSYNC_ERR) {
+	if (OMAP_MCBSP_READ(base, SPCR1) & RSYNC_ERR) {
 		/* clear error */
-		writew(readw(base + OMAP_MCBSP_REG_SPCR1) & (~RSYNC_ERR),
-		       base + OMAP_MCBSP_REG_SPCR1);
+		OMAP_MCBSP_WRITE(base, SPCR1,
+				OMAP_MCBSP_READ(base, SPCR1) & (~RSYNC_ERR));
 		/* resend */
 		return -1;
 	} else {
 		/* wait for recieve confirmation */
 		int attemps = 0;
-		while (!(readw(base + OMAP_MCBSP_REG_SPCR1) & RRDY)) {
+		while (!(OMAP_MCBSP_READ(base, SPCR1) & RRDY)) {
 			if (attemps++ > 1000) {
-				writew(readw(base + OMAP_MCBSP_REG_SPCR1) &
-				       (~RRST),
-				       base + OMAP_MCBSP_REG_SPCR1);
+				OMAP_MCBSP_WRITE(base, SPCR1,
+						OMAP_MCBSP_READ(base, SPCR1) &
+						(~RRST));
 				udelay(10);
-				writew(readw(base + OMAP_MCBSP_REG_SPCR1) |
-				       (RRST),
-				       base + OMAP_MCBSP_REG_SPCR1);
+				OMAP_MCBSP_WRITE(base, SPCR1,
+						OMAP_MCBSP_READ(base, SPCR1) |
+						(RRST));
 				udelay(10);
 				dev_err(mcbsp->dev, "Could not read from"
 					" McBSP%d Register\n", mcbsp->id);
@@ -692,7 +692,7 @@ int omap_mcbsp_pollread(unsigned int id,
 			}
 		}
 	}
-	*buf = readw(base + OMAP_MCBSP_REG_DRR1);
+	*buf = OMAP_MCBSP_READ(base, DRR1);
 
 	return 0;
 }

[PATCH v7 2/5] OMAP: McBSP: Modify macros/functions API for easy cache access

[Janusz Krzysztofik]

OMAP_MCBSP_READ()/_WRITE() macros and omap_mcbsp_read()/_write() functions
accept McBSP register base address as an argument. In order to support
caching, that must be replaced with an address of the omap_mcbsp structure
that would provide addresses for both register AND cache access.

Since OMAP_ prefix seems obvious in macro names, drop it off in order to
minimize line wrapping throughout the file.

Applies on top of patch 1 from this series:
[PATCH v7 1/5] OMAP: McBSP: Use macros for all register read/write operations

Tested on OMAP1510 based Amstrad Delta using linux-omap for-next,
commit 82f1d8f22f2c65e70206e40a6f17688bf64a892c.
Compile-tested with omap_3430sdp_defconfig.

Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>

---
Changes since v3:
- modify API of omap_mcbsp_read()/_write() functions as well, since those will
  actually provide caching operations, not macros like before.

 arch/arm/plat-omap/mcbsp.c |  281 
++++++++++++++++++++-------------------------
 1 file changed, 125 insertions(+), 156 deletions(-)

diff -upr git.orig/arch/arm/plat-omap/mcbsp.c git/arch/arm/plat-omap/mcbsp.c
--- git.orig/arch/arm/plat-omap/mcbsp.c	2009-12-09 15:28:33.000000000 +0100
+++ git/arch/arm/plat-omap/mcbsp.c	2009-12-09 15:29:16.000000000 +0100
@@ -30,26 +30,26 @@
 struct omap_mcbsp **mcbsp_ptr;
 int omap_mcbsp_count;
 
-void omap_mcbsp_write(void __iomem *io_base, u16 reg, u32 val)
+void omap_mcbsp_write(struct omap_mcbsp *mcbsp, u16 reg, u32 val)
 {
 	if (cpu_class_is_omap1() || cpu_is_omap2420())
-		__raw_writew((u16)val, io_base + reg);
+		__raw_writew((u16)val, mcbsp->io_base + reg);
 	else
-		__raw_writel(val, io_base + reg);
+		__raw_writel(val, mcbsp->io_base + reg);
 }
 
-int omap_mcbsp_read(void __iomem *io_base, u16 reg)
+int omap_mcbsp_read(struct omap_mcbsp *mcbsp, u16 reg)
 {
 	if (cpu_class_is_omap1() || cpu_is_omap2420())
-		return __raw_readw(io_base + reg);
+		return __raw_readw(mcbsp->io_base + reg);
 	else
-		return __raw_readl(io_base + reg);
+		return __raw_readl(mcbsp->io_base + reg);
 }
 
-#define OMAP_MCBSP_READ(base, reg) \
-			omap_mcbsp_read(base, OMAP_MCBSP_REG_##reg)
-#define OMAP_MCBSP_WRITE(base, reg, val) \
-			omap_mcbsp_write(base, OMAP_MCBSP_REG_##reg, val)
+#define MCBSP_READ(mcbsp, reg) \
+		omap_mcbsp_read(mcbsp, OMAP_MCBSP_REG_##reg)
+#define MCBSP_WRITE(mcbsp, reg, val) \
+		omap_mcbsp_write(mcbsp, OMAP_MCBSP_REG_##reg, val)
 
 #define omap_mcbsp_check_valid_id(id)	(id < omap_mcbsp_count)
 #define id_to_mcbsp_ptr(id)		mcbsp_ptr[id];
@@ -60,31 +60,31 @@ static void omap_mcbsp_dump_reg(u8 id)
 
 	dev_dbg(mcbsp->dev, "**** McBSP%d regs ****\n", mcbsp->id);
 	dev_dbg(mcbsp->dev, "DRR2:  0x%04x\n",
-			OMAP_MCBSP_READ(mcbsp->io_base, DRR2));
+			MCBSP_READ(mcbsp, DRR2));
 	dev_dbg(mcbsp->dev, "DRR1:  0x%04x\n",
-			OMAP_MCBSP_READ(mcbsp->io_base, DRR1));
+			MCBSP_READ(mcbsp, DRR1));
 	dev_dbg(mcbsp->dev, "DXR2:  0x%04x\n",
-			OMAP_MCBSP_READ(mcbsp->io_base, DXR2));
+			MCBSP_READ(mcbsp, DXR2));
 	dev_dbg(mcbsp->dev, "DXR1:  0x%04x\n",
-			OMAP_MCBSP_READ(mcbsp->io_base, DXR1));
+			MCBSP_READ(mcbsp, DXR1));
 	dev_dbg(mcbsp->dev, "SPCR2: 0x%04x\n",
-			OMAP_MCBSP_READ(mcbsp->io_base, SPCR2));
+			MCBSP_READ(mcbsp, SPCR2));
 	dev_dbg(mcbsp->dev, "SPCR1: 0x%04x\n",
-			OMAP_MCBSP_READ(mcbsp->io_base, SPCR1));
+			MCBSP_READ(mcbsp, SPCR1));
 	dev_dbg(mcbsp->dev, "RCR2:  0x%04x\n",
-			OMAP_MCBSP_READ(mcbsp->io_base, RCR2));
+			MCBSP_READ(mcbsp, RCR2));
 	dev_dbg(mcbsp->dev, "RCR1:  0x%04x\n",
-			OMAP_MCBSP_READ(mcbsp->io_base, RCR1));
+			MCBSP_READ(mcbsp, RCR1));
 	dev_dbg(mcbsp->dev, "XCR2:  0x%04x\n",
-			OMAP_MCBSP_READ(mcbsp->io_base, XCR2));
+			MCBSP_READ(mcbsp, XCR2));
 	dev_dbg(mcbsp->dev, "XCR1:  0x%04x\n",
-			OMAP_MCBSP_READ(mcbsp->io_base, XCR1));
+			MCBSP_READ(mcbsp, XCR1));
 	dev_dbg(mcbsp->dev, "SRGR2: 0x%04x\n",
-			OMAP_MCBSP_READ(mcbsp->io_base, SRGR2));
+			MCBSP_READ(mcbsp, SRGR2));
 	dev_dbg(mcbsp->dev, "SRGR1: 0x%04x\n",
-			OMAP_MCBSP_READ(mcbsp->io_base, SRGR1));
+			MCBSP_READ(mcbsp, SRGR1));
 	dev_dbg(mcbsp->dev, "PCR0:  0x%04x\n",
-			OMAP_MCBSP_READ(mcbsp->io_base, PCR0));
+			MCBSP_READ(mcbsp, PCR0));
 	dev_dbg(mcbsp->dev, "***********************\n");
 }
 
@@ -93,15 +93,14 @@ static irqreturn_t omap_mcbsp_tx_irq_han
 	struct omap_mcbsp *mcbsp_tx = dev_id;
 	u16 irqst_spcr2;
 
-	irqst_spcr2 = OMAP_MCBSP_READ(mcbsp_tx->io_base, SPCR2);
+	irqst_spcr2 = MCBSP_READ(mcbsp_tx, SPCR2);
 	dev_dbg(mcbsp_tx->dev, "TX IRQ callback : 0x%x\n", irqst_spcr2);
 
 	if (irqst_spcr2 & XSYNC_ERR) {
 		dev_err(mcbsp_tx->dev, "TX Frame Sync Error! : 0x%x\n",
 			irqst_spcr2);
 		/* Writing zero to XSYNC_ERR clears the IRQ */
-		OMAP_MCBSP_WRITE(mcbsp_tx->io_base, SPCR2,
-			irqst_spcr2 & ~(XSYNC_ERR));
+		MCBSP_WRITE(mcbsp_tx, SPCR2, irqst_spcr2 & ~(XSYNC_ERR));
 	} else {
 		complete(&mcbsp_tx->tx_irq_completion);
 	}
@@ -114,15 +113,14 @@ static irqreturn_t omap_mcbsp_rx_irq_han
 	struct omap_mcbsp *mcbsp_rx = dev_id;
 	u16 irqst_spcr1;
 
-	irqst_spcr1 = OMAP_MCBSP_READ(mcbsp_rx->io_base, SPCR1);
+	irqst_spcr1 = MCBSP_READ(mcbsp_rx, SPCR1);
 	dev_dbg(mcbsp_rx->dev, "RX IRQ callback : 0x%x\n", irqst_spcr1);
 
 	if (irqst_spcr1 & RSYNC_ERR) {
 		dev_err(mcbsp_rx->dev, "RX Frame Sync Error! : 0x%x\n",
 			irqst_spcr1);
 		/* Writing zero to RSYNC_ERR clears the IRQ */
-		OMAP_MCBSP_WRITE(mcbsp_rx->io_base, SPCR1,
-			irqst_spcr1 & ~(RSYNC_ERR));
+		MCBSP_WRITE(mcbsp_rx, SPCR1, irqst_spcr1 & ~(RSYNC_ERR));
 	} else {
 		complete(&mcbsp_rx->tx_irq_completion);
 	}
@@ -135,7 +133,7 @@ static void omap_mcbsp_tx_dma_callback(i
 	struct omap_mcbsp *mcbsp_dma_tx = data;
 
 	dev_dbg(mcbsp_dma_tx->dev, "TX DMA callback : 0x%x\n",
-		OMAP_MCBSP_READ(mcbsp_dma_tx->io_base, SPCR2));
+		MCBSP_READ(mcbsp_dma_tx, SPCR2));
 
 	/* We can free the channels */
 	omap_free_dma(mcbsp_dma_tx->dma_tx_lch);
@@ -149,7 +147,7 @@ static void omap_mcbsp_rx_dma_callback(i
 	struct omap_mcbsp *mcbsp_dma_rx = data;
 
 	dev_dbg(mcbsp_dma_rx->dev, "RX DMA callback : 0x%x\n",
-		OMAP_MCBSP_READ(mcbsp_dma_rx->io_base, SPCR2));
+		MCBSP_READ(mcbsp_dma_rx, SPCR2));
 
 	/* We can free the channels */
 	omap_free_dma(mcbsp_dma_rx->dma_rx_lch);
@@ -167,7 +165,6 @@ static void omap_mcbsp_rx_dma_callback(i
 void omap_mcbsp_config(unsigned int id, const struct omap_mcbsp_reg_cfg 
*config)
 {
 	struct omap_mcbsp *mcbsp;
-	void __iomem *io_base;
 
 	if (!omap_mcbsp_check_valid_id(id)) {
 		printk(KERN_ERR "%s: Invalid id (%d)\n", __func__, id + 1);
@@ -175,25 +172,24 @@ void omap_mcbsp_config(unsigned int id, 
 	}
 	mcbsp = id_to_mcbsp_ptr(id);
 
-	io_base = mcbsp->io_base;
 	dev_dbg(mcbsp->dev, "Configuring McBSP%d  phys_base: 0x%08lx\n",
 			mcbsp->id, mcbsp->phys_base);
 
 	/* We write the given config */
-	OMAP_MCBSP_WRITE(io_base, SPCR2, config->spcr2);
-	OMAP_MCBSP_WRITE(io_base, SPCR1, config->spcr1);
-	OMAP_MCBSP_WRITE(io_base, RCR2, config->rcr2);
-	OMAP_MCBSP_WRITE(io_base, RCR1, config->rcr1);
-	OMAP_MCBSP_WRITE(io_base, XCR2, config->xcr2);
-	OMAP_MCBSP_WRITE(io_base, XCR1, config->xcr1);
-	OMAP_MCBSP_WRITE(io_base, SRGR2, config->srgr2);
-	OMAP_MCBSP_WRITE(io_base, SRGR1, config->srgr1);
-	OMAP_MCBSP_WRITE(io_base, MCR2, config->mcr2);
-	OMAP_MCBSP_WRITE(io_base, MCR1, config->mcr1);
-	OMAP_MCBSP_WRITE(io_base, PCR0, config->pcr0);
+	MCBSP_WRITE(mcbsp, SPCR2, config->spcr2);
+	MCBSP_WRITE(mcbsp, SPCR1, config->spcr1);
+	MCBSP_WRITE(mcbsp, RCR2, config->rcr2);
+	MCBSP_WRITE(mcbsp, RCR1, config->rcr1);
+	MCBSP_WRITE(mcbsp, XCR2, config->xcr2);
+	MCBSP_WRITE(mcbsp, XCR1, config->xcr1);
+	MCBSP_WRITE(mcbsp, SRGR2, config->srgr2);
+	MCBSP_WRITE(mcbsp, SRGR1, config->srgr1);
+	MCBSP_WRITE(mcbsp, MCR2, config->mcr2);
+	MCBSP_WRITE(mcbsp, MCR1, config->mcr1);
+	MCBSP_WRITE(mcbsp, PCR0, config->pcr0);
 	if (cpu_is_omap2430() || cpu_is_omap34xx() || cpu_is_omap44xx()) {
-		OMAP_MCBSP_WRITE(io_base, XCCR, config->xccr);
-		OMAP_MCBSP_WRITE(io_base, RCCR, config->rccr);
+		MCBSP_WRITE(mcbsp, XCCR, config->xccr);
+		MCBSP_WRITE(mcbsp, RCCR, config->rccr);
 	}
 }
 EXPORT_SYMBOL(omap_mcbsp_config);
@@ -207,7 +203,6 @@ EXPORT_SYMBOL(omap_mcbsp_config);
 void omap_mcbsp_set_tx_threshold(unsigned int id, u16 threshold)
 {
 	struct omap_mcbsp *mcbsp;
-	void __iomem *io_base;
 
 	if (!cpu_is_omap34xx())
 		return;
@@ -217,9 +212,8 @@ void omap_mcbsp_set_tx_threshold(unsigne
 		return;
 	}
 	mcbsp = id_to_mcbsp_ptr(id);
-	io_base = mcbsp->io_base;
 
-	OMAP_MCBSP_WRITE(io_base, THRSH2, threshold);
+	MCBSP_WRITE(mcbsp, THRSH2, threshold);
 }
 EXPORT_SYMBOL(omap_mcbsp_set_tx_threshold);
 
@@ -231,7 +225,6 @@ EXPORT_SYMBOL(omap_mcbsp_set_tx_threshol
 void omap_mcbsp_set_rx_threshold(unsigned int id, u16 threshold)
 {
 	struct omap_mcbsp *mcbsp;
-	void __iomem *io_base;
 
 	if (!cpu_is_omap34xx())
 		return;
@@ -241,9 +234,8 @@ void omap_mcbsp_set_rx_threshold(unsigne
 		return;
 	}
 	mcbsp = id_to_mcbsp_ptr(id);
-	io_base = mcbsp->io_base;
 
-	OMAP_MCBSP_WRITE(io_base, THRSH1, threshold);
+	MCBSP_WRITE(mcbsp, THRSH1, threshold);
 }
 EXPORT_SYMBOL(omap_mcbsp_set_rx_threshold);
 
@@ -313,19 +305,18 @@ static inline void omap34xx_mcbsp_reques
 	if (cpu_is_omap34xx()) {
 		u16 syscon;
 
-		syscon = OMAP_MCBSP_READ(mcbsp->io_base, SYSCON);
+		syscon = MCBSP_READ(mcbsp, SYSCON);
 		syscon &= ~(ENAWAKEUP | SIDLEMODE(0x03) | CLOCKACTIVITY(0x03));
 
 		if (mcbsp->dma_op_mode == MCBSP_DMA_MODE_THRESHOLD) {
 			syscon |= (ENAWAKEUP | SIDLEMODE(0x02) |
 					CLOCKACTIVITY(0x02));
-			OMAP_MCBSP_WRITE(mcbsp->io_base, WAKEUPEN,
-					XRDYEN | RRDYEN);
+			MCBSP_WRITE(mcbsp, WAKEUPEN, XRDYEN | RRDYEN);
 		} else {
 			syscon |= SIDLEMODE(0x01);
 		}
 
-		OMAP_MCBSP_WRITE(mcbsp->io_base, SYSCON, syscon);
+		MCBSP_WRITE(mcbsp, SYSCON, syscon);
 	}
 }
 
@@ -337,7 +328,7 @@ static inline void omap34xx_mcbsp_free(s
 	if (cpu_is_omap34xx()) {
 		u16 syscon;
 
-		syscon = OMAP_MCBSP_READ(mcbsp->io_base, SYSCON);
+		syscon = MCBSP_READ(mcbsp, SYSCON);
 		syscon &= ~(ENAWAKEUP | SIDLEMODE(0x03) | CLOCKACTIVITY(0x03));
 		/*
 		 * HW bug workaround - If no_idle mode is taken, we need to
@@ -345,12 +336,12 @@ static inline void omap34xx_mcbsp_free(s
 		 * device will not hit retention anymore.
 		 */
 		syscon |= SIDLEMODE(0x02);
-		OMAP_MCBSP_WRITE(mcbsp->io_base, SYSCON, syscon);
+		MCBSP_WRITE(mcbsp, SYSCON, syscon);
 
 		syscon &= ~(SIDLEMODE(0x03));
-		OMAP_MCBSP_WRITE(mcbsp->io_base, SYSCON, syscon);
+		MCBSP_WRITE(mcbsp, SYSCON, syscon);
 
-		OMAP_MCBSP_WRITE(mcbsp->io_base, WAKEUPEN, 0);
+		MCBSP_WRITE(mcbsp, WAKEUPEN, 0);
 	}
 }
 #else
@@ -424,8 +415,8 @@ int omap_mcbsp_request(unsigned int id)
 	 * Make sure that transmitter, receiver and sample-rate generator are
 	 * not running before activating IRQs.
 	 */
-	OMAP_MCBSP_WRITE(mcbsp->io_base, SPCR1, 0);
-	OMAP_MCBSP_WRITE(mcbsp->io_base, SPCR2, 0);
+	MCBSP_WRITE(mcbsp, SPCR1, 0);
+	MCBSP_WRITE(mcbsp, SPCR2, 0);
 
 	if (mcbsp->io_type == OMAP_MCBSP_IRQ_IO) {
 		/* We need to get IRQs here */
@@ -501,7 +492,6 @@ EXPORT_SYMBOL(omap_mcbsp_free);
 void omap_mcbsp_start(unsigned int id, int tx, int rx)
 {
 	struct omap_mcbsp *mcbsp;
-	void __iomem *io_base;
 	int idle;
 	u16 w;
 
@@ -510,28 +500,26 @@ void omap_mcbsp_start(unsigned int id, i
 		return;
 	}
 	mcbsp = id_to_mcbsp_ptr(id);
-	io_base = mcbsp->io_base;
 
-	mcbsp->rx_word_length = (OMAP_MCBSP_READ(io_base, RCR1) >> 5) & 0x7;
-	mcbsp->tx_word_length = (OMAP_MCBSP_READ(io_base, XCR1) >> 5) & 0x7;
+	mcbsp->rx_word_length = (MCBSP_READ(mcbsp, RCR1) >> 5) & 0x7;
+	mcbsp->tx_word_length = (MCBSP_READ(mcbsp, XCR1) >> 5) & 0x7;
 
-	idle = !((OMAP_MCBSP_READ(io_base, SPCR2) |
-		  OMAP_MCBSP_READ(io_base, SPCR1)) & 1);
+	idle = !((MCBSP_READ(mcbsp, SPCR2) | MCBSP_READ(mcbsp, SPCR1)) & 1);
 
 	if (idle) {
 		/* Start the sample generator */
-		w = OMAP_MCBSP_READ(io_base, SPCR2);
-		OMAP_MCBSP_WRITE(io_base, SPCR2, w | (1 << 6));
+		w = MCBSP_READ(mcbsp, SPCR2);
+		MCBSP_WRITE(mcbsp, SPCR2, w | (1 << 6));
 	}
 
 	/* Enable transmitter and receiver */
 	tx &= 1;
-	w = OMAP_MCBSP_READ(io_base, SPCR2);
-	OMAP_MCBSP_WRITE(io_base, SPCR2, w | tx);
+	w = MCBSP_READ(mcbsp, SPCR2);
+	MCBSP_WRITE(mcbsp, SPCR2, w | tx);
 
 	rx &= 1;
-	w = OMAP_MCBSP_READ(io_base, SPCR1);
-	OMAP_MCBSP_WRITE(io_base, SPCR1, w | rx);
+	w = MCBSP_READ(mcbsp, SPCR1);
+	MCBSP_WRITE(mcbsp, SPCR1, w | rx);
 
 	/*
 	 * Worst case: CLKSRG*2 = 8000khz: (1/8000) * 2 * 2 usec
@@ -543,18 +531,18 @@ void omap_mcbsp_start(unsigned int id, i
 
 	if (idle) {
 		/* Start frame sync */
-		w = OMAP_MCBSP_READ(io_base, SPCR2);
-		OMAP_MCBSP_WRITE(io_base, SPCR2, w | (1 << 7));
+		w = MCBSP_READ(mcbsp, SPCR2);
+		MCBSP_WRITE(mcbsp, SPCR2, w | (1 << 7));
 	}
 
 	if (cpu_is_omap2430() || cpu_is_omap34xx()) {
 		/* Release the transmitter and receiver */
-		w = OMAP_MCBSP_READ(io_base, XCCR);
+		w = MCBSP_READ(mcbsp, XCCR);
 		w &= ~(tx ? XDISABLE : 0);
-		OMAP_MCBSP_WRITE(io_base, XCCR, w);
-		w = OMAP_MCBSP_READ(io_base, RCCR);
+		MCBSP_WRITE(mcbsp, XCCR, w);
+		w = MCBSP_READ(mcbsp, RCCR);
 		w &= ~(rx ? RDISABLE : 0);
-		OMAP_MCBSP_WRITE(io_base, RCCR, w);
+		MCBSP_WRITE(mcbsp, RCCR, w);
 	}
 
 	/* Dump McBSP Regs */
@@ -565,7 +553,6 @@ EXPORT_SYMBOL(omap_mcbsp_start);
 void omap_mcbsp_stop(unsigned int id, int tx, int rx)
 {
 	struct omap_mcbsp *mcbsp;
-	void __iomem *io_base;
 	int idle;
 	u16 w;
 
@@ -575,35 +562,33 @@ void omap_mcbsp_stop(unsigned int id, in
 	}
 
 	mcbsp = id_to_mcbsp_ptr(id);
-	io_base = mcbsp->io_base;
 
 	/* Reset transmitter */
 	tx &= 1;
 	if (cpu_is_omap2430() || cpu_is_omap34xx()) {
-		w = OMAP_MCBSP_READ(io_base, XCCR);
+		w = MCBSP_READ(mcbsp, XCCR);
 		w |= (tx ? XDISABLE : 0);
-		OMAP_MCBSP_WRITE(io_base, XCCR, w);
+		MCBSP_WRITE(mcbsp, XCCR, w);
 	}
-	w = OMAP_MCBSP_READ(io_base, SPCR2);
-	OMAP_MCBSP_WRITE(io_base, SPCR2, w & ~tx);
+	w = MCBSP_READ(mcbsp, SPCR2);
+	MCBSP_WRITE(mcbsp, SPCR2, w & ~tx);
 
 	/* Reset receiver */
 	rx &= 1;
 	if (cpu_is_omap2430() || cpu_is_omap34xx()) {
-		w = OMAP_MCBSP_READ(io_base, RCCR);
+		w = MCBSP_READ(mcbsp, RCCR);
 		w |= (rx ? RDISABLE : 0);
-		OMAP_MCBSP_WRITE(io_base, RCCR, w);
+		MCBSP_WRITE(mcbsp, RCCR, w);
 	}
-	w = OMAP_MCBSP_READ(io_base, SPCR1);
-	OMAP_MCBSP_WRITE(io_base, SPCR1, w & ~rx);
+	w = MCBSP_READ(mcbsp, SPCR1);
+	MCBSP_WRITE(mcbsp, SPCR1, w & ~rx);
 
-	idle = !((OMAP_MCBSP_READ(io_base, SPCR2) |
-		  OMAP_MCBSP_READ(io_base, SPCR1)) & 1);
+	idle = !((MCBSP_READ(mcbsp, SPCR2) | MCBSP_READ(mcbsp, SPCR1)) & 1);
 
 	if (idle) {
 		/* Reset the sample rate generator */
-		w = OMAP_MCBSP_READ(io_base, SPCR2);
-		OMAP_MCBSP_WRITE(io_base, SPCR2, w & ~(1 << 6));
+		w = MCBSP_READ(mcbsp, SPCR2);
+		MCBSP_WRITE(mcbsp, SPCR2, w & ~(1 << 6));
 	}
 }
 EXPORT_SYMBOL(omap_mcbsp_stop);
@@ -612,7 +597,6 @@ EXPORT_SYMBOL(omap_mcbsp_stop);
 int omap_mcbsp_pollwrite(unsigned int id, u16 buf)
 {
 	struct omap_mcbsp *mcbsp;
-	void __iomem *base;
 
 	if (!omap_mcbsp_check_valid_id(id)) {
 		printk(KERN_ERR "%s: Invalid id (%d)\n", __func__, id + 1);
@@ -620,28 +604,25 @@ int omap_mcbsp_pollwrite(unsigned int id
 	}
 
 	mcbsp = id_to_mcbsp_ptr(id);
-	base = mcbsp->io_base;
 
-	OMAP_MCBSP_WRITE(base, DXR1, buf);
+	MCBSP_WRITE(mcbsp, DXR1, buf);
 	/* if frame sync error - clear the error */
-	if (OMAP_MCBSP_READ(base, SPCR2) & XSYNC_ERR) {
+	if (MCBSP_READ(mcbsp, SPCR2) & XSYNC_ERR) {
 		/* clear error */
-		OMAP_MCBSP_WRITE(base, SPCR2,
-				OMAP_MCBSP_READ(base, SPCR2) & (~XSYNC_ERR));
+		MCBSP_WRITE(mcbsp, SPCR2,
+				MCBSP_READ(mcbsp, SPCR2) & (~XSYNC_ERR));
 		/* resend */
 		return -1;
 	} else {
 		/* wait for transmit confirmation */
 		int attemps = 0;
-		while (!(OMAP_MCBSP_READ(base, SPCR2) & XRDY)) {
+		while (!(MCBSP_READ(mcbsp, SPCR2) & XRDY)) {
 			if (attemps++ > 1000) {
-				OMAP_MCBSP_WRITE(base, SPCR2,
-						OMAP_MCBSP_READ(base, SPCR2) &
-						(~XRST));
+				MCBSP_WRITE(mcbsp, SPCR2,
+					    MCBSP_READ(mcbsp, SPCR2) & (~XRST));
 				udelay(10);
-				OMAP_MCBSP_WRITE(base, SPCR2,
-						OMAP_MCBSP_READ(base, SPCR2) |
-						(XRST));
+				MCBSP_WRITE(mcbsp, SPCR2,
+					    MCBSP_READ(mcbsp, SPCR2) | (XRST));
 				udelay(10);
 				dev_err(mcbsp->dev, "Could not write to"
 					" McBSP%d Register\n", mcbsp->id);
@@ -657,7 +638,6 @@ EXPORT_SYMBOL(omap_mcbsp_pollwrite);
 int omap_mcbsp_pollread(unsigned int id, u16 *buf)
 {
 	struct omap_mcbsp *mcbsp;
-	void __iomem *base;
 
 	if (!omap_mcbsp_check_valid_id(id)) {
 		printk(KERN_ERR "%s: Invalid id (%d)\n", __func__, id + 1);
@@ -665,26 +645,23 @@ int omap_mcbsp_pollread(unsigned int id,
 	}
 	mcbsp = id_to_mcbsp_ptr(id);
 
-	base = mcbsp->io_base;
 	/* if frame sync error - clear the error */
-	if (OMAP_MCBSP_READ(base, SPCR1) & RSYNC_ERR) {
+	if (MCBSP_READ(mcbsp, SPCR1) & RSYNC_ERR) {
 		/* clear error */
-		OMAP_MCBSP_WRITE(base, SPCR1,
-				OMAP_MCBSP_READ(base, SPCR1) & (~RSYNC_ERR));
+		MCBSP_WRITE(mcbsp, SPCR1,
+				MCBSP_READ(mcbsp, SPCR1) & (~RSYNC_ERR));
 		/* resend */
 		return -1;
 	} else {
 		/* wait for recieve confirmation */
 		int attemps = 0;
-		while (!(OMAP_MCBSP_READ(base, SPCR1) & RRDY)) {
+		while (!(MCBSP_READ(mcbsp, SPCR1) & RRDY)) {
 			if (attemps++ > 1000) {
-				OMAP_MCBSP_WRITE(base, SPCR1,
-						OMAP_MCBSP_READ(base, SPCR1) &
-						(~RRST));
+				MCBSP_WRITE(mcbsp, SPCR1,
+					    MCBSP_READ(mcbsp, SPCR1) & (~RRST));
 				udelay(10);
-				OMAP_MCBSP_WRITE(base, SPCR1,
-						OMAP_MCBSP_READ(base, SPCR1) |
-						(RRST));
+				MCBSP_WRITE(mcbsp, SPCR1,
+					    MCBSP_READ(mcbsp, SPCR1) | (RRST));
 				udelay(10);
 				dev_err(mcbsp->dev, "Could not read from"
 					" McBSP%d Register\n", mcbsp->id);
@@ -692,7 +669,7 @@ int omap_mcbsp_pollread(unsigned int id,
 			}
 		}
 	}
-	*buf = OMAP_MCBSP_READ(base, DRR1);
+	*buf = MCBSP_READ(mcbsp, DRR1);
 
 	return 0;
 }
@@ -704,7 +681,6 @@ EXPORT_SYMBOL(omap_mcbsp_pollread);
 void omap_mcbsp_xmit_word(unsigned int id, u32 word)
 {
 	struct omap_mcbsp *mcbsp;
-	void __iomem *io_base;
 	omap_mcbsp_word_length word_length;
 
 	if (!omap_mcbsp_check_valid_id(id)) {
@@ -713,21 +689,19 @@ void omap_mcbsp_xmit_word(unsigned int i
 	}
 
 	mcbsp = id_to_mcbsp_ptr(id);
-	io_base = mcbsp->io_base;
 	word_length = mcbsp->tx_word_length;
 
 	wait_for_completion(&mcbsp->tx_irq_completion);
 
 	if (word_length > OMAP_MCBSP_WORD_16)
-		OMAP_MCBSP_WRITE(io_base, DXR2, word >> 16);
-	OMAP_MCBSP_WRITE(io_base, DXR1, word & 0xffff);
+		MCBSP_WRITE(mcbsp, DXR2, word >> 16);
+	MCBSP_WRITE(mcbsp, DXR1, word & 0xffff);
 }
 EXPORT_SYMBOL(omap_mcbsp_xmit_word);
 
 u32 omap_mcbsp_recv_word(unsigned int id)
 {
 	struct omap_mcbsp *mcbsp;
-	void __iomem *io_base;
 	u16 word_lsb, word_msb = 0;
 	omap_mcbsp_word_length word_length;
 
@@ -738,13 +712,12 @@ u32 omap_mcbsp_recv_word(unsigned int id
 	mcbsp = id_to_mcbsp_ptr(id);
 
 	word_length = mcbsp->rx_word_length;
-	io_base = mcbsp->io_base;
 
 	wait_for_completion(&mcbsp->rx_irq_completion);
 
 	if (word_length > OMAP_MCBSP_WORD_16)
-		word_msb = OMAP_MCBSP_READ(io_base, DRR2);
-	word_lsb = OMAP_MCBSP_READ(io_base, DRR1);
+		word_msb = MCBSP_READ(mcbsp, DRR2);
+	word_lsb = MCBSP_READ(mcbsp, DRR1);
 
 	return (word_lsb | (word_msb << 16));
 }
@@ -753,7 +726,6 @@ EXPORT_SYMBOL(omap_mcbsp_recv_word);
 int omap_mcbsp_spi_master_xmit_word_poll(unsigned int id, u32 word)
 {
 	struct omap_mcbsp *mcbsp;
-	void __iomem *io_base;
 	omap_mcbsp_word_length tx_word_length;
 	omap_mcbsp_word_length rx_word_length;
 	u16 spcr2, spcr1, attempts = 0, word_lsb, word_msb = 0;
@@ -763,7 +735,6 @@ int omap_mcbsp_spi_master_xmit_word_poll
 		return -ENODEV;
 	}
 	mcbsp = id_to_mcbsp_ptr(id);
-	io_base = mcbsp->io_base;
 	tx_word_length = mcbsp->tx_word_length;
 	rx_word_length = mcbsp->rx_word_length;
 
@@ -771,14 +742,14 @@ int omap_mcbsp_spi_master_xmit_word_poll
 		return -EINVAL;
 
 	/* First we wait for the transmitter to be ready */
-	spcr2 = OMAP_MCBSP_READ(io_base, SPCR2);
+	spcr2 = MCBSP_READ(mcbsp, SPCR2);
 	while (!(spcr2 & XRDY)) {
-		spcr2 = OMAP_MCBSP_READ(io_base, SPCR2);
+		spcr2 = MCBSP_READ(mcbsp, SPCR2);
 		if (attempts++ > 1000) {
 			/* We must reset the transmitter */
-			OMAP_MCBSP_WRITE(io_base, SPCR2, spcr2 & (~XRST));
+			MCBSP_WRITE(mcbsp, SPCR2, spcr2 & (~XRST));
 			udelay(10);
-			OMAP_MCBSP_WRITE(io_base, SPCR2, spcr2 | XRST);
+			MCBSP_WRITE(mcbsp, SPCR2, spcr2 | XRST);
 			udelay(10);
 			dev_err(mcbsp->dev, "McBSP%d transmitter not "
 				"ready\n", mcbsp->id);
@@ -788,18 +759,18 @@ int omap_mcbsp_spi_master_xmit_word_poll
 
 	/* Now we can push the data */
 	if (tx_word_length > OMAP_MCBSP_WORD_16)
-		OMAP_MCBSP_WRITE(io_base, DXR2, word >> 16);
-	OMAP_MCBSP_WRITE(io_base, DXR1, word & 0xffff);
+		MCBSP_WRITE(mcbsp, DXR2, word >> 16);
+	MCBSP_WRITE(mcbsp, DXR1, word & 0xffff);
 
 	/* We wait for the receiver to be ready */
-	spcr1 = OMAP_MCBSP_READ(io_base, SPCR1);
+	spcr1 = MCBSP_READ(mcbsp, SPCR1);
 	while (!(spcr1 & RRDY)) {
-		spcr1 = OMAP_MCBSP_READ(io_base, SPCR1);
+		spcr1 = MCBSP_READ(mcbsp, SPCR1);
 		if (attempts++ > 1000) {
 			/* We must reset the receiver */
-			OMAP_MCBSP_WRITE(io_base, SPCR1, spcr1 & (~RRST));
+			MCBSP_WRITE(mcbsp, SPCR1, spcr1 & (~RRST));
 			udelay(10);
-			OMAP_MCBSP_WRITE(io_base, SPCR1, spcr1 | RRST);
+			MCBSP_WRITE(mcbsp, SPCR1, spcr1 | RRST);
 			udelay(10);
 			dev_err(mcbsp->dev, "McBSP%d receiver not "
 				"ready\n", mcbsp->id);
@@ -809,8 +780,8 @@ int omap_mcbsp_spi_master_xmit_word_poll
 
 	/* Receiver is ready, let's read the dummy data */
 	if (rx_word_length > OMAP_MCBSP_WORD_16)
-		word_msb = OMAP_MCBSP_READ(io_base, DRR2);
-	word_lsb = OMAP_MCBSP_READ(io_base, DRR1);
+		word_msb = MCBSP_READ(mcbsp, DRR2);
+	word_lsb = MCBSP_READ(mcbsp, DRR1);
 
 	return 0;
 }
@@ -820,7 +791,6 @@ int omap_mcbsp_spi_master_recv_word_poll
 {
 	struct omap_mcbsp *mcbsp;
 	u32 clock_word = 0;
-	void __iomem *io_base;
 	omap_mcbsp_word_length tx_word_length;
 	omap_mcbsp_word_length rx_word_length;
 	u16 spcr2, spcr1, attempts = 0, word_lsb, word_msb = 0;
@@ -831,7 +801,6 @@ int omap_mcbsp_spi_master_recv_word_poll
 	}
 
 	mcbsp = id_to_mcbsp_ptr(id);
-	io_base = mcbsp->io_base;
 
 	tx_word_length = mcbsp->tx_word_length;
 	rx_word_length = mcbsp->rx_word_length;
@@ -840,14 +809,14 @@ int omap_mcbsp_spi_master_recv_word_poll
 		return -EINVAL;
 
 	/* First we wait for the transmitter to be ready */
-	spcr2 = OMAP_MCBSP_READ(io_base, SPCR2);
+	spcr2 = MCBSP_READ(mcbsp, SPCR2);
 	while (!(spcr2 & XRDY)) {
-		spcr2 = OMAP_MCBSP_READ(io_base, SPCR2);
+		spcr2 = MCBSP_READ(mcbsp, SPCR2);
 		if (attempts++ > 1000) {
 			/* We must reset the transmitter */
-			OMAP_MCBSP_WRITE(io_base, SPCR2, spcr2 & (~XRST));
+			MCBSP_WRITE(mcbsp, SPCR2, spcr2 & (~XRST));
 			udelay(10);
-			OMAP_MCBSP_WRITE(io_base, SPCR2, spcr2 | XRST);
+			MCBSP_WRITE(mcbsp, SPCR2, spcr2 | XRST);
 			udelay(10);
 			dev_err(mcbsp->dev, "McBSP%d transmitter not "
 				"ready\n", mcbsp->id);
@@ -857,18 +826,18 @@ int omap_mcbsp_spi_master_recv_word_poll
 
 	/* We first need to enable the bus clock */
 	if (tx_word_length > OMAP_MCBSP_WORD_16)
-		OMAP_MCBSP_WRITE(io_base, DXR2, clock_word >> 16);
-	OMAP_MCBSP_WRITE(io_base, DXR1, clock_word & 0xffff);
+		MCBSP_WRITE(mcbsp, DXR2, clock_word >> 16);
+	MCBSP_WRITE(mcbsp, DXR1, clock_word & 0xffff);
 
 	/* We wait for the receiver to be ready */
-	spcr1 = OMAP_MCBSP_READ(io_base, SPCR1);
+	spcr1 = MCBSP_READ(mcbsp, SPCR1);
 	while (!(spcr1 & RRDY)) {
-		spcr1 = OMAP_MCBSP_READ(io_base, SPCR1);
+		spcr1 = MCBSP_READ(mcbsp, SPCR1);
 		if (attempts++ > 1000) {
 			/* We must reset the receiver */
-			OMAP_MCBSP_WRITE(io_base, SPCR1, spcr1 & (~RRST));
+			MCBSP_WRITE(mcbsp, SPCR1, spcr1 & (~RRST));
 			udelay(10);
-			OMAP_MCBSP_WRITE(io_base, SPCR1, spcr1 | RRST);
+			MCBSP_WRITE(mcbsp, SPCR1, spcr1 | RRST);
 			udelay(10);
 			dev_err(mcbsp->dev, "McBSP%d receiver not "
 				"ready\n", mcbsp->id);
@@ -878,8 +847,8 @@ int omap_mcbsp_spi_master_recv_word_poll
 
 	/* Receiver is ready, there is something for us */
 	if (rx_word_length > OMAP_MCBSP_WORD_16)
-		word_msb = OMAP_MCBSP_READ(io_base, DRR2);
-	word_lsb = OMAP_MCBSP_READ(io_base, DRR1);
+		word_msb = MCBSP_READ(mcbsp, DRR2);
+	word_lsb = MCBSP_READ(mcbsp, DRR1);
 
 	word[0] = (word_lsb | (word_msb << 16));
 

[PATCH v7 2/5] [Resend] OMAP: McBSP: Modify macros/functions API for easy cache access

[Janusz Krzysztofik]

OMAP_MCBSP_READ()/_WRITE() macros and omap_mcbsp_read()/_write() functions
accept McBSP register base address as an argument. In order to support
caching, that must be replaced with an address of the omap_mcbsp structure
that would provide addresses for both register AND cache access.

Since OMAP_ prefix seems obvious in macro names, drop it off in order to
minimize line wrapping throughout the file.

Applies on top of patch 1 from this series:
[PATCH v7 1/5] OMAP: McBSP: Use macros for all register read/write operations

Tested on OMAP1510 based Amstrad Delta using linux-omap for-next,
commit 82f1d8f22f2c65e70206e40a6f17688bf64a892c.
Compile-tested with omap_3430sdp_defconfig.

Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>

---
Line-wrapped again, sorry.

Changes since v3:
- modify API of omap_mcbsp_read()/_write() functions as well, since those will
  actually provide caching operations, not macros like before.

 arch/arm/plat-omap/mcbsp.c |  281 ++++++++++++++++++++-------------------------
 1 file changed, 125 insertions(+), 156 deletions(-)

diff -upr git.orig/arch/arm/plat-omap/mcbsp.c git/arch/arm/plat-omap/mcbsp.c
--- git.orig/arch/arm/plat-omap/mcbsp.c	2009-12-09 15:28:33.000000000 +0100
+++ git/arch/arm/plat-omap/mcbsp.c	2009-12-09 15:29:16.000000000 +0100
@@ -30,26 +30,26 @@
 struct omap_mcbsp **mcbsp_ptr;
 int omap_mcbsp_count;
 
-void omap_mcbsp_write(void __iomem *io_base, u16 reg, u32 val)
+void omap_mcbsp_write(struct omap_mcbsp *mcbsp, u16 reg, u32 val)
 {
 	if (cpu_class_is_omap1() || cpu_is_omap2420())
-		__raw_writew((u16)val, io_base + reg);
+		__raw_writew((u16)val, mcbsp->io_base + reg);
 	else
-		__raw_writel(val, io_base + reg);
+		__raw_writel(val, mcbsp->io_base + reg);
 }
 
-int omap_mcbsp_read(void __iomem *io_base, u16 reg)
+int omap_mcbsp_read(struct omap_mcbsp *mcbsp, u16 reg)
 {
 	if (cpu_class_is_omap1() || cpu_is_omap2420())
-		return __raw_readw(io_base + reg);
+		return __raw_readw(mcbsp->io_base + reg);
 	else
-		return __raw_readl(io_base + reg);
+		return __raw_readl(mcbsp->io_base + reg);
 }
 
-#define OMAP_MCBSP_READ(base, reg) \
-			omap_mcbsp_read(base, OMAP_MCBSP_REG_##reg)
-#define OMAP_MCBSP_WRITE(base, reg, val) \
-			omap_mcbsp_write(base, OMAP_MCBSP_REG_##reg, val)
+#define MCBSP_READ(mcbsp, reg) \
+		omap_mcbsp_read(mcbsp, OMAP_MCBSP_REG_##reg)
+#define MCBSP_WRITE(mcbsp, reg, val) \
+		omap_mcbsp_write(mcbsp, OMAP_MCBSP_REG_##reg, val)
 
 #define omap_mcbsp_check_valid_id(id)	(id < omap_mcbsp_count)
 #define id_to_mcbsp_ptr(id)		mcbsp_ptr[id];
@@ -60,31 +60,31 @@ static void omap_mcbsp_dump_reg(u8 id)
 
 	dev_dbg(mcbsp->dev, "**** McBSP%d regs ****\n", mcbsp->id);
 	dev_dbg(mcbsp->dev, "DRR2:  0x%04x\n",
-			OMAP_MCBSP_READ(mcbsp->io_base, DRR2));
+			MCBSP_READ(mcbsp, DRR2));
 	dev_dbg(mcbsp->dev, "DRR1:  0x%04x\n",
-			OMAP_MCBSP_READ(mcbsp->io_base, DRR1));
+			MCBSP_READ(mcbsp, DRR1));
 	dev_dbg(mcbsp->dev, "DXR2:  0x%04x\n",
-			OMAP_MCBSP_READ(mcbsp->io_base, DXR2));
+			MCBSP_READ(mcbsp, DXR2));
 	dev_dbg(mcbsp->dev, "DXR1:  0x%04x\n",
-			OMAP_MCBSP_READ(mcbsp->io_base, DXR1));
+			MCBSP_READ(mcbsp, DXR1));
 	dev_dbg(mcbsp->dev, "SPCR2: 0x%04x\n",
-			OMAP_MCBSP_READ(mcbsp->io_base, SPCR2));
+			MCBSP_READ(mcbsp, SPCR2));
 	dev_dbg(mcbsp->dev, "SPCR1: 0x%04x\n",
-			OMAP_MCBSP_READ(mcbsp->io_base, SPCR1));
+			MCBSP_READ(mcbsp, SPCR1));
 	dev_dbg(mcbsp->dev, "RCR2:  0x%04x\n",
-			OMAP_MCBSP_READ(mcbsp->io_base, RCR2));
+			MCBSP_READ(mcbsp, RCR2));
 	dev_dbg(mcbsp->dev, "RCR1:  0x%04x\n",
-			OMAP_MCBSP_READ(mcbsp->io_base, RCR1));
+			MCBSP_READ(mcbsp, RCR1));
 	dev_dbg(mcbsp->dev, "XCR2:  0x%04x\n",
-			OMAP_MCBSP_READ(mcbsp->io_base, XCR2));
+			MCBSP_READ(mcbsp, XCR2));
 	dev_dbg(mcbsp->dev, "XCR1:  0x%04x\n",
-			OMAP_MCBSP_READ(mcbsp->io_base, XCR1));
+			MCBSP_READ(mcbsp, XCR1));
 	dev_dbg(mcbsp->dev, "SRGR2: 0x%04x\n",
-			OMAP_MCBSP_READ(mcbsp->io_base, SRGR2));
+			MCBSP_READ(mcbsp, SRGR2));
 	dev_dbg(mcbsp->dev, "SRGR1: 0x%04x\n",
-			OMAP_MCBSP_READ(mcbsp->io_base, SRGR1));
+			MCBSP_READ(mcbsp, SRGR1));
 	dev_dbg(mcbsp->dev, "PCR0:  0x%04x\n",
-			OMAP_MCBSP_READ(mcbsp->io_base, PCR0));
+			MCBSP_READ(mcbsp, PCR0));
 	dev_dbg(mcbsp->dev, "***********************\n");
 }
 
@@ -93,15 +93,14 @@ static irqreturn_t omap_mcbsp_tx_irq_han
 	struct omap_mcbsp *mcbsp_tx = dev_id;
 	u16 irqst_spcr2;
 
-	irqst_spcr2 = OMAP_MCBSP_READ(mcbsp_tx->io_base, SPCR2);
+	irqst_spcr2 = MCBSP_READ(mcbsp_tx, SPCR2);
 	dev_dbg(mcbsp_tx->dev, "TX IRQ callback : 0x%x\n", irqst_spcr2);
 
 	if (irqst_spcr2 & XSYNC_ERR) {
 		dev_err(mcbsp_tx->dev, "TX Frame Sync Error! : 0x%x\n",
 			irqst_spcr2);
 		/* Writing zero to XSYNC_ERR clears the IRQ */
-		OMAP_MCBSP_WRITE(mcbsp_tx->io_base, SPCR2,
-			irqst_spcr2 & ~(XSYNC_ERR));
+		MCBSP_WRITE(mcbsp_tx, SPCR2, irqst_spcr2 & ~(XSYNC_ERR));
 	} else {
 		complete(&mcbsp_tx->tx_irq_completion);
 	}
@@ -114,15 +113,14 @@ static irqreturn_t omap_mcbsp_rx_irq_han
 	struct omap_mcbsp *mcbsp_rx = dev_id;
 	u16 irqst_spcr1;
 
-	irqst_spcr1 = OMAP_MCBSP_READ(mcbsp_rx->io_base, SPCR1);
+	irqst_spcr1 = MCBSP_READ(mcbsp_rx, SPCR1);
 	dev_dbg(mcbsp_rx->dev, "RX IRQ callback : 0x%x\n", irqst_spcr1);
 
 	if (irqst_spcr1 & RSYNC_ERR) {
 		dev_err(mcbsp_rx->dev, "RX Frame Sync Error! : 0x%x\n",
 			irqst_spcr1);
 		/* Writing zero to RSYNC_ERR clears the IRQ */
-		OMAP_MCBSP_WRITE(mcbsp_rx->io_base, SPCR1,
-			irqst_spcr1 & ~(RSYNC_ERR));
+		MCBSP_WRITE(mcbsp_rx, SPCR1, irqst_spcr1 & ~(RSYNC_ERR));
 	} else {
 		complete(&mcbsp_rx->tx_irq_completion);
 	}
@@ -135,7 +133,7 @@ static void omap_mcbsp_tx_dma_callback(i
 	struct omap_mcbsp *mcbsp_dma_tx = data;
 
 	dev_dbg(mcbsp_dma_tx->dev, "TX DMA callback : 0x%x\n",
-		OMAP_MCBSP_READ(mcbsp_dma_tx->io_base, SPCR2));
+		MCBSP_READ(mcbsp_dma_tx, SPCR2));
 
 	/* We can free the channels */
 	omap_free_dma(mcbsp_dma_tx->dma_tx_lch);
@@ -149,7 +147,7 @@ static void omap_mcbsp_rx_dma_callback(i
 	struct omap_mcbsp *mcbsp_dma_rx = data;
 
 	dev_dbg(mcbsp_dma_rx->dev, "RX DMA callback : 0x%x\n",
-		OMAP_MCBSP_READ(mcbsp_dma_rx->io_base, SPCR2));
+		MCBSP_READ(mcbsp_dma_rx, SPCR2));
 
 	/* We can free the channels */
 	omap_free_dma(mcbsp_dma_rx->dma_rx_lch);
@@ -167,7 +165,6 @@ static void omap_mcbsp_rx_dma_callback(i
 void omap_mcbsp_config(unsigned int id, const struct omap_mcbsp_reg_cfg *config)
 {
 	struct omap_mcbsp *mcbsp;
-	void __iomem *io_base;
 
 	if (!omap_mcbsp_check_valid_id(id)) {
 		printk(KERN_ERR "%s: Invalid id (%d)\n", __func__, id + 1);
@@ -175,25 +172,24 @@ void omap_mcbsp_config(unsigned int id, 
 	}
 	mcbsp = id_to_mcbsp_ptr(id);
 
-	io_base = mcbsp->io_base;
 	dev_dbg(mcbsp->dev, "Configuring McBSP%d  phys_base: 0x%08lx\n",
 			mcbsp->id, mcbsp->phys_base);
 
 	/* We write the given config */
-	OMAP_MCBSP_WRITE(io_base, SPCR2, config->spcr2);
-	OMAP_MCBSP_WRITE(io_base, SPCR1, config->spcr1);
-	OMAP_MCBSP_WRITE(io_base, RCR2, config->rcr2);
-	OMAP_MCBSP_WRITE(io_base, RCR1, config->rcr1);
-	OMAP_MCBSP_WRITE(io_base, XCR2, config->xcr2);
-	OMAP_MCBSP_WRITE(io_base, XCR1, config->xcr1);
-	OMAP_MCBSP_WRITE(io_base, SRGR2, config->srgr2);
-	OMAP_MCBSP_WRITE(io_base, SRGR1, config->srgr1);
-	OMAP_MCBSP_WRITE(io_base, MCR2, config->mcr2);
-	OMAP_MCBSP_WRITE(io_base, MCR1, config->mcr1);
-	OMAP_MCBSP_WRITE(io_base, PCR0, config->pcr0);
+	MCBSP_WRITE(mcbsp, SPCR2, config->spcr2);
+	MCBSP_WRITE(mcbsp, SPCR1, config->spcr1);
+	MCBSP_WRITE(mcbsp, RCR2, config->rcr2);
+	MCBSP_WRITE(mcbsp, RCR1, config->rcr1);
+	MCBSP_WRITE(mcbsp, XCR2, config->xcr2);
+	MCBSP_WRITE(mcbsp, XCR1, config->xcr1);
+	MCBSP_WRITE(mcbsp, SRGR2, config->srgr2);
+	MCBSP_WRITE(mcbsp, SRGR1, config->srgr1);
+	MCBSP_WRITE(mcbsp, MCR2, config->mcr2);
+	MCBSP_WRITE(mcbsp, MCR1, config->mcr1);
+	MCBSP_WRITE(mcbsp, PCR0, config->pcr0);
 	if (cpu_is_omap2430() || cpu_is_omap34xx() || cpu_is_omap44xx()) {
-		OMAP_MCBSP_WRITE(io_base, XCCR, config->xccr);
-		OMAP_MCBSP_WRITE(io_base, RCCR, config->rccr);
+		MCBSP_WRITE(mcbsp, XCCR, config->xccr);
+		MCBSP_WRITE(mcbsp, RCCR, config->rccr);
 	}
 }
 EXPORT_SYMBOL(omap_mcbsp_config);
@@ -207,7 +203,6 @@ EXPORT_SYMBOL(omap_mcbsp_config);
 void omap_mcbsp_set_tx_threshold(unsigned int id, u16 threshold)
 {
 	struct omap_mcbsp *mcbsp;
-	void __iomem *io_base;
 
 	if (!cpu_is_omap34xx())
 		return;
@@ -217,9 +212,8 @@ void omap_mcbsp_set_tx_threshold(unsigne
 		return;
 	}
 	mcbsp = id_to_mcbsp_ptr(id);
-	io_base = mcbsp->io_base;
 
-	OMAP_MCBSP_WRITE(io_base, THRSH2, threshold);
+	MCBSP_WRITE(mcbsp, THRSH2, threshold);
 }
 EXPORT_SYMBOL(omap_mcbsp_set_tx_threshold);
 
@@ -231,7 +225,6 @@ EXPORT_SYMBOL(omap_mcbsp_set_tx_threshol
 void omap_mcbsp_set_rx_threshold(unsigned int id, u16 threshold)
 {
 	struct omap_mcbsp *mcbsp;
-	void __iomem *io_base;
 
 	if (!cpu_is_omap34xx())
 		return;
@@ -241,9 +234,8 @@ void omap_mcbsp_set_rx_threshold(unsigne
 		return;
 	}
 	mcbsp = id_to_mcbsp_ptr(id);
-	io_base = mcbsp->io_base;
 
-	OMAP_MCBSP_WRITE(io_base, THRSH1, threshold);
+	MCBSP_WRITE(mcbsp, THRSH1, threshold);
 }
 EXPORT_SYMBOL(omap_mcbsp_set_rx_threshold);
 
@@ -313,19 +305,18 @@ static inline void omap34xx_mcbsp_reques
 	if (cpu_is_omap34xx()) {
 		u16 syscon;
 
-		syscon = OMAP_MCBSP_READ(mcbsp->io_base, SYSCON);
+		syscon = MCBSP_READ(mcbsp, SYSCON);
 		syscon &= ~(ENAWAKEUP | SIDLEMODE(0x03) | CLOCKACTIVITY(0x03));
 
 		if (mcbsp->dma_op_mode == MCBSP_DMA_MODE_THRESHOLD) {
 			syscon |= (ENAWAKEUP | SIDLEMODE(0x02) |
 					CLOCKACTIVITY(0x02));
-			OMAP_MCBSP_WRITE(mcbsp->io_base, WAKEUPEN,
-					XRDYEN | RRDYEN);
+			MCBSP_WRITE(mcbsp, WAKEUPEN, XRDYEN | RRDYEN);
 		} else {
 			syscon |= SIDLEMODE(0x01);
 		}
 
-		OMAP_MCBSP_WRITE(mcbsp->io_base, SYSCON, syscon);
+		MCBSP_WRITE(mcbsp, SYSCON, syscon);
 	}
 }
 
@@ -337,7 +328,7 @@ static inline void omap34xx_mcbsp_free(s
 	if (cpu_is_omap34xx()) {
 		u16 syscon;
 
-		syscon = OMAP_MCBSP_READ(mcbsp->io_base, SYSCON);
+		syscon = MCBSP_READ(mcbsp, SYSCON);
 		syscon &= ~(ENAWAKEUP | SIDLEMODE(0x03) | CLOCKACTIVITY(0x03));
 		/*
 		 * HW bug workaround - If no_idle mode is taken, we need to
@@ -345,12 +336,12 @@ static inline void omap34xx_mcbsp_free(s
 		 * device will not hit retention anymore.
 		 */
 		syscon |= SIDLEMODE(0x02);
-		OMAP_MCBSP_WRITE(mcbsp->io_base, SYSCON, syscon);
+		MCBSP_WRITE(mcbsp, SYSCON, syscon);
 
 		syscon &= ~(SIDLEMODE(0x03));
-		OMAP_MCBSP_WRITE(mcbsp->io_base, SYSCON, syscon);
+		MCBSP_WRITE(mcbsp, SYSCON, syscon);
 
-		OMAP_MCBSP_WRITE(mcbsp->io_base, WAKEUPEN, 0);
+		MCBSP_WRITE(mcbsp, WAKEUPEN, 0);
 	}
 }
 #else
@@ -424,8 +415,8 @@ int omap_mcbsp_request(unsigned int id)
 	 * Make sure that transmitter, receiver and sample-rate generator are
 	 * not running before activating IRQs.
 	 */
-	OMAP_MCBSP_WRITE(mcbsp->io_base, SPCR1, 0);
-	OMAP_MCBSP_WRITE(mcbsp->io_base, SPCR2, 0);
+	MCBSP_WRITE(mcbsp, SPCR1, 0);
+	MCBSP_WRITE(mcbsp, SPCR2, 0);
 
 	if (mcbsp->io_type == OMAP_MCBSP_IRQ_IO) {
 		/* We need to get IRQs here */
@@ -501,7 +492,6 @@ EXPORT_SYMBOL(omap_mcbsp_free);
 void omap_mcbsp_start(unsigned int id, int tx, int rx)
 {
 	struct omap_mcbsp *mcbsp;
-	void __iomem *io_base;
 	int idle;
 	u16 w;
 
@@ -510,28 +500,26 @@ void omap_mcbsp_start(unsigned int id, i
 		return;
 	}
 	mcbsp = id_to_mcbsp_ptr(id);
-	io_base = mcbsp->io_base;
 
-	mcbsp->rx_word_length = (OMAP_MCBSP_READ(io_base, RCR1) >> 5) & 0x7;
-	mcbsp->tx_word_length = (OMAP_MCBSP_READ(io_base, XCR1) >> 5) & 0x7;
+	mcbsp->rx_word_length = (MCBSP_READ(mcbsp, RCR1) >> 5) & 0x7;
+	mcbsp->tx_word_length = (MCBSP_READ(mcbsp, XCR1) >> 5) & 0x7;
 
-	idle = !((OMAP_MCBSP_READ(io_base, SPCR2) |
-		  OMAP_MCBSP_READ(io_base, SPCR1)) & 1);
+	idle = !((MCBSP_READ(mcbsp, SPCR2) | MCBSP_READ(mcbsp, SPCR1)) & 1);
 
 	if (idle) {
 		/* Start the sample generator */
-		w = OMAP_MCBSP_READ(io_base, SPCR2);
-		OMAP_MCBSP_WRITE(io_base, SPCR2, w | (1 << 6));
+		w = MCBSP_READ(mcbsp, SPCR2);
+		MCBSP_WRITE(mcbsp, SPCR2, w | (1 << 6));
 	}
 
 	/* Enable transmitter and receiver */
 	tx &= 1;
-	w = OMAP_MCBSP_READ(io_base, SPCR2);
-	OMAP_MCBSP_WRITE(io_base, SPCR2, w | tx);
+	w = MCBSP_READ(mcbsp, SPCR2);
+	MCBSP_WRITE(mcbsp, SPCR2, w | tx);
 
 	rx &= 1;
-	w = OMAP_MCBSP_READ(io_base, SPCR1);
-	OMAP_MCBSP_WRITE(io_base, SPCR1, w | rx);
+	w = MCBSP_READ(mcbsp, SPCR1);
+	MCBSP_WRITE(mcbsp, SPCR1, w | rx);
 
 	/*
 	 * Worst case: CLKSRG*2 = 8000khz: (1/8000) * 2 * 2 usec
@@ -543,18 +531,18 @@ void omap_mcbsp_start(unsigned int id, i
 
 	if (idle) {
 		/* Start frame sync */
-		w = OMAP_MCBSP_READ(io_base, SPCR2);
-		OMAP_MCBSP_WRITE(io_base, SPCR2, w | (1 << 7));
+		w = MCBSP_READ(mcbsp, SPCR2);
+		MCBSP_WRITE(mcbsp, SPCR2, w | (1 << 7));
 	}
 
 	if (cpu_is_omap2430() || cpu_is_omap34xx()) {
 		/* Release the transmitter and receiver */
-		w = OMAP_MCBSP_READ(io_base, XCCR);
+		w = MCBSP_READ(mcbsp, XCCR);
 		w &= ~(tx ? XDISABLE : 0);
-		OMAP_MCBSP_WRITE(io_base, XCCR, w);
-		w = OMAP_MCBSP_READ(io_base, RCCR);
+		MCBSP_WRITE(mcbsp, XCCR, w);
+		w = MCBSP_READ(mcbsp, RCCR);
 		w &= ~(rx ? RDISABLE : 0);
-		OMAP_MCBSP_WRITE(io_base, RCCR, w);
+		MCBSP_WRITE(mcbsp, RCCR, w);
 	}
 
 	/* Dump McBSP Regs */
@@ -565,7 +553,6 @@ EXPORT_SYMBOL(omap_mcbsp_start);
 void omap_mcbsp_stop(unsigned int id, int tx, int rx)
 {
 	struct omap_mcbsp *mcbsp;
-	void __iomem *io_base;
 	int idle;
 	u16 w;
 
@@ -575,35 +562,33 @@ void omap_mcbsp_stop(unsigned int id, in
 	}
 
 	mcbsp = id_to_mcbsp_ptr(id);
-	io_base = mcbsp->io_base;
 
 	/* Reset transmitter */
 	tx &= 1;
 	if (cpu_is_omap2430() || cpu_is_omap34xx()) {
-		w = OMAP_MCBSP_READ(io_base, XCCR);
+		w = MCBSP_READ(mcbsp, XCCR);
 		w |= (tx ? XDISABLE : 0);
-		OMAP_MCBSP_WRITE(io_base, XCCR, w);
+		MCBSP_WRITE(mcbsp, XCCR, w);
 	}
-	w = OMAP_MCBSP_READ(io_base, SPCR2);
-	OMAP_MCBSP_WRITE(io_base, SPCR2, w & ~tx);
+	w = MCBSP_READ(mcbsp, SPCR2);
+	MCBSP_WRITE(mcbsp, SPCR2, w & ~tx);
 
 	/* Reset receiver */
 	rx &= 1;
 	if (cpu_is_omap2430() || cpu_is_omap34xx()) {
-		w = OMAP_MCBSP_READ(io_base, RCCR);
+		w = MCBSP_READ(mcbsp, RCCR);
 		w |= (rx ? RDISABLE : 0);
-		OMAP_MCBSP_WRITE(io_base, RCCR, w);
+		MCBSP_WRITE(mcbsp, RCCR, w);
 	}
-	w = OMAP_MCBSP_READ(io_base, SPCR1);
-	OMAP_MCBSP_WRITE(io_base, SPCR1, w & ~rx);
+	w = MCBSP_READ(mcbsp, SPCR1);
+	MCBSP_WRITE(mcbsp, SPCR1, w & ~rx);
 
-	idle = !((OMAP_MCBSP_READ(io_base, SPCR2) |
-		  OMAP_MCBSP_READ(io_base, SPCR1)) & 1);
+	idle = !((MCBSP_READ(mcbsp, SPCR2) | MCBSP_READ(mcbsp, SPCR1)) & 1);
 
 	if (idle) {
 		/* Reset the sample rate generator */
-		w = OMAP_MCBSP_READ(io_base, SPCR2);
-		OMAP_MCBSP_WRITE(io_base, SPCR2, w & ~(1 << 6));
+		w = MCBSP_READ(mcbsp, SPCR2);
+		MCBSP_WRITE(mcbsp, SPCR2, w & ~(1 << 6));
 	}
 }
 EXPORT_SYMBOL(omap_mcbsp_stop);
@@ -612,7 +597,6 @@ EXPORT_SYMBOL(omap_mcbsp_stop);
 int omap_mcbsp_pollwrite(unsigned int id, u16 buf)
 {
 	struct omap_mcbsp *mcbsp;
-	void __iomem *base;
 
 	if (!omap_mcbsp_check_valid_id(id)) {
 		printk(KERN_ERR "%s: Invalid id (%d)\n", __func__, id + 1);
@@ -620,28 +604,25 @@ int omap_mcbsp_pollwrite(unsigned int id
 	}
 
 	mcbsp = id_to_mcbsp_ptr(id);
-	base = mcbsp->io_base;
 
-	OMAP_MCBSP_WRITE(base, DXR1, buf);
+	MCBSP_WRITE(mcbsp, DXR1, buf);
 	/* if frame sync error - clear the error */
-	if (OMAP_MCBSP_READ(base, SPCR2) & XSYNC_ERR) {
+	if (MCBSP_READ(mcbsp, SPCR2) & XSYNC_ERR) {
 		/* clear error */
-		OMAP_MCBSP_WRITE(base, SPCR2,
-				OMAP_MCBSP_READ(base, SPCR2) & (~XSYNC_ERR));
+		MCBSP_WRITE(mcbsp, SPCR2,
+				MCBSP_READ(mcbsp, SPCR2) & (~XSYNC_ERR));
 		/* resend */
 		return -1;
 	} else {
 		/* wait for transmit confirmation */
 		int attemps = 0;
-		while (!(OMAP_MCBSP_READ(base, SPCR2) & XRDY)) {
+		while (!(MCBSP_READ(mcbsp, SPCR2) & XRDY)) {
 			if (attemps++ > 1000) {
-				OMAP_MCBSP_WRITE(base, SPCR2,
-						OMAP_MCBSP_READ(base, SPCR2) &
-						(~XRST));
+				MCBSP_WRITE(mcbsp, SPCR2,
+					    MCBSP_READ(mcbsp, SPCR2) & (~XRST));
 				udelay(10);
-				OMAP_MCBSP_WRITE(base, SPCR2,
-						OMAP_MCBSP_READ(base, SPCR2) |
-						(XRST));
+				MCBSP_WRITE(mcbsp, SPCR2,
+					    MCBSP_READ(mcbsp, SPCR2) | (XRST));
 				udelay(10);
 				dev_err(mcbsp->dev, "Could not write to"
 					" McBSP%d Register\n", mcbsp->id);
@@ -657,7 +638,6 @@ EXPORT_SYMBOL(omap_mcbsp_pollwrite);
 int omap_mcbsp_pollread(unsigned int id, u16 *buf)
 {
 	struct omap_mcbsp *mcbsp;
-	void __iomem *base;
 
 	if (!omap_mcbsp_check_valid_id(id)) {
 		printk(KERN_ERR "%s: Invalid id (%d)\n", __func__, id + 1);
@@ -665,26 +645,23 @@ int omap_mcbsp_pollread(unsigned int id,
 	}
 	mcbsp = id_to_mcbsp_ptr(id);
 
-	base = mcbsp->io_base;
 	/* if frame sync error - clear the error */
-	if (OMAP_MCBSP_READ(base, SPCR1) & RSYNC_ERR) {
+	if (MCBSP_READ(mcbsp, SPCR1) & RSYNC_ERR) {
 		/* clear error */
-		OMAP_MCBSP_WRITE(base, SPCR1,
-				OMAP_MCBSP_READ(base, SPCR1) & (~RSYNC_ERR));
+		MCBSP_WRITE(mcbsp, SPCR1,
+				MCBSP_READ(mcbsp, SPCR1) & (~RSYNC_ERR));
 		/* resend */
 		return -1;
 	} else {
 		/* wait for recieve confirmation */
 		int attemps = 0;
-		while (!(OMAP_MCBSP_READ(base, SPCR1) & RRDY)) {
+		while (!(MCBSP_READ(mcbsp, SPCR1) & RRDY)) {
 			if (attemps++ > 1000) {
-				OMAP_MCBSP_WRITE(base, SPCR1,
-						OMAP_MCBSP_READ(base, SPCR1) &
-						(~RRST));
+				MCBSP_WRITE(mcbsp, SPCR1,
+					    MCBSP_READ(mcbsp, SPCR1) & (~RRST));
 				udelay(10);
-				OMAP_MCBSP_WRITE(base, SPCR1,
-						OMAP_MCBSP_READ(base, SPCR1) |
-						(RRST));
+				MCBSP_WRITE(mcbsp, SPCR1,
+					    MCBSP_READ(mcbsp, SPCR1) | (RRST));
 				udelay(10);
 				dev_err(mcbsp->dev, "Could not read from"
 					" McBSP%d Register\n", mcbsp->id);
@@ -692,7 +669,7 @@ int omap_mcbsp_pollread(unsigned int id,
 			}
 		}
 	}
-	*buf = OMAP_MCBSP_READ(base, DRR1);
+	*buf = MCBSP_READ(mcbsp, DRR1);
 
 	return 0;
 }
@@ -704,7 +681,6 @@ EXPORT_SYMBOL(omap_mcbsp_pollread);
 void omap_mcbsp_xmit_word(unsigned int id, u32 word)
 {
 	struct omap_mcbsp *mcbsp;
-	void __iomem *io_base;
 	omap_mcbsp_word_length word_length;
 
 	if (!omap_mcbsp_check_valid_id(id)) {
@@ -713,21 +689,19 @@ void omap_mcbsp_xmit_word(unsigned int i
 	}
 
 	mcbsp = id_to_mcbsp_ptr(id);
-	io_base = mcbsp->io_base;
 	word_length = mcbsp->tx_word_length;
 
 	wait_for_completion(&mcbsp->tx_irq_completion);
 
 	if (word_length > OMAP_MCBSP_WORD_16)
-		OMAP_MCBSP_WRITE(io_base, DXR2, word >> 16);
-	OMAP_MCBSP_WRITE(io_base, DXR1, word & 0xffff);
+		MCBSP_WRITE(mcbsp, DXR2, word >> 16);
+	MCBSP_WRITE(mcbsp, DXR1, word & 0xffff);
 }
 EXPORT_SYMBOL(omap_mcbsp_xmit_word);
 
 u32 omap_mcbsp_recv_word(unsigned int id)
 {
 	struct omap_mcbsp *mcbsp;
-	void __iomem *io_base;
 	u16 word_lsb, word_msb = 0;
 	omap_mcbsp_word_length word_length;
 
@@ -738,13 +712,12 @@ u32 omap_mcbsp_recv_word(unsigned int id
 	mcbsp = id_to_mcbsp_ptr(id);
 
 	word_length = mcbsp->rx_word_length;
-	io_base = mcbsp->io_base;
 
 	wait_for_completion(&mcbsp->rx_irq_completion);
 
 	if (word_length > OMAP_MCBSP_WORD_16)
-		word_msb = OMAP_MCBSP_READ(io_base, DRR2);
-	word_lsb = OMAP_MCBSP_READ(io_base, DRR1);
+		word_msb = MCBSP_READ(mcbsp, DRR2);
+	word_lsb = MCBSP_READ(mcbsp, DRR1);
 
 	return (word_lsb | (word_msb << 16));
 }
@@ -753,7 +726,6 @@ EXPORT_SYMBOL(omap_mcbsp_recv_word);
 int omap_mcbsp_spi_master_xmit_word_poll(unsigned int id, u32 word)
 {
 	struct omap_mcbsp *mcbsp;
-	void __iomem *io_base;
 	omap_mcbsp_word_length tx_word_length;
 	omap_mcbsp_word_length rx_word_length;
 	u16 spcr2, spcr1, attempts = 0, word_lsb, word_msb = 0;
@@ -763,7 +735,6 @@ int omap_mcbsp_spi_master_xmit_word_poll
 		return -ENODEV;
 	}
 	mcbsp = id_to_mcbsp_ptr(id);
-	io_base = mcbsp->io_base;
 	tx_word_length = mcbsp->tx_word_length;
 	rx_word_length = mcbsp->rx_word_length;
 
@@ -771,14 +742,14 @@ int omap_mcbsp_spi_master_xmit_word_poll
 		return -EINVAL;
 
 	/* First we wait for the transmitter to be ready */
-	spcr2 = OMAP_MCBSP_READ(io_base, SPCR2);
+	spcr2 = MCBSP_READ(mcbsp, SPCR2);
 	while (!(spcr2 & XRDY)) {
-		spcr2 = OMAP_MCBSP_READ(io_base, SPCR2);
+		spcr2 = MCBSP_READ(mcbsp, SPCR2);
 		if (attempts++ > 1000) {
 			/* We must reset the transmitter */
-			OMAP_MCBSP_WRITE(io_base, SPCR2, spcr2 & (~XRST));
+			MCBSP_WRITE(mcbsp, SPCR2, spcr2 & (~XRST));
 			udelay(10);
-			OMAP_MCBSP_WRITE(io_base, SPCR2, spcr2 | XRST);
+			MCBSP_WRITE(mcbsp, SPCR2, spcr2 | XRST);
 			udelay(10);
 			dev_err(mcbsp->dev, "McBSP%d transmitter not "
 				"ready\n", mcbsp->id);
@@ -788,18 +759,18 @@ int omap_mcbsp_spi_master_xmit_word_poll
 
 	/* Now we can push the data */
 	if (tx_word_length > OMAP_MCBSP_WORD_16)
-		OMAP_MCBSP_WRITE(io_base, DXR2, word >> 16);
-	OMAP_MCBSP_WRITE(io_base, DXR1, word & 0xffff);
+		MCBSP_WRITE(mcbsp, DXR2, word >> 16);
+	MCBSP_WRITE(mcbsp, DXR1, word & 0xffff);
 
 	/* We wait for the receiver to be ready */
-	spcr1 = OMAP_MCBSP_READ(io_base, SPCR1);
+	spcr1 = MCBSP_READ(mcbsp, SPCR1);
 	while (!(spcr1 & RRDY)) {
-		spcr1 = OMAP_MCBSP_READ(io_base, SPCR1);
+		spcr1 = MCBSP_READ(mcbsp, SPCR1);
 		if (attempts++ > 1000) {
 			/* We must reset the receiver */
-			OMAP_MCBSP_WRITE(io_base, SPCR1, spcr1 & (~RRST));
+			MCBSP_WRITE(mcbsp, SPCR1, spcr1 & (~RRST));
 			udelay(10);
-			OMAP_MCBSP_WRITE(io_base, SPCR1, spcr1 | RRST);
+			MCBSP_WRITE(mcbsp, SPCR1, spcr1 | RRST);
 			udelay(10);
 			dev_err(mcbsp->dev, "McBSP%d receiver not "
 				"ready\n", mcbsp->id);
@@ -809,8 +780,8 @@ int omap_mcbsp_spi_master_xmit_word_poll
 
 	/* Receiver is ready, let's read the dummy data */
 	if (rx_word_length > OMAP_MCBSP_WORD_16)
-		word_msb = OMAP_MCBSP_READ(io_base, DRR2);
-	word_lsb = OMAP_MCBSP_READ(io_base, DRR1);
+		word_msb = MCBSP_READ(mcbsp, DRR2);
+	word_lsb = MCBSP_READ(mcbsp, DRR1);
 
 	return 0;
 }
@@ -820,7 +791,6 @@ int omap_mcbsp_spi_master_recv_word_poll
 {
 	struct omap_mcbsp *mcbsp;
 	u32 clock_word = 0;
-	void __iomem *io_base;
 	omap_mcbsp_word_length tx_word_length;
 	omap_mcbsp_word_length rx_word_length;
 	u16 spcr2, spcr1, attempts = 0, word_lsb, word_msb = 0;
@@ -831,7 +801,6 @@ int omap_mcbsp_spi_master_recv_word_poll
 	}
 
 	mcbsp = id_to_mcbsp_ptr(id);
-	io_base = mcbsp->io_base;
 
 	tx_word_length = mcbsp->tx_word_length;
 	rx_word_length = mcbsp->rx_word_length;
@@ -840,14 +809,14 @@ int omap_mcbsp_spi_master_recv_word_poll
 		return -EINVAL;
 
 	/* First we wait for the transmitter to be ready */
-	spcr2 = OMAP_MCBSP_READ(io_base, SPCR2);
+	spcr2 = MCBSP_READ(mcbsp, SPCR2);
 	while (!(spcr2 & XRDY)) {
-		spcr2 = OMAP_MCBSP_READ(io_base, SPCR2);
+		spcr2 = MCBSP_READ(mcbsp, SPCR2);
 		if (attempts++ > 1000) {
 			/* We must reset the transmitter */
-			OMAP_MCBSP_WRITE(io_base, SPCR2, spcr2 & (~XRST));
+			MCBSP_WRITE(mcbsp, SPCR2, spcr2 & (~XRST));
 			udelay(10);
-			OMAP_MCBSP_WRITE(io_base, SPCR2, spcr2 | XRST);
+			MCBSP_WRITE(mcbsp, SPCR2, spcr2 | XRST);
 			udelay(10);
 			dev_err(mcbsp->dev, "McBSP%d transmitter not "
 				"ready\n", mcbsp->id);
@@ -857,18 +826,18 @@ int omap_mcbsp_spi_master_recv_word_poll
 
 	/* We first need to enable the bus clock */
 	if (tx_word_length > OMAP_MCBSP_WORD_16)
-		OMAP_MCBSP_WRITE(io_base, DXR2, clock_word >> 16);
-	OMAP_MCBSP_WRITE(io_base, DXR1, clock_word & 0xffff);
+		MCBSP_WRITE(mcbsp, DXR2, clock_word >> 16);
+	MCBSP_WRITE(mcbsp, DXR1, clock_word & 0xffff);
 
 	/* We wait for the receiver to be ready */
-	spcr1 = OMAP_MCBSP_READ(io_base, SPCR1);
+	spcr1 = MCBSP_READ(mcbsp, SPCR1);
 	while (!(spcr1 & RRDY)) {
-		spcr1 = OMAP_MCBSP_READ(io_base, SPCR1);
+		spcr1 = MCBSP_READ(mcbsp, SPCR1);
 		if (attempts++ > 1000) {
 			/* We must reset the receiver */
-			OMAP_MCBSP_WRITE(io_base, SPCR1, spcr1 & (~RRST));
+			MCBSP_WRITE(mcbsp, SPCR1, spcr1 & (~RRST));
 			udelay(10);
-			OMAP_MCBSP_WRITE(io_base, SPCR1, spcr1 | RRST);
+			MCBSP_WRITE(mcbsp, SPCR1, spcr1 | RRST);
 			udelay(10);
 			dev_err(mcbsp->dev, "McBSP%d receiver not "
 				"ready\n", mcbsp->id);
@@ -878,8 +847,8 @@ int omap_mcbsp_spi_master_recv_word_poll
 
 	/* Receiver is ready, there is something for us */
 	if (rx_word_length > OMAP_MCBSP_WORD_16)
-		word_msb = OMAP_MCBSP_READ(io_base, DRR2);
-	word_lsb = OMAP_MCBSP_READ(io_base, DRR1);
+		word_msb = MCBSP_READ(mcbsp, DRR2);
+	word_lsb = MCBSP_READ(mcbsp, DRR1);
 
 	word[0] = (word_lsb | (word_msb << 16));
 

RE: [PATCH v7 2/5] OMAP: McBSP: Modify macros/functions API for easy cache access

[Varadarajan, Charu Latha]

 

>-----Original Message-----
>From: linux-omap-owner@vger.kernel.org 
>[mailto:linux-omap-owner@vger.kernel.org] On Behalf Of Janusz 
>Krzysztofik
>Sent: Thursday, December 10, 2009 1:59 AM
>To: Tony Lindgren
>Cc: Jarkko Nikula; Peter Ujfalusi; linux-omap@vger.kernel.org
>Subject: [PATCH v7 2/5] OMAP: McBSP: Modify macros/functions 
>API for easy cache access
>
>OMAP_MCBSP_READ()/_WRITE() macros and 
>omap_mcbsp_read()/_write() functions
>accept McBSP register base address as an argument. In order to support
>caching, that must be replaced with an address of the 
>omap_mcbsp structure
>that would provide addresses for both register AND cache access.
>
>Since OMAP_ prefix seems obvious in macro names, drop it off 
>in order to
>minimize line wrapping throughout the file.
>
>Applies on top of patch 1 from this series:
>[PATCH v7 1/5] OMAP: McBSP: Use macros for all register 
>read/write operations
>
>Tested on OMAP1510 based Amstrad Delta using linux-omap for-next,
>commit 82f1d8f22f2c65e70206e40a6f17688bf64a892c.
>Compile-tested with omap_3430sdp_defconfig.
>
>Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
>
>---
>Changes since v3:
>- modify API of omap_mcbsp_read()/_write() functions as well, 
>since those will
>  actually provide caching operations, not macros like before.
>

<snip>



>@@ -313,19 +305,18 @@ static inline void omap34xx_mcbsp_reques
> 	if (cpu_is_omap34xx()) {
> 		u16 syscon;
> 
>-		syscon = OMAP_MCBSP_READ(mcbsp->io_base, SYSCON);
>+		syscon = MCBSP_READ(mcbsp, SYSCON);
> 		syscon &= ~(ENAWAKEUP | SIDLEMODE(0x03) | 
>CLOCKACTIVITY(0x03));

Would this driver get adpated to the hwmod framework? Then this 
would be invalid.

> 
> 		if (mcbsp->dma_op_mode == MCBSP_DMA_MODE_THRESHOLD) {
> 			syscon |= (ENAWAKEUP | SIDLEMODE(0x02) |
> 					CLOCKACTIVITY(0x02));
>-			OMAP_MCBSP_WRITE(mcbsp->io_base, WAKEUPEN,
>-					XRDYEN | RRDYEN);
>+			MCBSP_WRITE(mcbsp, WAKEUPEN, XRDYEN | RRDYEN);
> 		} else {
> 			syscon |= SIDLEMODE(0x01);
> 		}
> 
>-		OMAP_MCBSP_WRITE(mcbsp->io_base, SYSCON, syscon);
>+		MCBSP_WRITE(mcbsp, SYSCON, syscon);

Ditto

> 	}
> }
> 
>@@ -337,7 +328,7 @@ static inline void omap34xx_mcbsp_free(s
> 	if (cpu_is_omap34xx()) {
> 		u16 syscon;
> 
>-		syscon = OMAP_MCBSP_READ(mcbsp->io_base, SYSCON);
>+		syscon = MCBSP_READ(mcbsp, SYSCON);
> 		syscon &= ~(ENAWAKEUP | SIDLEMODE(0x03) | 
>CLOCKACTIVITY(0x03));
> 		/*
> 		 * HW bug workaround - If no_idle mode is 
>taken, we need to
>@@ -345,12 +336,12 @@ static inline void omap34xx_mcbsp_free(s
> 		 * device will not hit retention anymore.
> 		 */
> 		syscon |= SIDLEMODE(0x02);
>-		OMAP_MCBSP_WRITE(mcbsp->io_base, SYSCON, syscon);
>+		MCBSP_WRITE(mcbsp, SYSCON, syscon);

Ditto

> 
> 		syscon &= ~(SIDLEMODE(0x03));
>-		OMAP_MCBSP_WRITE(mcbsp->io_base, SYSCON, syscon);
>+		MCBSP_WRITE(mcbsp, SYSCON, syscon);

Ditto

> 
>-		OMAP_MCBSP_WRITE(mcbsp->io_base, WAKEUPEN, 0);
>+		MCBSP_WRITE(mcbsp, WAKEUPEN, 0);
> 	}
> }

<snip>

> 
> 	/*
> 	 * Worst case: CLKSRG*2 = 8000khz: (1/8000) * 2 * 2 usec
>@@ -543,18 +531,18 @@ void omap_mcbsp_start(unsigned int id, i
> 
> 	if (idle) {
> 		/* Start frame sync */
>-		w = OMAP_MCBSP_READ(io_base, SPCR2);
>-		OMAP_MCBSP_WRITE(io_base, SPCR2, w | (1 << 7));
>+		w = MCBSP_READ(mcbsp, SPCR2);
>+		MCBSP_WRITE(mcbsp, SPCR2, w | (1 << 7));
> 	}
> 
> 	if (cpu_is_omap2430() || cpu_is_omap34xx()) {

Why not 44xx?

> 		/* Release the transmitter and receiver */
>-		w = OMAP_MCBSP_READ(io_base, XCCR);
>+		w = MCBSP_READ(mcbsp, XCCR);
> 		w &= ~(tx ? XDISABLE : 0);
>-		OMAP_MCBSP_WRITE(io_base, XCCR, w);
>-		w = OMAP_MCBSP_READ(io_base, RCCR);
>+		MCBSP_WRITE(mcbsp, XCCR, w);
>+		w = MCBSP_READ(mcbsp, RCCR);
> 		w &= ~(rx ? RDISABLE : 0);
>-		OMAP_MCBSP_WRITE(io_base, RCCR, w);
>+		MCBSP_WRITE(mcbsp, RCCR, w);
> 	}
> 
> 	/* Dump McBSP Regs */
>@@ -565,7 +553,6 @@ EXPORT_SYMBOL(omap_mcbsp_start);
> void omap_mcbsp_stop(unsigned int id, int tx, int rx)
> {
> 	struct omap_mcbsp *mcbsp;
>-	void __iomem *io_base;
> 	int idle;
> 	u16 w;
> 
>@@ -575,35 +562,33 @@ void omap_mcbsp_stop(unsigned int id, in
> 	}
> 
> 	mcbsp = id_to_mcbsp_ptr(id);
>-	io_base = mcbsp->io_base;
> 
> 	/* Reset transmitter */
> 	tx &= 1;
> 	if (cpu_is_omap2430() || cpu_is_omap34xx()) {

Why not 44xx?

>-		w = OMAP_MCBSP_READ(io_base, XCCR);
>+		w = MCBSP_READ(mcbsp, XCCR);
> 		w |= (tx ? XDISABLE : 0);
>-		OMAP_MCBSP_WRITE(io_base, XCCR, w);
>+		MCBSP_WRITE(mcbsp, XCCR, w);
> 	}
>-	w = OMAP_MCBSP_READ(io_base, SPCR2);
>-	OMAP_MCBSP_WRITE(io_base, SPCR2, w & ~tx);
>+	w = MCBSP_READ(mcbsp, SPCR2);
>+	MCBSP_WRITE(mcbsp, SPCR2, w & ~tx);
> 
> 	/* Reset receiver */
> 	rx &= 1;
> 	if (cpu_is_omap2430() || cpu_is_omap34xx()) {

Ditto

>-		w = OMAP_MCBSP_READ(io_base, RCCR);
>+		w = MCBSP_READ(mcbsp, RCCR);
> 		w |= (rx ? RDISABLE : 0);
>-		OMAP_MCBSP_WRITE(io_base, RCCR, w);
>+		MCBSP_WRITE(mcbsp, RCCR, w);
> 	}
>-	w = OMAP_MCBSP_READ(io_base, SPCR1);
>-	OMAP_MCBSP_WRITE(io_base, SPCR1, w & ~rx);
>+	w = MCBSP_READ(mcbsp, SPCR1);
>+	MCBSP_WRITE(mcbsp, SPCR1, w & ~rx);
> 
>-	idle = !((OMAP_MCBSP_READ(io_base, SPCR2) |
>-		  OMAP_MCBSP_READ(io_base, SPCR1)) & 1);
>+	idle = !((MCBSP_READ(mcbsp, SPCR2) | MCBSP_READ(mcbsp, 
>SPCR1)) & 1);
> 
> 	if (idle) {
> 		/* Reset the sample rate generator */
>-		w = OMAP_MCBSP_READ(io_base, SPCR2);
>-		OMAP_MCBSP_WRITE(io_base, SPCR2, w & ~(1 << 6));
>+		w = MCBSP_READ(mcbsp, SPCR2);
>+		MCBSP_WRITE(mcbsp, SPCR2, w & ~(1 << 6));
> 	}
> }
> EXPORT_SYMBOL(omap_mcbsp_stop);
>@@ -612,7 +597,6 @@ EXPORT_SYMBOL(omap_mcbsp_stop);
> int omap_mcbsp_pollwrite(unsigned int id, u16 buf)
> {
> 	struct omap_mcbsp *mcbsp;
>-	void __iomem *base;
> 
> 	if (!omap_mcbsp_check_valid_id(id)) {
> 		printk(KERN_ERR "%s: Invalid id (%d)\n", 
>__func__, id + 1);
>@@ -620,28 +604,25 @@ int omap_mcbsp_pollwrite(unsigned int id
> 	}
> 
> 	mcbsp = id_to_mcbsp_ptr(id);
>-	base = mcbsp->io_base;
> 
>-	OMAP_MCBSP_WRITE(base, DXR1, buf);
>+	MCBSP_WRITE(mcbsp, DXR1, buf);

OMAP3/4 allows 32 bit data access also. 
How is it taken care? Please refer to 
http://patchwork.kernel.org/patch/54896/ for more details.

Quoting Tony's words:
"To me it smells like the all drivers using the the
omap_mcbsp_pollread/write are broken legacy drivers.
So maybe we should just remove these two functions?
If we really want to keep these around, we should review
the drivers using these functions. "


> 	/* if frame sync error - clear the error */
>-	if (OMAP_MCBSP_READ(base, SPCR2) & XSYNC_ERR) {
>+	if (MCBSP_READ(mcbsp, SPCR2) & XSYNC_ERR) {
> 		/* clear error */
>-		OMAP_MCBSP_WRITE(base, SPCR2,
>-				OMAP_MCBSP_READ(base, SPCR2) & 
>(~XSYNC_ERR));
>+		MCBSP_WRITE(mcbsp, SPCR2,
>+				MCBSP_READ(mcbsp, SPCR2) & 
>(~XSYNC_ERR));
> 		/* resend */
> 		return -1;
> 	} else {
> 		/* wait for transmit confirmation */
> 		int attemps = 0;
>-		while (!(OMAP_MCBSP_READ(base, SPCR2) & XRDY)) {
>+		while (!(MCBSP_READ(mcbsp, SPCR2) & XRDY)) {
> 			if (attemps++ > 1000) {
>-				OMAP_MCBSP_WRITE(base, SPCR2,
>-						
>OMAP_MCBSP_READ(base, SPCR2) &
>-						(~XRST));
>+				MCBSP_WRITE(mcbsp, SPCR2,
>+					    MCBSP_READ(mcbsp, 
>SPCR2) & (~XRST));
> 				udelay(10);
>-				OMAP_MCBSP_WRITE(base, SPCR2,
>-						
>OMAP_MCBSP_READ(base, SPCR2) |
>-						(XRST));
>+				MCBSP_WRITE(mcbsp, SPCR2,
>+					    MCBSP_READ(mcbsp, 
>SPCR2) | (XRST));
> 				udelay(10);
> 				dev_err(mcbsp->dev, "Could not write to"
> 					" McBSP%d Register\n", 
>mcbsp->id);
>@@ -657,7 +638,6 @@ EXPORT_SYMBOL(omap_mcbsp_pollwrite);
> int omap_mcbsp_pollread(unsigned int id, u16 *buf)
> {
> 	struct omap_mcbsp *mcbsp;
>-	void __iomem *base;
> 
> 	if (!omap_mcbsp_check_valid_id(id)) {
> 		printk(KERN_ERR "%s: Invalid id (%d)\n", 
>__func__, id + 1);
>@@ -665,26 +645,23 @@ int omap_mcbsp_pollread(unsigned int id,
> 	}
> 	mcbsp = id_to_mcbsp_ptr(id);
> 
>-	base = mcbsp->io_base;
> 	/* if frame sync error - clear the error */
>-	if (OMAP_MCBSP_READ(base, SPCR1) & RSYNC_ERR) {
>+	if (MCBSP_READ(mcbsp, SPCR1) & RSYNC_ERR) {
> 		/* clear error */
>-		OMAP_MCBSP_WRITE(base, SPCR1,
>-				OMAP_MCBSP_READ(base, SPCR1) & 
>(~RSYNC_ERR));
>+		MCBSP_WRITE(mcbsp, SPCR1,
>+				MCBSP_READ(mcbsp, SPCR1) & 
>(~RSYNC_ERR));
> 		/* resend */
> 		return -1;
> 	} else {
> 		/* wait for recieve confirmation */
> 		int attemps = 0;
>-		while (!(OMAP_MCBSP_READ(base, SPCR1) & RRDY)) {
>+		while (!(MCBSP_READ(mcbsp, SPCR1) & RRDY)) {
> 			if (attemps++ > 1000) {
>-				OMAP_MCBSP_WRITE(base, SPCR1,
>-						
>OMAP_MCBSP_READ(base, SPCR1) &
>-						(~RRST));
>+				MCBSP_WRITE(mcbsp, SPCR1,
>+					    MCBSP_READ(mcbsp, 
>SPCR1) & (~RRST));
> 				udelay(10);
>-				OMAP_MCBSP_WRITE(base, SPCR1,
>-						
>OMAP_MCBSP_READ(base, SPCR1) |
>-						(RRST));
>+				MCBSP_WRITE(mcbsp, SPCR1,
>+					    MCBSP_READ(mcbsp, 
>SPCR1) | (RRST));
> 				udelay(10);
> 				dev_err(mcbsp->dev, "Could not 
>read from"
> 					" McBSP%d Register\n", 
>mcbsp->id);
>@@ -692,7 +669,7 @@ int omap_mcbsp_pollread(unsigned int id,
> 			}
> 		}
> 	}
>-	*buf = OMAP_MCBSP_READ(base, DRR1);
>+	*buf = MCBSP_READ(mcbsp, DRR1);
> 
> 	return 0;
> }

<snip>

Was this code tested for different configurations 
like DMA mode, poll mode?

Re: [PATCH v7 2/5] OMAP: McBSP: Modify macros/functions API for easy cache access

[Janusz Krzysztofik]

Hi,

Friday 11 December 2009 15:10:26 Varadarajan, Charu Latha napisał(a):
> >-----Original Message-----
> >From: linux-omap-owner@vger.kernel.org
> >[mailto:linux-omap-owner@vger.kernel.org] On Behalf Of Janusz
> >Krzysztofik
> >Sent: Thursday, December 10, 2009 1:59 AM
> >To: Tony Lindgren
> >Cc: Jarkko Nikula; Peter Ujfalusi; linux-omap@vger.kernel.org
> >Subject: [PATCH v7 2/5] OMAP: McBSP: Modify macros/functions
> >API for easy cache access
> >
> >OMAP_MCBSP_READ()/_WRITE() macros and
> >omap_mcbsp_read()/_write() functions
> >accept McBSP register base address as an argument. In order to support
> >caching, that must be replaced with an address of the
> >omap_mcbsp structure
> >that would provide addresses for both register AND cache access.
> >
> >Since OMAP_ prefix seems obvious in macro names, drop it off
> >in order to
> >minimize line wrapping throughout the file.
> >
> >Applies on top of patch 1 from this series:
> >[PATCH v7 1/5] OMAP: McBSP: Use macros for all register
> >read/write operations
> >
> >Tested on OMAP1510 based Amstrad Delta using linux-omap for-next,
> >commit 82f1d8f22f2c65e70206e40a6f17688bf64a892c.
> >Compile-tested with omap_3430sdp_defconfig.
> >
> >Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
> >
> >---
> >Changes since v3:
> >- modify API of omap_mcbsp_read()/_write() functions as well,
> >since those will
> >  actually provide caching operations, not macros like before.
>
> <snip>
>
> >@@ -313,19 +305,18 @@ static inline void omap34xx_mcbsp_reques
> > 	if (cpu_is_omap34xx()) {
> > 		u16 syscon;
> >
> >-		syscon = OMAP_MCBSP_READ(mcbsp->io_base, SYSCON);
> >+		syscon = MCBSP_READ(mcbsp, SYSCON);
> > 		syscon &= ~(ENAWAKEUP | SIDLEMODE(0x03) |
> >CLOCKACTIVITY(0x03));
>
> Would this driver get adpated to the hwmod framework? Then this
> would be invalid.

That I don't know, but if really, it seems being already invalid before this 
patch (the same answer applies to several following Dittos).

> > 		if (mcbsp->dma_op_mode == MCBSP_DMA_MODE_THRESHOLD) {
> > 			syscon |= (ENAWAKEUP | SIDLEMODE(0x02) |
> > 					CLOCKACTIVITY(0x02));
> >-			OMAP_MCBSP_WRITE(mcbsp->io_base, WAKEUPEN,
> >-					XRDYEN | RRDYEN);
> >+			MCBSP_WRITE(mcbsp, WAKEUPEN, XRDYEN | RRDYEN);
> > 		} else {
> > 			syscon |= SIDLEMODE(0x01);
> > 		}
> >
> >-		OMAP_MCBSP_WRITE(mcbsp->io_base, SYSCON, syscon);
> >+		MCBSP_WRITE(mcbsp, SYSCON, syscon);
>
> Ditto
>
> > 	}
> > }
> >
> >@@ -337,7 +328,7 @@ static inline void omap34xx_mcbsp_free(s
> > 	if (cpu_is_omap34xx()) {
> > 		u16 syscon;
> >
> >-		syscon = OMAP_MCBSP_READ(mcbsp->io_base, SYSCON);
> >+		syscon = MCBSP_READ(mcbsp, SYSCON);
> > 		syscon &= ~(ENAWAKEUP | SIDLEMODE(0x03) |
> >CLOCKACTIVITY(0x03));
> > 		/*
> > 		 * HW bug workaround - If no_idle mode is
> >taken, we need to
> >@@ -345,12 +336,12 @@ static inline void omap34xx_mcbsp_free(s
> > 		 * device will not hit retention anymore.
> > 		 */
> > 		syscon |= SIDLEMODE(0x02);
> >-		OMAP_MCBSP_WRITE(mcbsp->io_base, SYSCON, syscon);
> >+		MCBSP_WRITE(mcbsp, SYSCON, syscon);
>
> Ditto
>
> > 		syscon &= ~(SIDLEMODE(0x03));
> >-		OMAP_MCBSP_WRITE(mcbsp->io_base, SYSCON, syscon);
> >+		MCBSP_WRITE(mcbsp, SYSCON, syscon);
>
> Ditto
>
> >-		OMAP_MCBSP_WRITE(mcbsp->io_base, WAKEUPEN, 0);
> >+		MCBSP_WRITE(mcbsp, WAKEUPEN, 0);
> > 	}
> > }
>
> <snip>
>
> > 	/*
> > 	 * Worst case: CLKSRG*2 = 8000khz: (1/8000) * 2 * 2 usec
> >@@ -543,18 +531,18 @@ void omap_mcbsp_start(unsigned int id, i
> >
> > 	if (idle) {
> > 		/* Start frame sync */
> >-		w = OMAP_MCBSP_READ(io_base, SPCR2);
> >-		OMAP_MCBSP_WRITE(io_base, SPCR2, w | (1 << 7));
> >+		w = MCBSP_READ(mcbsp, SPCR2);
> >+		MCBSP_WRITE(mcbsp, SPCR2, w | (1 << 7));
> > 	}
> >
> > 	if (cpu_is_omap2430() || cpu_is_omap34xx()) {
>
> Why not 44xx?

Sorry, you have to ask the one who put it here, not me. This is beyond the 
scope of this patch. Or submit a patch by your own if already you know the 
answer.

> > 		/* Release the transmitter and receiver */
> >-		w = OMAP_MCBSP_READ(io_base, XCCR);
> >+		w = MCBSP_READ(mcbsp, XCCR);
> > 		w &= ~(tx ? XDISABLE : 0);
> >-		OMAP_MCBSP_WRITE(io_base, XCCR, w);
> >-		w = OMAP_MCBSP_READ(io_base, RCCR);
> >+		MCBSP_WRITE(mcbsp, XCCR, w);
> >+		w = MCBSP_READ(mcbsp, RCCR);
> > 		w &= ~(rx ? RDISABLE : 0);
> >-		OMAP_MCBSP_WRITE(io_base, RCCR, w);
> >+		MCBSP_WRITE(mcbsp, RCCR, w);
> > 	}
> >
> > 	/* Dump McBSP Regs */
> >@@ -565,7 +553,6 @@ EXPORT_SYMBOL(omap_mcbsp_start);
> > void omap_mcbsp_stop(unsigned int id, int tx, int rx)
> > {
> > 	struct omap_mcbsp *mcbsp;
> >-	void __iomem *io_base;
> > 	int idle;
> > 	u16 w;
> >
> >@@ -575,35 +562,33 @@ void omap_mcbsp_stop(unsigned int id, in
> > 	}
> >
> > 	mcbsp = id_to_mcbsp_ptr(id);
> >-	io_base = mcbsp->io_base;
> >
> > 	/* Reset transmitter */
> > 	tx &= 1;
> > 	if (cpu_is_omap2430() || cpu_is_omap34xx()) {
>
> Why not 44xx?

Ditto

> >-		w = OMAP_MCBSP_READ(io_base, XCCR);
> >+		w = MCBSP_READ(mcbsp, XCCR);
> > 		w |= (tx ? XDISABLE : 0);
> >-		OMAP_MCBSP_WRITE(io_base, XCCR, w);
> >+		MCBSP_WRITE(mcbsp, XCCR, w);
> > 	}
> >-	w = OMAP_MCBSP_READ(io_base, SPCR2);
> >-	OMAP_MCBSP_WRITE(io_base, SPCR2, w & ~tx);
> >+	w = MCBSP_READ(mcbsp, SPCR2);
> >+	MCBSP_WRITE(mcbsp, SPCR2, w & ~tx);
> >
> > 	/* Reset receiver */
> > 	rx &= 1;
> > 	if (cpu_is_omap2430() || cpu_is_omap34xx()) {
>
> Ditto
>
> >-		w = OMAP_MCBSP_READ(io_base, RCCR);
> >+		w = MCBSP_READ(mcbsp, RCCR);
> > 		w |= (rx ? RDISABLE : 0);
> >-		OMAP_MCBSP_WRITE(io_base, RCCR, w);
> >+		MCBSP_WRITE(mcbsp, RCCR, w);
> > 	}
> >-	w = OMAP_MCBSP_READ(io_base, SPCR1);
> >-	OMAP_MCBSP_WRITE(io_base, SPCR1, w & ~rx);
> >+	w = MCBSP_READ(mcbsp, SPCR1);
> >+	MCBSP_WRITE(mcbsp, SPCR1, w & ~rx);
> >
> >-	idle = !((OMAP_MCBSP_READ(io_base, SPCR2) |
> >-		  OMAP_MCBSP_READ(io_base, SPCR1)) & 1);
> >+	idle = !((MCBSP_READ(mcbsp, SPCR2) | MCBSP_READ(mcbsp,
> >SPCR1)) & 1);
> >
> > 	if (idle) {
> > 		/* Reset the sample rate generator */
> >-		w = OMAP_MCBSP_READ(io_base, SPCR2);
> >-		OMAP_MCBSP_WRITE(io_base, SPCR2, w & ~(1 << 6));
> >+		w = MCBSP_READ(mcbsp, SPCR2);
> >+		MCBSP_WRITE(mcbsp, SPCR2, w & ~(1 << 6));
> > 	}
> > }
> > EXPORT_SYMBOL(omap_mcbsp_stop);
> >@@ -612,7 +597,6 @@ EXPORT_SYMBOL(omap_mcbsp_stop);
> > int omap_mcbsp_pollwrite(unsigned int id, u16 buf)
> > {
> > 	struct omap_mcbsp *mcbsp;
> >-	void __iomem *base;
> >
> > 	if (!omap_mcbsp_check_valid_id(id)) {
> > 		printk(KERN_ERR "%s: Invalid id (%d)\n",
> >__func__, id + 1);
> >@@ -620,28 +604,25 @@ int omap_mcbsp_pollwrite(unsigned int id
> > 	}
> >
> > 	mcbsp = id_to_mcbsp_ptr(id);
> >-	base = mcbsp->io_base;
> >
> >-	OMAP_MCBSP_WRITE(base, DXR1, buf);
> >+	MCBSP_WRITE(mcbsp, DXR1, buf);
>
> OMAP3/4 allows 32 bit data access also.
> How is it taken care?

Not at all in case of those several particular changes above. I'm pretty sure 
that just replacing old OMAP_MCBSP_WRITE(base, ...) with new 
MCBSP_WRITE(mcbsp, ...) changed nothing in terms of OMAP3/4 32-bit data 
access. Again, beyond the scope.

> Please refer to 
> http://patchwork.kernel.org/patch/54896/ for more details.
>
> Quoting Tony's words:
> "To me it smells like the all drivers using the the
> omap_mcbsp_pollread/write are broken legacy drivers.
> So maybe we should just remove these two functions?
> If we really want to keep these around, we should review
> the drivers using these functions. "

Please have a look at patch 1/5, there readw()/writew() has been replaced with 
OMAP_MCBSP_READ()/_WRITE() inside omap_mcbsp_pollwrite()/_pollread(). If you 
think there is something wrong with it, please comment that patch, not this 
one that changes nothing related here, I believe.

> > 	/* if frame sync error - clear the error */
> >-	if (OMAP_MCBSP_READ(base, SPCR2) & XSYNC_ERR) {
> >+	if (MCBSP_READ(mcbsp, SPCR2) & XSYNC_ERR) {
> > 		/* clear error */
> >-		OMAP_MCBSP_WRITE(base, SPCR2,
> >-				OMAP_MCBSP_READ(base, SPCR2) &
> >(~XSYNC_ERR));
> >+		MCBSP_WRITE(mcbsp, SPCR2,
> >+				MCBSP_READ(mcbsp, SPCR2) &
> >(~XSYNC_ERR));
> > 		/* resend */
> > 		return -1;
> > 	} else {
> > 		/* wait for transmit confirmation */
> > 		int attemps = 0;
> >-		while (!(OMAP_MCBSP_READ(base, SPCR2) & XRDY)) {
> >+		while (!(MCBSP_READ(mcbsp, SPCR2) & XRDY)) {
> > 			if (attemps++ > 1000) {
> >-				OMAP_MCBSP_WRITE(base, SPCR2,
> >-
> >OMAP_MCBSP_READ(base, SPCR2) &
> >-						(~XRST));
> >+				MCBSP_WRITE(mcbsp, SPCR2,
> >+					    MCBSP_READ(mcbsp,
> >SPCR2) & (~XRST));
> > 				udelay(10);
> >-				OMAP_MCBSP_WRITE(base, SPCR2,
> >-
> >OMAP_MCBSP_READ(base, SPCR2) |
> >-						(XRST));
> >+				MCBSP_WRITE(mcbsp, SPCR2,
> >+					    MCBSP_READ(mcbsp,
> >SPCR2) | (XRST));
> > 				udelay(10);
> > 				dev_err(mcbsp->dev, "Could not write to"
> > 					" McBSP%d Register\n",
> >mcbsp->id);
> >@@ -657,7 +638,6 @@ EXPORT_SYMBOL(omap_mcbsp_pollwrite);
> > int omap_mcbsp_pollread(unsigned int id, u16 *buf)
> > {
> > 	struct omap_mcbsp *mcbsp;
> >-	void __iomem *base;
> >
> > 	if (!omap_mcbsp_check_valid_id(id)) {
> > 		printk(KERN_ERR "%s: Invalid id (%d)\n",
> >__func__, id + 1);
> >@@ -665,26 +645,23 @@ int omap_mcbsp_pollread(unsigned int id,
> > 	}
> > 	mcbsp = id_to_mcbsp_ptr(id);
> >
> >-	base = mcbsp->io_base;
> > 	/* if frame sync error - clear the error */
> >-	if (OMAP_MCBSP_READ(base, SPCR1) & RSYNC_ERR) {
> >+	if (MCBSP_READ(mcbsp, SPCR1) & RSYNC_ERR) {
> > 		/* clear error */
> >-		OMAP_MCBSP_WRITE(base, SPCR1,
> >-				OMAP_MCBSP_READ(base, SPCR1) &
> >(~RSYNC_ERR));
> >+		MCBSP_WRITE(mcbsp, SPCR1,
> >+				MCBSP_READ(mcbsp, SPCR1) &
> >(~RSYNC_ERR));
> > 		/* resend */
> > 		return -1;
> > 	} else {
> > 		/* wait for recieve confirmation */
> > 		int attemps = 0;
> >-		while (!(OMAP_MCBSP_READ(base, SPCR1) & RRDY)) {
> >+		while (!(MCBSP_READ(mcbsp, SPCR1) & RRDY)) {
> > 			if (attemps++ > 1000) {
> >-				OMAP_MCBSP_WRITE(base, SPCR1,
> >-
> >OMAP_MCBSP_READ(base, SPCR1) &
> >-						(~RRST));
> >+				MCBSP_WRITE(mcbsp, SPCR1,
> >+					    MCBSP_READ(mcbsp,
> >SPCR1) & (~RRST));
> > 				udelay(10);
> >-				OMAP_MCBSP_WRITE(base, SPCR1,
> >-
> >OMAP_MCBSP_READ(base, SPCR1) |
> >-						(RRST));
> >+				MCBSP_WRITE(mcbsp, SPCR1,
> >+					    MCBSP_READ(mcbsp,
> >SPCR1) | (RRST));
> > 				udelay(10);
> > 				dev_err(mcbsp->dev, "Could not
> >read from"
> > 					" McBSP%d Register\n",
> >mcbsp->id);
> >@@ -692,7 +669,7 @@ int omap_mcbsp_pollread(unsigned int id,
> > 			}
> > 		}
> > 	}
> >-	*buf = OMAP_MCBSP_READ(base, DRR1);
> >+	*buf = MCBSP_READ(mcbsp, DRR1);
> >
> > 	return 0;
> > }
>
> <snip>
>
> Was this code tested for different configurations
> like DMA mode, poll mode?

I have only tested it on a hardware that I own, ie. OMAP1510 Amstrad Delta, 
using existing ASoC OMAP driver (I'm pretty sure you know it uses DMA mode). 
I never pretended the patch being tested on any other hardware/with any other 
client. Please test and report if you can.

To resume, even if your concerns about mcbsp code quality were perfectly 
valid, they still seem not related to the patch you are just commenting. 
Please correct me if I'm wrong.

And if you are trying to say that while addressing one issue I should care of 
any other that can be pointed out, then sorry, I'm not that capable. If your 
concearns are confirmed as valid, I can try to take care of them later, when 
I find some spare time.

Thanks,
Janusz
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v7 2/5] OMAP: McBSP: Modify macros/functions API for easy cache access

[Varadarajan, Charu Latha]

<snip>

>> >@@ -612,7 +597,6 @@ EXPORT_SYMBOL(omap_mcbsp_stop);
>> > int omap_mcbsp_pollwrite(unsigned int id, u16 buf)
>> > {
>> > 	struct omap_mcbsp *mcbsp;
>> >-	void __iomem *base;
>> >
>> > 	if (!omap_mcbsp_check_valid_id(id)) {
>> > 		printk(KERN_ERR "%s: Invalid id (%d)\n",
>> >__func__, id + 1);
>> >@@ -620,28 +604,25 @@ int omap_mcbsp_pollwrite(unsigned int id
>> > 	}
>> >
>> > 	mcbsp = id_to_mcbsp_ptr(id);
>> >-	base = mcbsp->io_base;
>> >
>> >-	OMAP_MCBSP_WRITE(base, DXR1, buf);
>> >+	MCBSP_WRITE(mcbsp, DXR1, buf);
>>
>> OMAP3/4 allows 32 bit data access also.
>> How is it taken care?
>
>Not at all in case of those several particular changes above. 
>I'm pretty sure 
>that just replacing old OMAP_MCBSP_WRITE(base, ...) with new 
>MCBSP_WRITE(mcbsp, ...) changed nothing in terms of OMAP3/4 
>32-bit data 
>access. Again, beyond the scope.
>
>> Please refer to 
>> http://patchwork.kernel.org/patch/54896/ for more details.
>>
>> Quoting Tony's words:
>> "To me it smells like the all drivers using the the
>> omap_mcbsp_pollread/write are broken legacy drivers.
>> So maybe we should just remove these two functions?
>> If we really want to keep these around, we should review
>> the drivers using these functions. "
>
>Please have a look at patch 1/5, there readw()/writew() has 
>been replaced with 
>OMAP_MCBSP_READ()/_WRITE() inside 
>omap_mcbsp_pollwrite()/_pollread(). If you 
>think there is something wrong with it, please comment that 
>patch, not this 
>one that changes nothing related here, I believe.

The point here is to review the driver before pushing any patch
touching these functions.

<snip>

Re: [PATCH v7 2/5] OMAP: McBSP: Modify macros/functions API for easy cache access

[Janusz Krzysztofik]

Monday 14 December 2009 07:05:06 Varadarajan, Charu Latha napisał(a):
> <snip>
>
> >> >@@ -612,7 +597,6 @@ EXPORT_SYMBOL(omap_mcbsp_stop);
> >> > int omap_mcbsp_pollwrite(unsigned int id, u16 buf)
> >> > {
> >> > 	struct omap_mcbsp *mcbsp;
> >> >-	void __iomem *base;
> >> >
> >> > 	if (!omap_mcbsp_check_valid_id(id)) {
> >> > 		printk(KERN_ERR "%s: Invalid id (%d)\n",
> >> >__func__, id + 1);
> >> >@@ -620,28 +604,25 @@ int omap_mcbsp_pollwrite(unsigned int id
> >> > 	}
> >> >
> >> > 	mcbsp = id_to_mcbsp_ptr(id);
> >> >-	base = mcbsp->io_base;
> >> >
> >> >-	OMAP_MCBSP_WRITE(base, DXR1, buf);
> >> >+	MCBSP_WRITE(mcbsp, DXR1, buf);
> >>
> >> OMAP3/4 allows 32 bit data access also.
> >> How is it taken care?
> >
> >Not at all in case of those several particular changes above.
> >I'm pretty sure
> >that just replacing old OMAP_MCBSP_WRITE(base, ...) with new
> >MCBSP_WRITE(mcbsp, ...) changed nothing in terms of OMAP3/4
> >32-bit data
> >access. Again, beyond the scope.
> >
> >> Please refer to
> >> http://patchwork.kernel.org/patch/54896/ for more details.
> >>
> >> Quoting Tony's words:
> >> "To me it smells like the all drivers using the the
> >> omap_mcbsp_pollread/write are broken legacy drivers.
> >> So maybe we should just remove these two functions?
> >> If we really want to keep these around, we should review
> >> the drivers using these functions. "
> >
> >Please have a look at patch 1/5, there readw()/writew() has
> >been replaced with
> >OMAP_MCBSP_READ()/_WRITE() inside
> >omap_mcbsp_pollwrite()/_pollread(). If you
> >think there is something wrong with it, please comment that
> >patch, not this
> >one that changes nothing related here, I believe.
>
> The point here is to review the driver before pushing any patch
> touching these functions.
>
> <snip>

If these functions are obsolete and going to be removed, I don't think it 
could be of any importance whether they are modified before removal or not. 
Otherwise, a solution seems simple to me: you submit a patch that addresses 
all concearns, yours, Tony's (BTW, have you already reviewed the drivers as 
Tony suggested?), maybe others. Then, after your patch is accepted for 
inclusion and it appears in conflict with this series still not applied for 
any reason (possibly waiting for your changes if that decided), Jarkko, or 
Tony, or anyone else pointed out by Tony, decides which one goes first and 
which is going to be refreshed on top of the other. Does it sound like a good 
plan for you?

Thanks,
Janusz
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7 2/5] OMAP: McBSP: Modify macros/functions API for easy cache access

[Jarkko Nikula]

On Mon, 14 Dec 2009 11:11:27 +0100
Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> wrote:

> If these functions are obsolete and going to be removed, I don't think it 
> could be of any importance whether they are modified before removal or not. 
> Otherwise, a solution seems simple to me: you submit a patch that addresses 
> all concearns, yours, Tony's (BTW, have you already reviewed the drivers as 
> Tony suggested?), maybe others. Then, after your patch is accepted for 
> inclusion and it appears in conflict with this series still not applied for 
> any reason (possibly waiting for your changes if that decided), Jarkko, or 
> Tony, or anyone else pointed out by Tony, decides which one goes first and 
> which is going to be refreshed on top of the other. Does it sound like a good 
> plan for you?
> 
I would favor the Janusz's set going in first since it will solve and
help the in-tree problems without making out-of-tree use any worse than
currently.

- Fixes register access in polled I/O functions (out-of-tree use)
- Fixes McBSP register corruption noted on Amstrad Delta
- McBSP register caching could help the PM development

Fixing any other issues in polled I/O API belongs to another context
than this patch set and IMO is easier to handle after this set since the
patch 1/5 already fixes the register access width.


-- 
Jarkko

Re: [PATCH v7 2/5] OMAP: McBSP: Modify macros/functions API for easy cache access

[Tony Lindgren]

* Jarkko Nikula <jhnikula@gmail.com> [091214 03:12]:
> On Mon, 14 Dec 2009 11:11:27 +0100
> Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> wrote:
> 
> > If these functions are obsolete and going to be removed, I don't think it 
> > could be of any importance whether they are modified before removal or not. 
> > Otherwise, a solution seems simple to me: you submit a patch that addresses 
> > all concearns, yours, Tony's (BTW, have you already reviewed the drivers as 
> > Tony suggested?), maybe others. Then, after your patch is accepted for 
> > inclusion and it appears in conflict with this series still not applied for 
> > any reason (possibly waiting for your changes if that decided), Jarkko, or 
> > Tony, or anyone else pointed out by Tony, decides which one goes first and 
> > which is going to be refreshed on top of the other. Does it sound like a good 
> > plan for you?
> > 
> I would favor the Janusz's set going in first since it will solve and
> help the in-tree problems without making out-of-tree use any worse than
> currently.
> 
> - Fixes register access in polled I/O functions (out-of-tree use)
> - Fixes McBSP register corruption noted on Amstrad Delta
> - McBSP register caching could help the PM development
> 
> Fixing any other issues in polled I/O API belongs to another context
> than this patch set and IMO is easier to handle after this set since the
> patch 1/5 already fixes the register access width.

Sounds good to me.

Tony

RE: [PATCH v7 2/5] OMAP: McBSP: Modify macros/functions API for easy cache access

[Varadarajan, Charu Latha]

 

>-----Original Message-----
>From: Tony Lindgren [mailto:tony@atomide.com] 
>Sent: Tuesday, December 15, 2009 1:06 AM
>To: Jarkko Nikula
>Cc: Janusz Krzysztofik; Varadarajan, Charu Latha; Peter 
>Ujfalusi; linux-omap@vger.kernel.org
>Subject: Re: [PATCH v7 2/5] OMAP: McBSP: Modify 
>macros/functions API for easy cache access
>
>* Jarkko Nikula <jhnikula@gmail.com> [091214 03:12]:
>> On Mon, 14 Dec 2009 11:11:27 +0100
>> Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> wrote:
>> 
>> > If these functions are obsolete and going to be removed, I 
>don't think it 
>> > could be of any importance whether they are modified 
>before removal or not. 
>> > Otherwise, a solution seems simple to me: you submit a 
>patch that addresses 
>> > all concearns, yours, Tony's (BTW, have you already 
>reviewed the drivers as 
>> > Tony suggested?), maybe others. Then, after your patch is 
>accepted for 
>> > inclusion and it appears in conflict with this series 
>still not applied for 
>> > any reason (possibly waiting for your changes if that 
>decided), Jarkko, or 
>> > Tony, or anyone else pointed out by Tony, decides which 
>one goes first and 
>> > which is going to be refreshed on top of the other. Does 
>it sound like a good 
>> > plan for you?
>> > 
>> I would favor the Janusz's set going in first since it will solve and
>> help the in-tree problems without making out-of-tree use any 
>worse than
>> currently.
>> 
>> - Fixes register access in polled I/O functions (out-of-tree use)
>> - Fixes McBSP register corruption noted on Amstrad Delta
>> - McBSP register caching could help the PM development
>> 
>> Fixing any other issues in polled I/O API belongs to another context
>> than this patch set and IMO is easier to handle after this 
>set since the
>> patch 1/5 already fixes the register access width.
>
>Sounds good to me.

Tony,
As pointed by you, the McBSP driver is having broken APIs. Please share your 
plans wrt McBSP driver. 
1) If there are plans to review and clean up the code,
we will focus on the same.
2) Else if the above is planned sometime in the future,
can you consider the patch for fixing McBSP poll mode
(http://patchwork.kernel.org/patch/54896/) as it is important to have a 
working code in place? If okay, I shall post a new version 
of the mentioned patch.

>
>Tony
>

Re: [PATCH v7 2/5] OMAP: McBSP: Modify macros/functions API for easy cache access

[Janusz Krzysztofik]

Tuesday 22 December 2009 09:58:37 Varadarajan, Charu Latha napisał(a):
> >-----Original Message-----
> >From: Tony Lindgren [mailto:tony@atomide.com]
> >Sent: Tuesday, December 15, 2009 1:06 AM
> >To: Jarkko Nikula
> >Cc: Janusz Krzysztofik; Varadarajan, Charu Latha; Peter
> >Ujfalusi; linux-omap@vger.kernel.org
> >Subject: Re: [PATCH v7 2/5] OMAP: McBSP: Modify
> >macros/functions API for easy cache access
> >
> >* Jarkko Nikula <jhnikula@gmail.com> [091214 03:12]:
> >> On Mon, 14 Dec 2009 11:11:27 +0100
> >> Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> wrote:
> >> > If these functions are obsolete and going to be removed, I
> >
> >don't think it
> >
> >> > could be of any importance whether they are modified
> >
> >before removal or not.
> >
> >> > Otherwise, a solution seems simple to me: you submit a
> >
> >patch that addresses
> >
> >> > all concearns, yours, Tony's (BTW, have you already
> >
> >reviewed the drivers as
> >
> >> > Tony suggested?), maybe others. Then, after your patch is
> >
> >accepted for
> >
> >> > inclusion and it appears in conflict with this series
> >
> >still not applied for
> >
> >> > any reason (possibly waiting for your changes if that
> >
> >decided), Jarkko, or
> >
> >> > Tony, or anyone else pointed out by Tony, decides which
> >
> >one goes first and
> >
> >> > which is going to be refreshed on top of the other. Does
> >
> >it sound like a good
> >
> >> > plan for you?
> >>
> >> I would favor the Janusz's set going in first since it will solve and
> >> help the in-tree problems without making out-of-tree use any
> >
> >worse than
> >
> >> currently.
> >>
> >> - Fixes register access in polled I/O functions (out-of-tree use)
> >> - Fixes McBSP register corruption noted on Amstrad Delta
> >> - McBSP register caching could help the PM development
> >>
> >> Fixing any other issues in polled I/O API belongs to another context
> >> than this patch set and IMO is easier to handle after this
> >
> >set since the
> >
> >> patch 1/5 already fixes the register access width.
> >
> >Sounds good to me.
>
> Tony,
> As pointed by you, the McBSP driver is having broken APIs. Please share
> your plans wrt McBSP driver.
> 1) If there are plans to review and clean up the code,
> we will focus on the same.
> 2) Else if the above is planned sometime in the future,
> can you consider the patch for fixing McBSP poll mode
> (http://patchwork.kernel.org/patch/54896/) as it is important to have a
> working code in place? If okay, I shall post a new version
> of the mentioned patch.
>
> >Tony

Charu,

Now I can see the source of your frustration: my v3 patch 1/4 
(http://patchwork.kernel.org/patch/63839/), followed by v7 1/5, is almost 
identical to v1 of yours mentioned above 
(http://patchwork.kernel.org/patch/53631/), that is earlier. I'm sorry, but I 
was not aware of that, neither when submitting my patch nor later referring 
your comments that you happened to place against my patch v7 2/5, not 1/5.

The difference is that my patch only modifies  
omap_mcbsp_pollread()/_pollwrite() function bodies by replacing 
readw()/writew() with OMAP_MCBSP_READ()/_WRITE(), while yours changes the 
functions API as well. Even if my rationale was different from fixing 32-bit 
register access inside these functions, it just happend to fix it since 
OMAP_MCBSP_READ()/_WRITE() handle 32-bit register access correctly unlike 
readw()/writew() that are 16-bit.

As we can see from what Jarkko says, our changes inside function bodies are 
perfectly acceptable, while your API changes are not.

That said, if you could remove API changes from your v1 patch, keeping those 
function to macro replacements only, you should be able to get it accepted by 
Jarkko, Peter and Tony. I think you could even get it accepted as a fix for 
the current rc cycle if you submit it promptly.

If you get this way, I'll drop my patch 1/5 in favour of yours. 
Please let me know your decision as soon as possible since now, as my fix on 
omap_mcbsp_request() has been applied (Tony, thanks), I'm going to resubmit a 
refreshed version of my register caching set in a few days.

Thanks,
Janusz
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v7 3/5] OMAP: McBSP: Introduce caching in register write operations

[Janusz Krzysztofik]

Determine cache size required per McBSP port at init time, based on processor
type running on.
Allocate space for storing cached copies of McBSP register values at port
request.
Modify omap_msbcp_write() function to update the cache with every register
write operation.
Modify omap_mcbsp_read() to support reading from cache or hardware.
Update MCBSP_READ() macro for modified omap_mcbsp_read() function API.
Introduce a new macro that reads from the cache.

Applies on top of patch 2 from this series:
[PATCH v7 2/5] OMAP: McBSP: Modify macros/functions API for easy cache access

Tested on OMAP1510 based Amstrad Delta using linux-omap for-next,
commit 82f1d8f22f2c65e70206e40a6f17688bf64a892c.
Compile-tested with: omap_perseus2_730_defconfig, omap_generic_1610_defconfig,
omap_generic_2420_defconfig, omap_2430sdp_defconfig, omap_3430sdp_defconfig,
omap_4430sdp_defconfig with CONFIG_OMAP_MCBSP=y selected.

Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>

---
Changes since v3:
- replace cache table with cache pointer inside omap_mcbsp structure,
- determine cache size at runtime, allocate dynamically when port requested,
- get rid of ifdef...else by placing processor dependent snippets of new code 
  correctly in respective source files.

 arch/arm/mach-omap1/mcbsp.c             |   16 ++++++++++---
 arch/arm/mach-omap2/mcbsp.c             |   20 +++++++++++++---
 arch/arm/plat-omap/include/plat/mcbsp.h |    3 +-
 arch/arm/plat-omap/mcbsp.c              |   39 ++++++++++++++++++++++++--------
 4 files changed, 61 insertions(+), 17 deletions(-)

diff -upr git.orig/arch/arm/mach-omap1/mcbsp.c git/arch/arm/mach-omap1/mcbsp.c
--- git.orig/arch/arm/mach-omap1/mcbsp.c	2009-12-02 15:48:37.000000000 +0100
+++ git/arch/arm/mach-omap1/mcbsp.c	2009-12-09 15:35:14.000000000 +0100
@@ -99,9 +99,11 @@ static struct omap_mcbsp_platform_data o
 	},
 };
 #define OMAP7XX_MCBSP_PDATA_SZ		ARRAY_SIZE(omap7xx_mcbsp_pdata)
+#define OMAP7XX_MCBSP_REG_NUM		(OMAP_MCBSP_REG_XCERH / sizeof(u16) + 1)
 #else
 #define omap7xx_mcbsp_pdata		NULL
 #define OMAP7XX_MCBSP_PDATA_SZ		0
+#define OMAP7XX_MCBSP_REG_NUM		0
 #endif
 
 #ifdef CONFIG_ARCH_OMAP15XX
@@ -132,9 +134,11 @@ static struct omap_mcbsp_platform_data o
 	},
 };
 #define OMAP15XX_MCBSP_PDATA_SZ		ARRAY_SIZE(omap15xx_mcbsp_pdata)
+#define OMAP15XX_MCBSP_REG_NUM		(OMAP_MCBSP_REG_XCERH / sizeof(u16) + 1)
 #else
 #define omap15xx_mcbsp_pdata		NULL
 #define OMAP15XX_MCBSP_PDATA_SZ		0
+#define OMAP15XX_MCBSP_REG_NUM		0
 #endif
 
 #ifdef CONFIG_ARCH_OMAP16XX
@@ -165,19 +169,25 @@ static struct omap_mcbsp_platform_data o
 	},
 };
 #define OMAP16XX_MCBSP_PDATA_SZ		ARRAY_SIZE(omap16xx_mcbsp_pdata)
+#define OMAP16XX_MCBSP_REG_NUM		(OMAP_MCBSP_REG_XCERH / sizeof(u16) + 1)
 #else
 #define omap16xx_mcbsp_pdata		NULL
 #define OMAP16XX_MCBSP_PDATA_SZ		0
+#define OMAP16XX_MCBSP_REG_NUM		0
 #endif
 
 int __init omap1_mcbsp_init(void)
 {
-	if (cpu_is_omap7xx())
+	if (cpu_is_omap7xx()) {
 		omap_mcbsp_count = OMAP7XX_MCBSP_PDATA_SZ;
-	if (cpu_is_omap15xx())
+		omap_mcbsp_cache_size = OMAP7XX_MCBSP_REG_NUM * sizeof(u16);
+	} else if (cpu_is_omap15xx()) {
 		omap_mcbsp_count = OMAP15XX_MCBSP_PDATA_SZ;
-	if (cpu_is_omap16xx())
+		omap_mcbsp_cache_size = OMAP15XX_MCBSP_REG_NUM * sizeof(u16);
+	} else if (cpu_is_omap16xx()) {
 		omap_mcbsp_count = OMAP16XX_MCBSP_PDATA_SZ;
+		omap_mcbsp_cache_size = OMAP16XX_MCBSP_REG_NUM * sizeof(u16);
+	}
 
 	mcbsp_ptr = kzalloc(omap_mcbsp_count * sizeof(struct omap_mcbsp *),
 								GFP_KERNEL);
diff -upr git.orig/arch/arm/mach-omap2/mcbsp.c git/arch/arm/mach-omap2/mcbsp.c
--- git.orig/arch/arm/mach-omap2/mcbsp.c	2009-12-02 15:48:38.000000000 +0100
+++ git/arch/arm/mach-omap2/mcbsp.c	2009-12-09 15:35:14.000000000 +0100
@@ -65,9 +65,11 @@ static struct omap_mcbsp_platform_data o
 	},
 };
 #define OMAP2420_MCBSP_PDATA_SZ		ARRAY_SIZE(omap2420_mcbsp_pdata)
+#define OMAP2420_MCBSP_REG_NUM		(OMAP_MCBSP_REG_RCCR / sizeof(u32) + 1)
 #else
 #define omap2420_mcbsp_pdata		NULL
 #define OMAP2420_MCBSP_PDATA_SZ		0
+#define OMAP2420_MCBSP_REG_NUM		0
 #endif
 
 #ifdef CONFIG_ARCH_OMAP2430
@@ -114,9 +116,11 @@ static struct omap_mcbsp_platform_data o
 	},
 };
 #define OMAP2430_MCBSP_PDATA_SZ		ARRAY_SIZE(omap2430_mcbsp_pdata)
+#define OMAP2430_MCBSP_REG_NUM		(OMAP_MCBSP_REG_RCCR / sizeof(u32) + 1)
 #else
 #define omap2430_mcbsp_pdata		NULL
 #define OMAP2430_MCBSP_PDATA_SZ		0
+#define OMAP2430_MCBSP_REG_NUM		0
 #endif
 
 #ifdef CONFIG_ARCH_OMAP34XX
@@ -168,9 +172,11 @@ static struct omap_mcbsp_platform_data o
 	},
 };
 #define OMAP34XX_MCBSP_PDATA_SZ		ARRAY_SIZE(omap34xx_mcbsp_pdata)
+#define OMAP34XX_MCBSP_REG_NUM		(OMAP_MCBSP_REG_RCCR / sizeof(u32) + 1)
 #else
 #define omap34xx_mcbsp_pdata		NULL
 #define OMAP34XX_MCBSP_PDATA_SZ		0
+#define OMAP34XX_MCBSP_REG_NUM		0
 #endif
 
 static struct omap_mcbsp_platform_data omap44xx_mcbsp_pdata[] = {
@@ -208,17 +214,23 @@ static struct omap_mcbsp_platform_data o
 	},
 };
 #define OMAP44XX_MCBSP_PDATA_SZ		ARRAY_SIZE(omap44xx_mcbsp_pdata)
+#define OMAP44XX_MCBSP_REG_NUM		(OMAP_MCBSP_REG_RCCR / sizeof(u32) + 1)
 
 static int __init omap2_mcbsp_init(void)
 {
-	if (cpu_is_omap2420())
+	if (cpu_is_omap2420()) {
 		omap_mcbsp_count = OMAP2420_MCBSP_PDATA_SZ;
-	if (cpu_is_omap2430())
+		omap_mcbsp_cache_size = OMAP2420_MCBSP_REG_NUM * sizeof(u16);
+	} else if (cpu_is_omap2430()) {
 		omap_mcbsp_count = OMAP2430_MCBSP_PDATA_SZ;
-	if (cpu_is_omap34xx())
+		omap_mcbsp_cache_size = OMAP2430_MCBSP_REG_NUM * sizeof(u32);
+	} else if (cpu_is_omap34xx()) {
 		omap_mcbsp_count = OMAP34XX_MCBSP_PDATA_SZ;
-	if (cpu_is_omap44xx())
+		omap_mcbsp_cache_size = OMAP34XX_MCBSP_REG_NUM * sizeof(u32);
+	} else if (cpu_is_omap44xx()) {
 		omap_mcbsp_count = OMAP44XX_MCBSP_PDATA_SZ;
+		omap_mcbsp_cache_size = OMAP44XX_MCBSP_REG_NUM * sizeof(u32);
+	}
 
 	mcbsp_ptr = kzalloc(omap_mcbsp_count * sizeof(struct omap_mcbsp *),
 								GFP_KERNEL);
diff -upr git.orig/arch/arm/plat-omap/include/plat/mcbsp.h git/arch/arm/plat-omap/include/plat/mcbsp.h
--- git.orig/arch/arm/plat-omap/include/plat/mcbsp.h	2009-12-02 15:48:51.000000000 +0100
+++ git/arch/arm/plat-omap/include/plat/mcbsp.h	2009-12-09 15:35:14.000000000 +0100
@@ -415,9 +415,10 @@ struct omap_mcbsp {
 	u16 max_tx_thres;
 	u16 max_rx_thres;
 #endif
+	void *reg_cache;
 };
 extern struct omap_mcbsp **mcbsp_ptr;
-extern int omap_mcbsp_count;
+extern int omap_mcbsp_count, omap_mcbsp_cache_size;
 
 int omap_mcbsp_init(void);
 void omap_mcbsp_register_board_cfg(struct omap_mcbsp_platform_data *config,
diff -upr git.orig/arch/arm/plat-omap/mcbsp.c git/arch/arm/plat-omap/mcbsp.c
--- git.orig/arch/arm/plat-omap/mcbsp.c	2009-12-09 15:34:57.000000000 +0100
+++ git/arch/arm/plat-omap/mcbsp.c	2009-12-09 15:35:14.000000000 +0100
@@ -28,28 +28,42 @@
 #include <plat/mcbsp.h>
 
 struct omap_mcbsp **mcbsp_ptr;
-int omap_mcbsp_count;
+int omap_mcbsp_count, omap_mcbsp_cache_size;
 
 void omap_mcbsp_write(struct omap_mcbsp *mcbsp, u16 reg, u32 val)
 {
-	if (cpu_class_is_omap1() || cpu_is_omap2420())
+	if (cpu_class_is_omap1()) {
+		((u16 *)mcbsp->reg_cache)[reg / sizeof(u16)] = (u16)val;
 		__raw_writew((u16)val, mcbsp->io_base + reg);
-	else
+	} else if (cpu_is_omap2420()) {
+		((u16 *)mcbsp->reg_cache)[reg / sizeof(u32)] = (u16)val;
+		__raw_writew((u16)val, mcbsp->io_base + reg);
+	} else {
+		((u32 *)mcbsp->reg_cache)[reg / sizeof(u32)] = val;
 		__raw_writel(val, mcbsp->io_base + reg);
+	}
 }
 
-int omap_mcbsp_read(struct omap_mcbsp *mcbsp, u16 reg)
+int omap_mcbsp_read(struct omap_mcbsp *mcbsp, u16 reg, bool from_cache)
 {
-	if (cpu_class_is_omap1() || cpu_is_omap2420())
-		return __raw_readw(mcbsp->io_base + reg);
-	else
-		return __raw_readl(mcbsp->io_base + reg);
+	if (cpu_class_is_omap1()) {
+		return !from_cache ? __raw_readw(mcbsp->io_base + reg) :
+				((u16 *)mcbsp->reg_cache)[reg / sizeof(u16)];
+	} else if (cpu_is_omap2420()) {
+		return !from_cache ? __raw_readw(mcbsp->io_base + reg) :
+				((u16 *)mcbsp->reg_cache)[reg / sizeof(u32)];
+	} else {
+		return !from_cache ? __raw_readl(mcbsp->io_base + reg) :
+				((u32 *)mcbsp->reg_cache)[reg / sizeof(u32)];
+	}
 }
 
 #define MCBSP_READ(mcbsp, reg) \
-		omap_mcbsp_read(mcbsp, OMAP_MCBSP_REG_##reg)
+		omap_mcbsp_read(mcbsp, OMAP_MCBSP_REG_##reg, 0)
 #define MCBSP_WRITE(mcbsp, reg, val) \
 		omap_mcbsp_write(mcbsp, OMAP_MCBSP_REG_##reg, val)
+#define MCBSP_READ_CACHE(mcbsp, reg) \
+		omap_mcbsp_read(mcbsp, OMAP_MCBSP_REG_##reg, 1)
 
 #define omap_mcbsp_check_valid_id(id)	(id < omap_mcbsp_count)
 #define id_to_mcbsp_ptr(id)		mcbsp_ptr[id];
@@ -391,6 +405,10 @@ int omap_mcbsp_request(unsigned int id)
 	}
 	mcbsp = id_to_mcbsp_ptr(id);
 
+	mcbsp->reg_cache = kzalloc(omap_mcbsp_cache_size, GFP_KERNEL);
+	if (!mcbsp->reg_cache)
+		return -ENOMEM;
+
 	spin_lock(&mcbsp->lock);
 	if (!mcbsp->free) {
 		dev_err(mcbsp->dev, "McBSP%d is currently in use\n",
@@ -481,6 +499,9 @@ void omap_mcbsp_free(unsigned int id)
 
 	mcbsp->free = 1;
 	spin_unlock(&mcbsp->lock);
+
+	kfree(mcbsp->reg_cache);
+	mcbsp->reg_cache = NULL;
 }
 EXPORT_SYMBOL(omap_mcbsp_free);
 

Re: [PATCH v7 3/5] OMAP: McBSP: Introduce caching in register write operations

[Jarkko Nikula]

On Wed, 9 Dec 2009 21:31:20 +0100
Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> wrote:

> @@ -391,6 +405,10 @@ int omap_mcbsp_request(unsigned int id)
>  	}
>  	mcbsp = id_to_mcbsp_ptr(id);
>  
> +	mcbsp->reg_cache = kzalloc(omap_mcbsp_cache_size, GFP_KERNEL);
> +	if (!mcbsp->reg_cache)
> +		return -ENOMEM;
> +
>  	spin_lock(&mcbsp->lock);
>  	if (!mcbsp->free) {
>  		dev_err(mcbsp->dev, "McBSP%d is currently in use\n",

Great work. Could have my ack to patches 1-4 by moving the cache
allocation after the mcbsp->free test lines. Memory leak and other
badness would happen otherwise in case of multiple call to
omap_mcbsp_request.

Just send an update to this mail if there is no comments to another
patches.


-- 
Jarkko

Re: [PATCH v7 3/5] OMAP: McBSP: Introduce caching in register write operations

[Janusz Krzysztofik]

Friday 11 December 2009 14:11:49 Jarkko Nikula napisał(a):
> On Wed, 9 Dec 2009 21:31:20 +0100
>
> Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> wrote:
> > @@ -391,6 +405,10 @@ int omap_mcbsp_request(unsigned int id)
> >  	}
> >  	mcbsp = id_to_mcbsp_ptr(id);
> >
> > +	mcbsp->reg_cache = kzalloc(omap_mcbsp_cache_size, GFP_KERNEL);
> > +	if (!mcbsp->reg_cache)
> > +		return -ENOMEM;
> > +
> >  	spin_lock(&mcbsp->lock);
> >  	if (!mcbsp->free) {
> >  		dev_err(mcbsp->dev, "McBSP%d is currently in use\n",
>
> Great work. Could have my ack to patches 1-4 by moving the cache
> allocation after the mcbsp->free test lines. Memory leak and other
> badness would happen otherwise in case of multiple call to
> omap_mcbsp_request.

Yes, it's a bug, sorry.

> Just send an update to this mail if there is no comments to another
> patches.

OK.

Thanks,
Janusz

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/5 v8] OMAP: McBSP: Introduce caching in register write operations

[Janusz Krzysztofik]

Determine cache size required per McBSP port at init time, based on processor
type running on.
Allocate space for storing cached copies of McBSP register values at port
request.
Modify omap_msbcp_write() function to update the cache with every register
write operation.
Modify omap_mcbsp_read() to support reading from cache or hardware.
Update MCBSP_READ() macro for modified omap_mcbsp_read() function API.
Introduce a new macro that reads from the cache.

Applies on top of patch 2 from this series:
[PATCH v7 2/5] OMAP: McBSP: Modify macros/functions API for easy cache access

Tested on OMAP1510 based Amstrad Delta using linux-omap for-next,
commit 82f1d8f22f2c65e70206e40a6f17688bf64a892c.
Compile-tested with: omap_perseus2_730_defconfig, omap_generic_1610_defconfig,
omap_generic_2420_defconfig, omap_2430sdp_defconfig, omap_3430sdp_defconfig,
omap_4430sdp_defconfig with CONFIG_OMAP_MCBSP=y selected.

Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>

---
Friday 11 December 2009 14:51:27 Janusz Krzysztofik napisał(a):
> Friday 11 December 2009 14:11:49 Jarkko Nikula napisał(a):
> >
> > ... Could have my ack to patches 1-4 by moving the cache
> > allocation after the mcbsp->free test lines. Memory leak and other
> > badness would happen otherwise in case of multiple call to
> > omap_mcbsp_request.
>
> Yes, it's a bug, sorry.

Actually, three distinct bugs could be recognized, not one :/
1. Possible overwriting of mcbsp->reg_cache pointer for an already occupied 
   port.
2. Not freeing allocated cache memory on return after the port appears not 
   free.
3. In omap_mcbsp_free(), marking the port as free before deallocating the 
   cache, could result in memory leak as well.

I hope all are addressed correctly. Since releasing the port with

	mcbsp->free = 1;

after kzalloc() failure should be atomic, I have assumed this doesn't require 
locking.

Tested both alone and with patch v7 4/5 on top, just for case.

Thanks,
Janusz

diff -upr git.orig/arch/arm/mach-omap1/mcbsp.c git/arch/arm/mach-omap1/mcbsp.c
--- git.orig/arch/arm/mach-omap1/mcbsp.c	2009-12-02 15:48:37.000000000 +0100
+++ git/arch/arm/mach-omap1/mcbsp.c	2009-12-14 21:32:56.000000000 +0100
@@ -99,9 +99,11 @@ static struct omap_mcbsp_platform_data o
 	},
 };
 #define OMAP7XX_MCBSP_PDATA_SZ		ARRAY_SIZE(omap7xx_mcbsp_pdata)
+#define OMAP7XX_MCBSP_REG_NUM		(OMAP_MCBSP_REG_XCERH / sizeof(u16) + 1)
 #else
 #define omap7xx_mcbsp_pdata		NULL
 #define OMAP7XX_MCBSP_PDATA_SZ		0
+#define OMAP7XX_MCBSP_REG_NUM		0
 #endif
 
 #ifdef CONFIG_ARCH_OMAP15XX
@@ -132,9 +134,11 @@ static struct omap_mcbsp_platform_data o
 	},
 };
 #define OMAP15XX_MCBSP_PDATA_SZ		ARRAY_SIZE(omap15xx_mcbsp_pdata)
+#define OMAP15XX_MCBSP_REG_NUM		(OMAP_MCBSP_REG_XCERH / sizeof(u16) + 1)
 #else
 #define omap15xx_mcbsp_pdata		NULL
 #define OMAP15XX_MCBSP_PDATA_SZ		0
+#define OMAP15XX_MCBSP_REG_NUM		0
 #endif
 
 #ifdef CONFIG_ARCH_OMAP16XX
@@ -165,19 +169,25 @@ static struct omap_mcbsp_platform_data o
 	},
 };
 #define OMAP16XX_MCBSP_PDATA_SZ		ARRAY_SIZE(omap16xx_mcbsp_pdata)
+#define OMAP16XX_MCBSP_REG_NUM		(OMAP_MCBSP_REG_XCERH / sizeof(u16) + 1)
 #else
 #define omap16xx_mcbsp_pdata		NULL
 #define OMAP16XX_MCBSP_PDATA_SZ		0
+#define OMAP16XX_MCBSP_REG_NUM		0
 #endif
 
 int __init omap1_mcbsp_init(void)
 {
-	if (cpu_is_omap7xx())
+	if (cpu_is_omap7xx()) {
 		omap_mcbsp_count = OMAP7XX_MCBSP_PDATA_SZ;
-	if (cpu_is_omap15xx())
+		omap_mcbsp_cache_size = OMAP7XX_MCBSP_REG_NUM * sizeof(u16);
+	} else if (cpu_is_omap15xx()) {
 		omap_mcbsp_count = OMAP15XX_MCBSP_PDATA_SZ;
-	if (cpu_is_omap16xx())
+		omap_mcbsp_cache_size = OMAP15XX_MCBSP_REG_NUM * sizeof(u16);
+	} else if (cpu_is_omap16xx()) {
 		omap_mcbsp_count = OMAP16XX_MCBSP_PDATA_SZ;
+		omap_mcbsp_cache_size = OMAP16XX_MCBSP_REG_NUM * sizeof(u16);
+	}
 
 	mcbsp_ptr = kzalloc(omap_mcbsp_count * sizeof(struct omap_mcbsp *),
 								GFP_KERNEL);
diff -upr git.orig/arch/arm/mach-omap2/mcbsp.c git/arch/arm/mach-omap2/mcbsp.c
--- git.orig/arch/arm/mach-omap2/mcbsp.c	2009-12-02 15:48:38.000000000 +0100
+++ git/arch/arm/mach-omap2/mcbsp.c	2009-12-14 21:32:56.000000000 +0100
@@ -65,9 +65,11 @@ static struct omap_mcbsp_platform_data o
 	},
 };
 #define OMAP2420_MCBSP_PDATA_SZ		ARRAY_SIZE(omap2420_mcbsp_pdata)
+#define OMAP2420_MCBSP_REG_NUM		(OMAP_MCBSP_REG_RCCR / sizeof(u32) + 1)
 #else
 #define omap2420_mcbsp_pdata		NULL
 #define OMAP2420_MCBSP_PDATA_SZ		0
+#define OMAP2420_MCBSP_REG_NUM		0
 #endif
 
 #ifdef CONFIG_ARCH_OMAP2430
@@ -114,9 +116,11 @@ static struct omap_mcbsp_platform_data o
 	},
 };
 #define OMAP2430_MCBSP_PDATA_SZ		ARRAY_SIZE(omap2430_mcbsp_pdata)
+#define OMAP2430_MCBSP_REG_NUM		(OMAP_MCBSP_REG_RCCR / sizeof(u32) + 1)
 #else
 #define omap2430_mcbsp_pdata		NULL
 #define OMAP2430_MCBSP_PDATA_SZ		0
+#define OMAP2430_MCBSP_REG_NUM		0
 #endif
 
 #ifdef CONFIG_ARCH_OMAP34XX
@@ -168,9 +172,11 @@ static struct omap_mcbsp_platform_data o
 	},
 };
 #define OMAP34XX_MCBSP_PDATA_SZ		ARRAY_SIZE(omap34xx_mcbsp_pdata)
+#define OMAP34XX_MCBSP_REG_NUM		(OMAP_MCBSP_REG_RCCR / sizeof(u32) + 1)
 #else
 #define omap34xx_mcbsp_pdata		NULL
 #define OMAP34XX_MCBSP_PDATA_SZ		0
+#define OMAP34XX_MCBSP_REG_NUM		0
 #endif
 
 static struct omap_mcbsp_platform_data omap44xx_mcbsp_pdata[] = {
@@ -208,17 +214,23 @@ static struct omap_mcbsp_platform_data o
 	},
 };
 #define OMAP44XX_MCBSP_PDATA_SZ		ARRAY_SIZE(omap44xx_mcbsp_pdata)
+#define OMAP44XX_MCBSP_REG_NUM		(OMAP_MCBSP_REG_RCCR / sizeof(u32) + 1)
 
 static int __init omap2_mcbsp_init(void)
 {
-	if (cpu_is_omap2420())
+	if (cpu_is_omap2420()) {
 		omap_mcbsp_count = OMAP2420_MCBSP_PDATA_SZ;
-	if (cpu_is_omap2430())
+		omap_mcbsp_cache_size = OMAP2420_MCBSP_REG_NUM * sizeof(u16);
+	} else if (cpu_is_omap2430()) {
 		omap_mcbsp_count = OMAP2430_MCBSP_PDATA_SZ;
-	if (cpu_is_omap34xx())
+		omap_mcbsp_cache_size = OMAP2430_MCBSP_REG_NUM * sizeof(u32);
+	} else if (cpu_is_omap34xx()) {
 		omap_mcbsp_count = OMAP34XX_MCBSP_PDATA_SZ;
-	if (cpu_is_omap44xx())
+		omap_mcbsp_cache_size = OMAP34XX_MCBSP_REG_NUM * sizeof(u32);
+	} else if (cpu_is_omap44xx()) {
 		omap_mcbsp_count = OMAP44XX_MCBSP_PDATA_SZ;
+		omap_mcbsp_cache_size = OMAP44XX_MCBSP_REG_NUM * sizeof(u32);
+	}
 
 	mcbsp_ptr = kzalloc(omap_mcbsp_count * sizeof(struct omap_mcbsp *),
 								GFP_KERNEL);
diff -upr git.orig/arch/arm/plat-omap/include/plat/mcbsp.h git/arch/arm/plat-omap/include/plat/mcbsp.h
--- git.orig/arch/arm/plat-omap/include/plat/mcbsp.h	2009-12-02 15:48:51.000000000 +0100
+++ git/arch/arm/plat-omap/include/plat/mcbsp.h	2009-12-14 21:32:56.000000000 +0100
@@ -415,9 +415,10 @@ struct omap_mcbsp {
 	u16 max_tx_thres;
 	u16 max_rx_thres;
 #endif
+	void *reg_cache;
 };
 extern struct omap_mcbsp **mcbsp_ptr;
-extern int omap_mcbsp_count;
+extern int omap_mcbsp_count, omap_mcbsp_cache_size;
 
 int omap_mcbsp_init(void);
 void omap_mcbsp_register_board_cfg(struct omap_mcbsp_platform_data *config,
diff -upr git.orig/arch/arm/plat-omap/mcbsp.c git/arch/arm/plat-omap/mcbsp.c
--- git.orig/arch/arm/plat-omap/mcbsp.c	2009-12-11 18:19:18.000000000 +0100
+++ git/arch/arm/plat-omap/mcbsp.c	2009-12-14 21:45:16.000000000 +0100
@@ -28,28 +28,42 @@
 #include <plat/mcbsp.h>
 
 struct omap_mcbsp **mcbsp_ptr;
-int omap_mcbsp_count;
+int omap_mcbsp_count, omap_mcbsp_cache_size;
 
 void omap_mcbsp_write(struct omap_mcbsp *mcbsp, u16 reg, u32 val)
 {
-	if (cpu_class_is_omap1() || cpu_is_omap2420())
+	if (cpu_class_is_omap1()) {
+		((u16 *)mcbsp->reg_cache)[reg / sizeof(u16)] = (u16)val;
 		__raw_writew((u16)val, mcbsp->io_base + reg);
-	else
+	} else if (cpu_is_omap2420()) {
+		((u16 *)mcbsp->reg_cache)[reg / sizeof(u32)] = (u16)val;
+		__raw_writew((u16)val, mcbsp->io_base + reg);
+	} else {
+		((u32 *)mcbsp->reg_cache)[reg / sizeof(u32)] = val;
 		__raw_writel(val, mcbsp->io_base + reg);
+	}
 }
 
-int omap_mcbsp_read(struct omap_mcbsp *mcbsp, u16 reg)
+int omap_mcbsp_read(struct omap_mcbsp *mcbsp, u16 reg, bool from_cache)
 {
-	if (cpu_class_is_omap1() || cpu_is_omap2420())
-		return __raw_readw(mcbsp->io_base + reg);
-	else
-		return __raw_readl(mcbsp->io_base + reg);
+	if (cpu_class_is_omap1()) {
+		return !from_cache ? __raw_readw(mcbsp->io_base + reg) :
+				((u16 *)mcbsp->reg_cache)[reg / sizeof(u16)];
+	} else if (cpu_is_omap2420()) {
+		return !from_cache ? __raw_readw(mcbsp->io_base + reg) :
+				((u16 *)mcbsp->reg_cache)[reg / sizeof(u32)];
+	} else {
+		return !from_cache ? __raw_readl(mcbsp->io_base + reg) :
+				((u32 *)mcbsp->reg_cache)[reg / sizeof(u32)];
+	}
 }
 
 #define MCBSP_READ(mcbsp, reg) \
-		omap_mcbsp_read(mcbsp, OMAP_MCBSP_REG_##reg)
+		omap_mcbsp_read(mcbsp, OMAP_MCBSP_REG_##reg, 0)
 #define MCBSP_WRITE(mcbsp, reg, val) \
 		omap_mcbsp_write(mcbsp, OMAP_MCBSP_REG_##reg, val)
+#define MCBSP_READ_CACHE(mcbsp, reg) \
+		omap_mcbsp_read(mcbsp, OMAP_MCBSP_REG_##reg, 1)
 
 #define omap_mcbsp_check_valid_id(id)	(id < omap_mcbsp_count)
 #define id_to_mcbsp_ptr(id)		mcbsp_ptr[id];
@@ -402,6 +416,12 @@ int omap_mcbsp_request(unsigned int id)
 	mcbsp->free = 0;
 	spin_unlock(&mcbsp->lock);
 
+	mcbsp->reg_cache = kzalloc(omap_mcbsp_cache_size, GFP_KERNEL);
+	if (!mcbsp->reg_cache) {
+		mcbsp->free = 1;
+		return -ENOMEM;
+	}
+
 	if (mcbsp->pdata && mcbsp->pdata->ops && mcbsp->pdata->ops->request)
 		mcbsp->pdata->ops->request(id);
 
@@ -471,6 +491,9 @@ void omap_mcbsp_free(unsigned int id)
 		free_irq(mcbsp->tx_irq, (void *)mcbsp);
 	}
 
+	kfree(mcbsp->reg_cache);
+	mcbsp->reg_cache = NULL;
+
 	spin_lock(&mcbsp->lock);
 	if (mcbsp->free) {
 		dev_err(mcbsp->dev, "McBSP%d was not reserved\n",
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/5 v8] OMAP: McBSP: Introduce caching in register write operations

[Jarkko Nikula]

On Tue, 15 Dec 2009 01:36:47 +0100
Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> wrote:

> 3. In omap_mcbsp_free(), marking the port as free before deallocating the 
>    cache, could result in memory leak as well.
> 
> I hope all are addressed correctly. Since releasing the port with
> 
> 	mcbsp->free = 1;
> 
> after kzalloc() failure should be atomic, I have assumed this doesn't require 
> locking.
> 
Good catch. One comment below.

> @@ -471,6 +491,9 @@ void omap_mcbsp_free(unsigned int id)
>  		free_irq(mcbsp->tx_irq, (void *)mcbsp);
>  	}
>  
> +	kfree(mcbsp->reg_cache);
> +	mcbsp->reg_cache = NULL;
> +
>  	spin_lock(&mcbsp->lock);
>  	if (mcbsp->free) {
>  		dev_err(mcbsp->dev, "McBSP%d was not reserved\n",

This is fine in current form but to play safe, lets do the kfree
inside the spin_lock section where the mcbsp->free is set. Then there
is no risk if some future code would use the mcbsp->free as a guard for
cache etc. access.


-- 
Jarkko

[PATCH v7 4/5] OMAP: McBSP: Use cache when modifying individual register bits

[Janusz Krzysztofik]

Change the way McBSP registers are updated: use cached values instead of
relying upon those read back from the device.

With this patch, I have finally managed to get rid of all random
playback/recording hangups on my OMAP1510 based Amstrad Delta hardware. Before
that, values read back from McBSP registers to be used for updating them
happened to be errornous.

>From the hardware side, the issue appeared to be caused by a relatively high
power requirements of an external USB adapter connected to the board's printer
dedicated USB port.

I think there is one important point that makes this patch worth of applying,
apart from my hardware quality. With the current code, if it ever happens to
any machine, no matter if OMAP1510 or newer, to read incorrect value from a
McBSP register, this wrong value will get written back without any checking.
That can lead to hardware damage if, for example, an input pin is turned into
output as a result.

Applies on top of patch 3 from this series:
[PATCH v7 3/5] OMAP: McBSP: Introduce caching in register write operations

Tested on OMAP1510 based Amstrad Delta using linux-omap for-next,
commit 82f1d8f22f2c65e70206e40a6f17688bf64a892c.
Compile-tested with omap_3430sdp_defconfig.

Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>

---
No functional changes since v3.

 arch/arm/plat-omap/mcbsp.c |   78 +++++++++++++++++++++++++++------------------
 1 file changed, 47 insertions(+), 31 deletions(-)

diff -upr git.orig/arch/arm/plat-omap/mcbsp.c git/arch/arm/plat-omap/mcbsp.c
--- git.orig/arch/arm/plat-omap/mcbsp.c	2009-12-09 15:49:53.000000000 +0100
+++ git/arch/arm/plat-omap/mcbsp.c	2009-12-09 15:50:05.000000000 +0100
@@ -114,7 +114,8 @@ static irqreturn_t omap_mcbsp_tx_irq_han
 		dev_err(mcbsp_tx->dev, "TX Frame Sync Error! : 0x%x\n",
 			irqst_spcr2);
 		/* Writing zero to XSYNC_ERR clears the IRQ */
-		MCBSP_WRITE(mcbsp_tx, SPCR2, irqst_spcr2 & ~(XSYNC_ERR));
+		MCBSP_WRITE(mcbsp_tx, SPCR2,
+			    MCBSP_READ_CACHE(mcbsp_tx, SPCR2) & ~(XSYNC_ERR));
 	} else {
 		complete(&mcbsp_tx->tx_irq_completion);
 	}
@@ -134,7 +135,8 @@ static irqreturn_t omap_mcbsp_rx_irq_han
 		dev_err(mcbsp_rx->dev, "RX Frame Sync Error! : 0x%x\n",
 			irqst_spcr1);
 		/* Writing zero to RSYNC_ERR clears the IRQ */
-		MCBSP_WRITE(mcbsp_rx, SPCR1, irqst_spcr1 & ~(RSYNC_ERR));
+		MCBSP_WRITE(mcbsp_rx, SPCR1,
+			    MCBSP_READ_CACHE(mcbsp_rx, SPCR1) & ~(RSYNC_ERR));
 	} else {
 		complete(&mcbsp_rx->tx_irq_completion);
 	}
@@ -522,24 +524,25 @@ void omap_mcbsp_start(unsigned int id, i
 	}
 	mcbsp = id_to_mcbsp_ptr(id);
 
-	mcbsp->rx_word_length = (MCBSP_READ(mcbsp, RCR1) >> 5) & 0x7;
-	mcbsp->tx_word_length = (MCBSP_READ(mcbsp, XCR1) >> 5) & 0x7;
+	mcbsp->rx_word_length = (MCBSP_READ_CACHE(mcbsp, RCR1) >> 5) & 0x7;
+	mcbsp->tx_word_length = (MCBSP_READ_CACHE(mcbsp, XCR1) >> 5) & 0x7;
 
-	idle = !((MCBSP_READ(mcbsp, SPCR2) | MCBSP_READ(mcbsp, SPCR1)) & 1);
+	idle = !((MCBSP_READ_CACHE(mcbsp, SPCR2) |
+			MCBSP_READ_CACHE(mcbsp, SPCR1)) & 1);
 
 	if (idle) {
 		/* Start the sample generator */
-		w = MCBSP_READ(mcbsp, SPCR2);
+		w = MCBSP_READ_CACHE(mcbsp, SPCR2);
 		MCBSP_WRITE(mcbsp, SPCR2, w | (1 << 6));
 	}
 
 	/* Enable transmitter and receiver */
 	tx &= 1;
-	w = MCBSP_READ(mcbsp, SPCR2);
+	w = MCBSP_READ_CACHE(mcbsp, SPCR2);
 	MCBSP_WRITE(mcbsp, SPCR2, w | tx);
 
 	rx &= 1;
-	w = MCBSP_READ(mcbsp, SPCR1);
+	w = MCBSP_READ_CACHE(mcbsp, SPCR1);
 	MCBSP_WRITE(mcbsp, SPCR1, w | rx);
 
 	/*
@@ -552,16 +555,16 @@ void omap_mcbsp_start(unsigned int id, i
 
 	if (idle) {
 		/* Start frame sync */
-		w = MCBSP_READ(mcbsp, SPCR2);
+		w = MCBSP_READ_CACHE(mcbsp, SPCR2);
 		MCBSP_WRITE(mcbsp, SPCR2, w | (1 << 7));
 	}
 
 	if (cpu_is_omap2430() || cpu_is_omap34xx()) {
 		/* Release the transmitter and receiver */
-		w = MCBSP_READ(mcbsp, XCCR);
+		w = MCBSP_READ_CACHE(mcbsp, XCCR);
 		w &= ~(tx ? XDISABLE : 0);
 		MCBSP_WRITE(mcbsp, XCCR, w);
-		w = MCBSP_READ(mcbsp, RCCR);
+		w = MCBSP_READ_CACHE(mcbsp, RCCR);
 		w &= ~(rx ? RDISABLE : 0);
 		MCBSP_WRITE(mcbsp, RCCR, w);
 	}
@@ -587,28 +590,29 @@ void omap_mcbsp_stop(unsigned int id, in
 	/* Reset transmitter */
 	tx &= 1;
 	if (cpu_is_omap2430() || cpu_is_omap34xx()) {
-		w = MCBSP_READ(mcbsp, XCCR);
+		w = MCBSP_READ_CACHE(mcbsp, XCCR);
 		w |= (tx ? XDISABLE : 0);
 		MCBSP_WRITE(mcbsp, XCCR, w);
 	}
-	w = MCBSP_READ(mcbsp, SPCR2);
+	w = MCBSP_READ_CACHE(mcbsp, SPCR2);
 	MCBSP_WRITE(mcbsp, SPCR2, w & ~tx);
 
 	/* Reset receiver */
 	rx &= 1;
 	if (cpu_is_omap2430() || cpu_is_omap34xx()) {
-		w = MCBSP_READ(mcbsp, RCCR);
+		w = MCBSP_READ_CACHE(mcbsp, RCCR);
 		w |= (rx ? RDISABLE : 0);
 		MCBSP_WRITE(mcbsp, RCCR, w);
 	}
-	w = MCBSP_READ(mcbsp, SPCR1);
+	w = MCBSP_READ_CACHE(mcbsp, SPCR1);
 	MCBSP_WRITE(mcbsp, SPCR1, w & ~rx);
 
-	idle = !((MCBSP_READ(mcbsp, SPCR2) | MCBSP_READ(mcbsp, SPCR1)) & 1);
+	idle = !((MCBSP_READ_CACHE(mcbsp, SPCR2) |
+			MCBSP_READ_CACHE(mcbsp, SPCR1)) & 1);
 
 	if (idle) {
 		/* Reset the sample rate generator */
-		w = MCBSP_READ(mcbsp, SPCR2);
+		w = MCBSP_READ_CACHE(mcbsp, SPCR2);
 		MCBSP_WRITE(mcbsp, SPCR2, w & ~(1 << 6));
 	}
 }
@@ -631,7 +635,7 @@ int omap_mcbsp_pollwrite(unsigned int id
 	if (MCBSP_READ(mcbsp, SPCR2) & XSYNC_ERR) {
 		/* clear error */
 		MCBSP_WRITE(mcbsp, SPCR2,
-				MCBSP_READ(mcbsp, SPCR2) & (~XSYNC_ERR));
+				MCBSP_READ_CACHE(mcbsp, SPCR2) & (~XSYNC_ERR));
 		/* resend */
 		return -1;
 	} else {
@@ -640,10 +644,12 @@ int omap_mcbsp_pollwrite(unsigned int id
 		while (!(MCBSP_READ(mcbsp, SPCR2) & XRDY)) {
 			if (attemps++ > 1000) {
 				MCBSP_WRITE(mcbsp, SPCR2,
-					    MCBSP_READ(mcbsp, SPCR2) & (~XRST));
+						MCBSP_READ_CACHE(mcbsp, SPCR2) &
+						(~XRST));
 				udelay(10);
 				MCBSP_WRITE(mcbsp, SPCR2,
-					    MCBSP_READ(mcbsp, SPCR2) | (XRST));
+						MCBSP_READ_CACHE(mcbsp, SPCR2) |
+						(XRST));
 				udelay(10);
 				dev_err(mcbsp->dev, "Could not write to"
 					" McBSP%d Register\n", mcbsp->id);
@@ -670,7 +676,7 @@ int omap_mcbsp_pollread(unsigned int id,
 	if (MCBSP_READ(mcbsp, SPCR1) & RSYNC_ERR) {
 		/* clear error */
 		MCBSP_WRITE(mcbsp, SPCR1,
-				MCBSP_READ(mcbsp, SPCR1) & (~RSYNC_ERR));
+				MCBSP_READ_CACHE(mcbsp, SPCR1) & (~RSYNC_ERR));
 		/* resend */
 		return -1;
 	} else {
@@ -679,10 +685,12 @@ int omap_mcbsp_pollread(unsigned int id,
 		while (!(MCBSP_READ(mcbsp, SPCR1) & RRDY)) {
 			if (attemps++ > 1000) {
 				MCBSP_WRITE(mcbsp, SPCR1,
-					    MCBSP_READ(mcbsp, SPCR1) & (~RRST));
+						MCBSP_READ_CACHE(mcbsp, SPCR1) &
+						(~RRST));
 				udelay(10);
 				MCBSP_WRITE(mcbsp, SPCR1,
-					    MCBSP_READ(mcbsp, SPCR1) | (RRST));
+						MCBSP_READ_CACHE(mcbsp, SPCR1) |
+						(RRST));
 				udelay(10);
 				dev_err(mcbsp->dev, "Could not read from"
 					" McBSP%d Register\n", mcbsp->id);
@@ -768,9 +776,11 @@ int omap_mcbsp_spi_master_xmit_word_poll
 		spcr2 = MCBSP_READ(mcbsp, SPCR2);
 		if (attempts++ > 1000) {
 			/* We must reset the transmitter */
-			MCBSP_WRITE(mcbsp, SPCR2, spcr2 & (~XRST));
+			MCBSP_WRITE(mcbsp, SPCR2,
+				    MCBSP_READ_CACHE(mcbsp, SPCR2) & (~XRST));
 			udelay(10);
-			MCBSP_WRITE(mcbsp, SPCR2, spcr2 | XRST);
+			MCBSP_WRITE(mcbsp, SPCR2,
+				    MCBSP_READ_CACHE(mcbsp, SPCR2) | XRST);
 			udelay(10);
 			dev_err(mcbsp->dev, "McBSP%d transmitter not "
 				"ready\n", mcbsp->id);
@@ -789,9 +799,11 @@ int omap_mcbsp_spi_master_xmit_word_poll
 		spcr1 = MCBSP_READ(mcbsp, SPCR1);
 		if (attempts++ > 1000) {
 			/* We must reset the receiver */
-			MCBSP_WRITE(mcbsp, SPCR1, spcr1 & (~RRST));
+			MCBSP_WRITE(mcbsp, SPCR1,
+				    MCBSP_READ_CACHE(mcbsp, SPCR1) & (~RRST));
 			udelay(10);
-			MCBSP_WRITE(mcbsp, SPCR1, spcr1 | RRST);
+			MCBSP_WRITE(mcbsp, SPCR1,
+				    MCBSP_READ_CACHE(mcbsp, SPCR1) | RRST);
 			udelay(10);
 			dev_err(mcbsp->dev, "McBSP%d receiver not "
 				"ready\n", mcbsp->id);
@@ -835,9 +847,11 @@ int omap_mcbsp_spi_master_recv_word_poll
 		spcr2 = MCBSP_READ(mcbsp, SPCR2);
 		if (attempts++ > 1000) {
 			/* We must reset the transmitter */
-			MCBSP_WRITE(mcbsp, SPCR2, spcr2 & (~XRST));
+			MCBSP_WRITE(mcbsp, SPCR2,
+				    MCBSP_READ_CACHE(mcbsp, SPCR2) & (~XRST));
 			udelay(10);
-			MCBSP_WRITE(mcbsp, SPCR2, spcr2 | XRST);
+			MCBSP_WRITE(mcbsp, SPCR2,
+				    MCBSP_READ_CACHE(mcbsp, SPCR2) | XRST);
 			udelay(10);
 			dev_err(mcbsp->dev, "McBSP%d transmitter not "
 				"ready\n", mcbsp->id);
@@ -856,9 +870,11 @@ int omap_mcbsp_spi_master_recv_word_poll
 		spcr1 = MCBSP_READ(mcbsp, SPCR1);
 		if (attempts++ > 1000) {
 			/* We must reset the receiver */
-			MCBSP_WRITE(mcbsp, SPCR1, spcr1 & (~RRST));
+			MCBSP_WRITE(mcbsp, SPCR1,
+				    MCBSP_READ_CACHE(mcbsp, SPCR1) & (~RRST));
 			udelay(10);
-			MCBSP_WRITE(mcbsp, SPCR1, spcr1 | RRST);
+			MCBSP_WRITE(mcbsp, SPCR1,
+				    MCBSP_READ_CACHE(mcbsp, SPCR1) | RRST);
 			udelay(10);
 			dev_err(mcbsp->dev, "McBSP%d receiver not "
 				"ready\n", mcbsp->id);

[PATCH v7 5/5] OMAP: McBSP: Split and move read/write functions to mach-omap1/2

[Janusz Krzysztofik]

Split omap_mcbsp_read()/_write() functions logic into omap1 and omap2/3/4
parts, then move them out of plat-omap/mcbsp.c into mach-omap1/mcbsp.c and
mach-omap2/mcbsp.c respectively, to leave some of the "if cpu_is_omapxxxx()
else" stuff.

Applies on top of patch 4 from this series:
[PATCH v7 4/5] OMAP: McBSP: Use cache when modifying individual register bits

Tested on OMAP1510 based Amstrad Delta using linux-omap for-next,
commit 82f1d8f22f2c65e70206e40a6f17688bf64a892c.
Compile-tested with omap_generic_2420_defconfig and omap_3430sdp_defconfig.

Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>

---
Wednesday 09 December 2009 00:39:16 Tony Lindgren napisał(a):
> * Tony Lindgren <tony@atomide.com> [091208 15:32]:
> > * Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> [091208 11:45]:
> > > Tuesday 08 December 2009 17:59:31 Tony Lindgren napisał(a):
> > > >
> > > > Actually since we already have mach-omap1/mcbsp.c and
> > > > mach-omap2/mcbsp.c, it would be best to pass the cache size from
> > > > omap1_mcbsp_init and omap2_mcbsp_init. That leaves some of the if
> > > > cpu_is_omapxxxx() else stuff.
> > >
> > > Tony,
> > > Almost ready with it, one more question: what do you think about
> > > splitting and moving omap_mcbsp_read()/_write() there as well? If you
> > > agree, should I submit 2 patches, one with this cleanup, the other one
> > > actually introducing cache support, or is one combined OK?
> >
> > Sounds good to me!
>
> Oh sorry forgot to reply to your question. If a single patch looks
> unreadable, then split it into two where the first patch splits
> omap_mcbsp_read/write.

Tony,
Since this one is new, in order to not block the 4 preceding patches that do
not really need this one, I decided to create this additional cleanup as the
last one in the series, to be dropped easily if not accepted for any problems
with it.

Thanks,
Janusz

 arch/arm/mach-omap1/mcbsp.c             |   12 ++++++++++++
 arch/arm/mach-omap2/mcbsp.c             |   22 ++++++++++++++++++++++
 arch/arm/plat-omap/include/plat/mcbsp.h |    3 +++
 arch/arm/plat-omap/mcbsp.c              |   28 ----------------------------
 4 files changed, 37 insertions(+), 28 deletions(-)

diff -upr git.orig/arch/arm/mach-omap1/mcbsp.c git/arch/arm/mach-omap1/mcbsp.c
--- git.orig/arch/arm/mach-omap1/mcbsp.c	2009-12-09 15:49:52.000000000 +0100
+++ git/arch/arm/mach-omap1/mcbsp.c	2009-12-09 16:20:43.000000000 +0100
@@ -31,6 +31,18 @@ static int dsp_use;
 static struct clk *api_clk;
 static struct clk *dsp_clk;
 
+void omap_mcbsp_write(struct omap_mcbsp *mcbsp, u16 reg, u32 val)
+{
+	((u16 *)mcbsp->reg_cache)[reg / sizeof(u16)] = (u16)val;
+	__raw_writew((u16)val, mcbsp->io_base + reg);
+}
+
+int omap_mcbsp_read(struct omap_mcbsp *mcbsp, u16 reg, bool from_cache)
+{
+	return !from_cache ? __raw_readw(mcbsp->io_base + reg) :
+			((u16 *)mcbsp->reg_cache)[reg / sizeof(u16)];
+}
+
 static void omap1_mcbsp_request(unsigned int id)
 {
 	/*
diff -upr git.orig/arch/arm/mach-omap2/mcbsp.c git/arch/arm/mach-omap2/mcbsp.c
--- git.orig/arch/arm/mach-omap2/mcbsp.c	2009-12-09 15:49:52.000000000 +0100
+++ git/arch/arm/mach-omap2/mcbsp.c	2009-12-09 16:20:43.000000000 +0100
@@ -23,6 +23,28 @@
 #include <plat/cpu.h>
 #include <plat/mcbsp.h>
 
+void omap_mcbsp_write(struct omap_mcbsp *mcbsp, u16 reg, u32 val)
+{
+	if (cpu_is_omap2420()) {
+		((u16 *)mcbsp->reg_cache)[reg / sizeof(u32)] = (u16)val;
+		__raw_writew((u16)val, mcbsp->io_base + reg);
+	} else {
+		((u32 *)mcbsp->reg_cache)[reg / sizeof(u32)] = val;
+		__raw_writel(val, mcbsp->io_base + reg);
+	}
+}
+
+int omap_mcbsp_read(struct omap_mcbsp *mcbsp, u16 reg, bool from_cache)
+{
+	if (cpu_is_omap2420()) {
+		return !from_cache ? __raw_readw(mcbsp->io_base + reg) :
+				((u16 *)mcbsp->reg_cache)[reg / sizeof(u32)];
+	} else {
+		return !from_cache ? __raw_readl(mcbsp->io_base + reg) :
+				((u32 *)mcbsp->reg_cache)[reg / sizeof(u32)];
+	}
+}
+
 static void omap2_mcbsp2_mux_setup(void)
 {
 	omap_cfg_reg(Y15_24XX_MCBSP2_CLKX);
diff -upr git.orig/arch/arm/plat-omap/include/plat/mcbsp.h git/arch/arm/plat-omap/include/plat/mcbsp.h
--- git.orig/arch/arm/plat-omap/include/plat/mcbsp.h	2009-12-09 15:49:53.000000000 +0100
+++ git/arch/arm/plat-omap/include/plat/mcbsp.h	2009-12-09 16:20:43.000000000 +0100
@@ -420,6 +420,9 @@ struct omap_mcbsp {
 extern struct omap_mcbsp **mcbsp_ptr;
 extern int omap_mcbsp_count, omap_mcbsp_cache_size;
 
+void omap_mcbsp_write(struct omap_mcbsp *mcbsp, u16 reg, u32 val);
+int omap_mcbsp_read(struct omap_mcbsp *mcbsp, u16 reg, bool from_cache);
+
 int omap_mcbsp_init(void);
 void omap_mcbsp_register_board_cfg(struct omap_mcbsp_platform_data *config,
 					int size);
diff -upr git.orig/arch/arm/plat-omap/mcbsp.c git/arch/arm/plat-omap/mcbsp.c
--- git.orig/arch/arm/plat-omap/mcbsp.c	2009-12-09 16:20:29.000000000 +0100
+++ git/arch/arm/plat-omap/mcbsp.c	2009-12-09 16:20:43.000000000 +0100
@@ -30,34 +30,6 @@
 struct omap_mcbsp **mcbsp_ptr;
 int omap_mcbsp_count, omap_mcbsp_cache_size;
 
-void omap_mcbsp_write(struct omap_mcbsp *mcbsp, u16 reg, u32 val)
-{
-	if (cpu_class_is_omap1()) {
-		((u16 *)mcbsp->reg_cache)[reg / sizeof(u16)] = (u16)val;
-		__raw_writew((u16)val, mcbsp->io_base + reg);
-	} else if (cpu_is_omap2420()) {
-		((u16 *)mcbsp->reg_cache)[reg / sizeof(u32)] = (u16)val;
-		__raw_writew((u16)val, mcbsp->io_base + reg);
-	} else {
-		((u32 *)mcbsp->reg_cache)[reg / sizeof(u32)] = val;
-		__raw_writel(val, mcbsp->io_base + reg);
-	}
-}
-
-int omap_mcbsp_read(struct omap_mcbsp *mcbsp, u16 reg, bool from_cache)
-{
-	if (cpu_class_is_omap1()) {
-		return !from_cache ? __raw_readw(mcbsp->io_base + reg) :
-				((u16 *)mcbsp->reg_cache)[reg / sizeof(u16)];
-	} else if (cpu_is_omap2420()) {
-		return !from_cache ? __raw_readw(mcbsp->io_base + reg) :
-				((u16 *)mcbsp->reg_cache)[reg / sizeof(u32)];
-	} else {
-		return !from_cache ? __raw_readl(mcbsp->io_base + reg) :
-				((u32 *)mcbsp->reg_cache)[reg / sizeof(u32)];
-	}
-}
-
 #define MCBSP_READ(mcbsp, reg) \
 		omap_mcbsp_read(mcbsp, OMAP_MCBSP_REG_##reg, 0)
 #define MCBSP_WRITE(mcbsp, reg, val) \
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7 5/5] OMAP: McBSP: Split and move read/write functions to mach-omap1/2

[Jarkko Nikula]

On Wed, 9 Dec 2009 21:34:30 +0100
Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> wrote:

> > > > Almost ready with it, one more question: what do you think about
> > > > splitting and moving omap_mcbsp_read()/_write() there as well? If you
> > > > agree, should I submit 2 patches, one with this cleanup, the other one
> > > > actually introducing cache support, or is one combined OK?
> > >
> > > Sounds good to me!
> >
...
> diff -upr git.orig/arch/arm/mach-omap1/mcbsp.c git/arch/arm/mach-omap1/mcbsp.c
> --- git.orig/arch/arm/mach-omap1/mcbsp.c	2009-12-09 15:49:52.000000000 +0100
> +++ git/arch/arm/mach-omap1/mcbsp.c	2009-12-09 16:20:43.000000000 +0100
>  
> +void omap_mcbsp_write(struct omap_mcbsp *mcbsp, u16 reg, u32 val)
...
> diff -upr git.orig/arch/arm/mach-omap2/mcbsp.c git/arch/arm/mach-omap2/mcbsp.c
> --- git.orig/arch/arm/mach-omap2/mcbsp.c	2009-12-09 15:49:52.000000000 +0100
> +++ git/arch/arm/mach-omap2/mcbsp.c	2009-12-09 16:20:43.000000000 +0100
>  
> +void omap_mcbsp_write(struct omap_mcbsp *mcbsp, u16 reg, u32 val)

These functions must be unique, otherwise multi-build is not possible
(no idea can we do it for OMAP1?).

IMO, the _write and _read functions in ./plat-omap/mcbsp.c are clean
after the patch 3/5 anyway so probably we don't need this splitting?

-- 
Jarkko

Re: [PATCH v7 5/5] OMAP: McBSP: Split and move read/write functions to mach-omap1/2

[Janusz Krzysztofik]

Friday 11 December 2009 14:21:16 Jarkko Nikula napisał(a):
> On Wed, 9 Dec 2009 21:34:30 +0100
>
> Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> wrote:
> > > > > Almost ready with it, one more question: what do you think about
> > > > > splitting and moving omap_mcbsp_read()/_write() there as well? If
> > > > > you agree, should I submit 2 patches, one with this cleanup, the
> > > > > other one actually introducing cache support, or is one combined
> > > > > OK?
> > > >
> > > > Sounds good to me!
>
> ...
>
> > diff -upr git.orig/arch/arm/mach-omap1/mcbsp.c
> > git/arch/arm/mach-omap1/mcbsp.c ---
> > git.orig/arch/arm/mach-omap1/mcbsp.c	2009-12-09 15:49:52.000000000 +0100
> > +++ git/arch/arm/mach-omap1/mcbsp.c	2009-12-09 16:20:43.000000000 +0100
> >
> > +void omap_mcbsp_write(struct omap_mcbsp *mcbsp, u16 reg, u32 val)
>
> ...
>
> > diff -upr git.orig/arch/arm/mach-omap2/mcbsp.c
> > git/arch/arm/mach-omap2/mcbsp.c ---
> > git.orig/arch/arm/mach-omap2/mcbsp.c	2009-12-09 15:49:52.000000000 +0100
> > +++ git/arch/arm/mach-omap2/mcbsp.c	2009-12-09 16:20:43.000000000 +0100
> >
> > +void omap_mcbsp_write(struct omap_mcbsp *mcbsp, u16 reg, u32 val)
>
> These functions must be unique, otherwise multi-build is not possible
> (no idea can we do it for OMAP1?).

Function name duplication was my concern to, but since I did the same before 
in v5b of 3/4 and noone objected that particular piece of code, I took into 
consideration that it could be acceptable in case of OMAP1 vs. OMAP2/3/4.

> IMO, the _write and _read functions in ./plat-omap/mcbsp.c are clean
> after the patch 3/5 anyway so probably we don't need this splitting?

I agree (just tried to endear myself to Tony ;) ).

Janusz
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7 0/5] OMAP: McBSP: Use register cache

[Peter Ujfalusi]

Hello Janusz,

I'll try to go through the series.

On Wednesday 09 December 2009 22:24:13 ext Janusz Krzysztofik wrote:

> This could help for developing the McBSP context save/restore features, as
> well as solve the problem of possible register corruption, experienced on
> OMAP1510 based Amstrad Delta board at least.

For the context save/restore feature:
On OMAP3 this is not that straight forward IMHO.
OMAP3 has FIFO on the McBSP ports (1024+256, or 128 word long).
This buffer will be invalidated in case of OFF mode (or when context 
save/restore is needed).
So if we ever face with this situation, we need to make sure that the McBSP FIFO 
is empty prior to context save, so when we are restoring we are not going to 
loose any data.
Note also, that the McBSP FIFO can not be bypassed on OMAP3, which means that 
the FIFO is most of the time is kind of full.
Well, full in element mode, and mostly full in threshold mode.

In other words: on OMAP3 the context save/restore is not that simple as it is on 
OMAP1/2. We need to make sure that the FIFO is empty at the right moment, which 
can not be done with the current code, and to be honest, I can not see how it 
can be done, unless we branch out the OMAP3 related McBSP/DMA code, and leave 
the current implementation to support only OMAP1/2 class devices.

-- 
Péter
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html