Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220623080009.1775574-1-vincent.whitchurch@axis.com>
In-Reply-To: <20220623080009.1775574-1-vincent.whitchurch@axis.com>
From:   Ulf Hansson <ulf.hansson@linaro.org>
Date:   Thu, 23 Jun 2022 15:53:41 +0200
Message-ID: <CAPDyKFpg9Gs3qOeXWM2A1gSDEKA89i72NoTcF0M_a3jO-AC+dw@mail.gmail.com>
Subject: Re: [PATCH] mmc: core: Allow speed modes to be adjusted via module param
To:     Vincent Whitchurch <vincent.whitchurch@axis.com>
Cc:     kernel@axis.com, linux-mmc@vger.kernel.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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
>