[U-Boot] [PATCH v2 00/13] driver model bring-up of dwc3 usb peripheral

This patch series enables ti dwc3 usb peripherial driver to adopt driver model and DT. And also adds support for to USB device boot using RNDIS for AM437x.
Tested on AM437x GP EVM in USB RNDIS boot mode. ROM downloads SPL, SPL loads U-Boot via usb ether and then U-Boot loads kernel image using the same interface.
Based on previous work submitted by Mugunthan here: https://lists.denx.de/pipermail/u-boot/2016-March/250115.html
Change log: * Reorder patches adding dwc3 driver model patches * Split patch 8 into 3 patches. * rebase to latest master and pick up Reviewed-bys
Mugunthan V N (7): drivers: usb: dwc3: remove devm_zalloc from linux_compact drivers: usb: dwc3-omap: move usb_gadget_handle_interrupts from board files to drivers am437x: board: do not register usb devices when CONFIG_DM_USB is defined omap5/am57xx/dra7xx: board: do not register usb devices when CONFIG_DM_USB is defined drivers: usb: common: add support to get maximum speed from dt drivers: usb: dwc3: add ti dwc3 peripheral driver with driver model support drivers: usb: dwc3: add ti dwc3 misc driver for wrapper
Vignesh R (6): usb: gadget: ether: Provide a way to read MAC address usb: gadget: ether: Populate DM_FLAG_PRE_RELOC flag usb: gadget: add DWC3 USB gadget support am43xx: Add USB device boot support configs: am43xx: Enable configs to support USB device boot ARM: am437x-gp-evm-u-boot.dtsi: Enable nodes for USB device boot
arch/arm/dts/am437x-gp-evm-u-boot.dtsi | 20 +++ arch/arm/mach-omap2/boot-common.c | 3 +- board/ti/am43xx/board.c | 41 +++--- board/ti/am57xx/board.c | 13 +- board/ti/dra7xx/evm.c | 13 +- board/ti/omap5_uevm/evm.c | 13 +- configs/am43xx_evm_defconfig | 13 +- configs/am43xx_evm_usbhost_boot_defconfig | 1 + drivers/usb/common/common.c | 29 ++++ drivers/usb/dwc3/core.c | 64 ++++++++- drivers/usb/dwc3/core.h | 6 + drivers/usb/dwc3/dwc3-omap.c | 231 +++++++++++++++++++++++++++++- drivers/usb/dwc3/gadget.c | 2 +- drivers/usb/dwc3/linux-compat.h | 5 - drivers/usb/dwc3/ti_usb_phy.c | 1 + drivers/usb/gadget/ether.c | 8 +- drivers/usb/gadget/gadget_chips.h | 2 + include/configs/am43xx_evm.h | 27 ++-- include/linux/usb/otg.h | 9 ++ scripts/Makefile.spl | 1 + 20 files changed, 416 insertions(+), 86 deletions(-)

From: Mugunthan V N mugunthanvnm@ti.com
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 mugunthanvnm@ti.com Signed-off-by: Vignesh R vigneshr@ti.com Reviewed-by: Simon Glass sjg@chromium.org --- 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(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 87b9c87edf6a..98102bd6b00a 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -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;
diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c index 3dcc2f484777..63551e780434 100644 --- a/drivers/usb/dwc3/dwc3-omap.c +++ b/drivers/usb/dwc3/dwc3-omap.c @@ -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;
diff --git a/drivers/usb/dwc3/linux-compat.h b/drivers/usb/dwc3/linux-compat.h index 9e944a31be11..f95d6152b7fe 100644 --- a/drivers/usb/dwc3/linux-compat.h +++ b/drivers/usb/dwc3/linux-compat.h @@ -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 diff --git a/drivers/usb/dwc3/ti_usb_phy.c b/drivers/usb/dwc3/ti_usb_phy.c index 218a8e586c8d..8088afc97047 100644 --- a/drivers/usb/dwc3/ti_usb_phy.c +++ b/drivers/usb/dwc3/ti_usb_phy.c @@ -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"

On 06/13/2017 02:09 PM, Vignesh R wrote:
From: Mugunthan V N mugunthanvnm@ti.com
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 ?
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com Signed-off-by: Vignesh R vigneshr@ti.com Reviewed-by: Simon Glass sjg@chromium.org
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(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 87b9c87edf6a..98102bd6b00a 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -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),
if (!evt) return ERR_PTR(-ENOMEM);GFP_KERNEL);
@@ -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,
if (!mem) return -ENOMEM;sizeof(*dwc) + DWC3_ALIGN_MASK, GFP_KERNEL);
diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c index 3dcc2f484777..63551e780434 100644 --- a/drivers/usb/dwc3/dwc3-omap.c +++ b/drivers/usb/dwc3/dwc3-omap.c @@ -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;
diff --git a/drivers/usb/dwc3/linux-compat.h b/drivers/usb/dwc3/linux-compat.h index 9e944a31be11..f95d6152b7fe 100644 --- a/drivers/usb/dwc3/linux-compat.h +++ b/drivers/usb/dwc3/linux-compat.h @@ -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 diff --git a/drivers/usb/dwc3/ti_usb_phy.c b/drivers/usb/dwc3/ti_usb_phy.c index 218a8e586c8d..8088afc97047 100644 --- a/drivers/usb/dwc3/ti_usb_phy.c +++ b/drivers/usb/dwc3/ti_usb_phy.c @@ -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"

On Tuesday 13 June 2017 07:28 PM, Marek Vasut wrote:
On 06/13/2017 02:09 PM, Vignesh R wrote:
From: Mugunthan V N mugunthanvnm@ti.com
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 ?
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.
Regards Vignesh

On 06/14/2017 11:05 AM, Vignesh R wrote:
On Tuesday 13 June 2017 07:28 PM, Marek Vasut wrote:
On 06/13/2017 02:09 PM, Vignesh R wrote:
From: Mugunthan V N mugunthanvnm@ti.com
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 ?
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.
OK, I see . That seems fine then.

From: Mugunthan V N mugunthanvnm@ti.com
In board files of am437x, dra7xx, omap5 and am5xx, usb_gadget_handle_interrupts() is just a place holder to handle dwc3 interrupts, nothing related to board is handled here, so move usb_gadget_handle_interrupts() from board files to dwc3-omap.c to avoid code duplication based on boards.
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com Signed-off-by: Vignesh R vigneshr@ti.com Reviewed-by: Simon Glass sjg@chromium.org --- board/ti/am43xx/board.c | 11 ----------- board/ti/am57xx/board.c | 11 ----------- board/ti/dra7xx/evm.c | 11 ----------- board/ti/omap5_uevm/evm.c | 11 ----------- drivers/usb/dwc3/dwc3-omap.c | 12 ++++++++++++ 5 files changed, 12 insertions(+), 44 deletions(-)
diff --git a/board/ti/am43xx/board.c b/board/ti/am43xx/board.c index 54f40e64a456..0bbe366a362c 100644 --- a/board/ti/am43xx/board.c +++ b/board/ti/am43xx/board.c @@ -668,17 +668,6 @@ static struct ti_usb_phy_device usb_phy2_device = { .usb2_phy_power = (void *)USB2_PHY2_POWER, .index = 1, }; - -int usb_gadget_handle_interrupts(int index) -{ - u32 status; - - status = dwc3_omap_uboot_interrupt_status(index); - if (status) - dwc3_uboot_handle_interrupt(index); - - return 0; -} #endif /* CONFIG_USB_DWC3 */
#if defined(CONFIG_USB_DWC3) || defined(CONFIG_USB_XHCI_OMAP) diff --git a/board/ti/am57xx/board.c b/board/ti/am57xx/board.c index bf8c8e1a678f..808145526299 100644 --- a/board/ti/am57xx/board.c +++ b/board/ti/am57xx/board.c @@ -764,17 +764,6 @@ static struct ti_usb_phy_device usb_phy2_device = { .usb2_phy_power = (void *)DRA7_USB2_PHY2_POWER, .index = 1, }; - -int usb_gadget_handle_interrupts(int index) -{ - u32 status; - - status = dwc3_omap_uboot_interrupt_status(index); - if (status) - dwc3_uboot_handle_interrupt(index); - - return 0; -} #endif /* CONFIG_USB_DWC3 */
#if defined(CONFIG_USB_DWC3) || defined(CONFIG_USB_XHCI_OMAP) diff --git a/board/ti/dra7xx/evm.c b/board/ti/dra7xx/evm.c index 7d36f03fa1ec..da6bb7d2c7ba 100644 --- a/board/ti/dra7xx/evm.c +++ b/board/ti/dra7xx/evm.c @@ -803,17 +803,6 @@ int omap_xhci_board_usb_cleanup(int index, enum usb_init_type init) disable_usb_clocks(index); return 0; } - -int usb_gadget_handle_interrupts(int index) -{ - u32 status; - - status = dwc3_omap_uboot_interrupt_status(index); - if (status) - dwc3_uboot_handle_interrupt(index); - - return 0; -} #endif
#if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_OS_BOOT) diff --git a/board/ti/omap5_uevm/evm.c b/board/ti/omap5_uevm/evm.c index 4b25cc2d7c3c..7e7ce5925931 100644 --- a/board/ti/omap5_uevm/evm.c +++ b/board/ti/omap5_uevm/evm.c @@ -118,17 +118,6 @@ int board_usb_cleanup(int index, enum usb_init_type init)
return 0; } - -int usb_gadget_handle_interrupts(int index) -{ - u32 status; - - status = dwc3_omap_uboot_interrupt_status(index); - if (status) - dwc3_uboot_handle_interrupt(index); - - return 0; -} #endif
/** diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c index 63551e780434..f18884f13392 100644 --- a/drivers/usb/dwc3/dwc3-omap.c +++ b/drivers/usb/dwc3/dwc3-omap.c @@ -24,6 +24,7 @@
#include <linux/usb/otg.h> #include <linux/compat.h> +#include <dwc3-uboot.h>
#include "linux-compat.h"
@@ -446,6 +447,17 @@ int dwc3_omap_uboot_interrupt_status(int index) return 0; }
+int usb_gadget_handle_interrupts(int index) +{ + u32 status; + + status = dwc3_omap_uboot_interrupt_status(index); + if (status) + dwc3_uboot_handle_interrupt(index); + + return 0; +} + MODULE_ALIAS("platform:omap-dwc3"); MODULE_AUTHOR("Felipe Balbi balbi@ti.com"); MODULE_LICENSE("GPL v2");

From: Mugunthan V N mugunthanvnm@ti.com
Do not register usb devices when CONFIG_DM_USB is define.
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com Signed-off-by: Vignesh R vigneshr@ti.com Reviewed-by: Simon Glass sjg@chromium.org --- board/ti/am43xx/board.c | 10 ++-------- configs/am43xx_evm_usbhost_boot_defconfig | 1 + 2 files changed, 3 insertions(+), 8 deletions(-)
diff --git a/board/ti/am43xx/board.c b/board/ti/am43xx/board.c index 0bbe366a362c..a2aefc08530a 100644 --- a/board/ti/am43xx/board.c +++ b/board/ti/am43xx/board.c @@ -632,7 +632,7 @@ int board_late_init(void) } #endif
-#ifdef CONFIG_USB_DWC3 +#if defined(CONFIG_USB_DWC3) && !defined(CONFIG_DM_USB) static struct dwc3_device usb_otg_ss1 = { .maximum_speed = USB_SPEED_HIGH, .base = USB_OTG_SS1_BASE, @@ -668,13 +668,10 @@ static struct ti_usb_phy_device usb_phy2_device = { .usb2_phy_power = (void *)USB2_PHY2_POWER, .index = 1, }; -#endif /* CONFIG_USB_DWC3 */
-#if defined(CONFIG_USB_DWC3) || defined(CONFIG_USB_XHCI_OMAP) int omap_xhci_board_usb_init(int index, enum usb_init_type init) { enable_usb_clocks(index); -#ifdef CONFIG_USB_DWC3 switch (index) { case 0: if (init == USB_INIT_DEVICE) { @@ -697,14 +694,12 @@ int omap_xhci_board_usb_init(int index, enum usb_init_type init) default: printf("Invalid Controller Index\n"); } -#endif
return 0; }
int omap_xhci_board_usb_cleanup(int index, enum usb_init_type init) { -#ifdef CONFIG_USB_DWC3 switch (index) { case 0: case 1: @@ -717,12 +712,11 @@ int omap_xhci_board_usb_cleanup(int index, enum usb_init_type init) default: printf("Invalid Controller Index\n"); } -#endif disable_usb_clocks(index);
return 0; } -#endif /* defined(CONFIG_USB_DWC3) || defined(CONFIG_USB_XHCI_OMAP) */ +#endif /* defined(CONFIG_USB_DWC3) && !defined(CONFIG_DM_USB) */
#ifdef CONFIG_DRIVER_TI_CPSW
diff --git a/configs/am43xx_evm_usbhost_boot_defconfig b/configs/am43xx_evm_usbhost_boot_defconfig index 870ed0fb24ab..e810b86c671c 100644 --- a/configs/am43xx_evm_usbhost_boot_defconfig +++ b/configs/am43xx_evm_usbhost_boot_defconfig @@ -69,6 +69,7 @@ CONFIG_USB_DWC3_PHY_OMAP=y CONFIG_USB_STORAGE=y CONFIG_USB_GADGET=y CONFIG_USB_GADGET_DOWNLOAD=y +CONFIG_SPL_USB_GADGET_SUPPORT=y CONFIG_G_DNL_MANUFACTURER="Texas Instruments" CONFIG_G_DNL_VENDOR_NUM=0x0403 CONFIG_G_DNL_PRODUCT_NUM=0xbd00

From: Mugunthan V N mugunthanvnm@ti.com
Do not register usb devices when CONFIG_DM_USB is defined.
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com Signed-off-by: Vignesh R vigneshr@ti.com Reviewed-by: Simon Glass sjg@chromium.org --- board/ti/am57xx/board.c | 2 +- board/ti/dra7xx/evm.c | 2 +- board/ti/omap5_uevm/evm.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/board/ti/am57xx/board.c b/board/ti/am57xx/board.c index 808145526299..2b466dd6a39b 100644 --- a/board/ti/am57xx/board.c +++ b/board/ti/am57xx/board.c @@ -746,7 +746,7 @@ int spl_start_uboot(void) } #endif
-#ifdef CONFIG_USB_DWC3 +#if defined(CONFIG_USB_DWC3) && !defined(CONFIG_DM_USB) static struct dwc3_device usb_otg_ss2 = { .maximum_speed = USB_SPEED_HIGH, .base = DRA7_USB_OTG_SS2_BASE, diff --git a/board/ti/dra7xx/evm.c b/board/ti/dra7xx/evm.c index da6bb7d2c7ba..1a72ff67c6df 100644 --- a/board/ti/dra7xx/evm.c +++ b/board/ti/dra7xx/evm.c @@ -712,7 +712,7 @@ int board_mmc_init(bd_t *bis) } #endif
-#ifdef CONFIG_USB_DWC3 +#if defined(CONFIG_USB_DWC3) && !defined(CONFIG_DM_USB) static struct dwc3_device usb_otg_ss1 = { .maximum_speed = USB_SPEED_SUPER, .base = DRA7_USB_OTG_SS1_BASE, diff --git a/board/ti/omap5_uevm/evm.c b/board/ti/omap5_uevm/evm.c index 7e7ce5925931..cf6fb6072554 100644 --- a/board/ti/omap5_uevm/evm.c +++ b/board/ti/omap5_uevm/evm.c @@ -60,7 +60,7 @@ struct tca642x_bank_info tca642x_init[] = { .configuration_reg = 0x40 }, };
-#ifdef CONFIG_USB_DWC3 +#if defined(CONFIG_USB_DWC3) && !defined(CONFIG_DM_USB) static struct dwc3_device usb_otg_ss = { .maximum_speed = USB_SPEED_SUPER, .base = OMAP5XX_USB_OTG_SS_BASE,

From: Mugunthan V N mugunthanvnm@ti.com
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 mugunthanvnm@ti.com Signed-off-by: Vignesh R vigneshr@ti.com Reviewed-by: Simon Glass sjg@chromium.org --- drivers/usb/common/common.c | 29 +++++++++++++++++++++++++++++ include/linux/usb/otg.h | 9 +++++++++ scripts/Makefile.spl | 1 + 3 files changed, 39 insertions(+)
diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c index 35c2dc18d955..c11689dc36fa 100644 --- a/drivers/usb/common/common.c +++ b/drivers/usb/common/common.c @@ -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; + 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"); + return USB_DR_MODE_UNKNOWN; + } + + for (i = 0; i < ARRAY_SIZE(speed_names); i++) + if (!strcmp(max_speed, speed_names[i])) + return i; + + return USB_SPEED_UNKNOWN; +} diff --git a/include/linux/usb/otg.h b/include/linux/usb/otg.h index 8f8ac6aeefe3..b61ef19b22f3 100644 --- a/include/linux/usb/otg.h +++ b/include/linux/usb/otg.h @@ -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 */ diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl index ac3c2c7f1342..bffea085b329 100644 --- a/scripts/Makefile.spl +++ b/scripts/Makefile.spl @@ -78,6 +78,7 @@ endif
libs-$(CONFIG_SPL_LIBDISK_SUPPORT) += disk/ libs-y += drivers/ +libs-y += drivers/usb/common/ libs-$(CONFIG_SPL_USB_GADGET_SUPPORT) += drivers/usb/dwc3/ libs-y += dts/ libs-y += fs/

On 06/13/2017 02:10 PM, Vignesh R wrote:
From: Mugunthan V N mugunthanvnm@ti.com
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 mugunthanvnm@ti.com Signed-off-by: Vignesh R vigneshr@ti.com Reviewed-by: Simon Glass sjg@chromium.org
drivers/usb/common/common.c | 29 +++++++++++++++++++++++++++++ include/linux/usb/otg.h | 9 +++++++++ scripts/Makefile.spl | 1 + 3 files changed, 39 insertions(+)
diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c index 35c2dc18d955..c11689dc36fa 100644 --- a/drivers/usb/common/common.c +++ b/drivers/usb/common/common.c @@ -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;
- 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 ?
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 ?
- return USB_SPEED_UNKNOWN;
+} diff --git a/include/linux/usb/otg.h b/include/linux/usb/otg.h index 8f8ac6aeefe3..b61ef19b22f3 100644 --- a/include/linux/usb/otg.h +++ b/include/linux/usb/otg.h @@ -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 */ diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl index ac3c2c7f1342..bffea085b329 100644 --- a/scripts/Makefile.spl +++ b/scripts/Makefile.spl @@ -78,6 +78,7 @@ endif
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 ?
libs-$(CONFIG_SPL_USB_GADGET_SUPPORT) += drivers/usb/dwc3/ libs-y += dts/ libs-y += fs/

On Tuesday 13 June 2017 07:31 PM, Marek Vasut wrote:
On 06/13/2017 02:10 PM, Vignesh R wrote:
From: Mugunthan V N mugunthanvnm@ti.com
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 mugunthanvnm@ti.com Signed-off-by: Vignesh R vigneshr@ti.com Reviewed-by: Simon Glass sjg@chromium.org
drivers/usb/common/common.c | 29 +++++++++++++++++++++++++++++ include/linux/usb/otg.h | 9 +++++++++ scripts/Makefile.spl | 1 + 3 files changed, 39 insertions(+)
diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c index 35c2dc18d955..c11689dc36fa 100644 --- a/drivers/usb/common/common.c +++ b/drivers/usb/common/common.c @@ -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;
- 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.
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.
- return USB_SPEED_UNKNOWN;
+} diff --git a/include/linux/usb/otg.h b/include/linux/usb/otg.h index 8f8ac6aeefe3..b61ef19b22f3 100644 --- a/include/linux/usb/otg.h +++ b/include/linux/usb/otg.h @@ -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 */ diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl index ac3c2c7f1342..bffea085b329 100644 --- a/scripts/Makefile.spl +++ b/scripts/Makefile.spl @@ -78,6 +78,7 @@ endif
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.
libs-$(CONFIG_SPL_USB_GADGET_SUPPORT) += drivers/usb/dwc3/ libs-y += dts/ libs-y += fs/

On 06/14/2017 11:16 AM, Vignesh R wrote:
On Tuesday 13 June 2017 07:31 PM, Marek Vasut wrote:
On 06/13/2017 02:10 PM, Vignesh R wrote:
From: Mugunthan V N mugunthanvnm@ti.com
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 mugunthanvnm@ti.com Signed-off-by: Vignesh R vigneshr@ti.com Reviewed-by: Simon Glass sjg@chromium.org
drivers/usb/common/common.c | 29 +++++++++++++++++++++++++++++ include/linux/usb/otg.h | 9 +++++++++ scripts/Makefile.spl | 1 + 3 files changed, 39 insertions(+)
diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c index 35c2dc18d955..c11689dc36fa 100644 --- a/drivers/usb/common/common.c +++ b/drivers/usb/common/common.c @@ -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;
- 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.
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.
I think the function should take into account the caps of the controller or if it doesn't, it's a misnomer.
- return USB_SPEED_UNKNOWN;
+} diff --git a/include/linux/usb/otg.h b/include/linux/usb/otg.h index 8f8ac6aeefe3..b61ef19b22f3 100644 --- a/include/linux/usb/otg.h +++ b/include/linux/usb/otg.h @@ -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 */ diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl index ac3c2c7f1342..bffea085b329 100644 --- a/scripts/Makefile.spl +++ b/scripts/Makefile.spl @@ -78,6 +78,7 @@ endif
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.
That's something to fix then, not work around like this ...

From: Mugunthan V N mugunthanvnm@ti.com
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 mugunthanvnm@ti.com Signed-off-by: Vignesh R vigneshr@ti.com --- drivers/usb/dwc3/core.c | 57 +++++++++++++++ drivers/usb/dwc3/core.h | 6 ++ drivers/usb/dwc3/dwc3-omap.c | 167 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 230 insertions(+)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 98102bd6b00a..a895f8fbddd3 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -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 balbi@ti.com"); 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); + if (ret) + goto err2; + + return 0; + +err2: + 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 diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 72d2fcdd3f42..972628751697 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -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); diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c index f18884f13392..b4dde726c6f3 100644 --- a/drivers/usb/dwc3/dwc3-omap.c +++ b/drivers/usb/dwc3/dwc3-omap.c @@ -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 balbi@ti.com"); MODULE_LICENSE("GPL v2"); MODULE_DESCRIPTION("DesignWare USB3 OMAP Glue Layer"); + +#else + +int usb_gadget_handle_interrupts(int index) +{ + 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 */

On 06/13/2017 02:10 PM, Vignesh R wrote:
From: Mugunthan V N mugunthanvnm@ti.com
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 mugunthanvnm@ti.com Signed-off-by: Vignesh R vigneshr@ti.com
drivers/usb/dwc3/core.c | 57 +++++++++++++++ drivers/usb/dwc3/core.h | 6 ++ drivers/usb/dwc3/dwc3-omap.c | 167 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 230 insertions(+)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 98102bd6b00a..a895f8fbddd3 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -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 balbi@ti.com"); 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);
- if (ret)
goto err2;
- return 0;
+err2:
Use more descriptive label names please ...
- 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 diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 72d2fcdd3f42..972628751697 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -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); diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c index f18884f13392..b4dde726c6f3 100644 --- a/drivers/usb/dwc3/dwc3-omap.c +++ b/drivers/usb/dwc3/dwc3-omap.c @@ -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 balbi@ti.com"); 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 ?
+{
- 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 */

On Tuesday 13 June 2017 07:33 PM, Marek Vasut wrote:
On 06/13/2017 02:10 PM, Vignesh R wrote:
From: Mugunthan V N mugunthanvnm@ti.com
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 mugunthanvnm@ti.com Signed-off-by: Vignesh R vigneshr@ti.com
drivers/usb/dwc3/core.c | 57 +++++++++++++++ drivers/usb/dwc3/core.h | 6 ++ drivers/usb/dwc3/dwc3-omap.c | 167 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 230 insertions(+)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 98102bd6b00a..a895f8fbddd3 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -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 balbi@ti.com"); 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);
- if (ret)
goto err2;
- return 0;
+err2:
Use more descriptive label names please ...
Ok, will do in next version.
- 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 diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 72d2fcdd3f42..972628751697 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -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); diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c index f18884f13392..b4dde726c6f3 100644 --- a/drivers/usb/dwc3/dwc3-omap.c +++ b/drivers/usb/dwc3/dwc3-omap.c @@ -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 balbi@ti.com"); 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 ?
How about moving this to drivers/usb/dwc3/gadget.c and then provide a callback to each of SoC specific drivers?
+{
- 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;
+}
Thanks for the review!

On 06/14/2017 02:25 PM, Vignesh R wrote: [...]
/**
- 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 balbi@ti.com"); 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 ?
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,
On Thursday 15 June 2017 10:27 PM, Marek Vasut wrote:
On 06/14/2017 02:25 PM, Vignesh R wrote: [...]
/**
- 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 balbi@ti.com"); 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 ?
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!

On 06/20/2017 02:00 PM, Vignesh R wrote:
Hi Marek,
On Thursday 15 June 2017 10:27 PM, Marek Vasut wrote:
On 06/14/2017 02:25 PM, Vignesh R wrote: [...]
/**
- 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 balbi@ti.com"); 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 ?
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.
This is something to consult with Lukasz then.
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.
Let me know your preference. Any suggestion appreciated. Thanks!

Hi Marek, Vignesh,
On 06/20/2017 02:00 PM, Vignesh R wrote:
Hi Marek,
On Thursday 15 June 2017 10:27 PM, Marek Vasut wrote:
On 06/14/2017 02:25 PM, Vignesh R wrote: [...]
/**
- 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 balbi@ti.com"); 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 ?
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.
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.....
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.
Let me know your preference. Any suggestion appreciated. Thanks!
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@denx.de

Hi,
On Tuesday 20 June 2017 07:14 PM, Lukasz Majewski wrote:
Hi Marek, Vignesh,
[...]
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.
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 will do my best to provide some ideas for this task.....
Thanks!

Hi Vignesh,
Hi,
On Tuesday 20 June 2017 07:14 PM, Lukasz Majewski wrote:
Hi Marek, Vignesh,
[...]
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.
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....
I will do my best to provide some ideas for this task.....
Thanks!
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@denx.de

On Wednesday 21 June 2017 01:39 PM, Lukasz Majewski wrote:
Hi Vignesh,
Hi,
On Tuesday 20 June 2017 07:14 PM, Lukasz Majewski wrote:
Hi Marek, Vignesh,
[...]
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.
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:
- 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.
- 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?
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).....
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.
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....
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!

On Thu, 22 Jun 2017 17:42:38 +0530 Vignesh R vigneshr@ti.com wrote:
On Wednesday 21 June 2017 01:39 PM, Lukasz Majewski wrote:
Hi Vignesh,
Hi,
On Tuesday 20 June 2017 07:14 PM, Lukasz Majewski wrote:
Hi Marek, Vignesh,
[...]
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.
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:
- 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.
- 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.
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.
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).....
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*).
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....
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.
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]).
Thanks for looking into this!
No problem.
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@denx.de

On Thursday 22 June 2017 06:30 PM, Lukasz Majewski wrote:
On Thu, 22 Jun 2017 17:42:38 +0530 Vignesh R vigneshr@ti.com wrote:
On Wednesday 21 June 2017 01:39 PM, Lukasz Majewski wrote:
Hi Vignesh,
Hi,
On Tuesday 20 June 2017 07:14 PM, Lukasz Majewski wrote:
Hi Marek, Vignesh,
[...]
> > 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.
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:
- 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.
- 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.
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.
Ok.
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).....
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...
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....
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.
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]).
yes, all UDCs needs to moved.. BTW, what platform would you be starting these migration on?

On Tue, 27 Jun 2017 14:38:38 +0530 Vignesh R vigneshr@ti.com wrote:
On Thursday 22 June 2017 06:30 PM, Lukasz Majewski wrote:
On Thu, 22 Jun 2017 17:42:38 +0530 Vignesh R vigneshr@ti.com wrote:
On Wednesday 21 June 2017 01:39 PM, Lukasz Majewski wrote:
Hi Vignesh,
Hi,
On Tuesday 20 June 2017 07:14 PM, Lukasz Majewski wrote:
Hi Marek, Vignesh,
[...]
>> >> 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.
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:
- 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.
- 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.
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.
Ok.
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).....
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...
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....
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.
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]).
yes, all UDCs needs to moved.. BTW, what platform would you be starting these migration on?
I do have two available:
- Beagle Bone Blach
- Odroid XU3
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@denx.de

Hi Lukasz,
Revisiting this old thread... On 21-Jun-17 1:39 PM, Lukasz Majewski wrote:
Hi Vignesh,
[...]
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...
Did you get a chance to work on DM conversion of USB gadget framework? Are there any plans to work on it as such?
I've sketched following plan for gadget conversion:
- 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.
- 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....
I will do my best to provide some ideas for this task.....

From: Mugunthan V N mugunthanvnm@ti.com
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 mugunthanvnm@ti.com Signed-off-by: Vignesh R vigneshr@ti.com --- drivers/usb/dwc3/dwc3-omap.c | 49 ++++++++++++++++++++++++++++++++++++++++++++ drivers/usb/dwc3/gadget.c | 2 +- 2 files changed, 50 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c index b4dde726c6f3..b7daf499515d 100644 --- a/drivers/usb/dwc3/dwc3-omap.c +++ b/drivers/usb/dwc3/dwc3-omap.c @@ -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 */ diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index e065c5aeb38d..18bbd5318e48 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -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); if (ret) { dev_err(dwc->dev, "failed to register udc\n"); goto err4;

On 06/13/2017 02:10 PM, Vignesh R wrote:
From: Mugunthan V N mugunthanvnm@ti.com
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 mugunthanvnm@ti.com Signed-off-by: Vignesh R vigneshr@ti.com
drivers/usb/dwc3/dwc3-omap.c | 49 ++++++++++++++++++++++++++++++++++++++++++++ drivers/usb/dwc3/gadget.c | 2 +- 2 files changed, 50 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c index b4dde726c6f3..b7daf499515d 100644 --- a/drivers/usb/dwc3/dwc3-omap.c +++ b/drivers/usb/dwc3/dwc3-omap.c @@ -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 */ diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index e065c5aeb38d..18bbd5318e48 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -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 ?
if (ret) { dev_err(dwc->dev, "failed to register udc\n"); goto err4;

On Tuesday 13 June 2017 07:35 PM, Marek Vasut wrote:
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index e065c5aeb38d..18bbd5318e48 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -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 ?
I will revisit this patch and get rid of this change
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 vigneshr@ti.com --- drivers/usb/gadget/ether.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c index 4137d76c42af..8854e8eb004a 100644 --- a/drivers/usb/gadget/ether.c +++ b/drivers/usb/gadget/ether.c @@ -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);

On 06/13/2017 02:10 PM, Vignesh R wrote:
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 vigneshr@ti.com
This patch is totally unrelated to this series. Moreover, just set eth*addr using setenv() to achieve the same iirc .
drivers/usb/gadget/ether.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c index 4137d76c42af..8854e8eb004a 100644 --- a/drivers/usb/gadget/ether.c +++ b/drivers/usb/gadget/ether.c @@ -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);

On Tuesday 13 June 2017 07:36 PM, Marek Vasut wrote:
On 06/13/2017 02:10 PM, Vignesh R wrote:
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 vigneshr@ti.com
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.
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()
drivers/usb/gadget/ether.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c index 4137d76c42af..8854e8eb004a 100644 --- a/drivers/usb/gadget/ether.c +++ b/drivers/usb/gadget/ether.c @@ -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);

On 06/14/2017 02:24 PM, Vignesh R wrote:
On Tuesday 13 June 2017 07:36 PM, Marek Vasut wrote:
On 06/13/2017 02:10 PM, Vignesh R wrote:
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 vigneshr@ti.com
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.
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.

Hi Lukasz,
On Thursday 15 June 2017 10:28 PM, Marek Vasut wrote:
On 06/14/2017 02:24 PM, Vignesh R wrote:
On Tuesday 13 June 2017 07:36 PM, Marek Vasut wrote:
On 06/13/2017 02:10 PM, Vignesh R wrote:
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 vigneshr@ti.com
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.
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.
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,
Hi Lukasz,
On Thursday 15 June 2017 10:28 PM, Marek Vasut wrote:
On 06/14/2017 02:24 PM, Vignesh R wrote:
On Tuesday 13 June 2017 07:36 PM, Marek Vasut wrote:
On 06/13/2017 02:10 PM, Vignesh R wrote:
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 vigneshr@ti.com
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.
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.
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.
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@denx.de

Hi Lukasz,
On 21 June 2017 at 01:33, Lukasz Majewski lukma@denx.de wrote:
Hi Vignesh,
Hi Lukasz,
On Thursday 15 June 2017 10:28 PM, Marek Vasut wrote:
On 06/14/2017 02:24 PM, Vignesh R wrote:
On Tuesday 13 June 2017 07:36 PM, Marek Vasut wrote:
On 06/13/2017 02:10 PM, Vignesh R wrote:
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 vigneshr@ti.com
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.
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.
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).
If it is just a different address then that could be described in the device tree.
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.
Rather than a weak function could we provide a misc 'driver' which other drivers can call when they need information? We could use an ioctl-like interface perhaps, if it is not worth adding a new uclass methods for this. Or perhaps we could have a UCLASS_BOARD with methods like get_mac_addr(). The the board could provide a driver.
We should not be needing weak functions with driver model - it tends to create a sloppy API IMO and makes it hard to know what code is called.
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@denx.de
Regards, Simon

Populate DM_FLAG_PRE_RELOC, so that usb_ether can be used before relocation in case of RNDIS boot.
Signed-off-by: Vignesh R vigneshr@ti.com --- drivers/usb/gadget/ether.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c index 8854e8eb004a..0040e6de3a40 100644 --- a/drivers/usb/gadget/ether.c +++ b/drivers/usb/gadget/ether.c @@ -2732,6 +2732,6 @@ U_BOOT_DRIVER(eth_usb) = { .ops = &usb_eth_ops, .priv_auto_alloc_size = sizeof(struct ether_priv), .platdata_auto_alloc_size = sizeof(struct eth_pdata), - .flags = DM_FLAG_ALLOC_PRIV_DMA, + .flags = DM_FLAG_ALLOC_PRIV_DMA | DM_FLAG_PRE_RELOC, }; #endif /* CONFIG_DM_ETH */

On 13 June 2017 at 06:10, Vignesh R vigneshr@ti.com wrote:
Populate DM_FLAG_PRE_RELOC, so that usb_ether can be used before relocation in case of RNDIS boot.
Signed-off-by: Vignesh R vigneshr@ti.com
drivers/usb/gadget/ether.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org

DWC3 can operate in either device mode or host mode. Add an entry to support DWC3 gadget.
Signed-off-by: Vignesh R vigneshr@ti.com --- drivers/usb/gadget/gadget_chips.h | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/usb/gadget/gadget_chips.h b/drivers/usb/gadget/gadget_chips.h index f32070843149..9b0ad2e62b69 100644 --- a/drivers/usb/gadget/gadget_chips.h +++ b/drivers/usb/gadget/gadget_chips.h @@ -214,5 +214,7 @@ static inline int usb_gadget_controller_number(struct usb_gadget *gadget) return 0x21; else if (gadget_is_fotg210(gadget)) return 0x22; + else if (gadget_is_dwc3(gadget)) + return 0x23; return -ENOENT; }

On 13 June 2017 at 06:10, Vignesh R vigneshr@ti.com wrote:
DWC3 can operate in either device mode or host mode. Add an entry to support DWC3 gadget.
Signed-off-by: Vignesh R vigneshr@ti.com
drivers/usb/gadget/gadget_chips.h | 2 ++ 1 file changed, 2 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org

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 vigneshr@ti.com --- arch/arm/mach-omap2/boot-common.c | 3 ++- board/ti/am43xx/board.c | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-omap2/boot-common.c b/arch/arm/mach-omap2/boot-common.c index 29c8f231917b..fc4c935477cf 100644 --- a/arch/arm/mach-omap2/boot-common.c +++ b/arch/arm/mach-omap2/boot-common.c @@ -194,7 +194,8 @@ void spl_board_init(void) #ifdef CONFIG_SPL_I2C_SUPPORT i2c_init(CONFIG_SYS_OMAP24_I2C_SPEED, CONFIG_SYS_OMAP24_I2C_SLAVE); #endif -#if defined(CONFIG_AM33XX) && defined(CONFIG_SPL_MUSB_NEW_SUPPORT) +#if (defined(CONFIG_AM33XX) && defined(CONFIG_SPL_MUSB_NEW_SUPPORT)) || \ + defined(CONFIG_SPL_USBETH_SUPPORT) arch_misc_init(); #endif #if defined(CONFIG_HW_WATCHDOG) diff --git a/board/ti/am43xx/board.c b/board/ti/am43xx/board.c index a2aefc08530a..73ae8fc1ff96 100644 --- a/board/ti/am43xx/board.c +++ b/board/ti/am43xx/board.c @@ -821,6 +821,26 @@ int board_eth_init(bd_t *bis) } #endif
+#if defined(CONFIG_DM_ETH) && defined(CONFIG_SPL_USBETH_SUPPORT) +void board_set_usbnet_devaddr(void) +{ + uint8_t mac_addr[6]; + uint32_t mac_hi, mac_lo; + + mac_lo = readl(&cdev->macid0l); + mac_hi = readl(&cdev->macid0h); + mac_addr[0] = mac_hi & 0xFF; + mac_addr[1] = (mac_hi & 0xFF00) >> 8; + mac_addr[2] = (mac_hi & 0xFF0000) >> 16; + mac_addr[3] = (mac_hi & 0xFF000000) >> 24; + mac_addr[4] = mac_lo & 0xFF; + mac_addr[5] = (mac_lo & 0xFF00) >> 8; + + if (is_valid_ethaddr(mac_addr)) + eth_setenv_enetaddr("usbnet_devaddr", mac_addr); +} +#endif + #ifdef CONFIG_SPL_LOAD_FIT int board_fit_config_name_match(const char *name) {

On 13 June 2017 at 06:10, Vignesh R vigneshr@ti.com wrote:
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 vigneshr@ti.com
arch/arm/mach-omap2/boot-common.c | 3 ++- board/ti/am43xx/board.c | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org

Clean up include/configs/am43xx_evm.h and add configs to support USB device boot for am43xx evm.
Signed-off-by: Vignesh R vigneshr@ti.com --- configs/am43xx_evm_defconfig | 13 ++++++++++++- include/configs/am43xx_evm.h | 27 ++++++++------------------- 2 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/configs/am43xx_evm_defconfig b/configs/am43xx_evm_defconfig index 4d9ec8841f8a..fc0bbdc4df71 100644 --- a/configs/am43xx_evm_defconfig +++ b/configs/am43xx_evm_defconfig @@ -22,7 +22,7 @@ CONFIG_ISO_PARTITION=y CONFIG_OF_CONTROL=y CONFIG_OF_LIST="am437x-gp-evm am437x-sk-evm am43x-epos-evm am437x-idk-evm" CONFIG_DM=y -# CONFIG_BLK is not set +CONFIG_BLK=y CONFIG_DFU_MMC=y CONFIG_DFU_RAM=y CONFIG_DFU_SF=y @@ -50,3 +50,14 @@ CONFIG_USB_GADGET_DOWNLOAD=y CONFIG_G_DNL_MANUFACTURER="Texas Instruments" CONFIG_G_DNL_VENDOR_NUM=0x0403 CONFIG_G_DNL_PRODUCT_NUM=0xbd00 +CONFIG_MISC=y +CONFIG_DM_USB=y +CONFIG_CMD_USB=y +CONFIG_DM_ETH=y +CONFIG_SPL_USBETH_SUPPORT=y +CONFIG_SPL_USB_GADGET_SUPPORT=y +CONFIG_SPL_DRIVERS_MISC_SUPPORT=y +CONFIG_SPL_USB_HOST_SUPPORT=y +CONFIG_SPL_USB_SUPPORT=y +CONFIG_SPL_NET_SUPPORT=y +CONFIG_SPL_NET_VCI_STRING="AM43xx U-Boot SPL" diff --git a/include/configs/am43xx_evm.h b/include/configs/am43xx_evm.h index 1d8e39c20352..96cb45108056 100644 --- a/include/configs/am43xx_evm.h +++ b/include/configs/am43xx_evm.h @@ -89,20 +89,6 @@ #define CONFIG_AM437X_USB2PHY2_HOST #endif
-#if defined(CONFIG_SPL_BUILD) && !defined(CONFIG_SPL_USBETH_SUPPORT) -#undef CONFIG_USB_DWC3_PHY_OMAP -#undef CONFIG_USB_DWC3_OMAP -#undef CONFIG_USB_DWC3 -#undef CONFIG_USB_DWC3_GADGET - -#undef CONFIG_USB_GADGET_DOWNLOAD -#undef CONFIG_USB_GADGET_VBUS_DRAW -#undef CONFIG_G_DNL_MANUFACTURER -#undef CONFIG_G_DNL_VENDOR_NUM -#undef CONFIG_G_DNL_PRODUCT_NUM -#undef CONFIG_USB_GADGET_DUALSPEED -#endif - /* * Disable MMC DM for SPL build and can be re-enabled after adding * DM support in SPL @@ -257,11 +243,6 @@ #define CONFIG_PHYLIB #define PHY_ANEG_TIMEOUT 8000 /* PHY needs longer aneg time at 1G */
-#if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_ETH_SUPPORT) -#undef CONFIG_ENV_IS_IN_FAT -#define CONFIG_ENV_IS_NOWHERE -#endif - #define CONFIG_SYS_RX_ETH_BUFFER 64
/* NAND support */ @@ -346,4 +327,12 @@ #define NANDBOOT #endif /* CONFIG_NAND */
+#define CONFIG_ARCH_MISC_INIT + +#ifdef CONFIG_SPL_USBETH_SUPPORT +#define CONFIG_USB_ETHER +#define CONFIG_USB_ETH_RNDIS +#define CONFIG_USBNET_HOST_ADDR "de:ad:be:af:00:00" +#endif + #endif /* __CONFIG_AM43XX_EVM_H */

On 13 June 2017 at 06:10, Vignesh R vigneshr@ti.com wrote:
Clean up include/configs/am43xx_evm.h and add configs to support USB device boot for am43xx evm.
Signed-off-by: Vignesh R vigneshr@ti.com
configs/am43xx_evm_defconfig | 13 ++++++++++++- include/configs/am43xx_evm.h | 27 ++++++++------------------- 2 files changed, 20 insertions(+), 20 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

Enable USB nodes required to support RNDIS boot in SPL.
Signed-off-by: Vignesh R vigneshr@ti.com --- arch/arm/dts/am437x-gp-evm-u-boot.dtsi | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
diff --git a/arch/arm/dts/am437x-gp-evm-u-boot.dtsi b/arch/arm/dts/am437x-gp-evm-u-boot.dtsi index 885a9a92dbd3..a689cd68a593 100644 --- a/arch/arm/dts/am437x-gp-evm-u-boot.dtsi +++ b/arch/arm/dts/am437x-gp-evm-u-boot.dtsi @@ -36,3 +36,23 @@ &phy_sel { u-boot,dm-pre-reloc; }; + +&am43xx_control_usb2phy1 { + u-boot,dm-pre-reloc; +}; + +&ocp2scp0 { + u-boot,dm-pre-reloc; + + phy@483a8000 { + u-boot,dm-pre-reloc; + }; +}; + +&dwc3_1 { + u-boot,dm-pre-reloc; + + usb@48390000 { + u-boot,dm-pre-reloc; + }; +};

On Tuesday 13 June 2017 05:40 PM, Vignesh R wrote:
Enable USB nodes required to support RNDIS boot in SPL.
Signed-off-by: Vignesh R vigneshr@ti.com
arch/arm/dts/am437x-gp-evm-u-boot.dtsi | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
diff --git a/arch/arm/dts/am437x-gp-evm-u-boot.dtsi b/arch/arm/dts/am437x-gp-evm-u-boot.dtsi index 885a9a92dbd3..a689cd68a593 100644 --- a/arch/arm/dts/am437x-gp-evm-u-boot.dtsi +++ b/arch/arm/dts/am437x-gp-evm-u-boot.dtsi @@ -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 ?
Thanks and regards, Lokesh

On Tuesday 13 June 2017 06:33 PM, Lokesh Vutla wrote:
On Tuesday 13 June 2017 05:40 PM, Vignesh R wrote:
Enable USB nodes required to support RNDIS boot in SPL.
Signed-off-by: Vignesh R vigneshr@ti.com
arch/arm/dts/am437x-gp-evm-u-boot.dtsi | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
diff --git a/arch/arm/dts/am437x-gp-evm-u-boot.dtsi b/arch/arm/dts/am437x-gp-evm-u-boot.dtsi index 885a9a92dbd3..a689cd68a593 100644 --- a/arch/arm/dts/am437x-gp-evm-u-boot.dtsi +++ b/arch/arm/dts/am437x-gp-evm-u-boot.dtsi @@ -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 ?
Oops, I overlooked this... Will fix in the next version.
participants (5)
-
Lokesh Vutla
-
Lukasz Majewski
-
Marek Vasut
-
Simon Glass
-
Vignesh R