Before I dive blindly in please note I have only taken a quick look as away from a computer with a decent size screen until tomorrow evening. I'll take a closer look at the driver then. On 09/02/10 23:41, Alan Cox wrote: > On Thu, 2 Sep 2010 15:16:20 -0700 > Andrew Chew <AChew@xxxxxxxxxx> wrote: > >>>> +static IIO_DEVICE_ATTR(mode, S_IRUGO | S_IWUSR, show_mode, >>> store_mode, 0); >>>> +static IIO_DEVICE_ATTR(magn_x_calibscale, S_IRUGO, >>> show_calibscale, NULL, 0); >>>> +static IIO_DEVICE_ATTR(magn_y_calibscale, S_IRUGO, >>> show_calibscale, NULL, 1); >>>> +static IIO_DEVICE_ATTR(magn_z_calibscale, S_IRUGO, >>> show_calibscale, NULL, 2); >>>> +static IIO_DEV_ATTR_MAGN_X(show_raw, AK8975_REG_HXL); >>>> +static IIO_DEV_ATTR_MAGN_Y(show_raw, AK8975_REG_HYL); >>>> +static IIO_DEV_ATTR_MAGN_Z(show_raw, AK8975_REG_HZL); >>> >>> This seems odd as an interface as it's raw when the maths to provide >>> non-raw (and thus abstract and easy for user space) data is trivial >>> enough to do in kernel >> >> IIO guys want to do normalization maths above the kernel-level magnetometer IIO layer. >>This interface came before me, so I'm just following current conventions. > > That will make a generic IIO <-> input bridge very hard to do right. I > can see why IIO wants to do that but you need both if so and this also > needs discussion. Note that, though there are few drivers using it (just the tsl2563 iirc) we do allow the hwmon style _input syntax for values in the standard units for the device type. If you are going to use the raw option then, assuming linear conversion, at the very least magn_scale should be provided to tell userspace how to do the conversion. If the bridge to input is done in userspace then it is trivial to apply this before passing into uinput. For background info, the bridge approach currently relies on implementation of the 'buffered' iio interface which uses chrdevs rather than the hwmon like 'simple' interface provided here. It would be a bit of a nightmare to bridge from sysfs (if it could even be done?) Typically all drivers start with the sysfs interface and the buffered one is only added if needed by someone. Note calibscale is meant for internally applied values. That is ones applied actually on the device (looking at what you have here, my choice of naming wasn't ideal, anyone have a better naming suggestion). The value used to do raw to standard unit conversion in userspace is magn_x_scale or equivalent. I think based on the comment in your code that you also need to provide magn_x_offset. Looks like our documentation could do to be a little clearer. Rather tediously these are both computed floating point values here. Perhaps we will need to think further on that interface (this is the first time we have hit this particular situation) The raw option is principally there because we share the attributes with the buffer access (there performance dictates doing as little as possible to the data on the way through.) Admittedly our overheads are way too high at the moment for other reasons but the intent is there. If you are intending to add this interface later then keeping to the _raw attributes makes for a more consistent choice (though as long as the scale and bias are there for the buffer you could use the processed value...) Thanks, Jonathan -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
