Re: sdmmc(4): check and retry bus width change
On Tue, Jun 01, 2021 at 09:29:55AM BST, Patrick Wildt wrote:
> Am Mon, Feb 22, 2021 at 08:10:21PM +0100 schrieb Patrick Wildt:
> > Hi,
> >
> > it seems like some eMMCs are not capable of doing 8-bit operation,
> > even if the controller supports it. I was questioning our drivers
> > first, but it looks like it's the same on Linux. In the case that
> > 8-bit doesn't work, they seem to fall back to lower values to make
> > that HW work.
> >
> > This diff implements a mechanism that tries 8-bit, if available,
> > then 4-bit and in the end falls back to 1-bit. This makes my HW
> > work, but I would like to have this tested by a broader audience.
> >
> > Apparently there's a "bus test" command, but it isn't implemented
> > on all host controllers. Hence I simply try to read the EXT_CSD
> > to make sure the transfer works.
> >
> > For testing, a print like
> >
> > printf("%s: using %u-bit width\n", DEVNAME(sc), width);
> >
> > could be added at line 928.
> >
> > What could possible regressions be? The width could become smaller
> > then previously. This would reduce the read/write transfer speed.
> > Also it's possible that eMMCs are not recognized/initialized anymore.
> >
> > What could possible improvements be? eMMCs that previously didn't
> > work now work, with at least 1-bit or 4-bit wide transfers.
> >
> > Please note that this only works for eMMCs. SD cards are *not* using
> > this code path. SD cards have a different initialization code path.
> >
> > Please report any changes or non-changes. If nothing changes, that's
> > perfect.
> >
> > Patrick
>
> Anyone want to give this a try? It's basically relevant for all
> ARM machines with eMMC ('soldered SD cards'), and they should work
> as well as before.
Hi Patrick,
I've managed to build the kernel with your patch and the machine
booted up just fine the first time - it was as slow as before ;^)
Then, I rebooted to the old kernel to compare dmesg before and after
- they're identical, BTW - and I shut the machine down.
I got a crash (ddb) on the next boot - don't have the time to
transcribe it right now, I'm afraid.
https://photos.app.goo.gl/Kr54uoppT4b6gR179
On subsequent boot I got:
ehci_device_bulk_start: not done, ex=0xff800800f378
scrolling(?) continuously on the screen.
Next boot, ddb again.
Old kernel booted up just fine.
Regards,
Raf
OpenBSD 6.9-current (GENERIC.MP) #0: Wed Jun 2 09:36:58 BST 2021
r...@foo.example.org:/usr/src/sys/arch/arm64/compile/GENERIC.MP
real mem = 2013835264 (1920MB)
avail mem = 1919496192 (1830MB)
random: good seed from bootblocks
mainbus0 at root: Pinebook
psci0 at mainbus0: PSCI 1.1, SMCCC 1.2
cpu0 at mainbus0 mpidr 0: ARM Cortex-A53 r0p4
cpu0: 32KB 64b/line 2-way L1 VIPT I-cache, 32KB 64b/line 4-way L1 D-cache
cpu0: 512KB 64b/line 16-way L2 cache
cpu0: CRC32,SHA2,SHA1,AES+PMULL,ASID16
cpu1 at mainbus0 mpidr 1: ARM Cortex-A53 r0p4
cpu1: 32KB 64b/line 2-way L1 VIPT I-cache, 32KB 64b/line 4-way L1 D-cache
cpu1: 512KB 64b/line 16-way L2 cache
cpu1: CRC32,SHA2,SHA1,AES+PMULL,ASID16
cpu2 at mainbus0 mpidr 2: ARM Cortex-A53 r0p4
cpu2: 32KB 64b/line 2-way L1 VIPT I-cache, 32KB 64b/line 4-way L1 D-cache
cpu2: 512KB 64b/line 16-way L2 cache
cpu2: CRC32,SHA2,SHA1,AES+PMULL,ASID16
cpu3 at mainbus0 mpidr 3: ARM Cortex-A53 r0p4
cpu3: 32KB 64b/line 2-way L1 VIPT I-cache, 32KB 64b/line 4-way L1 D-cache
cpu3: 512KB 64b/line 16-way L2 cache
cpu3: CRC32,SHA2,SHA1,AES+PMULL,ASID16
efi0 at mainbus0: UEFI 2.8
efi0: Das U-Boot rev 0x20200400
apm0 at mainbus0
"osc24M_clk" at mainbus0 not configured
"osc32k_clk" at mainbus0 not configured
"internal-osc-clk" at mainbus0 not configured
simpleaudio0 at mainbus0
"spdif-out" at mainbus0 not configured
agtimer0 at mainbus0: 24000 kHz
simplebus0 at mainbus0: "soc"
sxisyscon0 at simplebus0
sxisid0 at simplebus0
sxiccmu0 at simplebus0
sxipio0 at simplebus0: 103 pins
ampintc0 at simplebus0 nirq 224, ncpu 4 ipi: 0, 1: "interrupt-controller"
sxirtc0 at simplebus0
sxiccmu1 at simplebus0
sxipio1 at simplebus0: 13 pins
sxirsb0 at simplebus0
axppmic0 at sxirsb0 addr 0x3a3: AXP803
"de2" at simplebus0 not configured
"dma-controller" at simplebus0 not configured
"lcd-controller" at simplebus0 not configured
"lcd-controller" at simplebus0 not configured
sximmc0 at simplebus0
sdmmc0 at sximmc0: 4-bit, sd high-speed, mmc high-speed, dma
sximmc1 at simplebus0
sdmmc1 at sximmc1: 4-bit, sd high-speed, mmc high-speed, dma
sximmc2 at simplebus0
sdmmc2 at sximmc2: 8-bit, sd high-speed, mmc high-speed, dma
"phy" at simplebus0 not configured
ehci0 at simplebus0
usb0 at ehci0: USB revision 2.0
uhub0 at usb0 configuration 1 interface 0 "Generic EHCI root hub" rev 2.00/1.00
addr 1
ohci0 at simplebus0: version 1.0
ehci1 at simplebus0
usb1 at ehci1: USB revision 2.0
uhub1 at usb1 configuration 1 interface 0 "Generic EHCI root hub" rev 2.00/1.00
addr 1
ohci1 at simplebus0: version 1.0
com0 at simplebus0: ns16550, no working fifo
sxipwm0 at simplebus0
"hdmi-phy" at simplebus0
Re: sdmmc(4): check and retry bus width change
Am Mon, Feb 22, 2021 at 08:10:21PM +0100 schrieb Patrick Wildt:
> Hi,
>
> it seems like some eMMCs are not capable of doing 8-bit operation,
> even if the controller supports it. I was questioning our drivers
> first, but it looks like it's the same on Linux. In the case that
> 8-bit doesn't work, they seem to fall back to lower values to make
> that HW work.
>
> This diff implements a mechanism that tries 8-bit, if available,
> then 4-bit and in the end falls back to 1-bit. This makes my HW
> work, but I would like to have this tested by a broader audience.
>
> Apparently there's a "bus test" command, but it isn't implemented
> on all host controllers. Hence I simply try to read the EXT_CSD
> to make sure the transfer works.
>
> For testing, a print like
>
> printf("%s: using %u-bit width\n", DEVNAME(sc), width);
>
> could be added at line 928.
>
> What could possible regressions be? The width could become smaller
> then previously. This would reduce the read/write transfer speed.
> Also it's possible that eMMCs are not recognized/initialized anymore.
>
> What could possible improvements be? eMMCs that previously didn't
> work now work, with at least 1-bit or 4-bit wide transfers.
>
> Please note that this only works for eMMCs. SD cards are *not* using
> this code path. SD cards have a different initialization code path.
>
> Please report any changes or non-changes. If nothing changes, that's
> perfect.
>
> Patrick
Anyone want to give this a try? It's basically relevant for all
ARM machines with eMMC ('soldered SD cards'), and they should work
as well as before.
> diff --git a/sys/dev/sdmmc/sdmmc_mem.c b/sys/dev/sdmmc/sdmmc_mem.c
> index 59bcb1b4a11..5856b9bb1b3 100644
> --- a/sys/dev/sdmmc/sdmmc_mem.c
> +++ b/sys/dev/sdmmc/sdmmc_mem.c
> @@ -56,6 +56,8 @@ int sdmmc_mem_signal_voltage(struct sdmmc_softc *, int);
>
> int sdmmc_mem_sd_init(struct sdmmc_softc *, struct sdmmc_function *);
> int sdmmc_mem_mmc_init(struct sdmmc_softc *, struct sdmmc_function *);
> +int sdmmc_mem_mmc_select_bus_width(struct sdmmc_softc *,
> + struct sdmmc_function *, int);
> int sdmmc_mem_single_read_block(struct sdmmc_function *, int, u_char *,
> size_t);
> int sdmmc_mem_read_block_subr(struct sdmmc_function *, bus_dmamap_t,
> @@ -908,31 +910,20 @@ sdmmc_mem_mmc_init(struct sdmmc_softc *sc, struct
> sdmmc_function *sf)
> ext_csd[EXT_CSD_CARD_TYPE]);
> }
>
> - if (ISSET(sc->sc_caps, SMC_CAPS_8BIT_MODE)) {
> + if (ISSET(sc->sc_caps, SMC_CAPS_8BIT_MODE) &&
> + sdmmc_mem_mmc_select_bus_width(sc, sf, 8) == 0)
> width = 8;
> - value = EXT_CSD_BUS_WIDTH_8;
> - } else if (ISSET(sc->sc_caps, SMC_CAPS_4BIT_MODE)) {
> + else if (ISSET(sc->sc_caps, SMC_CAPS_4BIT_MODE) &&
> + sdmmc_mem_mmc_select_bus_width(sc, sf, 4) == 0)
> width = 4;
> - value = EXT_CSD_BUS_WIDTH_4;
> - } else {
> - width = 1;
> - value = EXT_CSD_BUS_WIDTH_1;
> - }
> -
> - if (width != 1) {
> - error = sdmmc_mem_mmc_switch(sf, EXT_CSD_CMD_SET_NORMAL,
> - EXT_CSD_BUS_WIDTH, value);
> - if (error == 0)
> - error = sdmmc_chip_bus_width(sc->sct,
> - sc->sch, width);
> - else {
> + else {
> + error = sdmmc_mem_mmc_select_bus_width(sc, sf, 1);
> + if (error != 0) {
> DPRINTF(("%s: can't change bus width"
> " (%d bit)\n", DEVNAME(sc), width));
> return error;
> }
> -
> - /* : need bus test? (using by CMD14 & CMD19) */
> - sdmmc_delay(1);
> + width = 1;
> }
>
> if (timing != SDMMC_TIMING_LEGACY) {
> @@ -1041,6 +1032,59 @@ sdmmc_mem_mmc_init(struct sdmmc_softc *sc, struct
> sdmmc_function *sf)
> return error;
> }
>
> +int
> +sdmmc_mem_mmc_select_bus_width(struct sdmmc_softc *sc, struct sdmmc_function
> *sf,
> +int width)
> +{
> + u_int8_t ext_csd[512];
> + int error, value;
> +
> + switch (width) {
> + case 8:
> + value = EXT_CSD_BUS_WIDTH_8;
> + break;
> + case 4:
> + value = EXT_CSD_BUS_WIDTH_4;
> + break;
> + case 1:
> + value = EXT_CSD_BUS_WIDTH_1;
> + break;
> + default:
> + printf("%s: invalid bus width\n", DEVNAME(sc));
> + return EINVAL;
> + }
> +
> + error = sdmmc_mem_mmc_switch(sf, EXT_CSD_CMD_SET_NORMAL,
> + EXT_CSD_BUS_WIDTH, value);
> + if (error != 0) {
> + DPRINTF(("%
sdmmc(4): check and retry bus width change
Hi,
it seems like some eMMCs are not capable of doing 8-bit operation,
even if the controller supports it. I was questioning our drivers
first, but it looks like it's the same on Linux. In the case that
8-bit doesn't work, they seem to fall back to lower values to make
that HW work.
This diff implements a mechanism that tries 8-bit, if available,
then 4-bit and in the end falls back to 1-bit. This makes my HW
work, but I would like to have this tested by a broader audience.
Apparently there's a "bus test" command, but it isn't implemented
on all host controllers. Hence I simply try to read the EXT_CSD
to make sure the transfer works.
For testing, a print like
printf("%s: using %u-bit width\n", DEVNAME(sc), width);
could be added at line 928.
What could possible regressions be? The width could become smaller
then previously. This would reduce the read/write transfer speed.
Also it's possible that eMMCs are not recognized/initialized anymore.
What could possible improvements be? eMMCs that previously didn't
work now work, with at least 1-bit or 4-bit wide transfers.
Please note that this only works for eMMCs. SD cards are *not* using
this code path. SD cards have a different initialization code path.
Please report any changes or non-changes. If nothing changes, that's
perfect.
Patrick
diff --git a/sys/dev/sdmmc/sdmmc_mem.c b/sys/dev/sdmmc/sdmmc_mem.c
index 59bcb1b4a11..5856b9bb1b3 100644
--- a/sys/dev/sdmmc/sdmmc_mem.c
+++ b/sys/dev/sdmmc/sdmmc_mem.c
@@ -56,6 +56,8 @@ int sdmmc_mem_signal_voltage(struct sdmmc_softc *, int);
intsdmmc_mem_sd_init(struct sdmmc_softc *, struct sdmmc_function *);
intsdmmc_mem_mmc_init(struct sdmmc_softc *, struct sdmmc_function *);
+intsdmmc_mem_mmc_select_bus_width(struct sdmmc_softc *,
+ struct sdmmc_function *, int);
intsdmmc_mem_single_read_block(struct sdmmc_function *, int, u_char *,
size_t);
intsdmmc_mem_read_block_subr(struct sdmmc_function *, bus_dmamap_t,
@@ -908,31 +910,20 @@ sdmmc_mem_mmc_init(struct sdmmc_softc *sc, struct
sdmmc_function *sf)
ext_csd[EXT_CSD_CARD_TYPE]);
}
- if (ISSET(sc->sc_caps, SMC_CAPS_8BIT_MODE)) {
+ if (ISSET(sc->sc_caps, SMC_CAPS_8BIT_MODE) &&
+ sdmmc_mem_mmc_select_bus_width(sc, sf, 8) == 0)
width = 8;
- value = EXT_CSD_BUS_WIDTH_8;
- } else if (ISSET(sc->sc_caps, SMC_CAPS_4BIT_MODE)) {
+ else if (ISSET(sc->sc_caps, SMC_CAPS_4BIT_MODE) &&
+ sdmmc_mem_mmc_select_bus_width(sc, sf, 4) == 0)
width = 4;
- value = EXT_CSD_BUS_WIDTH_4;
- } else {
- width = 1;
- value = EXT_CSD_BUS_WIDTH_1;
- }
-
- if (width != 1) {
- error = sdmmc_mem_mmc_switch(sf, EXT_CSD_CMD_SET_NORMAL,
- EXT_CSD_BUS_WIDTH, value);
- if (error == 0)
- error = sdmmc_chip_bus_width(sc->sct,
- sc->sch, width);
- else {
+ else {
+ error = sdmmc_mem_mmc_select_bus_width(sc, sf, 1);
+ if (error != 0) {
DPRINTF(("%s: can't change bus width"
" (%d bit)\n", DEVNAME(sc), width));
return error;
}
-
- /* : need bus test? (using by CMD14 & CMD19) */
- sdmmc_delay(1);
+ width = 1;
}
if (timing != SDMMC_TIMING_LEGACY) {
@@ -1041,6 +1032,59 @@ sdmmc_mem_mmc_init(struct sdmmc_softc *sc, struct
sdmmc_function *sf)
return error;
}
+int
+sdmmc_mem_mmc_select_bus_width(struct sdmmc_softc *sc, struct sdmmc_function
*sf,
+int width)
+{
+ u_int8_t ext_csd[512];
+ int error, value;
+
+ switch (width) {
+ case 8:
+ value = EXT_CSD_BUS_WIDTH_8;
+ break;
+ case 4:
+ value = EXT_CSD_BUS_WIDTH_4;
+ break;
+ case 1:
+ value = EXT_CSD_BUS_WIDTH_1;
+ break;
+ default:
+ printf("%s: invalid bus width\n", DEVNAME(sc));
+ return EINVAL;
+ }
+
+ error = sdmmc_mem_mmc_switch(sf, EXT_CSD_CMD_SET_NORMAL,
+ EXT_CSD_BUS_WIDTH, value);
+ if (error != 0) {
+ DPRINTF(("%s: can't change card bus width"
+ " (%d bit)\n", DEVNAME(sc), width));
+ return error;
+ }
+
+ error = sdmmc_chip_bus_width(sc->sct,
+ sc->sch, width);
+ if (error != 0) {
+ DPRINTF(("%s: can't change host bus width"
+ " (%d bit)\n", DEVNAM
