| Message ID | 1545896066-897-5-git-send-email-min.guo@mediatek.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add MediaTek MUSB Controller Driver | expand |
Hi, On Thu, Dec 27, 2018 at 03:34:26PM +0800, min.guo@mediatek.com wrote: > From: Min Guo <min.guo@mediatek.com> > > This adds support for MediaTek musb controller in > host, peripheral and otg mode > > Signed-off-by: Min Guo <min.guo@mediatek.com> > Signed-off-by: Yonglong Wu <yonglong.wu@mediatek.com> > --- > drivers/usb/musb/Kconfig | 8 +- > drivers/usb/musb/Makefile | 1 + > drivers/usb/musb/mediatek.c | 562 +++++++++++++++++++++++++++++++++++++++++++ > drivers/usb/musb/musb_core.c | 10 + > drivers/usb/musb/musb_core.h | 1 + > drivers/usb/musb/musb_dma.h | 1 + > drivers/usb/musb/musb_host.c | 79 ++++-- > drivers/usb/musb/musb_regs.h | 6 + > drivers/usb/musb/musbhsdma.c | 34 ++- > 9 files changed, 671 insertions(+), 31 deletions(-) > create mode 100644 drivers/usb/musb/mediatek.c > > diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig > index ad08895..540fc9f 100644 > --- a/drivers/usb/musb/Kconfig > +++ b/drivers/usb/musb/Kconfig > @@ -115,6 +115,12 @@ config USB_MUSB_JZ4740 > depends on USB_MUSB_GADGET > depends on USB_OTG_BLACKLIST_HUB > > +config USB_MUSB_MEDIATEK > + tristate "MediaTek platforms" > + depends on ARCH_MEDIATEK Please also add '|| COMPILE_TEST' to make testing easier. > + depends on NOP_USB_XCEIV > + depends on GENERIC_PHY > + > config USB_MUSB_AM335X_CHILD > tristate > > @@ -141,7 +147,7 @@ config USB_UX500_DMA > > config USB_INVENTRA_DMA > bool 'Inventra' > - depends on USB_MUSB_OMAP2PLUS > + depends on USB_MUSB_OMAP2PLUS || USB_MUSB_MEDIATEK > help > Enable DMA transfers using Mentor's engine. > > diff --git a/drivers/usb/musb/Makefile b/drivers/usb/musb/Makefile > index 3a88c79..63d82d0 100644 > --- a/drivers/usb/musb/Makefile > +++ b/drivers/usb/musb/Makefile > @@ -24,6 +24,7 @@ obj-$(CONFIG_USB_MUSB_DA8XX) += da8xx.o > obj-$(CONFIG_USB_MUSB_UX500) += ux500.o > obj-$(CONFIG_USB_MUSB_JZ4740) += jz4740.o > obj-$(CONFIG_USB_MUSB_SUNXI) += sunxi.o > +obj-$(CONFIG_USB_MUSB_MEDIATEK) += mediatek.o > > > obj-$(CONFIG_USB_MUSB_AM335X_CHILD) += musb_am335x.o > diff --git a/drivers/usb/musb/mediatek.c b/drivers/usb/musb/mediatek.c > new file mode 100644 > index 0000000..15a6460 > --- /dev/null > +++ b/drivers/usb/musb/mediatek.c [snip] I will review this section later after we sorted out other things. > diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c > index b7d5627..d60f76f 100644 > --- a/drivers/usb/musb/musb_core.c > +++ b/drivers/usb/musb/musb_core.c > @@ -1028,6 +1028,16 @@ static void musb_disable_interrupts(struct musb *musb) > temp = musb_readb(mbase, MUSB_INTRUSB); > temp = musb_readw(mbase, MUSB_INTRTX); > temp = musb_readw(mbase, MUSB_INTRRX); > + > + /* MediaTek controller interrupt status is W1C */ This W1C doesn't match to the MUSB Programming Guide that I have. Those registers are read-only. Is the difference due to the IP intergration in the mtk platforms? or is it a new version of the MUSB controller? If latter, what is the version? > + if (musb->ops->quirks & MUSB_MTK_QUIRKS) { Basically we don't want to use this type of platform specific quirks if possible, so let's try to not use it. > + musb_writeb(mbase, MUSB_INTRUSB, > + musb_readb(mbase, MUSB_INTRUSB)); For this clearing register bit operation, please create platform hooks musb_clearb() and musb_clearw() in struct musb_platform_ops instead, then follow how musb_readb() pointer is assigned in musb_init_controller() to use the W1C version for mtk platform. > + musb_writew(mbase, MUSB_INTRRX, > + musb_readw(mbase, MUSB_INTRRX)); > + musb_writew(mbase, MUSB_INTRTX, > + musb_readw(mbase, MUSB_INTRTX)); > + } > } > > static void musb_enable_interrupts(struct musb *musb) > diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h > index 04203b7..1bf4e9a 100644 > --- a/drivers/usb/musb/musb_core.h > +++ b/drivers/usb/musb/musb_core.h > @@ -138,6 +138,7 @@ enum musb_g_ep0_state { > */ > struct musb_platform_ops { > > +#define MUSB_MTK_QUIRKS BIT(10) > #define MUSB_G_NO_SKB_RESERVE BIT(9) > #define MUSB_DA8XX BIT(8) > #define MUSB_PRESERVE_SESSION BIT(7) > diff --git a/drivers/usb/musb/musb_dma.h b/drivers/usb/musb/musb_dma.h > index 281e75d3..b218210 100644 > --- a/drivers/usb/musb/musb_dma.h > +++ b/drivers/usb/musb/musb_dma.h > @@ -197,6 +197,7 @@ static inline void musb_dma_controller_destroy(struct dma_controller *d) { } > extern struct dma_controller * > musbhs_dma_controller_create(struct musb *musb, void __iomem *base); > extern void musbhs_dma_controller_destroy(struct dma_controller *c); > +extern irqreturn_t dma_controller_irq(int irq, void *private_data); > > extern struct dma_controller * > tusb_dma_controller_create(struct musb *musb, void __iomem *base); > diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c > index b59ce9a..b1b0216 100644 > --- a/drivers/usb/musb/musb_host.c > +++ b/drivers/usb/musb/musb_host.c > @@ -292,20 +292,73 @@ static inline void musb_save_toggle(struct musb_qh *qh, int is_in, > { > void __iomem *epio = qh->hw_ep->regs; > u16 csr; > + struct musb *musb = qh->hw_ep->musb; > > /* > * FIXME: the current Mentor DMA code seems to have > * problems getting toggle correct. > */ > > - if (is_in) > - csr = musb_readw(epio, MUSB_RXCSR) & MUSB_RXCSR_H_DATATOGGLE; > - else > - csr = musb_readw(epio, MUSB_TXCSR) & MUSB_TXCSR_H_DATATOGGLE; > + /* MediaTek controller has private toggle register */ only one toggle register for all endpoints? how does it handle difference toggle values for different endpoints? > + if (musb->ops->quirks & MUSB_MTK_QUIRKS) { > + u16 toggle; > + u8 epnum = qh->hw_ep->epnum; > + > + if (is_in) > + toggle = musb_readl(musb->mregs, MUSB_RXTOG); should use musb_readw() instead? MUSB_RXTOG seems to be 16bit. > + else > + toggle = musb_readl(musb->mregs, MUSB_TXTOG); > + > + csr = toggle & (1 << epnum); > + } else { > + if (is_in) > + csr = musb_readw(epio, MUSB_RXCSR) > + & MUSB_RXCSR_H_DATATOGGLE; > + else > + csr = musb_readw(epio, MUSB_TXCSR) > + & MUSB_TXCSR_H_DATATOGGLE; Does this logic still work for the mtk platform even if it has its own private toggle register? If so, we don't need to change here. If not, let's try to not use this quirk flag. Please create a hook musb_platform_get_toggle() in struct musb_platform_ops. > + } > > usb_settoggle(urb->dev, qh->epnum, !is_in, csr ? 1 : 0); > } > > +static inline u16 musb_set_toggle(struct musb_qh *qh, int is_in, > + struct urb *urb) > +{ > + u16 csr = 0; > + u16 toggle = 0; > + struct musb *musb = qh->hw_ep->musb; > + u8 epnum = qh->hw_ep->epnum; > + > + toggle = usb_gettoggle(urb->dev, qh->epnum, !is_in); > + > + /* MediaTek controller has private toggle register */ > + if (musb->ops->quirks & MUSB_MTK_QUIRKS) { > + if (is_in) { > + musb_writel(musb->mregs, MUSB_RXTOGEN, (1 << epnum)); > + musb_writel(musb->mregs, MUSB_RXTOG, (toggle << epnum)); > + } else { > + musb_writel(musb->mregs, MUSB_TXTOGEN, (1 << epnum)); > + musb_writel(musb->mregs, MUSB_TXTOG, (toggle << epnum)); > + } > + } else { > + if (is_in) { > + if (toggle) > + csr = MUSB_RXCSR_H_WR_DATATOGGLE > + | MUSB_RXCSR_H_DATATOGGLE; > + else > + csr = 0; > + } else { > + if (toggle) > + csr |= MUSB_TXCSR_H_WR_DATATOGGLE > + | MUSB_TXCSR_H_DATATOGGLE; > + else > + csr |= MUSB_TXCSR_CLRDATATOG; > + } > + } > + return csr; Please create a seperate patch for this musb_set_toggle() without adding the mtk logic. It is a nice cleanup. > +} > + > /* > * Advance this hardware endpoint's queue, completing the specified URB and > * advancing to either the next URB queued to that qh, or else invalidating > @@ -772,13 +825,8 @@ static void musb_ep_program(struct musb *musb, u8 epnum, > ); > csr |= MUSB_TXCSR_MODE; > > - if (!hw_ep->tx_double_buffered) { > - if (usb_gettoggle(urb->dev, qh->epnum, 1)) > - csr |= MUSB_TXCSR_H_WR_DATATOGGLE > - | MUSB_TXCSR_H_DATATOGGLE; > - else > - csr |= MUSB_TXCSR_CLRDATATOG; > - } > + if (!hw_ep->tx_double_buffered) > + csr |= musb_set_toggle(qh, !is_out, urb); > > musb_writew(epio, MUSB_TXCSR, csr); > /* REVISIT may need to clear FLUSHFIFO ... */ > @@ -860,17 +908,12 @@ static void musb_ep_program(struct musb *musb, u8 epnum, > > /* IN/receive */ > } else { > - u16 csr; > + u16 csr = 0; > > if (hw_ep->rx_reinit) { > musb_rx_reinit(musb, qh, epnum); > + csr |= musb_set_toggle(qh, !is_out, urb); > > - /* init new state: toggle and NYET, maybe DMA later */ > - if (usb_gettoggle(urb->dev, qh->epnum, 0)) > - csr = MUSB_RXCSR_H_WR_DATATOGGLE > - | MUSB_RXCSR_H_DATATOGGLE; > - else > - csr = 0; > if (qh->type == USB_ENDPOINT_XFER_INT) > csr |= MUSB_RXCSR_DISNYET; > > diff --git a/drivers/usb/musb/musb_regs.h b/drivers/usb/musb/musb_regs.h > index 5cd7264..ffbe267 100644 > --- a/drivers/usb/musb/musb_regs.h > +++ b/drivers/usb/musb/musb_regs.h > @@ -273,6 +273,12 @@ > #define MUSB_RXHUBADDR 0x06 > #define MUSB_RXHUBPORT 0x07 > > +/* MediaTek controller toggle enable and status reg */ > +#define MUSB_RXTOG 0x80 > +#define MUSB_RXTOGEN 0x82 > +#define MUSB_TXTOG 0x84 > +#define MUSB_TXTOGEN 0x86 Again, these offsets are for different registers in the MUSB version I have, please let me know if you have different version of the MUSB IP. > + > static inline u8 musb_read_configdata(void __iomem *mbase) > { > musb_writeb(mbase, MUSB_INDEX, 0); > diff --git a/drivers/usb/musb/musbhsdma.c b/drivers/usb/musb/musbhsdma.c > index 824adcb..c545475 100644 > --- a/drivers/usb/musb/musbhsdma.c > +++ b/drivers/usb/musb/musbhsdma.c > @@ -263,7 +263,7 @@ static int dma_channel_abort(struct dma_channel *channel) > return 0; > } > > -static irqreturn_t dma_controller_irq(int irq, void *private_data) > +irqreturn_t dma_controller_irq(int irq, void *private_data) > { > struct musb_dma_controller *controller = private_data; > struct musb *musb = controller->private_data; > @@ -285,6 +285,8 @@ static irqreturn_t dma_controller_irq(int irq, void *private_data) > spin_lock_irqsave(&musb->lock, flags); > > int_hsdma = musb_readb(mbase, MUSB_HSDMA_INTR); > + if (musb->ops->quirks & MUSB_MTK_QUIRKS) > + musb_writeb(musb->mregs, MUSB_HSDMA_INTR, int_hsdma); you can use musb_clearb() defined above to get rid of this quirk. > > if (!int_hsdma) { > musb_dbg(musb, "spurious DMA irq"); > @@ -377,15 +379,17 @@ static irqreturn_t dma_controller_irq(int irq, void *private_data) > spin_unlock_irqrestore(&musb->lock, flags); > return retval; > } > +EXPORT_SYMBOL_GPL(dma_controller_irq); > > void musbhs_dma_controller_destroy(struct dma_controller *c) > { > struct musb_dma_controller *controller = container_of(c, > struct musb_dma_controller, controller); > + struct musb *musb = controller->private_data; > > dma_controller_stop(controller); > > - if (controller->irq) > + if (!(musb->ops->quirks & MUSB_MTK_QUIRKS) && controller->irq) > free_irq(controller->irq, c); > > kfree(controller); > @@ -398,11 +402,15 @@ struct dma_controller *musbhs_dma_controller_create(struct musb *musb, > struct musb_dma_controller *controller; > struct device *dev = musb->controller; > struct platform_device *pdev = to_platform_device(dev); > - int irq = platform_get_irq_byname(pdev, "dma"); > + int irq = -1; > > - if (irq <= 0) { > - dev_err(dev, "No DMA interrupt line!\n"); > - return NULL; > + if (!(musb->ops->quirks & MUSB_MTK_QUIRKS)) { > + irq = platform_get_irq_byname(pdev, "dma"); > + > + if (irq < 0) { > + dev_err(dev, "No DMA interrupt line!\n"); > + return NULL; > + } > } Please create musbhs_dma_controller_destroy_noirq() for your platform to not use the quirk. > > controller = kzalloc(sizeof(*controller), GFP_KERNEL); > @@ -418,15 +426,17 @@ struct dma_controller *musbhs_dma_controller_create(struct musb *musb, > controller->controller.channel_program = dma_channel_program; > controller->controller.channel_abort = dma_channel_abort; > > - if (request_irq(irq, dma_controller_irq, 0, > + if (!(musb->ops->quirks & MUSB_MTK_QUIRKS)) { > + if (request_irq(irq, dma_controller_irq, 0, > dev_name(musb->controller), &controller->controller)) { > - dev_err(dev, "request_irq %d failed!\n", irq); > - musb_dma_controller_destroy(&controller->controller); > + dev_err(dev, "request_irq %d failed!\n", irq); > + musb_dma_controller_destroy(&controller->controller); > > - return NULL; > - } > + return NULL; > + } > > - controller->irq = irq; > + controller->irq = irq; > + } > > return &controller->controller; > } Same here, create musbhs_dma_controller_create_noirq(). Then use both new API for the mtk glue driver. Regards, -Bin.
Hi Bin, On Tue, 2019-01-08 at 09:44 -0600, Bin Liu wrote: > Hi, > > On Thu, Dec 27, 2018 at 03:34:26PM +0800, min.guo@mediatek.com wrote: > > From: Min Guo <min.guo@mediatek.com> > > > > This adds support for MediaTek musb controller in > > host, peripheral and otg mode > > > > Signed-off-by: Min Guo <min.guo@mediatek.com> > > Signed-off-by: Yonglong Wu <yonglong.wu@mediatek.com> > > --- > > drivers/usb/musb/Kconfig | 8 +- > > drivers/usb/musb/Makefile | 1 + > > drivers/usb/musb/mediatek.c | 562 +++++++++++++++++++++++++++++++++++++++++++ > > drivers/usb/musb/musb_core.c | 10 + > > drivers/usb/musb/musb_core.h | 1 + > > drivers/usb/musb/musb_dma.h | 1 + > > drivers/usb/musb/musb_host.c | 79 ++++-- > > drivers/usb/musb/musb_regs.h | 6 + > > drivers/usb/musb/musbhsdma.c | 34 ++- > > 9 files changed, 671 insertions(+), 31 deletions(-) > > create mode 100644 drivers/usb/musb/mediatek.c > > > > diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig > > index ad08895..540fc9f 100644 > > --- a/drivers/usb/musb/Kconfig > > +++ b/drivers/usb/musb/Kconfig > > @@ -115,6 +115,12 @@ config USB_MUSB_JZ4740 > > depends on USB_MUSB_GADGET > > depends on USB_OTG_BLACKLIST_HUB > > > > +config USB_MUSB_MEDIATEK > > + tristate "MediaTek platforms" > > + depends on ARCH_MEDIATEK > > Please also add '|| COMPILE_TEST' to make testing easier. Ok > > + depends on NOP_USB_XCEIV > > + depends on GENERIC_PHY > > + > > config USB_MUSB_AM335X_CHILD > > tristate > > > > @@ -141,7 +147,7 @@ config USB_UX500_DMA > > > > config USB_INVENTRA_DMA > > bool 'Inventra' > > - depends on USB_MUSB_OMAP2PLUS > > + depends on USB_MUSB_OMAP2PLUS || USB_MUSB_MEDIATEK > > help > > Enable DMA transfers using Mentor's engine. > > > > diff --git a/drivers/usb/musb/Makefile b/drivers/usb/musb/Makefile > > index 3a88c79..63d82d0 100644 > > --- a/drivers/usb/musb/Makefile > > +++ b/drivers/usb/musb/Makefile > > @@ -24,6 +24,7 @@ obj-$(CONFIG_USB_MUSB_DA8XX) += da8xx.o > > obj-$(CONFIG_USB_MUSB_UX500) += ux500.o > > obj-$(CONFIG_USB_MUSB_JZ4740) += jz4740.o > > obj-$(CONFIG_USB_MUSB_SUNXI) += sunxi.o > > +obj-$(CONFIG_USB_MUSB_MEDIATEK) += mediatek.o > > > > > > obj-$(CONFIG_USB_MUSB_AM335X_CHILD) += musb_am335x.o > > diff --git a/drivers/usb/musb/mediatek.c b/drivers/usb/musb/mediatek.c > > new file mode 100644 > > index 0000000..15a6460 > > --- /dev/null > > +++ b/drivers/usb/musb/mediatek.c > > [snip] > I will review this section later after we sorted out other things. > > > diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c > > index b7d5627..d60f76f 100644 > > --- a/drivers/usb/musb/musb_core.c > > +++ b/drivers/usb/musb/musb_core.c > > @@ -1028,6 +1028,16 @@ static void musb_disable_interrupts(struct musb *musb) > > temp = musb_readb(mbase, MUSB_INTRUSB); > > temp = musb_readw(mbase, MUSB_INTRTX); > > temp = musb_readw(mbase, MUSB_INTRRX); > > + > > + /* MediaTek controller interrupt status is W1C */ > > This W1C doesn't match to the MUSB Programming Guide that I have. Those > registers are read-only. > Is the difference due to the IP intergration in the mtk platforms? or is > it a new version of the MUSB controller? If latter, what is the version? This is difference due to the IP intergration in mtk platforms. W1C is easy for CpdeViser debug. > > + if (musb->ops->quirks & MUSB_MTK_QUIRKS) { > > Basically we don't want to use this type of platform specific quirks if > possible, so let's try to not use it. I will try my best to avoid using it. > > + musb_writeb(mbase, MUSB_INTRUSB, > > + musb_readb(mbase, MUSB_INTRUSB)); > > For this clearing register bit operation, please create platform hooks > musb_clearb() and musb_clearw() in struct musb_platform_ops instead, > then follow how musb_readb() pointer is assigned in > musb_init_controller() to use the W1C version for mtk platform. I have tried implementing musb_readb(), musb_readw() interface with interrupt status W1C function in struct musb_platform_ops. But this interface will require a global variable to hold MAC basic address for judgment, and then special handling of the interrupt state. A global variable will make the driver work with only a single instance, so it can't work on some MTK platforms which have two instances. How about creating musb_clearb/w() as following: void (*clearb)(void __iomem *addr, unsigned offset, u8 data); void (*clearw)(void __iomem *addr, unsigned offset, u16 data); > > + musb_writew(mbase, MUSB_INTRRX, > > + musb_readw(mbase, MUSB_INTRRX)); > > + musb_writew(mbase, MUSB_INTRTX, > > + musb_readw(mbase, MUSB_INTRTX)); > > + } > > } > > > > static void musb_enable_interrupts(struct musb *musb) > > diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h > > index 04203b7..1bf4e9a 100644 > > --- a/drivers/usb/musb/musb_core.h > > +++ b/drivers/usb/musb/musb_core.h > > @@ -138,6 +138,7 @@ enum musb_g_ep0_state { > > */ > > struct musb_platform_ops { > > > > +#define MUSB_MTK_QUIRKS BIT(10) > > #define MUSB_G_NO_SKB_RESERVE BIT(9) > > #define MUSB_DA8XX BIT(8) > > #define MUSB_PRESERVE_SESSION BIT(7) > > diff --git a/drivers/usb/musb/musb_dma.h b/drivers/usb/musb/musb_dma.h > > index 281e75d3..b218210 100644 > > --- a/drivers/usb/musb/musb_dma.h > > +++ b/drivers/usb/musb/musb_dma.h > > @@ -197,6 +197,7 @@ static inline void musb_dma_controller_destroy(struct dma_controller *d) { } > > extern struct dma_controller * > > musbhs_dma_controller_create(struct musb *musb, void __iomem *base); > > extern void musbhs_dma_controller_destroy(struct dma_controller *c); > > +extern irqreturn_t dma_controller_irq(int irq, void *private_data); > > > > extern struct dma_controller * > > tusb_dma_controller_create(struct musb *musb, void __iomem *base); > > diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c > > index b59ce9a..b1b0216 100644 > > --- a/drivers/usb/musb/musb_host.c > > +++ b/drivers/usb/musb/musb_host.c > > @@ -292,20 +292,73 @@ static inline void musb_save_toggle(struct musb_qh *qh, int is_in, > > { > > void __iomem *epio = qh->hw_ep->regs; > > u16 csr; > > + struct musb *musb = qh->hw_ep->musb; > > > > /* > > * FIXME: the current Mentor DMA code seems to have > > * problems getting toggle correct. > > */ > > > > - if (is_in) > > - csr = musb_readw(epio, MUSB_RXCSR) & MUSB_RXCSR_H_DATATOGGLE; > > - else > > - csr = musb_readw(epio, MUSB_TXCSR) & MUSB_TXCSR_H_DATATOGGLE; > > + /* MediaTek controller has private toggle register */ > > only one toggle register for all endpoints? how does it handle > difference toggle values for different endpoints? MediaTek controller has separate registers to describe TX/RX toggle. > > + if (musb->ops->quirks & MUSB_MTK_QUIRKS) { > > + u16 toggle; > > + u8 epnum = qh->hw_ep->epnum; > > + > > + if (is_in) > > + toggle = musb_readl(musb->mregs, MUSB_RXTOG); > > should use musb_readw() instead? MUSB_RXTOG seems to be 16bit. Ok > > + else > > + toggle = musb_readl(musb->mregs, MUSB_TXTOG); > > + > > + csr = toggle & (1 << epnum); > > + } else { > > + if (is_in) > > + csr = musb_readw(epio, MUSB_RXCSR) > > + & MUSB_RXCSR_H_DATATOGGLE; > > + else > > + csr = musb_readw(epio, MUSB_TXCSR) > > + & MUSB_TXCSR_H_DATATOGGLE; > > Does this logic still work for the mtk platform even if it has its own > private toggle register? If so, we don't need to change here. Sorry, this logic can not work on mtk platform, bit MUSB_RXCSR_H_DATATOGGLE and MUSB_TXCSR_H_DATATOGGLE are used for other function. > If not, let's try to not use this quirk flag. Please create a hook > musb_platform_get_toggle() in struct musb_platform_ops. Does the method of implement musb_platform_get_toggle() is prepare musb_default_get_toggle with common function, then follow how musb_readb() pointer is assigned in musb_init_controller()? How about creating musb_platform_get_toggle() as following: u16 (*get_toggle)(struct musb* musb, struct musb_qh *qh, int is_in); > > + } > > > > usb_settoggle(urb->dev, qh->epnum, !is_in, csr ? 1 : 0); > > } > > > > +static inline u16 musb_set_toggle(struct musb_qh *qh, int is_in, > > + struct urb *urb) > > +{ > > + u16 csr = 0; > > + u16 toggle = 0; > > + struct musb *musb = qh->hw_ep->musb; > > + u8 epnum = qh->hw_ep->epnum; > > + > > + toggle = usb_gettoggle(urb->dev, qh->epnum, !is_in); > > + > > + /* MediaTek controller has private toggle register */ > > + if (musb->ops->quirks & MUSB_MTK_QUIRKS) { > > + if (is_in) { > > + musb_writel(musb->mregs, MUSB_RXTOGEN, (1 << epnum)); > > + musb_writel(musb->mregs, MUSB_RXTOG, (toggle << epnum)); > > + } else { > > + musb_writel(musb->mregs, MUSB_TXTOGEN, (1 << epnum)); > > + musb_writel(musb->mregs, MUSB_TXTOG, (toggle << epnum)); > > + } > > + } else { > > + if (is_in) { > > + if (toggle) > > + csr = MUSB_RXCSR_H_WR_DATATOGGLE > > + | MUSB_RXCSR_H_DATATOGGLE; > > + else > > + csr = 0; > > + } else { > > + if (toggle) > > + csr |= MUSB_TXCSR_H_WR_DATATOGGLE > > + | MUSB_TXCSR_H_DATATOGGLE; > > + else > > + csr |= MUSB_TXCSR_CLRDATATOG; > > + } > > + } > > + return csr; > > Please create a seperate patch for this musb_set_toggle() without adding > the mtk logic. It is a nice cleanup. Does this like get toggle implementation, create a hook musb_platform_set_toggle() in struct musb_platform_ops? > > +} > > + > > /* > > * Advance this hardware endpoint's queue, completing the specified URB and > > * advancing to either the next URB queued to that qh, or else invalidating > > @@ -772,13 +825,8 @@ static void musb_ep_program(struct musb *musb, u8 epnum, > > ); > > csr |= MUSB_TXCSR_MODE; > > > > - if (!hw_ep->tx_double_buffered) { > > - if (usb_gettoggle(urb->dev, qh->epnum, 1)) > > - csr |= MUSB_TXCSR_H_WR_DATATOGGLE > > - | MUSB_TXCSR_H_DATATOGGLE; > > - else > > - csr |= MUSB_TXCSR_CLRDATATOG; > > - } > > + if (!hw_ep->tx_double_buffered) > > + csr |= musb_set_toggle(qh, !is_out, urb); > > > > musb_writew(epio, MUSB_TXCSR, csr); > > /* REVISIT may need to clear FLUSHFIFO ... */ > > @@ -860,17 +908,12 @@ static void musb_ep_program(struct musb *musb, u8 epnum, > > > > /* IN/receive */ > > } else { > > - u16 csr; > > + u16 csr = 0; > > > > if (hw_ep->rx_reinit) { > > musb_rx_reinit(musb, qh, epnum); > > + csr |= musb_set_toggle(qh, !is_out, urb); > > > > - /* init new state: toggle and NYET, maybe DMA later */ > > - if (usb_gettoggle(urb->dev, qh->epnum, 0)) > > - csr = MUSB_RXCSR_H_WR_DATATOGGLE > > - | MUSB_RXCSR_H_DATATOGGLE; > > - else > > - csr = 0; > > if (qh->type == USB_ENDPOINT_XFER_INT) > > csr |= MUSB_RXCSR_DISNYET; > > > > diff --git a/drivers/usb/musb/musb_regs.h b/drivers/usb/musb/musb_regs.h > > index 5cd7264..ffbe267 100644 > > --- a/drivers/usb/musb/musb_regs.h > > +++ b/drivers/usb/musb/musb_regs.h > > @@ -273,6 +273,12 @@ > > #define MUSB_RXHUBADDR 0x06 > > #define MUSB_RXHUBPORT 0x07 > > > > +/* MediaTek controller toggle enable and status reg */ > > +#define MUSB_RXTOG 0x80 > > +#define MUSB_RXTOGEN 0x82 > > +#define MUSB_TXTOG 0x84 > > +#define MUSB_TXTOGEN 0x86 > > Again, these offsets are for different registers in the MUSB version I > have, please let me know if you have different version of the MUSB IP. Sorry, these are MediaTek controller private registers used for control toggle. > > + > > static inline u8 musb_read_configdata(void __iomem *mbase) > > { > > musb_writeb(mbase, MUSB_INDEX, 0); > > diff --git a/drivers/usb/musb/musbhsdma.c b/drivers/usb/musb/musbhsdma.c > > index 824adcb..c545475 100644 > > --- a/drivers/usb/musb/musbhsdma.c > > +++ b/drivers/usb/musb/musbhsdma.c > > @@ -263,7 +263,7 @@ static int dma_channel_abort(struct dma_channel *channel) > > return 0; > > } > > > > -static irqreturn_t dma_controller_irq(int irq, void *private_data) > > +irqreturn_t dma_controller_irq(int irq, void *private_data) > > { > > struct musb_dma_controller *controller = private_data; > > struct musb *musb = controller->private_data; > > @@ -285,6 +285,8 @@ static irqreturn_t dma_controller_irq(int irq, void *private_data) > > spin_lock_irqsave(&musb->lock, flags); > > > > int_hsdma = musb_readb(mbase, MUSB_HSDMA_INTR); > > + if (musb->ops->quirks & MUSB_MTK_QUIRKS) > > + musb_writeb(musb->mregs, MUSB_HSDMA_INTR, int_hsdma); > > you can use musb_clearb() defined above to get rid of this quirk. OK. > > > > if (!int_hsdma) { > > musb_dbg(musb, "spurious DMA irq"); > > @@ -377,15 +379,17 @@ static irqreturn_t dma_controller_irq(int irq, void *private_data) > > spin_unlock_irqrestore(&musb->lock, flags); > > return retval; > > } > > +EXPORT_SYMBOL_GPL(dma_controller_irq); > > > > void musbhs_dma_controller_destroy(struct dma_controller *c) > > { > > struct musb_dma_controller *controller = container_of(c, > > struct musb_dma_controller, controller); > > + struct musb *musb = controller->private_data; > > > > dma_controller_stop(controller); > > > > - if (controller->irq) > > + if (!(musb->ops->quirks & MUSB_MTK_QUIRKS) && controller->irq) > > free_irq(controller->irq, c); > > > > kfree(controller); > > @@ -398,11 +402,15 @@ struct dma_controller *musbhs_dma_controller_create(struct musb *musb, > > struct musb_dma_controller *controller; > > struct device *dev = musb->controller; > > struct platform_device *pdev = to_platform_device(dev); > > - int irq = platform_get_irq_byname(pdev, "dma"); > > + int irq = -1; > > > > - if (irq <= 0) { > > - dev_err(dev, "No DMA interrupt line!\n"); > > - return NULL; > > + if (!(musb->ops->quirks & MUSB_MTK_QUIRKS)) { > > + irq = platform_get_irq_byname(pdev, "dma"); > > + > > + if (irq < 0) { > > + dev_err(dev, "No DMA interrupt line!\n"); > > + return NULL; > > + } > > } > > Please create musbhs_dma_controller_destroy_noirq() for your platform to > not use the quirk. OK. > > > > controller = kzalloc(sizeof(*controller), GFP_KERNEL); > > @@ -418,15 +426,17 @@ struct dma_controller *musbhs_dma_controller_create(struct musb *musb, > > controller->controller.channel_program = dma_channel_program; > > controller->controller.channel_abort = dma_channel_abort; > > > > - if (request_irq(irq, dma_controller_irq, 0, > > + if (!(musb->ops->quirks & MUSB_MTK_QUIRKS)) { > > + if (request_irq(irq, dma_controller_irq, 0, > > dev_name(musb->controller), &controller->controller)) { > > - dev_err(dev, "request_irq %d failed!\n", irq); > > - musb_dma_controller_destroy(&controller->controller); > > + dev_err(dev, "request_irq %d failed!\n", irq); > > + musb_dma_controller_destroy(&controller->controller); > > > > - return NULL; > > - } > > + return NULL; > > + } > > > > - controller->irq = irq; > > + controller->irq = irq; > > + } > > > > return &controller->controller; > > } > > Same here, create musbhs_dma_controller_create_noirq(). Then use both > new API for the mtk glue driver. OK. > Regards, > -Bin.
Hi Min, On Wed, Jan 09, 2019 at 08:31:08PM +0800, Min Guo wrote: > Hi Bin, > On Tue, 2019-01-08 at 09:44 -0600, Bin Liu wrote: > > Hi, > > > > On Thu, Dec 27, 2018 at 03:34:26PM +0800, min.guo@mediatek.com wrote: > > > From: Min Guo <min.guo@mediatek.com> > > > > > > This adds support for MediaTek musb controller in > > > host, peripheral and otg mode > > > > > > Signed-off-by: Min Guo <min.guo@mediatek.com> > > > Signed-off-by: Yonglong Wu <yonglong.wu@mediatek.com> > > > --- > > > drivers/usb/musb/Kconfig | 8 +- > > > drivers/usb/musb/Makefile | 1 + > > > drivers/usb/musb/mediatek.c | 562 +++++++++++++++++++++++++++++++++++++++++++ > > > drivers/usb/musb/musb_core.c | 10 + > > > drivers/usb/musb/musb_core.h | 1 + > > > drivers/usb/musb/musb_dma.h | 1 + > > > drivers/usb/musb/musb_host.c | 79 ++++-- > > > drivers/usb/musb/musb_regs.h | 6 + > > > drivers/usb/musb/musbhsdma.c | 34 ++- > > > 9 files changed, 671 insertions(+), 31 deletions(-) > > > create mode 100644 drivers/usb/musb/mediatek.c > > > > > > diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig > > > index ad08895..540fc9f 100644 > > > --- a/drivers/usb/musb/Kconfig > > > +++ b/drivers/usb/musb/Kconfig > > > @@ -115,6 +115,12 @@ config USB_MUSB_JZ4740 > > > depends on USB_MUSB_GADGET > > > depends on USB_OTG_BLACKLIST_HUB > > > > > > +config USB_MUSB_MEDIATEK > > > + tristate "MediaTek platforms" > > > + depends on ARCH_MEDIATEK > > > > Please also add '|| COMPILE_TEST' to make testing easier. > > Ok > > > > + depends on NOP_USB_XCEIV > > > + depends on GENERIC_PHY > > > + > > > config USB_MUSB_AM335X_CHILD > > > tristate > > > > > > @@ -141,7 +147,7 @@ config USB_UX500_DMA > > > > > > config USB_INVENTRA_DMA > > > bool 'Inventra' > > > - depends on USB_MUSB_OMAP2PLUS > > > + depends on USB_MUSB_OMAP2PLUS || USB_MUSB_MEDIATEK > > > help > > > Enable DMA transfers using Mentor's engine. > > > > > > diff --git a/drivers/usb/musb/Makefile b/drivers/usb/musb/Makefile > > > index 3a88c79..63d82d0 100644 > > > --- a/drivers/usb/musb/Makefile > > > +++ b/drivers/usb/musb/Makefile > > > @@ -24,6 +24,7 @@ obj-$(CONFIG_USB_MUSB_DA8XX) += da8xx.o > > > obj-$(CONFIG_USB_MUSB_UX500) += ux500.o > > > obj-$(CONFIG_USB_MUSB_JZ4740) += jz4740.o > > > obj-$(CONFIG_USB_MUSB_SUNXI) += sunxi.o > > > +obj-$(CONFIG_USB_MUSB_MEDIATEK) += mediatek.o > > > > > > > > > obj-$(CONFIG_USB_MUSB_AM335X_CHILD) += musb_am335x.o > > > diff --git a/drivers/usb/musb/mediatek.c b/drivers/usb/musb/mediatek.c > > > new file mode 100644 > > > index 0000000..15a6460 > > > --- /dev/null > > > +++ b/drivers/usb/musb/mediatek.c > > > > [snip] > > I will review this section later after we sorted out other things. > > > > > diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c > > > index b7d5627..d60f76f 100644 > > > --- a/drivers/usb/musb/musb_core.c > > > +++ b/drivers/usb/musb/musb_core.c > > > @@ -1028,6 +1028,16 @@ static void musb_disable_interrupts(struct musb *musb) > > > temp = musb_readb(mbase, MUSB_INTRUSB); > > > temp = musb_readw(mbase, MUSB_INTRTX); > > > temp = musb_readw(mbase, MUSB_INTRRX); > > > + > > > + /* MediaTek controller interrupt status is W1C */ > > > > This W1C doesn't match to the MUSB Programming Guide that I have. Those > > registers are read-only. > > Is the difference due to the IP intergration in the mtk platforms? or is > > it a new version of the MUSB controller? If latter, what is the version? > > This is difference due to the IP intergration in mtk platforms. W1C is > easy for CpdeViser debug. > > > > + if (musb->ops->quirks & MUSB_MTK_QUIRKS) { > > > > Basically we don't want to use this type of platform specific quirks if > > possible, so let's try to not use it. > > I will try my best to avoid using it. Thanks. > > > > + musb_writeb(mbase, MUSB_INTRUSB, > > > + musb_readb(mbase, MUSB_INTRUSB)); > > > > For this clearing register bit operation, please create platform hooks > > musb_clearb() and musb_clearw() in struct musb_platform_ops instead, > > then follow how musb_readb() pointer is assigned in > > musb_init_controller() to use the W1C version for mtk platform. > > I have tried implementing musb_readb(), musb_readw() interface with > interrupt status W1C function in struct musb_platform_ops. But this > interface will require a global variable to hold MAC basic address for > judgment, and then special handling of the interrupt state. A global > variable will make the driver work with only a single instance, so it > can't work on some MTK platforms which have two instances. I didn't mean to modify musb_read*(), but > How about creating musb_clearb/w() as following: > void (*clearb)(void __iomem *addr, unsigned offset, u8 data); > void (*clearw)(void __iomem *addr, unsigned offset, u16 data); this is what I was asking for, similar to what musb_readb/w() is implemented. > > > > > + musb_writew(mbase, MUSB_INTRRX, > > > + musb_readw(mbase, MUSB_INTRRX)); > > > + musb_writew(mbase, MUSB_INTRTX, > > > + musb_readw(mbase, MUSB_INTRTX)); > > > + } > > > } > > > > > > static void musb_enable_interrupts(struct musb *musb) > > > diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h > > > index 04203b7..1bf4e9a 100644 > > > --- a/drivers/usb/musb/musb_core.h > > > +++ b/drivers/usb/musb/musb_core.h > > > @@ -138,6 +138,7 @@ enum musb_g_ep0_state { > > > */ > > > struct musb_platform_ops { > > > > > > +#define MUSB_MTK_QUIRKS BIT(10) > > > #define MUSB_G_NO_SKB_RESERVE BIT(9) > > > #define MUSB_DA8XX BIT(8) > > > #define MUSB_PRESERVE_SESSION BIT(7) > > > diff --git a/drivers/usb/musb/musb_dma.h b/drivers/usb/musb/musb_dma.h > > > index 281e75d3..b218210 100644 > > > --- a/drivers/usb/musb/musb_dma.h > > > +++ b/drivers/usb/musb/musb_dma.h > > > @@ -197,6 +197,7 @@ static inline void musb_dma_controller_destroy(struct dma_controller *d) { } > > > extern struct dma_controller * > > > musbhs_dma_controller_create(struct musb *musb, void __iomem *base); > > > extern void musbhs_dma_controller_destroy(struct dma_controller *c); > > > +extern irqreturn_t dma_controller_irq(int irq, void *private_data); > > > > > > extern struct dma_controller * > > > tusb_dma_controller_create(struct musb *musb, void __iomem *base); > > > diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c > > > index b59ce9a..b1b0216 100644 > > > --- a/drivers/usb/musb/musb_host.c > > > +++ b/drivers/usb/musb/musb_host.c > > > @@ -292,20 +292,73 @@ static inline void musb_save_toggle(struct musb_qh *qh, int is_in, > > > { > > > void __iomem *epio = qh->hw_ep->regs; > > > u16 csr; > > > + struct musb *musb = qh->hw_ep->musb; > > > > > > /* > > > * FIXME: the current Mentor DMA code seems to have > > > * problems getting toggle correct. > > > */ > > > > > > - if (is_in) > > > - csr = musb_readw(epio, MUSB_RXCSR) & MUSB_RXCSR_H_DATATOGGLE; > > > - else > > > - csr = musb_readw(epio, MUSB_TXCSR) & MUSB_TXCSR_H_DATATOGGLE; > > > + /* MediaTek controller has private toggle register */ > > > > only one toggle register for all endpoints? how does it handle > > difference toggle values for different endpoints? > > MediaTek controller has separate registers to describe TX/RX toggle. Is it one register per endpoint? > > > > + if (musb->ops->quirks & MUSB_MTK_QUIRKS) { > > > + u16 toggle; > > > + u8 epnum = qh->hw_ep->epnum; > > > + > > > + if (is_in) > > > + toggle = musb_readl(musb->mregs, MUSB_RXTOG); this line seems telling there is just *one* register for all endpoints. > > > > should use musb_readw() instead? MUSB_RXTOG seems to be 16bit. > > Ok > > > > + else > > > + toggle = musb_readl(musb->mregs, MUSB_TXTOG); > > > + > > > + csr = toggle & (1 << epnum); > > > + } else { > > > + if (is_in) > > > + csr = musb_readw(epio, MUSB_RXCSR) > > > + & MUSB_RXCSR_H_DATATOGGLE; > > > + else > > > + csr = musb_readw(epio, MUSB_TXCSR) > > > + & MUSB_TXCSR_H_DATATOGGLE; > > > > Does this logic still work for the mtk platform even if it has its own > > private toggle register? If so, we don't need to change here. > > Sorry, this logic can not work on mtk platform, bit > MUSB_RXCSR_H_DATATOGGLE and MUSB_TXCSR_H_DATATOGGLE are used for other > function. Is there a different controller RTL version we can use to differentiate? > > > If not, let's try to not use this quirk flag. Please create a hook > > musb_platform_get_toggle() in struct musb_platform_ops. > > Does the method of implement musb_platform_get_toggle() is prepare > musb_default_get_toggle with common function, then follow how > musb_readb() pointer is assigned in musb_init_controller()? Yes, similar to musb_readb() implementation. > How about creating musb_platform_get_toggle() as following: > u16 (*get_toggle)(struct musb* musb, struct musb_qh *qh, int is_in); yes, it is part of the implementation, then add it in struct musb_io. > > > > + } > > > > > > usb_settoggle(urb->dev, qh->epnum, !is_in, csr ? 1 : 0); > > > } > > > > > > +static inline u16 musb_set_toggle(struct musb_qh *qh, int is_in, > > > + struct urb *urb) > > > +{ > > > + u16 csr = 0; > > > + u16 toggle = 0; > > > + struct musb *musb = qh->hw_ep->musb; > > > + u8 epnum = qh->hw_ep->epnum; > > > + > > > + toggle = usb_gettoggle(urb->dev, qh->epnum, !is_in); > > > + > > > + /* MediaTek controller has private toggle register */ > > > + if (musb->ops->quirks & MUSB_MTK_QUIRKS) { > > > + if (is_in) { > > > + musb_writel(musb->mregs, MUSB_RXTOGEN, (1 << epnum)); > > > + musb_writel(musb->mregs, MUSB_RXTOG, (toggle << epnum)); > > > + } else { > > > + musb_writel(musb->mregs, MUSB_TXTOGEN, (1 << epnum)); > > > + musb_writel(musb->mregs, MUSB_TXTOG, (toggle << epnum)); > > > + } > > > + } else { > > > + if (is_in) { > > > + if (toggle) > > > + csr = MUSB_RXCSR_H_WR_DATATOGGLE > > > + | MUSB_RXCSR_H_DATATOGGLE; > > > + else > > > + csr = 0; > > > + } else { > > > + if (toggle) > > > + csr |= MUSB_TXCSR_H_WR_DATATOGGLE > > > + | MUSB_TXCSR_H_DATATOGGLE; > > > + else > > > + csr |= MUSB_TXCSR_CLRDATATOG; '? :' operation probably is better than 'if else' here. > > > + } > > > + } > > > + return csr; > > > > Please create a seperate patch for this musb_set_toggle() without adding > > the mtk logic. It is a nice cleanup. > > Does this like get toggle implementation, create a hook > musb_platform_set_toggle() in struct musb_platform_ops? You did the code cleanup by creating musb_set_toggle(), please make it in a separate patch, like static u16 musb_set_toggle(struct musb_qh *qh, int is_in, struct urb *urb) { toggle = usb_gettoggle(urb->dev, qh->epnum, !is_in); if (is_in) csr = toggle ? (MUSB_RXCSR_H_WR_DATATOGGLE | MUSB_RXCSR_H_DATATOGGLE) : 0; else csr = toggle ? MUSB_TXCSR_H_WR_DATATOGGLE | MUSB_TXCSR_H_DATATOGGLE : MUSB_TXCSR_CLRDATATOG; return csr; } /* use musb_set_toggle() in the two instances */ then in this patch you add the mtk implementation similar as *get_toggle() discussed above. > > > > +} > > > + > > > /* > > > * Advance this hardware endpoint's queue, completing the specified URB and > > > * advancing to either the next URB queued to that qh, or else invalidating > > > @@ -772,13 +825,8 @@ static void musb_ep_program(struct musb *musb, u8 epnum, > > > ); > > > csr |= MUSB_TXCSR_MODE; > > > > > > - if (!hw_ep->tx_double_buffered) { > > > - if (usb_gettoggle(urb->dev, qh->epnum, 1)) > > > - csr |= MUSB_TXCSR_H_WR_DATATOGGLE > > > - | MUSB_TXCSR_H_DATATOGGLE; > > > - else > > > - csr |= MUSB_TXCSR_CLRDATATOG; > > > - } > > > + if (!hw_ep->tx_double_buffered) > > > + csr |= musb_set_toggle(qh, !is_out, urb); > > > > > > musb_writew(epio, MUSB_TXCSR, csr); > > > /* REVISIT may need to clear FLUSHFIFO ... */ > > > @@ -860,17 +908,12 @@ static void musb_ep_program(struct musb *musb, u8 epnum, > > > > > > /* IN/receive */ > > > } else { > > > - u16 csr; > > > + u16 csr = 0; > > > > > > if (hw_ep->rx_reinit) { > > > musb_rx_reinit(musb, qh, epnum); > > > + csr |= musb_set_toggle(qh, !is_out, urb); > > > > > > - /* init new state: toggle and NYET, maybe DMA later */ > > > - if (usb_gettoggle(urb->dev, qh->epnum, 0)) > > > - csr = MUSB_RXCSR_H_WR_DATATOGGLE > > > - | MUSB_RXCSR_H_DATATOGGLE; > > > - else > > > - csr = 0; > > > if (qh->type == USB_ENDPOINT_XFER_INT) > > > csr |= MUSB_RXCSR_DISNYET; > > > > > > diff --git a/drivers/usb/musb/musb_regs.h b/drivers/usb/musb/musb_regs.h > > > index 5cd7264..ffbe267 100644 > > > --- a/drivers/usb/musb/musb_regs.h > > > +++ b/drivers/usb/musb/musb_regs.h > > > @@ -273,6 +273,12 @@ > > > #define MUSB_RXHUBADDR 0x06 > > > #define MUSB_RXHUBPORT 0x07 > > > > > > +/* MediaTek controller toggle enable and status reg */ > > > +#define MUSB_RXTOG 0x80 > > > +#define MUSB_RXTOGEN 0x82 > > > +#define MUSB_TXTOG 0x84 > > > +#define MUSB_TXTOGEN 0x86 > > > > Again, these offsets are for different registers in the MUSB version I > > have, please let me know if you have different version of the MUSB IP. > > Sorry, these are MediaTek controller private registers used for control > toggle. Okay. Once the platform get/set_toggle() are implemented, those registers can be defined in the mtk glue driver instead of here. Regards, -Bin.
Hi Bin, On Wed, 2019-01-09 at 08:01 -0600, Bin Liu wrote: > Hi Min, > > On Wed, Jan 09, 2019 at 08:31:08PM +0800, Min Guo wrote: > > Hi Bin, > > On Tue, 2019-01-08 at 09:44 -0600, Bin Liu wrote: > > > Hi, > > > > > > On Thu, Dec 27, 2018 at 03:34:26PM +0800, min.guo@mediatek.com wrote: > > > > From: Min Guo <min.guo@mediatek.com> > > > > > > > > This adds support for MediaTek musb controller in > > > > host, peripheral and otg mode > > > > > > > > Signed-off-by: Min Guo <min.guo@mediatek.com> > > > > Signed-off-by: Yonglong Wu <yonglong.wu@mediatek.com> > > > > --- > > > > drivers/usb/musb/Kconfig | 8 +- > > > > drivers/usb/musb/Makefile | 1 + > > > > drivers/usb/musb/mediatek.c | 562 +++++++++++++++++++++++++++++++++++++++++++ > > > > drivers/usb/musb/musb_core.c | 10 + > > > > drivers/usb/musb/musb_core.h | 1 + > > > > drivers/usb/musb/musb_dma.h | 1 + > > > > drivers/usb/musb/musb_host.c | 79 ++++-- > > > > drivers/usb/musb/musb_regs.h | 6 + > > > > drivers/usb/musb/musbhsdma.c | 34 ++- > > > > 9 files changed, 671 insertions(+), 31 deletions(-) > > > > create mode 100644 drivers/usb/musb/mediatek.c > > > > > > > > diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig > > > > index ad08895..540fc9f 100644 > > > > --- a/drivers/usb/musb/Kconfig > > > > +++ b/drivers/usb/musb/Kconfig > > > > @@ -115,6 +115,12 @@ config USB_MUSB_JZ4740 > > > > depends on USB_MUSB_GADGET > > > > depends on USB_OTG_BLACKLIST_HUB > > > > > > > > +config USB_MUSB_MEDIATEK > > > > + tristate "MediaTek platforms" > > > > + depends on ARCH_MEDIATEK > > > > > > Please also add '|| COMPILE_TEST' to make testing easier. > > > > Ok > > > > > > + depends on NOP_USB_XCEIV > > > > + depends on GENERIC_PHY > > > > + > > > > config USB_MUSB_AM335X_CHILD > > > > tristate > > > > > > > > @@ -141,7 +147,7 @@ config USB_UX500_DMA > > > > > > > > config USB_INVENTRA_DMA > > > > bool 'Inventra' > > > > - depends on USB_MUSB_OMAP2PLUS > > > > + depends on USB_MUSB_OMAP2PLUS || USB_MUSB_MEDIATEK > > > > help > > > > Enable DMA transfers using Mentor's engine. > > > > > > > > diff --git a/drivers/usb/musb/Makefile b/drivers/usb/musb/Makefile > > > > index 3a88c79..63d82d0 100644 > > > > --- a/drivers/usb/musb/Makefile > > > > +++ b/drivers/usb/musb/Makefile > > > > @@ -24,6 +24,7 @@ obj-$(CONFIG_USB_MUSB_DA8XX) += da8xx.o > > > > obj-$(CONFIG_USB_MUSB_UX500) += ux500.o > > > > obj-$(CONFIG_USB_MUSB_JZ4740) += jz4740.o > > > > obj-$(CONFIG_USB_MUSB_SUNXI) += sunxi.o > > > > +obj-$(CONFIG_USB_MUSB_MEDIATEK) += mediatek.o > > > > > > > > > > > > obj-$(CONFIG_USB_MUSB_AM335X_CHILD) += musb_am335x.o > > > > diff --git a/drivers/usb/musb/mediatek.c b/drivers/usb/musb/mediatek.c > > > > new file mode 100644 > > > > index 0000000..15a6460 > > > > --- /dev/null > > > > +++ b/drivers/usb/musb/mediatek.c > > > > > > [snip] > > > I will review this section later after we sorted out other things. > > > > > > > diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c > > > > index b7d5627..d60f76f 100644 > > > > --- a/drivers/usb/musb/musb_core.c > > > > +++ b/drivers/usb/musb/musb_core.c > > > > @@ -1028,6 +1028,16 @@ static void musb_disable_interrupts(struct musb *musb) > > > > temp = musb_readb(mbase, MUSB_INTRUSB); > > > > temp = musb_readw(mbase, MUSB_INTRTX); > > > > temp = musb_readw(mbase, MUSB_INTRRX); > > > > + > > > > + /* MediaTek controller interrupt status is W1C */ > > > > > > This W1C doesn't match to the MUSB Programming Guide that I have. Those > > > registers are read-only. > > > Is the difference due to the IP intergration in the mtk platforms? or is > > > it a new version of the MUSB controller? If latter, what is the version? > > > > This is difference due to the IP intergration in mtk platforms. W1C is > > easy for CpdeViser debug. > > > > > > + if (musb->ops->quirks & MUSB_MTK_QUIRKS) { > > > > > > Basically we don't want to use this type of platform specific quirks if > > > possible, so let's try to not use it. > > > > I will try my best to avoid using it. > > Thanks. > > > > > > > + musb_writeb(mbase, MUSB_INTRUSB, > > > > + musb_readb(mbase, MUSB_INTRUSB)); > > > > > > For this clearing register bit operation, please create platform hooks > > > musb_clearb() and musb_clearw() in struct musb_platform_ops instead, > > > then follow how musb_readb() pointer is assigned in > > > musb_init_controller() to use the W1C version for mtk platform. > > > > I have tried implementing musb_readb(), musb_readw() interface with > > interrupt status W1C function in struct musb_platform_ops. But this > > interface will require a global variable to hold MAC basic address for > > judgment, and then special handling of the interrupt state. A global > > variable will make the driver work with only a single instance, so it > > can't work on some MTK platforms which have two instances. > > I didn't mean to modify musb_read*(), but > > > How about creating musb_clearb/w() as following: > > void (*clearb)(void __iomem *addr, unsigned offset, u8 data); > > void (*clearw)(void __iomem *addr, unsigned offset, u16 data); > > this is what I was asking for, similar to what musb_readb/w() is > implemented. I will prepare a patch for musb_clearb/w(). > > > > > > > > + musb_writew(mbase, MUSB_INTRRX, > > > > + musb_readw(mbase, MUSB_INTRRX)); > > > > + musb_writew(mbase, MUSB_INTRTX, > > > > + musb_readw(mbase, MUSB_INTRTX)); > > > > + } > > > > } > > > > > > > > static void musb_enable_interrupts(struct musb *musb) > > > > diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h > > > > index 04203b7..1bf4e9a 100644 > > > > --- a/drivers/usb/musb/musb_core.h > > > > +++ b/drivers/usb/musb/musb_core.h > > > > @@ -138,6 +138,7 @@ enum musb_g_ep0_state { > > > > */ > > > > struct musb_platform_ops { > > > > > > > > +#define MUSB_MTK_QUIRKS BIT(10) > > > > #define MUSB_G_NO_SKB_RESERVE BIT(9) > > > > #define MUSB_DA8XX BIT(8) > > > > #define MUSB_PRESERVE_SESSION BIT(7) > > > > diff --git a/drivers/usb/musb/musb_dma.h b/drivers/usb/musb/musb_dma.h > > > > index 281e75d3..b218210 100644 > > > > --- a/drivers/usb/musb/musb_dma.h > > > > +++ b/drivers/usb/musb/musb_dma.h > > > > @@ -197,6 +197,7 @@ static inline void musb_dma_controller_destroy(struct dma_controller *d) { } > > > > extern struct dma_controller * > > > > musbhs_dma_controller_create(struct musb *musb, void __iomem *base); > > > > extern void musbhs_dma_controller_destroy(struct dma_controller *c); > > > > +extern irqreturn_t dma_controller_irq(int irq, void *private_data); > > > > > > > > extern struct dma_controller * > > > > tusb_dma_controller_create(struct musb *musb, void __iomem *base); > > > > diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c > > > > index b59ce9a..b1b0216 100644 > > > > --- a/drivers/usb/musb/musb_host.c > > > > +++ b/drivers/usb/musb/musb_host.c > > > > @@ -292,20 +292,73 @@ static inline void musb_save_toggle(struct musb_qh *qh, int is_in, > > > > { > > > > void __iomem *epio = qh->hw_ep->regs; > > > > u16 csr; > > > > + struct musb *musb = qh->hw_ep->musb; > > > > > > > > /* > > > > * FIXME: the current Mentor DMA code seems to have > > > > * problems getting toggle correct. > > > > */ > > > > > > > > - if (is_in) > > > > - csr = musb_readw(epio, MUSB_RXCSR) & MUSB_RXCSR_H_DATATOGGLE; > > > > - else > > > > - csr = musb_readw(epio, MUSB_TXCSR) & MUSB_TXCSR_H_DATATOGGLE; > > > > + /* MediaTek controller has private toggle register */ > > > > > > only one toggle register for all endpoints? how does it handle > > > difference toggle values for different endpoints? > > > > MediaTek controller has separate registers to describe TX/RX toggle. > > Is it one register per endpoint? MUSB_RXTOG/MUSB_TXTOG is common register, each bit reflects the toggle state of an endpoint. bit[0] not used,bit[1~8] corresponds to ep[1~8] > > > > > > + if (musb->ops->quirks & MUSB_MTK_QUIRKS) { > > > > + u16 toggle; > > > > + u8 epnum = qh->hw_ep->epnum; > > > > + > > > > + if (is_in) > > > > + toggle = musb_readl(musb->mregs, MUSB_RXTOG); > > this line seems telling there is just *one* register for all endpoints. Yes, all endpoint share this register, endpoint and bit are one-to-one correspondence. > > > > > > should use musb_readw() instead? MUSB_RXTOG seems to be 16bit. > > > > Ok > > > > > > + else > > > > + toggle = musb_readl(musb->mregs, MUSB_TXTOG); > > > > + > > > > + csr = toggle & (1 << epnum); > > > > + } else { > > > > + if (is_in) > > > > + csr = musb_readw(epio, MUSB_RXCSR) > > > > + & MUSB_RXCSR_H_DATATOGGLE; > > > > + else > > > > + csr = musb_readw(epio, MUSB_TXCSR) > > > > + & MUSB_TXCSR_H_DATATOGGLE; > > > > > > Does this logic still work for the mtk platform even if it has its own > > > private toggle register? If so, we don't need to change here. > > > > Sorry, this logic can not work on mtk platform, bit > > MUSB_RXCSR_H_DATATOGGLE and MUSB_TXCSR_H_DATATOGGLE are used for other > > function. > > Is there a different controller RTL version we can use to > differentiate? Sorry, there is no controller RTL version can be used to indicate the differences. > > > > > If not, let's try to not use this quirk flag. Please create a hook > > > musb_platform_get_toggle() in struct musb_platform_ops. > > > > Does the method of implement musb_platform_get_toggle() is prepare > > musb_default_get_toggle with common function, then follow how > > musb_readb() pointer is assigned in musb_init_controller()? > > Yes, similar to musb_readb() implementation. OK. > > How about creating musb_platform_get_toggle() as following: > > u16 (*get_toggle)(struct musb* musb, struct musb_qh *qh, int is_in); > > yes, it is part of the implementation, then add it in struct musb_io. I will prepare a patch for (*get_toggle). > > > > > > + } > > > > > > > > usb_settoggle(urb->dev, qh->epnum, !is_in, csr ? 1 : 0); > > > > } > > > > > > > > +static inline u16 musb_set_toggle(struct musb_qh *qh, int is_in, > > > > + struct urb *urb) > > > > +{ > > > > + u16 csr = 0; > > > > + u16 toggle = 0; > > > > + struct musb *musb = qh->hw_ep->musb; > > > > + u8 epnum = qh->hw_ep->epnum; > > > > + > > > > + toggle = usb_gettoggle(urb->dev, qh->epnum, !is_in); > > > > + > > > > + /* MediaTek controller has private toggle register */ > > > > + if (musb->ops->quirks & MUSB_MTK_QUIRKS) { > > > > + if (is_in) { > > > > + musb_writel(musb->mregs, MUSB_RXTOGEN, (1 << epnum)); > > > > + musb_writel(musb->mregs, MUSB_RXTOG, (toggle << epnum)); > > > > + } else { > > > > + musb_writel(musb->mregs, MUSB_TXTOGEN, (1 << epnum)); > > > > + musb_writel(musb->mregs, MUSB_TXTOG, (toggle << epnum)); > > > > + } > > > > + } else { > > > > + if (is_in) { > > > > + if (toggle) > > > > + csr = MUSB_RXCSR_H_WR_DATATOGGLE > > > > + | MUSB_RXCSR_H_DATATOGGLE; > > > > + else > > > > + csr = 0; > > > > + } else { > > > > + if (toggle) > > > > + csr |= MUSB_TXCSR_H_WR_DATATOGGLE > > > > + | MUSB_TXCSR_H_DATATOGGLE; > > > > + else > > > > + csr |= MUSB_TXCSR_CLRDATATOG; > > '? :' operation probably is better than 'if else' here. OK. > > > > + } > > > > + } > > > > + return csr; > > > > > > Please create a seperate patch for this musb_set_toggle() without adding > > > the mtk logic. It is a nice cleanup. > > > > Does this like get toggle implementation, create a hook > > musb_platform_set_toggle() in struct musb_platform_ops? > > You did the code cleanup by creating musb_set_toggle(), please make it > in a separate patch, like > > static u16 musb_set_toggle(struct musb_qh *qh, int is_in, struct urb *urb) > { > toggle = usb_gettoggle(urb->dev, qh->epnum, !is_in); > if (is_in) > csr = toggle ? > (MUSB_RXCSR_H_WR_DATATOGGLE | MUSB_RXCSR_H_DATATOGGLE) : > 0; > else > csr = toggle ? > MUSB_TXCSR_H_WR_DATATOGGLE | MUSB_TXCSR_H_DATATOGGLE : > MUSB_TXCSR_CLRDATATOG; > return csr; > } > > /* use musb_set_toggle() in the two instances */ > > then in this patch you add the mtk implementation similar as > *get_toggle() discussed above. OK. > > > > > > +} > > > > + > > > > /* > > > > * Advance this hardware endpoint's queue, completing the specified URB and > > > > * advancing to either the next URB queued to that qh, or else invalidating > > > > @@ -772,13 +825,8 @@ static void musb_ep_program(struct musb *musb, u8 epnum, > > > > ); > > > > csr |= MUSB_TXCSR_MODE; > > > > > > > > - if (!hw_ep->tx_double_buffered) { > > > > - if (usb_gettoggle(urb->dev, qh->epnum, 1)) > > > > - csr |= MUSB_TXCSR_H_WR_DATATOGGLE > > > > - | MUSB_TXCSR_H_DATATOGGLE; > > > > - else > > > > - csr |= MUSB_TXCSR_CLRDATATOG; > > > > - } > > > > + if (!hw_ep->tx_double_buffered) > > > > + csr |= musb_set_toggle(qh, !is_out, urb); > > > > > > > > musb_writew(epio, MUSB_TXCSR, csr); > > > > /* REVISIT may need to clear FLUSHFIFO ... */ > > > > @@ -860,17 +908,12 @@ static void musb_ep_program(struct musb *musb, u8 epnum, > > > > > > > > /* IN/receive */ > > > > } else { > > > > - u16 csr; > > > > + u16 csr = 0; > > > > > > > > if (hw_ep->rx_reinit) { > > > > musb_rx_reinit(musb, qh, epnum); > > > > + csr |= musb_set_toggle(qh, !is_out, urb); > > > > > > > > - /* init new state: toggle and NYET, maybe DMA later */ > > > > - if (usb_gettoggle(urb->dev, qh->epnum, 0)) > > > > - csr = MUSB_RXCSR_H_WR_DATATOGGLE > > > > - | MUSB_RXCSR_H_DATATOGGLE; > > > > - else > > > > - csr = 0; > > > > if (qh->type == USB_ENDPOINT_XFER_INT) > > > > csr |= MUSB_RXCSR_DISNYET; > > > > > > > > diff --git a/drivers/usb/musb/musb_regs.h b/drivers/usb/musb/musb_regs.h > > > > index 5cd7264..ffbe267 100644 > > > > --- a/drivers/usb/musb/musb_regs.h > > > > +++ b/drivers/usb/musb/musb_regs.h > > > > @@ -273,6 +273,12 @@ > > > > #define MUSB_RXHUBADDR 0x06 > > > > #define MUSB_RXHUBPORT 0x07 > > > > > > > > +/* MediaTek controller toggle enable and status reg */ > > > > +#define MUSB_RXTOG 0x80 > > > > +#define MUSB_RXTOGEN 0x82 > > > > +#define MUSB_TXTOG 0x84 > > > > +#define MUSB_TXTOGEN 0x86 > > > > > > Again, these offsets are for different registers in the MUSB version I > > > have, please let me know if you have different version of the MUSB IP. > > > > Sorry, these are MediaTek controller private registers used for control > > toggle. > > Okay. Once the platform get/set_toggle() are implemented, those > registers can be defined in the mtk glue driver instead of here. OK. > Regards, > -Bin.
Hi Min, Please briefly summarize the controller differences in the commit log, such as - WIC interrupt registers; - data toggle bit; - no dedicated DMA interrupt line; so that we can quickly understand the core driver is modified accordingly to handle the differences. On Thu, Jan 10, 2019 at 03:24:22PM +0800, Min Guo wrote: > Hi Bin, [snip] > > > > > + musb_writeb(mbase, MUSB_INTRUSB, > > > > > + musb_readb(mbase, MUSB_INTRUSB)); > > > > > > > > For this clearing register bit operation, please create platform hooks > > > > musb_clearb() and musb_clearw() in struct musb_platform_ops instead, > > > > then follow how musb_readb() pointer is assigned in > > > > musb_init_controller() to use the W1C version for mtk platform. > > > > > > I have tried implementing musb_readb(), musb_readw() interface with > > > interrupt status W1C function in struct musb_platform_ops. But this > > > interface will require a global variable to hold MAC basic address for > > > judgment, and then special handling of the interrupt state. A global > > > variable will make the driver work with only a single instance, so it > > > can't work on some MTK platforms which have two instances. > > > > I didn't mean to modify musb_read*(), but > > > > > How about creating musb_clearb/w() as following: > > > void (*clearb)(void __iomem *addr, unsigned offset, u8 data); > > > void (*clearw)(void __iomem *addr, unsigned offset, u16 data); > > > > this is what I was asking for, similar to what musb_readb/w() is > > implemented. > > I will prepare a patch for musb_clearb/w(). This doesn't have to be a separate patch. > > > > > + musb_writew(mbase, MUSB_INTRRX, > > > > > + musb_readw(mbase, MUSB_INTRRX)); > > > > > + musb_writew(mbase, MUSB_INTRTX, > > > > > + musb_readw(mbase, MUSB_INTRTX)); > > > > > + } [snip] > > > > > + /* MediaTek controller has private toggle register */ > > > > > > > > only one toggle register for all endpoints? how does it handle > > > > difference toggle values for different endpoints? > > > > > > MediaTek controller has separate registers to describe TX/RX toggle. > > > > Is it one register per endpoint? > > MUSB_RXTOG/MUSB_TXTOG is common register, each bit reflects the toggle > state of an endpoint. bit[0] not used,bit[1~8] corresponds to ep[1~8] > > > > > > > > > + if (musb->ops->quirks & MUSB_MTK_QUIRKS) { > > > > > + u16 toggle; > > > > > + u8 epnum = qh->hw_ep->epnum; > > > > > + > > > > > + if (is_in) > > > > > + toggle = musb_readl(musb->mregs, MUSB_RXTOG); > > > > this line seems telling there is just *one* register for all endpoints. > > Yes, all endpoint share this register, endpoint and bit are one-to-one > correspondence. Okay, thanks. Sorry I missed the bit operation in the code below. > > > > > > > > > should use musb_readw() instead? MUSB_RXTOG seems to be 16bit. > > > > > > Ok > > > > > > > > + else > > > > > + toggle = musb_readl(musb->mregs, MUSB_TXTOG); > > > > > + > > > > > + csr = toggle & (1 << epnum); Regards, -Bin.
Hi Bin, On Thu, 2019-01-10 at 08:18 -0600, Bin Liu wrote: > Hi Min, > > Please briefly summarize the controller differences in the commit log, > such as > > - WIC interrupt registers; > - data toggle bit; > - no dedicated DMA interrupt line; > > so that we can quickly understand the core driver is modified > accordingly to handle the differences. Okay, tkanks. > On Thu, Jan 10, 2019 at 03:24:22PM +0800, Min Guo wrote: > > Hi Bin, > > [snip] > > > > > > > + musb_writeb(mbase, MUSB_INTRUSB, > > > > > > + musb_readb(mbase, MUSB_INTRUSB)); > > > > > > > > > > For this clearing register bit operation, please create platform hooks > > > > > musb_clearb() and musb_clearw() in struct musb_platform_ops instead, > > > > > then follow how musb_readb() pointer is assigned in > > > > > musb_init_controller() to use the W1C version for mtk platform. > > > > > > > > I have tried implementing musb_readb(), musb_readw() interface with > > > > interrupt status W1C function in struct musb_platform_ops. But this > > > > interface will require a global variable to hold MAC basic address for > > > > judgment, and then special handling of the interrupt state. A global > > > > variable will make the driver work with only a single instance, so it > > > > can't work on some MTK platforms which have two instances. > > > > > > I didn't mean to modify musb_read*(), but > > > > > > > How about creating musb_clearb/w() as following: > > > > void (*clearb)(void __iomem *addr, unsigned offset, u8 data); > > > > void (*clearw)(void __iomem *addr, unsigned offset, u16 data); > > > > > > this is what I was asking for, similar to what musb_readb/w() is > > > implemented. > > > > I will prepare a patch for musb_clearb/w(). > > This doesn't have to be a separate patch. Okay. > > > > > > + musb_writew(mbase, MUSB_INTRRX, > > > > > > + musb_readw(mbase, MUSB_INTRRX)); > > > > > > + musb_writew(mbase, MUSB_INTRTX, > > > > > > + musb_readw(mbase, MUSB_INTRTX)); > > > > > > + } > > [snip] > > > > > > > + /* MediaTek controller has private toggle register */ > > > > > > > > > > only one toggle register for all endpoints? how does it handle > > > > > difference toggle values for different endpoints? > > > > > > > > MediaTek controller has separate registers to describe TX/RX toggle. > > > > > > Is it one register per endpoint? > > > > MUSB_RXTOG/MUSB_TXTOG is common register, each bit reflects the toggle > > state of an endpoint. bit[0] not used,bit[1~8] corresponds to ep[1~8] > > > > > > > > > > > > + if (musb->ops->quirks & MUSB_MTK_QUIRKS) { > > > > > > + u16 toggle; > > > > > > + u8 epnum = qh->hw_ep->epnum; > > > > > > + > > > > > > + if (is_in) > > > > > > + toggle = musb_readl(musb->mregs, MUSB_RXTOG); > > > > > > this line seems telling there is just *one* register for all endpoints. > > > > Yes, all endpoint share this register, endpoint and bit are one-to-one > > correspondence. > > Okay, thanks. Sorry I missed the bit operation in the code below. > > > > > > > > > > > > > should use musb_readw() instead? MUSB_RXTOG seems to be 16bit. > > > > > > > > Ok > > > > > > > > > > + else > > > > > > + toggle = musb_readl(musb->mregs, MUSB_TXTOG); > > > > > > + > > > > > > + csr = toggle & (1 << epnum); > > Regards, > -Bin.
Hi Bin, I have some questions about musbhs_dma_controller_destroy/create_noirq(). 1,Because of controller->irq=0 in noirq case, destroy_noirq can reuse musbhs_dma_controller_destroy. Is it necessary to write a destroy_noirq function? 2, How about creating a common function for create musb dma controller as following: static struct musb_dma_controller *dma_controller_alloc(struct musb *musb, void __iomem *base) then musbhs_dma_controller_create() and musbhs_dma_controller_create_noirq() call it to alloc common resource. On Tue, 2019-01-08 at 09:44 -0600, Bin Liu wrote: > Hi, > > On Thu, Dec 27, 2018 at 03:34:26PM +0800, min.guo@mediatek.com wrote: > > From: Min Guo <min.guo@mediatek.com> > > > > This adds support for MediaTek musb controller in > > host, peripheral and otg mode > > > > diff --git a/drivers/usb/musb/musbhsdma.c b/drivers/usb/musb/musbhsdma.c > > index 824adcb..c545475 100644 > > --- a/drivers/usb/musb/musbhsdma.c > > +++ b/drivers/usb/musb/musbhsdma.c > > > > void musbhs_dma_controller_destroy(struct dma_controller *c) > > { > > struct musb_dma_controller *controller = container_of(c, > > struct musb_dma_controller, controller); > > + struct musb *musb = controller->private_data; > > > > dma_controller_stop(controller); > > > > - if (controller->irq) > > + if (!(musb->ops->quirks & MUSB_MTK_QUIRKS) && controller->irq) > > free_irq(controller->irq, c); > > > > kfree(controller); > > @@ -398,11 +402,15 @@ struct dma_controller *musbhs_dma_controller_create(struct musb *musb, > > struct musb_dma_controller *controller; > > struct device *dev = musb->controller; > > struct platform_device *pdev = to_platform_device(dev); > > - int irq = platform_get_irq_byname(pdev, "dma"); > > + int irq = -1; > > > > - if (irq <= 0) { > > - dev_err(dev, "No DMA interrupt line!\n"); > > - return NULL; > > + if (!(musb->ops->quirks & MUSB_MTK_QUIRKS)) { > > + irq = platform_get_irq_byname(pdev, "dma"); > > + > > + if (irq < 0) { > > + dev_err(dev, "No DMA interrupt line!\n"); > > + return NULL; > > + } > > } > > Please create musbhs_dma_controller_destroy_noirq() for your platform to > not use the quirk. > > > > > controller = kzalloc(sizeof(*controller), GFP_KERNEL); > > @@ -418,15 +426,17 @@ struct dma_controller *musbhs_dma_controller_create(struct musb *musb, > > controller->controller.channel_program = dma_channel_program; > > controller->controller.channel_abort = dma_channel_abort; > > > > - if (request_irq(irq, dma_controller_irq, 0, > > + if (!(musb->ops->quirks & MUSB_MTK_QUIRKS)) { > > + if (request_irq(irq, dma_controller_irq, 0, > > dev_name(musb->controller), &controller->controller)) { > > - dev_err(dev, "request_irq %d failed!\n", irq); > > - musb_dma_controller_destroy(&controller->controller); > > + dev_err(dev, "request_irq %d failed!\n", irq); > > + musb_dma_controller_destroy(&controller->controller); > > > > - return NULL; > > - } > > + return NULL; > > + } > > > > - controller->irq = irq; > > + controller->irq = irq; > > + } > > > > return &controller->controller; > > } > > Same here, create musbhs_dma_controller_create_noirq(). Then use both > new API for the mtk glue driver. > Regards, > -Bin.
diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig index ad08895..540fc9f 100644 --- a/drivers/usb/musb/Kconfig +++ b/drivers/usb/musb/Kconfig @@ -115,6 +115,12 @@ config USB_MUSB_JZ4740 depends on USB_MUSB_GADGET depends on USB_OTG_BLACKLIST_HUB +config USB_MUSB_MEDIATEK + tristate "MediaTek platforms" + depends on ARCH_MEDIATEK + depends on NOP_USB_XCEIV + depends on GENERIC_PHY + config USB_MUSB_AM335X_CHILD tristate @@ -141,7 +147,7 @@ config USB_UX500_DMA config USB_INVENTRA_DMA bool 'Inventra' - depends on USB_MUSB_OMAP2PLUS + depends on USB_MUSB_OMAP2PLUS || USB_MUSB_MEDIATEK help Enable DMA transfers using Mentor's engine. diff --git a/drivers/usb/musb/Makefile b/drivers/usb/musb/Makefile index 3a88c79..63d82d0 100644 --- a/drivers/usb/musb/Makefile +++ b/drivers/usb/musb/Makefile @@ -24,6 +24,7 @@ obj-$(CONFIG_USB_MUSB_DA8XX) += da8xx.o obj-$(CONFIG_USB_MUSB_UX500) += ux500.o obj-$(CONFIG_USB_MUSB_JZ4740) += jz4740.o obj-$(CONFIG_USB_MUSB_SUNXI) += sunxi.o +obj-$(CONFIG_USB_MUSB_MEDIATEK) += mediatek.o obj-$(CONFIG_USB_MUSB_AM335X_CHILD) += musb_am335x.o diff --git a/drivers/usb/musb/mediatek.c b/drivers/usb/musb/mediatek.c new file mode 100644 index 0000000..15a6460 --- /dev/null +++ b/drivers/usb/musb/mediatek.c @@ -0,0 +1,562 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2018 MediaTek Inc. + * + * Author: + * Min Guo <min.guo@mediatek.com> + * Yonglong Wu <yonglong.wu@mediatek.com> + */ + +#include <linux/clk.h> +#include <linux/dma-mapping.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/usb/usb_phy_generic.h> +#include "musb_core.h" +#include "musb_dma.h" + +#define USB_L1INTS 0x00a0 +#define USB_L1INTM 0x00a4 +#define MTK_MUSB_TXFUNCADDR 0x0480 + +#define TX_INT_STATUS BIT(0) +#define RX_INT_STATUS BIT(1) +#define USBCOM_INT_STATUS BIT(2) +#define DMA_INT_STATUS BIT(3) + +#define DMA_INTR_STATUS_MSK GENMASK(7, 0) +#define DMA_INTR_UNMASK_SET_MSK GENMASK(31, 24) + +enum mtk_vbus_id_state { + MTK_ID_FLOAT = 1, + MTK_ID_GROUND, + MTK_VBUS_OFF, + MTK_VBUS_VALID, +}; + +struct mtk_glue { + struct device *dev; + struct musb *musb; + struct platform_device *musb_pdev; + struct platform_device *usb_phy; + struct phy *phy; + struct usb_phy *xceiv; + enum phy_mode phy_mode; + struct clk *main; + struct clk *mcu; + struct clk *univpll; + struct regulator *vbus; + struct extcon_dev *edev; + struct notifier_block vbus_nb; + struct notifier_block id_nb; +}; + +static int mtk_musb_clks_get(struct mtk_glue *glue) +{ + struct device *dev = glue->dev; + + glue->main = devm_clk_get(dev, "main"); + if (IS_ERR(glue->main)) { + dev_err(dev, "fail to get main clock\n"); + return PTR_ERR(glue->main); + } + + glue->mcu = devm_clk_get(dev, "mcu"); + if (IS_ERR(glue->mcu)) { + dev_err(dev, "fail to get mcu clock\n"); + return PTR_ERR(glue->mcu); + } + + glue->univpll = devm_clk_get(dev, "univpll"); + if (IS_ERR(glue->univpll)) { + dev_err(dev, "fail to get univpll clock\n"); + return PTR_ERR(glue->univpll); + } + + return 0; +} + +static int mtk_musb_clks_enable(struct mtk_glue *glue) +{ + int ret; + + ret = clk_prepare_enable(glue->main); + if (ret) { + dev_err(glue->dev, "failed to enable main clock\n"); + goto err_main_clk; + } + + ret = clk_prepare_enable(glue->mcu); + if (ret) { + dev_err(glue->dev, "failed to enable mcu clock\n"); + goto err_mcu_clk; + } + + ret = clk_prepare_enable(glue->univpll); + if (ret) { + dev_err(glue->dev, "failed to enable univpll clock\n"); + goto err_univpll_clk; + } + + return 0; + +err_univpll_clk: + clk_disable_unprepare(glue->mcu); +err_mcu_clk: + clk_disable_unprepare(glue->main); +err_main_clk: + return ret; +} + +static void mtk_musb_clks_disable(struct mtk_glue *glue) +{ + clk_disable_unprepare(glue->univpll); + clk_disable_unprepare(glue->mcu); + clk_disable_unprepare(glue->main); +} + +static void mtk_musb_set_vbus(struct musb *musb, int is_on) +{ + struct device *dev = musb->controller; + struct mtk_glue *glue = dev_get_drvdata(dev->parent); + int ret; + + /* vbus is optional */ + if (!glue->vbus) + return; + + dev_dbg(musb->controller, "%s, is_on=%d\r\n", __func__, is_on); + if (is_on) { + ret = regulator_enable(glue->vbus); + if (ret) { + dev_err(glue->dev, "fail to enable vbus regulator\n"); + return; + } + } else { + regulator_disable(glue->vbus); + } +} + +/* + * switch to host: -> MTK_VBUS_OFF --> MTK_ID_GROUND + * switch to device: -> MTK_ID_FLOAT --> MTK_VBUS_VALID + */ +static void mtk_musb_set_mailbox(struct mtk_glue *glue, + enum mtk_vbus_id_state status) +{ + struct musb *musb = glue->musb; + u8 devctl = 0; + + dev_dbg(glue->dev, "mailbox state(%d)\n", status); + switch (status) { + case MTK_ID_GROUND: + phy_power_on(glue->phy); + devctl = readb(musb->mregs + MUSB_DEVCTL); + musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE; + mtk_musb_set_vbus(musb, 1); + glue->phy_mode = PHY_MODE_USB_HOST; + phy_set_mode(glue->phy, glue->phy_mode); + devctl |= MUSB_DEVCTL_SESSION; + musb_writeb(musb->mregs, MUSB_DEVCTL, devctl); + MUSB_HST_MODE(musb); + break; + /* + * MTK_ID_FLOAT process is the same as MTK_VBUS_VALID + * except that turn off VBUS + */ + case MTK_ID_FLOAT: + mtk_musb_set_vbus(musb, 0); + /* fall through */ + case MTK_VBUS_OFF: + musb->xceiv->otg->state = OTG_STATE_B_IDLE; + devctl &= ~MUSB_DEVCTL_SESSION; + musb_writeb(musb->mregs, MUSB_DEVCTL, devctl); + phy_power_off(glue->phy); + break; + case MTK_VBUS_VALID: + phy_power_on(glue->phy); + glue->phy_mode = PHY_MODE_USB_DEVICE; + phy_set_mode(glue->phy, glue->phy_mode); + MUSB_DEV_MODE(musb); + break; + default: + dev_err(glue->dev, "invalid state\n"); + } +} + +static int mtk_musb_id_notifier(struct notifier_block *nb, + unsigned long event, void *ptr) +{ + struct mtk_glue *glue = container_of(nb, struct mtk_glue, id_nb); + + if (event) + mtk_musb_set_mailbox(glue, MTK_ID_GROUND); + else + mtk_musb_set_mailbox(glue, MTK_ID_FLOAT); + + return NOTIFY_DONE; +} + +static int mtk_musb_vbus_notifier(struct notifier_block *nb, + unsigned long event, void *ptr) +{ + struct mtk_glue *glue = container_of(nb, struct mtk_glue, vbus_nb); + + if (event) + mtk_musb_set_mailbox(glue, MTK_VBUS_VALID); + else + mtk_musb_set_mailbox(glue, MTK_VBUS_OFF); + + return NOTIFY_DONE; +} + +static void mtk_otg_switch_init(struct mtk_glue *glue) +{ + int ret; + + /* extcon is optional */ + if (!glue->edev) + return; + + glue->vbus_nb.notifier_call = mtk_musb_vbus_notifier; + ret = devm_extcon_register_notifier(glue->dev, glue->edev, EXTCON_USB, + &glue->vbus_nb); + if (ret < 0) + dev_err(glue->dev, "failed to register notifier for USB\n"); + + glue->id_nb.notifier_call = mtk_musb_id_notifier; + ret = devm_extcon_register_notifier(glue->dev, glue->edev, + EXTCON_USB_HOST, &glue->id_nb); + if (ret < 0) + dev_err(glue->dev, "failed to register notifier for USB-HOST\n"); + + dev_dbg(glue->dev, "EXTCON_USB: %d, EXTCON_USB_HOST: %d\n", + extcon_get_state(glue->edev, EXTCON_USB), + extcon_get_state(glue->edev, EXTCON_USB_HOST)); + + /* default as host, switch to device mode if needed */ + if (extcon_get_state(glue->edev, EXTCON_USB_HOST) == false) + mtk_musb_set_mailbox(glue, MTK_ID_FLOAT); + if (extcon_get_state(glue->edev, EXTCON_USB) == true) + mtk_musb_set_mailbox(glue, MTK_VBUS_VALID); +} + +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); + /* MediaTek controller interrupt status is W1C */ + musb_writew(musb->mregs, MUSB_INTRRX, musb->int_rx); + musb_writew(musb->mregs, MUSB_INTRTX, musb->int_tx); + musb_writeb(musb->mregs, MUSB_INTRUSB, musb->int_usb); + + if (musb->int_usb || musb->int_tx || musb->int_rx) + retval = musb_interrupt(musb); + + spin_unlock_irqrestore(&musb->lock, flags); + + return retval; +} + +static irqreturn_t mtk_musb_interrupt(int irq, void *dev_id) +{ + irqreturn_t retval = IRQ_NONE; + struct musb *musb = (struct musb *)dev_id; + u32 l1_ints; + + l1_ints = musb_readl(musb->mregs, USB_L1INTS) & + musb_readl(musb->mregs, USB_L1INTM); + + if (l1_ints & (TX_INT_STATUS | RX_INT_STATUS | USBCOM_INT_STATUS)) + retval = generic_interrupt(irq, musb); + +#if defined(CONFIG_USB_INVENTRA_DMA) + if (l1_ints & DMA_INT_STATUS) + retval = dma_controller_irq(irq, musb->dma_controller); +#endif + return retval; +} + +static u32 mtk_musb_busctl_offset(u8 epnum, u16 offset) +{ + return MTK_MUSB_TXFUNCADDR + offset + 8 * epnum; +} + +static int mtk_musb_init(struct musb *musb) +{ + struct device *dev = musb->controller; + struct mtk_glue *glue = dev_get_drvdata(dev->parent); + int ret; + + glue->musb = musb; + musb->phy = glue->phy; + musb->xceiv = glue->xceiv; + musb->is_host = false; + musb->isr = mtk_musb_interrupt; + ret = phy_init(glue->phy); + if (ret) + return ret; + + ret = phy_power_on(glue->phy); + if (ret) { + phy_exit(glue->phy); + return ret; + } + + phy_set_mode(glue->phy, glue->phy_mode); + +#if defined(CONFIG_USB_INVENTRA_DMA) + musb_writel(musb->mregs, MUSB_HSDMA_INTR, + DMA_INTR_STATUS_MSK | DMA_INTR_UNMASK_SET_MSK); +#endif + musb_writel(musb->mregs, USB_L1INTM, TX_INT_STATUS | RX_INT_STATUS | + USBCOM_INT_STATUS | DMA_INT_STATUS); + return 0; +} + +static int mtk_musb_set_mode(struct musb *musb, u8 mode) +{ + struct device *dev = musb->controller; + struct mtk_glue *glue = dev_get_drvdata(dev->parent); + enum phy_mode new_mode; + + switch (mode) { + case MUSB_HOST: + new_mode = PHY_MODE_USB_HOST; + mtk_musb_set_vbus(musb, 1); + break; + case MUSB_PERIPHERAL: + new_mode = PHY_MODE_USB_DEVICE; + break; + case MUSB_OTG: + new_mode = PHY_MODE_USB_HOST; + break; + default: + dev_err(musb->controller->parent, + "Error requested mode not supported by this kernel\n"); + return -EINVAL; + } + if (glue->phy_mode == new_mode) + return 0; + + mtk_musb_set_mailbox(glue, MTK_ID_GROUND); + return 0; +} + +static int mtk_musb_exit(struct musb *musb) +{ + struct device *dev = musb->controller; + struct mtk_glue *glue = dev_get_drvdata(dev->parent); + + phy_power_off(glue->phy); + phy_exit(glue->phy); + mtk_musb_clks_disable(glue); + + pm_runtime_put_sync(dev); + pm_runtime_disable(dev); + return 0; +} + +static const struct musb_platform_ops mtk_musb_ops = { + .quirks = MUSB_DMA_INVENTRA | MUSB_MTK_QUIRKS, + .init = mtk_musb_init, + .exit = mtk_musb_exit, +#ifdef CONFIG_USB_INVENTRA_DMA + .dma_init = musbhs_dma_controller_create, + .dma_exit = musbhs_dma_controller_destroy, +#endif + .busctl_offset = mtk_musb_busctl_offset, + .set_mode = mtk_musb_set_mode, + .set_vbus = mtk_musb_set_vbus, +}; + +#define MTK_MUSB_MAX_EP_NUM 8 +#define MTK_MUSB_RAM_BITS 11 + +static struct musb_fifo_cfg mtk_musb_mode_cfg[] = { + { .hw_ep_num = 1, .style = FIFO_TX, .maxpacket = 512, }, + { .hw_ep_num = 1, .style = FIFO_RX, .maxpacket = 512, }, + { .hw_ep_num = 2, .style = FIFO_TX, .maxpacket = 512, }, + { .hw_ep_num = 2, .style = FIFO_RX, .maxpacket = 512, }, + { .hw_ep_num = 3, .style = FIFO_TX, .maxpacket = 512, }, + { .hw_ep_num = 3, .style = FIFO_RX, .maxpacket = 512, }, + { .hw_ep_num = 4, .style = FIFO_TX, .maxpacket = 512, }, + { .hw_ep_num = 4, .style = FIFO_RX, .maxpacket = 512, }, + { .hw_ep_num = 5, .style = FIFO_TX, .maxpacket = 512, }, + { .hw_ep_num = 5, .style = FIFO_RX, .maxpacket = 512, }, + { .hw_ep_num = 6, .style = FIFO_TX, .maxpacket = 1024, }, + { .hw_ep_num = 6, .style = FIFO_RX, .maxpacket = 1024, }, + { .hw_ep_num = 7, .style = FIFO_TX, .maxpacket = 512, }, + { .hw_ep_num = 7, .style = FIFO_RX, .maxpacket = 64, }, +}; + +static const struct musb_hdrc_config mtk_musb_hdrc_config = { + .fifo_cfg = mtk_musb_mode_cfg, + .fifo_cfg_size = ARRAY_SIZE(mtk_musb_mode_cfg), + .multipoint = true, + .dyn_fifo = true, + .num_eps = MTK_MUSB_MAX_EP_NUM, + .ram_bits = MTK_MUSB_RAM_BITS, +}; + +static const struct platform_device_info mtk_dev_info = { + .name = "musb-hdrc", + .id = PLATFORM_DEVID_AUTO, + .dma_mask = DMA_BIT_MASK(32), +}; + +static int mtk_musb_probe(struct platform_device *pdev) +{ + struct musb_hdrc_platform_data *pdata; + struct mtk_glue *glue; + struct platform_device_info pinfo; + struct device *dev = &pdev->dev; + struct device_node *np = dev->of_node; + int ret = -ENOMEM; + + glue = devm_kzalloc(dev, sizeof(*glue), GFP_KERNEL); + if (!glue) + return -ENOMEM; + + glue->dev = dev; + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); + if (!pdata) + return -ENOMEM; + + ret = mtk_musb_clks_get(glue); + if (ret) + return ret; + + glue->vbus = devm_regulator_get(dev, "vbus"); + if (IS_ERR(glue->vbus)) { + dev_err(dev, "fail to get vbus\n"); + return PTR_ERR(glue->vbus); + } + + pdata->config = &mtk_musb_hdrc_config; + pdata->platform_ops = &mtk_musb_ops; + if (of_property_read_bool(np, "extcon")) { + glue->edev = extcon_get_edev_by_phandle(dev, 0); + if (IS_ERR(glue->edev)) { + dev_err(dev, "fail to get extcon\n"); + return PTR_ERR(glue->edev); + } + } + + pdata->mode = usb_get_dr_mode(dev); + switch (pdata->mode) { + case USB_DR_MODE_HOST: + glue->phy_mode = PHY_MODE_USB_HOST; + break; + case USB_DR_MODE_PERIPHERAL: + glue->phy_mode = PHY_MODE_USB_DEVICE; + break; + default: + pdata->mode = USB_DR_MODE_OTG; + /* FALL THROUGH */ + case USB_DR_MODE_OTG: + glue->phy_mode = PHY_MODE_USB_OTG; + break; + } + + glue->phy = devm_phy_get(dev, "usb2-phy"); + if (IS_ERR(glue->phy)) { + dev_err(dev, "fail to getting phy %ld\n", + PTR_ERR(glue->phy)); + return PTR_ERR(glue->phy); + } + + glue->usb_phy = usb_phy_generic_register(); + if (IS_ERR(glue->usb_phy)) { + dev_err(dev, "fail to registering usb-phy %ld\n", + PTR_ERR(glue->usb_phy)); + return PTR_ERR(glue->usb_phy); + } + + glue->xceiv = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2); + if (IS_ERR(glue->xceiv)) { + dev_err(dev, "fail to getting usb-phy %d\n", ret); + ret = PTR_ERR(glue->xceiv); + goto err_unregister_usb_phy; + } + + platform_set_drvdata(pdev, glue); + pm_runtime_enable(dev); + pm_runtime_get_sync(dev); + + ret = mtk_musb_clks_enable(glue); + if (ret) + goto err_enable_clk; + + pinfo = mtk_dev_info; + pinfo.parent = dev; + pinfo.res = pdev->resource; + pinfo.num_res = pdev->num_resources; + pinfo.data = pdata; + pinfo.size_data = sizeof(*pdata); + + glue->musb_pdev = platform_device_register_full(&pinfo); + if (IS_ERR(glue->musb_pdev)) { + ret = PTR_ERR(glue->musb_pdev); + dev_err(dev, "failed to register musb device: %d\n", ret); + goto err_device_register; + } + + if (pdata->mode == USB_DR_MODE_OTG) + mtk_otg_switch_init(glue); + + dev_info(dev, "USB probe done!\n"); + return 0; + +err_device_register: + mtk_musb_clks_disable(glue); +err_enable_clk: + pm_runtime_put_sync(dev); + pm_runtime_disable(dev); +err_unregister_usb_phy: + usb_phy_generic_unregister(glue->usb_phy); + return ret; +} + +static int mtk_musb_remove(struct platform_device *pdev) +{ + struct mtk_glue *glue = platform_get_drvdata(pdev); + struct platform_device *usb_phy = glue->usb_phy; + + platform_device_unregister(glue->musb_pdev); + usb_phy_generic_unregister(usb_phy); + + return 0; +} + +#ifdef CONFIG_OF +static const struct of_device_id mtk_musb_match[] = { + {.compatible = "mediatek,mtk-musb",}, + {}, +}; +MODULE_DEVICE_TABLE(of, mtk_musb_match); +#endif + +static struct platform_driver mtk_musb_driver = { + .probe = mtk_musb_probe, + .remove = mtk_musb_remove, + .driver = { + .name = "musb-mtk", + .of_match_table = of_match_ptr(mtk_musb_match), + }, +}; + +module_platform_driver(mtk_musb_driver); + +MODULE_DESCRIPTION("MediaTek MUSB Glue Layer"); +MODULE_AUTHOR("Min Guo <min.guo@mediatek.com>"); +MODULE_LICENSE("GPL v2"); diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index b7d5627..d60f76f 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -1028,6 +1028,16 @@ static void musb_disable_interrupts(struct musb *musb) temp = musb_readb(mbase, MUSB_INTRUSB); temp = musb_readw(mbase, MUSB_INTRTX); temp = musb_readw(mbase, MUSB_INTRRX); + + /* MediaTek controller interrupt status is W1C */ + if (musb->ops->quirks & MUSB_MTK_QUIRKS) { + musb_writeb(mbase, MUSB_INTRUSB, + musb_readb(mbase, MUSB_INTRUSB)); + musb_writew(mbase, MUSB_INTRRX, + musb_readw(mbase, MUSB_INTRRX)); + musb_writew(mbase, MUSB_INTRTX, + musb_readw(mbase, MUSB_INTRTX)); + } } static void musb_enable_interrupts(struct musb *musb) diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h index 04203b7..1bf4e9a 100644 --- a/drivers/usb/musb/musb_core.h +++ b/drivers/usb/musb/musb_core.h @@ -138,6 +138,7 @@ enum musb_g_ep0_state { */ struct musb_platform_ops { +#define MUSB_MTK_QUIRKS BIT(10) #define MUSB_G_NO_SKB_RESERVE BIT(9) #define MUSB_DA8XX BIT(8) #define MUSB_PRESERVE_SESSION BIT(7) diff --git a/drivers/usb/musb/musb_dma.h b/drivers/usb/musb/musb_dma.h index 281e75d3..b218210 100644 --- a/drivers/usb/musb/musb_dma.h +++ b/drivers/usb/musb/musb_dma.h @@ -197,6 +197,7 @@ static inline void musb_dma_controller_destroy(struct dma_controller *d) { } extern struct dma_controller * musbhs_dma_controller_create(struct musb *musb, void __iomem *base); extern void musbhs_dma_controller_destroy(struct dma_controller *c); +extern irqreturn_t dma_controller_irq(int irq, void *private_data); extern struct dma_controller * tusb_dma_controller_create(struct musb *musb, void __iomem *base); diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c index b59ce9a..b1b0216 100644 --- a/drivers/usb/musb/musb_host.c +++ b/drivers/usb/musb/musb_host.c @@ -292,20 +292,73 @@ static inline void musb_save_toggle(struct musb_qh *qh, int is_in, { void __iomem *epio = qh->hw_ep->regs; u16 csr; + struct musb *musb = qh->hw_ep->musb; /* * FIXME: the current Mentor DMA code seems to have * problems getting toggle correct. */ - if (is_in) - csr = musb_readw(epio, MUSB_RXCSR) & MUSB_RXCSR_H_DATATOGGLE; - else - csr = musb_readw(epio, MUSB_TXCSR) & MUSB_TXCSR_H_DATATOGGLE; + /* MediaTek controller has private toggle register */ + if (musb->ops->quirks & MUSB_MTK_QUIRKS) { + u16 toggle; + u8 epnum = qh->hw_ep->epnum; + + if (is_in) + toggle = musb_readl(musb->mregs, MUSB_RXTOG); + else + toggle = musb_readl(musb->mregs, MUSB_TXTOG); + + csr = toggle & (1 << epnum); + } else { + if (is_in) + csr = musb_readw(epio, MUSB_RXCSR) + & MUSB_RXCSR_H_DATATOGGLE; + else + csr = musb_readw(epio, MUSB_TXCSR) + & MUSB_TXCSR_H_DATATOGGLE; + } usb_settoggle(urb->dev, qh->epnum, !is_in, csr ? 1 : 0); } +static inline u16 musb_set_toggle(struct musb_qh *qh, int is_in, + struct urb *urb) +{ + u16 csr = 0; + u16 toggle = 0; + struct musb *musb = qh->hw_ep->musb; + u8 epnum = qh->hw_ep->epnum; + + toggle = usb_gettoggle(urb->dev, qh->epnum, !is_in); + + /* MediaTek controller has private toggle register */ + if (musb->ops->quirks & MUSB_MTK_QUIRKS) { + if (is_in) { + musb_writel(musb->mregs, MUSB_RXTOGEN, (1 << epnum)); + musb_writel(musb->mregs, MUSB_RXTOG, (toggle << epnum)); + } else { + musb_writel(musb->mregs, MUSB_TXTOGEN, (1 << epnum)); + musb_writel(musb->mregs, MUSB_TXTOG, (toggle << epnum)); + } + } else { + if (is_in) { + if (toggle) + csr = MUSB_RXCSR_H_WR_DATATOGGLE + | MUSB_RXCSR_H_DATATOGGLE; + else + csr = 0; + } else { + if (toggle) + csr |= MUSB_TXCSR_H_WR_DATATOGGLE + | MUSB_TXCSR_H_DATATOGGLE; + else + csr |= MUSB_TXCSR_CLRDATATOG; + } + } + return csr; +} + /* * Advance this hardware endpoint's queue, completing the specified URB and * advancing to either the next URB queued to that qh, or else invalidating @@ -772,13 +825,8 @@ static void musb_ep_program(struct musb *musb, u8 epnum, ); csr |= MUSB_TXCSR_MODE; - if (!hw_ep->tx_double_buffered) { - if (usb_gettoggle(urb->dev, qh->epnum, 1)) - csr |= MUSB_TXCSR_H_WR_DATATOGGLE - | MUSB_TXCSR_H_DATATOGGLE; - else - csr |= MUSB_TXCSR_CLRDATATOG; - } + if (!hw_ep->tx_double_buffered) + csr |= musb_set_toggle(qh, !is_out, urb); musb_writew(epio, MUSB_TXCSR, csr); /* REVISIT may need to clear FLUSHFIFO ... */ @@ -860,17 +908,12 @@ static void musb_ep_program(struct musb *musb, u8 epnum, /* IN/receive */ } else { - u16 csr; + u16 csr = 0; if (hw_ep->rx_reinit) { musb_rx_reinit(musb, qh, epnum); + csr |= musb_set_toggle(qh, !is_out, urb); - /* init new state: toggle and NYET, maybe DMA later */ - if (usb_gettoggle(urb->dev, qh->epnum, 0)) - csr = MUSB_RXCSR_H_WR_DATATOGGLE - | MUSB_RXCSR_H_DATATOGGLE; - else - csr = 0; if (qh->type == USB_ENDPOINT_XFER_INT) csr |= MUSB_RXCSR_DISNYET; diff --git a/drivers/usb/musb/musb_regs.h b/drivers/usb/musb/musb_regs.h index 5cd7264..ffbe267 100644 --- a/drivers/usb/musb/musb_regs.h +++ b/drivers/usb/musb/musb_regs.h @@ -273,6 +273,12 @@ #define MUSB_RXHUBADDR 0x06 #define MUSB_RXHUBPORT 0x07 +/* MediaTek controller toggle enable and status reg */ +#define MUSB_RXTOG 0x80 +#define MUSB_RXTOGEN 0x82 +#define MUSB_TXTOG 0x84 +#define MUSB_TXTOGEN 0x86 + static inline u8 musb_read_configdata(void __iomem *mbase) { musb_writeb(mbase, MUSB_INDEX, 0); diff --git a/drivers/usb/musb/musbhsdma.c b/drivers/usb/musb/musbhsdma.c index 824adcb..c545475 100644 --- a/drivers/usb/musb/musbhsdma.c +++ b/drivers/usb/musb/musbhsdma.c @@ -263,7 +263,7 @@ static int dma_channel_abort(struct dma_channel *channel) return 0; } -static irqreturn_t dma_controller_irq(int irq, void *private_data) +irqreturn_t dma_controller_irq(int irq, void *private_data) { struct musb_dma_controller *controller = private_data; struct musb *musb = controller->private_data; @@ -285,6 +285,8 @@ static irqreturn_t dma_controller_irq(int irq, void *private_data) spin_lock_irqsave(&musb->lock, flags); int_hsdma = musb_readb(mbase, MUSB_HSDMA_INTR); + if (musb->ops->quirks & MUSB_MTK_QUIRKS) + musb_writeb(musb->mregs, MUSB_HSDMA_INTR, int_hsdma); if (!int_hsdma) { musb_dbg(musb, "spurious DMA irq"); @@ -377,15 +379,17 @@ static irqreturn_t dma_controller_irq(int irq, void *private_data) spin_unlock_irqrestore(&musb->lock, flags); return retval; } +EXPORT_SYMBOL_GPL(dma_controller_irq); void musbhs_dma_controller_destroy(struct dma_controller *c) { struct musb_dma_controller *controller = container_of(c, struct musb_dma_controller, controller); + struct musb *musb = controller->private_data; dma_controller_stop(controller); - if (controller->irq) + if (!(musb->ops->quirks & MUSB_MTK_QUIRKS) && controller->irq) free_irq(controller->irq, c); kfree(controller); @@ -398,11 +402,15 @@ struct dma_controller *musbhs_dma_controller_create(struct musb *musb, struct musb_dma_controller *controller; struct device *dev = musb->controller; struct platform_device *pdev = to_platform_device(dev); - int irq = platform_get_irq_byname(pdev, "dma"); + int irq = -1; - if (irq <= 0) { - dev_err(dev, "No DMA interrupt line!\n"); - return NULL; + if (!(musb->ops->quirks & MUSB_MTK_QUIRKS)) { + irq = platform_get_irq_byname(pdev, "dma"); + + if (irq < 0) { + dev_err(dev, "No DMA interrupt line!\n"); + return NULL; + } } controller = kzalloc(sizeof(*controller), GFP_KERNEL); @@ -418,15 +426,17 @@ struct dma_controller *musbhs_dma_controller_create(struct musb *musb, controller->controller.channel_program = dma_channel_program; controller->controller.channel_abort = dma_channel_abort; - if (request_irq(irq, dma_controller_irq, 0, + if (!(musb->ops->quirks & MUSB_MTK_QUIRKS)) { + if (request_irq(irq, dma_controller_irq, 0, dev_name(musb->controller), &controller->controller)) { - dev_err(dev, "request_irq %d failed!\n", irq); - musb_dma_controller_destroy(&controller->controller); + dev_err(dev, "request_irq %d failed!\n", irq); + musb_dma_controller_destroy(&controller->controller); - return NULL; - } + return NULL; + } - controller->irq = irq; + controller->irq = irq; + } return &controller->controller; }