diff mbox

[5/6] Staging: iio: adis16209: Add some informatic comments

Message ID 11a9db0dda77749c865e864c81924c0ec578dd86.1519995673.git.shreeya.patel23498@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Shreeya Patel March 2, 2018, 1:32 p.m. UTC
Some of the register names does not make it's puporse
very clear and hence, add some comments for more
information.
Also there are certain unit based comments which are not
providing sufficient information, so expand those comments.

Signed-off-by: Shreeya Patel <shreeya.patel23498@gmail.com>
---
 drivers/staging/iio/accel/adis16209.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

Comments

Shreeya Patel March 3, 2018, 3:33 p.m. UTC | #1
On Sat, 2018-03-03 at 16:01 +0000, Jonathan Cameron wrote:
> On Fri,  2 Mar 2018 19:02:48 +0530
> Shreeya Patel <shreeya.patel23498@gmail.com> wrote:
> 
> > 
> > Some of the register names does not make it's puporse
> > very clear and hence, add some comments for more
> > information.
> > Also there are certain unit based comments which are not
> > providing sufficient information, so expand those comments.
> Ah - serves me right for not reading on before commenting on the
> previous
> patch.  It would have been preferable to have merged at least some of
> this
> in there as they needed to be read together.
> 
> One comment in here doesn't quite cover everything I think should
> be explained.
> 
> Please fix that and merge this down with the previous patch
> (interactive rebase and marking it as a fixup makes this easy).
> 
> Thanks,
> 
> Jonathan
> 
> > 
> > 
> > Signed-off-by: Shreeya Patel <shreeya.patel23498@gmail.com>
> > ---
> >  drivers/staging/iio/accel/adis16209.c | 21 ++++++++++++++++++---
> >  1 file changed, 18 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/staging/iio/accel/adis16209.c
> > b/drivers/staging/iio/accel/adis16209.c
> > index d2d1254..7363fd0 100644
> > --- a/drivers/staging/iio/accel/adis16209.c
> > +++ b/drivers/staging/iio/accel/adis16209.c
> > @@ -27,13 +27,18 @@
> >  #define ADIS16209_SUPPLY_OUT_REG	0x02
> >  #define ADIS16209_XACCL_OUT_REG		0x04
> >  #define ADIS16209_YACCL_OUT_REG		0x06
> > +/* Output, auxiliary ADC */
> >  #define ADIS16209_AUX_ADC_REG		0x08
> > +/* Output, temperature */
> >  #define ADIS16209_TEMP_OUT_REG		0x0A
> > +/* Output, +/- 90 degrees X-axis inclination */
> >  #define ADIS16209_XINCL_OUT_REG		0x0C
> >  #define ADIS16209_YINCL_OUT_REG		0x0E
> >  #define ADIS16209_ROT_OUT_REG		0x10
> >  
> > -/* Calibration Register Definitions */
> > +/* Calibration Register Definitions.
> > + * Acceleration, inclination or rotation offset null.
> > + */
> >  #define ADIS16209_XACCL_NULL_REG	0x12
> >  #define ADIS16209_YACCL_NULL_REG	0x14
> >  #define ADIS16209_XINCL_NULL_REG	0x16
> > @@ -155,19 +160,29 @@ static int adis16209_read_raw(struct iio_dev
> > *indio_dev,
> >  			*val2 = 0;
> >  			return IIO_VAL_INT_PLUS_MICRO;
> >  		case IIO_ACCEL:
> > +			/*
> > +			 * IIO base unit for sensitivity of
> > accelerometer
> > +			 * is milligram.
> > +			 * 1 LSB represents 0.244 milligrams.
> Not miligrams. Milli g where 1 g is the 'standard' acceleration due
> to gravity.

Ah!!
Should have used my common sense here :(
Sorry for such mistake.

> 
> > 
> > +			 */
> >  			*val = 0;
> > -			*val2 = IIO_G_TO_M_S_2(244140); /*
> > 0.244140 mg */
> > +			*val2 = IIO_G_TO_M_S_2(244140);
> >  			return IIO_VAL_INT_PLUS_NANO;
> >  		case IIO_INCLI:
> >  		case IIO_ROT:
> > +			/*
> > +			 * IIO base units for rotation are
> > degrees.
> > +			 * 1 LSB represents 0.025 milli degrees.
> > +			 */
> >  			*val = 0;
> > -			*val2 = 25000; /* 0.025 degree */
> > +			*val2 = 25000;
> >  			return IIO_VAL_INT_PLUS_MICRO;
> >  		default:
> >  			return -EINVAL;
> >  		}
> >  		break;
> >  	case IIO_CHAN_INFO_OFFSET:
> > +		/* TEMP_OUT_REG has a scale factor of -0.47
> > degrees celcius. */
> This doesn't explain the magic 0x4FE so that needs doing as well.
> 
> > 
> >  		*val = 25000 / -470 - 0x4FE;
> >  		return IIO_VAL_INT;
> >  	case IIO_CHAN_INFO_CALIBBIAS:
--
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
Jonathan Cameron March 3, 2018, 4:01 p.m. UTC | #2
On Fri,  2 Mar 2018 19:02:48 +0530
Shreeya Patel <shreeya.patel23498@gmail.com> wrote:

> Some of the register names does not make it's puporse
> very clear and hence, add some comments for more
> information.
> Also there are certain unit based comments which are not
> providing sufficient information, so expand those comments.
Ah - serves me right for not reading on before commenting on the previous
patch.  It would have been preferable to have merged at least some of this
in there as they needed to be read together.

One comment in here doesn't quite cover everything I think should
be explained.

Please fix that and merge this down with the previous patch
(interactive rebase and marking it as a fixup makes this easy).

Thanks,

Jonathan

> 
> Signed-off-by: Shreeya Patel <shreeya.patel23498@gmail.com>
> ---
>  drivers/staging/iio/accel/adis16209.c | 21 ++++++++++++++++++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/iio/accel/adis16209.c b/drivers/staging/iio/accel/adis16209.c
> index d2d1254..7363fd0 100644
> --- a/drivers/staging/iio/accel/adis16209.c
> +++ b/drivers/staging/iio/accel/adis16209.c
> @@ -27,13 +27,18 @@
>  #define ADIS16209_SUPPLY_OUT_REG	0x02
>  #define ADIS16209_XACCL_OUT_REG		0x04
>  #define ADIS16209_YACCL_OUT_REG		0x06
> +/* Output, auxiliary ADC */
>  #define ADIS16209_AUX_ADC_REG		0x08
> +/* Output, temperature */
>  #define ADIS16209_TEMP_OUT_REG		0x0A
> +/* Output, +/- 90 degrees X-axis inclination */
>  #define ADIS16209_XINCL_OUT_REG		0x0C
>  #define ADIS16209_YINCL_OUT_REG		0x0E
>  #define ADIS16209_ROT_OUT_REG		0x10
>  
> -/* Calibration Register Definitions */
> +/* Calibration Register Definitions.
> + * Acceleration, inclination or rotation offset null.
> + */
>  #define ADIS16209_XACCL_NULL_REG	0x12
>  #define ADIS16209_YACCL_NULL_REG	0x14
>  #define ADIS16209_XINCL_NULL_REG	0x16
> @@ -155,19 +160,29 @@ static int adis16209_read_raw(struct iio_dev *indio_dev,
>  			*val2 = 0;
>  			return IIO_VAL_INT_PLUS_MICRO;
>  		case IIO_ACCEL:
> +			/*
> +			 * IIO base unit for sensitivity of accelerometer
> +			 * is milligram.
> +			 * 1 LSB represents 0.244 milligrams.
Not miligrams. Milli g where 1 g is the 'standard' acceleration due to gravity.

> +			 */
>  			*val = 0;
> -			*val2 = IIO_G_TO_M_S_2(244140); /* 0.244140 mg */
> +			*val2 = IIO_G_TO_M_S_2(244140);
>  			return IIO_VAL_INT_PLUS_NANO;
>  		case IIO_INCLI:
>  		case IIO_ROT:
> +			/*
> +			 * IIO base units for rotation are degrees.
> +			 * 1 LSB represents 0.025 milli degrees.
> +			 */
>  			*val = 0;
> -			*val2 = 25000; /* 0.025 degree */
> +			*val2 = 25000;
>  			return IIO_VAL_INT_PLUS_MICRO;
>  		default:
>  			return -EINVAL;
>  		}
>  		break;
>  	case IIO_CHAN_INFO_OFFSET:
> +		/* TEMP_OUT_REG has a scale factor of -0.47 degrees celcius. */

This doesn't explain the magic 0x4FE so that needs doing as well.

>  		*val = 25000 / -470 - 0x4FE;
>  		return IIO_VAL_INT;
>  	case IIO_CHAN_INFO_CALIBBIAS:

--
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
diff mbox

Patch

diff --git a/drivers/staging/iio/accel/adis16209.c b/drivers/staging/iio/accel/adis16209.c
index d2d1254..7363fd0 100644
--- a/drivers/staging/iio/accel/adis16209.c
+++ b/drivers/staging/iio/accel/adis16209.c
@@ -27,13 +27,18 @@ 
 #define ADIS16209_SUPPLY_OUT_REG	0x02
 #define ADIS16209_XACCL_OUT_REG		0x04
 #define ADIS16209_YACCL_OUT_REG		0x06
+/* Output, auxiliary ADC */
 #define ADIS16209_AUX_ADC_REG		0x08
+/* Output, temperature */
 #define ADIS16209_TEMP_OUT_REG		0x0A
+/* Output, +/- 90 degrees X-axis inclination */
 #define ADIS16209_XINCL_OUT_REG		0x0C
 #define ADIS16209_YINCL_OUT_REG		0x0E
 #define ADIS16209_ROT_OUT_REG		0x10
 
-/* Calibration Register Definitions */
+/* Calibration Register Definitions.
+ * Acceleration, inclination or rotation offset null.
+ */
 #define ADIS16209_XACCL_NULL_REG	0x12
 #define ADIS16209_YACCL_NULL_REG	0x14
 #define ADIS16209_XINCL_NULL_REG	0x16
@@ -155,19 +160,29 @@  static int adis16209_read_raw(struct iio_dev *indio_dev,
 			*val2 = 0;
 			return IIO_VAL_INT_PLUS_MICRO;
 		case IIO_ACCEL:
+			/*
+			 * IIO base unit for sensitivity of accelerometer
+			 * is milligram.
+			 * 1 LSB represents 0.244 milligrams.
+			 */
 			*val = 0;
-			*val2 = IIO_G_TO_M_S_2(244140); /* 0.244140 mg */
+			*val2 = IIO_G_TO_M_S_2(244140);
 			return IIO_VAL_INT_PLUS_NANO;
 		case IIO_INCLI:
 		case IIO_ROT:
+			/*
+			 * IIO base units for rotation are degrees.
+			 * 1 LSB represents 0.025 milli degrees.
+			 */
 			*val = 0;
-			*val2 = 25000; /* 0.025 degree */
+			*val2 = 25000;
 			return IIO_VAL_INT_PLUS_MICRO;
 		default:
 			return -EINVAL;
 		}
 		break;
 	case IIO_CHAN_INFO_OFFSET:
+		/* TEMP_OUT_REG has a scale factor of -0.47 degrees celcius. */
 		*val = 25000 / -470 - 0x4FE;
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_CALIBBIAS: