[U-Boot] [PATCH v1] mmc: sdhci: SDHCI controllers also need power

From: Simon Glass <sjg@chromium.org>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v1] mmc: sdhci: SDHCI controllers also need power
Date: Fri, 31 Mar 2017 22:24:06 -0600	[thread overview]
Message-ID: <CAPnjgZ1KWM9vY16P-t_ui+5zUQXg7Zkdo3Nuj9_yz2rh_umMPw@mail.gmail.com> (raw)
In-Reply-To: <1490014266.19767.101.camel@linux.intel.com>

Hi Andy,

On 20 March 2017 at 06:51, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Sun, 2017-03-19 at 20:30 -0600, Simon Glass wrote:
>> Hi Andy,
>>
>> On 15 March 2017 at 12:25, Andy Shevchenko
>> <andriy.shevchenko@linux.intel.com> wrote:
>> > On some systems SDHCI controllers may be powered off and it's
>> > required
>> > to bring them on before accessing.
>> >
>> > SDHCI generic driver currently lacks any mean of doing it. Call the
>> > same
>> > board_power_mmc_init() at sdhci_init() stage.
>> >
>> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> > ---
>> >  drivers/mmc/sdhci.c | 2 ++
>> >  1 file changed, 2 insertions(+)
>> >
>> > diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
>> > index 93cefd89cd..54a7e379ff 100644
>> > --- a/drivers/mmc/sdhci.c
>> > +++ b/drivers/mmc/sdhci.c
>> > @@ -469,6 +469,8 @@ static int sdhci_init(struct mmc *mmc)
>> >  {
>> >         struct sdhci_host *host = mmc->priv;
>> >
>> > +       board_mmc_power_init();
>>
>> You should be using driver model for this (CONFIG_DM_MMC*).
>
> I didn't get this part. It's used by the driver (tangier_sdhci) as far
> as I understand.
>
>>  So either
>> get the power supply from the device tree and enable it,
>
> Any example?

Well in mmc_power_init() there is:

ret = device_get_supply_regulator(mmc->dev, "vmmc-supply",
 &vmmc_supply);

or say you have a node like this (from firefly-rk3288):

vcc_sd: sdmmc-regulator {
compatible = "regulator-fixed";
gpio = <&gpio7 11 GPIO_ACTIVE_LOW>;
pinctrl-names = "default";
pinctrl-0 = <&sdmmc_pwr>;
regulator-name = "vcc_sd";
regulator-min-microvolt = <3300000>;
regulator-max-microvolt = <3300000>;
startup-delay-us = <100000>;
vin-supply = <&vcc_io>;
};

Then you can use regulator_get_by_platname("vcc_sd"...

>
>>  or do this in
>> the board code.
>
> How? It's already board code that powers on the controller. If you look
> at mmc_init() it does this. SDHCI on the other hand doesn't which is for
> my opinion is a bug. Otherwise why is the difference between
> initialization sequence of MMC and SHDCI controllers?

There should not really be a different I think, except that with
driver model we want to use drivers for power rather than hard-coding
things in custom code.

>
>>
>> > +
>> >         sdhci_reset(host, SDHCI_RESET_ALL);
>> >
>> >         if ((host->quirks & SDHCI_QUIRK_32BIT_DMA_ADDR) &&
>> > !aligned_buffer) {
>> > --
>> > 2.11.0

Regards,
Simon