[PATCH v2 0/3] mmc: support eMMC driver type selection with DT

[PATCH v2 0/3] mmc: support eMMC driver type selection with DT

[Wolfram Sang]

Some of the Renesas R-Car boards need to set the eMMC driver type to type 1 to
get stable HS200/400 speeds (HS400 support not upstream yet). Because the eMMC
memories are fixed (and thus the drive type), I decided to implement the DT
setup in a similar fashion to ACPI [1]. While implementing, I realized that all
I did was not Renesas specific but generic, so I ended up putting it into the
core.

So, whenever the new property (introduced in patch 1) is used, and there is no
host driver specific select_drive_strength callback, then the generic callback
(introduced in patch 2) is used. An example usage can be found in patch 3.

Note that currently only eMMC is handled, but the code is designed for easy SD
addition if that ever becomes necessary.

A branch for the Salvator-X(S) boards can be found here:

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/topic/emmc-drive-strength

A wiki page describing my test procedure can be found here:

http://elinux.org/Tests:eMMC-fixed-drive-strength

Changes since RFC v1 are described per patch. It is mainly a rebase to mmc/next
and addressing Rob's comments. Tested on Salvator-X (R-Car H3 ES1.0) and
Salvator-XS (R-Car H3 ES2.0).

I know that patch 3 needs to go via Simon, but I still include it here for
completeness.

Thanks,

   Wolfram

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=51ced59cc02e0d08f61772c888480b9b1749cef1


Wolfram Sang (3):
  dt-bindings: mmc: describe new eMMC binding for fixed driver type
  mmc: parse new binding for eMMC fixed driver type
  arm64: renesas: salvator: set driver type for eMMC

 Documentation/devicetree/bindings/mmc/mmc.txt    |  3 +++
 arch/arm64/boot/dts/renesas/salvator-common.dtsi |  1 +
 drivers/mmc/core/core.c                          | 28 +++++++++++++++++++-----
 drivers/mmc/core/host.c                          | 11 +++++++++-
 include/linux/mmc/host.h                         |  2 ++
 5 files changed, 39 insertions(+), 6 deletions(-)

-- 
2.11.0

[PATCH v2 1/3] dt-bindings: mmc: describe new eMMC binding for fixed driver type

[Wolfram Sang]

Some boards may have to use a certain driver type (or drive strength) to
achieve stable eMMC communication. Describe a binding to set this up via
DT.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Acked-by: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org
---
Changes since RFC v1:
* rebased to mmc/next based on 4.14-rc3
* proper prefix for subject
* added Rob's ack

 Documentation/devicetree/bindings/mmc/mmc.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
index b32ade645ad97c..94a90b49a6925d 100644
--- a/Documentation/devicetree/bindings/mmc/mmc.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc.txt
@@ -53,6 +53,9 @@ Optional properties:
 - no-sdio: controller is limited to send sdio cmd during initialization
 - no-sd: controller is limited to send sd cmd during initialization
 - no-mmc: controller is limited to send mmc cmd during initialization
+- fixed-emmc-driver-type: for non-removable eMMC, enforce this driver type.
+  The value <n> is the driver type as specified in the eMMC specification
+  (table 206 in spec version 5.1).
 
 *NOTE* on CD and WP polarity. To use common for all SD/MMC host controllers line
 polarity properties, we have to fix the meaning of the "normal" and "inverted"
-- 
2.11.0

[PATCH v2 2/3] mmc: parse new binding for eMMC fixed driver type

[Wolfram Sang]

Parse the new binding and store it in the host struct after doing some
sanity checks. The code is designed to support fixed SD driver type if
we ever need that.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
Changes since RFC v1:
* rebased to mmc/next based on 4.14-rc3

 drivers/mmc/core/core.c  | 28 +++++++++++++++++++++++-----
 drivers/mmc/core/host.c  | 11 ++++++++++-
 include/linux/mmc/host.h |  2 ++
 3 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index c287d297710aa4..64b454efc924c5 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1785,6 +1785,18 @@ void mmc_set_driver_type(struct mmc_host *host, unsigned int drv_type)
 	mmc_set_ios(host);
 }
 
+static int mmc_select_fixed_drive_strength(struct mmc_card *card,
+				       unsigned int max_dtr, int host_drv,
+				       int card_drv, int *drv_type)
+{
+	int ret = 0, preset = card->host->fixed_drv_type;
+
+	if (card->type == MMC_TYPE_MMC)
+		ret = card_drv & mmc_driver_type_mask(preset) ? preset : 0;
+
+	return ret;
+}
+
 int mmc_select_drive_strength(struct mmc_card *card, unsigned int max_dtr,
 			      int card_drv_type, int *drv_type)
 {
@@ -1793,7 +1805,7 @@ int mmc_select_drive_strength(struct mmc_card *card, unsigned int max_dtr,
 
 	*drv_type = 0;
 
-	if (!host->ops->select_drive_strength)
+	if (!host->ops->select_drive_strength && !host->fixed_drv_type)
 		return 0;
 
 	/* Use SD definition of driver strength for hosts */
@@ -1812,10 +1824,16 @@ int mmc_select_drive_strength(struct mmc_card *card, unsigned int max_dtr,
 	 * information and let the hardware specific code
 	 * return what is possible given the options
 	 */
-	return host->ops->select_drive_strength(card, max_dtr,
-						host_drv_type,
-						card_drv_type,
-						drv_type);
+	if (host->ops->select_drive_strength)
+		return host->ops->select_drive_strength(card, max_dtr,
+							host_drv_type,
+							card_drv_type,
+							drv_type);
+	else
+		return mmc_select_fixed_drive_strength(card, max_dtr,
+							host_drv_type,
+							card_drv_type,
+							drv_type);
 }
 
 /*
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 1539ce0da19646..d69afb8762dd52 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -179,7 +179,7 @@ static void mmc_retune_timer(unsigned long data)
 int mmc_of_parse(struct mmc_host *host)
 {
 	struct device *dev = host->parent;
-	u32 bus_width;
+	u32 bus_width, drv_type = 0;
 	int ret;
 	bool cd_cap_invert, cd_gpio_invert = false;
 	bool ro_cap_invert, ro_gpio_invert = false;
@@ -321,6 +321,15 @@ int mmc_of_parse(struct mmc_host *host)
 	if (device_property_read_bool(dev, "no-mmc"))
 		host->caps2 |= MMC_CAP2_NO_MMC;
 
+	/* Must be after "non-removable" check */
+	if (device_property_read_u32(dev, "fixed-emmc-driver-type", &drv_type) == 0) {
+		if (host->caps & MMC_CAP_NONREMOVABLE)
+			host->fixed_drv_type = drv_type;
+		else
+			dev_err(host->parent,
+				"can't use fixed driver type, media is removable\n");
+	}
+
 	host->dsr_req = !device_property_read_u32(dev, "dsr", &host->dsr);
 	if (host->dsr_req && (host->dsr & ~0xffff)) {
 		dev_err(host->parent,
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 9109265fe52991..ce2075d6f429cf 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -354,6 +354,8 @@ struct mmc_host {
 #define MMC_CAP2_CQE		(1 << 23)	/* Has eMMC command queue engine */
 #define MMC_CAP2_CQE_DCMD	(1 << 24)	/* CQE can issue a direct command */
 
+	int			fixed_drv_type;	/* fixed driver type for non-removable media */
+
 	mmc_pm_flag_t		pm_caps;	/* supported pm features */
 
 	/* host specific block data */
-- 
2.11.0

Re: [PATCH v2 2/3] mmc: parse new binding for eMMC fixed driver type

[Ulf Hansson]

On 4 October 2017 at 19:39, Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> Parse the new binding and store it in the host struct after doing some
> sanity checks. The code is designed to support fixed SD driver type if
> we ever need that.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
> Changes since RFC v1:
> * rebased to mmc/next based on 4.14-rc3
>
>  drivers/mmc/core/core.c  | 28 +++++++++++++++++++++++-----
>  drivers/mmc/core/host.c  | 11 ++++++++++-
>  include/linux/mmc/host.h |  2 ++
>  3 files changed, 35 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index c287d297710aa4..64b454efc924c5 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -1785,6 +1785,18 @@ void mmc_set_driver_type(struct mmc_host *host, unsigned int drv_type)
>         mmc_set_ios(host);
>  }
>
> +static int mmc_select_fixed_drive_strength(struct mmc_card *card,
> +                                      unsigned int max_dtr, int host_drv,
> +                                      int card_drv, int *drv_type)
> +{
> +       int ret = 0, preset = card->host->fixed_drv_type;
> +
> +       if (card->type == MMC_TYPE_MMC)
> +               ret = card_drv & mmc_driver_type_mask(preset) ? preset : 0;
> +
> +       return ret;
> +}

After a second thought, I would rather prefer to have this code
implemented in mmc.c, in mmc_select_driver_type(). If we later find a
case for SD/SDIO, we can consider to move this to core.c.

Moreover, I think when host->fixed_drv_type has a value set, using
that value should take precedence over calling
mmc_select_drive_strength(). That is not the case below.

Sorry for not realizing this in v1.

> +
>  int mmc_select_drive_strength(struct mmc_card *card, unsigned int max_dtr,
>                               int card_drv_type, int *drv_type)
>  {
> @@ -1793,7 +1805,7 @@ int mmc_select_drive_strength(struct mmc_card *card, unsigned int max_dtr,
>
>         *drv_type = 0;
>
> -       if (!host->ops->select_drive_strength)
> +       if (!host->ops->select_drive_strength && !host->fixed_drv_type)
>                 return 0;
>
>         /* Use SD definition of driver strength for hosts */
> @@ -1812,10 +1824,16 @@ int mmc_select_drive_strength(struct mmc_card *card, unsigned int max_dtr,
>          * information and let the hardware specific code
>          * return what is possible given the options
>          */
> -       return host->ops->select_drive_strength(card, max_dtr,
> -                                               host_drv_type,
> -                                               card_drv_type,
> -                                               drv_type);
> +       if (host->ops->select_drive_strength)
> +               return host->ops->select_drive_strength(card, max_dtr,
> +                                                       host_drv_type,
> +                                                       card_drv_type,
> +                                                       drv_type);
> +       else
> +               return mmc_select_fixed_drive_strength(card, max_dtr,
> +                                                       host_drv_type,
> +                                                       card_drv_type,
> +                                                       drv_type);
>  }

[...]

Kind regards
Uffe

Re: [PATCH v2 2/3] mmc: parse new binding for eMMC fixed driver type

[Simon Horman]

On Wed, Oct 04, 2017 at 07:39:19PM +0200, Wolfram Sang wrote:
> Parse the new binding and store it in the host struct after doing some
> sanity checks. The code is designed to support fixed SD driver type if
> we ever need that.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

[PATCH v2 3/3] arm64: renesas: salvator: set driver type for eMMC

[Wolfram Sang]

These boards are known to have eMMC issues with the default driver type.
Specify a working one.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
Changes since RFC v1:
* rebased to mmc/next based on 4.14-rc3
* moved the property to salvator common because the BSP applies this property
  to all Salvator variants, too

 arch/arm64/boot/dts/renesas/salvator-common.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
index 4786c67b5e6527..215e2d11a0490a 100644
--- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
+++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
@@ -579,6 +579,7 @@
 	bus-width = <8>;
 	mmc-hs200-1_8v;
 	non-removable;
+	fixed-emmc-driver-type = <1>;
 	status = "okay";
 };
 
-- 
2.11.0

Re: [PATCH v2 3/3] arm64: renesas: salvator: set driver type for eMMC

[Simon Horman]

On Wed, Oct 04, 2017 at 07:39:20PM +0200, Wolfram Sang wrote:
> These boards are known to have eMMC issues with the default driver type.
> Specify a working one.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>


Reviewed-by: Simon Horman <horms+renesas@verge.net.au>