From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andy Shevchenko Date: Sat, 01 Apr 2017 16:11:13 +0300 Subject: [U-Boot] [PATCH v1] mmc: sdhci: SDHCI controllers also need power In-Reply-To: References: <20170315182521.4359-1-andriy.shevchenko@linux.intel.com> <1490014266.19767.101.camel@linux.intel.com> Message-ID: <1491052273.708.95.camel@linux.intel.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de On Fri, 2017-03-31 at 22:24 -0600, Simon Glass wrote: > Hi Andy, > > On 20 March 2017 at 06:51, Andy Shevchenko > wrote: > > On Sun, 2017-03-19 at 20:30 -0600, Simon Glass wrote: > > > Hi Andy, > > > > > > On 15 March 2017 at 12:25, Andy Shevchenko > > > 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 > > > 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 Intel Finland Oy