Hi Bin, On Mon, 2019-01-21 at 10:07 -0600, Bin Liu wrote: > Hi Min, > > On Mon, Jan 21, 2019 at 08:22:31PM +0800, min.guo@xxxxxxxxxxxx wrote: > > From: Min Guo <min.guo@xxxxxxxxxxxx> > > > > This adds support for MediaTek musb controller in > > host, peripheral and otg mode. > > There are some quirk of MediaTek musb controller, such as: > > -W1C interrupt status registers > > -Private data toggle registers > > -No dedicated DMA interrupt line > > > > Signed-off-by: Min Guo <min.guo@xxxxxxxxxxxx> > > Signed-off-by: Yonglong Wu <yonglong.wu@xxxxxxxxxxxx> > > --- > > changes in v4: > > 1. no changes > > > > changes in v3: > > suggested by Bin: > > 1. Remove 'u8/u16 data' parameter in clearb/w() hooks > > 2. Replace musb_readb/w() with musb_clearb/w() to clear interrupts status > > > > changes in v2: > > suggested by Bin: > > 1. Add summarize of MediaTek musb controller differences in the commit log > > 2. Add "|| COMPILE_TEST" in Kconfig > > 3. Move MediaTek's private toggle registers from musb_regs.h to mediatek.c > > 4. Replace musb_readl() with musb_readw() to read 16bit toggle register > > --- > > drivers/usb/musb/Kconfig | 8 +- > > drivers/usb/musb/Makefile | 1 + > > drivers/usb/musb/mediatek.c | 624 ++++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 632 insertions(+), 1 deletion(-) > > create mode 100644 drivers/usb/musb/mediatek.c > > > > [snip] > > > +static irqreturn_t generic_interrupt(int irq, void *__hci) > > +{ > > + unsigned long flags; > > + irqreturn_t retval = IRQ_NONE; > > + struct musb *musb = __hci; > > + > > + spin_lock_irqsave(&musb->lock, flags); > > + musb->int_usb = musb_readb(musb->mregs, MUSB_INTRUSB) & > > + musb_readb(musb->mregs, MUSB_INTRUSBE); > > + musb->int_tx = musb_readw(musb->mregs, MUSB_INTRTX) > > + & musb_readw(musb->mregs, MUSB_INTRTXE); > > + musb->int_rx = musb_readw(musb->mregs, MUSB_INTRRX) > > + & musb_readw(musb->mregs, MUSB_INTRRXE); > > Based on my comment in 5/6, the above 3 musb_readb/w() can be changed to > > > + /* MediaTek controller interrupt status is W1C */ > > + musb_clearw(musb->mregs, MUSB_INTRRX); > > + musb_clearw(musb->mregs, MUSB_INTRTX); > > + musb_clearb(musb->mregs, MUSB_INTRUSB); > > musb->int_usb = musb_clearb(musb->mregs, MUSB_INTRUSB) > musb->int_tx = musb_clearw(musb->mregs, MUSB_INTRTX); > musb->int_rx = musb_clearw(musb->mregs, MUSB_INTRRX); Okay. > > + > > + if (musb->int_usb || musb->int_tx || musb->int_rx) > > + retval = musb_interrupt(musb); > > + > > + spin_unlock_irqrestore(&musb->lock, flags); > > + > > + return retval; > > +} > > + > > [snip] > > > + > > +static u8 mtk_musb_clearb(void __iomem *addr, unsigned int offset) > > +{ > > + u8 data; > > + > > + /* W1C */ > > + data = musb_readb(addr, offset); > > + musb_writeb(addr, offset, data); > > + return musb_readb(addr, offset); > > return data; Okay. > > +} > > + > > +static u16 mtk_musb_clearw(void __iomem *addr, unsigned int offset) > > +{ > > + u16 data; > > + > > + /* W1C */ > > + data = musb_readw(addr, offset); > > + musb_writew(addr, offset, data); > > + return musb_readw(addr, offset); > > return data; Okay. > > Regards, > -Bin.