On Wed, Apr 07, 2021 at 10:21:10PM +0200, Clemens Gruber wrote: > On Wed, Apr 07, 2021 at 07:46:58AM +0200, Uwe Kleine-König wrote: > > On Tue, Apr 06, 2021 at 06:41:37PM +0200, Clemens Gruber wrote: > > > If the flag PWM_STAGGERING_ALLOWED is set on a channel, the PWM driver > > > may (if supported by the HW) delay the ON time of the channel relative > > > to the channel number. > > > This does not alter the duty cycle ratio and is only relevant for PWM > > > chips with less prescalers than channels, which would otherwise assert > > > multiple or even all enabled channels at the same time. > > > > > > If this feature is supported by the driver and the flag is set on > > > multiple channels, their ON times are spread out to improve EMI and > > > reduce current spikes. > > > > As said in reply to patch 4/8 already: I don't like this idea and > > think this should be made explicit using a new offset member in struct > > pwm_state instead. That's because I think that the wave form a PWM > > generates should be (completely) defined by the consumer and not by a > > mix between consumer and device tree. Also the consumer has no (sane) > > way to determine if staggering is in use or not. > > I don't think offsets are ideal for this feature: It makes it more > cumbersome for the user, because he has to allocate the offsets > himself instead of a simple on/off switch. > The envisioned usecase is: "I want better EMI behavior and don't care > about the individual channels no longer being asserted at the exact same > time". The formal thing is: "I want better EMI behavior and don't care if periods start with the active phase, it might be anywhere, even over a period boundary." Being asserted at the exact same time is just a detail for the pca9685. > > One side effect (at least for the pca9685) is that when programming a > > new duty cycle it takes a bit longer than without staggering until the > > new setting is active. > > Yes, but it can be turned off if this is a problem, now even per-PWM. Yes and that is a good thing. (BTW: I'd call it per-PWM-consumer, but details.) > > Another objection I have is that we already have some technical debt > > because there are already two different types of drivers (.apply vs > > .config+.set_polarity+.enable+.disable) and I would like to unify this > > first before introducing new stuff. > > But there is already PWM_POLARITY_INVERTED, which can be set in the DT. > I am only adding another flag. I understand your reasoning, and similar to "This diplay backlight needs an inverted PWM (as a low duty-cycle results in a high brightness" this semantic "This consumer doesn't care if the active cycle is anywhere in the period". Hmm, maybe I just have to think about it a bit more to become friends with that thought. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
