>> Subject: [PATCH V2] [OMAP] GPIO Module disable when all pins
>> are inactive
>>
>> From: Charulatha V <charu@xxxxxx>
>>
>> This patch disables a GPIO module when all pins of a GPIO
>> module are inactive (clock gating forced at module level) and
>> enables the module when any gpio in the module is requested.
>>
>> The module is enabled only when mod_usage indicates that no GPIO
>> in that module is currently active and the GPIO being requested
>> is the 1st one to be active in that module.
>
>[sp] This para reads quite confusing. The subject talks of disable
> but this para indicates process for 'enable'.
Shall change the subject to "GPIO module enable/disable".
>
>>
>> Each module would be disabled in omap_gpio_free() API when all
>> GPIOs in a particular module becomes inactive. The module is
>> re-enabled in omap_gpio_request() API when a GPIO is requested
>> from the module that was previously disabled.
>
>>
>> Since individual GPIO's bookkeeping is introduced automatically
>> in this patch(mod_usage), the same is used in
>omap_set_gpio_debounce()
>
>[sp] Is book-keeping 'automatically introduced' or added in this
> patch?
The intention of the patch is not to introduce bookkeeping. But the
implementation has brought in bookkeeping concept modulewise.
I shall change it to "added" instead of "automatically introduced".
>
>> & omap_set_gpio_debounce_time() APIs to ensure that the gpio being
>> used is actually "requested" prior to being used (Nishant Menon's
>> <nm@xxxxxx> Suggestion)
>>
>> Higher layer keeps track of GPIOs individually. This patch
>> introduces bookkeeping information, modulewise in lower layer
>> since disabling clock is done at module level. GPIO module level
>> details are specific to hardware and introducing APIs in higher
>> level layer to handle them might not be correct. Hence GPIO module
>> level information (mod_usage) has to be handled only in
>> low-level layer.
>
>[sp] Again the description seems to be quite confusing between the
> higher layer and lower layer contexts.
I shall remove the above para and keep it as:
GPIO module level details are specific to hardware and hence introducing
this patch in low level layer (plat-omap/gpio.c).
>
>>
>> Signed-off-by: Charulatha V <charu@xxxxxx>
>> Acked-by: Nishanth Menon <nm@xxxxxx>
>> ---
>> arch/arm/plat-omap/gpio.c | 35 +++++++++++++++++++++++++++++++++++
>> 1 files changed, 35 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/plat-omap/gpio.c b/arch/arm/plat-omap/gpio.c
>> index 4c35f9f..5ee6a60 100644
>> --- a/arch/arm/plat-omap/gpio.c
>> +++ b/arch/arm/plat-omap/gpio.c
>> @@ -199,6 +199,7 @@ struct gpio_bank {
>> struct gpio_chip chip;
>> struct clk *dbck;
>> u32 dbck_enable_mask;
>> + u32 mod_usage;
>> };
>>
>> #define METHOD_MPUIO 0
>> @@ -691,6 +692,12 @@ void omap_set_gpio_debounce(int gpio,
>int enable)
>> reg += OMAP24XX_GPIO_DEBOUNCE_EN;
>> #endif
>>
>> + if ((cpu_is_omap24xx() || cpu_is_omap34xx() ||
>> cpu_is_omap44xx())
>> + && (!(bank->mod_usage & l))) {
>[sp] Is the AND operation really needed?
Yes, inorder to check if this GPIO was "requested" before calling
omap_set_gpio_debounce
>
>> + printk(KERN_ERR "GPIO not requested\n");
>> + return;
>> + }
>> +
<Snip>
>> reg += OMAP4_GPIO_DEBOUNCINGTIME;
>> @@ -1219,6 +1232,16 @@ static int omap_gpio_request(struct
>> gpio_chip *chip, unsigned offset)
>> __raw_writel(__raw_readl(reg) | (1 << offset), reg);
>> }
>> #endif
>> + if (cpu_is_omap24xx() || cpu_is_omap34xx() ||
>> cpu_is_omap44xx()) {
>> + u32 ctrl;
>
>[sp] This should move into next "if" where it is used.
Ok.
>
>> + if (!bank->mod_usage) {
>> + ctrl = __raw_readl(bank->base +
<Snip>--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
