Re: [PATCH 2/2] iio: proximity: vl53l0x-i2c: Added continuous mode support
From: Abhash jha
Date: Sat Aug 31 2024 - 12:42:52 EST
> Also, consider if other triggers could be used as if not you need to
> both document why and add the validation callbacks to stop other triggers
> being assigned (once you've added one that can be!)
>
> Feel free to ask if you have more questions, but your first reference
> should be other drivers (and I hope we don't have any that do it this way).
>
I used this driver as a reference
https://github.com/torvalds/linux/blob/master/drivers/iio/adc/vf610_adc.c#L556
>
> ouch, not a write 1 to clear register? I can't find docs, but this is really
> nasty bit of interface design if you have to toggle the bit.
>
Actually ST has not provided a register map or any application note
for the sensor.
So there's no way to cross reference. Hence I kept the original code.
But I will try to write 1 to clear register with my sensor.
> > + ret = devm_iio_triggered_buffer_setup(&client->dev,
> > + indio_dev,
> > + &iio_pollfunc_store_time,
>
> This is odd. You don't seem to have a function to be called to actually store
> the data. Note you also need to consider if other triggers might be used.
>
> I'm not sure what reason we have to do that here though as this is a very
> conventional one interrupt per 'scan' of data device.
>
> So you should be registering a trigger, and a buffer then letting the
> trigger drive the buffer.
Why do I need to register a trigger? Would it be fine to let the irq
fill the buffer
with data as it continuously reads it in the poll function?
So according to my understanding,
I need to move all the data reading and pushing to the poll function
and not do it in the irq handler.
Then register that poll function here during iio_triggered_buffer_setup.
Is there something that I am missing?
Regards,
Abhash