[PATCH 0/6] Add Tegra194 XUSB host and pad controller support

[PATCH 0/6] Add Tegra194 XUSB host and pad controller support

[JC Kuo]

Hi,

This series introduces support for Tegra194 XUSB host and pad 
controller. Tegra194 XUSB host and pad controller are highly
similar to the controllers found on Tegra186. Therefore, we
decided to resue xhci-tegra.c and xusb-tegra186.c for Tegra194.

JC Kuo (6):
  xhci: tegra: Parameterize mailbox register addresses
  usb: host: xhci-tegra: Add Tegra194 XHCI support
  phy: tegra: xusb: Add Tegra194 support
  dt-bindings: phy: tegra: Add Tegra194 support
  arm64: tegra: Add XUSB and pad controller on Tegra194
  arm64: tegra: Enable XUSB host in P2972-0000 board

 .../phy/nvidia,tegra124-xusb-padctl.txt       |  16 +++
 .../arm64/boot/dts/nvidia/tegra194-p2888.dtsi |  31 ++++-
 .../boot/dts/nvidia/tegra194-p2972-0000.dts   |  59 ++++++++
 arch/arm64/boot/dts/nvidia/tegra194.dtsi      | 130 ++++++++++++++++++
 drivers/phy/tegra/Makefile                    |   1 +
 drivers/phy/tegra/xusb-tegra186.c             |  77 +++++++++++
 drivers/phy/tegra/xusb.c                      |  13 ++
 drivers/phy/tegra/xusb.h                      |   4 +
 drivers/usb/host/xhci-tegra.c                 |  88 +++++++++---
 9 files changed, 402 insertions(+), 17 deletions(-)

-- 
2.17.1

[PATCH 1/6] xhci: tegra: Parameterize mailbox register addresses

[JC Kuo]

Tegra194 XUSB host controller has rearranged mailbox registers. This
commit makes mailbox registers address a part of "soc" data so that
xhci-tegra driver can be used for Tegra194.

Signed-off-by: JC Kuo <jckuo@nvidia.com>
---
 drivers/usb/host/xhci-tegra.c | 58 +++++++++++++++++++++++++----------
 1 file changed, 42 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
index 2ff7c911fbd0..add6b8fb40e1 100644
--- a/drivers/usb/host/xhci-tegra.c
+++ b/drivers/usb/host/xhci-tegra.c
@@ -42,19 +42,18 @@
 #define XUSB_CFG_CSB_BASE_ADDR			0x800
 
 /* FPCI mailbox registers */
-#define XUSB_CFG_ARU_MBOX_CMD			0x0e4
+/* XUSB_CFG_ARU_MBOX_CMD */
 #define  MBOX_DEST_FALC				BIT(27)
 #define  MBOX_DEST_PME				BIT(28)
 #define  MBOX_DEST_SMI				BIT(29)
 #define  MBOX_DEST_XHCI				BIT(30)
 #define  MBOX_INT_EN				BIT(31)
-#define XUSB_CFG_ARU_MBOX_DATA_IN		0x0e8
+/* XUSB_CFG_ARU_MBOX_DATA_IN and XUSB_CFG_ARU_MBOX_DATA_OUT */
 #define  CMD_DATA_SHIFT				0
 #define  CMD_DATA_MASK				0xffffff
 #define  CMD_TYPE_SHIFT				24
 #define  CMD_TYPE_MASK				0xff
-#define XUSB_CFG_ARU_MBOX_DATA_OUT		0x0ec
-#define XUSB_CFG_ARU_MBOX_OWNER			0x0f0
+/* XUSB_CFG_ARU_MBOX_OWNER */
 #define  MBOX_OWNER_NONE			0
 #define  MBOX_OWNER_FW				1
 #define  MBOX_OWNER_SW				2
@@ -146,6 +145,13 @@ struct tegra_xusb_phy_type {
 	unsigned int num;
 };
 
+struct tega_xusb_mbox_regs {
+	u16 cmd;
+	u16 data_in;
+	u16 data_out;
+	u16 owner;
+};
+
 struct tegra_xusb_soc {
 	const char *firmware;
 	const char * const *supply_names;
@@ -160,6 +166,8 @@ struct tegra_xusb_soc {
 		} usb2, ulpi, hsic, usb3;
 	} ports;
 
+	struct tega_xusb_mbox_regs mbox;
+
 	bool scale_ss_clock;
 	bool has_ipfs;
 };
@@ -395,15 +403,15 @@ static int tegra_xusb_mbox_send(struct tegra_xusb *tegra,
 	 * ACK/NAK messages.
 	 */
 	if (!(msg->cmd == MBOX_CMD_ACK || msg->cmd == MBOX_CMD_NAK)) {
-		value = fpci_readl(tegra, XUSB_CFG_ARU_MBOX_OWNER);
+		value = fpci_readl(tegra, tegra->soc->mbox.owner);
 		if (value != MBOX_OWNER_NONE) {
 			dev_err(tegra->dev, "mailbox is busy\n");
 			return -EBUSY;
 		}
 
-		fpci_writel(tegra, MBOX_OWNER_SW, XUSB_CFG_ARU_MBOX_OWNER);
+		fpci_writel(tegra, MBOX_OWNER_SW, tegra->soc->mbox.owner);
 
-		value = fpci_readl(tegra, XUSB_CFG_ARU_MBOX_OWNER);
+		value = fpci_readl(tegra, tegra->soc->mbox.owner);
 		if (value != MBOX_OWNER_SW) {
 			dev_err(tegra->dev, "failed to acquire mailbox\n");
 			return -EBUSY;
@@ -413,17 +421,17 @@ static int tegra_xusb_mbox_send(struct tegra_xusb *tegra,
 	}
 
 	value = tegra_xusb_mbox_pack(msg);
-	fpci_writel(tegra, value, XUSB_CFG_ARU_MBOX_DATA_IN);
+	fpci_writel(tegra, value, tegra->soc->mbox.data_in);
 
-	value = fpci_readl(tegra, XUSB_CFG_ARU_MBOX_CMD);
+	value = fpci_readl(tegra, tegra->soc->mbox.cmd);
 	value |= MBOX_INT_EN | MBOX_DEST_FALC;
-	fpci_writel(tegra, value, XUSB_CFG_ARU_MBOX_CMD);
+	fpci_writel(tegra, value, tegra->soc->mbox.cmd);
 
 	if (wait_for_idle) {
 		unsigned long timeout = jiffies + msecs_to_jiffies(250);
 
 		while (time_before(jiffies, timeout)) {
-			value = fpci_readl(tegra, XUSB_CFG_ARU_MBOX_OWNER);
+			value = fpci_readl(tegra, tegra->soc->mbox.owner);
 			if (value == MBOX_OWNER_NONE)
 				break;
 
@@ -431,7 +439,7 @@ static int tegra_xusb_mbox_send(struct tegra_xusb *tegra,
 		}
 
 		if (time_after(jiffies, timeout))
-			value = fpci_readl(tegra, XUSB_CFG_ARU_MBOX_OWNER);
+			value = fpci_readl(tegra, tegra->soc->mbox.owner);
 
 		if (value != MBOX_OWNER_NONE)
 			return -ETIMEDOUT;
@@ -598,16 +606,16 @@ static irqreturn_t tegra_xusb_mbox_thread(int irq, void *data)
 
 	mutex_lock(&tegra->lock);
 
-	value = fpci_readl(tegra, XUSB_CFG_ARU_MBOX_DATA_OUT);
+	value = fpci_readl(tegra, tegra->soc->mbox.data_out);
 	tegra_xusb_mbox_unpack(&msg, value);
 
-	value = fpci_readl(tegra, XUSB_CFG_ARU_MBOX_CMD);
+	value = fpci_readl(tegra, tegra->soc->mbox.cmd);
 	value &= ~MBOX_DEST_SMI;
-	fpci_writel(tegra, value, XUSB_CFG_ARU_MBOX_CMD);
+	fpci_writel(tegra, value, tegra->soc->mbox.cmd);
 
 	/* clear mailbox owner if no ACK/NAK is required */
 	if (!tegra_xusb_mbox_cmd_requires_ack(msg.cmd))
-		fpci_writel(tegra, MBOX_OWNER_NONE, XUSB_CFG_ARU_MBOX_OWNER);
+		fpci_writel(tegra, MBOX_OWNER_NONE, tegra->soc->mbox.owner);
 
 	tegra_xusb_mbox_handle(tegra, &msg);
 
@@ -1375,6 +1383,12 @@ static const struct tegra_xusb_soc tegra124_soc = {
 	},
 	.scale_ss_clock = true,
 	.has_ipfs = true,
+	.mbox = {
+		.cmd = 0xe4,
+		.data_in = 0xe8,
+		.data_out = 0xec,
+		.owner = 0xf0,
+	},
 };
 MODULE_FIRMWARE("nvidia/tegra124/xusb.bin");
 
@@ -1407,6 +1421,12 @@ static const struct tegra_xusb_soc tegra210_soc = {
 	},
 	.scale_ss_clock = false,
 	.has_ipfs = true,
+	.mbox = {
+		.cmd = 0xe4,
+		.data_in = 0xe8,
+		.data_out = 0xec,
+		.owner = 0xf0,
+	},
 };
 MODULE_FIRMWARE("nvidia/tegra210/xusb.bin");
 
@@ -1432,6 +1452,12 @@ static const struct tegra_xusb_soc tegra186_soc = {
 	},
 	.scale_ss_clock = false,
 	.has_ipfs = false,
+	.mbox = {
+		.cmd = 0xe4,
+		.data_in = 0xe8,
+		.data_out = 0xec,
+		.owner = 0xf0,
+	},
 };
 
 static const struct of_device_id tegra_xusb_of_match[] = {
-- 
2.17.1

Re: [PATCH 1/6] xhci: tegra: Parameterize mailbox register addresses

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 480 bytes --]

On Wed, Oct 02, 2019 at 04:00:46PM +0800, JC Kuo wrote:
> Tegra194 XUSB host controller has rearranged mailbox registers. This
> commit makes mailbox registers address a part of "soc" data so that
> xhci-tegra driver can be used for Tegra194.
> 
> Signed-off-by: JC Kuo <jckuo@nvidia.com>
> ---
>  drivers/usb/host/xhci-tegra.c | 58 +++++++++++++++++++++++++----------
>  1 file changed, 42 insertions(+), 16 deletions(-)

Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 2/6] usb: host: xhci-tegra: Add Tegra194 XHCI support

[JC Kuo]

This commit adds Tegra194 XUSB host mode controller support. This is
very similar to the existing Tegra124/Tegra210/Tegra186 XHCI, except
  1. the number of ports and PHYs differs
  2. the IPFS wrapper being removed
  3. mailbox registers address changes

Signed-off-by: JC Kuo <jckuo@nvidia.com>
---
 drivers/usb/host/xhci-tegra.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
index add6b8fb40e1..f19d39874a3b 100644
--- a/drivers/usb/host/xhci-tegra.c
+++ b/drivers/usb/host/xhci-tegra.c
@@ -1460,10 +1460,40 @@ static const struct tegra_xusb_soc tegra186_soc = {
 	},
 };
 
+static const char * const tegra194_supply_names[] = {
+};
+
+static const struct tegra_xusb_phy_type tegra194_phy_types[] = {
+	{ .name = "usb3", .num = 4, },
+	{ .name = "usb2", .num = 4, },
+};
+
+static const struct tegra_xusb_soc tegra194_soc = {
+	.firmware = "nvidia/tegra194/xusb.bin",
+	.supply_names = tegra194_supply_names,
+	.num_supplies = ARRAY_SIZE(tegra194_supply_names),
+	.phy_types = tegra194_phy_types,
+	.num_types = ARRAY_SIZE(tegra194_phy_types),
+	.ports = {
+		.usb3 = { .offset = 0, .count = 4, },
+		.usb2 = { .offset = 4, .count = 4, },
+	},
+	.scale_ss_clock = false,
+	.has_ipfs = false,
+	.mbox = {
+		.cmd = 0x68,
+		.data_in = 0x6c,
+		.data_out = 0x70,
+		.owner = 0x74,
+	},
+};
+MODULE_FIRMWARE("nvidia/tegra194/xusb.bin");
+
 static const struct of_device_id tegra_xusb_of_match[] = {
 	{ .compatible = "nvidia,tegra124-xusb", .data = &tegra124_soc },
 	{ .compatible = "nvidia,tegra210-xusb", .data = &tegra210_soc },
 	{ .compatible = "nvidia,tegra186-xusb", .data = &tegra186_soc },
+	{ .compatible = "nvidia,tegra194-xusb", .data = &tegra194_soc },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, tegra_xusb_of_match);
-- 
2.17.1

Re: [PATCH 2/6] usb: host: xhci-tegra: Add Tegra194 XHCI support

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 538 bytes --]

On Wed, Oct 02, 2019 at 04:00:47PM +0800, JC Kuo wrote:
> This commit adds Tegra194 XUSB host mode controller support. This is
> very similar to the existing Tegra124/Tegra210/Tegra186 XHCI, except
>   1. the number of ports and PHYs differs
>   2. the IPFS wrapper being removed
>   3. mailbox registers address changes
> 
> Signed-off-by: JC Kuo <jckuo@nvidia.com>
> ---
>  drivers/usb/host/xhci-tegra.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)

Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 3/6] phy: tegra: xusb: Add Tegra194 support

[JC Kuo]

Add support for the XUSB pad controller found on Tegra194 SoCs. It is
mostly similar to the same IP found on Tegra186, but the number of
pads exposed differs, as do the programming sequences. Because most of
the Tegra194 XUSB PADCTL registers definition and programming sequence
are the same as Tegra186, Tegra194 XUSB PADCTL can share the same
driver, xusb-tegra186.c, with Tegra186 XUSB PADCTL.

Tegra194 XUSB PADCTL supports up to USB 3.1 Gen 2 speed, however, it
is possible for some platforms have long signal trace that could not
provide sufficient electrical environment for Gen 2 speed. This patch
introduce a new device node property "nvidia,disable-gen2" that can
be used to specifically disable Gen 2 speed for a particular USB 3.0
port so that the port can be limited to Gen 1 speed and avoid the
instability.

Signed-off-by: JC Kuo <jckuo@nvidia.com>
---
 drivers/phy/tegra/Makefile        |  1 +
 drivers/phy/tegra/xusb-tegra186.c | 77 +++++++++++++++++++++++++++++++
 drivers/phy/tegra/xusb.c          | 13 ++++++
 drivers/phy/tegra/xusb.h          |  4 ++
 4 files changed, 95 insertions(+)

diff --git a/drivers/phy/tegra/Makefile b/drivers/phy/tegra/Makefile
index 320dd389f34d..89b84067cb4c 100644
--- a/drivers/phy/tegra/Makefile
+++ b/drivers/phy/tegra/Makefile
@@ -6,4 +6,5 @@ phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_124_SOC) += xusb-tegra124.o
 phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_132_SOC) += xusb-tegra124.o
 phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_210_SOC) += xusb-tegra210.o
 phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_186_SOC) += xusb-tegra186.o
+phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_194_SOC) += xusb-tegra186.o
 obj-$(CONFIG_PHY_TEGRA194_P2U) += phy-tegra194-p2u.o
diff --git a/drivers/phy/tegra/xusb-tegra186.c b/drivers/phy/tegra/xusb-tegra186.c
index 6f3afaf9398f..4e27acf398b2 100644
--- a/drivers/phy/tegra/xusb-tegra186.c
+++ b/drivers/phy/tegra/xusb-tegra186.c
@@ -64,6 +64,13 @@
 #define  SSPX_ELPG_CLAMP_EN_EARLY(x)		BIT(1 + (x) * 3)
 #define  SSPX_ELPG_VCORE_DOWN(x)		BIT(2 + (x) * 3)
 
+#if IS_ENABLED(CONFIG_ARCH_TEGRA_194_SOC)
+#define XUSB_PADCTL_SS_PORT_CFG			0x2c
+#define   PORTX_SPEED_SUPPORT_SHIFT(x)		((x) * 4)
+#define   PORTX_SPEED_SUPPORT_MASK		(0x3)
+#define     PORT_SPEED_SUPPORT_GEN1		(0x0)
+#endif
+
 #define XUSB_PADCTL_USB2_OTG_PADX_CTL0(x)	(0x88 + (x) * 0x40)
 #define  HS_CURR_LEVEL(x)			((x) & 0x3f)
 #define  TERM_SEL				BIT(25)
@@ -635,6 +642,17 @@ static int tegra186_usb3_phy_power_on(struct phy *phy)
 
 	padctl_writel(padctl, value, XUSB_PADCTL_SS_PORT_CAP);
 
+#if IS_ENABLED(CONFIG_ARCH_TEGRA_194_SOC)
+	if (padctl->soc == &tegra194_xusb_padctl_soc && port->disable_gen2) {
+		value = padctl_readl(padctl, XUSB_PADCTL_SS_PORT_CFG);
+		value &= ~(PORTX_SPEED_SUPPORT_MASK <<
+			PORTX_SPEED_SUPPORT_SHIFT(index));
+		value |= (PORT_SPEED_SUPPORT_GEN1 <<
+			PORTX_SPEED_SUPPORT_SHIFT(index));
+		padctl_writel(padctl, value, XUSB_PADCTL_SS_PORT_CFG);
+	}
+#endif
+
 	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM_1);
 	value &= ~SSPX_ELPG_VCORE_DOWN(index);
 	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM_1);
@@ -894,6 +912,65 @@ const struct tegra_xusb_padctl_soc tegra186_xusb_padctl_soc = {
 };
 EXPORT_SYMBOL_GPL(tegra186_xusb_padctl_soc);
 
+#if IS_ENABLED(CONFIG_ARCH_TEGRA_194_SOC)
+static const char * const tegra194_xusb_padctl_supply_names[] = {
+	"avdd-usb",
+	"vclamp-usb",
+};
+
+static const struct tegra_xusb_lane_soc tegra194_usb2_lanes[] = {
+	TEGRA186_LANE("usb2-0", 0,  0, 0, usb2),
+	TEGRA186_LANE("usb2-1", 0,  0, 0, usb2),
+	TEGRA186_LANE("usb2-2", 0,  0, 0, usb2),
+	TEGRA186_LANE("usb2-3", 0,  0, 0, usb2),
+};
+
+static const struct tegra_xusb_pad_soc tegra194_usb2_pad = {
+	.name = "usb2",
+	.num_lanes = ARRAY_SIZE(tegra194_usb2_lanes),
+	.lanes = tegra194_usb2_lanes,
+	.ops = &tegra186_usb2_pad_ops,
+};
+
+static const struct tegra_xusb_lane_soc tegra194_usb3_lanes[] = {
+	TEGRA186_LANE("usb3-0", 0,  0, 0, usb3),
+	TEGRA186_LANE("usb3-1", 0,  0, 0, usb3),
+	TEGRA186_LANE("usb3-2", 0,  0, 0, usb3),
+	TEGRA186_LANE("usb3-3", 0,  0, 0, usb3),
+};
+
+static const struct tegra_xusb_pad_soc tegra194_usb3_pad = {
+	.name = "usb3",
+	.num_lanes = ARRAY_SIZE(tegra194_usb3_lanes),
+	.lanes = tegra194_usb3_lanes,
+	.ops = &tegra186_usb3_pad_ops,
+};
+
+static const struct tegra_xusb_pad_soc * const tegra194_pads[] = {
+	&tegra194_usb2_pad,
+	&tegra194_usb3_pad,
+};
+
+const struct tegra_xusb_padctl_soc tegra194_xusb_padctl_soc = {
+	.num_pads = ARRAY_SIZE(tegra194_pads),
+	.pads = tegra194_pads,
+	.ports = {
+		.usb2 = {
+			.ops = &tegra186_usb2_port_ops,
+			.count = 4,
+		},
+		.usb3 = {
+			.ops = &tegra186_usb3_port_ops,
+			.count = 4,
+		},
+	},
+	.ops = &tegra186_xusb_padctl_ops,
+	.supply_names = tegra194_xusb_padctl_supply_names,
+	.num_supplies = ARRAY_SIZE(tegra194_xusb_padctl_supply_names),
+};
+EXPORT_SYMBOL_GPL(tegra194_xusb_padctl_soc);
+#endif
+
 MODULE_AUTHOR("JC Kuo <jckuo@nvidia.com>");
 MODULE_DESCRIPTION("NVIDIA Tegra186 XUSB Pad Controller driver");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
index 2ea8497af82a..266c08074b28 100644
--- a/drivers/phy/tegra/xusb.c
+++ b/drivers/phy/tegra/xusb.c
@@ -65,6 +65,12 @@ static const struct of_device_id tegra_xusb_padctl_of_match[] = {
 		.compatible = "nvidia,tegra186-xusb-padctl",
 		.data = &tegra186_xusb_padctl_soc,
 	},
+#endif
+#if defined(CONFIG_ARCH_TEGRA_194_SOC)
+	{
+		.compatible = "nvidia,tegra194-xusb-padctl",
+		.data = &tegra194_xusb_padctl_soc,
+	},
 #endif
 	{ }
 };
@@ -739,6 +745,13 @@ static int tegra_xusb_usb3_port_parse_dt(struct tegra_xusb_usb3_port *usb3)
 
 	usb3->internal = of_property_read_bool(np, "nvidia,internal");
 
+#if IS_ENABLED(CONFIG_ARCH_TEGRA_194_SOC)
+	if (port->padctl->soc == &tegra194_xusb_padctl_soc) {
+		usb3->disable_gen2 = of_property_read_bool(np,
+							"nvidia,disable-gen2");
+	}
+#endif
+
 	usb3->supply = devm_regulator_get(&port->dev, "vbus");
 	return PTR_ERR_OR_ZERO(usb3->supply);
 }
diff --git a/drivers/phy/tegra/xusb.h b/drivers/phy/tegra/xusb.h
index 093076ca27fd..6b71978ba15d 100644
--- a/drivers/phy/tegra/xusb.h
+++ b/drivers/phy/tegra/xusb.h
@@ -332,6 +332,7 @@ struct tegra_xusb_usb3_port {
 	bool context_saved;
 	unsigned int port;
 	bool internal;
+	bool disable_gen2;
 
 	u32 tap1;
 	u32 amp;
@@ -444,5 +445,8 @@ extern const struct tegra_xusb_padctl_soc tegra210_xusb_padctl_soc;
 #if defined(CONFIG_ARCH_TEGRA_186_SOC)
 extern const struct tegra_xusb_padctl_soc tegra186_xusb_padctl_soc;
 #endif
+#if defined(CONFIG_ARCH_TEGRA_194_SOC)
+extern const struct tegra_xusb_padctl_soc tegra194_xusb_padctl_soc;
+#endif
 
 #endif /* __PHY_TEGRA_XUSB_H */
-- 
2.17.1

Re: [PATCH 3/6] phy: tegra: xusb: Add Tegra194 support

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 9428 bytes --]

On Wed, Oct 02, 2019 at 04:00:48PM +0800, JC Kuo wrote:
> Add support for the XUSB pad controller found on Tegra194 SoCs. It is
> mostly similar to the same IP found on Tegra186, but the number of
> pads exposed differs, as do the programming sequences. Because most of
> the Tegra194 XUSB PADCTL registers definition and programming sequence
> are the same as Tegra186, Tegra194 XUSB PADCTL can share the same
> driver, xusb-tegra186.c, with Tegra186 XUSB PADCTL.
> 
> Tegra194 XUSB PADCTL supports up to USB 3.1 Gen 2 speed, however, it
> is possible for some platforms have long signal trace that could not
> provide sufficient electrical environment for Gen 2 speed. This patch
> introduce a new device node property "nvidia,disable-gen2" that can
> be used to specifically disable Gen 2 speed for a particular USB 3.0
> port so that the port can be limited to Gen 1 speed and avoid the
> instability.
> 
> Signed-off-by: JC Kuo <jckuo@nvidia.com>
> ---
>  drivers/phy/tegra/Makefile        |  1 +
>  drivers/phy/tegra/xusb-tegra186.c | 77 +++++++++++++++++++++++++++++++
>  drivers/phy/tegra/xusb.c          | 13 ++++++
>  drivers/phy/tegra/xusb.h          |  4 ++
>  4 files changed, 95 insertions(+)
> 
> diff --git a/drivers/phy/tegra/Makefile b/drivers/phy/tegra/Makefile
> index 320dd389f34d..89b84067cb4c 100644
> --- a/drivers/phy/tegra/Makefile
> +++ b/drivers/phy/tegra/Makefile
> @@ -6,4 +6,5 @@ phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_124_SOC) += xusb-tegra124.o
>  phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_132_SOC) += xusb-tegra124.o
>  phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_210_SOC) += xusb-tegra210.o
>  phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_186_SOC) += xusb-tegra186.o
> +phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_194_SOC) += xusb-tegra186.o
>  obj-$(CONFIG_PHY_TEGRA194_P2U) += phy-tegra194-p2u.o
> diff --git a/drivers/phy/tegra/xusb-tegra186.c b/drivers/phy/tegra/xusb-tegra186.c
> index 6f3afaf9398f..4e27acf398b2 100644
> --- a/drivers/phy/tegra/xusb-tegra186.c
> +++ b/drivers/phy/tegra/xusb-tegra186.c
> @@ -64,6 +64,13 @@
>  #define  SSPX_ELPG_CLAMP_EN_EARLY(x)		BIT(1 + (x) * 3)
>  #define  SSPX_ELPG_VCORE_DOWN(x)		BIT(2 + (x) * 3)
>  
> +#if IS_ENABLED(CONFIG_ARCH_TEGRA_194_SOC)
> +#define XUSB_PADCTL_SS_PORT_CFG			0x2c
> +#define   PORTX_SPEED_SUPPORT_SHIFT(x)		((x) * 4)
> +#define   PORTX_SPEED_SUPPORT_MASK		(0x3)
> +#define     PORT_SPEED_SUPPORT_GEN1		(0x0)
> +#endif

I wouldn't bother protecting these with the #if/#endif.

> +
>  #define XUSB_PADCTL_USB2_OTG_PADX_CTL0(x)	(0x88 + (x) * 0x40)
>  #define  HS_CURR_LEVEL(x)			((x) & 0x3f)
>  #define  TERM_SEL				BIT(25)
> @@ -635,6 +642,17 @@ static int tegra186_usb3_phy_power_on(struct phy *phy)
>  
>  	padctl_writel(padctl, value, XUSB_PADCTL_SS_PORT_CAP);
>  
> +#if IS_ENABLED(CONFIG_ARCH_TEGRA_194_SOC)
> +	if (padctl->soc == &tegra194_xusb_padctl_soc && port->disable_gen2) {
> +		value = padctl_readl(padctl, XUSB_PADCTL_SS_PORT_CFG);
> +		value &= ~(PORTX_SPEED_SUPPORT_MASK <<
> +			PORTX_SPEED_SUPPORT_SHIFT(index));
> +		value |= (PORT_SPEED_SUPPORT_GEN1 <<
> +			PORTX_SPEED_SUPPORT_SHIFT(index));
> +		padctl_writel(padctl, value, XUSB_PADCTL_SS_PORT_CFG);
> +	}
> +#endif

Same here. Also, I think you can drop the extra check for padctl->soc
and only rely on port->disable_gen2. This is not a lot of code, so might
as well make our life simpler by building it unconditionally.

On another note: checking the padctl->soc pointer against a SoC-specific
structure is a neat way to check for this support. However, it's not
very flexible. Consider what happens when the next chip is released. I
think we can assume that it will also support gen 2 and may also require
some boards to disable gen 2 because of long signal traces. In order to
accomodate that, you'd have to extend this check with another comparison
to that new SoC structure.

A better alternative would be to add this as a "feature" flag to the SoC
structure:

	struct tegra_xusb_pad_soc {
		...
		bool supports_gen2;
	};

Presumably every SoC that supports gen 2 will also need support for
explicitly disabling gen 2 if the board doesn't support it, so you can
use that new feature flag to conditionalize this code.

This way, the next SoC generation can support can simply be added by
setting supports_gen2 = true, without requiring any actual code changes
(unless of course if it supports new features).

Multi-SoC support is also a good argument for dropping the #if/#endif
protection, because those would need to be extended for the next SoC
generation as well.

> +
>  	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM_1);
>  	value &= ~SSPX_ELPG_VCORE_DOWN(index);
>  	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM_1);
> @@ -894,6 +912,65 @@ const struct tegra_xusb_padctl_soc tegra186_xusb_padctl_soc = {
>  };
>  EXPORT_SYMBOL_GPL(tegra186_xusb_padctl_soc);
>  
> +#if IS_ENABLED(CONFIG_ARCH_TEGRA_194_SOC)

It doesn't look like we have this type of protection for Tegra186. It
might make sense to add a patch to this series (before this patch) to
slightly clean up the Tegra186 SoC data (reshuffle the data so that a
single #if/#endif block can be used, like you do for Tegra194).

But we can equally well do that in a follow-up.

> +static const char * const tegra194_xusb_padctl_supply_names[] = {
> +	"avdd-usb",
> +	"vclamp-usb",
> +};
> +
> +static const struct tegra_xusb_lane_soc tegra194_usb2_lanes[] = {
> +	TEGRA186_LANE("usb2-0", 0,  0, 0, usb2),
> +	TEGRA186_LANE("usb2-1", 0,  0, 0, usb2),
> +	TEGRA186_LANE("usb2-2", 0,  0, 0, usb2),
> +	TEGRA186_LANE("usb2-3", 0,  0, 0, usb2),
> +};
> +
> +static const struct tegra_xusb_pad_soc tegra194_usb2_pad = {
> +	.name = "usb2",
> +	.num_lanes = ARRAY_SIZE(tegra194_usb2_lanes),
> +	.lanes = tegra194_usb2_lanes,
> +	.ops = &tegra186_usb2_pad_ops,
> +};
> +
> +static const struct tegra_xusb_lane_soc tegra194_usb3_lanes[] = {
> +	TEGRA186_LANE("usb3-0", 0,  0, 0, usb3),
> +	TEGRA186_LANE("usb3-1", 0,  0, 0, usb3),
> +	TEGRA186_LANE("usb3-2", 0,  0, 0, usb3),
> +	TEGRA186_LANE("usb3-3", 0,  0, 0, usb3),
> +};
> +
> +static const struct tegra_xusb_pad_soc tegra194_usb3_pad = {
> +	.name = "usb3",
> +	.num_lanes = ARRAY_SIZE(tegra194_usb3_lanes),
> +	.lanes = tegra194_usb3_lanes,
> +	.ops = &tegra186_usb3_pad_ops,
> +};
> +
> +static const struct tegra_xusb_pad_soc * const tegra194_pads[] = {
> +	&tegra194_usb2_pad,
> +	&tegra194_usb3_pad,
> +};
> +
> +const struct tegra_xusb_padctl_soc tegra194_xusb_padctl_soc = {
> +	.num_pads = ARRAY_SIZE(tegra194_pads),
> +	.pads = tegra194_pads,
> +	.ports = {
> +		.usb2 = {
> +			.ops = &tegra186_usb2_port_ops,
> +			.count = 4,
> +		},
> +		.usb3 = {
> +			.ops = &tegra186_usb3_port_ops,
> +			.count = 4,
> +		},
> +	},
> +	.ops = &tegra186_xusb_padctl_ops,
> +	.supply_names = tegra194_xusb_padctl_supply_names,
> +	.num_supplies = ARRAY_SIZE(tegra194_xusb_padctl_supply_names),
> +};
> +EXPORT_SYMBOL_GPL(tegra194_xusb_padctl_soc);
> +#endif
> +
>  MODULE_AUTHOR("JC Kuo <jckuo@nvidia.com>");
>  MODULE_DESCRIPTION("NVIDIA Tegra186 XUSB Pad Controller driver");
>  MODULE_LICENSE("GPL v2");
> diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
> index 2ea8497af82a..266c08074b28 100644
> --- a/drivers/phy/tegra/xusb.c
> +++ b/drivers/phy/tegra/xusb.c
> @@ -65,6 +65,12 @@ static const struct of_device_id tegra_xusb_padctl_of_match[] = {
>  		.compatible = "nvidia,tegra186-xusb-padctl",
>  		.data = &tegra186_xusb_padctl_soc,
>  	},
> +#endif
> +#if defined(CONFIG_ARCH_TEGRA_194_SOC)
> +	{
> +		.compatible = "nvidia,tegra194-xusb-padctl",
> +		.data = &tegra194_xusb_padctl_soc,
> +	},
>  #endif
>  	{ }
>  };
> @@ -739,6 +745,13 @@ static int tegra_xusb_usb3_port_parse_dt(struct tegra_xusb_usb3_port *usb3)
>  
>  	usb3->internal = of_property_read_bool(np, "nvidia,internal");
>  
> +#if IS_ENABLED(CONFIG_ARCH_TEGRA_194_SOC)
> +	if (port->padctl->soc == &tegra194_xusb_padctl_soc) {
> +		usb3->disable_gen2 = of_property_read_bool(np,
> +							"nvidia,disable-gen2");
> +	}
> +#endif

Do we really need the #if/#endif here? Or the conditional for that
matter? nvidia,disable-gen2 is only defined for Tegra194, so any earlier
SoCs are not going to have one, in which case the above code would just
set usb3->disable_gen2 to false (the default).

Removing the conditional allows you to have the above on a single line.

Thierry

> +
>  	usb3->supply = devm_regulator_get(&port->dev, "vbus");
>  	return PTR_ERR_OR_ZERO(usb3->supply);
>  }
> diff --git a/drivers/phy/tegra/xusb.h b/drivers/phy/tegra/xusb.h
> index 093076ca27fd..6b71978ba15d 100644
> --- a/drivers/phy/tegra/xusb.h
> +++ b/drivers/phy/tegra/xusb.h
> @@ -332,6 +332,7 @@ struct tegra_xusb_usb3_port {
>  	bool context_saved;
>  	unsigned int port;
>  	bool internal;
> +	bool disable_gen2;
>  
>  	u32 tap1;
>  	u32 amp;
> @@ -444,5 +445,8 @@ extern const struct tegra_xusb_padctl_soc tegra210_xusb_padctl_soc;
>  #if defined(CONFIG_ARCH_TEGRA_186_SOC)
>  extern const struct tegra_xusb_padctl_soc tegra186_xusb_padctl_soc;
>  #endif
> +#if defined(CONFIG_ARCH_TEGRA_194_SOC)
> +extern const struct tegra_xusb_padctl_soc tegra194_xusb_padctl_soc;
> +#endif
>  
>  #endif /* __PHY_TEGRA_XUSB_H */
> -- 
> 2.17.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 3/6] phy: tegra: xusb: Add Tegra194 support

[JC Kuo]

On 10/2/19 6:02 PM, Thierry Reding wrote:
> On Wed, Oct 02, 2019 at 04:00:48PM +0800, JC Kuo wrote:
>> Add support for the XUSB pad controller found on Tegra194 SoCs. It is
>> mostly similar to the same IP found on Tegra186, but the number of
>> pads exposed differs, as do the programming sequences. Because most of
>> the Tegra194 XUSB PADCTL registers definition and programming sequence
>> are the same as Tegra186, Tegra194 XUSB PADCTL can share the same
>> driver, xusb-tegra186.c, with Tegra186 XUSB PADCTL.
>>
>> Tegra194 XUSB PADCTL supports up to USB 3.1 Gen 2 speed, however, it
>> is possible for some platforms have long signal trace that could not
>> provide sufficient electrical environment for Gen 2 speed. This patch
>> introduce a new device node property "nvidia,disable-gen2" that can
>> be used to specifically disable Gen 2 speed for a particular USB 3.0
>> port so that the port can be limited to Gen 1 speed and avoid the
>> instability.
>>
>> Signed-off-by: JC Kuo <jckuo@nvidia.com>
>> ---
>>  drivers/phy/tegra/Makefile        |  1 +
>>  drivers/phy/tegra/xusb-tegra186.c | 77 +++++++++++++++++++++++++++++++
>>  drivers/phy/tegra/xusb.c          | 13 ++++++
>>  drivers/phy/tegra/xusb.h          |  4 ++
>>  4 files changed, 95 insertions(+)
>>
>> diff --git a/drivers/phy/tegra/Makefile b/drivers/phy/tegra/Makefile
>> index 320dd389f34d..89b84067cb4c 100644
>> --- a/drivers/phy/tegra/Makefile
>> +++ b/drivers/phy/tegra/Makefile
>> @@ -6,4 +6,5 @@ phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_124_SOC) += xusb-tegra124.o
>>  phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_132_SOC) += xusb-tegra124.o
>>  phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_210_SOC) += xusb-tegra210.o
>>  phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_186_SOC) += xusb-tegra186.o
>> +phy-tegra-xusb-$(CONFIG_ARCH_TEGRA_194_SOC) += xusb-tegra186.o
>>  obj-$(CONFIG_PHY_TEGRA194_P2U) += phy-tegra194-p2u.o
>> diff --git a/drivers/phy/tegra/xusb-tegra186.c b/drivers/phy/tegra/xusb-tegra186.c
>> index 6f3afaf9398f..4e27acf398b2 100644
>> --- a/drivers/phy/tegra/xusb-tegra186.c
>> +++ b/drivers/phy/tegra/xusb-tegra186.c
>> @@ -64,6 +64,13 @@
>>  #define  SSPX_ELPG_CLAMP_EN_EARLY(x)		BIT(1 + (x) * 3)
>>  #define  SSPX_ELPG_VCORE_DOWN(x)		BIT(2 + (x) * 3)
>>  
>> +#if IS_ENABLED(CONFIG_ARCH_TEGRA_194_SOC)
>> +#define XUSB_PADCTL_SS_PORT_CFG			0x2c
>> +#define   PORTX_SPEED_SUPPORT_SHIFT(x)		((x) * 4)
>> +#define   PORTX_SPEED_SUPPORT_MASK		(0x3)
>> +#define     PORT_SPEED_SUPPORT_GEN1		(0x0)
>> +#endif
> 
> I wouldn't bother protecting these with the #if/#endif.
It will be removed in the next revision.
> 
>> +
>>  #define XUSB_PADCTL_USB2_OTG_PADX_CTL0(x)	(0x88 + (x) * 0x40)
>>  #define  HS_CURR_LEVEL(x)			((x) & 0x3f)
>>  #define  TERM_SEL				BIT(25)
>> @@ -635,6 +642,17 @@ static int tegra186_usb3_phy_power_on(struct phy *phy)
>>  
>>  	padctl_writel(padctl, value, XUSB_PADCTL_SS_PORT_CAP);
>>  
>> +#if IS_ENABLED(CONFIG_ARCH_TEGRA_194_SOC)
>> +	if (padctl->soc == &tegra194_xusb_padctl_soc && port->disable_gen2) {
>> +		value = padctl_readl(padctl, XUSB_PADCTL_SS_PORT_CFG);
>> +		value &= ~(PORTX_SPEED_SUPPORT_MASK <<
>> +			PORTX_SPEED_SUPPORT_SHIFT(index));
>> +		value |= (PORT_SPEED_SUPPORT_GEN1 <<
>> +			PORTX_SPEED_SUPPORT_SHIFT(index));
>> +		padctl_writel(padctl, value, XUSB_PADCTL_SS_PORT_CFG);
>> +	}
>> +#endif
> 
> Same here. Also, I think you can drop the extra check for padctl->soc
> and only rely on port->disable_gen2. This is not a lot of code, so might
> as well make our life simpler by building it unconditionally.
> 
> On another note: checking the padctl->soc pointer against a SoC-specific
> structure is a neat way to check for this support. However, it's not
> very flexible. Consider what happens when the next chip is released. I
> think we can assume that it will also support gen 2 and may also require
> some boards to disable gen 2 because of long signal traces. In order to
> accomodate that, you'd have to extend this check with another comparison
> to that new SoC structure.
> 
> A better alternative would be to add this as a "feature" flag to the SoC
> structure:
> 
> 	struct tegra_xusb_pad_soc {
> 		...
> 		bool supports_gen2;
> 	};
> 
> Presumably every SoC that supports gen 2 will also need support for
> explicitly disabling gen 2 if the board doesn't support it, so you can
> use that new feature flag to conditionalize this code.
> 
> This way, the next SoC generation can support can simply be added by
> setting supports_gen2 = true, without requiring any actual code changes
> (unless of course if it supports new features).
> 
> Multi-SoC support is also a good argument for dropping the #if/#endif
> protection, because those would need to be extended for the next SoC
> generation as well.
> 
Thanks Thierry. This implementation is better. I will take it in the next revision.

>> +
>>  	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM_1);
>>  	value &= ~SSPX_ELPG_VCORE_DOWN(index);
>>  	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM_1);
>> @@ -894,6 +912,65 @@ const struct tegra_xusb_padctl_soc tegra186_xusb_padctl_soc = {
>>  };
>>  EXPORT_SYMBOL_GPL(tegra186_xusb_padctl_soc);
>>  
>> +#if IS_ENABLED(CONFIG_ARCH_TEGRA_194_SOC)
> 
> It doesn't look like we have this type of protection for Tegra186. It
> might make sense to add a patch to this series (before this patch) to
> slightly clean up the Tegra186 SoC data (reshuffle the data so that a
> single #if/#endif block can be used, like you do for Tegra194).
Okay, I will do it in the next revision.
> 
> But we can equally well do that in a follow-up.
> 
>> +static const char * const tegra194_xusb_padctl_supply_names[] = {
>> +	"avdd-usb",
>> +	"vclamp-usb",
>> +};
>> +
>> +static const struct tegra_xusb_lane_soc tegra194_usb2_lanes[] = {
>> +	TEGRA186_LANE("usb2-0", 0,  0, 0, usb2),
>> +	TEGRA186_LANE("usb2-1", 0,  0, 0, usb2),
>> +	TEGRA186_LANE("usb2-2", 0,  0, 0, usb2),
>> +	TEGRA186_LANE("usb2-3", 0,  0, 0, usb2),
>> +};
>> +
>> +static const struct tegra_xusb_pad_soc tegra194_usb2_pad = {
>> +	.name = "usb2",
>> +	.num_lanes = ARRAY_SIZE(tegra194_usb2_lanes),
>> +	.lanes = tegra194_usb2_lanes,
>> +	.ops = &tegra186_usb2_pad_ops,
>> +};
>> +
>> +static const struct tegra_xusb_lane_soc tegra194_usb3_lanes[] = {
>> +	TEGRA186_LANE("usb3-0", 0,  0, 0, usb3),
>> +	TEGRA186_LANE("usb3-1", 0,  0, 0, usb3),
>> +	TEGRA186_LANE("usb3-2", 0,  0, 0, usb3),
>> +	TEGRA186_LANE("usb3-3", 0,  0, 0, usb3),
>> +};
>> +
>> +static const struct tegra_xusb_pad_soc tegra194_usb3_pad = {
>> +	.name = "usb3",
>> +	.num_lanes = ARRAY_SIZE(tegra194_usb3_lanes),
>> +	.lanes = tegra194_usb3_lanes,
>> +	.ops = &tegra186_usb3_pad_ops,
>> +};
>> +
>> +static const struct tegra_xusb_pad_soc * const tegra194_pads[] = {
>> +	&tegra194_usb2_pad,
>> +	&tegra194_usb3_pad,
>> +};
>> +
>> +const struct tegra_xusb_padctl_soc tegra194_xusb_padctl_soc = {
>> +	.num_pads = ARRAY_SIZE(tegra194_pads),
>> +	.pads = tegra194_pads,
>> +	.ports = {
>> +		.usb2 = {
>> +			.ops = &tegra186_usb2_port_ops,
>> +			.count = 4,
>> +		},
>> +		.usb3 = {
>> +			.ops = &tegra186_usb3_port_ops,
>> +			.count = 4,
>> +		},
>> +	},
>> +	.ops = &tegra186_xusb_padctl_ops,
>> +	.supply_names = tegra194_xusb_padctl_supply_names,
>> +	.num_supplies = ARRAY_SIZE(tegra194_xusb_padctl_supply_names),
>> +};
>> +EXPORT_SYMBOL_GPL(tegra194_xusb_padctl_soc);
>> +#endif
>> +
>>  MODULE_AUTHOR("JC Kuo <jckuo@nvidia.com>");
>>  MODULE_DESCRIPTION("NVIDIA Tegra186 XUSB Pad Controller driver");
>>  MODULE_LICENSE("GPL v2");
>> diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
>> index 2ea8497af82a..266c08074b28 100644
>> --- a/drivers/phy/tegra/xusb.c
>> +++ b/drivers/phy/tegra/xusb.c
>> @@ -65,6 +65,12 @@ static const struct of_device_id tegra_xusb_padctl_of_match[] = {
>>  		.compatible = "nvidia,tegra186-xusb-padctl",
>>  		.data = &tegra186_xusb_padctl_soc,
>>  	},
>> +#endif
>> +#if defined(CONFIG_ARCH_TEGRA_194_SOC)
>> +	{
>> +		.compatible = "nvidia,tegra194-xusb-padctl",
>> +		.data = &tegra194_xusb_padctl_soc,
>> +	},
>>  #endif
>>  	{ }
>>  };
>> @@ -739,6 +745,13 @@ static int tegra_xusb_usb3_port_parse_dt(struct tegra_xusb_usb3_port *usb3)
>>  
>>  	usb3->internal = of_property_read_bool(np, "nvidia,internal");
>>  
>> +#if IS_ENABLED(CONFIG_ARCH_TEGRA_194_SOC)
>> +	if (port->padctl->soc == &tegra194_xusb_padctl_soc) {
>> +		usb3->disable_gen2 = of_property_read_bool(np,
>> +							"nvidia,disable-gen2");
>> +	}
>> +#endif
> 
> Do we really need the #if/#endif here? Or the conditional for that
> matter? nvidia,disable-gen2 is only defined for Tegra194, so any earlier
> SoCs are not going to have one, in which case the above code would just
> set usb3->disable_gen2 to false (the default).
> 
> Removing the conditional allows you to have the above on a single line.
> 
I will remove #if/#endif and the if() in the next revision. Thanks.
> Thierry
> 
>> +
>>  	usb3->supply = devm_regulator_get(&port->dev, "vbus");
>>  	return PTR_ERR_OR_ZERO(usb3->supply);
>>  }
>> diff --git a/drivers/phy/tegra/xusb.h b/drivers/phy/tegra/xusb.h
>> index 093076ca27fd..6b71978ba15d 100644
>> --- a/drivers/phy/tegra/xusb.h
>> +++ b/drivers/phy/tegra/xusb.h
>> @@ -332,6 +332,7 @@ struct tegra_xusb_usb3_port {
>>  	bool context_saved;
>>  	unsigned int port;
>>  	bool internal;
>> +	bool disable_gen2;
>>  
>>  	u32 tap1;
>>  	u32 amp;
>> @@ -444,5 +445,8 @@ extern const struct tegra_xusb_padctl_soc tegra210_xusb_padctl_soc;
>>  #if defined(CONFIG_ARCH_TEGRA_186_SOC)
>>  extern const struct tegra_xusb_padctl_soc tegra186_xusb_padctl_soc;
>>  #endif
>> +#if defined(CONFIG_ARCH_TEGRA_194_SOC)
>> +extern const struct tegra_xusb_padctl_soc tegra194_xusb_padctl_soc;
>> +#endif
>>  
>>  #endif /* __PHY_TEGRA_XUSB_H */
>> -- 
>> 2.17.1
>>

[PATCH 4/6] dt-bindings: phy: tegra: Add Tegra194 support

[JC Kuo]

Extend the bindings to cover the set of features found in Tegra194.
Note that, technically, there are four more supplies connected to the
XUSB pad controller (DVDD_PEX, DVDD_PEX_PLL, HVDD_PEX and HVDD_PEX_PLL)
, but the power sequencing requirements of Tegra194 require these to be
under the control of the PMIC.

Tegra194 XUSB PADCTL supports up to USB 3.1 Gen 2 speed, however, it is
possible for some platforms have long signal trace that could not
provide sufficient electrical environment for Gen 2 speed. To deal with
this, a new device node property "nvidia,disable-gen2" was added to
Tegra194 that be used to specifically disable Gen 2 speed for a
particular USB 3.0 port so that the port can be limited to Gen 1 speed
and avoid the instability.

Signed-off-by: JC Kuo <jckuo@nvidia.com>
---
 .../bindings/phy/nvidia,tegra124-xusb-padctl.txt | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt b/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
index 9fb682e47c29..3bef37e7c365 100644
--- a/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
+++ b/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
@@ -37,6 +37,7 @@ Required properties:
   - Tegra132: "nvidia,tegra132-xusb-padctl", "nvidia,tegra124-xusb-padctl"
   - Tegra210: "nvidia,tegra210-xusb-padctl"
   - Tegra186: "nvidia,tegra186-xusb-padctl"
+  - Tegra194: "nvidia,tegra194-xusb-padctl"
 - reg: Physical base address and length of the controller's registers.
 - resets: Must contain an entry for each entry in reset-names.
 - reset-names: Must include the following entries:
@@ -62,6 +63,10 @@ For Tegra186:
 - vclamp-usb-supply: Bias rail for USB pad. Must supply 1.8 V.
 - vddio-hsic-supply: HSIC PHY power supply. Must supply 1.2 V.
 
+For Tegra194:
+- avdd-usb-supply: USB I/Os, VBUS, ID, REXT, D+/D- power supply. Must supply
+  3.3 V.
+- vclamp-usb-supply: Bias rail for USB pad. Must supply 1.8 V.
 
 Pad nodes:
 ==========
@@ -154,6 +159,11 @@ For Tegra210, the list of valid PHY nodes is given below:
 - sata: sata-0
   - functions: "usb3-ss", "sata"
 
+For Tegra194, the list of valid PHY nodes is given below:
+- usb2: usb2-0, usb2-1, usb2-2, usb2-3
+  - functions: "xusb"
+- usb3: usb3-0, usb3-1, usb3-2, usb3-3
+  - functions: "xusb"
 
 Port nodes:
 ===========
@@ -221,6 +231,9 @@ Optional properties:
   is internal. In the absence of this property the port is considered to be
   external.
 
+- nvidia,disable-gen2: A boolean property whose presence determines that a port
+  should be limited to USB 3.1 Gen 1. This properlty is only for Tegra194.
+
 For Tegra124 and Tegra132, the XUSB pad controller exposes the following
 ports:
 - 3x USB2: usb2-0, usb2-1, usb2-2
@@ -233,6 +246,9 @@ For Tegra210, the XUSB pad controller exposes the following ports:
 - 2x HSIC: hsic-0, hsic-1
 - 4x super-speed USB: usb3-0, usb3-1, usb3-2, usb3-3
 
+For Tegra194, the XUSB pad controller exposes the following ports:
+- 4x USB2: usb2-0, usb2-1, usb2-2, usb2-3
+- 4x super-speed USB: usb3-0, usb3-1, usb3-2, usb3-3
 
 Examples:
 =========
-- 
2.17.1

Re: [PATCH 4/6] dt-bindings: phy: tegra: Add Tegra194 support

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 3007 bytes --]

On Wed, Oct 02, 2019 at 04:00:49PM +0800, JC Kuo wrote:
> Extend the bindings to cover the set of features found in Tegra194.
> Note that, technically, there are four more supplies connected to the
> XUSB pad controller (DVDD_PEX, DVDD_PEX_PLL, HVDD_PEX and HVDD_PEX_PLL)
> , but the power sequencing requirements of Tegra194 require these to be
> under the control of the PMIC.
> 
> Tegra194 XUSB PADCTL supports up to USB 3.1 Gen 2 speed, however, it is
> possible for some platforms have long signal trace that could not
> provide sufficient electrical environment for Gen 2 speed. To deal with
> this, a new device node property "nvidia,disable-gen2" was added to
> Tegra194 that be used to specifically disable Gen 2 speed for a
> particular USB 3.0 port so that the port can be limited to Gen 1 speed
> and avoid the instability.
> 
> Signed-off-by: JC Kuo <jckuo@nvidia.com>
> ---
>  .../bindings/phy/nvidia,tegra124-xusb-padctl.txt | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt b/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
> index 9fb682e47c29..3bef37e7c365 100644
> --- a/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
> +++ b/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
> @@ -37,6 +37,7 @@ Required properties:
>    - Tegra132: "nvidia,tegra132-xusb-padctl", "nvidia,tegra124-xusb-padctl"
>    - Tegra210: "nvidia,tegra210-xusb-padctl"
>    - Tegra186: "nvidia,tegra186-xusb-padctl"
> +  - Tegra194: "nvidia,tegra194-xusb-padctl"
>  - reg: Physical base address and length of the controller's registers.
>  - resets: Must contain an entry for each entry in reset-names.
>  - reset-names: Must include the following entries:
> @@ -62,6 +63,10 @@ For Tegra186:
>  - vclamp-usb-supply: Bias rail for USB pad. Must supply 1.8 V.
>  - vddio-hsic-supply: HSIC PHY power supply. Must supply 1.2 V.
>  
> +For Tegra194:
> +- avdd-usb-supply: USB I/Os, VBUS, ID, REXT, D+/D- power supply. Must supply
> +  3.3 V.
> +- vclamp-usb-supply: Bias rail for USB pad. Must supply 1.8 V.
>  
>  Pad nodes:
>  ==========
> @@ -154,6 +159,11 @@ For Tegra210, the list of valid PHY nodes is given below:
>  - sata: sata-0
>    - functions: "usb3-ss", "sata"
>  
> +For Tegra194, the list of valid PHY nodes is given below:
> +- usb2: usb2-0, usb2-1, usb2-2, usb2-3
> +  - functions: "xusb"
> +- usb3: usb3-0, usb3-1, usb3-2, usb3-3
> +  - functions: "xusb"
>  
>  Port nodes:
>  ===========
> @@ -221,6 +231,9 @@ Optional properties:
>    is internal. In the absence of this property the port is considered to be
>    external.
>  
> +- nvidia,disable-gen2: A boolean property whose presence determines that a port
> +  should be limited to USB 3.1 Gen 1. This properlty is only for Tegra194.

s/properlty/property/

With that:

Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 4/6] dt-bindings: phy: tegra: Add Tegra194 support

[JC Kuo]

Thanks Thierry. I will fix the typo in the next revision.

On 10/2/19 5:44 PM, Thierry Reding wrote:
> On Wed, Oct 02, 2019 at 04:00:49PM +0800, JC Kuo wrote:
>> Extend the bindings to cover the set of features found in Tegra194.
>> Note that, technically, there are four more supplies connected to the
>> XUSB pad controller (DVDD_PEX, DVDD_PEX_PLL, HVDD_PEX and HVDD_PEX_PLL)
>> , but the power sequencing requirements of Tegra194 require these to be
>> under the control of the PMIC.
>>
>> Tegra194 XUSB PADCTL supports up to USB 3.1 Gen 2 speed, however, it is
>> possible for some platforms have long signal trace that could not
>> provide sufficient electrical environment for Gen 2 speed. To deal with
>> this, a new device node property "nvidia,disable-gen2" was added to
>> Tegra194 that be used to specifically disable Gen 2 speed for a
>> particular USB 3.0 port so that the port can be limited to Gen 1 speed
>> and avoid the instability.
>>
>> Signed-off-by: JC Kuo <jckuo@nvidia.com>
>> ---
>>  .../bindings/phy/nvidia,tegra124-xusb-padctl.txt | 16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt b/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
>> index 9fb682e47c29..3bef37e7c365 100644
>> --- a/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
>> +++ b/Documentation/devicetree/bindings/phy/nvidia,tegra124-xusb-padctl.txt
>> @@ -37,6 +37,7 @@ Required properties:
>>    - Tegra132: "nvidia,tegra132-xusb-padctl", "nvidia,tegra124-xusb-padctl"
>>    - Tegra210: "nvidia,tegra210-xusb-padctl"
>>    - Tegra186: "nvidia,tegra186-xusb-padctl"
>> +  - Tegra194: "nvidia,tegra194-xusb-padctl"
>>  - reg: Physical base address and length of the controller's registers.
>>  - resets: Must contain an entry for each entry in reset-names.
>>  - reset-names: Must include the following entries:
>> @@ -62,6 +63,10 @@ For Tegra186:
>>  - vclamp-usb-supply: Bias rail for USB pad. Must supply 1.8 V.
>>  - vddio-hsic-supply: HSIC PHY power supply. Must supply 1.2 V.
>>  
>> +For Tegra194:
>> +- avdd-usb-supply: USB I/Os, VBUS, ID, REXT, D+/D- power supply. Must supply
>> +  3.3 V.
>> +- vclamp-usb-supply: Bias rail for USB pad. Must supply 1.8 V.
>>  
>>  Pad nodes:
>>  ==========
>> @@ -154,6 +159,11 @@ For Tegra210, the list of valid PHY nodes is given below:
>>  - sata: sata-0
>>    - functions: "usb3-ss", "sata"
>>  
>> +For Tegra194, the list of valid PHY nodes is given below:
>> +- usb2: usb2-0, usb2-1, usb2-2, usb2-3
>> +  - functions: "xusb"
>> +- usb3: usb3-0, usb3-1, usb3-2, usb3-3
>> +  - functions: "xusb"
>>  
>>  Port nodes:
>>  ===========
>> @@ -221,6 +231,9 @@ Optional properties:
>>    is internal. In the absence of this property the port is considered to be
>>    external.
>>  
>> +- nvidia,disable-gen2: A boolean property whose presence determines that a port
>> +  should be limited to USB 3.1 Gen 1. This properlty is only for Tegra194.
> 
> s/properlty/property/
> 
> With that:
> 
> Acked-by: Thierry Reding <treding@nvidia.com>
> 

[PATCH 5/6] arm64: tegra: Add XUSB and pad controller on Tegra194

[JC Kuo]

Adds the XUSB pad and XUSB controllers on Tegra194.

Signed-off-by: JC Kuo <jckuo@nvidia.com>
---
 arch/arm64/boot/dts/nvidia/tegra194.dtsi | 130 +++++++++++++++++++++++
 1 file changed, 130 insertions(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
index 3c0cf54f0aab..4d3371d3a407 100644
--- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
@@ -1599,4 +1599,134 @@
 		interrupt-parent = <&gic>;
 		always-on;
 	};
+
+	xusb_padctl: padctl@3520000 {
+		compatible = "nvidia,tegra194-xusb-padctl";
+		reg = <0x0 0x03520000 0x0 0x1000>,
+			<0x0 0x03540000 0x0 0x1000>;
+		reg-names = "padctl", "ao";
+
+		resets = <&bpmp TEGRA194_RESET_XUSB_PADCTL>;
+		reset-names = "padctl";
+
+		status = "disabled";
+
+		pads {
+			usb2 {
+				clocks = <&bpmp TEGRA194_CLK_USB2_TRK>;
+				clock-names = "trk";
+
+				lanes {
+					usb2-0 {
+						nvidia,function = "xusb";
+						status = "disabled";
+						#phy-cells = <0>;
+					};
+					usb2-1 {
+						nvidia,function = "xusb";
+						status = "disabled";
+						#phy-cells = <0>;
+					};
+					usb2-2 {
+						nvidia,function = "xusb";
+						status = "disabled";
+						#phy-cells = <0>;
+					};
+					usb2-3 {
+						nvidia,function = "xusb";
+						status = "disabled";
+						#phy-cells = <0>;
+					};
+				};
+			};
+			usb3 {
+				lanes {
+					usb3-0 {
+						nvidia,function = "xusb";
+						status = "disabled";
+						#phy-cells = <0>;
+					};
+					usb3-1 {
+						nvidia,function = "xusb";
+						status = "disabled";
+						#phy-cells = <0>;
+					};
+					usb3-2 {
+						nvidia,function = "xusb";
+						status = "disabled";
+						#phy-cells = <0>;
+					};
+					usb3-3 {
+						nvidia,function = "xusb";
+						status = "disabled";
+						#phy-cells = <0>;
+					};
+				};
+			};
+		};
+
+		ports {
+			usb2-0 {
+				status = "disabled";
+			};
+			usb2-1 {
+				status = "disabled";
+			};
+			usb2-2 {
+				status = "disabled";
+			};
+			usb2-3 {
+				status = "disabled";
+			};
+			usb3-0 {
+				status = "disabled";
+			};
+			usb3-1 {
+				status = "disabled";
+			};
+			usb3-2 {
+				status = "disabled";
+			};
+			usb3-3 {
+				status = "disabled";
+			};
+		};
+	};
+
+	tegra_xhci: xhci@3610000 {
+		compatible = "nvidia,tegra194-xusb";
+		reg = <0x0 0x03610000 0x0 0x40000>,
+			<0x0 0x03600000 0x0 0x10000>;
+		reg-names = "hcd", "fpci";
+
+		interrupts = <GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH>,
+				<GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>,
+				<GIC_SPI 167 IRQ_TYPE_LEVEL_HIGH>;
+
+		clocks = <&bpmp TEGRA194_CLK_XUSB_CORE_MUX>,
+			<&bpmp TEGRA194_CLK_XUSB_CORE_HOST>,
+			<&bpmp TEGRA194_CLK_XUSB_CORE_SS>,
+			<&bpmp TEGRA194_CLK_XUSB_FALCON>,
+			<&bpmp TEGRA194_CLK_XUSB_FALCON_HOST>,
+			<&bpmp TEGRA194_CLK_XUSB_FALCON_SS>,
+			<&bpmp TEGRA194_CLK_XUSB_FS>,
+			<&bpmp TEGRA194_CLK_XUSB_FS_HOST>,
+			<&bpmp TEGRA194_CLK_XUSB_SS>,
+			<&bpmp TEGRA194_CLK_XUSB_SS_SUPERSPEED>,
+			<&bpmp TEGRA194_CLK_UTMIPLL>,
+			<&bpmp TEGRA194_CLK_CLK_M>,
+			<&bpmp TEGRA194_CLK_PLLE>;
+		clock-names = "xusb_hs_src", "xusb_host",
+			"xusb_core_superspeed_clk", "xusb_falcon_src",
+			"xusb_falcon_host_clk", "xusb_falcon_superspeed_clk",
+			"xusb_fs_src", "xusb_fs_host_clk", "xusb_ss_src",
+			"xusb_ss", "pll_u_480m", "clk_m", "pll_e";
+
+		power-domains = <&bpmp TEGRA194_POWER_DOMAIN_XUSBC>,
+				<&bpmp TEGRA194_POWER_DOMAIN_XUSBA>;
+		power-domain-names = "xusb_host", "xusb_ss";
+
+		nvidia,xusb-padctl = <&xusb_padctl>;
+		status = "disabled";
+	};
 };
-- 
2.17.1

Re: [PATCH 5/6] arm64: tegra: Add XUSB and pad controller on Tegra194

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 4934 bytes --]

On Wed, Oct 02, 2019 at 04:00:50PM +0800, JC Kuo wrote:
> Adds the XUSB pad and XUSB controllers on Tegra194.
> 
> Signed-off-by: JC Kuo <jckuo@nvidia.com>
> ---
>  arch/arm64/boot/dts/nvidia/tegra194.dtsi | 130 +++++++++++++++++++++++
>  1 file changed, 130 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> index 3c0cf54f0aab..4d3371d3a407 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> @@ -1599,4 +1599,134 @@
>  		interrupt-parent = <&gic>;
>  		always-on;
>  	};
> +
> +	xusb_padctl: padctl@3520000 {
> +		compatible = "nvidia,tegra194-xusb-padctl";
> +		reg = <0x0 0x03520000 0x0 0x1000>,
> +			<0x0 0x03540000 0x0 0x1000>;

These should generally be aligned. Use tabs first and then spaces to
make the opening < on subsequent lines align with the opening < on the
first line. There are a couple more like this below.

> +		reg-names = "padctl", "ao";
> +
> +		resets = <&bpmp TEGRA194_RESET_XUSB_PADCTL>;
> +		reset-names = "padctl";
> +
> +		status = "disabled";
> +
> +		pads {
> +			usb2 {
> +				clocks = <&bpmp TEGRA194_CLK_USB2_TRK>;
> +				clock-names = "trk";
> +
> +				lanes {
> +					usb2-0 {
> +						nvidia,function = "xusb";
> +						status = "disabled";
> +						#phy-cells = <0>;
> +					};
> +					usb2-1 {

I prefer blank lines to visually separate blocks here and below.

> +						nvidia,function = "xusb";
> +						status = "disabled";
> +						#phy-cells = <0>;
> +					};
> +					usb2-2 {
> +						nvidia,function = "xusb";
> +						status = "disabled";
> +						#phy-cells = <0>;
> +					};
> +					usb2-3 {
> +						nvidia,function = "xusb";
> +						status = "disabled";
> +						#phy-cells = <0>;
> +					};
> +				};
> +			};
> +			usb3 {
> +				lanes {
> +					usb3-0 {
> +						nvidia,function = "xusb";
> +						status = "disabled";
> +						#phy-cells = <0>;
> +					};
> +					usb3-1 {
> +						nvidia,function = "xusb";
> +						status = "disabled";
> +						#phy-cells = <0>;
> +					};
> +					usb3-2 {
> +						nvidia,function = "xusb";
> +						status = "disabled";
> +						#phy-cells = <0>;
> +					};
> +					usb3-3 {
> +						nvidia,function = "xusb";
> +						status = "disabled";
> +						#phy-cells = <0>;
> +					};
> +				};
> +			};
> +		};
> +
> +		ports {
> +			usb2-0 {
> +				status = "disabled";
> +			};
> +			usb2-1 {
> +				status = "disabled";
> +			};
> +			usb2-2 {
> +				status = "disabled";
> +			};
> +			usb2-3 {
> +				status = "disabled";
> +			};
> +			usb3-0 {
> +				status = "disabled";
> +			};
> +			usb3-1 {
> +				status = "disabled";
> +			};
> +			usb3-2 {
> +				status = "disabled";
> +			};
> +			usb3-3 {
> +				status = "disabled";
> +			};
> +		};
> +	};
> +
> +	tegra_xhci: xhci@3610000 {

The tegra_xhci is unused, so I don't think we need to add it. Also, the
name of this node should be usb@3610000 since it's the compatible string
that defines (together with the bindings) that this is XHCI capable. But
it is fundamentally a USB controller, so the name should reflect that.

> +		compatible = "nvidia,tegra194-xusb";
> +		reg = <0x0 0x03610000 0x0 0x40000>,
> +			<0x0 0x03600000 0x0 0x10000>;
> +		reg-names = "hcd", "fpci";
> +
> +		interrupts = <GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH>,
> +				<GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>,
> +				<GIC_SPI 167 IRQ_TYPE_LEVEL_HIGH>;
> +
> +		clocks = <&bpmp TEGRA194_CLK_XUSB_CORE_MUX>,
> +			<&bpmp TEGRA194_CLK_XUSB_CORE_HOST>,
> +			<&bpmp TEGRA194_CLK_XUSB_CORE_SS>,
> +			<&bpmp TEGRA194_CLK_XUSB_FALCON>,
> +			<&bpmp TEGRA194_CLK_XUSB_FALCON_HOST>,
> +			<&bpmp TEGRA194_CLK_XUSB_FALCON_SS>,
> +			<&bpmp TEGRA194_CLK_XUSB_FS>,
> +			<&bpmp TEGRA194_CLK_XUSB_FS_HOST>,
> +			<&bpmp TEGRA194_CLK_XUSB_SS>,
> +			<&bpmp TEGRA194_CLK_XUSB_SS_SUPERSPEED>,
> +			<&bpmp TEGRA194_CLK_UTMIPLL>,
> +			<&bpmp TEGRA194_CLK_CLK_M>,
> +			<&bpmp TEGRA194_CLK_PLLE>;
> +		clock-names = "xusb_hs_src", "xusb_host",
> +			"xusb_core_superspeed_clk", "xusb_falcon_src",
> +			"xusb_falcon_host_clk", "xusb_falcon_superspeed_clk",
> +			"xusb_fs_src", "xusb_fs_host_clk", "xusb_ss_src",
> +			"xusb_ss", "pll_u_480m", "clk_m", "pll_e";

Some of these clocks are not defined in the bindings. Many of them are
also not used by the driver. Are all of these really needed? If they
are, please add the required ones to the bindings.

Also, for new ones, drop the _clk suffix. The fact that these are clocks
is already conveyed by the property name.

Thierry

> +
> +		power-domains = <&bpmp TEGRA194_POWER_DOMAIN_XUSBC>,
> +				<&bpmp TEGRA194_POWER_DOMAIN_XUSBA>;
> +		power-domain-names = "xusb_host", "xusb_ss";
> +
> +		nvidia,xusb-padctl = <&xusb_padctl>;
> +		status = "disabled";
> +	};
>  };
> -- 
> 2.17.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 5/6] arm64: tegra: Add XUSB and pad controller on Tegra194

[JC Kuo]

On 10/2/19 6:10 PM, Thierry Reding wrote:
> On Wed, Oct 02, 2019 at 04:00:50PM +0800, JC Kuo wrote:
>> Adds the XUSB pad and XUSB controllers on Tegra194.
>>
>> Signed-off-by: JC Kuo <jckuo@nvidia.com>
>> ---
>>  arch/arm64/boot/dts/nvidia/tegra194.dtsi | 130 +++++++++++++++++++++++
>>  1 file changed, 130 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
>> index 3c0cf54f0aab..4d3371d3a407 100644
>> --- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
>> +++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
>> @@ -1599,4 +1599,134 @@
>>  		interrupt-parent = <&gic>;
>>  		always-on;
>>  	};
>> +
>> +	xusb_padctl: padctl@3520000 {
>> +		compatible = "nvidia,tegra194-xusb-padctl";
>> +		reg = <0x0 0x03520000 0x0 0x1000>,
>> +			<0x0 0x03540000 0x0 0x1000>;
> 
> These should generally be aligned. Use tabs first and then spaces to
> make the opening < on subsequent lines align with the opening < on the
> first line. There are a couple more like this below.
Thanks. I will make those aligned.
> 
>> +		reg-names = "padctl", "ao";
>> +
>> +		resets = <&bpmp TEGRA194_RESET_XUSB_PADCTL>;
>> +		reset-names = "padctl";
>> +
>> +		status = "disabled";
>> +
>> +		pads {
>> +			usb2 {
>> +				clocks = <&bpmp TEGRA194_CLK_USB2_TRK>;
>> +				clock-names = "trk";
>> +
>> +				lanes {
>> +					usb2-0 {
>> +						nvidia,function = "xusb";
>> +						status = "disabled";
>> +						#phy-cells = <0>;
>> +					};
>> +					usb2-1 {
> 
> I prefer blank lines to visually separate blocks here and below.
Sure, will do.
> 
>> +						nvidia,function = "xusb";
>> +						status = "disabled";
>> +						#phy-cells = <0>;
>> +					};
>> +					usb2-2 {
>> +						nvidia,function = "xusb";
>> +						status = "disabled";
>> +						#phy-cells = <0>;
>> +					};
>> +					usb2-3 {
>> +						nvidia,function = "xusb";
>> +						status = "disabled";
>> +						#phy-cells = <0>;
>> +					};
>> +				};
>> +			};
>> +			usb3 {
>> +				lanes {
>> +					usb3-0 {
>> +						nvidia,function = "xusb";
>> +						status = "disabled";
>> +						#phy-cells = <0>;
>> +					};
>> +					usb3-1 {
>> +						nvidia,function = "xusb";
>> +						status = "disabled";
>> +						#phy-cells = <0>;
>> +					};
>> +					usb3-2 {
>> +						nvidia,function = "xusb";
>> +						status = "disabled";
>> +						#phy-cells = <0>;
>> +					};
>> +					usb3-3 {
>> +						nvidia,function = "xusb";
>> +						status = "disabled";
>> +						#phy-cells = <0>;
>> +					};
>> +				};
>> +			};
>> +		};
>> +
>> +		ports {
>> +			usb2-0 {
>> +				status = "disabled";
>> +			};
>> +			usb2-1 {
>> +				status = "disabled";
>> +			};
>> +			usb2-2 {
>> +				status = "disabled";
>> +			};
>> +			usb2-3 {
>> +				status = "disabled";
>> +			};
>> +			usb3-0 {
>> +				status = "disabled";
>> +			};
>> +			usb3-1 {
>> +				status = "disabled";
>> +			};
>> +			usb3-2 {
>> +				status = "disabled";
>> +			};
>> +			usb3-3 {
>> +				status = "disabled";
>> +			};
>> +		};
>> +	};
>> +
>> +	tegra_xhci: xhci@3610000 {
> 
> The tegra_xhci is unused, so I don't think we need to add it. Also, the
> name of this node should be usb@3610000 since it's the compatible string
> that defines (together with the bindings) that this is XHCI capable. But
> it is fundamentally a USB controller, so the name should reflect that.
> 
Understood. I will fix in the next revision.
>> +		compatible = "nvidia,tegra194-xusb";
>> +		reg = <0x0 0x03610000 0x0 0x40000>,
>> +			<0x0 0x03600000 0x0 0x10000>;
>> +		reg-names = "hcd", "fpci";
>> +
>> +		interrupts = <GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH>,
>> +				<GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>,
>> +				<GIC_SPI 167 IRQ_TYPE_LEVEL_HIGH>;
>> +
>> +		clocks = <&bpmp TEGRA194_CLK_XUSB_CORE_MUX>,
>> +			<&bpmp TEGRA194_CLK_XUSB_CORE_HOST>,
>> +			<&bpmp TEGRA194_CLK_XUSB_CORE_SS>,
>> +			<&bpmp TEGRA194_CLK_XUSB_FALCON>,
>> +			<&bpmp TEGRA194_CLK_XUSB_FALCON_HOST>,
>> +			<&bpmp TEGRA194_CLK_XUSB_FALCON_SS>,
>> +			<&bpmp TEGRA194_CLK_XUSB_FS>,
>> +			<&bpmp TEGRA194_CLK_XUSB_FS_HOST>,
>> +			<&bpmp TEGRA194_CLK_XUSB_SS>,
>> +			<&bpmp TEGRA194_CLK_XUSB_SS_SUPERSPEED>,
>> +			<&bpmp TEGRA194_CLK_UTMIPLL>,
>> +			<&bpmp TEGRA194_CLK_CLK_M>,
>> +			<&bpmp TEGRA194_CLK_PLLE>;
>> +		clock-names = "xusb_hs_src", "xusb_host",
>> +			"xusb_core_superspeed_clk", "xusb_falcon_src",
>> +			"xusb_falcon_host_clk", "xusb_falcon_superspeed_clk",
>> +			"xusb_fs_src", "xusb_fs_host_clk", "xusb_ss_src",
>> +			"xusb_ss", "pll_u_480m", "clk_m", "pll_e";
> 
> Some of these clocks are not defined in the bindings. Many of them are
> also not used by the driver. Are all of these really needed? If they
> are, please add the required ones to the bindings.
> 
> Also, for new ones, drop the _clk suffix. The fact that these are clocks
> is already conveyed by the property name.
> 
> Thierry
Thanks. I will clean this up.
> 
>> +
>> +		power-domains = <&bpmp TEGRA194_POWER_DOMAIN_XUSBC>,
>> +				<&bpmp TEGRA194_POWER_DOMAIN_XUSBA>;
>> +		power-domain-names = "xusb_host", "xusb_ss";
>> +
>> +		nvidia,xusb-padctl = <&xusb_padctl>;
>> +		status = "disabled";
>> +	};
>>  };
>> -- 
>> 2.17.1
>>

Re: [PATCH 5/6] arm64: tegra: Add XUSB and pad controller on Tegra194

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 4180 bytes --]

On Wed, Oct 02, 2019 at 04:00:50PM +0800, JC Kuo wrote:
> Adds the XUSB pad and XUSB controllers on Tegra194.
> 
> Signed-off-by: JC Kuo <jckuo@nvidia.com>
> ---
>  arch/arm64/boot/dts/nvidia/tegra194.dtsi | 130 +++++++++++++++++++++++
>  1 file changed, 130 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> index 3c0cf54f0aab..4d3371d3a407 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> @@ -1599,4 +1599,134 @@
>  		interrupt-parent = <&gic>;
>  		always-on;
>  	};
> +
> +	xusb_padctl: padctl@3520000 {

I also noticed that this is outside of the /cbb bus node. It really
belongs inside /cbb. Same for the XHCI controller node.

Thierry

> +		compatible = "nvidia,tegra194-xusb-padctl";
> +		reg = <0x0 0x03520000 0x0 0x1000>,
> +			<0x0 0x03540000 0x0 0x1000>;
> +		reg-names = "padctl", "ao";
> +
> +		resets = <&bpmp TEGRA194_RESET_XUSB_PADCTL>;
> +		reset-names = "padctl";
> +
> +		status = "disabled";
> +
> +		pads {
> +			usb2 {
> +				clocks = <&bpmp TEGRA194_CLK_USB2_TRK>;
> +				clock-names = "trk";
> +
> +				lanes {
> +					usb2-0 {
> +						nvidia,function = "xusb";
> +						status = "disabled";
> +						#phy-cells = <0>;
> +					};
> +					usb2-1 {
> +						nvidia,function = "xusb";
> +						status = "disabled";
> +						#phy-cells = <0>;
> +					};
> +					usb2-2 {
> +						nvidia,function = "xusb";
> +						status = "disabled";
> +						#phy-cells = <0>;
> +					};
> +					usb2-3 {
> +						nvidia,function = "xusb";
> +						status = "disabled";
> +						#phy-cells = <0>;
> +					};
> +				};
> +			};
> +			usb3 {
> +				lanes {
> +					usb3-0 {
> +						nvidia,function = "xusb";
> +						status = "disabled";
> +						#phy-cells = <0>;
> +					};
> +					usb3-1 {
> +						nvidia,function = "xusb";
> +						status = "disabled";
> +						#phy-cells = <0>;
> +					};
> +					usb3-2 {
> +						nvidia,function = "xusb";
> +						status = "disabled";
> +						#phy-cells = <0>;
> +					};
> +					usb3-3 {
> +						nvidia,function = "xusb";
> +						status = "disabled";
> +						#phy-cells = <0>;
> +					};
> +				};
> +			};
> +		};
> +
> +		ports {
> +			usb2-0 {
> +				status = "disabled";
> +			};
> +			usb2-1 {
> +				status = "disabled";
> +			};
> +			usb2-2 {
> +				status = "disabled";
> +			};
> +			usb2-3 {
> +				status = "disabled";
> +			};
> +			usb3-0 {
> +				status = "disabled";
> +			};
> +			usb3-1 {
> +				status = "disabled";
> +			};
> +			usb3-2 {
> +				status = "disabled";
> +			};
> +			usb3-3 {
> +				status = "disabled";
> +			};
> +		};
> +	};
> +
> +	tegra_xhci: xhci@3610000 {
> +		compatible = "nvidia,tegra194-xusb";
> +		reg = <0x0 0x03610000 0x0 0x40000>,
> +			<0x0 0x03600000 0x0 0x10000>;
> +		reg-names = "hcd", "fpci";
> +
> +		interrupts = <GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH>,
> +				<GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>,
> +				<GIC_SPI 167 IRQ_TYPE_LEVEL_HIGH>;
> +
> +		clocks = <&bpmp TEGRA194_CLK_XUSB_CORE_MUX>,
> +			<&bpmp TEGRA194_CLK_XUSB_CORE_HOST>,
> +			<&bpmp TEGRA194_CLK_XUSB_CORE_SS>,
> +			<&bpmp TEGRA194_CLK_XUSB_FALCON>,
> +			<&bpmp TEGRA194_CLK_XUSB_FALCON_HOST>,
> +			<&bpmp TEGRA194_CLK_XUSB_FALCON_SS>,
> +			<&bpmp TEGRA194_CLK_XUSB_FS>,
> +			<&bpmp TEGRA194_CLK_XUSB_FS_HOST>,
> +			<&bpmp TEGRA194_CLK_XUSB_SS>,
> +			<&bpmp TEGRA194_CLK_XUSB_SS_SUPERSPEED>,
> +			<&bpmp TEGRA194_CLK_UTMIPLL>,
> +			<&bpmp TEGRA194_CLK_CLK_M>,
> +			<&bpmp TEGRA194_CLK_PLLE>;
> +		clock-names = "xusb_hs_src", "xusb_host",
> +			"xusb_core_superspeed_clk", "xusb_falcon_src",
> +			"xusb_falcon_host_clk", "xusb_falcon_superspeed_clk",
> +			"xusb_fs_src", "xusb_fs_host_clk", "xusb_ss_src",
> +			"xusb_ss", "pll_u_480m", "clk_m", "pll_e";
> +
> +		power-domains = <&bpmp TEGRA194_POWER_DOMAIN_XUSBC>,
> +				<&bpmp TEGRA194_POWER_DOMAIN_XUSBA>;
> +		power-domain-names = "xusb_host", "xusb_ss";
> +
> +		nvidia,xusb-padctl = <&xusb_padctl>;
> +		status = "disabled";
> +	};
>  };
> -- 
> 2.17.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 5/6] arm64: tegra: Add XUSB and pad controller on Tegra194

[JC Kuo]

On 10/2/19 6:11 PM, Thierry Reding wrote:
> On Wed, Oct 02, 2019 at 04:00:50PM +0800, JC Kuo wrote:
>> Adds the XUSB pad and XUSB controllers on Tegra194.
>>
>> Signed-off-by: JC Kuo <jckuo@nvidia.com>
>> ---
>>  arch/arm64/boot/dts/nvidia/tegra194.dtsi | 130 +++++++++++++++++++++++
>>  1 file changed, 130 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
>> index 3c0cf54f0aab..4d3371d3a407 100644
>> --- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
>> +++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
>> @@ -1599,4 +1599,134 @@
>>  		interrupt-parent = <&gic>;
>>  		always-on;
>>  	};
>> +
>> +	xusb_padctl: padctl@3520000 {
> 
> I also noticed that this is outside of the /cbb bus node. It really
> belongs inside /cbb. Same for the XHCI controller node.
> 
> Thierry
Thanks. I will move both inside /cbb.
> 
>> +		compatible = "nvidia,tegra194-xusb-padctl";
>> +		reg = <0x0 0x03520000 0x0 0x1000>,
>> +			<0x0 0x03540000 0x0 0x1000>;
>> +		reg-names = "padctl", "ao";
>> +
>> +		resets = <&bpmp TEGRA194_RESET_XUSB_PADCTL>;
>> +		reset-names = "padctl";
>> +
>> +		status = "disabled";
>> +
>> +		pads {
>> +			usb2 {
>> +				clocks = <&bpmp TEGRA194_CLK_USB2_TRK>;
>> +				clock-names = "trk";
>> +
>> +				lanes {
>> +					usb2-0 {
>> +						nvidia,function = "xusb";
>> +						status = "disabled";
>> +						#phy-cells = <0>;
>> +					};
>> +					usb2-1 {
>> +						nvidia,function = "xusb";
>> +						status = "disabled";
>> +						#phy-cells = <0>;
>> +					};
>> +					usb2-2 {
>> +						nvidia,function = "xusb";
>> +						status = "disabled";
>> +						#phy-cells = <0>;
>> +					};
>> +					usb2-3 {
>> +						nvidia,function = "xusb";
>> +						status = "disabled";
>> +						#phy-cells = <0>;
>> +					};
>> +				};
>> +			};
>> +			usb3 {
>> +				lanes {
>> +					usb3-0 {
>> +						nvidia,function = "xusb";
>> +						status = "disabled";
>> +						#phy-cells = <0>;
>> +					};
>> +					usb3-1 {
>> +						nvidia,function = "xusb";
>> +						status = "disabled";
>> +						#phy-cells = <0>;
>> +					};
>> +					usb3-2 {
>> +						nvidia,function = "xusb";
>> +						status = "disabled";
>> +						#phy-cells = <0>;
>> +					};
>> +					usb3-3 {
>> +						nvidia,function = "xusb";
>> +						status = "disabled";
>> +						#phy-cells = <0>;
>> +					};
>> +				};
>> +			};
>> +		};
>> +
>> +		ports {
>> +			usb2-0 {
>> +				status = "disabled";
>> +			};
>> +			usb2-1 {
>> +				status = "disabled";
>> +			};
>> +			usb2-2 {
>> +				status = "disabled";
>> +			};
>> +			usb2-3 {
>> +				status = "disabled";
>> +			};
>> +			usb3-0 {
>> +				status = "disabled";
>> +			};
>> +			usb3-1 {
>> +				status = "disabled";
>> +			};
>> +			usb3-2 {
>> +				status = "disabled";
>> +			};
>> +			usb3-3 {
>> +				status = "disabled";
>> +			};
>> +		};
>> +	};
>> +
>> +	tegra_xhci: xhci@3610000 {
>> +		compatible = "nvidia,tegra194-xusb";
>> +		reg = <0x0 0x03610000 0x0 0x40000>,
>> +			<0x0 0x03600000 0x0 0x10000>;
>> +		reg-names = "hcd", "fpci";
>> +
>> +		interrupts = <GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH>,
>> +				<GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>,
>> +				<GIC_SPI 167 IRQ_TYPE_LEVEL_HIGH>;
>> +
>> +		clocks = <&bpmp TEGRA194_CLK_XUSB_CORE_MUX>,
>> +			<&bpmp TEGRA194_CLK_XUSB_CORE_HOST>,
>> +			<&bpmp TEGRA194_CLK_XUSB_CORE_SS>,
>> +			<&bpmp TEGRA194_CLK_XUSB_FALCON>,
>> +			<&bpmp TEGRA194_CLK_XUSB_FALCON_HOST>,
>> +			<&bpmp TEGRA194_CLK_XUSB_FALCON_SS>,
>> +			<&bpmp TEGRA194_CLK_XUSB_FS>,
>> +			<&bpmp TEGRA194_CLK_XUSB_FS_HOST>,
>> +			<&bpmp TEGRA194_CLK_XUSB_SS>,
>> +			<&bpmp TEGRA194_CLK_XUSB_SS_SUPERSPEED>,
>> +			<&bpmp TEGRA194_CLK_UTMIPLL>,
>> +			<&bpmp TEGRA194_CLK_CLK_M>,
>> +			<&bpmp TEGRA194_CLK_PLLE>;
>> +		clock-names = "xusb_hs_src", "xusb_host",
>> +			"xusb_core_superspeed_clk", "xusb_falcon_src",
>> +			"xusb_falcon_host_clk", "xusb_falcon_superspeed_clk",
>> +			"xusb_fs_src", "xusb_fs_host_clk", "xusb_ss_src",
>> +			"xusb_ss", "pll_u_480m", "clk_m", "pll_e";
>> +
>> +		power-domains = <&bpmp TEGRA194_POWER_DOMAIN_XUSBC>,
>> +				<&bpmp TEGRA194_POWER_DOMAIN_XUSBA>;
>> +		power-domain-names = "xusb_host", "xusb_ss";
>> +
>> +		nvidia,xusb-padctl = <&xusb_padctl>;
>> +		status = "disabled";
>> +	};
>>  };
>> -- 
>> 2.17.1
>>

[PATCH 6/6] arm64: tegra: Enable XUSB host in P2972-0000 board

[JC Kuo]

This commit enables XUSB host and pad controller in Tegra194
P2972-0000 board.

Signed-off-by: JC Kuo <jckuo@nvidia.com>
---
 .../arm64/boot/dts/nvidia/tegra194-p2888.dtsi | 31 +++++++++-
 .../boot/dts/nvidia/tegra194-p2972-0000.dts   | 59 +++++++++++++++++++
 2 files changed, 89 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi b/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi
index 4c38426a6969..cb236edc6a0d 100644
--- a/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi
@@ -229,7 +229,7 @@
 						regulator-max-microvolt = <3300000>;
 					};
 
-					ldo5 {
+					vdd_usb_3v3: ldo5 {
 						regulator-name = "VDD_USB_3V3";
 						regulator-min-microvolt = <3300000>;
 						regulator-max-microvolt = <3300000>;
@@ -313,5 +313,34 @@
 			regulator-boot-on;
 			enable-active-low;
 		};
+
+		vdd_5v_sata: regulator@4 {
+			compatible = "regulator-fixed";
+			reg = <4>;
+
+			regulator-name = "vdd-5v-sata";
+			regulator-min-microvolt = <5000000>;
+			regulator-max-microvolt = <5000000>;
+			gpio = <&gpio TEGRA194_MAIN_GPIO(Z, 1) GPIO_ACTIVE_LOW>;
+		};
+	};
+
+	padctl@3520000 {
+		avdd-usb-supply = <&vdd_usb_3v3>;
+		vclamp-usb-supply = <&vdd_1v8ao>;
+		ports {
+			usb2-1 {
+				vbus-supply = <&vdd_5v0_sys>;
+			};
+			usb2-3 {
+				vbus-supply = <&vdd_5v_sata>;
+			};
+			usb3-0 {
+				vbus-supply = <&vdd_5v0_sys>;
+			};
+			usb3-3 {
+				vbus-supply = <&vdd_5v0_sys>;
+			};
+		};
 	};
 };
diff --git a/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts b/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts
index d47cd8c4dd24..410221927dfa 100644
--- a/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts
+++ b/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts
@@ -222,4 +222,63 @@
 			};
 		};
 	};
+
+	padctl@3520000 {
+		status = "okay";
+
+		pads {
+			usb2 {
+				lanes {
+					usb2-1 {
+						status = "okay";
+					};
+					usb2-2 {
+						status = "okay";
+					};
+					usb2-3 {
+						status = "okay";
+					};
+				};
+			};
+			usb3 {
+				lanes {
+					usb3-0 {
+						status = "okay";
+					};
+					usb3-3 {
+						status = "okay";
+					};
+				};
+			};
+		};
+
+		ports {
+			usb2-1 {
+				mode = "host";
+				status = "okay";
+			};
+			usb2-3 {
+				mode = "host";
+				status = "okay";
+			};
+			usb3-0 {
+				nvidia,usb2-companion = <1>;
+				status = "okay";
+			};
+			usb3-3 {
+				nvidia,usb2-companion = <3>;
+				nvidia,disable-gen2;
+				status = "okay";
+			};
+		};
+	};
+
+	tegra_xhci: xhci@3610000 {
+		status = "okay";
+		phys =	<&{/padctl@3520000/pads/usb2/lanes/usb2-1}>,
+			<&{/padctl@3520000/pads/usb2/lanes/usb2-3}>,
+			<&{/padctl@3520000/pads/usb3/lanes/usb3-0}>,
+			<&{/padctl@3520000/pads/usb3/lanes/usb3-3}>;
+		phy-names = "usb2-1", "usb2-3", "usb3-0", "usb3-3";
+	};
 };
-- 
2.17.1

Re: [PATCH 6/6] arm64: tegra: Enable XUSB host in P2972-0000 board

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 5108 bytes --]

On Wed, Oct 02, 2019 at 04:00:51PM +0800, JC Kuo wrote:
> This commit enables XUSB host and pad controller in Tegra194
> P2972-0000 board.
> 
> Signed-off-by: JC Kuo <jckuo@nvidia.com>
> ---
>  .../arm64/boot/dts/nvidia/tegra194-p2888.dtsi | 31 +++++++++-
>  .../boot/dts/nvidia/tegra194-p2972-0000.dts   | 59 +++++++++++++++++++
>  2 files changed, 89 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi b/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi
> index 4c38426a6969..cb236edc6a0d 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi
> @@ -229,7 +229,7 @@
>  						regulator-max-microvolt = <3300000>;
>  					};
>  
> -					ldo5 {
> +					vdd_usb_3v3: ldo5 {
>  						regulator-name = "VDD_USB_3V3";
>  						regulator-min-microvolt = <3300000>;
>  						regulator-max-microvolt = <3300000>;
> @@ -313,5 +313,34 @@
>  			regulator-boot-on;
>  			enable-active-low;
>  		};
> +
> +		vdd_5v_sata: regulator@4 {
> +			compatible = "regulator-fixed";
> +			reg = <4>;
> +
> +			regulator-name = "vdd-5v-sata";

Please keep capitalization of regulator names consistent. We use
all-caps and underscores for the others (which mirrors the names in the
schematics), so please stick with that for this one as well.

Also, I'm wondering if perhaps you can clarify something here. My
understanding is that SATA functionality is provided via a controller on
the PCI bus. Why is it that we route the 5 V SATA power to the USB port?
Oh wait... this is one of those eSATAp (hybrid) ports that can take
either eSATA or USB, right? Do we need any additional setup to switch
between eSATA and USB modes? Or is this all done in hardware? That is,
if I plug in an eSATA, does it automatically hotplug detect the device
as SATA and if I plug in a USB device, does it automatically detect it
as USB?

> +			regulator-min-microvolt = <5000000>;
> +			regulator-max-microvolt = <5000000>;
> +			gpio = <&gpio TEGRA194_MAIN_GPIO(Z, 1) GPIO_ACTIVE_LOW>;

This will actually cause a warning on boot. For fixed regulators the
polarity of the enable GPIO is not specified in the GPIO specifier.
Instead you're supposed to use the boolean enable-active-high property
to specify if the enable GPIO is active-high. By default the enable GPIO
is considered to be active-low. The GPIO specifier needs to have the
GPIO_ACTIVE_HIGH flag set regardless for backwards-compatibilitiy
reasons.

Note that regulator@3 above your new entry does this wrongly, but
next-20191002 should have a fix for that.

> +		};
> +	};
> +
> +	padctl@3520000 {

Don't forget to move this into /cbb as well to match the changes to
patch 5/6.

> +		avdd-usb-supply = <&vdd_usb_3v3>;
> +		vclamp-usb-supply = <&vdd_1v8ao>;
> +		ports {

Blank line between the above two for better readability.

> +			usb2-1 {
> +				vbus-supply = <&vdd_5v0_sys>;
> +			};
> +			usb2-3 {

Same here and below.

> +				vbus-supply = <&vdd_5v_sata>;
> +			};
> +			usb3-0 {
> +				vbus-supply = <&vdd_5v0_sys>;
> +			};
> +			usb3-3 {
> +				vbus-supply = <&vdd_5v0_sys>;
> +			};
> +		};
>  	};
>  };
> diff --git a/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts b/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts
> index d47cd8c4dd24..410221927dfa 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts
> +++ b/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts
> @@ -222,4 +222,63 @@
>  			};
>  		};
>  	};
> +
> +	padctl@3520000 {

Same comment as above. Move this into /cbb.

> +		status = "okay";
> +
> +		pads {
> +			usb2 {
> +				lanes {
> +					usb2-1 {
> +						status = "okay";
> +					};
> +					usb2-2 {

And blank lines for readability here and below.

> +						status = "okay";
> +					};
> +					usb2-3 {
> +						status = "okay";
> +					};
> +				};
> +			};
> +			usb3 {
> +				lanes {
> +					usb3-0 {
> +						status = "okay";
> +					};
> +					usb3-3 {
> +						status = "okay";
> +					};
> +				};
> +			};
> +		};
> +
> +		ports {
> +			usb2-1 {
> +				mode = "host";
> +				status = "okay";
> +			};
> +			usb2-3 {
> +				mode = "host";
> +				status = "okay";
> +			};
> +			usb3-0 {
> +				nvidia,usb2-companion = <1>;
> +				status = "okay";
> +			};
> +			usb3-3 {
> +				nvidia,usb2-companion = <3>;
> +				nvidia,disable-gen2;
> +				status = "okay";
> +			};
> +		};
> +	};
> +
> +	tegra_xhci: xhci@3610000 {

Also needs to move into /cbb. Also, you can drop the tegra_xhci label
here since we never reference the controller from elsewhere.

Also make sure to update the name here to match the changes in 5/6.

Thierry

> +		status = "okay";
> +		phys =	<&{/padctl@3520000/pads/usb2/lanes/usb2-1}>,
> +			<&{/padctl@3520000/pads/usb2/lanes/usb2-3}>,
> +			<&{/padctl@3520000/pads/usb3/lanes/usb3-0}>,
> +			<&{/padctl@3520000/pads/usb3/lanes/usb3-3}>;
> +		phy-names = "usb2-1", "usb2-3", "usb3-0", "usb3-3";
> +	};
>  };
> -- 
> 2.17.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 6/6] arm64: tegra: Enable XUSB host in P2972-0000 board

[JC Kuo]

On 10/2/19 6:26 PM, Thierry Reding wrote:
> On Wed, Oct 02, 2019 at 04:00:51PM +0800, JC Kuo wrote:
>> This commit enables XUSB host and pad controller in Tegra194
>> P2972-0000 board.
>>
>> Signed-off-by: JC Kuo <jckuo@nvidia.com>
>> ---
>>  .../arm64/boot/dts/nvidia/tegra194-p2888.dtsi | 31 +++++++++-
>>  .../boot/dts/nvidia/tegra194-p2972-0000.dts   | 59 +++++++++++++++++++
>>  2 files changed, 89 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi b/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi
>> index 4c38426a6969..cb236edc6a0d 100644
>> --- a/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi
>> +++ b/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi
>> @@ -229,7 +229,7 @@
>>  						regulator-max-microvolt = <3300000>;
>>  					};
>>  
>> -					ldo5 {
>> +					vdd_usb_3v3: ldo5 {
>>  						regulator-name = "VDD_USB_3V3";
>>  						regulator-min-microvolt = <3300000>;
>>  						regulator-max-microvolt = <3300000>;
>> @@ -313,5 +313,34 @@
>>  			regulator-boot-on;
>>  			enable-active-low;
>>  		};
>> +
>> +		vdd_5v_sata: regulator@4 {
>> +			compatible = "regulator-fixed";
>> +			reg = <4>;
>> +
>> +			regulator-name = "vdd-5v-sata";
> 
> Please keep capitalization of regulator names consistent. We use
> all-caps and underscores for the others (which mirrors the names in the
> schematics), so please stick with that for this one as well.
> 
Sure. I will fix this.
> Also, I'm wondering if perhaps you can clarify something here. My
> understanding is that SATA functionality is provided via a controller on
> the PCI bus. Why is it that we route the 5 V SATA power to the USB port?
> Oh wait... this is one of those eSATAp (hybrid) ports that can take
> either eSATA or USB, right? Do we need any additional setup to switch
> between eSATA and USB modes? Or is this all done in hardware? That is,
> if I plug in an eSATA, does it automatically hotplug detect the device
> as SATA and if I plug in a USB device, does it automatically detect it
> as USB?
> 
Yes, this 5V supply is for the eSATAp port. eSATA cable will deliver the 5V to
SATA device. No SATA/USB switch is required as USB SuperSpeed and PCIE-to-SATA
controller each has its own UPHY lane. SATA TX/RX pairs also have dedicated pins
at the eSATAp connector. External SATA drive can be hotplug and hardware will
detect automatically.

>> +			regulator-min-microvolt = <5000000>;
>> +			regulator-max-microvolt = <5000000>;
>> +			gpio = <&gpio TEGRA194_MAIN_GPIO(Z, 1) GPIO_ACTIVE_LOW>;
> 
> This will actually cause a warning on boot. For fixed regulators the
> polarity of the enable GPIO is not specified in the GPIO specifier.
> Instead you're supposed to use the boolean enable-active-high property
> to specify if the enable GPIO is active-high. By default the enable GPIO
> is considered to be active-low. The GPIO specifier needs to have the
> GPIO_ACTIVE_HIGH flag set regardless for backwards-compatibilitiy
> reasons.
> 
> Note that regulator@3 above your new entry does this wrongly, but
> next-20191002 should have a fix for that.
> 
Thanks for the information. I will fix this in the next revision.
>> +		};
>> +	};
>> +
>> +	padctl@3520000 {
> 
> Don't forget to move this into /cbb as well to match the changes to
> patch 5/6.
> 
Sure, will do.
>> +		avdd-usb-supply = <&vdd_usb_3v3>;
>> +		vclamp-usb-supply = <&vdd_1v8ao>;
>> +		ports {
> 
> Blank line between the above two for better readability.
> 
Okay.
>> +			usb2-1 {
>> +				vbus-supply = <&vdd_5v0_sys>;
>> +			};
>> +			usb2-3 {
> 
> Same here and below.
> 
>> +				vbus-supply = <&vdd_5v_sata>;
>> +			};
>> +			usb3-0 {
>> +				vbus-supply = <&vdd_5v0_sys>;
>> +			};
>> +			usb3-3 {
>> +				vbus-supply = <&vdd_5v0_sys>;
>> +			};
>> +		};
>>  	};
>>  };
>> diff --git a/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts b/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts
>> index d47cd8c4dd24..410221927dfa 100644
>> --- a/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts
>> +++ b/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts
>> @@ -222,4 +222,63 @@
>>  			};
>>  		};
>>  	};
>> +
>> +	padctl@3520000 {
> 
> Same comment as above. Move this into /cbb.
> 
>> +		status = "okay";
>> +
>> +		pads {
>> +			usb2 {
>> +				lanes {
>> +					usb2-1 {
>> +						status = "okay";
>> +					};
>> +					usb2-2 {
> 
> And blank lines for readability here and below.
> 
>> +						status = "okay";
>> +					};
>> +					usb2-3 {
>> +						status = "okay";
>> +					};
>> +				};
>> +			};
>> +			usb3 {
>> +				lanes {
>> +					usb3-0 {
>> +						status = "okay";
>> +					};
>> +					usb3-3 {
>> +						status = "okay";
>> +					};
>> +				};
>> +			};
>> +		};
>> +
>> +		ports {
>> +			usb2-1 {
>> +				mode = "host";
>> +				status = "okay";
>> +			};
>> +			usb2-3 {
>> +				mode = "host";
>> +				status = "okay";
>> +			};
>> +			usb3-0 {
>> +				nvidia,usb2-companion = <1>;
>> +				status = "okay";
>> +			};
>> +			usb3-3 {
>> +				nvidia,usb2-companion = <3>;
>> +				nvidia,disable-gen2;
>> +				status = "okay";
>> +			};
>> +		};
>> +	};
>> +
>> +	tegra_xhci: xhci@3610000 {
> 
> Also needs to move into /cbb. Also, you can drop the tegra_xhci label
> here since we never reference the controller from elsewhere.
> 
> Also make sure to update the name here to match the changes in 5/6.
> 
Got it. Thanks for the reminder.
> Thierry
> 
>> +		status = "okay";
>> +		phys =	<&{/padctl@3520000/pads/usb2/lanes/usb2-1}>,
>> +			<&{/padctl@3520000/pads/usb2/lanes/usb2-3}>,
>> +			<&{/padctl@3520000/pads/usb3/lanes/usb3-0}>,
>> +			<&{/padctl@3520000/pads/usb3/lanes/usb3-3}>;
>> +		phy-names = "usb2-1", "usb2-3", "usb3-0", "usb3-3";
>> +	};
>>  };
>> -- 
>> 2.17.1
>>