linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* mmc: sunxi: Fix eMMC usage on H5 boards
@ 2019-02-03 15:56 Chen-Yu Tsai
  2019-02-03 15:56 ` [PATCH 1/3] mmc: sunxi: Disable HS-DDR mode for H5 eMMC controller by default Chen-Yu Tsai
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Chen-Yu Tsai @ 2019-02-03 15:56 UTC (permalink / raw)
  To: Ulf Hansson, Maxime Ripard
  Cc: Chen-Yu Tsai, linux-mmc, linux-arm-kernel, devicetree,
	linux-kernel, linux-sunxi, Chris Blake

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/3] mmc: sunxi: Disable HS-DDR mode for H5 eMMC controller by default
  2019-02-03 15:56 mmc: sunxi: Fix eMMC usage on H5 boards Chen-Yu Tsai
@ 2019-02-03 15:56 ` Chen-Yu Tsai
  2019-02-04  9:32   ` Maxime Ripard
  2019-02-03 15:56 ` [PATCH 2/3] mmc: sunxi: Filter out unsupported modes declared in the device tree Chen-Yu Tsai
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Chen-Yu Tsai @ 2019-02-03 15:56 UTC (permalink / raw)
  To: Ulf Hansson, Maxime Ripard
  Cc: Chen-Yu Tsai, linux-mmc, linux-arm-kernel, devicetree,
	linux-kernel, linux-sunxi, Chris Blake, stable

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 <chrisrblake93@gmail.com>
Fixes: 07bafc1e3536 ("mmc: sunxi: Use new timing mode for A64 eMMC controller")
Cc: <stable@vger.kernel.org>
Signed-off-by: Chen-Yu Tsai <wens@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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/3] mmc: sunxi: Filter out unsupported modes declared in the device tree
  2019-02-03 15:56 mmc: sunxi: Fix eMMC usage on H5 boards Chen-Yu Tsai
  2019-02-03 15:56 ` [PATCH 1/3] mmc: sunxi: Disable HS-DDR mode for H5 eMMC controller by default Chen-Yu Tsai
@ 2019-02-03 15:56 ` Chen-Yu Tsai
  2019-02-04  9:34   ` Maxime Ripard
  2019-02-03 15:56 ` [PATCH 3/3] arm64: dts: allwinner: h5: libretech-all-h3-cc: Mark eMMC HS-DDR 3.3V capable Chen-Yu Tsai
  2019-02-03 15:59 ` [PATCH 0/3] mmc: sunxi: Fix eMMC usage on H5 boards Chen-Yu Tsai
  3 siblings, 1 reply; 11+ messages in thread
From: Chen-Yu Tsai @ 2019-02-03 15:56 UTC (permalink / raw)
  To: Ulf Hansson, Maxime Ripard
  Cc: Chen-Yu Tsai, linux-mmc, linux-arm-kernel, devicetree,
	linux-kernel, linux-sunxi, Chris Blake, stable

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: <stable@vger.kernel.org>
Signed-off-by: Chen-Yu Tsai <wens@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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/3] arm64: dts: allwinner: h5: libretech-all-h3-cc: Mark eMMC HS-DDR 3.3V capable
  2019-02-03 15:56 mmc: sunxi: Fix eMMC usage on H5 boards Chen-Yu Tsai
  2019-02-03 15:56 ` [PATCH 1/3] mmc: sunxi: Disable HS-DDR mode for H5 eMMC controller by default Chen-Yu Tsai
  2019-02-03 15:56 ` [PATCH 2/3] mmc: sunxi: Filter out unsupported modes declared in the device tree Chen-Yu Tsai
@ 2019-02-03 15:56 ` Chen-Yu Tsai
  2019-02-03 15:59 ` [PATCH 0/3] mmc: sunxi: Fix eMMC usage on H5 boards Chen-Yu Tsai
  3 siblings, 0 replies; 11+ messages in thread
From: Chen-Yu Tsai @ 2019-02-03 15:56 UTC (permalink / raw)
  To: Ulf Hansson, Maxime Ripard
  Cc: Chen-Yu Tsai, linux-mmc, linux-arm-kernel, devicetree,
	linux-kernel, linux-sunxi, Chris Blake

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 <wens@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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 0/3] mmc: sunxi: Fix eMMC usage on H5 boards
  2019-02-03 15:56 mmc: sunxi: Fix eMMC usage on H5 boards Chen-Yu Tsai
                   ` (2 preceding siblings ...)
  2019-02-03 15:56 ` [PATCH 3/3] arm64: dts: allwinner: h5: libretech-all-h3-cc: Mark eMMC HS-DDR 3.3V capable Chen-Yu Tsai
@ 2019-02-03 15:59 ` Chen-Yu Tsai
  3 siblings, 0 replies; 11+ messages in thread
From: Chen-Yu Tsai @ 2019-02-03 15:59 UTC (permalink / raw)
  To: Ulf Hansson, Maxime Ripard
  Cc: Chen-Yu Tsai, linux-mmc, linux-arm-kernel, devicetree,
	linux-kernel, linux-sunxi, Chris Blake

(Resent with proper subject tag.)

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] mmc: sunxi: Disable HS-DDR mode for H5 eMMC controller by default
  2019-02-03 15:56 ` [PATCH 1/3] mmc: sunxi: Disable HS-DDR mode for H5 eMMC controller by default Chen-Yu Tsai
@ 2019-02-04  9:32   ` Maxime Ripard
  0 siblings, 0 replies; 11+ messages in thread
From: Maxime Ripard @ 2019-02-04  9:32 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Ulf Hansson, linux-mmc, linux-arm-kernel, devicetree,
	linux-kernel, linux-sunxi, Chris Blake, stable

[-- Attachment #1: Type: text/plain, Size: 876 bytes --]

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 <chrisrblake93@gmail.com>
> Fixes: 07bafc1e3536 ("mmc: sunxi: Use new timing mode for A64 eMMC controller")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

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

Maxime

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] mmc: sunxi: Filter out unsupported modes declared in the device tree
  2019-02-03 15:56 ` [PATCH 2/3] mmc: sunxi: Filter out unsupported modes declared in the device tree Chen-Yu Tsai
@ 2019-02-04  9:34   ` Maxime Ripard
  2019-02-04 10:16     ` Chen-Yu Tsai
  0 siblings, 1 reply; 11+ messages in thread
From: Maxime Ripard @ 2019-02-04  9:34 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Ulf Hansson, linux-mmc, linux-arm-kernel, devicetree,
	linux-kernel, linux-sunxi, Chris Blake, stable

[-- Attachment #1: Type: text/plain, Size: 2225 bytes --]

On Sun, Feb 03, 2019 at 11:56:27PM +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: <stable@vger.kernel.org>
> Signed-off-by: Chen-Yu Tsai <wens@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;

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.

Maxime

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] mmc: sunxi: Filter out unsupported modes declared in the device tree
  2019-02-04  9:34   ` Maxime Ripard
@ 2019-02-04 10:16     ` Chen-Yu Tsai
  2019-02-04 13:41       ` Maxime Ripard
  0 siblings, 1 reply; 11+ messages in thread
From: Chen-Yu Tsai @ 2019-02-04 10:16 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Ulf Hansson, linux-mmc, linux-arm-kernel, devicetree,
	linux-kernel, linux-sunxi, Chris Blake, stable

On Mon, Feb 4, 2019 at 5:34 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Sun, Feb 03, 2019 at 11:56:27PM +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: <stable@vger.kernel.org>
> > Signed-off-by: Chen-Yu Tsai <wens@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;
>
> I've tested up to SDR104 and it works on the A64 at least

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] mmc: sunxi: Filter out unsupported modes declared in the device tree
  2019-02-04 10:16     ` Chen-Yu Tsai
@ 2019-02-04 13:41       ` Maxime Ripard
  2019-02-05  8:42         ` Chen-Yu Tsai
  0 siblings, 1 reply; 11+ messages in thread
From: Maxime Ripard @ 2019-02-04 13:41 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Ulf Hansson, linux-mmc, linux-arm-kernel, devicetree,
	linux-kernel, linux-sunxi, Chris Blake, stable

[-- Attachment #1: Type: text/plain, Size: 3157 bytes --]

On Mon, Feb 04, 2019 at 06:16:24PM +0800, Chen-Yu Tsai wrote:
> On Mon, Feb 4, 2019 at 5:34 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > On Sun, Feb 03, 2019 at 11:56:27PM +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: <stable@vger.kernel.org>
> > > Signed-off-by: Chen-Yu Tsai <wens@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;
> >
> > I've tested up to SDR104 and it works on the A64 at least
> 
> 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.

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

Maxime

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] mmc: sunxi: Filter out unsupported modes declared in the device tree
  2019-02-04 13:41       ` Maxime Ripard
@ 2019-02-05  8:42         ` Chen-Yu Tsai
  2019-02-05  9:51           ` Maxime Ripard
  0 siblings, 1 reply; 11+ messages in thread
From: Chen-Yu Tsai @ 2019-02-05  8:42 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Ulf Hansson, linux-mmc, linux-arm-kernel, devicetree,
	linux-kernel, linux-sunxi, Chris Blake, stable

On Mon, Feb 4, 2019 at 9:41 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Mon, Feb 04, 2019 at 06:16:24PM +0800, Chen-Yu Tsai wrote:
> > On Mon, Feb 4, 2019 at 5:34 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > >
> > > On Sun, Feb 03, 2019 at 11:56:27PM +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: <stable@vger.kernel.org>
> > > > Signed-off-by: Chen-Yu Tsai <wens@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;
> > >
> > > I've tested up to SDR104 and it works on the A64 at least
> >
> > 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.
>
> 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 :/

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] mmc: sunxi: Filter out unsupported modes declared in the device tree
  2019-02-05  8:42         ` Chen-Yu Tsai
@ 2019-02-05  9:51           ` Maxime Ripard
  0 siblings, 0 replies; 11+ messages in thread
From: Maxime Ripard @ 2019-02-05  9:51 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Ulf Hansson, linux-mmc, linux-arm-kernel, devicetree,
	linux-kernel, linux-sunxi, Chris Blake, stable

[-- Attachment #1: Type: text/plain, Size: 3802 bytes --]

On Tue, Feb 05, 2019 at 04:42:53PM +0800, Chen-Yu Tsai wrote:
> On Mon, Feb 4, 2019 at 9:41 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > On Mon, Feb 04, 2019 at 06:16:24PM +0800, Chen-Yu Tsai wrote:
> > > On Mon, Feb 4, 2019 at 5:34 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > >
> > > > On Sun, Feb 03, 2019 at 11:56:27PM +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: <stable@vger.kernel.org>
> > > > > Signed-off-by: Chen-Yu Tsai <wens@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;
> > > >
> > > > I've tested up to SDR104 and it works on the A64 at least
> > >
> > > 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.
> >
> > 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 :/
> 
> 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?

Yep, that's perfect

Maxime

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-02-05  9:51 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-03 15:56 mmc: sunxi: Fix eMMC usage on H5 boards Chen-Yu Tsai
2019-02-03 15:56 ` [PATCH 1/3] mmc: sunxi: Disable HS-DDR mode for H5 eMMC controller by default Chen-Yu Tsai
2019-02-04  9:32   ` Maxime Ripard
2019-02-03 15:56 ` [PATCH 2/3] mmc: sunxi: Filter out unsupported modes declared in the device tree Chen-Yu Tsai
2019-02-04  9:34   ` Maxime Ripard
2019-02-04 10:16     ` Chen-Yu Tsai
2019-02-04 13:41       ` Maxime Ripard
2019-02-05  8:42         ` Chen-Yu Tsai
2019-02-05  9:51           ` Maxime Ripard
2019-02-03 15:56 ` [PATCH 3/3] arm64: dts: allwinner: h5: libretech-all-h3-cc: Mark eMMC HS-DDR 3.3V capable Chen-Yu Tsai
2019-02-03 15:59 ` [PATCH 0/3] mmc: sunxi: Fix eMMC usage on H5 boards Chen-Yu Tsai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).