linux-kernel - Re: [PATCH v9 14/27] pwm: jz4740: Improve algorithm of clock calculation

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: <1547129096.16183.0@crapouillou.net>
Date:   Thu, 10 Jan 2019 11:04:56 -0300
From:   Paul Cercueil <paul@...pouillou.net>
To:     Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>, Stephen Boyd <sboyd@...nel.org>
Cc:     Thierry Reding <thierry.reding@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ralf Baechle <ralf@...ux-mips.org>,
        Paul Burton <paul.burton@...s.com>,
        James Hogan <jhogan@...nel.org>,
        Jonathan Corbet <corbet@....net>, linux-pwm@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-watchdog@...r.kernel.org, linux-mips@...r.kernel.org,
        linux-doc@...r.kernel.org, linux-clk@...r.kernel.org,
        Russell King <linux@...linux.org.uk>
Subject: Re: [PATCH v9 14/27] pwm: jz4740: Improve algorithm of clock
 calculation

Adding Stephen to the discussion.
Adding Stephen to the discussion.

On Sat, Jan 5, 2019 at 6:27 PM, Uwe Kleine-König 
<u.kleine-koenig@...gutronix.de> wrote:
> Hello Paul,
> 
> On Sat, Jan 05, 2019 at 06:05:38PM -0300, Paul Cercueil wrote:
>>  On Sat, Jan 5, 2019 at 4:57 PM, Uwe Kleine-König
>>  <u.kleine-koenig@...gutronix.de> wrote:
>>  > You are assuming stuff here about the parent clk which isn't 
>> guaranteed
>>  > (AFAICT) by the clk framework: If you call clk_round_rate(clk, 
>> rate - 1)
>>  > this might well return rate even if the clock could run slower 
>> than
>>  > rate.
>> 
>>  It may not be guaranteed by the clock framework itself, but it is 
>> guaranteed
>>  to behave like that on this family of SoCs.
> 
> You shouldn't rely on that. Experience shows that people will start
> copying code to machines where this is not guaranteed. Even if they
> don't copy and only learn from reading this is bad. Also how do you
> guarantee that this won't change in the future making the pwm code 
> break
> without noticing?
> 
> If you use an API better don't assume more things given than are
> guaranteed by the API.
> 
> Having said that I would consider it sensible to introduce something
> like clk_roundup_rate() and clk_rounddown_rate() which would allow
> calculations like that.

@Stephen:
Some context: my algorithm makes use of clk_round_rate(clk, rate - 1) 
to get the
next (smaller) clock rate that a clock support.

Is it something safe to assume? If not is there a better way?

>>  > Wouldn't it make sense to start iterating with rate = 0xffff * 
>> 1e9 /
>>  > period? Otherwise you get bad configurations if rate is 
>> considerable
>>  > slower than necessary.
>> 
>>  The algorithm will start with 'rate' being the parent clock's rate, 
>> which
>>  will always be the highest rate that the child clock will support.
> 
> Ah right, I missed that bit.

Thanks,
-Paul