Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1384938 > unrolled thread

[PATCH V4 1/4] gpio: tegra: Don't open code of_device_get_match_data()

Started byLaxman Dewangan <ldewangan@nvidia.com>
First post2016-04-22 12:30 +0200
Last post2016-04-22 12:30 +0200
Articles 6 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH V4 1/4] gpio: tegra: Don't open code of_device_get_match_data() Laxman Dewangan <ldewangan@nvidia.com> - 2016-04-22 12:30 +0200
    [PATCH V4 4/4] gpio: tegra: Add support for gpio debounce Laxman Dewangan <ldewangan@nvidia.com> - 2016-04-22 12:30 +0200
      Re: [PATCH V4 4/4] gpio: tegra: Add support for gpio debounce Stephen Warren <swarren@wwwdotorg.org> - 2016-04-22 22:00 +0200
      Re: [PATCH V4 4/4] gpio: tegra: Add support for gpio debounce Alexandre Courbot <gnurou@gmail.com> - 2016-04-25 07:40 +0200
        Re: [PATCH V4 4/4] gpio: tegra: Add support for gpio debounce Laxman Dewangan <ldewangan@nvidia.com> - 2016-04-25 11:00 +0200
    [PATCH V4 2/4] gpio: tegra: Make of_device_id compatible data to constant Laxman Dewangan <ldewangan@nvidia.com> - 2016-04-22 12:30 +0200

#1384938 — [PATCH V4 1/4] gpio: tegra: Don't open code of_device_get_match_data()

FromLaxman Dewangan <ldewangan@nvidia.com>
Date2016-04-22 12:30 +0200
Subject[PATCH V4 1/4] gpio: tegra: Don't open code of_device_get_match_data()
Message-ID<rqGvw-qK-23@gated-at.bofh.it>
Use of_device_get_match_data() for getting matched data
instead of implementing this locally.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
Reviewed-by: Stephen Warren <swarren@nvidia.com>
Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
Acked-by: Thierry Reding <treding@nvidia.com>

---
Collected reviewed/ack by from Stephen, Alexandre and Thieery.

 drivers/gpio/gpio-tegra.c | 50 +++++++++++++++++++++++------------------------
 1 file changed, 24 insertions(+), 26 deletions(-)

diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c
index 790bb11..1b0c497 100644
--- a/drivers/gpio/gpio-tegra.c
+++ b/drivers/gpio/gpio-tegra.c
@@ -75,6 +75,11 @@ struct tegra_gpio_bank {
 #endif
 };
 
+struct tegra_gpio_soc_config {
+	u32 bank_stride;
+	u32 upper_offset;
+};
+
 static struct device *dev;
 static struct irq_domain *irq_domain;
 static void __iomem *regs;
@@ -445,27 +450,6 @@ static const struct dev_pm_ops tegra_gpio_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(tegra_gpio_suspend, tegra_gpio_resume)
 };
 
-struct tegra_gpio_soc_config {
-	u32 bank_stride;
-	u32 upper_offset;
-};
-
-static struct tegra_gpio_soc_config tegra20_gpio_config = {
-	.bank_stride = 0x80,
-	.upper_offset = 0x800,
-};
-
-static struct tegra_gpio_soc_config tegra30_gpio_config = {
-	.bank_stride = 0x100,
-	.upper_offset = 0x80,
-};
-
-static const struct of_device_id tegra_gpio_of_match[] = {
-	{ .compatible = "nvidia,tegra30-gpio", .data = &tegra30_gpio_config },
-	{ .compatible = "nvidia,tegra20-gpio", .data = &tegra20_gpio_config },
-	{ },
-};
-
 /* This lock class tells lockdep that GPIO irqs are in a different
  * category than their parents, so it won't report false recursion.
  */
@@ -473,8 +457,7 @@ static struct lock_class_key gpio_lock_class;
 
 static int tegra_gpio_probe(struct platform_device *pdev)
 {
-	const struct of_device_id *match;
-	struct tegra_gpio_soc_config *config;
+	const struct tegra_gpio_soc_config *config;
 	struct resource *res;
 	struct tegra_gpio_bank *bank;
 	int ret;
@@ -484,12 +467,11 @@ static int tegra_gpio_probe(struct platform_device *pdev)
 
 	dev = &pdev->dev;
 
-	match = of_match_device(tegra_gpio_of_match, &pdev->dev);
-	if (!match) {
+	config = of_device_get_match_data(&pdev->dev);
+	if (!config) {
 		dev_err(&pdev->dev, "Error: No device match found\n");
 		return -ENODEV;
 	}
-	config = (struct tegra_gpio_soc_config *)match->data;
 
 	tegra_gpio_bank_stride = config->bank_stride;
 	tegra_gpio_upper_offset = config->upper_offset;
@@ -578,6 +560,22 @@ static int tegra_gpio_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static struct tegra_gpio_soc_config tegra20_gpio_config = {
+	.bank_stride = 0x80,
+	.upper_offset = 0x800,
+};
+
+static struct tegra_gpio_soc_config tegra30_gpio_config = {
+	.bank_stride = 0x100,
+	.upper_offset = 0x80,
+};
+
+static const struct of_device_id tegra_gpio_of_match[] = {
+	{ .compatible = "nvidia,tegra30-gpio", .data = &tegra30_gpio_config },
+	{ .compatible = "nvidia,tegra20-gpio", .data = &tegra20_gpio_config },
+	{ },
+};
+
 static struct platform_driver tegra_gpio_driver = {
 	.driver		= {
 		.name	= "tegra-gpio",
-- 
2.1.4

[toc] | [next] | [standalone]


#1384939 — [PATCH V4 4/4] gpio: tegra: Add support for gpio debounce

FromLaxman Dewangan <ldewangan@nvidia.com>
Date2016-04-22 12:30 +0200
Subject[PATCH V4 4/4] gpio: tegra: Add support for gpio debounce
Message-ID<rqGvw-qK-27@gated-at.bofh.it>
In reply to#1384938
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 for debounce when SoC support debounce.

Changes from V3:
- Add locking mechanism in debounce count register update.
- Move DBC register from prev patch to here.

 drivers/gpio/gpio-tegra.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 68 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c
index 6af5eb2..45d80ec 100644
--- a/drivers/gpio/gpio-tegra.c
+++ b/drivers/gpio/gpio-tegra.c
@@ -46,10 +46,13 @@
 #define GPIO_INT_ENB(t, x)	(GPIO_REG(t, x) + 0x50)
 #define GPIO_INT_LVL(t, x)	(GPIO_REG(t, x) + 0x60)
 #define GPIO_INT_CLR(t, x)	(GPIO_REG(t, x) + 0x70)
+#define GPIO_DBC_CNT(t, x)	(GPIO_REG(t, x) + 0xF0)
+
 
 #define GPIO_MSK_CNF(t, x)	(GPIO_REG(t, x) + t->soc->upper_offset + 0x00)
 #define GPIO_MSK_OE(t, x)	(GPIO_REG(t, x) + t->soc->upper_offset + 0x10)
 #define GPIO_MSK_OUT(t, x)	(GPIO_REG(t, x) + t->soc->upper_offset + 0X20)
+#define GPIO_MSK_DBC_EN(t, x)	(GPIO_REG(t, x) + t->soc->upper_offset + 0x30)
 #define GPIO_MSK_INT_STA(t, x)	(GPIO_REG(t, x) + t->soc->upper_offset + 0x40)
 #define GPIO_MSK_INT_ENB(t, x)	(GPIO_REG(t, x) + t->soc->upper_offset + 0x50)
 #define GPIO_MSK_INT_LVL(t, x)	(GPIO_REG(t, x) + t->soc->upper_offset + 0x60)
@@ -67,6 +70,7 @@ struct tegra_gpio_bank {
 	int bank;
 	int irq;
 	spinlock_t lvl_lock[4];
+	spinlock_t dbc_lock[4];	/* Lock for updating debounce count register */
 #ifdef CONFIG_PM_SLEEP
 	u32 cnf[4];
 	u32 out[4];
@@ -74,11 +78,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;
 };
@@ -182,6 +189,38 @@ 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);
+	unsigned long flags;
+	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.
+	 */
+	spin_lock_irqsave(&tgi->bank_info[bank].dbc_lock[port], flags);
+	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;
+	}
+	spin_unlock_irqrestore(&tgi->bank_info[bank].dbc_lock[port], flags);
+
+	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);
@@ -197,6 +236,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,
 };
@@ -360,6 +400,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],
@@ -395,6 +443,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,
@@ -547,6 +602,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)
@@ -607,8 +665,10 @@ static int tegra_gpio_probe(struct platform_device *pdev)
 		irq_set_chained_handler_and_data(bank->irq,
 						 tegra_gpio_irq_handler, bank);
 
-		for (j = 0; j < 4; j++)
+		for (j = 0; j < 4; j++) {
 			spin_lock_init(&bank->lvl_lock[j]);
+			spin_lock_init(&bank->dbc_lock[j]);
+		}
 	}
 
 	tegra_gpio_debuginit(tgi);
@@ -626,7 +686,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 },
 	{ },
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]


#1385464 — Re: [PATCH V4 4/4] gpio: tegra: Add support for gpio debounce

FromStephen Warren <swarren@wwwdotorg.org>
Date2016-04-22 22:00 +0200
SubjectRe: [PATCH V4 4/4] gpio: tegra: Add support for gpio debounce
Message-ID<rqPp8-7ph-1@gated-at.bofh.it>
In reply to#1384939
On 04/22/2016 04:09 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.

Reviewed-by: Stephen Warren <swarren@nvidia.com>

[toc] | [prev] | [next] | [standalone]


#1386023 — Re: [PATCH V4 4/4] gpio: tegra: Add support for gpio debounce

FromAlexandre Courbot <gnurou@gmail.com>
Date2016-04-25 07:40 +0200
SubjectRe: [PATCH V4 4/4] gpio: tegra: Add support for gpio debounce
Message-ID<rrHpw-u5-9@gated-at.bofh.it>
In reply to#1384939
Sorry, just realized I commented on v3...

On Fri, Apr 22, 2016 at 7:09 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 for debounce when SoC support debounce.
>
> Changes from V3:
> - Add locking mechanism in debounce count register update.
> - Move DBC register from prev patch to here.
>
>  drivers/gpio/gpio-tegra.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 68 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c
> index 6af5eb2..45d80ec 100644
> --- a/drivers/gpio/gpio-tegra.c
> +++ b/drivers/gpio/gpio-tegra.c
> @@ -46,10 +46,13 @@
>  #define GPIO_INT_ENB(t, x)     (GPIO_REG(t, x) + 0x50)
>  #define GPIO_INT_LVL(t, x)     (GPIO_REG(t, x) + 0x60)
>  #define GPIO_INT_CLR(t, x)     (GPIO_REG(t, x) + 0x70)
> +#define GPIO_DBC_CNT(t, x)     (GPIO_REG(t, x) + 0xF0)
> +
>
>  #define GPIO_MSK_CNF(t, x)     (GPIO_REG(t, x) + t->soc->upper_offset + 0x00)
>  #define GPIO_MSK_OE(t, x)      (GPIO_REG(t, x) + t->soc->upper_offset + 0x10)
>  #define GPIO_MSK_OUT(t, x)     (GPIO_REG(t, x) + t->soc->upper_offset + 0X20)
> +#define GPIO_MSK_DBC_EN(t, x)  (GPIO_REG(t, x) + t->soc->upper_offset + 0x30)
>  #define GPIO_MSK_INT_STA(t, x) (GPIO_REG(t, x) + t->soc->upper_offset + 0x40)
>  #define GPIO_MSK_INT_ENB(t, x) (GPIO_REG(t, x) + t->soc->upper_offset + 0x50)
>  #define GPIO_MSK_INT_LVL(t, x) (GPIO_REG(t, x) + t->soc->upper_offset + 0x60)
> @@ -67,6 +70,7 @@ struct tegra_gpio_bank {
>         int bank;
>         int irq;
>         spinlock_t lvl_lock[4];
> +       spinlock_t dbc_lock[4]; /* Lock for updating debounce count register */

I'm nit'ing here, but maybe one spinlock shared by all ports would be
enough? (the same would apply to lvl_lock, so feel free to do this as
a separate patch) I don't think we expect *that* many concurrent
accesses, do we?

>  #ifdef CONFIG_PM_SLEEP
>         u32 cnf[4];
>         u32 out[4];
> @@ -74,11 +78,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;
>  };
> @@ -182,6 +189,38 @@ 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);
> +       unsigned long flags;
> +       int port = GPIO_PORT(offset);
> +       int bank = GPIO_BANK(offset);

Maybe declare "bank" as follows:

    struct tegra_gpio_bank *bank = &tgi->bank_info[GPIO_BANK(offset)];

This will allow you to simplify the code that follows:

> +
> +       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.
> +        */
> +       spin_lock_irqsave(&tgi->bank_info[bank].dbc_lock[port], flags);
> +       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;
> +       }
> +       spin_unlock_irqrestore(&tgi->bank_info[bank].dbc_lock[port], flags);

Becomes:

       spin_lock_irqsave(bank->dbc_lock[port], flags);
       if (bank->dbc_cnt[port] < debounce_ms) {
               tegra_gpio_writel(tgi, debounce_ms, GPIO_DBC_CNT(tgi, offset));
               bank->dbc_cnt[port] = debounce_ms;
       }
       spin_unlock_irqrestore(&bank->dbc_lock[port], flags);

Which is nicer to the eyes.

Extra points if you initialize port and bank after we ensure that
debounce_ms is not zero and that their value will actually be used.

> +
> +       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);
> @@ -197,6 +236,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,
>  };
> @@ -360,6 +400,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],
> @@ -395,6 +443,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,
> @@ -547,6 +602,9 @@ static int tegra_gpio_probe(struct platform_device *pdev)
>
>         platform_set_drvdata(pdev, tgi);
>
> +       if (!config->debounce_supported)
> +               tgi->gc->set_debounce = NULL;

Yep, we really want one gpio_chip instance in the tegra_gpio_info
struct, otherwise this kind of limits the purpose of getting rid of
these global variables...

[toc] | [prev] | [next] | [standalone]


#1386136 — Re: [PATCH V4 4/4] gpio: tegra: Add support for gpio debounce

FromLaxman Dewangan <ldewangan@nvidia.com>
Date2016-04-25 11:00 +0200
SubjectRe: [PATCH V4 4/4] gpio: tegra: Add support for gpio debounce
Message-ID<rrKx4-2Lh-29@gated-at.bofh.it>
In reply to#1386023
On Monday 25 April 2016 11:06 AM, Alexandre Courbot wrote:
> Sorry, just realized I commented on v3...
>
> On Fri, Apr 22, 2016 at 7:09 PM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
>> +       spinlock_t dbc_lock[4]; /* Lock for updating debounce count register */
> I'm nit'ing here, but maybe one spinlock shared by all ports would be
> enough? (the same would apply to lvl_lock, so feel free to do this as
> a separate patch) I don't think we expect *that* many concurrent
> accesses, do we?

Really no, but to make the stuff uniform, it should be fine here. If the 
registers are not conflicting then do not make under same lock.


>>   
>>
>>         spin_lock_irqsave(bank->dbc_lock[port], flags);
>>         if (bank->dbc_cnt[port] < debounce_ms) {
>>                 tegra_gpio_writel(tgi, debounce_ms, GPIO_DBC_CNT(tgi, offset));
>>                 bank->dbc_cnt[port] = debounce_ms;
>>         }
>>         spin_unlock_irqrestore(&bank->dbc_lock[port], flags);
>>
>> Which is nicer to the eyes.
>>


OK, this also looks fine. As I am goign to respin this for V5 (for gc as 
instance rather than pointer), I will take care of it.

[toc] | [prev] | [next] | [standalone]


#1384942 — [PATCH V4 2/4] gpio: tegra: Make of_device_id compatible data to constant

FromLaxman Dewangan <ldewangan@nvidia.com>
Date2016-04-22 12:30 +0200
Subject[PATCH V4 2/4] gpio: tegra: Make of_device_id compatible data to constant
Message-ID<rqGvw-qK-33@gated-at.bofh.it>
In reply to#1384938
The data member of the of_device_id is the constant type
and hence all static structure which is used for this
initialisation as static.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
Suggested-by: Thierry Reding <treding@nvidia.com>
Reviewed-by: Stephen Warren <swarren@nvidia.com>

---
Changes from V2:
-This is new in series based on discussion on previous patches.

Changes from V3:
- Collected RB of Stephen.

 drivers/gpio/gpio-tegra.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c
index 1b0c497..cd69422 100644
--- a/drivers/gpio/gpio-tegra.c
+++ b/drivers/gpio/gpio-tegra.c
@@ -560,12 +560,12 @@ static int tegra_gpio_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static struct tegra_gpio_soc_config tegra20_gpio_config = {
+static const struct tegra_gpio_soc_config tegra20_gpio_config = {
 	.bank_stride = 0x80,
 	.upper_offset = 0x800,
 };
 
-static struct tegra_gpio_soc_config tegra30_gpio_config = {
+static const struct tegra_gpio_soc_config tegra30_gpio_config = {
 	.bank_stride = 0x100,
 	.upper_offset = 0x80,
 };
-- 
2.1.4

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web