[PATCH 1/4] staging:iio:trigger:bfintmr: Avoid divide by zero

[PATCH 1/4] staging:iio:trigger:bfintmr: Avoid divide by zero

[Lars-Peter Clausen]

If the timer frequency has not been configured yet get_gptimer_period() will
return 0. Handle this case instead of blindly dividing by the returned value.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/trigger/iio-trig-bfin-timer.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/trigger/iio-trig-bfin-timer.c b/drivers/staging/iio/trigger/iio-trig-bfin-timer.c
index ce6a7b1..2772ea2 100644
--- a/drivers/staging/iio/trigger/iio-trig-bfin-timer.c
+++ b/drivers/staging/iio/trigger/iio-trig-bfin-timer.c
@@ -99,9 +99,15 @@ static ssize_t iio_bfin_tmr_frequency_show(struct device *dev,
 {
 	struct iio_trigger *trig = to_iio_trigger(dev);
 	struct bfin_tmr_state *st = trig->private_data;
+	unsigned int period = get_gptimer_period(st->t->id);
+	unsigned long val;
 
-	return sprintf(buf, "%lu\n",
-			get_sclk() / get_gptimer_period(st->t->id));
+	if (period == 0)
+		val = 0;
+	else
+		val = get_sclk() / get_gptimer_period(st->t->id);
+
+	return sprintf(buf, "%lu\n", val);
 }
 
 static DEVICE_ATTR(frequency, S_IRUGO | S_IWUSR, iio_bfin_tmr_frequency_show,
-- 
1.7.10.4

[PATCH 2/4] staging:iio:trigger:bfintmr: Only enable timer when necessary

[Lars-Peter Clausen]

This patch hooks up the set_trigger_state callback for the blackfin timer
trigger driver and only enables the timer when a trigger consumer requests it to
be enabled. There really is no reason to keep the timer running and generate
interrupts if nobody is listening to them.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/trigger/iio-trig-bfin-timer.c |   27 ++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/iio/trigger/iio-trig-bfin-timer.c b/drivers/staging/iio/trigger/iio-trig-bfin-timer.c
index 2772ea2..d9d3dc9 100644
--- a/drivers/staging/iio/trigger/iio-trig-bfin-timer.c
+++ b/drivers/staging/iio/trigger/iio-trig-bfin-timer.c
@@ -57,12 +57,28 @@ struct bfin_tmr_state {
 	int irq;
 };
 
+static int iio_bfin_tmr_set_state(struct iio_trigger *trig, bool state)
+{
+	struct bfin_tmr_state *st = trig->private_data;
+
+	if (get_gptimer_period(st->t->id) == 0)
+		return -EINVAL;
+
+	if (state)
+		enable_gptimers(st->t->bit);
+	else
+		disable_gptimers(st->t->bit);
+
+	return 0;
+}
+
 static ssize_t iio_bfin_tmr_frequency_store(struct device *dev,
 		struct device_attribute *attr, const char *buf, size_t count)
 {
 	struct iio_trigger *trig = to_iio_trigger(dev);
 	struct bfin_tmr_state *st = trig->private_data;
 	long val;
+	bool enabled;
 	int ret;
 
 	ret = strict_strtoul(buf, 10, &val);
@@ -74,7 +90,10 @@ static ssize_t iio_bfin_tmr_frequency_store(struct device *dev,
 		goto error_ret;
 	}
 
-	disable_gptimers(st->t->bit);
+	enabled = get_enabled_gptimers() & st->t->bit;
+
+	if (enabled)
+		disable_gptimers(st->t->bit);
 
 	if (!val)
 		goto error_ret;
@@ -87,7 +106,9 @@ static ssize_t iio_bfin_tmr_frequency_store(struct device *dev,
 
 	set_gptimer_period(st->t->id, val);
 	set_gptimer_pwidth(st->t->id, 1);
-	enable_gptimers(st->t->bit);
+
+	if (enabled)
+		enable_gptimers(st->t->bit);
 
 error_ret:
 	return ret ? ret : count;
@@ -127,7 +148,6 @@ static const struct attribute_group *iio_bfin_tmr_trigger_attr_groups[] = {
 	NULL
 };
 
-
 static irqreturn_t iio_bfin_tmr_trigger_isr(int irq, void *devid)
 {
 	struct bfin_tmr_state *st = devid;
@@ -151,6 +171,7 @@ static int iio_bfin_tmr_get_number(int irq)
 
 static const struct iio_trigger_ops iio_bfin_tmr_trigger_ops = {
 	.owner = THIS_MODULE,
+	.set_trigger_state = iio_bfin_tmr_set_state,
 };
 
 static int __devinit iio_bfin_tmr_trigger_probe(struct platform_device *pdev)
-- 
1.7.10.4

Re: [PATCH 2/4] staging:iio:trigger:bfintmr: Only enable timer when necessary

[Jonathan Cameron]

On 09/17/2012 01:26 PM, Lars-Peter Clausen wrote:
> This patch hooks up the set_trigger_state callback for the blackfin timer
> trigger driver and only enables the timer when a trigger consumer requests it to
> be enabled. There really is no reason to keep the timer running and generate
> interrupts if nobody is listening to them.
I'll take your word for this one ;)

merged to togreg branch of iio.git
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
>  drivers/staging/iio/trigger/iio-trig-bfin-timer.c |   27 ++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/iio/trigger/iio-trig-bfin-timer.c b/drivers/staging/iio/trigger/iio-trig-bfin-timer.c
> index 2772ea2..d9d3dc9 100644
> --- a/drivers/staging/iio/trigger/iio-trig-bfin-timer.c
> +++ b/drivers/staging/iio/trigger/iio-trig-bfin-timer.c
> @@ -57,12 +57,28 @@ struct bfin_tmr_state {
>  	int irq;
>  };
>  
> +static int iio_bfin_tmr_set_state(struct iio_trigger *trig, bool state)
> +{
> +	struct bfin_tmr_state *st = trig->private_data;
> +
> +	if (get_gptimer_period(st->t->id) == 0)
> +		return -EINVAL;
> +
> +	if (state)
> +		enable_gptimers(st->t->bit);
> +	else
> +		disable_gptimers(st->t->bit);
> +
> +	return 0;
> +}
> +
>  static ssize_t iio_bfin_tmr_frequency_store(struct device *dev,
>  		struct device_attribute *attr, const char *buf, size_t count)
>  {
>  	struct iio_trigger *trig = to_iio_trigger(dev);
>  	struct bfin_tmr_state *st = trig->private_data;
>  	long val;
> +	bool enabled;
>  	int ret;
>  
>  	ret = strict_strtoul(buf, 10, &val);
> @@ -74,7 +90,10 @@ static ssize_t iio_bfin_tmr_frequency_store(struct device *dev,
>  		goto error_ret;
>  	}
>  
> -	disable_gptimers(st->t->bit);
> +	enabled = get_enabled_gptimers() & st->t->bit;
> +
> +	if (enabled)
> +		disable_gptimers(st->t->bit);
>  
>  	if (!val)
>  		goto error_ret;
> @@ -87,7 +106,9 @@ static ssize_t iio_bfin_tmr_frequency_store(struct device *dev,
>  
>  	set_gptimer_period(st->t->id, val);
>  	set_gptimer_pwidth(st->t->id, 1);
> -	enable_gptimers(st->t->bit);
> +
> +	if (enabled)
> +		enable_gptimers(st->t->bit);
>  
>  error_ret:
>  	return ret ? ret : count;
> @@ -127,7 +148,6 @@ static const struct attribute_group *iio_bfin_tmr_trigger_attr_groups[] = {
>  	NULL
>  };
>  
> -
>  static irqreturn_t iio_bfin_tmr_trigger_isr(int irq, void *devid)
>  {
>  	struct bfin_tmr_state *st = devid;
> @@ -151,6 +171,7 @@ static int iio_bfin_tmr_get_number(int irq)
>  
>  static const struct iio_trigger_ops iio_bfin_tmr_trigger_ops = {
>  	.owner = THIS_MODULE,
> +	.set_trigger_state = iio_bfin_tmr_set_state,
>  };
>  
>  static int __devinit iio_bfin_tmr_trigger_probe(struct platform_device *pdev)
> 

[PATCH 3/4] staging:iio:trigger:bfintmr Add output support

[Lars-Peter Clausen]

Some converters require an external signal to start the conversion. This patch
adds support to the bfintmr trigger driver to generate such a signal.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/trigger/iio-trig-bfin-timer.c |   72 ++++++++++++++++-----
 drivers/staging/iio/trigger/iio-trig-bfin-timer.h |   24 +++++++
 2 files changed, 80 insertions(+), 16 deletions(-)
 create mode 100644 drivers/staging/iio/trigger/iio-trig-bfin-timer.h

diff --git a/drivers/staging/iio/trigger/iio-trig-bfin-timer.c b/drivers/staging/iio/trigger/iio-trig-bfin-timer.c
index d9d3dc9..52062d7 100644
--- a/drivers/staging/iio/trigger/iio-trig-bfin-timer.c
+++ b/drivers/staging/iio/trigger/iio-trig-bfin-timer.c
@@ -14,14 +14,18 @@
 #include <linux/delay.h>
 
 #include <asm/gptimers.h>
+#include <asm/portmux.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/trigger.h>
 
+#include "iio-trig-bfin-timer.h"
+
 struct bfin_timer {
 	unsigned short id, bit;
 	unsigned long irqbit;
 	int irq;
+	int pin;
 };
 
 /*
@@ -30,22 +34,22 @@ struct bfin_timer {
  */
 
 static struct bfin_timer iio_bfin_timer_code[MAX_BLACKFIN_GPTIMERS] = {
-	{TIMER0_id,  TIMER0bit,  TIMER_STATUS_TIMIL0,  IRQ_TIMER0},
-	{TIMER1_id,  TIMER1bit,  TIMER_STATUS_TIMIL1,  IRQ_TIMER1},
-	{TIMER2_id,  TIMER2bit,  TIMER_STATUS_TIMIL2,  IRQ_TIMER2},
+	{TIMER0_id,  TIMER0bit,  TIMER_STATUS_TIMIL0,  IRQ_TIMER0, P_TMR0},
+	{TIMER1_id,  TIMER1bit,  TIMER_STATUS_TIMIL1,  IRQ_TIMER1, P_TMR1},
+	{TIMER2_id,  TIMER2bit,  TIMER_STATUS_TIMIL2,  IRQ_TIMER2, P_TMR2},
 #if (MAX_BLACKFIN_GPTIMERS > 3)
-	{TIMER3_id,  TIMER3bit,  TIMER_STATUS_TIMIL3,  IRQ_TIMER3},
-	{TIMER4_id,  TIMER4bit,  TIMER_STATUS_TIMIL4,  IRQ_TIMER4},
-	{TIMER5_id,  TIMER5bit,  TIMER_STATUS_TIMIL5,  IRQ_TIMER5},
-	{TIMER6_id,  TIMER6bit,  TIMER_STATUS_TIMIL6,  IRQ_TIMER6},
-	{TIMER7_id,  TIMER7bit,  TIMER_STATUS_TIMIL7,  IRQ_TIMER7},
+	{TIMER3_id,  TIMER3bit,  TIMER_STATUS_TIMIL3,  IRQ_TIMER3, P_TMR3},
+	{TIMER4_id,  TIMER4bit,  TIMER_STATUS_TIMIL4,  IRQ_TIMER4, P_TMR4},
+	{TIMER5_id,  TIMER5bit,  TIMER_STATUS_TIMIL5,  IRQ_TIMER5, P_TMR5},
+	{TIMER6_id,  TIMER6bit,  TIMER_STATUS_TIMIL6,  IRQ_TIMER6, P_TMR6},
+	{TIMER7_id,  TIMER7bit,  TIMER_STATUS_TIMIL7,  IRQ_TIMER7, P_TMR7},
 #endif
 #if (MAX_BLACKFIN_GPTIMERS > 8)
-	{TIMER8_id,  TIMER8bit,  TIMER_STATUS_TIMIL8,  IRQ_TIMER8},
-	{TIMER9_id,  TIMER9bit,  TIMER_STATUS_TIMIL9,  IRQ_TIMER9},
-	{TIMER10_id, TIMER10bit, TIMER_STATUS_TIMIL10, IRQ_TIMER10},
+	{TIMER8_id,  TIMER8bit,  TIMER_STATUS_TIMIL8,  IRQ_TIMER8, P_TMR8},
+	{TIMER9_id,  TIMER9bit,  TIMER_STATUS_TIMIL9,  IRQ_TIMER9, P_TMR9},
+	{TIMER10_id, TIMER10bit, TIMER_STATUS_TIMIL10, IRQ_TIMER10, P_TMR10},
 #if (MAX_BLACKFIN_GPTIMERS > 11)
-	{TIMER11_id, TIMER11bit, TIMER_STATUS_TIMIL11, IRQ_TIMER11},
+	{TIMER11_id, TIMER11bit, TIMER_STATUS_TIMIL11, IRQ_TIMER11, P_TMR11},
 #endif
 #endif
 };
@@ -54,6 +58,8 @@ struct bfin_tmr_state {
 	struct iio_trigger *trig;
 	struct bfin_timer *t;
 	unsigned timer_num;
+	bool output_enable;
+	unsigned int duty;
 	int irq;
 };
 
@@ -77,7 +83,7 @@ static ssize_t iio_bfin_tmr_frequency_store(struct device *dev,
 {
 	struct iio_trigger *trig = to_iio_trigger(dev);
 	struct bfin_tmr_state *st = trig->private_data;
-	long val;
+	unsigned long val;
 	bool enabled;
 	int ret;
 
@@ -99,13 +105,13 @@ static ssize_t iio_bfin_tmr_frequency_store(struct device *dev,
 		goto error_ret;
 
 	val = get_sclk() / val;
-	if (val <= 4) {
+	if (val <= 4 || val <= st->duty) {
 		ret = -EINVAL;
 		goto error_ret;
 	}
 
 	set_gptimer_period(st->t->id, val);
-	set_gptimer_pwidth(st->t->id, 1);
+	set_gptimer_pwidth(st->t->id, val - st->duty);
 
 	if (enabled)
 		enable_gptimers(st->t->bit);
@@ -176,7 +182,9 @@ static const struct iio_trigger_ops iio_bfin_tmr_trigger_ops = {
 
 static int __devinit iio_bfin_tmr_trigger_probe(struct platform_device *pdev)
 {
+	struct iio_bfin_timer_trigger_pdata *pdata = pdev->dev.platform_data;
 	struct bfin_tmr_state *st;
+	unsigned int config;
 	int ret;
 
 	st = kzalloc(sizeof(*st), GFP_KERNEL);
@@ -220,13 +228,43 @@ static int __devinit iio_bfin_tmr_trigger_probe(struct platform_device *pdev)
 		goto out4;
 	}
 
-	set_gptimer_config(st->t->id, OUT_DIS | PWM_OUT | PERIOD_CNT | IRQ_ENA);
+	config = PWM_OUT | PERIOD_CNT | IRQ_ENA;
+
+	if (pdata && pdata->output_enable) {
+		unsigned long long val;
+
+		st->output_enable = true;
+
+		ret = peripheral_request(st->t->pin, st->trig->name);
+		if (ret)
+			goto out_free_irq;
+
+		val = (unsigned long long)get_sclk() * pdata->duty_ns;
+		do_div(val, NSEC_PER_SEC);
+		st->duty = val;
+
+		/**
+		 * The interrupt will be generated at the end of the period,
+		 * since we want the interrupt to be generated at end of the
+		 * pulse we invert both polarity and duty cycle, so that the
+		 * pulse will be generated directly before the interrupt.
+		 */
+		if (pdata->active_low)
+			config |= PULSE_HI;
+	} else {
+		st->duty = 1;
+		config |= OUT_DIS;
+	}
+
+	set_gptimer_config(st->t->id, config);
 
 	dev_info(&pdev->dev, "iio trigger Blackfin TMR%d, IRQ-%d",
 		 st->timer_num, st->irq);
 	platform_set_drvdata(pdev, st);
 
 	return 0;
+out_free_irq:
+	free_irq(st->irq, st);
 out4:
 	iio_trigger_unregister(st->trig);
 out2:
@@ -242,6 +280,8 @@ static int __devexit iio_bfin_tmr_trigger_remove(struct platform_device *pdev)
 	struct bfin_tmr_state *st = platform_get_drvdata(pdev);
 
 	disable_gptimers(st->t->bit);
+	if (st->output_enable)
+		peripheral_free(st->t->pin);
 	free_irq(st->irq, st);
 	iio_trigger_unregister(st->trig);
 	iio_trigger_put(st->trig);
diff --git a/drivers/staging/iio/trigger/iio-trig-bfin-timer.h b/drivers/staging/iio/trigger/iio-trig-bfin-timer.h
new file mode 100644
index 0000000..c07321f
--- /dev/null
+++ b/drivers/staging/iio/trigger/iio-trig-bfin-timer.h
@@ -0,0 +1,24 @@
+#ifndef __IIO_BFIN_TIMER_TRIGGER_H__
+#define __IIO_BFIN_TIMER_TRIGGER_H__
+
+/**
+ * struct iio_bfin_timer_trigger_pdata - timer trigger platform data
+ * @output_enable: Enable external trigger pulse generation.
+ * @active_low: Whether the trigger pulse is active low.
+ * @duty_ns: Length of the trigger pulse in nanoseconds.
+ *
+ * This struct is used to configure the output pulse generation of the blackfin
+ * timer trigger. If output_enable is set to true an external trigger signal
+ * will generated on the pin corresponding to the timer. This is useful for
+ * converters which needs an external signal to start conversion. active_low and
+ * duty_ns are used to configure the type of the trigger pulse. If output_enable
+ * is set to false no external trigger pulse will be generated and active_low
+ * and duty_ns are ignored.
+ **/
+struct iio_bfin_timer_trigger_pdata {
+	bool output_enable;
+	bool active_low;
+	unsigned int duty_ns;
+};
+
+#endif
-- 
1.7.10.4

Re: [PATCH 3/4] staging:iio:trigger:bfintmr Add output support

[Jonathan Cameron]

On 09/17/2012 01:26 PM, Lars-Peter Clausen wrote:
> Some converters require an external signal to start the conversion. This patch
> adds support to the bfintmr trigger driver to generate such a signal.
cute feature.  Much easier than hammering a pin at roughly the right time.

Anyhow merged to togreg branch.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
>  drivers/staging/iio/trigger/iio-trig-bfin-timer.c |   72 ++++++++++++++++-----
>  drivers/staging/iio/trigger/iio-trig-bfin-timer.h |   24 +++++++
>  2 files changed, 80 insertions(+), 16 deletions(-)
>  create mode 100644 drivers/staging/iio/trigger/iio-trig-bfin-timer.h
> 
> diff --git a/drivers/staging/iio/trigger/iio-trig-bfin-timer.c b/drivers/staging/iio/trigger/iio-trig-bfin-timer.c
> index d9d3dc9..52062d7 100644
> --- a/drivers/staging/iio/trigger/iio-trig-bfin-timer.c
> +++ b/drivers/staging/iio/trigger/iio-trig-bfin-timer.c
> @@ -14,14 +14,18 @@
>  #include <linux/delay.h>
>  
>  #include <asm/gptimers.h>
> +#include <asm/portmux.h>
>  
>  #include <linux/iio/iio.h>
>  #include <linux/iio/trigger.h>
>  
> +#include "iio-trig-bfin-timer.h"
> +
>  struct bfin_timer {
>  	unsigned short id, bit;
>  	unsigned long irqbit;
>  	int irq;
> +	int pin;
>  };
>  
>  /*
> @@ -30,22 +34,22 @@ struct bfin_timer {
>   */
>  
>  static struct bfin_timer iio_bfin_timer_code[MAX_BLACKFIN_GPTIMERS] = {
> -	{TIMER0_id,  TIMER0bit,  TIMER_STATUS_TIMIL0,  IRQ_TIMER0},
> -	{TIMER1_id,  TIMER1bit,  TIMER_STATUS_TIMIL1,  IRQ_TIMER1},
> -	{TIMER2_id,  TIMER2bit,  TIMER_STATUS_TIMIL2,  IRQ_TIMER2},
> +	{TIMER0_id,  TIMER0bit,  TIMER_STATUS_TIMIL0,  IRQ_TIMER0, P_TMR0},
> +	{TIMER1_id,  TIMER1bit,  TIMER_STATUS_TIMIL1,  IRQ_TIMER1, P_TMR1},
> +	{TIMER2_id,  TIMER2bit,  TIMER_STATUS_TIMIL2,  IRQ_TIMER2, P_TMR2},
>  #if (MAX_BLACKFIN_GPTIMERS > 3)
> -	{TIMER3_id,  TIMER3bit,  TIMER_STATUS_TIMIL3,  IRQ_TIMER3},
> -	{TIMER4_id,  TIMER4bit,  TIMER_STATUS_TIMIL4,  IRQ_TIMER4},
> -	{TIMER5_id,  TIMER5bit,  TIMER_STATUS_TIMIL5,  IRQ_TIMER5},
> -	{TIMER6_id,  TIMER6bit,  TIMER_STATUS_TIMIL6,  IRQ_TIMER6},
> -	{TIMER7_id,  TIMER7bit,  TIMER_STATUS_TIMIL7,  IRQ_TIMER7},
> +	{TIMER3_id,  TIMER3bit,  TIMER_STATUS_TIMIL3,  IRQ_TIMER3, P_TMR3},
> +	{TIMER4_id,  TIMER4bit,  TIMER_STATUS_TIMIL4,  IRQ_TIMER4, P_TMR4},
> +	{TIMER5_id,  TIMER5bit,  TIMER_STATUS_TIMIL5,  IRQ_TIMER5, P_TMR5},
> +	{TIMER6_id,  TIMER6bit,  TIMER_STATUS_TIMIL6,  IRQ_TIMER6, P_TMR6},
> +	{TIMER7_id,  TIMER7bit,  TIMER_STATUS_TIMIL7,  IRQ_TIMER7, P_TMR7},
>  #endif
>  #if (MAX_BLACKFIN_GPTIMERS > 8)
> -	{TIMER8_id,  TIMER8bit,  TIMER_STATUS_TIMIL8,  IRQ_TIMER8},
> -	{TIMER9_id,  TIMER9bit,  TIMER_STATUS_TIMIL9,  IRQ_TIMER9},
> -	{TIMER10_id, TIMER10bit, TIMER_STATUS_TIMIL10, IRQ_TIMER10},
> +	{TIMER8_id,  TIMER8bit,  TIMER_STATUS_TIMIL8,  IRQ_TIMER8, P_TMR8},
> +	{TIMER9_id,  TIMER9bit,  TIMER_STATUS_TIMIL9,  IRQ_TIMER9, P_TMR9},
> +	{TIMER10_id, TIMER10bit, TIMER_STATUS_TIMIL10, IRQ_TIMER10, P_TMR10},
>  #if (MAX_BLACKFIN_GPTIMERS > 11)
> -	{TIMER11_id, TIMER11bit, TIMER_STATUS_TIMIL11, IRQ_TIMER11},
> +	{TIMER11_id, TIMER11bit, TIMER_STATUS_TIMIL11, IRQ_TIMER11, P_TMR11},
>  #endif
>  #endif
>  };
> @@ -54,6 +58,8 @@ struct bfin_tmr_state {
>  	struct iio_trigger *trig;
>  	struct bfin_timer *t;
>  	unsigned timer_num;
> +	bool output_enable;
> +	unsigned int duty;
>  	int irq;
>  };
>  
> @@ -77,7 +83,7 @@ static ssize_t iio_bfin_tmr_frequency_store(struct device *dev,
>  {
>  	struct iio_trigger *trig = to_iio_trigger(dev);
>  	struct bfin_tmr_state *st = trig->private_data;
> -	long val;
> +	unsigned long val;
>  	bool enabled;
>  	int ret;
>  
> @@ -99,13 +105,13 @@ static ssize_t iio_bfin_tmr_frequency_store(struct device *dev,
>  		goto error_ret;
>  
>  	val = get_sclk() / val;
> -	if (val <= 4) {
> +	if (val <= 4 || val <= st->duty) {
>  		ret = -EINVAL;
>  		goto error_ret;
>  	}
>  
>  	set_gptimer_period(st->t->id, val);
> -	set_gptimer_pwidth(st->t->id, 1);
> +	set_gptimer_pwidth(st->t->id, val - st->duty);
>  
>  	if (enabled)
>  		enable_gptimers(st->t->bit);
> @@ -176,7 +182,9 @@ static const struct iio_trigger_ops iio_bfin_tmr_trigger_ops = {
>  
>  static int __devinit iio_bfin_tmr_trigger_probe(struct platform_device *pdev)
>  {
> +	struct iio_bfin_timer_trigger_pdata *pdata = pdev->dev.platform_data;
>  	struct bfin_tmr_state *st;
> +	unsigned int config;
>  	int ret;
>  
>  	st = kzalloc(sizeof(*st), GFP_KERNEL);
> @@ -220,13 +228,43 @@ static int __devinit iio_bfin_tmr_trigger_probe(struct platform_device *pdev)
>  		goto out4;
>  	}
>  
> -	set_gptimer_config(st->t->id, OUT_DIS | PWM_OUT | PERIOD_CNT | IRQ_ENA);
> +	config = PWM_OUT | PERIOD_CNT | IRQ_ENA;
> +
> +	if (pdata && pdata->output_enable) {
> +		unsigned long long val;
> +
> +		st->output_enable = true;
> +
> +		ret = peripheral_request(st->t->pin, st->trig->name);
> +		if (ret)
> +			goto out_free_irq;
> +
> +		val = (unsigned long long)get_sclk() * pdata->duty_ns;
> +		do_div(val, NSEC_PER_SEC);
> +		st->duty = val;
> +
> +		/**
> +		 * The interrupt will be generated at the end of the period,
> +		 * since we want the interrupt to be generated at end of the
> +		 * pulse we invert both polarity and duty cycle, so that the
> +		 * pulse will be generated directly before the interrupt.
> +		 */
> +		if (pdata->active_low)
> +			config |= PULSE_HI;
> +	} else {
> +		st->duty = 1;
> +		config |= OUT_DIS;
> +	}
> +
> +	set_gptimer_config(st->t->id, config);
>  
>  	dev_info(&pdev->dev, "iio trigger Blackfin TMR%d, IRQ-%d",
>  		 st->timer_num, st->irq);
>  	platform_set_drvdata(pdev, st);
>  
>  	return 0;
> +out_free_irq:
> +	free_irq(st->irq, st);
>  out4:
>  	iio_trigger_unregister(st->trig);
>  out2:
> @@ -242,6 +280,8 @@ static int __devexit iio_bfin_tmr_trigger_remove(struct platform_device *pdev)
>  	struct bfin_tmr_state *st = platform_get_drvdata(pdev);
>  
>  	disable_gptimers(st->t->bit);
> +	if (st->output_enable)
> +		peripheral_free(st->t->pin);
>  	free_irq(st->irq, st);
>  	iio_trigger_unregister(st->trig);
>  	iio_trigger_put(st->trig);
> diff --git a/drivers/staging/iio/trigger/iio-trig-bfin-timer.h b/drivers/staging/iio/trigger/iio-trig-bfin-timer.h
> new file mode 100644
> index 0000000..c07321f
> --- /dev/null
> +++ b/drivers/staging/iio/trigger/iio-trig-bfin-timer.h
> @@ -0,0 +1,24 @@
> +#ifndef __IIO_BFIN_TIMER_TRIGGER_H__
> +#define __IIO_BFIN_TIMER_TRIGGER_H__
> +
> +/**
> + * struct iio_bfin_timer_trigger_pdata - timer trigger platform data
> + * @output_enable: Enable external trigger pulse generation.
> + * @active_low: Whether the trigger pulse is active low.
> + * @duty_ns: Length of the trigger pulse in nanoseconds.
> + *
> + * This struct is used to configure the output pulse generation of the blackfin
> + * timer trigger. If output_enable is set to true an external trigger signal
> + * will generated on the pin corresponding to the timer. This is useful for
> + * converters which needs an external signal to start conversion. active_low and
> + * duty_ns are used to configure the type of the trigger pulse. If output_enable
> + * is set to false no external trigger pulse will be generated and active_low
> + * and duty_ns are ignored.
> + **/
> +struct iio_bfin_timer_trigger_pdata {
> +	bool output_enable;
> +	bool active_low;
> +	unsigned int duty_ns;
> +};
> +
> +#endif
> 

[PATCH 4/4] iio: ad7476: Add support for the ad7091r

[Lars-Peter Clausen]

Add support for the ad7091r 12 bit ADC to the ad7476 driver. Although the
ad7091r is not really related to any of the other devices supported by this
driver, luckily for us there are not so many ways (which are not totally insane)
how sampling a single channel ADC via SPI can be implemented and support for the
ad7091r can be added to the driver with just a few adjustments.

The ad7091r requires an external "conversion start" pulse to start a sample
conversion. After the conversion has finished the result can be read via SPI. We
depend on a IIO trigger to generate this signal, as a result only sampling in
buffered mode and not in manual mode is available.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/iio/adc/ad7476.c |   30 ++++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/adc/ad7476.c b/drivers/iio/adc/ad7476.c
index be2098d..7f2f45a 100644
--- a/drivers/iio/adc/ad7476.c
+++ b/drivers/iio/adc/ad7476.c
@@ -23,9 +23,12 @@
 
 #define RES_MASK(bits)	((1 << (bits)) - 1)
 
+struct ad7476_state;
+
 struct ad7476_chip_info {
 	unsigned int			int_vref_uv;
 	struct iio_chan_spec		channel[2];
+	void (*reset)(struct ad7476_state *);
 };
 
 struct ad7476_state {
@@ -45,6 +48,7 @@ struct ad7476_state {
 };
 
 enum ad7476_supported_device_ids {
+	ID_AD7091R,
 	ID_AD7276,
 	ID_AD7277,
 	ID_AD7278,
@@ -79,6 +83,12 @@ done:
 	return IRQ_HANDLED;
 }
 
+static void ad7091_reset(struct ad7476_state *st)
+{
+	/* Any transfers with 8 scl cycles will reset the device */
+	spi_read(st->spi, st->data, 1);
+}
+
 static int ad7476_scan_direct(struct ad7476_state *st)
 {
 	int ret;
@@ -130,11 +140,11 @@ static int ad7476_read_raw(struct iio_dev *indio_dev,
 	return -EINVAL;
 }
 
-#define _AD7476_CHAN(bits, _shift)				\
+#define _AD7476_CHAN(bits, _shift, _info_mask)			\
 	{							\
 	.type = IIO_VOLTAGE,					\
 	.indexed = 1,						\
-	.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT |		\
+	.info_mask = _info_mask |				\
 	IIO_CHAN_INFO_SCALE_SHARED_BIT,				\
 	.scan_type = {						\
 		.sign = 'u',					\
@@ -145,10 +155,18 @@ static int ad7476_read_raw(struct iio_dev *indio_dev,
 	},							\
 }
 
-#define AD7476_CHAN(bits) _AD7476_CHAN((bits), 13 - (bits))
-#define AD7940_CHAN(bits) _AD7476_CHAN((bits), 15 - (bits))
+#define AD7476_CHAN(bits) _AD7476_CHAN((bits), 13 - (bits), \
+		IIO_CHAN_INFO_RAW_SEPARATE_BIT)
+#define AD7940_CHAN(bits) _AD7476_CHAN((bits), 15 - (bits), \
+		IIO_CHAN_INFO_RAW_SEPARATE_BIT)
+#define AD7091R_CHAN(bits) _AD7476_CHAN((bits), 16 - (bits), 0)
 
 static const struct ad7476_chip_info ad7476_chip_info_tbl[] = {
+	[ID_AD7091R] = {
+		.channel[0] = AD7091R_CHAN(12),
+		.channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
+		.reset = ad7091_reset,
+	},
 	[ID_AD7276] = {
 		.channel[0] = AD7940_CHAN(12),
 		.channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
@@ -238,6 +256,9 @@ static int __devinit ad7476_probe(struct spi_device *spi)
 	if (ret)
 		goto error_disable_reg;
 
+	if (st->chip_info->reset)
+		st->chip_info->reset(st);
+
 	ret = iio_device_register(indio_dev);
 	if (ret)
 		goto error_ring_unregister;
@@ -271,6 +292,7 @@ static int __devexit ad7476_remove(struct spi_device *spi)
 }
 
 static const struct spi_device_id ad7476_id[] = {
+	{"ad7091r", ID_AD7091R},
 	{"ad7273", ID_AD7277},
 	{"ad7274", ID_AD7276},
 	{"ad7276", ID_AD7276},
-- 
1.7.10.4

Re: [PATCH 4/4] iio: ad7476: Add support for the ad7091r

[Jonathan Cameron]

On 09/17/2012 01:26 PM, Lars-Peter Clausen wrote:
> Add support for the ad7091r 12 bit ADC to the ad7476 driver. Although the
> ad7091r is not really related to any of the other devices supported by this
> driver, luckily for us there are not so many ways (which are not totally insane)
> how sampling a single channel ADC via SPI can be implemented and support for the
> ad7091r can be added to the driver with just a few adjustments.
Yup, eveything is nice and simple when there is only one channel ;)
> 
> The ad7091r requires an external "conversion start" pulse to start a sample
> conversion. After the conversion has finished the result can be read via SPI. We
> depend on a IIO trigger to generate this signal, as a result only sampling in
> buffered mode and not in manual mode is available.
This is a little restrictive given we could wire up some magic with a gpio to
do this, but I guess things like that can be added as and when people care.
We may need some way of 'knowing' whether a more general trigger has done
the start pulse or not... Something for another day.

merged to togreg branch of iio.git
Thanks.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
>  drivers/iio/adc/ad7476.c |   30 ++++++++++++++++++++++++++----
>  1 file changed, 26 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7476.c b/drivers/iio/adc/ad7476.c
> index be2098d..7f2f45a 100644
> --- a/drivers/iio/adc/ad7476.c
> +++ b/drivers/iio/adc/ad7476.c
> @@ -23,9 +23,12 @@
>  
>  #define RES_MASK(bits)	((1 << (bits)) - 1)
>  
> +struct ad7476_state;
> +
>  struct ad7476_chip_info {
>  	unsigned int			int_vref_uv;
>  	struct iio_chan_spec		channel[2];
> +	void (*reset)(struct ad7476_state *);
>  };
>  
>  struct ad7476_state {
> @@ -45,6 +48,7 @@ struct ad7476_state {
>  };
>  
>  enum ad7476_supported_device_ids {
> +	ID_AD7091R,
>  	ID_AD7276,
>  	ID_AD7277,
>  	ID_AD7278,
> @@ -79,6 +83,12 @@ done:
>  	return IRQ_HANDLED;
>  }
>  
> +static void ad7091_reset(struct ad7476_state *st)
> +{
> +	/* Any transfers with 8 scl cycles will reset the device */
> +	spi_read(st->spi, st->data, 1);
> +}
> +
>  static int ad7476_scan_direct(struct ad7476_state *st)
>  {
>  	int ret;
> @@ -130,11 +140,11 @@ static int ad7476_read_raw(struct iio_dev *indio_dev,
>  	return -EINVAL;
>  }
>  
> -#define _AD7476_CHAN(bits, _shift)				\
> +#define _AD7476_CHAN(bits, _shift, _info_mask)			\
>  	{							\
>  	.type = IIO_VOLTAGE,					\
>  	.indexed = 1,						\
> -	.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT |		\
> +	.info_mask = _info_mask |				\
>  	IIO_CHAN_INFO_SCALE_SHARED_BIT,				\
>  	.scan_type = {						\
>  		.sign = 'u',					\
> @@ -145,10 +155,18 @@ static int ad7476_read_raw(struct iio_dev *indio_dev,
>  	},							\
>  }
>  
> -#define AD7476_CHAN(bits) _AD7476_CHAN((bits), 13 - (bits))
> -#define AD7940_CHAN(bits) _AD7476_CHAN((bits), 15 - (bits))
> +#define AD7476_CHAN(bits) _AD7476_CHAN((bits), 13 - (bits), \
> +		IIO_CHAN_INFO_RAW_SEPARATE_BIT)
> +#define AD7940_CHAN(bits) _AD7476_CHAN((bits), 15 - (bits), \
> +		IIO_CHAN_INFO_RAW_SEPARATE_BIT)
> +#define AD7091R_CHAN(bits) _AD7476_CHAN((bits), 16 - (bits), 0)
>  
>  static const struct ad7476_chip_info ad7476_chip_info_tbl[] = {
> +	[ID_AD7091R] = {
> +		.channel[0] = AD7091R_CHAN(12),
> +		.channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> +		.reset = ad7091_reset,
> +	},
>  	[ID_AD7276] = {
>  		.channel[0] = AD7940_CHAN(12),
>  		.channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> @@ -238,6 +256,9 @@ static int __devinit ad7476_probe(struct spi_device *spi)
>  	if (ret)
>  		goto error_disable_reg;
>  
> +	if (st->chip_info->reset)
> +		st->chip_info->reset(st);
> +
>  	ret = iio_device_register(indio_dev);
>  	if (ret)
>  		goto error_ring_unregister;
> @@ -271,6 +292,7 @@ static int __devexit ad7476_remove(struct spi_device *spi)
>  }
>  
>  static const struct spi_device_id ad7476_id[] = {
> +	{"ad7091r", ID_AD7091R},
>  	{"ad7273", ID_AD7277},
>  	{"ad7274", ID_AD7276},
>  	{"ad7276", ID_AD7276},
> 

Re: [PATCH 4/4] iio: ad7476: Add support for the ad7091r

[Lars-Peter Clausen]

erOn 09/17/2012 11:14 PM, Jonathan Cameron wrote:
> On 09/17/2012 01:26 PM, Lars-Peter Clausen wrote:
>> Add support for the ad7091r 12 bit ADC to the ad7476 driver. Although the
>> ad7091r is not really related to any of the other devices supported by this
>> driver, luckily for us there are not so many ways (which are not totally insane)
>> how sampling a single channel ADC via SPI can be implemented and support for the
>> ad7091r can be added to the driver with just a few adjustments.
> Yup, eveything is nice and simple when there is only one channel ;)
>>
>> The ad7091r requires an external "conversion start" pulse to start a sample
>> conversion. After the conversion has finished the result can be read via SPI. We
>> depend on a IIO trigger to generate this signal, as a result only sampling in
>> buffered mode and not in manual mode is available.
> This is a little restrictive given we could wire up some magic with a gpio to
> do this, but I guess things like that can be added as and when people care.
> We may need some way of 'knowing' whether a more general trigger has done
> the start pulse or not... Something for another day.

Yes, I think this will eventually require a new class of devices, maybe a
trigger subclass. We can also generate a single pulse using the bfin timer.
But there is no interface to communicate this to trigger from the driver.
There are also some other ADC driver in staging/iio/adc which have a similar
convst signal and they indeed use a GPIO to generate the signal. But the
problem is that using a GPIO and a timer to generate the trigger pulse leads
on one hand to a lot less uniform and inaccurate waveform and thus decreases
the quality of the sampled data. On the other hand it increases the system
load. So I'm probably going to switch them over to something similar as in
this driver before moving them out of staging.

My short term goal is to retire the bfintmr trigger and replace it with a
more generic trigger which uses the PWM framework. But this requires some
infrastructure work on the PWM framework itself first.

The long term goal should be to extend the IIO trigger framework to better
integrate these kind of devices. In my opinion we can at least differentiate
between three different classes of triggers:

* Hardware triggers: Hardware triggers usually are hardwired on the board
  and can't be switched at runtime.
* Software triggers: Software triggers are as the name suggest triggered by
  software events and cause the sample to be read, e.g. by issuing a SPI
  read. Software triggers can be re-assigned at runtime.
* Built-in triggers: These are built into the sampling device and thus can
  be sort of seen as hardware triggers, but only the itself is able to use
  the trigger.

And not all devices support all kinds of triggers. Some may be able to
support multiple kinds, but need to change the device configuration to
switch between them. So the device needs to know which kind of trigger it is
currently used with.

> merged to togreg branch of iio.git

Thanks.

- Lars

Re: [PATCH 1/4] staging:iio:trigger:bfintmr: Avoid divide by zero

[Jonathan Cameron]

On 09/17/2012 01:26 PM, Lars-Peter Clausen wrote:
> If the timer frequency has not been configured yet get_gptimer_period() will
> return 0. Handle this case instead of blindly dividing by the returned value.
Sounds like the sort of thing that only happens when setting board code up
so I'll treat it as an improvement in the code rather than an urgent fix.

Merged to togreg branch.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
>  drivers/staging/iio/trigger/iio-trig-bfin-timer.c |   10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/iio/trigger/iio-trig-bfin-timer.c b/drivers/staging/iio/trigger/iio-trig-bfin-timer.c
> index ce6a7b1..2772ea2 100644
> --- a/drivers/staging/iio/trigger/iio-trig-bfin-timer.c
> +++ b/drivers/staging/iio/trigger/iio-trig-bfin-timer.c
> @@ -99,9 +99,15 @@ static ssize_t iio_bfin_tmr_frequency_show(struct device *dev,
>  {
>  	struct iio_trigger *trig = to_iio_trigger(dev);
>  	struct bfin_tmr_state *st = trig->private_data;
> +	unsigned int period = get_gptimer_period(st->t->id);
> +	unsigned long val;
>  
> -	return sprintf(buf, "%lu\n",
> -			get_sclk() / get_gptimer_period(st->t->id));
> +	if (period == 0)
> +		val = 0;
> +	else
> +		val = get_sclk() / get_gptimer_period(st->t->id);
> +
> +	return sprintf(buf, "%lu\n", val);
>  }
>  
>  static DEVICE_ATTR(frequency, S_IRUGO | S_IWUSR, iio_bfin_tmr_frequency_show,
>