Re: [PATCH] [RFC] mmc: mmci: Add support for probing bus voltage level translator

[Marek Vasut <marex@denx.de>]

On 1/13/21 12:38 PM, Ulf Hansson wrote:
[...]
>>>>    static int mmci_of_parse(struct device_node *np, struct mmc_host *mmc)
>>>>    {
>>>>           struct mmci_host *host = mmc_priv(mmc);
>>>> @@ -1913,7 +1973,7 @@ static int mmci_of_parse(struct device_node *np, struct mmc_host *mmc)
>>>>           if (of_get_property(np, "st,neg-edge", NULL))
>>>>                   host->clk_reg_add |= MCI_STM32_CLK_NEGEDGE;
>>>>           if (of_get_property(np, "st,use-ckin", NULL))
>>>> -               host->clk_reg_add |= MCI_STM32_CLK_SELCKIN;
>>>> +               mmci_probe_level_translator(mmc);
>>>
>>> I think you can make this change bit less invasive. Rather than having
>>> to shuffle code around in ->probe(), I suggest you call
>>> mmci_probe_level_translator() outside and after mmci_of_parse() has
>>> been called.
>>>
>>> In this way, you can also provide mmci_probe_level_translator() with a
>>> struct mmci_host *, rather than having to pick it up from
>>> mmc_priv(mmc), if you see what I mean.
>>>
>>> Of course, this also means in mmci_probe_level_translator() you will
>>> have to check if MCI_STM32_CLK_SELCKIN has been set, and if not then
>>> do an early return.
>>
>> Testing the translator presence when checking whether its enabled in DT
>> seems like the right place, but that's really just an implementation detail.
>>
>> I am more interested in knowing whether adding
>> mmci_probe_level_translator() is even acceptable in the first place. Is it ?
> 
> Honestly, I don't know.
> 
> I think I need to defer that question to Linus Walleij. And of course,
> it would be nice to get the opinion from Ludovic as well.

Good, that's what I was hoping for too.