[PATCH 1/2] mmc: dw_mmc: Handle late vmmc regulator with EPROBE_DEFER

[PATCH 1/2] mmc: dw_mmc: Handle late vmmc regulator with EPROBE_DEFER

[Doug Anderson]

It is possible to specify a regulator that should be turned on when
dw_mmc is probed.  This regulator is optional, though a warning will
be printed if it's missing.  The fact that the regulator is optional
means that (at the moment) it's not possible to use a regulator that
probes _after_ dw_mmc.

Fix this limitation by adding the ability to make vmmc required.  If a
vmmc-supply is specified in the device tree we'll assume that vmmc is
required.

Signed-off-by: Doug Anderson <dianders@chromium.org>
---
 .../devicetree/bindings/mmc/synopsis-dw-mshc.txt   |  3 +++
 drivers/mmc/host/dw_mmc.c                          | 22 ++++++++++++++++++++--
 include/linux/mmc/dw_mmc.h                         |  3 +++
 3 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt
index 726fd21..b09d2d0 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
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index bc3a1bc..d3a0f8a 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1987,8 +1987,17 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
 
 	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));
+		ret = PTR_ERR(host->vmmc);
 		host->vmmc = NULL;
+
+		if (host->pdata->vmmc_required) {
+			if (ret != -EPROBE_DEFER)
+				pr_err("%s: vmmc required: %d\n",
+					mmc_hostname(mmc), ret);
+			goto err_setup_bus;
+		}
+		pr_info("%s: no vmmc regulator found %d\n", mmc_hostname(mmc),
+			ret);
 	} else {
 		ret = regulator_enable(host->vmmc);
 		if (ret) {
@@ -2026,7 +2035,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
 
 err_setup_bus:
 	mmc_free_host(mmc);
-	return -EINVAL;
+	return ret;
 }
 
 static void dw_mci_cleanup_slot(struct dw_mci_slot *slot, unsigned int id)
@@ -2162,6 +2171,13 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
 	if (of_find_property(np, "enable-sdio-wakeup", NULL))
 		pdata->pm_caps |= MMC_PM_WAKE_SDIO_IRQ;
 
+	/*
+	 * If vmmc is directly specified in the device tree then we'll assume
+	 * it's required and honor EPROBE_DEFER so it can show up late.
+	 */
+	if (of_find_property(np, "vmmc-supply", NULL))
+		pdata->vmmc_required = 1;
+
 	return pdata;
 }
 
@@ -2352,6 +2368,8 @@ int dw_mci_probe(struct dw_mci *host)
 	/* We need at least one slot to succeed */
 	for (i = 0; i < host->num_slots; i++) {
 		ret = dw_mci_init_slot(host, i);
+		if (ret == -EPROBE_DEFER)
+			goto err_workqueue;
 		if (ret)
 			dev_dbg(host->dev, "slot %d init failed\n", i);
 		else
diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
index 198f0fa..e84d288 100644
--- a/include/linux/mmc/dw_mmc.h
+++ b/include/linux/mmc/dw_mmc.h
@@ -244,6 +244,9 @@ struct dw_mci_board {
 	/* delay in mS before detecting cards after interrupt */
 	u32 detect_delay_ms;
 
+	/* If true then we won't probe until vmmc is available */
+	bool vmmc_required;
+
 	int (*init)(u32 slot_id, irq_handler_t , void *);
 	int (*get_ro)(u32 slot_id);
 	int (*get_cd)(u32 slot_id);
-- 
1.8.3

[PATCH 2/2] mmc: dw_mmc: Add the ability to set the ciu clock frequency

[Doug Anderson]

As of now we rely on code outside of the driver to set the ciu clock
frequency.  There's no reason to do that.  Add support for setting up
the clock in the driver during probe.

Signed-off-by: Doug Anderson <dianders@chromium.org>
---
 .../devicetree/bindings/mmc/synopsis-dw-mshc.txt        | 13 +++++++++++++
 drivers/mmc/host/dw_mmc.c                               | 17 +++++++++++++----
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt
index b09d2d0..edb7659 100644
--- a/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt
+++ b/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt
@@ -39,6 +39,19 @@ Required Properties:
 
 Optional properties:
 
+* clocks: from common clock binding: handle to biu and ciu clocks for the
+  bus interface unit clock and the card interface unit clock.
+
+* clock-names: from common clock binding: Shall be "biu" and "ciu".
+  If the biu clock is missing we'll simply skip enabling it.  If the
+  ciu clock is missing we'll just assume that the clock is running at
+  clock-frequency.  It is an error to omit both the ciu clock and the
+  clock-frequency.
+
+* clock-frequency: should be the frequency (in Hz) of the ciu clock.  If this
+  is specified and the ciu clock is specified then we'll try to set the ciu
+  clock to this at probe time.
+
 * num-slots: specifies the number of slots supported by the controller.
   The number of physical slots actually used could be equal or less than the
   value specified by num-slots. If this property is not specified, the value
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index d3a0f8a..7ffb502 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2133,6 +2133,7 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
 	struct device_node *np = dev->of_node;
 	const struct dw_mci_drv_data *drv_data = host->drv_data;
 	int idx, ret;
+	u32 clock_frequency;
 
 	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
 	if (!pdata) {
@@ -2159,6 +2160,9 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
 
 	of_property_read_u32(np, "card-detect-delay", &pdata->detect_delay_ms);
 
+	if (!of_property_read_u32(np, "clock-frequency", &clock_frequency))
+		pdata->bus_hz = clock_frequency;
+
 	if (drv_data && drv_data->parse_dt) {
 		ret = drv_data->parse_dt(host);
 		if (ret)
@@ -2223,18 +2227,23 @@ int dw_mci_probe(struct dw_mci *host)
 	host->ciu_clk = devm_clk_get(host->dev, "ciu");
 	if (IS_ERR(host->ciu_clk)) {
 		dev_dbg(host->dev, "ciu clock not available\n");
+		host->bus_hz = host->pdata->bus_hz;
 	} else {
 		ret = clk_prepare_enable(host->ciu_clk);
 		if (ret) {
 			dev_err(host->dev, "failed to enable ciu clock\n");
 			goto err_clk_biu;
 		}
-	}
 
-	if (IS_ERR(host->ciu_clk))
-		host->bus_hz = host->pdata->bus_hz;
-	else
+		if (host->pdata->bus_hz) {
+			ret = clk_set_rate(host->ciu_clk, host->pdata->bus_hz);
+			if (ret)
+				dev_warn(host->dev,
+					 "Unable to set bus rate to %ul\n",
+					 host->pdata->bus_hz);
+		}
 		host->bus_hz = clk_get_rate(host->ciu_clk);
+	}
 
 	if (drv_data && drv_data->setup_clock) {
 		ret = drv_data->setup_clock(host);
-- 
1.8.3

Re: [PATCH 2/2] mmc: dw_mmc: Add the ability to set the ciu clock frequency

[Jaehoon Chung]

Looks good to me. If you can add the usage example, it will be more helpful to me.

Acked-by: Jaehoon Chung <jh80.chung@samsung.com>

On 06/07/2013 01:46 PM, Doug Anderson wrote:
> As of now we rely on code outside of the driver to set the ciu clock
> frequency.  There's no reason to do that.  Add support for setting up
> the clock in the driver during probe.
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
>  .../devicetree/bindings/mmc/synopsis-dw-mshc.txt        | 13 +++++++++++++
>  drivers/mmc/host/dw_mmc.c                               | 17 +++++++++++++----
>  2 files changed, 26 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt
> index b09d2d0..edb7659 100644
> --- a/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt
> +++ b/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt
> @@ -39,6 +39,19 @@ Required Properties:
>  
>  Optional properties:
>  
> +* clocks: from common clock binding: handle to biu and ciu clocks for the
> +  bus interface unit clock and the card interface unit clock.
> +
> +* clock-names: from common clock binding: Shall be "biu" and "ciu".
> +  If the biu clock is missing we'll simply skip enabling it.  If the
> +  ciu clock is missing we'll just assume that the clock is running at
> +  clock-frequency.  It is an error to omit both the ciu clock and the
> +  clock-frequency.
> +
> +* clock-frequency: should be the frequency (in Hz) of the ciu clock.  If this
> +  is specified and the ciu clock is specified then we'll try to set the ciu
> +  clock to this at probe time.
> +
>  * num-slots: specifies the number of slots supported by the controller.
>    The number of physical slots actually used could be equal or less than the
>    value specified by num-slots. If this property is not specified, the value
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index d3a0f8a..7ffb502 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -2133,6 +2133,7 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
>  	struct device_node *np = dev->of_node;
>  	const struct dw_mci_drv_data *drv_data = host->drv_data;
>  	int idx, ret;
> +	u32 clock_frequency;
>  
>  	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>  	if (!pdata) {
> @@ -2159,6 +2160,9 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
>  
>  	of_property_read_u32(np, "card-detect-delay", &pdata->detect_delay_ms);
>  
> +	if (!of_property_read_u32(np, "clock-frequency", &clock_frequency))
> +		pdata->bus_hz = clock_frequency;
> +
>  	if (drv_data && drv_data->parse_dt) {
>  		ret = drv_data->parse_dt(host);
>  		if (ret)
> @@ -2223,18 +2227,23 @@ int dw_mci_probe(struct dw_mci *host)
>  	host->ciu_clk = devm_clk_get(host->dev, "ciu");
>  	if (IS_ERR(host->ciu_clk)) {
>  		dev_dbg(host->dev, "ciu clock not available\n");
> +		host->bus_hz = host->pdata->bus_hz;
>  	} else {
>  		ret = clk_prepare_enable(host->ciu_clk);
>  		if (ret) {
>  			dev_err(host->dev, "failed to enable ciu clock\n");
>  			goto err_clk_biu;
>  		}
> -	}
>  
> -	if (IS_ERR(host->ciu_clk))
> -		host->bus_hz = host->pdata->bus_hz;
> -	else
> +		if (host->pdata->bus_hz) {
> +			ret = clk_set_rate(host->ciu_clk, host->pdata->bus_hz);
> +			if (ret)
> +				dev_warn(host->dev,
> +					 "Unable to set bus rate to %ul\n",
> +					 host->pdata->bus_hz);
> +		}
>  		host->bus_hz = clk_get_rate(host->ciu_clk);
> +	}
>  
>  	if (drv_data && drv_data->setup_clock) {
>  		ret = drv_data->setup_clock(host);
> 

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

[Tomasz Figa]

Hi Doug,

On Thursday 06 of June 2013 21:46:45 Doug Anderson wrote:
> It is possible to specify a regulator that should be turned on when
> dw_mmc is probed.  This regulator is optional, though a warning will
> be printed if it's missing.  The fact that the regulator is optional
> means that (at the moment) it's not possible to use a regulator that
> probes _after_ dw_mmc.
> 
> Fix this limitation by adding the ability to make vmmc required.  If a
> vmmc-supply is specified in the device tree we'll assume that vmmc is
> required.

This interesting case makes me think that regulator core should 
differentiate between regulator lookup failure due to no lookup specified 
and due to the regulator specified in lookup being unavailable, returning 
appropriate (different) error codes.

CCing Mark and Liam.

Best regards,
Tomasz

> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
>  .../devicetree/bindings/mmc/synopsis-dw-mshc.txt   |  3 +++
>  drivers/mmc/host/dw_mmc.c                          | 22
> ++++++++++++++++++++-- include/linux/mmc/dw_mmc.h                      
>   |  3 +++
>  3 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt
> b/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt index
> 726fd21..b09d2d0 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 diff --git a/drivers/mmc/host/dw_mmc.c
> b/drivers/mmc/host/dw_mmc.c index bc3a1bc..d3a0f8a 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1987,8 +1987,17 @@ static int dw_mci_init_slot(struct dw_mci *host,
> unsigned int id)
> 
>  	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));
> +		ret = PTR_ERR(host->vmmc);
>  		host->vmmc = NULL;
> +
> +		if (host->pdata->vmmc_required) {
> +			if (ret != -EPROBE_DEFER)
> +				pr_err("%s: vmmc required: %d\n",
> +					mmc_hostname(mmc), ret);
> +			goto err_setup_bus;
> +		}
> +		pr_info("%s: no vmmc regulator found %d\n", 
mmc_hostname(mmc),
> +			ret);
>  	} else {
>  		ret = regulator_enable(host->vmmc);
>  		if (ret) {
> @@ -2026,7 +2035,7 @@ static int dw_mci_init_slot(struct dw_mci *host,
> unsigned int id)
> 
>  err_setup_bus:
>  	mmc_free_host(mmc);
> -	return -EINVAL;
> +	return ret;
>  }
> 
>  static void dw_mci_cleanup_slot(struct dw_mci_slot *slot, unsigned int
> id) @@ -2162,6 +2171,13 @@ static struct dw_mci_board
> *dw_mci_parse_dt(struct dw_mci *host) if (of_find_property(np,
> "enable-sdio-wakeup", NULL))
>  		pdata->pm_caps |= MMC_PM_WAKE_SDIO_IRQ;
> 
> +	/*
> +	 * If vmmc is directly specified in the device tree then we'll 
assume
> +	 * it's required and honor EPROBE_DEFER so it can show up late.
> +	 */
> +	if (of_find_property(np, "vmmc-supply", NULL))
> +		pdata->vmmc_required = 1;
> +
>  	return pdata;
>  }
> 
> @@ -2352,6 +2368,8 @@ int dw_mci_probe(struct dw_mci *host)
>  	/* We need at least one slot to succeed */
>  	for (i = 0; i < host->num_slots; i++) {
>  		ret = dw_mci_init_slot(host, i);
> +		if (ret == -EPROBE_DEFER)
> +			goto err_workqueue;
>  		if (ret)
>  			dev_dbg(host->dev, "slot %d init failed\n", i);
>  		else
> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
> index 198f0fa..e84d288 100644
> --- a/include/linux/mmc/dw_mmc.h
> +++ b/include/linux/mmc/dw_mmc.h
> @@ -244,6 +244,9 @@ struct dw_mci_board {
>  	/* delay in mS before detecting cards after interrupt */
>  	u32 detect_delay_ms;
> 
> +	/* If true then we won't probe until vmmc is available */
> +	bool vmmc_required;
> +
>  	int (*init)(u32 slot_id, irq_handler_t , void *);
>  	int (*get_ro)(u32 slot_id);
>  	int (*get_cd)(u32 slot_id);

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

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 933 bytes --]

On Fri, Jun 07, 2013 at 12:19:58PM +0200, Tomasz Figa wrote:
> On Thursday 06 of June 2013 21:46:45 Doug Anderson wrote:

> > dw_mmc is probed.  This regulator is optional, though a warning will
> > be printed if it's missing.  The fact that the regulator is optional
> > means that (at the moment) it's not possible to use a regulator that
> > probes _after_ dw_mmc.

> > Fix this limitation by adding the ability to make vmmc required.  If a
> > vmmc-supply is specified in the device tree we'll assume that vmmc is
> > required.

> This interesting case makes me think that regulator core should 
> differentiate between regulator lookup failure due to no lookup specified 
> and due to the regulator specified in lookup being unavailable, returning 
> appropriate (different) error codes.

It does exactly that in so far as it can - you get -ENODEV if there's
definitely no supply and -EPROBE_DEFER otherwise.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Tomasz Figa]

On Friday 07 of June 2013 11:24:04 Mark Brown wrote:
> On Fri, Jun 07, 2013 at 12:19:58PM +0200, Tomasz Figa wrote:
> > On Thursday 06 of June 2013 21:46:45 Doug Anderson wrote:
> > > dw_mmc is probed.  This regulator is optional, though a warning will
> > > be printed if it's missing.  The fact that the regulator is optional
> > > means that (at the moment) it's not possible to use a regulator that
> > > probes _after_ dw_mmc.
> > > 
> > > Fix this limitation by adding the ability to make vmmc required.  If
> > > a
> > > vmmc-supply is specified in the device tree we'll assume that vmmc
> > > is
> > > required.
> > 
> > This interesting case makes me think that regulator core should
> > differentiate between regulator lookup failure due to no lookup
> > specified and due to the regulator specified in lookup being
> > unavailable, returning appropriate (different) error codes.
> 
> It does exactly that in so far as it can - you get -ENODEV if there's
> definitely no supply and -EPROBE_DEFER otherwise.

Oh, right, thanks. I somehow felt that it should be doing this already, 
but I was looking at 3.9 on Free Electron's LXR. It does so since commit

1e4b545cdd regulator: core: return err value for regulator_get if there is 
no DT binding

so I think this patch should be reworked to check the returned error code 
instead.

Best regards,
Tomasz

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

[Doug Anderson]

Tomasz / Mark,

On Fri, Jun 7, 2013 at 3:30 AM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> On Friday 07 of June 2013 11:24:04 Mark Brown wrote:
>> On Fri, Jun 07, 2013 at 12:19:58PM +0200, Tomasz Figa wrote:
>> > This interesting case makes me think that regulator core should
>> > differentiate between regulator lookup failure due to no lookup
>> > specified and due to the regulator specified in lookup being
>> > unavailable, returning appropriate (different) error codes.
>>
>> It does exactly that in so far as it can - you get -ENODEV if there's
>> definitely no supply and -EPROBE_DEFER otherwise.
>
> Oh, right, thanks. I somehow felt that it should be doing this already,
> but I was looking at 3.9 on Free Electron's LXR. It does so since commit
>
> 1e4b545cdd regulator: core: return err value for regulator_get if there is
> no DT binding

Thanks, this is much cleaner.  That's what I get for doing the
majority of my work on 3.8 at the moment.  :-P

I will rework the patch and send it out again.

-Doug

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

[Doug Anderson]

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 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);
 
-- 
1.8.3

[PATCH v2 2/2] mmc: dw_mmc: Add the ability to set the ciu clock frequency

[Doug Anderson]

As of now we rely on code outside of the driver to set the ciu clock
frequency.  There's no reason to do that.  Add support for setting up
the clock in the driver during probe.

Signed-off-by: Doug Anderson <dianders@chromium.org>
Acked-by: Jaehoon Chung <jh80.chung@samsung.com>
---
Changes in v2:
- Added example as per Jaehoon.

 .../devicetree/bindings/mmc/synopsis-dw-mshc.txt        | 16 ++++++++++++++++
 drivers/mmc/host/dw_mmc.c                               | 17 +++++++++++++----
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt
index d5cc94e..dd31b00 100644
--- a/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt
+++ b/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt
@@ -39,6 +39,19 @@ Required Properties:
 
 Optional properties:
 
+* clocks: from common clock binding: handle to biu and ciu clocks for the
+  bus interface unit clock and the card interface unit clock.
+
+* clock-names: from common clock binding: Shall be "biu" and "ciu".
+  If the biu clock is missing we'll simply skip enabling it.  If the
+  ciu clock is missing we'll just assume that the clock is running at
+  clock-frequency.  It is an error to omit both the ciu clock and the
+  clock-frequency.
+
+* clock-frequency: should be the frequency (in Hz) of the ciu clock.  If this
+  is specified and the ciu clock is specified then we'll try to set the ciu
+  clock to this at probe time.
+
 * num-slots: specifies the number of slots supported by the controller.
   The number of physical slots actually used could be equal or less than the
   value specified by num-slots. If this property is not specified, the value
@@ -70,6 +83,8 @@ board specific portions as listed below.
 
 	dwmmc0@12200000 {
 		compatible = "snps,dw-mshc";
+		clocks = <&clock 351>, <&clock 132>;
+		clock-names = "biu", "ciu";
 		reg = <0x12200000 0x1000>;
 		interrupts = <0 75 0>;
 		#address-cells = <1>;
@@ -77,6 +92,7 @@ board specific portions as listed below.
 	};
 
 	dwmmc0@12200000 {
+		clock-frequency = <400000000>;
 		num-slots = <1>;
 		supports-highspeed;
 		broken-cd;
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index ab5642d..b8a2f16 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2107,6 +2107,7 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
 	struct device_node *np = dev->of_node;
 	const struct dw_mci_drv_data *drv_data = host->drv_data;
 	int idx, ret;
+	u32 clock_frequency;
 
 	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
 	if (!pdata) {
@@ -2133,6 +2134,9 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
 
 	of_property_read_u32(np, "card-detect-delay", &pdata->detect_delay_ms);
 
+	if (!of_property_read_u32(np, "clock-frequency", &clock_frequency))
+		pdata->bus_hz = clock_frequency;
+
 	if (drv_data && drv_data->parse_dt) {
 		ret = drv_data->parse_dt(host);
 		if (ret)
@@ -2190,18 +2194,23 @@ int dw_mci_probe(struct dw_mci *host)
 	host->ciu_clk = devm_clk_get(host->dev, "ciu");
 	if (IS_ERR(host->ciu_clk)) {
 		dev_dbg(host->dev, "ciu clock not available\n");
+		host->bus_hz = host->pdata->bus_hz;
 	} else {
 		ret = clk_prepare_enable(host->ciu_clk);
 		if (ret) {
 			dev_err(host->dev, "failed to enable ciu clock\n");
 			goto err_clk_biu;
 		}
-	}
 
-	if (IS_ERR(host->ciu_clk))
-		host->bus_hz = host->pdata->bus_hz;
-	else
+		if (host->pdata->bus_hz) {
+			ret = clk_set_rate(host->ciu_clk, host->pdata->bus_hz);
+			if (ret)
+				dev_warn(host->dev,
+					 "Unable to set bus rate to %ul\n",
+					 host->pdata->bus_hz);
+		}
 		host->bus_hz = clk_get_rate(host->ciu_clk);
+	}
 
 	if (drv_data && drv_data->setup_clock) {
 		ret = drv_data->setup_clock(host);
-- 
1.8.3

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

[Tomasz Figa]

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);

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

[Doug Anderson]

Tomasz,

On Fri, Jun 7, 2013 at 10:42 AM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> 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.

I don't actually know.  I've never seen a multi-slot implementation so
I end up doing lots of speculation when I make changes.  Certainly the
code I'm moving wouldn't have worked correctly in the
more-than-one-slot case.  There's only space to pass one regulator
(it's not per-slot) and that one regulator would have just been
enabled multiple times.  ...and it would have only been disabled once
in places like suspend/resume.

If there is every a multi-slot implementation that needs vmmc on a
per-slot it seems like we could address at that time?

Thanks!

-Doug

Re: [PATCH v2 2/2] mmc: dw_mmc: Add the ability to set the ciu clock frequency

[Jaehoon Chung]

Hi Doug,

I have one question for using <clock-frequency>.
I found the fixed-rate-clocks feature.
If we want to set <clock-frequency>, then can we use the fixed-rate-clocks?
i'm not sure how use the fixed-rate-clocks. but it seems to set fixed-rate value for clock frequency.

clk_set_rate() didn't ensure to set the <clock-frequency> value.

Best Regards,
Jaehoon Chung

On 06/08/2013 02:28 AM, Doug Anderson wrote:
> As of now we rely on code outside of the driver to set the ciu clock
> frequency.  There's no reason to do that.  Add support for setting up
> the clock in the driver during probe.
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Acked-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
> Changes in v2:
> - Added example as per Jaehoon.
> 
>  .../devicetree/bindings/mmc/synopsis-dw-mshc.txt        | 16 ++++++++++++++++
>  drivers/mmc/host/dw_mmc.c                               | 17 +++++++++++++----
>  2 files changed, 29 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt
> index d5cc94e..dd31b00 100644
> --- a/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt
> +++ b/Documentation/devicetree/bindings/mmc/synopsis-dw-mshc.txt
> @@ -39,6 +39,19 @@ Required Properties:
>  
>  Optional properties:
>  
> +* clocks: from common clock binding: handle to biu and ciu clocks for the
> +  bus interface unit clock and the card interface unit clock.
> +
> +* clock-names: from common clock binding: Shall be "biu" and "ciu".
> +  If the biu clock is missing we'll simply skip enabling it.  If the
> +  ciu clock is missing we'll just assume that the clock is running at
> +  clock-frequency.  It is an error to omit both the ciu clock and the
> +  clock-frequency.
> +
> +* clock-frequency: should be the frequency (in Hz) of the ciu clock.  If this
> +  is specified and the ciu clock is specified then we'll try to set the ciu
> +  clock to this at probe time.
> +
>  * num-slots: specifies the number of slots supported by the controller.
>    The number of physical slots actually used could be equal or less than the
>    value specified by num-slots. If this property is not specified, the value
> @@ -70,6 +83,8 @@ board specific portions as listed below.
>  
>  	dwmmc0@12200000 {
>  		compatible = "snps,dw-mshc";
> +		clocks = <&clock 351>, <&clock 132>;
> +		clock-names = "biu", "ciu";
>  		reg = <0x12200000 0x1000>;
>  		interrupts = <0 75 0>;
>  		#address-cells = <1>;
> @@ -77,6 +92,7 @@ board specific portions as listed below.
>  	};
>  
>  	dwmmc0@12200000 {
> +		clock-frequency = <400000000>;
>  		num-slots = <1>;
>  		supports-highspeed;
>  		broken-cd;
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index ab5642d..b8a2f16 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -2107,6 +2107,7 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
>  	struct device_node *np = dev->of_node;
>  	const struct dw_mci_drv_data *drv_data = host->drv_data;
>  	int idx, ret;
> +	u32 clock_frequency;
>  
>  	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>  	if (!pdata) {
> @@ -2133,6 +2134,9 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
>  
>  	of_property_read_u32(np, "card-detect-delay", &pdata->detect_delay_ms);
>  
> +	if (!of_property_read_u32(np, "clock-frequency", &clock_frequency))
> +		pdata->bus_hz = clock_frequency;
> +
>  	if (drv_data && drv_data->parse_dt) {
>  		ret = drv_data->parse_dt(host);
>  		if (ret)
> @@ -2190,18 +2194,23 @@ int dw_mci_probe(struct dw_mci *host)
>  	host->ciu_clk = devm_clk_get(host->dev, "ciu");
>  	if (IS_ERR(host->ciu_clk)) {
>  		dev_dbg(host->dev, "ciu clock not available\n");
> +		host->bus_hz = host->pdata->bus_hz;
>  	} else {
>  		ret = clk_prepare_enable(host->ciu_clk);
>  		if (ret) {
>  			dev_err(host->dev, "failed to enable ciu clock\n");
>  			goto err_clk_biu;
>  		}
> -	}
>  
> -	if (IS_ERR(host->ciu_clk))
> -		host->bus_hz = host->pdata->bus_hz;
> -	else
> +		if (host->pdata->bus_hz) {
> +			ret = clk_set_rate(host->ciu_clk, host->pdata->bus_hz);
> +			if (ret)
> +				dev_warn(host->dev,
> +					 "Unable to set bus rate to %ul\n",
> +					 host->pdata->bus_hz);
> +		}
>  		host->bus_hz = clk_get_rate(host->ciu_clk);
> +	}
>  
>  	if (drv_data && drv_data->setup_clock) {
>  		ret = drv_data->setup_clock(host);
> 

Re: [PATCH v2 2/2] mmc: dw_mmc: Add the ability to set the ciu clock frequency

[Doug Anderson]

Jaehoon,

On Mon, Jun 17, 2013 at 9:51 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> Hi Doug,
>
> I have one question for using <clock-frequency>.
> I found the fixed-rate-clocks feature.
> If we want to set <clock-frequency>, then can we use the fixed-rate-clocks?
> i'm not sure how use the fixed-rate-clocks. but it seems to set fixed-rate value for clock frequency.
>
> clk_set_rate() didn't ensure to set the <clock-frequency> value.

I'm not sure I understand the question.  I don't think that the
fixed-rate-clocks have a close relation to the clock-frequency or the
ciu clock.  The fixed-rate-clock entries for a board usually specify
the root clock source for a board.  For instance in exynos5250-snow
you can see:

fixed-rate-clocks {
  xxti {
    compatible = "samsung,clock-xxti";
    clock-frequency = <24000000>;
  };
};

Other clocks in the board are derived from this clock through PLLs,
muxes, dividers, gates, etc.  On 5250 we have:

 fin_pll (xxti) -> fout_mpll -> fout_mplldiv2 -> mout_mpll_fout ->
 sclk_mpll -> sclk_mpll_user -> mout_mmc1 -> div_mmc1
 div_mmc_pre1 -> sclk_mmc1

In 5250 the ciu clock for mmc1 is sclk_mmc1, which is a simple gate.
When you "enable" this clock it, ungates it.  The sclk_mmc1 has the
flag CLK_SET_RATE_PARENT on it.  That means when you try to set the
rate it will involve the parent clock (div_mmc_pre1).  The parent
clock also has CLK_SET_RATE_PARENT, so it can also involve div_mmc1.
I haven't dug through to see how the clock framework splits up divides
between div_mmc1 and div_mmc_pre1, but it's supposed to handle that.

We don't allow clk_set_rate to percolate any higher (no
CLK_SET_RATE_PARENT at mout_mmc1).

-Doug

Re: [PATCH v2 2/2] mmc: dw_mmc: Add the ability to set the ciu clock frequency

[Jaehoon Chung]

Hi Doug,

I'm researching for fixed-rate-clocks.
Maybe i misunderstood for using <fixed-rate-clocks>. :)

Best Regards,
Jaehoon Chung

On 06/19/2013 12:15 AM, Doug Anderson wrote:
> Jaehoon,
> 
> On Mon, Jun 17, 2013 at 9:51 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>> Hi Doug,
>>
>> I have one question for using <clock-frequency>.
>> I found the fixed-rate-clocks feature.
>> If we want to set <clock-frequency>, then can we use the fixed-rate-clocks?
>> i'm not sure how use the fixed-rate-clocks. but it seems to set fixed-rate value for clock frequency.
>>
>> clk_set_rate() didn't ensure to set the <clock-frequency> value.
> 
> I'm not sure I understand the question.  I don't think that the
> fixed-rate-clocks have a close relation to the clock-frequency or the
> ciu clock.  The fixed-rate-clock entries for a board usually specify
> the root clock source for a board.  For instance in exynos5250-snow
> you can see:
> 
> fixed-rate-clocks {
>   xxti {
>     compatible = "samsung,clock-xxti";
>     clock-frequency = <24000000>;
>   };
> };
> 
> Other clocks in the board are derived from this clock through PLLs,
> muxes, dividers, gates, etc.  On 5250 we have:
> 
>  fin_pll (xxti) -> fout_mpll -> fout_mplldiv2 -> mout_mpll_fout ->
>  sclk_mpll -> sclk_mpll_user -> mout_mmc1 -> div_mmc1
>  div_mmc_pre1 -> sclk_mmc1
> 
> In 5250 the ciu clock for mmc1 is sclk_mmc1, which is a simple gate.
> When you "enable" this clock it, ungates it.  The sclk_mmc1 has the
> flag CLK_SET_RATE_PARENT on it.  That means when you try to set the
> rate it will involve the parent clock (div_mmc_pre1).  The parent
> clock also has CLK_SET_RATE_PARENT, so it can also involve div_mmc1.
> I haven't dug through to see how the clock framework splits up divides
> between div_mmc1 and div_mmc_pre1, but it's supposed to handle that.
> 
> We don't allow clk_set_rate to percolate any higher (no
> CLK_SET_RATE_PARENT at mout_mmc1).
> 
> -Doug
> --
> 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
> 

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

[Chris Ball]

Hi Doug,

On Fri, Jun 07 2013, 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 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.

Thanks, pushed to mmc-next for 3.11.

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [PATCH v2 2/2] mmc: dw_mmc: Add the ability to set the ciu clock frequency

[Chris Ball]

Hi Doug,

On Fri, Jun 07 2013, Doug Anderson wrote:
> As of now we rely on code outside of the driver to set the ciu clock
> frequency.  There's no reason to do that.  Add support for setting up
> the clock in the driver during probe.
>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Acked-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
> Changes in v2:
> - Added example as per Jaehoon.

Thanks, pushed to mmc-next for 3.11.

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

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

[Chris Ball]

Hi Doug,

On Fri, Jun 07 2013, Doug Anderson wrote:
> 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)

This hunk breaks the build for me, because err_setup_bus and ret are
used in the error path of the call to mmc_add_host() in this function.

I'll push a version that leaves those in.  Let me know if you think
something strange is happening that made this work okay for you, like
a mismerge.

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

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

[Doug Anderson]

Chris,

On Thu, Jun 27, 2013 at 9:44 AM, Chris Ball <cjb@laptop.org> wrote:
> This hunk breaks the build for me, because err_setup_bus and ret are
> used in the error path of the call to mmc_add_host() in this function.
>
> I'll push a version that leaves those in.  Let me know if you think
> something strange is happening that made this work okay for you, like
> a mismerge.

WTF.  OK, this is clearly my fault.  I went back to the exact patch
and it didn't compile for me, either.  :(  My typical workflow is to
develop something in our local tree and then cherry-pick (and test!)
on ToT linux.  Probably what happened in this case was that there was
a compile error and I didn't notice and somehow ended up testing my
local tree (which doesn't error-check mmc_add_host).

Sorry for the hassle.  Leaving the err_setup_bus in is correct (and
leaving the "ret" variable declared).

I did those fixups and it now compiles and things boot.  I haven't
tested the deferral case in ToT yet, though.  I'll try that now and
post if something weird comes up.

-Doug