RE: [PATCH v11 2/2] leds: ltc3208: Add driver for LTC3208 Multidisplay LED Driver

-----Original Message-----
From: sashiko-bot@kernel.org <sashiko-bot@kernel.org>
Sent: Thursday, September 3, 2026 7:56 AM
To: Roleda, Jan carlo <redacted>
Cc: conor+dt@kernel.org; linux-leds@vger.kernel.org; lee@kernel.org;
robh@kernel.org; devicetree@vger.kernel.org
Subject: Re: [PATCH v11 2/2] leds: ltc3208: Add driver for LTC3208
Multidisplay LED Driver

[External]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s)
to consider:
- [Low] Incorrect assignment of num_reg_defaults using a register address
macro instead of ARRAY_SIZE.
--

commit 13fa42bec99e4f9ce4669e4a6a7c6f12807ee3a4
Author: Jan Carlo Roleda [off-list ref]

leds: ltc3208: Add driver for LTC3208 Multidisplay LED Driver

This commit introduces a new kernel driver for the Analog Devices LTC3208
Multi-display LED driver. It configures the device via I2C and supports up to 7
distinct LED channels with configurable current levels.

quoted

diff --git a/drivers/leds/leds-ltc3208.c b/drivers/leds/leds-ltc3208.c
new file mode 100644
--- /dev/null
+++ b/drivers/leds/leds-ltc3208.c

[ ... ]

quoted

+static const struct regmap_config ltc3208_regmap_cfg = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = LTC3208_REG_G_OPT,
+	.cache_type = REGCACHE_FLAT_S,
+	.reg_defaults = ltc3208_reg_defaults,
+	.num_reg_defaults = LTC3208_REG_G_OPT,

[Severity: Low]
Does this code intentionally use a register address macro for the array length
instead of ARRAY_SIZE(ltc3208_reg_defaults)?

Because LTC3208_REG_G_OPT happens to evaluate to 7, it coincidentally
matches the exact number of elements in the ltc3208_reg_defaults array.
However, using a register address macro to specify an array length creates a
brittle semantic dependency. If elements are removed from the array, or if a
register with a different address is added in the future, this will result in an
incorrect size being passed to the regmap core.

This is indeed the way it was used in the regmap_cfg, as the end of the register list is LTC3208_REG_G_OPT (0x07) which matches	 the total register count.

I will add a new macro to define the number of registers available on the device to avoid this dependency.

--
Sashiko AI review ·
https://urldefense.com/v3/__https://sashiko.dev/*/patchset/20260903-
upstream-ltc3208-v11-0-
28f5f0fd364e@analog.com?part=2__;Iw!!A3Ni8CS0y2Y!6Lifhm0OhxOiYf_dk
0-zdrnQPIu9WNUGQ9BvU4yDJkBaHUC1GXb4YfHkHwz_CxZOV-
PgcyINnDP8wbCPQWF7I3vGcg$

Regards,
Carlo