* [PATCH V2 1/3] gpio: tegra: Don't open code of_device_get_match_data() @ 2016-04-19 9:43 Laxman Dewangan 2016-04-19 9:43 ` [PATCH V2 2/3] gpio: tegra: Remove the need of keeping device handle for gpio driver Laxman Dewangan ` (3 more replies) 0 siblings, 4 replies; 15+ messages in thread From: Laxman Dewangan @ 2016-04-19 9:43 UTC (permalink / raw) To: linus.walleij, gnurou, swarren, thierry.reding Cc: linux-gpio, linux-tegra, linux-kernel, Laxman Dewangan 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> --- Collected reviewed by from Stephen. --- 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH V2 2/3] gpio: tegra: Remove the need of keeping device handle for gpio driver 2016-04-19 9:43 [PATCH V2 1/3] gpio: tegra: Don't open code of_device_get_match_data() Laxman Dewangan @ 2016-04-19 9:43 ` Laxman Dewangan 2016-04-19 12:33 ` Thierry Reding 2016-04-19 9:43 ` [PATCH V2 3/3] gpio: tegra: Add support for gpio debounce Laxman Dewangan ` (2 subsequent siblings) 3 siblings, 1 reply; 15+ messages in thread From: Laxman Dewangan @ 2016-04-19 9:43 UTC (permalink / raw) To: linus.walleij, gnurou, swarren, thierry.reding Cc: linux-gpio, linux-tegra, linux-kernel, Laxman Dewangan Remove the file static device handle variable for keeping device handle of driver as this is just required for error prints. The required device handle are available from gpiochip structure. Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com> --- drivers/gpio/gpio-tegra.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c index 1b0c497..de022a9 100644 --- a/drivers/gpio/gpio-tegra.c +++ b/drivers/gpio/gpio-tegra.c @@ -80,7 +80,6 @@ struct tegra_gpio_soc_config { u32 upper_offset; }; -static struct device *dev; static struct irq_domain *irq_domain; static void __iomem *regs; static u32 tegra_gpio_bank_count; @@ -240,7 +239,8 @@ static int tegra_gpio_irq_set_type(struct irq_data *d, unsigned int type) ret = gpiochip_lock_as_irq(&tegra_gpio_chip, gpio); if (ret) { - dev_err(dev, "unable to lock Tegra GPIO %d as IRQ\n", gpio); + dev_err(tegra_gpio_chip.parent, + "unable to lock Tegra GPIO %d as IRQ\n", gpio); return ret; } @@ -465,8 +465,6 @@ static int tegra_gpio_probe(struct platform_device *pdev) int i; int j; - dev = &pdev->dev; - config = of_device_get_match_data(&pdev->dev); if (!config) { dev_err(&pdev->dev, "Error: No device match found\n"); @@ -488,6 +486,8 @@ static int tegra_gpio_probe(struct platform_device *pdev) } tegra_gpio_chip.ngpio = tegra_gpio_bank_count * 32; + tegra_gpio_chip.parent = &pdev->dev; + tegra_gpio_banks = devm_kzalloc(&pdev->dev, tegra_gpio_bank_count * sizeof(*tegra_gpio_banks), -- 2.1.4 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH V2 2/3] gpio: tegra: Remove the need of keeping device handle for gpio driver 2016-04-19 9:43 ` [PATCH V2 2/3] gpio: tegra: Remove the need of keeping device handle for gpio driver Laxman Dewangan @ 2016-04-19 12:33 ` Thierry Reding 2016-04-19 12:43 ` Laxman Dewangan 0 siblings, 1 reply; 15+ messages in thread From: Thierry Reding @ 2016-04-19 12:33 UTC (permalink / raw) To: Laxman Dewangan Cc: linus.walleij, gnurou, swarren, linux-gpio, linux-tegra, linux-kernel [-- Attachment #1: Type: text/plain, Size: 2025 bytes --] On Tue, Apr 19, 2016 at 03:13:39PM +0530, Laxman Dewangan wrote: > Remove the file static device handle variable for keeping device handle > of driver as this is just required for error prints. The required device > handle are available from gpiochip structure. > > Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com> > --- > drivers/gpio/gpio-tegra.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c > index 1b0c497..de022a9 100644 > --- a/drivers/gpio/gpio-tegra.c > +++ b/drivers/gpio/gpio-tegra.c > @@ -80,7 +80,6 @@ struct tegra_gpio_soc_config { > u32 upper_offset; > }; > > -static struct device *dev; > static struct irq_domain *irq_domain; > static void __iomem *regs; > static u32 tegra_gpio_bank_count; > @@ -240,7 +239,8 @@ static int tegra_gpio_irq_set_type(struct irq_data *d, unsigned int type) > > ret = gpiochip_lock_as_irq(&tegra_gpio_chip, gpio); > if (ret) { > - dev_err(dev, "unable to lock Tegra GPIO %d as IRQ\n", gpio); > + dev_err(tegra_gpio_chip.parent, > + "unable to lock Tegra GPIO %d as IRQ\n", gpio); Can't we get rid of the tegra_gpio_chip global variable altogether? We set a struct tegra_gpio_bank * as the chip data for each of the interrupts, so if we added a back link to the GPIO chip to each bank we could easily get at the GPIO chip (and its parent device) from the IRQ chip implementation. > return ret; > } > > @@ -465,8 +465,6 @@ static int tegra_gpio_probe(struct platform_device *pdev) > int i; > int j; > > - dev = &pdev->dev; > - > config = of_device_get_match_data(&pdev->dev); > if (!config) { > dev_err(&pdev->dev, "Error: No device match found\n"); > @@ -488,6 +486,8 @@ static int tegra_gpio_probe(struct platform_device *pdev) > } > > tegra_gpio_chip.ngpio = tegra_gpio_bank_count * 32; > + tegra_gpio_chip.parent = &pdev->dev; > + This adds a gratuitous blank line. Thierry [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH V2 2/3] gpio: tegra: Remove the need of keeping device handle for gpio driver 2016-04-19 12:33 ` Thierry Reding @ 2016-04-19 12:43 ` Laxman Dewangan 2016-04-19 16:04 ` Stephen Warren 0 siblings, 1 reply; 15+ messages in thread From: Laxman Dewangan @ 2016-04-19 12:43 UTC (permalink / raw) To: Thierry Reding Cc: linus.walleij, gnurou, swarren, linux-gpio, linux-tegra, linux-kernel On Tuesday 19 April 2016 06:03 PM, Thierry Reding wrote: > * PGP Signed by an unknown key > > On Tue, Apr 19, 2016 at 03:13:39PM +0530, Laxman Dewangan wrote: >> Remove the file static device handle variable for keeping device handle >> of driver as this is just required for error prints. The required device >> handle are available from gpiochip structure. >> >> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com> >> --- >> drivers/gpio/gpio-tegra.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c >> index 1b0c497..de022a9 100644 >> --- a/drivers/gpio/gpio-tegra.c >> +++ b/drivers/gpio/gpio-tegra.c >> @@ -80,7 +80,6 @@ struct tegra_gpio_soc_config { >> u32 upper_offset; >> }; >> >> -static struct device *dev; >> static struct irq_domain *irq_domain; >> static void __iomem *regs; >> static u32 tegra_gpio_bank_count; >> @@ -240,7 +239,8 @@ static int tegra_gpio_irq_set_type(struct irq_data *d, unsigned int type) >> >> ret = gpiochip_lock_as_irq(&tegra_gpio_chip, gpio); >> if (ret) { >> - dev_err(dev, "unable to lock Tegra GPIO %d as IRQ\n", gpio); >> + dev_err(tegra_gpio_chip.parent, >> + "unable to lock Tegra GPIO %d as IRQ\n", gpio); > Can't we get rid of the tegra_gpio_chip global variable altogether? We > set a struct tegra_gpio_bank * as the chip data for each of the > interrupts, so if we added a back link to the GPIO chip to each bank > we could easily get at the GPIO chip (and its parent device) from the > IRQ chip implementation. > We can get rid of all the global variables and can have only one (this is because our REGS macros uses the stride and changing then part of argument is too much change for single patch) as suggested by Stephen. I am planning to have rid of all global variables to on follow on patches once this series is fine. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH V2 2/3] gpio: tegra: Remove the need of keeping device handle for gpio driver 2016-04-19 12:43 ` Laxman Dewangan @ 2016-04-19 16:04 ` Stephen Warren 0 siblings, 0 replies; 15+ messages in thread From: Stephen Warren @ 2016-04-19 16:04 UTC (permalink / raw) To: Laxman Dewangan, Thierry Reding Cc: linus.walleij, gnurou, linux-gpio, linux-tegra, linux-kernel On 04/19/2016 06:43 AM, Laxman Dewangan wrote: > > On Tuesday 19 April 2016 06:03 PM, Thierry Reding wrote: >> * PGP Signed by an unknown key >> >> On Tue, Apr 19, 2016 at 03:13:39PM +0530, Laxman Dewangan wrote: >>> Remove the file static device handle variable for keeping device handle >>> of driver as this is just required for error prints. The required device >>> handle are available from gpiochip structure. >>> >>> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com> >>> --- >>> drivers/gpio/gpio-tegra.c | 8 ++++---- >>> 1 file changed, 4 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c >>> index 1b0c497..de022a9 100644 >>> --- a/drivers/gpio/gpio-tegra.c >>> +++ b/drivers/gpio/gpio-tegra.c >>> @@ -80,7 +80,6 @@ struct tegra_gpio_soc_config { >>> u32 upper_offset; >>> }; >>> -static struct device *dev; >>> static struct irq_domain *irq_domain; >>> static void __iomem *regs; >>> static u32 tegra_gpio_bank_count; >>> @@ -240,7 +239,8 @@ static int tegra_gpio_irq_set_type(struct >>> irq_data *d, unsigned int type) >>> ret = gpiochip_lock_as_irq(&tegra_gpio_chip, gpio); >>> if (ret) { >>> - dev_err(dev, "unable to lock Tegra GPIO %d as IRQ\n", gpio); >>> + dev_err(tegra_gpio_chip.parent, >>> + "unable to lock Tegra GPIO %d as IRQ\n", gpio); >> Can't we get rid of the tegra_gpio_chip global variable altogether? We >> set a struct tegra_gpio_bank * as the chip data for each of the >> interrupts, so if we added a back link to the GPIO chip to each bank >> we could easily get at the GPIO chip (and its parent device) from the >> IRQ chip implementation. >> > > We can get rid of all the global variables and can have only one (this > is because our REGS macros uses the stride and changing then part of > argument is too much change for single patch) as suggested by Stephen. The REGS macro can take a parameter that points to the struct tegra_gpio_chip, which in turn can point at the struct tegra_gpio_soc_config. That doesn't sound like too many parameters/... to me. There's little point getting rid of all-but-one global. ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH V2 3/3] gpio: tegra: Add support for gpio debounce 2016-04-19 9:43 [PATCH V2 1/3] gpio: tegra: Don't open code of_device_get_match_data() Laxman Dewangan 2016-04-19 9:43 ` [PATCH V2 2/3] gpio: tegra: Remove the need of keeping device handle for gpio driver Laxman Dewangan @ 2016-04-19 9:43 ` Laxman Dewangan 2016-04-19 10:43 ` Jon Hunter ` (2 more replies) 2016-04-19 12:06 ` [PATCH V2 1/3] gpio: tegra: Don't open code of_device_get_match_data() Thierry Reding 2016-04-29 8:56 ` Linus Walleij 3 siblings, 3 replies; 15+ messages in thread From: Laxman Dewangan @ 2016-04-19 9:43 UTC (permalink / raw) To: linus.walleij, gnurou, swarren, thierry.reding Cc: linux-gpio, linux-tegra, linux-kernel, Laxman Dewangan 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. --- drivers/gpio/gpio-tegra.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c index de022a9..8b1a2d3 100644 --- a/drivers/gpio/gpio-tegra.c +++ b/drivers/gpio/gpio-tegra.c @@ -46,6 +46,7 @@ #define GPIO_INT_ENB(x) (GPIO_REG(x) + 0x50) #define GPIO_INT_LVL(x) (GPIO_REG(x) + 0x60) #define GPIO_INT_CLR(x) (GPIO_REG(x) + 0x70) +#define GPIO_DBC_CNT(x) (GPIO_REG(x) + 0xF0) #define GPIO_MSK_CNF(x) (GPIO_REG(x) + tegra_gpio_upper_offset + 0x00) #define GPIO_MSK_OE(x) (GPIO_REG(x) + tegra_gpio_upper_offset + 0x10) @@ -53,6 +54,7 @@ #define GPIO_MSK_INT_STA(x) (GPIO_REG(x) + tegra_gpio_upper_offset + 0x40) #define GPIO_MSK_INT_ENB(x) (GPIO_REG(x) + tegra_gpio_upper_offset + 0x50) #define GPIO_MSK_INT_LVL(x) (GPIO_REG(x) + tegra_gpio_upper_offset + 0x60) +#define GPIO_MSK_DBC_EN(x) (GPIO_REG(x) + tegra_gpio_upper_offset + 0x30) #define GPIO_INT_LVL_MASK 0x010101 #define GPIO_INT_LVL_EDGE_RISING 0x000101 @@ -72,12 +74,15 @@ 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_soc_config { u32 bank_stride; u32 upper_offset; + bool debounce_supported; }; static struct irq_domain *irq_domain; @@ -164,6 +169,33 @@ 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) +{ + 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(GPIO_MSK_DBC_EN(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 (tegra_gpio_banks[bank].dbc_cnt[port] < debounce_ms) { + tegra_gpio_writel(debounce_ms, GPIO_DBC_CNT(offset)); + tegra_gpio_banks[bank].dbc_cnt[port] = debounce_ms; + } + + tegra_gpio_mask_write(GPIO_MSK_DBC_EN(offset), offset, 1); + + return 0; +} + static int tegra_gpio_to_irq(struct gpio_chip *chip, unsigned offset) { return irq_find_mapping(irq_domain, offset); @@ -177,6 +209,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, }; @@ -327,6 +360,9 @@ static int tegra_gpio_resume(struct device *dev) tegra_gpio_writel(bank->oe[p], GPIO_OE(gpio)); tegra_gpio_writel(bank->int_lvl[p], GPIO_INT_LVL(gpio)); tegra_gpio_writel(bank->int_enb[p], GPIO_INT_ENB(gpio)); + tegra_gpio_writel(bank->dbc_cnt[p], GPIO_DBC_CNT(gpio)); + tegra_gpio_writel(bank->dbc_enb[p], + GPIO_MSK_DBC_EN(gpio)); } } @@ -351,6 +387,10 @@ static int tegra_gpio_suspend(struct device *dev) bank->oe[p] = tegra_gpio_readl(GPIO_OE(gpio)); bank->int_enb[p] = tegra_gpio_readl(GPIO_INT_ENB(gpio)); bank->int_lvl[p] = tegra_gpio_readl(GPIO_INT_LVL(gpio)); + bank->dbc_enb[p] = tegra_gpio_readl( + GPIO_MSK_DBC_EN(gpio)); + bank->dbc_enb[p] = (bank->dbc_enb[p] << 8) || + bank->dbc_enb[p]; /* Enable gpio irq for wake up source */ tegra_gpio_writel(bank->wake_enb[p], @@ -473,6 +513,8 @@ static int tegra_gpio_probe(struct platform_device *pdev) tegra_gpio_bank_stride = config->bank_stride; tegra_gpio_upper_offset = config->upper_offset; + if (!config->debounce_supported) + tegra_gpio_chip.set_debounce = NULL; for (;;) { res = platform_get_resource(pdev, IORESOURCE_IRQ, tegra_gpio_bank_count); @@ -570,7 +612,14 @@ static struct tegra_gpio_soc_config tegra30_gpio_config = { .upper_offset = 0x80, }; +static struct tegra_gpio_soc_config tegra210_gpio_config = { + .bank_stride = 0x100, + .upper_offset = 0x80, + .debounce_supported = true, +}; + 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH V2 3/3] gpio: tegra: Add support for gpio debounce 2016-04-19 9:43 ` [PATCH V2 3/3] gpio: tegra: Add support for gpio debounce Laxman Dewangan @ 2016-04-19 10:43 ` Jon Hunter 2016-04-19 12:36 ` Thierry Reding 2016-04-19 12:37 ` Thierry Reding 2016-04-19 16:11 ` Stephen Warren 2 siblings, 1 reply; 15+ messages in thread From: Jon Hunter @ 2016-04-19 10:43 UTC (permalink / raw) To: Laxman Dewangan, linus.walleij, gnurou, swarren, thierry.reding Cc: linux-gpio, linux-tegra, linux-kernel On 19/04/16 10:43, 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. > > 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. > --- > drivers/gpio/gpio-tegra.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 49 insertions(+) [snip] > @@ -327,6 +360,9 @@ static int tegra_gpio_resume(struct device *dev) > tegra_gpio_writel(bank->oe[p], GPIO_OE(gpio)); > tegra_gpio_writel(bank->int_lvl[p], GPIO_INT_LVL(gpio)); > tegra_gpio_writel(bank->int_enb[p], GPIO_INT_ENB(gpio)); > + tegra_gpio_writel(bank->dbc_cnt[p], GPIO_DBC_CNT(gpio)); > + tegra_gpio_writel(bank->dbc_enb[p], > + GPIO_MSK_DBC_EN(gpio)); If these registers are not valid on Tegra devices prior to Tegra210, I don't think we should write to these locations on those devices (even if we are writing back the values read). > @@ -351,6 +387,10 @@ static int tegra_gpio_suspend(struct device *dev) > bank->oe[p] = tegra_gpio_readl(GPIO_OE(gpio)); > bank->int_enb[p] = tegra_gpio_readl(GPIO_INT_ENB(gpio)); > bank->int_lvl[p] = tegra_gpio_readl(GPIO_INT_LVL(gpio)); > + bank->dbc_enb[p] = tegra_gpio_readl( > + GPIO_MSK_DBC_EN(gpio)); > + bank->dbc_enb[p] = (bank->dbc_enb[p] << 8) || > + bank->dbc_enb[p]; Same here, not sure we should even bother reading these for Tegra's before Tegra210. Cheers Jon ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH V2 3/3] gpio: tegra: Add support for gpio debounce 2016-04-19 10:43 ` Jon Hunter @ 2016-04-19 12:36 ` Thierry Reding 0 siblings, 0 replies; 15+ messages in thread From: Thierry Reding @ 2016-04-19 12:36 UTC (permalink / raw) To: Jon Hunter Cc: Laxman Dewangan, linus.walleij, gnurou, swarren, linux-gpio, linux-tegra, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1978 bytes --] On Tue, Apr 19, 2016 at 11:43:25AM +0100, Jon Hunter wrote: > > On 19/04/16 10:43, 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. > > > > 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. > > --- > > drivers/gpio/gpio-tegra.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 49 insertions(+) > > [snip] > > > @@ -327,6 +360,9 @@ static int tegra_gpio_resume(struct device *dev) > > tegra_gpio_writel(bank->oe[p], GPIO_OE(gpio)); > > tegra_gpio_writel(bank->int_lvl[p], GPIO_INT_LVL(gpio)); > > tegra_gpio_writel(bank->int_enb[p], GPIO_INT_ENB(gpio)); > > + tegra_gpio_writel(bank->dbc_cnt[p], GPIO_DBC_CNT(gpio)); > > + tegra_gpio_writel(bank->dbc_enb[p], > > + GPIO_MSK_DBC_EN(gpio)); > > If these registers are not valid on Tegra devices prior to Tegra210, I > don't think we should write to these locations on those devices (even if > we are writing back the values read). > > > @@ -351,6 +387,10 @@ static int tegra_gpio_suspend(struct device *dev) > > bank->oe[p] = tegra_gpio_readl(GPIO_OE(gpio)); > > bank->int_enb[p] = tegra_gpio_readl(GPIO_INT_ENB(gpio)); > > bank->int_lvl[p] = tegra_gpio_readl(GPIO_INT_LVL(gpio)); > > + bank->dbc_enb[p] = tegra_gpio_readl( > > + GPIO_MSK_DBC_EN(gpio)); > > + bank->dbc_enb[p] = (bank->dbc_enb[p] << 8) || > > + bank->dbc_enb[p]; > > Same here, not sure we should even bother reading these for Tegra's > before Tegra210. Indeed. This patch already introduces the debounce_supported capability in the SoC data, so access to these registers could be guarded by that. Thierry [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH V2 3/3] gpio: tegra: Add support for gpio debounce 2016-04-19 9:43 ` [PATCH V2 3/3] gpio: tegra: Add support for gpio debounce Laxman Dewangan 2016-04-19 10:43 ` Jon Hunter @ 2016-04-19 12:37 ` Thierry Reding 2016-04-19 12:45 ` Laxman Dewangan 2016-04-19 16:11 ` Stephen Warren 2 siblings, 1 reply; 15+ messages in thread From: Thierry Reding @ 2016-04-19 12:37 UTC (permalink / raw) To: Laxman Dewangan Cc: linus.walleij, gnurou, swarren, linux-gpio, linux-tegra, linux-kernel [-- Attachment #1: Type: text/plain, Size: 817 bytes --] On Tue, Apr 19, 2016 at 03:13:40PM +0530, Laxman Dewangan wrote: [...] > @@ -570,7 +612,14 @@ static struct tegra_gpio_soc_config tegra30_gpio_config = { > .upper_offset = 0x80, > }; > > +static struct tegra_gpio_soc_config tegra210_gpio_config = { > + .bank_stride = 0x100, > + .upper_offset = 0x80, > + .debounce_supported = true, > +}; > + > 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 }, > { }, I think I'd split this hunk off into a separate patch. Oh, and perhaps follow up with a patch that makes the SoC configuration data const? Thierry [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH V2 3/3] gpio: tegra: Add support for gpio debounce 2016-04-19 12:37 ` Thierry Reding @ 2016-04-19 12:45 ` Laxman Dewangan 0 siblings, 0 replies; 15+ messages in thread From: Laxman Dewangan @ 2016-04-19 12:45 UTC (permalink / raw) To: Thierry Reding Cc: linus.walleij, gnurou, swarren, linux-gpio, linux-tegra, linux-kernel On Tuesday 19 April 2016 06:07 PM, Thierry Reding wrote: > * PGP Signed by an unknown key > > On Tue, Apr 19, 2016 at 03:13:40PM +0530, Laxman Dewangan wrote: > [...] >> @@ -570,7 +612,14 @@ static struct tegra_gpio_soc_config tegra30_gpio_config = { >> .upper_offset = 0x80, >> }; >> >> +static struct tegra_gpio_soc_config tegra210_gpio_config = { >> + .bank_stride = 0x100, >> + .upper_offset = 0x80, >> + .debounce_supported = true, >> +}; >> + >> 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 }, >> { }, > I think I'd split this hunk off into a separate patch. Oh, and perhaps > follow up with a patch that makes the SoC configuration data const? > OK, then I will do that on follow-on patch: - Get rid of all global variables. - Convert the soc config to the constant. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH V2 3/3] gpio: tegra: Add support for gpio debounce 2016-04-19 9:43 ` [PATCH V2 3/3] gpio: tegra: Add support for gpio debounce Laxman Dewangan 2016-04-19 10:43 ` Jon Hunter 2016-04-19 12:37 ` Thierry Reding @ 2016-04-19 16:11 ` Stephen Warren 2016-04-19 16:19 ` Laxman Dewangan 2 siblings, 1 reply; 15+ messages in thread From: Stephen Warren @ 2016-04-19 16:11 UTC (permalink / raw) To: Laxman Dewangan Cc: linus.walleij, gnurou, thierry.reding, linux-gpio, linux-tegra, linux-kernel On 04/19/2016 03:43 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 > @@ -327,6 +360,9 @@ static int tegra_gpio_resume(struct device *dev) > tegra_gpio_writel(bank->oe[p], GPIO_OE(gpio)); > tegra_gpio_writel(bank->int_lvl[p], GPIO_INT_LVL(gpio)); > tegra_gpio_writel(bank->int_enb[p], GPIO_INT_ENB(gpio)); > + tegra_gpio_writel(bank->dbc_cnt[p], GPIO_DBC_CNT(gpio)); > + tegra_gpio_writel(bank->dbc_enb[p], > + GPIO_MSK_DBC_EN(gpio)); Why not just write to the "regular" register rather than the mask register here... > @@ -351,6 +387,10 @@ static int tegra_gpio_suspend(struct device *dev) > bank->oe[p] = tegra_gpio_readl(GPIO_OE(gpio)); > bank->int_enb[p] = tegra_gpio_readl(GPIO_INT_ENB(gpio)); > bank->int_lvl[p] = tegra_gpio_readl(GPIO_INT_LVL(gpio)); > + bank->dbc_enb[p] = tegra_gpio_readl( > + GPIO_MSK_DBC_EN(gpio)); > + bank->dbc_enb[p] = (bank->dbc_enb[p] << 8) || > + bank->dbc_enb[p]; ... since that would avoid having to or in the mask value in the saved register value here; you could just save/restore the regular register in the same way as any other register. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH V2 3/3] gpio: tegra: Add support for gpio debounce 2016-04-19 16:11 ` Stephen Warren @ 2016-04-19 16:19 ` Laxman Dewangan 2016-04-19 16:36 ` Stephen Warren 0 siblings, 1 reply; 15+ messages in thread From: Laxman Dewangan @ 2016-04-19 16:19 UTC (permalink / raw) To: Stephen Warren Cc: linus.walleij, gnurou, thierry.reding, linux-gpio, linux-tegra, linux-kernel On Tuesday 19 April 2016 09:41 PM, Stephen Warren wrote: > On 04/19/2016 03:43 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 > >> @@ -327,6 +360,9 @@ static int tegra_gpio_resume(struct device *dev) >> tegra_gpio_writel(bank->oe[p], GPIO_OE(gpio)); >> tegra_gpio_writel(bank->int_lvl[p], GPIO_INT_LVL(gpio)); >> tegra_gpio_writel(bank->int_enb[p], GPIO_INT_ENB(gpio)); >> + tegra_gpio_writel(bank->dbc_cnt[p], GPIO_DBC_CNT(gpio)); >> + tegra_gpio_writel(bank->dbc_enb[p], >> + GPIO_MSK_DBC_EN(gpio)); > > Why not just write to the "regular" register rather than the mask > register here... There is no regular register for enabling debounce. Only masked register exist. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH V2 3/3] gpio: tegra: Add support for gpio debounce 2016-04-19 16:19 ` Laxman Dewangan @ 2016-04-19 16:36 ` Stephen Warren 0 siblings, 0 replies; 15+ messages in thread From: Stephen Warren @ 2016-04-19 16:36 UTC (permalink / raw) To: Laxman Dewangan Cc: linus.walleij, gnurou, thierry.reding, linux-gpio, linux-tegra, linux-kernel On 04/19/2016 10:19 AM, Laxman Dewangan wrote: > > On Tuesday 19 April 2016 09:41 PM, Stephen Warren wrote: >> On 04/19/2016 03:43 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 >> >>> @@ -327,6 +360,9 @@ static int tegra_gpio_resume(struct device *dev) >>> tegra_gpio_writel(bank->oe[p], GPIO_OE(gpio)); >>> tegra_gpio_writel(bank->int_lvl[p], GPIO_INT_LVL(gpio)); >>> tegra_gpio_writel(bank->int_enb[p], GPIO_INT_ENB(gpio)); >>> + tegra_gpio_writel(bank->dbc_cnt[p], GPIO_DBC_CNT(gpio)); >>> + tegra_gpio_writel(bank->dbc_enb[p], >>> + GPIO_MSK_DBC_EN(gpio)); >> >> Why not just write to the "regular" register rather than the mask >> register here... > > There is no regular register for enabling debounce. Only masked register > exist. Sigh. Ignore that comment then:-) ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH V2 1/3] gpio: tegra: Don't open code of_device_get_match_data() 2016-04-19 9:43 [PATCH V2 1/3] gpio: tegra: Don't open code of_device_get_match_data() Laxman Dewangan 2016-04-19 9:43 ` [PATCH V2 2/3] gpio: tegra: Remove the need of keeping device handle for gpio driver Laxman Dewangan 2016-04-19 9:43 ` [PATCH V2 3/3] gpio: tegra: Add support for gpio debounce Laxman Dewangan @ 2016-04-19 12:06 ` Thierry Reding 2016-04-29 8:56 ` Linus Walleij 3 siblings, 0 replies; 15+ messages in thread From: Thierry Reding @ 2016-04-19 12:06 UTC (permalink / raw) To: Laxman Dewangan Cc: linus.walleij, gnurou, swarren, linux-gpio, linux-tegra, linux-kernel [-- Attachment #1: Type: text/plain, Size: 522 bytes --] On Tue, Apr 19, 2016 at 03:13:38PM +0530, Laxman Dewangan wrote: > 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> > > --- > Collected reviewed by from Stephen. > --- > drivers/gpio/gpio-tegra.c | 50 +++++++++++++++++++++++------------------------ > 1 file changed, 24 insertions(+), 26 deletions(-) Acked-by: Thierry Reding <treding@nvidia.com> [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH V2 1/3] gpio: tegra: Don't open code of_device_get_match_data() 2016-04-19 9:43 [PATCH V2 1/3] gpio: tegra: Don't open code of_device_get_match_data() Laxman Dewangan ` (2 preceding siblings ...) 2016-04-19 12:06 ` [PATCH V2 1/3] gpio: tegra: Don't open code of_device_get_match_data() Thierry Reding @ 2016-04-29 8:56 ` Linus Walleij 3 siblings, 0 replies; 15+ messages in thread From: Linus Walleij @ 2016-04-29 8:56 UTC (permalink / raw) To: Laxman Dewangan Cc: Alexandre Courbot, Stephen Warren, Thierry Reding, linux-gpio, linux-tegra, linux-kernel On Tue, Apr 19, 2016 at 11:43 AM, Laxman Dewangan <ldewangan@nvidia.com> wrote: > 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> Patch applied with Thierry's ACK. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2016-04-29 8:56 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-04-19 9:43 [PATCH V2 1/3] gpio: tegra: Don't open code of_device_get_match_data() Laxman Dewangan 2016-04-19 9:43 ` [PATCH V2 2/3] gpio: tegra: Remove the need of keeping device handle for gpio driver Laxman Dewangan 2016-04-19 12:33 ` Thierry Reding 2016-04-19 12:43 ` Laxman Dewangan 2016-04-19 16:04 ` Stephen Warren 2016-04-19 9:43 ` [PATCH V2 3/3] gpio: tegra: Add support for gpio debounce Laxman Dewangan 2016-04-19 10:43 ` Jon Hunter 2016-04-19 12:36 ` Thierry Reding 2016-04-19 12:37 ` Thierry Reding 2016-04-19 12:45 ` Laxman Dewangan 2016-04-19 16:11 ` Stephen Warren 2016-04-19 16:19 ` Laxman Dewangan 2016-04-19 16:36 ` Stephen Warren 2016-04-19 12:06 ` [PATCH V2 1/3] gpio: tegra: Don't open code of_device_get_match_data() Thierry Reding 2016-04-29 8:56 ` Linus Walleij
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).