linux-kernel - Re: [PATCH v2] drm/msm: Use nvmem_cell_read_variable_le

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAE-0n52xEDak4-vuJQ6SQz83F54-oTm+TjeVJ_0GoezG8O_M5Q@mail.gmail.com>
Date:   Fri, 21 May 2021 15:02:26 -0700
From:   Stephen Boyd <swboyd@...omium.org>
To:     Douglas Anderson <dianders@...omium.org>,
        Rob Clark <robdclark@...il.com>
Cc:     John Stultz <john.stultz@...aro.org>,
        YongQin Liu <yongqin.liu@...aro.org>,
        Jordan Crouse <jordan@...micpenguin.net>,
        linux-arm-msm@...r.kernel.org,
        Akhil P Oommen <akhilpo@...eaurora.org>,
        Daniel Vetter <daniel@...ll.ch>,
        David Airlie <airlied@...ux.ie>, Eric Anholt <eric@...olt.net>,
        Jonathan Marek <jonathan@...ek.ca>,
        Sai Prakash Ranjan <saiprakash.ranjan@...eaurora.org>,
        Sean Paul <sean@...rly.run>,
        Sharat Masetty <smasetty@...eaurora.org>,
        dri-devel@...ts.freedesktop.org, freedreno@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] drm/msm: Use nvmem_cell_read_variable_le_u32() to read
 speed bin

Quoting Douglas Anderson (2021-05-21 13:45:50)
> Let's use the newly-added nvmem_cell_read_variable_le_u32() to future
> proof ourselves a little bit.
>
> Signed-off-by: Douglas Anderson <dianders@...omium.org>
> ---
> The patch that this depends on is now in mainline so it can be merged
> at will. I'm just sending this as a singleton patch to make it obvious
> that there are no dependencies now.
>
> Changes in v2:
> - Rebased
>
>  drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index b4d8e1b01ee4..a07214157ad3 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -1403,10 +1403,10 @@ static int a6xx_set_supported_hw(struct device *dev, struct a6xx_gpu *a6xx_gpu,
>  {
>         struct opp_table *opp_table;
>         u32 supp_hw = UINT_MAX;
> -       u16 speedbin;
> +       u32 speedbin;
>         int ret;
>
> -       ret = nvmem_cell_read_u16(dev, "speed_bin", &speedbin);
> +       ret = nvmem_cell_read_variable_le_u32(dev, "speed_bin", &speedbin);

I missed the review of this API, sorry. I wonder why it doesn't return
the value into an __le32 pointer. Then the caller could use
le32_to_cpu() like other places in the kernel and we know that code is
properly converting the little endian value to CPU native order. Right
now the API doesn't express the endianess of the bits in the return
value because it uses u32, so from a static checker perspective (sparse)
those bits are CPU native order, not little endian.

>         /*
>          * -ENOENT means that the platform doesn't support speedbin which is
>          * fine