[PATCH v9 0/4] OMAP: McBSP: Use register cache

[PATCH v9 0/4] 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
fb7380d70e041e4b3892f6b19dff7efb609d15a4 (2.6.33-rc3+ dated 2010-01-11).
All patches tested on OMAP1510 based Amstrad Delta and compile-tested using
omap_3430sdp_defconfig at least.

Latest changes:
- patch 3: allow for using the mcbsp->free as a guard for cache access,
	   adpot for fixed error handling in omap_mcbsp_request().
- patch 5: dropped
- all: refresh against latest l-o for-next tree.

Janusz Krzysztofik (4):
	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

 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              |  480 
++++++++++++++++----------------
 4 files changed, 278 insertions(+), 241 deletions(-)

Thanks,
Janusz

[PATCH v9 1/4] 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 
fb7380d70e041e4b3892f6b19dff7efb609d15a4 (2.6.33-rc3+ dated 2010-01-11).

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.

Charu,
After a week has passed with no answer from you to my proposal, I'm no longer 
waiting for you to submit your version, sorry.

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

--- git/arch/arm/plat-omap/mcbsp.c.orig	2010-01-13 12:56:42.000000000 +0100
+++ git/arch/arm/plat-omap/mcbsp.c	2010-01-13 23:43:13.000000000 +0100
@@ -636,26 +636,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);
@@ -681,24 +681,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);
@@ -706,7 +706,7 @@ int omap_mcbsp_pollread(unsigned int id,
 			}
 		}
 	}
-	*buf = readw(base + OMAP_MCBSP_REG_DRR1);
+	*buf = OMAP_MCBSP_READ(base, DRR1);
 
 	return 0;
 }

[PATCH v9 2/4] 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 v9 1/4] OMAP: McBSP: Use macros for all register read/write operations

Tested on OMAP1510 based Amstrad Delta using linux-omap for-next, commit 
fb7380d70e041e4b3892f6b19dff7efb609d15a4 (2.6.33-rc3+ dated 2010-01-11).
Compile-tested with omap_3430sdp_defconfig.

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

---
No functional changes since v7.
 
v7 changes:
- 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(-)

--- git/arch/arm/plat-omap/mcbsp.c.orig	2010-01-13 23:43:13.000000000 +0100
+++ git/arch/arm/plat-omap/mcbsp.c	2010-01-13 23:51:13.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 */
@@ -515,7 +506,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;
 
@@ -524,28 +514,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
@@ -557,18 +545,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 */
@@ -579,7 +567,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;
 
@@ -589,35 +576,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);
@@ -626,7 +611,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);
@@ -634,28 +618,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);
@@ -671,7 +652,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);
@@ -679,26 +659,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);
@@ -706,7 +683,7 @@ int omap_mcbsp_pollread(unsigned int id,
 			}
 		}
 	}
-	*buf = OMAP_MCBSP_READ(base, DRR1);
+	*buf = MCBSP_READ(mcbsp, DRR1);
 
 	return 0;
 }
@@ -718,7 +695,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)) {
@@ -727,21 +703,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;
 
@@ -752,13 +726,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));
 }
@@ -767,7 +740,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;
@@ -777,7 +749,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;
 
@@ -785,14 +756,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);
@@ -802,18 +773,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);
@@ -823,8 +794,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;
 }
@@ -834,7 +805,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;
@@ -845,7 +815,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;
@@ -854,14 +823,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);
@@ -871,18 +840,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);
@@ -892,8 +861,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 v9 3/4] 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 v9 2/4] OMAP: McBSP: Modify macros/functions API for easy cache access

Tested on OMAP1510 based Amstrad Delta using linux-omap for-next, commit 
fb7380d70e041e4b3892f6b19dff7efb609d15a4 (2.6.33-rc3+ dated 2010-01-11).
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>

---
v9 changes:
- allow for using the mcbsp->free as a guard for cache access,
- adpot for fixed error handling in omap_mcbsp_request(),
- fix style issues introduced with that error handling fix.

v8 changes:
- move the cache allocation after the mcbsp->free test lines, memory leak 
  and other badness would happen otherwise.

v7 changes:
- 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              |   77 ++++++++++++++++++++++----------
 4 files changed, 84 insertions(+), 32 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	2010-01-13 12:56:29.000000000 +0100
+++ git/arch/arm/mach-omap1/mcbsp.c	2010-01-13 23:55:55.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	2010-01-13 12:56:30.000000000 +0100
+++ git/arch/arm/mach-omap2/mcbsp.c	2010-01-13 23:55:55.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	2010-01-13 12:56:41.000000000 +0100
+++ git/arch/arm/plat-omap/include/plat/mcbsp.h	2010-01-13 23:55:55.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	2010-01-14 01:25:21.000000000 +0100
+++ git/arch/arm/plat-omap/mcbsp.c	2010-01-14 00:36: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];
@@ -383,6 +397,7 @@ EXPORT_SYMBOL(omap_mcbsp_set_io_type);
 int omap_mcbsp_request(unsigned int id)
 {
 	struct omap_mcbsp *mcbsp;
+	void *reg_cache;
 	int err;
 
 	if (!omap_mcbsp_check_valid_id(id)) {
@@ -391,15 +406,21 @@ int omap_mcbsp_request(unsigned int id)
 	}
 	mcbsp = id_to_mcbsp_ptr(id);
 
+	reg_cache = kzalloc(omap_mcbsp_cache_size, GFP_KERNEL);
+	if (!reg_cache) {
+		return -ENOMEM;
+	}
+
 	spin_lock(&mcbsp->lock);
 	if (!mcbsp->free) {
 		dev_err(mcbsp->dev, "McBSP%d is currently in use\n",
 			mcbsp->id);
-		spin_unlock(&mcbsp->lock);
-		return -EBUSY;
+		err = -EBUSY;
+		goto err_kfree;
 	}
 
 	mcbsp->free = 0;
+	mcbsp->reg_cache = reg_cache;
 	spin_unlock(&mcbsp->lock);
 
 	if (mcbsp->pdata && mcbsp->pdata->ops && mcbsp->pdata->ops->request)
@@ -427,7 +448,7 @@ int omap_mcbsp_request(unsigned int id)
 			dev_err(mcbsp->dev, "Unable to request TX IRQ %d "
 					"for McBSP%d\n", mcbsp->tx_irq,
 					mcbsp->id);
-			goto error;
+			goto err_clk_disable;
 		}
 
 		init_completion(&mcbsp->rx_irq_completion);
@@ -437,16 +458,16 @@ int omap_mcbsp_request(unsigned int id)
 			dev_err(mcbsp->dev, "Unable to request RX IRQ %d "
 					"for McBSP%d\n", mcbsp->rx_irq,
 					mcbsp->id);
-			goto tx_irq;
+			goto err_free_irq;
 		}
 	}
 
 	return 0;
-tx_irq:
+err_free_irq:
 	free_irq(mcbsp->tx_irq, (void *)mcbsp);
-error:
+err_clk_disable:
 	if (mcbsp->pdata && mcbsp->pdata->ops && mcbsp->pdata->ops->free)
-			mcbsp->pdata->ops->free(id);
+		mcbsp->pdata->ops->free(id);
 
 	/* Do procedure specific to omap34xx arch, if applicable */
 	omap34xx_mcbsp_free(mcbsp);
@@ -454,7 +475,12 @@ error:
 	clk_disable(mcbsp->fclk);
 	clk_disable(mcbsp->iclk);
 
+	spin_lock(&mcbsp->lock);
 	mcbsp->free = 1;
+	mcbsp->reg_cache = NULL;
+err_kfree:
+	spin_unlock(&mcbsp->lock);
+	kfree(reg_cache);
 
 	return err;
 }
@@ -463,6 +489,7 @@ EXPORT_SYMBOL(omap_mcbsp_request);
 void omap_mcbsp_free(unsigned int id)
 {
 	struct omap_mcbsp *mcbsp;
+	void *reg_cache;
 
 	if (!omap_mcbsp_check_valid_id(id)) {
 		printk(KERN_ERR "%s: Invalid id (%d)\n", __func__, id + 1);
@@ -485,16 +512,18 @@ void omap_mcbsp_free(unsigned int id)
 		free_irq(mcbsp->tx_irq, (void *)mcbsp);
 	}
 
-	spin_lock(&mcbsp->lock);
-	if (mcbsp->free) {
-		dev_err(mcbsp->dev, "McBSP%d was not reserved\n",
-			mcbsp->id);
-		spin_unlock(&mcbsp->lock);
-		return;
-	}
+	reg_cache = mcbsp->reg_cache;
 
-	mcbsp->free = 1;
+	spin_lock(&mcbsp->lock);
+	if (mcbsp->free)
+		dev_err(mcbsp->dev, "McBSP%d was not reserved\n", mcbsp->id);
+	else
+		mcbsp->free = 1;
+	mcbsp->reg_cache = NULL;
 	spin_unlock(&mcbsp->lock);
+
+	if (reg_cache)
+		kfree(reg_cache);
 }
 EXPORT_SYMBOL(omap_mcbsp_free);
 

[PATCH v9 4/4] 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 v9 3/4] OMAP: McBSP: Introduce caching in register write operations

Tested on OMAP1510 based Amstrad Delta using linux-omap for-next, commit 
fb7380d70e041e4b3892f6b19dff7efb609d15a4 (2.6.33-rc3+ dated 2010-01-11).
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(-)

--- git/arch/arm/plat-omap/mcbsp.c.orig	2010-01-14 00:36:14.000000000 +0100
+++ git/arch/arm/plat-omap/mcbsp.c	2010-01-14 02:05:23.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);
 	}
@@ -544,24 +546,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);
 
 	/*
@@ -574,16 +577,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);
 	}
@@ -609,28 +612,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));
 	}
 }
@@ -653,7 +657,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 {
@@ -662,10 +666,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);
@@ -692,7 +698,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 {
@@ -701,10 +707,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);
@@ -790,9 +798,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);
@@ -811,9 +821,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);
@@ -857,9 +869,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);
@@ -878,9 +892,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);

Re: [PATCH v9 0/4] OMAP: McBSP: Use register cache

[Jarkko Nikula]

On Thu, 14 Jan 2010 16:58:29 +0100
Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> wrote:

> 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.
> 
Looks good. I tested the set by running following loop for some time on
BeagleBoard without any problems:

while [ 1 ]; do (arecord -f dat -d 3 |aplay); sleep 7; done

Acked-by: Jarkko Nikula <jhnikula@gmail.com>

Re: [PATCH v9 4/4] OMAP: McBSP: Use cache when modifying individual register bits

[Peter Ujfalusi]

Hello,

I think there are some inconsistency in a way how for example the SPCR1 and 
SPCR2 registers are used.

On Thursday 14 January 2010 18:13:36 ext Janusz Krzysztofik wrote:
> 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 v9 3/4] OMAP: McBSP: Introduce caching in register write operations
> 
> Tested on OMAP1510 based Amstrad Delta using linux-omap for-next, commit
> fb7380d70e041e4b3892f6b19dff7efb609d15a4 (2.6.33-rc3+ dated 2010-01-11).
> 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(-)
> 
> --- git/arch/arm/plat-omap/mcbsp.c.orig	2010-01-14 00:36:14.000000000 +0100
> +++ git/arch/arm/plat-omap/mcbsp.c	2010-01-14 02:05:23.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));

The reg_cache will never have the XSYNC_ERR, or any other flags set, since these 
flags has never written to the reg_cache.
So it is kind of not necessary to clear the flag, which is actually always 0.

Another thing is that as far as I understand the reason behind of this series is 
that you have a problem, that you can not trust on the values that you read from 
the McBSP registers, if this is true, than the problem can occur when the above 
path has been taken:

...
	irqst_spcr2 = MCBSP_READ(mcbsp_tx, SPCR2);
...
	if (irqst_spcr2 & XSYNC_ERR) {

But since you read from McBSP registers much rarely, than probably the 
corruption is not that visible?

Anyway, clearing the status/error flags are not necessary, since the reg_cache 
will never got these bits set, you could just write back the SPCR2/SPCR1 
register content from the cache as it is...


>  	} 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));

Same here.

...

> @@ -653,7 +657,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));

Well, I think here also, the reg_cache does not have the XSYNC_ERR set, it is 
only set in the McBSP register.

>  		/* resend */
>  		return -1;
>  	} else {
> @@ -662,10 +666,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));

Also here, the XRST will surely not set in the cached SPCR2...

This applies fro all other cases regarding to status/error bits in McBSP.

>  				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);
> @@ -692,7 +698,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 {
> @@ -701,10 +707,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);
> @@ -790,9 +798,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);
> @@ -811,9 +821,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);
> @@ -857,9 +869,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);
> @@ -878,9 +892,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);
> 

Re: [PATCH v9 4/4] OMAP: McBSP: Use cache when modifying individual register bits

[Janusz Krzysztofik]

Peter Ujfalusi wrote:
> Hello,

Hi Peter,

> I think there are some inconsistency in a way how for example the SPCR1 and 
> SPCR2 registers are used.
> 
> On Thursday 14 January 2010 18:13:36 ext Janusz Krzysztofik wrote:
>> 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 v9 3/4] OMAP: McBSP: Introduce caching in register write operations
>>
>> Tested on OMAP1510 based Amstrad Delta using linux-omap for-next, commit
>> fb7380d70e041e4b3892f6b19dff7efb609d15a4 (2.6.33-rc3+ dated 2010-01-11).
>> 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(-)
>>
>> --- git/arch/arm/plat-omap/mcbsp.c.orig	2010-01-14 00:36:14.000000000 +0100
>> +++ git/arch/arm/plat-omap/mcbsp.c	2010-01-14 02:05:23.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));
> 
> The reg_cache will never have the XSYNC_ERR, or any other flags set, since these 
> flags has never written to the reg_cache.
> So it is kind of not necessary to clear the flag, which is actually always 0.

Agree.

> Another thing is that as far as I understand the reason behind of this series is 
> that you have a problem, that you can not trust on the values that you read from 
> the McBSP registers, if this is true, than the problem can occur when the above 
> path has been taken:
> 
> ...
> 	irqst_spcr2 = MCBSP_READ(mcbsp_tx, SPCR2);
> ...
> 	if (irqst_spcr2 & XSYNC_ERR) {
> 
> But since you read from McBSP registers much rarely, than probably the 
> corruption is not that visible?

Sure no software solution can correct my hardware issue in case of
register bits manintained by the hardware itself. Well, maybe software
that limits heat dissipation by lowering overal power consumption could
do to some extent ;).

What I'm going to address here is only a case when writing back possibly
corrupted bits can be avoided if correct values are well known and
can be determined without reading them back from the register itself.

> Anyway, clearing the status/error flags are not necessary, since the reg_cache 
> will never got these bits set, you could just write back the SPCR2/SPCR1 
> register content from the cache as it is...
> 
> 
>>  	} 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));
> 
> Same here.
> 
> ...
> 
>> @@ -653,7 +657,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));
> 
> Well, I think here also, the reg_cache does not have the XSYNC_ERR set, it is 
> only set in the McBSP register.
> 
>>  		/* resend */
>>  		return -1;
>>  	} else {
>> @@ -662,10 +666,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));
> 
> Also here, the XRST will surely not set in the cached SPCR2...
> 
> This applies fro all other cases regarding to status/error bits in McBSP.

OK, I can try to identify all those cases.

What about introducing this simplification in a separate followup patch,
quoting your rationale in its changelog? I can try to prepare one if you
agree.

Thanks,
Janusz

Re: [PATCH v9 4/4] OMAP: McBSP: Use cache when modifying individual register bits

[Peter Ujfalusi]

Hello Janusz,

On Friday 15 January 2010 18:43:11 ext Janusz Krzysztofik wrote:
> >> --- git/arch/arm/plat-omap/mcbsp.c.orig	2010-01-14 00:36:14.000000000
> >> +0100 +++ git/arch/arm/plat-omap/mcbsp.c	2010-01-14 02:05:23.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));
> >
> > The reg_cache will never have the XSYNC_ERR, or any other flags set,
> > since these flags has never written to the reg_cache.
> > So it is kind of not necessary to clear the flag, which is actually
> > always 0.
> 
> Agree.
> 
> > Another thing is that as far as I understand the reason behind of this
> > series is that you have a problem, that you can not trust on the values
> > that you read from the McBSP registers, if this is true, than the problem
> > can occur when the above path has been taken:
> >
> > ...
> > 	irqst_spcr2 = MCBSP_READ(mcbsp_tx, SPCR2);
> > ...
> > 	if (irqst_spcr2 & XSYNC_ERR) {
> >
> > But since you read from McBSP registers much rarely, than probably the
> > corruption is not that visible?
> 
> Sure no software solution can correct my hardware issue in case of
> register bits manintained by the hardware itself. Well, maybe software
> that limits heat dissipation by lowering overal power consumption could
> do to some extent ;).
> 
> What I'm going to address here is only a case when writing back possibly
> corrupted bits can be avoided if correct values are well known and
> can be determined without reading them back from the register itself.

Yeah, this is also my understanding, but I just did not found the right words ;)

> 
> > Anyway, clearing the status/error flags are not necessary, since the
> > reg_cache will never got these bits set, you could just write back the
> > SPCR2/SPCR1 register content from the cache as it is...
> >
> >>  	} 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));
> >
> > Same here.
> >
> > ...
> >
> >> @@ -653,7 +657,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));
> >
> > Well, I think here also, the reg_cache does not have the XSYNC_ERR set,
> > it is only set in the McBSP register.
> >
> >>  		/* resend */
> >>  		return -1;
> >>  	} else {
> >> @@ -662,10 +666,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));
> >
> > Also here, the XRST will surely not set in the cached SPCR2...
> >
> > This applies fro all other cases regarding to status/error bits in McBSP.
> 
> OK, I can try to identify all those cases.
> 
> What about introducing this simplification in a separate followup patch,
> quoting your rationale in its changelog? I can try to prepare one if you
> agree.

I think it is OK to have a followup patch addressing these.
Just mention in a comment, that you are writing the cached value back to the 
register, which does not have these status flags set, thus clearing the reason 
in McBSP.

Jarkko: What do you think?

Otherwise, I think this set is good to go.

> 
> Thanks,
> Janusz
> 

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

Re: [PATCH v9 0/4] OMAP: McBSP: Use register cache

[Peter Ujfalusi]

Hello,

On Thursday 14 January 2010 17:58:29 ext Janusz Krzysztofik wrote:
> 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.
> 

Please make a followup patch for the unneeded masking of status bits as we 
discussed regarding to patch 4.

Acked-by: Peter Ujfalusi <peter.ujfalusi@nokia.com>

Re: [PATCH v9 4/4] OMAP: McBSP: Use cache when modifying individual register bits

[Jarkko Nikula]

On Mon, 18 Jan 2010 10:05:12 +0200
Peter Ujfalusi <peter.ujfalusi@nokia.com> wrote:

> > What about introducing this simplification in a separate followup patch,
> > quoting your rationale in its changelog? I can try to prepare one if you
> > agree.
> 
> I think it is OK to have a followup patch addressing these.
> Just mention in a comment, that you are writing the cached value back to the 
> register, which does not have these status flags set, thus clearing the reason 
> in McBSP.
> 
> Jarkko: What do you think?
> 
I agree, follow-up patch sound cleaner and safer than modifying the
patch 4.

-- 
Jarkko

Re: [PATCH v9 0/4] OMAP: McBSP: Use register cache

[Tony Lindgren]

* Peter Ujfalusi <peter.ujfalusi@nokia.com> [100118 00:05]:
> Hello,
> 
> On Thursday 14 January 2010 17:58:29 ext Janusz Krzysztofik wrote:
> > 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.
> > 
> 
> Please make a followup patch for the unneeded masking of status bits as we 
> discussed regarding to patch 4.

Thanks for sorting out this issue Janusz.

I've applied this series into omap for-next with Acks from Jarkko and Peter.

Regards,

Tony