From: "Varadarajan, Charulatha" <charu@ti.com>
To: Kevin Hilman <khilman@ti.com>
Cc: Tarun Kanti DebBarma <tarun.kanti@ti.com>,
linux-omap@vger.kernel.org, santosh.shilimkar@ti.com,
tony@atomide.com, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 01/15] OMAP: GPIO: Avoid cpu_is checks during module ena/disable
Date: Thu, 26 May 2011 15:08:28 +0530 [thread overview]
Message-ID: <BANLkTi=4_Zu28DH8h0wSn=XrJFCd7Bg80A@mail.gmail.com> (raw)
In-Reply-To: <8762oytr43.fsf@ti.com>
On Thu, May 26, 2011 at 02:49, Kevin Hilman <khilman@ti.com> wrote:
> Tarun Kanti DebBarma <tarun.kanti@ti.com> writes:
>
>> From: Charulatha V <charu@ti.com>
>>
>> Remove cpu-is checks while enabling/disabling OMAP GPIO module
>> during a gpio request/free.
>>
>> Signed-off-by: Charulatha V <charu@ti.com>
>
> 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);
>> }
>
--
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
next prev parent reply other threads:[~2011-05-26 9:39 UTC|newest]
Thread overview: 59+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-05-24 14:24 [PATCH 00/15] OMAP: GPIO: Cleanup OMAP GPIO driver Tarun Kanti DebBarma
2011-05-24 14:24 ` [PATCH 01/15] OMAP: GPIO: Avoid cpu_is checks during module ena/disable Tarun Kanti DebBarma
2011-05-25 21:19 ` Kevin Hilman
2011-05-26 9:38 ` Varadarajan, Charulatha [this message]
2011-05-24 14:24 ` [PATCH 02/15] OMAP2PLUS: GPIO: Fix non-wakeup GPIO and rev_ids Tarun Kanti DebBarma
2011-05-25 21:34 ` Kevin Hilman
2011-05-26 9:38 ` Varadarajan, Charulatha
2011-05-26 17:15 ` Kevin Hilman
2011-05-26 17:39 ` Varadarajan, Charulatha
2011-05-26 18:32 ` Kevin Hilman
2011-05-26 9:23 ` Premi, Sanjeev
2011-05-26 9:43 ` Varadarajan, Charulatha
2011-05-26 10:11 ` Cousson, Benoit
2011-05-26 11:47 ` Premi, Sanjeev
2011-05-26 12:11 ` Cousson, Benoit
2011-05-26 12:38 ` Premi, Sanjeev
2011-05-26 12:46 ` Cousson, Benoit
2011-05-26 13:19 ` Premi, Sanjeev
2011-05-26 13:38 ` B.J. Buchalter
2011-05-26 14:12 ` Cousson, Benoit
2011-05-24 14:24 ` [PATCH 03/15] OMAP: GPIO: Remove dependency on gpio_bank_count Tarun Kanti DebBarma
2011-05-24 14:24 ` [PATCH 04/15] OMAP2PLUS: GPIO: Use flag to identify wkup dmn GPIO Tarun Kanti DebBarma
2011-05-25 21:40 ` Kevin Hilman
2011-05-24 14:24 ` [PATCH 05/15] OMAP: GPIO: Make gpio_context part of gpio_bank structure Tarun Kanti DebBarma
2011-05-25 21:41 ` Kevin Hilman
2011-05-26 9:58 ` Premi, Sanjeev
2011-05-26 10:07 ` Varadarajan, Charulatha
2011-05-26 9:59 ` Premi, Sanjeev
2011-05-24 14:24 ` [PATCH 06/15] OMAP4: GPIO: Save/restore context Tarun Kanti DebBarma
2011-05-25 21:43 ` Kevin Hilman
2011-05-26 9:37 ` Varadarajan, Charulatha
2011-05-24 14:24 ` [PATCH 07/15] OMAP: GPIO: handle save/restore ctx in GPIO driver Tarun Kanti DebBarma
2011-05-25 22:33 ` Kevin Hilman
2011-05-25 22:36 ` Kevin Hilman
2011-05-24 14:24 ` [PATCH 08/15] OMAP2+: GPIO: make workaround_enabled bank specific Tarun Kanti DebBarma
2011-05-25 22:39 ` Kevin Hilman
2011-05-26 9:37 ` Varadarajan, Charulatha
2011-05-24 14:24 ` [PATCH 09/15] OMAP: GPIO: cleanup suspend and resume functions Tarun Kanti DebBarma
2011-05-25 22:57 ` Kevin Hilman
2011-05-26 10:02 ` Varadarajan, Charulatha
2011-05-24 14:24 ` [PATCH 10/15] OMAP: GPIO: cleanup prepare/resume idle functions Tarun Kanti DebBarma
2011-05-25 23:00 ` Kevin Hilman
2011-05-24 14:24 ` [PATCH 11/15] OMAP: GPIO: Remove hardcoded offsets in ctxt save/restore Tarun Kanti DebBarma
2011-05-25 23:01 ` Kevin Hilman
2011-05-26 9:36 ` Varadarajan, Charulatha
2011-05-26 9:42 ` Premi, Sanjeev
2011-05-26 9:48 ` Varadarajan, Charulatha
2011-05-24 14:24 ` [PATCH 12/15] OMAP: GPIO: Fix: use wake set/clear regs Tarun Kanti DebBarma
2011-05-25 23:14 ` Kevin Hilman
2011-05-26 9:36 ` Varadarajan, Charulatha
2011-05-24 14:24 ` [PATCH 13/15] OMAP: GPIO: clean set_gpio_triggering function Tarun Kanti DebBarma
2011-05-25 23:27 ` Kevin Hilman
2011-05-26 9:55 ` Varadarajan, Charulatha
2011-05-24 14:24 ` [PATCH 14/15] OMAP: GPIO: Use memset for omap_gpio_reg_offs Tarun Kanti DebBarma
2011-05-25 23:30 ` Kevin Hilman
2011-05-24 14:24 ` [PATCH 15/15] OMAP: GPIO: clean omap_gpio_mod_init function Tarun Kanti DebBarma
2011-05-25 23:48 ` Kevin Hilman
2011-06-03 11:20 ` Varadarajan, Charulatha
2011-06-03 14:31 ` Kevin Hilman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='BANLkTi=4_Zu28DH8h0wSn=XrJFCd7Bg80A@mail.gmail.com' \
--to=charu@ti.com \
--cc=khilman@ti.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-omap@vger.kernel.org \
--cc=santosh.shilimkar@ti.com \
--cc=tarun.kanti@ti.com \
--cc=tony@atomide.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).