From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Rivshin Subject: Re: [PATCH 2/2] gpio: omap: compute debounce-time from actual debounce-clock rate Date: Fri, 17 Mar 2017 19:14:19 -0400 Message-ID: <20170317191419.021bdd84.drivshin@awxrd.com> References: <20170317005704.11971-1-drivshin@awxrd.com> <20170317005704.11971-3-drivshin@awxrd.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Grygorii Strashko Cc: Alexandre Courbot , Kevin Hilman , Linus Walleij , linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org, Santosh Shilimkar , linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org List-Id: linux-gpio@vger.kernel.org On Fri, 17 Mar 2017 14:43:02 -0500 Grygorii Strashko wrote: > On 03/16/2017 07:57 PM, David Rivshin wrote: > > From: David Rivshin > > > > 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: # 4.3+ > > Signed-off-by: David Rivshin > > --- > > > > 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 > > - * = (GPIO_DEBOUNCINGTIME[7:0].DEBOUNCETIME + 1) x 31 > > + * OMAP's debounce time is in 1/DBCK steps > > + * = (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; > > } > > > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751208AbdCQXQq (ORCPT ); Fri, 17 Mar 2017 19:16:46 -0400 Received: from out1-smtp.messagingengine.com ([66.111.4.25]:59335 "EHLO out1-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751061AbdCQXQn (ORCPT ); Fri, 17 Mar 2017 19:16:43 -0400 X-ME-Sender: X-Sasl-enc: Ka7AVR7ZqNxQxNXbm+z0YKollXYNuzdSqqjio6zgVzO+ 1489792461 Date: Fri, 17 Mar 2017 19:14:19 -0400 From: David Rivshin To: Grygorii Strashko Cc: , , Santosh Shilimkar , Kevin Hilman , Linus Walleij , Alexandre Courbot , , Subject: Re: [PATCH 2/2] gpio: omap: compute debounce-time from actual debounce-clock rate Message-ID: <20170317191419.021bdd84.drivshin@awxrd.com> In-Reply-To: References: <20170317005704.11971-1-drivshin@awxrd.com> <20170317005704.11971-3-drivshin@awxrd.com> Organization: Allworx X-Mailer: Claws Mail 3.14.1 (GTK+ 2.24.31; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 17 Mar 2017 14:43:02 -0500 Grygorii Strashko wrote: > On 03/16/2017 07:57 PM, David Rivshin wrote: > > From: David Rivshin > > > > 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: # 4.3+ > > Signed-off-by: David Rivshin > > --- > > > > 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 > > - * = (GPIO_DEBOUNCINGTIME[7:0].DEBOUNCETIME + 1) x 31 > > + * OMAP's debounce time is in 1/DBCK steps > > + * = (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; > > } > > > From mboxrd@z Thu Jan 1 00:00:00 1970 From: drivshin@awxrd.com (David Rivshin) Date: Fri, 17 Mar 2017 19:14:19 -0400 Subject: [PATCH 2/2] gpio: omap: compute debounce-time from actual debounce-clock rate In-Reply-To: References: <20170317005704.11971-1-drivshin@awxrd.com> <20170317005704.11971-3-drivshin@awxrd.com> Message-ID: <20170317191419.021bdd84.drivshin@awxrd.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Fri, 17 Mar 2017 14:43:02 -0500 Grygorii Strashko wrote: > On 03/16/2017 07:57 PM, David Rivshin wrote: > > From: David Rivshin > > > > 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: # 4.3+ > > Signed-off-by: David Rivshin > > --- > > > > 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 > > - * = (GPIO_DEBOUNCINGTIME[7:0].DEBOUNCETIME + 1) x 31 > > + * OMAP's debounce time is in 1/DBCK steps > > + * = (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; > > } > > >