mmc: sunxi: Fix eMMC usage on H5 boards

[Chen-Yu Tsai]

Hi everyone,

Since the HS-DDR mode was enabled for the A64 eMMC controller, there
have been reports of eMMC failing to work on some H5 boards. It seems
that while the H5 and A64 share the same controller for eMMC, some H5
boards don't have trace lengths that work under HS-DDR with the default
delay chain settings. Unfortunately we don't support tuning them at the
moment, and these boards didn't seem to come with any settings either.
Instead HS-DDR just wasn't enabled.

The failure is typically a data CRC error on data reads, such as the
partition scanning when the device is first probed. While this in itself
would result in the device being unusable, there seems to be a timing
issue in the recovery of the MMC controller. After the CRC error, the
driver manually issues a stop command to the device, which also fails.
After this a following command would stall: the MMC subsystem waits for
the completion notice of the request, which never happens. The stall
also blocks udev, which kind of blocks the whole boot process. However
if I turn on debug messages to try to narrow down the issue, it recovers
just fine. Any help on this issue would be much appreciated.

I propose we turn off HS-DDR on the H5 (maybe even the H6, but I don't
have anything to test right now) by default, and enable it per-board
using the common mmc binding properties for speed modes.

Patch 1 disables HS-DDR for H5 eMMC.

Patch 2 adds a check blocking (force disabling) any modes the driver
doesn't support. In retrospect this should have been added a long time
ago.

Patch 3 enables HS-DDR for the Libre Computer ALL-H3-CC H5, which works
normally.

If possible please merge all of them as fixes.


Regards
ChenYu

Chen-Yu Tsai (3):
mmc: sunxi: Disable HS-DDR mode for H5 eMMC controller by default
mmc: sunxi: Filter out unsupported modes declared in the device tree
arm64: dts: allwinner: h5: libretech-all-h3-cc: Mark eMMC HS-DDR 3.3V
capable

.../sun50i-h5-libretech-all-h3-cc.dts | 4 +++
drivers/mmc/host/sunxi-mmc.c | 27 ++++++++++++++++++-
2 files changed, 30 insertions(+), 1 deletion(-)

--
2.20.1

[Chen-Yu Tsai]

Some H5 boards seem to not have proper trace lengths for eMMC to be able
to use the default setting for the delay chains under HS-DDR mode. These
include the Bananapi M2+ H5 and NanoPi NEO Core2. However the Libre
Computer ALL-H3-CC-H5 works just fine.

For the H5 (at least for now), default to not enabling HS-DDR modes in
the driver, and expect the device tree to signal HS-DDR capability on
boards that work.

Reported-by: Chris Blake <chrisr...@gmail.com>
Fixes: 07bafc1e3536 ("mmc: sunxi: Use new timing mode for A64 eMMC controller")
Cc: <sta...@vger.kernel.org>
Signed-off-by: Chen-Yu Tsai <we...@csie.org>
---
drivers/mmc/host/sunxi-mmc.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index 279e326e397e..7415af8c8ff6 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -1399,7 +1399,16 @@ static int sunxi_mmc_probe(struct platform_device *pdev)
mmc->caps |= MMC_CAP_MMC_HIGHSPEED | MMC_CAP_SD_HIGHSPEED |
MMC_CAP_ERASE | MMC_CAP_SDIO_IRQ;

- if (host->cfg->clk_delays || host->use_new_timings)
+ /*
+ * Some H5 devices do not have signal traces precise enough to
+ * use HS DDR mode for their eMMC chips.
+ *
+ * We still enable HS DDR modes for all the other controller
+ * variants that support them.
+ */
+ if ((host->cfg->clk_delays || host->use_new_timings) &&
+ !of_device_is_compatible(pdev->dev.of_node,
+ "allwinner,sun50i-h5-emmc"))
mmc->caps |= MMC_CAP_1_8V_DDR | MMC_CAP_3_3V_DDR;

ret = mmc_of_parse(mmc);
--
2.20.1

[Chen-Yu Tsai]

The Libre Computer ALL-H3-CC H5 is one of the few boards that can have
its eMMC run at HS-DDR speed mode. Mark it as such.

Signed-off-by: Chen-Yu Tsai <we...@csie.org>
---

.../boot/dts/allwinner/sun50i-h5-libretech-all-h3-cc.dts | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h5-libretech-all-h3-cc.dts b/arch/arm64/boot/dts/allwinner/sun50i-h5-libretech-all-h3-cc.dts
index 95e113ce8699..d68bdfea2271 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h5-libretech-all-h3-cc.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h5-libretech-all-h3-cc.dts
@@ -12,3 +12,7 @@
model = "Libre Computer Board ALL-H3-CC H5";
compatible = "libretech,all-h3-cc-h5", "allwinner,sun50i-h5";
};
+
+&mmc2 {
+ mmc-ddr-3_3v;
+};
--
2.20.1

[Chen-Yu Tsai]

The MMC device tree bindings include properties used to signal various
signalling speed modes. Until now the sunxi driver was accepting them
without any further filtering, while the sunxi device trees were not
actually using them.

Since some of the H5 boards can not run at higher speed modes stably,
we are resorting to declaring the higher speed modes per-board.

Regardless, having boards declare modes and blindly following them,
even without proper support in the driver, is generally a bad thing.

Filter out all unsupported modes from the capabilities mask after
the device tree properties have been parsed.

Cc: <sta...@vger.kernel.org>

Signed-off-by: Chen-Yu Tsai <we...@csie.org>

---

This should be backported to stable kernels in case people try to run
new device trees (that declare newly supported modes) with old kernels.
---
drivers/mmc/host/sunxi-mmc.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)

diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index 7415af8c8ff6..a01433012db0 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -1415,6 +1415,22 @@ static int sunxi_mmc_probe(struct platform_device *pdev)
if (ret)
goto error_free_dma;

+ /*
+ * If we don't support delay chains in the SoC, we can't use any
+ * of the DDR speed modes. Mask them out in case the device
+ * tree specifies the properties for them, which gets added to
+ * the caps by mmc_of_parse() above.
+ */
+ if (!(host->cfg->clk_delays || host->use_new_timings))
+ mmc->caps &= ~(MMC_CAP_3_3V_DDR | MMC_CAP_1_8V_DDR |
+ MMC_CAP_1_2V_DDR);
+
+ /* TODO: UHS modes untested due to lack of supporting boards */
+ mmc->caps &= ~MMC_CAP_UHS;
+
+ /* TODO: This driver doesn't support HS200 and HS400 modes yet */
+ mmc->caps2 &= ~(MMC_CAP2_HS200 | MMC_CAP2_HS400);
+
ret = sunxi_mmc_init_host(host);
if (ret)
goto error_free_dma;
--
2.20.1

[Chen-Yu Tsai]

(Resent with proper subject tag.)

[Maxime Ripard]

On Sun, Feb 03, 2019 at 11:56:26PM +0800, Chen-Yu Tsai wrote:
> Some H5 boards seem to not have proper trace lengths for eMMC to be able
> to use the default setting for the delay chains under HS-DDR mode. These
> include the Bananapi M2+ H5 and NanoPi NEO Core2. However the Libre
> Computer ALL-H3-CC-H5 works just fine.
>
> For the H5 (at least for now), default to not enabling HS-DDR modes in
> the driver, and expect the device tree to signal HS-DDR capability on
> boards that work.
>
> Reported-by: Chris Blake <chrisr...@gmail.com>
> Fixes: 07bafc1e3536 ("mmc: sunxi: Use new timing mode for A64 eMMC controller")
> Cc: <sta...@vger.kernel.org>
> Signed-off-by: Chen-Yu Tsai <we...@csie.org>

Acked-by: Maxime Ripard <maxime...@bootlin.com>

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Maxime Ripard]

I've tested up to SDR104 and it works on the A64 at least

> + /* TODO: This driver doesn't support HS200 and HS400 modes yet */
> + mmc->caps2 &= ~(MMC_CAP2_HS200 | MMC_CAP2_HS400);

And HS200 works too.

[Chen-Yu Tsai]

That's good to know. What board was this on? I had given up hope waiting
for a vendor to produce a board that could do proper voltage switching for
SD cards.

> > + /* TODO: This driver doesn't support HS200 and HS400 modes yet */
> > + mmc->caps2 &= ~(MMC_CAP2_HS200 | MMC_CAP2_HS400);
>
> And HS200 works too.

OK. I thought there was some special magic required in the driver. Maybe
that was for HS400 only? Again, what board was this on?

Thanks
ChenYu

[Maxime Ripard]

On a Sootech SoM, that had an HS400 eMMC and an SDR104 Marvell WiFi
chip. I don't have that board anymore, and the website seems down now
though :/

> > > + /* TODO: This driver doesn't support HS200 and HS400 modes yet */
> > > + mmc->caps2 &= ~(MMC_CAP2_HS200 | MMC_CAP2_HS400);
> >
> > And HS200 works too.
>
> OK. I thought there was some special magic required in the driver. Maybe
> that was for HS400 only? Again, what board was this on?

Yeah, that was for HS400 only

[Chen-Yu Tsai]

Bummer. So no commercially available board still. :/

> > > > + /* TODO: This driver doesn't support HS200 and HS400 modes yet */
> > > > + mmc->caps2 &= ~(MMC_CAP2_HS200 | MMC_CAP2_HS400);
> > >
> > > And HS200 works too.
> >
> > OK. I thought there was some special magic required in the driver. Maybe
> > that was for HS400 only? Again, what board was this on?
>
> Yeah, that was for HS400 only

OK. So would unblocking UHS and HS200, but not enabling them by default,
which is essentially the original behavior, work for you?

If so, I'll respin a version tonight.

ChenYu

[Maxime Ripard]

Yep, that's perfect

[Chen-Yu Tsai]

The MMC device tree bindings include properties used to signal various
signalling speed modes. Until now the sunxi driver was accepting them
without any further filtering, while the sunxi device trees were not
actually using them.

Since some of the H5 boards can not run at higher speed modes stably,
we are resorting to declaring the higher speed modes per-board.

Regardless, having boards declare modes and blindly following them,
even without proper support in the driver, is generally a bad thing.

Filter out all unsupported modes from the capabilities mask after
the device tree properties have been parsed.

Cc: <sta...@vger.kernel.org>
Signed-off-by: Chen-Yu Tsai <we...@csie.org>

---

This should be backported to stable kernels in case people try to run
new device trees (that declare newly supported modes) with old kernels.
---

drivers/mmc/host/sunxi-mmc.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)

diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index 7415af8c8ff6..70fadc976795 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -1415,6 +1415,21 @@ static int sunxi_mmc_probe(struct platform_device *pdev)

if (ret)
goto error_free_dma;

+ /*
+ * If we don't support delay chains in the SoC, we can't use any

+ * of the higher speed modes. Mask them out in case the device

+ * tree specifies the properties for them, which gets added to
+ * the caps by mmc_of_parse() above.
+ */
+ if (!(host->cfg->clk_delays || host->use_new_timings)) {
+ mmc->caps &= ~(MMC_CAP_3_3V_DDR | MMC_CAP_1_8V_DDR |

+ MMC_CAP_1_2V_DDR | MMC_CAP_UHS);
+ mmc->caps2 &= ~MMC_CAP2_HS200;
+ }
+
+ /* TODO: This driver doesn't support HS400 mode yet */
+ mmc->caps2 &= ~MMC_CAP2_HS400;

[Chen-Yu Tsai]

The Libre Computer ALL-H3-CC H5 is one of the few boards that can have
its eMMC run at HS-DDR speed mode. Mark it as such.

Signed-off-by: Chen-Yu Tsai <we...@csie.org>
---

[Chen-Yu Tsai]

Hi everyone,

This is v2 of my H5 eMMC fix series. Changes since v1:

- Don't block HS200 and UHS modes, since these have been tested

Original cover letter:

drivers/mmc/host/sunxi-mmc.c | 26 ++++++++++++++++++-
2 files changed, 29 insertions(+), 1 deletion(-)

--
2.20.1

[Chen-Yu Tsai]

Some H5 boards seem to not have proper trace lengths for eMMC to be able
to use the default setting for the delay chains under HS-DDR mode. These
include the Bananapi M2+ H5 and NanoPi NEO Core2. However the Libre
Computer ALL-H3-CC-H5 works just fine.

For the H5 (at least for now), default to not enabling HS-DDR modes in
the driver, and expect the device tree to signal HS-DDR capability on
boards that work.

Reported-by: Chris Blake <chrisr...@gmail.com>
Fixes: 07bafc1e3536 ("mmc: sunxi: Use new timing mode for A64 eMMC controller")
Cc: <sta...@vger.kernel.org>

Acked-by: Maxime Ripard <maxime...@bootlin.com>

Signed-off-by: Chen-Yu Tsai <we...@csie.org>
---

drivers/mmc/host/sunxi-mmc.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index 279e326e397e..7415af8c8ff6 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c

[Maxime Ripard]

On Tue, Feb 05, 2019 at 11:42:24PM +0800, Chen-Yu Tsai wrote:
> The MMC device tree bindings include properties used to signal various
> signalling speed modes. Until now the sunxi driver was accepting them
> without any further filtering, while the sunxi device trees were not
> actually using them.
>
> Since some of the H5 boards can not run at higher speed modes stably,
> we are resorting to declaring the higher speed modes per-board.
>
> Regardless, having boards declare modes and blindly following them,
> even without proper support in the driver, is generally a bad thing.
>
> Filter out all unsupported modes from the capabilities mask after
> the device tree properties have been parsed.
>
> Cc: <sta...@vger.kernel.org>
> Signed-off-by: Chen-Yu Tsai <we...@csie.org>

Acked-by: Maxime Ripard <maxime...@bootlin.com>

[Maxime Ripard]

On Tue, Feb 05, 2019 at 11:42:25PM +0800, Chen-Yu Tsai wrote:
> The Libre Computer ALL-H3-CC H5 is one of the few boards that can have
> its eMMC run at HS-DDR speed mode. Mark it as such.
>
> Signed-off-by: Chen-Yu Tsai <we...@csie.org>

Applied, thanks!

[Ulf Hansson]

On Tue, 5 Feb 2019 at 16:42, Chen-Yu Tsai <we...@csie.org> wrote:
>
> Some H5 boards seem to not have proper trace lengths for eMMC to be able
> to use the default setting for the delay chains under HS-DDR mode. These
> include the Bananapi M2+ H5 and NanoPi NEO Core2. However the Libre
> Computer ALL-H3-CC-H5 works just fine.
>
> For the H5 (at least for now), default to not enabling HS-DDR modes in
> the driver, and expect the device tree to signal HS-DDR capability on
> boards that work.
>
> Reported-by: Chris Blake <chrisr...@gmail.com>
> Fixes: 07bafc1e3536 ("mmc: sunxi: Use new timing mode for A64 eMMC controller")
> Cc: <sta...@vger.kernel.org>
> Acked-by: Maxime Ripard <maxime...@bootlin.com>
> Signed-off-by: Chen-Yu Tsai <we...@csie.org>

Applied for fixes, thanks!

Kind regards
Uffe

[Ulf Hansson]

On Tue, 5 Feb 2019 at 16:42, Chen-Yu Tsai <we...@csie.org> wrote:
>

> The MMC device tree bindings include properties used to signal various
> signalling speed modes. Until now the sunxi driver was accepting them
> without any further filtering, while the sunxi device trees were not
> actually using them.
>
> Since some of the H5 boards can not run at higher speed modes stably,
> we are resorting to declaring the higher speed modes per-board.
>
> Regardless, having boards declare modes and blindly following them,
> even without proper support in the driver, is generally a bad thing.
>
> Filter out all unsupported modes from the capabilities mask after
> the device tree properties have been parsed.
>
> Cc: <sta...@vger.kernel.org>
> Signed-off-by: Chen-Yu Tsai <we...@csie.org>
>

Applied for fixes, thanks!

Kind regards
Uffe