MIME-Version: 1.0
References: <20211116190244.1417591-1-john@metanate.com> <20211116190244.1417591-3-john@metanate.com>
In-Reply-To: <20211116190244.1417591-3-john@metanate.com>
From:   Ulf Hansson <ulf.hansson@linaro.org>
Date:   Tue, 23 Nov 2021 20:34:20 +0100
Message-ID: <CAPDyKFp1zMBUfK7LteW0yEfTpqtU+P+EybLsJBFx_r54HwFdMg@mail.gmail.com>
Subject: Re: [PATCH 2/2] mmc: dw_mmc: use standard "mmc" alias stem
To:     John Keeping <john@metanate.com>
Cc:     linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org,
        Jaehoon Chung <jh80.chung@samsung.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, 16 Nov 2021 at 20:02, John Keeping <john@metanate.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@metanate.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
>