On Sun, May 29, 2016 at 7:14 PM, Jonathan Cameron <jic23@xxxxxxxxxx> wrote: > On 28/05/16 09:32, Gucea Doru wrote: >> On Fri, May 27, 2016 at 6:15 PM, Crestez Dan Leonard >> <leonard.crestez@xxxxxxxxx> wrote: >>> On 05/26/2016 03:23 PM, Doru Gucea wrote: >>>> In order to support software triggers (e.g.: hr-based trigger) the >>>> operations from trigger enable/disable are moved to buffer enable/ >>>> disable. >>>> >>>> This also allows us to remove the mutex from inv_mpu6050_read_fifo: >>>> inv_reset_fifo can't access in parallel shared fields because the >>>> buffers must be enabled prior to reading data. >>>> >>>> A benefit of removing the mutex is that we avoid a deadlock at >>>> buffer wqdisable time. The scenario was the following: >>>> (1) at buffer disable time the indio_dev->mlock is acquired >>>> (2) free_irq is called and waits for IRQs completion >>>> (3) inv_mpu6050_read_fifo tries to acquire the indio_dev->mlock >>> >>> I'm not sure this is safe, it seems to me that inv_reset_fifo assumes >>> mlock is taken and this needs to be assured. I think properly removing >>> the irq disabling deadlock require either using something other than >>> mlock or not calling inv_reset_fifo from the interrupt at all. The >>> latter seems preferable to me. But this issue should be handled >>> separately from software trigger support. >>> >> >> inv_reset_fifo still needs to be called: it might happen that we miss a >> DATA_RDY interrupt in which case we'll have a mismatch between >> time-stamps and bytes from the FIFO queues. The only way to recover >> from such a situation is to flush the FIFO's and restart the interrupt. >> >> Regarding the locking, before my patch, inv_reset_fifo could flush the >> FIFO's (see inv_mpu_data_rdy_trigger_set_state) while >> inv_mpu6050_read_fifo was reading data and the role of the mlock was >> to avoid this situation. This situation is not possible anymore because >> the operations from trigger enable/disable were moved to buffer >> enable/disable: trigger state can't be changed while buffers are enabled. >> Do you see another scenario? >> >> I'll split the software trigger support and deadlock solution in two patches. >> >>> Another issue is that the fifo is filled at the sampling frequency >>> configured in device registers and this can be different from the >>> sampling frequency configured on the software trigger. It seems to me >>> that the user needs to manually ensure that they are identical. >>> >> >> A solution would be to hook the iio_hrtimer_info_store_sampling_frequency >> method of the hrtimer for setting the sampling frequency together with >> the timer frequency. I think this will require changes to the hrtimer for >> exposing the above function to drivers. Daniel, what would be the right >> approach? > Back to basic questions from me ;) > > Why do you want to use a software trigger here at all? Trying to align the two > sampling frequencies is really a non starter - it'll never work however hard > you try. So I am guessing you want to support sampling when the interrupts > are not wired up? If so the only way to handle that is to use a thread > to sample the interrupt status register fast enough to get a reasonable idea > of when it changed... basically fake the interrupt wire. > Yes, I was trying to use a software trigger because I have a chip without GPIO support. I followed the model from BMC150 accel that allows installing a software trigger even if there is no interrupt line connected. At this moment, the probe for MPU60X0 fails if there is no interrupt line connected. Another reason for using a sw trigger was for masking the USB latency because the sensor is connected to an USB to SPI adapter while a hardware interrupt would have eaten some bandwidth. Sampling once the interrupt status register will need one USB TX operations and one USB RX operation. This is expensive compared to a hardware interrupt that will need only a USB RX operation. However, I could send a rework for this patch that uses a thread for sampling the intr register if you consider that someone needs it. > If you want to be able to run this device on say a much slower clock then we > need to look at how to bypass the fifo entirely (read from the relevant sensor > registers instead I think...) > > > > > > >> >>> Even if they are identical the timer ticks might not match perfectly. >>> When reading from the fifo all available samples are handled. >>> >>> If device frequency is set too high you will push multiple samples to >>> userspace, all but one of them with bad timestamps. It's also possible >>> to find zero samples if device frequency is set higher than timer frequency. >>> >> >> In case of timer ticks mismatch there will also be a mismatch between >> time-stamps and bytes from the FIFO queues. I wonder if a clean recovery >> from such a situation would be to call inv_reset_fifo together with resetting >> the hrtimer. > This is going to happen frequently - how frequently will depend on the drift > between the two clocks. Not a lot of point of having a high frequency sampling > system that resets every few seconds dropping some samples on the floor. > > Jonathan >> >>> -- >>> Regards, >>> Leonard >>> -- >>> 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 >> >> Regards, >> Doru >> > -- 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
