2014-06-30 16:04:09

by Andy Gross

[permalink] [raw]
Subject: [Patch v7 0/3] DWC3 USB support for Qualcomm platform

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 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.

Ivan T. Ivanov (3):
usb: dwc3: Add Qualcomm DWC3 glue layer driver
usb: phy: Add Qualcomm DWC3 HS/SS PHY drivers
usb: dwc3: qcom: Add device tree binding

.../devicetree/bindings/usb/qcom,dwc3.txt | 104 ++++++
drivers/usb/dwc3/Kconfig | 9 +
drivers/usb/dwc3/Makefile | 1 +
drivers/usb/dwc3/dwc3-qcom.c | 153 ++++++++
drivers/usb/phy/Kconfig | 11 +
drivers/usb/phy/Makefile | 2 +
drivers/usb/phy/phy-qcom-hsusb.c | 348 ++++++++++++++++++
drivers/usb/phy/phy-qcom-ssusb.c | 385 ++++++++++++++++++++
8 files changed, 1013 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/qcom,dwc3.txt
create mode 100644 drivers/usb/dwc3/dwc3-qcom.c
create mode 100644 drivers/usb/phy/phy-qcom-hsusb.c
create mode 100644 drivers/usb/phy/phy-qcom-ssusb.c

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


2014-06-30 16:04:11

by Andy Gross

[permalink] [raw]
Subject: [Patch v7 1/3] usb: dwc3: Add Qualcomm DWC3 glue layer driver

From: "Ivan T. Ivanov" <[email protected]>

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 <[email protected]>
Signed-off-by: Andy Gross <[email protected]>
---
drivers/usb/dwc3/Kconfig | 9 +++
drivers/usb/dwc3/Makefile | 1 +
drivers/usb/dwc3/dwc3-qcom.c | 153 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 163 insertions(+)
create mode 100644 drivers/usb/dwc3/dwc3-qcom.c

diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 8eb996e..29fcbfd 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -79,6 +79,15 @@ 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"
+ default USB_DWC3
+ select USB_QCOM_DWC3_PHY
+ 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..e99764a
--- /dev/null
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -0,0 +1,153 @@
+/* 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>
+#include <linux/regulator/consumer.h>
+#include <linux/usb/phy.h>
+
+#include "core.h"
+
+
+struct dwc3_qcom {
+ struct device *dev;
+
+ struct clk *core_clk;
+ struct clk *iface_clk;
+ struct clk *sleep_clk;
+
+ struct regulator *gdsc;
+};
+
+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->gdsc = devm_regulator_get(qdwc->dev, "gdsc");
+
+ qdwc->core_clk = devm_clk_get(qdwc->dev, "core");
+ if (IS_ERR(qdwc->core_clk)) {
+ dev_dbg(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 iface clock, skipping\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 sleep clock, skipping\n");
+ qdwc->sleep_clk = NULL;
+ }
+
+ if (!IS_ERR(qdwc->gdsc)) {
+ ret = regulator_enable(qdwc->gdsc);
+ if (ret)
+ dev_err(qdwc->dev, "cannot enable gdsc\n");
+ }
+
+ clk_prepare_enable(qdwc->core_clk);
+
+ if (qdwc->iface_clk)
+ clk_prepare_enable(qdwc->iface_clk);
+
+ if (qdwc->sleep_clk)
+ clk_prepare_enable(qdwc->sleep_clk);
+
+ ret = of_platform_populate(node, NULL, NULL, qdwc->dev);
+ if (ret) {
+ dev_err(qdwc->dev, "failed to register core - %d\n", ret);
+ dev_dbg(qdwc->dev, "failed to add create dwc3 core\n");
+ goto dis_clks;
+ }
+
+ return 0;
+
+dis_clks:
+ if (qdwc->sleep_clk)
+ clk_disable_unprepare(qdwc->sleep_clk);
+
+ if (qdwc->iface_clk)
+ clk_disable_unprepare(qdwc->iface_clk);
+
+ clk_disable_unprepare(qdwc->core_clk);
+
+ if (!IS_ERR(qdwc->gdsc)) {
+ ret = regulator_disable(qdwc->gdsc);
+ if (ret)
+ dev_dbg(qdwc->dev, "cannot disable gdsc\n");
+ }
+
+ return ret;
+}
+
+static int dwc3_qcom_remove(struct platform_device *pdev)
+{
+ int ret = 0;
+
+ struct dwc3_qcom *qdwc = platform_get_drvdata(pdev);
+
+ if (qdwc->sleep_clk)
+ clk_disable_unprepare(qdwc->sleep_clk);
+
+ if (qdwc->iface_clk)
+ clk_disable_unprepare(qdwc->iface_clk);
+
+ clk_disable_unprepare(qdwc->core_clk);
+
+ if (!IS_ERR(qdwc->gdsc)) {
+ ret = regulator_disable(qdwc->gdsc);
+ if (ret)
+ dev_dbg(qdwc->dev, "cannot disable gdsc\n");
+ }
+ return ret;
+}
+
+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");
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2014-06-30 16:04:16

by Andy Gross

[permalink] [raw]
Subject: [Patch v7 2/3] usb: phy: Add Qualcomm DWC3 HS/SS PHY drivers

From: "Ivan T. Ivanov" <[email protected]>

These drivers handles control and configuration of the HS
and SS USB PHY transceivers. They are part of the driver
which manage Synopsys DesignWare USB3 controller stack
inside Qualcomm SoC's.

Signed-off-by: Ivan T. Ivanov <[email protected]>
Signed-off-by: Andy Gross <[email protected]>
---
drivers/usb/phy/Kconfig | 11 ++
drivers/usb/phy/Makefile | 2 +
drivers/usb/phy/phy-qcom-hsusb.c | 348 ++++++++++++++++++++++++++++++++++
drivers/usb/phy/phy-qcom-ssusb.c | 385 ++++++++++++++++++++++++++++++++++++++
4 files changed, 746 insertions(+)
create mode 100644 drivers/usb/phy/phy-qcom-hsusb.c
create mode 100644 drivers/usb/phy/phy-qcom-ssusb.c

diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index e253fa0..5580c2f 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -229,4 +229,15 @@ config USB_ULPI_VIEWPORT
Provides read/write operations to the ULPI phy register set for
controllers with a viewport register (e.g. Chipidea/ARC controllers).

+config USB_QCOM_DWC3_PHY
+ tristate "Qualcomm USB controller DWC3 PHY wrappers support"
+ depends on (USB || USB_GADGET) && ARCH_QCOM
+ select USB_PHY
+ help
+ Enable this to support the DWC3 USB PHY transceivers on QCOM chips
+ with DWC3 USB core. It handles PHY initialization, clock
+ management required after resetting the hardware and power
+ management. This driver is required even for peripheral only or
+ host only mode configurations.
+
endmenu
diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
index 24a9133..ef09c9c 100644
--- a/drivers/usb/phy/Makefile
+++ b/drivers/usb/phy/Makefile
@@ -29,3 +29,5 @@ obj-$(CONFIG_USB_RCAR_GEN2_PHY) += phy-rcar-gen2-usb.o
obj-$(CONFIG_USB_ULPI) += phy-ulpi.o
obj-$(CONFIG_USB_ULPI_VIEWPORT) += phy-ulpi-viewport.o
obj-$(CONFIG_KEYSTONE_USB_PHY) += phy-keystone.o
+obj-$(CONFIG_USB_QCOM_DWC3_PHY) += phy-qcom-hsusb.o
+obj-$(CONFIG_USB_QCOM_DWC3_PHY) += phy-qcom-ssusb.o
diff --git a/drivers/usb/phy/phy-qcom-hsusb.c b/drivers/usb/phy/phy-qcom-hsusb.c
new file mode 100644
index 0000000..7a44b13
--- /dev/null
+++ b/drivers/usb/phy/phy-qcom-hsusb.c
@@ -0,0 +1,348 @@
+/* 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/platform_device.h>
+#include <linux/regulator/consumer.h>
+#include <linux/usb/phy.h>
+
+/**
+ * USB QSCRATCH Hardware registers
+ */
+#define QSCRATCH_CTRL_REG (0x04)
+#define QSCRATCH_GENERAL_CFG (0x08)
+#define PHY_CTRL_REG (0x10)
+#define PARAMETER_OVERRIDE_X_REG (0x14)
+#define CHARGING_DET_CTRL_REG (0x18)
+#define CHARGING_DET_OUTPUT_REG (0x1c)
+#define ALT_INTERRUPT_EN_REG (0x20)
+#define PHY_IRQ_STAT_REG (0x24)
+#define CGCTL_REG (0x28)
+
+#define PHY_3P3_VOL_MIN 3050000 /* uV */
+#define PHY_3P3_VOL_MAX 3300000 /* uV */
+#define PHY_3P3_HPM_LOAD 16000 /* uA */
+
+#define PHY_1P8_VOL_MIN 1800000 /* uV */
+#define PHY_1P8_VOL_MAX 1800000 /* uV */
+#define PHY_1P8_HPM_LOAD 19000 /* uA */
+
+/* TODO: these are suspicious */
+#define USB_VDDCX_NO 1 /* index */
+#define USB_VDDCX_MIN 5 /* index */
+#define USB_VDDCX_MAX 7 /* index */
+
+/* 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 HSYSB_CTRL_POR BIT(0)
+
+
+/* QSCRATCH_GENERAL_CFG */
+#define HSUSB_GCFG_XHCI_REV BIT(2)
+
+struct qcom_dwc3_hs_phy {
+ struct usb_phy phy;
+ void __iomem *base;
+ struct device *dev;
+
+ struct clk *xo_clk;
+ struct clk *utmi_clk;
+
+ struct regulator *v3p3;
+ struct regulator *v1p8;
+ struct regulator *vddcx;
+ struct regulator *vbus;
+};
+
+#define phy_to_dw_phy(x) container_of((x), struct qcom_dwc3_hs_phy, phy)
+
+/**
+ * Write register.
+ *
+ * @base - QCOM DWC3 PHY base virtual address.
+ * @offset - register offset.
+ * @val - value to write.
+ */
+static inline void qcom_dwc3_hs_write(void __iomem *base, u32 offset, u32 val)
+{
+ writel(val, base + offset);
+}
+
+/**
+ * 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_hs_write_readback(void __iomem *base, u32 offset,
+ const u32 mask, u32 val)
+{
+ u32 write_val, tmp = readl(base + offset);
+
+ tmp &= ~mask; /* retain other bits */
+ write_val = tmp | val;
+
+ writel(write_val, base + offset);
+
+ /* Read back to see if val was written */
+ tmp = readl(base + offset);
+ tmp &= mask; /* clear other bits */
+
+ if (tmp != val)
+ pr_err("write: %x to QSCRATCH: %x FAILED\n", val, offset);
+}
+
+static void qcom_dwc3_hs_phy_shutdown(struct usb_phy *x)
+{
+ struct qcom_dwc3_hs_phy *phy = phy_to_dw_phy(x);
+ int ret;
+
+ ret = regulator_set_voltage(phy->v3p3, 0, PHY_3P3_VOL_MAX);
+ if (ret)
+ dev_err(phy->dev, "cannot set voltage for v3p3\n");
+
+ ret = regulator_set_voltage(phy->v1p8, 0, PHY_1P8_VOL_MAX);
+ if (ret)
+ dev_err(phy->dev, "cannot set voltage for v1p8\n");
+
+ ret = regulator_disable(phy->v1p8);
+ if (ret)
+ dev_err(phy->dev, "cannot disable v1p8\n");
+
+ ret = regulator_disable(phy->v3p3);
+ if (ret)
+ dev_err(phy->dev, "cannot disable v3p3\n");
+
+ ret = regulator_set_voltage(phy->vddcx, USB_VDDCX_NO, USB_VDDCX_MAX);
+ if (ret)
+ dev_err(phy->dev, "cannot set voltage for vddcx\n");
+
+ ret = regulator_disable(phy->vddcx);
+ if (ret)
+ dev_err(phy->dev, "cannot enable vddcx\n");
+
+ clk_disable_unprepare(phy->utmi_clk);
+}
+
+static int qcom_dwc3_hs_phy_init(struct usb_phy *x)
+{
+ struct qcom_dwc3_hs_phy *phy = phy_to_dw_phy(x);
+ int ret;
+ u32 val;
+
+ clk_prepare_enable(phy->utmi_clk);
+
+ ret = regulator_set_voltage(phy->vddcx, USB_VDDCX_MIN, USB_VDDCX_MAX);
+ if (ret)
+ dev_err(phy->dev, "cannot set voltage for vddcx\n");
+
+ ret = regulator_enable(phy->vddcx);
+ if (ret)
+ dev_err(phy->dev, "cannot enable vddcx\n");
+
+ ret = regulator_set_voltage(phy->v3p3, PHY_3P3_VOL_MIN,
+ PHY_3P3_VOL_MAX);
+ if (ret)
+ dev_err(phy->dev, "cannot set voltage for v3p3\n");
+
+ ret = regulator_set_voltage(phy->v1p8, PHY_1P8_VOL_MIN,
+ PHY_1P8_VOL_MAX);
+ if (ret)
+ dev_err(phy->dev, "cannot set voltage for v1p8\n");
+
+ ret = regulator_set_optimum_mode(phy->v1p8, PHY_1P8_HPM_LOAD);
+ if (ret < 0)
+ dev_err(phy->dev, "cannot set HPM of regulator v1p8\n");
+
+ ret = regulator_enable(phy->v1p8);
+ if (ret)
+ dev_err(phy->dev, "cannot enable v1p8\n");
+
+ ret = regulator_set_optimum_mode(phy->v3p3, PHY_3P3_HPM_LOAD);
+ if (ret < 0)
+ dev_err(phy->dev, "cannot set HPM of regulator v3p3\n");
+
+ ret = regulator_enable(phy->v3p3);
+ if (ret)
+ dev_err(phy->dev, "cannot enable v3p3\n");
+
+ /*
+ * 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->xo_clk)
+ val |= HSUSB_CTRL_USE_CLKCORE;
+
+ qcom_dwc3_hs_write(phy->base, PHY_CTRL_REG, val);
+ usleep_range(2000, 2200);
+
+ /*
+ * write HSPHY init value to QSCRATCH reg to set HSPHY parameters like
+ * VBUS valid threshold, disconnect valid threshold, DC voltage level,
+ * preempasis and rise/fall time.
+ */
+ qcom_dwc3_hs_write_readback(phy->base, PARAMETER_OVERRIDE_X_REG,
+ 0x03ffffff, 0x00d191a4);
+
+ /* Disable (bypass) VBUS and ID filters */
+ qcom_dwc3_hs_write(phy->base, QSCRATCH_GENERAL_CFG,
+ HSUSB_GCFG_XHCI_REV);
+
+ return 0;
+}
+
+static int qcom_dwc3_hs_phy_set_vbus(struct usb_phy *x, int on)
+{
+ struct qcom_dwc3_hs_phy *phy = phy_to_dw_phy(x);
+ int ret = 0;
+
+ if (IS_ERR(phy->vbus))
+ return on ? PTR_ERR(phy->vbus) : 0;
+
+ if (on)
+ ret = regulator_enable(phy->vbus);
+ else
+ ret = regulator_disable(phy->vbus);
+
+ if (ret)
+ dev_err(x->dev, "Cannot %s Vbus\n", on ? "set" : "off");
+ return ret;
+}
+
+static int qcom_dwc3_hs_probe(struct platform_device *pdev)
+{
+ struct qcom_dwc3_hs_phy *phy;
+ struct resource *res;
+ void __iomem *base;
+
+ phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
+ if (!phy)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, phy);
+
+ phy->dev = &pdev->dev;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ base = devm_ioremap_resource(phy->dev, res);
+ if (IS_ERR(base))
+ return PTR_ERR(base);
+
+ phy->vddcx = devm_regulator_get(phy->dev, "vddcx");
+ if (IS_ERR(phy->vddcx)) {
+ dev_dbg(phy->dev, "cannot get vddcx\n");
+ return PTR_ERR(phy->vddcx);
+ }
+
+ phy->v3p3 = devm_regulator_get(phy->dev, "v3p3");
+ if (IS_ERR(phy->v3p3)) {
+ dev_dbg(phy->dev, "cannot get v3p3\n");
+ return PTR_ERR(phy->v3p3);
+ }
+
+ phy->v1p8 = devm_regulator_get(phy->dev, "v1p8");
+ if (IS_ERR(phy->v1p8)) {
+ dev_dbg(phy->dev, "cannot get v1p8\n");
+ return PTR_ERR(phy->v1p8);
+ }
+
+ phy->vbus = devm_regulator_get(phy->dev, "vbus");
+ if (IS_ERR(phy->vbus))
+ dev_dbg(phy->dev, "Failed to get vbus\n");
+
+ phy->utmi_clk = devm_clk_get(phy->dev, "utmi");
+ if (IS_ERR(phy->utmi_clk)) {
+ dev_dbg(phy->dev, "cannot get UTMI handle\n");
+ return PTR_ERR(phy->utmi_clk);
+ }
+
+ phy->xo_clk = devm_clk_get(phy->dev, "xo");
+ if (IS_ERR(phy->xo_clk)) {
+ dev_dbg(phy->dev, "cannot get TCXO buffer handle\n");
+ phy->xo_clk = NULL;
+ }
+
+ clk_set_rate(phy->utmi_clk, 60000000);
+
+ if (phy->xo_clk)
+ clk_prepare_enable(phy->xo_clk);
+
+ phy->base = base;
+ phy->phy.dev = phy->dev;
+ phy->phy.label = "qcom-dwc3-hsphy";
+ phy->phy.init = qcom_dwc3_hs_phy_init;
+ phy->phy.shutdown = qcom_dwc3_hs_phy_shutdown;
+ phy->phy.set_vbus = qcom_dwc3_hs_phy_set_vbus;
+ phy->phy.type = USB_PHY_TYPE_USB2;
+ phy->phy.state = OTG_STATE_UNDEFINED;
+
+ usb_add_phy_dev(&phy->phy);
+ return 0;
+}
+
+static int qcom_dwc3_hs_remove(struct platform_device *pdev)
+{
+ struct qcom_dwc3_hs_phy *phy = platform_get_drvdata(pdev);
+
+ if (phy->xo_clk)
+ clk_disable_unprepare(phy->xo_clk);
+
+ clk_disable_unprepare(phy->utmi_clk);
+
+ usb_remove_phy(&phy->phy);
+ return 0;
+}
+
+static const struct of_device_id qcom_dwc3_hs_id_table[] = {
+ { .compatible = "qcom,dwc3-hsphy" },
+ { /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, qcom_dwc3_hs_id_table);
+
+static struct platform_driver qcom_dwc3_hs_driver = {
+ .probe = qcom_dwc3_hs_probe,
+ .remove = qcom_dwc3_hs_remove,
+ .driver = {
+ .name = "qcom-dwc3-hsphy",
+ .owner = THIS_MODULE,
+ .of_match_table = qcom_dwc3_hs_id_table,
+ },
+};
+
+module_platform_driver(qcom_dwc3_hs_driver);
+
+MODULE_ALIAS("platform:qcom-dwc3-hsphy");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("DesignWare USB3 QCOM HSPHY driver");
diff --git a/drivers/usb/phy/phy-qcom-ssusb.c b/drivers/usb/phy/phy-qcom-ssusb.c
new file mode 100644
index 0000000..d340c8f
--- /dev/null
+++ b/drivers/usb/phy/phy-qcom-ssusb.c
@@ -0,0 +1,385 @@
+/* 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/platform_device.h>
+#include <linux/regulator/consumer.h>
+#include <linux/usb/phy.h>
+
+/**
+ * USB QSCRATCH Hardware registers
+ */
+#define PHY_CTRL_REG (0x00)
+#define PHY_PARAM_CTRL_1 (0x04)
+#define 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)
+
+#define PHY_1P8_VOL_MIN 1800000 /* uV */
+#define PHY_1P8_VOL_MAX 1800000 /* uV */
+#define PHY_1P8_HPM_LOAD 23000 /* uA */
+
+/* TODO: these are suspicious */
+#define USB_VDDCX_NO 1 /* index */
+#define USB_VDDCX_MIN 5 /* index */
+#define USB_VDDCX_MAX 7 /* index */
+
+/* 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)
+
+struct qcom_dwc3_ss_phy {
+ struct usb_phy phy;
+ void __iomem *base;
+ struct device *dev;
+
+ struct regulator *v1p8;
+ struct regulator *vddcx;
+
+ struct clk *xo_clk;
+ struct clk *ref_clk;
+};
+
+#define phy_to_dw_phy(x) container_of((x), struct qcom_dwc3_ss_phy, phy)
+
+
+/**
+ * Write register
+ *
+ * @base - QCOM DWC3 PHY base virtual address.
+ * @offset - register offset.
+ * @val - value to write.
+ */
+static inline void qcom_dwc3_ss_write(void __iomem *base, u32 offset, u32 val)
+{
+ writel(val, base + offset);
+}
+
+/**
+ * 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_ss_write_readback(void __iomem *base, u32 offset,
+ const u32 mask, u32 val)
+{
+ u32 write_val, tmp = readl(base + offset);
+
+ tmp &= ~mask; /* retain other bits */
+ write_val = tmp | val;
+
+ writel(write_val, base + offset);
+
+ /* Read back to see if val was written */
+ tmp = readl(base + offset);
+ tmp &= mask; /* clear other bits */
+
+ if (tmp != val)
+ pr_err("write: %x to QSCRATCH: %x FAILED\n", val, offset);
+}
+
+/**
+ * Write SSPHY register
+ *
+ * @base - QCOM DWC3 PHY base virtual address.
+ * @addr - SSPHY address to write.
+ * @val - value to write.
+ */
+static void qcom_dwc3_ss_write_phycreg(void __iomem *base, u32 addr, u32 val)
+{
+ writel(addr, base + CR_PROTOCOL_DATA_IN_REG);
+ writel(0x1, base + CR_PROTOCOL_CAP_ADDR_REG);
+ while (readl(base + CR_PROTOCOL_CAP_ADDR_REG))
+ cpu_relax();
+
+ writel(val, base + CR_PROTOCOL_DATA_IN_REG);
+ writel(0x1, base + CR_PROTOCOL_CAP_DATA_REG);
+ while (readl(base + CR_PROTOCOL_CAP_DATA_REG))
+ cpu_relax();
+
+ writel(0x1, base + CR_PROTOCOL_WRITE_REG);
+ while (readl(base + CR_PROTOCOL_WRITE_REG))
+ cpu_relax();
+}
+
+/**
+ * Read SSPHY register.
+ *
+ * @base - QCOM DWC3 PHY base virtual address.
+ * @addr - SSPHY address to read.
+ */
+static u32 qcom_dwc3_ss_read_phycreg(void __iomem *base, u32 addr)
+{
+ bool first_read = true;
+
+ writel(addr, base + CR_PROTOCOL_DATA_IN_REG);
+ writel(0x1, base + CR_PROTOCOL_CAP_ADDR_REG);
+ while (readl(base + CR_PROTOCOL_CAP_ADDR_REG))
+ cpu_relax();
+
+ /*
+ * 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);
+ while (readl(base + CR_PROTOCOL_READ_REG))
+ cpu_relax();
+
+ if (first_read) {
+ readl(base + CR_PROTOCOL_DATA_OUT_REG);
+ first_read = false;
+ goto retry;
+ }
+
+ return readl(base + CR_PROTOCOL_DATA_OUT_REG);
+}
+
+static void qcom_dwc3_ss_phy_shutdown(struct usb_phy *x)
+{
+ struct qcom_dwc3_ss_phy *phy = phy_to_dw_phy(x);
+ int ret;
+
+ /* 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_ss_write_readback(phy->base, PHY_CTRL_REG,
+ SSUSB_CTRL_SS_PHY_EN, 0x0);
+ qcom_dwc3_ss_write_readback(phy->base, PHY_CTRL_REG,
+ SSUSB_CTRL_REF_USE_PAD, 0x0);
+ qcom_dwc3_ss_write_readback(phy->base, PHY_CTRL_REG,
+ 0x0, SSUSB_CTRL_TEST_POWERDOWN);
+
+ usleep_range(1000, 1200);
+ clk_disable_unprepare(phy->ref_clk);
+
+ ret = regulator_set_voltage(phy->vddcx, USB_VDDCX_NO, USB_VDDCX_MAX);
+ if (ret)
+ dev_err(phy->dev, "cannot set voltage for vddcx\n");
+
+ regulator_disable(phy->vddcx);
+
+ ret = regulator_set_voltage(phy->v1p8, 0, PHY_1P8_VOL_MAX);
+ if (ret)
+ dev_err(phy->dev, "cannot set v1p8\n");
+
+ regulator_disable(phy->v1p8);
+}
+
+static int qcom_dwc3_ss_phy_init(struct usb_phy *x)
+{
+ struct qcom_dwc3_ss_phy *phy = phy_to_dw_phy(x);
+ u32 data = 0;
+ int ret;
+
+ ret = regulator_set_voltage(phy->vddcx, USB_VDDCX_MIN, USB_VDDCX_MAX);
+ if (ret)
+ dev_err(phy->dev, "cannot set voltage for vddcx\n");
+
+ ret = regulator_enable(phy->vddcx);
+ if (ret) {
+ dev_err(phy->dev, "cannot enable vddcx\n");
+ return ret;
+ }
+
+ ret = regulator_set_voltage(phy->v1p8, PHY_1P8_VOL_MIN,
+ PHY_1P8_VOL_MAX);
+ if (ret) {
+ regulator_disable(phy->vddcx);
+ dev_err(phy->dev, "cannot set v1p8\n");
+ }
+
+ ret = regulator_enable(phy->v1p8);
+ if (ret) {
+ regulator_disable(phy->vddcx);
+ dev_err(phy->dev, "cannot enable v1p8\n");
+ }
+
+ clk_prepare_enable(phy->ref_clk);
+ usleep_range(1000, 1200);
+
+ /* reset phy */
+ data = readl_relaxed(phy->base + PHY_CTRL_REG);
+ writel_relaxed(data | SSUSB_CTRL_SS_PHY_RESET,
+ phy->base + PHY_CTRL_REG);
+ usleep_range(2000, 2200);
+ writel_relaxed(data, phy->base + PHY_CTRL_REG);
+
+ /* clear REF_PAD, we don't have XO clk */
+ data &= ~SSUSB_CTRL_REF_USE_PAD;
+ writel_relaxed(data, phy->base + PHY_CTRL_REG);
+ msleep(30);
+
+ data |= SSUSB_CTRL_SS_PHY_EN | SSUSB_CTRL_LANE0_PWR_PRESENT;
+ writel_relaxed(data, phy->base + PHY_CTRL_REG);
+
+ /*
+ * WORKAROUND: There is SSPHY suspend bug due to which USB enumerates
+ * in HS mode instead of SS mode. Workaround it by asserting
+ * LANE0.TX_ALT_BLOCK.EN_ALT_BUS to enable TX to use alt bus mode
+ */
+ data = qcom_dwc3_ss_read_phycreg(phy->base, 0x102d);
+ data |= (1 << 7);
+ qcom_dwc3_ss_write_phycreg(phy->base, 0x102D, data);
+
+ data = qcom_dwc3_ss_read_phycreg(phy->base, 0x1010);
+ data &= ~0xff0;
+ data |= 0x20;
+ qcom_dwc3_ss_write_phycreg(phy->base, 0x1010, data);
+
+ /*
+ * 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
+ */
+ data = qcom_dwc3_ss_read_phycreg(phy->base, 0x1006);
+ data &= ~(1 << 6);
+ data |= (1 << 7);
+ data &= ~(0x7 << 8);
+ data |= (0x3 << 8);
+ data |= (0x1 << 11);
+ qcom_dwc3_ss_write_phycreg(phy->base, 0x1006, data);
+
+ /*
+ * 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.
+ */
+ data = qcom_dwc3_ss_read_phycreg(phy->base, 0x1002);
+ data &= ~0x3f80;
+ data |= (0x16 << 7);
+ data &= ~0x7f;
+ data |= (0x7f | (1 << 14));
+ qcom_dwc3_ss_write_phycreg(phy->base, 0x1002, data);
+
+ /*
+ * 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_ss_write_readback(phy->base, PHY_PARAM_CTRL_1,
+ 0x07f03f07, 0x07f01605);
+ return 0;
+}
+
+static int qcom_dwc3_ss_probe(struct platform_device *pdev)
+{
+ struct qcom_dwc3_ss_phy *phy;
+ struct resource *res;
+ void __iomem *base;
+ int ret;
+
+ phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
+ if (!phy)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, phy);
+
+ phy->dev = &pdev->dev;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ base = devm_ioremap_resource(phy->dev, res);
+ if (IS_ERR(base))
+ return PTR_ERR(base);
+
+ phy->vddcx = devm_regulator_get(phy->dev, "vddcx");
+ if (IS_ERR(phy->vddcx)) {
+ dev_dbg(phy->dev, "cannot get vddcx\n");
+ return PTR_ERR(phy->vddcx);
+ }
+
+ phy->v1p8 = devm_regulator_get(phy->dev, "v1p8");
+ if (IS_ERR(phy->v1p8)) {
+ dev_dbg(phy->dev, "cannot get v1p8\n");
+ return PTR_ERR(phy->v1p8);
+ }
+
+ phy->xo_clk = devm_clk_get(phy->dev, "xo");
+ if (IS_ERR(phy->xo_clk)) {
+ dev_dbg(phy->dev, "cannot get XO clk, assuming not present\n");
+ phy->xo_clk = NULL;
+ }
+
+ phy->ref_clk = devm_clk_get(phy->dev, "ref");
+ if (IS_ERR(phy->ref_clk)) {
+ dev_dbg(phy->dev, "cannot get ref clock handle\n");
+ return PTR_ERR(phy->ref_clk);
+ }
+
+ clk_set_rate(phy->ref_clk, 125000000);
+ if (phy->xo_clk)
+ clk_prepare_enable(phy->xo_clk);
+
+ phy->base = base;
+ phy->phy.dev = phy->dev;
+ phy->phy.label = "qcom-dwc3-ssphy";
+ phy->phy.init = qcom_dwc3_ss_phy_init;
+ phy->phy.shutdown = qcom_dwc3_ss_phy_shutdown;
+ phy->phy.type = USB_PHY_TYPE_USB3;
+
+ ret = usb_add_phy_dev(&phy->phy);
+ return ret;
+}
+
+static int qcom_dwc3_ss_remove(struct platform_device *pdev)
+{
+ struct qcom_dwc3_ss_phy *phy = platform_get_drvdata(pdev);
+
+ if (phy->xo_clk)
+ clk_disable_unprepare(phy->xo_clk);
+ usb_remove_phy(&phy->phy);
+ return 0;
+}
+
+static const struct of_device_id qcom_dwc3_ss_id_table[] = {
+ { .compatible = "qcom,dwc3-ssphy" },
+ { /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, qcom_dwc3_ss_id_table);
+
+static struct platform_driver qcom_dwc3_ss_driver = {
+ .probe = qcom_dwc3_ss_probe,
+ .remove = qcom_dwc3_ss_remove,
+ .driver = {
+ .name = "qcom-dwc3-ssphy",
+ .owner = THIS_MODULE,
+ .of_match_table = qcom_dwc3_ss_id_table,
+ },
+};
+
+module_platform_driver(qcom_dwc3_ss_driver);
+
+MODULE_ALIAS("platform:qcom-dwc3-ssphy");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("DesignWare USB3 QCOM SSPHY driver");
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2014-06-30 16:04:39

by Andy Gross

[permalink] [raw]
Subject: [Patch v7 3/3] usb: dwc3: qcom: Add device tree binding

From: "Ivan T. Ivanov" <[email protected]>

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 <[email protected]>
Signed-off-by: Andy Gross <[email protected]>
---
.../devicetree/bindings/usb/qcom,dwc3.txt | 104 ++++++++++++++++++++
1 file changed, 104 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/qcom,dwc3.txt

diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.txt b/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
new file mode 100644
index 0000000..105b6b7
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
@@ -0,0 +1,104 @@
+Qualcomm SuperSpeed DWC3 USB SoC controller
+
+
+QCOM DWC3 Highspeed USB PHY
+========================
+Required properities:
+- compatible: should contain "qcom,dwc3-hsphy";
+- reg: offset and length of the register set in the memory map
+- clocks: A list of phandle + clock-specifier pairs for the
+ clocks listed in clock-names
+- clock-names: Should contain the following:
+ "utmi" UTMI clock
+- v1p8-supply: phandle to the regulator for the 1.8v supply to HSPHY.
+- v3p3-supply: phandle to the regulator for the 3.3v supply to HSPHY.
+- vbus-supply: phandle to the regulator for the vbus supply for host
+ mode.
+- vddcx-supply: phandle to the regulator for the vdd supply for HSPHY
+ digital circuit operation.
+
+Optional clocks:
+ "xo" External reference clock
+
+
+QCOM DWC3 Superspeed USB PHY
+=========================
+Required properities:
+- compatible: should contain "qcom,dwc3-ssphy";
+- reg: offset and length of the register set in the memory map
+- clocks: A list of phandle + clock-specifier pairs for the
+ clocks listed in clock-names
+- clock-names: Should contain the following:
+ "ref" Reference clock used in host mode.
+- v1p8-supply: phandle to the regulator for the 1.8v supply to HSPHY.
+- vddcx-supply: phandle to the regulator for the vdd supply for HSPHY
+ digital circuit operation.
+
+Optional clocks:
+ "xo" External reference clock
+
+QCOM DWC3 controller wrapper
+===========================
+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).
+
+Optional regulator:
+- gdsc-supply: phandle to the regulator from globally distributed
+ switch controller
+
+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.
+
+Example device nodes:
+
+ hs_phy_0: phy@110f8800 {
+ compatible = "qcom,dwc3-hsphy";
+ reg = <0x110f8800 0x30>;
+ clocks = <&gcc USB30_0_UTMI_CLK>;
+ clock-names = "utmi";
+
+ status = "disabled";
+ };
+
+ ss_phy_0: phy@110f8830 {
+ compatible = "qcom,dwc3-ssphy";
+ reg = <0x110f8830 0x30>;
+
+ clocks = <&gcc USB30_0_MASTER_CLK>;
+ clock-names = "ref";
+
+ status = "disabled";
+ };
+
+ usb3_0: usb30@0 {
+ compatible = "qcom,dwc3";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ clocks = <&gcc USB30_0_MASTER_CLK>;
+ clock-names = "core";
+
+ ranges;
+
+ status = "disabled";
+
+ dwc3@11000000 {
+ compatible = "snps,dwc3";
+ reg = <0x11000000 0xcd00>;
+ interrupts = <0 110 0x4>;
+ usb-phy = <&hs_phy_0>, <&ss_phy_0>;
+ 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

2014-06-30 16:53:31

by Felipe Balbi

[permalink] [raw]
Subject: Re: [Patch v7 1/3] usb: dwc3: Add Qualcomm DWC3 glue layer driver

Hi,

On Mon, Jun 30, 2014 at 11:03:51AM -0500, Andy Gross wrote:
> +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->gdsc = devm_regulator_get(qdwc->dev, "gdsc");
> +
> + qdwc->core_clk = devm_clk_get(qdwc->dev, "core");
> + if (IS_ERR(qdwc->core_clk)) {
> + dev_dbg(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 iface clock, skipping\n");
> + qdwc->iface_clk = NULL;

so this clock and sleep_clk are optional, that's fine...

> + }
> +
> + qdwc->sleep_clk = devm_clk_get(qdwc->dev, "sleep");
> + if (IS_ERR(qdwc->sleep_clk)) {
> + dev_dbg(qdwc->dev, "failed to get sleep clock, skipping\n");
> + qdwc->sleep_clk = NULL;
> + }
> +
> + if (!IS_ERR(qdwc->gdsc)) {
> + ret = regulator_enable(qdwc->gdsc);
> + if (ret)
> + dev_err(qdwc->dev, "cannot enable gdsc\n");
> + }
> +
> + clk_prepare_enable(qdwc->core_clk);
> +
> + if (qdwc->iface_clk)
> + clk_prepare_enable(qdwc->iface_clk);

... right here you can drop the NULL check because clk_prepare_enable()
is safe against NULL pointers.

--
balbi


Attachments:
(No filename) (1.45 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2014-06-30 17:01:21

by Felipe Balbi

[permalink] [raw]
Subject: Re: [Patch v7 2/3] usb: phy: Add Qualcomm DWC3 HS/SS PHY drivers

Hi,

first of all, since this is a brand new PHY driver, could you guys use
the generic phy framework instead ? (drivers/phy)

On Mon, Jun 30, 2014 at 11:03:52AM -0500, Andy Gross wrote:
> diff --git a/drivers/usb/phy/phy-qcom-hsusb.c b/drivers/usb/phy/phy-qcom-hsusb.c
> new file mode 100644
> index 0000000..7a44b13
> --- /dev/null
> +++ b/drivers/usb/phy/phy-qcom-hsusb.c
> @@ -0,0 +1,348 @@
> +/* 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/platform_device.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/usb/phy.h>
> +
> +/**
> + * USB QSCRATCH Hardware registers
> + */
> +#define QSCRATCH_CTRL_REG (0x04)
> +#define QSCRATCH_GENERAL_CFG (0x08)
> +#define PHY_CTRL_REG (0x10)
> +#define PARAMETER_OVERRIDE_X_REG (0x14)
> +#define CHARGING_DET_CTRL_REG (0x18)
> +#define CHARGING_DET_OUTPUT_REG (0x1c)
> +#define ALT_INTERRUPT_EN_REG (0x20)
> +#define PHY_IRQ_STAT_REG (0x24)
> +#define CGCTL_REG (0x28)
> +
> +#define PHY_3P3_VOL_MIN 3050000 /* uV */
> +#define PHY_3P3_VOL_MAX 3300000 /* uV */
> +#define PHY_3P3_HPM_LOAD 16000 /* uA */
> +
> +#define PHY_1P8_VOL_MIN 1800000 /* uV */
> +#define PHY_1P8_VOL_MAX 1800000 /* uV */
> +#define PHY_1P8_HPM_LOAD 19000 /* uA */
> +
> +/* TODO: these are suspicious */
> +#define USB_VDDCX_NO 1 /* index */
> +#define USB_VDDCX_MIN 5 /* index */
> +#define USB_VDDCX_MAX 7 /* index */
> +
> +/* 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 HSYSB_CTRL_POR BIT(0)
> +
> +
> +/* QSCRATCH_GENERAL_CFG */
> +#define HSUSB_GCFG_XHCI_REV BIT(2)
> +
> +struct qcom_dwc3_hs_phy {
> + struct usb_phy phy;
> + void __iomem *base;
> + struct device *dev;
> +
> + struct clk *xo_clk;
> + struct clk *utmi_clk;
> +
> + struct regulator *v3p3;
> + struct regulator *v1p8;
> + struct regulator *vddcx;
> + struct regulator *vbus;
> +};
> +
> +#define phy_to_dw_phy(x) container_of((x), struct qcom_dwc3_hs_phy, phy)
> +
> +/**
> + * Write register.
> + *
> + * @base - QCOM DWC3 PHY base virtual address.
> + * @offset - register offset.
> + * @val - value to write.
> + */
> +static inline void qcom_dwc3_hs_write(void __iomem *base, u32 offset, u32 val)
> +{
> + writel(val, base + offset);
> +}
> +
> +/**
> + * 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_hs_write_readback(void __iomem *base, u32 offset,
> + const u32 mask, u32 val)
> +{
> + u32 write_val, tmp = readl(base + offset);
> +
> + tmp &= ~mask; /* retain other bits */
> + write_val = tmp | val;
> +
> + writel(write_val, base + offset);
> +
> + /* Read back to see if val was written */
> + tmp = readl(base + offset);
> + tmp &= mask; /* clear other bits */
> +
> + if (tmp != val)
> + pr_err("write: %x to QSCRATCH: %x FAILED\n", val, offset);

really nit-picking - and I'm not even sure I care - but passing a dev
pointer to use dev_err() here might be a good idea.

> +}
> +
> +static void qcom_dwc3_hs_phy_shutdown(struct usb_phy *x)
> +{
> + struct qcom_dwc3_hs_phy *phy = phy_to_dw_phy(x);
> + int ret;
> +
> + ret = regulator_set_voltage(phy->v3p3, 0, PHY_3P3_VOL_MAX);
> + if (ret)
> + dev_err(phy->dev, "cannot set voltage for v3p3\n");
> +
> + ret = regulator_set_voltage(phy->v1p8, 0, PHY_1P8_VOL_MAX);
> + if (ret)
> + dev_err(phy->dev, "cannot set voltage for v1p8\n");
> +
> + ret = regulator_disable(phy->v1p8);

alright, now this is really a personal doubt. Is it really necessary to
set zero 0 volts if you're going to shut it down ?

> + if (ret)
> + dev_err(phy->dev, "cannot disable v1p8\n");
> +
> + ret = regulator_disable(phy->v3p3);
> + if (ret)
> + dev_err(phy->dev, "cannot disable v3p3\n");
> +
> + ret = regulator_set_voltage(phy->vddcx, USB_VDDCX_NO, USB_VDDCX_MAX);
> + if (ret)
> + dev_err(phy->dev, "cannot set voltage for vddcx\n");
> +
> + ret = regulator_disable(phy->vddcx);
> + if (ret)
> + dev_err(phy->dev, "cannot enable vddcx\n");
> +
> + clk_disable_unprepare(phy->utmi_clk);
> +}
> +
> +static int qcom_dwc3_hs_phy_init(struct usb_phy *x)
> +{
> + struct qcom_dwc3_hs_phy *phy = phy_to_dw_phy(x);
> + int ret;
> + u32 val;
> +
> + clk_prepare_enable(phy->utmi_clk);
> +
> + ret = regulator_set_voltage(phy->vddcx, USB_VDDCX_MIN, USB_VDDCX_MAX);
> + if (ret)
> + dev_err(phy->dev, "cannot set voltage for vddcx\n");
> +
> + ret = regulator_enable(phy->vddcx);
> + if (ret)
> + dev_err(phy->dev, "cannot enable vddcx\n");
> +
> + ret = regulator_set_voltage(phy->v3p3, PHY_3P3_VOL_MIN,
> + PHY_3P3_VOL_MAX);
> + if (ret)
> + dev_err(phy->dev, "cannot set voltage for v3p3\n");
> +
> + ret = regulator_set_voltage(phy->v1p8, PHY_1P8_VOL_MIN,
> + PHY_1P8_VOL_MAX);
> + if (ret)
> + dev_err(phy->dev, "cannot set voltage for v1p8\n");
> +
> + ret = regulator_set_optimum_mode(phy->v1p8, PHY_1P8_HPM_LOAD);
> + if (ret < 0)
> + dev_err(phy->dev, "cannot set HPM of regulator v1p8\n");
> +
> + ret = regulator_enable(phy->v1p8);
> + if (ret)
> + dev_err(phy->dev, "cannot enable v1p8\n");
> +
> + ret = regulator_set_optimum_mode(phy->v3p3, PHY_3P3_HPM_LOAD);
> + if (ret < 0)
> + dev_err(phy->dev, "cannot set HPM of regulator v3p3\n");
> +
> + ret = regulator_enable(phy->v3p3);
> + if (ret)
> + dev_err(phy->dev, "cannot enable v3p3\n");
> +
> + /*
> + * 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->xo_clk)
> + val |= HSUSB_CTRL_USE_CLKCORE;
> +
> + qcom_dwc3_hs_write(phy->base, PHY_CTRL_REG, val);
> + usleep_range(2000, 2200);
> +
> + /*
> + * write HSPHY init value to QSCRATCH reg to set HSPHY parameters like
> + * VBUS valid threshold, disconnect valid threshold, DC voltage level,
> + * preempasis and rise/fall time.
> + */
> + qcom_dwc3_hs_write_readback(phy->base, PARAMETER_OVERRIDE_X_REG,
> + 0x03ffffff, 0x00d191a4);

this can be dangerous, actually. VBUS Valid Threshold, Session Valid
Threshold, etc, are all defined by the USB spec, why would you have to
change it ? Maybe this is related to some errata and should be wrapped
with a revision check or something ?

> + /* Disable (bypass) VBUS and ID filters */
> + qcom_dwc3_hs_write(phy->base, QSCRATCH_GENERAL_CFG,
> + HSUSB_GCFG_XHCI_REV);
> +
> + return 0;
> +}
> +
> +static int qcom_dwc3_hs_phy_set_vbus(struct usb_phy *x, int on)
> +{
> + struct qcom_dwc3_hs_phy *phy = phy_to_dw_phy(x);
> + int ret = 0;
> +
> + if (IS_ERR(phy->vbus))
> + return on ? PTR_ERR(phy->vbus) : 0;

this regulator seems to be optional, should you error out here if it's
not available at all ?

> +
> + if (on)
> + ret = regulator_enable(phy->vbus);
> + else
> + ret = regulator_disable(phy->vbus);
> +
> + if (ret)
> + dev_err(x->dev, "Cannot %s Vbus\n", on ? "set" : "off");
> + return ret;
> +}
> +
> +static int qcom_dwc3_hs_probe(struct platform_device *pdev)
> +{
> + struct qcom_dwc3_hs_phy *phy;
> + struct resource *res;
> + void __iomem *base;
> +
> + phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
> + if (!phy)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, phy);
> +
> + phy->dev = &pdev->dev;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + base = devm_ioremap_resource(phy->dev, res);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + phy->vddcx = devm_regulator_get(phy->dev, "vddcx");
> + if (IS_ERR(phy->vddcx)) {
> + dev_dbg(phy->dev, "cannot get vddcx\n");
> + return PTR_ERR(phy->vddcx);
> + }
> +
> + phy->v3p3 = devm_regulator_get(phy->dev, "v3p3");
> + if (IS_ERR(phy->v3p3)) {
> + dev_dbg(phy->dev, "cannot get v3p3\n");
> + return PTR_ERR(phy->v3p3);
> + }
> +
> + phy->v1p8 = devm_regulator_get(phy->dev, "v1p8");
> + if (IS_ERR(phy->v1p8)) {
> + dev_dbg(phy->dev, "cannot get v1p8\n");
> + return PTR_ERR(phy->v1p8);
> + }
> +
> + phy->vbus = devm_regulator_get(phy->dev, "vbus");
> + if (IS_ERR(phy->vbus))
> + dev_dbg(phy->dev, "Failed to get vbus\n");
> +
> + phy->utmi_clk = devm_clk_get(phy->dev, "utmi");
> + if (IS_ERR(phy->utmi_clk)) {
> + dev_dbg(phy->dev, "cannot get UTMI handle\n");
> + return PTR_ERR(phy->utmi_clk);
> + }
> +
> + phy->xo_clk = devm_clk_get(phy->dev, "xo");
> + if (IS_ERR(phy->xo_clk)) {
> + dev_dbg(phy->dev, "cannot get TCXO buffer handle\n");
> + phy->xo_clk = NULL;
> + }
> +
> + clk_set_rate(phy->utmi_clk, 60000000);
> +
> + if (phy->xo_clk)
> + clk_prepare_enable(phy->xo_clk);
> +
> + phy->base = base;
> + phy->phy.dev = phy->dev;
> + phy->phy.label = "qcom-dwc3-hsphy";
> + phy->phy.init = qcom_dwc3_hs_phy_init;
> + phy->phy.shutdown = qcom_dwc3_hs_phy_shutdown;
> + phy->phy.set_vbus = qcom_dwc3_hs_phy_set_vbus;
> + phy->phy.type = USB_PHY_TYPE_USB2;
> + phy->phy.state = OTG_STATE_UNDEFINED;
> +
> + usb_add_phy_dev(&phy->phy);
> + return 0;
> +}
> +
> +static int qcom_dwc3_hs_remove(struct platform_device *pdev)
> +{
> + struct qcom_dwc3_hs_phy *phy = platform_get_drvdata(pdev);
> +
> + if (phy->xo_clk)
> + clk_disable_unprepare(phy->xo_clk);
> +
> + clk_disable_unprepare(phy->utmi_clk);
> +
> + usb_remove_phy(&phy->phy);

you might have exposed a bug on the current PHY framework. There's no
guarantee that your regulators will be turned off when you remove this
driver. ehehehe..

ps: most comments are valid for the ssusb.c PHY driver as well. In fact,
they seem to be pretty similar; perhaps there's a way to share more code
between them ?

--
balbi


Attachments:
(No filename) (10.81 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2014-06-30 17:37:43

by Kumar Gala

[permalink] [raw]
Subject: Re: [Patch v7 3/3] usb: dwc3: qcom: Add device tree binding


On Jun 30, 2014, at 11:03 AM, Andy Gross <[email protected]> wrote:

> From: "Ivan T. Ivanov" <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Andy Gross <[email protected]>
> ---
> .../devicetree/bindings/usb/qcom,dwc3.txt | 104 ++++++++++++++++++++
> 1 file changed, 104 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/usb/qcom,dwc3.txt
>
> diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.txt b/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
> new file mode 100644
> index 0000000..105b6b7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
> @@ -0,0 +1,104 @@
> +Qualcomm SuperSpeed DWC3 USB SoC controller
> +
> +
> +QCOM DWC3 Highspeed USB PHY
> +========================
> +Required properities:
> +- compatible: should contain "qcom,dwc3-hsphy";
> +- reg: offset and length of the register set in the memory map
> +- clocks: A list of phandle + clock-specifier pairs for the
> + clocks listed in clock-names
> +- clock-names: Should contain the following:
> + "utmi" UTMI clock
> +- v1p8-supply: phandle to the regulator for the 1.8v supply to HSPHY.
> +- v3p3-supply: phandle to the regulator for the 3.3v supply to HSPHY.
> +- vbus-supply: phandle to the regulator for the vbus supply for host
> + mode.
> +- vddcx-supply: phandle to the regulator for the vdd supply for HSPHY
> + digital circuit operation.
> +
> +Optional clocks:
> + "xo" External reference clock
> +
> +
> +QCOM DWC3 Superspeed USB PHY
> +=========================
> +Required properities:
> +- compatible: should contain "qcom,dwc3-ssphy";
> +- reg: offset and length of the register set in the memory map
> +- clocks: A list of phandle + clock-specifier pairs for the
> + clocks listed in clock-names
> +- clock-names: Should contain the following:
> + "ref" Reference clock used in host mode.
> +- v1p8-supply: phandle to the regulator for the 1.8v supply to HSPHY.
> +- vddcx-supply: phandle to the regulator for the vdd supply for HSPHY
> + digital circuit operation.
> +
> +Optional clocks:
> + "xo" External reference clock
> +
> +QCOM DWC3 controller wrapper
> +===========================
> +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).

We should encode the an optional port number here in the cases that we have multiple controllers and need to know about it so we can set TCSR correctly.

> +
> +Optional regulator:
> +- gdsc-supply: phandle to the regulator from globally distributed
> + switch controller
> +
> +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.
> +
> +Example device nodes:
> +
> + hs_phy_0: phy@110f8800 {
> + compatible = "qcom,dwc3-hsphy";
> + reg = <0x110f8800 0x30>;
> + clocks = <&gcc USB30_0_UTMI_CLK>;
> + clock-names = "utmi";
> +
> + status = "disabled";
> + };
> +
> + ss_phy_0: phy@110f8830 {
> + compatible = "qcom,dwc3-ssphy";
> + reg = <0x110f8830 0x30>;
> +
> + clocks = <&gcc USB30_0_MASTER_CLK>;
> + clock-names = "ref";
> +
> + status = "disabled";
> + };
> +
> + usb3_0: usb30@0 {
> + compatible = "qcom,dwc3";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + clocks = <&gcc USB30_0_MASTER_CLK>;
> + clock-names = "core";
> +
> + ranges;
> +
> + status = "disabled";
> +
> + dwc3@11000000 {
> + compatible = "snps,dwc3";
> + reg = <0x11000000 0xcd00>;
> + interrupts = <0 110 0x4>;
> + usb-phy = <&hs_phy_0>, <&ss_phy_0>;
> + 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
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

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

2014-07-01 05:04:58

by Rob Herring

[permalink] [raw]
Subject: Re: [Patch v7 3/3] usb: dwc3: qcom: Add device tree binding

On Mon, Jun 30, 2014 at 11:03 AM, Andy Gross <[email protected]> wrote:
> From: "Ivan T. Ivanov" <[email protected]>

Please copy the right lists and maintainers.

>
> 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 <[email protected]>
> Signed-off-by: Andy Gross <[email protected]>
> ---
> .../devicetree/bindings/usb/qcom,dwc3.txt | 104 ++++++++++++++++++++
> 1 file changed, 104 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/usb/qcom,dwc3.txt
>
> diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.txt b/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
> new file mode 100644
> index 0000000..105b6b7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
> @@ -0,0 +1,104 @@
> +Qualcomm SuperSpeed DWC3 USB SoC controller
> +
> +
> +QCOM DWC3 Highspeed USB PHY
> +========================
> +Required properities:
> +- compatible: should contain "qcom,dwc3-hsphy";
> +- reg: offset and length of the register set in the memory map
> +- clocks: A list of phandle + clock-specifier pairs for the
> + clocks listed in clock-names
> +- clock-names: Should contain the following:
> + "utmi" UTMI clock
> +- v1p8-supply: phandle to the regulator for the 1.8v supply to HSPHY.
> +- v3p3-supply: phandle to the regulator for the 3.3v supply to HSPHY.
> +- vbus-supply: phandle to the regulator for the vbus supply for host
> + mode.
> +- vddcx-supply: phandle to the regulator for the vdd supply for HSPHY
> + digital circuit operation.
> +
> +Optional clocks:
> + "xo" External reference clock
> +
> +
> +QCOM DWC3 Superspeed USB PHY
> +=========================
> +Required properities:
> +- compatible: should contain "qcom,dwc3-ssphy";
> +- reg: offset and length of the register set in the memory map
> +- clocks: A list of phandle + clock-specifier pairs for the
> + clocks listed in clock-names
> +- clock-names: Should contain the following:
> + "ref" Reference clock used in host mode.
> +- v1p8-supply: phandle to the regulator for the 1.8v supply to HSPHY.
> +- vddcx-supply: phandle to the regulator for the vdd supply for HSPHY
> + digital circuit operation.
> +
> +Optional clocks:
> + "xo" External reference clock
> +
> +QCOM DWC3 controller wrapper
> +===========================
> +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

Really?, some platforms have a clockless bus?

> + "sleep" Sleep clock, used when USB3 core goes into low
> + power mode (U3).
> +
> +Optional regulator:
> +- gdsc-supply: phandle to the regulator from globally distributed
> + switch controller

The name should reflect the name of the input, not the source.

> +
> +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.
> +
> +Example device nodes:
> +
> + hs_phy_0: phy@110f8800 {
> + compatible = "qcom,dwc3-hsphy";
> + reg = <0x110f8800 0x30>;
> + clocks = <&gcc USB30_0_UTMI_CLK>;
> + clock-names = "utmi";
> +
> + status = "disabled";
> + };
> +
> + ss_phy_0: phy@110f8830 {
> + compatible = "qcom,dwc3-ssphy";
> + reg = <0x110f8830 0x30>;
> +
> + clocks = <&gcc USB30_0_MASTER_CLK>;
> + clock-names = "ref";
> +
> + status = "disabled";
> + };
> +
> + usb3_0: usb30@0 {
> + compatible = "qcom,dwc3";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + clocks = <&gcc USB30_0_MASTER_CLK>;
> + clock-names = "core";
> +
> + ranges;
> +
> + status = "disabled";
> +
> + dwc3@11000000 {
> + compatible = "snps,dwc3";

This sub-node is just wrong. Why can't you have a single node with '
"qcom,dwc3", "snps,dwc3" ' for the compatible property? All you are
adding here is clocks. Does the Synopsys block have no clocks?

I guess this is copied from other broken dwc3 bindings... That doesn't
mean you have to copy it.

Rob

2014-07-01 18:01:17

by Andy Gross

[permalink] [raw]
Subject: Re: [Patch v7 3/3] usb: dwc3: qcom: Add device tree binding

On Tue, Jul 01, 2014 at 12:04:35AM -0500, Rob Herring wrote:

<snip>

> > +- 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
>
> Really?, some platforms have a clockless bus?

Some platforms require core and interface. The specific platform I tested on
does not have an iface clk. I'll take a look at the ipq block diagram to see if
they did something cute, but i don't believe there is one.

>
> > + "sleep" Sleep clock, used when USB3 core goes into low
> > + power mode (U3).
> > +
> > +Optional regulator:
> > +- gdsc-supply: phandle to the regulator from globally distributed
> > + switch controller
>
> The name should reflect the name of the input, not the source.

Ok, I'll revisit this. I took this from the original patch set.

<snip>

> > +
> > + ranges;
> > +
> > + status = "disabled";
> > +
> > + dwc3@11000000 {
> > + compatible = "snps,dwc3";
>
> This sub-node is just wrong. Why can't you have a single node with '
> "qcom,dwc3", "snps,dwc3" ' for the compatible property? All you are
> adding here is clocks. Does the Synopsys block have no clocks?
>
> I guess this is copied from other broken dwc3 bindings... That doesn't
> mean you have to copy it.

The dwc3 core does not deal with clocks. That is why everyone has a wrapper.
That, in addition to pm, has to be handled from the wrapper. That's my take
anyway. I am sure Felipe can speak more to this.

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

2014-07-01 19:48:11

by Rob Herring

[permalink] [raw]
Subject: Re: [Patch v7 3/3] usb: dwc3: qcom: Add device tree binding

On Tue, Jul 1, 2014 at 1:01 PM, Andy Gross <[email protected]> wrote:
> On Tue, Jul 01, 2014 at 12:04:35AM -0500, Rob Herring wrote:
>
> <snip>
>
>> > +- 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
>>
>> Really?, some platforms have a clockless bus?
>
> Some platforms require core and interface. The specific platform I tested on
> does not have an iface clk. I'll take a look at the ipq block diagram to see if
> they did something cute, but i don't believe there is one.

Usually, that difference just means all the clock inputs are connected
to the same clock source. But the binding should describe the inputs.
If the dwc3 core has 3 clocks and your wrapper logic varies, I would
say you should follow the dwc3 core and ignore the wrapper for
purposes of the binding (unless there is some complex clock tree
there).

> <snip>
>
>> > +
>> > + ranges;
>> > +
>> > + status = "disabled";
>> > +
>> > + dwc3@11000000 {
>> > + compatible = "snps,dwc3";
>>
>> This sub-node is just wrong. Why can't you have a single node with '
>> "qcom,dwc3", "snps,dwc3" ' for the compatible property? All you are
>> adding here is clocks. Does the Synopsys block have no clocks?
>>
>> I guess this is copied from other broken dwc3 bindings... That doesn't
>> mean you have to copy it.
>
> The dwc3 core does not deal with clocks. That is why everyone has a wrapper.
> That, in addition to pm, has to be handled from the wrapper. That's my take
> anyway. I am sure Felipe can speak more to this.

That is a Linux driver issue which is irrelevant to the binding.

Rob

2014-07-02 08:43:59

by Ivan T. Ivanov

[permalink] [raw]
Subject: Re: [Patch v7 3/3] usb: dwc3: qcom: Add device tree binding


Hi,

On Tue, 2014-07-01 at 14:47 -0500, Rob Herring wrote:
> On Tue, Jul 1, 2014 at 1:01 PM, Andy Gross <[email protected]> wrote:
> > On Tue, Jul 01, 2014 at 12:04:35AM -0500, Rob Herring wrote:

<snip>

> > <snip>
> >
> >> > +
> >> > + ranges;
> >> > +
> >> > + status = "disabled";
> >> > +
> >> > + dwc3@11000000 {
> >> > + compatible = "snps,dwc3";
> >>
> >> This sub-node is just wrong. Why can't you have a single node with '
> >> "qcom,dwc3", "snps,dwc3" ' for the compatible property? All you are
> >> adding here is clocks. Does the Synopsys block have no clocks?
> >>
> >> I guess this is copied from other broken dwc3 bindings... That doesn't
> >> mean you have to copy it.
> >
> > The dwc3 core does not deal with clocks. That is why everyone has a wrapper.
> > That, in addition to pm, has to be handled from the wrapper. That's my take
> > anyway. I am sure Felipe can speak more to this.
>
> That is a Linux driver issue which is irrelevant to the binding.

DWC3 IP core from Synopsys is the same across SoC's (OMAP, Exynos..),
vendors are adding glue IP to provide necessary clocks and voltages.
I think that the DT bindings properly reflect this fact.

Regards,
Ivan

>
> Rob

2014-07-02 12:48:49

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [Patch v7 3/3] usb: dwc3: qcom: Add device tree binding

On Wednesday 02 July 2014 11:43:25 Ivan T. Ivanov wrote:
> > > <snip>
> > >
> > >> > +
> > >> > + ranges;
> > >> > +
> > >> > + status = "disabled";
> > >> > +
> > >> > + dwc3@11000000 {
> > >> > + compatible = "snps,dwc3";
> > >>
> > >> This sub-node is just wrong. Why can't you have a single node with '
> > >> "qcom,dwc3", "snps,dwc3" ' for the compatible property? All you are
> > >> adding here is clocks. Does the Synopsys block have no clocks?
> > >>
> > >> I guess this is copied from other broken dwc3 bindings... That doesn't
> > >> mean you have to copy it.
> > >
> > > The dwc3 core does not deal with clocks. That is why everyone has a wrapper.
> > > That, in addition to pm, has to be handled from the wrapper. That's my take
> > > anyway. I am sure Felipe can speak more to this.
> >
> > That is a Linux driver issue which is irrelevant to the binding.
>
> DWC3 IP core from Synopsys is the same across SoC's (OMAP, Exynos..),
> vendors are adding glue IP to provide necessary clocks and voltages.
> I think that the DT bindings properly reflect this fact.

Not really. The proper way to do this is to have a single node that
lists multiple compatible strings from the most specific (per SoC variant)
to most generic (the underlying IP core) and have all properties in it.

We have seen many cases before where it later turned out that whatever
feature people thought was SoC specific actually was common to all
of them and that we later want to change the code to handle it in a
common way, and to reflect it in the common binding. The clocks that
Rob mentioned are one example of that.

If you have a binding that separates one IP block into two device nodes,
you cannot later adapt the common driver to be more generic and handle
all sorts of SoCs. See the usb-ehci.txt for an example: it started out
really simple but the generic driver has been extended multiple times
to the point where it handles a lot of SoCs by default.

Arnd

2014-07-02 15:54:38

by Felipe Balbi

[permalink] [raw]
Subject: Re: [Patch v7 3/3] usb: dwc3: qcom: Add device tree binding

Hi,

On Wed, Jul 02, 2014 at 02:48:17PM +0200, Arnd Bergmann wrote:
> On Wednesday 02 July 2014 11:43:25 Ivan T. Ivanov wrote:
> > > > <snip>
> > > >
> > > >> > +
> > > >> > + ranges;
> > > >> > +
> > > >> > + status = "disabled";
> > > >> > +
> > > >> > + dwc3@11000000 {
> > > >> > + compatible = "snps,dwc3";
> > > >>
> > > >> This sub-node is just wrong. Why can't you have a single node with '
> > > >> "qcom,dwc3", "snps,dwc3" ' for the compatible property? All you are
> > > >> adding here is clocks. Does the Synopsys block have no clocks?
> > > >>
> > > >> I guess this is copied from other broken dwc3 bindings... That doesn't
> > > >> mean you have to copy it.
> > > >
> > > > The dwc3 core does not deal with clocks. That is why everyone has a wrapper.

everyone has a wrapper for several others reasons. PM from the point of
view of the Synopsys IP is *completely* different from PM from the point
of view the $current_soc. The wrapper helps abstract that. Currently,
there's little PM implemented in the driver, but when we get to
implementing the nice features Synopsys has given us (hibernation, for
instance) it will start to be aparent why the driver was split like
that.

> > > > That, in addition to pm, has to be handled from the wrapper. That's my take
> > > > anyway. I am sure Felipe can speak more to this.
> > >
> > > That is a Linux driver issue which is irrelevant to the binding.
> >
> > DWC3 IP core from Synopsys is the same across SoC's (OMAP, Exynos..),
> > vendors are adding glue IP to provide necessary clocks and voltages.
> > I think that the DT bindings properly reflect this fact.
>
> Not really. The proper way to do this is to have a single node that
> lists multiple compatible strings from the most specific (per SoC variant)
> to most generic (the underlying IP core) and have all properties in it.
>
> We have seen many cases before where it later turned out that whatever
> feature people thought was SoC specific actually was common to all
> of them and that we later want to change the code to handle it in a
> common way, and to reflect it in the common binding. The clocks that
> Rob mentioned are one example of that.
>
> If you have a binding that separates one IP block into two device nodes,
> you cannot later adapt the common driver to be more generic and handle
> all sorts of SoCs. See the usb-ehci.txt for an example: it started out
> really simple but the generic driver has been extended multiple times
> to the point where it handles a lot of SoCs by default.

The fact is that you _DO_ have *TWO* IP blocks. The synopsys DWC3 is one
IP and the wrapper is another IP which adapts the synopsys IP to the
SoC's integration context.

From the SoC point of view, the device only needs one (or maybe two)
clocks, an IRQ line, etc... The wrapper then, sometimes, enables that
particular memory region, enables IRQs and generates several other
clocks which are needed for proper operation of the IP.

Having this sort of "bridge" or "glue" driver also helps *HUGELY* in
different PM methodologies different SoCs use. I certainly don't want
any clock API calls in my dwc3 driver when I'm running on top of a PCI
bus on an intel platform. Likewise, I don't want any pci_enable_device()
calls in my dwc3 driver when I'm running on OMAP/Exynos/Qcom/etc.

Now, you guys just decided to give crap to the authors for no aparent
reason. If you really want to describe the HW then every single clock
node and every single voltage rail would have to be described but that
would just create a whole bunch of fixed clocks and fixed regulators
which have no way of being controlled whatsoever and just waste memory
due to several other instances of such drivers being needed.

It's pointless to continue discussing if we should have one clock node
or two clock nodes. If the wrapper doesn't need an interface clock, it
just doesn't need an interface clock and it's not up to us to start
*GUESSING* that it maybe has both clock inputs tied to the same clock
node if that's not what's written in the documentation. Sure, Qcom folks
could ask their HW designers, but what would that give us in practice ?
Nothing, not a single damn benefit.

So can we just move on from this pointless discussion now ?

cheers

--
balbi


Attachments:
(No filename) (4.26 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2014-07-17 10:36:52

by Kiran Padwal

[permalink] [raw]
Subject: RE: [Patch v7 2/3] usb: phy: Add Qualcomm DWC3 HS/SS PHY drivers

Hi,

On Mon, Jun 30, 2014 at 9:33 PM, Andy Gross <[email protected]> wrote:
> From: "Ivan T. Ivanov" <[email protected]>
>
> These drivers handles control and configuration of the HS
> and SS USB PHY transceivers. They are part of the driver
.
[snip]
.
> +#include <linux/usb/phy.h>
> +
> +/**
> + * USB QSCRATCH Hardware registers
> + */

unused define, can you please remove it

> +#define QSCRATCH_CTRL_REG (0x04)
> +#define QSCRATCH_GENERAL_CFG (0x08)
> +#define PHY_CTRL_REG (0x10)
> +#define PARAMETER_OVERRIDE_X_REG (0x14)

ditto

> +#define CHARGING_DET_CTRL_REG (0x18)

ditto

> +#define CHARGING_DET_OUTPUT_REG (0x1c)

ditto

> +#define ALT_INTERRUPT_EN_REG (0x20)

ditto

> +#define PHY_IRQ_STAT_REG (0x24)

ditto

> +#define CGCTL_REG (0x28)
> +
> +#define PHY_3P3_VOL_MIN 3050000 /* uV */
> +#define PHY_3P3_VOL_MAX 3300000 /* uV */
> +#define PHY_3P3_HPM_LOAD 16000 /* uA */
> +
.
[snip]
.
> +MODULE_ALIAS("platform:qcom-dwc3-ssphy");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("DesignWare USB3 QCOM SSPHY driver");
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

Regards,
Kiran Padwal

2014-07-17 18:28:30

by Andy Gross

[permalink] [raw]
Subject: Re: [Patch v7 2/3] usb: phy: Add Qualcomm DWC3 HS/SS PHY drivers

On Thu, Jul 17, 2014 at 06:30:24AM -0400, [email protected] wrote:
> Hi,
>
> On Mon, Jun 30, 2014 at 9:33 PM, Andy Gross <[email protected]> wrote:
> > From: "Ivan T. Ivanov" <[email protected]>
> >
> > These drivers handles control and configuration of the HS
> > and SS USB PHY transceivers. They are part of the driver
> .
> [snip]
> .

I'll remove all the unused/unnecessary defines in the next version.

Thanks!

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

2014-07-18 02:10:33

by Jingoo Han

[permalink] [raw]
Subject: Re: [Patch v7 1/3] usb: dwc3: Add Qualcomm DWC3 glue layer driver

On Tuesday, July 01, 2014 1:04 AM, Felipe Balbi wrote:
>
> From: "Ivan T. Ivanov" <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Andy Gross <[email protected]>
> ---
> drivers/usb/dwc3/Kconfig | 9 +++
> drivers/usb/dwc3/Makefile | 1 +
> drivers/usb/dwc3/dwc3-qcom.c | 153 ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 163 insertions(+)
> create mode 100644 drivers/usb/dwc3/dwc3-qcom.c
>
> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> index 8eb996e..29fcbfd 100644
> --- a/drivers/usb/dwc3/Kconfig
> +++ b/drivers/usb/dwc3/Kconfig
> @@ -79,6 +79,15 @@ 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"
> + default USB_DWC3

How about adding the machine dependency? Ex. ARCH_QCOM?
Currently, other dwc3 glue layers are using ARCH_* dependency.


> + select USB_QCOM_DWC3_PHY
> + help
> + Recent Qualcomm SoCs ship with one DesignWare Core USB3 IP inside,
> + say 'Y' or 'M' if you have one such device.
> +
> +

Please remove an unnecessary line. One line is enough.

> 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..e99764a
> --- /dev/null
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -0,0 +1,153 @@
> +/* 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>
> +#include <linux/regulator/consumer.h>
> +#include <linux/usb/phy.h>
> +
> +#include "core.h"
> +
> +

Please remove an unnecessary line. One line is enough.

> +struct dwc3_qcom {
> + struct device *dev;
> +
> + struct clk *core_clk;
> + struct clk *iface_clk;
> + struct clk *sleep_clk;
> +
> + struct regulator *gdsc;
> +};
> +
> +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->gdsc = devm_regulator_get(qdwc->dev, "gdsc");
> +
> + qdwc->core_clk = devm_clk_get(qdwc->dev, "core");
> + if (IS_ERR(qdwc->core_clk)) {
> + dev_dbg(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 iface clock, skipping\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 sleep clock, skipping\n");
> + qdwc->sleep_clk = NULL;
> + }
> +
> + if (!IS_ERR(qdwc->gdsc)) {
> + ret = regulator_enable(qdwc->gdsc);
> + if (ret)
> + dev_err(qdwc->dev, "cannot enable gdsc\n");
> + }
> +
> + clk_prepare_enable(qdwc->core_clk);
> +
> + if (qdwc->iface_clk)
> + clk_prepare_enable(qdwc->iface_clk);
> +
> + if (qdwc->sleep_clk)
> + clk_prepare_enable(qdwc->sleep_clk);
> +
> + ret = of_platform_populate(node, NULL, NULL, qdwc->dev);
> + if (ret) {
> + dev_err(qdwc->dev, "failed to register core - %d\n", ret);
> + dev_dbg(qdwc->dev, "failed to add create dwc3 core\n");

Is there any reason to use two similar messages?
For me, it looks duplicated.

> + goto dis_clks;
> + }
> +
> + return 0;
> +
> +dis_clks:
> + if (qdwc->sleep_clk)
> + clk_disable_unprepare(qdwc->sleep_clk);
> +
> + if (qdwc->iface_clk)
> + clk_disable_unprepare(qdwc->iface_clk);
> +
> + clk_disable_unprepare(qdwc->core_clk);
> +
> + if (!IS_ERR(qdwc->gdsc)) {
> + ret = regulator_disable(qdwc->gdsc);
> + if (ret)
> + dev_dbg(qdwc->dev, "cannot disable gdsc\n");
> + }
> +
> + return ret;
> +}
> +
> +static int dwc3_qcom_remove(struct platform_device *pdev)
> +{
> + int ret = 0;
> +
> + struct dwc3_qcom *qdwc = platform_get_drvdata(pdev);
> +
> + if (qdwc->sleep_clk)
> + clk_disable_unprepare(qdwc->sleep_clk);
> +
> + if (qdwc->iface_clk)
> + clk_disable_unprepare(qdwc->iface_clk);
> +
> + clk_disable_unprepare(qdwc->core_clk);
> +
> + if (!IS_ERR(qdwc->gdsc)) {
> + ret = regulator_disable(qdwc->gdsc);
> + if (ret)
> + dev_dbg(qdwc->dev, "cannot disable gdsc\n");
> + }
> + return ret;
> +}
> +
> +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");

How about adding the module author as Ivan T. Ivanov and you?

MODULE_AUTHOR("Ivan T. Ivanov <[email protected]>");
MODULE_AUTHOR("Andy Gross <[email protected]>");

Best regards,
Jingoo Han

> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("DesignWare USB3 QCOM Glue Layer");
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation