linux-kernel - Re: (EXT) Re: (EXT) Re: [PATCH 1/4] pwm: pca9685: remove unused duty

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: <189a1fb7f7aee153151f46fe3bb0f754160472e7.camel@ew.tq-group.com>
Date:   Tue, 07 Apr 2020 15:00:43 +0200
From:   Matthias Schiffer <matthias.schiffer@...tq-group.com>
To:     Thierry Reding <thierry.reding@...il.com>
Cc:     Clemens Gruber <clemens.gruber@...ruber.com>,
        Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>, linux-pwm@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        Sven Van Asbroeck <thesven73@...il.com>
Subject: Re: (EXT) Re: (EXT) Re: [PATCH 1/4] pwm: pca9685: remove unused
 duty_cycle struct element

On Mon, 2020-04-06 at 11:51 +0200, Thierry Reding wrote:
> 
> On Sat, Apr 04, 2020 at 04:17:00PM -0400, Sven Van Asbroeck wrote:
> > 
> > It does look like we have a square peg (this chip) in a round hole
> > (the
> > standard assumptions the pwm core makes) ?
> 
> There are other chips where a single period is shared across multiple
> PWM channels. Typically what we do there is once a period is
> configured
> for a given channel, all subsequent PWM channel configurations must
> use
> the same period, or otherwise the driver will return an error code.
> 
> See for example:
> 
>   - stm32_pwm_config() in drivers/pwm/pwm-stm32.c
>   - lpc18xx_pwm_config() in drivers/pwm/pwm-lpc18xx-sct.c
>   - pwm_imx_tpm_apply_hw() in drivers/pwm/pwm-imx-tpm.c
>   - fsl_pwm_apply_config() in drivers/pwm/pwm-fsl-ftm.c
> 
> The rationale behind that is that we must not change a PWM
> configuration
> without a consumer having explicitly requested it.

These implementations don't deal with the issue in a consistent way
either though:

1) stm32_pwm_config() only checks for channels that are actually
enabled, regardless if they're requested

2) lpc18xx_pwm_config() checks requested channels instead

"2)" seems more correct to me, as the parameters of a requested channel
can't be changed by another user, but it seems to prevent certain
sequences. I don't have a good grasp of the the usual PWM request
control flow - is it correct that the PWM state is not updated with the
PWM args from OF when the PWM is requested? I only see
pwm_adjust_config() doing something like that, which is not used in
many places (and nothing preventing races) - but I might be overlooking
something.

Meaning, is it possible that two drivers request PWMs from the same
pwmchip of type "2)" (or even a single driver using two PWMs) without
configuring them right away may get into a situation where neither can
set up a period at all even when all users want to use the same period?

Matthias