[U-Boot] [PATCH] OMAP3 SPI : Fixed bugs related to SPI transfer

From: ajoy akdas75@yahoo.in
Added posted writes (read after writes) to effect the change immediately for channel confiuration and channel enable register
Disable the channel to purge receieve data in TX_ONLY mode transfer otherwise rx data will get affected by the next immediate RX_ONLY mode transfer
Wait for the EOT bit to be set after last byte has been loaded to TX shift register in the the TX_ONLY mode.This ensures TX data has been completely shifted out
Disable the channel in RX_ONLY mode before reading the last data from RXX register to prevent the SPI slave to transmit next word
Signed-off-by: Ajoy Kumar Das akdas75@yahoo.in Cc: Tom Rini trini@ti.com Cc: jacopo mondi j.mondi@voltaelectronics.com --- drivers/spi/omap3_spi.c | 84 +++++++++++++++++++++++++++-------------------- drivers/spi/omap3_spi.h | 1 + 2 files changed, 49 insertions(+), 36 deletions(-)
diff --git a/drivers/spi/omap3_spi.c b/drivers/spi/omap3_spi.c index 6791a7e..dd4ae7c 100644 --- a/drivers/spi/omap3_spi.c +++ b/drivers/spi/omap3_spi.c @@ -57,6 +57,20 @@ static void spi_reset(struct omap3_spi_slave *ds) writel(OMAP3_MCSPI_WAKEUPENABLE_WKEN, &ds->regs->wakeupenable); }
+static void omap3_spi_write_chconf(struct omap3_spi_slave *ds, int val) +{ + writel(val, &ds->regs->channel[ds->slave.cs].chconf); + /* Flash post writes to make immediate effect */ + readl(&ds->regs->channel[ds->slave.cs].chconf); +} + +static void omap3_spi_set_enable(struct omap3_spi_slave *ds, int enable) +{ + writel(enable, &ds->regs->channel[ds->slave.cs].chctrl); + /* Flash post writes to make immediate effect */ + readl(&ds->regs->channel[ds->slave.cs].chctrl); +} + void spi_init() { /* do nothing */ @@ -211,8 +225,8 @@ int spi_claim_bus(struct spi_slave *slave)
/* Transmit & receive mode */ conf &= ~OMAP3_MCSPI_CHCONF_TRM_MASK; - - writel(conf, &ds->regs->channel[ds->slave.cs].chconf); + + omap3_spi_write_chconf(ds,conf);
return 0; } @@ -233,14 +247,13 @@ int omap3_spi_write(struct spi_slave *slave, unsigned int len, const u8 *txp, int timeout = SPI_WAIT_TIMEOUT; int chconf = readl(&ds->regs->channel[ds->slave.cs].chconf);
- if (flags & SPI_XFER_BEGIN) - writel(OMAP3_MCSPI_CHCTRL_EN, - &ds->regs->channel[ds->slave.cs].chctrl); + /* Enable the channel */ + omap3_spi_set_enable(ds,OMAP3_MCSPI_CHCTRL_EN);
chconf &= ~OMAP3_MCSPI_CHCONF_TRM_MASK; chconf |= OMAP3_MCSPI_CHCONF_TRM_TX_ONLY; chconf |= OMAP3_MCSPI_CHCONF_FORCE; - writel(chconf, &ds->regs->channel[ds->slave.cs].chconf); + omap3_spi_write_chconf(ds,chconf);
for (i = 0; i < len; i++) { /* wait till TX register is empty (TXS == 1) */ @@ -256,15 +269,17 @@ int omap3_spi_write(struct spi_slave *slave, unsigned int len, const u8 *txp, writel(txp[i], &ds->regs->channel[ds->slave.cs].tx); }
+ /* wait to finish of transfer */ + while (!(readl(&ds->regs->channel[ds->slave.cs].chstat) & + OMAP3_MCSPI_CHSTAT_EOT)); + + /* Disable the channel otherwise the next immediate RX will get affected */ + omap3_spi_set_enable(ds,OMAP3_MCSPI_CHCTRL_DIS); + if (flags & SPI_XFER_END) { - /* wait to finish of transfer */ - while (!(readl(&ds->regs->channel[ds->slave.cs].chstat) & - OMAP3_MCSPI_CHSTAT_EOT));
chconf &= ~OMAP3_MCSPI_CHCONF_FORCE; - writel(chconf, &ds->regs->channel[ds->slave.cs].chconf); - - writel(0, &ds->regs->channel[ds->slave.cs].chctrl); + omap3_spi_write_chconf(ds,chconf); } return 0; } @@ -277,15 +292,14 @@ int omap3_spi_read(struct spi_slave *slave, unsigned int len, u8 *rxp, int timeout = SPI_WAIT_TIMEOUT; int chconf = readl(&ds->regs->channel[ds->slave.cs].chconf);
- if (flags & SPI_XFER_BEGIN) - writel(OMAP3_MCSPI_CHCTRL_EN, - &ds->regs->channel[ds->slave.cs].chctrl); + /* Enable the channel */ + omap3_spi_set_enable(ds,OMAP3_MCSPI_CHCTRL_EN);
chconf &= ~OMAP3_MCSPI_CHCONF_TRM_MASK; chconf |= OMAP3_MCSPI_CHCONF_TRM_RX_ONLY; chconf |= OMAP3_MCSPI_CHCONF_FORCE; - writel(chconf, &ds->regs->channel[ds->slave.cs].chconf); - + omap3_spi_write_chconf(ds,chconf); + writel(0, &ds->regs->channel[ds->slave.cs].tx);
for (i = 0; i < len; i++) { @@ -298,15 +312,18 @@ int omap3_spi_read(struct spi_slave *slave, unsigned int len, u8 *rxp, return -1; } } + + /* Disable the channel to prevent furher receiving */ + if(i == (len - 1)) + omap3_spi_set_enable(ds,OMAP3_MCSPI_CHCTRL_DIS); + /* Read the data */ rxp[i] = readl(&ds->regs->channel[ds->slave.cs].rx); }
if (flags & SPI_XFER_END) { chconf &= ~OMAP3_MCSPI_CHCONF_FORCE; - writel(chconf, &ds->regs->channel[ds->slave.cs].chconf); - - writel(0, &ds->regs->channel[ds->slave.cs].chctrl); + omap3_spi_write_chconf(ds,chconf); }
return 0; @@ -323,15 +340,13 @@ int omap3_spi_txrx(struct spi_slave *slave, int i=0;
/*Enable SPI channel*/ - if (flags & SPI_XFER_BEGIN) - writel(OMAP3_MCSPI_CHCTRL_EN, - &ds->regs->channel[ds->slave.cs].chctrl); - + omap3_spi_set_enable(ds,OMAP3_MCSPI_CHCTRL_EN); + /*set TRANSMIT-RECEIVE Mode*/ chconf &= ~OMAP3_MCSPI_CHCONF_TRM_MASK; chconf |= OMAP3_MCSPI_CHCONF_FORCE; - writel(chconf, &ds->regs->channel[ds->slave.cs].chconf); - + omap3_spi_write_chconf(ds,chconf); + /*Shift in and out 1 byte at time*/ for (i=0; i < len; i++){ /* Write: wait for TX empty (TXS == 1)*/ @@ -359,13 +374,13 @@ int omap3_spi_txrx(struct spi_slave *slave, /* Read the data */ rxp[i] = readl(&ds->regs->channel[ds->slave.cs].rx); } + /* Disable the channel */ + omap3_spi_set_enable(ds,OMAP3_MCSPI_CHCTRL_DIS);
/*if transfer must be terminated disable the channel*/ if (flags & SPI_XFER_END) { chconf &= ~OMAP3_MCSPI_CHCONF_FORCE; - writel(chconf, &ds->regs->channel[ds->slave.cs].chconf); - - writel(0, &ds->regs->channel[ds->slave.cs].chctrl); + omap3_spi_write_chconf(ds,chconf); }
return 0; @@ -389,17 +404,14 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, int chconf = readl(&ds->regs->channel[ds->slave.cs].chconf);
if (flags & SPI_XFER_BEGIN) { - writel(OMAP3_MCSPI_CHCTRL_EN, - &ds->regs->channel[ds->slave.cs].chctrl); + omap3_spi_set_enable(ds,OMAP3_MCSPI_CHCTRL_EN); chconf |= OMAP3_MCSPI_CHCONF_FORCE; - writel(chconf, - &ds->regs->channel[ds->slave.cs].chconf); + omap3_spi_write_chconf(ds,chconf); } if (flags & SPI_XFER_END) { chconf &= ~OMAP3_MCSPI_CHCONF_FORCE; - writel(chconf, - &ds->regs->channel[ds->slave.cs].chconf); - writel(0, &ds->regs->channel[ds->slave.cs].chctrl); + omap3_spi_write_chconf(ds,chconf); + omap3_spi_set_enable(ds,OMAP3_MCSPI_CHCTRL_DIS); } ret = 0; } else { diff --git a/drivers/spi/omap3_spi.h b/drivers/spi/omap3_spi.h index bffa43c..5e00208 100644 --- a/drivers/spi/omap3_spi.h +++ b/drivers/spi/omap3_spi.h @@ -99,6 +99,7 @@ struct mcspi { #define OMAP3_MCSPI_CHSTAT_EOT (1 << 2)
#define OMAP3_MCSPI_CHCTRL_EN (1 << 0) +#define OMAP3_MCSPI_CHCTRL_DIS (0 << 0)
#define OMAP3_MCSPI_WAKEUPENABLE_WKEN (1 << 0)

"Ajoy" == Ajoy Kumar Das akdas75@yahoo.in writes:
Ajoy> From: ajoy akdas75@yahoo.in Ajoy> Added posted writes (read after writes) to effect the Ajoy> change immediately for channel confiuration and channel Ajoy> enable register
Ajoy> Disable the channel to purge receieve data in TX_ONLY Ajoy> mode transfer otherwise rx data will get affected by Ajoy> the next immediate RX_ONLY mode transfer
Ajoy> Wait for the EOT bit to be set after last byte has been Ajoy> loaded to TX shift register in the the TX_ONLY mode.This Ajoy> ensures TX data has been completely shifted out
Ajoy> Disable the channel in RX_ONLY mode before reading the Ajoy> last data from RXX register to prevent the SPI slave Ajoy> to transmit next word
So it is 4 seperate fixes? Could you please split it up in 4 seperate patches?

Hi Peter I described the entire patch in small paragraphs so it is better to understand The first paragraph is for posted writes (courtesy linux OMAP3 SPI driver code)
The last three paragraphs actually fixes the bug related to the operation of the OMAP3 SPI controller in TX_ONLY and RX_ONLY mode. Only after implementing the those three things 'together' the TX_ONLY and RX_ONLY transfers works perfectly. The problem occurs when the RX transfer is immediately followed by the TX transfer.
So shall I divide them in four patches according to the paragraphs Or shall I make two patches according to the functionality getting fixed
Please suggest
Ajoy
________________________________ From: Peter Korsgaard jacmet@sunsite.dk To: Ajoy Kumar Das akdas75@yahoo.in Cc: u-boot@lists.denx.de; Tom Rini trini@ti.com; jacopo mondi j.mondi@voltaelectronics.com Sent: Sunday, 18 November 2012 5:23 AM Subject: Re: [U-Boot] [PATCH] OMAP3 SPI : Fixed bugs related to SPI transfer
"Ajoy" == Ajoy Kumar Das akdas75@yahoo.in writes:
Ajoy> From: ajoy akdas75@yahoo.in Ajoy> Added posted writes (read after writes) to effect the Ajoy> change immediately for channel confiuration and channel Ajoy> enable register
Ajoy> Disable the channel to purge receieve data in TX_ONLY Ajoy> mode transfer otherwise rx data will get affected by Ajoy> the next immediate RX_ONLY mode transfer
Ajoy> Wait for the EOT bit to be set after last byte has been Ajoy> loaded to TX shift register in the the TX_ONLY mode.This Ajoy> ensures TX data has been completely shifted out
Ajoy> Disable the channel in RX_ONLY mode before reading the Ajoy> last data from RXX register to prevent the SPI slave Ajoy> to transmit next word
So it is 4 separate fixes? Could you please split it up in 4 separate patches?

On Sat, Nov 17, 2012 at 09:10:15PM -0000, Ajoy Kumar Das wrote:
From: ajoy akdas75@yahoo.in
Added posted writes (read after writes) to effect the change immediately for channel confiuration and channel enable register
Disable the channel to purge receieve data in TX_ONLY mode transfer otherwise rx data will get affected by the next immediate RX_ONLY mode transfer
Wait for the EOT bit to be set after last byte has been loaded to TX shift register in the the TX_ONLY mode.This ensures TX data has been completely shifted out
Disable the channel in RX_ONLY mode before reading the last data from RXX register to prevent the SPI slave to transmit next word
Signed-off-by: Ajoy Kumar Das akdas75@yahoo.in Cc: Tom Rini trini@ti.com Cc: jacopo mondi j.mondi@voltaelectronics.com
Applied to u-boot-ti/master, thanks!
participants (4)
-
Ajoy Das
-
Ajoy Kumar Das
-
Peter Korsgaard
-
Tom Rini