[U-Boot] [PATCH v3] usb: add support for generic EHCI devices
From: Alexey Brodkin Alexey.Brodkin@synopsys.com
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 abrodkin@synopsys.com Reviewed-by: Simon Glass sjg@chromium.org Reviewed-by: Marek Vasut marex@denx.de Cc: Stephen Warren swarren@nvidia.com ---
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
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index 2a2bffe..6bb9caa 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -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 diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile index f70f38c..b9b4471 100644 --- a/drivers/usb/host/Makefile +++ b/drivers/usb/host/Makefile @@ -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 diff --git a/drivers/usb/host/ehci-generic.c b/drivers/usb/host/ehci-generic.c new file mode 100644 index 0000000..22e1ad0 --- /dev/null +++ b/drivers/usb/host/ehci-generic.c @@ -0,0 +1,51 @@ +/* + * Copyright (C) 2015 Alexey Brodkin abrodkin@synopsys.com + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <dm.h> +#include "ehci.h" + +/* + * Even though here we don't explicitly use "struct ehci_ctrl" + * ehci_register() expects it to be the first thing that resides in + * device's private data. + */ +struct generic_ehci { + struct ehci_ctrl ctrl; +}; + +static int ehci_usb_probe(struct udevice *dev) +{ + struct ehci_hccr *hccr = (struct ehci_hccr *)dev_get_addr(dev); + struct ehci_hcor *hcor; + + hcor = (struct ehci_hcor *)((uintptr_t)hccr + + HC_LENGTH(ehci_readl(&hccr->cr_capbase))); + + return ehci_register(dev, hccr, hcor, NULL, 0, USB_INIT_HOST); +} + +static int ehci_usb_remove(struct udevice *dev) +{ + return ehci_deregister(dev); +} + +static const struct udevice_id ehci_usb_ids[] = { + { .compatible = "generic-ehci" }, + { } +}; + +U_BOOT_DRIVER(usb_ehci) = { + .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, +}; +
Hi Marek,
On Mon, 2015-11-30 at 20:47 +0300, Alexey Brodkin wrote:
From: Alexey Brodkin Alexey.Brodkin@synopsys.com
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 abrodkin@synopsys.com Reviewed-by: Simon Glass sjg@chromium.org Reviewed-by: Marek Vasut marex@denx.de Cc: Stephen Warren swarren@nvidia.com
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
On Monday, November 30, 2015 at 06:47:45 PM, Alexey Brodkin wrote:
From: Alexey Brodkin Alexey.Brodkin@synopsys.com
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 abrodkin@synopsys.com Reviewed-by: Simon Glass sjg@chromium.org Reviewed-by: Marek Vasut marex@denx.de Cc: Stephen Warren swarren@nvidia.com
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
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index 2a2bffe..6bb9caa 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -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) ?
[...]
+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.
- .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
Hi Marek,
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.
[...]
+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
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
Hi Marek,
On Mon, 2015-11-30 at 19:21 +0100, Marek Vasut wrote:
On Monday, November 30, 2015 at 07:13:30 PM, Alexey Brodkin wrote:
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).
[...]
+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.
Ok, I'll fix it for ehci generic as well.
And v4 is about to float on mailing list :)
-Alexey
On Tuesday, December 01, 2015 at 07:54:19 PM, Alexey Brodkin wrote:
Hi Marek,
On Mon, 2015-11-30 at 19:21 +0100, Marek Vasut wrote:
On Monday, November 30, 2015 at 07:13:30 PM, Alexey Brodkin wrote:
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).
[...]
+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.
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
Hi,
On 1 December 2015 at 12:07, Marek Vasut marex@denx.de wrote:
On Tuesday, December 01, 2015 at 07:54:19 PM, Alexey Brodkin wrote:
Hi Marek,
On Mon, 2015-11-30 at 19:21 +0100, Marek Vasut wrote:
On Monday, November 30, 2015 at 07:13:30 PM, Alexey Brodkin wrote:
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).
[...]
+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.
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
participants (3)
-
Alexey Brodkin -
Marek Vasut -
Simon Glass