mbox series

[v7,0/2] Add light channel for LTR390

Message ID 20240814113135.14575-1-abhashkumarjha123@gmail.com (mailing list archive)
Headers show
Series Add light channel for LTR390 | expand

Message

Abhash Jha Aug. 14, 2024, 11:31 a.m. UTC
Hello,

The first patch adds a new channel for the ALS feature of the sensor.
The same configuration of gain and resolution has to be provided for this
channel as well. As there are two IIO channels now, we would need to
switch the sensor's mode of operation depending on which sensor is being
accessed. Hence, mode switching is also provided.

Then the second patch adds support for calculating `counts_per_uvi` based
on the current gain and resolution value.

Changes in v7:
- Changed the `ltr390_set_mode` function to do better error handling.
- Link to v6: https://lore.kernel.org/linux-iio/20240803180950.32821-1-abhashkumarjha123@gmail.com/T/#t

Changes in v6:
- Changed IIO_CHAN_INFO_PROCESSED to IIO_CHAN_INFO_RAW
- Changed the scaling code
- Link to v5: https://lore.kernel.org/linux-iio/CAG=0Rq+q0WJzMroYwQy-4Ng0aSkTvaw-FEMx68i3MqAZwfteCg@mail.gmail.com/T/#t

Changes in v5:
- Replaced the IIO_INTENSITY channel with IIO_LIGHT channel
- We calculate the lux value directly using `als_data / (gain * int_time)`
- Provided a scale channel where the scale is 0.6 * WINDOW_FACTOR
- Link to v4: https://lore.kernel.org/linux-iio/20240730065822.5707-1-abhashkumarjha123@gmail.com/T/#m

Changes in v4:
- Added "bitfield.h" include to fix `-Wimplicit-function-declaration`.
- Link to v3: https://lore.kernel.org/linux-iio/20240729115056.355466-1-abhashkumarjha123@gmail.com/

Changes in v3:
- Added cover letter to the patch series.
- Fixed indentation in the patch description.
- Patch specific changes are listed below.

[PATCH v3 1/3]
	- Cleaned up the spurious changes made in v2.
	- ltr390_set_int_time and ltr390_set_gain now return -EINVAL to
	indicate no match.

[PATCH v3 2/3]
	- Used enum ltr390_mode inside the ltr390_data struct.
	- Refactored `ltr390_set_mode` function according to the comments in v2.

[PATCH v3 3/3]
	- Simplified the formula for `counts_per_uvi` calculation.
	- Removed spurious whitespace changes introduced in v2.

- Link to v2: https://lore.kernel.org/linux-iio/20240728151957.310237-1-abhashkumarjha123@gmail.com/

Changes in v2:
- Split the single patch into 3 patches.
- Used FIELD_PREP to perform bit shifting.
- Used enum for mode selection instead of defines.
- Fixed indentation and whitespace issues pointed out in the comments
- Replaced `mutex_lock(&data->lock)` with `guard(mutex)(&data->lock)`
- Provided available values for gain and resolution via `read_avail`
  instead of sysfs attributes.
- Refactored `ltr390_set_gain` and `ltr390_set_int_time`.
- Used early returns instead of single exit points.

- Link to v1: https://lore.kernel.org/linux-iio/20240718104947.7384-1-abhashkumarjha123@gmail.com/

Regards,
Abhash

Abhash Jha (2):
  iio: light: ltr390: Add ALS channel and support for gain and
    resolution
  iio: light: ltr390: Calculate 'counts_per_uvi' dynamically

 drivers/iio/light/ltr390.c | 115 ++++++++++++++++++++++++++++++++-----
 1 file changed, 100 insertions(+), 15 deletions(-)

Comments

Jonathan Cameron Aug. 17, 2024, 2:39 p.m. UTC | #1
On Wed, 14 Aug 2024 17:01:32 +0530
Abhash Jha <abhashkumarjha123@gmail.com> wrote:

> Hello,
> 
> The first patch adds a new channel for the ALS feature of the sensor.
> The same configuration of gain and resolution has to be provided for this
> channel as well. As there are two IIO channels now, we would need to
> switch the sensor's mode of operation depending on which sensor is being
> accessed. Hence, mode switching is also provided.
> 
> Then the second patch adds support for calculating `counts_per_uvi` based
> on the current gain and resolution value.

This is v7 mark 2.  I'm confused, but I think I picked up this version
(Seems I'd queued an earlier one and not mentioned it on the list though
so I've dropped that in favour of this).

> 
> Changes in v7:
> - Changed the `ltr390_set_mode` function to do better error handling.
> - Link to v6: https://lore.kernel.org/linux-iio/20240803180950.32821-1-abhashkumarjha123@gmail.com/T/#t
> 
> Changes in v6:
> - Changed IIO_CHAN_INFO_PROCESSED to IIO_CHAN_INFO_RAW
> - Changed the scaling code
> - Link to v5: https://lore.kernel.org/linux-iio/CAG=0Rq+q0WJzMroYwQy-4Ng0aSkTvaw-FEMx68i3MqAZwfteCg@mail.gmail.com/T/#t
> 
> Changes in v5:
> - Replaced the IIO_INTENSITY channel with IIO_LIGHT channel
> - We calculate the lux value directly using `als_data / (gain * int_time)`
> - Provided a scale channel where the scale is 0.6 * WINDOW_FACTOR
> - Link to v4: https://lore.kernel.org/linux-iio/20240730065822.5707-1-abhashkumarjha123@gmail.com/T/#m
> 
> Changes in v4:
> - Added "bitfield.h" include to fix `-Wimplicit-function-declaration`.
> - Link to v3: https://lore.kernel.org/linux-iio/20240729115056.355466-1-abhashkumarjha123@gmail.com/
> 
> Changes in v3:
> - Added cover letter to the patch series.
> - Fixed indentation in the patch description.
> - Patch specific changes are listed below.
> 
> [PATCH v3 1/3]
> 	- Cleaned up the spurious changes made in v2.
> 	- ltr390_set_int_time and ltr390_set_gain now return -EINVAL to
> 	indicate no match.
> 
> [PATCH v3 2/3]
> 	- Used enum ltr390_mode inside the ltr390_data struct.
> 	- Refactored `ltr390_set_mode` function according to the comments in v2.
> 
> [PATCH v3 3/3]
> 	- Simplified the formula for `counts_per_uvi` calculation.
> 	- Removed spurious whitespace changes introduced in v2.
> 
> - Link to v2: https://lore.kernel.org/linux-iio/20240728151957.310237-1-abhashkumarjha123@gmail.com/
> 
> Changes in v2:
> - Split the single patch into 3 patches.
> - Used FIELD_PREP to perform bit shifting.
> - Used enum for mode selection instead of defines.
> - Fixed indentation and whitespace issues pointed out in the comments
> - Replaced `mutex_lock(&data->lock)` with `guard(mutex)(&data->lock)`
> - Provided available values for gain and resolution via `read_avail`
>   instead of sysfs attributes.
> - Refactored `ltr390_set_gain` and `ltr390_set_int_time`.
> - Used early returns instead of single exit points.
> 
> - Link to v1: https://lore.kernel.org/linux-iio/20240718104947.7384-1-abhashkumarjha123@gmail.com/
> 
> Regards,
> Abhash
> 
> Abhash Jha (2):
>   iio: light: ltr390: Add ALS channel and support for gain and
>     resolution
>   iio: light: ltr390: Calculate 'counts_per_uvi' dynamically
> 
>  drivers/iio/light/ltr390.c | 115 ++++++++++++++++++++++++++++++++-----
>  1 file changed, 100 insertions(+), 15 deletions(-)
>
Abhash Jha Aug. 17, 2024, 3:50 p.m. UTC | #2
> This is v7 mark 2.  I'm confused, but I think I picked up this version
> (Seems I'd queued an earlier one and not mentioned it on the list though
> so I've dropped that in favour of this).
>
Does that mean that you have picked up the patch 1/2 and 2/2 of the v7 series
as well ?

Thanks,
Abhash
Jonathan Cameron Aug. 17, 2024, 4:49 p.m. UTC | #3
On Sat, 17 Aug 2024 21:20:10 +0530
Abhash jha <abhashkumarjha123@gmail.com> wrote:

> > This is v7 mark 2.  I'm confused, but I think I picked up this version
> > (Seems I'd queued an earlier one and not mentioned it on the list though
> > so I've dropped that in favour of this).
> >  
> Does that mean that you have picked up the patch 1/2 and 2/2 of the v7 series
> as well ?
I think I have.  But with two versions of v7 I'm not 100% sure which one got picked
up. I've pushed out now as testing, so take a look. 

https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/log/?h=testing
> 
> Thanks,
> Abhash
Abhash Jha Aug. 17, 2024, 5:37 p.m. UTC | #4
> I think I have.  But with two versions of v7 I'm not 100% sure which one got picked
> up. I've pushed out now as testing, so take a look.
>
> https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/log/?h=testing
> >
The two versions v7 patches are the same. I had sent the same thing
again because
I thought it might have gotten lost in your mail.
My apologies for getting you confused.

Thanks,
Abhash
Jonathan Cameron Aug. 18, 2024, 4:05 p.m. UTC | #5
On Sat, 17 Aug 2024 23:07:37 +0530
Abhash jha <abhashkumarjha123@gmail.com> wrote:

> > I think I have.  But with two versions of v7 I'm not 100% sure which one got picked
> > up. I've pushed out now as testing, so take a look.
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/log/?h=testing  
> > >  
> The two versions v7 patches are the same. I had sent the same thing
> again because
> I thought it might have gotten lost in your mail.
> My apologies for getting you confused.
Ah. Never bother doing that.  Just send a 'ping' to the original
thread.  

Most maintainers now use a lot of automation so tend not to drop messages
any more (it used to happen occasionally). 

Also convention is to wait at least 2 weeks before pinging.

Jonathan

> 
> Thanks,
> Abhash