Re: [RFC PATCH v2 7/9] iio: pressure: bmp280: implement sampling_frequency for BMx280

[ASSI <Stromeko@nexgo.de>]

Jonathan Cameron writes:
>> +	/* switch mode based on sampling_freq */
>> +	data->op_mode = data->sampling_freq ? BMP280_NORMAL : BMP280_FORCED;
>> +
>> +	/* Mode transition or measurement in MODE_FORCED */
>> +	ret = data->chip_info->set_mode(data, data->sampling_freq ? BMP280_NORMAL : BMP280_FORCED);
>
> Use data->op_mode?  However, generally we'd set that only after success of the set_mode
> call, so use a local variable for the new mode and set the data->op_mode to that once
> succeeded in setting the mode.

Ack.  In a previous version of the code I couldn't use it, but now I can
as I explicitly set it to the correct value further up.

>>  	if (ret)
>>  		return ret;
>>  
>> @@ -1078,6 +1156,7 @@ static int bmp280_wait_conv(struct bmp280_data *data)
>>  				       !(reg & BMP280_REG_STATUS_MEAS_BIT),
>>  				       MIN((2 * USEC_PER_MSEC), (meas_time_us >> 1)),
>>  				       (2 * USEC_PER_MSEC + meas_time_us));
>> +
> Unrelated and not necessarily a good change.  Shouldn't be here.

Already removed on my end, just didn't make it into v2.

>> +	if (ret)
>> +		return ret;
>> +
>> +	/* is this really necessary or can we skip if op_mode is already BMP280_NORMAL? */
>
> I'm not sure if this question is for reviewers or something you want to check out yourself.

It was a reminder to check why sometimes the measurement would get stuck
and not deliver any new values, which turned out to be a bug in handling
suspend / resume.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+