On 02/08/2019 14:48, Adrian Hunter wrote: > 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. Understood. I don't want to reopen a religious war (much). Phil
