[PATCH 00/21] Support qcom's HSIC USB and rewrite USB2 HS phy support

[PATCH 00/21] Support qcom's HSIC USB and rewrite USB2 HS phy support

[Stephen Boyd]

The state of USB ChipIdea support on Qualcomm's platforms is not great.
The DT description of these devices requires up to three different nodes
for what amounts to be the same hardware block, when there should really
only be one. Furthermore, the "phy" driver that is in mainline (phy-msm-usb.c)
duplicates the OTG state machine and touches the ci controller wrapper
registers when it should really be focused on the phy and the ULPI accesses
needed to get the phy working. There's also a slimmed down phy driver for
the msm8916 platform, but really the phy hardware is the same as other MSMs,
so we have two drivers doing pretty much the same thing. This leads to a
situtaion where we have the chipidea core driver, the "phy" driver, and
sometimes the ehci-msm.c driver operating the same device all at the same
time with very little coordination. This just isn't very safe and is
confusing from a driver perspective when trying to figure out who does what.
Finally, there isn't any HSIC support on platforms like apq8074 so we
should add that.

This patch series updates the ChipIdea driver and the MSM wrapper
(ci_hdrc_msm.c) to properly handle the PHY and wrapper bits at the right
times in the right places. To get there, we update the ChipIdea core to
have support for the ULPI phy bus introduced by Heikki. Along the way
we fix bugs with the extcon handling for peripheral and OTG mode controllers
and move the parts of phy-usb-msm.c that are touching the CI controller
wrapper into the wrapper driver (ci_hdrc_msm.c). Finally we add support
for the HSIC phy based on the ULPI bus and rewrite the HS phy driver
(phy-usb-msm.c) as a standard ULPI phy driver.

Once this series is accepted, we should be able to delete the phy-usb-msm.c
phy-qcom-8x16-usb.c, and ehci-msm.c drivers from the tree and use the ULPI
based phy driver (which also lives in drivers/phy/ instead of drivers/usb/phy/)
and the chipidea host core instead.

I've also sent seperate patches for other minor pieces to make this
all work. The full tree can be found here[3], hacks and all to get
things working. I've tested this on the db410c, apq8074 dragonboard,
and ifc6410 with configfs gadgets and otg cables.

TODO:
 * DMA fails on arm64 so we need something like [1] to make it work.

 * The HSIC phy on the apq8074 dragonboard is connected to a usb4604
   device which requires the i2c driver to probe and send an i2c
   sequence before the HSIC controller enumerates or HSIC doesn't work.
   Right now I have a hack to force the controller to probe defer
   once so that usb4604 probes first. This needs a more proper solution
   like having the DT describe a linkage between the controller and
   the usb device so we can enforce probe ordering.
 
 * OTG support requires a working VBUS supply on apq8074 dragonboard
   and that requires changes to the smbb_charger driver to support
   the OTG OVP switch as a regulator[2]. This series needs revival.

 * Sleeping while atomic problems exist when trying to do phy operations
   underneath some spinlocks in the ci core. I have a patch to remove a
   spinlock, but that needs more thought if it's correct. At the least
   it's necessary though because of how we need to initialize the HSIC
   phy after the reset bit is toggled in USBCMD.

[1] https://lkml.org/lkml/2016/2/22/7
[2] http://lkml.kernel.org/g/1449621618-11900-1-git-send-email-tim.bird@sonymobile.com
[3] https://git.linaro.org/people/stephen.boyd/linux.git/shortlog/refs/heads/usb-hsic-8074

Stephen Boyd (21):
  of: device: Support loading a module with OF based modalias
  usb: ulpi: Support device discovery via DT
  usb: ulpi: Avoid reading/writing in device creation with OF devices
  usb: chipidea: Only read/write OTGSC from one place
  usb: chipidea: Handle extcon events properly
  usb: chipidea: Initialize and reinitialize phy later
  usb: chipidea: Notify of reset when switching into host mode
  usb: chipidea: Kick OTG state machine for AVVIS with vbus extcon
  usb: chipidea: Add support for ULPI PHY bus
  usb: chipidea: msm: Rely on core to override AHBBURST
  usb: chipidea: msm: Use hw_write_id_reg() instead of writel directly
  usb: chipidea: msm: Keep device runtime enabled
  usb: chipidea: msm: Allow core to get usb phy
  usb: chipidea: msm: Add proper clk and reset support
  usb: chipidea: msm: Mux over secondary phy at the right time
  usb: chipidea: msm: Restore wrapper settings after reset
  usb: chipidea: msm: Make platform data driver local instead of global
  usb: chipidea: msm: Add reset controller for PHY POR bit
  usb: chipidea: msm: Be silent on probe defer errors
  phy: Add support for Qualcomm's USB HSIC phy
  phy: Add support for Qualcomm's USB HS phy

 .../devicetree/bindings/phy/qcom,usb-hs-phy.txt    |  71 ++++++
 .../devicetree/bindings/phy/qcom,usb-hsic-phy.txt  |  60 +++++
 Documentation/devicetree/bindings/usb/ulpi.txt     |  20 ++
 drivers/of/device.c                                |  50 ++++
 drivers/phy/Kconfig                                |  15 ++
 drivers/phy/Makefile                               |   2 +
 drivers/phy/phy-qcom-usb-hs.c                      | 283 +++++++++++++++++++++
 drivers/phy/phy-qcom-usb-hsic.c                    | 161 ++++++++++++
 drivers/usb/chipidea/Kconfig                       |   7 +
 drivers/usb/chipidea/Makefile                      |   1 +
 drivers/usb/chipidea/ci.h                          |  23 +-
 drivers/usb/chipidea/ci_hdrc_msm.c                 | 264 ++++++++++++++++---
 drivers/usb/chipidea/core.c                        |  85 +++----
 drivers/usb/chipidea/host.c                        |   6 +-
 drivers/usb/chipidea/otg.c                         |  81 +++++-
 drivers/usb/chipidea/otg_fsm.c                     |  17 ++
 drivers/usb/chipidea/udc.c                         |   2 +
 drivers/usb/chipidea/ulpi.c                        | 113 ++++++++
 drivers/usb/common/ulpi.c                          |  92 +++++--
 include/linux/of_device.h                          |   6 +
 include/linux/usb/chipidea.h                       |   2 +
 21 files changed, 1238 insertions(+), 123 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.txt
 create mode 100644 Documentation/devicetree/bindings/phy/qcom,usb-hsic-phy.txt
 create mode 100644 Documentation/devicetree/bindings/usb/ulpi.txt
 create mode 100644 drivers/phy/phy-qcom-usb-hs.c
 create mode 100644 drivers/phy/phy-qcom-usb-hsic.c
 create mode 100644 drivers/usb/chipidea/ulpi.c

-- 
2.9.0.rc2.8.ga28705d

[PATCH 01/21] of: device: Support loading a module with OF based modalias

[Stephen Boyd]

In the case of ULPI devices, we want to be able to load the
driver before registering the device so that we don't get stuck
in a loop waiting for the phy module to appear and failing usb
controller probe. Currently we request the ulpi module via the
ulpi ids, but in the DT case we might need to request it with the
OF based modalias instead. Add a common function that allows
anyone to request a module with the OF based modalias.

Cc: Rob Herring <robh+dt@kernel.org>
Cc: <devicetree@vger.kernel.org>
Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
---
 drivers/of/device.c       | 50 +++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/of_device.h |  6 ++++++
 2 files changed, 56 insertions(+)

diff --git a/drivers/of/device.c b/drivers/of/device.c
index fd5cfad7c403..f275e5beb736 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -226,6 +226,56 @@ ssize_t of_device_get_modalias(struct device *dev, char *str, ssize_t len)
 	return tsize;
 }
 
+static ssize_t of_device_modalias_size(struct device *dev)
+{
+	const char *compat;
+	int cplen, i;
+	ssize_t csize;
+
+	if ((!dev) || (!dev->of_node))
+		return -ENODEV;
+
+	/* Name & Type */
+	csize = 5 + strlen(dev->of_node->name) + strlen(dev->of_node->type);
+
+	/* Get compatible property if any */
+	compat = of_get_property(dev->of_node, "compatible", &cplen);
+	if (!compat)
+		return csize;
+
+	/* Find true end (we tolerate multiple \0 at the end */
+	for (i = (cplen - 1); i >= 0 && !compat[i]; i--)
+		cplen--;
+	if (!cplen)
+		return csize;
+	cplen++;
+
+	/* Check space (need cplen+1 chars including final \0) */
+	return csize + cplen;
+}
+
+int of_device_request_module(struct device *dev)
+{
+	char *str;
+	ssize_t size;
+	int ret;
+
+	size = of_device_modalias_size(dev);
+	if (size < 0)
+		return size;
+
+	str = kmalloc(size + 1, GFP_KERNEL);
+	if (!str)
+		return -ENOMEM;
+
+	of_device_get_modalias(dev, str, size);
+	str[size] = '\0';
+	ret = request_module(str);
+	kfree(str);
+
+	return ret;
+}
+
 /**
  * of_device_uevent - Display OF related uevent information
  */
diff --git a/include/linux/of_device.h b/include/linux/of_device.h
index cc7dd687a89d..e9afbcc8de12 100644
--- a/include/linux/of_device.h
+++ b/include/linux/of_device.h
@@ -37,6 +37,7 @@ extern const void *of_device_get_match_data(const struct device *dev);
 
 extern ssize_t of_device_get_modalias(struct device *dev,
 					char *str, ssize_t len);
+extern int of_device_request_module(struct device *dev);
 
 extern void of_device_uevent(struct device *dev, struct kobj_uevent_env *env);
 extern int of_device_uevent_modalias(struct device *dev, struct kobj_uevent_env *env);
@@ -78,6 +79,11 @@ static inline int of_device_get_modalias(struct device *dev,
 	return -ENODEV;
 }
 
+static inline int of_device_request_module(struct device *dev)
+{
+	return -ENODEV;
+}
+
 static inline int of_device_uevent_modalias(struct device *dev,
 				   struct kobj_uevent_env *env)
 {
-- 
2.9.0.rc2.8.ga28705d

[PATCH 02/21] usb: ulpi: Support device discovery via DT

[Stephen Boyd]

The qcom HSIC ulpi phy doesn't have any bits set in the vendor or
product id ulpi registers. This makes it impossible to make a
ulpi driver match against the id registers. Add support to
discover the ulpi phys via DT to help alleviate this problem.
We'll look for a ulpi bus node underneath the device registering
the ulpi viewport (or the parent of that device to support
chipidea's device layout) and then match up the phy node
underneath that with the ulpi device that's created.

The side benefit of this is that we can use standard DT
properties in the phy node like clks, regulators, gpios, etc.
because we don't have firmware like ACPI to turn these things on
for us. And we can use the DT phy binding to point our phy
consumer to the phy provider.

Cc: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
Cc: Heikki Krogerus <heikki.krogerus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Cc: <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Stephen Boyd <stephen.boyd-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 Documentation/devicetree/bindings/usb/ulpi.txt | 20 +++++++++
 drivers/usb/common/ulpi.c                      | 56 +++++++++++++++++++++++++-
 2 files changed, 74 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/usb/ulpi.txt

diff --git a/Documentation/devicetree/bindings/usb/ulpi.txt b/Documentation/devicetree/bindings/usb/ulpi.txt
new file mode 100644
index 000000000000..ca179dc4bd50
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/ulpi.txt
@@ -0,0 +1,20 @@
+ULPI bus binding
+----------------
+
+Phys that are behind a ULPI connection can be described with the following
+binding. The host controller shall have a "ulpi" named node as a child, and
+that node shall have one enabled node underneath it representing the ulpi
+device on the bus.
+
+EXAMPLE
+-------
+
+usb {
+	compatible = "vendor,usb-controller";
+
+	ulpi {
+		phy {
+			compatible = "vendor,phy";
+		};
+	};
+};
diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index 01c0c0477a9e..980af672bfe3 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -16,6 +16,9 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/acpi.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/clk/clk-conf.h>
 
 /* -------------------------------------------------------------------------- */
 
@@ -39,7 +42,10 @@ static int ulpi_match(struct device *dev, struct device_driver *driver)
 	struct ulpi *ulpi = to_ulpi_dev(dev);
 	const struct ulpi_device_id *id;
 
-	for (id = drv->id_table; id->vendor; id++)
+	if (of_driver_match_device(dev, driver))
+		return 1;
+
+	for (id = drv->id_table; id && id->vendor; id++)
 		if (id->vendor == ulpi->id.vendor &&
 		    id->product == ulpi->id.product)
 			return 1;
@@ -50,6 +56,11 @@ static int ulpi_match(struct device *dev, struct device_driver *driver)
 static int ulpi_uevent(struct device *dev, struct kobj_uevent_env *env)
 {
 	struct ulpi *ulpi = to_ulpi_dev(dev);
+	int ret;
+
+	ret = of_device_uevent_modalias(dev, env);
+	if (ret != -ENODEV)
+		return ret;
 
 	if (add_uevent_var(env, "MODALIAS=ulpi:v%04xp%04x",
 			   ulpi->id.vendor, ulpi->id.product))
@@ -60,6 +71,11 @@ static int ulpi_uevent(struct device *dev, struct kobj_uevent_env *env)
 static int ulpi_probe(struct device *dev)
 {
 	struct ulpi_driver *drv = to_ulpi_driver(dev->driver);
+	int ret;
+
+	ret = of_clk_set_defaults(dev->of_node, false);
+	if (ret < 0)
+		return ret;
 
 	return drv->probe(to_ulpi_dev(dev));
 }
@@ -87,8 +103,13 @@ static struct bus_type ulpi_bus = {
 static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
 			     char *buf)
 {
+	int len;
 	struct ulpi *ulpi = to_ulpi_dev(dev);
 
+	len = of_device_get_modalias(dev, buf, PAGE_SIZE - 1);
+	if (len != -ENODEV)
+		return len;
+
 	return sprintf(buf, "ulpi:v%04xp%04x\n",
 		       ulpi->id.vendor, ulpi->id.product);
 }
@@ -152,6 +173,28 @@ EXPORT_SYMBOL_GPL(ulpi_unregister_driver);
 
 /* -------------------------------------------------------------------------- */
 
+static int ulpi_of_register(struct ulpi *ulpi)
+{
+	struct device_node *np = NULL, *child;
+
+	/* Find a ulpi bus underneath the parent or the parent of the parent */
+	if (ulpi->dev.parent->of_node)
+		np = of_find_node_by_name(ulpi->dev.parent->of_node, "ulpi");
+	else if (ulpi->dev.parent->parent &&
+		 ulpi->dev.parent->parent->of_node)
+		np = of_find_node_by_name(ulpi->dev.parent->parent->of_node, "ulpi");
+	if (!np)
+		return 0;
+
+	child = of_get_next_available_child(np, NULL);
+	if (!child)
+		return -EINVAL;
+
+	ulpi->dev.of_node = child;
+
+	return 0;
+}
+
 static int ulpi_register(struct device *dev, struct ulpi *ulpi)
 {
 	int ret;
@@ -181,7 +224,15 @@ static int ulpi_register(struct device *dev, struct ulpi *ulpi)
 
 	ACPI_COMPANION_SET(&ulpi->dev, ACPI_COMPANION(dev));
 
-	request_module("ulpi:v%04xp%04x", ulpi->id.vendor, ulpi->id.product);
+	if (IS_ENABLED(CONFIG_OF)) {
+		ret = ulpi_of_register(ulpi);
+		if (ret)
+			return ret;
+	}
+
+	if (of_device_request_module(&ulpi->dev))
+		request_module("ulpi:v%04xp%04x", ulpi->id.vendor,
+			       ulpi->id.product);
 
 	ret = device_register(&ulpi->dev);
 	if (ret)
@@ -232,6 +283,7 @@ EXPORT_SYMBOL_GPL(ulpi_register_interface);
  */
 void ulpi_unregister_interface(struct ulpi *ulpi)
 {
+	of_node_put(ulpi->dev.of_node);
 	device_unregister(&ulpi->dev);
 }
 EXPORT_SYMBOL_GPL(ulpi_unregister_interface);
-- 
2.9.0.rc2.8.ga28705d

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 20/21] phy: Add support for Qualcomm's USB HSIC phy

[Stephen Boyd]

The HSIC USB controller on qcom SoCs has an integrated all
digital phy controlled via the ULPI viewport.

Cc: Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>
Cc: <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Signed-off-by: Stephen Boyd <stephen.boyd-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 .../devicetree/bindings/phy/qcom,usb-hsic-phy.txt  |  60 ++++++++
 drivers/phy/Kconfig                                |   7 +
 drivers/phy/Makefile                               |   1 +
 drivers/phy/phy-qcom-usb-hsic.c                    | 161 +++++++++++++++++++++
 4 files changed, 229 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/qcom,usb-hsic-phy.txt
 create mode 100644 drivers/phy/phy-qcom-usb-hsic.c

diff --git a/Documentation/devicetree/bindings/phy/qcom,usb-hsic-phy.txt b/Documentation/devicetree/bindings/phy/qcom,usb-hsic-phy.txt
new file mode 100644
index 000000000000..6b1c6aad2962
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/qcom,usb-hsic-phy.txt
@@ -0,0 +1,60 @@
+Qualcomm's USB HSIC PHY
+
+PROPERTIES
+
+- compatible:
+    Usage: required
+    Value type: <string>
+    Definition: Should contain "qcom,usb-hsic-phy"
+
+- #phy-cells:
+    Usage: required
+    Value type: <u32>
+    Definition: Should contain 0
+
+- clocks:
+    Usage: required
+    Value type: <prop-encoded-array>
+    Definition: Should contain clock specifier for phy, calibration and
+                optionally a calibration sleep clock
+
+- clock-names:
+    Usage: required
+    Value type: <stringlist>
+    Definition: Should contain "phy, "cal" and optionally "cal_sleep"
+
+- pinctrl-names:
+    Usage: required
+    Value type: <stringlist>
+    Definition: Should contain "init" and "default" in that order
+
+- pinctrl-0:
+    Usage: required
+    Value type: <prop-encoded-array>
+    Definition: List of pinctrl settings to apply to keep HSIC pins in a glitch
+                free state
+
+- pinctrl-1:
+    Usage: required
+    Value type: <prop-encoded-array>
+    Definition: List of pinctrl settings to apply to mux out the HSIC pins
+
+EXAMPLE
+
+usb-controller {
+	ulpi {
+		phy {
+			compatible = "qcom,usb-hsic-phy";
+			#phy-cells = <0>;
+			pinctrl-names = "init", "default";
+			pinctrl-0 = <&hsic_sleep>;
+			pinctrl-1 = <&hsic_default>;
+			clocks = <&gcc GCC_USB_HSIC_CLK>,
+				 <&gcc GCC_USB_HSIC_IO_CAL_CLK>,
+				 <&gcc GCC_USB_HSIC_IO_CAL_SLEEP_CLK>;
+			clock-names = "phy", "cal", "cal_sleep";
+			assigned-clocks = <&gcc GCC_USB_HSIC_IO_CAL_CLK>;
+			assigned-clock-rates = <960000>;
+		};
+	};
+};
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index b869b98835f4..a2866949dc97 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -406,6 +406,13 @@ config PHY_QCOM_UFS
 	help
 	  Support for UFS PHY on QCOM chipsets.
 
+config PHY_QCOM_USB_HSIC
+	tristate "Qualcomm USB HSIC ULPI PHY module"
+	depends on USB_ULPI_BUS
+	select GENERIC_PHY
+	help
+	  Support for the USB HSIC ULPI compliant PHY on QCOM chipsets.
+
 config PHY_TUSB1210
 	tristate "TI TUSB1210 ULPI PHY module"
 	depends on USB_ULPI_BUS
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 9c3e73ccabc4..982e84a290ec 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -49,6 +49,7 @@ obj-$(CONFIG_PHY_STIH41X_USB)		+= phy-stih41x-usb.o
 obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs.o
 obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs-qmp-20nm.o
 obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs-qmp-14nm.o
+obj-$(CONFIG_PHY_QCOM_USB_HSIC) 	+= phy-qcom-usb-hsic.o
 obj-$(CONFIG_PHY_TUSB1210)		+= phy-tusb1210.o
 obj-$(CONFIG_PHY_BRCM_SATA)		+= phy-brcm-sata.o
 obj-$(CONFIG_PHY_PISTACHIO_USB)		+= phy-pistachio-usb.o
diff --git a/drivers/phy/phy-qcom-usb-hsic.c b/drivers/phy/phy-qcom-usb-hsic.c
new file mode 100644
index 000000000000..a81b2f8bfe37
--- /dev/null
+++ b/drivers/phy/phy-qcom-usb-hsic.c
@@ -0,0 +1,161 @@
+/**
+ * Copyright (C) 2016 Linaro Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/module.h>
+#include <linux/ulpi/driver.h>
+#include <linux/ulpi/regs.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/pinctrl/pinctrl-state.h>
+#include <linux/delay.h>
+#include <linux/clk.h>
+
+#include "ulpi_phy.h"
+
+#define ULPI_HSIC_CFG		0x30
+#define ULPI_HSIC_IO_CAL	0x33
+
+struct qcom_usb_hsic_phy {
+	struct ulpi *ulpi;
+	struct phy *phy;
+	struct pinctrl *pctl;
+	struct clk *phy_clk;
+	struct clk *cal_clk;
+	struct clk *cal_sleep_clk;
+};
+
+static int qcom_usb_hsic_phy_power_on(struct phy *phy)
+{
+	struct qcom_usb_hsic_phy *uphy = phy_get_drvdata(phy);
+	struct ulpi *ulpi = uphy->ulpi;
+	struct pinctrl_state *pins_default;
+	int ret;
+
+	ret = clk_prepare_enable(uphy->phy_clk);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(uphy->cal_clk);
+	if (ret)
+		goto err_cal;
+
+	ret = clk_prepare_enable(uphy->cal_sleep_clk);
+	if (ret)
+		goto err_sleep;
+
+	/* Set periodic calibration interval to ~2.048sec in HSIC_IO_CAL_REG */
+	ret = ulpi_write(ulpi, ULPI_HSIC_IO_CAL, 0xff);
+	if (ret)
+		goto err_ulpi;
+
+	/* Enable periodic IO calibration in HSIC_CFG register */
+	ret = ulpi_write(ulpi, ULPI_HSIC_CFG, 0xa8);
+	if (ret)
+		goto err_ulpi;
+
+	/* Configure pins for HSIC functionality */
+	pins_default = pinctrl_lookup_state(uphy->pctl, PINCTRL_STATE_DEFAULT);
+	if (IS_ERR(pins_default))
+		return PTR_ERR(pins_default);
+
+	ret = pinctrl_select_state(uphy->pctl, pins_default);
+	if (ret)
+		goto err_ulpi;
+
+	 /* Enable HSIC mode in HSIC_CFG register */
+	ret = ulpi_write(ulpi, ULPI_SET(ULPI_HSIC_CFG), 0x01);
+	if (ret)
+		goto err_ulpi;
+
+	/* Disable auto-resume */
+	ret = ulpi_write(ulpi, ULPI_CLR(ULPI_IFC_CTRL),
+			 ULPI_IFC_CTRL_AUTORESUME);
+	if (ret)
+		goto err_ulpi;
+
+	return ret;
+err_ulpi:
+	clk_disable_unprepare(uphy->cal_sleep_clk);
+err_sleep:
+	clk_disable_unprepare(uphy->cal_clk);
+err_cal:
+	clk_disable_unprepare(uphy->phy_clk);
+	return ret;
+}
+
+static int qcom_usb_hsic_phy_power_off(struct phy *phy)
+{
+	struct qcom_usb_hsic_phy *uphy = phy_get_drvdata(phy);
+
+	clk_disable_unprepare(uphy->cal_sleep_clk);
+	clk_disable_unprepare(uphy->cal_clk);
+	clk_disable_unprepare(uphy->phy_clk);
+
+	return 0;
+}
+
+static const struct phy_ops qcom_usb_hsic_phy_ops = {
+	.power_on = qcom_usb_hsic_phy_power_on,
+	.power_off = qcom_usb_hsic_phy_power_off,
+	.owner = THIS_MODULE,
+};
+
+static int qcom_usb_hsic_phy_probe(struct ulpi *ulpi)
+{
+	struct qcom_usb_hsic_phy *uphy;
+	struct phy_provider *p;
+	struct clk *clk;
+
+	uphy = devm_kzalloc(&ulpi->dev, sizeof(*uphy), GFP_KERNEL);
+	if (!uphy)
+		return -ENOMEM;
+	ulpi_set_drvdata(ulpi, uphy);
+
+	uphy->ulpi = ulpi;
+	uphy->pctl = devm_pinctrl_get(&ulpi->dev);
+	if (IS_ERR(uphy->pctl))
+		return PTR_ERR(uphy->pctl);
+
+	uphy->phy_clk = clk = devm_clk_get(&ulpi->dev, "phy");
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	uphy->cal_clk = clk = devm_clk_get(&ulpi->dev, "cal");
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	uphy->cal_sleep_clk = clk = devm_clk_get(&ulpi->dev, "cal_sleep");
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	uphy->phy = devm_phy_create(&ulpi->dev, ulpi->dev.of_node,
+				    &qcom_usb_hsic_phy_ops);
+	if (IS_ERR(uphy->phy))
+		return PTR_ERR(uphy->phy);
+	phy_set_drvdata(uphy->phy, uphy);
+
+	p = devm_of_phy_provider_register(&ulpi->dev, of_phy_simple_xlate);
+	return PTR_ERR_OR_ZERO(p);
+}
+
+
+static const struct of_device_id qcom_usb_hsic_phy_match[] = {
+	{ .compatible = "qcom,usb-hsic-phy", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, qcom_usb_hsic_phy_match);
+
+static struct ulpi_driver qcom_usb_hsic_phy_driver = {
+	.probe = qcom_usb_hsic_phy_probe,
+	.driver = {
+		.name = "qcom_usb_hsic_phy",
+		.of_match_table = qcom_usb_hsic_phy_match
+	},
+};
+module_ulpi_driver(qcom_usb_hsic_phy_driver);
+
+MODULE_DESCRIPTION("Qualcomm USB HSIC phy");
+MODULE_LICENSE("GPL v2");
-- 
2.9.0.rc2.8.ga28705d

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 21/21] phy: Add support for Qualcomm's USB HS phy

[Stephen Boyd]

The high-speed phy on qcom SoCs is controlled via the ULPI
viewport.

Cc: Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>
Cc: <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Signed-off-by: Stephen Boyd <stephen.boyd-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 .../devicetree/bindings/phy/qcom,usb-hs-phy.txt    |  71 ++++++
 drivers/phy/Kconfig                                |   8 +
 drivers/phy/Makefile                               |   1 +
 drivers/phy/phy-qcom-usb-hs.c                      | 283 +++++++++++++++++++++
 4 files changed, 363 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.txt
 create mode 100644 drivers/phy/phy-qcom-usb-hs.c

diff --git a/Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.txt b/Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.txt
new file mode 100644
index 000000000000..2bd22c53cee0
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.txt
@@ -0,0 +1,71 @@
+Qualcomm's USB HS PHY
+
+PROPERTIES
+
+- compatible:
+    Usage: required
+    Value type: <string>
+    Definition: Should contain "qcom,usb-hs-phy"
+
+- #phy-cells:
+    Usage: required
+    Value type: <u32>
+    Definition: Should contain 0
+
+- clocks:
+    Usage: required
+    Value type: <prop-encoded-array>
+    Definition: Should contain clock specifier for the reference and sleep
+                clocks
+
+- clock-names:
+    Usage: required
+    Value type: <stringlist>
+    Definition: Should contain "ref" and "sleep" for the reference and sleep
+                clocks respectively
+
+- resets:
+    Usage: required
+    Value type: <prop-encoded-array>
+    Definition: Should contain the phy and POR resets
+
+- reset-names:
+    Usage: required
+    Value type: <stringlist>
+    Definition: Should contain "phy" and "por" for the phy and POR resets
+                respectively
+
+- v3p3-supply:
+    Usage: required
+    Value type: <phandle>
+    Definition: Should contain a reference to the 3.3V supply
+
+- v1p8-supply:
+    Usage: required
+    Value type: <phandle>
+    Definition: Should contain a reference to the 1.8V supply
+
+- qcom,init-seq:
+    Usage: optional
+    Value type: <u8 array>
+    Definition: Should contain a sequence of ULPI register and address pairs to
+                program into the ULPI_EXT_VENDOR_SPECIFIC area. This is related
+                to Device Mode Eye Diagram test.
+
+EXAMPLE
+
+otg: usb-controller {
+	ulpi {
+		phy {
+			compatible = "qcom,usb-hs-phy";
+			#phy-cells = <0>;
+			clocks = <&xo_board>, <&gcc GCC_USB2A_PHY_SLEEP_CLK>;
+			clock-names = "ref", "sleep";
+			resets = <&gcc GCC_USB2A_PHY_BCR>, <&otg 0>;
+			reset-names = "phy", "por";
+			v3p3-supply = <&pm8941_l24>;
+			v1p8-supply = <&pm8941_l6>;
+			qcom,init-seq = /bits/ 8 <0x81 0x63>;
+		};
+	};
+};
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index a2866949dc97..cfb3ded0896d 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -406,6 +406,14 @@ config PHY_QCOM_UFS
 	help
 	  Support for UFS PHY on QCOM chipsets.
 
+config PHY_QCOM_USB_HS
+	tristate "Qualcomm USB HS PHY module"
+	depends on USB_ULPI_BUS
+	select GENERIC_PHY
+	help
+	  Support for the USB high-speed ULPI compliant phy on Qualcomm
+	  chipsets.
+
 config PHY_QCOM_USB_HSIC
 	tristate "Qualcomm USB HSIC ULPI PHY module"
 	depends on USB_ULPI_BUS
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 982e84a290ec..21435fc0b656 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -49,6 +49,7 @@ obj-$(CONFIG_PHY_STIH41X_USB)		+= phy-stih41x-usb.o
 obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs.o
 obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs-qmp-20nm.o
 obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs-qmp-14nm.o
+obj-$(CONFIG_PHY_QCOM_USB_HS) 		+= phy-qcom-usb-hs.o
 obj-$(CONFIG_PHY_QCOM_USB_HSIC) 	+= phy-qcom-usb-hsic.o
 obj-$(CONFIG_PHY_TUSB1210)		+= phy-tusb1210.o
 obj-$(CONFIG_PHY_BRCM_SATA)		+= phy-brcm-sata.o
diff --git a/drivers/phy/phy-qcom-usb-hs.c b/drivers/phy/phy-qcom-usb-hs.c
new file mode 100644
index 000000000000..8be83100ecd9
--- /dev/null
+++ b/drivers/phy/phy-qcom-usb-hs.c
@@ -0,0 +1,283 @@
+/**
+ * Copyright (C) 2016 Linaro Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/module.h>
+#include <linux/ulpi/driver.h>
+#include <linux/ulpi/regs.h>
+#include <linux/clk.h>
+#include <linux/regulator/consumer.h>
+#include <linux/of_device.h>
+#include <linux/reset.h>
+#include <linux/extcon.h>
+#include <linux/notifier.h>
+#include <linux/usb/of.h>
+
+#include "ulpi_phy.h"
+
+#define ULPI_PWR_CLK_MNG_REG		0x88
+# define ULPI_PWR_OTG_COMP_DISABLE	BIT(0)
+
+#define ULPI_MISC_A			0x96
+# define ULPI_MISC_A_VBUSVLDEXTSEL	BIT(1)
+# define ULPI_MISC_A_VBUSVLDEXT		BIT(0)
+
+
+struct ulpi_seq {
+	u8 addr;
+	u8 val;
+};
+
+struct qcom_usb_hs_phy {
+	struct ulpi *ulpi;
+	struct phy *phy;
+	struct clk *ref_clk;
+	struct clk *sleep_clk;
+	struct regulator *v1p8;
+	struct regulator *v3p3;
+	struct reset_control *reset;
+	struct ulpi_seq *init_seq;
+	struct notifier_block vbus_notify;
+	struct extcon_dev *vbus_edev;
+	struct extcon_dev *id_edev;
+	enum usb_dr_mode dr_mode;
+};
+
+static int
+qcom_usb_hs_phy_vbus_notifier(struct notifier_block *nb, unsigned long event,
+			      void *ptr)
+{
+	struct qcom_usb_hs_phy *uphy;
+	int is_host;
+	u8 addr;
+
+	uphy = container_of(nb, struct qcom_usb_hs_phy, vbus_notify);
+	is_host = extcon_get_cable_state_(uphy->id_edev, EXTCON_USB_HOST);
+	if (is_host < 0)
+		is_host = 0; /* No id event means always a peripheral */
+
+	if (event && !is_host)
+		addr = ULPI_SET(ULPI_MISC_A);
+	else
+		addr = ULPI_CLR(ULPI_MISC_A);
+
+	return ulpi_write(uphy->ulpi, addr,
+			  ULPI_MISC_A_VBUSVLDEXTSEL | ULPI_MISC_A_VBUSVLDEXT);
+}
+
+static int qcom_usb_hs_phy_power_on(struct phy *phy)
+{
+	struct qcom_usb_hs_phy *uphy = phy_get_drvdata(phy);
+	struct ulpi *ulpi = uphy->ulpi;
+	const struct ulpi_seq *seq;
+	int ret, state;
+
+	ret = clk_prepare_enable(uphy->ref_clk);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(uphy->sleep_clk);
+	if (ret)
+		goto err_sleep;
+
+	ret = regulator_set_voltage(uphy->v1p8, 1800000, 1800000);
+	if (ret)
+		goto err_1p8;
+	ret = regulator_set_load(uphy->v1p8, 50000);
+	if (ret < 0)
+		goto err_1p8;
+
+	ret = regulator_enable(uphy->v1p8);
+	if (ret)
+		goto err_1p8;
+
+	ret = regulator_set_voltage_triplet(uphy->v3p3, 3050000, 3300000,
+					    3300000);
+	if (ret)
+		goto err_3p3;
+
+	ret = regulator_set_load(uphy->v3p3, 50000);
+	if (ret < 0)
+		goto err_3p3;
+
+	ret = regulator_enable(uphy->v3p3);
+	if (ret)
+		goto err_3p3;
+
+	for (seq = uphy->init_seq; seq->addr; seq++) {
+		ret = ulpi_write(ulpi, seq->addr, seq->val);
+		if (ret)
+			goto err_ulpi;
+	}
+
+	if (uphy->reset) {
+		ret = reset_control_reset(uphy->reset);
+		if (ret)
+			goto err_ulpi;
+	}
+
+	if (uphy->vbus_edev) {
+		ulpi_write(ulpi, ULPI_SET(ULPI_PWR_CLK_MNG_REG),
+			   ULPI_PWR_OTG_COMP_DISABLE);
+		state = extcon_get_cable_state_(uphy->vbus_edev, EXTCON_USB);
+		/* setup initial state */
+		qcom_usb_hs_phy_vbus_notifier(&uphy->vbus_notify, state,
+					      uphy->vbus_edev);
+	} else {
+		u8 val;
+
+		switch (uphy->dr_mode) {
+		case USB_DR_MODE_OTG:
+			val = ULPI_INT_IDGRD;
+		case USB_DR_MODE_PERIPHERAL:
+			val |= ULPI_INT_SESS_VALID;
+			break;
+		default:
+			val = 0;
+		}
+
+		ret = ulpi_write(ulpi, ULPI_USB_INT_EN_RISE, val);
+		if (ret)
+			goto err_ulpi;
+		ret = ulpi_write(ulpi, ULPI_USB_INT_EN_FALL, val);
+		if (ret)
+			goto err_ulpi;
+	}
+
+	return 0;
+err_ulpi:
+	regulator_disable(uphy->v3p3);
+err_3p3:
+	regulator_disable(uphy->v1p8);
+err_1p8:
+	clk_disable_unprepare(uphy->sleep_clk);
+err_sleep:
+	clk_disable_unprepare(uphy->ref_clk);
+	return ret;
+}
+
+static int qcom_usb_hs_phy_power_off(struct phy *phy)
+{
+	struct qcom_usb_hs_phy *uphy = phy_get_drvdata(phy);
+
+	regulator_disable(uphy->v3p3);
+	regulator_disable(uphy->v1p8);
+	clk_disable_unprepare(uphy->sleep_clk);
+	clk_disable_unprepare(uphy->ref_clk);
+
+	return 0;
+}
+
+static const struct phy_ops qcom_usb_hs_phy_ops = {
+	.power_on = qcom_usb_hs_phy_power_on,
+	.power_off = qcom_usb_hs_phy_power_off,
+	.owner = THIS_MODULE,
+};
+
+static int qcom_usb_hs_phy_probe(struct ulpi *ulpi)
+{
+	struct qcom_usb_hs_phy *uphy;
+	struct phy_provider *p;
+	struct clk *clk;
+	struct regulator *reg;
+	struct reset_control *reset;
+	int size;
+	int ret;
+
+	uphy = devm_kzalloc(&ulpi->dev, sizeof(*uphy), GFP_KERNEL);
+	if (!uphy)
+		return -ENOMEM;
+	ulpi_set_drvdata(ulpi, uphy);
+	uphy->ulpi = ulpi;
+	uphy->dr_mode = of_usb_get_dr_mode_by_phy(ulpi->dev.of_node);
+
+	size = of_property_count_u8_elems(ulpi->dev.of_node, "qcom,init-seq");
+	if (size < 0)
+		size = 0;
+	uphy->init_seq = devm_kmalloc_array(&ulpi->dev, (size / 2) + 1,
+					   sizeof(*uphy->init_seq), GFP_KERNEL);
+	if (!uphy->init_seq)
+		return -ENOMEM;
+	ret = of_property_read_u8_array(ulpi->dev.of_node, "qcom,init-seq",
+					(u8 *)uphy->init_seq, size);
+	if (ret && size)
+		return ret;
+	/* NUL terminate */
+	uphy->init_seq[size / 2].addr = uphy->init_seq[size / 2].val = 0;
+
+	uphy->ref_clk = clk = devm_clk_get(&ulpi->dev, "ref");
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	uphy->sleep_clk = clk = devm_clk_get(&ulpi->dev, "sleep");
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	uphy->v1p8 = reg = devm_regulator_get(&ulpi->dev, "v1p8");
+	if (IS_ERR(reg))
+		return PTR_ERR(reg);
+
+	uphy->v3p3 = reg = devm_regulator_get(&ulpi->dev, "v3p3");
+	if (IS_ERR(reg))
+		return PTR_ERR(reg);
+
+	uphy->reset = reset = devm_reset_control_get(&ulpi->dev, "por");
+	if (IS_ERR(reset)) {
+		if (PTR_ERR(reset) == -EPROBE_DEFER)
+			return PTR_ERR(reset);
+		uphy->reset = NULL;
+	}
+
+	uphy->phy = devm_phy_create(&ulpi->dev, ulpi->dev.of_node,
+				    &qcom_usb_hs_phy_ops);
+	if (IS_ERR(uphy->phy))
+		return PTR_ERR(uphy->phy);
+
+	uphy->vbus_edev = extcon_get_edev_by_phandle(&ulpi->dev, 0);
+	if (IS_ERR(uphy->vbus_edev)) {
+		if (PTR_ERR(uphy->vbus_edev) != -ENODEV)
+			return PTR_ERR(uphy->vbus_edev);
+		uphy->vbus_edev = NULL;
+	}
+
+	uphy->id_edev = extcon_get_edev_by_phandle(&ulpi->dev, 1);
+	if (IS_ERR(uphy->id_edev)) {
+		if (PTR_ERR(uphy->id_edev) != -ENODEV)
+			return PTR_ERR(uphy->id_edev);
+		uphy->id_edev = NULL;
+	}
+
+	if (uphy->vbus_edev) {
+		uphy->vbus_notify.notifier_call = qcom_usb_hs_phy_vbus_notifier;
+		ret = extcon_register_notifier(uphy->vbus_edev, EXTCON_USB,
+						&uphy->vbus_notify);
+		if (ret)
+			return ret;
+	}
+
+	phy_set_drvdata(uphy->phy, uphy);
+
+	p = devm_of_phy_provider_register(&ulpi->dev, of_phy_simple_xlate);
+	return PTR_ERR_OR_ZERO(p);
+}
+
+static const struct of_device_id qcom_usb_hs_phy_match[] = {
+	{ .compatible = "qcom,usb-hs-phy", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, qcom_usb_hs_phy_match);
+
+static struct ulpi_driver qcom_usb_hs_phy_driver = {
+	.probe = qcom_usb_hs_phy_probe,
+	.driver = {
+		.name = "qcom_usb_hs_phy",
+		.of_match_table = qcom_usb_hs_phy_match
+	},
+};
+module_ulpi_driver(qcom_usb_hs_phy_driver);
+
+MODULE_DESCRIPTION("Qualcomm USB HS phy");
+MODULE_LICENSE("GPL v2");
-- 
2.9.0.rc2.8.ga28705d

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/21] usb: ulpi: Support device discovery via DT

[kbuild test robot]

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

Hi,

[auto build test ERROR on peter.chen-usb/ci-for-usb-next]
[also build test ERROR on v4.7-rc5 next-20160624]
[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/Stephen-Boyd/Support-qcom-s-HSIC-USB-and-rewrite-USB2-HS-phy-support/20160627-102637
base:   https://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb ci-for-usb-next
config: x86_64-randconfig-h0-06270614 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

>> ERROR: "of_device_request_module" [drivers/usb/common/ulpi.ko] undefined!
>> ERROR: "of_device_get_modalias" [drivers/usb/common/ulpi.ko] undefined!
>> ERROR: "of_device_uevent_modalias" [drivers/usb/common/ulpi.ko] undefined!

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

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 22307 bytes --]

Re: [PATCH 02/21] usb: ulpi: Support device discovery via DT

[Heikki Krogerus]

Hi,

I'm fine with most of the patch, except..

On Sun, Jun 26, 2016 at 12:28:19AM -0700, Stephen Boyd wrote:
> @@ -39,7 +42,10 @@ static int ulpi_match(struct device *dev, struct device_driver *driver)
>  	struct ulpi *ulpi = to_ulpi_dev(dev);
>  	const struct ulpi_device_id *id;
>  
> -	for (id = drv->id_table; id->vendor; id++)
> +	if (of_driver_match_device(dev, driver))
> +		return 1;

I don't like this part. We should match separately like that only
if the bus does not support native enumeration, and of course ULPI
with its vendor and product IDs does. There really should always be
IDs to match with here. So exceptions have to be solved before we
attempt matching.

Since we also have to support platforms where the PHY is initially
powered off and reading the IDs from the registers is not possible
because of that, I think we should consider getting the product and
vendor IDs optionally from device properties. Something like this:


diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index 01c0c04..6228a85 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -152,7 +152,7 @@ EXPORT_SYMBOL_GPL(ulpi_unregister_driver);
 
 /* -------------------------------------------------------------------------- */
 
-static int ulpi_register(struct device *dev, struct ulpi *ulpi)
+static int ulpi_read_id(struct ulpi *ulpi)
 {
        int ret;
 
@@ -174,6 +174,21 @@ static int ulpi_register(struct device *dev, struct ulpi *ulpi)
        ulpi->id.product = ulpi_read(ulpi, ULPI_PRODUCT_ID_LOW);
        ulpi->id.product |= ulpi_read(ulpi, ULPI_PRODUCT_ID_HIGH) << 8;
 
+       return 0;
+}
+
+static int ulpi_register(struct device *dev, struct ulpi *ulpi)
+{
+       int ret;
+
+       ret = device_property_read_u16(dev, "ulpi-vendor", &ulpi->id.vendor);
+       ret |= device_property_read_u16(dev, "ulpi-product", &ulpi->id.product);
+       if (ret) {
+               ret = ulpi_read_id(ulpi);
+               if (ret)
+                       return ret;
+       }
+
        ulpi->dev.parent = dev;
        ulpi->dev.bus = &ulpi_bus;
        ulpi->dev.type = &ulpi_dev_type;


That should cover both cases. You would just have to create the IDs
yourself in this case.


Thanks,

-- 
heikki

Re: [PATCH 02/21] usb: ulpi: Support device discovery via DT

[Stephen Boyd]

Quoting Heikki Krogerus (2016-06-27 07:34:22)
> Hi,
> 
> I'm fine with most of the patch, except..
> 
> On Sun, Jun 26, 2016 at 12:28:19AM -0700, Stephen Boyd wrote:
> > @@ -39,7 +42,10 @@ static int ulpi_match(struct device *dev, struct device_driver *driver)
> >       struct ulpi *ulpi = to_ulpi_dev(dev);
> >       const struct ulpi_device_id *id;
> >  
> > -     for (id = drv->id_table; id->vendor; id++)
> > +     if (of_driver_match_device(dev, driver))
> > +             return 1;
> 
> I don't like this part. We should match separately like that only
> if the bus does not support native enumeration, and of course ULPI
> with its vendor and product IDs does. There really should always be
> IDs to match with here. So exceptions have to be solved before we
> attempt matching.
> 
> Since we also have to support platforms where the PHY is initially
> powered off and reading the IDs from the registers is not possible
> because of that, I think we should consider getting the product and
> vendor IDs optionally from device properties. Something like this:

Ok, I'm a little worried about conflating the powered off problem with
this product/vendor ID missing problem. But if you're ok with that I'll
combine the two patches into one using your approach below.

> 
> 
> diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
> index 01c0c04..6228a85 100644
> --- a/drivers/usb/common/ulpi.c
> +++ b/drivers/usb/common/ulpi.c
> @@ -152,7 +152,7 @@ EXPORT_SYMBOL_GPL(ulpi_unregister_driver);
>  
>  /* -------------------------------------------------------------------------- */
>  
> -static int ulpi_register(struct device *dev, struct ulpi *ulpi)
> +static int ulpi_read_id(struct ulpi *ulpi)
>  {
>         int ret;
>  
> @@ -174,6 +174,21 @@ static int ulpi_register(struct device *dev, struct ulpi *ulpi)
>         ulpi->id.product = ulpi_read(ulpi, ULPI_PRODUCT_ID_LOW);
>         ulpi->id.product |= ulpi_read(ulpi, ULPI_PRODUCT_ID_HIGH) << 8;
>  
> +       return 0;
> +}
> +
> +static int ulpi_register(struct device *dev, struct ulpi *ulpi)
> +{
> +       int ret;
> +
> +       ret = device_property_read_u16(dev, "ulpi-vendor", &ulpi->id.vendor);
> +       ret |= device_property_read_u16(dev, "ulpi-product", &ulpi->id.product);
> +       if (ret) {
> +               ret = ulpi_read_id(ulpi);
> +               if (ret)
> +                       return ret;
> +       }
> +
>         ulpi->dev.parent = dev;
>         ulpi->dev.bus = &ulpi_bus;
>         ulpi->dev.type = &ulpi_dev_type;
> 
> 
> That should cover both cases. You would just have to create the IDs
> yourself in this case.
> 

Right, I would have to make up some IDs in this case. I suppose I can
use the qcom vendor ID 0x05c6 and then product ids 0 and 1 for HS phy
and HSIC phy? That doesn't make me feel great because it's all made up,
but I guess there's no other option. I hope they don't decide to start
populating these ids in the future though and then we may have
conflicting product ids. If that happens I suppose we can do a
workaround based on compatible strings in the DT node. Fun!

Nice side effect of all that is I can drop requesting the module by DT
aliases and things become simpler. I'll try this out.

Re: [PATCH 00/21] Support qcom's HSIC USB and rewrite USB2 HS phy support

[John Stultz]

On Sun, Jun 26, 2016 at 12:28 AM, Stephen Boyd <stephen.boyd-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> The state of USB ChipIdea support on Qualcomm's platforms is not great.
> The DT description of these devices requires up to three different nodes
> for what amounts to be the same hardware block, when there should really
> only be one. Furthermore, the "phy" driver that is in mainline (phy-msm-usb.c)
> duplicates the OTG state machine and touches the ci controller wrapper
> registers when it should really be focused on the phy and the ULPI accesses
> needed to get the phy working. There's also a slimmed down phy driver for
> the msm8916 platform, but really the phy hardware is the same as other MSMs,
> so we have two drivers doing pretty much the same thing. This leads to a
> situtaion where we have the chipidea core driver, the "phy" driver, and
> sometimes the ehci-msm.c driver operating the same device all at the same
> time with very little coordination. This just isn't very safe and is
> confusing from a driver perspective when trying to figure out who does what.
> Finally, there isn't any HSIC support on platforms like apq8074 so we
> should add that.
>
> This patch series updates the ChipIdea driver and the MSM wrapper
> (ci_hdrc_msm.c) to properly handle the PHY and wrapper bits at the right
> times in the right places. To get there, we update the ChipIdea core to
> have support for the ULPI phy bus introduced by Heikki. Along the way
> we fix bugs with the extcon handling for peripheral and OTG mode controllers
> and move the parts of phy-usb-msm.c that are touching the CI controller
> wrapper into the wrapper driver (ci_hdrc_msm.c). Finally we add support
> for the HSIC phy based on the ULPI bus and rewrite the HS phy driver
> (phy-usb-msm.c) as a standard ULPI phy driver.
>
> Once this series is accepted, we should be able to delete the phy-usb-msm.c
> phy-qcom-8x16-usb.c, and ehci-msm.c drivers from the tree and use the ULPI
> based phy driver (which also lives in drivers/phy/ instead of drivers/usb/phy/)
> and the chipidea host core instead.
>
> I've also sent seperate patches for other minor pieces to make this
> all work. The full tree can be found here[3], hacks and all to get
> things working. I've tested this on the db410c, apq8074 dragonboard,
> and ifc6410 with configfs gadgets and otg cables.
...
> [3] https://git.linaro.org/people/stephen.boyd/linux.git/shortlog/refs/heads/usb-hsic-8074

Very excited to see this moving upstream!

Just a heads up, trying to build with this branch gives me:

drivers/usb/Kconfig:39:error: recursive dependency detected!
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/usb/Kconfig:39: symbol USB is selected by MOUSE_APPLETOUCH
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/input/mouse/Kconfig:187:        symbol MOUSE_APPLETOUCH depends on INPUT
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/input/Kconfig:8:        symbol INPUT is selected by VT
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/tty/Kconfig:12: symbol VT is selected by FB_STI
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/video/fbdev/Kconfig:674:        symbol FB_STI depends on FB
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/video/fbdev/Kconfig:5:  symbol FB is selected by DRM_KMS_FB_HELPER
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/gpu/drm/Kconfig:42:     symbol DRM_KMS_FB_HELPER is selected
by DRM_KMS_CMA_HELPER
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/gpu/drm/Kconfig:98:     symbol DRM_KMS_CMA_HELPER is selected by DRM_IMX
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/gpu/drm/imx/Kconfig:1:  symbol DRM_IMX depends on IMX_IPUV3_CORE
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/gpu/ipu-v3/Kconfig:1:   symbol IMX_IPUV3_CORE depends on
RESET_CONTROLLER
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/reset/Kconfig:4:        symbol RESET_CONTROLLER is selected by
USB_CHIPIDEA
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/usb/chipidea/Kconfig:1: symbol USB_CHIPIDEA depends on USB_EHCI_HCD
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/usb/host/Kconfig:84:    symbol USB_EHCI_HCD depends on USB
drivers/usb/chipidea/otg.c: In function ‘hw_write_otgsc’:
drivers/usb/chipidea/otg.c:120:2: warning: format ‘%x’ expects
argument of type ‘unsigned int’, but argument 3 has type ‘long
unsigned int’ [-Wformat]
drivers/usb/chipidea/otg.c:120:2: warning: format ‘%x’ expects
argument of type ‘unsigned int’, but argument 4 has type ‘long
unsigned int’ [-Wformat]

I haven't yet been able to test with this, as I need some other fixes
it seems too to deal with some of the iommu changes in my flo-WIP tree
(it can't find of_dma_configure), but will let you know how things
work once I have all that sorted.

thanks
-john
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/21] of: device: Support loading a module with OF based modalias

[Bjorn Andersson]

On Sun 26 Jun 00:28 PDT 2016, Stephen Boyd wrote:

> In the case of ULPI devices, we want to be able to load the
> driver before registering the device so that we don't get stuck
> in a loop waiting for the phy module to appear and failing usb
> controller probe. Currently we request the ulpi module via the
> ulpi ids, but in the DT case we might need to request it with the
> OF based modalias instead. Add a common function that allows
> anyone to request a module with the OF based modalias.
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: <devicetree@vger.kernel.org>
> Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> ---
>  drivers/of/device.c       | 50 +++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/of_device.h |  6 ++++++
>  2 files changed, 56 insertions(+)
> 
> diff --git a/drivers/of/device.c b/drivers/of/device.c
> index fd5cfad7c403..f275e5beb736 100644
> --- a/drivers/of/device.c
> +++ b/drivers/of/device.c
> @@ -226,6 +226,56 @@ ssize_t of_device_get_modalias(struct device *dev, char *str, ssize_t len)
>  	return tsize;
>  }
>  
> +static ssize_t of_device_modalias_size(struct device *dev)
> +{
> +	const char *compat;
> +	int cplen, i;
> +	ssize_t csize;
> +
> +	if ((!dev) || (!dev->of_node))
> +		return -ENODEV;
> +
> +	/* Name & Type */
> +	csize = 5 + strlen(dev->of_node->name) + strlen(dev->of_node->type);

It would be clearer if you replaced 5 with strlen("of:NT"), but...

> +
> +	/* Get compatible property if any */
> +	compat = of_get_property(dev->of_node, "compatible", &cplen);
> +	if (!compat)
> +		return csize;
> +
> +	/* Find true end (we tolerate multiple \0 at the end */
> +	for (i = (cplen - 1); i >= 0 && !compat[i]; i--)
> +		cplen--;
> +	if (!cplen)
> +		return csize;
> +	cplen++;
> +
> +	/* Check space (need cplen+1 chars including final \0) */
> +	return csize + cplen;
> +}

...if I understand of_device_get_modalias() correctly you should be able
to replace this function with:

  size = of_device_get_modalias(dev, NULL, 0);

snprintf() will not write to NULL, csize will be larger than 0 so tsize
will be returned before it will memcpy() to the buffer.

> +
> +int of_device_request_module(struct device *dev)
> +{
> +	char *str;
> +	ssize_t size;
> +	int ret;
> +
> +	size = of_device_modalias_size(dev);
> +	if (size < 0)
> +		return size;
> +
> +	str = kmalloc(size + 1, GFP_KERNEL);
> +	if (!str)
> +		return -ENOMEM;
> +
> +	of_device_get_modalias(dev, str, size);
> +	str[size] = '\0';
> +	ret = request_module(str);
> +	kfree(str);
> +
> +	return ret;
> +}
> +

Regards,
Bjorn

Re: [PATCH 01/21] of: device: Support loading a module with OF based modalias

[Rob Herring]

On Mon, Jun 27, 2016 at 11:17 PM, Bjorn Andersson <bjorn.andersson@linaro.org> wrote:
> On Sun 26 Jun 00:28 PDT 2016, Stephen Boyd wrote:
>
>> In the case of ULPI devices, we want to be able to load the
>> driver before registering the device so that we don't get stuck
>> in a loop waiting for the phy module to appear and failing usb
>> controller probe. Currently we request the ulpi module via the
>> ulpi ids, but in the DT case we might need to request it with the
>> OF based modalias instead. Add a common function that allows
>> anyone to request a module with the OF based modalias.
>>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: <devicetree@vger.kernel.org>
>> Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
>> ---
>>  drivers/of/device.c       | 50 +++++++++++++++++++++++++++++++++++++++++++++++
>>  include/linux/of_device.h |  6 ++++++
>>  2 files changed, 56 insertions(+)
>>
>> diff --git a/drivers/of/device.c b/drivers/of/device.c
>> index fd5cfad7c403..f275e5beb736 100644
>> --- a/drivers/of/device.c
>> +++ b/drivers/of/device.c
>> @@ -226,6 +226,56 @@ ssize_t of_device_get_modalias(struct device *dev, char *str, ssize_t len)
>>       return tsize;
>>  }
>>
>> +static ssize_t of_device_modalias_size(struct device *dev)
>> +{
>> +     const char *compat;
>> +     int cplen, i;
>> +     ssize_t csize;
>> +
>> +     if ((!dev) || (!dev->of_node))
>> +             return -ENODEV;
>> +
>> +     /* Name & Type */
>> +     csize = 5 + strlen(dev->of_node->name) + strlen(dev->of_node->type);
>
> It would be clearer if you replaced 5 with strlen("of:NT"), but...

Is the compiler smart enough to replace that with a constant 5? At least a comment of where 5 comes from as I was wondering.

>> +
>> +     /* Get compatible property if any */
>> +     compat = of_get_property(dev->of_node, "compatible", &cplen);
>> +     if (!compat)
>> +             return csize;
>> +
>> +     /* Find true end (we tolerate multiple \0 at the end */
>> +     for (i = (cplen - 1); i >= 0 && !compat[i]; i--)
>> +             cplen--;
>> +     if (!cplen)
>> +             return csize;
>> +     cplen++;
>> +
>> +     /* Check space (need cplen+1 chars including final \0) */
>> +     return csize + cplen;
>> +}
>
> ...if I understand of_device_get_modalias() correctly you should be able
> to replace this function with:
>
>   size = of_device_get_modalias(dev, NULL, 0);
>
> snprintf() will not write to NULL, csize will be larger than 0 so tsize
> will be returned before it will memcpy() to the buffer.
>
>> +
>> +int of_device_request_module(struct device *dev)
>> +{
>> +     char *str;
>> +     ssize_t size;
>> +     int ret;
>> +
>> +     size = of_device_modalias_size(dev);
>> +     if (size < 0)
>> +             return size;
>> +
>> +     str = kmalloc(size + 1, GFP_KERNEL);
>> +     if (!str)
>> +             return -ENOMEM;
>> +
>> +     of_device_get_modalias(dev, str, size);
>> +     str[size] = '\0';
>> +     ret = request_module(str);
>> +     kfree(str);
>> +
>> +     return ret;
>> +}
>> +
>
> Regards,
> Bjorn

Re: [PATCH 00/21] Support qcom's HSIC USB and rewrite USB2 HS phy support

[Stephen Boyd]

Quoting John Stultz (2016-06-27 20:09:30)
> 
> Just a heads up, trying to build with this branch gives me:
> 
> drivers/usb/Kconfig:39:error: recursive dependency detected!
> For a resolution refer to Documentation/kbuild/kconfig-language.txt
> subsection "Kconfig recursive dependency limitations"
> drivers/usb/Kconfig:39: symbol USB is selected by MOUSE_APPLETOUCH
> For a resolution refer to Documentation/kbuild/kconfig-language.txt
> subsection "Kconfig recursive dependency limitations"
> drivers/input/mouse/Kconfig:187:        symbol MOUSE_APPLETOUCH depends on INPUT
> For a resolution refer to Documentation/kbuild/kconfig-language.txt
> subsection "Kconfig recursive dependency limitations"
> drivers/input/Kconfig:8:        symbol INPUT is selected by VT
> For a resolution refer to Documentation/kbuild/kconfig-language.txt
> subsection "Kconfig recursive dependency limitations"
> drivers/tty/Kconfig:12: symbol VT is selected by FB_STI
> For a resolution refer to Documentation/kbuild/kconfig-language.txt
> subsection "Kconfig recursive dependency limitations"
> drivers/video/fbdev/Kconfig:674:        symbol FB_STI depends on FB
> For a resolution refer to Documentation/kbuild/kconfig-language.txt
> subsection "Kconfig recursive dependency limitations"
> drivers/video/fbdev/Kconfig:5:  symbol FB is selected by DRM_KMS_FB_HELPER
> For a resolution refer to Documentation/kbuild/kconfig-language.txt
> subsection "Kconfig recursive dependency limitations"
> drivers/gpu/drm/Kconfig:42:     symbol DRM_KMS_FB_HELPER is selected
> by DRM_KMS_CMA_HELPER
> For a resolution refer to Documentation/kbuild/kconfig-language.txt
> subsection "Kconfig recursive dependency limitations"
> drivers/gpu/drm/Kconfig:98:     symbol DRM_KMS_CMA_HELPER is selected by DRM_IMX
> For a resolution refer to Documentation/kbuild/kconfig-language.txt
> subsection "Kconfig recursive dependency limitations"
> drivers/gpu/drm/imx/Kconfig:1:  symbol DRM_IMX depends on IMX_IPUV3_CORE
> For a resolution refer to Documentation/kbuild/kconfig-language.txt
> subsection "Kconfig recursive dependency limitations"
> drivers/gpu/ipu-v3/Kconfig:1:   symbol IMX_IPUV3_CORE depends on
> RESET_CONTROLLER
> For a resolution refer to Documentation/kbuild/kconfig-language.txt
> subsection "Kconfig recursive dependency limitations"
> drivers/reset/Kconfig:4:        symbol RESET_CONTROLLER is selected by
> USB_CHIPIDEA
> For a resolution refer to Documentation/kbuild/kconfig-language.txt
> subsection "Kconfig recursive dependency limitations"
> drivers/usb/chipidea/Kconfig:1: symbol USB_CHIPIDEA depends on USB_EHCI_HCD
> For a resolution refer to Documentation/kbuild/kconfig-language.txt
> subsection "Kconfig recursive dependency limitations"
> drivers/usb/host/Kconfig:84:    symbol USB_EHCI_HCD depends on USB

Yeah, sorry I've updated the branch today with fixes reported by kbuild
robot. This problem starts to happen once I start selecting
RESET_CONTROLLER from the chipidea Kconfig symbol. I've layered another
patch on top to fix the build errors I'm seeing, although I'm not sure
it's a great solution.

> drivers/usb/chipidea/otg.c: In function ‘hw_write_otgsc’:
> drivers/usb/chipidea/otg.c:120:2: warning: format ‘%x’ expects
> argument of type ‘unsigned int’, but argument 3 has type ‘long
> unsigned int’ [-Wformat]
> drivers/usb/chipidea/otg.c:120:2: warning: format ‘%x’ expects
> argument of type ‘unsigned int’, but argument 4 has type ‘long
> unsigned int’ [-Wformat]

These are debug print warnings. Nothing to see here...

> 
> I haven't yet been able to test with this, as I need some other fixes
> it seems too to deal with some of the iommu changes in my flo-WIP tree
> (it can't find of_dma_configure), but will let you know how things
> work once I have all that sorted.

Cool, thanks for testing.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 20/21] phy: Add support for Qualcomm's USB HSIC phy

[Neil Armstrong]

On 06/26/2016 09:28 AM, Stephen Boyd wrote:
> The HSIC USB controller on qcom SoCs has an integrated all
> digital phy controlled via the ULPI viewport.
> 
> Cc: Kishon Vijay Abraham I <kishon@ti.com>
> Cc: <devicetree@vger.kernel.org>
> Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> ---
>  .../devicetree/bindings/phy/qcom,usb-hsic-phy.txt  |  60 ++++++++
>  drivers/phy/Kconfig                                |   7 +
>  drivers/phy/Makefile                               |   1 +
>  drivers/phy/phy-qcom-usb-hsic.c                    | 161 +++++++++++++++++++++
>  4 files changed, 229 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/qcom,usb-hsic-phy.txt
>  create mode 100644 drivers/phy/phy-qcom-usb-hsic.c
> 
> diff --git a/Documentation/devicetree/bindings/phy/qcom,usb-hsic-phy.txt b/Documentation/devicetree/bindings/phy/qcom,usb-hsic-phy.txt
> new file mode 100644
> index 000000000000..6b1c6aad2962
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom,usb-hsic-phy.txt
> @@ -0,0 +1,60 @@
> +Qualcomm's USB HSIC PHY
> +
> +PROPERTIES
> +
> +- compatible:
> +    Usage: required
> +    Value type: <string>
> +    Definition: Should contain "qcom,usb-hsic-phy"
> +
> +- #phy-cells:
> +    Usage: required
> +    Value type: <u32>
> +    Definition: Should contain 0
> +
> +- clocks:
> +    Usage: required
> +    Value type: <prop-encoded-array>
> +    Definition: Should contain clock specifier for phy, calibration and
> +                optionally a calibration sleep clock
> +
> +- clock-names:
> +    Usage: required
> +    Value type: <stringlist>
> +    Definition: Should contain "phy, "cal" and optionally "cal_sleep"
> +

[...]

> +
> +static int qcom_usb_hsic_phy_power_on(struct phy *phy)
> +{
> +	struct qcom_usb_hsic_phy *uphy = phy_get_drvdata(phy);
> +	struct ulpi *ulpi = uphy->ulpi;
> +	struct pinctrl_state *pins_default;
> +	int ret;
> +
> +	ret = clk_prepare_enable(uphy->phy_clk);
> +	if (ret)
> +		return ret;
> +
> +	ret = clk_prepare_enable(uphy->cal_clk);
> +	if (ret)
> +		goto err_cal;
> +
> +	ret = clk_prepare_enable(uphy->cal_sleep_clk);
> +	if (ret)
> +		goto err_sleep;
> +

[...]

> +
> +	return ret;
> +err_ulpi:
> +	clk_disable_unprepare(uphy->cal_sleep_clk);
> +err_sleep:
> +	clk_disable_unprepare(uphy->cal_clk);
> +err_cal:
> +	clk_disable_unprepare(uphy->phy_clk);
> +	return ret;
> +}
> +
> +static int qcom_usb_hsic_phy_power_off(struct phy *phy)
> +{
> +	struct qcom_usb_hsic_phy *uphy = phy_get_drvdata(phy);
> +
> +	clk_disable_unprepare(uphy->cal_sleep_clk);
> +	clk_disable_unprepare(uphy->cal_clk);
> +	clk_disable_unprepare(uphy->phy_clk);

[...]

> +static int qcom_usb_hsic_phy_probe(struct ulpi *ulpi)
> +{
> +	struct qcom_usb_hsic_phy *uphy;
> +	struct phy_provider *p;
> +	struct clk *clk;
> +
> +	uphy = devm_kzalloc(&ulpi->dev, sizeof(*uphy), GFP_KERNEL);
> +	if (!uphy)
> +		return -ENOMEM;
> +	ulpi_set_drvdata(ulpi, uphy);
> +
> +	uphy->ulpi = ulpi;
> +	uphy->pctl = devm_pinctrl_get(&ulpi->dev);
> +	if (IS_ERR(uphy->pctl))
> +		return PTR_ERR(uphy->pctl);
> +
> +	uphy->phy_clk = clk = devm_clk_get(&ulpi->dev, "phy");
> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);
> +
> +	uphy->cal_clk = clk = devm_clk_get(&ulpi->dev, "cal");
> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);
> +
> +	uphy->cal_sleep_clk = clk = devm_clk_get(&ulpi->dev, "cal_sleep");
> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);

Hi Stephen,

In the bindings the cal_sleep is marked optional, and I think should be since AFAIK
it's not present on MDM9615 for example.

Also MDM9615 HSIC requires "core", "alt-core", "phy", "cal" and "iface" clocks.
I assume "core" can be attributed to the main chipidea node, but I think "alt-core" and "iface" should be also optionnal.

Finally, it misses an optional reset line AFAIK mandatory on MDM9615.

Neil

> +
> +	uphy->phy = devm_phy_create(&ulpi->dev, ulpi->dev.of_node,
> +				    &qcom_usb_hsic_phy_ops);
> +	if (IS_ERR(uphy->phy))
> +		return PTR_ERR(uphy->phy);
> +	phy_set_drvdata(uphy->phy, uphy);
> +
> +	p = devm_of_phy_provider_register(&ulpi->dev, of_phy_simple_xlate);
> +	return PTR_ERR_OR_ZERO(p);
> +}
> +
> +
> +static const struct of_device_id qcom_usb_hsic_phy_match[] = {
> +	{ .compatible = "qcom,usb-hsic-phy", },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, qcom_usb_hsic_phy_match);
> +
> +static struct ulpi_driver qcom_usb_hsic_phy_driver = {
> +	.probe = qcom_usb_hsic_phy_probe,
> +	.driver = {
> +		.name = "qcom_usb_hsic_phy",
> +		.of_match_table = qcom_usb_hsic_phy_match
> +	},
> +};
> +module_ulpi_driver(qcom_usb_hsic_phy_driver);
> +
> +MODULE_DESCRIPTION("Qualcomm USB HSIC phy");
> +MODULE_LICENSE("GPL v2");
> 

Re: [PATCH 02/21] usb: ulpi: Support device discovery via DT

[Heikki Krogerus]

On Mon, Jun 27, 2016 at 03:10:40PM -0700, Stephen Boyd wrote:
> Quoting Heikki Krogerus (2016-06-27 07:34:22)
> > Hi,
> > 
> > I'm fine with most of the patch, except..
> > 
> > On Sun, Jun 26, 2016 at 12:28:19AM -0700, Stephen Boyd wrote:
> > > @@ -39,7 +42,10 @@ static int ulpi_match(struct device *dev, struct device_driver *driver)
> > >       struct ulpi *ulpi = to_ulpi_dev(dev);
> > >       const struct ulpi_device_id *id;
> > >  
> > > -     for (id = drv->id_table; id->vendor; id++)
> > > +     if (of_driver_match_device(dev, driver))
> > > +             return 1;
> > 
> > I don't like this part. We should match separately like that only
> > if the bus does not support native enumeration, and of course ULPI
> > with its vendor and product IDs does. There really should always be
> > IDs to match with here. So exceptions have to be solved before we
> > attempt matching.
> > 
> > Since we also have to support platforms where the PHY is initially
> > powered off and reading the IDs from the registers is not possible
> > because of that, I think we should consider getting the product and
> > vendor IDs optionally from device properties. Something like this:
> 
> Ok, I'm a little worried about conflating the powered off problem with
> this product/vendor ID missing problem. But if you're ok with that I'll
> combine the two patches into one using your approach below.
> 
> > 
> > 
> > diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
> > index 01c0c04..6228a85 100644
> > --- a/drivers/usb/common/ulpi.c
> > +++ b/drivers/usb/common/ulpi.c
> > @@ -152,7 +152,7 @@ EXPORT_SYMBOL_GPL(ulpi_unregister_driver);
> >  
> >  /* -------------------------------------------------------------------------- */
> >  
> > -static int ulpi_register(struct device *dev, struct ulpi *ulpi)
> > +static int ulpi_read_id(struct ulpi *ulpi)
> >  {
> >         int ret;
> >  
> > @@ -174,6 +174,21 @@ static int ulpi_register(struct device *dev, struct ulpi *ulpi)
> >         ulpi->id.product = ulpi_read(ulpi, ULPI_PRODUCT_ID_LOW);
> >         ulpi->id.product |= ulpi_read(ulpi, ULPI_PRODUCT_ID_HIGH) << 8;
> >  
> > +       return 0;
> > +}
> > +
> > +static int ulpi_register(struct device *dev, struct ulpi *ulpi)
> > +{
> > +       int ret;
> > +
> > +       ret = device_property_read_u16(dev, "ulpi-vendor", &ulpi->id.vendor);
> > +       ret |= device_property_read_u16(dev, "ulpi-product", &ulpi->id.product);
> > +       if (ret) {
> > +               ret = ulpi_read_id(ulpi);
> > +               if (ret)
> > +                       return ret;
> > +       }
> > +
> >         ulpi->dev.parent = dev;
> >         ulpi->dev.bus = &ulpi_bus;
> >         ulpi->dev.type = &ulpi_dev_type;
> > 
> > 
> > That should cover both cases. You would just have to create the IDs
> > yourself in this case.
> > 
> 
> Right, I would have to make up some IDs in this case. I suppose I can
> use the qcom vendor ID 0x05c6 and then product ids 0 and 1 for HS phy
> and HSIC phy? That doesn't make me feel great because it's all made up,
> but I guess there's no other option. I hope they don't decide to start
> populating these ids in the future though and then we may have
> conflicting product ids. If that happens I suppose we can do a
> workaround based on compatible strings in the DT node. Fun!
> 
> Nice side effect of all that is I can drop requesting the module by DT
> aliases and things become simpler. I'll try this out.

I was hoping that we could manage with product id 0 as an exception (I
failed to consider that you have multiple PHYs to deal with). I don't
think we can just come up with product id > 0.

I guess we should have the of_driver_match_device() call after all.
Let's just call it conditionally, only in cases where there is no
product ID, to make me feel a bit more better. I don't want to make it
too easy to use.

The properties for the vendor and product ID are still something that
we need to introduce in any case. We have the powered off problem on
all kinds of platforms, and not all of them use DT. Please feel free
to incorporate the diff into the patch you had for the powered off
case if you are OK with it. So I think in your case you would just
need to addthe correct ulpi-vendor id 0x05c6 and ulpi-product id 0 to
the chipidea device node, and I think this would work.

Sorry about the hassle.


Thanks,

-- 
heikki

Re: [PATCH 02/21] usb: ulpi: Support device discovery via DT

[Stephen Boyd]

Quoting Heikki Krogerus (2016-06-28 04:42:05)
> On Mon, Jun 27, 2016 at 03:10:40PM -0700, Stephen Boyd wrote:
> > 
> > Right, I would have to make up some IDs in this case. I suppose I can
> > use the qcom vendor ID 0x05c6 and then product ids 0 and 1 for HS phy
> > and HSIC phy? That doesn't make me feel great because it's all made up,
> > but I guess there's no other option. I hope they don't decide to start
> > populating these ids in the future though and then we may have
> > conflicting product ids. If that happens I suppose we can do a
> > workaround based on compatible strings in the DT node. Fun!
> > 
> > Nice side effect of all that is I can drop requesting the module by DT
> > aliases and things become simpler. I'll try this out.
> 
> I was hoping that we could manage with product id 0 as an exception (I
> failed to consider that you have multiple PHYs to deal with). I don't
> think we can just come up with product id > 0.
> 
> I guess we should have the of_driver_match_device() call after all.
> Let's just call it conditionally, only in cases where there is no
> product ID, to make me feel a bit more better. I don't want to make it
> too easy to use.
> 
> The properties for the vendor and product ID are still something that
> we need to introduce in any case. We have the powered off problem on
> all kinds of platforms, and not all of them use DT. Please feel free
> to incorporate the diff into the patch you had for the powered off
> case if you are OK with it. So I think in your case you would just
> need to addthe correct ulpi-vendor id 0x05c6 and ulpi-product id 0 to
> the chipidea device node, and I think this would work.
> 

Hmm ok. I'll have to bring back all the module loading and uevent stuff
based on DT compatible strings then. We'll have the same product id on
HSIC and HS phys, but that isn't a big deal.

Re: [PATCH 02/21] usb: ulpi: Support device discovery via DT

[Rob Herring]

On Sun, Jun 26, 2016 at 12:28:19AM -0700, Stephen Boyd wrote:
> The qcom HSIC ulpi phy doesn't have any bits set in the vendor or
> product id ulpi registers. This makes it impossible to make a
> ulpi driver match against the id registers. Add support to
> discover the ulpi phys via DT to help alleviate this problem.
> We'll look for a ulpi bus node underneath the device registering
> the ulpi viewport (or the parent of that device to support
> chipidea's device layout) and then match up the phy node
> underneath that with the ulpi device that's created.
> 
> The side benefit of this is that we can use standard DT
> properties in the phy node like clks, regulators, gpios, etc.
> because we don't have firmware like ACPI to turn these things on
> for us. And we can use the DT phy binding to point our phy
> consumer to the phy provider.
> 
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Cc: <devicetree@vger.kernel.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> ---
>  Documentation/devicetree/bindings/usb/ulpi.txt | 20 +++++++++
>  drivers/usb/common/ulpi.c                      | 56 +++++++++++++++++++++++++-
>  2 files changed, 74 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/usb/ulpi.txt
> 
> diff --git a/Documentation/devicetree/bindings/usb/ulpi.txt b/Documentation/devicetree/bindings/usb/ulpi.txt
> new file mode 100644
> index 000000000000..ca179dc4bd50
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/ulpi.txt
> @@ -0,0 +1,20 @@
> +ULPI bus binding
> +----------------
> +
> +Phys that are behind a ULPI connection can be described with the following
> +binding. The host controller shall have a "ulpi" named node as a child, and
> +that node shall have one enabled node underneath it representing the ulpi
> +device on the bus.

This needs to co-exist with the USB bus binding which has the controller 
ports for the child nodes. Maybe use the phy binding?

> +
> +EXAMPLE
> +-------
> +
> +usb {
> +	compatible = "vendor,usb-controller";
> +
> +	ulpi {
> +		phy {
> +			compatible = "vendor,phy";
> +		};
> +	};
> +};

Re: [PATCH 20/21] phy: Add support for Qualcomm's USB HSIC phy

[Stephen Boyd]

Quoting Neil Armstrong (2016-06-28 01:49:37)
> On 06/26/2016 09:28 AM, Stephen Boyd wrote:
> > +     uphy->cal_sleep_clk = clk = devm_clk_get(&ulpi->dev, "cal_sleep");
> > +     if (IS_ERR(clk))
> > +             return PTR_ERR(clk);
> 
> Hi Stephen,
> 
> In the bindings the cal_sleep is marked optional, and I think should be since AFAIK
> it's not present on MDM9615 for example.

The cal_sleep clk is just the sleep clk then (should be a board clk in
DT). Sometimes there's a gate in GCC to allow us to turn it off, other
times there isn't. Either way, it's always wired there so I'll update
the binding to say it isn't optional.

> 
> Also MDM9615 HSIC requires "core", "alt-core", "phy", "cal" and "iface" clocks.
> I assume "core" can be attributed to the main chipidea node, but I think "alt-core" and "iface" should be also optionnal.

Looking at the downstream sources I see this:

        "core_clk"     -> usb_hsic_sys_clk
        "iface_clk"    -> usb_hsic_p_clk
        "alt_core_clk" -> usb_hsic_xcvr_clk

        "cal_clk"      -> usb_hsic_hsio_cal_clk
        "phy_clk"      -> usb_hsic_clk

"core_clk" would be the core clk in ci_hdrc_msm. "iface_clk" would be
the iface clk in ci_hdrc_msm. "cal_clk" would be the cal clk in the hsic
phy and "phy_clk" would be the phy clk in the hsic phy.

That leaves alt_core_clk which seems to be a clock that needs to be on
during the reset assert/deassert and possibly for LPM and USB1.1 FS
modes. Sometimes it's referred to as the "housekeeping" clk. Due to the
way resets are done on msm8974 and later SoCs it looks like this clk was
removed. I can make this an optional clk in the ci_hdrc_msm driver, or
we can have two versions of the ci_hdrc_msm compatible string, one for a
device that has the housekeeping clk and one for the device that
doesn't.

> 
> Finally, it misses an optional reset line AFAIK mandatory on MDM9615.
> 

>From what I can tell downstream, all those clks point to the same bit 0
of HSIC_RESET register? So there isn't any phy reset, just the chipidea
controller wrapper reset bit, which should go into the wrapper node?

Re: [PATCH 02/21] usb: ulpi: Support device discovery via DT

[Stephen Boyd]

Quoting Rob Herring (2016-06-28 13:56:42)
> On Sun, Jun 26, 2016 at 12:28:19AM -0700, Stephen Boyd wrote:
> > The qcom HSIC ulpi phy doesn't have any bits set in the vendor or
> > product id ulpi registers. This makes it impossible to make a
> > ulpi driver match against the id registers. Add support to
> > discover the ulpi phys via DT to help alleviate this problem.
> > We'll look for a ulpi bus node underneath the device registering
> > the ulpi viewport (or the parent of that device to support
> > chipidea's device layout) and then match up the phy node
> > underneath that with the ulpi device that's created.
> > 
> > The side benefit of this is that we can use standard DT
> > properties in the phy node like clks, regulators, gpios, etc.
> > because we don't have firmware like ACPI to turn these things on
> > for us. And we can use the DT phy binding to point our phy
> > consumer to the phy provider.
> > 
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > Cc: <devicetree@vger.kernel.org>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> > ---
> >  Documentation/devicetree/bindings/usb/ulpi.txt | 20 +++++++++
> >  drivers/usb/common/ulpi.c                      | 56 +++++++++++++++++++++++++-
> >  2 files changed, 74 insertions(+), 2 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/usb/ulpi.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/usb/ulpi.txt b/Documentation/devicetree/bindings/usb/ulpi.txt
> > new file mode 100644
> > index 000000000000..ca179dc4bd50
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/ulpi.txt
> > @@ -0,0 +1,20 @@
> > +ULPI bus binding
> > +----------------
> > +
> > +Phys that are behind a ULPI connection can be described with the following
> > +binding. The host controller shall have a "ulpi" named node as a child, and
> > +that node shall have one enabled node underneath it representing the ulpi
> > +device on the bus.
> 
> This needs to co-exist with the USB bus binding which has the controller 
> ports for the child nodes. Maybe use the phy binding?

Which binding is that? bindings/usb/usb-device.txt? This ulpi binding is
to describe phys that are accessed through the ulpi "viewport" in the
usb controller. So controller ports don't come into the picture here.

Re: [PATCH 02/21] usb: ulpi: Support device discovery via DT

[Peter Chen]

On Tue, Jun 28, 2016 at 02:42:05PM +0300, Heikki Krogerus wrote:
> On Mon, Jun 27, 2016 at 03:10:40PM -0700, Stephen Boyd wrote:
> > Quoting Heikki Krogerus (2016-06-27 07:34:22)
> > > Hi,
> > > 
> > > I'm fine with most of the patch, except..
> > > 
> > > On Sun, Jun 26, 2016 at 12:28:19AM -0700, Stephen Boyd wrote:
> > > > @@ -39,7 +42,10 @@ static int ulpi_match(struct device *dev, struct device_driver *driver)
> > > >       struct ulpi *ulpi = to_ulpi_dev(dev);
> > > >       const struct ulpi_device_id *id;
> > > >  
> > > > -     for (id = drv->id_table; id->vendor; id++)
> > > > +     if (of_driver_match_device(dev, driver))
> > > > +             return 1;
> > > 
> > > I don't like this part. We should match separately like that only
> > > if the bus does not support native enumeration, and of course ULPI
> > > with its vendor and product IDs does. There really should always be
> > > IDs to match with here. So exceptions have to be solved before we
> > > attempt matching.
> > > 
> > > Since we also have to support platforms where the PHY is initially
> > > powered off and reading the IDs from the registers is not possible
> > > because of that, I think we should consider getting the product and
> > > vendor IDs optionally from device properties. Something like this:
> > 
> > Ok, I'm a little worried about conflating the powered off problem with
> > this product/vendor ID missing problem. But if you're ok with that I'll
> > combine the two patches into one using your approach below.
> > 
> > > 
> > > 
> > > diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
> > > index 01c0c04..6228a85 100644
> > > --- a/drivers/usb/common/ulpi.c
> > > +++ b/drivers/usb/common/ulpi.c
> > > @@ -152,7 +152,7 @@ EXPORT_SYMBOL_GPL(ulpi_unregister_driver);
> > >  
> > >  /* -------------------------------------------------------------------------- */
> > >  
> > > -static int ulpi_register(struct device *dev, struct ulpi *ulpi)
> > > +static int ulpi_read_id(struct ulpi *ulpi)
> > >  {
> > >         int ret;
> > >  
> > > @@ -174,6 +174,21 @@ static int ulpi_register(struct device *dev, struct ulpi *ulpi)
> > >         ulpi->id.product = ulpi_read(ulpi, ULPI_PRODUCT_ID_LOW);
> > >         ulpi->id.product |= ulpi_read(ulpi, ULPI_PRODUCT_ID_HIGH) << 8;
> > >  
> > > +       return 0;
> > > +}
> > > +
> > > +static int ulpi_register(struct device *dev, struct ulpi *ulpi)
> > > +{
> > > +       int ret;
> > > +
> > > +       ret = device_property_read_u16(dev, "ulpi-vendor", &ulpi->id.vendor);
> > > +       ret |= device_property_read_u16(dev, "ulpi-product", &ulpi->id.product);
> > > +       if (ret) {
> > > +               ret = ulpi_read_id(ulpi);
> > > +               if (ret)
> > > +                       return ret;
> > > +       }
> > > +
> > >         ulpi->dev.parent = dev;
> > >         ulpi->dev.bus = &ulpi_bus;
> > >         ulpi->dev.type = &ulpi_dev_type;
> > > 
> > > 
> > > That should cover both cases. You would just have to create the IDs
> > > yourself in this case.
> > > 
> > 
> > Right, I would have to make up some IDs in this case. I suppose I can
> > use the qcom vendor ID 0x05c6 and then product ids 0 and 1 for HS phy
> > and HSIC phy? That doesn't make me feel great because it's all made up,
> > but I guess there's no other option. I hope they don't decide to start
> > populating these ids in the future though and then we may have
> > conflicting product ids. If that happens I suppose we can do a
> > workaround based on compatible strings in the DT node. Fun!
> > 
> > Nice side effect of all that is I can drop requesting the module by DT
> > aliases and things become simpler. I'll try this out.
> 
> I was hoping that we could manage with product id 0 as an exception (I
> failed to consider that you have multiple PHYs to deal with). I don't
> think we can just come up with product id > 0.
> 
> I guess we should have the of_driver_match_device() call after all.
> Let's just call it conditionally, only in cases where there is no
> product ID, to make me feel a bit more better. I don't want to make it
> too easy to use.
> 
> The properties for the vendor and product ID are still something that
> we need to introduce in any case. We have the powered off problem on
> all kinds of platforms, and not all of them use DT.

I am thinking power sequence framework, how power sequence elements
(eg, clock, reset-gpios) can get for non-DT platform? Does ACPI does power
sequence for x86 platforms?

-- 

Best Regards,
Peter Chen

Re: [PATCH 20/21] phy: Add support for Qualcomm's USB HSIC phy

[Neil Armstrong]

On 06/28/2016 11:58 PM, Stephen Boyd wrote:
> Quoting Neil Armstrong (2016-06-28 01:49:37)
>> On 06/26/2016 09:28 AM, Stephen Boyd wrote:
>>> +     uphy->cal_sleep_clk = clk = devm_clk_get(&ulpi->dev, "cal_sleep");
>>> +     if (IS_ERR(clk))
>>> +             return PTR_ERR(clk);
>>
>> Hi Stephen,
>>
>> In the bindings the cal_sleep is marked optional, and I think should be since AFAIK
>> it's not present on MDM9615 for example.
> 
> The cal_sleep clk is just the sleep clk then (should be a board clk in
> DT). Sometimes there's a gate in GCC to allow us to turn it off, other
> times there isn't. Either way, it's always wired there so I'll update
> the binding to say it isn't optional.

Sorry I don't understand !
What should I do if GCC does not provide a gate here ? And looking at the driver, it could be optional.

> 
>>
>> Also MDM9615 HSIC requires "core", "alt-core", "phy", "cal" and "iface" clocks.
>> I assume "core" can be attributed to the main chipidea node, but I think "alt-core" and "iface" should be also optionnal.
> 
> Looking at the downstream sources I see this:
> 
>         "core_clk"     -> usb_hsic_sys_clk
>         "iface_clk"    -> usb_hsic_p_clk
>         "alt_core_clk" -> usb_hsic_xcvr_clk
> 
>         "cal_clk"      -> usb_hsic_hsio_cal_clk
>         "phy_clk"      -> usb_hsic_clk
> 
> "core_clk" would be the core clk in ci_hdrc_msm. "iface_clk" would be
> the iface clk in ci_hdrc_msm. "cal_clk" would be the cal clk in the hsic
> phy and "phy_clk" would be the phy clk in the hsic phy.
> 
> That leaves alt_core_clk which seems to be a clock that needs to be on
> during the reset assert/deassert and possibly for LPM and USB1.1 FS
> modes. Sometimes it's referred to as the "housekeeping" clk. Due to the
> way resets are done on msm8974 and later SoCs it looks like this clk was
> removed. I can make this an optional clk in the ci_hdrc_msm driver, or
> we can have two versions of the ci_hdrc_msm compatible string, one for a
> device that has the housekeeping clk and one for the device that
> doesn't.

Having it optional would be the best solution I think.

> 
>>
>> Finally, it misses an optional reset line AFAIK mandatory on MDM9615.
>>
> 
> From what I can tell downstream, all those clks point to the same bit 0
> of HSIC_RESET register? So there isn't any phy reset, just the chipidea
> controller wrapper reset bit, which should go into the wrapper node?

Ok, makes sense.

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 20/21] phy: Add support for Qualcomm's USB HSIC phy

[Stephen Boyd]

Quoting Neil Armstrong (2016-06-29 02:16:51)
> On 06/28/2016 11:58 PM, Stephen Boyd wrote:
> > Quoting Neil Armstrong (2016-06-28 01:49:37)
> >> On 06/26/2016 09:28 AM, Stephen Boyd wrote:
> >>> +     uphy->cal_sleep_clk = clk = devm_clk_get(&ulpi->dev, "cal_sleep");
> >>> +     if (IS_ERR(clk))
> >>> +             return PTR_ERR(clk);
> >>
> >> Hi Stephen,
> >>
> >> In the bindings the cal_sleep is marked optional, and I think should be since AFAIK
> >> it's not present on MDM9615 for example.
> > 
> > The cal_sleep clk is just the sleep clk then (should be a board clk in
> > DT). Sometimes there's a gate in GCC to allow us to turn it off, other
> > times there isn't. Either way, it's always wired there so I'll update
> > the binding to say it isn't optional.
> 
> Sorry I don't understand !
> What should I do if GCC does not provide a gate here ? And looking at the driver, it could be optional.

You should set the property to point to &sleep_clk which should be under
the "clocks" node at the root of the OF tree. For example, see the
sleep_clk node in arch/arm/boot/dts/qcom-apq8064.dtsi.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/21] usb: ulpi: Support device discovery via DT

[Rob Herring]

On Tue, Jun 28, 2016 at 03:09:21PM -0700, Stephen Boyd wrote:
> Quoting Rob Herring (2016-06-28 13:56:42)
> > On Sun, Jun 26, 2016 at 12:28:19AM -0700, Stephen Boyd wrote:
> > > The qcom HSIC ulpi phy doesn't have any bits set in the vendor or
> > > product id ulpi registers. This makes it impossible to make a
> > > ulpi driver match against the id registers. Add support to
> > > discover the ulpi phys via DT to help alleviate this problem.
> > > We'll look for a ulpi bus node underneath the device registering
> > > the ulpi viewport (or the parent of that device to support
> > > chipidea's device layout) and then match up the phy node
> > > underneath that with the ulpi device that's created.
> > > 
> > > The side benefit of this is that we can use standard DT
> > > properties in the phy node like clks, regulators, gpios, etc.
> > > because we don't have firmware like ACPI to turn these things on
> > > for us. And we can use the DT phy binding to point our phy
> > > consumer to the phy provider.
> > > 
> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > > Cc: <devicetree@vger.kernel.org>
> > > Cc: Rob Herring <robh+dt@kernel.org>
> > > Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> > > ---
> > >  Documentation/devicetree/bindings/usb/ulpi.txt | 20 +++++++++
> > >  drivers/usb/common/ulpi.c                      | 56 +++++++++++++++++++++++++-
> > >  2 files changed, 74 insertions(+), 2 deletions(-)
> > >  create mode 100644 Documentation/devicetree/bindings/usb/ulpi.txt
> > > 
> > > diff --git a/Documentation/devicetree/bindings/usb/ulpi.txt b/Documentation/devicetree/bindings/usb/ulpi.txt
> > > new file mode 100644
> > > index 000000000000..ca179dc4bd50
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/usb/ulpi.txt
> > > @@ -0,0 +1,20 @@
> > > +ULPI bus binding
> > > +----------------
> > > +
> > > +Phys that are behind a ULPI connection can be described with the following
> > > +binding. The host controller shall have a "ulpi" named node as a child, and
> > > +that node shall have one enabled node underneath it representing the ulpi
> > > +device on the bus.
> > 
> > This needs to co-exist with the USB bus binding which has the controller 
> > ports for the child nodes. Maybe use the phy binding?
> 
> Which binding is that? bindings/usb/usb-device.txt? 

Yes.

> This ulpi binding is
> to describe phys that are accessed through the ulpi "viewport" in the
> usb controller. So controller ports don't come into the picture here.

You just need to confirm that there's no collision with child nodes like 
it assumes all children are ports.

Rob

Re: [PATCH 00/21] Support qcom's HSIC USB and rewrite USB2 HS phy support

[John Stultz]

On Tue, Jun 28, 2016 at 1:34 AM, Stephen Boyd <stephen.boyd@linaro.org> wrote:
> Quoting John Stultz (2016-06-27 20:09:30)
>>
>> I haven't yet been able to test with this, as I need some other fixes
>> it seems too to deal with some of the iommu changes in my flo-WIP tree
>> (it can't find of_dma_configure), but will let you know how things
>> work once I have all that sorted.
>
> Cool, thanks for testing.

So after working out some merge issues w/ my flo-WIP branch for the
nexus7, I was still having trouble, so I backed out to just your
(updated) branch.

But even there, I'm not able to get the usb gadget up. I see:

[    1.869717] msm_hsusb 12500000.usb: failed to get phandle in
/soc/usb@12500000 node
[    1.882347] 12500000.usb supply vbus not found, using dummy regulator

So I'm not sure if the dts changes were quite right. I've got all the
ULPI configs enabled. Any ideas?

thanks
-john

Re: [PATCH 00/21] Support qcom's HSIC USB and rewrite USB2 HS phy support

[Stephen Boyd]

Quoting John Stultz (2016-07-01 23:03:38)
> On Tue, Jun 28, 2016 at 1:34 AM, Stephen Boyd <stephen.boyd@linaro.org> wrote:
> > Quoting John Stultz (2016-06-27 20:09:30)
> >>
> >> I haven't yet been able to test with this, as I need some other fixes
> >> it seems too to deal with some of the iommu changes in my flo-WIP tree
> >> (it can't find of_dma_configure), but will let you know how things
> >> work once I have all that sorted.
> >
> > Cool, thanks for testing.
> 
> So after working out some merge issues w/ my flo-WIP branch for the
> nexus7, I was still having trouble, so I backed out to just your
> (updated) branch.
> 
> But even there, I'm not able to get the usb gadget up. I see:

I can get the gadget working on the ifc6410 device I have. I'm using
this script to test things:

modprobe libcomposite
mkdir config
mount none config -t configfs
mkdir config/usb_gadget/g1
mkdir config/usb_gadget/g1/strings/0x409
echo 012345678 > config/usb_gadget/g1/strings/0x409/serialnumber
echo "manufacturer" > config/usb_gadget/g1/strings/0x409/manufacturer
echo "db8074" > config/usb_gadget/g1/strings/0x409/product
echo 0x1d6b > config/usb_gadget/g1/idVendor
echo 0x0104 > config/usb_gadget/g1/idProduct
mkdir config/usb_gadget/g1/functions/acm.GS0
mkdir config/usb_gadget/g1/functions/acm.GS1
mkdir config/usb_gadget/g1/functions/ecm.usb0
mkdir config/usb_gadget/g1/configs/c.1
mkdir config/usb_gadget/g1/configs/c.1/strings/0x409/
echo "CDC 2xACM+ECM" >
config/usb_gadget/g1/configs/c.1/strings/0x409/configuration
ln -s config/usb_gadget/g1/functions/acm.GS0
config/usb_gadget/g1/configs/c.1
ln -s config/usb_gadget/g1/functions/acm.GS1
config/usb_gadget/g1/configs/c.1
ln -s config/usb_gadget/g1/functions/ecm.usb0
config/usb_gadget/g1/configs/c.1
echo $(ls /sys/class/udc/) > config/usb_gadget/g1/UDC

> 
> [    1.869717] msm_hsusb 12500000.usb: failed to get phandle in
> /soc/usb@12500000 node

This is sort of ok (I've seen it before with the HSIC controller on
8074). The extcon is optional and so the error message should be
silenced. I sent a patch to move it to debug level.

I don't see an extcon in mainline for apq8064, but I think it may be
needed. At the least, I see that we're using a PMIC interrupt in the
msm-3.4 kernel for the ID pin and the charger is notifying of vbus
events similar to how smbb is doing it for apq8074.

> [    1.882347] 12500000.usb supply vbus not found, using dummy regulator

This is not great. We don't have a regulator specified on apq8064 so we
can't turn on vbus and really support host mode. This seems to already
be the case on mainline though, so it's not like we're any worse here.
To properly support this we need a regulator driver. I suppose if the
RPM supports it we can just use that, but if it doesn't support this
regulator then we need to port over the SSBI regulator driver to do
native regulator control. So far I haven't been able to test host mode
on apq8064 because of this.

> 
> So I'm not sure if the dts changes were quite right. I've got all the
> ULPI configs enabled. Any ideas?
> 

The phy and controller are both probing? If they're not failing to probe
then it's probably the phy that isn't working properly. Does gadget work
without my patches on nexus7?

Re: [PATCH 00/21] Support qcom's HSIC USB and rewrite USB2 HS phy support

[John Stultz]

On Tue, Jul 5, 2016 at 12:22 PM, Stephen Boyd <stephen.boyd@linaro.org> wrote:
> Quoting John Stultz (2016-07-01 23:03:38)
>> On Tue, Jun 28, 2016 at 1:34 AM, Stephen Boyd <stephen.boyd@linaro.org> wrote:
>> > Quoting John Stultz (2016-06-27 20:09:30)
>> >>
>> >> I haven't yet been able to test with this, as I need some other fixes
>> >> it seems too to deal with some of the iommu changes in my flo-WIP tree
>> >> (it can't find of_dma_configure), but will let you know how things
>> >> work once I have all that sorted.
>> >
>> > Cool, thanks for testing.
>>
>> So after working out some merge issues w/ my flo-WIP branch for the
>> nexus7, I was still having trouble, so I backed out to just your
>> (updated) branch.
>>
>> But even there, I'm not able to get the usb gadget up. I see:
>
> I can get the gadget working on the ifc6410 device I have. I'm using
> this script to test things:
>
> modprobe libcomposite
> mkdir config
> mount none config -t configfs
> mkdir config/usb_gadget/g1
> mkdir config/usb_gadget/g1/strings/0x409
> echo 012345678 > config/usb_gadget/g1/strings/0x409/serialnumber
> echo "manufacturer" > config/usb_gadget/g1/strings/0x409/manufacturer
> echo "db8074" > config/usb_gadget/g1/strings/0x409/product
> echo 0x1d6b > config/usb_gadget/g1/idVendor
> echo 0x0104 > config/usb_gadget/g1/idProduct
> mkdir config/usb_gadget/g1/functions/acm.GS0
> mkdir config/usb_gadget/g1/functions/acm.GS1
> mkdir config/usb_gadget/g1/functions/ecm.usb0
> mkdir config/usb_gadget/g1/configs/c.1
> mkdir config/usb_gadget/g1/configs/c.1/strings/0x409/
> echo "CDC 2xACM+ECM" >
> config/usb_gadget/g1/configs/c.1/strings/0x409/configuration
> ln -s config/usb_gadget/g1/functions/acm.GS0
> config/usb_gadget/g1/configs/c.1
> ln -s config/usb_gadget/g1/functions/acm.GS1
> config/usb_gadget/g1/configs/c.1
> ln -s config/usb_gadget/g1/functions/ecm.usb0
> config/usb_gadget/g1/configs/c.1
> echo $(ls /sys/class/udc/) > config/usb_gadget/g1/UDC
>
>>
>> [    1.869717] msm_hsusb 12500000.usb: failed to get phandle in
>> /soc/usb@12500000 node
>
> This is sort of ok (I've seen it before with the HSIC controller on
> 8074). The extcon is optional and so the error message should be
> silenced. I sent a patch to move it to debug level.
>
> I don't see an extcon in mainline for apq8064, but I think it may be
> needed. At the least, I see that we're using a PMIC interrupt in the
> msm-3.4 kernel for the ID pin and the charger is notifying of vbus
> events similar to how smbb is doing it for apq8074.
>
>> [    1.882347] 12500000.usb supply vbus not found, using dummy regulator
>
> This is not great. We don't have a regulator specified on apq8064 so we
> can't turn on vbus and really support host mode. This seems to already
> be the case on mainline though, so it's not like we're any worse here.
> To properly support this we need a regulator driver. I suppose if the
> RPM supports it we can just use that, but if it doesn't support this
> regulator then we need to port over the SSBI regulator driver to do
> native regulator control. So far I haven't been able to test host mode
> on apq8064 because of this.
>
>>
>> So I'm not sure if the dts changes were quite right. I've got all the
>> ULPI configs enabled. Any ideas?
>>
>
> The phy and controller are both probing? If they're not failing to probe
> then it's probably the phy that isn't working properly. Does gadget work
> without my patches on nexus7?

Without your patch, I need one dts tweak on nexus7 (which I sent out
on friday[1]) to get the default direction set to OTG or gadget. But
yea, it works with that against mainline.

I'll dig in some more on your suggestions above and let you know if I
figure it out.

thanks
-john

[1]: http://www.spinics.net/lists/devicetree/msg134274.html

Re: [PATCH 02/21] usb: ulpi: Support device discovery via DT

[Stephen Boyd]

Quoting Rob Herring (2016-06-30 17:59:15)
> On Tue, Jun 28, 2016 at 03:09:21PM -0700, Stephen Boyd wrote:
> > Quoting Rob Herring (2016-06-28 13:56:42)
> > > On Sun, Jun 26, 2016 at 12:28:19AM -0700, Stephen Boyd wrote:
> > > > The qcom HSIC ulpi phy doesn't have any bits set in the vendor or
> > > > product id ulpi registers. This makes it impossible to make a
> > > > ulpi driver match against the id registers. Add support to
> > > > discover the ulpi phys via DT to help alleviate this problem.
> > > > We'll look for a ulpi bus node underneath the device registering
> > > > the ulpi viewport (or the parent of that device to support
> > > > chipidea's device layout) and then match up the phy node
> > > > underneath that with the ulpi device that's created.
> > > > 
> > > > The side benefit of this is that we can use standard DT
> > > > properties in the phy node like clks, regulators, gpios, etc.
> > > > because we don't have firmware like ACPI to turn these things on
> > > > for us. And we can use the DT phy binding to point our phy
> > > > consumer to the phy provider.
> > > > 
> > > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > > > Cc: <devicetree@vger.kernel.org>
> > > > Cc: Rob Herring <robh+dt@kernel.org>
> > > > Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> > > > ---
> > > >  Documentation/devicetree/bindings/usb/ulpi.txt | 20 +++++++++
> > > >  drivers/usb/common/ulpi.c                      | 56 +++++++++++++++++++++++++-
> > > >  2 files changed, 74 insertions(+), 2 deletions(-)
> > > >  create mode 100644 Documentation/devicetree/bindings/usb/ulpi.txt
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/usb/ulpi.txt b/Documentation/devicetree/bindings/usb/ulpi.txt
> > > > new file mode 100644
> > > > index 000000000000..ca179dc4bd50
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/usb/ulpi.txt
> > > > @@ -0,0 +1,20 @@
> > > > +ULPI bus binding
> > > > +----------------
> > > > +
> > > > +Phys that are behind a ULPI connection can be described with the following
> > > > +binding. The host controller shall have a "ulpi" named node as a child, and
> > > > +that node shall have one enabled node underneath it representing the ulpi
> > > > +device on the bus.
> > > 
> > > This needs to co-exist with the USB bus binding which has the controller 
> > > ports for the child nodes. Maybe use the phy binding?
> > 
> > Which binding is that? bindings/usb/usb-device.txt? 
> 
> Yes.
> 
> > This ulpi binding is
> > to describe phys that are accessed through the ulpi "viewport" in the
> > usb controller. So controller ports don't come into the picture here.
> 
> You just need to confirm that there's no collision with child nodes like 
> it assumes all children are ports.
> 

Ok. It will work with the way usb_of_get_child_node() is written but it
doesn't seem like great DT design if we want to support usb-device
binding and ulpi bus at the same time, because we treat the controller
node as the usb bus.

&usb1 { 
        status = "okay";

        #address-cells = <1>;
        #size-cells = <0>;
	phys = <&uphy>;
	phy-names = "usb-phy";

        hub: genesys@1 {
                compatible = "usb5e3,608";
                reg = <1>;
        };

	ulpi {
		uphy: phy {
			compatible = "phy";
		};
	};
}

In this example, the ulpi bus doesn't have a reg property because it
isn't a port.

Perhaps we can move the usb ports to a subnode like 'usb' so that we can
have two logical busses underneath the controller, one for usb and one
for ulpi? We would need backwards compatibility code that looks for a
ports directly underneath the usb controller node when #address-cells
and #size-cells are present, otherwise it should fall down into a
'usb' subnode.