Skip to contentSkip to conversation

usb: add support for generic EHCI devices

PATCH Superseded Alexey Brodkin 3 participants 8 messages Nov 30, 2015 – Dec 1, 2015

Subsystems (2)
USB USB ehci driver

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Alexey Brodkin

author
#0 lore raw

PATCH v3 1/1 usb: add support for generic EHCI devices

From: Alexey Brodkin <[email protected]>

This driver is meant to be used with any EHCI-compatible host controller in case if there's no need for platform-specific glue such as setup of controller or PHY's power mode via GPIOs etc.

Signed-off-by Alexey Brodkin <[email protected]>
Reviewed-by Simon Glass <[email protected]>
Reviewed-by Marek Vasut <[email protected]>
Cc Stephen Warren <[email protected]>

---

Changes compared to v2:
* Driver is disabled by default now
* Use uintptr_t instead of uint32_t for "struct ehci_hcor" address calculation

Changes compared to v1:
* Updated commit message with removal of Synopsys board mention
* Cleaned-up ehci_usb_remove()

 drivers/usb/host/Kconfig        |  7 ++++++
 drivers/usb/host/Makefile       |  1 +
 drivers/usb/host/ehci-generic.c | 51 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 59 insertions(+)
 create mode 100644 drivers/usb/host/ehci-generic.c
drivers/usb/host/Kconfig +7 -0
index 2a2bffe..6bb9caa 100644
@@ -73,4 +73,11 @@ config USB_EHCI_UNIPHIER
 	---help---
 	  Enables support for the on-chip EHCI controller on UniPhier SoCs.
 
+config USB_EHCI_GENERIC
+	bool "Support for generic EHCI USB controller"
+	depends on OF_CONTROL
+	default n
+	---help---
+	  Enables support for generic EHCI controller.
+
 endif
drivers/usb/host/Makefile +1 -0
index f70f38c..b9b4471 100644
@@ -32,6 +32,7 @@ else
 obj-$(CONFIG_USB_EHCI_FSL) += ehci-fsl.o
 endif
 obj-$(CONFIG_USB_EHCI_FARADAY) += ehci-faraday.o
+obj-$(CONFIG_USB_EHCI_GENERIC) += ehci-generic.o
 obj-$(CONFIG_USB_EHCI_EXYNOS) += ehci-exynos.o
 obj-$(CONFIG_USB_EHCI_MXC) += ehci-mxc.o
 obj-$(CONFIG_USB_EHCI_MXS) += ehci-mxs.o
drivers/usb/host/ehci-generic.c new file +51 -0

51 lines not on this page · raw message

Signature · 1 line
2.5.0
author
#1 lore raw

PATCH v3 1/1 usb: add support for generic EHCI devices

Hi Marek,

Quoted 13 lines · Alexey Brodkin

From: Alexey Brodkin <[email protected]>

This driver is meant to be used with any EHCI-compatible host controller in case if there's no need for platform-specific glue such as setup of controller or PHY's power mode via GPIOs etc.

Signed-off-by Alexey Brodkin <[email protected]>
Reviewed-by Simon Glass <[email protected]>
Reviewed-by Marek Vasut <[email protected]>
Cc Stephen Warren <[email protected]>

---

Quoted 8 lines

Changes compared to v2:
* Driver is disabled by default now
* Use uintptr_t instead of uint32_t for "struct ehci_hcor" address calculation

Changes compared to v1:
* Updated commit message with removal of Synopsys board mention
* Cleaned-up ehci_usb_remove()

git doesn't Cc people from "Reviewed-by" tags, so adding you and Simon here. Sorry for that noise.

-Alexey

#2 lore raw

PATCH v3 1/1 usb: add support for generic EHCI devices

In reply to Alexey Brodkin
Quoted 34 lines · Alexey Brodkin

From: Alexey Brodkin <[email protected]>

This driver is meant to be used with any EHCI-compatible host controller in case if there's no need for platform-specific glue such as setup of controller or PHY's power mode via GPIOs etc.

Signed-off-by Alexey Brodkin <[email protected]>
Reviewed-by Simon Glass <[email protected]>
Reviewed-by Marek Vasut <[email protected]>
Cc Stephen Warren <[email protected]>

---

Changes compared to v2:
* Driver is disabled by default now
* Use uintptr_t instead of uint32_t for "struct ehci_hcor" address calculation

Changes compared to v1:
* Updated commit message with removal of Synopsys board mention
* Cleaned-up ehci_usb_remove()

 drivers/usb/host/Kconfig        |  7 ++++++
 drivers/usb/host/Makefile       |  1 +
 drivers/usb/host/ehci-generic.c | 51
+++++++++++++++++++++++++++++++++++++++++ 3 files changed, 59
insertions(+)
 create mode 100644 drivers/usb/host/ehci-generic.c
drivers/usb/host/Kconfig +0 -0
index 2a2bffe..6bb9caa 100644

drivers/usb/host/Kconfig config USB_EHCI_UNIPHIER · line 81

Quoted 9 lines
drivers/usb/host/Kconfig +6 -0
@@ -73,4 +73,11 @@ config USB_EHCI_UNIPHIER
 	---help---
 	  Enables support for the on-chip EHCI controller on UniPhier SoCs.
 
+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) ?

[...]

drivers/usb/host/Kconfig config USB_EHCI_UNIPHIER

Quoted 6 lines
drivers/usb/host/Kconfig +6 -0
+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.

drivers/usb/host/Kconfig

Quoted 10 lines
drivers/usb/host/Kconfig +10 -0
+	.name	= "ehci_generic",
+	.id	= UCLASS_USB,
+	.of_match = ehci_usb_ids,
+	.probe = ehci_usb_probe,
+	.remove = ehci_usb_remove,
+	.ops	= &ehci_usb_ops,
+	.priv_auto_alloc_size = sizeof(struct generic_ehci),
+	.flags	= DM_FLAG_ALLOC_PRIV_DMA,
+};
+

Best regards,
Marek Vasut

author
#3 lore raw

PATCH v3 1/1 usb: add support for generic EHCI devices

Hi Marek,

Quoted 12 lines · Marek Vasut

drivers/usb/host/Kconfig config USB_EHCI_UNIPHIER · line 81

Quoted 8 lines · Alexey Brodkin

From: Alexey Brodkin <[email protected]>

drivers/usb/host/Kconfig +6 -0
+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.

Quoted 11 lines

[...]

drivers/usb/host/ehci-generic.c line 41

Quoted 6 lines
drivers/usb/host/ehci-generic.c new file +6 -0
+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? 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) = { 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) = { drivers/usb/host/ehci-tegra.c:U_BOOT_DRIVER(usb_ehci) = { drivers/usb/host/ohci-sunxi.c:U_BOOT_DRIVER(usb_ohci) = { 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] :)

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]?

-Alexey

#4 lore raw

PATCH v3 1/1 usb: add support for generic EHCI devices

Quoted 1 line · Alexey Brodkin

Hi Marek,

Hi!

Quoted 14 lines · Marek Vasut
Quoted 12 lines · Alexey Brodkin

drivers/usb/host/Kconfig config USB_EHCI_UNIPHIER · line 81

Quoted 8 lines

From: Alexey Brodkin <[email protected]>

drivers/usb/host/Kconfig +6 -0
+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) ?

Quoted 8 lines

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 ?

Quoted 12 lines
Quoted 11 lines

[...]

drivers/usb/host/ehci-generic.c line 41

Quoted 6 lines
drivers/usb/host/ehci-generic.c new file +6 -0
+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.

Quoted 1 line

Ok but then some other drivers should be fixed as well, right?

Yes.

Quoted 5 lines

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

Quoted 4 lines

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.

Quoted 2 lines

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

CCing Hans.

Quoted 7 lines

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.

Quoted 3 lines

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

author
#5 lore raw

PATCH v3 1/1 usb: add support for generic EHCI devices

Hi Marek,

Quoted 9 lines · Marek Vasut
Quoted 3 lines · Alexey Brodkin

Hi Marek,

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 ?

Nope EHCI_HCD gets selected, but... ehci-generic uses CONFIG_DM_USB :) And ehci_degister()/ehci_deregister() are only defined in EHCI_HCD is CONFIG_DM_USB used.

So I need to add dependency on CONFIG_DM_USB for ehci_generic.

Looking for other examples of dependencies on CONFIG_DM_USB I was surprised to see ehci-exynos, ehci-marvell, ehci-pci, ehci-sunxi and ehci-tegra all are not yet added in Kconfig.

I.e. all those drivers are selected in headers (in include/configs/xxx.h).

Quoted 12 lines
Quoted 12 lines
Quoted 11 lines

[...]

drivers/usb/host/ehci-generic.c line 41

Quoted 6 lines
drivers/usb/host/ehci-generic.c new file +6 -0
+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.

Quoted 3 lines
Quoted 1 line

Ok but then some other drivers should be fixed as well, right?

Yes.

Ok, I'll fix it for ehci generic as well.

And v4 is about to float on mailing list :)

-Alexey

#6 lore raw

PATCH v3 1/1 usb: add support for generic EHCI devices

Quoted 37 lines · Alexey Brodkin

Hi Marek,

Quoted 9 lines · Marek Vasut
Quoted 3 lines · Alexey Brodkin

Hi Marek,

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 ?

Nope EHCI_HCD gets selected, but... ehci-generic uses CONFIG_DM_USB :) And ehci_degister()/ehci_deregister() are only defined in EHCI_HCD is CONFIG_DM_USB used.

So I need to add dependency on CONFIG_DM_USB for ehci_generic.

Looking for other examples of dependencies on CONFIG_DM_USB I was surprised to see ehci-exynos, ehci-marvell, ehci-pci, ehci-sunxi and ehci-tegra all are not yet added in Kconfig.

I.e. all those drivers are selected in headers (in include/configs/xxx.h).

Quoted 12 lines
Quoted 12 lines
Quoted 11 lines

[...]

drivers/usb/host/ehci-generic.c line 41

Quoted 6 lines
drivers/usb/host/ehci-generic.c new file +6 -0
+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.

Quoted 7 lines
Quoted 3 lines
Quoted 1 line

Ok but then some other drivers should be fixed as well, right?

Yes.

Ok, I'll fix it for ehci generic as well.

And v4 is about to float on mailing list :)

I see, thanks!

I'd like to hear Simon's opinion on this.

Best regards,
Marek Vasut

#7 lore raw

PATCH v3 1/1 usb: add support for generic EHCI devices

Hi,

Quoted 42 lines · Marek Vasut
Quoted 41 lines · Alexey Brodkin

Hi Marek,

Quoted 9 lines · Marek Vasut
Quoted 3 lines · Alexey Brodkin

Hi Marek,

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 ?

Nope EHCI_HCD gets selected, but... ehci-generic uses CONFIG_DM_USB :) And ehci_degister()/ehci_deregister() are only defined in EHCI_HCD is CONFIG_DM_USB used.

So I need to add dependency on CONFIG_DM_USB for ehci_generic.

Looking for other examples of dependencies on CONFIG_DM_USB I was surprised to see ehci-exynos, ehci-marvell, ehci-pci, ehci-sunxi and ehci-tegra all are not yet added in Kconfig.

I.e. all those drivers are selected in headers (in include/configs/xxx.h).

Quoted 15 lines
Quoted 12 lines
Quoted 11 lines

[...]

drivers/usb/host/ehci-generic.c line 41

Quoted 6 lines
drivers/usb/host/ehci-generic.c new file +6 -0
+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.

Quoted 1 line

Ok but then some other drivers should be fixed as well, right?

Yes.

Quoted 7 lines
Quoted 3 lines

Ok, I'll fix it for ehci generic as well.

And v4 is about to float on mailing list :)

I see, thanks!

I'd like to hear Simon's opinion on this.

Yes, good to fix this. When CONFIG_DM_USB goes away, we can drop them all.

Regards,
Simon