| Message ID | 20220623080009.1775574-1-vincent.whitchurch@axis.com |
|---|---|
| State | New |
| Headers | show |
| Series | mmc: core: Allow speed modes to be adjusted via module param | expand |
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 >
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) {
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
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)
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(+)