[PATCH RESEND V4 0/9] Tegra xHCI support

[PATCH RESEND V4 0/9] Tegra xHCI support

[Andrew Bresticker]

This series adds support for xHCI on NVIDIA Tegra SoCs.  This includes:
 - patches 1 and 2: adding a driver for the mailbox used to communicate
   with the xHCI controller's firmware,
 - patches 3 and 4: extending the XUSB pad controller driver to support
   the USB PHY types (UTMI, HSIC, and USB3),
 - patches 5 and 6: adding a xHCI host-controller driver, and
 - patches 7, 8, and 9: updating the relevant DT files.

The PHY and host drivers have compile-time dependencies on the mailbox
driver, and the host driver has compile-time dependencies on the PHY
driver.  It is probably best if these all get merged through the Tegra
tree.  This series still needs ACKs from the relevant maintainers for
the mailbox and xHCI host drivers and their device-tree bindings.

Tested on Venice2, Jetson TK1, and Big with a variety of USB2.0 and
USB3.0 memory sticks and ethernet dongles using controller firmware
recently posted by Andrew Chew [0].

Based on v3.18-rc2.  All other dependencies (xHCI modules and mailbox
series) have been merged.

Notes:
 - HSIC support is mostly untested and I think there are still some issues
   to work out there.  I do have a Tegra124 board with a HSIC hub so I'll
   try to sort those out later.
 - The XUSB padctl driver doesn't play nice with the existing Tegra USB2.0
   PHY driver, so all ports should be assigned to the XHCI controller.

Based on work by:
  a lot of people, but from what I can tell from the L4T tree [1], the
  original authors of the Tegra xHCI driver are:
    Ajay Gupta <ajayg@nvidia.com>
    Bharath Yadav <byadav@nvidia.com>

Changes from v3:
 - Fixed USB2.0 flakiness on Jetson-TK1.
 - Switched to 32-bit DMA mask for host.
 - Addressed Stephen's review comments.

Chagnes from v2:
 - Dropped mailbox channel specifier.  The mailbox driver allocates virtual
   channels backed by the single physical channel.
 - Added support for HS_CURR_LEVEL adjustment pinconfig property, which
   will be required for the Blaze board.
 - Addressed Stephen's review comments.

Changes from v1:
 - Converted mailbox driver to use the common mailbox framework.
 - Fixed up host driver so that it can now be built and used as a module.
 - Addressed Stephen's review comments.
 - Misc. cleanups.

Changes from RFC:
 - Dropped Tegra114 support.
 - Split out mailbox into separate driver.
 - Stopped using child xhci-plat device in xHCI host-controller driver.
 - Added PHY support to Thierry's XUSB padctl driver instead of in a separate
   USB PHY driver.
 - Added Jetson TK1 support.
 - Misc. cleanups.

[0] https://patchwork.ozlabs.org/patch/400110/
[1] git://nv-tegra.nvidia.com/linux-3.10.git

Andrew Bresticker (9):
  of: Add NVIDIA Tegra XUSB mailbox binding
  mailbox: Add NVIDIA Tegra XUSB mailbox driver
  of: Update Tegra XUSB pad controller binding for USB
  pinctrl: tegra-xusb: Add USB PHY support
  of: Add NVIDIA Tegra xHCI controller binding
  usb: xhci: Add NVIDIA Tegra xHCI host-controller driver
  ARM: tegra: Add Tegra124 XUSB mailbox and xHCI controller
  ARM: tegra: jetson-tk1: Add xHCI support
  ARM: tegra: venice2: Add xHCI support

 .../bindings/mailbox/nvidia,tegra124-xusb-mbox.txt |   32 +
 .../pinctrl/nvidia,tegra124-xusb-padctl.txt        |   56 +-
 .../bindings/usb/nvidia,tegra124-xhci.txt          |  104 ++
 arch/arm/boot/dts/tegra124-jetson-tk1.dts          |   46 +-
 arch/arm/boot/dts/tegra124-venice2.dts             |   79 +-
 arch/arm/boot/dts/tegra124.dtsi                    |   41 +
 drivers/mailbox/Kconfig                            |    3 +
 drivers/mailbox/Makefile                           |    2 +
 drivers/mailbox/tegra-xusb-mailbox.c               |  290 +++++
 drivers/pinctrl/Kconfig                            |    1 +
 drivers/pinctrl/pinctrl-tegra-xusb.c               | 1233 +++++++++++++++++++-
 drivers/usb/host/Kconfig                           |    9 +
 drivers/usb/host/Makefile                          |    1 +
 drivers/usb/host/xhci-tegra.c                      |  907 ++++++++++++++
 include/dt-bindings/pinctrl/pinctrl-tegra-xusb.h   |   20 +
 include/soc/tegra/xusb.h                           |   53 +
 16 files changed, 2797 insertions(+), 80 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mailbox/nvidia,tegra124-xusb-mbox.txt
 create mode 100644 Documentation/devicetree/bindings/usb/nvidia,tegra124-xhci.txt
 create mode 100644 drivers/mailbox/tegra-xusb-mailbox.c
 create mode 100644 drivers/usb/host/xhci-tegra.c
 create mode 100644 include/soc/tegra/xusb.h

-- 
2.1.0.rc2.206.gedb03e5

[PATCH RESEND V4 1/9] of: Add NVIDIA Tegra XUSB mailbox binding

[Andrew Bresticker]

Add device-tree bindings for the Tegra XUSB mailbox which will be used
for communication between the Tegra xHCI controller's firmware and the
host processor.

Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Reviewed-by: Stephen Warren <swarren@nvidia.com>
---
No changes from v3.
Changes from v2:
 - Dropped channel specifier.
 - Added pointer to mailbox documentation.
Changes from v1:
 - Updated to use common mailbox bindings.
---
 .../bindings/mailbox/nvidia,tegra124-xusb-mbox.txt | 32 ++++++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/nvidia,tegra124-xusb-mbox.txt

diff --git a/Documentation/devicetree/bindings/mailbox/nvidia,tegra124-xusb-mbox.txt b/Documentation/devicetree/bindings/mailbox/nvidia,tegra124-xusb-mbox.txt
new file mode 100644
index 0000000..b35ea6e
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/nvidia,tegra124-xusb-mbox.txt
@@ -0,0 +1,32 @@
+NVIDIA Tegra XUSB mailbox
+=========================
+
+The Tegra XUSB mailbox is used by the Tegra xHCI controller's firmware to
+communicate requests to the host and PHY drivers.
+
+Refer to ./mailbox.txt for generic information about mailbox device-tree
+bindings.
+
+Required properties:
+--------------------
+ - compatible: Should be "nvidia,tegra124-xusb-mbox".
+ - reg: Address and length of the XUSB FPCI registers.
+ - interrupts: XUSB mailbox interrupt.
+ - #mbox-cells: Should be 0.  There is only one physical channel.
+
+Example:
+--------
+	xusb_mbox: mailbox@0,70098000 {
+		compatible = "nvidia,tegra124-xusb-mbox";
+		reg = <0x0 0x70098000 0x0 0x1000>;
+		interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>;
+
+		#mbox-cells = <0>;
+	};
+
+	usb@0,70090000 {
+		...
+		mboxes = <&xusb_mbox>;
+		mbox-names = "xusb";
+		...
+	};
-- 
2.1.0.rc2.206.gedb03e5

[PATCH RESEND V4 2/9] mailbox: Add NVIDIA Tegra XUSB mailbox driver

[Andrew Bresticker]

The Tegra xHCI controller's firmware communicates requests to the host
processor through a mailbox interface.  While there is only a single
physical channel, messages sent by the controller can be divided
into two groups: those intended for the PHY driver and those intended
for the host-controller driver.  The requesting driver is assigned
one of two virtual channels when the single physical channel is
requested.  All incoming messages are sent to both virtual channels.

Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Reviewed-by: Stephen Warren <swarren@nvidia.com>
---
No changes from v3.
Changes from v2:
 - Fixed mailbox IRQ vs. channel alloc/free race.
 - Renamed defines to match TRM.
 - Dropped channel specifier and instead allocated virtual channels as they
   were requested.
 - Removed MODULE_ALIAS.
Changes from v1:
 - Converted to common mailbox framework.
 - Removed useless polling sequences in TX path.
 - Moved xusb include from linux/ to soc/tegra/
---
 drivers/mailbox/Kconfig              |   3 +
 drivers/mailbox/Makefile             |   2 +
 drivers/mailbox/tegra-xusb-mailbox.c | 290 +++++++++++++++++++++++++++++++++++
 include/soc/tegra/xusb.h             |  46 ++++++
 4 files changed, 341 insertions(+)
 create mode 100644 drivers/mailbox/tegra-xusb-mailbox.c
 create mode 100644 include/soc/tegra/xusb.h

diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index 9fd9c67..97369c2 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -33,4 +33,7 @@ config OMAP_MBOX_KFIFO_SIZE
 	  Specify the default size of mailbox's kfifo buffers (bytes).
 	  This can also be changed at runtime (via the mbox_kfifo_size
 	  module parameter).
+
+config TEGRA_XUSB_MBOX
+	def_bool y if ARCH_TEGRA
 endif
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index 94ed7ce..7f0af9c 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -5,3 +5,5 @@ obj-$(CONFIG_MAILBOX)		+= mailbox.o
 obj-$(CONFIG_PL320_MBOX)	+= pl320-ipc.o
 
 obj-$(CONFIG_OMAP2PLUS_MBOX)	+= omap-mailbox.o
+
+obj-$(CONFIG_TEGRA_XUSB_MBOX)	+= tegra-xusb-mailbox.o
diff --git a/drivers/mailbox/tegra-xusb-mailbox.c b/drivers/mailbox/tegra-xusb-mailbox.c
new file mode 100644
index 0000000..2d87b8a
--- /dev/null
+++ b/drivers/mailbox/tegra-xusb-mailbox.c
@@ -0,0 +1,290 @@
+/*
+ * NVIDIA Tegra XUSB mailbox driver
+ *
+ * Copyright (C) 2014 NVIDIA Corporation
+ * Copyright (C) 2014 Google, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ */
+
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/mailbox_controller.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#include <soc/tegra/xusb.h>
+
+#define XUSB_CFG_ARU_MBOX_CMD			0xe4
+#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		0xe8
+#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		0xec
+#define XUSB_CFG_ARU_MBOX_OWNER			0xf0
+#define  MBOX_OWNER_NONE			0
+#define  MBOX_OWNER_FW				1
+#define  MBOX_OWNER_SW				2
+#define XUSB_CFG_ARU_SMI_INTR			0x428
+#define  MBOX_SMI_INTR_FW_HANG			BIT(1)
+#define  MBOX_SMI_INTR_EN			BIT(3)
+
+struct tegra_xusb_mbox {
+	struct mbox_controller mbox;
+	int irq;
+	void __iomem *regs;
+	spinlock_t lock;
+	bool vchan_allocated[TEGRA_XUSB_MBOX_NUM_CHANS];
+};
+
+static inline u32 mbox_readl(struct tegra_xusb_mbox *mbox, unsigned long offset)
+{
+	return readl(mbox->regs + offset);
+}
+
+static inline void mbox_writel(struct tegra_xusb_mbox *mbox, u32 val,
+			       unsigned long offset)
+{
+	writel(val, mbox->regs + offset);
+}
+
+static inline u32 mbox_pack_msg(struct tegra_xusb_mbox_msg *msg)
+{
+	u32 val;
+
+	val = (msg->cmd & CMD_TYPE_MASK) << CMD_TYPE_SHIFT;
+	val |= (msg->data & CMD_DATA_MASK) << CMD_DATA_SHIFT;
+
+	return val;
+}
+
+static inline void mbox_unpack_msg(u32 val, struct tegra_xusb_mbox_msg *msg)
+{
+	msg->cmd = (val >> CMD_TYPE_SHIFT) & CMD_TYPE_MASK;
+	msg->data = (val >> CMD_DATA_SHIFT) & CMD_DATA_MASK;
+}
+
+static int tegra_xusb_mbox_send_data(struct mbox_chan *chan, void *data)
+{
+	struct tegra_xusb_mbox *mbox = dev_get_drvdata(chan->mbox->dev);
+	struct tegra_xusb_mbox_msg *msg = data;
+	unsigned long flags;
+	u32 reg, owner;
+
+	dev_dbg(mbox->mbox.dev, "TX message 0x%x:0x%x\n", msg->cmd, msg->data);
+
+	/* ACK/NAK must be sent with the controller as the mailbox owner */
+	if (msg->cmd == MBOX_CMD_ACK || msg->cmd == MBOX_CMD_NAK)
+		owner = MBOX_OWNER_FW;
+	else
+		owner = MBOX_OWNER_SW;
+
+	spin_lock_irqsave(&mbox->lock, flags);
+
+	/* Acquire mailbox */
+	if (mbox_readl(mbox, XUSB_CFG_ARU_MBOX_OWNER) != MBOX_OWNER_NONE) {
+		dev_err(mbox->mbox.dev, "Mailbox not idle\n");
+		goto busy;
+	}
+	mbox_writel(mbox, owner, XUSB_CFG_ARU_MBOX_OWNER);
+	if (mbox_readl(mbox, XUSB_CFG_ARU_MBOX_OWNER) != owner) {
+		dev_err(mbox->mbox.dev, "Failed to acquire mailbox");
+		goto busy;
+	}
+
+	mbox_writel(mbox, mbox_pack_msg(msg), XUSB_CFG_ARU_MBOX_DATA_IN);
+	reg = mbox_readl(mbox, XUSB_CFG_ARU_MBOX_CMD);
+	reg |= MBOX_INT_EN | MBOX_DEST_FALC;
+	mbox_writel(mbox, reg, XUSB_CFG_ARU_MBOX_CMD);
+
+	spin_unlock_irqrestore(&mbox->lock, flags);
+
+	return 0;
+busy:
+	spin_unlock_irqrestore(&mbox->lock, flags);
+	return -EBUSY;
+}
+
+static int tegra_xusb_mbox_startup(struct mbox_chan *chan)
+{
+	struct tegra_xusb_mbox *mbox = dev_get_drvdata(chan->mbox->dev);
+	int idx = chan - mbox->mbox.chans;
+	unsigned long flags;
+
+	spin_lock_irqsave(&mbox->lock, flags);
+	mbox->vchan_allocated[idx] = true;
+	spin_unlock_irqrestore(&mbox->lock, flags);
+
+	return 0;
+}
+
+static void tegra_xusb_mbox_shutdown(struct mbox_chan *chan)
+{
+	struct tegra_xusb_mbox *mbox = dev_get_drvdata(chan->mbox->dev);
+	int idx = chan - mbox->mbox.chans;
+	unsigned long flags;
+
+	spin_lock_irqsave(&mbox->lock, flags);
+	mbox->vchan_allocated[idx] = false;
+	spin_unlock_irqrestore(&mbox->lock, flags);
+}
+
+static bool tegra_xusb_mbox_last_tx_done(struct mbox_chan *chan)
+{
+	/*
+	 * Transmissions are assumed to be completed as soon as they are
+	 * written to the mailbox.
+	 */
+	return true;
+}
+
+static struct mbox_chan_ops tegra_xusb_mbox_chan_ops = {
+	.send_data = tegra_xusb_mbox_send_data,
+	.startup = tegra_xusb_mbox_startup,
+	.shutdown = tegra_xusb_mbox_shutdown,
+	.last_tx_done = tegra_xusb_mbox_last_tx_done,
+};
+
+static irqreturn_t tegra_xusb_mbox_irq(int irq, void *p)
+{
+	struct tegra_xusb_mbox *mbox = (struct tegra_xusb_mbox *)p;
+	struct tegra_xusb_mbox_msg msg;
+	int i;
+	u32 reg;
+
+	spin_lock(&mbox->lock);
+
+	/* Clear mbox interrupts */
+	reg = mbox_readl(mbox, XUSB_CFG_ARU_SMI_INTR);
+	if (reg & MBOX_SMI_INTR_FW_HANG)
+		dev_err(mbox->mbox.dev, "Controller firmware hang\n");
+	mbox_writel(mbox, reg, XUSB_CFG_ARU_SMI_INTR);
+
+	reg = mbox_readl(mbox, XUSB_CFG_ARU_MBOX_DATA_OUT);
+	mbox_unpack_msg(reg, &msg);
+
+	/*
+	 * Set the mailbox back to idle.  The recipient of the message is
+	 * responsible for sending an ACK/NAK, if necessary.
+	 */
+	reg = mbox_readl(mbox, XUSB_CFG_ARU_MBOX_CMD);
+	reg &= ~MBOX_DEST_SMI;
+	mbox_writel(mbox, reg, XUSB_CFG_ARU_MBOX_CMD);
+	mbox_writel(mbox, MBOX_OWNER_NONE, XUSB_CFG_ARU_MBOX_OWNER);
+
+	dev_dbg(mbox->mbox.dev, "RX message 0x%x:0x%x\n", msg.cmd, msg.data);
+	for (i = 0; i < ARRAY_SIZE(mbox->vchan_allocated); i++) {
+		if (mbox->vchan_allocated[i])
+			mbox_chan_received_data(&mbox->mbox.chans[i], &msg);
+	}
+
+	spin_unlock(&mbox->lock);
+
+	return IRQ_HANDLED;
+}
+
+static struct mbox_chan *tegra_xusb_mbox_of_xlate(struct mbox_controller *ctlr,
+					const struct of_phandle_args *sp)
+{
+	struct tegra_xusb_mbox *mbox = dev_get_drvdata(ctlr->dev);
+	struct mbox_chan *chan = NULL;
+	unsigned long flags;
+	int i;
+
+	/* Pick the first available (virtual) channel. */
+	spin_lock_irqsave(&mbox->lock, flags);
+	for (i = 0; i < ARRAY_SIZE(mbox->vchan_allocated); i++) {
+		if (!mbox->vchan_allocated[i]) {
+			chan = &ctlr->chans[i];
+			break;
+		}
+	}
+	spin_unlock_irqrestore(&mbox->lock, flags);
+
+	return chan;
+}
+
+static struct of_device_id tegra_xusb_mbox_of_match[] = {
+	{ .compatible = "nvidia,tegra124-xusb-mbox" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, tegra_xusb_mbox_of_match);
+
+static int tegra_xusb_mbox_probe(struct platform_device *pdev)
+{
+	struct tegra_xusb_mbox *mbox;
+	struct resource *res;
+	int ret;
+
+	mbox = devm_kzalloc(&pdev->dev, sizeof(*mbox), GFP_KERNEL);
+	if (!mbox)
+		return -ENOMEM;
+	platform_set_drvdata(pdev, mbox);
+	spin_lock_init(&mbox->lock);
+
+	mbox->mbox.dev = &pdev->dev;
+	mbox->mbox.chans = devm_kcalloc(&pdev->dev, TEGRA_XUSB_MBOX_NUM_CHANS,
+					sizeof(*mbox->mbox.chans), GFP_KERNEL);
+	if (!mbox->mbox.chans)
+		return -ENOMEM;
+	mbox->mbox.num_chans = TEGRA_XUSB_MBOX_NUM_CHANS;
+	mbox->mbox.ops = &tegra_xusb_mbox_chan_ops;
+	mbox->mbox.txdone_poll = true;
+	mbox->mbox.txpoll_period = 0; /* no need to actually poll */
+	mbox->mbox.of_xlate = tegra_xusb_mbox_of_xlate;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENODEV;
+	mbox->regs = devm_ioremap(&pdev->dev, res->start, resource_size(res));
+	if (!mbox->regs)
+		return -ENOMEM;
+
+	mbox->irq = platform_get_irq(pdev, 0);
+	if (mbox->irq < 0)
+		return mbox->irq;
+	ret = devm_request_irq(&pdev->dev, mbox->irq, tegra_xusb_mbox_irq, 0,
+			       dev_name(&pdev->dev), mbox);
+	if (ret < 0)
+		return ret;
+
+	ret = mbox_controller_register(&mbox->mbox);
+	if (ret < 0)
+		dev_err(&pdev->dev, "failed to register mailbox: %d\n", ret);
+
+	return ret;
+}
+
+static int tegra_xusb_mbox_remove(struct platform_device *pdev)
+{
+	struct tegra_xusb_mbox *mbox = platform_get_drvdata(pdev);
+
+	mbox_controller_unregister(&mbox->mbox);
+
+	return 0;
+}
+
+static struct platform_driver tegra_xusb_mbox_driver = {
+	.probe	= tegra_xusb_mbox_probe,
+	.remove	= tegra_xusb_mbox_remove,
+	.driver	= {
+		.name = "tegra-xusb-mbox",
+		.of_match_table = of_match_ptr(tegra_xusb_mbox_of_match),
+	},
+};
+module_platform_driver(tegra_xusb_mbox_driver);
+
+MODULE_AUTHOR("Andrew Bresticker <abrestic@chromium.org>");
+MODULE_DESCRIPTION("NVIDIA Tegra XUSB mailbox driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/soc/tegra/xusb.h b/include/soc/tegra/xusb.h
new file mode 100644
index 0000000..cfe211d
--- /dev/null
+++ b/include/soc/tegra/xusb.h
@@ -0,0 +1,46 @@
+/*
+ * Copyright (C) 2014 NVIDIA Corporation
+ * Copyright (C) 2014 Google, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ */
+
+#ifndef __SOC_TEGRA_XUSB_H__
+#define __SOC_TEGRA_XUSB_H__
+
+/* Two virtual channels: host + phy */
+#define TEGRA_XUSB_MBOX_NUM_CHANS 2
+
+/* Command requests from the firmware */
+enum tegra_xusb_mbox_cmd {
+	MBOX_CMD_MSG_ENABLED = 1,
+	MBOX_CMD_INC_FALC_CLOCK,
+	MBOX_CMD_DEC_FALC_CLOCK,
+	MBOX_CMD_INC_SSPI_CLOCK,
+	MBOX_CMD_DEC_SSPI_CLOCK,
+	MBOX_CMD_SET_BW, /* no ACK/NAK required */
+	MBOX_CMD_SET_SS_PWR_GATING,
+	MBOX_CMD_SET_SS_PWR_UNGATING,
+	MBOX_CMD_SAVE_DFE_CTLE_CTX,
+	MBOX_CMD_AIRPLANE_MODE_ENABLED, /* unused */
+	MBOX_CMD_AIRPLANE_MODE_DISABLED, /* unused */
+	MBOX_CMD_START_HSIC_IDLE,
+	MBOX_CMD_STOP_HSIC_IDLE,
+	MBOX_CMD_DBC_WAKE_STACK, /* unused */
+	MBOX_CMD_HSIC_PRETEND_CONNECT,
+
+	MBOX_CMD_MAX,
+
+	/* Response message to above commands */
+	MBOX_CMD_ACK = 128,
+	MBOX_CMD_NAK
+};
+
+struct tegra_xusb_mbox_msg {
+	u32 cmd;
+	u32 data;
+};
+
+#endif /* __SOC_TEGRA_XUSB_H__ */
-- 
2.1.0.rc2.206.gedb03e5

[PATCH RESEND V4 3/9] of: Update Tegra XUSB pad controller binding for USB

[Andrew Bresticker]

Add new bindings used for USB support by the Tegra XUSB pad controller.
This includes additional PHY types, USB-specific pinconfig properties, etc.

Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Reviewed-by: Stephen Warren <swarren@nvidia.com>
---
Changes from v2:
 - Added nvidia,otg-hs-curr-level-offset property.
 - Dropped "-otg" from VBUS supplies.
 - Added mbox-names property.
 - Removed extra whitespace.
Changes from v1:
 - Updated to use common mailbox bindings.
 - Made USB3 port-to-lane mappins a top-level binding rather than a pinconfig
   binding.
 - Add #defines for the padctl lanes.
---
 .../pinctrl/nvidia,tegra124-xusb-padctl.txt        | 56 ++++++++++++++++++++--
 include/dt-bindings/pinctrl/pinctrl-tegra-xusb.h   | 20 ++++++++
 2 files changed, 72 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-xusb-padctl.txt b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-xusb-padctl.txt
index 2f9c0bd..4a1b9475 100644
--- a/Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-xusb-padctl.txt
+++ b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-xusb-padctl.txt
@@ -21,6 +21,18 @@ Required properties:
   - padctl
 - #phy-cells: Should be 1. The specifier is the index of the PHY to reference.
   See <dt-bindings/pinctrl/pinctrl-tegra-xusb.h> for the list of valid values.
+- mboxes: Must contain an entry for the XUSB mailbox channel.
+  See ../mailbox/mailbox.txt for details.
+- mbox-names: Must include the following entries:
+  - xusb
+
+Optional properties:
+-------------------
+- vbus-{0,1,2}-supply: VBUS regulator for the corresponding UTMI pad.
+- vddio-hsic-supply: VDDIO regulator for the HSIC pads.
+- nvidia,usb3-port-{0,1}-lane: PCIe/SATA lane to which the corresponding USB3
+  port is mapped.  See <dt-bindings/pinctrl/pinctrl-tegra-xusb.h> for the list
+  of valid values.
 
 Lane muxing:
 ------------
@@ -50,6 +62,17 @@ Optional properties:
   pin or group should be assigned to. Valid values for function names are
   listed below.
 - nvidia,iddq: Enables IDDQ mode of the lane. (0: no, 1: yes)
+- nvidia,usb2-port-num: USB2 port (0, 1, or 2) to which the lane is mapped.
+- nvidia,hsic-strobe-trim: HSIC strobe trimmer value.
+- nvidia,hsic-rx-strobe-trim: HSIC RX strobe trimmer value.
+- nvidia,hsic-rx-data-trim: HSIC RX data trimmer value.
+- nvidia,hsic-tx-rtune-n: HSIC TX RTUNEN value.
+- nvidia,hsic-tx-rtune-p: HSIC TX RTUNEP value.
+- nvidia,hsic-tx-slew-n: HSIC TX SLEWN value.
+- nvidia,hsic-tx-slew-p: HSIC TX SLEWP value.
+- nvidia,hsic-auto-term: Enables HSIC AUTO_TERM. (0: no, 1: yes)
+- nvidia,otg-hs-curr-level-offset: Offset to be applied to the pad's fused
+  HS_CURR_LEVEL value.
 
 Note that not all of these properties are valid for all lanes. Lanes can be
 divided into three groups:
@@ -58,18 +81,21 @@ divided into three groups:
 
     Valid functions for this group are: "snps", "xusb", "uart", "rsvd".
 
-    The nvidia,iddq property does not apply to this group.
+    The nvidia,otg-hs-curr-level-offset property only applies.
 
   - ulpi-0, hsic-0, hsic-1:
 
     Valid functions for this group are: "snps", "xusb".
 
-    The nvidia,iddq property does not apply to this group.
+    The nvidia,hsic-* properties apply only to the pins hsic-{0,1} when
+    the function is xusb.
 
   - pcie-0, pcie-1, pcie-2, pcie-3, pcie-4, sata-0:
 
     Valid functions for this group are: "pcie", "usb3", "sata", "rsvd".
 
+    The nvidia,usb2-port-num property only applies and is required when
+    the function is usb3.
 
 Example:
 ========
@@ -82,6 +108,8 @@ SoC file extract:
 		reg = <0x0 0x7009f000 0x0 0x1000>;
 		resets = <&tegra_car 142>;
 		reset-names = "padctl";
+		mboxes = <&xusb_mbox>;
+		mbox-names = "xusb";
 
 		#phy-cells = <1>;
 	};
@@ -100,15 +128,35 @@ Board file extract:
 
 	...
 
+	usb@0,70090000 {
+		...
+
+		phys = <&padctl 5>, <&padctl 6>, <&padctl 7>;
+		phy-names = "utmi-1", "utmi-2", "usb3-0";
+
+		...
+	}
+
+	...
+
 	padctl: padctl@0,7009f000 {
 		pinctrl-0 = <&padctl_default>;
 		pinctrl-names = "default";
 
+		nvidia,usb3-port-0-lane = <TEGRA_XUSB_PADCTL_PIN_PCIE_0>;
+		vbus-2-supply = <&vdd_usb3_vbus>;
+
 		padctl_default: pinmux {
-			usb3 {
-				nvidia,lanes = "pcie-0", "pcie-1";
+			otg {
+				nvidia,lanes = "otg-1", "otg-2";
+				nvidia,function = "xusb";
+			};
+
+			usb3p0 {
+				nvidia,lanes = "pcie-0";
 				nvidia,function = "usb3";
 				nvidia,iddq = <0>;
+				nvidia,usb2-port-num = <2>;
 			};
 
 			pcie {
diff --git a/include/dt-bindings/pinctrl/pinctrl-tegra-xusb.h b/include/dt-bindings/pinctrl/pinctrl-tegra-xusb.h
index 914d56d..17b1aab 100644
--- a/include/dt-bindings/pinctrl/pinctrl-tegra-xusb.h
+++ b/include/dt-bindings/pinctrl/pinctrl-tegra-xusb.h
@@ -3,5 +3,25 @@
 
 #define TEGRA_XUSB_PADCTL_PCIE 0
 #define TEGRA_XUSB_PADCTL_SATA 1
+#define TEGRA_XUSB_PADCTL_USB3_P0 2
+#define TEGRA_XUSB_PADCTL_USB3_P1 3
+#define TEGRA_XUSB_PADCTL_UTMI_P0 4
+#define TEGRA_XUSB_PADCTL_UTMI_P1 5
+#define TEGRA_XUSB_PADCTL_UTMI_P2 6
+#define TEGRA_XUSB_PADCTL_HSIC_P0 7
+#define TEGRA_XUSB_PADCTL_HSIC_P1 8
+
+#define TEGRA_XUSB_PADCTL_PIN_OTG_0   0
+#define TEGRA_XUSB_PADCTL_PIN_OTG_1   1
+#define TEGRA_XUSB_PADCTL_PIN_OTG_2   2
+#define TEGRA_XUSB_PADCTL_PIN_ULPI_0  3
+#define TEGRA_XUSB_PADCTL_PIN_HSIC_0  4
+#define TEGRA_XUSB_PADCTL_PIN_HSIC_1  5
+#define TEGRA_XUSB_PADCTL_PIN_PCIE_0  6
+#define TEGRA_XUSB_PADCTL_PIN_PCIE_1  7
+#define TEGRA_XUSB_PADCTL_PIN_PCIE_2  8
+#define TEGRA_XUSB_PADCTL_PIN_PCIE_3  9
+#define TEGRA_XUSB_PADCTL_PIN_PCIE_4 10
+#define TEGRA_XUSB_PADCTL_PIN_SATA_0 11
 
 #endif /* _DT_BINDINGS_PINCTRL_TEGRA_XUSB_H */
-- 
2.1.0.rc2.206.gedb03e5

[PATCH RESEND V4 4/9] pinctrl: tegra-xusb: Add USB PHY support

[Andrew Bresticker]

In addition to the PCIe and SATA PHYs, the XUSB pad controller also
supports 3 UTMI, 2 HSIC, and 2 USB3 PHYs.  Each USB3 PHY uses a single
PCIe or SATA lane and is mapped to one of the three UTMI ports.

The xHCI controller will also send messages intended for the PHY driver,
so request and listen for messages on the mailbox's PHY channel.

Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Stephen Warren <swarren@nvidia.com>
---
No changes from v3.
Changes from v2:
 - Added support for nvidia,otg-hs-curr-level-offset property.
 - Moved mailbox request handling to workqueue.
 - Added filtering out of non-PHY mailbox messages.
 - Dropped "-otg" from VBUS supplies.
Changes from v1:
 - Updated to use common mailbox API.
 - Added SATA PHY enable sequence for USB3 ports using the SATA lane.
 - Made USB3 port-to-lane mappins a top-level binding rather than a pinconfig
   binding.
---
 drivers/pinctrl/Kconfig              |    1 +
 drivers/pinctrl/pinctrl-tegra-xusb.c | 1233 +++++++++++++++++++++++++++++++++-
 include/soc/tegra/xusb.h             |    7 +
 3 files changed, 1213 insertions(+), 28 deletions(-)

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index c6a66de..0f4cdef 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -163,6 +163,7 @@ config PINCTRL_TEGRA_XUSB
 	select GENERIC_PHY
 	select PINCONF
 	select PINMUX
+	select MAILBOX
 
 config PINCTRL_TZ1090
 	bool "Toumaz Xenif TZ1090 pin control driver"
diff --git a/drivers/pinctrl/pinctrl-tegra-xusb.c b/drivers/pinctrl/pinctrl-tegra-xusb.c
index 1631ec9..ecacc1d 100644
--- a/drivers/pinctrl/pinctrl-tegra-xusb.c
+++ b/drivers/pinctrl/pinctrl-tegra-xusb.c
@@ -13,23 +13,54 @@
 
 #include <linux/delay.h>
 #include <linux/io.h>
+#include <linux/mailbox_client.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/phy/phy.h>
 #include <linux/pinctrl/pinctrl.h>
 #include <linux/pinctrl/pinmux.h>
 #include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
 #include <linux/reset.h>
+#include <linux/workqueue.h>
+
+#include <soc/tegra/fuse.h>
+#include <soc/tegra/xusb.h>
 
 #include <dt-bindings/pinctrl/pinctrl-tegra-xusb.h>
 
 #include "core.h"
 #include "pinctrl-utils.h"
 
+#define FUSE_SKU_CALIB_HS_CURR_LEVEL_PADX_SHIFT(x) ((x) ? 15 : 0)
+#define FUSE_SKU_CALIB_HS_CURR_LEVEL_PAD_MASK 0x3f
+#define FUSE_SKU_CALIB_HS_IREF_CAP_SHIFT 13
+#define FUSE_SKU_CALIB_HS_IREF_CAP_MASK 0x3
+#define FUSE_SKU_CALIB_HS_SQUELCH_LEVEL_SHIFT 11
+#define FUSE_SKU_CALIB_HS_SQUELCH_LEVEL_MASK 0x3
+#define FUSE_SKU_CALIB_HS_TERM_RANGE_ADJ_SHIFT 7
+#define FUSE_SKU_CALIB_HS_TERM_RANGE_ADJ_MASK 0xf
+
+#define XUSB_PADCTL_USB2_PORT_CAP 0x008
+#define XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_SHIFT(x) ((x) * 4)
+#define XUSB_PADCTL_USB2_PORT_CAP_PORT_CAP_MASK 0x3
+#define XUSB_PADCTL_USB2_PORT_CAP_DISABLED 0x0
+#define XUSB_PADCTL_USB2_PORT_CAP_HOST 0x1
+#define XUSB_PADCTL_USB2_PORT_CAP_DEVICE 0x2
+#define XUSB_PADCTL_USB2_PORT_CAP_OTG 0x3
+
+#define XUSB_PADCTL_SS_PORT_MAP 0x014
+#define XUSB_PADCTL_SS_PORT_MAP_PORTX_SHIFT(x) ((x) * 4)
+#define XUSB_PADCTL_SS_PORT_MAP_PORT_MASK 0x7
+
 #define XUSB_PADCTL_ELPG_PROGRAM 0x01c
 #define XUSB_PADCTL_ELPG_PROGRAM_AUX_MUX_LP0_VCORE_DOWN (1 << 26)
 #define XUSB_PADCTL_ELPG_PROGRAM_AUX_MUX_LP0_CLAMP_EN_EARLY (1 << 25)
 #define XUSB_PADCTL_ELPG_PROGRAM_AUX_MUX_LP0_CLAMP_EN (1 << 24)
+#define XUSB_PADCTL_ELPG_PROGRAM_SSPX_ELPG_VCORE_DOWN(x) (1 << (18 + (x) * 4))
+#define XUSB_PADCTL_ELPG_PROGRAM_SSPX_ELPG_CLAMP_EN_EARLY(x) \
+							(1 << (17 + (x) * 4))
+#define XUSB_PADCTL_ELPG_PROGRAM_SSPX_ELPG_CLAMP_EN(x) (1 << (16 + (x) * 4))
 
 #define XUSB_PADCTL_IOPHY_PLL_P0_CTL1 0x040
 #define XUSB_PADCTL_IOPHY_PLL_P0_CTL1_PLL0_LOCKDET (1 << 19)
@@ -41,17 +72,136 @@
 #define XUSB_PADCTL_IOPHY_PLL_P0_CTL2_TXCLKREF_EN (1 << 5)
 #define XUSB_PADCTL_IOPHY_PLL_P0_CTL2_TXCLKREF_SEL (1 << 4)
 
+#define XUSB_PADCTL_IOPHY_USB3_PADX_CTL2(x) (0x058 + (x) * 4)
+#define XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_CDR_CNTL_SHIFT 24
+#define XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_CDR_CNTL_MASK 0xff
+#define XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_RX_EQ_Z_SHIFT 16
+#define XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_RX_EQ_Z_MASK 0x3f
+#define XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_RX_EQ_G_SHIFT 8
+#define XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_RX_EQ_G_MASK 0x3f
+#define XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_RX_EQ_SHIFT 8
+#define XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_RX_EQ_MASK 0xffff
+#define XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_RX_WANDER_SHIFT 4
+#define XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_RX_WANDER_MASK 0x7
+
+#define XUSB_PADCTL_IOPHY_USB3_PADX_CTL4(x) (0x068 + (x) * 4)
+#define XUSB_PADCTL_IOPHY_USB3_PAD_CTL4_DFE_CNTL_TAP_SHIFT 24
+#define XUSB_PADCTL_IOPHY_USB3_PAD_CTL4_DFE_CNTL_TAP_MASK 0x1f
+#define XUSB_PADCTL_IOPHY_USB3_PAD_CTL4_DFE_CNTL_AMP_SHIFT 16
+#define XUSB_PADCTL_IOPHY_USB3_PAD_CTL4_DFE_CNTL_AMP_MASK 0x7f
+
+#define XUSB_PADCTL_IOPHY_MISC_PAD_PX_CTL2(x) ((x) < 2 ? 0x078 + (x) * 4 : \
+					       0x0f8 + (x) * 4)
+#define XUSB_PADCTL_IOPHY_MISC_PAD_CTL2_SPARE_IN_SHIFT 28
+#define XUSB_PADCTL_IOPHY_MISC_PAD_CTL2_SPARE_IN_MASK 0x3
+
+#define XUSB_PADCTL_IOPHY_MISC_PAD_PX_CTL5(x) ((x) < 2 ? 0x090 + (x) * 4 : \
+					       0x11c + (x) * 4)
+#define XUSB_PADCTL_IOPHY_MISC_PAD_CTL5_RX_QEYE_EN (1 << 8)
+
+#define XUSB_PADCTL_IOPHY_MISC_PAD_PX_CTL6(x) ((x) < 2 ? 0x098 + (x) * 4 : \
+					       0x128 + (x) * 4)
+#define XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_SHIFT 24
+#define XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_G_Z_MASK 0x3f
+#define XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_TAP_MASK 0x1f
+#define XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_AMP_MASK 0x7f
+#define XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_SEL_SHIFT 16
+#define XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_SEL_MASK 0xff
+#define XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_SEL_G_Z 0x21
+#define XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_SEL_TAP 0x32
+#define XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_SEL_AMP 0x33
+#define XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_SEL_CTLE_Z 0x48
+#define XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_SEL_LATCH_G_Z 0xa1
+
+#define XUSB_PADCTL_USB2_OTG_PADX_CTL0(x) (0x0a0 + (x) * 4)
+#define XUSB_PADCTL_USB2_OTG_PAD_CTL0_PD_ZI (1 << 21)
+#define XUSB_PADCTL_USB2_OTG_PAD_CTL0_PD2 (1 << 20)
+#define XUSB_PADCTL_USB2_OTG_PAD_CTL0_PD (1 << 19)
+#define XUSB_PADCTL_USB2_OTG_PAD_CTL0_LS_RSLEW_SHIFT 14
+#define XUSB_PADCTL_USB2_OTG_PAD_CTL0_LS_RSLEW_MASK 0x3
+#define XUSB_PADCTL_USB2_OTG_PAD_CTL0_HS_SLEW_SHIFT 6
+#define XUSB_PADCTL_USB2_OTG_PAD_CTL0_HS_SLEW_MASK 0x3f
+#define XUSB_PADCTL_USB2_OTG_PAD_CTL0_HS_CURR_LEVEL_SHIFT 0
+#define XUSB_PADCTL_USB2_OTG_PAD_CTL0_HS_CURR_LEVEL_MASK 0x3f
+
+#define XUSB_PADCTL_USB2_OTG_PADX_CTL1(x) (0x0ac + (x) * 4)
+#define XUSB_PADCTL_USB2_OTG_PAD_CTL1_HS_IREF_CAP_SHIFT 9
+#define XUSB_PADCTL_USB2_OTG_PAD_CTL1_HS_IREF_CAP_MASK 0x3
+#define XUSB_PADCTL_USB2_OTG_PAD_CTL1_TERM_RANGE_ADJ_SHIFT 3
+#define XUSB_PADCTL_USB2_OTG_PAD_CTL1_TERM_RANGE_ADJ_MASK 0x7
+#define XUSB_PADCTL_USB2_OTG_PAD_CTL1_PD_DR (1 << 2)
+#define XUSB_PADCTL_USB2_OTG_PAD_CTL1_PD_DISC_FORCE_POWERUP (1 << 1)
+#define XUSB_PADCTL_USB2_OTG_PAD_CTL1_PD_CHRP_FORCE_POWERUP (1 << 0)
+
+#define XUSB_PADCTL_USB2_BIAS_PAD_CTL0 0x0b8
+#define XUSB_PADCTL_USB2_BIAS_PAD_CTL0_PD (1 << 12)
+#define XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_DISCON_LEVEL_SHIFT 2
+#define XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_DISCON_LEVEL_MASK 0x7
+#define XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_SQUELCH_LEVEL_SHIFT 0
+#define XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_SQUELCH_LEVEL_MASK 0x3
+
+#define XUSB_PADCTL_HSIC_PADX_CTL0(x) (0x0c0 + (x) * 4)
+#define XUSB_PADCTL_HSIC_PAD_CTL0_TX_RSLEWN_SHIFT 12
+#define XUSB_PADCTL_HSIC_PAD_CTL0_TX_RSLEWN_MASK 0x7
+#define XUSB_PADCTL_HSIC_PAD_CTL0_TX_RSLEWP_SHIFT 8
+#define XUSB_PADCTL_HSIC_PAD_CTL0_TX_RSLEWP_MASK 0x7
+#define XUSB_PADCTL_HSIC_PAD_CTL0_TX_RTUNEN_SHIFT 4
+#define XUSB_PADCTL_HSIC_PAD_CTL0_TX_RTUNEN_MASK 0x7
+#define XUSB_PADCTL_HSIC_PAD_CTL0_TX_RTUNEP_SHIFT 0
+#define XUSB_PADCTL_HSIC_PAD_CTL0_TX_RTUNEP_MASK 0x7
+
+#define XUSB_PADCTL_HSIC_PADX_CTL1(x) (0x0c8 + (x) * 4)
+#define XUSB_PADCTL_HSIC_PAD_CTL1_RPU_STROBE (1 << 10)
+#define XUSB_PADCTL_HSIC_PAD_CTL1_RPU_DATA (1 << 9)
+#define XUSB_PADCTL_HSIC_PAD_CTL1_RPD_STROBE (1 << 8)
+#define XUSB_PADCTL_HSIC_PAD_CTL1_RPD_DATA (1 << 7)
+#define XUSB_PADCTL_HSIC_PAD_CTL1_PD_ZI (1 << 5)
+#define XUSB_PADCTL_HSIC_PAD_CTL1_PD_RX (1 << 4)
+#define XUSB_PADCTL_HSIC_PAD_CTL1_PD_TRX (1 << 3)
+#define XUSB_PADCTL_HSIC_PAD_CTL1_PD_TX (1 << 2)
+#define XUSB_PADCTL_HSIC_PAD_CTL1_AUTO_TERM_EN (1 << 0)
+
+#define XUSB_PADCTL_HSIC_PADX_CTL2(x) (0x0d0 + (x) * 4)
+#define XUSB_PADCTL_HSIC_PAD_CTL2_RX_STROBE_TRIM_SHIFT 4
+#define XUSB_PADCTL_HSIC_PAD_CTL2_RX_STROBE_TRIM_MASK 0x7
+#define XUSB_PADCTL_HSIC_PAD_CTL2_RX_DATA_TRIM_SHIFT 0
+#define XUSB_PADCTL_HSIC_PAD_CTL2_RX_DATA_TRIM_MASK 0x7
+
+#define XUSB_PADCTL_HSIC_STRB_TRIM_CONTROL 0x0e0
+#define XUSB_PADCTL_HSIC_STRB_TRIM_CONTROL_STRB_TRIM_MASK 0x1f
+
 #define XUSB_PADCTL_IOPHY_PLL_S0_CTL1 0x138
 #define XUSB_PADCTL_IOPHY_PLL_S0_CTL1_PLL1_LOCKDET (1 << 27)
 #define XUSB_PADCTL_IOPHY_PLL_S0_CTL1_PLL1_MODE (1 << 24)
+#define XUSB_PADCTL_IOPHY_PLL_S0_CTL1_PLL0_REFCLK_NDIV_SHIFT 20
+#define XUSB_PADCTL_IOPHY_PLL_S0_CTL1_PLL0_REFCLK_NDIV_MASK 0x3
 #define XUSB_PADCTL_IOPHY_PLL_S0_CTL1_PLL_PWR_OVRD (1 << 3)
 #define XUSB_PADCTL_IOPHY_PLL_S0_CTL1_PLL_RST (1 << 1)
 #define XUSB_PADCTL_IOPHY_PLL_S0_CTL1_PLL_IDDQ (1 << 0)
 
+#define XUSB_PADCTL_IOPHY_PLL_S0_CTL2 0x13c
+#define XUSB_PADCTL_IOPHY_PLL_S0_CTL2_PLL1_CP_CNTL_SHIFT 20
+#define XUSB_PADCTL_IOPHY_PLL_S0_CTL2_PLL1_CP_CNTL_MASK 0xf
+#define XUSB_PADCTL_IOPHY_PLL_S0_CTL2_PLL0_CP_CNTL_SHIFT 16
+#define XUSB_PADCTL_IOPHY_PLL_S0_CTL2_PLL0_CP_CNTL_MASK 0xf
+#define XUSB_PADCTL_IOPHY_PLL_S0_CTL2_TCLKOUT_EN (1 << 12)
+#define XUSB_PADCTL_IOPHY_PLL_S0_CTL2_TXCLKREF_SEL (1 << 4)
+#define XUSB_PADCTL_IOPHY_PLL_S0_CTL2_XDIGCLK_SEL_SHIFT 0
+#define XUSB_PADCTL_IOPHY_PLL_S0_CTL2_XDIGCLK_SEL_MASK 0x7
+
+#define XUSB_PADCTL_IOPHY_PLL_S0_CTL3 0x140
+#define XUSB_PADCTL_IOPHY_PLL_S0_CTL3_RCAL_BYPASS (1 << 7)
+
 #define XUSB_PADCTL_IOPHY_MISC_PAD_S0_CTL1 0x148
 #define XUSB_PADCTL_IOPHY_MISC_PAD_S0_CTL1_IDDQ_OVRD (1 << 1)
 #define XUSB_PADCTL_IOPHY_MISC_PAD_S0_CTL1_IDDQ (1 << 0)
 
+#define XUSB_PADCTL_IOPHY_MISC_PAD_S0_CTL2 0x14c
+
+#define XUSB_PADCTL_IOPHY_MISC_PAD_S0_CTL5 0x158
+
+#define XUSB_PADCTL_IOPHY_MISC_PAD_S0_CTL6 0x15c
+
 struct tegra_xusb_padctl_function {
 	const char *name;
 	const char * const *groups;
@@ -72,6 +222,16 @@ struct tegra_xusb_padctl_soc {
 
 	const struct tegra_xusb_padctl_lane *lanes;
 	unsigned int num_lanes;
+
+	u32 rx_wander;
+	u32 rx_eq;
+	u32 cdr_cntl;
+	u32 dfe_cntl;
+	u32 hs_slew;
+	u32 ls_rslew[TEGRA_XUSB_UTMI_PHYS];
+	u32 hs_discon_level;
+	u32 spare_in;
+	int hsic_port_offset;
 };
 
 struct tegra_xusb_padctl_lane {
@@ -86,6 +246,22 @@ struct tegra_xusb_padctl_lane {
 	unsigned int num_funcs;
 };
 
+struct tegra_xusb_fuse_calibration {
+	u32 hs_curr_level[TEGRA_XUSB_UTMI_PHYS];
+	u32 hs_iref_cap;
+	u32 hs_term_range_adj;
+	u32 hs_squelch_level;
+};
+
+struct tegra_xusb_usb3_port {
+	int lane;
+	bool context_saved;
+	u32 tap1_val;
+	u32 amp_val;
+	u32 ctle_z_val;
+	u32 ctle_g_val;
+};
+
 struct tegra_xusb_padctl {
 	struct device *dev;
 	void __iomem *regs;
@@ -93,13 +269,25 @@ struct tegra_xusb_padctl {
 	struct reset_control *rst;
 
 	const struct tegra_xusb_padctl_soc *soc;
+	struct tegra_xusb_fuse_calibration calib;
 	struct pinctrl_dev *pinctrl;
 	struct pinctrl_desc desc;
 
 	struct phy_provider *provider;
-	struct phy *phys[2];
+	struct phy *phys[TEGRA_XUSB_NUM_PHYS];
 
 	unsigned int enable;
+
+	struct work_struct mbox_req_work;
+	struct tegra_xusb_mbox_msg mbox_req;
+	struct mbox_client mbox_client;
+	struct mbox_chan *mbox_chan;
+
+	struct tegra_xusb_usb3_port usb3_ports[TEGRA_XUSB_USB3_PHYS];
+	unsigned int utmi_enable;
+	unsigned int hs_curr_level_offset[TEGRA_XUSB_UTMI_PHYS];
+	struct regulator *vbus[TEGRA_XUSB_UTMI_PHYS];
+	struct regulator *vddio_hsic;
 };
 
 static inline void padctl_writel(struct tegra_xusb_padctl *padctl, u32 value,
@@ -114,6 +302,37 @@ static inline u32 padctl_readl(struct tegra_xusb_padctl *padctl,
 	return readl(padctl->regs + offset);
 }
 
+static inline bool is_otg_lane(unsigned int lane)
+{
+	return lane >= TEGRA_XUSB_PADCTL_PIN_OTG_0 &&
+		lane <= TEGRA_XUSB_PADCTL_PIN_OTG_2;
+}
+
+static inline bool is_hsic_lane(unsigned int lane)
+{
+	return lane >= TEGRA_XUSB_PADCTL_PIN_HSIC_0 &&
+		lane <= TEGRA_XUSB_PADCTL_PIN_HSIC_1;
+}
+
+static inline bool is_pcie_or_sata_lane(unsigned int lane)
+{
+	return lane >= TEGRA_XUSB_PADCTL_PIN_PCIE_0 &&
+		lane <= TEGRA_XUSB_PADCTL_PIN_SATA_0;
+}
+
+static int lane_to_usb3_port(struct tegra_xusb_padctl *padctl,
+			     unsigned int lane)
+{
+	int i;
+
+	for (i = 0; i < TEGRA_XUSB_USB3_PHYS; i++) {
+		if (padctl->usb3_ports[i].lane == lane)
+			return i;
+	}
+
+	return -1;
+}
+
 static int tegra_xusb_padctl_get_groups_count(struct pinctrl_dev *pinctrl)
 {
 	struct tegra_xusb_padctl *padctl = pinctrl_dev_get_drvdata(pinctrl);
@@ -131,6 +350,16 @@ static const char *tegra_xusb_padctl_get_group_name(struct pinctrl_dev *pinctrl,
 
 enum tegra_xusb_padctl_param {
 	TEGRA_XUSB_PADCTL_IDDQ,
+	TEGRA_XUSB_PADCTL_USB2_PORT_NUM,
+	TEGRA_XUSB_PADCTL_HSIC_STROBE_TRIM,
+	TEGRA_XUSB_PADCTL_HSIC_RX_STROBE_TRIM,
+	TEGRA_XUSB_PADCTL_HSIC_RX_DATA_TRIM,
+	TEGRA_XUSB_PADCTL_HSIC_TX_RTUNEN,
+	TEGRA_XUSB_PADCTL_HSIC_TX_RTUNEP,
+	TEGRA_XUSB_PADCTL_HSIC_TX_RSLEWN,
+	TEGRA_XUSB_PADCTL_HSIC_TX_RSLEWP,
+	TEGRA_XUSB_PADCTL_HSIC_AUTO_TERM,
+	TEGRA_XUSB_PADCTL_OTG_HS_CURR_LEVEL_OFFSET,
 };
 
 static const struct tegra_xusb_padctl_property {
@@ -138,6 +367,17 @@ static const struct tegra_xusb_padctl_property {
 	enum tegra_xusb_padctl_param param;
 } properties[] = {
 	{ "nvidia,iddq", TEGRA_XUSB_PADCTL_IDDQ },
+	{ "nvidia,usb2-port-num", TEGRA_XUSB_PADCTL_USB2_PORT_NUM },
+	{ "nvidia,hsic-strobe-trim", TEGRA_XUSB_PADCTL_HSIC_STROBE_TRIM },
+	{ "nvidia,hsic-rx-strobe-trim", TEGRA_XUSB_PADCTL_HSIC_RX_STROBE_TRIM },
+	{ "nvidia,hsic-rx-data-trim", TEGRA_XUSB_PADCTL_HSIC_RX_DATA_TRIM },
+	{ "nvidia,hsic-tx-rtune-n", TEGRA_XUSB_PADCTL_HSIC_TX_RTUNEN },
+	{ "nvidia,hsic-tx-rtune-p", TEGRA_XUSB_PADCTL_HSIC_TX_RTUNEP },
+	{ "nvidia,hsic-tx-rslew-n", TEGRA_XUSB_PADCTL_HSIC_TX_RSLEWN },
+	{ "nvidia,hsic-tx-rslew-p", TEGRA_XUSB_PADCTL_HSIC_TX_RSLEWP },
+	{ "nvidia,hsic-auto-term", TEGRA_XUSB_PADCTL_HSIC_AUTO_TERM },
+	{ "nvidia,otg-hs-curr-level-offset",
+	  TEGRA_XUSB_PADCTL_OTG_HS_CURR_LEVEL_OFFSET },
 };
 
 #define TEGRA_XUSB_PADCTL_PACK(param, value) ((param) << 16 | (value))
@@ -321,6 +561,7 @@ static int tegra_xusb_padctl_pinconf_group_get(struct pinctrl_dev *pinctrl,
 	struct tegra_xusb_padctl *padctl = pinctrl_dev_get_drvdata(pinctrl);
 	const struct tegra_xusb_padctl_lane *lane;
 	enum tegra_xusb_padctl_param param;
+	int port;
 	u32 value;
 
 	param = TEGRA_XUSB_PADCTL_UNPACK_PARAM(*config);
@@ -338,8 +579,127 @@ static int tegra_xusb_padctl_pinconf_group_get(struct pinctrl_dev *pinctrl,
 			value = 0;
 		else
 			value = 1;
+		break;
 
-		*config = TEGRA_XUSB_PADCTL_PACK(param, value);
+	case TEGRA_XUSB_PADCTL_USB2_PORT_NUM:
+		port = lane_to_usb3_port(padctl, group);
+		if (port < 0) {
+			dev_err(padctl->dev,
+				"Pin %d not mapped to USB3 port\n", group);
+			return -EINVAL;
+		}
+
+		value = padctl_readl(padctl, XUSB_PADCTL_SS_PORT_MAP) >>
+			XUSB_PADCTL_SS_PORT_MAP_PORTX_SHIFT(port);
+		value &= XUSB_PADCTL_SS_PORT_MAP_PORT_MASK;
+		break;
+
+	case TEGRA_XUSB_PADCTL_HSIC_STROBE_TRIM:
+		if (!is_hsic_lane(group)) {
+			dev_err(padctl->dev, "Pin %d not an HSIC\n", group);
+			return -EINVAL;
+		}
+
+		value = padctl_readl(padctl,
+				     XUSB_PADCTL_HSIC_STRB_TRIM_CONTROL);
+		value &= XUSB_PADCTL_HSIC_STRB_TRIM_CONTROL_STRB_TRIM_MASK;
+		break;
+
+	case TEGRA_XUSB_PADCTL_HSIC_RX_STROBE_TRIM:
+		if (!is_hsic_lane(group)) {
+			dev_err(padctl->dev, "Pin %d not an HSIC\n", group);
+			return -EINVAL;
+		}
+
+		port = group - TEGRA_XUSB_PADCTL_PIN_HSIC_0;
+		value = padctl_readl(padctl, XUSB_PADCTL_HSIC_PADX_CTL2(port)) >>
+			XUSB_PADCTL_HSIC_PAD_CTL2_RX_STROBE_TRIM_SHIFT;
+		value &= XUSB_PADCTL_HSIC_PAD_CTL2_RX_STROBE_TRIM_MASK;
+		break;
+
+	case TEGRA_XUSB_PADCTL_HSIC_RX_DATA_TRIM:
+		if (!is_hsic_lane(group)) {
+			dev_err(padctl->dev, "Pin %d not an HSIC\n", group);
+			return -EINVAL;
+		}
+
+		port = group - TEGRA_XUSB_PADCTL_PIN_HSIC_0;
+		value = padctl_readl(padctl, XUSB_PADCTL_HSIC_PADX_CTL2(port)) >>
+			XUSB_PADCTL_HSIC_PAD_CTL2_RX_DATA_TRIM_SHIFT;
+		value &= XUSB_PADCTL_HSIC_PAD_CTL2_RX_DATA_TRIM_MASK;
+		break;
+
+	case TEGRA_XUSB_PADCTL_HSIC_TX_RTUNEN:
+		if (!is_hsic_lane(group)) {
+			dev_err(padctl->dev, "Pin %d not an HSIC\n", group);
+			return -EINVAL;
+		}
+
+		port = group - TEGRA_XUSB_PADCTL_PIN_HSIC_0;
+		value = padctl_readl(padctl, XUSB_PADCTL_HSIC_PADX_CTL0(port)) >>
+			XUSB_PADCTL_HSIC_PAD_CTL0_TX_RTUNEN_SHIFT;
+		value &= XUSB_PADCTL_HSIC_PAD_CTL0_TX_RTUNEN_MASK;
+		break;
+
+	case TEGRA_XUSB_PADCTL_HSIC_TX_RTUNEP:
+		if (!is_hsic_lane(group)) {
+			dev_err(padctl->dev, "Pin %d not an HSIC\n", group);
+			return -EINVAL;
+		}
+
+		port = group - TEGRA_XUSB_PADCTL_PIN_HSIC_0;
+		value = padctl_readl(padctl, XUSB_PADCTL_HSIC_PADX_CTL0(port)) >>
+			XUSB_PADCTL_HSIC_PAD_CTL0_TX_RTUNEP_SHIFT;
+		value &= XUSB_PADCTL_HSIC_PAD_CTL0_TX_RTUNEP_MASK;
+		break;
+
+	case TEGRA_XUSB_PADCTL_HSIC_TX_RSLEWN:
+		if (!is_hsic_lane(group)) {
+			dev_err(padctl->dev, "Pin %d not an HSIC\n", group);
+			return -EINVAL;
+		}
+
+		port = group - TEGRA_XUSB_PADCTL_PIN_HSIC_0;
+		value = padctl_readl(padctl, XUSB_PADCTL_HSIC_PADX_CTL0(port)) >>
+			XUSB_PADCTL_HSIC_PAD_CTL0_TX_RSLEWN_SHIFT;
+		value &= XUSB_PADCTL_HSIC_PAD_CTL0_TX_RSLEWN_MASK;
+		break;
+
+	case TEGRA_XUSB_PADCTL_HSIC_TX_RSLEWP:
+		if (!is_hsic_lane(group)) {
+			dev_err(padctl->dev, "Pin %d not an HSIC\n", group);
+			return -EINVAL;
+		}
+
+		port = group - TEGRA_XUSB_PADCTL_PIN_HSIC_0;
+		value = padctl_readl(padctl, XUSB_PADCTL_HSIC_PADX_CTL0(port)) >>
+			XUSB_PADCTL_HSIC_PAD_CTL0_TX_RSLEWP_SHIFT;
+		value &= XUSB_PADCTL_HSIC_PAD_CTL0_TX_RSLEWP_MASK;
+		break;
+
+	case TEGRA_XUSB_PADCTL_HSIC_AUTO_TERM:
+		if (!is_hsic_lane(group)) {
+			dev_err(padctl->dev, "Pin %d not an HSIC\n", group);
+			return -EINVAL;
+		}
+
+		port = group - TEGRA_XUSB_PADCTL_PIN_HSIC_0;
+		value = padctl_readl(padctl, XUSB_PADCTL_HSIC_PADX_CTL1(port));
+		if (value & XUSB_PADCTL_HSIC_PAD_CTL1_AUTO_TERM_EN)
+			value = 1;
+		else
+			value = 0;
+		break;
+
+	case TEGRA_XUSB_PADCTL_OTG_HS_CURR_LEVEL_OFFSET:
+		if (!is_otg_lane(group)) {
+			dev_err(padctl->dev, "Pin %d is not an OTG pad\n",
+				group);
+			return -EINVAL;
+		}
+
+		port = group - TEGRA_XUSB_PADCTL_PIN_OTG_0;
+		value = padctl->hs_curr_level_offset[port];
 		break;
 
 	default:
@@ -348,6 +708,7 @@ static int tegra_xusb_padctl_pinconf_group_get(struct pinctrl_dev *pinctrl,
 		return -ENOTSUPP;
 	}
 
+	*config = TEGRA_XUSB_PADCTL_PACK(param, value);
 	return 0;
 }
 
@@ -362,6 +723,7 @@ static int tegra_xusb_padctl_pinconf_group_set(struct pinctrl_dev *pinctrl,
 	unsigned long value;
 	unsigned int i;
 	u32 regval;
+	int port;
 
 	lane = &padctl->soc->lanes[group];
 
@@ -385,6 +747,190 @@ static int tegra_xusb_padctl_pinconf_group_set(struct pinctrl_dev *pinctrl,
 			padctl_writel(padctl, regval, lane->offset);
 			break;
 
+		case TEGRA_XUSB_PADCTL_USB2_PORT_NUM:
+			if (value >= TEGRA_XUSB_UTMI_PHYS) {
+				dev_err(padctl->dev, "Invalid USB2 port: %lu\n",
+					value);
+				return -EINVAL;
+			}
+			if (!is_pcie_or_sata_lane(group)) {
+				dev_err(padctl->dev,
+					"USB2 port not applicable for pin %d\n",
+					group);
+				return -EINVAL;
+			}
+			port = lane_to_usb3_port(padctl, group);
+			if (port < 0) {
+				dev_err(padctl->dev,
+					"Pin %d not mapped to USB3 port\n",
+					group);
+				return -EINVAL;
+			}
+
+			regval = padctl_readl(padctl, XUSB_PADCTL_SS_PORT_MAP);
+			regval &= ~(XUSB_PADCTL_SS_PORT_MAP_PORT_MASK <<
+				    XUSB_PADCTL_SS_PORT_MAP_PORTX_SHIFT(port));
+			regval |= value <<
+				XUSB_PADCTL_SS_PORT_MAP_PORTX_SHIFT(port);
+			padctl_writel(padctl, regval, XUSB_PADCTL_SS_PORT_MAP);
+			break;
+
+		case TEGRA_XUSB_PADCTL_HSIC_STROBE_TRIM:
+			if (!is_hsic_lane(group)) {
+				dev_err(padctl->dev, "Pin %d not an HSIC\n",
+					group);
+				return -EINVAL;
+			}
+
+			value &= XUSB_PADCTL_HSIC_STRB_TRIM_CONTROL_STRB_TRIM_MASK;
+			padctl_writel(padctl, value,
+				      XUSB_PADCTL_HSIC_STRB_TRIM_CONTROL);
+			break;
+
+		case TEGRA_XUSB_PADCTL_HSIC_RX_STROBE_TRIM:
+			if (!is_hsic_lane(group)) {
+				dev_err(padctl->dev, "Pin %d not an HSIC\n",
+					group);
+				return -EINVAL;
+			}
+
+			port = group - TEGRA_XUSB_PADCTL_PIN_HSIC_0;
+			value &= XUSB_PADCTL_HSIC_PAD_CTL2_RX_STROBE_TRIM_MASK;
+			regval = padctl_readl(padctl,
+					      XUSB_PADCTL_HSIC_PADX_CTL2(port));
+			regval &= ~(XUSB_PADCTL_HSIC_PAD_CTL2_RX_STROBE_TRIM_MASK <<
+				    XUSB_PADCTL_HSIC_PAD_CTL2_RX_STROBE_TRIM_SHIFT);
+			regval |= value <<
+				XUSB_PADCTL_HSIC_PAD_CTL2_RX_STROBE_TRIM_SHIFT;
+			padctl_writel(padctl, regval,
+				      XUSB_PADCTL_HSIC_PADX_CTL2(port));
+			break;
+
+		case TEGRA_XUSB_PADCTL_HSIC_RX_DATA_TRIM:
+			if (!is_hsic_lane(group)) {
+				dev_err(padctl->dev, "Pin %d not an HSIC\n",
+					group);
+				return -EINVAL;
+			}
+
+			port = group - TEGRA_XUSB_PADCTL_PIN_HSIC_0;
+			value &= XUSB_PADCTL_HSIC_PAD_CTL2_RX_DATA_TRIM_MASK;
+			regval = padctl_readl(padctl,
+					      XUSB_PADCTL_HSIC_PADX_CTL2(port));
+			regval &= ~(XUSB_PADCTL_HSIC_PAD_CTL2_RX_DATA_TRIM_MASK <<
+				    XUSB_PADCTL_HSIC_PAD_CTL2_RX_DATA_TRIM_SHIFT);
+			regval |= value <<
+				XUSB_PADCTL_HSIC_PAD_CTL2_RX_DATA_TRIM_SHIFT;
+			padctl_writel(padctl, regval,
+				      XUSB_PADCTL_HSIC_PADX_CTL2(port));
+			break;
+
+		case TEGRA_XUSB_PADCTL_HSIC_TX_RTUNEN:
+			if (!is_hsic_lane(group)) {
+				dev_err(padctl->dev, "Pin %d not an HSIC\n",
+					group);
+				return -EINVAL;
+			}
+
+			port = group - TEGRA_XUSB_PADCTL_PIN_HSIC_0;
+			value &= XUSB_PADCTL_HSIC_PAD_CTL0_TX_RTUNEN_MASK;
+			regval = padctl_readl(padctl,
+					      XUSB_PADCTL_HSIC_PADX_CTL0(port));
+			regval &= ~(XUSB_PADCTL_HSIC_PAD_CTL0_TX_RTUNEN_MASK <<
+				    XUSB_PADCTL_HSIC_PAD_CTL0_TX_RTUNEN_SHIFT);
+			regval |= value <<
+				XUSB_PADCTL_HSIC_PAD_CTL0_TX_RTUNEN_SHIFT;
+			padctl_writel(padctl, regval,
+				      XUSB_PADCTL_HSIC_PADX_CTL0(port));
+			break;
+
+		case TEGRA_XUSB_PADCTL_HSIC_TX_RTUNEP:
+			if (!is_hsic_lane(group)) {
+				dev_err(padctl->dev, "Pin %d not an HSIC\n",
+					group);
+				return -EINVAL;
+			}
+
+			port = group - TEGRA_XUSB_PADCTL_PIN_HSIC_0;
+			value &= XUSB_PADCTL_HSIC_PAD_CTL0_TX_RTUNEP_MASK;
+			regval = padctl_readl(padctl,
+					      XUSB_PADCTL_HSIC_PADX_CTL0(port));
+			regval &= ~(XUSB_PADCTL_HSIC_PAD_CTL0_TX_RTUNEP_MASK <<
+				    XUSB_PADCTL_HSIC_PAD_CTL0_TX_RTUNEP_SHIFT);
+			regval |= value <<
+				XUSB_PADCTL_HSIC_PAD_CTL0_TX_RTUNEP_SHIFT;
+			padctl_writel(padctl, regval,
+				      XUSB_PADCTL_HSIC_PADX_CTL0(port));
+			break;
+
+		case TEGRA_XUSB_PADCTL_HSIC_TX_RSLEWN:
+			if (!is_hsic_lane(group)) {
+				dev_err(padctl->dev, "Pin %d not an HSIC\n",
+					group);
+				return -EINVAL;
+			}
+
+			port = group - TEGRA_XUSB_PADCTL_PIN_HSIC_0;
+			value &= XUSB_PADCTL_HSIC_PAD_CTL0_TX_RSLEWN_MASK;
+			regval = padctl_readl(padctl,
+					      XUSB_PADCTL_HSIC_PADX_CTL0(port));
+			regval &= ~(XUSB_PADCTL_HSIC_PAD_CTL0_TX_RSLEWN_MASK <<
+				    XUSB_PADCTL_HSIC_PAD_CTL0_TX_RSLEWN_SHIFT);
+			regval |= value <<
+				XUSB_PADCTL_HSIC_PAD_CTL0_TX_RSLEWN_SHIFT;
+			padctl_writel(padctl, regval,
+				      XUSB_PADCTL_HSIC_PADX_CTL0(port));
+			break;
+
+		case TEGRA_XUSB_PADCTL_HSIC_TX_RSLEWP:
+			if (!is_hsic_lane(group)) {
+				dev_err(padctl->dev, "Pin %d not an HSIC\n",
+					group);
+				return -EINVAL;
+			}
+
+			port = group - TEGRA_XUSB_PADCTL_PIN_HSIC_0;
+			value &= XUSB_PADCTL_HSIC_PAD_CTL0_TX_RSLEWP_MASK;
+			regval = padctl_readl(padctl,
+					      XUSB_PADCTL_HSIC_PADX_CTL0(port));
+			regval &= ~(XUSB_PADCTL_HSIC_PAD_CTL0_TX_RSLEWP_MASK <<
+				    XUSB_PADCTL_HSIC_PAD_CTL0_TX_RSLEWP_SHIFT);
+			regval |= value <<
+				XUSB_PADCTL_HSIC_PAD_CTL0_TX_RSLEWP_SHIFT;
+			padctl_writel(padctl, regval,
+				      XUSB_PADCTL_HSIC_PADX_CTL0(port));
+			break;
+
+		case TEGRA_XUSB_PADCTL_HSIC_AUTO_TERM:
+			if (!is_hsic_lane(group)) {
+				dev_err(padctl->dev, "Pin %d not an HSIC\n",
+					group);
+				return -EINVAL;
+			}
+
+			port = group - TEGRA_XUSB_PADCTL_PIN_HSIC_0;
+			regval = padctl_readl(padctl,
+					      XUSB_PADCTL_HSIC_PADX_CTL1(port));
+			if (!value)
+				regval &= ~XUSB_PADCTL_HSIC_PAD_CTL1_AUTO_TERM_EN;
+			else
+				regval |= XUSB_PADCTL_HSIC_PAD_CTL1_AUTO_TERM_EN;
+			padctl_writel(padctl, regval,
+				      XUSB_PADCTL_HSIC_PADX_CTL1(port));
+			break;
+
+		case TEGRA_XUSB_PADCTL_OTG_HS_CURR_LEVEL_OFFSET:
+			if (!is_otg_lane(group)) {
+				dev_err(padctl->dev,
+					"Pin %d is not an OTG pad\n", group);
+				return -EINVAL;
+			}
+
+			port = group - TEGRA_XUSB_PADCTL_PIN_OTG_0;
+			value &= XUSB_PADCTL_USB2_OTG_PAD_CTL0_HS_CURR_LEVEL_MASK;
+			padctl->hs_curr_level_offset[port] = value;
+			break;
+
 		default:
 			dev_err(padctl->dev,
 				"invalid configuration parameter: %04x\n",
@@ -671,6 +1217,529 @@ static const struct phy_ops sata_phy_ops = {
 	.owner = THIS_MODULE,
 };
 
+static int usb3_phy_to_port(struct phy *phy)
+{
+	struct tegra_xusb_padctl *padctl = phy_get_drvdata(phy);
+	int i;
+
+	for (i = 0; i < TEGRA_XUSB_USB3_PHYS; i++) {
+		if (phy == padctl->phys[TEGRA_XUSB_PADCTL_USB3_P0 + i])
+			break;
+	}
+	BUG_ON(i == TEGRA_XUSB_USB3_PHYS);
+
+	return i;
+}
+
+static int usb3_phy_power_on(struct phy *phy)
+{
+	struct tegra_xusb_padctl *padctl = phy_get_drvdata(phy);
+	int port = usb3_phy_to_port(phy);
+	int lane = padctl->usb3_ports[port].lane;
+	u32 value, offset;
+
+	if (!is_pcie_or_sata_lane(lane)) {
+		dev_err(padctl->dev, "USB3 PHY %d mapped to invalid lane: %d\n",
+			port, lane);
+		return -EINVAL;
+	}
+
+	value = padctl_readl(padctl, XUSB_PADCTL_IOPHY_USB3_PADX_CTL2(port));
+	value &= ~((XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_RX_WANDER_MASK <<
+		    XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_RX_WANDER_SHIFT) |
+		   (XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_RX_EQ_MASK <<
+		    XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_RX_EQ_SHIFT) |
+		   (XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_CDR_CNTL_MASK <<
+		    XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_CDR_CNTL_SHIFT));
+	value |= (padctl->soc->rx_wander <<
+		  XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_RX_WANDER_SHIFT) |
+		 (padctl->soc->cdr_cntl <<
+		  XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_CDR_CNTL_SHIFT) |
+		 (padctl->soc->rx_eq <<
+		  XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_RX_EQ_SHIFT);
+	if (padctl->usb3_ports[port].context_saved) {
+		value &= ~((XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_RX_EQ_G_MASK <<
+			    XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_RX_EQ_G_SHIFT) |
+			   (XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_RX_EQ_Z_MASK <<
+			    XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_RX_EQ_Z_SHIFT));
+		value |= (padctl->usb3_ports[port].ctle_g_val <<
+			  XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_RX_EQ_G_SHIFT) |
+			 (padctl->usb3_ports[port].ctle_z_val <<
+			  XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_RX_EQ_Z_SHIFT);
+	}
+	padctl_writel(padctl, value, XUSB_PADCTL_IOPHY_USB3_PADX_CTL2(port));
+
+	value = padctl->soc->dfe_cntl;
+	if (padctl->usb3_ports[port].context_saved) {
+		value &= ~((XUSB_PADCTL_IOPHY_USB3_PAD_CTL4_DFE_CNTL_TAP_MASK <<
+			    XUSB_PADCTL_IOPHY_USB3_PAD_CTL4_DFE_CNTL_TAP_SHIFT) |
+			   (XUSB_PADCTL_IOPHY_USB3_PAD_CTL4_DFE_CNTL_AMP_MASK <<
+			    XUSB_PADCTL_IOPHY_USB3_PAD_CTL4_DFE_CNTL_AMP_SHIFT));
+		value |= (padctl->usb3_ports[port].tap1_val <<
+			  XUSB_PADCTL_IOPHY_USB3_PAD_CTL4_DFE_CNTL_TAP_SHIFT) |
+			 (padctl->usb3_ports[port].amp_val <<
+			  XUSB_PADCTL_IOPHY_USB3_PAD_CTL4_DFE_CNTL_AMP_SHIFT);
+	}
+	padctl_writel(padctl, value, XUSB_PADCTL_IOPHY_USB3_PADX_CTL4(port));
+
+	offset = (lane == TEGRA_XUSB_PADCTL_PIN_SATA_0) ?
+		XUSB_PADCTL_IOPHY_MISC_PAD_S0_CTL2 :
+		XUSB_PADCTL_IOPHY_MISC_PAD_PX_CTL2(lane -
+						TEGRA_XUSB_PADCTL_PIN_PCIE_0);
+	value = padctl_readl(padctl, offset);
+	value &= ~(XUSB_PADCTL_IOPHY_MISC_PAD_CTL2_SPARE_IN_MASK <<
+		   XUSB_PADCTL_IOPHY_MISC_PAD_CTL2_SPARE_IN_SHIFT);
+	value |= padctl->soc->spare_in <<
+		XUSB_PADCTL_IOPHY_MISC_PAD_CTL2_SPARE_IN_SHIFT;
+	padctl_writel(padctl, value, offset);
+
+	offset = (lane == TEGRA_XUSB_PADCTL_PIN_SATA_0) ?
+		XUSB_PADCTL_IOPHY_MISC_PAD_S0_CTL5 :
+		XUSB_PADCTL_IOPHY_MISC_PAD_PX_CTL5(lane -
+						TEGRA_XUSB_PADCTL_PIN_PCIE_0);
+	value = padctl_readl(padctl, offset);
+	value |= XUSB_PADCTL_IOPHY_MISC_PAD_CTL5_RX_QEYE_EN;
+	padctl_writel(padctl, value, offset);
+
+	/* Enable SATA PHY when SATA lane is used */
+	if (lane == TEGRA_XUSB_PADCTL_PIN_SATA_0) {
+		value = padctl_readl(padctl, XUSB_PADCTL_IOPHY_PLL_S0_CTL1);
+		value &= ~(XUSB_PADCTL_IOPHY_PLL_S0_CTL1_PLL0_REFCLK_NDIV_MASK <<
+			   XUSB_PADCTL_IOPHY_PLL_S0_CTL1_PLL0_REFCLK_NDIV_SHIFT);
+		value |= 0x2 <<
+			XUSB_PADCTL_IOPHY_PLL_S0_CTL1_PLL0_REFCLK_NDIV_SHIFT;
+		padctl_writel(padctl, value, XUSB_PADCTL_IOPHY_PLL_S0_CTL1);
+
+		value = padctl_readl(padctl, XUSB_PADCTL_IOPHY_PLL_S0_CTL2);
+		value &= ~((XUSB_PADCTL_IOPHY_PLL_S0_CTL2_XDIGCLK_SEL_MASK <<
+			    XUSB_PADCTL_IOPHY_PLL_S0_CTL2_XDIGCLK_SEL_SHIFT) |
+			   (XUSB_PADCTL_IOPHY_PLL_S0_CTL2_PLL1_CP_CNTL_MASK <<
+			    XUSB_PADCTL_IOPHY_PLL_S0_CTL2_PLL1_CP_CNTL_SHIFT) |
+			   (XUSB_PADCTL_IOPHY_PLL_S0_CTL2_PLL0_CP_CNTL_MASK <<
+			    XUSB_PADCTL_IOPHY_PLL_S0_CTL2_PLL0_CP_CNTL_SHIFT) |
+			   XUSB_PADCTL_IOPHY_PLL_S0_CTL2_TCLKOUT_EN);
+		value |= (0x7 <<
+			  XUSB_PADCTL_IOPHY_PLL_S0_CTL2_XDIGCLK_SEL_SHIFT) |
+			 (0x8 <<
+			  XUSB_PADCTL_IOPHY_PLL_S0_CTL2_PLL1_CP_CNTL_SHIFT) |
+			 (0x8 <<
+			  XUSB_PADCTL_IOPHY_PLL_S0_CTL2_PLL0_CP_CNTL_SHIFT) |
+			 XUSB_PADCTL_IOPHY_PLL_S0_CTL2_TXCLKREF_SEL;
+		padctl_writel(padctl, value, XUSB_PADCTL_IOPHY_PLL_S0_CTL2);
+
+		value = padctl_readl(padctl, XUSB_PADCTL_IOPHY_PLL_S0_CTL3);
+		value &= ~XUSB_PADCTL_IOPHY_PLL_S0_CTL3_RCAL_BYPASS;
+		padctl_writel(padctl, value, XUSB_PADCTL_IOPHY_PLL_S0_CTL3);
+	}
+
+	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM);
+	value &= ~XUSB_PADCTL_ELPG_PROGRAM_SSPX_ELPG_CLAMP_EN_EARLY(port);
+	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM);
+
+	usleep_range(100, 200);
+
+	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM);
+	value &= ~XUSB_PADCTL_ELPG_PROGRAM_SSPX_ELPG_CLAMP_EN_EARLY(port);
+	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM);
+
+	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM);
+	value &= ~XUSB_PADCTL_ELPG_PROGRAM_SSPX_ELPG_VCORE_DOWN(port);
+	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM);
+
+	return 0;
+}
+
+static int usb3_phy_power_off(struct phy *phy)
+{
+	struct tegra_xusb_padctl *padctl = phy_get_drvdata(phy);
+	int port = usb3_phy_to_port(phy);
+	u32 value;
+
+	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM);
+	value |= XUSB_PADCTL_ELPG_PROGRAM_SSPX_ELPG_CLAMP_EN_EARLY(port);
+	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM);
+
+	usleep_range(100, 200);
+
+	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM);
+	value |= XUSB_PADCTL_ELPG_PROGRAM_SSPX_ELPG_CLAMP_EN_EARLY(port);
+	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM);
+
+	usleep_range(250, 350);
+
+	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM);
+	value |= XUSB_PADCTL_ELPG_PROGRAM_SSPX_ELPG_VCORE_DOWN(port);
+	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM);
+
+	return 0;
+}
+
+static void usb3_phy_save_context(struct tegra_xusb_padctl *padctl, int port)
+{
+	int lane = padctl->usb3_ports[port].lane;
+	u32 value, offset;
+
+	padctl->usb3_ports[port].context_saved = true;
+
+	offset = (lane == TEGRA_XUSB_PADCTL_PIN_SATA_0) ?
+		XUSB_PADCTL_IOPHY_MISC_PAD_S0_CTL6 :
+		XUSB_PADCTL_IOPHY_MISC_PAD_PX_CTL6(lane -
+						TEGRA_XUSB_PADCTL_PIN_PCIE_0);
+
+	value = padctl_readl(padctl, offset);
+	value &= ~(XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_SEL_MASK <<
+		   XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_SEL_SHIFT);
+	value |= XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_SEL_TAP <<
+		XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_SEL_SHIFT;
+	padctl_writel(padctl, value, offset);
+
+	value = padctl_readl(padctl, offset) >>
+		XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_SHIFT;
+	padctl->usb3_ports[port].tap1_val = value &
+		XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_TAP_MASK;
+
+	value = padctl_readl(padctl, offset);
+	value &= ~(XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_SEL_MASK <<
+		   XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_SEL_SHIFT);
+	value |= XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_SEL_AMP <<
+		XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_SEL_SHIFT;
+	padctl_writel(padctl, value, offset);
+
+	value = padctl_readl(padctl, offset) >>
+		XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_SHIFT;
+	padctl->usb3_ports[port].amp_val = value &
+		XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_AMP_MASK;
+
+	value = padctl_readl(padctl, XUSB_PADCTL_IOPHY_USB3_PADX_CTL4(port));
+	value &= ~((XUSB_PADCTL_IOPHY_USB3_PAD_CTL4_DFE_CNTL_TAP_MASK <<
+		    XUSB_PADCTL_IOPHY_USB3_PAD_CTL4_DFE_CNTL_TAP_SHIFT) |
+		   (XUSB_PADCTL_IOPHY_USB3_PAD_CTL4_DFE_CNTL_AMP_MASK <<
+		    XUSB_PADCTL_IOPHY_USB3_PAD_CTL4_DFE_CNTL_AMP_SHIFT));
+	value |= (padctl->usb3_ports[port].tap1_val <<
+		  XUSB_PADCTL_IOPHY_USB3_PAD_CTL4_DFE_CNTL_TAP_SHIFT) |
+		 (padctl->usb3_ports[port].amp_val <<
+		  XUSB_PADCTL_IOPHY_USB3_PAD_CTL4_DFE_CNTL_AMP_SHIFT);
+	padctl_writel(padctl, value, XUSB_PADCTL_IOPHY_USB3_PADX_CTL4(port));
+
+	value = padctl_readl(padctl, offset);
+	value &= ~(XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_SEL_MASK <<
+		   XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_SEL_SHIFT);
+	value |= XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_SEL_LATCH_G_Z <<
+		XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_SEL_SHIFT;
+	padctl_writel(padctl, value, offset);
+
+	value = padctl_readl(padctl, offset);
+	value &= ~(XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_SEL_MASK <<
+		   XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_SEL_SHIFT);
+	value |= XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_SEL_G_Z <<
+		XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_SEL_SHIFT;
+	padctl_writel(padctl, value, offset);
+
+	value = padctl_readl(padctl, offset) >>
+		XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_SHIFT;
+	padctl->usb3_ports[port].ctle_g_val = value &
+		XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_G_Z_MASK;
+
+	value = padctl_readl(padctl, offset);
+	value &= ~(XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_SEL_MASK <<
+		   XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_SEL_SHIFT);
+	value |= XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_SEL_CTLE_Z <<
+		XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_SEL_SHIFT;
+	padctl_writel(padctl, value, offset);
+
+	value = padctl_readl(padctl, offset) >>
+		XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_SHIFT;
+	padctl->usb3_ports[port].ctle_z_val = value &
+		XUSB_PADCTL_IOPHY_MISC_PAD_CTL6_MISC_OUT_G_Z_MASK;
+
+	value = padctl_readl(padctl, XUSB_PADCTL_IOPHY_USB3_PADX_CTL2(port));
+	value &= ~((XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_RX_EQ_G_MASK <<
+		    XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_RX_EQ_G_SHIFT) |
+		   (XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_RX_EQ_Z_MASK <<
+		    XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_RX_EQ_Z_SHIFT));
+	value |= (padctl->usb3_ports[port].ctle_g_val <<
+		  XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_RX_EQ_G_SHIFT) |
+		 (padctl->usb3_ports[port].ctle_z_val <<
+		  XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_RX_EQ_Z_SHIFT);
+	padctl_writel(padctl, value, XUSB_PADCTL_IOPHY_USB3_PADX_CTL2(port));
+}
+
+static const struct phy_ops usb3_phy_ops = {
+	.init = tegra_xusb_phy_init,
+	.exit = tegra_xusb_phy_exit,
+	.power_on = usb3_phy_power_on,
+	.power_off = usb3_phy_power_off,
+	.owner = THIS_MODULE,
+};
+
+static int utmi_phy_to_port(struct phy *phy)
+{
+	struct tegra_xusb_padctl *padctl = phy_get_drvdata(phy);
+	int i;
+
+	for (i = 0; i < TEGRA_XUSB_UTMI_PHYS; i++) {
+		if (phy == padctl->phys[TEGRA_XUSB_PADCTL_UTMI_P0 + i])
+			break;
+	}
+	BUG_ON(i == TEGRA_XUSB_UTMI_PHYS);
+
+	return i;
+}
+
+static int utmi_phy_power_on(struct phy *phy)
+{
+	struct tegra_xusb_padctl *padctl = phy_get_drvdata(phy);
+	int port = utmi_phy_to_port(phy);
+	int ret;
+	u32 value;
+
+	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) |
+		   (XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_DISCON_LEVEL_MASK <<
+		    XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_DISCON_LEVEL_SHIFT));
+	value |= (padctl->calib.hs_squelch_level <<
+		  XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_SQUELCH_LEVEL_SHIFT) |
+		 (padctl->soc->hs_discon_level <<
+		  XUSB_PADCTL_USB2_BIAS_PAD_CTL0_HS_DISCON_LEVEL_SHIFT);
+	padctl_writel(padctl, value, XUSB_PADCTL_USB2_BIAS_PAD_CTL0);
+
+	value = padctl_readl(padctl, XUSB_PADCTL_USB2_PORT_CAP);
+	value &= ~(XUSB_PADCTL_USB2_PORT_CAP_PORT_CAP_MASK <<
+		   XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_SHIFT(port));
+	value |= XUSB_PADCTL_USB2_PORT_CAP_HOST <<
+		XUSB_PADCTL_USB2_PORT_CAP_PORTX_CAP_SHIFT(port);
+	padctl_writel(padctl, value, XUSB_PADCTL_USB2_PORT_CAP);
+
+	value = padctl_readl(padctl, XUSB_PADCTL_USB2_OTG_PADX_CTL0(port));
+	value &= ~((XUSB_PADCTL_USB2_OTG_PAD_CTL0_HS_CURR_LEVEL_MASK <<
+		    XUSB_PADCTL_USB2_OTG_PAD_CTL0_HS_CURR_LEVEL_SHIFT) |
+		   (XUSB_PADCTL_USB2_OTG_PAD_CTL0_HS_SLEW_MASK <<
+		    XUSB_PADCTL_USB2_OTG_PAD_CTL0_HS_SLEW_SHIFT) |
+		   (XUSB_PADCTL_USB2_OTG_PAD_CTL0_LS_RSLEW_MASK <<
+		    XUSB_PADCTL_USB2_OTG_PAD_CTL0_LS_RSLEW_SHIFT) |
+		   XUSB_PADCTL_USB2_OTG_PAD_CTL0_PD |
+		   XUSB_PADCTL_USB2_OTG_PAD_CTL0_PD2 |
+		   XUSB_PADCTL_USB2_OTG_PAD_CTL0_PD_ZI);
+	value |= (padctl->calib.hs_curr_level[port] +
+		  padctl->hs_curr_level_offset[port]) <<
+		XUSB_PADCTL_USB2_OTG_PAD_CTL0_HS_CURR_LEVEL_SHIFT;
+	value |= padctl->soc->hs_slew <<
+		XUSB_PADCTL_USB2_OTG_PAD_CTL0_HS_SLEW_SHIFT;
+	value |= padctl->soc->ls_rslew[port] <<
+		XUSB_PADCTL_USB2_OTG_PAD_CTL0_LS_RSLEW_SHIFT;
+	padctl_writel(padctl, value, XUSB_PADCTL_USB2_OTG_PADX_CTL0(port));
+
+	value = padctl_readl(padctl, XUSB_PADCTL_USB2_OTG_PADX_CTL1(port));
+	value &= ~((XUSB_PADCTL_USB2_OTG_PAD_CTL1_TERM_RANGE_ADJ_MASK <<
+		    XUSB_PADCTL_USB2_OTG_PAD_CTL1_TERM_RANGE_ADJ_SHIFT) |
+		   (XUSB_PADCTL_USB2_OTG_PAD_CTL1_HS_IREF_CAP_MASK <<
+		    XUSB_PADCTL_USB2_OTG_PAD_CTL1_HS_IREF_CAP_SHIFT) |
+		   XUSB_PADCTL_USB2_OTG_PAD_CTL1_PD_DR |
+		   XUSB_PADCTL_USB2_OTG_PAD_CTL1_PD_CHRP_FORCE_POWERUP |
+		   XUSB_PADCTL_USB2_OTG_PAD_CTL1_PD_DISC_FORCE_POWERUP);
+	value |= (padctl->calib.hs_term_range_adj <<
+		  XUSB_PADCTL_USB2_OTG_PAD_CTL1_TERM_RANGE_ADJ_SHIFT) |
+		 (padctl->calib.hs_iref_cap <<
+		  XUSB_PADCTL_USB2_OTG_PAD_CTL1_HS_IREF_CAP_SHIFT);
+	padctl_writel(padctl, value, XUSB_PADCTL_USB2_OTG_PADX_CTL1(port));
+
+	ret = regulator_enable(padctl->vbus[port]);
+	if (ret)
+		return ret;
+
+	mutex_lock(&padctl->lock);
+
+	if (padctl->utmi_enable++ > 0)
+		goto out;
+
+	value = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL0);
+	value &= ~XUSB_PADCTL_USB2_BIAS_PAD_CTL0_PD;
+	padctl_writel(padctl, value, XUSB_PADCTL_USB2_BIAS_PAD_CTL0);
+
+out:
+	mutex_unlock(&padctl->lock);
+	return 0;
+}
+
+static int utmi_phy_power_off(struct phy *phy)
+{
+	struct tegra_xusb_padctl *padctl = phy_get_drvdata(phy);
+	int port = utmi_phy_to_port(phy);
+	u32 value;
+
+	regulator_disable(padctl->vbus[port]);
+
+	mutex_lock(&padctl->lock);
+
+	if (WARN_ON(padctl->utmi_enable == 0))
+		goto out;
+
+	if (--padctl->utmi_enable > 0)
+		goto out;
+
+	value = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL0);
+	value |= XUSB_PADCTL_USB2_BIAS_PAD_CTL0_PD;
+	padctl_writel(padctl, value, XUSB_PADCTL_USB2_BIAS_PAD_CTL0);
+
+out:
+	mutex_unlock(&padctl->lock);
+	return 0;
+}
+
+static const struct phy_ops utmi_phy_ops = {
+	.init = tegra_xusb_phy_init,
+	.exit = tegra_xusb_phy_exit,
+	.power_on = utmi_phy_power_on,
+	.power_off = utmi_phy_power_off,
+	.owner = THIS_MODULE,
+};
+
+static int hsic_phy_to_port(struct phy *phy)
+{
+	struct tegra_xusb_padctl *padctl = phy_get_drvdata(phy);
+	int i;
+
+	for (i = 0; i < TEGRA_XUSB_HSIC_PHYS; i++) {
+		if (phy == padctl->phys[TEGRA_XUSB_PADCTL_HSIC_P0 + i])
+			break;
+	}
+	BUG_ON(i == TEGRA_XUSB_HSIC_PHYS);
+
+	return i;
+}
+
+static int hsic_phy_power_on(struct phy *phy)
+{
+	struct tegra_xusb_padctl *padctl = phy_get_drvdata(phy);
+	int port = hsic_phy_to_port(phy);
+	int ret;
+	u32 value;
+
+	ret = regulator_enable(padctl->vddio_hsic);
+	if (ret)
+		return ret;
+
+	value = padctl_readl(padctl, XUSB_PADCTL_HSIC_PADX_CTL1(port));
+	value &= ~(XUSB_PADCTL_HSIC_PAD_CTL1_RPD_STROBE |
+		   XUSB_PADCTL_HSIC_PAD_CTL1_RPU_DATA |
+		   XUSB_PADCTL_HSIC_PAD_CTL1_PD_RX |
+		   XUSB_PADCTL_HSIC_PAD_CTL1_PD_ZI |
+		   XUSB_PADCTL_HSIC_PAD_CTL1_PD_TRX |
+		   XUSB_PADCTL_HSIC_PAD_CTL1_PD_TX);
+	value |= XUSB_PADCTL_HSIC_PAD_CTL1_RPD_DATA |
+		 XUSB_PADCTL_HSIC_PAD_CTL1_RPU_STROBE;
+	padctl_writel(padctl, value, XUSB_PADCTL_HSIC_PADX_CTL1(port));
+
+	return 0;
+}
+
+static int hsic_phy_power_off(struct phy *phy)
+{
+	struct tegra_xusb_padctl *padctl = phy_get_drvdata(phy);
+	int port = hsic_phy_to_port(phy);
+	u32 value;
+
+	regulator_disable(padctl->vddio_hsic);
+
+	value = padctl_readl(padctl, XUSB_PADCTL_HSIC_PADX_CTL1(port));
+	value |= XUSB_PADCTL_HSIC_PAD_CTL1_PD_RX |
+		 XUSB_PADCTL_HSIC_PAD_CTL1_PD_ZI |
+		 XUSB_PADCTL_HSIC_PAD_CTL1_PD_TRX |
+		 XUSB_PADCTL_HSIC_PAD_CTL1_PD_TX;
+	padctl_writel(padctl, value, XUSB_PADCTL_HSIC_PADX_CTL1(port));
+
+	return 0;
+}
+
+static void hsic_phy_set_idle(struct tegra_xusb_padctl *padctl, int port,
+			      bool set)
+{
+	u32 value;
+
+	value = padctl_readl(padctl, XUSB_PADCTL_HSIC_PADX_CTL1(port));
+	if (set)
+		value |= XUSB_PADCTL_HSIC_PAD_CTL1_RPD_DATA |
+			 XUSB_PADCTL_HSIC_PAD_CTL1_RPU_STROBE;
+	else
+		value &= ~(XUSB_PADCTL_HSIC_PAD_CTL1_RPD_DATA |
+			   XUSB_PADCTL_HSIC_PAD_CTL1_RPU_STROBE);
+	padctl_writel(padctl, value, XUSB_PADCTL_HSIC_PADX_CTL1(port));
+}
+
+static const struct phy_ops hsic_phy_ops = {
+	.init = tegra_xusb_phy_init,
+	.exit = tegra_xusb_phy_exit,
+	.power_on = hsic_phy_power_on,
+	.power_off = hsic_phy_power_off,
+	.owner = THIS_MODULE,
+};
+
+static bool is_phy_mbox_message(u32 cmd)
+{
+	switch (cmd) {
+	case MBOX_CMD_SAVE_DFE_CTLE_CTX:
+	case MBOX_CMD_START_HSIC_IDLE:
+	case MBOX_CMD_STOP_HSIC_IDLE:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static void tegra_xusb_phy_mbox_work(struct work_struct *work)
+{
+	struct tegra_xusb_padctl *padctl = container_of(work,
+				struct tegra_xusb_padctl, mbox_req_work);
+	struct tegra_xusb_mbox_msg *msg = &padctl->mbox_req;
+	struct tegra_xusb_mbox_msg resp;
+	u32 ports;
+	int i;
+
+	resp.cmd = 0;
+	switch (msg->cmd) {
+	case MBOX_CMD_SAVE_DFE_CTLE_CTX:
+		resp.data = msg->data;
+		if (msg->data > TEGRA_XUSB_USB3_PHYS) {
+			resp.cmd = MBOX_CMD_NAK;
+		} else {
+			usb3_phy_save_context(padctl, msg->data);
+			resp.cmd = MBOX_CMD_ACK;
+		}
+		break;
+	case MBOX_CMD_START_HSIC_IDLE:
+	case MBOX_CMD_STOP_HSIC_IDLE:
+		ports = msg->data >> (padctl->soc->hsic_port_offset + 1);
+		resp.data = msg->data;
+		resp.cmd = MBOX_CMD_ACK;
+		for (i = 0; i < TEGRA_XUSB_HSIC_PHYS; i++) {
+			if (!(ports & BIT(i)))
+				continue;
+			if (msg->cmd == MBOX_CMD_START_HSIC_IDLE)
+				hsic_phy_set_idle(padctl, i, true);
+			else
+				hsic_phy_set_idle(padctl, i, false);
+		}
+		break;
+	default:
+		break;
+	}
+
+	if (resp.cmd)
+		mbox_send_message(padctl->mbox_chan, &resp);
+}
+
+static void tegra_xusb_phy_mbox_rx(struct mbox_client *cl, void *data)
+{
+	struct tegra_xusb_padctl *padctl = dev_get_drvdata(cl->dev);
+	struct tegra_xusb_mbox_msg *msg = data;
+
+	if (is_phy_mbox_message(msg->cmd)) {
+		padctl->mbox_req = *msg;
+		schedule_work(&padctl->mbox_req_work);
+	}
+}
+
 static struct phy *tegra_xusb_padctl_xlate(struct device *dev,
 					   struct of_phandle_args *args)
 {
@@ -686,32 +1755,19 @@ static struct phy *tegra_xusb_padctl_xlate(struct device *dev,
 	return padctl->phys[index];
 }
 
-#define PIN_OTG_0   0
-#define PIN_OTG_1   1
-#define PIN_OTG_2   2
-#define PIN_ULPI_0  3
-#define PIN_HSIC_0  4
-#define PIN_HSIC_1  5
-#define PIN_PCIE_0  6
-#define PIN_PCIE_1  7
-#define PIN_PCIE_2  8
-#define PIN_PCIE_3  9
-#define PIN_PCIE_4 10
-#define PIN_SATA_0 11
-
 static const struct pinctrl_pin_desc tegra124_pins[] = {
-	PINCTRL_PIN(PIN_OTG_0,  "otg-0"),
-	PINCTRL_PIN(PIN_OTG_1,  "otg-1"),
-	PINCTRL_PIN(PIN_OTG_2,  "otg-2"),
-	PINCTRL_PIN(PIN_ULPI_0, "ulpi-0"),
-	PINCTRL_PIN(PIN_HSIC_0, "hsic-0"),
-	PINCTRL_PIN(PIN_HSIC_1, "hsic-1"),
-	PINCTRL_PIN(PIN_PCIE_0, "pcie-0"),
-	PINCTRL_PIN(PIN_PCIE_1, "pcie-1"),
-	PINCTRL_PIN(PIN_PCIE_2, "pcie-2"),
-	PINCTRL_PIN(PIN_PCIE_3, "pcie-3"),
-	PINCTRL_PIN(PIN_PCIE_4, "pcie-4"),
-	PINCTRL_PIN(PIN_SATA_0, "sata-0"),
+	PINCTRL_PIN(TEGRA_XUSB_PADCTL_PIN_OTG_0,  "otg-0"),
+	PINCTRL_PIN(TEGRA_XUSB_PADCTL_PIN_OTG_1,  "otg-1"),
+	PINCTRL_PIN(TEGRA_XUSB_PADCTL_PIN_OTG_2,  "otg-2"),
+	PINCTRL_PIN(TEGRA_XUSB_PADCTL_PIN_ULPI_0, "ulpi-0"),
+	PINCTRL_PIN(TEGRA_XUSB_PADCTL_PIN_HSIC_0, "hsic-0"),
+	PINCTRL_PIN(TEGRA_XUSB_PADCTL_PIN_HSIC_1, "hsic-1"),
+	PINCTRL_PIN(TEGRA_XUSB_PADCTL_PIN_PCIE_0, "pcie-0"),
+	PINCTRL_PIN(TEGRA_XUSB_PADCTL_PIN_PCIE_1, "pcie-1"),
+	PINCTRL_PIN(TEGRA_XUSB_PADCTL_PIN_PCIE_2, "pcie-2"),
+	PINCTRL_PIN(TEGRA_XUSB_PADCTL_PIN_PCIE_3, "pcie-3"),
+	PINCTRL_PIN(TEGRA_XUSB_PADCTL_PIN_PCIE_4, "pcie-4"),
+	PINCTRL_PIN(TEGRA_XUSB_PADCTL_PIN_SATA_0, "sata-0"),
 };
 
 static const char * const tegra124_snps_groups[] = {
@@ -856,6 +1912,15 @@ static const struct tegra_xusb_padctl_soc tegra124_soc = {
 	.functions = tegra124_functions,
 	.num_lanes = ARRAY_SIZE(tegra124_lanes),
 	.lanes = tegra124_lanes,
+	.rx_wander = 0xf,
+	.rx_eq = 0xf070,
+	.cdr_cntl = 0x24,
+	.dfe_cntl = 0x002008ee,
+	.hs_slew = 0xe,
+	.ls_rslew = {0x3, 0x0, 0x0},
+	.hs_discon_level = 0x5,
+	.spare_in = 0x1,
+	.hsic_port_offset = 6,
 };
 
 static const struct of_device_id tegra_xusb_padctl_of_match[] = {
@@ -864,13 +1929,40 @@ static const struct of_device_id tegra_xusb_padctl_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, tegra_xusb_padctl_of_match);
 
+static int tegra_xusb_read_fuse_calibration(struct tegra_xusb_padctl *padctl)
+{
+	int i, ret;
+	u32 value;
+
+	ret = tegra_fuse_readl(TEGRA_FUSE_SKU_CALIB_0, &value);
+	if (ret < 0)
+		return ret;
+
+	for (i = 0; i < TEGRA_XUSB_UTMI_PHYS; i++) {
+		padctl->calib.hs_curr_level[i] =
+			(value >> FUSE_SKU_CALIB_HS_CURR_LEVEL_PADX_SHIFT(i)) &
+			FUSE_SKU_CALIB_HS_CURR_LEVEL_PAD_MASK;
+	}
+	padctl->calib.hs_iref_cap =
+		(value >> FUSE_SKU_CALIB_HS_IREF_CAP_SHIFT) &
+		FUSE_SKU_CALIB_HS_IREF_CAP_MASK;
+	padctl->calib.hs_term_range_adj =
+		(value >> FUSE_SKU_CALIB_HS_TERM_RANGE_ADJ_SHIFT) &
+		FUSE_SKU_CALIB_HS_TERM_RANGE_ADJ_MASK;
+	padctl->calib.hs_squelch_level =
+		(value >> FUSE_SKU_CALIB_HS_SQUELCH_LEVEL_SHIFT) &
+		FUSE_SKU_CALIB_HS_SQUELCH_LEVEL_MASK;
+
+	return 0;
+}
+
 static int tegra_xusb_padctl_probe(struct platform_device *pdev)
 {
 	struct tegra_xusb_padctl *padctl;
 	const struct of_device_id *match;
 	struct resource *res;
 	struct phy *phy;
-	int err;
+	int err, i;
 
 	padctl = devm_kzalloc(&pdev->dev, sizeof(*padctl), GFP_KERNEL);
 	if (!padctl)
@@ -888,6 +1980,10 @@ static int tegra_xusb_padctl_probe(struct platform_device *pdev)
 	if (IS_ERR(padctl->regs))
 		return PTR_ERR(padctl->regs);
 
+	err = tegra_xusb_read_fuse_calibration(padctl);
+	if (err < 0)
+		return err;
+
 	padctl->rst = devm_reset_control_get(&pdev->dev, NULL);
 	if (IS_ERR(padctl->rst))
 		return PTR_ERR(padctl->rst);
@@ -896,6 +1992,24 @@ static int tegra_xusb_padctl_probe(struct platform_device *pdev)
 	if (err < 0)
 		return err;
 
+	for (i = 0; i < TEGRA_XUSB_USB3_PHYS; i++) {
+		char prop[sizeof("nvidia,usb3-port-N-lane")];
+		u32 lane;
+
+		sprintf(prop, "nvidia,usb3-port-%d-lane", i);
+		if (!of_property_read_u32(pdev->dev.of_node, prop, &lane)) {
+			if (!is_pcie_or_sata_lane(lane)) {
+				dev_err(&pdev->dev,
+					"USB3 port mapped to invalid lane\n");
+				err = -EINVAL;
+				goto unregister;
+			}
+			padctl->usb3_ports[i].lane = lane;
+		} else {
+			padctl->usb3_ports[i].lane = -EINVAL;
+		}
+	}
+
 	memset(&padctl->desc, 0, sizeof(padctl->desc));
 	padctl->desc.name = dev_name(padctl->dev);
 	padctl->desc.pctlops = &tegra_xusb_padctl_pinctrl_ops;
@@ -928,6 +2042,54 @@ static int tegra_xusb_padctl_probe(struct platform_device *pdev)
 	padctl->phys[TEGRA_XUSB_PADCTL_SATA] = phy;
 	phy_set_drvdata(phy, padctl);
 
+	for (i = 0; i < TEGRA_XUSB_USB3_PHYS; i++) {
+		phy = devm_phy_create(&pdev->dev, NULL, &usb3_phy_ops, NULL);
+		if (IS_ERR(phy)) {
+			err = PTR_ERR(phy);
+			goto unregister;
+		}
+
+		padctl->phys[TEGRA_XUSB_PADCTL_USB3_P0 + i] = phy;
+		phy_set_drvdata(phy, padctl);
+	}
+
+	for (i = 0; i < TEGRA_XUSB_UTMI_PHYS; i++) {
+		char reg_name[sizeof("vbus-N")];
+
+		sprintf(reg_name, "vbus-%d", i);
+		padctl->vbus[i] = devm_regulator_get(&pdev->dev, reg_name);
+		if (IS_ERR(padctl->vbus[i])) {
+			err = PTR_ERR(padctl->vbus[i]);
+			goto unregister;
+		}
+
+		phy = devm_phy_create(&pdev->dev, NULL, &utmi_phy_ops, NULL);
+		if (IS_ERR(phy)) {
+			err = PTR_ERR(phy);
+			goto unregister;
+		}
+
+		padctl->phys[TEGRA_XUSB_PADCTL_UTMI_P0 + i] = phy;
+		phy_set_drvdata(phy, padctl);
+	}
+
+	padctl->vddio_hsic = devm_regulator_get(&pdev->dev, "vddio-hsic");
+	if (IS_ERR(padctl->vddio_hsic)) {
+		err = PTR_ERR(padctl->vddio_hsic);
+		goto unregister;
+	}
+
+	for (i = 0; i < TEGRA_XUSB_HSIC_PHYS; i++) {
+		phy = devm_phy_create(&pdev->dev, NULL, &hsic_phy_ops, NULL);
+		if (IS_ERR(phy)) {
+			err = PTR_ERR(phy);
+			goto unregister;
+		}
+
+		padctl->phys[TEGRA_XUSB_PADCTL_HSIC_P0 + i] = phy;
+		phy_set_drvdata(phy, padctl);
+	}
+
 	padctl->provider = devm_of_phy_provider_register(&pdev->dev,
 							 tegra_xusb_padctl_xlate);
 	if (IS_ERR(padctl->provider)) {
@@ -936,6 +2098,18 @@ static int tegra_xusb_padctl_probe(struct platform_device *pdev)
 		goto unregister;
 	}
 
+	INIT_WORK(&padctl->mbox_req_work, tegra_xusb_phy_mbox_work);
+	padctl->mbox_client.dev = &pdev->dev;
+	padctl->mbox_client.tx_block = true;
+	padctl->mbox_client.tx_tout = 0;
+	padctl->mbox_client.rx_callback = tegra_xusb_phy_mbox_rx;
+	padctl->mbox_chan = mbox_request_channel(&padctl->mbox_client, 0);
+	if (IS_ERR(padctl->mbox_chan)) {
+		err = PTR_ERR(padctl->mbox_chan);
+		dev_err(&pdev->dev, "failed to request mailbox: %d\n", err);
+		goto unregister;
+	}
+
 	return 0;
 
 unregister:
@@ -950,6 +2124,9 @@ static int tegra_xusb_padctl_remove(struct platform_device *pdev)
 	struct tegra_xusb_padctl *padctl = platform_get_drvdata(pdev);
 	int err;
 
+	cancel_work_sync(&padctl->mbox_req_work);
+	mbox_free_channel(padctl->mbox_chan);
+
 	pinctrl_unregister(padctl->pinctrl);
 
 	err = reset_control_assert(padctl->rst);
diff --git a/include/soc/tegra/xusb.h b/include/soc/tegra/xusb.h
index cfe211d..149434f 100644
--- a/include/soc/tegra/xusb.h
+++ b/include/soc/tegra/xusb.h
@@ -10,6 +10,13 @@
 #ifndef __SOC_TEGRA_XUSB_H__
 #define __SOC_TEGRA_XUSB_H__
 
+#define TEGRA_XUSB_USB3_PHYS 2
+#define TEGRA_XUSB_UTMI_PHYS 3
+#define TEGRA_XUSB_HSIC_PHYS 2
+#define TEGRA_XUSB_NUM_USB_PHYS (TEGRA_XUSB_USB3_PHYS + TEGRA_XUSB_UTMI_PHYS + \
+				 TEGRA_XUSB_HSIC_PHYS)
+#define TEGRA_XUSB_NUM_PHYS (TEGRA_XUSB_NUM_USB_PHYS + 2) /* + SATA & PCIe */
+
 /* Two virtual channels: host + phy */
 #define TEGRA_XUSB_MBOX_NUM_CHANS 2
 
-- 
2.1.0.rc2.206.gedb03e5

[PATCH RESEND V4 5/9] of: Add NVIDIA Tegra xHCI controller binding

[Andrew Bresticker]

Add device-tree binding documentation for the xHCI controller present
on Tegra124 and later SoCs.

Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Reviewed-by: Stephen Warren <swarren@nvidia.com>
---
No changes from v3.
Changes from v2:
 - Added mbox-names property.
Changes from v1:
 - Updated to use common mailbox bindings.
 - Added remaining XUSB-related clocks and resets.
 - Updated list of power supplies to be more accurate wrt to the hardware.
---
 .../bindings/usb/nvidia,tegra124-xhci.txt          | 104 +++++++++++++++++++++
 1 file changed, 104 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/nvidia,tegra124-xhci.txt

diff --git a/Documentation/devicetree/bindings/usb/nvidia,tegra124-xhci.txt b/Documentation/devicetree/bindings/usb/nvidia,tegra124-xhci.txt
new file mode 100644
index 0000000..51a7751
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/nvidia,tegra124-xhci.txt
@@ -0,0 +1,104 @@
+NVIDIA Tegra xHCI controller
+============================
+
+The Tegra xHCI controller supports both USB2 and USB3 interfaces exposed
+by the Tegra XUSB pad controller.
+
+Required properties:
+--------------------
+ - compatible: Should be "nvidia,tegra124-xhci".
+ - reg: Address and length of the register sets.  There should be three
+   entries in the following order: xHCI host registers, FPCI registers, and
+   IPFS registers.
+ - interrupts: xHCI host interrupt.
+ - clocks: Must contain an entry for each entry in clock-names.
+   See ../clock/clock-bindings.txt for details.
+ - clock-names: Must include the following entries:
+    - xusb_host
+    - xusb_host_src
+    - xusb_dev
+    - xusb_dev_src
+    - xusb_falcon_src
+    - xusb_ss
+    - xusb_ss_src
+    - xusb_ss_div2
+    - xusb_hs_src
+    - xusb_fs_src
+    - pll_u_480m
+    - clk_m
+    - pll_e
+ - resets: Must contain an entry for each entry in reset-names.
+   See ../reset/reset.txt for details.
+ - reset-names: Must include the following entries:
+   - xusb_host
+   - xusb_dev
+   - xusb_ss
+   - xusb
+   Note that xusb_dev is the shared reset for xusb_dev and xusb_dev_src and
+   that xusb is the shared reset for xusb_{ss,hs,fs,falcon,host}_src.
+ - mboxes: Must contain an entry for the XUSB mailbox channel.
+   See ../mailbox/mailbox.txt for details.
+ - mbox-names: Must include the following entries:
+   - xusb
+
+Optional properties:
+--------------------
+ - phys: Must contain an entry for each entry in phy-names.
+   See ../phy/phy-bindings.txt for details.
+ - phy-names: Should include an entry for each PHY used by the controller.
+   May be a subset of the following:
+    - utmi-{0,1,2}
+    - hsic-{0,1}
+    - usb3-{0,1}
+ - avddio-pex-supply: PCIe/USB3 analog logic power supply.  Must supply 1.05V.
+ - dvddio-pex-supply: PCIe/USB3 digital logic power supply.  Must supply 1.05V.
+ - avdd-usb-supply: USB controller power supply.  Must supply 3.3V.
+ - avdd-pll-utmip-supply: UTMI PLL power supply.  Must supply 1.8V.
+ - avdd-pll-erefe-supply: PLLE reference PLL power supply.  Must supply 1.05V.
+ - avdd-pex-pll-supply: PCIe/USB3 PLL power supply.  Must supply 1.05V.
+ - hvdd-pex-supply: High-voltage PCIe/USB3 power supply.  Must supply 3.3V.
+ - hvdd-pex-plle-supply: High-voltage PLLE power supply.  Must supply 3.3V.
+
+Example:
+--------
+	usb@0,70090000 {
+		compatible = "nvidia,tegra124-xhci";
+		reg = <0x0 0x70090000 0x0 0x8000>,
+		      <0x0 0x70098000 0x0 0x1000>,
+		      <0x0 0x70099000 0x0 0x1000>;
+		interrupts = <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&tegra_car TEGRA124_CLK_XUSB_HOST>,
+			 <&tegra_car TEGRA124_CLK_XUSB_HOST_SRC>,
+			 <&tegra_car TEGRA124_CLK_XUSB_DEV>,
+			 <&tegra_car TEGRA124_CLK_XUSB_DEV_SRC>,
+			 <&tegra_car TEGRA124_CLK_XUSB_FALCON_SRC>,
+			 <&tegra_car TEGRA124_CLK_XUSB_SS>,
+			 <&tegra_car TEGRA124_CLK_XUSB_SS_DIV2>,
+			 <&tegra_car TEGRA124_CLK_XUSB_SS_SRC>,
+			 <&tegra_car TEGRA124_CLK_XUSB_HS_SRC>,
+			 <&tegra_car TEGRA124_CLK_XUSB_FS_SRC>,
+			 <&tegra_car TEGRA124_CLK_PLL_U_480M>,
+			 <&tegra_car TEGRA124_CLK_CLK_M>,
+			 <&tegra_car TEGRA124_CLK_PLL_E>;
+		clock-names = "xusb_host", "xusb_host_src", "xusb_dev",
+			      "xusb_dev_src", "xusb_falcon_src", "xusb_ss",
+			      "xusb_ss_div2", "xusb_ss_src", "xusb_hs_src",
+			      "xusb_fs_src", "pll_u_480m", "clk_m", "pll_e";
+		resets = <&tegra_car 89>, <&tegra_car 95>, <&tegra_car 156>,
+			 <&tegra_car 143>;
+		reset-names = "xusb_host", "xusb_dev", "xusb_ss", "xusb";
+		mboxes = <&xusb_mbox>;
+		mbox-names = "xusb";
+		phys = <&padctl TEGRA_XUSB_PADCTL_UTMI_P1>, /* mini-PCIe USB */
+		       <&padctl TEGRA_XUSB_PADCTL_UTMI_P2>, /* USB A */
+		       <&padctl TEGRA_XUSB_PADCTL_USB3_P0>; /* USB A */
+		phy-names = "utmi-1", "utmi-2", "usb3-0";
+		avddio-pex-supply = <&vdd_1v05_run>;
+		dvddio-pex-supply = <&vdd_1v05_run>;
+		avdd-usb-supply = <&vdd_3v3_lp0>;
+		avdd-pll-utmip-supply = <&vddio_1v8>;
+		avdd-pll-erefe-supply = <&avdd_1v05_run>;
+		avdd-pex-pll-supply = <&vdd_1v05_run>;
+		hvdd-pex-supply = <&vdd_3v3_lp0>;
+		hvdd-pex-plle-supply = <&vdd_3v3_lp0>;
+	};
-- 
2.1.0.rc2.206.gedb03e5

[PATCH RESEND V4 6/9] usb: xhci: Add NVIDIA Tegra xHCI host-controller driver

[Andrew Bresticker]

Add support for the on-chip xHCI host controller present on Tegra SoCs.

The driver is currently very basic: it loads the controller with its
firmware, starts the controller, and is able to service messages sent
by the controller's firmware.  The hardware also supports device mode
as well as powergating of the SuperSpeed and host-controller logic
when not in use, but support for these is not yet implemented.

Based on work by:
  Ajay Gupta <ajayg@nvidia.com>
  Bharath Yadav <byadav@nvidia.com>

Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Reviewed-by: Stephen Warren <swarren@nvidia.com>
---
Changes from v3:
 - Used 32-bit DMA mask (platforms may have > 32-bit physical address space
   and < 64-bit dma_addr_t).
 - Moved comment about SET_BW command.
Changes from v2:
 - Added filtering out of non-host mailbox messages.
 - Removed MODULE_ALIAS.
Changes from v1:
 - Updated to use common mailbox API.
 - Fixed up so that the driver can be built and used as a module.
 - Incorporated review feedback from Stephen.
 - Misc. cleanups.
---
 drivers/usb/host/Kconfig      |   9 +
 drivers/usb/host/Makefile     |   1 +
 drivers/usb/host/xhci-tegra.c | 907 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 917 insertions(+)
 create mode 100644 drivers/usb/host/xhci-tegra.c

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index a8a30b1..710bb68 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -50,6 +50,15 @@ config USB_XHCI_RCAR
 	  Say 'Y' to enable the support for the xHCI host controller
 	  found in Renesas R-Car ARM SoCs.
 
+config USB_XHCI_TEGRA
+	tristate "NVIDIA Tegra XHCI support"
+	depends on ARCH_TEGRA
+	select MAILBOX
+	select FW_LOADER
+	---help---
+	  Say 'Y' to enable the support for the xHCI host controller
+	  found in NVIDIA Tegra124 and later SoCs.
+
 endif # USB_XHCI_HCD
 
 config USB_EHCI_HCD
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index 348c243..af51487 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -30,6 +30,7 @@ obj-$(CONFIG_PCI)		+= pci-quirks.o
 
 obj-$(CONFIG_USB_XHCI_PCI)	+= xhci-pci.o
 obj-$(CONFIG_USB_XHCI_PLATFORM) += xhci-plat-hcd.o
+obj-$(CONFIG_USB_XHCI_TEGRA)	+= xhci-tegra.o
 
 obj-$(CONFIG_USB_EHCI_HCD)	+= ehci-hcd.o
 obj-$(CONFIG_USB_EHCI_PCI)	+= ehci-pci.o
diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
new file mode 100644
index 0000000..82d275f
--- /dev/null
+++ b/drivers/usb/host/xhci-tegra.c
@@ -0,0 +1,907 @@
+/*
+ * NVIDIA Tegra xHCI host controller driver
+ *
+ * Copyright (C) 2014 NVIDIA Corporation
+ * Copyright (C) 2014 Google, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/dma-mapping.h>
+#include <linux/firmware.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/mailbox_client.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/pm.h>
+#include <linux/regulator/consumer.h>
+#include <linux/reset.h>
+#include <linux/slab.h>
+#include <linux/workqueue.h>
+
+#include <soc/tegra/xusb.h>
+
+#include "xhci.h"
+
+#define TEGRA_XHCI_SS_CLK_HIGH_SPEED 120000000
+#define TEGRA_XHCI_SS_CLK_LOW_SPEED 12000000
+
+/* FPCI CFG registers */
+#define XUSB_CFG_1				0x004
+#define  XUSB_IO_SPACE_EN			BIT(0)
+#define  XUSB_MEM_SPACE_EN			BIT(1)
+#define  XUSB_BUS_MASTER_EN			BIT(2)
+#define XUSB_CFG_4				0x010
+#define  XUSB_BASE_ADDR_SHIFT			15
+#define  XUSB_BASE_ADDR_MASK			0x1ffff
+#define XUSB_CFG_ARU_C11_CSBRANGE		0x41c
+#define XUSB_CFG_CSB_BASE_ADDR			0x800
+
+/* IPFS registers */
+#define IPFS_XUSB_HOST_CONFIGURATION_0		0x180
+#define  IPFS_EN_FPCI				BIT(0)
+#define IPFS_XUSB_HOST_INTR_MASK_0		0x188
+#define  IPFS_IP_INT_MASK			BIT(16)
+#define IPFS_XUSB_HOST_CLKGATE_HYSTERESIS_0	0x1bc
+
+#define CSB_PAGE_SELECT_MASK			0x7fffff
+#define CSB_PAGE_SELECT_SHIFT			9
+#define CSB_PAGE_OFFSET_MASK			0x1ff
+#define CSB_PAGE_SELECT(addr)	((addr) >> (CSB_PAGE_SELECT_SHIFT) &	\
+				 CSB_PAGE_SELECT_MASK)
+#define CSB_PAGE_OFFSET(addr)	((addr) & CSB_PAGE_OFFSET_MASK)
+
+/* Falcon CSB registers */
+#define XUSB_FALC_CPUCTL			0x100
+#define  CPUCTL_STARTCPU			BIT(1)
+#define  CPUCTL_STATE_HALTED			BIT(4)
+#define XUSB_FALC_BOOTVEC			0x104
+#define XUSB_FALC_DMACTL			0x10c
+#define XUSB_FALC_IMFILLRNG1			0x154
+#define  IMFILLRNG1_TAG_MASK			0xffff
+#define  IMFILLRNG1_TAG_LO_SHIFT		0
+#define  IMFILLRNG1_TAG_HI_SHIFT		16
+#define XUSB_FALC_IMFILLCTL			0x158
+
+/* MP CSB registers */
+#define XUSB_CSB_MP_ILOAD_ATTR			0x101a00
+#define XUSB_CSB_MP_ILOAD_BASE_LO		0x101a04
+#define XUSB_CSB_MP_ILOAD_BASE_HI		0x101a08
+#define XUSB_CSB_MP_L2IMEMOP_SIZE		0x101a10
+#define  L2IMEMOP_SIZE_SRC_OFFSET_SHIFT		8
+#define  L2IMEMOP_SIZE_SRC_OFFSET_MASK		0x3ff
+#define  L2IMEMOP_SIZE_SRC_COUNT_SHIFT		24
+#define  L2IMEMOP_SIZE_SRC_COUNT_MASK		0xff
+#define XUSB_CSB_MP_L2IMEMOP_TRIG		0x101a14
+#define  L2IMEMOP_ACTION_SHIFT			24
+#define  L2IMEMOP_INVALIDATE_ALL		(0x40 << L2IMEMOP_ACTION_SHIFT)
+#define  L2IMEMOP_LOAD_LOCKED_RESULT		(0x11 << L2IMEMOP_ACTION_SHIFT)
+#define XUSB_CSB_MP_APMAP			0x10181c
+#define  APMAP_BOOTPATH				BIT(31)
+
+#define IMEM_BLOCK_SIZE				256
+
+struct tegra_xhci_fw_cfgtbl {
+	u32 boot_loadaddr_in_imem;
+	u32 boot_codedfi_offset;
+	u32 boot_codetag;
+	u32 boot_codesize;
+	u32 phys_memaddr;
+	u16 reqphys_memsize;
+	u16 alloc_phys_memsize;
+	u32 rodata_img_offset;
+	u32 rodata_section_start;
+	u32 rodata_section_end;
+	u32 main_fnaddr;
+	u32 fwimg_cksum;
+	u32 fwimg_created_time;
+	u32 imem_resident_start;
+	u32 imem_resident_end;
+	u32 idirect_start;
+	u32 idirect_end;
+	u32 l2_imem_start;
+	u32 l2_imem_end;
+	u32 version_id;
+	u8 init_ddirect;
+	u8 reserved[3];
+	u32 phys_addr_log_buffer;
+	u32 total_log_entries;
+	u32 dequeue_ptr;
+	u32 dummy_var[2];
+	u32 fwimg_len;
+	u8 magic[8];
+	u32 ss_low_power_entry_timeout;
+	u8 num_hsic_port;
+	u8 padding[139]; /* Padding to make 256-bytes cfgtbl */
+};
+
+struct tegra_xhci_soc_config {
+	const char *firmware_file;
+};
+
+#define TEGRA_XHCI_NUM_SUPPLIES 8
+static const char *tegra_xhci_supply_names[TEGRA_XHCI_NUM_SUPPLIES] = {
+	"avddio-pex",
+	"dvddio-pex",
+	"avdd-usb",
+	"avdd-pll-utmip",
+	"avdd-pll-erefe",
+	"avdd-pex-pll",
+	"hvdd-pex",
+	"hvdd-pex-plle",
+};
+
+static const struct {
+	const char *name;
+	int num;
+} tegra_xhci_phy_types[] = {
+	{
+		.name = "usb3",
+		.num = TEGRA_XUSB_USB3_PHYS,
+	}, {
+		.name = "utmi",
+		.num = TEGRA_XUSB_UTMI_PHYS,
+	}, {
+		.name = "hsic",
+		.num = TEGRA_XUSB_HSIC_PHYS,
+	},
+};
+
+struct tegra_xhci_hcd {
+	struct device *dev;
+	struct usb_hcd *hcd;
+
+	int irq;
+
+	void __iomem *fpci_base;
+	void __iomem *ipfs_base;
+
+	const struct tegra_xhci_soc_config *soc_config;
+
+	struct regulator_bulk_data supplies[TEGRA_XHCI_NUM_SUPPLIES];
+
+	struct clk *host_clk;
+	struct clk *falc_clk;
+	struct clk *ss_clk;
+	struct clk *ss_src_clk;
+	struct clk *hs_src_clk;
+	struct clk *fs_src_clk;
+	struct clk *pll_u_480m;
+	struct clk *clk_m;
+	struct clk *pll_e;
+
+	struct reset_control *host_rst;
+	struct reset_control *ss_rst;
+
+	struct phy *phys[TEGRA_XUSB_NUM_USB_PHYS];
+
+	struct work_struct mbox_req_work;
+	struct tegra_xusb_mbox_msg mbox_req;
+	struct mbox_client mbox_client;
+	struct mbox_chan *mbox_chan;
+
+	/* Firmware loading related */
+	void *fw_data;
+	size_t fw_size;
+	dma_addr_t fw_dma_addr;
+	bool fw_loaded;
+};
+
+static struct hc_driver __read_mostly tegra_xhci_hc_driver;
+
+static inline u32 fpci_readl(struct tegra_xhci_hcd *tegra, u32 addr)
+{
+	return readl(tegra->fpci_base + addr);
+}
+
+static inline void fpci_writel(struct tegra_xhci_hcd *tegra, u32 val, u32 addr)
+{
+	writel(val, tegra->fpci_base + addr);
+}
+
+static inline u32 ipfs_readl(struct tegra_xhci_hcd *tegra, u32 addr)
+{
+	return readl(tegra->ipfs_base + addr);
+}
+
+static inline void ipfs_writel(struct tegra_xhci_hcd *tegra, u32 val, u32 addr)
+{
+	writel(val, tegra->ipfs_base + addr);
+}
+
+static u32 csb_readl(struct tegra_xhci_hcd *tegra, u32 addr)
+{
+	u32 page, offset;
+
+	page = CSB_PAGE_SELECT(addr);
+	offset = CSB_PAGE_OFFSET(addr);
+	fpci_writel(tegra, page, XUSB_CFG_ARU_C11_CSBRANGE);
+	return fpci_readl(tegra, XUSB_CFG_CSB_BASE_ADDR + offset);
+}
+
+static void csb_writel(struct tegra_xhci_hcd *tegra, u32 val, u32 addr)
+{
+	u32 page, offset;
+
+	page = CSB_PAGE_SELECT(addr);
+	offset = CSB_PAGE_OFFSET(addr);
+	fpci_writel(tegra, page, XUSB_CFG_ARU_C11_CSBRANGE);
+	fpci_writel(tegra, val, XUSB_CFG_CSB_BASE_ADDR + offset);
+}
+
+static void tegra_xhci_cfg(struct tegra_xhci_hcd *tegra)
+{
+	u32 reg;
+
+	reg = ipfs_readl(tegra, IPFS_XUSB_HOST_CONFIGURATION_0);
+	reg |= IPFS_EN_FPCI;
+	ipfs_writel(tegra, reg, IPFS_XUSB_HOST_CONFIGURATION_0);
+	udelay(10);
+
+	/* Program Bar0 Space */
+	reg = fpci_readl(tegra, XUSB_CFG_4);
+	reg &= ~(XUSB_BASE_ADDR_MASK << XUSB_BASE_ADDR_SHIFT);
+	reg |= tegra->hcd->rsrc_start & (XUSB_BASE_ADDR_MASK <<
+					 XUSB_BASE_ADDR_SHIFT);
+	fpci_writel(tegra, reg, XUSB_CFG_4);
+	usleep_range(100, 200);
+
+	/* Enable Bus Master */
+	reg = fpci_readl(tegra, XUSB_CFG_1);
+	reg |= XUSB_IO_SPACE_EN | XUSB_MEM_SPACE_EN | XUSB_BUS_MASTER_EN;
+	fpci_writel(tegra, reg, XUSB_CFG_1);
+
+	/* Set intr mask to enable intr assertion */
+	reg = ipfs_readl(tegra, IPFS_XUSB_HOST_INTR_MASK_0);
+	reg |= IPFS_IP_INT_MASK;
+	ipfs_writel(tegra, reg, IPFS_XUSB_HOST_INTR_MASK_0);
+
+	/* Set hysteris to 0x80 */
+	ipfs_writel(tegra, 0x80, IPFS_XUSB_HOST_CLKGATE_HYSTERESIS_0);
+}
+
+static int tegra_xhci_load_firmware(struct tegra_xhci_hcd *tegra)
+{
+	struct device *dev = tegra->dev;
+	struct tegra_xhci_fw_cfgtbl *cfg_tbl;
+	u64 fw_base;
+	u32 val, code_tag_blocks, code_size_blocks;
+	time_t fw_time;
+	struct tm fw_tm;
+
+	if (csb_readl(tegra, XUSB_CSB_MP_ILOAD_BASE_LO) != 0) {
+		dev_info(dev, "Firmware already loaded, Falcon state 0x%x\n",
+			 csb_readl(tegra, XUSB_FALC_CPUCTL));
+		return 0;
+	}
+
+	cfg_tbl = (struct tegra_xhci_fw_cfgtbl *)tegra->fw_data;
+
+	/* Program the size of DFI into ILOAD_ATTR. */
+	csb_writel(tegra, tegra->fw_size, XUSB_CSB_MP_ILOAD_ATTR);
+
+	/*
+	 * Boot code of the firmware reads the ILOAD_BASE registers
+	 * to get to the start of the DFI in system memory.
+	 */
+	fw_base = tegra->fw_dma_addr + sizeof(*cfg_tbl);
+	csb_writel(tegra, fw_base, XUSB_CSB_MP_ILOAD_BASE_LO);
+	csb_writel(tegra, fw_base >> 32, XUSB_CSB_MP_ILOAD_BASE_HI);
+
+	/* Set BOOTPATH to 1 in APMAP. */
+	csb_writel(tegra, APMAP_BOOTPATH, XUSB_CSB_MP_APMAP);
+
+	/* Invalidate L2IMEM. */
+	csb_writel(tegra, L2IMEMOP_INVALIDATE_ALL, XUSB_CSB_MP_L2IMEMOP_TRIG);
+
+	/*
+	 * Initiate fetch of bootcode from system memory into L2IMEM.
+	 * Program bootcode location and size in system memory.
+	 */
+	code_tag_blocks = DIV_ROUND_UP(le32_to_cpu(cfg_tbl->boot_codetag),
+				       IMEM_BLOCK_SIZE);
+	code_size_blocks = DIV_ROUND_UP(le32_to_cpu(cfg_tbl->boot_codesize),
+					IMEM_BLOCK_SIZE);
+	val = ((code_tag_blocks & L2IMEMOP_SIZE_SRC_OFFSET_MASK) <<
+	       L2IMEMOP_SIZE_SRC_OFFSET_SHIFT) |
+	      ((code_size_blocks & L2IMEMOP_SIZE_SRC_COUNT_MASK) <<
+	       L2IMEMOP_SIZE_SRC_COUNT_SHIFT);
+	csb_writel(tegra, val, XUSB_CSB_MP_L2IMEMOP_SIZE);
+
+	/* Trigger L2IMEM Load operation. */
+	csb_writel(tegra, L2IMEMOP_LOAD_LOCKED_RESULT,
+		   XUSB_CSB_MP_L2IMEMOP_TRIG);
+
+	/* Setup Falcon Auto-fill. */
+	csb_writel(tegra, code_size_blocks, XUSB_FALC_IMFILLCTL);
+
+	val = ((code_tag_blocks & IMFILLRNG1_TAG_MASK) <<
+	       IMFILLRNG1_TAG_LO_SHIFT) |
+	      (((code_size_blocks + code_tag_blocks) & IMFILLRNG1_TAG_MASK) <<
+	       IMFILLRNG1_TAG_HI_SHIFT);
+	csb_writel(tegra, val, XUSB_FALC_IMFILLRNG1);
+
+	csb_writel(tegra, 0, XUSB_FALC_DMACTL);
+	msleep(50);
+
+	csb_writel(tegra, le32_to_cpu(cfg_tbl->boot_codetag),
+		   XUSB_FALC_BOOTVEC);
+
+	/* Start Falcon CPU. */
+	csb_writel(tegra, CPUCTL_STARTCPU, XUSB_FALC_CPUCTL);
+	usleep_range(1000, 2000);
+
+	fw_time = le32_to_cpu(cfg_tbl->fwimg_created_time);
+	time_to_tm(fw_time, 0, &fw_tm);
+	dev_info(dev,
+		 "Firmware timestamp: %ld-%02d-%02d %02d:%02d:%02d UTC, "
+		 "Falcon state 0x%x\n", fw_tm.tm_year + 1900,
+		 fw_tm.tm_mon + 1, fw_tm.tm_mday, fw_tm.tm_hour,
+		 fw_tm.tm_min, fw_tm.tm_sec,
+		 csb_readl(tegra, XUSB_FALC_CPUCTL));
+
+	/* Make sure Falcon CPU is now running. */
+	if (csb_readl(tegra, XUSB_FALC_CPUCTL) == CPUCTL_STATE_HALTED)
+		return -EIO;
+
+	return 0;
+}
+
+static int tegra_xhci_set_ss_clk(struct tegra_xhci_hcd *tegra,
+				 unsigned long rate)
+{
+	unsigned long new_parent_rate, old_parent_rate;
+	int ret, div;
+	struct clk *clk = tegra->ss_src_clk;
+
+	if (clk_get_rate(clk) == rate)
+		return 0;
+
+	switch (rate) {
+	case TEGRA_XHCI_SS_CLK_HIGH_SPEED:
+		/*
+		 * Reparent to PLLU_480M. Set divider first to avoid
+		 * overclocking.
+		 */
+		old_parent_rate = clk_get_rate(clk_get_parent(clk));
+		new_parent_rate = clk_get_rate(tegra->pll_u_480m);
+		div = new_parent_rate / rate;
+		ret = clk_set_rate(clk, old_parent_rate / div);
+		if (ret)
+			return ret;
+		ret = clk_set_parent(clk, tegra->pll_u_480m);
+		if (ret)
+			return ret;
+		/*
+		 * The rate should already be correct, but set it again just
+		 * to be sure.
+		 */
+		ret = clk_set_rate(clk, rate);
+		if (ret)
+			return ret;
+		break;
+	case TEGRA_XHCI_SS_CLK_LOW_SPEED:
+		/* Reparent to CLK_M */
+		ret = clk_set_parent(clk, tegra->clk_m);
+		if (ret)
+			return ret;
+		ret = clk_set_rate(clk, rate);
+		if (ret)
+			return ret;
+		break;
+	default:
+		dev_err(tegra->dev, "Invalid SS rate: %lu\n", rate);
+		return -EINVAL;
+	}
+
+	if (clk_get_rate(clk) != rate) {
+		dev_err(tegra->dev, "SS clock doesn't match requested rate\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int tegra_xhci_clk_enable(struct tegra_xhci_hcd *tegra)
+{
+	clk_prepare_enable(tegra->pll_e);
+	clk_prepare_enable(tegra->host_clk);
+	clk_prepare_enable(tegra->ss_clk);
+	clk_prepare_enable(tegra->falc_clk);
+	clk_prepare_enable(tegra->fs_src_clk);
+	clk_prepare_enable(tegra->hs_src_clk);
+
+	return tegra_xhci_set_ss_clk(tegra, TEGRA_XHCI_SS_CLK_HIGH_SPEED);
+}
+
+static void tegra_xhci_clk_disable(struct tegra_xhci_hcd *tegra)
+{
+	clk_disable_unprepare(tegra->pll_e);
+	clk_disable_unprepare(tegra->host_clk);
+	clk_disable_unprepare(tegra->ss_clk);
+	clk_disable_unprepare(tegra->falc_clk);
+	clk_disable_unprepare(tegra->fs_src_clk);
+	clk_disable_unprepare(tegra->hs_src_clk);
+}
+
+static int tegra_xhci_phy_enable(struct tegra_xhci_hcd *tegra)
+{
+	int ret;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(tegra->phys); i++) {
+		ret = phy_init(tegra->phys[i]);
+		if (ret)
+			goto disable_phy;
+		ret = phy_power_on(tegra->phys[i]);
+		if (ret) {
+			phy_exit(tegra->phys[i]);
+			goto disable_phy;
+		}
+	}
+
+	return 0;
+disable_phy:
+	for (i = i - 1; i >= 0; i--) {
+		phy_power_off(tegra->phys[i]);
+		phy_exit(tegra->phys[i]);
+	}
+	return ret;
+}
+
+static void tegra_xhci_phy_disable(struct tegra_xhci_hcd *tegra)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(tegra->phys); i++) {
+		phy_power_off(tegra->phys[i]);
+		phy_exit(tegra->phys[i]);
+	}
+}
+
+static bool is_host_mbox_message(u32 cmd)
+{
+	switch (cmd) {
+	case MBOX_CMD_INC_SSPI_CLOCK:
+	case MBOX_CMD_DEC_SSPI_CLOCK:
+	case MBOX_CMD_INC_FALC_CLOCK:
+	case MBOX_CMD_DEC_FALC_CLOCK:
+	case MBOX_CMD_SET_BW:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static void tegra_xhci_mbox_work(struct work_struct *work)
+{
+	struct tegra_xhci_hcd *tegra = container_of(work, struct tegra_xhci_hcd,
+						    mbox_req_work);
+	struct tegra_xusb_mbox_msg *msg = &tegra->mbox_req;
+	struct tegra_xusb_mbox_msg resp;
+	int ret;
+
+	resp.cmd = 0;
+	switch (msg->cmd) {
+	case MBOX_CMD_INC_SSPI_CLOCK:
+	case MBOX_CMD_DEC_SSPI_CLOCK:
+		ret = tegra_xhci_set_ss_clk(tegra, msg->data * 1000);
+		resp.data = clk_get_rate(tegra->ss_src_clk) / 1000;
+		if (ret)
+			resp.cmd = MBOX_CMD_NAK;
+		else
+			resp.cmd = MBOX_CMD_ACK;
+		break;
+	case MBOX_CMD_INC_FALC_CLOCK:
+	case MBOX_CMD_DEC_FALC_CLOCK:
+		resp.data = clk_get_rate(tegra->falc_clk) / 1000;
+		if (resp.data != msg->data)
+			resp.cmd = MBOX_CMD_NAK;
+		else
+			resp.cmd = MBOX_CMD_ACK;
+		break;
+	case MBOX_CMD_SET_BW:
+		/*
+		 * TODO: Request bandwidth once EMC scaling is supported.
+		 * Ignore for now since ACK/NAK is not required for SET_BW
+		 * messages.
+		 */
+		break;
+	default:
+		break;
+	}
+
+	if (resp.cmd)
+		mbox_send_message(tegra->mbox_chan, &resp);
+}
+
+static void tegra_xhci_mbox_rx(struct mbox_client *cl, void *data)
+{
+	struct tegra_xhci_hcd *tegra = dev_get_drvdata(cl->dev);
+	struct tegra_xusb_mbox_msg *msg = data;
+
+	if (is_host_mbox_message(msg->cmd)) {
+		tegra->mbox_req = *msg;
+		schedule_work(&tegra->mbox_req_work);
+	}
+}
+
+static void tegra_xhci_quirks(struct device *dev, struct xhci_hcd *xhci)
+{
+	xhci->quirks |= XHCI_PLAT;
+}
+
+static int tegra_xhci_setup(struct usb_hcd *hcd)
+{
+	return xhci_gen_setup(hcd, tegra_xhci_quirks);
+}
+
+static const struct tegra_xhci_soc_config tegra124_soc_config = {
+	.firmware_file = "nvidia/tegra124/xusb.bin",
+};
+MODULE_FIRMWARE("nvidia/tegra124/xusb.bin");
+
+static struct of_device_id tegra_xhci_of_match[] = {
+	{ .compatible = "nvidia,tegra124-xhci", .data = &tegra124_soc_config },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, tegra_xhci_of_match);
+
+static void tegra_xhci_probe_finish(const struct firmware *fw, void *context)
+{
+	struct tegra_xhci_hcd *tegra = context;
+	struct device *dev = tegra->dev;
+	struct xhci_hcd *xhci = NULL;
+	struct tegra_xhci_fw_cfgtbl *cfg_tbl;
+	struct tegra_xusb_mbox_msg msg;
+	int ret;
+
+	if (!fw)
+		goto put_usb2_hcd;
+
+	/* Load Falcon controller with its firmware. */
+	cfg_tbl = (struct tegra_xhci_fw_cfgtbl *)fw->data;
+	tegra->fw_size = le32_to_cpu(cfg_tbl->fwimg_len);
+	tegra->fw_data = dma_alloc_coherent(dev, tegra->fw_size,
+					    &tegra->fw_dma_addr,
+					    GFP_KERNEL);
+	if (!tegra->fw_data)
+		goto put_usb2_hcd;
+	memcpy(tegra->fw_data, fw->data, tegra->fw_size);
+
+	ret = tegra_xhci_load_firmware(tegra);
+	if (ret < 0)
+		goto put_usb2_hcd;
+
+	ret = usb_add_hcd(tegra->hcd, tegra->irq, IRQF_SHARED);
+	if (ret < 0)
+		goto put_usb2_hcd;
+	device_wakeup_enable(tegra->hcd->self.controller);
+
+	/*
+	 * USB 2.0 roothub is stored in drvdata now. Swap it with the Tegra HCD.
+	 */
+	tegra->hcd = dev_get_drvdata(dev);
+	dev_set_drvdata(dev, tegra);
+	xhci = hcd_to_xhci(tegra->hcd);
+	xhci->shared_hcd = usb_create_shared_hcd(&tegra_xhci_hc_driver,
+						 dev, dev_name(dev),
+						 tegra->hcd);
+	if (!xhci->shared_hcd)
+		goto dealloc_usb2_hcd;
+
+	/*
+	 * Set the xHCI pointer before xhci_plat_setup() (aka hcd_driver.reset)
+	 * is called by usb_add_hcd().
+	 */
+	*((struct xhci_hcd **) xhci->shared_hcd->hcd_priv) = xhci;
+	ret = usb_add_hcd(xhci->shared_hcd, tegra->irq, IRQF_SHARED);
+	if (ret < 0)
+		goto put_usb3_hcd;
+
+	/* Enable firmware messages from controller. */
+	msg.cmd = MBOX_CMD_MSG_ENABLED;
+	msg.data = 0;
+	ret = mbox_send_message(tegra->mbox_chan, &msg);
+	if (ret < 0)
+		goto dealloc_usb3_hcd;
+
+	tegra->fw_loaded = true;
+	release_firmware(fw);
+	return;
+
+	/* Free up as much as we can and wait to be unbound. */
+dealloc_usb3_hcd:
+	usb_remove_hcd(xhci->shared_hcd);
+put_usb3_hcd:
+	usb_put_hcd(xhci->shared_hcd);
+dealloc_usb2_hcd:
+	usb_remove_hcd(tegra->hcd);
+	kfree(xhci);
+put_usb2_hcd:
+	usb_put_hcd(tegra->hcd);
+	tegra->hcd = NULL;
+	release_firmware(fw);
+}
+
+static int tegra_xhci_probe(struct platform_device *pdev)
+{
+	struct tegra_xhci_hcd *tegra;
+	struct usb_hcd *hcd;
+	struct resource	*res;
+	struct phy *phy;
+	const struct of_device_id *match;
+	int ret, i, j, k;
+
+	BUILD_BUG_ON(sizeof(struct tegra_xhci_fw_cfgtbl) != 256);
+
+	tegra = devm_kzalloc(&pdev->dev, sizeof(*tegra), GFP_KERNEL);
+	if (!tegra)
+		return -ENOMEM;
+	tegra->dev = &pdev->dev;
+	platform_set_drvdata(pdev, tegra);
+
+	match = of_match_device(tegra_xhci_of_match, &pdev->dev);
+	if (!match) {
+		dev_err(&pdev->dev, "No device match found\n");
+		return -ENODEV;
+	}
+	tegra->soc_config = match->data;
+
+	/*
+	 * Right now device-tree probed devices don't get dma_mask set.
+	 * Since shared usb code relies on it, set it here for now.
+	 * Once we have dma capability bindings this can go away.
+	 */
+	ret = dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
+	if (ret)
+		return ret;
+
+	hcd = usb_create_hcd(&tegra_xhci_hc_driver, &pdev->dev,
+				    dev_name(&pdev->dev));
+	if (!hcd)
+		return -ENOMEM;
+	tegra->hcd = hcd;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	hcd->regs = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(hcd->regs)) {
+		ret = PTR_ERR(hcd->regs);
+		goto put_hcd;
+	}
+	hcd->rsrc_start = res->start;
+	hcd->rsrc_len = resource_size(res);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	tegra->fpci_base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(tegra->fpci_base)) {
+		ret = PTR_ERR(tegra->fpci_base);
+		goto put_hcd;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
+	tegra->ipfs_base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(tegra->ipfs_base)) {
+		ret = PTR_ERR(tegra->ipfs_base);
+		goto put_hcd;
+	}
+
+	tegra->irq = platform_get_irq(pdev, 0);
+	if (tegra->irq < 0) {
+		ret = tegra->irq;
+		goto put_hcd;
+	}
+
+	tegra->host_rst = devm_reset_control_get(&pdev->dev, "xusb_host");
+	if (IS_ERR(tegra->host_rst)) {
+		ret = PTR_ERR(tegra->host_rst);
+		goto put_hcd;
+	}
+	tegra->ss_rst = devm_reset_control_get(&pdev->dev, "xusb_ss");
+	if (IS_ERR(tegra->ss_rst)) {
+		ret = PTR_ERR(tegra->ss_rst);
+		goto put_hcd;
+	}
+
+	tegra->host_clk = devm_clk_get(&pdev->dev, "xusb_host");
+	if (IS_ERR(tegra->host_clk)) {
+		ret = PTR_ERR(tegra->host_clk);
+		goto put_hcd;
+	}
+	tegra->falc_clk = devm_clk_get(&pdev->dev, "xusb_falcon_src");
+	if (IS_ERR(tegra->falc_clk)) {
+		ret = PTR_ERR(tegra->falc_clk);
+		goto put_hcd;
+	}
+	tegra->ss_clk = devm_clk_get(&pdev->dev, "xusb_ss");
+	if (IS_ERR(tegra->ss_clk)) {
+		ret = PTR_ERR(tegra->ss_clk);
+		goto put_hcd;
+	}
+	tegra->ss_src_clk = devm_clk_get(&pdev->dev, "xusb_ss_src");
+	if (IS_ERR(tegra->ss_src_clk)) {
+		ret = PTR_ERR(tegra->ss_src_clk);
+		goto put_hcd;
+	}
+	tegra->hs_src_clk = devm_clk_get(&pdev->dev, "xusb_hs_src");
+	if (IS_ERR(tegra->hs_src_clk)) {
+		ret = PTR_ERR(tegra->hs_src_clk);
+		goto put_hcd;
+	}
+	tegra->fs_src_clk = devm_clk_get(&pdev->dev, "xusb_fs_src");
+	if (IS_ERR(tegra->fs_src_clk)) {
+		ret = PTR_ERR(tegra->fs_src_clk);
+		goto put_hcd;
+	}
+	tegra->pll_u_480m = devm_clk_get(&pdev->dev, "pll_u_480m");
+	if (IS_ERR(tegra->pll_u_480m)) {
+		ret = PTR_ERR(tegra->pll_u_480m);
+		goto put_hcd;
+	}
+	tegra->clk_m = devm_clk_get(&pdev->dev, "clk_m");
+	if (IS_ERR(tegra->clk_m)) {
+		ret = PTR_ERR(tegra->clk_m);
+		goto put_hcd;
+	}
+	tegra->pll_e = devm_clk_get(&pdev->dev, "pll_e");
+	if (IS_ERR(tegra->pll_e)) {
+		ret = PTR_ERR(tegra->pll_e);
+		goto put_hcd;
+	}
+	ret = tegra_xhci_clk_enable(tegra);
+	if (ret)
+		goto put_hcd;
+
+	for (i = 0; i < ARRAY_SIZE(tegra->supplies); i++)
+		tegra->supplies[i].supply = tegra_xhci_supply_names[i];
+	ret = devm_regulator_bulk_get(&pdev->dev, ARRAY_SIZE(tegra->supplies),
+				      tegra->supplies);
+	if (ret)
+		goto disable_clk;
+	ret = regulator_bulk_enable(ARRAY_SIZE(tegra->supplies),
+				    tegra->supplies);
+	if (ret)
+		goto disable_clk;
+
+	INIT_WORK(&tegra->mbox_req_work, tegra_xhci_mbox_work);
+	tegra->mbox_client.dev = &pdev->dev;
+	tegra->mbox_client.tx_block = true;
+	tegra->mbox_client.tx_tout = 0;
+	tegra->mbox_client.rx_callback = tegra_xhci_mbox_rx;
+	tegra->mbox_chan = mbox_request_channel(&tegra->mbox_client, 0);
+	if (IS_ERR(tegra->mbox_chan)) {
+		ret = PTR_ERR(tegra->mbox_chan);
+		goto disable_regulator;
+	}
+
+	k = 0;
+	for (i = 0; i < ARRAY_SIZE(tegra_xhci_phy_types); i++) {
+		char prop[8];
+
+		BUG_ON(sizeof(prop) < strlen(tegra_xhci_phy_types[i].name) + 3);
+		for (j = 0; j < tegra_xhci_phy_types[i].num; j++) {
+			sprintf(prop, "%s-%d", tegra_xhci_phy_types[i].name, j);
+			phy = devm_phy_optional_get(&pdev->dev, prop);
+			if (IS_ERR(phy)) {
+				ret = PTR_ERR(phy);
+				goto put_mbox;
+			}
+			tegra->phys[k++] = phy;
+		}
+	}
+
+	/* Setup IPFS access and BAR0 space. */
+	tegra_xhci_cfg(tegra);
+
+	ret = tegra_xhci_phy_enable(tegra);
+	if (ret < 0)
+		goto put_mbox;
+
+	ret = request_firmware_nowait(THIS_MODULE, true,
+				      tegra->soc_config->firmware_file,
+				      tegra->dev, GFP_KERNEL, tegra,
+				      tegra_xhci_probe_finish);
+	if (ret < 0)
+		goto disable_phy;
+
+	return 0;
+
+disable_phy:
+	tegra_xhci_phy_disable(tegra);
+put_mbox:
+	mbox_free_channel(tegra->mbox_chan);
+disable_regulator:
+	regulator_bulk_disable(ARRAY_SIZE(tegra->supplies), tegra->supplies);
+disable_clk:
+	tegra_xhci_clk_disable(tegra);
+put_hcd:
+	usb_put_hcd(hcd);
+	return ret;
+}
+
+static int tegra_xhci_remove(struct platform_device *pdev)
+{
+	struct tegra_xhci_hcd *tegra = platform_get_drvdata(pdev);
+	struct usb_hcd *hcd = tegra->hcd;
+	struct xhci_hcd *xhci;
+
+	if (tegra->fw_loaded) {
+		xhci = hcd_to_xhci(hcd);
+		usb_remove_hcd(xhci->shared_hcd);
+		usb_put_hcd(xhci->shared_hcd);
+		usb_remove_hcd(hcd);
+		usb_put_hcd(hcd);
+		kfree(xhci);
+	} else if (hcd) {
+		/* Unbound after probe(), but before firmware loading. */
+		usb_put_hcd(hcd);
+	}
+
+	if (tegra->fw_data)
+		dma_free_coherent(tegra->dev, tegra->fw_size, tegra->fw_data,
+				  tegra->fw_dma_addr);
+
+	cancel_work_sync(&tegra->mbox_req_work);
+	mbox_free_channel(tegra->mbox_chan);
+	tegra_xhci_phy_disable(tegra);
+	regulator_bulk_disable(ARRAY_SIZE(tegra->supplies), tegra->supplies);
+	tegra_xhci_clk_disable(tegra);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int tegra_xhci_suspend(struct device *dev)
+{
+	struct tegra_xhci_hcd *tegra = dev_get_drvdata(dev);
+	struct xhci_hcd *xhci = hcd_to_xhci(tegra->hcd);
+
+	return xhci_suspend(xhci);
+}
+
+static int tegra_xhci_resume(struct device *dev)
+{
+	struct tegra_xhci_hcd *tegra = dev_get_drvdata(dev);
+	struct xhci_hcd *xhci = hcd_to_xhci(tegra->hcd);
+
+	return xhci_resume(xhci, 0);
+}
+#endif
+
+static const struct dev_pm_ops tegra_xhci_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(tegra_xhci_suspend, tegra_xhci_resume)
+};
+
+static struct platform_driver tegra_xhci_driver = {
+	.probe	= tegra_xhci_probe,
+	.remove	= tegra_xhci_remove,
+	.driver	= {
+		.name = "xhci-tegra",
+		.pm = &tegra_xhci_pm_ops,
+		.of_match_table = of_match_ptr(tegra_xhci_of_match),
+	},
+};
+
+static int __init tegra_xhci_init(void)
+{
+	xhci_init_driver(&tegra_xhci_hc_driver, tegra_xhci_setup);
+	return platform_driver_register(&tegra_xhci_driver);
+}
+module_init(tegra_xhci_init);
+
+static void __exit tegra_xhci_exit(void)
+{
+	platform_driver_unregister(&tegra_xhci_driver);
+}
+module_exit(tegra_xhci_exit);
+
+MODULE_AUTHOR("Andrew Bresticker <abrestic@chromium.org>");
+MODULE_DESCRIPTION("NVIDIA Tegra xHCI host-controller driver");
+MODULE_LICENSE("GPL v2");
-- 
2.1.0.rc2.206.gedb03e5

[PATCH RESEND V4 7/9] ARM: tegra: Add Tegra124 XUSB mailbox and xHCI controller

[Andrew Bresticker]

Add nodes for the Tegra XUSB mailbox and Tegra xHCI controller and
add the PHY mailbox channel to the XUSB padctl node.

Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Reviewed-by: Stephen Warren <swarren@nvidia.com>
---
No changes from v3.
Changes from v2:
 - Dropped channel specifier from mailbox bindings.
 - Added mbox-names properties.
Changes from v1:
 - Updated to use common mailbox bindings.
 - Added remaining clocks/resets.
---
 arch/arm/boot/dts/tegra124.dtsi | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/arch/arm/boot/dts/tegra124.dtsi b/arch/arm/boot/dts/tegra124.dtsi
index 478c555..73bc7a4 100644
--- a/arch/arm/boot/dts/tegra124.dtsi
+++ b/arch/arm/boot/dts/tegra124.dtsi
@@ -591,11 +591,52 @@
 		status = "disabled";
 	};
 
+	usb@0,70090000 {
+		compatible = "nvidia,tegra124-xhci";
+		reg = <0x0 0x70090000 0x0 0x8000>,
+		      <0x0 0x70098000 0x0 0x1000>,
+		      <0x0 0x70099000 0x0 0x1000>;
+		interrupts = <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&tegra_car TEGRA124_CLK_XUSB_HOST>,
+			 <&tegra_car TEGRA124_CLK_XUSB_HOST_SRC>,
+			 <&tegra_car TEGRA124_CLK_XUSB_DEV>,
+			 <&tegra_car TEGRA124_CLK_XUSB_DEV_SRC>,
+			 <&tegra_car TEGRA124_CLK_XUSB_FALCON_SRC>,
+			 <&tegra_car TEGRA124_CLK_XUSB_SS>,
+			 <&tegra_car TEGRA124_CLK_XUSB_SS_DIV2>,
+			 <&tegra_car TEGRA124_CLK_XUSB_SS_SRC>,
+			 <&tegra_car TEGRA124_CLK_XUSB_HS_SRC>,
+			 <&tegra_car TEGRA124_CLK_XUSB_FS_SRC>,
+			 <&tegra_car TEGRA124_CLK_PLL_U_480M>,
+			 <&tegra_car TEGRA124_CLK_CLK_M>,
+			 <&tegra_car TEGRA124_CLK_PLL_E>;
+		clock-names = "xusb_host", "xusb_host_src", "xusb_dev",
+			      "xusb_dev_src", "xusb_falcon_src", "xusb_ss",
+			      "xusb_ss_div2", "xusb_ss_src", "xusb_hs_src",
+			      "xusb_fs_src", "pll_u_480m", "clk_m", "pll_e";
+		resets = <&tegra_car 89>, <&tegra_car 95>, <&tegra_car 156>,
+			 <&tegra_car 143>;
+		reset-names = "xusb_host", "xusb_dev", "xusb_ss", "xusb";
+		mboxes = <&xusb_mbox>;
+		mbox-names = "xusb";
+		status = "disabled";
+	};
+
+	xusb_mbox: mailbox@0,70098000 {
+		compatible = "nvidia,tegra124-xusb-mbox";
+		reg = <0x0 0x70098000 0x0 0x1000>;
+		interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>;
+
+		#mbox-cells = <0>;
+	};
+
 	padctl: padctl@0,7009f000 {
 		compatible = "nvidia,tegra124-xusb-padctl";
 		reg = <0x0 0x7009f000 0x0 0x1000>;
 		resets = <&tegra_car 142>;
 		reset-names = "padctl";
+		mboxes = <&xusb_mbox>;
+		mbox-names = "xusb";
 
 		#phy-cells = <1>;
 	};
-- 
2.1.0.rc2.206.gedb03e5

[PATCH RESEND V4 8/9] ARM: tegra: jetson-tk1: Add xHCI support

[Andrew Bresticker]

Assign USB ports previously owned by the EHCI controllers to the xHCI
controller.  There is a mini-PCIe USB port (UTMI port 1) and a USB A
connector (UTMI port 2, USB3 port 0).  PCIe lane 0 is used for USB3
port 0.

Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Reviewed-by: Stephen Warren <swarren@nvidia.com>
---
Changes from v3:
 - Assigned otg-0 to XUSB to avoid USB2.0 flakiness.
Changes from v2:
 - Updated VBUS power supply names.
Changes from v1:
 - Updated USB power supplies.
---
 arch/arm/boot/dts/tegra124-jetson-tk1.dts | 46 +++++++++++++++++--------------
 1 file changed, 26 insertions(+), 20 deletions(-)

diff --git a/arch/arm/boot/dts/tegra124-jetson-tk1.dts b/arch/arm/boot/dts/tegra124-jetson-tk1.dts
index 029c9a02..0dc1553 100644
--- a/arch/arm/boot/dts/tegra124-jetson-tk1.dts
+++ b/arch/arm/boot/dts/tegra124-jetson-tk1.dts
@@ -1686,15 +1686,40 @@
 		target-12v-supply = <&vdd_12v0_sata>;
 	};
 
+	usb@0,70090000 {
+		status = "okay";
+		phys = <&padctl TEGRA_XUSB_PADCTL_UTMI_P1>, /* mini-PCIe USB */
+		       <&padctl TEGRA_XUSB_PADCTL_UTMI_P2>, /* USB A */
+		       <&padctl TEGRA_XUSB_PADCTL_USB3_P0>; /* USB A */
+		phy-names = "utmi-1", "utmi-2", "usb3-0";
+		avddio-pex-supply = <&vdd_1v05_run>;
+		dvddio-pex-supply = <&vdd_1v05_run>;
+		avdd-usb-supply = <&vdd_3v3_lp0>;
+		avdd-pll-utmip-supply = <&vddio_1v8>;
+		avdd-pll-erefe-supply = <&avdd_1v05_run>;
+		avdd-pex-pll-supply = <&vdd_1v05_run>;
+		hvdd-pex-supply = <&vdd_3v3_lp0>;
+		hvdd-pex-plle-supply = <&vdd_3v3_lp0>;
+	};
+
 	padctl@0,7009f000 {
 		pinctrl-0 = <&padctl_default>;
 		pinctrl-names = "default";
 
+		vbus-2-supply = <&vdd_usb3_vbus>;
+		nvidia,usb3-port-0-lane = <TEGRA_XUSB_PADCTL_PIN_PCIE_0>;
+
 		padctl_default: pinmux {
+			otg {
+				nvidia,lanes = "otg-0", "otg-1", "otg-2";
+				nvidia,function = "xusb";
+			};
+
 			usb3 {
-				nvidia,lanes = "pcie-0", "pcie-1";
+				nvidia,lanes = "pcie-0";
 				nvidia,function = "usb3";
 				nvidia,iddq = <0>;
+				nvidia,usb2-port-num = <2>;
 			};
 
 			pcie {
@@ -1735,25 +1760,6 @@
 		};
 	};
 
-	/* mini-PCIe USB */
-	usb@0,7d004000 {
-		status = "okay";
-	};
-
-	usb-phy@0,7d004000 {
-		status = "okay";
-	};
-
-	/* USB A connector */
-	usb@0,7d008000 {
-		status = "okay";
-	};
-
-	usb-phy@0,7d008000 {
-		status = "okay";
-		vbus-supply = <&vdd_usb3_vbus>;
-	};
-
 	clocks {
 		compatible = "simple-bus";
 		#address-cells = <1>;
-- 
2.1.0.rc2.206.gedb03e5

[PATCH RESEND V4 9/9] ARM: tegra: venice2: Add xHCI support

[Andrew Bresticker]

Assign ports previously owned by the EHCI controllers to the xHCI
controller.  There are two external USB ports (UTMI ports 0/2 and
USB3 ports 0/1) and an internal USB port (UTMI port 1).  PCIe lanes
0 and 1 are used by the USB3 ports.

Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Reviewed-by: Stephen Warren <swarren@nvidia.com>
---
No changes from v3.
Changes from v2:
 - Updated VBUS power supply names.
Changes from v1:
 - Updated USB power supplies.
---
 arch/arm/boot/dts/tegra124-venice2.dts | 79 ++++++++++++++++++++++------------
 1 file changed, 51 insertions(+), 28 deletions(-)

diff --git a/arch/arm/boot/dts/tegra124-venice2.dts b/arch/arm/boot/dts/tegra124-venice2.dts
index 1300885..d7106f0 100644
--- a/arch/arm/boot/dts/tegra124-venice2.dts
+++ b/arch/arm/boot/dts/tegra124-venice2.dts
@@ -744,7 +744,7 @@
 					regulator-always-on;
 				};
 
-				ldo0 {
+				avdd_1v05_run: ldo0 {
 					regulator-name = "+1.05V_RUN_AVDD";
 					regulator-min-microvolt = <1050000>;
 					regulator-max-microvolt = <1050000>;
@@ -886,6 +886,56 @@
 		status = "okay";
 	};
 
+	usb@0,70090000 {
+		status = "okay";
+		phys = <&padctl TEGRA_XUSB_PADCTL_UTMI_P0>, /* 1st USB A */
+		       <&padctl TEGRA_XUSB_PADCTL_UTMI_P1>, /* Internal USB */
+		       <&padctl TEGRA_XUSB_PADCTL_UTMI_P2>, /* 2nd USB A */
+		       <&padctl TEGRA_XUSB_PADCTL_USB3_P0>, /* 1st USB A */
+		       <&padctl TEGRA_XUSB_PADCTL_USB3_P1>; /* 2nd USB A */
+		phy-names = "utmi-0", "utmi-1", "utmi-2", "usb3-0", "usb3-1";
+		avddio-pex-supply = <&vdd_1v05_run>;
+		dvddio-pex-supply = <&vdd_1v05_run>;
+		avdd-usb-supply = <&vdd_3v3_lp0>;
+		avdd-pll-utmip-supply = <&vddio_1v8>;
+		avdd-pll-erefe-supply = <&avdd_1v05_run>;
+		avdd-pex-pll-supply = <&vdd_1v05_run>;
+		hvdd-pex-supply = <&vdd_3v3_lp0>;
+		hvdd-pex-plle-supply = <&vdd_3v3_lp0>;
+	};
+
+	padctl@0,7009f000 {
+		pinctrl-0 = <&padctl_default>;
+		pinctrl-names = "default";
+
+		vbus-0-supply = <&vdd_usb1_vbus>;
+		vbus-1-supply = <&vdd_run_cam>;
+		vbus-2-supply = <&vdd_usb3_vbus>;
+		nvidia,usb3-port-0-lane = <TEGRA_XUSB_PADCTL_PIN_PCIE_0>;
+		nvidia,usb3-port-1-lane = <TEGRA_XUSB_PADCTL_PIN_PCIE_1>;
+
+		padctl_default: pinmux {
+			otg {
+				nvidia,lanes = "otg-0", "otg-1", "otg-2";
+				nvidia,function = "xusb";
+			};
+
+			usb3p0 {
+				nvidia,lanes = "pcie-0";
+				nvidia,function = "usb3";
+				nvidia,iddq = <0>;
+				nvidia,usb2-port-num = <0>;
+			};
+
+			usb3p1 {
+				nvidia,lanes = "pcie-1";
+				nvidia,function = "usb3";
+				nvidia,iddq = <0>;
+				nvidia,usb2-port-num = <2>;
+			};
+		};
+	};
+
 	sdhci@0,700b0400 {
 		cd-gpios = <&gpio TEGRA_GPIO(V, 2) GPIO_ACTIVE_HIGH>;
 		power-gpios = <&gpio TEGRA_GPIO(R, 0) GPIO_ACTIVE_HIGH>;
@@ -906,33 +956,6 @@
 		};
 	};
 
-	usb@0,7d000000 {
-		status = "okay";
-	};
-
-	usb-phy@0,7d000000 {
-		status = "okay";
-		vbus-supply = <&vdd_usb1_vbus>;
-	};
-
-	usb@0,7d004000 {
-		status = "okay";
-	};
-
-	usb-phy@0,7d004000 {
-		status = "okay";
-		vbus-supply = <&vdd_run_cam>;
-	};
-
-	usb@0,7d008000 {
-		status = "okay";
-	};
-
-	usb-phy@0,7d008000 {
-		status = "okay";
-		vbus-supply = <&vdd_usb3_vbus>;
-	};
-
 	backlight: backlight {
 		compatible = "pwm-backlight";
 
-- 
2.1.0.rc2.206.gedb03e5

Re: [PATCH RESEND V4 0/9] Tegra xHCI support

[Alexandre Courbot]

On Wed, Oct 29, 2014 at 7:27 AM, Andrew Bresticker
<abrestic@chromium.org> wrote:
> This series adds support for xHCI on NVIDIA Tegra SoCs.  This includes:
>  - patches 1 and 2: adding a driver for the mailbox used to communicate
>    with the xHCI controller's firmware,
>  - patches 3 and 4: extending the XUSB pad controller driver to support
>    the USB PHY types (UTMI, HSIC, and USB3),
>  - patches 5 and 6: adding a xHCI host-controller driver, and
>  - patches 7, 8, and 9: updating the relevant DT files.
>
> The PHY and host drivers have compile-time dependencies on the mailbox
> driver, and the host driver has compile-time dependencies on the PHY
> driver.  It is probably best if these all get merged through the Tegra
> tree.  This series still needs ACKs from the relevant maintainers for
> the mailbox and xHCI host drivers and their device-tree bindings.
>
> Tested on Venice2, Jetson TK1, and Big with a variety of USB2.0 and
> USB3.0 memory sticks and ethernet dongles using controller firmware
> recently posted by Andrew Chew [0].
>
> Based on v3.18-rc2.  All other dependencies (xHCI modules and mailbox
> series) have been merged.

I do not feel comfortable enough with xHCI to give a reviewed-by tag,
but FWIW I have found nothing wrong with these patches.

Re: [PATCH RESEND V4 5/9] of: Add NVIDIA Tegra xHCI controller binding

[Thierry Reding]

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

On Tue, Oct 28, 2014 at 03:27:50PM -0700, Andrew Bresticker wrote:
[...]
> diff --git a/Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-xusb-padctl.txt b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-xusb-padctl.txt
[...]
> +Optional properties:
> +-------------------
> +- vbus-{0,1,2}-supply: VBUS regulator for the corresponding UTMI pad.
> +- vddio-hsic-supply: VDDIO regulator for the HSIC pads.
> +- nvidia,usb3-port-{0,1}-lane: PCIe/SATA lane to which the corresponding USB3
> +  port is mapped.  See <dt-bindings/pinctrl/pinctrl-tegra-xusb.h> for the list
> +  of valid values.

I dislike how we now need to provide a list of all pins in the header
file, where previously we used strings for this. This could become very
ugly if the set of pins changes in future generations of this IP block.

Could we instead derive this from the pinmux nodes? For example you have
this in the example below:

	usb3p0 {
		nvidia,lanes = "pcie-0";
		...
	};

Perhaps what we need is to either key off the node name or add another
property, such as:

	nvidia,usb3-port = <0>;

This would match the nvidia,usb2-port property that you've added below.

>  Lane muxing:
>  ------------
> @@ -50,6 +62,17 @@ Optional properties:
>    pin or group should be assigned to. Valid values for function names are
>    listed below.
>  - nvidia,iddq: Enables IDDQ mode of the lane. (0: no, 1: yes)
> +- nvidia,usb2-port-num: USB2 port (0, 1, or 2) to which the lane is mapped.

I'd leave away the -num suffix since it is implied that it's a number.

> +- nvidia,hsic-strobe-trim: HSIC strobe trimmer value.
> +- nvidia,hsic-rx-strobe-trim: HSIC RX strobe trimmer value.
> +- nvidia,hsic-rx-data-trim: HSIC RX data trimmer value.
> +- nvidia,hsic-tx-rtune-n: HSIC TX RTUNEN value.
> +- nvidia,hsic-tx-rtune-p: HSIC TX RTUNEP value.
> +- nvidia,hsic-tx-slew-n: HSIC TX SLEWN value.
> +- nvidia,hsic-tx-slew-p: HSIC TX SLEWP value.

It would be useful for these to provide a range of valid values. I also
see that there are other registers that contain values for tuning. I
take it that the ones you've included here are the only ones that need
to be overridden on hardware you've tested this on? That should be fine,
we can always add new ones if necessary.

> +- nvidia,hsic-auto-term: Enables HSIC AUTO_TERM. (0: no, 1: yes)
> +- nvidia,otg-hs-curr-level-offset: Offset to be applied to the pad's fused
> +  HS_CURR_LEVEL value.
>  
>  Note that not all of these properties are valid for all lanes. Lanes can be
>  divided into three groups:
> @@ -58,18 +81,21 @@ divided into three groups:
>  
>      Valid functions for this group are: "snps", "xusb", "uart", "rsvd".
>  
> -    The nvidia,iddq property does not apply to this group.
> +    The nvidia,otg-hs-curr-level-offset property only applies.

The wording is confusing here in my opinion. Maybe better: "Only the ...
property applies."?

>    - ulpi-0, hsic-0, hsic-1:
>  
>      Valid functions for this group are: "snps", "xusb".
>  
> -    The nvidia,iddq property does not apply to this group.
> +    The nvidia,hsic-* properties apply only to the pins hsic-{0,1} when
> +    the function is xusb.

I assume that hsic-* properties also only apply to the hsic-* pins?
That's not clear from the above sentence.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH RESEND V4 5/9] of: Add NVIDIA Tegra xHCI controller binding

[Thierry Reding]

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

On Tue, Oct 28, 2014 at 03:27:52PM -0700, Andrew Bresticker wrote:
[...]
> diff --git a/Documentation/devicetree/bindings/usb/nvidia,tegra124-xhci.txt b/Documentation/devicetree/bindings/usb/nvidia,tegra124-xhci.txt
[...]
> +    - pll_u_480m
> +    - clk_m
> +    - pll_e

What are these used for? I guess I'll see when I get to the driver
patch.

> +Optional properties:
> +--------------------
> + - phys: Must contain an entry for each entry in phy-names.
> +   See ../phy/phy-bindings.txt for details.
> + - phy-names: Should include an entry for each PHY used by the controller.
> +   May be a subset of the following:
> +    - utmi-{0,1,2}
> +    - hsic-{0,1}
> +    - usb3-{0,1}
> + - avddio-pex-supply: PCIe/USB3 analog logic power supply.  Must supply 1.05V.
> + - dvddio-pex-supply: PCIe/USB3 digital logic power supply.  Must supply 1.05V.
> + - avdd-usb-supply: USB controller power supply.  Must supply 3.3V.
> + - avdd-pll-utmip-supply: UTMI PLL power supply.  Must supply 1.8V.
> + - avdd-pll-erefe-supply: PLLE reference PLL power supply.  Must supply 1.05V.
> + - avdd-pex-pll-supply: PCIe/USB3 PLL power supply.  Must supply 1.05V.
> + - hvdd-pex-supply: High-voltage PCIe/USB3 power supply.  Must supply 3.3V.
> + - hvdd-pex-plle-supply: High-voltage PLLE power supply.  Must supply 3.3V.

I think the name for this in the documentation is HVDD_PEX_PLL_E, which
would translate to hvdd-pex-pll-e. At least that's how we named this
supply for PCIe.

Alternatively it seems like there are aliases for the USB 3.0 related
supplies:

	avdd-pex-pll -> avdd-usb-ss-pll
	hvdd-pex -> hvdd-usb-ss
	hvdd-pex-pll-e -> hvdd-usb-ss-pll-e

So perhaps these could be used for the XHCI driver instead? Also, should
these supplies not be mandatory?

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH RESEND V4 6/9] usb: xhci: Add NVIDIA Tegra xHCI host-controller driver

[Thierry Reding]

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

On Tue, Oct 28, 2014 at 03:27:53PM -0700, Andrew Bresticker wrote:
[...]
> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
[...]
> +#define TEGRA_XHCI_NUM_SUPPLIES 8
> +static const char *tegra_xhci_supply_names[TEGRA_XHCI_NUM_SUPPLIES] = {
> +	"avddio-pex",
> +	"dvddio-pex",
> +	"avdd-usb",
> +	"avdd-pll-utmip",
> +	"avdd-pll-erefe",
> +	"avdd-pex-pll",
> +	"hvdd-pex",
> +	"hvdd-pex-plle",
> +};

This could be in a per-SoC structure since it's likely to change in a
future SoC. That could be done later on when it really becomes relevant,
though.

> +
> +static const struct {
> +	const char *name;
> +	int num;

unsigned?

> +} tegra_xhci_phy_types[] = {
> +	{
> +		.name = "usb3",
> +		.num = TEGRA_XUSB_USB3_PHYS,
> +	}, {
> +		.name = "utmi",
> +		.num = TEGRA_XUSB_UTMI_PHYS,
> +	}, {
> +		.name = "hsic",
> +		.num = TEGRA_XUSB_HSIC_PHYS,
> +	},
> +};

Should these constants perhaps be in a per-SoC structure like
tegra_xhci_soc_config rather than defined in a global header?

> +static int tegra_xhci_load_firmware(struct tegra_xhci_hcd *tegra)
> +{
[...]
> +	/* Start Falcon CPU. */
> +	csb_writel(tegra, CPUCTL_STARTCPU, XUSB_FALC_CPUCTL);
> +	usleep_range(1000, 2000);
> +
> +	fw_time = le32_to_cpu(cfg_tbl->fwimg_created_time);
> +	time_to_tm(fw_time, 0, &fw_tm);
> +	dev_info(dev,
> +		 "Firmware timestamp: %ld-%02d-%02d %02d:%02d:%02d UTC, "
> +		 "Falcon state 0x%x\n", fw_tm.tm_year + 1900,
> +		 fw_tm.tm_mon + 1, fw_tm.tm_mday, fw_tm.tm_hour,
> +		 fw_tm.tm_min, fw_tm.tm_sec,
> +		 csb_readl(tegra, XUSB_FALC_CPUCTL));
> +
> +	/* Make sure Falcon CPU is now running. */
> +	if (csb_readl(tegra, XUSB_FALC_CPUCTL) == CPUCTL_STATE_HALTED)
> +		return -EIO;

It seems somewhat strange to output the dev_info() message when in fact
it could be that the Falcon wasn't successfully booted. Also is it
guaranteed that the Falcon will always be up after 1 ms? Perhaps better
would be to use a timed loop?

> +static int tegra_xhci_set_ss_clk(struct tegra_xhci_hcd *tegra,
> +				 unsigned long rate)
> +{
> +	unsigned long new_parent_rate, old_parent_rate;
> +	int ret, div;
> +	struct clk *clk = tegra->ss_src_clk;
> +
> +	if (clk_get_rate(clk) == rate)
> +		return 0;
> +
> +	switch (rate) {
> +	case TEGRA_XHCI_SS_CLK_HIGH_SPEED:
> +		/*
> +		 * Reparent to PLLU_480M. Set divider first to avoid
> +		 * overclocking.
> +		 */
> +		old_parent_rate = clk_get_rate(clk_get_parent(clk));
> +		new_parent_rate = clk_get_rate(tegra->pll_u_480m);
> +		div = new_parent_rate / rate;
> +		ret = clk_set_rate(clk, old_parent_rate / div);
> +		if (ret)
> +			return ret;
> +		ret = clk_set_parent(clk, tegra->pll_u_480m);
> +		if (ret)
> +			return ret;
> +		/*
> +		 * The rate should already be correct, but set it again just
> +		 * to be sure.
> +		 */
> +		ret = clk_set_rate(clk, rate);
> +		if (ret)
> +			return ret;
> +		break;
> +	case TEGRA_XHCI_SS_CLK_LOW_SPEED:
> +		/* Reparent to CLK_M */
> +		ret = clk_set_parent(clk, tegra->clk_m);
> +		if (ret)
> +			return ret;
> +		ret = clk_set_rate(clk, rate);
> +		if (ret)
> +			return ret;
> +		break;
> +	default:
> +		dev_err(tegra->dev, "Invalid SS rate: %lu\n", rate);
> +		return -EINVAL;
> +	}
> +
> +	if (clk_get_rate(clk) != rate) {
> +		dev_err(tegra->dev, "SS clock doesn't match requested rate\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}

So this is why you need pllu_480m and clk_m clocks. I would've thought
it nice to use something like the assigned-clocks properties to take
care of this, but it seems like this may actually be required to be
updated dynamically at runtime, so a fixed property is not going to be
an option.

> +static int tegra_xhci_clk_enable(struct tegra_xhci_hcd *tegra)
> +{
> +	clk_prepare_enable(tegra->pll_e);
> +	clk_prepare_enable(tegra->host_clk);
> +	clk_prepare_enable(tegra->ss_clk);
> +	clk_prepare_enable(tegra->falc_clk);
> +	clk_prepare_enable(tegra->fs_src_clk);
> +	clk_prepare_enable(tegra->hs_src_clk);

You should error-check these.

> +static int tegra_xhci_phy_enable(struct tegra_xhci_hcd *tegra)
> +{
> +	int ret;
> +	int i;

I prefer unsigned when the value can't be negative as in this case.

> +
> +	for (i = 0; i < ARRAY_SIZE(tegra->phys); i++) {
> +		ret = phy_init(tegra->phys[i]);
> +		if (ret)
> +			goto disable_phy;
> +		ret = phy_power_on(tegra->phys[i]);
> +		if (ret) {
> +			phy_exit(tegra->phys[i]);
> +			goto disable_phy;
> +		}
> +	}

Perhaps a phy_init_and_power_on() helper would be useful. Nothing that
needs to be done as part of this patch, though.

> +
> +	return 0;
> +disable_phy:
> +	for (i = i - 1; i >= 0; i--) {
> +		phy_power_off(tegra->phys[i]);
> +		phy_exit(tegra->phys[i]);

You could write this as:

	for (i = i; i > 0; i--) {
		phy_power_off(tegra->phys[i - 1]);
		...
	}

for the unsigned case. But I guess this would be a reasonable exception
to let i be signed.

> +static void tegra_xhci_phy_disable(struct tegra_xhci_hcd *tegra)
> +{
> +	int i;

There's no reason for it to be signed here, though.

> +
> +	for (i = 0; i < ARRAY_SIZE(tegra->phys); i++) {
> +		phy_power_off(tegra->phys[i]);
> +		phy_exit(tegra->phys[i]);
> +	}
> +}
> +
> +static bool is_host_mbox_message(u32 cmd)
> +{
> +	switch (cmd) {
> +	case MBOX_CMD_INC_SSPI_CLOCK:
> +	case MBOX_CMD_DEC_SSPI_CLOCK:
> +	case MBOX_CMD_INC_FALC_CLOCK:
> +	case MBOX_CMD_DEC_FALC_CLOCK:
> +	case MBOX_CMD_SET_BW:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static void tegra_xhci_mbox_work(struct work_struct *work)
> +{
> +	struct tegra_xhci_hcd *tegra = container_of(work, struct tegra_xhci_hcd,
> +						    mbox_req_work);

There's only a single instance of this, but it might still be useful to
wrap it in a static inline for readability.

> +	/*
> +	 * Set the xHCI pointer before xhci_plat_setup() (aka hcd_driver.reset)

I don't think this driver calls xhci_plat_setup() (anymore?).

> +	 * is called by usb_add_hcd().
> +	 */
> +	*((struct xhci_hcd **) xhci->shared_hcd->hcd_priv) = xhci;

This makes me a little uneasy. Perhaps this should be an XHCI wrapper to
make it look less like you're doing something you shouldn't.

> +static int tegra_xhci_probe(struct platform_device *pdev)
> +{
> +	struct tegra_xhci_hcd *tegra;
> +	struct usb_hcd *hcd;
> +	struct resource	*res;

There's a tab between resource and *res which should be a space.

> +	struct phy *phy;
> +	const struct of_device_id *match;
> +	int ret, i, j, k;
> +
> +	BUILD_BUG_ON(sizeof(struct tegra_xhci_fw_cfgtbl) != 256);
> +
> +	tegra = devm_kzalloc(&pdev->dev, sizeof(*tegra), GFP_KERNEL);
> +	if (!tegra)
> +		return -ENOMEM;
> +	tegra->dev = &pdev->dev;
> +	platform_set_drvdata(pdev, tegra);
> +
> +	match = of_match_device(tegra_xhci_of_match, &pdev->dev);
> +	if (!match) {
> +		dev_err(&pdev->dev, "No device match found\n");
> +		return -ENODEV;
> +	}

I don't think this can happen. If there's no match in the table then
this function shouldn't have been called in the first place.

> +	tegra->soc_config = match->data;
> +
> +	/*
> +	 * Right now device-tree probed devices don't get dma_mask set.
> +	 * Since shared usb code relies on it, set it here for now.
> +	 * Once we have dma capability bindings this can go away.
> +	 */
> +	ret = dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> +	if (ret)
> +		return ret;

I think that's no longer necessary. of_dma_configure() should take care
of this now.

> +	k = 0;
> +	for (i = 0; i < ARRAY_SIZE(tegra_xhci_phy_types); i++) {

I think a more idiomatic way to write this would be:

	for (i = 0, k = 0; ...)

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH RESEND V4 2/9] mailbox: Add NVIDIA Tegra XUSB mailbox driver

[Thierry Reding]

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

On Tue, Oct 28, 2014 at 03:27:49PM -0700, Andrew Bresticker wrote:
[...]
> diff --git a/drivers/mailbox/tegra-xusb-mailbox.c b/drivers/mailbox/tegra-xusb-mailbox.c
[...]
> +struct tegra_xusb_mbox {
> +	struct mbox_controller mbox;
> +	int irq;

It seems like this is unused outside of tegra_xusb_mbox_probe()

> +static int tegra_xusb_mbox_send_data(struct mbox_chan *chan, void *data)
> +{
> +	struct tegra_xusb_mbox *mbox = dev_get_drvdata(chan->mbox->dev);

In my opinion, container_of(chan->mbox, struct tegra_xusb_mbox, mbox)
would be a slightly more idiomatic way to do this.

> +	struct tegra_xusb_mbox_msg *msg = data;
> +	unsigned long flags;
> +	u32 reg, owner;
> +
> +	dev_dbg(mbox->mbox.dev, "TX message 0x%x:0x%x\n", msg->cmd, msg->data);

%#x saves you a character in the format string above.

> +static int tegra_xusb_mbox_startup(struct mbox_chan *chan)
> +{
> +	struct tegra_xusb_mbox *mbox = dev_get_drvdata(chan->mbox->dev);
> +	int idx = chan - mbox->mbox.chans;

unsigned?

> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&mbox->lock, flags);
> +	mbox->vchan_allocated[idx] = true;
> +	spin_unlock_irqrestore(&mbox->lock, flags);

The struct mbox_chan has a con_priv field, perhaps that can be used to
store virtual channel private data instead of keeping the extra
vchan_allocated field in the controller private context? That way you
don't wouldn't have to compute an index and use it to index the field
in the controller private context.

In this particular case I don't think you even need that field, since a
channel is requested when the mbox_chan.cl field in non-NULL.

> +static struct mbox_chan_ops tegra_xusb_mbox_chan_ops = {

I think this really ought to be static const. That would currently still
throw a warning because the core doesn't mark the mbox_controller.ops
field const, but I think it really should. Now also seems like a good
time to make that change because there don't seem to be any users yet.

> +static irqreturn_t tegra_xusb_mbox_irq(int irq, void *p)
> +{
> +	struct tegra_xusb_mbox *mbox = (struct tegra_xusb_mbox *)p;

There's no need for the explicit cast here.

> +	struct tegra_xusb_mbox_msg msg;
> +	int i;

unsigned?

> +	u32 reg;
> +
> +	spin_lock(&mbox->lock);
> +
> +	/* Clear mbox interrupts */
> +	reg = mbox_readl(mbox, XUSB_CFG_ARU_SMI_INTR);
> +	if (reg & MBOX_SMI_INTR_FW_HANG)
> +		dev_err(mbox->mbox.dev, "Controller firmware hang\n");
> +	mbox_writel(mbox, reg, XUSB_CFG_ARU_SMI_INTR);
> +
> +	reg = mbox_readl(mbox, XUSB_CFG_ARU_MBOX_DATA_OUT);
> +	mbox_unpack_msg(reg, &msg);
> +
> +	/*
> +	 * Set the mailbox back to idle.  The recipient of the message is
> +	 * responsible for sending an ACK/NAK, if necessary.
> +	 */
> +	reg = mbox_readl(mbox, XUSB_CFG_ARU_MBOX_CMD);
> +	reg &= ~MBOX_DEST_SMI;
> +	mbox_writel(mbox, reg, XUSB_CFG_ARU_MBOX_CMD);
> +	mbox_writel(mbox, MBOX_OWNER_NONE, XUSB_CFG_ARU_MBOX_OWNER);
> +
> +	dev_dbg(mbox->mbox.dev, "RX message 0x%x:0x%x\n", msg.cmd, msg.data);

Again 0x%x -> %#x to save characters.

> +	for (i = 0; i < ARRAY_SIZE(mbox->vchan_allocated); i++) {
> +		if (mbox->vchan_allocated[i])
> +			mbox_chan_received_data(&mbox->mbox.chans[i], &msg);
> +	}

It seems like the only reason why you need to explicitly check for an
allocated channel is that mbox_chan_received_data() would otherwise
crash. Are mailbox drivers really supposed to keep track of whether a
channel has been requested by a client? Isn't that something that should
be done in the core?

> +static struct mbox_chan *tegra_xusb_mbox_of_xlate(struct mbox_controller *ctlr,
> +					const struct of_phandle_args *sp)
> +{
> +	struct tegra_xusb_mbox *mbox = dev_get_drvdata(ctlr->dev);

container_of()?

> +	struct mbox_chan *chan = NULL;
> +	unsigned long flags;
> +	int i;

unsigned?

> +static struct of_device_id tegra_xusb_mbox_of_match[] = {

static const, please.

> +static int tegra_xusb_mbox_probe(struct platform_device *pdev)
> +{
> +	struct tegra_xusb_mbox *mbox;
> +	struct resource *res;
> +	int ret;
> +
> +	mbox = devm_kzalloc(&pdev->dev, sizeof(*mbox), GFP_KERNEL);
> +	if (!mbox)
> +		return -ENOMEM;
> +	platform_set_drvdata(pdev, mbox);
> +	spin_lock_init(&mbox->lock);
> +
> +	mbox->mbox.dev = &pdev->dev;
> +	mbox->mbox.chans = devm_kcalloc(&pdev->dev, TEGRA_XUSB_MBOX_NUM_CHANS,
> +					sizeof(*mbox->mbox.chans), GFP_KERNEL);
> +	if (!mbox->mbox.chans)
> +		return -ENOMEM;
> +	mbox->mbox.num_chans = TEGRA_XUSB_MBOX_NUM_CHANS;
> +	mbox->mbox.ops = &tegra_xusb_mbox_chan_ops;
> +	mbox->mbox.txdone_poll = true;
> +	mbox->mbox.txpoll_period = 0; /* no need to actually poll */

Does the core perhaps need special handling for this? It seems like
poll_txdone() will always rearm the timer used to do the polling,
irrespective of whether the transfer is actually done or not. Maybe
something like this patch would be more correct in handling this:

diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
index afcb430508ec..85691a7d8ca6 100644
--- a/drivers/mailbox/mailbox.c
+++ b/drivers/mailbox/mailbox.c
@@ -117,10 +117,11 @@ static void poll_txdone(unsigned long data)
 		struct mbox_chan *chan = &mbox->chans[i];
 
 		if (chan->active_req && chan->cl) {
-			resched = true;
 			txdone = chan->mbox->ops->last_tx_done(chan);
 			if (txdone)
 				tx_tick(chan, 0);
+			else
+				resched = true;
 		}
 	}
 

The comment "no need to actually poll" isn't really appropriate in the
context of txpoll_period = 0.

> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res)
> +		return -ENODEV;
> +	mbox->regs = devm_ioremap(&pdev->dev, res->start, resource_size(res));
> +	if (!mbox->regs)
> +		return -ENOMEM;

This doesn't look right. Upon closer inspection, the reason why you
don't use devm_request_resource() is because these registers are shared
with the XHCI controller.

Perhaps a better design would be for the XHCI driver to expose the
mailbox rather than split it off into a separate driver.

> +static struct platform_driver tegra_xusb_mbox_driver = {
> +	.probe	= tegra_xusb_mbox_probe,
> +	.remove	= tegra_xusb_mbox_remove,
> +	.driver	= {
> +		.name = "tegra-xusb-mbox",
> +		.of_match_table = of_match_ptr(tegra_xusb_mbox_of_match),
> +	},

This uses tabs and spaces inconsistently for padding. I prefer a single
space on either side of the '=' like you've done for the fields of
.driver above.

> diff --git a/include/soc/tegra/xusb.h b/include/soc/tegra/xusb.h
> new file mode 100644
> index 0000000..cfe211d
> --- /dev/null
> +++ b/include/soc/tegra/xusb.h

Perhaps this should really be named xusb-mbox.h?

> @@ -0,0 +1,46 @@
> +/*
> + * Copyright (C) 2014 NVIDIA Corporation
> + * Copyright (C) 2014 Google, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + */
> +
> +#ifndef __SOC_TEGRA_XUSB_H__
> +#define __SOC_TEGRA_XUSB_H__
> +
> +/* Two virtual channels: host + phy */
> +#define TEGRA_XUSB_MBOX_NUM_CHANS 2

I don't think this belongs in this public header. Who is going to need
this besides the XUSB mailbox driver?

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH RESEND V4 4/9] pinctrl: tegra-xusb: Add USB PHY support

[Thierry Reding]

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

On Tue, Oct 28, 2014 at 03:27:51PM -0700, Andrew Bresticker wrote:
[...]
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index c6a66de..0f4cdef 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -163,6 +163,7 @@ config PINCTRL_TEGRA_XUSB
>  	select GENERIC_PHY
>  	select PINCONF
>  	select PINMUX
> +	select MAILBOX

I think this should be a "depends on" because we use the mailbox API as
a client rather than a provider.

> diff --git a/drivers/pinctrl/pinctrl-tegra-xusb.c b/drivers/pinctrl/pinctrl-tegra-xusb.c
[...]
>  struct tegra_xusb_padctl_function {
>  	const char *name;
>  	const char * const *groups;
> @@ -72,6 +222,16 @@ struct tegra_xusb_padctl_soc {
>  
>  	const struct tegra_xusb_padctl_lane *lanes;
>  	unsigned int num_lanes;
> +
> +	u32 rx_wander;
> +	u32 rx_eq;
> +	u32 cdr_cntl;
> +	u32 dfe_cntl;
> +	u32 hs_slew;
> +	u32 ls_rslew[TEGRA_XUSB_UTMI_PHYS];
> +	u32 hs_discon_level;
> +	u32 spare_in;
> +	int hsic_port_offset;

unsigned int? Are these values all SoC-specific or can they vary per
board?

> +struct tegra_xusb_fuse_calibration {
> +	u32 hs_curr_level[TEGRA_XUSB_UTMI_PHYS];
> +	u32 hs_iref_cap;
> +	u32 hs_term_range_adj;
> +	u32 hs_squelch_level;
> +};
> +
> +struct tegra_xusb_usb3_port {
> +	int lane;

unsigned

> +	bool context_saved;
> +	u32 tap1_val;
> +	u32 amp_val;
> +	u32 ctle_z_val;
> +	u32 ctle_g_val;
> +};
> +
[...]
> +static inline bool is_otg_lane(unsigned int lane)
> +{
> +	return lane >= TEGRA_XUSB_PADCTL_PIN_OTG_0 &&
> +		lane <= TEGRA_XUSB_PADCTL_PIN_OTG_2;
> +}
> +
> +static inline bool is_hsic_lane(unsigned int lane)
> +{
> +	return lane >= TEGRA_XUSB_PADCTL_PIN_HSIC_0 &&
> +		lane <= TEGRA_XUSB_PADCTL_PIN_HSIC_1;
> +}
> +
> +static inline bool is_pcie_or_sata_lane(unsigned int lane)
> +{
> +	return lane >= TEGRA_XUSB_PADCTL_PIN_PCIE_0 &&
> +		lane <= TEGRA_XUSB_PADCTL_PIN_SATA_0;
> +}
> +
> +static int lane_to_usb3_port(struct tegra_xusb_padctl *padctl,
> +			     unsigned int lane)
> +{
> +	int i;

unsigned

> +
> +	for (i = 0; i < TEGRA_XUSB_USB3_PHYS; i++) {
> +		if (padctl->usb3_ports[i].lane == lane)
> +			return i;
> +	}
> +
> +	return -1;
> +}

Why not return a proper error code here that callers can simply
propagate?

Also, for consistency, I'd prefer the is_*_lane() functions to be
renamed to lane_is_*().

> @@ -321,6 +561,7 @@ static int tegra_xusb_padctl_pinconf_group_get(struct pinctrl_dev *pinctrl,
>  	struct tegra_xusb_padctl *padctl = pinctrl_dev_get_drvdata(pinctrl);
>  	const struct tegra_xusb_padctl_lane *lane;
>  	enum tegra_xusb_padctl_param param;
> +	int port;
>  	u32 value;

The variable here were sorted in inverse christmas tree order, so port
should be below value.

> +static int usb3_phy_to_port(struct phy *phy)
> +{
> +	struct tegra_xusb_padctl *padctl = phy_get_drvdata(phy);
> +	int i;

unsigned

> +
> +	for (i = 0; i < TEGRA_XUSB_USB3_PHYS; i++) {
> +		if (phy == padctl->phys[TEGRA_XUSB_PADCTL_USB3_P0 + i])
> +			break;

You could simply return i here and then BUG_ON unconditionally.

> +	}
> +	BUG_ON(i == TEGRA_XUSB_USB3_PHYS);
> +
> +	return i;
> +}

Actually, thinking about it some more, perhaps making this a WARN_ON()
and returning an error so that we can continue and propagate the error
would be more useful. BUG_ON() will completely hang the kernel with no
way out but rebooting. WARN_ON() will give a hint about something being
wrong and returning an error will allow the kernel to continue to run,
which might be the only way to diagnose and fix the problem, even if it
means that USB 3.0 support will be disabled.

> +static void usb3_phy_save_context(struct tegra_xusb_padctl *padctl, int port)

unsigned for port...

> +{
> +	int lane = padctl->usb3_ports[port].lane;

... and lane.

> +	u32 value, offset;
> +
> +	padctl->usb3_ports[port].context_saved = true;

What's the purpose of saving the context here? This seems to be
triggered by a request from XUSB, but it's then restored when the PHY is
powered on. How does that even happen? Won't the PHY stay powered all
the time? Or shouldn't the context be saved when powering off the PHY?

> +static int utmi_phy_to_port(struct phy *phy)
> +{
> +	struct tegra_xusb_padctl *padctl = phy_get_drvdata(phy);
> +	int i;
> +
> +	for (i = 0; i < TEGRA_XUSB_UTMI_PHYS; i++) {
> +		if (phy == padctl->phys[TEGRA_XUSB_PADCTL_UTMI_P0 + i])
> +			break;
> +	}
> +	BUG_ON(i == TEGRA_XUSB_UTMI_PHYS);
> +
> +	return i;
> +}

Same comment as before.

> +static int utmi_phy_power_on(struct phy *phy)
> +{
> +	struct tegra_xusb_padctl *padctl = phy_get_drvdata(phy);
> +	int port = utmi_phy_to_port(phy);
> +	int ret;

The driver uses err as the name for variables that store error codes.
I'd like to remain consistent with that.

> +static int hsic_phy_to_port(struct phy *phy)
> +{
> +	struct tegra_xusb_padctl *padctl = phy_get_drvdata(phy);
> +	int i;
> +
> +	for (i = 0; i < TEGRA_XUSB_HSIC_PHYS; i++) {
> +		if (phy == padctl->phys[TEGRA_XUSB_PADCTL_HSIC_P0 + i])
> +			break;
> +	}
> +	BUG_ON(i == TEGRA_XUSB_HSIC_PHYS);
> +
> +	return i;
> +}

Again, as mentioned before.

> +static int hsic_phy_power_on(struct phy *phy)
> +{
> +	struct tegra_xusb_padctl *padctl = phy_get_drvdata(phy);
> +	int port = hsic_phy_to_port(phy);
> +	int ret;
> +	u32 value;
> +
> +	ret = regulator_enable(padctl->vddio_hsic);
> +	if (ret)
> +		return ret;
> +
> +	value = padctl_readl(padctl, XUSB_PADCTL_HSIC_PADX_CTL1(port));
> +	value &= ~(XUSB_PADCTL_HSIC_PAD_CTL1_RPD_STROBE |
> +		   XUSB_PADCTL_HSIC_PAD_CTL1_RPU_DATA |
> +		   XUSB_PADCTL_HSIC_PAD_CTL1_PD_RX |
> +		   XUSB_PADCTL_HSIC_PAD_CTL1_PD_ZI |
> +		   XUSB_PADCTL_HSIC_PAD_CTL1_PD_TRX |
> +		   XUSB_PADCTL_HSIC_PAD_CTL1_PD_TX);
> +	value |= XUSB_PADCTL_HSIC_PAD_CTL1_RPD_DATA |
> +		 XUSB_PADCTL_HSIC_PAD_CTL1_RPU_STROBE;
> +	padctl_writel(padctl, value, XUSB_PADCTL_HSIC_PADX_CTL1(port));
> +
> +	return 0;
> +}
> +
> +static int hsic_phy_power_off(struct phy *phy)
> +{
> +	struct tegra_xusb_padctl *padctl = phy_get_drvdata(phy);
> +	int port = hsic_phy_to_port(phy);
> +	u32 value;
> +
> +	regulator_disable(padctl->vddio_hsic);

It probably doesn't make much of a difference, but the sequence should
be the reverse of the power-on sequence, so regulator_disable() should
be last in this function.

> +static void hsic_phy_set_idle(struct tegra_xusb_padctl *padctl, int port,
> +			      bool set)

unsigned int for port. Also maybe rename to set to idle, which makes the
code below somewhat easier to read.

> +{
> +	u32 value;
> +
> +	value = padctl_readl(padctl, XUSB_PADCTL_HSIC_PADX_CTL1(port));
> +	if (set)
> +		value |= XUSB_PADCTL_HSIC_PAD_CTL1_RPD_DATA |
> +			 XUSB_PADCTL_HSIC_PAD_CTL1_RPU_STROBE;
> +	else
> +		value &= ~(XUSB_PADCTL_HSIC_PAD_CTL1_RPD_DATA |
> +			   XUSB_PADCTL_HSIC_PAD_CTL1_RPU_STROBE);
> +	padctl_writel(padctl, value, XUSB_PADCTL_HSIC_PADX_CTL1(port));
> +}
> +
> +static bool is_phy_mbox_message(u32 cmd)
> +{
> +	switch (cmd) {
> +	case MBOX_CMD_SAVE_DFE_CTLE_CTX:
> +	case MBOX_CMD_START_HSIC_IDLE:
> +	case MBOX_CMD_STOP_HSIC_IDLE:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}

This is oddly placed. It's only called by tegra_xusb_phy_mbox_rx() below
so would be better placed right in front of it.

> +static void tegra_xusb_phy_mbox_work(struct work_struct *work)
> +{
> +	struct tegra_xusb_padctl *padctl = container_of(work,
> +				struct tegra_xusb_padctl, mbox_req_work);

Maybe wrap this into a static inline function for readability?

> +	struct tegra_xusb_mbox_msg *msg = &padctl->mbox_req;
> +	struct tegra_xusb_mbox_msg resp;
> +	u32 ports;
> +	int i;

unsigned. There are other occurrences where unsigned makes more sense,
even if I haven't explicitly mentioned them anymore.

> +	resp.cmd = 0;
> +	switch (msg->cmd) {
> +	case MBOX_CMD_SAVE_DFE_CTLE_CTX:
> +		resp.data = msg->data;
> +		if (msg->data > TEGRA_XUSB_USB3_PHYS) {
> +			resp.cmd = MBOX_CMD_NAK;
> +		} else {
> +			usb3_phy_save_context(padctl, msg->data);
> +			resp.cmd = MBOX_CMD_ACK;
> +		}
> +		break;

Perhaps let usb3_phy_save_context() determine validity of the parameter
and return an error otherwise (or for any other failure)? Then you can
set the command to ACK or NAK depending on the return value.

> -#define PIN_OTG_0   0
> -#define PIN_OTG_1   1
> -#define PIN_OTG_2   2
> -#define PIN_ULPI_0  3
> -#define PIN_HSIC_0  4
> -#define PIN_HSIC_1  5
> -#define PIN_PCIE_0  6
> -#define PIN_PCIE_1  7
> -#define PIN_PCIE_2  8
> -#define PIN_PCIE_3  9
> -#define PIN_PCIE_4 10
> -#define PIN_SATA_0 11

I really don't think we should export these, unless we can't make it
work otherwise.

>  static int tegra_xusb_padctl_probe(struct platform_device *pdev)
>  {
>  	struct tegra_xusb_padctl *padctl;
>  	const struct of_device_id *match;
>  	struct resource *res;
>  	struct phy *phy;
> -	int err;
> +	int err, i;
>  
>  	padctl = devm_kzalloc(&pdev->dev, sizeof(*padctl), GFP_KERNEL);
>  	if (!padctl)
> @@ -888,6 +1980,10 @@ static int tegra_xusb_padctl_probe(struct platform_device *pdev)
>  	if (IS_ERR(padctl->regs))
>  		return PTR_ERR(padctl->regs);
>  
> +	err = tegra_xusb_read_fuse_calibration(padctl);
> +	if (err < 0)
> +		return err;
> +
>  	padctl->rst = devm_reset_control_get(&pdev->dev, NULL);
>  	if (IS_ERR(padctl->rst))
>  		return PTR_ERR(padctl->rst);
> @@ -896,6 +1992,24 @@ static int tegra_xusb_padctl_probe(struct platform_device *pdev)
>  	if (err < 0)
>  		return err;
>  
> +	for (i = 0; i < TEGRA_XUSB_USB3_PHYS; i++) {
> +		char prop[sizeof("nvidia,usb3-port-N-lane")];
> +		u32 lane;
> +
> +		sprintf(prop, "nvidia,usb3-port-%d-lane", i);

Like I mentioned while reviewing the binding changes, I think it'd be
better to reverse this and put a property to set this into the pinmux
nodes for the USB3 related pins, similar to the nvidia,usb2-port
property.

That should allow us to avoid these nasty dynamically generated property
names.

> @@ -936,6 +2098,18 @@ static int tegra_xusb_padctl_probe(struct platform_device *pdev)
>  		goto unregister;
>  	}
>  
> +	INIT_WORK(&padctl->mbox_req_work, tegra_xusb_phy_mbox_work);
> +	padctl->mbox_client.dev = &pdev->dev;
> +	padctl->mbox_client.tx_block = true;
> +	padctl->mbox_client.tx_tout = 0;
> +	padctl->mbox_client.rx_callback = tegra_xusb_phy_mbox_rx;
> +	padctl->mbox_chan = mbox_request_channel(&padctl->mbox_client, 0);
> +	if (IS_ERR(padctl->mbox_chan)) {
> +		err = PTR_ERR(padctl->mbox_chan);
> +		dev_err(&pdev->dev, "failed to request mailbox: %d\n", err);
> +		goto unregister;
> +	}

I think this should be done before the registering the PHY provider so
that we don't expose one (even for only a very short time) before we
haven't made sure that it can be used.

Also, this effectively makes the mailbox mandatory, which means that the
above code is going to break on older DTBs. So I think we have no choice
but to make mailbox (and hence XUSB) support optional.

>  	return 0;
>  
>  unregister:
> @@ -950,6 +2124,9 @@ static int tegra_xusb_padctl_remove(struct platform_device *pdev)
>  	struct tegra_xusb_padctl *padctl = platform_get_drvdata(pdev);
>  	int err;
>  
> +	cancel_work_sync(&padctl->mbox_req_work);
> +	mbox_free_channel(padctl->mbox_chan);
> +
>  	pinctrl_unregister(padctl->pinctrl);
>  
>  	err = reset_control_assert(padctl->rst);
> diff --git a/include/soc/tegra/xusb.h b/include/soc/tegra/xusb.h
> index cfe211d..149434f 100644
> --- a/include/soc/tegra/xusb.h
> +++ b/include/soc/tegra/xusb.h
> @@ -10,6 +10,13 @@
>  #ifndef __SOC_TEGRA_XUSB_H__
>  #define __SOC_TEGRA_XUSB_H__
>  
> +#define TEGRA_XUSB_USB3_PHYS 2
> +#define TEGRA_XUSB_UTMI_PHYS 3
> +#define TEGRA_XUSB_HSIC_PHYS 2
> +#define TEGRA_XUSB_NUM_USB_PHYS (TEGRA_XUSB_USB3_PHYS + TEGRA_XUSB_UTMI_PHYS + \
> +				 TEGRA_XUSB_HSIC_PHYS)
> +#define TEGRA_XUSB_NUM_PHYS (TEGRA_XUSB_NUM_USB_PHYS + 2) /* + SATA & PCIe */

These are really XUSB pad controller specific defines, why does anyone
else need to know this?

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH RESEND V4 5/9] of: Add NVIDIA Tegra xHCI controller binding

[Andrew Bresticker]

On Wed, Oct 29, 2014 at 2:43 AM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> On Tue, Oct 28, 2014 at 03:27:50PM -0700, Andrew Bresticker wrote:
> [...]
>> diff --git a/Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-xusb-padctl.txt b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-xusb-padctl.txt
> [...]
>> +Optional properties:
>> +-------------------
>> +- vbus-{0,1,2}-supply: VBUS regulator for the corresponding UTMI pad.
>> +- vddio-hsic-supply: VDDIO regulator for the HSIC pads.
>> +- nvidia,usb3-port-{0,1}-lane: PCIe/SATA lane to which the corresponding USB3
>> +  port is mapped.  See <dt-bindings/pinctrl/pinctrl-tegra-xusb.h> for the list
>> +  of valid values.
>
> I dislike how we now need to provide a list of all pins in the header
> file, where previously we used strings for this. This could become very
> ugly if the set of pins changes in future generations of this IP block.
>
> Could we instead derive this from the pinmux nodes? For example you have
> this in the example below:
>
>         usb3p0 {
>                 nvidia,lanes = "pcie-0";
>                 ...
>         };
>
> Perhaps what we need is to either key off the node name or add another
> property, such as:
>
>         nvidia,usb3-port = <0>;
>
> This would match the nvidia,usb2-port property that you've added below.

That is actually how I described the USB3 port to SS lane mapping
originally, but in review of an earlier version of this series,
Stephen suggested that I make it a separate, not pinconfig property
since it wasn't a value written directly to the hardware.  I'm fine
with changing it back as the pinconfig property makes more sense to me
as well.

Re: [PATCH RESEND V4 2/9] mailbox: Add NVIDIA Tegra XUSB mailbox driver

[Andrew Bresticker]

>> +     for (i = 0; i < ARRAY_SIZE(mbox->vchan_allocated); i++) {
>> +             if (mbox->vchan_allocated[i])
>> +                     mbox_chan_received_data(&mbox->mbox.chans[i], &msg);
>> +     }
>
> It seems like the only reason why you need to explicitly check for an
> allocated channel is that mbox_chan_received_data() would otherwise
> crash. Are mailbox drivers really supposed to keep track of whether a
> channel has been requested by a client? Isn't that something that should
> be done in the core?

Yeah, I'd agree that this is something that should be handled by the core.

>> +static int tegra_xusb_mbox_probe(struct platform_device *pdev)
>> +{
>> +     struct tegra_xusb_mbox *mbox;
>> +     struct resource *res;
>> +     int ret;
>> +
>> +     mbox = devm_kzalloc(&pdev->dev, sizeof(*mbox), GFP_KERNEL);
>> +     if (!mbox)
>> +             return -ENOMEM;
>> +     platform_set_drvdata(pdev, mbox);
>> +     spin_lock_init(&mbox->lock);
>> +
>> +     mbox->mbox.dev = &pdev->dev;
>> +     mbox->mbox.chans = devm_kcalloc(&pdev->dev, TEGRA_XUSB_MBOX_NUM_CHANS,
>> +                                     sizeof(*mbox->mbox.chans), GFP_KERNEL);
>> +     if (!mbox->mbox.chans)
>> +             return -ENOMEM;
>> +     mbox->mbox.num_chans = TEGRA_XUSB_MBOX_NUM_CHANS;
>> +     mbox->mbox.ops = &tegra_xusb_mbox_chan_ops;
>> +     mbox->mbox.txdone_poll = true;
>> +     mbox->mbox.txpoll_period = 0; /* no need to actually poll */
>
> Does the core perhaps need special handling for this? It seems like
> poll_txdone() will always rearm the timer used to do the polling,
> irrespective of whether the transfer is actually done or not.

Yeah, that doesn't seem quite right...

> Maybe something like this patch would be more correct in handling
> this:
>
> diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
> index afcb430508ec..85691a7d8ca6 100644
> --- a/drivers/mailbox/mailbox.c
> +++ b/drivers/mailbox/mailbox.c
> @@ -117,10 +117,11 @@ static void poll_txdone(unsigned long data)
>                 struct mbox_chan *chan = &mbox->chans[i];
>
>                 if (chan->active_req && chan->cl) {
> -                       resched = true;
>                         txdone = chan->mbox->ops->last_tx_done(chan);
>                         if (txdone)
>                                 tx_tick(chan, 0);
> +                       else
> +                               resched = true;
>                 }
>         }

... but we still need to re-arm the timer if tx_tick() submits another
message.  Perhaps the better thing to do is to have msg_submit() arm
the timer.

>> +     res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +     if (!res)
>> +             return -ENODEV;
>> +     mbox->regs = devm_ioremap(&pdev->dev, res->start, resource_size(res));
>> +     if (!mbox->regs)
>> +             return -ENOMEM;
>
> This doesn't look right. Upon closer inspection, the reason why you
> don't use devm_request_resource() is because these registers are shared
> with the XHCI controller.
>
> Perhaps a better design would be for the XHCI driver to expose the
> mailbox rather than split it off into a separate driver.

Well that's what I had originally, but then it was suggested I make it
a separate driver.

Stephen also brought this up during review and suggested that some
sort of MFD would be the best way to structure this, but was fine with
the way I have it now.  I can move this driver around (again) if you
feel that strongly about it...

>> diff --git a/include/soc/tegra/xusb.h b/include/soc/tegra/xusb.h
>> new file mode 100644
>> index 0000000..cfe211d
>> --- /dev/null
>> +++ b/include/soc/tegra/xusb.h
>
> Perhaps this should really be named xusb-mbox.h?

I'd prefer to leave it as xusb.h so that any other XUSB-related
definitions can be left here.

Re: [PATCH RESEND V4 4/9] pinctrl: tegra-xusb: Add USB PHY support

[Andrew Bresticker]

>> diff --git a/drivers/pinctrl/pinctrl-tegra-xusb.c b/drivers/pinctrl/pinctrl-tegra-xusb.c
> [...]
>>  struct tegra_xusb_padctl_function {
>>       const char *name;
>>       const char * const *groups;
>> @@ -72,6 +222,16 @@ struct tegra_xusb_padctl_soc {
>>
>>       const struct tegra_xusb_padctl_lane *lanes;
>>       unsigned int num_lanes;
>> +
>> +     u32 rx_wander;
>> +     u32 rx_eq;
>> +     u32 cdr_cntl;
>> +     u32 dfe_cntl;
>> +     u32 hs_slew;
>> +     u32 ls_rslew[TEGRA_XUSB_UTMI_PHYS];
>> +     u32 hs_discon_level;
>> +     u32 spare_in;
>> +     int hsic_port_offset;
>
> unsigned int? Are these values all SoC-specific or can they vary per
> board?

Yes, all the members I added to struct tegra_xusb_pactl_soc are SoC-specific.

>> +
>> +     for (i = 0; i < TEGRA_XUSB_USB3_PHYS; i++) {
>> +             if (phy == padctl->phys[TEGRA_XUSB_PADCTL_USB3_P0 + i])
>> +                     break;
>
> You could simply return i here and then BUG_ON unconditionally.
>
>> +     }
>> +     BUG_ON(i == TEGRA_XUSB_USB3_PHYS);
>> +
>> +     return i;
>> +}
>
> Actually, thinking about it some more, perhaps making this a WARN_ON()
> and returning an error so that we can continue and propagate the error
> would be more useful. BUG_ON() will completely hang the kernel with no
> way out but rebooting. WARN_ON() will give a hint about something being
> wrong and returning an error will allow the kernel to continue to run,
> which might be the only way to diagnose and fix the problem, even if it
> means that USB 3.0 support will be disabled.

I felt like BUG_ON is more appropriate here.  Hitting this case means
there's a bug in the PHY core or a driver has passed a bogus pointer
and the stack dump produced by the BUG_ON should make it obvious as to
what the issue is.  I don't feel too strongly about it though.

>> +     u32 value, offset;
>> +
>> +     padctl->usb3_ports[port].context_saved = true;
>
> What's the purpose of saving the context here? This seems to be
> triggered by a request from XUSB, but it's then restored when the PHY is
> powered on. How does that even happen? Won't the PHY stay powered all
> the time? Or shouldn't the context be saved when powering off the PHY?

Right, context is saved when requested by the XUSB controller and
restored on power on.  This is used during runtime power-gating or LP0
where the PHYs are powered off and on and this context is lost.
Neither of these are currently implemented by the host driver,
however.

As far as why the context is saved upon request and not at power off,
I'm not sure.  I've observed that these messages come in when a USB3.0
device is enumerated.  Perhaps the XUSB controller is doing some sort
of tuning.

>> @@ -936,6 +2098,18 @@ static int tegra_xusb_padctl_probe(struct platform_device *pdev)
>>               goto unregister;
>>       }
>>
>> +     INIT_WORK(&padctl->mbox_req_work, tegra_xusb_phy_mbox_work);
>> +     padctl->mbox_client.dev = &pdev->dev;
>> +     padctl->mbox_client.tx_block = true;
>> +     padctl->mbox_client.tx_tout = 0;
>> +     padctl->mbox_client.rx_callback = tegra_xusb_phy_mbox_rx;
>> +     padctl->mbox_chan = mbox_request_channel(&padctl->mbox_client, 0);
>> +     if (IS_ERR(padctl->mbox_chan)) {
>> +             err = PTR_ERR(padctl->mbox_chan);
>> +             dev_err(&pdev->dev, "failed to request mailbox: %d\n", err);
>> +             goto unregister;
>> +     }
>
> I think this should be done before the registering the PHY provider so
> that we don't expose one (even for only a very short time) before we
> haven't made sure that it can be used.
>
> Also, this effectively makes the mailbox mandatory, which means that the
> above code is going to break on older DTBs. So I think we have no choice
> but to make mailbox (and hence XUSB) support optional.

I understand the need for binding stability, but it's not like these
bindings have been around for very long (a release or two?) and this
series has existed for almost the same amount of time.  Are there
really any DTBs out there that are going to break because of this?

>> diff --git a/include/soc/tegra/xusb.h b/include/soc/tegra/xusb.h
>> index cfe211d..149434f 100644
>> --- a/include/soc/tegra/xusb.h
>> +++ b/include/soc/tegra/xusb.h
>> @@ -10,6 +10,13 @@
>>  #ifndef __SOC_TEGRA_XUSB_H__
>>  #define __SOC_TEGRA_XUSB_H__
>>
>> +#define TEGRA_XUSB_USB3_PHYS 2
>> +#define TEGRA_XUSB_UTMI_PHYS 3
>> +#define TEGRA_XUSB_HSIC_PHYS 2
>> +#define TEGRA_XUSB_NUM_USB_PHYS (TEGRA_XUSB_USB3_PHYS + TEGRA_XUSB_UTMI_PHYS + \
>> +                              TEGRA_XUSB_HSIC_PHYS)
>> +#define TEGRA_XUSB_NUM_PHYS (TEGRA_XUSB_NUM_USB_PHYS + 2) /* + SATA & PCIe */
>
> These are really XUSB pad controller specific defines, why does anyone
> else need to know this?

They're not pad controller specific.  They're also used in the xHCI host driver.

Re: [PATCH RESEND V4 2/9] mailbox: Add NVIDIA Tegra XUSB mailbox driver

[Thierry Reding]

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

On Wed, Oct 29, 2014 at 11:02:36AM -0700, Andrew Bresticker wrote:
[...]
> > Maybe something like this patch would be more correct in handling
> > this:
> >
> > diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
> > index afcb430508ec..85691a7d8ca6 100644
> > --- a/drivers/mailbox/mailbox.c
> > +++ b/drivers/mailbox/mailbox.c
> > @@ -117,10 +117,11 @@ static void poll_txdone(unsigned long data)
> >                 struct mbox_chan *chan = &mbox->chans[i];
> >
> >                 if (chan->active_req && chan->cl) {
> > -                       resched = true;
> >                         txdone = chan->mbox->ops->last_tx_done(chan);
> >                         if (txdone)
> >                                 tx_tick(chan, 0);
> > +                       else
> > +                               resched = true;
> >                 }
> >         }
> 
> ... but we still need to re-arm the timer if tx_tick() submits another
> message.  Perhaps the better thing to do is to have msg_submit() arm
> the timer.

I think we need both. If the last transmission isn't done yet we still
want to keep polling. And we also want to poll if a new message is sent
subsequently.

Perhaps it would be as easy as moving the poll handling code from
mbox_send_message() (if (chan->txdone_method == TXDONE_BY_POLL)) into
msg_submit()? That has the additional advantage of being able to omit
the polling when an error happens during the mbox' .send_data().

> >> +     res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >> +     if (!res)
> >> +             return -ENODEV;
> >> +     mbox->regs = devm_ioremap(&pdev->dev, res->start, resource_size(res));
> >> +     if (!mbox->regs)
> >> +             return -ENOMEM;
> >
> > This doesn't look right. Upon closer inspection, the reason why you
> > don't use devm_request_resource() is because these registers are shared
> > with the XHCI controller.
> >
> > Perhaps a better design would be for the XHCI driver to expose the
> > mailbox rather than split it off into a separate driver.
> 
> Well that's what I had originally, but then it was suggested I make it
> a separate driver.
> 
> Stephen also brought this up during review and suggested that some
> sort of MFD would be the best way to structure this, but was fine with
> the way I have it now.  I can move this driver around (again) if you
> feel that strongly about it...

We've had this discussion only recently about the memory controller
driver. It used to be that there were separate drivers for the memory
controller part and the IOMMU part. But that resulted in hilarious DT
bindings. Granted, most of the issues had to do with unfortunate inter-
leaving of register regions, but generally I think there's nothing wrong
with exposing multiple interfaces from a single driver.

The downside of course is that you have to choose which subsystem is the
primary one and then get it merged via that tree. There's also a problem
in that the driver is now in a different directory than the others, so a
subsystem-wide change may not notice the "out-of-place" driver. But I
think we have pretty good tools to help with this type of thing.

Also, managing all the resources (regulators, clocks, resets, ...) that
a hardware block requires is much easier to do in a single driver than
spread over several. MFD could be an option somewhere halfway between
the two but also has some downsides. Most importantly it isn't going to
scale in the long run.

> >> diff --git a/include/soc/tegra/xusb.h b/include/soc/tegra/xusb.h
> >> new file mode 100644
> >> index 0000000..cfe211d
> >> --- /dev/null
> >> +++ b/include/soc/tegra/xusb.h
> >
> > Perhaps this should really be named xusb-mbox.h?
> 
> I'd prefer to leave it as xusb.h so that any other XUSB-related
> definitions can be left here.

Okay, that makes sense given that the mbox really is part of the larger
XUSB block.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH RESEND V4 4/9] pinctrl: tegra-xusb: Add USB PHY support

[Thierry Reding]

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

On Wed, Oct 29, 2014 at 12:43:36PM -0700, Andrew Bresticker wrote:
> >> diff --git a/drivers/pinctrl/pinctrl-tegra-xusb.c b/drivers/pinctrl/pinctrl-tegra-xusb.c
[...]
> >> +
> >> +     for (i = 0; i < TEGRA_XUSB_USB3_PHYS; i++) {
> >> +             if (phy == padctl->phys[TEGRA_XUSB_PADCTL_USB3_P0 + i])
> >> +                     break;
> >
> > You could simply return i here and then BUG_ON unconditionally.
> >
> >> +     }
> >> +     BUG_ON(i == TEGRA_XUSB_USB3_PHYS);
> >> +
> >> +     return i;
> >> +}
> >
> > Actually, thinking about it some more, perhaps making this a WARN_ON()
> > and returning an error so that we can continue and propagate the error
> > would be more useful. BUG_ON() will completely hang the kernel with no
> > way out but rebooting. WARN_ON() will give a hint about something being
> > wrong and returning an error will allow the kernel to continue to run,
> > which might be the only way to diagnose and fix the problem, even if it
> > means that USB 3.0 support will be disabled.
> 
> I felt like BUG_ON is more appropriate here.  Hitting this case means
> there's a bug in the PHY core or a driver has passed a bogus pointer
> and the stack dump produced by the BUG_ON should make it obvious as to
> what the issue is.  I don't feel too strongly about it though.

The problem with BUG_ON() is that you won't be able to go any further.
So if this were to happen on a device with no serial you might not even
get to a point where you actually see an error message. Handling this
more gracefully by propagating the error code and failing .probe() does
not seem overly complicated and the WARN_ON() output will hopefully
still be noticed (it probably will be after the user can't get USB to
work).

Consider for example the case where a user has only one device to test
and report bugs on. If we crash the device using BUG_ON() they may not
be able to report a bug at all (or recover by reverting to some known
good kernel version). A WARN_ON() will hopefully be enough to get
noticed and unless users rely on XUSB for the root filesystem they'd
still be able to open up a web browser and file a bug report with the
oops attached.

> >> +     u32 value, offset;
> >> +
> >> +     padctl->usb3_ports[port].context_saved = true;
> >
> > What's the purpose of saving the context here? This seems to be
> > triggered by a request from XUSB, but it's then restored when the PHY is
> > powered on. How does that even happen? Won't the PHY stay powered all
> > the time? Or shouldn't the context be saved when powering off the PHY?
> 
> Right, context is saved when requested by the XUSB controller and
> restored on power on.  This is used during runtime power-gating or LP0
> where the PHYs are powered off and on and this context is lost.
> Neither of these are currently implemented by the host driver,
> however.
> 
> As far as why the context is saved upon request and not at power off,
> I'm not sure.  I've observed that these messages come in when a USB3.0
> device is enumerated.  Perhaps the XUSB controller is doing some sort
> of tuning.

I see. Perhaps these values are calibrated by the firmware? In that case
I guess it could redo the calibration. I'll see if I can find out why it
is necessary to store this.

> 
> >> @@ -936,6 +2098,18 @@ static int tegra_xusb_padctl_probe(struct platform_device *pdev)
> >>               goto unregister;
> >>       }
> >>
> >> +     INIT_WORK(&padctl->mbox_req_work, tegra_xusb_phy_mbox_work);
> >> +     padctl->mbox_client.dev = &pdev->dev;
> >> +     padctl->mbox_client.tx_block = true;
> >> +     padctl->mbox_client.tx_tout = 0;
> >> +     padctl->mbox_client.rx_callback = tegra_xusb_phy_mbox_rx;
> >> +     padctl->mbox_chan = mbox_request_channel(&padctl->mbox_client, 0);
> >> +     if (IS_ERR(padctl->mbox_chan)) {
> >> +             err = PTR_ERR(padctl->mbox_chan);
> >> +             dev_err(&pdev->dev, "failed to request mailbox: %d\n", err);
> >> +             goto unregister;
> >> +     }
> >
> > I think this should be done before the registering the PHY provider so
> > that we don't expose one (even for only a very short time) before we
> > haven't made sure that it can be used.
> >
> > Also, this effectively makes the mailbox mandatory, which means that the
> > above code is going to break on older DTBs. So I think we have no choice
> > but to make mailbox (and hence XUSB) support optional.
> 
> I understand the need for binding stability, but it's not like these
> bindings have been around for very long (a release or two?) and this
> series has existed for almost the same amount of time.  Are there
> really any DTBs out there that are going to break because of this?

Every DTB created from a kernel version that has the original binding
but not the one modified as part of this series is going to break. Last
time I checked there weren't any exceptions to this rule. Note, though,
that the rule is that existing functionality must not break. That is,
SATA and PCIe should remain functional, so it should be fine if you just
don't register any of the USB PHYs when the request for a mailbox
channel fails. Something along these lines should do it:

	padctl->mbox_chan = mbox_request_channel(...);
	if (!IS_ERR(padctl->mbox_chan)) {
		err = tegra_xusb_padctl_setup_usb(...);
		...
	}

> >> diff --git a/include/soc/tegra/xusb.h b/include/soc/tegra/xusb.h
> >> index cfe211d..149434f 100644
> >> --- a/include/soc/tegra/xusb.h
> >> +++ b/include/soc/tegra/xusb.h
> >> @@ -10,6 +10,13 @@
> >>  #ifndef __SOC_TEGRA_XUSB_H__
> >>  #define __SOC_TEGRA_XUSB_H__
> >>
> >> +#define TEGRA_XUSB_USB3_PHYS 2
> >> +#define TEGRA_XUSB_UTMI_PHYS 3
> >> +#define TEGRA_XUSB_HSIC_PHYS 2
> >> +#define TEGRA_XUSB_NUM_USB_PHYS (TEGRA_XUSB_USB3_PHYS + TEGRA_XUSB_UTMI_PHYS + \
> >> +                              TEGRA_XUSB_HSIC_PHYS)
> >> +#define TEGRA_XUSB_NUM_PHYS (TEGRA_XUSB_NUM_USB_PHYS + 2) /* + SATA & PCIe */
> >
> > These are really XUSB pad controller specific defines, why does anyone
> > else need to know this?
> 
> They're not pad controller specific.  They're also used in the xHCI host driver.

I keep thinking that there should be a way around this. Of course if
both the XHCI and mailbox drivers were merged, then there'd be no need
to expose this publicly at all.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH RESEND V4 5/9] of: Add NVIDIA Tegra xHCI controller binding

[Thierry Reding]

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

On Wed, Oct 29, 2014 at 09:37:14AM -0700, Andrew Bresticker wrote:
> On Wed, Oct 29, 2014 at 2:43 AM, Thierry Reding
> <thierry.reding@gmail.com> wrote:
> > On Tue, Oct 28, 2014 at 03:27:50PM -0700, Andrew Bresticker wrote:
> > [...]
> >> diff --git a/Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-xusb-padctl.txt b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-xusb-padctl.txt
> > [...]
> >> +Optional properties:
> >> +-------------------
> >> +- vbus-{0,1,2}-supply: VBUS regulator for the corresponding UTMI pad.
> >> +- vddio-hsic-supply: VDDIO regulator for the HSIC pads.
> >> +- nvidia,usb3-port-{0,1}-lane: PCIe/SATA lane to which the corresponding USB3
> >> +  port is mapped.  See <dt-bindings/pinctrl/pinctrl-tegra-xusb.h> for the list
> >> +  of valid values.
> >
> > I dislike how we now need to provide a list of all pins in the header
> > file, where previously we used strings for this. This could become very
> > ugly if the set of pins changes in future generations of this IP block.
> >
> > Could we instead derive this from the pinmux nodes? For example you have
> > this in the example below:
> >
> >         usb3p0 {
> >                 nvidia,lanes = "pcie-0";
> >                 ...
> >         };
> >
> > Perhaps what we need is to either key off the node name or add another
> > property, such as:
> >
> >         nvidia,usb3-port = <0>;
> >
> > This would match the nvidia,usb2-port property that you've added below.
> 
> That is actually how I described the USB3 port to SS lane mapping
> originally, but in review of an earlier version of this series,
> Stephen suggested that I make it a separate, not pinconfig property
> since it wasn't a value written directly to the hardware.  I'm fine
> with changing it back as the pinconfig property makes more sense to me
> as well.

Hmm... I had considered it a mux option of the specific lane. If the
function is usb3, it'd still need to be muxed to one of the ports. So
it's additional information associated with the usb3 function.

I did look through the driver changes and can't really make out which
part of the code actually performs this assignment. Can you point me to
it?

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH RESEND V4 2/9] mailbox: Add NVIDIA Tegra XUSB mailbox driver

[Andrew Bresticker]

On Thu, Oct 30, 2014 at 6:22 AM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> On Wed, Oct 29, 2014 at 11:02:36AM -0700, Andrew Bresticker wrote:
> [...]
>> > Maybe something like this patch would be more correct in handling
>> > this:
>> >
>> > diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
>> > index afcb430508ec..85691a7d8ca6 100644
>> > --- a/drivers/mailbox/mailbox.c
>> > +++ b/drivers/mailbox/mailbox.c
>> > @@ -117,10 +117,11 @@ static void poll_txdone(unsigned long data)
>> >                 struct mbox_chan *chan = &mbox->chans[i];
>> >
>> >                 if (chan->active_req && chan->cl) {
>> > -                       resched = true;
>> >                         txdone = chan->mbox->ops->last_tx_done(chan);
>> >                         if (txdone)
>> >                                 tx_tick(chan, 0);
>> > +                       else
>> > +                               resched = true;
>> >                 }
>> >         }
>>
>> ... but we still need to re-arm the timer if tx_tick() submits another
>> message.  Perhaps the better thing to do is to have msg_submit() arm
>> the timer.
>
> I think we need both. If the last transmission isn't done yet we still
> want to keep polling. And we also want to poll if a new message is sent
> subsequently.
>
> Perhaps it would be as easy as moving the poll handling code from
> mbox_send_message() (if (chan->txdone_method == TXDONE_BY_POLL)) into
> msg_submit()? That has the additional advantage of being able to omit
> the polling when an error happens during the mbox' .send_data().

Yes, this is exactly what I've done :).

Re: [PATCH RESEND V4 4/9] pinctrl: tegra-xusb: Add USB PHY support

[Andrew Bresticker]

On Thu, Oct 30, 2014 at 6:45 AM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> On Wed, Oct 29, 2014 at 12:43:36PM -0700, Andrew Bresticker wrote:
>> >> diff --git a/drivers/pinctrl/pinctrl-tegra-xusb.c b/drivers/pinctrl/pinctrl-tegra-xusb.c
> [...]
>> >> +
>> >> +     for (i = 0; i < TEGRA_XUSB_USB3_PHYS; i++) {
>> >> +             if (phy == padctl->phys[TEGRA_XUSB_PADCTL_USB3_P0 + i])
>> >> +                     break;
>> >
>> > You could simply return i here and then BUG_ON unconditionally.
>> >
>> >> +     }
>> >> +     BUG_ON(i == TEGRA_XUSB_USB3_PHYS);
>> >> +
>> >> +     return i;
>> >> +}
>> >
>> > Actually, thinking about it some more, perhaps making this a WARN_ON()
>> > and returning an error so that we can continue and propagate the error
>> > would be more useful. BUG_ON() will completely hang the kernel with no
>> > way out but rebooting. WARN_ON() will give a hint about something being
>> > wrong and returning an error will allow the kernel to continue to run,
>> > which might be the only way to diagnose and fix the problem, even if it
>> > means that USB 3.0 support will be disabled.
>>
>> I felt like BUG_ON is more appropriate here.  Hitting this case means
>> there's a bug in the PHY core or a driver has passed a bogus pointer
>> and the stack dump produced by the BUG_ON should make it obvious as to
>> what the issue is.  I don't feel too strongly about it though.
>
> The problem with BUG_ON() is that you won't be able to go any further.
> So if this were to happen on a device with no serial you might not even
> get to a point where you actually see an error message. Handling this
> more gracefully by propagating the error code and failing .probe() does
> not seem overly complicated and the WARN_ON() output will hopefully
> still be noticed (it probably will be after the user can't get USB to
> work).

Ok.

>> >> @@ -936,6 +2098,18 @@ static int tegra_xusb_padctl_probe(struct platform_device *pdev)
>> >>               goto unregister;
>> >>       }
>> >>
>> >> +     INIT_WORK(&padctl->mbox_req_work, tegra_xusb_phy_mbox_work);
>> >> +     padctl->mbox_client.dev = &pdev->dev;
>> >> +     padctl->mbox_client.tx_block = true;
>> >> +     padctl->mbox_client.tx_tout = 0;
>> >> +     padctl->mbox_client.rx_callback = tegra_xusb_phy_mbox_rx;
>> >> +     padctl->mbox_chan = mbox_request_channel(&padctl->mbox_client, 0);
>> >> +     if (IS_ERR(padctl->mbox_chan)) {
>> >> +             err = PTR_ERR(padctl->mbox_chan);
>> >> +             dev_err(&pdev->dev, "failed to request mailbox: %d\n", err);
>> >> +             goto unregister;
>> >> +     }
>> >
>> > I think this should be done before the registering the PHY provider so
>> > that we don't expose one (even for only a very short time) before we
>> > haven't made sure that it can be used.
>> >
>> > Also, this effectively makes the mailbox mandatory, which means that the
>> > above code is going to break on older DTBs. So I think we have no choice
>> > but to make mailbox (and hence XUSB) support optional.
>>
>> I understand the need for binding stability, but it's not like these
>> bindings have been around for very long (a release or two?) and this
>> series has existed for almost the same amount of time.  Are there
>> really any DTBs out there that are going to break because of this?
>
> Every DTB created from a kernel version that has the original binding
> but not the one modified as part of this series is going to break. Last
> time I checked there weren't any exceptions to this rule. Note, though,
> that the rule is that existing functionality must not break. That is,
> SATA and PCIe should remain functional, so it should be fine if you just
> don't register any of the USB PHYs when the request for a mailbox
> channel fails. Something along these lines should do it:
>
>         padctl->mbox_chan = mbox_request_channel(...);
>         if (!IS_ERR(padctl->mbox_chan)) {
>                 err = tegra_xusb_padctl_setup_usb(...);
>                 ...
>         }

Ok.

>> >> diff --git a/include/soc/tegra/xusb.h b/include/soc/tegra/xusb.h
>> >> index cfe211d..149434f 100644
>> >> --- a/include/soc/tegra/xusb.h
>> >> +++ b/include/soc/tegra/xusb.h
>> >> @@ -10,6 +10,13 @@
>> >>  #ifndef __SOC_TEGRA_XUSB_H__
>> >>  #define __SOC_TEGRA_XUSB_H__
>> >>
>> >> +#define TEGRA_XUSB_USB3_PHYS 2
>> >> +#define TEGRA_XUSB_UTMI_PHYS 3
>> >> +#define TEGRA_XUSB_HSIC_PHYS 2
>> >> +#define TEGRA_XUSB_NUM_USB_PHYS (TEGRA_XUSB_USB3_PHYS + TEGRA_XUSB_UTMI_PHYS + \
>> >> +                              TEGRA_XUSB_HSIC_PHYS)
>> >> +#define TEGRA_XUSB_NUM_PHYS (TEGRA_XUSB_NUM_USB_PHYS + 2) /* + SATA & PCIe */
>> >
>> > These are really XUSB pad controller specific defines, why does anyone
>> > else need to know this?
>>
>> They're not pad controller specific.  They're also used in the xHCI host driver.
>
> I keep thinking that there should be a way around this. Of course if
> both the XHCI and mailbox drivers were merged, then there'd be no need
> to expose this publicly at all.

I'm not sure what you mean.  They're SoC-specific constants that need
to be shared amongst multiple drivers.  It would make sense to place
them in a shared header, does it not?

Re: [PATCH RESEND V4 5/9] of: Add NVIDIA Tegra xHCI controller binding

[Andrew Bresticker]

On Thu, Oct 30, 2014 at 6:55 AM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> On Wed, Oct 29, 2014 at 09:37:14AM -0700, Andrew Bresticker wrote:
>> On Wed, Oct 29, 2014 at 2:43 AM, Thierry Reding
>> <thierry.reding@gmail.com> wrote:
>> > On Tue, Oct 28, 2014 at 03:27:50PM -0700, Andrew Bresticker wrote:
>> > [...]
>> >> diff --git a/Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-xusb-padctl.txt b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-xusb-padctl.txt
>> > [...]
>> >> +Optional properties:
>> >> +-------------------
>> >> +- vbus-{0,1,2}-supply: VBUS regulator for the corresponding UTMI pad.
>> >> +- vddio-hsic-supply: VDDIO regulator for the HSIC pads.
>> >> +- nvidia,usb3-port-{0,1}-lane: PCIe/SATA lane to which the corresponding USB3
>> >> +  port is mapped.  See <dt-bindings/pinctrl/pinctrl-tegra-xusb.h> for the list
>> >> +  of valid values.
>> >
>> > I dislike how we now need to provide a list of all pins in the header
>> > file, where previously we used strings for this. This could become very
>> > ugly if the set of pins changes in future generations of this IP block.
>> >
>> > Could we instead derive this from the pinmux nodes? For example you have
>> > this in the example below:
>> >
>> >         usb3p0 {
>> >                 nvidia,lanes = "pcie-0";
>> >                 ...
>> >         };
>> >
>> > Perhaps what we need is to either key off the node name or add another
>> > property, such as:
>> >
>> >         nvidia,usb3-port = <0>;
>> >
>> > This would match the nvidia,usb2-port property that you've added below.
>>
>> That is actually how I described the USB3 port to SS lane mapping
>> originally, but in review of an earlier version of this series,
>> Stephen suggested that I make it a separate, not pinconfig property
>> since it wasn't a value written directly to the hardware.  I'm fine
>> with changing it back as the pinconfig property makes more sense to me
>> as well.
>
> Hmm... I had considered it a mux option of the specific lane. If the
> function is usb3, it'd still need to be muxed to one of the ports. So
> it's additional information associated with the usb3 function.
>
> I did look through the driver changes and can't really make out which
> part of the code actually performs this assignment. Can you point me to
> it?

There's not really an assignment.  The property is used to map between
a lane (e.g. PCIe-0 or SATA) and the USB3.0 port it's mapped to.  For
an example of where it's used, take a look at usb3_phy_power_on().
There are certain per-lane registers which need to be programmed in
addition to the per-USB3.0 port pad registers.  This mapping is used
to determine which lane needs to be programmed.

Re: [PATCH RESEND V4 5/9] of: Add NVIDIA Tegra xHCI controller binding

[Thierry Reding]

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

On Thu, Oct 30, 2014 at 10:19:21AM -0700, Andrew Bresticker wrote:
> On Thu, Oct 30, 2014 at 6:55 AM, Thierry Reding
> <thierry.reding@gmail.com> wrote:
> > On Wed, Oct 29, 2014 at 09:37:14AM -0700, Andrew Bresticker wrote:
> >> On Wed, Oct 29, 2014 at 2:43 AM, Thierry Reding
> >> <thierry.reding@gmail.com> wrote:
> >> > On Tue, Oct 28, 2014 at 03:27:50PM -0700, Andrew Bresticker wrote:
> >> > [...]
> >> >> diff --git a/Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-xusb-padctl.txt b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-xusb-padctl.txt
> >> > [...]
> >> >> +Optional properties:
> >> >> +-------------------
> >> >> +- vbus-{0,1,2}-supply: VBUS regulator for the corresponding UTMI pad.
> >> >> +- vddio-hsic-supply: VDDIO regulator for the HSIC pads.
> >> >> +- nvidia,usb3-port-{0,1}-lane: PCIe/SATA lane to which the corresponding USB3
> >> >> +  port is mapped.  See <dt-bindings/pinctrl/pinctrl-tegra-xusb.h> for the list
> >> >> +  of valid values.
> >> >
> >> > I dislike how we now need to provide a list of all pins in the header
> >> > file, where previously we used strings for this. This could become very
> >> > ugly if the set of pins changes in future generations of this IP block.
> >> >
> >> > Could we instead derive this from the pinmux nodes? For example you have
> >> > this in the example below:
> >> >
> >> >         usb3p0 {
> >> >                 nvidia,lanes = "pcie-0";
> >> >                 ...
> >> >         };
> >> >
> >> > Perhaps what we need is to either key off the node name or add another
> >> > property, such as:
> >> >
> >> >         nvidia,usb3-port = <0>;
> >> >
> >> > This would match the nvidia,usb2-port property that you've added below.
> >>
> >> That is actually how I described the USB3 port to SS lane mapping
> >> originally, but in review of an earlier version of this series,
> >> Stephen suggested that I make it a separate, not pinconfig property
> >> since it wasn't a value written directly to the hardware.  I'm fine
> >> with changing it back as the pinconfig property makes more sense to me
> >> as well.
> >
> > Hmm... I had considered it a mux option of the specific lane. If the
> > function is usb3, it'd still need to be muxed to one of the ports. So
> > it's additional information associated with the usb3 function.
> >
> > I did look through the driver changes and can't really make out which
> > part of the code actually performs this assignment. Can you point me to
> > it?
> 
> There's not really an assignment.  The property is used to map between
> a lane (e.g. PCIe-0 or SATA) and the USB3.0 port it's mapped to.  For
> an example of where it's used, take a look at usb3_phy_power_on().
> There are certain per-lane registers which need to be programmed in
> addition to the per-USB3.0 port pad registers.  This mapping is used
> to determine which lane needs to be programmed.

Are you saying the mapping of lane to USB port is fixed? That is, PCIe-0
lane is always used for USB port X and SATA always for USB port Y?

If so I'd argue that we don't need this property in DT at all.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH RESEND V4 5/9] of: Add NVIDIA Tegra xHCI controller binding

[Andrew Bresticker]

On Thu, Oct 30, 2014 at 10:24 AM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> On Thu, Oct 30, 2014 at 10:19:21AM -0700, Andrew Bresticker wrote:
>> On Thu, Oct 30, 2014 at 6:55 AM, Thierry Reding
>> <thierry.reding@gmail.com> wrote:
>> > On Wed, Oct 29, 2014 at 09:37:14AM -0700, Andrew Bresticker wrote:
>> >> On Wed, Oct 29, 2014 at 2:43 AM, Thierry Reding
>> >> <thierry.reding@gmail.com> wrote:
>> >> > On Tue, Oct 28, 2014 at 03:27:50PM -0700, Andrew Bresticker wrote:
>> >> > [...]
>> >> >> diff --git a/Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-xusb-padctl.txt b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-xusb-padctl.txt
>> >> > [...]
>> >> >> +Optional properties:
>> >> >> +-------------------
>> >> >> +- vbus-{0,1,2}-supply: VBUS regulator for the corresponding UTMI pad.
>> >> >> +- vddio-hsic-supply: VDDIO regulator for the HSIC pads.
>> >> >> +- nvidia,usb3-port-{0,1}-lane: PCIe/SATA lane to which the corresponding USB3
>> >> >> +  port is mapped.  See <dt-bindings/pinctrl/pinctrl-tegra-xusb.h> for the list
>> >> >> +  of valid values.
>> >> >
>> >> > I dislike how we now need to provide a list of all pins in the header
>> >> > file, where previously we used strings for this. This could become very
>> >> > ugly if the set of pins changes in future generations of this IP block.
>> >> >
>> >> > Could we instead derive this from the pinmux nodes? For example you have
>> >> > this in the example below:
>> >> >
>> >> >         usb3p0 {
>> >> >                 nvidia,lanes = "pcie-0";
>> >> >                 ...
>> >> >         };
>> >> >
>> >> > Perhaps what we need is to either key off the node name or add another
>> >> > property, such as:
>> >> >
>> >> >         nvidia,usb3-port = <0>;
>> >> >
>> >> > This would match the nvidia,usb2-port property that you've added below.
>> >>
>> >> That is actually how I described the USB3 port to SS lane mapping
>> >> originally, but in review of an earlier version of this series,
>> >> Stephen suggested that I make it a separate, not pinconfig property
>> >> since it wasn't a value written directly to the hardware.  I'm fine
>> >> with changing it back as the pinconfig property makes more sense to me
>> >> as well.
>> >
>> > Hmm... I had considered it a mux option of the specific lane. If the
>> > function is usb3, it'd still need to be muxed to one of the ports. So
>> > it's additional information associated with the usb3 function.
>> >
>> > I did look through the driver changes and can't really make out which
>> > part of the code actually performs this assignment. Can you point me to
>> > it?
>>
>> There's not really an assignment.  The property is used to map between
>> a lane (e.g. PCIe-0 or SATA) and the USB3.0 port it's mapped to.  For
>> an example of where it's used, take a look at usb3_phy_power_on().
>> There are certain per-lane registers which need to be programmed in
>> addition to the per-USB3.0 port pad registers.  This mapping is used
>> to determine which lane needs to be programmed.
>
> Are you saying the mapping of lane to USB port is fixed? That is, PCIe-0
> lane is always used for USB port X and SATA always for USB port Y?

No, sorry if that was unclear, it's not fixed - it's a board specific property.

Re: [PATCH RESEND V4 3/9] of: Update Tegra XUSB pad controller binding for USB

[Linus Walleij]

On Tue, Oct 28, 2014 at 11:27 PM, Andrew Bresticker
<abrestic@chromium.org> wrote:

> Add new bindings used for USB support by the Tegra XUSB pad controller.
> This includes additional PHY types, USB-specific pinconfig properties, etc.
>
> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
> Reviewed-by: Stephen Warren <swarren@nvidia.com>
> ---
> Changes from v2:
>  - Added nvidia,otg-hs-curr-level-offset property.
>  - Dropped "-otg" from VBUS supplies.
>  - Added mbox-names property.
>  - Removed extra whitespace.
> Changes from v1:
>  - Updated to use common mailbox bindings.
>  - Made USB3 port-to-lane mappins a top-level binding rather than a pinconfig
>    binding.
>  - Add #defines for the padctl lanes.

Acked-by: Linus Walleij <linus.walleij@linaro.org>

I guess you will take this patch along with the rest through ARM SoC
or so?

Yours.
Linus Walleij

Re: [PATCH RESEND V4 4/9] pinctrl: tegra-xusb: Add USB PHY support

[Thierry Reding]

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

On Thu, Oct 30, 2014 at 10:10:06AM -0700, Andrew Bresticker wrote:
> On Thu, Oct 30, 2014 at 6:45 AM, Thierry Reding
> <thierry.reding@gmail.com> wrote:
> > On Wed, Oct 29, 2014 at 12:43:36PM -0700, Andrew Bresticker wrote:
> >> >> diff --git a/drivers/pinctrl/pinctrl-tegra-xusb.c b/drivers/pinctrl/pinctrl-tegra-xusb.c
[...]
> >> >> diff --git a/include/soc/tegra/xusb.h b/include/soc/tegra/xusb.h
> >> >> index cfe211d..149434f 100644
> >> >> --- a/include/soc/tegra/xusb.h
> >> >> +++ b/include/soc/tegra/xusb.h
> >> >> @@ -10,6 +10,13 @@
> >> >>  #ifndef __SOC_TEGRA_XUSB_H__
> >> >>  #define __SOC_TEGRA_XUSB_H__
> >> >>
> >> >> +#define TEGRA_XUSB_USB3_PHYS 2
> >> >> +#define TEGRA_XUSB_UTMI_PHYS 3
> >> >> +#define TEGRA_XUSB_HSIC_PHYS 2
> >> >> +#define TEGRA_XUSB_NUM_USB_PHYS (TEGRA_XUSB_USB3_PHYS + TEGRA_XUSB_UTMI_PHYS + \
> >> >> +                              TEGRA_XUSB_HSIC_PHYS)
> >> >> +#define TEGRA_XUSB_NUM_PHYS (TEGRA_XUSB_NUM_USB_PHYS + 2) /* + SATA & PCIe */
> >> >
> >> > These are really XUSB pad controller specific defines, why does anyone
> >> > else need to know this?
> >>
> >> They're not pad controller specific.  They're also used in the xHCI host driver.
> >
> > I keep thinking that there should be a way around this. Of course if
> > both the XHCI and mailbox drivers were merged, then there'd be no need
> > to expose this publicly at all.
> 
> I'm not sure what you mean.  They're SoC-specific constants that need
> to be shared amongst multiple drivers.  It would make sense to place
> them in a shared header, does it not?

The problem with this is that if those numbers ever change on a future
generation of Tegra then we're going to have to suffix them in some way
to support more than one generation. And the code to handle that is
going to be ugly because we'd need to differentiate on the compatible
string to match which suffixed version to use.

So I'd rather see this parameterized some way. Of course that's a lot
more difficult to do because these things are shared across XHCI and pad
controller drivers.

That said, I think it'd be fine to merge this as-is for now and rewrite
this in a better way if it ever becomes a problem.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH RESEND V4 5/9] of: Add NVIDIA Tegra xHCI controller binding

[Thierry Reding]

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

On Thu, Oct 30, 2014 at 10:26:47AM -0700, Andrew Bresticker wrote:
> On Thu, Oct 30, 2014 at 10:24 AM, Thierry Reding
> <thierry.reding@gmail.com> wrote:
> > On Thu, Oct 30, 2014 at 10:19:21AM -0700, Andrew Bresticker wrote:
> >> On Thu, Oct 30, 2014 at 6:55 AM, Thierry Reding
> >> <thierry.reding@gmail.com> wrote:
> >> > On Wed, Oct 29, 2014 at 09:37:14AM -0700, Andrew Bresticker wrote:
> >> >> On Wed, Oct 29, 2014 at 2:43 AM, Thierry Reding
> >> >> <thierry.reding@gmail.com> wrote:
> >> >> > On Tue, Oct 28, 2014 at 03:27:50PM -0700, Andrew Bresticker wrote:
> >> >> > [...]
> >> >> >> diff --git a/Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-xusb-padctl.txt b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-xusb-padctl.txt
> >> >> > [...]
> >> >> >> +Optional properties:
> >> >> >> +-------------------
> >> >> >> +- vbus-{0,1,2}-supply: VBUS regulator for the corresponding UTMI pad.
> >> >> >> +- vddio-hsic-supply: VDDIO regulator for the HSIC pads.
> >> >> >> +- nvidia,usb3-port-{0,1}-lane: PCIe/SATA lane to which the corresponding USB3
> >> >> >> +  port is mapped.  See <dt-bindings/pinctrl/pinctrl-tegra-xusb.h> for the list
> >> >> >> +  of valid values.
> >> >> >
> >> >> > I dislike how we now need to provide a list of all pins in the header
> >> >> > file, where previously we used strings for this. This could become very
> >> >> > ugly if the set of pins changes in future generations of this IP block.
> >> >> >
> >> >> > Could we instead derive this from the pinmux nodes? For example you have
> >> >> > this in the example below:
> >> >> >
> >> >> >         usb3p0 {
> >> >> >                 nvidia,lanes = "pcie-0";
> >> >> >                 ...
> >> >> >         };
> >> >> >
> >> >> > Perhaps what we need is to either key off the node name or add another
> >> >> > property, such as:
> >> >> >
> >> >> >         nvidia,usb3-port = <0>;
> >> >> >
> >> >> > This would match the nvidia,usb2-port property that you've added below.
> >> >>
> >> >> That is actually how I described the USB3 port to SS lane mapping
> >> >> originally, but in review of an earlier version of this series,
> >> >> Stephen suggested that I make it a separate, not pinconfig property
> >> >> since it wasn't a value written directly to the hardware.  I'm fine
> >> >> with changing it back as the pinconfig property makes more sense to me
> >> >> as well.
> >> >
> >> > Hmm... I had considered it a mux option of the specific lane. If the
> >> > function is usb3, it'd still need to be muxed to one of the ports. So
> >> > it's additional information associated with the usb3 function.
> >> >
> >> > I did look through the driver changes and can't really make out which
> >> > part of the code actually performs this assignment. Can you point me to
> >> > it?
> >>
> >> There's not really an assignment.  The property is used to map between
> >> a lane (e.g. PCIe-0 or SATA) and the USB3.0 port it's mapped to.  For
> >> an example of where it's used, take a look at usb3_phy_power_on().
> >> There are certain per-lane registers which need to be programmed in
> >> addition to the per-USB3.0 port pad registers.  This mapping is used
> >> to determine which lane needs to be programmed.
> >
> > Are you saying the mapping of lane to USB port is fixed? That is, PCIe-0
> > lane is always used for USB port X and SATA always for USB port Y?
> 
> No, sorry if that was unclear, it's not fixed - it's a board specific
> property.

Okay, but there's no register that contains the mapping of the port to a
lane, similar to what's done for the functions, right? I mean the driver
only uses the lane to find out which register to write. Doesn't that
imply that two lanes (or more) could be mapped to the same USB 3.0 port?

I'm not sure I'm being clear here, so let me try another way. In order
to establish a mapping between USB port and lane, I would've expected
one of the following to happen:

	- A value derived from the lane number is written to a register
	  belonging to a given port.

	- A value derived from the port number is written to a register
	  belonging to a given lane.

I can't see the code do either of the above, which to me implies that
there's a fixed mapping between lanes and ports. What am I missing?

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH RESEND V4 5/9] of: Add NVIDIA Tegra xHCI controller binding

[Andrew Bresticker]

On Fri, Oct 31, 2014 at 4:32 AM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> On Thu, Oct 30, 2014 at 10:26:47AM -0700, Andrew Bresticker wrote:
>> On Thu, Oct 30, 2014 at 10:24 AM, Thierry Reding
>> <thierry.reding@gmail.com> wrote:
>> > On Thu, Oct 30, 2014 at 10:19:21AM -0700, Andrew Bresticker wrote:
>> >> On Thu, Oct 30, 2014 at 6:55 AM, Thierry Reding
>> >> <thierry.reding@gmail.com> wrote:
>> >> > On Wed, Oct 29, 2014 at 09:37:14AM -0700, Andrew Bresticker wrote:
>> >> >> On Wed, Oct 29, 2014 at 2:43 AM, Thierry Reding
>> >> >> <thierry.reding@gmail.com> wrote:
>> >> >> > On Tue, Oct 28, 2014 at 03:27:50PM -0700, Andrew Bresticker wrote:
>> >> >> > [...]
>> >> >> >> diff --git a/Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-xusb-padctl.txt b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-xusb-padctl.txt
>> >> >> > [...]
>> >> >> >> +Optional properties:
>> >> >> >> +-------------------
>> >> >> >> +- vbus-{0,1,2}-supply: VBUS regulator for the corresponding UTMI pad.
>> >> >> >> +- vddio-hsic-supply: VDDIO regulator for the HSIC pads.
>> >> >> >> +- nvidia,usb3-port-{0,1}-lane: PCIe/SATA lane to which the corresponding USB3
>> >> >> >> +  port is mapped.  See <dt-bindings/pinctrl/pinctrl-tegra-xusb.h> for the list
>> >> >> >> +  of valid values.
>> >> >> >
>> >> >> > I dislike how we now need to provide a list of all pins in the header
>> >> >> > file, where previously we used strings for this. This could become very
>> >> >> > ugly if the set of pins changes in future generations of this IP block.
>> >> >> >
>> >> >> > Could we instead derive this from the pinmux nodes? For example you have
>> >> >> > this in the example below:
>> >> >> >
>> >> >> >         usb3p0 {
>> >> >> >                 nvidia,lanes = "pcie-0";
>> >> >> >                 ...
>> >> >> >         };
>> >> >> >
>> >> >> > Perhaps what we need is to either key off the node name or add another
>> >> >> > property, such as:
>> >> >> >
>> >> >> >         nvidia,usb3-port = <0>;
>> >> >> >
>> >> >> > This would match the nvidia,usb2-port property that you've added below.
>> >> >>
>> >> >> That is actually how I described the USB3 port to SS lane mapping
>> >> >> originally, but in review of an earlier version of this series,
>> >> >> Stephen suggested that I make it a separate, not pinconfig property
>> >> >> since it wasn't a value written directly to the hardware.  I'm fine
>> >> >> with changing it back as the pinconfig property makes more sense to me
>> >> >> as well.
>> >> >
>> >> > Hmm... I had considered it a mux option of the specific lane. If the
>> >> > function is usb3, it'd still need to be muxed to one of the ports. So
>> >> > it's additional information associated with the usb3 function.
>> >> >
>> >> > I did look through the driver changes and can't really make out which
>> >> > part of the code actually performs this assignment. Can you point me to
>> >> > it?
>> >>
>> >> There's not really an assignment.  The property is used to map between
>> >> a lane (e.g. PCIe-0 or SATA) and the USB3.0 port it's mapped to.  For
>> >> an example of where it's used, take a look at usb3_phy_power_on().
>> >> There are certain per-lane registers which need to be programmed in
>> >> addition to the per-USB3.0 port pad registers.  This mapping is used
>> >> to determine which lane needs to be programmed.
>> >
>> > Are you saying the mapping of lane to USB port is fixed? That is, PCIe-0
>> > lane is always used for USB port X and SATA always for USB port Y?
>>
>> No, sorry if that was unclear, it's not fixed - it's a board specific
>> property.
>
> Okay, but there's no register that contains the mapping of the port to a
> lane, similar to what's done for the functions, right?

Correct.

> I mean the driver only uses the lane to find out which register to write.
> Doesn't that imply that two lanes (or more) could be mapped to the same
> USB 3.0 port?

I guess?  Not sure how that would work in hardware to have two
SuperSpeed lanes wired up to a single port.

> I'm not sure I'm being clear here, so let me try another way. In order
> to establish a mapping between USB port and lane, I would've expected
> one of the following to happen:
>
>         - A value derived from the lane number is written to a register
>           belonging to a given port.
>
>         - A value derived from the port number is written to a register
>           belonging to a given lane.
>
> I can't see the code do either of the above, which to me implies that
> there's a fixed mapping between lanes and ports. What am I missing?

It's fixed in that it's not a software-modifiable property.  It's
describing an electrical connection between lane and port.  For
example, it's possible that one board has PCIe lane 1 wired up to
SuperSpeed port 1 and on another board for the SATA lane to be wired
up to SuperSpeed port 1.  Does that make sense?

Re: [PATCH RESEND V4 3/9] of: Update Tegra XUSB pad controller binding for USB

[Andrew Bresticker]

On Fri, Oct 31, 2014 at 2:44 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Tue, Oct 28, 2014 at 11:27 PM, Andrew Bresticker
> <abrestic@chromium.org> wrote:
>
>> Add new bindings used for USB support by the Tegra XUSB pad controller.
>> This includes additional PHY types, USB-specific pinconfig properties, etc.
>>
>> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
>> Reviewed-by: Stephen Warren <swarren@nvidia.com>
>> ---
>> Changes from v2:
>>  - Added nvidia,otg-hs-curr-level-offset property.
>>  - Dropped "-otg" from VBUS supplies.
>>  - Added mbox-names property.
>>  - Removed extra whitespace.
>> Changes from v1:
>>  - Updated to use common mailbox bindings.
>>  - Made USB3 port-to-lane mappins a top-level binding rather than a pinconfig
>>    binding.
>>  - Add #defines for the padctl lanes.
>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
>
> I guess you will take this patch along with the rest through ARM SoC
> or so?

Yes, that's the plan.

Re: [PATCH RESEND V4 5/9] of: Add NVIDIA Tegra xHCI controller binding

[Andrew Bresticker]

On Fri, Oct 31, 2014 at 9:41 AM, Andrew Bresticker
<abrestic@chromium.org> wrote:
> On Fri, Oct 31, 2014 at 4:32 AM, Thierry Reding
> <thierry.reding@gmail.com> wrote:
>> On Thu, Oct 30, 2014 at 10:26:47AM -0700, Andrew Bresticker wrote:
>>> On Thu, Oct 30, 2014 at 10:24 AM, Thierry Reding
>>> <thierry.reding@gmail.com> wrote:
>>> > On Thu, Oct 30, 2014 at 10:19:21AM -0700, Andrew Bresticker wrote:
>>> >> On Thu, Oct 30, 2014 at 6:55 AM, Thierry Reding
>>> >> <thierry.reding@gmail.com> wrote:
>>> >> > On Wed, Oct 29, 2014 at 09:37:14AM -0700, Andrew Bresticker wrote:
>>> >> >> On Wed, Oct 29, 2014 at 2:43 AM, Thierry Reding
>>> >> >> <thierry.reding@gmail.com> wrote:
>>> >> >> > On Tue, Oct 28, 2014 at 03:27:50PM -0700, Andrew Bresticker wrote:
>>> >> >> > [...]
>>> >> >> >> diff --git a/Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-xusb-padctl.txt b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-xusb-padctl.txt
>>> >> >> > [...]
>>> >> >> >> +Optional properties:
>>> >> >> >> +-------------------
>>> >> >> >> +- vbus-{0,1,2}-supply: VBUS regulator for the corresponding UTMI pad.
>>> >> >> >> +- vddio-hsic-supply: VDDIO regulator for the HSIC pads.
>>> >> >> >> +- nvidia,usb3-port-{0,1}-lane: PCIe/SATA lane to which the corresponding USB3
>>> >> >> >> +  port is mapped.  See <dt-bindings/pinctrl/pinctrl-tegra-xusb.h> for the list
>>> >> >> >> +  of valid values.
>>> >> >> >
>>> >> >> > I dislike how we now need to provide a list of all pins in the header
>>> >> >> > file, where previously we used strings for this. This could become very
>>> >> >> > ugly if the set of pins changes in future generations of this IP block.
>>> >> >> >
>>> >> >> > Could we instead derive this from the pinmux nodes? For example you have
>>> >> >> > this in the example below:
>>> >> >> >
>>> >> >> >         usb3p0 {
>>> >> >> >                 nvidia,lanes = "pcie-0";
>>> >> >> >                 ...
>>> >> >> >         };
>>> >> >> >
>>> >> >> > Perhaps what we need is to either key off the node name or add another
>>> >> >> > property, such as:
>>> >> >> >
>>> >> >> >         nvidia,usb3-port = <0>;
>>> >> >> >
>>> >> >> > This would match the nvidia,usb2-port property that you've added below.
>>> >> >>
>>> >> >> That is actually how I described the USB3 port to SS lane mapping
>>> >> >> originally, but in review of an earlier version of this series,
>>> >> >> Stephen suggested that I make it a separate, not pinconfig property
>>> >> >> since it wasn't a value written directly to the hardware.  I'm fine
>>> >> >> with changing it back as the pinconfig property makes more sense to me
>>> >> >> as well.
>>> >> >
>>> >> > Hmm... I had considered it a mux option of the specific lane. If the
>>> >> > function is usb3, it'd still need to be muxed to one of the ports. So
>>> >> > it's additional information associated with the usb3 function.
>>> >> >
>>> >> > I did look through the driver changes and can't really make out which
>>> >> > part of the code actually performs this assignment. Can you point me to
>>> >> > it?
>>> >>
>>> >> There's not really an assignment.  The property is used to map between
>>> >> a lane (e.g. PCIe-0 or SATA) and the USB3.0 port it's mapped to.  For
>>> >> an example of where it's used, take a look at usb3_phy_power_on().
>>> >> There are certain per-lane registers which need to be programmed in
>>> >> addition to the per-USB3.0 port pad registers.  This mapping is used
>>> >> to determine which lane needs to be programmed.
>>> >
>>> > Are you saying the mapping of lane to USB port is fixed? That is, PCIe-0
>>> > lane is always used for USB port X and SATA always for USB port Y?
>>>
>>> No, sorry if that was unclear, it's not fixed - it's a board specific
>>> property.
>>
>> Okay, but there's no register that contains the mapping of the port to a
>> lane, similar to what's done for the functions, right?
>
> Correct.
>
>> I mean the driver only uses the lane to find out which register to write.
>> Doesn't that imply that two lanes (or more) could be mapped to the same
>> USB 3.0 port?
>
> I guess?  Not sure how that would work in hardware to have two
> SuperSpeed lanes wired up to a single port.
>
>> I'm not sure I'm being clear here, so let me try another way. In order
>> to establish a mapping between USB port and lane, I would've expected
>> one of the following to happen:
>>
>>         - A value derived from the lane number is written to a register
>>           belonging to a given port.
>>
>>         - A value derived from the port number is written to a register
>>           belonging to a given lane.
>>
>> I can't see the code do either of the above, which to me implies that
>> there's a fixed mapping between lanes and ports. What am I missing?
>
> It's fixed in that it's not a software-modifiable property.  It's
> describing an electrical connection between lane and port.  For
> example, it's possible that one board has PCIe lane 1 wired up to
> SuperSpeed port 1 and on another board for the SATA lane to be wired
> up to SuperSpeed port 1.  Does that make sense?

So what's the verdict here?