Tegra 2 USB ULPI series
Subsystems (3)
v2 cover Tegra 2 USB ULPI series
With this series we are able to initialize USB controllers using an external ULPI phy AKA USB2 on Tegra 2 devices.
This was tested to work on a Toradex Colibri T20 board, where USB2 is used to access the ASIX ethernet chipset. Testing was done with "tegra20: usb: rework set_host_mode" [1] applied. I did not spot any regressions on the UTMI ports.
v2 incorporates all the review feedback I've got so far, including now trying harder to enable VBus in all configurations.
Patch 3 is already in u-boot-usb and only provided as the ULPI init code now depends on it.
Igor could you please take a look at Patch 4?
Patchset is based on top of u-boot-tegra/next
[1] http://lists.denx.de/pipermail/u-boot/2012-August/130177.html
Lucas Stach (5):
tegra20: complete periph_id enum
tegra20: add clock_set_pllout function
usb: fix ulpi_set_vbus prototype
usb: ulpi: add indicator configuration function
tegra20: add USB ULPI init code
arch/arm/cpu/armv7/tegra20/usb.c | 154 +++++++++++++++++++++++----- arch/arm/cpu/tegra20-common/clock.c | 39 +++++++ arch/arm/cpu/tegra20-common/warmboot_avp.c | 2 +- arch/arm/include/asm/arch-tegra20/clk_rst.h | 11 +- arch/arm/include/asm/arch-tegra20/clock.h | 25 +++++ arch/arm/include/asm/arch-tegra20/usb.h | 29 +++++- drivers/usb/ulpi/ulpi.c | 26 ++++- include/usb/ulpi.h | 13 ++-
8 Dateien ge?ndert, 262 Zeilen hinzugef?gt(+), 37 Zeilen entfernt(-)
Signature · 1 line
1.7.11.4
PATCH v2 1/5 tegra20: complete periph_id enum
Most Tegra boards output the ULPI reference clock on pad DEV2.
Complete the periph_id enum so that we are able to enable this clock output circuit.
--- arch/arm/cpu/tegra20-common/clock.c | 1 + arch/arm/include/asm/arch-tegra20/clock.h | 6 ++++++
2 Dateien ge?ndert, 7 Zeilen hinzugef?gt(+)
arch/arm/cpu/tegra20-common/clock.c +1 -0
@@ -502,6 +502,7 @@ static int clock_periph_id_isvalid(enum periph_id id)case PERIPH_ID_RESERVED81 :case PERIPH_ID_RESERVED82 :case PERIPH_ID_RESERVED83 : +case PERIPH_ID_RESERVED91 :printf ("Peripheral id %d is reserved\n" , id);break ;default :
arch/arm/include/asm/arch-tegra20/clock.h +6 -0
@@ -175,6 +175,12 @@ enum periph_id {/* 88 */ PERIPH_ID_CRAM2 , +PERIPH_ID_SYNC_CLK_DOUBLER , +PERIPH_ID_CLK_M_DOUBLER , +PERIPH_ID_RESERVED91 , +PERIPH_ID_SUS_OUT , +PERIPH_ID_DEV2_OUT , +PERIPH_ID_DEV1_OUT ,PERIPH_ID_COUNT , PERIPH_ID_NONE = -1,
Signature · 1 line
1.7.11.4
PATCH v2 2/5 tegra20: add clock_set_pllout function
In reply to Lucas StachCommon practice on Tegra 2 boards is to use the pllp_out4 FO to generate the ULPI reference clock. For this to work we have to override the default hardware generated output divider.
This function adds a clean way to do so.
v2:
- check if pllout is valid
--- arch/arm/cpu/tegra20-common/clock.c | 38 +++++++++++++++++++++++++++++ arch/arm/cpu/tegra20-common/warmboot_avp.c | 2 +- arch/arm/include/asm/arch-tegra20/clk_rst.h | 11 +++++++-- arch/arm/include/asm/arch-tegra20/clock.h | 19 +++++++++++++++
4 Dateien ge?ndert, 67 Zeilen hinzugef?gt(+), 3 Zeilen entfernt(-)
arch/arm/cpu/tegra20-common/clock.c +38 -0
@@ -396,6 +396,16 @@ static s8 periph_id_to_internal_id[PERIPH_ID_COUNT] = { NONE(CRAM2), }; +/* number of clock outputs of a PLL */ +static const u8 pll_num_clkouts[] = { +1 ,/* PLLC */ +1 ,/* PLLM */ +4 ,/* PLLP */ +1 ,/* PLLA */ +0 ,/* PLLU */ +0 ,/* PLLD */ +}; +/* * Get the oscillator frequency, from the corresponding hardware configuration * field.
@@ -604,6 +614,34 @@ unsigned long clock_get_periph_rate(enum periph_id periph_id, (readl(reg) & OUT_CLK_DIVISOR_MASK) >> OUT_CLK_DIVISOR_SHIFT); } +int clock_set_pllout (enum clock_id clkid,enum pll_out_id pllout,unsigned rate) +{ +struct clk_pll *pll =get_pll (clkid); +int data =0 , div =0 , offset =0 ; + +if (!clock_id_is_pll (clkid)) +return -1 ; + +if (pllout +1 > pll_num_clkouts[clkid]) +return -1 ; + + div =clk_get_divider (8 , pll_rate[clkid], rate); + +if (div <0 ) +return -1 ; + +/* out2 and out4 are in the high part of the register */ +if (pllout ==PLL_OUT2 || pllout ==PLL_OUT4 ) + offset =16 ; + + data = (div <<PLL_OUT_RATIO_SHIFT ) | +PLL_OUT_OVRRIDE |PLL_OUT_CLKEN |PLL_OUT_RSTN ; +clrsetbits_le32 (&pll->pll_out[pllout >>1 ], +PLL_OUT_RATIO_MASK << offset, data << offset); + +return 0 ; +} +/** * Find the best available 7.1 format divisor given a parent clock rate and * required child clock rate. This function assumes that a second-stage
arch/arm/cpu/tegra20-common/warmboot_avp.c +1 -1
@@ -214,7 +214,7 @@ void wb_start(void) reg =PLLM_OUT1_RSTN_RESET_DISABLE |PLLM_OUT1_CLKEN_ENABLE |PLLM_OUT1_RATIO_VAL_8 ; -writel (reg, &clkrst->crc_pll[CLOCK_ID_MEMORY ].pll_out); +writel (reg, &clkrst->crc_pll[CLOCK_ID_MEMORY ].pll_out[0 ]); reg =SCLK_SWAKE_FIQ_SRC_PLLM_OUT1 |SCLK_SWAKE_IRQ_SRC_PLLM_OUT1 | SCLK_SWAKE_RUN_SRC_PLLM_OUT1 | SCLK_SWAKE_IDLE_SRC_PLLM_OUT1 |
arch/arm/include/asm/arch-tegra20/clk_rst.h +9 -2
@@ -27,8 +27,7 @@/* PLL registers - there are several PLLs in the clock controller */ struct clk_pll {uint pll_base;/* the control register */ -uint pll_out;/* output control */ -uint reserved; +uint pll_out[2 ];/* output control */ uint pll_misc;/* other misc things */ };
@@ -112,6 +111,14 @@ struct clk_rst_ctlr {#define PLL_DIVM_SHIFT 0#define PLL_DIVM_MASK (0x1f << PLL_DIVM_SHIFT) +/* CLK_RST_CONTROLLER_PLLx_OUTx_0 */ +#define PLL_OUT_RSTN (1 << 0) +#define PLL_OUT_CLKEN (1 << 1) +#define PLL_OUT_OVRRIDE (1 << 2) + +#define PLL_OUT_RATIO_SHIFT 8 +#define PLL_OUT_RATIO_MASK (0xffU << PLL_OUT_RATIO_SHIFT) +/* CLK_RST_CONTROLLER_PLLx_MISC_0 */ #define PLL_CPCON_SHIFT 8#define PLL_CPCON_MASK (15U << PLL_CPCON_SHIFT)
arch/arm/include/asm/arch-tegra20/clock.h +19 -0
@@ -186,6 +186,13 @@ enum periph_id { PERIPH_ID_NONE = -1, }; +enum pll_out_id { +PLL_OUT1 , +PLL_OUT2 , +PLL_OUT3 , +PLL_OUT4 +}; +/* Converts a clock number to a clock register: 0=L, 1=H, 2=U */ #define PERIPH_REG (id) ((id) >> 5)
@@ -218,6 +225,18 @@ unsigned long clock_start_pll(enum clock_id id, u32 divm, u32 divn, u32 divp, u32 cpcon, u32 lfcon);/** +* Set PLL output frequency +* +* @param clkid clock id +* @param pllout pll output id ( +* @param rate desired output rate +* +* @return 0 if ok, -1 on error (invalid clock id or no suitable divider) +*/ +int clock_set_pllout (enum clock_id clkid,enum pll_out_id pllout, +unsigned rate); + +/** * Read low-level parameters of a PLL. * * @param id clock id to read (note: USB is not supported)
Signature · 1 line
1.7.11.4
PATCH v2 3/5 usb: fix ulpi_set_vbus prototype
In reply to Lucas StachMatch the name of the header prototype with the actual implementation.
--- include/usb/ulpi.h | 2 +-
1 Datei ge?ndert, 1 Zeile hinzugef?gt(+), 1 Zeile entfernt(-)
include/usb/ulpi.h +1 -1
@@ -61,7 +61,7 @@ int ulpi_select_transceiver(struct ulpi_viewport *ulpi_vp, unsigned speed);* * returns 0 on success, ULPI_ERROR on failure. */ -int ulpi_enable_vbus (struct ulpi_viewport *ulpi_vp, +int ulpi_set_vbus (struct ulpi_viewport *ulpi_vp,int on,int ext_power,int ext_ind);/*
Signature · 1 line
1.7.11.4
PATCH v2 4/5 usb: ulpi: add indicator configuration function
In reply to Lucas StachAllows for easy configuration of the VBUS indicator related ULPI config bits.
Also move the external indicator setup from ulpi_set_vbus() to the new function.
--- drivers/usb/ulpi/ulpi.c | 26 ++++++++++++++++++++++---- include/usb/ulpi.h | 13 +++++++++++--
2 Dateien ge?ndert, 33 Zeilen hinzugef?gt(+), 6 Zeilen entfernt(-)
drivers/usb/ulpi/ulpi.c +22 -4
@@ -106,20 +106,38 @@ int ulpi_select_transceiver(struct ulpi_viewport *ulpi_vp, unsigned speed)return ulpi_write (ulpi_vp, &ulpi->function_ctrl, val); } -int ulpi_set_vbus (struct ulpi_viewport *ulpi_vp,int on,int ext_power, -int ext_ind) +int ulpi_set_vbus (struct ulpi_viewport *ulpi_vp,int on,int ext_power) {u32 flags =ULPI_OTG_DRVVBUS ;u8 *reg = on ? &ulpi->otg_ctrl_set : &ulpi->otg_ctrl_clear;if (ext_power) flags |=ULPI_OTG_DRVVBUS_EXT ; -if (ext_ind) - flags |=ULPI_OTG_EXTVBUSIND ;return ulpi_write (ulpi_vp, reg, flags); } +int ulpi_set_vbus_indicator (struct ulpi_viewport *ulpi_vp,int external, +int passthu,int complement) +{ +u8 *reg; +int ret; + + reg = external ? &ulpi->otg_ctrl_set : &ulpi->otg_ctrl_clear; +if ((ret =ulpi_write (ulpi_vp, reg,ULPI_OTG_EXTVBUSIND ))) +return ret; + + reg = passthu ? &ulpi->iface_ctrl_set : &ulpi->iface_ctrl_clear; +if ((ret =ulpi_write (ulpi_vp, reg,ULPI_IFACE_PASSTHRU ))) +return ret; + + reg = complement ? &ulpi->iface_ctrl_set : &ulpi->iface_ctrl_clear; +if ((ret =ulpi_write (ulpi_vp, reg,ULPI_IFACE_EXTVBUS_COMPLEMENT ))) +return ret; + +return 0 ; +} +int ulpi_set_pd (struct ulpi_viewport *ulpi_vp,int enable) {u32 val =ULPI_OTG_DP_PULLDOWN |ULPI_OTG_DM_PULLDOWN ;
include/usb/ulpi.h +11 -2
@@ -61,8 +61,17 @@ int ulpi_select_transceiver(struct ulpi_viewport *ulpi_vp, unsigned speed);* * returns 0 on success, ULPI_ERROR on failure. */ -int ulpi_set_vbus (struct ulpi_viewport *ulpi_vp, -int on,int ext_power,int ext_ind); +int ulpi_set_vbus (struct ulpi_viewport *ulpi_vp,int on,int ext_power); + +/* +* Configure VBUS indicator +* @external - external VBUS over-current indicator is used +* @passthru - disables ANDing of internal VBUS comparator +* with external VBUS input +* @complement - inverts the external VBUS input +*/ +int ulpi_set_vbus_indicator (struct ulpi_viewport *ulpi_vp,int external, +int passthru,int complement);/* * Enable/disable pull-down resistors on D+ and D- USB lines.
Signature · 1 line
1.7.11.4
PATCH v2 4/5 usb: ulpi: add indicator configuration function
Hi Lucas, Tom,
I'm sorry for the late reply.
I understand, that Tom has already applied this to tegra/next, but as the changes/follow up patches are required, may be we can do this in another fashion...
1) Thanks for the patch and working on extending the generic framework!
2) This patch has no dependencies on tegra specific patches, so I think, it should go through Marex usb tree, but doing this will require the right merge order, so bisectability will not suffer. So, Marek, Tom, you should decide which way is fine with you both.
Tom,
Yesterday, I was wondering if the patch was already applied, and I had no clue what's its status. Also, the patchwork says "New". So, if it is not hard for you in the future, I'd like a short reply to the list, saying something like: "Applied, thanks.", like most custodians do. Thanks!
Quoted 7 lines · Lucas Stach
Allows for easy configuration of the VBUS indicator related ULPI config bits.
Also move the external indicator setup from
ulpi_set_vbus()to the new function.Signed-off-by Lucas Stach <[email protected]>
After the below comments are fixed:
Quoted 35 lines
--- drivers/usb/ulpi/ulpi.c | 26 ++++++++++++++++++++++---- include/usb/ulpi.h | 13 +++++++++++--2 Dateien ge?ndert, 33 Zeilen hinzugef?gt(+), 6 Zeilen entfernt(-)
drivers/usb/ulpi/ulpi.c +8
-4@@ -106,20 +106,38 @@ int ulpi_select_transceiver(struct ulpi_viewport *ulpi_vp, unsigned speed)return ulpi_write (ulpi_vp, &ulpi->function_ctrl, val); } -int ulpi_set_vbus (struct ulpi_viewport *ulpi_vp,int on,int ext_power, -int ext_ind) +int ulpi_set_vbus (struct ulpi_viewport *ulpi_vp,int on,int ext_power) {u32 flags =ULPI_OTG_DRVVBUS ;u8 *reg = on ? &ulpi->otg_ctrl_set : &ulpi->otg_ctrl_clear;if (ext_power) flags |=ULPI_OTG_DRVVBUS_EXT ; -if (ext_ind) - flags |=ULPI_OTG_EXTVBUSIND ;return ulpi_write (ulpi_vp, reg, flags); } +int ulpi_set_vbus_indicator (struct ulpi_viewport *ulpi_vp,int external, +int passthu,int complement) +{ +u8 *reg; +int ret; + + reg = external ? &ulpi->otg_ctrl_set : &ulpi->otg_ctrl_clear;
drivers/usb/ulpi/ulpi.c int ulpi_select_transceiver(struct ulpi_viewport *ulpi_vp, unsigned speed) · line 136
Quoted 10 lines
drivers/usb/ulpi/ulpi.c +10
-0+if ((ret =ulpi_write (ulpi_vp, reg,ULPI_OTG_EXTVBUSIND ))) +return ret; + + reg = passthu ? &ulpi->iface_ctrl_set : &ulpi->iface_ctrl_clear; +if ((ret =ulpi_write (ulpi_vp, reg,ULPI_IFACE_PASSTHRU ))) +return ret; + + reg = complement ? &ulpi->iface_ctrl_set : &ulpi->iface_ctrl_clear; +if ((ret =ulpi_write (ulpi_vp, reg,ULPI_IFACE_EXTVBUS_COMPLEMENT ))) +return ret;
These are fine, two requests though:
1) As Tom already pointed in the private email:
ERROR: do not use assignment in if condition
#361: FILE: drivers/usb/ulpi/ulpi.c:127:
+ if ((ret = ulpi_write(ulpi_vp, reg, ULPI_OTG_EXTVBUSIND)))
ERROR: do not use assignment in if condition
#365: FILE: drivers/usb/ulpi/ulpi.c:131:
+ if ((ret = ulpi_write(ulpi_vp, reg, ULPI_IFACE_PASSTHRU)))
ERROR: do not use assignment in if condition
#369: FILE: drivers/usb/ulpi/ulpi.c:135:
+ if ((ret = ulpi_write(ulpi_vp, reg, ULPI_IFACE_EXTVBUS_COMPLEMENT)))
those must be fixed.
2) Can you make only one access for each register? Use flags/val variable (like in other places) and do only one access per register. Can you?
Quoted 31 lines
drivers/usb/ulpi/ulpi.c +4
-0+ +return 0 ; +} +int ulpi_set_pd (struct ulpi_viewport *ulpi_vp,int enable) {u32 val =ULPI_OTG_DP_PULLDOWN |ULPI_OTG_DM_PULLDOWN ;include/usb/ulpi.h +11
-2@@ -61,8 +61,17 @@ int ulpi_select_transceiver(struct ulpi_viewport *ulpi_vp, unsigned speed);* * returns 0 on success, ULPI_ERROR on failure. */ -int ulpi_set_vbus (struct ulpi_viewport *ulpi_vp, -int on,int ext_power,int ext_ind); +int ulpi_set_vbus (struct ulpi_viewport *ulpi_vp,int on,int ext_power); + +/* +* Configure VBUS indicator +* @external - external VBUS over-current indicator is used +* @passthru - disables ANDing of internal VBUS comparator +* with external VBUS input +* @complement - inverts the external VBUS input +*/ +int ulpi_set_vbus_indicator (struct ulpi_viewport *ulpi_vp,int external, +int passthru,int complement);/* * Enable/disable pull-down resistors on D+ and D- USB lines.
Signature · 2 lines
Regards, Igor.
PATCH v2 4/5 usb: ulpi: add indicator configuration function
Dear Igor Grinberg,
Quoted 7 lines
Hi Lucas, Tom,
I'm sorry for the late reply.
I understand, that Tom has already applied this to tegra/next, but as the changes/follow up patches are required, may be we can do this in another fashion...
Quoted 5 lines
1) Thanks for the patch and working on extending the generic framework!
2) This patch has no dependencies on tegra specific patches, so I think, it should go through Marex usb tree, but doing this will require the right merge order, so bisectability will not suffer. So, Marek, Tom, you should decide which way is fine with you both.
_ALWAYS_ CC the right custodians. That is, me. Seeing this patch bypassed me completely, I'm really unhappy.
Quoted 7 lines
Tom,
Yesterday, I was wondering if the patch was already applied, and I had no clue what's its status. Also, the patchwork says "New". So, if it is not hard for you in the future, I'd like a short reply to the list, saying something like: "Applied, thanks.", like most custodians do. Thanks!
+1
Quoted 81 lines · Lucas Stach
Quoted 7 lines
Allows for easy configuration of the VBUS indicator related ULPI config bits.
Also move the external indicator setup from
ulpi_set_vbus()to the new function.Signed-off-by Lucas Stach <[email protected]>After the below comments are fixed:
Acked-by Igor Grinberg <[email protected]>Quoted 44 lines
---
drivers/usb/ulpi/ulpi.c | 26 ++++++++++++++++++++++---- include/usb/ulpi.h | 13 +++++++++++--2 Dateien ge?ndert, 33 Zeilen hinzugef?gt(+), 6 Zeilen entfernt(-)
drivers/usb/ulpi/ulpi.c +8
-4@@ -106,20 +106,38 @@ int ulpi_select_transceiver(struct ulpi_viewport *ulpi_vp, unsigned speed)return ulpi_write (ulpi_vp, &ulpi->function_ctrl, val); } -int ulpi_set_vbus (struct ulpi_viewport *ulpi_vp,int on,int ext_power, -int ext_ind) +int ulpi_set_vbus (struct ulpi_viewport *ulpi_vp,int on,int ext_power) {u32 flags =ULPI_OTG_DRVVBUS ;u8 *reg = on ? &ulpi->otg_ctrl_set : &ulpi->otg_ctrl_clear;if (ext_power) flags |=ULPI_OTG_DRVVBUS_EXT ; -if (ext_ind) - flags |=ULPI_OTG_EXTVBUSIND ;return ulpi_write (ulpi_vp, reg, flags); } +int ulpi_set_vbus_indicator (struct ulpi_viewport *ulpi_vp,int external, +int passthu,int complement) +{ +u8 *reg; +int ret; + + reg = external ? &ulpi->otg_ctrl_set : &ulpi->otg_ctrl_clear;drivers/usb/ulpi/ulpi.c int ulpi_select_transceiver(struct ulpi_viewport
Quoted 10 lines
drivers/usb/ulpi/ulpi.c +10
-0+if ((ret =ulpi_write (ulpi_vp, reg,ULPI_OTG_EXTVBUSIND ))) +return ret; + + reg = passthu ? &ulpi->iface_ctrl_set : &ulpi->iface_ctrl_clear; +if ((ret =ulpi_write (ulpi_vp, reg,ULPI_IFACE_PASSTHRU ))) +return ret; + + reg = complement ? &ulpi->iface_ctrl_set : &ulpi->iface_ctrl_clear; +if ((ret =ulpi_write (ulpi_vp, reg,ULPI_IFACE_EXTVBUS_COMPLEMENT ))) +return ret;These are fine, two requests though:
1) As Tom already pointed in the private email:
ERROR: do not use assignment in if condition
#361: FILE:drivers/usb/ulpi/ulpi.c:127:drivers/usb/ulpi/ulpi.c +1
-0+if ((ret =ulpi_write (ulpi_vp, reg,ULPI_OTG_EXTVBUSIND )))ERROR: do not use assignment in if condition
#365: FILE:drivers/usb/ulpi/ulpi.c:131:drivers/usb/ulpi/ulpi.c +1
-0+if ((ret =ulpi_write (ulpi_vp, reg,ULPI_IFACE_PASSTHRU )))ERROR: do not use assignment in if condition
#369: FILE:drivers/usb/ulpi/ulpi.c:135:drivers/usb/ulpi/ulpi.c +1
-0+if ((ret =ulpi_write (ulpi_vp, reg,ULPI_IFACE_EXTVBUS_COMPLEMENT )))
Quoted 1 line
those must be fixed.
Agreed, _ALWAYS_ run checkpatch.pl before submitting. It's even better idea to add a git precommit hook for that.
Quoted 42 lines
2) Can you make only one access for each register? Use flags/val variable (like in other places) and do only one access per register. Can you?
Quoted 38 lines
drivers/usb/ulpi/ulpi.c +4
-0+ +return 0 ; +} +int ulpi_set_pd (struct ulpi_viewport *ulpi_vp,int enable) {u32 val =ULPI_OTG_DP_PULLDOWN |ULPI_OTG_DM_PULLDOWN ;include/usb/ulpi.h +11
-2@@ -61,8 +61,17 @@ int ulpi_select_transceiver(struct ulpi_viewport*ulpi_vp, unsigned speed); * * returns 0 on success, ULPI_ERROR on failure. */ -int ulpi_set_vbus (struct ulpi_viewport *ulpi_vp, -int on,int ext_power,int ext_ind); +int ulpi_set_vbus (struct ulpi_viewport *ulpi_vp,int on,int ext_power); + +/* +* Configure VBUS indicator +* @external - external VBUS over-current indicator is used +* @passthru - disables ANDing of internal VBUS comparator +* with external VBUS input +* @complement - inverts the external VBUS input +*/ +int ulpi_set_vbus_indicator (struct ulpi_viewport *ulpi_vp,int external, +int passthru,int complement);/*
* Enable/disable pull-down resistors on D+ and D- USB lines.
PATCH v2 4/5 usb: ulpi: add indicator configuration function
Igor/Marek,
Quoted 7 lines
-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: Wednesday, September 05, 2012 1:52 AM
To: Igor Grinberg
Cc: Lucas Stach; u-boot at lists.denx.de; Stephen Warren; Tom Warren
Subject: Re: [PATCH v2 4/5] usb: ulpi: add indicator configuration function
Quoted 14 lines
Dear Igor Grinberg,
Quoted 12 lines
Hi Lucas, Tom,
I'm sorry for the late reply.
I understand, that Tom has already applied this to tegra/next, but as the changes/follow up patches are required, may be we can do this in another fashion...1) Thanks for the patch and working on extending the generic framework!
2) This patch has no dependencies on tegra specific patches, so I think, it should go through Marex usb tree, but doing this will require the right merge order, so bisectability will not suffer. So, Marek, Tom, you should decide which way is fine with you both.
I'm not sure how the USB and Tegra repos can coordinate on patches like this, since I don't pull from/rebase against USB, and AFAIK Marek doesn't reference Tegra when he updates his repo. I'm a sub-repo of ARM, which is a sub-repo of TOT (u-boot/master). What I usually do (and have always done) is to take the entire patchset that includes a Tegra component (USB, mmc, SPI, etc.) and hope (pray?) that anyone merging my changes upstream of me will be able to resolve the conflicts/pre-existing patches. So far, I haven't heard from anyone (Albert or Wolfgang) that's had a problem with that, perhaps because it's pretty rare. AFAICT, there's no other procedure outlined in the U-Boot wiki custodian's page. If there's a better procedure I should be following, let's get it documented and I'll be glad to hew to the line. I'm still on the learning curve for git merging, rebasing, etc.
Quoted 5 lines
_ALWAYS_ CC the right custodians. That is, me. Seeing this patch bypassed me completely, I'm really unhappy.
Quoted 1 line
Quoted 9 lines
Quoted 6 lines
Tom,
Yesterday, I was wondering if the patch was already applied, and I had no clue what's its status. Also, the patchwork says "New". So, if it is not hard for you in the future, I'd like a short reply to the list, saying something like: "Applied, thanks.", like most custodians do. Thanks!+1
I _do_ owe the list, and the patch's author(s) an 'applied' message, as Igor points out. I've been viewing u-boot-tegra/next as just a staging area for patches that'll eventually go into master, and I've been copying patch authors and Tegra devs on pull requests, but I'll also send out a notice in the future when I apply a patch to /next.
Thanks,
Tom
Quoted 131 lines
Quoted 43 lines · Lucas Stach
Quoted 7 lines
Allows for easy configuration of the VBUS indicator related ULPI config bits.
Also move the external indicator setup from
ulpi_set_vbus()to the new function.Signed-off-by Lucas Stach <[email protected]>After the below comments are fixed:
Acked-by Igor Grinberg <[email protected]>Quoted 31 lines
---
drivers/usb/ulpi/ulpi.c | 26 ++++++++++++++++++++++---- include/usb/ulpi.h | 13 +++++++++++--2 Dateien ge?ndert, 33 Zeilen hinzugef?gt(+), 6 Zeilen entfernt(-)
drivers/usb/ulpi/ulpi.c index renamed from drivers/usb/ulpi/ulpi.c +0
-0dde2585..f358bde 100644
drivers/usb/ulpi/ulpi.c +2
-2@@ -106,20 +106,38 @@ int ulpi_select_transceiver(struct ulpi_viewport *ulpi_vp, unsigned speed)return ulpi_write (ulpi_vp, &ulpi->function_ctrl, val); } -int ulpi_set_vbus (struct ulpi_viewport *ulpi_vp,int on,int ext_power, -int ext_ind) +int ulpi_set_vbus (struct ulpi_viewport *ulpi_vp,int on,int +ext_power) {u32 flags =ULPI_OTG_DRVVBUS ;u8 *reg = on ? &ulpi->otg_ctrl_set : &ulpi->otg_ctrl_clear;if (ext_power) flags |=ULPI_OTG_DRVVBUS_EXT ;Quoted 8 lines
drivers/usb/ulpi/ulpi.c int ulpi_select_transceiver(struct ulpi_viewport *ulpi_vp, unsigned speed)
Quoted 8 lines
drivers/usb/ulpi/ulpi.c +1
-2-if (ext_ind) - flags |=ULPI_OTG_EXTVBUSIND ;return ulpi_write (ulpi_vp, reg, flags); } +int ulpi_set_vbus_indicator(struct ulpi_viewport *ulpi_vp, intexternal,
Quoted 31 lines
Quoted 6 lines
drivers/usb/ulpi/ulpi.c +6
-0+ int passthu, int complement) +{ +u8 *reg; +int ret; + + reg = external ? &ulpi->otg_ctrl_set : &ulpi->otg_ctrl_clear;drivers/usb/ulpi/ulpi.c int ulpi_select_transceiver(struct ulpi_viewport *ulpi_vp, unsigned speed) · line 136
Quoted 10 lines
drivers/usb/ulpi/ulpi.c +10
-0+if ((ret =ulpi_write (ulpi_vp, reg,ULPI_OTG_EXTVBUSIND ))) +return ret; + + reg = passthu ? &ulpi->iface_ctrl_set : &ulpi->iface_ctrl_clear; +if ((ret =ulpi_write (ulpi_vp, reg,ULPI_IFACE_PASSTHRU ))) +return ret; + + reg = complement ? &ulpi->iface_ctrl_set : &ulpi->iface_ctrl_clear; +if ((ret =ulpi_write (ulpi_vp, reg,ULPI_IFACE_EXTVBUS_COMPLEMENT ))) +return ret;These are fine, two requests though:
1) As Tom already pointed in the private email:
ERROR: do not use assignment in if condition
#361: FILE:drivers/usb/ulpi/ulpi.c:127:drivers/usb/ulpi/ulpi.c +1
-0+if ((ret =ulpi_write (ulpi_vp, reg,ULPI_OTG_EXTVBUSIND )))ERROR: do not use assignment in if condition
#365: FILE:drivers/usb/ulpi/ulpi.c:131:drivers/usb/ulpi/ulpi.c +1
-0+if ((ret =ulpi_write (ulpi_vp, reg,ULPI_IFACE_PASSTHRU )))ERROR: do not use assignment in if condition
#369: FILE:drivers/usb/ulpi/ulpi.c:135:drivers/usb/ulpi/ulpi.c +1
-0+if ((ret =ulpi_write (ulpi_vp, reg,ULPI_IFACE_EXTVBUS_COMPLEMENT )))Quoted 1 line
those must be fixed.
Agreed, _ALWAYS_ run checkpatch.pl before submitting. It's even better idea to add a git precommit hook for that.
Quoted 24 lines
2) Can you make only one access for each register? Use flags/val variable (like in other places) and do only one access per register. Can you?
Quoted 20 lines
drivers/usb/ulpi/ulpi.c +4
-0+ +return 0 ; +} +int ulpi_set_pd (struct ulpi_viewport *ulpi_vp,int enable) {u32 val =ULPI_OTG_DP_PULLDOWN |ULPI_OTG_DM_PULLDOWN ;include/usb/ulpi.h index renamed from include/usb/ulpi.h +0
-09a75c24..99166c4 100644
include/usb/ulpi.h +0
-0@@ -61,8 +61,17 @@ int ulpi_select_transceiver(struct ulpi_viewport*ulpi_vp, unsigned speed); * * returns 0 on success, ULPI_ERROR on failure. */ Quoted 13 lines
include/usb/ulpi.h int ulpi_select_transceiver(struct ulpi_viewport *ulpi_vp, unsigned speed);
Quoted 13 lines
include/usb/ulpi.h +11
-2-int ulpi_set_vbus (struct ulpi_viewport *ulpi_vp, -int on,int ext_power,int ext_ind); +int ulpi_set_vbus (struct ulpi_viewport *ulpi_vp,int on,int +ext_power); + +/* +* Configure VBUS indicator +* @external - external VBUS over-current indicator is used +* @passthru - disables ANDing of internal VBUS comparator +* with external VBUS input +* @complement - inverts the external VBUS input +*/ +int ulpi_set_vbus_indicator(struct ulpi_viewport *ulpi_vp, intexternal,
Quoted 5 lines
Quoted 5 lines
include/usb/ulpi.h +1
-0+ int passthru, int complement);/* * Enable/disable pull-down resistors on D+ and D- USB lines.
Signature · 1 line
nvpublic
PATCH v2 4/5 usb: ulpi: add indicator configuration function
Hi Tom,
Am Mittwoch, den 05.09.2012, 09:25 -0700 schrieb Tom Warren:
Quoted 18 lines
Igor/Marek,
Quoted 16 lines
-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: Wednesday, September 05, 2012 1:52 AM
To: Igor Grinberg
Cc: Lucas Stach; u-boot at lists.denx.de; Stephen Warren; Tom Warren
Subject: Re: [PATCH v2 4/5] usb: ulpi: add indicator configuration functionDear Igor Grinberg,
Quoted 7 lines
Hi Lucas, Tom,
I'm sorry for the late reply.
I understand, that Tom has already applied this to tegra/next, but as the changes/follow up patches are required, may be we can do this in another fashion...
Quoted 8 lines
Quoted 5 lines
Quoted 5 lines
1) Thanks for the patch and working on extending the generic framework!
2) This patch has no dependencies on tegra specific patches, so I think, it should go through Marex usb tree, but doing this will require the right merge order, so bisectability will not suffer. So, Marek, Tom, you should decide which way is fine with you both.I'm not sure how the USB and Tegra repos can coordinate on patches like this, since I don't pull from/rebase against USB, and AFAIK Marek doesn't reference Tegra when he updates his repo. I'm a sub-repo of ARM, which is a sub-repo of TOT (u-boot/master). What I usually do (and have always done) is to take the entire patchset that includes a Tegra component (USB, mmc, SPI, etc.) and hope (pray?) that anyone merging my changes upstream of me will be able to resolve the conflicts/pre-existing patches. So far, I haven't heard from anyone (Albert or Wolfgang) that's had a problem with that, perhaps because it's pretty rare. AFAICT, there's no other procedure outlined in the U-Boot wiki custodian's page. If there's a better procedure I should be following, let's get it documented and I'll be glad to hew to the line. I'm still on the learning curve for git merging, rebasing, etc.
I thought about how we could merge all this without loosing our sanity. I've already wrote this a bit hidden in a reply to the multi controller
thread: I think it's best to handle all USB related changes through the u-boot-usb tree, as all this stuff should really be under drivers/usb.
This means: I'll split out the clock output related changes, so they can go in the Tegra tree. Everything touching USB goes into the u-boot-usb tree and I'll rebase my changes accordingly. This also means commit "dm:
Tegra: Staticize local functions" should be removed from the Tegra tree and move over to the USB tree.
This way we won't get any build breakages and there should be no merge conflicts. It also opens the possibility to move the Tegra USB implementation to the right location in the source tree a bit later in this cycle, without messing up the merge.
Thanks,
Lucas
PATCH v2 4/5 usb: ulpi: add indicator configuration function
Dear Lucas Stach,
Quoted 44 lines
Hi Tom,
Am Mittwoch, den 05.09.2012, 09:25 -0700 schrieb Tom Warren:
Quoted 27 lines
Igor/Marek,
Quoted 8 lines
-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: Wednesday, September 05, 2012 1:52 AM
To: Igor Grinberg
Cc: Lucas Stach; u-boot at lists.denx.de; Stephen Warren; Tom Warren
Subject: Re: [PATCH v2 4/5] usb: ulpi: add indicator configuration functionQuoted 16 lines
Dear Igor Grinberg,
Quoted 14 lines
Hi Lucas, Tom,
I'm sorry for the late reply.
I understand, that Tom has already applied this to tegra/next, but as the changes/follow up patches are required, may be we can do this in another fashion...1) Thanks for the patch and working on extending the generic framework! 2) This patch has no dependencies on tegra specific patches, so
I think, it should go through Marex usb tree, but doing this will require the right merge order, so bisectability will not suffer. So, Marek, Tom, you should decide which way is fine with you both.
Quoted 13 lines
I'm not sure how the USB and Tegra repos can coordinate on patches like this, since I don't pull from/rebase against USB, and AFAIK Marek doesn't reference Tegra when he updates his repo. I'm a sub-repo of ARM, which is a sub-repo of TOT (u-boot/master). What I usually do (and have always done) is to take the entire patchset that includes a Tegra component (USB, mmc, SPI, etc.) and hope (pray?) that anyone merging my changes upstream of me will be able to resolve the conflicts/pre-existing patches. So far, I haven't heard from anyone (Albert or Wolfgang) that's had a problem with that, perhaps because it's pretty rare. AFAICT, there's no other procedure outlined in the U-Boot wiki custodian's page. If there's a better procedure I should be following, let's get it documented and I'll be glad to hew to the line. I'm still on the learning curve for git merging, rebasing, etc.
Quoted 4 lines
I thought about how we could merge all this without loosing our sanity. I've already wrote this a bit hidden in a reply to the multi controller
thread: I think it's best to handle all USB related changes through the u-boot-usb tree, as all this stuff should really be underdrivers/usb.
Let's extend this a bit. Since I'm really under heavy load now, why don't you prepare a patchset (that includes all the tegra goo, stuff that Tom will drop etc) that I can pick, submit it (and Cc me) and I'll just pick it all ? That way we'll have a proper ordering and nothing will be lost and all will be tested.
Quoted 5 lines
This means: I'll split out the clock output related changes, so they can go in the Tegra tree. Everything touching USB goes into the u-boot-usb tree and I'll rebase my changes accordingly. This also means commit "dm:
Tegra: Staticize local functions" should be removed from the Tegra tree and move over to the USB tree.
Drop the dm: from it along the way.
Quoted 7 lines
This way we won't get any build breakages and there should be no merge conflicts. It also opens the possibility to move the Tegra USB implementation to the right location in the source tree a bit later in this cycle, without messing up the merge.
Thanks,
Lucas
Best regards,
Marek Vasut
PATCH v2 5/5 tegra20: add USB ULPI init code
In reply to Lucas StachThis 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()
--- 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(-)
arch/arm/cpu/armv7/tegra20/usb.c +131 -23
@@ -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 24000000 +#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 */ +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 =100000 ; loop_count !=0 ; loop_count--) { +if (readl (&usbctlr->susp_ctrl) &USB_PHY_CLK_VALID ) +break ; +udelay (1 ); + } +if (!loop_count) +return -1 ; +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 }static void config_clock (const u32 timing[])
@@ -327,24 +430,25 @@ static int add_port(struct fdt_usb *config, const u32 timing[])struct usb_ctlr *usbctlr = config->reg;if (port_count ==USB_PORTS_MAX ) { -debug ("tegrausb: Cannot register more than %d ports\n" , +printf ("tegrausb: Cannot register more than %d ports\n" ,USB_PORTS_MAX );return -1 ; } -if (init_usb_controller (config, usbctlr, timing)) { -debug ("tegrausb: Cannot init port\n" ); -return -1 ; - } +if (config->utmi) { -/* 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 ); -power_up_port (usbctlr); +if (init_utmi_usb_controller (config, usbctlr, timing)) { +printf ("tegrausb: Cannot init port\n" ); +return -1 ; + } + } + +if (config->ulpi) { +if (init_ulpi_usb_controller (config, usbctlr)) { +printf ("tegrausb: Cannot init port\n" ); +return -1 ; + } } + port[port_count++] = *config;return 0 ;
@@ -411,6 +515,7 @@ static int fdt_decode_usb(const void *blob, int node, phy =fdt_getprop (blob, node,"phy_type" ,NULL ); config->utmi = phy &&0 ==strcmp ("utmi" , phy); + config->ulpi = phy &&0 ==strcmp ("ulpi" , phy); config->enabled =fdtdec_get_is_enabled (blob, node); config->has_legacy_mode =fdtdec_get_bool (blob, node,"nvidia,has-legacy-mode" );
@@ -420,10 +525,13 @@ static int fdt_decode_usb(const void *blob, int node,return -FDT_ERR_NOTFOUND ; }fdtdec_decode_gpio (blob, node,"nvidia,vbus-gpio" , &config->vbus_gpio); -debug ("enabled=%d, legacy_mode=%d, utmi=%d, periph_id=%d, vbus=%d, " -"dr_mode=%d\n" , config->enabled, config->has_legacy_mode, - config->utmi, config->periph_id, config->vbus_gpio.gpio, - config->dr_mode); +fdtdec_decode_gpio (blob, node,"nvidia,phy-reset-gpio" , + &config->phy_reset_gpio); +debug ("enabled=%d, legacy_mode=%d, utmi=%d, ulpi=%d, periph_id=%d, " +"vbus=%d, phy_reset=%d, dr_mode=%d\n" , + config->enabled, config->has_legacy_mode, config->utmi, config->ulpi, + config->periph_id, config->vbus_gpio.gpio, + config->phy_reset_gpio.gpio, config->dr_mode);return 0 ; }
arch/arm/include/asm/arch-tegra20/usb.h +24 -5
@@ -100,10 +100,12 @@ struct usb_ctlr {/* 0x410 */ uint usb1_legacy_ctrl; -uint reserved12[3 ]; +uint reserved12[4 ]; -/* 0x420 */ -uint reserved13[56 ]; +/* 0x424 */ +uint ulpi_timing_ctrl_0; +uint ulpi_timing_ctrl_1; +uint reserved13[53 ];/* 0x500 */ uint reserved14[64 *3 ];
@@ -144,10 +146,24 @@ struct usb_ctlr {#define VBUS_SENSE_CTL_AB_SESS_VLD 2#define VBUS_SENSE_CTL_A_SESS_VLD 3 +/* USB2_IF_ULPI_TIMING_CTRL_0 */ +#define ULPI_OUTPUT_PINMUX_BYP (1 << 10) +#define ULPI_CLKOUT_PINMUX_BYP (1 << 11) + +/* USB2_IF_ULPI_TIMING_CTRL_1 */ +#define ULPI_DATA_TRIMMER_LOAD (1 << 0) +#define ULPI_DATA_TRIMMER_SEL (x) (((x) & 0x7) << 1) +#define ULPI_STPDIRNXT_TRIMMER_LOAD (1 << 16) +#define ULPI_STPDIRNXT_TRIMMER_SEL (x) (((x) & 0x7) << 17) +#define ULPI_DIR_TRIMMER_LOAD (1 << 24) +#define ULPI_DIR_TRIMMER_SEL (x) (((x) & 0x7) << 25) +/* USBx_IF_USB_SUSP_CTRL_0 */ +#define ULPI_PHY_ENB (1 << 13)#define UTMIP_PHY_ENB (1 << 12)#define UTMIP_RESET (1 << 11)#define USB_PHY_CLK_VALID (1 << 7) +#define USB_SUSP_CLR (1 << 5)/* USBx_UTMIP_MISC_CFG1 */ #define UTMIP_PLLU_STABLE_COUNT_SHIFT 6
@@ -203,12 +219,15 @@ struct usb_ctlr {/* SB2_CONTROLLER_2_USB2D_PORTSC1_0 */ #define PTS_SHIFT 30#define PTS_MASK (3U << PTS_SHIFT) -#define PTS_UTMI 0 +#define PTS_UTMI 0#define PTS_RESERVED 1 -#define PTS_ULP 2 +#define PTS_ULPI 2#define PTS_ICUSB_SER 3#define STS (1 << 29) +#define WKOC (1 << 22) +#define WKDS (1 << 21) +#define WKCN (1 << 20)/* USBx_UTMIP_XCVR_CFG0_0 */ #define UTMIP_FORCE_PD_POWERDOWN (1 << 14)
Signature · 1 line
1.7.11.4
PATCH v2 5/5 tegra20: add USB ULPI init code
Quoted 26 lines · 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_ULPIandCONFIG_USB_ULPI_VIEWPORThave 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 defineULPI_REF_CLKto 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 underdebug()Signed-off-by Lucas Stach <[email protected]>--- 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(-)
arch/arm/cpu/armv7/tegra20/usb.c +0
-0
[...]
arch/arm/cpu/armv7/tegra20/usb.c static int init_usb_controller(struct fdt_usb *config, · line 335
Quoted 9 lines
arch/arm/cpu/armv7/tegra20/usb.c +9
-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 24000000 +#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 24000000
#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.
Quoted 57 lines
arch/arm/cpu/armv7/tegra20/usb.c +57
-0+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 ); +
arch/arm/cpu/armv7/tegra20/usb.c static int init_usb_controller(struct fdt_usb *config, · line 402
Quoted 10 lines
arch/arm/cpu/armv7/tegra20/usb.c +10
-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 =100000 ; loop_count !=0 ; loop_count--) { +if (readl (&usbctlr->susp_ctrl) &USB_PHY_CLK_VALID ) +break ; +udelay (1 ); + }
an empty line here would be nice.
arch/arm/cpu/armv7/tegra20/usb.c
Quoted 2 lines
arch/arm/cpu/armv7/tegra20/usb.c +2
-0+if (!loop_count) +return -1 ;
and here.
Quoted 36 lines
arch/arm/cpu/armv7/tegra20/usb.c +12
-13+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 }static void config_clock (const u32 timing[])@@ -327,24 +430,25 @@ static int add_port(struct fdt_usb *config, const u32 timing[])struct usb_ctlr *usbctlr = config->reg;if (port_count ==USB_PORTS_MAX ) { -debug ("tegrausb: Cannot register more than %d ports\n" , +printf ("tegrausb: Cannot register more than %d ports\n" ,USB_PORTS_MAX );return -1 ; } -if (init_usb_controller (config, usbctlr, timing)) { -debug ("tegrausb: Cannot init port\n" ); -return -1 ; - } +if (config->utmi) { -/* 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 ); -power_up_port (usbctlr); +if (init_utmi_usb_controller (config, usbctlr, timing)) { +printf ("tegrausb: Cannot init port\n" );
arch/arm/cpu/armv7/tegra20/usb.c static int add_port(struct fdt_usb *config, const u32 timing[]) · line 450
Quoted 10 lines
arch/arm/cpu/armv7/tegra20/usb.c +9
-0+return -1 ; + } + } + +if (config->ulpi) { +if (init_ulpi_usb_controller (config, usbctlr)) { +printf ("tegrausb: Cannot init port\n" ); +return -1 ; + } }
here you can use:
if (config->ulpi && init_ulpi_usb_controller(config, usbctlr)) {
printf("tegrausb: Cannot init port\n");
return -1;
}and spare one indentation level, but I don't really insist...
Quoted 4 lines
arch/arm/cpu/armv7/tegra20/usb.c +1
-0+ port[port_count++] = *config;return 0 ;
[...]
Signature · 2 lines
Regards, Igor.
v2 cover Tegra 2 USB ULPI series
In reply to Lucas StachQuoted 11 lines · Lucas Stach
With this series we are able to initialize USB controllers using an external ULPI phy AKA USB2 on Tegra 2 devices.
This was tested to work on a Toradex Colibri T20 board, where USB2 is used to access the ASIX ethernet chipset. Testing was done with "tegra20: usb: rework
set_host_mode" [1] applied. I did not spot any regressions on the UTMI ports.v2 incorporates all the review feedback I've got so far, including now trying harder to enable VBus in all configurations.
Quoted 7 lines
Patch 3 is already in u-boot-usb and only provided as the ULPI init code now depends on it.
Igor could you please take a look at Patch 4?Patchset is based on top of u-boot-tegra/next
[1] http://lists.denx.de/pipermail/u-boot/2012-August/130177.html
OK, with the addition of an appropriate pin_mux_usb() function to harmony.c, this works now. So, please consider the series,
Note that I did see the following:
Tegra20 (Harmony) # usb start
(Re)start USB...
USB: Register 10011 NbrPorts 1
USB EHCI 1.00
scanning bus for devices... Register 10011 NbrPorts 1
USB EHCI 1.00
scanning bus for devices... 5 USB Device(s) found
scanning bus for storage devices... Device NOT ready
Request Sense returned 02 3A 00
2 Storage Device(s) found
scanning bus for ethernet devices... 1 Ethernet Device(s) found
However, both "usb tree" and then "ext2ls" on the USB device succeeded without issue, so I think that's tested well enough. I even booted a kernel from the USB device, and the kernel was able to use the USB device for the root filesystem, so U-Boot hadn't broken it in any way:-)