On Fri, May 6, 2016 at 1:32 PM, Crestez Dan Leonard
<leonard.crestez@xxxxxxxxx> wrote:
> On 05/06/2016 11:52 AM, Linus Walleij wrote:
>> commit 98ad8b41f58dff6b30713d7f09ae3834b8df7ded
>> ("iio: st_sensors: verify interrupt event to status") caused
>> a regression when reading ST sensors from a HRTimer trigger
>> rather than the intrinsic interrupts: the HRTimer may
>> trigger faster than the sensor provides new values, and
>> as the check against new values available as a cause of
>> the interrupt trigger was done in the poll function,
>> this would bail out of the HRTimer interrupt with
>> IRQ_NONE.
>>
>> So clearly we need to only check the new values available
>> from the proper interrupt handler and not from the poll
>> function, which should rather just read the raw values
>> from the registers, put them into the buffer and be happy.
>>
>> ---
>> ChangeLog v1->v2:
>> - v1 was missing timestamps since nothing ever stamped them.
>> Restore the timestamps by simply using
>> iio_trigger_generic_data_rdy_poll()
>> as the top half of the threaded interrupt handler.
>
> iio_trigger_generic_data_rdy_poll always returns IRQ_HANDLED. This means
> that the threaded handler is never called.
>
> I think the correct solution would be to call the buffer's hardirq
> handler from the trigger's hardirq and the buffer's threaded handler
> from the trigger's threaded handler, right? I don't know how to do this
> properly with iio. Having "iio_poll_func" create an irq is very strange.
Yeah checkout my v3 of the patch, hope it clears up.
>> /*
>> * If this was not caused by any channels on this sensor,
>> * return IRQ_NONE
>> */
>> + if (!indio_dev->active_scan_mask)
>> + return IRQ_NONE;
>
> This seems odd. I'm not sure it's even possible for active_scan_mask to
> be NULL, maybe you wanted to check if the mask is all-zero instead? Can
> this really happen?
Yes it happened to me, in the short time between installing the interrupt
handler and registering the sensor there can be a spurious interrupt
(because of static on the line or whatever).
> And if you're going to do such a check you should do it before reading
> the status anyway to avoid the pointless read. It's worth minimizing bus
> operations.
It's fixed in the add-on patch that looks for extra incoming samples,
check out the combo of the two patches I sent, I think together they
make for a pretty elegant solution.
Yours,
Linus Walleij
--
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
