Re: [PATCH 1/1] leds: st1202: add hardware-accelerated blink support

[Manuel Fombuena]

On Thu, 2026-07-23 at 23:17 +0100, Manuel Fombuena wrote:
> On Thu, 23 Jul 2026, Lee Jones wrote:
> 
> > > +	on = clamp_val(on, ST1202_MILLIS_PATTERN_DUR_MIN,
> > > ST1202_MILLIS_PATTERN_DUR_MAX);
> > > +	off = clamp_val(off, ST1202_MILLIS_PATTERN_DUR_MIN,
> > > ST1202_MILLIS_PATTERN_DUR_MAX);
> > > +	on = roundup(on, ST1202_MILLIS_PATTERN_DUR_MIN);
> > > +	off = roundup(off, ST1202_MILLIS_PATTERN_DUR_MIN);
> > 
> > Should we perform the 'roundup' before 'clamp_val' to ensure that
> > rounding the value up does not push it beyond
> > 'ST1202_MILLIS_PATTERN_DUR_MAX'?

This went in a full circle. v2 swapped to roundup before clamp_val,
but then an automated review flagged integer overflow risk for extreme
inputs near ULONG_MAX, since roundup() performs an addition internally.
v3 addressed that by prepending a min_t() cap, but introduced a
redundant trailing clamp_val() that could never fire.

It turns out clamp_val() before roundup() addresses both concerns:
clamping first eliminates the overflow risk, and since
ST1202_MILLIS_PATTERN_DUR_MAX (5610) is an exact multiple of
ST1202_MILLIS_PATTERN_DUR_MIN (22), roundup() on a clamped value
cannot exceed MAX. v4 reverts to the original order with this
explanation in the commit message.
 
--
Manuel Fombuena