On Thu, 26 Apr 2018 09:46:18 -0700 Martin Kelly <mkelly@xxxxxxxx> wrote: > On 04/26/2018 12:35 AM, Jean-Baptiste Maneyrol wrote: > > > > > > On 25/04/2018 20:06, Martin Kelly wrote: > >> On 04/06/2018 09:33 AM, Martin Kelly wrote: > >>> On 04/06/2018 08:41 AM, Jonathan Cameron wrote: > >>>> > >>>> > >>>> On 6 April 2018 16:21:05 BST, Jean-Baptiste Maneyrol > >>>> <JManeyrol@xxxxxxxxxxxxxx> wrote: > >>>>> Hello, > >>>>> > >>>>> > >>>>> there is just a problem if I'm understanding well. > >>>>> > >>>>> > >>>>> Reading FIFO count register under hard irq handler (when taking the > >>>>> timestamp) is not possible since i2c access is using a mutex. That's > >>>>> why we are using an irq thread for reading FIFO content. > >>>> > >>>> Good point. Need more sleep or caffeine! > >>>> > >>>> > >>> > >>> I was about to reply with the same, as I started coding it up :). Too > >>> bad, it was such a great plan! > >>> > >>> I have a little update: When switching to level triggered interrupts, > >>> the problem goes away for me, as do the bus errors I get at high > >>> frequencies. I'm working on a patch to support other interrupt types > >>> than rising edge, which is almost done. > >>> > >>> I also intend to look again at the bus errors for edge driven > >>> interrupts. Since they happen only at high frequency and go away with > >>> level driven interrupts (which must be acked and therefore prevent > >>> reentrancy), I suspect there's a concurrency bug. > >>> > >>> That said, I think the question remains: Since we can't get the FIFO > >>> count from the hard IRQ handler, and since it could be some time > >>> before the bottom half thread is scheduled, I don't see any way to > >>> accurately handle forward interpolation. > >>> > >>> Though we can't do forward interpolation, I think at least having > >>> backward interpolation is better than filling in blank timestamps, > >>> especially as we haven't seen an actual case of forward interpolation > >>> being needed, while we have real use cases that require backward > >>> interpolation (and can be easily verified in a logic analyzer). > >>> > >>> However, that's only my opinion. Jonathan, Jean-Baptiste, and others, > >>> what do you think? > >>> > >> > >> Hi, > >> > >> What can I do to help get closure on this? Is there any data I could > >> gather that would help make a decision? > >> > >> In cases of a troubled system, I think the interpolation is very > >> useful, and it's awkward to do in userspace, as it involves keeping a > >> history of past records, which can be inconvenient and not always > >> accurate (e.g. if userspace doesn't read fast enough and we miss > >> records). However, I certainly understand the concern about > >> interpolation. Should we consider making the interpolation > >> configurable via sysfs or device-tree? > >> > >> I'd be happy to hear other ideas too about better handling the case of > >> missed interrupts. > > > > Hello, > > > > I have implemented a new timestamp mechanism that do something similar > > but in a more precise way. > > > > The main ideas are: > > * check if interrupt timestamp is valid (computes interrupt timestamps > > interval and check against set period value with a margin) > > * use validated interrupt timestamps to measure chip internal period > > from the system (to have more accurate computed timestamp value) > > * use the interrupt timestamp for data if it is valid > > * if interrupt timestamp is invalid (meaning interrupt was delayed or > > missing), computes timestamp using the measured chip period > > > > I will send the corresponding patch series as soon as my last clean-up > > series has been accepted by Jonathan. > > > > Regards, > > JB > > Excellent, I look forward to the patches. Jonathan, are you OK with this > design approach? It does sound a rather complex solution, but should be accurate. We'll see when the patches are out, but it will probably do the job given I think we have concluded we want to hide this from userspace as much as possible. Thanks, 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
