Re: [PATCH v5 10/17] nvmem: microchip-otpc: nvmem: add emulation mode and OTP access validation

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

Pre-existing issues:
- [High] Missing locking in NVMEM read operations.
--

--- Patch [10]: [PATCH v5 10/17] nvmem: microchip-otpc: nvmem: add emulation mode and OTP access validation ---
commit ff3ee8b6ff65c931c28481b3cf9aabc752c44762
Author: Varshini Rajendran <[email protected]>

nvmem: microchip-otpc: nvmem: add emulation mode and OTP access validation

This commit adds validation of OTP memory accessibility and emulation mode
status during driver probe. It allows access to other packets when the boot
packet is not configured, and logs an informational message if neither is
available to assist with debugging.

> diff --git a/drivers/nvmem/microchip-otpc.c b/drivers/nvmem/microchip-otpc.c
> index a17f82afdc37d..6b0184423463e 100644
> --- a/drivers/nvmem/microchip-otpc.c
> +++ b/drivers/nvmem/microchip-otpc.c
> @@ -18,6 +18,7 @@
>  #define MCHP_OTPC_CR_READ		BIT(6)
>  #define MCHP_OTPC_MR			(0x4)
>  #define MCHP_OTPC_MR_ADDR		GENMASK(31, 16)
> +#define MCHP_OTPC_MR_EMUL		BIT(7)

[Severity: High]
This is a pre-existing issue, but does mchp_otpc_read() need serialization
to prevent concurrent hardware accesses from corrupting the read address?

Since this sysfs nvmem attribute defaults to world-readable, multiple
unprivileged userspace processes can read from the device concurrently.

If they do, they can race in mchp_otpc_prepare_read() when modifying the
Memory Register and Control Register:

mchp_otpc_read() {
        ...
        ret = mchp_otpc_prepare_read(otpc, offset);
        ...
}

mchp_otpc_prepare_read() {
        ...
        /* Set address. */
        tmp = readl_relaxed(otpc->base + MCHP_OTPC_MR);
        tmp &= ~MCHP_OTPC_MR_ADDR;
        tmp |= FIELD_PREP(MCHP_OTPC_MR_ADDR, offset);
        writel_relaxed(tmp, otpc->base + MCHP_OTPC_MR);
        ...
}

Could concurrent reads overwrite each other's requested addresses, leading
to incorrect OTP data being read or stalling the controller?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=10