| Message ID | 1346237295-7116-5-git-send-email-thomas.abraham@linaro.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
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 */ > -- 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 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. [...] -- 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 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 > -- 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 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. -- 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 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. -- 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
>>>>> + >>>>> + 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 > -- 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 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. -- 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
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 */