Re: sdmmc update
On Mon, Dec 02, 2013 at 04:52:51PM +0100, Sylvestre Gallon wrote:
On Wed, Nov 20, 2013 at 10:25:52AM +0100, Stefan Sperling wrote:
On Thu, Nov 07, 2013 at 08:10:16PM +0100, Stefan Sperling wrote:
On Thu, Nov 07, 2013 at 08:06:11PM +0100, Sylvestre Gallon wrote:
+int
+rtsx_bus_width(sdmmc_chipset_handle_t sch, int width)
+{
+ struct rtsx_softc *sc = sch;
+
+ return (rtsx_set_bus_width(sc, width));
rtsx_set_bus_width() currently never returns an error so you'll
need to tweak it as well.
I will test your diff on rtsx and sdhc.
This diff works with sdhc and provides an increase in read performance.
Unfortunately, the diff breaks rtsx. The disk manages to attach after
a CRC error, but it cannot be read. We'll need to figure this out.
Hi,
Does this diff works better ? I have just renable these lines in
rtsx_bus_power() :
- error = rtsx_set_bus_width(sc, 1);
- if (error)
- goto ret;
Unfortunately, this still doesn't work.
sdmmc0: SD_SEND_SCR send failed.
sdmmc0: mem init failed
Perhaps the problem is that the code doing SEND_SCR is trying to
peform a data transfer, i.e. cmd.c_data and cmd.c_datalen are being set.
Why is a data transfer needed? Doesn't the card include the 32bit SCR
value in its response (cmd.c_resp)?
The rtsx code is using data transfer for reading and writing blocks from
the card, i.e. after cmd 17, 18, etc. Perhaps we could try to make rtsx
use RTSX_TM_NORMAL_READ data transfer mode for commands like SEND_SCR,
instead of RTSX_TM_AUTO_READ3. But I don't really know if that would help.
Anyway, one possible path forward would be to not change existing
code paths for host adapters that don't set SMC_CAPS_4BIT_MODE.
Then we could leave this feature disabled for rtsx to avoid the
regression and enable 4bit mode once we get it working. Do you think
that is feasible? Just removing the line that sets SMC_CAPS_4BIT_MODE
for rtsx is not enough because your diff changes the behaviour of the
sdmmc stack anyway -- rtsx still fails in SEND_SCR. And perhaps we would
need another CAPS flag for SEND_SWITCH_FUNC support and just as carefully
tip-toe around host controller drivers that don't set it.
BTW, this part of your diff looks wrong:
+ if (ptr != NULL)
+ if (ISSET(sc-sc_caps, SMC_CAPS_DMA))
+ free(ptr, M_DEVBUF);
I think neither of these if-statements is needed.
You always need to free the pointer, and it's OK to free NULL.
Nothing sets CAPS_DMA right now, AFAIK. rtsx uses DMA, but it doesn't
require support from the sdmmc stack for this. It seems NetBSD are
doing that differently and you copied this CAPS_DMA check from there.
If it still not work with this diff can you add a printf in the
bus_clock code to know which frequency is required by the stack ?
See below. I've added some debug output with rtsxdebug=3 and a few
more printfs.
Note that the first 'soft reset' is normal, a timeout is expected
at that stage of card discovery. The second one means that data
transfer from the card has timed out.
rtsx0: card inserted
rtsx0: voltage change ocr=0x403c
rtsx0: setting buswidth to 1
rtsx_bus_clock: freq=400
rtsx0: executing cmd 5
rtsx0: executing cmd 0
rtsx0: executing cmd 55
rtsx0: executing cmd 41
rtsx0: voltage change ocr=0xc
rtsx0: setting buswidth to 1
rtsx0: executing cmd 0
rtsx0: executing cmd 8
rtsx0: executing cmd 55
rtsx0: executing cmd 41
rtsx0: executing cmd 55
rtsx0: executing cmd 41
rtsx0: executing cmd 55
rtsx0: executing cmd 41
rtsx0: executing cmd 55
rtsx0: executing cmd 41
rtsx0: executing cmd 55
rtsx0: executing cmd 41
rtsx0: executing cmd 55
rtsx0: executing cmd 41
rtsx0: executing cmd 55
rtsx0: executing cmd 41
rtsx0: executing cmd 55
rtsx0: executing cmd 41
rtsx0: executing cmd 55
rtsx0: executing cmd 41
rtsx0: executing cmd 55
rtsx0: executing cmd 41
rtsx0: executing cmd 55
rtsx0: executing cmd 41
rtsx0: executing cmd 2
rtsx0: executing cmd 3
rtsx0: executing cmd 2
rtsx0: soft reset
rtsx0: executing cmd 9
rtsx0: executing cmd 7
rtsx0: executing cmd 16
rtsx0: executing cmd 55
rtsx0: executing cmd 51
rtsx0: read xfer: 8 bytes with block size 8
rtsx0: soft reset
rtsx0: xfer done, error=60
sdmmc0: SD_SEND_SCR send failed.
sdmmc0: mem init failed
scsibus1 at sdmmc0: 2 targets, initiator 0
sd1 at scsibus1 targ 1 lun 0: SD/MMC, Drive #01, SCSI2 0/direct fixed
sd1: 1882MB, 512 bytes/sector, 3854336 sectors
rtsx0: executing cmd 17
rtsx0: soft reset
rtsx0: executing cmd 18
rtsx0: read xfer: 2048 bytes with block size 512
rtsx0: soft reset
rtsx0: xfer done, error=60
rtsx0: executing cmd 18
rtsx0: soft reset
Re: sdmmc update
On Wed, Nov 20, 2013 at 10:25:52AM +0100, Stefan Sperling wrote:
On Thu, Nov 07, 2013 at 08:10:16PM +0100, Stefan Sperling wrote:
On Thu, Nov 07, 2013 at 08:06:11PM +0100, Sylvestre Gallon wrote:
+int
+rtsx_bus_width(sdmmc_chipset_handle_t sch, int width)
+{
+ struct rtsx_softc *sc = sch;
+
+ return (rtsx_set_bus_width(sc, width));
rtsx_set_bus_width() currently never returns an error so you'll
need to tweak it as well.
I will test your diff on rtsx and sdhc.
This diff works with sdhc and provides an increase in read performance.
Unfortunately, the diff breaks rtsx. The disk manages to attach after
a CRC error, but it cannot be read. We'll need to figure this out.
Hi,
Does this diff works better ? I have just renable these lines in
rtsx_bus_power() :
- error = rtsx_set_bus_width(sc, 1);
- if (error)
- goto ret;
If it still not work with this diff can you add a printf in the
bus_clock code to know which frequency is required by the stack ?
Thanks,
Index: arch/arm/xscale/pxa2x0_mmc.c
===
RCS file: /cvs/src/sys/arch/arm/xscale/pxa2x0_mmc.c,v
retrieving revision 1.11
diff -u -p -u -p -r1.11 pxa2x0_mmc.c
--- arch/arm/xscale/pxa2x0_mmc.c22 Aug 2012 13:37:04 - 1.11
+++ arch/arm/xscale/pxa2x0_mmc.c2 Dec 2013 15:34:18 -
@@ -56,6 +56,7 @@ u_int32_t pxammc_host_ocr(sdmmc_chipset_
intpxammc_host_maxblklen(sdmmc_chipset_handle_t);
intpxammc_card_detect(sdmmc_chipset_handle_t);
intpxammc_bus_power(sdmmc_chipset_handle_t, u_int32_t);
+intpxammc_bus_width(sdmmc_chipset_handle_t, int);
intpxammc_bus_clock(sdmmc_chipset_handle_t, int);
void pxammc_exec_command(sdmmc_chipset_handle_t, struct sdmmc_command *);
void pxammc_clock_stop(struct pxammc_softc *);
@@ -90,6 +91,7 @@ struct sdmmc_chip_functions pxammc_funct
/* bus power and clock frequency */
pxammc_bus_power,
pxammc_bus_clock,
+ pxammc_bus_width,
/* command execution */
pxammc_exec_command
};
@@ -179,6 +181,8 @@ pxammc_attach(struct pxammc_softc *sc, v
*/
bzero(saa, sizeof saa);
saa.saa_busname = sdmmc;
+ saa.saa_clkmax = SDMMC_SDCLK_25MHZ;
+
saa.sct = pxammc_functions;
saa.sch = sc;
saa.flags = SMF_STOP_AFTER_MULTIPLE;
@@ -274,6 +278,12 @@ pxammc_bus_power(sdmmc_chipset_handle_t
DPRINTF(0,(%s: driver lacks set_power() function\n,
sc-sc_dev.dv_xname));
return ENXIO;
+}
+
+int
+pxammc_bus_width(sdmmc_chipset_handle_t sch, int width)
+{
+ return (0);
}
int
Index: arch/armv7/imx/imxesdhc.c
===
RCS file: /cvs/src/sys/arch/armv7/imx/imxesdhc.c,v
retrieving revision 1.4
diff -u -p -u -p -r1.4 imxesdhc.c
--- arch/armv7/imx/imxesdhc.c 6 Nov 2013 19:03:07 - 1.4
+++ arch/armv7/imx/imxesdhc.c 2 Dec 2013 15:34:18 -
@@ -195,6 +195,7 @@ int imxesdhc_host_maxblklen(sdmmc_chipse
intimxesdhc_card_detect(sdmmc_chipset_handle_t);
intimxesdhc_bus_power(sdmmc_chipset_handle_t, uint32_t);
intimxesdhc_bus_clock(sdmmc_chipset_handle_t, int);
+intimxesdhc_bus_width(sdmmc_chipset_handle_t, int);
void imxesdhc_card_intr_mask(sdmmc_chipset_handle_t, int);
void imxesdhc_card_intr_ack(sdmmc_chipset_handle_t);
void imxesdhc_exec_command(sdmmc_chipset_handle_t, struct sdmmc_command *);
@@ -225,6 +226,7 @@ struct sdmmc_chip_functions imxesdhc_fun
/* bus power and clock frequency */
imxesdhc_bus_power,
imxesdhc_bus_clock,
+ imxesdhc_bus_width,
/* command execution */
imxesdhc_exec_command,
/* card interrupt */
@@ -319,6 +321,7 @@ imxesdhc_attach(struct device *parent, s
bzero(saa, sizeof(saa));
saa.saa_busname = sdmmc;
+ saa.saa_clkmax = SDMMC_SDCLK_25MHZ;
saa.sct = imxesdhc_functions;
saa.sch = sc;
@@ -327,7 +330,7 @@ imxesdhc_attach(struct device *parent, s
error = 0;
goto err;
}
-
+
return;
err:
@@ -483,6 +486,12 @@ imxesdhc_card_detect(sdmmc_chipset_handl
*/
int
imxesdhc_bus_power(sdmmc_chipset_handle_t sch, uint32_t ocr)
+{
+ return 0;
+}
+
+int
+imxesdhc_bus_width(sdmmc_chipset_handle_t sch, int freq)
{
return 0;
}
Index: arch/armv7/omap/ommmc.c
===
RCS file: /cvs/src/sys/arch/armv7/omap/ommmc.c,v
retrieving revision 1.11
diff -u -p -u -p -r1.11 ommmc.c
--- arch/armv7/omap/ommmc.c 12 Nov 2013 17:51:52 - 1.11
+++ arch/armv7/omap/ommmc.c 2 Dec 2013 15:34:18 -
@@ -229,6 +229,7 @@ int ommmc_host_maxblklen(sdmmc_chipset_h
intommmc_card_detect(sdmmc_chipset_handle_t);
intommmc_bus_power(sdmmc_chipset_handle_t, uint32_t);
intommmc_bus_clock(sdmmc_chipset_handle_t, int);
+int
Re: sdmmc update
On Thu, Nov 07, 2013 at 08:10:16PM +0100, Stefan Sperling wrote:
On Thu, Nov 07, 2013 at 08:06:11PM +0100, Sylvestre Gallon wrote:
+int
+rtsx_bus_width(sdmmc_chipset_handle_t sch, int width)
+{
+ struct rtsx_softc *sc = sch;
+
+ return (rtsx_set_bus_width(sc, width));
rtsx_set_bus_width() currently never returns an error so you'll
need to tweak it as well.
I will test your diff on rtsx and sdhc.
This diff works with sdhc and provides an increase in read performance.
Unfortunately, the diff breaks rtsx. The disk manages to attach after
a CRC error, but it cannot be read. We'll need to figure this out.
rtsx0: CRC error
sdmmc0: SD_SEND_SCR send failed.
sdmmc0: mem init failed
scsibus3 at sdmmc0: 2 targets, initiator 0
sd2 at scsibus3 targ 1 lun 0: SD/MMC, Drive #01, SCSI2 0/direct fixed
sd2: 945MB, 512 bytes/sector, 1935360 sectors
sd2 detached
scsibus3 detached
sdmmc update
Hi tech@
Here is a diff that updates our sdmmc stack with support for
other frequencies than 25Mhz and for 4 pins transfers. Big
chunks of this code comes directly from NetBSD sdmmc stack.
Normally this diff should speedup things for some sdcards and
for some controllers.
I tried this diff on the only sdmmc controller I have (ommmc).
It will be awesome if people could give me feedback for these
drivers :
rtsx
sdhc
imxesdhc
pxammc
w83l518d_sdmmc
Any OK/Comments ?
Index: arch/arm/xscale/pxa2x0_mmc.c
===
RCS file: /cvs/src/sys/arch/arm/xscale/pxa2x0_mmc.c,v
retrieving revision 1.11
diff -u -p -u -p -r1.11 pxa2x0_mmc.c
--- arch/arm/xscale/pxa2x0_mmc.c22 Aug 2012 13:37:04 - 1.11
+++ arch/arm/xscale/pxa2x0_mmc.c6 Nov 2013 16:59:38 -
@@ -56,6 +56,7 @@ u_int32_t pxammc_host_ocr(sdmmc_chipset_
intpxammc_host_maxblklen(sdmmc_chipset_handle_t);
intpxammc_card_detect(sdmmc_chipset_handle_t);
intpxammc_bus_power(sdmmc_chipset_handle_t, u_int32_t);
+intpxammc_bus_width(sdmmc_chipset_handle_t, int);
intpxammc_bus_clock(sdmmc_chipset_handle_t, int);
void pxammc_exec_command(sdmmc_chipset_handle_t, struct sdmmc_command *);
void pxammc_clock_stop(struct pxammc_softc *);
@@ -90,6 +91,7 @@ struct sdmmc_chip_functions pxammc_funct
/* bus power and clock frequency */
pxammc_bus_power,
pxammc_bus_clock,
+ pxammc_bus_width,
/* command execution */
pxammc_exec_command
};
@@ -179,6 +181,8 @@ pxammc_attach(struct pxammc_softc *sc, v
*/
bzero(saa, sizeof saa);
saa.saa_busname = sdmmc;
+ saa.saa_clkmax = SDMMC_SDCLK_25MHZ;
+
saa.sct = pxammc_functions;
saa.sch = sc;
saa.flags = SMF_STOP_AFTER_MULTIPLE;
@@ -274,6 +278,12 @@ pxammc_bus_power(sdmmc_chipset_handle_t
DPRINTF(0,(%s: driver lacks set_power() function\n,
sc-sc_dev.dv_xname));
return ENXIO;
+}
+
+int
+pxammc_bus_width(sdmmc_chipset_handle_t sch, int width)
+{
+ return (0);
}
int
Index: arch/armv7/imx/imxesdhc.c
===
RCS file: /cvs/src/sys/arch/armv7/imx/imxesdhc.c,v
retrieving revision 1.3
diff -u -p -u -p -r1.3 imxesdhc.c
--- arch/armv7/imx/imxesdhc.c 27 Oct 2013 20:27:09 - 1.3
+++ arch/armv7/imx/imxesdhc.c 6 Nov 2013 16:59:43 -
@@ -195,6 +195,7 @@ int imxesdhc_host_maxblklen(sdmmc_chipse
intimxesdhc_card_detect(sdmmc_chipset_handle_t);
intimxesdhc_bus_power(sdmmc_chipset_handle_t, uint32_t);
intimxesdhc_bus_clock(sdmmc_chipset_handle_t, int);
+intimxesdhc_bus_width(sdmmc_chipset_handle_t, int);
void imxesdhc_card_intr_mask(sdmmc_chipset_handle_t, int);
void imxesdhc_card_intr_ack(sdmmc_chipset_handle_t);
void imxesdhc_exec_command(sdmmc_chipset_handle_t, struct sdmmc_command *);
@@ -225,6 +226,7 @@ struct sdmmc_chip_functions imxesdhc_fun
/* bus power and clock frequency */
imxesdhc_bus_power,
imxesdhc_bus_clock,
+ imxesdhc_bus_width,
/* command execution */
imxesdhc_exec_command,
/* card interrupt */
@@ -319,6 +321,7 @@ imxesdhc_attach(struct device *parent, s
bzero(saa, sizeof(saa));
saa.saa_busname = sdmmc;
+ saa.saa_clkmax = SDMMC_SDCLK_25MHZ;
saa.sct = imxesdhc_functions;
saa.sch = sc;
@@ -327,7 +330,7 @@ imxesdhc_attach(struct device *parent, s
error = 0;
goto err;
}
-
+
return;
err:
@@ -483,6 +486,12 @@ imxesdhc_card_detect(sdmmc_chipset_handl
*/
int
imxesdhc_bus_power(sdmmc_chipset_handle_t sch, uint32_t ocr)
+{
+ return 0;
+}
+
+int
+imxesdhc_bus_width(sdmmc_chipset_handle_t sch, int freq)
{
return 0;
}
Index: arch/armv7/omap/ommmc.c
===
RCS file: /cvs/src/sys/arch/armv7/omap/ommmc.c,v
retrieving revision 1.9
diff -u -p -u -p -r1.9 ommmc.c
--- arch/armv7/omap/ommmc.c 28 Oct 2013 20:45:20 - 1.9
+++ arch/armv7/omap/ommmc.c 6 Nov 2013 16:59:43 -
@@ -228,6 +228,7 @@ int ommmc_host_maxblklen(sdmmc_chipset_h
intommmc_card_detect(sdmmc_chipset_handle_t);
intommmc_bus_power(sdmmc_chipset_handle_t, uint32_t);
intommmc_bus_clock(sdmmc_chipset_handle_t, int);
+intommmc_bus_width(sdmmc_chipset_handle_t, int);
void ommmc_card_intr_mask(sdmmc_chipset_handle_t, int);
void ommmc_card_intr_ack(sdmmc_chipset_handle_t);
void ommmc_exec_command(sdmmc_chipset_handle_t, struct sdmmc_command *);
@@ -259,6 +260,7 @@ struct sdmmc_chip_functions ommmc_functi
/* bus power and clock frequency */
ommmc_bus_power,
ommmc_bus_clock,
+ ommmc_bus_width,
/* command execution */
ommmc_exec_command,
/* card interrupt */
@@ -386,9 +388,10 @@ ommmc_attach(struct device *parent, stru
Re: sdmmc update
On Thu, Nov 07, 2013 at 08:06:11PM +0100, Sylvestre Gallon wrote:
+int
+rtsx_bus_width(sdmmc_chipset_handle_t sch, int width)
+{
+ struct rtsx_softc *sc = sch;
+
+ return (rtsx_set_bus_width(sc, width));
rtsx_set_bus_width() currently never returns an error so you'll
need to tweak it as well.
I will test your diff on rtsx and sdhc.
