linux-kernel - Re: [PATCH 1/3] dt-bindings: added new pwm-sifive driver documentation

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: <CAMgXwThTd32cyEBcbGnYw72x-P9_P8ks2J6QJV69RUUUDsT+3A@mail.gmail.com>
Date:   Mon, 30 Apr 2018 12:09:29 -0700
From:   Wesley Terpstra <wesley@...ive.com>
To:     Thierry Reding <thierry.reding@...il.com>
Cc:     Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Andreas Färber <afaerber@...e.de>,
        Noralf Trønnes <noralf@...nnes.org>,
        David Lechner <david@...hnology.com>,
        Alexandre Belloni <alexandre.belloni@...e-electrons.com>,
        SZ Lin <sz.lin@...a.com>, linux-pwm@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] dt-bindings: added new pwm-sifive driver documentation

On Mon, Apr 30, 2018 at 1:27 AM, Thierry Reding
<thierry.reding@...il.com> wrote:
> I don't like the idea of specifying something in DT that is completely
> approximate because it doesn't give users any kind of control over what
> is considered acceptable. In some cases an approximation to within 10%
> might be acceptable, in other cases users may only want to allow 5%. In
> this case there's even no way to catch or report a deviation from the
> desired value.

My view was that you basically don't have period control on this IP.
You can give it a target that it tries to get as close to as possible,
but there's no guarantee of any kind wrt. the period.

I saw there were a couple other PWM drivers which had no period
control whatsoever. They just allowed duty cycle control. Because this
IP has such a broken period-control interface, I was essentially
trying to bin it in the same category as those drivers.

Perhaps I should just remove all pretense of supporting period and
just always default to the fastest period possible?

> How about you allow users to specify a valid frequency range with
> something like:
>
>         frequency-range = <MIN MAX>;
>
> or
>
>         period-range = <MIN MAX>;

Ok, but now you have to define what happens if a clock change prevents
you from staying within this range.

Rejecting the clock frequency change does not seem a good option for
the actual SoC for which I wrote this driver. There, all the PWM does
is drive an LED bank and clock changes are used to change the core
frequency.

> you could disable the PWM if it was fed with an invalid range.

Is that really desirable behavior? If the period is defined as being
best effort for this PWM IP, which is essentially what I've done, it's
clear you want the PWM to continue to operate. That's certainly the
behavior I want on the actual SoC with this IP.

I'll make all the DTS changes you guys suggested. ie: "-v0", clarified
clocks description, and remove unused interrupts comment.