[PATCH v4 0/3] initialize (multiple) PHYs in xhci-plat

[PATCH v4 0/3] initialize (multiple) PHYs in xhci-plat

[Martin Blumenstingl]

This series is the outcome of a discussion with Felipe Balbi,
see [0] and [1].
The quick-summary of this is:
- dwc3 already takes one USB2 and one USB3 PHY and initializes these
  correct
- some other HCI platform drivers (like ehci-platform.c, xhci-mtk.c and
  ohci-platform.c) do not have a limitation on the number of PHYs - they
  support one PHY per actual host port
- Amlogic Meson GXL and GXM SoCs come with a dwc3 IP block which has two
  or three USB2 ports enabled on the internal root-hub. The SoCs also
  provide separate USB2 PHYs, one per port. All USB2 PHYs (which are
  internally "connected" to the dwc3 roothub) need to be powered on,
  otherwise USB devices cannot be enumerated (even if just one PHY is
  disabled and if the device is plugged into another, enabled port)

In my first attempt to get USB supported on the GXL and GXM SoCs I tried
to work-around the problem that I could not pass multiple PHYs to the
dwc3 controller.
This was rejected by Rob Herring (which was definitely the thing to do in
my opinion), see [2]

This series adds a new "platform-roothub". This can be configured through
devicetree by passing a child-node with "reg = <0>" to the USB
controller. Additionally there has to be a child-node for each port on
the root-hub. Each of the child-nodes takes a "phys" and "phy-names"
property. This allows modeling the root-hub in devicetree similar to the
USB device binding (documented in devicetree/bindings/usb/usb-device.txt)
This avoids and backwards-compatibility problems (which was a concern
regardless of the solution, see [3]) since the binding for the root-hub
was previously not specified (and we're not using the "phys" property of
the controller, which might have served different purposes before,
depending on the drivers).

Additionally this integrates the new platform-roothub into xhci-plat.c
which automatically enables it for the dwc3 driver (in host-mode).


Changes since RFCv3 at [6]:
- moved the DT binding change from patch #3 to patch #1 as suggested
  by Rob Herring (and slightly adjusted the commit message to account
  for that)
- added Tested-by from Chunfeng Yun (who confirmed that the whole
  concept and implementation works fine on Mediatek SoCs - many thanks
  again!) to patch #2
- added Rob Herring's ACK to patches 1 and 3
- dropped RFC status (RFCv3 -> PATCH v4)

Changes since RFCv2 at [5]:
- split phy_{init,exit} and phy_power_{on,off} handling. up until RFCv2
  I called phy_init plus phy_power_on in platform_roothub_power_on and
  phy_power_off plus phy_exit in platform_roothub_power_off. However,
  Chunfeng Yun (a Mediatek SoC developer - many thanks for testing my
  series and providing great feedback) reported that only using
  phy_power_off (and omitting phy_exit) during system suspend fixes an
  issue where USB devices would be re-enumerated when resuming. His
  original problem description: "In order to keep link state on mt8173,
  we just power off all phys(not exit) when system enter suspend, then
  power on them again (needn't init, otherwise device will be
  disconnected) when system resume, this can avoid re-enumerating
  device.". This fix affects patch #2 and #3 as we now have
  platform_roothub_init (which calls phy_init internally),
  platform_roothub_power_on (which calls phy_power_on internally),
  platform_roothub_power_off (which calls phy_power_off internally) and
  platform_roothub_exit (which calls phy_exit internally). suspend and
  resume only call platform_roothub_power_{on,off} to prevent the issue
  described by Chunfeng Yun (unfortunately I cannot test this because
  the Amlogic platform currently does not support system suspend).
- dropped two struct forward declarations from platform-roothub.h which
  are not used in the header file (thanks to Chunfeng Yun for spotting
  this)

Changes since RFCv1 at [4]:
- split the usb-xhci dt-binding documentation into a separate patch
- fixed a typo ("usb-phy" -> "phys" in the dt-binding example)
- rebased to apply against latest usb-next


[0] http://lists.infradead.org/pipermail/linux-amlogic/2017-January/001945.html
[1] http://lists.infradead.org/pipermail/linux-amlogic/2017-January/001947.html
[2] http://lists.infradead.org/pipermail/linux-amlogic/2016-November/001818.html
[3] http://lists.infradead.org/pipermail/linux-amlogic/2017-January/001948.html
[4] http://marc.info/?l=linux-usb&m=148414866303604&w=2
[5] https://www.spinics.net/lists/linux-usb/msg158967.html
[6] https://www.spinics.net/lists/devicetree/msg190426.html

Martin Blumenstingl (3):
  dt-bindings: usb: add the documentation for USB root-hub
  usb: host: add a generic platform USB roothub driver
  usb: host: xhci: plat: integrate the platform-roothub

 .../devicetree/bindings/usb/usb-roothub.txt        |  46 ++++++
 Documentation/devicetree/bindings/usb/usb-xhci.txt |   7 +
 drivers/usb/host/Kconfig                           |   4 +
 drivers/usb/host/Makefile                          |   2 +
 drivers/usb/host/platform-roothub.c                | 180 +++++++++++++++++++++
 drivers/usb/host/platform-roothub.h                |  12 ++
 drivers/usb/host/xhci-plat.c                       |  35 +++-
 drivers/usb/host/xhci.h                            |   2 +
 8 files changed, 286 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/usb/usb-roothub.txt
 create mode 100644 drivers/usb/host/platform-roothub.c
 create mode 100644 drivers/usb/host/platform-roothub.h

-- 
2.14.1

--
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 v4 0/3] initialize (multiple) PHYs in xhci-plat

[Martin Blumenstingl]

This series is the outcome of a discussion with Felipe Balbi,
see [0] and [1].
The quick-summary of this is:
- dwc3 already takes one USB2 and one USB3 PHY and initializes these
  correct
- some other HCI platform drivers (like ehci-platform.c, xhci-mtk.c and
  ohci-platform.c) do not have a limitation on the number of PHYs - they
  support one PHY per actual host port
- Amlogic Meson GXL and GXM SoCs come with a dwc3 IP block which has two
  or three USB2 ports enabled on the internal root-hub. The SoCs also
  provide separate USB2 PHYs, one per port. All USB2 PHYs (which are
  internally "connected" to the dwc3 roothub) need to be powered on,
  otherwise USB devices cannot be enumerated (even if just one PHY is
  disabled and if the device is plugged into another, enabled port)

In my first attempt to get USB supported on the GXL and GXM SoCs I tried
to work-around the problem that I could not pass multiple PHYs to the
dwc3 controller.
This was rejected by Rob Herring (which was definitely the thing to do in
my opinion), see [2]

This series adds a new "platform-roothub". This can be configured through
devicetree by passing a child-node with "reg = <0>" to the USB
controller. Additionally there has to be a child-node for each port on
the root-hub. Each of the child-nodes takes a "phys" and "phy-names"
property. This allows modeling the root-hub in devicetree similar to the
USB device binding (documented in devicetree/bindings/usb/usb-device.txt)
This avoids and backwards-compatibility problems (which was a concern
regardless of the solution, see [3]) since the binding for the root-hub
was previously not specified (and we're not using the "phys" property of
the controller, which might have served different purposes before,
depending on the drivers).

Additionally this integrates the new platform-roothub into xhci-plat.c
which automatically enables it for the dwc3 driver (in host-mode).


Changes since RFCv3 at [6]:
- moved the DT binding change from patch #3 to patch #1 as suggested
  by Rob Herring (and slightly adjusted the commit message to account
  for that)
- added Tested-by from Chunfeng Yun (who confirmed that the whole
  concept and implementation works fine on Mediatek SoCs - many thanks
  again!) to patch #2
- added Rob Herring's ACK to patches 1 and 3
- dropped RFC status (RFCv3 -> PATCH v4)

Changes since RFCv2 at [5]:
- split phy_{init,exit} and phy_power_{on,off} handling. up until RFCv2
  I called phy_init plus phy_power_on in platform_roothub_power_on and
  phy_power_off plus phy_exit in platform_roothub_power_off. However,
  Chunfeng Yun (a Mediatek SoC developer - many thanks for testing my
  series and providing great feedback) reported that only using
  phy_power_off (and omitting phy_exit) during system suspend fixes an
  issue where USB devices would be re-enumerated when resuming. His
  original problem description: "In order to keep link state on mt8173,
  we just power off all phys(not exit) when system enter suspend, then
  power on them again (needn't init, otherwise device will be
  disconnected) when system resume, this can avoid re-enumerating
  device.". This fix affects patch #2 and #3 as we now have
  platform_roothub_init (which calls phy_init internally),
  platform_roothub_power_on (which calls phy_power_on internally),
  platform_roothub_power_off (which calls phy_power_off internally) and
  platform_roothub_exit (which calls phy_exit internally). suspend and
  resume only call platform_roothub_power_{on,off} to prevent the issue
  described by Chunfeng Yun (unfortunately I cannot test this because
  the Amlogic platform currently does not support system suspend).
- dropped two struct forward declarations from platform-roothub.h which
  are not used in the header file (thanks to Chunfeng Yun for spotting
  this)

Changes since RFCv1 at [4]:
- split the usb-xhci dt-binding documentation into a separate patch
- fixed a typo ("usb-phy" -> "phys" in the dt-binding example)
- rebased to apply against latest usb-next


[0] http://lists.infradead.org/pipermail/linux-amlogic/2017-January/001945.html
[1] http://lists.infradead.org/pipermail/linux-amlogic/2017-January/001947.html
[2] http://lists.infradead.org/pipermail/linux-amlogic/2016-November/001818.html
[3] http://lists.infradead.org/pipermail/linux-amlogic/2017-January/001948.html
[4] http://marc.info/?l=linux-usb&m=148414866303604&w=2
[5] https://www.spinics.net/lists/linux-usb/msg158967.html
[6] https://www.spinics.net/lists/devicetree/msg190426.html

Martin Blumenstingl (3):
  dt-bindings: usb: add the documentation for USB root-hub
  usb: host: add a generic platform USB roothub driver
  usb: host: xhci: plat: integrate the platform-roothub

 .../devicetree/bindings/usb/usb-roothub.txt        |  46 ++++++
 Documentation/devicetree/bindings/usb/usb-xhci.txt |   7 +
 drivers/usb/host/Kconfig                           |   4 +
 drivers/usb/host/Makefile                          |   2 +
 drivers/usb/host/platform-roothub.c                | 180 +++++++++++++++++++++
 drivers/usb/host/platform-roothub.h                |  12 ++
 drivers/usb/host/xhci-plat.c                       |  35 +++-
 drivers/usb/host/xhci.h                            |   2 +
 8 files changed, 286 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/usb/usb-roothub.txt
 create mode 100644 drivers/usb/host/platform-roothub.c
 create mode 100644 drivers/usb/host/platform-roothub.h

-- 
2.14.1

[PATCH v4 1/3] dt-bindings: usb: add the documentation for USB root-hub

[Martin Blumenstingl]

A USB root-hub may have several PHYs which need to be configured before
the root-hub starts working.
This adds the documentation for such a USB root-hub as well as a hint
regarding the child-nodes on XHCI controllers which can include the
roothub.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 .../devicetree/bindings/usb/usb-roothub.txt        | 46 ++++++++++++++++++++++
 Documentation/devicetree/bindings/usb/usb-xhci.txt |  7 ++++
 2 files changed, 53 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/usb-roothub.txt

diff --git a/Documentation/devicetree/bindings/usb/usb-roothub.txt b/Documentation/devicetree/bindings/usb/usb-roothub.txt
new file mode 100644
index 000000000000..fc0797d7cee9
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/usb-roothub.txt
@@ -0,0 +1,46 @@
+Generic USB root-hub Properties
+
+similar to the USB device bindings (documented in usb-device.txt from the
+current directory) this provides support for configuring the root-hub.
+
+Required properties:
+- compatible: should be at least one of "usb1d6b,3", "usb1d6b,2"
+- reg: must be 0.
+- address-cells: must be 1
+- size-cells: must be 0
+
+Required sub-nodes:
+a sub-node per actual USB port is required. each sub-node supports the
+following properties:
+  Required properties:
+    - reg: the port number on the root-hub (mandatory)
+  Optional properties:
+    - phys: optional, from the *Generic PHY* bindings (mandatory needed
+      when phy-names is given)
+    - phy-names: optional, from the *Generic PHY* bindings; supported names
+      are "usb2-phy" or "usb3-phy"
+
+Example:
+	&usb1 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		roothub@0 {
+			compatible = "usb1d6b,3", "usb1d6b,2";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0>;
+
+			port@1 {
+				reg = <1>;
+				phys = <&usb2_phy1>, <&usb3_phy1>;
+				phy-names = "usb2-phy", "usb3-phy";
+			};
+
+			port@2 {
+				reg = <2>;
+				phys = <&usb2_phy2>, <&usb3_phy2>;
+				phy-names = "usb2-phy", "usb3-phy";
+			};
+		};
+	}
diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt
index 2d80b60eeabe..31b4f681e9ca 100644
--- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
+++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
@@ -29,6 +29,13 @@ Optional properties:
   - usb3-lpm-capable: determines if platform is USB3 LPM capable
   - quirk-broken-port-ped: set if the controller has broken port disable mechanism
 
+sub-nodes:
+- optionally there can be a node for the root-hub, see usb-roothub.txt in the
+  current directory
+- one or more nodes with reg 1-31 for each port to which a device is connected.
+  See usb-device.txt in the current directory for more information.
+
+
 Example:
 	usb@f0931000 {
 		compatible = "generic-xhci";
-- 
2.14.1

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

[PATCH v4 1/3] dt-bindings: usb: add the documentation for USB root-hub

[Martin Blumenstingl]

A USB root-hub may have several PHYs which need to be configured before
the root-hub starts working.
This adds the documentation for such a USB root-hub as well as a hint
regarding the child-nodes on XHCI controllers which can include the
roothub.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/usb/usb-roothub.txt        | 46 ++++++++++++++++++++++
 Documentation/devicetree/bindings/usb/usb-xhci.txt |  7 ++++
 2 files changed, 53 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/usb-roothub.txt

diff --git a/Documentation/devicetree/bindings/usb/usb-roothub.txt b/Documentation/devicetree/bindings/usb/usb-roothub.txt
new file mode 100644
index 000000000000..fc0797d7cee9
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/usb-roothub.txt
@@ -0,0 +1,46 @@
+Generic USB root-hub Properties
+
+similar to the USB device bindings (documented in usb-device.txt from the
+current directory) this provides support for configuring the root-hub.
+
+Required properties:
+- compatible: should be at least one of "usb1d6b,3", "usb1d6b,2"
+- reg: must be 0.
+- address-cells: must be 1
+- size-cells: must be 0
+
+Required sub-nodes:
+a sub-node per actual USB port is required. each sub-node supports the
+following properties:
+  Required properties:
+    - reg: the port number on the root-hub (mandatory)
+  Optional properties:
+    - phys: optional, from the *Generic PHY* bindings (mandatory needed
+      when phy-names is given)
+    - phy-names: optional, from the *Generic PHY* bindings; supported names
+      are "usb2-phy" or "usb3-phy"
+
+Example:
+	&usb1 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		roothub at 0 {
+			compatible = "usb1d6b,3", "usb1d6b,2";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0>;
+
+			port at 1 {
+				reg = <1>;
+				phys = <&usb2_phy1>, <&usb3_phy1>;
+				phy-names = "usb2-phy", "usb3-phy";
+			};
+
+			port at 2 {
+				reg = <2>;
+				phys = <&usb2_phy2>, <&usb3_phy2>;
+				phy-names = "usb2-phy", "usb3-phy";
+			};
+		};
+	}
diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt
index 2d80b60eeabe..31b4f681e9ca 100644
--- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
+++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
@@ -29,6 +29,13 @@ Optional properties:
   - usb3-lpm-capable: determines if platform is USB3 LPM capable
   - quirk-broken-port-ped: set if the controller has broken port disable mechanism
 
+sub-nodes:
+- optionally there can be a node for the root-hub, see usb-roothub.txt in the
+  current directory
+- one or more nodes with reg 1-31 for each port to which a device is connected.
+  See usb-device.txt in the current directory for more information.
+
+
 Example:
 	usb at f0931000 {
 		compatible = "generic-xhci";
-- 
2.14.1

[PATCH v4 2/3] usb: host: add a generic platform USB roothub driver

[Martin Blumenstingl]

Many SoC platforms have separate devices for the USB PHY which are
registered through the generic PHY framework. These PHYs have to be
enabled to make the USB controller actually work. They also have to be
disabled again on shutdown/suspend.

Currently (at least) the following HCI platform drivers are using custom
code to obtain all PHYs via devicetree for the roothub/controller and
disable/enable them when required:
- ehci-platform.c has ehci_platform_power_{on,off}
- xhci-mtk.c has xhci_mtk_phy_{init,exit,power_on,power_off}
- ohci-platform.c has ohci_platform_power_{on,off}

These drivers are not using the generic devicetree USB device bindings
yet which were only introduced recently (documentation is available in
devicetree/bindings/usb/usb-device.txt).
With this new driver the usb2-phy and usb3-phy can be specified directly
in the child-node of the corresponding port of the roothub via
devicetree. This can be extended by not just parsing PHYs (some of the
other drivers listed above are for example also parsing a list of clocks
as well) when required.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
Tested-by: Chunfeng Yun <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
 drivers/usb/host/Kconfig            |   3 +
 drivers/usb/host/Makefile           |   2 +
 drivers/usb/host/platform-roothub.c | 180 ++++++++++++++++++++++++++++++++++++
 drivers/usb/host/platform-roothub.h |  12 +++
 4 files changed, 197 insertions(+)
 create mode 100644 drivers/usb/host/platform-roothub.c
 create mode 100644 drivers/usb/host/platform-roothub.h

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index fa5692dec832..b8b05c786b2a 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -805,6 +805,9 @@ config USB_HCD_SSB
 
 	  If unsure, say N.
 
+config USB_PLATFORM_ROOTHUB
+	bool
+
 config USB_HCD_TEST_MODE
 	bool "HCD test mode support"
 	---help---
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index cf2691fffcc0..dc817f82d632 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -29,6 +29,8 @@ obj-$(CONFIG_USB_WHCI_HCD)	+= whci/
 
 obj-$(CONFIG_USB_PCI)	+= pci-quirks.o
 
+obj-$(CONFIG_USB_PLATFORM_ROOTHUB)	+= platform-roothub.o
+
 obj-$(CONFIG_USB_EHCI_HCD)	+= ehci-hcd.o
 obj-$(CONFIG_USB_EHCI_PCI)	+= ehci-pci.o
 obj-$(CONFIG_USB_EHCI_HCD_PLATFORM)	+= ehci-platform.o
diff --git a/drivers/usb/host/platform-roothub.c b/drivers/usb/host/platform-roothub.c
new file mode 100644
index 000000000000..70d2d97aa8b2
--- /dev/null
+++ b/drivers/usb/host/platform-roothub.c
@@ -0,0 +1,180 @@
+/*
+ * platform roothub driver - a virtual PHY device which passes all phy_*
+ * function calls to multiple (actual) PHY devices. This is comes handy when
+ * initializing all PHYs on a root-hub (to keep them all in the same state).
+ *
+ * Copyright (C) 2017 Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/device.h>
+#include <linux/list.h>
+#include <linux/phy/phy.h>
+#include <linux/of.h>
+#include <linux/usb/of.h>
+
+#include "platform-roothub.h"
+
+#define ROOTHUB_PORTNUM		0
+
+struct platform_roothub {
+	struct phy		*phy;
+	struct list_head	list;
+};
+
+static struct platform_roothub *platform_roothub_alloc(struct device *dev)
+{
+	struct platform_roothub *roothub_entry;
+
+	roothub_entry = devm_kzalloc(dev, sizeof(*roothub_entry), GFP_KERNEL);
+	if (!roothub_entry)
+		return ERR_PTR(-ENOMEM);
+
+	INIT_LIST_HEAD(&roothub_entry->list);
+
+	return roothub_entry;
+}
+
+static int platform_roothub_add_phy(struct device *dev,
+				    struct device_node *port_np,
+				    const char *con_id, struct list_head *list)
+{
+	struct platform_roothub *roothub_entry;
+	struct phy *phy = devm_of_phy_get(dev, port_np, con_id);
+
+	if (IS_ERR_OR_NULL(phy)) {
+		if (!phy || PTR_ERR(phy) == -ENODEV)
+			return 0;
+		else
+			return PTR_ERR(phy);
+	}
+
+	roothub_entry = platform_roothub_alloc(dev);
+	if (IS_ERR(roothub_entry))
+		return PTR_ERR(roothub_entry);
+
+	roothub_entry->phy = phy;
+
+	list_add_tail(&roothub_entry->list, list);
+
+	return 0;
+}
+
+struct platform_roothub *platform_roothub_init(struct device *dev)
+{
+	struct device_node *roothub_np, *port_np;
+	struct platform_roothub *plat_roothub;
+	struct platform_roothub *roothub_entry;
+	struct list_head *head;
+	int err;
+
+	roothub_np = usb_of_get_child_node(dev->of_node, ROOTHUB_PORTNUM);
+	if (!of_device_is_available(roothub_np))
+		return NULL;
+
+	plat_roothub = platform_roothub_alloc(dev);
+	if (IS_ERR(plat_roothub))
+		return plat_roothub;
+
+	for_each_available_child_of_node(roothub_np, port_np) {
+		err = platform_roothub_add_phy(dev, port_np, "usb2-phy",
+					       &plat_roothub->list);
+		if (err)
+			goto err_out;
+
+		err = platform_roothub_add_phy(dev, port_np, "usb3-phy",
+					       &plat_roothub->list);
+		if (err)
+			goto err_out;
+	}
+
+	head = &plat_roothub->list;
+
+	list_for_each_entry(roothub_entry, head, list) {
+		err = phy_init(roothub_entry->phy);
+		if (err)
+			goto err_exit_phys;
+	}
+
+	return plat_roothub;
+
+err_exit_phys:
+	list_for_each_entry_continue_reverse(roothub_entry, head, list)
+		phy_exit(roothub_entry->phy);
+
+err_out:
+	return ERR_PTR(err);
+}
+EXPORT_SYMBOL_GPL(platform_roothub_init);
+
+int platform_roothub_exit(struct platform_roothub *plat_roothub)
+{
+	struct platform_roothub *roothub_entry;
+	struct list_head *head;
+	int err, ret = 0;
+
+	if (!plat_roothub)
+		return 0;
+
+	head = &plat_roothub->list;
+
+	list_for_each_entry(roothub_entry, head, list) {
+		err = phy_exit(roothub_entry->phy);
+		if (err)
+			ret = ret;
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(platform_roothub_exit);
+
+int platform_roothub_power_on(struct platform_roothub *plat_roothub)
+{
+	struct platform_roothub *roothub_entry;
+	struct list_head *head;
+	int err;
+
+	if (!plat_roothub)
+		return 0;
+
+	head = &plat_roothub->list;
+
+	list_for_each_entry(roothub_entry, head, list) {
+		err = phy_power_on(roothub_entry->phy);
+		if (err)
+			goto err_out;
+	}
+
+	return 0;
+
+err_out:
+	list_for_each_entry_continue_reverse(roothub_entry, head, list)
+		phy_power_off(roothub_entry->phy);
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(platform_roothub_power_on);
+
+int platform_roothub_power_off(struct platform_roothub *plat_roothub)
+{
+	struct platform_roothub *roothub_entry;
+	int err, ret = 0;
+
+	if (!plat_roothub)
+		return 0;
+
+	list_for_each_entry_reverse(roothub_entry, &plat_roothub->list, list) {
+		err = phy_power_off(roothub_entry->phy);
+		if (err)
+			ret = err;
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(platform_roothub_power_off);
diff --git a/drivers/usb/host/platform-roothub.h b/drivers/usb/host/platform-roothub.h
new file mode 100644
index 000000000000..0b801da66918
--- /dev/null
+++ b/drivers/usb/host/platform-roothub.h
@@ -0,0 +1,12 @@
+#ifndef USB_HOST_PLATFORM_ROOTHUB_H
+#define USB_HOST_PLATFORM_ROOTHUB_H
+
+struct platform_roothub;
+
+struct platform_roothub *platform_roothub_init(struct device *dev);
+int platform_roothub_exit(struct platform_roothub *plat_roothub);
+
+int platform_roothub_power_on(struct platform_roothub *plat_roothub);
+int platform_roothub_power_off(struct platform_roothub *plat_roothub);
+
+#endif /* USB_HOST_PLATFORM_ROOTHUB_H */
-- 
2.14.1

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

[PATCH v4 2/3] usb: host: add a generic platform USB roothub driver

[Martin Blumenstingl]

Many SoC platforms have separate devices for the USB PHY which are
registered through the generic PHY framework. These PHYs have to be
enabled to make the USB controller actually work. They also have to be
disabled again on shutdown/suspend.

Currently (at least) the following HCI platform drivers are using custom
code to obtain all PHYs via devicetree for the roothub/controller and
disable/enable them when required:
- ehci-platform.c has ehci_platform_power_{on,off}
- xhci-mtk.c has xhci_mtk_phy_{init,exit,power_on,power_off}
- ohci-platform.c has ohci_platform_power_{on,off}

These drivers are not using the generic devicetree USB device bindings
yet which were only introduced recently (documentation is available in
devicetree/bindings/usb/usb-device.txt).
With this new driver the usb2-phy and usb3-phy can be specified directly
in the child-node of the corresponding port of the roothub via
devicetree. This can be extended by not just parsing PHYs (some of the
other drivers listed above are for example also parsing a list of clocks
as well) when required.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Tested-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/host/Kconfig            |   3 +
 drivers/usb/host/Makefile           |   2 +
 drivers/usb/host/platform-roothub.c | 180 ++++++++++++++++++++++++++++++++++++
 drivers/usb/host/platform-roothub.h |  12 +++
 4 files changed, 197 insertions(+)
 create mode 100644 drivers/usb/host/platform-roothub.c
 create mode 100644 drivers/usb/host/platform-roothub.h

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index fa5692dec832..b8b05c786b2a 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -805,6 +805,9 @@ config USB_HCD_SSB
 
 	  If unsure, say N.
 
+config USB_PLATFORM_ROOTHUB
+	bool
+
 config USB_HCD_TEST_MODE
 	bool "HCD test mode support"
 	---help---
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index cf2691fffcc0..dc817f82d632 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -29,6 +29,8 @@ obj-$(CONFIG_USB_WHCI_HCD)	+= whci/
 
 obj-$(CONFIG_USB_PCI)	+= pci-quirks.o
 
+obj-$(CONFIG_USB_PLATFORM_ROOTHUB)	+= platform-roothub.o
+
 obj-$(CONFIG_USB_EHCI_HCD)	+= ehci-hcd.o
 obj-$(CONFIG_USB_EHCI_PCI)	+= ehci-pci.o
 obj-$(CONFIG_USB_EHCI_HCD_PLATFORM)	+= ehci-platform.o
diff --git a/drivers/usb/host/platform-roothub.c b/drivers/usb/host/platform-roothub.c
new file mode 100644
index 000000000000..70d2d97aa8b2
--- /dev/null
+++ b/drivers/usb/host/platform-roothub.c
@@ -0,0 +1,180 @@
+/*
+ * platform roothub driver - a virtual PHY device which passes all phy_*
+ * function calls to multiple (actual) PHY devices. This is comes handy when
+ * initializing all PHYs on a root-hub (to keep them all in the same state).
+ *
+ * Copyright (C) 2017 Martin Blumenstingl <martin.blumenstingl@googlemail.com>
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/device.h>
+#include <linux/list.h>
+#include <linux/phy/phy.h>
+#include <linux/of.h>
+#include <linux/usb/of.h>
+
+#include "platform-roothub.h"
+
+#define ROOTHUB_PORTNUM		0
+
+struct platform_roothub {
+	struct phy		*phy;
+	struct list_head	list;
+};
+
+static struct platform_roothub *platform_roothub_alloc(struct device *dev)
+{
+	struct platform_roothub *roothub_entry;
+
+	roothub_entry = devm_kzalloc(dev, sizeof(*roothub_entry), GFP_KERNEL);
+	if (!roothub_entry)
+		return ERR_PTR(-ENOMEM);
+
+	INIT_LIST_HEAD(&roothub_entry->list);
+
+	return roothub_entry;
+}
+
+static int platform_roothub_add_phy(struct device *dev,
+				    struct device_node *port_np,
+				    const char *con_id, struct list_head *list)
+{
+	struct platform_roothub *roothub_entry;
+	struct phy *phy = devm_of_phy_get(dev, port_np, con_id);
+
+	if (IS_ERR_OR_NULL(phy)) {
+		if (!phy || PTR_ERR(phy) == -ENODEV)
+			return 0;
+		else
+			return PTR_ERR(phy);
+	}
+
+	roothub_entry = platform_roothub_alloc(dev);
+	if (IS_ERR(roothub_entry))
+		return PTR_ERR(roothub_entry);
+
+	roothub_entry->phy = phy;
+
+	list_add_tail(&roothub_entry->list, list);
+
+	return 0;
+}
+
+struct platform_roothub *platform_roothub_init(struct device *dev)
+{
+	struct device_node *roothub_np, *port_np;
+	struct platform_roothub *plat_roothub;
+	struct platform_roothub *roothub_entry;
+	struct list_head *head;
+	int err;
+
+	roothub_np = usb_of_get_child_node(dev->of_node, ROOTHUB_PORTNUM);
+	if (!of_device_is_available(roothub_np))
+		return NULL;
+
+	plat_roothub = platform_roothub_alloc(dev);
+	if (IS_ERR(plat_roothub))
+		return plat_roothub;
+
+	for_each_available_child_of_node(roothub_np, port_np) {
+		err = platform_roothub_add_phy(dev, port_np, "usb2-phy",
+					       &plat_roothub->list);
+		if (err)
+			goto err_out;
+
+		err = platform_roothub_add_phy(dev, port_np, "usb3-phy",
+					       &plat_roothub->list);
+		if (err)
+			goto err_out;
+	}
+
+	head = &plat_roothub->list;
+
+	list_for_each_entry(roothub_entry, head, list) {
+		err = phy_init(roothub_entry->phy);
+		if (err)
+			goto err_exit_phys;
+	}
+
+	return plat_roothub;
+
+err_exit_phys:
+	list_for_each_entry_continue_reverse(roothub_entry, head, list)
+		phy_exit(roothub_entry->phy);
+
+err_out:
+	return ERR_PTR(err);
+}
+EXPORT_SYMBOL_GPL(platform_roothub_init);
+
+int platform_roothub_exit(struct platform_roothub *plat_roothub)
+{
+	struct platform_roothub *roothub_entry;
+	struct list_head *head;
+	int err, ret = 0;
+
+	if (!plat_roothub)
+		return 0;
+
+	head = &plat_roothub->list;
+
+	list_for_each_entry(roothub_entry, head, list) {
+		err = phy_exit(roothub_entry->phy);
+		if (err)
+			ret = ret;
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(platform_roothub_exit);
+
+int platform_roothub_power_on(struct platform_roothub *plat_roothub)
+{
+	struct platform_roothub *roothub_entry;
+	struct list_head *head;
+	int err;
+
+	if (!plat_roothub)
+		return 0;
+
+	head = &plat_roothub->list;
+
+	list_for_each_entry(roothub_entry, head, list) {
+		err = phy_power_on(roothub_entry->phy);
+		if (err)
+			goto err_out;
+	}
+
+	return 0;
+
+err_out:
+	list_for_each_entry_continue_reverse(roothub_entry, head, list)
+		phy_power_off(roothub_entry->phy);
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(platform_roothub_power_on);
+
+int platform_roothub_power_off(struct platform_roothub *plat_roothub)
+{
+	struct platform_roothub *roothub_entry;
+	int err, ret = 0;
+
+	if (!plat_roothub)
+		return 0;
+
+	list_for_each_entry_reverse(roothub_entry, &plat_roothub->list, list) {
+		err = phy_power_off(roothub_entry->phy);
+		if (err)
+			ret = err;
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(platform_roothub_power_off);
diff --git a/drivers/usb/host/platform-roothub.h b/drivers/usb/host/platform-roothub.h
new file mode 100644
index 000000000000..0b801da66918
--- /dev/null
+++ b/drivers/usb/host/platform-roothub.h
@@ -0,0 +1,12 @@
+#ifndef USB_HOST_PLATFORM_ROOTHUB_H
+#define USB_HOST_PLATFORM_ROOTHUB_H
+
+struct platform_roothub;
+
+struct platform_roothub *platform_roothub_init(struct device *dev);
+int platform_roothub_exit(struct platform_roothub *plat_roothub);
+
+int platform_roothub_power_on(struct platform_roothub *plat_roothub);
+int platform_roothub_power_off(struct platform_roothub *plat_roothub);
+
+#endif /* USB_HOST_PLATFORM_ROOTHUB_H */
-- 
2.14.1

[PATCH v4 3/3] usb: host: xhci: plat: integrate the platform-roothub

[Martin Blumenstingl]

This enables the platform-roothub for the xhci-plat driver. This allows
specifying a PHY for each port via devicetree. All PHYs will then be
enabled/disabled by the platform-roothub driver.

One example where this is required is the Amlogic GXL and GXM SoCs:
They are using a dwc3 USB controller with up to three ports enabled on
the internal roothub. Using only the top-level "phy" properties does not
work here since one can only specify one "usb2-phy" and one "usb3-phy",
while actually at least two "usb2-phy" have to be specified.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/usb/host/Kconfig     |  1 +
 drivers/usb/host/xhci-plat.c | 35 +++++++++++++++++++++++++++++++++--
 drivers/usb/host/xhci.h      |  2 ++
 3 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index b8b05c786b2a..3bdc49e89c0f 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -36,6 +36,7 @@ config USB_XHCI_PCI
 config USB_XHCI_PLATFORM
 	tristate "Generic xHCI driver for a platform device"
 	select USB_XHCI_RCAR if ARCH_RENESAS
+	select USB_PLATFORM_ROOTHUB
 	---help---
 	  Adds an xHCI host driver for a generic platform device, which
 	  provides a memory space and an irq.
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 163bafde709f..2c6ab3912443 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -21,6 +21,7 @@
 #include <linux/slab.h>
 #include <linux/acpi.h>
 
+#include "platform-roothub.h"
 #include "xhci.h"
 #include "xhci-plat.h"
 #include "xhci-mvebu.h"
@@ -277,9 +278,19 @@ static int xhci_plat_probe(struct platform_device *pdev)
 			goto put_usb3_hcd;
 	}
 
+	xhci->platform_roothub = platform_roothub_init(sysdev);
+	if (IS_ERR(xhci->platform_roothub)) {
+		ret = PTR_ERR(xhci->platform_roothub);
+		goto disable_usb_phy;
+	}
+
+	ret = platform_roothub_power_on(xhci->platform_roothub);
+	if (ret)
+		goto exit_plat_roothub;
+
 	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
 	if (ret)
-		goto disable_usb_phy;
+		goto disable_plat_roothub;
 
 	if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
 		xhci->shared_hcd->can_do_streams = 1;
@@ -303,6 +314,12 @@ static int xhci_plat_probe(struct platform_device *pdev)
 dealloc_usb2_hcd:
 	usb_remove_hcd(hcd);
 
+disable_plat_roothub:
+	platform_roothub_power_off(xhci->platform_roothub);
+
+exit_plat_roothub:
+	platform_roothub_exit(xhci->platform_roothub);
+
 disable_usb_phy:
 	usb_phy_shutdown(hcd->usb_phy);
 
@@ -334,6 +351,9 @@ static int xhci_plat_remove(struct platform_device *dev)
 	usb_remove_hcd(xhci->shared_hcd);
 	usb_phy_shutdown(hcd->usb_phy);
 
+	platform_roothub_power_off(xhci->platform_roothub);
+	platform_roothub_exit(xhci->platform_roothub);
+
 	usb_remove_hcd(hcd);
 	usb_put_hcd(xhci->shared_hcd);
 
@@ -366,7 +386,14 @@ static int __maybe_unused xhci_plat_suspend(struct device *dev)
 	if (!device_may_wakeup(dev) && !IS_ERR(xhci->clk))
 		clk_disable_unprepare(xhci->clk);
 
-	return ret;
+	if (ret)
+		return ret;
+
+	ret = platform_roothub_power_off(xhci->platform_roothub);
+	if (ret)
+		return ret;
+
+	return 0;
 }
 
 static int __maybe_unused xhci_plat_resume(struct device *dev)
@@ -378,6 +405,10 @@ static int __maybe_unused xhci_plat_resume(struct device *dev)
 	if (!device_may_wakeup(dev) && !IS_ERR(xhci->clk))
 		clk_prepare_enable(xhci->clk);
 
+	ret = platform_roothub_power_on(xhci->platform_roothub);
+	if (ret)
+		return ret;
+
 	ret = xhci_priv_resume_quirk(hcd);
 	if (ret)
 		return ret;
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 2abaa4d6d39d..c75c3247a441 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1732,6 +1732,8 @@ struct xhci_hcd {
 	int		msix_count;
 	/* optional clock */
 	struct clk		*clk;
+	/* optional platform root-hub */
+	struct platform_roothub	*platform_roothub;
 	/* data structures */
 	struct xhci_device_context_array *dcbaa;
 	struct xhci_ring	*cmd_ring;
-- 
2.14.1

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

[PATCH v4 3/3] usb: host: xhci: plat: integrate the platform-roothub

[Martin Blumenstingl]

This enables the platform-roothub for the xhci-plat driver. This allows
specifying a PHY for each port via devicetree. All PHYs will then be
enabled/disabled by the platform-roothub driver.

One example where this is required is the Amlogic GXL and GXM SoCs:
They are using a dwc3 USB controller with up to three ports enabled on
the internal roothub. Using only the top-level "phy" properties does not
work here since one can only specify one "usb2-phy" and one "usb3-phy",
while actually at least two "usb2-phy" have to be specified.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 drivers/usb/host/Kconfig     |  1 +
 drivers/usb/host/xhci-plat.c | 35 +++++++++++++++++++++++++++++++++--
 drivers/usb/host/xhci.h      |  2 ++
 3 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index b8b05c786b2a..3bdc49e89c0f 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -36,6 +36,7 @@ config USB_XHCI_PCI
 config USB_XHCI_PLATFORM
 	tristate "Generic xHCI driver for a platform device"
 	select USB_XHCI_RCAR if ARCH_RENESAS
+	select USB_PLATFORM_ROOTHUB
 	---help---
 	  Adds an xHCI host driver for a generic platform device, which
 	  provides a memory space and an irq.
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 163bafde709f..2c6ab3912443 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -21,6 +21,7 @@
 #include <linux/slab.h>
 #include <linux/acpi.h>
 
+#include "platform-roothub.h"
 #include "xhci.h"
 #include "xhci-plat.h"
 #include "xhci-mvebu.h"
@@ -277,9 +278,19 @@ static int xhci_plat_probe(struct platform_device *pdev)
 			goto put_usb3_hcd;
 	}
 
+	xhci->platform_roothub = platform_roothub_init(sysdev);
+	if (IS_ERR(xhci->platform_roothub)) {
+		ret = PTR_ERR(xhci->platform_roothub);
+		goto disable_usb_phy;
+	}
+
+	ret = platform_roothub_power_on(xhci->platform_roothub);
+	if (ret)
+		goto exit_plat_roothub;
+
 	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
 	if (ret)
-		goto disable_usb_phy;
+		goto disable_plat_roothub;
 
 	if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
 		xhci->shared_hcd->can_do_streams = 1;
@@ -303,6 +314,12 @@ static int xhci_plat_probe(struct platform_device *pdev)
 dealloc_usb2_hcd:
 	usb_remove_hcd(hcd);
 
+disable_plat_roothub:
+	platform_roothub_power_off(xhci->platform_roothub);
+
+exit_plat_roothub:
+	platform_roothub_exit(xhci->platform_roothub);
+
 disable_usb_phy:
 	usb_phy_shutdown(hcd->usb_phy);
 
@@ -334,6 +351,9 @@ static int xhci_plat_remove(struct platform_device *dev)
 	usb_remove_hcd(xhci->shared_hcd);
 	usb_phy_shutdown(hcd->usb_phy);
 
+	platform_roothub_power_off(xhci->platform_roothub);
+	platform_roothub_exit(xhci->platform_roothub);
+
 	usb_remove_hcd(hcd);
 	usb_put_hcd(xhci->shared_hcd);
 
@@ -366,7 +386,14 @@ static int __maybe_unused xhci_plat_suspend(struct device *dev)
 	if (!device_may_wakeup(dev) && !IS_ERR(xhci->clk))
 		clk_disable_unprepare(xhci->clk);
 
-	return ret;
+	if (ret)
+		return ret;
+
+	ret = platform_roothub_power_off(xhci->platform_roothub);
+	if (ret)
+		return ret;
+
+	return 0;
 }
 
 static int __maybe_unused xhci_plat_resume(struct device *dev)
@@ -378,6 +405,10 @@ static int __maybe_unused xhci_plat_resume(struct device *dev)
 	if (!device_may_wakeup(dev) && !IS_ERR(xhci->clk))
 		clk_prepare_enable(xhci->clk);
 
+	ret = platform_roothub_power_on(xhci->platform_roothub);
+	if (ret)
+		return ret;
+
 	ret = xhci_priv_resume_quirk(hcd);
 	if (ret)
 		return ret;
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 2abaa4d6d39d..c75c3247a441 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1732,6 +1732,8 @@ struct xhci_hcd {
 	int		msix_count;
 	/* optional clock */
 	struct clk		*clk;
+	/* optional platform root-hub */
+	struct platform_roothub	*platform_roothub;
 	/* data structures */
 	struct xhci_device_context_array *dcbaa;
 	struct xhci_ring	*cmd_ring;
-- 
2.14.1

Re: [PATCH v4 0/3] initialize (multiple) PHYs in xhci-plat

[Martin Blumenstingl]

Hello Mathias, Hello Greg,

On Sun, Sep 3, 2017 at 11:38 PM, Martin Blumenstingl
<martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
> This series is the outcome of a discussion with Felipe Balbi,
> see [0] and [1].
> The quick-summary of this is:
> - dwc3 already takes one USB2 and one USB3 PHY and initializes these
>   correct
> - some other HCI platform drivers (like ehci-platform.c, xhci-mtk.c and
>   ohci-platform.c) do not have a limitation on the number of PHYs - they
>   support one PHY per actual host port
> - Amlogic Meson GXL and GXM SoCs come with a dwc3 IP block which has two
>   or three USB2 ports enabled on the internal root-hub. The SoCs also
>   provide separate USB2 PHYs, one per port. All USB2 PHYs (which are
>   internally "connected" to the dwc3 roothub) need to be powered on,
>   otherwise USB devices cannot be enumerated (even if just one PHY is
>   disabled and if the device is plugged into another, enabled port)
>
> In my first attempt to get USB supported on the GXL and GXM SoCs I tried
> to work-around the problem that I could not pass multiple PHYs to the
> dwc3 controller.
> This was rejected by Rob Herring (which was definitely the thing to do in
> my opinion), see [2]
>
> This series adds a new "platform-roothub". This can be configured through
> devicetree by passing a child-node with "reg = <0>" to the USB
> controller. Additionally there has to be a child-node for each port on
> the root-hub. Each of the child-nodes takes a "phys" and "phy-names"
> property. This allows modeling the root-hub in devicetree similar to the
> USB device binding (documented in devicetree/bindings/usb/usb-device.txt)
> This avoids and backwards-compatibility problems (which was a concern
> regardless of the solution, see [3]) since the binding for the root-hub
> was previously not specified (and we're not using the "phys" property of
> the controller, which might have served different purposes before,
> depending on the drivers).
>
> Additionally this integrates the new platform-roothub into xhci-plat.c
> which automatically enables it for the dwc3 driver (in host-mode).
>
>
> Changes since RFCv3 at [6]:
> - moved the DT binding change from patch #3 to patch #1 as suggested
>   by Rob Herring (and slightly adjusted the commit message to account
>   for that)
> - added Tested-by from Chunfeng Yun (who confirmed that the whole
>   concept and implementation works fine on Mediatek SoCs - many thanks
>   again!) to patch #2
> - added Rob Herring's ACK to patches 1 and 3
> - dropped RFC status (RFCv3 -> PATCH v4)
I just wanted to rebase this to v4.14-rc1 (now that this is out) -
however I noticed that v4 still applies to v4.14-rc1 cleanly (the
patches are still identical to v4 after rebasing).

we have an ACK from the devicetree maintainers and a "Tested-by" for a
Mediatek (= non-Amlogic) SoC.
I already have patches for the Amlogic GXL/GXM platforms queued, those
are just waiting on this series.

what is still missing to get this series into v4.15?

> Changes since RFCv2 at [5]:
> - split phy_{init,exit} and phy_power_{on,off} handling. up until RFCv2
>   I called phy_init plus phy_power_on in platform_roothub_power_on and
>   phy_power_off plus phy_exit in platform_roothub_power_off. However,
>   Chunfeng Yun (a Mediatek SoC developer - many thanks for testing my
>   series and providing great feedback) reported that only using
>   phy_power_off (and omitting phy_exit) during system suspend fixes an
>   issue where USB devices would be re-enumerated when resuming. His
>   original problem description: "In order to keep link state on mt8173,
>   we just power off all phys(not exit) when system enter suspend, then
>   power on them again (needn't init, otherwise device will be
>   disconnected) when system resume, this can avoid re-enumerating
>   device.". This fix affects patch #2 and #3 as we now have
>   platform_roothub_init (which calls phy_init internally),
>   platform_roothub_power_on (which calls phy_power_on internally),
>   platform_roothub_power_off (which calls phy_power_off internally) and
>   platform_roothub_exit (which calls phy_exit internally). suspend and
>   resume only call platform_roothub_power_{on,off} to prevent the issue
>   described by Chunfeng Yun (unfortunately I cannot test this because
>   the Amlogic platform currently does not support system suspend).
> - dropped two struct forward declarations from platform-roothub.h which
>   are not used in the header file (thanks to Chunfeng Yun for spotting
>   this)
>
> Changes since RFCv1 at [4]:
> - split the usb-xhci dt-binding documentation into a separate patch
> - fixed a typo ("usb-phy" -> "phys" in the dt-binding example)
> - rebased to apply against latest usb-next
>
>
> [0] http://lists.infradead.org/pipermail/linux-amlogic/2017-January/001945.html
> [1] http://lists.infradead.org/pipermail/linux-amlogic/2017-January/001947.html
> [2] http://lists.infradead.org/pipermail/linux-amlogic/2016-November/001818.html
> [3] http://lists.infradead.org/pipermail/linux-amlogic/2017-January/001948.html
> [4] http://marc.info/?l=linux-usb&m=148414866303604&w=2
> [5] https://www.spinics.net/lists/linux-usb/msg158967.html
> [6] https://www.spinics.net/lists/devicetree/msg190426.html
>
> Martin Blumenstingl (3):
>   dt-bindings: usb: add the documentation for USB root-hub
>   usb: host: add a generic platform USB roothub driver
>   usb: host: xhci: plat: integrate the platform-roothub
>
>  .../devicetree/bindings/usb/usb-roothub.txt        |  46 ++++++
>  Documentation/devicetree/bindings/usb/usb-xhci.txt |   7 +
>  drivers/usb/host/Kconfig                           |   4 +
>  drivers/usb/host/Makefile                          |   2 +
>  drivers/usb/host/platform-roothub.c                | 180 +++++++++++++++++++++
>  drivers/usb/host/platform-roothub.h                |  12 ++
>  drivers/usb/host/xhci-plat.c                       |  35 +++-
>  drivers/usb/host/xhci.h                            |   2 +
>  8 files changed, 286 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/usb/usb-roothub.txt
>  create mode 100644 drivers/usb/host/platform-roothub.c
>  create mode 100644 drivers/usb/host/platform-roothub.h
>
> --
> 2.14.1
>

Have a great week!
Regards,
Martin
--
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 v4 0/3] initialize (multiple) PHYs in xhci-plat

[Martin Blumenstingl]

Hello Mathias, Hello Greg,

On Sun, Sep 3, 2017 at 11:38 PM, Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
> This series is the outcome of a discussion with Felipe Balbi,
> see [0] and [1].
> The quick-summary of this is:
> - dwc3 already takes one USB2 and one USB3 PHY and initializes these
>   correct
> - some other HCI platform drivers (like ehci-platform.c, xhci-mtk.c and
>   ohci-platform.c) do not have a limitation on the number of PHYs - they
>   support one PHY per actual host port
> - Amlogic Meson GXL and GXM SoCs come with a dwc3 IP block which has two
>   or three USB2 ports enabled on the internal root-hub. The SoCs also
>   provide separate USB2 PHYs, one per port. All USB2 PHYs (which are
>   internally "connected" to the dwc3 roothub) need to be powered on,
>   otherwise USB devices cannot be enumerated (even if just one PHY is
>   disabled and if the device is plugged into another, enabled port)
>
> In my first attempt to get USB supported on the GXL and GXM SoCs I tried
> to work-around the problem that I could not pass multiple PHYs to the
> dwc3 controller.
> This was rejected by Rob Herring (which was definitely the thing to do in
> my opinion), see [2]
>
> This series adds a new "platform-roothub". This can be configured through
> devicetree by passing a child-node with "reg = <0>" to the USB
> controller. Additionally there has to be a child-node for each port on
> the root-hub. Each of the child-nodes takes a "phys" and "phy-names"
> property. This allows modeling the root-hub in devicetree similar to the
> USB device binding (documented in devicetree/bindings/usb/usb-device.txt)
> This avoids and backwards-compatibility problems (which was a concern
> regardless of the solution, see [3]) since the binding for the root-hub
> was previously not specified (and we're not using the "phys" property of
> the controller, which might have served different purposes before,
> depending on the drivers).
>
> Additionally this integrates the new platform-roothub into xhci-plat.c
> which automatically enables it for the dwc3 driver (in host-mode).
>
>
> Changes since RFCv3 at [6]:
> - moved the DT binding change from patch #3 to patch #1 as suggested
>   by Rob Herring (and slightly adjusted the commit message to account
>   for that)
> - added Tested-by from Chunfeng Yun (who confirmed that the whole
>   concept and implementation works fine on Mediatek SoCs - many thanks
>   again!) to patch #2
> - added Rob Herring's ACK to patches 1 and 3
> - dropped RFC status (RFCv3 -> PATCH v4)
I just wanted to rebase this to v4.14-rc1 (now that this is out) -
however I noticed that v4 still applies to v4.14-rc1 cleanly (the
patches are still identical to v4 after rebasing).

we have an ACK from the devicetree maintainers and a "Tested-by" for a
Mediatek (= non-Amlogic) SoC.
I already have patches for the Amlogic GXL/GXM platforms queued, those
are just waiting on this series.

what is still missing to get this series into v4.15?

> Changes since RFCv2 at [5]:
> - split phy_{init,exit} and phy_power_{on,off} handling. up until RFCv2
>   I called phy_init plus phy_power_on in platform_roothub_power_on and
>   phy_power_off plus phy_exit in platform_roothub_power_off. However,
>   Chunfeng Yun (a Mediatek SoC developer - many thanks for testing my
>   series and providing great feedback) reported that only using
>   phy_power_off (and omitting phy_exit) during system suspend fixes an
>   issue where USB devices would be re-enumerated when resuming. His
>   original problem description: "In order to keep link state on mt8173,
>   we just power off all phys(not exit) when system enter suspend, then
>   power on them again (needn't init, otherwise device will be
>   disconnected) when system resume, this can avoid re-enumerating
>   device.". This fix affects patch #2 and #3 as we now have
>   platform_roothub_init (which calls phy_init internally),
>   platform_roothub_power_on (which calls phy_power_on internally),
>   platform_roothub_power_off (which calls phy_power_off internally) and
>   platform_roothub_exit (which calls phy_exit internally). suspend and
>   resume only call platform_roothub_power_{on,off} to prevent the issue
>   described by Chunfeng Yun (unfortunately I cannot test this because
>   the Amlogic platform currently does not support system suspend).
> - dropped two struct forward declarations from platform-roothub.h which
>   are not used in the header file (thanks to Chunfeng Yun for spotting
>   this)
>
> Changes since RFCv1 at [4]:
> - split the usb-xhci dt-binding documentation into a separate patch
> - fixed a typo ("usb-phy" -> "phys" in the dt-binding example)
> - rebased to apply against latest usb-next
>
>
> [0] http://lists.infradead.org/pipermail/linux-amlogic/2017-January/001945.html
> [1] http://lists.infradead.org/pipermail/linux-amlogic/2017-January/001947.html
> [2] http://lists.infradead.org/pipermail/linux-amlogic/2016-November/001818.html
> [3] http://lists.infradead.org/pipermail/linux-amlogic/2017-January/001948.html
> [4] http://marc.info/?l=linux-usb&m=148414866303604&w=2
> [5] https://www.spinics.net/lists/linux-usb/msg158967.html
> [6] https://www.spinics.net/lists/devicetree/msg190426.html
>
> Martin Blumenstingl (3):
>   dt-bindings: usb: add the documentation for USB root-hub
>   usb: host: add a generic platform USB roothub driver
>   usb: host: xhci: plat: integrate the platform-roothub
>
>  .../devicetree/bindings/usb/usb-roothub.txt        |  46 ++++++
>  Documentation/devicetree/bindings/usb/usb-xhci.txt |   7 +
>  drivers/usb/host/Kconfig                           |   4 +
>  drivers/usb/host/Makefile                          |   2 +
>  drivers/usb/host/platform-roothub.c                | 180 +++++++++++++++++++++
>  drivers/usb/host/platform-roothub.h                |  12 ++
>  drivers/usb/host/xhci-plat.c                       |  35 +++-
>  drivers/usb/host/xhci.h                            |   2 +
>  8 files changed, 286 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/usb/usb-roothub.txt
>  create mode 100644 drivers/usb/host/platform-roothub.c
>  create mode 100644 drivers/usb/host/platform-roothub.h
>
> --
> 2.14.1
>

Have a great week!
Regards,
Martin

Re: [PATCH v4 0/3] initialize (multiple) PHYs in xhci-plat

[Greg KH]

On Sun, Sep 17, 2017 at 10:51:31PM +0200, Martin Blumenstingl wrote:
> Hello Mathias, Hello Greg,
> 
> On Sun, Sep 3, 2017 at 11:38 PM, Martin Blumenstingl
> <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
> > This series is the outcome of a discussion with Felipe Balbi,
> > see [0] and [1].
> > The quick-summary of this is:
> > - dwc3 already takes one USB2 and one USB3 PHY and initializes these
> >   correct
> > - some other HCI platform drivers (like ehci-platform.c, xhci-mtk.c and
> >   ohci-platform.c) do not have a limitation on the number of PHYs - they
> >   support one PHY per actual host port
> > - Amlogic Meson GXL and GXM SoCs come with a dwc3 IP block which has two
> >   or three USB2 ports enabled on the internal root-hub. The SoCs also
> >   provide separate USB2 PHYs, one per port. All USB2 PHYs (which are
> >   internally "connected" to the dwc3 roothub) need to be powered on,
> >   otherwise USB devices cannot be enumerated (even if just one PHY is
> >   disabled and if the device is plugged into another, enabled port)
> >
> > In my first attempt to get USB supported on the GXL and GXM SoCs I tried
> > to work-around the problem that I could not pass multiple PHYs to the
> > dwc3 controller.
> > This was rejected by Rob Herring (which was definitely the thing to do in
> > my opinion), see [2]
> >
> > This series adds a new "platform-roothub". This can be configured through
> > devicetree by passing a child-node with "reg = <0>" to the USB
> > controller. Additionally there has to be a child-node for each port on
> > the root-hub. Each of the child-nodes takes a "phys" and "phy-names"
> > property. This allows modeling the root-hub in devicetree similar to the
> > USB device binding (documented in devicetree/bindings/usb/usb-device.txt)
> > This avoids and backwards-compatibility problems (which was a concern
> > regardless of the solution, see [3]) since the binding for the root-hub
> > was previously not specified (and we're not using the "phys" property of
> > the controller, which might have served different purposes before,
> > depending on the drivers).
> >
> > Additionally this integrates the new platform-roothub into xhci-plat.c
> > which automatically enables it for the dwc3 driver (in host-mode).
> >
> >
> > Changes since RFCv3 at [6]:
> > - moved the DT binding change from patch #3 to patch #1 as suggested
> >   by Rob Herring (and slightly adjusted the commit message to account
> >   for that)
> > - added Tested-by from Chunfeng Yun (who confirmed that the whole
> >   concept and implementation works fine on Mediatek SoCs - many thanks
> >   again!) to patch #2
> > - added Rob Herring's ACK to patches 1 and 3
> > - dropped RFC status (RFCv3 -> PATCH v4)
> I just wanted to rebase this to v4.14-rc1 (now that this is out) -
> however I noticed that v4 still applies to v4.14-rc1 cleanly (the
> patches are still identical to v4 after rebasing).
> 
> we have an ACK from the devicetree maintainers and a "Tested-by" for a
> Mediatek (= non-Amlogic) SoC.
> I already have patches for the Amlogic GXL/GXM platforms queued, those
> are just waiting on this series.
> 
> what is still missing to get this series into v4.15?

Well, we couldn't do anything until 4.14-rc1 is out, now that it is, let
us catch up on patch review please...

thanks,

greg k-h
--
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 v4 0/3] initialize (multiple) PHYs in xhci-plat

[Greg KH]

On Sun, Sep 17, 2017 at 10:51:31PM +0200, Martin Blumenstingl wrote:
> Hello Mathias, Hello Greg,
> 
> On Sun, Sep 3, 2017 at 11:38 PM, Martin Blumenstingl
> <martin.blumenstingl@googlemail.com> wrote:
> > This series is the outcome of a discussion with Felipe Balbi,
> > see [0] and [1].
> > The quick-summary of this is:
> > - dwc3 already takes one USB2 and one USB3 PHY and initializes these
> >   correct
> > - some other HCI platform drivers (like ehci-platform.c, xhci-mtk.c and
> >   ohci-platform.c) do not have a limitation on the number of PHYs - they
> >   support one PHY per actual host port
> > - Amlogic Meson GXL and GXM SoCs come with a dwc3 IP block which has two
> >   or three USB2 ports enabled on the internal root-hub. The SoCs also
> >   provide separate USB2 PHYs, one per port. All USB2 PHYs (which are
> >   internally "connected" to the dwc3 roothub) need to be powered on,
> >   otherwise USB devices cannot be enumerated (even if just one PHY is
> >   disabled and if the device is plugged into another, enabled port)
> >
> > In my first attempt to get USB supported on the GXL and GXM SoCs I tried
> > to work-around the problem that I could not pass multiple PHYs to the
> > dwc3 controller.
> > This was rejected by Rob Herring (which was definitely the thing to do in
> > my opinion), see [2]
> >
> > This series adds a new "platform-roothub". This can be configured through
> > devicetree by passing a child-node with "reg = <0>" to the USB
> > controller. Additionally there has to be a child-node for each port on
> > the root-hub. Each of the child-nodes takes a "phys" and "phy-names"
> > property. This allows modeling the root-hub in devicetree similar to the
> > USB device binding (documented in devicetree/bindings/usb/usb-device.txt)
> > This avoids and backwards-compatibility problems (which was a concern
> > regardless of the solution, see [3]) since the binding for the root-hub
> > was previously not specified (and we're not using the "phys" property of
> > the controller, which might have served different purposes before,
> > depending on the drivers).
> >
> > Additionally this integrates the new platform-roothub into xhci-plat.c
> > which automatically enables it for the dwc3 driver (in host-mode).
> >
> >
> > Changes since RFCv3 at [6]:
> > - moved the DT binding change from patch #3 to patch #1 as suggested
> >   by Rob Herring (and slightly adjusted the commit message to account
> >   for that)
> > - added Tested-by from Chunfeng Yun (who confirmed that the whole
> >   concept and implementation works fine on Mediatek SoCs - many thanks
> >   again!) to patch #2
> > - added Rob Herring's ACK to patches 1 and 3
> > - dropped RFC status (RFCv3 -> PATCH v4)
> I just wanted to rebase this to v4.14-rc1 (now that this is out) -
> however I noticed that v4 still applies to v4.14-rc1 cleanly (the
> patches are still identical to v4 after rebasing).
> 
> we have an ACK from the devicetree maintainers and a "Tested-by" for a
> Mediatek (= non-Amlogic) SoC.
> I already have patches for the Amlogic GXL/GXM platforms queued, those
> are just waiting on this series.
> 
> what is still missing to get this series into v4.15?

Well, we couldn't do anything until 4.14-rc1 is out, now that it is, let
us catch up on patch review please...

thanks,

greg k-h

Re: [PATCH v4 0/3] initialize (multiple) PHYs in xhci-plat

[Martin Blumenstingl]

Hello Greg, Hello Mathias,

On Mon, Sep 18, 2017 at 10:49 AM, Greg KH <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> wrote:
> On Sun, Sep 17, 2017 at 10:51:31PM +0200, Martin Blumenstingl wrote:
>> Hello Mathias, Hello Greg,
>>
>> On Sun, Sep 3, 2017 at 11:38 PM, Martin Blumenstingl
>> <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
>> > This series is the outcome of a discussion with Felipe Balbi,
>> > see [0] and [1].
>> > The quick-summary of this is:
>> > - dwc3 already takes one USB2 and one USB3 PHY and initializes these
>> >   correct
>> > - some other HCI platform drivers (like ehci-platform.c, xhci-mtk.c and
>> >   ohci-platform.c) do not have a limitation on the number of PHYs - they
>> >   support one PHY per actual host port
>> > - Amlogic Meson GXL and GXM SoCs come with a dwc3 IP block which has two
>> >   or three USB2 ports enabled on the internal root-hub. The SoCs also
>> >   provide separate USB2 PHYs, one per port. All USB2 PHYs (which are
>> >   internally "connected" to the dwc3 roothub) need to be powered on,
>> >   otherwise USB devices cannot be enumerated (even if just one PHY is
>> >   disabled and if the device is plugged into another, enabled port)
>> >
>> > In my first attempt to get USB supported on the GXL and GXM SoCs I tried
>> > to work-around the problem that I could not pass multiple PHYs to the
>> > dwc3 controller.
>> > This was rejected by Rob Herring (which was definitely the thing to do in
>> > my opinion), see [2]
>> >
>> > This series adds a new "platform-roothub". This can be configured through
>> > devicetree by passing a child-node with "reg = <0>" to the USB
>> > controller. Additionally there has to be a child-node for each port on
>> > the root-hub. Each of the child-nodes takes a "phys" and "phy-names"
>> > property. This allows modeling the root-hub in devicetree similar to the
>> > USB device binding (documented in devicetree/bindings/usb/usb-device.txt)
>> > This avoids and backwards-compatibility problems (which was a concern
>> > regardless of the solution, see [3]) since the binding for the root-hub
>> > was previously not specified (and we're not using the "phys" property of
>> > the controller, which might have served different purposes before,
>> > depending on the drivers).
>> >
>> > Additionally this integrates the new platform-roothub into xhci-plat.c
>> > which automatically enables it for the dwc3 driver (in host-mode).
>> >
>> >
>> > Changes since RFCv3 at [6]:
>> > - moved the DT binding change from patch #3 to patch #1 as suggested
>> >   by Rob Herring (and slightly adjusted the commit message to account
>> >   for that)
>> > - added Tested-by from Chunfeng Yun (who confirmed that the whole
>> >   concept and implementation works fine on Mediatek SoCs - many thanks
>> >   again!) to patch #2
>> > - added Rob Herring's ACK to patches 1 and 3
>> > - dropped RFC status (RFCv3 -> PATCH v4)
>> I just wanted to rebase this to v4.14-rc1 (now that this is out) -
>> however I noticed that v4 still applies to v4.14-rc1 cleanly (the
>> patches are still identical to v4 after rebasing).
>>
>> we have an ACK from the devicetree maintainers and a "Tested-by" for a
>> Mediatek (= non-Amlogic) SoC.
>> I already have patches for the Amlogic GXL/GXM platforms queued, those
>> are just waiting on this series.
>>
>> what is still missing to get this series into v4.15?
>
> Well, we couldn't do anything until 4.14-rc1 is out, now that it is, let
> us catch up on patch review please...
OK, I understand that.
please let me know once you've caught up with the review backlog - as
I said I would like to get this into 4.15 if nothing else comes up
during the code-review


Thank you!
Regards,
Martin
--
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

[PATCH v4 0/3] initialize (multiple) PHYs in xhci-plat

[Martin Blumenstingl]

Hello Greg, Hello Mathias,

On Mon, Sep 18, 2017 at 10:49 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Sun, Sep 17, 2017 at 10:51:31PM +0200, Martin Blumenstingl wrote:
>> Hello Mathias, Hello Greg,
>>
>> On Sun, Sep 3, 2017 at 11:38 PM, Martin Blumenstingl
>> <martin.blumenstingl@googlemail.com> wrote:
>> > This series is the outcome of a discussion with Felipe Balbi,
>> > see [0] and [1].
>> > The quick-summary of this is:
>> > - dwc3 already takes one USB2 and one USB3 PHY and initializes these
>> >   correct
>> > - some other HCI platform drivers (like ehci-platform.c, xhci-mtk.c and
>> >   ohci-platform.c) do not have a limitation on the number of PHYs - they
>> >   support one PHY per actual host port
>> > - Amlogic Meson GXL and GXM SoCs come with a dwc3 IP block which has two
>> >   or three USB2 ports enabled on the internal root-hub. The SoCs also
>> >   provide separate USB2 PHYs, one per port. All USB2 PHYs (which are
>> >   internally "connected" to the dwc3 roothub) need to be powered on,
>> >   otherwise USB devices cannot be enumerated (even if just one PHY is
>> >   disabled and if the device is plugged into another, enabled port)
>> >
>> > In my first attempt to get USB supported on the GXL and GXM SoCs I tried
>> > to work-around the problem that I could not pass multiple PHYs to the
>> > dwc3 controller.
>> > This was rejected by Rob Herring (which was definitely the thing to do in
>> > my opinion), see [2]
>> >
>> > This series adds a new "platform-roothub". This can be configured through
>> > devicetree by passing a child-node with "reg = <0>" to the USB
>> > controller. Additionally there has to be a child-node for each port on
>> > the root-hub. Each of the child-nodes takes a "phys" and "phy-names"
>> > property. This allows modeling the root-hub in devicetree similar to the
>> > USB device binding (documented in devicetree/bindings/usb/usb-device.txt)
>> > This avoids and backwards-compatibility problems (which was a concern
>> > regardless of the solution, see [3]) since the binding for the root-hub
>> > was previously not specified (and we're not using the "phys" property of
>> > the controller, which might have served different purposes before,
>> > depending on the drivers).
>> >
>> > Additionally this integrates the new platform-roothub into xhci-plat.c
>> > which automatically enables it for the dwc3 driver (in host-mode).
>> >
>> >
>> > Changes since RFCv3 at [6]:
>> > - moved the DT binding change from patch #3 to patch #1 as suggested
>> >   by Rob Herring (and slightly adjusted the commit message to account
>> >   for that)
>> > - added Tested-by from Chunfeng Yun (who confirmed that the whole
>> >   concept and implementation works fine on Mediatek SoCs - many thanks
>> >   again!) to patch #2
>> > - added Rob Herring's ACK to patches 1 and 3
>> > - dropped RFC status (RFCv3 -> PATCH v4)
>> I just wanted to rebase this to v4.14-rc1 (now that this is out) -
>> however I noticed that v4 still applies to v4.14-rc1 cleanly (the
>> patches are still identical to v4 after rebasing).
>>
>> we have an ACK from the devicetree maintainers and a "Tested-by" for a
>> Mediatek (= non-Amlogic) SoC.
>> I already have patches for the Amlogic GXL/GXM platforms queued, those
>> are just waiting on this series.
>>
>> what is still missing to get this series into v4.15?
>
> Well, we couldn't do anything until 4.14-rc1 is out, now that it is, let
> us catch up on patch review please...
OK, I understand that.
please let me know once you've caught up with the review backlog - as
I said I would like to get this into 4.15 if nothing else comes up
during the code-review


Thank you!
Regards,
Martin

Re: [PATCH v4 0/3] initialize (multiple) PHYs in xhci-plat

[Jerome Brunet]

On Sun, 2017-10-01 at 22:32 +0200, Martin Blumenstingl wrote:
> Hello Greg, Hello Mathias,
> 
> On Mon, Sep 18, 2017 at 10:49 AM, Greg KH <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> wrote:
> > On Sun, Sep 17, 2017 at 10:51:31PM +0200, Martin Blumenstingl wrote:
> > > Hello Mathias, Hello Greg,
> > > 
> > > On Sun, Sep 3, 2017 at 11:38 PM, Martin Blumenstingl
> > > <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
> > > > This series is the outcome of a discussion with Felipe Balbi,
> > > > see [0] and [1].
> > > > The quick-summary of this is:
> > > > - dwc3 already takes one USB2 and one USB3 PHY and initializes these
> > > >   correct
> > > > - some other HCI platform drivers (like ehci-platform.c, xhci-mtk.c and
> > > >   ohci-platform.c) do not have a limitation on the number of PHYs - they
> > > >   support one PHY per actual host port
> > > > - Amlogic Meson GXL and GXM SoCs come with a dwc3 IP block which has two
> > > >   or three USB2 ports enabled on the internal root-hub. The SoCs also
> > > >   provide separate USB2 PHYs, one per port. All USB2 PHYs (which are
> > > >   internally "connected" to the dwc3 roothub) need to be powered on,
> > > >   otherwise USB devices cannot be enumerated (even if just one PHY is
> > > >   disabled and if the device is plugged into another, enabled port)
> > > > 
> > > > In my first attempt to get USB supported on the GXL and GXM SoCs I tried
> > > > to work-around the problem that I could not pass multiple PHYs to the
> > > > dwc3 controller.
> > > > This was rejected by Rob Herring (which was definitely the thing to do
> > > > in
> > > > my opinion), see [2]
> > > > 
> > > > This series adds a new "platform-roothub". This can be configured
> > > > through
> > > > devicetree by passing a child-node with "reg = <0>" to the USB
> > > > controller. Additionally there has to be a child-node for each port on
> > > > the root-hub. Each of the child-nodes takes a "phys" and "phy-names"
> > > > property. This allows modeling the root-hub in devicetree similar to the
> > > > USB device binding (documented in devicetree/bindings/usb/usb-
> > > > device.txt)
> > > > This avoids and backwards-compatibility problems (which was a concern
> > > > regardless of the solution, see [3]) since the binding for the root-hub
> > > > was previously not specified (and we're not using the "phys" property of
> > > > the controller, which might have served different purposes before,
> > > > depending on the drivers).
> > > > 
> > > > Additionally this integrates the new platform-roothub into xhci-plat.c
> > > > which automatically enables it for the dwc3 driver (in host-mode).
> > > > 
> > > > 
> > > > Changes since RFCv3 at [6]:
> > > > - moved the DT binding change from patch #3 to patch #1 as suggested
> > > >   by Rob Herring (and slightly adjusted the commit message to account
> > > >   for that)
> > > > - added Tested-by from Chunfeng Yun (who confirmed that the whole
> > > >   concept and implementation works fine on Mediatek SoCs - many thanks
> > > >   again!) to patch #2
> > > > - added Rob Herring's ACK to patches 1 and 3
> > > > - dropped RFC status (RFCv3 -> PATCH v4)
> > > 
> > > I just wanted to rebase this to v4.14-rc1 (now that this is out) -
> > > however I noticed that v4 still applies to v4.14-rc1 cleanly (the
> > > patches are still identical to v4 after rebasing).
> > > 
> > > we have an ACK from the devicetree maintainers and a "Tested-by" for a
> > > Mediatek (= non-Amlogic) SoC.
> > > I already have patches for the Amlogic GXL/GXM platforms queued, those
> > > are just waiting on this series.
> > > 
> > > what is still missing to get this series into v4.15?
> > 
> > Well, we couldn't do anything until 4.14-rc1 is out, now that it is, let
> > us catch up on patch review please...
> 
> OK, I understand that.
> please let me know once you've caught up with the review backlog - as
> I said I would like to get this into 4.15 if nothing else comes up
> during the code-review

This series works well on the libretech-cc (le potato)
For the series:

Tested-by: Jerome Brunet <jbrunet-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>


> 
> 
> Thank you!
> Regards,
> Martin
> 
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic

--
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 v4 0/3] initialize (multiple) PHYs in xhci-plat

[Jerome Brunet]

On Sun, 2017-10-01 at 22:32 +0200, Martin Blumenstingl wrote:
> Hello Greg, Hello Mathias,
> 
> On Mon, Sep 18, 2017 at 10:49 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Sun, Sep 17, 2017 at 10:51:31PM +0200, Martin Blumenstingl wrote:
> > > Hello Mathias, Hello Greg,
> > > 
> > > On Sun, Sep 3, 2017 at 11:38 PM, Martin Blumenstingl
> > > <martin.blumenstingl@googlemail.com> wrote:
> > > > This series is the outcome of a discussion with Felipe Balbi,
> > > > see [0] and [1].
> > > > The quick-summary of this is:
> > > > - dwc3 already takes one USB2 and one USB3 PHY and initializes these
> > > >   correct
> > > > - some other HCI platform drivers (like ehci-platform.c, xhci-mtk.c and
> > > >   ohci-platform.c) do not have a limitation on the number of PHYs - they
> > > >   support one PHY per actual host port
> > > > - Amlogic Meson GXL and GXM SoCs come with a dwc3 IP block which has two
> > > >   or three USB2 ports enabled on the internal root-hub. The SoCs also
> > > >   provide separate USB2 PHYs, one per port. All USB2 PHYs (which are
> > > >   internally "connected" to the dwc3 roothub) need to be powered on,
> > > >   otherwise USB devices cannot be enumerated (even if just one PHY is
> > > >   disabled and if the device is plugged into another, enabled port)
> > > > 
> > > > In my first attempt to get USB supported on the GXL and GXM SoCs I tried
> > > > to work-around the problem that I could not pass multiple PHYs to the
> > > > dwc3 controller.
> > > > This was rejected by Rob Herring (which was definitely the thing to do
> > > > in
> > > > my opinion), see [2]
> > > > 
> > > > This series adds a new "platform-roothub". This can be configured
> > > > through
> > > > devicetree by passing a child-node with "reg = <0>" to the USB
> > > > controller. Additionally there has to be a child-node for each port on
> > > > the root-hub. Each of the child-nodes takes a "phys" and "phy-names"
> > > > property. This allows modeling the root-hub in devicetree similar to the
> > > > USB device binding (documented in devicetree/bindings/usb/usb-
> > > > device.txt)
> > > > This avoids and backwards-compatibility problems (which was a concern
> > > > regardless of the solution, see [3]) since the binding for the root-hub
> > > > was previously not specified (and we're not using the "phys" property of
> > > > the controller, which might have served different purposes before,
> > > > depending on the drivers).
> > > > 
> > > > Additionally this integrates the new platform-roothub into xhci-plat.c
> > > > which automatically enables it for the dwc3 driver (in host-mode).
> > > > 
> > > > 
> > > > Changes since RFCv3 at [6]:
> > > > - moved the DT binding change from patch #3 to patch #1 as suggested
> > > >   by Rob Herring (and slightly adjusted the commit message to account
> > > >   for that)
> > > > - added Tested-by from Chunfeng Yun (who confirmed that the whole
> > > >   concept and implementation works fine on Mediatek SoCs - many thanks
> > > >   again!) to patch #2
> > > > - added Rob Herring's ACK to patches 1 and 3
> > > > - dropped RFC status (RFCv3 -> PATCH v4)
> > > 
> > > I just wanted to rebase this to v4.14-rc1 (now that this is out) -
> > > however I noticed that v4 still applies to v4.14-rc1 cleanly (the
> > > patches are still identical to v4 after rebasing).
> > > 
> > > we have an ACK from the devicetree maintainers and a "Tested-by" for a
> > > Mediatek (= non-Amlogic) SoC.
> > > I already have patches for the Amlogic GXL/GXM platforms queued, those
> > > are just waiting on this series.
> > > 
> > > what is still missing to get this series into v4.15?
> > 
> > Well, we couldn't do anything until 4.14-rc1 is out, now that it is, let
> > us catch up on patch review please...
> 
> OK, I understand that.
> please let me know once you've caught up with the review backlog - as
> I said I would like to get this into 4.15 if nothing else comes up
> during the code-review

This series works well on the libretech-cc (le potato)
For the series:

Tested-by: Jerome Brunet <jbrunet@baylibre.com>


> 
> 
> Thank you!
> Regards,
> Martin
> 
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v4 0/3] initialize (multiple) PHYs in xhci-plat

[Greg KH]

On Mon, Oct 02, 2017 at 02:35:08PM +0200, Jerome Brunet wrote:
> On Sun, 2017-10-01 at 22:32 +0200, Martin Blumenstingl wrote:
> > Hello Greg, Hello Mathias,
> > 
> > On Mon, Sep 18, 2017 at 10:49 AM, Greg KH <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> wrote:
> > > On Sun, Sep 17, 2017 at 10:51:31PM +0200, Martin Blumenstingl wrote:
> > > > Hello Mathias, Hello Greg,
> > > > 
> > > > On Sun, Sep 3, 2017 at 11:38 PM, Martin Blumenstingl
> > > > <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
> > > > > This series is the outcome of a discussion with Felipe Balbi,
> > > > > see [0] and [1].
> > > > > The quick-summary of this is:
> > > > > - dwc3 already takes one USB2 and one USB3 PHY and initializes these
> > > > >   correct
> > > > > - some other HCI platform drivers (like ehci-platform.c, xhci-mtk.c and
> > > > >   ohci-platform.c) do not have a limitation on the number of PHYs - they
> > > > >   support one PHY per actual host port
> > > > > - Amlogic Meson GXL and GXM SoCs come with a dwc3 IP block which has two
> > > > >   or three USB2 ports enabled on the internal root-hub. The SoCs also
> > > > >   provide separate USB2 PHYs, one per port. All USB2 PHYs (which are
> > > > >   internally "connected" to the dwc3 roothub) need to be powered on,
> > > > >   otherwise USB devices cannot be enumerated (even if just one PHY is
> > > > >   disabled and if the device is plugged into another, enabled port)
> > > > > 
> > > > > In my first attempt to get USB supported on the GXL and GXM SoCs I tried
> > > > > to work-around the problem that I could not pass multiple PHYs to the
> > > > > dwc3 controller.
> > > > > This was rejected by Rob Herring (which was definitely the thing to do
> > > > > in
> > > > > my opinion), see [2]
> > > > > 
> > > > > This series adds a new "platform-roothub". This can be configured
> > > > > through
> > > > > devicetree by passing a child-node with "reg = <0>" to the USB
> > > > > controller. Additionally there has to be a child-node for each port on
> > > > > the root-hub. Each of the child-nodes takes a "phys" and "phy-names"
> > > > > property. This allows modeling the root-hub in devicetree similar to the
> > > > > USB device binding (documented in devicetree/bindings/usb/usb-
> > > > > device.txt)
> > > > > This avoids and backwards-compatibility problems (which was a concern
> > > > > regardless of the solution, see [3]) since the binding for the root-hub
> > > > > was previously not specified (and we're not using the "phys" property of
> > > > > the controller, which might have served different purposes before,
> > > > > depending on the drivers).
> > > > > 
> > > > > Additionally this integrates the new platform-roothub into xhci-plat.c
> > > > > which automatically enables it for the dwc3 driver (in host-mode).
> > > > > 
> > > > > 
> > > > > Changes since RFCv3 at [6]:
> > > > > - moved the DT binding change from patch #3 to patch #1 as suggested
> > > > >   by Rob Herring (and slightly adjusted the commit message to account
> > > > >   for that)
> > > > > - added Tested-by from Chunfeng Yun (who confirmed that the whole
> > > > >   concept and implementation works fine on Mediatek SoCs - many thanks
> > > > >   again!) to patch #2
> > > > > - added Rob Herring's ACK to patches 1 and 3
> > > > > - dropped RFC status (RFCv3 -> PATCH v4)
> > > > 
> > > > I just wanted to rebase this to v4.14-rc1 (now that this is out) -
> > > > however I noticed that v4 still applies to v4.14-rc1 cleanly (the
> > > > patches are still identical to v4 after rebasing).
> > > > 
> > > > we have an ACK from the devicetree maintainers and a "Tested-by" for a
> > > > Mediatek (= non-Amlogic) SoC.
> > > > I already have patches for the Amlogic GXL/GXM platforms queued, those
> > > > are just waiting on this series.
> > > > 
> > > > what is still missing to get this series into v4.15?
> > > 
> > > Well, we couldn't do anything until 4.14-rc1 is out, now that it is, let
> > > us catch up on patch review please...
> > 
> > OK, I understand that.
> > please let me know once you've caught up with the review backlog - as
> > I said I would like to get this into 4.15 if nothing else comes up
> > during the code-review
> 
> This series works well on the libretech-cc (le potato)
> For the series:
> 
> Tested-by: Jerome Brunet <jbrunet-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>

Hey, I have one of those boards now, was just about to try to get it to
work.  Is this series necessary for it to run properly on 4.14-rc?

thanks,

greg k-h
--
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 v4 0/3] initialize (multiple) PHYs in xhci-plat

[Greg KH]

On Mon, Oct 02, 2017 at 02:35:08PM +0200, Jerome Brunet wrote:
> On Sun, 2017-10-01 at 22:32 +0200, Martin Blumenstingl wrote:
> > Hello Greg, Hello Mathias,
> > 
> > On Mon, Sep 18, 2017 at 10:49 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > > On Sun, Sep 17, 2017 at 10:51:31PM +0200, Martin Blumenstingl wrote:
> > > > Hello Mathias, Hello Greg,
> > > > 
> > > > On Sun, Sep 3, 2017 at 11:38 PM, Martin Blumenstingl
> > > > <martin.blumenstingl@googlemail.com> wrote:
> > > > > This series is the outcome of a discussion with Felipe Balbi,
> > > > > see [0] and [1].
> > > > > The quick-summary of this is:
> > > > > - dwc3 already takes one USB2 and one USB3 PHY and initializes these
> > > > >   correct
> > > > > - some other HCI platform drivers (like ehci-platform.c, xhci-mtk.c and
> > > > >   ohci-platform.c) do not have a limitation on the number of PHYs - they
> > > > >   support one PHY per actual host port
> > > > > - Amlogic Meson GXL and GXM SoCs come with a dwc3 IP block which has two
> > > > >   or three USB2 ports enabled on the internal root-hub. The SoCs also
> > > > >   provide separate USB2 PHYs, one per port. All USB2 PHYs (which are
> > > > >   internally "connected" to the dwc3 roothub) need to be powered on,
> > > > >   otherwise USB devices cannot be enumerated (even if just one PHY is
> > > > >   disabled and if the device is plugged into another, enabled port)
> > > > > 
> > > > > In my first attempt to get USB supported on the GXL and GXM SoCs I tried
> > > > > to work-around the problem that I could not pass multiple PHYs to the
> > > > > dwc3 controller.
> > > > > This was rejected by Rob Herring (which was definitely the thing to do
> > > > > in
> > > > > my opinion), see [2]
> > > > > 
> > > > > This series adds a new "platform-roothub". This can be configured
> > > > > through
> > > > > devicetree by passing a child-node with "reg = <0>" to the USB
> > > > > controller. Additionally there has to be a child-node for each port on
> > > > > the root-hub. Each of the child-nodes takes a "phys" and "phy-names"
> > > > > property. This allows modeling the root-hub in devicetree similar to the
> > > > > USB device binding (documented in devicetree/bindings/usb/usb-
> > > > > device.txt)
> > > > > This avoids and backwards-compatibility problems (which was a concern
> > > > > regardless of the solution, see [3]) since the binding for the root-hub
> > > > > was previously not specified (and we're not using the "phys" property of
> > > > > the controller, which might have served different purposes before,
> > > > > depending on the drivers).
> > > > > 
> > > > > Additionally this integrates the new platform-roothub into xhci-plat.c
> > > > > which automatically enables it for the dwc3 driver (in host-mode).
> > > > > 
> > > > > 
> > > > > Changes since RFCv3 at [6]:
> > > > > - moved the DT binding change from patch #3 to patch #1 as suggested
> > > > >   by Rob Herring (and slightly adjusted the commit message to account
> > > > >   for that)
> > > > > - added Tested-by from Chunfeng Yun (who confirmed that the whole
> > > > >   concept and implementation works fine on Mediatek SoCs - many thanks
> > > > >   again!) to patch #2
> > > > > - added Rob Herring's ACK to patches 1 and 3
> > > > > - dropped RFC status (RFCv3 -> PATCH v4)
> > > > 
> > > > I just wanted to rebase this to v4.14-rc1 (now that this is out) -
> > > > however I noticed that v4 still applies to v4.14-rc1 cleanly (the
> > > > patches are still identical to v4 after rebasing).
> > > > 
> > > > we have an ACK from the devicetree maintainers and a "Tested-by" for a
> > > > Mediatek (= non-Amlogic) SoC.
> > > > I already have patches for the Amlogic GXL/GXM platforms queued, those
> > > > are just waiting on this series.
> > > > 
> > > > what is still missing to get this series into v4.15?
> > > 
> > > Well, we couldn't do anything until 4.14-rc1 is out, now that it is, let
> > > us catch up on patch review please...
> > 
> > OK, I understand that.
> > please let me know once you've caught up with the review backlog - as
> > I said I would like to get this into 4.15 if nothing else comes up
> > during the code-review
> 
> This series works well on the libretech-cc (le potato)
> For the series:
> 
> Tested-by: Jerome Brunet <jbrunet@baylibre.com>

Hey, I have one of those boards now, was just about to try to get it to
work.  Is this series necessary for it to run properly on 4.14-rc?

thanks,

greg k-h

Re: [PATCH v4 0/3] initialize (multiple) PHYs in xhci-plat

[Martin Blumenstingl]

Hi Greg,

On Mon, Oct 2, 2017 at 2:44 PM, Greg KH <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> wrote:
> On Mon, Oct 02, 2017 at 02:35:08PM +0200, Jerome Brunet wrote:
>> On Sun, 2017-10-01 at 22:32 +0200, Martin Blumenstingl wrote:
>> > Hello Greg, Hello Mathias,
>> >
>> > On Mon, Sep 18, 2017 at 10:49 AM, Greg KH <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> wrote:
>> > > On Sun, Sep 17, 2017 at 10:51:31PM +0200, Martin Blumenstingl wrote:
>> > > > Hello Mathias, Hello Greg,
>> > > >
>> > > > On Sun, Sep 3, 2017 at 11:38 PM, Martin Blumenstingl
>> > > > <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
>> > > > > This series is the outcome of a discussion with Felipe Balbi,
>> > > > > see [0] and [1].
>> > > > > The quick-summary of this is:
>> > > > > - dwc3 already takes one USB2 and one USB3 PHY and initializes these
>> > > > >   correct
>> > > > > - some other HCI platform drivers (like ehci-platform.c, xhci-mtk.c and
>> > > > >   ohci-platform.c) do not have a limitation on the number of PHYs - they
>> > > > >   support one PHY per actual host port
>> > > > > - Amlogic Meson GXL and GXM SoCs come with a dwc3 IP block which has two
>> > > > >   or three USB2 ports enabled on the internal root-hub. The SoCs also
>> > > > >   provide separate USB2 PHYs, one per port. All USB2 PHYs (which are
>> > > > >   internally "connected" to the dwc3 roothub) need to be powered on,
>> > > > >   otherwise USB devices cannot be enumerated (even if just one PHY is
>> > > > >   disabled and if the device is plugged into another, enabled port)
>> > > > >
>> > > > > In my first attempt to get USB supported on the GXL and GXM SoCs I tried
>> > > > > to work-around the problem that I could not pass multiple PHYs to the
>> > > > > dwc3 controller.
>> > > > > This was rejected by Rob Herring (which was definitely the thing to do
>> > > > > in
>> > > > > my opinion), see [2]
>> > > > >
>> > > > > This series adds a new "platform-roothub". This can be configured
>> > > > > through
>> > > > > devicetree by passing a child-node with "reg = <0>" to the USB
>> > > > > controller. Additionally there has to be a child-node for each port on
>> > > > > the root-hub. Each of the child-nodes takes a "phys" and "phy-names"
>> > > > > property. This allows modeling the root-hub in devicetree similar to the
>> > > > > USB device binding (documented in devicetree/bindings/usb/usb-
>> > > > > device.txt)
>> > > > > This avoids and backwards-compatibility problems (which was a concern
>> > > > > regardless of the solution, see [3]) since the binding for the root-hub
>> > > > > was previously not specified (and we're not using the "phys" property of
>> > > > > the controller, which might have served different purposes before,
>> > > > > depending on the drivers).
>> > > > >
>> > > > > Additionally this integrates the new platform-roothub into xhci-plat.c
>> > > > > which automatically enables it for the dwc3 driver (in host-mode).
>> > > > >
>> > > > >
>> > > > > Changes since RFCv3 at [6]:
>> > > > > - moved the DT binding change from patch #3 to patch #1 as suggested
>> > > > >   by Rob Herring (and slightly adjusted the commit message to account
>> > > > >   for that)
>> > > > > - added Tested-by from Chunfeng Yun (who confirmed that the whole
>> > > > >   concept and implementation works fine on Mediatek SoCs - many thanks
>> > > > >   again!) to patch #2
>> > > > > - added Rob Herring's ACK to patches 1 and 3
>> > > > > - dropped RFC status (RFCv3 -> PATCH v4)
>> > > >
>> > > > I just wanted to rebase this to v4.14-rc1 (now that this is out) -
>> > > > however I noticed that v4 still applies to v4.14-rc1 cleanly (the
>> > > > patches are still identical to v4 after rebasing).
>> > > >
>> > > > we have an ACK from the devicetree maintainers and a "Tested-by" for a
>> > > > Mediatek (= non-Amlogic) SoC.
>> > > > I already have patches for the Amlogic GXL/GXM platforms queued, those
>> > > > are just waiting on this series.
>> > > >
>> > > > what is still missing to get this series into v4.15?
>> > >
>> > > Well, we couldn't do anything until 4.14-rc1 is out, now that it is, let
>> > > us catch up on patch review please...
>> >
>> > OK, I understand that.
>> > please let me know once you've caught up with the review backlog - as
>> > I said I would like to get this into 4.15 if nothing else comes up
>> > during the code-review
>>
>> This series works well on the libretech-cc (le potato)
>> For the series:
>>
>> Tested-by: Jerome Brunet <jbrunet-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>
> Hey, I have one of those boards now, was just about to try to get it to
> work.  Is this series necessary for it to run properly on 4.14-rc?
if you're speaking of the "libretech-cc (le potato)" board then indeed, it is

however, it's just one piece of the whole puzzle:
1) we need this series to initialize all USB PHYs correctly
2) we need to initialize the USB3 PHY with a basic driver, see this series: [0]
3) all the USB PHYs and the dwc3 controller has to be added to the
.dts files (no patchset was sent yet because I don't want to maintain
another series of patches as long as this one is not accepted yet)

I made a branch which includes all puzzle pieces: [1]
please let me know if this works for you!

PS: I will send an official patchset for the .dts changes once the
other two patchsets are accepted


Thank you for testing this!


Regards,
Martin


[0] http://lists.infradead.org/pipermail/linux-amlogic/2017-September/004780.html
[1] https://github.com/xdarklight/linux/commits/meson-gxl-usb-test-greg1
--
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 v4 0/3] initialize (multiple) PHYs in xhci-plat

[Martin Blumenstingl]

Hi Greg,

On Mon, Oct 2, 2017 at 2:44 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Mon, Oct 02, 2017 at 02:35:08PM +0200, Jerome Brunet wrote:
>> On Sun, 2017-10-01 at 22:32 +0200, Martin Blumenstingl wrote:
>> > Hello Greg, Hello Mathias,
>> >
>> > On Mon, Sep 18, 2017 at 10:49 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
>> > > On Sun, Sep 17, 2017 at 10:51:31PM +0200, Martin Blumenstingl wrote:
>> > > > Hello Mathias, Hello Greg,
>> > > >
>> > > > On Sun, Sep 3, 2017 at 11:38 PM, Martin Blumenstingl
>> > > > <martin.blumenstingl@googlemail.com> wrote:
>> > > > > This series is the outcome of a discussion with Felipe Balbi,
>> > > > > see [0] and [1].
>> > > > > The quick-summary of this is:
>> > > > > - dwc3 already takes one USB2 and one USB3 PHY and initializes these
>> > > > >   correct
>> > > > > - some other HCI platform drivers (like ehci-platform.c, xhci-mtk.c and
>> > > > >   ohci-platform.c) do not have a limitation on the number of PHYs - they
>> > > > >   support one PHY per actual host port
>> > > > > - Amlogic Meson GXL and GXM SoCs come with a dwc3 IP block which has two
>> > > > >   or three USB2 ports enabled on the internal root-hub. The SoCs also
>> > > > >   provide separate USB2 PHYs, one per port. All USB2 PHYs (which are
>> > > > >   internally "connected" to the dwc3 roothub) need to be powered on,
>> > > > >   otherwise USB devices cannot be enumerated (even if just one PHY is
>> > > > >   disabled and if the device is plugged into another, enabled port)
>> > > > >
>> > > > > In my first attempt to get USB supported on the GXL and GXM SoCs I tried
>> > > > > to work-around the problem that I could not pass multiple PHYs to the
>> > > > > dwc3 controller.
>> > > > > This was rejected by Rob Herring (which was definitely the thing to do
>> > > > > in
>> > > > > my opinion), see [2]
>> > > > >
>> > > > > This series adds a new "platform-roothub". This can be configured
>> > > > > through
>> > > > > devicetree by passing a child-node with "reg = <0>" to the USB
>> > > > > controller. Additionally there has to be a child-node for each port on
>> > > > > the root-hub. Each of the child-nodes takes a "phys" and "phy-names"
>> > > > > property. This allows modeling the root-hub in devicetree similar to the
>> > > > > USB device binding (documented in devicetree/bindings/usb/usb-
>> > > > > device.txt)
>> > > > > This avoids and backwards-compatibility problems (which was a concern
>> > > > > regardless of the solution, see [3]) since the binding for the root-hub
>> > > > > was previously not specified (and we're not using the "phys" property of
>> > > > > the controller, which might have served different purposes before,
>> > > > > depending on the drivers).
>> > > > >
>> > > > > Additionally this integrates the new platform-roothub into xhci-plat.c
>> > > > > which automatically enables it for the dwc3 driver (in host-mode).
>> > > > >
>> > > > >
>> > > > > Changes since RFCv3 at [6]:
>> > > > > - moved the DT binding change from patch #3 to patch #1 as suggested
>> > > > >   by Rob Herring (and slightly adjusted the commit message to account
>> > > > >   for that)
>> > > > > - added Tested-by from Chunfeng Yun (who confirmed that the whole
>> > > > >   concept and implementation works fine on Mediatek SoCs - many thanks
>> > > > >   again!) to patch #2
>> > > > > - added Rob Herring's ACK to patches 1 and 3
>> > > > > - dropped RFC status (RFCv3 -> PATCH v4)
>> > > >
>> > > > I just wanted to rebase this to v4.14-rc1 (now that this is out) -
>> > > > however I noticed that v4 still applies to v4.14-rc1 cleanly (the
>> > > > patches are still identical to v4 after rebasing).
>> > > >
>> > > > we have an ACK from the devicetree maintainers and a "Tested-by" for a
>> > > > Mediatek (= non-Amlogic) SoC.
>> > > > I already have patches for the Amlogic GXL/GXM platforms queued, those
>> > > > are just waiting on this series.
>> > > >
>> > > > what is still missing to get this series into v4.15?
>> > >
>> > > Well, we couldn't do anything until 4.14-rc1 is out, now that it is, let
>> > > us catch up on patch review please...
>> >
>> > OK, I understand that.
>> > please let me know once you've caught up with the review backlog - as
>> > I said I would like to get this into 4.15 if nothing else comes up
>> > during the code-review
>>
>> This series works well on the libretech-cc (le potato)
>> For the series:
>>
>> Tested-by: Jerome Brunet <jbrunet@baylibre.com>
>
> Hey, I have one of those boards now, was just about to try to get it to
> work.  Is this series necessary for it to run properly on 4.14-rc?
if you're speaking of the "libretech-cc (le potato)" board then indeed, it is

however, it's just one piece of the whole puzzle:
1) we need this series to initialize all USB PHYs correctly
2) we need to initialize the USB3 PHY with a basic driver, see this series: [0]
3) all the USB PHYs and the dwc3 controller has to be added to the
.dts files (no patchset was sent yet because I don't want to maintain
another series of patches as long as this one is not accepted yet)

I made a branch which includes all puzzle pieces: [1]
please let me know if this works for you!

PS: I will send an official patchset for the .dts changes once the
other two patchsets are accepted


Thank you for testing this!


Regards,
Martin


[0] http://lists.infradead.org/pipermail/linux-amlogic/2017-September/004780.html
[1] https://github.com/xdarklight/linux/commits/meson-gxl-usb-test-greg1

Re: [PATCH v4 0/3] initialize (multiple) PHYs in xhci-plat

[Jerome Brunet]

On Mon, 2017-10-02 at 14:44 +0200, Greg KH wrote:
> On Mon, Oct 02, 2017 at 02:35:08PM +0200, Jerome Brunet wrote:
> > On Sun, 2017-10-01 at 22:32 +0200, Martin Blumenstingl wrote:
> > > Hello Greg, Hello Mathias,
> > > 
> > > On Mon, Sep 18, 2017 at 10:49 AM, Greg KH <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
> > > wrote:
> > > > On Sun, Sep 17, 2017 at 10:51:31PM +0200, Martin Blumenstingl wrote:
> > > > > Hello Mathias, Hello Greg,
> > > > > 
> > > > > On Sun, Sep 3, 2017 at 11:38 PM, Martin Blumenstingl
> > > > > <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
> > > > > > This series is the outcome of a discussion with Felipe Balbi,
> > > > > > see [0] and [1].
> > > > > > The quick-summary of this is:
> > > > > > - dwc3 already takes one USB2 and one USB3 PHY and initializes these
> > > > > >   correct
> > > > > > - some other HCI platform drivers (like ehci-platform.c, xhci-mtk.c
> > > > > > and
> > > > > >   ohci-platform.c) do not have a limitation on the number of PHYs -
> > > > > > they
> > > > > >   support one PHY per actual host port
> > > > > > - Amlogic Meson GXL and GXM SoCs come with a dwc3 IP block which has
> > > > > > two
> > > > > >   or three USB2 ports enabled on the internal root-hub. The SoCs
> > > > > > also
> > > > > >   provide separate USB2 PHYs, one per port. All USB2 PHYs (which are
> > > > > >   internally "connected" to the dwc3 roothub) need to be powered on,
> > > > > >   otherwise USB devices cannot be enumerated (even if just one PHY
> > > > > > is
> > > > > >   disabled and if the device is plugged into another, enabled port)
> > > > > > 
> > > > > > In my first attempt to get USB supported on the GXL and GXM SoCs I
> > > > > > tried
> > > > > > to work-around the problem that I could not pass multiple PHYs to
> > > > > > the
> > > > > > dwc3 controller.
> > > > > > This was rejected by Rob Herring (which was definitely the thing to
> > > > > > do
> > > > > > in
> > > > > > my opinion), see [2]
> > > > > > 
> > > > > > This series adds a new "platform-roothub". This can be configured
> > > > > > through
> > > > > > devicetree by passing a child-node with "reg = <0>" to the USB
> > > > > > controller. Additionally there has to be a child-node for each port
> > > > > > on
> > > > > > the root-hub. Each of the child-nodes takes a "phys" and "phy-names"
> > > > > > property. This allows modeling the root-hub in devicetree similar to
> > > > > > the
> > > > > > USB device binding (documented in devicetree/bindings/usb/usb-
> > > > > > device.txt)
> > > > > > This avoids and backwards-compatibility problems (which was a
> > > > > > concern
> > > > > > regardless of the solution, see [3]) since the binding for the root-
> > > > > > hub
> > > > > > was previously not specified (and we're not using the "phys"
> > > > > > property of
> > > > > > the controller, which might have served different purposes before,
> > > > > > depending on the drivers).
> > > > > > 
> > > > > > Additionally this integrates the new platform-roothub into xhci-
> > > > > > plat.c
> > > > > > which automatically enables it for the dwc3 driver (in host-mode).
> > > > > > 
> > > > > > 
> > > > > > Changes since RFCv3 at [6]:
> > > > > > - moved the DT binding change from patch #3 to patch #1 as suggested
> > > > > >   by Rob Herring (and slightly adjusted the commit message to
> > > > > > account
> > > > > >   for that)
> > > > > > - added Tested-by from Chunfeng Yun (who confirmed that the whole
> > > > > >   concept and implementation works fine on Mediatek SoCs - many
> > > > > > thanks
> > > > > >   again!) to patch #2
> > > > > > - added Rob Herring's ACK to patches 1 and 3
> > > > > > - dropped RFC status (RFCv3 -> PATCH v4)
> > > > > 
> > > > > I just wanted to rebase this to v4.14-rc1 (now that this is out) -
> > > > > however I noticed that v4 still applies to v4.14-rc1 cleanly (the
> > > > > patches are still identical to v4 after rebasing).
> > > > > 
> > > > > we have an ACK from the devicetree maintainers and a "Tested-by" for a
> > > > > Mediatek (= non-Amlogic) SoC.
> > > > > I already have patches for the Amlogic GXL/GXM platforms queued, those
> > > > > are just waiting on this series.
> > > > > 
> > > > > what is still missing to get this series into v4.15?
> > > > 
> > > > Well, we couldn't do anything until 4.14-rc1 is out, now that it is, let
> > > > us catch up on patch review please...
> > > 
> > > OK, I understand that.
> > > please let me know once you've caught up with the review backlog - as
> > > I said I would like to get this into 4.15 if nothing else comes up
> > > during the code-review
> > 
> > This series works well on the libretech-cc (le potato)
> > For the series:
> > 
> > Tested-by: Jerome Brunet <jbrunet-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> 
> Hey, I have one of those boards now, was just about to try to get it to
> work.  Is this series necessary for it to run properly on 4.14-rc?

To run w/o usb, no. It should already be decent on 4.14-rc.
To run with usb, you need this series and:

* the usb3 phy driver sent but (I believe) not yet merged [0]
* A few DT changes enable the whole thing.

The whole thing is available here [1]

However, while you were at KR2017, we found an unrelated, yet very annoying,
issue which may trigger "Synchronous External Aborts" ... The whole story is
available here [2] and the work around patch is available here [3].

If you have downloaded your image from here [4], the kernel is a v4.13 plus
* few changes already merged in next or under review (cec, mmc, usb, ...)
* Overlay support 
* WIP for the hdmi audio.
Sources are here [5]

[0]: https://lkml.kernel.org/r/20170924195000.13276-1-martin.blumenstingl@google
mail.com
[1]: https://github.com/libre-computer-project/libretech-linux/commits/linux-4.1
3/usb
[2]: https://lkml.kernel.org/r/1507017860.17300.106.camel-rdvid1DuHRBWk0Htik3J/w@public.gmane.org
[3]: https://github.com/libre-computer-project/libretech-linux/commit/0f71f762a9
c37ab423e0c9e0498081a9a17f2b8a
[4]: http://baylibre.com/kernel-recipes-mainline-aml-s905x-cc-le-potato/
[5]: https://github.com/libre-computer-project/libretech-linux/commits/linux-4.1
3/libretech-cc-overlays

> 
> thanks,
> 
> greg k-h

--
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 v4 0/3] initialize (multiple) PHYs in xhci-plat

[Jerome Brunet]

On Mon, 2017-10-02 at 14:44 +0200, Greg KH wrote:
> On Mon, Oct 02, 2017 at 02:35:08PM +0200, Jerome Brunet wrote:
> > On Sun, 2017-10-01 at 22:32 +0200, Martin Blumenstingl wrote:
> > > Hello Greg, Hello Mathias,
> > > 
> > > On Mon, Sep 18, 2017 at 10:49 AM, Greg KH <gregkh@linuxfoundation.org>
> > > wrote:
> > > > On Sun, Sep 17, 2017 at 10:51:31PM +0200, Martin Blumenstingl wrote:
> > > > > Hello Mathias, Hello Greg,
> > > > > 
> > > > > On Sun, Sep 3, 2017 at 11:38 PM, Martin Blumenstingl
> > > > > <martin.blumenstingl@googlemail.com> wrote:
> > > > > > This series is the outcome of a discussion with Felipe Balbi,
> > > > > > see [0] and [1].
> > > > > > The quick-summary of this is:
> > > > > > - dwc3 already takes one USB2 and one USB3 PHY and initializes these
> > > > > >   correct
> > > > > > - some other HCI platform drivers (like ehci-platform.c, xhci-mtk.c
> > > > > > and
> > > > > >   ohci-platform.c) do not have a limitation on the number of PHYs -
> > > > > > they
> > > > > >   support one PHY per actual host port
> > > > > > - Amlogic Meson GXL and GXM SoCs come with a dwc3 IP block which has
> > > > > > two
> > > > > >   or three USB2 ports enabled on the internal root-hub. The SoCs
> > > > > > also
> > > > > >   provide separate USB2 PHYs, one per port. All USB2 PHYs (which are
> > > > > >   internally "connected" to the dwc3 roothub) need to be powered on,
> > > > > >   otherwise USB devices cannot be enumerated (even if just one PHY
> > > > > > is
> > > > > >   disabled and if the device is plugged into another, enabled port)
> > > > > > 
> > > > > > In my first attempt to get USB supported on the GXL and GXM SoCs I
> > > > > > tried
> > > > > > to work-around the problem that I could not pass multiple PHYs to
> > > > > > the
> > > > > > dwc3 controller.
> > > > > > This was rejected by Rob Herring (which was definitely the thing to
> > > > > > do
> > > > > > in
> > > > > > my opinion), see [2]
> > > > > > 
> > > > > > This series adds a new "platform-roothub". This can be configured
> > > > > > through
> > > > > > devicetree by passing a child-node with "reg = <0>" to the USB
> > > > > > controller. Additionally there has to be a child-node for each port
> > > > > > on
> > > > > > the root-hub. Each of the child-nodes takes a "phys" and "phy-names"
> > > > > > property. This allows modeling the root-hub in devicetree similar to
> > > > > > the
> > > > > > USB device binding (documented in devicetree/bindings/usb/usb-
> > > > > > device.txt)
> > > > > > This avoids and backwards-compatibility problems (which was a
> > > > > > concern
> > > > > > regardless of the solution, see [3]) since the binding for the root-
> > > > > > hub
> > > > > > was previously not specified (and we're not using the "phys"
> > > > > > property of
> > > > > > the controller, which might have served different purposes before,
> > > > > > depending on the drivers).
> > > > > > 
> > > > > > Additionally this integrates the new platform-roothub into xhci-
> > > > > > plat.c
> > > > > > which automatically enables it for the dwc3 driver (in host-mode).
> > > > > > 
> > > > > > 
> > > > > > Changes since RFCv3 at [6]:
> > > > > > - moved the DT binding change from patch #3 to patch #1 as suggested
> > > > > >   by Rob Herring (and slightly adjusted the commit message to
> > > > > > account
> > > > > >   for that)
> > > > > > - added Tested-by from Chunfeng Yun (who confirmed that the whole
> > > > > >   concept and implementation works fine on Mediatek SoCs - many
> > > > > > thanks
> > > > > >   again!) to patch #2
> > > > > > - added Rob Herring's ACK to patches 1 and 3
> > > > > > - dropped RFC status (RFCv3 -> PATCH v4)
> > > > > 
> > > > > I just wanted to rebase this to v4.14-rc1 (now that this is out) -
> > > > > however I noticed that v4 still applies to v4.14-rc1 cleanly (the
> > > > > patches are still identical to v4 after rebasing).
> > > > > 
> > > > > we have an ACK from the devicetree maintainers and a "Tested-by" for a
> > > > > Mediatek (= non-Amlogic) SoC.
> > > > > I already have patches for the Amlogic GXL/GXM platforms queued, those
> > > > > are just waiting on this series.
> > > > > 
> > > > > what is still missing to get this series into v4.15?
> > > > 
> > > > Well, we couldn't do anything until 4.14-rc1 is out, now that it is, let
> > > > us catch up on patch review please...
> > > 
> > > OK, I understand that.
> > > please let me know once you've caught up with the review backlog - as
> > > I said I would like to get this into 4.15 if nothing else comes up
> > > during the code-review
> > 
> > This series works well on the libretech-cc (le potato)
> > For the series:
> > 
> > Tested-by: Jerome Brunet <jbrunet@baylibre.com>
> 
> Hey, I have one of those boards now, was just about to try to get it to
> work.  Is this series necessary for it to run properly on 4.14-rc?

To run w/o usb, no. It should already be decent on 4.14-rc.
To run with usb, you need this series and:

* the usb3 phy driver sent but (I believe) not yet merged [0]
* A few DT changes enable the whole thing.

The whole thing is available here [1]

However, while you were at KR2017, we found an unrelated, yet very annoying,
issue which may trigger "Synchronous External Aborts" ... The whole story is
available here [2] and the work around patch is available here [3].

If you have downloaded your image from here [4], the kernel is a v4.13 plus
* few changes already merged in next or under review (cec, mmc, usb, ...)
* Overlay support 
* WIP for the hdmi audio.
Sources are here [5]

[0]: https://lkml.kernel.org/r/20170924195000.13276-1-martin.blumenstingl at google
mail.com
[1]: https://github.com/libre-computer-project/libretech-linux/commits/linux-4.1
3/usb
[2]: https://lkml.kernel.org/r/1507017860.17300.106.camel at baylibre.com
[3]: https://github.com/libre-computer-project/libretech-linux/commit/0f71f762a9
c37ab423e0c9e0498081a9a17f2b8a
[4]: http://baylibre.com/kernel-recipes-mainline-aml-s905x-cc-le-potato/
[5]: https://github.com/libre-computer-project/libretech-linux/commits/linux-4.1
3/libretech-cc-overlays

> 
> thanks,
> 
> greg k-h

Re: [PATCH v4 2/3] usb: host: add a generic platform USB roothub driver

[Mathias Nyman]

On 04.09.2017 00:38, Martin Blumenstingl wrote:
> Many SoC platforms have separate devices for the USB PHY which are
> registered through the generic PHY framework. These PHYs have to be
> enabled to make the USB controller actually work. They also have to be
> disabled again on shutdown/suspend.
>
> Currently (at least) the following HCI platform drivers are using custom
> code to obtain all PHYs via devicetree for the roothub/controller and
> disable/enable them when required:
> - ehci-platform.c has ehci_platform_power_{on,off}
> - xhci-mtk.c has xhci_mtk_phy_{init,exit,power_on,power_off}
> - ohci-platform.c has ohci_platform_power_{on,off}
>
> These drivers are not using the generic devicetree USB device bindings
> yet which were only introduced recently (documentation is available in
> devicetree/bindings/usb/usb-device.txt).
> With this new driver the usb2-phy and usb3-phy can be specified directly
> in the child-node of the corresponding port of the roothub via
> devicetree. This can be extended by not just parsing PHYs (some of the
> other drivers listed above are for example also parsing a list of clocks
> as well) when required.

usb_add_hcd() in usb/core/hcd.c is already finding, initializing and turning
on a phy, would it make sense to expand that one to support several phys instead?

xhci will add two hcd's one for USB2 and one for USB3

>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> Tested-by: Chunfeng Yun <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---
>   drivers/usb/host/Kconfig            |   3 +
>   drivers/usb/host/Makefile           |   2 +
>   drivers/usb/host/platform-roothub.c | 180 ++++++++++++++++++++++++++++++++++++
>   drivers/usb/host/platform-roothub.h |  12 +++
>   4 files changed, 197 insertions(+)
>   create mode 100644 drivers/usb/host/platform-roothub.c
>   create mode 100644 drivers/usb/host/platform-roothub.h
>

Instead of creating  platform-roothub files could this content
be added into into core/hcd.*, core/phy.* and host/xhci-plat.c

> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index fa5692dec832..b8b05c786b2a 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -805,6 +805,9 @@ config USB_HCD_SSB
>
>   	  If unsure, say N.
>
> +config USB_PLATFORM_ROOTHUB
> +	bool
> +
>   config USB_HCD_TEST_MODE
>   	bool "HCD test mode support"
>   	---help---
> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
> index cf2691fffcc0..dc817f82d632 100644
> --- a/drivers/usb/host/Makefile
> +++ b/drivers/usb/host/Makefile
> @@ -29,6 +29,8 @@ obj-$(CONFIG_USB_WHCI_HCD)	+= whci/
>
>   obj-$(CONFIG_USB_PCI)	+= pci-quirks.o
>
> +obj-$(CONFIG_USB_PLATFORM_ROOTHUB)	+= platform-roothub.o
> +
>   obj-$(CONFIG_USB_EHCI_HCD)	+= ehci-hcd.o
>   obj-$(CONFIG_USB_EHCI_PCI)	+= ehci-pci.o
>   obj-$(CONFIG_USB_EHCI_HCD_PLATFORM)	+= ehci-platform.o
> diff --git a/drivers/usb/host/platform-roothub.c b/drivers/usb/host/platform-roothub.c
> new file mode 100644
> index 000000000000..70d2d97aa8b2
> --- /dev/null
> +++ b/drivers/usb/host/platform-roothub.c
> @@ -0,0 +1,180 @@
> +/*
> + * platform roothub driver - a virtual PHY device which passes all phy_*
> + * function calls to multiple (actual) PHY devices. This is comes handy when
> + * initializing all PHYs on a root-hub (to keep them all in the same state).
> + *
> + * Copyright (C) 2017 Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> + *
> + * 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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/list.h>
> +#include <linux/phy/phy.h>
> +#include <linux/of.h>
> +#include <linux/usb/of.h>
> +
> +#include "platform-roothub.h"
> +
> +#define ROOTHUB_PORTNUM		0
> +
> +struct platform_roothub {
> +	struct phy		*phy;
> +	struct list_head	list;
> +};
> +
> +static struct platform_roothub *platform_roothub_alloc(struct device *dev)
> +{
> +	struct platform_roothub *roothub_entry;
> +
> +	roothub_entry = devm_kzalloc(dev, sizeof(*roothub_entry), GFP_KERNEL);
> +	if (!roothub_entry)
> +		return ERR_PTR(-ENOMEM);
> +
> +	INIT_LIST_HEAD(&roothub_entry->list);
> +
> +	return roothub_entry;
> +}
> +
> +static int platform_roothub_add_phy(struct device *dev,
> +				    struct device_node *port_np,
> +				    const char *con_id, struct list_head *list)
> +{
> +	struct platform_roothub *roothub_entry;
> +	struct phy *phy = devm_of_phy_get(dev, port_np, con_id);
> +
> +	if (IS_ERR_OR_NULL(phy)) {
> +		if (!phy || PTR_ERR(phy) == -ENODEV)
> +			return 0;
> +		else
> +			return PTR_ERR(phy);
> +	}
> +
> +	roothub_entry = platform_roothub_alloc(dev);
> +	if (IS_ERR(roothub_entry))
> +		return PTR_ERR(roothub_entry);
> +
> +	roothub_entry->phy = phy;
> +
> +	list_add_tail(&roothub_entry->list, list);
> +
> +	return 0;
> +}
> +
> +struct platform_roothub *platform_roothub_init(struct device *dev)
> +{
> +	struct device_node *roothub_np, *port_np;
> +	struct platform_roothub *plat_roothub;
> +	struct platform_roothub *roothub_entry;
> +	struct list_head *head;
> +	int err;
> +
> +	roothub_np = usb_of_get_child_node(dev->of_node, ROOTHUB_PORTNUM);
> +	if (!of_device_is_available(roothub_np))
> +		return NULL;
> +
> +	plat_roothub = platform_roothub_alloc(dev);
> +	if (IS_ERR(plat_roothub))
> +		return plat_roothub;
> +
> +	for_each_available_child_of_node(roothub_np, port_np) {
> +		err = platform_roothub_add_phy(dev, port_np, "usb2-phy",
> +					       &plat_roothub->list);
> +		if (err)
> +			goto err_out;
> +
> +		err = platform_roothub_add_phy(dev, port_np, "usb3-phy",
> +					       &plat_roothub->list);

So if the first 10 ports have the same phy, and 11th and 12th have an other one, won't we end up
with a phy list with 12 entries for 2 phys, and initialize and turn on the same first phy 10 times?

I'm also not sure I understand the reason for having the "usb3-phy" and "usb2-phy" phy-names
for the ports if we anyways just add everything to one list.

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

[PATCH v4 2/3] usb: host: add a generic platform USB roothub driver

[Mathias Nyman]

On 04.09.2017 00:38, Martin Blumenstingl wrote:
> Many SoC platforms have separate devices for the USB PHY which are
> registered through the generic PHY framework. These PHYs have to be
> enabled to make the USB controller actually work. They also have to be
> disabled again on shutdown/suspend.
>
> Currently (at least) the following HCI platform drivers are using custom
> code to obtain all PHYs via devicetree for the roothub/controller and
> disable/enable them when required:
> - ehci-platform.c has ehci_platform_power_{on,off}
> - xhci-mtk.c has xhci_mtk_phy_{init,exit,power_on,power_off}
> - ohci-platform.c has ohci_platform_power_{on,off}
>
> These drivers are not using the generic devicetree USB device bindings
> yet which were only introduced recently (documentation is available in
> devicetree/bindings/usb/usb-device.txt).
> With this new driver the usb2-phy and usb3-phy can be specified directly
> in the child-node of the corresponding port of the roothub via
> devicetree. This can be extended by not just parsing PHYs (some of the
> other drivers listed above are for example also parsing a list of clocks
> as well) when required.

usb_add_hcd() in usb/core/hcd.c is already finding, initializing and turning
on a phy, would it make sense to expand that one to support several phys instead?

xhci will add two hcd's one for USB2 and one for USB3

>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> Tested-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
>   drivers/usb/host/Kconfig            |   3 +
>   drivers/usb/host/Makefile           |   2 +
>   drivers/usb/host/platform-roothub.c | 180 ++++++++++++++++++++++++++++++++++++
>   drivers/usb/host/platform-roothub.h |  12 +++
>   4 files changed, 197 insertions(+)
>   create mode 100644 drivers/usb/host/platform-roothub.c
>   create mode 100644 drivers/usb/host/platform-roothub.h
>

Instead of creating  platform-roothub files could this content
be added into into core/hcd.*, core/phy.* and host/xhci-plat.c

> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index fa5692dec832..b8b05c786b2a 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -805,6 +805,9 @@ config USB_HCD_SSB
>
>   	  If unsure, say N.
>
> +config USB_PLATFORM_ROOTHUB
> +	bool
> +
>   config USB_HCD_TEST_MODE
>   	bool "HCD test mode support"
>   	---help---
> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
> index cf2691fffcc0..dc817f82d632 100644
> --- a/drivers/usb/host/Makefile
> +++ b/drivers/usb/host/Makefile
> @@ -29,6 +29,8 @@ obj-$(CONFIG_USB_WHCI_HCD)	+= whci/
>
>   obj-$(CONFIG_USB_PCI)	+= pci-quirks.o
>
> +obj-$(CONFIG_USB_PLATFORM_ROOTHUB)	+= platform-roothub.o
> +
>   obj-$(CONFIG_USB_EHCI_HCD)	+= ehci-hcd.o
>   obj-$(CONFIG_USB_EHCI_PCI)	+= ehci-pci.o
>   obj-$(CONFIG_USB_EHCI_HCD_PLATFORM)	+= ehci-platform.o
> diff --git a/drivers/usb/host/platform-roothub.c b/drivers/usb/host/platform-roothub.c
> new file mode 100644
> index 000000000000..70d2d97aa8b2
> --- /dev/null
> +++ b/drivers/usb/host/platform-roothub.c
> @@ -0,0 +1,180 @@
> +/*
> + * platform roothub driver - a virtual PHY device which passes all phy_*
> + * function calls to multiple (actual) PHY devices. This is comes handy when
> + * initializing all PHYs on a root-hub (to keep them all in the same state).
> + *
> + * Copyright (C) 2017 Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> + *
> + * 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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/list.h>
> +#include <linux/phy/phy.h>
> +#include <linux/of.h>
> +#include <linux/usb/of.h>
> +
> +#include "platform-roothub.h"
> +
> +#define ROOTHUB_PORTNUM		0
> +
> +struct platform_roothub {
> +	struct phy		*phy;
> +	struct list_head	list;
> +};
> +
> +static struct platform_roothub *platform_roothub_alloc(struct device *dev)
> +{
> +	struct platform_roothub *roothub_entry;
> +
> +	roothub_entry = devm_kzalloc(dev, sizeof(*roothub_entry), GFP_KERNEL);
> +	if (!roothub_entry)
> +		return ERR_PTR(-ENOMEM);
> +
> +	INIT_LIST_HEAD(&roothub_entry->list);
> +
> +	return roothub_entry;
> +}
> +
> +static int platform_roothub_add_phy(struct device *dev,
> +				    struct device_node *port_np,
> +				    const char *con_id, struct list_head *list)
> +{
> +	struct platform_roothub *roothub_entry;
> +	struct phy *phy = devm_of_phy_get(dev, port_np, con_id);
> +
> +	if (IS_ERR_OR_NULL(phy)) {
> +		if (!phy || PTR_ERR(phy) == -ENODEV)
> +			return 0;
> +		else
> +			return PTR_ERR(phy);
> +	}
> +
> +	roothub_entry = platform_roothub_alloc(dev);
> +	if (IS_ERR(roothub_entry))
> +		return PTR_ERR(roothub_entry);
> +
> +	roothub_entry->phy = phy;
> +
> +	list_add_tail(&roothub_entry->list, list);
> +
> +	return 0;
> +}
> +
> +struct platform_roothub *platform_roothub_init(struct device *dev)
> +{
> +	struct device_node *roothub_np, *port_np;
> +	struct platform_roothub *plat_roothub;
> +	struct platform_roothub *roothub_entry;
> +	struct list_head *head;
> +	int err;
> +
> +	roothub_np = usb_of_get_child_node(dev->of_node, ROOTHUB_PORTNUM);
> +	if (!of_device_is_available(roothub_np))
> +		return NULL;
> +
> +	plat_roothub = platform_roothub_alloc(dev);
> +	if (IS_ERR(plat_roothub))
> +		return plat_roothub;
> +
> +	for_each_available_child_of_node(roothub_np, port_np) {
> +		err = platform_roothub_add_phy(dev, port_np, "usb2-phy",
> +					       &plat_roothub->list);
> +		if (err)
> +			goto err_out;
> +
> +		err = platform_roothub_add_phy(dev, port_np, "usb3-phy",
> +					       &plat_roothub->list);

So if the first 10 ports have the same phy, and 11th and 12th have an other one, won't we end up
with a phy list with 12 entries for 2 phys, and initialize and turn on the same first phy 10 times?

I'm also not sure I understand the reason for having the "usb3-phy" and "usb2-phy" phy-names
for the ports if we anyways just add everything to one list.

-Mathias

Re: [PATCH v4 2/3] usb: host: add a generic platform USB roothub driver

[Martin Blumenstingl]

Hi Mathias,

thank you for taking the time to go through my patch

On Wed, Oct 4, 2017 at 3:05 PM, Mathias Nyman
<mathias.nyman-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
> On 04.09.2017 00:38, Martin Blumenstingl wrote:
>>
>> Many SoC platforms have separate devices for the USB PHY which are
>> registered through the generic PHY framework. These PHYs have to be
>> enabled to make the USB controller actually work. They also have to be
>> disabled again on shutdown/suspend.
>>
>> Currently (at least) the following HCI platform drivers are using custom
>> code to obtain all PHYs via devicetree for the roothub/controller and
>> disable/enable them when required:
>> - ehci-platform.c has ehci_platform_power_{on,off}
>> - xhci-mtk.c has xhci_mtk_phy_{init,exit,power_on,power_off}
>> - ohci-platform.c has ohci_platform_power_{on,off}
>>
>> These drivers are not using the generic devicetree USB device bindings
>> yet which were only introduced recently (documentation is available in
>> devicetree/bindings/usb/usb-device.txt).
>> With this new driver the usb2-phy and usb3-phy can be specified directly
>> in the child-node of the corresponding port of the roothub via
>> devicetree. This can be extended by not just parsing PHYs (some of the
>> other drivers listed above are for example also parsing a list of clocks
>> as well) when required.
>
>
> usb_add_hcd() in usb/core/hcd.c is already finding, initializing and turning
> on a phy, would it make sense to expand that one to support several phys
> instead?
>
> xhci will add two hcd's one for USB2 and one for USB3
that is a great suggestion - thank you for bringing this up!
as a benefit we would add multiple PHY support for all the other
use-cases I found (at least: ehci-platform.c, xhci-mtk.c,
ohci-platform.c) - instead of just handling this in xhci-plat.c

I have one quick question regarding usb/core/hcd.c:
are hcd_bus_suspend() and hcd_bus_resume() the right places to
power_{off,on} the PHYs during suspend?
(currently usb/core/hcd.c doesn't touch the PHY during suspend/resume
- xhci-mtk.c on the other hand seems to require it during a
suspend/resume cycle)

>>
>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>> Tested-by: Chunfeng Yun <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>> ---
>>   drivers/usb/host/Kconfig            |   3 +
>>   drivers/usb/host/Makefile           |   2 +
>>   drivers/usb/host/platform-roothub.c | 180
>> ++++++++++++++++++++++++++++++++++++
>>   drivers/usb/host/platform-roothub.h |  12 +++
>>   4 files changed, 197 insertions(+)
>>   create mode 100644 drivers/usb/host/platform-roothub.c
>>   create mode 100644 drivers/usb/host/platform-roothub.h
>>
>
> Instead of creating  platform-roothub files could this content
> be added into into core/hcd.*, core/phy.* and host/xhci-plat.c
OK, I will try this and send a patch so we can have a look at the
potential result and start a discussion based on that (if required)

>
>> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
>> index fa5692dec832..b8b05c786b2a 100644
>> --- a/drivers/usb/host/Kconfig
>> +++ b/drivers/usb/host/Kconfig
>> @@ -805,6 +805,9 @@ config USB_HCD_SSB
>>
>>           If unsure, say N.
>>
>> +config USB_PLATFORM_ROOTHUB
>> +       bool
>> +
>>   config USB_HCD_TEST_MODE
>>         bool "HCD test mode support"
>>         ---help---
>> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
>> index cf2691fffcc0..dc817f82d632 100644
>> --- a/drivers/usb/host/Makefile
>> +++ b/drivers/usb/host/Makefile
>> @@ -29,6 +29,8 @@ obj-$(CONFIG_USB_WHCI_HCD)    += whci/
>>
>>   obj-$(CONFIG_USB_PCI) += pci-quirks.o
>>
>> +obj-$(CONFIG_USB_PLATFORM_ROOTHUB)     += platform-roothub.o
>> +
>>   obj-$(CONFIG_USB_EHCI_HCD)    += ehci-hcd.o
>>   obj-$(CONFIG_USB_EHCI_PCI)    += ehci-pci.o
>>   obj-$(CONFIG_USB_EHCI_HCD_PLATFORM)   += ehci-platform.o
>> diff --git a/drivers/usb/host/platform-roothub.c
>> b/drivers/usb/host/platform-roothub.c
>> new file mode 100644
>> index 000000000000..70d2d97aa8b2
>> --- /dev/null
>> +++ b/drivers/usb/host/platform-roothub.c
>> @@ -0,0 +1,180 @@
>> +/*
>> + * platform roothub driver - a virtual PHY device which passes all phy_*
>> + * function calls to multiple (actual) PHY devices. This is comes handy
>> when
>> + * initializing all PHYs on a root-hub (to keep them all in the same
>> state).
>> + *
>> + * Copyright (C) 2017 Martin Blumenstingl
>> <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>> + *
>> + * 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.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include <linux/device.h>
>> +#include <linux/list.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/of.h>
>> +#include <linux/usb/of.h>
>> +
>> +#include "platform-roothub.h"
>> +
>> +#define ROOTHUB_PORTNUM                0
>> +
>> +struct platform_roothub {
>> +       struct phy              *phy;
>> +       struct list_head        list;
>> +};
>> +
>> +static struct platform_roothub *platform_roothub_alloc(struct device
>> *dev)
>> +{
>> +       struct platform_roothub *roothub_entry;
>> +
>> +       roothub_entry = devm_kzalloc(dev, sizeof(*roothub_entry),
>> GFP_KERNEL);
>> +       if (!roothub_entry)
>> +               return ERR_PTR(-ENOMEM);
>> +
>> +       INIT_LIST_HEAD(&roothub_entry->list);
>> +
>> +       return roothub_entry;
>> +}
>> +
>> +static int platform_roothub_add_phy(struct device *dev,
>> +                                   struct device_node *port_np,
>> +                                   const char *con_id, struct list_head
>> *list)
>> +{
>> +       struct platform_roothub *roothub_entry;
>> +       struct phy *phy = devm_of_phy_get(dev, port_np, con_id);
>> +
>> +       if (IS_ERR_OR_NULL(phy)) {
>> +               if (!phy || PTR_ERR(phy) == -ENODEV)
>> +                       return 0;
>> +               else
>> +                       return PTR_ERR(phy);
>> +       }
>> +
>> +       roothub_entry = platform_roothub_alloc(dev);
>> +       if (IS_ERR(roothub_entry))
>> +               return PTR_ERR(roothub_entry);
>> +
>> +       roothub_entry->phy = phy;
>> +
>> +       list_add_tail(&roothub_entry->list, list);
>> +
>> +       return 0;
>> +}
>> +
>> +struct platform_roothub *platform_roothub_init(struct device *dev)
>> +{
>> +       struct device_node *roothub_np, *port_np;
>> +       struct platform_roothub *plat_roothub;
>> +       struct platform_roothub *roothub_entry;
>> +       struct list_head *head;
>> +       int err;
>> +
>> +       roothub_np = usb_of_get_child_node(dev->of_node, ROOTHUB_PORTNUM);
>> +       if (!of_device_is_available(roothub_np))
>> +               return NULL;
>> +
>> +       plat_roothub = platform_roothub_alloc(dev);
>> +       if (IS_ERR(plat_roothub))
>> +               return plat_roothub;
>> +
>> +       for_each_available_child_of_node(roothub_np, port_np) {
>> +               err = platform_roothub_add_phy(dev, port_np, "usb2-phy",
>> +                                              &plat_roothub->list);
>> +               if (err)
>> +                       goto err_out;
>> +
>> +               err = platform_roothub_add_phy(dev, port_np, "usb3-phy",
>> +                                              &plat_roothub->list);
>
>
> So if the first 10 ports have the same phy, and 11th and 12th have an other
> one, won't we end up
> with a phy list with 12 entries for 2 phys, and initialize and turn on the
> same first phy 10 times?
indeed, we would call phy_init() and phy_power_on() multiple times on
the same PHY.
however, this is not an issue since the PHY framework is doing
ref-counting for us (see [0] and [1] - the PHY driver's .init() and
.power_on() callbacks will only be called once for each PHY in your
scenario)
do you see any other issues with this?

> I'm also not sure I understand the reason for having the "usb3-phy" and
> "usb2-phy" phy-names
> for the ports if we anyways just add everything to one list.
the PHY devicetree bindings state that the "phy-names" property is
mandatory: [2]

when moving the whole multiple PHY logic to usb_add_hcd() then it even
makes sense to look for specific PHYs (imho):
- let's assume we have a XHCI controller with two ports
- both ports have a USB2 PHY, but only one has a USB3 PHY
- (this basically describes the situation on the Amlogic Meson GXL
SoCs, Mediatek uses similar designs)

this would result in the following flow:
1. usb_add_hcd is called for the high-speed HCD
2. we would parse the PHYs for the high-speed HCD
3. usb_add_hcd is called for the super-speed HCD
4. we would parse the PHYs for the super-speed HCD
depending on how we parse the PHYs (mapping the controller-type to
phy-name "usb2-phy" or "usb3-phy") we either end up with:
- 3 PHY handles (2x USB2, 1x USB3) if we take the
controller-type/phy-name into account
- 6 PHY handles (doubling the amount from the use-case above) if we don't

please let me know if there is an easier solution - I prefer simple
code myself, so I don't want to add complexity where it's not needed.


Regards,
Martin


[0] http://elixir.free-electrons.com/linux/v4.14-rc3/source/drivers/phy/phy-core.c#L231
[1] http://elixir.free-electrons.com/linux/v4.14-rc3/source/drivers/phy/phy-core.c#L296
[2] http://elixir.free-electrons.com/linux/v4.14-rc3/source/Documentation/devicetree/bindings/phy/phy-bindings.txt#L36
--
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 v4 2/3] usb: host: add a generic platform USB roothub driver

[Martin Blumenstingl]

Hi Mathias,

thank you for taking the time to go through my patch

On Wed, Oct 4, 2017 at 3:05 PM, Mathias Nyman
<mathias.nyman@linux.intel.com> wrote:
> On 04.09.2017 00:38, Martin Blumenstingl wrote:
>>
>> Many SoC platforms have separate devices for the USB PHY which are
>> registered through the generic PHY framework. These PHYs have to be
>> enabled to make the USB controller actually work. They also have to be
>> disabled again on shutdown/suspend.
>>
>> Currently (at least) the following HCI platform drivers are using custom
>> code to obtain all PHYs via devicetree for the roothub/controller and
>> disable/enable them when required:
>> - ehci-platform.c has ehci_platform_power_{on,off}
>> - xhci-mtk.c has xhci_mtk_phy_{init,exit,power_on,power_off}
>> - ohci-platform.c has ohci_platform_power_{on,off}
>>
>> These drivers are not using the generic devicetree USB device bindings
>> yet which were only introduced recently (documentation is available in
>> devicetree/bindings/usb/usb-device.txt).
>> With this new driver the usb2-phy and usb3-phy can be specified directly
>> in the child-node of the corresponding port of the roothub via
>> devicetree. This can be extended by not just parsing PHYs (some of the
>> other drivers listed above are for example also parsing a list of clocks
>> as well) when required.
>
>
> usb_add_hcd() in usb/core/hcd.c is already finding, initializing and turning
> on a phy, would it make sense to expand that one to support several phys
> instead?
>
> xhci will add two hcd's one for USB2 and one for USB3
that is a great suggestion - thank you for bringing this up!
as a benefit we would add multiple PHY support for all the other
use-cases I found (at least: ehci-platform.c, xhci-mtk.c,
ohci-platform.c) - instead of just handling this in xhci-plat.c

I have one quick question regarding usb/core/hcd.c:
are hcd_bus_suspend() and hcd_bus_resume() the right places to
power_{off,on} the PHYs during suspend?
(currently usb/core/hcd.c doesn't touch the PHY during suspend/resume
- xhci-mtk.c on the other hand seems to require it during a
suspend/resume cycle)

>>
>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>> Tested-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
>> ---
>>   drivers/usb/host/Kconfig            |   3 +
>>   drivers/usb/host/Makefile           |   2 +
>>   drivers/usb/host/platform-roothub.c | 180
>> ++++++++++++++++++++++++++++++++++++
>>   drivers/usb/host/platform-roothub.h |  12 +++
>>   4 files changed, 197 insertions(+)
>>   create mode 100644 drivers/usb/host/platform-roothub.c
>>   create mode 100644 drivers/usb/host/platform-roothub.h
>>
>
> Instead of creating  platform-roothub files could this content
> be added into into core/hcd.*, core/phy.* and host/xhci-plat.c
OK, I will try this and send a patch so we can have a look at the
potential result and start a discussion based on that (if required)

>
>> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
>> index fa5692dec832..b8b05c786b2a 100644
>> --- a/drivers/usb/host/Kconfig
>> +++ b/drivers/usb/host/Kconfig
>> @@ -805,6 +805,9 @@ config USB_HCD_SSB
>>
>>           If unsure, say N.
>>
>> +config USB_PLATFORM_ROOTHUB
>> +       bool
>> +
>>   config USB_HCD_TEST_MODE
>>         bool "HCD test mode support"
>>         ---help---
>> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
>> index cf2691fffcc0..dc817f82d632 100644
>> --- a/drivers/usb/host/Makefile
>> +++ b/drivers/usb/host/Makefile
>> @@ -29,6 +29,8 @@ obj-$(CONFIG_USB_WHCI_HCD)    += whci/
>>
>>   obj-$(CONFIG_USB_PCI) += pci-quirks.o
>>
>> +obj-$(CONFIG_USB_PLATFORM_ROOTHUB)     += platform-roothub.o
>> +
>>   obj-$(CONFIG_USB_EHCI_HCD)    += ehci-hcd.o
>>   obj-$(CONFIG_USB_EHCI_PCI)    += ehci-pci.o
>>   obj-$(CONFIG_USB_EHCI_HCD_PLATFORM)   += ehci-platform.o
>> diff --git a/drivers/usb/host/platform-roothub.c
>> b/drivers/usb/host/platform-roothub.c
>> new file mode 100644
>> index 000000000000..70d2d97aa8b2
>> --- /dev/null
>> +++ b/drivers/usb/host/platform-roothub.c
>> @@ -0,0 +1,180 @@
>> +/*
>> + * platform roothub driver - a virtual PHY device which passes all phy_*
>> + * function calls to multiple (actual) PHY devices. This is comes handy
>> when
>> + * initializing all PHYs on a root-hub (to keep them all in the same
>> state).
>> + *
>> + * Copyright (C) 2017 Martin Blumenstingl
>> <martin.blumenstingl@googlemail.com>
>> + *
>> + * 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.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include <linux/device.h>
>> +#include <linux/list.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/of.h>
>> +#include <linux/usb/of.h>
>> +
>> +#include "platform-roothub.h"
>> +
>> +#define ROOTHUB_PORTNUM                0
>> +
>> +struct platform_roothub {
>> +       struct phy              *phy;
>> +       struct list_head        list;
>> +};
>> +
>> +static struct platform_roothub *platform_roothub_alloc(struct device
>> *dev)
>> +{
>> +       struct platform_roothub *roothub_entry;
>> +
>> +       roothub_entry = devm_kzalloc(dev, sizeof(*roothub_entry),
>> GFP_KERNEL);
>> +       if (!roothub_entry)
>> +               return ERR_PTR(-ENOMEM);
>> +
>> +       INIT_LIST_HEAD(&roothub_entry->list);
>> +
>> +       return roothub_entry;
>> +}
>> +
>> +static int platform_roothub_add_phy(struct device *dev,
>> +                                   struct device_node *port_np,
>> +                                   const char *con_id, struct list_head
>> *list)
>> +{
>> +       struct platform_roothub *roothub_entry;
>> +       struct phy *phy = devm_of_phy_get(dev, port_np, con_id);
>> +
>> +       if (IS_ERR_OR_NULL(phy)) {
>> +               if (!phy || PTR_ERR(phy) == -ENODEV)
>> +                       return 0;
>> +               else
>> +                       return PTR_ERR(phy);
>> +       }
>> +
>> +       roothub_entry = platform_roothub_alloc(dev);
>> +       if (IS_ERR(roothub_entry))
>> +               return PTR_ERR(roothub_entry);
>> +
>> +       roothub_entry->phy = phy;
>> +
>> +       list_add_tail(&roothub_entry->list, list);
>> +
>> +       return 0;
>> +}
>> +
>> +struct platform_roothub *platform_roothub_init(struct device *dev)
>> +{
>> +       struct device_node *roothub_np, *port_np;
>> +       struct platform_roothub *plat_roothub;
>> +       struct platform_roothub *roothub_entry;
>> +       struct list_head *head;
>> +       int err;
>> +
>> +       roothub_np = usb_of_get_child_node(dev->of_node, ROOTHUB_PORTNUM);
>> +       if (!of_device_is_available(roothub_np))
>> +               return NULL;
>> +
>> +       plat_roothub = platform_roothub_alloc(dev);
>> +       if (IS_ERR(plat_roothub))
>> +               return plat_roothub;
>> +
>> +       for_each_available_child_of_node(roothub_np, port_np) {
>> +               err = platform_roothub_add_phy(dev, port_np, "usb2-phy",
>> +                                              &plat_roothub->list);
>> +               if (err)
>> +                       goto err_out;
>> +
>> +               err = platform_roothub_add_phy(dev, port_np, "usb3-phy",
>> +                                              &plat_roothub->list);
>
>
> So if the first 10 ports have the same phy, and 11th and 12th have an other
> one, won't we end up
> with a phy list with 12 entries for 2 phys, and initialize and turn on the
> same first phy 10 times?
indeed, we would call phy_init() and phy_power_on() multiple times on
the same PHY.
however, this is not an issue since the PHY framework is doing
ref-counting for us (see [0] and [1] - the PHY driver's .init() and
.power_on() callbacks will only be called once for each PHY in your
scenario)
do you see any other issues with this?

> I'm also not sure I understand the reason for having the "usb3-phy" and
> "usb2-phy" phy-names
> for the ports if we anyways just add everything to one list.
the PHY devicetree bindings state that the "phy-names" property is
mandatory: [2]

when moving the whole multiple PHY logic to usb_add_hcd() then it even
makes sense to look for specific PHYs (imho):
- let's assume we have a XHCI controller with two ports
- both ports have a USB2 PHY, but only one has a USB3 PHY
- (this basically describes the situation on the Amlogic Meson GXL
SoCs, Mediatek uses similar designs)

this would result in the following flow:
1. usb_add_hcd is called for the high-speed HCD
2. we would parse the PHYs for the high-speed HCD
3. usb_add_hcd is called for the super-speed HCD
4. we would parse the PHYs for the super-speed HCD
depending on how we parse the PHYs (mapping the controller-type to
phy-name "usb2-phy" or "usb3-phy") we either end up with:
- 3 PHY handles (2x USB2, 1x USB3) if we take the
controller-type/phy-name into account
- 6 PHY handles (doubling the amount from the use-case above) if we don't

please let me know if there is an easier solution - I prefer simple
code myself, so I don't want to add complexity where it's not needed.


Regards,
Martin


[0] http://elixir.free-electrons.com/linux/v4.14-rc3/source/drivers/phy/phy-core.c#L231
[1] http://elixir.free-electrons.com/linux/v4.14-rc3/source/drivers/phy/phy-core.c#L296
[2] http://elixir.free-electrons.com/linux/v4.14-rc3/source/Documentation/devicetree/bindings/phy/phy-bindings.txt#L36

Re: [PATCH v4 2/3] usb: host: add a generic platform USB roothub driver

[Mathias Nyman]

On 07.10.2017 20:08, Martin Blumenstingl wrote:
> Hi Mathias,
>
> thank you for taking the time to go through my patch
>
> On Wed, Oct 4, 2017 at 3:05 PM, Mathias Nyman
> <mathias.nyman-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
>> On 04.09.2017 00:38, Martin Blumenstingl wrote:
>>>
>>> Many SoC platforms have separate devices for the USB PHY which are
>>> registered through the generic PHY framework. These PHYs have to be
>>> enabled to make the USB controller actually work. They also have to be
>>> disabled again on shutdown/suspend.
>>>
>>> Currently (at least) the following HCI platform drivers are using custom
>>> code to obtain all PHYs via devicetree for the roothub/controller and
>>> disable/enable them when required:
>>> - ehci-platform.c has ehci_platform_power_{on,off}
>>> - xhci-mtk.c has xhci_mtk_phy_{init,exit,power_on,power_off}
>>> - ohci-platform.c has ohci_platform_power_{on,off}
>>>
>>> These drivers are not using the generic devicetree USB device bindings
>>> yet which were only introduced recently (documentation is available in
>>> devicetree/bindings/usb/usb-device.txt).
>>> With this new driver the usb2-phy and usb3-phy can be specified directly
>>> in the child-node of the corresponding port of the roothub via
>>> devicetree. This can be extended by not just parsing PHYs (some of the
>>> other drivers listed above are for example also parsing a list of clocks
>>> as well) when required.
>>
>>
>> usb_add_hcd() in usb/core/hcd.c is already finding, initializing and turning
>> on a phy, would it make sense to expand that one to support several phys
>> instead?
>>
>> xhci will add two hcd's one for USB2 and one for USB3
> that is a great suggestion - thank you for bringing this up!
> as a benefit we would add multiple PHY support for all the other
> use-cases I found (at least: ehci-platform.c, xhci-mtk.c,
> ohci-platform.c) - instead of just handling this in xhci-plat.c
>
> I have one quick question regarding usb/core/hcd.c:
> are hcd_bus_suspend() and hcd_bus_resume() the right places to
> power_{off,on} the PHYs during suspend?
> (currently usb/core/hcd.c doesn't touch the PHY during suspend/resume
> - xhci-mtk.c on the other hand seems to require it during a
> suspend/resume cycle)

I'm not sure what would be the correct place, hcd_bus_suspend() will be called
twice for xhci, oce for each hcd, so then we need to make sure we only turn off
phys related to that hcd. Host controller can still be running when bus is suspended.

xhci-mtk turns off phy when host controller is suspended and stopped.

>>
>>
>> So if the first 10 ports have the same phy, and 11th and 12th have an other
>> one, won't we end up
>> with a phy list with 12 entries for 2 phys, and initialize and turn on the
>> same first phy 10 times?
> indeed, we would call phy_init() and phy_power_on() multiple times on
> the same PHY.
> however, this is not an issue since the PHY framework is doing
> ref-counting for us (see [0] and [1] - the PHY driver's .init() and
> .power_on() callbacks will only be called once for each PHY in your
> scenario)

Ok

> do you see any other issues with this?
>

Not really, fine by me.

>> I'm also not sure I understand the reason for having the "usb3-phy" and
>> "usb2-phy" phy-names
>> for the ports if we anyways just add everything to one list.
> the PHY devicetree bindings state that the "phy-names" property is
> mandatory: [2]
>
> when moving the whole multiple PHY logic to usb_add_hcd() then it even
> makes sense to look for specific PHYs (imho):
> - let's assume we have a XHCI controller with two ports
> - both ports have a USB2 PHY, but only one has a USB3 PHY
> - (this basically describes the situation on the Amlogic Meson GXL
> SoCs, Mediatek uses similar designs)
>
> this would result in the following flow:
> 1. usb_add_hcd is called for the high-speed HCD
> 2. we would parse the PHYs for the high-speed HCD
> 3. usb_add_hcd is called for the super-speed HCD
> 4. we would parse the PHYs for the super-speed HCD
> depending on how we parse the PHYs (mapping the controller-type to
> phy-name "usb2-phy" or "usb3-phy") we either end up with:
> - 3 PHY handles (2x USB2, 1x USB3) if we take the
> controller-type/phy-name into account
> - 6 PHY handles (doubling the amount from the use-case above) if we don't
>
> please let me know if there is an easier solution - I prefer simple
> code myself, so I don't want to add complexity where it's not needed.

Ah, ok, I'm used to the ACPI tables way of listing ports where a physical USB3 connector
(HS & SS) is actually described as two separate ports. one HS and one SS.
With this layout one port can have only one PHY.

The Amlogic Meson GXL with two physical USB connectors (USB2&3 and USB2 only)
would look something like this in ACPI:

Device (RHUB) {
	...
	Device (HS01) {...}
	Device (HS02) {...}
	Device (SS01) {...}

xhci register layout is similar.
Each port has its own port status/control register.
Depending on if a USB2 or USB3 device is connected to a physical connector it will trigger
different port status registers.

Not sure which way is better.

-Mathias

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

[PATCH v4 2/3] usb: host: add a generic platform USB roothub driver

[Mathias Nyman]

On 07.10.2017 20:08, Martin Blumenstingl wrote:
> Hi Mathias,
>
> thank you for taking the time to go through my patch
>
> On Wed, Oct 4, 2017 at 3:05 PM, Mathias Nyman
> <mathias.nyman@linux.intel.com> wrote:
>> On 04.09.2017 00:38, Martin Blumenstingl wrote:
>>>
>>> Many SoC platforms have separate devices for the USB PHY which are
>>> registered through the generic PHY framework. These PHYs have to be
>>> enabled to make the USB controller actually work. They also have to be
>>> disabled again on shutdown/suspend.
>>>
>>> Currently (at least) the following HCI platform drivers are using custom
>>> code to obtain all PHYs via devicetree for the roothub/controller and
>>> disable/enable them when required:
>>> - ehci-platform.c has ehci_platform_power_{on,off}
>>> - xhci-mtk.c has xhci_mtk_phy_{init,exit,power_on,power_off}
>>> - ohci-platform.c has ohci_platform_power_{on,off}
>>>
>>> These drivers are not using the generic devicetree USB device bindings
>>> yet which were only introduced recently (documentation is available in
>>> devicetree/bindings/usb/usb-device.txt).
>>> With this new driver the usb2-phy and usb3-phy can be specified directly
>>> in the child-node of the corresponding port of the roothub via
>>> devicetree. This can be extended by not just parsing PHYs (some of the
>>> other drivers listed above are for example also parsing a list of clocks
>>> as well) when required.
>>
>>
>> usb_add_hcd() in usb/core/hcd.c is already finding, initializing and turning
>> on a phy, would it make sense to expand that one to support several phys
>> instead?
>>
>> xhci will add two hcd's one for USB2 and one for USB3
> that is a great suggestion - thank you for bringing this up!
> as a benefit we would add multiple PHY support for all the other
> use-cases I found (at least: ehci-platform.c, xhci-mtk.c,
> ohci-platform.c) - instead of just handling this in xhci-plat.c
>
> I have one quick question regarding usb/core/hcd.c:
> are hcd_bus_suspend() and hcd_bus_resume() the right places to
> power_{off,on} the PHYs during suspend?
> (currently usb/core/hcd.c doesn't touch the PHY during suspend/resume
> - xhci-mtk.c on the other hand seems to require it during a
> suspend/resume cycle)

I'm not sure what would be the correct place, hcd_bus_suspend() will be called
twice for xhci, oce for each hcd, so then we need to make sure we only turn off
phys related to that hcd. Host controller can still be running when bus is suspended.

xhci-mtk turns off phy when host controller is suspended and stopped.

>>
>>
>> So if the first 10 ports have the same phy, and 11th and 12th have an other
>> one, won't we end up
>> with a phy list with 12 entries for 2 phys, and initialize and turn on the
>> same first phy 10 times?
> indeed, we would call phy_init() and phy_power_on() multiple times on
> the same PHY.
> however, this is not an issue since the PHY framework is doing
> ref-counting for us (see [0] and [1] - the PHY driver's .init() and
> .power_on() callbacks will only be called once for each PHY in your
> scenario)

Ok

> do you see any other issues with this?
>

Not really, fine by me.

>> I'm also not sure I understand the reason for having the "usb3-phy" and
>> "usb2-phy" phy-names
>> for the ports if we anyways just add everything to one list.
> the PHY devicetree bindings state that the "phy-names" property is
> mandatory: [2]
>
> when moving the whole multiple PHY logic to usb_add_hcd() then it even
> makes sense to look for specific PHYs (imho):
> - let's assume we have a XHCI controller with two ports
> - both ports have a USB2 PHY, but only one has a USB3 PHY
> - (this basically describes the situation on the Amlogic Meson GXL
> SoCs, Mediatek uses similar designs)
>
> this would result in the following flow:
> 1. usb_add_hcd is called for the high-speed HCD
> 2. we would parse the PHYs for the high-speed HCD
> 3. usb_add_hcd is called for the super-speed HCD
> 4. we would parse the PHYs for the super-speed HCD
> depending on how we parse the PHYs (mapping the controller-type to
> phy-name "usb2-phy" or "usb3-phy") we either end up with:
> - 3 PHY handles (2x USB2, 1x USB3) if we take the
> controller-type/phy-name into account
> - 6 PHY handles (doubling the amount from the use-case above) if we don't
>
> please let me know if there is an easier solution - I prefer simple
> code myself, so I don't want to add complexity where it's not needed.

Ah, ok, I'm used to the ACPI tables way of listing ports where a physical USB3 connector
(HS & SS) is actually described as two separate ports. one HS and one SS.
With this layout one port can have only one PHY.

The Amlogic Meson GXL with two physical USB connectors (USB2&3 and USB2 only)
would look something like this in ACPI:

Device (RHUB) {
	...
	Device (HS01) {...}
	Device (HS02) {...}
	Device (SS01) {...}

xhci register layout is similar.
Each port has its own port status/control register.
Depending on if a USB2 or USB3 device is connected to a physical connector it will trigger
different port status registers.

Not sure which way is better.

-Mathias