Linux-mmc Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] mmc: sdhci: fix minimum clock rate for v3 controller
@ 2020-01-02 10:51 Michał Mirosław
  2020-01-14 13:08 ` Adrian Hunter
  0 siblings, 1 reply; 4+ messages in thread
From: Michał Mirosław @ 2020-01-02 10:51 UTC (permalink / raw)
  To: linux-mmc; +Cc: Adrian Hunter, Ulf Hansson, linux-kernel

For SDHCIv3+ with programmable clock mode, minimal clock frequency is
still base clock / max(divider). Minimal programmable clock frequency is
always greater than minimal divided clock frequency. Without this patch,
SDHCI uses out-of-spec initial frequency when multiplier is big enough:

mmc1: mmc_rescan_try_freq: trying to init card at 468750 Hz
[for 480 MHz source clock divided by 1024]

Cc: stable@vger.kernel.org
Fixes: c3ed3877625f ("mmc: sdhci: add support for programmable clock mode")
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/mmc/host/sdhci.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 275102c0a1bf..0036ddf85674 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -3902,11 +3902,9 @@ int sdhci_setup_host(struct sdhci_host *host)
 	if (host->ops->get_min_clock)
 		mmc->f_min = host->ops->get_min_clock(host);
 	else if (host->version >= SDHCI_SPEC_300) {
-		if (host->clk_mul) {
-			mmc->f_min = (host->max_clk * host->clk_mul) / 1024;
+		if (host->clk_mul)
 			max_clk = host->max_clk * host->clk_mul;
-		} else
-			mmc->f_min = host->max_clk / SDHCI_MAX_DIV_SPEC_300;
+		mmc->f_min = host->max_clk / SDHCI_MAX_DIV_SPEC_300;
 	} else
 		mmc->f_min = host->max_clk / SDHCI_MAX_DIV_SPEC_200;
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] mmc: sdhci: fix minimum clock rate for v3 controller
  2020-01-02 10:51 [PATCH] mmc: sdhci: fix minimum clock rate for v3 controller Michał Mirosław
@ 2020-01-14 13:08 ` Adrian Hunter
  2020-01-14 15:53   ` Michał Mirosław
  0 siblings, 1 reply; 4+ messages in thread
From: Adrian Hunter @ 2020-01-14 13:08 UTC (permalink / raw)
  To: Michał Mirosław, linux-mmc; +Cc: Ulf Hansson, linux-kernel

On 2/01/20 12:51 pm, Michał Mirosław wrote:
> For SDHCIv3+ with programmable clock mode, minimal clock frequency is
> still base clock / max(divider). Minimal programmable clock frequency is
> always greater than minimal divided clock frequency. Without this patch,
> SDHCI uses out-of-spec initial frequency when multiplier is big enough:
> 
> mmc1: mmc_rescan_try_freq: trying to init card at 468750 Hz
> [for 480 MHz source clock divided by 1024]

The maximum divisor in programmable clock mode is 1024.  So I do not
understand what is wrong.  Can you explain some more?

> 
> Cc: stable@vger.kernel.org
> Fixes: c3ed3877625f ("mmc: sdhci: add support for programmable clock mode")
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---
>  drivers/mmc/host/sdhci.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 275102c0a1bf..0036ddf85674 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -3902,11 +3902,9 @@ int sdhci_setup_host(struct sdhci_host *host)
>  	if (host->ops->get_min_clock)
>  		mmc->f_min = host->ops->get_min_clock(host);
>  	else if (host->version >= SDHCI_SPEC_300) {
> -		if (host->clk_mul) {
> -			mmc->f_min = (host->max_clk * host->clk_mul) / 1024;
> +		if (host->clk_mul)
>  			max_clk = host->max_clk * host->clk_mul;
> -		} else
> -			mmc->f_min = host->max_clk / SDHCI_MAX_DIV_SPEC_300;
> +		mmc->f_min = host->max_clk / SDHCI_MAX_DIV_SPEC_300;
>  	} else
>  		mmc->f_min = host->max_clk / SDHCI_MAX_DIV_SPEC_200;
>  
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] mmc: sdhci: fix minimum clock rate for v3 controller
  2020-01-14 13:08 ` Adrian Hunter
@ 2020-01-14 15:53   ` Michał Mirosław
  2020-01-15  7:16     ` Adrian Hunter
  0 siblings, 1 reply; 4+ messages in thread
From: Michał Mirosław @ 2020-01-14 15:53 UTC (permalink / raw)
  To: Adrian Hunter; +Cc: linux-mmc, Ulf Hansson, linux-kernel

On Tue, Jan 14, 2020 at 03:08:08PM +0200, Adrian Hunter wrote:
> On 2/01/20 12:51 pm, Michał Mirosław wrote:
> > For SDHCIv3+ with programmable clock mode, minimal clock frequency is
> > still base clock / max(divider). Minimal programmable clock frequency is
> > always greater than minimal divided clock frequency. Without this patch,
> > SDHCI uses out-of-spec initial frequency when multiplier is big enough:
> > 
> > mmc1: mmc_rescan_try_freq: trying to init card at 468750 Hz
> > [for 480 MHz source clock divided by 1024]
> 
> The maximum divisor in programmable clock mode is 1024.  So I do not
> understand what is wrong.  Can you explain some more?

This part of code misses the fact, that you can choose (switch) between
base clock mode and programmable clock mode. The code in
sdhci_calc_clk() already does the choosing part. This is actually
required on high programmable clock base to get conformant frequency for
the card initialization phase.

Best Regards,
Michał Mirosław

[...]
> > index 275102c0a1bf..0036ddf85674 100644
> > --- a/drivers/mmc/host/sdhci.c
> > +++ b/drivers/mmc/host/sdhci.c
> > @@ -3902,11 +3902,9 @@ int sdhci_setup_host(struct sdhci_host *host)
> >  	if (host->ops->get_min_clock)
> >  		mmc->f_min = host->ops->get_min_clock(host);
> >  	else if (host->version >= SDHCI_SPEC_300) {
> > -		if (host->clk_mul) {
> > -			mmc->f_min = (host->max_clk * host->clk_mul) / 1024;
> > +		if (host->clk_mul)
> >  			max_clk = host->max_clk * host->clk_mul;
> > -		} else
> > -			mmc->f_min = host->max_clk / SDHCI_MAX_DIV_SPEC_300;
> > +		mmc->f_min = host->max_clk / SDHCI_MAX_DIV_SPEC_300;
> >  	} else
> >  		mmc->f_min = host->max_clk / SDHCI_MAX_DIV_SPEC_200;

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] mmc: sdhci: fix minimum clock rate for v3 controller
  2020-01-14 15:53   ` Michał Mirosław
@ 2020-01-15  7:16     ` Adrian Hunter
  0 siblings, 0 replies; 4+ messages in thread
From: Adrian Hunter @ 2020-01-15  7:16 UTC (permalink / raw)
  To: Michał Mirosław; +Cc: linux-mmc, Ulf Hansson, linux-kernel

On 14/01/20 5:53 pm, Michał Mirosław wrote:
> On Tue, Jan 14, 2020 at 03:08:08PM +0200, Adrian Hunter wrote:
>> On 2/01/20 12:51 pm, Michał Mirosław wrote:
>>> For SDHCIv3+ with programmable clock mode, minimal clock frequency is
>>> still base clock / max(divider). Minimal programmable clock frequency is
>>> always greater than minimal divided clock frequency. Without this patch,
>>> SDHCI uses out-of-spec initial frequency when multiplier is big enough:
>>>
>>> mmc1: mmc_rescan_try_freq: trying to init card at 468750 Hz
>>> [for 480 MHz source clock divided by 1024]
>>
>> The maximum divisor in programmable clock mode is 1024.  So I do not
>> understand what is wrong.  Can you explain some more?
> 
> This part of code misses the fact, that you can choose (switch) between
> base clock mode and programmable clock mode. The code in
> sdhci_calc_clk() already does the choosing part. This is actually
> required on high programmable clock base to get conformant frequency for
> the card initialization phase.

Ok, so please add that explanation to the commit message, and add a comment
in the code too.

> 
> Best Regards,
> Michał Mirosław
> 
> [...]
>>> index 275102c0a1bf..0036ddf85674 100644
>>> --- a/drivers/mmc/host/sdhci.c
>>> +++ b/drivers/mmc/host/sdhci.c
>>> @@ -3902,11 +3902,9 @@ int sdhci_setup_host(struct sdhci_host *host)
>>>  	if (host->ops->get_min_clock)
>>>  		mmc->f_min = host->ops->get_min_clock(host);
>>>  	else if (host->version >= SDHCI_SPEC_300) {
>>> -		if (host->clk_mul) {
>>> -			mmc->f_min = (host->max_clk * host->clk_mul) / 1024;
>>> +		if (host->clk_mul)
>>>  			max_clk = host->max_clk * host->clk_mul;
>>> -		} else
>>> -			mmc->f_min = host->max_clk / SDHCI_MAX_DIV_SPEC_300;
>>> +		mmc->f_min = host->max_clk / SDHCI_MAX_DIV_SPEC_300;
>>>  	} else
>>>  		mmc->f_min = host->max_clk / SDHCI_MAX_DIV_SPEC_200;


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-02 10:51 [PATCH] mmc: sdhci: fix minimum clock rate for v3 controller Michał Mirosław
2020-01-14 13:08 ` Adrian Hunter
2020-01-14 15:53   ` Michał Mirosław
2020-01-15  7:16     ` Adrian Hunter

Linux-mmc Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mmc/0 linux-mmc/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mmc linux-mmc/ https://lore.kernel.org/linux-mmc \
		linux-mmc@vger.kernel.org
	public-inbox-index linux-mmc

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-mmc


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git