[PATCH v7 0/9] leds: st1202: fix multiple bugs in pattern engine and brightness handling

[PATCH v7 0/9] leds: st1202: fix multiple bugs in pattern engine and brightness handling

[Manuel Fombuena]

This series fixes several bugs in the LED1202 driver related to hardware
pattern programming and brightness control. The issues were found during
testing on a Linksys MX4200v2 router running OpenWrt.

--- Pattern sequence not stopped before reprogramming ---

The LED1202 datasheet (section 4.8) states that writes to PAT_REP and
pattern duration registers are only applied after the sequence completes
or is stopped. When running in infinite loop mode the sequence never
completes on its own, so these writes are silently ignored by the
hardware.

  Patch 1: Stop the running sequence by clearing PATS in the
  configuration register at the start of both pattern_clear() and
  pattern_set(), ensuring the hardware accepts new values immediately.

  Patch 2: Validate pattern input before stopping the sequence. An
  out-of-range duration value should be rejected without disrupting a
  running pattern, so input validation is moved ahead of the sequence
  stop.

--- pattern_clear() leaving hardware in inconsistent state ---

Several independent bugs in pattern_clear() left the hardware in a state
that affected subsequent pattern or brightness operations.

  Patch 3: Fix the duration prescaler formula and the skip marker written
  by pattern_clear(). The original formula (value / ST1202_MILLIS_PATTERN_DUR_MIN
  - 1) was off by one, producing durations ~22ms too short. Additionally,
  pattern_clear() relied on the broken formula to produce register value 0
  (the pattern skip marker) by passing the minimum duration. With the formula
  corrected, pattern_clear() now writes 0 directly to unused duration
  registers instead of going through the conversion function.

  Patch 4: Set all 8 pattern PWM slots to full after clearing. When only
  Pattern0 is restored and patterns 1-7 are left at zero, the cleared
  channel blinks if the global sequencer is subsequently started by
  another channel. Setting all 8 slots to ST1202_PATTERN_PWM_FULL ensures
  the channel outputs a steady ILED value regardless of which sequencer
  step is active.

--- Spurious pattern sequence start during setup ---

  Patch 5: Remove the write of PATS|PATSR to the configuration register
  in st1202_setup(). This accidentally started a pattern sequence before
  any pattern data was programmed, producing undefined output on startup.

--- Brightness control broken while pattern mode is active ---

  Patch 6: Fix brightness having no effect while pattern mode is active.
  When PATS=1, the LED output is ILED x Pattern_PWM / 4095, so writing
  only the ILED register has little visible effect. Rather than stopping
  the global sequencer (which would halt patterns on all other channels),
  set all 8 PWM slots for the channel to ST1202_PATTERN_PWM_FULL before
  writing ILED. With every step at full duty cycle the output is ILED
  regardless of sequencer state.

  Patch 7: Disable the hardware channel in brightness_set() when value
  is zero. Previously only the ILED DAC was zeroed while the channel
  remained enabled, causing residual current through the enabled channel
  and a visible dim glow on the LED.

--- Input validation ---

  Patch 8: Validate the reg property read from the device tree before
  using it as an array index into chip->leds[]. A value >= ST1202_MAX_LEDS
  would cause an out-of-bounds write during probe.

--- Documentation ---

  Patch 9: Correct and extend the hw_pattern documentation. The maximum
  pattern duration was stated as 5660ms but the correct value derived
  from the prescaler formula is 5610ms. The repeat field documentation
  is also corrected and the brightness range is made explicit.

--- Testing ---

Tested on LED1202 hardware via I2C. Register state verified with i2cget
at each step. Correct LED behaviour confirmed across pattern cycling,
infinite repeat, pattern_clear, and direct brightness control with
trigger=none.

--- Changes in v7 ---

  In response to automated review feedback (Sashiko) on v6:

  Patch 4: commit message inaccurately described the baseline behavior
  as restoring Pattern0 to full duty cycle with patterns 1-7 at zero,
  which reflected earlier iterations of the series rather than the
  actual v6 baseline (all 8 slots at LED_OFF). Commit message corrected.

  Other pre-existing issues identified by the automated review are
  outside the scope of this fix series and will be addressed in a
  follow-up submission.

--- Changes in v6 ---

  In response to automated review feedback (Sashiko) on v5:

  Patch 4: pattern_clear() restored only Pattern0 PWM to full, leaving
  patterns 1-7 at zero. If another channel subsequently starts the global
  sequencer, the cleared channel cycles through its PWM steps and blinks.
  All 8 slots are now set to ST1202_PATTERN_PWM_FULL, unifying the loop
  to a single pass from 0.

  Patch 6: brightness_set() stopped the global sequencer via a write to
  CONFIG_REG, disrupting patterns running on other channels. The sequencer
  stop is replaced by setting all 8 PWM slots for the channel to
  ST1202_PATTERN_PWM_FULL, so the channel outputs steady ILED regardless
  of sequencer state without affecting other channels.

  Other pre-existing issues identified by the automated review are
  outside the scope of this fix series and will be addressed in a
  follow-up submission.

--- Changes in v5 ---

  Patch 7: Restore Pattern0 PWM to full duty cycle in brightness_set()
  before writing ILED. When PATS is cleared the LED output is
  ILED × Pattern0_PWM / 4095. A prior zero-brightness hardware pattern
  leaves Pattern0 PWM at zero, causing brightness_set() to have no
  visible effect despite ILED being set correctly. Found through further
  hardware testing.

--- Changes in v4 ---

  In response to automated review feedback (Sashiko) on v3:

  Patch 7: pattern_set() did not re-enable the hardware channel before
  starting the sequencer. If brightness had previously been set to zero
  (disabling the channel), the pattern would run but produce no light.

  Patch 8: reg was declared as signed int but populated via
  of_property_read_u32(). A device tree value >= 0x80000000 would be
  interpreted as negative and silently bypass the bounds check. reg is
  now declared as u32.

  Other pre-existing issues identified by the automated review are
  outside the scope of this fix series and will be addressed in a
  follow-up submission.

--- Changes in v3 ---

  In response to automated review feedback (Sashiko) on v2:

  Patch 1: Extend commit message to clarify that the LED1202 has a single
  global pattern sequencer shared across all channels, and that stopping
  it in pattern_clear() is therefore an inherent hardware constraint rather
  than a deliberate design choice.

  Patches 3+4: Squashed into a single patch. The prescaler fix and the
  skip marker fix are tightly coupled — the skip marker bug was a direct
  consequence of the broken formula — and are clearer as one change.

  Patch 5 (v2): Dropped. Resetting PAT_REP in pattern_clear() is
  unnecessary because pattern_set() always stops the sequencer and writes
  its own PAT_REP value before restarting. The reset introduced a spurious
  failure point without fixing a real bug.

  Patch 4 (was 6): Start the clearing loop at Pattern1 to avoid writing
  Pattern0 twice (the loop previously zeroed it before the explicit full
  restore).

  Patch 5 (was 7): Simplify commit message — remove inaccurate claim that
  the SHFT bit is never re-enabled after probe; pattern_clear() restores
  it during probe.

  New patch 8: Validate the reg device tree property against ST1202_MAX_LEDS
  before using it as an array index.

--- Changes in v2 ---

  Patch 3: Fix commit message wording — programmed durations were ~22ms
  too short, not too long.
  Patch 7: Fix Signed-off-by typo.
  Patch 10: Add missing quotes around commit subject in Fixes: tag.

v1: https://lore.kernel.org/all/WA0P291MB03850F4E9B4EC7389FE89779C5052@WA0P291MB0385.POLP291.PROD.OUTLOOK.COM/
v2: https://lore.kernel.org/all/WA0P291MB03855101311F0506B6448A8EC50D2@WA0P291MB0385.POLP291.PROD.OUTLOOK.COM/
v3: https://lore.kernel.org/all/GV1PR08MB849762937DA5B82DC89F1CC4C5112@GV1PR08MB8497.eurprd08.prod.outlook.com/
v4: https://lore.kernel.org/all/GV1PR08MB849700625A78304DD5FFCA3EC5E32@GV1PR08MB8497.eurprd08.prod.outlook.com/
v5: https://lore.kernel.org/all/GV1PR08MB8497ABC8E6AEFF8AAA1AF883C5E32@GV1PR08MB8497.eurprd08.prod.outlook.com/
v6: https://lore.kernel.org/all/GV1PR08MB84972A16ED557BFB96D63F57C5F52@GV1PR08MB8497.eurprd08.prod.outlook.com/

Manuel Fombuena (9):
  leds: st1202: stop pattern sequence before reprogramming
  leds: st1202: validate pattern input before stopping the sequence
  leds: st1202: fix pattern duration prescaler and pattern_clear skip
    marker
  leds: st1202: set all pattern PWM slots to full after clearing pattern
  leds: st1202: fix spurious pattern sequence start in setup
  leds: st1202: fix brightness having no effect while pattern mode is
    active
  leds: st1202: disable channel when brightness is set to zero
  leds: st1202: validate LED reg property against channel count
  leds: st1202: correct and extend hw_pattern documentation

 Documentation/leds/leds-st1202.rst |   9 ++-
 drivers/leds/leds-st1202.c         | 102 ++++++++++++++++++-----------
 2 files changed, 67 insertions(+), 44 deletions(-)

-- 
2.55.0

[PATCH v7 1/9] leds: st1202: stop pattern sequence before reprogramming

[Manuel Fombuena]

The LED1202 datasheet (section 4.8) states that modifications to the
Pattern Sequence Repetition register (PAT_REP) and pattern duration
registers are only applied after the sequence has completed or been
stopped. When the device is running in infinite loop mode (PAT_REP =
0xFF) the sequence never completes on its own, so these writes are
silently ignored by the hardware.

Neither pattern_clear() nor pattern_set() stop the running sequence
before modifying pattern registers, causing any subsequent pattern
reprogramming to have no effect when the previous pattern was set to
infinite repeat.

Fix this by clearing PATS in the Configuration register before touching
any pattern registers in both functions, ensuring the hardware accepts
the new values immediately.

Note that the LED1202 has a single global pattern sequencer shared by
all channels: PATS, PATSR, the duration registers, and PAT_REP are
chip-wide. Stopping the sequencer in pattern_clear() therefore halts
any pattern running on other channels. This is an inherent hardware
constraint; pattern_set() restarts the sequencer when a new pattern is
programmed.

Fixes: 259230378c65 ("leds: Add LED1202 I2C driver")
Signed-off-by: Manuel Fombuena <fombuena@outlook.com>
Assisted-by: Claude:claude-sonnet-4-6
---
 drivers/leds/leds-st1202.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/leds/leds-st1202.c b/drivers/leds/leds-st1202.c
index 7f68d956f694..316ed8eb054f 100644
--- a/drivers/leds/leds-st1202.c
+++ b/drivers/leds/leds-st1202.c
@@ -200,6 +200,10 @@ static int st1202_led_pattern_clear(struct led_classdev *ldev)
 
 	guard(mutex)(&chip->lock);
 
+	ret = st1202_write_reg(chip, ST1202_CONFIG_REG, ST1202_CONFIG_REG_SHFT);
+	if (ret != 0)
+		return ret;
+
 	for (int patt = 0; patt < ST1202_MAX_PATTERNS; patt++) {
 		ret = st1202_pwm_pattern_write(chip, led->led_num, patt, LED_OFF);
 		if (ret != 0)
@@ -226,6 +230,10 @@ static int st1202_led_pattern_set(struct led_classdev *ldev,
 
 	guard(mutex)(&chip->lock);
 
+	ret = st1202_write_reg(chip, ST1202_CONFIG_REG, ST1202_CONFIG_REG_SHFT);
+	if (ret != 0)
+		return ret;
+
 	for (int patt = 0; patt < len; patt++) {
 		if (pattern[patt].delta_t < ST1202_MILLIS_PATTERN_DUR_MIN ||
 				pattern[patt].delta_t > ST1202_MILLIS_PATTERN_DUR_MAX)
-- 
2.55.0

Re: [PATCH v7 1/9] leds: st1202: stop pattern sequence before reprogramming

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The driver assigns a sleepable function to `led_cdev.brightness_set`, causing panics when called from atomic context.
--

commit b372e8814a9b45529499c4d9490a6fff3999cb74
Author: Manuel Fombuena <fombuena@outlook.com>

leds: st1202: stop pattern sequence before reprogramming

This commit updates st1202_led_pattern_clear() and st1202_led_pattern_set()
to clear the PATS bit in the Configuration register before modifying pattern
registers. This stops the sequence, ensuring the hardware accepts the new
values immediately instead of ignoring them during infinite loop mode.

> diff --git a/drivers/leds/leds-st1202.c b/drivers/leds/leds-st1202.c
> index 7f68d956f6948..316ed8eb054f9 100644
> --- a/drivers/leds/leds-st1202.c
> +++ b/drivers/leds/leds-st1202.c
> @@ -200,6 +200,10 @@ static int st1202_led_pattern_clear(struct led_classdev *ldev)
>  
>  	guard(mutex)(&chip->lock);
>  

[Severity: High]
This is a pre-existing issue, but I noticed that the driver assigns a
sleepable function to led_cdev.brightness_set in st1202_dt_init():

	led->led_cdev.brightness_set = st1202_brightness_set;

Because st1202_brightness_set() acquires a mutex and performs I2C writes,
it can sleep and must not be called from atomic context. When a user
configures an atomic LED trigger (such as 'timer' or 'heartbeat') via
sysfs, led_set_brightness() can be called in softirq/timer context.

Since this is an I2C device, should the driver only implement
brightness_set_blocking and leave brightness_set as NULL to prevent
kernel panics when called from atomic contexts?

> +	ret = st1202_write_reg(chip, ST1202_CONFIG_REG, ST1202_CONFIG_REG_SHFT);
> +	if (ret != 0)
> +		return ret;
> +
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/GV1PR08MB8497C0B898789BB73ACE6EE3C5F52@GV1PR08MB8497.eurprd08.prod.outlook.com?part=1

Re: [PATCH v7 1/9] leds: st1202: stop pattern sequence before reprogramming

[Manuel Fombuena]

On Thu, 2 Jul 2026, sashiko-bot@kernel.org wrote:

> Pre-existing issues:
> - [High] The driver assigns a sleepable function to `led_cdev.brightness_set`, causing panics when called from atomic context.

All issues raised are pre-existing and outside the scope of this patch.
They will be addressed in a follow-up submission.

--
Manuel Fombuena

[PATCH v7 2/9] leds: st1202: validate pattern input before stopping the sequence

[Manuel Fombuena]

Input validation for pattern duration is performed inside the write
loop, after the pattern sequence has already been stopped. If
validation fails mid-loop the chip is left with the sequence stopped
and partially written pattern data, with no recovery.

Move all input validation before the mutex and before any hardware
interaction, so an invalid input leaves the chip state unchanged.

Signed-off-by: Manuel Fombuena <fombuena@outlook.com>
Assisted-by: Claude:claude-sonnet-4-6
---
 drivers/leds/leds-st1202.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/leds/leds-st1202.c b/drivers/leds/leds-st1202.c
index 316ed8eb054f..a750c95ef3f8 100644
--- a/drivers/leds/leds-st1202.c
+++ b/drivers/leds/leds-st1202.c
@@ -228,6 +228,12 @@ static int st1202_led_pattern_set(struct led_classdev *ldev,
 	if (len > ST1202_MAX_PATTERNS)
 		return -EINVAL;
 
+	for (int patt = 0; patt < len; patt++) {
+		if (pattern[patt].delta_t < ST1202_MILLIS_PATTERN_DUR_MIN ||
+				pattern[patt].delta_t > ST1202_MILLIS_PATTERN_DUR_MAX)
+			return -EINVAL;
+	}
+
 	guard(mutex)(&chip->lock);
 
 	ret = st1202_write_reg(chip, ST1202_CONFIG_REG, ST1202_CONFIG_REG_SHFT);
@@ -235,10 +241,6 @@ static int st1202_led_pattern_set(struct led_classdev *ldev,
 		return ret;
 
 	for (int patt = 0; patt < len; patt++) {
-		if (pattern[patt].delta_t < ST1202_MILLIS_PATTERN_DUR_MIN ||
-				pattern[patt].delta_t > ST1202_MILLIS_PATTERN_DUR_MAX)
-			return -EINVAL;
-
 		ret = st1202_pwm_pattern_write(chip, led->led_num, patt, pattern[patt].brightness);
 		if (ret != 0)
 			return ret;
-- 
2.55.0

Re: [PATCH v7 2/9] leds: st1202: validate pattern input before stopping the sequence

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] st1202_led_pattern_set() fails to clear or mark the remaining pattern slots as skipped when len < ST1202_MAX_PATTERNS.
- [High] st1202_led_pattern_clear() unconditionally zeroes the shared global duration registers, destroying the timing of active sequences on all other LEDs.
--

commit 9126a03ff29680e83b737c78e164f544294aa400
Author: Manuel Fombuena <fombuena@outlook.com>

leds: st1202: validate pattern input before stopping the sequence

This commit moves input validation for pattern duration before taking the
mutex and interacting with the hardware. This prevents the chip from being
left in a stopped and partially written state if validation fails mid-loop.

> diff --git a/drivers/leds/leds-st1202.c b/drivers/leds/leds-st1202.c
> index 316ed8eb054f9..a750c95ef3f89 100644
> --- a/drivers/leds/leds-st1202.c
> +++ b/drivers/leds/leds-st1202.c
[ ... ]
> @@ -235,10 +241,6 @@ static int st1202_led_pattern_set(struct led_classdev *ldev,
>  		return ret;
>  
>  	for (int patt = 0; patt < len; patt++) {
> -		if (pattern[patt].delta_t < ST1202_MILLIS_PATTERN_DUR_MIN ||
> -				pattern[patt].delta_t > ST1202_MILLIS_PATTERN_DUR_MAX)
> -			return -EINVAL;
> -
>  		ret = st1202_pwm_pattern_write(chip, led->led_num, patt, pattern[patt].brightness);
>  		if (ret != 0)
>  			return ret;

[Severity: High]
This is a pre-existing issue, but does st1202_led_pattern_set() fail to clear
or mark the remaining pattern slots as skipped when len < ST1202_MAX_PATTERNS?

If a user writes a pattern of length L (where L < 8) to the sysfs pattern
attribute, this loop only iterates up to len, leaving the hardware duration
registers for slots len through 7 unmodified.

The ST1202 executes a fixed 8-slot sequence unless unused slots are explicitly
marked to skip by setting duration to 0.

Because the driver only programs the first L slots and abandons the remaining
slots without zeroing their duration registers, will the hardware sequencer
continue playing the stale durations and PWM values for the remaining steps
instead of truncating the pattern?

[Severity: High]
This isn't a bug introduced by this patch, but does st1202_led_pattern_clear()
unconditionally zero the shared global duration registers?

drivers/leds/leds-st1202.c:st1202_led_pattern_clear() {
    ...
    for (int patt = 0; patt < ST1202_MAX_PATTERNS; patt++) {
        ...
        ret = st1202_write_reg(chip, ST1202_PATTERN_DUR + patt, 0);
    }
    ...
}

If a user sets a pattern on LED A, then clears the pattern on LED B using sysfs,
this unconditional write to the global duration registers might destroy the timing
of active sequences on all other LEDs.

The ST1202 uses a single global sequencer with chip-wide duration registers.
When a pattern is cleared on a single LED, writing 0 to all ST1202_PATTERN_DUR
registers marks the slots as skipped globally.

This instantly freezes or destroys the running pattern on all other LEDs sharing
the chip. Should the driver only modify the target LED's PWM registers to visually
clear it, without touching the shared timing?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/GV1PR08MB8497C0B898789BB73ACE6EE3C5F52@GV1PR08MB8497.eurprd08.prod.outlook.com?part=2

Re: [PATCH v7 2/9] leds: st1202: validate pattern input before stopping the sequence

[Manuel Fombuena]

On Thu, 2 Jul 2026, sashiko-bot@kernel.org wrote:

> Pre-existing issues:
> - [High] st1202_led_pattern_set() fails to clear or mark the remaining pattern slots as skipped when len < ST1202_MAX_PATTERNS.
> - [High] st1202_led_pattern_clear() unconditionally zeroes the shared global duration registers, destroying the timing of active sequences on all other LEDs.

All issues raised are pre-existing and outside the scope of this patch.
They will be addressed in a follow-up submission.

--
Manuel Fombuena

[PATCH v7 3/9] leds: st1202: fix pattern duration prescaler and pattern_clear skip marker

[Manuel Fombuena]

The PATy_DUR register encodes duration as N × 22.2 ms, with register
value 0 reserved as a pattern skip indicator (§7.10). The driver
incorrectly subtracted 1 from the register value:

  value / ST1202_MILLIS_PATTERN_DUR_MIN - 1

This caused two problems:

  - All programmed durations were off by one step (~22 ms too short).
  - Writing the minimum duration (22 ms) produced register value 0,
    silently skipping the pattern step instead of setting a 22 ms
    duration.

The maximum duration constant was also wrong at 5660 ms. The 8-bit
register saturates at 255, giving a maximum of 5610 ms (22 ms × 255).
Values above 5653 ms were already producing a uint8_t overflow and
writing 0 to the hardware.

Fix the formula by removing the erroneous subtraction, and derive the
maximum from the register width so the relationship is explicit. Update
the documentation to reflect the correct maximum.

This exposes a secondary issue: pattern_clear() was calling
st1202_duration_pattern_write() with ST1202_MILLIS_PATTERN_DUR_MIN to
reset unused slots, accidentally relying on the broken formula to
produce register value 0. With the corrected formula, the same call
writes 0x01 (22 ms), leaving unused slots as valid 22 ms zero-PWM
steps and making the LED appear off for 7 × 22 ms out of every cycle.

Write 0 directly to the duration registers in pattern_clear() so unused
slots are always explicitly marked as skip, independently of the
conversion formula.

Fixes: 259230378c65 ("leds: Add LED1202 I2C driver")
Signed-off-by: Manuel Fombuena <fombuena@outlook.com>
Assisted-by: Claude:claude-sonnet-4-6
---
 Documentation/leds/leds-st1202.rst | 2 +-
 drivers/leds/leds-st1202.c         | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/leds/leds-st1202.rst b/Documentation/leds/leds-st1202.rst
index 1a09fbfcedcf..a2353549469e 100644
--- a/Documentation/leds/leds-st1202.rst
+++ b/Documentation/leds/leds-st1202.rst
@@ -17,7 +17,7 @@ To be compatible with the hardware pattern format, maximum 8 tuples of
 brightness (PWM) and duration must be written to hw_pattern.
 
 - Min pattern duration: 22 ms
-- Max pattern duration: 5660 ms
+- Max pattern duration: 5610 ms
 
 The format of the hardware pattern values should be:
 "brightness duration brightness duration ..."
diff --git a/drivers/leds/leds-st1202.c b/drivers/leds/leds-st1202.c
index a750c95ef3f8..b4adade49898 100644
--- a/drivers/leds/leds-st1202.c
+++ b/drivers/leds/leds-st1202.c
@@ -31,7 +31,7 @@
 #define ST1202_ILED_REG0                   0x09
 #define ST1202_MAX_LEDS                    12
 #define ST1202_MAX_PATTERNS                8
-#define ST1202_MILLIS_PATTERN_DUR_MAX      5660
+#define ST1202_MILLIS_PATTERN_DUR_MAX      (ST1202_MILLIS_PATTERN_DUR_MIN * U8_MAX)
 #define ST1202_MILLIS_PATTERN_DUR_MIN      22
 #define ST1202_PATTERN_DUR                 0x16
 #define ST1202_PATTERN_PWM                 0x1E
@@ -85,7 +85,7 @@ static int st1202_write_reg(struct st1202_chip *chip, int reg, uint8_t val)
 
 static uint8_t st1202_prescalar_to_miliseconds(unsigned int value)
 {
-	return value / ST1202_MILLIS_PATTERN_DUR_MIN - 1;
+	return value / ST1202_MILLIS_PATTERN_DUR_MIN;
 }
 
 static int st1202_pwm_pattern_write(struct st1202_chip *chip, int led_num,
@@ -209,7 +209,7 @@ static int st1202_led_pattern_clear(struct led_classdev *ldev)
 		if (ret != 0)
 			return ret;
 
-		ret = st1202_duration_pattern_write(chip, patt, ST1202_MILLIS_PATTERN_DUR_MIN);
+		ret = st1202_write_reg(chip, ST1202_PATTERN_DUR + patt, 0);
 		if (ret != 0)
 			return ret;
 	}
-- 
2.55.0

Re: [PATCH v7 3/9] leds: st1202: fix pattern duration prescaler and pattern_clear skip marker

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The driver improperly maps the single global ST1202 hardware pattern sequencer to independent per-LED operations, causing destructive interference between channels.
- [High] `st1202_led_pattern_set` does not clear trailing unused pattern slots, potentially executing stale delay steps if not preceded by `pattern_clear`.
--

commit b7d0b63932c36211ff0c19e9d1915d909b53394b
Author: Manuel Fombuena <fombuena@outlook.com>

leds: st1202: fix pattern duration prescaler and pattern_clear skip marker

This commit fixes the formula used to calculate the pattern duration
prescaler by removing an erroneous subtraction, updating the maximum
duration constant accordingly. It also explicitly writes 0 to the duration
registers in st1202_led_pattern_clear() so that unused slots are correctly
marked as skipped.

> diff --git a/drivers/leds/leds-st1202.c b/drivers/leds/leds-st1202.c
> index a750c95ef3f89..b4adade49898e 100644
> --- a/drivers/leds/leds-st1202.c
> +++ b/drivers/leds/leds-st1202.c
[ ... ]
> @@ -209,7 +209,7 @@ static int st1202_led_pattern_clear(struct led_classdev *ldev)
>  		if (ret != 0)
>  			return ret;
>  
> -		ret = st1202_duration_pattern_write(chip, patt, ST1202_MILLIS_PATTERN_DUR_MIN);
> +		ret = st1202_write_reg(chip, ST1202_PATTERN_DUR + patt, 0);

[Severity: High]
This isn't a bug introduced by this patch, but could zeroing out the duration
registers here freeze the sequence for other LEDs?

The ST1202 hardware uses a single, globally shared pattern sequencer with
shared step duration registers (ST1202_PATTERN_DUR). When
st1202_led_pattern_clear() is called for one LED, zeroing out these shared
duration registers would stop or corrupt the active pattern on any other LEDs
that are relying on the global sequencer.

Could this cause cross-channel interference, and should the driver manage the
sequencer state globally rather than on a per-LED basis?

[Severity: High]
This is also a pre-existing issue. Is st1202_led_pattern_set() leaving
trailing unused pattern slots active?

While st1202_led_pattern_clear() explicitly writes 0 to all 8 slots to mark
them as skipped, st1202_led_pattern_set() only updates the duration registers
up to the provided len. 

If a previous pattern was longer, won't the trailing duration registers still
contain non-zero values? This might cause the hardware sequencer to execute
those stale delay steps instead of skipping them, artificially lengthening
the new pattern's cycle time.

>  		if (ret != 0)
>  			return ret;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/GV1PR08MB8497C0B898789BB73ACE6EE3C5F52@GV1PR08MB8497.eurprd08.prod.outlook.com?part=3

Re: [PATCH v7 3/9] leds: st1202: fix pattern duration prescaler and pattern_clear skip marker

[Manuel Fombuena]

On Thu, 2 Jul 2026, sashiko-bot@kernel.org wrote:

> Pre-existing issues:
> - [High] The driver improperly maps the single global ST1202 hardware pattern sequencer to independent per-LED operations, causing destructive interference between channels.
> - [High] `st1202_led_pattern_set` does not clear trailing unused pattern slots, potentially executing stale delay steps if not preceded by `pattern_clear`.

All issues raised are pre-existing and outside the scope of this patch.
They will be addressed in a follow-up submission.

--
Manuel Fombuena

[PATCH v7 4/9] leds: st1202: set all pattern PWM slots to full after clearing pattern

[Manuel Fombuena]

pattern_clear() sets all PWM registers for the channel to LED_OFF (0).
In static mode (PATS=0), the LED output is ILED x Pattern0_PWM / 4095;
with Pattern0 at zero the LED remains dark regardless of the ILED value.

The LED1202 has a single global sequencer shared across all channels.
If another channel starts the sequencer after this one has been cleared,
the cleared channel runs through all 8 steps at zero duty cycle and
stays dark regardless of ILED.

Set all 8 PWM slots to ST1202_PATTERN_PWM_FULL so that ILED alone
controls the channel brightness in both static and sequencer modes.

Signed-off-by: Manuel Fombuena <fombuena@outlook.com>
Assisted-by: Claude:claude-sonnet-4-6
---
 drivers/leds/leds-st1202.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/leds/leds-st1202.c b/drivers/leds/leds-st1202.c
index b4adade49898..3203b3768ec5 100644
--- a/drivers/leds/leds-st1202.c
+++ b/drivers/leds/leds-st1202.c
@@ -35,6 +35,7 @@
 #define ST1202_MILLIS_PATTERN_DUR_MIN      22
 #define ST1202_PATTERN_DUR                 0x16
 #define ST1202_PATTERN_PWM                 0x1E
+#define ST1202_PATTERN_PWM_FULL            0x0FFF
 #define ST1202_PATTERN_REP                 0x15
 
 struct st1202_led {
@@ -205,7 +206,7 @@ static int st1202_led_pattern_clear(struct led_classdev *ldev)
 		return ret;
 
 	for (int patt = 0; patt < ST1202_MAX_PATTERNS; patt++) {
-		ret = st1202_pwm_pattern_write(chip, led->led_num, patt, LED_OFF);
+		ret = st1202_pwm_pattern_write(chip, led->led_num, patt, ST1202_PATTERN_PWM_FULL);
 		if (ret != 0)
 			return ret;
 
-- 
2.55.0

Re: [PATCH v7 4/9] leds: st1202: set all pattern PWM slots to full after clearing pattern

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

New issues:
- [High] Trailing unused pattern slots retain 100% duty cycle configurations, causing unintended maximum brightness flashes during shorter sequences.

Pre-existing issues:
- [Critical] The driver assigns a sleeping function to the non-blocking `brightness_set` callback, causing kernel panics in atomic contexts.
- [High] Single-channel pattern operations uncontrollably zero out shared pattern durations, breaking active patterns on all other LEDs.
- [High] Pattern brightness values are written to 12-bit registers without scaling, causing extremely dim LED patterns.
- [High] The `brightness_set_blocking` callback fails to update the brightness level.
--

commit 49e6ce09aaa5b6711e25e1f900651af3d5a1bdcd
Author: Manuel Fombuena <fombuena@outlook.com>

leds: st1202: set all pattern PWM slots to full after clearing pattern

This commit updates pattern_clear() to set all 8 PWM slots for a channel
to ST1202_PATTERN_PWM_FULL (4095). This ensures that the channel responds
to the ILED value in static mode and doesn't remain dark if the global
sequencer is started by another channel.

> diff --git a/drivers/leds/leds-st1202.c b/drivers/leds/leds-st1202.c
> index b4adade49898e..3203b3768ec52 100644
> --- a/drivers/leds/leds-st1202.c
> +++ b/drivers/leds/leds-st1202.c
[ ... ]
> @@ -205,7 +206,7 @@ static int st1202_led_pattern_clear(struct led_classdev *ldev)
>  		return ret;
>  
>  	for (int patt = 0; patt < ST1202_MAX_PATTERNS; patt++) {
> -		ret = st1202_pwm_pattern_write(chip, led->led_num, patt, LED_OFF);
> +		ret = st1202_pwm_pattern_write(chip, led->led_num, patt, ST1202_PATTERN_PWM_FULL);

[Severity: High]
If a pattern is later configured with len < 8 in st1202_led_pattern_set(),
will this cause the trailing unused slots to inherit a 100% duty cycle instead
of 0? 

If another channel concurrently sets an 8-step pattern (configuring the
globally shared step durations to non-zero values for all 8 steps), wouldn't
the first channel's LED flash at maximum brightness during the trailing unused
steps?

[Severity: High]
This is a pre-existing issue, but does the following line uncontrollably
zero out chip-wide shared duration registers for a single channel's pattern
clear, breaking active patterns on all other channels?

drivers/leds/leds-st1202.c:st1202_led_pattern_clear()
		ret = st1202_write_reg(chip, ST1202_PATTERN_DUR + patt, 0);

[Severity: Critical]
This is a pre-existing issue, but does assigning a sleeping function
to the non-blocking brightness_set callback cause a panic when invoked
from atomic contexts (like hardware triggers or timer interrupts)?

drivers/leds/leds-st1202.c:st1202_brightness_set()
	guard(mutex)(&chip->lock);
	st1202_write_reg(chip, ST1202_ILED_REG0 + led->led_num, value);

[Severity: High]
This is a pre-existing issue, but are pattern brightness values written
to 12-bit registers without scaling?

drivers/leds/leds-st1202.c:st1202_led_pattern_set()
		ret = st1202_pwm_pattern_write(chip, led->led_num, patt, pattern[patt].brightness);

Since ST1202 uses 12-bit PWM values (0 to 4095), writing the 8-bit
brightness value (0 to 255) directly into these registers without scaling
appears to cause extremely dim LED patterns.

[Severity: High]
This is a pre-existing issue, but does the brightness_set_blocking callback
fail to update the brightness level?

drivers/leds/leds-st1202.c:st1202_led_set()
	return st1202_channel_set(led->chip, led->led_num, !!value);

It looks like this discards the actual requested brightness value and acts
merely as an on/off boolean toggle, skipping writing the actual brightness
value to the ST1202_ILED_REG0 register.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/GV1PR08MB8497C0B898789BB73ACE6EE3C5F52@GV1PR08MB8497.eurprd08.prod.outlook.com?part=4

Re: [PATCH v7 4/9] leds: st1202: set all pattern PWM slots to full after clearing pattern

[Manuel Fombuena]

On Thu, 2 Jul 2026, sashiko-bot@kernel.org wrote:

> New issues:
> - [High] Trailing unused pattern slots retain 100% duty cycle configurations, causing unintended maximum brightness flashes during shorter sequences.
> 
> Pre-existing issues:
> - [Critical] The driver assigns a sleeping function to the non-blocking `brightness_set` callback, causing kernel panics in atomic contexts.
> - [High] Single-channel pattern operations uncontrollably zero out shared pattern durations, breaking active patterns on all other LEDs.
> - [High] Pattern brightness values are written to 12-bit registers without scaling, causing extremely dim LED patterns.
> - [High] The `brightness_set_blocking` callback fails to update the brightness level.

The comment about trailing slots retaining 100% duty cycle is a
consequence of the pre-existing issue where st1202_led_pattern_set()
does not zero trailing slots when len < ST1202_MAX_PATTERNS. The root
fix is outside the scope of this series and will be addressed in a
follow-up submission.

The remaining comments are pre-existing issues outside the scope of
this patch and will be addressed in a follow-up submission.

--
Manuel Fombuena

[PATCH v7 5/9] leds: st1202: fix spurious pattern sequence start in setup

[Manuel Fombuena]

st1202_setup() writes PATS and PATSR to the Configuration register as
its final step, which starts the hardware pattern sequencer during
device probe before any patterns have been programmed. This causes the
device to run a sequence with whatever values happen to be in the
pattern registers at the time.

Remove the write. The device reset at the start of setup restores all
registers to their power-on defaults, leaving PATS and PATSR cleared.

Fixes: 259230378c65 ("leds: Add LED1202 I2C driver")
Signed-off-by: Manuel Fombuena <fombuena@outlook.com>
Assisted-by: Claude:claude-sonnet-4-6
---
 drivers/leds/leds-st1202.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/leds/leds-st1202.c b/drivers/leds/leds-st1202.c
index 3203b3768ec5..b8a571282900 100644
--- a/drivers/leds/leds-st1202.c
+++ b/drivers/leds/leds-st1202.c
@@ -333,11 +333,6 @@ static int st1202_setup(struct st1202_chip *chip)
 	if (ret < 0)
 		return ret;
 
-	ret = st1202_write_reg(chip, ST1202_CONFIG_REG,
-				ST1202_CONFIG_REG_PATS | ST1202_CONFIG_REG_PATSR);
-	if (ret < 0)
-		return ret;
-
 	return 0;
 }
 
-- 
2.55.0

Re: [PATCH v7 5/9] leds: st1202: fix spurious pattern sequence start in setup

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [High] The driver assigns a sleeping I2C function to the non-blocking `brightness_set` callback, and the blocking callback ignores brightness scaling.
- [Medium] `st1202_led_pattern_set` fails to truncate patterns, leaving trailing sequence steps active.
- [Medium] Per-LED pattern configuration in `st1202_led_pattern_clear` destructively overwrites shared global sequencer registers, breaking patterns running on other channels.
- [Medium] The driver leaves the hardware powered on upon probe failure or device unbind.
--

commit 892a1c21c022e44fcdad1ed04d340f90f20762de
Author: Manuel Fombuena <fombuena@outlook.com>

leds: st1202: fix spurious pattern sequence start in setup

This commit removes a configuration register write in st1202_setup() that
started the hardware pattern sequencer before any patterns were programmed.
By allowing the device reset to keep the pattern bits cleared, it prevents
the device from running a sequence with uninitialized values.

> diff --git a/drivers/leds/leds-st1202.c b/drivers/leds/leds-st1202.c
> index 3203b3768ec52..b8a5712829009 100644
> --- a/drivers/leds/leds-st1202.c
> +++ b/drivers/leds/leds-st1202.c
> @@ -333,11 +333,6 @@ static int st1202_setup(struct st1202_chip *chip)
>  	if (ret < 0)
>  		return ret;
>  
> -	ret = st1202_write_reg(chip, ST1202_CONFIG_REG,
> -				ST1202_CONFIG_REG_PATS | ST1202_CONFIG_REG_PATSR);
> -	if (ret < 0)
> -		return ret;
> -
>  	return 0;
>  }

[Severity: High]
This is a pre-existing issue, but I noticed the driver assigns a sleeping
I2C function to the non-blocking brightness_set callback:

drivers/leds/leds-st1202.c:st1202_dt_init() {
    ...
        led->led_cdev.brightness_set_blocking = st1202_led_set;
        ...
        led->led_cdev.brightness_set = st1202_brightness_set;
    ...
}

Will this cause a kernel panic (scheduling while atomic) if a hardware
trigger updates the LED brightness from softirq/hardirq context?

The subsystem calls brightness_set without deferring to a workqueue,
expecting it to be non-blocking, but st1202_brightness_set acquires a mutex
and performs sleeping I2C writes:

drivers/leds/leds-st1202.c:st1202_brightness_set() {
    ...
    guard(mutex)(&chip->lock);
    ...
    st1202_write_reg(chip, ST1202_ILED_REG0 + led->led_num, value);
    ...
}

Additionally, does the blocking callback ignore actual brightness scaling?
It appears to only turn the channel on or off:

drivers/leds/leds-st1202.c:st1202_led_set() {
    ...
    return st1202_channel_set(led->chip, led->led_num, !!value);
}

[Severity: Medium]
This is a pre-existing issue, but does st1202_led_pattern_set() fail to
truncate patterns, leaving trailing sequence steps active?

drivers/leds/leds-st1202.c:st1202_led_pattern_set() {
    ...
    for (int patt = 0; patt < len; patt++) {
        ret = st1202_pwm_pattern_write(chip, led->led_num, patt,
                                       pattern[patt].brightness);
        if (ret != 0)
            return ret;

        ret = st1202_duration_pattern_write(chip, patt,
                                            pattern[patt].delta_t);
        if (ret != 0)
            return ret;
    }
    ...
}

If a shorter pattern is programmed over a longer one, the loop stops at
len, leaving the global pattern duration registers for trailing steps
unmodified.

Since the hardware sequencer runs up to 8 steps and uses a duration of 0 as
a skip marker, will the LED emit stale flashes from the previously
programmed pattern?

[Severity: Medium]
This is a pre-existing issue, but does st1202_led_pattern_clear()
destructively overwrite shared global sequencer registers?

drivers/leds/leds-st1202.c:st1202_led_pattern_clear() {
    ...
    for (int patt = 0; patt < ST1202_MAX_PATTERNS; patt++) {
        ...
        ret = st1202_write_reg(chip, ST1202_PATTERN_DUR + patt, 0);
        if (ret != 0)
            return ret;
    }
    ...
}

Calling this function on one LED unconditionally clears the pattern duration
array. Since this array is shared globally across all channels on the chip,
will this break patterns running concurrently on other LEDs?

[Severity: Medium]
This is a pre-existing issue, but does the driver leave the hardware
powered on upon probe failure or device unbind?

drivers/leds/leds-st1202.c:st1202_probe() {
    ...
    ret = st1202_setup(chip);
    if (ret < 0)
        return ret;

    ret = st1202_dt_init(chip);
    if (ret < 0)
        return ret;
    ...
        ret = devm_led_classdev_register_ext(&client->dev, &led->led_cdev,
                                             &init_data);
        if (ret < 0)
            return dev_err_probe(&client->dev, ret,
                    "Failed to register LED class device\n");
    ...
}

The hardware is activated by writing the enable flag during st1202_setup().
If st1202_dt_init() or devm_led_classdev_register_ext() fails, or if the
driver is unbound, there is no devm_add_action_or_reset() or remove hook to
gracefully turn off the chip.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/GV1PR08MB8497C0B898789BB73ACE6EE3C5F52@GV1PR08MB8497.eurprd08.prod.outlook.com?part=5

Re: [PATCH v7 5/9] leds: st1202: fix spurious pattern sequence start in setup

[Manuel Fombuena]

On Thu, 2 Jul 2026, sashiko-bot@kernel.org wrote:

> Pre-existing issues:
> - [High] The driver assigns a sleeping I2C function to the non-blocking `brightness_set` callback, and the blocking callback ignores brightness scaling.
> - [Medium] `st1202_led_pattern_set` fails to truncate patterns, leaving trailing sequence steps active.
> - [Medium] Per-LED pattern configuration in `st1202_led_pattern_clear` destructively overwrites shared global sequencer registers, breaking patterns running on other channels.
> - [Medium] The driver leaves the hardware powered on upon probe failure or device unbind.

All issues raised are pre-existing and outside the scope of this patch.
They will be addressed in a follow-up submission.

--
Manuel Fombuena

[PATCH v7 6/9] leds: st1202: fix brightness having no effect while pattern mode is active

[Manuel Fombuena]

Once a hardware pattern is running (PATS=1), writing to the brightness
sysfs attribute only updates the ILED register. The visible output is
ILED x Pattern_PWM / 4095, so the change has little effect and the LED
never returns to steady static operation as the user expects.

The LED1202 has a single global sequencer shared across all channels.
Stopping it in brightness_set() to force static mode would halt running
patterns on all other active LEDs.

Instead, set all 8 PWM slots for the channel to ST1202_PATTERN_PWM_FULL
before writing ILED. With every step at full duty cycle, the output is
ILED x FULL / 4095 = ILED regardless of the sequencer state, without
disturbing other channels.

This also enables basic LED operation without the pattern trigger: with
the trigger set to none, the brightness sysfs attribute fully controls
the LED as a simple on/off device.

Fixes: 259230378c65 ("leds: Add LED1202 I2C driver")
Signed-off-by: Manuel Fombuena <fombuena@outlook.com>
Assisted-by: Claude:claude-sonnet-4-6
---
 drivers/leds/leds-st1202.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/leds/leds-st1202.c b/drivers/leds/leds-st1202.c
index b8a571282900..4909985ade04 100644
--- a/drivers/leds/leds-st1202.c
+++ b/drivers/leds/leds-st1202.c
@@ -136,6 +136,8 @@ static void st1202_brightness_set(struct led_classdev *led_cdev,
 
 	guard(mutex)(&chip->lock);
 
+	for (int patt = 0; patt < ST1202_MAX_PATTERNS; patt++)
+		st1202_pwm_pattern_write(chip, led->led_num, patt, ST1202_PATTERN_PWM_FULL);
 	st1202_write_reg(chip, ST1202_ILED_REG0 + led->led_num, value);
 }
 
-- 
2.55.0

Re: [PATCH v7 6/9] leds: st1202: fix brightness having no effect while pattern mode is active

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

New issues:
- [Medium] The return value of `st1202_pwm_pattern_write` is ignored, resulting in missing error handling and I2C error log spam.

Pre-existing issues:
- [High] The driver assigns a sleeping function to the LED classdev `.brightness_set` callback, violating the atomic context constraint and exacerbating schedule-while-atomic bugs.
- [High] Per-channel pattern operations incorrectly overwrite global hardware state, breaking independent channel operation for all other active LEDs.
- [High] `st1202_led_pattern_set` does not scale the 8-bit brightness values to the 12-bit PWM register scale, resulting in extremely dim patterns.
- [High] Device node references fetched during `for_each_available_child_of_node_scoped` are not properly acquired, resulting in Use-After-Free when they are accessed later during LED registration.
--

commit c4f0635e8d1cc229cdb504a2e19927bd1b7b1d3d
Author: Manuel Fombuena <fombuena@outlook.com>

leds: st1202: fix brightness having no effect while pattern mode is active

Once a hardware pattern is running (PATS=1), writing to the brightness
sysfs attribute only updates the ILED register. The visible output is
ILED x Pattern_PWM / 4095, so the change has little effect and the LED
never returns to steady static operation as the user expects.

> diff --git a/drivers/leds/leds-st1202.c b/drivers/leds/leds-st1202.c
> index b8a5712829009..4909985ade04b 100644
> --- a/drivers/leds/leds-st1202.c
> +++ b/drivers/leds/leds-st1202.c
> @@ -136,6 +136,8 @@ static void st1202_brightness_set(struct led_classdev *led_cdev,
>  
>  	guard(mutex)(&chip->lock);
>  
> +	for (int patt = 0; patt < ST1202_MAX_PATTERNS; patt++)
> +		st1202_pwm_pattern_write(chip, led->led_num, patt, ST1202_PATTERN_PWM_FULL);

[Severity: Medium]
Does this loop need to handle errors from st1202_pwm_pattern_write()? 

If the underlying I2C transaction fails, the function returns an error code,
but it is ignored here. This could result in multiple unnecessary writes and
I2C error log spam upon a hardware bus failure.

[Severity: High]
This is a pre-existing issue, but does assigning this function to the 
brightness_set callback cause schedule-while-atomic panics?

The LED core expects brightness_set to be safe to call from atomic context
(like the timer trigger for software blinking), but st1202_brightness_set()
acquires a mutex and performs sleeping I2C writes. Adding this loop of up
to 16 additional I2C writes inside the callback exacerbates the blocking
behavior.

Should this driver use brightness_set_blocking instead?

>  	st1202_write_reg(chip, ST1202_ILED_REG0 + led->led_num, value);
>  }

[Severity: High]
This is a pre-existing issue, but does st1202_led_pattern_clear() break
patterns for all other active LEDs?

When clearing a pattern on a single channel, the code overwrites the global
config register and zeroes out the shared pattern duration array:

st1202_led_pattern_clear() {
        ...
        ret = st1202_write_reg(chip, ST1202_CONFIG_REG, ST1202_CONFIG_REG_SHFT);
        ...
        ret = st1202_write_reg(chip, ST1202_PATTERN_DUR + patt, 0);
}

Since the LED1202 uses a single global sequencer and shared pattern duration 
registers, it seems this halts and corrupts running patterns on all other
channels.

[Severity: High]
This isn't a bug introduced by this patch, but does st1202_led_pattern_set()
fail to scale 8-bit brightness values?

The driver initializes led_cdev.max_brightness = U8_MAX, meaning the pattern
trigger passes brightness values in the range [0, 255]. However, these are
written directly to 12-bit PATTERN_PWM hardware registers:

st1202_led_pattern_set() {
        ...
        ret = st1202_pwm_pattern_write(chip, led->led_num, patt, pattern[patt].brightness);
}

Would this result in the patterns being visually very dim, as the maximum 
brightness of 255 only scales to roughly 6% duty cycle on the hardware?

[Severity: High]
This is a pre-existing issue, but does st1202_dt_init() cause a use-after-free
during LED registration?

The device node references fetched during the scoped iterator loop do not seem
to be properly acquired:

st1202_dt_init() {
        ...
        for_each_available_child_of_node_scoped(...) {
                led->fwnode = of_fwnode_handle(child);
        }
}

Because the scoped iterator drops its reference at the end of each iteration,
the raw pointer assigned to led->fwnode might drop to a zero refcount before 
devm_led_classdev_register_ext() is called later in probe.

Should this be using fwnode_handle_get() to safely increment the refcount?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/GV1PR08MB8497C0B898789BB73ACE6EE3C5F52@GV1PR08MB8497.eurprd08.prod.outlook.com?part=6

Re: [PATCH v7 6/9] leds: st1202: fix brightness having no effect while pattern mode is active

[Manuel Fombuena]

On Thu, 2 Jul 2026, sashiko-bot@kernel.org wrote:

> New issues:
> - [Medium] The return value of `st1202_pwm_pattern_write` is ignored, resulting in missing error handling and I2C error log spam.
> 
> Pre-existing issues:
> - [High] The driver assigns a sleeping function to the LED classdev `.brightness_set` callback, violating the atomic context constraint and exacerbating schedule-while-atomic bugs.
> - [High] Per-channel pattern operations incorrectly overwrite global hardware state, breaking independent channel operation for all other active LEDs.
> - [High] `st1202_led_pattern_set` does not scale the 8-bit brightness values to the 12-bit PWM register scale, resulting in extremely dim patterns.
> - [High] Device node references fetched during `for_each_available_child_of_node_scoped` are not properly acquired, resulting in Use-After-Free when they are accessed later during LED registration.

The comment about the ignored return value of st1202_pwm_pattern_write()
was also raised in the v6 review and addressed in the response to that
review. This is intentional: a transient I2C error on one slot does not
guarantee subsequent writes will also fail. Breaking early would leave
remaining PWM slots unwritten unnecessarily, putting the channel in a
partially updated state. brightness_set() is a void callback and cannot
propagate errors regardless, and st1202_write_reg() already logs each
individual failure via dev_err().

The remaining comments are pre-existing issues outside the scope of
this patch and will be addressed in a follow-up submission.

--
Manuel Fombuena

[PATCH v7 7/9] leds: st1202: disable channel when brightness is set to zero

[Manuel Fombuena]

When brightness_set() is called with LED_OFF, only the ILED register is
zeroed; the channel enable bit is left set from probe time. A hardware
channel enabled with ILED=0 still draws a small residual current, causing
a dim glow even when the LED is supposed to be off.

Fix this by splitting st1202_channel_set() into a lockless inner function
__st1202_channel_set() and a locking wrapper, then calling the inner
function from brightness_set() while it already holds the mutex. The
channel is now disabled when value is zero and re-enabled when non-zero,
in the same lock region as the ILED write.

Fixes: 259230378c65 ("leds: Add LED1202 I2C driver")
Signed-off-by: Manuel Fombuena <fombuena@outlook.com>
Assisted-by: Claude:claude-sonnet-4-6
---
 drivers/leds/leds-st1202.c | 68 ++++++++++++++++++++++----------------
 1 file changed, 39 insertions(+), 29 deletions(-)

diff --git a/drivers/leds/leds-st1202.c b/drivers/leds/leds-st1202.c
index 4909985ade04..2ecd36aae336 100644
--- a/drivers/leds/leds-st1202.c
+++ b/drivers/leds/leds-st1202.c
@@ -128,39 +128,11 @@ static int st1202_duration_pattern_write(struct st1202_chip *chip, int pattern,
 				st1202_prescalar_to_miliseconds(value));
 }
 
-static void st1202_brightness_set(struct led_classdev *led_cdev,
-				enum led_brightness value)
-{
-	struct st1202_led *led = cdev_to_st1202_led(led_cdev);
-	struct st1202_chip *chip = led->chip;
-
-	guard(mutex)(&chip->lock);
-
-	for (int patt = 0; patt < ST1202_MAX_PATTERNS; patt++)
-		st1202_pwm_pattern_write(chip, led->led_num, patt, ST1202_PATTERN_PWM_FULL);
-	st1202_write_reg(chip, ST1202_ILED_REG0 + led->led_num, value);
-}
-
-static enum led_brightness st1202_brightness_get(struct led_classdev *led_cdev)
-{
-	struct st1202_led *led = cdev_to_st1202_led(led_cdev);
-	struct st1202_chip *chip = led->chip;
-	u8 value = 0;
-
-	guard(mutex)(&chip->lock);
-
-	st1202_read_reg(chip, ST1202_ILED_REG0 + led->led_num, &value);
-
-	return value;
-}
-
-static int st1202_channel_set(struct st1202_chip *chip, int led_num, bool active)
+static int __st1202_channel_set(struct st1202_chip *chip, int led_num, bool active)
 {
 	u8 chan_low, chan_high;
 	int ret;
 
-	guard(mutex)(&chip->lock);
-
 	if (led_num <= 7) {
 		ret = st1202_read_reg(chip, ST1202_CHAN_ENABLE_LOW, &chan_low);
 		if (ret < 0)
@@ -188,6 +160,40 @@ static int st1202_channel_set(struct st1202_chip *chip, int led_num, bool active
 	return 0;
 }
 
+static int st1202_channel_set(struct st1202_chip *chip, int led_num, bool active)
+{
+	guard(mutex)(&chip->lock);
+
+	return __st1202_channel_set(chip, led_num, active);
+}
+
+static void st1202_brightness_set(struct led_classdev *led_cdev,
+				enum led_brightness value)
+{
+	struct st1202_led *led = cdev_to_st1202_led(led_cdev);
+	struct st1202_chip *chip = led->chip;
+
+	guard(mutex)(&chip->lock);
+
+	for (int patt = 0; patt < ST1202_MAX_PATTERNS; patt++)
+		st1202_pwm_pattern_write(chip, led->led_num, patt, ST1202_PATTERN_PWM_FULL);
+	st1202_write_reg(chip, ST1202_ILED_REG0 + led->led_num, value);
+	__st1202_channel_set(chip, led->led_num, !!value);
+}
+
+static enum led_brightness st1202_brightness_get(struct led_classdev *led_cdev)
+{
+	struct st1202_led *led = cdev_to_st1202_led(led_cdev);
+	struct st1202_chip *chip = led->chip;
+	u8 value = 0;
+
+	guard(mutex)(&chip->lock);
+
+	st1202_read_reg(chip, ST1202_ILED_REG0 + led->led_num, &value);
+
+	return value;
+}
+
 static int st1202_led_set(struct led_classdev *ldev, enum led_brightness value)
 {
 	struct st1202_led *led = cdev_to_st1202_led(ldev);
@@ -257,6 +263,10 @@ static int st1202_led_pattern_set(struct led_classdev *ldev,
 	if (ret != 0)
 		return ret;
 
+	ret = __st1202_channel_set(chip, led->led_num, true);
+	if (ret != 0)
+		return ret;
+
 	ret = st1202_write_reg(chip, ST1202_CONFIG_REG, (ST1202_CONFIG_REG_PATSR |
 							ST1202_CONFIG_REG_PATS | ST1202_CONFIG_REG_SHFT));
 	if (ret != 0)
-- 
2.55.0

Re: [PATCH v7 7/9] leds: st1202: disable channel when brightness is set to zero

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

Pre-existing issues:
- [High] The driver assigns the sleeping `st1202_brightness_set` function to the non-blocking `brightness_set` callback, causing 'scheduling while atomic' panics.
- [High] The `st1202_led_set` function assigned to `brightness_set_blocking` fails to write the actual requested brightness value to the hardware.
- [High] Saving `fwnode_handle` from a child node without taking a reference inside a scoped iterator leads to a use-after-free.
- [Medium] The `brightness_get` callback silently drops I2C hardware errors, resulting in incorrect brightness state reporting.
- [Medium] Global hardware duration registers are corrupted by per-LED pattern changes.
--

commit f5fe39d06324d4108bb5a3ab6d86700cd360c555
Author: Manuel Fombuena <fombuena@outlook.com>

    leds: st1202: disable channel when brightness is set to zero
    
    When brightness_set() is called with LED_OFF, only the ILED register is
    zeroed; the channel enable bit is left set from probe time. A hardware
    channel enabled with ILED=0 still draws a small residual current, causing
    a dim glow even when the LED is supposed to be off.
    
    Fix this by splitting st1202_channel_set() into a lockless inner function
    __st1202_channel_set() and a locking wrapper, then calling the inner
    function from brightness_set() while it already holds the mutex. The
    channel is now disabled when value is zero and re-enabled when non-zero,
    in the same lock region as the ILED write.

> diff --git a/drivers/leds/leds-st1202.c b/drivers/leds/leds-st1202.c
> index 4909985ade04b..2ecd36aae336a 100644
> --- a/drivers/leds/leds-st1202.c
> +++ b/drivers/leds/leds-st1202.c
> @@ -188,6 +160,40 @@ static int st1202_channel_set(struct st1202_chip *chip, int led_num, bool active

[ ... ]

> +static void st1202_brightness_set(struct led_classdev *led_cdev,
> +				enum led_brightness value)
> +{
> +	struct st1202_led *led = cdev_to_st1202_led(led_cdev);
> +	struct st1202_chip *chip = led->chip;
> +
> +	guard(mutex)(&chip->lock);

[Severity: High]
This isn't a bug introduced by this patch, but is it safe to use a mutex and
i2c operations in st1202_brightness_set()?

The led subsystem requires the brightness_set callback to not sleep, as it can
be invoked from atomic contexts like software blink timers in softirq context.
This function acquires a mutex and performs i2c bus operations which can sleep.

Should this driver implement only brightness_set_blocking instead to avoid
scheduling while atomic panics?

> +
> +	for (int patt = 0; patt < ST1202_MAX_PATTERNS; patt++)
> +		st1202_pwm_pattern_write(chip, led->led_num, patt, ST1202_PATTERN_PWM_FULL);
> +	st1202_write_reg(chip, ST1202_ILED_REG0 + led->led_num, value);
> +	__st1202_channel_set(chip, led->led_num, !!value);
> +}
> +
> +static enum led_brightness st1202_brightness_get(struct led_classdev *led_cdev)
> +{
> +	struct st1202_led *led = cdev_to_st1202_led(led_cdev);
> +	struct st1202_chip *chip = led->chip;
> +	u8 value = 0;
> +
> +	guard(mutex)(&chip->lock);
> +
> +	st1202_read_reg(chip, ST1202_ILED_REG0 + led->led_num, &value);

[Severity: Medium]
This is a pre-existing issue, but does this correctly handle i2c read failures?

If st1202_read_reg() fails, the negative error code is dropped and it returns
the uninitialized value (which is 0). This could hide hardware failures and
trick the subsystem into believing the led is powered off.

Should it return the negative error code if the read fails?

> +
> +	return value;
> +}
> +
>  static int st1202_led_set(struct led_classdev *ldev, enum led_brightness value)
>  {
>  	struct st1202_led *led = cdev_to_st1202_led(ldev);
>  
>  	return st1202_channel_set(led->chip, led->led_num, !!value);
>  }

[Severity: High]
This is a pre-existing issue, but does this function actually write the
requested brightness value to the hardware?

It converts the brightness value to a boolean and toggles the channel enable
bit via st1202_channel_set(), but it seems to omit writing the actual
brightness value to the ST1202_ILED_REG0 register.

This means intermediate brightness levels might be ignored, and the led is only
toggled on or off with whatever brightness was previously configured.

> @@ -257,6 +263,10 @@ static int st1202_led_pattern_set(struct led_classdev *ldev,
>  	if (ret != 0)
>  		return ret;
>  
> +	ret = __st1202_channel_set(chip, led->led_num, true);
> +	if (ret != 0)
> +		return ret;
> +

[Severity: Medium]
This is a pre-existing issue, but does setting a pattern on one led corrupt
the timing of concurrent patterns on other leds?

The duration registers are global to the entire chip. If multiple leds
configure patterns, they will overwrite these same global duration registers.

Also, st1202_led_pattern_clear() unconditionally zeroes all global duration
registers, which would break running patterns on all other leds.

>  	ret = st1202_write_reg(chip, ST1202_CONFIG_REG, (ST1202_CONFIG_REG_PATSR |
>  							ST1202_CONFIG_REG_PATS | ST1202_CONFIG_REG_SHFT));

[Severity: High]
This is a pre-existing issue not shown in the diff, but could there be a
use-after-free in st1202_dt_init() when registering the led?

When using for_each_available_child_of_node_scoped(), it automatically calls
of_node_put() on the child device node at the end of each loop iteration.
Inside the loop, the driver saves the pointer via:

    led->fwnode = of_fwnode_handle(child);

but it doesn't increment the reference count. When the driver later registers
the led using led->fwnode, it accesses a node whose reference count has already
been decremented. Could this be an issue if dynamic device tree overlays are
unloaded?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/GV1PR08MB8497C0B898789BB73ACE6EE3C5F52@GV1PR08MB8497.eurprd08.prod.outlook.com?part=7

Re: [PATCH v7 7/9] leds: st1202: disable channel when brightness is set to zero

[Manuel Fombuena]

On Thu, 2 Jul 2026, sashiko-bot@kernel.org wrote:

> Pre-existing issues:
> - [High] The driver assigns the sleeping `st1202_brightness_set` function to the non-blocking `brightness_set` callback, causing 'scheduling while atomic' panics.
> - [High] The `st1202_led_set` function assigned to `brightness_set_blocking` fails to write the actual requested brightness value to the hardware.
> - [High] Saving `fwnode_handle` from a child node without taking a reference inside a scoped iterator leads to a use-after-free.
> - [Medium] The `brightness_get` callback silently drops I2C hardware errors, resulting in incorrect brightness state reporting.
> - [Medium] Global hardware duration registers are corrupted by per-LED pattern changes.

All issues raised are pre-existing and outside the scope of this patch.
They will be addressed in a follow-up submission.

--
Manuel Fombuena

[PATCH v7 8/9] leds: st1202: validate LED reg property against channel count

[Manuel Fombuena]

The reg property from the device tree is used directly as an array index
into chip->leds[] without bounds checking. A value >= ST1202_MAX_LEDS
would cause an out-of-bounds write during probe.

Fixes: 259230378c65 ("leds: Add LED1202 I2C driver")
Signed-off-by: Manuel Fombuena <fombuena@outlook.com>
Assisted-by: Claude:claude-sonnet-4-6
---
 drivers/leds/leds-st1202.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/leds/leds-st1202.c b/drivers/leds/leds-st1202.c
index 2ecd36aae336..168df5ecf27b 100644
--- a/drivers/leds/leds-st1202.c
+++ b/drivers/leds/leds-st1202.c
@@ -279,13 +279,19 @@ static int st1202_dt_init(struct st1202_chip *chip)
 {
 	struct device *dev = &chip->client->dev;
 	struct st1202_led *led;
-	int err, reg;
+	int err;
+	u32 reg;
 
 	for_each_available_child_of_node_scoped(dev_of_node(dev), child) {
 		err = of_property_read_u32(child, "reg", &reg);
 		if (err)
 			return dev_err_probe(dev, err, "Invalid register\n");
 
+		if (reg >= ST1202_MAX_LEDS)
+			return dev_err_probe(dev, -EINVAL,
+					"LED reg %u out of range [0, %d]\n",
+					reg, ST1202_MAX_LEDS - 1);
+
 		led = &chip->leds[reg];
 		led->is_active = true;
 		led->fwnode = of_fwnode_handle(child);
-- 
2.55.0

Re: [PATCH v7 8/9] leds: st1202: validate LED reg property against channel count

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The driver assigns a sleepable function to the LED subsystem's non-blocking `brightness_set` callback, leading to "scheduling while atomic" kernel panics.
--

Note: The format of this review is altered to use a free-form summary instead
of inline diff quotes due to recitation restrictions.

commit e35dcdba8764dc455863a8cb25600d5218b1d1a5
Author: Manuel Fombuena <fombuena@outlook.com>

leds: st1202: validate LED reg property against channel count

The reg property from the device tree is used directly as an array index
into chip->leds[] without bounds checking. A value >= ST1202_MAX_LEDS
would cause an out-of-bounds write during probe.

[Severity: High]
This is a pre-existing issue, but does this code cause a "scheduling while
atomic" panic when a software trigger like heartbeat or timer is configured?

In drivers/leds/leds-st1202.c during st1202_dt_init(), the driver assigns
st1202_brightness_set to led->led_cdev.brightness_set. The brightness_set
callback is expected to be non-blocking by the LED subsystem core.

However, st1202_brightness_set() uses guard(mutex)(&chip->lock) and performs
I2C transactions, both of which are blocking operations.

When a software trigger fires in a softirq timer context, it calls
led_set_brightness_nosleep(), which directly invokes the brightness_set
callback. Since the callback attempts to acquire a mutex and perform I2C
transfers from this atomic context, it causes a kernel panic.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/GV1PR08MB8497C0B898789BB73ACE6EE3C5F52@GV1PR08MB8497.eurprd08.prod.outlook.com?part=8

Re: [PATCH v7 8/9] leds: st1202: validate LED reg property against channel count

[Manuel Fombuena]

On Thu, 2 Jul 2026, sashiko-bot@kernel.org wrote:

> Pre-existing issues:
> - [High] The driver assigns a sleepable function to the LED subsystem's non-blocking `brightness_set` callback, leading to "scheduling while atomic" kernel panics.

All issues raised are pre-existing and outside the scope of this patch.
They will be addressed in a follow-up submission.

--
Manuel Fombuena

[PATCH v7 9/9] leds: st1202: correct and extend hw_pattern documentation

[Manuel Fombuena]

Fix the repeat section: -1 is a valid value meaning infinite repeat,
as accepted by the ledtrig-pattern sysfs interface; only 0 and values
below -1 are rejected. The previous text incorrectly stated all negative
numbers were invalid. Also remove the redundant trailing sentence since
the behaviour is now covered inline.

Add the brightness range (0-255) to the hw_pattern section, which was
previously undocumented.

Fixes: b1816b22381b ("Documentation:leds: Add leds-st1202.rst")
Signed-off-by: Manuel Fombuena <fombuena@outlook.com>
Assisted-by: Claude:claude-sonnet-4-6
---
 Documentation/leds/leds-st1202.rst | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/Documentation/leds/leds-st1202.rst b/Documentation/leds/leds-st1202.rst
index a2353549469e..ed32eb3a27d4 100644
--- a/Documentation/leds/leds-st1202.rst
+++ b/Documentation/leds/leds-st1202.rst
@@ -16,6 +16,7 @@ in terms of PWM duty-cycle and duration (ms).
 To be compatible with the hardware pattern format, maximum 8 tuples of
 brightness (PWM) and duration must be written to hw_pattern.
 
+- Brightness range: 0-255
 - Min pattern duration: 22 ms
 - Max pattern duration: 5610 ms
 
@@ -26,9 +27,7 @@ The format of the hardware pattern values should be:
 ----------------------------
 
 Specify a pattern repeat number, which is common for all channels.
-Default is 1; negative numbers and 0 are invalid.
+Default is 1. Writing 0 is invalid. Writing -1 or 255 repeats the
+pattern indefinitely.
 
 This file will always return the originally written repeat number.
-
-When the 255 value is written to it, all patterns will repeat
-indefinitely.
-- 
2.55.0

Re: [PATCH v7 9/9] leds: st1202: correct and extend hw_pattern documentation

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Hardware pattern brightness values are not scaled to the 12-bit PWM range, resulting in extremely dim patterns. The documentation update masks this bug by incorrectly blessing the 0-255 range.
- [High] The driver lacks bounds checking for the `repeat` count, leading to silent integer truncation. The documentation update inappropriately enshrines this hardware artifact, breaking API consistency.
--

commit 77ce9e120b28b32a84e379b2ca92614b4ebbf1aa
Author: Manuel Fombuena <fombuena@outlook.com>

leds: st1202: correct and extend hw_pattern documentation

The commit updates the hardware pattern documentation for the ST1202 LED
driver. It adds a brightness range of 0-255 and clarifies that writing -1 or
255 causes infinite repeats. It also removes a redundant trailing sentence.

> diff --git a/Documentation/leds/leds-st1202.rst b/Documentation/leds/leds-st1202.rst
> index a2353549469ee..ed32eb3a27d45 100644
> --- a/Documentation/leds/leds-st1202.rst
> +++ b/Documentation/leds/leds-st1202.rst
> @@ -16,6 +16,7 @@ in terms of PWM duty-cycle and duration (ms).
>  To be compatible with the hardware pattern format, maximum 8 tuples of
>  brightness (PWM) and duration must be written to hw_pattern.
>  
> +- Brightness range: 0-255

[Severity: High]
This is a pre-existing issue, but documenting 0-255 here appears to mask an
underlying bug in the driver.

The 8-bit brightness value (0-255) from the hw_pattern sysfs input is passed
directly to the 12-bit PWM write function without scaling:

drivers/leds/leds-st1202.c:st1202_led_pattern_set() {
    ...
    ret = st1202_pwm_pattern_write(chip, led->led_num, patt, pattern[patt].brightness);
    ...
}

Should the driver be updated to scale the brightness properly (e.g.,
brightness * 4095 / 255) so the hardware pattern isn't permanently
restricted to ~6.2% of its maximum brightness?

>  - Min pattern duration: 22 ms
>  - Max pattern duration: 5610 ms
>  
> @@ -26,9 +27,7 @@ The format of the hardware pattern values should be:
>  ----------------------------
>  
>  Specify a pattern repeat number, which is common for all channels.
> -Default is 1; negative numbers and 0 are invalid.
> +Default is 1. Writing 0 is invalid. Writing -1 or 255 repeats the
> +pattern indefinitely.

[Severity: High]
This isn't a bug introduced by this patch, but updating the documentation
to bless 255 as an infinite repeat value breaks the generic ledtrig-pattern
ABI contract.

The driver currently lacks bounds checking and passes the unvalidated repeat
integer directly to the 8-bit register write:

drivers/leds/leds-st1202.c:st1202_led_pattern_set() {
    ...
    ret = st1202_write_reg(chip, ST1202_PATTERN_REP, repeat);
    ...
}

Because of the implicit truncation to uint8_t, writing 256 causes it to loop
0 times, and writing 255 triggers the hardware's infinite loop instead of
looping 255 times.

Could we fix the bounds checking and truncation bug in the driver instead of
enshrining this hardware limit in the documentation?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/GV1PR08MB8497C0B898789BB73ACE6EE3C5F52@GV1PR08MB8497.eurprd08.prod.outlook.com?part=9

Re: [PATCH v7 9/9] leds: st1202: correct and extend hw_pattern documentation

[Manuel Fombuena]

On Thu, 2 Jul 2026, sashiko-bot@kernel.org wrote:

> Pre-existing issues:
> - [High] Hardware pattern brightness values are not scaled to the 12-bit PWM range, resulting in extremely dim patterns. The documentation update masks this bug by incorrectly blessing the 0-255 range.
> - [High] The driver lacks bounds checking for the `repeat` count, leading to silent integer truncation. The documentation update inappropriately enshrines this hardware artifact, breaking API consistency.

All issues raised are pre-existing and outside the scope of this patch.
They will be addressed in a follow-up submission.

--
Manuel Fombuena

Re: [PATCH v7 0/9] leds: st1202: fix multiple bugs in pattern engine and brightness handling

[Manuel Fombuena]

All Sashiko's comments on v7 are pre-existing issues outside the scope of 
this series, with the exception of one comment each on Patch 4 and Patch 
6. Unless there are any other reviewer comments, there will not be a v8.
The pre-existing issues will be addressed in a follow-up submission.

For the sake of clarity, these are the responses to the non-pre-existing
issues.

Patch 4 comment: after pattern_clear() sets all 8 PWM slots to FULL, a
shorter pattern programmed with len < 8 leaves the trailing slots at
100% duty cycle. If another channel runs an 8-step sequence, the
cleared channel flashes at maximum brightness during its unused steps.

Response: this is a consequence of the pre-existing issue where
st1202_led_pattern_set() does not zero trailing slots when
len < ST1202_MAX_PATTERNS. The root fix is outside the scope of this
series and will be addressed in a follow-up submission.

Patch 6 comment: the loop in st1202_brightness_set() ignores the
return value of st1202_pwm_pattern_write(), potentially continuing
writes after a bus failure.

Response: as noted in the response to the v6 review, this is
intentional. A transient I2C error on one slot does not guarantee
subsequent writes will also fail. Breaking early would leave remaining
PWM slots unwritten unnecessarily, putting the channel in a partially
updated state. brightness_set() is a void callback and cannot propagate
errors regardless, and st1202_write_reg() already logs each individual
failure via dev_err().

--
Manuel Fombuena