linux-kernel - Re: [PATCH v7 5/8] pwm: core: Support new PWM_STAGGERING

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210407054658.qdsjkstqwynxeuxj@pengutronix.de>
Date:   Wed, 7 Apr 2021 07:46:58 +0200
From:   Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
To:     Clemens Gruber <clemens.gruber@...ruber.com>
Cc:     linux-pwm@...r.kernel.org,
        Thierry Reding <thierry.reding@...il.com>,
        Sven Van Asbroeck <TheSven73@...il.com>,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v7 5/8] pwm: core: Support new PWM_STAGGERING_ALLOWED flag

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.

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. 

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.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)