Hi, > -----Original Message----- > From: Felipe Balbi [mailto:felipe.balbi@xxxxxxxxx] > Sent: Monday, November 23, 2009 1:17 PM > To: Gupta, Ajay Kumar > Cc: linux-usb@xxxxxxxxxxxxxxx; Balbi Felipe (Nokia-D/Helsinki); david- > b@xxxxxxxxxxx; Gadiyar, Anand; gregkh@xxxxxxx > Subject: Re: [PATCH 1/2] musb: Add context save and restore support > > Hi, > > On Sat, Nov 21, 2009 at 06:07:37AM +0100, ext Ajay Kumar Gupta wrote: > >+static struct musb_context_registers musb_context; > >+ > >+void musb_save_context(void __iomem *musb_base) > > why so many spaces between __iomem and *musb_base ? Will fix it in next version. > > >+{ > >+ int i; > >+ > >+ musb_platform_save_context(&musb_context); > >+ > >+ musb_context.faddr = musb_readb(musb_base, MUSB_FADDR); > >+ musb_context.power = musb_readb(musb_base, MUSB_POWER); > >+ musb_context.intrtx = musb_readw(musb_base, MUSB_INTRTX); > >+ musb_context.intrrx = musb_readw(musb_base, MUSB_INTRRX); > >+ musb_context.intrtxe = musb_readw(musb_base, MUSB_INTRTXE); > >+ musb_context.intrrxe = musb_readw(musb_base, MUSB_INTRRXE); > >+ musb_context.intrusb = musb_readb(musb_base, MUSB_INTRUSB); > >+ musb_context.intrusbe = musb_readb(musb_base, MUSB_INTRUSBE); > >+ musb_context.frame = musb_readw(musb_base, MUSB_FRAME); > >+ musb_context.index = musb_readb(musb_base, MUSB_INDEX); > >+ musb_context.testmode = musb_readb(musb_base, MUSB_TESTMODE); > >+ musb_context.devctl = musb_readb(musb_base, MUSB_DEVCTL); > > I would say it's unnecessary to save intr[rt]x and intr[rt]xe registers, > we will only have context save when cable is removed anyways, so you > shouldn't have any irqs pending. For the irq enable registers, you can > save them inside musb_context when you first write them, they don't > change that much anyway. Currently whenever cable is attached system doesn't enter into suspend on OMAP3EVM but there may be scenarios where we may use forced idle/stdby. We do change intr[rt]xe and so they need to be saved but I think intr[rt]x should not be saved. > > >+ > >+ for (i = 0; i < MUSB_C_NUM_EPS; ++i) { > >+ musb_writeb(musb_base, MUSB_INDEX, i); > >+ musb_context.index_regs[i].txmaxp = > >+ musb_readw(musb_base, 0x10 + MUSB_TXMAXP); <snip..> > >+} > >+ > >+void musb_restore_context(void __iomem *musb_base) > >+{ > >+ int i; > >+ void __iomem *ep_target_regs; > >+ > >+ musb_writeb(musb_base, MUSB_FADDR, musb_context.faddr); > >+ musb_writeb(musb_base, MUSB_POWER, musb_context.power); > >+ musb_writew(musb_base, MUSB_INTRTX, musb_context.intrtx); > >+ musb_writew(musb_base, MUSB_INTRRX, musb_context.intrrx); > >+ musb_writew(musb_base, MUSB_INTRTXE, musb_context.intrtxe); > >+ musb_writew(musb_base, MUSB_INTRRXE, musb_context.intrrxe); > >+ musb_writeb(musb_base, MUSB_INTRUSB, musb_context.intrusb); > >+ musb_writeb(musb_base, MUSB_INTRUSBE, musb_context.intrusbe); > >+ musb_writew(musb_base, MUSB_FRAME, musb_context.frame); > >+ musb_writeb(musb_base, MUSB_TESTMODE, musb_context.testmode); > >+ musb_writeb(musb_base, MUSB_DEVCTL, musb_context.devctl); > >+ > >+ > > one blank line only. Ok. Will correct. > > > static int musb_suspend(struct device *dev) > > { > > what I would like to see (this is for omap pm guys now) is that omap > uses pm_runtime.h and pm_qos.h. It would also be nice to add a PM_OFF to > pm_message_t so that we know when we're transitioning to off mode. Then > based on PM_OFF we would save context. Is it only OMAP3 specific ? Does other musb platform would support such Changes? -Ajay > > -- > balbi -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html