Re: [RFC PATCH 2/2] Make cmdq_en attribute writeable

[Ulf Hansson <ulf.hansson@linaro.org>]

On Mon, 15 Feb 2021 at 01:33, Luca Porzio <porzio@gmail.com> wrote:
>
> cmdq_en attribute in sysfs now can now be written.
> When 0 is written:
>   CMDQ is disabled and kept disabled across device reboots.
> When 1 is written:
>   CMDQ mode is instantly reneabled (if supported).
>
> Signed-off-by: Luca Porzio <lporzio@micron.com>
> Signed-off-by: Zhan Liu <zliua@micron.com>
> ---
>  drivers/mmc/core/mmc.c   | 152 ++++++++++++++++++++++++++++++---------
>  include/linux/mmc/card.h |   1 +
>  2 files changed, 118 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c

[...]

> +static ssize_t cmdq_en_store(struct device *dev, struct device_attribute *attr,
> +                                const char *buf, size_t count)
> +{
> +       struct mmc_card *card = mmc_dev_to_card(dev);
> +       struct mmc_host *host = card->host;
> +       unsigned long enable;
> +       int err;
> +
> +       if (!card || kstrtoul(buf, 0, &enable))
> +               return -EINVAL;
> +       if (!card->ext_csd.cmdq_support)
> +               return -EOPNOTSUPP;
> +
> +       enable = !!enable;
> +       if (enable == card->ext_csd.cmdq_en)
> +               return count;
> +
> +       mmc_get_card(card, NULL);

This means adding a new path for when the host needs to get locked
(claimed), which is the opposite direction of what we have been
working on for SD/eMMC during the last couple of years.

Please have a look at mmc_blk_issue_drv_op(), where you can find how
these kinds of requests are being funneled through the mmc block
device layer instead. This is the preferred option.

That said, I am actually wondering if we perhaps could manage the
enable/disable of CQE "automagically" for FFU, along the lines of what
we do for RPMB already. In fact, maybe the best/easiest approach is to
*always* disable CQE when there is a request being received through
the mmc ioctl interface. Of course, then if we disabled CQE we should
also re-enable it when the ioctl request has been completed.

What do you think?

> +       card->force_disable_cmdq = !enable;
> +       err = mmc_cmdq_setup(host, card);
> +       mmc_put_card(card, NULL);
> +
> +       if (err)
> +               return err;
> +       else
> +               return count;
> +}
> +
> +static DEVICE_ATTR_RW(cmdq_en);
> +
>

[...]

Kind regards
Uffe