[1/3] iio: buffer: align the size of scan bytes to size of the largest element
diff mbox series

Message ID 20191202130113.24005-1-lars.moellendorf@plating.de
State New
Headers show
Series
  • [1/3] iio: buffer: align the size of scan bytes to size of the largest element
Related show

Commit Message

Lars Möllendorf Dec. 2, 2019, 1:01 p.m. UTC
Signed-off-by: Lars Möllendorf <lars.moellendorf@plating.de>
---
 drivers/iio/industrialio-buffer.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

--
2.23.0

Comments

Alexandru Ardelean Dec. 2, 2019, 1:37 p.m. UTC | #1
On Mon, 2019-12-02 at 14:01 +0100, Lars Möllendorf wrote:
> 

Hey Lars,

Thank you for the patch.

Could you add a description of the problem in the commit description?
You did a really great job on describing it via email earlier, and it would
be great to have it in the git history as well.

Also, this patch is marked 1/3 ; curios: are there 2 more patches in a set?
Sometimes, some patches get lost via email clients/servers.

Maybe Jonathan [or someone else] has some more points to this.

Thanks
Alex

> Signed-off-by: Lars Möllendorf <lars.moellendorf@plating.de>
> ---
>  drivers/iio/industrialio-buffer.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/industrialio-buffer.c
> b/drivers/iio/industrialio-buffer.c
> index 5d05c38c4ba9..2f037cd59d53 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -546,7 +546,7 @@ static int iio_compute_scan_bytes(struct iio_dev
> *indio_dev,
>  				const unsigned long *mask, bool timestamp)
>  {
>  	unsigned bytes = 0;
> -	int length, i;
> +	int length, i, largest = 0;
> 
>  	/* How much space will the demuxed element take? */
>  	for_each_set_bit(i, mask,
> @@ -554,13 +554,17 @@ static int iio_compute_scan_bytes(struct iio_dev
> *indio_dev,
>  		length = iio_storage_bytes_for_si(indio_dev, i);
>  		bytes = ALIGN(bytes, length);
>  		bytes += length;
> +		largest = max(largest, length);
>  	}
> 
>  	if (timestamp) {
>  		length = iio_storage_bytes_for_timestamp(indio_dev);
>  		bytes = ALIGN(bytes, length);
>  		bytes += length;
> +		largest = max(largest, length);
>  	}
> +
> +	bytes = ALIGN(bytes, largest);
>  	return bytes;
>  }
> 
> --
> 2.23.0
>
Lars Möllendorf Dec. 4, 2019, 9:24 a.m. UTC | #2
On 02.12.19 14:37, Ardelean, Alexandru wrote:
> On Mon, 2019-12-02 at 14:01 +0100, Lars Möllendorf wrote:
>>
> 
> Hey Lars,
> 
> Thank you for the patch.
> 
> Could you add a description of the problem in the commit description?
> You did a really great job on describing it via email earlier, and it would
> be great to have it in the git history as well.

Is the description in my latest patch ok?


> Also, this patch is marked 1/3 ; curios: are there 2 more patches in a set?
> Sometimes, some patches get lost via email clients/servers.

No, there is only one patch. I just did not use `git format-patch`
correctly in my first attempt.

> Maybe Jonathan [or someone else] has some more points to this.

Anything else I can do to improve the patch? It is the first time I am
trying to submit a patch to the kernel. Would be nice to know if it is
accepted and if not, why. So I can learn from my mistakes.

> 
> Thanks
> Alex
> 
>> Signed-off-by: Lars Möllendorf <lars.moellendorf@plating.de>
>> ---
>>  drivers/iio/industrialio-buffer.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iio/industrialio-buffer.c
>> b/drivers/iio/industrialio-buffer.c
>> index 5d05c38c4ba9..2f037cd59d53 100644
>> --- a/drivers/iio/industrialio-buffer.c
>> +++ b/drivers/iio/industrialio-buffer.c
>> @@ -546,7 +546,7 @@ static int iio_compute_scan_bytes(struct iio_dev
>> *indio_dev,
>>  				const unsigned long *mask, bool timestamp)
>>  {
>>  	unsigned bytes = 0;
>> -	int length, i;
>> +	int length, i, largest = 0;
>>
>>  	/* How much space will the demuxed element take? */
>>  	for_each_set_bit(i, mask,
>> @@ -554,13 +554,17 @@ static int iio_compute_scan_bytes(struct iio_dev
>> *indio_dev,
>>  		length = iio_storage_bytes_for_si(indio_dev, i);
>>  		bytes = ALIGN(bytes, length);
>>  		bytes += length;
>> +		largest = max(largest, length);
>>  	}
>>
>>  	if (timestamp) {
>>  		length = iio_storage_bytes_for_timestamp(indio_dev);
>>  		bytes = ALIGN(bytes, length);
>>  		bytes += length;
>> +		largest = max(largest, length);
>>  	}
>> +
>> +	bytes = ALIGN(bytes, largest);
>>  	return bytes;
>>  }
>>
>> --
>> 2.23.0
>>
Alexandru Ardelean Dec. 4, 2019, 11:12 a.m. UTC | #3
On Wed, 2019-12-04 at 10:24 +0100, Lars Möllendorf wrote:
> On 02.12.19 14:37, Ardelean, Alexandru wrote:
> > On Mon, 2019-12-02 at 14:01 +0100, Lars Möllendorf wrote:
> > 
> > Hey Lars,
> > 
> > Thank you for the patch.
> > 
> > Could you add a description of the problem in the commit description?
> > You did a really great job on describing it via email earlier, and it
> > would
> > be great to have it in the git history as well.
> 
> Is the description in my latest patch ok?

Looks good to me.

I won't add any formal tags (Reviewed-by), since I don't understand the
full-scope of the issue.
I was just replying to the lack of description the commit.

> 
> 
> > Also, this patch is marked 1/3 ; curios: are there 2 more patches in a
> > set?
> > Sometimes, some patches get lost via email clients/servers.
> 
> No, there is only one patch. I just did not use `git format-patch`
> correctly in my first attempt.
> 
> > Maybe Jonathan [or someone else] has some more points to this.
> 
> Anything else I can do to improve the patch? It is the first time I am
> trying to submit a patch to the kernel. Would be nice to know if it is
> accepted and if not, why. So I can learn from my mistakes.
> 

So, Jonathan may come back to this and reply.
I have nothing more to add.

Typically he does that during weekends; but sometimes he replies during the
week.
If you don't get a reply from him in 1-2 weeks, maybe send a ping-email.
But, if he gets around it this week[end], then at least your patch has the
description part now :)


> > Thanks
> > Alex
> > 
> > > Signed-off-by: Lars Möllendorf <lars.moellendorf@plating.de>
> > > ---
> > >  drivers/iio/industrialio-buffer.c | 6 +++++-
> > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/iio/industrialio-buffer.c
> > > b/drivers/iio/industrialio-buffer.c
> > > index 5d05c38c4ba9..2f037cd59d53 100644
> > > --- a/drivers/iio/industrialio-buffer.c
> > > +++ b/drivers/iio/industrialio-buffer.c
> > > @@ -546,7 +546,7 @@ static int iio_compute_scan_bytes(struct iio_dev
> > > *indio_dev,
> > >  				const unsigned long *mask, bool timestamp)
> > >  {
> > >  	unsigned bytes = 0;
> > > -	int length, i;
> > > +	int length, i, largest = 0;
> > > 
> > >  	/* How much space will the demuxed element take? */
> > >  	for_each_set_bit(i, mask,
> > > @@ -554,13 +554,17 @@ static int iio_compute_scan_bytes(struct
> > > iio_dev
> > > *indio_dev,
> > >  		length = iio_storage_bytes_for_si(indio_dev, i);
> > >  		bytes = ALIGN(bytes, length);
> > >  		bytes += length;
> > > +		largest = max(largest, length);
> > >  	}
> > > 
> > >  	if (timestamp) {
> > >  		length = iio_storage_bytes_for_timestamp(indio_dev);
> > >  		bytes = ALIGN(bytes, length);
> > >  		bytes += length;
> > > +		largest = max(largest, length);
> > >  	}
> > > +
> > > +	bytes = ALIGN(bytes, largest);
> > >  	return bytes;
> > >  }
> > > 
> > > --
> > > 2.23.0
> > >

Patch
diff mbox series

diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index 5d05c38c4ba9..2f037cd59d53 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -546,7 +546,7 @@  static int iio_compute_scan_bytes(struct iio_dev *indio_dev,
 				const unsigned long *mask, bool timestamp)
 {
 	unsigned bytes = 0;
-	int length, i;
+	int length, i, largest = 0;

 	/* How much space will the demuxed element take? */
 	for_each_set_bit(i, mask,
@@ -554,13 +554,17 @@  static int iio_compute_scan_bytes(struct iio_dev *indio_dev,
 		length = iio_storage_bytes_for_si(indio_dev, i);
 		bytes = ALIGN(bytes, length);
 		bytes += length;
+		largest = max(largest, length);
 	}

 	if (timestamp) {
 		length = iio_storage_bytes_for_timestamp(indio_dev);
 		bytes = ALIGN(bytes, length);
 		bytes += length;
+		largest = max(largest, length);
 	}
+
+	bytes = ALIGN(bytes, largest);
 	return bytes;
 }