Messages in this thread | | | Date | Fri, 4 Dec 2020 12:21:15 +0100 | From | Uwe Kleine-König <> | Subject | Re: [PATCH v2] pwm: bcm2835: Support apply function for atomic configuration |
| |
Hello Lino,
On Fri, Dec 04, 2020 at 12:42:15AM +0100, Lino Sanfilippo wrote: > On 29.11.20 at 19:10, Uwe Kleine-König wrote: > > You're storing an unsigned long long (i.e. 64 bits) in an u32. If > > you are sure that this won't discard relevant bits, please explain > > this in a comment for the cursory reader. > > What about an extra check then to make sure that the period has not been truncated, > e.g: > > value = DIV_ROUND_CLOSEST_ULL(state->period, scaler); > > /* dont accept a period that is too small or has been truncated */ > if ((value < PERIOD_MIN) || > (value != DIV_ROUND_CLOSEST_ULL(state->period, scaler))) > return -EINVAL;
I'd make value an unsigned long long and check for > 0xffffffff instead of repeating the (expensive) division. (Hmm, maybe the compiler is smart enough to not actually repeat it, but still.)
Best regards Uwe
-- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ | [unhandled content-type:application/pgp-signature] | |