linux-kernel - Re: [PATCH 2/2] mmc: dw_mmc: use standard "mmc" alias stem

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]

Date:   Tue, 23 Nov 2021 20:34:20 +0100
From:   Ulf Hansson <ulf.hansson@...aro.org>
To:     John Keeping <john@...anate.com>
Cc:     linux-mmc@...r.kernel.org, linux-kernel@...r.kernel.org,
        Jaehoon Chung <jh80.chung@...sung.com>
Subject: Re: [PATCH 2/2] mmc: dw_mmc: use standard "mmc" alias stem

On Tue, 16 Nov 2021 at 20:02, John Keeping <john@...anate.com> wrote:
>
> The standard stem for MMC aliases is "mmc" and this is used by the MMC
> core to set the slot index.

This isn't the correct description of the mmc aliases. The below text
is copied from the DT doc:

"It is possible to assign a fixed index mmcN to an MMC host controller
(and the corresponding mmcblkN devices) by defining an alias in the
/aliases device tree node."

>
> Use this in preference to the non-standard "mshc" stem when setting the
> controller ID to avoid needing two aliases for each MMC device in order
> to cover both the core and dw_mmc-specific functionality.
>
> The old "mshc" lookup is kept for backwards compatibility.

The mshc alias is really weird!

It looks like some leftover from when the dw_mmc controller supported
multiple slots. This support was dropped a long time ago, simply
because it never really worked - and it was not worth trying to. Only
one slot per controller is supported.

Rather than re-using the mmc alias in the same weird way as the mshc
alias, I suggest we try to remove parsing of the mshc aliases
completely. By looking at the corresponding code and in combination
with the DTS files, it certainly looks doable to me. Do you want to
have a look at it?

Additionally, there is no need to deprecate the mshc alias binding, as
it seems like it has never been documented. :-)

Kind regards
Uffe

>
> Signed-off-by: John Keeping <john@...anate.com>
> ---
>  drivers/mmc/host/dw_mmc.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 37af1245304b..6ffaa354410a 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -112,7 +112,11 @@ int dw_mci_of_alias_get_id(struct dw_mci *host)
>         if (WARN_ON(!host->dev->of_node))
>                 return 0;
>
> -       ctrl_id = of_alias_get_id(host->dev->of_node, "mshc");
> +       ctrl_id = of_alias_get_id(host->dev->of_node, "mmc");
> +
> +       /* Compatibility fallback for old device trees. */
> +       if (ctrl_id < 0)
> +               ctrl_id = of_alias_get_id(host->dev->of_node, "mshc");
>
>         if (ctrl_id < 0)
>                 ctrl_id = 0;
> --
> 2.34.0
>