[PATCH RFC 4/6] iio: adc: max1363: make use of iio_is_soft_ts_enabled()

Vasileios Amoiridis posted 6 patches 3 weeks, 5 days ago
[PATCH RFC 4/6] iio: adc: max1363: make use of iio_is_soft_ts_enabled()
Posted by Vasileios Amoiridis 3 weeks, 5 days ago
Use the iio_is_soft_ts_enabled() accessor to access the value of the
scan_timestamp. This way, it can be marked as __private when there
are no direct accessors of it.

Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
 drivers/iio/adc/max1363.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/adc/max1363.c b/drivers/iio/adc/max1363.c
index 9a0baea08ab6..57d9aff729f4 100644
--- a/drivers/iio/adc/max1363.c
+++ b/drivers/iio/adc/max1363.c
@@ -1473,7 +1473,7 @@ static irqreturn_t max1363_trigger_handler(int irq, void *p)
 		d_size = numvals*2;
 	else
 		d_size = numvals;
-	if (indio_dev->scan_timestamp) {
+	if (iio_is_soft_ts_enabled(indio_dev)) {
 		d_size += sizeof(s64);
 		if (d_size % sizeof(s64))
 			d_size += sizeof(s64) - (d_size % sizeof(s64));
-- 
2.43.0
Re: [PATCH RFC 4/6] iio: adc: max1363: make use of iio_is_soft_ts_enabled()
Posted by Jonathan Cameron 3 weeks, 4 days ago
On Sat, 30 Nov 2024 01:27:08 +0100
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:

> Use the iio_is_soft_ts_enabled() accessor to access the value of the
> scan_timestamp. This way, it can be marked as __private when there
> are no direct accessors of it.
> 
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
Hmm.  A younger me one ;) 

A simple allocation that is always big enough is going to cost
us very little.  Should almost certainly be using kzalloc.

I'd change this driver to just stick an array of size
12 * 2  + 8 so 32bytes in the iio_priv()
(12 channels, possibly of 2 bytes each + aligned timestamp)
and always use that for the rx_buf.

Both a simplification and probably a performance improvement
as well by dropping the frequent allocations.


> ---
>  drivers/iio/adc/max1363.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/max1363.c b/drivers/iio/adc/max1363.c
> index 9a0baea08ab6..57d9aff729f4 100644
> --- a/drivers/iio/adc/max1363.c
> +++ b/drivers/iio/adc/max1363.c
> @@ -1473,7 +1473,7 @@ static irqreturn_t max1363_trigger_handler(int irq, void *p)
>  		d_size = numvals*2;
>  	else
>  		d_size = numvals;
> -	if (indio_dev->scan_timestamp) {
> +	if (iio_is_soft_ts_enabled(indio_dev)) {
>  		d_size += sizeof(s64);
>  		if (d_size % sizeof(s64))
>  			d_size += sizeof(s64) - (d_size % sizeof(s64));
Re: [PATCH RFC 4/6] iio: adc: max1363: make use of iio_is_soft_ts_enabled()
Posted by Vasileios Amoiridis 3 weeks, 1 day ago
On Sat, Nov 30, 2024 at 02:09:48PM +0000, Jonathan Cameron wrote:
> On Sat, 30 Nov 2024 01:27:08 +0100
> Vasileios Amoiridis <vassilisamir@gmail.com> wrote:
> 
> > Use the iio_is_soft_ts_enabled() accessor to access the value of the
> > scan_timestamp. This way, it can be marked as __private when there
> > are no direct accessors of it.
> > 
> > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> Hmm.  A younger me one ;) 
> 
> A simple allocation that is always big enough is going to cost
> us very little.  Should almost certainly be using kzalloc.
> 
> I'd change this driver to just stick an array of size
> 12 * 2  + 8 so 32bytes in the iio_priv()
> (12 channels, possibly of 2 bytes each + aligned timestamp)
> and always use that for the rx_buf.
> 
> Both a simplification and probably a performance improvement
> as well by dropping the frequent allocations.
> 
> 

Hi Jonathan,

I see your point, I can implement this as well.

Cheers,
Vasilis

> > ---
> >  drivers/iio/adc/max1363.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iio/adc/max1363.c b/drivers/iio/adc/max1363.c
> > index 9a0baea08ab6..57d9aff729f4 100644
> > --- a/drivers/iio/adc/max1363.c
> > +++ b/drivers/iio/adc/max1363.c
> > @@ -1473,7 +1473,7 @@ static irqreturn_t max1363_trigger_handler(int irq, void *p)
> >  		d_size = numvals*2;
> >  	else
> >  		d_size = numvals;
> > -	if (indio_dev->scan_timestamp) {
> > +	if (iio_is_soft_ts_enabled(indio_dev)) {
> >  		d_size += sizeof(s64);
> >  		if (d_size % sizeof(s64))
> >  			d_size += sizeof(s64) - (d_size % sizeof(s64));
>