iio: v2, add support for HMC5883/HMC5883L to HMC5843 driver

iio: v2, add support for HMC5883/HMC5883L to HMC5843 driver

[Peter Meerwald]

here is my updated patch series to extend the HMC5843 magnetometer 
driver to support the HMC5883/HMC5883L devices

patch 1 fixes access issues to the hmc5843 private data,
patches 2 to 7 are preparatory patches,
patch 8 actually brings in the support for the new devices

the timeout in the read loop has not yet been implemented (suggestions by 
Lars-Peter Clausen)

the driver has a severe limitation: X/Y/Z must ALL be read to allow new
data to become available (the device goes into a locked state until all 
are read)

[PATCH 1/8] iio: fix access to hmc5843 private data

[Peter Meerwald]

From: Peter Meerwald <p.meerwald@bct-electronic.com>

i2c_get_clientdata(client) points to iio_dev, not hmc5843_data; fixes
issue similar to 62d2feb9803f18c4e3c8a1a2c7e30a54df8a1d72

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/staging/iio/magnetometer/hmc5843.c |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
index 3ec6518..9725cf8 100644
--- a/drivers/staging/iio/magnetometer/hmc5843.c
+++ b/drivers/staging/iio/magnetometer/hmc5843.c
@@ -250,7 +250,8 @@ static IIO_DEVICE_ATTR(operating_mode,
 static s32 hmc5843_set_meas_conf(struct i2c_client *client,
 				      u8 meas_conf)
 {
-	struct hmc5843_data *data = i2c_get_clientdata(client);
+	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+	struct hmc5843_data *data = iio_priv(indio_dev);
 	u8 reg_val;
 	reg_val = (meas_conf & MEAS_CONF_MASK) |  (data->rate << RATE_OFFSET);
 	return i2c_smbus_write_byte_data(client, HMC5843_CONFIG_REG_A, reg_val);
@@ -272,7 +273,7 @@ static ssize_t hmc5843_set_measurement_configuration(struct device *dev,
 {
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct i2c_client *client = to_i2c_client(indio_dev->dev.parent);
-	struct hmc5843_data *data = i2c_get_clientdata(client);
+	struct hmc5843_data *data = iio_priv(indio_dev);
 	unsigned long meas_conf = 0;
 	int error = strict_strtoul(buf, 10, &meas_conf);
 	if (error)
@@ -314,7 +315,8 @@ static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("0.5 1 2 5 10 20 50");
 static s32 hmc5843_set_rate(struct i2c_client *client,
 				u8 rate)
 {
-	struct hmc5843_data *data = i2c_get_clientdata(client);
+	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+	struct hmc5843_data *data = iio_priv(indio_dev);
 	u8 reg_val;
 
 	reg_val = (data->meas_conf) |  (rate << RATE_OFFSET);
@@ -600,8 +602,10 @@ static int hmc5843_suspend(struct device *dev)
 
 static int hmc5843_resume(struct device *dev)
 {
-	struct hmc5843_data *data = i2c_get_clientdata(to_i2c_client(dev));
-	hmc5843_configure(to_i2c_client(dev), data->operating_mode);
+	struct i2c_client *client = to_i2c_client(dev);
+	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+	struct hmc5843_data *data = iio_priv(indio_dev);
+	hmc5843_configure(client, data->operating_mode);
 	return 0;
 }
 
-- 
1.7.5.4

[PATCH 2/8] iio: change strict_strtoul() to kstrtoul() in hmc5843

[Peter Meerwald]

From: Peter Meerwald <p.meerwald@bct-electronic.com>

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/staging/iio/magnetometer/hmc5843.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
index 9725cf8..018d3d8 100644
--- a/drivers/staging/iio/magnetometer/hmc5843.c
+++ b/drivers/staging/iio/magnetometer/hmc5843.c
@@ -202,7 +202,7 @@ static ssize_t hmc5843_set_operating_mode(struct device *dev,
 	s32 status;
 	int error;
 	mutex_lock(&data->lock);
-	error = strict_strtoul(buf, 10, &operating_mode);
+	error = kstrtoul(buf, 10, &operating_mode);
 	if (error) {
 		count = error;
 		goto exit;
@@ -275,7 +275,7 @@ static ssize_t hmc5843_set_measurement_configuration(struct device *dev,
 	struct i2c_client *client = to_i2c_client(indio_dev->dev.parent);
 	struct hmc5843_data *data = iio_priv(indio_dev);
 	unsigned long meas_conf = 0;
-	int error = strict_strtoul(buf, 10, &meas_conf);
+	int error = kstrtoul(buf, 10, &meas_conf);
 	if (error)
 		return error;
 	mutex_lock(&data->lock);
@@ -425,7 +425,7 @@ static ssize_t set_range(struct device *dev,
 	unsigned long range = 0;
 	int error;
 	mutex_lock(&data->lock);
-	error = strict_strtoul(buf, 10, &range);
+	error = kstrtoul(buf, 10, &range);
 	if (error) {
 		count = error;
 		goto exit;
-- 
1.7.5.4

[PATCH 3/8] iio: rename and prefix CONSTANTs to distinguish between HMC5843 and HMC5883

[Peter Meerwald]

From: Peter Meerwald <p.meerwald@bct-electronic.com>

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/staging/iio/magnetometer/hmc5843.c |  128 +++++++++++++++------------
 1 files changed, 71 insertions(+), 57 deletions(-)

diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
index 018d3d8..1318f7d 100644
--- a/drivers/staging/iio/magnetometer/hmc5843.c
+++ b/drivers/staging/iio/magnetometer/hmc5843.c
@@ -25,8 +25,6 @@
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 
-#define HMC5843_I2C_ADDRESS			0x1E
-
 #define HMC5843_CONFIG_REG_A			0x00
 #define HMC5843_CONFIG_REG_B			0x01
 #define HMC5843_MODE_REG			0x02
@@ -36,65 +34,80 @@
 #define HMC5843_DATA_OUT_Y_LSB_REG		0x06
 #define HMC5843_DATA_OUT_Z_MSB_REG		0x07
 #define HMC5843_DATA_OUT_Z_LSB_REG		0x08
+/* Beware: Y and Z are exchanged on HMC5883 */
+#define HMC5883_DATA_OUT_Z_MSB_REG		0x05
+#define HMC5883_DATA_OUT_Z_LSB_REG		0x06
+#define HMC5883_DATA_OUT_Y_MSB_REG		0x07
+#define HMC5883_DATA_OUT_Y_LSB_REG		0x08
 #define HMC5843_STATUS_REG			0x09
 #define HMC5843_ID_REG_A			0x0A
 #define HMC5843_ID_REG_B			0x0B
 #define HMC5843_ID_REG_C			0x0C
 
+
+/*
+ * Beware: identification of the HMC5883 is still "H43";
+ * I2C address is also unchanged
+ */
 #define HMC5843_ID_REG_LENGTH			0x03
 #define HMC5843_ID_STRING			"H43"
+#define HMC5843_I2C_ADDRESS			0x1E
 
 /*
- * Range settings in  (+-)Ga
- * */
-#define RANGE_GAIN_OFFSET			0x05
-
-#define	RANGE_0_7				0x00
-#define	RANGE_1_0				0x01 /* default */
-#define	RANGE_1_5				0x02
-#define	RANGE_2_0				0x03
-#define	RANGE_3_2				0x04
-#define	RANGE_3_8				0x05
-#define	RANGE_4_5				0x06
-#define	RANGE_6_5				0x07 /* Not recommended */
+ * Range gain settings in (+-)Ga
+ * Beware: HMC5843 and HMC5883 have different recommended sensor field
+ * ranges; default corresponds to +-1.0 Ga and +-1.3 Ga, respectively
+ */
+#define HMC5843_RANGE_GAIN_OFFSET		0x05
+#define HMC5843_RANGE_GAIN_DEFAULT		0x01
+#define HMC5843_RANGE_GAIN_MAX			0x07
 
 /*
  * Device status
  */
-#define	DATA_READY				0x01
-#define	DATA_OUTPUT_LOCK			0x02
-#define	VOLTAGE_REGULATOR_ENABLED		0x04
+#define HMC5843_DATA_READY			0x01
+#define HMC5843_DATA_OUTPUT_LOCK		0x02
+/* Does not exist on HMC5883, not used */
+#define HMC5843_VOLTAGE_REGULATOR_ENABLED	0x04
 
 /*
  * Mode register configuration
  */
-#define	MODE_CONVERSION_CONTINUOUS		0x00
-#define	MODE_CONVERSION_SINGLE			0x01
-#define	MODE_IDLE				0x02
-#define	MODE_SLEEP				0x03
-
-/* Minimum Data Output Rate in 1/10 Hz  */
-#define RATE_OFFSET				0x02
-#define RATE_BITMASK				0x1C
-#define	RATE_5					0x00
-#define	RATE_10					0x01
-#define	RATE_20					0x02
-#define	RATE_50					0x03
-#define	RATE_100				0x04
-#define	RATE_200				0x05
-#define	RATE_500				0x06
-#define	RATE_NOT_USED				0x07
+#define HMC5843_MODE_CONVERSION_CONTINUOUS	0x00
+#define HMC5843_MODE_CONVERSION_SINGLE		0x01
+#define HMC5843_MODE_IDLE			0x02
+#define HMC5843_MODE_SLEEP			0x03
+#define HMC5843_MODE_MASK			0x03
+
+/*
+ * HMC5843: Minimum data output rate
+ * HMC5883: Typical data output rate
+ */
+#define HMC5843_RATE_OFFSET			0x02
+#define HMC5843_RATE_BITMASK			0x1C
+#define RATE_5					0x00
+#define RATE_10					0x01
+#define RATE_20					0x02
+#define RATE_50					0x03
+#define RATE_100				0x04
+#define RATE_200				0x05
+#define RATE_500				0x06
+
+#define HMC5843_RATE_NOT_USED			0x07
 
 /*
- * Device Configuration
+ * Device measurement configuration
  */
-#define	CONF_NORMAL				0x00
-#define	CONF_POSITIVE_BIAS			0x01
-#define	CONF_NEGATIVE_BIAS			0x02
-#define	CONF_NOT_USED				0x03
-#define	MEAS_CONF_MASK				0x03
+#define HMC5843_MEAS_CONF_NORMAL		0x00
+#define HMC5843_MEAS_CONF_POSITIVE_BIAS		0x01
+#define HMC5843_MEAS_CONF_NEGATIVE_BIAS		0x02
+#define HMC5843_MEAS_CONF_NOT_USED		0x03
+#define HMC5843_MEAS_CONF_MASK			0x03
 
-static int hmc5843_regval_to_nanoscale[] = {
+/*
+ * Scaling factors: 10000000/Gain
+ */
+static const int hmc5843_regval_to_nanoscale[] = {
 	6173, 7692, 10309, 12821, 18868, 21739, 25641, 35714
 };
 
@@ -120,7 +133,7 @@ static const char * const regval_to_samp_freq[] = {
 
 /* Addresses to scan: 0x1E */
 static const unsigned short normal_i2c[] = { HMC5843_I2C_ADDRESS,
-							I2C_CLIENT_END };
+					     I2C_CLIENT_END };
 
 /* Each client has this additional data */
 struct hmc5843_data {
@@ -139,7 +152,7 @@ static s32 hmc5843_configure(struct i2c_client *client,
 	/* The lower two bits contain the current conversion mode */
 	return i2c_smbus_write_byte_data(client,
 					HMC5843_MODE_REG,
-					(operating_mode & 0x03));
+					operating_mode & HMC5843_MODE_MASK);
 }
 
 /* Return the measurement value from the specified channel */
@@ -153,7 +166,7 @@ static int hmc5843_read_measurement(struct iio_dev *indio_dev,
 
 	mutex_lock(&data->lock);
 	result = i2c_smbus_read_byte_data(client, HMC5843_STATUS_REG);
-	while (!(result & DATA_READY))
+	while (!(result & HMC5843_DATA_READY))
 		result = i2c_smbus_read_byte_data(client, HMC5843_STATUS_REG);
 
 	result = i2c_smbus_read_word_data(client, address);
@@ -208,7 +221,7 @@ static ssize_t hmc5843_set_operating_mode(struct device *dev,
 		goto exit;
 	}
 	dev_dbg(dev, "set Conversion mode to %lu\n", operating_mode);
-	if (operating_mode > MODE_SLEEP) {
+	if (operating_mode > HMC5843_MODE_SLEEP) {
 		count = -EINVAL;
 		goto exit;
 	}
@@ -253,7 +266,8 @@ static s32 hmc5843_set_meas_conf(struct i2c_client *client,
 	struct iio_dev *indio_dev = i2c_get_clientdata(client);
 	struct hmc5843_data *data = iio_priv(indio_dev);
 	u8 reg_val;
-	reg_val = (meas_conf & MEAS_CONF_MASK) |  (data->rate << RATE_OFFSET);
+	reg_val = (meas_conf & HMC5843_MEAS_CONF_MASK) |
+		(data->rate << HMC5843_RATE_OFFSET);
 	return i2c_smbus_write_byte_data(client, HMC5843_CONFIG_REG_A, reg_val);
 }
 
@@ -319,8 +333,8 @@ static s32 hmc5843_set_rate(struct i2c_client *client,
 	struct hmc5843_data *data = iio_priv(indio_dev);
 	u8 reg_val;
 
-	reg_val = (data->meas_conf) |  (rate << RATE_OFFSET);
-	if (rate >= RATE_NOT_USED) {
+	reg_val = (data->meas_conf) |  (rate << HMC5843_RATE_OFFSET);
+	if (rate >= HMC5843_RATE_NOT_USED) {
 		dev_err(&client->dev,
 			"This data output rate is not supported\n");
 		return -EINVAL;
@@ -379,7 +393,7 @@ static ssize_t show_sampling_frequency(struct device *dev,
 	rate = i2c_smbus_read_byte_data(client,  this_attr->address);
 	if (rate < 0)
 		return rate;
-	rate = (rate & RATE_BITMASK) >> RATE_OFFSET;
+	rate = (rate & HMC5843_RATE_BITMASK) >> HMC5843_RATE_OFFSET;
 	return sprintf(buf, "%s\n", regval_to_samp_freq[rate]);
 }
 static IIO_DEVICE_ATTR(sampling_frequency,
@@ -432,13 +446,13 @@ static ssize_t set_range(struct device *dev,
 	}
 	dev_dbg(dev, "set range to %lu\n", range);
 
-	if (range > RANGE_6_5) {
+	if (range > HMC5843_RANGE_GAIN_MAX) {
 		count = -EINVAL;
 		goto exit;
 	}
 
 	data->range = range;
-	range = range << RANGE_GAIN_OFFSET;
+	range = range << HMC5843_RANGE_GAIN_OFFSET;
 	if (i2c_smbus_write_byte_data(client, this_attr->address, range))
 		count = -EINVAL;
 
@@ -553,12 +567,12 @@ static int hmc5843_probe(struct i2c_client *client,
 		err = -ENOMEM;
 		goto exit;
 	}
-	data = iio_priv(indio_dev);
-	/* default settings at probe */
 
-	data->meas_conf = CONF_NORMAL;
-	data->range = RANGE_1_0;
-	data->operating_mode = MODE_CONVERSION_CONTINUOUS;
+	/* default settings at probe */
+	data = iio_priv(indio_dev);
+	data->meas_conf = HMC5843_MEAS_CONF_NORMAL;
+	data->range = HMC5843_RANGE_GAIN_DEFAULT;
+	data->operating_mode = HMC5843_MODE_CONVERSION_CONTINUOUS;
 
 	i2c_set_clientdata(client, indio_dev);
 
@@ -587,7 +601,7 @@ static int hmc5843_remove(struct i2c_client *client)
 
 	iio_device_unregister(indio_dev);
 	 /*  sleep mode to save power */
-	hmc5843_configure(client, MODE_SLEEP);
+	hmc5843_configure(client, HMC5843_MODE_SLEEP);
 	iio_device_free(indio_dev);
 
 	return 0;
@@ -596,7 +610,7 @@ static int hmc5843_remove(struct i2c_client *client)
 #ifdef CONFIG_PM_SLEEP
 static int hmc5843_suspend(struct device *dev)
 {
-	hmc5843_configure(to_i2c_client(dev), MODE_SLEEP);
+	hmc5843_configure(to_i2c_client(dev), HMC5843_MODE_SLEEP);
 	return 0;
 }
 
-- 
1.7.5.4

[PATCH 4/8] iio: rework sampling rate setting in hmc5843

[Peter Meerwald]

From: Peter Meerwald <p.meerwald@bct-electronic.com>

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/staging/iio/magnetometer/hmc5843.c |   51 +++++++++++++---------------
 1 files changed, 24 insertions(+), 27 deletions(-)

diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
index 1318f7d..6888e04 100644
--- a/drivers/staging/iio/magnetometer/hmc5843.c
+++ b/drivers/staging/iio/magnetometer/hmc5843.c
@@ -85,14 +85,6 @@
  */
 #define HMC5843_RATE_OFFSET			0x02
 #define HMC5843_RATE_BITMASK			0x1C
-#define RATE_5					0x00
-#define RATE_10					0x01
-#define RATE_20					0x02
-#define RATE_50					0x03
-#define RATE_100				0x04
-#define RATE_200				0x05
-#define RATE_500				0x06
-
 #define HMC5843_RATE_NOT_USED			0x07
 
 /*
@@ -342,6 +334,21 @@ static s32 hmc5843_set_rate(struct i2c_client *client,
 	return i2c_smbus_write_byte_data(client, HMC5843_CONFIG_REG_A, reg_val);
 }
 
+static int hmc5843_check_sampling_frequency(struct hmc5843_data *data,
+						const char *buf)
+{
+	const char * const *samp_freq = regval_to_samp_freq;
+	int i;
+
+	for (i = 0; i < HMC5843_RATE_NOT_USED; i++) {
+		if (strncmp(buf, samp_freq[i],
+			strlen(samp_freq[i])) == 0)
+			return i;
+	}
+
+	return -EINVAL;
+}
+
 static ssize_t set_sampling_frequency(struct device *dev,
 					struct device_attribute *attr,
 					const char *buf, size_t count)
@@ -350,27 +357,17 @@ static ssize_t set_sampling_frequency(struct device *dev,
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct i2c_client *client = to_i2c_client(indio_dev->dev.parent);
 	struct hmc5843_data *data = iio_priv(indio_dev);
-	unsigned long rate = 0;
-
-	if (strncmp(buf, "0.5" , 3) == 0)
-		rate = RATE_5;
-	else if (strncmp(buf, "1" , 1) == 0)
-		rate = RATE_10;
-	else if (strncmp(buf, "2", 1) == 0)
-		rate = RATE_20;
-	else if (strncmp(buf, "5", 1) == 0)
-		rate = RATE_50;
-	else if (strncmp(buf, "10", 2) == 0)
-		rate = RATE_100;
-	else if (strncmp(buf, "20" , 2) == 0)
-		rate = RATE_200;
-	else if (strncmp(buf, "50" , 2) == 0)
-		rate = RATE_500;
-	else
-		return -EINVAL;
+	int rate;
+
+	rate = hmc5843_check_sampling_frequency(data, buf);
+	if (rate < 0) {
+		dev_err(&client->dev,
+			"sampling frequency is not supported\n");
+		return rate;
+	}
 
 	mutex_lock(&data->lock);
-	dev_dbg(dev, "set rate to %lu\n", rate);
+	dev_dbg(dev, "set rate to %d\n", rate);
 	if (hmc5843_set_rate(client, rate)) {
 		count = -EINVAL;
 		goto exit;
-- 
1.7.5.4

[PATCH 5/8] iio: add check for measurement configuration value passed to hmc5843

[Peter Meerwald]

From: Peter Meerwald <p.meerwald@bct-electronic.com>

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/staging/iio/magnetometer/hmc5843.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
index 6888e04..57ab4fb 100644
--- a/drivers/staging/iio/magnetometer/hmc5843.c
+++ b/drivers/staging/iio/magnetometer/hmc5843.c
@@ -281,9 +281,14 @@ static ssize_t hmc5843_set_measurement_configuration(struct device *dev,
 	struct i2c_client *client = to_i2c_client(indio_dev->dev.parent);
 	struct hmc5843_data *data = iio_priv(indio_dev);
 	unsigned long meas_conf = 0;
-	int error = kstrtoul(buf, 10, &meas_conf);
+	int error;
+
+	error = kstrtoul(buf, 10, &meas_conf);
 	if (error)
 		return error;
+	if (meas_conf >= HMC5843_MEAS_CONF_NOT_USED)
+		return -EINVAL;
+
 	mutex_lock(&data->lock);
 
 	dev_dbg(dev, "set mode to %lu\n", meas_conf);
-- 
1.7.5.4

[PATCH 6/8] iio: cleanup and move comments in hmc5843

[Peter Meerwald]

From: Peter Meerwald <p.meerwald@bct-electronic.com>

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/staging/iio/magnetometer/hmc5843.c |  122 ++++++++++++++-------------
 1 files changed, 63 insertions(+), 59 deletions(-)

diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
index 57ab4fb..57ed182 100644
--- a/drivers/staging/iio/magnetometer/hmc5843.c
+++ b/drivers/staging/iio/magnetometer/hmc5843.c
@@ -103,6 +103,18 @@ static const int hmc5843_regval_to_nanoscale[] = {
 	6173, 7692, 10309, 12821, 18868, 21739, 25641, 35714
 };
 
+/*
+ * From the datasheet:
+ * Value	| Sensor input field range (Ga)	| Gain (counts/milli-Gauss)
+ * 0		| (+-)0.7			| 1620
+ * 1		| (+-)1.0			| 1300
+ * 2		| (+-)1.5			| 970
+ * 3		| (+-)2.0			| 780
+ * 4		| (+-)3.2			| 530
+ * 5		| (+-)3.8			| 460
+ * 6		| (+-)4.5			| 390
+ * 7		| (+-)6.5			| 280
+ */
 static const int regval_to_input_field_mg[] = {
 	700,
 	1000,
@@ -113,6 +125,19 @@ static const int regval_to_input_field_mg[] = {
 	4500,
 	6500
 };
+
+/*
+ * From the datasheet:
+ * Value	| Data output rate (Hz)
+ * 0		| 0.5
+ * 1		| 1
+ * 2		| 2
+ * 3		| 5
+ * 4		| 10 (default)
+ * 5		| 20
+ * 6		| 50
+ * 7		| Not used
+ */
 static const char * const regval_to_samp_freq[] = {
 	"0.5",
 	"1",
@@ -130,18 +155,16 @@ static const unsigned short normal_i2c[] = { HMC5843_I2C_ADDRESS,
 /* Each client has this additional data */
 struct hmc5843_data {
 	struct mutex lock;
-	u8		rate;
-	u8		meas_conf;
-	u8		operating_mode;
-	u8		range;
+	u8 rate;
+	u8 meas_conf;
+	u8 operating_mode;
+	u8 range;
 };
 
-static void hmc5843_init_client(struct i2c_client *client);
-
+/* The lower two bits contain the current conversion mode */
 static s32 hmc5843_configure(struct i2c_client *client,
 				       u8 operating_mode)
 {
-	/* The lower two bits contain the current conversion mode */
 	return i2c_smbus_write_byte_data(client,
 					HMC5843_MODE_REG,
 					operating_mode & HMC5843_MODE_MASK);
@@ -166,23 +189,22 @@ static int hmc5843_read_measurement(struct iio_dev *indio_dev,
 	if (result < 0)
 		return -EINVAL;
 
-	*val	= (s16)swab16((u16)result);
+	*val = (s16)swab16((u16)result);
 	return IIO_VAL_INT;
 }
 
-
 /*
- * From the datasheet
+ * From the datasheet:
  * 0 - Continuous-Conversion Mode: In continuous-conversion mode, the
- * device continuously performs conversions and places the result in the
- * data register.
+ *     device continuously performs conversions and places the result in
+ *     the data register.
  *
- * 1 - Single-Conversion Mode : device performs a single measurement,
- *  sets RDY high and returned to sleep mode
+ * 1 - Single-Conversion Mode : Device performs a single measurement,
+ *     sets RDY high and returns to sleep mode.
  *
- * 2 - Idle Mode :  Device is placed in idle mode.
+ * 2 - Idle Mode : Device is placed in idle mode.
  *
- * 3 - Sleep Mode. Device is placed in sleep mode.
+ * 3 - Sleep Mode : Device is placed in sleep mode.
  *
  */
 static ssize_t hmc5843_show_operating_mode(struct device *dev,
@@ -206,13 +228,14 @@ static ssize_t hmc5843_set_operating_mode(struct device *dev,
 	unsigned long operating_mode = 0;
 	s32 status;
 	int error;
+
 	mutex_lock(&data->lock);
 	error = kstrtoul(buf, 10, &operating_mode);
 	if (error) {
 		count = error;
 		goto exit;
 	}
-	dev_dbg(dev, "set Conversion mode to %lu\n", operating_mode);
+	dev_dbg(dev, "set conversion mode to %lu\n", operating_mode);
 	if (operating_mode > HMC5843_MODE_SLEEP) {
 		count = -EINVAL;
 		goto exit;
@@ -230,6 +253,7 @@ exit:
 	mutex_unlock(&data->lock);
 	return count;
 }
+
 static IIO_DEVICE_ATTR(operating_mode,
 			S_IWUSR | S_IRUGO,
 			hmc5843_show_operating_mode,
@@ -239,17 +263,19 @@ static IIO_DEVICE_ATTR(operating_mode,
 /*
  * API for setting the measurement configuration to
  * Normal, Positive bias and Negative bias
- * From the datasheet
  *
- * Normal measurement configuration (default): In normal measurement
- * configuration the device follows normal measurement flow. Pins BP and BN
- * are left floating and high impedance.
+ * From the datasheet:
+ * 0 - Normal measurement configuration (default): In normal measurement
+ *     configuration the device follows normal measurement flow. Pins BP
+ *     and BN are left floating and high impedance.
  *
- * Positive bias configuration: In positive bias configuration, a positive
- * current is forced across the resistive load on pins BP and BN.
+ * 1 - Positive bias configuration: In positive bias configuration, a
+ *     positive current is forced across the resistive load on pins BP
+ *     and BN.
  *
- * Negative bias configuration. In negative bias configuration, a negative
- * current is forced across the resistive load on pins BP and BN.
+ * 2 - Negative bias configuration. In negative bias configuration, a
+ *     negative current is forced across the resistive load on pins BP
+ *     and BN.
  *
  */
 static s32 hmc5843_set_meas_conf(struct i2c_client *client,
@@ -290,8 +316,7 @@ static ssize_t hmc5843_set_measurement_configuration(struct device *dev,
 		return -EINVAL;
 
 	mutex_lock(&data->lock);
-
-	dev_dbg(dev, "set mode to %lu\n", meas_conf);
+	dev_dbg(dev, "set measurement configuration to %lu\n", meas_conf);
 	if (hmc5843_set_meas_conf(client, meas_conf)) {
 		count = -EINVAL;
 		goto exit;
@@ -302,25 +327,13 @@ exit:
 	mutex_unlock(&data->lock);
 	return count;
 }
+
 static IIO_DEVICE_ATTR(meas_conf,
 			S_IWUSR | S_IRUGO,
 			hmc5843_show_measurement_configuration,
 			hmc5843_set_measurement_configuration,
 			0);
 
-/*
- * From Datasheet
- * The table shows the minimum data output
- * Value	| Minimum data output rate(Hz)
- * 0		| 0.5
- * 1		| 1
- * 2		| 2
- * 3		| 5
- * 4		| 10 (default)
- * 5		| 20
- * 6		| 50
- * 7		| Not used
- */
 static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("0.5 1 2 5 10 20 50");
 
 static s32 hmc5843_set_rate(struct i2c_client *client,
@@ -333,7 +346,7 @@ static s32 hmc5843_set_rate(struct i2c_client *client,
 	reg_val = (data->meas_conf) |  (rate << HMC5843_RATE_OFFSET);
 	if (rate >= HMC5843_RATE_NOT_USED) {
 		dev_err(&client->dev,
-			"This data output rate is not supported\n");
+			"data output rate is not supported\n");
 		return -EINVAL;
 	}
 	return i2c_smbus_write_byte_data(client, HMC5843_CONFIG_REG_A, reg_val);
@@ -392,31 +405,19 @@ static ssize_t show_sampling_frequency(struct device *dev,
 	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
 	s32 rate;
 
-	rate = i2c_smbus_read_byte_data(client,  this_attr->address);
+	rate = i2c_smbus_read_byte_data(client, this_attr->address);
 	if (rate < 0)
 		return rate;
 	rate = (rate & HMC5843_RATE_BITMASK) >> HMC5843_RATE_OFFSET;
 	return sprintf(buf, "%s\n", regval_to_samp_freq[rate]);
 }
+
 static IIO_DEVICE_ATTR(sampling_frequency,
 			S_IWUSR | S_IRUGO,
 			show_sampling_frequency,
 			set_sampling_frequency,
 			HMC5843_CONFIG_REG_A);
 
-/*
- * From Datasheet
- *	Nominal gain settings
- * Value	| Sensor Input Field Range(Ga)	| Gain(counts/ milli-gauss)
- *0		|(+-)0.7			|1620
- *1		|(+-)1.0			|1300
- *2		|(+-)1.5			|970
- *3		|(+-)2.0			|780
- *4		|(+-)3.2			|530
- *5		|(+-)3.8			|460
- *6		|(+-)4.5			|390
- *7		|(+-)6.5			|280
- */
 static ssize_t show_range(struct device *dev,
 				struct device_attribute *attr,
 				char *buf)
@@ -440,6 +441,7 @@ static ssize_t set_range(struct device *dev,
 	struct hmc5843_data *data = iio_priv(indio_dev);
 	unsigned long range = 0;
 	int error;
+
 	mutex_lock(&data->lock);
 	error = kstrtoul(buf, 10, &range);
 	if (error) {
@@ -461,8 +463,8 @@ static ssize_t set_range(struct device *dev,
 exit:
 	mutex_unlock(&data->lock);
 	return count;
-
 }
+
 static IIO_DEVICE_ATTR(in_magn_range,
 			S_IWUSR | S_IRUGO,
 			show_range,
@@ -537,7 +539,7 @@ static int hmc5843_detect(struct i2c_client *client,
 	return 0;
 }
 
-/* Called when we have found a new HMC5843. */
+/* Called when we have found a new HMC5843 */
 static void hmc5843_init_client(struct i2c_client *client)
 {
 	struct iio_dev *indio_dev = i2c_get_clientdata(client);
@@ -548,6 +550,7 @@ static void hmc5843_init_client(struct i2c_client *client)
 	hmc5843_configure(client, data->operating_mode);
 	i2c_smbus_write_byte_data(client, HMC5843_CONFIG_REG_B, data->range);
 	mutex_init(&data->lock);
+
 	pr_info("HMC5843 initialized\n");
 }
 
@@ -577,8 +580,6 @@ static int hmc5843_probe(struct i2c_client *client,
 	data->operating_mode = HMC5843_MODE_CONVERSION_CONTINUOUS;
 
 	i2c_set_clientdata(client, indio_dev);
-
-	/* Initialize the HMC5843 chip */
 	hmc5843_init_client(client);
 
 	indio_dev->info = &hmc5843_info;
@@ -587,10 +588,13 @@ static int hmc5843_probe(struct i2c_client *client,
 	indio_dev->num_channels = ARRAY_SIZE(hmc5843_channels);
 	indio_dev->dev.parent = &client->dev;
 	indio_dev->modes = INDIO_DIRECT_MODE;
+
 	err = iio_device_register(indio_dev);
 	if (err)
 		goto exit_free2;
+
 	return 0;
+
 exit_free2:
 	iio_device_free(indio_dev);
 exit:
-- 
1.7.5.4

[PATCH 7/8] iio: rename function/data to consistently start with hmc5843_

[Peter Meerwald]

From: Peter Meerwald <p.meerwald@bct-electronic.com>

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/staging/iio/magnetometer/hmc5843.c |   26 +++++++++++++-------------
 1 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
index 57ed182..2e1bc6c 100644
--- a/drivers/staging/iio/magnetometer/hmc5843.c
+++ b/drivers/staging/iio/magnetometer/hmc5843.c
@@ -115,7 +115,7 @@ static const int hmc5843_regval_to_nanoscale[] = {
  * 6		| (+-)4.5			| 390
  * 7		| (+-)6.5			| 280
  */
-static const int regval_to_input_field_mg[] = {
+static const int hmc5843_regval_to_input_field_mga[] = {
 	700,
 	1000,
 	1500,
@@ -138,7 +138,7 @@ static const int regval_to_input_field_mg[] = {
  * 6		| 50
  * 7		| Not used
  */
-static const char * const regval_to_samp_freq[] = {
+static const char * const hmc5843_regval_to_sample_freq[] = {
 	"0.5",
 	"1",
 	"2",
@@ -355,7 +355,7 @@ static s32 hmc5843_set_rate(struct i2c_client *client,
 static int hmc5843_check_sampling_frequency(struct hmc5843_data *data,
 						const char *buf)
 {
-	const char * const *samp_freq = regval_to_samp_freq;
+	const char * const *samp_freq = hmc5843_regval_to_sample_freq;
 	int i;
 
 	for (i = 0; i < HMC5843_RATE_NOT_USED; i++) {
@@ -367,7 +367,7 @@ static int hmc5843_check_sampling_frequency(struct hmc5843_data *data,
 	return -EINVAL;
 }
 
-static ssize_t set_sampling_frequency(struct device *dev,
+static ssize_t hmc5843_set_sampling_frequency(struct device *dev,
 					struct device_attribute *attr,
 					const char *buf, size_t count)
 {
@@ -397,7 +397,7 @@ exit:
 	return count;
 }
 
-static ssize_t show_sampling_frequency(struct device *dev,
+static ssize_t hmc5843_show_sampling_frequency(struct device *dev,
 			struct device_attribute *attr, char *buf)
 {
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
@@ -409,16 +409,16 @@ static ssize_t show_sampling_frequency(struct device *dev,
 	if (rate < 0)
 		return rate;
 	rate = (rate & HMC5843_RATE_BITMASK) >> HMC5843_RATE_OFFSET;
-	return sprintf(buf, "%s\n", regval_to_samp_freq[rate]);
+	return sprintf(buf, "%s\n", hmc5843_regval_to_sample_freq[rate]);
 }
 
 static IIO_DEVICE_ATTR(sampling_frequency,
 			S_IWUSR | S_IRUGO,
-			show_sampling_frequency,
-			set_sampling_frequency,
+			hmc5843_show_sampling_frequency,
+			hmc5843_set_sampling_frequency,
 			HMC5843_CONFIG_REG_A);
 
-static ssize_t show_range(struct device *dev,
+static ssize_t hmc5843_show_range_gain(struct device *dev,
 				struct device_attribute *attr,
 				char *buf)
 {
@@ -427,10 +427,10 @@ static ssize_t show_range(struct device *dev,
 	struct hmc5843_data *data = iio_priv(indio_dev);
 
 	range = data->range;
-	return sprintf(buf, "%d\n", regval_to_input_field_mg[range]);
+	return sprintf(buf, "%d\n", hmc5843_regval_to_input_field_mga[range]);
 }
 
-static ssize_t set_range(struct device *dev,
+static ssize_t hmc5843_set_range_gain(struct device *dev,
 			struct device_attribute *attr,
 			const char *buf,
 			size_t count)
@@ -467,8 +467,8 @@ exit:
 
 static IIO_DEVICE_ATTR(in_magn_range,
 			S_IWUSR | S_IRUGO,
-			show_range,
-			set_range,
+			hmc5843_show_range_gain,
+			hmc5843_set_range_gain,
 			HMC5843_CONFIG_REG_B);
 
 static int hmc5843_read_raw(struct iio_dev *indio_dev,
-- 
1.7.5.4

[PATCH 8/8] iio: add support for hmc5883/hmc5883l to hmc5843 magnetometer driver

[Peter Meerwald]

From: Peter Meerwald <p.meerwald@bct-electronic.com>

v2 addresses review comments:
* fixes and cleanups have been split out (Jonathan Cameron)
* constants are generally prefixed HMC5843_, except when related
  specifically to hmc5883 (Jonathan Cameron)
* simplify code and avoid temp buffer in
  hmc5843_show_sampling_frequencies_available() (Lars-Peter Clausen)
* use sysfs_streq() instead of strncmp()/strlen() in
  hmc5843_check_sampling_frequency() (Lars-Peter Clausen)

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/staging/iio/magnetometer/Kconfig   |    8 +-
 drivers/staging/iio/magnetometer/hmc5843.c |  181 +++++++++++++++++++++++----
 2 files changed, 158 insertions(+), 31 deletions(-)

diff --git a/drivers/staging/iio/magnetometer/Kconfig b/drivers/staging/iio/magnetometer/Kconfig
index 722c4e1..b9d9325 100644
--- a/drivers/staging/iio/magnetometer/Kconfig
+++ b/drivers/staging/iio/magnetometer/Kconfig
@@ -15,13 +15,13 @@ config SENSORS_AK8975
 	  will be called ak8975.
 
 config SENSORS_HMC5843
-	tristate "Honeywell HMC5843 3-Axis Magnetometer"
+	tristate "Honeywell HMC5843/5883/5883L 3-Axis Magnetometer"
 	depends on I2C
 	help
-	  Say Y here to add support for the Honeywell HMC 5843 3-Axis
-	  Magnetometer (digital compass).
+	  Say Y here to add support for the Honeywell HMC5843, HMC5883 and
+	  HMC5883L 3-Axis Magnetometer (digital compass).
 
 	  To compile this driver as a module, choose M here: the module
-	  will be called hmc5843
+	  will be called hmc5843.
 
 endmenu
diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
index 2e1bc6c..8321a77 100644
--- a/drivers/staging/iio/magnetometer/hmc5843.c
+++ b/drivers/staging/iio/magnetometer/hmc5843.c
@@ -2,6 +2,8 @@
     Author: Shubhrajyoti Datta <shubhrajyoti@ti.com>
     Acknowledgement: Jonathan Cameron <jic23@cam.ac.uk> for valuable inputs.
 
+    Support for HMC5883 and HMC5883L by Peter Meerwald <pmeerw@pmeerw.net>.
+
     This program is free software; you can redistribute it and/or modify
     it under the terms of the GNU General Public License as published by
     the Free Software Foundation; either version 2 of the License, or
@@ -44,6 +46,11 @@
 #define HMC5843_ID_REG_B			0x0B
 #define HMC5843_ID_REG_C			0x0C
 
+enum hmc5843_ids {
+	HMC5843_ID,
+	HMC5883_ID,
+	HMC5883L_ID,
+};
 
 /*
  * Beware: identification of the HMC5883 is still "H43";
@@ -103,8 +110,16 @@ static const int hmc5843_regval_to_nanoscale[] = {
 	6173, 7692, 10309, 12821, 18868, 21739, 25641, 35714
 };
 
+static const int hmc5883_regval_to_nanoscale[] = {
+	7812, 9766, 13021, 16287, 24096, 27701, 32573, 45662
+};
+
+static const int hmc5883l_regval_to_nanoscale[] = {
+	7299, 9174, 12195, 15152, 22727, 25641, 30303, 43478
+};
+
 /*
- * From the datasheet:
+ * From the HMC5843 datasheet:
  * Value	| Sensor input field range (Ga)	| Gain (counts/milli-Gauss)
  * 0		| (+-)0.7			| 1620
  * 1		| (+-)1.0			| 1300
@@ -114,6 +129,28 @@ static const int hmc5843_regval_to_nanoscale[] = {
  * 5		| (+-)3.8			| 460
  * 6		| (+-)4.5			| 390
  * 7		| (+-)6.5			| 280
+ *
+ * From the HMC5883 datasheet:
+ * Value	| Recommended sensor field range (Ga)	| Gain (counts/Gauss)
+ * 0		| (+-)0.9				| 1280
+ * 1		| (+-)1.2				| 1024
+ * 2		| (+-)1.9				| 768
+ * 3		| (+-)2.5				| 614
+ * 4		| (+-)4.0				| 415
+ * 5		| (+-)4.6				| 361
+ * 6		| (+-)5.5				| 307
+ * 7		| (+-)7.9				| 219
+ *
+ * From the HMC5883L datasheet:
+ * Value	| Recommended sensor field range (Ga)	| Gain (LSB/Gauss)
+ * 0		| (+-)0.88				| 1370
+ * 1		| (+-)1.3				| 1090
+ * 2		| (+-)1.9				| 820
+ * 3		| (+-)2.5				| 660
+ * 4		| (+-)4.0				| 440
+ * 5		| (+-)4.7				| 390
+ * 6		| (+-)5.6				| 330
+ * 7		| (+-)8.1				| 230
  */
 static const int hmc5843_regval_to_input_field_mga[] = {
 	700,
@@ -126,17 +163,41 @@ static const int hmc5843_regval_to_input_field_mga[] = {
 	6500
 };
 
+static const int hmc5883_regval_to_input_field_mga[] = {
+	900,
+	1200,
+	1900,
+	2500,
+	4000,
+	4600,
+	5500,
+	7900
+};
+
+static const int hmc5883l_regval_to_input_field_mga[] = {
+	880,
+	1300,
+	1900,
+	2500,
+	4000,
+	4700,
+	5600,
+	8100
+};
+
+
 /*
  * From the datasheet:
- * Value	| Data output rate (Hz)
- * 0		| 0.5
- * 1		| 1
- * 2		| 2
- * 3		| 5
- * 4		| 10 (default)
- * 5		| 20
- * 6		| 50
- * 7		| Not used
+ * Value	| HMC5843		| HMC5883/HMC5883L
+ *		| Data output rate (Hz)	| Data output rate (Hz)
+ * 0		| 0.5			| 0.75
+ * 1		| 1			| 1.5
+ * 2		| 2			| 3
+ * 3		| 5			| 7.5
+ * 4		| 10 (default)		| 15
+ * 5		| 20			| 30
+ * 6		| 50			| 75
+ * 7		| Not used		| Not used
  */
 static const char * const hmc5843_regval_to_sample_freq[] = {
 	"0.5",
@@ -148,6 +209,16 @@ static const char * const hmc5843_regval_to_sample_freq[] = {
 	"50",
 };
 
+static const char * const hmc5883_regval_to_sample_freq[] = {
+	"0.75",
+	"1.5",
+	"3",
+	"7.5",
+	"15",
+	"30",
+	"75",
+};
+
 /* Addresses to scan: 0x1E */
 static const unsigned short normal_i2c[] = { HMC5843_I2C_ADDRESS,
 					     I2C_CLIENT_END };
@@ -159,6 +230,9 @@ struct hmc5843_data {
 	u8 meas_conf;
 	u8 operating_mode;
 	u8 range;
+	const char * const *regval_to_sample_freq;
+	const int *regval_to_input_field_mga;
+	const int *regval_to_nanoscale;
 };
 
 /* The lower two bits contain the current conversion mode */
@@ -334,7 +408,25 @@ static IIO_DEVICE_ATTR(meas_conf,
 			hmc5843_set_measurement_configuration,
 			0);
 
-static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("0.5 1 2 5 10 20 50");
+static ssize_t hmc5843_show_sampling_frequencies_available(struct device *dev,
+						struct device_attribute *attr,
+						char *buf)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct hmc5843_data *data = iio_priv(indio_dev);
+	ssize_t total_n = 0;
+	int i;
+
+	for (i = 0; i < HMC5843_RATE_NOT_USED; i++) {
+		ssize_t n = sprintf(buf, "%s ", data->regval_to_sample_freq[i]);
+		buf += n;
+		total_n += n;
+	}
+
+	return total_n;
+}
+
+static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(hmc5843_show_sampling_frequencies_available);
 
 static s32 hmc5843_set_rate(struct i2c_client *client,
 				u8 rate)
@@ -343,24 +435,23 @@ static s32 hmc5843_set_rate(struct i2c_client *client,
 	struct hmc5843_data *data = iio_priv(indio_dev);
 	u8 reg_val;
 
-	reg_val = (data->meas_conf) |  (rate << HMC5843_RATE_OFFSET);
 	if (rate >= HMC5843_RATE_NOT_USED) {
 		dev_err(&client->dev,
 			"data output rate is not supported\n");
 		return -EINVAL;
 	}
+	reg_val = data->meas_conf | (rate << HMC5843_RATE_OFFSET);
 	return i2c_smbus_write_byte_data(client, HMC5843_CONFIG_REG_A, reg_val);
 }
 
 static int hmc5843_check_sampling_frequency(struct hmc5843_data *data,
 						const char *buf)
 {
-	const char * const *samp_freq = hmc5843_regval_to_sample_freq;
+	const char * const *samp_freq = data->regval_to_sample_freq;
 	int i;
 
 	for (i = 0; i < HMC5843_RATE_NOT_USED; i++) {
-		if (strncmp(buf, samp_freq[i],
-			strlen(samp_freq[i])) == 0)
+		if (sysfs_streq(buf, samp_freq[i]))
 			return i;
 	}
 
@@ -403,13 +494,14 @@ static ssize_t hmc5843_show_sampling_frequency(struct device *dev,
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct i2c_client *client = to_i2c_client(indio_dev->dev.parent);
 	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
+	struct hmc5843_data *data = iio_priv(indio_dev);
 	s32 rate;
 
 	rate = i2c_smbus_read_byte_data(client, this_attr->address);
 	if (rate < 0)
 		return rate;
 	rate = (rate & HMC5843_RATE_BITMASK) >> HMC5843_RATE_OFFSET;
-	return sprintf(buf, "%s\n", hmc5843_regval_to_sample_freq[rate]);
+	return sprintf(buf, "%s\n", data->regval_to_sample_freq[rate]);
 }
 
 static IIO_DEVICE_ATTR(sampling_frequency,
@@ -427,7 +519,7 @@ static ssize_t hmc5843_show_range_gain(struct device *dev,
 	struct hmc5843_data *data = iio_priv(indio_dev);
 
 	range = data->range;
-	return sprintf(buf, "%d\n", hmc5843_regval_to_input_field_mga[range]);
+	return sprintf(buf, "%d\n", data->regval_to_input_field_mga[range]);
 }
 
 static ssize_t hmc5843_set_range_gain(struct device *dev,
@@ -485,7 +577,7 @@ static int hmc5843_read_raw(struct iio_dev *indio_dev,
 						val);
 	case IIO_CHAN_INFO_SCALE:
 		*val = 0;
-		*val2 = hmc5843_regval_to_nanoscale[data->range];
+		*val2 = data->regval_to_nanoscale[data->range];
 		return IIO_VAL_INT_PLUS_NANO;
 	};
 	return -EINVAL;
@@ -507,12 +599,19 @@ static const struct iio_chan_spec hmc5843_channels[] = {
 	HMC5843_CHANNEL(Z, HMC5843_DATA_OUT_Z_MSB_REG),
 };
 
+static const struct iio_chan_spec hmc5883_channels[] = {
+	HMC5843_CHANNEL(X, HMC5843_DATA_OUT_X_MSB_REG),
+	HMC5843_CHANNEL(Y, HMC5883_DATA_OUT_Y_MSB_REG),
+	HMC5843_CHANNEL(Z, HMC5883_DATA_OUT_Z_MSB_REG),
+};
+
+
 static struct attribute *hmc5843_attributes[] = {
 	&iio_dev_attr_meas_conf.dev_attr.attr,
 	&iio_dev_attr_operating_mode.dev_attr.attr,
 	&iio_dev_attr_sampling_frequency.dev_attr.attr,
 	&iio_dev_attr_in_magn_range.dev_attr.attr,
-	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
+	&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
 	NULL
 };
 
@@ -539,19 +638,47 @@ static int hmc5843_detect(struct i2c_client *client,
 	return 0;
 }
 
-/* Called when we have found a new HMC5843 */
-static void hmc5843_init_client(struct i2c_client *client)
+/* Called when we have found a new HMC58X3 */
+static void hmc5843_init_client(struct i2c_client *client,
+				const struct i2c_device_id *id)
 {
 	struct iio_dev *indio_dev = i2c_get_clientdata(client);
 	struct hmc5843_data *data = iio_priv(indio_dev);
 
+	switch (id->driver_data) {
+	case HMC5843_ID:
+		data->regval_to_sample_freq = hmc5843_regval_to_sample_freq;
+		data->regval_to_input_field_mga =
+			hmc5843_regval_to_input_field_mga;
+		data->regval_to_nanoscale = hmc5843_regval_to_nanoscale;
+		indio_dev->channels = hmc5843_channels;
+		indio_dev->num_channels = ARRAY_SIZE(hmc5843_channels);
+		break;
+	case HMC5883_ID:
+		data->regval_to_sample_freq = hmc5883_regval_to_sample_freq;
+		data->regval_to_input_field_mga =
+			hmc5883_regval_to_input_field_mga;
+		data->regval_to_nanoscale = hmc5883_regval_to_nanoscale;
+		indio_dev->channels = hmc5883_channels;
+		indio_dev->num_channels = ARRAY_SIZE(hmc5883_channels);
+		break;
+	case HMC5883L_ID:
+		data->regval_to_sample_freq = hmc5883_regval_to_sample_freq;
+		data->regval_to_input_field_mga =
+			hmc5883l_regval_to_input_field_mga;
+		data->regval_to_nanoscale = hmc5883l_regval_to_nanoscale;
+		indio_dev->channels = hmc5883_channels;
+		indio_dev->num_channels = ARRAY_SIZE(hmc5883_channels);
+		break;
+	}
+
 	hmc5843_set_meas_conf(client, data->meas_conf);
 	hmc5843_set_rate(client, data->rate);
 	hmc5843_configure(client, data->operating_mode);
 	i2c_smbus_write_byte_data(client, HMC5843_CONFIG_REG_B, data->range);
 	mutex_init(&data->lock);
 
-	pr_info("HMC5843 initialized\n");
+	pr_info("%s initialized\n", id->name);
 }
 
 static const struct iio_info hmc5843_info = {
@@ -580,12 +707,10 @@ static int hmc5843_probe(struct i2c_client *client,
 	data->operating_mode = HMC5843_MODE_CONVERSION_CONTINUOUS;
 
 	i2c_set_clientdata(client, indio_dev);
-	hmc5843_init_client(client);
+	hmc5843_init_client(client, id);
 
 	indio_dev->info = &hmc5843_info;
 	indio_dev->name = id->name;
-	indio_dev->channels = hmc5843_channels;
-	indio_dev->num_channels = ARRAY_SIZE(hmc5843_channels);
 	indio_dev->dev.parent = &client->dev;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
@@ -636,7 +761,9 @@ static SIMPLE_DEV_PM_OPS(hmc5843_pm_ops, hmc5843_suspend, hmc5843_resume);
 #endif
 
 static const struct i2c_device_id hmc5843_id[] = {
-	{ "hmc5843", 0 },
+	{ "hmc5843", HMC5843_ID },
+	{ "hmc5883", HMC5883_ID },
+	{ "hmc5883l", HMC5883L_ID },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, hmc5843_id);
@@ -655,5 +782,5 @@ static struct i2c_driver hmc5843_driver = {
 module_i2c_driver(hmc5843_driver);
 
 MODULE_AUTHOR("Shubhrajyoti Datta <shubhrajyoti@ti.com");
-MODULE_DESCRIPTION("HMC5843 driver");
+MODULE_DESCRIPTION("HMC5843/5883/5883L driver");
 MODULE_LICENSE("GPL");
-- 
1.7.5.4

Re: iio: v2, add support for HMC5883/HMC5883L to HMC5843 driver

[Shubhrajyoti Datta]

Hi Peter,

On Wed, May 9, 2012 at 3:49 AM, Peter Meerwald <pmeerw@pmeerw.net> wrot=
e:
> here is my updated patch series to extend the HMC5843 magnetometer
> driver to support the HMC5883/HMC5883L devices
>
> patch 1 fixes access issues to the hmc5843 private data,
> patches 2 to 7 are preparatory patches,
> patch 8 actually brings in the support for the new devices
>
> the timeout in the read loop has not yet been implemented (suggestion=
s by
> Lars-Peter Clausen)
>
> the driver has a severe limitation: X/Y/Z must ALL be read to allow n=
ew
> data to become available (the device goes into a locked state until a=
ll
> are read)

Thats a limitation comming from the data sheet right?

Are you suggesting a read all and report only requested?
to work round that.

>
> --
> 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 =A0http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/8] iio: fix access to hmc5843 private data

[Shubhrajyoti Datta]

Hi Peter,
A minor observation your from and signoff are a little different was
it intended?

On Wed, May 9, 2012 at 3:49 AM, Peter Meerwald <pmeerw@pmeerw.net> wrot=
e:
> From: Peter Meerwald <p.meerwald@bct-electronic.com>
>
> i2c_get_clientdata(client) points to iio_dev, not hmc5843_data; fixes
> issue similar to 62d2feb9803f18c4e3c8a1a2c7e30a54df8a1d72
>
Looks good to me ack.

> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
> ---
> =A0drivers/staging/iio/magnetometer/hmc5843.c | =A0 14 +++++++++-----
> =A01 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/sta=
ging/iio/magnetometer/hmc5843.c
> index 3ec6518..9725cf8 100644
> --- a/drivers/staging/iio/magnetometer/hmc5843.c
> +++ b/drivers/staging/iio/magnetometer/hmc5843.c
> @@ -250,7 +250,8 @@ static IIO_DEVICE_ATTR(operating_mode,
> =A0static s32 hmc5843_set_meas_conf(struct i2c_client *client,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0u8 meas_conf)
> =A0{
> - =A0 =A0 =A0 struct hmc5843_data *data =3D i2c_get_clientdata(client=
);
> + =A0 =A0 =A0 struct iio_dev *indio_dev =3D i2c_get_clientdata(client=
);
> + =A0 =A0 =A0 struct hmc5843_data *data =3D iio_priv(indio_dev);
> =A0 =A0 =A0 =A0u8 reg_val;
> =A0 =A0 =A0 =A0reg_val =3D (meas_conf & MEAS_CONF_MASK) | =A0(data->r=
ate << RATE_OFFSET);
> =A0 =A0 =A0 =A0return i2c_smbus_write_byte_data(client, HMC5843_CONFI=
G_REG_A, reg_val);
> @@ -272,7 +273,7 @@ static ssize_t hmc5843_set_measurement_configurat=
ion(struct device *dev,
> =A0{
> =A0 =A0 =A0 =A0struct iio_dev *indio_dev =3D dev_get_drvdata(dev);
> =A0 =A0 =A0 =A0struct i2c_client *client =3D to_i2c_client(indio_dev-=
>dev.parent);
> - =A0 =A0 =A0 struct hmc5843_data *data =3D i2c_get_clientdata(client=
);
> + =A0 =A0 =A0 struct hmc5843_data *data =3D iio_priv(indio_dev);
> =A0 =A0 =A0 =A0unsigned long meas_conf =3D 0;
> =A0 =A0 =A0 =A0int error =3D strict_strtoul(buf, 10, &meas_conf);
> =A0 =A0 =A0 =A0if (error)
> @@ -314,7 +315,8 @@ static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("0.5 1 2 5 =
10 20 50");
> =A0static s32 hmc5843_set_rate(struct i2c_client *client,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0u8 rat=
e)
> =A0{
> - =A0 =A0 =A0 struct hmc5843_data *data =3D i2c_get_clientdata(client=
);
> + =A0 =A0 =A0 struct iio_dev *indio_dev =3D i2c_get_clientdata(client=
);
> + =A0 =A0 =A0 struct hmc5843_data *data =3D iio_priv(indio_dev);
> =A0 =A0 =A0 =A0u8 reg_val;
>
> =A0 =A0 =A0 =A0reg_val =3D (data->meas_conf) | =A0(rate << RATE_OFFSE=
T);
> @@ -600,8 +602,10 @@ static int hmc5843_suspend(struct device *dev)
>
> =A0static int hmc5843_resume(struct device *dev)
> =A0{
> - =A0 =A0 =A0 struct hmc5843_data *data =3D i2c_get_clientdata(to_i2c=
_client(dev));
> - =A0 =A0 =A0 hmc5843_configure(to_i2c_client(dev), data->operating_m=
ode);
> + =A0 =A0 =A0 struct i2c_client *client =3D to_i2c_client(dev);
> + =A0 =A0 =A0 struct iio_dev *indio_dev =3D i2c_get_clientdata(client=
);
> + =A0 =A0 =A0 struct hmc5843_data *data =3D iio_priv(indio_dev);
> + =A0 =A0 =A0 hmc5843_configure(client, data->operating_mode);
> =A0 =A0 =A0 =A0return 0;
> =A0}
>
> --
> 1.7.5.4
>
> --
> 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 =A0http://vger.kernel.org/majordomo-info.html

Re: iio: v2, add support for HMC5883/HMC5883L to HMC5843 driver

[Peter Meerwald]

Hello,

> > the driver has a severe limitation: X/Y/Z must ALL be read to allow new
> > data to become available (the device goes into a locked state until all
> > are read)

> Thats a limitation comming from the data sheet right?

yes, it affects all devices (hmc5843/5883/5883l)

> Are you suggesting a read all and report only requested?
> to work round that.

probably yes

I brought it up to see if there are ideas for a solution; maybe the issue 
is more common and this should be solved at iio core level?

with individual reads for X/Y/Z there is no guarantee that all values come 
from the same measurement cycle ('atomic read')

the hmc58x3 'solves' this with the lock logic and introduces another issue

maybe iio should only allow reads to all data elements at once?

regards, p.

-- 

Peter Meerwald
+43-664-2444418 (mobile)

Re: iio: v2, add support for HMC5883/HMC5883L to HMC5843 driver

[Jonathan Cameron]

On 5/9/2012 2:33 PM, Peter Meerwald wrote:
> Hello,
>
>>> the driver has a severe limitation: X/Y/Z must ALL be read to allow new
>>> data to become available (the device goes into a locked state until all
>>> are read)
>> Thats a limitation comming from the data sheet right?
> yes, it affects all devices (hmc5843/5883/5883l)
>
>> Are you suggesting a read all and report only requested?
>> to work round that.
> probably yes
>
> I brought it up to see if there are ideas for a solution; maybe the issue
> is more common and this should be solved at iio core level?
>
> with individual reads for X/Y/Z there is no guarantee that all values come
> from the same measurement cycle ('atomic read')
>
> the hmc58x3 'solves' this with the lock logic and introduces another issue
>
> maybe iio should only allow reads to all data elements at once?
If you need to do that, use buffered access rather than sysfs.  Anything 
else
makes for fiddly and hard to generalize interfaces I'm afraid.

It is indeed a common situation.  Actually I much prefer this to the 
alternative
of getting an inconsistent set depending on precise read times.

Jonathan

Re: [PATCH 1/8] iio: fix access to hmc5843 private data

[Jonathan Cameron]

On 5/8/2012 11:19 PM, Peter Meerwald wrote:
> From: Peter Meerwald<p.meerwald@bct-electronic.com>
>
> i2c_get_clientdata(client) points to iio_dev, not hmc5843_data; fixes
> issue similar to 62d2feb9803f18c4e3c8a1a2c7e30a54df8a1d72
>
Hmm.. Really hope there aren't many more of these hiding in the tree.
(All my fault in the first place ;(
> Signed-off-by: Peter Meerwald<pmeerw@pmeerw.net>
Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
>   drivers/staging/iio/magnetometer/hmc5843.c |   14 +++++++++-----
>   1 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
> index 3ec6518..9725cf8 100644
> --- a/drivers/staging/iio/magnetometer/hmc5843.c
> +++ b/drivers/staging/iio/magnetometer/hmc5843.c
> @@ -250,7 +250,8 @@ static IIO_DEVICE_ATTR(operating_mode,
>   static s32 hmc5843_set_meas_conf(struct i2c_client *client,
>   				      u8 meas_conf)
>   {
> -	struct hmc5843_data *data = i2c_get_clientdata(client);
> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +	struct hmc5843_data *data = iio_priv(indio_dev);
>   	u8 reg_val;
>   	reg_val = (meas_conf&  MEAS_CONF_MASK) |  (data->rate<<  RATE_OFFSET);
>   	return i2c_smbus_write_byte_data(client, HMC5843_CONFIG_REG_A, reg_val);
> @@ -272,7 +273,7 @@ static ssize_t hmc5843_set_measurement_configuration(struct device *dev,
>   {
>   	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>   	struct i2c_client *client = to_i2c_client(indio_dev->dev.parent);
> -	struct hmc5843_data *data = i2c_get_clientdata(client);
> +	struct hmc5843_data *data = iio_priv(indio_dev);
>   	unsigned long meas_conf = 0;
>   	int error = strict_strtoul(buf, 10,&meas_conf);
>   	if (error)
> @@ -314,7 +315,8 @@ static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("0.5 1 2 5 10 20 50");
>   static s32 hmc5843_set_rate(struct i2c_client *client,
>   				u8 rate)
>   {
> -	struct hmc5843_data *data = i2c_get_clientdata(client);
> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +	struct hmc5843_data *data = iio_priv(indio_dev);
>   	u8 reg_val;
>
>   	reg_val = (data->meas_conf) |  (rate<<  RATE_OFFSET);
> @@ -600,8 +602,10 @@ static int hmc5843_suspend(struct device *dev)
>
>   static int hmc5843_resume(struct device *dev)
>   {
> -	struct hmc5843_data *data = i2c_get_clientdata(to_i2c_client(dev));
> -	hmc5843_configure(to_i2c_client(dev), data->operating_mode);
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +	struct hmc5843_data *data = iio_priv(indio_dev);
> +	hmc5843_configure(client, data->operating_mode);
>   	return 0;
>   }
>

Re: [PATCH 2/8] iio: change strict_strtoul() to kstrtoul() in hmc5843

[Jonathan Cameron]

On 5/8/2012 11:20 PM, Peter Meerwald wrote:
> From: Peter Meerwald<p.meerwald@bct-electronic.com>
>
> Signed-off-by: Peter Meerwald<pmeerw@pmeerw.net>
Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
>   drivers/staging/iio/magnetometer/hmc5843.c |    6 +++---
>   1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
> index 9725cf8..018d3d8 100644
> --- a/drivers/staging/iio/magnetometer/hmc5843.c
> +++ b/drivers/staging/iio/magnetometer/hmc5843.c
> @@ -202,7 +202,7 @@ static ssize_t hmc5843_set_operating_mode(struct device *dev,
>   	s32 status;
>   	int error;
>   	mutex_lock(&data->lock);
> -	error = strict_strtoul(buf, 10,&operating_mode);
> +	error = kstrtoul(buf, 10,&operating_mode);
>   	if (error) {
>   		count = error;
>   		goto exit;
> @@ -275,7 +275,7 @@ static ssize_t hmc5843_set_measurement_configuration(struct device *dev,
>   	struct i2c_client *client = to_i2c_client(indio_dev->dev.parent);
>   	struct hmc5843_data *data = iio_priv(indio_dev);
>   	unsigned long meas_conf = 0;
> -	int error = strict_strtoul(buf, 10,&meas_conf);
> +	int error = kstrtoul(buf, 10,&meas_conf);
>   	if (error)
>   		return error;
>   	mutex_lock(&data->lock);
> @@ -425,7 +425,7 @@ static ssize_t set_range(struct device *dev,
>   	unsigned long range = 0;
>   	int error;
>   	mutex_lock(&data->lock);
> -	error = strict_strtoul(buf, 10,&range);
> +	error = kstrtoul(buf, 10,&range);
>   	if (error) {
>   		count = error;
>   		goto exit;

Re: [PATCH 3/8] iio: rename and prefix CONSTANTs to distinguish between HMC5843 and HMC5883

[Jonathan Cameron]

On 5/8/2012 11:20 PM, Peter Meerwald wrote:
> From: Peter Meerwald<p.meerwald@bct-electronic.com>
>
All appears sane.
> Signed-off-by: Peter Meerwald<pmeerw@pmeerw.net>
Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
>   drivers/staging/iio/magnetometer/hmc5843.c |  128 +++++++++++++++------------
>   1 files changed, 71 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
> index 018d3d8..1318f7d 100644
> --- a/drivers/staging/iio/magnetometer/hmc5843.c
> +++ b/drivers/staging/iio/magnetometer/hmc5843.c
> @@ -25,8 +25,6 @@
>   #include<linux/iio/iio.h>
>   #include<linux/iio/sysfs.h>
>
> -#define HMC5843_I2C_ADDRESS			0x1E
> -
>   #define HMC5843_CONFIG_REG_A			0x00
>   #define HMC5843_CONFIG_REG_B			0x01
>   #define HMC5843_MODE_REG			0x02
> @@ -36,65 +34,80 @@
>   #define HMC5843_DATA_OUT_Y_LSB_REG		0x06
>   #define HMC5843_DATA_OUT_Z_MSB_REG		0x07
>   #define HMC5843_DATA_OUT_Z_LSB_REG		0x08
> +/* Beware: Y and Z are exchanged on HMC5883 */
> +#define HMC5883_DATA_OUT_Z_MSB_REG		0x05
> +#define HMC5883_DATA_OUT_Z_LSB_REG		0x06
> +#define HMC5883_DATA_OUT_Y_MSB_REG		0x07
> +#define HMC5883_DATA_OUT_Y_LSB_REG		0x08
>   #define HMC5843_STATUS_REG			0x09
>   #define HMC5843_ID_REG_A			0x0A
>   #define HMC5843_ID_REG_B			0x0B
>   #define HMC5843_ID_REG_C			0x0C
>
> +
> +/*
> + * Beware: identification of the HMC5883 is still "H43";
> + * I2C address is also unchanged
> + */
>   #define HMC5843_ID_REG_LENGTH			0x03
>   #define HMC5843_ID_STRING			"H43"
> +#define HMC5843_I2C_ADDRESS			0x1E
>
>   /*
> - * Range settings in  (+-)Ga
> - * */
> -#define RANGE_GAIN_OFFSET			0x05
> -
> -#define	RANGE_0_7				0x00
> -#define	RANGE_1_0				0x01 /* default */
> -#define	RANGE_1_5				0x02
> -#define	RANGE_2_0				0x03
> -#define	RANGE_3_2				0x04
> -#define	RANGE_3_8				0x05
> -#define	RANGE_4_5				0x06
> -#define	RANGE_6_5				0x07 /* Not recommended */
> + * Range gain settings in (+-)Ga
> + * Beware: HMC5843 and HMC5883 have different recommended sensor field
> + * ranges; default corresponds to +-1.0 Ga and +-1.3 Ga, respectively
> + */
> +#define HMC5843_RANGE_GAIN_OFFSET		0x05
> +#define HMC5843_RANGE_GAIN_DEFAULT		0x01
> +#define HMC5843_RANGE_GAIN_MAX			0x07
>
>   /*
>    * Device status
>    */
> -#define	DATA_READY				0x01
> -#define	DATA_OUTPUT_LOCK			0x02
> -#define	VOLTAGE_REGULATOR_ENABLED		0x04
> +#define HMC5843_DATA_READY			0x01
> +#define HMC5843_DATA_OUTPUT_LOCK		0x02
> +/* Does not exist on HMC5883, not used */
> +#define HMC5843_VOLTAGE_REGULATOR_ENABLED	0x04
>
>   /*
>    * Mode register configuration
>    */
> -#define	MODE_CONVERSION_CONTINUOUS		0x00
> -#define	MODE_CONVERSION_SINGLE			0x01
> -#define	MODE_IDLE				0x02
> -#define	MODE_SLEEP				0x03
> -
> -/* Minimum Data Output Rate in 1/10 Hz  */
> -#define RATE_OFFSET				0x02
> -#define RATE_BITMASK				0x1C
> -#define	RATE_5					0x00
> -#define	RATE_10					0x01
> -#define	RATE_20					0x02
> -#define	RATE_50					0x03
> -#define	RATE_100				0x04
> -#define	RATE_200				0x05
> -#define	RATE_500				0x06
> -#define	RATE_NOT_USED				0x07
> +#define HMC5843_MODE_CONVERSION_CONTINUOUS	0x00
> +#define HMC5843_MODE_CONVERSION_SINGLE		0x01
> +#define HMC5843_MODE_IDLE			0x02
> +#define HMC5843_MODE_SLEEP			0x03
> +#define HMC5843_MODE_MASK			0x03
> +
> +/*
> + * HMC5843: Minimum data output rate
> + * HMC5883: Typical data output rate
> + */
> +#define HMC5843_RATE_OFFSET			0x02
> +#define HMC5843_RATE_BITMASK			0x1C
> +#define RATE_5					0x00
> +#define RATE_10					0x01
> +#define RATE_20					0x02
> +#define RATE_50					0x03
> +#define RATE_100				0x04
> +#define RATE_200				0x05
> +#define RATE_500				0x06
> +
> +#define HMC5843_RATE_NOT_USED			0x07
>
>   /*
> - * Device Configuration
> + * Device measurement configuration
>    */
> -#define	CONF_NORMAL				0x00
> -#define	CONF_POSITIVE_BIAS			0x01
> -#define	CONF_NEGATIVE_BIAS			0x02
> -#define	CONF_NOT_USED				0x03
> -#define	MEAS_CONF_MASK				0x03
> +#define HMC5843_MEAS_CONF_NORMAL		0x00
> +#define HMC5843_MEAS_CONF_POSITIVE_BIAS		0x01
> +#define HMC5843_MEAS_CONF_NEGATIVE_BIAS		0x02
> +#define HMC5843_MEAS_CONF_NOT_USED		0x03
> +#define HMC5843_MEAS_CONF_MASK			0x03
>
> -static int hmc5843_regval_to_nanoscale[] = {
> +/*
> + * Scaling factors: 10000000/Gain
> + */
> +static const int hmc5843_regval_to_nanoscale[] = {
>   	6173, 7692, 10309, 12821, 18868, 21739, 25641, 35714
>   };
>
> @@ -120,7 +133,7 @@ static const char * const regval_to_samp_freq[] = {
>
>   /* Addresses to scan: 0x1E */
>   static const unsigned short normal_i2c[] = { HMC5843_I2C_ADDRESS,
> -							I2C_CLIENT_END };
> +					     I2C_CLIENT_END };
>
>   /* Each client has this additional data */
>   struct hmc5843_data {
> @@ -139,7 +152,7 @@ static s32 hmc5843_configure(struct i2c_client *client,
>   	/* The lower two bits contain the current conversion mode */
>   	return i2c_smbus_write_byte_data(client,
>   					HMC5843_MODE_REG,
> -					(operating_mode&  0x03));
> +					operating_mode&  HMC5843_MODE_MASK);
>   }
>
>   /* Return the measurement value from the specified channel */
> @@ -153,7 +166,7 @@ static int hmc5843_read_measurement(struct iio_dev *indio_dev,
>
>   	mutex_lock(&data->lock);
>   	result = i2c_smbus_read_byte_data(client, HMC5843_STATUS_REG);
> -	while (!(result&  DATA_READY))
> +	while (!(result&  HMC5843_DATA_READY))
>   		result = i2c_smbus_read_byte_data(client, HMC5843_STATUS_REG);
>
>   	result = i2c_smbus_read_word_data(client, address);
> @@ -208,7 +221,7 @@ static ssize_t hmc5843_set_operating_mode(struct device *dev,
>   		goto exit;
>   	}
>   	dev_dbg(dev, "set Conversion mode to %lu\n", operating_mode);
> -	if (operating_mode>  MODE_SLEEP) {
> +	if (operating_mode>  HMC5843_MODE_SLEEP) {
>   		count = -EINVAL;
>   		goto exit;
>   	}
> @@ -253,7 +266,8 @@ static s32 hmc5843_set_meas_conf(struct i2c_client *client,
>   	struct iio_dev *indio_dev = i2c_get_clientdata(client);
>   	struct hmc5843_data *data = iio_priv(indio_dev);
>   	u8 reg_val;
> -	reg_val = (meas_conf&  MEAS_CONF_MASK) |  (data->rate<<  RATE_OFFSET);
> +	reg_val = (meas_conf&  HMC5843_MEAS_CONF_MASK) |
> +		(data->rate<<  HMC5843_RATE_OFFSET);
>   	return i2c_smbus_write_byte_data(client, HMC5843_CONFIG_REG_A, reg_val);
>   }
>
> @@ -319,8 +333,8 @@ static s32 hmc5843_set_rate(struct i2c_client *client,
>   	struct hmc5843_data *data = iio_priv(indio_dev);
>   	u8 reg_val;
>
> -	reg_val = (data->meas_conf) |  (rate<<  RATE_OFFSET);
> -	if (rate>= RATE_NOT_USED) {
> +	reg_val = (data->meas_conf) |  (rate<<  HMC5843_RATE_OFFSET);
> +	if (rate>= HMC5843_RATE_NOT_USED) {
>   		dev_err(&client->dev,
>   			"This data output rate is not supported\n");
>   		return -EINVAL;
> @@ -379,7 +393,7 @@ static ssize_t show_sampling_frequency(struct device *dev,
>   	rate = i2c_smbus_read_byte_data(client,  this_attr->address);
>   	if (rate<  0)
>   		return rate;
> -	rate = (rate&  RATE_BITMASK)>>  RATE_OFFSET;
> +	rate = (rate&  HMC5843_RATE_BITMASK)>>  HMC5843_RATE_OFFSET;
>   	return sprintf(buf, "%s\n", regval_to_samp_freq[rate]);
>   }
>   static IIO_DEVICE_ATTR(sampling_frequency,
> @@ -432,13 +446,13 @@ static ssize_t set_range(struct device *dev,
>   	}
>   	dev_dbg(dev, "set range to %lu\n", range);
>
> -	if (range>  RANGE_6_5) {
> +	if (range>  HMC5843_RANGE_GAIN_MAX) {
>   		count = -EINVAL;
>   		goto exit;
>   	}
>
>   	data->range = range;
> -	range = range<<  RANGE_GAIN_OFFSET;
> +	range = range<<  HMC5843_RANGE_GAIN_OFFSET;
>   	if (i2c_smbus_write_byte_data(client, this_attr->address, range))
>   		count = -EINVAL;
>
> @@ -553,12 +567,12 @@ static int hmc5843_probe(struct i2c_client *client,
>   		err = -ENOMEM;
>   		goto exit;
>   	}
> -	data = iio_priv(indio_dev);
> -	/* default settings at probe */
>
> -	data->meas_conf = CONF_NORMAL;
> -	data->range = RANGE_1_0;
> -	data->operating_mode = MODE_CONVERSION_CONTINUOUS;
> +	/* default settings at probe */
> +	data = iio_priv(indio_dev);
> +	data->meas_conf = HMC5843_MEAS_CONF_NORMAL;
> +	data->range = HMC5843_RANGE_GAIN_DEFAULT;
> +	data->operating_mode = HMC5843_MODE_CONVERSION_CONTINUOUS;
>
>   	i2c_set_clientdata(client, indio_dev);
>
> @@ -587,7 +601,7 @@ static int hmc5843_remove(struct i2c_client *client)
>
>   	iio_device_unregister(indio_dev);
>   	 /*  sleep mode to save power */
> -	hmc5843_configure(client, MODE_SLEEP);
> +	hmc5843_configure(client, HMC5843_MODE_SLEEP);
>   	iio_device_free(indio_dev);
>
>   	return 0;
> @@ -596,7 +610,7 @@ static int hmc5843_remove(struct i2c_client *client)
>   #ifdef CONFIG_PM_SLEEP
>   static int hmc5843_suspend(struct device *dev)
>   {
> -	hmc5843_configure(to_i2c_client(dev), MODE_SLEEP);
> +	hmc5843_configure(to_i2c_client(dev), HMC5843_MODE_SLEEP);
>   	return 0;
>   }
>

Re: [PATCH 4/8] iio: rework sampling rate setting in hmc5843

[Jonathan Cameron]

On 5/8/2012 11:20 PM, Peter Meerwald wrote:
> From: Peter Meerwald<p.meerwald@bct-electronic.com>
>
> Signed-off-by: Peter Meerwald<pmeerw@pmeerw.net>
Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
>   drivers/staging/iio/magnetometer/hmc5843.c |   51 +++++++++++++---------------
>   1 files changed, 24 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
> index 1318f7d..6888e04 100644
> --- a/drivers/staging/iio/magnetometer/hmc5843.c
> +++ b/drivers/staging/iio/magnetometer/hmc5843.c
> @@ -85,14 +85,6 @@
>    */
>   #define HMC5843_RATE_OFFSET			0x02
>   #define HMC5843_RATE_BITMASK			0x1C
> -#define RATE_5					0x00
> -#define RATE_10					0x01
> -#define RATE_20					0x02
> -#define RATE_50					0x03
> -#define RATE_100				0x04
> -#define RATE_200				0x05
> -#define RATE_500				0x06
> -
>   #define HMC5843_RATE_NOT_USED			0x07
>
>   /*
> @@ -342,6 +334,21 @@ static s32 hmc5843_set_rate(struct i2c_client *client,
>   	return i2c_smbus_write_byte_data(client, HMC5843_CONFIG_REG_A, reg_val);
>   }
>
> +static int hmc5843_check_sampling_frequency(struct hmc5843_data *data,
> +						const char *buf)
> +{
> +	const char * const *samp_freq = regval_to_samp_freq;
> +	int i;
> +
> +	for (i = 0; i<  HMC5843_RATE_NOT_USED; i++) {
> +		if (strncmp(buf, samp_freq[i],
> +			strlen(samp_freq[i])) == 0)
> +			return i;
> +	}
> +
> +	return -EINVAL;
> +}
> +
>   static ssize_t set_sampling_frequency(struct device *dev,
>   					struct device_attribute *attr,
>   					const char *buf, size_t count)
> @@ -350,27 +357,17 @@ static ssize_t set_sampling_frequency(struct device *dev,
>   	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>   	struct i2c_client *client = to_i2c_client(indio_dev->dev.parent);
>   	struct hmc5843_data *data = iio_priv(indio_dev);
> -	unsigned long rate = 0;
> -
> -	if (strncmp(buf, "0.5" , 3) == 0)
> -		rate = RATE_5;
> -	else if (strncmp(buf, "1" , 1) == 0)
> -		rate = RATE_10;
> -	else if (strncmp(buf, "2", 1) == 0)
> -		rate = RATE_20;
> -	else if (strncmp(buf, "5", 1) == 0)
> -		rate = RATE_50;
> -	else if (strncmp(buf, "10", 2) == 0)
> -		rate = RATE_100;
> -	else if (strncmp(buf, "20" , 2) == 0)
> -		rate = RATE_200;
> -	else if (strncmp(buf, "50" , 2) == 0)
> -		rate = RATE_500;
> -	else
> -		return -EINVAL;
> +	int rate;
> +
> +	rate = hmc5843_check_sampling_frequency(data, buf);
> +	if (rate<  0) {
> +		dev_err(&client->dev,
> +			"sampling frequency is not supported\n");
> +		return rate;
> +	}
>
>   	mutex_lock(&data->lock);
> -	dev_dbg(dev, "set rate to %lu\n", rate);
> +	dev_dbg(dev, "set rate to %d\n", rate);
>   	if (hmc5843_set_rate(client, rate)) {
>   		count = -EINVAL;
>   		goto exit;

Re: [PATCH 5/8] iio: add check for measurement configuration value passed to hmc5843

[Jonathan Cameron]

On 5/8/2012 11:20 PM, Peter Meerwald wrote:
> From: Peter Meerwald<p.meerwald@bct-electronic.com>
>
I'm not keen on the underlying code, but this is still worth having!
Something string based at least, or that played well with runtime
power management would be nice. Ah well, can't have everything :(
> Signed-off-by: Peter Meerwald<pmeerw@pmeerw.net>
Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
>   drivers/staging/iio/magnetometer/hmc5843.c |    7 ++++++-
>   1 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
> index 6888e04..57ab4fb 100644
> --- a/drivers/staging/iio/magnetometer/hmc5843.c
> +++ b/drivers/staging/iio/magnetometer/hmc5843.c
> @@ -281,9 +281,14 @@ static ssize_t hmc5843_set_measurement_configuration(struct device *dev,
>   	struct i2c_client *client = to_i2c_client(indio_dev->dev.parent);
>   	struct hmc5843_data *data = iio_priv(indio_dev);
>   	unsigned long meas_conf = 0;
> -	int error = kstrtoul(buf, 10,&meas_conf);
> +	int error;
> +
> +	error = kstrtoul(buf, 10,&meas_conf);
>   	if (error)
>   		return error;
> +	if (meas_conf>= HMC5843_MEAS_CONF_NOT_USED)
> +		return -EINVAL;
> +
>   	mutex_lock(&data->lock);
>
>   	dev_dbg(dev, "set mode to %lu\n", meas_conf);

Re: [PATCH 6/8] iio: cleanup and move comments in hmc5843

[Jonathan Cameron]

On 5/8/2012 11:20 PM, Peter Meerwald wrote:
> From: Peter Meerwald<p.meerwald@bct-electronic.com>
>
All sensible enough.  A few comments inline but I'm fine with what you 
have here.
> Signed-off-by: Peter Meerwald<pmeerw@pmeerw.net>
Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
>   drivers/staging/iio/magnetometer/hmc5843.c |  122 ++++++++++++++-------------
>   1 files changed, 63 insertions(+), 59 deletions(-)
>
> diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
> index 57ab4fb..57ed182 100644
> --- a/drivers/staging/iio/magnetometer/hmc5843.c
> +++ b/drivers/staging/iio/magnetometer/hmc5843.c
> @@ -103,6 +103,18 @@ static const int hmc5843_regval_to_nanoscale[] = {
>   	6173, 7692, 10309, 12821, 18868, 21739, 25641, 35714
>   };
>
> +/*
> + * From the datasheet:
> + * Value	| Sensor input field range (Ga)	| Gain (counts/milli-Gauss)
> + * 0		| (+-)0.7			| 1620
> + * 1		| (+-)1.0			| 1300
> + * 2		| (+-)1.5			| 970
> + * 3		| (+-)2.0			| 780
> + * 4		| (+-)3.2			| 530
> + * 5		| (+-)3.8			| 460
> + * 6		| (+-)4.5			| 390
> + * 7		| (+-)6.5			| 280
> + */
>   static const int regval_to_input_field_mg[] = {
>   	700,
>   	1000,
> @@ -113,6 +125,19 @@ static const int regval_to_input_field_mg[] = {
>   	4500,
>   	6500
>   };
> +
> +/*
> + * From the datasheet:
> + * Value	| Data output rate (Hz)
> + * 0		| 0.5
> + * 1		| 1
> + * 2		| 2
> + * 3		| 5
> + * 4		| 10 (default)
> + * 5		| 20
> + * 6		| 50
> + * 7		| Not used
> + */
>   static const char * const regval_to_samp_freq[] = {
>   	"0.5",
>   	"1",
> @@ -130,18 +155,16 @@ static const unsigned short normal_i2c[] = { HMC5843_I2C_ADDRESS,
>   /* Each client has this additional data */
>   struct hmc5843_data {
>   	struct mutex lock;
I'd argue for doing it the other way round and realigning lock rather 
than removing the tabs, but that's just personal taste.
> -	u8		rate;
> -	u8		meas_conf;
> -	u8		operating_mode;
> -	u8		range;
> +	u8 rate;
> +	u8 meas_conf;
> +	u8 operating_mode;
> +	u8 range;
>   };
>
> -static void hmc5843_init_client(struct i2c_client *client);
> -
> +/* The lower two bits contain the current conversion mode */
Would prefer this as a kerneldoc description. But this is better than 
nothing!
>   static s32 hmc5843_configure(struct i2c_client *client,
>   				       u8 operating_mode)
>   {
> -	/* The lower two bits contain the current conversion mode */
>   	return i2c_smbus_write_byte_data(client,
>   					HMC5843_MODE_REG,
>   					operating_mode&  HMC5843_MODE_MASK);
> @@ -166,23 +189,22 @@ static int hmc5843_read_measurement(struct iio_dev *indio_dev,
>   	if (result<  0)
>   		return -EINVAL;
>
> -	*val	= (s16)swab16((u16)result);
> +	*val = (s16)swab16((u16)result);
>   	return IIO_VAL_INT;
>   }
>
> -
>   /*
> - * From the datasheet
> + * From the datasheet:
>    * 0 - Continuous-Conversion Mode: In continuous-conversion mode, the
> - * device continuously performs conversions and places the result in the
> - * data register.
> + *     device continuously performs conversions and places the result in
> + *     the data register.
>    *
> - * 1 - Single-Conversion Mode : device performs a single measurement,
> - *  sets RDY high and returned to sleep mode
> + * 1 - Single-Conversion Mode : Device performs a single measurement,
> + *     sets RDY high and returns to sleep mode.
>    *
> - * 2 - Idle Mode :  Device is placed in idle mode.
> + * 2 - Idle Mode : Device is placed in idle mode.
>    *
> - * 3 - Sleep Mode. Device is placed in sleep mode.
> + * 3 - Sleep Mode : Device is placed in sleep mode.
>    *
>    */
>   static ssize_t hmc5843_show_operating_mode(struct device *dev,
> @@ -206,13 +228,14 @@ static ssize_t hmc5843_set_operating_mode(struct device *dev,
>   	unsigned long operating_mode = 0;
>   	s32 status;
>   	int error;
> +
>   	mutex_lock(&data->lock);
>   	error = kstrtoul(buf, 10,&operating_mode);
>   	if (error) {
>   		count = error;
>   		goto exit;
>   	}
> -	dev_dbg(dev, "set Conversion mode to %lu\n", operating_mode);
> +	dev_dbg(dev, "set conversion mode to %lu\n", operating_mode);
>   	if (operating_mode>  HMC5843_MODE_SLEEP) {
>   		count = -EINVAL;
>   		goto exit;
> @@ -230,6 +253,7 @@ exit:
>   	mutex_unlock(&data->lock);
>   	return count;
>   }
> +
>   static IIO_DEVICE_ATTR(operating_mode,
>   			S_IWUSR | S_IRUGO,
>   			hmc5843_show_operating_mode,
> @@ -239,17 +263,19 @@ static IIO_DEVICE_ATTR(operating_mode,
>   /*
>    * API for setting the measurement configuration to
>    * Normal, Positive bias and Negative bias
> - * From the datasheet
>    *
> - * Normal measurement configuration (default): In normal measurement
> - * configuration the device follows normal measurement flow. Pins BP and BN
> - * are left floating and high impedance.
> + * From the datasheet:
> + * 0 - Normal measurement configuration (default): In normal measurement
> + *     configuration the device follows normal measurement flow. Pins BP
> + *     and BN are left floating and high impedance.
>    *
> - * Positive bias configuration: In positive bias configuration, a positive
> - * current is forced across the resistive load on pins BP and BN.
> + * 1 - Positive bias configuration: In positive bias configuration, a
> + *     positive current is forced across the resistive load on pins BP
> + *     and BN.
>    *
> - * Negative bias configuration. In negative bias configuration, a negative
> - * current is forced across the resistive load on pins BP and BN.
> + * 2 - Negative bias configuration. In negative bias configuration, a
> + *     negative current is forced across the resistive load on pins BP
> + *     and BN.
>    *
>    */
>   static s32 hmc5843_set_meas_conf(struct i2c_client *client,
> @@ -290,8 +316,7 @@ static ssize_t hmc5843_set_measurement_configuration(struct device *dev,
>   		return -EINVAL;
>
>   	mutex_lock(&data->lock);
> -
> -	dev_dbg(dev, "set mode to %lu\n", meas_conf);
> +	dev_dbg(dev, "set measurement configuration to %lu\n", meas_conf);
>   	if (hmc5843_set_meas_conf(client, meas_conf)) {
>   		count = -EINVAL;
>   		goto exit;
> @@ -302,25 +327,13 @@ exit:
>   	mutex_unlock(&data->lock);
>   	return count;
>   }
> +
>   static IIO_DEVICE_ATTR(meas_conf,
>   			S_IWUSR | S_IRUGO,
>   			hmc5843_show_measurement_configuration,
>   			hmc5843_set_measurement_configuration,
>   			0);
>
> -/*
> - * From Datasheet
> - * The table shows the minimum data output
> - * Value	| Minimum data output rate(Hz)
> - * 0		| 0.5
> - * 1		| 1
> - * 2		| 2
> - * 3		| 5
> - * 4		| 10 (default)
> - * 5		| 20
> - * 6		| 50
> - * 7		| Not used
> - */
>   static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("0.5 1 2 5 10 20 50");
>
>   static s32 hmc5843_set_rate(struct i2c_client *client,
> @@ -333,7 +346,7 @@ static s32 hmc5843_set_rate(struct i2c_client *client,
>   	reg_val = (data->meas_conf) |  (rate<<  HMC5843_RATE_OFFSET);
>   	if (rate>= HMC5843_RATE_NOT_USED) {
>   		dev_err(&client->dev,
> -			"This data output rate is not supported\n");
> +			"data output rate is not supported\n");
>   		return -EINVAL;
>   	}
>   	return i2c_smbus_write_byte_data(client, HMC5843_CONFIG_REG_A, reg_val);
> @@ -392,31 +405,19 @@ static ssize_t show_sampling_frequency(struct device *dev,
>   	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
>   	s32 rate;
>
> -	rate = i2c_smbus_read_byte_data(client,  this_attr->address);
> +	rate = i2c_smbus_read_byte_data(client, this_attr->address);
>   	if (rate<  0)
>   		return rate;
>   	rate = (rate&  HMC5843_RATE_BITMASK)>>  HMC5843_RATE_OFFSET;
>   	return sprintf(buf, "%s\n", regval_to_samp_freq[rate]);
>   }
> +
>   static IIO_DEVICE_ATTR(sampling_frequency,
>   			S_IWUSR | S_IRUGO,
>   			show_sampling_frequency,
>   			set_sampling_frequency,
>   			HMC5843_CONFIG_REG_A);
>
> -/*
> - * From Datasheet
> - *	Nominal gain settings
> - * Value	| Sensor Input Field Range(Ga)	| Gain(counts/ milli-gauss)
> - *0		|(+-)0.7			|1620
> - *1		|(+-)1.0			|1300
> - *2		|(+-)1.5			|970
> - *3		|(+-)2.0			|780
> - *4		|(+-)3.2			|530
> - *5		|(+-)3.8			|460
> - *6		|(+-)4.5			|390
> - *7		|(+-)6.5			|280
> - */
>   static ssize_t show_range(struct device *dev,
>   				struct device_attribute *attr,
>   				char *buf)
> @@ -440,6 +441,7 @@ static ssize_t set_range(struct device *dev,
>   	struct hmc5843_data *data = iio_priv(indio_dev);
>   	unsigned long range = 0;
>   	int error;
> +
>   	mutex_lock(&data->lock);
>   	error = kstrtoul(buf, 10,&range);
>   	if (error) {
> @@ -461,8 +463,8 @@ static ssize_t set_range(struct device *dev,
>   exit:
>   	mutex_unlock(&data->lock);
>   	return count;
> -
>   }
> +
>   static IIO_DEVICE_ATTR(in_magn_range,
>   			S_IWUSR | S_IRUGO,
>   			show_range,
> @@ -537,7 +539,7 @@ static int hmc5843_detect(struct i2c_client *client,
>   	return 0;
>   }
>
> -/* Called when we have found a new HMC5843. */
> +/* Called when we have found a new HMC5843 */
>   static void hmc5843_init_client(struct i2c_client *client)
>   {
>   	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> @@ -548,6 +550,7 @@ static void hmc5843_init_client(struct i2c_client *client)
>   	hmc5843_configure(client, data->operating_mode);
>   	i2c_smbus_write_byte_data(client, HMC5843_CONFIG_REG_B, data->range);
>   	mutex_init(&data->lock);
> +
>   	pr_info("HMC5843 initialized\n");
>   }
>
> @@ -577,8 +580,6 @@ static int hmc5843_probe(struct i2c_client *client,
>   	data->operating_mode = HMC5843_MODE_CONVERSION_CONTINUOUS;
>
>   	i2c_set_clientdata(client, indio_dev);
> -
> -	/* Initialize the HMC5843 chip */
>   	hmc5843_init_client(client);
>
>   	indio_dev->info =&hmc5843_info;
> @@ -587,10 +588,13 @@ static int hmc5843_probe(struct i2c_client *client,
>   	indio_dev->num_channels = ARRAY_SIZE(hmc5843_channels);
>   	indio_dev->dev.parent =&client->dev;
>   	indio_dev->modes = INDIO_DIRECT_MODE;
> +
>   	err = iio_device_register(indio_dev);
>   	if (err)
>   		goto exit_free2;
> +
>   	return 0;
> +
>   exit_free2:
>   	iio_device_free(indio_dev);
>   exit:

Re: [PATCH 7/8] iio: rename function/data to consistently start with hmc5843_

[Jonathan Cameron]

On 5/8/2012 11:20 PM, Peter Meerwald wrote:
> From: Peter Meerwald<p.meerwald@bct-electronic.com>
>
> Signed-off-by: Peter Meerwald<pmeerw@pmeerw.net>
Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
>   drivers/staging/iio/magnetometer/hmc5843.c |   26 +++++++++++++-------------
>   1 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
> index 57ed182..2e1bc6c 100644
> --- a/drivers/staging/iio/magnetometer/hmc5843.c
> +++ b/drivers/staging/iio/magnetometer/hmc5843.c
> @@ -115,7 +115,7 @@ static const int hmc5843_regval_to_nanoscale[] = {
>    * 6		| (+-)4.5			| 390
>    * 7		| (+-)6.5			| 280
>    */
> -static const int regval_to_input_field_mg[] = {
> +static const int hmc5843_regval_to_input_field_mga[] = {
>   	700,
>   	1000,
>   	1500,
> @@ -138,7 +138,7 @@ static const int regval_to_input_field_mg[] = {
>    * 6		| 50
>    * 7		| Not used
>    */
> -static const char * const regval_to_samp_freq[] = {
> +static const char * const hmc5843_regval_to_sample_freq[] = {
>   	"0.5",
>   	"1",
>   	"2",
> @@ -355,7 +355,7 @@ static s32 hmc5843_set_rate(struct i2c_client *client,
>   static int hmc5843_check_sampling_frequency(struct hmc5843_data *data,
>   						const char *buf)
>   {
> -	const char * const *samp_freq = regval_to_samp_freq;
> +	const char * const *samp_freq = hmc5843_regval_to_sample_freq;
>   	int i;
>
>   	for (i = 0; i<  HMC5843_RATE_NOT_USED; i++) {
> @@ -367,7 +367,7 @@ static int hmc5843_check_sampling_frequency(struct hmc5843_data *data,
>   	return -EINVAL;
>   }
>
> -static ssize_t set_sampling_frequency(struct device *dev,
> +static ssize_t hmc5843_set_sampling_frequency(struct device *dev,
>   					struct device_attribute *attr,
>   					const char *buf, size_t count)
>   {
> @@ -397,7 +397,7 @@ exit:
>   	return count;
>   }
>
> -static ssize_t show_sampling_frequency(struct device *dev,
> +static ssize_t hmc5843_show_sampling_frequency(struct device *dev,
>   			struct device_attribute *attr, char *buf)
>   {
>   	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> @@ -409,16 +409,16 @@ static ssize_t show_sampling_frequency(struct device *dev,
>   	if (rate<  0)
>   		return rate;
>   	rate = (rate&  HMC5843_RATE_BITMASK)>>  HMC5843_RATE_OFFSET;
> -	return sprintf(buf, "%s\n", regval_to_samp_freq[rate]);
> +	return sprintf(buf, "%s\n", hmc5843_regval_to_sample_freq[rate]);
>   }
>
>   static IIO_DEVICE_ATTR(sampling_frequency,
>   			S_IWUSR | S_IRUGO,
> -			show_sampling_frequency,
> -			set_sampling_frequency,
> +			hmc5843_show_sampling_frequency,
> +			hmc5843_set_sampling_frequency,
>   			HMC5843_CONFIG_REG_A);
>
> -static ssize_t show_range(struct device *dev,
> +static ssize_t hmc5843_show_range_gain(struct device *dev,
>   				struct device_attribute *attr,
>   				char *buf)
>   {
> @@ -427,10 +427,10 @@ static ssize_t show_range(struct device *dev,
>   	struct hmc5843_data *data = iio_priv(indio_dev);
>
>   	range = data->range;
> -	return sprintf(buf, "%d\n", regval_to_input_field_mg[range]);
> +	return sprintf(buf, "%d\n", hmc5843_regval_to_input_field_mga[range]);
>   }
>
> -static ssize_t set_range(struct device *dev,
> +static ssize_t hmc5843_set_range_gain(struct device *dev,
>   			struct device_attribute *attr,
>   			const char *buf,
>   			size_t count)
> @@ -467,8 +467,8 @@ exit:
>
>   static IIO_DEVICE_ATTR(in_magn_range,
>   			S_IWUSR | S_IRUGO,
> -			show_range,
> -			set_range,
> +			hmc5843_show_range_gain,
> +			hmc5843_set_range_gain,
>   			HMC5843_CONFIG_REG_B);
>
>   static int hmc5843_read_raw(struct iio_dev *indio_dev,

Re: [PATCH 8/8] iio: add support for hmc5883/hmc5883l to hmc5843 magnetometer driver

[Jonathan Cameron]

On 5/8/2012 11:20 PM, Peter Meerwald wrote:
> From: Peter Meerwald<p.meerwald@bct-electronic.com>
>
Hi Peter,

Mostly fine.  Few trivial bits inline and one bigger one.

I'd prefer to see the difference between the parts implemented
via a table of static 'chip_variant' structures. It's generally
cleaner, less error prone and moves some stuff into static data
rather than code which is always nice.  It's also how this is
handled in most of the other drivers and as a lazy maintainer I
like everything to look similar!

Jonathan
> v2 addresses review comments:
> * fixes and cleanups have been split out (Jonathan Cameron)
> * constants are generally prefixed HMC5843_, except when related
>    specifically to hmc5883 (Jonathan Cameron)
> * simplify code and avoid temp buffer in
>    hmc5843_show_sampling_frequencies_available() (Lars-Peter Clausen)
> * use sysfs_streq() instead of strncmp()/strlen() in
>    hmc5843_check_sampling_frequency() (Lars-Peter Clausen)
>
> Signed-off-by: Peter Meerwald<pmeerw@pmeerw.net>
> ---
>   drivers/staging/iio/magnetometer/Kconfig   |    8 +-
>   drivers/staging/iio/magnetometer/hmc5843.c |  181 +++++++++++++++++++++++----
>   2 files changed, 158 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/staging/iio/magnetometer/Kconfig b/drivers/staging/iio/magnetometer/Kconfig
> index 722c4e1..b9d9325 100644
> --- a/drivers/staging/iio/magnetometer/Kconfig
> +++ b/drivers/staging/iio/magnetometer/Kconfig
> @@ -15,13 +15,13 @@ config SENSORS_AK8975
>   	  will be called ak8975.
>
>   config SENSORS_HMC5843
> -	tristate "Honeywell HMC5843 3-Axis Magnetometer"
> +	tristate "Honeywell HMC5843/5883/5883L 3-Axis Magnetometer"
>   	depends on I2C
>   	help
> -	  Say Y here to add support for the Honeywell HMC 5843 3-Axis
> -	  Magnetometer (digital compass).
> +	  Say Y here to add support for the Honeywell HMC5843, HMC5883 and
> +	  HMC5883L 3-Axis Magnetometer (digital compass).
>
>   	  To compile this driver as a module, choose M here: the module
> -	  will be called hmc5843
> +	  will be called hmc5843.
Technically an independent change so should be in separate patch, but 
frankly I don't care for something this trivial!
>
>   endmenu
> diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
> index 2e1bc6c..8321a77 100644
> --- a/drivers/staging/iio/magnetometer/hmc5843.c
> +++ b/drivers/staging/iio/magnetometer/hmc5843.c
> @@ -2,6 +2,8 @@
>       Author: Shubhrajyoti Datta<shubhrajyoti@ti.com>
>       Acknowledgement: Jonathan Cameron<jic23@cam.ac.uk>  for valuable inputs.
>
> +    Support for HMC5883 and HMC5883L by Peter Meerwald<pmeerw@pmeerw.net>.
> +
>       This program is free software; you can redistribute it and/or modify
>       it under the terms of the GNU General Public License as published by
>       the Free Software Foundation; either version 2 of the License, or
> @@ -44,6 +46,11 @@
>   #define HMC5843_ID_REG_B			0x0B
>   #define HMC5843_ID_REG_C			0x0C
>
> +enum hmc5843_ids {
> +	HMC5843_ID,
> +	HMC5883_ID,
> +	HMC5883L_ID,
> +};
>
>   /*
>    * Beware: identification of the HMC5883 is still "H43";
> @@ -103,8 +110,16 @@ static const int hmc5843_regval_to_nanoscale[] = {
>   	6173, 7692, 10309, 12821, 18868, 21739, 25641, 35714
>   };
>
> +static const int hmc5883_regval_to_nanoscale[] = {
> +	7812, 9766, 13021, 16287, 24096, 27701, 32573, 45662
> +};
> +
> +static const int hmc5883l_regval_to_nanoscale[] = {
> +	7299, 9174, 12195, 15152, 22727, 25641, 30303, 43478
> +};
> +
>   /*
> - * From the datasheet:
> + * From the HMC5843 datasheet:
>    * Value	| Sensor input field range (Ga)	| Gain (counts/milli-Gauss)
>    * 0		| (+-)0.7			| 1620
>    * 1		| (+-)1.0			| 1300
> @@ -114,6 +129,28 @@ static const int hmc5843_regval_to_nanoscale[] = {
>    * 5		| (+-)3.8			| 460
>    * 6		| (+-)4.5			| 390
>    * 7		| (+-)6.5			| 280
> + *
> + * From the HMC5883 datasheet:
> + * Value	| Recommended sensor field range (Ga)	| Gain (counts/Gauss)
> + * 0		| (+-)0.9				| 1280
> + * 1		| (+-)1.2				| 1024
> + * 2		| (+-)1.9				| 768
> + * 3		| (+-)2.5				| 614
> + * 4		| (+-)4.0				| 415
> + * 5		| (+-)4.6				| 361
> + * 6		| (+-)5.5				| 307
> + * 7		| (+-)7.9				| 219
> + *
> + * From the HMC5883L datasheet:
> + * Value	| Recommended sensor field range (Ga)	| Gain (LSB/Gauss)
> + * 0		| (+-)0.88				| 1370
> + * 1		| (+-)1.3				| 1090
> + * 2		| (+-)1.9				| 820
> + * 3		| (+-)2.5				| 660
> + * 4		| (+-)4.0				| 440
> + * 5		| (+-)4.7				| 390
> + * 6		| (+-)5.6				| 330
> + * 7		| (+-)8.1				| 230
>    */
>   static const int hmc5843_regval_to_input_field_mga[] = {
>   	700,
> @@ -126,17 +163,41 @@ static const int hmc5843_regval_to_input_field_mga[] = {
>   	6500
>   };
>
Maybe rotate these tables?  (e.g. same as done with the scale ones above)
> +static const int hmc5883_regval_to_input_field_mga[] = {
> +	900,
> +	1200,
> +	1900,
> +	2500,
> +	4000,
> +	4600,
> +	5500,
> +	7900
> +};
> +
> +static const int hmc5883l_regval_to_input_field_mga[] = {
> +	880,
> +	1300,
> +	1900,
> +	2500,
> +	4000,
> +	4700,
> +	5600,
> +	8100
> +};
> +
> +
>   /*
>    * From the datasheet:
> - * Value	| Data output rate (Hz)
> - * 0		| 0.5
> - * 1		| 1
> - * 2		| 2
> - * 3		| 5
> - * 4		| 10 (default)
> - * 5		| 20
> - * 6		| 50
> - * 7		| Not used
> + * Value	| HMC5843		| HMC5883/HMC5883L
> + *		| Data output rate (Hz)	| Data output rate (Hz)
> + * 0		| 0.5			| 0.75
> + * 1		| 1			| 1.5
> + * 2		| 2			| 3
> + * 3		| 5			| 7.5
> + * 4		| 10 (default)		| 15
> + * 5		| 20			| 30
> + * 6		| 50			| 75
> + * 7		| Not used		| Not used
>    */
>   static const char * const hmc5843_regval_to_sample_freq[] = {
>   	"0.5",
> @@ -148,6 +209,16 @@ static const char * const hmc5843_regval_to_sample_freq[] = {
>   	"50",
>   };
>
> +static const char * const hmc5883_regval_to_sample_freq[] = {
> +	"0.75",
> +	"1.5",
> +	"3",
> +	"7.5",
> +	"15",
> +	"30",
> +	"75",
> +};
> +
>   /* Addresses to scan: 0x1E */
>   static const unsigned short normal_i2c[] = { HMC5843_I2C_ADDRESS,
>   					     I2C_CLIENT_END };
> @@ -159,6 +230,9 @@ struct hmc5843_data {
>   	u8 meas_conf;
>   	u8 operating_mode;
>   	u8 range;
> +	const char * const *regval_to_sample_freq;
> +	const int *regval_to_input_field_mga;
> +	const int *regval_to_nanoscale;
>   };
>
>   /* The lower two bits contain the current conversion mode */
> @@ -334,7 +408,25 @@ static IIO_DEVICE_ATTR(meas_conf,
>   			hmc5843_set_measurement_configuration,
>   			0);
>
> -static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("0.5 1 2 5 10 20 50");
> +static ssize_t hmc5843_show_sampling_frequencies_available(struct device *dev,
> +						struct device_attribute *attr,
> +						char *buf)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct hmc5843_data *data = iio_priv(indio_dev);
> +	ssize_t total_n = 0;
> +	int i;
> +
> +	for (i = 0; i<  HMC5843_RATE_NOT_USED; i++) {
> +		ssize_t n = sprintf(buf, "%s ", data->regval_to_sample_freq[i]);
> +		buf += n;
> +		total_n += n;
> +	}
newline on the end? + ideally clear out the extra space...
> +
> +	return total_n;
> +}
> +
> +static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(hmc5843_show_sampling_frequencies_available);
>
>   static s32 hmc5843_set_rate(struct i2c_client *client,
>   				u8 rate)
> @@ -343,24 +435,23 @@ static s32 hmc5843_set_rate(struct i2c_client *client,
>   	struct hmc5843_data *data = iio_priv(indio_dev);
>   	u8 reg_val;
>
> -	reg_val = (data->meas_conf) |  (rate<<  HMC5843_RATE_OFFSET);
>   	if (rate>= HMC5843_RATE_NOT_USED) {
>   		dev_err(&client->dev,
>   			"data output rate is not supported\n");
>   		return -EINVAL;
>   	}
> +	reg_val = data->meas_conf | (rate<<  HMC5843_RATE_OFFSET);
Technically this is an unrelated change, sensible but shouldn't really
be in this patch!
>   	return i2c_smbus_write_byte_data(client, HMC5843_CONFIG_REG_A, reg_val);
>   }
>
>   static int hmc5843_check_sampling_frequency(struct hmc5843_data *data,
>   						const char *buf)
>   {
> -	const char * const *samp_freq = hmc5843_regval_to_sample_freq;
> +	const char * const *samp_freq = data->regval_to_sample_freq;
>   	int i;
>
>   	for (i = 0; i<  HMC5843_RATE_NOT_USED; i++) {
> -		if (strncmp(buf, samp_freq[i],
> -			strlen(samp_freq[i])) == 0)
> +		if (sysfs_streq(buf, samp_freq[i]))
>   			return i;
>   	}
>
> @@ -403,13 +494,14 @@ static ssize_t hmc5843_show_sampling_frequency(struct device *dev,
>   	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>   	struct i2c_client *client = to_i2c_client(indio_dev->dev.parent);
>   	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
> +	struct hmc5843_data *data = iio_priv(indio_dev);
>   	s32 rate;
>
>   	rate = i2c_smbus_read_byte_data(client, this_attr->address);
>   	if (rate<  0)
>   		return rate;
>   	rate = (rate&  HMC5843_RATE_BITMASK)>>  HMC5843_RATE_OFFSET;
> -	return sprintf(buf, "%s\n", hmc5843_regval_to_sample_freq[rate]);
> +	return sprintf(buf, "%s\n", data->regval_to_sample_freq[rate]);
>   }
>
>   static IIO_DEVICE_ATTR(sampling_frequency,
> @@ -427,7 +519,7 @@ static ssize_t hmc5843_show_range_gain(struct device *dev,
>   	struct hmc5843_data *data = iio_priv(indio_dev);
>
>   	range = data->range;
> -	return sprintf(buf, "%d\n", hmc5843_regval_to_input_field_mga[range]);
> +	return sprintf(buf, "%d\n", data->regval_to_input_field_mga[range]);
>   }
>
>   static ssize_t hmc5843_set_range_gain(struct device *dev,
> @@ -485,7 +577,7 @@ static int hmc5843_read_raw(struct iio_dev *indio_dev,
>   						val);
>   	case IIO_CHAN_INFO_SCALE:
>   		*val = 0;
> -		*val2 = hmc5843_regval_to_nanoscale[data->range];
> +		*val2 = data->regval_to_nanoscale[data->range];
>   		return IIO_VAL_INT_PLUS_NANO;
>   	};
>   	return -EINVAL;
> @@ -507,12 +599,19 @@ static const struct iio_chan_spec hmc5843_channels[] = {
>   	HMC5843_CHANNEL(Z, HMC5843_DATA_OUT_Z_MSB_REG),
>   };
>
> +static const struct iio_chan_spec hmc5883_channels[] = {
> +	HMC5843_CHANNEL(X, HMC5843_DATA_OUT_X_MSB_REG),
> +	HMC5843_CHANNEL(Y, HMC5883_DATA_OUT_Y_MSB_REG),
> +	HMC5843_CHANNEL(Z, HMC5883_DATA_OUT_Z_MSB_REG),
> +};
> +
> +
>   static struct attribute *hmc5843_attributes[] = {
>   	&iio_dev_attr_meas_conf.dev_attr.attr,
>   	&iio_dev_attr_operating_mode.dev_attr.attr,
>   	&iio_dev_attr_sampling_frequency.dev_attr.attr,
>   	&iio_dev_attr_in_magn_range.dev_attr.attr,
> -	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
> +	&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
>   	NULL
>   };
>
> @@ -539,19 +638,47 @@ static int hmc5843_detect(struct i2c_client *client,
>   	return 0;
>   }
>
> -/* Called when we have found a new HMC5843 */
> -static void hmc5843_init_client(struct i2c_client *client)
> +/* Called when we have found a new HMC58X3 */
> +static void hmc5843_init_client(struct i2c_client *client,
> +				const struct i2c_device_id *id)
>   {
>   	struct iio_dev *indio_dev = i2c_get_clientdata(client);
>   	struct hmc5843_data *data = iio_priv(indio_dev);
>
> +	switch (id->driver_data) {
> +	case HMC5843_ID:
This smacks of stuff that would be better done with a 'chip_variant' 
structure.  That way you get it into a simple selection of an element
in a table and it's all static data.  See how it is done in adc/max1363 
for example.

> +		data->regval_to_sample_freq = hmc5843_regval_to_sample_freq;
> +		data->regval_to_input_field_mga =
> +			hmc5843_regval_to_input_field_mga;
> +		data->regval_to_nanoscale = hmc5843_regval_to_nanoscale;
> +		indio_dev->channels = hmc5843_channels;
> +		indio_dev->num_channels = ARRAY_SIZE(hmc5843_channels);
> +		break;
> +	case HMC5883_ID:
> +		data->regval_to_sample_freq = hmc5883_regval_to_sample_freq;
> +		data->regval_to_input_field_mga =
> +			hmc5883_regval_to_input_field_mga;
> +		data->regval_to_nanoscale = hmc5883_regval_to_nanoscale;
> +		indio_dev->channels = hmc5883_channels;
> +		indio_dev->num_channels = ARRAY_SIZE(hmc5883_channels);
> +		break;
> +	case HMC5883L_ID:
> +		data->regval_to_sample_freq = hmc5883_regval_to_sample_freq;
> +		data->regval_to_input_field_mga =
> +			hmc5883l_regval_to_input_field_mga;
> +		data->regval_to_nanoscale = hmc5883l_regval_to_nanoscale;
> +		indio_dev->channels = hmc5883_channels;
> +		indio_dev->num_channels = ARRAY_SIZE(hmc5883_channels);
> +		break;
> +	}
> +
>   	hmc5843_set_meas_conf(client, data->meas_conf);
>   	hmc5843_set_rate(client, data->rate);
>   	hmc5843_configure(client, data->operating_mode);
>   	i2c_smbus_write_byte_data(client, HMC5843_CONFIG_REG_B, data->range);
>   	mutex_init(&data->lock);
>
> -	pr_info("HMC5843 initialized\n");
> +	pr_info("%s initialized\n", id->name);
>   }
>
>   static const struct iio_info hmc5843_info = {
> @@ -580,12 +707,10 @@ static int hmc5843_probe(struct i2c_client *client,
>   	data->operating_mode = HMC5843_MODE_CONVERSION_CONTINUOUS;
>
>   	i2c_set_clientdata(client, indio_dev);
> -	hmc5843_init_client(client);
> +	hmc5843_init_client(client, id);
>
>   	indio_dev->info =&hmc5843_info;
>   	indio_dev->name = id->name;
> -	indio_dev->channels = hmc5843_channels;
> -	indio_dev->num_channels = ARRAY_SIZE(hmc5843_channels);
>   	indio_dev->dev.parent =&client->dev;
>   	indio_dev->modes = INDIO_DIRECT_MODE;
>
> @@ -636,7 +761,9 @@ static SIMPLE_DEV_PM_OPS(hmc5843_pm_ops, hmc5843_suspend, hmc5843_resume);
>   #endif
>
>   static const struct i2c_device_id hmc5843_id[] = {
> -	{ "hmc5843", 0 },
> +	{ "hmc5843", HMC5843_ID },
> +	{ "hmc5883", HMC5883_ID },
> +	{ "hmc5883l", HMC5883L_ID },
>   	{ }
>   };
>   MODULE_DEVICE_TABLE(i2c, hmc5843_id);
> @@ -655,5 +782,5 @@ static struct i2c_driver hmc5843_driver = {
>   module_i2c_driver(hmc5843_driver);
>
>   MODULE_AUTHOR("Shubhrajyoti Datta<shubhrajyoti@ti.com");
> -MODULE_DESCRIPTION("HMC5843 driver");
> +MODULE_DESCRIPTION("HMC5843/5883/5883L driver");
>   MODULE_LICENSE("GPL");

Re: [PATCH 1/8] iio: fix access to hmc5843 private data

[Peter Meerwald]

> > i2c_get_clientdata(client) points to iio_dev, not hmc5843_data; fixes
> > issue similar to 62d2feb9803f18c4e3c8a1a2c7e30a54df8a1d72

> Hmm.. Really hope there aren't many more of these hiding in the tree.
> (All my fault in the first place ;(

part of the problem is that some (i2c) drivers inconsistently point to 
iio_dev or to private data

regards, p.

-- 

Peter Meerwald
+43-664-2444418 (mobile)

Re: [PATCH 1/8] iio: fix access to hmc5843 private data

[Shubhrajyoti Datta]

>
> On Wed, May 9, 2012 at 3:49 AM, Peter Meerwald <pmeerw@pmeerw.net> wrote:
>> From: Peter Meerwald <p.meerwald@bct-electronic.com>
>>
>> i2c_get_clientdata(client) points to iio_dev, not hmc5843_data; fixes
>> issue similar to 62d2feb9803f18c4e3c8a1a2c7e30a54df8a1d72
>>
> Looks good to me ack.
(correcting the ack format)
Acked-by:  Shubhrajyoti D <shubhrajyoti@ti.com>