On 18.04.2016 13:01, Vlad Dogaru wrote:
Hi Jonathan,
On Sat, Apr 16, 2016 at 11:04:05AM +0100, Jonathan Cameron wrote:
On 15/04/16 18:53, Akinobu Mita wrote:
[...]
> +static int bmp180_read_press(struct bmp280_data *data,
> + int *val, int *val2)
> +{
> + int ret;
> + s32 adc_press;
> + u32 comp_press;
> +
> + /* Read and compensate temperature so we get a reading of t_fine. */
> + ret = bmp180_read_temp(data, NULL);
> + if (ret)
> + return ret;
> +
> + ret = bmp180_read_adc_press(data, &adc_press);
> + if (ret)
> + return ret;
> +
> + comp_press = bmp180_compensate_press(data, adc_press);
> +
> + *val = comp_press;
> + *val2 = 1000;
This would be better done using a _scale info_mask element to handle
the divide
by 1000. That lets the decision on whether the conversion is even
needed be
moved to userspace.
Unfortunately that's also true for the bmp280 as it stands which is
annoying
as that makes it ABI which we shouldn't be changing. Ah well, the
device
is slow anyway so not terrible to leave this as it is. I should have
picked
that up when reviewing that driver originally - sometimes we have live
with
our mistakes!
I don't think that qualifies as breaking ABI. The ABI specifies which
files *may* exist and how they should be interpreted, if they exist.
It does not guarantee that a specific sensor such as BMP280 exposes its
measurement as in_temp_input, rather than in_temp_raw and
in_temp_scale.
It's kind of an open question when this happens. Sadly it counts as
breaking the
ABI even if the program that was using it was using it wrongly.
However, rule
1 of ABI breakage is 'If no one notices then it isn't broken ;)'. So if
people are all using the ABI correctly then we are fine... The question
is
'Do we think they are?' :)
That said, I don't feel strongly about the changes either way. When I
initially wrote the driver I thought the operations were harmless
enough
to do in the kernel.
Leave it as is, doesn't really matter either way.
Jonathan
Thanks,
Vlad
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
