Re: [PATCH v2 3/4] iio: accel: kionix-kx022a: Prevent memory leak and fix state

[Matti Vaittinen]

On 28/08/2026 10:56, Andy Shevchenko wrote:
> On Fri, Aug 28, 2026 at 10:40:51AM +0300, Matti Vaittinen wrote:
>
> > The driver allocates memory for samples at buffer enable path. If regmap
> > operation fails in the kx022a_fifo_enable() at the buffer enable path, the
> > allocated memory is never freed. Furthermore, the state information and
> > previous hardware configuration(s) aren't undone, potentially leaving
> > WMI interrupts and buffers enabled, or driver state flags wrong.
> >
> > Free the memory and revert the hardware configuration and state flags on
> > error path.
>
> ...
>
> > static int kx022a_fifo_enable(struct kx022a_data *data)
>
> With
>
> 	struct regmap *map = data->regmap;
>
> >   	guard(mutex)(&data->mutex);
> >   	ret = __kx022a_turn_on_off(data, false);
> >   	if (ret)
> > -		return ret;
> > +		goto err_free_out;
> >   
> >   	/* Update watermark to HW */
> >   	ret = kx022a_fifo_set_wmi(data);
> >   	if (ret)
> > -		return ret;
> > +		goto err_wmi_out;
> >   
> >   	/* Enable buffer */
> >   	ret = regmap_set_bits(data->regmap, data->chip_info->buf_cntl2,
> >   			      KX022A_MASK_BUF_EN);
> >   	if (ret)
> > -		return ret;
> > +		goto err_wmi_out;
> >   
> >   	data->state |= KX022A_STATE_FIFO;
> >   	ret = regmap_set_bits(data->regmap, data->ien_reg,
> >   			      KX022A_MASK_WMI);
> >   	if (ret)
> > -		return ret;
> > +		goto err_buf_en_out;
> >   
> > -	return __kx022a_turn_on_off(data, true);
> > +	ret = __kx022a_turn_on_off(data, true);
> > +	if (ret)
> > +		goto err_on_out;
> > +
> > +	return ret;
> > +
> > +err_on_out:
> > +	regmap_clear_bits(data->regmap, data->ien_reg,
> > +			  KX022A_MASK_WMI);
>
> in particular this will be
>
> 	regmap_clear_bits(map, data->ien_reg, KX022A_MASK_WMI);
>
> > +err_buf_en_out:
> > +	regmap_clear_bits(data->regmap, data->chip_info->buf_cntl2,
> > +			  KX022A_MASK_BUF_EN);
>
> 	regmap_clear_bits(map, data->chip_info->buf_cntl2, KX022A_MASK_BUF_EN);
>
> Which saves a single line, and the followup may shorten the existing code even
> more.

As far as I can see, the driver is consistently using the 'data->regmap' 
everywhere. Using 'map' just in one function would raise question if it 
is different regmap from every other place.

I am 100% Ok with anyone changing the data->regmap to 'map' throughout 
the whole driver in order to cut the amount of lines - but it is not 
something to be done in a bugfix commit.

Yours,
	-- Matti

--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~