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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



(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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux