diff mbox

[v5,4/9] mmc: dw_mmc: lookup for optional biu and ciu clocks

Message ID 1346237295-7116-5-git-send-email-thomas.abraham@linaro.org
State New
Headers show

Commit Message

thomas.abraham@linaro.org Aug. 29, 2012, 10:48 a.m. UTC
Some platforms allow for clock gating and control of bus interface unit clock
and card interface unit clock. Add support for clock lookup of optional biu
and ciu clocks for clock gating and clock speed determination.

Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com>
Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
Acked-by: Will Newton <will.newton@imgtec.com>
---
 drivers/mmc/host/dw_mmc.c  |   42 +++++++++++++++++++++++++++++++++++++++---
 include/linux/mmc/dw_mmc.h |    4 ++++
 2 files changed, 43 insertions(+), 3 deletions(-)

Comments

Jaehoon Chung Aug. 30, 2012, 8:48 a.m. UTC | #1
Hi Thomas,

On 08/29/2012 07:48 PM, Thomas Abraham wrote:
> Some platforms allow for clock gating and control of bus interface unit clock
> and card interface unit clock. Add support for clock lookup of optional biu
> and ciu clocks for clock gating and clock speed determination.
> 
> Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com>
> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
> Acked-by: Will Newton <will.newton@imgtec.com>
> ---
>  drivers/mmc/host/dw_mmc.c  |   42 +++++++++++++++++++++++++++++++++++++++---
>  include/linux/mmc/dw_mmc.h |    4 ++++
>  2 files changed, 43 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 227c42e..90c7c7b 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1960,18 +1960,38 @@ int dw_mci_probe(struct dw_mci *host)
>  		return -ENODEV;
>  	}
>  
> -	if (!host->pdata->bus_hz) {
> +	host->biu_clk = clk_get(host->dev, "biu");
> +	if (IS_ERR(host->biu_clk))
> +		dev_dbg(host->dev, "biu clock not available\n");
> +	else
> +		clk_prepare_enable(host->biu_clk);
biu is clock for bus interface?
if didn't get "biu_clk" or didn't clk_prepare_enable(), then can we initialize the card?
> +
> +	host->ciu_clk = clk_get(host->dev, "ciu");
> +	if (IS_ERR(host->ciu_clk))
> +		dev_dbg(host->dev, "ciu clock not available\n");
> +	else
> +		clk_prepare_enable(host->ciu_clk);
> +
> +	if (IS_ERR(host->ciu_clk))
> +		host->bus_hz = host->pdata->bus_hz;
> +	else
> +		host->bus_hz = clk_get_rate(host->ciu_clk);
if clk_get_rate() is incorrect value(ex,400MHz),
then mmc->f_min value is too high.
because mmc->f_min is assigned to DIV_ROUND_UP(host->bus_hz, 510) into dw_mc_init_slot.
Do you have any opinion for solving this?
> +
> +	if (!host->bus_hz) {
>  		dev_err(host->dev,
>  			"Platform data must supply bus speed\n");
> -		return -ENODEV;
> +		ret = -ENODEV;
> +		goto err_clk;
>  	}
>  
> -	host->bus_hz = host->pdata->bus_hz;
>  	host->quirks = host->pdata->quirks;
>  
>  	spin_lock_init(&host->lock);
>  	INIT_LIST_HEAD(&host->queue);
>  
> +	host->dma_ops = host->pdata->dma_ops;
> +	dw_mci_init_dma(host);
> +
>  	/*
>  	 * Get the host data width - this assumes that HCON has been set with
>  	 * the correct values.
> @@ -2116,6 +2136,16 @@ err_dmaunmap:
>  		regulator_disable(host->vmmc);
>  		regulator_put(host->vmmc);
>  	}
> +
> +err_clk:
> +	if (!IS_ERR(host->ciu_clk)) {
> +		clk_disable_unprepare(host->ciu_clk);
> +		clk_put(host->ciu_clk);
> +	}
> +	if (!IS_ERR(host->biu_clk)) {
> +		clk_disable_unprepare(host->biu_clk);
> +		clk_put(host->biu_clk);
> +	}
>  	return ret;
>  }
>  EXPORT_SYMBOL(dw_mci_probe);
> @@ -2149,6 +2179,12 @@ void dw_mci_remove(struct dw_mci *host)
>  		regulator_put(host->vmmc);
>  	}
>  
> +	if (!IS_ERR(host->ciu_clk))
> +		clk_disable_unprepare(host->ciu_clk);
> +	if (!IS_ERR(host->biu_clk))
> +		clk_disable_unprepare(host->biu_clk);
> +	clk_put(host->ciu_clk);
> +	clk_put(host->biu_clk);
>  }
>  EXPORT_SYMBOL(dw_mci_remove);
>  
> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
> index a37a573..787ad56 100644
> --- a/include/linux/mmc/dw_mmc.h
> +++ b/include/linux/mmc/dw_mmc.h
> @@ -78,6 +78,8 @@ struct mmc_data;
>   * @data_offset: Set the offset of DATA register according to VERID.
>   * @dev: Device associated with the MMC controller.
>   * @pdata: Platform data associated with the MMC controller.
> + * @biu_clk: Pointer to bus interface unit clock instance.
> + * @ciu_clk: Pointer to card interface unit clock instance.
>   * @slot: Slots sharing this MMC controller.
>   * @fifo_depth: depth of FIFO.
>   * @data_shift: log2 of FIFO item size.
> @@ -158,6 +160,8 @@ struct dw_mci {
>  	u16			data_offset;
>  	struct device		*dev;
>  	struct dw_mci_board	*pdata;
> +	struct clk		*biu_clk;
> +	struct clk		*ciu_clk;
>  	struct dw_mci_slot	*slot[MAX_MCI_SLOTS];
>  
>  	/* FIFO push and pull */
>
thomas.abraham@linaro.org Aug. 31, 2012, 5:29 a.m. UTC | #2
On 30 August 2012 14:18, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> Hi Thomas,
>
> On 08/29/2012 07:48 PM, Thomas Abraham wrote:
>> Some platforms allow for clock gating and control of bus interface unit clock
>> and card interface unit clock. Add support for clock lookup of optional biu
>> and ciu clocks for clock gating and clock speed determination.
>>
>> Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com>
>> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
>> Acked-by: Will Newton <will.newton@imgtec.com>
>> ---
>>  drivers/mmc/host/dw_mmc.c  |   42 +++++++++++++++++++++++++++++++++++++++---
>>  include/linux/mmc/dw_mmc.h |    4 ++++
>>  2 files changed, 43 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 227c42e..90c7c7b 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -1960,18 +1960,38 @@ int dw_mci_probe(struct dw_mci *host)
>>               return -ENODEV;
>>       }
>>
>> -     if (!host->pdata->bus_hz) {
>> +     host->biu_clk = clk_get(host->dev, "biu");
>> +     if (IS_ERR(host->biu_clk))
>> +             dev_dbg(host->dev, "biu clock not available\n");
>> +     else
>> +             clk_prepare_enable(host->biu_clk);
> biu is clock for bus interface?
> if didn't get "biu_clk" or didn't clk_prepare_enable(), then can we initialize the card?

Hi Jaehoon,

Yes, the biu clock is for bus interface. The biu and ciu clock lookup
and enable here is optional in the above change. If a platform does
not define these clocks, then the platform code is responsible for
enabling these clocks.

>> +
>> +     host->ciu_clk = clk_get(host->dev, "ciu");
>> +     if (IS_ERR(host->ciu_clk))
>> +             dev_dbg(host->dev, "ciu clock not available\n");
>> +     else
>> +             clk_prepare_enable(host->ciu_clk);
>> +
>> +     if (IS_ERR(host->ciu_clk))
>> +             host->bus_hz = host->pdata->bus_hz;
>> +     else
>> +             host->bus_hz = clk_get_rate(host->ciu_clk);
> if clk_get_rate() is incorrect value(ex,400MHz),
> then mmc->f_min value is too high.
> because mmc->f_min is assigned to DIV_ROUND_UP(host->bus_hz, 510) into dw_mc_init_slot.
> Do you have any opinion for solving this?

One option on Exynos5250 is to use the clock divider in the CLKSEL
register to divide the ciu clock to a lower value. For Exynos4, since
there is no clock divider in CLKSEL register, the platform code should
ensure that the ciu clock has a valid range.

Thanks,
Thomas.

[...]
Jaehoon Chung Aug. 31, 2012, 6:02 a.m. UTC | #3
On 08/31/2012 02:29 PM, Thomas Abraham wrote:
> On 30 August 2012 14:18, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>> Hi Thomas,
>>
>> On 08/29/2012 07:48 PM, Thomas Abraham wrote:
>>> Some platforms allow for clock gating and control of bus interface unit clock
>>> and card interface unit clock. Add support for clock lookup of optional biu
>>> and ciu clocks for clock gating and clock speed determination.
>>>
>>> Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com>
>>> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
>>> Acked-by: Will Newton <will.newton@imgtec.com>
>>> ---
>>>  drivers/mmc/host/dw_mmc.c  |   42 +++++++++++++++++++++++++++++++++++++++---
>>>  include/linux/mmc/dw_mmc.h |    4 ++++
>>>  2 files changed, 43 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>>> index 227c42e..90c7c7b 100644
>>> --- a/drivers/mmc/host/dw_mmc.c
>>> +++ b/drivers/mmc/host/dw_mmc.c
>>> @@ -1960,18 +1960,38 @@ int dw_mci_probe(struct dw_mci *host)
>>>               return -ENODEV;
>>>       }
>>>
>>> -     if (!host->pdata->bus_hz) {
>>> +     host->biu_clk = clk_get(host->dev, "biu");
>>> +     if (IS_ERR(host->biu_clk))
>>> +             dev_dbg(host->dev, "biu clock not available\n");
>>> +     else
>>> +             clk_prepare_enable(host->biu_clk);
>> biu is clock for bus interface?
>> if didn't get "biu_clk" or didn't clk_prepare_enable(), then can we initialize the card?
> 
> Hi Jaehoon,
> 
> Yes, the biu clock is for bus interface. The biu and ciu clock lookup
> and enable here is optional in the above change. If a platform does
> not define these clocks, then the platform code is responsible for
> enabling these clocks.
If biu_clk is presented...
Is there no probability that clk_prepare_enable is failed?
> 
>>> +
>>> +     host->ciu_clk = clk_get(host->dev, "ciu");
>>> +     if (IS_ERR(host->ciu_clk))
>>> +             dev_dbg(host->dev, "ciu clock not available\n");
>>> +     else
>>> +             clk_prepare_enable(host->ciu_clk);
>>> +
>>> +     if (IS_ERR(host->ciu_clk))
>>> +             host->bus_hz = host->pdata->bus_hz;
>>> +     else
>>> +             host->bus_hz = clk_get_rate(host->ciu_clk);
>> if clk_get_rate() is incorrect value(ex,400MHz),
>> then mmc->f_min value is too high.
>> because mmc->f_min is assigned to DIV_ROUND_UP(host->bus_hz, 510) into dw_mc_init_slot.
>> Do you have any opinion for solving this?
> 
> One option on Exynos5250 is to use the clock divider in the CLKSEL
> register to divide the ciu clock to a lower value. For Exynos4, since
> there is no clock divider in CLKSEL register, the platform code should
> ensure that the ciu clock has a valid range.
I know that can use div-ratio filed at the clksel register.
On Exynos5, i known that is used the div-ratio at CLKSEL register.
If ciu-clock is 400MHz, host->bus_hz is assigned to 400MHz.
1) host->bus_hz -> 400MHz (at dw-mmc-pltfm.c)
2) mmc->f_min = DIV_ROUND_UP(host->bus_hz, 510) then mmc->f_min is set to 784KHz.
3) then host->bus_hz is re-assigned to value that is divided to div-ratio at CLKSEL register.
   at this time, host->bus_hz = 100MHz...

I think this sequence is something wrong.
(Is 784KHz too high for init card?)

It's just my thinking..if my understanding is wrong, let me know plz.

Best Regards,
Jaehoon Chung

> 
> Thanks,
> Thomas.
> 
> [...]
> --
> 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
>
thomas.abraham@linaro.org Aug. 31, 2012, 6:32 a.m. UTC | #4
On 31 August 2012 11:32, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> On 08/31/2012 02:29 PM, Thomas Abraham wrote:
>> On 30 August 2012 14:18, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>> Hi Thomas,
>>>
>>> On 08/29/2012 07:48 PM, Thomas Abraham wrote:
>>>> Some platforms allow for clock gating and control of bus interface unit clock
>>>> and card interface unit clock. Add support for clock lookup of optional biu
>>>> and ciu clocks for clock gating and clock speed determination.
>>>>
>>>> Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com>
>>>> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
>>>> Acked-by: Will Newton <will.newton@imgtec.com>
>>>> ---
>>>>  drivers/mmc/host/dw_mmc.c  |   42 +++++++++++++++++++++++++++++++++++++++---
>>>>  include/linux/mmc/dw_mmc.h |    4 ++++
>>>>  2 files changed, 43 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>>>> index 227c42e..90c7c7b 100644
>>>> --- a/drivers/mmc/host/dw_mmc.c
>>>> +++ b/drivers/mmc/host/dw_mmc.c
>>>> @@ -1960,18 +1960,38 @@ int dw_mci_probe(struct dw_mci *host)
>>>>               return -ENODEV;
>>>>       }
>>>>
>>>> -     if (!host->pdata->bus_hz) {
>>>> +     host->biu_clk = clk_get(host->dev, "biu");
>>>> +     if (IS_ERR(host->biu_clk))
>>>> +             dev_dbg(host->dev, "biu clock not available\n");
>>>> +     else
>>>> +             clk_prepare_enable(host->biu_clk);
>>> biu is clock for bus interface?
>>> if didn't get "biu_clk" or didn't clk_prepare_enable(), then can we initialize the card?
>>
>> Hi Jaehoon,
>>
>> Yes, the biu clock is for bus interface. The biu and ciu clock lookup
>> and enable here is optional in the above change. If a platform does
>> not define these clocks, then the platform code is responsible for
>> enabling these clocks.
> If biu_clk is presented...
> Is there no probability that clk_prepare_enable is failed?

Yes, clk_prepare_enable can fail. I will fix this by setting biu_clk
and ciu_clk to NULL, in case the clk_prepare_enable fails.

>>
>>>> +
>>>> +     host->ciu_clk = clk_get(host->dev, "ciu");
>>>> +     if (IS_ERR(host->ciu_clk))
>>>> +             dev_dbg(host->dev, "ciu clock not available\n");
>>>> +     else
>>>> +             clk_prepare_enable(host->ciu_clk);
>>>> +
>>>> +     if (IS_ERR(host->ciu_clk))
>>>> +             host->bus_hz = host->pdata->bus_hz;
>>>> +     else
>>>> +             host->bus_hz = clk_get_rate(host->ciu_clk);
>>> if clk_get_rate() is incorrect value(ex,400MHz),
>>> then mmc->f_min value is too high.
>>> because mmc->f_min is assigned to DIV_ROUND_UP(host->bus_hz, 510) into dw_mc_init_slot.
>>> Do you have any opinion for solving this?
>>
>> One option on Exynos5250 is to use the clock divider in the CLKSEL
>> register to divide the ciu clock to a lower value. For Exynos4, since
>> there is no clock divider in CLKSEL register, the platform code should
>> ensure that the ciu clock has a valid range.
> I know that can use div-ratio filed at the clksel register.
> On Exynos5, i known that is used the div-ratio at CLKSEL register.
> If ciu-clock is 400MHz, host->bus_hz is assigned to 400MHz.
> 1) host->bus_hz -> 400MHz (at dw-mmc-pltfm.c)
> 2) mmc->f_min = DIV_ROUND_UP(host->bus_hz, 510) then mmc->f_min is set to 784KHz.
> 3) then host->bus_hz is re-assigned to value that is divided to div-ratio at CLKSEL register.
>    at this time, host->bus_hz = 100MHz...
>
> I think this sequence is something wrong.
> (Is 784KHz too high for init card?)
>
> It's just my thinking..if my understanding is wrong, let me know plz.

You have listed the steps 1 to 3 correctly. So, as per step 3, 100Mhz
/ 510 ~= 196KHz. Which is well within 400KHz. So do you still see a
problem here?

Thanks,
Thomas.
thomas.abraham@linaro.org Aug. 31, 2012, 6:32 a.m. UTC | #5
On 31 August 2012 11:32, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> On 08/31/2012 02:29 PM, Thomas Abraham wrote:
>> On 30 August 2012 14:18, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>> Hi Thomas,
>>>
>>> On 08/29/2012 07:48 PM, Thomas Abraham wrote:
>>>> Some platforms allow for clock gating and control of bus interface unit clock
>>>> and card interface unit clock. Add support for clock lookup of optional biu
>>>> and ciu clocks for clock gating and clock speed determination.
>>>>
>>>> Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com>
>>>> Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
>>>> Acked-by: Will Newton <will.newton@imgtec.com>
>>>> ---
>>>>  drivers/mmc/host/dw_mmc.c  |   42 +++++++++++++++++++++++++++++++++++++++---
>>>>  include/linux/mmc/dw_mmc.h |    4 ++++
>>>>  2 files changed, 43 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>>>> index 227c42e..90c7c7b 100644
>>>> --- a/drivers/mmc/host/dw_mmc.c
>>>> +++ b/drivers/mmc/host/dw_mmc.c
>>>> @@ -1960,18 +1960,38 @@ int dw_mci_probe(struct dw_mci *host)
>>>>               return -ENODEV;
>>>>       }
>>>>
>>>> -     if (!host->pdata->bus_hz) {
>>>> +     host->biu_clk = clk_get(host->dev, "biu");
>>>> +     if (IS_ERR(host->biu_clk))
>>>> +             dev_dbg(host->dev, "biu clock not available\n");
>>>> +     else
>>>> +             clk_prepare_enable(host->biu_clk);
>>> biu is clock for bus interface?
>>> if didn't get "biu_clk" or didn't clk_prepare_enable(), then can we initialize the card?
>>
>> Hi Jaehoon,
>>
>> Yes, the biu clock is for bus interface. The biu and ciu clock lookup
>> and enable here is optional in the above change. If a platform does
>> not define these clocks, then the platform code is responsible for
>> enabling these clocks.
> If biu_clk is presented...
> Is there no probability that clk_prepare_enable is failed?

Yes, clk_prepare_enable can fail. I will fix this by setting biu_clk
and ciu_clk to NULL, in case the clk_prepare_enable fails.

>>
>>>> +
>>>> +     host->ciu_clk = clk_get(host->dev, "ciu");
>>>> +     if (IS_ERR(host->ciu_clk))
>>>> +             dev_dbg(host->dev, "ciu clock not available\n");
>>>> +     else
>>>> +             clk_prepare_enable(host->ciu_clk);
>>>> +
>>>> +     if (IS_ERR(host->ciu_clk))
>>>> +             host->bus_hz = host->pdata->bus_hz;
>>>> +     else
>>>> +             host->bus_hz = clk_get_rate(host->ciu_clk);
>>> if clk_get_rate() is incorrect value(ex,400MHz),
>>> then mmc->f_min value is too high.
>>> because mmc->f_min is assigned to DIV_ROUND_UP(host->bus_hz, 510) into dw_mc_init_slot.
>>> Do you have any opinion for solving this?
>>
>> One option on Exynos5250 is to use the clock divider in the CLKSEL
>> register to divide the ciu clock to a lower value. For Exynos4, since
>> there is no clock divider in CLKSEL register, the platform code should
>> ensure that the ciu clock has a valid range.
> I know that can use div-ratio filed at the clksel register.
> On Exynos5, i known that is used the div-ratio at CLKSEL register.
> If ciu-clock is 400MHz, host->bus_hz is assigned to 400MHz.
> 1) host->bus_hz -> 400MHz (at dw-mmc-pltfm.c)
> 2) mmc->f_min = DIV_ROUND_UP(host->bus_hz, 510) then mmc->f_min is set to 784KHz.
> 3) then host->bus_hz is re-assigned to value that is divided to div-ratio at CLKSEL register.
>    at this time, host->bus_hz = 100MHz...
>
> I think this sequence is something wrong.
> (Is 784KHz too high for init card?)
>
> It's just my thinking..if my understanding is wrong, let me know plz.

You have listed the steps 1 to 3 correctly. So, as per step 3, 100Mhz
/ 510 ~= 196KHz. Which is well within 400KHz. So do you still see a
problem here?

Thanks,
Thomas.
Jaehoon Chung Aug. 31, 2012, 7:59 a.m. UTC | #6
>>>>> +
>>>>> +     host->ciu_clk = clk_get(host->dev, "ciu");
>>>>> +     if (IS_ERR(host->ciu_clk))
>>>>> +             dev_dbg(host->dev, "ciu clock not available\n");
>>>>> +     else
>>>>> +             clk_prepare_enable(host->ciu_clk);
>>>>> +
>>>>> +     if (IS_ERR(host->ciu_clk))
>>>>> +             host->bus_hz = host->pdata->bus_hz;
>>>>> +     else
>>>>> +             host->bus_hz = clk_get_rate(host->ciu_clk);
>>>> if clk_get_rate() is incorrect value(ex,400MHz),
>>>> then mmc->f_min value is too high.
>>>> because mmc->f_min is assigned to DIV_ROUND_UP(host->bus_hz, 510) into dw_mc_init_slot.
>>>> Do you have any opinion for solving this?
>>>
>>> One option on Exynos5250 is to use the clock divider in the CLKSEL
>>> register to divide the ciu clock to a lower value. For Exynos4, since
>>> there is no clock divider in CLKSEL register, the platform code should
>>> ensure that the ciu clock has a valid range.
>> I know that can use div-ratio filed at the clksel register.
>> On Exynos5, i known that is used the div-ratio at CLKSEL register.
>> If ciu-clock is 400MHz, host->bus_hz is assigned to 400MHz.
>> 1) host->bus_hz -> 400MHz (at dw-mmc-pltfm.c)
>> 2) mmc->f_min = DIV_ROUND_UP(host->bus_hz, 510) then mmc->f_min is set to 784KHz.
>> 3) then host->bus_hz is re-assigned to value that is divided to div-ratio at CLKSEL register.
>>    at this time, host->bus_hz = 100MHz...
>>
>> I think this sequence is something wrong.
>> (Is 784KHz too high for init card?)
>>
>> It's just my thinking..if my understanding is wrong, let me know plz.
> 
> You have listed the steps 1 to 3 correctly. So, as per step 3, 100Mhz
> / 510 ~= 196KHz. Which is well within 400KHz. So do you still see a
> problem here?
How do you think about this? 
[ 4.620000] mmc_host mmc0: Bus speed (slot 0) = 100000000Hz (slot req 784314Hz, actual 781250HZ div = 64)

Best Regards,
Jaehoon Chung
> 
> Thanks,
> Thomas.
> --
> 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
>
thomas.abraham@linaro.org Aug. 31, 2012, 8:48 a.m. UTC | #7
On 31 August 2012 13:29, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>>>>> +
>>>>>> +     host->ciu_clk = clk_get(host->dev, "ciu");
>>>>>> +     if (IS_ERR(host->ciu_clk))
>>>>>> +             dev_dbg(host->dev, "ciu clock not available\n");
>>>>>> +     else
>>>>>> +             clk_prepare_enable(host->ciu_clk);
>>>>>> +
>>>>>> +     if (IS_ERR(host->ciu_clk))
>>>>>> +             host->bus_hz = host->pdata->bus_hz;
>>>>>> +     else
>>>>>> +             host->bus_hz = clk_get_rate(host->ciu_clk);
>>>>> if clk_get_rate() is incorrect value(ex,400MHz),
>>>>> then mmc->f_min value is too high.
>>>>> because mmc->f_min is assigned to DIV_ROUND_UP(host->bus_hz, 510) into dw_mc_init_slot.
>>>>> Do you have any opinion for solving this?
>>>>
>>>> One option on Exynos5250 is to use the clock divider in the CLKSEL
>>>> register to divide the ciu clock to a lower value. For Exynos4, since
>>>> there is no clock divider in CLKSEL register, the platform code should
>>>> ensure that the ciu clock has a valid range.
>>> I know that can use div-ratio filed at the clksel register.
>>> On Exynos5, i known that is used the div-ratio at CLKSEL register.
>>> If ciu-clock is 400MHz, host->bus_hz is assigned to 400MHz.
>>> 1) host->bus_hz -> 400MHz (at dw-mmc-pltfm.c)
>>> 2) mmc->f_min = DIV_ROUND_UP(host->bus_hz, 510) then mmc->f_min is set to 784KHz.
>>> 3) then host->bus_hz is re-assigned to value that is divided to div-ratio at CLKSEL register.
>>>    at this time, host->bus_hz = 100MHz...
>>>
>>> I think this sequence is something wrong.
>>> (Is 784KHz too high for init card?)
>>>
>>> It's just my thinking..if my understanding is wrong, let me know plz.
>>
>> You have listed the steps 1 to 3 correctly. So, as per step 3, 100Mhz
>> / 510 ~= 196KHz. Which is well within 400KHz. So do you still see a
>> problem here?
> How do you think about this?
> [ 4.620000] mmc_host mmc0: Bus speed (slot 0) = 100000000Hz (slot req 784314Hz, actual 781250HZ div = 64)

You are right, the sequence is wrong. The host->bus_hz is set to the
correct value of 100Mhz in set_ios callback of exynos. But much prior
to that, the dw_mci_init_slot is called and that initializes
mmc->f_min to host->bus_hz / 510 = 400Mhz / 510 = 768KHz.

To fix this, I am planning to add another implementation specific
callback function that will adjust the host->bus_clk as per
implementation specific extensions. This callback will be called right
from dw_mci_probe right after the host->bus_hz is set using the
clk_get_rate() call.

Sorry, I was not paying proper attention to the log messages. Thanks
for your correction.

Regards,
Thomas.
diff mbox

Patch

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 227c42e..90c7c7b 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1960,18 +1960,38 @@  int dw_mci_probe(struct dw_mci *host)
 		return -ENODEV;
 	}
 
-	if (!host->pdata->bus_hz) {
+	host->biu_clk = clk_get(host->dev, "biu");
+	if (IS_ERR(host->biu_clk))
+		dev_dbg(host->dev, "biu clock not available\n");
+	else
+		clk_prepare_enable(host->biu_clk);
+
+	host->ciu_clk = clk_get(host->dev, "ciu");
+	if (IS_ERR(host->ciu_clk))
+		dev_dbg(host->dev, "ciu clock not available\n");
+	else
+		clk_prepare_enable(host->ciu_clk);
+
+	if (IS_ERR(host->ciu_clk))
+		host->bus_hz = host->pdata->bus_hz;
+	else
+		host->bus_hz = clk_get_rate(host->ciu_clk);
+
+	if (!host->bus_hz) {
 		dev_err(host->dev,
 			"Platform data must supply bus speed\n");
-		return -ENODEV;
+		ret = -ENODEV;
+		goto err_clk;
 	}
 
-	host->bus_hz = host->pdata->bus_hz;
 	host->quirks = host->pdata->quirks;
 
 	spin_lock_init(&host->lock);
 	INIT_LIST_HEAD(&host->queue);
 
+	host->dma_ops = host->pdata->dma_ops;
+	dw_mci_init_dma(host);
+
 	/*
 	 * Get the host data width - this assumes that HCON has been set with
 	 * the correct values.
@@ -2116,6 +2136,16 @@  err_dmaunmap:
 		regulator_disable(host->vmmc);
 		regulator_put(host->vmmc);
 	}
+
+err_clk:
+	if (!IS_ERR(host->ciu_clk)) {
+		clk_disable_unprepare(host->ciu_clk);
+		clk_put(host->ciu_clk);
+	}
+	if (!IS_ERR(host->biu_clk)) {
+		clk_disable_unprepare(host->biu_clk);
+		clk_put(host->biu_clk);
+	}
 	return ret;
 }
 EXPORT_SYMBOL(dw_mci_probe);
@@ -2149,6 +2179,12 @@  void dw_mci_remove(struct dw_mci *host)
 		regulator_put(host->vmmc);
 	}
 
+	if (!IS_ERR(host->ciu_clk))
+		clk_disable_unprepare(host->ciu_clk);
+	if (!IS_ERR(host->biu_clk))
+		clk_disable_unprepare(host->biu_clk);
+	clk_put(host->ciu_clk);
+	clk_put(host->biu_clk);
 }
 EXPORT_SYMBOL(dw_mci_remove);
 
diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
index a37a573..787ad56 100644
--- a/include/linux/mmc/dw_mmc.h
+++ b/include/linux/mmc/dw_mmc.h
@@ -78,6 +78,8 @@  struct mmc_data;
  * @data_offset: Set the offset of DATA register according to VERID.
  * @dev: Device associated with the MMC controller.
  * @pdata: Platform data associated with the MMC controller.
+ * @biu_clk: Pointer to bus interface unit clock instance.
+ * @ciu_clk: Pointer to card interface unit clock instance.
  * @slot: Slots sharing this MMC controller.
  * @fifo_depth: depth of FIFO.
  * @data_shift: log2 of FIFO item size.
@@ -158,6 +160,8 @@  struct dw_mci {
 	u16			data_offset;
 	struct device		*dev;
 	struct dw_mci_board	*pdata;
+	struct clk		*biu_clk;
+	struct clk		*ciu_clk;
 	struct dw_mci_slot	*slot[MAX_MCI_SLOTS];
 
 	/* FIFO push and pull */