From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marek Vasut <marex@denx.de>
Date: Mon, 30 Nov 2015 19:21:06 +0100
Subject: [U-Boot] [PATCH v3] usb: add support for generic EHCI devices
In-Reply-To: <1448907210.2903.26.camel@synopsys.com>
References: <1448905665-24748-1-git-send-email-abrodkin@synopsys.com>
 <201511301905.04848.marex@denx.de> <1448907210.2903.26.camel@synopsys.com>
Message-ID: <201511301921.06861.marex@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On Monday, November 30, 2015 at 07:13:30 PM, Alexey Brodkin wrote:
> Hi Marek,

Hi!

> On Mon, 2015-11-30 at 19:05 +0100, Marek Vasut wrote:
> > On Monday, November 30, 2015 at 06:47:45 PM, Alexey Brodkin wrote:
> > > From: Alexey Brodkin <Alexey.Brodkin@synopsys.com>
> > > 
> > > +config USB_EHCI_GENERIC
> > > +	bool "Support for generic EHCI USB controller"
> > > +	depends on OF_CONTROL
> > > +	default n
> > > +	---help---
> > > +	  Enables support for generic EHCI controller.
> > 
> > This should depend on EHCI_HCD somehow, no (since it's using
> > ehci_deregister and friends) ?
> 
> This symbol is in "if USB_EHCI_HCD" so if USB_EHCI_HCD is not enabled
> EHCI_GENERIC won't be visible and hence USB_EHCI_HCD in defconfig.
> 
> Otherwise we'll need to add USB_EHCI_HCD dependency for other EHCI drivers
> such as USB_EHCI_MARVELL, USB_EHCI_MX6 and USB_EHCI_UNIPHIER.
> Do we want to do it? :)
> 
> Please check drivers/usb/host/Kconfig.

The order there seems correct. But how is it possible that your driver
triggered the build error on the ph1_sld8,ph1_sld3,ph1_ld4 boards ? I
suspect because it was enabled by default, but didn't "select" the
EHCI_HCD ?

> > [...]
> > 
> > > +static const struct udevice_id ehci_usb_ids[] = {
> > > +	{ .compatible = "generic-ehci" },
> > > +	{ }
> > > +};
> > > +
> > > +U_BOOT_DRIVER(usb_ehci) = {
> > 
> > The driver name should be ehci_generic, not usb_ehci, otherwise this will
> > collide with other drivers who do the same mistake.
> 
> Ok but then some other drivers should be fixed as well, right?

Yes.

> See:
> ----------------------->8------------------------
> git grep U_BOOT_DRIVER drivers/usb/host/
> drivers/usb/host/dwc2.c:U_BOOT_DRIVER(usb_dwc2) = {
> drivers/usb/host/ehci-exynos.c:U_BOOT_DRIVER(usb_ehci) = {

CCing Lukasz

> drivers/usb/host/ehci-generic.c:U_BOOT_DRIVER(usb_ehci) = {
> drivers/usb/host/ehci-marvell.c:U_BOOT_DRIVER(ehci_mvebu) = {
> drivers/usb/host/ehci-pci.c:U_BOOT_DRIVER(ehci_pci) = {
> drivers/usb/host/ehci-sunxi.c:U_BOOT_DRIVER(usb_ehci) = {

This was fixed by a patch I posted just a while ago.

> drivers/usb/host/ehci-tegra.c:U_BOOT_DRIVER(usb_ehci) = {
> drivers/usb/host/ohci-sunxi.c:U_BOOT_DRIVER(usb_ohci) = {

CCing Hans.

> drivers/usb/host/usb-sandbox.c:U_BOOT_DRIVER(usb_sandbox) = {
> drivers/usb/host/usb-uclass.c:U_BOOT_DRIVER(usb_dev_generic_drv) = {
> drivers/usb/host/xhci-exynos5.c:U_BOOT_DRIVER(usb_xhci) = {
> ----------------------->8------------------------
> 
> I believe it all works because we don't enable 2 drivers at a time
> [usually] :)

Correct. I trapped this on sunxi just today. Propagation of this error
must be stopped.

> And in that light I don't see a point in having different names here.
> Or you think there's a chance to have more than one USB controller enabled
> simultaneously [and if it is possible at all with current implementation]?

I can have ehci-pci and ehci-somethingelse enabled, so yes, the possibility
is here and since Tom triggered it on sunxi, it already happens.

Best regards,
Marek Vasut