Re: [PATCH] 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:
- [Low] Device tree node names use underscores instead of hyphens, which will trigger `dtc` build warnings.

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but concurrent reads from the NVMEM driver lead to interleaved I/O operations and corrupted data returns.
--

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

powerpc: wii: Add the fixed OTP layout

This commit exposes the 128-byte fixed layout of the Wii OTP as cells
for either kernel or userspace consumption.

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

[Severity: Low]
This isn't a bug, but does using underscores in device tree node names
violate standard devicetree naming conventions?

It looks like this will trigger dtc build warnings when compiling with strict
checking enabled, such as "Character '_' not recommended in node name".
Would it be better to use hyphens here, for example boot1-sha1@0?

[ ... ]

While looking at how these new cells are consumed by the NVMEM subsystem,
I noticed a behavior in the underlying provider code:

drivers/nvmem/nintendo-otp.c:nintendo_otp_reg_read() {
    ...
    iowrite32be(OTP_READ | bank | addr, priv->regs + HW_OTPCMD);
    *val++ = ioread32be(priv->regs + HW_OTPDATA);
    ...
}

[Severity: High]
This is a pre-existing issue and was not introduced by this patch, but
is there a missing lock around this two-step I/O sequence?

If two threads concurrently read from the NVMEM provider, could their
command and data I/O register accesses interleave?

For example:
Thread A writes to HW_OTPCMD.
Thread B preempts, writes to HW_OTPCMD, and reads HW_OTPDATA.
Thread A resumes and reads HW_OTPDATA, incorrectly receiving Thread B's data.

Since the NVMEM core does not serialize reg_read callbacks, should the
provider implement locking to prevent silent data corruption on concurrent
accesses?

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