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

[Jonathan Cameron <jic23@kernel.org>]

On Wed, 23 Jul 2025 09:13:09 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 7/23/25 6:46 AM, Akshay Jindal 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>
> > ---
> > Testing details:
> > ================  
> > -> Tested on Raspberrypi 4B. Follow for more details.  
> >   
> 
> ...
> 
> >  drivers/iio/light/ltr390.c | 99 ++++++++++++++++++++++++++++++++++----
> >  1 file changed, 89 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/iio/light/ltr390.c b/drivers/iio/light/ltr390.c
> > index ee59bbb8aa09..1f6ee0fd6d19 100644
> > --- a/drivers/iio/light/ltr390.c
> > +++ b/drivers/iio/light/ltr390.c
> > @@ -38,12 +38,20 @@
> >  #define LTR390_ALS_UVS_GAIN		0x05
> >  #define LTR390_PART_ID			0x06
> >  #define LTR390_MAIN_STATUS		0x07
> > -#define LTR390_ALS_DATA			0x0D
> > -#define LTR390_UVS_DATA			0x10
> > +#define LTR390_ALS_DATA_0		0x0D
> > +#define LTR390_ALS_DATA_1		0x0E
> > +#define LTR390_ALS_DATA_2		0x0F
> > +#define LTR390_UVS_DATA_0		0x10
> > +#define LTR390_UVS_DATA_1		0x11
> > +#define LTR390_UVS_DATA_2		0x12
> >  #define LTR390_INT_CFG			0x19
> >  #define LTR390_INT_PST			0x1A
> > -#define LTR390_THRESH_UP		0x21
> > -#define LTR390_THRESH_LOW		0x24
> > +#define LTR390_THRESH_UP_0		0x21
> > +#define LTR390_THRESH_UP_1		0x22
> > +#define LTR390_THRESH_UP_2		0x23
> > +#define LTR390_THRESH_LOW_0		0x24
> > +#define LTR390_THRESH_LOW_1		0x25
> > +#define LTR390_THRESH_LOW_2		0x26
> >  
> >  #define LTR390_PART_NUMBER_ID		0xb
> >  #define LTR390_ALS_UVS_GAIN_MASK	GENMASK(2, 0)
> > @@ -98,11 +106,62 @@ struct ltr390_data {
> >  	int int_time_us;
> >  };
> >  
> > +static bool ltr390_is_readable_reg(struct device *dev, unsigned int reg)
> > +{
> > +	switch (reg) {
> > +	case LTR390_MAIN_CTRL:
> > +	case LTR390_ALS_UVS_MEAS_RATE:
> > +	case LTR390_ALS_UVS_GAIN:
> > +	case LTR390_PART_ID:
> > +	case LTR390_MAIN_STATUS:
> > +	case LTR390_ALS_DATA_0:
> > +	case LTR390_ALS_DATA_1:
> > +	case LTR390_ALS_DATA_2:
> > +	case LTR390_UVS_DATA_0:
> > +	case LTR390_UVS_DATA_1:
> > +	case LTR390_UVS_DATA_2:
> > +	case LTR390_INT_CFG:
> > +	case LTR390_INT_PST:
> > +	case LTR390_THRESH_UP_0:
> > +	case LTR390_THRESH_UP_1:
> > +	case LTR390_THRESH_UP_2:
> > +	case LTR390_THRESH_LOW_0:
> > +	case LTR390_THRESH_LOW_1:
> > +	case LTR390_THRESH_LOW_2:
> > +		return true;
> > +	default:
> > +		return false;
> > +	}
> > +}
> > +
> > +static bool ltr390_is_writeable_reg(struct device *dev, unsigned int reg)
> > +{
> > +	switch (reg) {
> > +	case LTR390_MAIN_CTRL:
> > +	case LTR390_ALS_UVS_MEAS_RATE:
> > +	case LTR390_ALS_UVS_GAIN:
> > +	case LTR390_INT_CFG:
> > +	case LTR390_INT_PST:
> > +	case LTR390_THRESH_UP_0:
> > +	case LTR390_THRESH_UP_1:
> > +	case LTR390_THRESH_UP_2:
> > +	case LTR390_THRESH_LOW_0:
> > +	case LTR390_THRESH_LOW_1:
> > +	case LTR390_THRESH_LOW_2:
> > +		return true;
> > +	default:
> > +		return false;
> > +	}
> > +}  
> 
> Using rd_table and wr_table is a bit more compact than readable_reg
> writeable_reg.

Or given a lot of these are ranges of related registers you could use
ranges
	switch (reg) {
	case LTR390_MAIN_CTRL:
	case LTR390_ALS_UVS_MEAS_RATE:
	case LTR390_ALS_UVS_GAIN:
	case LTR390_INT_CFG:
	case LTR390_INT_PST:
	case LTR390_THRESH_UP_0 ... LTR390_THRESH_UP_2:
	case LTR390_THRESH_LOW_0 ... LTR390_THRESH_LOW_2:
		return true;

(with macros as Andy suggested)
	}