2018-03-13 10:39:47

by Manu Gautam

[permalink] [raw]
Subject: [PATCH v1 1/2] dt-bindings: usb: Update documentation for Qualcomm DWC3 driver

Existing documentation has lot of incorrect information as it
was originally added for a driver that no longer exists.

Signed-off-by: Manu Gautam <[email protected]>
---
.../devicetree/bindings/usb/qcom,dwc3.txt | 87 +++++++++++++++-------
1 file changed, 59 insertions(+), 28 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.txt b/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
index bc8a2fa..df312f7 100644
--- a/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
+++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
@@ -1,54 +1,85 @@
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).
+- compatible: should contain "qcom,dwc3"
+- reg: offset and length of register set for QSCRATCH wrapper
+- reg-names: should be "qscratch"
+- power-domains: specifies a phandle to PM domain provider node
+- clocks: list of phandle + clock-specifier pairs
+- assigned-clocks: should be:
+ MOCK_UTMI_CLK
+ MASTER_CLK
+- assigned-clock-rates: should be:
+ 19.2Mhz (192000000) for MOCK_UTMI_CLK
+ >=125Mhz (125000000) for MASTER_CLK in SS mode
+ >=60Mhz (60000000) for MASTER_CLK in HS mode
+
+Optional properties:
+- resets: list of phandle and reset specifier pairs
+- interrupts: specifies interrupts from controller wrapper used
+ to wakeup from low power/susepnd state. Must contain
+ one or more entry for interrupt-names property
+- interrupt-names: Must include the following entries:
+ - "hs_phy_irq": The interrupt that is asserted when a
+ wakeup event is received on USB2 bus
+ - "ss_phy_irq": The interrupt that is asserted when a
+ wakeup event is received on USB3 bus
+ - "dm_hs_phy_irq" and "dp_hs_phy_irq": Separate
+ interrupts for any wakeup event on DM and DP lines
+- qcom,select-utmi-as-pipe-clk: if present, disable USB3 pipe_clk requirement.
+ Used when dwc3 operates without SSPHY and only
+ HS/FS/LS modes are supported.

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
+Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt - USB3 QMP PHY
+Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt - USB2 QUSB2 PHY

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>;
-
+ compatible = "qcom,qusb2-v2-phy";
+ ...
};

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>;
-
+ compatible = "qcom,qmp-v3-usb3-phy";
+ ...
};

- usb3_0: usb30@0 {
+ usb3_0: usb30@a6f8800 {
compatible = "qcom,dwc3";
+ reg = <0xa6f8800 0x400>;
+ reg-names = "qscratch";
#address-cells = <1>;
#size-cells = <1>;
- clocks = <&gcc USB30_0_MASTER_CLK>;
- clock-names = "core";
-
ranges;

+ interrupts = <0 131 0>, <0 486 0>, <0 488 0>, <0 489 0>;
+ interrupt-names = "hs_phy_irq", "ss_phy_irq",
+ "dm_hs_phy_irq", "dp_hs_phy_irq";
+
+ clocks = <&clock_gcc GCC_CFG_NOC_USB3_PRIM_AXI_CLK>,
+ <&clock_gcc GCC_USB30_PRIM_MASTER_CLK>,
+ <&clock_gcc GCC_AGGRE_USB3_PRIM_AXI_CLK>,
+ <&clock_gcc GCC_USB30_PRIM_MOCK_UTMI_CLK>,
+ <&clock_gcc GCC_USB30_PRIM_SLEEP_CLK>,
+ <&clock_gcc GCC_USB_PHY_CFG_AHB2PHY_CLK>,
+ <&clock_gcc GCC_USB3_PRIM_CLKREF_CLK>;
+ clock-names = "cfg_noc", "core", "aggr_noc", "utmi",
+ "sleep", "cfg_ahb", "ref";
+
+ assigned-clocks = <&clock_gcc GCC_USB30_PRIM_MOCK_UTMI_CLK>,
+ <&clock_gcc GCC_USB30_PRIM_MASTER_CLK>;
+ assigned-clock-rates = <19200000>, <133000000>;
+
+ resets = <&clock_gcc GCC_USB30_PRIM_BCR>;
+ reset-names = "core_reset";
+ power-domains = <&clock_gcc USB30_PRIM_GDSC>;
+ qcom,select-utmi-as-pipe-clk;

dwc3@10000000 {
compatible = "snps,dwc3";
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



2018-03-13 10:43:32

by Manu Gautam

[permalink] [raw]
Subject: [PATCH v1 2/2] usb: dwc3: Add Qualcomm DWC3 glue driver

DWC3 controller on Qualcomm SOCs has a Qscratch wrapper.
Some of its uses are described below resulting in need to
have a separate glue driver instead of using dwc3-of-simple:
- It exposes register interface to override vbus-override
and lane0-pwr-present signals going to hardware. These
must be updated in peripheral mode for DWC3 if vbus lines
are not connected to hardware block. Otherwise RX termination
in SS mode or DP pull-up is not applied by device controller.
- pwr_events_irq_stat support to ensure USB2 PHY is in L2 state
before glue driver can turn-off clocks and suspend PHY.
- Support for wakeup interrupts lines that are asserted whenever
there is any wakeup event on USB3 or USB2 bus.
- Support to replace pip3 clock going to DWC3 with utmi clock
for hardware configuration where SSPHY is not used with DWC3.

Other than above hardware features in Qscratch wrapper there
are some limitations on QCOM SOCs that require special handling
of power management e.g. suspending PHY using GUSB2PHYCFG
register and ensuring PHY enters L2 before turning off clocks etc.

Signed-off-by: Manu Gautam <[email protected]>
---
drivers/usb/dwc3/Kconfig | 11 +
drivers/usb/dwc3/Makefile | 1 +
drivers/usb/dwc3/dwc3-of-simple.c | 1 -
drivers/usb/dwc3/dwc3-qcom.c | 635 ++++++++++++++++++++++++++++++++++++++
4 files changed, 647 insertions(+), 1 deletion(-)
create mode 100644 drivers/usb/dwc3/dwc3-qcom.c

diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index ab8c0e0..f5bb4f1 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -106,4 +106,15 @@ config USB_DWC3_ST
inside (i.e. STiH407).
Say 'Y' or 'M' if you have one such device.

+config USB_DWC3_QCOM
+ tristate "Qualcomm Platform"
+ depends on ARCH_QCOM || COMPILE_TEST
+ depends on OF
+ default USB_DWC3
+ help
+ Some Qualcomm SoCs use DesignWare Core IP for USB2/3
+ functionality.
+
+ Say 'Y' or 'M' if you have one such device.
+
endif
diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
index 7ac7250..c3ce697 100644
--- a/drivers/usb/dwc3/Makefile
+++ b/drivers/usb/dwc3/Makefile
@@ -48,3 +48,4 @@ obj-$(CONFIG_USB_DWC3_PCI) += dwc3-pci.o
obj-$(CONFIG_USB_DWC3_KEYSTONE) += dwc3-keystone.o
obj-$(CONFIG_USB_DWC3_OF_SIMPLE) += dwc3-of-simple.o
obj-$(CONFIG_USB_DWC3_ST) += dwc3-st.o
+obj-$(CONFIG_USB_DWC3_QCOM) += dwc3-qcom.o
diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c
index cb2ee96..0fd0e8e 100644
--- a/drivers/usb/dwc3/dwc3-of-simple.c
+++ b/drivers/usb/dwc3/dwc3-of-simple.c
@@ -208,7 +208,6 @@ static int dwc3_of_simple_runtime_resume(struct device *dev)
};

static const struct of_device_id of_dwc3_simple_match[] = {
- { .compatible = "qcom,dwc3" },
{ .compatible = "rockchip,rk3399-dwc3" },
{ .compatible = "xlnx,zynqmp-dwc3" },
{ .compatible = "cavium,octeon-7130-usb-uctl" },
diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
new file mode 100644
index 0000000..917199e
--- /dev/null
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -0,0 +1,635 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ *
+ * Inspired by dwc3-of-simple.c
+ */
+
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/clk.h>
+#include <linux/irq.h>
+#include <linux/clk-provider.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/extcon.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/phy/phy.h>
+#include <linux/usb/of.h>
+#include <linux/reset.h>
+#include <linux/iopoll.h>
+
+#include "core.h"
+#include "io.h"
+
+/* USB QSCRATCH Hardware registers */
+#define QSCRATCH_HS_PHY_CTRL 0x10
+#define UTMI_OTG_VBUS_VALID BIT(20)
+#define SW_SESSVLD_SEL BIT(28)
+
+#define QSCRATCH_SS_PHY_CTRL 0x30
+#define LANE0_PWR_PRESENT BIT(24)
+
+#define QSCRATCH_GENERAL_CFG 0x08
+#define PIPE_UTMI_CLK_SEL BIT(0)
+#define PIPE3_PHYSTATUS_SW BIT(3)
+#define PIPE_UTMI_CLK_DIS BIT(8)
+
+#define PWR_EVNT_IRQ_STAT_REG 0x58
+#define PWR_EVNT_LPM_IN_L2_MASK BIT(4)
+#define PWR_EVNT_LPM_OUT_L2_MASK BIT(5)
+
+#define HSPHY_L2_ENTER_TIMEOUT_US 5000
+
+struct dwc3_qcom {
+ struct device *dev;
+ void __iomem *qscratch_base;
+ struct platform_device *dwc3;
+ struct clk **clks;
+ int num_clocks;
+ struct reset_control *resets;
+
+ int dp_hs_phy_irq;
+ int dm_hs_phy_irq;
+ int ss_phy_irq;
+
+ struct extcon_dev *edev;
+ struct extcon_dev *host_edev;
+ struct notifier_block vbus_nb;
+ struct notifier_block host_nb;
+
+ enum usb_dr_mode mode;
+ bool is_suspended;
+ bool pm_suspended;
+};
+
+static inline void dwc3_qcom_setbits(void __iomem *base, u32 offset, u32 val)
+{
+ u32 reg;
+
+ reg = readl(base + offset);
+ reg |= val;
+ writel(reg, base + offset);
+
+ /* ensure that above write is through */
+ readl(base + offset);
+}
+
+static inline void dwc3_qcom_clrbits(void __iomem *base, u32 offset, u32 val)
+{
+ u32 reg;
+
+ reg = readl(base + offset);
+ reg &= ~val;
+ writel(reg, base + offset);
+
+ /* ensure that above write is through */
+ readl(base + offset);
+}
+
+static void dwc3_qcom_vbus_overrride_enable(struct dwc3_qcom *qcom, bool enable)
+{
+ if (enable) {
+ dwc3_qcom_setbits(qcom->qscratch_base, QSCRATCH_SS_PHY_CTRL,
+ LANE0_PWR_PRESENT);
+ dwc3_qcom_setbits(qcom->qscratch_base, QSCRATCH_HS_PHY_CTRL,
+ UTMI_OTG_VBUS_VALID | SW_SESSVLD_SEL);
+ } else {
+ dwc3_qcom_clrbits(qcom->qscratch_base, QSCRATCH_SS_PHY_CTRL,
+ LANE0_PWR_PRESENT);
+ dwc3_qcom_clrbits(qcom->qscratch_base, QSCRATCH_HS_PHY_CTRL,
+ UTMI_OTG_VBUS_VALID | SW_SESSVLD_SEL);
+ }
+}
+
+static int dwc3_qcom_vbus_notifier(struct notifier_block *nb,
+ unsigned long event, void *ptr)
+{
+ struct dwc3_qcom *qcom = container_of(nb, struct dwc3_qcom, vbus_nb);
+
+ dev_dbg(qcom->dev, "vbus:%ld event received\n", event);
+
+ /* enable vbus override for device mode */
+ dwc3_qcom_vbus_overrride_enable(qcom, event);
+ qcom->mode = event ? USB_DR_MODE_PERIPHERAL : USB_DR_MODE_HOST;
+
+ return NOTIFY_DONE;
+}
+
+static int dwc3_qcom_host_notifier(struct notifier_block *nb,
+ unsigned long event, void *ptr)
+{
+ struct dwc3_qcom *qcom = container_of(nb, struct dwc3_qcom, host_nb);
+
+ dev_dbg(qcom->dev, "host:%ld event received\n", event);
+
+ /* disable vbus override in host mode */
+ dwc3_qcom_vbus_overrride_enable(qcom, !event);
+ qcom->mode = event ? USB_DR_MODE_HOST : USB_DR_MODE_PERIPHERAL;
+
+ return NOTIFY_DONE;
+}
+
+static int dwc3_qcom_register_extcon(struct dwc3_qcom *qcom)
+{
+ struct device *dev = qcom->dev;
+ struct extcon_dev *host_edev;
+ int ret;
+
+ if (!of_property_read_bool(dev->of_node, "extcon"))
+ return 0;
+
+ qcom->edev = extcon_get_edev_by_phandle(dev, 0);
+ if (IS_ERR(qcom->edev))
+ return PTR_ERR(qcom->edev);
+
+ qcom->vbus_nb.notifier_call = dwc3_qcom_vbus_notifier;
+
+ qcom->host_edev = extcon_get_edev_by_phandle(dev, 1);
+ if (IS_ERR(qcom->host_edev)) {
+ qcom->host_edev = NULL;
+ dev_info(dev, "No separate ID/host extcon device\n");
+ }
+
+ ret = devm_extcon_register_notifier(dev, qcom->edev, EXTCON_USB,
+ &qcom->vbus_nb);
+ if (ret < 0) {
+ dev_err(dev, "VBUS notifier register failed\n");
+ return ret;
+ }
+
+ if (qcom->host_edev)
+ host_edev = qcom->host_edev;
+ else
+ host_edev = qcom->edev;
+
+ qcom->host_nb.notifier_call = dwc3_qcom_host_notifier;
+ ret = devm_extcon_register_notifier(dev, host_edev, EXTCON_USB_HOST,
+ &qcom->host_nb);
+ if (ret < 0) {
+ dev_err(dev, "Host notifier register failed\n");
+ return ret;
+ }
+
+ /* Update initial VBUS override state from extcon */
+ if (extcon_get_state(qcom->edev, EXTCON_USB) ||
+ !extcon_get_state(host_edev, EXTCON_USB_HOST))
+ dwc3_qcom_vbus_notifier(&qcom->vbus_nb, true, qcom->edev);
+
+ return 0;
+}
+
+static void dwc3_qcom_suspend_hsphy(struct dwc3_qcom *qcom)
+{
+ struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
+ int ret;
+ u32 val;
+
+ /* Clear previous L2 events */
+ dwc3_qcom_setbits(qcom->qscratch_base, PWR_EVNT_IRQ_STAT_REG,
+ PWR_EVNT_LPM_IN_L2_MASK | PWR_EVNT_LPM_OUT_L2_MASK);
+
+ /* Allow controller to suspend HSPHY */
+ val = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
+ val |= DWC3_GUSB2PHYCFG_ENBLSLPM | DWC3_GUSB2PHYCFG_SUSPHY;
+ dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), val);
+
+ /* Wait for PHY to go into L2 */
+ ret = readl_poll_timeout(qcom->qscratch_base + PWR_EVNT_IRQ_STAT_REG,
+ val, (val & PWR_EVNT_LPM_IN_L2_MASK), 100,
+ HSPHY_L2_ENTER_TIMEOUT_US);
+ if (ret)
+ dev_err(qcom->dev, "could not transition HS PHY to L2\n");
+
+ /* Clear L2 event bit */
+ dwc3_qcom_setbits(qcom->qscratch_base, PWR_EVNT_IRQ_STAT_REG,
+ PWR_EVNT_LPM_IN_L2_MASK);
+}
+
+static int dwc3_qcom_suspend(struct dwc3_qcom *qcom)
+{
+ struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
+ int i;
+
+ if (qcom->is_suspended)
+ return 0;
+
+ if (!dwc)
+ return -EBUSY;
+
+ dwc3_qcom_suspend_hsphy(qcom);
+
+ if (dwc->usb2_generic_phy)
+ phy_pm_runtime_put_sync(dwc->usb2_generic_phy);
+ if (dwc->usb3_generic_phy)
+ phy_pm_runtime_put_sync(dwc->usb3_generic_phy);
+
+ for (i = qcom->num_clocks - 1; i >= 0; i--)
+ clk_disable_unprepare(qcom->clks[i]);
+
+ if (qcom->dp_hs_phy_irq) {
+ enable_irq(qcom->dp_hs_phy_irq);
+ enable_irq_wake(qcom->dp_hs_phy_irq);
+ }
+
+ if (qcom->dm_hs_phy_irq) {
+ enable_irq(qcom->dm_hs_phy_irq);
+ enable_irq_wake(qcom->dm_hs_phy_irq);
+ }
+
+ if (qcom->ss_phy_irq) {
+ enable_irq(qcom->ss_phy_irq);
+ enable_irq_wake(qcom->ss_phy_irq);
+ }
+
+ qcom->is_suspended = true;
+ dev_dbg(qcom->dev, "DWC3 in low power mode\n");
+
+ return 0;
+}
+
+static int dwc3_qcom_resume(struct dwc3_qcom *qcom)
+{
+ struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
+ int i, ret;
+ u32 val;
+
+ if (!qcom->is_suspended)
+ return 0;
+
+ if (qcom->dp_hs_phy_irq) {
+ disable_irq_wake(qcom->dp_hs_phy_irq);
+ disable_irq_nosync(qcom->dp_hs_phy_irq);
+ }
+
+ if (qcom->dm_hs_phy_irq) {
+ disable_irq_wake(qcom->dm_hs_phy_irq);
+ disable_irq_nosync(qcom->dm_hs_phy_irq);
+ }
+
+ if (qcom->ss_phy_irq) {
+ disable_irq_wake(qcom->ss_phy_irq);
+ disable_irq_nosync(qcom->ss_phy_irq);
+ }
+
+ for (i = 0; i < qcom->num_clocks; i++) {
+ ret = clk_prepare_enable(qcom->clks[i]);
+ if (ret < 0) {
+ while (--i >= 0)
+ clk_disable_unprepare(qcom->clks[i]);
+ return ret;
+ }
+ }
+
+ if (dwc->usb2_generic_phy)
+ phy_pm_runtime_get_sync(dwc->usb2_generic_phy);
+ if (dwc->usb3_generic_phy)
+ phy_pm_runtime_get_sync(dwc->usb3_generic_phy);
+
+ /* Disable HSPHY auto suspend */
+ val = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
+ val &= ~(DWC3_GUSB2PHYCFG_ENBLSLPM | DWC3_GUSB2PHYCFG_SUSPHY);
+ dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), val);
+
+ qcom->is_suspended = false;
+
+ dev_dbg(qcom->dev, "DWC3 exited from low power mode\n");
+
+ return 0;
+}
+
+static irqreturn_t qcom_dwc3_resume_irq(int irq, void *data)
+{
+ struct dwc3_qcom *qcom = data;
+ struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
+
+ /* If pm_suspended then let pm_resume take care of resuming h/w */
+ if (qcom->pm_suspended)
+ return IRQ_HANDLED;
+
+ dwc3_qcom_resume(qcom);
+
+ if (dwc->xhci) {
+ dev_dbg(qcom->dev, "%s: resuming xhci\n", __func__);
+ pm_runtime_resume(&dwc->xhci->dev);
+ }
+
+ return IRQ_HANDLED;
+}
+
+static void dwc3_qcom_select_utmi_clk(struct dwc3_qcom *qcom)
+{
+ /* Configure mux to use UTMI clock since PIPE clock not present */
+ dwc3_qcom_setbits(qcom->qscratch_base, QSCRATCH_GENERAL_CFG,
+ PIPE_UTMI_CLK_DIS);
+
+ usleep_range(100, 1000);
+
+ dwc3_qcom_setbits(qcom->qscratch_base, QSCRATCH_GENERAL_CFG,
+ PIPE_UTMI_CLK_SEL | PIPE3_PHYSTATUS_SW);
+
+ usleep_range(100, 1000);
+
+ dwc3_qcom_clrbits(qcom->qscratch_base, QSCRATCH_GENERAL_CFG,
+ PIPE_UTMI_CLK_DIS);
+}
+
+static int dwc3_qcom_clk_init(struct dwc3_qcom *qcom, int count)
+{
+ struct device *dev = qcom->dev;
+ struct device_node *np = dev->of_node;
+ int i;
+
+ qcom->num_clocks = count;
+
+ if (!count)
+ return 0;
+
+ qcom->clks = devm_kcalloc(dev, qcom->num_clocks,
+ sizeof(struct clk *), GFP_KERNEL);
+ if (!qcom->clks)
+ return -ENOMEM;
+
+ for (i = 0; i < qcom->num_clocks; i++) {
+ struct clk *clk;
+ int ret;
+
+ clk = of_clk_get(np, i);
+ if (IS_ERR(clk)) {
+ while (--i >= 0)
+ clk_put(qcom->clks[i]);
+ return PTR_ERR(clk);
+ }
+
+ ret = clk_prepare_enable(clk);
+ if (ret < 0) {
+ while (--i >= 0) {
+ clk_disable_unprepare(qcom->clks[i]);
+ clk_put(qcom->clks[i]);
+ }
+ clk_put(clk);
+
+ return ret;
+ }
+
+ qcom->clks[i] = clk;
+ }
+
+ return 0;
+}
+
+static int dwc3_qcom_probe(struct platform_device *pdev)
+{
+ struct device_node *np = pdev->dev.of_node, *dwc3_np;
+ struct dwc3_qcom *qcom;
+ struct resource *res;
+ int irq, ret, i;
+ bool ignore_pipe_clk;
+
+ qcom = devm_kzalloc(&pdev->dev, sizeof(*qcom), GFP_KERNEL);
+ if (!qcom)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, qcom);
+ qcom->dev = &pdev->dev;
+
+ qcom->resets = of_reset_control_array_get_optional_exclusive(np);
+ if (IS_ERR(qcom->resets)) {
+ ret = PTR_ERR(qcom->resets);
+ dev_err(&pdev->dev, "failed to get resets, err=%d\n", ret);
+ return ret;
+ }
+
+ ret = reset_control_deassert(qcom->resets);
+ if (ret)
+ goto reset_put;
+
+ ret = dwc3_qcom_clk_init(qcom, of_clk_get_parent_count(np));
+ if (ret) {
+ dev_err(qcom->dev, "failed to get clocks\n");
+ goto reset_assert;
+ }
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "qscratch");
+ qcom->qscratch_base = devm_ioremap_resource(qcom->dev, res);
+ if (IS_ERR(qcom->qscratch_base)) {
+ dev_err(qcom->dev, "failed to map qscratch - %d\n", ret);
+ ret = PTR_ERR(qcom->qscratch_base);
+ goto clk_disable;
+ }
+
+ irq = platform_get_irq_byname(pdev, "hs_phy_irq");
+ if (irq > 0) {
+ ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
+ qcom_dwc3_resume_irq,
+ IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
+ "qcom_dwc3 HS", qcom);
+ if (ret) {
+ dev_err(qcom->dev, "hs_phy_irq failed: %d\n", ret);
+ goto clk_disable;
+ }
+ }
+
+ irq = platform_get_irq_byname(pdev, "dp_hs_phy_irq");
+ if (irq > 0) {
+ irq_set_status_flags(irq, IRQ_NOAUTOEN);
+ ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
+ qcom_dwc3_resume_irq,
+ IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
+ "qcom_dwc3 DP_HS", qcom);
+ if (ret) {
+ dev_err(qcom->dev, "dp_hs_phy_irq failed: %d\n", ret);
+ goto clk_disable;
+ }
+ qcom->dp_hs_phy_irq = irq;
+ }
+
+ irq = platform_get_irq_byname(pdev, "dm_hs_phy_irq");
+ if (irq > 0) {
+ irq_set_status_flags(irq, IRQ_NOAUTOEN);
+ ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
+ qcom_dwc3_resume_irq,
+ IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
+ "qcom_dwc3 DM_HS", qcom);
+ if (ret) {
+ dev_err(qcom->dev, "dm_hs_phy_irq failed: %d\n", ret);
+ goto clk_disable;
+ }
+ qcom->dm_hs_phy_irq = irq;
+ }
+
+ irq = platform_get_irq_byname(pdev, "ss_phy_irq");
+ if (irq > 0) {
+ irq_set_status_flags(irq, IRQ_NOAUTOEN);
+ ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
+ qcom_dwc3_resume_irq,
+ IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
+ "qcom_dwc3 SS", qcom);
+ if (ret) {
+ dev_err(qcom->dev, "ss_phy_irq failed: %d\n", ret);
+ goto clk_disable;
+ }
+ qcom->ss_phy_irq = irq;
+ }
+
+ dwc3_np = of_get_child_by_name(np, "dwc3");
+ if (!dwc3_np) {
+ dev_err(qcom->dev, "failed to find dwc3 core child\n");
+ ret = -ENODEV;
+ goto clk_disable;
+ }
+
+ /*
+ * Disable pipe_clk requirement if specified. Used when dwc3
+ * operates without SSPHY and only HS/FS/LS modes are supported.
+ */
+ ignore_pipe_clk = device_property_read_bool(qcom->dev,
+ "qcom,select-utmi-as-pipe-clk");
+ if (ignore_pipe_clk)
+ dwc3_qcom_select_utmi_clk(qcom);
+
+ ret = of_platform_populate(np, NULL, NULL, qcom->dev);
+ if (ret) {
+ dev_err(qcom->dev, "failed to register dwc3 core - %d\n", ret);
+ goto clk_disable;
+ }
+
+ qcom->dwc3 = of_find_device_by_node(dwc3_np);
+ if (!qcom->dwc3) {
+ dev_err(&pdev->dev, "failed to get dwc3 platform device\n");
+ goto depopulate;
+ }
+
+ qcom->mode = usb_get_dr_mode(&qcom->dwc3->dev);
+
+ /* register extcon to override vbus later on mode switch */
+ if (qcom->mode == USB_DR_MODE_OTG) {
+ ret = dwc3_qcom_register_extcon(qcom);
+ if (ret)
+ goto depopulate;
+ } else if (qcom->mode == USB_DR_MODE_PERIPHERAL) {
+ /* enable vbus override for device mode */
+ dwc3_qcom_vbus_overrride_enable(qcom, true);
+ }
+
+ device_init_wakeup(&pdev->dev, 1);
+ qcom->is_suspended = false;
+ pm_runtime_set_active(qcom->dev);
+ pm_runtime_enable(qcom->dev);
+
+ return 0;
+
+depopulate:
+ of_platform_depopulate(&pdev->dev);
+clk_disable:
+ for (i = qcom->num_clocks - 1; i >= 0; i--) {
+ clk_disable_unprepare(qcom->clks[i]);
+ clk_put(qcom->clks[i]);
+ }
+reset_assert:
+ reset_control_assert(qcom->resets);
+reset_put:
+ reset_control_put(qcom->resets);
+
+ return ret;
+}
+
+static int dwc3_qcom_remove(struct platform_device *pdev)
+{
+ struct dwc3_qcom *qcom = platform_get_drvdata(pdev);
+ struct device *dev = qcom->dev;
+ int i;
+
+ of_platform_depopulate(&pdev->dev);
+
+ for (i = qcom->num_clocks - 1; i >= 0; i--) {
+ clk_disable_unprepare(qcom->clks[i]);
+ clk_put(qcom->clks[i]);
+ }
+ qcom->num_clocks = 0;
+
+ reset_control_assert(qcom->resets);
+ reset_control_put(qcom->resets);
+
+ pm_runtime_disable(dev);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int dwc3_qcom_pm_suspend(struct device *dev)
+{
+ struct dwc3_qcom *qcom = dev_get_drvdata(dev);
+ int ret = 0;
+
+ dev_err(dev, "dwc3-qcom PM suspend\n");
+
+ ret = dwc3_qcom_suspend(qcom);
+ if (!ret)
+ qcom->pm_suspended = true;
+
+ return ret;
+}
+
+static int dwc3_qcom_pm_resume(struct device *dev)
+{
+ struct dwc3_qcom *qcom = dev_get_drvdata(dev);
+ int ret;
+
+ dev_err(dev, "dwc3-qcom PM resume\n");
+
+ ret = dwc3_qcom_resume(qcom);
+ if (!ret)
+ qcom->pm_suspended = false;
+
+ return ret;
+}
+#endif
+
+#ifdef CONFIG_PM
+static int dwc3_qcom_runtime_suspend(struct device *dev)
+{
+ struct dwc3_qcom *qcom = dev_get_drvdata(dev);
+
+ dev_err(dev, "DWC3-qcom runtime suspend\n");
+
+ return dwc3_qcom_suspend(qcom);
+}
+
+static int dwc3_qcom_runtime_resume(struct device *dev)
+{
+ struct dwc3_qcom *qcom = dev_get_drvdata(dev);
+
+ dev_err(dev, "DWC3-qcom runtime resume\n");
+
+ return dwc3_qcom_resume(qcom);
+}
+#endif
+
+static const struct dev_pm_ops dwc3_qcom_dev_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(dwc3_qcom_pm_suspend, dwc3_qcom_pm_resume)
+ SET_RUNTIME_PM_OPS(dwc3_qcom_runtime_suspend, dwc3_qcom_runtime_resume,
+ NULL)
+};
+
+
+static const struct of_device_id dwc3_qcom_of_match[] = {
+ { .compatible = "qcom,dwc3" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, dwc3_qcom_of_match);
+
+static struct platform_driver dwc3_qcom_driver = {
+ .probe = dwc3_qcom_probe,
+ .remove = dwc3_qcom_remove,
+ .driver = {
+ .name = "dwc3-qcom",
+ .pm = &dwc3_qcom_dev_pm_ops,
+ .of_match_table = dwc3_qcom_of_match,
+ },
+};
+
+module_platform_driver(dwc3_qcom_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("DesignWare DWC3 QCOM Glue Driver");
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


2018-03-13 11:09:58

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] usb: dwc3: Add Qualcomm DWC3 glue driver


Hi,

+Andy

Manu Gautam <[email protected]> writes:
> DWC3 controller on Qualcomm SOCs has a Qscratch wrapper.
> Some of its uses are described below resulting in need to
> have a separate glue driver instead of using dwc3-of-simple:
> - It exposes register interface to override vbus-override
> and lane0-pwr-present signals going to hardware. These
> must be updated in peripheral mode for DWC3 if vbus lines
> are not connected to hardware block. Otherwise RX termination
> in SS mode or DP pull-up is not applied by device controller.

right, core needs to know that VBUS is above 4.4V. Why wasn't this a
problem when the original glue layer was first published?

> - pwr_events_irq_stat support to ensure USB2 PHY is in L2 state
> before glue driver can turn-off clocks and suspend PHY.

Core manages PHY suspend automatically. Isn't that working for you? Why?

> - Support for wakeup interrupts lines that are asserted whenever
> there is any wakeup event on USB3 or USB2 bus.

ok

> - Support to replace pip3 clock going to DWC3 with utmi clock
> for hardware configuration where SSPHY is not used with DWC3.

Is that SW configurable? Really? In any case seems like this and SESSVLD
valid should be handled using Hans' and Heikki's mux support.

> Other than above hardware features in Qscratch wrapper there
> are some limitations on QCOM SOCs that require special handling
> of power management e.g. suspending PHY using GUSB2PHYCFG
> register and ensuring PHY enters L2 before turning off clocks etc.
>
> Signed-off-by: Manu Gautam <[email protected]>
> ---
> drivers/usb/dwc3/Kconfig | 11 +
> drivers/usb/dwc3/Makefile | 1 +
> drivers/usb/dwc3/dwc3-of-simple.c | 1 -
> drivers/usb/dwc3/dwc3-qcom.c | 635 ++++++++++++++++++++++++++++++++++++++
> 4 files changed, 647 insertions(+), 1 deletion(-)
> create mode 100644 drivers/usb/dwc3/dwc3-qcom.c
>
> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> index ab8c0e0..f5bb4f1 100644
> --- a/drivers/usb/dwc3/Kconfig
> +++ b/drivers/usb/dwc3/Kconfig
> @@ -106,4 +106,15 @@ config USB_DWC3_ST
> inside (i.e. STiH407).
> Say 'Y' or 'M' if you have one such device.
>
> +config USB_DWC3_QCOM
> + tristate "Qualcomm Platform"
> + depends on ARCH_QCOM || COMPILE_TEST
> + depends on OF
> + default USB_DWC3
> + help
> + Some Qualcomm SoCs use DesignWare Core IP for USB2/3
> + functionality.
> +
> + Say 'Y' or 'M' if you have one such device.
> +
> endif
> diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
> index 7ac7250..c3ce697 100644
> --- a/drivers/usb/dwc3/Makefile
> +++ b/drivers/usb/dwc3/Makefile
> @@ -48,3 +48,4 @@ obj-$(CONFIG_USB_DWC3_PCI) += dwc3-pci.o
> obj-$(CONFIG_USB_DWC3_KEYSTONE) += dwc3-keystone.o
> obj-$(CONFIG_USB_DWC3_OF_SIMPLE) += dwc3-of-simple.o
> obj-$(CONFIG_USB_DWC3_ST) += dwc3-st.o
> +obj-$(CONFIG_USB_DWC3_QCOM) += dwc3-qcom.o
> diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c
> index cb2ee96..0fd0e8e 100644
> --- a/drivers/usb/dwc3/dwc3-of-simple.c
> +++ b/drivers/usb/dwc3/dwc3-of-simple.c
> @@ -208,7 +208,6 @@ static int dwc3_of_simple_runtime_resume(struct device *dev)
> };
>
> static const struct of_device_id of_dwc3_simple_match[] = {
> - { .compatible = "qcom,dwc3" },
> { .compatible = "rockchip,rk3399-dwc3" },
> { .compatible = "xlnx,zynqmp-dwc3" },
> { .compatible = "cavium,octeon-7130-usb-uctl" },
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> new file mode 100644
> index 0000000..917199e
> --- /dev/null
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -0,0 +1,635 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2018, The Linux Foundation. All rights reserved.
> + *
> + * Inspired by dwc3-of-simple.c
> + */
> +
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/clk.h>
> +#include <linux/irq.h>
> +#include <linux/clk-provider.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/extcon.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/phy/phy.h>
> +#include <linux/usb/of.h>
> +#include <linux/reset.h>
> +#include <linux/iopoll.h>
> +
> +#include "core.h"
> +#include "io.h"

you must be kidding, right? There's no way I'm gonna let a glue poke at
core registers.

> +static void dwc3_qcom_suspend_hsphy(struct dwc3_qcom *qcom)
> +{
> + struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
> + int ret;
> + u32 val;
> +
> + /* Clear previous L2 events */
> + dwc3_qcom_setbits(qcom->qscratch_base, PWR_EVNT_IRQ_STAT_REG,
> + PWR_EVNT_LPM_IN_L2_MASK | PWR_EVNT_LPM_OUT_L2_MASK);
> +
> + /* Allow controller to suspend HSPHY */
> + val = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
> + val |= DWC3_GUSB2PHYCFG_ENBLSLPM | DWC3_GUSB2PHYCFG_SUSPHY;
> + dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), val);

core should handle PHY bits. In any case, why don't you let core handle
PHY suspend? It handles it automatically for us, there should be no need
for SW intervention.

> +static int dwc3_qcom_suspend(struct dwc3_qcom *qcom)
> +{
> + struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);

nope! Glue shouldn't touch dwc3 at all.

> + int i;
> +
> + if (qcom->is_suspended)
> + return 0;
> +
> + if (!dwc)
> + return -EBUSY;

-ENODEV?

> + dwc3_qcom_suspend_hsphy(qcom);
> +
> + if (dwc->usb2_generic_phy)
> + phy_pm_runtime_put_sync(dwc->usb2_generic_phy);
> + if (dwc->usb3_generic_phy)
> + phy_pm_runtime_put_sync(dwc->usb3_generic_phy);

core.c should do this.

> + /* Disable HSPHY auto suspend */
> + val = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
> + val &= ~(DWC3_GUSB2PHYCFG_ENBLSLPM | DWC3_GUSB2PHYCFG_SUSPHY);
> + dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), val);
> +
> + qcom->is_suspended = false;
> +
> + dev_dbg(qcom->dev, "DWC3 exited from low power mode\n");

no dev_dbg() or dev_info() in this driver.

> +static irqreturn_t qcom_dwc3_resume_irq(int irq, void *data)
> +{
> + struct dwc3_qcom *qcom = data;
> + struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
> +
> + /* If pm_suspended then let pm_resume take care of resuming h/w */
> + if (qcom->pm_suspended)
> + return IRQ_HANDLED;
> +
> + dwc3_qcom_resume(qcom);

instead, why don't you pm_runtime_resume() here, and let runtime_resume()
handle dwc3_qcom_resume() ?

> +static int dwc3_qcom_clk_init(struct dwc3_qcom *qcom, int count)
> +{
> + struct device *dev = qcom->dev;
> + struct device_node *np = dev->of_node;
> + int i;
> +
> + qcom->num_clocks = count;
> +
> + if (!count)
> + return 0;
> +
> + qcom->clks = devm_kcalloc(dev, qcom->num_clocks,
> + sizeof(struct clk *), GFP_KERNEL);
> + if (!qcom->clks)
> + return -ENOMEM;
> +
> + for (i = 0; i < qcom->num_clocks; i++) {
> + struct clk *clk;
> + int ret;
> +
> + clk = of_clk_get(np, i);
> + if (IS_ERR(clk)) {
> + while (--i >= 0)
> + clk_put(qcom->clks[i]);
> + return PTR_ERR(clk);
> + }
> +
> + ret = clk_prepare_enable(clk);
> + if (ret < 0) {
> + while (--i >= 0) {
> + clk_disable_unprepare(qcom->clks[i]);
> + clk_put(qcom->clks[i]);
> + }
> + clk_put(clk);
> +
> + return ret;
> + }
> +
> + qcom->clks[i] = clk;
> + }
> +
> + return 0;
> +}
> +
> +static int dwc3_qcom_probe(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node, *dwc3_np;
> + struct dwc3_qcom *qcom;
> + struct resource *res;
> + int irq, ret, i;
> + bool ignore_pipe_clk;
> +
> + qcom = devm_kzalloc(&pdev->dev, sizeof(*qcom), GFP_KERNEL);
> + if (!qcom)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, qcom);
> + qcom->dev = &pdev->dev;
> +
> + qcom->resets = of_reset_control_array_get_optional_exclusive(np);
> + if (IS_ERR(qcom->resets)) {
> + ret = PTR_ERR(qcom->resets);
> + dev_err(&pdev->dev, "failed to get resets, err=%d\n", ret);
> + return ret;
> + }
> +
> + ret = reset_control_deassert(qcom->resets);
> + if (ret)
> + goto reset_put;
> +
> + ret = dwc3_qcom_clk_init(qcom, of_clk_get_parent_count(np));
> + if (ret) {
> + dev_err(qcom->dev, "failed to get clocks\n");
> + goto reset_assert;
> + }
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "qscratch");
> + qcom->qscratch_base = devm_ioremap_resource(qcom->dev, res);
> + if (IS_ERR(qcom->qscratch_base)) {
> + dev_err(qcom->dev, "failed to map qscratch - %d\n", ret);
> + ret = PTR_ERR(qcom->qscratch_base);
> + goto clk_disable;
> + }
> +
> + irq = platform_get_irq_byname(pdev, "hs_phy_irq");
> + if (irq > 0) {
> + ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
> + qcom_dwc3_resume_irq,
> + IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
> + "qcom_dwc3 HS", qcom);
> + if (ret) {
> + dev_err(qcom->dev, "hs_phy_irq failed: %d\n", ret);
> + goto clk_disable;
> + }
> + }
> +
> + irq = platform_get_irq_byname(pdev, "dp_hs_phy_irq");
> + if (irq > 0) {
> + irq_set_status_flags(irq, IRQ_NOAUTOEN);

why do you need to set this flag?

> + ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
> + qcom_dwc3_resume_irq,
> + IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
> + "qcom_dwc3 DP_HS", qcom);

this is the same as devm_request_irq()

> + if (ret) {
> + dev_err(qcom->dev, "dp_hs_phy_irq failed: %d\n", ret);
> + goto clk_disable;
> + }
> + qcom->dp_hs_phy_irq = irq;
> + }
> +
> + irq = platform_get_irq_byname(pdev, "dm_hs_phy_irq");
> + if (irq > 0) {
> + irq_set_status_flags(irq, IRQ_NOAUTOEN);
> + ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
> + qcom_dwc3_resume_irq,
> + IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
> + "qcom_dwc3 DM_HS", qcom);
> + if (ret) {
> + dev_err(qcom->dev, "dm_hs_phy_irq failed: %d\n", ret);
> + goto clk_disable;
> + }
> + qcom->dm_hs_phy_irq = irq;
> + }
> +
> + irq = platform_get_irq_byname(pdev, "ss_phy_irq");
> + if (irq > 0) {
> + irq_set_status_flags(irq, IRQ_NOAUTOEN);
> + ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
> + qcom_dwc3_resume_irq,
> + IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
> + "qcom_dwc3 SS", qcom);
> + if (ret) {
> + dev_err(qcom->dev, "ss_phy_irq failed: %d\n", ret);
> + goto clk_disable;
> + }
> + qcom->ss_phy_irq = irq;
> + }
> +
> + dwc3_np = of_get_child_by_name(np, "dwc3");
> + if (!dwc3_np) {
> + dev_err(qcom->dev, "failed to find dwc3 core child\n");
> + ret = -ENODEV;
> + goto clk_disable;
> + }
> +
> + /*
> + * Disable pipe_clk requirement if specified. Used when dwc3
> + * operates without SSPHY and only HS/FS/LS modes are supported.
> + */
> + ignore_pipe_clk = device_property_read_bool(qcom->dev,
> + "qcom,select-utmi-as-pipe-clk");
> + if (ignore_pipe_clk)
> + dwc3_qcom_select_utmi_clk(qcom);
> +
> + ret = of_platform_populate(np, NULL, NULL, qcom->dev);
> + if (ret) {
> + dev_err(qcom->dev, "failed to register dwc3 core - %d\n", ret);
> + goto clk_disable;
> + }
> +
> + qcom->dwc3 = of_find_device_by_node(dwc3_np);
> + if (!qcom->dwc3) {
> + dev_err(&pdev->dev, "failed to get dwc3 platform device\n");
> + goto depopulate;
> + }
> +
> + qcom->mode = usb_get_dr_mode(&qcom->dwc3->dev);
> +
> + /* register extcon to override vbus later on mode switch */
> + if (qcom->mode == USB_DR_MODE_OTG) {
> + ret = dwc3_qcom_register_extcon(qcom);
> + if (ret)
> + goto depopulate;
> + } else if (qcom->mode == USB_DR_MODE_PERIPHERAL) {
> + /* enable vbus override for device mode */
> + dwc3_qcom_vbus_overrride_enable(qcom, true);
> + }
> +
> + device_init_wakeup(&pdev->dev, 1);
> + qcom->is_suspended = false;
> + pm_runtime_set_active(qcom->dev);
> + pm_runtime_enable(qcom->dev);
> +
> + return 0;
> +
> +depopulate:
> + of_platform_depopulate(&pdev->dev);
> +clk_disable:
> + for (i = qcom->num_clocks - 1; i >= 0; i--) {
> + clk_disable_unprepare(qcom->clks[i]);
> + clk_put(qcom->clks[i]);
> + }
> +reset_assert:
> + reset_control_assert(qcom->resets);
> +reset_put:
> + reset_control_put(qcom->resets);
> +
> + return ret;
> +}
> +
> +static int dwc3_qcom_remove(struct platform_device *pdev)
> +{
> + struct dwc3_qcom *qcom = platform_get_drvdata(pdev);
> + struct device *dev = qcom->dev;
> + int i;
> +
> + of_platform_depopulate(&pdev->dev);
> +
> + for (i = qcom->num_clocks - 1; i >= 0; i--) {
> + clk_disable_unprepare(qcom->clks[i]);
> + clk_put(qcom->clks[i]);
> + }
> + qcom->num_clocks = 0;
> +
> + reset_control_assert(qcom->resets);
> + reset_control_put(qcom->resets);
> +
> + pm_runtime_disable(dev);
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int dwc3_qcom_pm_suspend(struct device *dev)
> +{
> + struct dwc3_qcom *qcom = dev_get_drvdata(dev);
> + int ret = 0;
> +
> + dev_err(dev, "dwc3-qcom PM suspend\n");

why an error message here? There's no error. Remove this, it's unnecessary.

> + ret = dwc3_qcom_suspend(qcom);
> + if (!ret)
> + qcom->pm_suspended = true;
> +
> + return ret;
> +}
> +
> +static int dwc3_qcom_pm_resume(struct device *dev)
> +{
> + struct dwc3_qcom *qcom = dev_get_drvdata(dev);
> + int ret;
> +
> + dev_err(dev, "dwc3-qcom PM resume\n");

ditto

> + ret = dwc3_qcom_resume(qcom);
> + if (!ret)
> + qcom->pm_suspended = false;
> +
> + return ret;
> +}
> +#endif
> +
> +#ifdef CONFIG_PM
> +static int dwc3_qcom_runtime_suspend(struct device *dev)
> +{
> + struct dwc3_qcom *qcom = dev_get_drvdata(dev);
> +
> + dev_err(dev, "DWC3-qcom runtime suspend\n");

ditto

> + return dwc3_qcom_suspend(qcom);
> +}
> +
> +static int dwc3_qcom_runtime_resume(struct device *dev)
> +{
> + struct dwc3_qcom *qcom = dev_get_drvdata(dev);
> +
> + dev_err(dev, "DWC3-qcom runtime resume\n");

ditto

> + return dwc3_qcom_resume(qcom);
> +}
> +#endif
> +
> +static const struct dev_pm_ops dwc3_qcom_dev_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(dwc3_qcom_pm_suspend, dwc3_qcom_pm_resume)
> + SET_RUNTIME_PM_OPS(dwc3_qcom_runtime_suspend, dwc3_qcom_runtime_resume,
> + NULL)

why don't you have runtime_idle?

--
balbi


Attachments:
signature.asc (847.00 B)

2018-03-14 04:23:24

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] usb: dwc3: Add Qualcomm DWC3 glue driver

Hi Manu,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v4.16-rc4]
[also build test ERROR on next-20180313]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Manu-Gautam/dt-bindings-usb-Update-documentation-for-Qualcomm-DWC3-driver/20180314-095557
config: sparc64-allmodconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sparc64

All errors (new ones prefixed by >>):

drivers/usb/dwc3/dwc3-qcom.c: In function 'dwc3_qcom_probe':
>> drivers/usb/dwc3/dwc3-qcom.c:407:33: error: implicit declaration of function 'of_clk_get_parent_count'; did you mean 'clk_get_parent'? [-Werror=implicit-function-declaration]
ret = dwc3_qcom_clk_init(qcom, of_clk_get_parent_count(np));
^~~~~~~~~~~~~~~~~~~~~~~
clk_get_parent
cc1: some warnings being treated as errors

vim +407 drivers/usb/dwc3/dwc3-qcom.c

380
381 static int dwc3_qcom_probe(struct platform_device *pdev)
382 {
383 struct device_node *np = pdev->dev.of_node, *dwc3_np;
384 struct dwc3_qcom *qcom;
385 struct resource *res;
386 int irq, ret, i;
387 bool ignore_pipe_clk;
388
389 qcom = devm_kzalloc(&pdev->dev, sizeof(*qcom), GFP_KERNEL);
390 if (!qcom)
391 return -ENOMEM;
392
393 platform_set_drvdata(pdev, qcom);
394 qcom->dev = &pdev->dev;
395
396 qcom->resets = of_reset_control_array_get_optional_exclusive(np);
397 if (IS_ERR(qcom->resets)) {
398 ret = PTR_ERR(qcom->resets);
399 dev_err(&pdev->dev, "failed to get resets, err=%d\n", ret);
400 return ret;
401 }
402
403 ret = reset_control_deassert(qcom->resets);
404 if (ret)
405 goto reset_put;
406
> 407 ret = dwc3_qcom_clk_init(qcom, of_clk_get_parent_count(np));
408 if (ret) {
409 dev_err(qcom->dev, "failed to get clocks\n");
410 goto reset_assert;
411 }
412
413 res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "qscratch");
414 qcom->qscratch_base = devm_ioremap_resource(qcom->dev, res);
415 if (IS_ERR(qcom->qscratch_base)) {
416 dev_err(qcom->dev, "failed to map qscratch - %d\n", ret);
417 ret = PTR_ERR(qcom->qscratch_base);
418 goto clk_disable;
419 }
420
421 irq = platform_get_irq_byname(pdev, "hs_phy_irq");
422 if (irq > 0) {
423 ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
424 qcom_dwc3_resume_irq,
425 IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
426 "qcom_dwc3 HS", qcom);
427 if (ret) {
428 dev_err(qcom->dev, "hs_phy_irq failed: %d\n", ret);
429 goto clk_disable;
430 }
431 }
432
433 irq = platform_get_irq_byname(pdev, "dp_hs_phy_irq");
434 if (irq > 0) {
435 irq_set_status_flags(irq, IRQ_NOAUTOEN);
436 ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
437 qcom_dwc3_resume_irq,
438 IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
439 "qcom_dwc3 DP_HS", qcom);
440 if (ret) {
441 dev_err(qcom->dev, "dp_hs_phy_irq failed: %d\n", ret);
442 goto clk_disable;
443 }
444 qcom->dp_hs_phy_irq = irq;
445 }
446
447 irq = platform_get_irq_byname(pdev, "dm_hs_phy_irq");
448 if (irq > 0) {
449 irq_set_status_flags(irq, IRQ_NOAUTOEN);
450 ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
451 qcom_dwc3_resume_irq,
452 IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
453 "qcom_dwc3 DM_HS", qcom);
454 if (ret) {
455 dev_err(qcom->dev, "dm_hs_phy_irq failed: %d\n", ret);
456 goto clk_disable;
457 }
458 qcom->dm_hs_phy_irq = irq;
459 }
460
461 irq = platform_get_irq_byname(pdev, "ss_phy_irq");
462 if (irq > 0) {
463 irq_set_status_flags(irq, IRQ_NOAUTOEN);
464 ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
465 qcom_dwc3_resume_irq,
466 IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
467 "qcom_dwc3 SS", qcom);
468 if (ret) {
469 dev_err(qcom->dev, "ss_phy_irq failed: %d\n", ret);
470 goto clk_disable;
471 }
472 qcom->ss_phy_irq = irq;
473 }
474
475 dwc3_np = of_get_child_by_name(np, "dwc3");
476 if (!dwc3_np) {
477 dev_err(qcom->dev, "failed to find dwc3 core child\n");
478 ret = -ENODEV;
479 goto clk_disable;
480 }
481
482 /*
483 * Disable pipe_clk requirement if specified. Used when dwc3
484 * operates without SSPHY and only HS/FS/LS modes are supported.
485 */
486 ignore_pipe_clk = device_property_read_bool(qcom->dev,
487 "qcom,select-utmi-as-pipe-clk");
488 if (ignore_pipe_clk)
489 dwc3_qcom_select_utmi_clk(qcom);
490
491 ret = of_platform_populate(np, NULL, NULL, qcom->dev);
492 if (ret) {
493 dev_err(qcom->dev, "failed to register dwc3 core - %d\n", ret);
494 goto clk_disable;
495 }
496
497 qcom->dwc3 = of_find_device_by_node(dwc3_np);
498 if (!qcom->dwc3) {
499 dev_err(&pdev->dev, "failed to get dwc3 platform device\n");
500 goto depopulate;
501 }
502
503 qcom->mode = usb_get_dr_mode(&qcom->dwc3->dev);
504
505 /* register extcon to override vbus later on mode switch */
506 if (qcom->mode == USB_DR_MODE_OTG) {
507 ret = dwc3_qcom_register_extcon(qcom);
508 if (ret)
509 goto depopulate;
510 } else if (qcom->mode == USB_DR_MODE_PERIPHERAL) {
511 /* enable vbus override for device mode */
512 dwc3_qcom_vbus_overrride_enable(qcom, true);
513 }
514
515 device_init_wakeup(&pdev->dev, 1);
516 qcom->is_suspended = false;
517 pm_runtime_set_active(qcom->dev);
518 pm_runtime_enable(qcom->dev);
519
520 return 0;
521
522 depopulate:
523 of_platform_depopulate(&pdev->dev);
524 clk_disable:
525 for (i = qcom->num_clocks - 1; i >= 0; i--) {
526 clk_disable_unprepare(qcom->clks[i]);
527 clk_put(qcom->clks[i]);
528 }
529 reset_assert:
530 reset_control_assert(qcom->resets);
531 reset_put:
532 reset_control_put(qcom->resets);
533
534 return ret;
535 }
536

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (6.70 kB)
.config.gz (52.12 kB)
Download all attachments

2018-03-14 04:41:18

by Manu Gautam

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] usb: dwc3: Add Qualcomm DWC3 glue driver

Hi,


On 3/13/2018 4:38 PM, Felipe Balbi wrote:
> Hi,
>
> +Andy
>
> Manu Gautam <[email protected]> writes:
>> DWC3 controller on Qualcomm SOCs has a Qscratch wrapper.
>> Some of its uses are described below resulting in need to
>> have a separate glue driver instead of using dwc3-of-simple:
>> - It exposes register interface to override vbus-override
>> and lane0-pwr-present signals going to hardware. These
>> must be updated in peripheral mode for DWC3 if vbus lines
>> are not connected to hardware block. Otherwise RX termination
>> in SS mode or DP pull-up is not applied by device controller.
> right, core needs to know that VBUS is above 4.4V. Why wasn't this a
> problem when the original glue layer was first published?

Thanks for reviewing.
Original glue layer supported only host mode, hence this wasn't needed.

>
>> - pwr_events_irq_stat support to ensure USB2 PHY is in L2 state
>> before glue driver can turn-off clocks and suspend PHY.
> Core manages PHY suspend automatically. Isn't that working for you? Why?

Yes, it is not supported with QUSB2 PHY (usb2-phy on Qualcomm SOCs).
Issue is that If core suspends USB2 PHY (say in host mode if some SS device connected),
USB2 PHY stops responding to any attach event as it can't exit suspend state by itself.
But in case of driver suspend, along with PHY suspend wakeup events are enabled.
Glue driver can register for wakeup interrupts to exit suspend.

>
>> - Support for wakeup interrupts lines that are asserted whenever
>> there is any wakeup event on USB3 or USB2 bus.
> ok
>
>> - Support to replace pip3 clock going to DWC3 with utmi clock
>> for hardware configuration where SSPHY is not used with DWC3.
> Is that SW configurable? Really? In any case seems like this and SESSVLD
> valid should be handled using Hans' and Heikki's mux support.

Yes, with this we can use dwc3 without using SSPHY. Please point me to
those patches. There are only bunch of register writes in glue wrapper to
achieve the same.

>
>> Other than above hardware features in Qscratch wrapper there
>> are some limitations on QCOM SOCs that require special handling
>> of power management e.g. suspending PHY using GUSB2PHYCFG
>> register and ensuring PHY enters L2 before turning off clocks etc.
>>
>> Signed-off-by: Manu Gautam <[email protected]>
>> ---
>> drivers/usb/dwc3/Kconfig | 11 +
>> drivers/usb/dwc3/Makefile | 1 +
>> drivers/usb/dwc3/dwc3-of-simple.c | 1 -
>> drivers/usb/dwc3/dwc3-qcom.c | 635 ++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 647 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/usb/dwc3/dwc3-qcom.c
>>
>> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
>> index ab8c0e0..f5bb4f1 100644
>> --- a/drivers/usb/dwc3/Kconfig
>> +++ b/drivers/usb/dwc3/Kconfig
>> @@ -106,4 +106,15 @@ config USB_DWC3_ST
>> inside (i.e. STiH407).
>> Say 'Y' or 'M' if you have one such device.
>>
>> +config USB_DWC3_QCOM
>> + tristate "Qualcomm Platform"
>> + depends on ARCH_QCOM || COMPILE_TEST
>> + depends on OF
>> + default USB_DWC3
>> + help
>> + Some Qualcomm SoCs use DesignWare Core IP for USB2/3
>> + functionality.
>> +
>> + Say 'Y' or 'M' if you have one such device.
>> +
>> endif
>> diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
>> index 7ac7250..c3ce697 100644
>> --- a/drivers/usb/dwc3/Makefile
>> +++ b/drivers/usb/dwc3/Makefile
>> @@ -48,3 +48,4 @@ obj-$(CONFIG_USB_DWC3_PCI) += dwc3-pci.o
>> obj-$(CONFIG_USB_DWC3_KEYSTONE) += dwc3-keystone.o
>> obj-$(CONFIG_USB_DWC3_OF_SIMPLE) += dwc3-of-simple.o
>> obj-$(CONFIG_USB_DWC3_ST) += dwc3-st.o
>> +obj-$(CONFIG_USB_DWC3_QCOM) += dwc3-qcom.o
>> diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c
>> index cb2ee96..0fd0e8e 100644
>> --- a/drivers/usb/dwc3/dwc3-of-simple.c
>> +++ b/drivers/usb/dwc3/dwc3-of-simple.c
>> @@ -208,7 +208,6 @@ static int dwc3_of_simple_runtime_resume(struct device *dev)
>> };
>>
>> static const struct of_device_id of_dwc3_simple_match[] = {
>> - { .compatible = "qcom,dwc3" },
>> { .compatible = "rockchip,rk3399-dwc3" },
>> { .compatible = "xlnx,zynqmp-dwc3" },
>> { .compatible = "cavium,octeon-7130-usb-uctl" },
>> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
>> new file mode 100644
>> index 0000000..917199e
>> --- /dev/null
>> +++ b/drivers/usb/dwc3/dwc3-qcom.c
>> @@ -0,0 +1,635 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Copyright (c) 2018, The Linux Foundation. All rights reserved.
>> + *
>> + * Inspired by dwc3-of-simple.c
>> + */
>> +
>> +#include <linux/io.h>
>> +#include <linux/of.h>
>> +#include <linux/clk.h>
>> +#include <linux/irq.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/module.h>
>> +#include <linux/kernel.h>
>> +#include <linux/extcon.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/usb/of.h>
>> +#include <linux/reset.h>
>> +#include <linux/iopoll.h>
>> +
>> +#include "core.h"
>> +#include "io.h"
> you must be kidding, right? There's no way I'm gonna let a glue poke at
> core registers.
>
>> +static void dwc3_qcom_suspend_hsphy(struct dwc3_qcom *qcom)
>> +{
>> + struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
>> + int ret;
>> + u32 val;
>> +
>> + /* Clear previous L2 events */
>> + dwc3_qcom_setbits(qcom->qscratch_base, PWR_EVNT_IRQ_STAT_REG,
>> + PWR_EVNT_LPM_IN_L2_MASK | PWR_EVNT_LPM_OUT_L2_MASK);
>> +
>> + /* Allow controller to suspend HSPHY */
>> + val = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
>> + val |= DWC3_GUSB2PHYCFG_ENBLSLPM | DWC3_GUSB2PHYCFG_SUSPHY;
>> + dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), val);
> core should handle PHY bits. In any case, why don't you let core handle
> PHY suspend? It handles it automatically for us, there should be no need
> for SW intervention.
I agree with you. That would have been straight forward, but due to PHY issue
I mentioned earlier I had to do this here. I added more info on this in response to
your comment below.

>
>> +static int dwc3_qcom_suspend(struct dwc3_qcom *qcom)
>> +{
>> + struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
> nope! Glue shouldn't touch dwc3 at all.
Other than PHY handles, need this to fail runtime suspend if dwc3 hasn't
probed yet.

>
>> + int i;
>> +
>> + if (qcom->is_suspended)
>> + return 0;
>> +
>> + if (!dwc)
>> + return -EBUSY;
> -ENODEV?
Ok.
>
>> + dwc3_qcom_suspend_hsphy(qcom);
>> +
>> + if (dwc->usb2_generic_phy)
>> + phy_pm_runtime_put_sync(dwc->usb2_generic_phy);
>> + if (dwc->usb3_generic_phy)
>> + phy_pm_runtime_put_sync(dwc->usb3_generic_phy);
> core.c should do this.
Recommended sequence from h/w programming guide is that:
1. PHY autosuspend must be left disabled - snps,dis_u2_susphy_quirk/dis_enblslpm_quirk
2. During runtime-suspend (say on xhci bus_suspend) , PHY should be suspended
    using GUSB2PHYCFG register
3. Wait until pwr_event_irq_stat in qscratch reflects PHY transition to L2.
3. USB2 PHY driver can suspend - enable wakeup events and turns off clocks etc.
4. dwc3 glue driver can suspend.

Since, pwr_event_irq stat can't be checked in core driver, I added this handling
in glue driver. Alternative approach I can think of is to let dwc3 core suspend
PHY using GUSBPHYCFG register on suspend,  add some delay before
suspending PHY. Glue driver can check for pwr_event_irq stat and throw a
warning if PHY not in L2.
On runtime-resume, dwc3 core based on quirks - snps,dis_u2_susphy_quirk
can restore/clear the GUSB2PHYCFG register.

>
>> + /* Disable HSPHY auto suspend */
>> + val = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
>> + val &= ~(DWC3_GUSB2PHYCFG_ENBLSLPM | DWC3_GUSB2PHYCFG_SUSPHY);
>> + dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), val);
>> +
>> + qcom->is_suspended = false;
>> +
>> + dev_dbg(qcom->dev, "DWC3 exited from low power mode\n");
> no dev_dbg() or dev_info() in this driver.
ok

>
>> +static irqreturn_t qcom_dwc3_resume_irq(int irq, void *data)
>> +{
>> + struct dwc3_qcom *qcom = data;
>> + struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
>> +
>> + /* If pm_suspended then let pm_resume take care of resuming h/w */
>> + if (qcom->pm_suspended)
>> + return IRQ_HANDLED;
>> +
>> + dwc3_qcom_resume(qcom);
> instead, why don't you pm_runtime_resume() here, and let runtime_resume()
> handle dwc3_qcom_resume() ?

Sure. I added this earlier because runtime_pm might be disabled
if irq races with pm_resume. Now with pm_suspended check above, I can try
your suggestion.

>
>> +static int dwc3_qcom_clk_init(struct dwc3_qcom *qcom, int count)
>> +{
>> + struct device *dev = qcom->dev;
>> + struct device_node *np = dev->of_node;
>> + int i;
>> +
>> + qcom->num_clocks = count;
>> +
>> + if (!count)
>> + return 0;
>> +
>> + qcom->clks = devm_kcalloc(dev, qcom->num_clocks,
>> + sizeof(struct clk *), GFP_KERNEL);
>> + if (!qcom->clks)
>> + return -ENOMEM;
>> +
>> + for (i = 0; i < qcom->num_clocks; i++) {
>> + struct clk *clk;
>> + int ret;
>> +
>> + clk = of_clk_get(np, i);
>> + if (IS_ERR(clk)) {
>> + while (--i >= 0)
>> + clk_put(qcom->clks[i]);
>> + return PTR_ERR(clk);
>> + }
>> +
>> + ret = clk_prepare_enable(clk);
>> + if (ret < 0) {
>> + while (--i >= 0) {
>> + clk_disable_unprepare(qcom->clks[i]);
>> + clk_put(qcom->clks[i]);
>> + }
>> + clk_put(clk);
>> +
>> + return ret;
>> + }
>> +
>> + qcom->clks[i] = clk;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int dwc3_qcom_probe(struct platform_device *pdev)
>> +{
>> + struct device_node *np = pdev->dev.of_node, *dwc3_np;
>> + struct dwc3_qcom *qcom;
>> + struct resource *res;
>> + int irq, ret, i;
>> + bool ignore_pipe_clk;
>> +
>> + qcom = devm_kzalloc(&pdev->dev, sizeof(*qcom), GFP_KERNEL);
>> + if (!qcom)
>> + return -ENOMEM;
>> +
>> + platform_set_drvdata(pdev, qcom);
>> + qcom->dev = &pdev->dev;
>> +
>> + qcom->resets = of_reset_control_array_get_optional_exclusive(np);
>> + if (IS_ERR(qcom->resets)) {
>> + ret = PTR_ERR(qcom->resets);
>> + dev_err(&pdev->dev, "failed to get resets, err=%d\n", ret);
>> + return ret;
>> + }
>> +
>> + ret = reset_control_deassert(qcom->resets);
>> + if (ret)
>> + goto reset_put;
>> +
>> + ret = dwc3_qcom_clk_init(qcom, of_clk_get_parent_count(np));
>> + if (ret) {
>> + dev_err(qcom->dev, "failed to get clocks\n");
>> + goto reset_assert;
>> + }
>> +
>> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "qscratch");
>> + qcom->qscratch_base = devm_ioremap_resource(qcom->dev, res);
>> + if (IS_ERR(qcom->qscratch_base)) {
>> + dev_err(qcom->dev, "failed to map qscratch - %d\n", ret);
>> + ret = PTR_ERR(qcom->qscratch_base);
>> + goto clk_disable;
>> + }
>> +
>> + irq = platform_get_irq_byname(pdev, "hs_phy_irq");
>> + if (irq > 0) {
>> + ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
>> + qcom_dwc3_resume_irq,
>> + IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
>> + "qcom_dwc3 HS", qcom);
>> + if (ret) {
>> + dev_err(qcom->dev, "hs_phy_irq failed: %d\n", ret);
>> + goto clk_disable;
>> + }
>> + }
>> +
>> + irq = platform_get_irq_byname(pdev, "dp_hs_phy_irq");
>> + if (irq > 0) {
>> + irq_set_status_flags(irq, IRQ_NOAUTOEN);
> why do you need to set this flag?
These wakeup_irqs should be enabled only during suspend. With this flag I
don't need to disable irq immediately after requesting it.

>
>> + ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
>> + qcom_dwc3_resume_irq,
>> + IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
>> + "qcom_dwc3 DP_HS", qcom);
> this is the same as devm_request_irq()
I am passing hard_irq handler as NULL whereas thread_fn is not NULL.
devm_request_irq is just the opposite.


>
>> + if (ret) {
>> + dev_err(qcom->dev, "dp_hs_phy_irq failed: %d\n", ret);
>> + goto clk_disable;
>> + }
>> + qcom->dp_hs_phy_irq = irq;
>> + }
>> +
>> + irq = platform_get_irq_byname(pdev, "dm_hs_phy_irq");
>> + if (irq > 0) {
>> + irq_set_status_flags(irq, IRQ_NOAUTOEN);
>> + ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
>> + qcom_dwc3_resume_irq,
>> + IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
>> + "qcom_dwc3 DM_HS", qcom);
>> + if (ret) {
>> + dev_err(qcom->dev, "dm_hs_phy_irq failed: %d\n", ret);
>> + goto clk_disable;
>> + }
>> + qcom->dm_hs_phy_irq = irq;
>> + }
>> +
>> + irq = platform_get_irq_byname(pdev, "ss_phy_irq");
>> + if (irq > 0) {
>> + irq_set_status_flags(irq, IRQ_NOAUTOEN);
>> + ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
>> + qcom_dwc3_resume_irq,
>> + IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
>> + "qcom_dwc3 SS", qcom);
>> + if (ret) {
>> + dev_err(qcom->dev, "ss_phy_irq failed: %d\n", ret);
>> + goto clk_disable;
>> + }
>> + qcom->ss_phy_irq = irq;
>> + }
>> +
>> + dwc3_np = of_get_child_by_name(np, "dwc3");
>> + if (!dwc3_np) {
>> + dev_err(qcom->dev, "failed to find dwc3 core child\n");
>> + ret = -ENODEV;
>> + goto clk_disable;
>> + }
>> +
>> + /*
>> + * Disable pipe_clk requirement if specified. Used when dwc3
>> + * operates without SSPHY and only HS/FS/LS modes are supported.
>> + */
>> + ignore_pipe_clk = device_property_read_bool(qcom->dev,
>> + "qcom,select-utmi-as-pipe-clk");
>> + if (ignore_pipe_clk)
>> + dwc3_qcom_select_utmi_clk(qcom);
>> +
>> + ret = of_platform_populate(np, NULL, NULL, qcom->dev);
>> + if (ret) {
>> + dev_err(qcom->dev, "failed to register dwc3 core - %d\n", ret);
>> + goto clk_disable;
>> + }
>> +
>> + qcom->dwc3 = of_find_device_by_node(dwc3_np);
>> + if (!qcom->dwc3) {
>> + dev_err(&pdev->dev, "failed to get dwc3 platform device\n");
>> + goto depopulate;
>> + }
>> +
>> + qcom->mode = usb_get_dr_mode(&qcom->dwc3->dev);
>> +
>> + /* register extcon to override vbus later on mode switch */
>> + if (qcom->mode == USB_DR_MODE_OTG) {
>> + ret = dwc3_qcom_register_extcon(qcom);
>> + if (ret)
>> + goto depopulate;
>> + } else if (qcom->mode == USB_DR_MODE_PERIPHERAL) {
>> + /* enable vbus override for device mode */
>> + dwc3_qcom_vbus_overrride_enable(qcom, true);
>> + }
>> +
>> + device_init_wakeup(&pdev->dev, 1);
>> + qcom->is_suspended = false;
>> + pm_runtime_set_active(qcom->dev);
>> + pm_runtime_enable(qcom->dev);
>> +
>> + return 0;
>> +
>> +depopulate:
>> + of_platform_depopulate(&pdev->dev);
>> +clk_disable:
>> + for (i = qcom->num_clocks - 1; i >= 0; i--) {
>> + clk_disable_unprepare(qcom->clks[i]);
>> + clk_put(qcom->clks[i]);
>> + }
>> +reset_assert:
>> + reset_control_assert(qcom->resets);
>> +reset_put:
>> + reset_control_put(qcom->resets);
>> +
>> + return ret;
>> +}
>> +
>> +static int dwc3_qcom_remove(struct platform_device *pdev)
>> +{
>> + struct dwc3_qcom *qcom = platform_get_drvdata(pdev);
>> + struct device *dev = qcom->dev;
>> + int i;
>> +
>> + of_platform_depopulate(&pdev->dev);
>> +
>> + for (i = qcom->num_clocks - 1; i >= 0; i--) {
>> + clk_disable_unprepare(qcom->clks[i]);
>> + clk_put(qcom->clks[i]);
>> + }
>> + qcom->num_clocks = 0;
>> +
>> + reset_control_assert(qcom->resets);
>> + reset_control_put(qcom->resets);
>> +
>> + pm_runtime_disable(dev);
>> +
>> + return 0;
>> +}
>> +
>> +#ifdef CONFIG_PM_SLEEP
>> +static int dwc3_qcom_pm_suspend(struct device *dev)
>> +{
>> + struct dwc3_qcom *qcom = dev_get_drvdata(dev);
>> + int ret = 0;
>> +
>> + dev_err(dev, "dwc3-qcom PM suspend\n");
> why an error message here? There's no error. Remove this, it's unnecessary.
That is by mistake. I wanted a dev_dbg here. As you suggested I can just
remove this and following error messages.

>
>> + ret = dwc3_qcom_suspend(qcom);
>> + if (!ret)
>> + qcom->pm_suspended = true;
>> +
>> + return ret;
>> +}
>> +
>> +static int dwc3_qcom_pm_resume(struct device *dev)
>> +{
>> + struct dwc3_qcom *qcom = dev_get_drvdata(dev);
>> + int ret;
>> +
>> + dev_err(dev, "dwc3-qcom PM resume\n");
> ditto
>
>> + ret = dwc3_qcom_resume(qcom);
>> + if (!ret)
>> + qcom->pm_suspended = false;
>> +
>> + return ret;
>> +}
>> +#endif
>> +
>> +#ifdef CONFIG_PM
>> +static int dwc3_qcom_runtime_suspend(struct device *dev)
>> +{
>> + struct dwc3_qcom *qcom = dev_get_drvdata(dev);
>> +
>> + dev_err(dev, "DWC3-qcom runtime suspend\n");
> ditto
>
>> + return dwc3_qcom_suspend(qcom);
>> +}
>> +
>> +static int dwc3_qcom_runtime_resume(struct device *dev)
>> +{
>> + struct dwc3_qcom *qcom = dev_get_drvdata(dev);
>> +
>> + dev_err(dev, "DWC3-qcom runtime resume\n");
> ditto
>
>> + return dwc3_qcom_resume(qcom);
>> +}
>> +#endif
>> +
>> +static const struct dev_pm_ops dwc3_qcom_dev_pm_ops = {
>> + SET_SYSTEM_SLEEP_PM_OPS(dwc3_qcom_pm_suspend, dwc3_qcom_pm_resume)
>> + SET_RUNTIME_PM_OPS(dwc3_qcom_runtime_suspend, dwc3_qcom_runtime_resume,
>> + NULL)
> why don't you have runtime_idle?

I didn't see any need for that. rpm_idle will invoke rpm_suspend if idle callback
is not specified.

>

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


2018-03-14 08:53:27

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] usb: dwc3: Add Qualcomm DWC3 glue driver


Hi,

Manu Gautam <[email protected]> writes:
> Hi,
>
>
> On 3/13/2018 4:38 PM, Felipe Balbi wrote:
>> Hi,
>>
>> +Andy
>>
>> Manu Gautam <[email protected]> writes:
>>> DWC3 controller on Qualcomm SOCs has a Qscratch wrapper.
>>> Some of its uses are described below resulting in need to
>>> have a separate glue driver instead of using dwc3-of-simple:
>>> - It exposes register interface to override vbus-override
>>> and lane0-pwr-present signals going to hardware. These
>>> must be updated in peripheral mode for DWC3 if vbus lines
>>> are not connected to hardware block. Otherwise RX termination
>>> in SS mode or DP pull-up is not applied by device controller.
>> right, core needs to know that VBUS is above 4.4V. Why wasn't this a
>> problem when the original glue layer was first published?
>
> Thanks for reviewing.
> Original glue layer supported only host mode, hence this wasn't needed.

okay

>>> - pwr_events_irq_stat support to ensure USB2 PHY is in L2 state
>>> before glue driver can turn-off clocks and suspend PHY.
>> Core manages PHY suspend automatically. Isn't that working for you? Why?
>
> Yes, it is not supported with QUSB2 PHY (usb2-phy on Qualcomm SOCs).

but the PHY doesn't need to support it, DWC3 does :-)

> Issue is that If core suspends USB2 PHY (say in host mode if some SS device connected),
> USB2 PHY stops responding to any attach event as it can't exit suspend state by itself.

Okay, so we miss remote wakeup events. Fair enough.

>>> - Support to replace pip3 clock going to DWC3 with utmi clock
>>> for hardware configuration where SSPHY is not used with DWC3.
>> Is that SW configurable? Really? In any case seems like this and SESSVLD
>> valid should be handled using Hans' and Heikki's mux support.
>
> Yes, with this we can use dwc3 without using SSPHY. Please point me to
> those patches. There are only bunch of register writes in glue wrapper to
> achieve the same.

https://www.spinics.net/lists/linux-usb/msg160868.html

>>> +static int dwc3_qcom_suspend(struct dwc3_qcom *qcom)
>>> +{
>>> + struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
>> nope! Glue shouldn't touch dwc3 at all.
> Other than PHY handles, need this to fail runtime suspend if dwc3 hasn't
> probed yet.

Will that even happen? You should pm_runtime_forbid() by default,
anyway and expect it to be enabled via sysfs later, no?

>>> + dwc3_qcom_suspend_hsphy(qcom);
>>> +
>>> + if (dwc->usb2_generic_phy)
>>> + phy_pm_runtime_put_sync(dwc->usb2_generic_phy);
>>> + if (dwc->usb3_generic_phy)
>>> + phy_pm_runtime_put_sync(dwc->usb3_generic_phy);
>> core.c should do this.
> Recommended sequence from h/w programming guide is that:
> 1. PHY autosuspend must be left disabled - snps,dis_u2_susphy_quirk/dis_enblslpm_quirk
> 2. During runtime-suspend (say on xhci bus_suspend) , PHY should be suspended
>     using GUSB2PHYCFG register

this is something that dwc3 core can do on its own suspend implementation.

> 3. Wait until pwr_event_irq_stat in qscratch reflects PHY transition to L2.

this is interesting part. Is this register accessible by the PHY driver?
Seems like that would be the best place to stuff this...

> 3. USB2 PHY driver can suspend - enable wakeup events and turns off clocks etc.

... together with this.

> 4. dwc3 glue driver can suspend.
>
> Since, pwr_event_irq stat can't be checked in core driver, I added this handling
> in glue driver. Alternative approach I can think of is to let dwc3 core suspend
> PHY using GUSBPHYCFG register on suspend,  add some delay before
> suspending PHY. Glue driver can check for pwr_event_irq stat and throw a
> warning if PHY not in L2.

almost :-) core_suspend fiddles with GUSB2PHYCFG for suspend and calls
phy_suspend() (or whatever the function is called heh), that will go to
your phy driver's suspend callback, which checks pwr_event_irq_stat and
then pm_runtime_put() to schedule ->runtime_suspend() so that can enable
wakeups and switch off clocks.

>>> + irq = platform_get_irq_byname(pdev, "dp_hs_phy_irq");
>>> + if (irq > 0) {
>>> + irq_set_status_flags(irq, IRQ_NOAUTOEN);
>> why do you need to set this flag?
> These wakeup_irqs should be enabled only during suspend. With this flag I
> don't need to disable irq immediately after requesting it.

oh, okay. You may want to add a comment here :-)

>
>>
>>> + ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
>>> + qcom_dwc3_resume_irq,
>>> + IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
>>> + "qcom_dwc3 DP_HS", qcom);
>> this is the same as devm_request_irq()
> I am passing hard_irq handler as NULL whereas thread_fn is not NULL.
> devm_request_irq is just the opposite.

oh, indeed. I misparsed it.

>>> +static const struct dev_pm_ops dwc3_qcom_dev_pm_ops = {
>>> + SET_SYSTEM_SLEEP_PM_OPS(dwc3_qcom_pm_suspend, dwc3_qcom_pm_resume)
>>> + SET_RUNTIME_PM_OPS(dwc3_qcom_runtime_suspend, dwc3_qcom_runtime_resume,
>>> + NULL)
>> why don't you have runtime_idle?
>
> I didn't see any need for that. rpm_idle will invoke rpm_suspend if idle callback
> is not specified.

Right, but ->runtime_idle() helps making things a little more
readable. It is, however, a matter of taste. No strong feelings here. :-)

--
balbi


Attachments:
signature.asc (847.00 B)

2018-03-14 10:49:11

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] usb: dwc3: Add Qualcomm DWC3 glue driver

Hi Manu,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v4.16-rc4]
[also build test ERROR on next-20180314]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Manu-Gautam/dt-bindings-usb-Update-documentation-for-Qualcomm-DWC3-driver/20180314-095557
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64

All errors (new ones prefixed by >>):

>> ERROR: "__tracepoint_dwc3_writel" [drivers/usb/dwc3/dwc3-qcom.ko] undefined!
>> ERROR: "__tracepoint_dwc3_readl" [drivers/usb/dwc3/dwc3-qcom.ko] undefined!

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (944.00 B)
.config.gz (61.62 kB)
Download all attachments

2018-03-14 13:41:51

by Heikki Krogerus

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] usb: dwc3: Add Qualcomm DWC3 glue driver

Hi,

On Wed, Mar 14, 2018 at 10:50:12AM +0200, Felipe Balbi wrote:
> >>> - Support to replace pip3 clock going to DWC3 with utmi clock
> >>> for hardware configuration where SSPHY is not used with DWC3.
> >> Is that SW configurable? Really? In any case seems like this and SESSVLD
> >> valid should be handled using Hans' and Heikki's mux support.
> >
> > Yes, with this we can use dwc3 without using SSPHY. Please point me to
> > those patches. There are only bunch of register writes in glue wrapper to
> > achieve the same.
>
> https://www.spinics.net/lists/linux-usb/msg160868.html

I'm not sure if that series is usable in this case. Though we do
control the VBUSVLD pin on DWC3 in the driver for the "Intel USB
mux" Hans is introducing, the framework for the USB role switches does
not provide any support for it.

The Intel USB mux driver simply enables VBUSVLD when device mode is
selected, and disables it if host mode is selected, and when the
selection is cleared (disconnection).


Br,

--
heikki

2018-03-15 08:18:02

by Manu Gautam

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] usb: dwc3: Add Qualcomm DWC3 glue driver

Hi,


On 3/14/2018 2:20 PM, Felipe Balbi wrote:
> Hi,
>
> Manu Gautam <[email protected]> writes:
>
[snip]
>>>> - Support to replace pip3 clock going to DWC3 with utmi clock
>>>> for hardware configuration where SSPHY is not used with DWC3.
>>> Is that SW configurable? Really? In any case seems like this and SESSVLD
>>> valid should be handled using Hans' and Heikki's mux support.
>> Yes, with this we can use dwc3 without using SSPHY. Please point me to
>> those patches. There are only bunch of register writes in glue wrapper to
>> achieve the same.
> https://www.spinics.net/lists/linux-usb/msg160868.html

I looked at the patchset and thinking that adding mux for this may
not be of much help here. Qscratch is part of dwc3 wrapper
and uses same clock domain for its registers. Hence, I can't
have a separate mux driver for same. Will have to register
mux controller from this driver for these and use only in this driver
as I dont expect any other client for same. So, can I proceed with
existing logic?

>
>>>> +static int dwc3_qcom_suspend(struct dwc3_qcom *qcom)
>>>> +{
>>>> + struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
>>> nope! Glue shouldn't touch dwc3 at all.
>> Other than PHY handles, need this to fail runtime suspend if dwc3 hasn't
>> probed yet.
> Will that even happen? You should pm_runtime_forbid() by default,
> anyway and expect it to be enabled via sysfs later, no?

It happens if I enable runtime_pm from probe which is the case right now.
I will disable it from probe as you suggested.
In any case dwc3 uses pm_runtime_forbid and expects it to be enabled from sysfs,
so I dont get any benefit of enabling it from glue driver probe.

Other than this, I need to access 'dwc->xhci' to resume root hub on remote wakeup.
That I missed to mention earlier.

>
>>>> + dwc3_qcom_suspend_hsphy(qcom);
>>>> +
>>>> + if (dwc->usb2_generic_phy)
>>>> + phy_pm_runtime_put_sync(dwc->usb2_generic_phy);
>>>> + if (dwc->usb3_generic_phy)
>>>> + phy_pm_runtime_put_sync(dwc->usb3_generic_phy);
>>> core.c should do this.
>> Recommended sequence from h/w programming guide is that:
>> 1. PHY autosuspend must be left disabled - snps,dis_u2_susphy_quirk/dis_enblslpm_quirk
>> 2. During runtime-suspend (say on xhci bus_suspend) , PHY should be suspended
>>     using GUSB2PHYCFG register
> this is something that dwc3 core can do on its own suspend implementation.
>
>> 3. Wait until pwr_event_irq_stat in qscratch reflects PHY transition to L2.
> this is interesting part. Is this register accessible by the PHY driver?
> Seems like that would be the best place to stuff this...

This register is in controller wrapper which PHY driver can't access.
Also clock domain is different.

>
>> 3. USB2 PHY driver can suspend - enable wakeup events and turns off clocks etc.
> ... together with this.
>
>> 4. dwc3 glue driver can suspend.
>>
>> Since, pwr_event_irq stat can't be checked in core driver, I added this handling
>> in glue driver. Alternative approach I can think of is to let dwc3 core suspend
>> PHY using GUSBPHYCFG register on suspend,  add some delay before
>> suspending PHY. Glue driver can check for pwr_event_irq stat and throw a
>> warning if PHY not in L2.
> almost :-) core_suspend fiddles with GUSB2PHYCFG for suspend and calls
> phy_suspend() (or whatever the function is called heh), that will go to
> your phy driver's suspend callback, which checks pwr_event_irq_stat and
> then pm_runtime_put() to schedule ->runtime_suspend() so that can enable
> wakeups and switch off clocks.

Since phy driver can not access pwr_event_irq_stat, should I just use some delay
and assume PHY gets suspended?

>
>>>> + irq = platform_get_irq_byname(pdev, "dp_hs_phy_irq");
>>>> + if (irq > 0) {
>>>> + irq_set_status_flags(irq, IRQ_NOAUTOEN);
>>> why do you need to set this flag?
>> These wakeup_irqs should be enabled only during suspend. With this flag I
>> don't need to disable irq immediately after requesting it.
> oh, okay. You may want to add a comment here :-)
Sure.

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


2018-03-18 12:52:56

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v1 1/2] dt-bindings: usb: Update documentation for Qualcomm DWC3 driver

On Tue, Mar 13, 2018 at 04:06:00PM +0530, Manu Gautam wrote:
> Existing documentation has lot of incorrect information as it
> was originally added for a driver that no longer exists.
>
> Signed-off-by: Manu Gautam <[email protected]>
> ---
> .../devicetree/bindings/usb/qcom,dwc3.txt | 87 +++++++++++++++-------
> 1 file changed, 59 insertions(+), 28 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.txt b/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
> index bc8a2fa..df312f7 100644
> --- a/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
> +++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
> @@ -1,54 +1,85 @@
> 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).
> +- compatible: should contain "qcom,dwc3"
> +- reg: offset and length of register set for QSCRATCH wrapper
> +- reg-names: should be "qscratch"

reg-names is pointless for a single range.

> +- power-domains: specifies a phandle to PM domain provider node
> +- clocks: list of phandle + clock-specifier pairs

How many clocks and what are they?

> +- assigned-clocks: should be:
> + MOCK_UTMI_CLK
> + MASTER_CLK
> +- assigned-clock-rates: should be:
> + 19.2Mhz (192000000) for MOCK_UTMI_CLK
> + >=125Mhz (125000000) for MASTER_CLK in SS mode
> + >=60Mhz (60000000) for MASTER_CLK in HS mode
> +
> +Optional properties:
> +- resets: list of phandle and reset specifier pairs
> +- interrupts: specifies interrupts from controller wrapper used
> + to wakeup from low power/susepnd state. Must contain
> + one or more entry for interrupt-names property
> +- interrupt-names: Must include the following entries:
> + - "hs_phy_irq": The interrupt that is asserted when a
> + wakeup event is received on USB2 bus
> + - "ss_phy_irq": The interrupt that is asserted when a
> + wakeup event is received on USB3 bus
> + - "dm_hs_phy_irq" and "dp_hs_phy_irq": Separate
> + interrupts for any wakeup event on DM and DP lines
> +- qcom,select-utmi-as-pipe-clk: if present, disable USB3 pipe_clk requirement.
> + Used when dwc3 operates without SSPHY and only
> + HS/FS/LS modes are supported.
>
> 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
> +Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt - USB3 QMP PHY
> +Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt - USB2 QUSB2 PHY

2018-03-19 04:21:37

by Manu Gautam

[permalink] [raw]
Subject: Re: [PATCH v1 1/2] dt-bindings: usb: Update documentation for Qualcomm DWC3 driver

Hi,


On 3/18/2018 6:19 PM, Rob Herring wrote:
> On Tue, Mar 13, 2018 at 04:06:00PM +0530, Manu Gautam wrote:
>> Existing documentation has lot of incorrect information as it
>> was originally added for a driver that no longer exists.
>>
>> Signed-off-by: Manu Gautam <[email protected]>
>> ---
>> .../devicetree/bindings/usb/qcom,dwc3.txt | 87 +++++++++++++++-------
>> 1 file changed, 59 insertions(+), 28 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.txt b/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
>> index bc8a2fa..df312f7 100644
>> --- a/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
>> +++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.txt
>> @@ -1,54 +1,85 @@
>> 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).
>> +- compatible: should contain "qcom,dwc3"
>> +- reg: offset and length of register set for QSCRATCH wrapper
>> +- reg-names: should be "qscratch"
> reg-names is pointless for a single range.
Ok. Will change this.

>
>> +- power-domains: specifies a phandle to PM domain provider node
>> +- clocks: list of phandle + clock-specifier pairs
> How many clocks and what are they?
I will add description of the clocks in next version of patchset.


>
>> +- assigned-clocks: should be:
>> + MOCK_UTMI_CLK
>> + MASTER_CLK
>> +- assigned-clock-rates: should be:
>> + 19.2Mhz (192000000) for MOCK_UTMI_CLK
>> + >=125Mhz (125000000) for MASTER_CLK in SS mode
>> + >=60Mhz (60000000) for MASTER_CLK in HS mode
>> +
>> +Optional properties:
>> +- resets: list of phandle and reset specifier pairs
>> +- interrupts: specifies interrupts from controller wrapper used
>> + to wakeup from low power/susepnd state. Must contain
>> + one or more entry for interrupt-names property
>> +- interrupt-names: Must include the following entries:
>> + - "hs_phy_irq": The interrupt that is asserted when a
>> + wakeup event is received on USB2 bus
>> + - "ss_phy_irq": The interrupt that is asserted when a
>> + wakeup event is received on USB3 bus
>> + - "dm_hs_phy_irq" and "dp_hs_phy_irq": Separate
>> + interrupts for any wakeup event on DM and DP lines
>> +- qcom,select-utmi-as-pipe-clk: if present, disable USB3 pipe_clk requirement.
>> + Used when dwc3 operates without SSPHY and only
>> + HS/FS/LS modes are supported.
>>
>> 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
>> +Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt - USB3 QMP PHY
>> +Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt - USB2 QUSB2 PHY

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