Nishanth Menon <nm@xxxxxx> writes:
> Kevin Hilman had written, on 10/05/2009 12:35 PM, the following:
>> Nishanth Menon <menon.nishanth@xxxxxxxxx> writes:
>>
>>> Kevin Hilman said the following on 10/01/2009 06:58 PM:
>>>> From: Rajendra Nayak <rnayak@xxxxxx>
>>>>
>>>> Signed-off-by: Rajendra Nayak <rnayak@xxxxxx>
>>>> ---
>>>> arch/arm/plat-omap/gpio.c | 92 ++++++++++++++++++++++++++++++++
>>>> arch/arm/plat-omap/include/mach/gpio.h | 3 +-
>>>> 2 files changed, 94 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/arch/arm/plat-omap/gpio.c b/arch/arm/plat-omap/gpio.c
>>>> index b0c7361..9850ade 100644
>>>> --- a/arch/arm/plat-omap/gpio.c
>>>> +++ b/arch/arm/plat-omap/gpio.c
>>>> +
>>>> +/* restore the required registers of bank 2-6 */
>>>> +void omap3_gpio_restore_context(void)
>>>> +{
>>>> + int i;
>>>> + for (i = 1; i < gpio_bank_count; i++) {
>>>> + struct gpio_bank *bank = &gpio_bank[i];
>>>> + __raw_writel(gpio_context[i].sysconfig,
>>>> + bank->base + OMAP24XX_GPIO_SYSCONFIG);
>>>> + __raw_writel(gpio_context[i].irqenable1,
>>>> + bank->base + OMAP24XX_GPIO_IRQENABLE1);
>>>> + __raw_writel(gpio_context[i].irqenable2,
>>>> + bank->base + OMAP24XX_GPIO_IRQENABLE2);
>>>>
>>> do you want to write to the IRQENABLE register even before configuring
>>> the rest of the registers (such as data direction etc?
>>> usually my understanding was:
>>> configure the device,
>>> enable the irq..
>>
>> IIUC, the save/restore sequence was taken directly from TI internal
>> kernels, so I'm not sure of the history there. Rajendra should speak
>> to that as the original author of this patch.
>>
>> That being said, this sequence is being done in the idle path with
>> interrupts disabled, so by the time interrupts are enabled, the GPIO
>> banks will be fully configured.
>
> Still troublesome to my amateur eyes. if ENABLE bit is set -it means
> that GPIO block can assert interrupt & as part of the rest of the
> half-baked configuration, there is a possibility of event happening
> (as we configure, there will be intermediate configured state), we do
> not want an event to be set at all. irq_locked state will just ensure
> that my isr will be called/not called - if the events are banked,
> there could be spurious events detected - but again might be a
> theoretical thought here.
I agree that this is potentially troublesome.
However, I hesitated to change this since this sequence has been used
on TI internal kernels as well as the PM branch for a while now.
Until a changed sequence can be tested and patch proposed with test
results, I prefer to keep this one.
Kevin
--
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
