[PATCH v2 00/15] staging:iio:hmc5843 cleanup

[PATCH v2 00/15] staging:iio:hmc5843 cleanup

[Peter Meerwald]

updated v2 patch series of the hmc5843 cleanup, addressnig comments by 
Jonathan Cameron

patch 1: correctly dereferencing dev, fixed suspend/resume
patch 7: checked CALIBSCALE, output is as follows

  # cat calibscale_available 
  0.880000 1.300000 1.900000 2.500000 4.0 4.700000 5.600000 8.100000
  # cat in_magn_calibscale 
  1.300000
  # cat in_magn_scale 
  0.000009174
  # cat in_magn_x_raw 
  252
  # echo 1.900000 > in_magn_calibscale
  # cat in_magn_scale 
  0.000012195
  # cat in_magn_x_raw 
  198
  # echo 0.88 > in_magn_calibscale
  # cat in_magn_scale 
  0.000007299
  # cat in_magn_x_raw
  318

  calibscale adjusts the measurement range and scale is used to keep Gauss
  reading the same
  
patch 9: buffer is now part of hmc5843_data, using IIO core demux as suggested

remaining patches should not have changed

Peter Meerwald (15):
  staging:iio:hmc5843: Add pointer to i2c client to data struct
  staging:iio:hmc5843: Rewrite init function
  staging:iio:hmc5843: Use INFO_SAMP_FREQ
  staging:iio:hmc5843: Remove unused LSB register #defines
  staging:iio:hmc5843: Tighten comments
  staging:iio:hmc5843: Introduce helper functions to show/check list of
    int pairs
  staging:iio:hmc5843: Use CALIBSCALE instead of magn_range
  staging:iio:hmc5843: Always read all channels values otherwise no
    updates
  staging:iio:hmc5843: Add trigger handling
  staging:iio:hmc5843: Remove ability to change operating mode
  staging:iio:hmc5843: Rename _configure() to _set_mode()
  staging:iio:hmc5843: Reorganize _set_meas_conf()
  staging:iio:hmc5843: Rename _set_rate() to _set_samp_freq()
  staging:iio:hmc5843: Introduce _set_range_gain()
  staging:iio:hmc5843: Check initialization and chip identifier

 drivers/staging/iio/magnetometer/hmc5843.c | 612 ++++++++++++++---------------
 1 file changed, 287 insertions(+), 325 deletions(-)

-- 
1.8.4

[PATCH v2 01/15] staging:iio:hmc5843: Add pointer to i2c client to data struct

[Peter Meerwald]

and use it to simplify code

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

diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
index 76e0cf4..1b4e972 100644
--- a/drivers/staging/iio/magnetometer/hmc5843.c
+++ b/drivers/staging/iio/magnetometer/hmc5843.c
@@ -182,6 +182,7 @@ struct hmc5843_chip_info {
 
 /* Each client has this additional data */
 struct hmc5843_data {
+	struct i2c_client *client;
 	struct mutex lock;
 	u8 rate;
 	u8 meas_conf;
@@ -200,18 +201,15 @@ static s32 hmc5843_configure(struct i2c_client *client,
 }
 
 /* Return the measurement value from the specified channel */
-static int hmc5843_read_measurement(struct iio_dev *indio_dev,
-				    int address,
-				    int *val)
+static int hmc5843_read_measurement(struct hmc5843_data *data,
+				    int address, int *val)
 {
-	struct i2c_client *client = to_i2c_client(indio_dev->dev.parent);
-	struct hmc5843_data *data = iio_priv(indio_dev);
 	s32 result;
 	int tries = 150;
 
 	mutex_lock(&data->lock);
 	while (tries-- > 0) {
-		result = i2c_smbus_read_byte_data(client,
+		result = i2c_smbus_read_byte_data(data->client,
 			HMC5843_STATUS_REG);
 		if (result & HMC5843_DATA_READY)
 			break;
@@ -219,12 +217,12 @@ static int hmc5843_read_measurement(struct iio_dev *indio_dev,
 	}
 
 	if (tries < 0) {
-		dev_err(&client->dev, "data not ready\n");
+		dev_err(&data->client->dev, "data not ready\n");
 		mutex_unlock(&data->lock);
 		return -EIO;
 	}
 
-	result = i2c_smbus_read_word_swapped(client, address);
+	result = i2c_smbus_read_word_swapped(data->client, address);
 	mutex_unlock(&data->lock);
 	if (result < 0)
 		return -EINVAL;
@@ -318,15 +316,13 @@ static IIO_DEVICE_ATTR(operating_mode,
  *     and BN.
  *
  */
-static s32 hmc5843_set_meas_conf(struct i2c_client *client,
-				      u8 meas_conf)
+static s32 hmc5843_set_meas_conf(struct hmc5843_data *data, u8 meas_conf)
 {
-	struct iio_dev *indio_dev = i2c_get_clientdata(client);
-	struct hmc5843_data *data = iio_priv(indio_dev);
 	u8 reg_val;
 	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);
+	return i2c_smbus_write_byte_data(data->client, HMC5843_CONFIG_REG_A,
+		reg_val);
 }
 
 static ssize_t hmc5843_show_measurement_configuration(struct device *dev,
@@ -344,7 +340,6 @@ static ssize_t hmc5843_set_measurement_configuration(struct device *dev,
 						size_t count)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(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;
@@ -357,7 +352,7 @@ static ssize_t hmc5843_set_measurement_configuration(struct device *dev,
 
 	mutex_lock(&data->lock);
 	dev_dbg(dev, "set measurement configuration to %lu\n", meas_conf);
-	if (hmc5843_set_meas_conf(client, meas_conf)) {
+	if (hmc5843_set_meas_conf(data, meas_conf)) {
 		count = -EINVAL;
 		goto exit;
 	}
@@ -396,21 +391,19 @@ static ssize_t hmc5843_show_sampling_frequencies_available(struct device *dev,
 
 static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(hmc5843_show_sampling_frequencies_available);
 
-static s32 hmc5843_set_rate(struct i2c_client *client,
-				u8 rate)
+static s32 hmc5843_set_rate(struct hmc5843_data *data, u8 rate)
 {
-	struct iio_dev *indio_dev = i2c_get_clientdata(client);
-	struct hmc5843_data *data = iio_priv(indio_dev);
 	u8 reg_val;
 
 	if (rate >= HMC5843_RATE_NOT_USED) {
-		dev_err(&client->dev,
+		dev_err(&data->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);
+	return i2c_smbus_write_byte_data(data->client, HMC5843_CONFIG_REG_A,
+		reg_val);
 }
 
 static int hmc5843_check_sampling_frequency(struct hmc5843_data *data,
@@ -433,20 +426,19 @@ static ssize_t hmc5843_set_sampling_frequency(struct device *dev,
 {
 
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct i2c_client *client = to_i2c_client(indio_dev->dev.parent);
 	struct hmc5843_data *data = iio_priv(indio_dev);
 	int rate;
 
 	rate = hmc5843_check_sampling_frequency(data, buf);
 	if (rate < 0) {
-		dev_err(&client->dev,
+		dev_err(&data->client->dev,
 			"sampling frequency is not supported\n");
 		return rate;
 	}
 
 	mutex_lock(&data->lock);
 	dev_dbg(dev, "set rate to %d\n", rate);
-	if (hmc5843_set_rate(client, rate)) {
+	if (hmc5843_set_rate(data->client, rate)) {
 		count = -EINVAL;
 		goto exit;
 	}
@@ -461,12 +453,11 @@ static ssize_t hmc5843_show_sampling_frequency(struct device *dev,
 			struct device_attribute *attr, char *buf)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(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);
+	rate = i2c_smbus_read_byte_data(data->client, this_attr->address);
 	if (rate < 0)
 		return rate;
 	rate = (rate & HMC5843_RATE_BITMASK) >> HMC5843_RATE_OFFSET;
@@ -497,7 +488,6 @@ static ssize_t hmc5843_set_range_gain(struct device *dev,
 			size_t count)
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(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);
 	unsigned long range = 0;
@@ -518,7 +508,7 @@ static ssize_t hmc5843_set_range_gain(struct device *dev,
 
 	data->range = range;
 	range = range << HMC5843_RANGE_GAIN_OFFSET;
-	if (i2c_smbus_write_byte_data(client, this_attr->address, range))
+	if (i2c_smbus_write_byte_data(data->client, this_attr->address, range))
 		count = -EINVAL;
 
 exit:
@@ -541,9 +531,7 @@ static int hmc5843_read_raw(struct iio_dev *indio_dev,
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		return hmc5843_read_measurement(indio_dev,
-						chan->address,
-						val);
+		return hmc5843_read_measurement(data, chan->address, val);
 	case IIO_CHAN_INFO_SCALE:
 		*val = 0;
 		*val2 = data->variant->regval_to_nanoscale[data->range];
@@ -621,8 +609,8 @@ static void hmc5843_init_client(struct i2c_client *client,
 	data->variant = &hmc5843_chip_info_tbl[id->driver_data];
 	indio_dev->channels = data->variant->channels;
 	indio_dev->num_channels = 3;
-	hmc5843_set_meas_conf(client, data->meas_conf);
-	hmc5843_set_rate(client, data->rate);
+	hmc5843_set_meas_conf(data, data->meas_conf);
+	hmc5843_set_rate(data, data->rate);
 	hmc5843_configure(client, data->operating_mode);
 	i2c_smbus_write_byte_data(client, HMC5843_CONFIG_REG_B, data->range);
 	mutex_init(&data->lock);
@@ -649,6 +637,7 @@ static int hmc5843_probe(struct i2c_client *client,
 
 	/* default settings at probe */
 	data = iio_priv(indio_dev);
+	data->client = client;
 	data->meas_conf = HMC5843_MEAS_CONF_NORMAL;
 	data->range = HMC5843_RANGE_GAIN_DEFAULT;
 	data->operating_mode = HMC5843_MODE_CONVERSION_CONTINUOUS;
@@ -687,10 +676,10 @@ static int hmc5843_suspend(struct device *dev)
 
 static int hmc5843_resume(struct device *dev)
 {
-	struct i2c_client *client = to_i2c_client(dev);
-	struct hmc5843_data *data = iio_priv(i2c_get_clientdata(client));
+	struct hmc5843_data *data = iio_priv(i2c_get_clientdata(
+		to_i2c_client(dev)));
 
-	hmc5843_configure(client, data->operating_mode);
+	hmc5843_configure(data->client, data->operating_mode);
 
 	return 0;
 }
-- 
1.8.4

Re: [PATCH v2 01/15] staging:iio:hmc5843: Add pointer to i2c client to data struct

[Jonathan Cameron]

On 09/19/13 20:35, Peter Meerwald wrote:
> and use it to simplify code
>
> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>

  CHECK   drivers/staging/iio/magnetometer/hmc5843.c
drivers/staging/iio/magnetometer/hmc5843.c:441:34: warning: incorrect type in argument 1 (different base types)
drivers/staging/iio/magnetometer/hmc5843.c:441:34:    expected struct hmc5843_data *data
drivers/staging/iio/magnetometer/hmc5843.c:441:34:    got struct i2c_client *client
  CC [M]  drivers/staging/iio/magnetometer/hmc5843.o
drivers/staging/iio/magnetometer/hmc5843.c: In function 'hmc5843_set_sampling_frequency':
drivers/staging/iio/magnetometer/hmc5843.c:441:2: warning: passing argument 1 of 'hmc5843_set_rate' from incompatible
pointer type
drivers/staging/iio/magnetometer/hmc5843.c:394:12: note: expected 'struct hmc5843_data *' but argument is of type
'struct i2c_client *'

You missed one call by the look of it.


> ---
>  drivers/staging/iio/magnetometer/hmc5843.c | 61 ++++++++++++------------------
>  1 file changed, 25 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
> index 76e0cf4..1b4e972 100644
> --- a/drivers/staging/iio/magnetometer/hmc5843.c
> +++ b/drivers/staging/iio/magnetometer/hmc5843.c
> @@ -182,6 +182,7 @@ struct hmc5843_chip_info {
>
>  /* Each client has this additional data */
>  struct hmc5843_data {
> +	struct i2c_client *client;
>  	struct mutex lock;
>  	u8 rate;
>  	u8 meas_conf;
> @@ -200,18 +201,15 @@ static s32 hmc5843_configure(struct i2c_client *client,
>  }
>
>  /* Return the measurement value from the specified channel */
> -static int hmc5843_read_measurement(struct iio_dev *indio_dev,
> -				    int address,
> -				    int *val)
> +static int hmc5843_read_measurement(struct hmc5843_data *data,
> +				    int address, int *val)
>  {
> -	struct i2c_client *client = to_i2c_client(indio_dev->dev.parent);
> -	struct hmc5843_data *data = iio_priv(indio_dev);
>  	s32 result;
>  	int tries = 150;
>
>  	mutex_lock(&data->lock);
>  	while (tries-- > 0) {
> -		result = i2c_smbus_read_byte_data(client,
> +		result = i2c_smbus_read_byte_data(data->client,
>  			HMC5843_STATUS_REG);
>  		if (result & HMC5843_DATA_READY)
>  			break;
> @@ -219,12 +217,12 @@ static int hmc5843_read_measurement(struct iio_dev *indio_dev,
>  	}
>
>  	if (tries < 0) {
> -		dev_err(&client->dev, "data not ready\n");
> +		dev_err(&data->client->dev, "data not ready\n");
>  		mutex_unlock(&data->lock);
>  		return -EIO;
>  	}
>
> -	result = i2c_smbus_read_word_swapped(client, address);
> +	result = i2c_smbus_read_word_swapped(data->client, address);
>  	mutex_unlock(&data->lock);
>  	if (result < 0)
>  		return -EINVAL;
> @@ -318,15 +316,13 @@ static IIO_DEVICE_ATTR(operating_mode,
>   *     and BN.
>   *
>   */
> -static s32 hmc5843_set_meas_conf(struct i2c_client *client,
> -				      u8 meas_conf)
> +static s32 hmc5843_set_meas_conf(struct hmc5843_data *data, u8 meas_conf)
>  {
> -	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> -	struct hmc5843_data *data = iio_priv(indio_dev);
>  	u8 reg_val;
>  	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);
> +	return i2c_smbus_write_byte_data(data->client, HMC5843_CONFIG_REG_A,
> +		reg_val);
>  }
>
>  static ssize_t hmc5843_show_measurement_configuration(struct device *dev,
> @@ -344,7 +340,6 @@ static ssize_t hmc5843_set_measurement_configuration(struct device *dev,
>  						size_t count)
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(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;
> @@ -357,7 +352,7 @@ static ssize_t hmc5843_set_measurement_configuration(struct device *dev,
>
>  	mutex_lock(&data->lock);
>  	dev_dbg(dev, "set measurement configuration to %lu\n", meas_conf);
> -	if (hmc5843_set_meas_conf(client, meas_conf)) {
> +	if (hmc5843_set_meas_conf(data, meas_conf)) {
>  		count = -EINVAL;
>  		goto exit;
>  	}
> @@ -396,21 +391,19 @@ static ssize_t hmc5843_show_sampling_frequencies_available(struct device *dev,
>
>  static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(hmc5843_show_sampling_frequencies_available);
>
> -static s32 hmc5843_set_rate(struct i2c_client *client,
> -				u8 rate)
> +static s32 hmc5843_set_rate(struct hmc5843_data *data, u8 rate)
>  {
> -	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> -	struct hmc5843_data *data = iio_priv(indio_dev);
>  	u8 reg_val;
>
>  	if (rate >= HMC5843_RATE_NOT_USED) {
> -		dev_err(&client->dev,
> +		dev_err(&data->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);
> +	return i2c_smbus_write_byte_data(data->client, HMC5843_CONFIG_REG_A,
> +		reg_val);
>  }
>
>  static int hmc5843_check_sampling_frequency(struct hmc5843_data *data,
> @@ -433,20 +426,19 @@ static ssize_t hmc5843_set_sampling_frequency(struct device *dev,
>  {
>
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> -	struct i2c_client *client = to_i2c_client(indio_dev->dev.parent);
>  	struct hmc5843_data *data = iio_priv(indio_dev);
>  	int rate;
>
>  	rate = hmc5843_check_sampling_frequency(data, buf);
>  	if (rate < 0) {
> -		dev_err(&client->dev,
> +		dev_err(&data->client->dev,
>  			"sampling frequency is not supported\n");
>  		return rate;
>  	}
>
>  	mutex_lock(&data->lock);
>  	dev_dbg(dev, "set rate to %d\n", rate);
> -	if (hmc5843_set_rate(client, rate)) {
> +	if (hmc5843_set_rate(data->client, rate)) {
>  		count = -EINVAL;
>  		goto exit;
>  	}
> @@ -461,12 +453,11 @@ static ssize_t hmc5843_show_sampling_frequency(struct device *dev,
>  			struct device_attribute *attr, char *buf)
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(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);
> +	rate = i2c_smbus_read_byte_data(data->client, this_attr->address);
>  	if (rate < 0)
>  		return rate;
>  	rate = (rate & HMC5843_RATE_BITMASK) >> HMC5843_RATE_OFFSET;
> @@ -497,7 +488,6 @@ static ssize_t hmc5843_set_range_gain(struct device *dev,
>  			size_t count)
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(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);
>  	unsigned long range = 0;
> @@ -518,7 +508,7 @@ static ssize_t hmc5843_set_range_gain(struct device *dev,
>
>  	data->range = range;
>  	range = range << HMC5843_RANGE_GAIN_OFFSET;
> -	if (i2c_smbus_write_byte_data(client, this_attr->address, range))
> +	if (i2c_smbus_write_byte_data(data->client, this_attr->address, range))
>  		count = -EINVAL;
>
>  exit:
> @@ -541,9 +531,7 @@ static int hmc5843_read_raw(struct iio_dev *indio_dev,
>
>  	switch (mask) {
>  	case IIO_CHAN_INFO_RAW:
> -		return hmc5843_read_measurement(indio_dev,
> -						chan->address,
> -						val);
> +		return hmc5843_read_measurement(data, chan->address, val);
>  	case IIO_CHAN_INFO_SCALE:
>  		*val = 0;
>  		*val2 = data->variant->regval_to_nanoscale[data->range];
> @@ -621,8 +609,8 @@ static void hmc5843_init_client(struct i2c_client *client,
>  	data->variant = &hmc5843_chip_info_tbl[id->driver_data];
>  	indio_dev->channels = data->variant->channels;
>  	indio_dev->num_channels = 3;
> -	hmc5843_set_meas_conf(client, data->meas_conf);
> -	hmc5843_set_rate(client, data->rate);
> +	hmc5843_set_meas_conf(data, data->meas_conf);
> +	hmc5843_set_rate(data, data->rate);
>  	hmc5843_configure(client, data->operating_mode);
>  	i2c_smbus_write_byte_data(client, HMC5843_CONFIG_REG_B, data->range);
>  	mutex_init(&data->lock);
> @@ -649,6 +637,7 @@ static int hmc5843_probe(struct i2c_client *client,
>
>  	/* default settings at probe */
>  	data = iio_priv(indio_dev);
> +	data->client = client;
>  	data->meas_conf = HMC5843_MEAS_CONF_NORMAL;
>  	data->range = HMC5843_RANGE_GAIN_DEFAULT;
>  	data->operating_mode = HMC5843_MODE_CONVERSION_CONTINUOUS;
> @@ -687,10 +676,10 @@ static int hmc5843_suspend(struct device *dev)
>
>  static int hmc5843_resume(struct device *dev)
>  {
> -	struct i2c_client *client = to_i2c_client(dev);
> -	struct hmc5843_data *data = iio_priv(i2c_get_clientdata(client));
> +	struct hmc5843_data *data = iio_priv(i2c_get_clientdata(
> +		to_i2c_client(dev)));
>
> -	hmc5843_configure(client, data->operating_mode);
> +	hmc5843_configure(data->client, data->operating_mode);
>
>  	return 0;
>  }
>

Re: [PATCH v2 01/15] staging:iio:hmc5843: Add pointer to i2c client to data struct

[Jonathan Cameron]

On 09/21/13 11:46, Jonathan Cameron wrote:
> On 09/19/13 20:35, Peter Meerwald wrote:
>> and use it to simplify code
>>
>> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
> 
>   CHECK   drivers/staging/iio/magnetometer/hmc5843.c
> drivers/staging/iio/magnetometer/hmc5843.c:441:34: warning: incorrect type in argument 1 (different base types)
> drivers/staging/iio/magnetometer/hmc5843.c:441:34:    expected struct hmc5843_data *data
> drivers/staging/iio/magnetometer/hmc5843.c:441:34:    got struct i2c_client *client
>   CC [M]  drivers/staging/iio/magnetometer/hmc5843.o
> drivers/staging/iio/magnetometer/hmc5843.c: In function 'hmc5843_set_sampling_frequency':
> drivers/staging/iio/magnetometer/hmc5843.c:441:2: warning: passing argument 1 of 'hmc5843_set_rate' from incompatible
> pointer type
> drivers/staging/iio/magnetometer/hmc5843.c:394:12: note: expected 'struct hmc5843_data *' but argument is of type
> 'struct i2c_client *'
> 
> You missed one call by the look of it.

I clearly have too much time on my hands this morning.  Pesky teachers strike and kid
still asleep ;)

Anyhow have fixed the wrong paramter up and applied to the togreg branch of iio.git

Thanks,

Jonathan
> 
> 
>> ---
>>  drivers/staging/iio/magnetometer/hmc5843.c | 61 ++++++++++++------------------
>>  1 file changed, 25 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
>> index 76e0cf4..1b4e972 100644
>> --- a/drivers/staging/iio/magnetometer/hmc5843.c
>> +++ b/drivers/staging/iio/magnetometer/hmc5843.c
>> @@ -182,6 +182,7 @@ struct hmc5843_chip_info {
>>
>>  /* Each client has this additional data */
>>  struct hmc5843_data {
>> +	struct i2c_client *client;
>>  	struct mutex lock;
>>  	u8 rate;
>>  	u8 meas_conf;
>> @@ -200,18 +201,15 @@ static s32 hmc5843_configure(struct i2c_client *client,
>>  }
>>
>>  /* Return the measurement value from the specified channel */
>> -static int hmc5843_read_measurement(struct iio_dev *indio_dev,
>> -				    int address,
>> -				    int *val)
>> +static int hmc5843_read_measurement(struct hmc5843_data *data,
>> +				    int address, int *val)
>>  {
>> -	struct i2c_client *client = to_i2c_client(indio_dev->dev.parent);
>> -	struct hmc5843_data *data = iio_priv(indio_dev);
>>  	s32 result;
>>  	int tries = 150;
>>
>>  	mutex_lock(&data->lock);
>>  	while (tries-- > 0) {
>> -		result = i2c_smbus_read_byte_data(client,
>> +		result = i2c_smbus_read_byte_data(data->client,
>>  			HMC5843_STATUS_REG);
>>  		if (result & HMC5843_DATA_READY)
>>  			break;
>> @@ -219,12 +217,12 @@ static int hmc5843_read_measurement(struct iio_dev *indio_dev,
>>  	}
>>
>>  	if (tries < 0) {
>> -		dev_err(&client->dev, "data not ready\n");
>> +		dev_err(&data->client->dev, "data not ready\n");
>>  		mutex_unlock(&data->lock);
>>  		return -EIO;
>>  	}
>>
>> -	result = i2c_smbus_read_word_swapped(client, address);
>> +	result = i2c_smbus_read_word_swapped(data->client, address);
>>  	mutex_unlock(&data->lock);
>>  	if (result < 0)
>>  		return -EINVAL;
>> @@ -318,15 +316,13 @@ static IIO_DEVICE_ATTR(operating_mode,
>>   *     and BN.
>>   *
>>   */
>> -static s32 hmc5843_set_meas_conf(struct i2c_client *client,
>> -				      u8 meas_conf)
>> +static s32 hmc5843_set_meas_conf(struct hmc5843_data *data, u8 meas_conf)
>>  {
>> -	struct iio_dev *indio_dev = i2c_get_clientdata(client);
>> -	struct hmc5843_data *data = iio_priv(indio_dev);
>>  	u8 reg_val;
>>  	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);
>> +	return i2c_smbus_write_byte_data(data->client, HMC5843_CONFIG_REG_A,
>> +		reg_val);
>>  }
>>
>>  static ssize_t hmc5843_show_measurement_configuration(struct device *dev,
>> @@ -344,7 +340,6 @@ static ssize_t hmc5843_set_measurement_configuration(struct device *dev,
>>  						size_t count)
>>  {
>>  	struct iio_dev *indio_dev = dev_to_iio_dev(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;
>> @@ -357,7 +352,7 @@ static ssize_t hmc5843_set_measurement_configuration(struct device *dev,
>>
>>  	mutex_lock(&data->lock);
>>  	dev_dbg(dev, "set measurement configuration to %lu\n", meas_conf);
>> -	if (hmc5843_set_meas_conf(client, meas_conf)) {
>> +	if (hmc5843_set_meas_conf(data, meas_conf)) {
>>  		count = -EINVAL;
>>  		goto exit;
>>  	}
>> @@ -396,21 +391,19 @@ static ssize_t hmc5843_show_sampling_frequencies_available(struct device *dev,
>>
>>  static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(hmc5843_show_sampling_frequencies_available);
>>
>> -static s32 hmc5843_set_rate(struct i2c_client *client,
>> -				u8 rate)
>> +static s32 hmc5843_set_rate(struct hmc5843_data *data, u8 rate)
>>  {
>> -	struct iio_dev *indio_dev = i2c_get_clientdata(client);
>> -	struct hmc5843_data *data = iio_priv(indio_dev);
>>  	u8 reg_val;
>>
>>  	if (rate >= HMC5843_RATE_NOT_USED) {
>> -		dev_err(&client->dev,
>> +		dev_err(&data->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);
>> +	return i2c_smbus_write_byte_data(data->client, HMC5843_CONFIG_REG_A,
>> +		reg_val);
>>  }
>>
>>  static int hmc5843_check_sampling_frequency(struct hmc5843_data *data,
>> @@ -433,20 +426,19 @@ static ssize_t hmc5843_set_sampling_frequency(struct device *dev,
>>  {
>>
>>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>> -	struct i2c_client *client = to_i2c_client(indio_dev->dev.parent);
>>  	struct hmc5843_data *data = iio_priv(indio_dev);
>>  	int rate;
>>
>>  	rate = hmc5843_check_sampling_frequency(data, buf);
>>  	if (rate < 0) {
>> -		dev_err(&client->dev,
>> +		dev_err(&data->client->dev,
>>  			"sampling frequency is not supported\n");
>>  		return rate;
>>  	}
>>
>>  	mutex_lock(&data->lock);
>>  	dev_dbg(dev, "set rate to %d\n", rate);
>> -	if (hmc5843_set_rate(client, rate)) {
>> +	if (hmc5843_set_rate(data->client, rate)) {
>>  		count = -EINVAL;
>>  		goto exit;
>>  	}
>> @@ -461,12 +453,11 @@ static ssize_t hmc5843_show_sampling_frequency(struct device *dev,
>>  			struct device_attribute *attr, char *buf)
>>  {
>>  	struct iio_dev *indio_dev = dev_to_iio_dev(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);
>> +	rate = i2c_smbus_read_byte_data(data->client, this_attr->address);
>>  	if (rate < 0)
>>  		return rate;
>>  	rate = (rate & HMC5843_RATE_BITMASK) >> HMC5843_RATE_OFFSET;
>> @@ -497,7 +488,6 @@ static ssize_t hmc5843_set_range_gain(struct device *dev,
>>  			size_t count)
>>  {
>>  	struct iio_dev *indio_dev = dev_to_iio_dev(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);
>>  	unsigned long range = 0;
>> @@ -518,7 +508,7 @@ static ssize_t hmc5843_set_range_gain(struct device *dev,
>>
>>  	data->range = range;
>>  	range = range << HMC5843_RANGE_GAIN_OFFSET;
>> -	if (i2c_smbus_write_byte_data(client, this_attr->address, range))
>> +	if (i2c_smbus_write_byte_data(data->client, this_attr->address, range))
>>  		count = -EINVAL;
>>
>>  exit:
>> @@ -541,9 +531,7 @@ static int hmc5843_read_raw(struct iio_dev *indio_dev,
>>
>>  	switch (mask) {
>>  	case IIO_CHAN_INFO_RAW:
>> -		return hmc5843_read_measurement(indio_dev,
>> -						chan->address,
>> -						val);
>> +		return hmc5843_read_measurement(data, chan->address, val);
>>  	case IIO_CHAN_INFO_SCALE:
>>  		*val = 0;
>>  		*val2 = data->variant->regval_to_nanoscale[data->range];
>> @@ -621,8 +609,8 @@ static void hmc5843_init_client(struct i2c_client *client,
>>  	data->variant = &hmc5843_chip_info_tbl[id->driver_data];
>>  	indio_dev->channels = data->variant->channels;
>>  	indio_dev->num_channels = 3;
>> -	hmc5843_set_meas_conf(client, data->meas_conf);
>> -	hmc5843_set_rate(client, data->rate);
>> +	hmc5843_set_meas_conf(data, data->meas_conf);
>> +	hmc5843_set_rate(data, data->rate);
>>  	hmc5843_configure(client, data->operating_mode);
>>  	i2c_smbus_write_byte_data(client, HMC5843_CONFIG_REG_B, data->range);
>>  	mutex_init(&data->lock);
>> @@ -649,6 +637,7 @@ static int hmc5843_probe(struct i2c_client *client,
>>
>>  	/* default settings at probe */
>>  	data = iio_priv(indio_dev);
>> +	data->client = client;
>>  	data->meas_conf = HMC5843_MEAS_CONF_NORMAL;
>>  	data->range = HMC5843_RANGE_GAIN_DEFAULT;
>>  	data->operating_mode = HMC5843_MODE_CONVERSION_CONTINUOUS;
>> @@ -687,10 +676,10 @@ static int hmc5843_suspend(struct device *dev)
>>
>>  static int hmc5843_resume(struct device *dev)
>>  {
>> -	struct i2c_client *client = to_i2c_client(dev);
>> -	struct hmc5843_data *data = iio_priv(i2c_get_clientdata(client));
>> +	struct hmc5843_data *data = iio_priv(i2c_get_clientdata(
>> +		to_i2c_client(dev)));
>>
>> -	hmc5843_configure(client, data->operating_mode);
>> +	hmc5843_configure(data->client, data->operating_mode);
>>
>>  	return 0;
>>  }
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH v2 01/15] staging:iio:hmc5843: Add pointer to i2c client to data struct

[Peter Meerwald]

> I clearly have too much time on my hands this morning.  Pesky teachers strike and kid
> still asleep ;)
> Anyhow have fixed the wrong paramter up and applied to the togreg branch of iio.git

opposite is true for me at the moment; I have yet to fix up the suggested 
scale vs. calibscale cleanup

I think the broken code gets removed in the next commit anyway

regards, p.

-- 

Peter Meerwald
+43-664-2444418 (mobile)

Re: [PATCH v2 01/15] staging:iio:hmc5843: Add pointer to i2c client to data struct

[Jonathan Cameron]

On 10/01/13 09:44, Peter Meerwald wrote:
> 
>> I clearly have too much time on my hands this morning.  Pesky teachers strike and kid
>> still asleep ;)
>> Anyhow have fixed the wrong paramter up and applied to the togreg branch of iio.git
> 
> opposite is true for me at the moment; I have yet to fix up the suggested 
> scale vs. calibscale cleanup
Sure, no particular rush.  I just like to minimize how many patches I have
queued up to make me feel slightly less guilty for the amount of time they
have been sitting there (even if it is only half a set ;)
> 
> I think the broken code gets removed in the next commit anyway
Indeed, it is gone fairly soon after.

The reset of this series are fine so I'll pick those up once you have
done the scale vs calibscale stuff in patch 7.

Thanks,

Jonathan

[PATCH v2 02/15] staging:iio:hmc5843: Rewrite init function

[Peter Meerwald]

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

diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
index 1b4e972..49a105d 100644
--- a/drivers/staging/iio/magnetometer/hmc5843.c
+++ b/drivers/staging/iio/magnetometer/hmc5843.c
@@ -78,6 +78,7 @@ enum hmc5843_ids {
  * HMC5883: Typical data output rate
  */
 #define HMC5843_RATE_OFFSET			0x02
+#define HMC5843_RATE_DEFAULT			0x04
 #define HMC5843_RATE_BITMASK			0x1C
 #define HMC5843_RATE_NOT_USED			0x07
 
@@ -599,23 +600,13 @@ static const struct hmc5843_chip_info hmc5843_chip_info_tbl[] = {
 	},
 };
 
-/* Called when we have found a new HMC58X3 */
-static void hmc5843_init_client(struct i2c_client *client,
-				const struct i2c_device_id *id)
+static void hmc5843_init(struct hmc5843_data *data)
 {
-	struct iio_dev *indio_dev = i2c_get_clientdata(client);
-	struct hmc5843_data *data = iio_priv(indio_dev);
-
-	data->variant = &hmc5843_chip_info_tbl[id->driver_data];
-	indio_dev->channels = data->variant->channels;
-	indio_dev->num_channels = 3;
-	hmc5843_set_meas_conf(data, data->meas_conf);
-	hmc5843_set_rate(data, 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("%s initialized\n", id->name);
+	hmc5843_set_meas_conf(data, HMC5843_MEAS_CONF_NORMAL);
+	hmc5843_set_rate(data, HMC5843_RATE_DEFAULT);
+	hmc5843_configure(data->client, HMC5843_MODE_CONVERSION_CONTINUOUS);
+	i2c_smbus_write_byte_data(data->client, HMC5843_CONFIG_REG_B,
+		HMC5843_RANGE_GAIN_DEFAULT);
 }
 
 static const struct iio_info hmc5843_info = {
@@ -638,17 +629,18 @@ static int hmc5843_probe(struct i2c_client *client,
 	/* default settings at probe */
 	data = iio_priv(indio_dev);
 	data->client = client;
-	data->meas_conf = HMC5843_MEAS_CONF_NORMAL;
-	data->range = HMC5843_RANGE_GAIN_DEFAULT;
-	data->operating_mode = HMC5843_MODE_CONVERSION_CONTINUOUS;
+	data->variant = &hmc5843_chip_info_tbl[id->driver_data];
+	mutex_init(&data->lock);
 
 	i2c_set_clientdata(client, indio_dev);
-	hmc5843_init_client(client, id);
-
 	indio_dev->info = &hmc5843_info;
 	indio_dev->name = id->name;
 	indio_dev->dev.parent = &client->dev;
 	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = data->variant->channels;
+	indio_dev->num_channels = 3;
+
+	hmc5843_init(data);
 
 	err = iio_device_register(indio_dev);
 	if (err)
-- 
1.8.4

Re: [PATCH v2 02/15] staging:iio:hmc5843: Rewrite init function

[Jonathan Cameron]

On 09/19/13 20:35, Peter Meerwald wrote:
> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
Applied to the togreg branch of iio.git

Thanks,
> ---
>  drivers/staging/iio/magnetometer/hmc5843.c | 34 ++++++++++++------------------
>  1 file changed, 13 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
> index 1b4e972..49a105d 100644
> --- a/drivers/staging/iio/magnetometer/hmc5843.c
> +++ b/drivers/staging/iio/magnetometer/hmc5843.c
> @@ -78,6 +78,7 @@ enum hmc5843_ids {
>   * HMC5883: Typical data output rate
>   */
>  #define HMC5843_RATE_OFFSET			0x02
> +#define HMC5843_RATE_DEFAULT			0x04
>  #define HMC5843_RATE_BITMASK			0x1C
>  #define HMC5843_RATE_NOT_USED			0x07
>  
> @@ -599,23 +600,13 @@ static const struct hmc5843_chip_info hmc5843_chip_info_tbl[] = {
>  	},
>  };
>  
> -/* Called when we have found a new HMC58X3 */
> -static void hmc5843_init_client(struct i2c_client *client,
> -				const struct i2c_device_id *id)
> +static void hmc5843_init(struct hmc5843_data *data)
>  {
> -	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> -	struct hmc5843_data *data = iio_priv(indio_dev);
> -
> -	data->variant = &hmc5843_chip_info_tbl[id->driver_data];
> -	indio_dev->channels = data->variant->channels;
> -	indio_dev->num_channels = 3;
> -	hmc5843_set_meas_conf(data, data->meas_conf);
> -	hmc5843_set_rate(data, 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("%s initialized\n", id->name);
> +	hmc5843_set_meas_conf(data, HMC5843_MEAS_CONF_NORMAL);
> +	hmc5843_set_rate(data, HMC5843_RATE_DEFAULT);
> +	hmc5843_configure(data->client, HMC5843_MODE_CONVERSION_CONTINUOUS);
> +	i2c_smbus_write_byte_data(data->client, HMC5843_CONFIG_REG_B,
> +		HMC5843_RANGE_GAIN_DEFAULT);
>  }
>  
>  static const struct iio_info hmc5843_info = {
> @@ -638,17 +629,18 @@ static int hmc5843_probe(struct i2c_client *client,
>  	/* default settings at probe */
>  	data = iio_priv(indio_dev);
>  	data->client = client;
> -	data->meas_conf = HMC5843_MEAS_CONF_NORMAL;
> -	data->range = HMC5843_RANGE_GAIN_DEFAULT;
> -	data->operating_mode = HMC5843_MODE_CONVERSION_CONTINUOUS;
> +	data->variant = &hmc5843_chip_info_tbl[id->driver_data];
> +	mutex_init(&data->lock);
>  
>  	i2c_set_clientdata(client, indio_dev);
> -	hmc5843_init_client(client, id);
> -
>  	indio_dev->info = &hmc5843_info;
>  	indio_dev->name = id->name;
>  	indio_dev->dev.parent = &client->dev;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = data->variant->channels;
> +	indio_dev->num_channels = 3;
> +
> +	hmc5843_init(data);
>  
>  	err = iio_device_register(indio_dev);
>  	if (err)
> 

[PATCH v2 03/15] staging:iio:hmc5843: Use INFO_SAMP_FREQ

[Peter Meerwald]

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

diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
index 49a105d..faeb115 100644
--- a/drivers/staging/iio/magnetometer/hmc5843.c
+++ b/drivers/staging/iio/magnetometer/hmc5843.c
@@ -165,18 +165,19 @@ static const int hmc5883l_regval_to_input_field_mga[] = {
  * 6		| 50			| 75
  * 7		| Not used		| Not used
  */
-static const char * const hmc5843_regval_to_sample_freq[] = {
-	"0.5", "1", "2", "5", "10", "20", "50",
+static const int hmc5843_regval_to_samp_freq[7][2] = {
+	{0, 500000}, {1, 0}, {2, 0}, {5, 0}, {10, 0}, {20, 0}, {50, 0}
 };
 
-static const char * const hmc5883_regval_to_sample_freq[] = {
-	"0.75", "1.5", "3", "7.5", "15", "30", "75",
+static const int hmc5883_regval_to_samp_freq[7][2] = {
+	{0, 750000}, {1, 500000}, {3, 0}, {7, 500000}, {15, 0}, {30, 0},
+	{75, 0}
 };
 
 /* Describe chip variants */
 struct hmc5843_chip_info {
 	const struct iio_chan_spec *channels;
-	const char * const *regval_to_sample_freq;
+	const int (*regval_to_samp_freq)[2];
 	const int *regval_to_input_field_mga;
 	const int *regval_to_nanoscale;
 };
@@ -370,17 +371,17 @@ static IIO_DEVICE_ATTR(meas_conf,
 			hmc5843_set_measurement_configuration,
 			0);
 
-static ssize_t hmc5843_show_sampling_frequencies_available(struct device *dev,
-						struct device_attribute *attr,
-						char *buf)
+static ssize_t hmc5843_show_samp_freq_avail(struct device *dev,
+				struct device_attribute *attr, char *buf)
 {
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct hmc5843_data *data = iio_priv(indio_dev);
+	struct hmc5843_data *data = iio_priv(dev_to_iio_dev(dev));
 	ssize_t total_n = 0;
 	int i;
 
 	for (i = 0; i < HMC5843_RATE_NOT_USED; i++) {
-		ssize_t n = sprintf(buf, "%s ", data->variant->regval_to_sample_freq[i]);
+		ssize_t n = sprintf(buf, "%d.%d ",
+			data->variant->regval_to_samp_freq[i][0],
+			data->variant->regval_to_samp_freq[i][1]);
 		buf += n;
 		total_n += n;
 	}
@@ -390,87 +391,30 @@ static ssize_t hmc5843_show_sampling_frequencies_available(struct device *dev,
 	return total_n;
 }
 
-static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(hmc5843_show_sampling_frequencies_available);
+static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(hmc5843_show_samp_freq_avail);
 
 static s32 hmc5843_set_rate(struct hmc5843_data *data, u8 rate)
 {
-	u8 reg_val;
-
-	if (rate >= HMC5843_RATE_NOT_USED) {
-		dev_err(&data->client->dev,
-			"data output rate is not supported\n");
-		return -EINVAL;
-	}
+	u8 reg_val = data->meas_conf | (rate << HMC5843_RATE_OFFSET);
 
-	reg_val = data->meas_conf | (rate << HMC5843_RATE_OFFSET);
 	return i2c_smbus_write_byte_data(data->client, HMC5843_CONFIG_REG_A,
 		reg_val);
 }
 
-static int hmc5843_check_sampling_frequency(struct hmc5843_data *data,
-						const char *buf)
+static int hmc5843_check_samp_freq(struct hmc5843_data *data,
+	int val, int val2)
 {
-	const char * const *samp_freq = data->variant->regval_to_sample_freq;
 	int i;
 
 	for (i = 0; i < HMC5843_RATE_NOT_USED; i++) {
-		if (sysfs_streq(buf, samp_freq[i]))
+		if (val == data->variant->regval_to_samp_freq[i][0] &&
+			val2 == data->variant->regval_to_samp_freq[i][1])
 			return i;
 	}
 
 	return -EINVAL;
 }
 
-static ssize_t hmc5843_set_sampling_frequency(struct device *dev,
-					struct device_attribute *attr,
-					const char *buf, size_t count)
-{
-
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct hmc5843_data *data = iio_priv(indio_dev);
-	int rate;
-
-	rate = hmc5843_check_sampling_frequency(data, buf);
-	if (rate < 0) {
-		dev_err(&data->client->dev,
-			"sampling frequency is not supported\n");
-		return rate;
-	}
-
-	mutex_lock(&data->lock);
-	dev_dbg(dev, "set rate to %d\n", rate);
-	if (hmc5843_set_rate(data->client, rate)) {
-		count = -EINVAL;
-		goto exit;
-	}
-	data->rate = rate;
-
-exit:
-	mutex_unlock(&data->lock);
-	return count;
-}
-
-static ssize_t hmc5843_show_sampling_frequency(struct device *dev,
-			struct device_attribute *attr, char *buf)
-{
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	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(data->client, this_attr->address);
-	if (rate < 0)
-		return rate;
-	rate = (rate & HMC5843_RATE_BITMASK) >> HMC5843_RATE_OFFSET;
-	return sprintf(buf, "%s\n", data->variant->regval_to_sample_freq[rate]);
-}
-
-static IIO_DEVICE_ATTR(sampling_frequency,
-			S_IWUSR | S_IRUGO,
-			hmc5843_show_sampling_frequency,
-			hmc5843_set_sampling_frequency,
-			HMC5843_CONFIG_REG_A);
-
 static ssize_t hmc5843_show_range_gain(struct device *dev,
 				struct device_attribute *attr,
 				char *buf)
@@ -525,8 +469,7 @@ static IIO_DEVICE_ATTR(in_magn_range,
 
 static int hmc5843_read_raw(struct iio_dev *indio_dev,
 			    struct iio_chan_spec const *chan,
-			    int *val, int *val2,
-			    long mask)
+			    int *val, int *val2, long mask)
 {
 	struct hmc5843_data *data = iio_priv(indio_dev);
 
@@ -537,17 +480,47 @@ static int hmc5843_read_raw(struct iio_dev *indio_dev,
 		*val = 0;
 		*val2 = data->variant->regval_to_nanoscale[data->range];
 		return IIO_VAL_INT_PLUS_NANO;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*val = data->variant->regval_to_samp_freq[data->rate][0];
+		*val2 = data->variant->regval_to_samp_freq[data->rate][1];
+		return IIO_VAL_INT_PLUS_MICRO;
 	}
 	return -EINVAL;
 }
 
+static int hmc5843_write_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int val, int val2, long mask)
+{
+	struct hmc5843_data *data = iio_priv(indio_dev);
+	int ret, rate;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		rate = hmc5843_check_samp_freq(data, val, val2);
+		if (rate < 0)
+			return -EINVAL;
+
+		mutex_lock(&data->lock);
+		ret = hmc5843_set_rate(data, rate);
+		if (ret >= 0)
+			data->rate = rate;
+		mutex_unlock(&data->lock);
+
+		return ret;
+	default:
+		return -EINVAL;
+	}
+}
+
 #define HMC5843_CHANNEL(axis, addr)					\
 	{								\
 		.type = IIO_MAGN,					\
 		.modified = 1,						\
 		.channel2 = IIO_MOD_##axis,				\
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
-		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
+			BIT(IIO_CHAN_INFO_SAMP_FREQ),			\
 		.address = addr						\
 	}
 
@@ -566,7 +539,6 @@ static const struct iio_chan_spec hmc5883_channels[] = {
 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_dev_attr_sampling_frequency_available.dev_attr.attr,
 	NULL
@@ -579,21 +551,21 @@ static const struct attribute_group hmc5843_group = {
 static const struct hmc5843_chip_info hmc5843_chip_info_tbl[] = {
 	[HMC5843_ID] = {
 		.channels = hmc5843_channels,
-		.regval_to_sample_freq = hmc5843_regval_to_sample_freq,
+		.regval_to_samp_freq = hmc5843_regval_to_samp_freq,
 		.regval_to_input_field_mga =
 			hmc5843_regval_to_input_field_mga,
 		.regval_to_nanoscale = hmc5843_regval_to_nanoscale,
 	},
 	[HMC5883_ID] = {
 		.channels = hmc5883_channels,
-		.regval_to_sample_freq = hmc5883_regval_to_sample_freq,
+		.regval_to_samp_freq = hmc5883_regval_to_samp_freq,
 		.regval_to_input_field_mga =
 			hmc5883_regval_to_input_field_mga,
 		.regval_to_nanoscale = hmc5883_regval_to_nanoscale,
 	},
 	[HMC5883L_ID] = {
 		.channels = hmc5883_channels,
-		.regval_to_sample_freq = hmc5883_regval_to_sample_freq,
+		.regval_to_samp_freq = hmc5883_regval_to_samp_freq,
 		.regval_to_input_field_mga =
 			hmc5883l_regval_to_input_field_mga,
 		.regval_to_nanoscale = hmc5883l_regval_to_nanoscale,
@@ -612,6 +584,7 @@ static void hmc5843_init(struct hmc5843_data *data)
 static const struct iio_info hmc5843_info = {
 	.attrs = &hmc5843_group,
 	.read_raw = &hmc5843_read_raw,
+	.write_raw = &hmc5843_write_raw,
 	.driver_module = THIS_MODULE,
 };
 
-- 
1.8.4

Re: [PATCH v2 03/15] staging:iio:hmc5843: Use INFO_SAMP_FREQ

[Jonathan Cameron]

On 09/19/13 20:35, Peter Meerwald wrote:
> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
Applied to the togreg branch of iio.git with a fair bit
of hand editting given the context clash with the fixup I
put in the first patch.

Please check I didn't mess it up!

Jonathan
> ---
>  drivers/staging/iio/magnetometer/hmc5843.c | 135 ++++++++++++-----------------
>  1 file changed, 54 insertions(+), 81 deletions(-)
> 
> diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
> index 49a105d..faeb115 100644
> --- a/drivers/staging/iio/magnetometer/hmc5843.c
> +++ b/drivers/staging/iio/magnetometer/hmc5843.c
> @@ -165,18 +165,19 @@ static const int hmc5883l_regval_to_input_field_mga[] = {
>   * 6		| 50			| 75
>   * 7		| Not used		| Not used
>   */
> -static const char * const hmc5843_regval_to_sample_freq[] = {
> -	"0.5", "1", "2", "5", "10", "20", "50",
> +static const int hmc5843_regval_to_samp_freq[7][2] = {
> +	{0, 500000}, {1, 0}, {2, 0}, {5, 0}, {10, 0}, {20, 0}, {50, 0}
>  };
>  
> -static const char * const hmc5883_regval_to_sample_freq[] = {
> -	"0.75", "1.5", "3", "7.5", "15", "30", "75",
> +static const int hmc5883_regval_to_samp_freq[7][2] = {
> +	{0, 750000}, {1, 500000}, {3, 0}, {7, 500000}, {15, 0}, {30, 0},
> +	{75, 0}
>  };
>  
>  /* Describe chip variants */
>  struct hmc5843_chip_info {
>  	const struct iio_chan_spec *channels;
> -	const char * const *regval_to_sample_freq;
> +	const int (*regval_to_samp_freq)[2];
>  	const int *regval_to_input_field_mga;
>  	const int *regval_to_nanoscale;
>  };
> @@ -370,17 +371,17 @@ static IIO_DEVICE_ATTR(meas_conf,
>  			hmc5843_set_measurement_configuration,
>  			0);
>  
> -static ssize_t hmc5843_show_sampling_frequencies_available(struct device *dev,
> -						struct device_attribute *attr,
> -						char *buf)
> +static ssize_t hmc5843_show_samp_freq_avail(struct device *dev,
> +				struct device_attribute *attr, char *buf)
>  {
> -	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> -	struct hmc5843_data *data = iio_priv(indio_dev);
> +	struct hmc5843_data *data = iio_priv(dev_to_iio_dev(dev));
>  	ssize_t total_n = 0;
>  	int i;
>  
>  	for (i = 0; i < HMC5843_RATE_NOT_USED; i++) {
> -		ssize_t n = sprintf(buf, "%s ", data->variant->regval_to_sample_freq[i]);
> +		ssize_t n = sprintf(buf, "%d.%d ",
> +			data->variant->regval_to_samp_freq[i][0],
> +			data->variant->regval_to_samp_freq[i][1]);
>  		buf += n;
>  		total_n += n;
>  	}
> @@ -390,87 +391,30 @@ static ssize_t hmc5843_show_sampling_frequencies_available(struct device *dev,
>  	return total_n;
>  }
>  
> -static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(hmc5843_show_sampling_frequencies_available);
> +static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(hmc5843_show_samp_freq_avail);
>  
>  static s32 hmc5843_set_rate(struct hmc5843_data *data, u8 rate)
>  {
> -	u8 reg_val;
> -
> -	if (rate >= HMC5843_RATE_NOT_USED) {
> -		dev_err(&data->client->dev,
> -			"data output rate is not supported\n");
> -		return -EINVAL;
> -	}
> +	u8 reg_val = data->meas_conf | (rate << HMC5843_RATE_OFFSET);
>  
> -	reg_val = data->meas_conf | (rate << HMC5843_RATE_OFFSET);
>  	return i2c_smbus_write_byte_data(data->client, HMC5843_CONFIG_REG_A,
>  		reg_val);
>  }
>  
> -static int hmc5843_check_sampling_frequency(struct hmc5843_data *data,
> -						const char *buf)
> +static int hmc5843_check_samp_freq(struct hmc5843_data *data,
> +	int val, int val2)
>  {
> -	const char * const *samp_freq = data->variant->regval_to_sample_freq;
>  	int i;
>  
>  	for (i = 0; i < HMC5843_RATE_NOT_USED; i++) {
> -		if (sysfs_streq(buf, samp_freq[i]))
> +		if (val == data->variant->regval_to_samp_freq[i][0] &&
> +			val2 == data->variant->regval_to_samp_freq[i][1])
>  			return i;
>  	}
>  
>  	return -EINVAL;
>  }
>  
> -static ssize_t hmc5843_set_sampling_frequency(struct device *dev,
> -					struct device_attribute *attr,
> -					const char *buf, size_t count)
> -{
> -
> -	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> -	struct hmc5843_data *data = iio_priv(indio_dev);
> -	int rate;
> -
> -	rate = hmc5843_check_sampling_frequency(data, buf);
> -	if (rate < 0) {
> -		dev_err(&data->client->dev,
> -			"sampling frequency is not supported\n");
> -		return rate;
> -	}
> -
> -	mutex_lock(&data->lock);
> -	dev_dbg(dev, "set rate to %d\n", rate);
> -	if (hmc5843_set_rate(data->client, rate)) {
> -		count = -EINVAL;
> -		goto exit;
> -	}
> -	data->rate = rate;
> -
> -exit:
> -	mutex_unlock(&data->lock);
> -	return count;
> -}
> -
> -static ssize_t hmc5843_show_sampling_frequency(struct device *dev,
> -			struct device_attribute *attr, char *buf)
> -{
> -	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> -	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(data->client, this_attr->address);
> -	if (rate < 0)
> -		return rate;
> -	rate = (rate & HMC5843_RATE_BITMASK) >> HMC5843_RATE_OFFSET;
> -	return sprintf(buf, "%s\n", data->variant->regval_to_sample_freq[rate]);
> -}
> -
> -static IIO_DEVICE_ATTR(sampling_frequency,
> -			S_IWUSR | S_IRUGO,
> -			hmc5843_show_sampling_frequency,
> -			hmc5843_set_sampling_frequency,
> -			HMC5843_CONFIG_REG_A);
> -
>  static ssize_t hmc5843_show_range_gain(struct device *dev,
>  				struct device_attribute *attr,
>  				char *buf)
> @@ -525,8 +469,7 @@ static IIO_DEVICE_ATTR(in_magn_range,
>  
>  static int hmc5843_read_raw(struct iio_dev *indio_dev,
>  			    struct iio_chan_spec const *chan,
> -			    int *val, int *val2,
> -			    long mask)
> +			    int *val, int *val2, long mask)
>  {
>  	struct hmc5843_data *data = iio_priv(indio_dev);
>  
> @@ -537,17 +480,47 @@ static int hmc5843_read_raw(struct iio_dev *indio_dev,
>  		*val = 0;
>  		*val2 = data->variant->regval_to_nanoscale[data->range];
>  		return IIO_VAL_INT_PLUS_NANO;
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		*val = data->variant->regval_to_samp_freq[data->rate][0];
> +		*val2 = data->variant->regval_to_samp_freq[data->rate][1];
> +		return IIO_VAL_INT_PLUS_MICRO;
>  	}
>  	return -EINVAL;
>  }
>  
> +static int hmc5843_write_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     int val, int val2, long mask)
> +{
> +	struct hmc5843_data *data = iio_priv(indio_dev);
> +	int ret, rate;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		rate = hmc5843_check_samp_freq(data, val, val2);
> +		if (rate < 0)
> +			return -EINVAL;
> +
> +		mutex_lock(&data->lock);
> +		ret = hmc5843_set_rate(data, rate);
> +		if (ret >= 0)
> +			data->rate = rate;
> +		mutex_unlock(&data->lock);
> +
> +		return ret;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
>  #define HMC5843_CHANNEL(axis, addr)					\
>  	{								\
>  		.type = IIO_MAGN,					\
>  		.modified = 1,						\
>  		.channel2 = IIO_MOD_##axis,				\
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
> -		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
> +			BIT(IIO_CHAN_INFO_SAMP_FREQ),			\
>  		.address = addr						\
>  	}
>  
> @@ -566,7 +539,6 @@ static const struct iio_chan_spec hmc5883_channels[] = {
>  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_dev_attr_sampling_frequency_available.dev_attr.attr,
>  	NULL
> @@ -579,21 +551,21 @@ static const struct attribute_group hmc5843_group = {
>  static const struct hmc5843_chip_info hmc5843_chip_info_tbl[] = {
>  	[HMC5843_ID] = {
>  		.channels = hmc5843_channels,
> -		.regval_to_sample_freq = hmc5843_regval_to_sample_freq,
> +		.regval_to_samp_freq = hmc5843_regval_to_samp_freq,
>  		.regval_to_input_field_mga =
>  			hmc5843_regval_to_input_field_mga,
>  		.regval_to_nanoscale = hmc5843_regval_to_nanoscale,
>  	},
>  	[HMC5883_ID] = {
>  		.channels = hmc5883_channels,
> -		.regval_to_sample_freq = hmc5883_regval_to_sample_freq,
> +		.regval_to_samp_freq = hmc5883_regval_to_samp_freq,
>  		.regval_to_input_field_mga =
>  			hmc5883_regval_to_input_field_mga,
>  		.regval_to_nanoscale = hmc5883_regval_to_nanoscale,
>  	},
>  	[HMC5883L_ID] = {
>  		.channels = hmc5883_channels,
> -		.regval_to_sample_freq = hmc5883_regval_to_sample_freq,
> +		.regval_to_samp_freq = hmc5883_regval_to_samp_freq,
>  		.regval_to_input_field_mga =
>  			hmc5883l_regval_to_input_field_mga,
>  		.regval_to_nanoscale = hmc5883l_regval_to_nanoscale,
> @@ -612,6 +584,7 @@ static void hmc5843_init(struct hmc5843_data *data)
>  static const struct iio_info hmc5843_info = {
>  	.attrs = &hmc5843_group,
>  	.read_raw = &hmc5843_read_raw,
> +	.write_raw = &hmc5843_write_raw,
>  	.driver_module = THIS_MODULE,
>  };
>  
> 

[PATCH v2 04/15] staging:iio:hmc5843: Remove unused LSB register #defines

[Peter Meerwald]

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

diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
index faeb115..d953107 100644
--- a/drivers/staging/iio/magnetometer/hmc5843.c
+++ b/drivers/staging/iio/magnetometer/hmc5843.c
@@ -29,16 +29,11 @@
 #define HMC5843_CONFIG_REG_B			0x01
 #define HMC5843_MODE_REG			0x02
 #define HMC5843_DATA_OUT_X_MSB_REG		0x03
-#define HMC5843_DATA_OUT_X_LSB_REG		0x04
 #define HMC5843_DATA_OUT_Y_MSB_REG		0x05
-#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
 
 enum hmc5843_ids {
@@ -61,8 +56,6 @@ enum hmc5843_ids {
  */
 #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
-- 
1.8.4

Re: [PATCH v2 04/15] staging:iio:hmc5843: Remove unused LSB register #defines

[Jonathan Cameron]

On 09/19/13 20:35, Peter Meerwald wrote:
> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
Applied, thanks.
> ---
>  drivers/staging/iio/magnetometer/hmc5843.c | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
> index faeb115..d953107 100644
> --- a/drivers/staging/iio/magnetometer/hmc5843.c
> +++ b/drivers/staging/iio/magnetometer/hmc5843.c
> @@ -29,16 +29,11 @@
>  #define HMC5843_CONFIG_REG_B			0x01
>  #define HMC5843_MODE_REG			0x02
>  #define HMC5843_DATA_OUT_X_MSB_REG		0x03
> -#define HMC5843_DATA_OUT_X_LSB_REG		0x04
>  #define HMC5843_DATA_OUT_Y_MSB_REG		0x05
> -#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
>  
>  enum hmc5843_ids {
> @@ -61,8 +56,6 @@ enum hmc5843_ids {
>   */
>  #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
> 

[PATCH v2 05/15] staging:iio:hmc5843: Tighten comments

[Peter Meerwald]

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

diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
index d953107..2d89549 100644
--- a/drivers/staging/iio/magnetometer/hmc5843.c
+++ b/drivers/staging/iio/magnetometer/hmc5843.c
@@ -51,15 +51,11 @@ enum hmc5843_ids {
 #define HMC5843_RANGE_GAIN_DEFAULT		0x01
 #define HMC5843_RANGE_GAIN_MAX			0x07
 
-/*
- * Device status
- */
+/* Device status */
 #define HMC5843_DATA_READY			0x01
 #define HMC5843_DATA_OUTPUT_LOCK		0x02
 
-/*
- * Mode register configuration
- */
+/* Mode register configuration */
 #define HMC5843_MODE_CONVERSION_CONTINUOUS	0x00
 #define HMC5843_MODE_CONVERSION_SINGLE		0x01
 #define HMC5843_MODE_IDLE			0x02
@@ -75,18 +71,14 @@ enum hmc5843_ids {
 #define HMC5843_RATE_BITMASK			0x1C
 #define HMC5843_RATE_NOT_USED			0x07
 
-/*
- * Device measurement configuration
- */
+/* Device measurement configuration */
 #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
 
-/*
- * Scaling factors: 10000000/Gain
- */
+/* Scaling factors: 10000000/Gain */
 static const int hmc5843_regval_to_nanoscale[] = {
 	6173, 7692, 10309, 12821, 18868, 21739, 25641, 35714
 };
-- 
1.8.4

[PATCH v2 06/15] staging:iio:hmc5843: Introduce helper functions to show/check list of int pairs

[Peter Meerwald]

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

diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
index 2d89549..71f26de 100644
--- a/drivers/staging/iio/magnetometer/hmc5843.c
+++ b/drivers/staging/iio/magnetometer/hmc5843.c
@@ -356,24 +356,42 @@ static IIO_DEVICE_ATTR(meas_conf,
 			hmc5843_set_measurement_configuration,
 			0);
 
-static ssize_t hmc5843_show_samp_freq_avail(struct device *dev,
-				struct device_attribute *attr, char *buf)
+static ssize_t hmc5843_show_int_plus_micros(char *buf,
+	const int (*vals)[2], int n)
 {
-	struct hmc5843_data *data = iio_priv(dev_to_iio_dev(dev));
-	ssize_t total_n = 0;
+	size_t len = 0;
 	int i;
 
-	for (i = 0; i < HMC5843_RATE_NOT_USED; i++) {
-		ssize_t n = sprintf(buf, "%d.%d ",
-			data->variant->regval_to_samp_freq[i][0],
-			data->variant->regval_to_samp_freq[i][1]);
-		buf += n;
-		total_n += n;
-	}
+	for (i = 0; i < n; i++)
+		len += scnprintf(buf + len, PAGE_SIZE - len,
+			"%d.%d ", vals[i][0], vals[i][1]);
+
 	/* replace trailing space by newline */
-	buf[-1] = '\n';
+	buf[len - 1] = '\n';
+
+	return len;
+}
 
-	return total_n;
+static int hmc5843_check_int_plus_micros(const int (*vals)[2], int n,
+					int val, int val2)
+{
+	int i;
+
+	for (i = 0; i < n; i++) {
+		if (val == vals[i][0] && val2 == vals[i][1])
+			return i;
+	}
+
+	return -EINVAL;
+}
+
+static ssize_t hmc5843_show_samp_freq_avail(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct hmc5843_data *data = iio_priv(dev_to_iio_dev(dev));
+
+	return hmc5843_show_int_plus_micros(buf,
+		data->variant->regval_to_samp_freq, HMC5843_RATE_NOT_USED);
 }
 
 static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(hmc5843_show_samp_freq_avail);
@@ -389,15 +407,9 @@ static s32 hmc5843_set_rate(struct hmc5843_data *data, u8 rate)
 static int hmc5843_check_samp_freq(struct hmc5843_data *data,
 	int val, int val2)
 {
-	int i;
-
-	for (i = 0; i < HMC5843_RATE_NOT_USED; i++) {
-		if (val == data->variant->regval_to_samp_freq[i][0] &&
-			val2 == data->variant->regval_to_samp_freq[i][1])
-			return i;
-	}
-
-	return -EINVAL;
+	return hmc5843_check_int_plus_micros(
+		data->variant->regval_to_samp_freq, HMC5843_RATE_NOT_USED,
+		val, val2);
 }
 
 static ssize_t hmc5843_show_range_gain(struct device *dev,
-- 
1.8.4

Re: [PATCH v2 06/15] staging:iio:hmc5843: Introduce helper functions to show/check list of int pairs

[Jonathan Cameron]

On 09/19/13 20:35, Peter Meerwald wrote:
> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
Applied, thanks
> ---
>  drivers/staging/iio/magnetometer/hmc5843.c | 56 ++++++++++++++++++------------
>  1 file changed, 34 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
> index 2d89549..71f26de 100644
> --- a/drivers/staging/iio/magnetometer/hmc5843.c
> +++ b/drivers/staging/iio/magnetometer/hmc5843.c
> @@ -356,24 +356,42 @@ static IIO_DEVICE_ATTR(meas_conf,
>  			hmc5843_set_measurement_configuration,
>  			0);
>  
> -static ssize_t hmc5843_show_samp_freq_avail(struct device *dev,
> -				struct device_attribute *attr, char *buf)
> +static ssize_t hmc5843_show_int_plus_micros(char *buf,
> +	const int (*vals)[2], int n)
>  {
> -	struct hmc5843_data *data = iio_priv(dev_to_iio_dev(dev));
> -	ssize_t total_n = 0;
> +	size_t len = 0;
>  	int i;
>  
> -	for (i = 0; i < HMC5843_RATE_NOT_USED; i++) {
> -		ssize_t n = sprintf(buf, "%d.%d ",
> -			data->variant->regval_to_samp_freq[i][0],
> -			data->variant->regval_to_samp_freq[i][1]);
> -		buf += n;
> -		total_n += n;
> -	}
> +	for (i = 0; i < n; i++)
> +		len += scnprintf(buf + len, PAGE_SIZE - len,
> +			"%d.%d ", vals[i][0], vals[i][1]);
> +
>  	/* replace trailing space by newline */
> -	buf[-1] = '\n';
> +	buf[len - 1] = '\n';
> +
> +	return len;
> +}
>  
> -	return total_n;
> +static int hmc5843_check_int_plus_micros(const int (*vals)[2], int n,
> +					int val, int val2)
> +{
> +	int i;
> +
> +	for (i = 0; i < n; i++) {
> +		if (val == vals[i][0] && val2 == vals[i][1])
> +			return i;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static ssize_t hmc5843_show_samp_freq_avail(struct device *dev,
> +				struct device_attribute *attr, char *buf)
> +{
> +	struct hmc5843_data *data = iio_priv(dev_to_iio_dev(dev));
> +
> +	return hmc5843_show_int_plus_micros(buf,
> +		data->variant->regval_to_samp_freq, HMC5843_RATE_NOT_USED);
>  }
>  
>  static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(hmc5843_show_samp_freq_avail);
> @@ -389,15 +407,9 @@ static s32 hmc5843_set_rate(struct hmc5843_data *data, u8 rate)
>  static int hmc5843_check_samp_freq(struct hmc5843_data *data,
>  	int val, int val2)
>  {
> -	int i;
> -
> -	for (i = 0; i < HMC5843_RATE_NOT_USED; i++) {
> -		if (val == data->variant->regval_to_samp_freq[i][0] &&
> -			val2 == data->variant->regval_to_samp_freq[i][1])
> -			return i;
> -	}
> -
> -	return -EINVAL;
> +	return hmc5843_check_int_plus_micros(
> +		data->variant->regval_to_samp_freq, HMC5843_RATE_NOT_USED,
> +		val, val2);
>  }
>  
>  static ssize_t hmc5843_show_range_gain(struct device *dev,
> 

Re: [PATCH v2 06/15] staging:iio:hmc5843: Introduce helper functions to show/check list of int pairs

[Jonathan Cameron]

On 10/01/13 10:55, Jonathan Cameron wrote:
> On 09/19/13 20:35, Peter Meerwald wrote:
>> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
> Applied, thanks
Little premature.  I had slightly different alignment of some
code in one of the patches I had merged, so this needed a bit of hand
editting as well.  Result is the same after both those patches are applied
though!


>> ---
>>  drivers/staging/iio/magnetometer/hmc5843.c | 56 ++++++++++++++++++------------
>>  1 file changed, 34 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
>> index 2d89549..71f26de 100644
>> --- a/drivers/staging/iio/magnetometer/hmc5843.c
>> +++ b/drivers/staging/iio/magnetometer/hmc5843.c
>> @@ -356,24 +356,42 @@ static IIO_DEVICE_ATTR(meas_conf,
>>  			hmc5843_set_measurement_configuration,
>>  			0);
>>  
>> -static ssize_t hmc5843_show_samp_freq_avail(struct device *dev,
>> -				struct device_attribute *attr, char *buf)
>> +static ssize_t hmc5843_show_int_plus_micros(char *buf,
>> +	const int (*vals)[2], int n)
>>  {
>> -	struct hmc5843_data *data = iio_priv(dev_to_iio_dev(dev));
>> -	ssize_t total_n = 0;
>> +	size_t len = 0;
>>  	int i;
>>  
>> -	for (i = 0; i < HMC5843_RATE_NOT_USED; i++) {
>> -		ssize_t n = sprintf(buf, "%d.%d ",
>> -			data->variant->regval_to_samp_freq[i][0],
>> -			data->variant->regval_to_samp_freq[i][1]);
>> -		buf += n;
>> -		total_n += n;
>> -	}
>> +	for (i = 0; i < n; i++)
>> +		len += scnprintf(buf + len, PAGE_SIZE - len,
>> +			"%d.%d ", vals[i][0], vals[i][1]);
>> +
>>  	/* replace trailing space by newline */
>> -	buf[-1] = '\n';
>> +	buf[len - 1] = '\n';
>> +
>> +	return len;
>> +}
>>  
>> -	return total_n;
>> +static int hmc5843_check_int_plus_micros(const int (*vals)[2], int n,
>> +					int val, int val2)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < n; i++) {
>> +		if (val == vals[i][0] && val2 == vals[i][1])
>> +			return i;
>> +	}
>> +
>> +	return -EINVAL;
>> +}
>> +
>> +static ssize_t hmc5843_show_samp_freq_avail(struct device *dev,
>> +				struct device_attribute *attr, char *buf)
>> +{
>> +	struct hmc5843_data *data = iio_priv(dev_to_iio_dev(dev));
>> +
>> +	return hmc5843_show_int_plus_micros(buf,
>> +		data->variant->regval_to_samp_freq, HMC5843_RATE_NOT_USED);
>>  }
>>  
>>  static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(hmc5843_show_samp_freq_avail);
>> @@ -389,15 +407,9 @@ static s32 hmc5843_set_rate(struct hmc5843_data *data, u8 rate)
>>  static int hmc5843_check_samp_freq(struct hmc5843_data *data,
>>  	int val, int val2)
>>  {
>> -	int i;
>> -
>> -	for (i = 0; i < HMC5843_RATE_NOT_USED; i++) {
>> -		if (val == data->variant->regval_to_samp_freq[i][0] &&
>> -			val2 == data->variant->regval_to_samp_freq[i][1])
>> -			return i;
>> -	}
>> -
>> -	return -EINVAL;
>> +	return hmc5843_check_int_plus_micros(
>> +		data->variant->regval_to_samp_freq, HMC5843_RATE_NOT_USED,
>> +		val, val2);
>>  }
>>  
>>  static ssize_t hmc5843_show_range_gain(struct device *dev,
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[PATCH v2 07/15] staging:iio:hmc5843: Use CALIBSCALE instead of magn_range

[Peter Meerwald]

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

diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
index 71f26de..0c3f41e 100644
--- a/drivers/staging/iio/magnetometer/hmc5843.c
+++ b/drivers/staging/iio/magnetometer/hmc5843.c
@@ -125,16 +125,19 @@ static const int hmc5883l_regval_to_nanoscale[] = {
  * 6		| (+-)5.6				| 330
  * 7		| (+-)8.1				| 230
  */
-static const int hmc5843_regval_to_input_field_mga[] = {
-	700, 1000, 1500, 2000, 3200, 3800, 4500, 6500
+static const int hmc5843_regval_to_range_ga[8][2] = {
+	{0, 700000}, {1, 0}, {1, 500000}, {2, 0}, {3, 200000},
+	{3, 800000}, {4, 500000}, {6, 500000}
 };
 
-static const int hmc5883_regval_to_input_field_mga[] = {
-	900, 1200, 1900, 2500, 4000, 4600, 5500, 7900
+static const int hmc5883_regval_to_range_ga[8][2] = {
+	{0, 900000}, {1, 200000}, {1, 900000}, {2, 500000}, {4, 0},
+	{4, 600000}, {5, 500000}, {7, 900000}
 };
 
-static const int hmc5883l_regval_to_input_field_mga[] = {
-	880, 1300, 1900, 2500, 4000, 4700, 5600, 8100
+static const int hmc5883l_regval_to_range_ga[8][2] = {
+	{0, 880000}, {1, 300000}, {1, 900000}, {2, 500000}, {4, 0},
+	{4, 700000}, {5, 600000}, {8, 100000}
 };
 
 /*
@@ -163,7 +166,7 @@ static const int hmc5883_regval_to_samp_freq[7][2] = {
 struct hmc5843_chip_info {
 	const struct iio_chan_spec *channels;
 	const int (*regval_to_samp_freq)[2];
-	const int *regval_to_input_field_mga;
+	const int (*regval_to_range_ga)[2];
 	const int *regval_to_nanoscale;
 };
 
@@ -412,58 +415,25 @@ static int hmc5843_check_samp_freq(struct hmc5843_data *data,
 		val, val2);
 }
 
-static ssize_t hmc5843_show_range_gain(struct device *dev,
-				struct device_attribute *attr,
-				char *buf)
+static ssize_t hmc5843_show_calibscale_avail(struct device *dev,
+				struct device_attribute *attr, char *buf)
 {
-	u8 range;
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct hmc5843_data *data = iio_priv(indio_dev);
+	struct hmc5843_data *data = iio_priv(dev_to_iio_dev(dev));
 
-	range = data->range;
-	return sprintf(buf, "%d\n", data->variant->regval_to_input_field_mga[range]);
+	return hmc5843_show_int_plus_micros(buf,
+		data->variant->regval_to_range_ga, HMC5843_RANGE_GAIN_MAX+1);
 }
 
-static ssize_t hmc5843_set_range_gain(struct device *dev,
-			struct device_attribute *attr,
-			const char *buf,
-			size_t count)
-{
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
-	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) {
-		count = error;
-		goto exit;
-	}
-	dev_dbg(dev, "set range to %lu\n", range);
+static IIO_DEVICE_ATTR(calibscale_available, S_IRUGO,
+	hmc5843_show_calibscale_avail, NULL, 0);
 
-	if (range > HMC5843_RANGE_GAIN_MAX) {
-		count = -EINVAL;
-		goto exit;
-	}
-
-	data->range = range;
-	range = range << HMC5843_RANGE_GAIN_OFFSET;
-	if (i2c_smbus_write_byte_data(data->client, this_attr->address, range))
-		count = -EINVAL;
-
-exit:
-	mutex_unlock(&data->lock);
-	return count;
+static int hmc5843_check_range(struct hmc5843_data *data,
+	int val, int val2)
+{
+	return hmc5843_check_int_plus_micros(data->variant->regval_to_range_ga,
+		HMC5843_RANGE_GAIN_MAX+1, val, val2);
 }
 
-static IIO_DEVICE_ATTR(in_magn_range,
-			S_IWUSR | S_IRUGO,
-			hmc5843_show_range_gain,
-			hmc5843_set_range_gain,
-			HMC5843_CONFIG_REG_B);
-
 static int hmc5843_read_raw(struct iio_dev *indio_dev,
 			    struct iio_chan_spec const *chan,
 			    int *val, int *val2, long mask)
@@ -481,6 +451,10 @@ static int hmc5843_read_raw(struct iio_dev *indio_dev,
 		*val = data->variant->regval_to_samp_freq[data->rate][0];
 		*val2 = data->variant->regval_to_samp_freq[data->rate][1];
 		return IIO_VAL_INT_PLUS_MICRO;
+	case IIO_CHAN_INFO_CALIBSCALE:
+		*val = data->variant->regval_to_range_ga[data->range][0];
+		*val2 = data->variant->regval_to_range_ga[data->range][1];
+		return IIO_VAL_INT_PLUS_MICRO;
 	}
 	return -EINVAL;
 }
@@ -490,7 +464,7 @@ static int hmc5843_write_raw(struct iio_dev *indio_dev,
 			     int val, int val2, long mask)
 {
 	struct hmc5843_data *data = iio_priv(indio_dev);
-	int ret, rate;
+	int ret, rate, range;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_SAMP_FREQ:
@@ -505,6 +479,20 @@ static int hmc5843_write_raw(struct iio_dev *indio_dev,
 		mutex_unlock(&data->lock);
 
 		return ret;
+	case IIO_CHAN_INFO_CALIBSCALE:
+		range = hmc5843_check_range(data, val, val2);
+		if (range < 0)
+			return -EINVAL;
+
+		range <<= HMC5843_RANGE_GAIN_OFFSET;
+		mutex_lock(&data->lock);
+		ret = i2c_smbus_write_byte_data(data->client,
+			HMC5843_CONFIG_REG_B, range);
+		if (ret >= 0)
+			data->range = range;
+		mutex_unlock(&data->lock);
+
+		return ret;
 	default:
 		return -EINVAL;
 	}
@@ -517,7 +505,8 @@ static int hmc5843_write_raw(struct iio_dev *indio_dev,
 		.channel2 = IIO_MOD_##axis,				\
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
 		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
-			BIT(IIO_CHAN_INFO_SAMP_FREQ),			\
+			BIT(IIO_CHAN_INFO_SAMP_FREQ) |			\
+			BIT(IIO_CHAN_INFO_CALIBSCALE),			\
 		.address = addr						\
 	}
 
@@ -536,7 +525,7 @@ static const struct iio_chan_spec hmc5883_channels[] = {
 static struct attribute *hmc5843_attributes[] = {
 	&iio_dev_attr_meas_conf.dev_attr.attr,
 	&iio_dev_attr_operating_mode.dev_attr.attr,
-	&iio_dev_attr_in_magn_range.dev_attr.attr,
+	&iio_dev_attr_calibscale_available.dev_attr.attr,
 	&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
 	NULL
 };
@@ -549,22 +538,19 @@ static const struct hmc5843_chip_info hmc5843_chip_info_tbl[] = {
 	[HMC5843_ID] = {
 		.channels = hmc5843_channels,
 		.regval_to_samp_freq = hmc5843_regval_to_samp_freq,
-		.regval_to_input_field_mga =
-			hmc5843_regval_to_input_field_mga,
+		.regval_to_range_ga = hmc5843_regval_to_range_ga,
 		.regval_to_nanoscale = hmc5843_regval_to_nanoscale,
 	},
 	[HMC5883_ID] = {
 		.channels = hmc5883_channels,
 		.regval_to_samp_freq = hmc5883_regval_to_samp_freq,
-		.regval_to_input_field_mga =
-			hmc5883_regval_to_input_field_mga,
+		.regval_to_range_ga = hmc5883_regval_to_range_ga,
 		.regval_to_nanoscale = hmc5883_regval_to_nanoscale,
 	},
 	[HMC5883L_ID] = {
 		.channels = hmc5883_channels,
 		.regval_to_samp_freq = hmc5883_regval_to_samp_freq,
-		.regval_to_input_field_mga =
-			hmc5883l_regval_to_input_field_mga,
+		.regval_to_range_ga = hmc5883l_regval_to_range_ga,
 		.regval_to_nanoscale = hmc5883l_regval_to_nanoscale,
 	},
 };
-- 
1.8.4

Re: [PATCH v2 07/15] staging:iio:hmc5843: Use CALIBSCALE instead of magn_range

[Jonathan Cameron]

On 09/19/13 20:35, Peter Meerwald wrote:
> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
Hi Peter,

I'm afraid I don't follow why you don't just make scale writeable
rather than introducing a second element to control the same thing.
Calibscale is intended for the case where the hardware can change
the scale without it being apparent from the raw readings. Sometimes
it also gets used to handling amplifiers in cases where the conversion
function is 'interesting' and we have no choice but to do it in kernel
in order to have data provided to userspace in a comprehensible form
but that doesn't apply here.

Jonathan

> ---
>  drivers/staging/iio/magnetometer/hmc5843.c | 108 +++++++++++++----------------
>  1 file changed, 47 insertions(+), 61 deletions(-)
> 
> diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
> index 71f26de..0c3f41e 100644
> --- a/drivers/staging/iio/magnetometer/hmc5843.c
> +++ b/drivers/staging/iio/magnetometer/hmc5843.c
> @@ -125,16 +125,19 @@ static const int hmc5883l_regval_to_nanoscale[] = {
>   * 6		| (+-)5.6				| 330
>   * 7		| (+-)8.1				| 230
>   */
> -static const int hmc5843_regval_to_input_field_mga[] = {
> -	700, 1000, 1500, 2000, 3200, 3800, 4500, 6500
> +static const int hmc5843_regval_to_range_ga[8][2] = {
> +	{0, 700000}, {1, 0}, {1, 500000}, {2, 0}, {3, 200000},
> +	{3, 800000}, {4, 500000}, {6, 500000}
>  };
>  
> -static const int hmc5883_regval_to_input_field_mga[] = {
> -	900, 1200, 1900, 2500, 4000, 4600, 5500, 7900
> +static const int hmc5883_regval_to_range_ga[8][2] = {
> +	{0, 900000}, {1, 200000}, {1, 900000}, {2, 500000}, {4, 0},
> +	{4, 600000}, {5, 500000}, {7, 900000}
>  };
>  
> -static const int hmc5883l_regval_to_input_field_mga[] = {
> -	880, 1300, 1900, 2500, 4000, 4700, 5600, 8100
> +static const int hmc5883l_regval_to_range_ga[8][2] = {
> +	{0, 880000}, {1, 300000}, {1, 900000}, {2, 500000}, {4, 0},
> +	{4, 700000}, {5, 600000}, {8, 100000}
>  };
>  
>  /*
> @@ -163,7 +166,7 @@ static const int hmc5883_regval_to_samp_freq[7][2] = {
>  struct hmc5843_chip_info {
>  	const struct iio_chan_spec *channels;
>  	const int (*regval_to_samp_freq)[2];
> -	const int *regval_to_input_field_mga;
> +	const int (*regval_to_range_ga)[2];
>  	const int *regval_to_nanoscale;
>  };
>  
> @@ -412,58 +415,25 @@ static int hmc5843_check_samp_freq(struct hmc5843_data *data,
>  		val, val2);
>  }
>  
> -static ssize_t hmc5843_show_range_gain(struct device *dev,
> -				struct device_attribute *attr,
> -				char *buf)
> +static ssize_t hmc5843_show_calibscale_avail(struct device *dev,
> +				struct device_attribute *attr, char *buf)
>  {
> -	u8 range;
> -	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> -	struct hmc5843_data *data = iio_priv(indio_dev);
> +	struct hmc5843_data *data = iio_priv(dev_to_iio_dev(dev));
>  
> -	range = data->range;
> -	return sprintf(buf, "%d\n", data->variant->regval_to_input_field_mga[range]);
> +	return hmc5843_show_int_plus_micros(buf,
> +		data->variant->regval_to_range_ga, HMC5843_RANGE_GAIN_MAX+1);
>  }
>  
> -static ssize_t hmc5843_set_range_gain(struct device *dev,
> -			struct device_attribute *attr,
> -			const char *buf,
> -			size_t count)
> -{
> -	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> -	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
> -	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) {
> -		count = error;
> -		goto exit;
> -	}
> -	dev_dbg(dev, "set range to %lu\n", range);
> +static IIO_DEVICE_ATTR(calibscale_available, S_IRUGO,
> +	hmc5843_show_calibscale_avail, NULL, 0);
>  
> -	if (range > HMC5843_RANGE_GAIN_MAX) {
> -		count = -EINVAL;
> -		goto exit;
> -	}
> -
> -	data->range = range;
> -	range = range << HMC5843_RANGE_GAIN_OFFSET;
> -	if (i2c_smbus_write_byte_data(data->client, this_attr->address, range))
> -		count = -EINVAL;
> -
> -exit:
> -	mutex_unlock(&data->lock);
> -	return count;
> +static int hmc5843_check_range(struct hmc5843_data *data,
> +	int val, int val2)
> +{
> +	return hmc5843_check_int_plus_micros(data->variant->regval_to_range_ga,
> +		HMC5843_RANGE_GAIN_MAX+1, val, val2);
>  }
>  
> -static IIO_DEVICE_ATTR(in_magn_range,
> -			S_IWUSR | S_IRUGO,
> -			hmc5843_show_range_gain,
> -			hmc5843_set_range_gain,
> -			HMC5843_CONFIG_REG_B);
> -
>  static int hmc5843_read_raw(struct iio_dev *indio_dev,
>  			    struct iio_chan_spec const *chan,
>  			    int *val, int *val2, long mask)
> @@ -481,6 +451,10 @@ static int hmc5843_read_raw(struct iio_dev *indio_dev,
>  		*val = data->variant->regval_to_samp_freq[data->rate][0];
>  		*val2 = data->variant->regval_to_samp_freq[data->rate][1];
>  		return IIO_VAL_INT_PLUS_MICRO;
> +	case IIO_CHAN_INFO_CALIBSCALE:
> +		*val = data->variant->regval_to_range_ga[data->range][0];
> +		*val2 = data->variant->regval_to_range_ga[data->range][1];
> +		return IIO_VAL_INT_PLUS_MICRO;
>  	}
>  	return -EINVAL;
>  }
> @@ -490,7 +464,7 @@ static int hmc5843_write_raw(struct iio_dev *indio_dev,
>  			     int val, int val2, long mask)
>  {
>  	struct hmc5843_data *data = iio_priv(indio_dev);
> -	int ret, rate;
> +	int ret, rate, range;
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_SAMP_FREQ:
> @@ -505,6 +479,20 @@ static int hmc5843_write_raw(struct iio_dev *indio_dev,
>  		mutex_unlock(&data->lock);
>  
>  		return ret;
> +	case IIO_CHAN_INFO_CALIBSCALE:
> +		range = hmc5843_check_range(data, val, val2);
> +		if (range < 0)
> +			return -EINVAL;
> +
> +		range <<= HMC5843_RANGE_GAIN_OFFSET;
> +		mutex_lock(&data->lock);
> +		ret = i2c_smbus_write_byte_data(data->client,
> +			HMC5843_CONFIG_REG_B, range);
> +		if (ret >= 0)
> +			data->range = range;
> +		mutex_unlock(&data->lock);
> +
> +		return ret;
>  	default:
>  		return -EINVAL;
>  	}
> @@ -517,7 +505,8 @@ static int hmc5843_write_raw(struct iio_dev *indio_dev,
>  		.channel2 = IIO_MOD_##axis,				\
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
>  		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
> -			BIT(IIO_CHAN_INFO_SAMP_FREQ),			\
> +			BIT(IIO_CHAN_INFO_SAMP_FREQ) |			\
> +			BIT(IIO_CHAN_INFO_CALIBSCALE),			\
>  		.address = addr						\
>  	}
>  
> @@ -536,7 +525,7 @@ static const struct iio_chan_spec hmc5883_channels[] = {
>  static struct attribute *hmc5843_attributes[] = {
>  	&iio_dev_attr_meas_conf.dev_attr.attr,
>  	&iio_dev_attr_operating_mode.dev_attr.attr,
> -	&iio_dev_attr_in_magn_range.dev_attr.attr,
> +	&iio_dev_attr_calibscale_available.dev_attr.attr,
>  	&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
>  	NULL
>  };
> @@ -549,22 +538,19 @@ static const struct hmc5843_chip_info hmc5843_chip_info_tbl[] = {
>  	[HMC5843_ID] = {
>  		.channels = hmc5843_channels,
>  		.regval_to_samp_freq = hmc5843_regval_to_samp_freq,
> -		.regval_to_input_field_mga =
> -			hmc5843_regval_to_input_field_mga,
> +		.regval_to_range_ga = hmc5843_regval_to_range_ga,
>  		.regval_to_nanoscale = hmc5843_regval_to_nanoscale,
>  	},
>  	[HMC5883_ID] = {
>  		.channels = hmc5883_channels,
>  		.regval_to_samp_freq = hmc5883_regval_to_samp_freq,
> -		.regval_to_input_field_mga =
> -			hmc5883_regval_to_input_field_mga,
> +		.regval_to_range_ga = hmc5883_regval_to_range_ga,
>  		.regval_to_nanoscale = hmc5883_regval_to_nanoscale,
>  	},
>  	[HMC5883L_ID] = {
>  		.channels = hmc5883_channels,
>  		.regval_to_samp_freq = hmc5883_regval_to_samp_freq,
> -		.regval_to_input_field_mga =
> -			hmc5883l_regval_to_input_field_mga,
> +		.regval_to_range_ga = hmc5883l_regval_to_range_ga,
>  		.regval_to_nanoscale = hmc5883l_regval_to_nanoscale,
>  	},
>  };
> 

Re: [PATCH v2 07/15] staging:iio:hmc5843: Use CALIBSCALE instead of magn_range

[Peter Meerwald]

Hi,

> I'm afraid I don't follow why you don't just make scale writeable
> rather than introducing a second element to control the same thing.
> Calibscale is intended for the case where the hardware can change
> the scale without it being apparent from the raw readings. Sometimes
> it also gets used to handling amplifiers in cases where the conversion
> function is 'interesting' and we have no choice but to do it in kernel
> in order to have data provided to userspace in a comprehensible form
> but that doesn't apply here.

I guess this is reminiscent of the staging code which had magn_range and 
scale; magn_range being the nice, human-readable measurement range (such 
as +- 1.9 Gauss)

setting the expected measurement range sounds like calibration to me...

I think this is one of the biggest issues of iio: that there is no 
guideline when to use what

regards, p.

-- 

Peter Meerwald
+43-664-2444418 (mobile)

Re: [PATCH v2 07/15] staging:iio:hmc5843: Use CALIBSCALE instead of magn_range

[Jonathan Cameron]

On 09/20/13 21:12, Peter Meerwald wrote:
> Hi,
> 
>> I'm afraid I don't follow why you don't just make scale writeable
>> rather than introducing a second element to control the same thing.
>> Calibscale is intended for the case where the hardware can change
>> the scale without it being apparent from the raw readings. Sometimes
>> it also gets used to handling amplifiers in cases where the conversion
>> function is 'interesting' and we have no choice but to do it in kernel
>> in order to have data provided to userspace in a comprehensible form
>> but that doesn't apply here.
> 
> I guess this is reminiscent of the staging code which had magn_range and 
> scale; magn_range being the nice, human-readable measurement range (such 
> as +- 1.9 Gauss)
True enough, but unfortunately we then have to have two controls for
the same things which leads to a messy inconsistent interface.

The best I can offer at the moment is the much stated plan to have an
extending 'available' type attribute for every element of the info mask.
Then combining the available scale with the available values that _raw
can take will give you the range of values that the part can take.

In the vast majority of cases this would provide userspace with all the
numbers to present this however is desired. The only nasty case would
be where the available raw values are stricted for some of the scales
but that would be rather unusual.

Note that such a generic _available (or other equivalent named) attribute
would need to cover both the discrete case and the data range cases.  Things
like

0.1111111 0.222222 0.3333333 0.444444

vs

0.111111..0.111111..0.444444

to specify the same thing.

An RFC on this has been my 'next thing to do' for the last couple of weeks
but I've been rather swamped with reviews to do so it hasn't happened yet!

> 
> setting the expected measurement range sounds like calibration to me...
Not really.  Things like a trim resistor value in an amplifier circuit
would be calibration. Ways  of fixing inaccuracies in the hardware.
The use in the various light sensors is sort of equivalent to this as
there is no direct output.
> 
> I think this is one of the biggest issues of iio: that there is no 
> guideline when to use what

True enough.  We are kind of working by precedence which is fine if it
is easy to find an equivalent case.   Perhaps adding more stuff to
the dummy driver to cover the uses of some of this stuff would help?

> 
> regards, p.
> 

[PATCH v2 08/15] staging:iio:hmc5843: Always read all channels values otherwise no updates

[Peter Meerwald]

Split out data ready/wait for read measurement

fix bug in case reading status register fails

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

diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
index 0c3f41e..d2a5da1 100644
--- a/drivers/staging/iio/magnetometer/hmc5843.c
+++ b/drivers/staging/iio/magnetometer/hmc5843.c
@@ -28,12 +28,7 @@
 #define HMC5843_CONFIG_REG_A			0x00
 #define HMC5843_CONFIG_REG_B			0x01
 #define HMC5843_MODE_REG			0x02
-#define HMC5843_DATA_OUT_X_MSB_REG		0x03
-#define HMC5843_DATA_OUT_Y_MSB_REG		0x05
-#define HMC5843_DATA_OUT_Z_MSB_REG		0x07
-/* Beware: Y and Z are exchanged on HMC5883 */
-#define HMC5883_DATA_OUT_Z_MSB_REG		0x05
-#define HMC5883_DATA_OUT_Y_MSB_REG		0x07
+#define HMC5843_DATA_OUT_MSB_REGS		0x03
 #define HMC5843_STATUS_REG			0x09
 
 enum hmc5843_ids {
@@ -190,17 +185,16 @@ static s32 hmc5843_configure(struct i2c_client *client,
 					operating_mode & HMC5843_MODE_MASK);
 }
 
-/* Return the measurement value from the specified channel */
-static int hmc5843_read_measurement(struct hmc5843_data *data,
-				    int address, int *val)
+static int hmc5843_wait_measurement(struct hmc5843_data *data)
 {
 	s32 result;
 	int tries = 150;
 
-	mutex_lock(&data->lock);
 	while (tries-- > 0) {
 		result = i2c_smbus_read_byte_data(data->client,
 			HMC5843_STATUS_REG);
+		if (result < 0)
+			return result;
 		if (result & HMC5843_DATA_READY)
 			break;
 		msleep(20);
@@ -208,16 +202,32 @@ static int hmc5843_read_measurement(struct hmc5843_data *data,
 
 	if (tries < 0) {
 		dev_err(&data->client->dev, "data not ready\n");
-		mutex_unlock(&data->lock);
 		return -EIO;
 	}
 
-	result = i2c_smbus_read_word_swapped(data->client, address);
+	return 0;
+}
+
+/* Return the measurement value from the specified channel */
+static int hmc5843_read_measurement(struct hmc5843_data *data,
+				    int idx, int *val)
+{
+	s32 result;
+	s16 values[3];
+
+	mutex_lock(&data->lock);
+	result = hmc5843_wait_measurement(data);
+	if (result < 0) {
+		mutex_unlock(&data->lock);
+		return result;
+	}
+	result = i2c_smbus_read_i2c_block_data(data->client,
+		HMC5843_DATA_OUT_MSB_REGS, sizeof(values), (u8 *) values);
 	mutex_unlock(&data->lock);
 	if (result < 0)
 		return -EINVAL;
 
-	*val = sign_extend32(result, 15);
+	*val = sign_extend32(be16_to_cpu(values[idx]), 15);
 	return IIO_VAL_INT;
 }
 
@@ -498,7 +508,7 @@ static int hmc5843_write_raw(struct iio_dev *indio_dev,
 	}
 }
 
-#define HMC5843_CHANNEL(axis, addr)					\
+#define HMC5843_CHANNEL(axis, idx)					\
 	{								\
 		.type = IIO_MAGN,					\
 		.modified = 1,						\
@@ -507,19 +517,20 @@ static int hmc5843_write_raw(struct iio_dev *indio_dev,
 		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
 			BIT(IIO_CHAN_INFO_SAMP_FREQ) |			\
 			BIT(IIO_CHAN_INFO_CALIBSCALE),			\
-		.address = addr						\
+		.address = idx						\
 	}
 
 static const struct iio_chan_spec hmc5843_channels[] = {
-	HMC5843_CHANNEL(X, HMC5843_DATA_OUT_X_MSB_REG),
-	HMC5843_CHANNEL(Y, HMC5843_DATA_OUT_Y_MSB_REG),
-	HMC5843_CHANNEL(Z, HMC5843_DATA_OUT_Z_MSB_REG),
+	HMC5843_CHANNEL(X, 0),
+	HMC5843_CHANNEL(Y, 1),
+	HMC5843_CHANNEL(Z, 2),
 };
 
+/* Beware: Y and Z are exchanged on HMC5883 */
 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),
+	HMC5843_CHANNEL(X, 0),
+	HMC5843_CHANNEL(Z, 1),
+	HMC5843_CHANNEL(Y, 2),
 };
 
 static struct attribute *hmc5843_attributes[] = {
-- 
1.8.4

[PATCH v2 09/15] staging:iio:hmc5843: Add trigger handling

[Peter Meerwald]

v2 (thanks to Jonathan Cameron):
* drop dynamic buffer allocation, buffer is in hmc5842_data
* grab timestamp near data acquisition
* restrict available scan masks (only read all axis)

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

diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
index d2a5da1..41e2dc8 100644
--- a/drivers/staging/iio/magnetometer/hmc5843.c
+++ b/drivers/staging/iio/magnetometer/hmc5843.c
@@ -23,6 +23,9 @@
 #include <linux/i2c.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/triggered_buffer.h>
 #include <linux/delay.h>
 
 #define HMC5843_CONFIG_REG_A			0x00
@@ -174,6 +177,7 @@ struct hmc5843_data {
 	u8 operating_mode;
 	u8 range;
 	const struct hmc5843_chip_info *variant;
+	s16 buffer[8]; /* 3x 16-bit channels + padding + 64-bit timestamp */
 };
 
 /* The lower two bits contain the current conversion mode */
@@ -452,7 +456,7 @@ static int hmc5843_read_raw(struct iio_dev *indio_dev,
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		return hmc5843_read_measurement(data, chan->address, val);
+		return hmc5843_read_measurement(data, chan->scan_index, val);
 	case IIO_CHAN_INFO_SCALE:
 		*val = 0;
 		*val2 = data->variant->regval_to_nanoscale[data->range];
@@ -508,6 +512,38 @@ static int hmc5843_write_raw(struct iio_dev *indio_dev,
 	}
 }
 
+static irqreturn_t hmc5843_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct hmc5843_data *data = iio_priv(indio_dev);
+	int ret;
+
+	mutex_lock(&data->lock);
+	ret = hmc5843_wait_measurement(data);
+	if (ret < 0) {
+		mutex_unlock(&data->lock);
+		goto done;
+	}
+
+	ret = i2c_smbus_read_i2c_block_data(data->client,
+		HMC5843_DATA_OUT_MSB_REGS, 3 * sizeof(s16),
+			(u8 *) data->buffer);
+	mutex_unlock(&data->lock);
+	if (ret < 0)
+		goto done;
+
+	if (indio_dev->scan_timestamp)
+		*(s64 *)((u8 *)data->buffer + ALIGN(6, sizeof(s64)))
+			= iio_get_time_ns();
+	iio_push_to_buffers(indio_dev, data->buffer);
+
+done:
+	iio_trigger_notify_done(indio_dev->trig);
+
+	return IRQ_HANDLED;
+}
+
 #define HMC5843_CHANNEL(axis, idx)					\
 	{								\
 		.type = IIO_MAGN,					\
@@ -517,13 +553,15 @@ static int hmc5843_write_raw(struct iio_dev *indio_dev,
 		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
 			BIT(IIO_CHAN_INFO_SAMP_FREQ) |			\
 			BIT(IIO_CHAN_INFO_CALIBSCALE),			\
-		.address = idx						\
+		.scan_index = idx,					\
+		.scan_type = IIO_ST('s', 16, 16, IIO_BE),		\
 	}
 
 static const struct iio_chan_spec hmc5843_channels[] = {
 	HMC5843_CHANNEL(X, 0),
 	HMC5843_CHANNEL(Y, 1),
 	HMC5843_CHANNEL(Z, 2),
+	IIO_CHAN_SOFT_TIMESTAMP(3),
 };
 
 /* Beware: Y and Z are exchanged on HMC5883 */
@@ -531,6 +569,7 @@ static const struct iio_chan_spec hmc5883_channels[] = {
 	HMC5843_CHANNEL(X, 0),
 	HMC5843_CHANNEL(Z, 1),
 	HMC5843_CHANNEL(Y, 2),
+	IIO_CHAN_SOFT_TIMESTAMP(3),
 };
 
 static struct attribute *hmc5843_attributes[] = {
@@ -582,12 +621,14 @@ static const struct iio_info hmc5843_info = {
 	.driver_module = THIS_MODULE,
 };
 
+static const unsigned long hmc5843_scan_masks[] = {0x7, 0};
+
 static int hmc5843_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
 	struct hmc5843_data *data;
 	struct iio_dev *indio_dev;
-	int err = 0;
+	int ret;
 
 	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
 	if (indio_dev == NULL)
@@ -605,20 +646,34 @@ static int hmc5843_probe(struct i2c_client *client,
 	indio_dev->dev.parent = &client->dev;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->channels = data->variant->channels;
-	indio_dev->num_channels = 3;
+	indio_dev->num_channels = 4;
+	indio_dev->available_scan_masks = hmc5843_scan_masks;
 
 	hmc5843_init(data);
 
-	err = iio_device_register(indio_dev);
-	if (err)
-		return err;
+	ret = iio_triggered_buffer_setup(indio_dev, NULL,
+		hmc5843_trigger_handler, NULL);
+	if (ret < 0)
+		return ret;
+
+	ret = iio_device_register(indio_dev);
+	if (ret < 0)
+		goto buffer_cleanup;
 
 	return 0;
+
+buffer_cleanup:
+	iio_triggered_buffer_cleanup(indio_dev);
+	return ret;
 }
 
 static int hmc5843_remove(struct i2c_client *client)
 {
-	iio_device_unregister(i2c_get_clientdata(client));
+	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+
+	iio_device_unregister(indio_dev);
+	iio_triggered_buffer_cleanup(indio_dev);
+
 	 /*  sleep mode to save power */
 	hmc5843_configure(client, HMC5843_MODE_SLEEP);
 
-- 
1.8.4

[PATCH v2 10/15] staging:iio:hmc5843: Remove ability to change operating mode

[Peter Meerwald]

only continuous mode is supported for now; the driver could/should
be switched to single conversion mode

operating mode should be determined by the way IIO accesses the device
and not exposed explicitly

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/staging/iio/magnetometer/hmc5843.c | 68 ------------------------------
 1 file changed, 68 deletions(-)

diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
index 41e2dc8..f8e6b38 100644
--- a/drivers/staging/iio/magnetometer/hmc5843.c
+++ b/drivers/staging/iio/magnetometer/hmc5843.c
@@ -236,73 +236,6 @@ static int hmc5843_read_measurement(struct hmc5843_data *data,
 }
 
 /*
- * From the datasheet:
- * 0 - Continuous-Conversion Mode: In continuous-conversion mode, the
- *     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 returns to sleep mode.
- *
- * 2 - Idle Mode : Device is placed in idle mode.
- *
- * 3 - Sleep Mode : Device is placed in sleep mode.
- *
- */
-static ssize_t hmc5843_show_operating_mode(struct device *dev,
-					struct device_attribute *attr,
-					char *buf)
-{
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct hmc5843_data *data = iio_priv(indio_dev);
-	return sprintf(buf, "%d\n", data->operating_mode);
-}
-
-static ssize_t hmc5843_set_operating_mode(struct device *dev,
-				struct device_attribute *attr,
-				const char *buf,
-				size_t count)
-{
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct i2c_client *client = to_i2c_client(indio_dev->dev.parent);
-	struct hmc5843_data *data = iio_priv(indio_dev);
-	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
-	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);
-	if (operating_mode > HMC5843_MODE_SLEEP) {
-		count = -EINVAL;
-		goto exit;
-	}
-
-	status = i2c_smbus_write_byte_data(client, this_attr->address,
-					operating_mode);
-	if (status) {
-		count = -EINVAL;
-		goto exit;
-	}
-	data->operating_mode = operating_mode;
-
-exit:
-	mutex_unlock(&data->lock);
-	return count;
-}
-
-static IIO_DEVICE_ATTR(operating_mode,
-			S_IWUSR | S_IRUGO,
-			hmc5843_show_operating_mode,
-			hmc5843_set_operating_mode,
-			HMC5843_MODE_REG);
-
-/*
  * API for setting the measurement configuration to
  * Normal, Positive bias and Negative bias
  *
@@ -574,7 +507,6 @@ static const struct iio_chan_spec hmc5883_channels[] = {
 
 static struct attribute *hmc5843_attributes[] = {
 	&iio_dev_attr_meas_conf.dev_attr.attr,
-	&iio_dev_attr_operating_mode.dev_attr.attr,
 	&iio_dev_attr_calibscale_available.dev_attr.attr,
 	&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
 	NULL
-- 
1.8.4

[PATCH v2 11/15] staging:iio:hmc5843: Rename _configure() to _set_mode()

[Peter Meerwald]

and be consistent with other setter functions in that first argument
is hmc5843_data

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

diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
index f8e6b38..2bbaea1 100644
--- a/drivers/staging/iio/magnetometer/hmc5843.c
+++ b/drivers/staging/iio/magnetometer/hmc5843.c
@@ -181,12 +181,18 @@ struct hmc5843_data {
 };
 
 /* The lower two bits contain the current conversion mode */
-static s32 hmc5843_configure(struct i2c_client *client,
-				       u8 operating_mode)
+static s32 hmc5843_set_mode(struct hmc5843_data *data, u8 operating_mode)
 {
-	return i2c_smbus_write_byte_data(client,
-					HMC5843_MODE_REG,
+	int ret;
+
+	mutex_lock(&data->lock);
+	ret = i2c_smbus_write_byte_data(data->client, HMC5843_MODE_REG,
 					operating_mode & HMC5843_MODE_MASK);
+	if (ret >= 0)
+		data->operating_mode = operating_mode;
+	mutex_unlock(&data->lock);
+
+	return ret;
 }
 
 static int hmc5843_wait_measurement(struct hmc5843_data *data)
@@ -541,7 +547,7 @@ static void hmc5843_init(struct hmc5843_data *data)
 {
 	hmc5843_set_meas_conf(data, HMC5843_MEAS_CONF_NORMAL);
 	hmc5843_set_rate(data, HMC5843_RATE_DEFAULT);
-	hmc5843_configure(data->client, HMC5843_MODE_CONVERSION_CONTINUOUS);
+	hmc5843_set_mode(data, HMC5843_MODE_CONVERSION_CONTINUOUS);
 	i2c_smbus_write_byte_data(data->client, HMC5843_CONFIG_REG_B,
 		HMC5843_RANGE_GAIN_DEFAULT);
 }
@@ -607,7 +613,7 @@ static int hmc5843_remove(struct i2c_client *client)
 	iio_triggered_buffer_cleanup(indio_dev);
 
 	 /*  sleep mode to save power */
-	hmc5843_configure(client, HMC5843_MODE_SLEEP);
+	hmc5843_set_mode(iio_priv(indio_dev), HMC5843_MODE_SLEEP);
 
 	return 0;
 }
@@ -615,9 +621,10 @@ 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), HMC5843_MODE_SLEEP);
+	struct hmc5843_data *data = iio_priv(i2c_get_clientdata(
+		to_i2c_client(dev)));
 
-	return 0;
+	return hmc5843_set_mode(data, HMC5843_MODE_SLEEP);
 }
 
 static int hmc5843_resume(struct device *dev)
@@ -625,9 +632,7 @@ static int hmc5843_resume(struct device *dev)
 	struct hmc5843_data *data = iio_priv(i2c_get_clientdata(
 		to_i2c_client(dev)));
 
-	hmc5843_configure(data->client, data->operating_mode);
-
-	return 0;
+	return hmc5843_set_mode(data, HMC5843_MODE_CONVERSION_CONTINUOUS);
 }
 
 static SIMPLE_DEV_PM_OPS(hmc5843_pm_ops, hmc5843_suspend, hmc5843_resume);
-- 
1.8.4

[PATCH v2 12/15] staging:iio:hmc5843: Reorganize _set_meas_conf()

[Peter Meerwald]

move locking inside _set() function

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

diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
index 2bbaea1..abbddc4 100644
--- a/drivers/staging/iio/magnetometer/hmc5843.c
+++ b/drivers/staging/iio/magnetometer/hmc5843.c
@@ -261,19 +261,24 @@ static int hmc5843_read_measurement(struct hmc5843_data *data,
  */
 static s32 hmc5843_set_meas_conf(struct hmc5843_data *data, u8 meas_conf)
 {
-	u8 reg_val;
-	reg_val = (meas_conf & HMC5843_MEAS_CONF_MASK) |
-		(data->rate << HMC5843_RATE_OFFSET);
-	return i2c_smbus_write_byte_data(data->client, HMC5843_CONFIG_REG_A,
-		reg_val);
+	int ret;
+
+	mutex_lock(&data->lock);
+	ret = i2c_smbus_write_byte_data(data->client, HMC5843_CONFIG_REG_A,
+		(meas_conf & HMC5843_MEAS_CONF_MASK) |
+		(data->rate << HMC5843_RATE_OFFSET));
+	if (ret >= 0)
+		data->meas_conf = meas_conf;
+	mutex_unlock(&data->lock);
+
+	return ret;
 }
 
 static ssize_t hmc5843_show_measurement_configuration(struct device *dev,
 						struct device_attribute *attr,
 						char *buf)
 {
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct hmc5843_data *data = iio_priv(indio_dev);
+	struct hmc5843_data *data = iio_priv(dev_to_iio_dev(dev));
 	return sprintf(buf, "%d\n", data->meas_conf);
 }
 
@@ -282,28 +287,19 @@ static ssize_t hmc5843_set_measurement_configuration(struct device *dev,
 						const char *buf,
 						size_t count)
 {
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct hmc5843_data *data = iio_priv(indio_dev);
+	struct hmc5843_data *data = iio_priv(dev_to_iio_dev(dev));
 	unsigned long meas_conf = 0;
-	int error;
+	int ret;
 
-	error = kstrtoul(buf, 10, &meas_conf);
-	if (error)
-		return error;
+	ret = kstrtoul(buf, 10, &meas_conf);
+	if (ret)
+		return ret;
 	if (meas_conf >= HMC5843_MEAS_CONF_NOT_USED)
 		return -EINVAL;
 
-	mutex_lock(&data->lock);
-	dev_dbg(dev, "set measurement configuration to %lu\n", meas_conf);
-	if (hmc5843_set_meas_conf(data, meas_conf)) {
-		count = -EINVAL;
-		goto exit;
-	}
-	data->meas_conf = meas_conf;
+	ret = hmc5843_set_meas_conf(data, meas_conf);
 
-exit:
-	mutex_unlock(&data->lock);
-	return count;
+	return (ret < 0) ? ret : count;
 }
 
 static IIO_DEVICE_ATTR(meas_conf,
-- 
1.8.4

[PATCH v2 13/15] staging:iio:hmc5843: Rename _set_rate() to _set_samp_freq()

[Peter Meerwald]

move locking inside _set() function

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

diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
index abbddc4..3c3a3dd 100644
--- a/drivers/staging/iio/magnetometer/hmc5843.c
+++ b/drivers/staging/iio/magnetometer/hmc5843.c
@@ -348,12 +348,18 @@ static ssize_t hmc5843_show_samp_freq_avail(struct device *dev,
 
 static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(hmc5843_show_samp_freq_avail);
 
-static s32 hmc5843_set_rate(struct hmc5843_data *data, u8 rate)
+static int hmc5843_set_samp_freq(struct hmc5843_data *data, u8 rate)
 {
-	u8 reg_val = data->meas_conf | (rate << HMC5843_RATE_OFFSET);
+	int ret;
+
+	mutex_lock(&data->lock);
+	ret = i2c_smbus_write_byte_data(data->client, HMC5843_CONFIG_REG_A,
+		data->meas_conf | (rate << HMC5843_RATE_OFFSET));
+	if (ret >= 0)
+		data->rate = rate;
+	mutex_unlock(&data->lock);
 
-	return i2c_smbus_write_byte_data(data->client, HMC5843_CONFIG_REG_A,
-		reg_val);
+	return ret;
 }
 
 static int hmc5843_check_samp_freq(struct hmc5843_data *data,
@@ -421,13 +427,7 @@ static int hmc5843_write_raw(struct iio_dev *indio_dev,
 		if (rate < 0)
 			return -EINVAL;
 
-		mutex_lock(&data->lock);
-		ret = hmc5843_set_rate(data, rate);
-		if (ret >= 0)
-			data->rate = rate;
-		mutex_unlock(&data->lock);
-
-		return ret;
+		return hmc5843_set_samp_freq(data, rate);
 	case IIO_CHAN_INFO_CALIBSCALE:
 		range = hmc5843_check_range(data, val, val2);
 		if (range < 0)
@@ -542,7 +542,7 @@ static const struct hmc5843_chip_info hmc5843_chip_info_tbl[] = {
 static void hmc5843_init(struct hmc5843_data *data)
 {
 	hmc5843_set_meas_conf(data, HMC5843_MEAS_CONF_NORMAL);
-	hmc5843_set_rate(data, HMC5843_RATE_DEFAULT);
+	hmc5843_set_samp_freq(data, HMC5843_RATE_DEFAULT);
 	hmc5843_set_mode(data, HMC5843_MODE_CONVERSION_CONTINUOUS);
 	i2c_smbus_write_byte_data(data->client, HMC5843_CONFIG_REG_B,
 		HMC5843_RANGE_GAIN_DEFAULT);
-- 
1.8.4

[PATCH v2 14/15] staging:iio:hmc5843: Introduce _set_range_gain()

[Peter Meerwald]

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

diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
index 3c3a3dd..9f4571f 100644
--- a/drivers/staging/iio/magnetometer/hmc5843.c
+++ b/drivers/staging/iio/magnetometer/hmc5843.c
@@ -370,6 +370,20 @@ static int hmc5843_check_samp_freq(struct hmc5843_data *data,
 		val, val2);
 }
 
+static int hmc5843_set_range_gain(struct hmc5843_data *data, u8 range)
+{
+	int ret;
+
+	mutex_lock(&data->lock);
+	ret = i2c_smbus_write_byte_data(data->client, HMC5843_CONFIG_REG_B,
+		range << HMC5843_RANGE_GAIN_OFFSET);
+	if (ret >= 0)
+		data->range = range;
+	mutex_unlock(&data->lock);
+
+	return ret;
+}
+
 static ssize_t hmc5843_show_calibscale_avail(struct device *dev,
 				struct device_attribute *attr, char *buf)
 {
@@ -382,7 +396,7 @@ static ssize_t hmc5843_show_calibscale_avail(struct device *dev,
 static IIO_DEVICE_ATTR(calibscale_available, S_IRUGO,
 	hmc5843_show_calibscale_avail, NULL, 0);
 
-static int hmc5843_check_range(struct hmc5843_data *data,
+static int hmc5843_check_range_gain(struct hmc5843_data *data,
 	int val, int val2)
 {
 	return hmc5843_check_int_plus_micros(data->variant->regval_to_range_ga,
@@ -419,7 +433,7 @@ static int hmc5843_write_raw(struct iio_dev *indio_dev,
 			     int val, int val2, long mask)
 {
 	struct hmc5843_data *data = iio_priv(indio_dev);
-	int ret, rate, range;
+	int rate, range;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_SAMP_FREQ:
@@ -429,19 +443,11 @@ static int hmc5843_write_raw(struct iio_dev *indio_dev,
 
 		return hmc5843_set_samp_freq(data, rate);
 	case IIO_CHAN_INFO_CALIBSCALE:
-		range = hmc5843_check_range(data, val, val2);
+		range = hmc5843_check_range_gain(data, val, val2);
 		if (range < 0)
 			return -EINVAL;
 
-		range <<= HMC5843_RANGE_GAIN_OFFSET;
-		mutex_lock(&data->lock);
-		ret = i2c_smbus_write_byte_data(data->client,
-			HMC5843_CONFIG_REG_B, range);
-		if (ret >= 0)
-			data->range = range;
-		mutex_unlock(&data->lock);
-
-		return ret;
+		return hmc5843_set_range_gain(data, range);
 	default:
 		return -EINVAL;
 	}
@@ -543,9 +549,8 @@ static void hmc5843_init(struct hmc5843_data *data)
 {
 	hmc5843_set_meas_conf(data, HMC5843_MEAS_CONF_NORMAL);
 	hmc5843_set_samp_freq(data, HMC5843_RATE_DEFAULT);
+	hmc5843_set_range_gain(data, HMC5843_RANGE_GAIN_DEFAULT);
 	hmc5843_set_mode(data, HMC5843_MODE_CONVERSION_CONTINUOUS);
-	i2c_smbus_write_byte_data(data->client, HMC5843_CONFIG_REG_B,
-		HMC5843_RANGE_GAIN_DEFAULT);
 }
 
 static const struct iio_info hmc5843_info = {
-- 
1.8.4

[PATCH v2 15/15] staging:iio:hmc5843: Check initialization and chip identifier

[Peter Meerwald]

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

diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
index 9f4571f..4b4b2e2 100644
--- a/drivers/staging/iio/magnetometer/hmc5843.c
+++ b/drivers/staging/iio/magnetometer/hmc5843.c
@@ -33,6 +33,7 @@
 #define HMC5843_MODE_REG			0x02
 #define HMC5843_DATA_OUT_MSB_REGS		0x03
 #define HMC5843_STATUS_REG			0x09
+#define HMC5843_ID_REG				0x0a
 
 enum hmc5843_ids {
 	HMC5843_ID,
@@ -545,12 +546,30 @@ static const struct hmc5843_chip_info hmc5843_chip_info_tbl[] = {
 	},
 };
 
-static void hmc5843_init(struct hmc5843_data *data)
+static int hmc5843_init(struct hmc5843_data *data)
 {
-	hmc5843_set_meas_conf(data, HMC5843_MEAS_CONF_NORMAL);
-	hmc5843_set_samp_freq(data, HMC5843_RATE_DEFAULT);
-	hmc5843_set_range_gain(data, HMC5843_RANGE_GAIN_DEFAULT);
-	hmc5843_set_mode(data, HMC5843_MODE_CONVERSION_CONTINUOUS);
+	int ret;
+	u8 id[3];
+
+	ret = i2c_smbus_read_i2c_block_data(data->client, HMC5843_ID_REG,
+		sizeof(id), id);
+	if (ret < 0)
+		return ret;
+	if (id[0] != 'H' || id[1] != '4' || id[2] != '3') {
+		dev_err(&data->client->dev, "no HMC5843/5883/5883L sensor\n");
+		return -ENODEV;
+	}
+
+	ret = hmc5843_set_meas_conf(data, HMC5843_MEAS_CONF_NORMAL);
+	if (ret < 0)
+		return ret;
+	ret = hmc5843_set_samp_freq(data, HMC5843_RATE_DEFAULT);
+	if (ret < 0)
+		return ret;
+	ret = hmc5843_set_range_gain(data, HMC5843_RANGE_GAIN_DEFAULT);
+	if (ret < 0)
+		return ret;
+	return hmc5843_set_mode(data, HMC5843_MODE_CONVERSION_CONTINUOUS);
 }
 
 static const struct iio_info hmc5843_info = {
@@ -588,7 +607,9 @@ static int hmc5843_probe(struct i2c_client *client,
 	indio_dev->num_channels = 4;
 	indio_dev->available_scan_masks = hmc5843_scan_masks;
 
-	hmc5843_init(data);
+	ret = hmc5843_init(data);
+	if (ret < 0)
+		return ret;
 
 	ret = iio_triggered_buffer_setup(indio_dev, NULL,
 		hmc5843_trigger_handler, NULL);
-- 
1.8.4

Re: [PATCH v2 00/15] staging:iio:hmc5843 cleanup

[Jonathan Cameron]

On 09/19/13 20:35, Peter Meerwald wrote:
> updated v2 patch series of the hmc5843 cleanup, addressnig comments by 
> Jonathan Cameron
> 
All fine except for the outstanding question of why you need calibscale?
> patch 1: correctly dereferencing dev, fixed suspend/resume
> patch 7: checked CALIBSCALE, output is as follows
> 
>   # cat calibscale_available 
>   0.880000 1.300000 1.900000 2.500000 4.0 4.700000 5.600000 8.100000
>   # cat in_magn_calibscale 
>   1.300000
>   # cat in_magn_scale 
>   0.000009174
>   # cat in_magn_x_raw 
>   252
>   # echo 1.900000 > in_magn_calibscale
>   # cat in_magn_scale 
>   0.000012195
Whilst we do 'allow' for any attribute to result in a change in any other,
it is nice to avoid it if possible (and it is kind of assumed
that changes to calibscale and caliboffset are for tweaking miss
calibrate hardware, not gross changes like this), hence just have
echo 0.000012195 > in_magn_scale
rather than the
1.9 to > in_magn_calibscale

>   # cat in_magn_x_raw 
>   198
>   # echo 0.88 > in_magn_calibscale
>   # cat in_magn_scale 
>   0.000007299
>   # cat in_magn_x_raw
>   318
> 
>   calibscale adjusts the measurement range and scale is used to keep Gauss
>   reading the same
>   
> patch 9: buffer is now part of hmc5843_data, using IIO core demux as suggested
> 
> remaining patches should not have changed
> 
> Peter Meerwald (15):
>   staging:iio:hmc5843: Add pointer to i2c client to data struct
>   staging:iio:hmc5843: Rewrite init function
>   staging:iio:hmc5843: Use INFO_SAMP_FREQ
>   staging:iio:hmc5843: Remove unused LSB register #defines
>   staging:iio:hmc5843: Tighten comments
>   staging:iio:hmc5843: Introduce helper functions to show/check list of
>     int pairs
>   staging:iio:hmc5843: Use CALIBSCALE instead of magn_range
>   staging:iio:hmc5843: Always read all channels values otherwise no
>     updates
>   staging:iio:hmc5843: Add trigger handling
>   staging:iio:hmc5843: Remove ability to change operating mode
>   staging:iio:hmc5843: Rename _configure() to _set_mode()
>   staging:iio:hmc5843: Reorganize _set_meas_conf()
>   staging:iio:hmc5843: Rename _set_rate() to _set_samp_freq()
>   staging:iio:hmc5843: Introduce _set_range_gain()
>   staging:iio:hmc5843: Check initialization and chip identifier
> 
>  drivers/staging/iio/magnetometer/hmc5843.c | 612 ++++++++++++++---------------
>  1 file changed, 287 insertions(+), 325 deletions(-)
>