linux-kernel - Re: [PATCH v2] pwm: mediatek: support inverted polarity

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: <ZC7Xf8Wy1x9gnaAY@orome>
Date:   Thu, 6 Apr 2023 16:30:23 +0200
From:   Thierry Reding <thierry.reding@...il.com>
To:     Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
Cc:     Lorenz Brun <lorenz@...n.one>,
        Matthias Brugger <matthias.bgg@...il.com>,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>,
        linux-pwm@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org,
        linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH v2] pwm: mediatek: support inverted polarity

On Thu, Apr 06, 2023 at 03:53:58PM +0200, Uwe Kleine-König wrote:
> Hello Thierry,
> 
> On Thu, Apr 06, 2023 at 03:38:48PM +0200, Thierry Reding wrote:
> > On Thu, Mar 09, 2023 at 02:04:10AM +0100, Lorenz Brun wrote:
> > > +	 * appear to have the capability to invert the output.
> > > +	 * This means that inverted mode can not be fully supported as the
> > > +	 * waveform will always start with the low period and end with the high
> > > +	 * period. Thus reject non-normal polarity if the shape of the waveform
> > > +	 * matters, i.e. usage_power is not set.
> > > +	 */
> > > +	if (state->polarity != PWM_POLARITY_NORMAL && !state->usage_power)
> > >  		return -EINVAL;
> > >  
> > >  	if (!state->enabled) {
> > > @@ -213,7 +221,11 @@ static int pwm_mediatek_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> > >  		return 0;
> > >  	}
> > >  
> > > -	err = pwm_mediatek_config(pwm->chip, pwm, state->duty_cycle, state->period);
> > > +	duty_cycle = state->duty_cycle;
> > > +	if (state->polarity == PWM_POLARITY_INVERSED)
> > > +		duty_cycle = state->period - state->duty_cycle;
> > 
> > That's not really what state->usage_power was meant to address.
> 
> I don't understand your concern here. I don't like .usage_power, but
> AFAICT this is a legitimite use. With .usage_power = true, the lowlevel
> driver is free to shift the phase_offset and even modify the period size
> and the goal is just that the average power-output matches.
> 
> Lorenz's patch does exactly this: It even keeps the period and only
> shifts the phase (by period - duty_cycle). If you consider this not
> legitmate, I think we have to improve the docs about .usage_power.

I realize that I'm being nitpicky here. Setting usage_power = true and
duty = period - duty is a lazy way of achieving what you can easily do
by adjusting the input duty cycle.

If you all really want this, then it should go into the core, because
it's something that can be implemented on basically every single PWM
controller.

Thierry

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