Re: [PATCH v2 1/2] mmc: dw_mmc: Handle late vmmc regulators with EPROBE_DEFER

[Tomasz Figa <tomasz.figa@gmail.com>]

On Friday 07 of June 2013 10:28:29 Doug Anderson wrote:
> It is possible to specify a regulator that should be turned on when
> dw_mmc is probed.  At the moment dw_mmc will fail to use the regulator
> properly if the regulator probes after dw_mmc.  Fix this problem by
> honoring EPROBE_DEFER.
> 
> At the same time move the regulator code out of the slot init code.
> We only specify one regulator for the whole device

A question just out of curiousity: Is it really correct to have one vmmc 
regulator for the whole device, instead of one regulator per slot?

This might be something obvious, but I don't know any details about 
dw_mmc, so sorry if it's the case.

Best regards,
Tomasz

> and other parts of
> the code (like suspend/resume) assume that the regulator has only been
> enabled once.
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
> Changes in v2:
> - Avoid hackery to detect regulators that will never show up.
> - Move regulator out of slot init--it doesn't belong there.
> 
>  .../devicetree/bindings/mmc/synopsis-dw-mshc.txt   |  4 +++
>  drivers/mmc/host/dw_mmc.c                          | 40
> ++++++++++++---------- 2 files changed, 25 insertions(+), 19
> deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt
> b/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt index
> 726fd21..d5cc94e 100644
> --- a/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt
> +++ b/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt
> @@ -55,6 +55,9 @@ Optional properties:
> 
>  * broken-cd: as documented in mmc core bindings.
> 
> +* vmmc-supply: The phandle to the regulator to use for vmmc.  If this
> is +  specified we'll defer probe until we can find this regulator. +
>  Aliases:
> 
>  - All the MSHC controller nodes should be represented in the aliases
> node using @@ -79,6 +82,7 @@ board specific portions as listed below.
>  		broken-cd;
>  		fifo-depth = <0x80>;
>  		card-detect-delay = <200>;
> +		vmmc-supply = <&buck8>;
> 
>  		slot@0 {
>  			reg = <0>;
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index bc3a1bc..ab5642d 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1895,7 +1895,7 @@ static int dw_mci_init_slot(struct dw_mci *host,
> unsigned int id) struct mmc_host *mmc;
>  	struct dw_mci_slot *slot;
>  	const struct dw_mci_drv_data *drv_data = host->drv_data;
> -	int ctrl_id, ret;
> +	int ctrl_id;
>  	u8 bus_width;
> 
>  	mmc = mmc_alloc_host(sizeof(struct dw_mci_slot), host->dev);
> @@ -1985,19 +1985,6 @@ static int dw_mci_init_slot(struct dw_mci *host,
> unsigned int id) #endif /* CONFIG_MMC_DW_IDMAC */
>  	}
> 
> -	host->vmmc = devm_regulator_get(mmc_dev(mmc), "vmmc");
> -	if (IS_ERR(host->vmmc)) {
> -		pr_info("%s: no vmmc regulator found\n", 
mmc_hostname(mmc));
> -		host->vmmc = NULL;
> -	} else {
> -		ret = regulator_enable(host->vmmc);
> -		if (ret) {
> -			dev_err(host->dev,
> -				"failed to enable regulator: %d\n", ret);
> -			goto err_setup_bus;
> -		}
> -	}
> -
>  	if (dw_mci_get_cd(mmc))
>  		set_bit(DW_MMC_CARD_PRESENT, &slot->flags);
>  	else
> @@ -2023,10 +2010,6 @@ static int dw_mci_init_slot(struct dw_mci *host,
> unsigned int id) queue_work(host->card_workqueue, &host->card_work);
> 
>  	return 0;
> -
> -err_setup_bus:
> -	mmc_free_host(mmc);
> -	return -EINVAL;
>  }
> 
>  static void dw_mci_cleanup_slot(struct dw_mci_slot *slot, unsigned int
> id) @@ -2229,11 +2212,29 @@ int dw_mci_probe(struct dw_mci *host)
>  		}
>  	}
> 
> +	host->vmmc = devm_regulator_get(host->dev, "vmmc");
> +	if (IS_ERR(host->vmmc)) {
> +		ret = PTR_ERR(host->vmmc);
> +		if (ret == -EPROBE_DEFER)
> +			goto err_clk_ciu;
> +
> +		dev_info(host->dev, "no vmmc regulator found: %d\n", ret);
> +		host->vmmc = NULL;
> +	} else {
> +		ret = regulator_enable(host->vmmc);
> +		if (ret) {
> +			if (ret != -EPROBE_DEFER)
> +				dev_err(host->dev,
> +					"regulator_enable fail: %d\n", 
ret);
> +			goto err_clk_ciu;
> +		}
> +	}
> +
>  	if (!host->bus_hz) {
>  		dev_err(host->dev,
>  			"Platform data must supply bus speed\n");
>  		ret = -ENODEV;
> -		goto err_clk_ciu;
> +		goto err_regulator;
>  	}
> 
>  	host->quirks = host->pdata->quirks;
> @@ -2378,6 +2379,7 @@ err_dmaunmap:
>  	if (host->use_dma && host->dma_ops->exit)
>  		host->dma_ops->exit(host);
> 
> +err_regulator:
>  	if (host->vmmc)
>  		regulator_disable(host->vmmc);