[U-Boot] [PATCH v2] usb: add support for generic EHCI devices

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 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..a500578 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 y + ---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..f0e2b85 --- /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 *)((uint32_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 Simon, Marek,
On Wed, 2015-11-18 at 18:26 +0300, Alexey Brodkin wrote:
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 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..a500578 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 y
- ---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..f0e2b85 --- /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 *)((uint32_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,
+};
Any comments on that one or there's a chance it could be applied?
-Alexey

On 18 November 2015 at 08:26, Alexey Brodkin Alexey.Brodkin@synopsys.com wrote:
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 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
Reviewed-by: Simon Glass sjg@chromium.org
Marek do you want to pick this up?
- Simon

On Friday, November 20, 2015 at 09:38:39 PM, Simon Glass wrote:
On 18 November 2015 at 08:26, Alexey Brodkin
Alexey.Brodkin@synopsys.com wrote:
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 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
Reviewed-by: Simon Glass sjg@chromium.org
Marek do you want to pick this up?
I have some comments.
Best regards, Marek Vasut

On Wednesday, November 18, 2015 at 04:26:21 PM, Alexey Brodkin wrote:
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 v1:
- Updated commit message with removal of Synopsys board mention
- Cleaned-up ehci_usb_remove()
[...]
+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 *)((uint32_t)hccr +
This should be uintptr_t for the sake of 64bit systems, no ?
HC_LENGTH(ehci_readl(&hccr->cr_capbase)));
- return ehci_register(dev, hccr, hcor, NULL, 0, USB_INIT_HOST);
+}
I can fix that nit when applying, so let me know what you think please.
Best regards, Marek Vasut

Hi Marek,
On Fri, 2015-11-20 at 21:48 +0100, Marek Vasut wrote:
On Wednesday, November 18, 2015 at 04:26:21 PM, Alexey Brodkin wrote:
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 v1:
- Updated commit message with removal of Synopsys board mention
- Cleaned-up ehci_usb_remove()
[...]
+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 *)((uint32_t)hccr +
This should be uintptr_t for the sake of 64bit systems, no ?
Hm, that's a good point! Indeed I was only thinking about 32-bit systems that I work with. So please do that change.
What's interesting most of other USB drivers do use "uint32_t" so there's a room for improvement it seems :)
-Alexey

On Friday, November 20, 2015 at 10:28:34 PM, Alexey Brodkin wrote:
Hi Marek,
Hi!
On Fri, 2015-11-20 at 21:48 +0100, Marek Vasut wrote:
On Wednesday, November 18, 2015 at 04:26:21 PM, Alexey Brodkin wrote:
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 v1:
- Updated commit message with removal of Synopsys board mention
- Cleaned-up ehci_usb_remove()
[...]
+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 *)((uint32_t)hccr +
This should be uintptr_t for the sake of 64bit systems, no ?
Hm, that's a good point! Indeed I was only thinking about 32-bit systems that I work with.
You work for hardware design company, no ? I'd expect you guys thing about 256byte long cachelines and other such stuff :-)
So please do that change.
What's interesting most of other USB drivers do use "uint32_t" so there's a room for improvement it seems :)
Patches are welcome ;-) Besides, please use u32 instead of uint32_t.
-Alexey
Best regards, Marek Vasut

Hi Marek,
On Fri, 2015-11-20 at 21:48 +0100, Marek Vasut wrote:
On Wednesday, November 18, 2015 at 04:26:21 PM, Alexey Brodkin wrote:
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 v1:
- Updated commit message with removal of Synopsys board mention
- Cleaned-up ehci_usb_remove()
[...]
+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 *)((uint32_t)hccr +
This should be uintptr_t for the sake of 64bit systems, no ?
HC_LENGTH(ehci_readl(&hccr->cr_capbase)));
- return ehci_register(dev, hccr, hcor, NULL, 0, USB_INIT_HOST);
+}
I can fix that nit when applying, so let me know what you think please.
Could you please do that fix and apply the patch?
-Alexey

On Monday, November 30, 2015 at 12:10:23 PM, Alexey Brodkin wrote:
Hi Marek,
On Fri, 2015-11-20 at 21:48 +0100, Marek Vasut wrote:
On Wednesday, November 18, 2015 at 04:26:21 PM, Alexey Brodkin wrote:
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 v1:
- Updated commit message with removal of Synopsys board mention
- Cleaned-up ehci_usb_remove()
[...]
+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 *)((uint32_t)hccr +
This should be uintptr_t for the sake of 64bit systems, no ?
HC_LENGTH(ehci_readl(&hccr->cr_capbase)));
- return ehci_register(dev, hccr, hcor, NULL, 0, USB_INIT_HOST);
+}
I can fix that nit when applying, so let me know what you think please.
Could you please do that fix and apply the patch?
Done and applied, thanks!
Best regards, Marek Vasut
participants (3)
-
Alexey Brodkin
-
Marek Vasut
-
Simon Glass