Messages in this thread | | | Date | Thu, 28 Oct 2021 10:14:42 +0100 | From | Sean Young <> | Subject | Re: [PATCH v4] media: rc: pwm-ir-tx: Switch to atomic PWM API |
| |
On Thu, Oct 28, 2021 at 08:45:13AM +0200, Uwe Kleine-König wrote: > The conversion is right (I think),
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.
> note this could be optimized a bit > further: state.period only depends on carrier which rarely changes, so > the calculation could be done in pwm_ir_set_carrier(). Ditto for duty > which only depends on state.period and pwm_ir->duty_cycle. (This is for > a separate commit though.)
I'm not sure what caching this is much of a win. The calculation is a few instructions, so you're not winning in the way of speed. On the flip side you use more memory since pwm_state has to be kmalloc() rather than existing just on the stack, and both ioctl handlers and the probe function need to recalculate the period/duty cycle, so there is a slight increase in code size.
This change does not improve anything measurably and only increases code complexity.
> Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Thanks for your review.
Sean
| |