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

[Simon Glass]

Hi Andy,

On 1 April 2017 at 07:11, Andy Shevchenko
<andriy.shevchenko at linux.intel.com> wrote:
> On Fri, 2017-03-31 at 22:24 -0600, Simon Glass wrote:
>> Hi Andy,
>>
>> On 20 March 2017 at 06:51, Andy Shevchenko
>> <andriy.shevchenko at 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 at 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 at linux.intel.co
>> > > > m>
>> > > > ---
>> > > >  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"...
>
> Oh, we are talking about host controller's power management which is
> done using PMU (power management unit) inside SoC. It's *not* a power
> regulator.
>
> Above is clearly about card power management, which we also have (in
> case of Wi-Fi), but it's not applicable for eMMC soldered on the module.

Still if the eMMC is soldered on, it needs power, right? What is the
distinction?

In any case we cannot call board code from the driver with DM - it's
just not how things work. So can you init it in your board_init() code
perhaps, if you can't use a power driver?

>
>> > >  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.
>
> I totally agree with this, though since we have no clear PCI
> implementation on that board (*) we can't have good described PCI power
> management for it.
>
> (*) It's called "fake PCI" meaning it mimics PCI programming interface
> while being not 100% compatible with PCI specification on hardware and
> firmware levels.
>
> So, for now I have been seeing no alternatives than my initial approach,
> though I'm all ears for better solution.

Well you can create a regulator driver which has a single regulator to
handle whatever needs doing to enable MMC power.

Regards,
Simon