linux-kernel - Re: [PATCH 1/3] nvmem: core: Add functions to make number reading easy

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: <6f567b9b-fb67-6bcc-1970-99f96eded929@linaro.org>
Date:   Wed, 10 Mar 2021 16:19:09 +0000
From:   Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
To:     Doug Anderson <dianders@...omium.org>
Cc:     "Rafael J . Wysocki" <rafael.j.wysocki@...el.com>,
        Rob Clark <robdclark@...il.com>,
        Jordan Crouse <jcrouse@...eaurora.org>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        Stephen Boyd <swboyd@...omium.org>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Akhil P Oommen <akhilpo@...eaurora.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/3] nvmem: core: Add functions to make number reading
 easy



On 10/03/2021 15:50, Doug Anderson wrote:
> Hi,
> 
> On Wed, Mar 10, 2021 at 2:37 AM Srinivas Kandagatla
> <srinivas.kandagatla@...aro.org> wrote:
>>
>>
>>
>> On 06/03/2021 00:26, Douglas Anderson wrote:
>>> Sometimes the clients of nvmem just want to get a number out of
>>> nvmem. They don't want to think about exactly how many bytes the nvmem
>>> cell took up. They just want the number. Let's make it easy.
>>>
>>> In general this concept is useful because nvmem space is precious and
>>> usually the fewest bits are allocated that will hold a given value on
>>> a given system. However, even though small numbers might be fine on
>>> one system that doesn't mean that logically the number couldn't be
>>> bigger. Imagine nvmem containing a max frequency for a component. On
>>> one system perhaps that fits in 16 bits. On another system it might
>>> fit in 32 bits. The code reading this number doesn't care--it just
>>> wants the number.
>>>
>>> We'll provide two functions: nvmem_cell_read_variable_le_u32() and
>>> nvmem_cell_read_variable_le_u64().
>>>
>>> Comparing these to the existing functions like nvmem_cell_read_u32():
>>> * These new functions have no problems if the value was stored in
>>>     nvmem in fewer bytes. It's OK to use these function as long as the
>>>     value stored will fit in 32-bits (or 64-bits).
>>> * These functions avoid problems that the earlier APIs had with bit
>>>     offsets. For instance, you can't use nvmem_cell_read_u32() to read a
>>>     value has nbits=32 and bit_offset=4 because the nvmem cell must be
>>>     at least 5 bytes big to hold this value. The new API accounts for
>>>     this and works fine.
>>> * These functions make it very explicit that they assume that the
>>>     number was stored in little endian format. The old functions made
>>>     this assumption whenever bit_offset was non-zero (see
>>>     nvmem_shift_read_buffer_in_place()) but didn't whenever the
>>>     bit_offset was zero.
>>>
>>> NOTE: it's assumed that we don't need an 8-bit or 16-bit version of
>>> this function. The 32-bit version of the function can be used to read
>>> 8-bit or 16-bit data.
>>>
>>> At the moment, I'm only adding the "unsigned" versions of these
>>> functions, but if it ends up being useful someone could add a "signed"
>>> version that did 2's complement sign extension.
>>>
>>> At the moment, I'm only adding the "little endian" versions of these
>>> functions. Adding the "big endian" version would require adding "big
>>> endian" support to nvmem_shift_read_buffer_in_place().
>>>
>>> Signed-off-by: Douglas Anderson <dianders@...omium.org>
>>> ---
>>> This is a logical follow-up to:
>>>     https://lore.kernel.org/r/20210227002603.3260599-1-dianders@chromium.org/
>>> ...but since it doesn't really share any of the same patches I'm not
>>> marking it as a v2.
>>>
>>>    drivers/nvmem/core.c           | 95 ++++++++++++++++++++++++++++++++++
>>>    include/linux/nvmem-consumer.h |  4 ++
>>>    2 files changed, 99 insertions(+)
>>>
>>
>> This patch as it is LGTM.
>>
>> If you plan to take this via other trees, here is
>>
>> Reviewed-by: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
> 
> Thanks! I think none of this is terribly urgent, though. Unless
> someone has a different opinion, my thought would be:
> 
> * This patch lands in your tree for 5.13.
> 
> * I'll snooze the email for 2 months and poke patch #2 and #3 once
> 5.13-rc1 is out.
> 
> Does that sound OK to you?
That works for me!

Applied thanks!

--srini
> 
> -Doug
>