| Message ID | 1449840989-563-1-git-send-email-jszhang@marvell.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
+ Ludovic (We had some discussions around this code recently as well) On 11 December 2015 at 14:36, Jisheng Zhang <jszhang@marvell.com> wrote: > After commit 52221610dd84 ("mmc: sdhci: Improve external VDD regulator > support"), for the VDD is supplied via external regulators, we ignore > the code to convert a VDD voltage request into one of the standard > SDHCI voltage levels, then program it in the SDHCI_POWER_CONTROL. This > brings two issues: > > 1. SDHCI_QUIRK2_CARD_ON_NEEDS_BUS_ON quirk isn't handled properly any > more. > > 2. What's more, once SDHCI_POWER_ON bit is set, some controllers such > as the sdhci-pxav3 used in marvell berlin SoCs require the voltage > levels programming in the SDHCI_POWER_CONTROL register, even the VDD > is supplied by external regulator. So the host in marvell berlin SoCs > still works fine after the commit. However, commit 3cbc6123a93d ("mmc: > sdhci: Set SDHCI_POWER_ON with external vmmc") sets the SDHCI_POWER_ON > bit, this would make the host in marvell berlin SoCs won't work any > more with external vmmc. > > This patch restores the behavior when setting VDD through external > regulator by moving the call of mmc_regulator_set_ocr() to the end > of sdhci_set_power() function. > > After this patch, the sdcard on Marvell Berlin SoC boards work again. > > Signed-off-by: Jisheng Zhang <jszhang@marvell.com> > Fixes: 52221610dd84 ("mmc: sdhci: Improve external VDD ...") > --- > Since v1: > - add more details about why the sdhci-pxav3 used in marvell berlin > SoCs need this patch. > > drivers/mmc/host/sdhci.c | 19 ++++++------------- > 1 file changed, 6 insertions(+), 13 deletions(-) > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index b48565e..616aa90 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -1274,19 +1274,6 @@ static void sdhci_set_power(struct sdhci_host *host, unsigned char mode, > struct mmc_host *mmc = host->mmc; > u8 pwr = 0; > > - if (!IS_ERR(mmc->supply.vmmc)) { > - spin_unlock_irq(&host->lock); > - mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd); > - spin_lock_irq(&host->lock); > - > - if (mode != MMC_POWER_OFF) > - sdhci_writeb(host, SDHCI_POWER_ON, SDHCI_POWER_CONTROL); > - else > - sdhci_writeb(host, 0, SDHCI_POWER_CONTROL); > - > - return; > - } > - > if (mode != MMC_POWER_OFF) { > switch (1 << vdd) { > case MMC_VDD_165_195: > @@ -1345,6 +1332,12 @@ static void sdhci_set_power(struct sdhci_host *host, unsigned char mode, > if (host->quirks & SDHCI_QUIRK_DELAY_AFTER_POWER) > mdelay(10); > } > + > + if (!IS_ERR(mmc->supply.vmmc)) { > + spin_unlock_irq(&host->lock); > + mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd); > + spin_lock_irq(&host->lock); > + } > } > > /*****************************************************************************\ > -- > 2.6.3 > My concern with this patch is that it might fix the problem for your SDHCI variant, but will break it for others. I guess we can give it try, unless or until someone reports a problem. Although, I would like to get Ludovic's input on this change, before I decide to do anything. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dear Ulf, Ludovic, Sorry for top post. I'm under webmail for I have no proper email client now. OOPS, I should ask mmc maintainers and experts for help early. I spent two days to debug the issue then found the two problematic commits. PS: It seems other sdhci-pxav3 users never complained, it's a bit strange ;) Thanks for reviewing this patch, Jisheng
On Fri, Dec 11, 2015 at 03:48:04PM +0100, Ulf Hansson wrote: > + Ludovic (We had some discussions around this code recently as well) > Thanks Ulf. > On 11 December 2015 at 14:36, Jisheng Zhang <jszhang@marvell.com> wrote: > > After commit 52221610dd84 ("mmc: sdhci: Improve external VDD regulator > > support"), for the VDD is supplied via external regulators, we ignore > > the code to convert a VDD voltage request into one of the standard > > SDHCI voltage levels, then program it in the SDHCI_POWER_CONTROL. This > > brings two issues: > > > > 1. SDHCI_QUIRK2_CARD_ON_NEEDS_BUS_ON quirk isn't handled properly any > > more. > > > > 2. What's more, once SDHCI_POWER_ON bit is set, some controllers such > > as the sdhci-pxav3 used in marvell berlin SoCs require the voltage > > levels programming in the SDHCI_POWER_CONTROL register, even the VDD > > is supplied by external regulator.So the host in marvell berlin SoCs > > still works fine after the commit. I am not sure to understand this part. You explain that the controller in berlin SoC requireis the voltage level programming even if there is an external regulator for VDD. I agree this part, I am in the same situation with atmel controller. It is not smart to rely on the voltage level if we have an external regulator but it follows the sdhci specs. That I don't understand is that you say it still works fine after this commit... If you need to set the voltage level in the SDHCI_POWER_CONTROL register, it is broken by this commit if you declare an external regulator. > > However, commit 3cbc6123a93d ("mmc: > > sdhci: Set SDHCI_POWER_ON with external vmmc") sets the SDHCI_POWER_ON > > bit, this would make the host in marvell berlin SoCs won't work any > > more with external vmmc. > > > > This patch restores the behavior when setting VDD through external > > regulator by moving the call of mmc_regulator_set_ocr() to the end > > of sdhci_set_power() function. > > > > After this patch, the sdcard on Marvell Berlin SoC boards work again. > > > > Signed-off-by: Jisheng Zhang <jszhang@marvell.com> > > Fixes: 52221610dd84 ("mmc: sdhci: Improve external VDD ...") > > --- > > Since v1: > > - add more details about why the sdhci-pxav3 used in marvell berlin > > SoCs need this patch. > > > > drivers/mmc/host/sdhci.c | 19 ++++++------------- > > 1 file changed, 6 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > > index b48565e..616aa90 100644 > > --- a/drivers/mmc/host/sdhci.c > > +++ b/drivers/mmc/host/sdhci.c > > @@ -1274,19 +1274,6 @@ static void sdhci_set_power(struct sdhci_host *host, unsigned char mode, > > struct mmc_host *mmc = host->mmc; > > u8 pwr = 0; > > > > - if (!IS_ERR(mmc->supply.vmmc)) { > > - spin_unlock_irq(&host->lock); > > - mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd); > > - spin_lock_irq(&host->lock); > > - > > - if (mode != MMC_POWER_OFF) > > - sdhci_writeb(host, SDHCI_POWER_ON, SDHCI_POWER_CONTROL); > > - else > > - sdhci_writeb(host, 0, SDHCI_POWER_CONTROL); > > - > > - return; > > - } > > - > > if (mode != MMC_POWER_OFF) { > > switch (1 << vdd) { > > case MMC_VDD_165_195: > > @@ -1345,6 +1332,12 @@ static void sdhci_set_power(struct sdhci_host *host, unsigned char mode, > > if (host->quirks & SDHCI_QUIRK_DELAY_AFTER_POWER) > > mdelay(10); > > } > > + > > + if (!IS_ERR(mmc->supply.vmmc)) { > > + spin_unlock_irq(&host->lock); > > + mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd); > > + spin_lock_irq(&host->lock); > > + } > > } > > > > /*****************************************************************************\ > > -- > > 2.6.3 > > > > My concern with this patch is that it might fix the problem for your > SDHCI variant, but will break it for others. > I guess we can give it try, unless or until someone reports a problem. > > Although, I would like to get Ludovic's input on this change, before I > decide to do anything. > I would be pleased to get this patch since it would solve one of my issues. Concerning the risk to take this patch. I would say one part of this patch is safe, the other one maybe not. Reading the log of commit 52221610dd84, it is not a bug fix. It was done in this way because it seemed logical to not set the voltage level in the SDHCI_POWER_CONTROL if we have an external regulator. Moving mmc_regulator_set_ocr at the end could cause issue since it changes the sequence order: the regulator is configured after the SDHCI_POWER_CONTROL register. Regards Ludovic -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dear Ludovic, On Fri, 11 Dec 2015 18:06:17 +0100 Ludovic Desroches wrote: > On Fri, Dec 11, 2015 at 03:48:04PM +0100, Ulf Hansson wrote: > > + Ludovic (We had some discussions around this code recently as well) > > > > Thanks Ulf. > > > On 11 December 2015 at 14:36, Jisheng Zhang <jszhang@marvell.com> wrote: > > > After commit 52221610dd84 ("mmc: sdhci: Improve external VDD regulator > > > support"), for the VDD is supplied via external regulators, we ignore > > > the code to convert a VDD voltage request into one of the standard > > > SDHCI voltage levels, then program it in the SDHCI_POWER_CONTROL. This > > > brings two issues: > > > > > > 1. SDHCI_QUIRK2_CARD_ON_NEEDS_BUS_ON quirk isn't handled properly any > > > more. > > > > > > 2. What's more, once SDHCI_POWER_ON bit is set, some controllers such > > > as the sdhci-pxav3 used in marvell berlin SoCs require the voltage > > > levels programming in the SDHCI_POWER_CONTROL register, even the VDD > > > is supplied by external regulator.So the host in marvell berlin SoCs > > > still works fine after the commit. > > I am not sure to understand this part. You explain that the controller > in berlin SoC requireis the voltage level programming even if there is an > external regulator for VDD. I agree this part, I am in the same plus one more condition ;) -- "once SDHCI_POWER_ON bit is set", that's to say either not touching SDHCI_POWER_CONTROL register at all or setting SDHCI_POWER_ON bit and voltage level at the same time is fine, but the sdhci-pxav3 in berlin case can't work if we set SDHCI_POWER_ON but don't program the voltage level , unfortunately this is true after commit 3cbc6123a93d ("mmc: sdhci: Set SDHCI_POWER_ON with external vmmc") > situation with atmel controller. It is not smart to rely on the voltage > level if we have an external regulator but it follows the sdhci specs. > > That I don't understand is that you say it still works fine after this > commit... If you need to set the voltage level in the > SDHCI_POWER_CONTROL register, it is broken by this commit if you declare > an external regulator. See above, commit 52221610dd84 doesn't break the host controller, it still works fine after commit 52221610dd84 but the combination of 52221610dd84 and 3cbc6123a93d do break the host controller. > > > > However, commit 3cbc6123a93d ("mmc: > > > sdhci: Set SDHCI_POWER_ON with external vmmc") sets the SDHCI_POWER_ON > > > bit, this would make the host in marvell berlin SoCs won't work any > > > more with external vmmc. > > > > > > This patch restores the behavior when setting VDD through external > > > regulator by moving the call of mmc_regulator_set_ocr() to the end > > > of sdhci_set_power() function. > > > > > > After this patch, the sdcard on Marvell Berlin SoC boards work again. > > > > > > Signed-off-by: Jisheng Zhang <jszhang@marvell.com> > > > Fixes: 52221610dd84 ("mmc: sdhci: Improve external VDD ...") > > > --- > > > Since v1: > > > - add more details about why the sdhci-pxav3 used in marvell berlin > > > SoCs need this patch. > > > > > > drivers/mmc/host/sdhci.c | 19 ++++++------------- > > > 1 file changed, 6 insertions(+), 13 deletions(-) > > > > > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > > > index b48565e..616aa90 100644 > > > --- a/drivers/mmc/host/sdhci.c > > > +++ b/drivers/mmc/host/sdhci.c > > > @@ -1274,19 +1274,6 @@ static void sdhci_set_power(struct sdhci_host *host, unsigned char mode, > > > struct mmc_host *mmc = host->mmc; > > > u8 pwr = 0; > > > > > > - if (!IS_ERR(mmc->supply.vmmc)) { > > > - spin_unlock_irq(&host->lock); > > > - mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd); > > > - spin_lock_irq(&host->lock); > > > - > > > - if (mode != MMC_POWER_OFF) > > > - sdhci_writeb(host, SDHCI_POWER_ON, SDHCI_POWER_CONTROL); > > > - else > > > - sdhci_writeb(host, 0, SDHCI_POWER_CONTROL); > > > - > > > - return; > > > - } > > > - > > > if (mode != MMC_POWER_OFF) { > > > switch (1 << vdd) { > > > case MMC_VDD_165_195: > > > @@ -1345,6 +1332,12 @@ static void sdhci_set_power(struct sdhci_host *host, unsigned char mode, > > > if (host->quirks & SDHCI_QUIRK_DELAY_AFTER_POWER) > > > mdelay(10); > > > } > > > + > > > + if (!IS_ERR(mmc->supply.vmmc)) { > > > + spin_unlock_irq(&host->lock); > > > + mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd); > > > + spin_lock_irq(&host->lock); > > > + } > > > } > > > > > > /*****************************************************************************\ > > > -- > > > 2.6.3 > > > > > > > My concern with this patch is that it might fix the problem for your > > SDHCI variant, but will break it for others. > > I guess we can give it try, unless or until someone reports a problem. > > > > Although, I would like to get Ludovic's input on this change, before I > > decide to do anything. > > > > I would be pleased to get this patch since it would solve one of my > issues. > > Concerning the risk to take this patch. I would say one part of this > patch is safe, the other one maybe not. > > Reading the log of commit 52221610dd84, it is not a bug fix. It was done > in this way because it seemed logical to not set the voltage level in > the SDHCI_POWER_CONTROL if we have an external regulator. > > Moving mmc_regulator_set_ocr at the end could cause issue since it > changes the sequence order: the regulator is configured after the > SDHCI_POWER_CONTROL register. hmmm, this sequence order is the same as the one before commit 52221610dd84. IOW, the patch restores the old sequence order: the regulator is configured after the SDHCI_POWER_CONTROL register. Thanks for reviewing, Jisheng -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ulf, Jisheng, On Fri, Dec 11, 2015 at 03:48:04PM +0100, Ulf Hansson wrote: > + Ludovic (We had some discussions around this code recently as well) > > On 11 December 2015 at 14:36, Jisheng Zhang <jszhang@marvell.com> wrote: > > After commit 52221610dd84 ("mmc: sdhci: Improve external VDD regulator > > support"), for the VDD is supplied via external regulators, we ignore > > the code to convert a VDD voltage request into one of the standard > > SDHCI voltage levels, then program it in the SDHCI_POWER_CONTROL. This > > brings two issues: > > > > 1. SDHCI_QUIRK2_CARD_ON_NEEDS_BUS_ON quirk isn't handled properly any > > more. > > > > 2. What's more, once SDHCI_POWER_ON bit is set, some controllers such > > as the sdhci-pxav3 used in marvell berlin SoCs require the voltage > > levels programming in the SDHCI_POWER_CONTROL register, even the VDD > > is supplied by external regulator. So the host in marvell berlin SoCs > > still works fine after the commit. However, commit 3cbc6123a93d ("mmc: > > sdhci: Set SDHCI_POWER_ON with external vmmc") sets the SDHCI_POWER_ON > > bit, this would make the host in marvell berlin SoCs won't work any > > more with external vmmc. > > > > This patch restores the behavior when setting VDD through external > > regulator by moving the call of mmc_regulator_set_ocr() to the end > > of sdhci_set_power() function. > > > > After this patch, the sdcard on Marvell Berlin SoC boards work again. > > > > Signed-off-by: Jisheng Zhang <jszhang@marvell.com> > > Fixes: 52221610dd84 ("mmc: sdhci: Improve external VDD ...") Reviewed-by: Ludovic Desroches <ludovic.desroches@atmel.com> Tested-by: Ludovic Desroches <ludovic.desroches@atmel.com> Even if the patch sounds good for me, I wanted to test it. As planned, with this patch, I can describe my vcc regulator without breaking the behavior of my sdhci controller. Regards Ludovic > > --- > > Since v1: > > - add more details about why the sdhci-pxav3 used in marvell berlin > > SoCs need this patch. > > > > drivers/mmc/host/sdhci.c | 19 ++++++------------- > > 1 file changed, 6 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > > index b48565e..616aa90 100644 > > --- a/drivers/mmc/host/sdhci.c > > +++ b/drivers/mmc/host/sdhci.c > > @@ -1274,19 +1274,6 @@ static void sdhci_set_power(struct sdhci_host *host, unsigned char mode, > > struct mmc_host *mmc = host->mmc; > > u8 pwr = 0; > > > > - if (!IS_ERR(mmc->supply.vmmc)) { > > - spin_unlock_irq(&host->lock); > > - mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd); > > - spin_lock_irq(&host->lock); > > - > > - if (mode != MMC_POWER_OFF) > > - sdhci_writeb(host, SDHCI_POWER_ON, SDHCI_POWER_CONTROL); > > - else > > - sdhci_writeb(host, 0, SDHCI_POWER_CONTROL); > > - > > - return; > > - } > > - > > if (mode != MMC_POWER_OFF) { > > switch (1 << vdd) { > > case MMC_VDD_165_195: > > @@ -1345,6 +1332,12 @@ static void sdhci_set_power(struct sdhci_host *host, unsigned char mode, > > if (host->quirks & SDHCI_QUIRK_DELAY_AFTER_POWER) > > mdelay(10); > > } > > + > > + if (!IS_ERR(mmc->supply.vmmc)) { > > + spin_unlock_irq(&host->lock); > > + mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd); > > + spin_lock_irq(&host->lock); > > + } > > } > > > > /*****************************************************************************\ > > -- > > 2.6.3 > > > > My concern with this patch is that it might fix the problem for your > SDHCI variant, but will break it for others. > I guess we can give it try, unless or until someone reports a problem. > > Although, I would like to get Ludovic's input on this change, before I > decide to do anything. > > Kind regards > Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 18 December 2015 at 09:11, Ludovic Desroches <ludovic.desroches@atmel.com> wrote: > Hi Ulf, Jisheng, > > On Fri, Dec 11, 2015 at 03:48:04PM +0100, Ulf Hansson wrote: >> + Ludovic (We had some discussions around this code recently as well) >> >> On 11 December 2015 at 14:36, Jisheng Zhang <jszhang@marvell.com> wrote: >> > After commit 52221610dd84 ("mmc: sdhci: Improve external VDD regulator >> > support"), for the VDD is supplied via external regulators, we ignore >> > the code to convert a VDD voltage request into one of the standard >> > SDHCI voltage levels, then program it in the SDHCI_POWER_CONTROL. This >> > brings two issues: >> > >> > 1. SDHCI_QUIRK2_CARD_ON_NEEDS_BUS_ON quirk isn't handled properly any >> > more. >> > >> > 2. What's more, once SDHCI_POWER_ON bit is set, some controllers such >> > as the sdhci-pxav3 used in marvell berlin SoCs require the voltage >> > levels programming in the SDHCI_POWER_CONTROL register, even the VDD >> > is supplied by external regulator. So the host in marvell berlin SoCs >> > still works fine after the commit. However, commit 3cbc6123a93d ("mmc: >> > sdhci: Set SDHCI_POWER_ON with external vmmc") sets the SDHCI_POWER_ON >> > bit, this would make the host in marvell berlin SoCs won't work any >> > more with external vmmc. >> > >> > This patch restores the behavior when setting VDD through external >> > regulator by moving the call of mmc_regulator_set_ocr() to the end >> > of sdhci_set_power() function. >> > >> > After this patch, the sdcard on Marvell Berlin SoC boards work again. >> > >> > Signed-off-by: Jisheng Zhang <jszhang@marvell.com> >> > Fixes: 52221610dd84 ("mmc: sdhci: Improve external VDD ...") > > Reviewed-by: Ludovic Desroches <ludovic.desroches@atmel.com> > Tested-by: Ludovic Desroches <ludovic.desroches@atmel.com> > > Even if the patch sounds good for me, I wanted to test it. As planned, > with this patch, I can describe my vcc regulator without breaking the > behavior of my sdhci controller. > Okay! According to the reasoning and testing, this change seems like a good idea! I have now queued the v2 for next. Thanks! Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 18/12/15 11:55, Ulf Hansson wrote: > On 18 December 2015 at 09:11, Ludovic Desroches > <ludovic.desroches@atmel.com> wrote: >> Hi Ulf, Jisheng, >> >> On Fri, Dec 11, 2015 at 03:48:04PM +0100, Ulf Hansson wrote: >>> + Ludovic (We had some discussions around this code recently as well) >>> >>> On 11 December 2015 at 14:36, Jisheng Zhang <jszhang@marvell.com> wrote: >>>> After commit 52221610dd84 ("mmc: sdhci: Improve external VDD regulator >>>> support"), for the VDD is supplied via external regulators, we ignore >>>> the code to convert a VDD voltage request into one of the standard >>>> SDHCI voltage levels, then program it in the SDHCI_POWER_CONTROL. This >>>> brings two issues: >>>> >>>> 1. SDHCI_QUIRK2_CARD_ON_NEEDS_BUS_ON quirk isn't handled properly any >>>> more. For the record, the way it was working made more sense to me i.e. if you have control of an external regulator then you know the power is not disrupted by runtime suspend. Also AFAIK Intel is the only user of SDHCI_QUIRK2_CARD_ON_NEEDS_BUS_ON, so I am surprised you listed it as an issue that you have. -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index b48565e..616aa90 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -1274,19 +1274,6 @@ static void sdhci_set_power(struct sdhci_host *host, unsigned char mode, struct mmc_host *mmc = host->mmc; u8 pwr = 0; - if (!IS_ERR(mmc->supply.vmmc)) { - spin_unlock_irq(&host->lock); - mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd); - spin_lock_irq(&host->lock); - - if (mode != MMC_POWER_OFF) - sdhci_writeb(host, SDHCI_POWER_ON, SDHCI_POWER_CONTROL); - else - sdhci_writeb(host, 0, SDHCI_POWER_CONTROL); - - return; - } - if (mode != MMC_POWER_OFF) { switch (1 << vdd) { case MMC_VDD_165_195: @@ -1345,6 +1332,12 @@ static void sdhci_set_power(struct sdhci_host *host, unsigned char mode, if (host->quirks & SDHCI_QUIRK_DELAY_AFTER_POWER) mdelay(10); } + + if (!IS_ERR(mmc->supply.vmmc)) { + spin_unlock_irq(&host->lock); + mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd); + spin_lock_irq(&host->lock); + } } /*****************************************************************************\
After commit 52221610dd84 ("mmc: sdhci: Improve external VDD regulator support"), for the VDD is supplied via external regulators, we ignore the code to convert a VDD voltage request into one of the standard SDHCI voltage levels, then program it in the SDHCI_POWER_CONTROL. This brings two issues: 1. SDHCI_QUIRK2_CARD_ON_NEEDS_BUS_ON quirk isn't handled properly any more. 2. What's more, once SDHCI_POWER_ON bit is set, some controllers such as the sdhci-pxav3 used in marvell berlin SoCs require the voltage levels programming in the SDHCI_POWER_CONTROL register, even the VDD is supplied by external regulator. So the host in marvell berlin SoCs still works fine after the commit. However, commit 3cbc6123a93d ("mmc: sdhci: Set SDHCI_POWER_ON with external vmmc") sets the SDHCI_POWER_ON bit, this would make the host in marvell berlin SoCs won't work any more with external vmmc. This patch restores the behavior when setting VDD through external regulator by moving the call of mmc_regulator_set_ocr() to the end of sdhci_set_power() function. After this patch, the sdcard on Marvell Berlin SoC boards work again. Signed-off-by: Jisheng Zhang <jszhang@marvell.com> Fixes: 52221610dd84 ("mmc: sdhci: Improve external VDD ...") --- Since v1: - add more details about why the sdhci-pxav3 used in marvell berlin SoCs need this patch. drivers/mmc/host/sdhci.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-)