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

2015-11-30 Thread Alexey Brodkin 
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 
> > Reviewed-by: Simon Glass 
> > Reviewed-by: Marek Vasut 
> > Cc: Stephen Warren 
> > ---
> > 
> > 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(>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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2015-11-30 Thread Marek Vasut 
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 
> > > Reviewed-by: Simon Glass 
> > > Reviewed-by: Marek Vasut 
> > > Cc: Stephen Warren 
> > > ---
> > > 
> > > 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(>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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2015-11-20 Thread Alexey Brodkin 
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 
> > Reviewed-by: Simon Glass 
> > Reviewed-by: Marek Vasut 
> > Cc: Stephen Warren 
> > ---
> > 
> > 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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2015-11-20 Thread Marek Vasut 
On Friday, November 20, 2015 at 09:38:39 PM, Simon Glass wrote:
> On 18 November 2015 at 08:26, 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 
> > Reviewed-by: Simon Glass 
> > Reviewed-by: Marek Vasut 
> > Cc: Stephen Warren 
> > ---
> > 
> > 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 
> 
> Marek do you want to pick this up?

I have some comments.

Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2015-11-20 Thread Simon Glass 
On 18 November 2015 at 08:26, 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 
> Reviewed-by: Simon Glass 
> Reviewed-by: Marek Vasut 
> Cc: Stephen Warren 
> ---
>
> 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 

Marek do you want to pick this up?

- Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2015-11-20 Thread 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 
> Reviewed-by: Simon Glass 
> Reviewed-by: Marek Vasut 
> Cc: Stephen Warren 
> ---
> 
> 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(>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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2015-11-20 Thread Alexey Brodkin 
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 
> Reviewed-by: Simon Glass 
> Reviewed-by: Marek Vasut 
> Cc: Stephen Warren 
> ---
> 
> 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 000..f0e2b85
> --- /dev/null
> +++ b/drivers/usb/host/ehci-generic.c
> @@ -0,0 +1,51 @@
> +/*
> + * Copyright (C) 2015 Alexey Brodkin 
> + *
> + * SPDX-License-Identifier:  GPL-2.0+
> + */
> +
> +#include 
> +#include 
> +#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(>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= _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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2015-11-20 Thread Marek Vasut 
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 
> > > Reviewed-by: Simon Glass 
> > > Reviewed-by: Marek Vasut 
> > > Cc: Stephen Warren 
> > > ---
> > > 
> > > 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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2015-11-18 Thread Alexey Brodkin 
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 
Reviewed-by: Simon Glass 
Reviewed-by: Marek Vasut 
Cc: Stephen Warren 
---

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 000..f0e2b85
--- /dev/null
+++ b/drivers/usb/host/ehci-generic.c
@@ -0,0 +1,51 @@
+/*
+ * Copyright (C) 2015 Alexey Brodkin 
+ *
+ * SPDX-License-Identifier:GPL-2.0+
+ */
+
+#include 
+#include 
+#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(>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= _usb_ops,
+   .priv_auto_alloc_size = sizeof(struct generic_ehci),
+   .flags  = DM_FLAG_ALLOC_PRIV_DMA,
+};
+
-- 
2.5.0

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot