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

Similarly to Linux kernel it's nice to have generic driver for EHCI-compatible host controllers.
This implementation is very minimalistic and doesn't have any platform-specific glue code nor phy-related operations.
For example this allows usage of USB-storage devices with Synopsys DesignWare AXS10x boards.
Signed-off-by: Alexey Brodkin abrodkin@synopsys.com Cc: Stephen Warren swarren@nvidia.com Cc: Simon Glass sjg@chromium.org Cc: Marek Vasut marex@denx.de --- drivers/usb/host/Kconfig | 7 +++++ drivers/usb/host/Makefile | 1 + drivers/usb/host/ehci-generic.c | 57 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 65 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..c57ddef --- /dev/null +++ b/drivers/usb/host/ehci-generic.c @@ -0,0 +1,57 @@ +/* + * 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 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) +{ + int ret; + + ret = ehci_deregister(dev); + if (ret) + return ret; + + return 0; +} + +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, +}; +

On Friday, November 13, 2015 at 07:10:42 PM, Alexey Brodkin wrote:
Similarly to Linux kernel it's nice to have generic driver for EHCI-compatible host controllers.
This implementation is very minimalistic and doesn't have any platform-specific glue code nor phy-related operations.
For example this allows usage of USB-storage devices with Synopsys DesignWare AXS10x boards.
Signed-off-by: Alexey Brodkin abrodkin@synopsys.com Cc: Stephen Warren swarren@nvidia.com Cc: Simon Glass sjg@chromium.org Cc: Marek Vasut marex@denx.de
Reviewed-by: Marek Vasut marex@denx.de
drivers/usb/host/Kconfig | 7 +++++ drivers/usb/host/Makefile | 1 + drivers/usb/host/ehci-generic.c | 57 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+) create mode 100644 drivers/usb/host/ehci-generic.c
I'd like to get review from Simon too on the DM part, but from my side, it's OK.
Best regards, Marek Vasut

Hi,
On 13 November 2015 at 11:10, Alexey Brodkin Alexey.Brodkin@synopsys.com wrote:
Similarly to Linux kernel it's nice to have generic driver for EHCI-compatible host controllers.
This implementation is very minimalistic and doesn't have any platform-specific glue code nor phy-related operations.
For example this allows usage of USB-storage devices with Synopsys DesignWare AXS10x boards.
Signed-off-by: Alexey Brodkin abrodkin@synopsys.com Cc: Stephen Warren swarren@nvidia.com Cc: Simon Glass sjg@chromium.org Cc: Marek Vasut marex@denx.de
drivers/usb/host/Kconfig | 7 +++++ drivers/usb/host/Makefile | 1 + drivers/usb/host/ehci-generic.c | 57 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+) create mode 100644 drivers/usb/host/ehci-generic.c
Reviewed-by: Simon Glass sjg@chromium.org
Please see nits below.
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.
such as Synopsys ... what does 'generic' mean? Please add a few more details.
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..c57ddef --- /dev/null +++ b/drivers/usb/host/ehci-generic.c @@ -0,0 +1,57 @@ +/*
- 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 private data.
Yes it probably makes sense to have your own structure here rather than just using struct ehci_ctrl.
- */
+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) +{
int ret;
ret = ehci_deregister(dev);
Up to you, but you could use:
return ehci_deregister(dev);
if (ret)
return ret;
return 0;
+}
+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,
+};
-- 2.4.3
Regards, Simon

On Saturday, November 14, 2015 at 03:05:37 AM, Simon Glass wrote:
Hi,
On 13 November 2015 at 11:10, Alexey Brodkin
Alexey.Brodkin@synopsys.com wrote:
Similarly to Linux kernel it's nice to have generic driver for EHCI-compatible host controllers.
This implementation is very minimalistic and doesn't have any platform-specific glue code nor phy-related operations.
For example this allows usage of USB-storage devices with Synopsys DesignWare AXS10x boards.
Signed-off-by: Alexey Brodkin abrodkin@synopsys.com Cc: Stephen Warren swarren@nvidia.com Cc: Simon Glass sjg@chromium.org Cc: Marek Vasut marex@denx.de
drivers/usb/host/Kconfig | 7 +++++ drivers/usb/host/Makefile | 1 + drivers/usb/host/ehci-generic.c | 57 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+) create mode 100644 drivers/usb/host/ehci-generic.c
Reviewed-by: Simon Glass sjg@chromium.org
Please see nits below.
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.
such as Synopsys ...
Please don't add "such as FOO", it's confusing.
what does 'generic' mean? Please add a few more details.
Otherwise, I agree with the rest.
Thanks!

Hi Simon,
On Fri, 2015-11-13 at 19:05 -0700, Simon Glass wrote:
Hi,
On 13 November 2015 at 11:10, Alexey Brodkin Alexey.Brodkin@synopsys.com wrote:
Similarly to Linux kernel it's nice to have generic driver for EHCI-compatible host controllers.
This implementation is very minimalistic and doesn't have any platform-specific glue code nor phy-related operations.
For example this allows usage of USB-storage devices with Synopsys DesignWare AXS10x boards.
Signed-off-by: Alexey Brodkin abrodkin@synopsys.com Cc: Stephen Warren swarren@nvidia.com Cc: Simon Glass sjg@chromium.org Cc: Marek Vasut marex@denx.de
drivers/usb/host/Kconfig | 7 +++++ drivers/usb/host/Makefile | 1 + drivers/usb/host/ehci-generic.c | 57 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+) create mode 100644 drivers/usb/host/ehci-generic.c
Reviewed-by: Simon Glass sjg@chromium.org
Please see nits below.
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.
such as Synopsys ... what does 'generic' mean? Please add a few more details.
Well "generic" here really means generic. I.e. every EHCI-compatible controller that requires no platform glue like enabling/disabling power/phy via GPIOs etc will work perfectly fine with this driver.
So I'm not really sure what I may put here in description that makes more sense.
Any suggestions?
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..c57ddef --- /dev/null +++ b/drivers/usb/host/ehci-generic.c @@ -0,0 +1,57 @@ +/*
- 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 private data.
Yes it probably makes sense to have your own structure here rather than just using struct ehci_ctrl.
Not clear what do you mean. See http://git.denx.de/?p=u-boot.git;a=blob;f=drivers/usb/host/ehci-hcd.c#l1636 ---------------->8---------------- int ehci_register(struct udevice *dev, struct ehci_hccr *hccr, struct ehci_hcor *hcor, const struct ehci_ops *ops, uint tweaks, enum usb_init_type init) { ... struct ehci_ctrl *ctrl = dev_get_priv(dev); ---------------->8----------------
And dev_get_priv(), see http://git.denx.de/?p=u-boot.git;a=blob;f=drivers/core/device.c#l378 ---------------->8---------------- void *dev_get_priv(struct udevice *dev) { if (!dev) { dm_warn("%s: null device\n", __func__); return NULL; }
return dev->priv; } ---------------->8----------------
So "struct ehci_ctrl" must exist and be the first member of device's private structure.
- */
+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) +{
int ret;
ret = ehci_deregister(dev);
Up to you, but you could use:
return ehci_deregister(dev);
True, this is a reminder of debugging stuff. Will rework in v2.
-Alexey
participants (3)
-
Alexey Brodkin
-
Marek Vasut
-
Simon Glass