[PATCH 0/6] Staging: iio/frequency: Fix style issues

[PATCH 0/6] Staging: iio/frequency: Fix style issues

[Cristina Opriceana]

This patchset involves removing style issues in iio/frequency and
cleaning up as suggested by checkpatch.pl.

Cristina Opriceana (6):
  Staging: iio: Remove space after type cast
  Staging: iio: Remove explicit comparison to NULL
  Staging: iio: Prefer using the BIT macro
  Staging: iio: Do not use multiple blank lines
  Staging: iio: Alignment should match open parenthesis
  Staging: iio: Use braces on all arms of if statement

 drivers/staging/iio/frequency/ad9832.c | 10 +++++-----
 drivers/staging/iio/frequency/ad9832.h | 12 ++++++------
 drivers/staging/iio/frequency/ad9834.c | 31 +++++++++++++++----------------
 drivers/staging/iio/frequency/ad9834.h | 27 +++++++++++++--------------
 4 files changed, 39 insertions(+), 41 deletions(-)

-- 
1.9.1

[PATCH 1/6] Staging: iio: Remove space after type cast

[Cristina Opriceana]

This patch removes unnecessary space after type casts.
Warning found by checkpatch.pl.

Signed-off-by: Cristina Opriceana <cristina.opriceana@gmail.com>
---
 drivers/staging/iio/frequency/ad9832.c | 6 +++---
 drivers/staging/iio/frequency/ad9834.c | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
index cf68159..9a16699 100644
--- a/drivers/staging/iio/frequency/ad9832.c
+++ b/drivers/staging/iio/frequency/ad9832.c
@@ -24,8 +24,8 @@
 
 static unsigned long ad9832_calc_freqreg(unsigned long mclk, unsigned long fout)
 {
-	unsigned long long freqreg = (u64) fout *
-				     (u64) ((u64) 1L << AD9832_FREQ_BITS);
+	unsigned long long freqreg = (u64)fout *
+				     (u64)((u64)1L << AD9832_FREQ_BITS);
 	do_div(freqreg, mclk);
 	return freqreg;
 }
@@ -86,7 +86,7 @@ static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr,
 		goto error_ret;
 
 	mutex_lock(&indio_dev->mlock);
-	switch ((u32) this_attr->address) {
+	switch ((u32)this_attr->address) {
 	case AD9832_FREQ0HM:
 	case AD9832_FREQ1HM:
 		ret = ad9832_write_frequency(st, this_attr->address, val);
diff --git a/drivers/staging/iio/frequency/ad9834.c b/drivers/staging/iio/frequency/ad9834.c
index 5c80319..a346673 100644
--- a/drivers/staging/iio/frequency/ad9834.c
+++ b/drivers/staging/iio/frequency/ad9834.c
@@ -27,7 +27,7 @@
 
 static unsigned int ad9834_calc_freqreg(unsigned long mclk, unsigned long fout)
 {
-	unsigned long long freqreg = (u64) fout * (u64) (1 << AD9834_FREQ_BITS);
+	unsigned long long freqreg = (u64)fout * (u64)(1 << AD9834_FREQ_BITS);
 
 	do_div(freqreg, mclk);
 	return freqreg;
@@ -78,7 +78,7 @@ static ssize_t ad9834_write(struct device *dev,
 		goto error_ret;
 
 	mutex_lock(&indio_dev->mlock);
-	switch ((u32) this_attr->address) {
+	switch ((u32)this_attr->address) {
 	case AD9834_REG_FREQ0:
 	case AD9834_REG_FREQ1:
 		ret = ad9834_write_frequency(st, this_attr->address, val);
@@ -154,7 +154,7 @@ static ssize_t ad9834_store_wavetype(struct device *dev,
 
 	mutex_lock(&indio_dev->mlock);
 
-	switch ((u32) this_attr->address) {
+	switch ((u32)this_attr->address) {
 	case 0:
 		if (sysfs_streq(buf, "sine")) {
 			st->control &= ~AD9834_MODE;
-- 
1.9.1

[PATCH 2/6] Staging: iio: Remove explicit comparison to NULL

[Cristina Opriceana]

This patch simplifies pointer comparison to NULL and makes code
easier to read. Warning found by checkpatch.pl.

Signed-off-by: Cristina Opriceana <cristina.opriceana@gmail.com>
---
 drivers/staging/iio/frequency/ad9832.c | 2 +-
 drivers/staging/iio/frequency/ad9834.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
index 9a16699..8ecc0bc 100644
--- a/drivers/staging/iio/frequency/ad9832.c
+++ b/drivers/staging/iio/frequency/ad9832.c
@@ -220,7 +220,7 @@ static int ad9832_probe(struct spi_device *spi)
 	}
 
 	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
-	if (indio_dev == NULL) {
+	if (!indio_dev) {
 		ret = -ENOMEM;
 		goto error_disable_reg;
 	}
diff --git a/drivers/staging/iio/frequency/ad9834.c b/drivers/staging/iio/frequency/ad9834.c
index a346673..efea560 100644
--- a/drivers/staging/iio/frequency/ad9834.c
+++ b/drivers/staging/iio/frequency/ad9834.c
@@ -336,7 +336,7 @@ static int ad9834_probe(struct spi_device *spi)
 	}
 
 	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
-	if (indio_dev == NULL) {
+	if (!indio_dev) {
 		ret = -ENOMEM;
 		goto error_disable_reg;
 	}
-- 
1.9.1

[PATCH 3/6] Staging: iio: Prefer using the BIT macro

[Cristina Opriceana]

This patch replaces bit shifting on 1 with the BIT(x) macro
as it's extensively used by other functions in this driver.

This was done with coccinelle:
@@ int g; @@

-(1 << g)
+BIT(g)

Signed-off-by: Cristina Opriceana <cristina.opriceana@gmail.com>
---
 drivers/staging/iio/frequency/ad9832.c |  2 +-
 drivers/staging/iio/frequency/ad9832.h | 12 ++++++------
 drivers/staging/iio/frequency/ad9834.c |  4 ++--
 drivers/staging/iio/frequency/ad9834.h | 26 +++++++++++++-------------
 4 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
index 8ecc0bc..a861fe0 100644
--- a/drivers/staging/iio/frequency/ad9832.c
+++ b/drivers/staging/iio/frequency/ad9832.c
@@ -59,7 +59,7 @@ static int ad9832_write_frequency(struct ad9832_state *st,
 static int ad9832_write_phase(struct ad9832_state *st,
 			      unsigned long addr, unsigned long phase)
 {
-	if (phase > (1 << AD9832_PHASE_BITS))
+	if (phase > BIT(AD9832_PHASE_BITS))
 		return -EINVAL;
 
 	st->phase_data[0] = cpu_to_be16((AD9832_CMD_PHA8BITSW << CMD_SHIFT) |
diff --git a/drivers/staging/iio/frequency/ad9832.h b/drivers/staging/iio/frequency/ad9832.h
index 386f4dc..d32323b 100644
--- a/drivers/staging/iio/frequency/ad9832.h
+++ b/drivers/staging/iio/frequency/ad9832.h
@@ -42,13 +42,13 @@
 #define AD9832_CMD_SYNCSELSRC	0x8
 #define AD9832_CMD_SLEEPRESCLR	0xC
 
-#define AD9832_FREQ		(1 << 11)
+#define AD9832_FREQ		BIT(11)
 #define AD9832_PHASE(x)		(((x) & 3) << 9)
-#define AD9832_SYNC		(1 << 13)
-#define AD9832_SELSRC		(1 << 12)
-#define AD9832_SLEEP		(1 << 13)
-#define AD9832_RESET		(1 << 12)
-#define AD9832_CLR		(1 << 11)
+#define AD9832_SYNC		BIT(13)
+#define AD9832_SELSRC		BIT(12)
+#define AD9832_SLEEP		BIT(13)
+#define AD9832_RESET		BIT(12)
+#define AD9832_CLR		BIT(11)
 #define CMD_SHIFT		12
 #define ADD_SHIFT		8
 #define AD9832_FREQ_BITS	32
diff --git a/drivers/staging/iio/frequency/ad9834.c b/drivers/staging/iio/frequency/ad9834.c
index efea560..342c713 100644
--- a/drivers/staging/iio/frequency/ad9834.c
+++ b/drivers/staging/iio/frequency/ad9834.c
@@ -27,7 +27,7 @@
 
 static unsigned int ad9834_calc_freqreg(unsigned long mclk, unsigned long fout)
 {
-	unsigned long long freqreg = (u64)fout * (u64)(1 << AD9834_FREQ_BITS);
+	unsigned long long freqreg = (u64)fout * (u64)BIT(AD9834_FREQ_BITS);
 
 	do_div(freqreg, mclk);
 	return freqreg;
@@ -55,7 +55,7 @@ static int ad9834_write_frequency(struct ad9834_state *st,
 static int ad9834_write_phase(struct ad9834_state *st,
 				  unsigned long addr, unsigned long phase)
 {
-	if (phase > (1 << AD9834_PHASE_BITS))
+	if (phase > BIT(AD9834_PHASE_BITS))
 		return -EINVAL;
 	st->data = cpu_to_be16(addr | phase);
 
diff --git a/drivers/staging/iio/frequency/ad9834.h b/drivers/staging/iio/frequency/ad9834.h
index 8ca6e52..b54fb33 100644
--- a/drivers/staging/iio/frequency/ad9834.h
+++ b/drivers/staging/iio/frequency/ad9834.h
@@ -11,25 +11,25 @@
 /* Registers */
 
 #define AD9834_REG_CMD		(0 << 14)
-#define AD9834_REG_FREQ0	(1 << 14)
+#define AD9834_REG_FREQ0	BIT(14)
 #define AD9834_REG_FREQ1	(2 << 14)
 #define AD9834_REG_PHASE0	(6 << 13)
 #define AD9834_REG_PHASE1	(7 << 13)
 
 /* Command Control Bits */
 
-#define AD9834_B28		(1 << 13)
-#define AD9834_HLB		(1 << 12)
-#define AD9834_FSEL		(1 << 11)
-#define AD9834_PSEL		(1 << 10)
-#define AD9834_PIN_SW		(1 << 9)
-#define AD9834_RESET		(1 << 8)
-#define AD9834_SLEEP1		(1 << 7)
-#define AD9834_SLEEP12		(1 << 6)
-#define AD9834_OPBITEN		(1 << 5)
-#define AD9834_SIGN_PIB		(1 << 4)
-#define AD9834_DIV2		(1 << 3)
-#define AD9834_MODE		(1 << 1)
+#define AD9834_B28		BIT(13)
+#define AD9834_HLB		BIT(12)
+#define AD9834_FSEL		BIT(11)
+#define AD9834_PSEL		BIT(10)
+#define AD9834_PIN_SW		BIT(9)
+#define AD9834_RESET		BIT(8)
+#define AD9834_SLEEP1		BIT(7)
+#define AD9834_SLEEP12		BIT(6)
+#define AD9834_OPBITEN		BIT(5)
+#define AD9834_SIGN_PIB		BIT(4)
+#define AD9834_DIV2		BIT(3)
+#define AD9834_MODE		BIT(1)
 
 #define AD9834_FREQ_BITS	28
 #define AD9834_PHASE_BITS	12
-- 
1.9.1

[PATCH 4/6] Staging: iio: Do not use multiple blank lines

[Cristina Opriceana]

This patch removes unnecessary blank lines between functions.
Found by checkpatch.pl:
"CHECK: Please don't use multiple blank lines".

Signed-off-by: Cristina Opriceana <cristina.opriceana@gmail.com>
---
 drivers/staging/iio/frequency/ad9834.c | 1 -
 drivers/staging/iio/frequency/ad9834.h | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/staging/iio/frequency/ad9834.c b/drivers/staging/iio/frequency/ad9834.c
index 342c713..f06b14d 100644
--- a/drivers/staging/iio/frequency/ad9834.c
+++ b/drivers/staging/iio/frequency/ad9834.c
@@ -218,7 +218,6 @@ static ssize_t ad9834_show_out0_wavetype_available(struct device *dev,
 	return sprintf(buf, "%s\n", str);
 }
 
-
 static IIO_DEVICE_ATTR(out_altvoltage0_out0_wavetype_available, S_IRUGO,
 		       ad9834_show_out0_wavetype_available, NULL, 0);
 
diff --git a/drivers/staging/iio/frequency/ad9834.h b/drivers/staging/iio/frequency/ad9834.h
index b54fb33..c24dd90 100644
--- a/drivers/staging/iio/frequency/ad9834.h
+++ b/drivers/staging/iio/frequency/ad9834.h
@@ -69,7 +69,6 @@ struct ad9834_state {
 	__be16				freq_data[2];
 };
 
-
 /*
  * TODO: struct ad7887_platform_data needs to go into include/linux/iio
  */
-- 
1.9.1

[PATCH 5/6] Staging: iio: Alignment should match open parenthesis

[Cristina Opriceana]

This patch arranges multiple-line parameters in accordance with
the left side parenthesis to improve coding style.
Found by checkpatch.pl.

Signed-off-by: Cristina Opriceana <cristina.opriceana@gmail.com>
---
 drivers/staging/iio/frequency/ad9834.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/iio/frequency/ad9834.c b/drivers/staging/iio/frequency/ad9834.c
index f06b14d..cc465d5 100644
--- a/drivers/staging/iio/frequency/ad9834.c
+++ b/drivers/staging/iio/frequency/ad9834.c
@@ -53,7 +53,7 @@ static int ad9834_write_frequency(struct ad9834_state *st,
 }
 
 static int ad9834_write_phase(struct ad9834_state *st,
-				  unsigned long addr, unsigned long phase)
+			      unsigned long addr, unsigned long phase)
 {
 	if (phase > BIT(AD9834_PHASE_BITS))
 		return -EINVAL;
@@ -63,9 +63,9 @@ static int ad9834_write_phase(struct ad9834_state *st,
 }
 
 static ssize_t ad9834_write(struct device *dev,
-		struct device_attribute *attr,
-		const char *buf,
-		size_t len)
+			    struct device_attribute *attr,
+			    const char *buf,
+			    size_t len)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct ad9834_state *st = iio_priv(indio_dev);
@@ -142,9 +142,9 @@ error_ret:
 }
 
 static ssize_t ad9834_store_wavetype(struct device *dev,
-				 struct device_attribute *attr,
-				 const char *buf,
-				 size_t len)
+				     struct device_attribute *attr,
+				     const char *buf,
+				     size_t len)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct ad9834_state *st = iio_priv(indio_dev);
@@ -179,7 +179,7 @@ static ssize_t ad9834_store_wavetype(struct device *dev,
 		break;
 	case 1:
 		if (sysfs_streq(buf, "square") &&
-			!(st->control & AD9834_MODE)) {
+		    !(st->control & AD9834_MODE)) {
 			st->control &= ~AD9834_MODE;
 			st->control |= AD9834_OPBITEN;
 		} else {
-- 
1.9.1

[PATCH 6/6] Staging: iio: Use braces on all arms of if statement

[Cristina Opriceana]

Add braces to improve consistency when using if statements.
Found by checkpatch.pl:
"CHECK: braces {} should be used on all arms of this statement".

Signed-off-by: Cristina Opriceana <cristina.opriceana@gmail.com>
---
 drivers/staging/iio/frequency/ad9834.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/frequency/ad9834.c b/drivers/staging/iio/frequency/ad9834.c
index cc465d5..8e6cce5 100644
--- a/drivers/staging/iio/frequency/ad9834.c
+++ b/drivers/staging/iio/frequency/ad9834.c
@@ -111,9 +111,9 @@ static ssize_t ad9834_write(struct device *dev,
 		break;
 	case AD9834_FSEL:
 	case AD9834_PSEL:
-		if (val == 0)
+		if (val == 0) {
 			st->control &= ~(this_attr->address | AD9834_PIN_SW);
-		else if (val == 1) {
+		} else if (val == 1) {
 			st->control |= this_attr->address;
 			st->control &= ~AD9834_PIN_SW;
 		} else {
-- 
1.9.1

Re: [Outreachy kernel] [PATCH 5/6] Staging: iio: Alignment should match open parenthesis

[Julia Lawall]

On Sat, 14 Mar 2015, Cristina Opriceana wrote:

> This patch arranges multiple-line parameters in accordance with
> the left side parenthesis to improve coding style.
> Found by checkpatch.pl.

Why the left side of the (?  The right side, ie lined up with the 
parameter above, would look nicer.

julia

> Signed-off-by: Cristina Opriceana <cristina.opriceana@gmail.com>
> ---
>  drivers/staging/iio/frequency/ad9834.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/iio/frequency/ad9834.c b/drivers/staging/iio/frequency/ad9834.c
> index f06b14d..cc465d5 100644
> --- a/drivers/staging/iio/frequency/ad9834.c
> +++ b/drivers/staging/iio/frequency/ad9834.c
> @@ -53,7 +53,7 @@ static int ad9834_write_frequency(struct ad9834_state *st,
>  }
>  
>  static int ad9834_write_phase(struct ad9834_state *st,
> -				  unsigned long addr, unsigned long phase)
> +			      unsigned long addr, unsigned long phase)
>  {
>  	if (phase > BIT(AD9834_PHASE_BITS))
>  		return -EINVAL;
> @@ -63,9 +63,9 @@ static int ad9834_write_phase(struct ad9834_state *st,
>  }
>  
>  static ssize_t ad9834_write(struct device *dev,
> -		struct device_attribute *attr,
> -		const char *buf,
> -		size_t len)
> +			    struct device_attribute *attr,
> +			    const char *buf,
> +			    size_t len)
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	struct ad9834_state *st = iio_priv(indio_dev);
> @@ -142,9 +142,9 @@ error_ret:
>  }
>  
>  static ssize_t ad9834_store_wavetype(struct device *dev,
> -				 struct device_attribute *attr,
> -				 const char *buf,
> -				 size_t len)
> +				     struct device_attribute *attr,
> +				     const char *buf,
> +				     size_t len)
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	struct ad9834_state *st = iio_priv(indio_dev);
> @@ -179,7 +179,7 @@ static ssize_t ad9834_store_wavetype(struct device *dev,
>  		break;
>  	case 1:
>  		if (sysfs_streq(buf, "square") &&
> -			!(st->control & AD9834_MODE)) {
> +		    !(st->control & AD9834_MODE)) {
>  			st->control &= ~AD9834_MODE;
>  			st->control |= AD9834_OPBITEN;
>  		} else {
> -- 
> 1.9.1
> 
> -- 
> 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 post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/62ccda665be354990b06a5d12ccf410ce9e1013f.1426296329.git.cristina.opriceana%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
> 

Re: [Outreachy kernel] [PATCH 3/6] Staging: iio: Prefer using the BIT macro

[Julia Lawall]

> diff --git a/drivers/staging/iio/frequency/ad9834.h b/drivers/staging/iio/frequency/ad9834.h
> index 8ca6e52..b54fb33 100644
> --- a/drivers/staging/iio/frequency/ad9834.h
> +++ b/drivers/staging/iio/frequency/ad9834.h
> @@ -11,25 +11,25 @@
>  /* Registers */
>  
>  #define AD9834_REG_CMD		(0 << 14)
> -#define AD9834_REG_FREQ0	(1 << 14)
> +#define AD9834_REG_FREQ0	BIT(14)

This makes it different from its neighbors.  It doesn't seem like a good
idea here.

>  #define AD9834_REG_FREQ1	(2 << 14)
>  #define AD9834_REG_PHASE0	(6 << 13)
>  #define AD9834_REG_PHASE1	(7 << 13)
>  
>  /* Command Control Bits */
>  
> -#define AD9834_B28		(1 << 13)
> -#define AD9834_HLB		(1 << 12)
> -#define AD9834_FSEL		(1 << 11)
> -#define AD9834_PSEL		(1 << 10)
> -#define AD9834_PIN_SW		(1 << 9)
> -#define AD9834_RESET		(1 << 8)
> -#define AD9834_SLEEP1		(1 << 7)
> -#define AD9834_SLEEP12		(1 << 6)
> -#define AD9834_OPBITEN		(1 << 5)
> -#define AD9834_SIGN_PIB		(1 << 4)
> -#define AD9834_DIV2		(1 << 3)
> -#define AD9834_MODE		(1 << 1)
> +#define AD9834_B28		BIT(13)
> +#define AD9834_HLB		BIT(12)
> +#define AD9834_FSEL		BIT(11)
> +#define AD9834_PSEL		BIT(10)
> +#define AD9834_PIN_SW		BIT(9)

You could clean up the indentation at the same time.

julia

> +#define AD9834_RESET		BIT(8)
> +#define AD9834_SLEEP1		BIT(7)
> +#define AD9834_SLEEP12		BIT(6)
> +#define AD9834_OPBITEN		BIT(5)
> +#define AD9834_SIGN_PIB		BIT(4)
> +#define AD9834_DIV2		BIT(3)
> +#define AD9834_MODE		BIT(1)
>  
>  #define AD9834_FREQ_BITS	28
>  #define AD9834_PHASE_BITS	12
> -- 
> 1.9.1
> 
> -- 
> 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 post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/c77aa9f0cb01943bd9b2b8afd1a87807da692eb5.1426296329.git.cristina.opriceana%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
> 

Re: [Outreachy kernel] [PATCH 3/6] Staging: iio: Prefer using the BIT macro

[Daniel Baluta]

On Sat, Mar 14, 2015 at 8:36 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>> diff --git a/drivers/staging/iio/frequency/ad9834.h b/drivers/staging/iio/frequency/ad9834.h
>> index 8ca6e52..b54fb33 100644
>> --- a/drivers/staging/iio/frequency/ad9834.h
>> +++ b/drivers/staging/iio/frequency/ad9834.h
>> @@ -11,25 +11,25 @@
>>  /* Registers */
>>
>>  #define AD9834_REG_CMD               (0 << 14)
>> -#define AD9834_REG_FREQ0     (1 << 14)
>> +#define AD9834_REG_FREQ0     BIT(14)
>
> This makes it different from its neighbors.  It doesn't seem like a good
> idea here.

I guess computing the correct bits would be more readable.

e.g:

2 << 14 = BIT(15) | BIT(14)
7 << 13 = BIT(15) | BIT(14) | BIT(13)

You could also search for the datasheet to see the correct bit
definitions.

>
>>  #define AD9834_REG_FREQ1     (2 << 14)
>>  #define AD9834_REG_PHASE0    (6 << 13)
>>  #define AD9834_REG_PHASE1    (7 << 13)
>>
>>  /* Command Control Bits */
>>
>> -#define AD9834_B28           (1 << 13)
>> -#define AD9834_HLB           (1 << 12)
>> -#define AD9834_FSEL          (1 << 11)
>> -#define AD9834_PSEL          (1 << 10)
>> -#define AD9834_PIN_SW                (1 << 9)
>> -#define AD9834_RESET         (1 << 8)
>> -#define AD9834_SLEEP1                (1 << 7)
>> -#define AD9834_SLEEP12               (1 << 6)
>> -#define AD9834_OPBITEN               (1 << 5)
>> -#define AD9834_SIGN_PIB              (1 << 4)
>> -#define AD9834_DIV2          (1 << 3)
>> -#define AD9834_MODE          (1 << 1)
>> +#define AD9834_B28           BIT(13)
>> +#define AD9834_HLB           BIT(12)
>> +#define AD9834_FSEL          BIT(11)
>> +#define AD9834_PSEL          BIT(10)
>> +#define AD9834_PIN_SW                BIT(9)
>
> You could clean up the indentation at the same time.
>
> julia
>
>> +#define AD9834_RESET         BIT(8)
>> +#define AD9834_SLEEP1                BIT(7)
>> +#define AD9834_SLEEP12               BIT(6)
>> +#define AD9834_OPBITEN               BIT(5)
>> +#define AD9834_SIGN_PIB              BIT(4)
>> +#define AD9834_DIV2          BIT(3)
>> +#define AD9834_MODE          BIT(1)
>>
>>  #define AD9834_FREQ_BITS     28
>>  #define AD9834_PHASE_BITS    12

Re: [Outreachy kernel] [PATCH 5/6] Staging: iio: Alignment should match open parenthesis

[Daniel Baluta]

On Sat, Mar 14, 2015 at 8:34 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>
> On Sat, 14 Mar 2015, Cristina Opriceana wrote:
>
>> This patch arranges multiple-line parameters in accordance with
>> the left side parenthesis to improve coding style.
>> Found by checkpatch.pl.
>
> Why the left side of the (?  The right side, ie lined up with the
> parameter above, would look nicer.
>
> julia
>
>> Signed-off-by: Cristina Opriceana <cristina.opriceana@gmail.com>
>> ---
>>  drivers/staging/iio/frequency/ad9834.c | 16 ++++++++--------
>>  1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/staging/iio/frequency/ad9834.c b/drivers/staging/iio/frequency/ad9834.c
>> index f06b14d..cc465d5 100644
>> --- a/drivers/staging/iio/frequency/ad9834.c
>> +++ b/drivers/staging/iio/frequency/ad9834.c
>> @@ -53,7 +53,7 @@ static int ad9834_write_frequency(struct ad9834_state *st,
>>  }
>>
>>  static int ad9834_write_phase(struct ad9834_state *st,
>> -                               unsigned long addr, unsigned long phase)
>> +                           unsigned long addr, unsigned long phase)
>>  {
>>       if (phase > BIT(AD9834_PHASE_BITS))
>>               return -EINVAL;
>> @@ -63,9 +63,9 @@ static int ad9834_write_phase(struct ad9834_state *st,
>>  }
>>
>>  static ssize_t ad9834_write(struct device *dev,
>> -             struct device_attribute *attr,
>> -             const char *buf,
>> -             size_t len)
>> +                         struct device_attribute *attr,
>> +                         const char *buf,
>> +                         size_t len)
>>  {
>>       struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>       struct ad9834_state *st = iio_priv(indio_dev);
>> @@ -142,9 +142,9 @@ error_ret:
>>  }
>>
>>  static ssize_t ad9834_store_wavetype(struct device *dev,
>> -                              struct device_attribute *attr,
>> -                              const char *buf,
>> -                              size_t len)
>> +                                  struct device_attribute *attr,
>> +                                  const char *buf,
>> +                                  size_t len)
>>  {
>>       struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>       struct ad9834_state *st = iio_priv(indio_dev);
>> @@ -179,7 +179,7 @@ static ssize_t ad9834_store_wavetype(struct device *dev,
>>               break;
>>       case 1:
>>               if (sysfs_streq(buf, "square") &&
>> -                     !(st->control & AD9834_MODE)) {
>> +                 !(st->control & AD9834_MODE)) {
>>                       st->control &= ~AD9834_MODE;
>>                       st->control |= AD9834_OPBITEN;
>>               } else {

There are still few places to be fixed:
* ad9834_show_out0_wavetype_available
* ad9834_show_out1_wavetype_available

thanks,
Daniel.

Re: [Outreachy kernel] [PATCH 3/6] Staging: iio: Prefer using the BIT macro

[Cristina Opriceana]

> e.g:
> 
> 2 << 14 = BIT(15) | BIT(14)
> 7 << 13 = BIT(15) | BIT(14) | BIT(13)
> 
> You could also search for the datasheet to see the correct bit
> definitions.
> 
That's a very nice idea, thank you! 

> >>  /* Command Control Bits */
> >>
> >> -#define AD9834_B28           (1 << 13)
> >> -#define AD9834_HLB           (1 << 12)
> >> -#define AD9834_FSEL          (1 << 11)
> >> -#define AD9834_PSEL          (1 << 10)
> >> -#define AD9834_PIN_SW                (1 << 9)
> >> -#define AD9834_RESET         (1 << 8)
> >> -#define AD9834_SLEEP1                (1 << 7)
> >> -#define AD9834_SLEEP12               (1 << 6)
> >> -#define AD9834_OPBITEN               (1 << 5)
> >> -#define AD9834_SIGN_PIB              (1 << 4)
> >> -#define AD9834_DIV2          (1 << 3)
> >> -#define AD9834_MODE          (1 << 1)
> >> +#define AD9834_B28           BIT(13)
> >> +#define AD9834_HLB           BIT(12)
> >> +#define AD9834_FSEL          BIT(11)
> >> +#define AD9834_PSEL          BIT(10)
> >> +#define AD9834_PIN_SW                BIT(9)
> >
> > You could clean up the indentation at the same time.
> >
> > julia

I did check the indentation, cause it seemed to be inappropriate for me
too, but they are aligned in my sources. I don't know why git diff
messes with the indentation so badly. I thought it might be the
mix of tabs and spaces, but this is not the case, since they are aligned
at 2 tabs.
I'll try to modify them by hand, without the script, to see if this
problem still persists.

Thanks,
Cristina

Re: [Outreachy kernel] [PATCH 5/6] Staging: iio: Alignment should match open parenthesis

[Cristina Opriceana]

> There are still few places to be fixed:
> * ad9834_show_out0_wavetype_available
> * ad9834_show_out1_wavetype_available
> 
> thanks,
> Daniel.

I had doubts about fixing these because I had the following options:
1. Aligning them would go beyond the limit of 80 characters for the
second line.
2. Put static on top and make the function definition inconsistent with
the others.

static 
ssize_t ad9834_show_out0_wavetype_available(...)

Re: [Outreachy kernel] [PATCH 5/6] Staging: iio: Alignment should match open parenthesis

[Julia Lawall]

On Sat, 14 Mar 2015, Cristina Opriceana wrote:

>
> > There are still few places to be fixed:
> > * ad9834_show_out0_wavetype_available
> > * ad9834_show_out1_wavetype_available
> >
> > thanks,
> > Daniel.
>
> I had doubts about fixing these because I had the following options:
> 1. Aligning them would go beyond the limit of 80 characters for the
> second line.
> 2. Put static on top and make the function definition inconsistent with
> the others.
>
> static
> ssize_t ad9834_show_out0_wavetype_available(...)

That seems ok.

julia

>
> --
> 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 post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/1426337918.6721.4.camel%40Inspiron.
> For more options, visit https://groups.google.com/d/optout.
>