linux-kernel - Re: [PATCH] mmc: sdhci: fix minimum clock rate for v3 controller

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200114155333.GA29422@qmqm.qmqm.pl>
Date:   Tue, 14 Jan 2020 16:53:33 +0100
From:   Michał Mirosław <mirq-linux@...e.qmqm.pl>
To:     Adrian Hunter <adrian.hunter@...el.com>
Cc:     linux-mmc@...r.kernel.org, Ulf Hansson <ulf.hansson@...aro.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mmc: sdhci: fix minimum clock rate for v3 controller

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;