* [PATCH 1/6] usb: host: add has_tdi_phy_lpm capability bit
2013-07-31 17:41 [PATCH 0/6] USB tree changes for Tegra30 and Tegra114 USB Host support Tuomas Tynkkynen
@ 2013-07-31 17:41 ` Tuomas Tynkkynen
2013-07-31 18:35 ` Alan Stern
2013-08-01 8:05 ` Matthieu CASTET
2013-07-31 17:41 ` [PATCH 2/6] usb: phy: tegra: Fix wrong PHY parameters Tuomas Tynkkynen
` (5 subsequent siblings)
6 siblings, 2 replies; 20+ messages in thread
From: Tuomas Tynkkynen @ 2013-07-31 17:41 UTC (permalink / raw)
To: balbi
Cc: linux-tegra, linux-kernel, linux-usb, swarren, gregkh, stern,
Tuomas Tynkkynen, Matthieu CASTET, Alexander Shishkin
The has_hostpc capability bit indicates that the host controller has the
HOSTPC register extensions, but at the same time enables clock disabling
power saving features with the PHY Low Power Clock Disable (PHCD) bit.
However, some host controllers have the HOSTPC extensions but don't
support the low-power feature, so the PHCD bit must not be set on those
controllers. Add a separate capability bit for the low-power feature
instead, and change all existing users of has_hostpc to use this new
capability bit.
The idea for this commit is taken from an old 2012 commit that never got
merged ("disociate chipidea PHY low power suspend control from hostpc")
Inspired-by: Matthieu CASTET <matthieu.castet@parrot.com>
Signed-off-by: Tuomas Tynkkynen <ttynkkynen@nvidia.com>
---
drivers/usb/chipidea/host.c | 1 +
drivers/usb/host/ehci-hub.c | 14 +++++++-------
drivers/usb/host/ehci.h | 1 +
3 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
index 40d0fda..9b3aaf1 100644
--- a/drivers/usb/chipidea/host.c
+++ b/drivers/usb/chipidea/host.c
@@ -63,6 +63,7 @@ static int host_start(struct ci_hdrc *ci)
ehci = hcd_to_ehci(hcd);
ehci->caps = ci->hw_bank.cap;
ehci->has_hostpc = ci->hw_bank.lpm;
+ ehci->has_tdi_phy_lpm = ci->hw_bank.lpm;
ret = usb_add_hcd(hcd, 0, 0);
if (ret)
diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
index 2b70277..8044a74 100644
--- a/drivers/usb/host/ehci-hub.c
+++ b/drivers/usb/host/ehci-hub.c
@@ -183,7 +183,7 @@ static void ehci_adjust_port_wakeup_flags(struct ehci_hcd *ehci,
spin_lock_irq(&ehci->lock);
/* clear phy low-power mode before changing wakeup flags */
- if (ehci->has_hostpc) {
+ if (ehci->has_tdi_phy_lpm) {
port = HCS_N_PORTS(ehci->hcs_params);
while (port--) {
u32 __iomem *hostpc_reg = &ehci->regs->hostpc[port];
@@ -217,7 +217,7 @@ static void ehci_adjust_port_wakeup_flags(struct ehci_hcd *ehci,
}
/* enter phy low-power mode again */
- if (ehci->has_hostpc) {
+ if (ehci->has_tdi_phy_lpm) {
port = HCS_N_PORTS(ehci->hcs_params);
while (port--) {
u32 __iomem *hostpc_reg = &ehci->regs->hostpc[port];
@@ -309,7 +309,7 @@ static int ehci_bus_suspend (struct usb_hcd *hcd)
}
}
- if (changed && ehci->has_hostpc) {
+ if (changed && ehci->has_tdi_phy_lpm) {
spin_unlock_irq(&ehci->lock);
msleep(5); /* 5 ms for HCD to enter low-power mode */
spin_lock_irq(&ehci->lock);
@@ -435,7 +435,7 @@ static int ehci_bus_resume (struct usb_hcd *hcd)
goto shutdown;
/* clear phy low-power mode before resume */
- if (ehci->bus_suspended && ehci->has_hostpc) {
+ if (ehci->bus_suspended && ehci->has_tdi_phy_lpm) {
i = HCS_N_PORTS(ehci->hcs_params);
while (i--) {
if (test_bit(i, &ehci->bus_suspended)) {
@@ -788,7 +788,7 @@ static int ehci_hub_control (
goto error;
/* clear phy low-power mode before resume */
- if (ehci->has_hostpc) {
+ if (ehci->has_tdi_phy_lpm) {
temp1 = ehci_readl(ehci, hostpc_reg);
ehci_writel(ehci, temp1 & ~HOSTPC_PHCD,
hostpc_reg);
@@ -1031,12 +1031,12 @@ static int ehci_hub_control (
/* After above check the port must be connected.
* Set appropriate bit thus could put phy into low power
- * mode if we have hostpc feature
+ * mode if we have tdi_phy_lpm feature
*/
temp &= ~PORT_WKCONN_E;
temp |= PORT_WKDISC_E | PORT_WKOC_E;
ehci_writel(ehci, temp | PORT_SUSPEND, status_reg);
- if (ehci->has_hostpc) {
+ if (ehci->has_tdi_phy_lpm) {
spin_unlock_irqrestore(&ehci->lock, flags);
msleep(5);/* 5ms for HCD enter low pwr mode */
spin_lock_irqsave(&ehci->lock, flags);
diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index 64f9a08..d034d94 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -210,6 +210,7 @@ struct ehci_hcd { /* one per controller */
#define OHCI_HCCTRL_LEN 0x4
__hc32 *ohci_hcctrl_reg;
unsigned has_hostpc:1;
+ unsigned has_tdi_phy_lpm:1;
unsigned has_ppcd:1; /* support per-port change bits */
u8 sbrn; /* packed release number */
--
1.8.1.5
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 1/6] usb: host: add has_tdi_phy_lpm capability bit
2013-07-31 17:41 ` [PATCH 1/6] usb: host: add has_tdi_phy_lpm capability bit Tuomas Tynkkynen
@ 2013-07-31 18:35 ` Alan Stern
2013-08-01 8:05 ` Matthieu CASTET
1 sibling, 0 replies; 20+ messages in thread
From: Alan Stern @ 2013-07-31 18:35 UTC (permalink / raw)
To: Tuomas Tynkkynen
Cc: balbi, linux-tegra, linux-kernel, linux-usb, swarren, gregkh,
Matthieu CASTET, Alexander Shishkin
On Wed, 31 Jul 2013, Tuomas Tynkkynen wrote:
> The has_hostpc capability bit indicates that the host controller has the
> HOSTPC register extensions, but at the same time enables clock disabling
> power saving features with the PHY Low Power Clock Disable (PHCD) bit.
>
> However, some host controllers have the HOSTPC extensions but don't
> support the low-power feature, so the PHCD bit must not be set on those
> controllers. Add a separate capability bit for the low-power feature
> instead, and change all existing users of has_hostpc to use this new
> capability bit.
>
> The idea for this commit is taken from an old 2012 commit that never got
> merged ("disociate chipidea PHY low power suspend control from hostpc")
>
> Inspired-by: Matthieu CASTET <matthieu.castet@parrot.com>
> Signed-off-by: Tuomas Tynkkynen <ttynkkynen@nvidia.com>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 1/6] usb: host: add has_tdi_phy_lpm capability bit
2013-07-31 17:41 ` [PATCH 1/6] usb: host: add has_tdi_phy_lpm capability bit Tuomas Tynkkynen
2013-07-31 18:35 ` Alan Stern
@ 2013-08-01 8:05 ` Matthieu CASTET
2013-08-02 15:42 ` Tuomas Tynkkynen
1 sibling, 1 reply; 20+ messages in thread
From: Matthieu CASTET @ 2013-08-01 8:05 UTC (permalink / raw)
To: Tuomas Tynkkynen
Cc: balbi@ti.com, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
swarren@wwwdotorg.org, gregkh@linuxfoundation.org,
stern@rowland.harvard.edu, Alexander Shishkin
Hi,
Le Wed, 31 Jul 2013 18:41:57 +0100,
Tuomas Tynkkynen <ttynkkynen@nvidia.com> a écrit :
> The has_hostpc capability bit indicates that the host controller has
> the HOSTPC register extensions, but at the same time enables clock
> disabling power saving features with the PHY Low Power Clock Disable
> (PHCD) bit.
>
> However, some host controllers have the HOSTPC extensions but don't
> support the low-power feature, so the PHCD bit must not be set on
> those controllers. Add a separate capability bit for the low-power
> feature instead, and change all existing users of has_hostpc to use
> this new capability bit.
>
> The idea for this commit is taken from an old 2012 commit that never
> got merged ("disociate chipidea PHY low power suspend control from
> hostpc")
Note that because of the different register layout (see "add phy low
power suspend for older chipidea core" commit in the same series), we
should not set has_tdi_phy_lpm if has_hostpc == 0 with the current code.
May be you should have change the ehci->has_hostpc to (ehci->has_hostpc
&& ehci->has_tdi_phy_lpm).
BTW Alan make some comment on the commit :
http://marc.info/?l=linux-usb&m=133701342028213&w=2
They may apply to your commit.
>
> Inspired-by: Matthieu CASTET <matthieu.castet@parrot.com>
> Signed-off-by: Tuomas Tynkkynen <ttynkkynen@nvidia.com>
> ---
> drivers/usb/chipidea/host.c | 1 +
> drivers/usb/host/ehci-hub.c | 14 +++++++-------
> drivers/usb/host/ehci.h | 1 +
> 3 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
> index 40d0fda..9b3aaf1 100644
> --- a/drivers/usb/chipidea/host.c
> +++ b/drivers/usb/chipidea/host.c
> @@ -63,6 +63,7 @@ static int host_start(struct ci_hdrc *ci)
> ehci = hcd_to_ehci(hcd);
> ehci->caps = ci->hw_bank.cap;
> ehci->has_hostpc = ci->hw_bank.lpm;
> + ehci->has_tdi_phy_lpm = ci->hw_bank.lpm;
>
> ret = usb_add_hcd(hcd, 0, 0);
> if (ret)
> diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
> index 2b70277..8044a74 100644
> --- a/drivers/usb/host/ehci-hub.c
> +++ b/drivers/usb/host/ehci-hub.c
> @@ -183,7 +183,7 @@ static void ehci_adjust_port_wakeup_flags(struct
> ehci_hcd *ehci, spin_lock_irq(&ehci->lock);
>
> /* clear phy low-power mode before changing wakeup flags */
> - if (ehci->has_hostpc) {
> + if (ehci->has_tdi_phy_lpm) {
> port = HCS_N_PORTS(ehci->hcs_params);
> while (port--) {
> u32 __iomem *hostpc_reg =
> &ehci->regs->hostpc[port]; @@ -217,7 +217,7 @@ static void
> ehci_adjust_port_wakeup_flags(struct ehci_hcd *ehci, }
>
> /* enter phy low-power mode again */
> - if (ehci->has_hostpc) {
> + if (ehci->has_tdi_phy_lpm) {
> port = HCS_N_PORTS(ehci->hcs_params);
> while (port--) {
> u32 __iomem *hostpc_reg =
> &ehci->regs->hostpc[port]; @@ -309,7 +309,7 @@ static int
> ehci_bus_suspend (struct usb_hcd *hcd) }
> }
>
> - if (changed && ehci->has_hostpc) {
> + if (changed && ehci->has_tdi_phy_lpm) {
> spin_unlock_irq(&ehci->lock);
> msleep(5); /* 5 ms for HCD to enter low-power
> mode */ spin_lock_irq(&ehci->lock);
> @@ -435,7 +435,7 @@ static int ehci_bus_resume (struct usb_hcd *hcd)
> goto shutdown;
>
> /* clear phy low-power mode before resume */
> - if (ehci->bus_suspended && ehci->has_hostpc) {
> + if (ehci->bus_suspended && ehci->has_tdi_phy_lpm) {
> i = HCS_N_PORTS(ehci->hcs_params);
> while (i--) {
> if (test_bit(i, &ehci->bus_suspended)) {
> @@ -788,7 +788,7 @@ static int ehci_hub_control (
> goto error;
>
> /* clear phy low-power mode before resume */
> - if (ehci->has_hostpc) {
> + if (ehci->has_tdi_phy_lpm) {
> temp1 = ehci_readl(ehci, hostpc_reg);
> ehci_writel(ehci, temp1 &
> ~HOSTPC_PHCD, hostpc_reg);
> @@ -1031,12 +1031,12 @@ static int ehci_hub_control (
>
> /* After above check the port must be
> connected.
> * Set appropriate bit thus could put phy
> into low power
> - * mode if we have hostpc feature
> + * mode if we have tdi_phy_lpm feature
> */
> temp &= ~PORT_WKCONN_E;
> temp |= PORT_WKDISC_E | PORT_WKOC_E;
> ehci_writel(ehci, temp | PORT_SUSPEND,
> status_reg);
> - if (ehci->has_hostpc) {
> + if (ehci->has_tdi_phy_lpm) {
> spin_unlock_irqrestore(&ehci->lock,
> flags); msleep(5);/* 5ms for HCD enter low pwr mode */
> spin_lock_irqsave(&ehci->lock,
> flags); diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
> index 64f9a08..d034d94 100644
> --- a/drivers/usb/host/ehci.h
> +++ b/drivers/usb/host/ehci.h
> @@ -210,6 +210,7 @@ struct ehci_hcd { /* one
> per controller */ #define OHCI_HCCTRL_LEN 0x4
> __hc32 *ohci_hcctrl_reg;
> unsigned has_hostpc:1;
> + unsigned has_tdi_phy_lpm:1;
> unsigned has_ppcd:1; /* support per-port
> change bits */ u8 sbrn; /*
> packed release number */
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 1/6] usb: host: add has_tdi_phy_lpm capability bit
2013-08-01 8:05 ` Matthieu CASTET
@ 2013-08-02 15:42 ` Tuomas Tynkkynen
0 siblings, 0 replies; 20+ messages in thread
From: Tuomas Tynkkynen @ 2013-08-02 15:42 UTC (permalink / raw)
To: Matthieu CASTET
Cc: balbi@ti.com, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
swarren@wwwdotorg.org, gregkh@linuxfoundation.org,
stern@rowland.harvard.edu, Alexander Shishkin
On 08/01/2013 11:05 AM, Matthieu CASTET wrote:
> Hi,
>
> Le Wed, 31 Jul 2013 18:41:57 +0100,
> Tuomas Tynkkynen <ttynkkynen@nvidia.com> a écrit :
>
>> The has_hostpc capability bit indicates that the host controller has
>> the HOSTPC register extensions, but at the same time enables clock
>> disabling power saving features with the PHY Low Power Clock Disable
>> (PHCD) bit.
>>
>> However, some host controllers have the HOSTPC extensions but don't
>> support the low-power feature, so the PHCD bit must not be set on
>> those controllers. Add a separate capability bit for the low-power
>> feature instead, and change all existing users of has_hostpc to use
>> this new capability bit.
>>
>> The idea for this commit is taken from an old 2012 commit that never
>> got merged ("disociate chipidea PHY low power suspend control from
>> hostpc")
> Note that because of the different register layout (see "add phy low
> power suspend for older chipidea core" commit in the same series), we
> should not set has_tdi_phy_lpm if has_hostpc == 0 with the current code.
>
> May be you should have change the ehci->has_hostpc to (ehci->has_hostpc
> && ehci->has_tdi_phy_lpm).
Hmm, I see. Do you think there could be a case where that could get accidentally
get triggered via autodetection? That patch series seems to set either both or neither.
And I figure no one will be explicitly setting that flag (if has_hostpc == 0) without
implementing the non-has_hostpc case first.
>
> BTW Alan make some comment on the commit :
> http://marc.info/?l=linux-usb&m=133701342028213&w=2
>
> They may apply to your commit.
>
>>
>> Inspired-by: Matthieu CASTET <matthieu.castet@parrot.com>
>> Signed-off-by: Tuomas Tynkkynen <ttynkkynen@nvidia.com>
>> ---
>> drivers/usb/chipidea/host.c | 1 +
>> drivers/usb/host/ehci-hub.c | 14 +++++++-------
>> drivers/usb/host/ehci.h | 1 +
>> 3 files changed, 9 insertions(+), 7 deletions(-)
>>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/6] usb: phy: tegra: Fix wrong PHY parameters
2013-07-31 17:41 [PATCH 0/6] USB tree changes for Tegra30 and Tegra114 USB Host support Tuomas Tynkkynen
2013-07-31 17:41 ` [PATCH 1/6] usb: host: add has_tdi_phy_lpm capability bit Tuomas Tynkkynen
@ 2013-07-31 17:41 ` Tuomas Tynkkynen
2013-08-01 21:09 ` Stephen Warren
2013-07-31 17:41 ` [PATCH 3/6] usb: phy: tegra: Tegra30 support Tuomas Tynkkynen
` (4 subsequent siblings)
6 siblings, 1 reply; 20+ messages in thread
From: Tuomas Tynkkynen @ 2013-07-31 17:41 UTC (permalink / raw)
To: balbi
Cc: linux-tegra, linux-kernel, linux-usb, swarren, gregkh, stern,
Tuomas Tynkkynen
Some of the PHY parameters are not set according to the TRMs:
- UTMIP_FS_PREABMLE_J should be set, not cleared
- UTMIP_XCVR_LSBIAS_SEL should be cleared, not set
- UTMIP_PD_CHRG should be set in host mode and cleared in device mode
- UTMIP_XCVR_SETUP is a two-part field; the upper bits were not set
properly
Signed-off-by: Tuomas Tynkkynen <ttynkkynen@nvidia.com>
---
drivers/usb/phy/phy-tegra-usb.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
index 01c30ff..936b4a2 100644
--- a/drivers/usb/phy/phy-tegra-usb.c
+++ b/drivers/usb/phy/phy-tegra-usb.c
@@ -86,11 +86,13 @@
#define UTMIP_XCVR_CFG0 0x808
#define UTMIP_XCVR_SETUP(x) (((x) & 0xf) << 0)
+#define UTMIP_XCVR_SETUP_MSB(x) ((((x) & 0x7f) >> 4) << 22)
#define UTMIP_XCVR_LSRSLEW(x) (((x) & 0x3) << 8)
#define UTMIP_XCVR_LSFSLEW(x) (((x) & 0x3) << 10)
#define UTMIP_FORCE_PD_POWERDOWN (1 << 14)
#define UTMIP_FORCE_PD2_POWERDOWN (1 << 16)
#define UTMIP_FORCE_PDZI_POWERDOWN (1 << 18)
+#define UTMIP_XCVR_LSBIAS_SEL (1 << 21)
#define UTMIP_XCVR_HSSLEW_MSB(x) (((x) & 0x7f) << 25)
#define UTMIP_BIAS_CFG0 0x80c
@@ -342,7 +344,7 @@ static int utmi_phy_power_on(struct tegra_usb_phy *phy)
}
val = readl(base + UTMIP_TX_CFG0);
- val &= ~UTMIP_FS_PREABMLE_J;
+ val |= UTMIP_FS_PREABMLE_J;
writel(val, base + UTMIP_TX_CFG0);
val = readl(base + UTMIP_HSRX_CFG0);
@@ -381,16 +383,26 @@ static int utmi_phy_power_on(struct tegra_usb_phy *phy)
val = readl(base + USB_SUSP_CTRL);
val &= ~(USB_WAKE_ON_CNNT_EN_DEV | USB_WAKE_ON_DISCON_EN_DEV);
writel(val, base + USB_SUSP_CTRL);
+
+ val = readl(base + UTMIP_BAT_CHRG_CFG0);
+ val &= ~UTMIP_PD_CHRG;
+ writel(val, base + UTMIP_BAT_CHRG_CFG0);
+ } else {
+ val = readl(base + UTMIP_BAT_CHRG_CFG0);
+ val |= UTMIP_PD_CHRG;
+ writel(val, base + UTMIP_BAT_CHRG_CFG0);
}
utmip_pad_power_on(phy);
val = readl(base + UTMIP_XCVR_CFG0);
val &= ~(UTMIP_FORCE_PD_POWERDOWN | UTMIP_FORCE_PD2_POWERDOWN |
- UTMIP_FORCE_PDZI_POWERDOWN | UTMIP_XCVR_SETUP(~0) |
+ UTMIP_FORCE_PDZI_POWERDOWN | UTMIP_XCVR_LSBIAS_SEL |
+ UTMIP_XCVR_SETUP(~0) | UTMIP_XCVR_SETUP_MSB(~0) |
UTMIP_XCVR_LSFSLEW(~0) | UTMIP_XCVR_LSRSLEW(~0) |
UTMIP_XCVR_HSSLEW_MSB(~0));
val |= UTMIP_XCVR_SETUP(config->xcvr_setup);
+ val |= UTMIP_XCVR_SETUP_MSB(config->xcvr_setup);
val |= UTMIP_XCVR_LSFSLEW(config->xcvr_lsfslew);
val |= UTMIP_XCVR_LSRSLEW(config->xcvr_lsrslew);
writel(val, base + UTMIP_XCVR_CFG0);
@@ -401,10 +413,6 @@ static int utmi_phy_power_on(struct tegra_usb_phy *phy)
val |= UTMIP_XCVR_TERM_RANGE_ADJ(config->term_range_adj);
writel(val, base + UTMIP_XCVR_CFG1);
- val = readl(base + UTMIP_BAT_CHRG_CFG0);
- val &= ~UTMIP_PD_CHRG;
- writel(val, base + UTMIP_BAT_CHRG_CFG0);
-
val = readl(base + UTMIP_BIAS_CFG1);
val &= ~UTMIP_BIAS_PDTRK_COUNT(~0);
val |= UTMIP_BIAS_PDTRK_COUNT(0x5);
--
1.8.1.5
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 2/6] usb: phy: tegra: Fix wrong PHY parameters
2013-07-31 17:41 ` [PATCH 2/6] usb: phy: tegra: Fix wrong PHY parameters Tuomas Tynkkynen
@ 2013-08-01 21:09 ` Stephen Warren
2013-08-02 14:15 ` Tuomas Tynkkynen
0 siblings, 1 reply; 20+ messages in thread
From: Stephen Warren @ 2013-08-01 21:09 UTC (permalink / raw)
To: Tuomas Tynkkynen
Cc: balbi, linux-tegra, linux-kernel, linux-usb, gregkh, stern
On 07/31/2013 11:41 AM, Tuomas Tynkkynen wrote:
> Some of the PHY parameters are not set according to the TRMs:
>
> - UTMIP_FS_PREABMLE_J should be set, not cleared
> - UTMIP_XCVR_LSBIAS_SEL should be cleared, not set
> - UTMIP_PD_CHRG should be set in host mode and cleared in device mode
> - UTMIP_XCVR_SETUP is a two-part field; the upper bits were not set
> diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
> #define UTMIP_XCVR_SETUP(x) (((x) & 0xf) << 0)
> +#define UTMIP_XCVR_SETUP_MSB(x) ((((x) & 0x7f) >> 4) << 22)
You may as well s/0x7f/0x70/ since the shift clears the 4 LSBs. I'm
pretty sure I mentioned this in downstream review. Perhaps check my
review comments to see if anything else was missed?
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/6] usb: phy: tegra: Fix wrong PHY parameters
2013-08-01 21:09 ` Stephen Warren
@ 2013-08-02 14:15 ` Tuomas Tynkkynen
0 siblings, 0 replies; 20+ messages in thread
From: Tuomas Tynkkynen @ 2013-08-02 14:15 UTC (permalink / raw)
To: Stephen Warren
Cc: balbi@ti.com, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
gregkh@linuxfoundation.org, stern@rowland.harvard.edu
On 08/02/2013 12:09 AM, Stephen Warren wrote:
> On 07/31/2013 11:41 AM, Tuomas Tynkkynen wrote:
>> Some of the PHY parameters are not set according to the TRMs:
>>
>> - UTMIP_FS_PREABMLE_J should be set, not cleared
>> - UTMIP_XCVR_LSBIAS_SEL should be cleared, not set
>> - UTMIP_PD_CHRG should be set in host mode and cleared in device mode
>> - UTMIP_XCVR_SETUP is a two-part field; the upper bits were not set
>
>> diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
>
>> #define UTMIP_XCVR_SETUP(x) (((x) & 0xf) << 0)
>> +#define UTMIP_XCVR_SETUP_MSB(x) ((((x) & 0x7f) >> 4) << 22)
>
> You may as well s/0x7f/0x70/ since the shift clears the 4 LSBs. I'm
> pretty sure I mentioned this in downstream review. Perhaps check my
> review comments to see if anything else was missed?
>
Well in my opinion that increases the risk of typoing the mask.
I'll fix along with the other things.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 3/6] usb: phy: tegra: Tegra30 support
2013-07-31 17:41 [PATCH 0/6] USB tree changes for Tegra30 and Tegra114 USB Host support Tuomas Tynkkynen
2013-07-31 17:41 ` [PATCH 1/6] usb: host: add has_tdi_phy_lpm capability bit Tuomas Tynkkynen
2013-07-31 17:41 ` [PATCH 2/6] usb: phy: tegra: Fix wrong PHY parameters Tuomas Tynkkynen
@ 2013-07-31 17:41 ` Tuomas Tynkkynen
2013-07-31 17:42 ` [PATCH 4/6] usb: phy: tegra: Program new PHY parameters Tuomas Tynkkynen
` (3 subsequent siblings)
6 siblings, 0 replies; 20+ messages in thread
From: Tuomas Tynkkynen @ 2013-07-31 17:41 UTC (permalink / raw)
To: balbi
Cc: linux-tegra, linux-kernel, linux-usb, swarren, gregkh, stern,
Tuomas Tynkkynen
The Tegra30 USB PHY is a bit different than the Tegra20 PHY:
- The EHCI controller supports the HOSTPC register extension, and some
of the fields that the PHY needs to modify (PHCD and PTS) have moved
to the new HOSTPC register.
- Some of the UTMI PLL configuration registers have moved from the USB
register space to the Clock-And-Reset controller space. In Tegra30
the clock driver is responsible for configuring the UTMI PLL.
- The USBMODE register must be explicitly written to enter host mode.
- Certain PHY parameters need to be programmed for optimal signal
quality. Support for this will be added in the next patch.
The new tegra_phy_soc_config structure is added to describe the
differences between the SoCs.
Signed-off-by: Tuomas Tynkkynen <ttynkkynen@nvidia.com>
---
drivers/usb/phy/phy-tegra-usb.c | 121 +++++++++++++++++++++++++++++---------
include/linux/usb/tegra_usb_phy.h | 19 ++++++
2 files changed, 112 insertions(+), 28 deletions(-)
diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
index 936b4a2..85ec70e 100644
--- a/drivers/usb/phy/phy-tegra-usb.c
+++ b/drivers/usb/phy/phy-tegra-usb.c
@@ -28,6 +28,7 @@
#include <linux/io.h>
#include <linux/gpio.h>
#include <linux/of.h>
+#include <linux/of_device.h>
#include <linux/of_gpio.h>
#include <linux/usb/otg.h>
#include <linux/usb/ulpi.h>
@@ -39,11 +40,16 @@
#define ULPI_VIEWPORT 0x170
-/* PORTSC registers */
+/* PORTSC PTS/PHCD bits, Tegra20 only */
#define TEGRA_USB_PORTSC1 0x184
#define TEGRA_USB_PORTSC1_PTS(x) (((x) & 0x3) << 30)
#define TEGRA_USB_PORTSC1_PHCD (1 << 23)
+/* HOSTPC1 PTS/PHCD bits, Tegra30 and above */
+#define TEGRA_USB_HOSTPC1_DEVLC 0x1b4
+#define TEGRA_USB_HOSTPC1_DEVLC_PTS(x) (((x) & 0x7) << 29)
+#define TEGRA_USB_HOSTPC1_DEVLC_PHCD (1 << 22)
+
/* Bits of PORTSC1, which will get cleared by writing 1 into them */
#define TEGRA_PORTSC1_RWC_BITS (PORT_CSC | PORT_PEC | PORT_OCC)
@@ -141,6 +147,12 @@
#define UTMIP_BIAS_CFG1 0x83c
#define UTMIP_BIAS_PDTRK_COUNT(x) (((x) & 0x1f) << 3)
+/* For Tegra30 and above only, the address is different in Tegra20 */
+#define USB_USBMODE 0x1f8
+#define USB_USBMODE_MASK (3 << 0)
+#define USB_USBMODE_HOST (3 << 0)
+#define USB_USBMODE_DEVICE (2 << 0)
+
static DEFINE_SPINLOCK(utmip_pad_lock);
static int utmip_pad_count;
@@ -193,10 +205,17 @@ static void set_pts(struct tegra_usb_phy *phy, u8 pts_val)
void __iomem *base = phy->regs;
unsigned long val;
- val = readl(base + TEGRA_USB_PORTSC1) & ~TEGRA_PORTSC1_RWC_BITS;
- val &= ~TEGRA_USB_PORTSC1_PTS(3);
- val |= TEGRA_USB_PORTSC1_PTS(pts_val & 3);
- writel(val, base + TEGRA_USB_PORTSC1);
+ if (phy->soc_config->has_hostpc) {
+ val = readl(base + TEGRA_USB_HOSTPC1_DEVLC);
+ val &= ~TEGRA_USB_HOSTPC1_DEVLC_PTS(~0);
+ val |= TEGRA_USB_HOSTPC1_DEVLC_PTS(pts_val);
+ writel(val, base + TEGRA_USB_HOSTPC1_DEVLC);
+ } else {
+ val = readl(base + TEGRA_USB_PORTSC1) & ~TEGRA_PORTSC1_RWC_BITS;
+ val &= ~TEGRA_USB_PORTSC1_PTS(~0);
+ val |= TEGRA_USB_PORTSC1_PTS(pts_val);
+ writel(val, base + TEGRA_USB_PORTSC1);
+ }
}
static void set_phcd(struct tegra_usb_phy *phy, bool enable)
@@ -204,12 +223,21 @@ static void set_phcd(struct tegra_usb_phy *phy, bool enable)
void __iomem *base = phy->regs;
unsigned long val;
- val = readl(base + TEGRA_USB_PORTSC1) & ~TEGRA_PORTSC1_RWC_BITS;
- if (enable)
- val |= TEGRA_USB_PORTSC1_PHCD;
- else
- val &= ~TEGRA_USB_PORTSC1_PHCD;
- writel(val, base + TEGRA_USB_PORTSC1);
+ if (phy->soc_config->has_hostpc) {
+ val = readl(base + TEGRA_USB_HOSTPC1_DEVLC);
+ if (enable)
+ val |= TEGRA_USB_HOSTPC1_DEVLC_PHCD;
+ else
+ val &= ~TEGRA_USB_HOSTPC1_DEVLC_PHCD;
+ writel(val, base + TEGRA_USB_HOSTPC1_DEVLC);
+ } else {
+ val = readl(base + TEGRA_USB_PORTSC1) & ~PORT_RWC_BITS;
+ if (enable)
+ val |= TEGRA_USB_PORTSC1_PHCD;
+ else
+ val &= ~TEGRA_USB_PORTSC1_PHCD;
+ writel(val, base + TEGRA_USB_PORTSC1);
+ }
}
static int utmip_pad_open(struct tegra_usb_phy *phy)
@@ -367,17 +395,21 @@ static int utmi_phy_power_on(struct tegra_usb_phy *phy)
val &= ~UTMIP_SUSPEND_EXIT_ON_EDGE;
writel(val, base + UTMIP_MISC_CFG0);
- val = readl(base + UTMIP_MISC_CFG1);
- val &= ~(UTMIP_PLL_ACTIVE_DLY_COUNT(~0) | UTMIP_PLLU_STABLE_COUNT(~0));
- val |= UTMIP_PLL_ACTIVE_DLY_COUNT(phy->freq->active_delay) |
- UTMIP_PLLU_STABLE_COUNT(phy->freq->stable_count);
- writel(val, base + UTMIP_MISC_CFG1);
-
- val = readl(base + UTMIP_PLL_CFG1);
- val &= ~(UTMIP_XTAL_FREQ_COUNT(~0) | UTMIP_PLLU_ENABLE_DLY_COUNT(~0));
- val |= UTMIP_XTAL_FREQ_COUNT(phy->freq->xtal_freq_count) |
- UTMIP_PLLU_ENABLE_DLY_COUNT(phy->freq->enable_delay);
- writel(val, base + UTMIP_PLL_CFG1);
+ if (!phy->soc_config->utmi_pll_config_in_car_module) {
+ val = readl(base + UTMIP_MISC_CFG1);
+ val &= ~(UTMIP_PLL_ACTIVE_DLY_COUNT(~0) |
+ UTMIP_PLLU_STABLE_COUNT(~0));
+ val |= UTMIP_PLL_ACTIVE_DLY_COUNT(phy->freq->active_delay) |
+ UTMIP_PLLU_STABLE_COUNT(phy->freq->stable_count);
+ writel(val, base + UTMIP_MISC_CFG1);
+
+ val = readl(base + UTMIP_PLL_CFG1);
+ val &= ~(UTMIP_XTAL_FREQ_COUNT(~0) |
+ UTMIP_PLLU_ENABLE_DLY_COUNT(~0));
+ val |= UTMIP_XTAL_FREQ_COUNT(phy->freq->xtal_freq_count) |
+ UTMIP_PLLU_ENABLE_DLY_COUNT(phy->freq->enable_delay);
+ writel(val, base + UTMIP_PLL_CFG1);
+ }
if (phy->mode == USB_DR_MODE_PERIPHERAL) {
val = readl(base + USB_SUSP_CTRL);
@@ -448,6 +480,16 @@ static int utmi_phy_power_on(struct tegra_usb_phy *phy)
utmi_phy_clk_enable(phy);
+ if (phy->soc_config->requires_usbmode_setup) {
+ val = readl(base + USB_USBMODE);
+ val &= ~USB_USBMODE_MASK;
+ if (phy->mode == USB_DR_MODE_HOST)
+ val |= USB_USBMODE_HOST;
+ else
+ val |= USB_USBMODE_DEVICE;
+ writel(val, base + USB_USBMODE);
+ }
+
if (!phy->is_legacy_phy)
set_pts(phy, 0);
@@ -864,8 +906,30 @@ static int utmi_phy_probe(struct tegra_usb_phy *tegra_phy,
return 0;
}
+static const struct tegra_phy_soc_config tegra20_soc_config = {
+ .utmi_pll_config_in_car_module = false,
+ .has_hostpc = false,
+ .requires_usbmode_setup = false,
+ .requires_extra_tuning_parameters = false,
+};
+
+static const struct tegra_phy_soc_config tegra30_soc_config = {
+ .utmi_pll_config_in_car_module = true,
+ .has_hostpc = true,
+ .requires_usbmode_setup = true,
+ .requires_extra_tuning_parameters = true,
+};
+
+static struct of_device_id tegra_usb_phy_id_table[] = {
+ { .compatible = "nvidia,tegra30-usb-phy", .data = &tegra30_soc_config },
+ { .compatible = "nvidia,tegra20-usb-phy", .data = &tegra20_soc_config },
+ { },
+};
+MODULE_DEVICE_TABLE(of, tegra_usb_phy_id_table);
+
static int tegra_usb_phy_probe(struct platform_device *pdev)
{
+ const struct of_device_id *match;
struct resource *res;
struct tegra_usb_phy *tegra_phy = NULL;
struct device_node *np = pdev->dev.of_node;
@@ -878,6 +942,13 @@ static int tegra_usb_phy_probe(struct platform_device *pdev)
return -ENOMEM;
}
+ match = of_match_device(tegra_usb_phy_id_table, &pdev->dev);
+ if (!match) {
+ dev_err(&pdev->dev, "Error: No device match found\n");
+ return -ENODEV;
+ }
+ tegra_phy->soc_config = match->data;
+
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res) {
dev_err(&pdev->dev, "Failed to get I/O memory\n");
@@ -963,12 +1034,6 @@ static int tegra_usb_phy_remove(struct platform_device *pdev)
return 0;
}
-static struct of_device_id tegra_usb_phy_id_table[] = {
- { .compatible = "nvidia,tegra20-usb-phy", },
- { },
-};
-MODULE_DEVICE_TABLE(of, tegra_usb_phy_id_table);
-
static struct platform_driver tegra_usb_phy_driver = {
.probe = tegra_usb_phy_probe,
.remove = tegra_usb_phy_remove,
diff --git a/include/linux/usb/tegra_usb_phy.h b/include/linux/usb/tegra_usb_phy.h
index d3db274..d3a63c3 100644
--- a/include/linux/usb/tegra_usb_phy.h
+++ b/include/linux/usb/tegra_usb_phy.h
@@ -18,6 +18,24 @@
#include <linux/clk.h>
#include <linux/usb/otg.h>
+/*
+ * utmi_pll_config_in_car_module: true if the UTMI PLL configuration registers
+ * should be set up by clk-tegra, false if by the PHY code
+ * has_hostpc: true if the USB controller has the HOSTPC extension, which
+ * changes the location of the PHCD and PTS fields
+ * requires_usbmode_setup: true if the USBMODE register needs to be set to
+ * enter host mode
+ * requires_extra_tuning_parameters: true if xcvr_hsslew, hssquelch_level
+ * and hsdiscon_level should be set for adequate signal quality
+ */
+
+struct tegra_phy_soc_config {
+ bool utmi_pll_config_in_car_module;
+ bool has_hostpc;
+ bool requires_usbmode_setup;
+ bool requires_extra_tuning_parameters;
+};
+
struct tegra_utmip_config {
u8 hssync_start_delay;
u8 elastic_limit;
@@ -47,6 +65,7 @@ struct tegra_usb_phy {
struct regulator *vbus;
enum usb_dr_mode mode;
void *config;
+ const struct tegra_phy_soc_config *soc_config;
struct usb_phy *ulpi;
struct usb_phy u_phy;
bool is_legacy_phy;
--
1.8.1.5
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 4/6] usb: phy: tegra: Program new PHY parameters
2013-07-31 17:41 [PATCH 0/6] USB tree changes for Tegra30 and Tegra114 USB Host support Tuomas Tynkkynen
` (2 preceding siblings ...)
2013-07-31 17:41 ` [PATCH 3/6] usb: phy: tegra: Tegra30 support Tuomas Tynkkynen
@ 2013-07-31 17:42 ` Tuomas Tynkkynen
2013-08-01 21:16 ` Stephen Warren
2013-07-31 17:42 ` [PATCH 5/6] Documentation: New DT parameters for tegra30-usb-phy Tuomas Tynkkynen
` (2 subsequent siblings)
6 siblings, 1 reply; 20+ messages in thread
From: Tuomas Tynkkynen @ 2013-07-31 17:42 UTC (permalink / raw)
To: balbi
Cc: linux-tegra, linux-kernel, linux-usb, swarren, gregkh, stern,
Tuomas Tynkkynen
The Tegra30 TRM recommends configuration of certain PHY parameters for
optimal quality. Program the following registers based on device tree
parameters:
- UTMIP_XCVR_HSSLEW: HS slew rate control.
- UTMIP_HSSQUELCH_LEVEL: HS squelch detector level
- UTMIP_HSDISCON_LEVEL: HS disconnect detector level.
These registers exist in Tegra20, but programming them hasn't been
necessary, so these parameters won't be set on Tegra20 to keep the
device trees backward compatible.
Additionally, the UTMIP_XCVR_SETUP parameter can be set from fuses
instead of a software-programmed value, as the optimal value can
vary between invidual boards. The boolean property
nvidia,xcvr-setup-use-fuses can be used to enable this behaviour.
Signed-off-by: Tuomas Tynkkynen <ttynkkynen@nvidia.com>
---
drivers/usb/phy/phy-tegra-usb.c | 75 +++++++++++++++++++++++++++++----------
include/linux/usb/tegra_usb_phy.h | 4 +++
2 files changed, 61 insertions(+), 18 deletions(-)
diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
index 85ec70e..5a34b45 100644
--- a/drivers/usb/phy/phy-tegra-usb.c
+++ b/drivers/usb/phy/phy-tegra-usb.c
@@ -99,11 +99,15 @@
#define UTMIP_FORCE_PD2_POWERDOWN (1 << 16)
#define UTMIP_FORCE_PDZI_POWERDOWN (1 << 18)
#define UTMIP_XCVR_LSBIAS_SEL (1 << 21)
-#define UTMIP_XCVR_HSSLEW_MSB(x) (((x) & 0x7f) << 25)
+#define UTMIP_XCVR_HSSLEW(x) (((x) & 0x3) << 4)
+#define UTMIP_XCVR_HSSLEW_MSB(x) ((((x) & 0x1ff) >> 2) << 25)
#define UTMIP_BIAS_CFG0 0x80c
#define UTMIP_OTGPD (1 << 11)
#define UTMIP_BIASPD (1 << 10)
+#define UTMIP_HSSQUELCH_LEVEL(x) (((x) & 0x3) << 0)
+#define UTMIP_HSDISCON_LEVEL(x) (((x) & 0x3) << 2)
+#define UTMIP_HSDISCON_LEVEL_MSB(x) ((((x) & 0x4) >> 2) << 24)
#define UTMIP_HSRX_CFG0 0x810
#define UTMIP_ELASTIC_LIMIT(x) (((x) & 0x1f) << 10)
@@ -255,6 +259,7 @@ static void utmip_pad_power_on(struct tegra_usb_phy *phy)
{
unsigned long val, flags;
void __iomem *base = phy->pad_regs;
+ struct tegra_utmip_config *config = phy->config;
clk_prepare_enable(phy->pad_clk);
@@ -262,7 +267,14 @@ static void utmip_pad_power_on(struct tegra_usb_phy *phy)
if (utmip_pad_count++ == 0) {
val = readl(base + UTMIP_BIAS_CFG0);
- val &= ~(UTMIP_OTGPD | UTMIP_BIASPD);
+ val &= ~(UTMIP_OTGPD | UTMIP_BIASPD |
+ UTMIP_HSSQUELCH_LEVEL(~0) |
+ UTMIP_HSDISCON_LEVEL(~0) |
+ UTMIP_HSDISCON_LEVEL_MSB(~0));
+
+ val |= UTMIP_HSSQUELCH_LEVEL(config->hssquelch_level);
+ val |= UTMIP_HSDISCON_LEVEL(config->hsdiscon_level);
+ val |= UTMIP_HSDISCON_LEVEL_MSB(config->hsdiscon_level);
writel(val, base + UTMIP_BIAS_CFG0);
}
@@ -432,11 +444,16 @@ static int utmi_phy_power_on(struct tegra_usb_phy *phy)
UTMIP_FORCE_PDZI_POWERDOWN | UTMIP_XCVR_LSBIAS_SEL |
UTMIP_XCVR_SETUP(~0) | UTMIP_XCVR_SETUP_MSB(~0) |
UTMIP_XCVR_LSFSLEW(~0) | UTMIP_XCVR_LSRSLEW(~0) |
- UTMIP_XCVR_HSSLEW_MSB(~0));
- val |= UTMIP_XCVR_SETUP(config->xcvr_setup);
- val |= UTMIP_XCVR_SETUP_MSB(config->xcvr_setup);
+ UTMIP_XCVR_HSSLEW(~0) | UTMIP_XCVR_HSSLEW_MSB(~0));
+
+ if (!config->xcvr_setup_use_fuses) {
+ val |= UTMIP_XCVR_SETUP(config->xcvr_setup);
+ val |= UTMIP_XCVR_SETUP_MSB(config->xcvr_setup);
+ }
val |= UTMIP_XCVR_LSFSLEW(config->xcvr_lsfslew);
val |= UTMIP_XCVR_LSRSLEW(config->xcvr_lsrslew);
+ val |= UTMIP_XCVR_HSSLEW(config->xcvr_hsslew);
+ val |= UTMIP_XCVR_HSSLEW_MSB(config->xcvr_hsslew);
writel(val, base + UTMIP_XCVR_CFG0);
val = readl(base + UTMIP_XCVR_CFG1);
@@ -450,14 +467,14 @@ static int utmi_phy_power_on(struct tegra_usb_phy *phy)
val |= UTMIP_BIAS_PDTRK_COUNT(0x5);
writel(val, base + UTMIP_BIAS_CFG1);
- if (phy->is_legacy_phy) {
- val = readl(base + UTMIP_SPARE_CFG0);
- if (phy->mode == USB_DR_MODE_PERIPHERAL)
- val &= ~FUSE_SETUP_SEL;
- else
- val |= FUSE_SETUP_SEL;
- writel(val, base + UTMIP_SPARE_CFG0);
- } else {
+ val = readl(base + UTMIP_SPARE_CFG0);
+ if (config->xcvr_setup_use_fuses)
+ val |= FUSE_SETUP_SEL;
+ else
+ val &= ~FUSE_SETUP_SEL;
+ writel(val, base + UTMIP_SPARE_CFG0);
+
+ if (!phy->is_legacy_phy) {
val = readl(base + USB_SUSP_CTRL);
val |= UTMIP_PHY_ENABLE;
writel(val, base + USB_SUSP_CTRL);
@@ -888,11 +905,6 @@ static int utmi_phy_probe(struct tegra_usb_phy *tegra_phy,
if (err < 0)
return err;
- err = read_utmi_param(pdev, "nvidia,xcvr-setup",
- &config->xcvr_setup);
- if (err < 0)
- return err;
-
err = read_utmi_param(pdev, "nvidia,xcvr-lsfslew",
&config->xcvr_lsfslew);
if (err < 0)
@@ -903,6 +915,33 @@ static int utmi_phy_probe(struct tegra_usb_phy *tegra_phy,
if (err < 0)
return err;
+ if (tegra_phy->soc_config->requires_extra_tuning_parameters) {
+ err = read_utmi_param(pdev, "nvidia,xcvr-hsslew",
+ &config->xcvr_hsslew);
+ if (err < 0)
+ return err;
+
+ err = read_utmi_param(pdev, "nvidia,hssquelch-level",
+ &config->hssquelch_level);
+ if (err < 0)
+ return err;
+
+ err = read_utmi_param(pdev, "nvidia,hsdiscon-level",
+ &config->hsdiscon_level);
+ if (err < 0)
+ return err;
+ }
+
+ config->xcvr_setup_use_fuses = of_property_read_bool(
+ pdev->dev.of_node, "nvidia,xcvr-setup-use-fuses");
+
+ if (!config->xcvr_setup_use_fuses) {
+ err = read_utmi_param(pdev, "nvidia,xcvr-setup",
+ &config->xcvr_setup);
+ if (err < 0)
+ return err;
+ }
+
return 0;
}
diff --git a/include/linux/usb/tegra_usb_phy.h b/include/linux/usb/tegra_usb_phy.h
index d3a63c3..1de16c3 100644
--- a/include/linux/usb/tegra_usb_phy.h
+++ b/include/linux/usb/tegra_usb_phy.h
@@ -41,9 +41,13 @@ struct tegra_utmip_config {
u8 elastic_limit;
u8 idle_wait_delay;
u8 term_range_adj;
+ bool xcvr_setup_use_fuses;
u8 xcvr_setup;
u8 xcvr_lsfslew;
u8 xcvr_lsrslew;
+ u8 xcvr_hsslew;
+ u8 hssquelch_level;
+ u8 hsdiscon_level;
};
enum tegra_usb_phy_port_speed {
--
1.8.1.5
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 4/6] usb: phy: tegra: Program new PHY parameters
2013-07-31 17:42 ` [PATCH 4/6] usb: phy: tegra: Program new PHY parameters Tuomas Tynkkynen
@ 2013-08-01 21:16 ` Stephen Warren
2013-08-02 14:05 ` Tuomas Tynkkynen
0 siblings, 1 reply; 20+ messages in thread
From: Stephen Warren @ 2013-08-01 21:16 UTC (permalink / raw)
To: Tuomas Tynkkynen
Cc: balbi, linux-tegra, linux-kernel, linux-usb, gregkh, stern
On 07/31/2013 11:42 AM, Tuomas Tynkkynen wrote:
> The Tegra30 TRM recommends configuration of certain PHY parameters for
> optimal quality. Program the following registers based on device tree
> parameters:
>
> - UTMIP_XCVR_HSSLEW: HS slew rate control.
> - UTMIP_HSSQUELCH_LEVEL: HS squelch detector level
> - UTMIP_HSDISCON_LEVEL: HS disconnect detector level.
>
> These registers exist in Tegra20, but programming them hasn't been
> necessary, so these parameters won't be set on Tegra20 to keep the
> device trees backward compatible.
>
> Additionally, the UTMIP_XCVR_SETUP parameter can be set from fuses
> instead of a software-programmed value, as the optimal value can
> vary between invidual boards. The boolean property
> nvidia,xcvr-setup-use-fuses can be used to enable this behaviour.
> diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
> -#define UTMIP_XCVR_HSSLEW_MSB(x) (((x) & 0x7f) << 25)
> +#define UTMIP_XCVR_HSSLEW(x) (((x) & 0x3) << 4)
> +#define UTMIP_XCVR_HSSLEW_MSB(x) ((((x) & 0x1ff) >> 2) << 25)
Similarly, may as well s/0x1ff/0x1fc/ there too.
> @@ -262,7 +267,14 @@ static void utmip_pad_power_on(struct tegra_usb_phy *phy)
>
> if (utmip_pad_count++ == 0) {
> val = readl(base + UTMIP_BIAS_CFG0);
> - val &= ~(UTMIP_OTGPD | UTMIP_BIASPD);
> + val &= ~(UTMIP_OTGPD | UTMIP_BIASPD |
> + UTMIP_HSSQUELCH_LEVEL(~0) |
> + UTMIP_HSDISCON_LEVEL(~0) |
> + UTMIP_HSDISCON_LEVEL_MSB(~0));
> +
> + val |= UTMIP_HSSQUELCH_LEVEL(config->hssquelch_level);
> + val |= UTMIP_HSDISCON_LEVEL(config->hsdiscon_level);
> + val |= UTMIP_HSDISCON_LEVEL_MSB(config->hsdiscon_level);
> writel(val, base + UTMIP_BIAS_CFG0);
> }
>
> @@ -432,11 +444,16 @@ static int utmi_phy_power_on(struct tegra_usb_phy *phy)
> UTMIP_FORCE_PDZI_POWERDOWN | UTMIP_XCVR_LSBIAS_SEL |
> UTMIP_XCVR_SETUP(~0) | UTMIP_XCVR_SETUP_MSB(~0) |
> UTMIP_XCVR_LSFSLEW(~0) | UTMIP_XCVR_LSRSLEW(~0) |
> - UTMIP_XCVR_HSSLEW_MSB(~0));
> - val |= UTMIP_XCVR_SETUP(config->xcvr_setup);
> - val |= UTMIP_XCVR_SETUP_MSB(config->xcvr_setup);
> + UTMIP_XCVR_HSSLEW(~0) | UTMIP_XCVR_HSSLEW_MSB(~0));
> +
> + if (!config->xcvr_setup_use_fuses) {
> + val |= UTMIP_XCVR_SETUP(config->xcvr_setup);
> + val |= UTMIP_XCVR_SETUP_MSB(config->xcvr_setup);
> + }
> val |= UTMIP_XCVR_LSFSLEW(config->xcvr_lsfslew);
> val |= UTMIP_XCVR_LSRSLEW(config->xcvr_lsrslew);
> + val |= UTMIP_XCVR_HSSLEW(config->xcvr_hsslew);
> + val |= UTMIP_XCVR_HSSLEW_MSB(config->xcvr_hsslew);
Those two chunks end up clearing some fields in the register now even on
earlier chips, whereas before their values were maintained when doing
the read/modify/write. Yet, the commit description says the new fields
aren't changed on Tegra20. Do the changes above need to be guarded by if
(requires_extra_tuning_parameters)?
(When I tested this series, I only tested Tegra30/114; I didn't any
Tegra20 devices...)
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 4/6] usb: phy: tegra: Program new PHY parameters
2013-08-01 21:16 ` Stephen Warren
@ 2013-08-02 14:05 ` Tuomas Tynkkynen
0 siblings, 0 replies; 20+ messages in thread
From: Tuomas Tynkkynen @ 2013-08-02 14:05 UTC (permalink / raw)
To: Stephen Warren; +Cc: balbi, linux-tegra, linux-kernel, linux-usb, gregkh, stern
On 08/02/2013 12:16 AM, Stephen Warren wrote:
> On 07/31/2013 11:42 AM, Tuomas Tynkkynen wrote:
>> The Tegra30 TRM recommends configuration of certain PHY parameters for
>> optimal quality. Program the following registers based on device tree
>> parameters:
>>
>> - UTMIP_XCVR_HSSLEW: HS slew rate control.
>> - UTMIP_HSSQUELCH_LEVEL: HS squelch detector level
>> - UTMIP_HSDISCON_LEVEL: HS disconnect detector level.
>>
>> These registers exist in Tegra20, but programming them hasn't been
>> necessary, so these parameters won't be set on Tegra20 to keep the
>> device trees backward compatible.
>>
>> Additionally, the UTMIP_XCVR_SETUP parameter can be set from fuses
>> instead of a software-programmed value, as the optimal value can
>> vary between invidual boards. The boolean property
>> nvidia,xcvr-setup-use-fuses can be used to enable this behaviour.
>
>> diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
> Those two chunks end up clearing some fields in the register now even on
> earlier chips, whereas before their values were maintained when doing
> the read/modify/write. Yet, the commit description says the new fields
> aren't changed on Tegra20. Do the changes above need to be guarded by if
> (requires_extra_tuning_parameters)?
Oops, you are right. I overlooked that some of those fields have non-zero reset values.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 5/6] Documentation: New DT parameters for tegra30-usb-phy
2013-07-31 17:41 [PATCH 0/6] USB tree changes for Tegra30 and Tegra114 USB Host support Tuomas Tynkkynen
` (3 preceding siblings ...)
2013-07-31 17:42 ` [PATCH 4/6] usb: phy: tegra: Program new PHY parameters Tuomas Tynkkynen
@ 2013-07-31 17:42 ` Tuomas Tynkkynen
2013-08-01 21:17 ` Stephen Warren
2013-07-31 17:42 ` [PATCH 6/6] usb: host: tegra: Tegra30 support Tuomas Tynkkynen
2013-07-31 23:22 ` [PATCH 0/6] USB tree changes for Tegra30 and Tegra114 USB Host support Stephen Warren
6 siblings, 1 reply; 20+ messages in thread
From: Tuomas Tynkkynen @ 2013-07-31 17:42 UTC (permalink / raw)
To: balbi
Cc: linux-tegra, linux-kernel, linux-usb, swarren, gregkh, stern,
Tuomas Tynkkynen
Document the new device tree parameters for Tegra30 USB PHY.
Signed-off-by: Tuomas Tynkkynen <ttynkkynen@nvidia.com>
---
.../devicetree/bindings/usb/nvidia,tegra20-usb-phy.txt | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/nvidia,tegra20-usb-phy.txt b/Documentation/devicetree/bindings/usb/nvidia,tegra20-usb-phy.txt
index 59c4854..8b5901d 100644
--- a/Documentation/devicetree/bindings/usb/nvidia,tegra20-usb-phy.txt
+++ b/Documentation/devicetree/bindings/usb/nvidia,tegra20-usb-phy.txt
@@ -3,7 +3,7 @@ Tegra SOC USB PHY
The device node for Tegra SOC USB PHY:
Required properties :
- - compatible : Should be "nvidia,tegra20-usb-phy".
+ - compatible : Should be "nvidia,tegra<chip>-usb-phy".
- reg : Defines the following set of registers, in the order listed:
- The PHY's own register set.
Always present.
@@ -24,17 +24,26 @@ Required properties :
Required properties for phy_type == ulpi:
- nvidia,phy-reset-gpio : The GPIO used to reset the PHY.
-Required PHY timing params for utmi phy:
+Required PHY timing params for utmi phy, for all chips:
- nvidia,hssync-start-delay : Number of 480 Mhz clock cycles to wait before
start of sync launches RxActive
- nvidia,elastic-limit : Variable FIFO Depth of elastic input store
- nvidia,idle-wait-delay : Number of 480 Mhz clock cycles of idle to wait
before declare IDLE.
- nvidia,term-range-adj : Range adjusment on terminations
- - nvidia,xcvr-setup : HS driver output control
+ - Either one of the following for HS driver output control:
+ - nvidia,xcvr-setup : integer, uses the provided value.
+ - nvidia,xcvr-setup-use-fuses : boolean, indicates that the value is read
+ from the on-chip fuses
+ If both are provided, nvidia,xcvr-setup-use-fuses takes precedence.
- nvidia,xcvr-lsfslew : LS falling slew rate control.
- nvidia,xcvr-lsrslew : LS rising slew rate control.
+Required PHY timing params for utmi phy, only on Tegra30 and above:
+ - nvidia,xcvr-hsslew : HS slew rate control.
+ - nvidia,hssquelch-level : HS squelch detector level.
+ - nvidia,hsdiscon-level : HS disconnect detector level.
+
Optional properties:
- nvidia,has-legacy-mode : boolean indicates whether this controller can
operate in legacy mode (as APX 2500 / 2600). In legacy mode some
--
1.8.1.5
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 6/6] usb: host: tegra: Tegra30 support
2013-07-31 17:41 [PATCH 0/6] USB tree changes for Tegra30 and Tegra114 USB Host support Tuomas Tynkkynen
` (4 preceding siblings ...)
2013-07-31 17:42 ` [PATCH 5/6] Documentation: New DT parameters for tegra30-usb-phy Tuomas Tynkkynen
@ 2013-07-31 17:42 ` Tuomas Tynkkynen
2013-08-01 21:18 ` Stephen Warren
2013-07-31 23:22 ` [PATCH 0/6] USB tree changes for Tegra30 and Tegra114 USB Host support Stephen Warren
6 siblings, 1 reply; 20+ messages in thread
From: Tuomas Tynkkynen @ 2013-07-31 17:42 UTC (permalink / raw)
To: balbi
Cc: linux-tegra, linux-kernel, linux-usb, swarren, gregkh, stern,
Tuomas Tynkkynen
The Tegra30 EHCI controller is mostly compatible with the Tegra20
controller, except Tegra30 includes the HOSTPC register extension.
The has_hostpc capability bit must be set in the ehci_hcd structure if
the controller has such extensions. The new tegra_ehci_soc_config
structure is added to describe the differences between the SoCs.
Signed-off-by: Tuomas Tynkkynen <ttynkkynen@nvidia.com>
---
drivers/usb/host/ehci-tegra.c | 34 +++++++++++++++++++++++++++++-----
1 file changed, 29 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
index db8031f..c0d1f27 100644
--- a/drivers/usb/host/ehci-tegra.c
+++ b/drivers/usb/host/ehci-tegra.c
@@ -25,6 +25,7 @@
#include <linux/irq.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/of_device.h>
#include <linux/of_gpio.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
@@ -50,6 +51,10 @@
static struct hc_driver __read_mostly tegra_ehci_hc_driver;
+struct tegra_ehci_soc_config {
+ bool has_hostpc;
+};
+
static int (*orig_hub_control)(struct usb_hcd *hcd,
u16 typeReq, u16 wValue, u16 wIndex,
char *buf, u16 wLength);
@@ -320,8 +325,24 @@ static void tegra_ehci_unmap_urb_for_dma(struct usb_hcd *hcd, struct urb *urb)
free_dma_aligned_buffer(urb);
}
+static const struct tegra_ehci_soc_config tegra30_soc_config = {
+ .has_hostpc = true,
+};
+
+static const struct tegra_ehci_soc_config tegra20_soc_config = {
+ .has_hostpc = false,
+};
+
+static struct of_device_id tegra_ehci_of_match[] = {
+ { .compatible = "nvidia,tegra30-ehci", .data = &tegra30_soc_config },
+ { .compatible = "nvidia,tegra20-ehci", .data = &tegra20_soc_config },
+ { },
+};
+
static int tegra_ehci_probe(struct platform_device *pdev)
{
+ const struct of_device_id *match;
+ const struct tegra_ehci_soc_config *soc_config;
struct resource *res;
struct usb_hcd *hcd;
struct ehci_hcd *ehci;
@@ -330,6 +351,13 @@ static int tegra_ehci_probe(struct platform_device *pdev)
int irq;
struct usb_phy *u_phy;
+ match = of_match_device(tegra_ehci_of_match, &pdev->dev);
+ if (!match) {
+ dev_err(&pdev->dev, "Error: No device match found\n");
+ return -ENODEV;
+ }
+ soc_config = (struct tegra_ehci_soc_config *)match->data;
+
/* Right now device-tree probed devices don't get dma_mask set.
* Since shared usb code relies on it, set it here for now.
* Once we have dma capability bindings this can go away.
@@ -391,6 +419,7 @@ static int tegra_ehci_probe(struct platform_device *pdev)
goto cleanup_clk_en;
}
ehci->caps = hcd->regs + 0x100;
+ ehci->has_hostpc = soc_config->has_hostpc;
err = usb_phy_init(hcd->phy);
if (err) {
@@ -468,11 +497,6 @@ static void tegra_ehci_hcd_shutdown(struct platform_device *pdev)
hcd->driver->shutdown(hcd);
}
-static struct of_device_id tegra_ehci_of_match[] = {
- { .compatible = "nvidia,tegra20-ehci", },
- { },
-};
-
static struct platform_driver tegra_ehci_driver = {
.probe = tegra_ehci_probe,
.remove = tegra_ehci_remove,
--
1.8.1.5
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 6/6] usb: host: tegra: Tegra30 support
2013-07-31 17:42 ` [PATCH 6/6] usb: host: tegra: Tegra30 support Tuomas Tynkkynen
@ 2013-08-01 21:18 ` Stephen Warren
0 siblings, 0 replies; 20+ messages in thread
From: Stephen Warren @ 2013-08-01 21:18 UTC (permalink / raw)
To: Tuomas Tynkkynen
Cc: balbi, linux-tegra, linux-kernel, linux-usb, gregkh, stern
On 07/31/2013 11:42 AM, Tuomas Tynkkynen wrote:
> The Tegra30 EHCI controller is mostly compatible with the Tegra20
> controller, except Tegra30 includes the HOSTPC register extension.
> The has_hostpc capability bit must be set in the ehci_hcd structure if
> the controller has such extensions. The new tegra_ehci_soc_config
> structure is added to describe the differences between the SoCs.
Aside from the minor issues I already pointed out, the series briefly,
Reviewed-by: Stephen Warren <swarren@nvidia.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/6] USB tree changes for Tegra30 and Tegra114 USB Host support
2013-07-31 17:41 [PATCH 0/6] USB tree changes for Tegra30 and Tegra114 USB Host support Tuomas Tynkkynen
` (5 preceding siblings ...)
2013-07-31 17:42 ` [PATCH 6/6] usb: host: tegra: Tegra30 support Tuomas Tynkkynen
@ 2013-07-31 23:22 ` Stephen Warren
2013-08-01 13:02 ` Tuomas Tynkkynen
6 siblings, 1 reply; 20+ messages in thread
From: Stephen Warren @ 2013-07-31 23:22 UTC (permalink / raw)
To: Tuomas Tynkkynen
Cc: balbi, linux-tegra, linux-kernel, linux-usb, gregkh, stern
On 07/31/2013 11:41 AM, Tuomas Tynkkynen wrote:
> Hi all,
>
> Here are the patches for the USB tree to enable USB Host support on Tegra30 and
> Tegra114. These are based on my and Mikko's cleanup patches that just got
> merged to Felipe's tree.
This series works fine for me on Dalmore. However, on Beaver, I see the
following:
[ 2.428480] platform 7d008000.usb: Driver tegra-ehci requests probe
deferral
... which never seems to be resolved. You can take a look at my
linux-next_common branch to repro this. Does Beaver work for you?
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 0/6] USB tree changes for Tegra30 and Tegra114 USB Host support
2013-07-31 23:22 ` [PATCH 0/6] USB tree changes for Tegra30 and Tegra114 USB Host support Stephen Warren
@ 2013-08-01 13:02 ` Tuomas Tynkkynen
2013-08-01 16:22 ` Stephen Warren
0 siblings, 1 reply; 20+ messages in thread
From: Tuomas Tynkkynen @ 2013-08-01 13:02 UTC (permalink / raw)
To: Stephen Warren
Cc: balbi@ti.com, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
gregkh@linuxfoundation.org, stern@rowland.harvard.edu
On 08/01/2013 02:22 AM, Stephen Warren wrote:
> On 07/31/2013 11:41 AM, Tuomas Tynkkynen wrote:
>> Hi all,
>>
>> Here are the patches for the USB tree to enable USB Host support on Tegra30 and
>> Tegra114. These are based on my and Mikko's cleanup patches that just got
>> merged to Felipe's tree.
>
> This series works fine for me on Dalmore. However, on Beaver, I see the
> following:
>
> [ 2.428480] platform 7d008000.usb: Driver tegra-ehci requests probe
> deferral
>
> ... which never seems to be resolved. You can take a look at my
> linux-next_common branch to repro this. Does Beaver work for you?
>
The clock driver patch is missing from your branch. Applying it should fix that.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/6] USB tree changes for Tegra30 and Tegra114 USB Host support
2013-08-01 13:02 ` Tuomas Tynkkynen
@ 2013-08-01 16:22 ` Stephen Warren
2013-08-02 7:52 ` Felipe Balbi
0 siblings, 1 reply; 20+ messages in thread
From: Stephen Warren @ 2013-08-01 16:22 UTC (permalink / raw)
To: Tuomas Tynkkynen
Cc: balbi@ti.com, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
gregkh@linuxfoundation.org, stern@rowland.harvard.edu
On 08/01/2013 07:02 AM, Tuomas Tynkkynen wrote:
> On 08/01/2013 02:22 AM, Stephen Warren wrote:
>> On 07/31/2013 11:41 AM, Tuomas Tynkkynen wrote:
>>> Hi all,
>>>
>>> Here are the patches for the USB tree to enable USB Host support on Tegra30 and
>>> Tegra114. These are based on my and Mikko's cleanup patches that just got
>>> merged to Felipe's tree.
>>
>> This series works fine for me on Dalmore. However, on Beaver, I see the
>> following:
>>
>> [ 2.428480] platform 7d008000.usb: Driver tegra-ehci requests probe
>> deferral
>>
>> ... which never seems to be resolved. You can take a look at my
>> linux-next_common branch to repro this. Does Beaver work for you?
>>
>
> The clock driver patch is missing from your branch. Applying it should fix that.
Oh so it does. Sorry for forgetting about that patch. Given that, the
series,
Tested-by: Stephen Warren <swarren@nvidia.com>
I'll also try to review/ack the patches ASAP.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/6] USB tree changes for Tegra30 and Tegra114 USB Host support
2013-08-01 16:22 ` Stephen Warren
@ 2013-08-02 7:52 ` Felipe Balbi
0 siblings, 0 replies; 20+ messages in thread
From: Felipe Balbi @ 2013-08-02 7:52 UTC (permalink / raw)
To: Stephen Warren
Cc: Tuomas Tynkkynen, balbi@ti.com, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
gregkh@linuxfoundation.org, stern@rowland.harvard.edu
[-- Attachment #1: Type: text/plain, Size: 1170 bytes --]
On Thu, Aug 01, 2013 at 10:22:19AM -0600, Stephen Warren wrote:
> On 08/01/2013 07:02 AM, Tuomas Tynkkynen wrote:
> > On 08/01/2013 02:22 AM, Stephen Warren wrote:
> >> On 07/31/2013 11:41 AM, Tuomas Tynkkynen wrote:
> >>> Hi all,
> >>>
> >>> Here are the patches for the USB tree to enable USB Host support on Tegra30 and
> >>> Tegra114. These are based on my and Mikko's cleanup patches that just got
> >>> merged to Felipe's tree.
> >>
> >> This series works fine for me on Dalmore. However, on Beaver, I see the
> >> following:
> >>
> >> [ 2.428480] platform 7d008000.usb: Driver tegra-ehci requests probe
> >> deferral
> >>
> >> ... which never seems to be resolved. You can take a look at my
> >> linux-next_common branch to repro this. Does Beaver work for you?
> >>
> >
> > The clock driver patch is missing from your branch. Applying it should fix that.
>
> Oh so it does. Sorry for forgetting about that patch. Given that, the
> series,
>
> Tested-by: Stephen Warren <swarren@nvidia.com>
>
> I'll also try to review/ack the patches ASAP.
I see you requested one minor change, so I'll wait for a new version.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread