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: <20190105212711.s765knwwerceytvk@pengutronix.de>
Date:   Sat, 5 Jan 2019 22:27:11 +0100
From:   Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>
To:     Paul Cercueil <paul@...pouillou.net>
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

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.

> > 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.
 
Best regards
Uwe

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