linux-kernel - Re: [PATCH v11 06/12] pwm: imx27: Use 64-bit division macro and function

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 3 Apr 2020 10:37:19 -0700
From:   Guru Das Srinagesh <gurus@...eaurora.org>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Thierry Reding <thierry.reding@...il.com>,
        Linux PWM List <linux-pwm@...r.kernel.org>,
        Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>,
        Subbaraman Narayanamurthy <subbaram@...eaurora.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Shawn Guo <shawnguo@...nel.org>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        Pengutronix Kernel Team <kernel@...gutronix.de>,
        Fabio Estevam <festevam@...il.com>,
        NXP Linux Team <linux-imx@....com>,
        David Collins <collinsd@...eaurora.org>
Subject: Re: [PATCH v11 06/12] pwm: imx27: Use 64-bit division macro and
 function

On Thu, Apr 02, 2020 at 11:16:22PM +0200, Arnd Bergmann wrote:
> This looks correct, but very expensive, and you don't really have to
> go this far, given that c1 is guaranteed to be a 32-bit number, and
> you divide by a constant in the end.
> 
> Why not do something like
> 
> #define SHIFT 41 /* arbitrarily picked, not too big, not too small */
> #define MUL 2199 /* 2^SHIFT / NSEC_PER_SEC */
> period_cycles = clk_get_rate(imx->clk_per) * ((state->period * MUL) >> SHIFT);

I have two concerns with this:

1. This actually results in the division by 1000010575.5125057 instead
   of NSECS_PER_SEC whereas both the existing as well as the proposed logic
   divide exactly by NSECS_PER_SEC.
2. What method shall be used to pick the SHIFT value? How is this to be
   chosen?

Also, this seems sort of similar to my initial attempt at this
problem, where period was being pre-divided prior to the multiplication,
which was (rightly) NACKed.

	c *= div_u64(state->period, 1000000000);

Thank you.

Guru Das.