Re: [PATCH 1/4] nvmem: rockchip-otp: Serialize reads

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Mixing goto-based error handling with scope-based guard() cleanup in the same function.
--

commit 279787a491898a378d6e600f26ecd6fbd75f2d26
Author: Alexey Charkov <alchark@flipper.net>

nvmem: rockchip-otp: Serialize reads

The OTP controller is driven through a single set of registers holding a
state machine which has to be stepped through for every word read, yet
nothing keeps two readers out of each other's way. Concurrent reads
interleave, and the outcome is either a reader bailing out or, worse, one
of them silently taking delivery of the other's data.

Guard the read path with a mutex.

> diff --git a/drivers/nvmem/rockchip-otp.c b/drivers/nvmem/rockchip-otp.c
> index 2c0feb036f3fd..b034b7455016a 100644
> --- a/drivers/nvmem/rockchip-otp.c
> +++ b/drivers/nvmem/rockchip-otp.c
[ ... ]
> @@ -272,6 +275,8 @@ static int rockchip_otp_read(void *context, unsigned int offset,
>  	if (!otp->data || !otp->data->reg_read)
>  		return -EINVAL;
>  
> +	guard(mutex)(&otp->mutex);
> +

[Severity: Medium]
Does adding this guard() mix scope-based cleanup with the goto-based
error handling further down in rockchip_otp_read()?

The kernel cleanup guidelines in include/linux/cleanup.h state that usage
of goto and cleanup helpers should never be mixed in the same function.

Since rockchip_otp_read() still contains a goto err path:

		buf = kzalloc(array_size(count, word_size), GFP_KERNEL);
		if (!buf) {
			ret = -ENOMEM;
			goto err;
		}

and a manual cleanup label:

err:
	clk_bulk_disable_unprepare(otp->data->num_clks, otp->clks);

could the clock cleanup be converted to use scope-based cleanup as well,
or should a plain mutex_lock and mutex_unlock be used instead of guard()
to avoid mixing cleanup styles?

>  	ret = clk_bulk_prepare_enable(otp->data->num_clks, otp->clks);
>  	if (ret < 0) {
>  		dev_err(otp->dev, "failed to prepare/enable clks\n");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901-rk3576-otp-cpuid-mac-v1-0-ea9135270fc2@flipper.net?part=1