linux-kernel - Re: [PATCH 2/2] iio: proximity: vl53l0x-i2c: Added continuous mode support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAG=0RqLfjPF2_ebuabo0_ddvRxZkTLmKUAXQZWYHRaKxDaOkew@mail.gmail.com>
Date: Sat, 31 Aug 2024 22:12:27 +0530
From: Abhash jha <abhashkumarjha123@...il.com>
To: Jonathan Cameron <jic23@...nel.org>
Cc: linux-iio@...r.kernel.org, songqiang1304521@...il.com, lars@...afoo.de, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] iio: proximity: vl53l0x-i2c: Added continuous mode support

> 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