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

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

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@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.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.

> > >  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.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy