Hi Uwe, On Thu, Oct 28, 2021 at 01:15:35PM +0200, Uwe Kleine-König wrote: > On Thu, Oct 28, 2021 at 10:14:42AM +0100, Sean Young wrote: > > We still have the problem that the pwm drivers calculate the period > > incorrectly by rounding down (except pwm-bcm2835). So the period is not > > as good as it could be in most cases, but this driver can't do anything > > about that. > > Yeah, some time ago I started coding a round_state function > (wip at > https://git.pengutronix.de/cgit/ukl/linux/commit/?h=pwm-wip&id=ae348eb6a55d6526f30ef4a49819197d9616391e) > but this was pushed down on my todo-list by more important stuff. > > If you want to experiment with that ... I was thinking about this problem this morning. - The pwm-ir-tx driver gets a carrier set in Hz, which it has to convert to a period (1e9 / carrier). There is loss of accuracy there. - When it gets to the pwm driver, the period is converted into the format the pwm hardware expects. For example the pwm-bcm2835 driver converts it into clock cycles (1e9 / 8e8). Both calculations involve loss of accuracy because of integer representation. Would it make more sense for the pwm interface to use numer/denom rational numbers? struct rational { u64 numer; u64 denom; }; If pwm-ir-tx would like to set the carrier, it could it like so: struct rational period = { .numer = NUSEC_PER_SEC, .denom = carrier, }; pwm_set_period(&period); Now pwm-bcm2835 could do it like so: int bcm2835_set_period(struct rational *period) { struct rational rate = { .numer = NUSEC_PER_SEC, .denum = clk_get_rate(clk), }; rational_div(&rate, period); int step = rational_to_u64(&rate); } Alternatively, since most of the pwm hardware is doing scaling based on the clock (I think), would not make more sense for the pwm driver interface to take a frequency rather than a period? Then the integer calculations can be simpler: just divide the clock rate by the required frequency and you have the period. Just some thoughts. Sean
