| Message ID | 1310804152-9243-14-git-send-email-tarun.kanti@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, Jul 16, 2011 at 01:45:45PM +0530, 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. Initialization specific to OMAP16xx has been moved within > omap16xx_gpio_init(). > ... > 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; > + } The value of base isn't saved anywhere, and the memory is not unmapped, looks like a virtual memory leak. If the purpose of the ioremap is to perform the single write below then iounmap when done? The previous code to perform that write used a struct gpio_bank *bank->base ioremapped by omap_gpio_probe, but apparently omap16xx_gpio_init isn't called in that path. > + > + __raw_writel(0x0014, base + OMAP1610_GPIO_SYSCONFIG); Suggest a symbol for the 0x14 value, or add a comment describing what this does. (I realize the existing code has many naked constants.) Todd -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[...] > > 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; > > + } > > The value of base isn't saved anywhere, and the memory is not > unmapped, looks like a virtual memory leak. If the purpose of the > ioremap is to perform the single write below then iounmap when done? This is one time write only. I will iounmap(base) after use. Thanks. > The previous code to perform that write used a > struct gpio_bank *bank->base ioremapped by omap_gpio_probe, but > apparently omap16xx_gpio_init isn't called in that path. Right. > > > + > > + __raw_writel(0x0014, base + OMAP1610_GPIO_SYSCONFIG); > > Suggest a symbol for the 0x14 value, or add a comment describing what > this does. (I realize the existing code has many naked constants.) Sure. -- Tarun > > > > Todd -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/mach-omap1/gpio16xx.c b/arch/arm/mach-omap1/gpio16xx.c index 08dda3c..bd9c49e 100644 --- a/arch/arm/mach-omap1/gpio16xx.c +++ b/arch/arm/mach-omap1/gpio16xx.c @@ -218,12 +218,41 @@ 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(0x0014, base + OMAP1610_GPIO_SYSCONFIG); + + /* + * 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 5b32bbf..0da78f3 100644 --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c @@ -875,62 +875,26 @@ 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(0xffffffff, bank->base + - OMAP4_GPIO_IRQSTATUSCLR0); - __raw_writel(0x00000000, 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(0x00000000, bank->base + - OMAP24XX_GPIO_IRQENABLE1); - __raw_writel(0xffffffff, bank->base + - OMAP24XX_GPIO_IRQSTATUS1); - __raw_writel(0x00000000, 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(0xffff, bank->base + - OMAP_MPUIO_GPIO_MASKIT / bank->stride); - mpuio_init(bank); - } - if (cpu_is_omap15xx() && bank->method == METHOD_GPIO_1510) { - __raw_writew(0xffff, bank->base - + OMAP1510_GPIO_INT_MASK); - __raw_writew(0x0000, bank->base - + OMAP1510_GPIO_INT_STATUS); - } - if (cpu_is_omap16xx() && bank->method == METHOD_GPIO_1610) { - __raw_writew(0x0000, bank->base - + OMAP1610_GPIO_IRQENABLE1); - __raw_writew(0xffff, bank->base - + OMAP1610_GPIO_IRQSTATUS1); - __raw_writew(0x0014, bank->base - + OMAP1610_GPIO_SYSCONFIG); + void __iomem *base = bank->base; + u32 l = 0xffffffff; - /* - * 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(0xffffffff, bank->base - + OMAP7XX_GPIO_INT_MASK); - __raw_writel(0x00000000, bank->base - + OMAP7XX_GPIO_INT_STATUS); - } + if (bank_is_mpuio(bank)) { + __raw_writel(0xffff, bank->base + bank->regs->irqenable); + if (bank->regs->wkup_status) + mpuio_init(bank); + return; } + + if (bank->width == 16) + l = 0xffff; + + _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); } static __init void