Messages in this thread | | | Subject | Re: [PATCH 1/3] nvmem: core: Add functions to make number reading easy | From | Srinivas Kandagatla <> | Date | Wed, 10 Mar 2021 16:19:09 +0000 |
| |
On 10/03/2021 15:50, Doug Anderson wrote: > Hi, > > On Wed, Mar 10, 2021 at 2:37 AM Srinivas Kandagatla > <srinivas.kandagatla@linaro.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@chromium.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@linaro.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 >
| |