linux-kernel - Re: [PATCH v6 6/7] pwm: pca9685: Restrict period change for prescaler users

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]

Date:   Mon, 29 Mar 2021 19:33:56 +0200
From:   Clemens Gruber <clemens.gruber@...ruber.com>
To:     Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>
Cc:     linux-pwm@...r.kernel.org,
        Thierry Reding <thierry.reding@...il.com>,
        Sven Van Asbroeck <TheSven73@...il.com>,
        linux-kernel@...r.kernel.org, kernel@...gutronix.de
Subject: Re: [PATCH v6 6/7] pwm: pca9685: Restrict period change for
 prescaler users

On Mon, Mar 29, 2021 at 07:15:59PM +0200, Uwe Kleine-König wrote:
> On Mon, Mar 29, 2021 at 02:57:06PM +0200, Clemens Gruber wrote:
> > @@ -330,14 +345,22 @@ static int pca9685_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> >  
> >  	if (!state->enabled || duty < 1) {
> >  		pca9685_pwm_set_duty(pca, pwm->hwpwm, 0);
> > +		clear_bit(pwm->hwpwm, pca->prescaler_users);
> 
> Hmm, so if "my" channel runs at say
> 
> 	.duty_cycle = 2539520 ns
> 	.period = 5079040 ns
> 
> and I call pwm_apply_state(mypwm, { .duty_cycle = 0, .period = 5079040,
> enabled = true }); it might happen that another channel modifies the
> period and I won't be able to return to the initial setting.

Yes, that's correct.

But that also applies to PWMs set to 100%:

pwm_apply_state(mypwm, { .duty_cycle = 5079040, .period = 5079040,
enabled = true });

As this sets the full ON bit and does not use the prescaler, with the
current code, another channel could modify the period and you wouldn't
be able to return to the initial setting of 50%.

> 
> So I think it's sensible to only clear the user bit if the PWM is
> disabled, but not if it is configured for duty_cycle = 0.
> 
> Does this make sense?

With both cases in mind, you are suggesting we block modifications of
the prescaler if other PWMs are enabled and not if other PWMs are using
the prescaler?

Clemens