linux-kernel - Re: [PATCH v2 2/3] iio: light: ltr390: Add ALS channel and support for gain and resolution

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240728175841.7ca04e58@jic23-huawei>
Date: Sun, 28 Jul 2024 17:58:41 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Abhash Jha <abhashkumarjha123@...il.com>
Cc: linux-iio@...r.kernel.org, anshulusr@...il.com, lars@...afoo.de,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/3] iio: light: ltr390: Add ALS channel and support
 for gain and resolution

On Sun, 28 Jul 2024 20:49:55 +0530
Abhash Jha <abhashkumarjha123@...il.com> wrote:

>     Add new ALS channel and allow reading raw and scale values.
>     Also provide gain and resolution configuration for ALS channel.
>     Add automatic mode switching between the UVS and ALS channel
>     based on which channel is being accessed.
>     The default mode in which the sensor start is ALS mode.
Same issue with indent as in patch 1.

> 
> Signed-off-by: Abhash Jha <abhashkumarjha123@...il.com>
Later patches should not be in reply to the first one.
Use a cover letter.

Minor comments inline on using the enum to maintain the type of the mode
setting.


> ---
>  drivers/iio/light/ltr390.c | 111 ++++++++++++++++++++++++++++++++-----
>  1 file changed, 96 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/iio/light/ltr390.c b/drivers/iio/light/ltr390.c
> index 9f00661c3..d1a259141 100644
> --- a/drivers/iio/light/ltr390.c
> +++ b/drivers/iio/light/ltr390.c
> @@ -62,11 +62,17 @@
>   */
>  #define LTR390_WINDOW_FACTOR 1
>  
> +enum ltr390_mode {
> +	LTR390_SET_ALS_MODE,
> +	LTR390_SET_UVS_MODE,
> +};
> +
>  struct ltr390_data {
>  	struct regmap *regmap;
>  	struct i2c_client *client;
>  	/* Protects device from simulataneous reads */
>  	struct mutex lock;
> +	int mode;
enum ltr380_mode mode;

>  	int gain;
>  	int int_time_us;
>  };
> @@ -94,6 +100,28 @@ static int ltr390_register_read(struct ltr390_data *data, u8 register_address)
>  	return get_unaligned_le24(recieve_buffer);
>  }
>  
> +static int ltr390_set_mode(struct ltr390_data *data, int mode)

pass in the enum lt380_mode.

> +{
> +	if (data->mode == mode)
> +		return 0;
> +
> +	switch (mode) {
> +	case LTR390_SET_ALS_MODE:
> +		regmap_clear_bits(data->regmap, LTR390_MAIN_CTRL, LTR390_UVS_MODE);
> +		break;
> +
> +	case LTR390_SET_UVS_MODE:
> +		regmap_set_bits(data->regmap, LTR390_MAIN_CTRL, LTR390_UVS_MODE);
> +		break;
> +
> +	default:
> +		return -EINVAL;
Should be able to drop this if passing in the enum as the compiler can see you
have cases for every value.

> +	}
> +
> +	data->mode = mode;
> +	return 0;
> +}
> +