All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/1] drivers:iio:light:isl29125: added macros for sensing range
@ 2016-06-24 11:16 Bijosh Thykkoottathil
  2016-06-26 13:56 ` Jonathan Cameron
  0 siblings, 1 reply; 3+ messages in thread
From: Bijosh Thykkoottathil @ 2016-06-24 11:16 UTC (permalink / raw)
  To: jic23, knaack.h, lars, pmeerw, k.kozlowski; +Cc: linux-iio, linux-kernel

Added macros for sensing range as the corresponding magic numbers
were used at multiple places.
   - SENSING_RANGE_0 for 375 lux full range
   - SENSING_RANGE_1 for 10k lux full range

Signed-off-by: Bijosh Thykkoottathil <bijosh.t@hotmail.com>
---
 drivers/iio/light/isl29125.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/light/isl29125.c b/drivers/iio/light/isl29125.c
index e2945a2..83a5a7a 100644
--- a/drivers/iio/light/isl29125.c
+++ b/drivers/iio/light/isl29125.c
@@ -44,6 +44,9 @@
 #define ISL29125_MODE_B 0x3
 #define ISL29125_MODE_RGB 0x5
=20
+#define SENSING_RANGE_0 5722   /* 375 lux full range */
+#define SENSING_RANGE_1 152590 /* 10k lux full range */
+
 #define ISL29125_MODE_RANGE BIT(3)
=20
 #define ISL29125_STATUS_CONV BIT(1)
@@ -140,9 +143,9 @@ static int isl29125_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_SCALE:
 		*val =3D 0;
 		if (data->conf1 & ISL29125_MODE_RANGE)
-			*val2 =3D 152590; /* 10k lux full range */
+			*val2 =3D SENSING_RANGE_1; /* 10k lux full range */
 		else
-			*val2 =3D 5722; /* 375 lux full range */
+			*val2 =3D SENSING_RANGE_0; /* 375 lux full range */
 		return IIO_VAL_INT_PLUS_MICRO;
 	}
 	return -EINVAL;
@@ -158,9 +161,9 @@ static int isl29125_write_raw(struct iio_dev *indio_dev=
,
 	case IIO_CHAN_INFO_SCALE:
 		if (val !=3D 0)
 			return -EINVAL;
-		if (val2 =3D=3D 152590)
+		if (val2 =3D=3D SENSING_RANGE_1)
 			data->conf1 |=3D ISL29125_MODE_RANGE;
-		else if (val2 =3D=3D 5722)
+		else if (val2 =3D=3D SENSING_RANGE_0)
 			data->conf1 &=3D ~ISL29125_MODE_RANGE;
 		else
 			return -EINVAL;
--=20
2.1.4

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH 1/1] drivers:iio:light:isl29125: added macros for sensing range
  2016-06-24 11:16 [PATCH 1/1] drivers:iio:light:isl29125: added macros for sensing range Bijosh Thykkoottathil
@ 2016-06-26 13:56 ` Jonathan Cameron
  2016-06-27 13:56   ` Peter Meerwald-Stadler
  0 siblings, 1 reply; 3+ messages in thread
From: Jonathan Cameron @ 2016-06-26 13:56 UTC (permalink / raw)
  To: Bijosh Thykkoottathil, knaack.h, lars, pmeerw, k.kozlowski
  Cc: linux-iio, linux-kernel

On 24/06/16 12:16, Bijosh Thykkoottathil wrote:
> Added macros for sensing range as the corresponding magic numbers
> were used at multiple places.
>    - SENSING_RANGE_0 for 375 lux full range
>    - SENSING_RANGE_1 for 10k lux full range
> 
> Signed-off-by: Bijosh Thykkoottathil <bijosh.t@hotmail.com>
Hi Bijosh,

Macros are great for making cases of 'magic numbers' more readable.
However here we are wrapping up 'real' numbers in macros that actually
obscure what they are...

Now the fact they are used twice does make it a little more worthwhile.
Peter, what do you think?  Your driver so up to you ;)
*one buck passed*
> ---
>  drivers/iio/light/isl29125.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/light/isl29125.c b/drivers/iio/light/isl29125.c
> index e2945a2..83a5a7a 100644
> --- a/drivers/iio/light/isl29125.c
> +++ b/drivers/iio/light/isl29125.c
> @@ -44,6 +44,9 @@
>  #define ISL29125_MODE_B 0x3
>  #define ISL29125_MODE_RGB 0x5
>  
> +#define SENSING_RANGE_0 5722   /* 375 lux full range */
> +#define SENSING_RANGE_1 152590 /* 10k lux full range */
> +
>  #define ISL29125_MODE_RANGE BIT(3)
>  
>  #define ISL29125_STATUS_CONV BIT(1)
> @@ -140,9 +143,9 @@ static int isl29125_read_raw(struct iio_dev *indio_dev,
>  	case IIO_CHAN_INFO_SCALE:
>  		*val = 0;
>  		if (data->conf1 & ISL29125_MODE_RANGE)
> -			*val2 = 152590; /* 10k lux full range */
> +			*val2 = SENSING_RANGE_1; /* 10k lux full range */
>  		else
> -			*val2 = 5722; /* 375 lux full range */
> +			*val2 = SENSING_RANGE_0; /* 375 lux full range */
>  		return IIO_VAL_INT_PLUS_MICRO;
>  	}
>  	return -EINVAL;
> @@ -158,9 +161,9 @@ static int isl29125_write_raw(struct iio_dev *indio_dev,
>  	case IIO_CHAN_INFO_SCALE:
>  		if (val != 0)
>  			return -EINVAL;
> -		if (val2 == 152590)
> +		if (val2 == SENSING_RANGE_1)
>  			data->conf1 |= ISL29125_MODE_RANGE;
> -		else if (val2 == 5722)
> +		else if (val2 == SENSING_RANGE_0)
>  			data->conf1 &= ~ISL29125_MODE_RANGE;
>  		else
>  			return -EINVAL;
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH 1/1] drivers:iio:light:isl29125: added macros for sensing range
  2016-06-26 13:56 ` Jonathan Cameron
@ 2016-06-27 13:56   ` Peter Meerwald-Stadler
  0 siblings, 0 replies; 3+ messages in thread
From: Peter Meerwald-Stadler @ 2016-06-27 13:56 UTC (permalink / raw)
  To: Bijosh Thykkoottathil, Jonathan Cameron
  Cc: knaack.h, lars, k.kozlowski, linux-iio, linux-kernel


> On 24/06/16 12:16, Bijosh Thykkoottathil wrote:
> > Added macros for sensing range as the corresponding magic numbers
> > were used at multiple places.
> >    - SENSING_RANGE_0 for 375 lux full range
> >    - SENSING_RANGE_1 for 10k lux full range

> Macros are great for making cases of 'magic numbers' more readable.
> However here we are wrapping up 'real' numbers in macros that actually
> obscure what they are...
> 
> Now the fact they are used twice does make it a little more worthwhile.
> Peter, what do you think?  Your driver so up to you ;)

I'm fine with the proposed change, but a proper prefix is required, hence
ISL29125_SENSING_RANGE_0 and _1

regards, p.

> > ---
> >  drivers/iio/light/isl29125.c | 11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/iio/light/isl29125.c b/drivers/iio/light/isl29125.c
> > index e2945a2..83a5a7a 100644
> > --- a/drivers/iio/light/isl29125.c
> > +++ b/drivers/iio/light/isl29125.c
> > @@ -44,6 +44,9 @@
> >  #define ISL29125_MODE_B 0x3
> >  #define ISL29125_MODE_RGB 0x5
> >  
> > +#define SENSING_RANGE_0 5722   /* 375 lux full range */
> > +#define SENSING_RANGE_1 152590 /* 10k lux full range */
> > +
> >  #define ISL29125_MODE_RANGE BIT(3)
> >  
> >  #define ISL29125_STATUS_CONV BIT(1)
> > @@ -140,9 +143,9 @@ static int isl29125_read_raw(struct iio_dev *indio_dev,
> >  	case IIO_CHAN_INFO_SCALE:
> >  		*val = 0;
> >  		if (data->conf1 & ISL29125_MODE_RANGE)
> > -			*val2 = 152590; /* 10k lux full range */
> > +			*val2 = SENSING_RANGE_1; /* 10k lux full range */
> >  		else
> > -			*val2 = 5722; /* 375 lux full range */
> > +			*val2 = SENSING_RANGE_0; /* 375 lux full range */
> >  		return IIO_VAL_INT_PLUS_MICRO;
> >  	}
> >  	return -EINVAL;
> > @@ -158,9 +161,9 @@ static int isl29125_write_raw(struct iio_dev *indio_dev,
> >  	case IIO_CHAN_INFO_SCALE:
> >  		if (val != 0)
> >  			return -EINVAL;
> > -		if (val2 == 152590)
> > +		if (val2 == SENSING_RANGE_1)
> >  			data->conf1 |= ISL29125_MODE_RANGE;
> > -		else if (val2 == 5722)
> > +		else if (val2 == SENSING_RANGE_0)
> >  			data->conf1 &= ~ISL29125_MODE_RANGE;
> >  		else
> >  			return -EINVAL;
> > 
> 

-- 

Peter Meerwald-Stadler
+43-664-2444418 (mobile)

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-06-27 13:56 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-24 11:16 [PATCH 1/1] drivers:iio:light:isl29125: added macros for sensing range Bijosh Thykkoottathil
2016-06-26 13:56 ` Jonathan Cameron
2016-06-27 13:56   ` Peter Meerwald-Stadler

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.