[PATCH 00/10] leds: ktd202x: Add Etek ET6326, hardware breathing and fixes

[PATCH 00/10] leds: ktd202x: Add Etek ET6326, hardware breathing and fixes

[Joseph Telaak via B4 Relay]

The Etek ET6326 drives the status LED of the GL.iNet GL-SFT1200 (Opal)
router. It is register compatible with the Kinetic KTD2026, except for
the flash period: Reg1 = n is (n + 1) * 128 ms on the ET6326 and
256 ms + n * 128 ms on the KTD202x. Patches 5 and 6 add it with the
KTD2026 as fallback, so older kernels drive it with slightly off blinks.

Patches 1 to 4 fix bugs that have been there since the driver was
added, and come first so they can be applied on their own:
- a blink shorter than 256 ms became a 16 s blink on 32 bit
- with fewer LEDs in the DT than channels, as on this board, the driver
  read past the end of its LED array
- a blink with a very short on time was programmed as never on
- delays from the timer trigger that add up to 2^32 (or 2^64) divided
  by zero, and long blinks overflowed the on time on 32 bit

Patch 7 lets the pattern trigger's hw_pattern use the chip's rise and
fall ramps, so a breathing LED needs no I2C traffic. Patch 8 honors
led-max-microamp on single color LEDs: the GL-SFT1200 drives three LEDs
in parallel on each channel at 1.375 mA, and the driver's full
brightness is 24 mA. Patch 9 fails the probe when nothing answers at the
address. Patch 10 stops a second blinking LED from silently retiming the
first: it shares the flash timer only with the same timing, and
otherwise blinks in software. The second PWM timer is no help, since in
the timer slot the driver uses it repeats the first.

ET6326 datasheet:
https://www.etekmicro.com/wp-content/uploads/datasheets/ET6326_datasheet.pdf

Tested on a GL-SFT1200 (SF19A2890, MIPS32) with OpenWrt and Linux
6.18.55, with the two LEDs of the board (blue and white, three LEDs in
parallel on each channel, led-max-microamp = <1375>):
- brightness 1 to 11 (max_brightness 11 from led-max-microamp), and
  waking the chip from sleep
- the timer trigger from 1/1 ms to 500/500 ms; a camera measured a
  0.916 s period for 896 ms, the same 2.2 % slow oscillator as with the
  stock firmware, so the ET6326 period encoding is the one used here
- breathing through hw_pattern, written right after selecting the
  pattern trigger
- invalid patterns and finite repeats rejected
- a second LED blinking at another rate falls back to software and
  leaves the first one alone
- 240 writes to one LED while the other stayed lit, module unload and
  reload, and a reboot while blinking
The bug fixes were also checked with a simulated chip under ASan and
UBSan, and the series builds with W=1 on x86_64, i386, MIPS and arm64.

---
Joseph Telaak (10):
      leds: rgb: ktd202x: Fix the flash period of short blinks on 32 bit
      leds: rgb: ktd202x: Fix an out-of-bounds read with unused channels
      leds: rgb: ktd202x: Never program an on time of zero
      leds: rgb: ktd202x: Fix a division by zero with long blinks
      dt-bindings: leds: kinetic,ktd202x: Add Etek ET6326
      leds: rgb: ktd202x: Add Etek ET6326 support
      leds: rgb: ktd202x: Add hardware pattern support
      leds: rgb: ktd202x: Limit the LED current to led-max-microamp
      leds: rgb: ktd202x: Fail the probe when the chip does not answer
      leds: rgb: ktd202x: Don't let a blinking LED retime another one

 .../devicetree/bindings/leds/kinetic,ktd202x.yaml  |  15 +-
 Documentation/leds/index.rst                       |   1 +
 Documentation/leds/leds-ktd202x.rst                |  48 +++
 drivers/leds/rgb/Kconfig                           |   3 +-
 drivers/leds/rgb/leds-ktd202x.c                    | 423 +++++++++++++++++----
 5 files changed, 419 insertions(+), 71 deletions(-)
---
base-commit: 59cf91cf87abb67a49b1bb7b9d3b06486abde8cf
change-id: 20261101-ktd202x-et6326-73ed3046c534

Best regards,
--  
Joseph Telaak <joseph@telaak.dev>

[PATCH 00/10] leds: ktd202x: Add Etek ET6326, hardware breathing and fixes

[Joseph Telaak]

The Etek ET6326 drives the status LED of the GL.iNet GL-SFT1200 (Opal)
router. It is register compatible with the Kinetic KTD2026, except for
the flash period: Reg1 = n is (n + 1) * 128 ms on the ET6326 and
256 ms + n * 128 ms on the KTD202x. Patches 5 and 6 add it with the
KTD2026 as fallback, so older kernels drive it with slightly off blinks.

Patches 1 to 4 fix bugs that have been there since the driver was
added, and come first so they can be applied on their own:
- a blink shorter than 256 ms became a 16 s blink on 32 bit
- with fewer LEDs in the DT than channels, as on this board, the driver
  read past the end of its LED array
- a blink with a very short on time was programmed as never on
- delays from the timer trigger that add up to 2^32 (or 2^64) divided
  by zero, and long blinks overflowed the on time on 32 bit

Patch 7 lets the pattern trigger's hw_pattern use the chip's rise and
fall ramps, so a breathing LED needs no I2C traffic. Patch 8 honors
led-max-microamp on single color LEDs: the GL-SFT1200 drives three LEDs
in parallel on each channel at 1.375 mA, and the driver's full
brightness is 24 mA. Patch 9 fails the probe when nothing answers at the
address. Patch 10 stops a second blinking LED from silently retiming the
first: it shares the flash timer only with the same timing, and
otherwise blinks in software. The second PWM timer is no help, since in
the timer slot the driver uses it repeats the first.

ET6326 datasheet:
https://www.etekmicro.com/wp-content/uploads/datasheets/ET6326_datasheet.pdf

Tested on a GL-SFT1200 (SF19A2890, MIPS32) with OpenWrt and Linux
6.18.55, with the two LEDs of the board (blue and white, three LEDs in
parallel on each channel, led-max-microamp = <1375>):
- brightness 1 to 11 (max_brightness 11 from led-max-microamp), and
  waking the chip from sleep
- the timer trigger from 1/1 ms to 500/500 ms; a camera measured a
  0.916 s period for 896 ms, the same 2.2 % slow oscillator as with the
  stock firmware, so the ET6326 period encoding is the one used here
- breathing through hw_pattern, written right after selecting the
  pattern trigger
- invalid patterns and finite repeats rejected
- a second LED blinking at another rate falls back to software and
  leaves the first one alone
- 240 writes to one LED while the other stayed lit, module unload and
  reload, and a reboot while blinking
The bug fixes were also checked with a simulated chip under ASan and
UBSan, and the series builds with W=1 on x86_64, i386, MIPS and arm64.

---
Joseph Telaak (10):
      leds: rgb: ktd202x: Fix the flash period of short blinks on 32 bit
      leds: rgb: ktd202x: Fix an out-of-bounds read with unused channels
      leds: rgb: ktd202x: Never program an on time of zero
      leds: rgb: ktd202x: Fix a division by zero with long blinks
      dt-bindings: leds: kinetic,ktd202x: Add Etek ET6326
      leds: rgb: ktd202x: Add Etek ET6326 support
      leds: rgb: ktd202x: Add hardware pattern support
      leds: rgb: ktd202x: Limit the LED current to led-max-microamp
      leds: rgb: ktd202x: Fail the probe when the chip does not answer
      leds: rgb: ktd202x: Don't let a blinking LED retime another one

 .../devicetree/bindings/leds/kinetic,ktd202x.yaml  |  15 +-
 Documentation/leds/index.rst                       |   1 +
 Documentation/leds/leds-ktd202x.rst                |  48 +++
 drivers/leds/rgb/Kconfig                           |   3 +-
 drivers/leds/rgb/leds-ktd202x.c                    | 423 +++++++++++++++++----
 5 files changed, 419 insertions(+), 71 deletions(-)
---
base-commit: 59cf91cf87abb67a49b1bb7b9d3b06486abde8cf
change-id: 20261101-ktd202x-et6326-73ed3046c534

Best regards,
--  
Joseph Telaak <joseph@telaak.dev>

[PATCH 01/10] leds: rgb: ktd202x: Fix the flash period of short blinks on 32 bit

[Joseph Telaak via B4 Relay]

From: Joseph Telaak <joseph@telaak.dev>

When delay_on + delay_off is below 256 ms, the unsigned subtraction of
KTD202X_FLASH_PERIOD_MIN_MS wraps. On 64 bit the result truncates to -1
or -2 and gets clamped to the shortest period, but on 32 bit it
truncates to 33554431 or 33554430 and gets clamped to the longest one,
so asking for a fast blink gives a blink every 16 seconds.

Compare before subtracting.

Fixes: 0ebdb7210943 ("leds: Add ktd202x driver")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Joseph Telaak <joseph@telaak.dev>
---
 drivers/leds/rgb/leds-ktd202x.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/leds/rgb/leds-ktd202x.c b/drivers/leds/rgb/leds-ktd202x.c
index 143020945..539cf425c 100644
--- a/drivers/leds/rgb/leds-ktd202x.c
+++ b/drivers/leds/rgb/leds-ktd202x.c
@@ -296,9 +296,13 @@ static int ktd202x_blink_set(struct ktd202x_led *led, unsigned long *delay_on,
 	/* Convert into values the HW will understand. */
 
 	/* Integer representation of time of flash period */
-	num_steps = (*delay_on + *delay_off - KTD202X_FLASH_PERIOD_MIN_MS) /
-		    KTD202X_FLASH_PERIOD_STEP_MS;
-	num_steps = clamp(num_steps, 0, KTD202X_FLASH_PERIOD_MAX_STEPS);
+	delay_total_ms = *delay_on + *delay_off;
+	if (delay_total_ms < KTD202X_FLASH_PERIOD_MIN_MS)
+		num_steps = 0;
+	else
+		num_steps = min_t(unsigned long, KTD202X_FLASH_PERIOD_MAX_STEPS,
+				  (delay_total_ms - KTD202X_FLASH_PERIOD_MIN_MS) /
+				  KTD202X_FLASH_PERIOD_STEP_MS);
 
 	/* Integer representation of percentage of LED ON time */
 	on = (*delay_on * KTD202X_FLASH_ON_MAX) / (*delay_on + *delay_off);

-- 
Git-155)

[PATCH 01/10] leds: rgb: ktd202x: Fix the flash period of short blinks on 32 bit

[Joseph Telaak]

When delay_on + delay_off is below 256 ms, the unsigned subtraction of
KTD202X_FLASH_PERIOD_MIN_MS wraps. On 64 bit the result truncates to -1
or -2 and gets clamped to the shortest period, but on 32 bit it
truncates to 33554431 or 33554430 and gets clamped to the longest one,
so asking for a fast blink gives a blink every 16 seconds.

Compare before subtracting.

Fixes: 0ebdb7210943 ("leds: Add ktd202x driver")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Joseph Telaak <joseph@telaak.dev>
---
 drivers/leds/rgb/leds-ktd202x.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/leds/rgb/leds-ktd202x.c b/drivers/leds/rgb/leds-ktd202x.c
index 143020945..539cf425c 100644
--- a/drivers/leds/rgb/leds-ktd202x.c
+++ b/drivers/leds/rgb/leds-ktd202x.c
@@ -296,9 +296,13 @@ static int ktd202x_blink_set(struct ktd202x_led *led, unsigned long *delay_on,
 	/* Convert into values the HW will understand. */
 
 	/* Integer representation of time of flash period */
-	num_steps = (*delay_on + *delay_off - KTD202X_FLASH_PERIOD_MIN_MS) /
-		    KTD202X_FLASH_PERIOD_STEP_MS;
-	num_steps = clamp(num_steps, 0, KTD202X_FLASH_PERIOD_MAX_STEPS);
+	delay_total_ms = *delay_on + *delay_off;
+	if (delay_total_ms < KTD202X_FLASH_PERIOD_MIN_MS)
+		num_steps = 0;
+	else
+		num_steps = min_t(unsigned long, KTD202X_FLASH_PERIOD_MAX_STEPS,
+				  (delay_total_ms - KTD202X_FLASH_PERIOD_MIN_MS) /
+				  KTD202X_FLASH_PERIOD_STEP_MS);
 
 	/* Integer representation of percentage of LED ON time */
 	on = (*delay_on * KTD202X_FLASH_ON_MAX) / (*delay_on + *delay_off);

-- 
Git-155)

Re: [PATCH 01/10] leds: rgb: ktd202x: Fix the flash period of short blinks on 32 bit

[sashiko-bot]

> From: Joseph Telaak <joseph@telaak.dev>
> 
> When delay_on + delay_off is below 256 ms, the unsigned subtraction of
> KTD202X_FLASH_PERIOD_MIN_MS wraps. On 64 bit the result truncates to -1
> or -2 and gets clamped to the shortest period, but on 32 bit it
> truncates to 33554431 or 33554430 and gets clamped to the longest one,
> so asking for a fast blink gives a blink every 16 seconds.
> 
> Compare before subtracting.
> 
> Fixes: 0ebdb7210943 ("leds: Add ktd202x driver")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Joseph Telaak <joseph@telaak.dev>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261101-ktd202x-et6326-v1-0-eb51eddad08c@telaak.dev?part=1

[PATCH 02/10] leds: rgb: ktd202x: Fix an out-of-bounds read with unused channels

[Joseph Telaak via B4 Relay]

From: Joseph Telaak <joseph@telaak.dev>

The LED array is allocated for the number of child nodes, but
ktd202x_chip_in_use() walks it up to num_leds, the number of channels of
the chip. With fewer LEDs in the device tree than channels, for example
two LEDs on a KTD2026, it reads past the end of the allocation and the
__counted_by() annotation hides that from the bounds checkers.

Allocate one entry per channel.

Fixes: 0ebdb7210943 ("leds: Add ktd202x driver")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Joseph Telaak <joseph@telaak.dev>
---
 drivers/leds/rgb/leds-ktd202x.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/leds/rgb/leds-ktd202x.c b/drivers/leds/rgb/leds-ktd202x.c
index 539cf425c..482af28e8 100644
--- a/drivers/leds/rgb/leds-ktd202x.c
+++ b/drivers/leds/rgb/leds-ktd202x.c
@@ -537,6 +537,7 @@ static int ktd202x_probe(struct i2c_client *client)
 {
 	struct device *dev = &client->dev;
 	struct ktd202x *chip;
+	unsigned long num_leds;
 	int count;
 	int ret;
 
@@ -544,10 +545,13 @@ static int ktd202x_probe(struct i2c_client *client)
 	if (!count || count > KTD202X_MAX_LEDS)
 		return dev_err_probe(dev, -EINVAL, "Incorrect number of leds (%d)", count);
 
-	chip = devm_kzalloc(dev, struct_size(chip, leds, count), GFP_KERNEL);
+	/* Loops over the LEDs go up to the number of channels, not of child nodes */
+	num_leds = (unsigned long)i2c_get_match_data(client);
+	chip = devm_kzalloc(dev, struct_size(chip, leds, num_leds), GFP_KERNEL);
 	if (!chip)
 		return -ENOMEM;
 
+	chip->num_leds = num_leds;
 	chip->dev = dev;
 	i2c_set_clientdata(client, chip);
 
@@ -562,8 +566,6 @@ static int ktd202x_probe(struct i2c_client *client)
 	if (ret)
 		return ret;
 
-	chip->num_leds = (unsigned long)i2c_get_match_data(client);
-
 	chip->regulators[0].supply = "vin";
 	chip->regulators[1].supply = "vio";
 	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(chip->regulators), chip->regulators);

-- 
Git-155)

[PATCH 02/10] leds: rgb: ktd202x: Fix an out-of-bounds read with unused channels

[Joseph Telaak]

The LED array is allocated for the number of child nodes, but
ktd202x_chip_in_use() walks it up to num_leds, the number of channels of
the chip. With fewer LEDs in the device tree than channels, for example
two LEDs on a KTD2026, it reads past the end of the allocation and the
__counted_by() annotation hides that from the bounds checkers.

Allocate one entry per channel.

Fixes: 0ebdb7210943 ("leds: Add ktd202x driver")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Joseph Telaak <joseph@telaak.dev>
---
 drivers/leds/rgb/leds-ktd202x.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/leds/rgb/leds-ktd202x.c b/drivers/leds/rgb/leds-ktd202x.c
index 539cf425c..482af28e8 100644
--- a/drivers/leds/rgb/leds-ktd202x.c
+++ b/drivers/leds/rgb/leds-ktd202x.c
@@ -537,6 +537,7 @@ static int ktd202x_probe(struct i2c_client *client)
 {
 	struct device *dev = &client->dev;
 	struct ktd202x *chip;
+	unsigned long num_leds;
 	int count;
 	int ret;
 
@@ -544,10 +545,13 @@ static int ktd202x_probe(struct i2c_client *client)
 	if (!count || count > KTD202X_MAX_LEDS)
 		return dev_err_probe(dev, -EINVAL, "Incorrect number of leds (%d)", count);
 
-	chip = devm_kzalloc(dev, struct_size(chip, leds, count), GFP_KERNEL);
+	/* Loops over the LEDs go up to the number of channels, not of child nodes */
+	num_leds = (unsigned long)i2c_get_match_data(client);
+	chip = devm_kzalloc(dev, struct_size(chip, leds, num_leds), GFP_KERNEL);
 	if (!chip)
 		return -ENOMEM;
 
+	chip->num_leds = num_leds;
 	chip->dev = dev;
 	i2c_set_clientdata(client, chip);
 
@@ -562,8 +566,6 @@ static int ktd202x_probe(struct i2c_client *client)
 	if (ret)
 		return ret;
 
-	chip->num_leds = (unsigned long)i2c_get_match_data(client);
-
 	chip->regulators[0].supply = "vin";
 	chip->regulators[1].supply = "vio";
 	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(chip->regulators), chip->regulators);

-- 
Git-155)

Re: [PATCH 02/10] leds: rgb: ktd202x: Fix an out-of-bounds read with unused channels

[sashiko-bot]

> From: Joseph Telaak <joseph@telaak.dev>
> 
> The LED array is allocated for the number of child nodes, but
> ktd202x_chip_in_use() walks it up to num_leds, the number of channels of
> the chip. With fewer LEDs in the device tree than channels, for example
> two LEDs on a KTD2026, it reads past the end of the allocation and the
> __counted_by() annotation hides that from the bounds checkers.
> 
> Allocate one entry per channel.
> 
> Fixes: 0ebdb7210943 ("leds: Add ktd202x driver")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Joseph Telaak <joseph@telaak.dev>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261101-ktd202x-et6326-v1-0-eb51eddad08c@telaak.dev?part=2

[PATCH 03/10] leds: rgb: ktd202x: Never program an on time of zero

[Joseph Telaak via B4 Relay]

From: Joseph Telaak <joseph@telaak.dev>

blink_set() turns delay_on into a fraction of the period in 1/256 steps.
When delay_on is less than 1/256 of the period, for example 1 ms on and
1000 ms off, that fraction is 0: the LED stays dark, while the timer
trigger reports that it is blinking with delay_on = 0.

Use the shortest on time the hardware has, 1/256 of the period, and
report it back.

Fixes: 0ebdb7210943 ("leds: Add ktd202x driver")
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Joseph Telaak <joseph@telaak.dev>
---
 drivers/leds/rgb/leds-ktd202x.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/leds/rgb/leds-ktd202x.c b/drivers/leds/rgb/leds-ktd202x.c
index 482af28e8..8c890ea5a 100644
--- a/drivers/leds/rgb/leds-ktd202x.c
+++ b/drivers/leds/rgb/leds-ktd202x.c
@@ -304,8 +304,12 @@ static int ktd202x_blink_set(struct ktd202x_led *led, unsigned long *delay_on,
 				  (delay_total_ms - KTD202X_FLASH_PERIOD_MIN_MS) /
 				  KTD202X_FLASH_PERIOD_STEP_MS);
 
-	/* Integer representation of percentage of LED ON time */
+	/*
+	 * Integer representation of percentage of LED ON time. A value of 0
+	 * keeps the LED dark, so use the shortest on time instead.
+	 */
 	on = (*delay_on * KTD202X_FLASH_ON_MAX) / (*delay_on + *delay_off);
+	on = max(on, 1);
 
 	/* Actually used delay_{on,off} values */
 	delay_total_ms = num_steps * KTD202X_FLASH_PERIOD_STEP_MS + KTD202X_FLASH_PERIOD_MIN_MS;

-- 
Git-155)

[PATCH 03/10] leds: rgb: ktd202x: Never program an on time of zero

[Joseph Telaak]

blink_set() turns delay_on into a fraction of the period in 1/256 steps.
When delay_on is less than 1/256 of the period, for example 1 ms on and
1000 ms off, that fraction is 0: the LED stays dark, while the timer
trigger reports that it is blinking with delay_on = 0.

Use the shortest on time the hardware has, 1/256 of the period, and
report it back.

Fixes: 0ebdb7210943 ("leds: Add ktd202x driver")
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Joseph Telaak <joseph@telaak.dev>
---
 drivers/leds/rgb/leds-ktd202x.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/leds/rgb/leds-ktd202x.c b/drivers/leds/rgb/leds-ktd202x.c
index 482af28e8..8c890ea5a 100644
--- a/drivers/leds/rgb/leds-ktd202x.c
+++ b/drivers/leds/rgb/leds-ktd202x.c
@@ -304,8 +304,12 @@ static int ktd202x_blink_set(struct ktd202x_led *led, unsigned long *delay_on,
 				  (delay_total_ms - KTD202X_FLASH_PERIOD_MIN_MS) /
 				  KTD202X_FLASH_PERIOD_STEP_MS);
 
-	/* Integer representation of percentage of LED ON time */
+	/*
+	 * Integer representation of percentage of LED ON time. A value of 0
+	 * keeps the LED dark, so use the shortest on time instead.
+	 */
 	on = (*delay_on * KTD202X_FLASH_ON_MAX) / (*delay_on + *delay_off);
+	on = max(on, 1);
 
 	/* Actually used delay_{on,off} values */
 	delay_total_ms = num_steps * KTD202X_FLASH_PERIOD_STEP_MS + KTD202X_FLASH_PERIOD_MIN_MS;

-- 
Git-155)

Re: [PATCH 03/10] leds: rgb: ktd202x: Never program an on time of zero

[sashiko-bot]

> From: Joseph Telaak <joseph@telaak.dev>
> 
> blink_set() turns delay_on into a fraction of the period in 1/256 steps.
> When delay_on is less than 1/256 of the period, for example 1 ms on and
> 1000 ms off, that fraction is 0: the LED stays dark, while the timer
> trigger reports that it is blinking with delay_on = 0.
> 
> Use the shortest on time the hardware has, 1/256 of the period, and
> report it back.
> 
> Fixes: 0ebdb7210943 ("leds: Add ktd202x driver")
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Joseph Telaak <joseph@telaak.dev>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261101-ktd202x-et6326-v1-0-eb51eddad08c@telaak.dev?part=3

[PATCH 04/10] leds: rgb: ktd202x: Fix a division by zero with long blinks

[Joseph Telaak via B4 Relay]

From: Joseph Telaak <joseph@telaak.dev>

blink_set() works out the on time as delay_on * 256 / (delay_on +
delay_off) in unsigned long, with delays straight from the timer
trigger's sysfs files:

- when the two delays add up to exactly 2^32 on 32 bit, or 2^64 on
  64 bit, the sum wraps to 0 and the kernel divides by zero, for
  example with "echo 2147483648" into both delay_on and delay_off.
  That oopses on x86 and gives an arbitrary result on MIPS and arm64.
- on 32 bit the product already overflows once delay_on is above about
  4.6 hours, and a sum that wraps sets the wrong period, so long blinks
  get an arbitrary duty cycle

Saturate the sum, and compute the on time with mul_u64_u64_div_u64().

Fixes: 0ebdb7210943 ("leds: Add ktd202x driver")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Joseph Telaak <joseph@telaak.dev>
---
 drivers/leds/rgb/leds-ktd202x.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/leds/rgb/leds-ktd202x.c b/drivers/leds/rgb/leds-ktd202x.c
index 8c890ea5a..feea33b7e 100644
--- a/drivers/leds/rgb/leds-ktd202x.c
+++ b/drivers/leds/rgb/leds-ktd202x.c
@@ -9,10 +9,12 @@
 
 #include <linux/i2c.h>
 #include <linux/led-class-multicolor.h>
+#include <linux/math64.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/overflow.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 
@@ -296,7 +298,8 @@ static int ktd202x_blink_set(struct ktd202x_led *led, unsigned long *delay_on,
 	/* Convert into values the HW will understand. */
 
 	/* Integer representation of time of flash period */
-	delay_total_ms = *delay_on + *delay_off;
+	if (check_add_overflow(*delay_on, *delay_off, &delay_total_ms))
+		delay_total_ms = ULONG_MAX;
 	if (delay_total_ms < KTD202X_FLASH_PERIOD_MIN_MS)
 		num_steps = 0;
 	else
@@ -308,8 +311,8 @@ static int ktd202x_blink_set(struct ktd202x_led *led, unsigned long *delay_on,
 	 * Integer representation of percentage of LED ON time. A value of 0
 	 * keeps the LED dark, so use the shortest on time instead.
 	 */
-	on = (*delay_on * KTD202X_FLASH_ON_MAX) / (*delay_on + *delay_off);
-	on = max(on, 1);
+	on = mul_u64_u64_div_u64(*delay_on, KTD202X_FLASH_ON_MAX, delay_total_ms);
+	on = clamp(on, 1, KTD202X_FLASH_ON_MAX - 1);
 
 	/* Actually used delay_{on,off} values */
 	delay_total_ms = num_steps * KTD202X_FLASH_PERIOD_STEP_MS + KTD202X_FLASH_PERIOD_MIN_MS;

-- 
Git-155)

[PATCH 04/10] leds: rgb: ktd202x: Fix a division by zero with long blinks

[Joseph Telaak]

blink_set() works out the on time as delay_on * 256 / (delay_on +
delay_off) in unsigned long, with delays straight from the timer
trigger's sysfs files:

- when the two delays add up to exactly 2^32 on 32 bit, or 2^64 on
  64 bit, the sum wraps to 0 and the kernel divides by zero, for
  example with "echo 2147483648" into both delay_on and delay_off.
  That oopses on x86 and gives an arbitrary result on MIPS and arm64.
- on 32 bit the product already overflows once delay_on is above about
  4.6 hours, and a sum that wraps sets the wrong period, so long blinks
  get an arbitrary duty cycle

Saturate the sum, and compute the on time with mul_u64_u64_div_u64().

Fixes: 0ebdb7210943 ("leds: Add ktd202x driver")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Joseph Telaak <joseph@telaak.dev>
---
 drivers/leds/rgb/leds-ktd202x.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/leds/rgb/leds-ktd202x.c b/drivers/leds/rgb/leds-ktd202x.c
index 8c890ea5a..feea33b7e 100644
--- a/drivers/leds/rgb/leds-ktd202x.c
+++ b/drivers/leds/rgb/leds-ktd202x.c
@@ -9,10 +9,12 @@
 
 #include <linux/i2c.h>
 #include <linux/led-class-multicolor.h>
+#include <linux/math64.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/overflow.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 
@@ -296,7 +298,8 @@ static int ktd202x_blink_set(struct ktd202x_led *led, unsigned long *delay_on,
 	/* Convert into values the HW will understand. */
 
 	/* Integer representation of time of flash period */
-	delay_total_ms = *delay_on + *delay_off;
+	if (check_add_overflow(*delay_on, *delay_off, &delay_total_ms))
+		delay_total_ms = ULONG_MAX;
 	if (delay_total_ms < KTD202X_FLASH_PERIOD_MIN_MS)
 		num_steps = 0;
 	else
@@ -308,8 +311,8 @@ static int ktd202x_blink_set(struct ktd202x_led *led, unsigned long *delay_on,
 	 * Integer representation of percentage of LED ON time. A value of 0
 	 * keeps the LED dark, so use the shortest on time instead.
 	 */
-	on = (*delay_on * KTD202X_FLASH_ON_MAX) / (*delay_on + *delay_off);
-	on = max(on, 1);
+	on = mul_u64_u64_div_u64(*delay_on, KTD202X_FLASH_ON_MAX, delay_total_ms);
+	on = clamp(on, 1, KTD202X_FLASH_ON_MAX - 1);
 
 	/* Actually used delay_{on,off} values */
 	delay_total_ms = num_steps * KTD202X_FLASH_PERIOD_STEP_MS + KTD202X_FLASH_PERIOD_MIN_MS;

-- 
Git-155)

Re: [PATCH 04/10] leds: rgb: ktd202x: Fix a division by zero with long blinks

[sashiko-bot]

> From: Joseph Telaak <joseph@telaak.dev>
> 
> blink_set() works out the on time as delay_on * 256 / (delay_on +
> delay_off) in unsigned long, with delays straight from the timer
> trigger's sysfs files:
> 
> - when the two delays add up to exactly 2^32 on 32 bit, or 2^64 on
>   64 bit, the sum wraps to 0 and the kernel divides by zero, for
>   example with "echo 2147483648" into both delay_on and delay_off.
>   That oopses on x86 and gives an arbitrary result on MIPS and arm64.
> - on 32 bit the product already overflows once delay_on is above about
>   4.6 hours, and a sum that wraps sets the wrong period, so long blinks
>   get an arbitrary duty cycle
> 
> Saturate the sum, and compute the on time with mul_u64_u64_div_u64().
> 
> Fixes: 0ebdb7210943 ("leds: Add ktd202x driver")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Joseph Telaak <joseph@telaak.dev>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261101-ktd202x-et6326-v1-0-eb51eddad08c@telaak.dev?part=4

[PATCH 05/10] dt-bindings: leds: kinetic,ktd202x: Add Etek ET6326

[Joseph Telaak via B4 Relay]

From: Joseph Telaak <joseph@telaak.dev>

The Etek ET6326 is a 3 channel LED driver with the register map, the I2C
address and the current steps of the KTD2026. It only differs in the
flash period, which is (Reg1 + 1) * 128 ms instead of 256 ms + Reg1 *
128 ms, so a KTD2026 driver runs it with blink periods that are 128 ms
off. Document it with the KTD2026 as fallback.

The ET6326 drives the status LED of the GL.iNet GL-SFT1200 (Opal).

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Joseph Telaak <joseph@telaak.dev>
---
 .../devicetree/bindings/leds/kinetic,ktd202x.yaml         | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/kinetic,ktd202x.yaml b/Documentation/devicetree/bindings/leds/kinetic,ktd202x.yaml
index 832c030a5..b13c71721 100644
--- a/Documentation/devicetree/bindings/leds/kinetic,ktd202x.yaml
+++ b/Documentation/devicetree/bindings/leds/kinetic,ktd202x.yaml
@@ -15,11 +15,20 @@ description: |
   The data sheet can be found at:
     https://www.kinet-ic.com/uploads/KTD2026-7-04h.pdf
 
+  The Etek ET6326 is a 3 channel variant that is register compatible with
+  the KTD2026, but encodes the flash period differently. Its data sheet can
+  be found at:
+    https://www.etekmicro.com/wp-content/uploads/datasheets/ET6326_datasheet.pdf
+
 properties:
   compatible:
-    enum:
-      - kinetic,ktd2026
-      - kinetic,ktd2027
+    oneOf:
+      - enum:
+          - kinetic,ktd2026
+          - kinetic,ktd2027
+      - items:
+          - const: etekmicro,et6326
+          - const: kinetic,ktd2026
 
   reg:
     maxItems: 1

-- 
Git-155)

[PATCH 05/10] dt-bindings: leds: kinetic,ktd202x: Add Etek ET6326

[Joseph Telaak]

The Etek ET6326 is a 3 channel LED driver with the register map, the I2C
address and the current steps of the KTD2026. It only differs in the
flash period, which is (Reg1 + 1) * 128 ms instead of 256 ms + Reg1 *
128 ms, so a KTD2026 driver runs it with blink periods that are 128 ms
off. Document it with the KTD2026 as fallback.

The ET6326 drives the status LED of the GL.iNet GL-SFT1200 (Opal).

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Joseph Telaak <joseph@telaak.dev>
---
 .../devicetree/bindings/leds/kinetic,ktd202x.yaml         | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/kinetic,ktd202x.yaml b/Documentation/devicetree/bindings/leds/kinetic,ktd202x.yaml
index 832c030a5..b13c71721 100644
--- a/Documentation/devicetree/bindings/leds/kinetic,ktd202x.yaml
+++ b/Documentation/devicetree/bindings/leds/kinetic,ktd202x.yaml
@@ -15,11 +15,20 @@ description: |
   The data sheet can be found at:
     https://www.kinet-ic.com/uploads/KTD2026-7-04h.pdf
 
+  The Etek ET6326 is a 3 channel variant that is register compatible with
+  the KTD2026, but encodes the flash period differently. Its data sheet can
+  be found at:
+    https://www.etekmicro.com/wp-content/uploads/datasheets/ET6326_datasheet.pdf
+
 properties:
   compatible:
-    enum:
-      - kinetic,ktd2026
-      - kinetic,ktd2027
+    oneOf:
+      - enum:
+          - kinetic,ktd2026
+          - kinetic,ktd2027
+      - items:
+          - const: etekmicro,et6326
+          - const: kinetic,ktd2026
 
   reg:
     maxItems: 1

-- 
Git-155)

Re: [PATCH 05/10] dt-bindings: leds: kinetic,ktd202x: Add Etek ET6326

[sashiko-bot]

> From: Joseph Telaak <joseph@telaak.dev>
> 
> The Etek ET6326 is a 3 channel LED driver with the register map, the I2C
> address and the current steps of the KTD2026. It only differs in the
> flash period, which is (Reg1 + 1) * 128 ms instead of 256 ms + Reg1 *
> 128 ms, so a KTD2026 driver runs it with blink periods that are 128 ms
> off. Document it with the KTD2026 as fallback.
> 
> The ET6326 drives the status LED of the GL.iNet GL-SFT1200 (Opal).
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Joseph Telaak <joseph@telaak.dev>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261101-ktd202x-et6326-v1-0-eb51eddad08c@telaak.dev?part=5

Re: [PATCH 05/10] dt-bindings: leds: kinetic,ktd202x: Add Etek ET6326

[Krzysztof Kozlowski]

On Sun, Nov 01, 2026 at 09:12:29PM -0500, Joseph Telaak wrote:
> The Etek ET6326 is a 3 channel LED driver with the register map, the I2C
> address and the current steps of the KTD2026. It only differs in the
> flash period, which is (Reg1 + 1) * 128 ms instead of 256 ms + Reg1 *
> 128 ms, so a KTD2026 driver runs it with blink periods that are 128 ms
> off. Document it with the KTD2026 as fallback.
> 
> The ET6326 drives the status LED of the GL.iNet GL-SFT1200 (Opal).
> 
> Assisted-by: Claude:claude-opus-5-5

Drop.

Please run scripts/checkpatch.pl on the patches and fix reported
warnings. After that, run also 'scripts/checkpatch.pl --strict' on the
patches and (probably) fix more warnings. Some warnings can be ignored,
especially from --strict run, but the code here looks like it needs a
fix. Feel free to get in touch if the warning is not clear.

> Signed-off-by: Joseph Telaak <joseph@telaak.dev>
> ---
>  .../devicetree/bindings/leds/kinetic,ktd202x.yaml         | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof

[PATCH 06/10] leds: rgb: ktd202x: Add Etek ET6326 support

[Joseph Telaak via B4 Relay]

From: Joseph Telaak <joseph@telaak.dev>

The Etek ET6326 is a KTD2026 with a different flash period encoding:
Reg1 = n selects (n + 1) * 128 ms, from 128 ms to 16.38 s, where the
KTD202x selects 256 ms + n * 128 ms and uses Reg1 = 127 for one shot
mode.

Move the number of LEDs and the flash period limits into per chip data
and add the ET6326, so the timer trigger gets the delays it asked for.

Link: https://www.etekmicro.com/wp-content/uploads/datasheets/ET6326_datasheet.pdf

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Joseph Telaak <joseph@telaak.dev>
---
 drivers/leds/rgb/Kconfig        |  3 +-
 drivers/leds/rgb/leds-ktd202x.c | 64 +++++++++++++++++++++++++++++++----------
 2 files changed, 51 insertions(+), 16 deletions(-)

diff --git a/drivers/leds/rgb/Kconfig b/drivers/leds/rgb/Kconfig
index 6e9ab5f60..3a57e5819 100644
--- a/drivers/leds/rgb/Kconfig
+++ b/drivers/leds/rgb/Kconfig
@@ -20,7 +20,8 @@ config LEDS_KTD202X
 	select REGMAP_I2C
 	help
 	  This option enables support for the Kinetic KTD2026/KTD2027
-	  RGB/White LED driver found in different BQ mobile phones.
+	  RGB/White LED driver found in different BQ mobile phones, and
+	  for the compatible Etek ET6326.
 	  It is a 3 or 4 channel LED driver programmed via an I2C interface.
 
 	  To compile this driver as a module, choose M here: the module
diff --git a/drivers/leds/rgb/leds-ktd202x.c b/drivers/leds/rgb/leds-ktd202x.c
index feea33b7e..67e65c1fc 100644
--- a/drivers/leds/rgb/leds-ktd202x.c
+++ b/drivers/leds/rgb/leds-ktd202x.c
@@ -5,6 +5,7 @@
  * Copyright 2023 André Apitzsch <git@apitzsch.eu>
  *
  * Datasheet: https://www.kinet-ic.com/uploads/KTD2026-7-04h.pdf
+ * Datasheet: https://www.etekmicro.com/wp-content/uploads/datasheets/ET6326_datasheet.pdf
  */
 
 #include <linux/i2c.h>
@@ -65,9 +66,7 @@
 /* Register 6-9 */
 #define KTD202X_LED_CURRENT_10_mA		0x4f
 
-#define KTD202X_FLASH_PERIOD_MIN_MS 256
 #define KTD202X_FLASH_PERIOD_STEP_MS 128
-#define KTD202X_FLASH_PERIOD_MAX_STEPS 126
 #define KTD202X_FLASH_ON_MAX 256
 
 #define KTD202X_MAX_BRIGHTNESS 192
@@ -86,6 +85,33 @@ static const struct reg_default ktd202x_reg_defaults[] = {
 	{ KTD202X_REG_LED_IOUT(3), KTD202X_LED_CURRENT_10_mA },
 };
 
+struct ktd202x_info {
+	unsigned long num_leds;
+	/* Flash period encoded by a Reg1 value of 0 */
+	unsigned int flash_period_min_ms;
+	unsigned int flash_period_max_steps;
+};
+
+/* Reg1 = 127 selects one shot mode on the KTD202x */
+static const struct ktd202x_info ktd2026_info = {
+	.num_leds = KTD2026_NUM_LEDS,
+	.flash_period_min_ms = 256,
+	.flash_period_max_steps = 126,
+};
+
+static const struct ktd202x_info ktd2027_info = {
+	.num_leds = KTD2027_NUM_LEDS,
+	.flash_period_min_ms = 256,
+	.flash_period_max_steps = 126,
+};
+
+/* The ET6326 flash period is (Reg1 + 1) * 128 ms, up to 16.38 s */
+static const struct ktd202x_info et6326_info = {
+	.num_leds = KTD2026_NUM_LEDS,
+	.flash_period_min_ms = 128,
+	.flash_period_max_steps = 127,
+};
+
 struct ktd202x_led {
 	struct ktd202x *chip;
 	union {
@@ -100,6 +126,7 @@ struct ktd202x {
 	struct regulator_bulk_data regulators[2];
 	struct device *dev;
 	struct regmap *regmap;
+	const struct ktd202x_info *info;
 	bool enabled;
 	unsigned long num_leds;
 	struct ktd202x_led leds[] __counted_by(num_leds);
@@ -271,6 +298,7 @@ static int ktd202x_blink_set(struct ktd202x_led *led, unsigned long *delay_on,
 			     unsigned long *delay_off, struct mc_subled *subleds,
 			     unsigned int num_channels)
 {
+	const struct ktd202x_info *info = led->chip->info;
 	unsigned long delay_total_ms;
 	int ret, num_steps, on;
 	u8 ctrl_mask = 0;
@@ -300,11 +328,11 @@ static int ktd202x_blink_set(struct ktd202x_led *led, unsigned long *delay_on,
 	/* Integer representation of time of flash period */
 	if (check_add_overflow(*delay_on, *delay_off, &delay_total_ms))
 		delay_total_ms = ULONG_MAX;
-	if (delay_total_ms < KTD202X_FLASH_PERIOD_MIN_MS)
+	if (delay_total_ms < info->flash_period_min_ms)
 		num_steps = 0;
 	else
-		num_steps = min_t(unsigned long, KTD202X_FLASH_PERIOD_MAX_STEPS,
-				  (delay_total_ms - KTD202X_FLASH_PERIOD_MIN_MS) /
+		num_steps = min_t(unsigned long, info->flash_period_max_steps,
+				  (delay_total_ms - info->flash_period_min_ms) /
 				  KTD202X_FLASH_PERIOD_STEP_MS);
 
 	/*
@@ -315,7 +343,7 @@ static int ktd202x_blink_set(struct ktd202x_led *led, unsigned long *delay_on,
 	on = clamp(on, 1, KTD202X_FLASH_ON_MAX - 1);
 
 	/* Actually used delay_{on,off} values */
-	delay_total_ms = num_steps * KTD202X_FLASH_PERIOD_STEP_MS + KTD202X_FLASH_PERIOD_MIN_MS;
+	delay_total_ms = num_steps * KTD202X_FLASH_PERIOD_STEP_MS + info->flash_period_min_ms;
 	*delay_on = (delay_total_ms * on) / KTD202X_FLASH_ON_MAX;
 	*delay_off = delay_total_ms - *delay_on;
 
@@ -543,8 +571,8 @@ static const struct regmap_config ktd202x_regmap_config = {
 static int ktd202x_probe(struct i2c_client *client)
 {
 	struct device *dev = &client->dev;
+	const struct ktd202x_info *info;
 	struct ktd202x *chip;
-	unsigned long num_leds;
 	int count;
 	int ret;
 
@@ -552,13 +580,17 @@ static int ktd202x_probe(struct i2c_client *client)
 	if (!count || count > KTD202X_MAX_LEDS)
 		return dev_err_probe(dev, -EINVAL, "Incorrect number of leds (%d)", count);
 
+	info = i2c_get_match_data(client);
+	if (!info)
+		return -ENODEV;
+
 	/* Loops over the LEDs go up to the number of channels, not of child nodes */
-	num_leds = (unsigned long)i2c_get_match_data(client);
-	chip = devm_kzalloc(dev, struct_size(chip, leds, num_leds), GFP_KERNEL);
+	chip = devm_kzalloc(dev, struct_size(chip, leds, info->num_leds), GFP_KERNEL);
 	if (!chip)
 		return -ENOMEM;
 
-	chip->num_leds = num_leds;
+	chip->info = info;
+	chip->num_leds = info->num_leds;
 	chip->dev = dev;
 	i2c_set_clientdata(client, chip);
 
@@ -618,15 +650,17 @@ static void ktd202x_shutdown(struct i2c_client *client)
 }
 
 static const struct i2c_device_id ktd202x_id[] = {
-	{ .name = "ktd2026", .driver_data = KTD2026_NUM_LEDS },
-	{ .name = "ktd2027", .driver_data = KTD2027_NUM_LEDS },
+	{ .name = "et6326", .driver_data = (kernel_ulong_t)&et6326_info },
+	{ .name = "ktd2026", .driver_data = (kernel_ulong_t)&ktd2026_info },
+	{ .name = "ktd2027", .driver_data = (kernel_ulong_t)&ktd2027_info },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, ktd202x_id);
 
 static const struct of_device_id ktd202x_match_table[] = {
-	{ .compatible = "kinetic,ktd2026", .data = (void *)KTD2026_NUM_LEDS },
-	{ .compatible = "kinetic,ktd2027", .data = (void *)KTD2027_NUM_LEDS },
+	{ .compatible = "etekmicro,et6326", .data = &et6326_info },
+	{ .compatible = "kinetic,ktd2026", .data = &ktd2026_info },
+	{ .compatible = "kinetic,ktd2027", .data = &ktd2027_info },
 	{}
 };
 MODULE_DEVICE_TABLE(of, ktd202x_match_table);
@@ -644,5 +678,5 @@ static struct i2c_driver ktd202x_driver = {
 module_i2c_driver(ktd202x_driver);
 
 MODULE_AUTHOR("André Apitzsch <git@apitzsch.eu>");
-MODULE_DESCRIPTION("Kinetic KTD2026/7 LED driver");
+MODULE_DESCRIPTION("Kinetic KTD2026/7 and Etek ET6326 LED driver");
 MODULE_LICENSE("GPL");

-- 
Git-155)

[PATCH 06/10] leds: rgb: ktd202x: Add Etek ET6326 support

[Joseph Telaak]

The Etek ET6326 is a KTD2026 with a different flash period encoding:
Reg1 = n selects (n + 1) * 128 ms, from 128 ms to 16.38 s, where the
KTD202x selects 256 ms + n * 128 ms and uses Reg1 = 127 for one shot
mode.

Move the number of LEDs and the flash period limits into per chip data
and add the ET6326, so the timer trigger gets the delays it asked for.

Link: https://www.etekmicro.com/wp-content/uploads/datasheets/ET6326_datasheet.pdf

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Joseph Telaak <joseph@telaak.dev>
---
 drivers/leds/rgb/Kconfig        |  3 +-
 drivers/leds/rgb/leds-ktd202x.c | 64 +++++++++++++++++++++++++++++++----------
 2 files changed, 51 insertions(+), 16 deletions(-)

diff --git a/drivers/leds/rgb/Kconfig b/drivers/leds/rgb/Kconfig
index 6e9ab5f60..3a57e5819 100644
--- a/drivers/leds/rgb/Kconfig
+++ b/drivers/leds/rgb/Kconfig
@@ -20,7 +20,8 @@ config LEDS_KTD202X
 	select REGMAP_I2C
 	help
 	  This option enables support for the Kinetic KTD2026/KTD2027
-	  RGB/White LED driver found in different BQ mobile phones.
+	  RGB/White LED driver found in different BQ mobile phones, and
+	  for the compatible Etek ET6326.
 	  It is a 3 or 4 channel LED driver programmed via an I2C interface.
 
 	  To compile this driver as a module, choose M here: the module
diff --git a/drivers/leds/rgb/leds-ktd202x.c b/drivers/leds/rgb/leds-ktd202x.c
index feea33b7e..67e65c1fc 100644
--- a/drivers/leds/rgb/leds-ktd202x.c
+++ b/drivers/leds/rgb/leds-ktd202x.c
@@ -5,6 +5,7 @@
  * Copyright 2023 André Apitzsch <git@apitzsch.eu>
  *
  * Datasheet: https://www.kinet-ic.com/uploads/KTD2026-7-04h.pdf
+ * Datasheet: https://www.etekmicro.com/wp-content/uploads/datasheets/ET6326_datasheet.pdf
  */
 
 #include <linux/i2c.h>
@@ -65,9 +66,7 @@
 /* Register 6-9 */
 #define KTD202X_LED_CURRENT_10_mA		0x4f
 
-#define KTD202X_FLASH_PERIOD_MIN_MS 256
 #define KTD202X_FLASH_PERIOD_STEP_MS 128
-#define KTD202X_FLASH_PERIOD_MAX_STEPS 126
 #define KTD202X_FLASH_ON_MAX 256
 
 #define KTD202X_MAX_BRIGHTNESS 192
@@ -86,6 +85,33 @@ static const struct reg_default ktd202x_reg_defaults[] = {
 	{ KTD202X_REG_LED_IOUT(3), KTD202X_LED_CURRENT_10_mA },
 };
 
+struct ktd202x_info {
+	unsigned long num_leds;
+	/* Flash period encoded by a Reg1 value of 0 */
+	unsigned int flash_period_min_ms;
+	unsigned int flash_period_max_steps;
+};
+
+/* Reg1 = 127 selects one shot mode on the KTD202x */
+static const struct ktd202x_info ktd2026_info = {
+	.num_leds = KTD2026_NUM_LEDS,
+	.flash_period_min_ms = 256,
+	.flash_period_max_steps = 126,
+};
+
+static const struct ktd202x_info ktd2027_info = {
+	.num_leds = KTD2027_NUM_LEDS,
+	.flash_period_min_ms = 256,
+	.flash_period_max_steps = 126,
+};
+
+/* The ET6326 flash period is (Reg1 + 1) * 128 ms, up to 16.38 s */
+static const struct ktd202x_info et6326_info = {
+	.num_leds = KTD2026_NUM_LEDS,
+	.flash_period_min_ms = 128,
+	.flash_period_max_steps = 127,
+};
+
 struct ktd202x_led {
 	struct ktd202x *chip;
 	union {
@@ -100,6 +126,7 @@ struct ktd202x {
 	struct regulator_bulk_data regulators[2];
 	struct device *dev;
 	struct regmap *regmap;
+	const struct ktd202x_info *info;
 	bool enabled;
 	unsigned long num_leds;
 	struct ktd202x_led leds[] __counted_by(num_leds);
@@ -271,6 +298,7 @@ static int ktd202x_blink_set(struct ktd202x_led *led, unsigned long *delay_on,
 			     unsigned long *delay_off, struct mc_subled *subleds,
 			     unsigned int num_channels)
 {
+	const struct ktd202x_info *info = led->chip->info;
 	unsigned long delay_total_ms;
 	int ret, num_steps, on;
 	u8 ctrl_mask = 0;
@@ -300,11 +328,11 @@ static int ktd202x_blink_set(struct ktd202x_led *led, unsigned long *delay_on,
 	/* Integer representation of time of flash period */
 	if (check_add_overflow(*delay_on, *delay_off, &delay_total_ms))
 		delay_total_ms = ULONG_MAX;
-	if (delay_total_ms < KTD202X_FLASH_PERIOD_MIN_MS)
+	if (delay_total_ms < info->flash_period_min_ms)
 		num_steps = 0;
 	else
-		num_steps = min_t(unsigned long, KTD202X_FLASH_PERIOD_MAX_STEPS,
-				  (delay_total_ms - KTD202X_FLASH_PERIOD_MIN_MS) /
+		num_steps = min_t(unsigned long, info->flash_period_max_steps,
+				  (delay_total_ms - info->flash_period_min_ms) /
 				  KTD202X_FLASH_PERIOD_STEP_MS);
 
 	/*
@@ -315,7 +343,7 @@ static int ktd202x_blink_set(struct ktd202x_led *led, unsigned long *delay_on,
 	on = clamp(on, 1, KTD202X_FLASH_ON_MAX - 1);
 
 	/* Actually used delay_{on,off} values */
-	delay_total_ms = num_steps * KTD202X_FLASH_PERIOD_STEP_MS + KTD202X_FLASH_PERIOD_MIN_MS;
+	delay_total_ms = num_steps * KTD202X_FLASH_PERIOD_STEP_MS + info->flash_period_min_ms;
 	*delay_on = (delay_total_ms * on) / KTD202X_FLASH_ON_MAX;
 	*delay_off = delay_total_ms - *delay_on;
 
@@ -543,8 +571,8 @@ static const struct regmap_config ktd202x_regmap_config = {
 static int ktd202x_probe(struct i2c_client *client)
 {
 	struct device *dev = &client->dev;
+	const struct ktd202x_info *info;
 	struct ktd202x *chip;
-	unsigned long num_leds;
 	int count;
 	int ret;
 
@@ -552,13 +580,17 @@ static int ktd202x_probe(struct i2c_client *client)
 	if (!count || count > KTD202X_MAX_LEDS)
 		return dev_err_probe(dev, -EINVAL, "Incorrect number of leds (%d)", count);
 
+	info = i2c_get_match_data(client);
+	if (!info)
+		return -ENODEV;
+
 	/* Loops over the LEDs go up to the number of channels, not of child nodes */
-	num_leds = (unsigned long)i2c_get_match_data(client);
-	chip = devm_kzalloc(dev, struct_size(chip, leds, num_leds), GFP_KERNEL);
+	chip = devm_kzalloc(dev, struct_size(chip, leds, info->num_leds), GFP_KERNEL);
 	if (!chip)
 		return -ENOMEM;
 
-	chip->num_leds = num_leds;
+	chip->info = info;
+	chip->num_leds = info->num_leds;
 	chip->dev = dev;
 	i2c_set_clientdata(client, chip);
 
@@ -618,15 +650,17 @@ static void ktd202x_shutdown(struct i2c_client *client)
 }
 
 static const struct i2c_device_id ktd202x_id[] = {
-	{ .name = "ktd2026", .driver_data = KTD2026_NUM_LEDS },
-	{ .name = "ktd2027", .driver_data = KTD2027_NUM_LEDS },
+	{ .name = "et6326", .driver_data = (kernel_ulong_t)&et6326_info },
+	{ .name = "ktd2026", .driver_data = (kernel_ulong_t)&ktd2026_info },
+	{ .name = "ktd2027", .driver_data = (kernel_ulong_t)&ktd2027_info },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, ktd202x_id);
 
 static const struct of_device_id ktd202x_match_table[] = {
-	{ .compatible = "kinetic,ktd2026", .data = (void *)KTD2026_NUM_LEDS },
-	{ .compatible = "kinetic,ktd2027", .data = (void *)KTD2027_NUM_LEDS },
+	{ .compatible = "etekmicro,et6326", .data = &et6326_info },
+	{ .compatible = "kinetic,ktd2026", .data = &ktd2026_info },
+	{ .compatible = "kinetic,ktd2027", .data = &ktd2027_info },
 	{}
 };
 MODULE_DEVICE_TABLE(of, ktd202x_match_table);
@@ -644,5 +678,5 @@ static struct i2c_driver ktd202x_driver = {
 module_i2c_driver(ktd202x_driver);
 
 MODULE_AUTHOR("André Apitzsch <git@apitzsch.eu>");
-MODULE_DESCRIPTION("Kinetic KTD2026/7 LED driver");
+MODULE_DESCRIPTION("Kinetic KTD2026/7 and Etek ET6326 LED driver");
 MODULE_LICENSE("GPL");

-- 
Git-155)

Re: [PATCH 06/10] leds: rgb: ktd202x: Add Etek ET6326 support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] leds: ktd202x: delay_on truncation to 0 in ktd202x_blink_set() on ET6326
--

commit 7c2c5ad9958f725ed6f4b50a7cb2208921d4f6b4
Author: Joseph Telaak <joseph@telaak.dev>

leds: rgb: ktd202x: Add Etek ET6326 support

This commit moves the LED count and flash period parameters into per-chip
match data and adds support for the Etek ET6326 LED controller.

Link: https://www.etekmicro.com/wp-content/uploads/datasheets/ET6326_datasheet.pdf

> diff --git a/drivers/leds/rgb/leds-ktd202x.c b/drivers/leds/rgb/leds-ktd202x.c
> index feea33b7e8856..67e65c1fcb5fb 100644
> --- a/drivers/leds/rgb/leds-ktd202x.c
> +++ b/drivers/leds/rgb/leds-ktd202x.c
[ ... ]
> @@ -86,6 +85,33 @@ static const struct reg_default ktd202x_reg_defaults[] = {
>  	{ KTD202X_REG_LED_IOUT(3), KTD202X_LED_CURRENT_10_mA },
>  };
>  
> +struct ktd202x_info {
> +	unsigned long num_leds;
> +	/* Flash period encoded by a Reg1 value of 0 */
> +	unsigned int flash_period_min_ms;
> +	unsigned int flash_period_max_steps;
> +};
[ ... ]
> +/* The ET6326 flash period is (Reg1 + 1) * 128 ms, up to 16.38 s */
> +static const struct ktd202x_info et6326_info = {
> +	.num_leds = KTD2026_NUM_LEDS,
> +	.flash_period_min_ms = 128,
> +	.flash_period_max_steps = 127,
> +};
[ ... ]
> @@ -300,11 +328,11 @@ static int ktd202x_blink_set(struct ktd202x_led *led, unsigned long *delay_on,
>  	/* Integer representation of time of flash period */
>  	if (check_add_overflow(*delay_on, *delay_off, &delay_total_ms))
>  		delay_total_ms = ULONG_MAX;
> -	if (delay_total_ms < KTD202X_FLASH_PERIOD_MIN_MS)
> +	if (delay_total_ms < info->flash_period_min_ms)
>  		num_steps = 0;
>  	else
> -		num_steps = min_t(unsigned long, KTD202X_FLASH_PERIOD_MAX_STEPS,
> -				  (delay_total_ms - KTD202X_FLASH_PERIOD_MIN_MS) /
> +		num_steps = min_t(unsigned long, info->flash_period_max_steps,
> +				  (delay_total_ms - info->flash_period_min_ms) /
>  				  KTD202X_FLASH_PERIOD_STEP_MS);
>  
>  	/*
> @@ -315,7 +343,7 @@ static int ktd202x_blink_set(struct ktd202x_led *led, unsigned long *delay_on,
>  	on = clamp(on, 1, KTD202X_FLASH_ON_MAX - 1);
>  
>  	/* Actually used delay_{on,off} values */
> -	delay_total_ms = num_steps * KTD202X_FLASH_PERIOD_STEP_MS + KTD202X_FLASH_PERIOD_MIN_MS;
> +	delay_total_ms = num_steps * KTD202X_FLASH_PERIOD_STEP_MS + info->flash_period_min_ms;
>  	*delay_on = (delay_total_ms * on) / KTD202X_FLASH_ON_MAX;

[Severity: Medium]
Can this integer division truncate *delay_on to 0 on the ET6326?

On the KTD2026 and KTD2027, flash_period_min_ms is 256, which matches
KTD202X_FLASH_ON_MAX (256), so (delay_total_ms * on) / KTD202X_FLASH_ON_MAX
is always at least 1 when on is clamped to a minimum of 1.

With et6326_info setting flash_period_min_ms to 128, if a user configures
a short period where num_steps is 0 (delay_total_ms < 256 ms) and on is
clamped to 1 (for example, writing delay_on = 1 and delay_off = 127 via
the timer trigger), ktd202x_blink_set() computes:

	delay_total_ms = 0 * 128 + 128;
	*delay_on = (128 * 1) / 256;

which sets *delay_on to 0 while still programming the hardware to blink
with on = 1.

When led_blink_setup() stores blink_delay_on = 0, any subsequent write to
delay_off in sysfs calls led_delay_off_store() -> led_blink_set() with
*delay_on == 0, hitting the !*delay_on check in ktd202x_blink_single_set()
or ktd202x_blink_mc_set():

	/* Never on - just set to off */
	if (!*delay_on)
		return ktd202x_brightness_single_set(cdev, LED_OFF);

and turning the LED off instead of updating the blink period, which breaks
the invariant from commit fb5b897c7d3f ("leds: rgb: ktd202x: Never program
an on time of zero").

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261101-ktd202x-et6326-v1-0-eb51eddad08c@telaak.dev?part=6

[PATCH 07/10] leds: rgb: ktd202x: Add hardware pattern support

[Joseph Telaak via B4 Relay]

From: Joseph Telaak <joseph@telaak.dev>

The KTD202x and the ET6326 can ramp the LED current up and down on
their own: the flash timer that already does blink_set also has rise
and fall times of up to 1.92 s. Without them, a breathing light needs
the software pattern trigger, which is an I2C write every 50 ms.

Implement pattern_set and pattern_clear for the pattern trigger's
hw_pattern. A pattern is the one breathing cycle the hardware has,
"0 rise brightness high brightness fall 0 low", which the software
pattern trigger plays as the same breath, repeated forever.

blink_set now clears the ramp times, so a blink after a pattern stays
sharp.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Joseph Telaak <joseph@telaak.dev>
---
 Documentation/leds/index.rst        |   1 +
 Documentation/leds/leds-ktd202x.rst |  41 ++++++++
 drivers/leds/rgb/leds-ktd202x.c     | 186 ++++++++++++++++++++++++++++++++++--
 3 files changed, 221 insertions(+), 7 deletions(-)

diff --git a/Documentation/leds/index.rst b/Documentation/leds/index.rst
index 23fa9ff7a..54d1c7368 100644
--- a/Documentation/leds/index.rst
+++ b/Documentation/leds/index.rst
@@ -19,6 +19,7 @@ LEDs
    leds-blinkm
    leds-cht-wcove
    leds-el15203000
+   leds-ktd202x
    leds-lm3556
    leds-lp3944
    leds-lp5521
diff --git a/Documentation/leds/leds-ktd202x.rst b/Documentation/leds/leds-ktd202x.rst
new file mode 100644
index 000000000..2ee537f57
--- /dev/null
+++ b/Documentation/leds/leds-ktd202x.rst
@@ -0,0 +1,41 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=============================================
+Kinetic KTD2026/7 and Etek ET6326 LED driver
+=============================================
+
+The chips have a hardware timer that blinks or breathes the LEDs without the
+CPU. The timer trigger uses it for blinking, and the pattern trigger uses it
+for breathing through the ``hw_pattern`` file.
+
+The hardware pattern is one breathing cycle of four steps, which repeats
+forever::
+
+    "0 rise brightness high brightness fall 0 low"
+
+rise
+    Ramp-up time in ms, from 0 to 1920 in steps of 128 ms.
+
+high
+    Time spent at the brightness in ms.
+
+fall
+    Ramp-down time in ms, from 0 to 1920 in steps of 128 ms.
+
+low
+    Time spent off in ms.
+
+The first and the last brightness must be 0, and the two in the middle must be
+equal and not 0. ``repeat`` must be -1, its default.
+
+The sum of the four times is the flash period. The driver rounds it up to the
+next step of 128 ms, between 256 ms (128 ms on the ET6326) and 16384 ms.
+
+The flash period, the on time and the ramp times are shared by all LEDs of a
+chip. When several LEDs blink or breathe at once, they all follow the timing
+that was set last.
+
+For example, to breathe with a period of 4 seconds::
+
+    echo pattern > /sys/class/leds/blue:status/trigger
+    echo "0 1000 192 1000 192 1000 0 1000" > /sys/class/leds/blue:status/hw_pattern
diff --git a/drivers/leds/rgb/leds-ktd202x.c b/drivers/leds/rgb/leds-ktd202x.c
index 67e65c1fc..ed0e8e74c 100644
--- a/drivers/leds/rgb/leds-ktd202x.c
+++ b/drivers/leds/rgb/leds-ktd202x.c
@@ -62,12 +62,20 @@
 
 /* Register 5 */
 #define KTD202X_RAMP_TIMES_2_MS			0x00
+#define KTD202X_RAMP_TRISE(x)			(x)
+#define KTD202X_RAMP_TFALL(x)			((x) << 4)
 
 /* Register 6-9 */
 #define KTD202X_LED_CURRENT_10_mA		0x4f
 
 #define KTD202X_FLASH_PERIOD_STEP_MS 128
 #define KTD202X_FLASH_ON_MAX 256
+#define KTD202X_RAMP_STEP_MS 128
+#define KTD202X_RAMP_MAX_STEPS 15
+
+/* rise, high, fall and low */
+#define KTD202X_PATTERN_LEN 4
+#define KTD202X_PATTERN_MAX_MS 16384
 
 #define KTD202X_MAX_BRIGHTNESS 192
 
@@ -294,6 +302,23 @@ static int ktd202x_brightness_mc_set(struct led_classdev *cdev,
 	return ret;
 }
 
+static unsigned int ktd202x_flash_period_steps(const struct ktd202x_info *info,
+					       unsigned long period_ms)
+{
+	if (period_ms < info->flash_period_min_ms)
+		return 0;
+
+	return min_t(unsigned long, info->flash_period_max_steps,
+		     (period_ms - info->flash_period_min_ms) /
+		     KTD202X_FLASH_PERIOD_STEP_MS);
+}
+
+static unsigned long ktd202x_flash_period_ms(const struct ktd202x_info *info,
+					     unsigned int num_steps)
+{
+	return num_steps * KTD202X_FLASH_PERIOD_STEP_MS + info->flash_period_min_ms;
+}
+
 static int ktd202x_blink_set(struct ktd202x_led *led, unsigned long *delay_on,
 			     unsigned long *delay_off, struct mc_subled *subleds,
 			     unsigned int num_channels)
@@ -328,12 +353,7 @@ static int ktd202x_blink_set(struct ktd202x_led *led, unsigned long *delay_on,
 	/* Integer representation of time of flash period */
 	if (check_add_overflow(*delay_on, *delay_off, &delay_total_ms))
 		delay_total_ms = ULONG_MAX;
-	if (delay_total_ms < info->flash_period_min_ms)
-		num_steps = 0;
-	else
-		num_steps = min_t(unsigned long, info->flash_period_max_steps,
-				  (delay_total_ms - info->flash_period_min_ms) /
-				  KTD202X_FLASH_PERIOD_STEP_MS);
+	num_steps = ktd202x_flash_period_steps(info, delay_total_ms);
 
 	/*
 	 * Integer representation of percentage of LED ON time. A value of 0
@@ -343,7 +363,7 @@ static int ktd202x_blink_set(struct ktd202x_led *led, unsigned long *delay_on,
 	on = clamp(on, 1, KTD202X_FLASH_ON_MAX - 1);
 
 	/* Actually used delay_{on,off} values */
-	delay_total_ms = num_steps * KTD202X_FLASH_PERIOD_STEP_MS + info->flash_period_min_ms;
+	delay_total_ms = ktd202x_flash_period_ms(info, num_steps);
 	*delay_on = (delay_total_ms * on) / KTD202X_FLASH_ON_MAX;
 	*delay_off = delay_total_ms - *delay_on;
 
@@ -356,6 +376,12 @@ static int ktd202x_blink_set(struct ktd202x_led *led, unsigned long *delay_on,
 	if (ret)
 		goto out;
 
+	/* Drop the ramps a hardware pattern may have left behind */
+	ret = regmap_write(led->chip->regmap, KTD202X_REG_TRISE_FALL,
+			   KTD202X_RAMP_TIMES_2_MS);
+	if (ret)
+		goto out;
+
 	ret = regmap_update_bits(led->chip->regmap, KTD202X_REG_CHANNEL_CTRL,
 				 ctrl_mask, ctrl_pwm1);
 out:
@@ -420,6 +446,148 @@ static int ktd202x_blink_mc_set(struct led_classdev *cdev,
 				 mc->num_colors);
 }
 
+/*
+ * The hardware pattern is a single breathing cycle that repeats forever:
+ *
+ *   "0 rise brightness high brightness fall 0 low"
+ *
+ * The LED ramps up for the rise time, stays at the brightness for the high
+ * time, ramps down for the fall time and stays off for the low time.
+ */
+static int ktd202x_pattern_check(struct led_classdev *cdev,
+				 struct led_pattern *pattern, u32 len,
+				 int repeat)
+{
+	/*
+	 * Only forever: the pattern trigger passes -1, or 0 when repeat
+	 * has not been written since the trigger was set.
+	 */
+	if (len != KTD202X_PATTERN_LEN || repeat > 0)
+		return -EINVAL;
+
+	if (pattern[0].brightness || pattern[3].brightness ||
+	    pattern[1].brightness != pattern[2].brightness ||
+	    pattern[1].brightness <= 0 ||
+	    pattern[1].brightness > cdev->max_brightness)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int ktd202x_pattern_program(struct ktd202x_led *led,
+				   struct led_pattern *pattern,
+				   struct mc_subled *subleds,
+				   unsigned int num_channels)
+{
+	const struct ktd202x_info *info = led->chip->info;
+	unsigned int t[KTD202X_PATTERN_LEN];
+	unsigned int rise, fall, num_steps, on;
+	unsigned long period_ms, ton_ms;
+	u8 ctrl_mask = 0;
+	u8 ctrl_pwm1 = 0;
+	int ret;
+	int i;
+
+	for (i = 0; i < num_channels; i++) {
+		int channel = subleds[i].channel;
+
+		ctrl_mask |= KTD202X_CHANNEL_CTRL_MASK(channel);
+		ctrl_pwm1 |= KTD202X_CHANNEL_CTRL_PWM1(channel);
+	}
+
+	for (i = 0; i < KTD202X_PATTERN_LEN; i++)
+		t[i] = min_t(u32, pattern[i].delta_t, KTD202X_PATTERN_MAX_MS);
+
+	rise = min_t(unsigned int, KTD202X_RAMP_MAX_STEPS,
+		     DIV_ROUND_CLOSEST(t[0], KTD202X_RAMP_STEP_MS));
+	fall = min_t(unsigned int, KTD202X_RAMP_MAX_STEPS,
+		     DIV_ROUND_CLOSEST(t[2], KTD202X_RAMP_STEP_MS));
+
+	/* Round the period up so that the fall ramp is not cut short */
+	period_ms = t[0] + t[1] + t[2] + t[3];
+	num_steps = ktd202x_flash_period_steps(info, period_ms +
+					       KTD202X_FLASH_PERIOD_STEP_MS - 1);
+	period_ms = ktd202x_flash_period_ms(info, num_steps);
+
+	/* The on time includes the rise time */
+	ton_ms = t[0] + t[1];
+	on = clamp_t(unsigned long,
+		     DIV_ROUND_CLOSEST(ton_ms * KTD202X_FLASH_ON_MAX, period_ms),
+		     1, KTD202X_FLASH_ON_MAX - 1);
+
+	mutex_lock(&led->chip->mutex);
+
+	ret = regmap_write(led->chip->regmap, KTD202X_REG_FLASH_PERIOD, num_steps);
+	if (ret)
+		goto out;
+
+	ret = regmap_write(led->chip->regmap, KTD202X_REG_PWM1_TIMER, on);
+	if (ret)
+		goto out;
+
+	ret = regmap_write(led->chip->regmap, KTD202X_REG_TRISE_FALL,
+			   KTD202X_RAMP_TFALL(fall) | KTD202X_RAMP_TRISE(rise));
+	if (ret)
+		goto out;
+
+	ret = regmap_update_bits(led->chip->regmap, KTD202X_REG_CHANNEL_CTRL,
+				 ctrl_mask, ctrl_pwm1);
+out:
+	mutex_unlock(&led->chip->mutex);
+	return ret;
+}
+
+static int ktd202x_pattern_single_set(struct led_classdev *cdev,
+				      struct led_pattern *pattern, u32 len,
+				      int repeat)
+{
+	struct ktd202x_led *led = container_of(cdev, struct ktd202x_led, cdev);
+	struct mc_subled info;
+	int ret;
+
+	ret = ktd202x_pattern_check(cdev, pattern, len, repeat);
+	if (ret)
+		return ret;
+
+	ret = ktd202x_brightness_single_set(cdev, pattern[1].brightness);
+	if (ret)
+		return ret;
+
+	info.channel = led->index;
+
+	return ktd202x_pattern_program(led, pattern, &info, 1);
+}
+
+static int ktd202x_pattern_single_clear(struct led_classdev *cdev)
+{
+	return ktd202x_brightness_single_set(cdev, LED_OFF);
+}
+
+static int ktd202x_pattern_mc_set(struct led_classdev *cdev,
+				  struct led_pattern *pattern, u32 len,
+				  int repeat)
+{
+	struct led_classdev_mc *mc = lcdev_to_mccdev(cdev);
+	struct ktd202x_led *led = container_of(mc, struct ktd202x_led, mcdev);
+	int ret;
+
+	ret = ktd202x_pattern_check(cdev, pattern, len, repeat);
+	if (ret)
+		return ret;
+
+	ret = ktd202x_brightness_mc_set(cdev, pattern[1].brightness);
+	if (ret)
+		return ret;
+
+	return ktd202x_pattern_program(led, pattern, mc->subled_info,
+				       mc->num_colors);
+}
+
+static int ktd202x_pattern_mc_clear(struct led_classdev *cdev)
+{
+	return ktd202x_brightness_mc_set(cdev, LED_OFF);
+}
+
 static int ktd202x_setup_led_rgb(struct ktd202x *chip, struct fwnode_handle *fwnode,
 				 struct ktd202x_led *led, struct led_init_data *init_data)
 {
@@ -471,6 +639,8 @@ static int ktd202x_setup_led_rgb(struct ktd202x *chip, struct fwnode_handle *fwn
 	cdev = &led->mcdev.led_cdev;
 	cdev->brightness_set_blocking = ktd202x_brightness_mc_set;
 	cdev->blink_set = ktd202x_blink_mc_set;
+	cdev->pattern_set = ktd202x_pattern_mc_set;
+	cdev->pattern_clear = ktd202x_pattern_mc_clear;
 
 	return devm_led_classdev_multicolor_register_ext(chip->dev, &led->mcdev, init_data);
 }
@@ -492,6 +662,8 @@ static int ktd202x_setup_led_single(struct ktd202x *chip, struct fwnode_handle *
 	cdev = &led->cdev;
 	cdev->brightness_set_blocking = ktd202x_brightness_single_set;
 	cdev->blink_set = ktd202x_blink_single_set;
+	cdev->pattern_set = ktd202x_pattern_single_set;
+	cdev->pattern_clear = ktd202x_pattern_single_clear;
 
 	return devm_led_classdev_register_ext(chip->dev, &led->cdev, init_data);
 }

-- 
Git-155)

[PATCH 07/10] leds: rgb: ktd202x: Add hardware pattern support

[Joseph Telaak]

The KTD202x and the ET6326 can ramp the LED current up and down on
their own: the flash timer that already does blink_set also has rise
and fall times of up to 1.92 s. Without them, a breathing light needs
the software pattern trigger, which is an I2C write every 50 ms.

Implement pattern_set and pattern_clear for the pattern trigger's
hw_pattern. A pattern is the one breathing cycle the hardware has,
"0 rise brightness high brightness fall 0 low", which the software
pattern trigger plays as the same breath, repeated forever.

blink_set now clears the ramp times, so a blink after a pattern stays
sharp.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Joseph Telaak <joseph@telaak.dev>
---
 Documentation/leds/index.rst        |   1 +
 Documentation/leds/leds-ktd202x.rst |  41 ++++++++
 drivers/leds/rgb/leds-ktd202x.c     | 186 ++++++++++++++++++++++++++++++++++--
 3 files changed, 221 insertions(+), 7 deletions(-)

diff --git a/Documentation/leds/index.rst b/Documentation/leds/index.rst
index 23fa9ff7a..54d1c7368 100644
--- a/Documentation/leds/index.rst
+++ b/Documentation/leds/index.rst
@@ -19,6 +19,7 @@ LEDs
    leds-blinkm
    leds-cht-wcove
    leds-el15203000
+   leds-ktd202x
    leds-lm3556
    leds-lp3944
    leds-lp5521
diff --git a/Documentation/leds/leds-ktd202x.rst b/Documentation/leds/leds-ktd202x.rst
new file mode 100644
index 000000000..2ee537f57
--- /dev/null
+++ b/Documentation/leds/leds-ktd202x.rst
@@ -0,0 +1,41 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=============================================
+Kinetic KTD2026/7 and Etek ET6326 LED driver
+=============================================
+
+The chips have a hardware timer that blinks or breathes the LEDs without the
+CPU. The timer trigger uses it for blinking, and the pattern trigger uses it
+for breathing through the ``hw_pattern`` file.
+
+The hardware pattern is one breathing cycle of four steps, which repeats
+forever::
+
+    "0 rise brightness high brightness fall 0 low"
+
+rise
+    Ramp-up time in ms, from 0 to 1920 in steps of 128 ms.
+
+high
+    Time spent at the brightness in ms.
+
+fall
+    Ramp-down time in ms, from 0 to 1920 in steps of 128 ms.
+
+low
+    Time spent off in ms.
+
+The first and the last brightness must be 0, and the two in the middle must be
+equal and not 0. ``repeat`` must be -1, its default.
+
+The sum of the four times is the flash period. The driver rounds it up to the
+next step of 128 ms, between 256 ms (128 ms on the ET6326) and 16384 ms.
+
+The flash period, the on time and the ramp times are shared by all LEDs of a
+chip. When several LEDs blink or breathe at once, they all follow the timing
+that was set last.
+
+For example, to breathe with a period of 4 seconds::
+
+    echo pattern > /sys/class/leds/blue:status/trigger
+    echo "0 1000 192 1000 192 1000 0 1000" > /sys/class/leds/blue:status/hw_pattern
diff --git a/drivers/leds/rgb/leds-ktd202x.c b/drivers/leds/rgb/leds-ktd202x.c
index 67e65c1fc..ed0e8e74c 100644
--- a/drivers/leds/rgb/leds-ktd202x.c
+++ b/drivers/leds/rgb/leds-ktd202x.c
@@ -62,12 +62,20 @@
 
 /* Register 5 */
 #define KTD202X_RAMP_TIMES_2_MS			0x00
+#define KTD202X_RAMP_TRISE(x)			(x)
+#define KTD202X_RAMP_TFALL(x)			((x) << 4)
 
 /* Register 6-9 */
 #define KTD202X_LED_CURRENT_10_mA		0x4f
 
 #define KTD202X_FLASH_PERIOD_STEP_MS 128
 #define KTD202X_FLASH_ON_MAX 256
+#define KTD202X_RAMP_STEP_MS 128
+#define KTD202X_RAMP_MAX_STEPS 15
+
+/* rise, high, fall and low */
+#define KTD202X_PATTERN_LEN 4
+#define KTD202X_PATTERN_MAX_MS 16384
 
 #define KTD202X_MAX_BRIGHTNESS 192
 
@@ -294,6 +302,23 @@ static int ktd202x_brightness_mc_set(struct led_classdev *cdev,
 	return ret;
 }
 
+static unsigned int ktd202x_flash_period_steps(const struct ktd202x_info *info,
+					       unsigned long period_ms)
+{
+	if (period_ms < info->flash_period_min_ms)
+		return 0;
+
+	return min_t(unsigned long, info->flash_period_max_steps,
+		     (period_ms - info->flash_period_min_ms) /
+		     KTD202X_FLASH_PERIOD_STEP_MS);
+}
+
+static unsigned long ktd202x_flash_period_ms(const struct ktd202x_info *info,
+					     unsigned int num_steps)
+{
+	return num_steps * KTD202X_FLASH_PERIOD_STEP_MS + info->flash_period_min_ms;
+}
+
 static int ktd202x_blink_set(struct ktd202x_led *led, unsigned long *delay_on,
 			     unsigned long *delay_off, struct mc_subled *subleds,
 			     unsigned int num_channels)
@@ -328,12 +353,7 @@ static int ktd202x_blink_set(struct ktd202x_led *led, unsigned long *delay_on,
 	/* Integer representation of time of flash period */
 	if (check_add_overflow(*delay_on, *delay_off, &delay_total_ms))
 		delay_total_ms = ULONG_MAX;
-	if (delay_total_ms < info->flash_period_min_ms)
-		num_steps = 0;
-	else
-		num_steps = min_t(unsigned long, info->flash_period_max_steps,
-				  (delay_total_ms - info->flash_period_min_ms) /
-				  KTD202X_FLASH_PERIOD_STEP_MS);
+	num_steps = ktd202x_flash_period_steps(info, delay_total_ms);
 
 	/*
 	 * Integer representation of percentage of LED ON time. A value of 0
@@ -343,7 +363,7 @@ static int ktd202x_blink_set(struct ktd202x_led *led, unsigned long *delay_on,
 	on = clamp(on, 1, KTD202X_FLASH_ON_MAX - 1);
 
 	/* Actually used delay_{on,off} values */
-	delay_total_ms = num_steps * KTD202X_FLASH_PERIOD_STEP_MS + info->flash_period_min_ms;
+	delay_total_ms = ktd202x_flash_period_ms(info, num_steps);
 	*delay_on = (delay_total_ms * on) / KTD202X_FLASH_ON_MAX;
 	*delay_off = delay_total_ms - *delay_on;
 
@@ -356,6 +376,12 @@ static int ktd202x_blink_set(struct ktd202x_led *led, unsigned long *delay_on,
 	if (ret)
 		goto out;
 
+	/* Drop the ramps a hardware pattern may have left behind */
+	ret = regmap_write(led->chip->regmap, KTD202X_REG_TRISE_FALL,
+			   KTD202X_RAMP_TIMES_2_MS);
+	if (ret)
+		goto out;
+
 	ret = regmap_update_bits(led->chip->regmap, KTD202X_REG_CHANNEL_CTRL,
 				 ctrl_mask, ctrl_pwm1);
 out:
@@ -420,6 +446,148 @@ static int ktd202x_blink_mc_set(struct led_classdev *cdev,
 				 mc->num_colors);
 }
 
+/*
+ * The hardware pattern is a single breathing cycle that repeats forever:
+ *
+ *   "0 rise brightness high brightness fall 0 low"
+ *
+ * The LED ramps up for the rise time, stays at the brightness for the high
+ * time, ramps down for the fall time and stays off for the low time.
+ */
+static int ktd202x_pattern_check(struct led_classdev *cdev,
+				 struct led_pattern *pattern, u32 len,
+				 int repeat)
+{
+	/*
+	 * Only forever: the pattern trigger passes -1, or 0 when repeat
+	 * has not been written since the trigger was set.
+	 */
+	if (len != KTD202X_PATTERN_LEN || repeat > 0)
+		return -EINVAL;
+
+	if (pattern[0].brightness || pattern[3].brightness ||
+	    pattern[1].brightness != pattern[2].brightness ||
+	    pattern[1].brightness <= 0 ||
+	    pattern[1].brightness > cdev->max_brightness)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int ktd202x_pattern_program(struct ktd202x_led *led,
+				   struct led_pattern *pattern,
+				   struct mc_subled *subleds,
+				   unsigned int num_channels)
+{
+	const struct ktd202x_info *info = led->chip->info;
+	unsigned int t[KTD202X_PATTERN_LEN];
+	unsigned int rise, fall, num_steps, on;
+	unsigned long period_ms, ton_ms;
+	u8 ctrl_mask = 0;
+	u8 ctrl_pwm1 = 0;
+	int ret;
+	int i;
+
+	for (i = 0; i < num_channels; i++) {
+		int channel = subleds[i].channel;
+
+		ctrl_mask |= KTD202X_CHANNEL_CTRL_MASK(channel);
+		ctrl_pwm1 |= KTD202X_CHANNEL_CTRL_PWM1(channel);
+	}
+
+	for (i = 0; i < KTD202X_PATTERN_LEN; i++)
+		t[i] = min_t(u32, pattern[i].delta_t, KTD202X_PATTERN_MAX_MS);
+
+	rise = min_t(unsigned int, KTD202X_RAMP_MAX_STEPS,
+		     DIV_ROUND_CLOSEST(t[0], KTD202X_RAMP_STEP_MS));
+	fall = min_t(unsigned int, KTD202X_RAMP_MAX_STEPS,
+		     DIV_ROUND_CLOSEST(t[2], KTD202X_RAMP_STEP_MS));
+
+	/* Round the period up so that the fall ramp is not cut short */
+	period_ms = t[0] + t[1] + t[2] + t[3];
+	num_steps = ktd202x_flash_period_steps(info, period_ms +
+					       KTD202X_FLASH_PERIOD_STEP_MS - 1);
+	period_ms = ktd202x_flash_period_ms(info, num_steps);
+
+	/* The on time includes the rise time */
+	ton_ms = t[0] + t[1];
+	on = clamp_t(unsigned long,
+		     DIV_ROUND_CLOSEST(ton_ms * KTD202X_FLASH_ON_MAX, period_ms),
+		     1, KTD202X_FLASH_ON_MAX - 1);
+
+	mutex_lock(&led->chip->mutex);
+
+	ret = regmap_write(led->chip->regmap, KTD202X_REG_FLASH_PERIOD, num_steps);
+	if (ret)
+		goto out;
+
+	ret = regmap_write(led->chip->regmap, KTD202X_REG_PWM1_TIMER, on);
+	if (ret)
+		goto out;
+
+	ret = regmap_write(led->chip->regmap, KTD202X_REG_TRISE_FALL,
+			   KTD202X_RAMP_TFALL(fall) | KTD202X_RAMP_TRISE(rise));
+	if (ret)
+		goto out;
+
+	ret = regmap_update_bits(led->chip->regmap, KTD202X_REG_CHANNEL_CTRL,
+				 ctrl_mask, ctrl_pwm1);
+out:
+	mutex_unlock(&led->chip->mutex);
+	return ret;
+}
+
+static int ktd202x_pattern_single_set(struct led_classdev *cdev,
+				      struct led_pattern *pattern, u32 len,
+				      int repeat)
+{
+	struct ktd202x_led *led = container_of(cdev, struct ktd202x_led, cdev);
+	struct mc_subled info;
+	int ret;
+
+	ret = ktd202x_pattern_check(cdev, pattern, len, repeat);
+	if (ret)
+		return ret;
+
+	ret = ktd202x_brightness_single_set(cdev, pattern[1].brightness);
+	if (ret)
+		return ret;
+
+	info.channel = led->index;
+
+	return ktd202x_pattern_program(led, pattern, &info, 1);
+}
+
+static int ktd202x_pattern_single_clear(struct led_classdev *cdev)
+{
+	return ktd202x_brightness_single_set(cdev, LED_OFF);
+}
+
+static int ktd202x_pattern_mc_set(struct led_classdev *cdev,
+				  struct led_pattern *pattern, u32 len,
+				  int repeat)
+{
+	struct led_classdev_mc *mc = lcdev_to_mccdev(cdev);
+	struct ktd202x_led *led = container_of(mc, struct ktd202x_led, mcdev);
+	int ret;
+
+	ret = ktd202x_pattern_check(cdev, pattern, len, repeat);
+	if (ret)
+		return ret;
+
+	ret = ktd202x_brightness_mc_set(cdev, pattern[1].brightness);
+	if (ret)
+		return ret;
+
+	return ktd202x_pattern_program(led, pattern, mc->subled_info,
+				       mc->num_colors);
+}
+
+static int ktd202x_pattern_mc_clear(struct led_classdev *cdev)
+{
+	return ktd202x_brightness_mc_set(cdev, LED_OFF);
+}
+
 static int ktd202x_setup_led_rgb(struct ktd202x *chip, struct fwnode_handle *fwnode,
 				 struct ktd202x_led *led, struct led_init_data *init_data)
 {
@@ -471,6 +639,8 @@ static int ktd202x_setup_led_rgb(struct ktd202x *chip, struct fwnode_handle *fwn
 	cdev = &led->mcdev.led_cdev;
 	cdev->brightness_set_blocking = ktd202x_brightness_mc_set;
 	cdev->blink_set = ktd202x_blink_mc_set;
+	cdev->pattern_set = ktd202x_pattern_mc_set;
+	cdev->pattern_clear = ktd202x_pattern_mc_clear;
 
 	return devm_led_classdev_multicolor_register_ext(chip->dev, &led->mcdev, init_data);
 }
@@ -492,6 +662,8 @@ static int ktd202x_setup_led_single(struct ktd202x *chip, struct fwnode_handle *
 	cdev = &led->cdev;
 	cdev->brightness_set_blocking = ktd202x_brightness_single_set;
 	cdev->blink_set = ktd202x_blink_single_set;
+	cdev->pattern_set = ktd202x_pattern_single_set;
+	cdev->pattern_clear = ktd202x_pattern_single_clear;
 
 	return devm_led_classdev_register_ext(chip->dev, &led->cdev, init_data);
 }

-- 
Git-155)

Re: [PATCH 07/10] leds: rgb: ktd202x: Add hardware pattern support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] leds: ktd202x: ktd202x_pattern_program() enables PWM1 on 0-brightness channels
--

commit cb2ed015f23bfa814b09df784ba07474c986d4de
Author: Joseph Telaak <joseph@telaak.dev>

leds: rgb: ktd202x: Add hardware pattern support

This commit adds hardware pattern support to the ktd202x LED driver by
implementing the pattern_set and pattern_clear callbacks using the chip's
hardware flash timer and rise/fall ramp registers.

> diff --git a/drivers/leds/rgb/leds-ktd202x.c b/drivers/leds/rgb/leds-ktd202x.c
> index 67e65c1fcb5fb..ed0e8e74c9eab 100644
> --- a/drivers/leds/rgb/leds-ktd202x.c
> +++ b/drivers/leds/rgb/leds-ktd202x.c
[ ... ]
> @@ -420,6 +446,148 @@ static int ktd202x_blink_mc_set(struct led_classdev *cdev,
[ ... ]
> +static int ktd202x_pattern_program(struct ktd202x_led *led,
> +				   struct led_pattern *pattern,
> +				   struct mc_subled *subleds,
> +				   unsigned int num_channels)
> +{
> +	const struct ktd202x_info *info = led->chip->info;
> +	unsigned int t[KTD202X_PATTERN_LEN];
> +	unsigned int rise, fall, num_steps, on;
> +	unsigned long period_ms, ton_ms;
> +	u8 ctrl_mask = 0;
> +	u8 ctrl_pwm1 = 0;
> +	int ret;
> +	int i;
> +
> +	for (i = 0; i < num_channels; i++) {
> +		int channel = subleds[i].channel;
> +
> +		ctrl_mask |= KTD202X_CHANNEL_CTRL_MASK(channel);
> +		ctrl_pwm1 |= KTD202X_CHANNEL_CTRL_PWM1(channel);
> +	}
> +
> +	for (i = 0; i < KTD202X_PATTERN_LEN; i++)
> +		t[i] = min_t(u32, pattern[i].delta_t, KTD202X_PATTERN_MAX_MS);
> +
> +	rise = min_t(unsigned int, KTD202X_RAMP_MAX_STEPS,
> +		     DIV_ROUND_CLOSEST(t[0], KTD202X_RAMP_STEP_MS));
> +	fall = min_t(unsigned int, KTD202X_RAMP_MAX_STEPS,
> +		     DIV_ROUND_CLOSEST(t[2], KTD202X_RAMP_STEP_MS));
> +
> +	/* Round the period up so that the fall ramp is not cut short */
> +	period_ms = t[0] + t[1] + t[2] + t[3];
> +	num_steps = ktd202x_flash_period_steps(info, period_ms +
> +					       KTD202X_FLASH_PERIOD_STEP_MS - 1);
> +	period_ms = ktd202x_flash_period_ms(info, num_steps);
> +
> +	/* The on time includes the rise time */
> +	ton_ms = t[0] + t[1];
> +	on = clamp_t(unsigned long,
> +		     DIV_ROUND_CLOSEST(ton_ms * KTD202X_FLASH_ON_MAX, period_ms),
> +		     1, KTD202X_FLASH_ON_MAX - 1);
> +
> +	mutex_lock(&led->chip->mutex);
> +
> +	ret = regmap_write(led->chip->regmap, KTD202X_REG_FLASH_PERIOD, num_steps);
> +	if (ret)
> +		goto out;
> +
> +	ret = regmap_write(led->chip->regmap, KTD202X_REG_PWM1_TIMER, on);
> +	if (ret)
> +		goto out;
> +
> +	ret = regmap_write(led->chip->regmap, KTD202X_REG_TRISE_FALL,
> +			   KTD202X_RAMP_TFALL(fall) | KTD202X_RAMP_TRISE(rise));
> +	if (ret)
> +		goto out;
> +
> +	ret = regmap_update_bits(led->chip->regmap, KTD202X_REG_CHANNEL_CTRL,
> +				 ctrl_mask, ctrl_pwm1);
> +out:
> +	mutex_unlock(&led->chip->mutex);
> +	return ret;
> +}
> +
> +static int ktd202x_pattern_single_set(struct led_classdev *cdev,
> +				      struct led_pattern *pattern, u32 len,
> +				      int repeat)
> +{
> +	struct ktd202x_led *led = container_of(cdev, struct ktd202x_led, cdev);
> +	struct mc_subled info;
> +	int ret;
> +
> +	ret = ktd202x_pattern_check(cdev, pattern, len, repeat);
> +	if (ret)
> +		return ret;
> +
> +	ret = ktd202x_brightness_single_set(cdev, pattern[1].brightness);
> +	if (ret)
> +		return ret;
> +
> +	info.channel = led->index;
> +
> +	return ktd202x_pattern_program(led, pattern, &info, 1);
> +}
> +
> +static int ktd202x_pattern_single_clear(struct led_classdev *cdev)
> +{
> +	return ktd202x_brightness_single_set(cdev, LED_OFF);
> +}
> +
> +static int ktd202x_pattern_mc_set(struct led_classdev *cdev,
> +				  struct led_pattern *pattern, u32 len,
> +				  int repeat)
> +{
> +	struct led_classdev_mc *mc = lcdev_to_mccdev(cdev);
> +	struct ktd202x_led *led = container_of(mc, struct ktd202x_led, mcdev);
> +	int ret;
> +
> +	ret = ktd202x_pattern_check(cdev, pattern, len, repeat);
> +	if (ret)
> +		return ret;
> +
> +	ret = ktd202x_brightness_mc_set(cdev, pattern[1].brightness);
> +	if (ret)
> +		return ret;
> +
> +	return ktd202x_pattern_program(led, pattern, mc->subled_info,
> +				       mc->num_colors);
> +}

[Severity: Medium]
When a hardware pattern is set on a multicolor LED where one or more color
channels have a computed brightness of 0 (for example, via multi_intensity),
won't ktd202x_pattern_program() turn those 0-brightness channels on at their
previous or reset-default (10 mA) current?

In ktd202x_pattern_mc_set(), the call to ktd202x_brightness_mc_set() computes
per-channel brightness into mc->subled_info[i].brightness and calls
ktd202x_brightness_set():

drivers/leds/rgb/leds-ktd202x.c:ktd202x_brightness_set() {
	...
	for (i = 0; i < num_channels; i++) {
		enum led_brightness brightness;
		int mode;

		brightness = subleds[i].brightness;
		channel = subleds[i].channel;

		if (brightness) {
			/* Register expects brightness between 0 and MAX_BRIGHTNESS - 1 */
			ret = regmap_write(led->chip->regmap, KTD202X_REG_LED_IOUT(channel),
					   brightness - 1);
			...
		} else {
			mode = KTD202X_CHANNEL_CTRL_OFF;
		}
		ret = regmap_update_bits(led->chip->regmap, KTD202X_REG_CHANNEL_CTRL,
					 KTD202X_CHANNEL_CTRL_MASK(channel), mode);
		...
	}
}

Because KTD202X_REG_LED_IOUT(channel) encodes currents starting at 0.125 mA
(brightness - 1) and cannot encode 0 mA, channels with brightness == 0 do not
update KTD202X_REG_LED_IOUT(channel) and instead rely on
KTD202X_CHANNEL_CTRL_OFF in KTD202X_REG_CHANNEL_CTRL to remain off.

However, ktd202x_pattern_program() then loops over all num_channels and
unconditionally sets KTD202X_CHANNEL_CTRL_PWM1(channel) in ctrl_pwm1 for every
channel regardless of subleds[i].brightness, overwriting
KTD202X_REG_CHANNEL_CTRL so that the 0-brightness channels also breathe on
PWM1.

Also note that ktd202x_pattern_single_set() leaves info.brightness
uninitialized on the stack before passing &info to ktd202x_pattern_program(),
and ktd202x_pattern_program() builds ctrl_pwm1 before acquiring
led->chip->mutex (after ktd202x_brightness_mc_set() has already dropped it).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261101-ktd202x-et6326-v1-0-eb51eddad08c@telaak.dev?part=7

[PATCH 08/10] leds: rgb: ktd202x: Limit the LED current to led-max-microamp

[Joseph Telaak via B4 Relay]

From: Joseph Telaak <joseph@telaak.dev>

The full brightness of 192 is 24 mA, which is more than many indicator
LEDs are made for, and a channel may drive several LEDs in parallel. The
GL.iNet GL-SFT1200 firmware, for example, runs each of its two channels,
three LEDs each, at 1.375 mA.

Honor the common led-max-microamp property for single color LEDs by
lowering max_brightness, since each brightness step adds 0.125 mA. Set
max_brightness before the LED is registered, and blink at max_brightness
rather than at the chip maximum.

Multicolor LEDs are left as they are: the LED core already bounds each
color intensity, and the hardware this was tested on has none.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Joseph Telaak <joseph@telaak.dev>
---
 drivers/leds/rgb/leds-ktd202x.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/leds/rgb/leds-ktd202x.c b/drivers/leds/rgb/leds-ktd202x.c
index ed0e8e74c..48521f1e5 100644
--- a/drivers/leds/rgb/leds-ktd202x.c
+++ b/drivers/leds/rgb/leds-ktd202x.c
@@ -78,6 +78,7 @@
 #define KTD202X_PATTERN_MAX_MS 16384
 
 #define KTD202X_MAX_BRIGHTNESS 192
+#define KTD202X_LED_CURRENT_STEP_uA 125
 
 static const struct reg_default ktd202x_reg_defaults[] = {
 	{ KTD202X_REG_RESET_CONTROL, KTD202X_TIMER_SLOT_CONTROL_TSLOT1 |
@@ -398,7 +399,7 @@ static int ktd202x_blink_single_set(struct led_classdev *cdev,
 	int ret;
 
 	if (!cdev->brightness) {
-		ret = ktd202x_brightness_single_set(cdev, KTD202X_MAX_BRIGHTNESS);
+		ret = ktd202x_brightness_single_set(cdev, cdev->max_brightness);
 		if (ret)
 			return ret;
 	}
@@ -588,6 +589,18 @@ static int ktd202x_pattern_mc_clear(struct led_classdev *cdev)
 	return ktd202x_brightness_mc_set(cdev, LED_OFF);
 }
 
+/* The current limit of a LED in brightness steps, each of which adds 0.125 mA */
+static unsigned int ktd202x_max_brightness(struct fwnode_handle *fwnode)
+{
+	u32 max_microamp;
+
+	if (fwnode_property_read_u32(fwnode, "led-max-microamp", &max_microamp))
+		return KTD202X_MAX_BRIGHTNESS;
+
+	return clamp_t(u32, max_microamp / KTD202X_LED_CURRENT_STEP_uA, 1,
+		       KTD202X_MAX_BRIGHTNESS);
+}
+
 static int ktd202x_setup_led_rgb(struct ktd202x *chip, struct fwnode_handle *fwnode,
 				 struct ktd202x_led *led, struct led_init_data *init_data)
 {
@@ -637,6 +650,7 @@ static int ktd202x_setup_led_rgb(struct ktd202x *chip, struct fwnode_handle *fwn
 	led->mcdev.num_colors = num_channels;
 
 	cdev = &led->mcdev.led_cdev;
+	cdev->max_brightness = KTD202X_MAX_BRIGHTNESS;
 	cdev->brightness_set_blocking = ktd202x_brightness_mc_set;
 	cdev->blink_set = ktd202x_blink_mc_set;
 	cdev->pattern_set = ktd202x_pattern_mc_set;
@@ -660,6 +674,8 @@ static int ktd202x_setup_led_single(struct ktd202x *chip, struct fwnode_handle *
 	led->index = reg;
 
 	cdev = &led->cdev;
+	cdev->max_brightness = ktd202x_max_brightness(fwnode);
+
 	cdev->brightness_set_blocking = ktd202x_brightness_single_set;
 	cdev->blink_set = ktd202x_blink_single_set;
 	cdev->pattern_set = ktd202x_pattern_single_set;
@@ -699,8 +715,6 @@ static int ktd202x_add_led(struct ktd202x *chip, struct fwnode_handle *fwnode, u
 		return ret;
 	}
 
-	cdev->max_brightness = KTD202X_MAX_BRIGHTNESS;
-
 	return 0;
 }
 

-- 
Git-155)

[PATCH 08/10] leds: rgb: ktd202x: Limit the LED current to led-max-microamp

[Joseph Telaak]

The full brightness of 192 is 24 mA, which is more than many indicator
LEDs are made for, and a channel may drive several LEDs in parallel. The
GL.iNet GL-SFT1200 firmware, for example, runs each of its two channels,
three LEDs each, at 1.375 mA.

Honor the common led-max-microamp property for single color LEDs by
lowering max_brightness, since each brightness step adds 0.125 mA. Set
max_brightness before the LED is registered, and blink at max_brightness
rather than at the chip maximum.

Multicolor LEDs are left as they are: the LED core already bounds each
color intensity, and the hardware this was tested on has none.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Joseph Telaak <joseph@telaak.dev>
---
 drivers/leds/rgb/leds-ktd202x.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/leds/rgb/leds-ktd202x.c b/drivers/leds/rgb/leds-ktd202x.c
index ed0e8e74c..48521f1e5 100644
--- a/drivers/leds/rgb/leds-ktd202x.c
+++ b/drivers/leds/rgb/leds-ktd202x.c
@@ -78,6 +78,7 @@
 #define KTD202X_PATTERN_MAX_MS 16384
 
 #define KTD202X_MAX_BRIGHTNESS 192
+#define KTD202X_LED_CURRENT_STEP_uA 125
 
 static const struct reg_default ktd202x_reg_defaults[] = {
 	{ KTD202X_REG_RESET_CONTROL, KTD202X_TIMER_SLOT_CONTROL_TSLOT1 |
@@ -398,7 +399,7 @@ static int ktd202x_blink_single_set(struct led_classdev *cdev,
 	int ret;
 
 	if (!cdev->brightness) {
-		ret = ktd202x_brightness_single_set(cdev, KTD202X_MAX_BRIGHTNESS);
+		ret = ktd202x_brightness_single_set(cdev, cdev->max_brightness);
 		if (ret)
 			return ret;
 	}
@@ -588,6 +589,18 @@ static int ktd202x_pattern_mc_clear(struct led_classdev *cdev)
 	return ktd202x_brightness_mc_set(cdev, LED_OFF);
 }
 
+/* The current limit of a LED in brightness steps, each of which adds 0.125 mA */
+static unsigned int ktd202x_max_brightness(struct fwnode_handle *fwnode)
+{
+	u32 max_microamp;
+
+	if (fwnode_property_read_u32(fwnode, "led-max-microamp", &max_microamp))
+		return KTD202X_MAX_BRIGHTNESS;
+
+	return clamp_t(u32, max_microamp / KTD202X_LED_CURRENT_STEP_uA, 1,
+		       KTD202X_MAX_BRIGHTNESS);
+}
+
 static int ktd202x_setup_led_rgb(struct ktd202x *chip, struct fwnode_handle *fwnode,
 				 struct ktd202x_led *led, struct led_init_data *init_data)
 {
@@ -637,6 +650,7 @@ static int ktd202x_setup_led_rgb(struct ktd202x *chip, struct fwnode_handle *fwn
 	led->mcdev.num_colors = num_channels;
 
 	cdev = &led->mcdev.led_cdev;
+	cdev->max_brightness = KTD202X_MAX_BRIGHTNESS;
 	cdev->brightness_set_blocking = ktd202x_brightness_mc_set;
 	cdev->blink_set = ktd202x_blink_mc_set;
 	cdev->pattern_set = ktd202x_pattern_mc_set;
@@ -660,6 +674,8 @@ static int ktd202x_setup_led_single(struct ktd202x *chip, struct fwnode_handle *
 	led->index = reg;
 
 	cdev = &led->cdev;
+	cdev->max_brightness = ktd202x_max_brightness(fwnode);
+
 	cdev->brightness_set_blocking = ktd202x_brightness_single_set;
 	cdev->blink_set = ktd202x_blink_single_set;
 	cdev->pattern_set = ktd202x_pattern_single_set;
@@ -699,8 +715,6 @@ static int ktd202x_add_led(struct ktd202x *chip, struct fwnode_handle *fwnode, u
 		return ret;
 	}
 
-	cdev->max_brightness = KTD202X_MAX_BRIGHTNESS;
-
 	return 0;
 }
 

-- 
Git-155)

Re: [PATCH 08/10] leds: rgb: ktd202x: Limit the LED current to led-max-microamp

[sashiko-bot]

> From: Joseph Telaak <joseph@telaak.dev>
> 
> The full brightness of 192 is 24 mA, which is more than many indicator
> LEDs are made for, and a channel may drive several LEDs in parallel. The
> GL.iNet GL-SFT1200 firmware, for example, runs each of its two channels,
> three LEDs each, at 1.375 mA.
> 
> Honor the common led-max-microamp property for single color LEDs by
> lowering max_brightness, since each brightness step adds 0.125 mA. Set
> max_brightness before the LED is registered, and blink at max_brightness
> rather than at the chip maximum.
> 
> Multicolor LEDs are left as they are: the LED core already bounds each
> color intensity, and the hardware this was tested on has none.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Joseph Telaak <joseph@telaak.dev>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261101-ktd202x-et6326-v1-0-eb51eddad08c@telaak.dev?part=8

[PATCH 09/10] leds: rgb: ktd202x: Fail the probe when the chip does not answer

[Joseph Telaak via B4 Relay]

From: Joseph Telaak <joseph@telaak.dev>

The driver never reads from the chip: the register cache starts from the
defaults. So with a wrong address, or no chip at all, the reset write at
probe NAKs, the error is ignored and the LEDs get registered anyway. Every
later write then fails.

Fail the probe when the reset write fails.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Joseph Telaak <joseph@telaak.dev>
---
 drivers/leds/rgb/leds-ktd202x.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/leds/rgb/leds-ktd202x.c b/drivers/leds/rgb/leds-ktd202x.c
index 48521f1e5..5f51df993 100644
--- a/drivers/leds/rgb/leds-ktd202x.c
+++ b/drivers/leds/rgb/leds-ktd202x.c
@@ -722,20 +722,23 @@ static int ktd202x_probe_fw(struct ktd202x *chip)
 {
 	struct device *dev = chip->dev;
 	int count;
+	int ret;
 	int i = 0;
 
 	count = device_get_child_node_count(dev);
 	if (!count || count > chip->num_leds)
 		return -EINVAL;
 
-	regmap_write(chip->regmap, KTD202X_REG_RESET_CONTROL, KTD202X_RSTR_RESET);
+	/* Nothing is read back, so a failed reset is the only sign of a missing chip */
+	ret = regmap_write(chip->regmap, KTD202X_REG_RESET_CONTROL, KTD202X_RSTR_RESET);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to reset the chip\n");
 
 	/* Allow the device to execute the complete reset */
 	usleep_range(200, 300);
 
 	device_for_each_child_node_scoped(dev, child) {
-		int ret = ktd202x_add_led(chip, child, i);
-
+		ret = ktd202x_add_led(chip, child, i);
 		if (ret)
 			return ret;
 

-- 
Git-155)

[PATCH 09/10] leds: rgb: ktd202x: Fail the probe when the chip does not answer

[Joseph Telaak]

The driver never reads from the chip: the register cache starts from the
defaults. So with a wrong address, or no chip at all, the reset write at
probe NAKs, the error is ignored and the LEDs get registered anyway. Every
later write then fails.

Fail the probe when the reset write fails.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Joseph Telaak <joseph@telaak.dev>
---
 drivers/leds/rgb/leds-ktd202x.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/leds/rgb/leds-ktd202x.c b/drivers/leds/rgb/leds-ktd202x.c
index 48521f1e5..5f51df993 100644
--- a/drivers/leds/rgb/leds-ktd202x.c
+++ b/drivers/leds/rgb/leds-ktd202x.c
@@ -722,20 +722,23 @@ static int ktd202x_probe_fw(struct ktd202x *chip)
 {
 	struct device *dev = chip->dev;
 	int count;
+	int ret;
 	int i = 0;
 
 	count = device_get_child_node_count(dev);
 	if (!count || count > chip->num_leds)
 		return -EINVAL;
 
-	regmap_write(chip->regmap, KTD202X_REG_RESET_CONTROL, KTD202X_RSTR_RESET);
+	/* Nothing is read back, so a failed reset is the only sign of a missing chip */
+	ret = regmap_write(chip->regmap, KTD202X_REG_RESET_CONTROL, KTD202X_RSTR_RESET);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to reset the chip\n");
 
 	/* Allow the device to execute the complete reset */
 	usleep_range(200, 300);
 
 	device_for_each_child_node_scoped(dev, child) {
-		int ret = ktd202x_add_led(chip, child, i);
-
+		ret = ktd202x_add_led(chip, child, i);
 		if (ret)
 			return ret;
 

-- 
Git-155)

Re: [PATCH 09/10] leds: rgb: ktd202x: Fail the probe when the chip does not answer

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] leds: rgb: ktd202x: failing probe on reset write breaks KTD2026/KTD2027
--

commit cbade43891d8603172b94d1364ff11d464697a37
Author: Joseph Telaak <joseph@telaak.dev>

leds: rgb: ktd202x: Fail the probe when the chip does not answer

This commit checks the return value of the reset register write in
ktd202x_probe_fw() and fails probing if the I2C write fails.

> diff --git a/drivers/leds/rgb/leds-ktd202x.c b/drivers/leds/rgb/leds-ktd202x.c
> index 48521f1e5a704..5f51df9930cb2 100644
> --- a/drivers/leds/rgb/leds-ktd202x.c
> +++ b/drivers/leds/rgb/leds-ktd202x.c
> @@ -722,20 +722,23 @@ static int ktd202x_probe_fw(struct ktd202x *chip)
>  {
>  	struct device *dev = chip->dev;
>  	int count;
> +	int ret;
>  	int i = 0;
>  
>  	count = device_get_child_node_count(dev);
>  	if (!count || count > chip->num_leds)
>  		return -EINVAL;
>  
> -	regmap_write(chip->regmap, KTD202X_REG_RESET_CONTROL, KTD202X_RSTR_RESET);
> +	/* Nothing is read back, so a failed reset is the only sign of a missing chip */
> +	ret = regmap_write(chip->regmap, KTD202X_REG_RESET_CONTROL, KTD202X_RSTR_RESET);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to reset the chip\n");

[Severity: High]
Will failing ktd202x_probe_fw() when the KTD202X_RSTR_RESET write returns an
error break probing on genuine Kinetic KTD2026 and KTD2027 hardware?

Per the Kinetic KTD2026/KTD2027 datasheet (Register 0 Reset/Control bits
[2:0] = 111, Reset Complete Chip), executing the reset command resets the
entire chip including its I2C state machine immediately upon latching the
data byte, so the chip does not return an I2C ACK pulse for the reset
command byte and requires a 200 us wait before the next I2C transfer.

Because the KTD2026/KTD2027 NACKs the KTD202X_RSTR_RESET write by design,
regmap_write() returns a NACK error (-ENXIO or -EREMOTEIO) during
ktd202x_probe() -> ktd202x_probe_fw(), which is why commit 0ebdb7210943
("leds: Add ktd202x driver") ignored the return value of the reset write.

To detect a missing chip at probe time without regressing KTD2026/KTD2027,
could the driver instead check the return value of a register write issued
after usleep_range(200, 300) completes (for example, writing
KTD202X_ENABLE_CTRL_SLEEP to KTD202X_REG_RESET_CONTROL after the reset
delay)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261101-ktd202x-et6326-v1-0-eb51eddad08c@telaak.dev?part=9

Re: [PATCH 09/10] leds: rgb: ktd202x: Fail the probe when the chip does not answer

[Joseph Telaak]

On Sun, Nov 1, 2026 at 9:22 PM <sashiko-bot@kernel.org> wrote:
> - regmap_write(chip->regmap, KTD202X_REG_RESET_CONTROL, KTD202X_RSTR_RESET);
> + /* Nothing is read back, so a failed reset is the only sign of a missing chip */
> + ret = regmap_write(chip->regmap, KTD202X_REG_RESET_CONTROL, KTD202X_RSTR_RESET);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to reset the chip\n");

Please don't apply this one as is. As Sashiko pointed out, the KTD2026/7
does not ACK the reset command: its data sheet says the byte that resets
the chip is followed by a "not acknowledge", which is to be ignored. The
ET6326 I tested on does ACK it, so I missed that this breaks the probe on
every KTD202x.

v2 will ignore the reset write again, and fail the probe only when the
write that puts the chip to sleep after the reset delay fails.

Joseph

[PATCH 10/10] leds: rgb: ktd202x: Don't let a blinking LED retime another one

[Joseph Telaak via B4 Relay]

From: Joseph Telaak <joseph@telaak.dev>

Every LED blinks with the PWM1 flash timer. So when a second LED blinks,
its period and on time silently replace those of the first one, while
the sysfs of the first one still shows its old delays. The same happens
between a blink and a hardware pattern, and the ramps a pattern sets
leak into a later blink.

The PWM2 timer can't take the second LED: in timer slot 1, which the
driver uses, it repeats PWM1 (Reg3 has no effect, measured on an
ET6326), and in the other slots it only runs after PWM1 within the same
period.

Track which LEDs are on the timer instead of reading the channel state
back. A LED may join the timer while no other LED uses it, or when it
asks for the very same period, on time and ramps. Otherwise it stays on
and gets -EBUSY, which makes the LED core blink it in software. Only
write the timer registers when they change, so that a LED joining the
timer does not restart the blink of the others.

The match is on the programmed values. The timer trigger writes delay_on
and delay_off one at a time, and each write is rounded against the other's
rounded value, so two LEDs set to the same delays can get on times a few
steps apart. The second one then blinks in software, at the timing it
asked for. The documentation says how to make them share.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Joseph Telaak <joseph@telaak.dev>
---
 Documentation/leds/leds-ktd202x.rst |  13 ++-
 drivers/leds/rgb/leds-ktd202x.c     | 213 ++++++++++++++++++++++--------------
 2 files changed, 143 insertions(+), 83 deletions(-)

diff --git a/Documentation/leds/leds-ktd202x.rst b/Documentation/leds/leds-ktd202x.rst
index 2ee537f57..32ea9d6f6 100644
--- a/Documentation/leds/leds-ktd202x.rst
+++ b/Documentation/leds/leds-ktd202x.rst
@@ -31,9 +31,16 @@ equal and not 0. ``repeat`` must be -1, its default.
 The sum of the four times is the flash period. The driver rounds it up to the
 next step of 128 ms, between 256 ms (128 ms on the ET6326) and 16384 ms.
 
-The flash period, the on time and the ramp times are shared by all LEDs of a
-chip. When several LEDs blink or breathe at once, they all follow the timing
-that was set last.
+All LEDs of a chip blink and breathe with one flash timer. Several LEDs can
+use it at once only with the same period, on time and ramps. A LED that asks
+for different ones while another LED is on the timer gets -EBUSY: blinking
+then falls back to the software timer, and ``hw_pattern`` returns the error.
+
+The match is on the values programmed into the chip. The timer trigger sets
+``delay_on`` and ``delay_off`` one at a time and the driver rounds each pair,
+so two LEDs given the same delays can end up with slightly different on times,
+and then the second one blinks in software. To share the timer, give the
+second LED the delays that the first one reads back.
 
 For example, to breathe with a period of 4 seconds::
 
diff --git a/drivers/leds/rgb/leds-ktd202x.c b/drivers/leds/rgb/leds-ktd202x.c
index 5f51df993..f4544ba33 100644
--- a/drivers/leds/rgb/leds-ktd202x.c
+++ b/drivers/leds/rgb/leds-ktd202x.c
@@ -128,6 +128,8 @@ struct ktd202x_led {
 		struct led_classdev_mc mcdev;
 	};
 	u32 index;
+	/* Blinking or breathing with the flash timer */
+	bool on_timer;
 };
 
 struct ktd202x {
@@ -137,6 +139,10 @@ struct ktd202x {
 	struct regmap *regmap;
 	const struct ktd202x_info *info;
 	bool enabled;
+	/* Settings of the flash timer, valid while a LED is on it */
+	u8 flash_period;
+	u8 flash_on;
+	u8 ramp_times;
 	unsigned long num_leds;
 	struct ktd202x_led leds[] __counted_by(num_leds);
 };
@@ -196,13 +202,20 @@ static bool ktd202x_chip_in_use(struct ktd202x *chip)
 	return false;
 }
 
+/* The channel mode of a lit LED: on, or blinking with the flash timer */
+static unsigned int ktd202x_channel_mode(struct ktd202x_led *led, int channel)
+{
+	if (led->on_timer)
+		return KTD202X_CHANNEL_CTRL_PWM1(channel);
+
+	return KTD202X_CHANNEL_CTRL_ON(channel);
+}
+
 static int ktd202x_brightness_set(struct ktd202x_led *led,
 				  struct mc_subled *subleds,
 				  unsigned int num_channels)
 {
-	bool mode_blink = false;
 	int channel;
-	int state;
 	int ret;
 	int i;
 
@@ -212,24 +225,9 @@ static int ktd202x_brightness_set(struct ktd202x_led *led,
 			return ret;
 	}
 
-	ret = regmap_read(led->chip->regmap, KTD202X_REG_CHANNEL_CTRL, &state);
-	if (ret)
-		return ret;
-
-	/*
-	 * In multicolor case, assume blink mode if PWM is set for at least one
-	 * channel because another channel cannot be in state ON at the same time
-	 */
-	for (i = 0; i < num_channels; i++) {
-		int channel_state;
-
-		channel = subleds[i].channel;
-		channel_state = (state >> 2 * channel) & KTD202X_CHANNEL_CTRL_MASK(0);
-		if (channel_state == KTD202X_CHANNEL_CTRL_OFF)
-			continue;
-		mode_blink = channel_state == KTD202X_CHANNEL_CTRL_PWM1(0);
-		break;
-	}
+	/* A LED that is turned off stops blinking and leaves the timer */
+	if (!led->cdev.brightness)
+		led->on_timer = false;
 
 	for (i = 0; i < num_channels; i++) {
 		enum led_brightness brightness;
@@ -245,10 +243,7 @@ static int ktd202x_brightness_set(struct ktd202x_led *led,
 			if (ret)
 				return ret;
 
-			if (mode_blink)
-				mode = KTD202X_CHANNEL_CTRL_PWM1(channel);
-			else
-				mode = KTD202X_CHANNEL_CTRL_ON(channel);
+			mode = ktd202x_channel_mode(led, channel);
 		} else {
 			mode = KTD202X_CHANNEL_CTRL_OFF;
 		}
@@ -320,6 +315,89 @@ static unsigned long ktd202x_flash_period_ms(const struct ktd202x_info *info,
 	return num_steps * KTD202X_FLASH_PERIOD_STEP_MS + info->flash_period_min_ms;
 }
 
+/*
+ * All LEDs blink with the PWM1 flash timer. PWM2 is no help: in timer slot 1
+ * it repeats PWM1, and in the other slots it only runs after PWM1 within the
+ * same period. So a LED may only join the timer while it is unused, or used
+ * with the very same period, on time and ramps, so that it never retimes
+ * another LED behind its back.
+ */
+static bool ktd202x_timer_busy(struct ktd202x_led *led, u8 flash_period,
+			       u8 flash_on, u8 ramp_times)
+{
+	struct ktd202x *chip = led->chip;
+	int i;
+
+	for (i = 0; i < chip->num_leds; i++) {
+		struct ktd202x_led *other = &chip->leds[i];
+
+		if (other == led || !other->on_timer)
+			continue;
+
+		if (flash_period != chip->flash_period ||
+		    flash_on != chip->flash_on || ramp_times != chip->ramp_times)
+			return true;
+	}
+
+	return false;
+}
+
+/* Called with the chip mutex held */
+static int ktd202x_timer_set(struct ktd202x_led *led, struct mc_subled *subleds,
+			     unsigned int num_channels, u8 flash_period, u8 flash_on,
+			     u8 ramp_times)
+{
+	struct ktd202x *chip = led->chip;
+	u8 ctrl_mask = 0;
+	u8 ctrl_mode = 0;
+	bool busy;
+	int ret;
+	int i;
+
+	busy = ktd202x_timer_busy(led, flash_period, flash_on, ramp_times);
+	led->on_timer = !busy;
+
+	for (i = 0; i < num_channels; i++) {
+		int channel = subleds[i].channel;
+
+		ctrl_mask |= KTD202X_CHANNEL_CTRL_MASK(channel);
+		ctrl_mode |= ktd202x_channel_mode(led, channel);
+	}
+
+	/* Without the timer, leave the LED steadily on */
+	if (busy) {
+		ret = regmap_update_bits(chip->regmap, KTD202X_REG_CHANNEL_CTRL,
+					 ctrl_mask, ctrl_mode);
+		return ret ? ret : -EBUSY;
+	}
+
+	chip->flash_period = flash_period;
+	chip->flash_on = flash_on;
+	chip->ramp_times = ramp_times;
+
+	/*
+	 * Update the timer registers only when they change, so that the
+	 * blink of another LED on the timer is not restarted.
+	 */
+	ret = regmap_update_bits(chip->regmap, KTD202X_REG_FLASH_PERIOD, 0xff,
+				 flash_period);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(chip->regmap, KTD202X_REG_PWM1_TIMER, 0xff,
+				 flash_on);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(chip->regmap, KTD202X_REG_TRISE_FALL, 0xff,
+				 ramp_times);
+	if (ret)
+		return ret;
+
+	return regmap_update_bits(chip->regmap, KTD202X_REG_CHANNEL_CTRL,
+				  ctrl_mask, ctrl_mode);
+}
+
 static int ktd202x_blink_set(struct ktd202x_led *led, unsigned long *delay_on,
 			     unsigned long *delay_off, struct mc_subled *subleds,
 			     unsigned int num_channels)
@@ -328,22 +406,21 @@ static int ktd202x_blink_set(struct ktd202x_led *led, unsigned long *delay_on,
 	unsigned long delay_total_ms;
 	int ret, num_steps, on;
 	u8 ctrl_mask = 0;
-	u8 ctrl_pwm1 = 0;
 	u8 ctrl_on = 0;
 	int i;
 
 	mutex_lock(&led->chip->mutex);
 
-	for (i = 0; i < num_channels; i++) {
-		int channel = subleds[i].channel;
-
-		ctrl_mask |= KTD202X_CHANNEL_CTRL_MASK(channel);
-		ctrl_on |= KTD202X_CHANNEL_CTRL_ON(channel);
-		ctrl_pwm1 |= KTD202X_CHANNEL_CTRL_PWM1(channel);
-	}
-
 	/* Never off - brightness is already set, disable blinking */
 	if (!*delay_off) {
+		for (i = 0; i < num_channels; i++) {
+			int channel = subleds[i].channel;
+
+			ctrl_mask |= KTD202X_CHANNEL_CTRL_MASK(channel);
+			ctrl_on |= KTD202X_CHANNEL_CTRL_ON(channel);
+		}
+
+		led->on_timer = false;
 		ret = regmap_update_bits(led->chip->regmap, KTD202X_REG_CHANNEL_CTRL,
 					 ctrl_mask, ctrl_on);
 		goto out;
@@ -363,28 +440,19 @@ static int ktd202x_blink_set(struct ktd202x_led *led, unsigned long *delay_on,
 	on = mul_u64_u64_div_u64(*delay_on, KTD202X_FLASH_ON_MAX, delay_total_ms);
 	on = clamp(on, 1, KTD202X_FLASH_ON_MAX - 1);
 
+	/*
+	 * Without the ramps a hardware pattern may have set. On -EBUSY the
+	 * LED core falls back to software blinking.
+	 */
+	ret = ktd202x_timer_set(led, subleds, num_channels, num_steps, on,
+				KTD202X_RAMP_TIMES_2_MS);
+	if (ret)
+		goto out;
+
 	/* Actually used delay_{on,off} values */
 	delay_total_ms = ktd202x_flash_period_ms(info, num_steps);
 	*delay_on = (delay_total_ms * on) / KTD202X_FLASH_ON_MAX;
 	*delay_off = delay_total_ms - *delay_on;
-
-	/* Set timings */
-	ret = regmap_write(led->chip->regmap, KTD202X_REG_FLASH_PERIOD, num_steps);
-	if (ret)
-		goto out;
-
-	ret = regmap_write(led->chip->regmap, KTD202X_REG_PWM1_TIMER, on);
-	if (ret)
-		goto out;
-
-	/* Drop the ramps a hardware pattern may have left behind */
-	ret = regmap_write(led->chip->regmap, KTD202X_REG_TRISE_FALL,
-			   KTD202X_RAMP_TIMES_2_MS);
-	if (ret)
-		goto out;
-
-	ret = regmap_update_bits(led->chip->regmap, KTD202X_REG_CHANNEL_CTRL,
-				 ctrl_mask, ctrl_pwm1);
 out:
 	mutex_unlock(&led->chip->mutex);
 	return ret;
@@ -484,18 +552,9 @@ static int ktd202x_pattern_program(struct ktd202x_led *led,
 	unsigned int t[KTD202X_PATTERN_LEN];
 	unsigned int rise, fall, num_steps, on;
 	unsigned long period_ms, ton_ms;
-	u8 ctrl_mask = 0;
-	u8 ctrl_pwm1 = 0;
 	int ret;
 	int i;
 
-	for (i = 0; i < num_channels; i++) {
-		int channel = subleds[i].channel;
-
-		ctrl_mask |= KTD202X_CHANNEL_CTRL_MASK(channel);
-		ctrl_pwm1 |= KTD202X_CHANNEL_CTRL_PWM1(channel);
-	}
-
 	for (i = 0; i < KTD202X_PATTERN_LEN; i++)
 		t[i] = min_t(u32, pattern[i].delta_t, KTD202X_PATTERN_MAX_MS);
 
@@ -517,24 +576,10 @@ static int ktd202x_pattern_program(struct ktd202x_led *led,
 		     1, KTD202X_FLASH_ON_MAX - 1);
 
 	mutex_lock(&led->chip->mutex);
-
-	ret = regmap_write(led->chip->regmap, KTD202X_REG_FLASH_PERIOD, num_steps);
-	if (ret)
-		goto out;
-
-	ret = regmap_write(led->chip->regmap, KTD202X_REG_PWM1_TIMER, on);
-	if (ret)
-		goto out;
-
-	ret = regmap_write(led->chip->regmap, KTD202X_REG_TRISE_FALL,
-			   KTD202X_RAMP_TFALL(fall) | KTD202X_RAMP_TRISE(rise));
-	if (ret)
-		goto out;
-
-	ret = regmap_update_bits(led->chip->regmap, KTD202X_REG_CHANNEL_CTRL,
-				 ctrl_mask, ctrl_pwm1);
-out:
+	ret = ktd202x_timer_set(led, subleds, num_channels, num_steps, on,
+				KTD202X_RAMP_TFALL(fall) | KTD202X_RAMP_TRISE(rise));
 	mutex_unlock(&led->chip->mutex);
+
 	return ret;
 }
 
@@ -556,7 +601,11 @@ static int ktd202x_pattern_single_set(struct led_classdev *cdev,
 
 	info.channel = led->index;
 
-	return ktd202x_pattern_program(led, pattern, &info, 1);
+	ret = ktd202x_pattern_program(led, pattern, &info, 1);
+	if (ret)
+		ktd202x_brightness_single_set(cdev, LED_OFF);
+
+	return ret;
 }
 
 static int ktd202x_pattern_single_clear(struct led_classdev *cdev)
@@ -580,8 +629,12 @@ static int ktd202x_pattern_mc_set(struct led_classdev *cdev,
 	if (ret)
 		return ret;
 
-	return ktd202x_pattern_program(led, pattern, mc->subled_info,
-				       mc->num_colors);
+	ret = ktd202x_pattern_program(led, pattern, mc->subled_info,
+				      mc->num_colors);
+	if (ret)
+		ktd202x_brightness_mc_set(cdev, LED_OFF);
+
+	return ret;
 }
 
 static int ktd202x_pattern_mc_clear(struct led_classdev *cdev)

-- 
Git-155)

[PATCH 10/10] leds: rgb: ktd202x: Don't let a blinking LED retime another one

[Joseph Telaak]

Every LED blinks with the PWM1 flash timer. So when a second LED blinks,
its period and on time silently replace those of the first one, while
the sysfs of the first one still shows its old delays. The same happens
between a blink and a hardware pattern, and the ramps a pattern sets
leak into a later blink.

The PWM2 timer can't take the second LED: in timer slot 1, which the
driver uses, it repeats PWM1 (Reg3 has no effect, measured on an
ET6326), and in the other slots it only runs after PWM1 within the same
period.

Track which LEDs are on the timer instead of reading the channel state
back. A LED may join the timer while no other LED uses it, or when it
asks for the very same period, on time and ramps. Otherwise it stays on
and gets -EBUSY, which makes the LED core blink it in software. Only
write the timer registers when they change, so that a LED joining the
timer does not restart the blink of the others.

The match is on the programmed values. The timer trigger writes delay_on
and delay_off one at a time, and each write is rounded against the other's
rounded value, so two LEDs set to the same delays can get on times a few
steps apart. The second one then blinks in software, at the timing it
asked for. The documentation says how to make them share.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Joseph Telaak <joseph@telaak.dev>
---
 Documentation/leds/leds-ktd202x.rst |  13 ++-
 drivers/leds/rgb/leds-ktd202x.c     | 213 ++++++++++++++++++++++--------------
 2 files changed, 143 insertions(+), 83 deletions(-)

diff --git a/Documentation/leds/leds-ktd202x.rst b/Documentation/leds/leds-ktd202x.rst
index 2ee537f57..32ea9d6f6 100644
--- a/Documentation/leds/leds-ktd202x.rst
+++ b/Documentation/leds/leds-ktd202x.rst
@@ -31,9 +31,16 @@ equal and not 0. ``repeat`` must be -1, its default.
 The sum of the four times is the flash period. The driver rounds it up to the
 next step of 128 ms, between 256 ms (128 ms on the ET6326) and 16384 ms.
 
-The flash period, the on time and the ramp times are shared by all LEDs of a
-chip. When several LEDs blink or breathe at once, they all follow the timing
-that was set last.
+All LEDs of a chip blink and breathe with one flash timer. Several LEDs can
+use it at once only with the same period, on time and ramps. A LED that asks
+for different ones while another LED is on the timer gets -EBUSY: blinking
+then falls back to the software timer, and ``hw_pattern`` returns the error.
+
+The match is on the values programmed into the chip. The timer trigger sets
+``delay_on`` and ``delay_off`` one at a time and the driver rounds each pair,
+so two LEDs given the same delays can end up with slightly different on times,
+and then the second one blinks in software. To share the timer, give the
+second LED the delays that the first one reads back.
 
 For example, to breathe with a period of 4 seconds::
 
diff --git a/drivers/leds/rgb/leds-ktd202x.c b/drivers/leds/rgb/leds-ktd202x.c
index 5f51df993..f4544ba33 100644
--- a/drivers/leds/rgb/leds-ktd202x.c
+++ b/drivers/leds/rgb/leds-ktd202x.c
@@ -128,6 +128,8 @@ struct ktd202x_led {
 		struct led_classdev_mc mcdev;
 	};
 	u32 index;
+	/* Blinking or breathing with the flash timer */
+	bool on_timer;
 };
 
 struct ktd202x {
@@ -137,6 +139,10 @@ struct ktd202x {
 	struct regmap *regmap;
 	const struct ktd202x_info *info;
 	bool enabled;
+	/* Settings of the flash timer, valid while a LED is on it */
+	u8 flash_period;
+	u8 flash_on;
+	u8 ramp_times;
 	unsigned long num_leds;
 	struct ktd202x_led leds[] __counted_by(num_leds);
 };
@@ -196,13 +202,20 @@ static bool ktd202x_chip_in_use(struct ktd202x *chip)
 	return false;
 }
 
+/* The channel mode of a lit LED: on, or blinking with the flash timer */
+static unsigned int ktd202x_channel_mode(struct ktd202x_led *led, int channel)
+{
+	if (led->on_timer)
+		return KTD202X_CHANNEL_CTRL_PWM1(channel);
+
+	return KTD202X_CHANNEL_CTRL_ON(channel);
+}
+
 static int ktd202x_brightness_set(struct ktd202x_led *led,
 				  struct mc_subled *subleds,
 				  unsigned int num_channels)
 {
-	bool mode_blink = false;
 	int channel;
-	int state;
 	int ret;
 	int i;
 
@@ -212,24 +225,9 @@ static int ktd202x_brightness_set(struct ktd202x_led *led,
 			return ret;
 	}
 
-	ret = regmap_read(led->chip->regmap, KTD202X_REG_CHANNEL_CTRL, &state);
-	if (ret)
-		return ret;
-
-	/*
-	 * In multicolor case, assume blink mode if PWM is set for at least one
-	 * channel because another channel cannot be in state ON at the same time
-	 */
-	for (i = 0; i < num_channels; i++) {
-		int channel_state;
-
-		channel = subleds[i].channel;
-		channel_state = (state >> 2 * channel) & KTD202X_CHANNEL_CTRL_MASK(0);
-		if (channel_state == KTD202X_CHANNEL_CTRL_OFF)
-			continue;
-		mode_blink = channel_state == KTD202X_CHANNEL_CTRL_PWM1(0);
-		break;
-	}
+	/* A LED that is turned off stops blinking and leaves the timer */
+	if (!led->cdev.brightness)
+		led->on_timer = false;
 
 	for (i = 0; i < num_channels; i++) {
 		enum led_brightness brightness;
@@ -245,10 +243,7 @@ static int ktd202x_brightness_set(struct ktd202x_led *led,
 			if (ret)
 				return ret;
 
-			if (mode_blink)
-				mode = KTD202X_CHANNEL_CTRL_PWM1(channel);
-			else
-				mode = KTD202X_CHANNEL_CTRL_ON(channel);
+			mode = ktd202x_channel_mode(led, channel);
 		} else {
 			mode = KTD202X_CHANNEL_CTRL_OFF;
 		}
@@ -320,6 +315,89 @@ static unsigned long ktd202x_flash_period_ms(const struct ktd202x_info *info,
 	return num_steps * KTD202X_FLASH_PERIOD_STEP_MS + info->flash_period_min_ms;
 }
 
+/*
+ * All LEDs blink with the PWM1 flash timer. PWM2 is no help: in timer slot 1
+ * it repeats PWM1, and in the other slots it only runs after PWM1 within the
+ * same period. So a LED may only join the timer while it is unused, or used
+ * with the very same period, on time and ramps, so that it never retimes
+ * another LED behind its back.
+ */
+static bool ktd202x_timer_busy(struct ktd202x_led *led, u8 flash_period,
+			       u8 flash_on, u8 ramp_times)
+{
+	struct ktd202x *chip = led->chip;
+	int i;
+
+	for (i = 0; i < chip->num_leds; i++) {
+		struct ktd202x_led *other = &chip->leds[i];
+
+		if (other == led || !other->on_timer)
+			continue;
+
+		if (flash_period != chip->flash_period ||
+		    flash_on != chip->flash_on || ramp_times != chip->ramp_times)
+			return true;
+	}
+
+	return false;
+}
+
+/* Called with the chip mutex held */
+static int ktd202x_timer_set(struct ktd202x_led *led, struct mc_subled *subleds,
+			     unsigned int num_channels, u8 flash_period, u8 flash_on,
+			     u8 ramp_times)
+{
+	struct ktd202x *chip = led->chip;
+	u8 ctrl_mask = 0;
+	u8 ctrl_mode = 0;
+	bool busy;
+	int ret;
+	int i;
+
+	busy = ktd202x_timer_busy(led, flash_period, flash_on, ramp_times);
+	led->on_timer = !busy;
+
+	for (i = 0; i < num_channels; i++) {
+		int channel = subleds[i].channel;
+
+		ctrl_mask |= KTD202X_CHANNEL_CTRL_MASK(channel);
+		ctrl_mode |= ktd202x_channel_mode(led, channel);
+	}
+
+	/* Without the timer, leave the LED steadily on */
+	if (busy) {
+		ret = regmap_update_bits(chip->regmap, KTD202X_REG_CHANNEL_CTRL,
+					 ctrl_mask, ctrl_mode);
+		return ret ? ret : -EBUSY;
+	}
+
+	chip->flash_period = flash_period;
+	chip->flash_on = flash_on;
+	chip->ramp_times = ramp_times;
+
+	/*
+	 * Update the timer registers only when they change, so that the
+	 * blink of another LED on the timer is not restarted.
+	 */
+	ret = regmap_update_bits(chip->regmap, KTD202X_REG_FLASH_PERIOD, 0xff,
+				 flash_period);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(chip->regmap, KTD202X_REG_PWM1_TIMER, 0xff,
+				 flash_on);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(chip->regmap, KTD202X_REG_TRISE_FALL, 0xff,
+				 ramp_times);
+	if (ret)
+		return ret;
+
+	return regmap_update_bits(chip->regmap, KTD202X_REG_CHANNEL_CTRL,
+				  ctrl_mask, ctrl_mode);
+}
+
 static int ktd202x_blink_set(struct ktd202x_led *led, unsigned long *delay_on,
 			     unsigned long *delay_off, struct mc_subled *subleds,
 			     unsigned int num_channels)
@@ -328,22 +406,21 @@ static int ktd202x_blink_set(struct ktd202x_led *led, unsigned long *delay_on,
 	unsigned long delay_total_ms;
 	int ret, num_steps, on;
 	u8 ctrl_mask = 0;
-	u8 ctrl_pwm1 = 0;
 	u8 ctrl_on = 0;
 	int i;
 
 	mutex_lock(&led->chip->mutex);
 
-	for (i = 0; i < num_channels; i++) {
-		int channel = subleds[i].channel;
-
-		ctrl_mask |= KTD202X_CHANNEL_CTRL_MASK(channel);
-		ctrl_on |= KTD202X_CHANNEL_CTRL_ON(channel);
-		ctrl_pwm1 |= KTD202X_CHANNEL_CTRL_PWM1(channel);
-	}
-
 	/* Never off - brightness is already set, disable blinking */
 	if (!*delay_off) {
+		for (i = 0; i < num_channels; i++) {
+			int channel = subleds[i].channel;
+
+			ctrl_mask |= KTD202X_CHANNEL_CTRL_MASK(channel);
+			ctrl_on |= KTD202X_CHANNEL_CTRL_ON(channel);
+		}
+
+		led->on_timer = false;
 		ret = regmap_update_bits(led->chip->regmap, KTD202X_REG_CHANNEL_CTRL,
 					 ctrl_mask, ctrl_on);
 		goto out;
@@ -363,28 +440,19 @@ static int ktd202x_blink_set(struct ktd202x_led *led, unsigned long *delay_on,
 	on = mul_u64_u64_div_u64(*delay_on, KTD202X_FLASH_ON_MAX, delay_total_ms);
 	on = clamp(on, 1, KTD202X_FLASH_ON_MAX - 1);
 
+	/*
+	 * Without the ramps a hardware pattern may have set. On -EBUSY the
+	 * LED core falls back to software blinking.
+	 */
+	ret = ktd202x_timer_set(led, subleds, num_channels, num_steps, on,
+				KTD202X_RAMP_TIMES_2_MS);
+	if (ret)
+		goto out;
+
 	/* Actually used delay_{on,off} values */
 	delay_total_ms = ktd202x_flash_period_ms(info, num_steps);
 	*delay_on = (delay_total_ms * on) / KTD202X_FLASH_ON_MAX;
 	*delay_off = delay_total_ms - *delay_on;
-
-	/* Set timings */
-	ret = regmap_write(led->chip->regmap, KTD202X_REG_FLASH_PERIOD, num_steps);
-	if (ret)
-		goto out;
-
-	ret = regmap_write(led->chip->regmap, KTD202X_REG_PWM1_TIMER, on);
-	if (ret)
-		goto out;
-
-	/* Drop the ramps a hardware pattern may have left behind */
-	ret = regmap_write(led->chip->regmap, KTD202X_REG_TRISE_FALL,
-			   KTD202X_RAMP_TIMES_2_MS);
-	if (ret)
-		goto out;
-
-	ret = regmap_update_bits(led->chip->regmap, KTD202X_REG_CHANNEL_CTRL,
-				 ctrl_mask, ctrl_pwm1);
 out:
 	mutex_unlock(&led->chip->mutex);
 	return ret;
@@ -484,18 +552,9 @@ static int ktd202x_pattern_program(struct ktd202x_led *led,
 	unsigned int t[KTD202X_PATTERN_LEN];
 	unsigned int rise, fall, num_steps, on;
 	unsigned long period_ms, ton_ms;
-	u8 ctrl_mask = 0;
-	u8 ctrl_pwm1 = 0;
 	int ret;
 	int i;
 
-	for (i = 0; i < num_channels; i++) {
-		int channel = subleds[i].channel;
-
-		ctrl_mask |= KTD202X_CHANNEL_CTRL_MASK(channel);
-		ctrl_pwm1 |= KTD202X_CHANNEL_CTRL_PWM1(channel);
-	}
-
 	for (i = 0; i < KTD202X_PATTERN_LEN; i++)
 		t[i] = min_t(u32, pattern[i].delta_t, KTD202X_PATTERN_MAX_MS);
 
@@ -517,24 +576,10 @@ static int ktd202x_pattern_program(struct ktd202x_led *led,
 		     1, KTD202X_FLASH_ON_MAX - 1);
 
 	mutex_lock(&led->chip->mutex);
-
-	ret = regmap_write(led->chip->regmap, KTD202X_REG_FLASH_PERIOD, num_steps);
-	if (ret)
-		goto out;
-
-	ret = regmap_write(led->chip->regmap, KTD202X_REG_PWM1_TIMER, on);
-	if (ret)
-		goto out;
-
-	ret = regmap_write(led->chip->regmap, KTD202X_REG_TRISE_FALL,
-			   KTD202X_RAMP_TFALL(fall) | KTD202X_RAMP_TRISE(rise));
-	if (ret)
-		goto out;
-
-	ret = regmap_update_bits(led->chip->regmap, KTD202X_REG_CHANNEL_CTRL,
-				 ctrl_mask, ctrl_pwm1);
-out:
+	ret = ktd202x_timer_set(led, subleds, num_channels, num_steps, on,
+				KTD202X_RAMP_TFALL(fall) | KTD202X_RAMP_TRISE(rise));
 	mutex_unlock(&led->chip->mutex);
+
 	return ret;
 }
 
@@ -556,7 +601,11 @@ static int ktd202x_pattern_single_set(struct led_classdev *cdev,
 
 	info.channel = led->index;
 
-	return ktd202x_pattern_program(led, pattern, &info, 1);
+	ret = ktd202x_pattern_program(led, pattern, &info, 1);
+	if (ret)
+		ktd202x_brightness_single_set(cdev, LED_OFF);
+
+	return ret;
 }
 
 static int ktd202x_pattern_single_clear(struct led_classdev *cdev)
@@ -580,8 +629,12 @@ static int ktd202x_pattern_mc_set(struct led_classdev *cdev,
 	if (ret)
 		return ret;
 
-	return ktd202x_pattern_program(led, pattern, mc->subled_info,
-				       mc->num_colors);
+	ret = ktd202x_pattern_program(led, pattern, mc->subled_info,
+				      mc->num_colors);
+	if (ret)
+		ktd202x_brightness_mc_set(cdev, LED_OFF);
+
+	return ret;
 }
 
 static int ktd202x_pattern_mc_clear(struct led_classdev *cdev)

-- 
Git-155)

Re: [PATCH 10/10] leds: rgb: ktd202x: Don't let a blinking LED retime another one

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [Medium] leds: ktd202x: double truncation in ktd202x_blink_set() breaks timer sharing
- [Medium] leds: ktd202x: ktd202x_timer_set() leaves on_timer set on register write error
- [Medium] leds: ktd202x: regmap_update_bits() skips timer setup after regulator disable
- [Medium] leds: rgb: ktd202x: race on cdev.brightness in ktd202x_brightness_set()
- [Medium] leds: rgb: ktd202x: blink_set() clobbers brightness after -EBUSY fallback
- [Medium] leds: ktd202x: race and glitch from unlocked gap before ktd202x_timer_set()
- [Low] leds: ktd202x: pattern_set turns LED on in hardware before timer busy check
--

commit 06fc1f38dbec3466d62b49ae0ee28d54bc03cc75
Author: Joseph Telaak <joseph@telaak.dev>

leds: rgb: ktd202x: Don't let a blinking LED retime another one

This commit tracks which LEDs are actively using the shared PWM1 hardware
flash timer so that a newly blinking or breathing LED only joins the hardware
timer if it is unused or already configured with the same period, on time,
and ramp timings, returning -EBUSY otherwise.

> diff --git a/drivers/leds/rgb/leds-ktd202x.c b/drivers/leds/rgb/leds-ktd202x.c
> index 5f51df9930cb2..f4544ba337d17 100644
> --- a/drivers/leds/rgb/leds-ktd202x.c
> +++ b/drivers/leds/rgb/leds-ktd202x.c
[ ... ]
> @@ -212,24 +225,9 @@ static int ktd202x_brightness_set(struct ktd202x_led *led,
>  			return ret;
>  	}
>  
> -	ret = regmap_read(led->chip->regmap, KTD202X_REG_CHANNEL_CTRL, &state);
> -	if (ret)
> -		return ret;
> -
> -	/*
> -	 * In multicolor case, assume blink mode if PWM is set for at least one
> -	 * channel because another channel cannot be in state ON at the same time
> -	 */
> -	for (i = 0; i < num_channels; i++) {
> -		int channel_state;
> -
> -		channel = subleds[i].channel;
> -		channel_state = (state >> 2 * channel) & KTD202X_CHANNEL_CTRL_MASK(0);
> -		if (channel_state == KTD202X_CHANNEL_CTRL_OFF)
> -			continue;
> -		mode_blink = channel_state == KTD202X_CHANNEL_CTRL_PWM1(0);
> -		break;
> -	}
> +	/* A LED that is turned off stops blinking and leaves the timer */
> +	if (!led->cdev.brightness)
> +		led->on_timer = false;

[Severity: Medium]
Can checking led->cdev.brightness here race with concurrent brightness
updates and fail to clear led->on_timer?

In ktd202x_brightness_single_set() and ktd202x_brightness_mc_set(),
cdev->brightness is written before acquiring led->chip->mutex:

ktd202x_brightness_single_set()
    cdev->brightness = value;
    mutex_lock(&led->chip->mutex);
    info.brightness = value;
    info.channel = led->index;
    ret = ktd202x_brightness_set(led, &info, 1);

In addition, the LED core updates led_cdev->brightness synchronously in
led_set_brightness_nosleep() before scheduling set_brightness_work. When a
blinking LED is turned off and immediately set to a new steady brightness
(for example via LED_SET_BRIGHTNESS_OFF followed by LED_SET_BRIGHTNESS in
set_brightness_delayed(), or two concurrent updates), led->cdev.brightness
can already be non-zero by the time ktd202x_brightness_set() runs for the
LED_OFF transition.

In that case, subleds[i].brightness is 0, yet led->cdev.brightness is
non-zero, so led->on_timer stays true and the subsequent non-zero brightness
write puts the channel back into KTD202X_CHANNEL_CTRL_PWM1 instead of steady
KTD202X_CHANNEL_CTRL_ON.

Also, because led_set_brightness(cdev, LED_OFF) defers brightness_set_blocking
to set_brightness_work, turning off a blinking LED and immediately setting a
blink or pattern on a second LED can still see other->on_timer set in
ktd202x_timer_busy() before the work item runs. Should ktd202x_brightness_set()
determine whether all channels are off from the subled brightness values
passed under led->chip->mutex?

[ ... ]
> @@ -320,6 +315,89 @@ static unsigned long ktd202x_flash_period_ms(const struct ktd202x_info *info,
>  	return num_steps * KTD202X_FLASH_PERIOD_STEP_MS + info->flash_period_min_ms;
>  }
>  
[ ... ]
> +/* Called with the chip mutex held */
> +static int ktd202x_timer_set(struct ktd202x_led *led, struct mc_subled *subleds,
> +			     unsigned int num_channels, u8 flash_period, u8 flash_on,
> +			     u8 ramp_times)
> +{
> +	struct ktd202x *chip = led->chip;
> +	u8 ctrl_mask = 0;
> +	u8 ctrl_mode = 0;
> +	bool busy;
> +	int ret;
> +	int i;
> +
> +	busy = ktd202x_timer_busy(led, flash_period, flash_on, ramp_times);
> +	led->on_timer = !busy;
> +
> +	for (i = 0; i < num_channels; i++) {
> +		int channel = subleds[i].channel;
> +
> +		ctrl_mask |= KTD202X_CHANNEL_CTRL_MASK(channel);
> +		ctrl_mode |= ktd202x_channel_mode(led, channel);
> +	}
> +
> +	/* Without the timer, leave the LED steadily on */
> +	if (busy) {
> +		ret = regmap_update_bits(chip->regmap, KTD202X_REG_CHANNEL_CTRL,
> +					 ctrl_mask, ctrl_mode);
> +		return ret ? ret : -EBUSY;
> +	}
> +
> +	chip->flash_period = flash_period;
> +	chip->flash_on = flash_on;
> +	chip->ramp_times = ramp_times;
> +
> +	/*
> +	 * Update the timer registers only when they change, so that the
> +	 * blink of another LED on the timer is not restarted.
> +	 */
> +	ret = regmap_update_bits(chip->regmap, KTD202X_REG_FLASH_PERIOD, 0xff,
> +				 flash_period);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_update_bits(chip->regmap, KTD202X_REG_PWM1_TIMER, 0xff,
> +				 flash_on);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_update_bits(chip->regmap, KTD202X_REG_TRISE_FALL, 0xff,
> +				 ramp_times);
> +	if (ret)
> +		return ret;
> +
> +	return regmap_update_bits(chip->regmap, KTD202X_REG_CHANNEL_CTRL,
> +				  ctrl_mask, ctrl_mode);
> +}

[Severity: Medium]
If one of the four regmap_update_bits() calls in the !busy path of
ktd202x_timer_set() fails, won't led->on_timer remain true?

When called from ktd202x_blink_set(), returning a negative error code causes
led_blink_setup() in the LED core to fall back to software blinking without
first setting brightness to LED_OFF. With led->on_timer left set to true,
each software blink ON tick in ktd202x_brightness_set() will program
KTD202X_CHANNEL_CTRL_PWM1 instead of KTD202X_CHANNEL_CTRL_ON, and
ktd202x_timer_busy() will treat this LED as occupying the hardware timer
with the partially updated chip->flash_period, chip->flash_on, and
chip->ramp_times values.

Should led->on_timer and the chip timer fields only be committed once the
register writes succeed (or rolled back on error)?

[Severity: Medium]
Also in ktd202x_timer_set(), could switching KTD202X_REG_FLASH_PERIOD,
KTD202X_REG_PWM1_TIMER, and KTD202X_REG_TRISE_FALL from regmap_write() to
regmap_update_bits() skip programming the hardware timer after the chip has
been powered down?

When all LEDs are turned off, ktd202x_brightness_set() calls
ktd202x_chip_disable(), which disables the vin and vio regulators without
invalidating the REGCACHE_FLAT cache:

ktd202x_brightness_set()
    ktd202x_chip_disable()
        regmap_write(chip->regmap, KTD202X_REG_RESET_CONTROL, KTD202X_ENABLE_CTRL_SLEEP);
        regulator_bulk_disable(ARRAY_SIZE(chip->regulators), chip->regulators);

If disabling the regulators removes power from the chip, the hardware timer
registers reset to their power-on defaults, while REGCACHE_FLAT retains the
last values written before power-down. Previously regmap_write() always
transmitted the timer registers over I2C, whereas regmap_update_bits() skips
the write when the cached value matches. If an LED is later set to blink or
breathe with the same timings as before the chip was disabled, won't
regmap_update_bits() elide the register writes and leave the chip running at
its reset defaults?

> @@ -363,28 +440,19 @@ static int ktd202x_blink_set(struct ktd202x_led *led, unsigned long *delay_on,
>  	on = mul_u64_u64_div_u64(*delay_on, KTD202X_FLASH_ON_MAX, delay_total_ms);
>  	on = clamp(on, 1, KTD202X_FLASH_ON_MAX - 1);
>  
> +	/*
> +	 * Without the ramps a hardware pattern may have set. On -EBUSY the
> +	 * LED core falls back to software blinking.
> +	 */
> +	ret = ktd202x_timer_set(led, subleds, num_channels, num_steps, on,
> +				KTD202X_RAMP_TIMES_2_MS);
> +	if (ret)
> +		goto out;
> +
>  	/* Actually used delay_{on,off} values */
>  	delay_total_ms = ktd202x_flash_period_ms(info, num_steps);
>  	*delay_on = (delay_total_ms * on) / KTD202X_FLASH_ON_MAX;
>  	*delay_off = delay_total_ms - *delay_on;

[Severity: Medium]
Documentation/leds/leds-ktd202x.rst notes:

"To share the timer, give the second LED the delays that the first one reads
back."

Does round-tripping the read-back delay_on and delay_off values through
ktd202x_blink_set() actually produce the same flash_on value when both
(delay_total_ms / 128) and on are odd?

Because delay_total_ms is a multiple of 128 ms (128 * k) and
KTD202X_FLASH_ON_MAX is 256, computing the read-back delay_on on the first
LED truncates towards zero:

    *delay_on = (delay_total_ms * on) / KTD202X_FLASH_ON_MAX;

which is (k * on) / 2. Whenever k and on are both odd (for example, the
default 500 ms / 500 ms blink on KTD2026/KTD2027 gives num_steps = 5,
delay_total_ms = 896 ms with k = 7, and any odd on step such as on = 129
reads back as delay_on = 451 ms, delay_off = 445 ms), *delay_on truncates
to (k * on - 1) / 2.

When those read-back delays are written to the second LED,
ktd202x_blink_set() computes:

    on = mul_u64_u64_div_u64(*delay_on, KTD202X_FLASH_ON_MAX, delay_total_ms);

which evaluates to ((k * chip->flash_on - 1) / 2 * 256) / (128 * k) =
chip->flash_on - 1. That causes ktd202x_timer_busy() to see flash_on !=
chip->flash_on and reject the second LED with -EBUSY. Should the conversion
from *delay_on to on (or both conversions) round to nearest instead of
truncating twice?

[Severity: Medium]
When ktd202x_timer_set() returns -EBUSY here and the LED core falls back to
software blinking, can a subsequent update to delay_on or delay_off clobber
the LED's configured brightness?

In drivers/leds/led-core.c, led_blink_setup() falls back to
led_set_software_blink(), where led_timer_function() alternates
cdev->brightness between cdev->blink_brightness and LED_OFF (0). If the user
later writes delay_on or delay_off via sysfs while the software blink timer
happens to be in its OFF half-cycle (so cdev->brightness is 0 while
cdev->blink_brightness holds the configured brightness),
ktd202x_blink_single_set() and ktd202x_blink_mc_set() do:

ktd202x_blink_single_set()
    if (!cdev->brightness) {
        ret = ktd202x_brightness_single_set(cdev, cdev->max_brightness);
        if (ret)
            return ret;
    }

Wouldn't this overwrite a lower configured brightness with max_brightness (or
KTD202X_MAX_BRIGHTNESS in ktd202x_blink_mc_set()) instead of preserving
cdev->blink_brightness?

[ ... ]
> @@ -556,7 +601,11 @@ static int ktd202x_pattern_single_set(struct led_classdev *cdev,
>  
>  	info.channel = led->index;
>  
> -	return ktd202x_pattern_program(led, pattern, &info, 1);
> +	ret = ktd202x_pattern_program(led, pattern, &info, 1);
> +	if (ret)
> +		ktd202x_brightness_single_set(cdev, LED_OFF);
> +
> +	return ret;
>  }
>  
>  static int ktd202x_pattern_single_clear(struct led_classdev *cdev)
> @@ -580,8 +629,12 @@ static int ktd202x_pattern_mc_set(struct led_classdev *cdev,
>  	if (ret)
>  		return ret;
>  
> -	return ktd202x_pattern_program(led, pattern, mc->subled_info,
> -				       mc->num_colors);
> +	ret = ktd202x_pattern_program(led, pattern, mc->subled_info,
> +				      mc->num_colors);
> +	if (ret)
> +		ktd202x_brightness_mc_set(cdev, LED_OFF);
> +
> +	return ret;
>  }

[Severity: Low]
In ktd202x_pattern_single_set() and ktd202x_pattern_mc_set(),
ktd202x_brightness_single_set() or ktd202x_brightness_mc_set() is called
before ktd202x_pattern_program().

When another LED is already using the timer with different parameters, won't
this turn the LED physically on in hardware over I2C at pattern[1].brightness
(and write KTD202X_CHANNEL_CTRL_ON again in the busy branch of
ktd202x_timer_set()), only to turn it back off over I2C when
ktd202x_pattern_program() returns -EBUSY, causing a brief flash on an off LED
(or turning off an LED that was previously lit)?

[Severity: Medium]
Relatedly, because ktd202x_brightness_single_set() and
ktd202x_brightness_mc_set() release led->chip->mutex before
ktd202x_blink_set() or ktd202x_pattern_program() re-acquires it to call
ktd202x_timer_set(), can an intervening LED_OFF update race in that unlocked
window?

For example, if a queued set_brightness_work item (from a prior software
blink or software pattern timer tick, or multi_intensity_store() when
brightness is 0) or a concurrent delay_on=0 write runs
ktd202x_brightness_set() with LED_OFF between the brightness call and
ktd202x_timer_set():

CPU0                                     CPU1
ktd202x_pattern_single_set()
  ktd202x_brightness_single_set()
    mutex_lock(&chip->mutex)
    ...
    mutex_unlock(&chip->mutex)
                                         set_brightness_delayed()
                                           ktd202x_brightness_single_set(cdev, LED_OFF)
                                             mutex_lock(&chip->mutex)
                                             led->on_timer = false;
                                             /* channel OFF, chip disabled */
                                             mutex_unlock(&chip->mutex)
  ktd202x_pattern_program()
    mutex_lock(&chip->mutex)
    ktd202x_timer_set()
      led->on_timer = true;
      /* sets channel to PWM1 */
    mutex_unlock(&chip->mutex)

Wouldn't ktd202x_timer_set() set led->on_timer = true and enable PWM1 on the
channel even though cdev->brightness is now 0 (and the chip may have been
disabled by ktd202x_chip_disable())?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261101-ktd202x-et6326-v1-0-eb51eddad08c@telaak.dev?part=10