Lars-Peter Clausen <lars@xxxxxxxxxx> wrote: >On 01/02/2012 06:25 PM, Jonathan Cameron wrote: >> This came up whilst I was taking a look at Pirmin Duss' driver the >other day. >> When we originally created these the attribute groups were passed >directly >> through the core and registered with sysfs. Since the introduction >of >> iio_chan_spec structures this is no longer true. Their elements are >simply >> coppied into attribute_groups created by the core. >> >> Hence, what other reasons are there for having these as attribute >groups? >> The only one I can see is the availablity of the is_visible callback. >> >> Now I've always had mixed feelings about that one. It's handy on >occasion >> but mostly gets used to handle variations across the different >devices >> that are supported by a given driver. This is true of ad7192, >ad9834, >> ad5446 (indirectly). These can all be easily unwound and handled at >a >> higher level (which iio_info varient we use for the device instance). > >This sounds sensible. > >> >> The ad7606 is the only more complex use case here. There we have >> attributes whose visibility is dependent on some gpios being >specified >> in platform data. There are two different sets so we have a total of >4 >> resulting iio_info structures. Not too bad I suppose. > >But this is getting a bit ugly in my opinion. Maybe we can add a attrs >fields to the iio_dev struct to handle these. This would also allow us >to >allocate attributes at runtime if necessary. You are right. These do seem like they are too dynamic to be in iio _info. I will make that change as an additional patch. > >> >> A side effect of this is that we can remove the code Lars-Peter just >added >> to copy the is_visible callback over. > >Hm, while I have such a patch locally I don’t think I've send it out >yet. > >> That copy is a little odd anyway >> as it is applied to far more attributes than are initially visible. >whilst >> all current drivers do a 'is condition true then mask attribute' we >only >> need one to do the reverse logic to get nasty to find bug. >> >> Note this set is on top of Lars-Peters recent rearrangement of the >event >> code as obviously the last patch here edits code he moves. > >- 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 -- Sent from my Android phone with K-9 Mail. Please excuse my brevity. -- 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