Re: [U-Boot] [PATCH 1/2] usb: musb-new: Add DA8xx MUSB Glue and enable Host mode

2 Jul 2019


      On Tue, Jul 2, 2019 at 4:41 PM Marek Vasut marex@denx.de wrote:
...
On 7/2/19 11:30 PM, Adam Ford wrote:
[...]
...
@@ -0,0 +-2,466 @@
+// SPDX-License-Identifier: GPL-2.0
+/*


Texas Instruments da8xx "glue layer"







Copyright (c) 2010, by Texas Instruments







Based on the DA8xx "glue layer" code.



Copyright (c) 2008-2009, MontaVista Software, Inc. source@mvista.com







This file is part of the Inventra Controller Driver for Linux.



Shouldn't you grow copyright on this too ?
Sure.  I borrowed much of this code from the Linux kernel, so I'll
reference that too.
...
[...]
...

                musb->int_usb &= ~MUSB_INTR_VBUSERROR;


                //musb->xceiv->otg->state = OTG_STATE_A_WAIT_VFALL;


                //mod_timer(&musb->dev_timer, jiffies + POLL_SECONDS * HZ);



Why is this commented out in such a crude c++ way ? Surely , checkpatch
complains about it ?
I was part of the Linux kernel code, and I didn't want to lose it.
I'll put an #ifdef __UBOOT__ around it like other drivers do.
...
...

                WARNING("VBUS error workaround (delay coming)\n");


        } else if (drvvbus) {


                MUSB_HST_MODE(musb);


                //musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;


                //portstate(musb->port1_status |= USB_PORT_STAT_POWER);


                //del_timer(&musb->dev_timer);


        } else if (!(musb->int_usb & MUSB_INTR_BABBLE)) {


                /*


                 * When babble condition happens, drvvbus interrupt


                 * is also generated. Ignore this drvvbus interrupt


                 * and let babble interrupt handler recovers the


                 * controller; otherwise, the host-mode flag is lost


                 * due to the MUSB_DEV_MODE() call below and babble


                 * recovery logic will not be called.


                 */


                musb->is_active = 0;


                MUSB_DEV_MODE(musb);


                //musb->xceiv->otg->state = OTG_STATE_B_IDLE;


                //portstate(musb->port1_status &= ~USB_PORT_STAT_POWER);


        }



+#if 0
#if 0? Delete the code then ?
I'll do the same as I mentioned above.
...
...

        dev_dbg(musb->controller, "VBUS %s (%s)%s, devctl %02x\n",


                drvvbus ? "on" : "off",


                        usb_otg_state_string(musb->xceiv->otg->state),


                        err ? " ERROR" : "",


                        devctl);



+#endif

        ret = IRQ_HANDLED;


}



if (musb->int_tx || musb->int_rx || musb->int_usb)


        ret |= musb_interrupt(musb);



+eoi:

/* EOI needs to be written for the IRQ to be re-asserted. */


if (ret == IRQ_HANDLED || status)


        musb_writel(reg_base, DA8XX_USB_END_OF_INTR_REG, 0);



spin_unlock_irqrestore(&musb->lock, flags);



return ret;



+}



+static void enable_vbus(void)
+{

u32 value;




+#if 0

gpio_request(CONFIG_OMAP_EHCI_PHY1_RESET_GPIO, "USB PHY1 reset");


gpio_direction_output(CONFIG_OMAP_EHCI_PHY1_RESET_GPIO, !on);



/* configure GPIO bank4 pin 15 in output direction */


value = readl(&davinci_gpio_bank45->dir);


writel((value & (~DA8XX_USB_VBUS_GPIO)), &davinci_gpio_bank45->dir);



/* set GPIO bank4 pin 15 high to drive VBUS */


value = readl(&davinci_gpio_bank45->set_data);


writel((value | DA8XX_USB_VBUS_GPIO), &davinci_gpio_bank45->set_data);



+#endif
+}



+/*


Enable the usb0 phy. This initialization procedure is explained in



the DA8xx USB user guide document.


*/

+static u8 phy_on(void)
+{

u32 timeout;


u32 cfgchip2;



cfgchip2 = readl(&davinci_syscfg_regs->cfgchip2);



cfgchip2 &= ~(CFGCHIP2_RESET | CFGCHIP2_PHYPWRDN | CFGCHIP2_OTGPWRDN |


              CFGCHIP2_OTGMODE | CFGCHIP2_REFFREQ);


cfgchip2 |= CFGCHIP2_SESENDEN | CFGCHIP2_VBDTCTEN | CFGCHIP2_PHY_PLLON |


            CFGCHIP2_REFFREQ_24MHZ;



writel(cfgchip2, &davinci_syscfg_regs->cfgchip2);



/* wait until the usb phy pll locks */


timeout = 0x3FFFFFF; //musb_cfg.timeout;


while (timeout--)


        if (readl(&davinci_syscfg_regs->cfgchip2) & CFGCHIP2_PHYCLKGD)


                return 1;



wait_for_bit ...
...

debug("Phy was not turned on\n");


/* USB phy was not turned on */


return 0;



+}



+/*


Disable the usb phy


*/

+static void phy_off(void)
+{

u32 cfgchip2;



Implement proper PHY driver.
I don't have an example to use, and other OMAP drivers do it this way.
...
...

/*


 * Power down the on-chip PHY.


 */


cfgchip2 = readl(&davinci_syscfg_regs->cfgchip2);



clrsetbits_le32()
...

cfgchip2 &= ~CFGCHIP2_PHY_PLLON;


cfgchip2 |= CFGCHIP2_PHYPWRDN | CFGCHIP2_OTGPWRDN;


writel(cfgchip2, &davinci_syscfg_regs->cfgchip2);



+}



+static void da8xx_musb_phy_power(struct udevice *dev, u8 on)
+{

unsigned long start = get_timer(0);



start is unused
...

if (on)


        phy_on();


else


        phy_off();



+}



+static int da8xx_musb_init(struct musb *musb)
+{

u32  revision;


void __iomem *reg_base = musb->ctrl_base;



/* enable psc for usb2.0 */


lpsc_on(33);



/* enable usb vbus */


enable_vbus();



/* reset the controller */


writel(0x1, &da8xx_usb_regs->control);


udelay(5000);



/* start the on-chip usb phy and its pll */


if (phy_on() == 0) {


        return -1;



Use errno.h error codes
This was how the older, deleted driver did it, so I thought if it
worked well enough before, i would work again.  I can update it to use
erro codes.
...
...

}



/* Returns zero if e.g. not clocked */


revision = readl(&da8xx_usb_regs->revision);


if (revision == 0) {


        return -1;


}



/* Disable all interrupts */


writel((DA8XX_USB_USBINT_MASK | DA8XX_USB_TXINT_MASK |


        DA8XX_USB_RXINT_MASK), &da8xx_usb_regs->intmsk_set);



musb->mregs += DA8XX_MENTOR_CORE_OFFSET;



/* NOTE: IRQs are in mixed mode, not bypass to pure MUSB */


debug("DA8xx OTG revision %08x, control %02x\n", revision,


      musb_readb(reg_base, DA8XX_USB_CTRL_REG));



musb->isr = da8xx_musb_interrupt;


return 0;



+}



+static int da8xx_musb_exit(struct musb *musb)
+{

/* Turn of the phy */


phy_off();



/* flush any interrupts */


writel((DA8XX_USB_USBINT_MASK | DA8XX_USB_TXINT_MASK |


        DA8XX_USB_RXINT_MASK), &da8xx_usb_regs->intmsk_clr);


writel(0, &da8xx_usb_regs->eoi);



return 0;



+}



+/**


da8xx_musb_enable - enable interrupts


*/

+static int da8xx_musb_enable(struct musb *musb)
+{

void __iomem *reg_base = musb->ctrl_base;


u32 mask;



/* Workaround: setup IRQs through both register sets. */


mask = ((musb->epmask & DA8XX_USB_TX_EP_MASK) << DA8XX_INTR_TX_SHIFT) |


       ((musb->epmask & DA8XX_USB_RX_EP_MASK) << DA8XX_INTR_RX_SHIFT) |


       DA8XX_INTR_USB_MASK;


musb_writel(reg_base, DA8XX_USB_INTR_MASK_SET_REG, mask);



/* Force the DRVVBUS IRQ so we can start polling for ID change. */


musb_writel(reg_base, DA8XX_USB_INTR_SRC_SET_REG,


            DA8XX_INTR_DRVVBUS << DA8XX_INTR_USB_SHIFT);



return 0;



+}



+/**


da8xx_musb_disable - disable HDRC and flush interrupts


*/

+static void da8xx_musb_disable(struct musb *musb)
+{

void __iomem *reg_base = musb->ctrl_base;



musb_writel(reg_base, DA8XX_USB_INTR_MASK_CLEAR_REG,


            DA8XX_INTR_USB_MASK |


            DA8XX_INTR_TX_MASK | DA8XX_INTR_RX_MASK);


musb_writel(reg_base, DA8XX_USB_END_OF_INTR_REG, 0);



+}



+void da8xx_musb_reset(struct udevice *dev)
+{

void *reg_base = dev_read_addr_ptr(dev);


/* Reset the controller */


        musb_writel(reg_base, DA8XX_USB_CTRL_REG, DA8XX_SOFT_RESET_MASK);



+}



+void da8xx_musb_clear_irq(struct udevice *dev)
+{

/* flush any interrupts */


        writel((DA8XX_USB_USBINT_MASK | DA8XX_USB_TXINT_MASK |


                DA8XX_USB_RXINT_MASK), &da8xx_usb_regs->intmsk_clr);


        writel(0, &da8xx_usb_regs->eoi);



+}



+const struct musb_platform_ops da8xx_ops = {

.init           = da8xx_musb_init,


.exit           = da8xx_musb_exit,


.enable         = da8xx_musb_enable,


.disable        = da8xx_musb_disable,



+};



+#if CONFIG_IS_ENABLED(DM_USB)
DM should always be enabled for new code, any reason for adding non-DM
stuff ?
Not really. I'll assume it's all DM_USB going forward.
...
...
+struct da8xx_musb_platdata {

void *base;


void *ctrl_mod_base;


struct musb_hdrc_platform_data plat;


struct musb_hdrc_config musb_config;


struct omap_musb_board_data otg_board_data;



+};



+static int da8xx_musb_ofdata_to_platdata(struct udevice *dev)
+{

struct da8xx_musb_platdata *platdata = dev_get_platdata(dev);


const void *fdt = gd->fdt_blob;


int node = dev_of_offset(dev);



platdata->base = (void *)dev_read_addr_ptr(dev);


platdata->musb_config.multipoint = 1;


platdata->musb_config.dyn_fifo = 1;


platdata->musb_config.num_eps = 5;


platdata->musb_config.ram_bits = 10;


platdata->plat.power = fdtdec_get_int(fdt, node,


                                      "power", -1);



Set default to 50 ^ and drop this code below v
Make sense.
...
...

if (platdata->plat.power < 0) {


        platdata->plat.power = 50;      /* If power isn't specified, assume 50mA */


}



platdata->otg_board_data.interface_type = MUSB_INTERFACE_UTMI;




+#if 0 /* In a perfect world, mode would be set to OTG, mode 3 from DT */
Drop dead code
...

platdata->plat.mode = fdtdec_get_int(fdt, node,


                                     "mode", -1);


if (platdata->plat.mode < 0) {


        pr_err("MUSB mode DT entry missing\n");


        return -ENOENT;


}



+#else /* MUSB_OTG, it doesn't work */
Why are you submitting stuff that does not work ?
Because the code nearly works, and I am not sure when I can get back
to it again.  The host portion does, and currently we have nothing.
Having these comments in there leave breadcrumbs for myself or someone
else to try and get it working.  Partially working seems better than
not working at all.
...
...
+#ifdef CONFIG_USB_MUSB_HOST /* Host seems to be the only option that works */

platdata->plat.mode = MUSB_HOST;



+#else /* For that matter, MUSB_PERIPHERAL doesn't either */

platdata->plat.mode = MUSB_PERIPHERAL;



+#endif
+#endif

platdata->otg_board_data.dev = dev;


platdata->plat.config = &platdata->musb_config;


platdata->plat.platform_ops = &da8xx_ops;


platdata->plat.board_data = &platdata->otg_board_data;


platdata->otg_board_data.set_phy_power = da8xx_musb_phy_power;


platdata->otg_board_data.clear_irq = da8xx_musb_clear_irq;


platdata->otg_board_data.reset = da8xx_musb_reset;


return 0;



+}



+static int da8xx_musb_probe(struct udevice *dev)
+{
+#ifdef CONFIG_USB_MUSB_HOST

struct musb_host_data *host = dev_get_priv(dev);



+#endif

struct da8xx_musb_platdata *platdata = dev_get_platdata(dev);


struct usb_bus_priv *priv = dev_get_uclass_priv(dev);


struct omap_musb_board_data *otg_board_data;


int ret;


void *base = dev_read_addr_ptr(dev);



priv->desc_before_addr = true;



otg_board_data = &platdata->otg_board_data;




+#ifdef CONFIG_USB_MUSB_HOST

host->host = musb_init_controller(&platdata->plat,


                                  (struct device *)otg_board_data,


                                  platdata->base);


if (!host->host) {


        return -EIO;


}



ret = musb_lowlevel_init(host);



+#else

ret = musb_register(&platdata->plat,


                    (struct device *)otg_board_data,


                  platdata->base);



+#endif

return ret;



+}



+static int da8xx_musb_remove(struct udevice *dev)
+{

struct musb_host_data *host = dev_get_priv(dev);



musb_stop(host->host);



return 0;



+}



+static const struct udevice_id da8xx_musb_ids[] = {

{ .compatible = "ti,da830-musb" },


{ }



+};



+U_BOOT_DRIVER(da8xx_musb) = {

.name   = "da8xx-musb",



+#ifdef CONFIG_USB_MUSB_HOST

.id             = UCLASS_USB,



+#else

.id             = UCLASS_USB_GADGET_GENERIC,



+#endif

.of_match = da8xx_musb_ids,


.ofdata_to_platdata = da8xx_musb_ofdata_to_platdata,


.probe = da8xx_musb_probe,


.remove = da8xx_musb_remove,



+#ifdef CONFIG_USB_MUSB_HOST

.ops = &musb_usb_ops,



+#endif

.platdata_auto_alloc_size = sizeof(struct da8xx_musb_platdata),


.priv_auto_alloc_size = sizeof(struct musb_host_data),



+};



+#endif /* CONFIG_IS_ENABLED(DM_USB) */



Thanks for the review.  I'll work on a V2 tomorrow.
adam
...
--
Best regards,
Marek Vasut
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-56 Email: marex@denx.de

    