Re: [PATCH v5 03/22] gpio/omap: make gpio_context part of gpio_bank structure

Santosh
2011-08-26T14:11:27Z

On Friday 26 August 2011 01:53 AM, Kevin Hilman wrote:
> Santosh<santosh.shilimkar@ti.com> writes:
>
>> On Thursday 04 August 2011 04:34 PM, Tarun Kanti DebBarma wrote:
>>> From: Charulatha V<charu@ti.com>
>>>
>>> Currently gpio_context array used to save gpio bank's context, is used only for
>>> OMAP3 architecture. Move gpio_context as part of gpio_bank structure so that it
>>> can be specific to each gpio bank and can be used for any OMAP architecture
>>>
>>> Signed-off-by: Charulatha V<charu@ti.com>
>>> ---
>> Few comments.
>>
>
> [...]
>
>>> @@ -1494,33 +1490,31 @@ void omap2_gpio_resume_after_idle(void)
>>> void omap_gpio_save_context(void)
>>> {
>>> struct gpio_bank *bank;
>>> - int i = 0;
>>>
>>> list_for_each_entry(bank,&omap_gpio_list, node) {
>>> - i++;
>>>
>>> if (!bank->loses_context)
>>> continue;
>>>
>>> - gpio_context[i].irqenable1 =
>>> + bank->context.irqenable1 =
>>> __raw_readl(bank->base + OMAP24XX_GPIO_IRQENABLE1);
>>> - gpio_context[i].irqenable2 =
>>> + bank->context.irqenable2 =
>>> __raw_readl(bank->base + OMAP24XX_GPIO_IRQENABLE2);
>>
>> The context restore procedure should be done carefully. For instance
>> IRQ enabled register should be restored last to avoid any spurious
>> interrupts.
>
> For the sake of clean, easy-to-review patches, this kind of functional
> change should be a separate patch.
>
> The goal of $SUBJECT patch is simply to move the context struct into the
> bank struct, not change the order of the save restore.
>
> Any changing of the order of save/restore should be in a dedicated patch
> with a descriptive changelog since that is changing behavior of the code.
>
Agree.

Regards
Santosh