On Fri, 6 Sep 2019 22:03:52 +0530 rishi gupta <gupt21@xxxxxxxxx> wrote: > Hi experts, Hi Rishi, > > I am writing driver for veml6030 ambient light sensor. Application can > set persistence, gain & threshold which will in turn update registers > in sensor. > > In IIO framework there is no standard constant for this. So first element here is whether there aren't standard ABI elements for these. A quick look suggest threshold maps to an IIO event, so look at that ABI. Persistence - I'm guessing as the datasheet I have doesn't actually explain what it is... Probably number of cycles for which the value has to be over the threshold for an interrupt to occur? This maps to period, though you'll have to do the magic to convert form cycles to seconds. Gain maps to 1/Gain == scale in IIO terms. What userspace cares about is what maths it needs to do to get a real world value and that is given by scale. Here you have the complexity of integration time also feeding into that value so it's not that straight forward. Think about what combination makes sense for light sensor uses... Gain typically increases noise, integration time decreases it. Otherwise they both effect the scale. So various options exist: 1) Just provide scale and pick sensible combinations to give low noise and reasonable integration time for a given scale. 2) Map scale control just to gain, but the values it takes will change as the integration time is changed. >From an IIO ABI point of view either is fine. > > 1) Does using sysfs entries will be fine or there is better alternative. > > static IIO_DEVICE_ATTR_WO(gain, 0); > static IIO_DEVICE_ATTR_WO(threshold, 0); > static IIO_DEVICE_ATTR_WO(persistence, 0); > static IIO_CONST_ATTR(persistence_available, "1 2 4 8"); > static IIO_CONST_ATTR(gain_available, "0.125 0.25 1 2"); > > static struct attribute *veml6030_attributes[] = { > &iio_const_attr_gain.dev_attr.attr, > &iio_const_attr_threshold.dev_attr.attr, > &iio_const_attr_persistence.dev_attr.attr, > &iio_const_attr_persistence_available.dev_attr.attr, > NULL > }; If we do need to define things for a particular device that aren't generic enough to add to the main ABI then this approach is one option. However, it makes them only available to userspace. May be fine for this sort of device, but in general the use of iio_chan_spec_ext_info is preferred. Here however, I think the examples all map to existing ABI. > > 2) Can IIO_CHAN_INFO_HARDWAREGAIN can be used for gain instead of > custom sysfs file. > > Datasheet: https://www.vishay.com/docs/84366/veml6030.pdf Nope. Hardware gain is normally used for things like time of flight sensors, where it is changing the gain of something that is only indirectly connected to the thing being measured. In those cases it lets them measure at larger distances but doesn't make any difference as long as the measurement is successful. > > Regards, > Rishi Thanks, Jonathan