linux-kernel - Re: [PATCH] mmc: meson-gx: fix wrong conversion of __bf_shf to _

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] [day] [month] [year] [list]

Message-ID: <1j8qx2x73f.fsf@starbuckisacylon.baylibre.com>
Date: Mon, 12 Aug 2024 14:23:48 +0200
From: Jerome Brunet <jbrunet@...libre.com>
To: Christian Marangi <ansuelsmth@...il.com>
Cc: Ulf Hansson <ulf.hansson@...aro.org>,  Neil Armstrong
 <neil.armstrong@...aro.org>,  Kevin Hilman <khilman@...libre.com>,  Martin
 Blumenstingl <martin.blumenstingl@...glemail.com>,
  linux-mmc@...r.kernel.org,  linux-arm-kernel@...ts.infradead.org,
  linux-amlogic@...ts.infradead.org,  linux-kernel@...r.kernel.org,
  stable@...r.kernel.org
Subject: Re: [PATCH] mmc: meson-gx: fix wrong conversion of __bf_shf to __ffs

On Mon 12 Aug 2024 at 13:55, Christian Marangi <ansuelsmth@...il.com> wrote:

> Commit 795c633f6093 ("mmc: meson-gx: fix __ffsdi2 undefined on arm32")
> changed __bf_shf to __ffs to fix a compile error on 32bit arch that have
> problems with __ffsdi2. This comes from the fact that __bf_shf use
> __builtin_ffsll and on 32bit __ffsdi2 is missing.
>
> Problem is that __bf_shf is defined as
>
>   #define __bf_shf(x) (__builtin_ffsll(x) - 1)
>
> but the patch doesn't account for the - 1.
>
> Fix this by using the __builtin_ffs and add the - 1 to reflect the
> original implementation.
>
> The commit also converted other entry of __bf_shf in the code but those
> got dropped in later patches.
>
> Fixes: 795c633f6093 ("mmc: meson-gx: fix __ffsdi2 undefined on arm32")
> Signed-off-by: Christian Marangi <ansuelsmth@...il.com>

Hi Christian,

Are you fixing an actual problem you've seen with the platform and or
this solely based on the original commit description ?

If I dump the shift values with what we have right now, on sm1 at least
* Mux shift is 6
* Div shift is 0

This is aligned with the datasheet and has been working for while now.

> Cc: stable@...r.kernel.org # see patch description, needs adjustements for < 5.2
> ---
>  drivers/mmc/host/meson-gx-mmc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index c7c067b9415a..8f64083a08fa 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -464,7 +464,7 @@ static int meson_mmc_clk_init(struct meson_host *host)
>  	init.num_parents = MUX_CLK_NUM_PARENTS;
>  
>  	mux->reg = host->regs + SD_EMMC_CLOCK;
> -	mux->shift = __ffs(CLK_SRC_MASK);
> +	mux->shift = __builtin_ffs(CLK_SRC_MASK) - 1;
>  	mux->mask = CLK_SRC_MASK >> mux->shift;
>  	mux->hw.init = &init;
>  
> @@ -486,7 +486,7 @@ static int meson_mmc_clk_init(struct meson_host *host)
>  	init.num_parents = 1;
>  
>  	div->reg = host->regs + SD_EMMC_CLOCK;
> -	div->shift = __ffs(CLK_DIV_MASK);
> +	div->shift = __builtin_ffs(CLK_DIV_MASK) - 1;
>  	div->width = __builtin_popcountl(CLK_DIV_MASK);
>  	div->hw.init = &init;
>  	div->flags = CLK_DIVIDER_ONE_BASED;

-- 
Jerome