Message-ID: <20250724133933.220d00e4@jic23-huawei>
Date: Thu, 24 Jul 2025 13:39:33 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: David Lechner <dlechner@...libre.com>
Cc: Akshay Jindal <akshayaj.lkd@...il.com>, anshulusr@...il.com,
 nuno.sa@...log.com, andy@...nel.org, shuah@...nel.org,
 linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] iio: light: ltr390: Add sysfs attribute to report
 data freshness

On Tue, 22 Jul 2025 16:15:55 -0500
David Lechner <dlechner@...libre.com> wrote:

> On 7/21/25 10:55 PM, Akshay Jindal wrote:
> > Thanks David for the swift and valuable feedback. Please find replies inline.
> > 
> > On Tue, Jul 22, 2025 at 1:55 AM David Lechner <dlechner@...libre.com> wrote:  
> >>
> >> For debugging, using debugfs would make more sense.  
> > Will it make sense if I simply give register access via debugfs?
> > Something like this:
> > static const struct iio_info ad7606_info_sw_mode = {
> >     .read_raw = &ad7606_read_raw,
> >     .write_raw = &ad7606_write_raw,
> >     .debugfs_reg_access = &ad7606_reg_access,<---------
> >     .validate_trigger = &ad7606_validate_trigger,
> >     .update_scan_mode = &ad7606_update_scan_mode,
> > };
> > This way the information about data status will be available on demand,
> > without exposing any special sysfs attribute.  
> 
> Yes, this would be fine.
> 
> >>
> >> For application level filtering, why does it matter if we have
> >> read the same value before or not? The sampling_frequency is
> >> available, so the application should be able to deduce when
> >> fresh data should be available based on time.  
> > Agreed.  
> >>
> >> I could see maybe polling this in the kernel in order to implement
> >> a buffered capture mode, but not sure passing this on to userspace
> >> is the best way to go about it.  
> > I had sent 2 patches. This was my PATCH 1 of 2. In patch 2 of 2,
> > I have done something similar to what you mentioned, not exactly polling,  
> 
> Here is what the IIO docs have to say about it (reading the in_light_raw
> attribute is considered INDIO_DIRECT_MODE):
> 
>  * @INDIO_DIRECT_MODE: There is an access to either:
>  * a) The last single value available for devices that do not provide
>  *    on-demand reads.
>  * b) A new value after performing an on-demand read otherwise.
>  * On most devices, this is a single-shot read. On some devices with data
>  * streams without an 'on-demand' function, this might also be the 'last value'
>  * feature. Above all, this mode internally means that we are not in any of the
>  * other modes, and sysfs reads should work.
>  * Device drivers should inform the core if they support this mode.
> 
> So I don't think the current implementation is really in violation
> of this.
> 
> Additionally, changes you proposed in patch 2/2 would break existing
> users by returning an error when it didn't used to, so we couldn't
> accept that.
> 
> If this was a new driver, we might consider having the raw read
> poll for fresh data so that each read was guaranteed to be new
> data, but even that change in timing could break existing users,
> so probably not something we would want to do now.
> 
> 
> I still think there is a reasonable workaround of just having userspace
> wait for the sample period between reads to guarantee fresh data
> without changing the driver at all.
> 

Agreed.  Is the interrupt wired on this board?  If it is and you
want to do filtering with the knowledge that the data is fresh then
add a data ready trigger and buffered capture support.
It's a much bigger job, but it is standard ABI and as such of more
general use.

Jonathan


> 
> > but reading the data status bit before reading actual data.
> > Both patches are independent. Please have a look at that as well.
> > 
> > Thanks,
> > Akshay  
> 
> 

Powered by blists - more mailing lists