On 15/06/15 19:14, Srinivas Pandruvada wrote: > On Sun, 2015-06-14 at 18:57 +0100, Jonathan Cameron wrote: >> On 14/06/15 18:25, simon@xxxxxxxxxxxxx wrote: >>> Thank you for your comments, I'm just getting started with IIO so it's all >>> good stuff... >>> >> Srinivas, cc'd you as a few queries came up about the hid-sensors driver. >> (it's got me thoroughly confused ;( >>>>> +++++++++++++++++++++++++++++++++++++++++++++++++ >>>> Hmm. This driver is already a substantial mash up of a number of different >>>> types of driver (as far as linux is concerned), with input, battery and >>>> led >>>> drivers. Might be worth considering a more formal MFD approach as it'll >>>> break the driver up into a number of sub components that will then sit >>>> in the various subsystems in a cleaner fashion. >>>> Just a thought! >>> >>> Might be, but that sounds like more work ;-) If pushing some ideas around >>> prompts that, it can't be a bad thing.... right? >>> >>>>> + case IIO_CHAN_INFO_SCALE: >>>>> + switch (chan->type) { >>>> If the scale really is 1 then don't export it. Note the units would >>>> have to be in m/s^2 which seems unlikely though. I'm guessing this >>>> is a placeholder.. >>> >>> Yes place holder, different controllers (SixAxis, DS4 and PSMove) have >>> different values. In the far future I'd hope to use the per-device >>> calibration stored on the PSMove. >>> >>> https://github.com/nitsch/moveonpc/wiki/Calibration-data >>> >>>>> +static const struct iio_chan_spec sony_sixaxis_channels[] = { >>>>> + SONY_ACC_CHANNEL(X), >>>>> + SONY_ACC_CHANNEL(Y), >>>>> + SONY_ACC_CHANNEL(Z), >>>> No gyro channels yet? >>>> Just to note, if the gyro frequency etc is different from the >>>> accelerometer >>>> (pretty common) then you'll want to register two IIO devices rather than >>>> just the one so that the control and buffers are separate. >>> >>> I have a vague memory that the SixAxis has a 1-ch gyro but this is not >>> showing on hidraw (might be 'hidden' behind MultiTouch ID/Bug). >>> >>> The DS4 has Accel/Gyros, and the PSMove has Accel/Gyro/Mag. I didn't >>> expose the mags over input/joystick axis as I didn't want to corrupt >>> stream the PSMoveAPI (it needs to be re-ordered) and it's unlikely anyone >>> would actually use via joystick. >>> >>> The PSMove's report actually contains 2 frames assumed to be 1/2 sample >>> rate apart for the Accel/Gyro, but only one Mag reading. >>> >>> >>> >>> I have further advanced the patch to include reading via buffer, but I'm >>> having trigger 'conceptual' problems getting my head around the HID device >>> issuing an interrupt when a input report is received. Looking at >>> iio_dummy_event and iios_sysfs for inspiration.... >> You can skip the triggers. It's not obligatory, you can push directly into a buffer. >> Triggers are nice for 'data ready' type signals (which is closest to what we >> have here) if you might want to hang other sensors off the timing (so read >> on demand ADCs etc. They aren't actually 'required' as such. >> >> Looking at the hid drivers I'm not entirely sure they are actually doing anything. >> (been a while since I looked at these in detail!). Srinivas, perhaps you could help >> out with what's going on there? >> >> I can't find an iio_trigger_poll call so the trigger is neither used by the >> hid sensors drivers, not usable by anything else as far as I can see.. >> Looks like it is used purely to get the device into the correct power state >> etc. If so that stuff should be in the buffer preenable / postdisable not >> the trigger stuff. >> >> I get the feeling that this might all be a work around for a perceived need >> for a trigger (particularly in userspace app examples?) as it predates >> the am355x driver where we absolutely said they weren't required and the >> userspace demo changes that were made to support that. >>> > The user space was developed using the iio user space demo. We could > have done with buffer pre/post enable. But triggers provided path for > enhancements. Fair enough in principle, I was just getting lost on whether any actual triggers ever occurred. That would require iio_trigger_poll to be called somewhere. If this isn't happening and the trigger is just used as a kind of internal placeholder then that is fine as long as there are validate functions so no other drivers think they can use the triggers. In particular the trigger validate_device callback needs to prevent this. > We are currently looking how camera buffer sample and > accelerometer data can be tied together by using trigger buffers. Even > one sensor data ready can trigger read on a companion senor on the hub. > > Thanks, > Srinivas > >>> On the assumption that there will be multiple devices (either same type or >>> with different HID drivers) all trying to issue triggers, we'd need to be >>> a little careful. >>> >>> Is there a 'short-cut' we can use if a HID device is only required to >>> trigger itself (and not other iio devices)? ie. not need true interrupt >>> system. >> Yes, you can use the 'validate_trigger' and 'validate_device' callbacks to >> reject either other devices trying to use the trigger, or your device trying >> to use a different one. Lots of drivers do this in the first place (as you >> point out it is a short cut to get things working) then if anyone cares >> relax the constraint later by making the true interrupt stuff work. >> >> >>> >>> Simon. >>> >>> -- >>> 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