Re: [PATCH] IIO: trigger: New hrtimer based trigger driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2011-03-10 03:31, Jonathan Cameron wrote:
Obviously run checkpatch over it at some point.
Will be done.

Otherwise, nice clean little trigger driver.  After cleanup and
testing I would be happy to see this in IIO.  Never realised it
would be this simple :)
Looking over your comments I realized I forgot to pull over one part (or rather, thought I didn't need it but I do), which makes it a little bit more complex..

hrtimer callbacks are sent in hardirq context, which was OK in my driver, but as some IIO things involve SPI/I2C communications etc it needs to be moved over to a softirq setup. I will look into getting this fixed for a v2 patch.

+static enum hrtimer_restart iio_trig_hrtimer_trig(struct hrtimer *timer)
+{
+	struct iio_hrtimer_trig_info *trig_info = container_of(timer, struct iio_hrtimer_trig_info, timer);
+
+	iio_trigger_poll(trig_info->trigger, 0);
Out of curiosity, how suceptible to drift is this?  I guess the timer doesn't take
into account the amout of time this function takes, e.g how late are we by the time
the timer is restarted?
This function is missing one line that updates the expiry time of the timer.. and the new expiry time is calculated based on the old time which means it should not drift at all. It can however "miss ticks" if the trigger poll takes longer than the interval.

+
+	trig_info->frequency = val;
Only applied on trigger being stopped and restarted I think?
Maybe return -EBUSY if the trigger is running to make this
obvious to userspace.
Yes, I was unsure how to handle this properly.. I think I can make it update it properly if it is running though.

I'll have a v2 patch out in a couple of days, still untested as seems to be a few weeks before I get the second spin of our board on which I can run it.

Regards
Marten Svanfeldt
--
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


[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux