[U-Boot] [PATCH v2 1/4] USB: gadget: Add the cadence USB3 gadget driver

[Marek Vasut]

On 8/14/19 2:16 PM, sherry sun wrote:
> From: Sherry Sun <sherry.sun at nxp.com>
> 
> This driver is ported from NXP i.MX U-Boot version imx_v2019.04
> and some changes have also been made to adapt to U-Boot.
> 
> Add the Cadence USB3 IP(CDNS3) driver for the gadget (device mode).
> The CDNS3 gadget driver support DM mode. CONFIG_DM_USB_GADGET should
> be enabled when use this driver. And gadget_is_cdns3 checking is
> added to provide bcdUSB value in device descriptor.

The cadence core isn't xhci compatible ? Sigh ...

[...]

> +++ b/doc/device-tree-bindings/usb/cdns-usb3.txt
> @@ -0,0 +1,39 @@
> +* Cadence USB3 Controller
> +
> +Required properties:
> +- compatible: "Cadence,usb3";

cdns , no ?

[...]

> +static int cdns3_generic_peripheral_clk_init(struct udevice *dev,
> +					     struct cdns3_generic_peripheral
> +					     *priv)
> +{
> +	int ret;
> +
> +	ret = clk_get_bulk(dev, &priv->clks);
> +	if (ret)
> +		return ret;
> +
> +#if CONFIG_IS_ENABLED(CLK)

Why is the ifdef protecting only half of the clock functions ?

> +	ret = clk_enable_bulk(&priv->clks);
> +	if (ret) {
> +		clk_release_bulk(&priv->clks);
> +		return ret;
> +	}
> +#endif
> +
> +	return 0;
> +}


[...]

> +static void cdns3_set_role(struct cdns3 *cdns, enum cdns3_roles role)
> +{
> +	u32 value;
> +	int timeout_us = 100000;
> +	struct cdns3_generic_peripheral *priv = container_of(cdns,
> +			struct cdns3_generic_peripheral, cdns3);
> +
> +	if (role == CDNS3_ROLE_END)
> +		return;
> +
> +	/* Wait clk value */
> +	value = readl(cdns->none_core_regs + USB3_SSPHY_STATUS);
> +	writel(value, cdns->none_core_regs + USB3_SSPHY_STATUS);
> +	udelay(1);
> +	value = readl(cdns->none_core_regs + USB3_SSPHY_STATUS);
> +	while ((value & 0xf0000000) != 0xf0000000 && timeout_us-- > 0) {
> +		value = readl(cdns->none_core_regs + USB3_SSPHY_STATUS);
> +		udelay(1);
> +	}

Is this like wait_for_bit() ?

> +	if (timeout_us <= 0)
> +		dev_err(cdns->dev, "wait clkvld timeout\n");
> +
> +	/* Set all Reset bits */
> +	setbits_le32(cdns->none_core_regs + USB3_CORE_CTRL1, ALL_SW_RESET);
> +	udelay(1);
> +
> +	if (role == CDNS3_ROLE_HOST) {

Do we need custom controller role definition ? I don't think so, just
use the one in the USB stack.

Also, use clrsetbits_le32()

> +		value = readl(cdns->none_core_regs + USB3_CORE_CTRL1);
> +		value = (value & ~MODE_STRAP_MASK) | HOST_MODE | OC_DISABLE;
> +		writel(value, cdns->none_core_regs + USB3_CORE_CTRL1);
> +		clrbits_le32(cdns->none_core_regs + USB3_CORE_CTRL1,
> +			     PHYAHB_SW_RESET);
> +		mdelay(1);
> +		generic_phy_init(&priv->phy);
> +		setbits_le32(cdns->phy_regs + TB_ADDR_TX_RCVDETSC_CTRL,
> +			     RXDET_IN_P3_32KHZ);
> +		udelay(10);
> +		/* Force B Session Valid as 1 */
> +		writel(0x0060, cdns->phy_regs + 0x380a4);
> +		mdelay(1);
> +
> +		setbits_le32(cdns->none_core_regs + USB3_INT_REG, HOST_INT1_EN);
> +
> +		clrbits_le32(cdns->none_core_regs + USB3_CORE_CTRL1,
> +			     ALL_SW_RESET);
> +
> +		dev_dbg(cdns->dev, "wait xhci_power_on_ready\n");
> +
> +		value = readl(cdns->none_core_regs + USB3_CORE_STATUS);
> +		timeout_us = 100000;
> +		while (!(value & HOST_POWER_ON_READY) && timeout_us-- > 0) {
> +			value = readl(cdns->none_core_regs + USB3_CORE_STATUS);
> +			udelay(1);
> +		}
> +
> +		if (timeout_us <= 0)
> +			dev_err(cdns->dev, "wait xhci_power_on_ready timeout\n");
> +
> +		dev_dbg(cdns->dev, "switch to host role successfully\n");
> +	} else { /* gadget mode */

Split this into sensibly long funtions please.

> +		value = readl(cdns->none_core_regs + USB3_CORE_CTRL1);
> +		value = (value & ~MODE_STRAP_MASK) | DEV_MODE;
> +		writel(value, cdns->none_core_regs + USB3_CORE_CTRL1);
> +		clrbits_le32(cdns->none_core_regs + USB3_CORE_CTRL1,
> +			     PHYAHB_SW_RESET);
> +
> +		generic_phy_init(&priv->phy);
> +		setbits_le32(cdns->phy_regs + TB_ADDR_TX_RCVDETSC_CTRL,
> +			     RXDET_IN_P3_32KHZ);
> +		udelay(10);
> +		/* Force B Session Valid as 1 */
> +		writel(0x0060, cdns->phy_regs + 0x380a4);
> +		setbits_le32(cdns->none_core_regs + USB3_INT_REG, DEV_INT_EN);
> +
> +		clrbits_le32(cdns->none_core_regs + USB3_CORE_CTRL1,
> +			     ALL_SW_RESET);
> +
> +		dev_dbg(cdns->dev, "wait gadget_power_on_ready\n");
> +
> +		value = readl(cdns->none_core_regs + USB3_CORE_STATUS);
> +		timeout_us = 100000;
> +		while (!(value & DEV_POWER_ON_READY) && timeout_us-- > 0) {
> +			value = readl(cdns->none_core_regs + USB3_CORE_STATUS);
> +			udelay(1);
> +		}

wait_for_bit()

[...]

> +/* macros for field CFGSTS */
> +#define USB_STS__CFGSTS__MASK                                       0x00000001U
> +#define USB_STS__USBSPEED__READ(src)     (((uint32_t)(src) & 0x00000070U) >> 4)

u32, fix globally

[...]

> +static void __cdns3_gadget_start(struct usb_ss_dev *usb_ss)
> +{
> +	u32 usb_conf_reg = 0;
> +
> +	/* configure endpoint 0 hardware */
> +	cdns_ep0_config(usb_ss);
> +
> +	/* enable interrupts for endpoint 0 (in and out) */
> +	cdns_writel(&usb_ss->regs->ep_ien,
> +		      EP_IEN__EOUTEN0__MASK | EP_IEN__EINEN0__MASK);
> +
> +	/* enable interrupt for device */
> +	cdns_writel(&usb_ss->regs->usb_ien,
> +		      USB_IEN__U2RESIEN__MASK
> +		      | USB_ISTS__DIS2I__MASK
> +		      | USB_IEN__CON2IEN__MASK
> +		      | USB_IEN__UHRESIEN__MASK
> +		      | USB_IEN__UWRESIEN__MASK
> +		      | USB_IEN__DISIEN__MASK
> +		      | USB_IEN__CONIEN__MASK
> +		      | USB_IEN__U3EXTIEN__MASK
> +		      | USB_IEN__L2ENTIEN__MASK
> +		      | USB_IEN__L2EXTIEN__MASK);
> +
> +	usb_conf_reg = USB_CONF__CLK2OFFDS__MASK |
> +			USB_CONF__L1DS__MASK;
> +	if (usb_ss->gadget.max_speed == USB_SPEED_HIGH)
> +		usb_conf_reg |= USB_CONF__USB3DIS__MASK;
> +	cdns_writel(&usb_ss->regs->usb_conf, usb_conf_reg);
> +
> +	cdns_writel(&usb_ss->regs->usb_conf,
> +		      USB_CONF__U1DS__MASK
> +		      | USB_CONF__U2DS__MASK
> +			/*
> +			 * TODO:
> +			 * | USB_CONF__L1EN__MASK
> +			 */
> +			);

Fix the TODO ?