iio:magnetometer:ak8975: Add support for DT and interrupt handling

iio:magnetometer:ak8975: Add support for DT and interrupt handling

[Jacek Anaszewski]

Hello,

These patches add 'data ready' interrupt handling for AK8975
magnetometer, so as to provide more efficient alternative of
waiting for the end of conversion. This short series adds also
support for parsing 'gpios' property when initializing from oftree.

Thanks,
Jacek Anaszewski

[PATCH 1/2] iio: ak8975: Add support for gpios DT property

[Jacek Anaszewski]

Add support for parsing 'gpios' property when initializing
from oftree.
This patch adds also the binding documentation file.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Andrew Chew <achew@nvidia.com>
---
 .../bindings/staging/iio/magnetometer/ak8975.txt   |   19 +++++++++++++++++++
 drivers/iio/magnetometer/ak8975.c                  |    5 ++++-
 2 files changed, 23 insertions(+), 1 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/staging/iio/magnetometer/ak8975.txt

diff --git a/Documentation/devicetree/bindings/staging/iio/magnetometer/ak8975.txt b/Documentation/devicetree/bindings/staging/iio/magnetometer/ak8975.txt
new file mode 100644
index 0000000..d085d90
--- /dev/null
+++ b/Documentation/devicetree/bindings/staging/iio/magnetometer/ak8975.txt
@@ -0,0 +1,19 @@
+* AsahiKASEI AK8975 device driver
+
+Required properties:
+
+  - compatible : should be "asahi-kasei,ak8975"
+  - reg : the I2C address of the magnetometer
+
+Optional properties:
+
+  - gpios : should be device tree identifier of magnetometer DRDY pin
+
+Example:
+
+ak8975@0C {
+        compatible = "asahi-kasei,ak8975";
+        reg = <0x0C>;
+        gpios = <&gpj0 7 0>;
+};
+
diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index af6c320..eb17eca 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -29,6 +29,7 @@
 #include <linux/delay.h>
 
 #include <linux/gpio.h>
+#include <linux/of_gpio.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
@@ -384,7 +385,9 @@ static int ak8975_probe(struct i2c_client *client,
 	int err;
 
 	/* Grab and set up the supplied GPIO. */
-	if (client->dev.platform_data == NULL)
+	if (client->dev.of_node)
+		eoc_gpio = of_get_gpio(client->dev.of_node, 0);
+	else if (client->dev.platform_data == NULL)
 		eoc_gpio = -1;
 	else
 		eoc_gpio = *(int *)(client->dev.platform_data);
-- 
1.7.5.4

Re: [PATCH 1/2] iio: ak8975: Add support for gpios DT property

[Jonathan Cameron]

On 04/16/2013 03:37 PM, Jacek Anaszewski wrote:
> Add support for parsing 'gpios' property when initializing
> from oftree.
> This patch adds also the binding documentation file.
> 
> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Andrew Chew <achew@nvidia.com>
One issue here.

The driver has moved out of staging, yet your are documenting it's bindings
as if it hasn't.  Please just move the docs and resend the series.

Also, there is a bonus blank line at the end of the bindings file. Please remove.

Jonathan
> ---
>  .../bindings/staging/iio/magnetometer/ak8975.txt   |   19 +++++++++++++++++++
>  drivers/iio/magnetometer/ak8975.c                  |    5 ++++-
>  2 files changed, 23 insertions(+), 1 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/staging/iio/magnetometer/ak8975.txt
> 
> diff --git a/Documentation/devicetree/bindings/staging/iio/magnetometer/ak8975.txt b/Documentation/devicetree/bindings/staging/iio/magnetometer/ak8975.txt
> new file mode 100644
> index 0000000..d085d90
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/staging/iio/magnetometer/ak8975.txt
> @@ -0,0 +1,19 @@
> +* AsahiKASEI AK8975 device driver
> +
> +Required properties:
> +
> +  - compatible : should be "asahi-kasei,ak8975"
> +  - reg : the I2C address of the magnetometer
> +
> +Optional properties:
> +
> +  - gpios : should be device tree identifier of magnetometer DRDY pin
> +
> +Example:
> +
> +ak8975@0C {
> +        compatible = "asahi-kasei,ak8975";
> +        reg = <0x0C>;
> +        gpios = <&gpj0 7 0>;
> +};
> +
> diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
> index af6c320..eb17eca 100644
> --- a/drivers/iio/magnetometer/ak8975.c
> +++ b/drivers/iio/magnetometer/ak8975.c
> @@ -29,6 +29,7 @@
>  #include <linux/delay.h>
>  
>  #include <linux/gpio.h>
> +#include <linux/of_gpio.h>
>  
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
> @@ -384,7 +385,9 @@ static int ak8975_probe(struct i2c_client *client,
>  	int err;
>  
>  	/* Grab and set up the supplied GPIO. */
> -	if (client->dev.platform_data == NULL)
> +	if (client->dev.of_node)
> +		eoc_gpio = of_get_gpio(client->dev.of_node, 0);
> +	else if (client->dev.platform_data == NULL)
>  		eoc_gpio = -1;
>  	else
>  		eoc_gpio = *(int *)(client->dev.platform_data);
> 

Re: [PATCH 1/2] iio: ak8975: Add support for gpios DT property

[Lars-Peter Clausen]

On 05/06/2013 06:44 PM, Jonathan Cameron wrote:
> On 04/16/2013 03:37 PM, Jacek Anaszewski wrote:
[...]
>> diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
>> index af6c320..eb17eca 100644
>> --- a/drivers/iio/magnetometer/ak8975.c
>> +++ b/drivers/iio/magnetometer/ak8975.c
>> @@ -29,6 +29,7 @@
>>  #include <linux/delay.h>
>>  
>>  #include <linux/gpio.h>
>> +#include <linux/of_gpio.h>
>>  
>>  #include <linux/iio/iio.h>
>>  #include <linux/iio/sysfs.h>
>> @@ -384,7 +385,9 @@ static int ak8975_probe(struct i2c_client *client,
>>  	int err;
>>  
>>  	/* Grab and set up the supplied GPIO. */
>> -	if (client->dev.platform_data == NULL)
>> +	if (client->dev.of_node)
>> +		eoc_gpio = of_get_gpio(client->dev.of_node, 0);
>> +	else if (client->dev.platform_data == NULL)
>>  		eoc_gpio = -1;
>>  	else
>>  		eoc_gpio = *(int *)(client->dev.platform_data);

Usually this is done in the opposite order. First check for platform_data,
then, of_node. Also it probably makes sense to check for -EPROBE_DEFER in the
of case.

- Lars

[PATCH 2/2] iio: ak8975: Implement data ready interrupt handling

[Jacek Anaszewski]

Implement "data ready" interrupt handling in addition to the
two existing read modes - DRDY GPIO polling and ST1 register
DRDY bit polling.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Andrew Chew <achew@nvidia.com>
---
 drivers/iio/magnetometer/ak8975.c |   91 ++++++++++++++++++++++++++++++++++++-
 1 files changed, 89 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index eb17eca..d379c4c 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -24,10 +24,11 @@
 #include <linux/kernel.h>
 #include <linux/slab.h>
 #include <linux/i2c.h>
+#include <linux/interrupt.h>
 #include <linux/err.h>
 #include <linux/mutex.h>
 #include <linux/delay.h>
-
+#include <linux/bitops.h>
 #include <linux/gpio.h>
 #include <linux/of_gpio.h>
 
@@ -83,6 +84,7 @@
  */
 #define AK8975_MAX_CONVERSION_TIMEOUT	500
 #define AK8975_CONVERSION_DONE_POLL_TIME 10
+#define AK8975_DATA_READY_TIMEOUT	((100*HZ)/1000)
 
 /*
  * Per-instance context data for the device.
@@ -95,6 +97,9 @@ struct ak8975_data {
 	long			raw_to_gauss[3];
 	u8			reg_cache[AK8975_MAX_REGS];
 	int			eoc_gpio;
+	int			eoc_irq;
+	wait_queue_head_t	data_ready_queue;
+	unsigned long		flags;
 };
 
 static const int ak8975_index_to_reg[] = {
@@ -124,6 +129,51 @@ static int ak8975_write_data(struct i2c_client *client,
 }
 
 /*
+ * Handle data ready irq
+ */
+static irqreturn_t ak8975_irq_handler(int irq, void *data)
+{
+	struct ak8975_data *ak8975 = data;
+
+	set_bit(0, &ak8975->flags);
+	wake_up(&ak8975->data_ready_queue);
+
+	return IRQ_HANDLED;
+}
+
+/*
+ * Install data ready interrupt handler
+ */
+static int ak8975_setup_irq(struct ak8975_data *data)
+{
+	struct i2c_client *client = data->client;
+	int rc;
+	int irq;
+
+	if (client->irq)
+		irq = client->irq;
+	else
+		irq = gpio_to_irq(data->eoc_gpio);
+
+	rc = request_irq(irq, ak8975_irq_handler,
+			 IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+			 dev_name(&client->dev), data);
+	if (rc < 0) {
+		dev_err(&client->dev,
+			"irq %d request failed, (gpio %d): %d\n",
+			irq, data->eoc_gpio, rc);
+		return rc;
+	}
+
+	init_waitqueue_head(&data->data_ready_queue);
+	clear_bit(0, &data->flags);
+	data->eoc_irq = irq;
+
+	return rc;
+}
+
+
+/*
  * Perform some start-of-day setup, including reading the asa calibration
  * values and caching them.
  */
@@ -171,6 +221,16 @@ static int ak8975_setup(struct i2c_client *client)
 				AK8975_REG_CNTL_MODE_POWER_DOWN,
 				AK8975_REG_CNTL_MODE_MASK,
 				AK8975_REG_CNTL_MODE_SHIFT);
+
+	if (data->eoc_gpio > 0 || client->irq) {
+		ret = ak8975_setup_irq(data);
+		if (ret < 0) {
+			dev_err(&client->dev,
+				"Error setting data ready interrupt\n");
+			return ret;
+		}
+	}
+
 	if (ret < 0) {
 		dev_err(&client->dev, "Error in setting power-down mode\n");
 		return ret;
@@ -267,9 +327,23 @@ static int wait_conversion_complete_polled(struct ak8975_data *data)
 		dev_err(&client->dev, "Conversion timeout happened\n");
 		return -EINVAL;
 	}
+
 	return read_status;
 }
 
+/* Returns 0 if the end of conversion interrupt occured or -ETIME otherwise */
+static int wait_conversion_complete_interrupt(struct ak8975_data *data)
+{
+	int ret;
+
+	ret = wait_event_timeout(data->data_ready_queue,
+				 test_bit(0, &data->flags),
+				 AK8975_DATA_READY_TIMEOUT);
+	clear_bit(0, &data->flags);
+
+	return ret > 0 ? 0 : -ETIME;
+}
+
 /*
  * Emits the raw flux value for the x, y, or z axis.
  */
@@ -295,13 +369,16 @@ static int ak8975_read_axis(struct iio_dev *indio_dev, int index, int *val)
 	}
 
 	/* Wait for the conversion to complete. */
-	if (gpio_is_valid(data->eoc_gpio))
+	if (data->eoc_irq)
+		ret = wait_conversion_complete_interrupt(data);
+	else if (gpio_is_valid(data->eoc_gpio))
 		ret = wait_conversion_complete_gpio(data);
 	else
 		ret = wait_conversion_complete_polled(data);
 	if (ret < 0)
 		goto exit;
 
+	/* This will be executed only for non-interrupt based waiting case */
 	if (ret & AK8975_REG_ST1_DRDY_MASK) {
 		ret = i2c_smbus_read_byte_data(client, AK8975_REG_ST2);
 		if (ret < 0) {
@@ -412,6 +489,11 @@ static int ak8975_probe(struct i2c_client *client,
 	}
 	data = iio_priv(indio_dev);
 	i2c_set_clientdata(client, indio_dev);
+
+	data->client = client;
+	data->eoc_gpio = eoc_gpio;
+	data->eoc_irq = 0;
+
 	/* Perform some basic start-of-day setup of the device. */
 	err = ak8975_setup(client);
 	if (err < 0) {
@@ -436,6 +518,8 @@ static int ak8975_probe(struct i2c_client *client,
 
 exit_free_iio:
 	iio_device_free(indio_dev);
+	if (data->eoc_irq)
+		free_irq(data->eoc_irq, NULL);
 exit_gpio:
 	if (gpio_is_valid(eoc_gpio))
 		gpio_free(eoc_gpio);
@@ -450,6 +534,9 @@ static int ak8975_remove(struct i2c_client *client)
 
 	iio_device_unregister(indio_dev);
 
+	if (data->eoc_irq)
+		free_irq(data->eoc_irq, NULL);
+
 	if (gpio_is_valid(data->eoc_gpio))
 		gpio_free(data->eoc_gpio);
 
-- 
1.7.5.4

Re: [PATCH 2/2] iio: ak8975: Implement data ready interrupt handling

[Jonathan Cameron]

On 04/16/2013 03:37 PM, Jacek Anaszewski wrote:
> Implement "data ready" interrupt handling in addition to the
> two existing read modes - DRDY GPIO polling and ST1 register
> DRDY bit polling.
> 
> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Andrew Chew <achew@nvidia.com>

Allowing either the gpio or the interrupt is a little ususual, but
I suppose does avoid having to change existing users to gain interrupt
support (if their gpio subsystem happens to support interrupts).

As there is no rush for this cycle, I'll take this once you have made
 the requested changes to the other patch as long as no one else
has raised any issues.

Jonathan
> ---
>  drivers/iio/magnetometer/ak8975.c |   91 ++++++++++++++++++++++++++++++++++++-
>  1 files changed, 89 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
> index eb17eca..d379c4c 100644
> --- a/drivers/iio/magnetometer/ak8975.c
> +++ b/drivers/iio/magnetometer/ak8975.c
> @@ -24,10 +24,11 @@
>  #include <linux/kernel.h>
>  #include <linux/slab.h>
>  #include <linux/i2c.h>
> +#include <linux/interrupt.h>
>  #include <linux/err.h>
>  #include <linux/mutex.h>
>  #include <linux/delay.h>
> -
> +#include <linux/bitops.h>
>  #include <linux/gpio.h>
>  #include <linux/of_gpio.h>
>  
> @@ -83,6 +84,7 @@
>   */
>  #define AK8975_MAX_CONVERSION_TIMEOUT	500
>  #define AK8975_CONVERSION_DONE_POLL_TIME 10
> +#define AK8975_DATA_READY_TIMEOUT	((100*HZ)/1000)
>  
>  /*
>   * Per-instance context data for the device.
> @@ -95,6 +97,9 @@ struct ak8975_data {
>  	long			raw_to_gauss[3];
>  	u8			reg_cache[AK8975_MAX_REGS];
>  	int			eoc_gpio;
> +	int			eoc_irq;
> +	wait_queue_head_t	data_ready_queue;
> +	unsigned long		flags;
>  };
>  
>  static const int ak8975_index_to_reg[] = {
> @@ -124,6 +129,51 @@ static int ak8975_write_data(struct i2c_client *client,
>  }
>  
>  /*
> + * Handle data ready irq
> + */
> +static irqreturn_t ak8975_irq_handler(int irq, void *data)
> +{
> +	struct ak8975_data *ak8975 = data;
> +
> +	set_bit(0, &ak8975->flags);
> +	wake_up(&ak8975->data_ready_queue);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +/*
> + * Install data ready interrupt handler
> + */
> +static int ak8975_setup_irq(struct ak8975_data *data)
> +{
> +	struct i2c_client *client = data->client;
> +	int rc;
> +	int irq;
> +
> +	if (client->irq)
> +		irq = client->irq;
> +	else
> +		irq = gpio_to_irq(data->eoc_gpio);
> +
> +	rc = request_irq(irq, ak8975_irq_handler,
> +			 IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> +			 dev_name(&client->dev), data);
> +	if (rc < 0) {
> +		dev_err(&client->dev,
> +			"irq %d request failed, (gpio %d): %d\n",
> +			irq, data->eoc_gpio, rc);
> +		return rc;
> +	}
> +
> +	init_waitqueue_head(&data->data_ready_queue);
> +	clear_bit(0, &data->flags);
> +	data->eoc_irq = irq;
> +
> +	return rc;
> +}
> +
> +
> +/*
>   * Perform some start-of-day setup, including reading the asa calibration
>   * values and caching them.
>   */
> @@ -171,6 +221,16 @@ static int ak8975_setup(struct i2c_client *client)
>  				AK8975_REG_CNTL_MODE_POWER_DOWN,
>  				AK8975_REG_CNTL_MODE_MASK,
>  				AK8975_REG_CNTL_MODE_SHIFT);
> +
> +	if (data->eoc_gpio > 0 || client->irq) {
> +		ret = ak8975_setup_irq(data);
> +		if (ret < 0) {
> +			dev_err(&client->dev,
> +				"Error setting data ready interrupt\n");
> +			return ret;
> +		}
> +	}
> +
>  	if (ret < 0) {
>  		dev_err(&client->dev, "Error in setting power-down mode\n");
>  		return ret;
> @@ -267,9 +327,23 @@ static int wait_conversion_complete_polled(struct ak8975_data *data)
>  		dev_err(&client->dev, "Conversion timeout happened\n");
>  		return -EINVAL;
>  	}
> +
>  	return read_status;
>  }
>  
> +/* Returns 0 if the end of conversion interrupt occured or -ETIME otherwise */
> +static int wait_conversion_complete_interrupt(struct ak8975_data *data)
> +{
> +	int ret;
> +
> +	ret = wait_event_timeout(data->data_ready_queue,
> +				 test_bit(0, &data->flags),
> +				 AK8975_DATA_READY_TIMEOUT);
> +	clear_bit(0, &data->flags);
> +
> +	return ret > 0 ? 0 : -ETIME;
> +}
> +
>  /*
>   * Emits the raw flux value for the x, y, or z axis.
>   */
> @@ -295,13 +369,16 @@ static int ak8975_read_axis(struct iio_dev *indio_dev, int index, int *val)
>  	}
>  
>  	/* Wait for the conversion to complete. */
> -	if (gpio_is_valid(data->eoc_gpio))
> +	if (data->eoc_irq)
> +		ret = wait_conversion_complete_interrupt(data);
> +	else if (gpio_is_valid(data->eoc_gpio))
>  		ret = wait_conversion_complete_gpio(data);
>  	else
>  		ret = wait_conversion_complete_polled(data);
>  	if (ret < 0)
>  		goto exit;
>  
> +	/* This will be executed only for non-interrupt based waiting case */
>  	if (ret & AK8975_REG_ST1_DRDY_MASK) {
>  		ret = i2c_smbus_read_byte_data(client, AK8975_REG_ST2);
>  		if (ret < 0) {
> @@ -412,6 +489,11 @@ static int ak8975_probe(struct i2c_client *client,
>  	}
>  	data = iio_priv(indio_dev);
>  	i2c_set_clientdata(client, indio_dev);
> +
> +	data->client = client;
> +	data->eoc_gpio = eoc_gpio;
> +	data->eoc_irq = 0;
> +
>  	/* Perform some basic start-of-day setup of the device. */
>  	err = ak8975_setup(client);
>  	if (err < 0) {
> @@ -436,6 +518,8 @@ static int ak8975_probe(struct i2c_client *client,
>  
>  exit_free_iio:
>  	iio_device_free(indio_dev);
> +	if (data->eoc_irq)
> +		free_irq(data->eoc_irq, NULL);
>  exit_gpio:
>  	if (gpio_is_valid(eoc_gpio))
>  		gpio_free(eoc_gpio);
> @@ -450,6 +534,9 @@ static int ak8975_remove(struct i2c_client *client)
>  
>  	iio_device_unregister(indio_dev);
>  
> +	if (data->eoc_irq)
> +		free_irq(data->eoc_irq, NULL);
> +
>  	if (gpio_is_valid(data->eoc_gpio))
>  		gpio_free(data->eoc_gpio);
>  
> 

Re: [PATCH 2/2] iio: ak8975: Implement data ready interrupt handling

[Jacek Anaszewski]

On 05/06/2013 06:49 PM, Jonathan Cameron wrote:
> On 04/16/2013 03:37 PM, Jacek Anaszewski wrote:
>> Implement "data ready" interrupt handling in addition to the
>> two existing read modes - DRDY GPIO polling and ST1 register
>> DRDY bit polling.
>>
>> Signed-off-by: Jacek Anaszewski<j.anaszewski@samsung.com>
>> Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com>
>> Cc: Andrew Chew<achew@nvidia.com>
>
> Allowing either the gpio or the interrupt is a little ususual, but
> I suppose does avoid having to change existing users to gain interrupt
> support (if their gpio subsystem happens to support interrupts).

Thanks for the review. I am going to send new series promptly.

Regarding the gpio issue - I can imagine the situation when there is no 
available interrupt pin at the CPU, as they were all connected to the 
other devices, there are some free general purpose pins though. It is 
always better to have a possibility to periodically read the state of
such a pin, instead of performing cycling I2C transmission to check the 
status of DRDY bit in the ST1 register, which is a fallback in the driver.

Thanks,
Jacek Anaszewski