On 07/20/11 17:04, Jonathan Cameron wrote: > On 07/20/11 16:01, Michael Hennerich wrote: >> On 07/20/2011 03:28 PM, Jonathan Cameron wrote: >>> Hi All, >>> >>> The cc list is based on who turns up a lot in my in box so please >>> forward to any other interested parties! >>> >>> It's time we updated our todo list and started moves towards >>> leaving staging. Looking at it the todo is way out of date >>> so I'll just start from scratch. >>> >>> First purely administrative change is to push elements of this >>> TODO down close to the drivers. There are now way to many to >>> document sensibly in one place. I propose the following structure >>> >>> iio/TODO with stuff that applies to all drivers. >>> iio/accel/TODO for general accel stuff. >>> iio/accel/TODO-lis3l02dq etc for individual drivers. >> That makes a lot of sense. >> >>> Anyhow we'll leave drivers for now. Lets consider what >>> we might have in top level TODO >>> >>> Obvious and still required. >>> 1) Review. >>> 2) Testing. >>> >>> Big sections: >>> >>> * Core >>> ( ida cleanups, though that's not sensible to put in the TODO file. >>> Basically it's a case of rolling all 'device id' uses of ida's into >>> a central library with a single lock so as to avoid the huge amount of >>> boiler plate code.) >>> >>> * Buffering >>> 1) Replace or get significant review of sw-ring. As I've mentioned many >>> times I really don't like my code. >>> 2) More options. >>> 3) Documentation. Arnd suggested a man page given unusual nature of the chrdev. >>> Our main docs probably need a thorough read over and possibly updates as well. >>> >>> * Triggering >>> Post the locking / registration fix set, not sure we want to do more in here? >> Does it address the potential bug in case triggers happen faster than consumers can take? > No. That's another one for the todo list. Can you give an example of a device set on > which it still occurs? > > I think I'm right in saying this will only effect devices that schedule. Anything else > should be a ONESHOT threaded interrupt and hence the incoming interrupt is masked. The > trick is probably just to do manual masking of the interrupt in the driver. > > The other issue I know of is tight looping in dataready interrupts. Some of them on > try_reenable do a level check to see if they missed an interrupt, if they did they > redo whatever should clear it (typically data read). If that's not quick enough the > individual drivers should fail. Under those conditions I'm not sure we shut the trigger > down cleanly... > >> >>> * Events >>> Post making the fixes Arnd suggested and chasing down allocation / freeing issues >>> as a result of refactoring in my recent RFC. >>> >>> 1) Event codes need rethink. Michael already has a device where he ran out of space. >>> Perhaps we just make the code 64 bit right now. It actually costs us almost nothing >>> given padding of the event structure. >> Also the scan mask stuff is limited to 31 channels. >> Switching to bitmaps using DECLARE_BITMAP, __clear_bit, __set_bit and friends >> will help here. Not sure if we really want to carry this channel limitation out of staging. > Agreed. Sensible to update that now. Do you want to propose a patch or shall I? >> >>> 2) Replace the event handling code. It's simplistic and single user. Is this an issue? >> I discussed similar things with Manuel recently. A user space iio daemon could easily >> gate the single user buffers for multiuser purposes. > Certainly possible. So shall we say we want to go that way? > > Other bit I forgot here is adding poll support to events. That's fairly trivial but it's > not there now. > >> >>> 3) Consider matching event structure format with input's evdev? >> I think the iio to input event gateway is an important thing. >> There can be various ways to do that, however I think doing it in user space by >> translating events and injecting them back into the input queue is not a good idea >> considering the additional latencies. > Agreed. My main issue here is that the events from interrupt are pretty restricted, > so translation at some level would still be needed. Also, remember our data stream > is completely separate, so needs translation into input events as well. > > I have thought about just adding a mask that says 'these channels are input' from > a board config file then registering the input device directly from within IIO. > (basically hooking right in at the core level). >> >> A lot of accelerometer drivers make their way into linux-input. >> Starting with Android 2.3 GINGERBREAD, the API adds support for several new sensor types, >> including gyroscope, rotation vector, linear acceleration, and barometer sensors. >> >> Not sure if they also will go into input. > Some will, but arguing a barometer is human input would be 'interesting'. >> >>> * sysfs attribute names. >>> 1) Consider dropping 'compatibility' with hwmon to allow more consistency. in -> voltage >>> for example. >>> 2) Have a prefix to specify direction. So in_voltageX_raw out_voltageX_raw. Michael >>> brought this up. If we are going to do it, now is the time. We may need a >>> 'compatibility mode' to smooth the transition. Note however that we don't want to >>> carry that mode out of staging. (do we also need an inout option?) >>> >> Yes - that's a good thing todo. >> In addition we should also make sure that buffers may function in both directions. > Hmm.. The actual implementation would at least initially be completely separate. I think > the abi would certainly allow this as is though. Whilst I agree this would be useful, we > don't have an implementation and it seems non trivial. Last time we talked about it, > I got the impression quite a lot of bus level stuff had to be added before this > could actually be useful? > > Lets audit the abi to make sure this can be done in future though. > >> >>> The next big discussion is how to propose a patch set moving out of staging. This is >>> hopefully where we'll start to get more feedback. >>> >>> I propose the following: >>> >>> 1) Move the namespace of key exported bits and bobs to iio_st* (for staging) >>> 2) Add a prefix to drivers in kconfig (as we move them); >>> IIO_ seems the obvious choice. >>> >>> That leaves us clear to lift code across. How about a series that looks like: >>> >>> 1) core sysfs only infrastructure + docs. >>> 2) A few example drivers >>> ---review period--- >>> 3) Event support + docs >>> 4) A few example drivers >>> ---review period--- >>> 5) Core buffer support + docs >>> 6) Hardware buffer example (sca3000 given I have one). >>> 7) Software buffer example (kfifo first as it's easier to review). >>> 8) A few example drivers. >>> ---review period--- >>> 9) Sets of clean drivers >>> continue till we run out. >> That sounds like a plan. >> >>> Note some drivers will probably be in staging for quite >>> a large period - ultimately some need a complete rewrite. >>> >>> As we go along I suggest we try and keep the staging tree in sync with >>> any changes. At the end, I'd like to drop the core from the staging >>> tree if possible and just have all remaining drivers use the new core >>> implementation whether they are in staging or not. >>> >>> The only other option that really makes sense is to do the events >>> and buffering in the opposite order. >>> >>> So we need to figure out: >>> >>> a) are we happy with this order. >> No strong p >>> b) which drivers are we taking at each stage. >> I think we decide on this when we get close to the next stage. > Makes sense. >>> c) how to post the changes (linux-iio first then lkml after a week?) >> Sure >>> For the which drivers, the set I will personally carry are (listed by where >>> they are updated in the above tree). >>> >>> 2) max1363, adis16400, tsl2563 >>> >>> 4) max1363, tsl2563 (adis16400 doesn't actually have support for events >>> in tree) >>> >>> 6) sca3000 >>> 8) max1363, sysfs trigger. >>> >>> I'd certainly like to take a few of our other nice drivers along for the >>> ride but I can't test them. I'm not taking the lis3l02dq for now despite >>> it being one of my core test drivers, because it will cause issues given >>> the misc/lis3 driver. >>> >>> So what do people want to carry through the process? >> Given the fact that we only need a handful of good examples for >> the initial review process. I think I can take care of two or three >> ADxxxx going forward. > Excellent. > > Just to see how hard it would be I've hacked 1 and 2 from above > carrying the max1363 for now. Looks fine. I've left a few elements in > at the driver level that aren't used to reduce silly churn, but commented > them as such in the code (to hopefully keep reviewers quiet!) > > I'll hold that branch for now as we need to do the two changes above. > (in/out naming - which is afterall a big abi change and larger masks / events). > > I'll do the in out naming now and propose it as an RFC. We can have > a compile time 'compatibility' option if enough people should loud enough. I've actually taken a look at how input actually passes events around and I'm less sure this is a good idea for our main event path. Timestamps are generated very late (in evdev). The form a simple type, code, value set. The value wouldn't really make sense for what we have in iio events, leaving us with a pair of 16 bit numbers, code would probably have to be the channel number (or combination of two in differential channels) and include the modifier in there. Probably split as: Differential channel: unsigned channel:8; unsigned channel2:8; Unmodified channel unsigned channel:16; Modified channel unsigned channel:8; unsigned modified:8; So 65536 channels non differential, 256 for each end of differential, 256 modified channels with 256 possible modifiers. We could allocate the type as: unsigned modified:1 unsigned direction:3; unsigned type:4; unsigned channel_class:8 This is a bit of a weird split, but there we are. Ultimately I'm not convinced this is worth doing given we'll still need a translation layer (unavoidable as inputs events are too human input directed and frankly simplistic)). Perhaps we just assume our own event structure and move to 64 bits? > >> >>> Mostly it will involve stripping elements out and then slowly building things >>> back up again as the support becomes available. One important >>> point I'd like to do is remove the IIO_CHAN macro and do things >>> explicitly. Partly that macro is becoming restrictive and confusing >>> + it will make for a much cleaner build up of support. >>> >>> All comments welcome! >>> >>> Once a consensus has formed *cross fingers*, I'll do a clean >>> version to post more widely so others are kept in the loop. >>> >>> Jonathan >>> >>> >>> >>> -- >>> 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 >>> >> >> > > -- > 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 > -- 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
