diff mbox

[V3,4/4] gpio: tegra: Add support for gpio debounce

Message ID 1461159058-1439-5-git-send-email-ldewangan@nvidia.com
State New
Headers show

Commit Message

Laxman Dewangan April 20, 2016, 1:30 p.m. UTC
NVIDIA's Tegra210 support the HW debounce in the GPIO
controller for all its GPIO pins.

Add support for setting debounce timing by implementing the
set_debounce callback of gpiochip.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>

---
Changes from V1:
- Write debounce count before enable.
- Make sure the debounce count do not have any boot residuals.

Changes from V2:
- Only access register fo debounce when SoC support debounce.
---
 drivers/gpio/gpio-tegra.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

Comments

Stephen Warren April 21, 2016, 6:35 p.m. UTC | #1
On 04/20/2016 07:30 AM, Laxman Dewangan wrote:
> NVIDIA's Tegra210 support the HW debounce in the GPIO
> controller for all its GPIO pins.
>
> Add support for setting debounce timing by implementing the
> set_debounce callback of gpiochip.

> diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c

> +static int tegra_gpio_set_debounce(struct gpio_chip *chip, unsigned int offset,

> +	/* There is only one debounce count register per port and hence
> +	 * set the maximum of current and requested debounce time.
> +	 */
> +	if (tgi->bank_info[bank].dbc_cnt[port] < debounce_ms) {
> +		tegra_gpio_writel(tgi, debounce_ms, GPIO_DBC_CNT(tgi, offset));
> +		tgi->bank_info[bank].dbc_cnt[port] = debounce_ms;
> +	}

Do we need any locking there? I imagine the GPIO core doesn't prevent 
different threads/drivers from manipulating different GPIOs in parallel 
on different cores.
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexandre Courbot April 25, 2016, 4:55 a.m. UTC | #2
On Wed, Apr 20, 2016 at 10:30 PM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
> NVIDIA's Tegra210 support the HW debounce in the GPIO
> controller for all its GPIO pins.
>
> Add support for setting debounce timing by implementing the
> set_debounce callback of gpiochip.
>
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
>
> ---
> Changes from V1:
> - Write debounce count before enable.
> - Make sure the debounce count do not have any boot residuals.
>
> Changes from V2:
> - Only access register fo debounce when SoC support debounce.
> ---
>  drivers/gpio/gpio-tegra.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
>
> diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c
> index 36e865f..1f8ec24 100644
> --- a/drivers/gpio/gpio-tegra.c
> +++ b/drivers/gpio/gpio-tegra.c
> @@ -76,11 +76,14 @@ struct tegra_gpio_bank {
>         u32 int_enb[4];
>         u32 int_lvl[4];
>         u32 wake_enb[4];
> +       u32 dbc_enb[4];
>  #endif
> +       u32 dbc_cnt[4];
>         struct tegra_gpio_info *tgi;
>  };
>
>  struct tegra_gpio_soc_config {
> +       bool debounce_supported;
>         u32 bank_stride;
>         u32 upper_offset;
>  };
> @@ -184,6 +187,35 @@ static int tegra_gpio_direction_output(struct gpio_chip *chip, unsigned offset,
>         return 0;
>  }
>
> +static int tegra_gpio_set_debounce(struct gpio_chip *chip, unsigned int offset,
> +                                  unsigned int debounce)
> +{
> +       struct tegra_gpio_info *tgi = gpiochip_get_data(chip);
> +       unsigned int debounce_ms = DIV_ROUND_UP(debounce, 1000);
> +       int port = GPIO_PORT(offset);
> +       int bank = GPIO_BANK(offset);
> +
> +       if (!debounce_ms) {
> +               tegra_gpio_mask_write(tgi, GPIO_MSK_DBC_EN(tgi, offset),
> +                                     offset, 0);
> +               return 0;
> +       }
> +
> +       debounce_ms = min(debounce_ms, 255U);
> +
> +       /* There is only one debounce count register per port and hence
> +        * set the maximum of current and requested debounce time.
> +        */
> +       if (tgi->bank_info[bank].dbc_cnt[port] < debounce_ms) {
> +               tegra_gpio_writel(tgi, debounce_ms, GPIO_DBC_CNT(tgi, offset));
> +               tgi->bank_info[bank].dbc_cnt[port] = debounce_ms;
> +       }
> +
> +       tegra_gpio_mask_write(tgi, GPIO_MSK_DBC_EN(tgi, offset), offset, 1);
> +
> +       return 0;
> +}
> +
>  static int tegra_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
>  {
>         struct tegra_gpio_info *tgi = gpiochip_get_data(chip);
> @@ -199,6 +231,7 @@ static struct gpio_chip tegra_gpio_chip = {
>         .get                    = tegra_gpio_get,
>         .direction_output       = tegra_gpio_direction_output,
>         .set                    = tegra_gpio_set,
> +       .set_debounce           = tegra_gpio_set_debounce,
>         .to_irq                 = tegra_gpio_to_irq,
>         .base                   = 0,
>  };
> @@ -363,6 +396,14 @@ static int tegra_gpio_resume(struct device *dev)
>                         unsigned int gpio = (b<<5) | (p<<3);
>                         tegra_gpio_writel(tgi, bank->cnf[p],
>                                           GPIO_CNF(tgi, gpio));
> +
> +                       if (tgi->soc->debounce_supported) {
> +                               tegra_gpio_writel(tgi, bank->dbc_cnt[p],
> +                                                 GPIO_DBC_CNT(tgi, gpio));
> +                               tegra_gpio_writel(tgi, bank->dbc_enb[p],
> +                                                 GPIO_MSK_DBC_EN(tgi, gpio));
> +                       }
> +
>                         tegra_gpio_writel(tgi, bank->out[p],
>                                           GPIO_OUT(tgi, gpio));
>                         tegra_gpio_writel(tgi, bank->oe[p],
> @@ -398,6 +439,13 @@ static int tegra_gpio_suspend(struct device *dev)
>                                                         GPIO_OUT(tgi, gpio));
>                         bank->oe[p] = tegra_gpio_readl(tgi,
>                                                        GPIO_OE(tgi, gpio));
> +                       if (tgi->soc->debounce_supported) {
> +                               bank->dbc_enb[p] = tegra_gpio_readl(tgi,
> +                                               GPIO_MSK_DBC_EN(tgi, gpio));
> +                               bank->dbc_enb[p] = (bank->dbc_enb[p] << 8) |
> +                                                       bank->dbc_enb[p];
> +                       }
> +
>                         bank->int_enb[p] = tegra_gpio_readl(tgi,
>                                                 GPIO_INT_ENB(tgi, gpio));
>                         bank->int_lvl[p] = tegra_gpio_readl(tgi,
> @@ -550,6 +598,9 @@ static int tegra_gpio_probe(struct platform_device *pdev)
>
>         platform_set_drvdata(pdev, tgi);
>
> +       if (!config->debounce_supported)
> +               tgi->gc->set_debounce = NULL;

This last line is equivalent to doing

     tegra_gpio_chip.set_debounce = NULL

Which means that after that no one can reinstanciate this driver and
use debounce. Granted, this does not happen in real life, but the
purpose of the previous patch that removes all static variables is
supposedly to make this scenario possible. I think you can easily fix
this though: make tgi->gc a non-pointer member, do tgi->gc =
tegra_gpio_chip to copy the initial data, and then set
tgi->gc.set_debounce to NULL if needed.

tegra_gpio_chip can then be made constant, and maybe even __initdata?
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Laxman Dewangan April 25, 2016, 8:29 a.m. UTC | #3
On Monday 25 April 2016 10:25 AM, Alexandre Courbot wrote:
> On Wed, Apr 20, 2016 at 10:30 PM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
>>                       bank->int_lvl[p] = tegra_gpio_readl(tgi,
>> @@ -550,6 +598,9 @@ static int tegra_gpio_probe(struct platform_device *pdev)
>>
>>          platform_set_drvdata(pdev, tgi);
>>
>> +       if (!config->debounce_supported)
>> +               tgi->gc->set_debounce = NULL;
> This last line is equivalent to doing
>
>       tegra_gpio_chip.set_debounce = NULL
>
> Which means that after that no one can reinstanciate this driver and
> use debounce. Granted, this does not happen in real life, but the
> purpose of the previous patch that removes all static variables is
> supposedly to make this scenario possible. I think you can easily fix
> this though: make tgi->gc a non-pointer member, do tgi->gc =
> tegra_gpio_chip to copy the initial data, and then set
> tgi->gc.set_debounce to NULL if needed.

As there is only single instance of the tegra gpio driver, I avoided the 
copy of the tegra_gpio_chip.
I am not expecting multiple instance of this driver and hence should be 
fine. When multiple instances are needed, it can be just use as:

But let me make this as you suggested.


> tegra_gpio_chip can then be made constant, and maybe even __initdata?
I have not seen the usage of __initdata now a days. I think it is 
removed from most of places.
Will use the const.

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c
index 36e865f..1f8ec24 100644
--- a/drivers/gpio/gpio-tegra.c
+++ b/drivers/gpio/gpio-tegra.c
@@ -76,11 +76,14 @@  struct tegra_gpio_bank {
 	u32 int_enb[4];
 	u32 int_lvl[4];
 	u32 wake_enb[4];
+	u32 dbc_enb[4];
 #endif
+	u32 dbc_cnt[4];
 	struct tegra_gpio_info *tgi;
 };
 
 struct tegra_gpio_soc_config {
+	bool debounce_supported;
 	u32 bank_stride;
 	u32 upper_offset;
 };
@@ -184,6 +187,35 @@  static int tegra_gpio_direction_output(struct gpio_chip *chip, unsigned offset,
 	return 0;
 }
 
+static int tegra_gpio_set_debounce(struct gpio_chip *chip, unsigned int offset,
+				   unsigned int debounce)
+{
+	struct tegra_gpio_info *tgi = gpiochip_get_data(chip);
+	unsigned int debounce_ms = DIV_ROUND_UP(debounce, 1000);
+	int port = GPIO_PORT(offset);
+	int bank = GPIO_BANK(offset);
+
+	if (!debounce_ms) {
+		tegra_gpio_mask_write(tgi, GPIO_MSK_DBC_EN(tgi, offset),
+				      offset, 0);
+		return 0;
+	}
+
+	debounce_ms = min(debounce_ms, 255U);
+
+	/* There is only one debounce count register per port and hence
+	 * set the maximum of current and requested debounce time.
+	 */
+	if (tgi->bank_info[bank].dbc_cnt[port] < debounce_ms) {
+		tegra_gpio_writel(tgi, debounce_ms, GPIO_DBC_CNT(tgi, offset));
+		tgi->bank_info[bank].dbc_cnt[port] = debounce_ms;
+	}
+
+	tegra_gpio_mask_write(tgi, GPIO_MSK_DBC_EN(tgi, offset), offset, 1);
+
+	return 0;
+}
+
 static int tegra_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
 {
 	struct tegra_gpio_info *tgi = gpiochip_get_data(chip);
@@ -199,6 +231,7 @@  static struct gpio_chip tegra_gpio_chip = {
 	.get			= tegra_gpio_get,
 	.direction_output	= tegra_gpio_direction_output,
 	.set			= tegra_gpio_set,
+	.set_debounce		= tegra_gpio_set_debounce,
 	.to_irq			= tegra_gpio_to_irq,
 	.base			= 0,
 };
@@ -363,6 +396,14 @@  static int tegra_gpio_resume(struct device *dev)
 			unsigned int gpio = (b<<5) | (p<<3);
 			tegra_gpio_writel(tgi, bank->cnf[p],
 					  GPIO_CNF(tgi, gpio));
+
+			if (tgi->soc->debounce_supported) {
+				tegra_gpio_writel(tgi, bank->dbc_cnt[p],
+						  GPIO_DBC_CNT(tgi, gpio));
+				tegra_gpio_writel(tgi, bank->dbc_enb[p],
+						  GPIO_MSK_DBC_EN(tgi, gpio));
+			}
+
 			tegra_gpio_writel(tgi, bank->out[p],
 					  GPIO_OUT(tgi, gpio));
 			tegra_gpio_writel(tgi, bank->oe[p],
@@ -398,6 +439,13 @@  static int tegra_gpio_suspend(struct device *dev)
 							GPIO_OUT(tgi, gpio));
 			bank->oe[p] = tegra_gpio_readl(tgi,
 						       GPIO_OE(tgi, gpio));
+			if (tgi->soc->debounce_supported) {
+				bank->dbc_enb[p] = tegra_gpio_readl(tgi,
+						GPIO_MSK_DBC_EN(tgi, gpio));
+				bank->dbc_enb[p] = (bank->dbc_enb[p] << 8) |
+							bank->dbc_enb[p];
+			}
+
 			bank->int_enb[p] = tegra_gpio_readl(tgi,
 						GPIO_INT_ENB(tgi, gpio));
 			bank->int_lvl[p] = tegra_gpio_readl(tgi,
@@ -550,6 +598,9 @@  static int tegra_gpio_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, tgi);
 
+	if (!config->debounce_supported)
+		tgi->gc->set_debounce = NULL;
+
 	tgi->bank_info = devm_kzalloc(&pdev->dev, tgi->bank_count *
 				      sizeof(*tgi->bank_info), GFP_KERNEL);
 	if (!tgi->bank_info)
@@ -629,7 +680,14 @@  static const struct tegra_gpio_soc_config tegra30_gpio_config = {
 	.upper_offset = 0x80,
 };
 
+static const struct tegra_gpio_soc_config tegra210_gpio_config = {
+	.debounce_supported = true,
+	.bank_stride = 0x100,
+	.upper_offset = 0x80,
+};
+
 static const struct of_device_id tegra_gpio_of_match[] = {
+	{ .compatible = "nvidia,tegra210-gpio", .data = &tegra210_gpio_config },
 	{ .compatible = "nvidia,tegra30-gpio", .data = &tegra30_gpio_config },
 	{ .compatible = "nvidia,tegra20-gpio", .data = &tegra20_gpio_config },
 	{ },