| Message ID | 87a9xfyko6.fsf@octavius.laptop.org (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Headers | show |
Hi Chris, On 28 August 2012 01:33, Chris Ball <cjb@laptop.org> wrote: > Some distributions build with CONFIG_REGULATOR_DUMMY=y -- this makes > regulator setup succeed harmlessly, but then trying to set the voltage > on the dummy regulator will fail as below: > > [ 6.413866] sdhci-pci 0000:03:00.0: SDHCI controller found [1180:e822] > (rev 0) > [ 6.414081] _regulator_get: 0000:03:00.0 supply vmmc not found, using > dummy regulator > [ 6.415252] mmc0: SDHCI controller on PCI [0000:03:00.0] using DMA > [ 6.415294] sdhci-pci 0000:03:00.0: could not set regulator OCR (-22) > [ 6.444241] sdhci-pci 0000:03:00.0: could not set regulator OCR (-22) > [ 6.473648] sdhci-pci 0000:03:00.0: could not set regulator OCR (-22) > [..] > > Since this isn't an error in this case, move the error to pr_debug(). > Reference: https://bugs.archlinux.org/task/29008 > > Signed-off-by: Chris Ball <cjb@laptop.org> > --- > drivers/mmc/core/core.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c > index 835c9f0..30d9357 100644 > --- a/drivers/mmc/core/core.c > +++ b/drivers/mmc/core/core.c > @@ -1017,8 +1017,9 @@ int mmc_regulator_set_ocr(struct mmc_host *mmc, > } > > if (result) > - dev_err(mmc_dev(mmc), > - "could not set regulator OCR (%d)\n", result); > + pr_debug("%s: could not set regulator OCR (%d)\n", > + mmc_hostname(mmc), result); > + I am not sure this is the correct solution. Would it be possible prevent sdhci-* from calling mmc_regulator_set_ocr when it has gotten a "dummy" regulator some how? In the other scenarios were we use a real regulator, I would interpret this error as critical, since the power to the card could not be set according to the vdd bit. Of course an option could be to leave the error printing to the caller of mmc_regulator_set_ocr.. :-) > return result; > } > EXPORT_SYMBOL_GPL(mmc_regulator_set_ocr); > -- > Chris Ball <cjb@laptop.org> <http://printf.net/> > One Laptop Per Child > -- > 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 Kind regards Ulf Hansson -- 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, adding Mark and Liam, On Wed, Aug 29 2012, Ulf Hansson wrote: > On 28 August 2012 01:33, Chris Ball <cjb@laptop.org> wrote: >> Some distributions build with CONFIG_REGULATOR_DUMMY=y -- this makes >> regulator setup succeed harmlessly, but then trying to set the voltage >> on the dummy regulator will fail as below: >> >> [ 6.413866] sdhci-pci 0000:03:00.0: SDHCI controller found [1180:e822] >> (rev 0) >> [ 6.414081] _regulator_get: 0000:03:00.0 supply vmmc not found, using >> dummy regulator >> [ 6.415252] mmc0: SDHCI controller on PCI [0000:03:00.0] using DMA >> [ 6.415294] sdhci-pci 0000:03:00.0: could not set regulator OCR (-22) >> [ 6.444241] sdhci-pci 0000:03:00.0: could not set regulator OCR (-22) >> [ 6.473648] sdhci-pci 0000:03:00.0: could not set regulator OCR (-22) >> [..] >> >> Since this isn't an error in this case, move the error to pr_debug(). >> Reference: https://bugs.archlinux.org/task/29008 >> >> Signed-off-by: Chris Ball <cjb@laptop.org> >> --- >> drivers/mmc/core/core.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c >> index 835c9f0..30d9357 100644 >> --- a/drivers/mmc/core/core.c >> +++ b/drivers/mmc/core/core.c >> @@ -1017,8 +1017,9 @@ int mmc_regulator_set_ocr(struct mmc_host *mmc, >> } >> >> if (result) >> - dev_err(mmc_dev(mmc), >> - "could not set regulator OCR (%d)\n", result); >> + pr_debug("%s: could not set regulator OCR (%d)\n", >> + mmc_hostname(mmc), result); >> + > > I am not sure this is the correct solution. Would it be possible > prevent sdhci-* from calling mmc_regulator_set_ocr when it has gotten > a "dummy" regulator some how? Looks like regulator_desc->name should be "dummy", but I think it would be an API violation for us to start caring about that. Mark, would you be interested in offering an "regulator_is_dummy(supply)" function so that we can limit our error messages to non-dummy supplies? > In the other scenarios were we use a real regulator, I would interpret > this error as critical, since the power to the card could not be set > according to the vdd bit. > Of course an option could be to leave the error printing to the caller > of mmc_regulator_set_ocr.. :-) Makes sense. If we can't work out a compromise, we should just leave it as-is -- the error messages appear to be worrying some users, but they're not breaking any functionality. Thanks for the review, - Chris.
On Wed, Aug 29, 2012 at 09:08:47AM -0400, Chris Ball wrote: > On Wed, Aug 29 2012, Ulf Hansson wrote: > > On 28 August 2012 01:33, Chris Ball <cjb@laptop.org> wrote: > >> Some distributions build with CONFIG_REGULATOR_DUMMY=y -- this makes > >> regulator setup succeed harmlessly, but then trying to set the voltage > >> on the dummy regulator will fail as below: > >> [ 6.413866] sdhci-pci 0000:03:00.0: SDHCI controller found [1180:e822] > >> (rev 0) > >> [ 6.414081] _regulator_get: 0000:03:00.0 supply vmmc not found, using > >> dummy regulator > >> [ 6.415252] mmc0: SDHCI controller on PCI [0000:03:00.0] using DMA > >> [ 6.415294] sdhci-pci 0000:03:00.0: could not set regulator OCR (-22) > >> [ 6.444241] sdhci-pci 0000:03:00.0: could not set regulator OCR (-22) > >> [ 6.473648] sdhci-pci 0000:03:00.0: could not set regulator OCR (-22) > >> [..] > >> Since this isn't an error in this case, move the error to pr_debug(). > >> Reference: https://bugs.archlinux.org/task/29008 > >> Signed-off-by: Chris Ball <cjb@laptop.org> > > I am not sure this is the correct solution. Would it be possible > > prevent sdhci-* from calling mmc_regulator_set_ocr when it has gotten > > a "dummy" regulator some how? > Looks like regulator_desc->name should be "dummy", but I think it would > be an API violation for us to start caring about that. Mark, would you > be interested in offering an "regulator_is_dummy(supply)" function so > that we can limit our error messages to non-dummy supplies? No, that's a terrible idea as previously discussed. The *sole* purpose of the dummy regulator driver is to provide a crutch to keep systems with broken configuraitons running, it's not indended to be used on production systems and is there solely as an aid for development. This is why it can only be enabled via Kconfig. If there is no software controllable regulator there the board should be providing fixed regulator. Conversely there is nothing stopping other regulator drivers having similar properties to the dummy driver, or constraints stopping them implementing things. If the MMC framework wants to silently accept regulators that don't allow the voltage to be set then the MMC framework should enumerate the range of voltages which can be set using the regulator it has and then take appropriate action. -- 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, On Thu, Aug 30 2012, Mark Brown wrote: >> Looks like regulator_desc->name should be "dummy", but I think it would >> be an API violation for us to start caring about that. Mark, would you >> be interested in offering an "regulator_is_dummy(supply)" function so >> that we can limit our error messages to non-dummy supplies? > > No, that's a terrible idea as previously discussed. > > The *sole* purpose of the dummy regulator driver is to provide a crutch > to keep systems with broken configuraitons running, it's not indended to > be used on production systems and is there solely as an aid for > development. This is why it can only be enabled via Kconfig. If there > is no software controllable regulator there the board should be > providing fixed regulator. It's being used on production systems, else I wouldn't be responding to bug reports from distro users of x86 laptops. Turns out that distro kernel maintainers often like to answer "y" to new Kconfig options without reading/understanding them fully. > Conversely there is nothing stopping other regulator drivers having > similar properties to the dummy driver, or constraints stopping them > implementing things. If the MMC framework wants to silently accept > regulators that don't allow the voltage to be set then the MMC framework > should enumerate the range of voltages which can be set using the > regulator it has and then take appropriate action. I didn't do this because I don't want to silently accept real regulators with insufficient voltage ranges, because that would make it harder to debug real MMC failures. Sounds like my response to the bug reports should be "persuade your distro kernel maintainers to turn off CONFIG_REGULATOR" -- I'll try doing that. Thanks, - Chris.
On Thu, Aug 30, 2012 at 01:25:58PM -0400, Chris Ball wrote: > On Thu, Aug 30 2012, Mark Brown wrote: > > Conversely there is nothing stopping other regulator drivers having > > similar properties to the dummy driver, or constraints stopping them > > implementing things. If the MMC framework wants to silently accept > > regulators that don't allow the voltage to be set then the MMC framework > > should enumerate the range of voltages which can be set using the > > regulator it has and then take appropriate action. > I didn't do this because I don't want to silently accept real regulators > with insufficient voltage ranges, because that would make it harder to > debug real MMC failures. > Sounds like my response to the bug reports should be "persuade your > distro kernel maintainers to turn off CONFIG_REGULATOR" -- I'll try > doing that. Or alternatively persuade someone to provide the appropriate fixed voltage regulators as part of instantiating the MMC device on these systems. -- 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/core/core.c b/drivers/mmc/core/core.c index 835c9f0..30d9357 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -1017,8 +1017,9 @@ int mmc_regulator_set_ocr(struct mmc_host *mmc, } if (result) - dev_err(mmc_dev(mmc), - "could not set regulator OCR (%d)\n", result); + pr_debug("%s: could not set regulator OCR (%d)\n", + mmc_hostname(mmc), result); + return result; } EXPORT_SYMBOL_GPL(mmc_regulator_set_ocr);
Some distributions build with CONFIG_REGULATOR_DUMMY=y -- this makes regulator setup succeed harmlessly, but then trying to set the voltage on the dummy regulator will fail as below: [ 6.413866] sdhci-pci 0000:03:00.0: SDHCI controller found [1180:e822] (rev 0) [ 6.414081] _regulator_get: 0000:03:00.0 supply vmmc not found, using dummy regulator [ 6.415252] mmc0: SDHCI controller on PCI [0000:03:00.0] using DMA [ 6.415294] sdhci-pci 0000:03:00.0: could not set regulator OCR (-22) [ 6.444241] sdhci-pci 0000:03:00.0: could not set regulator OCR (-22) [ 6.473648] sdhci-pci 0000:03:00.0: could not set regulator OCR (-22) [..] Since this isn't an error in this case, move the error to pr_debug(). Reference: https://bugs.archlinux.org/task/29008 Signed-off-by: Chris Ball <cjb@laptop.org> --- drivers/mmc/core/core.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)