| Message ID | 1401284878-16937-1-git-send-email-srinivas.kandagatla@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 28 May 2014 15:47, <srinivas.kandagatla@linaro.org> wrote: > From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > > On Controllers like Qcom SD card controller where cclk is mclk and mclk should > be directly controlled by the driver. > > This patch adds support to control mclk directly in the driver, and also > adds explicit_mclk_control flag in variant structure giving more flexibility > to the driver. > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > --- > drivers/mmc/host/mmci.c | 96 ++++++++++++++++++++++++++++++++----------------- > drivers/mmc/host/mmci.h | 2 ++ > 2 files changed, 65 insertions(+), 33 deletions(-) > > diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c > index 202f2d5..6eb0a29 100644 > --- a/drivers/mmc/host/mmci.c > +++ b/drivers/mmc/host/mmci.c > @@ -72,6 +72,7 @@ static unsigned int fmax = 515633; > * @pwrreg_clkgate: MMCIPOWER register must be used to gate the clock > * @busy_detect: true if busy detection on dat0 is supported > * @pwrreg_nopower: bits in MMCIPOWER don't controls ext. power supply > + * @explicit_mclk_control: enable explicit mclk control in driver. > */ > struct variant_data { > unsigned int clkreg; > @@ -93,6 +94,7 @@ struct variant_data { > bool pwrreg_clkgate; > bool busy_detect; > bool pwrreg_nopower; > + bool explicit_mclk_control; > }; > > static struct variant_data variant_arm = { > @@ -199,6 +201,7 @@ static struct variant_data variant_qcom = { > .datalength_bits = 24, > .pwrreg_powerup = MCI_PWR_UP, > .f_max = 208000000, > + .explicit_mclk_control = true, This should go in patch3 instead. > }; > > static int mmci_card_busy(struct mmc_host *mmc) > @@ -301,7 +304,9 @@ static void mmci_set_clkreg(struct mmci_host *host, unsigned int desired) > host->cclk = 0; > > if (desired) { > - if (desired >= host->mclk) { > + if (variant->explicit_mclk_control) { > + host->cclk = host->mclk; > + } else if (desired >= host->mclk) { > clk = MCI_CLK_BYPASS; > if (variant->st_clkdiv) > clk |= MCI_ST_UX500_NEG_EDGE; > @@ -1340,6 +1345,18 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) > if (!ios->clock && variant->pwrreg_clkgate) > pwr &= ~MCI_PWR_ON; > > + if ((host->variant->explicit_mclk_control) && > + (ios->clock != host->mclk_req)) { > + int rc = clk_set_rate(host->clk, ios->clock); > + if (rc < 0) { > + dev_err(mmc_dev(host->mmc), > + "Error setting clock rate (%d)\n", rc); > + } else { > + host->mclk = clk_get_rate(host->clk); > + host->mclk_req = ios->clock; > + } > + } > + > spin_lock_irqsave(&host->lock, flags); > > mmci_set_clkreg(host, ios->clock); > @@ -1490,19 +1507,6 @@ static int mmci_probe(struct amba_device *dev, > host->plat = plat; > host->variant = variant; > host->mclk = clk_get_rate(host->clk); > - /* > - * According to the spec, mclk is max 100 MHz, > - * so we try to adjust the clock down to this, > - * (if possible). > - */ > - if (host->mclk > host->variant->f_max) { > - ret = clk_set_rate(host->clk, host->variant->f_max); > - if (ret < 0) > - goto clk_disable; > - host->mclk = clk_get_rate(host->clk); > - dev_dbg(mmc_dev(mmc), "eventual mclk rate: %u Hz\n", > - host->mclk); > - } The above code is relevant for Qcom as well, I don't think you need change/move it. > > host->phybase = dev->res.start; > host->base = devm_ioremap_resource(&dev->dev, &dev->res); > @@ -1511,25 +1515,51 @@ static int mmci_probe(struct amba_device *dev, > goto clk_disable; > } > > - /* > - * The ARM and ST versions of the block have slightly different > - * clock divider equations which means that the minimum divider > - * differs too. > - */ > - if (variant->st_clkdiv) > - mmc->f_min = DIV_ROUND_UP(host->mclk, 257); I think you could simplify the change of fixing with f_min and f_max. Start by, just add another statement here "else if (variant->explicit_mclk_control)" and do the clk_round_rate() > - else > - mmc->f_min = DIV_ROUND_UP(host->mclk, 512); > - /* > - * If no maximum operating frequency is supplied, fall back to use > - * the module parameter, which has a (low) default value in case it > - * is not specified. Either value must not exceed the clock rate into > - * the block, of course. > - */ > - if (mmc->f_max) > - mmc->f_max = min(host->mclk, mmc->f_max); > - else > - mmc->f_max = min(host->mclk, fmax); Further replace the above with: if (mmc->f_max) mmc->f_max = variant->explicit_mclk_control ? min(mmc->f_max, variant->f_max) : min(host->mclk, mmc->f_max); else mmc->f_max = variant->explicit_mclk_control ? fmax : min(host->mclk, fmax); That should do it, right? > + if (variant->explicit_mclk_control) { > + /* get the nearest minimum clock to 100Khz */ > + mmc->f_min = clk_round_rate(host->clk, 100000); > + > + if (mmc->f_max) > + mmc->f_max = min(host->variant->f_max, mmc->f_max); > + else > + mmc->f_max = min(host->variant->f_max, fmax); > + > + } else { > + /* > + * According to the spec, mclk is max 100 MHz, > + * so we try to adjust the clock down to this, > + * (if possible). > + */ > + if (host->mclk > host->variant->f_max) { > + ret = clk_set_rate(host->clk, host->variant->f_max); > + if (ret < 0) > + goto clk_disable; > + host->mclk = clk_get_rate(host->clk); > + dev_dbg(mmc_dev(mmc), "eventual mclk rate: %u Hz\n", > + host->mclk); > + } > + /* > + * The ARM and ST versions of the block have slightly different > + * clock divider equations which means that the minimum divider > + * differs too. > + */ > + if (variant->st_clkdiv) > + mmc->f_min = DIV_ROUND_UP(host->mclk, 257); > + else > + mmc->f_min = DIV_ROUND_UP(host->mclk, 512); > + /* > + * If no maximum operating frequency is supplied, fall back to > + * use the module parameter, which has a (low) default value in > + * case it is not specified. Either value must not exceed the > + * clock rate into the block, of course. > + */ > + if (mmc->f_max) > + mmc->f_max = min(host->mclk, mmc->f_max); > + else > + mmc->f_max = min(host->mclk, fmax); > + > + } > + > dev_dbg(mmc_dev(mmc), "clocking block at %u Hz\n", mmc->f_max); > > /* Get regulators and the supported OCR mask */ > diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h > index 706eb513..1882e20 100644 > --- a/drivers/mmc/host/mmci.h > +++ b/drivers/mmc/host/mmci.h > @@ -208,6 +208,8 @@ struct mmci_host { > spinlock_t lock; > > unsigned int mclk; > + /* cached value of requested clk in set_ios */ > + unsigned int mclk_req; How about "clock_cache" instead? > unsigned int cclk; > u32 pwr_reg; > u32 pwr_reg_add; > -- > 1.9.1 > Kind regards Ulf Hansson -- 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
Thanks Ulf for reviewing. On 30/05/14 11:25, Ulf Hansson wrote: > On 28 May 2014 15:47, <srinivas.kandagatla@linaro.org> wrote: >> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >> >> On Controllers like Qcom SD card controller where cclk is mclk and mclk should >> be directly controlled by the driver. >> >> This patch adds support to control mclk directly in the driver, and also >> adds explicit_mclk_control flag in variant structure giving more flexibility >> to the driver. >> >> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >> --- >> drivers/mmc/host/mmci.c | 96 ++++++++++++++++++++++++++++++++----------------- >> drivers/mmc/host/mmci.h | 2 ++ >> 2 files changed, 65 insertions(+), 33 deletions(-) >> >> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c >> index 202f2d5..6eb0a29 100644 >> --- a/drivers/mmc/host/mmci.c >> +++ b/drivers/mmc/host/mmci.c >> @@ -72,6 +72,7 @@ static unsigned int fmax = 515633; >> * @pwrreg_clkgate: MMCIPOWER register must be used to gate the clock >> * @busy_detect: true if busy detection on dat0 is supported >> * @pwrreg_nopower: bits in MMCIPOWER don't controls ext. power supply >> + * @explicit_mclk_control: enable explicit mclk control in driver. >> */ >> struct variant_data { >> unsigned int clkreg; >> @@ -93,6 +94,7 @@ struct variant_data { >> bool pwrreg_clkgate; >> bool busy_detect; >> bool pwrreg_nopower; >> + bool explicit_mclk_control; >> }; >> >> static struct variant_data variant_arm = { >> @@ -199,6 +201,7 @@ static struct variant_data variant_qcom = { >> .datalength_bits = 24, >> .pwrreg_powerup = MCI_PWR_UP, >> .f_max = 208000000, >> + .explicit_mclk_control = true, > > This should go in patch3 instead. yes.. will get this in to the patch3 too. > >> }; >> >> static int mmci_card_busy(struct mmc_host *mmc) >> @@ -301,7 +304,9 @@ static void mmci_set_clkreg(struct mmci_host *host, unsigned int desired) >> host->cclk = 0; >> >> if (desired) { >> - if (desired >= host->mclk) { >> + if (variant->explicit_mclk_control) { >> + host->cclk = host->mclk; >> + } else if (desired >= host->mclk) { >> clk = MCI_CLK_BYPASS; >> if (variant->st_clkdiv) >> clk |= MCI_ST_UX500_NEG_EDGE; >> @@ -1340,6 +1345,18 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) >> if (!ios->clock && variant->pwrreg_clkgate) >> pwr &= ~MCI_PWR_ON; >> >> + if ((host->variant->explicit_mclk_control) && >> + (ios->clock != host->mclk_req)) { >> + int rc = clk_set_rate(host->clk, ios->clock); >> + if (rc < 0) { >> + dev_err(mmc_dev(host->mmc), >> + "Error setting clock rate (%d)\n", rc); >> + } else { >> + host->mclk = clk_get_rate(host->clk); >> + host->mclk_req = ios->clock; >> + } >> + } >> + >> spin_lock_irqsave(&host->lock, flags); >> >> mmci_set_clkreg(host, ios->clock); >> @@ -1490,19 +1507,6 @@ static int mmci_probe(struct amba_device *dev, >> host->plat = plat; >> host->variant = variant; >> host->mclk = clk_get_rate(host->clk); >> - /* >> - * According to the spec, mclk is max 100 MHz, >> - * so we try to adjust the clock down to this, >> - * (if possible). >> - */ >> - if (host->mclk > host->variant->f_max) { >> - ret = clk_set_rate(host->clk, host->variant->f_max); >> - if (ret < 0) >> - goto clk_disable; >> - host->mclk = clk_get_rate(host->clk); >> - dev_dbg(mmc_dev(mmc), "eventual mclk rate: %u Hz\n", >> - host->mclk); >> - } > > The above code is relevant for Qcom as well, I don't think you need > change/move it. Yes, you are right. I will fix it in next version. > >> >> host->phybase = dev->res.start; >> host->base = devm_ioremap_resource(&dev->dev, &dev->res); >> @@ -1511,25 +1515,51 @@ static int mmci_probe(struct amba_device *dev, >> goto clk_disable; >> } >> >> - /* >> - * The ARM and ST versions of the block have slightly different >> - * clock divider equations which means that the minimum divider >> - * differs too. >> - */ >> - if (variant->st_clkdiv) >> - mmc->f_min = DIV_ROUND_UP(host->mclk, 257); > > I think you could simplify the change of fixing with f_min and f_max. > > Start by, just add another statement here "else if > (variant->explicit_mclk_control)" and do the clk_round_rate() > >> - else >> - mmc->f_min = DIV_ROUND_UP(host->mclk, 512); >> - /* >> - * If no maximum operating frequency is supplied, fall back to use >> - * the module parameter, which has a (low) default value in case it >> - * is not specified. Either value must not exceed the clock rate into >> - * the block, of course. >> - */ >> - if (mmc->f_max) >> - mmc->f_max = min(host->mclk, mmc->f_max); >> - else >> - mmc->f_max = min(host->mclk, fmax); > > Further replace the above with: > if (mmc->f_max) > mmc->f_max = variant->explicit_mclk_control ? min(mmc->f_max, > variant->f_max) : min(host->mclk, mmc->f_max); > else > mmc->f_max = variant->explicit_mclk_control ? fmax : min(host->mclk, fmax); > > That should do it, right? yes, that will work. It simplifies the logic too. > >> + if (variant->explicit_mclk_control) { >> + /* get the nearest minimum clock to 100Khz */ >> + mmc->f_min = clk_round_rate(host->clk, 100000); >> + >> + if (mmc->f_max) >> + mmc->f_max = min(host->variant->f_max, mmc->f_max); >> + else >> + mmc->f_max = min(host->variant->f_max, fmax); >> + >> + } else { >> + /* >> + * According to the spec, mclk is max 100 MHz, >> + * so we try to adjust the clock down to this, >> + * (if possible). >> + */ >> + if (host->mclk > host->variant->f_max) { >> + ret = clk_set_rate(host->clk, host->variant->f_max); >> + if (ret < 0) >> + goto clk_disable; >> + host->mclk = clk_get_rate(host->clk); >> + dev_dbg(mmc_dev(mmc), "eventual mclk rate: %u Hz\n", >> + host->mclk); >> + } >> + /* >> + * The ARM and ST versions of the block have slightly different >> + * clock divider equations which means that the minimum divider >> + * differs too. >> + */ >> + if (variant->st_clkdiv) >> + mmc->f_min = DIV_ROUND_UP(host->mclk, 257); >> + else >> + mmc->f_min = DIV_ROUND_UP(host->mclk, 512); >> + /* >> + * If no maximum operating frequency is supplied, fall back to >> + * use the module parameter, which has a (low) default value in >> + * case it is not specified. Either value must not exceed the >> + * clock rate into the block, of course. >> + */ >> + if (mmc->f_max) >> + mmc->f_max = min(host->mclk, mmc->f_max); >> + else >> + mmc->f_max = min(host->mclk, fmax); >> + >> + } >> + >> dev_dbg(mmc_dev(mmc), "clocking block at %u Hz\n", mmc->f_max); >> >> /* Get regulators and the supported OCR mask */ >> diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h >> index 706eb513..1882e20 100644 >> --- a/drivers/mmc/host/mmci.h >> +++ b/drivers/mmc/host/mmci.h >> @@ -208,6 +208,8 @@ struct mmci_host { >> spinlock_t lock; >> >> unsigned int mclk; >> + /* cached value of requested clk in set_ios */ >> + unsigned int mclk_req; > > How about "clock_cache" instead? Sounds ok to me, I will change it in next version. Thanks, srini > >> unsigned int cclk; >> u32 pwr_reg; >> u32 pwr_reg_add; >> -- >> 1.9.1 >> > > Kind regards > Ulf Hansson > -- 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/mmci.c b/drivers/mmc/host/mmci.c index 202f2d5..6eb0a29 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -72,6 +72,7 @@ static unsigned int fmax = 515633; * @pwrreg_clkgate: MMCIPOWER register must be used to gate the clock * @busy_detect: true if busy detection on dat0 is supported * @pwrreg_nopower: bits in MMCIPOWER don't controls ext. power supply + * @explicit_mclk_control: enable explicit mclk control in driver. */ struct variant_data { unsigned int clkreg; @@ -93,6 +94,7 @@ struct variant_data { bool pwrreg_clkgate; bool busy_detect; bool pwrreg_nopower; + bool explicit_mclk_control; }; static struct variant_data variant_arm = { @@ -199,6 +201,7 @@ static struct variant_data variant_qcom = { .datalength_bits = 24, .pwrreg_powerup = MCI_PWR_UP, .f_max = 208000000, + .explicit_mclk_control = true, }; static int mmci_card_busy(struct mmc_host *mmc) @@ -301,7 +304,9 @@ static void mmci_set_clkreg(struct mmci_host *host, unsigned int desired) host->cclk = 0; if (desired) { - if (desired >= host->mclk) { + if (variant->explicit_mclk_control) { + host->cclk = host->mclk; + } else if (desired >= host->mclk) { clk = MCI_CLK_BYPASS; if (variant->st_clkdiv) clk |= MCI_ST_UX500_NEG_EDGE; @@ -1340,6 +1345,18 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) if (!ios->clock && variant->pwrreg_clkgate) pwr &= ~MCI_PWR_ON; + if ((host->variant->explicit_mclk_control) && + (ios->clock != host->mclk_req)) { + int rc = clk_set_rate(host->clk, ios->clock); + if (rc < 0) { + dev_err(mmc_dev(host->mmc), + "Error setting clock rate (%d)\n", rc); + } else { + host->mclk = clk_get_rate(host->clk); + host->mclk_req = ios->clock; + } + } + spin_lock_irqsave(&host->lock, flags); mmci_set_clkreg(host, ios->clock); @@ -1490,19 +1507,6 @@ static int mmci_probe(struct amba_device *dev, host->plat = plat; host->variant = variant; host->mclk = clk_get_rate(host->clk); - /* - * According to the spec, mclk is max 100 MHz, - * so we try to adjust the clock down to this, - * (if possible). - */ - if (host->mclk > host->variant->f_max) { - ret = clk_set_rate(host->clk, host->variant->f_max); - if (ret < 0) - goto clk_disable; - host->mclk = clk_get_rate(host->clk); - dev_dbg(mmc_dev(mmc), "eventual mclk rate: %u Hz\n", - host->mclk); - } host->phybase = dev->res.start; host->base = devm_ioremap_resource(&dev->dev, &dev->res); @@ -1511,25 +1515,51 @@ static int mmci_probe(struct amba_device *dev, goto clk_disable; } - /* - * The ARM and ST versions of the block have slightly different - * clock divider equations which means that the minimum divider - * differs too. - */ - if (variant->st_clkdiv) - mmc->f_min = DIV_ROUND_UP(host->mclk, 257); - else - mmc->f_min = DIV_ROUND_UP(host->mclk, 512); - /* - * If no maximum operating frequency is supplied, fall back to use - * the module parameter, which has a (low) default value in case it - * is not specified. Either value must not exceed the clock rate into - * the block, of course. - */ - if (mmc->f_max) - mmc->f_max = min(host->mclk, mmc->f_max); - else - mmc->f_max = min(host->mclk, fmax); + if (variant->explicit_mclk_control) { + /* get the nearest minimum clock to 100Khz */ + mmc->f_min = clk_round_rate(host->clk, 100000); + + if (mmc->f_max) + mmc->f_max = min(host->variant->f_max, mmc->f_max); + else + mmc->f_max = min(host->variant->f_max, fmax); + + } else { + /* + * According to the spec, mclk is max 100 MHz, + * so we try to adjust the clock down to this, + * (if possible). + */ + if (host->mclk > host->variant->f_max) { + ret = clk_set_rate(host->clk, host->variant->f_max); + if (ret < 0) + goto clk_disable; + host->mclk = clk_get_rate(host->clk); + dev_dbg(mmc_dev(mmc), "eventual mclk rate: %u Hz\n", + host->mclk); + } + /* + * The ARM and ST versions of the block have slightly different + * clock divider equations which means that the minimum divider + * differs too. + */ + if (variant->st_clkdiv) + mmc->f_min = DIV_ROUND_UP(host->mclk, 257); + else + mmc->f_min = DIV_ROUND_UP(host->mclk, 512); + /* + * If no maximum operating frequency is supplied, fall back to + * use the module parameter, which has a (low) default value in + * case it is not specified. Either value must not exceed the + * clock rate into the block, of course. + */ + if (mmc->f_max) + mmc->f_max = min(host->mclk, mmc->f_max); + else + mmc->f_max = min(host->mclk, fmax); + + } + dev_dbg(mmc_dev(mmc), "clocking block at %u Hz\n", mmc->f_max); /* Get regulators and the supported OCR mask */ diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h index 706eb513..1882e20 100644 --- a/drivers/mmc/host/mmci.h +++ b/drivers/mmc/host/mmci.h @@ -208,6 +208,8 @@ struct mmci_host { spinlock_t lock; unsigned int mclk; + /* cached value of requested clk in set_ios */ + unsigned int mclk_req; unsigned int cclk; u32 pwr_reg; u32 pwr_reg_add;