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

[PATCH v3 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.

Janusz Krzysztofik (4):
	OMAP: McBSP: Use macros for all register read/write operations
	OMAP: McBSP: Prepare register read/write macros API for caching
	OMAP: McBSP: Introduce caching in register write operations
	OMAP: McBSP: Use cache when modifying individual register bits

 arch/arm/plat-omap/include/plat/mcbsp.h |    5
 arch/arm/plat-omap/mcbsp.c              |  397 ++++++++++++++++++++++++---------------------------
 2 files changed, 198 insertions(+), 204 deletions(-)

Thanks,
Janusz

[PATCH v3 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 4421752e331cfb1d942b47ffdb26e451a8da58a0.

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

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

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

--- git/arch/arm/plat-omap/mcbsp.c.orig	2009-11-27 11:53:46.000000000 +0100
+++ git/arch/arm/plat-omap/mcbsp.c	2009-12-01 03:02:17.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 v3 2/4] OMAP: McBSP: Prepare register read/write macros API for caching

[Janusz Krzysztofik]

OMAP_MCBSP_READ()/WRITE() macros 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 both register AND cache
addresses to deal with.
Since OMAP_ prefix seems obvious, drop it off from macro names in order to
minimize line wrapping.

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

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

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

---
 arch/arm/plat-omap/mcbsp.c |  269 ++++++++++++++++++++++++++++------------------------------------
 1 file changed, 119 insertions(+), 150 deletions(-)

--- git/arch/arm/plat-omap/mcbsp.c.orig	2009-12-01 03:02:17.000000000 +0100
+++ git/arch/arm/plat-omap/mcbsp.c	2009-12-01 03:19:56.000000000 +0100
@@ -46,10 +46,10 @@ int omap_mcbsp_read(void __iomem *io_bas
 		return __raw_readl(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->io_base, OMAP_MCBSP_REG_##reg)
+#define MCBSP_WRITE(mcbsp, reg, val) \
+		omap_mcbsp_write(mcbsp->io_base, 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 v3 3/4] OMAP: McBSP: Introduce caching in register write operations

[Janusz Krzysztofik]

Reserve space inside omap_mcbsp structure for storing cached copies of McBSP
register values.
Modify the MCBSP_WRITE() macro to update the cache with every register write
operation.
Introduce a new macro that reads from the cache instead of hardware.

Applies on top of patch 2 from this series:
[PATCH v3 2/4] OMAP: McBSP: Prepare register read/write macros API for caching

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

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

---
 arch/arm/plat-omap/include/plat/mcbsp.h |    5 +++++
 arch/arm/plat-omap/mcbsp.c              |    6 +++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

--- git/arch/arm/plat-omap/include/plat/mcbsp.h.orig	2009-11-27 11:53:45.000000000 +0100
+++ git/arch/arm/plat-omap/include/plat/mcbsp.h	2009-12-01 03:37:21.000000000 +0100
@@ -415,6 +415,11 @@ struct omap_mcbsp {
 	u16 max_tx_thres;
 	u16 max_rx_thres;
 #endif
+#ifdef CONFIG_ARCH_OMAP1
+	u16 reg_cache[OMAP_MCBSP_REG_XCERH / sizeof(u16) + 1];
+#else
+	u32 reg_cache[OMAP_MCBSP_REG_RCCR / sizeof(u32) + 1];
+#endif
 };
 extern struct omap_mcbsp **mcbsp_ptr;
 extern int omap_mcbsp_count;
--- git/arch/arm/plat-omap/mcbsp.c.orig	2009-12-01 03:19:56.000000000 +0100
+++ git/arch/arm/plat-omap/mcbsp.c	2009-12-01 03:37:21.000000000 +0100
@@ -49,7 +49,11 @@ int omap_mcbsp_read(void __iomem *io_bas
 #define MCBSP_READ(mcbsp, reg) \
 		omap_mcbsp_read(mcbsp->io_base, OMAP_MCBSP_REG_##reg)
 #define MCBSP_WRITE(mcbsp, reg, val) \
-		omap_mcbsp_write(mcbsp->io_base, OMAP_MCBSP_REG_##reg, val)
+		omap_mcbsp_write(mcbsp->io_base, OMAP_MCBSP_REG_##reg, \
+		mcbsp->reg_cache[OMAP_MCBSP_REG_##reg / OMAP_MCBSP_REG_DRR1] \
+			= val)
+#define MCBSP_READ_CACHE(mcbsp, reg) \
+		(mcbsp->reg_cache[OMAP_MCBSP_REG_##reg / OMAP_MCBSP_REG_DRR1])
 
 #define omap_mcbsp_check_valid_id(id)	(id < omap_mcbsp_count)
 #define id_to_mcbsp_ptr(id)		mcbsp_ptr[id];

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

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

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

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

--- git/arch/arm/plat-omap/mcbsp.c.orig	2009-12-01 03:37:21.000000000 +0100
+++ git/arch/arm/plat-omap/mcbsp.c	2009-12-01 03:59:33.000000000 +0100
@@ -104,7 +104,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);
 	}
@@ -124,7 +125,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);
 	}
@@ -505,24 +507,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);
 
 	/*
@@ -535,16 +538,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);
 	}
@@ -570,28 +573,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));
 	}
 }
@@ -614,7 +618,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 {
@@ -623,10 +627,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);
@@ -653,7 +659,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 {
@@ -662,10 +668,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);
@@ -751,9 +759,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);
@@ -772,9 +782,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);
@@ -818,9 +830,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);
@@ -839,9 +853,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);

[Resend] [PATCH v3 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 4421752e331cfb1d942b47ffdb26e451a8da58a0.

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

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

---
Initial submission seems line wrapped, sorry.

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

--- git/arch/arm/plat-omap/mcbsp.c.orig	2009-11-27 11:53:46.000000000 +0100
+++ git/arch/arm/plat-omap/mcbsp.c	2009-12-01 03:02:17.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;
 }

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

[Jarkko Nikula]

On Tue, 1 Dec 2009 04:10:07 +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.
> 
> Janusz Krzysztofik (4):
> 	OMAP: McBSP: Use macros for all register read/write operations
> 	OMAP: McBSP: Prepare register read/write macros API for caching
> 	OMAP: McBSP: Introduce caching in register write operations
> 	OMAP: McBSP: Use cache when modifying individual register bits
> 
>  arch/arm/plat-omap/include/plat/mcbsp.h |    5
>  arch/arm/plat-omap/mcbsp.c              |  397 ++++++++++++++++++++++++---------------------------
>  2 files changed, 198 insertions(+), 204 deletions(-)
> 
Looks good to me and audio works fine with the Beagle. One minor
comment to the patch 3.

-- 
Jarkko

Re: [PATCH v3 3/4] OMAP: McBSP: Introduce caching in register write operations

[Jarkko Nikula]

On Tue, 1 Dec 2009 04:15:50 +0100
Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> wrote:

>  #define MCBSP_WRITE(mcbsp, reg, val) \
> -		omap_mcbsp_write(mcbsp->io_base, OMAP_MCBSP_REG_##reg, val)
> +		omap_mcbsp_write(mcbsp->io_base, OMAP_MCBSP_REG_##reg, \
> +		mcbsp->reg_cache[OMAP_MCBSP_REG_##reg / OMAP_MCBSP_REG_DRR1] \
> +			= val)
> +#define MCBSP_READ_CACHE(mcbsp, reg) \
> +		(mcbsp->reg_cache[OMAP_MCBSP_REG_##reg / OMAP_MCBSP_REG_DRR1])
>  
These divisions by DDR1 are confusing. Use rather sizeof:

reg_cache[OMAP_MCBSP_REG_##reg / sizeof(*mcbsp->reg_cache)]


-- 
Jarkko

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

[Peter Ujfalusi]

Hello,

On Thursday 03 December 2009 09:49:05 ext Jarkko Nikula wrote:
> On Tue, 1 Dec 2009 04:10:07 +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.
> >
> > Janusz Krzysztofik (4):
> > 	OMAP: McBSP: Use macros for all register read/write operations
> > 	OMAP: McBSP: Prepare register read/write macros API for caching
> > 	OMAP: McBSP: Introduce caching in register write operations
> > 	OMAP: McBSP: Use cache when modifying individual register bits
> >
> >  arch/arm/plat-omap/include/plat/mcbsp.h |    5
> >  arch/arm/plat-omap/mcbsp.c              |  397
> > ++++++++++++++++++++++++--------------------------- 2 files changed, 198
> > insertions(+), 204 deletions(-)
> 
> Looks good to me and audio works fine with the Beagle. One minor
> comment to the patch 3.

Sorry, but I have totally missed the discussion about this in l-o..
It looks good for me as well, the only comment I have is the same as Jarkko for 
patch 3.
I was about to give it a try, but since Jarkko already tested it on Beagle, I'm 
OK with this series (with the comment for patch 3).

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

[PATCH 3/4 v4] OMAP: McBSP: Introduce caching in register write operations

[Janusz Krzysztofik]

Reserve space inside omap_mcbsp structure for storing cached copies of McBSP
register values.
Modify the MCBSP_WRITE() macro to update the cache with every register write
operation.
Introduce a new macro that reads from the cache instead of hardware.

Applies on top of patch 2 from this series:
[PATCH v3 2/4] OMAP: McBSP: Prepare register read/write macros API for caching

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

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

---
Thursday 03 December 2009 08:49:21 Jarkko Nikula wrote:
> On Tue, 1 Dec 2009 04:15:50 +0100
> Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> wrote:
> >  #define MCBSP_WRITE(mcbsp, reg, val) \
> > -		omap_mcbsp_write(mcbsp->io_base, OMAP_MCBSP_REG_##reg, val)
> > +		omap_mcbsp_write(mcbsp->io_base, OMAP_MCBSP_REG_##reg, \
> > +		mcbsp->reg_cache[OMAP_MCBSP_REG_##reg / OMAP_MCBSP_REG_DRR1] \
> > +			= val)
> > +#define MCBSP_READ_CACHE(mcbsp, reg) \
> > +		(mcbsp->reg_cache[OMAP_MCBSP_REG_##reg / OMAP_MCBSP_REG_DRR1])
>
> These divisions by DDR1 are confusing. Use rather sizeof:
>
> reg_cache[OMAP_MCBSP_REG_##reg / sizeof(*mcbsp->reg_cache)]

Done.

Jarkko, many thanks for your cooperation :).

Janusz

 arch/arm/plat-omap/include/plat/mcbsp.h |    5 +++++
 arch/arm/plat-omap/mcbsp.c              |    7 ++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

--- git/arch/arm/plat-omap/include/plat/mcbsp.h.orig	2009-11-27 11:53:45.000000000 +0100
+++ git/arch/arm/plat-omap/include/plat/mcbsp.h	2009-12-01 03:37:21.000000000 +0100
@@ -415,6 +415,11 @@ struct omap_mcbsp {
 	u16 max_tx_thres;
 	u16 max_rx_thres;
 #endif
+#ifdef CONFIG_ARCH_OMAP1
+	u16 reg_cache[OMAP_MCBSP_REG_XCERH / sizeof(u16) + 1];
+#else
+	u32 reg_cache[OMAP_MCBSP_REG_RCCR / sizeof(u32) + 1];
+#endif
 };
 extern struct omap_mcbsp **mcbsp_ptr;
 extern int omap_mcbsp_count;
--- git/arch/arm/plat-omap/mcbsp.c.orig	2009-12-01 03:19:56.000000000 +0100
+++ git/arch/arm/plat-omap/mcbsp.c	2009-12-03 11:37:21.000000000 +0100
@@ -49,7 +49,12 @@ int omap_mcbsp_read(void __iomem *io_bas
 #define MCBSP_READ(mcbsp, reg) \
 		omap_mcbsp_read(mcbsp->io_base, OMAP_MCBSP_REG_##reg)
 #define MCBSP_WRITE(mcbsp, reg, val) \
-		omap_mcbsp_write(mcbsp->io_base, OMAP_MCBSP_REG_##reg, val)
+		omap_mcbsp_write(mcbsp->io_base, OMAP_MCBSP_REG_##reg, \
+		mcbsp->reg_cache[OMAP_MCBSP_REG_##reg / \
+				 sizeof(*mcbsp->reg_cache)] = val)
+#define MCBSP_READ_CACHE(mcbsp, reg) \
+		(mcbsp->reg_cache[OMAP_MCBSP_REG_##reg / \
+				  sizeof(*mcbsp->reg_cache)])
 
 #define omap_mcbsp_check_valid_id(id)	(id < omap_mcbsp_count)
 #define id_to_mcbsp_ptr(id)		mcbsp_ptr[id];

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

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

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

---
No changes since v3, just line numbers refreshed against v4 of patch 3/4.

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

--- git/arch/arm/plat-omap/mcbsp.c.orig	2009-12-03 11:37:21.000000000 +0100
+++ git/arch/arm/plat-omap/mcbsp.c	2009-12-03 11:59:33.000000000 +0100
@@ -105,7 +105,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);
 	}
@@ -125,7 +126,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);
 	}
@@ -506,24 +508,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);
 
 	/*
@@ -536,16 +539,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);
 	}
@@ -571,28 +574,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));
 	}
 }
@@ -615,7 +619,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 {
@@ -624,10 +628,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);
@@ -654,7 +660,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 {
@@ -663,10 +669,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);
@@ -752,9 +760,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);
@@ -773,9 +783,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);
@@ -819,9 +831,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);
@@ -840,9 +854,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 v3 0/4] OMAP: McBSP: Use register cache

[Janusz Krzysztofik]

Thursday 03 December 2009 10:35:37 Peter Ujfalusi napisał(a):
> Hello,
>
> On Thursday 03 December 2009 09:49:05 ext Jarkko Nikula wrote:
> > On Tue, 1 Dec 2009 04:10:07 +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.
> > >
> > > Janusz Krzysztofik (4):
> > > 	OMAP: McBSP: Use macros for all register read/write operations
> > > 	OMAP: McBSP: Prepare register read/write macros API for caching
> > > 	OMAP: McBSP: Introduce caching in register write operations
> > > 	OMAP: McBSP: Use cache when modifying individual register bits
> > >
> > >  arch/arm/plat-omap/include/plat/mcbsp.h |    5
> > >  arch/arm/plat-omap/mcbsp.c              |  397
> > > ++++++++++++++++++++++++--------------------------- 2 files changed,
> > > 198 insertions(+), 204 deletions(-)
> >
> > Looks good to me and audio works fine with the Beagle. One minor
> > comment to the patch 3.
>
> Sorry, but I have totally missed the discussion about this in l-o..
> It looks good for me as well, the only comment I have is the same as Jarkko
> for patch 3.
> I was about to give it a try, but since Jarkko already tested it on Beagle,
> I'm OK with this series (with the comment for patch 3).

Hi Peter,

That's probably my fault, I've dropped you from CC: at some point, sorry.

I'm still learning whom I should CC: and whom I shouldn't in order to rise my 
chances on patches being commented, acknowledged and applied in a reasonable 
time ;). Initial version of this one was waiting since August, that's why I 
tried to narrow the audience in hope it could help getting it rejected or 
accepted finally.

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

[Tony Lindgren]

Hi,

* Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> [091203 04:29]:
> Reserve space inside omap_mcbsp structure for storing cached copies of McBSP
> register values.
> Modify the MCBSP_WRITE() macro to update the cache with every register write
> operation.
> Introduce a new macro that reads from the cache instead of hardware.
> 
> Applies on top of patch 2 from this series:
> [PATCH v3 2/4] OMAP: McBSP: Prepare register read/write macros API for caching
> 
> Tested on OMAP1510 based Amstrad Delta using linux-omap for-next,
> commit 4421752e331cfb1d942b47ffdb26e451a8da58a0.
> Compile-tested with omap_3430sdp_defconfig.
> 
> Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
> 
> ---
> Thursday 03 December 2009 08:49:21 Jarkko Nikula wrote:
> > On Tue, 1 Dec 2009 04:15:50 +0100
> > Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> wrote:
> > >  #define MCBSP_WRITE(mcbsp, reg, val) \
> > > -		omap_mcbsp_write(mcbsp->io_base, OMAP_MCBSP_REG_##reg, val)
> > > +		omap_mcbsp_write(mcbsp->io_base, OMAP_MCBSP_REG_##reg, \
> > > +		mcbsp->reg_cache[OMAP_MCBSP_REG_##reg / OMAP_MCBSP_REG_DRR1] \
> > > +			= val)
> > > +#define MCBSP_READ_CACHE(mcbsp, reg) \
> > > +		(mcbsp->reg_cache[OMAP_MCBSP_REG_##reg / OMAP_MCBSP_REG_DRR1])
> >
> > These divisions by DDR1 are confusing. Use rather sizeof:
> >
> > reg_cache[OMAP_MCBSP_REG_##reg / sizeof(*mcbsp->reg_cache)]
> 
> Done.
> 
> Jarkko, many thanks for your cooperation :).
> 
> Janusz
> 
>  arch/arm/plat-omap/include/plat/mcbsp.h |    5 +++++
>  arch/arm/plat-omap/mcbsp.c              |    7 ++++++-
>  2 files changed, 11 insertions(+), 1 deletion(-)
> 
> --- git/arch/arm/plat-omap/include/plat/mcbsp.h.orig	2009-11-27 11:53:45.000000000 +0100
> +++ git/arch/arm/plat-omap/include/plat/mcbsp.h	2009-12-01 03:37:21.000000000 +0100
> @@ -415,6 +415,11 @@ struct omap_mcbsp {
>  	u16 max_tx_thres;
>  	u16 max_rx_thres;
>  #endif
> +#ifdef CONFIG_ARCH_OMAP1
> +	u16 reg_cache[OMAP_MCBSP_REG_XCERH / sizeof(u16) + 1];
> +#else
> +	u32 reg_cache[OMAP_MCBSP_REG_RCCR / sizeof(u32) + 1];
> +#endif
>  };
>  extern struct omap_mcbsp **mcbsp_ptr;
>  extern int omap_mcbsp_count;

How about rather set:

static u16 omap1_reg_cache[];

and

static u32 omap2_reg_cache[];


> --- git/arch/arm/plat-omap/mcbsp.c.orig	2009-12-01 03:19:56.000000000 +0100
> +++ git/arch/arm/plat-omap/mcbsp.c	2009-12-03 11:37:21.000000000 +0100
> @@ -49,7 +49,12 @@ int omap_mcbsp_read(void __iomem *io_bas
>  #define MCBSP_READ(mcbsp, reg) \
>  		omap_mcbsp_read(mcbsp->io_base, OMAP_MCBSP_REG_##reg)
>  #define MCBSP_WRITE(mcbsp, reg, val) \
> -		omap_mcbsp_write(mcbsp->io_base, OMAP_MCBSP_REG_##reg, val)
> +		omap_mcbsp_write(mcbsp->io_base, OMAP_MCBSP_REG_##reg, \
> +		mcbsp->reg_cache[OMAP_MCBSP_REG_##reg / \
> +				 sizeof(*mcbsp->reg_cache)] = val)
> +#define MCBSP_READ_CACHE(mcbsp, reg) \
> +		(mcbsp->reg_cache[OMAP_MCBSP_REG_##reg / \
> +				  sizeof(*mcbsp->reg_cache)])
>  
>  #define omap_mcbsp_check_valid_id(id)	(id < omap_mcbsp_count)
>  #define id_to_mcbsp_ptr(id)		mcbsp_ptr[id];

Then rather than doing this cache manipulation in the macro, just
do it in the omap_mcbsp_read/write functions based on the
cpu_class_is_omap1() accessing omap1_reg_cache or omap2_reg_cache.
This way you can get rid of the ifdef else.

Regards,

Tony

Re: [PATCH 3/4 v4] OMAP: McBSP: Introduce caching in register write operations

[Janusz Krzysztofik]

Thursday 03 December 2009 21:03:02 Tony Lindgren napisał(a):
> Hi,
>
> * Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> [091203 04:29]:
> > Reserve space inside omap_mcbsp structure for storing cached copies of
> > McBSP register values.
> > Modify the MCBSP_WRITE() macro to update the cache with every register
> > write operation.
> > Introduce a new macro that reads from the cache instead of hardware.
> >
> > Applies on top of patch 2 from this series:
> > [PATCH v3 2/4] OMAP: McBSP: Prepare register read/write macros API for
> > caching
> >
> > Tested on OMAP1510 based Amstrad Delta using linux-omap for-next,
> > commit 4421752e331cfb1d942b47ffdb26e451a8da58a0.
> > Compile-tested with omap_3430sdp_defconfig.
> >
> > Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
> >
> > ---
> >
> > Thursday 03 December 2009 08:49:21 Jarkko Nikula wrote:
> > > On Tue, 1 Dec 2009 04:15:50 +0100
> > >
> > > Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> wrote:
> > > >  #define MCBSP_WRITE(mcbsp, reg, val) \
> > > > -		omap_mcbsp_write(mcbsp->io_base, OMAP_MCBSP_REG_##reg, val)
> > > > +		omap_mcbsp_write(mcbsp->io_base, OMAP_MCBSP_REG_##reg, \
> > > > +		mcbsp->reg_cache[OMAP_MCBSP_REG_##reg / OMAP_MCBSP_REG_DRR1] \
> > > > +			= val)
> > > > +#define MCBSP_READ_CACHE(mcbsp, reg) \
> > > > +		(mcbsp->reg_cache[OMAP_MCBSP_REG_##reg / OMAP_MCBSP_REG_DRR1])
> > >
> > > These divisions by DDR1 are confusing. Use rather sizeof:
> > >
> > > reg_cache[OMAP_MCBSP_REG_##reg / sizeof(*mcbsp->reg_cache)]
> >
> > Done.
> >
> > Jarkko, many thanks for your cooperation :).
> >
> > Janusz
> >
> >  arch/arm/plat-omap/include/plat/mcbsp.h |    5 +++++
> >  arch/arm/plat-omap/mcbsp.c              |    7 ++++++-
> >  2 files changed, 11 insertions(+), 1 deletion(-)
> >
> > --- git/arch/arm/plat-omap/include/plat/mcbsp.h.orig	2009-11-27
> > 11:53:45.000000000 +0100 +++
> > git/arch/arm/plat-omap/include/plat/mcbsp.h	2009-12-01 03:37:21.000000000
> > +0100 @@ -415,6 +415,11 @@ struct omap_mcbsp {
> >  	u16 max_tx_thres;
> >  	u16 max_rx_thres;
> >  #endif
> > +#ifdef CONFIG_ARCH_OMAP1
> > +	u16 reg_cache[OMAP_MCBSP_REG_XCERH / sizeof(u16) + 1];
> > +#else
> > +	u32 reg_cache[OMAP_MCBSP_REG_RCCR / sizeof(u32) + 1];
> > +#endif
> >  };
> >  extern struct omap_mcbsp **mcbsp_ptr;
> >  extern int omap_mcbsp_count;
>
> How about rather set:
>
> static u16 omap1_reg_cache[];
>
> and
>
> static u32 omap2_reg_cache[];

Hi Tony,

Let me understand more precisely what you mean before I submit next version.

Did you mean something like:

static u16 omap1_reg_cache[3][OMAP_MCBSP_REG_XCERH / sizeof(u16) + 1];
static u32 omap2_reg_cache[5][OMAP_MCBSP_REG_RCCR / sizeof(u32) + 1];

both put in arch/arm/plat-omap/mcbsp.c without any ifdefs?

> > --- git/arch/arm/plat-omap/mcbsp.c.orig	2009-12-01 03:19:56.000000000
> > +0100 +++ git/arch/arm/plat-omap/mcbsp.c	2009-12-03 11:37:21.000000000
> > +0100 @@ -49,7 +49,12 @@ int omap_mcbsp_read(void __iomem *io_bas
> >  #define MCBSP_READ(mcbsp, reg) \
> >  		omap_mcbsp_read(mcbsp->io_base, OMAP_MCBSP_REG_##reg)
> >  #define MCBSP_WRITE(mcbsp, reg, val) \
> > -		omap_mcbsp_write(mcbsp->io_base, OMAP_MCBSP_REG_##reg, val)
> > +		omap_mcbsp_write(mcbsp->io_base, OMAP_MCBSP_REG_##reg, \
> > +		mcbsp->reg_cache[OMAP_MCBSP_REG_##reg / \
> > +				 sizeof(*mcbsp->reg_cache)] = val)
> > +#define MCBSP_READ_CACHE(mcbsp, reg) \
> > +		(mcbsp->reg_cache[OMAP_MCBSP_REG_##reg / \
> > +				  sizeof(*mcbsp->reg_cache)])
> >
> >  #define omap_mcbsp_check_valid_id(id)	(id < omap_mcbsp_count)
> >  #define id_to_mcbsp_ptr(id)		mcbsp_ptr[id];
>
> Then rather than doing this cache manipulation in the macro, just
> do it in the omap_mcbsp_read/write functions based on the
> cpu_class_is_omap1() accessing omap1_reg_cache or omap2_reg_cache.

omap_mcbsp_wrtie() case seems clear to me, but:

Did you mean defining a new omap_mcbsp_read_cache() function, similiar to 
omap_mcbsp_read(), but reading from cache instead of hardware?

Or do you prefere if I modify omap_mcbsp_read() to be able to read from both 
hardware and cache, dependig on an additional argument, for example?

Thanks,
Janusz

> This way you can get rid of the ifdef else.
>
> Regards,
>
> Tony
>
> --
> 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


--
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 v3 0/4] OMAP: McBSP: Use register cache

[Peter Ujfalusi]

Hello Janusz,

On Thursday 03 December 2009 14:31:20 ext Janusz Krzysztofik wrote:
> > Sorry, but I have totally missed the discussion about this in l-o..
> > It looks good for me as well, the only comment I have is the same as
> > Jarkko for patch 3.
> > I was about to give it a try, but since Jarkko already tested it on
> > Beagle, I'm OK with this series (with the comment for patch 3).
> 
> Hi Peter,
> 
> That's probably my fault, I've dropped you from CC: at some point, sorry.

Well, it is mostly my fault. I need to follow the l-o list more closely, than I 
did.. I have a faint recollection of some earlier patches, but for some reason I 
have not reacted to them (can't really recall why).

Anyways, I'll try to be more active on McBSP issues. My name is in the files 
thanks to Jarkko, which comes with some responsibility as well ;)
 
> I'm still learning whom I should CC: and whom I shouldn't in order to rise
>  my chances on patches being commented, acknowledged and applied in a
>  reasonable time ;). Initial version of this one was waiting since August,
>  that's why I tried to narrow the audience in hope it could help getting it
>  rejected or accepted finally.
> 
> Thanks,
> Janusz
> 

Thanks,
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 3/4 v4] OMAP: McBSP: Introduce caching in register write operations

[Janusz Krzysztofik]

Janusz Krzysztofik napisał(a):
> Thursday 03 December 2009 21:03:02 Tony Lindgren napisał(a):
>> Hi,
>>
>> * Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> [091203 04:29]:
>>> Reserve space inside omap_mcbsp structure for storing cached copies of
>>> McBSP register values.
>>> Modify the MCBSP_WRITE() macro to update the cache with every register
>>> write operation.
>>> Introduce a new macro that reads from the cache instead of hardware.
>>>
>>> Applies on top of patch 2 from this series:
>>> [PATCH v3 2/4] OMAP: McBSP: Prepare register read/write macros API for
>>> caching
>>>
>>> Tested on OMAP1510 based Amstrad Delta using linux-omap for-next,
>>> commit 4421752e331cfb1d942b47ffdb26e451a8da58a0.
>>> Compile-tested with omap_3430sdp_defconfig.
>>>
>>> Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
>>>
>>> ---
>>>
>>> Thursday 03 December 2009 08:49:21 Jarkko Nikula wrote:
>>>> On Tue, 1 Dec 2009 04:15:50 +0100
>>>>
>>>> Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> wrote:
>>>>>  #define MCBSP_WRITE(mcbsp, reg, val) \
>>>>> -		omap_mcbsp_write(mcbsp->io_base, OMAP_MCBSP_REG_##reg, val)
>>>>> +		omap_mcbsp_write(mcbsp->io_base, OMAP_MCBSP_REG_##reg, \
>>>>> +		mcbsp->reg_cache[OMAP_MCBSP_REG_##reg / OMAP_MCBSP_REG_DRR1] \
>>>>> +			= val)
>>>>> +#define MCBSP_READ_CACHE(mcbsp, reg) \
>>>>> +		(mcbsp->reg_cache[OMAP_MCBSP_REG_##reg / OMAP_MCBSP_REG_DRR1])
>>>> These divisions by DDR1 are confusing. Use rather sizeof:
>>>>
>>>> reg_cache[OMAP_MCBSP_REG_##reg / sizeof(*mcbsp->reg_cache)]
>>> Done.
>>>
>>> Jarkko, many thanks for your cooperation :).
>>>
>>> Janusz
>>>
>>>  arch/arm/plat-omap/include/plat/mcbsp.h |    5 +++++
>>>  arch/arm/plat-omap/mcbsp.c              |    7 ++++++-
>>>  2 files changed, 11 insertions(+), 1 deletion(-)
>>>
>>> --- git/arch/arm/plat-omap/include/plat/mcbsp.h.orig	2009-11-27
>>> 11:53:45.000000000 +0100 +++
>>> git/arch/arm/plat-omap/include/plat/mcbsp.h	2009-12-01 03:37:21.000000000
>>> +0100 @@ -415,6 +415,11 @@ struct omap_mcbsp {
>>>  	u16 max_tx_thres;
>>>  	u16 max_rx_thres;
>>>  #endif
>>> +#ifdef CONFIG_ARCH_OMAP1
>>> +	u16 reg_cache[OMAP_MCBSP_REG_XCERH / sizeof(u16) + 1];
>>> +#else
>>> +	u32 reg_cache[OMAP_MCBSP_REG_RCCR / sizeof(u32) + 1];
>>> +#endif
>>>  };
>>>  extern struct omap_mcbsp **mcbsp_ptr;
>>>  extern int omap_mcbsp_count;
>> How about rather set:
>>
>> static u16 omap1_reg_cache[];
>>
>> and
>>
>> static u32 omap2_reg_cache[];
> 
> Hi Tony,
> 
> Let me understand more precisely what you mean before I submit next version.
> 
> Did you mean something like:
> 
> static u16 omap1_reg_cache[3][OMAP_MCBSP_REG_XCERH / sizeof(u16) + 1];
> static u32 omap2_reg_cache[5][OMAP_MCBSP_REG_RCCR / sizeof(u32) + 1];
> 
> both put in arch/arm/plat-omap/mcbsp.c without any ifdefs?

Tony,

I think I've already found one part of the answer myself: you probably 
mean putting each table definition in a corresponding 
arch/arm/mach-omap[12]/mcbsp.c source file, don't you?

What remains unclear for me is if you really intend to reserve static 
cache space for all McBSP ports supported by a machine, even if not 
supported by a particular cpu or not used on a particular board.

>>> --- git/arch/arm/plat-omap/mcbsp.c.orig	2009-12-01 03:19:56.000000000
>>> +0100 +++ git/arch/arm/plat-omap/mcbsp.c	2009-12-03 11:37:21.000000000
>>> +0100 @@ -49,7 +49,12 @@ int omap_mcbsp_read(void __iomem *io_bas
>>>  #define MCBSP_READ(mcbsp, reg) \
>>>  		omap_mcbsp_read(mcbsp->io_base, OMAP_MCBSP_REG_##reg)
>>>  #define MCBSP_WRITE(mcbsp, reg, val) \
>>> -		omap_mcbsp_write(mcbsp->io_base, OMAP_MCBSP_REG_##reg, val)
>>> +		omap_mcbsp_write(mcbsp->io_base, OMAP_MCBSP_REG_##reg, \
>>> +		mcbsp->reg_cache[OMAP_MCBSP_REG_##reg / \
>>> +				 sizeof(*mcbsp->reg_cache)] = val)
>>> +#define MCBSP_READ_CACHE(mcbsp, reg) \
>>> +		(mcbsp->reg_cache[OMAP_MCBSP_REG_##reg / \
>>> +				  sizeof(*mcbsp->reg_cache)])
>>>
>>>  #define omap_mcbsp_check_valid_id(id)	(id < omap_mcbsp_count)
>>>  #define id_to_mcbsp_ptr(id)		mcbsp_ptr[id];
>> Then rather than doing this cache manipulation in the macro, just
>> do it in the omap_mcbsp_read/write functions based on the
>> cpu_class_is_omap1() accessing omap1_reg_cache or omap2_reg_cache.
> 
> omap_mcbsp_wrtie() case seems clear to me, but:
> 
> Did you mean defining a new omap_mcbsp_read_cache() function, similiar to 
> omap_mcbsp_read(), but reading from cache instead of hardware?
> 
> Or do you prefere if I modify omap_mcbsp_read() to be able to read from both 
> hardware and cache, dependig on an additional argument, for example?
> 
> Thanks,
> Janusz
> 
>> This way you can get rid of the ifdef else.

But shouldn't declarations for both tables be placed in 
arch/arm/plat-omap/include/plat/mcbsp.h withing an ifdef else block? If 
yes, one ifdef else block would be replaced with another. Or did you 
mean placing those declarations inside existing ifdef block with defines 
for register offsets?

Thanks,
Janusz


>>
>> Regards,
>>
>> Tony
>>
>> --
>> 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
> 
> 
> --
> 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
> 

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

[Tony Lindgren]

* Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> [091204 04:56]:
> Janusz Krzysztofik napisał(a):

<snip>
 
> I think I've already found one part of the answer myself: you
> probably mean putting each table definition in a corresponding
> arch/arm/mach-omap[12]/mcbsp.c source file, don't you?

Well I did not initially, but sounds like that's the cleanest
way to go.
 
> What remains unclear for me is if you really intend to reserve
> static cache space for all McBSP ports supported by a machine, even
> if not supported by a particular cpu or not used on a particular
> board.

No idea :) But please consider what happens if we want to compile in
omap2 + omap3 into a single binary. Or what happens if some newer omaps
add more McBSP registers.

<snip>
 
> >
> >Did you mean defining a new omap_mcbsp_read_cache() function,
> >similiar to omap_mcbsp_read(), but reading from cache instead of
> >hardware?

Yeah something like that. That is more future proof.

> >Or do you prefere if I modify omap_mcbsp_read() to be able to read
> >from both hardware and cache, dependig on an additional argument,
> >for example?

That actually sounds like the best way to solve it to me!

> But shouldn't declarations for both tables be placed in
> arch/arm/plat-omap/include/plat/mcbsp.h withing an ifdef else block?
> If yes, one ifdef else block would be replaced with another. Or did
> you mean placing those declarations inside existing ifdef block with
> defines for register offsets?

Ifdefs are OK if they just optimize out code when some omap is not
compiled in. But if you do ifdef else, then it means trouble sooner
or later. In this case you should probably have the cache initialized
separately in mach-omap1/mcbsp.c and mach-omap2/mcbsp.c.

Regards,

Tony
--
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/4 v5a] OMAP: McBSP: Introduce caching in register write operations

[Janusz Krzysztofik]

Reserve space inside omap_mcbsp structure for storing cached copies of McBSP
register values.
Modify the MCBSP_WRITE() macro to update the cache with every register write
operation.
Introduce a new macro that reads from the cache instead of hardware.

Applies on top of patch 2 from this series:
[PATCH v3 2/4] OMAP: McBSP: Prepare register read/write macros API for caching

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

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

---

Tony,

Since I was not sure what your primary concern was, I've prepared two
alternative versions of patch 3/4.
Please take this one as a base for further discussion if your primary concern
was potentially dangerous #else clause. I've replaced it with #elif, that
should be more future proof.
Otherwise, have a look the other one (v5b).

Thanks,
Janusz

--- git/arch/arm/plat-omap/include/plat/mcbsp.h.orig	2009-11-27 11:53:45.000000000 +0100
+++ git/arch/arm/plat-omap/include/plat/mcbsp.h	2009-12-05 03:37:21.000000000 +0100
@@ -415,6 +415,13 @@ struct omap_mcbsp {
 	u16 max_tx_thres;
 	u16 max_rx_thres;
 #endif
+#if defined(CONFIG_ARCH_OMAP15XX) || defined(CONFIG_ARCH_OMAP16XX) || \
+		defined(CONFIG_ARCH_OMAP730) || defined(CONFIG_ARCH_OMAP850)
+	u16 reg_cache[OMAP_MCBSP_REG_XCERH / sizeof(u16) + 1];
+#elif defined(CONFIG_ARCH_OMAP24XX) || defined(CONFIG_ARCH_OMAP34XX) || \
+		defined(CONFIG_ARCH_OMAP4)
+	u32 reg_cache[OMAP_MCBSP_REG_RCCR / sizeof(u32) + 1];
+#endif
 };
 extern struct omap_mcbsp **mcbsp_ptr;
 extern int omap_mcbsp_count;
--- git/arch/arm/plat-omap/mcbsp.c.orig	2009-12-01 03:19:56.000000000 +0100
+++ git/arch/arm/plat-omap/mcbsp.c	2009-12-01 03:37:21.000000000 +0100
@@ -49,7 +49,12 @@ int omap_mcbsp_read(void __iomem *io_bas
 #define MCBSP_READ(mcbsp, reg) \
 		omap_mcbsp_read(mcbsp->io_base, OMAP_MCBSP_REG_##reg)
 #define MCBSP_WRITE(mcbsp, reg, val) \
-		omap_mcbsp_write(mcbsp->io_base, OMAP_MCBSP_REG_##reg, val)
+		omap_mcbsp_write(mcbsp->io_base, OMAP_MCBSP_REG_##reg, \
+		mcbsp->reg_cache[OMAP_MCBSP_REG_##reg / \
+				 sizeof(*mcbsp->reg_cache)] = val)
+#define MCBSP_READ_CACHE(mcbsp, reg) \
+		(mcbsp->reg_cache[OMAP_MCBSP_REG_##reg / \
+				  sizeof(*mcbsp->reg_cache)])
 
 #define omap_mcbsp_check_valid_id(id)	(id < omap_mcbsp_count)
 #define id_to_mcbsp_ptr(id)		mcbsp_ptr[id];

[RFC][RFT][PATCH 3/4 v5b] OMAP: McBSP: Introduce caching in register write operations

[Janusz Krzysztofik]

Reserve static space for storing cached copies of McBSP register values.
Split omap_mcbsp_read()/omap_mcbsp_write() into separate functions for OMAP1
and OMAP2/3/4, move them from plat-omap to mach-omap1 / mach-omap2 to be able
to access the static cache.
Modify omap_msbcp_write() to update the cache with every register write
operation.
Modify omap_mcbsp_read() to support reading from cache or hardware.
Update MCBSP_READ/MCBSP_WRITE macros for modified function APIs.
Introduce a new macro that reads from the cache.

Applies on top of patch 2 from this series:
[PATCH v3 2/4] OMAP: McBSP: Prepare register read/write macros API for caching

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

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

---

Tony,

Since I was not sure what your primary concern was, I've prepared two
alternative versions of patch 3/4.
Please take this one as a base for further discussion if your primary concern
was storage class for the cache.
Otherwise, have a look at the other one (v5a), since this one has even more
ifdefs than before.

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-05 06:42:35.000000000 +0100
@@ -200,3 +200,26 @@ int __init omap1_mcbsp_init(void)
 }
 
 arch_initcall(omap1_mcbsp_init);
+
+/* if adding more, put larger first */
+#if defined(CONFIG_ARCH_OMAP16XX)
+static u16 omap_mcbsp_cache[OMAP16XX_MCBSP_PDATA_SZ][OMAP_MCBSP_REG_COUNT];
+#elif defined(CONFIG_ARCH_OMAP15XX)
+static u16 omap_mcbsp_cache[OMAP15XX_MCBSP_PDATA_SZ][OMAP_MCBSP_REG_COUNT];
+#elif defined(CONFIG_ARCH_OMAP7XX)
+static u16 omap_mcbsp_cache[OMAP7XX_MCBSP_PDATA_SZ][OMAP_MCBSP_REG_COUNT];
+#else
+#define omap_mcbsp_cache	NULL
+#endif
+
+void omap_mcbsp_write(struct omap_mcbsp *mcbsp, u16 reg, u32 val)
+{
+	omap_mcbsp_cache[mcbsp->id][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) :
+			omap_mcbsp_cache[mcbsp->id][reg / sizeof(u16)];
+}
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-05 06:42:35.000000000 +0100
@@ -241,3 +241,40 @@ static int __init omap2_mcbsp_init(void)
 	return omap_mcbsp_init();
 }
 arch_initcall(omap2_mcbsp_init);
+
+/* if adding more, put larger first */
+#if defined(CONFIG_ARCH_OMAP34XX)
+static u32 omap_mcbsp_cache[OMAP34XX_MCBSP_PDATA_SZ][OMAP_MCBSP_REG_COUNT];
+#elif defined(CONFIG_ARCH_OMAP2430)
+static u32 omap_mcbsp_cache[OMAP2430_MCBSP_PDATA_SZ][OMAP_MCBSP_REG_COUNT];
+#elif defined(CONFIG_ARCH_OMAP44XX)
+static u32 omap_mcbsp_cache[OMAP44XX_MCBSP_PDATA_SZ][OMAP_MCBSP_REG_COUNT];
+#elif defined(CONFIG_ARCH_OMAP2420)
+static u16 omap_mcbsp_cache[OMAP2420_MCBSP_PDATA_SZ][OMAP_MCBSP_REG_COUNT];
+#else
+#define omap_mcbsp_cache	NULL
+#endif
+
+void omap_mcbsp_write(struct omap_mcbsp *mcbsp, u16 reg, u32 val)
+{
+	if (cpu_is_omap2420()) {
+		omap_mcbsp_cache[mcbsp->id][reg / sizeof(*omap_mcbsp_cache)] \
+				= (u16)val;
+		__raw_writew((u16)val, mcbsp->io_base + reg);
+	} else {
+		omap_mcbsp_cache[mcbsp->id][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)omap_mcbsp_cache[mcbsp->id][reg /
+					sizeof(*omap_mcbsp_cache)];
+	} else {
+		return !from_cache ? __raw_readl(mcbsp->io_base + reg) :
+			omap_mcbsp_cache[mcbsp->id][reg / sizeof(u32)];
+	}
+}
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-05 06:42:35.000000000 +0100
@@ -92,6 +92,8 @@
 #define OMAP_MCBSP_REG_XCERG	0x3A
 #define OMAP_MCBSP_REG_XCERH	0x3C
 
+#define OMAP_MCBSP_REG_COUNT	(OMAP_MCBSP_REG_XCERH / OMAP_MCBSP_REG_DRR1 + 1)
+
 /* Dummy defines, these are not available on omap1 */
 #define OMAP_MCBSP_REG_XCCR	0x00
 #define OMAP_MCBSP_REG_RCCR	0x00
@@ -148,6 +150,8 @@
 #define OMAP_MCBSP_REG_XCCR	0xAC
 #define OMAP_MCBSP_REG_RCCR	0xB0
 
+#define OMAP_MCBSP_REG_COUNT	(OMAP_MCBSP_REG_RCCR / OMAP_MCBSP_REG_DRR1 + 1)
+
 #define AUDIO_MCBSP_DATAWRITE	(OMAP24XX_MCBSP2_BASE + OMAP_MCBSP_REG_DXR1)
 #define AUDIO_MCBSP_DATAREAD	(OMAP24XX_MCBSP2_BASE + OMAP_MCBSP_REG_DRR1)
 
@@ -460,3 +464,6 @@ int omap_mcbsp_pollwrite(unsigned int id
 int omap_mcbsp_set_io_type(unsigned int id, omap_mcbsp_io_type_t io_type);
 
 #endif
+
+extern int omap_mcbsp_read(struct omap_mcbsp *mcbsp, u16 reg, bool from_cache);
+extern void omap_mcbsp_write(struct omap_mcbsp *mcbsp, u16 reg, u32 val);
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-05 06:23:39.000000000 +0100
+++ git/arch/arm/plat-omap/mcbsp.c	2009-12-05 06:42:35.000000000 +0100
@@ -30,26 +30,12 @@
 struct omap_mcbsp **mcbsp_ptr;
 int omap_mcbsp_count;
 
-void omap_mcbsp_write(void __iomem *io_base, u16 reg, u32 val)
-{
-	if (cpu_class_is_omap1() || cpu_is_omap2420())
-		__raw_writew((u16)val, io_base + reg);
-	else
-		__raw_writel(val, io_base + reg);
-}
-
-int omap_mcbsp_read(void __iomem *io_base, u16 reg)
-{
-	if (cpu_class_is_omap1() || cpu_is_omap2420())
-		return __raw_readw(io_base + reg);
-	else
-		return __raw_readl(io_base + reg);
-}
-
 #define MCBSP_READ(mcbsp, reg) \
-		omap_mcbsp_read(mcbsp->io_base, OMAP_MCBSP_REG_##reg)
+		omap_mcbsp_read(mcbsp, OMAP_MCBSP_REG_##reg, 0)
 #define MCBSP_WRITE(mcbsp, reg, val) \
-		omap_mcbsp_write(mcbsp->io_base, OMAP_MCBSP_REG_##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];

Re: [RFC][RFT][PATCH 3/4 v5b] OMAP: McBSP: Introduce caching in register write operations

[Tony Lindgren]

Hi,

This is pretty close :) Few more tweaks are still needed.

* Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> [091205 05:46]:
> Reserve static space for storing cached copies of McBSP register values.
> Split omap_mcbsp_read()/omap_mcbsp_write() into separate functions for OMAP1
> and OMAP2/3/4, move them from plat-omap to mach-omap1 / mach-omap2 to be able
> to access the static cache.
> Modify omap_msbcp_write() to update the cache with every register write
> operation.
> Modify omap_mcbsp_read() to support reading from cache or hardware.
> Update MCBSP_READ/MCBSP_WRITE macros for modified function APIs.
> Introduce a new macro that reads from the cache.
> 
> Applies on top of patch 2 from this series:
> [PATCH v3 2/4] OMAP: McBSP: Prepare register read/write macros API for caching
> 
> Tested on OMAP1510 based Amstrad Delta using linux-omap for-next,
> commit 82f1d8f22f2c65e70206e40a6f17688bf64a892c.
> Compile-tested with omap_generic_2420_defconfig, omap_3430sdp_defconfig.
> 
> Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
> 
> ---
> 
> Tony,
> 
> Since I was not sure what your primary concern was, I've prepared two
> alternative versions of patch 3/4.
> Please take this one as a base for further discussion if your primary concern
> was storage class for the cache.
> Otherwise, have a look at the other one (v5a), since this one has even more
> ifdefs than before.
> 
> 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-05 06:42:35.000000000 +0100
> @@ -200,3 +200,26 @@ int __init omap1_mcbsp_init(void)
>  }
>  
>  arch_initcall(omap1_mcbsp_init);
> +
> +/* if adding more, put larger first */
> +#if defined(CONFIG_ARCH_OMAP16XX)
> +static u16 omap_mcbsp_cache[OMAP16XX_MCBSP_PDATA_SZ][OMAP_MCBSP_REG_COUNT];
> +#elif defined(CONFIG_ARCH_OMAP15XX)
> +static u16 omap_mcbsp_cache[OMAP15XX_MCBSP_PDATA_SZ][OMAP_MCBSP_REG_COUNT];
> +#elif defined(CONFIG_ARCH_OMAP7XX)
> +static u16 omap_mcbsp_cache[OMAP7XX_MCBSP_PDATA_SZ][OMAP_MCBSP_REG_COUNT];
> +#else
> +#define omap_mcbsp_cache	NULL
> +#endif

As 15xx and 16xx can be compiled into the same kernel binary, we need to
get rid of the ifdef elif else (untested):

#ifdef(CONFIG_ARCH_OMAP7XX)
static u16 omap7xx_mcbsp_cache[OMAP7XX_MCBSP_PDATA_SZ][OMAP7XX_MCBSP_REG_COUNT];
#else
static 16 omap7xx_mcbsp_cache;
#endif

#if defined(CONFIG_ARCH_OMAP15XX)
static u16 omap15xx_mcbsp_cache[OMAP15XX_MCBSP_PDATA_SZ][OMAP15XX_MCBSP_REG_COUNT];
#else
static 16 omap15xx_mcbsp_cache;
#endif

#if defined(CONFIG_ARCH_OMAP16XX)
static u16 omap16xx_mcbsp_cache[OMAP16XX_MCBSP_PDATA_SZ][OMAP_16XXMCBSP_REG_COUNT];
#else
static 16 omap16xx_mcbsp_cache;
#endif
...


> +void omap_mcbsp_write(struct omap_mcbsp *mcbsp, u16 reg, u32 val)
> +{
> +	omap_mcbsp_cache[mcbsp->id][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) :
> +			omap_mcbsp_cache[mcbsp->id][reg / sizeof(u16)];
> +}

Then these can become something like this:

void omap_mcbsp_write(struct omap_mcbsp *mcbsp, u16 reg, u32 val)
{
	if (cpu_is_omap7xx())
		omap7xx_mcbsp_cache[mcbsp->id][reg / sizeof(u16)] = (u16)val;
	else if (cpu_is_omap15xx())
		omap15xx_mcbsp_cache[mcbsp->id][reg / sizeof(u16)] = (u16)val;
	else if (cpu_is_omap16xx())
		omap16xx_mcbsp_cache[mcbsp->id][reg / sizeof(u16)] = (u16)val;

	__raw_writew((u16)val, mcbsp->io_base + reg);
}
...

The code for processors that are not compiled in will get automatically
optimized out as the cpu_is_xxxx() becomes 0.

Regards,

Tony

Re: [RFC][RFT][PATCH 3/4 v5b] OMAP: McBSP: Introduce caching in register write operations

[Janusz Krzysztofik]

Monday 07 December 2009 18:55:35 Tony Lindgren napisał(a):
> Hi,
>
> This is pretty close :) Few more tweaks are still needed.
>
> * Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> [091205 05:46]:
> > Reserve static space for storing cached copies of McBSP register values.
> > Split omap_mcbsp_read()/omap_mcbsp_write() into separate functions for
> > OMAP1 and OMAP2/3/4, move them from plat-omap to mach-omap1 / mach-omap2
> > to be able to access the static cache.
> > Modify omap_msbcp_write() to update the cache with every register write
> > operation.
> > Modify omap_mcbsp_read() to support reading from cache or hardware.
> > Update MCBSP_READ/MCBSP_WRITE macros for modified function APIs.
> > Introduce a new macro that reads from the cache.
> >
> > Applies on top of patch 2 from this series:
> > [PATCH v3 2/4] OMAP: McBSP: Prepare register read/write macros API for
> > caching
> >
> > Tested on OMAP1510 based Amstrad Delta using linux-omap for-next,
> > commit 82f1d8f22f2c65e70206e40a6f17688bf64a892c.
> > Compile-tested with omap_generic_2420_defconfig, omap_3430sdp_defconfig.
> >
> > Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
> >
> > ---
> >
> > Tony,
> >
> > Since I was not sure what your primary concern was, I've prepared two
> > alternative versions of patch 3/4.
> > Please take this one as a base for further discussion if your primary
> > concern was storage class for the cache.
> > Otherwise, have a look at the other one (v5a), since this one has even
> > more ifdefs than before.
> >
> > 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-05 06:42:35.000000000 +0100
> > @@ -200,3 +200,26 @@ int __init omap1_mcbsp_init(void)
> >  }
> >
> >  arch_initcall(omap1_mcbsp_init);
> > +
> > +/* if adding more, put larger first */
> > +#if defined(CONFIG_ARCH_OMAP16XX)
> > +static u16 omap_mcbsp_cache[OMAP16XX_MCBSP_PDATA_SZ][OMAP_MCBSP_REG_COUNT];
> > +#elif defined(CONFIG_ARCH_OMAP15XX)
> > +static u16 omap_mcbsp_cache[OMAP15XX_MCBSP_PDATA_SZ][OMAP_MCBSP_REG_COUNT];
> > +#elif defined(CONFIG_ARCH_OMAP7XX)
> > +static u16 omap_mcbsp_cache[OMAP7XX_MCBSP_PDATA_SZ][OMAP_MCBSP_REG_COUNT];
> > +#else 
> > +#define omap_mcbsp_cache	NULL
> > +#endif
>
> As 15xx and 16xx can be compiled into the same kernel binary, we need to
> get rid of the ifdef elif else (untested):

> #ifdef(CONFIG_ARCH_OMAP7XX)
> static u16 omap7xx_mcbsp_cache[OMAP7XX_MCBSP_PDATA_SZ][OMAP7XX_MCBSP_REG_COUNT];
> #else 
> static 16 omap7xx_mcbsp_cache;

???

static u16 omap7xx_mcbsp_cache; ?

#define omap7xx_mcbsp_cache NULL ?

> #endif
>
> #if defined(CONFIG_ARCH_OMAP15XX)
> static u16 omap15xx_mcbsp_cache[OMAP15XX_MCBSP_PDATA_SZ][OMAP15XX_MCBSP_REG_COUNT];
> #else
> static 16 omap15xx_mcbsp_cache;
> #endif
>
> #if defined(CONFIG_ARCH_OMAP16XX)
> static u16 omap16xx_mcbsp_cache[OMAP16XX_MCBSP_PDATA_SZ][OMAP_16XXMCBSP_REG_COUNT];
> #else
> static 16 omap16xx_mcbsp_cache;
> #endif
> ...
>
> > +void omap_mcbsp_write(struct omap_mcbsp *mcbsp, u16 reg, u32 val)
> > +{
> > +	omap_mcbsp_cache[mcbsp->id][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) :
> > +			omap_mcbsp_cache[mcbsp->id][reg / sizeof(u16)];
> > +}
>
> Then these can become something like this:
>
> void omap_mcbsp_write(struct omap_mcbsp *mcbsp, u16 reg, u32 val)
> {
> 	if (cpu_is_omap7xx())
> 		omap7xx_mcbsp_cache[mcbsp->id][reg / sizeof(u16)] = (u16)val;
> 	else if (cpu_is_omap15xx())
> 		omap15xx_mcbsp_cache[mcbsp->id][reg / sizeof(u16)] = (u16)val;
> 	else if (cpu_is_omap16xx())
> 		omap16xx_mcbsp_cache[mcbsp->id][reg / sizeof(u16)] = (u16)val;
>
> 	__raw_writew((u16)val, mcbsp->io_base + reg);
> }
> ...
>
> The code for processors that are not compiled in will get automatically
> optimized out as the cpu_is_xxxx() becomes 0.

Tony,

For me, the fact that more than one processor type can be configured side by 
side it not enough reason here. With your code, if more then one processor 
type is configured, twice or tripple as much memory space will be devoted to  
two or three cache tables instead of one that can be reused easily, as it is 
with mine. Since you cannot run the same instance of the kernel on several 
machines simultaneously, only one of those two or three tables is required at 
runtime for storing data, so this looks like a waste of expensive memory 
unless some kind of runtime optimization that I am not familiar with can
happen here. I was even affraid before that one of objections against my idea 
of using the cache could be unnecessary waste of memory space.

Nevertheless, I'll do it your way. Maybe there are still some other reasons
not explicitly expressed yet.

I guess that omap2 part should follow the same pattern, shouldn't it?

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

[Tony Lindgren]

* Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> [091207 11:38]:
> Monday 07 December 2009 18:55:35 Tony Lindgren napisał(a):
> > Hi,
> >
> > This is pretty close :) Few more tweaks are still needed.
> >
> > * Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> [091205 05:46]:
> > > Reserve static space for storing cached copies of McBSP register values.
> > > Split omap_mcbsp_read()/omap_mcbsp_write() into separate functions for
> > > OMAP1 and OMAP2/3/4, move them from plat-omap to mach-omap1 / mach-omap2
> > > to be able to access the static cache.
> > > Modify omap_msbcp_write() to update the cache with every register write
> > > operation.
> > > Modify omap_mcbsp_read() to support reading from cache or hardware.
> > > Update MCBSP_READ/MCBSP_WRITE macros for modified function APIs.
> > > Introduce a new macro that reads from the cache.
> > >
> > > Applies on top of patch 2 from this series:
> > > [PATCH v3 2/4] OMAP: McBSP: Prepare register read/write macros API for
> > > caching
> > >
> > > Tested on OMAP1510 based Amstrad Delta using linux-omap for-next,
> > > commit 82f1d8f22f2c65e70206e40a6f17688bf64a892c.
> > > Compile-tested with omap_generic_2420_defconfig, omap_3430sdp_defconfig.
> > >
> > > Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
> > >
> > > ---
> > >
> > > Tony,
> > >
> > > Since I was not sure what your primary concern was, I've prepared two
> > > alternative versions of patch 3/4.
> > > Please take this one as a base for further discussion if your primary
> > > concern was storage class for the cache.
> > > Otherwise, have a look at the other one (v5a), since this one has even
> > > more ifdefs than before.
> > >
> > > 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-05 06:42:35.000000000 +0100
> > > @@ -200,3 +200,26 @@ int __init omap1_mcbsp_init(void)
> > >  }
> > >
> > >  arch_initcall(omap1_mcbsp_init);
> > > +
> > > +/* if adding more, put larger first */
> > > +#if defined(CONFIG_ARCH_OMAP16XX)
> > > +static u16 omap_mcbsp_cache[OMAP16XX_MCBSP_PDATA_SZ][OMAP_MCBSP_REG_COUNT];
> > > +#elif defined(CONFIG_ARCH_OMAP15XX)
> > > +static u16 omap_mcbsp_cache[OMAP15XX_MCBSP_PDATA_SZ][OMAP_MCBSP_REG_COUNT];
> > > +#elif defined(CONFIG_ARCH_OMAP7XX)
> > > +static u16 omap_mcbsp_cache[OMAP7XX_MCBSP_PDATA_SZ][OMAP_MCBSP_REG_COUNT];
> > > +#else 
> > > +#define omap_mcbsp_cache	NULL
> > > +#endif
> >
> > As 15xx and 16xx can be compiled into the same kernel binary, we need to
> > get rid of the ifdef elif else (untested):
> 
> > #ifdef(CONFIG_ARCH_OMAP7XX)
> > static u16 omap7xx_mcbsp_cache[OMAP7XX_MCBSP_PDATA_SZ][OMAP7XX_MCBSP_REG_COUNT];
> > #else 
> > static 16 omap7xx_mcbsp_cache;
> 
> ???
> 
> static u16 omap7xx_mcbsp_cache; ?
> 
> #define omap7xx_mcbsp_cache NULL ?

Yeah NULL is better here :)
 
> > #endif
> >
> > #if defined(CONFIG_ARCH_OMAP15XX)
> > static u16 omap15xx_mcbsp_cache[OMAP15XX_MCBSP_PDATA_SZ][OMAP15XX_MCBSP_REG_COUNT];
> > #else
> > static 16 omap15xx_mcbsp_cache;
> > #endif
> >
> > #if defined(CONFIG_ARCH_OMAP16XX)
> > static u16 omap16xx_mcbsp_cache[OMAP16XX_MCBSP_PDATA_SZ][OMAP_16XXMCBSP_REG_COUNT];
> > #else
> > static 16 omap16xx_mcbsp_cache;
> > #endif
> > ...
> >
> > > +void omap_mcbsp_write(struct omap_mcbsp *mcbsp, u16 reg, u32 val)
> > > +{
> > > +	omap_mcbsp_cache[mcbsp->id][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) :
> > > +			omap_mcbsp_cache[mcbsp->id][reg / sizeof(u16)];
> > > +}
> >
> > Then these can become something like this:
> >
> > void omap_mcbsp_write(struct omap_mcbsp *mcbsp, u16 reg, u32 val)
> > {
> > 	if (cpu_is_omap7xx())
> > 		omap7xx_mcbsp_cache[mcbsp->id][reg / sizeof(u16)] = (u16)val;
> > 	else if (cpu_is_omap15xx())
> > 		omap15xx_mcbsp_cache[mcbsp->id][reg / sizeof(u16)] = (u16)val;
> > 	else if (cpu_is_omap16xx())
> > 		omap16xx_mcbsp_cache[mcbsp->id][reg / sizeof(u16)] = (u16)val;
> >
> > 	__raw_writew((u16)val, mcbsp->io_base + reg);
> > }
> > ...
> >
> > The code for processors that are not compiled in will get automatically
> > optimized out as the cpu_is_xxxx() becomes 0.
> 
> Tony,
> 
> For me, the fact that more than one processor type can be configured side by 
> side it not enough reason here. With your code, if more then one processor 
> type is configured, twice or tripple as much memory space will be devoted to  
> two or three cache tables instead of one that can be reused easily, as it is 
> with mine. Since you cannot run the same instance of the kernel on several 
> machines simultaneously, only one of those two or three tables is required at 
> runtime for storing data, so this looks like a waste of expensive memory 
> unless some kind of runtime optimization that I am not familiar with can
> happen here. I was even affraid before that one of objections against my idea 
> of using the cache could be unnecessary waste of memory space.

Well if you want to optimize it out further, how about just kzalloc it
during init? Then you have just one copy that gets set the right size
depending on what you boot.
 
> Nevertheless, I'll do it your way. Maybe there are still some other reasons
> not explicitly expressed yet.
> 
> I guess that omap2 part should follow the same pattern, shouldn't it?

Yeas please.

Regards,

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

[Jarkko Nikula]

Hi, sorry not commenting for few days.

On Mon, 7 Dec 2009 13:06:31 -0800
Tony Lindgren <tony@atomide.com> wrote:

> > For me, the fact that more than one processor type can be configured side by 
> > side it not enough reason here. With your code, if more then one processor 
> > type is configured, twice or tripple as much memory space will be devoted to  
> > two or three cache tables instead of one that can be reused easily, as it is 
> > with mine. Since you cannot run the same instance of the kernel on several 
> > machines simultaneously, only one of those two or three tables is required at 
> > runtime for storing data, so this looks like a waste of expensive memory 
> > unless some kind of runtime optimization that I am not familiar with can
> > happen here. I was even affraid before that one of objections against my idea 
> > of using the cache could be unnecessary waste of memory space.
> 
> Well if you want to optimize it out further, how about just kzalloc it
> during init? Then you have just one copy that gets set the right size
> depending on what you boot.
>  
> > Nevertheless, I'll do it your way. Maybe there are still some other reasons
> > not explicitly expressed yet.
> > 
> > I guess that omap2 part should follow the same pattern, shouldn't it?
> 
How about declaring the reg_cache as a void pointer in struct
omap_mcbsp and allocate the cache and its size in omap_mcbsp_request
according to omap type? Read and write functions would access the
*reg_cache either as 16-bit or 32-bit table depending on omap.

No ifdefs, no unused cache tables in multi-omap binary and cache tables
are allocated only for used ports. I don't think there is need to
preserve cache content over omap_mcbsp_free and new omap_mcbsp_request?


-- 
Jarkko

[RFC][RFT][PATCH 3/4 v6] OMAP: McBSP: Introduce caching in register write operations

[Janusz Krzysztofik]

Allocate space for storing cached copies of McBSP register values.
Modify omap_msbcp_write() to update the cache with every register write
operation.
Modify omap_mcbsp_read() to support reading from cache or hardware.
Update MCBSP_READ/MCBSP_WRITE macros for modified function APIs.
Introduce a new macro that reads from the cache.

Applies on top of patch 2 from this series:
[PATCH v3 2/4] OMAP: McBSP: Prepare register read/write macros API for caching

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

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

---
Monday 07 December 2009 22:06:31 Tony Lindgren wrote:
>
> Well if you want to optimize it out further, how about just kzalloc it
> during init? Then you have just one copy that gets set the right size
> depending on what you boot.

Tony,
In v4 of the patch, a single table was actually kzalloced during init as a
part of struct omap_mcbsp for each McBSP port configured. Since you objected
it, and the reason was not quite obvious to me, I submitted two alternate
versions: 5a, with ifdef...else cleanup, maybe still not good enough but
keeping that dynamic allocation, and 5b, using static storage instead, since I
could suspect you may like it better. Now it appears not quite true.

Anyway, sorry for all that long discussion, but it's really important for me
to understand precisely what your concearns, goals and preferences are when
you request changes before I'm able to implement them correctly, ie. in an way
acceptable for you. Otherwise, it all turns into a kind of quiz.

Tuesday 08 December 2009 08:35:21 Jarkko Nikula wrote:
>
> How about declaring the reg_cache as a void pointer in struct
> omap_mcbsp and allocate the cache and its size in omap_mcbsp_request
> according to omap type? Read and write functions would access the
> *reg_cache either as 16-bit or 32-bit table depending on omap.
>
> No ifdefs, no unused cache tables in multi-omap binary and cache tables
> are allocated only for used ports. I don't think there is need to
> preserve cache content over omap_mcbsp_free and new omap_mcbsp_request?

Jarkko,
I think this is the best solution. I had almost already sent a modified
version last night, with two pointers, u16 *omap1_cache and u32 *omap2_cache,
inside omap_mcbsp structre, but now I think that if using casts inside
omap_mcbsp_read()/_write() is not objected by anyone, the version below,
based on your idea, seems the most clean one.

I was not sure about deferring memory allocation until omap_mcbsp_request(),
but if you say it should be ok, I'm all in favour of that.

Note: 2420 handling may look weird, but I just tried to follow exactly what I
was able to learn about it by reading the code: register address spacing
were 32-bit, register read/write operations - 16-bit, since u16 reg_cache
elements.

Thanks,
Janusz

 arch/arm/plat-omap/include/plat/mcbsp.h |    9 ++++++
 arch/arm/plat-omap/mcbsp.c              |   66 +++++++++++++++++++++++++++++++++++++++++----------
 2 files changed, 63 insertions(+), 12 deletions(-)

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-08 14:26:42.000000000 +0100
@@ -157,6 +157,14 @@
 
 #endif
 
+#define OMAP7XX_MCBSP_REG_NUM	(OMAP_MCBSP_REG_XCERH / sizeof(u16) + 1)
+#define OMAP15XX_MCBSP_REG_NUM	(OMAP_MCBSP_REG_XCERH / sizeof(u16) + 1)
+#define OMAP16XX_MCBSP_REG_NUM	(OMAP_MCBSP_REG_XCERH / sizeof(u16) + 1)
+
+#define OMAP24XX_MCBSP_REG_NUM	(OMAP_MCBSP_REG_RCCR / sizeof(u32) + 1)
+#define OMAP34XX_MCBSP_REG_NUM	(OMAP_MCBSP_REG_RCCR / sizeof(u32) + 1)
+#define OMAP44XX_MCBSP_REG_NUM	(OMAP_MCBSP_REG_RCCR / sizeof(u32) + 1)
+
 /************************** McBSP SPCR1 bit definitions ***********************/
 #define RRST			0x0001
 #define RRDY			0x0002
@@ -415,6 +423,7 @@ 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;
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-07 22:43:56.000000000 +0100
+++ git/arch/arm/plat-omap/mcbsp.c	2009-12-08 14:38:38.000000000 +0100
@@ -30,26 +30,40 @@
 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);
-	else
-		__raw_writel(val, io_base + reg);
+	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(void __iomem *io_base, 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(io_base + reg);
-	else
-		return __raw_readl(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->io_base, OMAP_MCBSP_REG_##reg)
+		omap_mcbsp_read(mcbsp, OMAP_MCBSP_REG_##reg, 0)
 #define MCBSP_WRITE(mcbsp, reg, val) \
-		omap_mcbsp_write(mcbsp->io_base, OMAP_MCBSP_REG_##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,31 @@ int omap_mcbsp_request(unsigned int id)
 	}
 	mcbsp = id_to_mcbsp_ptr(id);
 
+	if (cpu_is_omap7xx()) {
+		mcbsp->reg_cache = kzalloc(sizeof(u16) *
+				OMAP7XX_MCBSP_REG_NUM, GFP_KERNEL);
+	} else if (cpu_is_omap15xx()) {
+		mcbsp->reg_cache = kzalloc(sizeof(u16) *
+				OMAP15XX_MCBSP_REG_NUM, GFP_KERNEL);
+	} else if (cpu_is_omap16xx()) {
+		mcbsp->reg_cache = kzalloc(sizeof(u16) *
+				OMAP16XX_MCBSP_REG_NUM, GFP_KERNEL);
+	} else if (cpu_is_omap2420()) {
+		mcbsp->reg_cache = kzalloc(sizeof(u16) *
+				OMAP24XX_MCBSP_REG_NUM, GFP_KERNEL);
+	} else if (cpu_is_omap2430()) {
+		mcbsp->reg_cache = kzalloc(sizeof(u32) *
+				OMAP24XX_MCBSP_REG_NUM, GFP_KERNEL);
+	} else if (cpu_is_omap34xx()) {
+		mcbsp->reg_cache = kzalloc(sizeof(u32) *
+				OMAP34XX_MCBSP_REG_NUM, GFP_KERNEL);
+	} else if (cpu_is_omap44xx()) {
+		mcbsp->reg_cache = kzalloc(sizeof(u32) *
+				OMAP44XX_MCBSP_REG_NUM, 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 +520,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: [RFC][RFT][PATCH 3/4 v6] OMAP: McBSP: Introduce caching in register write operations

[Tony Lindgren]

Hi,

Almost there :) Just one more comment below.

* Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> [091208 08:06]:
> Allocate space for storing cached copies of McBSP register values.
> Modify omap_msbcp_write() to update the cache with every register write
> operation.
> Modify omap_mcbsp_read() to support reading from cache or hardware.
> Update MCBSP_READ/MCBSP_WRITE macros for modified function APIs.
> Introduce a new macro that reads from the cache.
> 
> Applies on top of patch 2 from this series:
> [PATCH v3 2/4] OMAP: McBSP: Prepare register read/write macros API for caching
> 
> Tested on OMAP1510 based Amstrad Delta using linux-omap for-next,
> commit 82f1d8f22f2c65e70206e40a6f17688bf64a892c.
> Compile-tested with omap_generic_2420_defconfig, omap_3430sdp_defconfig.
> 
> Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
> 
> ---
> Monday 07 December 2009 22:06:31 Tony Lindgren wrote:
> >
> > Well if you want to optimize it out further, how about just kzalloc it
> > during init? Then you have just one copy that gets set the right size
> > depending on what you boot.
> 
> Tony,
> In v4 of the patch, a single table was actually kzalloced during init as a
> part of struct omap_mcbsp for each McBSP port configured. Since you objected
> it, and the reason was not quite obvious to me, I submitted two alternate
> versions: 5a, with ifdef...else cleanup, maybe still not good enough but
> keeping that dynamic allocation, and 5b, using static storage instead, since I
> could suspect you may like it better. Now it appears not quite true.
> 
> Anyway, sorry for all that long discussion, but it's really important for me
> to understand precisely what your concearns, goals and preferences are when
> you request changes before I'm able to implement them correctly, ie. in an way
> acceptable for you. Otherwise, it all turns into a kind of quiz.

Yeah, thanks for considering all the issues, hopefully it will make
things easier for us in the long run.

 
> Tuesday 08 December 2009 08:35:21 Jarkko Nikula wrote:
> >
> > How about declaring the reg_cache as a void pointer in struct
> > omap_mcbsp and allocate the cache and its size in omap_mcbsp_request
> > according to omap type? Read and write functions would access the
> > *reg_cache either as 16-bit or 32-bit table depending on omap.
> >
> > No ifdefs, no unused cache tables in multi-omap binary and cache tables
> > are allocated only for used ports. I don't think there is need to
> > preserve cache content over omap_mcbsp_free and new omap_mcbsp_request?
> 
> Jarkko,
> I think this is the best solution. I had almost already sent a modified
> version last night, with two pointers, u16 *omap1_cache and u32 *omap2_cache,
> inside omap_mcbsp structre, but now I think that if using casts inside
> omap_mcbsp_read()/_write() is not objected by anyone, the version below,
> based on your idea, seems the most clean one.
> 
> I was not sure about deferring memory allocation until omap_mcbsp_request(),
> but if you say it should be ok, I'm all in favour of that.
> 
> Note: 2420 handling may look weird, but I just tried to follow exactly what I
> was able to learn about it by reading the code: register address spacing
> were 32-bit, register read/write operations - 16-bit, since u16 reg_cache
> elements.
> 
> Thanks,
> Janusz
> 
>  arch/arm/plat-omap/include/plat/mcbsp.h |    9 ++++++
>  arch/arm/plat-omap/mcbsp.c              |   66 +++++++++++++++++++++++++++++++++++++++++----------
>  2 files changed, 63 insertions(+), 12 deletions(-)
> 
> 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-08 14:26:42.000000000 +0100
> @@ -157,6 +157,14 @@
>  
>  #endif
>  
> +#define OMAP7XX_MCBSP_REG_NUM	(OMAP_MCBSP_REG_XCERH / sizeof(u16) + 1)
> +#define OMAP15XX_MCBSP_REG_NUM	(OMAP_MCBSP_REG_XCERH / sizeof(u16) + 1)
> +#define OMAP16XX_MCBSP_REG_NUM	(OMAP_MCBSP_REG_XCERH / sizeof(u16) + 1)
> +
> +#define OMAP24XX_MCBSP_REG_NUM	(OMAP_MCBSP_REG_RCCR / sizeof(u32) + 1)
> +#define OMAP34XX_MCBSP_REG_NUM	(OMAP_MCBSP_REG_RCCR / sizeof(u32) + 1)
> +#define OMAP44XX_MCBSP_REG_NUM	(OMAP_MCBSP_REG_RCCR / sizeof(u32) + 1)
> +
>  /************************** McBSP SPCR1 bit definitions ***********************/
>  #define RRST			0x0001
>  #define RRDY			0x0002
> @@ -415,6 +423,7 @@ 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;
> 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-07 22:43:56.000000000 +0100
> +++ git/arch/arm/plat-omap/mcbsp.c	2009-12-08 14:38:38.000000000 +0100
> @@ -30,26 +30,40 @@
>  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);
> -	else
> -		__raw_writel(val, io_base + reg);
> +	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(void __iomem *io_base, 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(io_base + reg);
> -	else
> -		return __raw_readl(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->io_base, OMAP_MCBSP_REG_##reg)
> +		omap_mcbsp_read(mcbsp, OMAP_MCBSP_REG_##reg, 0)
>  #define MCBSP_WRITE(mcbsp, reg, val) \
> -		omap_mcbsp_write(mcbsp->io_base, OMAP_MCBSP_REG_##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,31 @@ int omap_mcbsp_request(unsigned int id)
>  	}
>  	mcbsp = id_to_mcbsp_ptr(id);
>  
> +	if (cpu_is_omap7xx()) {
> +		mcbsp->reg_cache = kzalloc(sizeof(u16) *
> +				OMAP7XX_MCBSP_REG_NUM, GFP_KERNEL);
> +	} else if (cpu_is_omap15xx()) {
> +		mcbsp->reg_cache = kzalloc(sizeof(u16) *
> +				OMAP15XX_MCBSP_REG_NUM, GFP_KERNEL);
> +	} else if (cpu_is_omap16xx()) {
> +		mcbsp->reg_cache = kzalloc(sizeof(u16) *
> +				OMAP16XX_MCBSP_REG_NUM, GFP_KERNEL);
> +	} else if (cpu_is_omap2420()) {
> +		mcbsp->reg_cache = kzalloc(sizeof(u16) *
> +				OMAP24XX_MCBSP_REG_NUM, GFP_KERNEL);
> +	} else if (cpu_is_omap2430()) {
> +		mcbsp->reg_cache = kzalloc(sizeof(u32) *
> +				OMAP24XX_MCBSP_REG_NUM, GFP_KERNEL);
> +	} else if (cpu_is_omap34xx()) {
> +		mcbsp->reg_cache = kzalloc(sizeof(u32) *
> +				OMAP34XX_MCBSP_REG_NUM, GFP_KERNEL);
> +	} else if (cpu_is_omap44xx()) {
> +		mcbsp->reg_cache = kzalloc(sizeof(u32) *
> +				OMAP44XX_MCBSP_REG_NUM, GFP_KERNEL);
> +	}

How about just set the cache size above based on the processor,
then do kzalloc here:

mcbsp->reg_cache = kzalloc(size, GFP_KERNEL);
> +	if (!mcbsp->reg_cache)
> +		return -ENOMEM;
> +

That way the kzalloc and error checking are in the same place.

Regards,

Tony

Re: [RFC][RFT][PATCH 3/4 v6] OMAP: McBSP: Introduce caching in register write operations

[Tony Lindgren]

* Tony Lindgren <tony@atomide.com> [091208 08:39]:
> > @@ -391,6 +405,31 @@ int omap_mcbsp_request(unsigned int id)
> >  	}
> >  	mcbsp = id_to_mcbsp_ptr(id);
> >  
> > +	if (cpu_is_omap7xx()) {
> > +		mcbsp->reg_cache = kzalloc(sizeof(u16) *
> > +				OMAP7XX_MCBSP_REG_NUM, GFP_KERNEL);
> > +	} else if (cpu_is_omap15xx()) {
> > +		mcbsp->reg_cache = kzalloc(sizeof(u16) *
> > +				OMAP15XX_MCBSP_REG_NUM, GFP_KERNEL);
> > +	} else if (cpu_is_omap16xx()) {
> > +		mcbsp->reg_cache = kzalloc(sizeof(u16) *
> > +				OMAP16XX_MCBSP_REG_NUM, GFP_KERNEL);
> > +	} else if (cpu_is_omap2420()) {
> > +		mcbsp->reg_cache = kzalloc(sizeof(u16) *
> > +				OMAP24XX_MCBSP_REG_NUM, GFP_KERNEL);
> > +	} else if (cpu_is_omap2430()) {
> > +		mcbsp->reg_cache = kzalloc(sizeof(u32) *
> > +				OMAP24XX_MCBSP_REG_NUM, GFP_KERNEL);
> > +	} else if (cpu_is_omap34xx()) {
> > +		mcbsp->reg_cache = kzalloc(sizeof(u32) *
> > +				OMAP34XX_MCBSP_REG_NUM, GFP_KERNEL);
> > +	} else if (cpu_is_omap44xx()) {
> > +		mcbsp->reg_cache = kzalloc(sizeof(u32) *
> > +				OMAP44XX_MCBSP_REG_NUM, GFP_KERNEL);
> > +	}
> 
> How about just set the cache size above based on the processor,
> then do kzalloc here:
> 
> mcbsp->reg_cache = kzalloc(size, GFP_KERNEL);
> > +	if (!mcbsp->reg_cache)
> > +		return -ENOMEM;
> > +
> 
> That way the kzalloc and error checking are in the same place.

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.

Regards,

Tony

Re: [RFC][RFT][PATCH 3/4 v6] OMAP: McBSP: Introduce caching in register write operations

[Janusz Krzysztofik]

Tuesday 08 December 2009 17:59:31 Tony Lindgren napisał(a):
> * Tony Lindgren <tony@atomide.com> [091208 08:39]:
> >
> > How about just set the cache size above based on the processor,
> > then do kzalloc here:
> >
> > mcbsp->reg_cache = kzalloc(size, GFP_KERNEL);
> >
> > > +	if (!mcbsp->reg_cache)
> > > +		return -ENOMEM;
> > > +
> >
> > That way the kzalloc and error checking are in the same place.
>
> 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?

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

[Tony Lindgren]

* Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> [091208 11:45]:
> Tuesday 08 December 2009 17:59:31 Tony Lindgren napisał(a):
> > * Tony Lindgren <tony@atomide.com> [091208 08:39]:
> > >
> > > How about just set the cache size above based on the processor,
> > > then do kzalloc here:
> > >
> > > mcbsp->reg_cache = kzalloc(size, GFP_KERNEL);
> > >
> > > > +	if (!mcbsp->reg_cache)
> > > > +		return -ENOMEM;
> > > > +
> > >
> > > That way the kzalloc and error checking are in the same place.
> >
> > 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!

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

[Tony Lindgren]

* 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):
> > > * Tony Lindgren <tony@atomide.com> [091208 08:39]:
> > > >
> > > > How about just set the cache size above based on the processor,
> > > > then do kzalloc here:
> > > >
> > > > mcbsp->reg_cache = kzalloc(size, GFP_KERNEL);
> > > >
> > > > > +	if (!mcbsp->reg_cache)
> > > > > +		return -ENOMEM;
> > > > > +
> > > >
> > > > That way the kzalloc and error checking are in the same place.
> > >
> > > 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
--
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