2013-07-31 17:42:12

by Tuomas Tynkkynen

[permalink] [raw]
Subject: [PATCH 0/6] USB tree changes for Tegra30 and Tegra114 USB Host support

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.

The first one touches the core hub code to prevent certain (non-standard) clock
disable features as our controller doesn't support those.
The rest are changes to our PHY and EHCI drivers to deal with a few changed
register addresses and a few additional PHY parameters that need to be set
for proper signal quality.

Tuomas Tynkkynen (6):
usb: host: add has_tdi_phy_lpm capability bit
usb: phy: tegra: Fix wrong PHY parameters
usb: phy: tegra: Tegra30 support
usb: phy: tegra: Program new PHY parameters
Documentation: New DT parameters for tegra30-usb-phy
usb: host: tegra: Tegra30 support

.../bindings/usb/nvidia,tegra20-usb-phy.txt | 15 +-
drivers/usb/chipidea/host.c | 1 +
drivers/usb/host/ehci-hub.c | 14 +-
drivers/usb/host/ehci-tegra.c | 34 +++-
drivers/usb/host/ehci.h | 1 +
drivers/usb/phy/phy-tegra-usb.c | 214 ++++++++++++++++-----
include/linux/usb/tegra_usb_phy.h | 23 +++
7 files changed, 236 insertions(+), 66 deletions(-)

--
1.8.1.5


2013-07-31 17:42:14

by Tuomas Tynkkynen

[permalink] [raw]
Subject: [PATCH 1/6] usb: host: add has_tdi_phy_lpm capability bit

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 <[email protected]>
Signed-off-by: Tuomas Tynkkynen <[email protected]>
---
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

2013-07-31 17:42:23

by Tuomas Tynkkynen

[permalink] [raw]
Subject: [PATCH 2/6] usb: phy: tegra: Fix wrong PHY parameters

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 <[email protected]>
---
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

2013-07-31 17:42:29

by Tuomas Tynkkynen

[permalink] [raw]
Subject: [PATCH 6/6] usb: host: tegra: Tegra30 support

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 <[email protected]>
---
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

2013-07-31 17:44:42

by Tuomas Tynkkynen

[permalink] [raw]
Subject: [PATCH 5/6] Documentation: New DT parameters for tegra30-usb-phy

Document the new device tree parameters for Tegra30 USB PHY.

Signed-off-by: Tuomas Tynkkynen <[email protected]>
---
.../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

2013-07-31 17:42:20

by Tuomas Tynkkynen

[permalink] [raw]
Subject: [PATCH 3/6] usb: phy: tegra: Tegra30 support

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 <[email protected]>
---
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

2013-07-31 17:45:41

by Tuomas Tynkkynen

[permalink] [raw]
Subject: [PATCH 4/6] usb: phy: tegra: Program new PHY parameters

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 <[email protected]>
---
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

2013-07-31 18:35:50

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH 1/6] usb: host: add has_tdi_phy_lpm capability bit

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 <[email protected]>
> Signed-off-by: Tuomas Tynkkynen <[email protected]>

Acked-by: Alan Stern <[email protected]>

2013-07-31 23:30:18

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH 0/6] USB tree changes for Tegra30 and Tegra114 USB Host support

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?

2013-08-01 08:05:55

by Matthieu CASTET

[permalink] [raw]
Subject: Re: [PATCH 1/6] usb: host: add has_tdi_phy_lpm capability bit

Hi,

Le Wed, 31 Jul 2013 18:41:57 +0100,
Tuomas Tynkkynen <[email protected]> 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 <[email protected]>
> Signed-off-by: Tuomas Tynkkynen <[email protected]>
> ---
> 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 */

2013-08-01 13:02:24

by Tuomas Tynkkynen

[permalink] [raw]
Subject: Re: [PATCH 0/6] USB tree changes for Tegra30 and Tegra114 USB Host support

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.

2013-08-01 16:22:26

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH 0/6] USB tree changes for Tegra30 and Tegra114 USB Host support

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 <[email protected]>

I'll also try to review/ack the patches ASAP.

2013-08-01 21:09:13

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH 2/6] usb: phy: tegra: Fix wrong PHY parameters

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?

2013-08-01 21:16:21

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH 4/6] usb: phy: tegra: Program new PHY parameters

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...)

2013-08-01 21:17:30

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH 5/6] Documentation: New DT parameters for tegra30-usb-phy

On 07/31/2013 11:42 AM, Tuomas Tynkkynen wrote:
> Document the new device tree parameters for Tegra30 USB PHY.

Very minor nit: It would make sense to move this patch before the
previous patch (or squash them together) so that the documentation is
updated first. But, it makes no difference to the code/git-bisect/...

2013-08-01 21:18:29

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH 6/6] usb: host: tegra: Tegra30 support

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 <[email protected]>

2013-08-02 07:53:16

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH 0/6] USB tree changes for Tegra30 and Tegra114 USB Host support

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 <[email protected]>
>
> 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


Attachments:
(No filename) (1.11 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2013-08-02 14:05:45

by Tuomas Tynkkynen

[permalink] [raw]
Subject: Re: [PATCH 4/6] usb: phy: tegra: Program new PHY parameters

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.

2013-08-02 14:15:44

by Tuomas Tynkkynen

[permalink] [raw]
Subject: Re: [PATCH 2/6] usb: phy: tegra: Fix wrong PHY parameters

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.

2013-08-02 15:42:53

by Tuomas Tynkkynen

[permalink] [raw]
Subject: Re: [PATCH 1/6] usb: host: add has_tdi_phy_lpm capability bit

On 08/01/2013 11:05 AM, Matthieu CASTET wrote:
> Hi,
>
> Le Wed, 31 Jul 2013 18:41:57 +0100,
> Tuomas Tynkkynen <[email protected]> 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 <[email protected]>
>> Signed-off-by: Tuomas Tynkkynen <[email protected]>
>> ---
>> 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(-)
>>