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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20220627100840.GA12705@axis.com>
Date:   Mon, 27 Jun 2022 12:08:40 +0200
From:   Vincent Whitchurch <vincent.whitchurch@...s.com>
To:     Ulf Hansson <ulf.hansson@...aro.org>
CC:     kernel <kernel@...s.com>,
        "linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mmc: core: Allow speed modes to be adjusted via module
 param

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@...s.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) {