Re: [PATCH v9 11/25] gpio/omap: cleanup omap_gpio_mod_init function
DebBarma, Tarun Kanti tarun.ka...@ti.com writes: On Wed, Apr 25, 2012 at 7:15 PM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: [...] Correction. Don't email your patch in any way to the stable folk _before_ it has been taken into Linus' tree. However, you _may_ add in the patch attributations a Cc: sta...@vger.kernel.org tag if you want the stable folk to automatically pick up your patch when it _does_ end up in Linus' tree. But... make sure that git send-email or whatever doesn't automatically add that to the recipients for the emailed patch. If you send the stable people a patch before its in mainline, you'll get a whinge telling you to read Documentation/stable_kernel_rules.txt Alright, I will add Cc: sta...@vger.kernel.org tag in the patch. If you do that, be sure to use --suppress-cc=bodycc when you send it with git-send-email. Kevin -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 11/25] gpio/omap: cleanup omap_gpio_mod_init function
On Wed, Apr 25, 2012 at 10:04 AM, DebBarma, Tarun Kanti tarun.ka...@ti.com wrote: On Tue, Apr 24, 2012 at 9:34 PM, Tony Lindgren t...@atomide.com wrote: * DebBarma, Tarun Kanti tarun.ka...@ti.com [120424 08:40]: Hi Janusz, On Tue, Apr 24, 2012 at 12:24 AM, DebBarma, Tarun Kanti tarun.ka...@ti.com wrote: On Sat, Apr 21, 2012 at 7:33 PM, Janusz Krzysztofik jkrzy...@tis.icnet.pl wrote: On Thursday 02 of February 2012 23:00:37 Tarun Kanti DebBarma wrote: With register offsets now defined for respective OMAP versions we can get rid of cpu_class_* checks. This function now has common initialization code for all OMAP versions... Signed-off-by: Tarun Kanti DebBarma tarun.ka...@ti.com Signed-off-by: Charulatha V ch...@ti.com Reviewed-by: Santosh Shilimkar santosh.shilim...@ti.com Acked-by: Tony Lindgren t...@atomide.com Sorry for being so late with my comment for chanes already present in mainline, but this patch breaks GPIO on Amstrad Delta at least, and I have neither enough spare time nor enough experience with non OMAP1 machines to provide a solution myself. Yes, I looked at the omap_gpio_mod_init() and OMAP1 configurations are overwritten. Also looks like there is issue in making distinction between omap15xx and omap16xx. I will post a patch and you can help me testing it in OMAP1 platform. Thanks for pointing this out. ... Here is the patch, with attachment as well. I have just tested on OMAP4 platform. Testing on other OMAP2+ platforms is pending. In the meantime can you please validate on OMAP1 platform and confirm? Thanks. -- Tarun From: Tarun Kanti DebBarma tarun.ka...@ti.com Date: Tue, 24 Apr 2012 20:34:32 +0530 Subject: [PATCH] gpio/omap: fix omap1 register overwrite in omap_gpio_mod_init Initialization of irqenable, irqstatus registers is the common operation done in this function for all OMAP platforms, viz. OMAP1, OMAP2+. The latter _gpio_rmw()'s to irqenable register was overwriting the correctly programmed OMAP1 value at the beginning. As a result, even though it worked on OMAP2+ platforms it was breaking OMAP1 functionality. Sounds like the original patch was never tested on omap1? That's right, only bootup test was done on OMAP1710-SDP. On closer observation it is found that the first _gpio_rmw() which is supposedly done to take care of OMAP1 platform is generic enough and takes care of OMAP2+ platform as well. Therefore remove the latter _gpio_rmw() to irqenable as they are redundant. Also, changing the sequence and logic of initializing the irqstatus. Please mention also the breaking commit here and get this fix merged as a regression as soon as it's tested for the current -rc series. Sure, I will do that! Looks like it is regression on 3.4 as well so CC stable when you post the patch. Regards Santosh -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 11/25] gpio/omap: cleanup omap_gpio_mod_init function
On Wed, Apr 25, 2012 at 12:09 PM, Shilimkar, Santosh santosh.shilim...@ti.com wrote: On Wed, Apr 25, 2012 at 10:04 AM, DebBarma, Tarun Kanti tarun.ka...@ti.com wrote: On Tue, Apr 24, 2012 at 9:34 PM, Tony Lindgren t...@atomide.com wrote: * DebBarma, Tarun Kanti tarun.ka...@ti.com [120424 08:40]: Hi Janusz, On Tue, Apr 24, 2012 at 12:24 AM, DebBarma, Tarun Kanti tarun.ka...@ti.com wrote: On Sat, Apr 21, 2012 at 7:33 PM, Janusz Krzysztofik jkrzy...@tis.icnet.pl wrote: On Thursday 02 of February 2012 23:00:37 Tarun Kanti DebBarma wrote: With register offsets now defined for respective OMAP versions we can get rid of cpu_class_* checks. This function now has common initialization code for all OMAP versions... Signed-off-by: Tarun Kanti DebBarma tarun.ka...@ti.com Signed-off-by: Charulatha V ch...@ti.com Reviewed-by: Santosh Shilimkar santosh.shilim...@ti.com Acked-by: Tony Lindgren t...@atomide.com Sorry for being so late with my comment for chanes already present in mainline, but this patch breaks GPIO on Amstrad Delta at least, and I have neither enough spare time nor enough experience with non OMAP1 machines to provide a solution myself. Yes, I looked at the omap_gpio_mod_init() and OMAP1 configurations are overwritten. Also looks like there is issue in making distinction between omap15xx and omap16xx. I will post a patch and you can help me testing it in OMAP1 platform. Thanks for pointing this out. ... Here is the patch, with attachment as well. I have just tested on OMAP4 platform. Testing on other OMAP2+ platforms is pending. In the meantime can you please validate on OMAP1 platform and confirm? Thanks. -- Tarun From: Tarun Kanti DebBarma tarun.ka...@ti.com Date: Tue, 24 Apr 2012 20:34:32 +0530 Subject: [PATCH] gpio/omap: fix omap1 register overwrite in omap_gpio_mod_init Initialization of irqenable, irqstatus registers is the common operation done in this function for all OMAP platforms, viz. OMAP1, OMAP2+. The latter _gpio_rmw()'s to irqenable register was overwriting the correctly programmed OMAP1 value at the beginning. As a result, even though it worked on OMAP2+ platforms it was breaking OMAP1 functionality. Sounds like the original patch was never tested on omap1? That's right, only bootup test was done on OMAP1710-SDP. On closer observation it is found that the first _gpio_rmw() which is supposedly done to take care of OMAP1 platform is generic enough and takes care of OMAP2+ platform as well. Therefore remove the latter _gpio_rmw() to irqenable as they are redundant. Also, changing the sequence and logic of initializing the irqstatus. Please mention also the breaking commit here and get this fix merged as a regression as soon as it's tested for the current -rc series. Sure, I will do that! Looks like it is regression on 3.4 as well so CC stable when you post the patch. Ok, I will do that. -- Tarun Regards Santosh -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 11/25] gpio/omap: cleanup omap_gpio_mod_init function
On Wed, Apr 25, 2012 at 06:24:14PM +0530, DebBarma, Tarun Kanti wrote: On Wed, Apr 25, 2012 at 12:09 PM, Shilimkar, Santosh santosh.shilim...@ti.com wrote: On Wed, Apr 25, 2012 at 10:04 AM, DebBarma, Tarun Kanti tarun.ka...@ti.com wrote: On Tue, Apr 24, 2012 at 9:34 PM, Tony Lindgren t...@atomide.com wrote: * DebBarma, Tarun Kanti tarun.ka...@ti.com [120424 08:40]: Hi Janusz, On Tue, Apr 24, 2012 at 12:24 AM, DebBarma, Tarun Kanti tarun.ka...@ti.com wrote: On Sat, Apr 21, 2012 at 7:33 PM, Janusz Krzysztofik jkrzy...@tis.icnet.pl wrote: On Thursday 02 of February 2012 23:00:37 Tarun Kanti DebBarma wrote: With register offsets now defined for respective OMAP versions we can get rid of cpu_class_* checks. This function now has common initialization code for all OMAP versions... Signed-off-by: Tarun Kanti DebBarma tarun.ka...@ti.com Signed-off-by: Charulatha V ch...@ti.com Reviewed-by: Santosh Shilimkar santosh.shilim...@ti.com Acked-by: Tony Lindgren t...@atomide.com Sorry for being so late with my comment for chanes already present in mainline, but this patch breaks GPIO on Amstrad Delta at least, and I have neither enough spare time nor enough experience with non OMAP1 machines to provide a solution myself. Yes, I looked at the omap_gpio_mod_init() and OMAP1 configurations are overwritten. Also looks like there is issue in making distinction between omap15xx and omap16xx. I will post a patch and you can help me testing it in OMAP1 platform. Thanks for pointing this out. ... Here is the patch, with attachment as well. I have just tested on OMAP4 platform. Testing on other OMAP2+ platforms is pending. In the meantime can you please validate on OMAP1 platform and confirm? Thanks. -- Tarun From: Tarun Kanti DebBarma tarun.ka...@ti.com Date: Tue, 24 Apr 2012 20:34:32 +0530 Subject: [PATCH] gpio/omap: fix omap1 register overwrite in omap_gpio_mod_init Initialization of irqenable, irqstatus registers is the common operation done in this function for all OMAP platforms, viz. OMAP1, OMAP2+. The latter _gpio_rmw()'s to irqenable register was overwriting the correctly programmed OMAP1 value at the beginning. As a result, even though it worked on OMAP2+ platforms it was breaking OMAP1 functionality. Sounds like the original patch was never tested on omap1? That's right, only bootup test was done on OMAP1710-SDP. On closer observation it is found that the first _gpio_rmw() which is supposedly done to take care of OMAP1 platform is generic enough and takes care of OMAP2+ platform as well. Therefore remove the latter _gpio_rmw() to irqenable as they are redundant. Also, changing the sequence and logic of initializing the irqstatus. Please mention also the breaking commit here and get this fix merged as a regression as soon as it's tested for the current -rc series. Sure, I will do that! Looks like it is regression on 3.4 as well so CC stable when you post the patch. Ok, I will do that. Correction. Don't email your patch in any way to the stable folk _before_ it has been taken into Linus' tree. However, you _may_ add in the patch attributations a Cc: sta...@vger.kernel.org tag if you want the stable folk to automatically pick up your patch when it _does_ end up in Linus' tree. But... make sure that git send-email or whatever doesn't automatically add that to the recipients for the emailed patch. If you send the stable people a patch before its in mainline, you'll get a whinge telling you to read Documentation/stable_kernel_rules.txt -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 11/25] gpio/omap: cleanup omap_gpio_mod_init function
On Wed, Apr 25, 2012 at 7:15 PM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Wed, Apr 25, 2012 at 06:24:14PM +0530, DebBarma, Tarun Kanti wrote: On Wed, Apr 25, 2012 at 12:09 PM, Shilimkar, Santosh santosh.shilim...@ti.com wrote: On Wed, Apr 25, 2012 at 10:04 AM, DebBarma, Tarun Kanti tarun.ka...@ti.com wrote: On Tue, Apr 24, 2012 at 9:34 PM, Tony Lindgren t...@atomide.com wrote: * DebBarma, Tarun Kanti tarun.ka...@ti.com [120424 08:40]: Hi Janusz, On Tue, Apr 24, 2012 at 12:24 AM, DebBarma, Tarun Kanti tarun.ka...@ti.com wrote: On Sat, Apr 21, 2012 at 7:33 PM, Janusz Krzysztofik jkrzy...@tis.icnet.pl wrote: On Thursday 02 of February 2012 23:00:37 Tarun Kanti DebBarma wrote: With register offsets now defined for respective OMAP versions we can get rid of cpu_class_* checks. This function now has common initialization code for all OMAP versions... Signed-off-by: Tarun Kanti DebBarma tarun.ka...@ti.com Signed-off-by: Charulatha V ch...@ti.com Reviewed-by: Santosh Shilimkar santosh.shilim...@ti.com Acked-by: Tony Lindgren t...@atomide.com Sorry for being so late with my comment for chanes already present in mainline, but this patch breaks GPIO on Amstrad Delta at least, and I have neither enough spare time nor enough experience with non OMAP1 machines to provide a solution myself. Yes, I looked at the omap_gpio_mod_init() and OMAP1 configurations are overwritten. Also looks like there is issue in making distinction between omap15xx and omap16xx. I will post a patch and you can help me testing it in OMAP1 platform. Thanks for pointing this out. ... Here is the patch, with attachment as well. I have just tested on OMAP4 platform. Testing on other OMAP2+ platforms is pending. In the meantime can you please validate on OMAP1 platform and confirm? Thanks. -- Tarun From: Tarun Kanti DebBarma tarun.ka...@ti.com Date: Tue, 24 Apr 2012 20:34:32 +0530 Subject: [PATCH] gpio/omap: fix omap1 register overwrite in omap_gpio_mod_init Initialization of irqenable, irqstatus registers is the common operation done in this function for all OMAP platforms, viz. OMAP1, OMAP2+. The latter _gpio_rmw()'s to irqenable register was overwriting the correctly programmed OMAP1 value at the beginning. As a result, even though it worked on OMAP2+ platforms it was breaking OMAP1 functionality. Sounds like the original patch was never tested on omap1? That's right, only bootup test was done on OMAP1710-SDP. On closer observation it is found that the first _gpio_rmw() which is supposedly done to take care of OMAP1 platform is generic enough and takes care of OMAP2+ platform as well. Therefore remove the latter _gpio_rmw() to irqenable as they are redundant. Also, changing the sequence and logic of initializing the irqstatus. Please mention also the breaking commit here and get this fix merged as a regression as soon as it's tested for the current -rc series. Sure, I will do that! Looks like it is regression on 3.4 as well so CC stable when you post the patch. Ok, I will do that. Correction. Don't email your patch in any way to the stable folk _before_ it has been taken into Linus' tree. However, you _may_ add in the patch attributations a Cc: sta...@vger.kernel.org tag if you want the stable folk to automatically pick up your patch when it _does_ end up in Linus' tree. But... make sure that git send-email or whatever doesn't automatically add that to the recipients for the emailed patch. If you send the stable people a patch before its in mainline, you'll get a whinge telling you to read Documentation/stable_kernel_rules.txt Alright, I will add Cc: sta...@vger.kernel.org tag in the patch. Thanks. -- Tarun -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 11/25] gpio/omap: cleanup omap_gpio_mod_init function
Hi Janusz, On Tue, Apr 24, 2012 at 12:24 AM, DebBarma, Tarun Kanti tarun.ka...@ti.com wrote: On Sat, Apr 21, 2012 at 7:33 PM, Janusz Krzysztofik jkrzy...@tis.icnet.pl wrote: On Thursday 02 of February 2012 23:00:37 Tarun Kanti DebBarma wrote: With register offsets now defined for respective OMAP versions we can get rid of cpu_class_* checks. This function now has common initialization code for all OMAP versions... Signed-off-by: Tarun Kanti DebBarma tarun.ka...@ti.com Signed-off-by: Charulatha V ch...@ti.com Reviewed-by: Santosh Shilimkar santosh.shilim...@ti.com Acked-by: Tony Lindgren t...@atomide.com Sorry for being so late with my comment for chanes already present in mainline, but this patch breaks GPIO on Amstrad Delta at least, and I have neither enough spare time nor enough experience with non OMAP1 machines to provide a solution myself. Yes, I looked at the omap_gpio_mod_init() and OMAP1 configurations are overwritten. Also looks like there is issue in making distinction between omap15xx and omap16xx. I will post a patch and you can help me testing it in OMAP1 platform. Thanks for pointing this out. -- Tarun --- arch/arm/mach-omap1/gpio16xx.c | 35 +- drivers/gpio/gpio-omap.c | 77 2 files changed, 57 insertions(+), 55 deletions(-) ... diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c index f39d9e4..a948351 100644 --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c ... @@ -898,62 +896,30 @@ static void __init omap_gpio_show_rev(struct gpio_bank *bank) */ static struct lock_class_key gpio_lock_class; -/* TODO: Cleanup cpu_is_* checks */ static void omap_gpio_mod_init(struct gpio_bank *bank) { - if (cpu_class_is_omap2()) { - if (cpu_is_omap44xx()) { - __raw_writel(0x, bank-base + - OMAP4_GPIO_IRQSTATUSCLR0); - __raw_writel(0x, bank-base + - OMAP4_GPIO_DEBOUNCENABLE); - /* Initialize interface clk ungated, module enabled */ - __raw_writel(0, bank-base + OMAP4_GPIO_CTRL); - } else if (cpu_is_omap34xx()) { - __raw_writel(0x, bank-base + - OMAP24XX_GPIO_IRQENABLE1); - __raw_writel(0x, bank-base + - OMAP24XX_GPIO_IRQSTATUS1); - __raw_writel(0x, bank-base + - OMAP24XX_GPIO_DEBOUNCE_EN); - - /* Initialize interface clk ungated, module enabled */ - __raw_writel(0, bank-base + OMAP24XX_GPIO_CTRL); - } - } else if (cpu_class_is_omap1()) { - if (bank_is_mpuio(bank)) { - __raw_writew(0x, bank-base + - OMAP_MPUIO_GPIO_MASKIT / bank-stride); - mpuio_init(bank); - } - if (cpu_is_omap15xx() bank-method == METHOD_GPIO_1510) { - __raw_writew(0x, bank-base - + OMAP1510_GPIO_INT_MASK); - __raw_writew(0x, bank-base - + OMAP1510_GPIO_INT_STATUS); - } - if (cpu_is_omap16xx() bank-method == METHOD_GPIO_1610) { - __raw_writew(0x, bank-base - + OMAP1610_GPIO_IRQENABLE1); - __raw_writew(0x, bank-base - + OMAP1610_GPIO_IRQSTATUS1); - __raw_writew(0x0014, bank-base - + OMAP1610_GPIO_SYSCONFIG); + void __iomem *base = bank-base; + u32 l = 0x; - /* - * Enable system clock for GPIO module. - * The CAM_CLK_CTRL *is* really the right place. - */ - omap_writel(omap_readl(ULPD_CAM_CLK_CTRL) | 0x04, - ULPD_CAM_CLK_CTRL); - } - if (cpu_is_omap7xx() bank-method == METHOD_GPIO_7XX) { - __raw_writel(0x, bank-base - + OMAP7XX_GPIO_INT_MASK); - __raw_writel(0x, bank-base - + OMAP7XX_GPIO_INT_STATUS); - } + if (bank-width == 16) + l = 0x; + + if (bank_is_mpuio(bank)) { + __raw_writel(l, bank-base + bank-regs-irqenable); + return; } + + _gpio_rmw(base, bank-regs-irqenable, l, bank-regs-irqenable_inv); + _gpio_rmw(base, bank-regs-irqstatus,
Re: [PATCH v9 11/25] gpio/omap: cleanup omap_gpio_mod_init function
* DebBarma, Tarun Kanti tarun.ka...@ti.com [120424 08:40]: Hi Janusz, On Tue, Apr 24, 2012 at 12:24 AM, DebBarma, Tarun Kanti tarun.ka...@ti.com wrote: On Sat, Apr 21, 2012 at 7:33 PM, Janusz Krzysztofik jkrzy...@tis.icnet.pl wrote: On Thursday 02 of February 2012 23:00:37 Tarun Kanti DebBarma wrote: With register offsets now defined for respective OMAP versions we can get rid of cpu_class_* checks. This function now has common initialization code for all OMAP versions... Signed-off-by: Tarun Kanti DebBarma tarun.ka...@ti.com Signed-off-by: Charulatha V ch...@ti.com Reviewed-by: Santosh Shilimkar santosh.shilim...@ti.com Acked-by: Tony Lindgren t...@atomide.com Sorry for being so late with my comment for chanes already present in mainline, but this patch breaks GPIO on Amstrad Delta at least, and I have neither enough spare time nor enough experience with non OMAP1 machines to provide a solution myself. Yes, I looked at the omap_gpio_mod_init() and OMAP1 configurations are overwritten. Also looks like there is issue in making distinction between omap15xx and omap16xx. I will post a patch and you can help me testing it in OMAP1 platform. Thanks for pointing this out. ... Here is the patch, with attachment as well. I have just tested on OMAP4 platform. Testing on other OMAP2+ platforms is pending. In the meantime can you please validate on OMAP1 platform and confirm? Thanks. -- Tarun From: Tarun Kanti DebBarma tarun.ka...@ti.com Date: Tue, 24 Apr 2012 20:34:32 +0530 Subject: [PATCH] gpio/omap: fix omap1 register overwrite in omap_gpio_mod_init Initialization of irqenable, irqstatus registers is the common operation done in this function for all OMAP platforms, viz. OMAP1, OMAP2+. The latter _gpio_rmw()'s to irqenable register was overwriting the correctly programmed OMAP1 value at the beginning. As a result, even though it worked on OMAP2+ platforms it was breaking OMAP1 functionality. Sounds like the original patch was never tested on omap1? On closer observation it is found that the first _gpio_rmw() which is supposedly done to take care of OMAP1 platform is generic enough and takes care of OMAP2+ platform as well. Therefore remove the latter _gpio_rmw() to irqenable as they are redundant. Also, changing the sequence and logic of initializing the irqstatus. Please mention also the breaking commit here and get this fix merged as a regression as soon as it's tested for the current -rc series. Regards, Tony Signed-off-by: Tarun Kanti DebBarma tarun.ka...@ti.com --- drivers/gpio/gpio-omap.c |5 + 1 files changed, 1 insertions(+), 4 deletions(-) diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c index 1adc2ec..b8f01c1 100644 --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c @@ -964,11 +964,8 @@ static void omap_gpio_mod_init(struct gpio_bank *bank) return; } + _gpio_rmw(base, bank-regs-irqstatus, l, bank-regs-irqenable_inv == 0 ); _gpio_rmw(base, bank-regs-irqenable, l, bank-regs-irqenable_inv); - _gpio_rmw(base, bank-regs-irqstatus, l, - bank-regs-irqenable_inv == false); - _gpio_rmw(base, bank-regs-irqenable, l, bank-regs-debounce_en != 0); - _gpio_rmw(base, bank-regs-irqenable, l, bank-regs-ctrl != 0); if (bank-regs-debounce_en) _gpio_rmw(base, bank-regs-debounce_en, 0, 1); -- 1.7.0.4 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 11/25] gpio/omap: cleanup omap_gpio_mod_init function
Dnia wtorek, 24 kwietnia 2012 21:06:59 DebBarma, Tarun Kanti pisze: Hi Janusz, Here is the patch, with attachment as well. I have just tested on OMAP4 platform. Testing on other OMAP2+ platforms is pending. In the meantime can you please validate on OMAP1 platform and confirm? Thanks. Hi, Please give me a day or two. Thanks, Janusz -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 11/25] gpio/omap: cleanup omap_gpio_mod_init function
On Tue, Apr 24, 2012 at 9:34 PM, Tony Lindgren t...@atomide.com wrote: * DebBarma, Tarun Kanti tarun.ka...@ti.com [120424 08:40]: Hi Janusz, On Tue, Apr 24, 2012 at 12:24 AM, DebBarma, Tarun Kanti tarun.ka...@ti.com wrote: On Sat, Apr 21, 2012 at 7:33 PM, Janusz Krzysztofik jkrzy...@tis.icnet.pl wrote: On Thursday 02 of February 2012 23:00:37 Tarun Kanti DebBarma wrote: With register offsets now defined for respective OMAP versions we can get rid of cpu_class_* checks. This function now has common initialization code for all OMAP versions... Signed-off-by: Tarun Kanti DebBarma tarun.ka...@ti.com Signed-off-by: Charulatha V ch...@ti.com Reviewed-by: Santosh Shilimkar santosh.shilim...@ti.com Acked-by: Tony Lindgren t...@atomide.com Sorry for being so late with my comment for chanes already present in mainline, but this patch breaks GPIO on Amstrad Delta at least, and I have neither enough spare time nor enough experience with non OMAP1 machines to provide a solution myself. Yes, I looked at the omap_gpio_mod_init() and OMAP1 configurations are overwritten. Also looks like there is issue in making distinction between omap15xx and omap16xx. I will post a patch and you can help me testing it in OMAP1 platform. Thanks for pointing this out. ... Here is the patch, with attachment as well. I have just tested on OMAP4 platform. Testing on other OMAP2+ platforms is pending. In the meantime can you please validate on OMAP1 platform and confirm? Thanks. -- Tarun From: Tarun Kanti DebBarma tarun.ka...@ti.com Date: Tue, 24 Apr 2012 20:34:32 +0530 Subject: [PATCH] gpio/omap: fix omap1 register overwrite in omap_gpio_mod_init Initialization of irqenable, irqstatus registers is the common operation done in this function for all OMAP platforms, viz. OMAP1, OMAP2+. The latter _gpio_rmw()'s to irqenable register was overwriting the correctly programmed OMAP1 value at the beginning. As a result, even though it worked on OMAP2+ platforms it was breaking OMAP1 functionality. Sounds like the original patch was never tested on omap1? That's right, only bootup test was done on OMAP1710-SDP. On closer observation it is found that the first _gpio_rmw() which is supposedly done to take care of OMAP1 platform is generic enough and takes care of OMAP2+ platform as well. Therefore remove the latter _gpio_rmw() to irqenable as they are redundant. Also, changing the sequence and logic of initializing the irqstatus. Please mention also the breaking commit here and get this fix merged as a regression as soon as it's tested for the current -rc series. Sure, I will do that! -- Tarun Regards, Tony Signed-off-by: Tarun Kanti DebBarma tarun.ka...@ti.com --- drivers/gpio/gpio-omap.c | 5 + 1 files changed, 1 insertions(+), 4 deletions(-) diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c index 1adc2ec..b8f01c1 100644 --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c @@ -964,11 +964,8 @@ static void omap_gpio_mod_init(struct gpio_bank *bank) return; } + _gpio_rmw(base, bank-regs-irqstatus, l, bank-regs-irqenable_inv == 0 ); _gpio_rmw(base, bank-regs-irqenable, l, bank-regs-irqenable_inv); - _gpio_rmw(base, bank-regs-irqstatus, l, - bank-regs-irqenable_inv == false); - _gpio_rmw(base, bank-regs-irqenable, l, bank-regs-debounce_en != 0); - _gpio_rmw(base, bank-regs-irqenable, l, bank-regs-ctrl != 0); if (bank-regs-debounce_en) _gpio_rmw(base, bank-regs-debounce_en, 0, 1); -- 1.7.0.4 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 11/25] gpio/omap: cleanup omap_gpio_mod_init function
On Sat, Apr 21, 2012 at 7:33 PM, Janusz Krzysztofik jkrzy...@tis.icnet.pl wrote: On Thursday 02 of February 2012 23:00:37 Tarun Kanti DebBarma wrote: With register offsets now defined for respective OMAP versions we can get rid of cpu_class_* checks. This function now has common initialization code for all OMAP versions... Signed-off-by: Tarun Kanti DebBarma tarun.ka...@ti.com Signed-off-by: Charulatha V ch...@ti.com Reviewed-by: Santosh Shilimkar santosh.shilim...@ti.com Acked-by: Tony Lindgren t...@atomide.com Sorry for being so late with my comment for chanes already present in mainline, but this patch breaks GPIO on Amstrad Delta at least, and I have neither enough spare time nor enough experience with non OMAP1 machines to provide a solution myself. Yes, I looked at the omap_gpio_mod_init() and OMAP1 configurations are overwritten. Also looks like there is issue in making distinction between omap15xx and omap16xx. I will post a patch and you can help me testing it in OMAP1 platform. Thanks for pointing this out. -- Tarun --- arch/arm/mach-omap1/gpio16xx.c | 35 +- drivers/gpio/gpio-omap.c | 77 2 files changed, 57 insertions(+), 55 deletions(-) ... diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c index f39d9e4..a948351 100644 --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c ... @@ -898,62 +896,30 @@ static void __init omap_gpio_show_rev(struct gpio_bank *bank) */ static struct lock_class_key gpio_lock_class; -/* TODO: Cleanup cpu_is_* checks */ static void omap_gpio_mod_init(struct gpio_bank *bank) { - if (cpu_class_is_omap2()) { - if (cpu_is_omap44xx()) { - __raw_writel(0x, bank-base + - OMAP4_GPIO_IRQSTATUSCLR0); - __raw_writel(0x, bank-base + - OMAP4_GPIO_DEBOUNCENABLE); - /* Initialize interface clk ungated, module enabled */ - __raw_writel(0, bank-base + OMAP4_GPIO_CTRL); - } else if (cpu_is_omap34xx()) { - __raw_writel(0x, bank-base + - OMAP24XX_GPIO_IRQENABLE1); - __raw_writel(0x, bank-base + - OMAP24XX_GPIO_IRQSTATUS1); - __raw_writel(0x, bank-base + - OMAP24XX_GPIO_DEBOUNCE_EN); - - /* Initialize interface clk ungated, module enabled */ - __raw_writel(0, bank-base + OMAP24XX_GPIO_CTRL); - } - } else if (cpu_class_is_omap1()) { - if (bank_is_mpuio(bank)) { - __raw_writew(0x, bank-base + - OMAP_MPUIO_GPIO_MASKIT / bank-stride); - mpuio_init(bank); - } - if (cpu_is_omap15xx() bank-method == METHOD_GPIO_1510) { - __raw_writew(0x, bank-base - + OMAP1510_GPIO_INT_MASK); - __raw_writew(0x, bank-base - + OMAP1510_GPIO_INT_STATUS); - } - if (cpu_is_omap16xx() bank-method == METHOD_GPIO_1610) { - __raw_writew(0x, bank-base - + OMAP1610_GPIO_IRQENABLE1); - __raw_writew(0x, bank-base - + OMAP1610_GPIO_IRQSTATUS1); - __raw_writew(0x0014, bank-base - + OMAP1610_GPIO_SYSCONFIG); + void __iomem *base = bank-base; + u32 l = 0x; - /* - * Enable system clock for GPIO module. - * The CAM_CLK_CTRL *is* really the right place. - */ - omap_writel(omap_readl(ULPD_CAM_CLK_CTRL) | 0x04, - ULPD_CAM_CLK_CTRL); - } - if (cpu_is_omap7xx() bank-method == METHOD_GPIO_7XX) { - __raw_writel(0x, bank-base - + OMAP7XX_GPIO_INT_MASK); - __raw_writel(0x, bank-base - + OMAP7XX_GPIO_INT_STATUS); - } + if (bank-width == 16) + l = 0x; + + if (bank_is_mpuio(bank)) { + __raw_writel(l, bank-base + bank-regs-irqenable); + return; } + + _gpio_rmw(base, bank-regs-irqenable, l, bank-regs-irqenable_inv); + _gpio_rmw(base, bank-regs-irqstatus, l, + bank-regs-irqenable_inv == false); + _gpio_rmw(base,
Re: [PATCH v9 11/25] gpio/omap: cleanup omap_gpio_mod_init function
On Thursday 02 of February 2012 23:00:37 Tarun Kanti DebBarma wrote: With register offsets now defined for respective OMAP versions we can get rid of cpu_class_* checks. This function now has common initialization code for all OMAP versions... Signed-off-by: Tarun Kanti DebBarma tarun.ka...@ti.com Signed-off-by: Charulatha V ch...@ti.com Reviewed-by: Santosh Shilimkar santosh.shilim...@ti.com Acked-by: Tony Lindgren t...@atomide.com Sorry for being so late with my comment for chanes already present in mainline, but this patch breaks GPIO on Amstrad Delta at least, and I have neither enough spare time nor enough experience with non OMAP1 machines to provide a solution myself. --- arch/arm/mach-omap1/gpio16xx.c | 35 +- drivers/gpio/gpio-omap.c | 77 2 files changed, 57 insertions(+), 55 deletions(-) ... diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c index f39d9e4..a948351 100644 --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c ... @@ -898,62 +896,30 @@ static void __init omap_gpio_show_rev(struct gpio_bank *bank) */ static struct lock_class_key gpio_lock_class; -/* TODO: Cleanup cpu_is_* checks */ static void omap_gpio_mod_init(struct gpio_bank *bank) { - if (cpu_class_is_omap2()) { - if (cpu_is_omap44xx()) { - __raw_writel(0x, bank-base + - OMAP4_GPIO_IRQSTATUSCLR0); - __raw_writel(0x, bank-base + - OMAP4_GPIO_DEBOUNCENABLE); - /* Initialize interface clk ungated, module enabled */ - __raw_writel(0, bank-base + OMAP4_GPIO_CTRL); - } else if (cpu_is_omap34xx()) { - __raw_writel(0x, bank-base + - OMAP24XX_GPIO_IRQENABLE1); - __raw_writel(0x, bank-base + - OMAP24XX_GPIO_IRQSTATUS1); - __raw_writel(0x, bank-base + - OMAP24XX_GPIO_DEBOUNCE_EN); - - /* Initialize interface clk ungated, module enabled */ - __raw_writel(0, bank-base + OMAP24XX_GPIO_CTRL); - } - } else if (cpu_class_is_omap1()) { - if (bank_is_mpuio(bank)) { - __raw_writew(0x, bank-base + - OMAP_MPUIO_GPIO_MASKIT / bank-stride); - mpuio_init(bank); - } - if (cpu_is_omap15xx() bank-method == METHOD_GPIO_1510) { - __raw_writew(0x, bank-base - + OMAP1510_GPIO_INT_MASK); - __raw_writew(0x, bank-base - + OMAP1510_GPIO_INT_STATUS); - } - if (cpu_is_omap16xx() bank-method == METHOD_GPIO_1610) { - __raw_writew(0x, bank-base - + OMAP1610_GPIO_IRQENABLE1); - __raw_writew(0x, bank-base - + OMAP1610_GPIO_IRQSTATUS1); - __raw_writew(0x0014, bank-base - + OMAP1610_GPIO_SYSCONFIG); + void __iomem *base = bank-base; + u32 l = 0x; - /* - * Enable system clock for GPIO module. - * The CAM_CLK_CTRL *is* really the right place. - */ - omap_writel(omap_readl(ULPD_CAM_CLK_CTRL) | 0x04, - ULPD_CAM_CLK_CTRL); - } - if (cpu_is_omap7xx() bank-method == METHOD_GPIO_7XX) { - __raw_writel(0x, bank-base - + OMAP7XX_GPIO_INT_MASK); - __raw_writel(0x, bank-base - + OMAP7XX_GPIO_INT_STATUS); - } + if (bank-width == 16) + l = 0x; + + if (bank_is_mpuio(bank)) { + __raw_writel(l, bank-base + bank-regs-irqenable); + return; } + + _gpio_rmw(base, bank-regs-irqenable, l, bank-regs-irqenable_inv); + _gpio_rmw(base, bank-regs-irqstatus, l, + bank-regs-irqenable_inv == false); + _gpio_rmw(base, bank-regs-irqenable, l, bank-regs-debounce_en != 0); + _gpio_rmw(base, bank-regs-irqenable, l, bank-regs-ctrl != 0); bank-regs-irqenable trgister is manipulated 3 times in a row, each time based on different criteria. This breaks GPIO on Amstrad Delta at least, and generally looks wrong. I was only able to verify that commenting out the above two lines
[PATCH v9 11/25] gpio/omap: cleanup omap_gpio_mod_init function
With register offsets now defined for respective OMAP versions we can get rid of cpu_class_* checks. This function now has common initialization code for all OMAP versions. Initialization specific to OMAP16xx has been moved within omap16xx_gpio_init(). Signed-off-by: Tarun Kanti DebBarma tarun.ka...@ti.com Signed-off-by: Charulatha V ch...@ti.com Reviewed-by: Santosh Shilimkar santosh.shilim...@ti.com Acked-by: Tony Lindgren t...@atomide.com --- arch/arm/mach-omap1/gpio16xx.c | 35 +- drivers/gpio/gpio-omap.c | 77 2 files changed, 57 insertions(+), 55 deletions(-) diff --git a/arch/arm/mach-omap1/gpio16xx.c b/arch/arm/mach-omap1/gpio16xx.c index 46bb57a..86ac415 100644 --- a/arch/arm/mach-omap1/gpio16xx.c +++ b/arch/arm/mach-omap1/gpio16xx.c @@ -24,6 +24,9 @@ #define OMAP1610_GPIO4_BASE0xfffbbc00 #define OMAP1_MPUIO_VBASE OMAP1_MPUIO_BASE +/* smart idle, enable wakeup */ +#define SYSCONFIG_WORD 0x14 + /* mpu gpio */ static struct __initdata resource omap16xx_mpu_gpio_resources[] = { { @@ -218,12 +221,42 @@ static struct __initdata platform_device * omap16xx_gpio_dev[] = { static int __init omap16xx_gpio_init(void) { int i; + void __iomem *base; + struct resource *res; + struct platform_device *pdev; + struct omap_gpio_platform_data *pdata; if (!cpu_is_omap16xx()) return -EINVAL; - for (i = 0; i ARRAY_SIZE(omap16xx_gpio_dev); i++) + for (i = 0; i ARRAY_SIZE(omap16xx_gpio_dev); i++) { + pdev = omap16xx_gpio_dev[i]; + pdata = pdev-dev.platform_data; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (unlikely(!res)) { + dev_err(pdev-dev, Invalid mem resource.\n); + return -ENODEV; + } + + base = ioremap(res-start, resource_size(res)); + if (unlikely(!base)) { + dev_err(pdev-dev, ioremap failed.\n); + return -ENOMEM; + } + + __raw_writel(SYSCONFIG_WORD, base + OMAP1610_GPIO_SYSCONFIG); + iounmap(base); + + /* +* Enable system clock for GPIO module. +* The CAM_CLK_CTRL *is* really the right place. +*/ + omap_writel(omap_readl(ULPD_CAM_CLK_CTRL) | 0x04, + ULPD_CAM_CLK_CTRL); + platform_device_register(omap16xx_gpio_dev[i]); + } return 0; } diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c index f39d9e4..a948351 100644 --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c @@ -610,7 +610,6 @@ static void gpio_irq_handler(unsigned int irq, struct irq_desc *desc) if (!(isr 1)) continue; -#ifdef CONFIG_ARCH_OMAP1 /* * Some chips can't respond to both rising and falling * at the same time. If this irq was requested with @@ -620,7 +619,6 @@ static void gpio_irq_handler(unsigned int irq, struct irq_desc *desc) */ if (bank-toggle_mask (1 gpio_index)) _toggle_gpio_edge_triggering(bank, gpio_index); -#endif generic_handle_irq(gpio_irq); } @@ -898,62 +896,30 @@ static void __init omap_gpio_show_rev(struct gpio_bank *bank) */ static struct lock_class_key gpio_lock_class; -/* TODO: Cleanup cpu_is_* checks */ static void omap_gpio_mod_init(struct gpio_bank *bank) { - if (cpu_class_is_omap2()) { - if (cpu_is_omap44xx()) { - __raw_writel(0x, bank-base + - OMAP4_GPIO_IRQSTATUSCLR0); - __raw_writel(0x, bank-base + -OMAP4_GPIO_DEBOUNCENABLE); - /* Initialize interface clk ungated, module enabled */ - __raw_writel(0, bank-base + OMAP4_GPIO_CTRL); - } else if (cpu_is_omap34xx()) { - __raw_writel(0x, bank-base + - OMAP24XX_GPIO_IRQENABLE1); - __raw_writel(0x, bank-base + - OMAP24XX_GPIO_IRQSTATUS1); - __raw_writel(0x, bank-base + - OMAP24XX_GPIO_DEBOUNCE_EN); - - /* Initialize interface clk ungated, module enabled */ - __raw_writel(0, bank-base + OMAP24XX_GPIO_CTRL); - } - } else if (cpu_class_is_omap1()) { - if (bank_is_mpuio(bank)) { - __raw_writew(0x,