Re: [RFC][PATCH] ARM: OMAP: McBSP: Use register cache

From: Janusz Krzysztofik
Date: Fri Nov 13 2009 - 21:07:48 EST

Next message: Herbert Xu: "Re: [RFC PATCH] net: add dataref destructor to sk_buff"
Previous message: Alexander Holler: "Re: Problems with p55-express and sleeping usb-hds"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Next message: Herbert Xu: "Re: [RFC PATCH] net: add dataref destructor to sk_buff"
Previous message: Alexander Holler: "Re: Problems with p55-express and sleeping usb-hds"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]