driver model bring-up of dwc3 usb peripheral

From: Mugunthan V N <[email protected]>

devm_zalloc() is already defined in dm/device.h header, so devm_zalloc can be removed from linux_compact.h beader file.

Signed-off-by Mugunthan V N <[email protected]>

Signed-off-by Vignesh R <[email protected]>

Reviewed-by Simon Glass <[email protected]>

---
 drivers/usb/dwc3/core.c         | 7 +++++--
 drivers/usb/dwc3/dwc3-omap.c    | 3 ++-
 drivers/usb/dwc3/linux-compat.h | 5 -----
 drivers/usb/dwc3/ti_usb_phy.c   | 1 +
 4 files changed, 8 insertions(+), 8 deletions(-)

drivers/usb/dwc3/core.c +5 -2

index 87b9c87edf6a..98102bd6b00a 100644
@@ -19,6 +19,7 @@
 #include <dwc3-uboot.h>
 #include <asm/dma-mapping.h>
 #include <linux/ioport.h>
+#include <dm.h>
 
 #include <linux/usb/ch9.h>
 #include <linux/usb/gadget.h>

@@ -111,7 +112,8 @@ static struct dwc3_event_buffer *dwc3_alloc_one_event_buffer(struct dwc3 *dwc,
 {
 	struct dwc3_event_buffer	*evt;
 
-	evt = devm_kzalloc(dwc->dev, sizeof(*evt), GFP_KERNEL);
+	evt = devm_kzalloc((struct udevice *)dwc->dev, sizeof(*evt),
+			   GFP_KERNEL);
 	if (!evt)
 		return ERR_PTR(-ENOMEM);
 

@@ -624,7 +626,8 @@ int dwc3_uboot_init(struct dwc3_device *dwc3_dev)
 
 	void			*mem;
 
-	mem = devm_kzalloc(dev, sizeof(*dwc) + DWC3_ALIGN_MASK, GFP_KERNEL);
+	mem = devm_kzalloc((struct udevice *)dev,
+			   sizeof(*dwc) + DWC3_ALIGN_MASK, GFP_KERNEL);
 	if (!mem)
 		return -ENOMEM;
 

drivers/usb/dwc3/dwc3-omap.c +2 -1

index 3dcc2f484777..63551e780434 100644
@@ -20,6 +20,7 @@
 #include <dwc3-omap-uboot.h>
 #include <linux/usb/dwc3-omap.h>
 #include <linux/ioport.h>
+#include <dm.h>
 
 #include <linux/usb/otg.h>
 #include <linux/compat.h>

@@ -377,7 +378,7 @@ int dwc3_omap_uboot_init(struct dwc3_omap_device *omap_dev)
 	struct device		*dev = NULL;
 	struct dwc3_omap	*omap;
 
-	omap = devm_kzalloc(dev, sizeof(*omap), GFP_KERNEL);
+	omap = devm_kzalloc((struct udevice *)dev, sizeof(*omap), GFP_KERNEL);
 	if (!omap)
 		return -ENOMEM;
 

drivers/usb/dwc3/linux-compat.h +0 -5

index 9e944a31be11..f95d6152b7fe 100644
@@ -23,9 +23,4 @@ static inline size_t strlcat(char *dest, const char *src, size_t n)
 	return strlen(dest) + strlen(src);
 }
 
-static inline void *devm_kzalloc(struct device *dev, unsigned int size,
-				 unsigned int flags)
-{
-	return kzalloc(size, flags);
-}
 #endif

drivers/usb/dwc3/ti_usb_phy.c +1 -0

index 218a8e586c8d..8088afc97047 100644
@@ -24,6 +24,7 @@
 #include <linux/ioport.h>
 #include <asm/io.h>
 #include <asm/arch/sys_proto.h>
+#include <dm.h>
 
 #include "linux-compat.h"
 

Quoted 4 lines · Vignesh R

From: Mugunthan V N <[email protected]>

devm_zalloc() is already defined in dm/device.h header, so devm_zalloc can be removed from linux_compact.h beader file.

Shouldn't it be left in linux_compat.h instead ?

Quoted 6 lines · Marek Vasut

Quoted 4 lines · Vignesh R

From: Mugunthan V N <[email protected]>

devm_zalloc() is already defined in dm/device.h header, so devm_zalloc can be removed from linux_compact.h beader file.

Quoted 1 line

Shouldn't it be left in linux_compat.h instead ?

linux-compat.h is local to dwc3 folder where as dm/device.h is available globally. My understanding is linux-compat.h is to define functions/wrappers needed by dwc3 part picked from linux that are not yet directly available in U-Boot. But, given that devm_kzalloc() is provided in U-Boot, I thought it would be better to get rid of definition in linux-compat.h in order to avoid conflicting definition error.

From: Mugunthan V N <[email protected]>

Add support to get maximum speed from dt so that usb drivers makes use of it for DT parsing.

Signed-off-by Mugunthan V N <[email protected]>

Signed-off-by Vignesh R <[email protected]>

Reviewed-by Simon Glass <[email protected]>

---
 drivers/usb/common/common.c | 29 +++++++++++++++++++++++++++++
 include/linux/usb/otg.h     |  9 +++++++++
 scripts/Makefile.spl        |  1 +
 3 files changed, 39 insertions(+)

drivers/usb/common/common.c +10 -0

index 35c2dc18d955..c11689dc36fa 100644
@@ -10,6 +10,7 @@
 #include <common.h>
 #include <libfdt.h>
 #include <linux/usb/otg.h>
+#include <linux/usb/ch9.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 

@@ -38,3 +39,31 @@ enum usb_dr_mode usb_get_dr_mode(int node)
 
 	return USB_DR_MODE_UNKNOWN;
 }
+
+static const char *const speed_names[] = {
+	[USB_SPEED_UNKNOWN] = "UNKNOWN",
+	[USB_SPEED_LOW] = "low-speed",
+	[USB_SPEED_FULL] = "full-speed",
+	[USB_SPEED_HIGH] = "high-speed",
+	[USB_SPEED_WIRELESS] = "wireless",
+	[USB_SPEED_SUPER] = "super-speed",
+};

drivers/usb/common/common.c +2 -0

+	return USB_SPEED_UNKNOWN;
+}

include/linux/usb/otg.h +9 -0

index 8f8ac6aeefe3..b61ef19b22f3 100644
@@ -26,4 +26,13 @@ enum usb_dr_mode {
  */
 enum usb_dr_mode usb_get_dr_mode(int node);
 
+/**
+ * usb_get_maximum_speed() - Get maximum speed for given device
+ * @node: Node offset to the given device
+ *
+ * The function gets phy interface string from property 'maximum-speed',
+ * and returns the correspondig enum usb_device_speed
+ */
+enum usb_device_speed usb_get_maximum_speed(int node);
+
 #endif /* __LINUX_USB_OTG_H */

scripts/Makefile.spl +0 -0

index ac3c2c7f1342..bffea085b329 100644

scripts/Makefile.spl +0 -0

 libs-$(CONFIG_SPL_USB_GADGET_SUPPORT) += drivers/usb/dwc3/
 libs-y += dts/
 libs-y += fs/

Quoted 43 lines · Vignesh R

From: Mugunthan V N <[email protected]>

Add support to get maximum speed from dt so that usb drivers makes use of it for DT parsing.

Signed-off-by Mugunthan V N <[email protected]>

Signed-off-by Vignesh R <[email protected]>

Reviewed-by Simon Glass <[email protected]>

---
 drivers/usb/common/common.c | 29 +++++++++++++++++++++++++++++
 include/linux/usb/otg.h     |  9 +++++++++
 scripts/Makefile.spl        |  1 +
 3 files changed, 39 insertions(+)

drivers/usb/common/common.c +14 -0

index 35c2dc18d955..c11689dc36fa 100644
@@ -10,6 +10,7 @@
 #include <common.h>
 #include <libfdt.h>
 #include <linux/usb/otg.h>
+#include <linux/usb/ch9.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 

@@ -38,3 +39,31 @@ enum usb_dr_mode usb_get_dr_mode(int node)
 
 	return USB_DR_MODE_UNKNOWN;
 }
+
+static const char *const speed_names[] = {
+	[USB_SPEED_UNKNOWN] = "UNKNOWN",
+	[USB_SPEED_LOW] = "low-speed",
+	[USB_SPEED_FULL] = "full-speed",
+	[USB_SPEED_HIGH] = "high-speed",
+	[USB_SPEED_WIRELESS] = "wireless",
+	[USB_SPEED_SUPER] = "super-speed",
+};
+
+enum usb_device_speed usb_get_maximum_speed(int node)
+{
+	const void *fdt = gd->fdt_blob;

drivers/usb/common/common.c enum usb_dr_mode usb_get_dr_mode(int node) · line 60

Quoted 6 lines

drivers/usb/common/common.c +6 -0

+	const char *max_speed;
+	int i;
+
+	max_speed = fdt_getprop(fdt, node, "maximum-speed", NULL);
+	if (!max_speed) {
+		error("usb dr_mode not found\n");

Really dr_mode ?

drivers/usb/common/common.c enum usb_dr_mode usb_get_dr_mode(int node) · line 66

Quoted 7 lines

drivers/usb/common/common.c +6 -0

+		return USB_DR_MODE_UNKNOWN;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(speed_names); i++)
+		if (!strcmp(max_speed, speed_names[i]))
+			return i;

Shouldn't this somehow take into account the controller capabilities if the maximum-speed node is missing ?

Quoted 27 lines

drivers/usb/common/common.c +2 -0

+	return USB_SPEED_UNKNOWN;
+}

include/linux/usb/otg.h +9 -0

index 8f8ac6aeefe3..b61ef19b22f3 100644
@@ -26,4 +26,13 @@ enum usb_dr_mode {
  */
 enum usb_dr_mode usb_get_dr_mode(int node);
 
+/**
+ * usb_get_maximum_speed() - Get maximum speed for given device
+ * @node: Node offset to the given device
+ *
+ * The function gets phy interface string from property 'maximum-speed',
+ * and returns the correspondig enum usb_device_speed
+ */
+enum usb_device_speed usb_get_maximum_speed(int node);
+
 #endif /* __LINUX_USB_OTG_H */

scripts/Makefile.spl +0 -0

index ac3c2c7f1342..bffea085b329 100644
@@ -78,6 +78,7 @@ endif
 

scripts/Makefile.spl endif · line 81

Quoted 3 lines

scripts/Makefile.spl +1 -0

 libs-$(CONFIG_SPL_LIBDISK_SUPPORT) += disk/
 libs-y += drivers/
+libs-y += drivers/usb/common/

This looks weird, drivers/ is already included so why this explicit path addition ?

Quoted 5 lines

scripts/Makefile.spl +0 -0

 libs-$(CONFIG_SPL_USB_GADGET_SUPPORT) += drivers/usb/dwc3/
 libs-y += dts/
 libs-y += fs/

Quoted 44 lines · Marek Vasut

Quoted 43 lines · Vignesh R

From: Mugunthan V N <[email protected]>

Add support to get maximum speed from dt so that usb drivers makes use of it for DT parsing.

Signed-off-by Mugunthan V N <[email protected]>

Signed-off-by Vignesh R <[email protected]>

Reviewed-by Simon Glass <[email protected]>

---
 drivers/usb/common/common.c | 29 +++++++++++++++++++++++++++++
 include/linux/usb/otg.h     |  9 +++++++++
 scripts/Makefile.spl        |  1 +
 3 files changed, 39 insertions(+)

drivers/usb/common/common.c +14 -0

index 35c2dc18d955..c11689dc36fa 100644
@@ -10,6 +10,7 @@
 #include <common.h>
 #include <libfdt.h>
 #include <linux/usb/otg.h>
+#include <linux/usb/ch9.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 

@@ -38,3 +39,31 @@ enum usb_dr_mode usb_get_dr_mode(int node)
 
 	return USB_DR_MODE_UNKNOWN;
 }
+
+static const char *const speed_names[] = {
+	[USB_SPEED_UNKNOWN] = "UNKNOWN",
+	[USB_SPEED_LOW] = "low-speed",
+	[USB_SPEED_FULL] = "full-speed",
+	[USB_SPEED_HIGH] = "high-speed",
+	[USB_SPEED_WIRELESS] = "wireless",
+	[USB_SPEED_SUPER] = "super-speed",
+};
+
+enum usb_device_speed usb_get_maximum_speed(int node)
+{
+	const void *fdt = gd->fdt_blob;

Quoted 8 lines

drivers/usb/common/common.c enum usb_dr_mode usb_get_dr_mode(int node) · line 60

Quoted 6 lines

drivers/usb/common/common.c +6 -0

+	const char *max_speed;
+	int i;
+
+	max_speed = fdt_getprop(fdt, node, "maximum-speed", NULL);
+	if (!max_speed) {
+		error("usb dr_mode not found\n");

Really dr_mode ?

Sorry, will fix this.

Quoted 10 lines

drivers/usb/common/common.c enum usb_dr_mode usb_get_dr_mode(int node) · line 66

Quoted 7 lines

drivers/usb/common/common.c +6 -0

+		return USB_DR_MODE_UNKNOWN;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(speed_names); i++)
+		if (!strcmp(max_speed, speed_names[i]))
+			return i;

Shouldn't this somehow take into account the controller capabilities if the maximum-speed node is missing ?

Idea was, this is just a helper function and will return USB_SPEED_UNKNOWN when "maximum-speed" property is missing. The particular USB controller driver (like dwc3) can take decision of falling back to default capabilities.
I will leave this as is, but will update patch 6/13 to fall back to default capability when USB_DR_MODE_UNKNOWN is returned.

Quoted 26 lines

Quoted 26 lines

drivers/usb/common/common.c +2 -0

+	return USB_SPEED_UNKNOWN;
+}

include/linux/usb/otg.h +9 -0

index 8f8ac6aeefe3..b61ef19b22f3 100644
@@ -26,4 +26,13 @@ enum usb_dr_mode {
  */
 enum usb_dr_mode usb_get_dr_mode(int node);
 
+/**
+ * usb_get_maximum_speed() - Get maximum speed for given device
+ * @node: Node offset to the given device
+ *
+ * The function gets phy interface string from property 'maximum-speed',
+ * and returns the correspondig enum usb_device_speed
+ */
+enum usb_device_speed usb_get_maximum_speed(int node);
+
 #endif /* __LINUX_USB_OTG_H */

scripts/Makefile.spl +0 -0

index ac3c2c7f1342..bffea085b329 100644
@@ -78,6 +78,7 @@ endif
 

Quoted 6 lines

scripts/Makefile.spl endif · line 81

Quoted 3 lines

scripts/Makefile.spl +1 -0

 libs-$(CONFIG_SPL_LIBDISK_SUPPORT) += disk/
 libs-y += drivers/
+libs-y += drivers/usb/common/

This looks weird, drivers/ is already included so why this explicit path addition ?

Makefile under drivers/ neither includes usb/ directory nor does usb/ directory has a Makefile. Hence, above entry is needed.
I guess, this is how it is for SPL build.

From: Mugunthan V N <[email protected]>

Add a TI DWC3 peripheral driver with driver model support and the driver will be bound by the DWC3 wrapper driver based on the dr_mode device tree entry.

Signed-off-by Mugunthan V N <[email protected]>

Signed-off-by Vignesh R <[email protected]>

---
 drivers/usb/dwc3/core.c      |  57 +++++++++++++++
 drivers/usb/dwc3/core.h      |   6 ++
 drivers/usb/dwc3/dwc3-omap.c | 167 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 230 insertions(+)

drivers/usb/dwc3/core.c +26 -0

index 98102bd6b00a..a895f8fbddd3 100644
@@ -603,6 +603,8 @@ static void dwc3_core_exit_mode(struct dwc3 *dwc)
 
 #define DWC3_ALIGN_MASK		(16 - 1)
 
+#ifndef CONFIG_DM_USB
+
 /**
  * dwc3_uboot_init - dwc3 core uboot initialization code
  * @dwc3_dev: struct dwc3_device containing initialization data

@@ -789,3 +791,58 @@ MODULE_ALIAS("platform:dwc3");
 MODULE_AUTHOR("Felipe Balbi <[email protected]>");
 MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("DesignWare USB3 DRD Controller Driver");
+
+#else
+
+int dwc3_init(struct dwc3 *dwc)
+{
+	int ret;
+
+	dwc3_cache_hwparams(dwc);
+
+	ret = dwc3_alloc_event_buffers(dwc, DWC3_EVENT_BUFFERS_SIZE);
+	if (ret) {
+		dev_err(dwc->dev, "failed to allocate event buffers\n");
+		return -ENOMEM;
+	}
+
+	ret = dwc3_core_init(dwc);
+	if (ret) {
+		dev_err(dev, "failed to initialize core\n");
+		goto err0;
+	}
+
+	ret = dwc3_event_buffers_setup(dwc);
+	if (ret) {
+		dev_err(dwc->dev, "failed to setup event buffers\n");

drivers/usb/dwc3/core.c +21 -0

+	dwc3_event_buffers_cleanup(dwc);
+
+err1:
+	dwc3_core_exit(dwc);
+
+err0:
+	dwc3_free_event_buffers(dwc);
+
+	return ret;
+}
+
+void dwc3_remove(struct dwc3 *dwc)
+{
+	dwc3_core_exit_mode(dwc);
+	dwc3_event_buffers_cleanup(dwc);
+	dwc3_free_event_buffers(dwc);
+	dwc3_core_exit(dwc);
+	kfree(dwc->mem);
+}
+
+#endif

drivers/usb/dwc3/core.h +6 -0

index 72d2fcdd3f42..972628751697 100644
@@ -713,7 +713,11 @@ struct dwc3 {
 	/* device lock */
 	spinlock_t		lock;
 
+#ifndef CONFIG_DM_USB
 	struct device		*dev;
+#else
+	struct udevice		*dev;
+#endif
 
 	struct platform_device	*xhci;
 	struct resource		xhci_resources[DWC3_XHCI_RESOURCES_NUM];

@@ -988,6 +992,8 @@ struct dwc3_gadget_ep_cmd_params {
 
 /* prototypes */
 int dwc3_gadget_resize_tx_fifos(struct dwc3 *dwc);
+int dwc3_init(struct dwc3 *dwc);
+void dwc3_remove(struct dwc3 *dwc);
 
 #ifdef CONFIG_USB_DWC3_HOST
 int dwc3_host_init(struct dwc3 *dwc);

drivers/usb/dwc3/dwc3-omap.c +25 -0

index f18884f13392..b4dde726c6f3 100644
@@ -27,6 +27,23 @@
 #include <dwc3-uboot.h>
 
 #include "linux-compat.h"
+#include <linux/usb/ch9.h>
+#include <linux/usb/gadget.h>
+#include <ti-usb-phy-uboot.h>
+#include <usb.h>
+
+#include "core.h"
+
+#include <libfdt.h>
+#include <dm/device.h>
+#include <dm/uclass.h>
+#include <dm/lists.h>
+#include <dwc3-uboot.h>
+
+#include <asm/omap_common.h>
+#include "gadget.h"
+
+DECLARE_GLOBAL_DATA_PTR;
 
 /*
  * All these registers belong to OMAP's Wrapper around the

@@ -135,6 +152,12 @@ struct dwc3_omap {
 	u32			index;
 };
 
+struct omap_dwc3_priv {
+	struct dwc3_omap omap;
+	struct dwc3 dwc3;
+	struct ti_usb_phy_device phy_device;
+};
+
 static LIST_HEAD(dwc3_omap_list);
 
 static inline u32 dwc3_omap_readl(void __iomem *base, u32 offset)

@@ -362,6 +385,8 @@ static void dwc3_omap_set_utmi_mode(struct dwc3_omap *omap, int utmi_mode)
 	dwc3_omap_write_utmi_status(omap, reg);
 }
 
+#ifndef CONFIG_DM_USB
+
 /**
  * dwc3_omap_uboot_init - dwc3 omap uboot initialization code
  * @dev: struct dwc3_omap_device containing initialization data

drivers/usb/dwc3/dwc3-omap.c +138 -0

+{
+	struct omap_dwc3_priv *priv;
+	struct dwc3_omap *omap;
+	struct dwc3 *dwc;
+	struct udevice *dev;
+	u32 status;
+	int ret;
+
+	ret = uclass_first_device(UCLASS_USB_DEV_GENERIC, &dev);
+	if (!dev || ret) {
+		error("No USB device found\n");
+		return -ENODEV;
+	}
+
+	priv = dev_get_priv(dev);
+	omap = &priv->omap;
+	dwc = &priv->dwc3;
+
+	status = dwc3_omap_interrupt(-1, omap);
+	if (status)
+		dwc3_gadget_uboot_handle_interrupt(dwc);
+
+	return 0;
+}
+
+static int dwc3_omap_peripheral_probe(struct udevice *dev)
+{
+	struct omap_dwc3_priv *priv = dev_get_priv(dev);
+	struct dwc3_omap *omap = &priv->omap;
+	struct dwc3 *dwc = &priv->dwc3;
+	u32 reg;
+	int ret;
+
+	enable_usb_clocks(0);
+
+	/* Initialize usb phy */
+	ret = ti_usb_phy_uboot_init(&priv->phy_device);
+	if (ret)
+		return ret;
+
+	dwc3_omap_map_offset(omap);
+	dwc3_omap_set_utmi_mode(omap, DWC3_OMAP_UTMI_MODE_SW);
+
+	/* check the DMA Status */
+	reg = dwc3_omap_readl(omap->base, USBOTGSS_SYSCONFIG);
+	omap->dma_status = !!(reg & USBOTGSS_SYSCONFIG_DMADISABLE);
+
+	dwc3_omap_enable_irqs(omap);
+
+	dwc3_omap_set_mailbox(omap, OMAP_DWC3_ID_GROUND);
+
+	/* default to highest possible threshold */
+	dwc->lpm_nyet_threshold = 0xff;
+	/*
+	 * default to assert utmi_sleep_n and use maximum allowed HIRD
+	 * threshold value of 0b1100
+	 */
+	dwc->hird_threshold = 12;
+	/* default to -3.5dB de-emphasis */
+	dwc->tx_de_emphasis = 1;
+
+	dwc->needs_fifo_resize = false;
+	dwc->index = 0;
+
+	return dwc3_init(dwc);
+}
+
+static int dwc3_omap_peripheral_remove(struct udevice *dev)
+{
+	struct omap_dwc3_priv *priv = dev_get_priv(dev);
+	struct dwc3_omap *omap = &priv->omap;
+	struct dwc3 *dwc = &priv->dwc3;
+
+	dwc3_omap_disable_irqs(omap);
+	dwc3_remove(dwc);
+
+	return 0;
+}
+
+static int dwc3_omap_ofdata_to_platdata(struct udevice *dev)
+{
+	struct omap_dwc3_priv *priv = dev_get_priv(dev);
+	const void *fdt = gd->fdt_blob;
+	int node = dev_of_offset(dev);
+	int ctrlmodnode;
+	int physnode;
+
+	priv->omap.base = map_physmem(fdtdec_get_addr(fdt,
+				dev_of_offset(dev->parent), "reg"), 0,
+				MAP_NOCACHE);
+
+	priv->dwc3.regs = devfdt_map_physmem(dev,
+					     sizeof(priv->dwc3.regs_size));
+	priv->dwc3.regs += DWC3_GLOBALS_REGS_START;
+
+	physnode = fdtdec_lookup_phandle(fdt, node, "phys");
+	ctrlmodnode = fdtdec_lookup_phandle(fdt, physnode, "ctrl-module");
+	priv->phy_device.usb2_phy_power =
+		map_physmem(fdtdec_get_addr(fdt, ctrlmodnode, "reg"),
+			    0, MAP_NOCACHE);
+	priv->phy_device.index = 0;
+
+	priv->dwc3.maximum_speed = usb_get_maximum_speed(node);
+	if (priv->dwc3.maximum_speed < 0) {
+		error("Invalid usb maximum speed\n");
+		return priv->dwc3.maximum_speed;
+	}
+
+	return 0;
+}
+
+static int dwc3_omap_peripheral_ofdata_to_platdata(struct udevice *dev)
+{
+	struct omap_dwc3_priv *priv = dev_get_priv(dev);
+	int ret;
+
+	ret = dwc3_omap_ofdata_to_platdata(dev);
+	if (ret) {
+		error("platform dt parse error\n");
+		return ret;
+	}
+
+	priv->dwc3.dr_mode = USB_DR_MODE_PERIPHERAL;
+
+	return 0;
+}
+
+U_BOOT_DRIVER(dwc3_omap_peripheral) = {
+	.name	= "dwc3-omap-peripheral",
+	.id	= UCLASS_USB_DEV_GENERIC,
+	.ofdata_to_platdata = dwc3_omap_peripheral_ofdata_to_platdata,
+	.probe = dwc3_omap_peripheral_probe,
+	.remove = dwc3_omap_peripheral_remove,
+	.platdata_auto_alloc_size = sizeof(struct usb_platdata),
+	.priv_auto_alloc_size = sizeof(struct omap_dwc3_priv),
+	.flags	= DM_FLAG_ALLOC_PRIV_DMA,
+};
+#endif /* CONFIG_DM_USB */

Quoted 59 lines · Vignesh R

From: Mugunthan V N <[email protected]>

Add a TI DWC3 peripheral driver with driver model support and the driver will be bound by the DWC3 wrapper driver based on the dr_mode device tree entry.

Signed-off-by Mugunthan V N <[email protected]>

Signed-off-by Vignesh R <[email protected]>

---
 drivers/usb/dwc3/core.c      |  57 +++++++++++++++
 drivers/usb/dwc3/core.h      |   6 ++
 drivers/usb/dwc3/dwc3-omap.c | 167 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 230 insertions(+)

drivers/usb/dwc3/core.c +30 -0

index 98102bd6b00a..a895f8fbddd3 100644
@@ -603,6 +603,8 @@ static void dwc3_core_exit_mode(struct dwc3 *dwc)
 
 #define DWC3_ALIGN_MASK		(16 - 1)
 
+#ifndef CONFIG_DM_USB
+
 /**
  * dwc3_uboot_init - dwc3 core uboot initialization code
  * @dwc3_dev: struct dwc3_device containing initialization data

@@ -789,3 +791,58 @@ MODULE_ALIAS("platform:dwc3");
 MODULE_AUTHOR("Felipe Balbi <[email protected]>");
 MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("DesignWare USB3 DRD Controller Driver");
+
+#else
+
+int dwc3_init(struct dwc3 *dwc)
+{
+	int ret;
+
+	dwc3_cache_hwparams(dwc);
+
+	ret = dwc3_alloc_event_buffers(dwc, DWC3_EVENT_BUFFERS_SIZE);
+	if (ret) {
+		dev_err(dwc->dev, "failed to allocate event buffers\n");
+		return -ENOMEM;
+	}
+
+	ret = dwc3_core_init(dwc);
+	if (ret) {
+		dev_err(dev, "failed to initialize core\n");
+		goto err0;
+	}
+
+	ret = dwc3_event_buffers_setup(dwc);
+	if (ret) {
+		dev_err(dwc->dev, "failed to setup event buffers\n");
+		goto err1;
+	}
+
+	ret = dwc3_core_init_mode(dwc);

drivers/usb/dwc3/core.c MODULE_ALIAS("platform:dwc3"); · line 827

Quoted 6 lines

drivers/usb/dwc3/core.c +6 -0

+	if (ret)
+		goto err2;
+
+	return 0;
+
+err2:

Use more descriptive label names please ...

Quoted 101 lines

drivers/usb/dwc3/core.c +21 -0

+	dwc3_event_buffers_cleanup(dwc);
+
+err1:
+	dwc3_core_exit(dwc);
+
+err0:
+	dwc3_free_event_buffers(dwc);
+
+	return ret;
+}
+
+void dwc3_remove(struct dwc3 *dwc)
+{
+	dwc3_core_exit_mode(dwc);
+	dwc3_event_buffers_cleanup(dwc);
+	dwc3_free_event_buffers(dwc);
+	dwc3_core_exit(dwc);
+	kfree(dwc->mem);
+}
+
+#endif

drivers/usb/dwc3/core.h +6 -0

index 72d2fcdd3f42..972628751697 100644
@@ -713,7 +713,11 @@ struct dwc3 {
 	/* device lock */
 	spinlock_t		lock;
 
+#ifndef CONFIG_DM_USB
 	struct device		*dev;
+#else
+	struct udevice		*dev;
+#endif
 
 	struct platform_device	*xhci;
 	struct resource		xhci_resources[DWC3_XHCI_RESOURCES_NUM];

@@ -988,6 +992,8 @@ struct dwc3_gadget_ep_cmd_params {
 
 /* prototypes */
 int dwc3_gadget_resize_tx_fifos(struct dwc3 *dwc);
+int dwc3_init(struct dwc3 *dwc);
+void dwc3_remove(struct dwc3 *dwc);
 
 #ifdef CONFIG_USB_DWC3_HOST
 int dwc3_host_init(struct dwc3 *dwc);

drivers/usb/dwc3/dwc3-omap.c +25 -0

index f18884f13392..b4dde726c6f3 100644
@@ -27,6 +27,23 @@
 #include <dwc3-uboot.h>
 
 #include "linux-compat.h"
+#include <linux/usb/ch9.h>
+#include <linux/usb/gadget.h>
+#include <ti-usb-phy-uboot.h>
+#include <usb.h>
+
+#include "core.h"
+
+#include <libfdt.h>
+#include <dm/device.h>
+#include <dm/uclass.h>
+#include <dm/lists.h>
+#include <dwc3-uboot.h>
+
+#include <asm/omap_common.h>
+#include "gadget.h"
+
+DECLARE_GLOBAL_DATA_PTR;
 
 /*
  * All these registers belong to OMAP's Wrapper around the

@@ -135,6 +152,12 @@ struct dwc3_omap {
 	u32			index;
 };
 
+struct omap_dwc3_priv {
+	struct dwc3_omap omap;
+	struct dwc3 dwc3;
+	struct ti_usb_phy_device phy_device;
+};
+
 static LIST_HEAD(dwc3_omap_list);
 
 static inline u32 dwc3_omap_readl(void __iomem *base, u32 offset)

@@ -362,6 +385,8 @@ static void dwc3_omap_set_utmi_mode(struct dwc3_omap *omap, int utmi_mode)
 	dwc3_omap_write_utmi_status(omap, reg);
 }
 
+#ifndef CONFIG_DM_USB
+
 /**
  * dwc3_omap_uboot_init - dwc3 omap uboot initialization code
  * @dev: struct dwc3_omap_device containing initialization data

@@ -462,3 +487,145 @@ MODULE_ALIAS("platform:omap-dwc3");
 MODULE_AUTHOR("Felipe Balbi <[email protected]>");
 MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("DesignWare USB3 OMAP Glue Layer");

drivers/usb/dwc3/dwc3-omap.c MODULE_ALIAS("platform:omap-dwc3"); · line 493

Quoted 4 lines

drivers/usb/dwc3/dwc3-omap.c +4 -0

+
+#else
+
+int usb_gadget_handle_interrupts(int index)

Can this be made somehow more generic , so that the core code would contain the basic interrupt handling and probe routines and the various SoC-specific drivers would add their specific bits to it ?

Quoted 16 lines

drivers/usb/dwc3/dwc3-omap.c +16 -0

+{
+	struct omap_dwc3_priv *priv;
+	struct dwc3_omap *omap;
+	struct dwc3 *dwc;
+	struct udevice *dev;
+	u32 status;
+	int ret;
+
+	ret = uclass_first_device(UCLASS_USB_DEV_GENERIC, &dev);
+	if (!dev || ret) {
+		error("No USB device found\n");
+		return -ENODEV;
+	}
+
+	priv = dev_get_priv(dev);
+	omap = &priv->omap;

drivers/usb/dwc3/dwc3-omap.c MODULE_ALIAS("platform:omap-dwc3"); · line 517

Quoted 8 lines

drivers/usb/dwc3/dwc3-omap.c +8 -0

+	dwc = &priv->dwc3;
+
+	status = dwc3_omap_interrupt(-1, omap);
+	if (status)
+		dwc3_gadget_uboot_handle_interrupt(dwc);
+
+	return 0;
+}

Quoted 7 lines

Quoted 7 lines

/**
* dwc3_omap_uboot_init - dwc3 omap uboot initialization code
* @dev: struct dwc3_omap_device containing initialization data

drivers/usb/dwc3/dwc3-omap.c +0 -0

@@ -462,3 +487,145 @@ MODULE_ALIAS("platform:omap-dwc3");
 MODULE_AUTHOR("Felipe Balbi <[email protected]>");
 MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("DesignWare USB3 OMAP Glue Layer");

Quoted 9 lines

drivers/usb/dwc3/dwc3-omap.c MODULE_ALIAS("platform:omap-dwc3"); · line 493

Quoted 4 lines

drivers/usb/dwc3/dwc3-omap.c +4 -0

+
+#else
+
+int usb_gadget_handle_interrupts(int index)

Can this be made somehow more generic , so that the core code would contain the basic interrupt handling and probe routines and the various SoC-specific drivers would add their specific bits to it ?

How about moving this to drivers/usb/dwc3/gadget.c and then provide a callback to each of SoC specific drivers?

On 06/14/2017 02:25 PM, Vignesh R wrote:
[...]

Quoted 17 lines

Quoted 16 lines

drivers/usb/dwc3/dwc3-omap.c MODULE_ALIAS("platform:omap-dwc3"); · line 493

Quoted 11 lines

/**
* dwc3_omap_uboot_init - dwc3 omap uboot initialization code
* @dev: struct dwc3_omap_device containing initialization data

drivers/usb/dwc3/dwc3-omap.c +4 -0

@@ -462,3 +487,145 @@ MODULE_ALIAS("platform:omap-dwc3");
 MODULE_AUTHOR("Felipe Balbi <[email protected]>");
 MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("DesignWare USB3 OMAP Glue Layer");
+
+#else
+
+int usb_gadget_handle_interrupts(int index)

Can this be made somehow more generic , so that the core code would contain the basic interrupt handling and probe routines and the various SoC-specific drivers would add their specific bits to it ?

Quoted 3 lines

How about moving this to drivers/usb/dwc3/gadget.c and then provide a callback to each of SoC specific drivers?

That could work.

Hi Marek,

Quoted 19 lines · Marek Vasut

On 06/14/2017 02:25 PM, Vignesh R wrote:
[...]

Quoted 17 lines

Quoted 16 lines

drivers/usb/dwc3/dwc3-omap.c MODULE_ALIAS("platform:omap-dwc3"); · line 493

Quoted 11 lines

/**
* dwc3_omap_uboot_init - dwc3 omap uboot initialization code
* @dev: struct dwc3_omap_device containing initialization data

drivers/usb/dwc3/dwc3-omap.c +4 -0

@@ -462,3 +487,145 @@ MODULE_ALIAS("platform:omap-dwc3");
 MODULE_AUTHOR("Felipe Balbi <[email protected]>");
 MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("DesignWare USB3 OMAP Glue Layer");
+
+#else
+
+int usb_gadget_handle_interrupts(int index)

Can this be made somehow more generic , so that the core code would contain the basic interrupt handling and probe routines and the various SoC-specific drivers would add their specific bits to it ?

Quoted 4 lines

Quoted 3 lines

How about moving this to drivers/usb/dwc3/gadget.c and then provide a callback to each of SoC specific drivers?

That could work.

Sorry, looking at this further I dont see a easy way of doing this.

All gadget drivers like ether.c or f_mass_storage.c call usb_gadget_handle_interrupts() just passing the index of the USB instance. This does not help at all in dm case. What we would need is usb_gadget_handle_interrupts() to provide at least the usb_gadget instance as parameter from which we could derive controller specific structure using container_of(). And then, we could call the SoC specific isr callback.
This would require modifying all gadget driver like ether.c to call a different function instead of usb_gadget_handle_interrupts() when DM_USB is used.

I see MUSB driver uses a global pointer to musb struct (see drivers/usb/musb-new/musb_uboot.c::usb_gadget_handle_interrupts()), but I dont think thats a good option.

Let me know your preference. Any suggestion appreciated. Thanks!

Quoted 30 lines

Hi Marek,

Quoted 20 lines · Marek Vasut

On 06/14/2017 02:25 PM, Vignesh R wrote:
[...]

Quoted 18 lines

Quoted 17 lines

drivers/usb/dwc3/dwc3-omap.c MODULE_ALIAS("platform:omap-dwc3"); · line 493

Quoted 11 lines

/**
* dwc3_omap_uboot_init - dwc3 omap uboot initialization code
* @dev: struct dwc3_omap_device containing initialization data

drivers/usb/dwc3/dwc3-omap.c +4 -0

@@ -462,3 +487,145 @@ MODULE_ALIAS("platform:omap-dwc3");
 MODULE_AUTHOR("Felipe Balbi <[email protected]>");
 MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("DesignWare USB3 OMAP Glue Layer");
+
+#else
+
+int usb_gadget_handle_interrupts(int index)

Can this be made somehow more generic , so that the core code would contain the basic interrupt handling and probe routines and the various SoC-specific drivers would add their specific bits to it ?

Quoted 4 lines

Quoted 3 lines

How about moving this to drivers/usb/dwc3/gadget.c and then provide a callback to each of SoC specific drivers?

That could work.

Sorry, looking at this further I dont see a easy way of doing this.

Quoted 10 lines

All gadget drivers like ether.c or f_mass_storage.c call usb_gadget_handle_interrupts() just passing the index of the USB instance. This does not help at all in dm case. What we would need is usb_gadget_handle_interrupts() to provide at least the usb_gadget instance as parameter from which we could derive controller specific structure using container_of(). And then, we could call the SoC specific isr callback.
This would require modifying all gadget driver like ether.c to call a different function instead of usb_gadget_handle_interrupts() when DM_USB is used.

This is something to consult with Lukasz then.

Quoted 4 lines

I see MUSB driver uses a global pointer to musb struct (see drivers/usb/musb-new/musb_uboot.c::usb_gadget_handle_interrupts()), but I dont think thats a good option.

Nope, that's not a good option, just like any global stuff in drivers.

Quoted 2 lines

Let me know your preference. Any suggestion appreciated. Thanks!

Hi Marek, Vignesh,

Quoted 12 lines

Quoted 11 lines

All gadget drivers like ether.c or f_mass_storage.c call usb_gadget_handle_interrupts() just passing the index of the USB instance. This does not help at all in dm case. What we would need is usb_gadget_handle_interrupts() to provide at least the usb_gadget instance as parameter from which we could derive controller specific structure using container_of(). And then, we could call the SoC specific isr callback.
This would require modifying all gadget driver like ether.c to call a different function instead of usb_gadget_handle_interrupts() when DM_USB is used.

Quoted 1 line

This is something to consult with Lukasz then.

And it seems that we are heading to adding "gadget" infrastructure to DM.....

I will do my best to provide some ideas for this task.....

Hi,

Quoted 1 line · Lukasz Majewski

Hi Marek, Vignesh,

[...]

Quoted 14 lines

Quoted 13 lines

Quoted 11 lines

All gadget drivers like ether.c or f_mass_storage.c call usb_gadget_handle_interrupts() just passing the index of the USB instance. This does not help at all in dm case. What we would need is usb_gadget_handle_interrupts() to provide at least the usb_gadget instance as parameter from which we could derive controller specific structure using container_of(). And then, we could call the SoC specific isr callback.
This would require modifying all gadget driver like ether.c to call a different function instead of usb_gadget_handle_interrupts() when DM_USB is used.

This is something to consult with Lukasz then.

Quoted 3 lines

And it seems that we are heading to adding "gadget" infrastructure to DM.....

Yes, U-Boot is moving to DM for good and this has cascading effect. I was actually trying to enable DM_ETH on some TI platforms which forced me to move USB_ETH to DM as well and therefore seems like USB gadget framework needs tweaks to adapt to DM...

Quoted 3 lines

I will do my best to provide some ideas for this task.....

Thanks!

Hi Vignesh,

Quoted 19 lines

Hi,

Quoted 1 line · Lukasz Majewski

Hi Marek, Vignesh,

[...]

Quoted 14 lines

Quoted 13 lines

Quoted 11 lines

All gadget drivers like ether.c or f_mass_storage.c call usb_gadget_handle_interrupts() just passing the index of the USB instance. This does not help at all in dm case. What we would need is usb_gadget_handle_interrupts() to provide at least the usb_gadget instance as parameter from which we could derive controller specific structure using container_of(). And then, we could call the SoC specific isr callback.
This would require modifying all gadget driver like ether.c to call a different function instead of usb_gadget_handle_interrupts() when DM_USB is used.

This is something to consult with Lukasz then.

Quoted 8 lines

Quoted 3 lines

And it seems that we are heading to adding "gadget" infrastructure to DM.....

Yes, U-Boot is moving to DM for good and this has cascading effect. I was actually trying to enable DM_ETH on some TI platforms which forced me to move USB_ETH to DM as well and therefore seems like USB gadget framework needs tweaks to adapt to DM...

I've sketched following plan for gadget conversion:

1. Each u-boot command (dfu, ums, thor and in the future rockchip I hope), which uses gadget goes through g_dnl_{register|unregister}, so the idea is to add this driver first to DM.

2. Afterwards, we could add functions as children of g_dnl.

This would be easily modeled in Kconfig (to have g_dnl - gadget - menu with submenu to chose the USB function - e.g. f_dfu*).

However, we also need to take care of several UDC (USB device controller) drivers including also the "composite" usb layer.

This would be tougher to do since there are many udc drivers - but it should be possible to separate DM's UDC drivers and g_dnl/function code.

Another problem is that some archs use gadgets (RNDIS?) without g_dnl and composite - on top of UDC driver (like musb).....

For example:

board/ti/beagle/beagle.c -> board_eth_init()

		|
	       \|/
drivers/usb/gadget/ether.c -> usb_eth_initialize()
[ether.c seems to partially support DM]
		|
	       \|/

(also in the ether.c)
_usb_eth_init() in which we loop on
usb_gadget_handle_interrupts()

From what I see, the ether.c now supports DM and legacy code, so some work has been already done for DM....

Quoted 36 lines · Lukasz Majewski

Hi Vignesh,

Quoted 19 lines

Hi,

Quoted 1 line · Lukasz Majewski

Hi Marek, Vignesh,

[...]

Quoted 14 lines

Quoted 13 lines

Quoted 11 lines

All gadget drivers like ether.c or f_mass_storage.c call usb_gadget_handle_interrupts() just passing the index of the USB instance. This does not help at all in dm case. What we would need is usb_gadget_handle_interrupts() to provide at least the usb_gadget instance as parameter from which we could derive controller specific structure using container_of(). And then, we could call the SoC specific isr callback.
This would require modifying all gadget driver like ether.c to call a different function instead of usb_gadget_handle_interrupts() when DM_USB is used.

This is something to consult with Lukasz then.

Quoted 8 lines

Quoted 3 lines

And it seems that we are heading to adding "gadget" infrastructure to DM.....

Yes, U-Boot is moving to DM for good and this has cascading effect. I was actually trying to enable DM_ETH on some TI platforms which forced me to move USB_ETH to DM as well and therefore seems like USB gadget framework needs tweaks to adapt to DM...

I've sketched following plan for gadget conversion:

1. Each u-boot command (dfu, ums, thor and in the future rockchip I hope), which uses gadget goes through g_dnl_{register|unregister}, so the idea is to add this driver first to DM.

Quoted 5 lines

2. Afterwards, we could add functions as children of g_dnl.

This would be easily modeled in Kconfig (to have g_dnl - gadget - menu with submenu to chose the USB function - e.g. f_dfu*).

Wondering, if there is case where more than one functions may be used like f_dfu and f_mass_storage?

Like a single defconfig may want to
have both f_dfu and f_mass_storage enabled?

Quoted 8 lines

However, we also need to take care of several UDC (USB device controller) drivers including also the "composite" usb layer.

This would be tougher to do since there are many udc drivers - but it should be possible to separate DM's UDC drivers and g_dnl/function code.

Quoted 3 lines

Another problem is that some archs use gadgets (RNDIS?) without g_dnl and composite - on top of UDC driver (like musb).....

Yes, rndis does not use g_dnl. Both MUSB and DWC3 gadget seem to use UDC w/o g_dnl/composite. I guess, we will have to either support RNDIS directly with UDC or convert MUSB/DWC3 gadget interface as well as convert ether.c to g_dnl function.

Quoted 15 lines

For example:

board/ti/beagle/beagle.c -> board_eth_init()

		|
	       \|/
drivers/usb/gadget/ether.c -> usb_eth_initialize()
[ether.c seems to partially support DM]
		|
	       \|/

(also in the ether.c)
_usb_eth_init() in which we loop on
usb_gadget_handle_interrupts()

Quoted 3 lines

From what I see, the ether.c now supports DM and legacy code, so some work has been already done for DM....

Yeah, I think this was done as part of making MUSB DM conversion. RNDIS boot is one the boot mode for many TI platforms and is used quite often.

Legacy code is what is largely in use, am335x has been recently moved to use DM based RNDIS(although I feel its not complete and working yet). I guess once UDC is moved DM, we can see how ether.c can be integrated with it.

Thanks for looking into this!

Quoted 39 lines · On Thu, 22 Jun 2017 17:42:38 +0530 Vignesh R <[email protected]> wrote:

Quoted 36 lines · Lukasz Majewski

Hi Vignesh,

Quoted 19 lines

Hi,

Quoted 1 line · Lukasz Majewski

Hi Marek, Vignesh,

[...]

Quoted 14 lines

Quoted 13 lines

Quoted 11 lines

All gadget drivers like ether.c or f_mass_storage.c call usb_gadget_handle_interrupts() just passing the index of the USB instance. This does not help at all in dm case. What we would need is usb_gadget_handle_interrupts() to provide at least the usb_gadget instance as parameter from which we could derive controller specific structure using container_of(). And then, we could call the SoC specific isr callback.
This would require modifying all gadget driver like ether.c to call a different function instead of usb_gadget_handle_interrupts() when DM_USB is used.

This is something to consult with Lukasz then.

Quoted 8 lines

Quoted 3 lines

And it seems that we are heading to adding "gadget" infrastructure to DM.....

Yes, U-Boot is moving to DM for good and this has cascading effect. I was actually trying to enable DM_ETH on some TI platforms which forced me to move USB_ETH to DM as well and therefore seems like USB gadget framework needs tweaks to adapt to DM...

I've sketched following plan for gadget conversion:

1. Each u-boot command (dfu, ums, thor and in the future rockchip I hope), which uses gadget goes through g_dnl_{register|unregister}, so the idea is to add this driver first to DM.

Quoted 8 lines

Quoted 5 lines

2. Afterwards, we could add functions as children of g_dnl.

This would be easily modeled in Kconfig (to have g_dnl - gadget - menu with submenu to chose the USB function - e.g. f_dfu*).

Wondering, if there is case where more than one functions may be used like f_dfu and f_mass_storage?

The "composite" layer was supposed to provide that.

Quoted 2 lines

Like a single defconfig may want to
have both f_dfu and f_mass_storage enabled?

When we developed the g_dnl/f_* code we wanted to have support for many functions.

However, finally, we only implemented the single function since u-boot is a single thread SW (and we had some problems with DFU/ODIN endpoints assignment, IIRC).

Theoretically it should be possible to have many functions enabled.

Quoted 12 lines

Quoted 11 lines

However, we also need to take care of several UDC (USB device controller) drivers including also the "composite" usb layer.

This would be tougher to do since there are many udc drivers - but it should be possible to separate DM's UDC drivers and g_dnl/function code.

Another problem is that some archs use gadgets (RNDIS?) without g_dnl and composite - on top of UDC driver (like musb).....

Quoted 4 lines

Yes, rndis does not use g_dnl. Both MUSB and DWC3 gadget seem to use UDC w/o g_dnl/composite. I guess, we will have to either support RNDIS directly with UDC or convert MUSB/DWC3 gadget interface as well as convert ether.c to g_dnl function.

I would opt for latter option (f_rndis*).

Quoted 15 lines

Quoted 15 lines

For example:

board/ti/beagle/beagle.c -> board_eth_init()

		|
	       \|/
drivers/usb/gadget/ether.c -> usb_eth_initialize()
[ether.c seems to partially support DM]
		|
	       \|/

(also in the ether.c)
_usb_eth_init() in which we loop on
usb_gadget_handle_interrupts()

Quoted 7 lines

Quoted 3 lines

From what I see, the ether.c now supports DM and legacy code, so some work has been already done for DM....

Yeah, I think this was done as part of making MUSB DM conversion. RNDIS boot is one the boot mode for many TI platforms and is used quite often.

Ok.

Quoted 4 lines

Legacy code is what is largely in use, am335x has been recently moved to use DM based RNDIS(although I feel its not complete and working yet). I guess once UDC is moved DM, we can see how ether.c can be integrated with it.

And other UDCs should be moved as well.... (like dwc[23]).

Quoted 58 lines · Lukasz Majewski

Quoted 41 lines · On Thu, 22 Jun 2017 17:42:38 +0530 Vignesh R <[email protected]> wrote:

Quoted 38 lines · Lukasz Majewski

Hi Vignesh,

Quoted 23 lines

Hi,

Quoted 1 line · Lukasz Majewski

Hi Marek, Vignesh,

[...]

Quoted 12 lines

Quoted 12 lines

Quoted 11 lines

All gadget drivers like ether.c or f_mass_storage.c call usb_gadget_handle_interrupts() just passing the index of the USB instance. This does not help at all in dm case. What we would need is usb_gadget_handle_interrupts() to provide at least the usb_gadget instance as parameter from which we could derive controller specific structure using container_of(). And then, we could call the SoC specific isr callback. This would require modifying all gadget driver like ether.c to call a different function instead of usb_gadget_handle_interrupts() when DM_USB is used.

Quoted 5 lines

Quoted 1 line

This is something to consult with Lukasz then.

And it seems that we are heading to adding "gadget" infrastructure to DM.....

Quoted 5 lines

Yes, U-Boot is moving to DM for good and this has cascading effect. I was actually trying to enable DM_ETH on some TI platforms which forced me to move USB_ETH to DM as well and therefore seems like USB gadget framework needs tweaks to adapt to DM...

I've sketched following plan for gadget conversion:

1. Each u-boot command (dfu, ums, thor and in the future rockchip I hope), which uses gadget goes through g_dnl_{register|unregister}, so the idea is to add this driver first to DM.

Quoted 8 lines

Quoted 5 lines

2. Afterwards, we could add functions as children of g_dnl.

This would be easily modeled in Kconfig (to have g_dnl - gadget - menu with submenu to chose the USB function - e.g. f_dfu*).

Wondering, if there is case where more than one functions may be used like f_dfu and f_mass_storage?

The "composite" layer was supposed to provide that.

Quoted 2 lines

Like a single defconfig may want to
have both f_dfu and f_mass_storage enabled?

Quoted 9 lines

When we developed the g_dnl/f_* code we wanted to have support for many functions.

However, finally, we only implemented the single function since u-boot is a single thread SW (and we had some problems with DFU/ODIN endpoints assignment, IIRC).

Theoretically it should be possible to have many functions enabled.

Ok.

Quoted 12 lines

Quoted 12 lines

Quoted 11 lines

However, we also need to take care of several UDC (USB device controller) drivers including also the "composite" usb layer.

This would be tougher to do since there are many udc drivers - but it should be possible to separate DM's UDC drivers and g_dnl/function code.

Another problem is that some archs use gadgets (RNDIS?) without g_dnl and composite - on top of UDC driver (like musb).....

Quoted 7 lines

Quoted 4 lines

Yes, rndis does not use g_dnl. Both MUSB and DWC3 gadget seem to use UDC w/o g_dnl/composite. I guess, we will have to either support RNDIS directly with UDC or convert MUSB/DWC3 gadget interface as well as convert ether.c to g_dnl function.

I would opt for latter option (f_rndis*).

I agree...

Quoted 30 lines

Quoted 15 lines

Quoted 15 lines

For example:

board/ti/beagle/beagle.c -> board_eth_init()

		|
	       \|/
drivers/usb/gadget/ether.c -> usb_eth_initialize()
[ether.c seems to partially support DM]
		|
	       \|/

(also in the ether.c)
_usb_eth_init() in which we loop on
usb_gadget_handle_interrupts()

Quoted 7 lines

Quoted 3 lines

From what I see, the ether.c now supports DM and legacy code, so some work has been already done for DM....

Yeah, I think this was done as part of making MUSB DM conversion. RNDIS boot is one the boot mode for many TI platforms and is used quite often.

Ok.

Quoted 4 lines

Legacy code is what is largely in use, am335x has been recently moved to use DM based RNDIS(although I feel its not complete and working yet). I guess once UDC is moved DM, we can see how ether.c can be integrated with it.

Quoted 2 lines

And other UDCs should be moved as well.... (like dwc[23]).

yes, all UDCs needs to moved..
BTW, what platform would you be starting these migration on?

Hi Vignesh,

Quoted 10 lines

Quoted 5 lines

Quoted 1 line

This is something to consult with Lukasz then.

And it seems that we are heading to adding "gadget" infrastructure to DM.....

Yes, U-Boot is moving to DM for good and this has cascading effect. I was actually trying to enable DM_ETH on some TI platforms which forced me to move USB_ETH to DM as well and therefore seems like USB gadget framework needs tweaks to adapt to DM...

I've sketched following plan for gadget conversion:

1. Each u-boot command (dfu, ums, thor and in the future rockchip I hope), which uses gadget goes through g_dnl_{register|unregister}, so the idea is to add this driver first to DM.

2. Afterwards, we could add functions as children of g_dnl.

This would be easily modeled in Kconfig (to have g_dnl - gadget - menu with submenu to chose the USB function - e.g. f_dfu*).

However, we also need to take care of several UDC (USB device controller) drivers including also the "composite" usb layer.

This would be tougher to do since there are many udc drivers - but it should be possible to separate DM's UDC drivers and g_dnl/function code.

Another problem is that some archs use gadgets (RNDIS?) without g_dnl and composite - on top of UDC driver (like musb).....

For example:

board/ti/beagle/beagle.c -> board_eth_init()

		|
	       \|/
drivers/usb/gadget/ether.c -> usb_eth_initialize()
[ether.c seems to partially support DM]
		|
	       \|/

(also in the ether.c)
_usb_eth_init() in which we loop on
usb_gadget_handle_interrupts()

From what I see, the ether.c now supports DM and legacy code, so some work has been already done for DM....

Quoted 3 lines

Quoted 3 lines

I will do my best to provide some ideas for this task.....

From: Mugunthan V N <[email protected]>

Add a misc driver for DWC3 wrapper, so that based on dr_mode the USB devices can bind to USB host or USB device drivers.

Based on dr_mode property, the device node needs to bind to either "host" driver or "peripheral" driver. This wrapper will on bind read dr_mode property and appropriately associate USB DT node with DWC3 peripheral driver or host driver.

This is similar to what exists today for MUSB and is instantiated by arch_misc_init() function for am33xx and am43xx.

Signed-off-by Mugunthan V N <[email protected]>

Signed-off-by Vignesh R <[email protected]>

---
 drivers/usb/dwc3/dwc3-omap.c | 49 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/usb/dwc3/gadget.c    |  2 +-
 2 files changed, 50 insertions(+), 1 deletion(-)

drivers/usb/dwc3/dwc3-omap.c +49 -0

index b4dde726c6f3..b7daf499515d 100644
@@ -628,4 +628,53 @@ U_BOOT_DRIVER(dwc3_omap_peripheral) = {
 	.priv_auto_alloc_size = sizeof(struct omap_dwc3_priv),
 	.flags	= DM_FLAG_ALLOC_PRIV_DMA,
 };
+
+static int ti_dwc3_wrapper_bind(struct udevice *parent)
+{
+	ofnode node;
+	int ret;
+
+	dev_for_each_subnode(node, parent) {
+		const char *name = ofnode_get_name(node);
+		enum usb_dr_mode dr_mode;
+		struct udevice *dev;
+
+		if (strncmp(name, "usb@", 4))
+			continue;
+
+		dr_mode = usb_get_dr_mode(ofnode_to_offset(node));
+		switch (dr_mode) {
+		case USB_DR_MODE_PERIPHERAL:
+		case USB_DR_MODE_OTG:
+			/* Bind MUSB device */
+			ret = device_bind_driver_to_node(parent,
+							 "dwc3-omap-peripheral",
+							 name, node, &dev);
+			if (ret) {
+				error("dwc3 - not able to bind usb device node\n");
+				return ret;
+			}
+			break;
+		case USB_DR_MODE_HOST:
+			/* Bind MUSB host */
+			break;
+		default:
+			break;
+		};
+	}
+	return 0;
+}
+
+static const struct udevice_id ti_dwc3_ids[] = {
+	{ .compatible = "ti,am437x-dwc3" },
+	{ }
+};
+
+U_BOOT_DRIVER(ti_dwc3_wrapper) = {
+	.name	= "ti-dwc3-wrapper",
+	.id	= UCLASS_MISC,
+	.of_match = ti_dwc3_ids,
+	.bind = ti_dwc3_wrapper_bind,
+};
+
 #endif /* CONFIG_DM_USB */

drivers/usb/dwc3/gadget.c +0 -0

index e065c5aeb38d..18bbd5318e48 100644

drivers/usb/dwc3/gadget.c +0 -0

 	if (ret) {
 		dev_err(dwc->dev, "failed to register udc\n");
 		goto err4;

drivers/usb/dwc3/gadget.c int dwc3_gadget_init(struct dwc3 *dwc) · line 2613

Quoted 10 lines

drivers/usb/dwc3/gadget.c +1 -1

index e065c5aeb38d..18bbd5318e48 100644
@@ -2610,7 +2610,7 @@ int dwc3_gadget_init(struct dwc3 *dwc)
 	if (ret)
 		goto err4;
 
-	ret = usb_add_gadget_udc(dwc->dev, &dwc->gadget);
+	ret = usb_add_gadget_udc((struct device *)dwc->dev, &dwc->gadget>

This is probably unintended ?

Quoted 5 lines

drivers/usb/dwc3/gadget.c +0 -0

 	if (ret) {
 		dev_err(dwc->dev, "failed to register udc\n");
 		goto err4;

Provide a way to read MAC address for usb_ether device from board function. Board files can override board_set_usbnet_devaddr() to populate MAC address to be used by usb_ether as device address.

Signed-off-by Vignesh R <[email protected]>

---
 drivers/usb/gadget/ether.c | 6 ++++++
 1 file changed, 6 insertions(+)

drivers/usb/gadget/ether.c +6 -0

index 4137d76c42af..8854e8eb004a 100644
@@ -2619,6 +2619,10 @@ int usb_eth_initialize(bd_t *bi)
 	return 0;
 }
 #else
+void __weak board_set_usbnet_devaddr(void)
+{
+}
+
 static int usb_eth_start(struct udevice *dev)
 {
 	struct ether_priv *priv = dev_get_priv(dev);

@@ -2683,6 +2687,8 @@ static int usb_eth_probe(struct udevice *dev)
 
 	priv->netdev = dev;
 	l_priv = priv;
+	/* Get MAC address for USB ETH interface */
+	board_set_usbnet_devaddr();
 
 	get_ether_addr(CONFIG_USBNET_DEVADDR, pdata->enetaddr);
 	eth_setenv_enetaddr("usbnet_devaddr", pdata->enetaddr);

Quoted 5 lines · Vignesh R

Provide a way to read MAC address for usb_ether device from board function. Board files can override board_set_usbnet_devaddr() to populate MAC address to be used by usb_ether as device address.

Signed-off-by Vignesh R <[email protected]>

This patch is totally unrelated to this series.

Moreover, just set eth*addr using setenv() to achieve the same iirc .

Quoted 29 lines

---
 drivers/usb/gadget/ether.c | 6 ++++++
 1 file changed, 6 insertions(+)

drivers/usb/gadget/ether.c +6 -0

index 4137d76c42af..8854e8eb004a 100644
@@ -2619,6 +2619,10 @@ int usb_eth_initialize(bd_t *bi)
 	return 0;
 }
 #else
+void __weak board_set_usbnet_devaddr(void)
+{
+}
+
 static int usb_eth_start(struct udevice *dev)
 {
 	struct ether_priv *priv = dev_get_priv(dev);

@@ -2683,6 +2687,8 @@ static int usb_eth_probe(struct udevice *dev)
 
 	priv->netdev = dev;
 	l_priv = priv;
+	/* Get MAC address for USB ETH interface */
+	board_set_usbnet_devaddr();
 
 	get_ether_addr(CONFIG_USBNET_DEVADDR, pdata->enetaddr);
 	eth_setenv_enetaddr("usbnet_devaddr", pdata->enetaddr);

Quoted 5 lines · Marek Vasut

Quoted 4 lines · Vignesh R

Provide a way to read MAC address for usb_ether device from board function. Board files can override board_set_usbnet_devaddr() to populate MAC address to be used by usb_ether as device address.

Quoted 3 lines

Quoted 1 line

Signed-off-by Vignesh R <[email protected]>

This patch is totally unrelated to this series.

This series converts dwc3 peripheral to device model and the best way to test this on TI platform to use it on ether gadget and test RNDIS boot. Hence, I had to do patches 8 to 13. Therefore I posted them together for completeness, if somebody wanted to test this out.

Quoted 2 lines

Moreover, just set eth*addr using setenv() to achieve the same iirc .

eth*addr is used for regular ethernet interface and usually set by regular ethernet driver, but I could not find a way to set MAC address for usb ethernet interface
Board file will now override board_set_usbnet_devaddr() to read MAC and then call setenv()

Quoted 8 lines · Vignesh R

Quoted 5 lines · Marek Vasut

Quoted 4 lines · Vignesh R

Provide a way to read MAC address for usb_ether device from board function. Board files can override board_set_usbnet_devaddr() to populate MAC address to be used by usb_ether as device address.

Quoted 8 lines

Quoted 3 lines

Quoted 1 line

Signed-off-by Vignesh R <[email protected]>

This patch is totally unrelated to this series.

This series converts dwc3 peripheral to device model and the best way to test this on TI platform to use it on ether gadget and test RNDIS boot. Hence, I had to do patches 8 to 13. Therefore I posted them together for completeness, if somebody wanted to test this out.

Testing is great, but this is probably a fix and it's unrelated to the series anyway.

Quoted 3 lines

Quoted 2 lines

Moreover, just set eth*addr using setenv() to achieve the same iirc .

Quoted 5 lines

eth*addr is used for regular ethernet interface and usually set by regular ethernet driver, but I could not find a way to set MAC address for usb ethernet interface
Board file will now override board_set_usbnet_devaddr() to read MAC and then call setenv()

Lukasz should be able to jump in and help.

Hi Lukasz,

Quoted 32 lines · Marek Vasut

Quoted 10 lines · Vignesh R

Quoted 7 lines · Marek Vasut

Quoted 5 lines · Vignesh R

Provide a way to read MAC address for usb_ether device from board function. Board files can override board_set_usbnet_devaddr() to populate MAC address to be used by usb_ether as device address.

Signed-off-by Vignesh R <[email protected]>

Quoted 7 lines

Quoted 1 line

This patch is totally unrelated to this series.

This series converts dwc3 peripheral to device model and the best way to test this on TI platform to use it on ether gadget and test RNDIS boot. Hence, I had to do patches 8 to 13. Therefore I posted them together for completeness, if somebody wanted to test this out.

Testing is great, but this is probably a fix and it's unrelated to the series anyway.

Quoted 9 lines

Quoted 3 lines

Moreover, just set eth*addr using setenv() to achieve the same iirc .

eth*addr is used for regular ethernet interface and usually set by regular ethernet driver, but I could not find a way to set MAC address for usb ethernet interface
Board file will now override board_set_usbnet_devaddr() to read MAC and then call setenv()

Quoted 2 lines

Lukasz should be able to jump in and help.

Any preference on how to handle this?
Basically, I need a way to pass MAC address to usb_ether driver. MAC address is available in specific efuse registers.

Previously, this was
handled by board_eth_init(), this is no longer available when using DM based ethernet driver.

Hi Vignesh,

Quoted 38 lines

Hi Lukasz,

Quoted 34 lines · Marek Vasut

Quoted 10 lines · Vignesh R

Quoted 7 lines · Marek Vasut

Quoted 5 lines · Vignesh R

Provide a way to read MAC address for usb_ether device from board function. Board files can override board_set_usbnet_devaddr() to populate MAC address to be used by usb_ether as device address.

Signed-off-by Vignesh R <[email protected]>

Quoted 7 lines

Quoted 1 line

This patch is totally unrelated to this series.

This series converts dwc3 peripheral to device model and the best way to test this on TI platform to use it on ether gadget and test RNDIS boot. Hence, I had to do patches 8 to 13. Therefore I posted them together for completeness, if somebody wanted to test this out.

Testing is great, but this is probably a fix and it's unrelated to the series anyway.

Quoted 9 lines

Quoted 3 lines

Moreover, just set eth*addr using setenv() to achieve the same iirc .

eth*addr is used for regular ethernet interface and usually set by regular ethernet driver, but I could not find a way to set MAC address for usb ethernet interface
Board file will now override board_set_usbnet_devaddr() to read MAC and then call setenv()

Lukasz should be able to jump in and help.

Quoted 3 lines

Any preference on how to handle this?
Basically, I need a way to pass MAC address to usb_ether driver. MAC address is available in specific efuse registers.

It seems to me like we need to read some board specific code anyway (because the efused data can be placed at different addresses for different SoC - e.g. am4xx or dra7).

The __weak function could check if e.g. "ethusbaddr" is set and read eth addr (if one would like to override it from envs).

And as far as I remember TI even for "ordinary" ETH driver reads data (MAC addr) in board file, so there should be no problem for also reading ETH data from there.

Quoted 6 lines

Previously, this was
handled by board_eth_init(), this is no longer available when using DM based ethernet driver.

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de

Populate DM_FLAG_PRE_RELOC, so that usb_ether can be used before relocation in case of RNDIS boot.

Signed-off-by Vignesh R <[email protected]>

---
 drivers/usb/gadget/ether.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

DWC3 can operate in either device mode or host mode. Add an entry to support DWC3 gadget.

Signed-off-by Vignesh R <[email protected]>

---
 drivers/usb/gadget/gadget_chips.h | 2 ++
 1 file changed, 2 insertions(+)

Add function to populate MAC address for usb ether device to support RNDIS in SPL. Also make arch_misc_init() available when CONFIG_SPL_USBEHT_SUPPORT is defined so that usb_ether_init() is called for am43xx as well.

Signed-off-by Vignesh R <[email protected]>

---
 arch/arm/mach-omap2/boot-common.c |  3 ++-
 board/ti/am43xx/board.c           | 20 ++++++++++++++++++++
 2 files changed, 22 insertions(+), 1 deletion(-)

Clean up include/configs/am43xx_evm.h and add configs to support USB device boot for am43xx evm.

Signed-off-by Vignesh R <[email protected]>

---
 configs/am43xx_evm_defconfig | 13 ++++++++++++-
 include/configs/am43xx_evm.h | 27 ++++++++-------------------
 2 files changed, 20 insertions(+), 20 deletions(-)

Enable USB nodes required to support RNDIS boot in SPL.

Signed-off-by Vignesh R <[email protected]>

---
 arch/arm/dts/am437x-gp-evm-u-boot.dtsi | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

arch/arm/dts/am437x-gp-evm-u-boot.dtsi +0 -0

index 885a9a92dbd3..a689cd68a593 100644

Quoted 7 lines · Vignesh R

Enable USB nodes required to support RNDIS boot in SPL.

Signed-off-by Vignesh R <[email protected]>

---
 arch/arm/dts/am437x-gp-evm-u-boot.dtsi | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

arch/arm/dts/am437x-gp-evm-u-boot.dtsi line 42

Quoted 12 lines

arch/arm/dts/am437x-gp-evm-u-boot.dtsi +4 -0

index 885a9a92dbd3..a689cd68a593 100644
@@ -36,3 +36,23 @@
 &phy_sel {
 	u-boot,dm-pre-reloc;
 };
+
+&am43xx_control_usb2phy1 {
+	u-boot,dm-pre-reloc;
+};

As asked earlier, any reason not to use u-boot,dm-spl ?