[PATCH] gpio: omap: fix debounce time calculation

[PATCH] gpio: omap: fix debounce time calculation

[Felipe Balbi]

According to TRM, debounce is measured in periods of
the functional clock of the GPIO IP. This means that
we should divide by the rate of functional clock.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/gpio/gpio-omap.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 56d2d026e62e..2b29fd195521 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -217,15 +217,29 @@ static void omap2_set_gpio_debounce(struct gpio_bank *bank, unsigned offset,
 	u32			val;
 	u32			l;
 	bool			enable = !!debounce;
+	unsigned long		flags;
 
 	if (!bank->dbck_flag)
 		return;
 
 	if (enable) {
-		debounce = DIV_ROUND_UP(debounce, 31) - 1;
+		struct clk	*clk;
+		unsigned long	rate;
+
+		clk = clk_get(bank->dev, "fck");
+		if (IS_ERR(clk)) {
+			dev_err(bank->dev, "can't get clock\n");
+			return;
+		}
+
+		rate = clk_get_rate(clk);
+		clk_put(clk);
+
+		debounce = DIV_ROUND_UP(debounce, rate);
 		debounce &= OMAP4_GPIO_DEBOUNCINGTIME_MASK;
 	}
 
+	raw_spin_lock_irqsave(&bank->lock, flags);
 	l = BIT(offset);
 
 	clk_enable(bank->dbck);
@@ -256,6 +270,7 @@ static void omap2_set_gpio_debounce(struct gpio_bank *bank, unsigned offset,
 		bank->context.debounce = debounce;
 		bank->context.debounce_en = val;
 	}
+	raw_spin_unlock_irqrestore(&bank->lock, flags);
 }
 
 /**
@@ -1002,14 +1017,9 @@ static int omap_gpio_output(struct gpio_chip *chip, unsigned offset, int value)
 static int omap_gpio_debounce(struct gpio_chip *chip, unsigned offset,
 			      unsigned debounce)
 {
-	struct gpio_bank *bank;
-	unsigned long flags;
-
-	bank = container_of(chip, struct gpio_bank, chip);
+	struct gpio_bank *bank = container_of(chip, struct gpio_bank, chip);
 
-	raw_spin_lock_irqsave(&bank->lock, flags);
 	omap2_set_gpio_debounce(bank, offset, debounce);
-	raw_spin_unlock_irqrestore(&bank->lock, flags);
 
 	return 0;
 }
-- 
2.6.2

Re: [PATCH] gpio: omap: fix debounce time calculation

[Grygorii Strashko]

On 11/12/2015 07:50 PM, Felipe Balbi wrote:
> According to TRM, debounce is measured in periods of
> the functional clock of the GPIO IP. This means that


What TRM? link pls.

http://www.ti.com/lit/ug/spruhl7d/spruhl7d.pdf

28.4.1.24 GPIO_DEBOUNCINGTIME Register (offset = 154h) [reset = 0h]

The GPIO_DEBOUNCINGTIME register controls debouncing time (the value is
global for all ports). The debouncing cell is running with the
debouncing clock (32 kHz), this register represents the number of the
clock cycle(s) (31 s long) to be used.

Debouncing Value in 31 microsecond steps.
Debouncing Value = (DEBOUNCETIME + 1) * 31 microseconds.

> we should divide by the rate of functional clock.
> 
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> ---
>   drivers/gpio/gpio-omap.c | 24 +++++++++++++++++-------
>   1 file changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> index 56d2d026e62e..2b29fd195521 100644
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -217,15 +217,29 @@ static void omap2_set_gpio_debounce(struct gpio_bank *bank, unsigned offset,
>   	u32			val;
>   	u32			l;
>   	bool			enable = !!debounce;
> +	unsigned long		flags;
>   
>   	if (!bank->dbck_flag)
>   		return;
>   
>   	if (enable) {
> -		debounce = DIV_ROUND_UP(debounce, 31) - 1;
> +		struct clk	*clk;
> +		unsigned long	rate;
> +
> +		clk = clk_get(bank->dev, "fck");
> +		if (IS_ERR(clk)) {
> +			dev_err(bank->dev, "can't get clock\n");
> +			return;
> +		}
> +
> +		rate = clk_get_rate(clk);
> +		clk_put(clk);
> +
> +		debounce = DIV_ROUND_UP(debounce, rate);
>   		debounce &= OMAP4_GPIO_DEBOUNCINGTIME_MASK;
>   	}
>   
> +	raw_spin_lock_irqsave(&bank->lock, flags);
>   	l = BIT(offset);
>   
>   	clk_enable(bank->dbck);
> @@ -256,6 +270,7 @@ static void omap2_set_gpio_debounce(struct gpio_bank *bank, unsigned offset,
>   		bank->context.debounce = debounce;
>   		bank->context.debounce_en = val;
>   	}
> +	raw_spin_unlock_irqrestore(&bank->lock, flags);
>   }
>   
>   /**
> @@ -1002,14 +1017,9 @@ static int omap_gpio_output(struct gpio_chip *chip, unsigned offset, int value)
>   static int omap_gpio_debounce(struct gpio_chip *chip, unsigned offset,
>   			      unsigned debounce)
>   {
> -	struct gpio_bank *bank;
> -	unsigned long flags;
> -
> -	bank = container_of(chip, struct gpio_bank, chip);
> +	struct gpio_bank *bank = container_of(chip, struct gpio_bank, chip);
>   
> -	raw_spin_lock_irqsave(&bank->lock, flags);
>   	omap2_set_gpio_debounce(bank, offset, debounce);
> -	raw_spin_unlock_irqrestore(&bank->lock, flags);
>   
>   	return 0;
>   }
> 


-- 
regards,
-grygorii

Re: [PATCH] gpio: omap: fix debounce time calculation

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1042 bytes --]


Hi,

Grygorii Strashko <grygorii.strashko@ti.com> writes:
> On 11/12/2015 07:50 PM, Felipe Balbi wrote:
>> According to TRM, debounce is measured in periods of
>> the functional clock of the GPIO IP. This means that
>
>
> What TRM? link pls.
>
> http://www.ti.com/lit/ug/spruhl7d/spruhl7d.pdf
>
> 28.4.1.24 GPIO_DEBOUNCINGTIME Register (offset = 154h) [reset = 0h]
>
> The GPIO_DEBOUNCINGTIME register controls debouncing time (the value is
> global for all ports). The debouncing cell is running with the
> debouncing clock (32 kHz), this register represents the number of the
> clock cycle(s) (31 s long) to be used.
>
> Debouncing Value in 31 microsecond steps.
> Debouncing Value = (DEBOUNCETIME + 1) * 31 microseconds.

DRA7xx:

"
8-bit values specifying the debouncing time. It is n-
periods of the muxed clock, which can come from either
a true 32k oscillator/pad of from the system clock. It
depends on which boot mode is selected. For more
information see Chapter 32, Initialization.
"

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH] gpio: omap: fix debounce time calculation

[Grygorii Strashko]

On 11/12/2015 08:09 PM, Felipe Balbi wrote:
>
> Hi,
>
> Grygorii Strashko <grygorii.strashko@ti.com> writes:
>> On 11/12/2015 07:50 PM, Felipe Balbi wrote:
>>> According to TRM, debounce is measured in periods of
>>> the functional clock of the GPIO IP. This means that
>>
>>
>> What TRM? link pls.
>>
>> http://www.ti.com/lit/ug/spruhl7d/spruhl7d.pdf
>>
>> 28.4.1.24 GPIO_DEBOUNCINGTIME Register (offset = 154h) [reset = 0h]
>>
>> The GPIO_DEBOUNCINGTIME register controls debouncing time (the value is
>> global for all ports). The debouncing cell is running with the
>> debouncing clock (32 kHz), this register represents the number of the
>> clock cycle(s) (31 s long) to be used.
>>
>> Debouncing Value in 31 microsecond steps.
>> Debouncing Value = (DEBOUNCETIME + 1) * 31 microseconds.
>
> DRA7xx:
>
> "
> 8-bit values specifying the debouncing time. It is n-
> periods of the muxed clock, which can come from either
> a true 32k oscillator/pad of from the system clock. It
> depends on which boot mode is selected. For more
> information see Chapter 32, Initialization.
> "
>

See
http://www.ti.com/lit/ug/spruhz6d/spruhz6d.pdf
27.4.3 General-Purpose Interface Clock Configuration
27.4.3.1 Clocking

This completely unclear. Sry, I think this patch can't be used as is,
first of all because of backward compatibility issues.

-- 
regards,
-grygorii

Re: [PATCH] gpio: omap: fix debounce time calculation

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1633 bytes --]


Hi,

Grygorii Strashko <grygorii.strashko@ti.com> writes:
> On 11/12/2015 08:09 PM, Felipe Balbi wrote:
>>
>> Hi,
>>
>> Grygorii Strashko <grygorii.strashko@ti.com> writes:
>>> On 11/12/2015 07:50 PM, Felipe Balbi wrote:
>>>> According to TRM, debounce is measured in periods of
>>>> the functional clock of the GPIO IP. This means that
>>>
>>>
>>> What TRM? link pls.
>>>
>>> http://www.ti.com/lit/ug/spruhl7d/spruhl7d.pdf
>>>
>>> 28.4.1.24 GPIO_DEBOUNCINGTIME Register (offset = 154h) [reset = 0h]
>>>
>>> The GPIO_DEBOUNCINGTIME register controls debouncing time (the value is
>>> global for all ports). The debouncing cell is running with the
>>> debouncing clock (32 kHz), this register represents the number of the
>>> clock cycle(s) (31 s long) to be used.
>>>
>>> Debouncing Value in 31 microsecond steps.
>>> Debouncing Value = (DEBOUNCETIME + 1) * 31 microseconds.
>>
>> DRA7xx:
>>
>> "
>> 8-bit values specifying the debouncing time. It is n-
>> periods of the muxed clock, which can come from either
>> a true 32k oscillator/pad of from the system clock. It
>> depends on which boot mode is selected. For more
>> information see Chapter 32, Initialization.
>> "
>>
>
> See
> http://www.ti.com/lit/ug/spruhz6d/spruhz6d.pdf
> 27.4.3 General-Purpose Interface Clock Configuration
> 27.4.3.1 Clocking
>
> This completely unclear. Sry, I think this patch can't be used as is,
> first of all because of backward compatibility issues.

yeah, might be. No issues, I'll just go dig older TRMs and trying to
figure this one out. Meanwhile, let's let's keep it as is.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH] gpio: omap: fix debounce time calculation

[santosh shilimkar]

On 11/12/2015 11:33 AM, Felipe Balbi wrote:
>
> Hi,
>
> Grygorii Strashko <grygorii.strashko@ti.com> writes:
>> On 11/12/2015 08:09 PM, Felipe Balbi wrote:
>>>
>>> Hi,
>>>
>>> Grygorii Strashko <grygorii.strashko@ti.com> writes:
>>>> On 11/12/2015 07:50 PM, Felipe Balbi wrote:
>>>>> According to TRM, debounce is measured in periods of
>>>>> the functional clock of the GPIO IP. This means that
>>>>
>>>>
>>>> What TRM? link pls.
>>>>
>>>> http://www.ti.com/lit/ug/spruhl7d/spruhl7d.pdf
>>>>
>>>> 28.4.1.24 GPIO_DEBOUNCINGTIME Register (offset = 154h) [reset = 0h]
>>>>
>>>> The GPIO_DEBOUNCINGTIME register controls debouncing time (the value is
>>>> global for all ports). The debouncing cell is running with the
>>>> debouncing clock (32 kHz), this register represents the number of the
>>>> clock cycle(s) (31 s long) to be used.
>>>>
>>>> Debouncing Value in 31 microsecond steps.
>>>> Debouncing Value = (DEBOUNCETIME + 1) * 31 microseconds.
>>>
>>> DRA7xx:
>>>
>>> "
>>> 8-bit values specifying the debouncing time. It is n-
>>> periods of the muxed clock, which can come from either
>>> a true 32k oscillator/pad of from the system clock. It
>>> depends on which boot mode is selected. For more
>>> information see Chapter 32, Initialization.
>>> "
>>>
>>
>> See
>> http://www.ti.com/lit/ug/spruhz6d/spruhz6d.pdf
>> 27.4.3 General-Purpose Interface Clock Configuration
>> 27.4.3.1 Clocking
>>
>> This completely unclear. Sry, I think this patch can't be used as is,
>> first of all because of backward compatibility issues.
>
> yeah, might be. No issues, I'll just go dig older TRMs and trying to
> figure this one out. Meanwhile, let's let's keep it as is.
>
Just to be clear, being a common GPIO driver, it can't break
backward compatibility. So NAK for current patch.
If needed, please setup different debounce functions based
on GPIO IP numbers if it makes is efficient/accurate.

Regards,
Santosh