[PATCH 0/3] iio: ti_am335x_adc: small fix and cleanup

[PATCH 0/3] iio: ti_am335x_adc: small fix and cleanup

[Zubair Lutfullah]

These apply on togreg branch.

Fix/cleanup the am335x-adc driver.

1. Autobuilder picked up a missing static in function header
2. Trigger related code is redundant as driver is not using it
3. An error case that I could not figure out does what and why. 
Seemed like debug code to me. Removed it. ADC works without it.

Thanks
Zubair

Zubair Lutfullah (3):
  iio: ti_am335x_adc: fix static in function header
  iio: ti_am335x_adc: cleanup trigger related code
  iio: ti_am335x_adc: cleanup error case

 drivers/iio/adc/ti_am335x_adc.c |   16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

-- 
1.7.9.5

[PATCH 1/3] iio: ti_am335x_adc: fix static in function header

[Zubair Lutfullah]

Static is missing in function header. Corrected.

Signed-off-by: Zubair Lutfullah <zubair.lutfullah@gmail.com>
---
 drivers/iio/adc/ti_am335x_adc.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index 5287bff..91dce1f 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c
@@ -226,7 +226,7 @@ static const struct iio_buffer_setup_ops tiadc_buffer_setup_ops = {
 	.postdisable = &tiadc_buffer_postdisable,
 };
 
-int tiadc_iio_buffered_hardware_setup(struct iio_dev *indio_dev,
+static int tiadc_iio_buffered_hardware_setup(struct iio_dev *indio_dev,
 	irqreturn_t (*pollfunc_bh)(int irq, void *p),
 	irqreturn_t (*pollfunc_th)(int irq, void *p),
 	int irq,
-- 
1.7.9.5

Re: [PATCH 1/3] iio: ti_am335x_adc: fix static in function header

[Jonathan Cameron]

On 09/22/13 09:20, Zubair Lutfullah wrote:
> Static is missing in function header. Corrected.
> 
> Signed-off-by: Zubair Lutfullah <zubair.lutfullah@gmail.com>
Applied to the togreg branch of iio.git

As you've probably gathered, a run of sparse on your code before posting is a
great way to pick up on little things like this.  Also, coccinelle and
smatch if you want to do a thorough job - each gets a slightly different
set of common bugs.  I tend to run some combination of those, but wasn't
set up to build this driver so didn't pick up on this.

Another one clocked up to the autobuilders.  Note it is normal to give
a reported-by line to give them some credit. I've added one.

Jonathan
> ---
>  drivers/iio/adc/ti_am335x_adc.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
> index 5287bff..91dce1f 100644
> --- a/drivers/iio/adc/ti_am335x_adc.c
> +++ b/drivers/iio/adc/ti_am335x_adc.c
> @@ -226,7 +226,7 @@ static const struct iio_buffer_setup_ops tiadc_buffer_setup_ops = {
>  	.postdisable = &tiadc_buffer_postdisable,
>  };
>  
> -int tiadc_iio_buffered_hardware_setup(struct iio_dev *indio_dev,
> +static int tiadc_iio_buffered_hardware_setup(struct iio_dev *indio_dev,
>  	irqreturn_t (*pollfunc_bh)(int irq, void *p),
>  	irqreturn_t (*pollfunc_th)(int irq, void *p),
>  	int irq,
> 

Re: [PATCH 1/3] iio: ti_am335x_adc: fix static in function header

[Zubair Lutfullah :]

On Sun, Sep 22, 2013 at 07:03:53PM +0100, Jonathan Cameron wrote:
> On 09/22/13 09:20, Zubair Lutfullah wrote:
> > Static is missing in function header. Corrected.
> > 
> > Signed-off-by: Zubair Lutfullah <zubair.lutfullah@gmail.com>
> Applied to the togreg branch of iio.git
> 
> As you've probably gathered, a run of sparse on your code before posting is a
> great way to pick up on little things like this.  Also, coccinelle and
> smatch if you want to do a thorough job - each gets a slightly different
> set of common bugs.  I tend to run some combination of those, but wasn't
> set up to build this driver so didn't pick up on this.

Thank-you. I'll look into it. Are these scripts within the scripts/ directory?
Zubair

> 
> Another one clocked up to the autobuilders.  Note it is normal to give
> a reported-by line to give them some credit. I've added one.

Noted
 
> Jonathan
> > ---
> >  drivers/iio/adc/ti_am335x_adc.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
> > index 5287bff..91dce1f 100644
> > --- a/drivers/iio/adc/ti_am335x_adc.c
> > +++ b/drivers/iio/adc/ti_am335x_adc.c
> > @@ -226,7 +226,7 @@ static const struct iio_buffer_setup_ops tiadc_buffer_setup_ops = {
> >  	.postdisable = &tiadc_buffer_postdisable,
> >  };
> >  
> > -int tiadc_iio_buffered_hardware_setup(struct iio_dev *indio_dev,
> > +static int tiadc_iio_buffered_hardware_setup(struct iio_dev *indio_dev,
> >  	irqreturn_t (*pollfunc_bh)(int irq, void *p),
> >  	irqreturn_t (*pollfunc_th)(int irq, void *p),
> >  	int irq,
> > 

[PATCH 2/3] iio: ti_am335x_adc: cleanup trigger related code

[Zubair Lutfullah]

Trigger related headers and variables are not needed
as driver is now based on INDIO_BUFFER_HARDWARE mode

Signed-off-by: Zubair Lutfullah <zubair.lutfullah@gmail.com>
---
 drivers/iio/adc/ti_am335x_adc.c |    4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index 91dce1f..4eb63eb 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c
@@ -30,9 +30,6 @@
 #include <linux/mfd/ti_am335x_tscadc.h>
 #include <linux/iio/buffer.h>
 #include <linux/iio/kfifo_buf.h>
-#include <linux/iio/trigger.h>
-#include <linux/iio/trigger_consumer.h>
-#include <linux/iio/triggered_buffer.h>
 
 struct tiadc_device {
 	struct ti_tscadc_dev *mfd_tscadc;
@@ -40,7 +37,6 @@ struct tiadc_device {
 	u8 channel_line[8];
 	u8 channel_step[8];
 	int buffer_en_ch_steps;
-	struct iio_trigger *trig;
 	u16 data[8];
 };
 
-- 
1.7.9.5

Re: [PATCH 2/3] iio: ti_am335x_adc: cleanup trigger related code

[Jonathan Cameron]

On 09/22/13 09:20, Zubair Lutfullah wrote:
> Trigger related headers and variables are not needed
> as driver is now based on INDIO_BUFFER_HARDWARE mode
> 
> Signed-off-by: Zubair Lutfullah <zubair.lutfullah@gmail.com>
Applied
> ---
>  drivers/iio/adc/ti_am335x_adc.c |    4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
> index 91dce1f..4eb63eb 100644
> --- a/drivers/iio/adc/ti_am335x_adc.c
> +++ b/drivers/iio/adc/ti_am335x_adc.c
> @@ -30,9 +30,6 @@
>  #include <linux/mfd/ti_am335x_tscadc.h>
>  #include <linux/iio/buffer.h>
>  #include <linux/iio/kfifo_buf.h>
> -#include <linux/iio/trigger.h>
> -#include <linux/iio/trigger_consumer.h>
> -#include <linux/iio/triggered_buffer.h>
>  
>  struct tiadc_device {
>  	struct ti_tscadc_dev *mfd_tscadc;
> @@ -40,7 +37,6 @@ struct tiadc_device {
>  	u8 channel_line[8];
>  	u8 channel_step[8];
>  	int buffer_en_ch_steps;
> -	struct iio_trigger *trig;
>  	u16 data[8];
>  };
>  
> 

[PATCH 3/3] iio: ti_am335x_adc: cleanup error case

[Zubair Lutfullah]

Driver is functional without this error case. Cleaned up.

Signed-off-by: Zubair Lutfullah <zubair.lutfullah@gmail.com>
---
 drivers/iio/adc/ti_am335x_adc.c |   10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index 4eb63eb..8fb5429 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c
@@ -323,7 +323,6 @@ static int tiadc_read_raw(struct iio_dev *indio_dev,
 	struct tiadc_device *adc_dev = iio_priv(indio_dev);
 	int i, map_val;
 	unsigned int fifo1count, read, stepid;
-	u32 step = UINT_MAX;
 	bool found = false;
 	u32 step_en;
 	unsigned long timeout = jiffies + usecs_to_jiffies
@@ -353,15 +352,6 @@ static int tiadc_read_raw(struct iio_dev *indio_dev,
 	 * Hence we need to flush out this data.
 	 */
 
-	for (i = 0; i < ARRAY_SIZE(adc_dev->channel_step); i++) {
-		if (chan->channel == adc_dev->channel_line[i]) {
-			step = adc_dev->channel_step[i];
-			break;
-		}
-	}
-	if (WARN_ON_ONCE(step == UINT_MAX))
-		return -EINVAL;
-
 	fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
 	for (i = 0; i < fifo1count; i++) {
 		read = tiadc_readl(adc_dev, REG_FIFO1);
-- 
1.7.9.5

Re: [PATCH 3/3] iio: ti_am335x_adc: cleanup error case

[Jonathan Cameron]

On 09/22/13 09:20, Zubair Lutfullah wrote:
> Driver is functional without this error case. Cleaned up.
> 
> Signed-off-by: Zubair Lutfullah <zubair.lutfullah@gmail.com>
Applied and pull request just sent to Greg.

> ---
>  drivers/iio/adc/ti_am335x_adc.c |   10 ----------
>  1 file changed, 10 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
> index 4eb63eb..8fb5429 100644
> --- a/drivers/iio/adc/ti_am335x_adc.c
> +++ b/drivers/iio/adc/ti_am335x_adc.c
> @@ -323,7 +323,6 @@ static int tiadc_read_raw(struct iio_dev *indio_dev,
>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);
>  	int i, map_val;
>  	unsigned int fifo1count, read, stepid;
> -	u32 step = UINT_MAX;
>  	bool found = false;
>  	u32 step_en;
>  	unsigned long timeout = jiffies + usecs_to_jiffies
> @@ -353,15 +352,6 @@ static int tiadc_read_raw(struct iio_dev *indio_dev,
>  	 * Hence we need to flush out this data.
>  	 */
>  
> -	for (i = 0; i < ARRAY_SIZE(adc_dev->channel_step); i++) {
> -		if (chan->channel == adc_dev->channel_line[i]) {
> -			step = adc_dev->channel_step[i];
> -			break;
> -		}
> -	}
> -	if (WARN_ON_ONCE(step == UINT_MAX))
> -		return -EINVAL;
> -
>  	fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
>  	for (i = 0; i < fifo1count; i++) {
>  		read = tiadc_readl(adc_dev, REG_FIFO1);
>