Patchwork [2/6] usb: phy: tegra: Fix wrong PHY parameters

login
register
mail settings
Submitter Tuomas Tynkkynen
Date July 31, 2013, 5:41 p.m.
Message ID <1375292522-7855-3-git-send-email-ttynkkynen@nvidia.com>
Download mbox | patch
Permalink /patch/263780/
State Not Applicable, archived
Headers show

Comments

Tuomas Tynkkynen - July 31, 2013, 5:41 p.m.
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(-)
Stephen Warren - Aug. 1, 2013, 9:09 p.m.
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?
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tuomas Tynkkynen - Aug. 2, 2013, 2:15 p.m.
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

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