Re: [PATCH 07/12] iio: pressure: rohm-bm1390: Fix AVE_NUM initialization

On Mon, 17 Aug 2026 08:51:46 +0300
Matti Vaittinen <[email protected]> wrote:

> On 17/08/2026 04:12, Jonathan Cameron wrote:
> > On Mon, 10 Aug 2026 10:53:07 +0300
> > Matti Vaittinen <[email protected]> wrote:
> >   
> >> From: Matti Vaittinen <[email protected]>
> >>
> >> The BM1390 tries to initialize the AVE_NUM to 110b at the start-up. The
> >> field location is not taken into account, and value is written unsifted.
> >> This causes the AVE_NUM to be initialized to zero.
> >>
> >> Use FIELD_PREP() to shift the intended AVE_NUM value to correct field.
> >>
> >> Signed-off-by: Matti Vaittinen <[email protected]>
> >> Fixes: 81ca5979b6ed ("iio: pressure: Support ROHM BU1390")
> >> ---
> >>   drivers/iio/pressure/rohm-bm1390.c | 4 +++-
> >>   1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/iio/pressure/rohm-bm1390.c b/drivers/iio/pressure/rohm-bm1390.c
> >> index d00d7ed54cb1..29454570f257 100644
> >> --- a/drivers/iio/pressure/rohm-bm1390.c
> >> +++ b/drivers/iio/pressure/rohm-bm1390.c
> >> @@ -479,6 +479,7 @@ static const struct iio_info bm1390_info = {
> >>   
> >>   static int bm1390_chip_init(struct bm1390_data *data)
> >>   {
> >> +	u8 regval;
> >>   	int ret;
> >>   
> >>   	ret = regmap_write_bits(data->regmap, BM1390_REG_POWER,
> >> @@ -512,8 +513,9 @@ static int bm1390_chip_init(struct bm1390_data *data)
> >>   	 * Default to use IIR filter in "middle" mode. Also the AVE_NUM must
> >>   	 * be fixed when IIR is in use.
> >>   	 */
> >> +	regval = FIELD_PREP(BM1390_MASK_AVE_NUM, BM1390_IIR_AVE_NUM);
> >>   	ret = regmap_update_bits(data->regmap, BM1390_REG_MODE_CTRL,
> >> -				 BM1390_MASK_AVE_NUM, BM1390_IIR_AVE_NUM);
> >> +				 BM1390_MASK_AVE_NUM,  
> > 				 FIELD_PREP(BM1390_MASK_AVE_NUM, BM1390_IIR_AVE_NUM));
> > 
> > If respining I would drop the local variable and just go a bit long on the line.
> > I like the mask to be clearly visible in both parameters and a local variable
> > prevents that. Fine if the line is really long, but it's only about 83 chars here.  
> 
> I am not going to insist - but I will explain my view - which will sound 
> like insisting XD
> 
> For me it is always a tad harder to read code when function or macro 
> "calls" are done inside a parameter list. Hence I like a local variable 
> here. I do 100% agree with you that mask (and sometimes raw numbers) 
> should be visible in code - and not hidden into variables / defines. 
> Still, in this case, the mask is assigned to the variable right above 
> the line it gets used. So, I could argue the mask is still pretty 
> visible, and FIELD_PREP() is called on it's own line and not inside the 
> regmap_update_bits() parameter list - which is more readable for me.
> 
> Well, if you feel strongly about the variable - then I can ditch it. 83 
> chars is still okay(ish) - even on my "old eyes, big font and 3 
> terminals in parallel" -display setup ;)
I don't care enough to insist :)

J
> 
> Yours,
> 	-- Matti
>