Messages in this thread | | | Date | Fri, 2 Apr 2021 21:48:05 +0200 | From | Uwe Kleine-König <> | Subject | Re: [PATCH v6 4/7] pwm: pca9685: Support staggered output ON times |
| |
Hello,
On Wed, Mar 31, 2021 at 06:21:32PM +0200, Thierry Reding wrote: > However, I'm a bit hesitant about this staggering output mode. From what > I understand what's going to happen for these is basically that overall > each PWM will be running at the requested duty cycle, but the on/off > times will be evenly spread out over the whole period. In other words, > the output *power* of the PWM signal will be the same as if the signal > was a single on/off cycle. That's not technically a PWM signal as the > PWM framework defines it. See the kerneldoc for enum pwm_polarity for > what signals are expected to look like.
After reading this thread I had the impression that there is no (externally visible) difference between using ON = 0 plus programming a new setting when the counter is say 70 and using ON = 30 plus programming a new setting when the counter is 100. But that's not the case and I agree that defaulting to staggering is a bad idea.
Having said that I doubt that adding a property to the device tree is a good solution, because it changes behaviour without the consumer being aware and additionally it's not really a hardware description.
The solution I'd prefer is to change struct pwm_state to include the delay in it. (This would then make the polarity obsolete, because
.duty_cycle = 30 .period = 100 .polarity = POLARITY_INVERTED .offset = 0
is equivalent to
.duty_cycle = 30 .period = 100 .polarity = POLARITY_NORMAL .offset = 70
. Other inverted states can be modified similarily.) Then consumers can be coordinated to use different offsets.
I'm aware changing this isn't trivial, and it's not thought out completely, but I think the end result is rechnically superior to the approach suggested in the patch under discussion.
Best regards Uwe
-- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ | [unhandled content-type:application/pgp-signature] | |