linux-kernel - Re: [PATCH v3 4/7] pwm: sun4i: Add support to output source clock directly

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: <20191107065118.j4s5cghj4ark7sql@pengutronix.de>
Date:   Thu, 7 Nov 2019 07:51:18 +0100
From:   Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>
To:     Clément Péron <peron.clem@...il.com>
Cc:     Thierry Reding <thierry.reding@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Maxime Ripard <mripard@...nel.org>,
        Chen-Yu Tsai <wens@...e.org>,
        Philipp Zabel <pza@...gutronix.de>, linux-pwm@...r.kernel.org,
        devicetree <devicetree@...r.kernel.org>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Jernej Skrabec <jernej.skrabec@...l.net>
Subject: Re: [PATCH v3 4/7] pwm: sun4i: Add support to output source clock
 directly

Hello Clément,

On Wed, Nov 06, 2019 at 10:24:39PM +0100, Clément Péron wrote:
> On Tue, 5 Nov 2019 at 15:57, Uwe Kleine-König
> <u.kleine-koenig@...gutronix.de> wrote:
> > On Tue, Nov 05, 2019 at 02:14:53PM +0100, Clément Péron wrote:
> > > +     bypass = state->enabled &&
> > > +              (state->period * clk_rate >= NSEC_PER_SEC) &&
> >
> > This is too coarse. With state->period = 1000000 this is fulfilled
> > (unless the multiplication overflows).
> 
> Sorry, misunderstood the previous mail
> 
> What about something like this ?
> ((state->period - 1) * clk_rate <= NSEC_PER_SEC) &&
> ((state->period + 1) * clk_rate >= NSEC_PER_SEC) &&
>  ((state->duty_cycle - 1) * 2 <= state->period) &&
>  ((state->duty_cycle + 1) * 2 >= state->period);
> 
> We are sure that the user is looking for a PWM around the OSC with a
> 50% duty cycle ?

This again is too strict. The general policy to fulfill a request is:

 1) provide the longest possible period not bigger than requested
 2) provide the longest possible duty cycle not bigger than requested
 3) if possible complete the currently running period before switching
    and don't return to the user before the new setting is active.
    Document the behaviour prominently because the code (usually)
    doesn't allow to understand the hardware's features here.
 4) A disabled PWM should output the inactive level

And then there is a corner case: If the user requests .duty_cycle = 0,
.enabled = 1 it is ok to provide .enabled = 0 iff otherwise 0% isn't
possible.

So the right check for bypass is:

  state->period * clk_rate >= NSEC_PER_SEC &&
  state->period * clk_rate < whatevercanbereachedwithoutbypass &&
  state->duty_cycle * clk_rate * 2 >= NSEC_PER_SEC

Best regards
Uwe

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