gpio/omap: remaining cleanups and fix
Subsystems (6)
cover gpio/omap: remaining cleanups and fix
Here are the remaining cleanup patches. There are broadly two categories of cleanups.
Cat-1: Those missed while introducing new feature like SPARSE_IRQ
handling and DT support; use edge/level handlers from generic IRQ framework.
Cat-2: Removal of redundant fields from struct gpio_bank{} as a
result of they being already covered by members within context field of struct gpio_bank{}.
Reference: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
Commit: 66f75a5d028beaf67c931435fdc3e7823125730c (Linux 3.4-rc4)
Series is available for reference here:git://gitorious.org/~tarunkanti/omap-sw-develoment/tarunkantis-linux-omap-dev.git for_3.5/gpio_more_cleanup_fixes
Test info:
OMAP2+ platforms: OMAP2430SDP, OMAP3430SDP, OMAP4430SDP
OMAP1: Bootup test on OMAP1710SDP.
Cc: Kevin Hilman <[email protected]>
Cc: Tony Lindgren <[email protected]>
Cc: Santosh Shilimkar <[email protected]>
Cc: Cousson, Benoit <[email protected]>
Cc: Grant Likely <[email protected]>
Tarun Kanti DebBarma (8):
gpio/omap: remove virtual_irq_start variable
gpio/omap: remove saved_fallingdetect, saved_risingdetect gpio/omap: remove suspend_wakeup field from struct gpio_bank gpio/omap: remove saved_wakeup field from struct gpio_bank gpio/omap: remove retrigger variable in gpio_irq_handler gpio/omap: remove suspend/resume callbacks
gpio/omap: remove cpu_is_omapxxxx() checks from *_runtime_resume() gpio/omap: fix missing check in *_runtime_suspend()
arch/arm/mach-omap1/gpio15xx.c | 2 - arch/arm/mach-omap1/gpio16xx.c | 5 -- arch/arm/mach-omap1/gpio7xx.c | 7 -- arch/arm/mach-omap2/gpio.c | 3 +- arch/arm/plat-omap/include/plat/gpio.h | 3 +- drivers/gpio/gpio-omap.c | 103 +++++++------------------------- 6 files changed, 27 insertions(+), 96 deletions(-)
PATCH 1/8 gpio/omap: remove virtual_irq_start variable
This cleanup got missed while implementing following:
25db711 gpio/omap: Fix IRQ handling for SPARSE_IRQ 384ebe1 gpio/omap: Add DT support to GPIO driver
--- arch/arm/mach-omap1/gpio15xx.c | 2 -- arch/arm/mach-omap1/gpio16xx.c | 5 ----- arch/arm/mach-omap1/gpio7xx.c | 7 ------- arch/arm/mach-omap2/gpio.c | 1 - arch/arm/plat-omap/include/plat/gpio.h | 1 - 5 files changed, 0 insertions(+), 16 deletions(-)
arch/arm/mach-omap1/gpio15xx.c +0 -2
@@ -46,7 +46,6 @@ static struct omap_gpio_reg_offs omap15xx_mpuio_regs = { };static struct __initdata omap_gpio_platform_data omap15xx_mpu_gpio_config = { - .virtual_irq_start =IH_MPUIO_BASE , .is_mpuio = true, .bank_width =16 , .bank_stride =1 ,
@@ -89,7 +88,6 @@ static struct omap_gpio_reg_offs omap15xx_gpio_regs = { };static struct __initdata omap_gpio_platform_data omap15xx_gpio_config = { - .virtual_irq_start =IH_GPIO_BASE , .bank_width =16 , .regs = &omap15xx_gpio_regs, };
arch/arm/mach-omap1/gpio16xx.c +0 -5
@@ -52,7 +52,6 @@ static struct omap_gpio_reg_offs omap16xx_mpuio_regs = { };static struct __initdata omap_gpio_platform_data omap16xx_mpu_gpio_config = { - .virtual_irq_start =IH_MPUIO_BASE , .is_mpuio = true, .bank_width =16 , .bank_stride =1 ,
@@ -99,7 +98,6 @@ static struct omap_gpio_reg_offs omap16xx_gpio_regs = { };static struct __initdata omap_gpio_platform_data omap16xx_gpio1_config = { - .virtual_irq_start =IH_GPIO_BASE , .bank_width =16 , .regs = &omap16xx_gpio_regs, };
@@ -128,7 +126,6 @@ static struct __initdata resource omap16xx_gpio2_resources[] = { };static struct __initdata omap_gpio_platform_data omap16xx_gpio2_config = { - .virtual_irq_start =IH_GPIO_BASE +16 , .bank_width =16 , .regs = &omap16xx_gpio_regs, };
@@ -157,7 +154,6 @@ static struct __initdata resource omap16xx_gpio3_resources[] = { };static struct __initdata omap_gpio_platform_data omap16xx_gpio3_config = { - .virtual_irq_start =IH_GPIO_BASE +32 , .bank_width =16 , .regs = &omap16xx_gpio_regs, };
@@ -186,7 +182,6 @@ static struct __initdata resource omap16xx_gpio4_resources[] = { };static struct __initdata omap_gpio_platform_data omap16xx_gpio4_config = { - .virtual_irq_start =IH_GPIO_BASE +48 , .bank_width =16 , .regs = &omap16xx_gpio_regs, };
arch/arm/mach-omap1/gpio7xx.c +0 -7
@@ -51,7 +51,6 @@ static struct omap_gpio_reg_offs omap7xx_mpuio_regs = { };static struct __initdata omap_gpio_platform_data omap7xx_mpu_gpio_config = { - .virtual_irq_start =IH_MPUIO_BASE , .is_mpuio = true, .bank_width =16 , .bank_stride =2 ,
@@ -93,7 +92,6 @@ static struct omap_gpio_reg_offs omap7xx_gpio_regs = { };static struct __initdata omap_gpio_platform_data omap7xx_gpio1_config = { - .virtual_irq_start =IH_GPIO_BASE , .bank_width =32 , .regs = &omap7xx_gpio_regs, };
@@ -122,7 +120,6 @@ static struct __initdata resource omap7xx_gpio2_resources[] = { };static struct __initdata omap_gpio_platform_data omap7xx_gpio2_config = { - .virtual_irq_start =IH_GPIO_BASE +32 , .bank_width =32 , .regs = &omap7xx_gpio_regs, };
@@ -151,7 +148,6 @@ static struct __initdata resource omap7xx_gpio3_resources[] = { };static struct __initdata omap_gpio_platform_data omap7xx_gpio3_config = { - .virtual_irq_start =IH_GPIO_BASE +64 , .bank_width =32 , .regs = &omap7xx_gpio_regs, };
@@ -180,7 +176,6 @@ static struct __initdata resource omap7xx_gpio4_resources[] = { };static struct __initdata omap_gpio_platform_data omap7xx_gpio4_config = { - .virtual_irq_start =IH_GPIO_BASE +96 , .bank_width =32 , .regs = &omap7xx_gpio_regs, };
@@ -209,7 +204,6 @@ static struct __initdata resource omap7xx_gpio5_resources[] = { };static struct __initdata omap_gpio_platform_data omap7xx_gpio5_config = { - .virtual_irq_start =IH_GPIO_BASE +128 , .bank_width =32 , .regs = &omap7xx_gpio_regs, };
@@ -238,7 +232,6 @@ static struct __initdata resource omap7xx_gpio6_resources[] = { };static struct __initdata omap_gpio_platform_data omap7xx_gpio6_config = { - .virtual_irq_start =IH_GPIO_BASE +160 , .bank_width =32 , .regs = &omap7xx_gpio_regs, };
arch/arm/mach-omap2/gpio.c +0 -1
@@ -55,7 +55,6 @@ static int __init omap2_gpio_dev_init(struct omap_hwmod *oh, void *unused) dev_attr = (struct omap_gpio_dev_attr *)oh->dev_attr; pdata->bank_width = dev_attr->bank_width; pdata->dbck_flag = dev_attr->dbck_flag; - pdata->virtual_irq_start =IH_GPIO_BASE +32 * (id -1 ); pdata->get_context_loss_count = omap_pm_get_dev_context_loss_count; pdata->regs =kzalloc (sizeof (struct omap_gpio_reg_offs ),GFP_KERNEL );if (!pdata) {
arch/arm/plat-omap/include/plat/gpio.h +0 -1
@@ -193,7 +193,6 @@ struct omap_gpio_reg_offs { };struct omap_gpio_platform_data { -u16 virtual_irq_start;int bank_type;int bank_width;/* GPIO bank width */ int bank_stride;/* Only needed for omap1 MPUIO */
Signature · 1 line
1.7.0.4
PATCH 1/8 gpio/omap: remove virtual_irq_start variable
Quoted 10 lines · Tarun Kanti DebBarma
This cleanup got missed while implementing following:
25db711 gpio/omap: Fix IRQ handling forSPARSE_IRQ384ebe1 gpio/omap: Add DT support to GPIO driverCc Kevin Hilman <[email protected]>Cc Tony Lindgren <[email protected]>Cc Santosh Shilimkar <[email protected]>Cc Cousson, Benoit <[email protected]>Cc Grant Likely <[email protected]>Signed-off-by Tarun Kanti DebBarma <[email protected]>
Quoted 8 lines
--- arch/arm/mach-omap1/gpio15xx.c | 2 -- arch/arm/mach-omap1/gpio16xx.c | 5 ----- arch/arm/mach-omap1/gpio7xx.c | 7 ------- arch/arm/mach-omap2/gpio.c | 1 - arch/arm/plat-omap/include/plat/gpio.h | 1 - 5 files changed, 0 insertions(+), 16 deletions(-)
Looks good.
PATCH 2/8 gpio/omap: remove saved_fallingdetect, saved_risingdetect
In reply to Tarun Kanti DebBarmaSince we already have context.fallingdetect and context.risingdetect there is no more need to have these additional fields. Also, getting rid of extra reads associated with them.
--- drivers/gpio/gpio-omap.c | 19 ++++++++----------- 1 files changed, 8 insertions(+), 11 deletions(-)
drivers/gpio/gpio-omap.c +8 -11
@@ -63,8 +63,6 @@ struct gpio_bank {u32 enabled_non_wakeup_gpios;struct gpio_regs context;u32 saved_datain; -u32 saved_fallingdetect; -u32 saved_risingdetect;u32 level_mask;u32 toggle_mask;spinlock_t lock;
@@ -1247,11 +1245,9 @@ static int omap_gpio_runtime_suspend(struct device *dev)*/ bank->saved_datain =__raw_readl (bank->base + bank->regs->datain); - l1 =__raw_readl (bank->base + bank->regs->fallingdetect); - l2 =__raw_readl (bank->base + bank->regs->risingdetect); + l1 = bank->context.fallingdetect; + l2 = bank->context.risingdetect; - bank->saved_fallingdetect = l1; - bank->saved_risingdetect = l2; l1 &= ~bank->enabled_non_wakeup_gpios; l2 &= ~bank->enabled_non_wakeup_gpios;
@@ -1310,9 +1306,9 @@ static int omap_gpio_runtime_resume(struct device *dev) } } -__raw_writel (bank->saved_fallingdetect, +__raw_writel (bank->context.fallingdetect, bank->base + bank->regs->fallingdetect); -__raw_writel (bank->saved_risingdetect, +__raw_writel (bank->context.risingdetect, bank->base + bank->regs->risingdetect); l =__raw_readl (bank->base + bank->regs->datain);
@@ -1329,14 +1325,15 @@ static int omap_gpio_runtime_resume(struct device *dev)* No need to generate IRQs for the rising edge for gpio IRQs * configured with falling edge only; and vice versa. */ - gen0 = l & bank->saved_fallingdetect; + gen0 = l & bank->context.fallingdetect; gen0 &= bank->saved_datain; - gen1 = l & bank->saved_risingdetect; + gen1 = l & bank->context.risingdetect; gen1 &= ~(bank->saved_datain);/* FIXME: Consider GPIO IRQs with level detections properly! */ - gen = l & (~(bank->saved_fallingdetect) & ~(bank->saved_risingdetect)); + gen = l & (~(bank->context.fallingdetect) & + ~(bank->context.risingdetect));/* Consider all GPIO IRQs needed to be updated */ gen |= gen0 | gen1;
Signature · 1 line
1.7.0.4
PATCH 3/8 gpio/omap: remove suspend_wakeup field from struct gpio_bank
In reply to Tarun Kanti DebBarmaSince we already have bank->context.wake_en to keep track of gpios which are wakeup enabled, there is no need to have this field any more.
--- drivers/gpio/gpio-omap.c | 13 ++++++------- 1 files changed, 6 insertions(+), 7 deletions(-)
drivers/gpio/gpio-omap.c +6 -7
@@ -57,7 +57,6 @@ struct gpio_bank {u16 irq;int irq_base;struct irq_domain *domain; -u32 suspend_wakeup;u32 saved_wakeup;u32 non_wakeup_gpios;u32 enabled_non_wakeup_gpios;
@@ -514,11 +513,11 @@ static int _set_gpio_wakeup(struct gpio_bank *bank, int gpio, int enable)spin_lock_irqsave (&bank->lock, flags);if (enable) - bank->suspend_wakeup |= gpio_bit; + bank->context.wake_en |= gpio_bit;else - bank->suspend_wakeup &= ~gpio_bit; + bank->context.wake_en &= ~gpio_bit; -__raw_writel (bank->suspend_wakeup, bank->base + bank->regs->wkup_en); +__raw_writel (bank->context.wake_en, bank->base + bank->regs->wkup_en);spin_unlock_irqrestore (&bank->lock, flags);return 0 ;
@@ -788,7 +787,7 @@ static int omap_mpuio_suspend_noirq(struct device *dev)spin_lock_irqsave (&bank->lock, flags); bank->saved_wakeup =__raw_readl (mask_reg); -__raw_writel (0xffff & ~bank->suspend_wakeup, mask_reg); +__raw_writel (0xffff & ~bank->context.wake_en, mask_reg);spin_unlock_irqrestore (&bank->lock, flags);return 0 ;
@@ -1165,7 +1164,7 @@ static int omap_gpio_suspend(struct device *dev)if (!bank->mod_usage || !bank->loses_context)return 0 ; -if (!bank->regs->wkup_en || !bank->suspend_wakeup) +if (!bank->regs->wkup_en || !bank->context.wake_en)return 0 ; wakeup_enable = bank->base + bank->regs->wkup_en;
@@ -1173,7 +1172,7 @@ static int omap_gpio_suspend(struct device *dev)spin_lock_irqsave (&bank->lock, flags); bank->saved_wakeup =__raw_readl (wakeup_enable);_gpio_rmw (base, bank->regs->wkup_en,0xffffffff ,0 ); -_gpio_rmw (base, bank->regs->wkup_en, bank->suspend_wakeup,1 ); +_gpio_rmw (base, bank->regs->wkup_en, bank->context.wake_en,1 );spin_unlock_irqrestore (&bank->lock, flags);return 0 ;
Signature · 1 line
1.7.0.4
PATCH 4/8 gpio/omap: remove saved_wakeup field from struct gpio_bank
In reply to Tarun Kanti DebBarmaThere is no more need to have saved_wakeup because bank->context.wake_en already holds that value. So getting rid of read/write operation associated with this field.
--- drivers/gpio/gpio-omap.c | 12 +++--------- 1 files changed, 3 insertions(+), 9 deletions(-)
drivers/gpio/gpio-omap.c +3 -9
@@ -57,7 +57,6 @@ struct gpio_bank {u16 irq;int irq_base;struct irq_domain *domain; -u32 saved_wakeup;u32 non_wakeup_gpios;u32 enabled_non_wakeup_gpios;struct gpio_regs context;
@@ -786,7 +785,6 @@ static int omap_mpuio_suspend_noirq(struct device *dev)unsigned long flags;spin_lock_irqsave (&bank->lock, flags); - bank->saved_wakeup =__raw_readl (mask_reg);__raw_writel (0xffff & ~bank->context.wake_en, mask_reg);spin_unlock_irqrestore (&bank->lock, flags);
@@ -802,7 +800,7 @@ static int omap_mpuio_resume_noirq(struct device *dev)unsigned long flags;spin_lock_irqsave (&bank->lock, flags); -__raw_writel (bank->saved_wakeup, mask_reg); +__raw_writel (bank->context.wake_en, mask_reg);spin_unlock_irqrestore (&bank->lock, flags);return 0 ;
@@ -1158,7 +1156,6 @@ static int omap_gpio_suspend(struct device *dev)struct platform_device *pdev =to_platform_device (dev);struct gpio_bank *bank =platform_get_drvdata (pdev);void __iomem *base = bank->base; -void __iomem *wakeup_enable;unsigned long flags;if (!bank->mod_usage || !bank->loses_context)
@@ -1167,10 +1164,7 @@ static int omap_gpio_suspend(struct device *dev)if (!bank->regs->wkup_en || !bank->context.wake_en)return 0 ; - wakeup_enable = bank->base + bank->regs->wkup_en; -spin_lock_irqsave (&bank->lock, flags); - bank->saved_wakeup =__raw_readl (wakeup_enable);_gpio_rmw (base, bank->regs->wkup_en,0xffffffff ,0 );_gpio_rmw (base, bank->regs->wkup_en, bank->context.wake_en,1 );spin_unlock_irqrestore (&bank->lock, flags);
@@ -1188,12 +1182,12 @@ static int omap_gpio_resume(struct device *dev)if (!bank->mod_usage || !bank->loses_context)return 0 ; -if (!bank->regs->wkup_en || !bank->saved_wakeup) +if (!bank->regs->wkup_en || !bank->context.wake_en)return 0 ;spin_lock_irqsave (&bank->lock, flags);_gpio_rmw (base, bank->regs->wkup_en,0xffffffff ,0 ); -_gpio_rmw (base, bank->regs->wkup_en, bank->saved_wakeup,1 ); +_gpio_rmw (base, bank->regs->wkup_en, bank->context.wake_en,1 );spin_unlock_irqrestore (&bank->lock, flags);return 0 ;
Signature · 1 line
1.7.0.4
PATCH 5/8 gpio/omap: remove retrigger variable in gpio_irq_handler
In reply to Tarun Kanti DebBarmacommit 672e302e3c (ARM: OMAP: use edge/level handlers from generic IRQ framework) removed retrigger support in favor of using generic IRQ framework. This patch cleans up some unused remnants of that removal.
--- drivers/gpio/gpio-omap.c | 3 --- 1 files changed, 0 insertions(+), 3 deletions(-)
drivers/gpio/gpio-omap.c +0 -3
@@ -636,7 +636,6 @@ static void gpio_irq_handler(unsigned int irq, struct irq_desc *desc)u32 isr;unsigned int gpio_irq, gpio_index;struct gpio_bank *bank; -u32 retrigger =0 ;int unmasked =0 ;struct irq_chip *chip =irq_desc_get_chip (desc);
@@ -673,8 +672,6 @@ static void gpio_irq_handler(unsigned int irq, struct irq_desc *desc)chained_irq_exit (chip, desc); } - isr |= retrigger; - retrigger =0 ;if (!isr)break ;
Signature · 1 line
1.7.0.4
PATCH 6/8 gpio/omap: remove suspend/resume callbacks
In reply to Tarun Kanti DebBarmaBoth omap_gpio_suspend() and omap_gpio_resume() does programming of wakeup_en register._gpio_rmw(base, bank->regs->wkup_en, 0xffffffff, 0); _gpio_rmw(base, bank->regs->wkup_en, bank->context.wake_en, 1);
This is redundant in omap_gpio_suspend() because wakeup_en register automatically gets initialized in _set_gpio_wakeup() and set_gpio_trigger() while being called either from chip.irq_set_wake() or chip.irq_set_type().
This is also redundant in omap_gpio_resume() because wakeup_en register is programmed in omap_gpio_restore_context() called which is called from runtime resume callback.
--- drivers/gpio/gpio-omap.c | 47 ---------------------------------------------- 1 files changed, 0 insertions(+), 47 deletions(-)
drivers/gpio/gpio-omap.c +0 -47
@@ -1147,50 +1147,6 @@ static int __devinit omap_gpio_probe(struct platform_device *pdev)#ifdef CONFIG_ARCH_OMAP2PLUS -#if defined(CONFIG_PM_SLEEP ) -static int omap_gpio_suspend (struct device *dev) -{ -struct platform_device *pdev =to_platform_device (dev); -struct gpio_bank *bank =platform_get_drvdata (pdev); -void __iomem *base = bank->base; -unsigned long flags; - -if (!bank->mod_usage || !bank->loses_context) -return 0 ; - -if (!bank->regs->wkup_en || !bank->context.wake_en) -return 0 ; - -spin_lock_irqsave (&bank->lock, flags); -_gpio_rmw (base, bank->regs->wkup_en,0xffffffff ,0 ); -_gpio_rmw (base, bank->regs->wkup_en, bank->context.wake_en,1 ); -spin_unlock_irqrestore (&bank->lock, flags); - -return 0 ; -} - -static int omap_gpio_resume (struct device *dev) -{ -struct platform_device *pdev =to_platform_device (dev); -struct gpio_bank *bank =platform_get_drvdata (pdev); -void __iomem *base = bank->base; -unsigned long flags; - -if (!bank->mod_usage || !bank->loses_context) -return 0 ; - -if (!bank->regs->wkup_en || !bank->context.wake_en) -return 0 ; - -spin_lock_irqsave (&bank->lock, flags); -_gpio_rmw (base, bank->regs->wkup_en,0xffffffff ,0 ); -_gpio_rmw (base, bank->regs->wkup_en, bank->context.wake_en,1 ); -spin_unlock_irqrestore (&bank->lock, flags); - -return 0 ; -} -#endif /* CONFIG_PM_SLEEP */ -#if defined(CONFIG_PM_RUNTIME )static void omap_gpio_restore_context (struct gpio_bank *bank);
@@ -1419,14 +1375,11 @@ static void omap_gpio_restore_context(struct gpio_bank *bank) }#endif /* CONFIG_PM_RUNTIME */ #else -#define omap_gpio_suspend NULL -#define omap_gpio_resume NULL#define omap_gpio_runtime_suspend NULL#define omap_gpio_runtime_resume NULL#endif static const struct dev_pm_ops gpio_pm_ops = { - SET_SYSTEM_SLEEP_PM_OPS(omap_gpio_suspend, omap_gpio_resume)SET_RUNTIME_PM_OPS (omap_gpio_runtime_suspend, omap_gpio_runtime_resume,NULL ) };
Signature · 1 line
1.7.0.4
PATCH 7/8 gpio/omap: remove cpu_is_omapxxxx() checks from *_runtime_resume()
In reply to Tarun Kanti DebBarmaAdd register offsets for GPIO_IRQSTATUS_RAW_0, GPIO_IRQSTATUS_RAW_0 which are present on OMAP4+ processors. Now we can distinguish conditions applicable to OMAP4,5 and those specific to OMAP24xx and OMAP3xxx.
--- arch/arm/mach-omap2/gpio.c | 2 ++ arch/arm/plat-omap/include/plat/gpio.h | 2 ++ drivers/gpio/gpio-omap.c | 4 ++-- 3 files changed, 6 insertions(+), 2 deletions(-)
arch/arm/mach-omap2/gpio.c +2 -0
@@ -101,6 +101,8 @@ static int __init omap2_gpio_dev_init(struct omap_hwmod *oh, void *unused) pdata->regs->dataout =OMAP4_GPIO_DATAOUT ; pdata->regs->set_dataout =OMAP4_GPIO_SETDATAOUT ; pdata->regs->clr_dataout =OMAP4_GPIO_CLEARDATAOUT ; + pdata->regs->irqstatus_raw0 =OMAP4_GPIO_IRQSTATUSRAW0 ; + pdata->regs->irqstatus_raw1 =OMAP4_GPIO_IRQSTATUSRAW1 ; pdata->regs->irqstatus =OMAP4_GPIO_IRQSTATUS0 ; pdata->regs->irqstatus2 =OMAP4_GPIO_IRQSTATUS1 ; pdata->regs->irqenable =OMAP4_GPIO_IRQSTATUSSET0 ;
arch/arm/plat-omap/include/plat/gpio.h +2 -0
@@ -172,6 +172,8 @@ struct omap_gpio_reg_offs {u16 clr_dataout;u16 irqstatus;u16 irqstatus2; +u16 irqstatus_raw0; +u16 irqstatus_raw1;u16 irqenable;u16 irqenable2;u16 set_irqenable;
drivers/gpio/gpio-omap.c +2 -2
@@ -1289,14 +1289,14 @@ static int omap_gpio_runtime_resume(struct device *dev) old0 =__raw_readl (bank->base + bank->regs->leveldetect0); old1 =__raw_readl (bank->base + bank->regs->leveldetect1); -if (cpu_is_omap24xx () ||cpu_is_omap34xx ()) { +if (!bank->regs->irqstatus_raw0) {__raw_writel (old0 | gen, bank->base + bank->regs->leveldetect0);__raw_writel (old1 | gen, bank->base + bank->regs->leveldetect1); } -if (cpu_is_omap44xx ()) { +if (bank->regs->irqstatus_raw0) {__raw_writel (old0 | l, bank->base + bank->regs->leveldetect0); __raw_writel(old1 | l, bank->base +
Signature · 1 line
1.7.0.4
PATCH 7/8 gpio/omap: remove cpu_is_omapxxxx() checks from *_runtime_resume()
Quoted 11 lines · Tarun Kanti DebBarma
Add register offsets for
GPIO_IRQSTATUS_RAW_0,GPIO_IRQSTATUS_RAW_0which are present on OMAP4+ processors. Now we can distinguish conditions applicable to OMAP4,5 and those specific to OMAP24xx and OMAP3xxx.Cc Kevin Hilman <[email protected]>Cc Tony Lindgren <[email protected]>Cc Santosh Shilimkar <[email protected]>Cc Cousson, Benoit <[email protected]>Cc Grant Likely <[email protected]>Signed-off-by Tarun Kanti DebBarma <[email protected]>
Quoted 6 lines
--- arch/arm/mach-omap2/gpio.c | 2 ++ arch/arm/plat-omap/include/plat/gpio.h | 2 ++ drivers/gpio/gpio-omap.c | 4 ++-- 3 files changed, 6 insertions(+), 2 deletions(-)
Looks like this was the last cpu_is_*() in gpio driver. Thanks for cleaning up this.
Regards
Santosh
PATCH 8/8 gpio/omap: fix missing check in *_runtime_suspend()
In reply to Tarun Kanti DebBarmaWe do checking for bank->enabled_non_wakeup_gpios in order to skip redundant operations. Somehow, the check got missed while doing the cleanup series.
Just to make sure that we do context restore correctly in *_runtime_resume(), the bank->workaround_enabled check is moved after context restore. Otherwise, it would prevent context restore when bank->enabled_non_wakeup_gpios is 0.
--- drivers/gpio/gpio-omap.c | 13 ++++++++----- 1 files changed, 8 insertions(+), 5 deletions(-)
drivers/gpio/gpio-omap.c +8 -5
@@ -1160,6 +1160,9 @@ static int omap_gpio_runtime_suspend(struct device *dev)spin_lock_irqsave (&bank->lock, flags); +if (!bank->enabled_non_wakeup_gpios) + goto update_gpio_context_count; +/* * Only edges can generate a wakeup event to the PRCM. *
@@ -1235,11 +1238,6 @@ static int omap_gpio_runtime_resume(struct device *dev)__raw_writel (bank->context.risingdetect, bank->base + bank->regs->risingdetect); -if (!bank->workaround_enabled) { -spin_unlock_irqrestore (&bank->lock, flags); -return 0 ; - } -if (bank->get_context_loss_count) { context_lost_cnt_after = bank->get_context_loss_count (bank->dev);
@@ -1252,6 +1250,11 @@ static int omap_gpio_runtime_resume(struct device *dev) } } +if (!bank->workaround_enabled) { +spin_unlock_irqrestore (&bank->lock, flags); +return 0 ; + } +__raw_writel (bank->context.fallingdetect, bank->base + bank->regs->fallingdetect); __raw_writel(bank->context.risingdetect,
Signature · 1 line
1.7.0.4
PATCH 8/8 gpio/omap: fix missing check in *_runtime_suspend()
Quoted 9 lines · Tarun Kanti DebBarma
We do checking for
bank->enabled_non_wakeup_gpiosin order to skip redundant operations. Somehow, the check got missed while doing the cleanup series.Just to make sure that we do context restore correctly in
*_runtime_resume(), thebank->workaround_enabledcheck is moved after context restore. Otherwise, it would prevent context restore whenbank->enabled_non_wakeup_gpiosis 0.
Quoted 7 lines
Cc Kevin Hilman <[email protected]>Cc Tony Lindgren <[email protected]>Cc Santosh Shilimkar <[email protected]>Cc Cousson, Benoit <[email protected]>Cc Grant Likely <[email protected]>Signed-off-by Tarun Kanti DebBarma <[email protected]>---
Looks good.
PATCH 8/8 gpio/omap: fix missing check in *_runtime_suspend()
In reply to Tarun Kanti DebBarmaTarun, Santosh,
Quoted 9 lines · Tarun Kanti DebBarma
We do checking for
bank->enabled_non_wakeup_gpiosin order to skip redundant operations. Somehow, the check got missed while doing the cleanup series.Just to make sure that we do context restore correctly in
*_runtime_resume(), thebank->workaround_enabledcheck is moved after context restore. Otherwise, it would prevent context restore whenbank->enabled_non_wakeup_gpiosis 0.
Quoted 6 lines
Cc Kevin Hilman <[email protected]>Cc Tony Lindgren <[email protected]>Cc Santosh Shilimkar <[email protected]>Cc Cousson, Benoit <[email protected]>Cc Grant Likely <[email protected]>Signed-off-by Tarun Kanti DebBarma <[email protected]>
I just noticed that this patch has caused some strange problems, notably with the GPIO IRQ used by smsc911x NIC (Overo, Zoom3, 2430SDP, etc. etc.)
The patch itself is OK, but it has exposed a bug in other parts of the context restore path that was previously hidden.
We seem to have been finding lots of GPIO bugs by just testing the network chips with GPIO IRQs. Can I suggest that a platform with a GPIO IRQ NIC be added to the test platforms you're using?
Quoted 19 lines
--- drivers/gpio/gpio-omap.c | 13 ++++++++----- 1 files changed, 8 insertions(+), 5 deletions(-)drivers/gpio/gpio-omap.c +3
-0@@ -1160,6 +1160,9 @@ static int omap_gpio_runtime_suspend(struct device *dev)spin_lock_irqsave (&bank->lock, flags); +if (!bank->enabled_non_wakeup_gpios) + goto update_gpio_context_count; +/* * Only edges can generate a wakeup event to the PRCM. *
drivers/gpio/gpio-omap.c static int omap_gpio_runtime_resume(struct device *dev) · line 1242
Quoted 8 lines
drivers/gpio/gpio-omap.c +0
-5@@ -1235,11 +1238,6 @@ static int omap_gpio_runtime_resume(struct device *dev)__raw_writel (bank->context.risingdetect, bank->base + bank->regs->risingdetect); -if (!bank->workaround_enabled) { -spin_unlock_irqrestore (&bank->lock, flags); -return 0 ; - } -
My moving this below, you exposed some buggy code that can now get executed in non-OFF mode, wherease before $SUBJECT patch, it would never be exectued in non-OFF mode.
drivers/gpio/gpio-omap.c static int omap_gpio_runtime_resume(struct device *dev) · line 1243
Quoted 3 lines
drivers/gpio/gpio-omap.c +0
-0if (bank->get_context_loss_count) { context_lost_cnt_after = bank->get_context_loss_count (bank->dev);
...right below this line, we have:
if (context_lost_cnt_after != bank->context_loss_count ||
!context_lost_cnt_after) {
omap_gpio_restore_context(bank);While we've never hit off-mode, context_lost_cnt_after will always be zero. However, becasue of the !context_lost_cnt_after check there, we will *always* restore the bank context, even though context has never been lost.
I have no idea why that !context_lost_cnt_after check is there, but clearly it is wrong. Now that you moved the 'workraound_enabled' check below this section, as long as off-mode is disabled, the some GPIO context will be restored here on *every* runtime PM transition.
The patch below fixes the problem.
Grant, after Tarun/Santosh have a chance to review/ack this, can you still get this in for 3.5? If not, getting it into -rc should be fine.
Kevin
Quoted 1 line
From
83874df8e0dd78a3609f5ba81d608df7217fd212Mon Sep 17 00:00:00 2001
From: Kevin Hilman <[email protected]>
Date: Thu, 17 May 2012 14:52:56 -0700
Subject: [PATCH] gpio/omap: fix broken context restore for non-OFF mode
transitions
The fix in commit 1b12870 (gpio/omap: fix missing check in *_runtime_suspend()) exposed another bug in the context restore path.
Currently, the per-bank context restore happens whenever the context loss count is different in runtime suspend and runtime resume *and* whenever the per-bank contex_loss_count == 0:
if (context_lost_cnt_after != bank->context_loss_count ||
!context_lost_cnt_after) {
omap_gpio_restore_context(bank);Restoring context when the context_lost_cnt_after == 0 is clearly wrong, since this will be true until the first off-mode transition (which could be never, if off-mode is never enabled.) This check causes the context to be restored on *every* runtime PM transition.
Before commit 1b12870 (gpio/omap: fix missing check in *_runtime_suspend()), this code was never executed in non-OFF mode, so there were never spurious context restores happening. After that change though, spurious context restores could happen.
To fix, simply remove the !context_lost_cnt_after check. It is not needed.
This bug was found when noticing that the smc911x NIC on 3530/Overo was not working, and git bisect tracked it down to this patch. It seems that the spurious context restore was causing the smsc911x to not be properly probed on this platform.
--- drivers/gpio/gpio-omap.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/gpio/gpio-omap.c +1 -2
@@ -1238,8 +1238,7 @@ static int omap_gpio_runtime_resume(struct device *dev)if (bank->get_context_loss_count) { context_lost_cnt_after = bank->get_context_loss_count (bank->dev); -if (context_lost_cnt_after != bank->context_loss_count || - !context_lost_cnt_after) { +if (context_lost_cnt_after != bank->context_loss_count) {omap_gpio_restore_context (bank); }else {spin_unlock_irqrestore (&bank->lock, flags);
Signature · 1 line
1.7.9.2
Tony Lindgren
maintainerPATCH 8/8 gpio/omap: fix missing check in *_runtime_suspend()
Quoted 10 lines · Kevin Hilman
I just noticed that this patch has caused some strange problems, notably with the GPIO IRQ used by smsc911x NIC (Overo, Zoom3, 2430SDP, etc. etc.)
The patch itself is OK, but it has exposed a bug in other parts of the context restore path that was previously hidden.
We seem to have been finding lots of GPIO bugs by just testing the network chips with GPIO IRQs. Can I suggest that a platform with a GPIO IRQ NIC be added to the test platforms you're using?
Yes considering the breakage it's pretty obvious the original series was never properly tested on omaps.
Regarding this fix, using gpio/next + this patch fixes nfsroot for 2430sdp, and gets zoom3 nfsroot booting going a bit better.
However, at least on zoom3 nfsroot now takes several _minutes_ to get to
login: with gpio/next + this patch. The system is totally unusable.
It seems that the GPIO interrupt wake-up events are not properly working now?
Reverting the "gpio/omap: fix missing check in *_runtime_suspend()" patch seems to fix the issue.
Quoted 5 lines
Subject: [PATCH] gpio/omap: fix broken context restore for non-OFF mode
transitionsThe fix in commit 1b12870 (gpio/omap: fix missing check in
*_runtime_suspend()) exposed another bug in the context restore path.
Kevin, looks like commit 1b12870 does not exist in gpio/next?
Regards,
Tony
PATCH 8/8 gpio/omap: fix missing check in *_runtime_suspend()
Quoted 8 lines · Tony Lindgren
Quoted 7 lines · Kevin Hilman
I just noticed that this patch has caused some strange problems, notably with the GPIO IRQ used by smsc911x NIC (Overo, Zoom3, 2430SDP, etc. etc.)
The patch itself is OK, but it has exposed a bug in other parts of the context restore path that was previously hidden.
Quoted 6 lines
Quoted 3 lines
We seem to have been finding lots of GPIO bugs by just testing the network chips with GPIO IRQs. Can I suggest that a platform with a GPIO IRQ NIC be added to the test platforms you're using?
Yes considering the breakage it's pretty obvious the original series was never properly tested on omaps.
Agreed.
Quoted 11 lines
Regarding this fix, using gpio/next + this patch fixes nfsroot for 2430sdp, and gets zoom3 nfsroot booting going a bit better.
However, at least on zoom3 nfsroot now takes several _minutes_ to get to
login: with gpio/next + this patch. The system is totally unusable.It seems that the GPIO interrupt wake-up events are not properly working now?
Reverting the "gpio/omap: fix missing check in
*_runtime_suspend()" patch seems to fix the issue.
Argh, then $SUBJECT patch here has caused brokeness in multiple ways. It managed to break both runtime suspend and runtime resume at the same time. :(
The change added by this patch to runtime_suspend effectively disables the fix I did in 68942edb09 (gpio/omap: fix wakeups on level-triggered GPIOs) causing the sluggish network problems to reappear, since that GPIO IRQ is no longer causing wakeups.
Simple fix is below, which just moves the check added in $SUBJECT patch below the workaround for the edge/level fix. Can you confirm it works on Zoom3 (applies on gpio/next + my previous fix.)
I didn't notice the same problem on Overo, but I guess it's because Overo had some enabled non-wakeup GPIOs in the same bank, so it didn't bypass the level-triggered IRQ fix.
Quoted 7 lines
Quoted 5 lines
Subject: [PATCH] gpio/omap: fix broken context restore for non-OFF mode
transitionsThe fix in commit 1b12870 (gpio/omap: fix missing check in
*_runtime_suspend()) exposed another bug in the context restore path.Kevin, looks like commit 1b12870 does not exist in gpio/next?
Will update the changelog.
Because of this new problem, I have to add the patch below too, so I'll get them both queued up for Grant
In the mean time, they're availble in my for_3.5/fixes/gpio-2 branch.
Kevin
[1]
Quoted 1 line
From
afb4f0836dc3c432aa999fc98a80bf75e1481e04Mon Sep 17 00:00:00 2001
From: Kevin Hilman <[email protected]>
Date: Thu, 17 May 2012 16:42:16 -0700
Subject: [PATCH] gpio/omap: (re)fix wakeups on level-triggered GPIOs
commit 1b1287032 (gpio/omap: fix missing check in *_runtime_suspend()) broke wakeups on level-triggered GPIOs by adding the enabled non-wakeup GPIO check before the workaround that enables wakeups on level-triggered IRQs, effectively disabling that workaround.
To fix, move the enabled non-wakeup GPIO check after the level-triggered IRQ workaround.
--- drivers/gpio/gpio-omap.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/gpio/gpio-omap.c +3 -3
@@ -1157,9 +1157,6 @@ static int omap_gpio_runtime_suspend(struct device *dev)spin_lock_irqsave (&bank->lock, flags); -if (!bank->enabled_non_wakeup_gpios) - goto update_gpio_context_count; -/* * Only edges can generate a wakeup event to the PRCM. *
@@ -1180,6 +1177,9 @@ static int omap_gpio_runtime_suspend(struct device *dev)__raw_writel (wake_hi | bank->context.risingdetect, bank->base + bank->regs->risingdetect); +if (!bank->enabled_non_wakeup_gpios) + goto update_gpio_context_count; +if (bank->power_mode !=OFF_MODE ) { bank->power_mode =0 ; goto update_gpio_context_count;
Signature · 1 line
1.7.9.2
Tony Lindgren
maintainerPATCH 8/8 gpio/omap: fix missing check in *_runtime_suspend()
Quoted 5 lines · Kevin Hilman
Argh, then $SUBJECT patch here has caused brokeness in multiple ways. It managed to break both runtime suspend and runtime resume at the same time. :(
Quoted 8 lines
The change added by this patch to
runtime_suspendeffectively disables the fix I did in68942edb09(gpio/omap: fix wakeups on level-triggered GPIOs) causing the sluggish network problems to reappear, since that GPIO IRQ is no longer causing wakeups.Simple fix is below, which just moves the check added in $SUBJECT patch below the workaround for the edge/level fix. Can you confirm it works on Zoom3 (applies on gpio/next + my previous fix.)
Thanks yes having both of your fixes applied fixes the issues I was seeing, so for both:
Quoted 3 lines
I didn't notice the same problem on Overo, but I guess it's because Overo had some enabled non-wakeup GPIOs in the same bank, so it didn't bypass the level-triggered IRQ fix.
Makes sense for why it only appears on some boards.
Quoted 1 line
In the mean time, they're availble in my for_3.5/fixes/gpio-2 branch.
I'll merge that also into l-o master for some more testing before the merge window.
Regards,
Tony
PATCH 8/8 gpio/omap: fix missing check in *_runtime_suspend()
In reply to Kevin HilmanQuoted 76 lines · Kevin Hilman
Quoted 8 lines · Tony Lindgren
Quoted 7 lines · Kevin Hilman
I just noticed that this patch has caused some strange problems, notably with the GPIO IRQ used by smsc911x NIC (Overo, Zoom3, 2430SDP, etc. etc.)
The patch itself is OK, but it has exposed a bug in other parts of the context restore path that was previously hidden.
Quoted 6 lines
Quoted 3 lines
We seem to have been finding lots of GPIO bugs by just testing the network chips with GPIO IRQs. ?Can I suggest that a platform with a GPIO IRQ NIC be added to the test platforms you're using?
Yes considering the breakage it's pretty obvious the original series was never properly tested on omaps.
Agreed.
Quoted 11 lines
Regarding this fix, using gpio/next + this patch fixes nfsroot for 2430sdp, and gets zoom3 nfsroot booting going a bit better.
However, at least on zoom3 nfsroot now takes several _minutes_ to get to
login: with gpio/next + this patch. The system is totally unusable.It seems that the GPIO interrupt wake-up events are not properly working now?
Reverting the "gpio/omap: fix missing check in
*_runtime_suspend()" patch seems to fix the issue.Argh, then $SUBJECT patch here has caused brokeness in multiple ways. It managed to break both runtime suspend and runtime resume at the same time. :(
The change added by this patch to
runtime_suspendeffectively disables the fix I did in68942edb09(gpio/omap: fix wakeups on level-triggered GPIOs) causing the sluggish network problems to reappear, since that GPIO IRQ is no longer causing wakeups.Simple fix is below, which just moves the check added in $SUBJECT patch below the workaround for the edge/level fix. ?Can you confirm it works on Zoom3 (applies on gpio/next + my previous fix.)
I didn't notice the same problem on Overo, but I guess it's because Overo had some enabled non-wakeup GPIOs in the same bank, so it didn't bypass the level-triggered IRQ fix.
Quoted 7 lines
Quoted 5 lines
Subject: [PATCH] gpio/omap: fix broken context restore for non-OFF mode ?transitions
The fix in commit 1b12870 (gpio/omap: fix missing check in
*_runtime_suspend()) exposed another bug in the context restore path.Kevin, looks like commit 1b12870 does not exist in gpio/next?
Will update the changelog.
Because of this new problem, I have to add the patch below too, so I'll get them both queued up for Grant
In the mean time, they're availble in my for_3.5/fixes/gpio-2 branch.
Kevin
[1]
Fromafb4f0836dc3c432aa999fc98a80bf75e1481e04Mon Sep 17 00:00:00 2001
From: Kevin Hilman <[email protected]>
Date: Thu, 17 May 2012 16:42:16 -0700
Subject: [PATCH] gpio/omap: (re)fix wakeups on level-triggered GPIOscommit
1b1287032(gpio/omap: fix missing check in*_runtime_suspend()) broke wakeups on level-triggered GPIOs by adding the enabled non-wakeup GPIO check before the workaround that enables wakeups on level-triggered IRQs, effectively disabling that workaround.
Quoted 7 lines
To fix, move the enabled non-wakeup GPIO check after the level-triggered IRQ workaround.
Reported-by Tony Lindgren <[email protected]>Cc Santosh Shilimkar <[email protected]>Cc Tarun Kanti DebBarma <[email protected]>Signed-off-by Kevin Hilman <[email protected]>
Quoted 31 lines
---
?drivers/gpio/gpio-omap.c | ? ?6 +++---
?1 file changed, 3 insertions(+), 3 deletions(-)drivers/gpio/gpio-omap.c +0
-3@@ -1157,9 +1157,6 @@ static int omap_gpio_runtime_suspend(struct device *dev) ? ? ? ?spin_lock_irqsave(&bank->lock, flags); - ? ? ? if (!bank->enabled_non_wakeup_gpios) - ? ? ? ? ? ? ? goto update_gpio_context_count; -? ? ? ?/*
? ? ? ? * Only edges can generate a wakeup event to the PRCM. ? ? ? ? *drivers/gpio/gpio-omap.c +3
-1@@ -1180,6 +1177,9 @@ static int omap_gpio_runtime_suspend(struct device *dev) ? ? ? ? ? ? ? ?__raw_writel(wake_hi | bank->context.risingdetect, ? ? ? ? ? ? ? ? ? ? ? ? ? ? bank->base + bank->regs->risingdetect); + ? ? ? if (!bank->enabled_non_wakeup_gpios) + ? ? ? ? ? ? ? goto update_gpio_context_count; + ? ? ? ?if (bank->power_mode != OFF_MODE) { ? ? ? ? ? ? ? ?bank->power_mode = 0; ? ? ? ? ? ? ? ?goto update_gpio_context_count; --1.7.9.2
PATCH 8/8 gpio/omap: fix missing check in *_runtime_suspend()
In reply to Kevin HilmanQuoted 10 lines · Kevin Hilman
Quoted 8 lines · Tony Lindgren
Quoted 7 lines · Kevin Hilman
I just noticed that this patch has caused some strange problems, notably with the GPIO IRQ used by smsc911x NIC (Overo, Zoom3, 2430SDP, etc. etc.)
The patch itself is OK, but it has exposed a bug in other parts of the context restore path that was previously hidden.
Quoted 9 lines
Quoted 6 lines
Quoted 3 lines
We seem to have been finding lots of GPIO bugs by just testing the network chips with GPIO IRQs. ?Can I suggest that a platform with a GPIO IRQ NIC be added to the test platforms you're using?
Yes considering the breakage it's pretty obvious the original series was never properly tested on omaps.
Agreed.
Actually OMAP4 network interface as well uses the GPIO as a interrupt line but that didn't show the issue. But I agree more and more test scenario's are needed for infrastructure components like GPIO/DMA/TImer because of their multiple types of usages.
Quoted 12 lines
Quoted 11 lines
Regarding this fix, using gpio/next + this patch fixes nfsroot for 2430sdp, and gets zoom3 nfsroot booting going a bit better.
However, at least on zoom3 nfsroot now takes several _minutes_ to get to
login: with gpio/next + this patch. The system is totally unusable.It seems that the GPIO interrupt wake-up events are not properly working now?
Reverting the "gpio/omap: fix missing check in
*_runtime_suspend()" patch seems to fix the issue.
Quoted 9 lines
Argh, then $SUBJECT patch here has caused brokeness in multiple ways. It managed to break both runtime suspend and runtime resume at the same time. :(
The change added by this patch to
runtime_suspendeffectively disables the fix I did in68942edb09(gpio/omap: fix wakeups on level-triggered GPIOs) causing the sluggish network problems to reappear, since that GPIO IRQ is no longer causing wakeups.
That's pretty bad.
Quoted 36 lines
Simple fix is below, which just moves the check added in $SUBJECT patch below the workaround for the edge/level fix. ?Can you confirm it works on Zoom3 (applies on gpio/next + my previous fix.)
I didn't notice the same problem on Overo, but I guess it's because Overo had some enabled non-wakeup GPIOs in the same bank, so it didn't bypass the level-triggered IRQ fix.
Quoted 7 lines
Quoted 5 lines
Subject: [PATCH] gpio/omap: fix broken context restore for non-OFF mode ?transitions
The fix in commit 1b12870 (gpio/omap: fix missing check in
*_runtime_suspend()) exposed another bug in the context restore path.Kevin, looks like commit 1b12870 does not exist in gpio/next?
Will update the changelog.
Because of this new problem, I have to add the patch below too, so I'll get them both queued up for Grant
In the mean time, they're availble in my for_3.5/fixes/gpio-2 branch.
Kevin
[1]
Fromafb4f0836dc3c432aa999fc98a80bf75e1481e04Mon Sep 17 00:00:00 2001
From: Kevin Hilman <[email protected]>
Date: Thu, 17 May 2012 16:42:16 -0700
Subject: [PATCH] gpio/omap: (re)fix wakeups on level-triggered GPIOscommit
1b1287032(gpio/omap: fix missing check in*_runtime_suspend()) broke wakeups on level-triggered GPIOs by adding the enabled non-wakeup GPIO check before the workaround that enables wakeups on level-triggered IRQs, effectively disabling that workaround.
Quoted 8 lines
To fix, move the enabled non-wakeup GPIO check after the level-triggered IRQ workaround.
Reported-by Tony Lindgren <[email protected]>Cc Tarun Kanti DebBarma <[email protected]>Signed-off-by Kevin Hilman <[email protected]>---
Thanks for the Fix Kevin.
PATCH 8/8 gpio/omap: fix missing check in *_runtime_suspend()
In reply to Kevin HilmanQuoted 82 lines · Kevin Hilman
Tarun, Santosh,
Quoted 9 lines · Tarun Kanti DebBarma
We do checking for
bank->enabled_non_wakeup_gpiosin order to skip redundant operations. Somehow, the check got missed while doing the cleanup series.Just to make sure that we do context restore correctly in
*_runtime_resume(), thebank->workaround_enabledcheck is moved after context restore. Otherwise, it would prevent context restore whenbank->enabled_non_wakeup_gpiosis 0.Quoted 6 lines
Cc Kevin Hilman <[email protected]>Cc Tony Lindgren <[email protected]>Cc Santosh Shilimkar <[email protected]>Cc Cousson, Benoit <[email protected]>Cc Grant Likely <[email protected]>Signed-off-by Tarun Kanti DebBarma <[email protected]>I just noticed that this patch has caused some strange problems, notably with the GPIO IRQ used by smsc911x NIC (Overo, Zoom3, 2430SDP, etc. etc.)
The patch itself is OK, but it has exposed a bug in other parts of the context restore path that was previously hidden.
We seem to have been finding lots of GPIO bugs by just testing the network chips with GPIO IRQs. Can I suggest that a platform with a GPIO IRQ NIC be added to the test platforms you're using?
Quoted 19 lines
--- drivers/gpio/gpio-omap.c | 13 ++++++++----- 1 files changed, 8 insertions(+), 5 deletions(-)drivers/gpio/gpio-omap.c +3
-0@@ -1160,6 +1160,9 @@ static int omap_gpio_runtime_suspend(struct device *dev)spin_lock_irqsave (&bank->lock, flags); +if (!bank->enabled_non_wakeup_gpios) + goto update_gpio_context_count; +/* * Only edges can generate a wakeup event to the PRCM. * drivers/gpio/gpio-omap.c static int omap_gpio_runtime_resume(struct device *dev) · line 1242
Quoted 8 lines
drivers/gpio/gpio-omap.c +0
-5@@ -1235,11 +1238,6 @@ static int omap_gpio_runtime_resume(struct device *dev)__raw_writel (bank->context.risingdetect, bank->base + bank->regs->risingdetect); -if (!bank->workaround_enabled) { -spin_unlock_irqrestore (&bank->lock, flags); -return 0 ; - } -My moving this below, you exposed some buggy code that can now get executed in non-OFF mode, wherease before $SUBJECT patch, it would never be exectued in non-OFF mode.
drivers/gpio/gpio-omap.c static int omap_gpio_runtime_resume(struct device *dev) · line 1240
Quoted 3 lines
drivers/gpio/gpio-omap.c +0
-0if (bank->get_context_loss_count) { context_lost_cnt_after = bank->get_context_loss_count (bank->dev);...right below this line, we have:
if (context_lost_cnt_after != bank->context_loss_count || !context_lost_cnt_after) { omap_gpio_restore_context(bank);While we've never hit off-mode,
context_lost_cnt_afterwill always be zero. However, becasue of the !context_lost_cnt_after check there, we will *always* restore the bank context, even though context has never been lost.I have no idea why that !context_lost_cnt_after check is there, but clearly it is wrong. Now that you moved the '
workraound_enabled' check below this section, as long as off-mode is disabled, the some GPIO context will be restored here on *every* runtime PM transition.
Quoted 4 lines
The patch below fixes the problem.
Grant, after Tarun/Santosh have a chance to review/ack this, can you still get this in for 3.5? If not, getting it into -rc should be fine.
Yes. Do you have any other omap patches? Do you want to send me a pull req?
g.
Quoted 62 lines
Quoted 1 line
Kevin
From
83874df8e0dd78a3609f5ba81d608df7217fd212Mon Sep 17 00:00:00 2001
From: Kevin Hilman <[email protected]>
Date: Thu, 17 May 2012 14:52:56 -0700
Subject: [PATCH] gpio/omap: fix broken context restore for non-OFF mode
transitionsThe fix in commit 1b12870 (gpio/omap: fix missing check in
*_runtime_suspend()) exposed another bug in the context restore path.Currently, the per-bank context restore happens whenever the context loss count is different in runtime suspend and runtime resume *and* whenever the per-bank
contex_loss_count== 0:if (context_lost_cnt_after != bank->context_loss_count || !context_lost_cnt_after) { omap_gpio_restore_context(bank);Restoring context when the
context_lost_cnt_after== 0 is clearly wrong, since this will be true until the first off-mode transition (which could be never, if off-mode is never enabled.) This check causes the context to be restored on *every* runtime PM transition.Before commit 1b12870 (gpio/omap: fix missing check in
*_runtime_suspend()), this code was never executed in non-OFF mode, so there were never spurious context restores happening. After that change though, spurious context restores could happen.To fix, simply remove the !context_lost_cnt_after check. It is not needed.
This bug was found when noticing that the smc911x NIC on 3530/Overo was not working, and git bisect tracked it down to this patch. It seems that the spurious context restore was causing the smsc911x to not be properly probed on this platform.
Cc Tarun Kanti DebBarma <[email protected]>Cc Santosh Shilimkar <[email protected]>Signed-off-by Kevin Hilman <[email protected]>--- drivers/gpio/gpio-omap.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)drivers/gpio/gpio-omap.c +1
-2@@ -1238,8 +1238,7 @@ static int omap_gpio_runtime_resume(struct device *dev)if (bank->get_context_loss_count) { context_lost_cnt_after = bank->get_context_loss_count (bank->dev); -if (context_lost_cnt_after != bank->context_loss_count || - !context_lost_cnt_after) { +if (context_lost_cnt_after != bank->context_loss_count) {omap_gpio_restore_context (bank); }else {spin_unlock_irqrestore (&bank->lock, flags);Signature · 1 line
1.7.9.2
Signature · 2 lines
Grant Likely, B.Sc, P.Eng. Secret Lab Technologies, Ltd.
PATCH 8/8 gpio/omap: fix missing check in *_runtime_suspend()
Quoted 5 lines · Grant Likely
Quoted 2 lines
Grant, after Tarun/Santosh have a chance to review/ack this, can you still get this in for 3.5? If not, getting it into -rc should be fine.
Yes. Do you have any other omap patches? Do you want to send me a pull req?
We just found one more fix needed.
I'm collecting the acks/tested-bys and will have a branch for you shortly.
Kevin
PATCH 8/8 gpio/omap: fix missing check in *_runtime_suspend()
In reply to Kevin HilmanQuoted 120 lines · Kevin Hilman
Tarun, Santosh,
Quoted 9 lines · Tarun Kanti DebBarma
We do checking for
bank->enabled_non_wakeup_gpiosin order to skip redundant operations. Somehow, the check got missed while doing the cleanup series.Just to make sure that we do context restore correctly in
*_runtime_resume(), thebank->workaround_enabledcheck is moved after context restore. Otherwise, it would prevent context restore whenbank->enabled_non_wakeup_gpiosis 0.Quoted 6 lines
Cc Kevin Hilman <[email protected]>Cc Tony Lindgren <[email protected]>Cc Santosh Shilimkar <[email protected]>Cc Cousson, Benoit <[email protected]>Cc Grant Likely <[email protected]>Signed-off-by Tarun Kanti DebBarma <[email protected]>I just noticed that this patch has caused some strange problems, notably with the GPIO IRQ used by smsc911x NIC (Overo, Zoom3, 2430SDP, etc. etc.)
The patch itself is OK, but it has exposed a bug in other parts of the context restore path that was previously hidden.
We seem to have been finding lots of GPIO bugs by just testing the network chips with GPIO IRQs. ?Can I suggest that a platform with a GPIO IRQ NIC be added to the test platforms you're using?
Quoted 19 lines
---
?drivers/gpio/gpio-omap.c | ? 13 ++++++++-----
?1 files changed, 8 insertions(+), 5 deletions(-)drivers/gpio/gpio-omap.c +3
-0@@ -1160,6 +1160,9 @@ static int omap_gpio_runtime_suspend(struct device *dev) ? ? ? spin_lock_irqsave(&bank->lock, flags); + ? ? if (!bank->enabled_non_wakeup_gpios) + ? ? ? ? ? ? goto update_gpio_context_count; +? ? ? /*
? ? ? ?* Only edges can generate a wakeup event to the PRCM. ? ? ? ?*drivers/gpio/gpio-omap.c static int omap_gpio_runtime_resume(struct device *dev) · line 1242
Quoted 8 lines
drivers/gpio/gpio-omap.c +0
-5@@ -1235,11 +1238,6 @@ static int omap_gpio_runtime_resume(struct device *dev) ? ? ? __raw_writel(bank->context.risingdetect, ? ? ? ? ? ? ? ? ? ?bank->base + bank->regs->risingdetect); - ? ? if (!bank->workaround_enabled) { - ? ? ? ? ? ? spin_unlock_irqrestore(&bank->lock, flags); - ? ? ? ? ? ? return 0; - ? ? } -My moving this below, you exposed some buggy code that can now get executed in non-OFF mode, wherease before $SUBJECT patch, it would never be exectued in non-OFF mode.
Quoted 3 lines
drivers/gpio/gpio-omap.c +0
-0? ? ? if (bank->get_context_loss_count) {? ? ? ? ? ? ?
context_lost_cnt_after=
? ? ? ? ? ? ? ? ? ? ?bank->get_context_loss_count(bank->dev);...right below this line, we have:
? ? ? ? ? ? ? ?if (
context_lost_cnt_after!=bank->context_loss_count|| ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?!context_lost_cnt_after) { ? ? ? ? ? ? ? ? ? ? ? ?omap_gpio_restore_context(bank);While we've never hit off-mode,
context_lost_cnt_afterwill always be zero. ?However, becasue of the !context_lost_cnt_after check there, we will *always* restore the bank context, even though context has never been lost.I have no idea why that !context_lost_cnt_after check is there, but clearly it is wrong. ?Now that you moved the '
workraound_enabled' check below this section, as long as off-mode is disabled, the some GPIO context will be restored here on *every* runtime PM transition.The patch below fixes the problem.
Grant, after Tarun/Santosh have a chance to review/ack this, can you still get this in for 3.5? ?If not, getting it into -rc should be fine.
Kevin
From
83874df8e0dd78a3609f5ba81d608df7217fd212Mon Sep 17 00:00:00 2001
From: Kevin Hilman <[email protected]>
Date: Thu, 17 May 2012 14:52:56 -0700
Subject: [PATCH] gpio/omap: fix broken context restore for non-OFF mode ?transitionsThe fix in commit 1b12870 (gpio/omap: fix missing check in
*_runtime_suspend()) exposed another bug in the context restore path.Currently, the per-bank context restore happens whenever the context loss count is different in runtime suspend and runtime resume *and* whenever the per-bank
contex_loss_count== 0:? ? ? ?if (
context_lost_cnt_after!=bank->context_loss_count|| ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?!context_lost_cnt_after) { ? ? ? ? ? ? ? ?omap_gpio_restore_context(bank);Restoring context when the
context_lost_cnt_after== 0 is clearly wrong, since this will be true until the first off-mode transition (which could be never, if off-mode is never enabled.) ?This check causes the context to be restored on *every* runtime PM transition.Before commit 1b12870 (gpio/omap: fix missing check in
*_runtime_suspend()), this code was never executed in non-OFF mode, so there were never spurious context restores happening. ?After that change though, spurious context restores could happen.To fix, simply remove the !context_lost_cnt_after check. It is not needed.
Quoted 8 lines
This bug was found when noticing that the smc911x NIC on 3530/Overo was not working, and git bisect tracked it down to this patch. ?It seems that the spurious context restore was causing the smsc911x to not be properly probed on this platform.
Cc Tarun Kanti DebBarma <[email protected]>Cc Santosh Shilimkar <[email protected]>Signed-off-by Kevin Hilman <[email protected]>
Quoted 21 lines
---
?drivers/gpio/gpio-omap.c | ? ?3 +--
?1 file changed, 1 insertion(+), 2 deletions(-)drivers/gpio/gpio-omap.c +0
-0@@ -1238,8 +1238,7 @@ static int omap_gpio_runtime_resume(struct device *dev) ? ? ? ?if (bank->get_context_loss_count) {? ? ? ? ? ? ? ?context_lost_cnt_after =
? ? ? ? ? ? ? ? ? ? ? ?bank->get_context_loss_count(bank->dev);
- ? ? ? ? ? ? ? if (context_lost_cnt_after!=bank->context_loss_count||
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? !context_lost_cnt_after) {
+ ? ? ? ? ? ? ? if (context_lost_cnt_after!=bank->context_loss_count) { ? ? ? ? ? ? ? ? ? ? ? ?omap_gpio_restore_context(bank);
? ? ? ? ? ? ? ?} else {
? ? ? ? ? ? ? ? ? ? ? ?spin_unlock_irqrestore(&bank->lock, flags); --
1.7.9.2
PATCH 8/8 gpio/omap: fix missing check in *_runtime_suspend()
In reply to Kevin HilmanQuoted 23 lines · Kevin Hilman
Tarun, Santosh,
Quoted 9 lines · Tarun Kanti DebBarma
We do checking for
bank->enabled_non_wakeup_gpiosin order to skip redundant operations. Somehow, the check got missed while doing the cleanup series.Just to make sure that we do context restore correctly in
*_runtime_resume(), thebank->workaround_enabledcheck is moved after context restore. Otherwise, it would prevent context restore whenbank->enabled_non_wakeup_gpiosis 0.Quoted 6 lines
Cc Kevin Hilman <[email protected]>Cc Tony Lindgren <[email protected]>Cc Santosh Shilimkar <[email protected]>Cc Cousson, Benoit <[email protected]>Cc Grant Likely <[email protected]>Signed-off-by Tarun Kanti DebBarma <[email protected]>I just noticed that this patch has caused some strange problems, notably with the GPIO IRQ used by smsc911x NIC (Overo, Zoom3, 2430SDP, etc. etc.)
Quoted 6 lines
The patch itself is OK, but it has exposed a bug in other parts of the context restore path that was previously hidden.
We seem to have been finding lots of GPIO bugs by just testing the network chips with GPIO IRQs. ?Can I suggest that a platform with a GPIO IRQ NIC be added to the test platforms you're using?
Able to reproduce the problem. Tested on Zoom3 by enabling NFS. Verified the time difference for the filesystem to boot. Without your fixes it takes
around 60 seconds while with the fix it takes less than 7 seconds.
Quoted 120 lines
Quoted 20 lines
---
?drivers/gpio/gpio-omap.c | ? 13 ++++++++-----
?1 files changed, 8 insertions(+), 5 deletions(-)drivers/gpio/gpio-omap.c +3
-0@@ -1160,6 +1160,9 @@ static int omap_gpio_runtime_suspend(struct device *dev) ? ? ? spin_lock_irqsave(&bank->lock, flags); + ? ? if (!bank->enabled_non_wakeup_gpios) + ? ? ? ? ? ? goto update_gpio_context_count; +? ? ? /*
? ? ? ?* Only edges can generate a wakeup event to the PRCM. ? ? ? ?*drivers/gpio/gpio-omap.c static int omap_gpio_runtime_resume(struct device *dev) · line 1242
Quoted 8 lines
drivers/gpio/gpio-omap.c +0
-5@@ -1235,11 +1238,6 @@ static int omap_gpio_runtime_resume(struct device *dev) ? ? ? __raw_writel(bank->context.risingdetect, ? ? ? ? ? ? ? ? ? ?bank->base + bank->regs->risingdetect); - ? ? if (!bank->workaround_enabled) { - ? ? ? ? ? ? spin_unlock_irqrestore(&bank->lock, flags); - ? ? ? ? ? ? return 0; - ? ? } -My moving this below, you exposed some buggy code that can now get executed in non-OFF mode, wherease before $SUBJECT patch, it would never be exectued in non-OFF mode.
Quoted 3 lines
drivers/gpio/gpio-omap.c +0
-0? ? ? if (bank->get_context_loss_count) {? ? ? ? ? ? ?
context_lost_cnt_after=
? ? ? ? ? ? ? ? ? ? ?bank->get_context_loss_count(bank->dev);...right below this line, we have:
? ? ? ? ? ? ? ?if (
context_lost_cnt_after!=bank->context_loss_count|| ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?!context_lost_cnt_after) { ? ? ? ? ? ? ? ? ? ? ? ?omap_gpio_restore_context(bank);While we've never hit off-mode,
context_lost_cnt_afterwill always be zero. ?However, becasue of the !context_lost_cnt_after check there, we will *always* restore the bank context, even though context has never been lost.I have no idea why that !context_lost_cnt_after check is there, but clearly it is wrong. ?Now that you moved the '
workraound_enabled' check below this section, as long as off-mode is disabled, the some GPIO context will be restored here on *every* runtime PM transition.The patch below fixes the problem.
Grant, after Tarun/Santosh have a chance to review/ack this, can you still get this in for 3.5? ?If not, getting it into -rc should be fine.
Kevin
From
83874df8e0dd78a3609f5ba81d608df7217fd212Mon Sep 17 00:00:00 2001
From: Kevin Hilman <[email protected]>
Date: Thu, 17 May 2012 14:52:56 -0700
Subject: [PATCH] gpio/omap: fix broken context restore for non-OFF mode ?transitionsThe fix in commit 1b12870 (gpio/omap: fix missing check in
*_runtime_suspend()) exposed another bug in the context restore path.Currently, the per-bank context restore happens whenever the context loss count is different in runtime suspend and runtime resume *and* whenever the per-bank
contex_loss_count== 0:? ? ? ?if (
context_lost_cnt_after!=bank->context_loss_count|| ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?!context_lost_cnt_after) { ? ? ? ? ? ? ? ?omap_gpio_restore_context(bank);Restoring context when the
context_lost_cnt_after== 0 is clearly wrong, since this will be true until the first off-mode transition (which could be never, if off-mode is never enabled.) ?This check causes the context to be restored on *every* runtime PM transition.Before commit 1b12870 (gpio/omap: fix missing check in
*_runtime_suspend()), this code was never executed in non-OFF mode, so there were never spurious context restores happening. ?After that change though, spurious context restores could happen.To fix, simply remove the !context_lost_cnt_after check. It is not needed.
This bug was found when noticing that the smc911x NIC on 3530/Overo was not working, and git bisect tracked it down to this patch. ?It seems that the spurious context restore was causing the smsc911x to not be properly probed on this platform.
Cc Tarun Kanti DebBarma <[email protected]>Cc Santosh Shilimkar <[email protected]>Signed-off-by Kevin Hilman <[email protected]>---
?drivers/gpio/gpio-omap.c | ? ?3 +--
?1 file changed, 1 insertion(+), 2 deletions(-)drivers/gpio/gpio-omap.c +0
-0@@ -1238,8 +1238,7 @@ static int omap_gpio_runtime_resume(struct device *dev) ? ? ? ?if (bank->get_context_loss_count) {? ? ? ? ? ? ? ?context_lost_cnt_after =
? ? ? ? ? ? ? ? ? ? ? ?bank->get_context_loss_count(bank->dev);
- ? ? ? ? ? ? ? if (context_lost_cnt_after!=bank->context_loss_count||
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? !context_lost_cnt_after) {
+ ? ? ? ? ? ? ? if (context_lost_cnt_after!=bank->context_loss_count) { ? ? ? ? ? ? ? ? ? ? ? ?omap_gpio_restore_context(bank);
? ? ? ? ? ? ? ?} else {
? ? ? ? ? ? ? ? ? ? ? ?spin_unlock_irqrestore(&bank->lock, flags); --
1.7.9.2
cover gpio/omap: remaining cleanups and fix
In reply to Tarun Kanti DebBarmaQuoted 21 lines · Tarun Kanti DebBarma
Here are the remaining cleanup patches. There are broadly two categories of cleanups.
Cat-1: Those missed while introducing new feature like
SPARSE_IRQ
handling and DT support; use edge/level handlers from generic IRQ framework.Cat-2: Removal of redundant fields from struct gpio_bank{} as a
result of they being already covered by members within context field of struct gpio_bank{}.Reference:
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
Commit:66f75a5d028beaf67c931435fdc3e7823125730c(Linux 3.4-rc4)Series is available for reference here:
git://gitorious.org/~tarunkanti/omap-sw-develoment/tarunkantis-linux-omap-dev.gitfor_3.5/gpio_more_cleanup_fixesTest info:
OMAP2+ platforms: OMAP2430SDP, OMAP3430SDP, OMAP4430SDP
OMAP1: Bootup test on OMAP1710SDP.
Quoted 14 lines
Cc: Kevin Hilman <[email protected]>
Cc: Tony Lindgren <[email protected]>
Cc: Santosh Shilimkar <[email protected]>
Cc: Cousson, Benoit <[email protected]>
Cc: Grant Likely <[email protected]>
Tarun Kanti DebBarma (8):
gpio/omap: removevirtual_irq_startvariable
gpio/omap: removesaved_fallingdetect,saved_risingdetectgpio/omap: removesuspend_wakeupfield from structgpio_bankgpio/omap: removesaved_wakeupfield from structgpio_bankgpio/omap: remove retrigger variable ingpio_irq_handlergpio/omap: remove suspend/resume callbacks
gpio/omap: removecpu_is_omapxxxx()checks from*_runtime_resume()gpio/omap: fix missing check in*_runtime_suspend()
The changes look fine to me. Which branch should this series be merged through? It can either go via arm-soc or my gpio/next branch.
g.
cover gpio/omap: remaining cleanups and fix
Quoted 37 lines · Grant Likely
Quoted 21 lines · Tarun Kanti DebBarma
Here are the remaining cleanup patches. There are broadly two categories of cleanups.
Cat-1: Those missed while introducing new feature like
SPARSE_IRQ
handling and DT support; use edge/level handlers from generic IRQ framework.Cat-2: Removal of redundant fields from struct gpio_bank{} as a
result of they being already covered by members within context field of struct gpio_bank{}.Reference:
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
Commit:66f75a5d028beaf67c931435fdc3e7823125730c(Linux 3.4-rc4)Series is available for reference here:
git://gitorious.org/~tarunkanti/omap-sw-develoment/tarunkantis-linux-omap-dev.gitfor_3.5/gpio_more_cleanup_fixesTest info:
OMAP2+ platforms: OMAP2430SDP, OMAP3430SDP, OMAP4430SDP
OMAP1: Bootup test on OMAP1710SDP.Quoted 14 lines
Cc: Kevin Hilman <[email protected]>
Cc: Tony Lindgren <[email protected]>
Cc: Santosh Shilimkar <[email protected]>
Cc: Cousson, Benoit <[email protected]>
Cc: Grant Likely <[email protected]>
Tarun Kanti DebBarma (8):
gpio/omap: removevirtual_irq_startvariable
gpio/omap: removesaved_fallingdetect,saved_risingdetectgpio/omap: removesuspend_wakeupfield from structgpio_bankgpio/omap: removesaved_wakeupfield from structgpio_bankgpio/omap: remove retrigger variable ingpio_irq_handlergpio/omap: remove suspend/resume callbacks
gpio/omap: removecpu_is_omapxxxx()checks from*_runtime_resume()gpio/omap: fix missing check in*_runtime_suspend()
Quoted 2 lines
The changes look fine to me. Which branch should this series be merged through? It can either go via arm-soc or my gpio/next branch.
This needs a little more review/testing on OMAP.
Expect a pull request from me when it's ready and you can take it through gpio/nex.
Thanks,
Kevin
cover gpio/omap: remaining cleanups and fix
Quoted 39 lines · Kevin Hilman
Quoted 37 lines · Grant Likely
Quoted 35 lines · Tarun Kanti DebBarma
Here are the remaining cleanup patches. There are broadly two categories of cleanups.
Cat-1: Those missed while introducing new feature like
SPARSE_IRQ? ? ? ?handling and DT support; use edge/level handlers from ? ? ? ?generic IRQ framework.Cat-2: Removal of redundant fields from struct gpio_bank{} as a ? ? ? ?result of they being already covered by members within ? ? ? ?context field of struct gpio_bank{}.
Reference:
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
Commit:66f75a5d028beaf67c931435fdc3e7823125730c(Linux 3.4-rc4)Series is available for reference here:
git://gitorious.org/~tarunkanti/omap-sw-develoment/tarunkantis-linux-omap-dev.gitfor_3.5/gpio_more_cleanup_fixesTest info:
OMAP2+ platforms: OMAP2430SDP, OMAP3430SDP, OMAP4430SDP
OMAP1: Bootup test on OMAP1710SDP.Cc: Kevin Hilman <[email protected]>
Cc: Tony Lindgren <[email protected]>
Cc: Santosh Shilimkar <[email protected]>
Cc: Cousson, Benoit <[email protected]>
Cc: Grant Likely <[email protected]>
Tarun Kanti DebBarma (8):
? gpio/omap: removevirtual_irq_startvariable
? gpio/omap: removesaved_fallingdetect,saved_risingdetect? gpio/omap: removesuspend_wakeupfield from structgpio_bank? gpio/omap: removesaved_wakeupfield from structgpio_bank? gpio/omap: remove retrigger variable ingpio_irq_handler? gpio/omap: remove suspend/resume callbacks
? gpio/omap: removecpu_is_omapxxxx()checks from*_runtime_resume()? gpio/omap: fix missing check in*_runtime_suspend()
Quoted 7 lines
Quoted 2 lines
The changes look fine to me. ?Which branch should this series be merged through? ?It can either go via arm-soc or my gpio/next branch.
This needs a little more review/testing on OMAP.
Expect a pull request from me when it's ready and you can take it through gpio/nex.
Okay.
g.
cover gpio/omap: remaining cleanups and fix
In reply to Tarun Kanti DebBarmaTarun,
Quoted 21 lines · Tarun Kanti DebBarma
Here are the remaining cleanup patches. There are broadly two categories of cleanups.
Cat-1: Those missed while introducing new feature like
SPARSE_IRQ
handling and DT support; use edge/level handlers from generic IRQ framework.Cat-2: Removal of redundant fields from struct gpio_bank{} as a
result of they being already covered by members within context field of struct gpio_bank{}.Reference:
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
Commit:66f75a5d028beaf67c931435fdc3e7823125730c(Linux 3.4-rc4)Series is available for reference here:
git://gitorious.org/~tarunkanti/omap-sw-develoment/tarunkantis-linux-omap-dev.gitfor_3.5/gpio_more_cleanup_fixesTest info:
OMAP2+ platforms: OMAP2430SDP, OMAP3430SDP, OMAP4430SDP
OMAP1: Bootup test on OMAP1710SDP.
Quoted 15 lines
Cc: Kevin Hilman <[email protected]>
Cc: Tony Lindgren <[email protected]>
Cc: Santosh Shilimkar <[email protected]>
Cc: Cousson, Benoit <[email protected]>
Cc: Grant Likely <[email protected]>
Tarun Kanti DebBarma (8):
gpio/omap: removevirtual_irq_startvariable
gpio/omap: removesaved_fallingdetect,saved_risingdetectgpio/omap: removesuspend_wakeupfield from structgpio_bankgpio/omap: removesaved_wakeupfield from structgpio_bankgpio/omap: remove retrigger variable ingpio_irq_handlergpio/omap: remove suspend/resume callbacks
gpio/omap: removecpu_is_omapxxxx()checks from*_runtime_resume()gpio/omap: fix missing check in*_runtime_suspend()
I have reviewed the remaining 3 patches from this series. Thanks.
Regards
santosh
cover gpio/omap: remaining cleanups and fix
In reply to Tarun Kanti DebBarmaHi Tarun,
Quoted 11 lines · Tarun Kanti DebBarma
Here are the remaining cleanup patches. There are broadly two categories of cleanups.
Cat-1: Those missed while introducing new feature like
SPARSE_IRQ? ? ? handling and DT support; use edge/level handlers from ? ? ? generic IRQ framework.Cat-2: Removal of redundant fields from struct gpio_bank{} as a ? ? ? result of they being already covered by members within ? ? ? context field of struct gpio_bank{}.
Quoted 5 lines
Reference:
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
Commit:66f75a5d028beaf67c931435fdc3e7823125730c(Linux 3.4-rc4)Series is available for reference here:
git://gitorious.org/~tarunkanti/omap-sw-develoment/tarunkantis-linux-omap-dev.gitfor_3.5/gpio_more_cleanup_fixes
I tested this series from your tree on Beagle-XM retention, offmode, suspend to mem and resume with and and without enabling offmode from sysfs was checked and found retention count and off mode count was getting incremented.
Also ran few test cases available from here[1] to request and release gpio, test cases passed.
Feel Free to add:
--
Thanks
Govindraj.R
[1]: http://gitorious.org/omap-ddt/omap-ddt/trees/master/gpio/test_code/utils/gpio_mods
cover gpio/omap: remaining cleanups and fix
Hi Govindraj,
"Raja, Govindraj" <[email protected]> writes:
[...]
Quoted 11 lines
I tested this series from your tree on Beagle-XM retention, offmode, suspend to mem and resume with and and without enabling offmode from sysfs was checked and found retention count and off mode count was getting incremented.
Also ran few test cases available from here[1] to request and release gpio, test cases passed.
Feel Free to add:
Tested-by Govindraj.R <[email protected]>
Awesome!
Many thanks for the testing. It *greatly* helps when maintainers can see more testing by folks other than the author.
Thank you!
Kevin