Re: [U-Boot] [PATCH v2 2/5] sunxi: add USB EHCI driver

2014-07-18 Thread Ian Campbell
On Tue, 2014-07-15 at 23:56 +0200, Roman Byshko wrote:
[...]
> + /* this should be used instead of next two lines if
> +  * sunxi_gpio.c is merged upstream
> +  * gpio_direction_output(sunxi_ehci->gpio_vbus, 1); */
> + sunxi_gpio_set_cfgpin(sunxi_ehci->gpio_vbus, SUNXI_GPIO_OUTPUT);
> + sunxi_gpio_output(sunxi_ehci->gpio_vbus, 1);

Please can you base v3 on:
  git://git.denx.de/u-boot-sunxi.git master

This includes the gpio stuff which you need here.

Cheers,
Ian.


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


Re: [U-Boot] [PATCH v2 2/5] sunxi: add USB EHCI driver

2014-07-17 Thread Marek Vasut
On Tuesday, July 15, 2014 at 11:56:49 PM, Roman Byshko wrote:

Please start writing commit messages for the patches.
[...]
> diff --git a/drivers/usb/host/ehci-sunxi.c b/drivers/usb/host/ehci-sunxi.c
> new file mode 100644
> index 000..8e2baa9
> --- /dev/null
> +++ b/drivers/usb/host/ehci-sunxi.c
> @@ -0,0 +1,212 @@
> +/*
> + * Copyright (C) 2014 Roman Byshko
> + *
> + * Roman Byshko 
> + *
> + * Based on code from
> + * Allwinner Technology Co., Ltd. 
> + *
> + * SPDX-License-Identifier:  GPL-2.0+
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include "ehci.h"
> +
> +#define BIT(x)   (1 << (x))

Please remove this code obfuscation, just use (1 << n) in the definitions below.

> +#define SUNXI_USB1_IO_BASE   0x01c14000
> +#define SUNXI_USB2_IO_BASE   0x01c1c000

Please implement an get_io_base() or such function, since these addresses are 
likely to change eventually.

> +#define SUNXI_USB_PMU_IRQ_ENABLE 0x800
> +#define SUNXI_USB_CSR0x01c13404
> +#define SUNXI_USB_PASSBY_EN  1
> +
> +#define SUNXI_EHCI_AHB_ICHR8_EN  BIT(10)
> +#define SUNXI_EHCI_AHB_INCR4_BURST_ENBIT(9)
> +#define SUNXI_EHCI_AHB_INCRX_ALIGN_ENBIT(8)
> +#define SUNXI_EHCI_ULPI_BYPASS_ENBIT(0)
> +
> +static struct sunxi_ehci_hcd {
> + void *ehci_base;
> + struct usb_hcd *hcd;
> + int usb_rst_mask;
> + int ahb_clk_mask;
> + int gpio_vbus;
> + void *csr;
> + int irq;
> + int id;
> +} sunxi_echi_hcd[CONFIG_USB_MAX_CONTROLLER_COUNT] = {

No need to use this [CONFIG...] , just use [] and the compiler will calculate 
the size.

> + [0] = {

No need for such explicit enumeration.

> + .ehci_base = (void *) SUNXI_USB1_IO_BASE,
> + .usb_rst_mask = CCM_USB_CTRL_PHY1_RST,
> + .ahb_clk_mask = BIT(AHB_GATE_OFFSET_USB_EHCI0),
> + .gpio_vbus = CONFIG_SUNXI_USB_VBUS0_GPIO,
> + .csr = (void*) SUNXI_USB_CSR,
> + .irq = 39,
> + .id = 1,
> + },
> +#if (CONFIG_USB_MAX_CONTROLLER_COUNT > 1)
> + [1] = {
> + .ehci_base = (void *) SUNXI_USB2_IO_BASE,
> + .usb_rst_mask = CCM_USB_CTRL_PHY2_RST,
> + .ahb_clk_mask = BIT(AHB_GATE_OFFSET_USB_EHCI1),
> + .gpio_vbus = CONFIG_SUNXI_USB_VBUS1_GPIO,
> + .csr = (void*) SUNXI_USB_CSR,
> + .irq = 40,
> + .id = 2,
> + }
> +#endif
> +};
> +
> +static int sunxi_gpio_output(u32 pin, u32 val)
> +{
> + u32 bank = GPIO_BANK(pin);
> + u32 num = GPIO_NUM(pin);
> + struct sunxi_gpio *pio =
> + &((struct sunxi_gpio_reg *)SUNXI_PIO_BASE)->gpio_bank[bank];

Is this still an USB driver or is this now a GPIO driver ?

> + if (val)
> + setbits_le32(&pio->dat, 0x1 << num);
> + else
> + clrbits_le32(&pio->dat, 0x1 << num);
> +
> + return 0;
> +}

[...]

> +static void sunxi_ehci_enable(struct sunxi_ehci_hcd *sunxi_ehci)
> +{
> + struct sunxi_ccm_reg *ccm = (struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
> +
> + setbits_le32(&ccm->usb_clk_cfg, sunxi_ehci->usb_rst_mask);
> + setbits_le32(&ccm->ahb_gate0, sunxi_ehci->ahb_clk_mask);
> +
> + sunxi_usb_phy_init(sunxi_ehci);
> +
> + sunxi_usb_passby(sunxi_ehci, SUNXI_USB_PASSBY_EN);
> +
> + /* this should be used instead of next two lines if
> +  * sunxi_gpio.c is merged upstream
> +  * gpio_direction_output(sunxi_ehci->gpio_vbus, 1); */

Please fix the comment ( http://www.denx.de/wiki/U-Boot/CodingStyle )

> + sunxi_gpio_set_cfgpin(sunxi_ehci->gpio_vbus, SUNXI_GPIO_OUTPUT);
> + sunxi_gpio_output(sunxi_ehci->gpio_vbus, 1);
> +}
> +
> +static void sunxi_ehci_disable(struct sunxi_ehci_hcd *sunxi_ehci)
> +{
> + struct sunxi_ccm_reg *ccm = (struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
> +
> + /* this should be used instead of next two lines if
> +  * sunxi_gpio.c is merged upstream
> +  * gpio_direction_output(sunxi_ehci->gpio_vbus, 0); */

DTTO.

> + sunxi_gpio_set_cfgpin(sunxi_ehci->gpio_vbus, SUNXI_GPIO_OUTPUT);
> + sunxi_gpio_output(sunxi_ehci->gpio_vbus, 0);
> +
> + sunxi_usb_passby(sunxi_ehci, !SUNXI_USB_PASSBY_EN);
> +
> + clrbits_le32(&ccm->ahb_gate0, sunxi_ehci->ahb_clk_mask);
> + clrbits_le32(&ccm->usb_clk_cfg, sunxi_ehci->usb_rst_mask);
> +}
> +
> +int ehci_hcd_init(int index, enum usb_init_type init, struct ehci_hccr
> **hccr, + struct ehci_hcor **hcor)
> +{
> + struct sunxi_ccm_reg *ccm = (struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
> + struct sunxi_ehci_hcd *sunxi_ehci = &sunxi_echi_hcd[index];
> +
> + /* enable common PHY only once */
> + if (index == 0)
> + setbits_le32(&ccm->usb_clk_cfg, CCM_USB_CTRL_PHYGATE);

This would fail if I enabled only controller #1 , which is perfectly legal 
operation. Just add a counter here and disable the clock upon last call of 
ehci_hcd_stop() .

> + s

Re: [U-Boot] [PATCH v2 2/5] sunxi: add USB EHCI driver

2014-07-15 Thread Ian Campbell
On Tue, 2014-07-15 at 23:56 +0200, Roman Byshko wrote:
> diff --git a/drivers/usb/host/ehci-sunxi.c b/drivers/usb/host/ehci-sunxi.c
> new file mode 100644
> index 000..8e2baa9
> --- /dev/null
> +++ b/drivers/usb/host/ehci-sunxi.c
> @@ -0,0 +1,212 @@
> +/*
> + * Copyright (C) 2014 Roman Byshko
> + *
> + * Roman Byshko 

Are Roman Byshko and "arokux " the same person then?
If not then who wrote this code?

Ian.

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


[U-Boot] [PATCH v2 2/5] sunxi: add USB EHCI driver

2014-07-15 Thread Roman Byshko
Signed-off-by: Roman Byshko 
---
 drivers/usb/host/Makefile |   1 +
 drivers/usb/host/ehci-sunxi.c | 212 ++
 2 files changed, 213 insertions(+)
 create mode 100644 drivers/usb/host/ehci-sunxi.c

diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index 04c1a64..c4f5157 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -35,6 +35,7 @@ obj-$(CONFIG_USB_EHCI_PPC4XX) += ehci-ppc4xx.o
 obj-$(CONFIG_USB_EHCI_MARVELL) += ehci-marvell.o
 obj-$(CONFIG_USB_EHCI_PCI) += ehci-pci.o
 obj-$(CONFIG_USB_EHCI_SPEAR) += ehci-spear.o
+obj-$(CONFIG_USB_EHCI_SUNXI) += ehci-sunxi.o
 obj-$(CONFIG_USB_EHCI_TEGRA) += ehci-tegra.o
 obj-$(CONFIG_USB_EHCI_VCT) += ehci-vct.o
 obj-$(CONFIG_USB_EHCI_RMOBILE) += ehci-rmobile.o
diff --git a/drivers/usb/host/ehci-sunxi.c b/drivers/usb/host/ehci-sunxi.c
new file mode 100644
index 000..8e2baa9
--- /dev/null
+++ b/drivers/usb/host/ehci-sunxi.c
@@ -0,0 +1,212 @@
+/*
+ * Copyright (C) 2014 Roman Byshko
+ *
+ * Roman Byshko 
+ *
+ * Based on code from
+ * Allwinner Technology Co., Ltd. 
+ *
+ * SPDX-License-Identifier:GPL-2.0+
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "ehci.h"
+
+#define BIT(x) (1 << (x))
+
+#define SUNXI_USB1_IO_BASE 0x01c14000
+#define SUNXI_USB2_IO_BASE 0x01c1c000
+
+#define SUNXI_USB_PMU_IRQ_ENABLE   0x800
+#define SUNXI_USB_CSR  0x01c13404
+#define SUNXI_USB_PASSBY_EN1
+
+#define SUNXI_EHCI_AHB_ICHR8_ENBIT(10)
+#define SUNXI_EHCI_AHB_INCR4_BURST_EN  BIT(9)
+#define SUNXI_EHCI_AHB_INCRX_ALIGN_EN  BIT(8)
+#define SUNXI_EHCI_ULPI_BYPASS_EN  BIT(0)
+
+static struct sunxi_ehci_hcd {
+   void *ehci_base;
+   struct usb_hcd *hcd;
+   int usb_rst_mask;
+   int ahb_clk_mask;
+   int gpio_vbus;
+   void *csr;
+   int irq;
+   int id;
+} sunxi_echi_hcd[CONFIG_USB_MAX_CONTROLLER_COUNT] = {
+   [0] = {
+   .ehci_base = (void *) SUNXI_USB1_IO_BASE,
+   .usb_rst_mask = CCM_USB_CTRL_PHY1_RST,
+   .ahb_clk_mask = BIT(AHB_GATE_OFFSET_USB_EHCI0),
+   .gpio_vbus = CONFIG_SUNXI_USB_VBUS0_GPIO,
+   .csr = (void*) SUNXI_USB_CSR,
+   .irq = 39,
+   .id = 1,
+   },
+#if (CONFIG_USB_MAX_CONTROLLER_COUNT > 1)
+   [1] = {
+   .ehci_base = (void *) SUNXI_USB2_IO_BASE,
+   .usb_rst_mask = CCM_USB_CTRL_PHY2_RST,
+   .ahb_clk_mask = BIT(AHB_GATE_OFFSET_USB_EHCI1),
+   .gpio_vbus = CONFIG_SUNXI_USB_VBUS1_GPIO,
+   .csr = (void*) SUNXI_USB_CSR,
+   .irq = 40,
+   .id = 2,
+   }
+#endif
+};
+
+static int sunxi_gpio_output(u32 pin, u32 val)
+{
+   u32 bank = GPIO_BANK(pin);
+   u32 num = GPIO_NUM(pin);
+   struct sunxi_gpio *pio =
+   &((struct sunxi_gpio_reg *)SUNXI_PIO_BASE)->gpio_bank[bank];
+
+   if (val)
+   setbits_le32(&pio->dat, 0x1 << num);
+   else
+   clrbits_le32(&pio->dat, 0x1 << num);
+
+   return 0;
+}
+
+static void usb_phy_write(struct sunxi_ehci_hcd *sunxi_ehci, int addr,
+ int data, int len)
+{
+   int temp = 0, j = 0, usbc_bit = 0;
+   void *dest = sunxi_ehci->csr;
+
+   usbc_bit = BIT(sunxi_ehci->id * 2);
+   for (j = 0; j < len; j++) {
+   /* set the bit address to be written */
+   clrbits_le32(dest, 0xff << 8);
+   setbits_le32(dest, (addr + j) << 8);
+
+   clrbits_le32(dest, usbc_bit);
+   /* set data bit */
+   if (data & 0x1)
+   setbits_le32(dest, BIT(7));
+   else
+   clrbits_le32(dest, BIT(7));
+
+   setbits_le32(dest, usbc_bit);
+
+   clrbits_le32(dest, usbc_bit);
+
+   data >>= 1;
+   }
+}
+
+static void sunxi_usb_phy_init(struct sunxi_ehci_hcd *sunxi_ehci)
+{
+   /* The following comments are machine
+* translated from Chinese, you have been warned!
+*/
+
+   /* adjust PHY's magnitude and rate */
+   usb_phy_write(sunxi_ehci, 0x20, 0x14, 5);
+
+   /* threshold adjustment disconnect */
+   usb_phy_write(sunxi_ehci, 0x2a, 3, 2);
+
+   return;
+}
+
+static void sunxi_usb_passby(struct sunxi_ehci_hcd *sunxi_ehci, int enable)
+{
+   unsigned long reg_value = 0;
+   unsigned long bits = 0;
+   void *addr = sunxi_ehci->ehci_base + SUNXI_USB_PMU_IRQ_ENABLE;
+
+   bits = SUNXI_EHCI_AHB_ICHR8_EN |
+   SUNXI_EHCI_AHB_INCR4_BURST_EN |
+   SUNXI_EHCI_AHB_INCRX_ALIGN_EN |
+   SUNXI_EHCI_ULPI_BYPASS_EN;
+
+   if (enable)
+   setbits_le32(addr, bits);
+   else
+   clrbits_le32(addr, bits);
+
+   return;
+}
+
+static void sunxi_ehci_enable(struct sunxi_ehci_hcd *sunxi_ehci)
+{
+   struct su