Re: [PATCH 03/10] iio: accel: BMA220 migrate to regmap API

[Jonathan Cameron]

On Mon, 8 Sep 2025 06:27:05 +0300
Petre Rodan <petre.rodan@xxxxxxxxxxxxxxx> wrote:

> On Sun, Sep 07, 2025 at 01:45:06PM +0100, Jonathan Cameron wrote:
> > > +static int bma220_reset(struct bma220_data *data, bool up)
> > >  {
> > > +	int i, ret;
> > > +	unsigned int val;
> > > +	guard(mutex)(&data->lock);
> > > 
> > > +	/**
> > > +	 * The chip can be reset by a simple register read.
> > > +	 * We need up to 2 register reads of the softreset register  
> > 
> > May need?  Given you return early if the first one succeeds. If you actually
> > need two drop the loop and only check values on second read.
> >   
> > > +	 * to make sure that the device is in the desired state.
> > > +	 */
> > > +	for (i = 0; i < 2; i++) {
> > > +		ret = regmap_read(data->regmap, BMA220_REG_SOFTRESET, &val);
> > > +		if (ret < 0)
> > > +			return ret;  
> 
> I'm not sure how eloquently I can explain this. the sensor can be in
> 
> sleep state / non-sleep state
> reset state / non-reset state
> (these overlap)
> 
> the sensor toggles between these states when the master reads the suspend and
> the soft_reset registers respectively.
> based on the value read one can tell what was the previous state the sensor was in.
> 
> bma220_init() simply places the sensor in the non-sleep AND non-reset modes (and
> resets all configuration registers so that we start from a known initial condition)
> 
> 'may need' is used because the sensor might have been left in an unexpected mode
> in the previous session.
> we need at most two reads of a register to make sure bma220 ends up in the state we need.

Fair enough.  That is obscure and generally weird. 'may need' seems a valid short comment!

J
> 
> best regards,
> peter
>