> From: Varadarajan, Charu Latha
> Sent: Monday, October 26, 2009 4:26 AM
> To: Menon, Nishanth; me@xxxxxxxxxxxxxxx
>
> >Felipe Balbi had written, on 10/23/2009 05:56 PM, the following:
> >> On Fri, Oct 23, 2009 at 09:25:29PM +0530, charu@xxxxxx wrote:
> >>> From: Charulatha V <charu@xxxxxx>
> >>>
> >>> During initialization, GPIO module is reset using soft reset bit
> >>> of SYSCONFIG register
> >>>
> >>> Signed-off-by: Charulatha V <charu@xxxxxx>
> >>> ---
> >>> arch/arm/plat-omap/gpio.c | 12 +++++++++++-
> >>> 1 files changed, 11 insertions(+), 1 deletions(-)
> >>>
> >>> diff --git a/arch/arm/plat-omap/gpio.c b/arch/arm/plat-omap/gpio.c
> >>> index 2304a5d..4579650 100644
> >>> --- a/arch/arm/plat-omap/gpio.c
> >>> +++ b/arch/arm/plat-omap/gpio.c
> >>> @@ -21,6 +21,7 @@
> >>> #include <linux/err.h>
> >>> #include <linux/clk.h>
> >>> #include <linux/io.h>
> >>> +#include <linux/delay.h>
> >>>
> >>> #include <mach/hardware.h>
> >>> #include <asm/irq.h>
> >>> @@ -1670,7 +1671,7 @@ static int __init _omap_gpio_init(void)
> >>> }
> >>> #endif
> >>> for (i = 0; i < gpio_bank_count; i++) {
> >>> - int j, gpio_count = 16;
> >>> + int j, gpio_count = 16, attempt = 0;
> >>
> >> decrementing is better, so:
> Okay
> >>
> >> attempt = 1000
> >please move attempt out of here to the {} for 3430.. Warning for OMAP1.
> Okay
> >>
> >>>
> >>> bank = &gpio_bank[i];
> >>> spin_lock_init(&bank->lock);
> >>> @@ -1698,6 +1699,15 @@ static int __init _omap_gpio_init(void)
> >>> static const u32 non_wakeup_gpios[] = {
> >>> 0xe203ffc0, 0x08700040
> >>> };
> >>> +
> >>> + /* Software Reset of GPIO module */
> >>> + __raw_writel(0x0002, bank->base +
> OMAP24XX_GPIO_SYSCONFIG);
> >>> + while (((__raw_readl(bank->base +
> OMAP24XX_GPIO_SYSSTATUS)
> >>> + & 0x1) == 0) && attempt < 5) {
> >>
> >> && attemp)
> >>
> >>> + udelay(1);
> >>
> >> i guess cpu_relax() is better here.
> I guess cpu_relax is not required because this part of code is called only
> from
> board file during boot-up. Hence this would be the only code to be
> executed.
> >>
> >>> + attempt++;
> >>>
> >>> attempt--;
> >>>
> >>cant we improve this code as following:
> >{
> > u8 attempts = 25;
> > /* Software Reset of GPIO module */
> > __raw_writel(0x0002, bank->base
> > + OMAP24XX_GPIO_SYSCONFIG);
> > /* wait for reset to be done */
> > while (((__raw_readl(bank->base +
> > OMAP24XX_GPIO_SYSSTATUS) & 0x1) == 0)
> > && attempts) {
> > cpu_relax();
> > if (attempts % 5)
> > udelay(1);
> > attempts--;
> > }
> >
> >allows the kernel to do somethin else while we also ensure we have a 5
> >usec guarenteed delay before giving up..
> Doesn't modulo operation cost more in terms of performance? Any specific
> reason for specific 5 microseconds?
You could replace it with >> operator if you like and use 2^x multiples.. I am just sticking 5 us there based on your original code.. so the same logic over here I suppose.. unless I missed something?
Regards,
Nishanth Menon
--
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
