linux-kernel - Re: [PATCH 1/7] mmc: sdhci: fix base clock usage in preset value

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: <218dd61b-48cc-a161-240f-b3823e8f48cb@intel.com>
Date:   Wed, 15 Apr 2020 15:25:52 +0300
From:   Adrian Hunter <adrian.hunter@...el.com>
To:     Michał Mirosław <mirq-linux@...e.qmqm.pl>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        Kevin Liu <kliu5@...vell.com>,
        Michal Simek <michal.simek@...inx.com>,
        Suneel Garapati <suneel.garapati@...inx.com>
Cc:     linux-mmc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/7] mmc: sdhci: fix base clock usage in preset value

On 2/04/20 2:54 pm, Michał Mirosław wrote:
> Fixed commit added an unnecessary read of CLOCK_CONTROL. The value read
> is overwritten for programmable clock preset, but is carried over for
> divided clock preset. This can confuse sdhci_enable_clk() if the register
> has enable bits set for some reason at time time of clock calculation.
> value to be ORed with enable flags. Remove the read.

The read is not needed, but drivers usually manage the enable bits,
especially disabling the clock before changing the frequency.  What driver
is it?

> 
> Fixes: 52983382c74f ("mmc: sdhci: enhance preset value function")
> Signed-off-by: Michał Mirosław <mirq-linux@...e.qmqm.pl>
> ---
>  drivers/mmc/host/sdhci.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 3f716466fcfd..9aa3af5826df 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1765,7 +1765,6 @@ u16 sdhci_calc_clk(struct sdhci_host *host, unsigned int clock,
>  		if (host->preset_enabled) {
>  			u16 pre_val;
>  
> -			clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
>  			pre_val = sdhci_get_preset_value(host);
>  			div = FIELD_GET(SDHCI_PRESET_SDCLK_FREQ_MASK, pre_val);
>  			if (host->clk_mul &&
>