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

Manuel Fombuena <[email protected]> Tue, 28 Jul 2026 12:04:35 +0100
Newsgroups org.kernel.vger.linux-leds,org.kernel.vger.linux-kernel
Message-ID <GV1PR08MB8497DF840169A845B8397910C5CB2@GV1PR08MB8497.eurprd08.prod.outlook.com>
On Thu, 2026-07-23 at 23:17 +0100, Manuel Fombuena wrote:
> On Thu, 23 Jul 2026, Lee Jones wrote:
>=20
> > > +	on =3D clamp_val(on, ST1202_MILLIS_PATTERN_DUR_MIN,
> > > ST1202_MILLIS_PATTERN_DUR_MAX);
> > > +	off =3D clamp_val(off, ST1202_MILLIS_PATTERN_DUR_MIN,
> > > ST1202_MILLIS_PATTERN_DUR_MAX);
> > > +	on =3D roundup(on, ST1202_MILLIS_PATTERN_DUR_MIN);
> > > +	off =3D roundup(off, ST1202_MILLIS_PATTERN_DUR_MIN);
> >=20
> > 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.
=20
--
Manuel Fombuena