[Patch v9 0/3] DWC3 USB support for Qualcomm platform

[Patch v9 0/3] DWC3 USB support for Qualcomm platform

[Andy Gross]

These patches add basic support for USB3.0 controllers found
on MSM platforms. USB3.0 core is based on Synopsys DesignWare
SuperSpeed IP.

This work was started by Ivan Ivanov and went through a number of iterations.  I
picked these patches up and did a little rework to get them working.

Changes since v8:
* Fixed minor comments from reviewers.

Changes since v7:
* Reworked phy driver to conform to the generic phy framework.
* Collapsed HS and SS phy drivers into same source file.
* Dropped regulators from the drivers.  The only platform where we can utilize
  this driver has regulators on all the time.  Will add regulators back in when
  they are required.
* Added timeouts to the phy register accesses
* Put in definitions for the phy specific registers and bitfields
* Removed unnecessary workarounds.

Changes since v6:
* Renamed all MSM references to qcom, including file names
* Removed direct TCSR manipulation.  This done from new TCSR driver.
* Added defines for register bits
* XO clk is optional on some platforms.  Corrected logic to handle this.
* Ignore set_voltage failures.  This allows us to support dummy regulators on
  platforms where there is no voltage control.
* Reworked devicetree binding to remove TCSR reg resources

Changes since v5:
* devicetree bindings descriptions fixes
* Fixed NULL pointer dereferences in dev_prink's
* Removed extra space in "sleep " clock name

Changes since v4:
* Substitute references to "wc3" with just "dw" in USB PHY drivers and
  file names. This is to indicate that the PHY's are DesignWare, but
  not necessarily related to DWC3 IP core.

Changes since v3:
* Remove "_clk" suffix from clock names
* Clarify required child node for qcom,dwc3
* Fix comments in functions headers
* Use dbg instead err in drivers probe functions.

Changes since v2:
* Several improvements in devicetree bindings description
* Disable regulators in glue layer if there is error during
  ioremap.

Changes since first version:
* Split devicetree bindings description file to separate patch
* Address comments for device bindings description
* Fix typo in 'gdsc' requlator name.

Andy Gross (1):
  phy: Add Qualcomm DWC3 HS/SS PHY driver

Ivan T. Ivanov (2):
  usb: dwc3: qcom: Add device tree binding
  usb: dwc3: Add Qualcomm DWC3 glue layer driver

 .../devicetree/bindings/phy/qcom-dwc3-usb-phy.txt  |   39 ++
 .../devicetree/bindings/usb/qcom,dwc3.txt          |   66 +++
 drivers/phy/Kconfig                                |   11 +
 drivers/phy/Makefile                               |    1 +
 drivers/phy/phy-qcom-dwc3.c                        |  483 ++++++++++++++++++++
 drivers/usb/dwc3/Kconfig                           |    8 +
 drivers/usb/dwc3/Makefile                          |    1 +
 drivers/usb/dwc3/dwc3-qcom.c                       |  131 ++++++
 8 files changed, 740 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/qcom-dwc3-usb-phy.txt
 create mode 100644 Documentation/devicetree/bindings/usb/qcom,dwc3.txt
 create mode 100644 drivers/phy/phy-qcom-dwc3.c
 create mode 100644 drivers/usb/dwc3/dwc3-qcom.c

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[Patch v9 1/3] usb: dwc3: qcom: Add device tree binding

[Andy Gross]

From: "Ivan T. Ivanov" <iivanov@mm-sol.com>

QCOM USB3.0 core wrapper consist of USB3.0 IP from Synopsys
(SNPS) and HS, SS PHY's control and configuration registers.

It could operate in device mode (SS, HS, FS) and host
mode (SS, HS, FS, LS).

Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com>
Signed-off-by: Andy Gross <agross@codeaurora.org>
---
 .../devicetree/bindings/phy/qcom-dwc3-usb-phy.txt  |   39 ++++++++++++
 .../devicetree/bindings/usb/qcom,dwc3.txt          |   66 ++++++++++++++++++++
 2 files changed, 105 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/qcom-dwc3-usb-phy.txt
 create mode 100644 Documentation/devicetree/bindings/usb/qcom,dwc3.txt

diff --git a/Documentation/devicetree/bindings/phy/qcom-dwc3-usb-phy.txt b/Documentation/devicetree/bindings/phy/qcom-dwc3-usb-phy.txt
new file mode 100644
index 0000000..86f2dbe
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/qcom-dwc3-usb-phy.txt
@@ -0,0 +1,39 @@
+Qualcomm DWC3 HS AND SS PHY CONTROLLER
+--------------------------------------
+
+DWC3 PHY nodes are defined to describe on-chip Synopsis Physical layer
+controllers.  Each DWC3 PHY controller should have its own node.
+
+Required properties:
+- compatible: should contain one of the following:
+	- "qcom,dwc3-hs-usb-phy" for High Speed Synopsis PHY controller
+	- "qcom,dwc3-ss-usb-phy" for Super Speed Synopsis PHY controller
+- reg: offset and length of the DWC3 PHY controller register set
+- #phy-cells: must be zero
+- clocks: a list of phandles and clock-specifier pairs, one for each entry in
+  clock-names.
+- clock-names: Should contain "ref" for the PHY reference clock
+
+Optional clocks:
+  "xo"		External reference clock
+
+Example:
+		phy@100f8800 {
+			compatible = "qcom,dwc3-hs-usb-phy";
+			reg = <0x100f8800 0x30>;
+			clocks = <&gcc USB30_0_UTMI_CLK>;
+			clock-names = "ref";
+			#phy-cells = <0>;
+
+			status = "ok";
+		};
+
+		phy@100f8830 {
+			compatible = "qcom,dwc3-ss-usb-phy";
+			reg = <0x100f8830 0x30>;
+			clocks = <&gcc USB30_0_MASTER_CLK>;
+			clock-names = "ref";
+			#phy-cells = <0>;
+
+			status = "ok";
+		};
diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.txt b/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
new file mode 100644
index 0000000..ca164e7
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
@@ -0,0 +1,66 @@
+Qualcomm SuperSpeed DWC3 USB SoC controller
+
+Required properties:
+- compatible:	should contain "qcom,dwc3"
+- clocks:		A list of phandle + clock-specifier pairs for the
+				clocks listed in clock-names
+- clock-names:	Should contain the following:
+  "core"		Master/Core clock, have to be >= 125 MHz for SS
+				operation and >= 60MHz for HS operation
+
+Optional clocks:
+  "iface"		System bus AXI clock.  Not present on all platforms
+  "sleep"		Sleep clock, used when USB3 core goes into low
+				power mode (U3).
+
+Required child node:
+A child node must exist to represent the core DWC3 IP block. The name of
+the node is not important. The content of the node is defined in dwc3.txt.
+
+Phy documentation is provided in the following places:
+Documentation/devicetree/bindings/phy/qcom,dwc3-usb-phy.txt
+
+Example device nodes:
+
+		hs_phy: phy@100f8800 {
+			compatible = "qcom,dwc3-hs-usb-phy";
+			reg = <0x100f8800 0x30>;
+			clocks = <&gcc USB30_0_UTMI_CLK>;
+			clock-names = "ref";
+			#phy-cells = <0>;
+
+			status = "ok";
+		};
+
+		ss_phy: phy@100f8830 {
+			compatible = "qcom,dwc3-ss-usb-phy";
+			reg = <0x100f8830 0x30>;
+			clocks = <&gcc USB30_0_MASTER_CLK>;
+			clock-names = "ref";
+			#phy-cells = <0>;
+
+			status = "ok";
+		};
+
+		usb3_0: usb30@0 {
+			compatible = "qcom,dwc3";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			clocks = <&gcc USB30_0_MASTER_CLK>;
+			clock-names = "core";
+
+			ranges;
+
+			status = "ok";
+
+			dwc3@10000000 {
+				compatible = "snps,dwc3";
+				reg = <0x10000000 0xcd00>;
+				interrupts = <0 205 0x4>;
+				phys = <&hs_phy>, <&ss_phy>;
+				phy-names = "usb2-phy", "usb3-phy";
+				tx-fifo-resize;
+				dr_mode = "host";
+			};
+		};
+
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[Patch v9 2/3] usb: dwc3: Add Qualcomm DWC3 glue layer driver

[Andy Gross]

From: "Ivan T. Ivanov" <iivanov@mm-sol.com>

DWC3 glue layer is hardware layer around Synopsys DesignWare
USB3 core. Its purpose is to supply Synopsys IP with required
clocks, voltages and interface it with the rest of the SoC.

Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com>
Signed-off-by: Andy Gross <agross@codeaurora.org>
---
 drivers/usb/dwc3/Kconfig     |    8 +++
 drivers/usb/dwc3/Makefile    |    1 +
 drivers/usb/dwc3/dwc3-qcom.c |  131 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 140 insertions(+)
 create mode 100644 drivers/usb/dwc3/dwc3-qcom.c

diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 785510a..a816fe4 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -80,6 +80,14 @@ config USB_DWC3_KEYSTONE
 	  Support of USB2/3 functionality in TI Keystone2 platforms.
 	  Say 'Y' or 'M' here if you have one such device
 
+config USB_DWC3_QCOM
+	tristate "Qualcomm Platforms"
+	depends on ARCH_QCOM || COMPILE_TEST
+	default USB_DWC3
+	help
+	  Recent Qualcomm SoCs ship with one DesignWare Core USB3 IP inside,
+	  say 'Y' or 'M' if you have one such device.
+
 comment "Debugging features"
 
 config USB_DWC3_DEBUG
diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
index 10ac3e7..0da8e75 100644
--- a/drivers/usb/dwc3/Makefile
+++ b/drivers/usb/dwc3/Makefile
@@ -33,3 +33,4 @@ obj-$(CONFIG_USB_DWC3_OMAP)		+= dwc3-omap.o
 obj-$(CONFIG_USB_DWC3_EXYNOS)		+= dwc3-exynos.o
 obj-$(CONFIG_USB_DWC3_PCI)		+= dwc3-pci.o
 obj-$(CONFIG_USB_DWC3_KEYSTONE)		+= dwc3-keystone.o
+obj-$(CONFIG_USB_DWC3_QCOM)		+= dwc3-qcom.o
diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
new file mode 100644
index 0000000..f255335
--- /dev/null
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -0,0 +1,131 @@
+/* Copyright (c) 2013-2014, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+
+struct dwc3_qcom {
+	struct device		*dev;
+
+	struct clk		*core_clk;
+	struct clk		*iface_clk;
+	struct clk		*sleep_clk;
+};
+
+static int dwc3_qcom_probe(struct platform_device *pdev)
+{
+	struct device_node *node = pdev->dev.of_node;
+	struct dwc3_qcom *qdwc;
+	int ret = 0;
+
+	qdwc = devm_kzalloc(&pdev->dev, sizeof(*qdwc), GFP_KERNEL);
+	if (!qdwc)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, qdwc);
+
+	qdwc->dev = &pdev->dev;
+
+	qdwc->core_clk = devm_clk_get(qdwc->dev, "core");
+	if (IS_ERR(qdwc->core_clk)) {
+		dev_err(qdwc->dev, "failed to get core clock\n");
+		return PTR_ERR(qdwc->core_clk);
+	}
+
+	qdwc->iface_clk = devm_clk_get(qdwc->dev, "iface");
+	if (IS_ERR(qdwc->iface_clk)) {
+		dev_dbg(qdwc->dev, "failed to get optional iface clock\n");
+		qdwc->iface_clk = NULL;
+	}
+
+	qdwc->sleep_clk = devm_clk_get(qdwc->dev, "sleep");
+	if (IS_ERR(qdwc->sleep_clk)) {
+		dev_dbg(qdwc->dev, "failed to get optional sleep clock\n");
+		qdwc->sleep_clk = NULL;
+	}
+
+	ret = clk_prepare_enable(qdwc->core_clk);
+	if (ret) {
+		dev_err(qdwc->dev, "failed to enable core clock\n");
+		goto err_core;
+	}
+
+	ret = clk_prepare_enable(qdwc->iface_clk);
+	if (ret) {
+		dev_err(qdwc->dev, "failed to enable optional iface clock\n");
+		goto err_iface;
+	}
+
+	ret = clk_prepare_enable(qdwc->sleep_clk);
+	if (ret) {
+		dev_err(qdwc->dev, "failed to enable optional sleep clock\n");
+		goto err_sleep;
+	}
+
+	ret = of_platform_populate(node, NULL, NULL, qdwc->dev);
+	if (ret) {
+		dev_err(qdwc->dev, "failed to register core - %d\n", ret);
+		goto err_clks;
+	}
+
+	return 0;
+
+err_clks:
+	clk_disable_unprepare(qdwc->sleep_clk);
+err_sleep:
+	clk_disable_unprepare(qdwc->iface_clk);
+err_iface:
+	clk_disable_unprepare(qdwc->core_clk);
+err_core:
+	return ret;
+}
+
+static int dwc3_qcom_remove(struct platform_device *pdev)
+{
+	struct dwc3_qcom *qdwc = platform_get_drvdata(pdev);
+
+	of_platform_depopulate(&pdev->dev);
+
+	clk_disable_unprepare(qdwc->sleep_clk);
+	clk_disable_unprepare(qdwc->iface_clk);
+	clk_disable_unprepare(qdwc->core_clk);
+
+	return 0;
+}
+
+static const struct of_device_id of_dwc3_match[] = {
+	{ .compatible = "qcom,dwc3" },
+	{ /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, of_dwc3_match);
+
+static struct platform_driver dwc3_qcom_driver = {
+	.probe		= dwc3_qcom_probe,
+	.remove		= dwc3_qcom_remove,
+	.driver		= {
+		.name	= "qcom-dwc3",
+		.owner	= THIS_MODULE,
+		.of_match_table	= of_dwc3_match,
+	},
+};
+
+module_platform_driver(dwc3_qcom_driver);
+
+MODULE_ALIAS("platform:qcom-dwc3");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("DesignWare USB3 QCOM Glue Layer");
+MODULE_AUTHOR("Ivan T. Ivanov <iivanov@mm-sol.com>");
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[Patch v9 3/3] phy: Add Qualcomm DWC3 HS/SS PHY driver

[Andy Gross]

This patch adds a new driver for the Qualcomm USB 3.0 PHY that exists on some
Qualcomm platforms.  This driver uses the generic PHY framework and will
interact with the DWC3 controller.

Signed-off-by: Andy Gross <agross@codeaurora.org>
---
 drivers/phy/Kconfig         |   11 +
 drivers/phy/Makefile        |    1 +
 drivers/phy/phy-qcom-dwc3.c |  483 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 495 insertions(+)
 create mode 100644 drivers/phy/phy-qcom-dwc3.c

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 0dd7427..5d56161 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -230,4 +230,15 @@ config PHY_XGENE
 	help
 	  This option enables support for APM X-Gene SoC multi-purpose PHY.
 
+config PHY_QCOM_DWC3
+	tristate "QCOM DWC3 USB PHY support"
+	depends on ARCH_QCOM
+	depends on HAS_IOMEM
+	depends on OF
+	select GENERIC_PHY
+	help
+	  This option enables support for the Synopsis PHYs present inside the
+	  Qualcomm USB3.0 DWC3 controller.  This driver supports both HS and SS
+	  PHY controllers.
+
 endmenu
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 95c69ed..aa16f30 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -28,3 +28,4 @@ obj-$(CONFIG_PHY_QCOM_IPQ806X_SATA)	+= phy-qcom-ipq806x-sata.o
 obj-$(CONFIG_PHY_ST_SPEAR1310_MIPHY)	+= phy-spear1310-miphy.o
 obj-$(CONFIG_PHY_ST_SPEAR1340_MIPHY)	+= phy-spear1340-miphy.o
 obj-$(CONFIG_PHY_XGENE)			+= phy-xgene.o
+obj-$(CONFIG_PHY_QCOM_DWC3)		+= phy-qcom-dwc3.o
diff --git a/drivers/phy/phy-qcom-dwc3.c b/drivers/phy/phy-qcom-dwc3.c
new file mode 100644
index 0000000..2c7b316
--- /dev/null
+++ b/drivers/phy/phy-qcom-dwc3.c
@@ -0,0 +1,483 @@
+/* Copyright (c) 2013-2014, Code Aurora Forum. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/delay.h>
+
+/**
+ *  USB QSCRATCH Hardware registers
+ */
+#define QSCRATCH_GENERAL_CFG		(0x08)
+#define HSUSB_PHY_CTRL_REG		(0x10)
+
+/* PHY_CTRL_REG */
+#define HSUSB_CTRL_DMSEHV_CLAMP			BIT(24)
+#define HSUSB_CTRL_USB2_SUSPEND			BIT(23)
+#define HSUSB_CTRL_UTMI_CLK_EN			BIT(21)
+#define	HSUSB_CTRL_UTMI_OTG_VBUS_VALID		BIT(20)
+#define HSUSB_CTRL_USE_CLKCORE			BIT(18)
+#define HSUSB_CTRL_DPSEHV_CLAMP			BIT(17)
+#define HSUSB_CTRL_COMMONONN			BIT(11)
+#define HSUSB_CTRL_ID_HV_CLAMP			BIT(9)
+#define HSUSB_CTRL_OTGSESSVLD_CLAMP		BIT(8)
+#define HSUSB_CTRL_CLAMP_EN			BIT(7)
+#define HSUSB_CTRL_RETENABLEN			BIT(1)
+#define HSUSB_CTRL_POR				BIT(0)
+
+/* QSCRATCH_GENERAL_CFG */
+#define HSUSB_GCFG_XHCI_REV		BIT(2)
+
+/**
+ *  USB QSCRATCH Hardware registers
+ */
+#define SSUSB_PHY_CTRL_REG		(0x00)
+#define SSUSB_PHY_PARAM_CTRL_1		(0x04)
+#define SSUSB_PHY_PARAM_CTRL_2		(0x08)
+#define CR_PROTOCOL_DATA_IN_REG		(0x0c)
+#define CR_PROTOCOL_DATA_OUT_REG	(0x10)
+#define CR_PROTOCOL_CAP_ADDR_REG	(0x14)
+#define CR_PROTOCOL_CAP_DATA_REG	(0x18)
+#define CR_PROTOCOL_READ_REG		(0x1c)
+#define CR_PROTOCOL_WRITE_REG		(0x20)
+
+/* PHY_CTRL_REG */
+#define SSUSB_CTRL_REF_USE_PAD		BIT(28)
+#define SSUSB_CTRL_TEST_POWERDOWN	BIT(27)
+#define SSUSB_CTRL_LANE0_PWR_PRESENT	BIT(24)
+#define SSUSB_CTRL_SS_PHY_EN		BIT(8)
+#define SSUSB_CTRL_SS_PHY_RESET		BIT(7)
+
+/* SSPHY control registers */
+#define SSPHY_CTRL_RX_OVRD_IN_HI(lane)	(0x1006 + 0x100 * lane)
+#define SSPHY_CTRL_TX_OVRD_DRV_LO(lane)	(0x1002 + 0x100 * lane)
+
+/* RX OVRD IN HI bits */
+#define RX_OVRD_IN_HI_RX_RESET_OVRD		BIT(13)
+#define RX_OVRD_IN_HI_RX_RX_RESET		BIT(12)
+#define RX_OVRD_IN_HI_RX_EQ_OVRD		BIT(11)
+#define RX_OVRD_IN_HI_RX_EQ_MASK		0x0700
+#define RX_OVRD_IN_HI_RX_EQ_SHIFT		8
+#define RX_OVRD_IN_HI_RX_EQ_EN_OVRD		BIT(7)
+#define RX_OVRD_IN_HI_RX_EQ_EN			BIT(6)
+#define RX_OVRD_IN_HI_RX_LOS_FILTER_OVRD	BIT(5)
+#define RX_OVRD_IN_HI_RX_LOS_FILTER_MASK	0x0018
+#define RX_OVRD_IN_HI_RX_RATE_OVRD		BIT(2)
+#define RX_OVRD_IN_HI_RX_RATE_MASK		0x0003
+
+/* TX OVRD DRV LO register bits */
+#define TX_OVRD_DRV_LO_AMPLITUDE_MASK	0x007F
+#define TX_OVRD_DRV_LO_PREEMPH_MASK	0x3F80
+#define TX_OVRD_DRV_LO_PREEMPH_SHIFT	7
+#define TX_OVRD_DRV_LO_EN		BIT(14)
+
+struct qcom_dwc3_usb_phy {
+	void __iomem		*base;
+	struct device		*dev;
+	struct phy *phy;
+
+	int (*phy_init)(struct qcom_dwc3_usb_phy *phy_dwc3);
+	int (*phy_exit)(struct qcom_dwc3_usb_phy *phy_dwc3);
+
+	struct clk		*xo_clk;
+	struct clk		*ref_clk;
+};
+
+/**
+ * Write register and read back masked value to confirm it is written
+ *
+ * @base - QCOM DWC3 PHY base virtual address.
+ * @offset - register offset.
+ * @mask - register bitmask specifying what should be updated
+ * @val - value to write.
+ */
+static inline void qcom_dwc3_phy_write_readback(
+	struct qcom_dwc3_usb_phy *phy_dwc3, u32 offset,
+	const u32 mask, u32 val)
+{
+	u32 write_val, tmp = readl(phy_dwc3->base + offset);
+
+	tmp &= ~mask;		/* retain other bits */
+	write_val = tmp | val;
+
+	writel(write_val, phy_dwc3->base + offset);
+
+	/* Read back to see if val was written */
+	tmp = readl(phy_dwc3->base + offset);
+	tmp &= mask;		/* clear other bits */
+
+	if (tmp != val)
+		dev_err(phy_dwc3->dev, "write: %x to QSCRATCH: %x FAILED\n",
+			val, offset);
+}
+
+static int wait_for_latch(void __iomem *addr)
+{
+	u32 retry = 10;
+
+	while (true) {
+		if (!readl(addr))
+			break;
+
+		if (--retry == 0)
+			return -ETIMEDOUT;
+
+		usleep_range(10, 20);
+	}
+
+	return 0;
+}
+
+/**
+ * Write SSPHY register
+ *
+ * @base - QCOM DWC3 PHY base virtual address.
+ * @addr - SSPHY address to write.
+ * @val - value to write.
+ */
+static int qcom_dwc3_ss_write_phycreg(void __iomem *base, u32 addr, u32 val)
+{
+	int ret;
+
+	writel(addr, base + CR_PROTOCOL_DATA_IN_REG);
+	writel(0x1, base + CR_PROTOCOL_CAP_ADDR_REG);
+
+	ret = wait_for_latch(base + CR_PROTOCOL_CAP_ADDR_REG);
+	if (ret)
+		goto err_wait;
+
+	writel(val, base + CR_PROTOCOL_DATA_IN_REG);
+	writel(0x1, base + CR_PROTOCOL_CAP_DATA_REG);
+
+	ret = wait_for_latch(base + CR_PROTOCOL_CAP_DATA_REG);
+	if (ret)
+		goto err_wait;
+
+	writel(0x1, base + CR_PROTOCOL_WRITE_REG);
+
+	ret = wait_for_latch(base + CR_PROTOCOL_WRITE_REG);
+
+err_wait:
+	return ret;
+}
+
+/**
+ * Read SSPHY register.
+ *
+ * @base - QCOM DWC3 PHY base virtual address.
+ * @addr - SSPHY address to read.
+ */
+static int qcom_dwc3_ss_read_phycreg(void __iomem *base, u32 addr, u32 *val)
+{
+	int ret;
+	bool first_read = true;
+
+	writel(addr, base + CR_PROTOCOL_DATA_IN_REG);
+	writel(0x1, base + CR_PROTOCOL_CAP_ADDR_REG);
+
+	ret = wait_for_latch(base + CR_PROTOCOL_CAP_ADDR_REG);
+	if (ret)
+		goto err_wait;
+
+	/*
+	 * Due to hardware bug, first read of SSPHY register might be
+	 * incorrect. Hence as workaround, SW should perform SSPHY register
+	 * read twice, but use only second read and ignore first read.
+	 */
+retry:
+	writel(0x1, base + CR_PROTOCOL_READ_REG);
+
+	ret = wait_for_latch(base + CR_PROTOCOL_READ_REG);
+	if (ret)
+		goto err_wait;
+
+	if (first_read) {
+		readl(base + CR_PROTOCOL_DATA_OUT_REG);
+		first_read = false;
+		goto retry;
+	}
+
+	*val = readl(base + CR_PROTOCOL_DATA_OUT_REG);
+
+err_wait:
+	return ret;
+}
+
+static int qcom_dwc3_phy_power_on(struct phy *phy)
+{
+	int ret;
+	struct qcom_dwc3_usb_phy *phy_dwc3 = phy_get_drvdata(phy);
+
+	ret = clk_prepare_enable(phy_dwc3->xo_clk);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(phy_dwc3->ref_clk);
+	if (ret)
+		clk_disable_unprepare(phy_dwc3->xo_clk);
+
+	return ret;
+}
+
+static int qcom_dwc3_phy_power_off(struct phy *phy)
+{
+	struct qcom_dwc3_usb_phy *phy_dwc3 = phy_get_drvdata(phy);
+
+	clk_disable_unprepare(phy_dwc3->ref_clk);
+	clk_disable_unprepare(phy_dwc3->xo_clk);
+
+	return 0;
+}
+
+static int qcom_dwc3_hs_phy_init(struct qcom_dwc3_usb_phy *phy_dwc3)
+{
+	u32 val;
+
+	/*
+	 * HSPHY Initialization: Enable UTMI clock, select 19.2MHz fsel
+	 * enable clamping, and disable RETENTION (power-on default is ENABLED)
+	 */
+	val = HSUSB_CTRL_DPSEHV_CLAMP | HSUSB_CTRL_DMSEHV_CLAMP |
+		HSUSB_CTRL_RETENABLEN  | HSUSB_CTRL_COMMONONN |
+		HSUSB_CTRL_OTGSESSVLD_CLAMP | HSUSB_CTRL_ID_HV_CLAMP |
+		HSUSB_CTRL_DPSEHV_CLAMP | HSUSB_CTRL_UTMI_OTG_VBUS_VALID |
+		HSUSB_CTRL_UTMI_CLK_EN | HSUSB_CTRL_CLAMP_EN | 0x70;
+
+	/* use core clock if external reference is not present */
+	if (!phy_dwc3->xo_clk)
+		val |= HSUSB_CTRL_USE_CLKCORE;
+
+	writel(val, phy_dwc3->base + HSUSB_PHY_CTRL_REG);
+	usleep_range(2000, 2200);
+
+	/* Disable (bypass) VBUS and ID filters */
+	writel(HSUSB_GCFG_XHCI_REV, phy_dwc3->base + QSCRATCH_GENERAL_CFG);
+
+	return 0;
+}
+
+static int qcom_dwc3_ss_phy_init(struct qcom_dwc3_usb_phy *phy_dwc3)
+{
+	int ret;
+	u32 data = 0;
+
+	/* reset phy */
+	data = readl_relaxed(phy_dwc3->base + SSUSB_PHY_CTRL_REG);
+	writel_relaxed(data | SSUSB_CTRL_SS_PHY_RESET,
+		phy_dwc3->base + SSUSB_PHY_CTRL_REG);
+	usleep_range(2000, 2200);
+	writel_relaxed(data, phy_dwc3->base + SSUSB_PHY_CTRL_REG);
+
+	/* clear REF_PAD if we don't have XO clk */
+	if (!phy_dwc3->xo_clk)
+		data &= ~SSUSB_CTRL_REF_USE_PAD;
+	else
+		data |= SSUSB_CTRL_REF_USE_PAD;
+
+	writel_relaxed(data, phy_dwc3->base + SSUSB_PHY_CTRL_REG);
+	msleep(30);
+
+	data |= SSUSB_CTRL_SS_PHY_EN | SSUSB_CTRL_LANE0_PWR_PRESENT;
+	writel_relaxed(data, phy_dwc3->base + SSUSB_PHY_CTRL_REG);
+
+	/*
+	 * Fix RX Equalization setting as follows
+	 * LANE0.RX_OVRD_IN_HI. RX_EQ_EN set to 0
+	 * LANE0.RX_OVRD_IN_HI.RX_EQ_EN_OVRD set to 1
+	 * LANE0.RX_OVRD_IN_HI.RX_EQ set to 3
+	 * LANE0.RX_OVRD_IN_HI.RX_EQ_OVRD set to 1
+	 */
+	ret = qcom_dwc3_ss_read_phycreg(phy_dwc3->base,
+			SSPHY_CTRL_RX_OVRD_IN_HI(0), &data);
+	if (ret)
+		goto err_phy_trans;
+
+	data &= ~RX_OVRD_IN_HI_RX_EQ_EN;
+	data |= RX_OVRD_IN_HI_RX_EQ_EN_OVRD;
+	data &= ~RX_OVRD_IN_HI_RX_EQ_MASK;
+	data |= 0x3 << RX_OVRD_IN_HI_RX_EQ_SHIFT;
+	data |= RX_OVRD_IN_HI_RX_EQ_OVRD;
+	ret = qcom_dwc3_ss_write_phycreg(phy_dwc3->base,
+		SSPHY_CTRL_RX_OVRD_IN_HI(0), data);
+	if (ret)
+		goto err_phy_trans;
+
+	/*
+	 * Set EQ and TX launch amplitudes as follows
+	 * LANE0.TX_OVRD_DRV_LO.PREEMPH set to 22
+	 * LANE0.TX_OVRD_DRV_LO.AMPLITUDE set to 127
+	 * LANE0.TX_OVRD_DRV_LO.EN set to 1.
+	 */
+	ret = qcom_dwc3_ss_read_phycreg(phy_dwc3->base,
+		SSPHY_CTRL_TX_OVRD_DRV_LO(0), &data);
+	if (ret)
+		goto err_phy_trans;
+
+	data &= ~TX_OVRD_DRV_LO_PREEMPH_MASK;
+	data |= 0x16 << TX_OVRD_DRV_LO_PREEMPH_SHIFT;
+	data &= ~TX_OVRD_DRV_LO_AMPLITUDE_MASK;
+	data |= 0x7f;
+	data |= TX_OVRD_DRV_LO_EN;
+	ret = qcom_dwc3_ss_write_phycreg(phy_dwc3->base,
+		SSPHY_CTRL_TX_OVRD_DRV_LO(0), data);
+	if (ret)
+		goto err_phy_trans;
+
+	/*
+	 * Set the QSCRATCH PHY_PARAM_CTRL1 parameters as follows
+	 * TX_FULL_SWING [26:20] amplitude to 127
+	 * TX_DEEMPH_3_5DB [13:8] to 22
+	 * LOS_BIAS [2:0] to 0x5
+	 */
+	qcom_dwc3_phy_write_readback(phy_dwc3, SSUSB_PHY_PARAM_CTRL_1,
+				   0x07f03f07, 0x07f01605);
+
+err_phy_trans:
+	return ret;
+}
+
+static int qcom_dwc3_ss_phy_exit(struct qcom_dwc3_usb_phy *phy_dwc3)
+{
+	/* Sequence to put SSPHY in low power state:
+	 * 1. Clear REF_PHY_EN in PHY_CTRL_REG
+	 * 2. Clear REF_USE_PAD in PHY_CTRL_REG
+	 * 3. Set TEST_POWERED_DOWN in PHY_CTRL_REG to enable PHY retention
+	 * 4. Disable SSPHY ref clk
+	 */
+	qcom_dwc3_phy_write_readback(phy_dwc3, SSUSB_PHY_CTRL_REG,
+		SSUSB_CTRL_SS_PHY_EN, 0x0);
+	qcom_dwc3_phy_write_readback(phy_dwc3, SSUSB_PHY_CTRL_REG,
+		SSUSB_CTRL_REF_USE_PAD, 0x0);
+	qcom_dwc3_phy_write_readback(phy_dwc3, SSUSB_PHY_CTRL_REG,
+		0x0, SSUSB_CTRL_TEST_POWERDOWN);
+
+	return 0;
+}
+
+static int qcom_dwc3_phy_init(struct phy *phy)
+{
+	struct qcom_dwc3_usb_phy *phy_dwc3 = phy_get_drvdata(phy);
+
+	if (phy_dwc3->phy_init)
+		return phy_dwc3->phy_init(phy_dwc3);
+
+	return 0;
+}
+
+static int qcom_dwc3_phy_exit(struct phy *phy)
+{
+	struct qcom_dwc3_usb_phy *phy_dwc3 = phy_get_drvdata(phy);
+
+	if (phy_dwc3->phy_exit)
+		return qcom_dwc3_ss_phy_exit(phy_dwc3);
+
+	return 0;
+}
+
+static struct phy_ops qcom_dwc3_phy_ops = {
+	.init		= qcom_dwc3_phy_init,
+	.exit		= qcom_dwc3_phy_exit,
+	.power_on	= qcom_dwc3_phy_power_on,
+	.power_off	= qcom_dwc3_phy_power_off,
+	.owner		= THIS_MODULE,
+};
+
+static const struct of_device_id qcom_dwc3_phy_table[] = {
+	{ .compatible = "qcom,dwc3-hs-usb-phy", },
+	{ .compatible = "qcom,dwc3-ss-usb-phy", },
+	{ /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, qcom_dwc3_phy_table);
+
+static int qcom_dwc3_phy_probe(struct platform_device *pdev)
+{
+	struct qcom_dwc3_usb_phy	*phy_dwc3;
+	struct phy_provider		*phy_provider;
+	struct resource			*res;
+
+	phy_dwc3 = devm_kzalloc(&pdev->dev, sizeof(*phy_dwc3), GFP_KERNEL);
+	if (!phy_dwc3)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, phy_dwc3);
+
+	phy_dwc3->dev = &pdev->dev;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	phy_dwc3->base = devm_ioremap_resource(phy_dwc3->dev, res);
+	if (IS_ERR(phy_dwc3->base))
+		return PTR_ERR(phy_dwc3->base);
+
+	phy_dwc3->ref_clk = devm_clk_get(phy_dwc3->dev, "ref");
+	if (IS_ERR(phy_dwc3->ref_clk)) {
+		dev_dbg(phy_dwc3->dev, "cannot get reference clock\n");
+		return PTR_ERR(phy_dwc3->ref_clk);
+	}
+
+	if (of_device_is_compatible(pdev->dev.of_node,
+			"qcom,dwc3-hs-usb-phy")) {
+		clk_set_rate(phy_dwc3->ref_clk, 60000000);
+		phy_dwc3->phy_init = qcom_dwc3_hs_phy_init;
+	} else if (of_device_is_compatible(pdev->dev.of_node,
+			"qcom,dwc3-ss-usb-phy")) {
+		phy_dwc3->phy_init = qcom_dwc3_ss_phy_init;
+		phy_dwc3->phy_exit = qcom_dwc3_ss_phy_exit;
+		clk_set_rate(phy_dwc3->ref_clk, 125000000);
+	} else {
+		dev_err(phy_dwc3->dev, "Unknown phy\n");
+		return -EINVAL;
+	}
+
+	phy_dwc3->xo_clk = devm_clk_get(phy_dwc3->dev, "xo");
+	if (IS_ERR(phy_dwc3->xo_clk)) {
+		dev_dbg(phy_dwc3->dev, "cannot get TCXO clock\n");
+		phy_dwc3->xo_clk = NULL;
+	}
+
+	phy_dwc3->phy = devm_phy_create(phy_dwc3->dev, NULL, &qcom_dwc3_phy_ops,
+		NULL);
+
+	if (IS_ERR(phy_dwc3->phy))
+		return PTR_ERR(phy_dwc3->phy);
+
+	phy_set_drvdata(phy_dwc3->phy, phy_dwc3);
+
+	phy_provider = devm_of_phy_provider_register(phy_dwc3->dev,
+			of_phy_simple_xlate);
+
+	if (IS_ERR(phy_provider))
+		return PTR_ERR(phy_provider);
+
+	return 0;
+}
+
+static struct platform_driver qcom_dwc3_phy_driver = {
+	.probe		= qcom_dwc3_phy_probe,
+	.driver		= {
+		.name	= "qcom-dwc3-usb-phy",
+		.owner	= THIS_MODULE,
+		.of_match_table = qcom_dwc3_phy_table,
+	},
+};
+
+module_platform_driver(qcom_dwc3_phy_driver);
+
+MODULE_ALIAS("platform:phy-qcom-dwc3");
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Andy Gross <agross@codeaurora.org>");
+MODULE_AUTHOR("Ivan T. Ivanov <iivanov@mm-sol.com>");
+MODULE_DESCRIPTION("DesignWare USB3 QCOM PHY driver");
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

Re: [Patch v9 2/3] usb: dwc3: Add Qualcomm DWC3 glue layer driver

[Felipe Balbi]

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

On Sat, Sep 13, 2014 at 01:44:25AM +0530, Pramod Gurav wrote:
> Andy,
> Couple of minor comments.
> 
> On Sat, Sep 13, 2014 at 12:58 AM, Andy Gross <agross@codeaurora.org> wrote:
> 
> > From: "Ivan T. Ivanov" <iivanov@mm-sol.com>
> >
> > DWC3 glue layer is hardware layer around Synopsys DesignWare
> > USB3 core. Its purpose is to supply Synopsys IP with required
> > clocks, voltages and interface it with the rest of the SoC.
> >
> > Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com>
> > Signed-off-by: Andy Gross <agross@codeaurora.org>
> > ---
> >  drivers/usb/dwc3/Kconfig     |    8 +++
> >  drivers/usb/dwc3/Makefile    |    1 +
> >  drivers/usb/dwc3/dwc3-qcom.c |  131
> > ++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 140 insertions(+)
> >  create mode 100644 drivers/usb/dwc3/dwc3-qcom.c
> >
> >
> <..>
> 
> 
> > +#include <linux/platform_device.h>
> > +
> > +struct dwc3_qcom {
> > +       struct device           *dev;
> > +
> >
> Extra new line here.

that's not an issue however.

> > +       struct clk              *core_clk;
> > +       struct clk              *iface_clk;
> > +       struct clk              *sleep_clk;
> > +};
> > +
> > +static int dwc3_qcom_probe(struct platform_device *pdev)
> > +{
> > +       struct device_node *node = pdev->dev.of_node;
> > +       struct dwc3_qcom *qdwc;
> > +       int ret = 0;
> >
> Initialization not required.

I'll fix this one as I'm already applying this patch.

> > +
> > +       qdwc = devm_kzalloc(&pdev->dev, sizeof(*qdwc), GFP_KERNEL);
> > +       if (!qdwc)
> > +               return -ENOMEM;
> > +
> > +       platform_set_drvdata(pdev, qdwc);
> > +
> > +       qdwc->dev = &pdev->dev;
> > +
> > +       qdwc->core_clk = devm_clk_get(qdwc->dev, "core");
> > +       if (IS_ERR(qdwc->core_clk)) {
> > +               dev_err(qdwc->dev, "failed to get core clock\n");
> > +               return PTR_ERR(qdwc->core_clk);
> > +       }
> > +
> > +       qdwc->iface_clk = devm_clk_get(qdwc->dev, "iface");
> > +       if (IS_ERR(qdwc->iface_clk)) {
> > +               dev_dbg(qdwc->dev, "failed to get optional iface clock\n");
> > +               qdwc->iface_clk = NULL;
> > +       }
> > +
> > +       qdwc->sleep_clk = devm_clk_get(qdwc->dev, "sleep");
> > +       if (IS_ERR(qdwc->sleep_clk)) {
> > +               dev_dbg(qdwc->dev, "failed to get optional sleep clock\n");
> > +               qdwc->sleep_clk = NULL;
> > +       }
> > +
> > +       ret = clk_prepare_enable(qdwc->core_clk);
> > +       if (ret) {
> > +               dev_err(qdwc->dev, "failed to enable core clock\n");
> > +               goto err_core;
> > +       }
> > +
> > +       ret = clk_prepare_enable(qdwc->iface_clk);
> >
> Should not we check if  qdwc->iface_clk is valid?

read the sources luke.

> > +err_clks:
> > +       clk_disable_unprepare(qdwc->sleep_clk);
> >
> IS_ERR check before above statement not needed as we have continued with
> probe even after failure og devm_clk_get?

read more carefully, there's a detail which you're missing.

-- 
balbi

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

Re: [Patch v9 2/3] usb: dwc3: Add Qualcomm DWC3 glue layer driver

[Pramod Gurav]

Hi Felipe,

On 13-09-2014 01:50 AM, Felipe Balbi wrote:
> On Sat, Sep 13, 2014 at 01:44:25AM +0530, Pramod Gurav wrote:
>> Andy,
>> Couple of minor comments.
>>
>> On Sat, Sep 13, 2014 at 12:58 AM, Andy Gross <agross@codeaurora.org> wrote:
>>
>>> From: "Ivan T. Ivanov" <iivanov@mm-sol.com>
>>>
>>> DWC3 glue layer is hardware layer around Synopsys DesignWare
>>> USB3 core. Its purpose is to supply Synopsys IP with required
>>> clocks, voltages and interface it with the rest of the SoC.
>>>
>>> Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com>
>>> Signed-off-by: Andy Gross <agross@codeaurora.org>
>>> ---
>>>  drivers/usb/dwc3/Kconfig     |    8 +++
>>>  drivers/usb/dwc3/Makefile    |    1 +
>>>  drivers/usb/dwc3/dwc3-qcom.c |  131
>>> ++++++++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 140 insertions(+)
>>>  create mode 100644 drivers/usb/dwc3/dwc3-qcom.c
>>>
>>>
>> <..>
>>
>>
>>> +#include <linux/platform_device.h>
>>> +
>>> +struct dwc3_qcom {
>>> +       struct device           *dev;
>>> +
>>>
>> Extra new line here.
> 
> that's not an issue however.
> 
>>> +       struct clk              *core_clk;
>>> +       struct clk              *iface_clk;
>>> +       struct clk              *sleep_clk;
>>> +};
>>> +
>>> +static int dwc3_qcom_probe(struct platform_device *pdev)
>>> +{
>>> +       struct device_node *node = pdev->dev.of_node;
>>> +       struct dwc3_qcom *qdwc;
>>> +       int ret = 0;
>>>
>> Initialization not required.
> 
> I'll fix this one as I'm already applying this patch.
> 
>>> +
>>> +       qdwc = devm_kzalloc(&pdev->dev, sizeof(*qdwc), GFP_KERNEL);
>>> +       if (!qdwc)
>>> +               return -ENOMEM;
>>> +
>>> +       platform_set_drvdata(pdev, qdwc);
>>> +
>>> +       qdwc->dev = &pdev->dev;
>>> +
>>> +       qdwc->core_clk = devm_clk_get(qdwc->dev, "core");
>>> +       if (IS_ERR(qdwc->core_clk)) {
>>> +               dev_err(qdwc->dev, "failed to get core clock\n");
>>> +               return PTR_ERR(qdwc->core_clk);
>>> +       }
>>> +
>>> +       qdwc->iface_clk = devm_clk_get(qdwc->dev, "iface");
>>> +       if (IS_ERR(qdwc->iface_clk)) {
>>> +               dev_dbg(qdwc->dev, "failed to get optional iface clock\n");
>>> +               qdwc->iface_clk = NULL;
>>> +       }
>>> +
>>> +       qdwc->sleep_clk = devm_clk_get(qdwc->dev, "sleep");
>>> +       if (IS_ERR(qdwc->sleep_clk)) {
>>> +               dev_dbg(qdwc->dev, "failed to get optional sleep clock\n");
>>> +               qdwc->sleep_clk = NULL;
>>> +       }
>>> +
>>> +       ret = clk_prepare_enable(qdwc->core_clk);
>>> +       if (ret) {
>>> +               dev_err(qdwc->dev, "failed to enable core clock\n");
>>> +               goto err_core;
>>> +       }
>>> +
>>> +       ret = clk_prepare_enable(qdwc->iface_clk);
>>>
>> Should not we check if  qdwc->iface_clk is valid?
> 
> read the sources luke.
Now I read that its initialized to NULL in fail case but should we call
prepare_enable at all if its NULL?
> 
>>> +err_clks:
>>> +       clk_disable_unprepare(qdwc->sleep_clk);
>>>
>> IS_ERR check before above statement not needed as we have continued with
>> probe even after failure og devm_clk_get?
> 
> read more carefully, there's a detail which you're missing.
> 

Re: [Patch v9 2/3] usb: dwc3: Add Qualcomm DWC3 glue layer driver

[Felipe Balbi]

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

Hi,

On Sat, Sep 13, 2014 at 01:55:50AM +0530, Pramod Gurav wrote:
> >>> +       qdwc = devm_kzalloc(&pdev->dev, sizeof(*qdwc), GFP_KERNEL);
> >>> +       if (!qdwc)
> >>> +               return -ENOMEM;
> >>> +
> >>> +       platform_set_drvdata(pdev, qdwc);
> >>> +
> >>> +       qdwc->dev = &pdev->dev;
> >>> +
> >>> +       qdwc->core_clk = devm_clk_get(qdwc->dev, "core");
> >>> +       if (IS_ERR(qdwc->core_clk)) {
> >>> +               dev_err(qdwc->dev, "failed to get core clock\n");
> >>> +               return PTR_ERR(qdwc->core_clk);
> >>> +       }
> >>> +
> >>> +       qdwc->iface_clk = devm_clk_get(qdwc->dev, "iface");
> >>> +       if (IS_ERR(qdwc->iface_clk)) {
> >>> +               dev_dbg(qdwc->dev, "failed to get optional iface clock\n");
> >>> +               qdwc->iface_clk = NULL;
> >>> +       }
> >>> +
> >>> +       qdwc->sleep_clk = devm_clk_get(qdwc->dev, "sleep");
> >>> +       if (IS_ERR(qdwc->sleep_clk)) {
> >>> +               dev_dbg(qdwc->dev, "failed to get optional sleep clock\n");
> >>> +               qdwc->sleep_clk = NULL;
> >>> +       }
> >>> +
> >>> +       ret = clk_prepare_enable(qdwc->core_clk);
> >>> +       if (ret) {
> >>> +               dev_err(qdwc->dev, "failed to enable core clock\n");
> >>> +               goto err_core;
> >>> +       }
> >>> +
> >>> +       ret = clk_prepare_enable(qdwc->iface_clk);
> >>>
> >> Should not we check if  qdwc->iface_clk is valid?
> > 
> > read the sources luke.
> Now I read that its initialized to NULL in fail case but should we call
> prepare_enable at all if its NULL?

now read the source of clk_enable() and clk_prepare() ;-) NULL is a
valid clock, it just returns 0. This is better than sprinkling IS_ERR()
all over the place.

-- 
balbi

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

Re: [Patch v9 2/3] usb: dwc3: Add Qualcomm DWC3 glue layer driver

[Pramod Gurav]

On 13-09-2014 01:59 AM, Felipe Balbi wrote:
> Hi,
> 
> On Sat, Sep 13, 2014 at 01:55:50AM +0530, Pramod Gurav wrote:
>>>>> +       qdwc = devm_kzalloc(&pdev->dev, sizeof(*qdwc), GFP_KERNEL);
>>>>> +       if (!qdwc)
>>>>> +               return -ENOMEM;
>>>>> +
>>>>> +       platform_set_drvdata(pdev, qdwc);
>>>>> +
>>>>> +       qdwc->dev = &pdev->dev;
>>>>> +
>>>>> +       qdwc->core_clk = devm_clk_get(qdwc->dev, "core");
>>>>> +       if (IS_ERR(qdwc->core_clk)) {
>>>>> +               dev_err(qdwc->dev, "failed to get core clock\n");
>>>>> +               return PTR_ERR(qdwc->core_clk);
>>>>> +       }
>>>>> +
>>>>> +       qdwc->iface_clk = devm_clk_get(qdwc->dev, "iface");
>>>>> +       if (IS_ERR(qdwc->iface_clk)) {
>>>>> +               dev_dbg(qdwc->dev, "failed to get optional iface clock\n");
>>>>> +               qdwc->iface_clk = NULL;
>>>>> +       }
>>>>> +
>>>>> +       qdwc->sleep_clk = devm_clk_get(qdwc->dev, "sleep");
>>>>> +       if (IS_ERR(qdwc->sleep_clk)) {
>>>>> +               dev_dbg(qdwc->dev, "failed to get optional sleep clock\n");
>>>>> +               qdwc->sleep_clk = NULL;
>>>>> +       }
>>>>> +
>>>>> +       ret = clk_prepare_enable(qdwc->core_clk);
>>>>> +       if (ret) {
>>>>> +               dev_err(qdwc->dev, "failed to enable core clock\n");
>>>>> +               goto err_core;
>>>>> +       }
>>>>> +
>>>>> +       ret = clk_prepare_enable(qdwc->iface_clk);
>>>>>
>>>> Should not we check if  qdwc->iface_clk is valid?
>>>
>>> read the sources luke.
>> Now I read that its initialized to NULL in fail case but should we call
>> prepare_enable at all if its NULL?
> 
> now read the source of clk_enable() and clk_prepare() ;-) NULL is a
> valid clock, it just returns 0. This is better than sprinkling IS_ERR()
> all over the place.
Seen that. I was wrong about IS_ERR. Thanks. :)
> 

Re: [Patch v9 3/3] phy: Add Qualcomm DWC3 HS/SS PHY driver

[Kishon Vijay Abraham I]

Hi,

On Saturday 13 September 2014 12:58 AM, Andy Gross wrote:
> This patch adds a new driver for the Qualcomm USB 3.0 PHY that exists on some
> Qualcomm platforms.  This driver uses the generic PHY framework and will
> interact with the DWC3 controller.

Do you have dt documentation for this driver?
> 
> Signed-off-by: Andy Gross <agross@codeaurora.org>
> ---
>  drivers/phy/Kconfig         |   11 +
>  drivers/phy/Makefile        |    1 +
>  drivers/phy/phy-qcom-dwc3.c |  483 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 495 insertions(+)
>  create mode 100644 drivers/phy/phy-qcom-dwc3.c
> 
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 0dd7427..5d56161 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -230,4 +230,15 @@ config PHY_XGENE
>  	help
>  	  This option enables support for APM X-Gene SoC multi-purpose PHY.
>  
> +config PHY_QCOM_DWC3
> +	tristate "QCOM DWC3 USB PHY support"
> +	depends on ARCH_QCOM
> +	depends on HAS_IOMEM
> +	depends on OF
> +	select GENERIC_PHY
> +	help
> +	  This option enables support for the Synopsis PHYs present inside the
> +	  Qualcomm USB3.0 DWC3 controller.  This driver supports both HS and SS
> +	  PHY controllers.
> +
>  endmenu
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 95c69ed..aa16f30 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -28,3 +28,4 @@ obj-$(CONFIG_PHY_QCOM_IPQ806X_SATA)	+= phy-qcom-ipq806x-sata.o
>  obj-$(CONFIG_PHY_ST_SPEAR1310_MIPHY)	+= phy-spear1310-miphy.o
>  obj-$(CONFIG_PHY_ST_SPEAR1340_MIPHY)	+= phy-spear1340-miphy.o
>  obj-$(CONFIG_PHY_XGENE)			+= phy-xgene.o
> +obj-$(CONFIG_PHY_QCOM_DWC3)		+= phy-qcom-dwc3.o
> diff --git a/drivers/phy/phy-qcom-dwc3.c b/drivers/phy/phy-qcom-dwc3.c
> new file mode 100644
> index 0000000..2c7b316
> --- /dev/null
> +++ b/drivers/phy/phy-qcom-dwc3.c
> @@ -0,0 +1,483 @@
> +/* Copyright (c) 2013-2014, Code Aurora Forum. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/delay.h>
> +
> +/**
> + *  USB QSCRATCH Hardware registers
> + */
> +#define QSCRATCH_GENERAL_CFG		(0x08)
> +#define HSUSB_PHY_CTRL_REG		(0x10)
> +
> +/* PHY_CTRL_REG */
> +#define HSUSB_CTRL_DMSEHV_CLAMP			BIT(24)
> +#define HSUSB_CTRL_USB2_SUSPEND			BIT(23)
> +#define HSUSB_CTRL_UTMI_CLK_EN			BIT(21)
> +#define	HSUSB_CTRL_UTMI_OTG_VBUS_VALID		BIT(20)
	  ^^^^
alignment went wrong here..

> +#define HSUSB_CTRL_USE_CLKCORE			BIT(18)
> +#define HSUSB_CTRL_DPSEHV_CLAMP			BIT(17)
> +#define HSUSB_CTRL_COMMONONN			BIT(11)
> +#define HSUSB_CTRL_ID_HV_CLAMP			BIT(9)
> +#define HSUSB_CTRL_OTGSESSVLD_CLAMP		BIT(8)
> +#define HSUSB_CTRL_CLAMP_EN			BIT(7)
> +#define HSUSB_CTRL_RETENABLEN			BIT(1)
> +#define HSUSB_CTRL_POR				BIT(0)
> +
> +/* QSCRATCH_GENERAL_CFG */
> +#define HSUSB_GCFG_XHCI_REV		BIT(2)
> +
> +/**
> + *  USB QSCRATCH Hardware registers
> + */
> +#define SSUSB_PHY_CTRL_REG		(0x00)
> +#define SSUSB_PHY_PARAM_CTRL_1		(0x04)
> +#define SSUSB_PHY_PARAM_CTRL_2		(0x08)
> +#define CR_PROTOCOL_DATA_IN_REG		(0x0c)
> +#define CR_PROTOCOL_DATA_OUT_REG	(0x10)
> +#define CR_PROTOCOL_CAP_ADDR_REG	(0x14)
> +#define CR_PROTOCOL_CAP_DATA_REG	(0x18)
> +#define CR_PROTOCOL_READ_REG		(0x1c)
> +#define CR_PROTOCOL_WRITE_REG		(0x20)
> +
> +/* PHY_CTRL_REG */
> +#define SSUSB_CTRL_REF_USE_PAD		BIT(28)
> +#define SSUSB_CTRL_TEST_POWERDOWN	BIT(27)
> +#define SSUSB_CTRL_LANE0_PWR_PRESENT	BIT(24)
> +#define SSUSB_CTRL_SS_PHY_EN		BIT(8)
> +#define SSUSB_CTRL_SS_PHY_RESET		BIT(7)
> +
> +/* SSPHY control registers */
> +#define SSPHY_CTRL_RX_OVRD_IN_HI(lane)	(0x1006 + 0x100 * lane)
> +#define SSPHY_CTRL_TX_OVRD_DRV_LO(lane)	(0x1002 + 0x100 * lane)
> +
> +/* RX OVRD IN HI bits */
> +#define RX_OVRD_IN_HI_RX_RESET_OVRD		BIT(13)
> +#define RX_OVRD_IN_HI_RX_RX_RESET		BIT(12)
> +#define RX_OVRD_IN_HI_RX_EQ_OVRD		BIT(11)
> +#define RX_OVRD_IN_HI_RX_EQ_MASK		0x0700
> +#define RX_OVRD_IN_HI_RX_EQ_SHIFT		8
> +#define RX_OVRD_IN_HI_RX_EQ_EN_OVRD		BIT(7)
> +#define RX_OVRD_IN_HI_RX_EQ_EN			BIT(6)
> +#define RX_OVRD_IN_HI_RX_LOS_FILTER_OVRD	BIT(5)
> +#define RX_OVRD_IN_HI_RX_LOS_FILTER_MASK	0x0018
> +#define RX_OVRD_IN_HI_RX_RATE_OVRD		BIT(2)
> +#define RX_OVRD_IN_HI_RX_RATE_MASK		0x0003
> +
> +/* TX OVRD DRV LO register bits */
> +#define TX_OVRD_DRV_LO_AMPLITUDE_MASK	0x007F
> +#define TX_OVRD_DRV_LO_PREEMPH_MASK	0x3F80
> +#define TX_OVRD_DRV_LO_PREEMPH_SHIFT	7
> +#define TX_OVRD_DRV_LO_EN		BIT(14)
> +
> +struct qcom_dwc3_usb_phy {
> +	void __iomem		*base;
> +	struct device		*dev;
> +	struct phy *phy;
		  ^^
Pls keep the alignment same for all the members..
> +
> +	int (*phy_init)(struct qcom_dwc3_usb_phy *phy_dwc3);
> +	int (*phy_exit)(struct qcom_dwc3_usb_phy *phy_dwc3);
> +
> +	struct clk		*xo_clk;
> +	struct clk		*ref_clk;
> +};
> +
> +/**
> + * Write register and read back masked value to confirm it is written
> + *
> + * @base - QCOM DWC3 PHY base virtual address.
> + * @offset - register offset.
> + * @mask - register bitmask specifying what should be updated
> + * @val - value to write.
> + */
> +static inline void qcom_dwc3_phy_write_readback(
> +	struct qcom_dwc3_usb_phy *phy_dwc3, u32 offset,
> +	const u32 mask, u32 val)
> +{
> +	u32 write_val, tmp = readl(phy_dwc3->base + offset);
> +
> +	tmp &= ~mask;		/* retain other bits */
> +	write_val = tmp | val;
> +
> +	writel(write_val, phy_dwc3->base + offset);
> +
> +	/* Read back to see if val was written */

Does it fail sometime? I'm not sure if this should be present in the driver
since this looks more of a debug code.
> +	tmp = readl(phy_dwc3->base + offset);
> +	tmp &= mask;		/* clear other bits */
> +
> +	if (tmp != val)
> +		dev_err(phy_dwc3->dev, "write: %x to QSCRATCH: %x FAILED\n",
> +			val, offset);
> +}
> +
> +static int wait_for_latch(void __iomem *addr)
> +{
> +	u32 retry = 10;
> +
> +	while (true) {
> +		if (!readl(addr))
> +			break;
> +
> +		if (--retry == 0)
> +			return -ETIMEDOUT;
> +
> +		usleep_range(10, 20);
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * Write SSPHY register
> + *
> + * @base - QCOM DWC3 PHY base virtual address.
> + * @addr - SSPHY address to write.
> + * @val - value to write.
> + */
> +static int qcom_dwc3_ss_write_phycreg(void __iomem *base, u32 addr, u32 val)
> +{
> +	int ret;
> +
> +	writel(addr, base + CR_PROTOCOL_DATA_IN_REG);
> +	writel(0x1, base + CR_PROTOCOL_CAP_ADDR_REG);

use macros here and below..
> +
> +	ret = wait_for_latch(base + CR_PROTOCOL_CAP_ADDR_REG);
> +	if (ret)
> +		goto err_wait;

a timed out error message here?
> +
> +	writel(val, base + CR_PROTOCOL_DATA_IN_REG);
> +	writel(0x1, base + CR_PROTOCOL_CAP_DATA_REG);
> +
> +	ret = wait_for_latch(base + CR_PROTOCOL_CAP_DATA_REG);
> +	if (ret)
> +		goto err_wait;
> +
> +	writel(0x1, base + CR_PROTOCOL_WRITE_REG);
> +
> +	ret = wait_for_latch(base + CR_PROTOCOL_WRITE_REG);
> +
> +err_wait:
> +	return ret;
> +}
> +
> +/**
> + * Read SSPHY register.
> + *
> + * @base - QCOM DWC3 PHY base virtual address.
> + * @addr - SSPHY address to read.
> + */
> +static int qcom_dwc3_ss_read_phycreg(void __iomem *base, u32 addr, u32 *val)
> +{
> +	int ret;
> +	bool first_read = true;
> +
> +	writel(addr, base + CR_PROTOCOL_DATA_IN_REG);
> +	writel(0x1, base + CR_PROTOCOL_CAP_ADDR_REG);
> +
> +	ret = wait_for_latch(base + CR_PROTOCOL_CAP_ADDR_REG);
> +	if (ret)
> +		goto err_wait;
> +
> +	/*
> +	 * Due to hardware bug, first read of SSPHY register might be
> +	 * incorrect. Hence as workaround, SW should perform SSPHY register
> +	 * read twice, but use only second read and ignore first read.
> +	 */
> +retry:
> +	writel(0x1, base + CR_PROTOCOL_READ_REG);
> +
> +	ret = wait_for_latch(base + CR_PROTOCOL_READ_REG);
> +	if (ret)
> +		goto err_wait;
> +

_retry_ should be here? since only the read fails..
> +	if (first_read) {
> +		readl(base + CR_PROTOCOL_DATA_OUT_REG);
> +		first_read = false;
> +		goto retry;
> +	}
> +
> +	*val = readl(base + CR_PROTOCOL_DATA_OUT_REG);
> +
> +err_wait:
> +	return ret;
> +}
> +
> +static int qcom_dwc3_phy_power_on(struct phy *phy)
> +{
> +	int ret;
> +	struct qcom_dwc3_usb_phy *phy_dwc3 = phy_get_drvdata(phy);
> +
> +	ret = clk_prepare_enable(phy_dwc3->xo_clk);
> +	if (ret)
> +		return ret;
> +
> +	ret = clk_prepare_enable(phy_dwc3->ref_clk);
> +	if (ret)
> +		clk_disable_unprepare(phy_dwc3->xo_clk);
> +
> +	return ret;
> +}
> +
> +static int qcom_dwc3_phy_power_off(struct phy *phy)
> +{
> +	struct qcom_dwc3_usb_phy *phy_dwc3 = phy_get_drvdata(phy);
> +
> +	clk_disable_unprepare(phy_dwc3->ref_clk);
> +	clk_disable_unprepare(phy_dwc3->xo_clk);
> +
> +	return 0;
> +}
> +
> +static int qcom_dwc3_hs_phy_init(struct qcom_dwc3_usb_phy *phy_dwc3)
> +{
> +	u32 val;
> +
> +	/*
> +	 * HSPHY Initialization: Enable UTMI clock, select 19.2MHz fsel
> +	 * enable clamping, and disable RETENTION (power-on default is ENABLED)
> +	 */
> +	val = HSUSB_CTRL_DPSEHV_CLAMP | HSUSB_CTRL_DMSEHV_CLAMP |
> +		HSUSB_CTRL_RETENABLEN  | HSUSB_CTRL_COMMONONN |
> +		HSUSB_CTRL_OTGSESSVLD_CLAMP | HSUSB_CTRL_ID_HV_CLAMP |
> +		HSUSB_CTRL_DPSEHV_CLAMP | HSUSB_CTRL_UTMI_OTG_VBUS_VALID |
> +		HSUSB_CTRL_UTMI_CLK_EN | HSUSB_CTRL_CLAMP_EN | 0x70;
> +
> +	/* use core clock if external reference is not present */
> +	if (!phy_dwc3->xo_clk)
> +		val |= HSUSB_CTRL_USE_CLKCORE;
> +
> +	writel(val, phy_dwc3->base + HSUSB_PHY_CTRL_REG);
> +	usleep_range(2000, 2200);
> +
> +	/* Disable (bypass) VBUS and ID filters */
> +	writel(HSUSB_GCFG_XHCI_REV, phy_dwc3->base + QSCRATCH_GENERAL_CFG);
> +
> +	return 0;
> +}
> +
> +static int qcom_dwc3_ss_phy_init(struct qcom_dwc3_usb_phy *phy_dwc3)
> +{
> +	int ret;
> +	u32 data = 0;
> +
> +	/* reset phy */
> +	data = readl_relaxed(phy_dwc3->base + SSUSB_PHY_CTRL_REG);

Why readl_relaxed here?
> +	writel_relaxed(data | SSUSB_CTRL_SS_PHY_RESET,
> +		phy_dwc3->base + SSUSB_PHY_CTRL_REG);
> +	usleep_range(2000, 2200);

use msleep here..
> +	writel_relaxed(data, phy_dwc3->base + SSUSB_PHY_CTRL_REG);
> +
> +	/* clear REF_PAD if we don't have XO clk */
> +	if (!phy_dwc3->xo_clk)
> +		data &= ~SSUSB_CTRL_REF_USE_PAD;
> +	else
> +		data |= SSUSB_CTRL_REF_USE_PAD;
> +
> +	writel_relaxed(data, phy_dwc3->base + SSUSB_PHY_CTRL_REG);
> +	msleep(30);

add a comment on why this delay is required. If the hw spec gives this delay,
note it here.
> +
> +	data |= SSUSB_CTRL_SS_PHY_EN | SSUSB_CTRL_LANE0_PWR_PRESENT;
> +	writel_relaxed(data, phy_dwc3->base + SSUSB_PHY_CTRL_REG);
> +
> +	/*
> +	 * Fix RX Equalization setting as follows
> +	 * LANE0.RX_OVRD_IN_HI. RX_EQ_EN set to 0
> +	 * LANE0.RX_OVRD_IN_HI.RX_EQ_EN_OVRD set to 1
> +	 * LANE0.RX_OVRD_IN_HI.RX_EQ set to 3
> +	 * LANE0.RX_OVRD_IN_HI.RX_EQ_OVRD set to 1
> +	 */
> +	ret = qcom_dwc3_ss_read_phycreg(phy_dwc3->base,
> +			SSPHY_CTRL_RX_OVRD_IN_HI(0), &data);
> +	if (ret)
> +		goto err_phy_trans;
> +
> +	data &= ~RX_OVRD_IN_HI_RX_EQ_EN;
> +	data |= RX_OVRD_IN_HI_RX_EQ_EN_OVRD;
> +	data &= ~RX_OVRD_IN_HI_RX_EQ_MASK;
> +	data |= 0x3 << RX_OVRD_IN_HI_RX_EQ_SHIFT;
> +	data |= RX_OVRD_IN_HI_RX_EQ_OVRD;
> +	ret = qcom_dwc3_ss_write_phycreg(phy_dwc3->base,
> +		SSPHY_CTRL_RX_OVRD_IN_HI(0), data);
> +	if (ret)
> +		goto err_phy_trans;
> +
> +	/*
> +	 * Set EQ and TX launch amplitudes as follows
> +	 * LANE0.TX_OVRD_DRV_LO.PREEMPH set to 22
> +	 * LANE0.TX_OVRD_DRV_LO.AMPLITUDE set to 127
> +	 * LANE0.TX_OVRD_DRV_LO.EN set to 1.
> +	 */
> +	ret = qcom_dwc3_ss_read_phycreg(phy_dwc3->base,
> +		SSPHY_CTRL_TX_OVRD_DRV_LO(0), &data);
> +	if (ret)
> +		goto err_phy_trans;
> +
> +	data &= ~TX_OVRD_DRV_LO_PREEMPH_MASK;
> +	data |= 0x16 << TX_OVRD_DRV_LO_PREEMPH_SHIFT;
> +	data &= ~TX_OVRD_DRV_LO_AMPLITUDE_MASK;
> +	data |= 0x7f;
> +	data |= TX_OVRD_DRV_LO_EN;
> +	ret = qcom_dwc3_ss_write_phycreg(phy_dwc3->base,
> +		SSPHY_CTRL_TX_OVRD_DRV_LO(0), data);
> +	if (ret)
> +		goto err_phy_trans;
> +
> +	/*
> +	 * Set the QSCRATCH PHY_PARAM_CTRL1 parameters as follows
> +	 * TX_FULL_SWING [26:20] amplitude to 127
> +	 * TX_DEEMPH_3_5DB [13:8] to 22
> +	 * LOS_BIAS [2:0] to 0x5
> +	 */
> +	qcom_dwc3_phy_write_readback(phy_dwc3, SSUSB_PHY_PARAM_CTRL_1,
> +				   0x07f03f07, 0x07f01605);
> +
> +err_phy_trans:
> +	return ret;
> +}
> +
> +static int qcom_dwc3_ss_phy_exit(struct qcom_dwc3_usb_phy *phy_dwc3)
> +{
> +	/* Sequence to put SSPHY in low power state:
> +	 * 1. Clear REF_PHY_EN in PHY_CTRL_REG
> +	 * 2. Clear REF_USE_PAD in PHY_CTRL_REG
> +	 * 3. Set TEST_POWERED_DOWN in PHY_CTRL_REG to enable PHY retention
> +	 * 4. Disable SSPHY ref clk

The last step is missing below..
> +	 */
> +	qcom_dwc3_phy_write_readback(phy_dwc3, SSUSB_PHY_CTRL_REG,
> +		SSUSB_CTRL_SS_PHY_EN, 0x0);
> +	qcom_dwc3_phy_write_readback(phy_dwc3, SSUSB_PHY_CTRL_REG,
> +		SSUSB_CTRL_REF_USE_PAD, 0x0);
> +	qcom_dwc3_phy_write_readback(phy_dwc3, SSUSB_PHY_CTRL_REG,
> +		0x0, SSUSB_CTRL_TEST_POWERDOWN);
> +
> +	return 0;
> +}
> +
> +static int qcom_dwc3_phy_init(struct phy *phy)
> +{
> +	struct qcom_dwc3_usb_phy *phy_dwc3 = phy_get_drvdata(phy);
> +
> +	if (phy_dwc3->phy_init)
> +		return phy_dwc3->phy_init(phy_dwc3);

This indirection is not needed at all. qcom_dwc3_ss_phy_init can be directly
populated in probe depending on the PHY.
> +
> +	return 0;
> +}
> +
> +static int qcom_dwc3_phy_exit(struct phy *phy)
> +{
> +	struct qcom_dwc3_usb_phy *phy_dwc3 = phy_get_drvdata(phy);
> +
> +	if (phy_dwc3->phy_exit)
> +		return qcom_dwc3_ss_phy_exit(phy_dwc3);

you meant return phy_dwc3->phy_exit(phy_dwc3) here?

Thanks
Kishon

Re: [Patch v9 3/3] phy: Add Qualcomm DWC3 HS/SS PHY driver

[Felipe Balbi]

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

Hi,

On Sat, Sep 13, 2014 at 12:16:01PM +0530, Kishon Vijay Abraham I wrote:
> On Saturday 13 September 2014 12:58 AM, Andy Gross wrote:
> > This patch adds a new driver for the Qualcomm USB 3.0 PHY that exists on some
> > Qualcomm platforms.  This driver uses the generic PHY framework and will
> > interact with the DWC3 controller.
> 
> Do you have dt documentation for this driver?

see patch 1

> > +static inline void qcom_dwc3_phy_write_readback(
> > +	struct qcom_dwc3_usb_phy *phy_dwc3, u32 offset,
> > +	const u32 mask, u32 val)
> > +{
> > +	u32 write_val, tmp = readl(phy_dwc3->base + offset);
> > +
> > +	tmp &= ~mask;		/* retain other bits */
> > +	write_val = tmp | val;
> > +
> > +	writel(write_val, phy_dwc3->base + offset);
> > +
> > +	/* Read back to see if val was written */
> 
> Does it fail sometime? I'm not sure if this should be present in the
> driver since this looks more of a debug code.

this was mentioned before. Silicon bug.

> > +	writel_relaxed(data | SSUSB_CTRL_SS_PHY_RESET,
> > +		phy_dwc3->base + SSUSB_PHY_CTRL_REG);
> > +	usleep_range(2000, 2200);
> 
> use msleep here..

why ? usleep_range() gives the scheduler oportunity to group timers.

-- 
balbi

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

Re: [Patch v9 3/3] phy: Add Qualcomm DWC3 HS/SS PHY driver

[Kishon Vijay Abraham I]

Hi,

On Sunday 14 September 2014 07:54 AM, Felipe Balbi wrote:
> Hi,
> 
> On Sat, Sep 13, 2014 at 12:16:01PM +0530, Kishon Vijay Abraham I wrote:
>> On Saturday 13 September 2014 12:58 AM, Andy Gross wrote:
>>> This patch adds a new driver for the Qualcomm USB 3.0 PHY that exists on some
>>> Qualcomm platforms.  This driver uses the generic PHY framework and will
>>> interact with the DWC3 controller.
>>
>> Do you have dt documentation for this driver?
> 
> see patch 1

hmm.. missed that.
> 
>>> +static inline void qcom_dwc3_phy_write_readback(
>>> +	struct qcom_dwc3_usb_phy *phy_dwc3, u32 offset,
>>> +	const u32 mask, u32 val)
>>> +{
>>> +	u32 write_val, tmp = readl(phy_dwc3->base + offset);
>>> +
>>> +	tmp &= ~mask;		/* retain other bits */
>>> +	write_val = tmp | val;
>>> +
>>> +	writel(write_val, phy_dwc3->base + offset);
>>> +
>>> +	/* Read back to see if val was written */
>>
>> Does it fail sometime? I'm not sure if this should be present in the
>> driver since this looks more of a debug code.
> 
> this was mentioned before. Silicon bug.

okay.
> 
>>> +	writel_relaxed(data | SSUSB_CTRL_SS_PHY_RESET,
>>> +		phy_dwc3->base + SSUSB_PHY_CTRL_REG);
>>> +	usleep_range(2000, 2200);
>>
>> use msleep here..
> 
> why ? usleep_range() gives the scheduler oportunity to group timers.

Was of the opinion that for larger delays msleep should be used. But looks like
it is only for 10+ ms (Documentation/timers/timers-howto.txt).

Thanks
Kishon

Re: [Patch v9 1/3] usb: dwc3: qcom: Add device tree binding

[Jack Pham]

Hi Andy,

On Fri, Sep 12, 2014 at 02:28:06PM -0500, Andy Gross wrote:
> +Example device nodes:
> +
> +		hs_phy: phy@100f8800 {
> +			compatible = "qcom,dwc3-hs-usb-phy";
> +			reg = <0x100f8800 0x30>;

Just wanted to point out that in our downstream code, the glue
device/driver, i.e. "qcom,dwc3", does claim an entire register region
that encompasses the same registers used by these PHYs. I noticed you're
not adding a reg resource to the glue node below in this patchset. In
order to allow multiple devices to use the overlapping regions, we avoid
calling devm_ioremap_resource() and instead call devm_ioremap_nocache()
directly which bypasses the request_mem_region() call, which I don't
think is an entirely correct thing to do.

On the other hand I'm trying to think of use cases on our other SOCs
(MSM8974, APQ8084) where the glue actually would need access to the same
or adjacent IO registers that couldn't just be handled directly by these
PHY drivers. Should we accommodate for the potential of overlapping
regions or should we just hold the line here and maybe only expose with
finer granularity the registers that will actually be needed by the PHYs
& glue respectively?

> +			clocks = <&gcc USB30_0_UTMI_CLK>;
> +			clock-names = "ref";
> +			#phy-cells = <0>;
> +
> +			status = "ok";
> +		};
> +
> +		ss_phy: phy@100f8830 {
> +			compatible = "qcom,dwc3-ss-usb-phy";
> +			reg = <0x100f8830 0x30>;
> +			clocks = <&gcc USB30_0_MASTER_CLK>;
> +			clock-names = "ref";
> +			#phy-cells = <0>;
> +
> +			status = "ok";
> +		};
> +
> +		usb3_0: usb30@0 {
> +			compatible = "qcom,dwc3";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			clocks = <&gcc USB30_0_MASTER_CLK>;
> +			clock-names = "core";
> +
> +			ranges;
> +
> +			status = "ok";
> +
> +			dwc3@10000000 {
> +				compatible = "snps,dwc3";
> +				reg = <0x10000000 0xcd00>;
> +				interrupts = <0 205 0x4>;
> +				phys = <&hs_phy>, <&ss_phy>;
> +				phy-names = "usb2-phy", "usb3-phy";
> +				tx-fifo-resize;
> +				dr_mode = "host";
> +			};
> +		};
> +

Thanks,
Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [Patch v9 3/3] phy: Add Qualcomm DWC3 HS/SS PHY driver

[Jack Pham]

Hi Andy,

On Fri, Sep 12, 2014 at 02:28:08PM -0500, Andy Gross wrote:
> +static int qcom_dwc3_hs_phy_init(struct qcom_dwc3_usb_phy *phy_dwc3)
> +{
> +	u32 val;
> +
> +	/*
> +	 * HSPHY Initialization: Enable UTMI clock, select 19.2MHz fsel
> +	 * enable clamping, and disable RETENTION (power-on default is ENABLED)
> +	 */
> +	val = HSUSB_CTRL_DPSEHV_CLAMP | HSUSB_CTRL_DMSEHV_CLAMP |
> +		HSUSB_CTRL_RETENABLEN  | HSUSB_CTRL_COMMONONN |
> +		HSUSB_CTRL_OTGSESSVLD_CLAMP | HSUSB_CTRL_ID_HV_CLAMP |
> +		HSUSB_CTRL_DPSEHV_CLAMP | HSUSB_CTRL_UTMI_OTG_VBUS_VALID |
> +		HSUSB_CTRL_UTMI_CLK_EN | HSUSB_CTRL_CLAMP_EN | 0x70;
> +
> +	/* use core clock if external reference is not present */
> +	if (!phy_dwc3->xo_clk)
> +		val |= HSUSB_CTRL_USE_CLKCORE;
> +
> +	writel(val, phy_dwc3->base + HSUSB_PHY_CTRL_REG);
> +	usleep_range(2000, 2200);
> +
> +	/* Disable (bypass) VBUS and ID filters */
> +	writel(HSUSB_GCFG_XHCI_REV, phy_dwc3->base + QSCRATCH_GENERAL_CFG);

Is this comment accurate? I believe this bit forces the IP to behave in
XHCI rev 1.0. In which case, shouldn't it be done in the glue driver?

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [Patch v9 1/3] usb: dwc3: qcom: Add device tree binding

[Felipe Balbi]

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

Hi,

On Tue, Sep 16, 2014 at 11:15:43AM -0700, Jack Pham wrote:
> Hi Andy,
> 
> On Fri, Sep 12, 2014 at 02:28:06PM -0500, Andy Gross wrote:
> > +Example device nodes:
> > +
> > +		hs_phy: phy@100f8800 {
> > +			compatible = "qcom,dwc3-hs-usb-phy";
> > +			reg = <0x100f8800 0x30>;
> 
> Just wanted to point out that in our downstream code, the glue
> device/driver, i.e. "qcom,dwc3", does claim an entire register region
> that encompasses the same registers used by these PHYs. I noticed you're
> not adding a reg resource to the glue node below in this patchset. In
> order to allow multiple devices to use the overlapping regions, we avoid
> calling devm_ioremap_resource() and instead call devm_ioremap_nocache()
> directly which bypasses the request_mem_region() call, which I don't
> think is an entirely correct thing to do.
> 
> On the other hand I'm trying to think of use cases on our other SOCs
> (MSM8974, APQ8084) where the glue actually would need access to the same
> or adjacent IO registers that couldn't just be handled directly by these
> PHY drivers. Should we accommodate for the potential of overlapping
> regions or should we just hold the line here and maybe only expose with
> finer granularity the registers that will actually be needed by the PHYs
> & glue respectively?

I prefer to keep things separated. Glue only needs to access whatever
belongs to glue. If a register related to PHY settings, it should be in
the PHY address space.

cheers

-- 
balbi

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

Re: [Patch v9 3/3] phy: Add Qualcomm DWC3 HS/SS PHY driver

[Andy Gross]

On Tue, Sep 16, 2014 at 11:27:52AM -0700, Jack Pham wrote:
> Hi Andy,
> 
> On Fri, Sep 12, 2014 at 02:28:08PM -0500, Andy Gross wrote:
> > +static int qcom_dwc3_hs_phy_init(struct qcom_dwc3_usb_phy *phy_dwc3)
> > +{
> > +	u32 val;
> > +
> > +	/*
> > +	 * HSPHY Initialization: Enable UTMI clock, select 19.2MHz fsel
> > +	 * enable clamping, and disable RETENTION (power-on default is ENABLED)
> > +	 */
> > +	val = HSUSB_CTRL_DPSEHV_CLAMP | HSUSB_CTRL_DMSEHV_CLAMP |
> > +		HSUSB_CTRL_RETENABLEN  | HSUSB_CTRL_COMMONONN |
> > +		HSUSB_CTRL_OTGSESSVLD_CLAMP | HSUSB_CTRL_ID_HV_CLAMP |
> > +		HSUSB_CTRL_DPSEHV_CLAMP | HSUSB_CTRL_UTMI_OTG_VBUS_VALID |
> > +		HSUSB_CTRL_UTMI_CLK_EN | HSUSB_CTRL_CLAMP_EN | 0x70;
> > +
> > +	/* use core clock if external reference is not present */
> > +	if (!phy_dwc3->xo_clk)
> > +		val |= HSUSB_CTRL_USE_CLKCORE;
> > +
> > +	writel(val, phy_dwc3->base + HSUSB_PHY_CTRL_REG);
> > +	usleep_range(2000, 2200);
> > +
> > +	/* Disable (bypass) VBUS and ID filters */
> > +	writel(HSUSB_GCFG_XHCI_REV, phy_dwc3->base + QSCRATCH_GENERAL_CFG);
> 
> Is this comment accurate? I believe this bit forces the IP to behave in
> XHCI rev 1.0. In which case, shouldn't it be done in the glue driver?

I'll double check.  I was taking the bit values and converting them to names.
If this is doing that, then I'll move it to the glue.


-- 
sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

Re: [Patch v9 3/3] phy: Add Qualcomm DWC3 HS/SS PHY driver

[Jack Pham]

Hi Andy,

On Fri, Sep 12, 2014 at 02:28:08PM -0500, Andy Gross wrote:
> This patch adds a new driver for the Qualcomm USB 3.0 PHY that exists on some
> Qualcomm platforms.  This driver uses the generic PHY framework and will
> interact with the DWC3 controller.
> 
> Signed-off-by: Andy Gross <agross@codeaurora.org>
> ---
>  drivers/phy/Kconfig         |   11 +
>  drivers/phy/Makefile        |    1 +
>  drivers/phy/phy-qcom-dwc3.c |  483 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 495 insertions(+)
>  create mode 100644 drivers/phy/phy-qcom-dwc3.c

What happened to this patch? Looks like [1/3] & [2/3] made it in during
the 3.18 merge, but did this one not get picked up by Kishon?

Thanks,
Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [Patch v9 3/3] phy: Add Qualcomm DWC3 HS/SS PHY driver

[Andy Gross]

On Thu, Jan 22, 2015 at 10:59:14AM -0800, Jack Pham wrote:
> Hi Andy,
> 
> On Fri, Sep 12, 2014 at 02:28:08PM -0500, Andy Gross wrote:
> > This patch adds a new driver for the Qualcomm USB 3.0 PHY that exists on some
> > Qualcomm platforms.  This driver uses the generic PHY framework and will
> > interact with the DWC3 controller.
> > 
> > Signed-off-by: Andy Gross <agross@codeaurora.org>
> > ---
> >  drivers/phy/Kconfig         |   11 +
> >  drivers/phy/Makefile        |    1 +
> >  drivers/phy/phy-qcom-dwc3.c |  483 +++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 495 insertions(+)
> >  create mode 100644 drivers/phy/phy-qcom-dwc3.c
> 
> What happened to this patch? Looks like [1/3] & [2/3] made it in during
> the 3.18 merge, but did this one not get picked up by Kishon?

Got sidetracked.  I'll rework and resend in.  It'll be a couple days.

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project