[PATCH 0/2] staging: iio: adc: General code reformatting / cleanup patchset

[PATCH 0/2] staging: iio: adc: General code reformatting / cleanup patchset

[Deepak R Varma]

Address code formatting warnings and check messages flagged by
checkpatch script. Changes intended to improve readability of code.


Deepak R Varma (2):
  staging: iio: adc: ad7192: Reformat lines crossing 80 columns
  staging: iio: adc: ad7280a: Add spaces around operators

 drivers/staging/iio/adc/ad7192.c  | 15 ++++++++-------
 drivers/staging/iio/adc/ad7280a.c |  4 ++--
 2 files changed, 10 insertions(+), 9 deletions(-)

-- 
2.17.1

[PATCH 1/2] staging: iio: adc: ad7192: Reformat lines crossing 80 columns

[Deepak R Varma]

Macro arguments are computed at the time of macro invocation. This makes
the lines cross 80 column width. Add variables to perform the
calculations before hand and use these new variable in the macro calls
instead.

Also re-indent enum members to address checkpatch warning / check messages.

Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
---
 drivers/staging/iio/adc/ad7192.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
index bf3e2a9cc07f..0265f6607d75 100644
--- a/drivers/staging/iio/adc/ad7192.c
+++ b/drivers/staging/iio/adc/ad7192.c
@@ -156,8 +156,8 @@
  */
 
 enum {
-   AD7192_SYSCALIB_ZERO_SCALE,
-   AD7192_SYSCALIB_FULL_SCALE,
+	AD7192_SYSCALIB_ZERO_SCALE,
+	AD7192_SYSCALIB_FULL_SCALE,
 };
 
 struct ad7192_state {
@@ -477,17 +477,18 @@ static ssize_t ad7192_set(struct device *dev,
 }
 
 static void ad7192_get_available_filter_freq(struct ad7192_state *st,
-						    int *freq)
+					     int *freq)
 {
 	unsigned int fadc;
+	unsigned int sync3_filter, sync4_filter;
 
 	/* Formulas for filter at page 25 of the datasheet */
-	fadc = DIV_ROUND_CLOSEST(st->fclk,
-				 AD7192_SYNC4_FILTER * AD7192_MODE_RATE(st->mode));
+	sync4_filter = AD7192_SYNC4_FILTER * AD7192_MODE_RATE(st->mode);
+	fadc = DIV_ROUND_CLOSEST(st->fclk, sync4_filter);
 	freq[0] = DIV_ROUND_CLOSEST(fadc * 240, 1024);
 
-	fadc = DIV_ROUND_CLOSEST(st->fclk,
-				 AD7192_SYNC3_FILTER * AD7192_MODE_RATE(st->mode));
+	sync3_filter = AD7192_SYNC3_FILTER * AD7192_MODE_RATE(st->mode);
+	fadc = DIV_ROUND_CLOSEST(st->fclk, sync3_filter);
 	freq[1] = DIV_ROUND_CLOSEST(fadc * 240, 1024);
 
 	fadc = DIV_ROUND_CLOSEST(st->fclk, AD7192_MODE_RATE(st->mode));
-- 
2.17.1

[PATCH 2/2] staging: iio: adc: ad7280a: Add spaces around operators

[Deepak R Varma]

Add spaces around operator symbols to improve readability. Warning
flagged by checkpatch script.

Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
---
 drivers/staging/iio/adc/ad7280a.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7280a.c b/drivers/staging/iio/adc/ad7280a.c
index 19a5f244dcae..34ca0d09db85 100644
--- a/drivers/staging/iio/adc/ad7280a.c
+++ b/drivers/staging/iio/adc/ad7280a.c
@@ -825,14 +825,14 @@ static irqreturn_t ad7280_event_handler(int irq, void *private)
 }
 
 static IIO_DEVICE_ATTR_NAMED(in_thresh_low_value,
-			     in_voltage-voltage_thresh_low_value,
+			     in_voltage - voltage_thresh_low_value,
 			     0644,
 			     ad7280_read_channel_config,
 			     ad7280_write_channel_config,
 			     AD7280A_CELL_UNDERVOLTAGE);
 
 static IIO_DEVICE_ATTR_NAMED(in_thresh_high_value,
-			     in_voltage-voltage_thresh_high_value,
+			     in_voltage - voltage_thresh_high_value,
 			     0644,
 			     ad7280_read_channel_config,
 			     ad7280_write_channel_config,
-- 
2.17.1

Re: [PATCH 1/2] staging: iio: adc: ad7192: Reformat lines crossing 80 columns

[Greg KH]

On Wed, Mar 18, 2020 at 09:56:59AM +0530, Deepak R Varma wrote:
> Macro arguments are computed at the time of macro invocation. This makes
> the lines cross 80 column width. Add variables to perform the
> calculations before hand and use these new variable in the macro calls
> instead.
> 
> Also re-indent enum members to address checkpatch warning / check messages.

When you say "also" in a changelog description, that's a huge hint the
patch needs to be broken up.

And that is what needs to happen here.

> 
> Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> ---
>  drivers/staging/iio/adc/ad7192.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
> index bf3e2a9cc07f..0265f6607d75 100644
> --- a/drivers/staging/iio/adc/ad7192.c
> +++ b/drivers/staging/iio/adc/ad7192.c
> @@ -156,8 +156,8 @@
>   */
>  
>  enum {
> -   AD7192_SYSCALIB_ZERO_SCALE,
> -   AD7192_SYSCALIB_FULL_SCALE,
> +	AD7192_SYSCALIB_ZERO_SCALE,
> +	AD7192_SYSCALIB_FULL_SCALE,

Because this has nothing to do with the subject, please make it a
separate patch.

thanks,

greg k-h

Re: [PATCH 2/2] staging: iio: adc: ad7280a: Add spaces around operators

[Greg KH]

On Wed, Mar 18, 2020 at 09:58:13AM +0530, Deepak R Varma wrote:
> Add spaces around operator symbols to improve readability. Warning
> flagged by checkpatch script.
> 
> Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> ---
>  drivers/staging/iio/adc/ad7280a.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7280a.c b/drivers/staging/iio/adc/ad7280a.c
> index 19a5f244dcae..34ca0d09db85 100644
> --- a/drivers/staging/iio/adc/ad7280a.c
> +++ b/drivers/staging/iio/adc/ad7280a.c
> @@ -825,14 +825,14 @@ static irqreturn_t ad7280_event_handler(int irq, void *private)
>  }
>  
>  static IIO_DEVICE_ATTR_NAMED(in_thresh_low_value,
> -			     in_voltage-voltage_thresh_low_value,
> +			     in_voltage - voltage_thresh_low_value,
>  			     0644,
>  			     ad7280_read_channel_config,
>  			     ad7280_write_channel_config,
>  			     AD7280A_CELL_UNDERVOLTAGE);
>  
>  static IIO_DEVICE_ATTR_NAMED(in_thresh_high_value,
> -			     in_voltage-voltage_thresh_high_value,
> +			     in_voltage - voltage_thresh_high_value,
>  			     0644,
>  			     ad7280_read_channel_config,
>  			     ad7280_write_channel_config,

Did you try building this code?

It catches everyone...

greg k-h

Re: [Outreachy kernel] [PATCH 1/2] staging: iio: adc: ad7192: Reformat lines crossing 80 columns

[Stefano Brivio]

On Wed, 18 Mar 2020 09:56:59 +0530
Deepak R Varma <mh12gx2825@gmail.com> wrote:

> Macro arguments are computed at the time of macro invocation. This makes
> the lines cross 80 column width. Add variables to perform the
> calculations before hand and use these new variable in the macro calls
> instead.
> 
> Also re-indent enum members to address checkpatch warning / check messages.
> 
> Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> ---
>  drivers/staging/iio/adc/ad7192.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
> index bf3e2a9cc07f..0265f6607d75 100644
> --- a/drivers/staging/iio/adc/ad7192.c
> +++ b/drivers/staging/iio/adc/ad7192.c
> @@ -156,8 +156,8 @@
>   */
>  
>  enum {
> -   AD7192_SYSCALIB_ZERO_SCALE,
> -   AD7192_SYSCALIB_FULL_SCALE,
> +	AD7192_SYSCALIB_ZERO_SCALE,
> +	AD7192_SYSCALIB_FULL_SCALE,
>  };
>  
>  struct ad7192_state {
> @@ -477,17 +477,18 @@ static ssize_t ad7192_set(struct device *dev,
>  }
>  
>  static void ad7192_get_available_filter_freq(struct ad7192_state *st,
> -						    int *freq)
> +					     int *freq)
>  {
>  	unsigned int fadc;
> +	unsigned int sync3_filter, sync4_filter;
>  
>  	/* Formulas for filter at page 25 of the datasheet */
> -	fadc = DIV_ROUND_CLOSEST(st->fclk,
> -				 AD7192_SYNC4_FILTER * AD7192_MODE_RATE(st->mode));
> +	sync4_filter = AD7192_SYNC4_FILTER * AD7192_MODE_RATE(st->mode);

Have you read page 25 of the datasheet? Why is this called
sync4_filter, with a 'y'?

-- 
Stefano

Re: [PATCH 2/2] staging: iio: adc: ad7280a: Add spaces around operators

[DEEPAK VARMA]

On Wed, Mar 18, 2020 at 07:00:38AM +0100, Greg KH wrote:
> On Wed, Mar 18, 2020 at 09:58:13AM +0530, Deepak R Varma wrote:
> > Add spaces around operator symbols to improve readability. Warning
> > flagged by checkpatch script.
> > 
> > Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> > ---
> >  drivers/staging/iio/adc/ad7280a.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/staging/iio/adc/ad7280a.c b/drivers/staging/iio/adc/ad7280a.c
> > index 19a5f244dcae..34ca0d09db85 100644
> > --- a/drivers/staging/iio/adc/ad7280a.c
> > +++ b/drivers/staging/iio/adc/ad7280a.c
> > @@ -825,14 +825,14 @@ static irqreturn_t ad7280_event_handler(int irq, void *private)
> >  }
> >  
> >  static IIO_DEVICE_ATTR_NAMED(in_thresh_low_value,
> > -			     in_voltage-voltage_thresh_low_value,
> > +			     in_voltage - voltage_thresh_low_value,
> >  			     0644,
> >  			     ad7280_read_channel_config,
> >  			     ad7280_write_channel_config,
> >  			     AD7280A_CELL_UNDERVOLTAGE);
> >  
> >  static IIO_DEVICE_ATTR_NAMED(in_thresh_high_value,
> > -			     in_voltage-voltage_thresh_high_value,
> > +			     in_voltage - voltage_thresh_high_value,
> >  			     0644,
> >  			     ad7280_read_channel_config,
> >  			     ad7280_write_channel_config,
> 
> Did you try building this code?
> 
> It catches everyone...

yes sir, I did. See this:

  CC [M]  drivers/staging/fbtft/fbtft.mod.o
  CC [M]  drivers/staging/iio/adc/ad7192.mod.o
  LD [M]  drivers/staging/iio/adc/ad7280a.ko
  LD [M]  drivers/staging/media/ipu3/ipu3-imgu.ko
  LD [M]  drivers/staging/fbtft/fbtft.ko
  LD [M]  drivers/staging/iio/adc/ad7192.ko

Let me know if I missed something.

Thank you,
Deepak.

> 
> greg k-h

Re: [PATCH 2/2] staging: iio: adc: ad7280a: Add spaces around operators

[Lars-Peter Clausen]

On 3/18/20 7:00 AM, Greg KH wrote:
> On Wed, Mar 18, 2020 at 09:58:13AM +0530, Deepak R Varma wrote:
>> Add spaces around operator symbols to improve readability. Warning
>> flagged by checkpatch script.
>>
>> Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
>> ---
>>   drivers/staging/iio/adc/ad7280a.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/iio/adc/ad7280a.c b/drivers/staging/iio/adc/ad7280a.c
>> index 19a5f244dcae..34ca0d09db85 100644
>> --- a/drivers/staging/iio/adc/ad7280a.c
>> +++ b/drivers/staging/iio/adc/ad7280a.c
>> @@ -825,14 +825,14 @@ static irqreturn_t ad7280_event_handler(int irq, void *private)
>>   }
>>   
>>   static IIO_DEVICE_ATTR_NAMED(in_thresh_low_value,
>> -			     in_voltage-voltage_thresh_low_value,
>> +			     in_voltage - voltage_thresh_low_value,
>>   			     0644,
>>   			     ad7280_read_channel_config,
>>   			     ad7280_write_channel_config,
>>   			     AD7280A_CELL_UNDERVOLTAGE);
>>   
>>   static IIO_DEVICE_ATTR_NAMED(in_thresh_high_value,
>> -			     in_voltage-voltage_thresh_high_value,
>> +			     in_voltage - voltage_thresh_high_value,
>>   			     0644,
>>   			     ad7280_read_channel_config,
>>   			     ad7280_write_channel_config,
> 
> Did you try building this code?
> 
> It catches everyone...

The problem is it builds. The token is stringyfied and
"in_voltage - voltage_thresh_high_value" is a valid string.

- Lars

Re: [PATCH 1/2] staging: iio: adc: ad7192: Reformat lines crossing 80 columns

[DEEPAK VARMA]

On Wed, Mar 18, 2020 at 07:00:04AM +0100, Greg KH wrote:
> On Wed, Mar 18, 2020 at 09:56:59AM +0530, Deepak R Varma wrote:
> > Macro arguments are computed at the time of macro invocation. This makes
> > the lines cross 80 column width. Add variables to perform the
> > calculations before hand and use these new variable in the macro calls
> > instead.
> > 
> > Also re-indent enum members to address checkpatch warning / check messages.
> 
> When you say "also" in a changelog description, that's a huge hint the
> patch needs to be broken up.
> 
> And that is what needs to happen here.
> 
> > 
> > Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> > ---
> >  drivers/staging/iio/adc/ad7192.c | 15 ++++++++-------
> >  1 file changed, 8 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
> > index bf3e2a9cc07f..0265f6607d75 100644
> > --- a/drivers/staging/iio/adc/ad7192.c
> > +++ b/drivers/staging/iio/adc/ad7192.c
> > @@ -156,8 +156,8 @@
> >   */
> >  
> >  enum {
> > -   AD7192_SYSCALIB_ZERO_SCALE,
> > -   AD7192_SYSCALIB_FULL_SCALE,
> > +	AD7192_SYSCALIB_ZERO_SCALE,
> > +	AD7192_SYSCALIB_FULL_SCALE,
> 
> Because this has nothing to do with the subject, please make it a
> separate patch.
> 

Okay. Got your point. I was thinking since this is a clean up patch I can include both
the changes for the same file in a single patch. No problem; I will
correct and send in a v2.

Thank you,
Deepak.


> thanks,
> 
> greg k-h

Re: [PATCH 1/2] staging: iio: adc: ad7192: Reformat lines crossing 80 columns

[Greg KH]

On Wed, Mar 18, 2020 at 08:42:45PM +0530, DEEPAK VARMA wrote:
> On Wed, Mar 18, 2020 at 07:00:04AM +0100, Greg KH wrote:
> > On Wed, Mar 18, 2020 at 09:56:59AM +0530, Deepak R Varma wrote:
> > > Macro arguments are computed at the time of macro invocation. This makes
> > > the lines cross 80 column width. Add variables to perform the
> > > calculations before hand and use these new variable in the macro calls
> > > instead.
> > > 
> > > Also re-indent enum members to address checkpatch warning / check messages.
> > 
> > When you say "also" in a changelog description, that's a huge hint the
> > patch needs to be broken up.
> > 
> > And that is what needs to happen here.
> > 
> > > 
> > > Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> > > ---
> > >  drivers/staging/iio/adc/ad7192.c | 15 ++++++++-------
> > >  1 file changed, 8 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
> > > index bf3e2a9cc07f..0265f6607d75 100644
> > > --- a/drivers/staging/iio/adc/ad7192.c
> > > +++ b/drivers/staging/iio/adc/ad7192.c
> > > @@ -156,8 +156,8 @@
> > >   */
> > >  
> > >  enum {
> > > -   AD7192_SYSCALIB_ZERO_SCALE,
> > > -   AD7192_SYSCALIB_FULL_SCALE,
> > > +	AD7192_SYSCALIB_ZERO_SCALE,
> > > +	AD7192_SYSCALIB_FULL_SCALE,
> > 
> > Because this has nothing to do with the subject, please make it a
> > separate patch.
> > 
> 
> Okay. Got your point. I was thinking since this is a clean up patch I can include both
> the changes for the same file in a single patch. No problem; I will
> correct and send in a v2.

From my patch bot which would have normally triggered on this patch had
it not been part of the outrechy project:


- Your patch did many different things all at once, making it difficult
  to review.  All Linux kernel patches need to only do one thing at a
  time.  If you need to do multiple things (such as clean up all coding
  style issues in a file/driver), do it in a sequence of patches, each
  one doing only one thing.  This will make it easier to review the
  patches to ensure that they are correct, and to help alleviate any
  merge issues that larger patches can cause.

Re: [PATCH 2/2] staging: iio: adc: ad7280a: Add spaces around operators

[Greg KH]

On Wed, Mar 18, 2020 at 04:12:28PM +0100, Lars-Peter Clausen wrote:
> On 3/18/20 7:00 AM, Greg KH wrote:
> > On Wed, Mar 18, 2020 at 09:58:13AM +0530, Deepak R Varma wrote:
> > > Add spaces around operator symbols to improve readability. Warning
> > > flagged by checkpatch script.
> > > 
> > > Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> > > ---
> > >   drivers/staging/iio/adc/ad7280a.c | 4 ++--
> > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/staging/iio/adc/ad7280a.c b/drivers/staging/iio/adc/ad7280a.c
> > > index 19a5f244dcae..34ca0d09db85 100644
> > > --- a/drivers/staging/iio/adc/ad7280a.c
> > > +++ b/drivers/staging/iio/adc/ad7280a.c
> > > @@ -825,14 +825,14 @@ static irqreturn_t ad7280_event_handler(int irq, void *private)
> > >   }
> > >   static IIO_DEVICE_ATTR_NAMED(in_thresh_low_value,
> > > -			     in_voltage-voltage_thresh_low_value,
> > > +			     in_voltage - voltage_thresh_low_value,
> > >   			     0644,
> > >   			     ad7280_read_channel_config,
> > >   			     ad7280_write_channel_config,
> > >   			     AD7280A_CELL_UNDERVOLTAGE);
> > >   static IIO_DEVICE_ATTR_NAMED(in_thresh_high_value,
> > > -			     in_voltage-voltage_thresh_high_value,
> > > +			     in_voltage - voltage_thresh_high_value,
> > >   			     0644,
> > >   			     ad7280_read_channel_config,
> > >   			     ad7280_write_channel_config,
> > 
> > Did you try building this code?
> > 
> > It catches everyone...
> 
> The problem is it builds. The token is stringyfied and
> "in_voltage - voltage_thresh_high_value" is a valid string.

Ah, I thought it used to break the build when it happened.  Oh well,
it's still a great "trick" to see if people understand C or not :)

thanks,

greg k-h

Re: [Outreachy kernel] [PATCH 1/2] staging: iio: adc: ad7192: Reformat lines crossing 80 columns

[DEEPAK VARMA]

On Wed, Mar 18, 2020 at 09:31:58AM +0100, Stefano Brivio wrote:
> On Wed, 18 Mar 2020 09:56:59 +0530
> Deepak R Varma <mh12gx2825@gmail.com> wrote:
> 
> > Macro arguments are computed at the time of macro invocation. This makes
> > the lines cross 80 column width. Add variables to perform the
> > calculations before hand and use these new variable in the macro calls
> > instead.
> > 
> > Also re-indent enum members to address checkpatch warning / check messages.
> > 
> > Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> > ---
> >  drivers/staging/iio/adc/ad7192.c | 15 ++++++++-------
> >  1 file changed, 8 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
> > index bf3e2a9cc07f..0265f6607d75 100644
> > --- a/drivers/staging/iio/adc/ad7192.c
> > +++ b/drivers/staging/iio/adc/ad7192.c
> > @@ -156,8 +156,8 @@
> >   */
> >  
> >  enum {
> > -   AD7192_SYSCALIB_ZERO_SCALE,
> > -   AD7192_SYSCALIB_FULL_SCALE,
> > +	AD7192_SYSCALIB_ZERO_SCALE,
> > +	AD7192_SYSCALIB_FULL_SCALE,
> >  };
> >  
> >  struct ad7192_state {
> > @@ -477,17 +477,18 @@ static ssize_t ad7192_set(struct device *dev,
> >  }
> >  
> >  static void ad7192_get_available_filter_freq(struct ad7192_state *st,
> > -						    int *freq)
> > +					     int *freq)
> >  {
> >  	unsigned int fadc;
> > +	unsigned int sync3_filter, sync4_filter;
> >  
> >  	/* Formulas for filter at page 25 of the datasheet */
> > -	fadc = DIV_ROUND_CLOSEST(st->fclk,
> > -				 AD7192_SYNC4_FILTER * AD7192_MODE_RATE(st->mode));
> > +	sync4_filter = AD7192_SYNC4_FILTER * AD7192_MODE_RATE(st->mode);
> 
> Have you read page 25 of the datasheet? Why is this called
> sync4_filter, with a 'y'?
> 

Sorry, I am not sure what you are referring to. Can you please elaborate
or point me to where the data sheet is located?

Deepak.

> -- 
> Stefano
> 

Re: [Outreachy kernel] [PATCH 1/2] staging: iio: adc: ad7192: Reformat lines crossing 80 columns

[Rohit Sarkar]

On Wed, Mar 18, 2020 at 09:36:50PM +0530, DEEPAK VARMA wrote:
> On Wed, Mar 18, 2020 at 09:31:58AM +0100, Stefano Brivio wrote:
> > On Wed, 18 Mar 2020 09:56:59 +0530
> > Deepak R Varma <mh12gx2825@gmail.com> wrote:
> > 
> > > Macro arguments are computed at the time of macro invocation. This makes
> > > the lines cross 80 column width. Add variables to perform the
> > > calculations before hand and use these new variable in the macro calls
> > > instead.
> > > 
> > > Also re-indent enum members to address checkpatch warning / check messages.
> > > 
> > > Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> > > ---
> > >  drivers/staging/iio/adc/ad7192.c | 15 ++++++++-------
> > >  1 file changed, 8 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
> > > index bf3e2a9cc07f..0265f6607d75 100644
> > > --- a/drivers/staging/iio/adc/ad7192.c
> > > +++ b/drivers/staging/iio/adc/ad7192.c
> > > @@ -156,8 +156,8 @@
> > >   */
> > >  
> > >  enum {
> > > -   AD7192_SYSCALIB_ZERO_SCALE,
> > > -   AD7192_SYSCALIB_FULL_SCALE,
> > > +	AD7192_SYSCALIB_ZERO_SCALE,
> > > +	AD7192_SYSCALIB_FULL_SCALE,
> > >  };
> > >  
> > >  struct ad7192_state {
> > > @@ -477,17 +477,18 @@ static ssize_t ad7192_set(struct device *dev,
> > >  }
> > >  
> > >  static void ad7192_get_available_filter_freq(struct ad7192_state *st,
> > > -						    int *freq)
> > > +					     int *freq)
> > >  {
> > >  	unsigned int fadc;
> > > +	unsigned int sync3_filter, sync4_filter;
> > >  
> > >  	/* Formulas for filter at page 25 of the datasheet */
> > > -	fadc = DIV_ROUND_CLOSEST(st->fclk,
> > > -				 AD7192_SYNC4_FILTER * AD7192_MODE_RATE(st->mode));
> > > +	sync4_filter = AD7192_SYNC4_FILTER * AD7192_MODE_RATE(st->mode);
> > 
> > Have you read page 25 of the datasheet? Why is this called
> > sync4_filter, with a 'y'?
> > 
> 
> Sorry, I am not sure what you are referring to. Can you please elaborate
> or point me to where the data sheet is located?
> 
> Deepak.

Hey Deepak,
You can find the datasheet for ad7192 here https://pdf1.alldatasheet.com/datasheet-pdf/view/988287/AD/AD7192.html

Thanks,
Rohit

Re: [PATCH 2/2] staging: iio: adc: ad7280a: Add spaces around operators

[DEEPAK VARMA]

On Wed, Mar 18, 2020 at 04:19:24PM +0100, Greg KH wrote:
> On Wed, Mar 18, 2020 at 04:12:28PM +0100, Lars-Peter Clausen wrote:
> > On 3/18/20 7:00 AM, Greg KH wrote:
> > > On Wed, Mar 18, 2020 at 09:58:13AM +0530, Deepak R Varma wrote:
> > > > Add spaces around operator symbols to improve readability. Warning
> > > > flagged by checkpatch script.
> > > > 
> > > > Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> > > > ---
> > > >   drivers/staging/iio/adc/ad7280a.c | 4 ++--
> > > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/staging/iio/adc/ad7280a.c b/drivers/staging/iio/adc/ad7280a.c
> > > > index 19a5f244dcae..34ca0d09db85 100644
> > > > --- a/drivers/staging/iio/adc/ad7280a.c
> > > > +++ b/drivers/staging/iio/adc/ad7280a.c
> > > > @@ -825,14 +825,14 @@ static irqreturn_t ad7280_event_handler(int irq, void *private)
> > > >   }
> > > >   static IIO_DEVICE_ATTR_NAMED(in_thresh_low_value,
> > > > -			     in_voltage-voltage_thresh_low_value,
> > > > +			     in_voltage - voltage_thresh_low_value,
> > > >   			     0644,
> > > >   			     ad7280_read_channel_config,
> > > >   			     ad7280_write_channel_config,
> > > >   			     AD7280A_CELL_UNDERVOLTAGE);
> > > >   static IIO_DEVICE_ATTR_NAMED(in_thresh_high_value,
> > > > -			     in_voltage-voltage_thresh_high_value,
> > > > +			     in_voltage - voltage_thresh_high_value,
> > > >   			     0644,
> > > >   			     ad7280_read_channel_config,
> > > >   			     ad7280_write_channel_config,
> > > 
> > > Did you try building this code?
> > > 
> > > It catches everyone...
> > 
> > The problem is it builds. The token is stringyfied and
> > "in_voltage - voltage_thresh_high_value" is a valid string.
> 
> Ah, I thought it used to break the build when it happened.  Oh well,
> it's still a great "trick" to see if people understand C or not :)
> 
Yes, it did build. I am sorry but I am not following you fully. Can you
please let me know if the change I introduced is not good. You may
please direct me to a document where I can read more about it.

Thanks,
Deepak.
> thanks,
> 
> greg k-h

Re: [PATCH 2/2] staging: iio: adc: ad7280a: Add spaces around operators

[Greg KH]

On Wed, Mar 18, 2020 at 09:53:55PM +0530, DEEPAK VARMA wrote:
> On Wed, Mar 18, 2020 at 04:19:24PM +0100, Greg KH wrote:
> > On Wed, Mar 18, 2020 at 04:12:28PM +0100, Lars-Peter Clausen wrote:
> > > On 3/18/20 7:00 AM, Greg KH wrote:
> > > > On Wed, Mar 18, 2020 at 09:58:13AM +0530, Deepak R Varma wrote:
> > > > > Add spaces around operator symbols to improve readability. Warning
> > > > > flagged by checkpatch script.
> > > > > 
> > > > > Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> > > > > ---
> > > > >   drivers/staging/iio/adc/ad7280a.c | 4 ++--
> > > > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/staging/iio/adc/ad7280a.c b/drivers/staging/iio/adc/ad7280a.c
> > > > > index 19a5f244dcae..34ca0d09db85 100644
> > > > > --- a/drivers/staging/iio/adc/ad7280a.c
> > > > > +++ b/drivers/staging/iio/adc/ad7280a.c
> > > > > @@ -825,14 +825,14 @@ static irqreturn_t ad7280_event_handler(int irq, void *private)
> > > > >   }
> > > > >   static IIO_DEVICE_ATTR_NAMED(in_thresh_low_value,
> > > > > -			     in_voltage-voltage_thresh_low_value,
> > > > > +			     in_voltage - voltage_thresh_low_value,
> > > > >   			     0644,
> > > > >   			     ad7280_read_channel_config,
> > > > >   			     ad7280_write_channel_config,
> > > > >   			     AD7280A_CELL_UNDERVOLTAGE);
> > > > >   static IIO_DEVICE_ATTR_NAMED(in_thresh_high_value,
> > > > > -			     in_voltage-voltage_thresh_high_value,
> > > > > +			     in_voltage - voltage_thresh_high_value,
> > > > >   			     0644,
> > > > >   			     ad7280_read_channel_config,
> > > > >   			     ad7280_write_channel_config,
> > > > 
> > > > Did you try building this code?
> > > > 
> > > > It catches everyone...
> > > 
> > > The problem is it builds. The token is stringyfied and
> > > "in_voltage - voltage_thresh_high_value" is a valid string.
> > 
> > Ah, I thought it used to break the build when it happened.  Oh well,
> > it's still a great "trick" to see if people understand C or not :)
> > 
> Yes, it did build. I am sorry but I am not following you fully. Can you
> please let me know if the change I introduced is not good. You may
> please direct me to a document where I can read more about it.

That field is a string, not variables that are being subtracted.

thanks,

greg k-h

Re: [Outreachy kernel] Re: [PATCH 2/2] staging: iio: adc: ad7280a: Add spaces around operators

[Julia Lawall]

On Wed, 18 Mar 2020, DEEPAK VARMA wrote:

> On Wed, Mar 18, 2020 at 04:19:24PM +0100, Greg KH wrote:
> > On Wed, Mar 18, 2020 at 04:12:28PM +0100, Lars-Peter Clausen wrote:
> > > On 3/18/20 7:00 AM, Greg KH wrote:
> > > > On Wed, Mar 18, 2020 at 09:58:13AM +0530, Deepak R Varma wrote:
> > > > > Add spaces around operator symbols to improve readability. Warning
> > > > > flagged by checkpatch script.
> > > > >
> > > > > Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> > > > > ---
> > > > >   drivers/staging/iio/adc/ad7280a.c | 4 ++--
> > > > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/drivers/staging/iio/adc/ad7280a.c b/drivers/staging/iio/adc/ad7280a.c
> > > > > index 19a5f244dcae..34ca0d09db85 100644
> > > > > --- a/drivers/staging/iio/adc/ad7280a.c
> > > > > +++ b/drivers/staging/iio/adc/ad7280a.c
> > > > > @@ -825,14 +825,14 @@ static irqreturn_t ad7280_event_handler(int irq, void *private)
> > > > >   }
> > > > >   static IIO_DEVICE_ATTR_NAMED(in_thresh_low_value,
> > > > > -			     in_voltage-voltage_thresh_low_value,
> > > > > +			     in_voltage - voltage_thresh_low_value,
> > > > >   			     0644,
> > > > >   			     ad7280_read_channel_config,
> > > > >   			     ad7280_write_channel_config,
> > > > >   			     AD7280A_CELL_UNDERVOLTAGE);
> > > > >   static IIO_DEVICE_ATTR_NAMED(in_thresh_high_value,
> > > > > -			     in_voltage-voltage_thresh_high_value,
> > > > > +			     in_voltage - voltage_thresh_high_value,
> > > > >   			     0644,
> > > > >   			     ad7280_read_channel_config,
> > > > >   			     ad7280_write_channel_config,
> > > >
> > > > Did you try building this code?
> > > >
> > > > It catches everyone...
> > >
> > > The problem is it builds. The token is stringyfied and
> > > "in_voltage - voltage_thresh_high_value" is a valid string.
> >
> > Ah, I thought it used to break the build when it happened.  Oh well,
> > it's still a great "trick" to see if people understand C or not :)
> >
> Yes, it did build. I am sorry but I am not following you fully. Can you
> please let me know if the change I introduced is not good. You may
> please direct me to a document where I can read more about it.

The code involves a macro, as indicated by the capital letters.  You will
see the issue when you look at the definition of the macro.

julia

>
> Thanks,
> Deepak.
> > thanks,
> >
> > greg k-h
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20200318162353.GA23226%40deeUbuntu.
>

Re: [Outreachy kernel] [PATCH 1/2] staging: iio: adc: ad7192: Reformat lines crossing 80 columns

[DEEPAK VARMA]

On Wed, Mar 18, 2020 at 09:52:41PM +0530, Rohit Sarkar wrote:
> On Wed, Mar 18, 2020 at 09:36:50PM +0530, DEEPAK VARMA wrote:
> > On Wed, Mar 18, 2020 at 09:31:58AM +0100, Stefano Brivio wrote:
> > > On Wed, 18 Mar 2020 09:56:59 +0530
> > > Deepak R Varma <mh12gx2825@gmail.com> wrote:
> > > 
> > > > Macro arguments are computed at the time of macro invocation. This makes
> > > > the lines cross 80 column width. Add variables to perform the
> > > > calculations before hand and use these new variable in the macro calls
> > > > instead.
> > > > 
> > > > Also re-indent enum members to address checkpatch warning / check messages.
> > > > 
> > > > Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> > > > ---
> > > >  drivers/staging/iio/adc/ad7192.c | 15 ++++++++-------
> > > >  1 file changed, 8 insertions(+), 7 deletions(-)
> > > > 
> > > > diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
> > > > index bf3e2a9cc07f..0265f6607d75 100644
> > > > --- a/drivers/staging/iio/adc/ad7192.c
> > > > +++ b/drivers/staging/iio/adc/ad7192.c
> > > > @@ -156,8 +156,8 @@
> > > >   */
> > > >  
> > > >  enum {
> > > > -   AD7192_SYSCALIB_ZERO_SCALE,
> > > > -   AD7192_SYSCALIB_FULL_SCALE,
> > > > +	AD7192_SYSCALIB_ZERO_SCALE,
> > > > +	AD7192_SYSCALIB_FULL_SCALE,
> > > >  };
> > > >  
> > > >  struct ad7192_state {
> > > > @@ -477,17 +477,18 @@ static ssize_t ad7192_set(struct device *dev,
> > > >  }
> > > >  
> > > >  static void ad7192_get_available_filter_freq(struct ad7192_state *st,
> > > > -						    int *freq)
> > > > +					     int *freq)
> > > >  {
> > > >  	unsigned int fadc;
> > > > +	unsigned int sync3_filter, sync4_filter;
> > > >  
> > > >  	/* Formulas for filter at page 25 of the datasheet */
> > > > -	fadc = DIV_ROUND_CLOSEST(st->fclk,
> > > > -				 AD7192_SYNC4_FILTER * AD7192_MODE_RATE(st->mode));
> > > > +	sync4_filter = AD7192_SYNC4_FILTER * AD7192_MODE_RATE(st->mode);
> > > 
> > > Have you read page 25 of the datasheet? Why is this called
> > > sync4_filter, with a 'y'?
> > > 
> > 
> > Sorry, I am not sure what you are referring to. Can you please elaborate
> > or point me to where the data sheet is located?
> > 
> > Deepak.
> 
> Hey Deepak,
> You can find the datasheet for ad7192 here https://pdf1.alldatasheet.com/datasheet-pdf/view/988287/AD/AD7192.html
> 

Thank you Rohit. I got it. I understand Stefano's comments now. I named
the variables with a 'y' to keep it similar to the macro
AD7192_SYNCn_FILTER. Let me know if the variable name looks odd and I
should rename it to sinc4_filter instead.

Thanks,
Deepak.

> Thanks,
> Rohit

Re: [Outreachy kernel] [PATCH 1/2] staging: iio: adc: ad7192: Reformat lines crossing 80 columns

[Lars-Peter Clausen]

On 3/18/20 5:43 PM, DEEPAK VARMA wrote:
> On Wed, Mar 18, 2020 at 09:52:41PM +0530, Rohit Sarkar wrote:
>> On Wed, Mar 18, 2020 at 09:36:50PM +0530, DEEPAK VARMA wrote:
>>> On Wed, Mar 18, 2020 at 09:31:58AM +0100, Stefano Brivio wrote:
>>>> On Wed, 18 Mar 2020 09:56:59 +0530
>>>> Deepak R Varma <mh12gx2825@gmail.com> wrote:
>>>>
>>>>> Macro arguments are computed at the time of macro invocation. This makes
>>>>> the lines cross 80 column width. Add variables to perform the
>>>>> calculations before hand and use these new variable in the macro calls
>>>>> instead.
>>>>>
>>>>> Also re-indent enum members to address checkpatch warning / check messages.
>>>>>
>>>>> Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
>>>>> ---
>>>>>   drivers/staging/iio/adc/ad7192.c | 15 ++++++++-------
>>>>>   1 file changed, 8 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
>>>>> index bf3e2a9cc07f..0265f6607d75 100644
>>>>> --- a/drivers/staging/iio/adc/ad7192.c
>>>>> +++ b/drivers/staging/iio/adc/ad7192.c
>>>>> @@ -156,8 +156,8 @@
>>>>>    */
>>>>>   
>>>>>   enum {
>>>>> -   AD7192_SYSCALIB_ZERO_SCALE,
>>>>> -   AD7192_SYSCALIB_FULL_SCALE,
>>>>> +	AD7192_SYSCALIB_ZERO_SCALE,
>>>>> +	AD7192_SYSCALIB_FULL_SCALE,
>>>>>   };
>>>>>   
>>>>>   struct ad7192_state {
>>>>> @@ -477,17 +477,18 @@ static ssize_t ad7192_set(struct device *dev,
>>>>>   }
>>>>>   
>>>>>   static void ad7192_get_available_filter_freq(struct ad7192_state *st,
>>>>> -						    int *freq)
>>>>> +					     int *freq)
>>>>>   {
>>>>>   	unsigned int fadc;
>>>>> +	unsigned int sync3_filter, sync4_filter;
>>>>>   
>>>>>   	/* Formulas for filter at page 25 of the datasheet */
>>>>> -	fadc = DIV_ROUND_CLOSEST(st->fclk,
>>>>> -				 AD7192_SYNC4_FILTER * AD7192_MODE_RATE(st->mode));
>>>>> +	sync4_filter = AD7192_SYNC4_FILTER * AD7192_MODE_RATE(st->mode);
>>>>
>>>> Have you read page 25 of the datasheet? Why is this called
>>>> sync4_filter, with a 'y'?
>>>>
>>>
>>> Sorry, I am not sure what you are referring to. Can you please elaborate
>>> or point me to where the data sheet is located?
>>>
>>> Deepak.
>>
>> Hey Deepak,
>> You can find the datasheet for ad7192 here https://pdf1.alldatasheet.com/datasheet-pdf/view/988287/AD/AD7192.html
>>
> 
> Thank you Rohit. I got it. I understand Stefano's comments now. I named
> the variables with a 'y' to keep it similar to the macro
> AD7192_SYNCn_FILTER. Let me know if the variable name looks odd and I
> should rename it to sinc4_filter instead.

Hi,

Please send a patch to rename the macro to SINC as well. This is a typo 
in the macro.

Thanks,
- Lars

Re: [Outreachy kernel] Re: [PATCH 2/2] staging: iio: adc: ad7280a: Add spaces around operators

[DEEPAK VARMA]

On Wed, Mar 18, 2020 at 05:28:17PM +0100, Julia Lawall wrote:
> 
> 
> On Wed, 18 Mar 2020, DEEPAK VARMA wrote:
> 
> > On Wed, Mar 18, 2020 at 04:19:24PM +0100, Greg KH wrote:
> > > On Wed, Mar 18, 2020 at 04:12:28PM +0100, Lars-Peter Clausen wrote:
> > > > On 3/18/20 7:00 AM, Greg KH wrote:
> > > > > On Wed, Mar 18, 2020 at 09:58:13AM +0530, Deepak R Varma wrote:
> > > > > > Add spaces around operator symbols to improve readability. Warning
> > > > > > flagged by checkpatch script.
> > > > > >
> > > > > > Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> > > > > > ---
> > > > > >   drivers/staging/iio/adc/ad7280a.c | 4 ++--
> > > > > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/staging/iio/adc/ad7280a.c b/drivers/staging/iio/adc/ad7280a.c
> > > > > > index 19a5f244dcae..34ca0d09db85 100644
> > > > > > --- a/drivers/staging/iio/adc/ad7280a.c
> > > > > > +++ b/drivers/staging/iio/adc/ad7280a.c
> > > > > > @@ -825,14 +825,14 @@ static irqreturn_t ad7280_event_handler(int irq, void *private)
> > > > > >   }
> > > > > >   static IIO_DEVICE_ATTR_NAMED(in_thresh_low_value,
> > > > > > -			     in_voltage-voltage_thresh_low_value,
> > > > > > +			     in_voltage - voltage_thresh_low_value,
> > > > > >   			     0644,
> > > > > >   			     ad7280_read_channel_config,
> > > > > >   			     ad7280_write_channel_config,
> > > > > >   			     AD7280A_CELL_UNDERVOLTAGE);
> > > > > >   static IIO_DEVICE_ATTR_NAMED(in_thresh_high_value,
> > > > > > -			     in_voltage-voltage_thresh_high_value,
> > > > > > +			     in_voltage - voltage_thresh_high_value,
> > > > > >   			     0644,
> > > > > >   			     ad7280_read_channel_config,
> > > > > >   			     ad7280_write_channel_config,
> > > > >
> > > > > Did you try building this code?
> > > > >
> > > > > It catches everyone...
> > > >
> > > > The problem is it builds. The token is stringyfied and
> > > > "in_voltage - voltage_thresh_high_value" is a valid string.
> > >
> > > Ah, I thought it used to break the build when it happened.  Oh well,
> > > it's still a great "trick" to see if people understand C or not :)
> > >
> > Yes, it did build. I am sorry but I am not following you fully. Can you
> > please let me know if the change I introduced is not good. You may
> > please direct me to a document where I can read more about it.
> 
> The code involves a macro, as indicated by the capital letters.  You will
> see the issue when you look at the definition of the macro.
> 
> julia

Thank you Julia and all. I got my mistake. I will revert the change.
Sorry for the trouble.

Deepak.
> 
> >
> > Thanks,
> > Deepak.
> > > thanks,
> > >
> > > greg k-h
> >
> > --
> > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20200318162353.GA23226%40deeUbuntu.
> >

Re: [Outreachy kernel] [PATCH 1/2] staging: iio: adc: ad7192: Reformat lines crossing 80 columns

[DEEPAK VARMA]

On Wed, Mar 18, 2020 at 05:44:16PM +0100, Lars-Peter Clausen wrote:
> On 3/18/20 5:43 PM, DEEPAK VARMA wrote:
> > On Wed, Mar 18, 2020 at 09:52:41PM +0530, Rohit Sarkar wrote:
> > > On Wed, Mar 18, 2020 at 09:36:50PM +0530, DEEPAK VARMA wrote:
> > > > On Wed, Mar 18, 2020 at 09:31:58AM +0100, Stefano Brivio wrote:
> > > > > On Wed, 18 Mar 2020 09:56:59 +0530
> > > > > Deepak R Varma <mh12gx2825@gmail.com> wrote:
> > > > > 
> > > > > > Macro arguments are computed at the time of macro invocation. This makes
> > > > > > the lines cross 80 column width. Add variables to perform the
> > > > > > calculations before hand and use these new variable in the macro calls
> > > > > > instead.
> > > > > > 
> > > > > > Also re-indent enum members to address checkpatch warning / check messages.
> > > > > > 
> > > > > > Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> > > > > > ---
> > > > > >   drivers/staging/iio/adc/ad7192.c | 15 ++++++++-------
> > > > > >   1 file changed, 8 insertions(+), 7 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
> > > > > > index bf3e2a9cc07f..0265f6607d75 100644
> > > > > > --- a/drivers/staging/iio/adc/ad7192.c
> > > > > > +++ b/drivers/staging/iio/adc/ad7192.c
> > > > > > @@ -156,8 +156,8 @@
> > > > > >    */
> > > > > >   enum {
> > > > > > -   AD7192_SYSCALIB_ZERO_SCALE,
> > > > > > -   AD7192_SYSCALIB_FULL_SCALE,
> > > > > > +	AD7192_SYSCALIB_ZERO_SCALE,
> > > > > > +	AD7192_SYSCALIB_FULL_SCALE,
> > > > > >   };
> > > > > >   struct ad7192_state {
> > > > > > @@ -477,17 +477,18 @@ static ssize_t ad7192_set(struct device *dev,
> > > > > >   }
> > > > > >   static void ad7192_get_available_filter_freq(struct ad7192_state *st,
> > > > > > -						    int *freq)
> > > > > > +					     int *freq)
> > > > > >   {
> > > > > >   	unsigned int fadc;
> > > > > > +	unsigned int sync3_filter, sync4_filter;
> > > > > >   	/* Formulas for filter at page 25 of the datasheet */
> > > > > > -	fadc = DIV_ROUND_CLOSEST(st->fclk,
> > > > > > -				 AD7192_SYNC4_FILTER * AD7192_MODE_RATE(st->mode));
> > > > > > +	sync4_filter = AD7192_SYNC4_FILTER * AD7192_MODE_RATE(st->mode);
> > > > > 
> > > > > Have you read page 25 of the datasheet? Why is this called
> > > > > sync4_filter, with a 'y'?
> > > > > 
> > > > 
> > > > Sorry, I am not sure what you are referring to. Can you please elaborate
> > > > or point me to where the data sheet is located?
> > > > 
> > > > Deepak.
> > > 
> > > Hey Deepak,
> > > You can find the datasheet for ad7192 here https://pdf1.alldatasheet.com/datasheet-pdf/view/988287/AD/AD7192.html
> > > 
> > 
> > Thank you Rohit. I got it. I understand Stefano's comments now. I named
> > the variables with a 'y' to keep it similar to the macro
> > AD7192_SYNCn_FILTER. Let me know if the variable name looks odd and I
> > should rename it to sinc4_filter instead.
> 
> Hi,
> 
> Please send a patch to rename the macro to SINC as well. This is a typo in
> the macro.
> 
> Thanks,
> - Lars

Sure. Will send a separate patch for the macro name correction in the same
pathset.

Deepak.

Re: [Outreachy kernel] Re: [PATCH 2/2] staging: iio: adc: ad7280a: Add spaces around operators

[Ardelean, Alexandru]

On Wed, 2020-03-18 at 22:30 +0530, DEEPAK VARMA wrote:
> [External]
> 
> On Wed, Mar 18, 2020 at 05:28:17PM +0100, Julia Lawall wrote:
> > 
> > On Wed, 18 Mar 2020, DEEPAK VARMA wrote:
> > 
> > > On Wed, Mar 18, 2020 at 04:19:24PM +0100, Greg KH wrote:
> > > > On Wed, Mar 18, 2020 at 04:12:28PM +0100, Lars-Peter Clausen wrote:
> > > > > On 3/18/20 7:00 AM, Greg KH wrote:
> > > > > > On Wed, Mar 18, 2020 at 09:58:13AM +0530, Deepak R Varma wrote:
> > > > > > > Add spaces around operator symbols to improve readability. Warning
> > > > > > > flagged by checkpatch script.
> > > > > > > 
> > > > > > > Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> > > > > > > ---
> > > > > > >   drivers/staging/iio/adc/ad7280a.c | 4 ++--
> > > > > > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/drivers/staging/iio/adc/ad7280a.c
> > > > > > > b/drivers/staging/iio/adc/ad7280a.c
> > > > > > > index 19a5f244dcae..34ca0d09db85 100644
> > > > > > > --- a/drivers/staging/iio/adc/ad7280a.c
> > > > > > > +++ b/drivers/staging/iio/adc/ad7280a.c
> > > > > > > @@ -825,14 +825,14 @@ static irqreturn_t ad7280_event_handler(int
> > > > > > > irq, void *private)
> > > > > > >   }
> > > > > > >   static IIO_DEVICE_ATTR_NAMED(in_thresh_low_value,
> > > > > > > -			     in_voltage-voltage_thresh_low_value,
> > > > > > > +			     in_voltage - voltage_thresh_low_value,
> > > > > > >   			     0644,
> > > > > > >   			     ad7280_read_channel_config,
> > > > > > >   			     ad7280_write_channel_config,
> > > > > > >   			     AD7280A_CELL_UNDERVOLTAGE);
> > > > > > >   static IIO_DEVICE_ATTR_NAMED(in_thresh_high_value,
> > > > > > > -			     in_voltage-voltage_thresh_high_value,
> > > > > > > +			     in_voltage - voltage_thresh_high_value,
> > > > > > >   			     0644,
> > > > > > >   			     ad7280_read_channel_config,
> > > > > > >   			     ad7280_write_channel_config,
> > > > > > 
> > > > > > Did you try building this code?
> > > > > > 
> > > > > > It catches everyone...
> > > > > 
> > > > > The problem is it builds. The token is stringyfied and
> > > > > "in_voltage - voltage_thresh_high_value" is a valid string.
> > > > 
> > > > Ah, I thought it used to break the build when it happened.  Oh well,
> > > > it's still a great "trick" to see if people understand C or not :)
> > > > 
> > > Yes, it did build. I am sorry but I am not following you fully. Can you
> > > please let me know if the change I introduced is not good. You may
> > > please direct me to a document where I can read more about it.
> > 
> > The code involves a macro, as indicated by the capital letters.  You will
> > see the issue when you look at the definition of the macro.
> > 
> > julia
> 
> Thank you Julia and all. I got my mistake. I will revert the change.
> Sorry for the trouble.

I'll try to make some time to address this somehow, so that checkpatch doesn't
bump into this.

In the last 2-3 years, I think I saw 3-4 patches trying to address this [for
various Analog drivers].
So, don't feel too bad.


> 
> Deepak.
> > > Thanks,
> > > Deepak.
> > > > thanks,
> > > > 
> > > > greg k-h
> > > 
> > > --
> > > You received this message because you are subscribed to the Google Groups
> > > "outreachy-kernel" group.
> > > To unsubscribe from this group and stop receiving emails from it, send an
> > > email to outreachy-kernel+unsubscribe@googlegroups.com.
> > > To view this discussion on the web visit 
> > > https://groups.google.com/d/msgid/outreachy-kernel/20200318162353.GA23226%40deeUbuntu
> > > .
> > > 

Re: [Outreachy kernel] Re: [PATCH 2/2] staging: iio: adc: ad7280a: Add spaces around operators

[Julia Lawall]

On Thu, 19 Mar 2020, Ardelean, Alexandru wrote:

> On Wed, 2020-03-18 at 22:30 +0530, DEEPAK VARMA wrote:
> > [External]
> >
> > On Wed, Mar 18, 2020 at 05:28:17PM +0100, Julia Lawall wrote:
> > >
> > > On Wed, 18 Mar 2020, DEEPAK VARMA wrote:
> > >
> > > > On Wed, Mar 18, 2020 at 04:19:24PM +0100, Greg KH wrote:
> > > > > On Wed, Mar 18, 2020 at 04:12:28PM +0100, Lars-Peter Clausen wrote:
> > > > > > On 3/18/20 7:00 AM, Greg KH wrote:
> > > > > > > On Wed, Mar 18, 2020 at 09:58:13AM +0530, Deepak R Varma wrote:
> > > > > > > > Add spaces around operator symbols to improve readability. Warning
> > > > > > > > flagged by checkpatch script.
> > > > > > > >
> > > > > > > > Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> > > > > > > > ---
> > > > > > > >   drivers/staging/iio/adc/ad7280a.c | 4 ++--
> > > > > > > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/drivers/staging/iio/adc/ad7280a.c
> > > > > > > > b/drivers/staging/iio/adc/ad7280a.c
> > > > > > > > index 19a5f244dcae..34ca0d09db85 100644
> > > > > > > > --- a/drivers/staging/iio/adc/ad7280a.c
> > > > > > > > +++ b/drivers/staging/iio/adc/ad7280a.c
> > > > > > > > @@ -825,14 +825,14 @@ static irqreturn_t ad7280_event_handler(int
> > > > > > > > irq, void *private)
> > > > > > > >   }
> > > > > > > >   static IIO_DEVICE_ATTR_NAMED(in_thresh_low_value,
> > > > > > > > -			     in_voltage-voltage_thresh_low_value,
> > > > > > > > +			     in_voltage - voltage_thresh_low_value,
> > > > > > > >   			     0644,
> > > > > > > >   			     ad7280_read_channel_config,
> > > > > > > >   			     ad7280_write_channel_config,
> > > > > > > >   			     AD7280A_CELL_UNDERVOLTAGE);
> > > > > > > >   static IIO_DEVICE_ATTR_NAMED(in_thresh_high_value,
> > > > > > > > -			     in_voltage-voltage_thresh_high_value,
> > > > > > > > +			     in_voltage - voltage_thresh_high_value,
> > > > > > > >   			     0644,
> > > > > > > >   			     ad7280_read_channel_config,
> > > > > > > >   			     ad7280_write_channel_config,
> > > > > > >
> > > > > > > Did you try building this code?
> > > > > > >
> > > > > > > It catches everyone...
> > > > > >
> > > > > > The problem is it builds. The token is stringyfied and
> > > > > > "in_voltage - voltage_thresh_high_value" is a valid string.
> > > > >
> > > > > Ah, I thought it used to break the build when it happened.  Oh well,
> > > > > it's still a great "trick" to see if people understand C or not :)
> > > > >
> > > > Yes, it did build. I am sorry but I am not following you fully. Can you
> > > > please let me know if the change I introduced is not good. You may
> > > > please direct me to a document where I can read more about it.
> > >
> > > The code involves a macro, as indicated by the capital letters.  You will
> > > see the issue when you look at the definition of the macro.
> > >
> > > julia
> >
> > Thank you Julia and all. I got my mistake. I will revert the change.
> > Sorry for the trouble.
>
> I'll try to make some time to address this somehow, so that checkpatch doesn't
> bump into this.
>
> In the last 2-3 years, I think I saw 3-4 patches trying to address this [for
> various Analog drivers].
> So, don't feel too bad.

Maybe a comment?

julia

>
>
> >
> > Deepak.
> > > > Thanks,
> > > > Deepak.
> > > > > thanks,
> > > > >
> > > > > greg k-h
> > > >
> > > > --
> > > > You received this message because you are subscribed to the Google Groups
> > > > "outreachy-kernel" group.
> > > > To unsubscribe from this group and stop receiving emails from it, send an
> > > > email to outreachy-kernel+unsubscribe@googlegroups.com.
> > > > To view this discussion on the web visit
> > > > https://groups.google.com/d/msgid/outreachy-kernel/20200318162353.GA23226%40deeUbuntu
> > > > .
> > > >
>

Re: [Outreachy kernel] [PATCH 1/2] staging: iio: adc: ad7192: Reformat lines crossing 80 columns

[Ardelean, Alexandru]

On Wed, 2020-03-18 at 21:52 +0530, Rohit Sarkar wrote:
> On Wed, Mar 18, 2020 at 09:36:50PM +0530, DEEPAK VARMA wrote:
> > On Wed, Mar 18, 2020 at 09:31:58AM +0100, Stefano Brivio wrote:
> > > On Wed, 18 Mar 2020 09:56:59 +0530
> > > Deepak R Varma <mh12gx2825@gmail.com> wrote:
> > > 
> > > > Macro arguments are computed at the time of macro invocation. This makes
> > > > the lines cross 80 column width. Add variables to perform the
> > > > calculations before hand and use these new variable in the macro calls
> > > > instead.
> > > > 
> > > > Also re-indent enum members to address checkpatch warning / check
> > > > messages.
> > > > 
> > > > Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> > > > ---
> > > >  drivers/staging/iio/adc/ad7192.c | 15 ++++++++-------
> > > >  1 file changed, 8 insertions(+), 7 deletions(-)
> > > > 
> > > > diff --git a/drivers/staging/iio/adc/ad7192.c
> > > > b/drivers/staging/iio/adc/ad7192.c
> > > > index bf3e2a9cc07f..0265f6607d75 100644
> > > > --- a/drivers/staging/iio/adc/ad7192.c
> > > > +++ b/drivers/staging/iio/adc/ad7192.c
> > > > @@ -156,8 +156,8 @@
> > > >   */
> > > >  
> > > >  enum {
> > > > -   AD7192_SYSCALIB_ZERO_SCALE,
> > > > -   AD7192_SYSCALIB_FULL_SCALE,
> > > > +	AD7192_SYSCALIB_ZERO_SCALE,
> > > > +	AD7192_SYSCALIB_FULL_SCALE,
> > > >  };
> > > >  
> > > >  struct ad7192_state {
> > > > @@ -477,17 +477,18 @@ static ssize_t ad7192_set(struct device *dev,
> > > >  }
> > > >  
> > > >  static void ad7192_get_available_filter_freq(struct ad7192_state *st,
> > > > -						    int *freq)
> > > > +					     int *freq)
> > > >  {
> > > >  	unsigned int fadc;
> > > > +	unsigned int sync3_filter, sync4_filter;
> > > >  
> > > >  	/* Formulas for filter at page 25 of the datasheet */
> > > > -	fadc = DIV_ROUND_CLOSEST(st->fclk,
> > > > -				 AD7192_SYNC4_FILTER *
> > > > AD7192_MODE_RATE(st->mode));
> > > > +	sync4_filter = AD7192_SYNC4_FILTER * AD7192_MODE_RATE(st->mode);
> > > 
> > > Have you read page 25 of the datasheet? Why is this called
> > > sync4_filter, with a 'y'?
> > > 
> > 
> > Sorry, I am not sure what you are referring to. Can you please elaborate
> > or point me to where the data sheet is located?
> > 
> > Deepak.
> 
> Hey Deepak,
> You can find the datasheet for ad7192 here 
> https://pdf1.alldatasheet.com/datasheet-pdf/view/988287/AD/AD7192.html


Most Analog datasheets can be found directly on the official website.

https://www.analog.com/media/en/technical-documentation/data-sheets/AD7192.pdf

The simplest way is to open a browser, type:  analog.com/<part-name>  [in this
case analog.com/ad7192], that opens-up the part page, scroll down and there's a
link to the datasheet [in this case, the link above].

Particularly for Analog parts, this is a bit simpler than searching on Google.
Though Google will find the official datasheets quicker, after you visit the
analog.com site a few times.

> 
> Thanks,
> Rohit

Re: [Outreachy kernel] Re: [PATCH 2/2] staging: iio: adc: ad7280a: Add spaces around operators

[Ardelean, Alexandru]

On Thu, 2020-03-19 at 08:03 +0100, Julia Lawall wrote:
> [External]
> 
> 
> 
> On Thu, 19 Mar 2020, Ardelean, Alexandru wrote:
> 
> > On Wed, 2020-03-18 at 22:30 +0530, DEEPAK VARMA wrote:
> > > [External]
> > > 
> > > On Wed, Mar 18, 2020 at 05:28:17PM +0100, Julia Lawall wrote:
> > > > On Wed, 18 Mar 2020, DEEPAK VARMA wrote:
> > > > 
> > > > > On Wed, Mar 18, 2020 at 04:19:24PM +0100, Greg KH wrote:
> > > > > > On Wed, Mar 18, 2020 at 04:12:28PM +0100, Lars-Peter Clausen wrote:
> > > > > > > On 3/18/20 7:00 AM, Greg KH wrote:
> > > > > > > > On Wed, Mar 18, 2020 at 09:58:13AM +0530, Deepak R Varma wrote:
> > > > > > > > > Add spaces around operator symbols to improve readability.
> > > > > > > > > Warning
> > > > > > > > > flagged by checkpatch script.
> > > > > > > > > 
> > > > > > > > > Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> > > > > > > > > ---
> > > > > > > > >   drivers/staging/iio/adc/ad7280a.c | 4 ++--
> > > > > > > > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > > > > > 
> > > > > > > > > diff --git a/drivers/staging/iio/adc/ad7280a.c
> > > > > > > > > b/drivers/staging/iio/adc/ad7280a.c
> > > > > > > > > index 19a5f244dcae..34ca0d09db85 100644
> > > > > > > > > --- a/drivers/staging/iio/adc/ad7280a.c
> > > > > > > > > +++ b/drivers/staging/iio/adc/ad7280a.c
> > > > > > > > > @@ -825,14 +825,14 @@ static irqreturn_t
> > > > > > > > > ad7280_event_handler(int
> > > > > > > > > irq, void *private)
> > > > > > > > >   }
> > > > > > > > >   static IIO_DEVICE_ATTR_NAMED(in_thresh_low_value,
> > > > > > > > > -			     in_voltage-
> > > > > > > > > voltage_thresh_low_value,
> > > > > > > > > +			     in_voltage -
> > > > > > > > > voltage_thresh_low_value,
> > > > > > > > >   			     0644,
> > > > > > > > >   			     ad7280_read_channel_config,
> > > > > > > > >   			     ad7280_write_channel_config,
> > > > > > > > >   			     AD7280A_CELL_UNDERVOLTAGE);
> > > > > > > > >   static IIO_DEVICE_ATTR_NAMED(in_thresh_high_value,
> > > > > > > > > -			     in_voltage-
> > > > > > > > > voltage_thresh_high_value,
> > > > > > > > > +			     in_voltage -
> > > > > > > > > voltage_thresh_high_value,
> > > > > > > > >   			     0644,
> > > > > > > > >   			     ad7280_read_channel_config,
> > > > > > > > >   			     ad7280_write_channel_config,
> > > > > > > > 
> > > > > > > > Did you try building this code?
> > > > > > > > 
> > > > > > > > It catches everyone...
> > > > > > > 
> > > > > > > The problem is it builds. The token is stringyfied and
> > > > > > > "in_voltage - voltage_thresh_high_value" is a valid string.
> > > > > > 
> > > > > > Ah, I thought it used to break the build when it happened.  Oh well,
> > > > > > it's still a great "trick" to see if people understand C or not :)
> > > > > > 
> > > > > Yes, it did build. I am sorry but I am not following you fully. Can
> > > > > you
> > > > > please let me know if the change I introduced is not good. You may
> > > > > please direct me to a document where I can read more about it.
> > > > 
> > > > The code involves a macro, as indicated by the capital letters.  You
> > > > will
> > > > see the issue when you look at the definition of the macro.
> > > > 
> > > > julia
> > > 
> > > Thank you Julia and all. I got my mistake. I will revert the change.
> > > Sorry for the trouble.
> > 
> > I'll try to make some time to address this somehow, so that checkpatch
> > doesn't
> > bump into this.
> > 
> > In the last 2-3 years, I think I saw 3-4 patches trying to address this [for
> > various Analog drivers].
> > So, don't feel too bad.
> 
> Maybe a comment?

Comment works for now.
Anybody wants to do a patch for that?
If nobody sends a patch for this in 1-2 weeks, I'll send one.

Particularly, this would help with review, since people that are unfamiliar with
IIO-specific macros would also find it easier at review.

The good part, is that it's only needed for AD7280A.
AD7192 has been re-worked, and is no longer on checkpatch's radar.
I'm reworking AD7793 now.


> 
> julia
> 
> > 
> > > Deepak.
> > > > > Thanks,
> > > > > Deepak.
> > > > > > thanks,
> > > > > > 
> > > > > > greg k-h
> > > > > 
> > > > > --
> > > > > You received this message because you are subscribed to the Google
> > > > > Groups
> > > > > "outreachy-kernel" group.
> > > > > To unsubscribe from this group and stop receiving emails from it, send
> > > > > an
> > > > > email to outreachy-kernel+unsubscribe@googlegroups.com.
> > > > > To view this discussion on the web visit
> > > > > https://groups.google.com/d/msgid/outreachy-kernel/20200318162353.GA23226%40deeUbuntu
> > > > > .
> > > > > 

Re: [Outreachy kernel] Re: [PATCH 2/2] staging: iio: adc: ad7280a: Add spaces around operators

[DEEPAK VARMA]

On Thu, Mar 19, 2020 at 07:07:20AM +0000, Ardelean, Alexandru wrote:
> On Thu, 2020-03-19 at 08:03 +0100, Julia Lawall wrote:
> > [External]
> > 
> > 
> > 
> > On Thu, 19 Mar 2020, Ardelean, Alexandru wrote:
> > 
> > > On Wed, 2020-03-18 at 22:30 +0530, DEEPAK VARMA wrote:
> > > > [External]
> > > > 
> > > > On Wed, Mar 18, 2020 at 05:28:17PM +0100, Julia Lawall wrote:
> > > > > On Wed, 18 Mar 2020, DEEPAK VARMA wrote:
> > > > > 
> > > > > > On Wed, Mar 18, 2020 at 04:19:24PM +0100, Greg KH wrote:
> > > > > > > On Wed, Mar 18, 2020 at 04:12:28PM +0100, Lars-Peter Clausen wrote:
> > > > > > > > On 3/18/20 7:00 AM, Greg KH wrote:
> > > > > > > > > On Wed, Mar 18, 2020 at 09:58:13AM +0530, Deepak R Varma wrote:
> > > > > > > > > > Add spaces around operator symbols to improve readability.
> > > > > > > > > > Warning
> > > > > > > > > > flagged by checkpatch script.
> > > > > > > > > > 
> > > > > > > > > > Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> > > > > > > > > > ---
> > > > > > > > > >   drivers/staging/iio/adc/ad7280a.c | 4 ++--
> > > > > > > > > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > > > > > > 
> > > > > > > > > > diff --git a/drivers/staging/iio/adc/ad7280a.c
> > > > > > > > > > b/drivers/staging/iio/adc/ad7280a.c
> > > > > > > > > > index 19a5f244dcae..34ca0d09db85 100644
> > > > > > > > > > --- a/drivers/staging/iio/adc/ad7280a.c
> > > > > > > > > > +++ b/drivers/staging/iio/adc/ad7280a.c
> > > > > > > > > > @@ -825,14 +825,14 @@ static irqreturn_t
> > > > > > > > > > ad7280_event_handler(int
> > > > > > > > > > irq, void *private)
> > > > > > > > > >   }
> > > > > > > > > >   static IIO_DEVICE_ATTR_NAMED(in_thresh_low_value,
> > > > > > > > > > -			     in_voltage-
> > > > > > > > > > voltage_thresh_low_value,
> > > > > > > > > > +			     in_voltage -
> > > > > > > > > > voltage_thresh_low_value,
> > > > > > > > > >   			     0644,
> > > > > > > > > >   			     ad7280_read_channel_config,
> > > > > > > > > >   			     ad7280_write_channel_config,
> > > > > > > > > >   			     AD7280A_CELL_UNDERVOLTAGE);
> > > > > > > > > >   static IIO_DEVICE_ATTR_NAMED(in_thresh_high_value,
> > > > > > > > > > -			     in_voltage-
> > > > > > > > > > voltage_thresh_high_value,
> > > > > > > > > > +			     in_voltage -
> > > > > > > > > > voltage_thresh_high_value,
> > > > > > > > > >   			     0644,
> > > > > > > > > >   			     ad7280_read_channel_config,
> > > > > > > > > >   			     ad7280_write_channel_config,
> > > > > > > > > 
> > > > > > > > > Did you try building this code?
> > > > > > > > > 
> > > > > > > > > It catches everyone...
> > > > > > > > 
> > > > > > > > The problem is it builds. The token is stringyfied and
> > > > > > > > "in_voltage - voltage_thresh_high_value" is a valid string.
> > > > > > > 
> > > > > > > Ah, I thought it used to break the build when it happened.  Oh well,
> > > > > > > it's still a great "trick" to see if people understand C or not :)
> > > > > > > 
> > > > > > Yes, it did build. I am sorry but I am not following you fully. Can
> > > > > > you
> > > > > > please let me know if the change I introduced is not good. You may
> > > > > > please direct me to a document where I can read more about it.
> > > > > 
> > > > > The code involves a macro, as indicated by the capital letters.  You
> > > > > will
> > > > > see the issue when you look at the definition of the macro.
> > > > > 
> > > > > julia
> > > > 
> > > > Thank you Julia and all. I got my mistake. I will revert the change.
> > > > Sorry for the trouble.
> > > 
> > > I'll try to make some time to address this somehow, so that checkpatch
> > > doesn't
> > > bump into this.
> > > 
> > > In the last 2-3 years, I think I saw 3-4 patches trying to address this [for
> > > various Analog drivers].
> > > So, don't feel too bad.
> > 
> > Maybe a comment?
> 
> Comment works for now.
> Anybody wants to do a patch for that?
> If nobody sends a patch for this in 1-2 weeks, I'll send one.
> 
> Particularly, this would help with review, since people that are unfamiliar with
> IIO-specific macros would also find it easier at review.
> 
> The good part, is that it's only needed for AD7280A.
> AD7192 has been re-worked, and is no longer on checkpatch's radar.
> I'm reworking AD7793 now.
> 

I will be happy to add a comment around the code area to indicate
ignore checkpatch warning for the mentioned argument. Please confirm if
that is what you are expecting to be done.

Thanks,
Deepak.

> 
> > 
> > julia
> > 
> > > 
> > > > Deepak.
> > > > > > Thanks,
> > > > > > Deepak.
> > > > > > > thanks,
> > > > > > > 
> > > > > > > greg k-h
> > > > > > 
> > > > > > --
> > > > > > You received this message because you are subscribed to the Google
> > > > > > Groups
> > > > > > "outreachy-kernel" group.
> > > > > > To unsubscribe from this group and stop receiving emails from it, send
> > > > > > an
> > > > > > email to outreachy-kernel+unsubscribe@googlegroups.com.
> > > > > > To view this discussion on the web visit
> > > > > > https://groups.google.com/d/msgid/outreachy-kernel/20200318162353.GA23226%40deeUbuntu
> > > > > > .
> > > > > > 

Re: [Outreachy kernel] Re: [PATCH 2/2] staging: iio: adc: ad7280a: Add spaces around operators

[Ardelean, Alexandru]

On Thu, 2020-03-19 at 21:46 +0530, DEEPAK VARMA wrote:
> [External]
> 
> On Thu, Mar 19, 2020 at 07:07:20AM +0000, Ardelean, Alexandru wrote:
> > On Thu, 2020-03-19 at 08:03 +0100, Julia Lawall wrote:
> > > [External]
> > > 
> > > 
> > > 
> > > On Thu, 19 Mar 2020, Ardelean, Alexandru wrote:
> > > 
> > > > On Wed, 2020-03-18 at 22:30 +0530, DEEPAK VARMA wrote:
> > > > > [External]
> > > > > 
> > > > > On Wed, Mar 18, 2020 at 05:28:17PM +0100, Julia Lawall wrote:
> > > > > > On Wed, 18 Mar 2020, DEEPAK VARMA wrote:
> > > > > > 
> > > > > > > On Wed, Mar 18, 2020 at 04:19:24PM +0100, Greg KH wrote:
> > > > > > > > On Wed, Mar 18, 2020 at 04:12:28PM +0100, Lars-Peter Clausen
> > > > > > > > wrote:
> > > > > > > > > On 3/18/20 7:00 AM, Greg KH wrote:
> > > > > > > > > > On Wed, Mar 18, 2020 at 09:58:13AM +0530, Deepak R Varma
> > > > > > > > > > wrote:
> > > > > > > > > > > Add spaces around operator symbols to improve readability.
> > > > > > > > > > > Warning
> > > > > > > > > > > flagged by checkpatch script.
> > > > > > > > > > > 
> > > > > > > > > > > Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> > > > > > > > > > > ---
> > > > > > > > > > >   drivers/staging/iio/adc/ad7280a.c | 4 ++--
> > > > > > > > > > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > > > > > > > 
> > > > > > > > > > > diff --git a/drivers/staging/iio/adc/ad7280a.c
> > > > > > > > > > > b/drivers/staging/iio/adc/ad7280a.c
> > > > > > > > > > > index 19a5f244dcae..34ca0d09db85 100644
> > > > > > > > > > > --- a/drivers/staging/iio/adc/ad7280a.c
> > > > > > > > > > > +++ b/drivers/staging/iio/adc/ad7280a.c
> > > > > > > > > > > @@ -825,14 +825,14 @@ static irqreturn_t
> > > > > > > > > > > ad7280_event_handler(int
> > > > > > > > > > > irq, void *private)
> > > > > > > > > > >   }
> > > > > > > > > > >   static IIO_DEVICE_ATTR_NAMED(in_thresh_low_value,
> > > > > > > > > > > -			     in_voltage-
> > > > > > > > > > > voltage_thresh_low_value,
> > > > > > > > > > > +			     in_voltage -
> > > > > > > > > > > voltage_thresh_low_value,
> > > > > > > > > > >   			     0644,
> > > > > > > > > > >   			     ad7280_read_channel_config,
> > > > > > > > > > >   			     ad7280_write_channel_config
> > > > > > > > > > > ,
> > > > > > > > > > >   			     AD7280A_CELL_UNDERVOLTAGE);
> > > > > > > > > > >   static IIO_DEVICE_ATTR_NAMED(in_thresh_high_value,
> > > > > > > > > > > -			     in_voltage-
> > > > > > > > > > > voltage_thresh_high_value,
> > > > > > > > > > > +			     in_voltage -
> > > > > > > > > > > voltage_thresh_high_value,
> > > > > > > > > > >   			     0644,
> > > > > > > > > > >   			     ad7280_read_channel_config,
> > > > > > > > > > >   			     ad7280_write_channel_config
> > > > > > > > > > > ,
> > > > > > > > > > 
> > > > > > > > > > Did you try building this code?
> > > > > > > > > > 
> > > > > > > > > > It catches everyone...
> > > > > > > > > 
> > > > > > > > > The problem is it builds. The token is stringyfied and
> > > > > > > > > "in_voltage - voltage_thresh_high_value" is a valid string.
> > > > > > > > 
> > > > > > > > Ah, I thought it used to break the build when it happened.  Oh
> > > > > > > > well,
> > > > > > > > it's still a great "trick" to see if people understand C or not
> > > > > > > > :)
> > > > > > > > 
> > > > > > > Yes, it did build. I am sorry but I am not following you fully.
> > > > > > > Can
> > > > > > > you
> > > > > > > please let me know if the change I introduced is not good. You may
> > > > > > > please direct me to a document where I can read more about it.
> > > > > > 
> > > > > > The code involves a macro, as indicated by the capital letters.  You
> > > > > > will
> > > > > > see the issue when you look at the definition of the macro.
> > > > > > 
> > > > > > julia
> > > > > 
> > > > > Thank you Julia and all. I got my mistake. I will revert the change.
> > > > > Sorry for the trouble.
> > > > 
> > > > I'll try to make some time to address this somehow, so that checkpatch
> > > > doesn't
> > > > bump into this.
> > > > 
> > > > In the last 2-3 years, I think I saw 3-4 patches trying to address this
> > > > [for
> > > > various Analog drivers].
> > > > So, don't feel too bad.
> > > 
> > > Maybe a comment?
> > 
> > Comment works for now.
> > Anybody wants to do a patch for that?
> > If nobody sends a patch for this in 1-2 weeks, I'll send one.
> > 
> > Particularly, this would help with review, since people that are unfamiliar
> > with
> > IIO-specific macros would also find it easier at review.
> > 
> > The good part, is that it's only needed for AD7280A.
> > AD7192 has been re-worked, and is no longer on checkpatch's radar.
> > I'm reworking AD7793 now.
> > 
> 
> I will be happy to add a comment around the code area to indicate
> ignore checkpatch warning for the mentioned argument. Please confirm if
> that is what you are expecting to be done.

Sure.
Please send a patch.

Thanks
Alex

> 
> Thanks,
> Deepak.
> 
> > > julia
> > > 
> > > > > Deepak.
> > > > > > > Thanks,
> > > > > > > Deepak.
> > > > > > > > thanks,
> > > > > > > > 
> > > > > > > > greg k-h
> > > > > > > 
> > > > > > > --
> > > > > > > You received this message because you are subscribed to the Google
> > > > > > > Groups
> > > > > > > "outreachy-kernel" group.
> > > > > > > To unsubscribe from this group and stop receiving emails from it,
> > > > > > > send
> > > > > > > an
> > > > > > > email to outreachy-kernel+unsubscribe@googlegroups.com.
> > > > > > > To view this discussion on the web visit
> > > > > > > https://groups.google.com/d/msgid/outreachy-kernel/20200318162353.GA23226%40deeUbuntu
> > > > > > > .
> > > > > > > 

Re: [Outreachy kernel] Re: [PATCH 2/2] staging: iio: adc: ad7280a: Add spaces around operators

[DEEPAK VARMA]

On Thu, Mar 19, 2020 at 09:46:10PM +0530, DEEPAK VARMA wrote:
> On Thu, Mar 19, 2020 at 07:07:20AM +0000, Ardelean, Alexandru wrote:
> > On Thu, 2020-03-19 at 08:03 +0100, Julia Lawall wrote:
> > > [External]
> > > 
> > > 
> > > 
> > > On Thu, 19 Mar 2020, Ardelean, Alexandru wrote:
> > > 
> > > > On Wed, 2020-03-18 at 22:30 +0530, DEEPAK VARMA wrote:
> > > > > [External]
> > > > > 
> > > > > On Wed, Mar 18, 2020 at 05:28:17PM +0100, Julia Lawall wrote:
> > > > > > On Wed, 18 Mar 2020, DEEPAK VARMA wrote:
> > > > > > 
> > > > > > > On Wed, Mar 18, 2020 at 04:19:24PM +0100, Greg KH wrote:
> > > > > > > > On Wed, Mar 18, 2020 at 04:12:28PM +0100, Lars-Peter Clausen wrote:
> > > > > > > > > On 3/18/20 7:00 AM, Greg KH wrote:
> > > > > > > > > > On Wed, Mar 18, 2020 at 09:58:13AM +0530, Deepak R Varma wrote:
> > > > > > > > > > > Add spaces around operator symbols to improve readability.
> > > > > > > > > > > Warning
> > > > > > > > > > > flagged by checkpatch script.
> > > > > > > > > > > 
> > > > > > > > > > > Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> > > > > > > > > > > ---
> > > > > > > > > > >   drivers/staging/iio/adc/ad7280a.c | 4 ++--
> > > > > > > > > > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > > > > > > > 
> > > > > > > > > > > diff --git a/drivers/staging/iio/adc/ad7280a.c
> > > > > > > > > > > b/drivers/staging/iio/adc/ad7280a.c
> > > > > > > > > > > index 19a5f244dcae..34ca0d09db85 100644
> > > > > > > > > > > --- a/drivers/staging/iio/adc/ad7280a.c
> > > > > > > > > > > +++ b/drivers/staging/iio/adc/ad7280a.c
> > > > > > > > > > > @@ -825,14 +825,14 @@ static irqreturn_t
> > > > > > > > > > > ad7280_event_handler(int
> > > > > > > > > > > irq, void *private)
> > > > > > > > > > >   }
> > > > > > > > > > >   static IIO_DEVICE_ATTR_NAMED(in_thresh_low_value,
> > > > > > > > > > > -			     in_voltage-
> > > > > > > > > > > voltage_thresh_low_value,
> > > > > > > > > > > +			     in_voltage -
> > > > > > > > > > > voltage_thresh_low_value,
> > > > > > > > > > >   			     0644,
> > > > > > > > > > >   			     ad7280_read_channel_config,
> > > > > > > > > > >   			     ad7280_write_channel_config,
> > > > > > > > > > >   			     AD7280A_CELL_UNDERVOLTAGE);
> > > > > > > > > > >   static IIO_DEVICE_ATTR_NAMED(in_thresh_high_value,
> > > > > > > > > > > -			     in_voltage-
> > > > > > > > > > > voltage_thresh_high_value,
> > > > > > > > > > > +			     in_voltage -
> > > > > > > > > > > voltage_thresh_high_value,
> > > > > > > > > > >   			     0644,
> > > > > > > > > > >   			     ad7280_read_channel_config,
> > > > > > > > > > >   			     ad7280_write_channel_config,
> > > > > > > > > > 
> > > > > > > > > > Did you try building this code?
> > > > > > > > > > 
> > > > > > > > > > It catches everyone...
> > > > > > > > > 
> > > > > > > > > The problem is it builds. The token is stringyfied and
> > > > > > > > > "in_voltage - voltage_thresh_high_value" is a valid string.
> > > > > > > > 
> > > > > > > > Ah, I thought it used to break the build when it happened.  Oh well,
> > > > > > > > it's still a great "trick" to see if people understand C or not :)
> > > > > > > > 
> > > > > > > Yes, it did build. I am sorry but I am not following you fully. Can
> > > > > > > you
> > > > > > > please let me know if the change I introduced is not good. You may
> > > > > > > please direct me to a document where I can read more about it.
> > > > > > 
> > > > > > The code involves a macro, as indicated by the capital letters.  You
> > > > > > will
> > > > > > see the issue when you look at the definition of the macro.
> > > > > > 
> > > > > > julia
> > > > > 
> > > > > Thank you Julia and all. I got my mistake. I will revert the change.
> > > > > Sorry for the trouble.
> > > > 
> > > > I'll try to make some time to address this somehow, so that checkpatch
> > > > doesn't
> > > > bump into this.
> > > > 
> > > > In the last 2-3 years, I think I saw 3-4 patches trying to address this [for
> > > > various Analog drivers].
> > > > So, don't feel too bad.
> > > 
> > > Maybe a comment?
> > 
> > Comment works for now.
> > Anybody wants to do a patch for that?
> > If nobody sends a patch for this in 1-2 weeks, I'll send one.
> > 
> > Particularly, this would help with review, since people that are unfamiliar with
> > IIO-specific macros would also find it easier at review.
> > 
> > The good part, is that it's only needed for AD7280A.
> > AD7192 has been re-worked, and is no longer on checkpatch's radar.
> > I'm reworking AD7793 now.
> > 
> 
> I will be happy to add a comment around the code area to indicate
> ignore checkpatch warning for the mentioned argument. Please confirm if
> that is what you are expecting to be done.
> 

Hello Alexandru,
could you please confirm if I should add a comment around the code in
ad7280a.c to avoid further changes being made to the string argument?


> Thanks,
> Deepak.
> 
> > 
> > > 
> > > julia
> > > 
> > > > 
> > > > > Deepak.
> > > > > > > Thanks,
> > > > > > > Deepak.
> > > > > > > > thanks,
> > > > > > > > 
> > > > > > > > greg k-h
> > > > > > > 
> > > > > > > --
> > > > > > > You received this message because you are subscribed to the Google
> > > > > > > Groups
> > > > > > > "outreachy-kernel" group.
> > > > > > > To unsubscribe from this group and stop receiving emails from it, send
> > > > > > > an
> > > > > > > email to outreachy-kernel+unsubscribe@googlegroups.com.
> > > > > > > To view this discussion on the web visit
> > > > > > > https://groups.google.com/d/msgid/outreachy-kernel/20200318162353.GA23226%40deeUbuntu
> > > > > > > .
> > > > > > > 

Re: [Outreachy kernel] Re: [PATCH 2/2] staging: iio: adc: ad7280a: Add spaces around operators

[Ardelean, Alexandru]

On Sat, 2020-03-21 at 15:55 +0530, DEEPAK VARMA wrote:
> [External]
> 
> On Thu, Mar 19, 2020 at 09:46:10PM +0530, DEEPAK VARMA wrote:
> > On Thu, Mar 19, 2020 at 07:07:20AM +0000, Ardelean, Alexandru wrote:
> > > On Thu, 2020-03-19 at 08:03 +0100, Julia Lawall wrote:
> > > > [External]
> > > > 
> > > > 
> > > > 
> > > > On Thu, 19 Mar 2020, Ardelean, Alexandru wrote:
> > > > 
> > > > > On Wed, 2020-03-18 at 22:30 +0530, DEEPAK VARMA wrote:
> > > > > > [External]
> > > > > > 
> > > > > > On Wed, Mar 18, 2020 at 05:28:17PM +0100, Julia Lawall wrote:
> > > > > > > On Wed, 18 Mar 2020, DEEPAK VARMA wrote:
> > > > > > > 
> > > > > > > > On Wed, Mar 18, 2020 at 04:19:24PM +0100, Greg KH wrote:
> > > > > > > > > On Wed, Mar 18, 2020 at 04:12:28PM +0100, Lars-Peter Clausen
> > > > > > > > > wrote:
> > > > > > > > > > On 3/18/20 7:00 AM, Greg KH wrote:
> > > > > > > > > > > On Wed, Mar 18, 2020 at 09:58:13AM +0530, Deepak R Varma
> > > > > > > > > > > wrote:
> > > > > > > > > > > > Add spaces around operator symbols to improve
> > > > > > > > > > > > readability.
> > > > > > > > > > > > Warning
> > > > > > > > > > > > flagged by checkpatch script.
> > > > > > > > > > > > 
> > > > > > > > > > > > Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> > > > > > > > > > > > ---
> > > > > > > > > > > >   drivers/staging/iio/adc/ad7280a.c | 4 ++--
> > > > > > > > > > > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > > > > > > > > 
> > > > > > > > > > > > diff --git a/drivers/staging/iio/adc/ad7280a.c
> > > > > > > > > > > > b/drivers/staging/iio/adc/ad7280a.c
> > > > > > > > > > > > index 19a5f244dcae..34ca0d09db85 100644
> > > > > > > > > > > > --- a/drivers/staging/iio/adc/ad7280a.c
> > > > > > > > > > > > +++ b/drivers/staging/iio/adc/ad7280a.c
> > > > > > > > > > > > @@ -825,14 +825,14 @@ static irqreturn_t
> > > > > > > > > > > > ad7280_event_handler(int
> > > > > > > > > > > > irq, void *private)
> > > > > > > > > > > >   }
> > > > > > > > > > > >   static IIO_DEVICE_ATTR_NAMED(in_thresh_low_value,
> > > > > > > > > > > > -			     in_voltage-
> > > > > > > > > > > > voltage_thresh_low_value,
> > > > > > > > > > > > +			     in_voltage -
> > > > > > > > > > > > voltage_thresh_low_value,
> > > > > > > > > > > >   			     0644,
> > > > > > > > > > > >   			     ad7280_read_channel_config,
> > > > > > > > > > > >   			     ad7280_write_channel_config
> > > > > > > > > > > > ,
> > > > > > > > > > > >   			     AD7280A_CELL_UNDERVOLTAGE);
> > > > > > > > > > > >   static IIO_DEVICE_ATTR_NAMED(in_thresh_high_value,
> > > > > > > > > > > > -			     in_voltage-
> > > > > > > > > > > > voltage_thresh_high_value,
> > > > > > > > > > > > +			     in_voltage -
> > > > > > > > > > > > voltage_thresh_high_value,
> > > > > > > > > > > >   			     0644,
> > > > > > > > > > > >   			     ad7280_read_channel_config,
> > > > > > > > > > > >   			     ad7280_write_channel_config
> > > > > > > > > > > > ,
> > > > > > > > > > > 
> > > > > > > > > > > Did you try building this code?
> > > > > > > > > > > 
> > > > > > > > > > > It catches everyone...
> > > > > > > > > > 
> > > > > > > > > > The problem is it builds. The token is stringyfied and
> > > > > > > > > > "in_voltage - voltage_thresh_high_value" is a valid string.
> > > > > > > > > 
> > > > > > > > > Ah, I thought it used to break the build when it happened.  Oh
> > > > > > > > > well,
> > > > > > > > > it's still a great "trick" to see if people understand C or
> > > > > > > > > not :)
> > > > > > > > > 
> > > > > > > > Yes, it did build. I am sorry but I am not following you fully.
> > > > > > > > Can
> > > > > > > > you
> > > > > > > > please let me know if the change I introduced is not good. You
> > > > > > > > may
> > > > > > > > please direct me to a document where I can read more about it.
> > > > > > > 
> > > > > > > The code involves a macro, as indicated by the capital
> > > > > > > letters.  You
> > > > > > > will
> > > > > > > see the issue when you look at the definition of the macro.
> > > > > > > 
> > > > > > > julia
> > > > > > 
> > > > > > Thank you Julia and all. I got my mistake. I will revert the change.
> > > > > > Sorry for the trouble.
> > > > > 
> > > > > I'll try to make some time to address this somehow, so that checkpatch
> > > > > doesn't
> > > > > bump into this.
> > > > > 
> > > > > In the last 2-3 years, I think I saw 3-4 patches trying to address
> > > > > this [for
> > > > > various Analog drivers].
> > > > > So, don't feel too bad.
> > > > 
> > > > Maybe a comment?
> > > 
> > > Comment works for now.
> > > Anybody wants to do a patch for that?
> > > If nobody sends a patch for this in 1-2 weeks, I'll send one.
> > > 
> > > Particularly, this would help with review, since people that are
> > > unfamiliar with
> > > IIO-specific macros would also find it easier at review.
> > > 
> > > The good part, is that it's only needed for AD7280A.
> > > AD7192 has been re-worked, and is no longer on checkpatch's radar.
> > > I'm reworking AD7793 now.
> > > 
> > 
> > I will be happy to add a comment around the code area to indicate
> > ignore checkpatch warning for the mentioned argument. Please confirm if
> > that is what you are expecting to be done.
> > 
> 
> Hello Alexandru,
> could you please confirm if I should add a comment around the code in
> ad7280a.c to avoid further changes being made to the string argument?

Sure.
Please send a patch.
[ I already replied this a few days ago; must have gotten lost in some emails ]


> 
> 
> > Thanks,
> > Deepak.
> > 
> > > > julia
> > > > 
> > > > > > Deepak.
> > > > > > > > Thanks,
> > > > > > > > Deepak.
> > > > > > > > > thanks,
> > > > > > > > > 
> > > > > > > > > greg k-h
> > > > > > > > 
> > > > > > > > --
> > > > > > > > You received this message because you are subscribed to the
> > > > > > > > Google
> > > > > > > > Groups
> > > > > > > > "outreachy-kernel" group.
> > > > > > > > To unsubscribe from this group and stop receiving emails from
> > > > > > > > it, send
> > > > > > > > an
> > > > > > > > email to outreachy-kernel+unsubscribe@googlegroups.com.
> > > > > > > > To view this discussion on the web visit
> > > > > > > > https://urldefense.com/v3/__https://groups.google.com/d/msgid/outreachy-kernel/20200318162353.GA23226*40deeUbuntu__;JQ!!A3Ni8CS0y2Y!uU95kG8KOMvTxz7PtgIm2gziXrrKPEfRDslhYEuDsrhPhXlYxaYcX-70a6Ipi1oLtgyinw$ 
> > > > > > > > .
> > > > > > > >