lkml.org 
[lkml]   [2011]   [Oct]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 5/6] IIO:hwmon interface client driver.
    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!



    \
     
     \ /
      Last update: 2011-10-24 18:01    [W:0.029 / U:211.020 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site