Skip to contentSkip to conversation

usb: add support for generic EHCI devices

PATCH New Alexey Brodkin 3 participants 5 messages Nov 13, 2015 – Nov 16, 2015

Subsystems (2)
USB USB ehci driver
lore
Share & tools
lore

Alexey Brodkin

author
#0 lore raw

PATCH 1/1 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 <[email protected]>
Cc Stephen Warren <[email protected]>
Cc Simon Glass <[email protected]>
Cc Marek Vasut <[email protected]>
---
 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
drivers/usb/host/Kconfig +7 -0
index 2a2bffe..a500578 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 y
+	---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 +57 -0

57 lines not on this page · raw message

Signature · 1 line
2.4.3
#1 lore raw

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

Quoted 6 lines · Alexey Brodkin

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.

Quoted 7 lines

For example this allows usage of USB-storage devices with Synopsys DesignWare AXS10x boards.

Signed-off-by Alexey Brodkin <[email protected]>
Cc Stephen Warren <[email protected]>
Cc Simon Glass <[email protected]>
Cc Marek Vasut <[email protected]>
Reviewed-by Marek Vasut <[email protected]>
Quoted 7 lines
---
 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

#2 lore raw

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

In reply to Alexey Brodkin

Hi,

Quoted 13 lines · Alexey Brodkin

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 <[email protected]>
Cc Stephen Warren <[email protected]>
Cc Simon Glass <[email protected]>
Cc Marek Vasut <[email protected]>
Quoted 7 lines
---
 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 <[email protected]>

Please see nits below.

Quoted 5 lines
drivers/usb/host/Kconfig +0 -0
index 2a2bffe..a500578 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 y
+       ---help---
+         Enables support for generic EHCI controller.

such as Synopsys ...
what does 'generic' mean?
Please add a few more details.

Quoted 24 lines
drivers/usb/host/Kconfig +1 -0
+
 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 +4 -0
new file mode 100644
index 0000000..c57ddef
@@ -0,0 +1,57 @@
+/*
+ * Copyright (C) 2015 Alexey Brodkin <[email protected]>
+ *
+ * SPDX-License-Identifier:    GPL-2.0+

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

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

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

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

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

Quoted 23 lines
drivers/usb/host/ehci-generic.c new file +22 -1
+       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,
+};
+
--
Quoted 2 lines

2.4.3

Regards,
Simon

#3 lore raw

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

Quoted 42 lines · Simon Glass

Hi,

On 13 November 2015 at 11:10, Alexey Brodkin

Quoted 13 lines · <[email protected]> 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 <[email protected]>
Cc Stephen Warren <[email protected]>
Cc Simon Glass <[email protected]>
Cc Marek Vasut <[email protected]>
Quoted 8 lines

---

 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 <[email protected]>

Please see nits below.

Quoted 11 lines
drivers/usb/host/Kconfig +0 -0
index 2a2bffe..a500578 100644
@@ -73,4 +73,11 @@ config USB_EHCI_UNIPHIER
 
        ---help---
 
          Enables support for the on-chip EHCI controller on UniPhier
          SoCs.
Quoted 8 lines

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

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

Quoted 2 lines

what does 'generic' mean?
Please add a few more details.

Otherwise, I agree with the rest.

Thanks!

author
#4 lore raw

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

In reply to Simon Glass

Hi Simon,

Quoted 37 lines · Simon Glass

Hi,

Quoted 13 lines · Alexey Brodkin

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 <[email protected]>
Cc Stephen Warren <[email protected]>
Cc Simon Glass <[email protected]>
Cc Marek Vasut <[email protected]>
Quoted 7 lines
---
 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 <[email protected]>

Please see nits below.

Quoted 8 lines
drivers/usb/host/Kconfig +0 -0
index 2a2bffe..a500578 100644
@@ -73,4 +73,11 @@ config USB_EHCI_UNIPHIER
        ---help---
          Enables support for the on-chip EHCI controller on UniPhier SoCs.
Quoted 10 lines

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

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

Quoted 29 lines
Quoted 29 lines
drivers/usb/host/Kconfig +1 -0
+
 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 +9 -0
new file mode 100644
index 0000000..c57ddef
@@ -0,0 +1,57 @@
+/*
+ * Copyright (C) 2015 Alexey Brodkin <[email protected]>
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+
+#include <common.h>
+#include <dm.h>
+#include "ehci.h"
Quoted 8 lines

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

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

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

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

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

Quoted 1 line

return ehci_deregister(dev);

True, this is a reminder of debugging stuff.
Will rework in v2.

-Alexey