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

[Akshay Jindal]

On Thu, Jul 24, 2025 at 1:44 AM David Lechner <dlechner@xxxxxxxxxxxx> wrote:
> We don't need separate lines if there is no gap. I didn't check all of the lines
> but at least these last two can be combined since LTR390_THRESH_UP_BYTE(2) is
> 0x23 and LTR390_THRESH_LOW_BYTE(0) is 0x24.
Done. In other lines there is no gap.
For your convenience, I am writing the actual value of these macros as comments.
static const struct regmap_range ltr390_readable_reg_ranges[] = {
       regmap_reg_range(LTR390_MAIN_CTRL, LTR390_MAIN_CTRL), // (0x0-->0x0)
       regmap_reg_range(LTR390_ALS_UVS_MEAS_RATE,
LTR390_MAIN_STATUS),//(0x4-->0x7)
       regmap_reg_range(LTR390_ALS_DATA_BYTE(0),
LTR390_ALS_DATA_BYTE(2)),//(0xd-->0xf)
       regmap_reg_range(LTR390_UVS_DATA_BYTE(0),
LTR390_UVS_DATA_BYTE(2)),//(0x10-->0x12)
       regmap_reg_range(LTR390_INT_CFG, LTR390_INT_PST),//(0x19-->0x1A)
       regmap_reg_range(LTR390_THRESH_UP_BYTE(0),
LTR390_THRESH_LOW_BYTE(2)),//(0x21-->0x26)
};

>
> And we could avoid adding the extra macros just for this since they
> aren't used anywhere else.
>
I only added the following macros:
+#define LTR390_ALS_DATA_BYTE(n)                ((LTR390_ALS_DATA) + (n))
+#define LTR390_UVS_DATA_BYTE(n)               ((LTR390_UVS_DATA) + (n))
+#define LTR390_THRESH_UP_BYTE(n)            ((LTR390_THRESH_UP) + (n))
+#define LTR390_THRESH_LOW_BYTE(n)         ((LTR390_THRESH_LOW) + (n))

I can remove them too, but then in regmap_range, I will have to
statically write like you mentioned.
regmap_reg_range(LTR390_THRESH_UP, LTR390_THRESH_LOW + 2).
That is also fine, but I feel that using parameterized macros is a
much cleaner way, as also mentioned
in v1 review comments by other maintainers. Let me know your thoughts.
I am happy to let them go too.

Thanks,
Akshay