Re: [RFC PATCH 0/4] staging:iio:adis16350 various updates and fixes

[Jonathan Cameron <jic23@cam.ac.uk>]

On 09/06/10 10:00, Manuel Stahl wrote:
> Am 06.09.2010 00:20, schrieb Jonathan Cameron:
>>
>> Patch 3 puts in event support.  It's complex, but then what the
>> device has some complex abilities. Still if anyone can see any
>> simplifications without breaking the interface I would definitely
>> like to hear them!
> 
>> @@ -36,3 +36,9 @@
>>
>>  #define IIO_EVENT_CODE_IN_HIGH_THRESH(a) (IIO_EVENT_CODE_ADC_BASE  + a)
>>  #define IIO_EVENT_CODE_IN_LOW_THRESH(a) (IIO_EVENT_CODE_ADC_BASE  + a + 32)
>> +#define IIO_EVENT_CODE_IN_HIGH_ROC(a) (IIO_EVENT_CODE_ADC_BASE  + a + 64)
>> +#define IIO_EVENT_CODE_IN_LOW_ROC(a) (IIO_EVENT_CODE_ADC_BASE  + a + 96)
>> +#define IIO_EVENT_CODE_IN_SUPPLY_HIGH (IIO_EVENT_CODE_ADC_BASE + 97)
>> +#define IIO_EVENT_CODE_IN_SUPPLY_LOW (IIO_EVENT_CODE_ADC_BASE + 98)
>> +#define IIO_EVENT_CODE_IN_SUPPLY_ROC_HIGH (IIO_EVENT_CODE_ADC_BASE + 99)
>> +#define IIO_EVENT_CODE_IN_SUPPLY_ROC_LOW (IIO_EVENT_CODE_ADC_BASE + 100)
> 
> What about putting some more structure in the event codes.
> Maybe <type_id> <channel> <event_id> each in it's own byte, where
> <type_id> is one of accel, gyro, ...
> <channel> is one of x, y, z
> <event_id> is one of high_thresh, low_thresh, ...
> maybe even a defined bit for high or low, one for thresh or roc, ...
> 
> Then we don't need a big event table, but can figure out all parts independently.
Good idea.  Lets merge this driver and any others in this round as we have it
then propose a change to something more organized that we can then apply to all in
tree drivers at the same time.  This won't effect Analog's drivers much as they
haven't made extensive use of the event interface as yet.

Perhaps we want to open this one up to more general opinions (lkml)?  It's a big
change and we always said we would keep posting them there.
> 
> Could we generate some of the event attributes in sysfs on the fly?
> Like have an attribute alarm0_input, where we can write in a list of
> channels and then only generate the limit attributes for these
> channels.
Not terribly generalizable.  We basically have the same issue we had with
devices with limited scan options (max1363). We have about half of devices
with alarms on every channel and half like this one that have a small finite number
of alarms that we can connect to a given device.  I really don't want to go
towards having two different interfaces for the two cases (like we had for
a while with the scan mode stuff).  The cost here is lots of attributes,
but I don't think it is too bad and I doubt we'll ever have any devices
that are significantly worse than this one... With the macro usage it doesn't
lead to all that much code.

Jonathan