[PATCH 1/5] iio:magnetometer:mag3110: Report busy in _read_raw() / write_raw() when buffer is enabled

[PATCH 1/5] iio:magnetometer:mag3110: Report busy in _read_raw() / write_raw() when buffer is enabled

[Peter Meerwald]

individual reads are not permitted concurrently with buffered reads

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

diff --git a/drivers/iio/magnetometer/mag3110.c b/drivers/iio/magnetometer/mag3110.c
index 4b65b6d..b88cb44 100644
--- a/drivers/iio/magnetometer/mag3110.c
+++ b/drivers/iio/magnetometer/mag3110.c
@@ -154,6 +154,9 @@ static int mag3110_read_raw(struct iio_dev *indio_dev,
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
+		if (iio_buffer_enabled(indio_dev))
+			return -EBUSY;
+
 		switch (chan->type) {
 		case IIO_MAGN: /* in 0.1 uT / LSB */
 			ret = mag3110_read(data, buffer);
@@ -199,6 +202,9 @@ static int mag3110_write_raw(struct iio_dev *indio_dev,
 	struct mag3110_data *data = iio_priv(indio_dev);
 	int rate;
 
+	if (iio_buffer_enabled(indio_dev))
+		return -EBUSY;
+
 	switch (mask) {
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		rate = mag3110_get_samp_freq_index(data, val, val2);
-- 
1.8.3.2

[PATCH 2/5] iio:magnetometer:mag3110: Fix output of decimal digits in show_int_plus_micros()

[Peter Meerwald]

need to print leading zeros, hence "%d.%06d"

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/iio/magnetometer/mag3110.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/magnetometer/mag3110.c b/drivers/iio/magnetometer/mag3110.c
index b88cb44..f66955f 100644
--- a/drivers/iio/magnetometer/mag3110.c
+++ b/drivers/iio/magnetometer/mag3110.c
@@ -106,7 +106,7 @@ static ssize_t mag3110_show_int_plus_micros(char *buf,
 
 	while (n-- > 0)
 		len += scnprintf(buf + len, PAGE_SIZE - len,
-			"%d.%d ", vals[n][0], vals[n][1]);
+			"%d.%06d ", vals[n][0], vals[n][1]);
 
 	/* replace trailing space by newline */
 	buf[len - 1] = '\n';
-- 
1.8.3.2

Re: [PATCH 2/5] iio:magnetometer:mag3110: Fix output of decimal digits in show_int_plus_micros()

[Jonathan Cameron]

On 10/01/14 21:59, Peter Meerwald wrote:
> need to print leading zeros, hence "%d.%06d"
>
> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
Applied to the fixes-for-3.14new branch of iio.git

Thanks,
> ---
>   drivers/iio/magnetometer/mag3110.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/magnetometer/mag3110.c b/drivers/iio/magnetometer/mag3110.c
> index b88cb44..f66955f 100644
> --- a/drivers/iio/magnetometer/mag3110.c
> +++ b/drivers/iio/magnetometer/mag3110.c
> @@ -106,7 +106,7 @@ static ssize_t mag3110_show_int_plus_micros(char *buf,
>
>   	while (n-- > 0)
>   		len += scnprintf(buf + len, PAGE_SIZE - len,
> -			"%d.%d ", vals[n][0], vals[n][1]);
> +			"%d.%06d ", vals[n][0], vals[n][1]);
>
>   	/* replace trailing space by newline */
>   	buf[len - 1] = '\n';
>

[PATCH 3/5] iio:magnetometer:mag3110: Scale factor missing for temperature

[Peter Meerwald]

temperature is reported in milli-Celsius

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

diff --git a/drivers/iio/magnetometer/mag3110.c b/drivers/iio/magnetometer/mag3110.c
index f66955f..02e4f97 100644
--- a/drivers/iio/magnetometer/mag3110.c
+++ b/drivers/iio/magnetometer/mag3110.c
@@ -183,8 +183,17 @@ static int mag3110_read_raw(struct iio_dev *indio_dev,
 			return -EINVAL;
 		}
 	case IIO_CHAN_INFO_SCALE:
-		*val = 0;
-		*val2 = 1000;
+		switch (chan->type) {
+		case IIO_MAGN:
+			*val = 0;
+			*val2 = 1000;
+			return IIO_VAL_INT_PLUS_MICRO;
+		case IIO_TEMP:
+			*val = 1000;
+			return IIO_VAL_INT;
+		default:
+			return -EINVAL;
+		}
 		return IIO_VAL_INT_PLUS_MICRO;
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		i = data->ctrl_reg1 >> MAG3110_CTRL_DR_SHIFT;
@@ -270,7 +279,8 @@ static const struct iio_chan_spec mag3110_channels[] = {
 	MAG3110_CHANNEL(Z, 2),
 	{
 		.type = IIO_TEMP,
-		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+			BIT(IIO_CHAN_INFO_SCALE),
 		.scan_index = 3,
 		.scan_type = {
 			.sign = 's',
-- 
1.8.3.2

Re: [PATCH 3/5] iio:magnetometer:mag3110: Scale factor missing for temperature

[Jonathan Cameron]

On 10/01/14 21:59, Peter Meerwald wrote:
> temperature is reported in milli-Celsius
>
> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
This one is sort of new functionality rather than a fix so I've going to hold
this one until after the others have propogated through then apply it
via the normal togreg branch.

> ---
>   drivers/iio/magnetometer/mag3110.c | 16 +++++++++++++---
>   1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/magnetometer/mag3110.c b/drivers/iio/magnetometer/mag3110.c
> index f66955f..02e4f97 100644
> --- a/drivers/iio/magnetometer/mag3110.c
> +++ b/drivers/iio/magnetometer/mag3110.c
> @@ -183,8 +183,17 @@ static int mag3110_read_raw(struct iio_dev *indio_dev,
>   			return -EINVAL;
>   		}
>   	case IIO_CHAN_INFO_SCALE:
> -		*val = 0;
> -		*val2 = 1000;
> +		switch (chan->type) {
> +		case IIO_MAGN:
> +			*val = 0;
> +			*val2 = 1000;
> +			return IIO_VAL_INT_PLUS_MICRO;
> +		case IIO_TEMP:
> +			*val = 1000;
> +			return IIO_VAL_INT;
> +		default:
> +			return -EINVAL;
> +		}
>   		return IIO_VAL_INT_PLUS_MICRO;
>   	case IIO_CHAN_INFO_SAMP_FREQ:
>   		i = data->ctrl_reg1 >> MAG3110_CTRL_DR_SHIFT;
> @@ -270,7 +279,8 @@ static const struct iio_chan_spec mag3110_channels[] = {
>   	MAG3110_CHANNEL(Z, 2),
>   	{
>   		.type = IIO_TEMP,
> -		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +			BIT(IIO_CHAN_INFO_SCALE),
>   		.scan_index = 3,
>   		.scan_type = {
>   			.sign = 's',
>

[PATCH 4/5] iio:magnetometer:mag3110: Fix initialization of ctrl_reg1

[Peter Meerwald]

DR bits need to be shifted; since MAG3110_CTRL_DR_DEFAULT is
zero, the change has no effect

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/iio/magnetometer/mag3110.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/magnetometer/mag3110.c b/drivers/iio/magnetometer/mag3110.c
index 02e4f97..9e774de 100644
--- a/drivers/iio/magnetometer/mag3110.c
+++ b/drivers/iio/magnetometer/mag3110.c
@@ -339,7 +339,7 @@ static int mag3110_probe(struct i2c_client *client,
 	indio_dev->num_channels = ARRAY_SIZE(mag3110_channels);
 	indio_dev->available_scan_masks = mag3110_scan_masks;
 
-	data->ctrl_reg1 = MAG3110_CTRL_DR_DEFAULT;
+	data->ctrl_reg1 = MAG3110_CTRL_DR_DEFAULT << MAG3110_CTRL_DR_SHIFT;
 	ret = i2c_smbus_write_byte_data(client, MAG3110_CTRL_REG1,
 		data->ctrl_reg1);
 	if (ret < 0)
-- 
1.8.3.2

Re: [PATCH 4/5] iio:magnetometer:mag3110: Fix initialization of ctrl_reg1

[Jonathan Cameron]

On 10/01/14 21:59, Peter Meerwald wrote:
> DR bits need to be shifted; since MAG3110_CTRL_DR_DEFAULT is
> zero, the change has no effect
>
> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
Again, I'll hold this one for after the fixes are in.

Thanks,

Jonathan
> ---
>   drivers/iio/magnetometer/mag3110.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/magnetometer/mag3110.c b/drivers/iio/magnetometer/mag3110.c
> index 02e4f97..9e774de 100644
> --- a/drivers/iio/magnetometer/mag3110.c
> +++ b/drivers/iio/magnetometer/mag3110.c
> @@ -339,7 +339,7 @@ static int mag3110_probe(struct i2c_client *client,
>   	indio_dev->num_channels = ARRAY_SIZE(mag3110_channels);
>   	indio_dev->available_scan_masks = mag3110_scan_masks;
>
> -	data->ctrl_reg1 = MAG3110_CTRL_DR_DEFAULT;
> +	data->ctrl_reg1 = MAG3110_CTRL_DR_DEFAULT << MAG3110_CTRL_DR_SHIFT;
>   	ret = i2c_smbus_write_byte_data(client, MAG3110_CTRL_REG1,
>   		data->ctrl_reg1);
>   	if (ret < 0)
>

[PATCH 5/5] iio:magnetometer:mag3110: Enable user offset calibration

[Peter Meerwald]

add IIO_CHAN_INFO_CALIBBIAS

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/iio/magnetometer/mag3110.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/magnetometer/mag3110.c b/drivers/iio/magnetometer/mag3110.c
index 9e774de..9f260a5 100644
--- a/drivers/iio/magnetometer/mag3110.c
+++ b/drivers/iio/magnetometer/mag3110.c
@@ -200,6 +200,13 @@ static int mag3110_read_raw(struct iio_dev *indio_dev,
 		*val = mag3110_samp_freq[i][0];
 		*val2 = mag3110_samp_freq[i][1];
 		return IIO_VAL_INT_PLUS_MICRO;
+	case IIO_CHAN_INFO_CALIBBIAS:
+		ret = i2c_smbus_read_word_swapped(data->client,
+			MAG3110_OFF_X +	2 * chan->scan_index);
+		if (ret < 0)
+			return ret;
+		*val = sign_extend32(ret >> 1, 14);
+		return IIO_VAL_INT;
 	}
 	return -EINVAL;
 }
@@ -224,6 +231,11 @@ static int mag3110_write_raw(struct iio_dev *indio_dev,
 		data->ctrl_reg1 |= rate << MAG3110_CTRL_DR_SHIFT;
 		return i2c_smbus_write_byte_data(data->client,
 			MAG3110_CTRL_REG1, data->ctrl_reg1);
+	case IIO_CHAN_INFO_CALIBBIAS:
+		if (val < -10000 || val > 10000)
+			return -EINVAL;
+		return i2c_smbus_write_word_swapped(data->client,
+			MAG3110_OFF_X + 2 * chan->scan_index, val << 1);
 	default:
 		return -EINVAL;
 	}
@@ -261,7 +273,8 @@ done:
 	.type = IIO_MAGN, \
 	.modified = 1, \
 	.channel2 = IIO_MOD_##axis, \
-	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+		BIT(IIO_CHAN_INFO_CALIBBIAS), \
 	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
 		BIT(IIO_CHAN_INFO_SCALE), \
 	.scan_index = idx, \
@@ -346,7 +359,7 @@ static int mag3110_probe(struct i2c_client *client,
 		return ret;
 
 	ret = i2c_smbus_write_byte_data(client, MAG3110_CTRL_REG2,
-		MAG3110_CTRL_AUTO_MRST_EN | MAG3110_CTRL_RAW);
+		MAG3110_CTRL_AUTO_MRST_EN);
 	if (ret < 0)
 		return ret;
 
-- 
1.8.3.2

Re: [PATCH 5/5] iio:magnetometer:mag3110: Enable user offset calibration

[Jonathan Cameron]

On 10/01/14 21:59, Peter Meerwald wrote:
> add IIO_CHAN_INFO_CALIBBIAS
>
> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
Looks good, but held until the fixes have made it into staging-next.
> ---
>   drivers/iio/magnetometer/mag3110.c | 17 +++++++++++++++--
>   1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/magnetometer/mag3110.c b/drivers/iio/magnetometer/mag3110.c
> index 9e774de..9f260a5 100644
> --- a/drivers/iio/magnetometer/mag3110.c
> +++ b/drivers/iio/magnetometer/mag3110.c
> @@ -200,6 +200,13 @@ static int mag3110_read_raw(struct iio_dev *indio_dev,
>   		*val = mag3110_samp_freq[i][0];
>   		*val2 = mag3110_samp_freq[i][1];
>   		return IIO_VAL_INT_PLUS_MICRO;
> +	case IIO_CHAN_INFO_CALIBBIAS:
> +		ret = i2c_smbus_read_word_swapped(data->client,
> +			MAG3110_OFF_X +	2 * chan->scan_index);
> +		if (ret < 0)
> +			return ret;
> +		*val = sign_extend32(ret >> 1, 14);
> +		return IIO_VAL_INT;
>   	}
>   	return -EINVAL;
>   }
> @@ -224,6 +231,11 @@ static int mag3110_write_raw(struct iio_dev *indio_dev,
>   		data->ctrl_reg1 |= rate << MAG3110_CTRL_DR_SHIFT;
>   		return i2c_smbus_write_byte_data(data->client,
>   			MAG3110_CTRL_REG1, data->ctrl_reg1);
> +	case IIO_CHAN_INFO_CALIBBIAS:
> +		if (val < -10000 || val > 10000)
> +			return -EINVAL;
> +		return i2c_smbus_write_word_swapped(data->client,
> +			MAG3110_OFF_X + 2 * chan->scan_index, val << 1);
>   	default:
>   		return -EINVAL;
>   	}
> @@ -261,7 +273,8 @@ done:
>   	.type = IIO_MAGN, \
>   	.modified = 1, \
>   	.channel2 = IIO_MOD_##axis, \
> -	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> +		BIT(IIO_CHAN_INFO_CALIBBIAS), \
>   	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
>   		BIT(IIO_CHAN_INFO_SCALE), \
>   	.scan_index = idx, \
> @@ -346,7 +359,7 @@ static int mag3110_probe(struct i2c_client *client,
>   		return ret;
>
>   	ret = i2c_smbus_write_byte_data(client, MAG3110_CTRL_REG2,
> -		MAG3110_CTRL_AUTO_MRST_EN | MAG3110_CTRL_RAW);
> +		MAG3110_CTRL_AUTO_MRST_EN);
>   	if (ret < 0)
>   		return ret;
>
>

Re: [PATCH 1/5] iio:magnetometer:mag3110: Report busy in _read_raw() / write_raw() when buffer is enabled

[Jonathan Cameron]

On 10/01/14 21:59, Peter Meerwald wrote:
> individual reads are not permitted concurrently with buffered reads
>
This isn't technically a requirement.  The reasoning behind doing it
is normally that a userspace read could mess up the timing of a fixed
frequency triggered signal.  I guess that's true here but is probably
not a strong enough reason to make an ABI change.

Perhaps an ammended description to describe the reasoning for not
allowing it in this driver?

While obscure this is an ABI change as reads that would previously
have succeeded will fail (I doubt anyone will notice, but you never
know...)

The arguement for protecting against changes in the sampling frequency
is perhaps a little stronger as that may cause nasty issues in the buffered
read...
> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
> ---
>   drivers/iio/magnetometer/mag3110.c | 6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/drivers/iio/magnetometer/mag3110.c b/drivers/iio/magnetometer/mag3110.c
> index 4b65b6d..b88cb44 100644
> --- a/drivers/iio/magnetometer/mag3110.c
> +++ b/drivers/iio/magnetometer/mag3110.c
> @@ -154,6 +154,9 @@ static int mag3110_read_raw(struct iio_dev *indio_dev,
>
>   	switch (mask) {
>   	case IIO_CHAN_INFO_RAW:
> +		if (iio_buffer_enabled(indio_dev))
> +			return -EBUSY;
> +
>   		switch (chan->type) {
>   		case IIO_MAGN: /* in 0.1 uT / LSB */
>   			ret = mag3110_read(data, buffer);
> @@ -199,6 +202,9 @@ static int mag3110_write_raw(struct iio_dev *indio_dev,
>   	struct mag3110_data *data = iio_priv(indio_dev);
>   	int rate;
>
> +	if (iio_buffer_enabled(indio_dev))
> +		return -EBUSY;
> +
>   	switch (mask) {
>   	case IIO_CHAN_INFO_SAMP_FREQ:
>   		rate = mag3110_get_samp_freq_index(data, val, val2);
>

Re: [PATCH 1/5] iio:magnetometer:mag3110: Report busy in _read_raw() / write_raw() when buffer is enabled

[Jonathan Cameron]

On 11/01/14 12:07, Jonathan Cameron wrote:
> On 10/01/14 21:59, Peter Meerwald wrote:
>> individual reads are not permitted concurrently with buffered reads
>>
> This isn't technically a requirement.  The reasoning behind doing it
> is normally that a userspace read could mess up the timing of a fixed
> frequency triggered signal.  I guess that's true here but is probably
> not a strong enough reason to make an ABI change.
>
> Perhaps an ammended description to describe the reasoning for not
> allowing it in this driver?
>
> While obscure this is an ABI change as reads that would previously
> have succeeded will fail (I doubt anyone will notice, but you never
> know...)
>
> The arguement for protecting against changes in the sampling frequency
> is perhaps a little stronger as that may cause nasty issues in the buffered
> read...

Ah, I'd forgotten we only merged this very recently.  I think this counts as a fix,
so will queue it up for after the merge window.  As the original ABI was never in
a release kernel we can change it without worrying ;)

There's a new temporary branch for iio.git containing this
fixes-for-3.14new


>> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
>> ---
>>   drivers/iio/magnetometer/mag3110.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/iio/magnetometer/mag3110.c b/drivers/iio/magnetometer/mag3110.c
>> index 4b65b6d..b88cb44 100644
>> --- a/drivers/iio/magnetometer/mag3110.c
>> +++ b/drivers/iio/magnetometer/mag3110.c
>> @@ -154,6 +154,9 @@ static int mag3110_read_raw(struct iio_dev *indio_dev,
>>
>>       switch (mask) {
>>       case IIO_CHAN_INFO_RAW:
>> +        if (iio_buffer_enabled(indio_dev))
>> +            return -EBUSY;
>> +
>>           switch (chan->type) {
>>           case IIO_MAGN: /* in 0.1 uT / LSB */
>>               ret = mag3110_read(data, buffer);
>> @@ -199,6 +202,9 @@ static int mag3110_write_raw(struct iio_dev *indio_dev,
>>       struct mag3110_data *data = iio_priv(indio_dev);
>>       int rate;
>>
>> +    if (iio_buffer_enabled(indio_dev))
>> +        return -EBUSY;
>> +
>>       switch (mask) {
>>       case IIO_CHAN_INFO_SAMP_FREQ:
>>           rate = mag3110_get_samp_freq_index(data, val, val2);
>>
> --
> 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