| Message ID | 1350976740-19284-3-git-send-email-pkunapuli@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Am Dienstag, den 23.10.2012, 12:49 +0530 schrieb Pavan Kunapuli: > vmmc and vqmmc regulators control the voltage to > the host and device. Defer the probe if either of > them is not registered. > Does this work with boards where we don't have any MMC supplies? Or are we just deferring the probe indefinitely there? For boards that power MMC unconditionally, are we supposed to add dummy regulators to make them work with this patchset? > Signed-off-by: Pavan Kunapuli <pkunapuli@nvidia.com> > --- > drivers/mmc/host/sdhci.c | 25 ++++++++++++++++++++++--- > 1 files changed, 22 insertions(+), 3 deletions(-) > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index 7922adb..925c403 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -2844,11 +2844,17 @@ int sdhci_add_host(struct sdhci_host *host) > !(host->mmc->caps & MMC_CAP_NONREMOVABLE)) > mmc->caps |= MMC_CAP_NEEDS_POLL; > > - /* If vqmmc regulator and no 1.8V signalling, then there's no UHS */ > + /* > + * If vqmmc regulator and no 1.8V signalling, then there's no UHS. > + * vqmmc regulator should be present. If it's not present, > + * assume the regulator driver registration is not yet done and > + * defer the probe. > + */ > host->vqmmc = regulator_get(mmc_dev(mmc), "vqmmc"); > if (IS_ERR(host->vqmmc)) { > - pr_info("%s: no vqmmc regulator found\n", mmc_hostname(mmc)); > + pr_err("%s: no vqmmc regulator found\n", mmc_hostname(mmc)); > host->vqmmc = NULL; > + return -EPROBE_DEFER; > } > else if (regulator_is_supported_voltage(host->vqmmc, 1800000, 1800000)) > regulator_enable(host->vqmmc); > @@ -2903,10 +2909,17 @@ int sdhci_add_host(struct sdhci_host *host) > > ocr_avail = 0; > > + /* > + * vmmc regulator should be present. If it's not present, > + * assume the regulator driver registration is not yet done > + * and defer the probe. > + */ > host->vmmc = regulator_get(mmc_dev(mmc), "vmmc"); > if (IS_ERR(host->vmmc)) { > - pr_info("%s: no vmmc regulator found\n", mmc_hostname(mmc)); > + pr_err("%s: no vmmc regulator found\n", mmc_hostname(mmc)); > host->vmmc = NULL; > + ret = -EPROBE_DEFER; > + goto vmmc_err; > } else > regulator_enable(host->vmmc); > > @@ -3121,7 +3134,13 @@ reset: > untasklet: > tasklet_kill(&host->card_tasklet); > tasklet_kill(&host->finish_tasklet); > +vmmc_err: > + if (host->vqmmc) { > + if (regulator_is_enabled(host->vqmmc)) > + regulator_disable(host->vqmmc); > > + regulator_put(host->vqmmc); > + } > return ret; > } >
>> Does this work with boards where we don't have any MMC supplies? Or are we just deferring the probe indefinitely there? The probe will be deferred indefinitely. >> For boards that power MMC unconditionally, are we supposed to add dummy regulators to make them work with this patchset? Yes. We need to add dummy regulators for boards that power MMC unconditionally. I am not sure if there is any other solution other than using deferred probe that would guarantee regulators availability. -----Original Message----- From: Lucas Stach [mailto:dev@lynxeye.de] Sent: 23 October 2012 13:27 To: Pavan Kunapuli Cc: swarren@wwwdotorg.org; linux@arm.linux.org.uk; cjb@laptop.org; linux-tegra@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-mmc@vger.kernel.org Subject: Re: [PATCH 2/2] mmc: sdhci: Defer probe if regulator_get fails Am Dienstag, den 23.10.2012, 12:49 +0530 schrieb Pavan Kunapuli: > vmmc and vqmmc regulators control the voltage to the host and device. > Defer the probe if either of them is not registered. > Does this work with boards where we don't have any MMC supplies? Or are we just deferring the probe indefinitely there? For boards that power MMC unconditionally, are we supposed to add dummy regulators to make them work with this patchset? > Signed-off-by: Pavan Kunapuli <pkunapuli@nvidia.com> > --- > drivers/mmc/host/sdhci.c | 25 ++++++++++++++++++++++--- > 1 files changed, 22 insertions(+), 3 deletions(-) > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index > 7922adb..925c403 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -2844,11 +2844,17 @@ int sdhci_add_host(struct sdhci_host *host) > !(host->mmc->caps & MMC_CAP_NONREMOVABLE)) > mmc->caps |= MMC_CAP_NEEDS_POLL; > > - /* If vqmmc regulator and no 1.8V signalling, then there's no UHS */ > + /* > + * If vqmmc regulator and no 1.8V signalling, then there's no UHS. > + * vqmmc regulator should be present. If it's not present, > + * assume the regulator driver registration is not yet done and > + * defer the probe. > + */ > host->vqmmc = regulator_get(mmc_dev(mmc), "vqmmc"); > if (IS_ERR(host->vqmmc)) { > - pr_info("%s: no vqmmc regulator found\n", mmc_hostname(mmc)); > + pr_err("%s: no vqmmc regulator found\n", mmc_hostname(mmc)); > host->vqmmc = NULL; > + return -EPROBE_DEFER; > } > else if (regulator_is_supported_voltage(host->vqmmc, 1800000, 1800000)) > regulator_enable(host->vqmmc); > @@ -2903,10 +2909,17 @@ int sdhci_add_host(struct sdhci_host *host) > > ocr_avail = 0; > > + /* > + * vmmc regulator should be present. If it's not present, > + * assume the regulator driver registration is not yet done > + * and defer the probe. > + */ > host->vmmc = regulator_get(mmc_dev(mmc), "vmmc"); > if (IS_ERR(host->vmmc)) { > - pr_info("%s: no vmmc regulator found\n", mmc_hostname(mmc)); > + pr_err("%s: no vmmc regulator found\n", mmc_hostname(mmc)); > host->vmmc = NULL; > + ret = -EPROBE_DEFER; > + goto vmmc_err; > } else > regulator_enable(host->vmmc); > > @@ -3121,7 +3134,13 @@ reset: > untasklet: > tasklet_kill(&host->card_tasklet); > tasklet_kill(&host->finish_tasklet); > +vmmc_err: > + if (host->vqmmc) { > + if (regulator_is_enabled(host->vqmmc)) > + regulator_disable(host->vqmmc); > > + regulator_put(host->vqmmc); > + } > return ret; > } >
On 23 Oct 2012, at 09:19, Pavan Kunapuli <pkunapuli@nvidia.com> wrote: > vmmc and vqmmc regulators control the voltage to > the host and device. Defer the probe if either of > them is not registered. > > Signed-off-by: Pavan Kunapuli <pkunapuli@nvidia.com> > --- > drivers/mmc/host/sdhci.c | 25 ++++++++++++++++++++++--- > 1 files changed, 22 insertions(+), 3 deletions(-) > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index 7922adb..925c403 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -2844,11 +2844,17 @@ int sdhci_add_host(struct sdhci_host *host) > !(host->mmc->caps & MMC_CAP_NONREMOVABLE)) > mmc->caps |= MMC_CAP_NEEDS_POLL; > > - /* If vqmmc regulator and no 1.8V signalling, then there's no UHS */ > + /* > + * If vqmmc regulator and no 1.8V signalling, then there's no UHS. > + * vqmmc regulator should be present. If it's not present, > + * assume the regulator driver registration is not yet done and > + * defer the probe. > + */ > host->vqmmc = regulator_get(mmc_dev(mmc), "vqmmc"); > if (IS_ERR(host->vqmmc)) { > - pr_info("%s: no vqmmc regulator found\n", mmc_hostname(mmc)); > + pr_err("%s: no vqmmc regulator found\n", mmc_hostname(mmc)); > host->vqmmc = NULL; > + return -EPROBE_DEFER; Some systems exist where vmmc regulator exists and vqmmc regulator does not. The vmmc regular is fixed at 3.3v. Deferring the probe will cause issues. > } > else if (regulator_is_supported_voltage(host->vqmmc, 1800000, 1800000)) > regulator_enable(host->vqmmc); > @@ -2903,10 +2909,17 @@ int sdhci_add_host(struct sdhci_host *host) > > ocr_avail = 0; > > + /* > + * vmmc regulator should be present. If it's not present, > + * assume the regulator driver registration is not yet done > + * and defer the probe. > + */ > host->vmmc = regulator_get(mmc_dev(mmc), "vmmc"); > if (IS_ERR(host->vmmc)) { > - pr_info("%s: no vmmc regulator found\n", mmc_hostname(mmc)); > + pr_err("%s: no vmmc regulator found\n", mmc_hostname(mmc)); > host->vmmc = NULL; > + ret = -EPROBE_DEFER; > + goto vmmc_err; > } else > regulator_enable(host->vmmc); > > @@ -3121,7 +3134,13 @@ reset: > untasklet: > tasklet_kill(&host->card_tasklet); > tasklet_kill(&host->finish_tasklet); > +vmmc_err: > + if (host->vqmmc) { > + if (regulator_is_enabled(host->vqmmc)) > + regulator_disable(host->vqmmc); > > + regulator_put(host->vqmmc); > + } > return ret; > } > > -- > 1.7.1.1 > > -- > 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 Tue, Oct 23, 2012 at 03:21:58PM +0200, Philip Rakity wrote: > On 23 Oct 2012, at 09:19, Pavan Kunapuli <pkunapuli@nvidia.com> wrote: > > - /* If vqmmc regulator and no 1.8V signalling, then there's no UHS */ > > + /* > > + * If vqmmc regulator and no 1.8V signalling, then there's no UHS. > > + * vqmmc regulator should be present. If it's not present, > > + * assume the regulator driver registration is not yet done and > > + * defer the probe. > > + */ > > host->vqmmc = regulator_get(mmc_dev(mmc), "vqmmc"); > > if (IS_ERR(host->vqmmc)) { > > - pr_info("%s: no vqmmc regulator found\n", mmc_hostname(mmc)); > > + pr_err("%s: no vqmmc regulator found\n", mmc_hostname(mmc)); > > host->vqmmc = NULL; > > + return -EPROBE_DEFER; > > Some systems exist where vmmc regulator exists and vqmmc regulator does not. The vmmc regular is fixed at 3.3v. > Deferring the probe will cause issues. That's one of the issues of deferred probing: you don't know if one of the _get() functions failed because it just isn't present, or whether it's failed because it hasn't been registered yet. The two conditions are indistinguishable from the driver point of view. The solution to it is to ensure that those drivers where hardware resource X is optional provide a dummy resource to the driver when the hardware resource is not available. That means, as far as the driver is concerned, that it will always expect to get a full set of resources whether or not the hardware has them. If drivers care about such stuff, then we should have xxx_is_dummy() functions (eg, clk_is_dummy(clk)) which return TRUE when the hardware resource is not available. (which we could use NULL to indicate and would be in keeping with the specified IS_ERR() usage of these APIs.)
On 10/23/2012 01:57 AM, Lucas Stach wrote: > Am Dienstag, den 23.10.2012, 12:49 +0530 schrieb Pavan Kunapuli: >> vmmc and vqmmc regulators control the voltage to >> the host and device. Defer the probe if either of >> them is not registered. > > Does this work with boards where we don't have any MMC supplies? Or are > we just deferring the probe indefinitely there? > > For boards that power MMC unconditionally, are we supposed to add dummy > regulators to make them work with this patchset? I believe that dummy (fixed) regulators are supposed to be provided in all cases where the platform doesn't actually have one. The fact that everything worked OK without them before this patch was most likely a mistake/accident.
On Tue, Oct 23, 2012 at 10:29:41AM -0600, Stephen Warren wrote: > On 10/23/2012 01:57 AM, Lucas Stach wrote: > > Does this work with boards where we don't have any MMC supplies? Or are > > we just deferring the probe indefinitely there? > > For boards that power MMC unconditionally, are we supposed to add dummy > > regulators to make them work with this patchset? > I believe that dummy (fixed) regulators are supposed to be provided in > all cases where the platform doesn't actually have one. The fact that > everything worked OK without them before this patch was most likely a > mistake/accident. That's the general idea; clearly there *are* supplies here, they're just not software controlled supplies.
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 7922adb..925c403 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -2844,11 +2844,17 @@ int sdhci_add_host(struct sdhci_host *host) !(host->mmc->caps & MMC_CAP_NONREMOVABLE)) mmc->caps |= MMC_CAP_NEEDS_POLL; - /* If vqmmc regulator and no 1.8V signalling, then there's no UHS */ + /* + * If vqmmc regulator and no 1.8V signalling, then there's no UHS. + * vqmmc regulator should be present. If it's not present, + * assume the regulator driver registration is not yet done and + * defer the probe. + */ host->vqmmc = regulator_get(mmc_dev(mmc), "vqmmc"); if (IS_ERR(host->vqmmc)) { - pr_info("%s: no vqmmc regulator found\n", mmc_hostname(mmc)); + pr_err("%s: no vqmmc regulator found\n", mmc_hostname(mmc)); host->vqmmc = NULL; + return -EPROBE_DEFER; } else if (regulator_is_supported_voltage(host->vqmmc, 1800000, 1800000)) regulator_enable(host->vqmmc); @@ -2903,10 +2909,17 @@ int sdhci_add_host(struct sdhci_host *host) ocr_avail = 0; + /* + * vmmc regulator should be present. If it's not present, + * assume the regulator driver registration is not yet done + * and defer the probe. + */ host->vmmc = regulator_get(mmc_dev(mmc), "vmmc"); if (IS_ERR(host->vmmc)) { - pr_info("%s: no vmmc regulator found\n", mmc_hostname(mmc)); + pr_err("%s: no vmmc regulator found\n", mmc_hostname(mmc)); host->vmmc = NULL; + ret = -EPROBE_DEFER; + goto vmmc_err; } else regulator_enable(host->vmmc); @@ -3121,7 +3134,13 @@ reset: untasklet: tasklet_kill(&host->card_tasklet); tasklet_kill(&host->finish_tasklet); +vmmc_err: + if (host->vqmmc) { + if (regulator_is_enabled(host->vqmmc)) + regulator_disable(host->vqmmc); + regulator_put(host->vqmmc); + } return ret; }
vmmc and vqmmc regulators control the voltage to the host and device. Defer the probe if either of them is not registered. Signed-off-by: Pavan Kunapuli <pkunapuli@nvidia.com> --- drivers/mmc/host/sdhci.c | 25 ++++++++++++++++++++++--- 1 files changed, 22 insertions(+), 3 deletions(-)