Re: [PATCH v2] powerpc: wii: Add the fixed OTP layout

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] The `device-private-key@28` NVMEM cell has a length of 30 bytes, which is unaligned to the provider's 4-byte `word_size`, causing an unconditional `dev_err` to be logged during boot.

Pre-existing issues:
- [High] Concurrent reads to the Nintendo OTP NVMEM device cause data corruption due to missing locking in `nintendo_otp_reg_read`.
--

--- Patch [1]: [PATCH v2] powerpc: wii: Add the fixed OTP layout ---
commit 790200474049a449f0c4b09d4900a200ca3ca581
Author: Link Mauve <linkmauve@linkmauve.fr>

powerpc: wii: Add the fixed OTP layout

This commit adds a fixed nvmem-layout to the hollywood-otp device tree
node for the Nintendo Wii. It defines the locations and sizes of various
keys and IDs within the 128-byte OTP memory, exposing them as NVMEM cells.

Link: https://lore.kernel.org/lkml/20260726181921.16269-1-linkmauve@linkmauve.fr/T/

> diff --git a/arch/powerpc/boot/dts/wii.dts b/arch/powerpc/boot/dts/wii.dts
> index 473d9feb9cdb8..d3a94d94e9e19 100644
> --- a/arch/powerpc/boot/dts/wii.dts
> +++ b/arch/powerpc/boot/dts/wii.dts
> @@ -233,6 +233,48 @@ control@d800100 {
>  		otp@d8001ec {
>  			compatible = "nintendo,hollywood-otp";
>  			reg = <0x0d8001ec 0x8>;
> +
> +			nvmem-layout {
> +				compatible = "fixed-layout";
> +				#address-cells = <1>;
> +				#size-cells = <1>;

[ ... ]

> +				/*
> +				 * device-private-key@28 and nand-hmac-key@44
> +				 * overlap on two bytes, that’s expected.
> +				 */
> +				device-private-key@28 {
> +					reg = <0x28 0x1e>;
> +				};

[Severity: Medium]
Does this 30-byte length trigger a spurious error log on boot?

The nintendo-otp driver specifies a word_size of 4. When the NVMEM core
parses this cell via nvmem_add_cells_from_dt(), it sets the raw length
to 30. Later, nvmem_cell_info_to_nvmem_cell_entry_nodup() checks if the
raw length is aligned to the word size.

Since 30 is not a multiple of 4, will the NVMEM core unconditionally log
a dev_err before rounding it up?

Would adding the bits property (e.g., bits = <0 240>) allow the NVMEM core
to gracefully pre-align the raw read length without emitting the spurious
error?

[ ... ]

> +				unknown@78 {
> +					reg = <0x78 0x8>;
> +				};
> +			};

[Severity: High]
This is a pre-existing issue, but does concurrent reading of these newly
exposed OTP cells cause data corruption?

Looking at the driver in drivers/nvmem/nintendo-otp.c:

nintendo_otp_reg_read() {
    ...
    iowrite32be(OTP_READ | bank | addr, priv->regs + HW_OTPCMD);
    *val++ = ioread32be(priv->regs + HW_OTPDATA);
    ...
}

Because this uses a stateful two-step I/O sequence (writing the target
address to HW_OTPCMD, then reading the data from HW_OTPDATA) without any
synchronization primitives, what happens if multiple drivers or userspace
processes read from these cells concurrently?

Will the executions interleave, causing one thread to read the data requested
by another thread?

>  		};
>  
>  		disk@d806000 {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729225010.838760-1-linkmauve@linkmauve.fr?part=1