From mboxrd@z Thu Jan 1 00:00:00 1970 From: charu@ti.com (Varadarajan, Charulatha) Date: Thu, 26 May 2011 15:08:28 +0530 Subject: [PATCH 01/15] OMAP: GPIO: Avoid cpu_is checks during module ena/disable In-Reply-To: <8762oytr43.fsf@ti.com> References: <1306247094-25372-1-git-send-email-tarun.kanti@ti.com> <1306247094-25372-2-git-send-email-tarun.kanti@ti.com> <8762oytr43.fsf@ti.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Thu, May 26, 2011 at 02:49, Kevin Hilman wrote: > Tarun Kanti DebBarma writes: > >> From: Charulatha V >> >> Remove cpu-is checks while enabling/disabling OMAP GPIO module >> during a gpio request/free. >> >> Signed-off-by: Charulatha V > > This looks mostly OK, but one nitpick about the usage of USHRT_MAX. > > For most registers, you should just test for a non-zero register offset > to determine if it's present or not instead of USHRT_MAX. Is it safe to assume that zero offset is always for revision register only? IMHO I feel that it is safe to assume an offset of 0xFFFF as invalid. Pls add your thoughts on this. > > I used USHRT_MAX for the revision register because it is at offset zero > on most SoCs, so testing for non-zero wouldn't work there. > > Other than that, the approach looks fine. > > Thanks, > > Kevin > >> --- >> ?arch/arm/mach-omap1/gpio15xx.c ? ? ? ? | ? ?2 + >> ?arch/arm/mach-omap1/gpio16xx.c ? ? ? ? | ? ?2 + >> ?arch/arm/mach-omap1/gpio7xx.c ? ? ? ? ?| ? ?2 + >> ?arch/arm/mach-omap2/gpio.c ? ? ? ? ? ? | ? ?2 + >> ?arch/arm/plat-omap/include/plat/gpio.h | ? ?1 + >> ?drivers/gpio/gpio_omap.c ? ? ? ? ? ? ? | ? 53 ++++++++++++++------------------ >> ?6 files changed, 32 insertions(+), 30 deletions(-) >> >> diff --git a/arch/arm/mach-omap1/gpio15xx.c b/arch/arm/mach-omap1/gpio15xx.c >> index f79c6ae..6d83e0a 100644 >> --- a/arch/arm/mach-omap1/gpio15xx.c >> +++ b/arch/arm/mach-omap1/gpio15xx.c >> @@ -42,6 +42,7 @@ static struct omap_gpio_reg_offs omap15xx_mpuio_regs = { >> ? ? ? .irqstatus ? ? ?= OMAP_MPUIO_GPIO_INT, >> ? ? ? .irqenable ? ? ?= OMAP_MPUIO_GPIO_MASKIT, >> ? ? ? .irqenable_inv ?= true, >> + ? ? .ctrl ? ? ? ? ? = USHRT_MAX, >> ?}; >> >> ?static struct __initdata omap_gpio_platform_data omap15xx_mpu_gpio_config = { >> @@ -83,6 +84,7 @@ static struct omap_gpio_reg_offs omap15xx_gpio_regs = { >> ? ? ? .irqstatus ? ? ?= OMAP1510_GPIO_INT_STATUS, >> ? ? ? .irqenable ? ? ?= OMAP1510_GPIO_INT_MASK, >> ? ? ? .irqenable_inv ?= true, >> + ? ? .ctrl ? ? ? ? ? = USHRT_MAX, >> ?}; >> >> ?static struct __initdata omap_gpio_platform_data omap15xx_gpio_config = { >> diff --git a/arch/arm/mach-omap1/gpio16xx.c b/arch/arm/mach-omap1/gpio16xx.c >> index c69b3b1..6bba196 100644 >> --- a/arch/arm/mach-omap1/gpio16xx.c >> +++ b/arch/arm/mach-omap1/gpio16xx.c >> @@ -45,6 +45,7 @@ static struct omap_gpio_reg_offs omap16xx_mpuio_regs = { >> ? ? ? .irqstatus ? ? ?= OMAP_MPUIO_GPIO_INT, >> ? ? ? .irqenable ? ? ?= OMAP_MPUIO_GPIO_MASKIT, >> ? ? ? .irqenable_inv ?= true, >> + ? ? .ctrl ? ? ? ? ? = USHRT_MAX, >> ?}; >> >> ?static struct __initdata omap_gpio_platform_data omap16xx_mpu_gpio_config = { >> @@ -89,6 +90,7 @@ static struct omap_gpio_reg_offs omap16xx_gpio_regs = { >> ? ? ? .irqenable ? ? ?= OMAP1610_GPIO_IRQENABLE1, >> ? ? ? .set_irqenable ?= OMAP1610_GPIO_SET_IRQENABLE1, >> ? ? ? .clr_irqenable ?= OMAP1610_GPIO_CLEAR_IRQENABLE1, >> + ? ? .ctrl ? ? ? ? ? = USHRT_MAX, >> ?}; >> >> ?static struct __initdata omap_gpio_platform_data omap16xx_gpio1_config = { >> diff --git a/arch/arm/mach-omap1/gpio7xx.c b/arch/arm/mach-omap1/gpio7xx.c >> index d7f2ad3..0fc2557 100644 >> --- a/arch/arm/mach-omap1/gpio7xx.c >> +++ b/arch/arm/mach-omap1/gpio7xx.c >> @@ -47,6 +47,7 @@ static struct omap_gpio_reg_offs omap7xx_mpuio_regs = { >> ? ? ? .irqstatus ? ? ?= OMAP_MPUIO_GPIO_INT / 2, >> ? ? ? .irqenable ? ? ?= OMAP_MPUIO_GPIO_MASKIT / 2, >> ? ? ? .irqenable_inv ?= true, >> + ? ? .ctrl ? ? ? ? ? = USHRT_MAX, >> ?}; >> >> ?static struct __initdata omap_gpio_platform_data omap7xx_mpu_gpio_config = { >> @@ -88,6 +89,7 @@ static struct omap_gpio_reg_offs omap7xx_gpio_regs = { >> ? ? ? .irqstatus ? ? ?= OMAP7XX_GPIO_INT_STATUS, >> ? ? ? .irqenable ? ? ?= OMAP7XX_GPIO_INT_MASK, >> ? ? ? .irqenable_inv ?= true, >> + ? ? .ctrl ? ? ? ? ? = USHRT_MAX, >> ?}; >> >> ?static struct __initdata omap_gpio_platform_data omap7xx_gpio1_config = { >> diff --git a/arch/arm/mach-omap2/gpio.c b/arch/arm/mach-omap2/gpio.c >> index 9a46d77..0446bd1 100644 >> --- a/arch/arm/mach-omap2/gpio.c >> +++ b/arch/arm/mach-omap2/gpio.c >> @@ -84,6 +84,7 @@ static int omap2_gpio_dev_init(struct omap_hwmod *oh, void *unused) >> ? ? ? ? ? ? ? pdata->regs->clr_irqenable = OMAP24XX_GPIO_CLEARIRQENABLE1; >> ? ? ? ? ? ? ? pdata->regs->debounce = OMAP24XX_GPIO_DEBOUNCE_VAL; >> ? ? ? ? ? ? ? pdata->regs->debounce_en = OMAP24XX_GPIO_DEBOUNCE_EN; >> + ? ? ? ? ? ? pdata->regs->ctrl = OMAP24XX_GPIO_CTRL; >> ? ? ? ? ? ? ? break; >> ? ? ? case 2: >> ? ? ? ? ? ? ? pdata->bank_type = METHOD_GPIO_44XX; >> @@ -100,6 +101,7 @@ static int omap2_gpio_dev_init(struct omap_hwmod *oh, void *unused) >> ? ? ? ? ? ? ? pdata->regs->clr_irqenable = OMAP4_GPIO_IRQSTATUSCLR0; >> ? ? ? ? ? ? ? pdata->regs->debounce = OMAP4_GPIO_DEBOUNCINGTIME; >> ? ? ? ? ? ? ? pdata->regs->debounce_en = OMAP4_GPIO_DEBOUNCENABLE; >> + ? ? ? ? ? ? pdata->regs->ctrl = OMAP4_GPIO_CTRL; >> ? ? ? ? ? ? ? break; >> ? ? ? default: >> ? ? ? ? ? ? ? WARN(1, "Invalid gpio bank_type\n"); >> diff --git a/arch/arm/plat-omap/include/plat/gpio.h b/arch/arm/plat-omap/include/plat/gpio.h >> index 91e8de3..caf432c 100644 >> --- a/arch/arm/plat-omap/include/plat/gpio.h >> +++ b/arch/arm/plat-omap/include/plat/gpio.h >> @@ -188,6 +188,7 @@ struct omap_gpio_reg_offs { >> ? ? ? u16 clr_irqenable; >> ? ? ? u16 debounce; >> ? ? ? u16 debounce_en; >> + ? ? u16 ctrl; >> >> ? ? ? bool irqenable_inv; >> ?}; >> diff --git a/drivers/gpio/gpio_omap.c b/drivers/gpio/gpio_omap.c >> index a490cb1..dfdc45e 100644 >> --- a/drivers/gpio/gpio_omap.c >> +++ b/drivers/gpio/gpio_omap.c >> @@ -89,6 +89,7 @@ int gpio_bank_count; >> >> ?#define GPIO_INDEX(bank, gpio) (gpio % bank->width) >> ?#define GPIO_BIT(bank, gpio) (1 << GPIO_INDEX(bank, gpio)) >> +#define GPIO_MOD_CTRL_BIT ? ?BIT(0) >> >> ?static void _set_gpio_direction(struct gpio_bank *bank, int gpio, int is_input) >> ?{ >> @@ -586,22 +587,18 @@ static int omap_gpio_request(struct gpio_chip *chip, unsigned offset) >> ? ? ? ? ? ? ? __raw_writel(__raw_readl(reg) | (1 << offset), reg); >> ? ? ? } >> ?#endif >> - ? ? if (!cpu_class_is_omap1()) { >> - ? ? ? ? ? ? if (!bank->mod_usage) { >> - ? ? ? ? ? ? ? ? ? ? void __iomem *reg = bank->base; >> - ? ? ? ? ? ? ? ? ? ? u32 ctrl; >> - >> - ? ? ? ? ? ? ? ? ? ? if (cpu_is_omap24xx() || cpu_is_omap34xx()) >> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? reg += OMAP24XX_GPIO_CTRL; >> - ? ? ? ? ? ? ? ? ? ? else if (cpu_is_omap44xx()) >> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? reg += OMAP4_GPIO_CTRL; >> - ? ? ? ? ? ? ? ? ? ? ctrl = __raw_readl(reg); >> - ? ? ? ? ? ? ? ? ? ? /* Module is enabled, clocks are not gated */ >> - ? ? ? ? ? ? ? ? ? ? ctrl &= 0xFFFFFFFE; >> - ? ? ? ? ? ? ? ? ? ? __raw_writel(ctrl, reg); >> - ? ? ? ? ? ? } >> - ? ? ? ? ? ? bank->mod_usage |= 1 << offset; >> + ? ? if ((bank->regs->ctrl != USHRT_MAX) && (!bank->mod_usage)) { >> + ? ? ? ? ? ? void __iomem *reg = bank->base + bank->regs->ctrl; >> + ? ? ? ? ? ? u32 ctrl; >> + >> + ? ? ? ? ? ? ctrl = __raw_readl(reg); >> + ? ? ? ? ? ? /* Module is enabled, clocks are not gated */ >> + ? ? ? ? ? ? ctrl &= ~GPIO_MOD_CTRL_BIT; >> + ? ? ? ? ? ? __raw_writel(ctrl, reg); >> ? ? ? } >> + >> + ? ? bank->mod_usage |= 1 << offset; >> + >> ? ? ? spin_unlock_irqrestore(&bank->lock, flags); >> >> ? ? ? return 0; >> @@ -634,22 +631,18 @@ static void omap_gpio_free(struct gpio_chip *chip, unsigned offset) >> ? ? ? ? ? ? ? __raw_writel(1 << offset, reg); >> ? ? ? } >> ?#endif >> - ? ? if (!cpu_class_is_omap1()) { >> - ? ? ? ? ? ? bank->mod_usage &= ~(1 << offset); >> - ? ? ? ? ? ? if (!bank->mod_usage) { >> - ? ? ? ? ? ? ? ? ? ? void __iomem *reg = bank->base; >> - ? ? ? ? ? ? ? ? ? ? u32 ctrl; >> - >> - ? ? ? ? ? ? ? ? ? ? if (cpu_is_omap24xx() || cpu_is_omap34xx()) >> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? reg += OMAP24XX_GPIO_CTRL; >> - ? ? ? ? ? ? ? ? ? ? else if (cpu_is_omap44xx()) >> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? reg += OMAP4_GPIO_CTRL; >> - ? ? ? ? ? ? ? ? ? ? ctrl = __raw_readl(reg); >> - ? ? ? ? ? ? ? ? ? ? /* Module is disabled, clocks are gated */ >> - ? ? ? ? ? ? ? ? ? ? ctrl |= 1; >> - ? ? ? ? ? ? ? ? ? ? __raw_writel(ctrl, reg); >> - ? ? ? ? ? ? } >> + ? ? bank->mod_usage &= ~(1 << offset); >> + >> + ? ? if ((bank->regs->ctrl != USHRT_MAX) && (!bank->mod_usage)) { >> + ? ? ? ? ? ? void __iomem *reg = bank->base + bank->regs->ctrl; >> + ? ? ? ? ? ? u32 ctrl; >> + >> + ? ? ? ? ? ? ctrl = __raw_readl(reg); >> + ? ? ? ? ? ? /* Module is disabled, clocks are gated */ >> + ? ? ? ? ? ? ctrl |= GPIO_MOD_CTRL_BIT; >> + ? ? ? ? ? ? __raw_writel(ctrl, reg); >> ? ? ? } >> + >> ? ? ? _reset_gpio(bank, bank->chip.base + offset); >> ? ? ? spin_unlock_irqrestore(&bank->lock, flags); >> ?} >