On 10/12/2013 02:02 PM, Jonathan Cameron wrote: > On 10/07/13 15:11, Lars-Peter Clausen wrote: >> Register the event threshold hysteresis attributes by using the new >> IIO_EV_INFO_HYSTERESIS event spec type. This allows us to throw away a good >> portion of boiler-plate code. >> >> Signed-off-by: Lars-Peter Clausen <lars@xxxxxxxxxx> > > Applied, as here as it is a great improvement. > > I do wonder if we can handle the EITHER direction of the event more > cleanly, now we have the different event masks. > > Maybe that is even uglier than the current option. > > What do you think? [...] >> + }, { >> + .type = IIO_EV_TYPE_THRESH, >> + .dir = IIO_EV_DIR_EITHER, >> + .mask_separate = BIT(IIO_EV_INFO_HYSTERESIS), > This is a little ugly, but I can see why you did it. Do we need to allow an additional > event_mask, perhaps > mask_shared_by_both_directions ? > > Then we could drop the IIO_EV_DIR_EITHER direction entirely. Yes, you are right this is not exactly beautiful. I had to struggle with myself a bit to convince me that it could go in as it is. I've tried a few alternatives, but couldn't really find anything better. The problem is that the setting whether we want to share the attribute by event direction and e.g. by channel type are orthogonal. It is possible that an attribute is shared by both event direction and channel type. That's why a separate event mask doesn't work. The other two options I saw were: 1) Encoding the direction directly in the bit mask like we did before with the old style event mask and the IIO_EV_BIT() macro. E.g. something like: .mask_separate = IIO_EV_BIT(IIO_EV_DIR_EITHER, IIO_EV_INFO_HYSTERESIS) The problem with this approach is that it takes up a lot of space in the mask and limits the number of info attributes we can have. 2) Have one entry in the event spec array for each attribute e.g. { .type = IIO_EV_TYPE_THRESH. .dir = IIO_EV_DIR_EITHER, .info = IIO_EV_INFO_HYSTERESIS, .shared_by = IIO_SEPARATE, } This again takes up extra space considering that the type and direction are usually shared between the event attributes. So the current approach is kind of a hybrid between the two. It allows you to have multiple entries when necessary, but also allows you to group similar attributes together. - Lars -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html