linux-kernel - Re: [PATCH 3/6] iio: magnetometer: mmc35240: Fix broken processed value

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Wed, 3 Jun 2015 10:44:11 +0200 (CEST)
From:	Peter Meerwald <pmeerw@...erw.net>
To:	Daniel Baluta <daniel.baluta@...el.com>
cc:	jic23@...nel.org, knaack.h@....de, lars@...afoo.de,
	linux-kernel@...r.kernel.org, linux-iio@...r.kernel.org
Subject: Re: [PATCH 3/6] iio: magnetometer: mmc35240: Fix broken processed
 value

On Wed, 3 Jun 2015, Daniel Baluta wrote:

> The current computation for fractional part of the magnetic
> field is broken. This patch fixes it by taking a different
> approach. We expose the raw reading in mili Gauss (to avoid
> rounding errors) with a scale of 0.001.

milli

comments below

> 
> Thus the final computation is done in userspace where floating
> point operation are more relaxed.
> 
> Fixes: abeb6b1e7b ("iio: magnetometer: Add support for MEMSIC MMC35240")
> Signed-off-by: Daniel Baluta <daniel.baluta@...el.com>
> ---
>  drivers/iio/magnetometer/mmc35240.c | 43 +++++++++++++++++++++++--------------
>  1 file changed, 27 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/iio/magnetometer/mmc35240.c b/drivers/iio/magnetometer/mmc35240.c
> index 9d62c53..c520117 100644
> --- a/drivers/iio/magnetometer/mmc35240.c
> +++ b/drivers/iio/magnetometer/mmc35240.c
> @@ -113,8 +113,9 @@ static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("100 200 333 666");
>  	.modified = 1, \
>  	.channel2 = IIO_MOD_ ## _axis, \
>  	.address = AXIS_ ## _axis, \
> -	.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), \
> -	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ) |\

probably a space before \

> +			BIT(IIO_CHAN_INFO_SCALE),  \
>  }
>  
>  static const struct iio_chan_spec mmc35240_channels[] = {
> @@ -241,9 +242,19 @@ static int mmc35240_read_measurement(struct mmc35240_data *data, __le16 buf[3])
>  				3 * sizeof(__le16));
>  }
>  
> -static int mmc35240_raw_to_gauss(struct mmc35240_data *data, int index,
> -				 __le16 buf[],
> -				 int *val, int *val2)
> +/**
> + * mmc35240_raw_to_mgauss - convert raw readings to mili gauss. Also apply

milli

> +			    compensation for output value.
> + *
> + * @data: device private data
> + * @index: axis index for which we want the conversion
> + * @buf: raw data to be converted, 2 bytes in little endian format
> + * @val: compensated output reading (unit is mili gauss)

milli

> + *
> + * Returns: 0 in case of success, -EINVAL when @index is not valid
> + */
> +static int mmc35240_raw_to_mgauss(struct mmc35240_data *data, int index,
> +				  __le16 buf[], int *val)
>  {
>  	int raw_x, raw_y, raw_z;
>  	int sens_x, sens_y, sens_z;
> @@ -261,18 +272,15 @@ static int mmc35240_raw_to_gauss(struct mmc35240_data *data, int index,
>  
>  	switch (index) {
>  	case AXIS_X:
> -		*val = (raw_x - nfo) / sens_x;
> -		*val2 = ((raw_x - nfo) % sens_x) * 1000000;
> +		*val = (raw_x - nfo) * 1000 / sens_x;
>  		break;
>  	case AXIS_Y:
> -		*val = (raw_y - nfo) / sens_y - (raw_z - nfo) / sens_z;
> -		*val2 = (((raw_y - nfo) % sens_y - (raw_z - nfo) % sens_z))
> -			* 1000000;
> +		*val = (raw_y - nfo) * 1000 / sens_y -
> +			(raw_z - nfo)  * 1000 / sens_z;
>  		break;
>  	case AXIS_Z:
> -		*val = (raw_y - nfo) / sens_y + (raw_z - nfo) / sens_z;
> -		*val2 = (((raw_y - nfo) % sens_y + (raw_z - nfo) % sens_z))
> -			* 1000000;
> +		*val = (raw_y - nfo) * 1000 / sens_y +
> +			(raw_z - nfo) * 1000 / sens_z;
>  		break;
>  	default:
>  		return -EINVAL;
> @@ -290,16 +298,19 @@ static int mmc35240_read_raw(struct iio_dev *indio_dev,
>  	__le16 buf[3];
>  
>  	switch (mask) {
> -	case IIO_CHAN_INFO_PROCESSED:
> +	case IIO_CHAN_INFO_RAW:
>  		mutex_lock(&data->mutex);
>  		ret = mmc35240_read_measurement(data, buf);
>  		mutex_unlock(&data->mutex);
>  		if (ret < 0)
>  			return ret;
> -		ret = mmc35240_raw_to_gauss(data, chan->address,
> -					    buf, val, val2);
> +		ret = mmc35240_raw_to_mgauss(data, chan->address, buf, val);
>  		if (ret < 0)
>  			return ret;
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = 0;
> +		*val2 = 1000;
>  		return IIO_VAL_INT_PLUS_MICRO;
>  	case IIO_CHAN_INFO_SAMP_FREQ:
>  		mutex_lock(&data->mutex);
> 

-- 

Peter Meerwald
+43-664-2444418 (mobile)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/