[U-Boot] [PATCH v5 0/7] usb: Add cadence USB3 gadget/host/phy driver

[Sherry Sun]

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%2F11114415%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