[PATCH] mmc: core: Allow speed modes to be adjusted via module param

[PATCH] mmc: core: Allow speed modes to be adjusted via module param

[Vincent Whitchurch]

During board verification, there is a need to test the various supported
eMMC/SD speed modes.  However, since the framework chooses the best mode
supported by the card and the host controller's caps, this currently
necessitates changing the devicetree for every iteration.

To make changing the modes easier, allow the various host controller
capabilities to be cleared via a module parameter.  (A per-controller
debugfs wouldn't work since the controller needs to be re-probed to
trigger re-init of cards.  A module parameter is used instead of a
global debugfs to allow this to be also set via the kernel command
line.)

The values to be written are the raw MMC_CAP* values from
include/linux/mmc/host.h.  This is rather low-level, and these defines
are not guaranteed to be stable, but it is perhaps good enough for the
indented use case.  A warning is emitted when the caps clearing is in
effect.

Example of use:

 # grep timing /sys/kernel/debug/mmc0/ios
 timing spec:	9 (mmc HS200)

 // MMC_CAP2_HS200_1_8V_SDR
 # echo $((1 << 5)) > /sys/module/mmc_core/parameters/caps2_clear

 # echo 16d40000.mmc > /sys/bus/platform/drivers/dwmmc_exynos/unbind
 # echo 16d40000.mmc > /sys/bus/platform/drivers/dwmmc_exynos/bind
 # grep timing /sys/kernel/debug/mmc0/ios
 timing spec:	8 (mmc DDR52)

 // MMC_CAP_1_8V_DDR
 # echo $((1 << 12)) > /sys/module/mmc_core/parameters/caps_clear

 # echo 16d40000.mmc > /sys/bus/platform/drivers/dwmmc_exynos/unbind
 # echo 16d40000.mmc > /sys/bus/platform/drivers/dwmmc_exynos/bind
 # grep timing /sys/kernel/debug/mmc0/ios
 timing spec:	1 (mmc high-speed)

 # echo 0 > /sys/module/mmc_core/parameters/caps2_clear

 # echo 16d40000.mmc > /sys/bus/platform/drivers/dwmmc_exynos/unbind
 # echo 16d40000.mmc > /sys/bus/platform/drivers/dwmmc_exynos/bind
 # grep timing /sys/kernel/debug/mmc0/ios
 timing spec:	9 (mmc HS200)

Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
---
 drivers/mmc/core/host.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 2ed2b4d5e5a5..37971b7c7f62 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -34,6 +34,10 @@
 #define cls_dev_to_mmc_host(d)	container_of(d, struct mmc_host, class_dev)
 
 static DEFINE_IDA(mmc_host_ida);
+static unsigned int caps_clear, caps2_clear;
+
+module_param(caps_clear, uint, 0644);
+module_param(caps2_clear, uint, 0644);
 
 #ifdef CONFIG_PM_SLEEP
 static int mmc_host_class_prepare(struct device *dev)
@@ -411,6 +415,14 @@ int mmc_of_parse(struct mmc_host *host)
 		host->caps2 &= ~(MMC_CAP2_HS400_1_8V | MMC_CAP2_HS400_1_2V |
 				 MMC_CAP2_HS400_ES);
 
+	if (caps_clear || caps2_clear)
+		dev_warn(host->parent,
+			 "clearing host controller caps %#x caps2 %#x\n",
+			 caps_clear, caps2_clear);
+
+	host->caps &= ~caps_clear;
+	host->caps2 &= ~caps2_clear;
+
 	/* 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)
-- 
2.34.1

Re: [PATCH] mmc: core: Allow speed modes to be adjusted via module param

[Ulf Hansson]

On Thu, 23 Jun 2022 at 10:00, Vincent Whitchurch
<vincent.whitchurch@axis.com> wrote:
>
> During board verification, there is a need to test the various supported
> eMMC/SD speed modes.  However, since the framework chooses the best mode
> supported by the card and the host controller's caps, this currently
> necessitates changing the devicetree for every iteration.
>
> To make changing the modes easier, allow the various host controller
> capabilities to be cleared via a module parameter.  (A per-controller
> debugfs wouldn't work since the controller needs to be re-probed to
> trigger re-init of cards.  A module parameter is used instead of a

I think we could make use of a per-controller debugfs thing, if used
in combination with MMC_CAP_AGGRESSIVE_PM and runtime PM.

As runtime PM also has sysfs interface for each device, we can control
runtime PM for the card's device (to trigger re-initialization of the
card). In between runtime suspend/resume of the card's device, we
should be able to change the supported speed modes, through debug fs.

Would this work for you?

Kind regards
Uffe

> global debugfs to allow this to be also set via the kernel command
> line.)
>
> The values to be written are the raw MMC_CAP* values from
> include/linux/mmc/host.h.  This is rather low-level, and these defines
> are not guaranteed to be stable, but it is perhaps good enough for the
> indented use case.  A warning is emitted when the caps clearing is in
> effect.
>
> Example of use:
>
>  # grep timing /sys/kernel/debug/mmc0/ios
>  timing spec:   9 (mmc HS200)
>
>  // MMC_CAP2_HS200_1_8V_SDR
>  # echo $((1 << 5)) > /sys/module/mmc_core/parameters/caps2_clear
>
>  # echo 16d40000.mmc > /sys/bus/platform/drivers/dwmmc_exynos/unbind
>  # echo 16d40000.mmc > /sys/bus/platform/drivers/dwmmc_exynos/bind
>  # grep timing /sys/kernel/debug/mmc0/ios
>  timing spec:   8 (mmc DDR52)
>
>  // MMC_CAP_1_8V_DDR
>  # echo $((1 << 12)) > /sys/module/mmc_core/parameters/caps_clear
>
>  # echo 16d40000.mmc > /sys/bus/platform/drivers/dwmmc_exynos/unbind
>  # echo 16d40000.mmc > /sys/bus/platform/drivers/dwmmc_exynos/bind
>  # grep timing /sys/kernel/debug/mmc0/ios
>  timing spec:   1 (mmc high-speed)
>
>  # echo 0 > /sys/module/mmc_core/parameters/caps2_clear
>
>  # echo 16d40000.mmc > /sys/bus/platform/drivers/dwmmc_exynos/unbind
>  # echo 16d40000.mmc > /sys/bus/platform/drivers/dwmmc_exynos/bind
>  # grep timing /sys/kernel/debug/mmc0/ios
>  timing spec:   9 (mmc HS200)
>
> Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
> ---
>  drivers/mmc/core/host.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> index 2ed2b4d5e5a5..37971b7c7f62 100644
> --- a/drivers/mmc/core/host.c
> +++ b/drivers/mmc/core/host.c
> @@ -34,6 +34,10 @@
>  #define cls_dev_to_mmc_host(d) container_of(d, struct mmc_host, class_dev)
>
>  static DEFINE_IDA(mmc_host_ida);
> +static unsigned int caps_clear, caps2_clear;
> +
> +module_param(caps_clear, uint, 0644);
> +module_param(caps2_clear, uint, 0644);
>
>  #ifdef CONFIG_PM_SLEEP
>  static int mmc_host_class_prepare(struct device *dev)
> @@ -411,6 +415,14 @@ int mmc_of_parse(struct mmc_host *host)
>                 host->caps2 &= ~(MMC_CAP2_HS400_1_8V | MMC_CAP2_HS400_1_2V |
>                                  MMC_CAP2_HS400_ES);
>
> +       if (caps_clear || caps2_clear)
> +               dev_warn(host->parent,
> +                        "clearing host controller caps %#x caps2 %#x\n",
> +                        caps_clear, caps2_clear);
> +
> +       host->caps &= ~caps_clear;
> +       host->caps2 &= ~caps2_clear;
> +
>         /* 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)
> --
> 2.34.1
>

Re: [PATCH] mmc: core: Allow speed modes to be adjusted via module param

[Vincent Whitchurch]

On Thu, Jun 23, 2022 at 03:53:41PM +0200, Ulf Hansson wrote:
> On Thu, 23 Jun 2022 at 10:00, Vincent Whitchurch
> <vincent.whitchurch@axis.com> wrote:
> > During board verification, there is a need to test the various supported
> > eMMC/SD speed modes.  However, since the framework chooses the best mode
> > supported by the card and the host controller's caps, this currently
> > necessitates changing the devicetree for every iteration.
> >
> > To make changing the modes easier, allow the various host controller
> > capabilities to be cleared via a module parameter.  (A per-controller
> > debugfs wouldn't work since the controller needs to be re-probed to
> > trigger re-init of cards.  A module parameter is used instead of a
> 
> I think we could make use of a per-controller debugfs thing, if used
> in combination with MMC_CAP_AGGRESSIVE_PM and runtime PM.
> 
> As runtime PM also has sysfs interface for each device, we can control
> runtime PM for the card's device (to trigger re-initialization of the
> card). In between runtime suspend/resume of the card's device, we
> should be able to change the supported speed modes, through debug fs.
> 
> Would this work for you?

I got it to work with the below commands and the following patch.  Note
that:

(1) MMC_CAP_AGGRESSIVE_PM also needs to be turned on via debugfs to
    avoid having to patch the kernel.  The cap is checked on every call
    to runtime_suspend so it (currently) works to set it without
    re-probing the host.

(2) I had to call mmc_select_card_type() even if there is an old card
    since currently it's only called from mmc_decode_ext_csd().

Also, unlike the module parameter, this can't be set from bootargs, but
that part is not important for my use case.

 root@(none):/sys/kernel/debug/mmc0# grep timing ios
 timing spec:	9 (mmc HS200)
 
 // Turn on MMC_CAP_AGGRESSIVE_PM and re-trigger runtime suspend
 root@(none):/sys/kernel/debug/mmc0# echo $(($(cat caps) | (1 << 7))) > caps
 root@(none):/sys/kernel/debug/mmc0# echo on > /sys/bus/mmc/devices/mmc0\:0001/power/control
 root@(none):/sys/kernel/debug/mmc0# echo auto > /sys/bus/mmc/devices/mmc0\:0001/power/control
 
 // MMC_CAP2_HS200_1_8V_SDR
 root@(none):/sys/kernel/debug/mmc0# echo $(($(cat caps2) & ~(1 << 5))) > caps2
 root@(none):/sys/kernel/debug/mmc0# echo on > /sys/bus/mmc/devices/mmc0\:0001/power/control
 root@(none):/sys/kernel/debug/mmc0# grep timing ios
 timing spec:	8 (mmc DDR52)

8<----
diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
index 3fdbc801e64a..721925300611 100644
--- a/drivers/mmc/core/debugfs.c
+++ b/drivers/mmc/core/debugfs.c
@@ -12,9 +12,12 @@
 #include <linux/slab.h>
 #include <linux/stat.h>
 #include <linux/fault-inject.h>
+#include <linux/time.h>
 
 #include <linux/mmc/card.h>
 #include <linux/mmc/host.h>
+#include <linux/mmc/mmc.h>
+#include <linux/mmc/sd.h>
 
 #include "core.h"
 #include "card.h"
@@ -223,6 +226,47 @@ static int mmc_clock_opt_set(void *data, u64 val)
 DEFINE_DEBUGFS_ATTRIBUTE(mmc_clock_fops, mmc_clock_opt_get, mmc_clock_opt_set,
 	"%llu\n");
 
+static int mmc_caps_get(void *data, u64 *val)
+{
+	*val = *(u32 *)data;
+	return 0;
+}
+
+static int mmc_caps_set(void *data, u64 val)
+{
+	u32 *caps = data;
+	u32 diff = *caps ^ val;
+	u32 allowed = MMC_CAP_AGGRESSIVE_PM |
+		MMC_CAP_SD_HIGHSPEED |
+		MMC_CAP_MMC_HIGHSPEED |
+		MMC_CAP_UHS |
+		MMC_CAP_DDR;
+
+	if (diff & ~allowed)
+		return -EINVAL;
+
+	*caps = val;
+
+	return 0;
+}
+
+static int mmc_caps2_set(void *data, u64 val)
+{
+	u32 *caps = data;
+	u32 diff = *caps ^ val;
+	u32 allowed = MMC_CAP2_HSX00_1_8V | MMC_CAP2_HSX00_1_2V;
+
+	if (diff & ~allowed)
+		return -EINVAL;
+
+	*caps = val;
+
+	return 0;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(mmc_caps_fops, mmc_caps_get, mmc_caps_set, "0x%08llx\n");
+DEFINE_DEBUGFS_ATTRIBUTE(mmc_caps2_fops, mmc_caps_get, mmc_caps2_set, "0x%08llx\n");
+
 void mmc_add_host_debugfs(struct mmc_host *host)
 {
 	struct dentry *root;
@@ -231,8 +275,10 @@ void mmc_add_host_debugfs(struct mmc_host *host)
 	host->debugfs_root = root;
 
 	debugfs_create_file("ios", S_IRUSR, root, host, &mmc_ios_fops);
-	debugfs_create_x32("caps", S_IRUSR, root, &host->caps);
-	debugfs_create_x32("caps2", S_IRUSR, root, &host->caps2);
+	debugfs_create_file("caps", S_IRUSR | S_IWUSR, root, &host->caps,
+			&mmc_caps_fops);
+	debugfs_create_file("caps2", S_IRUSR | S_IWUSR, root, &host->caps2,
+			&mmc_caps2_fops);
 	debugfs_create_file_unsafe("clock", S_IRUSR | S_IWUSR, root, host,
 				   &mmc_clock_fops);
 
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 89cd48fcec79..c79c26add3da 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1730,7 +1730,8 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
 
 		/* Erase size depends on CSD and Extended CSD */
 		mmc_set_erase_size(card);
-	}
+	} else
+		mmc_select_card_type(card);
 
 	/* Enable ERASE_GRP_DEF. This bit is lost after a reset or power off. */
 	if (card->ext_csd.rev >= 3) {

Re: [PATCH] mmc: core: Allow speed modes to be adjusted via module param

[Ulf Hansson]

On Mon, 27 Jun 2022 at 12:08, Vincent Whitchurch
<vincent.whitchurch@axis.com> wrote:
>
> On Thu, Jun 23, 2022 at 03:53:41PM +0200, Ulf Hansson wrote:
> > On Thu, 23 Jun 2022 at 10:00, Vincent Whitchurch
> > <vincent.whitchurch@axis.com> wrote:
> > > During board verification, there is a need to test the various supported
> > > eMMC/SD speed modes.  However, since the framework chooses the best mode
> > > supported by the card and the host controller's caps, this currently
> > > necessitates changing the devicetree for every iteration.
> > >
> > > To make changing the modes easier, allow the various host controller
> > > capabilities to be cleared via a module parameter.  (A per-controller
> > > debugfs wouldn't work since the controller needs to be re-probed to
> > > trigger re-init of cards.  A module parameter is used instead of a
> >
> > I think we could make use of a per-controller debugfs thing, if used
> > in combination with MMC_CAP_AGGRESSIVE_PM and runtime PM.
> >
> > As runtime PM also has sysfs interface for each device, we can control
> > runtime PM for the card's device (to trigger re-initialization of the
> > card). In between runtime suspend/resume of the card's device, we
> > should be able to change the supported speed modes, through debug fs.
> >
> > Would this work for you?
>
> I got it to work with the below commands and the following patch.  Note
> that:
>
> (1) MMC_CAP_AGGRESSIVE_PM also needs to be turned on via debugfs to
>     avoid having to patch the kernel.  The cap is checked on every call
>     to runtime_suspend so it (currently) works to set it without
>     re-probing the host.
>
> (2) I had to call mmc_select_card_type() even if there is an old card
>     since currently it's only called from mmc_decode_ext_csd().
>
> Also, unlike the module parameter, this can't be set from bootargs, but
> that part is not important for my use case.
>
>  root@(none):/sys/kernel/debug/mmc0# grep timing ios
>  timing spec:   9 (mmc HS200)
>
>  // Turn on MMC_CAP_AGGRESSIVE_PM and re-trigger runtime suspend
>  root@(none):/sys/kernel/debug/mmc0# echo $(($(cat caps) | (1 << 7))) > caps
>  root@(none):/sys/kernel/debug/mmc0# echo on > /sys/bus/mmc/devices/mmc0\:0001/power/control
>  root@(none):/sys/kernel/debug/mmc0# echo auto > /sys/bus/mmc/devices/mmc0\:0001/power/control
>
>  // MMC_CAP2_HS200_1_8V_SDR
>  root@(none):/sys/kernel/debug/mmc0# echo $(($(cat caps2) & ~(1 << 5))) > caps2
>  root@(none):/sys/kernel/debug/mmc0# echo on > /sys/bus/mmc/devices/mmc0\:0001/power/control
>  root@(none):/sys/kernel/debug/mmc0# grep timing ios
>  timing spec:   8 (mmc DDR52)
>
> 8<----
> diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
> index 3fdbc801e64a..721925300611 100644
> --- a/drivers/mmc/core/debugfs.c
> +++ b/drivers/mmc/core/debugfs.c
> @@ -12,9 +12,12 @@
>  #include <linux/slab.h>
>  #include <linux/stat.h>
>  #include <linux/fault-inject.h>
> +#include <linux/time.h>
>
>  #include <linux/mmc/card.h>
>  #include <linux/mmc/host.h>
> +#include <linux/mmc/mmc.h>
> +#include <linux/mmc/sd.h>
>
>  #include "core.h"
>  #include "card.h"
> @@ -223,6 +226,47 @@ static int mmc_clock_opt_set(void *data, u64 val)
>  DEFINE_DEBUGFS_ATTRIBUTE(mmc_clock_fops, mmc_clock_opt_get, mmc_clock_opt_set,
>         "%llu\n");
>
> +static int mmc_caps_get(void *data, u64 *val)
> +{
> +       *val = *(u32 *)data;
> +       return 0;
> +}
> +
> +static int mmc_caps_set(void *data, u64 val)
> +{
> +       u32 *caps = data;
> +       u32 diff = *caps ^ val;
> +       u32 allowed = MMC_CAP_AGGRESSIVE_PM |
> +               MMC_CAP_SD_HIGHSPEED |
> +               MMC_CAP_MMC_HIGHSPEED |
> +               MMC_CAP_UHS |
> +               MMC_CAP_DDR;
> +
> +       if (diff & ~allowed)
> +               return -EINVAL;
> +
> +       *caps = val;
> +
> +       return 0;
> +}
> +
> +static int mmc_caps2_set(void *data, u64 val)
> +{
> +       u32 *caps = data;
> +       u32 diff = *caps ^ val;
> +       u32 allowed = MMC_CAP2_HSX00_1_8V | MMC_CAP2_HSX00_1_2V;
> +
> +       if (diff & ~allowed)
> +               return -EINVAL;
> +
> +       *caps = val;
> +
> +       return 0;
> +}
> +
> +DEFINE_DEBUGFS_ATTRIBUTE(mmc_caps_fops, mmc_caps_get, mmc_caps_set, "0x%08llx\n");
> +DEFINE_DEBUGFS_ATTRIBUTE(mmc_caps2_fops, mmc_caps_get, mmc_caps2_set, "0x%08llx\n");
> +
>  void mmc_add_host_debugfs(struct mmc_host *host)
>  {
>         struct dentry *root;
> @@ -231,8 +275,10 @@ void mmc_add_host_debugfs(struct mmc_host *host)
>         host->debugfs_root = root;
>
>         debugfs_create_file("ios", S_IRUSR, root, host, &mmc_ios_fops);
> -       debugfs_create_x32("caps", S_IRUSR, root, &host->caps);
> -       debugfs_create_x32("caps2", S_IRUSR, root, &host->caps2);
> +       debugfs_create_file("caps", S_IRUSR | S_IWUSR, root, &host->caps,
> +                       &mmc_caps_fops);
> +       debugfs_create_file("caps2", S_IRUSR | S_IWUSR, root, &host->caps2,
> +                       &mmc_caps2_fops);
>         debugfs_create_file_unsafe("clock", S_IRUSR | S_IWUSR, root, host,
>                                    &mmc_clock_fops);
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 89cd48fcec79..c79c26add3da 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1730,7 +1730,8 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
>
>                 /* Erase size depends on CSD and Extended CSD */
>                 mmc_set_erase_size(card);
> -       }
> +       } else
> +               mmc_select_card_type(card);
>
>         /* Enable ERASE_GRP_DEF. This bit is lost after a reset or power off. */
>         if (card->ext_csd.rev >= 3) {

Overall, the approach seems reasonable to me. Perhaps we can
reorganize the code so mmc_select_card_type() is always called from
mmc_init_card(), rather than from mmc_decode_ext_csd() too.

Kind regards
Uffe