Re: [U-Boot] [PATCH v2 5/5] tegra20: add USB ULPI init code

2012-09-05 Thread Igor Grinberg
On 08/21/12 23:18, Lucas Stach wrote:
 This adds the required code to set up a ULPI USB port. It is
 mostly a port of the Linux ULPI setup code with some tweaks
 added for more correctness, discovered along the way of
 debugging this.
 
 To use this both CONFIG_USB_ULPI and CONFIG_USB_ULPI_VIEWPORT
 have to be set in the board configuration file.
 
 v2:
 - move all controller init stuff in the respective functions to
   make them self contained
 - let board define ULPI_REF_CLK to account for the possibility
   that some ULPI phys need a other ref clk than 24MHz
 - don't touch ULPI regs directly, use ULPI framework functions
 - don't hide error messages under debug()
 
 Signed-off-by: Lucas Stach d...@lynxeye.de
 ---
  arch/arm/cpu/armv7/tegra20/usb.c| 154 
 +++-
  arch/arm/include/asm/arch-tegra20/usb.h |  29 --
  2 Dateien geändert, 155 Zeilen hinzugefügt(+), 28 Zeilen entfernt(-)
 
 diff --git a/arch/arm/cpu/armv7/tegra20/usb.c 
 b/arch/arm/cpu/armv7/tegra20/usb.c
 index 77966e5..351300b 100644
 --- a/arch/arm/cpu/armv7/tegra20/usb.c
 +++ b/arch/arm/cpu/armv7/tegra20/usb.c

[...]

 +/* set up the ULPI USB controller with the parameters provided */
 +static int init_ulpi_usb_controller(struct fdt_usb *config,
 + struct usb_ctlr *usbctlr)
 +{
 +#ifdef CONFIG_USB_ULPI
 +/* if board file does not set a ULPI reference frequency we default to 24MHz 
 */
 +#ifndef ULPI_REF_CLK
 +#define ULPI_REF_CLK 2400
 +#endif

I would really like the above ifdefs out of any function.
So, how about something like:
#ifdef CONFIG_USB_ULPI
#ifndef ULPI_REF_CLK
#define ULPI_REF_CLK 2400
#endif
static int init_ulpi_usb_controller(struct fdt_usb *config,
struct usb_ctlr *usbctlr)
{
...
}
#else
static int init_ulpi_usb_controller(struct fdt_usb *config,
struct usb_ctlr *usbctlr)
{
printf(No code to set up ULPI controller, please enable
CONFIG_USB_ULPI and CONFIG_USB_ULPI_VIEWPORT);
return -1;
}
#endif

This way all the ifdef are out of the functions and it makes code
really cleaner and pleasant to read.

Also, if this is to work from config files, then we should
make it CONFIG_ULPI_REF_CLK and add some documentation to the README.


 + u32 val;
 + int loop_count;
 + struct ulpi_viewport ulpi_vp;
 +
 + /* set up ULPI reference clock on pllp_out4 */
 + clock_enable(PERIPH_ID_DEV2_OUT);
 + clock_set_pllout(CLOCK_ID_PERIPH, PLL_OUT4, ULPI_REF_CLK);
 +
 + /* reset ULPI phy */
 + if (fdt_gpio_isvalid(config-phy_reset_gpio)) {
 + fdtdec_setup_gpio(config-phy_reset_gpio);
 + gpio_direction_output(config-phy_reset_gpio.gpio, 0);
 + mdelay(5);
 + gpio_set_value(config-phy_reset_gpio.gpio, 1);
 + }
 +
 + /* Reset the usb controller */
 + clock_enable(config-periph_id);
 + usbf_reset_controller(config, usbctlr);
 +
 + /* enable pinmux bypass */
 + setbits_le32(usbctlr-ulpi_timing_ctrl_0,
 + ULPI_CLKOUT_PINMUX_BYP | 
 ULPI_OUTPUT_PINMUX_BYP);
 +
 + /* Select ULPI parallel interface */
 + clrsetbits_le32(usbctlr-port_sc1, PTS_MASK, PTS_ULPI  PTS_SHIFT);
 +
 + /* enable ULPI transceiver */
 + setbits_le32(usbctlr-susp_ctrl, ULPI_PHY_ENB);
 +
 + /* configure ULPI transceiver timings */
 + val = 0;
 + writel(val, usbctlr-ulpi_timing_ctrl_1);
 +
 + val |= ULPI_DATA_TRIMMER_SEL(4);
 + val |= ULPI_STPDIRNXT_TRIMMER_SEL(4);
 + val |= ULPI_DIR_TRIMMER_SEL(4);
 + writel(val, usbctlr-ulpi_timing_ctrl_1);
 + udelay(10);
 +
 + val |= ULPI_DATA_TRIMMER_LOAD;
 + val |= ULPI_STPDIRNXT_TRIMMER_LOAD;
 + val |= ULPI_DIR_TRIMMER_LOAD;
 + writel(val, usbctlr-ulpi_timing_ctrl_1);
 +
 + /* set up phy for host operation with external vbus supply */
 + ulpi_vp.port_num = 0;
 + ulpi_vp.viewport_addr = (u32)usbctlr-ulpi_viewport;
 +
 + if (ulpi_init(ulpi_vp)) {
 + printf(Tegra ULPI viewport init failed\n);
 + return -1;
 + }
 +
 + ulpi_set_vbus(ulpi_vp, 1, 1);
 + ulpi_set_vbus_indicator(ulpi_vp, 1, 1, 0);
 +
 + /* enable wakeup events */
 + setbits_le32(usbctlr-port_sc1, WKCN | WKDS | WKOC);
 +
 + setbits_le32(usbctlr-susp_ctrl, USB_SUSP_CLR);
 + /* Wait for the phy clock to become valid in 100 ms */
 + for (loop_count = 10; loop_count != 0; loop_count--) {
 + if (readl(usbctlr-susp_ctrl)  USB_PHY_CLK_VALID)
 + break;
 + udelay(1);
 + }

an empty line here would be nice.

 + if (!loop_count)
 + return -1;

and here.

 + clrbits_le32(usbctlr-susp_ctrl, USB_SUSP_CLR);
 +
 + return 0;
 +#else
 + printf(No code to set up ULPI controller, please enable
 + CONFIG_USB_ULPI and CONFIG_USB_ULPI_VIEWPORT);
 + return -1;
 +#endif
  }
  
  

[U-Boot] [PATCH v2 5/5] tegra20: add USB ULPI init code

2012-08-21 Thread Lucas Stach
This adds the required code to set up a ULPI USB port. It is
mostly a port of the Linux ULPI setup code with some tweaks
added for more correctness, discovered along the way of
debugging this.

To use this both CONFIG_USB_ULPI and CONFIG_USB_ULPI_VIEWPORT
have to be set in the board configuration file.

v2:
- move all controller init stuff in the respective functions to
  make them self contained
- let board define ULPI_REF_CLK to account for the possibility
  that some ULPI phys need a other ref clk than 24MHz
- don't touch ULPI regs directly, use ULPI framework functions
- don't hide error messages under debug()

Signed-off-by: Lucas Stach d...@lynxeye.de
---
 arch/arm/cpu/armv7/tegra20/usb.c| 154 +++-
 arch/arm/include/asm/arch-tegra20/usb.h |  29 --
 2 Dateien geändert, 155 Zeilen hinzugefügt(+), 28 Zeilen entfernt(-)

diff --git a/arch/arm/cpu/armv7/tegra20/usb.c b/arch/arm/cpu/armv7/tegra20/usb.c
index 77966e5..351300b 100644
--- a/arch/arm/cpu/armv7/tegra20/usb.c
+++ b/arch/arm/cpu/armv7/tegra20/usb.c
@@ -32,9 +32,17 @@
 #include asm/arch/sys_proto.h
 #include asm/arch/uart.h
 #include asm/arch/usb.h
+#include usb/ulpi.h
 #include libfdt.h
 #include fdtdec.h
 
+#ifdef CONFIG_USB_ULPI
+   #ifndef CONFIG_USB_ULPI_VIEWPORT
+   #error  To use CONFIG_USB_ULPI on Tegra Boards you have to also \
+   define CONFIG_USB_ULPI_VIEWPORT
+   #endif
+#endif
+
 enum {
USB_PORTS_MAX   = 4,/* Maximum ports we allow */
 };
@@ -68,11 +76,13 @@ enum dr_mode {
 struct fdt_usb {
struct usb_ctlr *reg;   /* address of registers in physical memory */
unsigned utmi:1;/* 1 if port has external tranceiver, else 0 */
+   unsigned ulpi:1;/* 1 if port has external ULPI transceiver */
unsigned enabled:1; /* 1 to enable, 0 to disable */
unsigned has_legacy_mode:1; /* 1 if this port has legacy mode */
enum dr_mode dr_mode;   /* dual role mode */
enum periph_id periph_id;/* peripheral id */
struct fdt_gpio_state vbus_gpio;/* GPIO for vbus enable */
+   struct fdt_gpio_state phy_reset_gpio; /* GPIO to reset ULPI phy */
 };
 
 static struct fdt_usb port[USB_PORTS_MAX]; /* List of valid USB ports */
@@ -187,8 +197,8 @@ static void usbf_reset_controller(struct fdt_usb *config,
 */
 }
 
-/* set up the USB controller with the parameters provided */
-static int init_usb_controller(struct fdt_usb *config,
+/* set up the UTMI USB controller with the parameters provided */
+static int init_utmi_usb_controller(struct fdt_usb *config,
struct usb_ctlr *usbctlr, const u32 timing[])
 {
u32 val;
@@ -297,16 +307,109 @@ static int init_usb_controller(struct fdt_usb *config,
if (!loop_count)
return -1;
 
-   return 0;
-}
+   /* Disable ICUSB FS/LS transceiver */
+   clrbits_le32(usbctlr-icusb_ctrl, IC_ENB1);
+
+   /* Select UTMI parallel interface */
+   clrsetbits_le32(usbctlr-port_sc1, PTS_MASK,
+   PTS_UTMI  PTS_SHIFT);
+   clrbits_le32(usbctlr-port_sc1, STS);
 
-static void power_up_port(struct usb_ctlr *usbctlr)
-{
/* Deassert power down state */
clrbits_le32(usbctlr-utmip_xcvr_cfg0, UTMIP_FORCE_PD_POWERDOWN |
UTMIP_FORCE_PD2_POWERDOWN | UTMIP_FORCE_PDZI_POWERDOWN);
clrbits_le32(usbctlr-utmip_xcvr_cfg1, UTMIP_FORCE_PDDISC_POWERDOWN |
UTMIP_FORCE_PDCHRP_POWERDOWN | UTMIP_FORCE_PDDR_POWERDOWN);
+
+   return 0;
+}
+
+/* set up the ULPI USB controller with the parameters provided */
+static int init_ulpi_usb_controller(struct fdt_usb *config,
+   struct usb_ctlr *usbctlr)
+{
+#ifdef CONFIG_USB_ULPI
+/* if board file does not set a ULPI reference frequency we default to 24MHz */
+#ifndef ULPI_REF_CLK
+#define ULPI_REF_CLK 2400
+#endif
+   u32 val;
+   int loop_count;
+   struct ulpi_viewport ulpi_vp;
+
+   /* set up ULPI reference clock on pllp_out4 */
+   clock_enable(PERIPH_ID_DEV2_OUT);
+   clock_set_pllout(CLOCK_ID_PERIPH, PLL_OUT4, ULPI_REF_CLK);
+
+   /* reset ULPI phy */
+   if (fdt_gpio_isvalid(config-phy_reset_gpio)) {
+   fdtdec_setup_gpio(config-phy_reset_gpio);
+   gpio_direction_output(config-phy_reset_gpio.gpio, 0);
+   mdelay(5);
+   gpio_set_value(config-phy_reset_gpio.gpio, 1);
+   }
+
+   /* Reset the usb controller */
+   clock_enable(config-periph_id);
+   usbf_reset_controller(config, usbctlr);
+
+   /* enable pinmux bypass */
+   setbits_le32(usbctlr-ulpi_timing_ctrl_0,
+   ULPI_CLKOUT_PINMUX_BYP | 
ULPI_OUTPUT_PINMUX_BYP);
+
+   /* Select ULPI parallel interface */
+   clrsetbits_le32(usbctlr-port_sc1, PTS_MASK, PTS_ULPI  PTS_SHIFT);
+
+   /* enable ULPI transceiver */
+