Patchwork [1/8] phy: tegra: xusb: t210: add XUSB device mode support

login
register
mail settings
Submitter Nagarjuna Kristam
Date Jan. 3, 2019, 10:04 a.m.
Message ID <1546509899-5071-2-git-send-email-nkristam@nvidia.com>
Download mbox | patch
Permalink /patch/692031/
State New
Headers show

Comments

Nagarjuna Kristam - Jan. 3, 2019, 10:04 a.m.
Add support for XUSB device mode controller on Tegra210.
Update PADCTL driver to set port cap based on DT config.
Add code to handle property "nvidia,usb3-port-fake"
Provide API's to control vbus override and utmi pad power control

Signed-off-by: Nagarjuna Kristam <nkristam@nvidia.com>
---
 drivers/phy/tegra/xusb-tegra210.c | 297 +++++++++++++++++++++++++++++++++++++-
 drivers/phy/tegra/xusb.c          |  75 +++++++++-
 drivers/phy/tegra/xusb.h          |  16 +-
 include/linux/phy/tegra/xusb.h    |   7 +-
 4 files changed, 386 insertions(+), 9 deletions(-)
Thierry Reding - Feb. 4, 2019, 12:27 p.m.
On Thu, Jan 03, 2019 at 03:34:52PM +0530, Nagarjuna Kristam wrote:
> Add support for XUSB device mode controller on Tegra210.
> Update PADCTL driver to set port cap based on DT config.
> Add code to handle property "nvidia,usb3-port-fake"
> Provide API's to control vbus override and utmi pad power control

You essentially list three features additions here, so it makes sense to
provide each of them in a separate patch. That helps reviewers
concentrate on one feature at a time without having to load context for
three different things in parallel.

Also, for each feature please provide a bit more background information
as well as reasons for why they are added.

> Signed-off-by: Nagarjuna Kristam <nkristam@nvidia.com>
> ---
>  drivers/phy/tegra/xusb-tegra210.c | 297 +++++++++++++++++++++++++++++++++++++-
>  drivers/phy/tegra/xusb.c          |  75 +++++++++-
>  drivers/phy/tegra/xusb.h          |  16 +-
>  include/linux/phy/tegra/xusb.h    |   7 +-
>  4 files changed, 386 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/phy/tegra/xusb-tegra210.c b/drivers/phy/tegra/xusb-tegra210.c
> index 05bee32..0289e10 100644
> --- a/drivers/phy/tegra/xusb-tegra210.c
> +++ b/drivers/phy/tegra/xusb-tegra210.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2014, NVIDIA CORPORATION.  All rights reserved.
> + * Copyright (c) 2014-2018, NVIDIA CORPORATION.  All rights reserved.
>   * Copyright (C) 2015 Google, Inc.
>   *
>   * This program is free software; you can redistribute it and/or modify it
> @@ -47,7 +47,10 @@
>  #define XUSB_PADCTL_USB2_PAD_MUX_USB2_BIAS_PAD_XUSB 0x1
>  
>  #define XUSB_PADCTL_USB2_PORT_CAP 0x008
> +#define XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_DISABLED(x) (0x0 << ((x) * 4))
>  #define XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_HOST(x) (0x1 << ((x) * 4))
> +#define XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_DEVICE(x) (0x2 << ((x) * 4))
> +#define XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_OTG(x) (0x3 << ((x) * 4))
>  #define XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_MASK(x) (0x3 << ((x) * 4))
>  
>  #define XUSB_PADCTL_SS_PORT_MAP 0x014
> @@ -55,6 +58,7 @@
>  #define XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_SHIFT(x) ((x) * 5)
>  #define XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_MASK(x) (0x7 << ((x) * 5))
>  #define XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP(x, v) (((v) & 0x7) << ((x) * 5))
> +#define XUSB_PADCTL_SS_PORT_MAP_PORT_DISABLED 0x7
>  
>  #define XUSB_PADCTL_ELPG_PROGRAM1 0x024
>  #define XUSB_PADCTL_ELPG_PROGRAM1_AUX_MUX_LP0_VCORE_DOWN (1 << 31)
> @@ -69,9 +73,14 @@
>  #define XUSB_PADCTL_USB3_PAD_MUX_PCIE_IDDQ_DISABLE(x) (1 << (1 + (x)))
>  #define XUSB_PADCTL_USB3_PAD_MUX_SATA_IDDQ_DISABLE(x) (1 << (8 + (x)))
>  
> +#define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPADX_CTL0(x) (0x080 + (x) * 0x40)
> +#define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL0_ZIP (1 << 18)
> +#define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL0_ZIN (1 << 22)
> +
>  #define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPADX_CTL1(x) (0x084 + (x) * 0x40)
>  #define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_SHIFT 7
>  #define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_MASK 0x3
> +#define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_VAL 0x1
>  #define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_FIX18 (1 << 6)
>  
>  #define XUSB_PADCTL_USB2_OTG_PADX_CTL0(x) (0x088 + (x) * 0x40)
> @@ -230,6 +239,12 @@
>  #define XUSB_PADCTL_UPHY_USB3_PADX_ECTL6(x) (0xa74 + (x) * 0x40)
>  #define XUSB_PADCTL_UPHY_USB3_PAD_ECTL6_RX_EQ_CTRL_H_VAL 0xfcf01368
>  
> +#define XUSB_PADCTL_USB2_VBUS_ID 0xc60
> +#define XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_VBUS_ON (1 << 14)
> +#define XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_SHIFT 18
> +#define XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_MASK 0xf
> +#define XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_VAL 8
> +
>  struct tegra210_xusb_fuse_calibration {
>  	u32 hs_curr_level[4];
>  	u32 hs_term_range_adj;
> @@ -905,6 +920,161 @@ static const struct tegra_xusb_lane_ops tegra210_usb2_lane_ops = {
>  	.remove = tegra210_usb2_lane_remove,
>  };
>  
> +/* must be called under padctl->lock */
> +static void tegra210_utmi_bias_pad_power_on(struct tegra_xusb_padctl *padctl,
> +				struct tegra_xusb_usb2_pad *pad)
> +{
> +	u32 reg;
> +
> +	if (pad->enable++ > 0)
> +		return;
> +
> +	dev_dbg(padctl->dev, "power on BIAS PAD & USB2 tracking\n");
> +
> +	if (clk_prepare_enable(pad->clk))
> +		dev_warn(padctl->dev, "failed to enable BIAS PAD & USB2 tracking\n");

Maybe keep track of the exact error and make that part of the error
message?

> +
> +	reg = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
> +
> +	reg &= ~((XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_START_TIMER_MASK <<
> +		    XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_START_TIMER_SHIFT) |
> +		   (XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_DONE_RESET_TIMER_MASK <<
> +		    XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_DONE_RESET_TIMER_SHIFT));
> +	reg |= (XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_START_TIMER_VAL <<
> +		  XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_START_TIMER_SHIFT) |
> +		 (XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_DONE_RESET_TIMER_VAL <<
> +		  XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_DONE_RESET_TIMER_SHIFT);
> +	padctl_writel(padctl, reg, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
> +
> +	reg = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL0);
> +	reg &= ~XUSB_PADCTL_USB2_BIAS_PAD_CTL0_PD;
> +
> +	reg &= ~((XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_SQUELCH_LEVEL_MASK <<
> +		    XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_SQUELCH_LEVEL_SHIFT) |
> +		   (XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_DISCON_LEVEL_MASK <<
> +		    XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_DISCON_LEVEL_SHIFT));
> +	reg |= (XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_DISCON_LEVEL_VAL <<
> +		  XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_DISCON_LEVEL_SHIFT);
> +
> +	if (tegra_sku_info.revision < TEGRA_REVISION_A02)
> +		reg |= (XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_SQUELCH_LEVEL_VAL <<
> +			XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_SQUELCH_LEVEL_SHIFT);
> +	padctl_writel(padctl, reg, XUSB_PADCTL_USB2_BIAS_PAD_CTL0);
> +
> +	udelay(1);
> +
> +	reg = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
> +	reg &= ~XUSB_PADCTL_USB2_BIAS_PAD_CTL1_PD_TRK;
> +	padctl_writel(padctl, reg, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
> +}
> +
> +/* must be called under padctl->lock */
> +static void tegra210_utmi_bias_pad_power_off(struct tegra_xusb_padctl *padctl,
> +				struct tegra_xusb_usb2_pad *pad)
> +{
> +	u32 reg;
> +
> +	if (WARN_ON(pad->enable == 0))
> +		return;
> +
> +	if (--pad->enable > 0)
> +		return;
> +
> +	dev_dbg(padctl->dev, "power off USB2 tracking\n");
> +	reg = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
> +	reg |= XUSB_PADCTL_USB2_BIAS_PAD_CTL1_PD_TRK;
> +	padctl_writel(padctl, reg, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
> +
> +	clk_disable_unprepare(pad->clk);
> +}
> +
> +void tegra210_utmi_pad_power_on(struct phy *phy)
> +{
> +	struct tegra_xusb_lane *lane;
> +	struct tegra_xusb_usb2_lane *usb2;
> +	struct tegra_xusb_usb2_pad *pad;
> +	struct tegra_xusb_padctl *padctl;
> +	unsigned int index;
> +	struct device *dev;
> +	u32 reg;
> +
> +	if (!phy)
> +		return;
> +
> +	lane = phy_get_drvdata(phy);
> +	usb2 = to_usb2_lane(lane);
> +	pad = to_usb2_pad(lane->pad);
> +	padctl = lane->pad->padctl;
> +	index = lane->index;
> +	dev = padctl->dev;
> +
> +	if (usb2->powered_on)
> +		return;
> +
> +	mutex_lock(&padctl->lock);
> +
> +	dev_info(dev, "power on UTMI pads %d\n", index);

dev_dbg()? Also, index is unsigned, so you should use %u to print it.

> +
> +	tegra210_utmi_bias_pad_power_on(padctl, pad);
> +
> +	udelay(2);
> +
> +	reg = padctl_readl(padctl, XUSB_PADCTL_USB2_OTG_PADX_CTL0(index));
> +	reg &= ~XUSB_PADCTL_USB2_OTG_PAD_CTL0_PD;
> +	padctl_writel(padctl, reg, XUSB_PADCTL_USB2_OTG_PADX_CTL0(index));
> +
> +	reg = padctl_readl(padctl, XUSB_PADCTL_USB2_OTG_PADX_CTL1(index));
> +	reg &= ~XUSB_PADCTL_USB2_OTG_PAD_CTL1_PD_DR;
> +	padctl_writel(padctl, reg, XUSB_PADCTL_USB2_OTG_PADX_CTL1(index));
> +
> +	usb2->powered_on = true;
> +
> +	mutex_unlock(&padctl->lock);
> +}
> +
> +void tegra210_utmi_pad_power_down(struct phy *phy)
> +{
> +	struct tegra_xusb_lane *lane;
> +	struct tegra_xusb_usb2_lane *usb2;
> +	struct tegra_xusb_usb2_pad *pad;
> +	struct tegra_xusb_padctl *padctl;
> +	unsigned int index;
> +	struct device *dev;
> +	u32 reg;
> +
> +	if (!phy)
> +		return;
> +
> +	lane = phy_get_drvdata(phy);
> +	usb2 = to_usb2_lane(lane);
> +	pad = to_usb2_pad(lane->pad);
> +	padctl = lane->pad->padctl;
> +	index = lane->index;
> +	dev = padctl->dev;
> +
> +	if (!usb2->powered_on)
> +		return;
> +
> +	mutex_lock(&padctl->lock);
> +
> +	dev_info(dev, "power down UTMI pad %d\n", index);

Same as above.

> +
> +	reg = padctl_readl(padctl, XUSB_PADCTL_USB2_OTG_PADX_CTL0(index));
> +	reg |= XUSB_PADCTL_USB2_OTG_PAD_CTL0_PD;
> +	padctl_writel(padctl, reg, XUSB_PADCTL_USB2_OTG_PADX_CTL0(index));
> +
> +	reg = padctl_readl(padctl, XUSB_PADCTL_USB2_OTG_PADX_CTL1(index));
> +	reg |= XUSB_PADCTL_USB2_OTG_PAD_CTL1_PD_DR;
> +	padctl_writel(padctl, reg, XUSB_PADCTL_USB2_OTG_PADX_CTL1(index));
> +
> +	udelay(2);
> +
> +	tegra210_utmi_bias_pad_power_off(padctl, pad);
> +	usb2->powered_on = false;
> +
> +	mutex_unlock(&padctl->lock);
> +}

I'm not sure I understand this correctly. This seems to implement some
reference counting of its own. Why is that? Is it because it is shared
between multiple pads? But then, you're writing to indexed registers,
which indicates to me that these are in fact separate pads and not a
shared resource. So why the reference counting?

I'm also not sure why these functions need to be accessible outside this
file. They are not used outside, as far as I can tell.

>  static int tegra210_usb2_phy_init(struct phy *phy)
>  {
>  	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
> @@ -948,6 +1118,34 @@ static int tegra210_usb2_phy_power_on(struct phy *phy)
>  
>  	priv = to_tegra210_xusb_padctl(padctl);
>  
> +	if (port->usb3_port_fake != -1) {
> +		value = padctl_readl(padctl, XUSB_PADCTL_SS_PORT_MAP);
> +		value &= ~XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_MASK(
> +					port->usb3_port_fake);
> +		value |= XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP(
> +					port->usb3_port_fake, index);
> +		padctl_writel(padctl, value, XUSB_PADCTL_SS_PORT_MAP);
> +
> +		value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
> +		value &= ~XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_VCORE_DOWN(
> +					port->usb3_port_fake);
> +		padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
> +
> +		usleep_range(100, 200);
> +
> +		value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
> +		value &= ~XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN_EARLY(
> +					port->usb3_port_fake);
> +		padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
> +
> +		usleep_range(100, 200);
> +
> +		value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
> +		value &= ~XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN(
> +					port->usb3_port_fake);
> +		padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
> +	}
> +
>  	value = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL0);
>  	value &= ~((XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_SQUELCH_LEVEL_MASK <<
>  		    XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_SQUELCH_LEVEL_SHIFT) |
> @@ -965,7 +1163,14 @@ static int tegra210_usb2_phy_power_on(struct phy *phy)
>  
>  	value = padctl_readl(padctl, XUSB_PADCTL_USB2_PORT_CAP);
>  	value &= ~XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_MASK(index);
> -	value |= XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_HOST(index);
> +	if (port->port_cap == USB_PORT_DISABLED)
> +		value |= XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_DISABLED(index);
> +	else if (port->port_cap == USB_DEVICE_CAP)
> +		value |= XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_DEVICE(index);
> +	else if (port->port_cap == USB_HOST_CAP)
> +		value |= XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_HOST(index);
> +	else if (port->port_cap == USB_OTG_CAP)
> +		value |= XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_OTG(index);
>  	padctl_writel(padctl, value, XUSB_PADCTL_USB2_PORT_CAP);
>  
>  	value = padctl_readl(padctl, XUSB_PADCTL_USB2_OTG_PADX_CTL0(index));
> @@ -997,7 +1202,12 @@ static int tegra210_usb2_phy_power_on(struct phy *phy)
>  			     XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPADX_CTL1(index));
>  	value &= ~(XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_MASK <<
>  		   XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_SHIFT);
> -	value |= XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_FIX18;
> +	if (port->port_cap == USB_HOST_CAP)
> +		value |= XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_FIX18;
> +	else
> +		value |=
> +		      XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_VAL <<
> +		      XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_SHIFT;
>  	padctl_writel(padctl, value,
>  		      XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPADX_CTL1(index));
>  
> @@ -1070,6 +1280,34 @@ static int tegra210_usb2_phy_power_off(struct phy *phy)
>  
>  	mutex_lock(&padctl->lock);
>  
> +	if (port->usb3_port_fake != -1) {
> +		value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
> +		value |= XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN_EARLY(
> +					port->usb3_port_fake);
> +		padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
> +
> +		usleep_range(100, 200);
> +
> +		value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
> +		value |= XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN(
> +					port->usb3_port_fake);
> +		padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
> +
> +		usleep_range(250, 350);
> +
> +		value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
> +		value |= XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_VCORE_DOWN(
> +					port->usb3_port_fake);
> +		padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
> +
> +		value = padctl_readl(padctl, XUSB_PADCTL_SS_PORT_MAP);
> +		value &= ~XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_MASK(
> +					port->usb3_port_fake);
> +		value |= XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_MASK(
> +					port->usb3_port_fake);
> +		padctl_writel(padctl, value, XUSB_PADCTL_SS_PORT_MAP);
> +	}
> +
>  	if (WARN_ON(pad->enable == 0))
>  		goto out;
>  
> @@ -1953,6 +2191,55 @@ static const struct tegra_xusb_port_ops tegra210_usb3_port_ops = {
>  	.map = tegra210_usb3_port_map,
>  };
>  
> +static int tegra210_xusb_padctl_vbus_override(struct tegra_xusb_padctl *padctl,
> +					      bool set)
> +{
> +	u32 reg;
> +
> +	dev_dbg(padctl->dev, "%s vbus override\n", set ? "set" : "clear");
> +
> +	reg = padctl_readl(padctl, XUSB_PADCTL_USB2_VBUS_ID);
> +	if (set) {
> +		reg |= XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_VBUS_ON;
> +		reg &= ~(XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_MASK <<
> +			   XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_SHIFT);
> +		reg |= XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_VAL <<
> +			 XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_SHIFT;
> +	} else
> +		reg &= ~XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_VBUS_ON;
> +	padctl_writel(padctl, reg, XUSB_PADCTL_USB2_VBUS_ID);
> +
> +	return 0;
> +}

It'd be good to briefly explain in a comment (or the commit message)
what exactly this does and what it's used for.

> +
> +static int tegra210_utmi_port_reset_quirk(struct phy *phy)
> +{
> +	struct tegra_xusb_padctl *padctl;
> +	struct tegra_xusb_lane *lane;
> +	struct device *dev;
> +	u32 reg;
> +
> +	if (!phy)
> +		return -ENODEV;
> +
> +	lane = phy_get_drvdata(phy);
> +	padctl = lane->pad->padctl;
> +	dev = padctl->dev;
> +
> +	reg = padctl_readl(padctl,
> +				XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPADX_CTL0(0));
> +	dev_dbg(dev, "BATTERY_CHRG_OTGPADX_CTL0(0): 0x%x\n", reg);
> +
> +	if ((reg & XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL0_ZIP) ||
> +	    (reg & XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL0_ZIN)) {
> +		dev_dbg(dev, "Toggle vbus\n");
> +		tegra210_xusb_padctl_vbus_override(padctl, false);
> +		tegra210_xusb_padctl_vbus_override(padctl, true);
> +		return 1;
> +	}
> +	return 0;
> +}

This uses the VBUS override mechanism, but at the same time you also
export it to make use of it in the XUDC driver (presumably). Why have
two ways of running this code? Again, an explanation of what exactly
this is used for would clarify.

>  static int
>  tegra210_xusb_read_fuse_calibration(struct tegra210_xusb_fuse_calibration *fuse)
>  {
> @@ -2015,6 +2302,10 @@ static const struct tegra_xusb_padctl_ops tegra210_xusb_padctl_ops = {
>  	.remove = tegra210_xusb_padctl_remove,
>  	.usb3_set_lfps_detect = tegra210_usb3_set_lfps_detect,
>  	.hsic_set_idle = tegra210_hsic_set_idle,
> +	.vbus_override = tegra210_xusb_padctl_vbus_override,
> +	.utmi_pad_power_on = tegra210_utmi_pad_power_on,
> +	.utmi_pad_power_down = tegra210_utmi_pad_power_down,
> +	.utmi_port_reset_quirk = tegra210_utmi_port_reset_quirk,
>  };
>  
>  const struct tegra_xusb_padctl_soc tegra210_xusb_padctl_soc = {
> diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
> index 5b3b886..9a25b5d 100644
> --- a/drivers/phy/tegra/xusb.c
> +++ b/drivers/phy/tegra/xusb.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2014-2015, NVIDIA CORPORATION.  All rights reserved.
> + * Copyright (c) 2014-2018, NVIDIA CORPORATION.  All rights reserved.
>   *
>   * This program is free software; you can redistribute it and/or modify it
>   * under the terms and conditions of the GNU General Public License,
> @@ -546,9 +546,26 @@ static int tegra_xusb_usb2_port_parse_dt(struct tegra_xusb_usb2_port *usb2)
>  {
>  	struct tegra_xusb_port *port = &usb2->base;
>  	struct device_node *np = port->dev.of_node;
> +	const char *prop_string;
> +	u32 value;
>  
>  	usb2->internal = of_property_read_bool(np, "nvidia,internal");
>  
> +	usb2->port_cap = USB_PORT_DISABLED; /* default */
> +	if (!of_property_read_string(np, "mode", &prop_string)) {
> +		if (!strcmp("host", prop_string))
> +			usb2->port_cap = USB_HOST_CAP;
> +		else if (!strcmp("device", prop_string))
> +			usb2->port_cap = USB_DEVICE_CAP;
> +		else if (!strcmp("otg", prop_string))
> +			usb2->port_cap = USB_OTG_CAP;
> +	}

This seems to fulfill the same purpose as this:

	http://patchwork.ozlabs.org/patch/1031014/

Although, looking more closely at that, we can probably just the
usb_get_dr_mode() helper for this. There's also a standard property for
the dual-role mode, named "dr_mode", albeit the name does not seem to
match standard best practices. Seems like it would still make sense to
move to that standard property name.

> +	if (!of_property_read_u32(np, "nvidia,usb3-port-fake", &value))
> +		usb2->usb3_port_fake = value;
> +	else
> +		usb2->usb3_port_fake = -1; /* default */
> +
>  	usb2->supply = devm_regulator_get(&port->dev, "vbus");
>  	return PTR_ERR_OR_ZERO(usb2->supply);
>  }
> @@ -976,7 +993,7 @@ int tegra_xusb_padctl_usb3_save_context(struct tegra_xusb_padctl *padctl,
>  	if (padctl->soc->ops->usb3_save_context)
>  		return padctl->soc->ops->usb3_save_context(padctl, port);
>  
> -	return -ENOSYS;
> +	return -EINVAL;

Why change the error code for these? EINVAL has a pretty standard
meaning ("invalid argument") and doesn't match the error condition at
all. ENOSYS was chosen here because it's the closest there is to "not
implemented". ENOSYS is somewhat special in relation to syscalls, but
since we never propagate errors from here to userspace, it's safe to
use.

>  }
>  EXPORT_SYMBOL_GPL(tegra_xusb_padctl_usb3_save_context);
>  
> @@ -986,7 +1003,7 @@ int tegra_xusb_padctl_hsic_set_idle(struct tegra_xusb_padctl *padctl,
>  	if (padctl->soc->ops->hsic_set_idle)
>  		return padctl->soc->ops->hsic_set_idle(padctl, port, idle);
>  
> -	return -ENOSYS;
> +	return -EINVAL;
>  }
>  EXPORT_SYMBOL_GPL(tegra_xusb_padctl_hsic_set_idle);
>  
> @@ -997,10 +1014,60 @@ int tegra_xusb_padctl_usb3_set_lfps_detect(struct tegra_xusb_padctl *padctl,
>  		return padctl->soc->ops->usb3_set_lfps_detect(padctl, port,
>  							      enable);
>  
> -	return -ENOSYS;
> +	return -EINVAL;
>  }
>  EXPORT_SYMBOL_GPL(tegra_xusb_padctl_usb3_set_lfps_detect);
>  
> +int tegra_xusb_padctl_set_vbus_override(struct tegra_xusb_padctl *padctl)
> +{
> +	if (padctl->soc->ops->vbus_override)
> +		return padctl->soc->ops->vbus_override(padctl, true);
> +
> +	return -EINVAL;

It'd be best to stick to the -ENOSYS error code here.

> +void tegra_phy_xusb_utmi_pad_power_on(struct phy *phy)
> +{
> +	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
> +	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
> +
> +	if (padctl->soc->ops->utmi_pad_power_on)
> +		padctl->soc->ops->utmi_pad_power_on(phy);
> +}
> +EXPORT_SYMBOL_GPL(tegra_phy_xusb_utmi_pad_power_on);
> +
> +void tegra_phy_xusb_utmi_pad_power_down(struct phy *phy)
> +{
> +	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
> +	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
> +
> +	if (padctl->soc->ops->utmi_pad_power_down)
> +		padctl->soc->ops->utmi_pad_power_down(phy);
> +}
> +EXPORT_SYMBOL_GPL(tegra_phy_xusb_utmi_pad_power_down);

I already briefly touched on this earlier, bu why do these have to be
exported functions? Can't these simply be abstracted behind the PHY
abstraction?

> diff --git a/drivers/phy/tegra/xusb.h b/drivers/phy/tegra/xusb.h
> index b49dbc3..74acc08 100644
> --- a/drivers/phy/tegra/xusb.h
> +++ b/drivers/phy/tegra/xusb.h
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2014-2015, NVIDIA CORPORATION.  All rights reserved.
> + * Copyright (c) 2014-2018, NVIDIA CORPORATION.  All rights reserved.
>   * Copyright (c) 2015, Google Inc.
>   *
>   * This program is free software; you can redistribute it and/or modify it
> @@ -58,6 +58,7 @@ struct tegra_xusb_usb2_lane {
>  	struct tegra_xusb_lane base;
>  
>  	u32 hs_curr_level_offset;
> +	bool powered_on;
>  };
>  
>  static inline struct tegra_xusb_usb2_lane *
> @@ -267,11 +268,20 @@ struct tegra_xusb_port *
>  tegra_xusb_find_port(struct tegra_xusb_padctl *padctl, const char *type,
>  		     unsigned int index);
>  
> +enum tegra_xusb_usb_port_cap {
> +	USB_PORT_DISABLED = 0,
> +	USB_HOST_CAP,
> +	USB_DEVICE_CAP,
> +	USB_OTG_CAP,
> +};

Maybe use enum usb_dr_mode from include/linux/usb/otg.h instead of
essentially duplicating it?

> +
>  struct tegra_xusb_usb2_port {
>  	struct tegra_xusb_port base;
>  
>  	struct regulator *supply;
>  	bool internal;
> +	enum tegra_xusb_usb_port_cap port_cap;
> +	int usb3_port_fake;
>  };
>  
>  static inline struct tegra_xusb_usb2_port *
> @@ -353,6 +363,10 @@ struct tegra_xusb_padctl_ops {
>  			     unsigned int index, bool idle);
>  	int (*usb3_set_lfps_detect)(struct tegra_xusb_padctl *padctl,
>  				    unsigned int index, bool enable);
> +	int (*vbus_override)(struct tegra_xusb_padctl *padctl, bool set);
> +	void (*utmi_pad_power_on)(struct phy *phy);
> +	void (*utmi_pad_power_down)(struct phy *phy);
> +	int (*utmi_port_reset_quirk)(struct phy *phy);
>  };
>  
>  struct tegra_xusb_padctl_soc {
> diff --git a/include/linux/phy/tegra/xusb.h b/include/linux/phy/tegra/xusb.h
> index 8e1a57a..143af44 100644
> --- a/include/linux/phy/tegra/xusb.h
> +++ b/include/linux/phy/tegra/xusb.h
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2016, NVIDIA CORPORATION.  All rights reserved.
> + * Copyright (c) 2016-2018, NVIDIA CORPORATION.  All rights reserved.
>   *
>   * This program is free software; you can redistribute it and/or modify it
>   * under the terms and conditions of the GNU General Public License,
> @@ -26,5 +26,10 @@ int tegra_xusb_padctl_hsic_set_idle(struct tegra_xusb_padctl *padctl,
>  				    unsigned int port, bool idle);
>  int tegra_xusb_padctl_usb3_set_lfps_detect(struct tegra_xusb_padctl *padctl,
>  					   unsigned int port, bool enable);
> +int tegra_xusb_padctl_set_vbus_override(struct tegra_xusb_padctl *padctl);
> +int tegra_xusb_padctl_clear_vbus_override(struct tegra_xusb_padctl *padctl);
>  
> +void tegra_phy_xusb_utmi_pad_power_on(struct phy *phy);
> +void tegra_phy_xusb_utmi_pad_power_down(struct phy *phy);
> +int tegra_phy_xusb_utmi_port_reset_quirk(struct phy *phy);

We already have more custom APIs here than I'd like, so I really want to
make sure that we're not adding to these if not absolutely necessary.

Are there any reasons why we can't somehow hide these behind the
existing PHY framework?

tegra_phy_xusb_utmi_port_reset_quirk() seems like something that we may
be able to implement using the phy_ops->reset() callback. The power_on()
and power_off() for the UTMI pad seem like they should be just part of
the regular pad ->power_on() and ->power_off() callback implementations
(perhaps this means that we need to introduce another pad for this?).

Set/clear VBUS override seems like maybe something that could be hidden
behind phy_ops->configure(). This is a fairly recent addition, but
sounds like we could also push things like HSIC idle and USB3 LPFS
detection into that.

Thierry
Nagarjuna Kristam - Feb. 13, 2019, 6:47 a.m.
Hi JC,

On 25-01-2019 14:27, JC Kuo wrote:
> Hi Nagarjuna,
> 1. tegra210_utmi_bias_pad_power_off() should
> set XUSB_PADCTL_USB2_BIAS_PAD_CTL0_PD in order to really power down bias
> pad.
Will update the setting accordingly 
> 2. The XUSB_PADCTL_USB2_BIAS_PAD_CTL0 register programming
> in tegra210_usb2_phy_power_on() can be removed since you have added it
> to tegra210_utmi_bias_pad_power_on().
Will check and remove code from phy_power_on which is used in utmi pad and
bias pad power on API's 
> 3. In tegra210_usb2_phy_power_off(), there is
> + value = padctl_readl(padctl, XUSB_PADCTL_SS_PORT_MAP);
> + value &= ~XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_MASK(
> + port->usb3_port_fake);
> + value |= XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_MASK(
> + port->usb3_port_fake);
> + padctl_writel(padctl, value, XUSB_PADCTL_SS_PORT_MAP);
> I think it should be
> + value = padctl_readl(padctl, XUSB_PADCTL_SS_PORT_MAP);
> + value &= ~XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_MASK(
> + port->usb3_port_fake);
> + value |= XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP(port->usb3_port_fake,
> +                           XUSB_PADCTL_SS_PORT_MAP_PORT_DISABLED );
> + padctl_writel(padctl, value, XUSB_PADCTL_SS_PORT_MAP);
> 
Yes, suggest change is right setting for disabled, will do the same.
> 4. XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_VAL should be renamed
> with XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_FLOATING . Another possible ID pin
> state is "grounded" should be defined
> XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_GROUNDED with value 0x0.
> 
Will do
> Thanks,
> JC
> 
> 
> On Thu, Jan 3, 2019 at 7:43 PM Nagarjuna Kristam <nkristam@nvidia.com>
> wrote:
> 
>> Add support for XUSB device mode controller on Tegra210.
>> Update PADCTL driver to set port cap based on DT config.
>> Add code to handle property "nvidia,usb3-port-fake"
>> Provide API's to control vbus override and utmi pad power control
>>
>> Signed-off-by: Nagarjuna Kristam <nkristam@nvidia.com>
>> ---
>>  drivers/phy/tegra/xusb-tegra210.c | 297
>> +++++++++++++++++++++++++++++++++++++-
>>  drivers/phy/tegra/xusb.c          |  75 +++++++++-
>>  drivers/phy/tegra/xusb.h          |  16 +-
>>  include/linux/phy/tegra/xusb.h    |   7 +-
>>  4 files changed, 386 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/phy/tegra/xusb-tegra210.c
>> b/drivers/phy/tegra/xusb-tegra210.c
>> index 05bee32..0289e10 100644
>> --- a/drivers/phy/tegra/xusb-tegra210.c
>> +++ b/drivers/phy/tegra/xusb-tegra210.c
>> @@ -1,5 +1,5 @@
>>  /*
>> - * Copyright (c) 2014, NVIDIA CORPORATION.  All rights reserved.
>> + * Copyright (c) 2014-2018, NVIDIA CORPORATION.  All rights reserved.
>>   * Copyright (C) 2015 Google, Inc.
>>   *
>>   * This program is free software; you can redistribute it and/or modify it
>> @@ -47,7 +47,10 @@
>>  #define XUSB_PADCTL_USB2_PAD_MUX_USB2_BIAS_PAD_XUSB 0x1
>>
>>  #define XUSB_PADCTL_USB2_PORT_CAP 0x008
>> +#define XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_DISABLED(x) (0x0 << ((x) * 4))
>>  #define XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_HOST(x) (0x1 << ((x) * 4))
>> +#define XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_DEVICE(x) (0x2 << ((x) * 4))
>> +#define XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_OTG(x) (0x3 << ((x) * 4))
>>  #define XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_MASK(x) (0x3 << ((x) * 4))
>>
>>  #define XUSB_PADCTL_SS_PORT_MAP 0x014
>> @@ -55,6 +58,7 @@
>>  #define XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_SHIFT(x) ((x) * 5)
>>  #define XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_MASK(x) (0x7 << ((x) * 5))
>>  #define XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP(x, v) (((v) & 0x7) << ((x) * 5))
>> +#define XUSB_PADCTL_SS_PORT_MAP_PORT_DISABLED 0x7
>>
>>  #define XUSB_PADCTL_ELPG_PROGRAM1 0x024
>>  #define XUSB_PADCTL_ELPG_PROGRAM1_AUX_MUX_LP0_VCORE_DOWN (1 << 31)
>> @@ -69,9 +73,14 @@
>>  #define XUSB_PADCTL_USB3_PAD_MUX_PCIE_IDDQ_DISABLE(x) (1 << (1 + (x)))
>>  #define XUSB_PADCTL_USB3_PAD_MUX_SATA_IDDQ_DISABLE(x) (1 << (8 + (x)))
>>
>> +#define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPADX_CTL0(x) (0x080 + (x) * 0x40)
>> +#define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL0_ZIP (1 << 18)
>> +#define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL0_ZIN (1 << 22)
>> +
>>  #define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPADX_CTL1(x) (0x084 + (x) * 0x40)
>>  #define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_SHIFT 7
>>  #define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_MASK 0x3
>> +#define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_VAL 0x1
>>  #define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_FIX18 (1 << 6)
>>
>>  #define XUSB_PADCTL_USB2_OTG_PADX_CTL0(x) (0x088 + (x) * 0x40)
>> @@ -230,6 +239,12 @@
>>  #define XUSB_PADCTL_UPHY_USB3_PADX_ECTL6(x) (0xa74 + (x) * 0x40)
>>  #define XUSB_PADCTL_UPHY_USB3_PAD_ECTL6_RX_EQ_CTRL_H_VAL 0xfcf01368
>>
>> +#define XUSB_PADCTL_USB2_VBUS_ID 0xc60
>> +#define XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_VBUS_ON (1 << 14)
>> +#define XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_SHIFT 18
>> +#define XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_MASK 0xf
>> +#define XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_VAL 8
>> +
>>  struct tegra210_xusb_fuse_calibration {
>>         u32 hs_curr_level[4];
>>         u32 hs_term_range_adj;
>> @@ -905,6 +920,161 @@ static const struct tegra_xusb_lane_ops
>> tegra210_usb2_lane_ops = {
>>         .remove = tegra210_usb2_lane_remove,
>>  };
>>
>> +/* must be called under padctl->lock */
>> +static void tegra210_utmi_bias_pad_power_on(struct tegra_xusb_padctl
>> *padctl,
>> +                               struct tegra_xusb_usb2_pad *pad)
>> +{
>> +       u32 reg;
>> +
>> +       if (pad->enable++ > 0)
>> +               return;
>> +
>> +       dev_dbg(padctl->dev, "power on BIAS PAD & USB2 tracking\n");
>> +
>> +       if (clk_prepare_enable(pad->clk))
>> +               dev_warn(padctl->dev, "failed to enable BIAS PAD & USB2
>> tracking\n");
>> +
>> +       reg = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
>> +
>> +       reg &= ~((XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_START_TIMER_MASK <<
>> +                   XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_START_TIMER_SHIFT) |
>> +
>> (XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_DONE_RESET_TIMER_MASK <<
>> +
>>  XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_DONE_RESET_TIMER_SHIFT));
>> +       reg |= (XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_START_TIMER_VAL <<
>> +                 XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_START_TIMER_SHIFT) |
>> +                (XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_DONE_RESET_TIMER_VAL
>> <<
>> +
>>  XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_DONE_RESET_TIMER_SHIFT);
>> +       padctl_writel(padctl, reg, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
>> +
>> +       reg = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL0);
>> +       reg &= ~XUSB_PADCTL_USB2_BIAS_PAD_CTL0_PD;
>> +
>> +       reg &= ~((XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_SQUELCH_LEVEL_MASK <<
>> +                   XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_SQUELCH_LEVEL_SHIFT)
>> |
>> +                  (XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_DISCON_LEVEL_MASK <<
>> +                   XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_DISCON_LEVEL_SHIFT));
>> +       reg |= (XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_DISCON_LEVEL_VAL <<
>> +                 XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_DISCON_LEVEL_SHIFT);
>> +
>> +       if (tegra_sku_info.revision < TEGRA_REVISION_A02)
>> +               reg |=
>> (XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_SQUELCH_LEVEL_VAL <<
>> +
>>  XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_SQUELCH_LEVEL_SHIFT);
>> +       padctl_writel(padctl, reg, XUSB_PADCTL_USB2_BIAS_PAD_CTL0);
>> +
>> +       udelay(1);
>> +
>> +       reg = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
>> +       reg &= ~XUSB_PADCTL_USB2_BIAS_PAD_CTL1_PD_TRK;
>> +       padctl_writel(padctl, reg, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
>> +}
>> +
>> +/* must be called under padctl->lock */
>> +static void tegra210_utmi_bias_pad_power_off(struct tegra_xusb_padctl
>> *padctl,
>> +                               struct tegra_xusb_usb2_pad *pad)
>> +{
>> +       u32 reg;
>> +
>> +       if (WARN_ON(pad->enable == 0))
>> +               return;
>> +
>> +       if (--pad->enable > 0)
>> +               return;
>> +
>> +       dev_dbg(padctl->dev, "power off USB2 tracking\n");
>> +       reg = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
>> +       reg |= XUSB_PADCTL_USB2_BIAS_PAD_CTL1_PD_TRK;
>> +       padctl_writel(padctl, reg, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
>> +
>> +       clk_disable_unprepare(pad->clk);
>> +}
>> +
>> +void tegra210_utmi_pad_power_on(struct phy *phy)
>> +{
>> +       struct tegra_xusb_lane *lane;
>> +       struct tegra_xusb_usb2_lane *usb2;
>> +       struct tegra_xusb_usb2_pad *pad;
>> +       struct tegra_xusb_padctl *padctl;
>> +       unsigned int index;
>> +       struct device *dev;
>> +       u32 reg;
>> +
>> +       if (!phy)
>> +               return;
>> +
>> +       lane = phy_get_drvdata(phy);
>> +       usb2 = to_usb2_lane(lane);
>> +       pad = to_usb2_pad(lane->pad);
>> +       padctl = lane->pad->padctl;
>> +       index = lane->index;
>> +       dev = padctl->dev;
>> +
>> +       if (usb2->powered_on)
>> +               return;
>> +
>> +       mutex_lock(&padctl->lock);
>> +
>> +       dev_info(dev, "power on UTMI pads %d\n", index);
>> +
>> +       tegra210_utmi_bias_pad_power_on(padctl, pad);
>> +
>> +       udelay(2);
>> +
>> +       reg = padctl_readl(padctl, XUSB_PADCTL_USB2_OTG_PADX_CTL0(index));
>> +       reg &= ~XUSB_PADCTL_USB2_OTG_PAD_CTL0_PD;
>> +       padctl_writel(padctl, reg, XUSB_PADCTL_USB2_OTG_PADX_CTL0(index));
>> +
>> +       reg = padctl_readl(padctl, XUSB_PADCTL_USB2_OTG_PADX_CTL1(index));
>> +       reg &= ~XUSB_PADCTL_USB2_OTG_PAD_CTL1_PD_DR;
>> +       padctl_writel(padctl, reg, XUSB_PADCTL_USB2_OTG_PADX_CTL1(index));
>> +
>> +       usb2->powered_on = true;
>> +
>> +       mutex_unlock(&padctl->lock);
>> +}
>> +
>> +void tegra210_utmi_pad_power_down(struct phy *phy)
>> +{
>> +       struct tegra_xusb_lane *lane;
>> +       struct tegra_xusb_usb2_lane *usb2;
>> +       struct tegra_xusb_usb2_pad *pad;
>> +       struct tegra_xusb_padctl *padctl;
>> +       unsigned int index;
>> +       struct device *dev;
>> +       u32 reg;
>> +
>> +       if (!phy)
>> +               return;
>> +
>> +       lane = phy_get_drvdata(phy);
>> +       usb2 = to_usb2_lane(lane);
>> +       pad = to_usb2_pad(lane->pad);
>> +       padctl = lane->pad->padctl;
>> +       index = lane->index;
>> +       dev = padctl->dev;
>> +
>> +       if (!usb2->powered_on)
>> +               return;
>> +
>> +       mutex_lock(&padctl->lock);
>> +
>> +       dev_info(dev, "power down UTMI pad %d\n", index);
>> +
>> +       reg = padctl_readl(padctl, XUSB_PADCTL_USB2_OTG_PADX_CTL0(index));
>> +       reg |= XUSB_PADCTL_USB2_OTG_PAD_CTL0_PD;
>> +       padctl_writel(padctl, reg, XUSB_PADCTL_USB2_OTG_PADX_CTL0(index));
>> +
>> +       reg = padctl_readl(padctl, XUSB_PADCTL_USB2_OTG_PADX_CTL1(index));
>> +       reg |= XUSB_PADCTL_USB2_OTG_PAD_CTL1_PD_DR;
>> +       padctl_writel(padctl, reg, XUSB_PADCTL_USB2_OTG_PADX_CTL1(index));
>> +
>> +       udelay(2);
>> +
>> +       tegra210_utmi_bias_pad_power_off(padctl, pad);
>> +       usb2->powered_on = false;
>> +
>> +       mutex_unlock(&padctl->lock);
>> +}
>> +
>>  static int tegra210_usb2_phy_init(struct phy *phy)
>>  {
>>         struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
>> @@ -948,6 +1118,34 @@ static int tegra210_usb2_phy_power_on(struct phy
>> *phy)
>>
>>         priv = to_tegra210_xusb_padctl(padctl);
>>
>> +       if (port->usb3_port_fake != -1) {
>> +               value = padctl_readl(padctl, XUSB_PADCTL_SS_PORT_MAP);
>> +               value &= ~XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_MASK(
>> +                                       port->usb3_port_fake);
>> +               value |= XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP(
>> +                                       port->usb3_port_fake, index);
>> +               padctl_writel(padctl, value, XUSB_PADCTL_SS_PORT_MAP);
>> +
>> +               value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
>> +               value &= ~XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_VCORE_DOWN(
>> +                                       port->usb3_port_fake);
>> +               padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
>> +
>> +               usleep_range(100, 200);
>> +
>> +               value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
>> +               value &=
>> ~XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN_EARLY(
>> +                                       port->usb3_port_fake);
>> +               padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
>> +
>> +               usleep_range(100, 200);
>> +
>> +               value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
>> +               value &= ~XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN(
>> +                                       port->usb3_port_fake);
>> +               padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
>> +       }
>> +
>>         value = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL0);
>>         value &= ~((XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_SQUELCH_LEVEL_MASK <<
>>                     XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_SQUELCH_LEVEL_SHIFT)
>> |
>> @@ -965,7 +1163,14 @@ static int tegra210_usb2_phy_power_on(struct phy
>> *phy)
>>
>>         value = padctl_readl(padctl, XUSB_PADCTL_USB2_PORT_CAP);
>>         value &= ~XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_MASK(index);
>> -       value |= XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_HOST(index);
>> +       if (port->port_cap == USB_PORT_DISABLED)
>> +               value |=
>> XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_DISABLED(index);
>> +       else if (port->port_cap == USB_DEVICE_CAP)
>> +               value |= XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_DEVICE(index);
>> +       else if (port->port_cap == USB_HOST_CAP)
>> +               value |= XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_HOST(index);
>> +       else if (port->port_cap == USB_OTG_CAP)
>> +               value |= XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_OTG(index);
>>         padctl_writel(padctl, value, XUSB_PADCTL_USB2_PORT_CAP);
>>
>>         value = padctl_readl(padctl,
>> XUSB_PADCTL_USB2_OTG_PADX_CTL0(index));
>> @@ -997,7 +1202,12 @@ static int tegra210_usb2_phy_power_on(struct phy
>> *phy)
>>
>>  XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPADX_CTL1(index));
>>         value &= ~(XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_MASK
>> <<
>>
>>  XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_SHIFT);
>> -       value |= XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_FIX18;
>> +       if (port->port_cap == USB_HOST_CAP)
>> +               value |=
>> XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_FIX18;
>> +       else
>> +               value |=
>> +
>>  XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_VAL <<
>> +
>>  XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_SHIFT;
>>         padctl_writel(padctl, value,
>>                       XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPADX_CTL1(index));
>>
>> @@ -1070,6 +1280,34 @@ static int tegra210_usb2_phy_power_off(struct phy
>> *phy)
>>
>>         mutex_lock(&padctl->lock);
>>
>> +       if (port->usb3_port_fake != -1) {
>> +               value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
>> +               value |=
>> XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN_EARLY(
>> +                                       port->usb3_port_fake);
>> +               padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
>> +
>> +               usleep_range(100, 200);
>> +
>> +               value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
>> +               value |= XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN(
>> +                                       port->usb3_port_fake);
>> +               padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
>> +
>> +               usleep_range(250, 350);
>> +
>> +               value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
>> +               value |= XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_VCORE_DOWN(
>> +                                       port->usb3_port_fake);
>> +               padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
>> +
>> +               value = padctl_readl(padctl, XUSB_PADCTL_SS_PORT_MAP);
>> +               value &= ~XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_MASK(
>> +                                       port->usb3_port_fake);
>> +               value |= XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_MASK(
>> +                                       port->usb3_port_fake);
>> +               padctl_writel(padctl, value, XUSB_PADCTL_SS_PORT_MAP);
>> +       }
>> +
>>         if (WARN_ON(pad->enable == 0))
>>                 goto out;
>>
>> @@ -1953,6 +2191,55 @@ static const struct tegra_xusb_port_ops
>> tegra210_usb3_port_ops = {
>>         .map = tegra210_usb3_port_map,
>>  };
>>
>> +static int tegra210_xusb_padctl_vbus_override(struct tegra_xusb_padctl
>> *padctl,
>> +                                             bool set)
>> +{
>> +       u32 reg;
>> +
>> +       dev_dbg(padctl->dev, "%s vbus override\n", set ? "set" : "clear");
>> +
>> +       reg = padctl_readl(padctl, XUSB_PADCTL_USB2_VBUS_ID);
>> +       if (set) {
>> +               reg |= XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_VBUS_ON;
>> +               reg &= ~(XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_MASK <<
>> +                          XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_SHIFT);
>> +               reg |= XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_VAL <<
>> +                        XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_SHIFT;
>> +       } else
>> +               reg &= ~XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_VBUS_ON;
>> +       padctl_writel(padctl, reg, XUSB_PADCTL_USB2_VBUS_ID);
>> +
>> +       return 0;
>> +}
>> +
>> +static int tegra210_utmi_port_reset_quirk(struct phy *phy)
>> +{
>> +       struct tegra_xusb_padctl *padctl;
>> +       struct tegra_xusb_lane *lane;
>> +       struct device *dev;
>> +       u32 reg;
>> +
>> +       if (!phy)
>> +               return -ENODEV;
>> +
>> +       lane = phy_get_drvdata(phy);
>> +       padctl = lane->pad->padctl;
>> +       dev = padctl->dev;
>> +
>> +       reg = padctl_readl(padctl,
>> +
>>  XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPADX_CTL0(0));
>> +       dev_dbg(dev, "BATTERY_CHRG_OTGPADX_CTL0(0): 0x%x\n", reg);
>> +
>> +       if ((reg & XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL0_ZIP) ||
>> +           (reg & XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL0_ZIN)) {
>> +               dev_dbg(dev, "Toggle vbus\n");
>> +               tegra210_xusb_padctl_vbus_override(padctl, false);
>> +               tegra210_xusb_padctl_vbus_override(padctl, true);
>> +               return 1;
>> +       }
>> +       return 0;
>> +}
>> +
>>  static int
>>  tegra210_xusb_read_fuse_calibration(struct tegra210_xusb_fuse_calibration
>> *fuse)
>>  {
>> @@ -2015,6 +2302,10 @@ static const struct tegra_xusb_padctl_ops
>> tegra210_xusb_padctl_ops = {
>>         .remove = tegra210_xusb_padctl_remove,
>>         .usb3_set_lfps_detect = tegra210_usb3_set_lfps_detect,
>>         .hsic_set_idle = tegra210_hsic_set_idle,
>> +       .vbus_override = tegra210_xusb_padctl_vbus_override,
>> +       .utmi_pad_power_on = tegra210_utmi_pad_power_on,
>> +       .utmi_pad_power_down = tegra210_utmi_pad_power_down,
>> +       .utmi_port_reset_quirk = tegra210_utmi_port_reset_quirk,
>>  };
>>
>>  const struct tegra_xusb_padctl_soc tegra210_xusb_padctl_soc = {
>> diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
>> index 5b3b886..9a25b5d 100644
>> --- a/drivers/phy/tegra/xusb.c
>> +++ b/drivers/phy/tegra/xusb.c
>> @@ -1,5 +1,5 @@
>>  /*
>> - * Copyright (c) 2014-2015, NVIDIA CORPORATION.  All rights reserved.
>> + * Copyright (c) 2014-2018, NVIDIA CORPORATION.  All rights reserved.
>>   *
>>   * This program is free software; you can redistribute it and/or modify it
>>   * under the terms and conditions of the GNU General Public License,
>> @@ -546,9 +546,26 @@ static int tegra_xusb_usb2_port_parse_dt(struct
>> tegra_xusb_usb2_port *usb2)
>>  {
>>         struct tegra_xusb_port *port = &usb2->base;
>>         struct device_node *np = port->dev.of_node;
>> +       const char *prop_string;
>> +       u32 value;
>>
>>         usb2->internal = of_property_read_bool(np, "nvidia,internal");
>>
>> +       usb2->port_cap = USB_PORT_DISABLED; /* default */
>> +       if (!of_property_read_string(np, "mode", &prop_string)) {
>> +               if (!strcmp("host", prop_string))
>> +                       usb2->port_cap = USB_HOST_CAP;
>> +               else if (!strcmp("device", prop_string))
>> +                       usb2->port_cap = USB_DEVICE_CAP;
>> +               else if (!strcmp("otg", prop_string))
>> +                       usb2->port_cap = USB_OTG_CAP;
>> +       }
>> +
>> +       if (!of_property_read_u32(np, "nvidia,usb3-port-fake", &value))
>> +               usb2->usb3_port_fake = value;
>> +       else
>> +               usb2->usb3_port_fake = -1; /* default */
>> +
>>         usb2->supply = devm_regulator_get(&port->dev, "vbus");
>>         return PTR_ERR_OR_ZERO(usb2->supply);
>>  }
>> @@ -976,7 +993,7 @@ int tegra_xusb_padctl_usb3_save_context(struct
>> tegra_xusb_padctl *padctl,
>>         if (padctl->soc->ops->usb3_save_context)
>>                 return padctl->soc->ops->usb3_save_context(padctl, port);
>>
>> -       return -ENOSYS;
>> +       return -EINVAL;
>>  }
>>  EXPORT_SYMBOL_GPL(tegra_xusb_padctl_usb3_save_context);
>>
>> @@ -986,7 +1003,7 @@ int tegra_xusb_padctl_hsic_set_idle(struct
>> tegra_xusb_padctl *padctl,
>>         if (padctl->soc->ops->hsic_set_idle)
>>                 return padctl->soc->ops->hsic_set_idle(padctl, port, idle);
>>
>> -       return -ENOSYS;
>> +       return -EINVAL;
>>  }
>>  EXPORT_SYMBOL_GPL(tegra_xusb_padctl_hsic_set_idle);
>>
>> @@ -997,10 +1014,60 @@ int tegra_xusb_padctl_usb3_set_lfps_detect(struct
>> tegra_xusb_padctl *padctl,
>>                 return padctl->soc->ops->usb3_set_lfps_detect(padctl, port,
>>                                                               enable);
>>
>> -       return -ENOSYS;
>> +       return -EINVAL;
>>  }
>>  EXPORT_SYMBOL_GPL(tegra_xusb_padctl_usb3_set_lfps_detect);
>>
>> +int tegra_xusb_padctl_set_vbus_override(struct tegra_xusb_padctl *padctl)
>> +{
>> +       if (padctl->soc->ops->vbus_override)
>> +               return padctl->soc->ops->vbus_override(padctl, true);
>> +
>> +       return -EINVAL;
>> +}
>> +EXPORT_SYMBOL_GPL(tegra_xusb_padctl_set_vbus_override);
>> +
>> +int tegra_xusb_padctl_clear_vbus_override(struct tegra_xusb_padctl
>> *padctl)
>> +{
>> +       if (padctl->soc->ops->vbus_override)
>> +               return padctl->soc->ops->vbus_override(padctl, false);
>> +
>> +       return -EINVAL;
>> +}
>> +EXPORT_SYMBOL_GPL(tegra_xusb_padctl_clear_vbus_override);
>> +
>> +void tegra_phy_xusb_utmi_pad_power_on(struct phy *phy)
>> +{
>> +       struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
>> +       struct tegra_xusb_padctl *padctl = lane->pad->padctl;
>> +
>> +       if (padctl->soc->ops->utmi_pad_power_on)
>> +               padctl->soc->ops->utmi_pad_power_on(phy);
>> +}
>> +EXPORT_SYMBOL_GPL(tegra_phy_xusb_utmi_pad_power_on);
>> +
>> +void tegra_phy_xusb_utmi_pad_power_down(struct phy *phy)
>> +{
>> +       struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
>> +       struct tegra_xusb_padctl *padctl = lane->pad->padctl;
>> +
>> +       if (padctl->soc->ops->utmi_pad_power_down)
>> +               padctl->soc->ops->utmi_pad_power_down(phy);
>> +}
>> +EXPORT_SYMBOL_GPL(tegra_phy_xusb_utmi_pad_power_down);
>> +
>> +int tegra_phy_xusb_utmi_port_reset_quirk(struct phy *phy)
>> +{
>> +       struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
>> +       struct tegra_xusb_padctl *padctl = lane->pad->padctl;
>> +
>> +       if (padctl->soc->ops->utmi_port_reset_quirk)
>> +               return padctl->soc->ops->utmi_port_reset_quirk(phy);
>> +
>> +       return -EINVAL;
>> +}
>> +EXPORT_SYMBOL_GPL(tegra_phy_xusb_utmi_port_reset_quirk);
>> +
>>  MODULE_AUTHOR("Thierry Reding <treding@nvidia.com>");
>>  MODULE_DESCRIPTION("Tegra XUSB Pad Controller driver");
>>  MODULE_LICENSE("GPL v2");
>> diff --git a/drivers/phy/tegra/xusb.h b/drivers/phy/tegra/xusb.h
>> index b49dbc3..74acc08 100644
>> --- a/drivers/phy/tegra/xusb.h
>> +++ b/drivers/phy/tegra/xusb.h
>> @@ -1,5 +1,5 @@
>>  /*
>> - * Copyright (c) 2014-2015, NVIDIA CORPORATION.  All rights reserved.
>> + * Copyright (c) 2014-2018, NVIDIA CORPORATION.  All rights reserved.
>>   * Copyright (c) 2015, Google Inc.
>>   *
>>   * This program is free software; you can redistribute it and/or modify it
>> @@ -58,6 +58,7 @@ struct tegra_xusb_usb2_lane {
>>         struct tegra_xusb_lane base;
>>
>>         u32 hs_curr_level_offset;
>> +       bool powered_on;
>>  };
>>
>>  static inline struct tegra_xusb_usb2_lane *
>> @@ -267,11 +268,20 @@ struct tegra_xusb_port *
>>  tegra_xusb_find_port(struct tegra_xusb_padctl *padctl, const char *type,
>>                      unsigned int index);
>>
>> +enum tegra_xusb_usb_port_cap {
>> +       USB_PORT_DISABLED = 0,
>> +       USB_HOST_CAP,
>> +       USB_DEVICE_CAP,
>> +       USB_OTG_CAP,
>> +};
>> +
>>  struct tegra_xusb_usb2_port {
>>         struct tegra_xusb_port base;
>>
>>         struct regulator *supply;
>>         bool internal;
>> +       enum tegra_xusb_usb_port_cap port_cap;
>> +       int usb3_port_fake;
>>  };
>>
>>  static inline struct tegra_xusb_usb2_port *
>> @@ -353,6 +363,10 @@ struct tegra_xusb_padctl_ops {
>>                              unsigned int index, bool idle);
>>         int (*usb3_set_lfps_detect)(struct tegra_xusb_padctl *padctl,
>>                                     unsigned int index, bool enable);
>> +       int (*vbus_override)(struct tegra_xusb_padctl *padctl, bool set);
>> +       void (*utmi_pad_power_on)(struct phy *phy);
>> +       void (*utmi_pad_power_down)(struct phy *phy);
>> +       int (*utmi_port_reset_quirk)(struct phy *phy);
>>  };
>>
>>  struct tegra_xusb_padctl_soc {
>> diff --git a/include/linux/phy/tegra/xusb.h
>> b/include/linux/phy/tegra/xusb.h
>> index 8e1a57a..143af44 100644
>> --- a/include/linux/phy/tegra/xusb.h
>> +++ b/include/linux/phy/tegra/xusb.h
>> @@ -1,5 +1,5 @@
>>  /*
>> - * Copyright (c) 2016, NVIDIA CORPORATION.  All rights reserved.
>> + * Copyright (c) 2016-2018, NVIDIA CORPORATION.  All rights reserved.
>>   *
>>   * This program is free software; you can redistribute it and/or modify it
>>   * under the terms and conditions of the GNU General Public License,
>> @@ -26,5 +26,10 @@ int tegra_xusb_padctl_hsic_set_idle(struct
>> tegra_xusb_padctl *padctl,
>>                                     unsigned int port, bool idle);
>>  int tegra_xusb_padctl_usb3_set_lfps_detect(struct tegra_xusb_padctl
>> *padctl,
>>                                            unsigned int port, bool enable);
>> +int tegra_xusb_padctl_set_vbus_override(struct tegra_xusb_padctl *padctl);
>> +int tegra_xusb_padctl_clear_vbus_override(struct tegra_xusb_padctl
>> *padctl);
>>
>> +void tegra_phy_xusb_utmi_pad_power_on(struct phy *phy);
>> +void tegra_phy_xusb_utmi_pad_power_down(struct phy *phy);
>> +int tegra_phy_xusb_utmi_port_reset_quirk(struct phy *phy);
>>  #endif /* PHY_TEGRA_XUSB_H */
>> --
>> 2.7.4
>>
>>
>
Nagarjuna Kristam - Feb. 13, 2019, 7:04 a.m.
Hi Thierry,

On 04-02-2019 17:57, Thierry Reding wrote:
> On Thu, Jan 03, 2019 at 03:34:52PM +0530, Nagarjuna Kristam wrote:
>> Add support for XUSB device mode controller on Tegra210.
>> Update PADCTL driver to set port cap based on DT config.
>> Add code to handle property "nvidia,usb3-port-fake"
>> Provide API's to control vbus override and utmi pad power control
> 
> You essentially list three features additions here, so it makes sense to
> provide each of them in a separate patch. That helps reviewers
> concentrate on one feature at a time without having to load context for
> three different things in parallel.
> 
> Also, for each feature please provide a bit more background information
> as well as reasons for why they are added.
> 

For XUSB device control Feature support, we needed all three and hence
added them as part of one change. If we can have group of individuals CL's
for providing one feature support, will split them accordingly.
Doing above will mark current changes as a different patch series, considering
new patch series will be made, is it fine to move USB gadget driver changes to
a new series ?
Please share your inputs

>> Signed-off-by: Nagarjuna Kristam <nkristam@nvidia.com>
>> ---
>>  drivers/phy/tegra/xusb-tegra210.c | 297 +++++++++++++++++++++++++++++++++++++-
>>  drivers/phy/tegra/xusb.c          |  75 +++++++++-
>>  drivers/phy/tegra/xusb.h          |  16 +-
>>  include/linux/phy/tegra/xusb.h    |   7 +-
>>  4 files changed, 386 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/phy/tegra/xusb-tegra210.c b/drivers/phy/tegra/xusb-tegra210.c
>> index 05bee32..0289e10 100644
>> --- a/drivers/phy/tegra/xusb-tegra210.c
>> +++ b/drivers/phy/tegra/xusb-tegra210.c
>> @@ -1,5 +1,5 @@
>>  /*
>> - * Copyright (c) 2014, NVIDIA CORPORATION.  All rights reserved.
>> + * Copyright (c) 2014-2018, NVIDIA CORPORATION.  All rights reserved.
>>   * Copyright (C) 2015 Google, Inc.
>>   *
>>   * This program is free software; you can redistribute it and/or modify it
>> @@ -47,7 +47,10 @@
>>  #define XUSB_PADCTL_USB2_PAD_MUX_USB2_BIAS_PAD_XUSB 0x1
>>  
>>  #define XUSB_PADCTL_USB2_PORT_CAP 0x008
>> +#define XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_DISABLED(x) (0x0 << ((x) * 4))
>>  #define XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_HOST(x) (0x1 << ((x) * 4))
>> +#define XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_DEVICE(x) (0x2 << ((x) * 4))
>> +#define XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_OTG(x) (0x3 << ((x) * 4))
>>  #define XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_MASK(x) (0x3 << ((x) * 4))
>>  
>>  #define XUSB_PADCTL_SS_PORT_MAP 0x014
>> @@ -55,6 +58,7 @@
>>  #define XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_SHIFT(x) ((x) * 5)
>>  #define XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_MASK(x) (0x7 << ((x) * 5))
>>  #define XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP(x, v) (((v) & 0x7) << ((x) * 5))
>> +#define XUSB_PADCTL_SS_PORT_MAP_PORT_DISABLED 0x7
>>  
>>  #define XUSB_PADCTL_ELPG_PROGRAM1 0x024
>>  #define XUSB_PADCTL_ELPG_PROGRAM1_AUX_MUX_LP0_VCORE_DOWN (1 << 31)
>> @@ -69,9 +73,14 @@
>>  #define XUSB_PADCTL_USB3_PAD_MUX_PCIE_IDDQ_DISABLE(x) (1 << (1 + (x)))
>>  #define XUSB_PADCTL_USB3_PAD_MUX_SATA_IDDQ_DISABLE(x) (1 << (8 + (x)))
>>  
>> +#define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPADX_CTL0(x) (0x080 + (x) * 0x40)
>> +#define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL0_ZIP (1 << 18)
>> +#define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL0_ZIN (1 << 22)
>> +
>>  #define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPADX_CTL1(x) (0x084 + (x) * 0x40)
>>  #define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_SHIFT 7
>>  #define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_MASK 0x3
>> +#define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_VAL 0x1
>>  #define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_FIX18 (1 << 6)
>>  
>>  #define XUSB_PADCTL_USB2_OTG_PADX_CTL0(x) (0x088 + (x) * 0x40)
>> @@ -230,6 +239,12 @@
>>  #define XUSB_PADCTL_UPHY_USB3_PADX_ECTL6(x) (0xa74 + (x) * 0x40)
>>  #define XUSB_PADCTL_UPHY_USB3_PAD_ECTL6_RX_EQ_CTRL_H_VAL 0xfcf01368
>>  
>> +#define XUSB_PADCTL_USB2_VBUS_ID 0xc60
>> +#define XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_VBUS_ON (1 << 14)
>> +#define XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_SHIFT 18
>> +#define XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_MASK 0xf
>> +#define XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_VAL 8
>> +
>>  struct tegra210_xusb_fuse_calibration {
>>  	u32 hs_curr_level[4];
>>  	u32 hs_term_range_adj;
>> @@ -905,6 +920,161 @@ static const struct tegra_xusb_lane_ops tegra210_usb2_lane_ops = {
>>  	.remove = tegra210_usb2_lane_remove,
>>  };
>>  
>> +/* must be called under padctl->lock */
>> +static void tegra210_utmi_bias_pad_power_on(struct tegra_xusb_padctl *padctl,
>> +				struct tegra_xusb_usb2_pad *pad)
>> +{
>> +	u32 reg;
>> +
>> +	if (pad->enable++ > 0)
>> +		return;
>> +
>> +	dev_dbg(padctl->dev, "power on BIAS PAD & USB2 tracking\n");
>> +
>> +	if (clk_prepare_enable(pad->clk))
>> +		dev_warn(padctl->dev, "failed to enable BIAS PAD & USB2 tracking\n");
> 
> Maybe keep track of the exact error and make that part of the error
> message?
> 

Will do

>> +
>> +	reg = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
>> +
>> +	reg &= ~((XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_START_TIMER_MASK <<
>> +		    XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_START_TIMER_SHIFT) |
>> +		   (XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_DONE_RESET_TIMER_MASK <<
>> +		    XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_DONE_RESET_TIMER_SHIFT));
>> +	reg |= (XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_START_TIMER_VAL <<
>> +		  XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_START_TIMER_SHIFT) |
>> +		 (XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_DONE_RESET_TIMER_VAL <<
>> +		  XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_DONE_RESET_TIMER_SHIFT);
>> +	padctl_writel(padctl, reg, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
>> +
>> +	reg = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL0);
>> +	reg &= ~XUSB_PADCTL_USB2_BIAS_PAD_CTL0_PD;
>> +
>> +	reg &= ~((XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_SQUELCH_LEVEL_MASK <<
>> +		    XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_SQUELCH_LEVEL_SHIFT) |
>> +		   (XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_DISCON_LEVEL_MASK <<
>> +		    XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_DISCON_LEVEL_SHIFT));
>> +	reg |= (XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_DISCON_LEVEL_VAL <<
>> +		  XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_DISCON_LEVEL_SHIFT);
>> +
>> +	if (tegra_sku_info.revision < TEGRA_REVISION_A02)
>> +		reg |= (XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_SQUELCH_LEVEL_VAL <<
>> +			XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_SQUELCH_LEVEL_SHIFT);
>> +	padctl_writel(padctl, reg, XUSB_PADCTL_USB2_BIAS_PAD_CTL0);
>> +
>> +	udelay(1);
>> +
>> +	reg = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
>> +	reg &= ~XUSB_PADCTL_USB2_BIAS_PAD_CTL1_PD_TRK;
>> +	padctl_writel(padctl, reg, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
>> +}
>> +
>> +/* must be called under padctl->lock */
>> +static void tegra210_utmi_bias_pad_power_off(struct tegra_xusb_padctl *padctl,
>> +				struct tegra_xusb_usb2_pad *pad)
>> +{
>> +	u32 reg;
>> +
>> +	if (WARN_ON(pad->enable == 0))
>> +		return;
>> +
>> +	if (--pad->enable > 0)
>> +		return;
>> +
>> +	dev_dbg(padctl->dev, "power off USB2 tracking\n");
>> +	reg = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
>> +	reg |= XUSB_PADCTL_USB2_BIAS_PAD_CTL1_PD_TRK;
>> +	padctl_writel(padctl, reg, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
>> +
>> +	clk_disable_unprepare(pad->clk);
>> +}
>> +
>> +void tegra210_utmi_pad_power_on(struct phy *phy)
>> +{
>> +	struct tegra_xusb_lane *lane;
>> +	struct tegra_xusb_usb2_lane *usb2;
>> +	struct tegra_xusb_usb2_pad *pad;
>> +	struct tegra_xusb_padctl *padctl;
>> +	unsigned int index;
>> +	struct device *dev;
>> +	u32 reg;
>> +
>> +	if (!phy)
>> +		return;
>> +
>> +	lane = phy_get_drvdata(phy);
>> +	usb2 = to_usb2_lane(lane);
>> +	pad = to_usb2_pad(lane->pad);
>> +	padctl = lane->pad->padctl;
>> +	index = lane->index;
>> +	dev = padctl->dev;
>> +
>> +	if (usb2->powered_on)
>> +		return;
>> +
>> +	mutex_lock(&padctl->lock);
>> +
>> +	dev_info(dev, "power on UTMI pads %d\n", index);
> 
> dev_dbg()? Also, index is unsigned, so you should use %u to print it.
> 

Will change to dev_dbg and print index as %u

>> +
>> +	tegra210_utmi_bias_pad_power_on(padctl, pad);
>> +
>> +	udelay(2);
>> +
>> +	reg = padctl_readl(padctl, XUSB_PADCTL_USB2_OTG_PADX_CTL0(index));
>> +	reg &= ~XUSB_PADCTL_USB2_OTG_PAD_CTL0_PD;
>> +	padctl_writel(padctl, reg, XUSB_PADCTL_USB2_OTG_PADX_CTL0(index));
>> +
>> +	reg = padctl_readl(padctl, XUSB_PADCTL_USB2_OTG_PADX_CTL1(index));
>> +	reg &= ~XUSB_PADCTL_USB2_OTG_PAD_CTL1_PD_DR;
>> +	padctl_writel(padctl, reg, XUSB_PADCTL_USB2_OTG_PADX_CTL1(index));
>> +
>> +	usb2->powered_on = true;
>> +
>> +	mutex_unlock(&padctl->lock);
>> +}
>> +
>> +void tegra210_utmi_pad_power_down(struct phy *phy)
>> +{
>> +	struct tegra_xusb_lane *lane;
>> +	struct tegra_xusb_usb2_lane *usb2;
>> +	struct tegra_xusb_usb2_pad *pad;
>> +	struct tegra_xusb_padctl *padctl;
>> +	unsigned int index;
>> +	struct device *dev;
>> +	u32 reg;
>> +
>> +	if (!phy)
>> +		return;
>> +
>> +	lane = phy_get_drvdata(phy);
>> +	usb2 = to_usb2_lane(lane);
>> +	pad = to_usb2_pad(lane->pad);
>> +	padctl = lane->pad->padctl;
>> +	index = lane->index;
>> +	dev = padctl->dev;
>> +
>> +	if (!usb2->powered_on)
>> +		return;
>> +
>> +	mutex_lock(&padctl->lock);
>> +
>> +	dev_info(dev, "power down UTMI pad %d\n", index);
> 
> Same as above.
> 

will do

>> +
>> +	reg = padctl_readl(padctl, XUSB_PADCTL_USB2_OTG_PADX_CTL0(index));
>> +	reg |= XUSB_PADCTL_USB2_OTG_PAD_CTL0_PD;
>> +	padctl_writel(padctl, reg, XUSB_PADCTL_USB2_OTG_PADX_CTL0(index));
>> +
>> +	reg = padctl_readl(padctl, XUSB_PADCTL_USB2_OTG_PADX_CTL1(index));
>> +	reg |= XUSB_PADCTL_USB2_OTG_PAD_CTL1_PD_DR;
>> +	padctl_writel(padctl, reg, XUSB_PADCTL_USB2_OTG_PADX_CTL1(index));
>> +
>> +	udelay(2);
>> +
>> +	tegra210_utmi_bias_pad_power_off(padctl, pad);
>> +	usb2->powered_on = false;
>> +
>> +	mutex_unlock(&padctl->lock);
>> +}
> 
> I'm not sure I understand this correctly. This seems to implement some
> reference counting of its own. Why is that? Is it because it is shared
> between multiple pads? But then, you're writing to indexed registers,
> which indicates to me that these are in fact separate pads and not a
> shared resource. So why the reference counting?
> 
> I'm also not sure why these functions need to be accessible outside this
> file. They are not used outside, as far as I can tell.
> 

Lock was added, since the API area called from external context(XUDC driver)
and to protect against on/off, mutex_lock was done.
Based on later suggestion in the review, will move utmi pad and utmi bias pad
power on functions to phy_power_on, with that, mutex lock is not needed any more.

>>  static int tegra210_usb2_phy_init(struct phy *phy)
>>  {
>>  	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
>> @@ -948,6 +1118,34 @@ static int tegra210_usb2_phy_power_on(struct phy *phy)
>>  
>>  	priv = to_tegra210_xusb_padctl(padctl);
>>  
>> +	if (port->usb3_port_fake != -1) {
>> +		value = padctl_readl(padctl, XUSB_PADCTL_SS_PORT_MAP);
>> +		value &= ~XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_MASK(
>> +					port->usb3_port_fake);
>> +		value |= XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP(
>> +					port->usb3_port_fake, index);
>> +		padctl_writel(padctl, value, XUSB_PADCTL_SS_PORT_MAP);
>> +
>> +		value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
>> +		value &= ~XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_VCORE_DOWN(
>> +					port->usb3_port_fake);
>> +		padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
>> +
>> +		usleep_range(100, 200);
>> +
>> +		value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
>> +		value &= ~XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN_EARLY(
>> +					port->usb3_port_fake);
>> +		padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
>> +
>> +		usleep_range(100, 200);
>> +
>> +		value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
>> +		value &= ~XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN(
>> +					port->usb3_port_fake);
>> +		padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
>> +	}
>> +
>>  	value = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL0);
>>  	value &= ~((XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_SQUELCH_LEVEL_MASK <<
>>  		    XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_SQUELCH_LEVEL_SHIFT) |
>> @@ -965,7 +1163,14 @@ static int tegra210_usb2_phy_power_on(struct phy *phy)
>>  
>>  	value = padctl_readl(padctl, XUSB_PADCTL_USB2_PORT_CAP);
>>  	value &= ~XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_MASK(index);
>> -	value |= XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_HOST(index);
>> +	if (port->port_cap == USB_PORT_DISABLED)
>> +		value |= XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_DISABLED(index);
>> +	else if (port->port_cap == USB_DEVICE_CAP)
>> +		value |= XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_DEVICE(index);
>> +	else if (port->port_cap == USB_HOST_CAP)
>> +		value |= XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_HOST(index);
>> +	else if (port->port_cap == USB_OTG_CAP)
>> +		value |= XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_OTG(index);
>>  	padctl_writel(padctl, value, XUSB_PADCTL_USB2_PORT_CAP);
>>  
>>  	value = padctl_readl(padctl, XUSB_PADCTL_USB2_OTG_PADX_CTL0(index));
>> @@ -997,7 +1202,12 @@ static int tegra210_usb2_phy_power_on(struct phy *phy)
>>  			     XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPADX_CTL1(index));
>>  	value &= ~(XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_MASK <<
>>  		   XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_SHIFT);
>> -	value |= XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_FIX18;
>> +	if (port->port_cap == USB_HOST_CAP)
>> +		value |= XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_FIX18;
>> +	else
>> +		value |=
>> +		      XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_VAL <<
>> +		      XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_SHIFT;
>>  	padctl_writel(padctl, value,
>>  		      XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPADX_CTL1(index));
>>  
>> @@ -1070,6 +1280,34 @@ static int tegra210_usb2_phy_power_off(struct phy *phy)
>>  
>>  	mutex_lock(&padctl->lock);
>>  
>> +	if (port->usb3_port_fake != -1) {
>> +		value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
>> +		value |= XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN_EARLY(
>> +					port->usb3_port_fake);
>> +		padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
>> +
>> +		usleep_range(100, 200);
>> +
>> +		value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
>> +		value |= XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN(
>> +					port->usb3_port_fake);
>> +		padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
>> +
>> +		usleep_range(250, 350);
>> +
>> +		value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
>> +		value |= XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_VCORE_DOWN(
>> +					port->usb3_port_fake);
>> +		padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
>> +
>> +		value = padctl_readl(padctl, XUSB_PADCTL_SS_PORT_MAP);
>> +		value &= ~XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_MASK(
>> +					port->usb3_port_fake);
>> +		value |= XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_MASK(
>> +					port->usb3_port_fake);
>> +		padctl_writel(padctl, value, XUSB_PADCTL_SS_PORT_MAP);
>> +	}
>> +
>>  	if (WARN_ON(pad->enable == 0))
>>  		goto out;
>>  
>> @@ -1953,6 +2191,55 @@ static const struct tegra_xusb_port_ops tegra210_usb3_port_ops = {
>>  	.map = tegra210_usb3_port_map,
>>  };
>>  
>> +static int tegra210_xusb_padctl_vbus_override(struct tegra_xusb_padctl *padctl,
>> +					      bool set)
>> +{
>> +	u32 reg;
>> +
>> +	dev_dbg(padctl->dev, "%s vbus override\n", set ? "set" : "clear");
>> +
>> +	reg = padctl_readl(padctl, XUSB_PADCTL_USB2_VBUS_ID);
>> +	if (set) {
>> +		reg |= XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_VBUS_ON;
>> +		reg &= ~(XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_MASK <<
>> +			   XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_SHIFT);
>> +		reg |= XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_VAL <<
>> +			 XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_SHIFT;
>> +	} else
>> +		reg &= ~XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_VBUS_ON;
>> +	padctl_writel(padctl, reg, XUSB_PADCTL_USB2_VBUS_ID);
>> +
>> +	return 0;
>> +}
> 
> It'd be good to briefly explain in a comment (or the commit message)
> what exactly this does and what it's used for.
> 

vbus_override setting is needed to inform controller who drives vbus.
For device mode, gadget driver needs to set override so that vbus is
detected on connected host.
Will Add the comment accordingly

>> +
>> +static int tegra210_utmi_port_reset_quirk(struct phy *phy)
>> +{
>> +	struct tegra_xusb_padctl *padctl;
>> +	struct tegra_xusb_lane *lane;
>> +	struct device *dev;
>> +	u32 reg;
>> +
>> +	if (!phy)
>> +		return -ENODEV;
>> +
>> +	lane = phy_get_drvdata(phy);
>> +	padctl = lane->pad->padctl;
>> +	dev = padctl->dev;
>> +
>> +	reg = padctl_readl(padctl,
>> +				XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPADX_CTL0(0));
>> +	dev_dbg(dev, "BATTERY_CHRG_OTGPADX_CTL0(0): 0x%x\n", reg);
>> +
>> +	if ((reg & XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL0_ZIP) ||
>> +	    (reg & XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL0_ZIN)) {
>> +		dev_dbg(dev, "Toggle vbus\n");
>> +		tegra210_xusb_padctl_vbus_override(padctl, false);
>> +		tegra210_xusb_padctl_vbus_override(padctl, true);
>> +		return 1;
>> +	}
>> +	return 0;
>> +}
> 
> This uses the VBUS override mechanism, but at the same time you also
> export it to make use of it in the XUDC driver (presumably). Why have
> two ways of running this code? Again, an explanation of what exactly
> this is used for would clarify.
> 

This is a reset Quirk executed by XUDC driver on PLC inactive case and is
planned to be done only if bits ZIN or ZIP are set in XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPADX_CTL0(0)
register.
Will add comment accordingly

>>  static int
>>  tegra210_xusb_read_fuse_calibration(struct tegra210_xusb_fuse_calibration *fuse)
>>  {
>> @@ -2015,6 +2302,10 @@ static const struct tegra_xusb_padctl_ops tegra210_xusb_padctl_ops = {
>>  	.remove = tegra210_xusb_padctl_remove,
>>  	.usb3_set_lfps_detect = tegra210_usb3_set_lfps_detect,
>>  	.hsic_set_idle = tegra210_hsic_set_idle,
>> +	.vbus_override = tegra210_xusb_padctl_vbus_override,
>> +	.utmi_pad_power_on = tegra210_utmi_pad_power_on,
>> +	.utmi_pad_power_down = tegra210_utmi_pad_power_down,
>> +	.utmi_port_reset_quirk = tegra210_utmi_port_reset_quirk,
>>  };
>>  
>>  const struct tegra_xusb_padctl_soc tegra210_xusb_padctl_soc = {
>> diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
>> index 5b3b886..9a25b5d 100644
>> --- a/drivers/phy/tegra/xusb.c
>> +++ b/drivers/phy/tegra/xusb.c
>> @@ -1,5 +1,5 @@
>>  /*
>> - * Copyright (c) 2014-2015, NVIDIA CORPORATION.  All rights reserved.
>> + * Copyright (c) 2014-2018, NVIDIA CORPORATION.  All rights reserved.
>>   *
>>   * This program is free software; you can redistribute it and/or modify it
>>   * under the terms and conditions of the GNU General Public License,
>> @@ -546,9 +546,26 @@ static int tegra_xusb_usb2_port_parse_dt(struct tegra_xusb_usb2_port *usb2)
>>  {
>>  	struct tegra_xusb_port *port = &usb2->base;
>>  	struct device_node *np = port->dev.of_node;
>> +	const char *prop_string;
>> +	u32 value;
>>  
>>  	usb2->internal = of_property_read_bool(np, "nvidia,internal");
>>  
>> +	usb2->port_cap = USB_PORT_DISABLED; /* default */
>> +	if (!of_property_read_string(np, "mode", &prop_string)) {
>> +		if (!strcmp("host", prop_string))
>> +			usb2->port_cap = USB_HOST_CAP;
>> +		else if (!strcmp("device", prop_string))
>> +			usb2->port_cap = USB_DEVICE_CAP;
>> +		else if (!strcmp("otg", prop_string))
>> +			usb2->port_cap = USB_OTG_CAP;
>> +	}
> 
> This seems to fulfill the same purpose as this:
> 
> 	http://patchwork.ozlabs.org/patch/1031014/
> 
> Although, looking more closely at that, we can probably just the
> usb_get_dr_mode() helper for this. There's also a standard property for
> the dual-role mode, named "dr_mode", albeit the name does not seem to
> match standard best practices. Seems like it would still make sense to
> move to that standard property name.
> 

Yes, this part of change is what below patch is doing
	http://patchwork.ozlabs.org/patch/1031014/
w.r.t using usb_get_dr_mode(), it parses dr_mode string if present in fwnode
But in our case, we have mode part of port setting and is not inline with
expectation of API usb_get_dr_mode(). I believe we need to stay with implementation
as in above link.

Considering current patch and changes in patch/1031014/ are same,
please suggest, if changes w.r.t mode reading will be taken as part of
patch/1031014/ or shall i continue in current patch series

>> +	if (!of_property_read_u32(np, "nvidia,usb3-port-fake", &value))
>> +		usb2->usb3_port_fake = value;
>> +	else
>> +		usb2->usb3_port_fake = -1; /* default */
>> +
>>  	usb2->supply = devm_regulator_get(&port->dev, "vbus");
>>  	return PTR_ERR_OR_ZERO(usb2->supply);
>>  }
>> @@ -976,7 +993,7 @@ int tegra_xusb_padctl_usb3_save_context(struct tegra_xusb_padctl *padctl,
>>  	if (padctl->soc->ops->usb3_save_context)
>>  		return padctl->soc->ops->usb3_save_context(padctl, port);
>>  
>> -	return -ENOSYS;
>> +	return -EINVAL;
> 
> Why change the error code for these? EINVAL has a pretty standard
> meaning ("invalid argument") and doesn't match the error condition at
> all. ENOSYS was chosen here because it's the closest there is to "not
> implemented". ENOSYS is somewhat special in relation to syscalls, but
> since we never propagate errors from here to userspace, it's safe to
> use.
> 

ENOSYS only add correct error info.
will revert change to return type and also use ENOSYS for new export functions

>>  }
>>  EXPORT_SYMBOL_GPL(tegra_xusb_padctl_usb3_save_context);
>>  
>> @@ -986,7 +1003,7 @@ int tegra_xusb_padctl_hsic_set_idle(struct tegra_xusb_padctl *padctl,
>>  	if (padctl->soc->ops->hsic_set_idle)
>>  		return padctl->soc->ops->hsic_set_idle(padctl, port, idle);
>>  
>> -	return -ENOSYS;
>> +	return -EINVAL;
>>  }
>>  EXPORT_SYMBOL_GPL(tegra_xusb_padctl_hsic_set_idle);
>>  
>> @@ -997,10 +1014,60 @@ int tegra_xusb_padctl_usb3_set_lfps_detect(struct tegra_xusb_padctl *padctl,
>>  		return padctl->soc->ops->usb3_set_lfps_detect(padctl, port,
>>  							      enable);
>>  
>> -	return -ENOSYS;
>> +	return -EINVAL;
>>  }
>>  EXPORT_SYMBOL_GPL(tegra_xusb_padctl_usb3_set_lfps_detect);
>>  
>> +int tegra_xusb_padctl_set_vbus_override(struct tegra_xusb_padctl *padctl)
>> +{
>> +	if (padctl->soc->ops->vbus_override)
>> +		return padctl->soc->ops->vbus_override(padctl, true);
>> +
>> +	return -EINVAL;
> 
> It'd be best to stick to the -ENOSYS error code here.
> 

will do

>> +void tegra_phy_xusb_utmi_pad_power_on(struct phy *phy)
>> +{
>> +	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
>> +	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
>> +
>> +	if (padctl->soc->ops->utmi_pad_power_on)
>> +		padctl->soc->ops->utmi_pad_power_on(phy);
>> +}
>> +EXPORT_SYMBOL_GPL(tegra_phy_xusb_utmi_pad_power_on);
>> +
>> +void tegra_phy_xusb_utmi_pad_power_down(struct phy *phy)
>> +{
>> +	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
>> +	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
>> +
>> +	if (padctl->soc->ops->utmi_pad_power_down)
>> +		padctl->soc->ops->utmi_pad_power_down(phy);
>> +}
>> +EXPORT_SYMBOL_GPL(tegra_phy_xusb_utmi_pad_power_down);
> 
> I already briefly touched on this earlier, bu why do these have to be
> exported functions? Can't these simply be abstracted behind the PHY
> abstraction?
> 

yes, they can be part of phy_power_xxx(). Will remove
tegra_phy_xusb_utmi_pad_power_xx export functions

>> diff --git a/drivers/phy/tegra/xusb.h b/drivers/phy/tegra/xusb.h
>> index b49dbc3..74acc08 100644
>> --- a/drivers/phy/tegra/xusb.h
>> +++ b/drivers/phy/tegra/xusb.h
>> @@ -1,5 +1,5 @@
>>  /*
>> - * Copyright (c) 2014-2015, NVIDIA CORPORATION.  All rights reserved.
>> + * Copyright (c) 2014-2018, NVIDIA CORPORATION.  All rights reserved.
>>   * Copyright (c) 2015, Google Inc.
>>   *
>>   * This program is free software; you can redistribute it and/or modify it
>> @@ -58,6 +58,7 @@ struct tegra_xusb_usb2_lane {
>>  	struct tegra_xusb_lane base;
>>  
>>  	u32 hs_curr_level_offset;
>> +	bool powered_on;
>>  };
>>  
>>  static inline struct tegra_xusb_usb2_lane *
>> @@ -267,11 +268,20 @@ struct tegra_xusb_port *
>>  tegra_xusb_find_port(struct tegra_xusb_padctl *padctl, const char *type,
>>  		     unsigned int index);
>>  
>> +enum tegra_xusb_usb_port_cap {
>> +	USB_PORT_DISABLED = 0,
>> +	USB_HOST_CAP,
>> +	USB_DEVICE_CAP,
>> +	USB_OTG_CAP,
>> +};
> 
> Maybe use enum usb_dr_mode from include/linux/usb/otg.h instead of
> essentially duplicating it?
> 

Yes, its the same, will re-use usb_dr_mode

>> +
>>  struct tegra_xusb_usb2_port {
>>  	struct tegra_xusb_port base;
>>  
>>  	struct regulator *supply;
>>  	bool internal;
>> +	enum tegra_xusb_usb_port_cap port_cap;
>> +	int usb3_port_fake;
>>  };
>>  
>>  static inline struct tegra_xusb_usb2_port *
>> @@ -353,6 +363,10 @@ struct tegra_xusb_padctl_ops {
>>  			     unsigned int index, bool idle);
>>  	int (*usb3_set_lfps_detect)(struct tegra_xusb_padctl *padctl,
>>  				    unsigned int index, bool enable);
>> +	int (*vbus_override)(struct tegra_xusb_padctl *padctl, bool set);
>> +	void (*utmi_pad_power_on)(struct phy *phy);
>> +	void (*utmi_pad_power_down)(struct phy *phy);
>> +	int (*utmi_port_reset_quirk)(struct phy *phy);
>>  };
>>  
>>  struct tegra_xusb_padctl_soc {
>> diff --git a/include/linux/phy/tegra/xusb.h b/include/linux/phy/tegra/xusb.h
>> index 8e1a57a..143af44 100644
>> --- a/include/linux/phy/tegra/xusb.h
>> +++ b/include/linux/phy/tegra/xusb.h
>> @@ -1,5 +1,5 @@
>>  /*
>> - * Copyright (c) 2016, NVIDIA CORPORATION.  All rights reserved.
>> + * Copyright (c) 2016-2018, NVIDIA CORPORATION.  All rights reserved.
>>   *
>>   * This program is free software; you can redistribute it and/or modify it
>>   * under the terms and conditions of the GNU General Public License,
>> @@ -26,5 +26,10 @@ int tegra_xusb_padctl_hsic_set_idle(struct tegra_xusb_padctl *padctl,
>>  				    unsigned int port, bool idle);
>>  int tegra_xusb_padctl_usb3_set_lfps_detect(struct tegra_xusb_padctl *padctl,
>>  					   unsigned int port, bool enable);
>> +int tegra_xusb_padctl_set_vbus_override(struct tegra_xusb_padctl *padctl);
>> +int tegra_xusb_padctl_clear_vbus_override(struct tegra_xusb_padctl *padctl);
>>  
>> +void tegra_phy_xusb_utmi_pad_power_on(struct phy *phy);
>> +void tegra_phy_xusb_utmi_pad_power_down(struct phy *phy);
>> +int tegra_phy_xusb_utmi_port_reset_quirk(struct phy *phy);
> 
> We already have more custom APIs here than I'd like, so I really want to
> make sure that we're not adding to these if not absolutely necessary.
> 
> Are there any reasons why we can't somehow hide these behind the
> existing PHY framework?
> 
> tegra_phy_xusb_utmi_port_reset_quirk() seems like something that we may
> be able to implement using the phy_ops->reset() callback. The power_on()
> and power_off() for the UTMI pad seem like they should be just part of
> the regular pad ->power_on() and ->power_off() callback implementations
> (perhaps this means that we need to introduce another pad for this?).
> 

As mentioned earlier, will move utmi pad and utmi bias pad power function
can be to phy power functions.

tegra_phy_xusb_utmi_port_reset_quirk() does a vbus override only on a specific
condition, so it cannot be direct replacement of phy_ops->reset.

> Set/clear VBUS override seems like maybe something that could be hidden
> behind phy_ops->configure(). This is a fairly recent addition, but
> sounds like we could also push things like HSIC idle and USB3 LPFS
> detection into that.
> 
AFAIU, we can not use phy_ops->configure(), since the same has enum argument
phy_configure_opts_mipi_dphy, which is not inline with needs of vbus override.
Also, API is not present in current kernel.
So i beleive we need to stay with vbus override API, which instead of 2, will
export 1(and take argument as set/clear) and other port_reset_quirk

> Thierry
> 

- Nagarjuna
Thierry Reding - Feb. 13, 2019, 8:35 a.m.
On Wed, Feb 13, 2019 at 12:34:39PM +0530, Nagarjuna Kristam wrote:
> Hi Thierry,
> 
> On 04-02-2019 17:57, Thierry Reding wrote:
> > On Thu, Jan 03, 2019 at 03:34:52PM +0530, Nagarjuna Kristam wrote:
> >> Add support for XUSB device mode controller on Tegra210.
> >> Update PADCTL driver to set port cap based on DT config.
> >> Add code to handle property "nvidia,usb3-port-fake"
> >> Provide API's to control vbus override and utmi pad power control
> > 
> > You essentially list three features additions here, so it makes sense to
> > provide each of them in a separate patch. That helps reviewers
> > concentrate on one feature at a time without having to load context for
> > three different things in parallel.
> > 
> > Also, for each feature please provide a bit more background information
> > as well as reasons for why they are added.
> > 
> 
> For XUSB device control Feature support, we needed all three and hence
> added them as part of one change. If we can have group of individuals CL's
> for providing one feature support, will split them accordingly.
> Doing above will mark current changes as a different patch series, considering
> new patch series will be made, is it fine to move USB gadget driver changes to
> a new series ?
> Please share your inputs

Yes, it's generally fine to rearrange patches in a series or add new
ones as the need arises. One thing to consider is to keep the subject of
the cover letter intact and use proper versioning of patches. Both of
these should help track the series as it evolves.

When splitting up patches, though, make sure that all patches can be
successfully built in the order that you send them. This is important in
order to be able to run git bisections later on. You can easily verify
this by doing something like:

	$ git rebase -i --exec '/bin/sh'

once the series is ready. That will automatically drop you into a shell
after each commit and you can run your build commands to verify that
each commit builds. You can also replace "/bin/sh" with the build
command directly to fully automate this process. However you have to
make sure that your build command returns proper error codes on failure
for this to work.

[...]
> >> +	reg = padctl_readl(padctl, XUSB_PADCTL_USB2_OTG_PADX_CTL0(index));
> >> +	reg |= XUSB_PADCTL_USB2_OTG_PAD_CTL0_PD;
> >> +	padctl_writel(padctl, reg, XUSB_PADCTL_USB2_OTG_PADX_CTL0(index));
> >> +
> >> +	reg = padctl_readl(padctl, XUSB_PADCTL_USB2_OTG_PADX_CTL1(index));
> >> +	reg |= XUSB_PADCTL_USB2_OTG_PAD_CTL1_PD_DR;
> >> +	padctl_writel(padctl, reg, XUSB_PADCTL_USB2_OTG_PADX_CTL1(index));
> >> +
> >> +	udelay(2);
> >> +
> >> +	tegra210_utmi_bias_pad_power_off(padctl, pad);
> >> +	usb2->powered_on = false;
> >> +
> >> +	mutex_unlock(&padctl->lock);
> >> +}
> > 
> > I'm not sure I understand this correctly. This seems to implement some
> > reference counting of its own. Why is that? Is it because it is shared
> > between multiple pads? But then, you're writing to indexed registers,
> > which indicates to me that these are in fact separate pads and not a
> > shared resource. So why the reference counting?
> > 
> > I'm also not sure why these functions need to be accessible outside this
> > file. They are not used outside, as far as I can tell.
> > 
> 
> Lock was added, since the API area called from external context(XUDC driver)
> and to protect against on/off, mutex_lock was done.
> Based on later suggestion in the review, will move utmi pad and utmi bias pad
> power on functions to phy_power_on, with that, mutex lock is not needed any more.

Sounds great!

[...]
> >> @@ -546,9 +546,26 @@ static int tegra_xusb_usb2_port_parse_dt(struct tegra_xusb_usb2_port *usb2)
> >>  {
> >>  	struct tegra_xusb_port *port = &usb2->base;
> >>  	struct device_node *np = port->dev.of_node;
> >> +	const char *prop_string;
> >> +	u32 value;
> >>  
> >>  	usb2->internal = of_property_read_bool(np, "nvidia,internal");
> >>  
> >> +	usb2->port_cap = USB_PORT_DISABLED; /* default */
> >> +	if (!of_property_read_string(np, "mode", &prop_string)) {
> >> +		if (!strcmp("host", prop_string))
> >> +			usb2->port_cap = USB_HOST_CAP;
> >> +		else if (!strcmp("device", prop_string))
> >> +			usb2->port_cap = USB_DEVICE_CAP;
> >> +		else if (!strcmp("otg", prop_string))
> >> +			usb2->port_cap = USB_OTG_CAP;
> >> +	}
> > 
> > This seems to fulfill the same purpose as this:
> > 
> > 	http://patchwork.ozlabs.org/patch/1031014/
> > 
> > Although, looking more closely at that, we can probably just the
> > usb_get_dr_mode() helper for this. There's also a standard property for
> > the dual-role mode, named "dr_mode", albeit the name does not seem to
> > match standard best practices. Seems like it would still make sense to
> > move to that standard property name.
> > 
> 
> Yes, this part of change is what below patch is doing
> 	http://patchwork.ozlabs.org/patch/1031014/
> w.r.t using usb_get_dr_mode(), it parses dr_mode string if present in fwnode
> But in our case, we have mode part of port setting and is not inline with
> expectation of API usb_get_dr_mode(). I believe we need to stay with implementation
> as in above link.

Yeah, I realized the same thing when I tried to convert the above patch
to use usb_get_dr_mode() instead. I agree, I think we need to stick with
the current implementation.

> Considering current patch and changes in patch/1031014/ are same,
> please suggest, if changes w.r.t mode reading will be taken as part of
> patch/1031014/ or shall i continue in current patch series

I would suggest that you either stick with this patch or replace it with
the above for now. If the above series goes in first, you can rebase
your series on top of it and drop the duplicate patch. If your series
goes in first, then the above series will need to be rebased and drop
the duplicate patch.

> >> +void tegra_phy_xusb_utmi_pad_power_on(struct phy *phy)
> >> +{
> >> +	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
> >> +	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
> >> +
> >> +	if (padctl->soc->ops->utmi_pad_power_on)
> >> +		padctl->soc->ops->utmi_pad_power_on(phy);
> >> +}
> >> +EXPORT_SYMBOL_GPL(tegra_phy_xusb_utmi_pad_power_on);
> >> +
> >> +void tegra_phy_xusb_utmi_pad_power_down(struct phy *phy)
> >> +{
> >> +	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
> >> +	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
> >> +
> >> +	if (padctl->soc->ops->utmi_pad_power_down)
> >> +		padctl->soc->ops->utmi_pad_power_down(phy);
> >> +}
> >> +EXPORT_SYMBOL_GPL(tegra_phy_xusb_utmi_pad_power_down);
> > 
> > I already briefly touched on this earlier, bu why do these have to be
> > exported functions? Can't these simply be abstracted behind the PHY
> > abstraction?
> > 
> 
> yes, they can be part of phy_power_xxx(). Will remove
> tegra_phy_xusb_utmi_pad_power_xx export functions

Excellent!

[...]
> >> diff --git a/include/linux/phy/tegra/xusb.h b/include/linux/phy/tegra/xusb.h
> >> index 8e1a57a..143af44 100644
> >> --- a/include/linux/phy/tegra/xusb.h
> >> +++ b/include/linux/phy/tegra/xusb.h
> >> @@ -1,5 +1,5 @@
> >>  /*
> >> - * Copyright (c) 2016, NVIDIA CORPORATION.  All rights reserved.
> >> + * Copyright (c) 2016-2018, NVIDIA CORPORATION.  All rights reserved.
> >>   *
> >>   * This program is free software; you can redistribute it and/or modify it
> >>   * under the terms and conditions of the GNU General Public License,
> >> @@ -26,5 +26,10 @@ int tegra_xusb_padctl_hsic_set_idle(struct tegra_xusb_padctl *padctl,
> >>  				    unsigned int port, bool idle);
> >>  int tegra_xusb_padctl_usb3_set_lfps_detect(struct tegra_xusb_padctl *padctl,
> >>  					   unsigned int port, bool enable);
> >> +int tegra_xusb_padctl_set_vbus_override(struct tegra_xusb_padctl *padctl);
> >> +int tegra_xusb_padctl_clear_vbus_override(struct tegra_xusb_padctl *padctl);
> >>  
> >> +void tegra_phy_xusb_utmi_pad_power_on(struct phy *phy);
> >> +void tegra_phy_xusb_utmi_pad_power_down(struct phy *phy);
> >> +int tegra_phy_xusb_utmi_port_reset_quirk(struct phy *phy);
> > 
> > We already have more custom APIs here than I'd like, so I really want to
> > make sure that we're not adding to these if not absolutely necessary.
> > 
> > Are there any reasons why we can't somehow hide these behind the
> > existing PHY framework?
> > 
> > tegra_phy_xusb_utmi_port_reset_quirk() seems like something that we may
> > be able to implement using the phy_ops->reset() callback. The power_on()
> > and power_off() for the UTMI pad seem like they should be just part of
> > the regular pad ->power_on() and ->power_off() callback implementations
> > (perhaps this means that we need to introduce another pad for this?).
> > 
> 
> As mentioned earlier, will move utmi pad and utmi bias pad power function
> can be to phy power functions.
> 
> tegra_phy_xusb_utmi_port_reset_quirk() does a vbus override only on a specific
> condition, so it cannot be direct replacement of phy_ops->reset.

If XUDC is the only user of the PHY, then it can call phy_reset() on the
PHY at any time that it wants, so it could be a replacement for the
custom API if we choose to.

That said, it seems like this actually operates on the "port" registers
rather than the "pad" registers, so phy_reset() might not be the best
candidate after all, since the PHYs really only deal with pads.

> > Set/clear VBUS override seems like maybe something that could be hidden
> > behind phy_ops->configure(). This is a fairly recent addition, but
> > sounds like we could also push things like HSIC idle and USB3 LPFS
> > detection into that.
> > 
> AFAIU, we can not use phy_ops->configure(), since the same has enum argument
> phy_configure_opts_mipi_dphy, which is not inline with needs of vbus override.
> Also, API is not present in current kernel.

The API is part of linux-next, so you could rebase the series on that
and use it. Also, what I was suggesting is that we could extend the new
configure hook with options that can model the VBUS override
functionality. But I also realize that it may be a bit of a stretch to
consider the VBUS override configuration. Again, it's also dealing with
a property that's more in the domain of the port rather than the PHY, so
phy_configure() may not be a good candidate after all.

> So i beleive we need to stay with vbus override API, which instead of 2, will
> export 1(and take argument as set/clear) and other port_reset_quirk

Sounds good. Although I'm wondering if perhaps the name could be made a
bit more generic. What if we called the VBUS override APIs something
like:

	int tegra_xusb_padctl_set_otg_mode(struct tegra_xusb_padctl *padctl,
					   enum usb_dr_mode mode);

That way we avoid having a boolean argument. I find those somewhat
annoying because you can't immediately see from the callsite what they
mean. It's fairly clear in this case, but:

	tegra_xusb_padctl_set_otg_mode(padctl, USB_DR_MODE_PERIPHERAL);

is even clearer. And it has the added benefit that we can add more code
to it if we need to without making the name ambiguous.

Again, this isn't a strong objection, so if you prefer to keep the
tegra_xusb_padctl_set_vbus_override(padctl, true/false), feel free to do
so.

Thierry
Nagarjuna Kristam - Feb. 15, 2019, 6:18 a.m.
On 13-02-2019 12:34, Nagarjuna Kristam wrote:
>>>  EXPORT_SYMBOL_GPL(tegra_xusb_padctl_usb3_set_lfps_detect);
>>>  
>>> +int tegra_xusb_padctl_set_vbus_override(struct tegra_xusb_padctl *padctl)
>>> +{
>>> +	if (padctl->soc->ops->vbus_override)
>>> +		return padctl->soc->ops->vbus_override(padctl, true);
>>> +
>>> +	return -EINVAL;
>> It'd be best to stick to the -ENOSYS error code here.
>>
> will do
> 
Using ENOSYS is marked as checkpatch warning
	WARNING: ENOSYS means 'invalid syscall nr' and nothing else

I suggest to use ENOTSUP instead of EINVAL/ENOSYS.

-Nagarjuna
>>> +void tegra_phy_xusb_utmi_pad_power_on(struct phy *phy)
>>> +{
>>> +	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
>>> +	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
>>> +
>>> +	if (padctl->soc->ops->utmi_pad_power_on)
>>> +		padctl->soc->ops->utmi_pad_power_on(phy);
Thierry Reding - Feb. 25, 2019, 12:23 p.m.
On Fri, Feb 15, 2019 at 11:48:37AM +0530, Nagarjuna Kristam wrote:
> 
> 
> On 13-02-2019 12:34, Nagarjuna Kristam wrote:
> >>>  EXPORT_SYMBOL_GPL(tegra_xusb_padctl_usb3_set_lfps_detect);
> >>>  
> >>> +int tegra_xusb_padctl_set_vbus_override(struct tegra_xusb_padctl *padctl)
> >>> +{
> >>> +	if (padctl->soc->ops->vbus_override)
> >>> +		return padctl->soc->ops->vbus_override(padctl, true);
> >>> +
> >>> +	return -EINVAL;
> >> It'd be best to stick to the -ENOSYS error code here.
> >>
> > will do
> > 
> Using ENOSYS is marked as checkpatch warning
> 	WARNING: ENOSYS means 'invalid syscall nr' and nothing else
> 
> I suggest to use ENOTSUP instead of EINVAL/ENOSYS.

Sounds good to me.

Thierry

Patch

diff --git a/drivers/phy/tegra/xusb-tegra210.c b/drivers/phy/tegra/xusb-tegra210.c
index 05bee32..0289e10 100644
--- a/drivers/phy/tegra/xusb-tegra210.c
+++ b/drivers/phy/tegra/xusb-tegra210.c
@@ -1,5 +1,5 @@ 
 /*
- * Copyright (c) 2014, NVIDIA CORPORATION.  All rights reserved.
+ * Copyright (c) 2014-2018, NVIDIA CORPORATION.  All rights reserved.
  * Copyright (C) 2015 Google, Inc.
  *
  * This program is free software; you can redistribute it and/or modify it
@@ -47,7 +47,10 @@ 
 #define XUSB_PADCTL_USB2_PAD_MUX_USB2_BIAS_PAD_XUSB 0x1
 
 #define XUSB_PADCTL_USB2_PORT_CAP 0x008
+#define XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_DISABLED(x) (0x0 << ((x) * 4))
 #define XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_HOST(x) (0x1 << ((x) * 4))
+#define XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_DEVICE(x) (0x2 << ((x) * 4))
+#define XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_OTG(x) (0x3 << ((x) * 4))
 #define XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_MASK(x) (0x3 << ((x) * 4))
 
 #define XUSB_PADCTL_SS_PORT_MAP 0x014
@@ -55,6 +58,7 @@ 
 #define XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_SHIFT(x) ((x) * 5)
 #define XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_MASK(x) (0x7 << ((x) * 5))
 #define XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP(x, v) (((v) & 0x7) << ((x) * 5))
+#define XUSB_PADCTL_SS_PORT_MAP_PORT_DISABLED 0x7
 
 #define XUSB_PADCTL_ELPG_PROGRAM1 0x024
 #define XUSB_PADCTL_ELPG_PROGRAM1_AUX_MUX_LP0_VCORE_DOWN (1 << 31)
@@ -69,9 +73,14 @@ 
 #define XUSB_PADCTL_USB3_PAD_MUX_PCIE_IDDQ_DISABLE(x) (1 << (1 + (x)))
 #define XUSB_PADCTL_USB3_PAD_MUX_SATA_IDDQ_DISABLE(x) (1 << (8 + (x)))
 
+#define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPADX_CTL0(x) (0x080 + (x) * 0x40)
+#define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL0_ZIP (1 << 18)
+#define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL0_ZIN (1 << 22)
+
 #define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPADX_CTL1(x) (0x084 + (x) * 0x40)
 #define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_SHIFT 7
 #define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_MASK 0x3
+#define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_VAL 0x1
 #define XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_FIX18 (1 << 6)
 
 #define XUSB_PADCTL_USB2_OTG_PADX_CTL0(x) (0x088 + (x) * 0x40)
@@ -230,6 +239,12 @@ 
 #define XUSB_PADCTL_UPHY_USB3_PADX_ECTL6(x) (0xa74 + (x) * 0x40)
 #define XUSB_PADCTL_UPHY_USB3_PAD_ECTL6_RX_EQ_CTRL_H_VAL 0xfcf01368
 
+#define XUSB_PADCTL_USB2_VBUS_ID 0xc60
+#define XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_VBUS_ON (1 << 14)
+#define XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_SHIFT 18
+#define XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_MASK 0xf
+#define XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_VAL 8
+
 struct tegra210_xusb_fuse_calibration {
 	u32 hs_curr_level[4];
 	u32 hs_term_range_adj;
@@ -905,6 +920,161 @@  static const struct tegra_xusb_lane_ops tegra210_usb2_lane_ops = {
 	.remove = tegra210_usb2_lane_remove,
 };
 
+/* must be called under padctl->lock */
+static void tegra210_utmi_bias_pad_power_on(struct tegra_xusb_padctl *padctl,
+				struct tegra_xusb_usb2_pad *pad)
+{
+	u32 reg;
+
+	if (pad->enable++ > 0)
+		return;
+
+	dev_dbg(padctl->dev, "power on BIAS PAD & USB2 tracking\n");
+
+	if (clk_prepare_enable(pad->clk))
+		dev_warn(padctl->dev, "failed to enable BIAS PAD & USB2 tracking\n");
+
+	reg = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
+
+	reg &= ~((XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_START_TIMER_MASK <<
+		    XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_START_TIMER_SHIFT) |
+		   (XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_DONE_RESET_TIMER_MASK <<
+		    XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_DONE_RESET_TIMER_SHIFT));
+	reg |= (XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_START_TIMER_VAL <<
+		  XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_START_TIMER_SHIFT) |
+		 (XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_DONE_RESET_TIMER_VAL <<
+		  XUSB_PADCTL_USB2_BIAS_PAD_CTL1_TRK_DONE_RESET_TIMER_SHIFT);
+	padctl_writel(padctl, reg, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
+
+	reg = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL0);
+	reg &= ~XUSB_PADCTL_USB2_BIAS_PAD_CTL0_PD;
+
+	reg &= ~((XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_SQUELCH_LEVEL_MASK <<
+		    XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_SQUELCH_LEVEL_SHIFT) |
+		   (XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_DISCON_LEVEL_MASK <<
+		    XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_DISCON_LEVEL_SHIFT));
+	reg |= (XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_DISCON_LEVEL_VAL <<
+		  XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_DISCON_LEVEL_SHIFT);
+
+	if (tegra_sku_info.revision < TEGRA_REVISION_A02)
+		reg |= (XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_SQUELCH_LEVEL_VAL <<
+			XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_SQUELCH_LEVEL_SHIFT);
+	padctl_writel(padctl, reg, XUSB_PADCTL_USB2_BIAS_PAD_CTL0);
+
+	udelay(1);
+
+	reg = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
+	reg &= ~XUSB_PADCTL_USB2_BIAS_PAD_CTL1_PD_TRK;
+	padctl_writel(padctl, reg, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
+}
+
+/* must be called under padctl->lock */
+static void tegra210_utmi_bias_pad_power_off(struct tegra_xusb_padctl *padctl,
+				struct tegra_xusb_usb2_pad *pad)
+{
+	u32 reg;
+
+	if (WARN_ON(pad->enable == 0))
+		return;
+
+	if (--pad->enable > 0)
+		return;
+
+	dev_dbg(padctl->dev, "power off USB2 tracking\n");
+	reg = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
+	reg |= XUSB_PADCTL_USB2_BIAS_PAD_CTL1_PD_TRK;
+	padctl_writel(padctl, reg, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
+
+	clk_disable_unprepare(pad->clk);
+}
+
+void tegra210_utmi_pad_power_on(struct phy *phy)
+{
+	struct tegra_xusb_lane *lane;
+	struct tegra_xusb_usb2_lane *usb2;
+	struct tegra_xusb_usb2_pad *pad;
+	struct tegra_xusb_padctl *padctl;
+	unsigned int index;
+	struct device *dev;
+	u32 reg;
+
+	if (!phy)
+		return;
+
+	lane = phy_get_drvdata(phy);
+	usb2 = to_usb2_lane(lane);
+	pad = to_usb2_pad(lane->pad);
+	padctl = lane->pad->padctl;
+	index = lane->index;
+	dev = padctl->dev;
+
+	if (usb2->powered_on)
+		return;
+
+	mutex_lock(&padctl->lock);
+
+	dev_info(dev, "power on UTMI pads %d\n", index);
+
+	tegra210_utmi_bias_pad_power_on(padctl, pad);
+
+	udelay(2);
+
+	reg = padctl_readl(padctl, XUSB_PADCTL_USB2_OTG_PADX_CTL0(index));
+	reg &= ~XUSB_PADCTL_USB2_OTG_PAD_CTL0_PD;
+	padctl_writel(padctl, reg, XUSB_PADCTL_USB2_OTG_PADX_CTL0(index));
+
+	reg = padctl_readl(padctl, XUSB_PADCTL_USB2_OTG_PADX_CTL1(index));
+	reg &= ~XUSB_PADCTL_USB2_OTG_PAD_CTL1_PD_DR;
+	padctl_writel(padctl, reg, XUSB_PADCTL_USB2_OTG_PADX_CTL1(index));
+
+	usb2->powered_on = true;
+
+	mutex_unlock(&padctl->lock);
+}
+
+void tegra210_utmi_pad_power_down(struct phy *phy)
+{
+	struct tegra_xusb_lane *lane;
+	struct tegra_xusb_usb2_lane *usb2;
+	struct tegra_xusb_usb2_pad *pad;
+	struct tegra_xusb_padctl *padctl;
+	unsigned int index;
+	struct device *dev;
+	u32 reg;
+
+	if (!phy)
+		return;
+
+	lane = phy_get_drvdata(phy);
+	usb2 = to_usb2_lane(lane);
+	pad = to_usb2_pad(lane->pad);
+	padctl = lane->pad->padctl;
+	index = lane->index;
+	dev = padctl->dev;
+
+	if (!usb2->powered_on)
+		return;
+
+	mutex_lock(&padctl->lock);
+
+	dev_info(dev, "power down UTMI pad %d\n", index);
+
+	reg = padctl_readl(padctl, XUSB_PADCTL_USB2_OTG_PADX_CTL0(index));
+	reg |= XUSB_PADCTL_USB2_OTG_PAD_CTL0_PD;
+	padctl_writel(padctl, reg, XUSB_PADCTL_USB2_OTG_PADX_CTL0(index));
+
+	reg = padctl_readl(padctl, XUSB_PADCTL_USB2_OTG_PADX_CTL1(index));
+	reg |= XUSB_PADCTL_USB2_OTG_PAD_CTL1_PD_DR;
+	padctl_writel(padctl, reg, XUSB_PADCTL_USB2_OTG_PADX_CTL1(index));
+
+	udelay(2);
+
+	tegra210_utmi_bias_pad_power_off(padctl, pad);
+	usb2->powered_on = false;
+
+	mutex_unlock(&padctl->lock);
+}
+
 static int tegra210_usb2_phy_init(struct phy *phy)
 {
 	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
@@ -948,6 +1118,34 @@  static int tegra210_usb2_phy_power_on(struct phy *phy)
 
 	priv = to_tegra210_xusb_padctl(padctl);
 
+	if (port->usb3_port_fake != -1) {
+		value = padctl_readl(padctl, XUSB_PADCTL_SS_PORT_MAP);
+		value &= ~XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_MASK(
+					port->usb3_port_fake);
+		value |= XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP(
+					port->usb3_port_fake, index);
+		padctl_writel(padctl, value, XUSB_PADCTL_SS_PORT_MAP);
+
+		value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
+		value &= ~XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_VCORE_DOWN(
+					port->usb3_port_fake);
+		padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
+
+		usleep_range(100, 200);
+
+		value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
+		value &= ~XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN_EARLY(
+					port->usb3_port_fake);
+		padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
+
+		usleep_range(100, 200);
+
+		value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
+		value &= ~XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN(
+					port->usb3_port_fake);
+		padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
+	}
+
 	value = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL0);
 	value &= ~((XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_SQUELCH_LEVEL_MASK <<
 		    XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_SQUELCH_LEVEL_SHIFT) |
@@ -965,7 +1163,14 @@  static int tegra210_usb2_phy_power_on(struct phy *phy)
 
 	value = padctl_readl(padctl, XUSB_PADCTL_USB2_PORT_CAP);
 	value &= ~XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_MASK(index);
-	value |= XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_HOST(index);
+	if (port->port_cap == USB_PORT_DISABLED)
+		value |= XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_DISABLED(index);
+	else if (port->port_cap == USB_DEVICE_CAP)
+		value |= XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_DEVICE(index);
+	else if (port->port_cap == USB_HOST_CAP)
+		value |= XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_HOST(index);
+	else if (port->port_cap == USB_OTG_CAP)
+		value |= XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_OTG(index);
 	padctl_writel(padctl, value, XUSB_PADCTL_USB2_PORT_CAP);
 
 	value = padctl_readl(padctl, XUSB_PADCTL_USB2_OTG_PADX_CTL0(index));
@@ -997,7 +1202,12 @@  static int tegra210_usb2_phy_power_on(struct phy *phy)
 			     XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPADX_CTL1(index));
 	value &= ~(XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_MASK <<
 		   XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_SHIFT);
-	value |= XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_FIX18;
+	if (port->port_cap == USB_HOST_CAP)
+		value |= XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_FIX18;
+	else
+		value |=
+		      XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_VAL <<
+		      XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL1_VREG_LEV_SHIFT;
 	padctl_writel(padctl, value,
 		      XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPADX_CTL1(index));
 
@@ -1070,6 +1280,34 @@  static int tegra210_usb2_phy_power_off(struct phy *phy)
 
 	mutex_lock(&padctl->lock);
 
+	if (port->usb3_port_fake != -1) {
+		value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
+		value |= XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN_EARLY(
+					port->usb3_port_fake);
+		padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
+
+		usleep_range(100, 200);
+
+		value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
+		value |= XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN(
+					port->usb3_port_fake);
+		padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
+
+		usleep_range(250, 350);
+
+		value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
+		value |= XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_VCORE_DOWN(
+					port->usb3_port_fake);
+		padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
+
+		value = padctl_readl(padctl, XUSB_PADCTL_SS_PORT_MAP);
+		value &= ~XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_MASK(
+					port->usb3_port_fake);
+		value |= XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_MASK(
+					port->usb3_port_fake);
+		padctl_writel(padctl, value, XUSB_PADCTL_SS_PORT_MAP);
+	}
+
 	if (WARN_ON(pad->enable == 0))
 		goto out;
 
@@ -1953,6 +2191,55 @@  static const struct tegra_xusb_port_ops tegra210_usb3_port_ops = {
 	.map = tegra210_usb3_port_map,
 };
 
+static int tegra210_xusb_padctl_vbus_override(struct tegra_xusb_padctl *padctl,
+					      bool set)
+{
+	u32 reg;
+
+	dev_dbg(padctl->dev, "%s vbus override\n", set ? "set" : "clear");
+
+	reg = padctl_readl(padctl, XUSB_PADCTL_USB2_VBUS_ID);
+	if (set) {
+		reg |= XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_VBUS_ON;
+		reg &= ~(XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_MASK <<
+			   XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_SHIFT);
+		reg |= XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_VAL <<
+			 XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_SHIFT;
+	} else
+		reg &= ~XUSB_PADCTL_USB2_VBUS_ID_OVERRIDE_VBUS_ON;
+	padctl_writel(padctl, reg, XUSB_PADCTL_USB2_VBUS_ID);
+
+	return 0;
+}
+
+static int tegra210_utmi_port_reset_quirk(struct phy *phy)
+{
+	struct tegra_xusb_padctl *padctl;
+	struct tegra_xusb_lane *lane;
+	struct device *dev;
+	u32 reg;
+
+	if (!phy)
+		return -ENODEV;
+
+	lane = phy_get_drvdata(phy);
+	padctl = lane->pad->padctl;
+	dev = padctl->dev;
+
+	reg = padctl_readl(padctl,
+				XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPADX_CTL0(0));
+	dev_dbg(dev, "BATTERY_CHRG_OTGPADX_CTL0(0): 0x%x\n", reg);
+
+	if ((reg & XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL0_ZIP) ||
+	    (reg & XUSB_PADCTL_USB2_BATTERY_CHRG_OTGPAD_CTL0_ZIN)) {
+		dev_dbg(dev, "Toggle vbus\n");
+		tegra210_xusb_padctl_vbus_override(padctl, false);
+		tegra210_xusb_padctl_vbus_override(padctl, true);
+		return 1;
+	}
+	return 0;
+}
+
 static int
 tegra210_xusb_read_fuse_calibration(struct tegra210_xusb_fuse_calibration *fuse)
 {
@@ -2015,6 +2302,10 @@  static const struct tegra_xusb_padctl_ops tegra210_xusb_padctl_ops = {
 	.remove = tegra210_xusb_padctl_remove,
 	.usb3_set_lfps_detect = tegra210_usb3_set_lfps_detect,
 	.hsic_set_idle = tegra210_hsic_set_idle,
+	.vbus_override = tegra210_xusb_padctl_vbus_override,
+	.utmi_pad_power_on = tegra210_utmi_pad_power_on,
+	.utmi_pad_power_down = tegra210_utmi_pad_power_down,
+	.utmi_port_reset_quirk = tegra210_utmi_port_reset_quirk,
 };
 
 const struct tegra_xusb_padctl_soc tegra210_xusb_padctl_soc = {
diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
index 5b3b886..9a25b5d 100644
--- a/drivers/phy/tegra/xusb.c
+++ b/drivers/phy/tegra/xusb.c
@@ -1,5 +1,5 @@ 
 /*
- * Copyright (c) 2014-2015, NVIDIA CORPORATION.  All rights reserved.
+ * Copyright (c) 2014-2018, NVIDIA CORPORATION.  All rights reserved.
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms and conditions of the GNU General Public License,
@@ -546,9 +546,26 @@  static int tegra_xusb_usb2_port_parse_dt(struct tegra_xusb_usb2_port *usb2)
 {
 	struct tegra_xusb_port *port = &usb2->base;
 	struct device_node *np = port->dev.of_node;
+	const char *prop_string;
+	u32 value;
 
 	usb2->internal = of_property_read_bool(np, "nvidia,internal");
 
+	usb2->port_cap = USB_PORT_DISABLED; /* default */
+	if (!of_property_read_string(np, "mode", &prop_string)) {
+		if (!strcmp("host", prop_string))
+			usb2->port_cap = USB_HOST_CAP;
+		else if (!strcmp("device", prop_string))
+			usb2->port_cap = USB_DEVICE_CAP;
+		else if (!strcmp("otg", prop_string))
+			usb2->port_cap = USB_OTG_CAP;
+	}
+
+	if (!of_property_read_u32(np, "nvidia,usb3-port-fake", &value))
+		usb2->usb3_port_fake = value;
+	else
+		usb2->usb3_port_fake = -1; /* default */
+
 	usb2->supply = devm_regulator_get(&port->dev, "vbus");
 	return PTR_ERR_OR_ZERO(usb2->supply);
 }
@@ -976,7 +993,7 @@  int tegra_xusb_padctl_usb3_save_context(struct tegra_xusb_padctl *padctl,
 	if (padctl->soc->ops->usb3_save_context)
 		return padctl->soc->ops->usb3_save_context(padctl, port);
 
-	return -ENOSYS;
+	return -EINVAL;
 }
 EXPORT_SYMBOL_GPL(tegra_xusb_padctl_usb3_save_context);
 
@@ -986,7 +1003,7 @@  int tegra_xusb_padctl_hsic_set_idle(struct tegra_xusb_padctl *padctl,
 	if (padctl->soc->ops->hsic_set_idle)
 		return padctl->soc->ops->hsic_set_idle(padctl, port, idle);
 
-	return -ENOSYS;
+	return -EINVAL;
 }
 EXPORT_SYMBOL_GPL(tegra_xusb_padctl_hsic_set_idle);
 
@@ -997,10 +1014,60 @@  int tegra_xusb_padctl_usb3_set_lfps_detect(struct tegra_xusb_padctl *padctl,
 		return padctl->soc->ops->usb3_set_lfps_detect(padctl, port,
 							      enable);
 
-	return -ENOSYS;
+	return -EINVAL;
 }
 EXPORT_SYMBOL_GPL(tegra_xusb_padctl_usb3_set_lfps_detect);
 
+int tegra_xusb_padctl_set_vbus_override(struct tegra_xusb_padctl *padctl)
+{
+	if (padctl->soc->ops->vbus_override)
+		return padctl->soc->ops->vbus_override(padctl, true);
+
+	return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(tegra_xusb_padctl_set_vbus_override);
+
+int tegra_xusb_padctl_clear_vbus_override(struct tegra_xusb_padctl *padctl)
+{
+	if (padctl->soc->ops->vbus_override)
+		return padctl->soc->ops->vbus_override(padctl, false);
+
+	return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(tegra_xusb_padctl_clear_vbus_override);
+
+void tegra_phy_xusb_utmi_pad_power_on(struct phy *phy)
+{
+	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
+	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
+
+	if (padctl->soc->ops->utmi_pad_power_on)
+		padctl->soc->ops->utmi_pad_power_on(phy);
+}
+EXPORT_SYMBOL_GPL(tegra_phy_xusb_utmi_pad_power_on);
+
+void tegra_phy_xusb_utmi_pad_power_down(struct phy *phy)
+{
+	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
+	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
+
+	if (padctl->soc->ops->utmi_pad_power_down)
+		padctl->soc->ops->utmi_pad_power_down(phy);
+}
+EXPORT_SYMBOL_GPL(tegra_phy_xusb_utmi_pad_power_down);
+
+int tegra_phy_xusb_utmi_port_reset_quirk(struct phy *phy)
+{
+	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
+	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
+
+	if (padctl->soc->ops->utmi_port_reset_quirk)
+		return padctl->soc->ops->utmi_port_reset_quirk(phy);
+
+	return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(tegra_phy_xusb_utmi_port_reset_quirk);
+
 MODULE_AUTHOR("Thierry Reding <treding@nvidia.com>");
 MODULE_DESCRIPTION("Tegra XUSB Pad Controller driver");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/phy/tegra/xusb.h b/drivers/phy/tegra/xusb.h
index b49dbc3..74acc08 100644
--- a/drivers/phy/tegra/xusb.h
+++ b/drivers/phy/tegra/xusb.h
@@ -1,5 +1,5 @@ 
 /*
- * Copyright (c) 2014-2015, NVIDIA CORPORATION.  All rights reserved.
+ * Copyright (c) 2014-2018, NVIDIA CORPORATION.  All rights reserved.
  * Copyright (c) 2015, Google Inc.
  *
  * This program is free software; you can redistribute it and/or modify it
@@ -58,6 +58,7 @@  struct tegra_xusb_usb2_lane {
 	struct tegra_xusb_lane base;
 
 	u32 hs_curr_level_offset;
+	bool powered_on;
 };
 
 static inline struct tegra_xusb_usb2_lane *
@@ -267,11 +268,20 @@  struct tegra_xusb_port *
 tegra_xusb_find_port(struct tegra_xusb_padctl *padctl, const char *type,
 		     unsigned int index);
 
+enum tegra_xusb_usb_port_cap {
+	USB_PORT_DISABLED = 0,
+	USB_HOST_CAP,
+	USB_DEVICE_CAP,
+	USB_OTG_CAP,
+};
+
 struct tegra_xusb_usb2_port {
 	struct tegra_xusb_port base;
 
 	struct regulator *supply;
 	bool internal;
+	enum tegra_xusb_usb_port_cap port_cap;
+	int usb3_port_fake;
 };
 
 static inline struct tegra_xusb_usb2_port *
@@ -353,6 +363,10 @@  struct tegra_xusb_padctl_ops {
 			     unsigned int index, bool idle);
 	int (*usb3_set_lfps_detect)(struct tegra_xusb_padctl *padctl,
 				    unsigned int index, bool enable);
+	int (*vbus_override)(struct tegra_xusb_padctl *padctl, bool set);
+	void (*utmi_pad_power_on)(struct phy *phy);
+	void (*utmi_pad_power_down)(struct phy *phy);
+	int (*utmi_port_reset_quirk)(struct phy *phy);
 };
 
 struct tegra_xusb_padctl_soc {
diff --git a/include/linux/phy/tegra/xusb.h b/include/linux/phy/tegra/xusb.h
index 8e1a57a..143af44 100644
--- a/include/linux/phy/tegra/xusb.h
+++ b/include/linux/phy/tegra/xusb.h
@@ -1,5 +1,5 @@ 
 /*
- * Copyright (c) 2016, NVIDIA CORPORATION.  All rights reserved.
+ * Copyright (c) 2016-2018, NVIDIA CORPORATION.  All rights reserved.
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms and conditions of the GNU General Public License,
@@ -26,5 +26,10 @@  int tegra_xusb_padctl_hsic_set_idle(struct tegra_xusb_padctl *padctl,
 				    unsigned int port, bool idle);
 int tegra_xusb_padctl_usb3_set_lfps_detect(struct tegra_xusb_padctl *padctl,
 					   unsigned int port, bool enable);
+int tegra_xusb_padctl_set_vbus_override(struct tegra_xusb_padctl *padctl);
+int tegra_xusb_padctl_clear_vbus_override(struct tegra_xusb_padctl *padctl);
 
+void tegra_phy_xusb_utmi_pad_power_on(struct phy *phy);
+void tegra_phy_xusb_utmi_pad_power_down(struct phy *phy);
+int tegra_phy_xusb_utmi_port_reset_quirk(struct phy *phy);
 #endif /* PHY_TEGRA_XUSB_H */