* [RFC][PATCH] ARM: OMAP: McBSP: Use register cache
@ 2009-08-12 10:39 Janusz Krzysztofik
2009-08-28 11:26 ` Janusz Krzysztofik
2009-11-14 2:06 ` Janusz Krzysztofik
0 siblings, 2 replies; 15+ messages in thread
From: Janusz Krzysztofik @ 2009-08-12 10:39 UTC (permalink / raw)
To: Tony Lindgren, Jarkko Nikula, Peter Ujfalusi
Cc: alsa-devel@alsa-project.org, linux-omap@vger.kernel.org,
linux-arm-kernel@lists.arm.linux.org.uk,
linux-kernel@vger.kernel.org
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 (buggy?)
hardware. Before that, I could suspect that values read back from McBSP
registers before updating them happened to be errornous.
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.
Created against linux-2.6.31-rc5
Tested on Amstrad Delta
Signed-off-by: Janusz Krzysztofik
---
--- linux-2.6.31-rc5/arch/arm/plat-omap/include/mach/mcbsp.h.orig 2009-08-11 23:43:25.000000000 +0200
+++ linux-2.6.31-rc5/arch/arm/plat-omap/include/mach/mcbsp.h 2009-08-11 23:45:46.000000000 +0200
@@ -377,6 +377,8 @@ struct omap_mcbsp {
struct omap_mcbsp_platform_data *pdata;
struct clk *iclk;
struct clk *fclk;
+
+ struct omap_mcbsp_reg_cfg reg_cache;
};
extern struct omap_mcbsp **mcbsp_ptr;
extern int omap_mcbsp_count;
--- linux-2.6.31-rc5/arch/arm/plat-omap/mcbsp.c.orig 2009-08-11 23:43:25.000000000 +0200
+++ linux-2.6.31-rc5/arch/arm/plat-omap/mcbsp.c 2009-08-11 23:45:35.000000000 +0200
@@ -91,6 +91,7 @@ static void omap_mcbsp_dump_reg(u8 id)
static irqreturn_t omap_mcbsp_tx_irq_handler(int irq, void *dev_id)
{
struct omap_mcbsp *mcbsp_tx = dev_id;
+ struct omap_mcbsp_reg_cfg *reg_cache = &mcbsp_tx->reg_cache;
u16 irqst_spcr2;
irqst_spcr2 = OMAP_MCBSP_READ(mcbsp_tx->io_base, SPCR2);
@@ -101,7 +102,7 @@ static irqreturn_t omap_mcbsp_tx_irq_han
irqst_spcr2);
/* Writing zero to XSYNC_ERR clears the IRQ */
OMAP_MCBSP_WRITE(mcbsp_tx->io_base, SPCR2,
- irqst_spcr2 & ~(XSYNC_ERR));
+ reg_cache->spcr2 &= ~(XSYNC_ERR));
} else {
complete(&mcbsp_tx->tx_irq_completion);
}
@@ -112,6 +113,7 @@ static irqreturn_t omap_mcbsp_tx_irq_han
static irqreturn_t omap_mcbsp_rx_irq_handler(int irq, void *dev_id)
{
struct omap_mcbsp *mcbsp_rx = dev_id;
+ struct omap_mcbsp_reg_cfg *reg_cache = &mcbsp_rx->reg_cache;
u16 irqst_spcr1;
irqst_spcr1 = OMAP_MCBSP_READ(mcbsp_rx->io_base, SPCR1);
@@ -122,7 +124,7 @@ static irqreturn_t omap_mcbsp_rx_irq_han
irqst_spcr1);
/* Writing zero to RSYNC_ERR clears the IRQ */
OMAP_MCBSP_WRITE(mcbsp_rx->io_base, SPCR1,
- irqst_spcr1 & ~(RSYNC_ERR));
+ reg_cache->spcr1 &= ~(RSYNC_ERR));
} else {
complete(&mcbsp_rx->tx_irq_completion);
}
@@ -167,6 +169,7 @@ 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;
+ struct omap_mcbsp_reg_cfg *reg_cache;
void __iomem *io_base;
if (!omap_mcbsp_check_valid_id(id)) {
@@ -174,26 +177,27 @@ void omap_mcbsp_config(unsigned int id,
return;
}
mcbsp = id_to_mcbsp_ptr(id);
+ reg_cache = &mcbsp->reg_cache;
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);
+ OMAP_MCBSP_WRITE(io_base, SPCR2, reg_cache->spcr2 = config->spcr2);
+ OMAP_MCBSP_WRITE(io_base, SPCR1, reg_cache->spcr1 = config->spcr1);
+ OMAP_MCBSP_WRITE(io_base, RCR2, reg_cache->rcr2 = config->rcr2);
+ OMAP_MCBSP_WRITE(io_base, RCR1, reg_cache->rcr1 = config->rcr1);
+ OMAP_MCBSP_WRITE(io_base, XCR2, reg_cache->xcr2 = config->xcr2);
+ OMAP_MCBSP_WRITE(io_base, XCR1, reg_cache->xcr1 = config->xcr1);
+ OMAP_MCBSP_WRITE(io_base, SRGR2, reg_cache->srgr2 = config->srgr2);
+ OMAP_MCBSP_WRITE(io_base, SRGR1, reg_cache->srgr1 = config->srgr1);
+ OMAP_MCBSP_WRITE(io_base, MCR2, reg_cache->mcr2 = config->mcr2);
+ OMAP_MCBSP_WRITE(io_base, MCR1, reg_cache->mcr1 = config->mcr1);
+ OMAP_MCBSP_WRITE(io_base, PCR0, reg_cache->pcr0 = config->pcr0);
if (cpu_is_omap2430() || cpu_is_omap34xx()) {
- OMAP_MCBSP_WRITE(io_base, XCCR, config->xccr);
- OMAP_MCBSP_WRITE(io_base, RCCR, config->rccr);
+ OMAP_MCBSP_WRITE(io_base, XCCR, reg_cache->xccr = config->xccr);
+ OMAP_MCBSP_WRITE(io_base, RCCR, reg_cache->rccr = config->rccr);
}
}
EXPORT_SYMBOL(omap_mcbsp_config);
@@ -232,6 +236,7 @@ EXPORT_SYMBOL(omap_mcbsp_set_io_type);
int omap_mcbsp_request(unsigned int id)
{
struct omap_mcbsp *mcbsp;
+ struct omap_mcbsp_reg_cfg *reg_cache;
int err;
if (!omap_mcbsp_check_valid_id(id)) {
@@ -239,6 +244,7 @@ int omap_mcbsp_request(unsigned int id)
return -ENODEV;
}
mcbsp = id_to_mcbsp_ptr(id);
+ reg_cache = &mcbsp->reg_cache;
spin_lock(&mcbsp->lock);
if (!mcbsp->free) {
@@ -261,8 +267,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);
+ OMAP_MCBSP_WRITE(mcbsp->io_base, SPCR1, reg_cache->spcr1 = 0);
+ OMAP_MCBSP_WRITE(mcbsp->io_base, SPCR2, reg_cache->spcr2 = 0);
if (mcbsp->io_type == OMAP_MCBSP_IRQ_IO) {
/* We need to get IRQs here */
@@ -335,42 +341,38 @@ EXPORT_SYMBOL(omap_mcbsp_free);
void omap_mcbsp_start(unsigned int id, int tx, int rx)
{
struct omap_mcbsp *mcbsp;
+ struct omap_mcbsp_reg_cfg *reg_cache;
void __iomem *io_base;
int idle;
- u16 w;
if (!omap_mcbsp_check_valid_id(id)) {
printk(KERN_ERR "%s: Invalid id (%d)\n", __func__, id + 1);
return;
}
mcbsp = id_to_mcbsp_ptr(id);
+ reg_cache = &mcbsp->reg_cache;
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 = (reg_cache->rcr1 >> 5) & 0x7;
+ mcbsp->tx_word_length = (reg_cache->xcr1 >> 5) & 0x7;
- idle = !((OMAP_MCBSP_READ(io_base, SPCR2) |
- OMAP_MCBSP_READ(io_base, SPCR1)) & 1);
+ idle = !((reg_cache->spcr2 | reg_cache->spcr1) & 1);
if (idle) {
/* Start the sample generator */
- w = OMAP_MCBSP_READ(io_base, SPCR2);
- OMAP_MCBSP_WRITE(io_base, SPCR2, w | (1 << 6));
+ OMAP_MCBSP_WRITE(io_base, SPCR2, reg_cache->spcr2 |= (1 << 6));
}
/* Enable transmitter and receiver */
- w = OMAP_MCBSP_READ(io_base, SPCR2);
- OMAP_MCBSP_WRITE(io_base, SPCR2, w | (tx & 1));
+ OMAP_MCBSP_WRITE(io_base, SPCR2, reg_cache->spcr2 |= (tx & 1));
- w = OMAP_MCBSP_READ(io_base, SPCR1);
- OMAP_MCBSP_WRITE(io_base, SPCR1, w | (rx & 1));
+ OMAP_MCBSP_WRITE(io_base, SPCR1, reg_cache->spcr1 |= (rx & 1));
udelay(100);
if (idle) {
/* Start frame sync */
- w = OMAP_MCBSP_READ(io_base, SPCR2);
- OMAP_MCBSP_WRITE(io_base, SPCR2, w | (1 << 7));
+ OMAP_MCBSP_WRITE(io_base, SPCR2, reg_cache->spcr2 |= (1 << 7));
}
/* Dump McBSP Regs */
@@ -381,9 +383,9 @@ EXPORT_SYMBOL(omap_mcbsp_start);
void omap_mcbsp_stop(unsigned int id, int tx, int rx)
{
struct omap_mcbsp *mcbsp;
+ struct omap_mcbsp_reg_cfg *reg_cache;
void __iomem *io_base;
int idle;
- u16 w;
if (!omap_mcbsp_check_valid_id(id)) {
printk(KERN_ERR "%s: Invalid id (%d)\n", __func__, id + 1);
@@ -391,23 +393,20 @@ void omap_mcbsp_stop(unsigned int id, in
}
mcbsp = id_to_mcbsp_ptr(id);
+ reg_cache = &mcbsp->reg_cache;
io_base = mcbsp->io_base;
/* Reset transmitter */
- w = OMAP_MCBSP_READ(io_base, SPCR2);
- OMAP_MCBSP_WRITE(io_base, SPCR2, w & ~(tx & 1));
+ OMAP_MCBSP_WRITE(io_base, SPCR2, reg_cache->spcr2 &= ~(tx & 1));
/* Reset receiver */
- w = OMAP_MCBSP_READ(io_base, SPCR1);
- OMAP_MCBSP_WRITE(io_base, SPCR1, w & ~(rx & 1));
+ OMAP_MCBSP_WRITE(io_base, SPCR1, reg_cache->spcr1 &= ~(rx & 1));
- idle = !((OMAP_MCBSP_READ(io_base, SPCR2) |
- OMAP_MCBSP_READ(io_base, SPCR1)) & 1);
+ idle = !((reg_cache->spcr2 | reg_cache->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));
+ OMAP_MCBSP_WRITE(io_base, SPCR2, reg_cache->spcr2 &= ~(1 << 6));
}
}
EXPORT_SYMBOL(omap_mcbsp_stop);
@@ -557,6 +556,7 @@ EXPORT_SYMBOL(omap_mcbsp_recv_word);
int omap_mcbsp_spi_master_xmit_word_poll(unsigned int id, u32 word)
{
struct omap_mcbsp *mcbsp;
+ struct omap_mcbsp_reg_cfg *reg_cache;
void __iomem *io_base;
omap_mcbsp_word_length tx_word_length;
omap_mcbsp_word_length rx_word_length;
@@ -567,6 +567,7 @@ int omap_mcbsp_spi_master_xmit_word_poll
return -ENODEV;
}
mcbsp = id_to_mcbsp_ptr(id);
+ reg_cache = &mcbsp->reg_cache;
io_base = mcbsp->io_base;
tx_word_length = mcbsp->tx_word_length;
rx_word_length = mcbsp->rx_word_length;
@@ -580,9 +581,11 @@ int omap_mcbsp_spi_master_xmit_word_poll
spcr2 = OMAP_MCBSP_READ(io_base, SPCR2);
if (attempts++ > 1000) {
/* We must reset the transmitter */
- OMAP_MCBSP_WRITE(io_base, SPCR2, spcr2 & (~XRST));
+ OMAP_MCBSP_WRITE(io_base, SPCR2,
+ reg_cache->spcr2 &= (~XRST));
udelay(10);
- OMAP_MCBSP_WRITE(io_base, SPCR2, spcr2 | XRST);
+ OMAP_MCBSP_WRITE(io_base, SPCR2,
+ reg_cache->spcr2 |= XRST);
udelay(10);
dev_err(mcbsp->dev, "McBSP%d transmitter not "
"ready\n", mcbsp->id);
@@ -601,9 +604,11 @@ int omap_mcbsp_spi_master_xmit_word_poll
spcr1 = OMAP_MCBSP_READ(io_base, SPCR1);
if (attempts++ > 1000) {
/* We must reset the receiver */
- OMAP_MCBSP_WRITE(io_base, SPCR1, spcr1 & (~RRST));
+ OMAP_MCBSP_WRITE(io_base, SPCR1,
+ reg_cache->spcr1 &= (~RRST));
udelay(10);
- OMAP_MCBSP_WRITE(io_base, SPCR1, spcr1 | RRST);
+ OMAP_MCBSP_WRITE(io_base, SPCR1,
+ reg_cache->spcr1 |= RRST);
udelay(10);
dev_err(mcbsp->dev, "McBSP%d receiver not "
"ready\n", mcbsp->id);
@@ -623,6 +628,7 @@ EXPORT_SYMBOL(omap_mcbsp_spi_master_xmit
int omap_mcbsp_spi_master_recv_word_poll(unsigned int id, u32 *word)
{
struct omap_mcbsp *mcbsp;
+ struct omap_mcbsp_reg_cfg *reg_cache;
u32 clock_word = 0;
void __iomem *io_base;
omap_mcbsp_word_length tx_word_length;
@@ -635,6 +641,7 @@ int omap_mcbsp_spi_master_recv_word_poll
}
mcbsp = id_to_mcbsp_ptr(id);
+ reg_cache = &mcbsp->reg_cache;
io_base = mcbsp->io_base;
tx_word_length = mcbsp->tx_word_length;
@@ -649,9 +656,11 @@ int omap_mcbsp_spi_master_recv_word_poll
spcr2 = OMAP_MCBSP_READ(io_base, SPCR2);
if (attempts++ > 1000) {
/* We must reset the transmitter */
- OMAP_MCBSP_WRITE(io_base, SPCR2, spcr2 & (~XRST));
+ OMAP_MCBSP_WRITE(io_base, SPCR2,
+ reg_cache->spcr2 &= (~XRST));
udelay(10);
- OMAP_MCBSP_WRITE(io_base, SPCR2, spcr2 | XRST);
+ OMAP_MCBSP_WRITE(io_base, SPCR2,
+ reg_cache->spcr2 |= XRST);
udelay(10);
dev_err(mcbsp->dev, "McBSP%d transmitter not "
"ready\n", mcbsp->id);
@@ -670,9 +679,11 @@ int omap_mcbsp_spi_master_recv_word_poll
spcr1 = OMAP_MCBSP_READ(io_base, SPCR1);
if (attempts++ > 1000) {
/* We must reset the receiver */
- OMAP_MCBSP_WRITE(io_base, SPCR1, spcr1 & (~RRST));
+ OMAP_MCBSP_WRITE(io_base, SPCR1,
+ reg_cache->spcr1 &= (~RRST));
udelay(10);
- OMAP_MCBSP_WRITE(io_base, SPCR1, spcr1 | RRST);
+ OMAP_MCBSP_WRITE(io_base, SPCR1,
+ reg_cache->spcr1 |= RRST);
udelay(10);
dev_err(mcbsp->dev, "McBSP%d receiver not "
"ready\n", mcbsp->id);
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC][PATCH] ARM: OMAP: McBSP: Use register cache
2009-08-12 10:39 [RFC][PATCH] ARM: OMAP: McBSP: Use register cache Janusz Krzysztofik
@ 2009-08-28 11:26 ` Janusz Krzysztofik
2009-11-14 2:06 ` Janusz Krzysztofik
1 sibling, 0 replies; 15+ messages in thread
From: Janusz Krzysztofik @ 2009-08-28 11:26 UTC (permalink / raw)
To: Tony Lindgren, Jarkko Nikula, Peter Ujfalusi
Cc: alsa-devel@alsa-project.org, linux-omap@vger.kernel.org,
Mark Brown, linux-arm-kernel@lists.arm.linux.org.uk,
linux-kernel@vger.kernel.org
Hi,
No single comment on this idea?
Thanks,
Janusz
--------------------------
On 2009-08-12 12:39, Janusz Krzysztofik wrote:
> Change the way McBSP registers are updated: use cached values instead of
> relying upon those read back from the device.
>
> With this patch, I have finally managed to get rid of all random
> playback/recording hangups on my OMAP1510 based Amstrad Delta (buggy?)
> hardware. Before that, I could suspect that values read back from McBSP
> registers before updating them happened to be errornous.
>
> 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.
>
> Created against linux-2.6.31-rc5
>
> Tested on Amstrad Delta
>
> Signed-off-by: Janusz Krzysztofik
>
> ---
> --- linux-2.6.31-rc5/arch/arm/plat-omap/include/mach/mcbsp.h.orig 2009-08-11 23:43:25.000000000 +0200
> +++ linux-2.6.31-rc5/arch/arm/plat-omap/include/mach/mcbsp.h 2009-08-11 23:45:46.000000000 +0200
> @@ -377,6 +377,8 @@ struct omap_mcbsp {
> struct omap_mcbsp_platform_data *pdata;
> struct clk *iclk;
> struct clk *fclk;
> +
> + struct omap_mcbsp_reg_cfg reg_cache;
> };
> extern struct omap_mcbsp **mcbsp_ptr;
> extern int omap_mcbsp_count;
> --- linux-2.6.31-rc5/arch/arm/plat-omap/mcbsp.c.orig 2009-08-11 23:43:25.000000000 +0200
> +++ linux-2.6.31-rc5/arch/arm/plat-omap/mcbsp.c 2009-08-11 23:45:35.000000000 +0200
> @@ -91,6 +91,7 @@ static void omap_mcbsp_dump_reg(u8 id)
> static irqreturn_t omap_mcbsp_tx_irq_handler(int irq, void *dev_id)
> {
> struct omap_mcbsp *mcbsp_tx = dev_id;
> + struct omap_mcbsp_reg_cfg *reg_cache = &mcbsp_tx->reg_cache;
> u16 irqst_spcr2;
>
> irqst_spcr2 = OMAP_MCBSP_READ(mcbsp_tx->io_base, SPCR2);
> @@ -101,7 +102,7 @@ static irqreturn_t omap_mcbsp_tx_irq_han
> irqst_spcr2);
> /* Writing zero to XSYNC_ERR clears the IRQ */
> OMAP_MCBSP_WRITE(mcbsp_tx->io_base, SPCR2,
> - irqst_spcr2 & ~(XSYNC_ERR));
> + reg_cache->spcr2 &= ~(XSYNC_ERR));
> } else {
> complete(&mcbsp_tx->tx_irq_completion);
> }
> @@ -112,6 +113,7 @@ static irqreturn_t omap_mcbsp_tx_irq_han
> static irqreturn_t omap_mcbsp_rx_irq_handler(int irq, void *dev_id)
> {
> struct omap_mcbsp *mcbsp_rx = dev_id;
> + struct omap_mcbsp_reg_cfg *reg_cache = &mcbsp_rx->reg_cache;
> u16 irqst_spcr1;
>
> irqst_spcr1 = OMAP_MCBSP_READ(mcbsp_rx->io_base, SPCR1);
> @@ -122,7 +124,7 @@ static irqreturn_t omap_mcbsp_rx_irq_han
> irqst_spcr1);
> /* Writing zero to RSYNC_ERR clears the IRQ */
> OMAP_MCBSP_WRITE(mcbsp_rx->io_base, SPCR1,
> - irqst_spcr1 & ~(RSYNC_ERR));
> + reg_cache->spcr1 &= ~(RSYNC_ERR));
> } else {
> complete(&mcbsp_rx->tx_irq_completion);
> }
> @@ -167,6 +169,7 @@ 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;
> + struct omap_mcbsp_reg_cfg *reg_cache;
> void __iomem *io_base;
>
> if (!omap_mcbsp_check_valid_id(id)) {
> @@ -174,26 +177,27 @@ void omap_mcbsp_config(unsigned int id,
> return;
> }
> mcbsp = id_to_mcbsp_ptr(id);
> + reg_cache = &mcbsp->reg_cache;
>
> 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);
> + OMAP_MCBSP_WRITE(io_base, SPCR2, reg_cache->spcr2 = config->spcr2);
> + OMAP_MCBSP_WRITE(io_base, SPCR1, reg_cache->spcr1 = config->spcr1);
> + OMAP_MCBSP_WRITE(io_base, RCR2, reg_cache->rcr2 = config->rcr2);
> + OMAP_MCBSP_WRITE(io_base, RCR1, reg_cache->rcr1 = config->rcr1);
> + OMAP_MCBSP_WRITE(io_base, XCR2, reg_cache->xcr2 = config->xcr2);
> + OMAP_MCBSP_WRITE(io_base, XCR1, reg_cache->xcr1 = config->xcr1);
> + OMAP_MCBSP_WRITE(io_base, SRGR2, reg_cache->srgr2 = config->srgr2);
> + OMAP_MCBSP_WRITE(io_base, SRGR1, reg_cache->srgr1 = config->srgr1);
> + OMAP_MCBSP_WRITE(io_base, MCR2, reg_cache->mcr2 = config->mcr2);
> + OMAP_MCBSP_WRITE(io_base, MCR1, reg_cache->mcr1 = config->mcr1);
> + OMAP_MCBSP_WRITE(io_base, PCR0, reg_cache->pcr0 = config->pcr0);
> if (cpu_is_omap2430() || cpu_is_omap34xx()) {
> - OMAP_MCBSP_WRITE(io_base, XCCR, config->xccr);
> - OMAP_MCBSP_WRITE(io_base, RCCR, config->rccr);
> + OMAP_MCBSP_WRITE(io_base, XCCR, reg_cache->xccr = config->xccr);
> + OMAP_MCBSP_WRITE(io_base, RCCR, reg_cache->rccr = config->rccr);
> }
> }
> EXPORT_SYMBOL(omap_mcbsp_config);
> @@ -232,6 +236,7 @@ EXPORT_SYMBOL(omap_mcbsp_set_io_type);
> int omap_mcbsp_request(unsigned int id)
> {
> struct omap_mcbsp *mcbsp;
> + struct omap_mcbsp_reg_cfg *reg_cache;
> int err;
>
> if (!omap_mcbsp_check_valid_id(id)) {
> @@ -239,6 +244,7 @@ int omap_mcbsp_request(unsigned int id)
> return -ENODEV;
> }
> mcbsp = id_to_mcbsp_ptr(id);
> + reg_cache = &mcbsp->reg_cache;
>
> spin_lock(&mcbsp->lock);
> if (!mcbsp->free) {
> @@ -261,8 +267,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);
> + OMAP_MCBSP_WRITE(mcbsp->io_base, SPCR1, reg_cache->spcr1 = 0);
> + OMAP_MCBSP_WRITE(mcbsp->io_base, SPCR2, reg_cache->spcr2 = 0);
>
> if (mcbsp->io_type == OMAP_MCBSP_IRQ_IO) {
> /* We need to get IRQs here */
> @@ -335,42 +341,38 @@ EXPORT_SYMBOL(omap_mcbsp_free);
> void omap_mcbsp_start(unsigned int id, int tx, int rx)
> {
> struct omap_mcbsp *mcbsp;
> + struct omap_mcbsp_reg_cfg *reg_cache;
> void __iomem *io_base;
> int idle;
> - u16 w;
>
> if (!omap_mcbsp_check_valid_id(id)) {
> printk(KERN_ERR "%s: Invalid id (%d)\n", __func__, id + 1);
> return;
> }
> mcbsp = id_to_mcbsp_ptr(id);
> + reg_cache = &mcbsp->reg_cache;
> 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 = (reg_cache->rcr1 >> 5) & 0x7;
> + mcbsp->tx_word_length = (reg_cache->xcr1 >> 5) & 0x7;
>
> - idle = !((OMAP_MCBSP_READ(io_base, SPCR2) |
> - OMAP_MCBSP_READ(io_base, SPCR1)) & 1);
> + idle = !((reg_cache->spcr2 | reg_cache->spcr1) & 1);
>
> if (idle) {
> /* Start the sample generator */
> - w = OMAP_MCBSP_READ(io_base, SPCR2);
> - OMAP_MCBSP_WRITE(io_base, SPCR2, w | (1 << 6));
> + OMAP_MCBSP_WRITE(io_base, SPCR2, reg_cache->spcr2 |= (1 << 6));
> }
>
> /* Enable transmitter and receiver */
> - w = OMAP_MCBSP_READ(io_base, SPCR2);
> - OMAP_MCBSP_WRITE(io_base, SPCR2, w | (tx & 1));
> + OMAP_MCBSP_WRITE(io_base, SPCR2, reg_cache->spcr2 |= (tx & 1));
>
> - w = OMAP_MCBSP_READ(io_base, SPCR1);
> - OMAP_MCBSP_WRITE(io_base, SPCR1, w | (rx & 1));
> + OMAP_MCBSP_WRITE(io_base, SPCR1, reg_cache->spcr1 |= (rx & 1));
>
> udelay(100);
>
> if (idle) {
> /* Start frame sync */
> - w = OMAP_MCBSP_READ(io_base, SPCR2);
> - OMAP_MCBSP_WRITE(io_base, SPCR2, w | (1 << 7));
> + OMAP_MCBSP_WRITE(io_base, SPCR2, reg_cache->spcr2 |= (1 << 7));
> }
>
> /* Dump McBSP Regs */
> @@ -381,9 +383,9 @@ EXPORT_SYMBOL(omap_mcbsp_start);
> void omap_mcbsp_stop(unsigned int id, int tx, int rx)
> {
> struct omap_mcbsp *mcbsp;
> + struct omap_mcbsp_reg_cfg *reg_cache;
> void __iomem *io_base;
> int idle;
> - u16 w;
>
> if (!omap_mcbsp_check_valid_id(id)) {
> printk(KERN_ERR "%s: Invalid id (%d)\n", __func__, id + 1);
> @@ -391,23 +393,20 @@ void omap_mcbsp_stop(unsigned int id, in
> }
>
> mcbsp = id_to_mcbsp_ptr(id);
> + reg_cache = &mcbsp->reg_cache;
> io_base = mcbsp->io_base;
>
> /* Reset transmitter */
> - w = OMAP_MCBSP_READ(io_base, SPCR2);
> - OMAP_MCBSP_WRITE(io_base, SPCR2, w & ~(tx & 1));
> + OMAP_MCBSP_WRITE(io_base, SPCR2, reg_cache->spcr2 &= ~(tx & 1));
>
> /* Reset receiver */
> - w = OMAP_MCBSP_READ(io_base, SPCR1);
> - OMAP_MCBSP_WRITE(io_base, SPCR1, w & ~(rx & 1));
> + OMAP_MCBSP_WRITE(io_base, SPCR1, reg_cache->spcr1 &= ~(rx & 1));
>
> - idle = !((OMAP_MCBSP_READ(io_base, SPCR2) |
> - OMAP_MCBSP_READ(io_base, SPCR1)) & 1);
> + idle = !((reg_cache->spcr2 | reg_cache->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));
> + OMAP_MCBSP_WRITE(io_base, SPCR2, reg_cache->spcr2 &= ~(1 << 6));
> }
> }
> EXPORT_SYMBOL(omap_mcbsp_stop);
> @@ -557,6 +556,7 @@ EXPORT_SYMBOL(omap_mcbsp_recv_word);
> int omap_mcbsp_spi_master_xmit_word_poll(unsigned int id, u32 word)
> {
> struct omap_mcbsp *mcbsp;
> + struct omap_mcbsp_reg_cfg *reg_cache;
> void __iomem *io_base;
> omap_mcbsp_word_length tx_word_length;
> omap_mcbsp_word_length rx_word_length;
> @@ -567,6 +567,7 @@ int omap_mcbsp_spi_master_xmit_word_poll
> return -ENODEV;
> }
> mcbsp = id_to_mcbsp_ptr(id);
> + reg_cache = &mcbsp->reg_cache;
> io_base = mcbsp->io_base;
> tx_word_length = mcbsp->tx_word_length;
> rx_word_length = mcbsp->rx_word_length;
> @@ -580,9 +581,11 @@ int omap_mcbsp_spi_master_xmit_word_poll
> spcr2 = OMAP_MCBSP_READ(io_base, SPCR2);
> if (attempts++ > 1000) {
> /* We must reset the transmitter */
> - OMAP_MCBSP_WRITE(io_base, SPCR2, spcr2 & (~XRST));
> + OMAP_MCBSP_WRITE(io_base, SPCR2,
> + reg_cache->spcr2 &= (~XRST));
> udelay(10);
> - OMAP_MCBSP_WRITE(io_base, SPCR2, spcr2 | XRST);
> + OMAP_MCBSP_WRITE(io_base, SPCR2,
> + reg_cache->spcr2 |= XRST);
> udelay(10);
> dev_err(mcbsp->dev, "McBSP%d transmitter not "
> "ready\n", mcbsp->id);
> @@ -601,9 +604,11 @@ int omap_mcbsp_spi_master_xmit_word_poll
> spcr1 = OMAP_MCBSP_READ(io_base, SPCR1);
> if (attempts++ > 1000) {
> /* We must reset the receiver */
> - OMAP_MCBSP_WRITE(io_base, SPCR1, spcr1 & (~RRST));
> + OMAP_MCBSP_WRITE(io_base, SPCR1,
> + reg_cache->spcr1 &= (~RRST));
> udelay(10);
> - OMAP_MCBSP_WRITE(io_base, SPCR1, spcr1 | RRST);
> + OMAP_MCBSP_WRITE(io_base, SPCR1,
> + reg_cache->spcr1 |= RRST);
> udelay(10);
> dev_err(mcbsp->dev, "McBSP%d receiver not "
> "ready\n", mcbsp->id);
> @@ -623,6 +628,7 @@ EXPORT_SYMBOL(omap_mcbsp_spi_master_xmit
> int omap_mcbsp_spi_master_recv_word_poll(unsigned int id, u32 *word)
> {
> struct omap_mcbsp *mcbsp;
> + struct omap_mcbsp_reg_cfg *reg_cache;
> u32 clock_word = 0;
> void __iomem *io_base;
> omap_mcbsp_word_length tx_word_length;
> @@ -635,6 +641,7 @@ int omap_mcbsp_spi_master_recv_word_poll
> }
>
> mcbsp = id_to_mcbsp_ptr(id);
> + reg_cache = &mcbsp->reg_cache;
> io_base = mcbsp->io_base;
>
> tx_word_length = mcbsp->tx_word_length;
> @@ -649,9 +656,11 @@ int omap_mcbsp_spi_master_recv_word_poll
> spcr2 = OMAP_MCBSP_READ(io_base, SPCR2);
> if (attempts++ > 1000) {
> /* We must reset the transmitter */
> - OMAP_MCBSP_WRITE(io_base, SPCR2, spcr2 & (~XRST));
> + OMAP_MCBSP_WRITE(io_base, SPCR2,
> + reg_cache->spcr2 &= (~XRST));
> udelay(10);
> - OMAP_MCBSP_WRITE(io_base, SPCR2, spcr2 | XRST);
> + OMAP_MCBSP_WRITE(io_base, SPCR2,
> + reg_cache->spcr2 |= XRST);
> udelay(10);
> dev_err(mcbsp->dev, "McBSP%d transmitter not "
> "ready\n", mcbsp->id);
> @@ -670,9 +679,11 @@ int omap_mcbsp_spi_master_recv_word_poll
> spcr1 = OMAP_MCBSP_READ(io_base, SPCR1);
> if (attempts++ > 1000) {
> /* We must reset the receiver */
> - OMAP_MCBSP_WRITE(io_base, SPCR1, spcr1 & (~RRST));
> + OMAP_MCBSP_WRITE(io_base, SPCR1,
> + reg_cache->spcr1 &= (~RRST));
> udelay(10);
> - OMAP_MCBSP_WRITE(io_base, SPCR1, spcr1 | RRST);
> + OMAP_MCBSP_WRITE(io_base, SPCR1,
> + reg_cache->spcr1 |= RRST);
> udelay(10);
> dev_err(mcbsp->dev, "McBSP%d receiver not "
> "ready\n", mcbsp->id);
> --
> 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
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC][PATCH] ARM: OMAP: McBSP: Use register cache
@ 2009-09-04 5:39 Eero Nurkkala
2009-09-04 10:34 ` Janusz Krzysztofik
0 siblings, 1 reply; 15+ messages in thread
From: Eero Nurkkala @ 2009-09-04 5:39 UTC (permalink / raw)
To: linux-omap
Hello,
Register values should not get corrupted. Could you please point out the
registers that may get corrupted? Do they get corrupted during audio
play? I haven't seen corrupted register values.
- Eero
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC][PATCH] ARM: OMAP: McBSP: Use register cache
2009-09-04 5:39 [RFC][PATCH] ARM: " Eero Nurkkala
@ 2009-09-04 10:34 ` Janusz Krzysztofik
2009-09-04 11:07 ` Eero Nurkkala
0 siblings, 1 reply; 15+ messages in thread
From: Janusz Krzysztofik @ 2009-09-04 10:34 UTC (permalink / raw)
To: ext-eero.nurkkala; +Cc: linux-omap, e3-hacking
Friday 04 September 2009 07:39:36 Eero Nurkkala wrote:
>
> Register values should not get corrupted. Could you please point out the
> registers that may get corrupted? Do they get corrupted during audio
> play? I haven't seen corrupted register values.
>
Hi Eero,
Please have a look at some debug log lines below. Those were generated on my
Amstrad Delta videophone, using an application (asterisk softpbx with alsa
channel) that can restart a receiver of a prevoiusly requested McBSP by
calling omap_mcbsp_start() on user request. As you can see, corrupted values
can happen to get read from any register that is expected to return
predictable results.
Sep 4 08:21:17 amsdelta user.debug kernel: [309910.170000] omap-mcbsp omap-mcbsp.1: mcbsp: **** McBSP1 regs ****
Sep 4 08:21:17 amsdelta user.debug kernel: [309910.170000] omap-mcbsp omap-mcbsp.1: mcbsp: DRR2: 0x1000
Sep 4 08:21:17 amsdelta user.debug kernel: [309910.170000] omap-mcbsp omap-mcbsp.1: mcbsp: DRR1: 0x009d
Sep 4 08:21:17 amsdelta user.debug kernel: [309910.170000] omap-mcbsp omap-mcbsp.1: mcbsp: DXR2: 0x3000
Sep 4 08:21:17 amsdelta user.debug kernel: [309910.170000] omap-mcbsp omap-mcbsp.1: mcbsp: DXR1: 0x7da1
Sep 4 08:21:17 amsdelta user.debug kernel: [309910.170000] omap-mcbsp omap-mcbsp.1: mcbsp: SPCR2: 0x02f0
Sep 4 08:21:17 amsdelta user.debug kernel: [309910.170000] omap-mcbsp omap-mcbsp.1: mcbsp: SPCR1: 0x7031
Sep 4 08:21:17 amsdelta user.debug kernel: [309910.170000] omap-mcbsp omap-mcbsp.1: mcbsp: RCR2: 0x0045
Sep 4 08:21:17 amsdelta user.debug kernel: [309910.170000] omap-mcbsp omap-mcbsp.1: mcbsp: RCR1: 0x0040
Sep 4 08:21:17 amsdelta user.debug kernel: [309910.170000] omap-mcbsp omap-mcbsp.1: mcbsp: XCR2: 0x1045
Sep 4 08:21:17 amsdelta user.debug kernel: [309910.170000] omap-mcbsp omap-mcbsp.1: mcbsp: XCR1: 0x0040
Sep 4 08:21:17 amsdelta user.debug kernel: [309910.170000] omap-mcbsp omap-mcbsp.1: mcbsp: SRGR2: 0x310f
Sep 4 08:21:17 amsdelta user.debug kernel: [309910.170000] omap-mcbsp omap-mcbsp.1: mcbsp: SRGR1: 0x0000
Sep 4 08:21:17 amsdelta user.debug kernel: [309910.170000] omap-mcbsp omap-mcbsp.1: mcbsp: PCR0: 0x0003
Sep 4 08:21:17 amsdelta user.debug kernel: [309910.170000] omap-mcbsp omap-mcbsp.1: mcbsp: ***********************
Sep 4 08:21:19 amsdelta user.debug kernel: [309912.050000] omap-mcbsp omap-mcbsp.1: mcbsp: **** McBSP1 regs ****
Sep 4 08:21:19 amsdelta user.debug kernel: [309912.050000] omap-mcbsp omap-mcbsp.1: mcbsp: DRR2: 0x0000
Sep 4 08:21:19 amsdelta user.debug kernel: [309912.050000] omap-mcbsp omap-mcbsp.1: mcbsp: DRR1: 0x1086
Sep 4 08:21:19 amsdelta user.debug kernel: [309912.050000] omap-mcbsp omap-mcbsp.1: mcbsp: DXR2: 0x0000
Sep 4 08:21:19 amsdelta user.debug kernel: [309912.050000] omap-mcbsp omap-mcbsp.1: mcbsp: DXR1: 0x7da1
Sep 4 08:21:19 amsdelta user.debug kernel: [309912.050000] omap-mcbsp omap-mcbsp.1: mcbsp: SPCR2: 0x02f0
Sep 4 08:21:19 amsdelta user.debug kernel: [309912.050000] omap-mcbsp omap-mcbsp.1: mcbsp: SPCR1: 0x0031
Sep 4 08:21:19 amsdelta user.debug kernel: [309912.050000] omap-mcbsp omap-mcbsp.1: mcbsp: RCR2: 0x1045
Sep 4 08:21:19 amsdelta user.debug kernel: [309912.050000] omap-mcbsp omap-mcbsp.1: mcbsp: RCR1: 0x1040
Sep 4 08:21:19 amsdelta user.debug kernel: [309912.050000] omap-mcbsp omap-mcbsp.1: mcbsp: XCR2: 0x3045
Sep 4 08:21:19 amsdelta user.debug kernel: [309912.050000] omap-mcbsp omap-mcbsp.1: mcbsp: XCR1: 0x1040
Sep 4 08:21:19 amsdelta user.debug kernel: [309912.050000] omap-mcbsp omap-mcbsp.1: mcbsp: SRGR2: 0x000f
Sep 4 08:21:19 amsdelta user.debug kernel: [309912.050000] omap-mcbsp omap-mcbsp.1: mcbsp: SRGR1: 0x0000
Sep 4 08:21:19 amsdelta user.debug kernel: [309912.050000] omap-mcbsp omap-mcbsp.1: mcbsp: PCR0: 0x1003
Sep 4 08:21:19 amsdelta user.debug kernel: [309912.050000] omap-mcbsp omap-mcbsp.1: mcbsp: ***********************
Sep 4 08:21:21 amsdelta user.debug kernel: [309913.950000] omap-mcbsp omap-mcbsp.1: mcbsp: **** McBSP1 regs ****
Sep 4 08:21:21 amsdelta user.debug kernel: [309913.950000] omap-mcbsp omap-mcbsp.1: mcbsp: DRR2: 0x1000
Sep 4 08:21:21 amsdelta user.debug kernel: [309913.950000] omap-mcbsp omap-mcbsp.1: mcbsp: DRR1: 0x109e
Sep 4 08:21:21 amsdelta user.debug kernel: [309913.950000] omap-mcbsp omap-mcbsp.1: mcbsp: DXR2: 0x1000
Sep 4 08:21:21 amsdelta user.debug kernel: [309913.950000] omap-mcbsp omap-mcbsp.1: mcbsp: DXR1: 0x7da1
Sep 4 08:21:21 amsdelta user.debug kernel: [309913.950000] omap-mcbsp omap-mcbsp.1: mcbsp: SPCR2: 0x02f0
Sep 4 08:21:21 amsdelta user.debug kernel: [309913.950000] omap-mcbsp omap-mcbsp.1: mcbsp: SPCR1: 0x1031
Sep 4 08:21:21 amsdelta user.debug kernel: [309913.950000] omap-mcbsp omap-mcbsp.1: mcbsp: RCR2: 0x1045
Sep 4 08:21:21 amsdelta user.debug kernel: [309913.950000] omap-mcbsp omap-mcbsp.1: mcbsp: RCR1: 0x1040
Sep 4 08:21:21 amsdelta user.debug kernel: [309913.950000] omap-mcbsp omap-mcbsp.1: mcbsp: XCR2: 0x2045
Sep 4 08:21:21 amsdelta user.debug kernel: [309913.950000] omap-mcbsp omap-mcbsp.1: mcbsp: XCR1: 0x1040
Sep 4 08:21:21 amsdelta user.debug kernel: [309913.950000] omap-mcbsp omap-mcbsp.1: mcbsp: SRGR2: 0x100f
Sep 4 08:21:21 amsdelta user.debug kernel: [309913.950000] omap-mcbsp omap-mcbsp.1: mcbsp: SRGR1: 0x1000
Sep 4 08:21:21 amsdelta user.debug kernel: [309913.950000] omap-mcbsp omap-mcbsp.1: mcbsp: PCR0: 0x2003
Sep 4 08:21:21 amsdelta user.debug kernel: [309913.950000] omap-mcbsp omap-mcbsp.1: mcbsp: ***********************
Those corruptions, even if should never happen, were actually caused by a
wireless adapter connected to the machine USB port. Extra piece of hardware,
very usefull in some cases, but obviously not taken into account while the
machine was being designed. While playback seems unaffected, WiFi noise in
capture stream prevents from using the videophone in speakerphone mode, but
in normal (handset) mode, the noise level seems still acceptable. The only
missing piece of a puzzle is my patch, that prevents the kernel driver from
putting corrupted values into the McBSP control registers.
That learned, please reconsider:
1. Can my patch break anything, related or not?
2. How does it affect performance of otherwise unaffected machines?
3. Is correcting a poorly designed machine behaviour worth of the change?
Thanks,
Janusz
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC][PATCH] ARM: OMAP: McBSP: Use register cache
2009-09-04 10:34 ` Janusz Krzysztofik
@ 2009-09-04 11:07 ` Eero Nurkkala
2009-09-05 17:31 ` Anuj Aggarwal
0 siblings, 1 reply; 15+ messages in thread
From: Eero Nurkkala @ 2009-09-04 11:07 UTC (permalink / raw)
To: ext Janusz Krzysztofik; +Cc: linux-omap@vger.kernel.org, e3-hacking@earth.li
On Fri, 2009-09-04 at 12:34 +0200, ext Janusz Krzysztofik wrote:
> in normal (handset) mode, the noise level seems still acceptable. The only
> missing piece of a puzzle is my patch, that prevents the kernel driver from
> putting corrupted values into the McBSP control registers.
>
> That learned, please reconsider:
> 1. Can my patch break anything, related or not?
That needs to be investigated in more detail.
> 2. How does it affect performance of otherwise unaffected machines?
Like you already know, shouldn't.
> 3. Is correcting a poorly designed machine behaviour worth of the change?
>
> Thanks,
> Janusz
I'd say this is in the right track. All McBSP registers
(not status etc) will need to be stored in memory. (Now, they're not).
And all those register contents will need to be written back
in certain situations. So this is the case when there is an external
audio chip, that takes audio in in bursts. So once the
burst is complete, all McBSP clocks will be disabled for a duration.
Then (at least >= 3430) device will hit OFF mode, meaning
all register contents are lost, also McBSP's. They will
need to be written back at some point.
My quick verdict is, that your patch is, going into right
direction. But the thing is that my words don't count much ;)
(Possibly worth taking the patch in, if guaranteed to not break
any others).
- Eero
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC][PATCH] ARM: OMAP: McBSP: Use register cache
2009-09-04 11:07 ` Eero Nurkkala
@ 2009-09-05 17:31 ` Anuj Aggarwal
2009-09-05 20:46 ` ext-Eero.Nurkkala
0 siblings, 1 reply; 15+ messages in thread
From: Anuj Aggarwal @ 2009-09-05 17:31 UTC (permalink / raw)
To: ext-eero.nurkkala
Cc: ext Janusz Krzysztofik, linux-omap@vger.kernel.org,
e3-hacking@earth.li
On Fri, Sep 4, 2009 at 4:37 PM, Eero
Nurkkala<ext-eero.nurkkala@nokia.com> wrote:
> On Fri, 2009-09-04 at 12:34 +0200, ext Janusz Krzysztofik wrote:
>> in normal (handset) mode, the noise level seems still acceptable. The only
>> missing piece of a puzzle is my patch, that prevents the kernel driver from
>> putting corrupted values into the McBSP control registers.
>>
>> That learned, please reconsider:
>> 1. Can my patch break anything, related or not?
>
> That needs to be investigated in more detail.
>
>> 2. How does it affect performance of otherwise unaffected machines?
>
> Like you already know, shouldn't.
>
>> 3. Is correcting a poorly designed machine behaviour worth of the change?
>>
>> Thanks,
>> Janusz
>
>
> I'd say this is in the right track. All McBSP registers
> (not status etc) will need to be stored in memory. (Now, they're not).
> And all those register contents will need to be written back
> in certain situations. So this is the case when there is an external
> audio chip, that takes audio in in bursts. So once the
> burst is complete, all McBSP clocks will be disabled for a duration.
>
> Then (at least >= 3430) device will hit OFF mode, meaning
> all register contents are lost, also McBSP's. They will
> need to be written back at some point.
Isn't the better way to achieve this thing, at least from the OFF mode
perspective, is to provide similar implementation for McBSP in
omap3_core_save_context () / omap3_core_restore_context(), in
file arch/arm/mach-omap2/pm34xx.c? Code for other core peripherals
is already there and we have to add support for rest of the core-domain
peripherals.
>
> My quick verdict is, that your patch is, going into right
> direction. But the thing is that my words don't count much ;)
>
> (Possibly worth taking the patch in, if guaranteed to not break
> any others).
>
> - Eero
>
> --
> 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
>
--
Best Regards,
Anuj Aggarwal
--
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [RFC][PATCH] ARM: OMAP: McBSP: Use register cache
2009-09-05 17:31 ` Anuj Aggarwal
@ 2009-09-05 20:46 ` ext-Eero.Nurkkala
0 siblings, 0 replies; 15+ messages in thread
From: ext-Eero.Nurkkala @ 2009-09-05 20:46 UTC (permalink / raw)
To: anuj.aggarwal; +Cc: jkrzyszt, linux-omap, e3-hacking
> Isn't the better way to achieve this thing, at least from the OFF mode
> perspective, is to provide similar implementation for McBSP in
> omap3_core_save_context () / omap3_core_restore_context(), in
> file arch/arm/mach-omap2/pm34xx.c? Code for other core peripherals
> is already there and we have to add support for rest of the core-domain
> peripherals.
Yes and no. I'm not sure McBSP context would be needed to be restored
after every wake up from PER off mode.. isn't that how it's handled now
for other peripherals? But maybe restore context for McBSP whenever
there's need for it. Rewriting millions of registers just for the case doesn't
sound good. IMO, with an irq from an external device, a rewrite of the
McBSP registers occurs. You loose the point of the OFF mode if you always
start rewriting all registers? (you're not saving power.. the opposite).
Or did I not understand something? (most likely the case) =)
- Eero
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC][PATCH] ARM: OMAP: McBSP: Use register cache
2009-08-12 10:39 [RFC][PATCH] ARM: OMAP: McBSP: Use register cache Janusz Krzysztofik
2009-08-28 11:26 ` Janusz Krzysztofik
@ 2009-11-14 2:06 ` Janusz Krzysztofik
2009-11-23 20:42 ` Janusz Krzysztofik
1 sibling, 1 reply; 15+ messages in thread
From: Janusz Krzysztofik @ 2009-11-14 2:06 UTC (permalink / raw)
To: Tony Lindgren, Jarkko Nikula, Samuel Ortiz
Cc: Peter Ujfalusi, alsa-devel@alsa-project.org,
linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Hi,
This patch got outdated and does not apply any more, but the idea still hangs
in my queue. I am not sure why the patch has never been acknowledged nor
rejected. Maybe it just got missed?
Could you please take a position on this idea?
Thanks,
Janusz
http://www.spinics.net/lists/linux-omap/msg16720.html
Wednesday 12 August 2009 12:39:25 Janusz Krzysztofik wrote:
> Change the way McBSP registers are updated: use cached values instead of
> relying upon those read back from the device.
>
> With this patch, I have finally managed to get rid of all random
> playback/recording hangups on my OMAP1510 based Amstrad Delta (buggy?)
> hardware. Before that, I could suspect that values read back from McBSP
> registers before updating them happened to be errornous.
>
> 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.
>
> Created against linux-2.6.31-rc5
>
> Tested on Amstrad Delta
>
> Signed-off-by: Janusz Krzysztofik
>
> ---
> ---
> linux-2.6.31-rc5/arch/arm/plat-omap/include/mach/mcbsp.h.orig 2009-08-11
> 23:43:25.000000000 +0200 +++
> linux-2.6.31-rc5/arch/arm/plat-omap/include/mach/mcbsp.h 2009-08-11
> 23:45:46.000000000 +0200 @@ -377,6 +377,8 @@ struct omap_mcbsp {
> struct omap_mcbsp_platform_data *pdata;
> struct clk *iclk;
> struct clk *fclk;
> +
> + struct omap_mcbsp_reg_cfg reg_cache;
> };
> extern struct omap_mcbsp **mcbsp_ptr;
> extern int omap_mcbsp_count;
> --- linux-2.6.31-rc5/arch/arm/plat-omap/mcbsp.c.orig 2009-08-11
> 23:43:25.000000000 +0200 +++
> linux-2.6.31-rc5/arch/arm/plat-omap/mcbsp.c 2009-08-11 23:45:35.000000000
> +0200 @@ -91,6 +91,7 @@ static void omap_mcbsp_dump_reg(u8 id)
> static irqreturn_t omap_mcbsp_tx_irq_handler(int irq, void *dev_id)
> {
> struct omap_mcbsp *mcbsp_tx = dev_id;
> + struct omap_mcbsp_reg_cfg *reg_cache = &mcbsp_tx->reg_cache;
> u16 irqst_spcr2;
>
> irqst_spcr2 = OMAP_MCBSP_READ(mcbsp_tx->io_base, SPCR2);
> @@ -101,7 +102,7 @@ static irqreturn_t omap_mcbsp_tx_irq_han
> irqst_spcr2);
> /* Writing zero to XSYNC_ERR clears the IRQ */
> OMAP_MCBSP_WRITE(mcbsp_tx->io_base, SPCR2,
> - irqst_spcr2 & ~(XSYNC_ERR));
> + reg_cache->spcr2 &= ~(XSYNC_ERR));
> } else {
> complete(&mcbsp_tx->tx_irq_completion);
> }
> @@ -112,6 +113,7 @@ static irqreturn_t omap_mcbsp_tx_irq_han
> static irqreturn_t omap_mcbsp_rx_irq_handler(int irq, void *dev_id)
> {
> struct omap_mcbsp *mcbsp_rx = dev_id;
> + struct omap_mcbsp_reg_cfg *reg_cache = &mcbsp_rx->reg_cache;
> u16 irqst_spcr1;
>
> irqst_spcr1 = OMAP_MCBSP_READ(mcbsp_rx->io_base, SPCR1);
> @@ -122,7 +124,7 @@ static irqreturn_t omap_mcbsp_rx_irq_han
> irqst_spcr1);
> /* Writing zero to RSYNC_ERR clears the IRQ */
> OMAP_MCBSP_WRITE(mcbsp_rx->io_base, SPCR1,
> - irqst_spcr1 & ~(RSYNC_ERR));
> + reg_cache->spcr1 &= ~(RSYNC_ERR));
> } else {
> complete(&mcbsp_rx->tx_irq_completion);
> }
> @@ -167,6 +169,7 @@ 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;
> + struct omap_mcbsp_reg_cfg *reg_cache;
> void __iomem *io_base;
>
> if (!omap_mcbsp_check_valid_id(id)) {
> @@ -174,26 +177,27 @@ void omap_mcbsp_config(unsigned int id,
> return;
> }
> mcbsp = id_to_mcbsp_ptr(id);
> + reg_cache = &mcbsp->reg_cache;
>
> 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);
> + OMAP_MCBSP_WRITE(io_base, SPCR2, reg_cache->spcr2 = config->spcr2);
> + OMAP_MCBSP_WRITE(io_base, SPCR1, reg_cache->spcr1 = config->spcr1);
> + OMAP_MCBSP_WRITE(io_base, RCR2, reg_cache->rcr2 = config->rcr2);
> + OMAP_MCBSP_WRITE(io_base, RCR1, reg_cache->rcr1 = config->rcr1);
> + OMAP_MCBSP_WRITE(io_base, XCR2, reg_cache->xcr2 = config->xcr2);
> + OMAP_MCBSP_WRITE(io_base, XCR1, reg_cache->xcr1 = config->xcr1);
> + OMAP_MCBSP_WRITE(io_base, SRGR2, reg_cache->srgr2 = config->srgr2);
> + OMAP_MCBSP_WRITE(io_base, SRGR1, reg_cache->srgr1 = config->srgr1);
> + OMAP_MCBSP_WRITE(io_base, MCR2, reg_cache->mcr2 = config->mcr2);
> + OMAP_MCBSP_WRITE(io_base, MCR1, reg_cache->mcr1 = config->mcr1);
> + OMAP_MCBSP_WRITE(io_base, PCR0, reg_cache->pcr0 = config->pcr0);
> if (cpu_is_omap2430() || cpu_is_omap34xx()) {
> - OMAP_MCBSP_WRITE(io_base, XCCR, config->xccr);
> - OMAP_MCBSP_WRITE(io_base, RCCR, config->rccr);
> + OMAP_MCBSP_WRITE(io_base, XCCR, reg_cache->xccr = config->xccr);
> + OMAP_MCBSP_WRITE(io_base, RCCR, reg_cache->rccr = config->rccr);
> }
> }
> EXPORT_SYMBOL(omap_mcbsp_config);
> @@ -232,6 +236,7 @@ EXPORT_SYMBOL(omap_mcbsp_set_io_type);
> int omap_mcbsp_request(unsigned int id)
> {
> struct omap_mcbsp *mcbsp;
> + struct omap_mcbsp_reg_cfg *reg_cache;
> int err;
>
> if (!omap_mcbsp_check_valid_id(id)) {
> @@ -239,6 +244,7 @@ int omap_mcbsp_request(unsigned int id)
> return -ENODEV;
> }
> mcbsp = id_to_mcbsp_ptr(id);
> + reg_cache = &mcbsp->reg_cache;
>
> spin_lock(&mcbsp->lock);
> if (!mcbsp->free) {
> @@ -261,8 +267,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);
> + OMAP_MCBSP_WRITE(mcbsp->io_base, SPCR1, reg_cache->spcr1 = 0);
> + OMAP_MCBSP_WRITE(mcbsp->io_base, SPCR2, reg_cache->spcr2 = 0);
>
> if (mcbsp->io_type == OMAP_MCBSP_IRQ_IO) {
> /* We need to get IRQs here */
> @@ -335,42 +341,38 @@ EXPORT_SYMBOL(omap_mcbsp_free);
> void omap_mcbsp_start(unsigned int id, int tx, int rx)
> {
> struct omap_mcbsp *mcbsp;
> + struct omap_mcbsp_reg_cfg *reg_cache;
> void __iomem *io_base;
> int idle;
> - u16 w;
>
> if (!omap_mcbsp_check_valid_id(id)) {
> printk(KERN_ERR "%s: Invalid id (%d)\n", __func__, id + 1);
> return;
> }
> mcbsp = id_to_mcbsp_ptr(id);
> + reg_cache = &mcbsp->reg_cache;
> 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 = (reg_cache->rcr1 >> 5) & 0x7;
> + mcbsp->tx_word_length = (reg_cache->xcr1 >> 5) & 0x7;
>
> - idle = !((OMAP_MCBSP_READ(io_base, SPCR2) |
> - OMAP_MCBSP_READ(io_base, SPCR1)) & 1);
> + idle = !((reg_cache->spcr2 | reg_cache->spcr1) & 1);
>
> if (idle) {
> /* Start the sample generator */
> - w = OMAP_MCBSP_READ(io_base, SPCR2);
> - OMAP_MCBSP_WRITE(io_base, SPCR2, w | (1 << 6));
> + OMAP_MCBSP_WRITE(io_base, SPCR2, reg_cache->spcr2 |= (1 << 6));
> }
>
> /* Enable transmitter and receiver */
> - w = OMAP_MCBSP_READ(io_base, SPCR2);
> - OMAP_MCBSP_WRITE(io_base, SPCR2, w | (tx & 1));
> + OMAP_MCBSP_WRITE(io_base, SPCR2, reg_cache->spcr2 |= (tx & 1));
>
> - w = OMAP_MCBSP_READ(io_base, SPCR1);
> - OMAP_MCBSP_WRITE(io_base, SPCR1, w | (rx & 1));
> + OMAP_MCBSP_WRITE(io_base, SPCR1, reg_cache->spcr1 |= (rx & 1));
>
> udelay(100);
>
> if (idle) {
> /* Start frame sync */
> - w = OMAP_MCBSP_READ(io_base, SPCR2);
> - OMAP_MCBSP_WRITE(io_base, SPCR2, w | (1 << 7));
> + OMAP_MCBSP_WRITE(io_base, SPCR2, reg_cache->spcr2 |= (1 << 7));
> }
>
> /* Dump McBSP Regs */
> @@ -381,9 +383,9 @@ EXPORT_SYMBOL(omap_mcbsp_start);
> void omap_mcbsp_stop(unsigned int id, int tx, int rx)
> {
> struct omap_mcbsp *mcbsp;
> + struct omap_mcbsp_reg_cfg *reg_cache;
> void __iomem *io_base;
> int idle;
> - u16 w;
>
> if (!omap_mcbsp_check_valid_id(id)) {
> printk(KERN_ERR "%s: Invalid id (%d)\n", __func__, id + 1);
> @@ -391,23 +393,20 @@ void omap_mcbsp_stop(unsigned int id, in
> }
>
> mcbsp = id_to_mcbsp_ptr(id);
> + reg_cache = &mcbsp->reg_cache;
> io_base = mcbsp->io_base;
>
> /* Reset transmitter */
> - w = OMAP_MCBSP_READ(io_base, SPCR2);
> - OMAP_MCBSP_WRITE(io_base, SPCR2, w & ~(tx & 1));
> + OMAP_MCBSP_WRITE(io_base, SPCR2, reg_cache->spcr2 &= ~(tx & 1));
>
> /* Reset receiver */
> - w = OMAP_MCBSP_READ(io_base, SPCR1);
> - OMAP_MCBSP_WRITE(io_base, SPCR1, w & ~(rx & 1));
> + OMAP_MCBSP_WRITE(io_base, SPCR1, reg_cache->spcr1 &= ~(rx & 1));
>
> - idle = !((OMAP_MCBSP_READ(io_base, SPCR2) |
> - OMAP_MCBSP_READ(io_base, SPCR1)) & 1);
> + idle = !((reg_cache->spcr2 | reg_cache->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));
> + OMAP_MCBSP_WRITE(io_base, SPCR2, reg_cache->spcr2 &= ~(1 << 6));
> }
> }
> EXPORT_SYMBOL(omap_mcbsp_stop);
> @@ -557,6 +556,7 @@ EXPORT_SYMBOL(omap_mcbsp_recv_word);
> int omap_mcbsp_spi_master_xmit_word_poll(unsigned int id, u32 word)
> {
> struct omap_mcbsp *mcbsp;
> + struct omap_mcbsp_reg_cfg *reg_cache;
> void __iomem *io_base;
> omap_mcbsp_word_length tx_word_length;
> omap_mcbsp_word_length rx_word_length;
> @@ -567,6 +567,7 @@ int omap_mcbsp_spi_master_xmit_word_poll
> return -ENODEV;
> }
> mcbsp = id_to_mcbsp_ptr(id);
> + reg_cache = &mcbsp->reg_cache;
> io_base = mcbsp->io_base;
> tx_word_length = mcbsp->tx_word_length;
> rx_word_length = mcbsp->rx_word_length;
> @@ -580,9 +581,11 @@ int omap_mcbsp_spi_master_xmit_word_poll
> spcr2 = OMAP_MCBSP_READ(io_base, SPCR2);
> if (attempts++ > 1000) {
> /* We must reset the transmitter */
> - OMAP_MCBSP_WRITE(io_base, SPCR2, spcr2 & (~XRST));
> + OMAP_MCBSP_WRITE(io_base, SPCR2,
> + reg_cache->spcr2 &= (~XRST));
> udelay(10);
> - OMAP_MCBSP_WRITE(io_base, SPCR2, spcr2 | XRST);
> + OMAP_MCBSP_WRITE(io_base, SPCR2,
> + reg_cache->spcr2 |= XRST);
> udelay(10);
> dev_err(mcbsp->dev, "McBSP%d transmitter not "
> "ready\n", mcbsp->id);
> @@ -601,9 +604,11 @@ int omap_mcbsp_spi_master_xmit_word_poll
> spcr1 = OMAP_MCBSP_READ(io_base, SPCR1);
> if (attempts++ > 1000) {
> /* We must reset the receiver */
> - OMAP_MCBSP_WRITE(io_base, SPCR1, spcr1 & (~RRST));
> + OMAP_MCBSP_WRITE(io_base, SPCR1,
> + reg_cache->spcr1 &= (~RRST));
> udelay(10);
> - OMAP_MCBSP_WRITE(io_base, SPCR1, spcr1 | RRST);
> + OMAP_MCBSP_WRITE(io_base, SPCR1,
> + reg_cache->spcr1 |= RRST);
> udelay(10);
> dev_err(mcbsp->dev, "McBSP%d receiver not "
> "ready\n", mcbsp->id);
> @@ -623,6 +628,7 @@ EXPORT_SYMBOL(omap_mcbsp_spi_master_xmit
> int omap_mcbsp_spi_master_recv_word_poll(unsigned int id, u32 *word)
> {
> struct omap_mcbsp *mcbsp;
> + struct omap_mcbsp_reg_cfg *reg_cache;
> u32 clock_word = 0;
> void __iomem *io_base;
> omap_mcbsp_word_length tx_word_length;
> @@ -635,6 +641,7 @@ int omap_mcbsp_spi_master_recv_word_poll
> }
>
> mcbsp = id_to_mcbsp_ptr(id);
> + reg_cache = &mcbsp->reg_cache;
> io_base = mcbsp->io_base;
>
> tx_word_length = mcbsp->tx_word_length;
> @@ -649,9 +656,11 @@ int omap_mcbsp_spi_master_recv_word_poll
> spcr2 = OMAP_MCBSP_READ(io_base, SPCR2);
> if (attempts++ > 1000) {
> /* We must reset the transmitter */
> - OMAP_MCBSP_WRITE(io_base, SPCR2, spcr2 & (~XRST));
> + OMAP_MCBSP_WRITE(io_base, SPCR2,
> + reg_cache->spcr2 &= (~XRST));
> udelay(10);
> - OMAP_MCBSP_WRITE(io_base, SPCR2, spcr2 | XRST);
> + OMAP_MCBSP_WRITE(io_base, SPCR2,
> + reg_cache->spcr2 |= XRST);
> udelay(10);
> dev_err(mcbsp->dev, "McBSP%d transmitter not "
> "ready\n", mcbsp->id);
> @@ -670,9 +679,11 @@ int omap_mcbsp_spi_master_recv_word_poll
> spcr1 = OMAP_MCBSP_READ(io_base, SPCR1);
> if (attempts++ > 1000) {
> /* We must reset the receiver */
> - OMAP_MCBSP_WRITE(io_base, SPCR1, spcr1 & (~RRST));
> + OMAP_MCBSP_WRITE(io_base, SPCR1,
> + reg_cache->spcr1 &= (~RRST));
> udelay(10);
> - OMAP_MCBSP_WRITE(io_base, SPCR1, spcr1 | RRST);
> + OMAP_MCBSP_WRITE(io_base, SPCR1,
> + reg_cache->spcr1 |= RRST);
> udelay(10);
> dev_err(mcbsp->dev, "McBSP%d receiver not "
> "ready\n", mcbsp->id);
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC][PATCH] ARM: OMAP: McBSP: Use register cache
2009-11-14 2:06 ` Janusz Krzysztofik
@ 2009-11-23 20:42 ` Janusz Krzysztofik
2009-11-23 23:53 ` Tony Lindgren
0 siblings, 1 reply; 15+ messages in thread
From: Janusz Krzysztofik @ 2009-11-23 20:42 UTC (permalink / raw)
To: Tony Lindgren; +Cc: linux-omap@vger.kernel.org, Jarkko Nikula
(drop all except Tony, Jarkko and linux-omap)
Saturday 14 November 2009 03:06:39 Janusz Krzysztofik napisał(a):
> Hi,
>
> This patch got outdated and does not apply any more, but the idea still
> hangs in my queue. I am not sure why the patch has never been acknowledged
> nor rejected. Maybe it just got missed?
Tony,
I have just found this patch, initially submitted on 2009-08-12, archived in
linux-omap patchwork with State: Awaiting Upstream. What does it mean? Am I
supposed to do something about it? After that many weeks of no negative
comments nor requests for changes, and taking into account what Jarkko said
lately in a different thread[*] about the McBSP register cache concept, can I
assume that it has been generally accepted? Is it worth of refreshing against
current linux-omap?
Thanks,
Janusz
> Could you please take a position on this idea?
>
> Thanks,
> Janusz
>
> http://www.spinics.net/lists/linux-omap/msg16720.html
[*] http://www.spinics.net/lists/linux-omap/msg20597.html
> Wednesday 12 August 2009 12:39:25 Janusz Krzysztofik wrote:
> > Change the way McBSP registers are updated: use cached values instead of
> > relying upon those read back from the device.
> >
> > With this patch, I have finally managed to get rid of all random
> > playback/recording hangups on my OMAP1510 based Amstrad Delta (buggy?)
> > hardware. Before that, I could suspect that values read back from McBSP
> > registers before updating them happened to be errornous.
> >
> > 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.
> >
> > Created against linux-2.6.31-rc5
> >
> > Tested on Amstrad Delta
> >
> > Signed-off-by: Janusz Krzysztofik
> >
> > ---
> > ---
> > linux-2.6.31-rc5/arch/arm/plat-omap/include/mach/mcbsp.h.orig 2009-08-11
> > 23:43:25.000000000 +0200 +++
> > linux-2.6.31-rc5/arch/arm/plat-omap/include/mach/mcbsp.h 2009-08-11
> > 23:45:46.000000000 +0200 @@ -377,6 +377,8 @@ struct omap_mcbsp {
> > struct omap_mcbsp_platform_data *pdata;
> > struct clk *iclk;
> > struct clk *fclk;
> > +
> > + struct omap_mcbsp_reg_cfg reg_cache;
> > };
> > extern struct omap_mcbsp **mcbsp_ptr;
> > extern int omap_mcbsp_count;
> > --- linux-2.6.31-rc5/arch/arm/plat-omap/mcbsp.c.orig 2009-08-11
> > 23:43:25.000000000 +0200 +++
> > linux-2.6.31-rc5/arch/arm/plat-omap/mcbsp.c 2009-08-11 23:45:35.000000000
> > +0200 @@ -91,6 +91,7 @@ static void omap_mcbsp_dump_reg(u8 id)
> > static irqreturn_t omap_mcbsp_tx_irq_handler(int irq, void *dev_id)
> > {
> > struct omap_mcbsp *mcbsp_tx = dev_id;
> > + struct omap_mcbsp_reg_cfg *reg_cache = &mcbsp_tx->reg_cache;
> > u16 irqst_spcr2;
> >
> > irqst_spcr2 = OMAP_MCBSP_READ(mcbsp_tx->io_base, SPCR2);
> > @@ -101,7 +102,7 @@ static irqreturn_t omap_mcbsp_tx_irq_han
> > irqst_spcr2);
> > /* Writing zero to XSYNC_ERR clears the IRQ */
> > OMAP_MCBSP_WRITE(mcbsp_tx->io_base, SPCR2,
> > - irqst_spcr2 & ~(XSYNC_ERR));
> > + reg_cache->spcr2 &= ~(XSYNC_ERR));
> > } else {
> > complete(&mcbsp_tx->tx_irq_completion);
> > }
> > @@ -112,6 +113,7 @@ static irqreturn_t omap_mcbsp_tx_irq_han
> > static irqreturn_t omap_mcbsp_rx_irq_handler(int irq, void *dev_id)
> > {
> > struct omap_mcbsp *mcbsp_rx = dev_id;
> > + struct omap_mcbsp_reg_cfg *reg_cache = &mcbsp_rx->reg_cache;
> > u16 irqst_spcr1;
> >
> > irqst_spcr1 = OMAP_MCBSP_READ(mcbsp_rx->io_base, SPCR1);
> > @@ -122,7 +124,7 @@ static irqreturn_t omap_mcbsp_rx_irq_han
> > irqst_spcr1);
> > /* Writing zero to RSYNC_ERR clears the IRQ */
> > OMAP_MCBSP_WRITE(mcbsp_rx->io_base, SPCR1,
> > - irqst_spcr1 & ~(RSYNC_ERR));
> > + reg_cache->spcr1 &= ~(RSYNC_ERR));
> > } else {
> > complete(&mcbsp_rx->tx_irq_completion);
> > }
> > @@ -167,6 +169,7 @@ 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;
> > + struct omap_mcbsp_reg_cfg *reg_cache;
> > void __iomem *io_base;
> >
> > if (!omap_mcbsp_check_valid_id(id)) {
> > @@ -174,26 +177,27 @@ void omap_mcbsp_config(unsigned int id,
> > return;
> > }
> > mcbsp = id_to_mcbsp_ptr(id);
> > + reg_cache = &mcbsp->reg_cache;
> >
> > 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);
> > + OMAP_MCBSP_WRITE(io_base, SPCR2, reg_cache->spcr2 = config->spcr2);
> > + OMAP_MCBSP_WRITE(io_base, SPCR1, reg_cache->spcr1 = config->spcr1);
> > + OMAP_MCBSP_WRITE(io_base, RCR2, reg_cache->rcr2 = config->rcr2);
> > + OMAP_MCBSP_WRITE(io_base, RCR1, reg_cache->rcr1 = config->rcr1);
> > + OMAP_MCBSP_WRITE(io_base, XCR2, reg_cache->xcr2 = config->xcr2);
> > + OMAP_MCBSP_WRITE(io_base, XCR1, reg_cache->xcr1 = config->xcr1);
> > + OMAP_MCBSP_WRITE(io_base, SRGR2, reg_cache->srgr2 = config->srgr2);
> > + OMAP_MCBSP_WRITE(io_base, SRGR1, reg_cache->srgr1 = config->srgr1);
> > + OMAP_MCBSP_WRITE(io_base, MCR2, reg_cache->mcr2 = config->mcr2);
> > + OMAP_MCBSP_WRITE(io_base, MCR1, reg_cache->mcr1 = config->mcr1);
> > + OMAP_MCBSP_WRITE(io_base, PCR0, reg_cache->pcr0 = config->pcr0);
> > if (cpu_is_omap2430() || cpu_is_omap34xx()) {
> > - OMAP_MCBSP_WRITE(io_base, XCCR, config->xccr);
> > - OMAP_MCBSP_WRITE(io_base, RCCR, config->rccr);
> > + OMAP_MCBSP_WRITE(io_base, XCCR, reg_cache->xccr = config->xccr);
> > + OMAP_MCBSP_WRITE(io_base, RCCR, reg_cache->rccr = config->rccr);
> > }
> > }
> > EXPORT_SYMBOL(omap_mcbsp_config);
> > @@ -232,6 +236,7 @@ EXPORT_SYMBOL(omap_mcbsp_set_io_type);
> > int omap_mcbsp_request(unsigned int id)
> > {
> > struct omap_mcbsp *mcbsp;
> > + struct omap_mcbsp_reg_cfg *reg_cache;
> > int err;
> >
> > if (!omap_mcbsp_check_valid_id(id)) {
> > @@ -239,6 +244,7 @@ int omap_mcbsp_request(unsigned int id)
> > return -ENODEV;
> > }
> > mcbsp = id_to_mcbsp_ptr(id);
> > + reg_cache = &mcbsp->reg_cache;
> >
> > spin_lock(&mcbsp->lock);
> > if (!mcbsp->free) {
> > @@ -261,8 +267,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);
> > + OMAP_MCBSP_WRITE(mcbsp->io_base, SPCR1, reg_cache->spcr1 = 0);
> > + OMAP_MCBSP_WRITE(mcbsp->io_base, SPCR2, reg_cache->spcr2 = 0);
> >
> > if (mcbsp->io_type == OMAP_MCBSP_IRQ_IO) {
> > /* We need to get IRQs here */
> > @@ -335,42 +341,38 @@ EXPORT_SYMBOL(omap_mcbsp_free);
> > void omap_mcbsp_start(unsigned int id, int tx, int rx)
> > {
> > struct omap_mcbsp *mcbsp;
> > + struct omap_mcbsp_reg_cfg *reg_cache;
> > void __iomem *io_base;
> > int idle;
> > - u16 w;
> >
> > if (!omap_mcbsp_check_valid_id(id)) {
> > printk(KERN_ERR "%s: Invalid id (%d)\n", __func__, id + 1);
> > return;
> > }
> > mcbsp = id_to_mcbsp_ptr(id);
> > + reg_cache = &mcbsp->reg_cache;
> > 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 = (reg_cache->rcr1 >> 5) & 0x7;
> > + mcbsp->tx_word_length = (reg_cache->xcr1 >> 5) & 0x7;
> >
> > - idle = !((OMAP_MCBSP_READ(io_base, SPCR2) |
> > - OMAP_MCBSP_READ(io_base, SPCR1)) & 1);
> > + idle = !((reg_cache->spcr2 | reg_cache->spcr1) & 1);
> >
> > if (idle) {
> > /* Start the sample generator */
> > - w = OMAP_MCBSP_READ(io_base, SPCR2);
> > - OMAP_MCBSP_WRITE(io_base, SPCR2, w | (1 << 6));
> > + OMAP_MCBSP_WRITE(io_base, SPCR2, reg_cache->spcr2 |= (1 << 6));
> > }
> >
> > /* Enable transmitter and receiver */
> > - w = OMAP_MCBSP_READ(io_base, SPCR2);
> > - OMAP_MCBSP_WRITE(io_base, SPCR2, w | (tx & 1));
> > + OMAP_MCBSP_WRITE(io_base, SPCR2, reg_cache->spcr2 |= (tx & 1));
> >
> > - w = OMAP_MCBSP_READ(io_base, SPCR1);
> > - OMAP_MCBSP_WRITE(io_base, SPCR1, w | (rx & 1));
> > + OMAP_MCBSP_WRITE(io_base, SPCR1, reg_cache->spcr1 |= (rx & 1));
> >
> > udelay(100);
> >
> > if (idle) {
> > /* Start frame sync */
> > - w = OMAP_MCBSP_READ(io_base, SPCR2);
> > - OMAP_MCBSP_WRITE(io_base, SPCR2, w | (1 << 7));
> > + OMAP_MCBSP_WRITE(io_base, SPCR2, reg_cache->spcr2 |= (1 << 7));
> > }
> >
> > /* Dump McBSP Regs */
> > @@ -381,9 +383,9 @@ EXPORT_SYMBOL(omap_mcbsp_start);
> > void omap_mcbsp_stop(unsigned int id, int tx, int rx)
> > {
> > struct omap_mcbsp *mcbsp;
> > + struct omap_mcbsp_reg_cfg *reg_cache;
> > void __iomem *io_base;
> > int idle;
> > - u16 w;
> >
> > if (!omap_mcbsp_check_valid_id(id)) {
> > printk(KERN_ERR "%s: Invalid id (%d)\n", __func__, id + 1);
> > @@ -391,23 +393,20 @@ void omap_mcbsp_stop(unsigned int id, in
> > }
> >
> > mcbsp = id_to_mcbsp_ptr(id);
> > + reg_cache = &mcbsp->reg_cache;
> > io_base = mcbsp->io_base;
> >
> > /* Reset transmitter */
> > - w = OMAP_MCBSP_READ(io_base, SPCR2);
> > - OMAP_MCBSP_WRITE(io_base, SPCR2, w & ~(tx & 1));
> > + OMAP_MCBSP_WRITE(io_base, SPCR2, reg_cache->spcr2 &= ~(tx & 1));
> >
> > /* Reset receiver */
> > - w = OMAP_MCBSP_READ(io_base, SPCR1);
> > - OMAP_MCBSP_WRITE(io_base, SPCR1, w & ~(rx & 1));
> > + OMAP_MCBSP_WRITE(io_base, SPCR1, reg_cache->spcr1 &= ~(rx & 1));
> >
> > - idle = !((OMAP_MCBSP_READ(io_base, SPCR2) |
> > - OMAP_MCBSP_READ(io_base, SPCR1)) & 1);
> > + idle = !((reg_cache->spcr2 | reg_cache->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));
> > + OMAP_MCBSP_WRITE(io_base, SPCR2, reg_cache->spcr2 &= ~(1 << 6));
> > }
> > }
> > EXPORT_SYMBOL(omap_mcbsp_stop);
> > @@ -557,6 +556,7 @@ EXPORT_SYMBOL(omap_mcbsp_recv_word);
> > int omap_mcbsp_spi_master_xmit_word_poll(unsigned int id, u32 word)
> > {
> > struct omap_mcbsp *mcbsp;
> > + struct omap_mcbsp_reg_cfg *reg_cache;
> > void __iomem *io_base;
> > omap_mcbsp_word_length tx_word_length;
> > omap_mcbsp_word_length rx_word_length;
> > @@ -567,6 +567,7 @@ int omap_mcbsp_spi_master_xmit_word_poll
> > return -ENODEV;
> > }
> > mcbsp = id_to_mcbsp_ptr(id);
> > + reg_cache = &mcbsp->reg_cache;
> > io_base = mcbsp->io_base;
> > tx_word_length = mcbsp->tx_word_length;
> > rx_word_length = mcbsp->rx_word_length;
> > @@ -580,9 +581,11 @@ int omap_mcbsp_spi_master_xmit_word_poll
> > spcr2 = OMAP_MCBSP_READ(io_base, SPCR2);
> > if (attempts++ > 1000) {
> > /* We must reset the transmitter */
> > - OMAP_MCBSP_WRITE(io_base, SPCR2, spcr2 & (~XRST));
> > + OMAP_MCBSP_WRITE(io_base, SPCR2,
> > + reg_cache->spcr2 &= (~XRST));
> > udelay(10);
> > - OMAP_MCBSP_WRITE(io_base, SPCR2, spcr2 | XRST);
> > + OMAP_MCBSP_WRITE(io_base, SPCR2,
> > + reg_cache->spcr2 |= XRST);
> > udelay(10);
> > dev_err(mcbsp->dev, "McBSP%d transmitter not "
> > "ready\n", mcbsp->id);
> > @@ -601,9 +604,11 @@ int omap_mcbsp_spi_master_xmit_word_poll
> > spcr1 = OMAP_MCBSP_READ(io_base, SPCR1);
> > if (attempts++ > 1000) {
> > /* We must reset the receiver */
> > - OMAP_MCBSP_WRITE(io_base, SPCR1, spcr1 & (~RRST));
> > + OMAP_MCBSP_WRITE(io_base, SPCR1,
> > + reg_cache->spcr1 &= (~RRST));
> > udelay(10);
> > - OMAP_MCBSP_WRITE(io_base, SPCR1, spcr1 | RRST);
> > + OMAP_MCBSP_WRITE(io_base, SPCR1,
> > + reg_cache->spcr1 |= RRST);
> > udelay(10);
> > dev_err(mcbsp->dev, "McBSP%d receiver not "
> > "ready\n", mcbsp->id);
> > @@ -623,6 +628,7 @@ EXPORT_SYMBOL(omap_mcbsp_spi_master_xmit
> > int omap_mcbsp_spi_master_recv_word_poll(unsigned int id, u32 *word)
> > {
> > struct omap_mcbsp *mcbsp;
> > + struct omap_mcbsp_reg_cfg *reg_cache;
> > u32 clock_word = 0;
> > void __iomem *io_base;
> > omap_mcbsp_word_length tx_word_length;
> > @@ -635,6 +641,7 @@ int omap_mcbsp_spi_master_recv_word_poll
> > }
> >
> > mcbsp = id_to_mcbsp_ptr(id);
> > + reg_cache = &mcbsp->reg_cache;
> > io_base = mcbsp->io_base;
> >
> > tx_word_length = mcbsp->tx_word_length;
> > @@ -649,9 +656,11 @@ int omap_mcbsp_spi_master_recv_word_poll
> > spcr2 = OMAP_MCBSP_READ(io_base, SPCR2);
> > if (attempts++ > 1000) {
> > /* We must reset the transmitter */
> > - OMAP_MCBSP_WRITE(io_base, SPCR2, spcr2 & (~XRST));
> > + OMAP_MCBSP_WRITE(io_base, SPCR2,
> > + reg_cache->spcr2 &= (~XRST));
> > udelay(10);
> > - OMAP_MCBSP_WRITE(io_base, SPCR2, spcr2 | XRST);
> > + OMAP_MCBSP_WRITE(io_base, SPCR2,
> > + reg_cache->spcr2 |= XRST);
> > udelay(10);
> > dev_err(mcbsp->dev, "McBSP%d transmitter not "
> > "ready\n", mcbsp->id);
> > @@ -670,9 +679,11 @@ int omap_mcbsp_spi_master_recv_word_poll
> > spcr1 = OMAP_MCBSP_READ(io_base, SPCR1);
> > if (attempts++ > 1000) {
> > /* We must reset the receiver */
> > - OMAP_MCBSP_WRITE(io_base, SPCR1, spcr1 & (~RRST));
> > + OMAP_MCBSP_WRITE(io_base, SPCR1,
> > + reg_cache->spcr1 &= (~RRST));
> > udelay(10);
> > - OMAP_MCBSP_WRITE(io_base, SPCR1, spcr1 | RRST);
> > + OMAP_MCBSP_WRITE(io_base, SPCR1,
> > + reg_cache->spcr1 |= RRST);
> > udelay(10);
> > dev_err(mcbsp->dev, "McBSP%d receiver not "
> > "ready\n", mcbsp->id);
--
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC][PATCH] ARM: OMAP: McBSP: Use register cache
2009-11-23 20:42 ` Janusz Krzysztofik
@ 2009-11-23 23:53 ` Tony Lindgren
2009-11-24 7:59 ` Jarkko Nikula
0 siblings, 1 reply; 15+ messages in thread
From: Tony Lindgren @ 2009-11-23 23:53 UTC (permalink / raw)
To: Janusz Krzysztofik; +Cc: linux-omap@vger.kernel.org, Jarkko Nikula
* Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> [091123 12:42]:
> (drop all except Tony, Jarkko and linux-omap)
>
> Saturday 14 November 2009 03:06:39 Janusz Krzysztofik napisał(a):
> > Hi,
> >
> > This patch got outdated and does not apply any more, but the idea still
> > hangs in my queue. I am not sure why the patch has never been acknowledged
> > nor rejected. Maybe it just got missed?
>
> Tony,
>
> I have just found this patch, initially submitted on 2009-08-12, archived in
> linux-omap patchwork with State: Awaiting Upstream. What does it mean? Am I
> supposed to do something about it? After that many weeks of no negative
> comments nor requests for changes, and taking into account what Jarkko said
> lately in a different thread[*] about the McBSP register cache concept, can I
> assume that it has been generally accepted? Is it worth of refreshing against
> current linux-omap?
I'll just mark patches "Awaiting upstream" if I feel like somebody else
should ack or merge it. If there are obvious changes needed, I'll mark
it "Changes requested".
So for the patch, please refresh. Then let's apply it assuming Jarkko
acks it.
Regards,
Tony
> Thanks,
> Janusz
>
> > Could you please take a position on this idea?
> >
> > Thanks,
> > Janusz
> >
> > http://www.spinics.net/lists/linux-omap/msg16720.html
>
> [*] http://www.spinics.net/lists/linux-omap/msg20597.html
>
> > Wednesday 12 August 2009 12:39:25 Janusz Krzysztofik wrote:
> > > Change the way McBSP registers are updated: use cached values instead of
> > > relying upon those read back from the device.
> > >
> > > With this patch, I have finally managed to get rid of all random
> > > playback/recording hangups on my OMAP1510 based Amstrad Delta (buggy?)
> > > hardware. Before that, I could suspect that values read back from McBSP
> > > registers before updating them happened to be errornous.
> > >
> > > 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.
> > >
> > > Created against linux-2.6.31-rc5
> > >
> > > Tested on Amstrad Delta
> > >
> > > Signed-off-by: Janusz Krzysztofik
> > >
> > > ---
> > > ---
> > > linux-2.6.31-rc5/arch/arm/plat-omap/include/mach/mcbsp.h.orig 2009-08-11
> > > 23:43:25.000000000 +0200 +++
> > > linux-2.6.31-rc5/arch/arm/plat-omap/include/mach/mcbsp.h 2009-08-11
> > > 23:45:46.000000000 +0200 @@ -377,6 +377,8 @@ struct omap_mcbsp {
> > > struct omap_mcbsp_platform_data *pdata;
> > > struct clk *iclk;
> > > struct clk *fclk;
> > > +
> > > + struct omap_mcbsp_reg_cfg reg_cache;
> > > };
> > > extern struct omap_mcbsp **mcbsp_ptr;
> > > extern int omap_mcbsp_count;
> > > --- linux-2.6.31-rc5/arch/arm/plat-omap/mcbsp.c.orig 2009-08-11
> > > 23:43:25.000000000 +0200 +++
> > > linux-2.6.31-rc5/arch/arm/plat-omap/mcbsp.c 2009-08-11 23:45:35.000000000
> > > +0200 @@ -91,6 +91,7 @@ static void omap_mcbsp_dump_reg(u8 id)
> > > static irqreturn_t omap_mcbsp_tx_irq_handler(int irq, void *dev_id)
> > > {
> > > struct omap_mcbsp *mcbsp_tx = dev_id;
> > > + struct omap_mcbsp_reg_cfg *reg_cache = &mcbsp_tx->reg_cache;
> > > u16 irqst_spcr2;
> > >
> > > irqst_spcr2 = OMAP_MCBSP_READ(mcbsp_tx->io_base, SPCR2);
> > > @@ -101,7 +102,7 @@ static irqreturn_t omap_mcbsp_tx_irq_han
> > > irqst_spcr2);
> > > /* Writing zero to XSYNC_ERR clears the IRQ */
> > > OMAP_MCBSP_WRITE(mcbsp_tx->io_base, SPCR2,
> > > - irqst_spcr2 & ~(XSYNC_ERR));
> > > + reg_cache->spcr2 &= ~(XSYNC_ERR));
> > > } else {
> > > complete(&mcbsp_tx->tx_irq_completion);
> > > }
> > > @@ -112,6 +113,7 @@ static irqreturn_t omap_mcbsp_tx_irq_han
> > > static irqreturn_t omap_mcbsp_rx_irq_handler(int irq, void *dev_id)
> > > {
> > > struct omap_mcbsp *mcbsp_rx = dev_id;
> > > + struct omap_mcbsp_reg_cfg *reg_cache = &mcbsp_rx->reg_cache;
> > > u16 irqst_spcr1;
> > >
> > > irqst_spcr1 = OMAP_MCBSP_READ(mcbsp_rx->io_base, SPCR1);
> > > @@ -122,7 +124,7 @@ static irqreturn_t omap_mcbsp_rx_irq_han
> > > irqst_spcr1);
> > > /* Writing zero to RSYNC_ERR clears the IRQ */
> > > OMAP_MCBSP_WRITE(mcbsp_rx->io_base, SPCR1,
> > > - irqst_spcr1 & ~(RSYNC_ERR));
> > > + reg_cache->spcr1 &= ~(RSYNC_ERR));
> > > } else {
> > > complete(&mcbsp_rx->tx_irq_completion);
> > > }
> > > @@ -167,6 +169,7 @@ 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;
> > > + struct omap_mcbsp_reg_cfg *reg_cache;
> > > void __iomem *io_base;
> > >
> > > if (!omap_mcbsp_check_valid_id(id)) {
> > > @@ -174,26 +177,27 @@ void omap_mcbsp_config(unsigned int id,
> > > return;
> > > }
> > > mcbsp = id_to_mcbsp_ptr(id);
> > > + reg_cache = &mcbsp->reg_cache;
> > >
> > > 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);
> > > + OMAP_MCBSP_WRITE(io_base, SPCR2, reg_cache->spcr2 = config->spcr2);
> > > + OMAP_MCBSP_WRITE(io_base, SPCR1, reg_cache->spcr1 = config->spcr1);
> > > + OMAP_MCBSP_WRITE(io_base, RCR2, reg_cache->rcr2 = config->rcr2);
> > > + OMAP_MCBSP_WRITE(io_base, RCR1, reg_cache->rcr1 = config->rcr1);
> > > + OMAP_MCBSP_WRITE(io_base, XCR2, reg_cache->xcr2 = config->xcr2);
> > > + OMAP_MCBSP_WRITE(io_base, XCR1, reg_cache->xcr1 = config->xcr1);
> > > + OMAP_MCBSP_WRITE(io_base, SRGR2, reg_cache->srgr2 = config->srgr2);
> > > + OMAP_MCBSP_WRITE(io_base, SRGR1, reg_cache->srgr1 = config->srgr1);
> > > + OMAP_MCBSP_WRITE(io_base, MCR2, reg_cache->mcr2 = config->mcr2);
> > > + OMAP_MCBSP_WRITE(io_base, MCR1, reg_cache->mcr1 = config->mcr1);
> > > + OMAP_MCBSP_WRITE(io_base, PCR0, reg_cache->pcr0 = config->pcr0);
> > > if (cpu_is_omap2430() || cpu_is_omap34xx()) {
> > > - OMAP_MCBSP_WRITE(io_base, XCCR, config->xccr);
> > > - OMAP_MCBSP_WRITE(io_base, RCCR, config->rccr);
> > > + OMAP_MCBSP_WRITE(io_base, XCCR, reg_cache->xccr = config->xccr);
> > > + OMAP_MCBSP_WRITE(io_base, RCCR, reg_cache->rccr = config->rccr);
> > > }
> > > }
> > > EXPORT_SYMBOL(omap_mcbsp_config);
> > > @@ -232,6 +236,7 @@ EXPORT_SYMBOL(omap_mcbsp_set_io_type);
> > > int omap_mcbsp_request(unsigned int id)
> > > {
> > > struct omap_mcbsp *mcbsp;
> > > + struct omap_mcbsp_reg_cfg *reg_cache;
> > > int err;
> > >
> > > if (!omap_mcbsp_check_valid_id(id)) {
> > > @@ -239,6 +244,7 @@ int omap_mcbsp_request(unsigned int id)
> > > return -ENODEV;
> > > }
> > > mcbsp = id_to_mcbsp_ptr(id);
> > > + reg_cache = &mcbsp->reg_cache;
> > >
> > > spin_lock(&mcbsp->lock);
> > > if (!mcbsp->free) {
> > > @@ -261,8 +267,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);
> > > + OMAP_MCBSP_WRITE(mcbsp->io_base, SPCR1, reg_cache->spcr1 = 0);
> > > + OMAP_MCBSP_WRITE(mcbsp->io_base, SPCR2, reg_cache->spcr2 = 0);
> > >
> > > if (mcbsp->io_type == OMAP_MCBSP_IRQ_IO) {
> > > /* We need to get IRQs here */
> > > @@ -335,42 +341,38 @@ EXPORT_SYMBOL(omap_mcbsp_free);
> > > void omap_mcbsp_start(unsigned int id, int tx, int rx)
> > > {
> > > struct omap_mcbsp *mcbsp;
> > > + struct omap_mcbsp_reg_cfg *reg_cache;
> > > void __iomem *io_base;
> > > int idle;
> > > - u16 w;
> > >
> > > if (!omap_mcbsp_check_valid_id(id)) {
> > > printk(KERN_ERR "%s: Invalid id (%d)\n", __func__, id + 1);
> > > return;
> > > }
> > > mcbsp = id_to_mcbsp_ptr(id);
> > > + reg_cache = &mcbsp->reg_cache;
> > > 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 = (reg_cache->rcr1 >> 5) & 0x7;
> > > + mcbsp->tx_word_length = (reg_cache->xcr1 >> 5) & 0x7;
> > >
> > > - idle = !((OMAP_MCBSP_READ(io_base, SPCR2) |
> > > - OMAP_MCBSP_READ(io_base, SPCR1)) & 1);
> > > + idle = !((reg_cache->spcr2 | reg_cache->spcr1) & 1);
> > >
> > > if (idle) {
> > > /* Start the sample generator */
> > > - w = OMAP_MCBSP_READ(io_base, SPCR2);
> > > - OMAP_MCBSP_WRITE(io_base, SPCR2, w | (1 << 6));
> > > + OMAP_MCBSP_WRITE(io_base, SPCR2, reg_cache->spcr2 |= (1 << 6));
> > > }
> > >
> > > /* Enable transmitter and receiver */
> > > - w = OMAP_MCBSP_READ(io_base, SPCR2);
> > > - OMAP_MCBSP_WRITE(io_base, SPCR2, w | (tx & 1));
> > > + OMAP_MCBSP_WRITE(io_base, SPCR2, reg_cache->spcr2 |= (tx & 1));
> > >
> > > - w = OMAP_MCBSP_READ(io_base, SPCR1);
> > > - OMAP_MCBSP_WRITE(io_base, SPCR1, w | (rx & 1));
> > > + OMAP_MCBSP_WRITE(io_base, SPCR1, reg_cache->spcr1 |= (rx & 1));
> > >
> > > udelay(100);
> > >
> > > if (idle) {
> > > /* Start frame sync */
> > > - w = OMAP_MCBSP_READ(io_base, SPCR2);
> > > - OMAP_MCBSP_WRITE(io_base, SPCR2, w | (1 << 7));
> > > + OMAP_MCBSP_WRITE(io_base, SPCR2, reg_cache->spcr2 |= (1 << 7));
> > > }
> > >
> > > /* Dump McBSP Regs */
> > > @@ -381,9 +383,9 @@ EXPORT_SYMBOL(omap_mcbsp_start);
> > > void omap_mcbsp_stop(unsigned int id, int tx, int rx)
> > > {
> > > struct omap_mcbsp *mcbsp;
> > > + struct omap_mcbsp_reg_cfg *reg_cache;
> > > void __iomem *io_base;
> > > int idle;
> > > - u16 w;
> > >
> > > if (!omap_mcbsp_check_valid_id(id)) {
> > > printk(KERN_ERR "%s: Invalid id (%d)\n", __func__, id + 1);
> > > @@ -391,23 +393,20 @@ void omap_mcbsp_stop(unsigned int id, in
> > > }
> > >
> > > mcbsp = id_to_mcbsp_ptr(id);
> > > + reg_cache = &mcbsp->reg_cache;
> > > io_base = mcbsp->io_base;
> > >
> > > /* Reset transmitter */
> > > - w = OMAP_MCBSP_READ(io_base, SPCR2);
> > > - OMAP_MCBSP_WRITE(io_base, SPCR2, w & ~(tx & 1));
> > > + OMAP_MCBSP_WRITE(io_base, SPCR2, reg_cache->spcr2 &= ~(tx & 1));
> > >
> > > /* Reset receiver */
> > > - w = OMAP_MCBSP_READ(io_base, SPCR1);
> > > - OMAP_MCBSP_WRITE(io_base, SPCR1, w & ~(rx & 1));
> > > + OMAP_MCBSP_WRITE(io_base, SPCR1, reg_cache->spcr1 &= ~(rx & 1));
> > >
> > > - idle = !((OMAP_MCBSP_READ(io_base, SPCR2) |
> > > - OMAP_MCBSP_READ(io_base, SPCR1)) & 1);
> > > + idle = !((reg_cache->spcr2 | reg_cache->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));
> > > + OMAP_MCBSP_WRITE(io_base, SPCR2, reg_cache->spcr2 &= ~(1 << 6));
> > > }
> > > }
> > > EXPORT_SYMBOL(omap_mcbsp_stop);
> > > @@ -557,6 +556,7 @@ EXPORT_SYMBOL(omap_mcbsp_recv_word);
> > > int omap_mcbsp_spi_master_xmit_word_poll(unsigned int id, u32 word)
> > > {
> > > struct omap_mcbsp *mcbsp;
> > > + struct omap_mcbsp_reg_cfg *reg_cache;
> > > void __iomem *io_base;
> > > omap_mcbsp_word_length tx_word_length;
> > > omap_mcbsp_word_length rx_word_length;
> > > @@ -567,6 +567,7 @@ int omap_mcbsp_spi_master_xmit_word_poll
> > > return -ENODEV;
> > > }
> > > mcbsp = id_to_mcbsp_ptr(id);
> > > + reg_cache = &mcbsp->reg_cache;
> > > io_base = mcbsp->io_base;
> > > tx_word_length = mcbsp->tx_word_length;
> > > rx_word_length = mcbsp->rx_word_length;
> > > @@ -580,9 +581,11 @@ int omap_mcbsp_spi_master_xmit_word_poll
> > > spcr2 = OMAP_MCBSP_READ(io_base, SPCR2);
> > > if (attempts++ > 1000) {
> > > /* We must reset the transmitter */
> > > - OMAP_MCBSP_WRITE(io_base, SPCR2, spcr2 & (~XRST));
> > > + OMAP_MCBSP_WRITE(io_base, SPCR2,
> > > + reg_cache->spcr2 &= (~XRST));
> > > udelay(10);
> > > - OMAP_MCBSP_WRITE(io_base, SPCR2, spcr2 | XRST);
> > > + OMAP_MCBSP_WRITE(io_base, SPCR2,
> > > + reg_cache->spcr2 |= XRST);
> > > udelay(10);
> > > dev_err(mcbsp->dev, "McBSP%d transmitter not "
> > > "ready\n", mcbsp->id);
> > > @@ -601,9 +604,11 @@ int omap_mcbsp_spi_master_xmit_word_poll
> > > spcr1 = OMAP_MCBSP_READ(io_base, SPCR1);
> > > if (attempts++ > 1000) {
> > > /* We must reset the receiver */
> > > - OMAP_MCBSP_WRITE(io_base, SPCR1, spcr1 & (~RRST));
> > > + OMAP_MCBSP_WRITE(io_base, SPCR1,
> > > + reg_cache->spcr1 &= (~RRST));
> > > udelay(10);
> > > - OMAP_MCBSP_WRITE(io_base, SPCR1, spcr1 | RRST);
> > > + OMAP_MCBSP_WRITE(io_base, SPCR1,
> > > + reg_cache->spcr1 |= RRST);
> > > udelay(10);
> > > dev_err(mcbsp->dev, "McBSP%d receiver not "
> > > "ready\n", mcbsp->id);
> > > @@ -623,6 +628,7 @@ EXPORT_SYMBOL(omap_mcbsp_spi_master_xmit
> > > int omap_mcbsp_spi_master_recv_word_poll(unsigned int id, u32 *word)
> > > {
> > > struct omap_mcbsp *mcbsp;
> > > + struct omap_mcbsp_reg_cfg *reg_cache;
> > > u32 clock_word = 0;
> > > void __iomem *io_base;
> > > omap_mcbsp_word_length tx_word_length;
> > > @@ -635,6 +641,7 @@ int omap_mcbsp_spi_master_recv_word_poll
> > > }
> > >
> > > mcbsp = id_to_mcbsp_ptr(id);
> > > + reg_cache = &mcbsp->reg_cache;
> > > io_base = mcbsp->io_base;
> > >
> > > tx_word_length = mcbsp->tx_word_length;
> > > @@ -649,9 +656,11 @@ int omap_mcbsp_spi_master_recv_word_poll
> > > spcr2 = OMAP_MCBSP_READ(io_base, SPCR2);
> > > if (attempts++ > 1000) {
> > > /* We must reset the transmitter */
> > > - OMAP_MCBSP_WRITE(io_base, SPCR2, spcr2 & (~XRST));
> > > + OMAP_MCBSP_WRITE(io_base, SPCR2,
> > > + reg_cache->spcr2 &= (~XRST));
> > > udelay(10);
> > > - OMAP_MCBSP_WRITE(io_base, SPCR2, spcr2 | XRST);
> > > + OMAP_MCBSP_WRITE(io_base, SPCR2,
> > > + reg_cache->spcr2 |= XRST);
> > > udelay(10);
> > > dev_err(mcbsp->dev, "McBSP%d transmitter not "
> > > "ready\n", mcbsp->id);
> > > @@ -670,9 +679,11 @@ int omap_mcbsp_spi_master_recv_word_poll
> > > spcr1 = OMAP_MCBSP_READ(io_base, SPCR1);
> > > if (attempts++ > 1000) {
> > > /* We must reset the receiver */
> > > - OMAP_MCBSP_WRITE(io_base, SPCR1, spcr1 & (~RRST));
> > > + OMAP_MCBSP_WRITE(io_base, SPCR1,
> > > + reg_cache->spcr1 &= (~RRST));
> > > udelay(10);
> > > - OMAP_MCBSP_WRITE(io_base, SPCR1, spcr1 | RRST);
> > > + OMAP_MCBSP_WRITE(io_base, SPCR1,
> > > + reg_cache->spcr1 |= RRST);
> > > udelay(10);
> > > dev_err(mcbsp->dev, "McBSP%d receiver not "
> > > "ready\n", mcbsp->id);
>
>
--
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC][PATCH] ARM: OMAP: McBSP: Use register cache
2009-11-23 23:53 ` Tony Lindgren
@ 2009-11-24 7:59 ` Jarkko Nikula
2009-11-24 11:31 ` [RFC][PATCH v2] " Janusz Krzysztofik
0 siblings, 1 reply; 15+ messages in thread
From: Jarkko Nikula @ 2009-11-24 7:59 UTC (permalink / raw)
To: Tony Lindgren; +Cc: Janusz Krzysztofik, linux-omap@vger.kernel.org
On Mon, 23 Nov 2009 15:53:55 -0800
Tony Lindgren <tony@atomide.com> wrote:
> > I have just found this patch, initially submitted on 2009-08-12, archived in
> > linux-omap patchwork with State: Awaiting Upstream. What does it mean? Am I
> > supposed to do something about it? After that many weeks of no negative
> > comments nor requests for changes, and taking into account what Jarkko said
> > lately in a different thread[*] about the McBSP register cache concept, can I
> > assume that it has been generally accepted? Is it worth of refreshing against
> > current linux-omap?
>
> I'll just mark patches "Awaiting upstream" if I feel like somebody else
> should ack or merge it. If there are obvious changes needed, I'll mark
> it "Changes requested".
>
> So for the patch, please refresh. Then let's apply it assuming Jarkko
> acks it.
>
Yep, I was aware of this Janusz's patch and was thinking that it could
help also for developing the McBSP context save/restore features.
> > > > + OMAP_MCBSP_WRITE(io_base, SPCR2, reg_cache->spcr2 = config->spcr2);
> > > > + OMAP_MCBSP_WRITE(io_base, SPCR1, reg_cache->spcr1 = config->spcr1);
...
> > > > - 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 = (reg_cache->rcr1 >> 5) & 0x7;
> > > > + mcbsp->tx_word_length = (reg_cache->xcr1 >> 5) & 0x7;
IMO these would be cleaner if you embed cache handling into read and
write functions only. Probably reading could have explicit _read and
read_cache functions when code is dealing with the status bits and when
just updating some cached bits.
--
Jarkko
^ permalink raw reply [flat|nested] 15+ messages in thread
* [RFC][PATCH v2] OMAP: McBSP: Use register cache
2009-11-24 7:59 ` Jarkko Nikula
@ 2009-11-24 11:31 ` Janusz Krzysztofik
2009-11-26 8:00 ` Jarkko Nikula
0 siblings, 1 reply; 15+ messages in thread
From: Janusz Krzysztofik @ 2009-11-24 11:31 UTC (permalink / raw)
To: Jarkko Nikula; +Cc: Tony Lindgren, linux-omap@vger.kernel.org
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.
Created against linux-omap for-next,
commit 2963c21fab52bfa8227da7f22864db393ebbc858
Tested on Amstrad Delta
Compile tested with omap_generic_2420_defconfig
Signed-off-by: Janusz Krzysztofik
---
Tuesday 24 November 2009 08:59:23 Jarkko Nikula wrote:
>
> > > > > + OMAP_MCBSP_WRITE(io_base, SPCR2, reg_cache->spcr2 =
> > > > > config->spcr2); + OMAP_MCBSP_WRITE(io_base, SPCR1, reg_cache->spcr1
> > > > > = config->spcr1);
>
> ...
>
> > > > > - 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 = (reg_cache->rcr1 >> 5) & 0x7;
> > > > > + mcbsp->tx_word_length = (reg_cache->xcr1 >> 5) & 0x7;
>
> IMO these would be cleaner if you embed cache handling into read and
> write functions only. Probably reading could have explicit _read and
> read_cache functions when code is dealing with the status bits and when
> just updating some cached bits.
Hi Jarkko,
Could you please have a look at this modified version? It seems to address
your comment above more or less closely.
Thanks,
Janusz
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-11-21 00:38:46.000000000 +0100
+++ git/arch/arm/plat-omap/include/plat/mcbsp.h 2009-11-24 02:38:32.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;
Only in git/arch/arm/plat-omap/include/plat: mcbsp.h.orig
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-11-21 00:38:46.000000000 +0100
+++ git/arch/arm/plat-omap/mcbsp.c 2009-11-24 12:22:42.000000000 +0100
@@ -46,10 +46,21 @@ 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 OMAP_MCBSP_READ(mcbsp, reg) \
+ omap_mcbsp_read(mcbsp->io_base, OMAP_MCBSP_REG_##reg)
+#define OMAP_MCBSP_WRITE(mcbsp, reg, val) \
+ omap_mcbsp_write(mcbsp->io_base, OMAP_MCBSP_REG_##reg, val)
+
+#define OMAP_MCBSP_CREAD(mcbsp, reg) \
+ (mcbsp->reg_cache[OMAP_MCBSP_REG_##reg / OMAP_MCBSP_REG_DRR1])
+#define OMAP_MCBSP_CWRITE(mcbsp, 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 OMAP_MCBSP_BREAD(mcbsp, reg) \
+ (mcbsp->reg_cache[OMAP_MCBSP_REG_##reg / OMAP_MCBSP_REG_DRR1] \
+ = OMAP_MCBSP_READ(mcbsp->io_base, reg))
#define omap_mcbsp_check_valid_id(id) (id < omap_mcbsp_count)
#define id_to_mcbsp_ptr(id) mcbsp_ptr[id];
@@ -60,31 +71,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));
+ OMAP_MCBSP_READ(mcbsp, DRR2));
dev_dbg(mcbsp->dev, "DRR1: 0x%04x\n",
- OMAP_MCBSP_READ(mcbsp->io_base, DRR1));
+ OMAP_MCBSP_READ(mcbsp, DRR1));
dev_dbg(mcbsp->dev, "DXR2: 0x%04x\n",
- OMAP_MCBSP_READ(mcbsp->io_base, DXR2));
+ OMAP_MCBSP_READ(mcbsp, DXR2));
dev_dbg(mcbsp->dev, "DXR1: 0x%04x\n",
- OMAP_MCBSP_READ(mcbsp->io_base, DXR1));
+ OMAP_MCBSP_READ(mcbsp, DXR1));
dev_dbg(mcbsp->dev, "SPCR2: 0x%04x\n",
- OMAP_MCBSP_READ(mcbsp->io_base, SPCR2));
+ OMAP_MCBSP_READ(mcbsp, SPCR2));
dev_dbg(mcbsp->dev, "SPCR1: 0x%04x\n",
- OMAP_MCBSP_READ(mcbsp->io_base, SPCR1));
+ OMAP_MCBSP_READ(mcbsp, SPCR1));
dev_dbg(mcbsp->dev, "RCR2: 0x%04x\n",
- OMAP_MCBSP_READ(mcbsp->io_base, RCR2));
+ OMAP_MCBSP_READ(mcbsp, RCR2));
dev_dbg(mcbsp->dev, "RCR1: 0x%04x\n",
- OMAP_MCBSP_READ(mcbsp->io_base, RCR1));
+ OMAP_MCBSP_READ(mcbsp, RCR1));
dev_dbg(mcbsp->dev, "XCR2: 0x%04x\n",
- OMAP_MCBSP_READ(mcbsp->io_base, XCR2));
+ OMAP_MCBSP_READ(mcbsp, XCR2));
dev_dbg(mcbsp->dev, "XCR1: 0x%04x\n",
- OMAP_MCBSP_READ(mcbsp->io_base, XCR1));
+ OMAP_MCBSP_READ(mcbsp, XCR1));
dev_dbg(mcbsp->dev, "SRGR2: 0x%04x\n",
- OMAP_MCBSP_READ(mcbsp->io_base, SRGR2));
+ OMAP_MCBSP_READ(mcbsp, SRGR2));
dev_dbg(mcbsp->dev, "SRGR1: 0x%04x\n",
- OMAP_MCBSP_READ(mcbsp->io_base, SRGR1));
+ OMAP_MCBSP_READ(mcbsp, SRGR1));
dev_dbg(mcbsp->dev, "PCR0: 0x%04x\n",
- OMAP_MCBSP_READ(mcbsp->io_base, PCR0));
+ OMAP_MCBSP_READ(mcbsp, PCR0));
dev_dbg(mcbsp->dev, "***********************\n");
}
@@ -93,15 +104,15 @@ 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 = OMAP_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));
+ OMAP_MCBSP_CWRITE(mcbsp_tx, SPCR2,
+ OMAP_MCBSP_CREAD(mcbsp_tx, SPCR2) & ~(XSYNC_ERR));
} else {
complete(&mcbsp_tx->tx_irq_completion);
}
@@ -114,15 +125,15 @@ 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 = OMAP_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));
+ OMAP_MCBSP_CWRITE(mcbsp_rx, SPCR1,
+ OMAP_MCBSP_CREAD(mcbsp_rx, SPCR1) & ~(RSYNC_ERR));
} else {
complete(&mcbsp_rx->tx_irq_completion);
}
@@ -135,7 +146,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));
+ OMAP_MCBSP_READ(mcbsp_dma_tx, SPCR2));
/* We can free the channels */
omap_free_dma(mcbsp_dma_tx->dma_tx_lch);
@@ -149,7 +160,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));
+ OMAP_MCBSP_READ(mcbsp_dma_rx, SPCR2));
/* We can free the channels */
omap_free_dma(mcbsp_dma_rx->dma_rx_lch);
@@ -167,7 +178,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 +185,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);
+ OMAP_MCBSP_CWRITE(mcbsp, SPCR2, config->spcr2);
+ OMAP_MCBSP_CWRITE(mcbsp, SPCR1, config->spcr1);
+ OMAP_MCBSP_CWRITE(mcbsp, RCR2, config->rcr2);
+ OMAP_MCBSP_CWRITE(mcbsp, RCR1, config->rcr1);
+ OMAP_MCBSP_CWRITE(mcbsp, XCR2, config->xcr2);
+ OMAP_MCBSP_CWRITE(mcbsp, XCR1, config->xcr1);
+ OMAP_MCBSP_CWRITE(mcbsp, SRGR2, config->srgr2);
+ OMAP_MCBSP_CWRITE(mcbsp, SRGR1, config->srgr1);
+ OMAP_MCBSP_CWRITE(mcbsp, MCR2, config->mcr2);
+ OMAP_MCBSP_CWRITE(mcbsp, MCR1, config->mcr1);
+ OMAP_MCBSP_CWRITE(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);
+ OMAP_MCBSP_CWRITE(mcbsp, XCCR, config->xccr);
+ OMAP_MCBSP_CWRITE(mcbsp, RCCR, config->rccr);
}
}
EXPORT_SYMBOL(omap_mcbsp_config);
@@ -207,7 +216,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 +225,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);
+ OMAP_MCBSP_CWRITE(mcbsp, THRSH2, threshold);
}
EXPORT_SYMBOL(omap_mcbsp_set_tx_threshold);
@@ -231,7 +238,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 +247,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);
+ OMAP_MCBSP_CWRITE(mcbsp, THRSH1, threshold);
}
EXPORT_SYMBOL(omap_mcbsp_set_rx_threshold);
@@ -313,19 +318,18 @@ static inline void omap34xx_mcbsp_reques
if (cpu_is_omap34xx()) {
u16 syscon;
- syscon = OMAP_MCBSP_READ(mcbsp->io_base, SYSCON);
+ syscon = OMAP_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);
+ OMAP_MCBSP_CWRITE(mcbsp, WAKEUPEN, XRDYEN | RRDYEN);
} else {
syscon |= SIDLEMODE(0x01);
}
- OMAP_MCBSP_WRITE(mcbsp->io_base, SYSCON, syscon);
+ OMAP_MCBSP_CWRITE(mcbsp, SYSCON, syscon);
}
}
@@ -337,7 +341,7 @@ static inline void omap34xx_mcbsp_free(s
if (cpu_is_omap34xx()) {
u16 syscon;
- syscon = OMAP_MCBSP_READ(mcbsp->io_base, SYSCON);
+ syscon = OMAP_MCBSP_READ(mcbsp, SYSCON);
syscon &= ~(ENAWAKEUP | SIDLEMODE(0x03) | CLOCKACTIVITY(0x03));
/*
* HW bug workaround - If no_idle mode is taken, we need to
@@ -345,12 +349,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);
+ OMAP_MCBSP_CWRITE(mcbsp, SYSCON, syscon);
syscon &= ~(SIDLEMODE(0x03));
- OMAP_MCBSP_WRITE(mcbsp->io_base, SYSCON, syscon);
+ OMAP_MCBSP_CWRITE(mcbsp, SYSCON, syscon);
- OMAP_MCBSP_WRITE(mcbsp->io_base, WAKEUPEN, 0);
+ OMAP_MCBSP_CWRITE(mcbsp, WAKEUPEN, 0);
}
}
#else
@@ -424,8 +428,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);
+ OMAP_MCBSP_CWRITE(mcbsp, SPCR1, 0);
+ OMAP_MCBSP_CWRITE(mcbsp, SPCR2, 0);
if (mcbsp->io_type == OMAP_MCBSP_IRQ_IO) {
/* We need to get IRQs here */
@@ -501,7 +505,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 +513,27 @@ 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 = (OMAP_MCBSP_CREAD(mcbsp, RCR1) >> 5) & 0x7;
+ mcbsp->tx_word_length = (OMAP_MCBSP_CREAD(mcbsp, XCR1) >> 5) & 0x7;
- idle = !((OMAP_MCBSP_READ(io_base, SPCR2) |
- OMAP_MCBSP_READ(io_base, SPCR1)) & 1);
+ idle = !((OMAP_MCBSP_CREAD(mcbsp, SPCR2) |
+ OMAP_MCBSP_CREAD(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 = OMAP_MCBSP_CREAD(mcbsp, SPCR2);
+ OMAP_MCBSP_CWRITE(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 = OMAP_MCBSP_CREAD(mcbsp, SPCR2);
+ OMAP_MCBSP_CWRITE(mcbsp, SPCR2, w | tx);
rx &= 1;
- w = OMAP_MCBSP_READ(io_base, SPCR1);
- OMAP_MCBSP_WRITE(io_base, SPCR1, w | rx);
+ w = OMAP_MCBSP_CREAD(mcbsp, SPCR1);
+ OMAP_MCBSP_CWRITE(mcbsp, SPCR1, w | rx);
/*
* Worst case: CLKSRG*2 = 8000khz: (1/8000) * 2 * 2 usec
@@ -543,18 +545,18 @@ void omap_mcbsp_start(unsigned int id, i
if (idle) {
/* Start frame sync */
- w = OMAP_MCBSP_READ(io_base, SPCR2);
- OMAP_MCBSP_WRITE(io_base, SPCR2, w | (1 << 7));
+ w = OMAP_MCBSP_CREAD(mcbsp, SPCR2);
+ OMAP_MCBSP_CWRITE(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 = OMAP_MCBSP_CREAD(mcbsp, XCCR);
w &= ~(tx ? XDISABLE : 0);
- OMAP_MCBSP_WRITE(io_base, XCCR, w);
- w = OMAP_MCBSP_READ(io_base, RCCR);
+ OMAP_MCBSP_CWRITE(mcbsp, XCCR, w);
+ w = OMAP_MCBSP_CREAD(mcbsp, RCCR);
w &= ~(rx ? RDISABLE : 0);
- OMAP_MCBSP_WRITE(io_base, RCCR, w);
+ OMAP_MCBSP_CWRITE(mcbsp, RCCR, w);
}
/* Dump McBSP Regs */
@@ -565,7 +567,6 @@ EXPORT_SYMBOL(omap_mcbsp_start);
void omap_mcbsp_stop(unsigned int id, int tx, int rx)
{
struct omap_mcbsp *mcbsp;
- void __iomem *io_base;
int idle;
u16 w;
@@ -575,35 +576,34 @@ 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 = OMAP_MCBSP_CREAD(mcbsp, XCCR);
w |= (tx ? XDISABLE : 0);
- OMAP_MCBSP_WRITE(io_base, XCCR, w);
+ OMAP_MCBSP_CWRITE(mcbsp, XCCR, w);
}
- w = OMAP_MCBSP_READ(io_base, SPCR2);
- OMAP_MCBSP_WRITE(io_base, SPCR2, w & ~tx);
+ w = OMAP_MCBSP_CREAD(mcbsp, SPCR2);
+ OMAP_MCBSP_CWRITE(mcbsp, SPCR2, w & ~tx);
/* Reset receiver */
rx &= 1;
if (cpu_is_omap2430() || cpu_is_omap34xx()) {
- w = OMAP_MCBSP_READ(io_base, RCCR);
+ w = OMAP_MCBSP_CREAD(mcbsp, RCCR);
w |= (rx ? RDISABLE : 0);
- OMAP_MCBSP_WRITE(io_base, RCCR, w);
+ OMAP_MCBSP_CWRITE(mcbsp, RCCR, w);
}
- w = OMAP_MCBSP_READ(io_base, SPCR1);
- OMAP_MCBSP_WRITE(io_base, SPCR1, w & ~rx);
+ w = OMAP_MCBSP_CREAD(mcbsp, SPCR1);
+ OMAP_MCBSP_CWRITE(mcbsp, SPCR1, w & ~rx);
- idle = !((OMAP_MCBSP_READ(io_base, SPCR2) |
- OMAP_MCBSP_READ(io_base, SPCR1)) & 1);
+ idle = !((OMAP_MCBSP_CREAD(mcbsp, SPCR2) |
+ OMAP_MCBSP_CREAD(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 = OMAP_MCBSP_CREAD(mcbsp, SPCR2);
+ OMAP_MCBSP_CWRITE(mcbsp, SPCR2, w & ~(1 << 6));
}
}
EXPORT_SYMBOL(omap_mcbsp_stop);
@@ -612,7 +612,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 +619,27 @@ 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(mcbsp, DXR1, buf);
/* if frame sync error - clear the error */
- if (readw(base + OMAP_MCBSP_REG_SPCR2) & XSYNC_ERR) {
+ if (OMAP_MCBSP_READ(mcbsp, SPCR2) & XSYNC_ERR) {
/* clear error */
- writew(readw(base + OMAP_MCBSP_REG_SPCR2) & (~XSYNC_ERR),
- base + OMAP_MCBSP_REG_SPCR2);
+ OMAP_MCBSP_CWRITE(mcbsp, SPCR2,
+ OMAP_MCBSP_CREAD(mcbsp, 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(mcbsp, SPCR2) & XRDY)) {
if (attemps++ > 1000) {
- writew(readw(base + OMAP_MCBSP_REG_SPCR2) &
- (~XRST),
- base + OMAP_MCBSP_REG_SPCR2);
+ OMAP_MCBSP_CWRITE(mcbsp, SPCR2,
+ OMAP_MCBSP_CREAD(mcbsp, SPCR2) &
+ (~XRST));
udelay(10);
- writew(readw(base + OMAP_MCBSP_REG_SPCR2) |
- (XRST),
- base + OMAP_MCBSP_REG_SPCR2);
+ OMAP_MCBSP_CWRITE(mcbsp, SPCR2,
+ OMAP_MCBSP_CREAD(mcbsp, SPCR2) |
+ (XRST));
udelay(10);
dev_err(mcbsp->dev, "Could not write to"
" McBSP%d Register\n", mcbsp->id);
@@ -657,7 +655,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 +662,25 @@ 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 (readw(base + OMAP_MCBSP_REG_SPCR1) & RSYNC_ERR) {
+ if (OMAP_MCBSP_READ(mcbsp, SPCR1) & RSYNC_ERR) {
/* clear error */
- writew(readw(base + OMAP_MCBSP_REG_SPCR1) & (~RSYNC_ERR),
- base + OMAP_MCBSP_REG_SPCR1);
+ OMAP_MCBSP_CWRITE(mcbsp, SPCR1,
+ OMAP_MCBSP_CREAD(mcbsp, 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(mcbsp, SPCR1) & RRDY)) {
if (attemps++ > 1000) {
- writew(readw(base + OMAP_MCBSP_REG_SPCR1) &
- (~RRST),
- base + OMAP_MCBSP_REG_SPCR1);
+ OMAP_MCBSP_CWRITE(mcbsp, SPCR1,
+ OMAP_MCBSP_CREAD(mcbsp, SPCR1) &
+ (~RRST));
udelay(10);
- writew(readw(base + OMAP_MCBSP_REG_SPCR1) |
- (RRST),
- base + OMAP_MCBSP_REG_SPCR1);
+ OMAP_MCBSP_CWRITE(mcbsp, SPCR1,
+ OMAP_MCBSP_CREAD(mcbsp, SPCR1) |
+ (RRST));
udelay(10);
dev_err(mcbsp->dev, "Could not read from"
" McBSP%d Register\n", mcbsp->id);
@@ -692,7 +688,7 @@ int omap_mcbsp_pollread(unsigned int id,
}
}
}
- *buf = readw(base + OMAP_MCBSP_REG_DRR1);
+ *buf = OMAP_MCBSP_READ(mcbsp, DRR1);
return 0;
}
@@ -704,7 +700,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 +708,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);
+ OMAP_MCBSP_WRITE(mcbsp, DXR2, word >> 16);
+ OMAP_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 +731,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 = OMAP_MCBSP_READ(mcbsp, DRR2);
+ word_lsb = OMAP_MCBSP_READ(mcbsp, DRR1);
return (word_lsb | (word_msb << 16));
}
@@ -753,7 +745,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 +754,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 +761,16 @@ 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 = OMAP_MCBSP_READ(mcbsp, SPCR2);
while (!(spcr2 & XRDY)) {
- spcr2 = OMAP_MCBSP_READ(io_base, SPCR2);
+ spcr2 = OMAP_MCBSP_READ(mcbsp, SPCR2);
if (attempts++ > 1000) {
/* We must reset the transmitter */
- OMAP_MCBSP_WRITE(io_base, SPCR2, spcr2 & (~XRST));
+ OMAP_MCBSP_CWRITE(mcbsp, SPCR2,
+ OMAP_MCBSP_CREAD(mcbsp, SPCR2) & (~XRST));
udelay(10);
- OMAP_MCBSP_WRITE(io_base, SPCR2, spcr2 | XRST);
+ OMAP_MCBSP_CWRITE(mcbsp, SPCR2,
+ OMAP_MCBSP_CREAD(mcbsp, SPCR2) | XRST);
udelay(10);
dev_err(mcbsp->dev, "McBSP%d transmitter not "
"ready\n", mcbsp->id);
@@ -788,18 +780,20 @@ 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);
+ OMAP_MCBSP_WRITE(mcbsp, DXR2, word >> 16);
+ OMAP_MCBSP_WRITE(mcbsp, DXR1, word & 0xffff);
/* We wait for the receiver to be ready */
- spcr1 = OMAP_MCBSP_READ(io_base, SPCR1);
+ spcr1 = OMAP_MCBSP_READ(mcbsp, SPCR1);
while (!(spcr1 & RRDY)) {
- spcr1 = OMAP_MCBSP_READ(io_base, SPCR1);
+ spcr1 = OMAP_MCBSP_READ(mcbsp, SPCR1);
if (attempts++ > 1000) {
/* We must reset the receiver */
- OMAP_MCBSP_WRITE(io_base, SPCR1, spcr1 & (~RRST));
+ OMAP_MCBSP_CWRITE(mcbsp, SPCR1,
+ OMAP_MCBSP_CREAD(mcbsp, SPCR1) & (~RRST));
udelay(10);
- OMAP_MCBSP_WRITE(io_base, SPCR1, spcr1 | RRST);
+ OMAP_MCBSP_CWRITE(mcbsp, SPCR1,
+ OMAP_MCBSP_CREAD(mcbsp, SPCR1) | RRST);
udelay(10);
dev_err(mcbsp->dev, "McBSP%d receiver not "
"ready\n", mcbsp->id);
@@ -809,8 +803,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 = OMAP_MCBSP_READ(mcbsp, DRR2);
+ word_lsb = OMAP_MCBSP_READ(mcbsp, DRR1);
return 0;
}
@@ -820,7 +814,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 +824,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 +832,16 @@ 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 = OMAP_MCBSP_READ(mcbsp, SPCR2);
while (!(spcr2 & XRDY)) {
- spcr2 = OMAP_MCBSP_READ(io_base, SPCR2);
+ spcr2 = OMAP_MCBSP_READ(mcbsp, SPCR2);
if (attempts++ > 1000) {
/* We must reset the transmitter */
- OMAP_MCBSP_WRITE(io_base, SPCR2, spcr2 & (~XRST));
+ OMAP_MCBSP_CWRITE(mcbsp, SPCR2,
+ OMAP_MCBSP_CREAD(mcbsp, SPCR2) & (~XRST));
udelay(10);
- OMAP_MCBSP_WRITE(io_base, SPCR2, spcr2 | XRST);
+ OMAP_MCBSP_CWRITE(mcbsp, SPCR2,
+ OMAP_MCBSP_CREAD(mcbsp, SPCR2) | XRST);
udelay(10);
dev_err(mcbsp->dev, "McBSP%d transmitter not "
"ready\n", mcbsp->id);
@@ -857,18 +851,20 @@ 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);
+ OMAP_MCBSP_WRITE(mcbsp, DXR2, clock_word >> 16);
+ OMAP_MCBSP_WRITE(mcbsp, DXR1, clock_word & 0xffff);
/* We wait for the receiver to be ready */
- spcr1 = OMAP_MCBSP_READ(io_base, SPCR1);
+ spcr1 = OMAP_MCBSP_READ(mcbsp, SPCR1);
while (!(spcr1 & RRDY)) {
- spcr1 = OMAP_MCBSP_READ(io_base, SPCR1);
+ spcr1 = OMAP_MCBSP_READ(mcbsp, SPCR1);
if (attempts++ > 1000) {
/* We must reset the receiver */
- OMAP_MCBSP_WRITE(io_base, SPCR1, spcr1 & (~RRST));
+ OMAP_MCBSP_CWRITE(mcbsp, SPCR1,
+ OMAP_MCBSP_CREAD(mcbsp, SPCR1) & (~RRST));
udelay(10);
- OMAP_MCBSP_WRITE(io_base, SPCR1, spcr1 | RRST);
+ OMAP_MCBSP_CWRITE(mcbsp, SPCR1,
+ OMAP_MCBSP_CREAD(mcbsp, SPCR1) | RRST);
udelay(10);
dev_err(mcbsp->dev, "McBSP%d receiver not "
"ready\n", mcbsp->id);
@@ -878,8 +874,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 = OMAP_MCBSP_READ(mcbsp, DRR2);
+ word_lsb = OMAP_MCBSP_READ(mcbsp, DRR1);
word[0] = (word_lsb | (word_msb << 16));
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC][PATCH v2] OMAP: McBSP: Use register cache
2009-11-24 11:31 ` [RFC][PATCH v2] " Janusz Krzysztofik
@ 2009-11-26 8:00 ` Jarkko Nikula
2009-11-26 13:25 ` Janusz Krzysztofik
0 siblings, 1 reply; 15+ messages in thread
From: Jarkko Nikula @ 2009-11-26 8:00 UTC (permalink / raw)
To: Janusz Krzysztofik; +Cc: Tony Lindgren, linux-omap@vger.kernel.org
Hi
On Tue, 24 Nov 2009 12:31:16 +0100
Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> wrote:
> -#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 OMAP_MCBSP_READ(mcbsp, reg) \
> + omap_mcbsp_read(mcbsp->io_base, OMAP_MCBSP_REG_##reg)
> +#define OMAP_MCBSP_WRITE(mcbsp, reg, val) \
> + omap_mcbsp_write(mcbsp->io_base, OMAP_MCBSP_REG_##reg, val)
> +
> +#define OMAP_MCBSP_CREAD(mcbsp, reg) \
> + (mcbsp->reg_cache[OMAP_MCBSP_REG_##reg / OMAP_MCBSP_REG_DRR1])
> +#define OMAP_MCBSP_CWRITE(mcbsp, reg, val) \
> + omap_mcbsp_write(mcbsp->io_base, OMAP_MCBSP_REG_##reg, \
> + mcbsp->reg_cache[OMAP_MCBSP_REG_##reg / OMAP_MCBSP_REG_DRR1] \
> + = val)
> +
Have rather single write writing both the cache and actual register.
I.e. keep the OMAP_MCBSP_WRITE since writes should always go to hw and
makes the patch smaller. I also found the OMAP_MCBSP_CREAD a little
unclear so I suggest to name is as OMAP_MCBSP_READ_CACHE.
> +#define OMAP_MCBSP_BREAD(mcbsp, reg) \
> + (mcbsp->reg_cache[OMAP_MCBSP_REG_##reg / OMAP_MCBSP_REG_DRR1] \
> + = OMAP_MCBSP_READ(mcbsp->io_base, reg))
>
Why is this? There is nothing eating this :-)
> dev_dbg(mcbsp->dev, "DRR2: 0x%04x\n",
> - OMAP_MCBSP_READ(mcbsp->io_base, DRR2));
> + OMAP_MCBSP_READ(mcbsp, DRR2));
These changes are worth to send as a separate cleanup patch. Will make
the actual cache patch smaller and easier to follow.
> @@ -93,15 +104,15 @@ 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 = OMAP_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));
> + OMAP_MCBSP_CWRITE(mcbsp_tx, SPCR2,
> + OMAP_MCBSP_CREAD(mcbsp_tx, SPCR2) & ~(XSYNC_ERR));
I was thinking that should these read+read_cache changes be a separate
patch for fixing the 1510 issues since no other OMAPs are known to
corrupt registers and plain hw read is enough for them.
> @@ -612,7 +612,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 +619,27 @@ 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(mcbsp, DXR1, buf);
> /* if frame sync error - clear the error */
> - if (readw(base + OMAP_MCBSP_REG_SPCR2) & XSYNC_ERR) {
> + if (OMAP_MCBSP_READ(mcbsp, SPCR2) & XSYNC_ERR) {
These looks also that these are better to cover with its own separate
cleanup patch.
---
I'm not completely sure are there any code path expecting to read reset
default values from the cache or are there always explicit write or
read before it? I was thinking would it be necessary to initialize the
cache by issuing dummy reads for all registers. IRCC the OMAP2420 has
fewer registers than 2430 or OMAP3 so that should be took into account
if there is a need to do so.
--
Jarkko
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC][PATCH v2] OMAP: McBSP: Use register cache
2009-11-26 8:00 ` Jarkko Nikula
@ 2009-11-26 13:25 ` Janusz Krzysztofik
2009-11-28 16:14 ` Jarkko Nikula
0 siblings, 1 reply; 15+ messages in thread
From: Janusz Krzysztofik @ 2009-11-26 13:25 UTC (permalink / raw)
To: Jarkko Nikula; +Cc: Tony Lindgren, linux-omap@vger.kernel.org
Thursday 26 November 2009 09:00:00 Jarkko Nikula napisał(a):
> Hi
>
> On Tue, 24 Nov 2009 12:31:16 +0100
>
> Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> wrote:
> > -#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 OMAP_MCBSP_READ(mcbsp, reg) \
> > + omap_mcbsp_read(mcbsp->io_base, OMAP_MCBSP_REG_##reg)
> > +#define OMAP_MCBSP_WRITE(mcbsp, reg, val) \
> > + omap_mcbsp_write(mcbsp->io_base, OMAP_MCBSP_REG_##reg, val)
> > +
> > +#define OMAP_MCBSP_CREAD(mcbsp, reg) \
> > + (mcbsp->reg_cache[OMAP_MCBSP_REG_##reg / OMAP_MCBSP_REG_DRR1])
> > +#define OMAP_MCBSP_CWRITE(mcbsp, reg, val) \
> > + omap_mcbsp_write(mcbsp->io_base, OMAP_MCBSP_REG_##reg, \
> > + mcbsp->reg_cache[OMAP_MCBSP_REG_##reg / OMAP_MCBSP_REG_DRR1] \
> > + = val)
> > +
>
> Have rather single write writing both the cache and actual register.
> I.e. keep the OMAP_MCBSP_WRITE since writes should always go to hw and
> makes the patch smaller.
Jarkko,
Not that I am against making things simpler, but please reconsider:
1. Is there any point for caching data written to DXR registers?
2. There is at least one register, WAKEUPEN, never updated bit by bit, but
always rewritten from scratch with a few different values that can be
computed easily from other msbsp structure member values. For these rare
cases, is it any worth to bypass the cache while writing them, saving a few
instructions maybe?
3. Sometimes a register is written several times in succession with different
values, like SYSCON inside omap34xx_mcbsp_free(), for example. Could all
writes but the last one bypass the cache for the same reason?
Another axample is omap_mcbsp_start() fiddling with SPCR2 several times.
However, there is a udelay(500) inbetween that I am not sure if can ever
happen to be interleaved with a concurrent suspend or idle or something like
that requiring the cache being up-to-date.
That said, please confirm if a single write macro with caching still seems
sufficient, or a separate one without caching would provide enough benefits
to keep it as well.
> I also found the OMAP_MCBSP_CREAD a little
> unclear so I suggest to name is as OMAP_MCBSP_READ_CACHE.
I just tried to minimize line wrapping :/. If we agree on a single caching
version of OMAP_MCBSP_WRITE(), what about OMAP_CACHE_READ()? If no better
ideas, I'll use what you suggest.
> > +#define OMAP_MCBSP_BREAD(mcbsp, reg) \
> > + (mcbsp->reg_cache[OMAP_MCBSP_REG_##reg / OMAP_MCBSP_REG_DRR1] \
> > + = OMAP_MCBSP_READ(mcbsp->io_base, reg))
>
> Why is this? There is nothing eating this :-)
It's a part of a possible solution I am considering for SYSCON register
handling. See below.
> > dev_dbg(mcbsp->dev, "DRR2: 0x%04x\n",
> > - OMAP_MCBSP_READ(mcbsp->io_base, DRR2));
> > + OMAP_MCBSP_READ(mcbsp, DRR2));
>
> These changes are worth to send as a separate cleanup patch. Will make
> the actual cache patch smaller and easier to follow.
OK, but let me recap that first. We have two options:
1. All macros accept very same arguments, as long as applicable.
This is what I have put in my patch.
2. Every macro is optimized in respect of its argumnets, ie.
OMAP_MCBSP_READ(io_base, ...)
OMAP_MCBSP_READ_CACHE(reg_cache, ...)
OMAP_MCBSP_WRITE(mcbsp, ...)
Which one do you think is better?
> > @@ -93,15 +104,15 @@ 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 = OMAP_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));
> > + OMAP_MCBSP_CWRITE(mcbsp_tx, SPCR2,
> > + OMAP_MCBSP_CREAD(mcbsp_tx, SPCR2) & ~(XSYNC_ERR));
>
> I was thinking that should these read+read_cache changes be a separate
> patch for fixing the 1510 issues since no other OMAPs are known to
> corrupt registers and plain hw read is enough for them.
I see. Do you want the change limited to cpu_is_omap1510() case only?
> > @@ -612,7 +612,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 +619,27 @@ 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(mcbsp, DXR1, buf);
> > /* if frame sync error - clear the error */
> > - if (readw(base + OMAP_MCBSP_REG_SPCR2) & XSYNC_ERR) {
> > + if (OMAP_MCBSP_READ(mcbsp, SPCR2) & XSYNC_ERR) {
>
> These looks also that these are better to cover with its own separate
> cleanup patch.
Ok, the series will start with this cleanup therefore.
> I'm not completely sure are there any code path expecting to read reset
> default values from the cache or are there always explicit write or
> read before it? I was thinking would it be necessary to initialize the
> cache by issuing dummy reads for all registers. IRCC the OMAP2420 has
> fewer registers than 2430 or OMAP3 so that should be took into account
> if there is a need to do so.
First of all, I decided not touching any registers not maintained by the code
so far. I have not enough knowledge to deal with them unless someone decides
to shepherd me there :-).
The only issue I am not satisfied how I have dealt with so far is SYSCON
register case. I have noticed that the register is never initialized like
others , but always updated based on values read back from hardware.
Initially I didn't find any good idea how to deal with this in respect of
caching, so decided to take an intermediate path: read from hardware, then
write with caching, even if the cached value is never referred for now. Here
is the relevant part of my patch:
@@ -313,19 +318,18 @@ static inline void omap34xx_mcbsp_reques
if (cpu_is_omap34xx()) {
u16 syscon;
- syscon = OMAP_MCBSP_READ(mcbsp->io_base, SYSCON);
+ syscon = OMAP_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);
+ OMAP_MCBSP_CWRITE(mcbsp, WAKEUPEN, XRDYEN | RRDYEN);
} else {
syscon |= SIDLEMODE(0x01);
}
- OMAP_MCBSP_WRITE(mcbsp->io_base, SYSCON, syscon);
+ OMAP_MCBSP_CWRITE(mcbsp, SYSCON, syscon);
}
}
@@ -337,7 +341,7 @@ static inline void omap34xx_mcbsp_free(s
if (cpu_is_omap34xx()) {
u16 syscon;
- syscon = OMAP_MCBSP_READ(mcbsp->io_base, SYSCON);
+ syscon = OMAP_MCBSP_READ(mcbsp, SYSCON);
syscon &= ~(ENAWAKEUP | SIDLEMODE(0x03) | CLOCKACTIVITY(0x03));
/*
* HW bug workaround - If no_idle mode is taken, we need to
@@ -345,12 +349,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);
+ OMAP_MCBSP_CWRITE(mcbsp, SYSCON, syscon);
syscon &= ~(SIDLEMODE(0x03));
- OMAP_MCBSP_WRITE(mcbsp->io_base, SYSCON, syscon);
+ OMAP_MCBSP_CWRITE(mcbsp, SYSCON, syscon);
- OMAP_MCBSP_WRITE(mcbsp->io_base, WAKEUPEN, 0);
+ OMAP_MCBSP_CWRITE(mcbsp, WAKEUPEN, 0);
}
}
#else
How about caching the SYSCON register initial value from inside
omap_mcbsp_config() by copying it's initial value from the hardware with
something like that OMAP_MCBSP_BREAD() macro not eaten so far (probably
renamed to OMAP_CACHE_INIT/COPY() or OMAP_MCBSP_SAVE/STORE() or something
similiar)? Would that be enough for addressing your concern?
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC][PATCH v2] OMAP: McBSP: Use register cache
2009-11-26 13:25 ` Janusz Krzysztofik
@ 2009-11-28 16:14 ` Jarkko Nikula
0 siblings, 0 replies; 15+ messages in thread
From: Jarkko Nikula @ 2009-11-28 16:14 UTC (permalink / raw)
To: Janusz Krzysztofik; +Cc: Tony Lindgren, linux-omap@vger.kernel.org
Hi
On Thu, 26 Nov 2009 14:25:36 +0100
Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> wrote:
> 1. Is there any point for caching data written to DXR registers?
>
> 2. There is at least one register, WAKEUPEN, never updated bit by bit, but
> always rewritten from scratch with a few different values that can be
> computed easily from other msbsp structure member values. For these rare
> cases, is it any worth to bypass the cache while writing them, saving a few
> instructions maybe?
>
I would say it's better to favor simplicity and uniformity first.
Trying to avoid caching for few registers makes the cache code harder to
follow, more error prone and may not save the total memory footprint.
> 3. Sometimes a register is written several times in succession with different
> values, like SYSCON inside omap34xx_mcbsp_free(), for example. Could all
> writes but the last one bypass the cache for the same reason?
>
> Another axample is omap_mcbsp_start() fiddling with SPCR2 several times.
> However, there is a udelay(500) inbetween that I am not sure if can ever
> happen to be interleaved with a concurrent suspend or idle or something like
> that requiring the cache being up-to-date.
>
I'm not sure about SYSCON but there are registers like SPCR2 where
specifications and practical findings define the minimum times between
activating the logic so caching must not optimize those away.
> That said, please confirm if a single write macro with caching still seems
> sufficient, or a separate one without caching would provide enough benefits
> to keep it as well.
>
Single write with caching would be sufficient I would say.
> I just tried to minimize line wrapping :/. If we agree on a single caching
> version of OMAP_MCBSP_WRITE(), what about OMAP_CACHE_READ()? If no better
> ideas, I'll use what you suggest.
>
Ok, I see. How about these? OMAP is obvious in this case so we can drop
it off from macro names.
MCBSP_WRITE
MCBSP_READ
MCBSP_READ_CACHE
> 1. All macros accept very same arguments, as long as applicable.
> This is what I have put in my patch.
>
Definitely this. Pass instance of "struct omap_mcbsp" as you did since
all other data is found from there.
> > I was thinking that should these read+read_cache changes be a separate
> > patch for fixing the 1510 issues since no other OMAPs are known to
> > corrupt registers and plain hw read is enough for them.
>
> I see. Do you want the change limited to cpu_is_omap1510() case only?
>
I think that's not necessary. The separate patch would explain that the
change is fixing 1510 issue and the code is fine for other OMAPs as
well.
> The only issue I am not satisfied how I have dealt with so far is SYSCON
> register case. I have noticed that the register is never initialized like
> others , but always updated based on values read back from hardware.
> Initially I didn't find any good idea how to deal with this in respect of
> caching, so decided to take an intermediate path: read from hardware, then
> write with caching, even if the cached value is never referred for now. Here
> is the relevant part of my patch:
>
Probably simplest and safest is to do so that one patch will change the
macro arguments and rename them, another introducing caching with
adding _READ_CACHE and then one for 1510 fix. Then the code behaves the
same and more _READ_CACHE changes can be introduced gradually by
anothers if needed.
--
Jarkko
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2009-11-28 16:13 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-08-12 10:39 [RFC][PATCH] ARM: OMAP: McBSP: Use register cache Janusz Krzysztofik
2009-08-28 11:26 ` Janusz Krzysztofik
2009-11-14 2:06 ` Janusz Krzysztofik
2009-11-23 20:42 ` Janusz Krzysztofik
2009-11-23 23:53 ` Tony Lindgren
2009-11-24 7:59 ` Jarkko Nikula
2009-11-24 11:31 ` [RFC][PATCH v2] " Janusz Krzysztofik
2009-11-26 8:00 ` Jarkko Nikula
2009-11-26 13:25 ` Janusz Krzysztofik
2009-11-28 16:14 ` Jarkko Nikula
-- strict thread matches above, loose matches on Subject: below --
2009-09-04 5:39 [RFC][PATCH] ARM: " Eero Nurkkala
2009-09-04 10:34 ` Janusz Krzysztofik
2009-09-04 11:07 ` Eero Nurkkala
2009-09-05 17:31 ` Anuj Aggarwal
2009-09-05 20:46 ` ext-Eero.Nurkkala
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox