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

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 ;)

Yours,
	-- Matti

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

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