Re: [PATCH v2 4/4] usb: musb: Add support for MediaTek musb controller
From: Min Guo
Date: Tue Jan 15 2019 - 21:43:31 EST
Hi Bin,
On Tue, 2019-01-15 at 14:38 -0600, Bin Liu wrote:
> Hi Min,
>
> very close, thanks.
> Below I tried to explain a further cleanup in musb_clearb/w() and
> musb_get/set_toggle() implementation. Please let me know if it is not
> clear.
>
> Basically, we don't need musb_default_clearb/w(), just assign the
> musb_io function pointers to musb_readb/w().
>
> Then the mtk platform musb_clearb/w() calls musb_readb/w() and
> musb_writeb/w() to handle W1C.
Okay.
> On Tue, Jan 15, 2019 at 09:43:46AM +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>
> > ---
> > drivers/usb/musb/Kconfig | 8 +-
> > drivers/usb/musb/Makefile | 1 +
> > drivers/usb/musb/mediatek.c | 617 +++++++++++++++++++++++++++++++++++++++++++
> > drivers/usb/musb/musb_core.c | 69 +++++
> > drivers/usb/musb/musb_core.h | 9 +
> > drivers/usb/musb/musb_dma.h | 9 +
> > drivers/usb/musb/musb_host.c | 26 +-
> > drivers/usb/musb/musb_io.h | 6 +
> > drivers/usb/musb/musbhsdma.c | 55 ++--
> > 9 files changed, 762 insertions(+), 38 deletions(-)
> > create mode 100644 drivers/usb/musb/mediatek.c
> >
> > diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
> > index ad08895..b72b7c1 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 || COMPILE_TEST
> > + 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..7221989
> > --- /dev/null
> > +++ b/drivers/usb/musb/mediatek.c
> > @@ -0,0 +1,617 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2018 MediaTek Inc.
> > + *
> > + * Author:
> > + * Min Guo <min.guo@xxxxxxxxxxxx>
> > + * Yonglong Wu <yonglong.wu@xxxxxxxxxxxx>
> > + */
> > +
> > +#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
> > +
> > +/* MediaTek controller toggle enable and status reg */
> > +#define MUSB_RXTOG 0x80
> > +#define MUSB_RXTOGEN 0x82
> > +#define MUSB_TXTOG 0x84
> > +#define MUSB_TXTOGEN 0x86
> > +
> > +#define TX_INT_STATUS BIT(0)
> > +#define RX_INT_STATUS BIT(1)
> > +#define USBCOM_INT_STATUS BIT(2)
>
> missing a TAB for the alignment?
Okay.
> > +#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)
>
> add one more TAB for params.
Okay.
> > +{
> > + 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_clearw(musb->mregs, MUSB_INTRRX, musb->int_rx);
> > + musb_clearw(musb->mregs, MUSB_INTRTX, musb->int_tx);
> > + musb_clearb(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 void mtk_musb_clearb(void __iomem *addr, unsigned int offset, u8 data)
>
> remove 'u8 data' parameter, then add:
Okay.
> > +{
>
> u8 data;
>
> > + /* W1C */
> data = musb_readb(addr, offset);
> > + musb_writeb(addr, offset, data);
> > +}
> > +
> > +static void mtk_musb_clearw(void __iomem *addr, unsigned int offset, u16 data)
> > +{
> > + /* W1C */
> > + musb_writew(addr, offset, data);
> > +}
>
> similar as mtk_musb_clearb() above.
Okay.
> > +
> > +static int mtk_musb_init(struct musb *musb)
> > +{
> > + struct device *dev = musb->controller;
> > + struct mtk_glue *glue = dev_get_drvdata(dev->parent);
> > + int ret;
>
> [snip]
>
> > + if (pdata->mode == USB_DR_MODE_OTG)
> > + mtk_otg_switch_init(glue);
> > +
> > + dev_info(dev, "USB probe done!\n");
>
> not really useful, can be removed.
Okay.
> > + 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@xxxxxxxxxxxx>");
> > +MODULE_LICENSE("GPL v2");
> > diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> > index b7d5627..2c0d102 100644
> > --- a/drivers/usb/musb/musb_core.c
> > +++ b/drivers/usb/musb/musb_core.c
> > @@ -260,6 +260,11 @@ static void musb_default_writeb(void __iomem *addr, unsigned offset, u8 data)
> > __raw_writeb(data, addr + offset);
> > }
> >
> > +static void
> > +musb_default_clearb(void __iomem *addr, unsigned int offset, u8 data)
> > +{
> > +}
>
> don't need this, use musb_readb() for the function pointer.
Okay.
> > +
> > static u16 musb_default_readw(const void __iomem *addr, unsigned offset)
> > {
> > u16 data = __raw_readw(addr + offset);
> > @@ -274,6 +279,43 @@ static void musb_default_writew(void __iomem *addr, unsigned offset, u16 data)
> > __raw_writew(data, addr + offset);
> > }
> >
> > +static void
> > +musb_default_clearw(void __iomem *addr, unsigned int offset, u16 data)
> > +{
> > +}
>
> don't need this, use musb_readw() for the function pointer.
Okay.
> > +
> > +static u16 musb_default_get_toggle(struct musb_qh *qh, int is_in)
> > +{
> > + void __iomem *epio = qh->hw_ep->regs;
> > + u16 csr;
> > +
> > + if (is_in)
> > + csr = musb_readw(epio, MUSB_RXCSR) & MUSB_RXCSR_H_DATATOGGLE;
> > + else
> > + csr = musb_readw(epio, MUSB_TXCSR) & MUSB_TXCSR_H_DATATOGGLE;
> > +
> > + return csr;
> > +}
> > +
> > +static u16 musb_default_set_toggle(struct musb_qh *qh, int is_in,
> > + struct urb *urb)
> > +{
> > + u16 csr = 0;
> > + u16 toggle = 0;
>
> no need to assign them 0.
Okay.
> > +
> > + 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;
> > +}
> > +
> > /*
> > * Load an endpoint's FIFO
> > */
> > @@ -370,12 +412,18 @@ static void musb_default_read_fifo(struct musb_hw_ep *hw_ep, u16 len, u8 *dst)
> > void (*musb_writeb)(void __iomem *addr, unsigned offset, u8 data);
> > EXPORT_SYMBOL_GPL(musb_writeb);
> >
> > +void (*musb_clearb)(void __iomem *addr, unsigned int offset, u8 data);
> > +EXPORT_SYMBOL_GPL(musb_clearb);
> > +
> > u16 (*musb_readw)(const void __iomem *addr, unsigned offset);
> > EXPORT_SYMBOL_GPL(musb_readw);
> >
> > void (*musb_writew)(void __iomem *addr, unsigned offset, u16 data);
> > EXPORT_SYMBOL_GPL(musb_writew);
> >
> > +void (*musb_clearw)(void __iomem *addr, unsigned int offset, u16 data);
> > +EXPORT_SYMBOL_GPL(musb_clearw);
> > +
> > u32 musb_readl(const void __iomem *addr, unsigned offset)
> > {
> > u32 data = __raw_readl(addr + offset);
> > @@ -1028,6 +1076,11 @@ 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);
>
> replace the 3 line above with
> musb_clearb/w();
Okay.
> > +
> > + /* some platform needs clear pending interrupts by manual */
> > + musb_clearb(mbase, MUSB_INTRUSB, musb_readb(mbase, MUSB_INTRUSB));
> > + musb_clearw(mbase, MUSB_INTRRX, musb_readw(mbase, MUSB_INTRRX));
> > + musb_clearw(mbase, MUSB_INTRTX, musb_readw(mbase, MUSB_INTRTX));
>
> then those are no longer needed.
Okay.
> > }
> >
> > static void musb_enable_interrupts(struct musb *musb)
> > @@ -2192,6 +2245,8 @@ static void musb_deassert_reset(struct work_struct *work)
> > musb_writeb = musb_default_writeb;
> > musb_readw = musb_default_readw;
> > musb_writew = musb_default_writew;
> > + musb_clearb = musb_default_clearb;
> > + musb_clearw = musb_default_clearw;
> >
> > /* The musb_platform_init() call:
> > * - adjusts musb->mregs
> > @@ -2252,10 +2307,14 @@ static void musb_deassert_reset(struct work_struct *work)
> > musb_readb = musb->ops->readb;
> > if (musb->ops->writeb)
> > musb_writeb = musb->ops->writeb;
> > + if (musb->ops->clearb)
> > + musb_clearb = musb->ops->clearb;
> else
> musb_clearb = musb_readb;
Okay.
> > if (musb->ops->readw)
> > musb_readw = musb->ops->readw;
> > if (musb->ops->writew)
> > musb_writew = musb->ops->writew;
> > + if (musb->ops->clearw)
> > + musb_clearw = musb->ops->clearw;
> else
> musb_clearw = musb_readw;
Okay.
> >
> > #ifndef CONFIG_MUSB_PIO_ONLY
> > if (!musb->ops->dma_init || !musb->ops->dma_exit) {
> > @@ -2277,6 +2336,16 @@ static void musb_deassert_reset(struct work_struct *work)
> > else
> > musb->io.write_fifo = musb_default_write_fifo;
> >
> > + if (musb->ops->get_toggle)
> > + musb->io.get_toggle = musb->ops->get_toggle;
> > + else
> > + musb->io.get_toggle = musb_default_get_toggle;
> > +
> > + if (musb->ops->set_toggle)
> > + musb->io.set_toggle = musb->ops->set_toggle;
> > + else
> > + musb->io.set_toggle = musb_default_set_toggle;
> > +
> > if (!musb->xceiv->io_ops) {
> > musb->xceiv->io_dev = musb->controller;
> > musb->xceiv->io_priv = musb->mregs;
> > diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
> > index 04203b7..71dca80 100644
> > --- a/drivers/usb/musb/musb_core.h
> > +++ b/drivers/usb/musb/musb_core.h
> > @@ -27,6 +27,7 @@
> > struct musb;
> > struct musb_hw_ep;
> > struct musb_ep;
> > +struct musb_qh;
> >
> > /* Helper defines for struct musb->hwvers */
> > #define MUSB_HWVERS_MAJOR(x) ((x >> 10) & 0x1f)
> > @@ -119,10 +120,14 @@ enum musb_g_ep0_state {
> > * @fifo_offset: returns the fifo offset
> > * @readb: read 8 bits
> > * @writeb: write 8 bits
> > + * @clearb: clear 8 bits,
>
> add "could be clear-on-read or W1C" to give more info.
Okay.
> > * @readw: read 16 bits
> > * @writew: write 16 bits
> > + * @clearw: clear 16 bits
> > * @read_fifo: reads the fifo
> > * @write_fifo: writes to fifo
> > + * @get_toggle: platform specific get toggle function
> > + * @set_toggle: platform specific set toggle function
> > * @dma_init: platform specific dma init function
> > * @dma_exit: platform specific dma exit function
> > * @init: turns on clocks, sets up platform-specific registers, etc
> > @@ -163,10 +168,14 @@ struct musb_platform_ops {
> > u32 (*busctl_offset)(u8 epnum, u16 offset);
> > u8 (*readb)(const void __iomem *addr, unsigned offset);
> > void (*writeb)(void __iomem *addr, unsigned offset, u8 data);
> > + void (*clearb)(void __iomem *addr, unsigned int offset, u8 data);
> > u16 (*readw)(const void __iomem *addr, unsigned offset);
> > void (*writew)(void __iomem *addr, unsigned offset, u16 data);
> > + void (*clearw)(void __iomem *addr, unsigned int offset, u16 data);
> > void (*read_fifo)(struct musb_hw_ep *hw_ep, u16 len, u8 *buf);
> > void (*write_fifo)(struct musb_hw_ep *hw_ep, u16 len, const u8 *buf);
> > + u16 (*get_toggle)(struct musb_qh *qh, int is_in);
> > + u16 (*set_toggle)(struct musb_qh *qh, int is_in, struct urb *urb);
> > struct dma_controller *
> > (*dma_init) (struct musb *musb, void __iomem *base);
> > void (*dma_exit)(struct dma_controller *c);
> > diff --git a/drivers/usb/musb/musb_dma.h b/drivers/usb/musb/musb_dma.h
> > index 8f60271..05103ea 100644
> > --- a/drivers/usb/musb/musb_dma.h
> > +++ b/drivers/usb/musb/musb_dma.h
> > @@ -35,6 +35,12 @@
> > * whether shared with the Inventra core or separate.
> > */
> >
> > +#define MUSB_HSDMA_BASE 0x200
> > +#define MUSB_HSDMA_INTR (MUSB_HSDMA_BASE + 0)
> > +#define MUSB_HSDMA_CONTROL 0x4
> > +#define MUSB_HSDMA_ADDRESS 0x8
> > +#define MUSB_HSDMA_COUNT 0xc
> > +
> > #define DMA_ADDR_INVALID (~(dma_addr_t)0)
> >
> > #ifdef CONFIG_MUSB_PIO_ONLY
> > @@ -191,6 +197,9 @@ 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 struct dma_controller *
> > +musbhs_dma_controller_create_noirq(struct musb *musb, void __iomem *base);
> > +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 16d0ba4..ba66f44 100644
> > --- a/drivers/usb/musb/musb_host.c
> > +++ b/drivers/usb/musb/musb_host.c
> > @@ -290,39 +290,23 @@ static void musb_giveback(struct musb *musb, struct urb *urb, int status)
> > static inline void musb_save_toggle(struct musb_qh *qh, int is_in,
> > struct urb *urb)
> > {
> > - void __iomem *epio = qh->hw_ep->regs;
> > - u16 csr;
> > + struct musb *musb = qh->hw_ep->musb;
> > + u16 csr;
> >
> > /*
> > * 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;
> > -
> > + csr = musb->io.get_toggle(qh, 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;
> > -
> > - 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;
> > + struct musb *musb = qh->hw_ep->musb;
> >
> > - return csr;
> > + return musb->io.set_toggle(qh, is_in, urb);
> > }
>
> Those two functions - musb_save_toggle() and musb_set_toggle() are very
> short now, we can get rid of them, and directly use
> musb->io.get/set_toggle().
Okay.
> >
> > /*
> > diff --git a/drivers/usb/musb/musb_io.h b/drivers/usb/musb/musb_io.h
> > index 8058a58..9bae09b 100644
> > --- a/drivers/usb/musb/musb_io.h
> > +++ b/drivers/usb/musb/musb_io.h
> > @@ -22,6 +22,8 @@
> > * @read_fifo: platform specific function to read fifo
> > * @write_fifo: platform specific function to write fifo
> > * @busctl_offset: platform specific function to get busctl offset
> > + * @get_toggle: platform specific function to get toggle
> > + * @set_toggle: platform specific function to set toggle
> > */
> > struct musb_io {
> > u32 (*ep_offset)(u8 epnum, u16 offset);
> > @@ -30,13 +32,17 @@ struct musb_io {
> > void (*read_fifo)(struct musb_hw_ep *hw_ep, u16 len, u8 *buf);
> > void (*write_fifo)(struct musb_hw_ep *hw_ep, u16 len, const u8 *buf);
> > u32 (*busctl_offset)(u8 epnum, u16 offset);
> > + u16 (*get_toggle)(struct musb_qh *qh, int is_in);
> > + u16 (*set_toggle)(struct musb_qh *qh, int is_in, struct urb *urb);
> > };
> >
> > /* Do not add new entries here, add them the struct musb_io instead */
> > extern u8 (*musb_readb)(const void __iomem *addr, unsigned offset);
> > extern void (*musb_writeb)(void __iomem *addr, unsigned offset, u8 data);
> > +extern void (*musb_clearb)(void __iomem *addr, unsigned int offset, u8 data);
> > extern u16 (*musb_readw)(const void __iomem *addr, unsigned offset);
> > extern void (*musb_writew)(void __iomem *addr, unsigned offset, u16 data);
> > +extern void (*musb_clearw)(void __iomem *addr, unsigned int offset, u16 data);
> > extern u32 musb_readl(const void __iomem *addr, unsigned offset);
> > extern void musb_writel(void __iomem *addr, unsigned offset, u32 data);
> >
> > diff --git a/drivers/usb/musb/musbhsdma.c b/drivers/usb/musb/musbhsdma.c
> > index a688f7f..b05fe68 100644
> > --- a/drivers/usb/musb/musbhsdma.c
> > +++ b/drivers/usb/musb/musbhsdma.c
> > @@ -10,12 +10,7 @@
> > #include <linux/platform_device.h>
> > #include <linux/slab.h>
> > #include "musb_core.h"
> > -
> > -#define MUSB_HSDMA_BASE 0x200
> > -#define MUSB_HSDMA_INTR (MUSB_HSDMA_BASE + 0)
> > -#define MUSB_HSDMA_CONTROL 0x4
> > -#define MUSB_HSDMA_ADDRESS 0x8
> > -#define MUSB_HSDMA_COUNT 0xc
> > +#include "musb_dma.h"
> >
> > #define MUSB_HSDMA_CHANNEL_OFFSET(_bchannel, _offset) \
> > (MUSB_HSDMA_BASE + (_bchannel << 4) + _offset)
> > @@ -268,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;
> > @@ -291,6 +286,9 @@ static irqreturn_t dma_controller_irq(int irq, void *private_data)
> >
> > int_hsdma = musb_readb(mbase, MUSB_HSDMA_INTR);
> >
> > + /* some platform needs clear pending interrupts by manual */
> > + musb_clearb(musb->mregs, MUSB_HSDMA_INTR, int_hsdma);
> > +
> > if (!int_hsdma) {
> > musb_dbg(musb, "spurious DMA irq");
> >
> > @@ -382,6 +380,7 @@ 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)
> > {
> > @@ -397,18 +396,10 @@ void musbhs_dma_controller_destroy(struct dma_controller *c)
> > }
> > EXPORT_SYMBOL_GPL(musbhs_dma_controller_destroy);
> >
> > -struct dma_controller *musbhs_dma_controller_create(struct musb *musb,
> > +static struct musb_dma_controller *dma_controller_alloc(struct musb *musb,
> > void __iomem *base)
> > {
> > 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");
> > -
> > - if (irq <= 0) {
> > - dev_err(dev, "No DMA interrupt line!\n");
> > - return NULL;
> > - }
> >
> > controller = kzalloc(sizeof(*controller), GFP_KERNEL);
> > if (!controller)
> > @@ -422,6 +413,25 @@ struct dma_controller *musbhs_dma_controller_create(struct musb *musb,
> > controller->controller.channel_release = dma_channel_release;
> > controller->controller.channel_program = dma_channel_program;
> > controller->controller.channel_abort = dma_channel_abort;
> > + return controller;
> > +}
> > +
> > +struct dma_controller *musbhs_dma_controller_create(struct musb *musb,
> > + void __iomem *base)
> > +{
> > + 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");
> > +
> > + if (irq <= 0) {
> > + dev_err(dev, "No DMA interrupt line!\n");
> > + return NULL;
> > + }
> > +
> > + controller = dma_controller_alloc(musb, base);
> > + if (!controller)
> > + return NULL;
> >
> > if (request_irq(irq, dma_controller_irq, 0,
> > dev_name(musb->controller), &controller->controller)) {
> > @@ -436,3 +446,16 @@ struct dma_controller *musbhs_dma_controller_create(struct musb *musb,
> > return &controller->controller;
> > }
> > EXPORT_SYMBOL_GPL(musbhs_dma_controller_create);
> > +
> > +struct dma_controller *musbhs_dma_controller_create_noirq(struct musb *musb,
> > + void __iomem *base)
> > +{
> > + struct musb_dma_controller *controller;
> > +
> > + controller = dma_controller_alloc(musb, base);
> > + if (!controller)
> > + return NULL;
> > +
> > + return &controller->controller;
> > +}
> > +EXPORT_SYMBOL_GPL(musbhs_dma_controller_create_noirq);
>
> regards,
> -Bin.