Re: [PATCH v3 0/7] qcom: Add NVMEM bitfield support and reboot‑mode integration

[Aswin Murugan]

On 3/31/2026 12:44 AM, Heinrich Schuchardt wrote:
> Am 30. März 2026 19:14:12 MESZ schrieb Aswin Murugan <aswin.murugan at oss.qualcomm.com>:
>> This patch series introduces bit-level granularity to NVMEM cells and
>> adds complete reboot-mode support for Qualcomm platforms that store
>> reboot reasons in PMIC registers.
>>
>> Qualcomm SoCs rely on PMIC-backed reboot reason storage to implement
>> features like "reboot bootloader" for entering fastboot mode. However,
>> these PMIC registers often pack multiple fields into a single byte,
>> requiring fine-grained bit access that the current NVMEM subsystem does
>> not support.
>>
>> In addition, PMIC generations differ in how reboot-related data is
>> stored: older PMICs use PON (Power On) registers, while newer ones
>> provide SDAM regions. This series introduces a unified, NVMEM-based
>> approach that works seamlessly across both architectures.
>>
>> This version also integrates reboot-mode handling into Qualcomm board
>> initialization, enabling automatic fastboot entry when the reboot reason
>> indicates bootloader mode.
>>
>> Signed-off-by: Aswin Murugan <aswin.murugan at oss.qualcomm.com>
> It is unclear to me how endianness is handled in bitfields. Do you assume low-endianness or do you consider the system endianness? Please, ensure that the tests are executed on both low- and big-endian systems.
>
> Best regards
>
> Heinrich
>
>
>> ---
>> Changes in v3:
>> 1. Simplified bit field handling to maximum u32 size (32 bits).
>> 2. Enforced strict size matching (size == cell->size) when nbits == 0.
>> 3. Enhanced test function for NVMEM read & write
>> 4. Updated NVMEM API documentation
>>
>> Link to V2:
>> https://lore.kernel.org/all/20260213112717.1256823-1-aswin.murugan@oss.qualcomm.com/
>>
>> Changes in v2:
>> 1. Replaced custom reboot reason handling with the standard U-Boot
>>    reboot-mode subsystem, per review feedback.
>> 2. Added bit-field support to the NVMEM core using the new "bits"
>>    property.
>> 3. Introduced the Qualcomm SPMI SDAM driver for unified PMIC storage
>>    access.
>> 4. Updated the reboot-mode driver to support variable-sized NVMEM cells.
>> 5. Added device tree configuration for the QCS615 RIDE board.
>> 6. Enabled reboot-mode in qcom_defconfig.
>> 7. Integrated reboot-mode detection into Snapdragon board
>>    initialization:
>>    - Added qcom_handle_reboot_mode() in board_late_init() to enable
>>      automatic fastboot entry on "reboot bootloader".
>>
>> Link to v1:
>> https://lore.kernel.org/all/20260108065533.1143179-1-aswin.murugan@oss.qualcomm.com/
>> ---
>>
>>
>> Aswin Murugan (7):
>>   misc: Add support for bit fields in NVMEM cells
>>   misc: qcom: Add Qualcomm SPMI SDAM NVMEM driver
>>   mach-snapdragon: Integrate reboot-mode handling
>>   dts: qcs615-ride-u-boot.dtsi: Add reboot-mode support
>>   qcom_defconfig: Enable reboot-mode support in qcom_defconfig
>>   test: dm: add comprehensive tests for NVMEM bit field operations
>>   misc: update API documentation for bit field support in NVMEM
>>
>> arch/arm/dts/qcs615-ride-u-boot.dtsi |  26 ++++
>> arch/arm/mach-snapdragon/board.c     |  56 ++++++--
>> arch/sandbox/dts/test.dts            |  12 ++
>> configs/qcom_defconfig               |   3 +
>> drivers/misc/Kconfig                 |   8 ++
>> drivers/misc/Makefile                |   1 +
>> drivers/misc/nvmem.c                 | 159 ++++++++++++++++++---
>> drivers/misc/qcom-spmi-sdam.c        | 200 +++++++++++++++++++++++++++
>> include/nvmem.h                      |  28 +++-
>> test/dm/reboot-mode.c                | 137 ++++++++++++++++++
>> 10 files changed, 595 insertions(+), 35 deletions(-)
>> create mode 100644 drivers/misc/qcom-spmi-sdam.c
>>
Hi Heinrich,

Thank you for raising this concern.
The proposed change explicitly interprets hardware bytes as 
little‑endian when nbits is set, ensuring consistent and
predictable behavior across architectures.


On the read path, hardware data is treated as little‑endian, converted to
CPU‑native endianness for bitfield extraction, and the final value is 
returned
in CPU‑native format:


/* *Read: Hardware LE → CPU native → bit ops → return CPU native* */
/* Interpret hardware bytes as little-endian for consistent bit field 
operations */
value = le32_to_cpu(*((__le32 *)buf));
value >>= cell->bit_offset;
/* Handle nbits == 32 specially to avoid undefined behavior */
if (cell->nbits < 32)
     value &= (1U << cell->nbits) - 1;
/* Return value in CPU native endianness for caller */
*(u32 *)buf = value;


On the write path, both the existing hardware value and the caller‑provided
value are handled in CPU‑native endianness during bit manipulation, and the
final result is converted back to little‑endian before being written to
hardware:

*
/* Write: HW(LE) → le32_to_cpu() → CPU native → bit ops → cpu_to_le32() 
→ HW */*
current = le32_to_cpu(*((__le32 *)&current));
/* Caller provides value in CPU native endianness */
value = *(u32 *)buf;
value &= (1U << cell->nbits) - 1;
value <<= cell->bit_offset;
mask = ((1U << cell->nbits) - 1) << cell->bit_offset;
*((__le32 *)buf) = cpu_to_le32((current & ~mask) | value);


Hope this approach addresses the concern raised.

Regards,

Aswin