[U-Boot] convention for SPI dummy data?
Hi,
I have finally got MMC/SD working on my coldfire board and would like to send some patches.
However, one of the things I had to change was the dummy data sent out by SPI for read-only transactions. The original driver had all zeros, for SD/MMC all ones (0xFF) is needed.
Is such a change acceptable, or is there any configuration option/flag I could use?
Regards, Wolfgang
On Friday 23 April 2010 04:43:07 Wolfgang Wegner wrote:
However, one of the things I had to change was the dummy data sent out by SPI for read-only transactions. The original driver had all zeros, for SD/MMC all ones (0xFF) is needed.
Is such a change acceptable, or is there any configuration option/flag I could use?
if it isnt part of the SPI/MMC spec, use a config option named like IDLE_VAL. changing the default to 0xff is OK i think. -mike
Hi Mike,
On Fri, Apr 23, 2010 at 11:08:10AM -0400, Mike Frysinger wrote:
On Friday 23 April 2010 04:43:07 Wolfgang Wegner wrote:
However, one of the things I had to change was the dummy data sent out by SPI for read-only transactions. The original driver had all zeros, for SD/MMC all ones (0xFF) is needed.
Is such a change acceptable, or is there any configuration option/flag I could use?
if it isnt part of the SPI/MMC spec, use a config option named like IDLE_VAL.
the problem exists in the (coldfire) SPI driver, not in the MMC/SD code. (For SD, the spec IMHO clearly states 0xFF for all idle transfers.)
changing the default to 0xff is OK i think.
I could add CONFIG_SPI_IDLE_VAL and default it to 0x0 in the coldfire SPI driver. In case one wants to use MMC/SD with this driver, one could then add CONFIG_SPI_IDLE_VAL as 0xFF in the board to override it. (Only pitfall is that the current 0x0 is used for 8- as well as 16-bit transfers...)
ok?
-mike
Regards, Wolfgang
On Friday 23 April 2010 11:18:29 Wolfgang Wegner wrote:
On Fri, Apr 23, 2010 at 11:08:10AM -0400, Mike Frysinger wrote:
On Friday 23 April 2010 04:43:07 Wolfgang Wegner wrote:
However, one of the things I had to change was the dummy data sent out by SPI for read-only transactions. The original driver had all zeros, for SD/MMC all ones (0xFF) is needed.
Is such a change acceptable, or is there any configuration option/flag I could use?
if it isnt part of the SPI/MMC spec, use a config option named like IDLE_VAL.
the problem exists in the (coldfire) SPI driver, not in the MMC/SD code.
sorry, i thought you were proposing to fix it in the SPI/MMC driver
(For SD, the spec IMHO clearly states 0xFF for all idle transfers.)
thanks, wasnt aware
changing the default to 0xff is OK i think.
I could add CONFIG_SPI_IDLE_VAL and default it to 0x0 in the coldfire SPI driver. In case one wants to use MMC/SD with this driver, one could then add CONFIG_SPI_IDLE_VAL as 0xFF in the board to override it. (Only pitfall is that the current 0x0 is used for 8- as well as 16-bit transfers...)
what i've seen so far is that the idle val doesnt matter to most spi devices. so on the Blackfin side, we've been defaulting to 0xff so that things do work "out of the box" for everyone (at least so far). -mike
On Fri, Apr 23, 2010 at 11:34:53AM -0400, Mike Frysinger wrote: [...]
the problem exists in the (coldfire) SPI driver, not in the MMC/SD code.
sorry, i thought you were proposing to fix it in the SPI/MMC driver
no, as far as I can see it is always set to 0xFF there if set explicitly, i.e. except for the read-only transfers.
what i've seen so far is that the idle val doesnt matter to most spi devices. so on the Blackfin side, we've been defaulting to 0xff so that things do work "out of the box" for everyone (at least so far).
Sounds good - however I had just sent a patch implementing CONFIG_SPI_IDLE_VAL.
(I am quite hesitating to simply change such a basic default value because I fear there might be other devices around relying on current behaviour. On the other hand, my implementation adds 7 lines of preprocessor stuff, which also does not look really good.)
Regards, Wolfgang
This patch adds CONFIG_SPI_IDLE_VAL to cf_spi.c The default setting is 0x0 to behave same as current version, in case CONFIG_SPI_MMC is set, the value is set to 0xFFFF (all ones). In either case, the value can be overwritten by board configuration.
Signed-off-by: Wolfgang Wegner w.wegner@astro-kom.de --- drivers/spi/cf_spi.c | 14 +++++++++++--- 1 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/spi/cf_spi.c b/drivers/spi/cf_spi.c index 8cc1d03..7b26f2a 100644 --- a/drivers/spi/cf_spi.c +++ b/drivers/spi/cf_spi.c @@ -49,6 +49,14 @@ extern void cfspi_release_bus(uint bus, uint cs);
DECLARE_GLOBAL_DATA_PTR;
+#ifndef CONFIG_SPI_IDLE_VAL +#if defined(CONFIG_SPI_MMC) +#define CONFIG_SPI_IDLE_VAL 0xFFFF +#else +#define CONFIG_SPI_IDLE_VAL 0x0 +#endif +#endif + #if defined(CONFIG_CF_DSPI) /* DSPI specific mode */ #define SPI_MODE_MOD 0x00200000 @@ -145,7 +153,7 @@ int cfspi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, }
if (din != NULL) { - cfspi_tx(ctrl, 0); + cfspi_tx(ctrl, CONFIG_SPI_IDLE_VAL); if (cfslave->charbit == 16) *spi_rd16++ = cfspi_rx(); else @@ -169,7 +177,7 @@ int cfspi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, }
if (din != NULL) { - cfspi_tx(ctrl, 0); + cfspi_tx(ctrl, CONFIG_SPI_IDLE_VAL); if (cfslave->charbit == 16) *spi_rd16 = cfspi_rx(); else @@ -177,7 +185,7 @@ int cfspi_xfer(struct spi_slave *slave, uint bitlen, const void *dout, } } else { /* dummy read */ - cfspi_tx(ctrl, 0); + cfspi_tx(ctrl, CONFIG_SPI_IDLE_VAL); cfspi_rx(); }
Wolfgang Wegner <w.wegner <at> astro-kom.de> writes:
This patch adds CONFIG_SPI_IDLE_VAL to cf_spi.c The default setting is 0x0 to behave same as current version, in case CONFIG_SPI_MMC is set, the value is set to 0xFFFF (all ones). In either case, the value can be overwritten by board configuration.
Signed-off-by: Wolfgang Wegner <w.wegner <at> astro-kom.de>
Dear Wolfgang,
I am about to add support for SPI to SD/MMC to my Coldfire board (MCF5235 based) both for das u-boot and for uclinux. Do you have any pointers and/or code you can assist with? Your words 'I have finally got MMC/SD working on my coldfire board' is exciting.
Thanks for any help in advance.
Best regards, Terje B. Nilsen
participants (4)
-
Mike Frysinger -
Terje B. Nilsen -
Wolfgang Wegner -
wolfgang＠leila.ping.de