RE: [PATCH v2 08/18] GPIO: OMAP: Use wkup regs off/suspend support flag

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: Hilman, Kevin
> Sent: Thursday, June 16, 2011 11:09 PM
> To: DebBarma, Tarun Kanti
> Cc: linux-omap@xxxxxxxxxxxxxxx; Shilimkar, Santosh; tony@xxxxxxxxxxx
> Subject: Re: [PATCH v2 08/18] GPIO: OMAP: Use wkup regs off/suspend
> support flag
> 
> Tarun Kanti DebBarma <tarun.kanti@xxxxxx> writes:
> 
> > Wakeup register offsets are initialized according to OMAP versions
> > during device registration. These explicit checks are no longer needed.
> >
> > mpuio_init() function is defined under #ifdefs. It is required only in
> case
> > of MPUIO bank type and only when PM operations are supported by it.
> > This is applicable only in case of OMAP16xx SoC's MPUIO GPIO bank type.
> > For all the other cases it is a dummy function. Hence clean up the same
> > and remove all the OMAP SoC specific #ifdefs.
> >
> > bank_is_mpuio() is defined as a check to identify if the bank type is
> MPUIO.
> > It is not required to define it separately as zero for OMAP2plus. Remove
> this.
> >
> > Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@xxxxxx>
> > Signed-off-by: Charulatha V <charu@xxxxxx>
> 
> [...]
> 
> > diff --git a/arch/arm/mach-omap2/gpio.c b/arch/arm/mach-omap2/gpio.c
> > index cdbc728..ea1556b 100644
> > --- a/arch/arm/mach-omap2/gpio.c
> > +++ b/arch/arm/mach-omap2/gpio.c
> > @@ -72,6 +72,7 @@ static int omap2_gpio_dev_init(struct omap_hwmod *oh,
> void *unused)
> >
> >  	dev_attr = (struct omap_gpio_dev_attr *)oh->dev_attr;
> >  	pdata->bank_width = dev_attr->bank_width;
> > +	pdata->suspend_support = true;
> >  	pdata->dbck_flag = dev_attr->dbck_flag;
> >  	pdata->virtual_irq_start = IH_GPIO_BASE + 32 * (id - 1);
> >  	pdata->get_context_loss_count = omap_gpio_get_context_loss;
> > @@ -108,6 +109,9 @@ static int omap2_gpio_dev_init(struct omap_hwmod
> *oh, void *unused)
> >  		pdata->regs->debounce = OMAP24XX_GPIO_DEBOUNCE_VAL;
> >  		pdata->regs->debounce_en = OMAP24XX_GPIO_DEBOUNCE_EN;
> >  		pdata->regs->ctrl = OMAP24XX_GPIO_CTRL;
> > +		pdata->regs->wkup_status = OMAP24XX_GPIO_WAKE_EN;
> > +		pdata->regs->wkup_clear = OMAP24XX_GPIO_CLEARWKUENA;
> > +		pdata->regs->wkup_set = OMAP24XX_GPIO_SETWKUENA;
> >  		break;
> >  	case 2:
> >  		pdata->bank_type = METHOD_GPIO_44XX;
> > @@ -125,6 +129,9 @@ static int omap2_gpio_dev_init(struct omap_hwmod
> *oh, void *unused)
> >  		pdata->regs->debounce = OMAP4_GPIO_DEBOUNCINGTIME;
> >  		pdata->regs->debounce_en = OMAP4_GPIO_DEBOUNCENABLE;
> >  		pdata->regs->ctrl = OMAP4_GPIO_CTRL;
> > +		pdata->regs->wkup_status = OMAP4_GPIO_IRQWAKEN0;
> > +		pdata->regs->wkup_clear = OMAP4_GPIO_IRQWAKEN0;
> > +		pdata->regs->wkup_set = OMAP4_GPIO_IRQWAKEN0;
> >  		break;
> 
> This is wrong for OMAP4.
> 
> The wkup_clear & wkup_set registers offsets are for the registers
> *dedicated* to set and clear.    Any usage of these offets assumes that
> a single write will either set or clear the register, which is clearly
> not the case with IRQWAKEN, which requires a read/modify/write.
> 
> For example, in suspend this is done:
> 
> 	__raw_writel(0xffffffff, wake_clear);
> 	__raw_writel(bank->suspend_wakeup, wake_set);
> 
> As both the set & clear are set to IRQWAKEN on OMAP4, this means that
> wakeups are actually enabled for *all* GPIOs in every bank during
> suspend.  This is clearly not what's intended.  Also, since resume does
> something similar, wakeups for *all* GPIOs are left enabled after resume
> as well.
Agreed! Thanks.

> 
> Also, the 44xx TRMs recommend not using the set/clear registers at all
> for OMAP4, so they should be left blank.
Yes, I will not assign those offsets.

> 
> Instead, any usage of the wake set/clear registers in the code should
> probably be converted to just use read/modify/writes on wake_status so
> it's the same for all SoCs.
OK.
--
Tarun
> 
> 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


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux