Patchwork [3/3] powerpc/86xx: Add MMC SPI support for MPC8610HPCD boards

login
register
mail settings
Submitter Anton Vorontsov
Date April 4, 2009, 7:28 p.m.
Message ID <20090404192826.GC12183@oksana.dev.rtsoft.ru>
Download mbox | patch
Permalink /patch/25598/
State Superseded
Delegated to: Kumar Gala
Headers show

Comments

Anton Vorontsov - April 4, 2009, 7:28 p.m.
This patch adds spi and mmc-spi-slot nodes, plus a gpio-controller
for PIXIS' sdcsr bank that is used for managing SPI chip-select and
for reading card's states.

Note that spi-max-frequency is lowered since at high frequencies SD
cards don't work reliably (there is some problem with the chip select
line, it's probably quite slow because it's routed via PIXIS).

Previously there was a work around: we didn't use chip-select line
at all, but some cards don't like that, so we'd better use the low
frequency.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 arch/powerpc/boot/dts/mpc8610_hpcd.dts     |   32 ++++++++++++++++++++++++++++
 arch/powerpc/platforms/86xx/mpc8610_hpcd.c |    4 +++
 2 files changed, 36 insertions(+), 0 deletions(-)
H. Peter Anvin - April 5, 2009, 12:38 a.m.
Anton Vorontsov wrote:
> This patch adds spi and mmc-spi-slot nodes, plus a gpio-controller
> for PIXIS' sdcsr bank that is used for managing SPI chip-select and
> for reading card's states.
> 
> Note that spi-max-frequency is lowered since at high frequencies SD
> cards don't work reliably (there is some problem with the chip select
> line, it's probably quite slow because it's routed via PIXIS).
> 
> Previously there was a work around: we didn't use chip-select line
> at all, but some cards don't like that, so we'd better use the low
> frequency.
> 
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>

I have to admit being somewhat confused.  The CS# line is pulled low and
stays low for the duration of a command, so it shouldn't affect the SPI
frequency, only the delay between commands.

You're saying "some cards don't like not using the CS# line" ... in
particular, I know there are hardware devices which toggle the clock at
low frequency (< 400 kHz) for a while (I think the minimum is 88 clocks
or so) and then permanently assert the CS# line before giving the
starting CMD0.  Even so, the CMD0 is supposed to be sent at < 400 kHz
since the card will still be in open drain mode at that point.

Does that disagree with your observations?  If so, I'd be really
interested to find out what you have seen in more detail, as it adds to
the understanding of MMC/SD cards in the field.

	-hpa
Anton Vorontsov - April 30, 2009, 8:51 p.m.
Sorry for the late response,

On Sat, Apr 04, 2009 at 05:38:44PM -0700, H. Peter Anvin wrote:
> Anton Vorontsov wrote:
> > This patch adds spi and mmc-spi-slot nodes, plus a gpio-controller
> > for PIXIS' sdcsr bank that is used for managing SPI chip-select and
> > for reading card's states.
> > 
> > Note that spi-max-frequency is lowered since at high frequencies SD
> > cards don't work reliably (there is some problem with the chip select
> > line, it's probably quite slow because it's routed via PIXIS).
> > 
> > Previously there was a work around: we didn't use chip-select line
> > at all, but some cards don't like that, so we'd better use the low
> > frequency.
> > 
> > Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> 
> I have to admit being somewhat confused.  The CS# line is pulled low and
> stays low for the duration of a command, so it shouldn't affect the SPI
> frequency, only the delay between commands.

Yes. Regarding delay between commands, inserting a 50 ns delay after
asserting CS makes the things work, so proves that PIXIS GPIO lines
are a bit slow and that causes SD malfunctions.

> You're saying "some cards don't like not using the CS# line" ... in
> particular, I know there are hardware devices which toggle the clock at
> low frequency (< 400 kHz) for a while (I think the minimum is 88 clocks
> or so) and then permanently assert the CS# line

Yes, but before that, CS should be de-asserted, otherwise things
won't work, like this:

mmc_spi spi32766.0: mmc_spi: power up (21)
mmc0: clock 400000Hz busmode 2 powermode 2 cs 1 Vdd 21 width 0 timing 0
mmc_spi spi32766.0: mmc_spi: power on (21)
mmc0: starting CMD0 arg 00000000 flags 000000c0
mmc_spi spi32766.0:   mmc_spi: CMD0, resp R1
mmc0: req done (CMD0): 0: 00000001 00000000 00000000 00000000
mmc0: starting CMD8 arg 000001aa flags 000002f5
mmc_spi spi32766.0:   mmc_spi: CMD8, resp R3/R4/R7
mmc_spi spi32766.0:   ... CMD8 response SPI_R3/R4/R: resp 0017 ffffffff
mmc0: req done (CMD8): -22: 00000017 ffffffff 00000000 00000000
mmc0: starting CMD5 arg 00000000 flags 000002e1
mmc_spi spi32766.0:   mmc_spi: CMD5, resp R3/R4/R7
mmc_spi spi32766.0:   ... CMD5 response SPI_R3/R4/R: resp 0017 ffffffff
mmc0: req done (CMD5): -22: 00000017 ffffffff 00000000 00000000
mmc0: starting CMD55 arg 00000000 flags 000000f5
mmc_spi spi32766.0:   mmc_spi: CMD55, resp R1
mmc_spi spi32766.0:   ... CMD55 response SPI_R1: resp 0007 00000000

So we can't just assert the CS line, inserte SD card and expect
to work.

> before giving the
> starting CMD0.  Even so, the CMD0 is supposed to be sent at < 400 kHz
> since the card will still be in open drain mode at that point.

> Does that disagree with your observations?  If so, I'd be really
> interested to find out what you have seen in more detail, as it adds to
> the understanding of MMC/SD cards in the field.

With CS toggling enabled and without 50 ns delay after CS
assertion, everything is fine until we switch to high frequency:

mmc0: clock 25000000Hz busmode 2 powermode 2 cs 1 Vdd 20 width 0 timing 0
mmc_spi spi32766.0: mmc_spi:  clock to 25000000 Hz, 0
mmc0: new SD card on SPI
mmc0: starting CMD16 arg 00000200 flags 00000095
mmc_spi spi32766.0:   mmc_spi: CMD16, resp R1
mmc0: req done (CMD16): 0: 00000000 00000000 00000000 00000000
mmcblk0: mmc0:0000 SD01G 982 MiB
 mmcblk0:<7>mmc0: starting CMD18 arg 00000000 flags 000000b5
mmc0:     blksz 512 blocks 8 flags 00000200 tsac 100 ms nsac 0
mmc0:     CMD12 arg 00000000 flags 0000049d
mmc_spi spi32766.0:   mmc_spi: CMD18, resp R1
mmc_spi spi32766.0:     mmc_spi: read block, 512 bytes
mmc_spi spi32766.0:     mmc_spi: read block, 512 bytes
mmc_spi spi32766.0: read - crc error: crc_val=0x00ff, computed=0x0000 len=512
mmc_spi spi32766.0: read status -84
mmc_spi spi32766.0:   mmc_spi: CMD12, resp R1B


Thanks,

Patch

diff --git a/arch/powerpc/boot/dts/mpc8610_hpcd.dts b/arch/powerpc/boot/dts/mpc8610_hpcd.dts
index 1bd3ebe..a18fa5c 100644
--- a/arch/powerpc/boot/dts/mpc8610_hpcd.dts
+++ b/arch/powerpc/boot/dts/mpc8610_hpcd.dts
@@ -100,8 +100,18 @@ 
 		};
 
 		board-control@3,0 {
+			#address-cells = <1>;
+			#size-cells = <1>;
 			compatible = "fsl,fpga-pixis";
 			reg = <3 0 0x20>;
+			ranges = <0 3 0 0x20>;
+
+			sdcsr_pio: gpio-controller@a {
+				#gpio-cells = <2>;
+				compatible = "fsl,fpga-pixis-gpio-bank";
+				reg = <0xa 1>;
+				gpio-controller;
+			};
 		};
 	};
 
@@ -164,6 +174,28 @@ 
 			interrupt-parent = <&mpic>;
 		};
 
+		spi@7000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "fsl,mpc8610-spi", "fsl,spi";
+			reg = <0x7000 0x40>;
+			cell-index = <0>;
+			interrupts = <59 2>;
+			interrupt-parent = <&mpic>;
+			mode = "cpu";
+			gpios = <&sdcsr_pio 7 0>;
+
+			mmc-slot@0 {
+				compatible = "fsl,mpc8610hpcd-mmc-slot",
+					     "mmc-spi-slot";
+				reg = <0>;
+				gpios = <&sdcsr_pio 0 1   /* nCD */
+					 &sdcsr_pio 1 0>; /*  WP */
+				voltage-ranges = <3300 3300>;
+				spi-max-frequency = <6000000>;
+			};
+		};
+
 		display@2c000 {
 			compatible = "fsl,diu";
 			reg = <0x2c000 100>;
diff --git a/arch/powerpc/platforms/86xx/mpc8610_hpcd.c b/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
index 3f49a6f..ce64572 100644
--- a/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
+++ b/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
@@ -38,6 +38,7 @@ 
 #include <linux/of_platform.h>
 #include <sysdev/fsl_pci.h>
 #include <sysdev/fsl_soc.h>
+#include <sysdev/simple_gpio.h>
 
 #include "mpc86xx.h"
 
@@ -52,6 +53,9 @@  static struct of_device_id __initdata mpc8610_ids[] = {
 
 static int __init mpc8610_declare_of_platform_devices(void)
 {
+	/* Firstly, register PIXIS GPIOs. */
+	simple_gpiochip_init("fsl,fpga-pixis-gpio-bank");
+
 	/* Without this call, the SSI device driver won't get probed. */
 	of_platform_bus_probe(NULL, mpc8610_ids, NULL);