Messages in this thread | | | Date | Sat, 1 Apr 2023 21:50:47 +0100 | From | Conor Dooley <> | Subject | Re: [PATCH v15 1/2] pwm: add microchip soft ip corePWM driver |
| |
On Thu, Mar 30, 2023 at 08:12:03AM +0100, Conor Dooley wrote:
> + /* > + * Because 0xff is not a permitted value some error will seep into the > + * calculation of prescale as prescale grows. Specifically, this error > + * occurs where the remainder of the prescale calculation is less than > + * prescale. > + * For small values of prescale, only a handful of values will need > + * correction, but overall this applies to almost half of the valid > + * values for tmp. > + * > + * To keep the algorithm's decision making consistent, this case is > + * checked for and the simple solution is to, in these cases, > + * decrement prescale and check that the resulting value of period_steps > + * is valid. > + * > + * period_steps can be computed from prescale: > + * period * clk_rate > + * period_steps = ----------------------------- - 1 > + * NSEC_PER_SEC * (prescale + 1) > + * > + */ > + if (tmp % (MCHPCOREPWM_PERIOD_STEPS_MAX + 1) < *prescale) {
Hmm, looks like 32-bit doesn't like this modulus. I pushed things out for LKP to test before sending as I felt I'd not be allowed to do that operation, but got a build success email from it. I'm not sure why the mail wasn't sent as a reply to this, but <202304020410.A86IBNES-lkp@intel.com> complains: pwm-microchip-core.c:(.text+0x20a): undefined reference to `__aeabi_uldivmod'
I know that tmp < 65536 at this point, so if the general approach is fine, I can always cast it to a non 64-bit type without losing any information.
> + u16 smaller_prescale = *prescale - 1; > + > + *period_steps = div_u64(tmp, smaller_prescale + 1) - 1; > + if (*period_steps < 255) { > + *prescale = smaller_prescale; > + > + return 0; > + } > + } [unhandled content-type:application/pgp-signature] | |