On 2/08/19 4:28 PM, Ulf Hansson wrote: > + Adrian > > On Fri, 2 Aug 2019 at 15:23, Phil Elwell <phil@xxxxxxxxxxxxxxx> wrote: >> >> Hi Uffe, >> >> On 02/08/2019 14:15, Ulf Hansson wrote: >>> On Tue, 30 Jul 2019 at 15:17, Phil Elwell <phil@xxxxxxxxxxxxxxx> wrote: >>>> >>>> Hi MMC team, >>>> >>>> I've spent some time trying to add a regulator to control power to an SD card >>>> (vmmc-supply) on an SDHCI-equipped system. After a few false starts and red herrings >>>> I found that powering up the regulator during the boot process was effectively disabling >>>> the SDHCI controller. Note that this was despite having regulator-boot-on set in the >>>> device tree. >>>> >>>> The problem seems to be in sdhci_set_power_reg: >>>> >>>> mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd); >>>> >>>> if (mode != MMC_POWER_OFF) >>>> sdhci_writeb(host, SDHCI_POWER_ON, SDHCI_POWER_CONTROL); >>>> else >>>> sdhci_writeb(host, 0, SDHCI_POWER_CONTROL); >>>> >>>> This looks plausible for the MMC_POWER_OFF case, but setting the SDHCI_POWER_CONTROL >>>> register to SDHCI_POWER_ON (0x01) has the side effect of settings the SD Bus Voltage >>>> Select bits to 0b000 (a reserved value). >>>> >>>> sdhci_set_power_noreg() includes logic to calculate the correct values for the voltage >>>> select bits, so I found that (in my limited test cases) replacing the if/else above >>>> with a chain call to sdhci_set_power_noreg() was sufficient to get everything working. >>>> >>>> Can anyone tell me what I've been doing wrong, or suggest a better "fix"? >>> >>> There are other sdhci variants having the similar needs, hence we have >>> the flexibility available via using the optional ->set_power() >>> callback. >>> >>> Have a look at sdhci_arasan_set_power(), for example. >> >> I don't understand. Can you explain what writing 0x0001 to the SDHCI_POWER_CONTROL register >> is supposed to do according to the specification? And why is the Arasan SDHCI implementation >> not the default? > > Good question, but unfortunate I don't know the SDHCI spec in that > great detail. And in the end, it has turned out that variants seems to > differs in regards to this register. > > I looped in Adrian, to see if he has some comments. Different hardware works differently. People have already fought, won and lost over how the power should be set with regulators, so now we have ->set_power() so everyone can do what they want. Please use that.
