On 10/24/11 16:39, Guenter Roeck wrote:
> On Mon, 2011-10-24 at 06:09 -0400, Jonathan Cameron wrote:
> [ ... ]
>>>>> +/*
>>>>> + * Assumes that IIO and hwmon operate in the same base units.
>>>>> + * This is supposed to be true, but needs verification for
>>>>> + * new channel types.
>>>>> + */
>>>>> +static ssize_t iio_hwmon_read_val(struct device *dev,
>>>>> + struct device_attribute *attr,
>>>>> + char *buf)
>>>>> +{
>>>>> + long result;
>>>>> + int val, ret, scaleint, scalepart;
>>>>> + struct sensor_device_attribute *sattr = to_sensor_dev_attr(attr);
>>>>> + struct iio_hwmon_state *state = dev_get_drvdata(dev);
>>>>> +
>>>>> + /*
>>>>> + * No locking between this pair, so theoretically possible
>>>>> + * the scale has changed.
>>>>> + */
>>>>> + ret = iio_read_channel_raw(state->channels[sattr->index],
>>>>> + &val);
>>>>> + if (ret < 0)
>>>>> + return ret;
>>>>> +
>>>>> + ret = iio_read_channel_scale(state->channels[sattr->index],
>>>>> + &scaleint, &scalepart);
>>>>> + if (ret < 0)
>>>>> + return ret;
>>>>> + switch (ret) {
>>>>> + case IIO_VAL_INT:
>>>>> + result = val * scaleint;
>>>>> + break;
>>>>> + case IIO_VAL_INT_PLUS_MICRO:
>>>>> + result = (long)val * (long)scaleint +
>>>>> + (long)val * (long)scalepart / 1000000L;
>>>>> + break;
>>>>> + case IIO_VAL_INT_PLUS_NANO:
>>>>> + result = (long)val * (long)scaleint +
>>>>> + (long)val * (long)scalepart / 1000000000L;
>>>>> + break;
>>>>
>>>> Still easy to imagine that val * scalepart gets larger than 2147483647L
>>>> (on machines where sizeof(long) = 4) ... it will already happen if the
>>>> result of (val * scalepart / 1000000000) is larger than 2.
>>> Good point. I really ought to have done the calcs.
>>> If we have maximum possible value in here things will be ugly.
>>>
>>> Worst case is scalepart is 9999999999. (could be done as 1 - 0.000000001
>>> which would be nicer, but we don't specify a preference - from this
>>> discussion I am suspecting we should!)
>>>
>>> Looks like 64 bits is going to be a requirement as you say.
>>>>
>>>> What value range do you expect to see here ?
>>>>
>>>> If (val * scaleint) is already the milli-unit, scalepart would possibly
>>>> only address fractions of milli-units. If so, the result of (val *
>>>> scalepart / 1000000000L) might always be smaller than 1, ie 0.
>>> It certainly should be.
>>>> If so, for the calculation to have any value, you might be better off using
>>>> DIV_ROUND_CLOSEST(val * scalepart, 1000000000L).
>>> Good idea.
>>>>
>>>> I am a bit confused by this anyway. Since hwmon in general reports
>>>> milli-units, VAL_INT appears to reflect milli-units, VAL_INT_PLUS_MICRO
>>>> really means nano-units, and IIO_VAL_INT_PLUS_NANO really means
>>>> pico-units. Is this correct ?
>>> Micro units of the scale factor.
>>>
>>> Take my test part a max1363...
>>> Scale is actually 0.5 so each adc count (e.g. raw value) is 0.5millivolts.
>>>
>>> scale int here is 0,
>>> scale part is 500,000 (so 0.5) and it returns IIO_VAL_INT_PLUS_MICRO.
>>
>> How about the following? It'll be extremely costly, but this isn't exactly
>> a fast path!
>>
>> case IIO_VAL_INT_PLUS_MICRO:
>> result = (s64)val * (s64)scaleint +
>> div_s64((s64)val * (s64)scalepart, 1000000LL);
>> break;
>> case IIO_VAL_INT_PLUS_NANO:
>> result = (s64)val * (s64)scaleint +
>> div_s64((s64)val * (s64)scalepart, 1000000000LL);
>> break;
>
> Is div_s64 really necessary, or would
>
> result = (long)val * (long)scaleint +
> DIV_ROUND_CLOSEST((s64)val * (s64)scalepart,
> 1000000000LL);
>
> work as well ?
Not if you want it to compile on arm v5 by the look of it.
ERROR: "__aeabi_ldivmod" [drivers/staging/iio/iio_hwmon.ko] undefined!
_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
