On Thu, Nov 18, 2010 at 11:36:24PM +0000, Seibel, Eric wrote:
> >From 5d9f15f00ea02f46385857ba7fb5a6db8741fca0 Mon Sep 17 00:00:00 2001
> From: Pierre Tardy <[email protected]>
> Date: Fri, 12 Nov 2010 17:37:48 +0100
> Subject: [PATCH 1/6] sdhci: allow platform to trigger regulator detection
>
> One some platforms, the regulators associated to the mmc are available
> late in the boot.
> There might be race condition between the time gpio expander controlling the
> mmc regulator is probed and sdhci is probed.
>
> sdhci_pci_request_regulators() is called by platform code at a time where it
> is known that the regulator
> is available to retry getting the regulators associated to pci sdhcis
>
> Signed-off-by: Pierre Tardy <[email protected]>
> Signed-off-by: Sebastien Busson <[email protected]>
> ---
> drivers/mmc/host/sdhci-pci.c | 42
> ++++++++++++++++++++++++++++++++++++++++++
> drivers/mmc/host/sdhci.c | 30 ++++++++++++++++++++++--------
> drivers/mmc/host/sdhci.h | 1 +
> 3 files changed, 65 insertions(+), 8 deletions(-)
Is this patch accepted upstream yet? If not, why not? If so, what is
the git commit id of it?
>
> diff --git a/drivers/mmc/host/sdhci-pci.c b/drivers/mmc/host/sdhci-pci.c
> index 7bdbd9c..f251d06 100644
> --- a/drivers/mmc/host/sdhci-pci.c
> +++ b/drivers/mmc/host/sdhci-pci.c
> @@ -24,6 +24,7 @@
> #include <asm/scatterlist.h>
> #include <asm/io.h>
> #include <asm/intel_scu_ipc.h>
> +#include <linux/regulator/consumer.h>
That needs to go before the asm/ includes.
>
> #include "sdhci.h"
>
> @@ -637,6 +638,45 @@ static struct sdhci_ops sdhci_pci_ops = {
> .enable_dma = sdhci_pci_enable_dma,
> };
>
> +
> +/*****************************************************************************\
> + *
> *
> + * SDHCI regulator workaround
> *
> + * One some platforms, the regulators associated to the mmc are available
> *
> + * late in the boot. *
> + * sdhci_pci_request_regulators() is called by platform code to retry
> getting*
> + * the regulators associated to pci sdhcis *
Those trailing * don't seem to line up for me.
> + *
> *
> + *
> *
> +\*****************************************************************************/
That's not the kernel comment style.
> +
> +static int try_request_regulator(struct device *dev, void *data)
> +{
> + struct pci_dev *pdev = container_of(dev, struct pci_dev, dev);
> + struct sdhci_pci_chip *chip;
> + struct sdhci_pci_slot *slot;
> + struct sdhci_host *host;
Mix of tabs and no tabs for variables?
> + int i;
> + chip = pci_get_drvdata(pdev);
You need an extra line after the i declaration.
> + if (!chip)
> + return 0;
Shouldn't this be an error?
> +
> + for (i = 0; i < chip->num_slots; i++) {
> + slot = chip->slots[i];
> + host = slot->host;
> + if (!host)
> + continue;
> + sdhci_try_get_regulator(host);
> + }
> + return 0;
> +}
> +static struct pci_driver sdhci_driver;
Extra lines here please
> +int sdhci_pci_request_regulators(void)
> +{
> + return driver_for_each_device(&sdhci_driver.driver, NULL, NULL,
> try_request_regulator);
> +}
> +EXPORT_SYMBOL_GPL(sdhci_pci_request_regulators);
You obviously ignored the checkpatch.pl warnings for a valid reason,
what was it?
> +
>
> /*****************************************************************************\
> *
> *
> * Suspend/resume
> *
> @@ -932,7 +972,9 @@ static int __devinit sdhci_pci_probe(struct pci_dev *pdev,
>
> chip->slots[i] = slot;
> }
> + /* prevent race condition with platform code */
>
> + sdhci_pci_request_regulators();
> return 0;
>
> free:
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index f6a2b8a..894c086 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1778,6 +1778,27 @@ struct sdhci_host *sdhci_alloc_host(struct device *dev,
>
> EXPORT_SYMBOL_GPL(sdhci_alloc_host);
>
> +void sdhci_try_get_regulator(struct sdhci_host *host)
Where's the documentation on this new, global function? It needs to be
here, in kerneldoc format.
> +{
> + struct regulator *vmmc;
> + unsigned long flags;
> + if (!host->vmmc) {
> + vmmc = regulator_get(mmc_dev(host->mmc), "vmmc");
> + if (!IS_ERR(vmmc)) {
> + spin_lock_irqsave(&host->lock, flags);
> + if (!host->vmmc) {
> + host->vmmc = vmmc;
> + spin_unlock_irqrestore(&host->lock, flags);
> + regulator_enable(host->vmmc);
> + mmc_detect_change(host->mmc, 0);
> + } else { /* race! we got the regulator twice */
Put the comment on the next line.
> + spin_unlock_irqrestore(&host->lock, flags);
> + regulator_put(vmmc);
> + }
> + }
> + }
> +}
> +EXPORT_SYMBOL_GPL(sdhci_try_get_regulator);
> int sdhci_add_host(struct sdhci_host *host)
Extra line before the new function please.
thanks,
greg k-h
_______________________________________________
MeeGo-kernel mailing list
[email protected]
http://lists.meego.com/listinfo/meego-kernel
