Re: [U-Boot] [PATCH v5 0/7] usb: Add cadence USB3 gadget/host/phy driver
Hi Vignesh, > > Hi, > > On 28/08/19 7:52 PM, Sherry Sun wrote: > > Hi Vignesh, > [...] > >> I see that Cadence USB driver for Linux kernel is still under > >> development and DT compatible binding is supposed to be "cdns,usb3" > not "cdns,usb3-1.0.0". > >> See v11: > >> https://patch > >> > work.kernel.org%2Fpatch%2F4415%2F&data=02%7C01%7Csherry.s > >> > un%40nxp.com%7C97d431d6b569451ff8ac08d72bb5f883%7C686ea1d3bc2b > >> > 4c6fa92cd99c5c301635%7C0%7C0%7C637025932942549390&sdata=Jltlk > >> Te7SXTAQAKtF7HzOqY291upS67Eixeke9oXQ2w%3D&reserved=0 > >> Deviating from kernel binding will break sync'ing of DTs b/w kernel > >> and U- Boot. > >> > > > > Thanks for your reminder, I only noticed the DT compatible in v5 last time. > So I will change it to "cdns,usb3" in next version. > > > >> Also, why not sync the latest host/gadget driver patches from kernel as is? > >> That should help in borrowing bug fixes/features whenever Cadence > >> updates kernel driver in future. > > > > Since there are many differences between the cdns3 driver in uboot and > > kernel, such as in uboot, we didn't support host mode in cdns3 core.c, > > instead we add an > > xhci-imx8 driver as host driver. So the patches from kernel can't be applied > to this driver. > > > > I see that xhci-imx8.c is generic enough to be used with v11 upstream > cdns3/core.c and cdns3/host.c once IMX specific stuff is moved to top level > wrapper. > > I see you use a separate compatible for host driver which does not match > with kernel (as there is no separate node for host vs device). Moreover the dt > bindings listed in patch 1/7 lists non core registers as part of Cadence USB3 > controller node. > > Above issues make it impossible to sync Kernel DT nodes with U-Boot DT > nodes which is not acceptable for U-Boot. All non Cadence core registers and > configurations need to part of a separate wrapper driver that then binds to > appropriate host/device Cadence USB3 controller node based on dr_mode > property. E.g: see who dwc3 is modeled [3]: > Also see cdns-ti.c in the TI U-Boot branch [1] [2]) > > If there are more customization required for host driver cdns3/host.c we can > provide vendor specific hooks/callbacks to be called from cdns3/host.c > Current series is not usable on TI platform with Cadence USB3 IP at least in > host mode and would need core host driver to be ported from kernel. > > Keeping Linux kernel and U-Boot driver stack in sync has a big advantage for > you as well. It simplifies borrowing bug fixes and new features or functions > (especially in subsystems like USB where code is pretty large) > > BTW, I have a tree with v10 of Cadence USB3 kernel driver ported to U-Boot > here[1]. It is based on 2019.01 U-Boot but should apply as is on the latest > tree as well. (Tree is still missing USB3 PHY support and thus USB 3.0 > support) > and works on TI platform. I am waiting for bindings to be frozen in Linux > before posting to U-Boot list. > > Hopefully we can find a way to collaborate! I think your suggestions are reasonable, I agree that keep Linux kernel and U-Boot driver stack in sync is a good choice! So I will try to test your code[1] on i.MX8 platform later, if the common core part can work well, maybe you can post it to U-Boot list after the Linux cdns3 driver version is frozen. Best regards Sherry sun > > [1] git://git.ti.com/~vigneshr/ti-u-boot/vigneshr-ti-u-boot.git > branch: dfu > [2] > https://eur01.safelinks.protection.outlook.com/?url=http:%2F%2Fgit.ti.com% > 2Fcgit%2Fcgit.cgi%2F~vigneshr%2Fti-u-boot%2Fvigneshr-ti-u- > boot.git%2Ftree%2Farch%2Farm%2Fdts%2Fk3-j721e- > main.dtsi%3Fh%3Ddfu%23n430&data=02%7C01%7Csherry.sun%40nxp. > com%7C25c139bf8e6c4d6575ab08d72c996ba4%7C686ea1d3bc2b4c6fa92cd9 > 9c5c301635%7C0%7C0%7C637026909770138936&sdata=lj44%2BFC0AKi > cD%2BY81zHKhUl23DzxY4b%2Fu9fGstr3nOI%3D&reserved=0 > [3] > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Felixir. > bootlin.com%2Fu- > boot%2Flatest%2Fsource%2Fdrivers%2Fusb%2Fdwc3%2Fdwc3- > generic.c&data=02%7C01%7Csherry.sun%40nxp.com%7C25c139bf8e6c > 4d6575ab08d72c996ba4%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0 > %7C637026909770138936&sdata=mGur%2FVZFiUef5tblBuVPh3i86FQpe > yunPg15m%2Bwh1mk%3D&reserved=0 > > > > -- > Regards > Vignesh ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v5 0/7] usb: Add cadence USB3 gadget/host/phy driver
Hi, On 28/08/19 7:52 PM, Sherry Sun wrote: > Hi Vignesh, [...] >> I see that Cadence USB driver for Linux kernel is still under development and >> DT compatible binding is supposed to be "cdns,usb3" not "cdns,usb3-1.0.0". >> See v11: >> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch >> work.kernel.org%2Fpatch%2F4415%2F&data=02%7C01%7Csherry.s >> un%40nxp.com%7C97d431d6b569451ff8ac08d72bb5f883%7C686ea1d3bc2b >> 4c6fa92cd99c5c301635%7C0%7C0%7C637025932942549390&sdata=Jltlk >> Te7SXTAQAKtF7HzOqY291upS67Eixeke9oXQ2w%3D&reserved=0 >> Deviating from kernel binding will break sync'ing of DTs b/w kernel and U- >> Boot. >> > > Thanks for your reminder, I only noticed the DT compatible in v5 last time. > So I will change it to "cdns,usb3" in next version. > >> Also, why not sync the latest host/gadget driver patches from kernel as is? >> That should help in borrowing bug fixes/features whenever Cadence updates >> kernel driver in future. > > Since there are many differences between the cdns3 driver in uboot and > kernel, > such as in uboot, we didn't support host mode in cdns3 core.c, instead we add > an > xhci-imx8 driver as host driver. So the patches from kernel can't be applied > to this driver. > I see that xhci-imx8.c is generic enough to be used with v11 upstream cdns3/core.c and cdns3/host.c once IMX specific stuff is moved to top level wrapper. I see you use a separate compatible for host driver which does not match with kernel (as there is no separate node for host vs device). Moreover the dt bindings listed in patch 1/7 lists non core registers as part of Cadence USB3 controller node. Above issues make it impossible to sync Kernel DT nodes with U-Boot DT nodes which is not acceptable for U-Boot. All non Cadence core registers and configurations need to part of a separate wrapper driver that then binds to appropriate host/device Cadence USB3 controller node based on dr_mode property. E.g: see who dwc3 is modeled [3]: Also see cdns-ti.c in the TI U-Boot branch [1] [2]) If there are more customization required for host driver cdns3/host.c we can provide vendor specific hooks/callbacks to be called from cdns3/host.c Current series is not usable on TI platform with Cadence USB3 IP at least in host mode and would need core host driver to be ported from kernel. Keeping Linux kernel and U-Boot driver stack in sync has a big advantage for you as well. It simplifies borrowing bug fixes and new features or functions (especially in subsystems like USB where code is pretty large) BTW, I have a tree with v10 of Cadence USB3 kernel driver ported to U-Boot here[1]. It is based on 2019.01 U-Boot but should apply as is on the latest tree as well. (Tree is still missing USB3 PHY support and thus USB 3.0 support) and works on TI platform. I am waiting for bindings to be frozen in Linux before posting to U-Boot list. Hopefully we can find a way to collaborate! [1] git://git.ti.com/~vigneshr/ti-u-boot/vigneshr-ti-u-boot.git branch: dfu [2] http://git.ti.com/cgit/cgit.cgi/~vigneshr/ti-u-boot/vigneshr-ti-u-boot.git/tree/arch/arm/dts/k3-j721e-main.dtsi?h=dfu#n430 [3] https://elixir.bootlin.com/u-boot/latest/source/drivers/usb/dwc3/dwc3-generic.c -- Regards Vignesh ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v5 0/7] usb: Add cadence USB3 gadget/host/phy driver
Hi Vignesh, > > Hi Sherry, > > On 21/08/19 8:05 PM, Sherry Sun wrote: > > These patches introduce new Cadence driver to U-Boot. > > The first patch is to add the Cadence USB3 IP(CDNS3) core and driver > > for the usb gadget. > > The second patch introduce the xhci-imx8 usb host driver separately. > > The third patch introduce the cdns3 phy driver which can be used for > > both > > cdns3 host driver and gadget driver. > > The cdns3 usb gadget/host/phy driver are all used DM mode. > > > > The current driver has been validated on i.MX8 platform. > > If someone want to test it, please note that the additional dts nodes/ > > config macros/clock driver are also essential. You can also get my > > test patches at > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub > .com%2Fsherrysun1%2Fu-boot- > imx.git&data=02%7C01%7Csherry.sun%40nxp.com%7C97d431d6b5694 > 51ff8ac08d72bb5f883%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0% > 7C637025932942549390&sdata=XEFSCN0TtgU1H1zhNAgZeZIRgDInL%2B > 2Gb8ec3KO4AsI%3D&reserved=0 to start your test quickly. > > > > I see that Cadence USB driver for Linux kernel is still under development and > DT compatible binding is supposed to be "cdns,usb3" not "cdns,usb3-1.0.0". > See v11: > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch > work.kernel.org%2Fpatch%2F4415%2F&data=02%7C01%7Csherry.s > un%40nxp.com%7C97d431d6b569451ff8ac08d72bb5f883%7C686ea1d3bc2b > 4c6fa92cd99c5c301635%7C0%7C0%7C637025932942549390&sdata=Jltlk > Te7SXTAQAKtF7HzOqY291upS67Eixeke9oXQ2w%3D&reserved=0 > Deviating from kernel binding will break sync'ing of DTs b/w kernel and U- > Boot. > Thanks for your reminder, I only noticed the DT compatible in v5 last time. So I will change it to "cdns,usb3" in next version. > Also, why not sync the latest host/gadget driver patches from kernel as is? > That should help in borrowing bug fixes/features whenever Cadence updates > kernel driver in future. Since there are many differences between the cdns3 driver in uboot and kernel, such as in uboot, we didn't support host mode in cdns3 core.c, instead we add an xhci-imx8 driver as host driver. So the patches from kernel can't be applied to this driver. > > BTW, have you tested gadget driver with functions that use bulk endpoints > such as f_mass_storage or f_fastboot? Yes, I have tested the ums and fastboot function already, and it has proved to work well on i.MX8 platform. Best regards Sherry sun > > Regards > Vignesh > > > Changes in v5: > > - Delete some unnecessary code. > > - Fix some issues about lack of parentheses. > > - Add separate patch for core framework changes. > > - Add "reg-names" for usb nodes in DT and use it to get reg values. > > - Use "cdns,usb3-1.0.0" compatible instead "cdns,usb3". > > - Add DM_FLAG_OS_PREPARE flag to xhci-imx8 driver. > > > > Sherry Sun (7): > > dt-bindings: add dt-binding doc for CDNS3 controller > > usb: gadget: Add the cadence USB3 gadget driver > > usb: gadget: Add match_ep call back to usb_gadget_ops > > usb: gadget: Add gadget_is_cdns3 checking to provide bcdUSB value > > usb: host: Add the USB3 host driver > > phy: Add USB PHY driver for the cadence USB3 > > usb: gadget: core: introduce ->udc_set_speed() method > > > > Makefile |1 + > > doc/device-tree-bindings/usb/cdns-usb3.txt | 53 + > > drivers/phy/Kconfig|8 + > > drivers/phy/Makefile |1 + > > drivers/phy/cdns3-usb-phy.c| 241 +++ > > drivers/usb/Kconfig|2 + > > drivers/usb/cdns3/Kconfig | 20 + > > drivers/usb/cdns3/Makefile |5 + > > drivers/usb/cdns3/cdns3-generic.c | 114 + > > drivers/usb/cdns3/cdns3-nxp-reg-def.h | 93 + > > drivers/usb/cdns3/core.c | 203 ++ > > drivers/usb/cdns3/core.h | 118 ++ > > drivers/usb/cdns3/dev-regs-macro.h | 116 ++ > > drivers/usb/cdns3/dev-regs-map.h | 117 ++ > > drivers/usb/cdns3/gadget-export.h | 26 + > > drivers/usb/cdns3/gadget.c | 2183 > > drivers/usb/cdns3/gadget.h | 225 ++ > > drivers/usb/cdns3/io.h | 27 + > > drivers/usb/gadget/epautoconf.c|4 + > > drivers/usb/gadget/gadget_chips.h |7 + > > drivers/usb/gadget/udc/Makefile|1 + > > drivers/usb/gadget/udc/udc-core.c | 23 + > > drivers/usb/host/Kconfig |9 + > > drivers/usb/host/Makefile |1 + > > drivers/usb/host/xhci-imx8.c | 210 ++ > > include/linux/usb/gadget.h |5 + > > scripts/Makefile.spl |1 + > > 27 files changed, 3814 insertions(+) > > create mode 100644 doc/device-tree-bindings/usb/cdns-usb3.txt > > create mode 100644 drivers/ph
Re: [U-Boot] [PATCH v5 0/7] usb: Add cadence USB3 gadget/host/phy driver
Hi Sherry, On 21/08/19 8:05 PM, Sherry Sun wrote: > These patches introduce new Cadence driver to U-Boot. > The first patch is to add the Cadence USB3 IP(CDNS3) core and driver for > the usb gadget. > The second patch introduce the xhci-imx8 usb host driver separately. > The third patch introduce the cdns3 phy driver which can be used for both > cdns3 host driver and gadget driver. > The cdns3 usb gadget/host/phy driver are all used DM mode. > > The current driver has been validated on i.MX8 platform. > If someone want to test it, please note that the additional dts nodes/ > config macros/clock driver are also essential. You can also get > my test patches at https://github.com/sherrysun1/u-boot-imx.git to > start your test quickly. > I see that Cadence USB driver for Linux kernel is still under development and DT compatible binding is supposed to be "cdns,usb3" not "cdns,usb3-1.0.0". See v11: https://patchwork.kernel.org/patch/4415/ Deviating from kernel binding will break sync'ing of DTs b/w kernel and U-Boot. Also, why not sync the latest host/gadget driver patches from kernel as is? That should help in borrowing bug fixes/features whenever Cadence updates kernel driver in future. BTW, have you tested gadget driver with functions that use bulk endpoints such as f_mass_storage or f_fastboot? Regards Vignesh > Changes in v5: > - Delete some unnecessary code. > - Fix some issues about lack of parentheses. > - Add separate patch for core framework changes. > - Add "reg-names" for usb nodes in DT and use it to get reg > values. > - Use "cdns,usb3-1.0.0" compatible instead "cdns,usb3". > - Add DM_FLAG_OS_PREPARE flag to xhci-imx8 driver. > > Sherry Sun (7): > dt-bindings: add dt-binding doc for CDNS3 controller > usb: gadget: Add the cadence USB3 gadget driver > usb: gadget: Add match_ep call back to usb_gadget_ops > usb: gadget: Add gadget_is_cdns3 checking to provide bcdUSB value > usb: host: Add the USB3 host driver > phy: Add USB PHY driver for the cadence USB3 > usb: gadget: core: introduce ->udc_set_speed() method > > Makefile |1 + > doc/device-tree-bindings/usb/cdns-usb3.txt | 53 + > drivers/phy/Kconfig|8 + > drivers/phy/Makefile |1 + > drivers/phy/cdns3-usb-phy.c| 241 +++ > drivers/usb/Kconfig|2 + > drivers/usb/cdns3/Kconfig | 20 + > drivers/usb/cdns3/Makefile |5 + > drivers/usb/cdns3/cdns3-generic.c | 114 + > drivers/usb/cdns3/cdns3-nxp-reg-def.h | 93 + > drivers/usb/cdns3/core.c | 203 ++ > drivers/usb/cdns3/core.h | 118 ++ > drivers/usb/cdns3/dev-regs-macro.h | 116 ++ > drivers/usb/cdns3/dev-regs-map.h | 117 ++ > drivers/usb/cdns3/gadget-export.h | 26 + > drivers/usb/cdns3/gadget.c | 2183 > drivers/usb/cdns3/gadget.h | 225 ++ > drivers/usb/cdns3/io.h | 27 + > drivers/usb/gadget/epautoconf.c|4 + > drivers/usb/gadget/gadget_chips.h |7 + > drivers/usb/gadget/udc/Makefile|1 + > drivers/usb/gadget/udc/udc-core.c | 23 + > drivers/usb/host/Kconfig |9 + > drivers/usb/host/Makefile |1 + > drivers/usb/host/xhci-imx8.c | 210 ++ > include/linux/usb/gadget.h |5 + > scripts/Makefile.spl |1 + > 27 files changed, 3814 insertions(+) > create mode 100644 doc/device-tree-bindings/usb/cdns-usb3.txt > create mode 100644 drivers/phy/cdns3-usb-phy.c > create mode 100644 drivers/usb/cdns3/Kconfig > create mode 100644 drivers/usb/cdns3/Makefile > create mode 100644 drivers/usb/cdns3/cdns3-generic.c > create mode 100644 drivers/usb/cdns3/cdns3-nxp-reg-def.h > create mode 100644 drivers/usb/cdns3/core.c > create mode 100644 drivers/usb/cdns3/core.h > create mode 100644 drivers/usb/cdns3/dev-regs-macro.h > create mode 100644 drivers/usb/cdns3/dev-regs-map.h > create mode 100644 drivers/usb/cdns3/gadget-export.h > create mode 100644 drivers/usb/cdns3/gadget.c > create mode 100644 drivers/usb/cdns3/gadget.h > create mode 100644 drivers/usb/cdns3/io.h > create mode 100644 drivers/usb/host/xhci-imx8.c > -- Regards Vignesh ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v5 0/7] usb: Add cadence USB3 gadget/host/phy driver
These patches introduce new Cadence driver to U-Boot. The first patch is to add the Cadence USB3 IP(CDNS3) core and driver for the usb gadget. The second patch introduce the xhci-imx8 usb host driver separately. The third patch introduce the cdns3 phy driver which can be used for both cdns3 host driver and gadget driver. The cdns3 usb gadget/host/phy driver are all used DM mode. The current driver has been validated on i.MX8 platform. If someone want to test it, please note that the additional dts nodes/ config macros/clock driver are also essential. You can also get my test patches at https://github.com/sherrysun1/u-boot-imx.git to start your test quickly. Changes in v5: - Delete some unnecessary code. - Fix some issues about lack of parentheses. - Add separate patch for core framework changes. - Add "reg-names" for usb nodes in DT and use it to get reg values. - Use "cdns,usb3-1.0.0" compatible instead "cdns,usb3". - Add DM_FLAG_OS_PREPARE flag to xhci-imx8 driver. Sherry Sun (7): dt-bindings: add dt-binding doc for CDNS3 controller usb: gadget: Add the cadence USB3 gadget driver usb: gadget: Add match_ep call back to usb_gadget_ops usb: gadget: Add gadget_is_cdns3 checking to provide bcdUSB value usb: host: Add the USB3 host driver phy: Add USB PHY driver for the cadence USB3 usb: gadget: core: introduce ->udc_set_speed() method Makefile |1 + doc/device-tree-bindings/usb/cdns-usb3.txt | 53 + drivers/phy/Kconfig|8 + drivers/phy/Makefile |1 + drivers/phy/cdns3-usb-phy.c| 241 +++ drivers/usb/Kconfig|2 + drivers/usb/cdns3/Kconfig | 20 + drivers/usb/cdns3/Makefile |5 + drivers/usb/cdns3/cdns3-generic.c | 114 + drivers/usb/cdns3/cdns3-nxp-reg-def.h | 93 + drivers/usb/cdns3/core.c | 203 ++ drivers/usb/cdns3/core.h | 118 ++ drivers/usb/cdns3/dev-regs-macro.h | 116 ++ drivers/usb/cdns3/dev-regs-map.h | 117 ++ drivers/usb/cdns3/gadget-export.h | 26 + drivers/usb/cdns3/gadget.c | 2183 drivers/usb/cdns3/gadget.h | 225 ++ drivers/usb/cdns3/io.h | 27 + drivers/usb/gadget/epautoconf.c|4 + drivers/usb/gadget/gadget_chips.h |7 + drivers/usb/gadget/udc/Makefile|1 + drivers/usb/gadget/udc/udc-core.c | 23 + drivers/usb/host/Kconfig |9 + drivers/usb/host/Makefile |1 + drivers/usb/host/xhci-imx8.c | 210 ++ include/linux/usb/gadget.h |5 + scripts/Makefile.spl |1 + 27 files changed, 3814 insertions(+) create mode 100644 doc/device-tree-bindings/usb/cdns-usb3.txt create mode 100644 drivers/phy/cdns3-usb-phy.c create mode 100644 drivers/usb/cdns3/Kconfig create mode 100644 drivers/usb/cdns3/Makefile create mode 100644 drivers/usb/cdns3/cdns3-generic.c create mode 100644 drivers/usb/cdns3/cdns3-nxp-reg-def.h create mode 100644 drivers/usb/cdns3/core.c create mode 100644 drivers/usb/cdns3/core.h create mode 100644 drivers/usb/cdns3/dev-regs-macro.h create mode 100644 drivers/usb/cdns3/dev-regs-map.h create mode 100644 drivers/usb/cdns3/gadget-export.h create mode 100644 drivers/usb/cdns3/gadget.c create mode 100644 drivers/usb/cdns3/gadget.h create mode 100644 drivers/usb/cdns3/io.h create mode 100644 drivers/usb/host/xhci-imx8.c -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
