Re: [PATCH v3] iio: light: ltr390: Add debugfs register access support

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 24 Jul 2025 20:09:39 +0530
Akshay Jindal <akshayaj.lkd@gmail.com> wrote:

> Add support for debugfs_reg_access through the driver's iio_info structure
> to enable low-level register read/write access for debugging.
> 
> Signed-off-by: Akshay Jindal <akshayaj.lkd@gmail.com>

>  drivers/iio/light/ltr390.c | 53 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
> 
> diff --git a/drivers/iio/light/ltr390.c b/drivers/iio/light/ltr390.c
> index ee59bbb8aa09..b1028d027e1b 100644
> --- a/drivers/iio/light/ltr390.c
> +++ b/drivers/iio/light/ltr390.c
> @@ -38,12 +38,21 @@
>  #define LTR390_ALS_UVS_GAIN		0x05
>  #define LTR390_PART_ID			0x06
>  #define LTR390_MAIN_STATUS		0x07
> +
>  #define LTR390_ALS_DATA			0x0D
> +#define LTR390_ALS_DATA_BYTE(n)		((LTR390_ALS_DATA) + (n))
						(LTR390_ALS_DATA + (n))

etc  There is no benefit in brackets around the bit that isn't a parameter
of the macro that I can see.


> +
>  #define LTR390_UVS_DATA			0x10
> +#define LTR390_UVS_DATA_BYTE(n)		((LTR390_UVS_DATA) + (n))
> +
>  #define LTR390_INT_CFG			0x19
>  #define LTR390_INT_PST			0x1A
> +
>  #define LTR390_THRESH_UP		0x21
> +#define LTR390_THRESH_UP_BYTE(n)	((LTR390_THRESH_UP) + (n))
> +
>  #define LTR390_THRESH_LOW		0x24
> +#define LTR390_THRESH_LOW_BYTE(n)	((LTR390_THRESH_LOW) + (n))
>  
>  #define LTR390_PART_NUMBER_ID		0xb
>  #define LTR390_ALS_UVS_GAIN_MASK	GENMASK(2, 0)
> @@ -98,11 +107,40 @@ struct ltr390_data {
>  	int int_time_us;
>  };
>  
> +static const struct regmap_range ltr390_readable_reg_ranges[] = {
> +	regmap_reg_range(LTR390_MAIN_CTRL, LTR390_MAIN_CTRL),
> +	regmap_reg_range(LTR390_ALS_UVS_MEAS_RATE, LTR390_MAIN_STATUS),
> +	regmap_reg_range(LTR390_ALS_DATA_BYTE(0), LTR390_ALS_DATA_BYTE(2)),
> +	regmap_reg_range(LTR390_UVS_DATA_BYTE(0), LTR390_UVS_DATA_BYTE(2)),

If we are doing maximum length ranges can we not combine the two above?
Looks to be 0xd,0xe,0xf,0x10,0x11 and 0x012, so a continuous range similar
to the thresholds below.

> +	regmap_reg_range(LTR390_INT_CFG, LTR390_INT_PST),
> +	regmap_reg_range(LTR390_THRESH_UP_BYTE(0), LTR390_THRESH_LOW_BYTE(2)),
> +};

Thanks,

Jonathan