On 1/13/21 3:21 PM, Yann GAUTIER wrote:
On 1/13/21 12:52 PM, Marek Vasut wrote:
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.
Hi,
Ludovic is out of office this week.
The feature of detecting a level translator seems to be quite generic,
and not dedicated to MMCI driver or the ST dedicated code, and with new
st,* properties. It may be in generic mmc code. But I'll let Linus
comment about that.
I also wonder if this HW detection should be done in kernel, or if it
should be done in Bootloader. But it may be more complex, to add the
st,use_ckin in kernel DT if bootloader detects this translator.
Lets not attempt to hide inobvious functionality in bootloaders, the
kernel should be independent of bootloader where possible. And here it
is clearly and easily possible.
