[PATCH 1/3] iio: ak8975 : Add AK8963 compatibility mode support

[PATCH 1/3] iio: ak8975 : Add AK8963 compatibility mode support

[Srinivas Pandruvada]

AK8963 and AK8975 use same register definitions, except the range
of X,Y,Z values. Added support of 8963 based on i2c_device_id.
Unfortunately there is no way to detect the type via registers,
both device registers return 0x48 as id of chipset.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/iio/magnetometer/Kconfig  |  3 ++-
 drivers/iio/magnetometer/ak8975.c | 40 +++++++++++++++++++++++++++++++++++----
 2 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/magnetometer/Kconfig b/drivers/iio/magnetometer/Kconfig
index d86d226..05a364c54 100644
--- a/drivers/iio/magnetometer/Kconfig
+++ b/drivers/iio/magnetometer/Kconfig
@@ -11,7 +11,8 @@ config AK8975
 	depends on GPIOLIB
 	help
 	  Say yes here to build support for Asahi Kasei AK8975 3-Axis
-	  Magnetometer.
+	  Magnetometer. This driver can also support AK8963, if i2c
+	  device name is identified as ak8963.
 
 	  To compile this driver as a module, choose M here: the module
 	  will be called ak8975.
diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index 0542354..a55c94f 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -85,7 +85,15 @@
 #define AK8975_MAX_CONVERSION_TIMEOUT	500
 #define AK8975_CONVERSION_DONE_POLL_TIME 10
 #define AK8975_DATA_READY_TIMEOUT	((100*HZ)/1000)
-#define RAW_TO_GAUSS(asa) ((((asa) + 128) * 3000) / 256)
+#define RAW_TO_GAUSS_8975(asa) ((((asa) + 128) * 3000) / 256)
+#define RAW_TO_GAUSS_8963(asa) ((((asa) + 128) * 6000) / 256)
+
+/* Compatible Asahi Kasei Compass parts */
+enum asahi_compass_chipset {
+	AK8975,
+	AK8963,
+	AK_INVALID,
+};
 
 /*
  * Per-instance context data for the device.
@@ -101,6 +109,7 @@ struct ak8975_data {
 	int			eoc_irq;
 	wait_queue_head_t	data_ready_queue;
 	unsigned long		flags;
+	int			chipset;
 };
 
 static const int ak8975_index_to_reg[] = {
@@ -272,9 +281,21 @@ static int ak8975_setup(struct i2c_client *client)
  * Since ASA doesn't change, we cache the resultant scale factor into the
  * device context in ak8975_setup().
  */
-	data->raw_to_gauss[0] = RAW_TO_GAUSS(data->asa[0]);
-	data->raw_to_gauss[1] = RAW_TO_GAUSS(data->asa[1]);
-	data->raw_to_gauss[2] = RAW_TO_GAUSS(data->asa[2]);
+	if (data->chipset == AK8963) {
+		/*
+		 * H range is +-8190 and magnetometer range is +-4912.
+		 * So HuT using the above explanation for 8975,
+		 * 4912/8190 = ~ 6/10.
+		 * So the Hadj should use 6/10 instead of 3/10.
+		 */
+		data->raw_to_gauss[0] = RAW_TO_GAUSS_8963(data->asa[0]);
+		data->raw_to_gauss[1] = RAW_TO_GAUSS_8963(data->asa[1]);
+		data->raw_to_gauss[2] = RAW_TO_GAUSS_8963(data->asa[2]);
+	} else {
+		data->raw_to_gauss[0] = RAW_TO_GAUSS_8975(data->asa[0]);
+		data->raw_to_gauss[1] = RAW_TO_GAUSS_8975(data->asa[1]);
+		data->raw_to_gauss[2] = RAW_TO_GAUSS_8975(data->asa[2]);
+	}
 
 	return 0;
 }
@@ -499,6 +520,16 @@ static int ak8975_probe(struct i2c_client *client,
 	data->eoc_gpio = eoc_gpio;
 	data->eoc_irq = 0;
 
+	if (id) {
+		data->chipset = (int)(id->driver_data);
+		if (data->chipset == AK8963)
+			dev_dbg(&client->dev,
+				"AK8975 driver is running in AK8963 mode\n");
+		else
+			dev_dbg(&client->dev,
+				"AK8975 driver is running in normal mode\n");
+	}
+
 	/* Perform some basic start-of-day setup of the device. */
 	err = ak8975_setup(client);
 	if (err < 0) {
@@ -552,6 +583,7 @@ static int ak8975_remove(struct i2c_client *client)
 
 static const struct i2c_device_id ak8975_id[] = {
 	{"ak8975", 0},
+	{"ak8963", AK8963},
 	{}
 };
 
-- 
1.7.11.7

[PATCH 2/3] iio: ak8975: Added ACPI enumeration

[Srinivas Pandruvada]

Added capability so that this device can be enumerated via ACPI.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/iio/magnetometer/ak8975.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index a55c94f..fe5e9c8 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -31,6 +31,7 @@
 #include <linux/bitops.h>
 #include <linux/gpio.h>
 #include <linux/of_gpio.h>
+#include <linux/acpi.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
@@ -476,6 +477,27 @@ static const struct iio_info ak8975_info = {
 	.driver_module = THIS_MODULE,
 };
 
+static const struct acpi_device_id ak_acpi_match[] = {
+	{"AK8975", 0},
+	{"AK8963", AK8963},
+	{"INVN6500", AK8963},
+	{ },
+};
+MODULE_DEVICE_TABLE(acpi, ak_acpi_match);
+
+static char *ak8975_match_acpi_device(struct device *dev,
+							int *chipset)
+{
+	const struct acpi_device_id *id;
+
+	id = acpi_match_device(dev->driver->acpi_match_table, dev);
+	if (!id)
+		return NULL;
+	*chipset = (int)id->driver_data;
+
+	return (char *)dev_name(dev);
+}
+
 static int ak8975_probe(struct i2c_client *client,
 			const struct i2c_device_id *id)
 {
@@ -483,7 +505,10 @@ static int ak8975_probe(struct i2c_client *client,
 	struct iio_dev *indio_dev;
 	int eoc_gpio;
 	int err;
+	char *name = NULL;
 
+	if (id)
+		name = (char *) id->name;
 	/* Grab and set up the supplied GPIO. */
 	if (client->dev.platform_data)
 		eoc_gpio = *(int *)(client->dev.platform_data);
@@ -529,6 +554,13 @@ static int ak8975_probe(struct i2c_client *client,
 			dev_dbg(&client->dev,
 				"AK8975 driver is running in normal mode\n");
 	}
+	if (ACPI_HANDLE(&client->dev)) {
+		int chip_id;
+
+		name = ak8975_match_acpi_device(&client->dev, &chip_id);
+		if (chip_id > 0)
+			data->chipset = chip_id;
+	}
 
 	/* Perform some basic start-of-day setup of the device. */
 	err = ak8975_setup(client);
@@ -545,7 +577,7 @@ static int ak8975_probe(struct i2c_client *client,
 	indio_dev->num_channels = ARRAY_SIZE(ak8975_channels);
 	indio_dev->info = &ak8975_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
-
+	indio_dev->name = name;
 	err = iio_device_register(indio_dev);
 	if (err < 0)
 		goto exit_free_iio;
@@ -600,6 +632,7 @@ static struct i2c_driver ak8975_driver = {
 	.driver = {
 		.name	= "ak8975",
 		.of_match_table = ak8975_of_match,
+		.acpi_match_table = ACPI_PTR(ak_acpi_match),
 	},
 	.probe		= ak8975_probe,
 	.remove		= ak8975_remove,
-- 
1.7.11.7

[PATCH 3/3] iio: ak8975: Don't bail out for irq setup error

[Srinivas Pandruvada]

This driver can very well work via polling, if request_irq failed.
The problem is that many times i2c interrupts are shared. This
driver is not ready for shared interrupt as it will return IRQ_HANDLED,
in every case. Here we will get EBUSY when some other driver registered
handler and this driver is grabbing in exclusive mode.
So in this case just print error and continue in polling mode.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/iio/magnetometer/ak8975.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index fe5e9c8..0f9c407 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -236,9 +236,15 @@ static int ak8975_setup(struct i2c_client *client)
 	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 == -EBUSY) {
+				dev_err(&client->dev,
+					"device Intr busy:polling required\n");
+				ret = 0;
+			} else {
+				dev_err(&client->dev,
+					"Error setting data ready interrupt\n");
+				return ret;
+			}
 		}
 	}
 
-- 
1.7.11.7

Re: [PATCH 1/3] iio: ak8975 : Add AK8963 compatibility mode support

[Lars-Peter Clausen]

On 03/11/2014 10:15 PM, Srinivas Pandruvada wrote:
> AK8963 and AK8975 use same register definitions, except the range
> of X,Y,Z values. Added support of 8963 based on i2c_device_id.
> Unfortunately there is no way to detect the type via registers,
> both device registers return 0x48 as id of chipset.
>
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

Hi,

a few small comments.

> ---
>   drivers/iio/magnetometer/Kconfig  |  3 ++-
>   drivers/iio/magnetometer/ak8975.c | 40 +++++++++++++++++++++++++++++++++++----
>   2 files changed, 38 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iio/magnetometer/Kconfig b/drivers/iio/magnetometer/Kconfig
> index d86d226..05a364c54 100644
> --- a/drivers/iio/magnetometer/Kconfig
> +++ b/drivers/iio/magnetometer/Kconfig
> @@ -11,7 +11,8 @@ config AK8975
>   	depends on GPIOLIB
>   	help
>   	  Say yes here to build support for Asahi Kasei AK8975 3-Axis
> -	  Magnetometer.
> +	  Magnetometer. This driver can also support AK8963, if i2c
> +	  device name is identified as ak8963.
>
>   	  To compile this driver as a module, choose M here: the module
>   	  will be called ak8975.
> diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
> index 0542354..a55c94f 100644
> --- a/drivers/iio/magnetometer/ak8975.c
> +++ b/drivers/iio/magnetometer/ak8975.c
> @@ -85,7 +85,15 @@
>   #define AK8975_MAX_CONVERSION_TIMEOUT	500
>   #define AK8975_CONVERSION_DONE_POLL_TIME 10
>   #define AK8975_DATA_READY_TIMEOUT	((100*HZ)/1000)
> -#define RAW_TO_GAUSS(asa) ((((asa) + 128) * 3000) / 256)
> +#define RAW_TO_GAUSS_8975(asa) ((((asa) + 128) * 3000) / 256)
> +#define RAW_TO_GAUSS_8963(asa) ((((asa) + 128) * 6000) / 256)
> +
> +/* Compatible Asahi Kasei Compass parts */
> +enum asahi_compass_chipset {
> +	AK8975,
> +	AK8963,
> +	AK_INVALID,

Not sure if you need AK_INVALID, it is never used.

> +};
>
>   /*
>    * Per-instance context data for the device.
> @@ -101,6 +109,7 @@ struct ak8975_data {
>   	int			eoc_irq;
>   	wait_queue_head_t	data_ready_queue;
>   	unsigned long		flags;
> +	int			chipset;

enum asahi_compass_chipset ...

>   };
>
>   static const int ak8975_index_to_reg[] = {
> @@ -272,9 +281,21 @@ static int ak8975_setup(struct i2c_client *client)
>    * Since ASA doesn't change, we cache the resultant scale factor into the
>    * device context in ak8975_setup().
>    */
> -	data->raw_to_gauss[0] = RAW_TO_GAUSS(data->asa[0]);
> -	data->raw_to_gauss[1] = RAW_TO_GAUSS(data->asa[1]);
> -	data->raw_to_gauss[2] = RAW_TO_GAUSS(data->asa[2]);
> +	if (data->chipset == AK8963) {
> +		/*
> +		 * H range is +-8190 and magnetometer range is +-4912.
> +		 * So HuT using the above explanation for 8975,
> +		 * 4912/8190 = ~ 6/10.
> +		 * So the Hadj should use 6/10 instead of 3/10.
> +		 */
> +		data->raw_to_gauss[0] = RAW_TO_GAUSS_8963(data->asa[0]);
> +		data->raw_to_gauss[1] = RAW_TO_GAUSS_8963(data->asa[1]);
> +		data->raw_to_gauss[2] = RAW_TO_GAUSS_8963(data->asa[2]);
> +	} else {
> +		data->raw_to_gauss[0] = RAW_TO_GAUSS_8975(data->asa[0]);
> +		data->raw_to_gauss[1] = RAW_TO_GAUSS_8975(data->asa[1]);
> +		data->raw_to_gauss[2] = RAW_TO_GAUSS_8975(data->asa[2]);
> +	}
>
>   	return 0;
>   }
> @@ -499,6 +520,16 @@ static int ak8975_probe(struct i2c_client *client,
>   	data->eoc_gpio = eoc_gpio;
>   	data->eoc_irq = 0;
>
> +	if (id) {

id will never be NULL.

> +		data->chipset = (int)(id->driver_data);
> +		if (data->chipset == AK8963)
> +			dev_dbg(&client->dev,
> +				"AK8975 driver is running in AK8963 mode\n");
> +		else
> +			dev_dbg(&client->dev,
> +				"AK8975 driver is running in normal mode\n");

How about printing just id->name, this way you do not have to add a new else 
branch when a new part is supported.

> +	}
> +
>   	/* Perform some basic start-of-day setup of the device. */
>   	err = ak8975_setup(client);
>   	if (err < 0) {
> @@ -552,6 +583,7 @@ static int ak8975_remove(struct i2c_client *client)
>
>   static const struct i2c_device_id ak8975_id[] = {
>   	{"ak8975", 0},

Should change the 0 to AK8975

> +	{"ak8963", AK8963},
>   	{}
>   };
>
>

Re: [PATCH 1/3] iio: ak8975 : Add AK8963 compatibility mode support

[Srinivas Pandruvada]

On 03/12/2014 03:07 AM, Lars-Peter Clausen wrote:
> On 03/11/2014 10:15 PM, Srinivas Pandruvada wrote:
>> AK8963 and AK8975 use same register definitions, except the range
>> of X,Y,Z values. Added support of 8963 based on i2c_device_id.
>> Unfortunately there is no way to detect the type via registers,
>> both device registers return 0x48 as id of chipset.
>>
>> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
>
> Hi,
>
> a few small comments.
>

Thanks. I will take care in the next version.

-Srinivas

>> ---
>>   drivers/iio/magnetometer/Kconfig  |  3 ++-
>>   drivers/iio/magnetometer/ak8975.c | 40 
>> +++++++++++++++++++++++++++++++++++----
>>   2 files changed, 38 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/iio/magnetometer/Kconfig 
>> b/drivers/iio/magnetometer/Kconfig
>> index d86d226..05a364c54 100644
>> --- a/drivers/iio/magnetometer/Kconfig
>> +++ b/drivers/iio/magnetometer/Kconfig
>> @@ -11,7 +11,8 @@ config AK8975
>>       depends on GPIOLIB
>>       help
>>         Say yes here to build support for Asahi Kasei AK8975 3-Axis
>> -      Magnetometer.
>> +      Magnetometer. This driver can also support AK8963, if i2c
>> +      device name is identified as ak8963.
>>
>>         To compile this driver as a module, choose M here: the module
>>         will be called ak8975.
>> diff --git a/drivers/iio/magnetometer/ak8975.c 
>> b/drivers/iio/magnetometer/ak8975.c
>> index 0542354..a55c94f 100644
>> --- a/drivers/iio/magnetometer/ak8975.c
>> +++ b/drivers/iio/magnetometer/ak8975.c
>> @@ -85,7 +85,15 @@
>>   #define AK8975_MAX_CONVERSION_TIMEOUT    500
>>   #define AK8975_CONVERSION_DONE_POLL_TIME 10
>>   #define AK8975_DATA_READY_TIMEOUT    ((100*HZ)/1000)
>> -#define RAW_TO_GAUSS(asa) ((((asa) + 128) * 3000) / 256)
>> +#define RAW_TO_GAUSS_8975(asa) ((((asa) + 128) * 3000) / 256)
>> +#define RAW_TO_GAUSS_8963(asa) ((((asa) + 128) * 6000) / 256)
>> +
>> +/* Compatible Asahi Kasei Compass parts */
>> +enum asahi_compass_chipset {
>> +    AK8975,
>> +    AK8963,
>> +    AK_INVALID,
>
> Not sure if you need AK_INVALID, it is never used.
>
>> +};
>>
>>   /*
>>    * Per-instance context data for the device.
>> @@ -101,6 +109,7 @@ struct ak8975_data {
>>       int            eoc_irq;
>>       wait_queue_head_t    data_ready_queue;
>>       unsigned long        flags;
>> +    int            chipset;
>
> enum asahi_compass_chipset ...
>
>>   };
>>
>>   static const int ak8975_index_to_reg[] = {
>> @@ -272,9 +281,21 @@ static int ak8975_setup(struct i2c_client *client)
>>    * Since ASA doesn't change, we cache the resultant scale factor 
>> into the
>>    * device context in ak8975_setup().
>>    */
>> -    data->raw_to_gauss[0] = RAW_TO_GAUSS(data->asa[0]);
>> -    data->raw_to_gauss[1] = RAW_TO_GAUSS(data->asa[1]);
>> -    data->raw_to_gauss[2] = RAW_TO_GAUSS(data->asa[2]);
>> +    if (data->chipset == AK8963) {
>> +        /*
>> +         * H range is +-8190 and magnetometer range is +-4912.
>> +         * So HuT using the above explanation for 8975,
>> +         * 4912/8190 = ~ 6/10.
>> +         * So the Hadj should use 6/10 instead of 3/10.
>> +         */
>> +        data->raw_to_gauss[0] = RAW_TO_GAUSS_8963(data->asa[0]);
>> +        data->raw_to_gauss[1] = RAW_TO_GAUSS_8963(data->asa[1]);
>> +        data->raw_to_gauss[2] = RAW_TO_GAUSS_8963(data->asa[2]);
>> +    } else {
>> +        data->raw_to_gauss[0] = RAW_TO_GAUSS_8975(data->asa[0]);
>> +        data->raw_to_gauss[1] = RAW_TO_GAUSS_8975(data->asa[1]);
>> +        data->raw_to_gauss[2] = RAW_TO_GAUSS_8975(data->asa[2]);
>> +    }
>>
>>       return 0;
>>   }
>> @@ -499,6 +520,16 @@ static int ak8975_probe(struct i2c_client *client,
>>       data->eoc_gpio = eoc_gpio;
>>       data->eoc_irq = 0;
>>
>> +    if (id) {
>
> id will never be NULL.
>
>> +        data->chipset = (int)(id->driver_data);
>> +        if (data->chipset == AK8963)
>> +            dev_dbg(&client->dev,
>> +                "AK8975 driver is running in AK8963 mode\n");
>> +        else
>> +            dev_dbg(&client->dev,
>> +                "AK8975 driver is running in normal mode\n");
>
> How about printing just id->name, this way you do not have to add a 
> new else branch when a new part is supported.
>
>> +    }
>> +
>>       /* Perform some basic start-of-day setup of the device. */
>>       err = ak8975_setup(client);
>>       if (err < 0) {
>> @@ -552,6 +583,7 @@ static int ak8975_remove(struct i2c_client *client)
>>
>>   static const struct i2c_device_id ak8975_id[] = {
>>       {"ak8975", 0},
>
> Should change the 0 to AK8975
>
>> +    {"ak8963", AK8963},
>>       {}
>>   };
>>
>>
>
>

Re: [PATCH 1/3] iio: ak8975 : Add AK8963 compatibility mode support

[Jonathan Cameron]

On 12/03/14 18:30, Srinivas Pandruvada wrote:
> On 03/12/2014 03:07 AM, Lars-Peter Clausen wrote:
>> On 03/11/2014 10:15 PM, Srinivas Pandruvada wrote:
>>> AK8963 and AK8975 use same register definitions, except the range
>>> of X,Y,Z values. Added support of 8963 based on i2c_device_id.
>>> Unfortunately there is no way to detect the type via registers,
>>> both device registers return 0x48 as id of chipset.
>>>
>>> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
>>
>> Hi,
>>
>> a few small comments.
>>
>
> Thanks. I will take care in the next version.
>
> -Srinivas
Other than points Lars raised, this looks fine to me.

Jonathan
>
>>> ---
>>>   drivers/iio/magnetometer/Kconfig  |  3 ++-
>>>   drivers/iio/magnetometer/ak8975.c | 40 +++++++++++++++++++++++++++++++++++----
>>>   2 files changed, 38 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/iio/magnetometer/Kconfig b/drivers/iio/magnetometer/Kconfig
>>> index d86d226..05a364c54 100644
>>> --- a/drivers/iio/magnetometer/Kconfig
>>> +++ b/drivers/iio/magnetometer/Kconfig
>>> @@ -11,7 +11,8 @@ config AK8975
>>>       depends on GPIOLIB
>>>       help
>>>         Say yes here to build support for Asahi Kasei AK8975 3-Axis
>>> -      Magnetometer.
>>> +      Magnetometer. This driver can also support AK8963, if i2c
>>> +      device name is identified as ak8963.
>>>
>>>         To compile this driver as a module, choose M here: the module
>>>         will be called ak8975.
>>> diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
>>> index 0542354..a55c94f 100644
>>> --- a/drivers/iio/magnetometer/ak8975.c
>>> +++ b/drivers/iio/magnetometer/ak8975.c
>>> @@ -85,7 +85,15 @@
>>>   #define AK8975_MAX_CONVERSION_TIMEOUT    500
>>>   #define AK8975_CONVERSION_DONE_POLL_TIME 10
>>>   #define AK8975_DATA_READY_TIMEOUT    ((100*HZ)/1000)
>>> -#define RAW_TO_GAUSS(asa) ((((asa) + 128) * 3000) / 256)
>>> +#define RAW_TO_GAUSS_8975(asa) ((((asa) + 128) * 3000) / 256)
>>> +#define RAW_TO_GAUSS_8963(asa) ((((asa) + 128) * 6000) / 256)
>>> +
>>> +/* Compatible Asahi Kasei Compass parts */
>>> +enum asahi_compass_chipset {
>>> +    AK8975,
>>> +    AK8963,
>>> +    AK_INVALID,
>>
>> Not sure if you need AK_INVALID, it is never used.
>>
>>> +};
>>>
>>>   /*
>>>    * Per-instance context data for the device.
>>> @@ -101,6 +109,7 @@ struct ak8975_data {
>>>       int            eoc_irq;
>>>       wait_queue_head_t    data_ready_queue;
>>>       unsigned long        flags;
>>> +    int            chipset;
>>
>> enum asahi_compass_chipset ...
>>
>>>   };
>>>
>>>   static const int ak8975_index_to_reg[] = {
>>> @@ -272,9 +281,21 @@ static int ak8975_setup(struct i2c_client *client)
>>>    * Since ASA doesn't change, we cache the resultant scale factor into the
>>>    * device context in ak8975_setup().
>>>    */
>>> -    data->raw_to_gauss[0] = RAW_TO_GAUSS(data->asa[0]);
>>> -    data->raw_to_gauss[1] = RAW_TO_GAUSS(data->asa[1]);
>>> -    data->raw_to_gauss[2] = RAW_TO_GAUSS(data->asa[2]);
>>> +    if (data->chipset == AK8963) {
>>> +        /*
>>> +         * H range is +-8190 and magnetometer range is +-4912.
>>> +         * So HuT using the above explanation for 8975,
>>> +         * 4912/8190 = ~ 6/10.
>>> +         * So the Hadj should use 6/10 instead of 3/10.
>>> +         */
>>> +        data->raw_to_gauss[0] = RAW_TO_GAUSS_8963(data->asa[0]);
>>> +        data->raw_to_gauss[1] = RAW_TO_GAUSS_8963(data->asa[1]);
>>> +        data->raw_to_gauss[2] = RAW_TO_GAUSS_8963(data->asa[2]);
>>> +    } else {
>>> +        data->raw_to_gauss[0] = RAW_TO_GAUSS_8975(data->asa[0]);
>>> +        data->raw_to_gauss[1] = RAW_TO_GAUSS_8975(data->asa[1]);
>>> +        data->raw_to_gauss[2] = RAW_TO_GAUSS_8975(data->asa[2]);
>>> +    }
>>>
>>>       return 0;
>>>   }
>>> @@ -499,6 +520,16 @@ static int ak8975_probe(struct i2c_client *client,
>>>       data->eoc_gpio = eoc_gpio;
>>>       data->eoc_irq = 0;
>>>
>>> +    if (id) {
>>
>> id will never be NULL.
>>
>>> +        data->chipset = (int)(id->driver_data);
>>> +        if (data->chipset == AK8963)
>>> +            dev_dbg(&client->dev,
>>> +                "AK8975 driver is running in AK8963 mode\n");
>>> +        else
>>> +            dev_dbg(&client->dev,
>>> +                "AK8975 driver is running in normal mode\n");
>>
>> How about printing just id->name, this way you do not have to add a new else branch when a new part is supported.
>>
>>> +    }
>>> +
>>>       /* Perform some basic start-of-day setup of the device. */
>>>       err = ak8975_setup(client);
>>>       if (err < 0) {
>>> @@ -552,6 +583,7 @@ static int ak8975_remove(struct i2c_client *client)
>>>
>>>   static const struct i2c_device_id ak8975_id[] = {
>>>       {"ak8975", 0},
>>
>> Should change the 0 to AK8975
>>
>>> +    {"ak8963", AK8963},
>>>       {}
>>>   };
>>>
>>>
>>
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] iio: ak8975: Added ACPI enumeration

[Jonathan Cameron]

On 11/03/14 21:15, Srinivas Pandruvada wrote:
> Added capability so that this device can be enumerated via ACPI.
>
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Please include a relevant list for the ACPI side of this in the CCs for the next
version.  I certainly have very little ACPI experience, so would be good if someone
who has can take a quick look!

The comments below are much the same as for the previous patch...
> ---
>   drivers/iio/magnetometer/ak8975.c | 35 ++++++++++++++++++++++++++++++++++-
>   1 file changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
> index a55c94f..fe5e9c8 100644
> --- a/drivers/iio/magnetometer/ak8975.c
> +++ b/drivers/iio/magnetometer/ak8975.c
> @@ -31,6 +31,7 @@
>   #include <linux/bitops.h>
>   #include <linux/gpio.h>
>   #include <linux/of_gpio.h>
> +#include <linux/acpi.h>
>
>   #include <linux/iio/iio.h>
>   #include <linux/iio/sysfs.h>
> @@ -476,6 +477,27 @@ static const struct iio_info ak8975_info = {
>   	.driver_module = THIS_MODULE,
>   };
>
> +static const struct acpi_device_id ak_acpi_match[] = {
> +	{"AK8975", 0},
As with the previous patch, use the enum entry for the 0 as well.
> +	{"AK8963", AK8963},
> +	{"INVN6500", AK8963},
I'll take your word for it that this last one makes sense!
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(acpi, ak_acpi_match);
> +
> +static char *ak8975_match_acpi_device(struct device *dev,
> +							int *chipset)
> +{
> +	const struct acpi_device_id *id;
> +
> +	id = acpi_match_device(dev->driver->acpi_match_table, dev);
> +	if (!id)
> +		return NULL;
> +	*chipset = (int)id->driver_data;
> +
> +	return (char *)dev_name(dev);
> +}
> +
>   static int ak8975_probe(struct i2c_client *client,
>   			const struct i2c_device_id *id)
>   {
> @@ -483,7 +505,10 @@ static int ak8975_probe(struct i2c_client *client,
>   	struct iio_dev *indio_dev;
>   	int eoc_gpio;
>   	int err;
> +	char *name = NULL;
>
I'm guessing id still can't be null.  Hence no need to set name to NULL either.
    
> +	if (id)
> +		name = (char *) id->name;
>   	/* Grab and set up the supplied GPIO. */
>   	if (client->dev.platform_data)
>   		eoc_gpio = *(int *)(client->dev.platform_data);
> @@ -529,6 +554,13 @@ static int ak8975_probe(struct i2c_client *client,
>   			dev_dbg(&client->dev,
>   				"AK8975 driver is running in normal mode\n");
>   	}
> +	if (ACPI_HANDLE(&client->dev)) {
> +		int chip_id;
> +
> +		name = ak8975_match_acpi_device(&client->dev, &chip_id);
> +		if (chip_id > 0)
> +			data->chipset = chip_id;
> +	}
>
>   	/* Perform some basic start-of-day setup of the device. */
>   	err = ak8975_setup(client);
> @@ -545,7 +577,7 @@ static int ak8975_probe(struct i2c_client *client,
>   	indio_dev->num_channels = ARRAY_SIZE(ak8975_channels);
>   	indio_dev->info = &ak8975_info;
>   	indio_dev->modes = INDIO_DIRECT_MODE;
> -
Just use id->name directly?
> +	indio_dev->name = name;
>   	err = iio_device_register(indio_dev);
>   	if (err < 0)
>   		goto exit_free_iio;
> @@ -600,6 +632,7 @@ static struct i2c_driver ak8975_driver = {
>   	.driver = {
>   		.name	= "ak8975",
>   		.of_match_table = ak8975_of_match,
> +		.acpi_match_table = ACPI_PTR(ak_acpi_match),
>   	},
>   	.probe		= ak8975_probe,
>   	.remove		= ak8975_remove,
>

Re: [PATCH 3/3] iio: ak8975: Don't bail out for irq setup error

[Jonathan Cameron]

On 11/03/14 21:15, Srinivas Pandruvada wrote:
> This driver can very well work via polling, if request_irq failed.
> The problem is that many times i2c interrupts are shared. This
> driver is not ready for shared interrupt as it will return IRQ_HANDLED,
> in every case. Here we will get EBUSY when some other driver registered
> handler and this driver is grabbing in exclusive mode.
> So in this case just print error and continue in polling mode.
>
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
I'm not sure this is a good solution as it means that we will get very different
results depending on the probe order of various drivers.  How do other i2c drivers
cope with the fact that there is no way of knowing within the interrupt routine
that the device was the one causing the interrupt?

A quick grep suggests that the normal trick is to had over to an irq thread
then return IRQ_NONE from that once we know it isn't our irq.  Could we have
move the queue wakeup into a a thread (converting to a threaded interrupt) but
before it query the hardware to establish it is actually our interrupt?
> ---
>   drivers/iio/magnetometer/ak8975.c | 12 +++++++++---
>   1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
> index fe5e9c8..0f9c407 100644
> --- a/drivers/iio/magnetometer/ak8975.c
> +++ b/drivers/iio/magnetometer/ak8975.c
> @@ -236,9 +236,15 @@ static int ak8975_setup(struct i2c_client *client)
>   	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 == -EBUSY) {
> +				dev_err(&client->dev,
> +					"device Intr busy:polling required\n");
> +				ret = 0;
> +			} else {
> +				dev_err(&client->dev,
> +					"Error setting data ready interrupt\n");
> +				return ret;
> +			}
>   		}
>   	}
>
>

Re: [PATCH 3/3] iio: ak8975: Don't bail out for irq setup error

[Srinivas Pandruvada]

On 03/15/2014 08:14 AM, Jonathan Cameron wrote:
> On 11/03/14 21:15, Srinivas Pandruvada wrote:
>> This driver can very well work via polling, if request_irq failed.
>> The problem is that many times i2c interrupts are shared. This
>> driver is not ready for shared interrupt as it will return IRQ_HANDLED,
>> in every case. Here we will get EBUSY when some other driver registered
>> handler and this driver is grabbing in exclusive mode.
>> So in this case just print error and continue in polling mode.
>>
>> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> I'm not sure this is a good solution as it means that we will get very 
> different
> results depending on the probe order of various drivers.  How do other 
> i2c drivers
> cope with the fact that there is no way of knowing within the 
> interrupt routine
> that the device was the one causing the interrupt?
>
Agreed. The i2c driver currently don't receive irq flags in the client 
structure. They
only get irq number. We do get irq flags from the ACPI in this case or 
from some
platform init code. I have another patch to enhance ACPI i2c binding and 
also enhance
i2c_client structure.
In addition this driver can be enhanced to use shared irq.

Thanks,
Srinivas


> A quick grep suggests that the normal trick is to had over to an irq 
> thread
> then return IRQ_NONE from that once we know it isn't our irq. Could we 
> have
> move the queue wakeup into a a thread (converting to a threaded 
> interrupt) but
> before it query the hardware to establish it is actually our interrupt?
>> ---
>>   drivers/iio/magnetometer/ak8975.c | 12 +++++++++---
>>   1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iio/magnetometer/ak8975.c 
>> b/drivers/iio/magnetometer/ak8975.c
>> index fe5e9c8..0f9c407 100644
>> --- a/drivers/iio/magnetometer/ak8975.c
>> +++ b/drivers/iio/magnetometer/ak8975.c
>> @@ -236,9 +236,15 @@ static int ak8975_setup(struct i2c_client *client)
>>       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 == -EBUSY) {
>> +                dev_err(&client->dev,
>> +                    "device Intr busy:polling required\n");
>> +                ret = 0;
>> +            } else {
>> +                dev_err(&client->dev,
>> +                    "Error setting data ready interrupt\n");
>> +                return ret;
>> +            }
>>           }
>>       }
>>
>>
>
>

Re: [PATCH 3/3] iio: ak8975: Don't bail out for irq setup error

[Jonathan Cameron]

On March 17, 2014 3:44:43 PM GMT+00:00, Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote:
>On 03/15/2014 08:14 AM, Jonathan Cameron wrote:
>> On 11/03/14 21:15, Srinivas Pandruvada wrote:
>>> This driver can very well work via polling, if request_irq failed.
>>> The problem is that many times i2c interrupts are shared. This
>>> driver is not ready for shared interrupt as it will return
>IRQ_HANDLED,
>>> in every case. Here we will get EBUSY when some other driver
>registered
>>> handler and this driver is grabbing in exclusive mode.
>>> So in this case just print error and continue in polling mode.
>>>
>>> Signed-off-by: Srinivas Pandruvada
><srinivas.pandruvada@linux.intel.com>
>> I'm not sure this is a good solution as it means that we will get
>very 
>> different
>> results depending on the probe order of various drivers.  How do
>other 
>> i2c drivers
>> cope with the fact that there is no way of knowing within the 
>> interrupt routine
>> that the device was the one causing the interrupt?
>>
>Agreed. The i2c driver currently don't receive irq flags in the client 
>structure. They
>only get irq number. We do get irq flags from the ACPI in this case or 
>from some
>platform init code. I have another patch to enhance ACPI i2c binding
>and 
>also enhance
>i2c_client structure.
>In addition this driver can be enhanced to use shared irq.
Let's hold this patch and do it properly once support is in place... 
>
>Thanks,
>Srinivas
>
>
>> A quick grep suggests that the normal trick is to had over to an irq 
>> thread
>> then return IRQ_NONE from that once we know it isn't our irq. Could
>we 
>> have
>> move the queue wakeup into a a thread (converting to a threaded 
>> interrupt) but
>> before it query the hardware to establish it is actually our
>interrupt?
>>> ---
>>>   drivers/iio/magnetometer/ak8975.c | 12 +++++++++---
>>>   1 file changed, 9 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/iio/magnetometer/ak8975.c 
>>> b/drivers/iio/magnetometer/ak8975.c
>>> index fe5e9c8..0f9c407 100644
>>> --- a/drivers/iio/magnetometer/ak8975.c
>>> +++ b/drivers/iio/magnetometer/ak8975.c
>>> @@ -236,9 +236,15 @@ static int ak8975_setup(struct i2c_client
>*client)
>>>       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 == -EBUSY) {
>>> +                dev_err(&client->dev,
>>> +                    "device Intr busy:polling required\n");
>>> +                ret = 0;
>>> +            } else {
>>> +                dev_err(&client->dev,
>>> +                    "Error setting data ready interrupt\n");
>>> +                return ret;
>>> +            }
>>>           }
>>>       }
>>>
>>>
>>
>>
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Sent from my Android phone with K-9 Mail. Please excuse my brevity.