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

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

Message-ID: <CAE3SzaTAAWKpwe0X6969tzfhoj4px1hEuEu-P-nK-Hh7axDHUQ@mail.gmail.com>
Date: Thu, 24 Jul 2025 20:30:20 +0530
From: Akshay Jindal <akshayaj.lkd@...il.com>
To: David Lechner <dlechner@...libre.com>
Cc: anshulusr@...il.com, jic23@...nel.org, nuno.sa@...log.com, andy@...nel.org, 
	shuah@...nel.org, linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] iio: light: ltr390: Add debugfs register access support

On Thu, Jul 24, 2025 at 1:44 AM David Lechner <dlechner@...libre.com> 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