linux-kernel - Re: [PATCH 2/2] gpio: omap: compute debounce-time from actual debounce-clock rate

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-next>] [day] [month] [year] [list]

Date:   Fri, 17 Mar 2017 19:14:19 -0400
From:   David Rivshin <drivshin@...rd.com>
To:     Grygorii Strashko <grygorii.strashko@...com>
Cc:     <linux-gpio@...r.kernel.org>, <linux-omap@...r.kernel.org>,
        Santosh Shilimkar <ssantosh@...nel.org>,
        Kevin Hilman <khilman@...nel.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        Alexandre Courbot <gnurou@...il.com>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] gpio: omap: compute debounce-time from actual
 debounce-clock rate

On Fri, 17 Mar 2017 14:43:02 -0500
Grygorii Strashko <grygorii.strashko@...com> wrote:

> On 03/16/2017 07:57 PM, David Rivshin wrote:
> > From: David Rivshin <DRivshin@...worx.com>
> > 
> > omap2_set_gpio_debounce() assumes the debounce clock runs at 32768Hz,
> > leading to 31us granularity. In reality the debounce clock (which
> > is provided by other modules) could be at different rate, leading to
> > an incorrect computation of the number of debounce clock cycles for
> > GPIO_DEBOUNCINGTIME[DEBOUNCETIME].
> > 
> > Also, even with a standard 32768Hz input clock, the actual granularity
> > is ~30.5us. This leads to the actual debounce time being ~1.5% too
> > short.
> > 
> > Fix both issues by simply querying the dbck rate, rather than
> > hardcoding.  
> 
> Pls, hold on with this - I'm trying to check it, as it doesn't follow TRMs.

Yes, the TRMs are somewhat cryptic about the details, perhaps I can give
some more background on how I came to this. I think the chapters for the 
GPIO block are written with a strong assumption that the input debounce 
clock is 32768Hz, and further round 1/32768Hz to the nearest whole 
microsecond (~30.5us->31us). 

However, at least on AM335x the debounce clock (i.e. the CLK_32KHZ) can 
be made to run at other rates via the Control Module.

Here is an example from clk_summary:
 clk_24mhz                    1            1    24000000          0 0
    clkdiv32k_ck              1            1       65536          0 0
       clkdiv32k_ick          2            6       65536          0 0
          timer7_fck          1            1       65536          0 0
          wdt1_fck            0            1       65536          0 0
          gpio3_dbclk         0            2       65536          0 0
          gpio2_dbclk         0            2       65536          0 0
          gpio1_dbclk         0            2       65536          0 0

This is accomplished by setting clk32kdivratio_ctl[clkdivopp50_en] to 1 
even while in OPP100, and adjusting the devicetree to match. Timer7 is
known to truly have a 65536Hz fck by configuring it as a 50% duty cycle 
PWM and measuring with an oscilloscope.

Here are some relevant sections from the AM335x TRM (spruh73o):
  25.2.2 GPIO Clock and Reset Management
    Debounce Functional clock (GPIO[123]) comes from CLK_32KHZ.
  8.1.6.8 Peripheral PLL Description
    CLK_32KHZ is CLK_24 divided by either 732.4219 or 366.2109.
    In OPP100 this results in either 32768Hz or 65536Hz.
    In OPP50 this results in either 16384Hz or 32768Hz.
  9.3.1.8 clk32kdivratio_ctrl Register
    Names of the register and field differ from 8.1.6.8, but it's
     clear they are the same.
  
I'm not sure if other devices can be similarly cajoled into running
their equivalent 32KHZ clocks at a different rate, but it certainly
works for AM335x. Checking OMAP4430 TRM as an example makes it look 
like OMAP44xx has a fixed 32768Hz clock with no adjustment possible 
(backed up by omap44xx-clocks.dtsi having it just be a fixed-clock).

 
> > 
> > Fixes: e85ec6c3047b ("gpio: omap: fix omap2_set_gpio_debounce")
> > Cc: <stable@...r.kernel.org> # 4.3+
> > Signed-off-by: David Rivshin <drivshin@...worx.com>
> > ---
> > 
> > This logical bug existed before e85ec6c3047b, but if backporting
> > further it's probably best to just cherry-pick/backport e85ec6c3047b
> > first.
> > 
> >  drivers/gpio/gpio-omap.c | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> > index 33ec02d..865a831 100644
> > --- a/drivers/gpio/gpio-omap.c
> > +++ b/drivers/gpio/gpio-omap.c
> > @@ -205,8 +205,8 @@ static inline void omap_gpio_dbck_disable(struct gpio_bank *bank)
> >   * @offset: the gpio number on this @bank
> >   * @debounce: debounce time to use
> >   *
> > - * OMAP's debounce time is in 31us steps
> > - *   <debounce time> = (GPIO_DEBOUNCINGTIME[7:0].DEBOUNCETIME + 1) x 31
> > + * OMAP's debounce time is in 1/DBCK steps
> > + *   <debounce time> = (GPIO_DEBOUNCINGTIME[7:0].DEBOUNCETIME + 1) / DBCK
> >   * so we need to convert and round up to the closest unit.
> >   *
> >   * Return: 0 on success, negative error otherwise.
> > @@ -223,7 +223,9 @@ static int omap2_set_gpio_debounce(struct gpio_bank *bank, unsigned offset,
> >  		return -ENOTSUPP;
> >  
> >  	if (enable) {
> > -		debounce = DIV_ROUND_UP(debounce, 31) - 1;
> > +		u64 tmp = (u64)debounce * clk_get_rate(bank->dbck);
> > +
> > +		debounce = DIV_ROUND_UP_ULL(tmp, 1000000) - 1;
> >  		if ((debounce & OMAP4_GPIO_DEBOUNCINGTIME_MASK) != debounce)
> >  			return -EINVAL;
> >  	}
> >   
>