[RFCv2 usb-next 0/3] initialize (multiple) PHYs in xhci-plat

[RFCv2 usb-next 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 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


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                | 146 +++++++++++++++++++++
 drivers/usb/host/platform-roothub.h                |  14 ++
 drivers/usb/host/xhci-plat.c                       |  27 +++-
 drivers/usb/host/xhci.h                            |   2 +
 8 files changed, 247 insertions(+), 1 deletion(-)
 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.13.2

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

[RFCv2 usb-next 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.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 .../devicetree/bindings/usb/usb-roothub.txt        | 46 ++++++++++++++++++++++
 1 file changed, 46 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";
+			};
+		};
+	}
-- 
2.13.2

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

Re: [RFCv2 usb-next 1/3] dt-bindings: usb: add the documentation for USB root-hub

[Rob Herring]

On Thu, Jul 13, 2017 at 12:59:37PM +0200, Martin Blumenstingl wrote:
> 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.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> ---
>  .../devicetree/bindings/usb/usb-roothub.txt        | 46 ++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/usb-roothub.txt

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
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

[RFCv2 usb-next 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>
---
 drivers/usb/host/Kconfig            |   3 +
 drivers/usb/host/Makefile           |   2 +
 drivers/usb/host/platform-roothub.c | 146 ++++++++++++++++++++++++++++++++++++
 drivers/usb/host/platform-roothub.h |  14 ++++
 4 files changed, 165 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..84837e42b006
--- /dev/null
+++ b/drivers/usb/host/platform-roothub.c
@@ -0,0 +1,146 @@
+/*
+ * 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;
+	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)
+			return ERR_PTR(err);
+
+		err = platform_roothub_add_phy(dev, port_np, "usb3-phy",
+					       &plat_roothub->list);
+		if (err)
+			return ERR_PTR(err);
+	}
+
+	return plat_roothub;
+}
+EXPORT_SYMBOL_GPL(platform_roothub_init);
+
+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_init(roothub_entry->phy);
+		if (err)
+			goto err_out;
+
+		err = phy_power_on(roothub_entry->phy);
+		if (err) {
+			phy_exit(roothub_entry->phy);
+			goto err_out;
+		}
+	}
+
+	return 0;
+
+err_out:
+	list_for_each_entry_continue_reverse(roothub_entry, head, list) {
+		phy_power_off(roothub_entry->phy);
+		phy_exit(roothub_entry->phy);
+	}
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(platform_roothub_power_on);
+
+void platform_roothub_power_off(struct platform_roothub *plat_roothub)
+{
+	struct platform_roothub *roothub_entry;
+
+	if (!plat_roothub)
+		return;
+
+	list_for_each_entry_reverse(roothub_entry, &plat_roothub->list, list) {
+		phy_power_off(roothub_entry->phy);
+		phy_exit(roothub_entry->phy);
+	}
+}
+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..bde0bf299e3b
--- /dev/null
+++ b/drivers/usb/host/platform-roothub.h
@@ -0,0 +1,14 @@
+#ifndef USB_HOST_PLATFORM_ROOTHUB_H
+#define USB_HOST_PLATFORM_ROOTHUB_H
+
+struct phy;
+struct device_node;
+
+struct platform_roothub;
+
+struct platform_roothub *platform_roothub_init(struct device *dev);
+
+int platform_roothub_power_on(struct platform_roothub *plat_roothub);
+void platform_roothub_power_off(struct platform_roothub *plat_roothub);
+
+#endif /* USB_HOST_PLATFORM_ROOTHUB_H */
-- 
2.13.2

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

Re: [RFCv2 usb-next 2/3] usb: host: add a generic platform USB roothub driver

[Chunfeng Yun]

On Thu, 2017-07-13 at 12:59 +0200, 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.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> ---
>  drivers/usb/host/Kconfig            |   3 +
>  drivers/usb/host/Makefile           |   2 +
>  drivers/usb/host/platform-roothub.c | 146 ++++++++++++++++++++++++++++++++++++
>  drivers/usb/host/platform-roothub.h |  14 ++++
>  4 files changed, 165 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..84837e42b006
> --- /dev/null
> +++ b/drivers/usb/host/platform-roothub.c
> @@ -0,0 +1,146 @@
> +/*
> + * 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;
> +	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)
> +			return ERR_PTR(err);
> +
> +		err = platform_roothub_add_phy(dev, port_np, "usb3-phy",
> +					       &plat_roothub->list);
> +		if (err)
> +			return ERR_PTR(err);
> +	}
> +
> +	return plat_roothub;
> +}
> +EXPORT_SYMBOL_GPL(platform_roothub_init);
Can we process the case without phy-names property?
as following one, phy-names may be redundant, because the phy type is
informed in phys property.

port@2 {
    reg = <2>;
    phys = <&u2port1 PHY_TYPE_USB2>, <&u3port1 PHY_TYPE_USB3>;
    phy-names = "usb2-phy", "usb3-phy";
};


> +
> +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_init(roothub_entry->phy);
> +		if (err)
> +			goto err_out;
> +
> +		err = phy_power_on(roothub_entry->phy);
> +		if (err) {
> +			phy_exit(roothub_entry->phy);
> +			goto err_out;
> +		}
> +	}
> +
> +	return 0;
> +
> +err_out:
> +	list_for_each_entry_continue_reverse(roothub_entry, head, list) {
> +		phy_power_off(roothub_entry->phy);
> +		phy_exit(roothub_entry->phy);
> +	}
> +
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(platform_roothub_power_on);
> +
> +void platform_roothub_power_off(struct platform_roothub *plat_roothub)
> +{
> +	struct platform_roothub *roothub_entry;
> +
> +	if (!plat_roothub)
> +		return;
> +
> +	list_for_each_entry_reverse(roothub_entry, &plat_roothub->list, list) {
> +		phy_power_off(roothub_entry->phy);
> +		phy_exit(roothub_entry->phy);
> +	}
> +}
> +EXPORT_SYMBOL_GPL(platform_roothub_power_off);
I test it, and works fine on mt8173.
It will be flexible enough if some more APIs are provided, such as
porwer_on/off all phys but not init/exit them.
e.g. 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.

> diff --git a/drivers/usb/host/platform-roothub.h b/drivers/usb/host/platform-roothub.h
> new file mode 100644
> index 000000000000..bde0bf299e3b
> --- /dev/null
> +++ b/drivers/usb/host/platform-roothub.h
> @@ -0,0 +1,14 @@
> +#ifndef USB_HOST_PLATFORM_ROOTHUB_H
> +#define USB_HOST_PLATFORM_ROOTHUB_H
> +
> +struct phy;
> +struct device_node;
These two declarations are not needed, when I remove them, and still
compile pass.

> +
> +struct platform_roothub;
> +
> +struct platform_roothub *platform_roothub_init(struct device *dev);
> +
> +int platform_roothub_power_on(struct platform_roothub *plat_roothub);
> +void platform_roothub_power_off(struct platform_roothub *plat_roothub);
> +
> +#endif /* USB_HOST_PLATFORM_ROOTHUB_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

Re: [RFCv2 usb-next 2/3] usb: host: add a generic platform USB roothub driver

[Martin Blumenstingl]

On Tue, Jul 25, 2017 at 8:40 AM, Chunfeng Yun <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> On Thu, 2017-07-13 at 12:59 +0200, 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.
>>
>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>> ---
>>  drivers/usb/host/Kconfig            |   3 +
>>  drivers/usb/host/Makefile           |   2 +
>>  drivers/usb/host/platform-roothub.c | 146 ++++++++++++++++++++++++++++++++++++
>>  drivers/usb/host/platform-roothub.h |  14 ++++
>>  4 files changed, 165 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..84837e42b006
>> --- /dev/null
>> +++ b/drivers/usb/host/platform-roothub.c
>> @@ -0,0 +1,146 @@
>> +/*
>> + * 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;
>> +     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)
>> +                     return ERR_PTR(err);
>> +
>> +             err = platform_roothub_add_phy(dev, port_np, "usb3-phy",
>> +                                            &plat_roothub->list);
>> +             if (err)
>> +                     return ERR_PTR(err);
>> +     }
>> +
>> +     return plat_roothub;
>> +}
>> +EXPORT_SYMBOL_GPL(platform_roothub_init);
> Can we process the case without phy-names property?
> as following one, phy-names may be redundant, because the phy type is
> informed in phys property.
>
> port@2 {
>     reg = <2>;
>     phys = <&u2port1 PHY_TYPE_USB2>, <&u3port1 PHY_TYPE_USB3>;
>     phy-names = "usb2-phy", "usb3-phy";
> };
according to the PHY binding documentation the phy-names property is
mandatory: [0]
this would have to be discussed with the PHY framework maintainers

>
>> +
>> +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_init(roothub_entry->phy);
>> +             if (err)
>> +                     goto err_out;
>> +
>> +             err = phy_power_on(roothub_entry->phy);
>> +             if (err) {
>> +                     phy_exit(roothub_entry->phy);
>> +                     goto err_out;
>> +             }
>> +     }
>> +
>> +     return 0;
>> +
>> +err_out:
>> +     list_for_each_entry_continue_reverse(roothub_entry, head, list) {
>> +             phy_power_off(roothub_entry->phy);
>> +             phy_exit(roothub_entry->phy);
>> +     }
>> +
>> +     return err;
>> +}
>> +EXPORT_SYMBOL_GPL(platform_roothub_power_on);
>> +
>> +void platform_roothub_power_off(struct platform_roothub *plat_roothub)
>> +{
>> +     struct platform_roothub *roothub_entry;
>> +
>> +     if (!plat_roothub)
>> +             return;
>> +
>> +     list_for_each_entry_reverse(roothub_entry, &plat_roothub->list, list) {
>> +             phy_power_off(roothub_entry->phy);
>> +             phy_exit(roothub_entry->phy);
>> +     }
>> +}
>> +EXPORT_SYMBOL_GPL(platform_roothub_power_off);
> I test it, and works fine on mt8173.
many thanks!

> It will be flexible enough if some more APIs are provided, such as
> porwer_on/off all phys but not init/exit them.
> e.g. 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.
OK, that sounds reasonable - we don't have suspend support on the
Amlogic platforms yet so I couldn't test this. I will add separate
_init and _exit functions and use them in the appropriate places in
the third patch. thanks for this suggestion!

>
>> diff --git a/drivers/usb/host/platform-roothub.h b/drivers/usb/host/platform-roothub.h
>> new file mode 100644
>> index 000000000000..bde0bf299e3b
>> --- /dev/null
>> +++ b/drivers/usb/host/platform-roothub.h
>> @@ -0,0 +1,14 @@
>> +#ifndef USB_HOST_PLATFORM_ROOTHUB_H
>> +#define USB_HOST_PLATFORM_ROOTHUB_H
>> +
>> +struct phy;
>> +struct device_node;
> These two declarations are not needed, when I remove them, and still
> compile pass.
thanks for spotting - neither of these is used inside the header, so
you are right: they should be removed

>> +
>> +struct platform_roothub;
>> +
>> +struct platform_roothub *platform_roothub_init(struct device *dev);
>> +
>> +int platform_roothub_power_on(struct platform_roothub *plat_roothub);
>> +void platform_roothub_power_off(struct platform_roothub *plat_roothub);
>> +
>> +#endif /* USB_HOST_PLATFORM_ROOTHUB_H */
>
>

I will fix these issues and send an update next weekend.
it would be great if the USB maintainers (especially Mathias Nyman,
since you're the xhci-plat.c maintainer) could comment on this series


Regards,
Martin


[0] http://elixir.free-electrons.com/linux/latest/source/Documentation/devicetree/bindings/phy/phy-bindings.txt#L33
--
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

Re: [RFCv2 usb-next 2/3] usb: host: add a generic platform USB roothub driver

[Martin Blumenstingl]

Hi,

On Tue, Jul 25, 2017 at 8:40 AM, Chunfeng Yun <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> On Thu, 2017-07-13 at 12:59 +0200, 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.
>>
>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>> ---
>>  drivers/usb/host/Kconfig            |   3 +
>>  drivers/usb/host/Makefile           |   2 +
>>  drivers/usb/host/platform-roothub.c | 146 ++++++++++++++++++++++++++++++++++++
>>  drivers/usb/host/platform-roothub.h |  14 ++++
>>  4 files changed, 165 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..84837e42b006
>> --- /dev/null
>> +++ b/drivers/usb/host/platform-roothub.c
>> @@ -0,0 +1,146 @@
>> +/*
>> + * 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;
>> +     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)
>> +                     return ERR_PTR(err);
>> +
>> +             err = platform_roothub_add_phy(dev, port_np, "usb3-phy",
>> +                                            &plat_roothub->list);
>> +             if (err)
>> +                     return ERR_PTR(err);
>> +     }
>> +
>> +     return plat_roothub;
>> +}
>> +EXPORT_SYMBOL_GPL(platform_roothub_init);
> Can we process the case without phy-names property?
> as following one, phy-names may be redundant, because the phy type is
> informed in phys property.
>
> port@2 {
>     reg = <2>;
>     phys = <&u2port1 PHY_TYPE_USB2>, <&u3port1 PHY_TYPE_USB3>;
>     phy-names = "usb2-phy", "usb3-phy";
> };
>
>
>> +
>> +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_init(roothub_entry->phy);
>> +             if (err)
>> +                     goto err_out;
>> +
>> +             err = phy_power_on(roothub_entry->phy);
>> +             if (err) {
>> +                     phy_exit(roothub_entry->phy);
>> +                     goto err_out;
>> +             }
>> +     }
>> +
>> +     return 0;
>> +
>> +err_out:
>> +     list_for_each_entry_continue_reverse(roothub_entry, head, list) {
>> +             phy_power_off(roothub_entry->phy);
>> +             phy_exit(roothub_entry->phy);
>> +     }
>> +
>> +     return err;
>> +}
>> +EXPORT_SYMBOL_GPL(platform_roothub_power_on);
>> +
>> +void platform_roothub_power_off(struct platform_roothub *plat_roothub)
>> +{
>> +     struct platform_roothub *roothub_entry;
>> +
>> +     if (!plat_roothub)
>> +             return;
>> +
>> +     list_for_each_entry_reverse(roothub_entry, &plat_roothub->list, list) {
>> +             phy_power_off(roothub_entry->phy);
>> +             phy_exit(roothub_entry->phy);
>> +     }
>> +}
>> +EXPORT_SYMBOL_GPL(platform_roothub_power_off);
> I test it, and works fine on mt8173.
> It will be flexible enough if some more APIs are provided, such as
> porwer_on/off all phys but not init/exit them.
> e.g. 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.
RFCv3 contains a fix for this (which I can't properly test myself,
since the Amlogic platform currently does not have suspend support).
it would be great if you code give it another try (see [0]) - if it
works for you then a Tested-by, Reviewed-By or Acked-By would be
awesome!

>> diff --git a/drivers/usb/host/platform-roothub.h b/drivers/usb/host/platform-roothub.h
>> new file mode 100644
>> index 000000000000..bde0bf299e3b
>> --- /dev/null
>> +++ b/drivers/usb/host/platform-roothub.h
>> @@ -0,0 +1,14 @@
>> +#ifndef USB_HOST_PLATFORM_ROOTHUB_H
>> +#define USB_HOST_PLATFORM_ROOTHUB_H
>> +
>> +struct phy;
>> +struct device_node;
> These two declarations are not needed, when I remove them, and still
> compile pass.
>
>> +
>> +struct platform_roothub;
>> +
>> +struct platform_roothub *platform_roothub_init(struct device *dev);
>> +
>> +int platform_roothub_power_on(struct platform_roothub *plat_roothub);
>> +void platform_roothub_power_off(struct platform_roothub *plat_roothub);
>> +
>> +#endif /* USB_HOST_PLATFORM_ROOTHUB_H */
>
>


Regards,
Martin


[0] http://lists.infradead.org/pipermail/linux-amlogic/2017-August/004596.html
--
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

[RFCv2 usb-next 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>
---
 Documentation/devicetree/bindings/usb/usb-xhci.txt |  7 ++++++
 drivers/usb/host/Kconfig                           |  1 +
 drivers/usb/host/xhci-plat.c                       | 27 +++++++++++++++++++++-
 drivers/usb/host/xhci.h                            |  2 ++
 4 files changed, 36 insertions(+), 1 deletion(-)

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";
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 c04144b25a67..6fa27393e241 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"
@@ -285,10 +286,20 @@ static int xhci_plat_probe(struct platform_device *pdev)
 			goto put_usb3_hcd;
 	}
 
-	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
+	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 disable_usb_phy;
 
+	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
+	if (ret)
+		goto disable_plat_roothub;
+
 	if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
 		xhci->shared_hcd->can_do_streams = 1;
 
@@ -311,6 +322,9 @@ 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);
+
 disable_usb_phy:
 	usb_phy_shutdown(hcd->usb_phy);
 
@@ -342,6 +356,8 @@ 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);
+
 	usb_remove_hcd(hcd);
 	usb_put_hcd(xhci->shared_hcd);
 
@@ -374,6 +390,11 @@ static int __maybe_unused xhci_plat_suspend(struct device *dev)
 	if (!device_may_wakeup(dev) && !IS_ERR(xhci->clk))
 		clk_disable_unprepare(xhci->clk);
 
+	if (ret)
+		return ret;
+
+	platform_roothub_power_off(xhci->platform_roothub);
+
 	return ret;
 }
 
@@ -386,6 +407,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 3c6da1f93c84..1079ac1dc596 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1725,6 +1725,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.13.2

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

Re: [RFCv2 usb-next 3/3] usb: host: xhci: plat: integrate the platform-roothub

[Rob Herring]

On Thu, Jul 13, 2017 at 12:59:39PM +0200, Martin Blumenstingl wrote:
> 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>
> ---
>  Documentation/devicetree/bindings/usb/usb-xhci.txt |  7 ++++++

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

>  drivers/usb/host/Kconfig                           |  1 +
>  drivers/usb/host/xhci-plat.c                       | 27 +++++++++++++++++++++-
>  drivers/usb/host/xhci.h                            |  2 ++
>  4 files changed, 36 insertions(+), 1 deletion(-)
--
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

Re: [RFCv2 usb-next 0/3] initialize (multiple) PHYs in xhci-plat

[Chunfeng Yun]

Hi Martin,

On Thu, 2017-07-13 at 12:59 +0200, Martin Blumenstingl 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).
> 
How to handle the phy0(one u2phy and one u3phy) when port1 support
dual-role mode? leave them to peripheral side as felipe suggested
before? If so, no port1 node for roothub, is there any problem when
change the port1 to host-only mode?
 
> 
> 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
> 
> 
> 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                | 146 +++++++++++++++++++++
>  drivers/usb/host/platform-roothub.h                |  14 ++
>  drivers/usb/host/xhci-plat.c                       |  27 +++-
>  drivers/usb/host/xhci.h                            |   2 +
>  8 files changed, 247 insertions(+), 1 deletion(-)
>  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
> 


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

Re: [RFCv2 usb-next 0/3] initialize (multiple) PHYs in xhci-plat

[Martin Blumenstingl]

Hi,

On Sat, Jul 15, 2017 at 11:33 AM, Chunfeng Yun
<chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> Hi Martin,
>
> On Thu, 2017-07-13 at 12:59 +0200, Martin Blumenstingl 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).
>>
> How to handle the phy0(one u2phy and one u3phy) when port1 support
> dual-role mode? leave them to peripheral side as felipe suggested
> before? If so, no port1 node for roothub, is there any problem when
> change the port1 to host-only mode?
on Amlogic Meson GXL we have the following IP blocks:
- 2x USB2 PHYs, some external component has to tell them which mode
(host/device) they should operate in
- there is an additional (1x) USB3 PHY with built-in OTG detection logic

on Amlogic Meson GXL it could work like this:
USB2 and USB3 phy0 can be passed to the root-hub. Additionally the
USB2 phy0 could be passed to the USB3 PHY. The USB3 PHY would then
tell the USB2 PHY in which mode it should operate.

please note that device mode and OTG support on Amlogic Meson GXL is
more complicated, as it uses dwc2 and dwc3 controllers in combination:
- dwc3 is reponsible for host-only mode
- dwc2 is responsible for device-only mode
- OTG detection is done by the USB3 PHY

would you mind sharing a short overview of host/device/OTG support
works on Mediatek SoCs? I assume that the Amlogic Meson GXL
implementation is quite special, so comparing this with another
implementation (for example the Mediatek one) may help spotting
potential issues.

>>
>> 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
>>
>>
>> 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                | 146 +++++++++++++++++++++
>>  drivers/usb/host/platform-roothub.h                |  14 ++
>>  drivers/usb/host/xhci-plat.c                       |  27 +++-
>>  drivers/usb/host/xhci.h                            |   2 +
>>  8 files changed, 247 insertions(+), 1 deletion(-)
>>  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
>>
>
>


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

Re: [RFCv2 usb-next 0/3] initialize (multiple) PHYs in xhci-plat

[Chunfeng Yun]

Hi,
On Sat, 2017-07-15 at 14:11 +0200, Martin Blumenstingl wrote:
> Hi,
> 
> On Sat, Jul 15, 2017 at 11:33 AM, Chunfeng Yun
> <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> > Hi Martin,
> >
> > On Thu, 2017-07-13 at 12:59 +0200, Martin Blumenstingl 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).
> >>
> > How to handle the phy0(one u2phy and one u3phy) when port1 support
> > dual-role mode? leave them to peripheral side as felipe suggested
> > before? If so, no port1 node for roothub, is there any problem when
> > change the port1 to host-only mode?
> on Amlogic Meson GXL we have the following IP blocks:
> - 2x USB2 PHYs, some external component has to tell them which mode
> (host/device) they should operate in
> - there is an additional (1x) USB3 PHY with built-in OTG detection logic
> 
> on Amlogic Meson GXL it could work like this:
> USB2 and USB3 phy0 can be passed to the root-hub. Additionally the
> USB2 phy0 could be passed to the USB3 PHY. The USB3 PHY would then
> tell the USB2 PHY in which mode it should operate.
> 
> please note that device mode and OTG support on Amlogic Meson GXL is
> more complicated, as it uses dwc2 and dwc3 controllers in combination:
> - dwc3 is reponsible for host-only mode
> - dwc2 is responsible for device-only mode
> - OTG detection is done by the USB3 PHY
> 
> would you mind sharing a short overview of host/device/OTG support
> works on Mediatek SoCs? I assume that the Amlogic Meson GXL
> implementation is quite special, so comparing this with another
> implementation (for example the Mediatek one) may help spotting
> potential issues.
> 
MTK's mtu3 IP supports at most 5x USB2 phys and 4x USB3 phys. They work
as following:

1. device mode works as HS only:

u2phy0 --- dual-role/OTG

u2phy1 ---|
          +  U3 host-only
u3phy0 ---|

...
u2phy4 ---|
          +  U3 host-only
u3phy3 ---|
(e.g. MT8173 supports 2x u2phys and 1x u3phy, u2phy0 can work as
dual-role mode, u2phy1 & u3phy0 are host-only)

2. device mode works as HS & SS, or host only:

u2phy0 ---|
          + dual-role or host-only
u3phy0 ---|

...
u2phy3 ---|
          + U3 host-only
u3phy3 ---|

u2phy4 --- U2 host-only
(e.g. on MT2701, u2phy0 and u3phy0 work as host-only mode)

mtu3 driver supports host-only, device-only and dual-role mode(use IDDIG
pin), and will take all phys it needed, include host-only phys; 
But if just host-only mode is supported, we can skip mtu3 driver and
make use of xhci-mtk driver directly, then xhci-mtk will take all phys.

> >>
> >> 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
> >>
> >>
> >> 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                | 146 +++++++++++++++++++++
> >>  drivers/usb/host/platform-roothub.h                |  14 ++
> >>  drivers/usb/host/xhci-plat.c                       |  27 +++-
> >>  drivers/usb/host/xhci.h                            |   2 +
> >>  8 files changed, 247 insertions(+), 1 deletion(-)
> >>  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
> >>
> >
> >
> 
> 
> 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

Re: [RFCv2 usb-next 0/3] initialize (multiple) PHYs in xhci-plat

[Martin Blumenstingl]

Hi,

On Mon, Jul 17, 2017 at 9:21 AM, Chunfeng Yun <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> Hi,
> On Sat, 2017-07-15 at 14:11 +0200, Martin Blumenstingl wrote:
>> Hi,
>>
>> On Sat, Jul 15, 2017 at 11:33 AM, Chunfeng Yun
>> <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
>> > Hi Martin,
>> >
>> > On Thu, 2017-07-13 at 12:59 +0200, Martin Blumenstingl 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).
>> >>
>> > How to handle the phy0(one u2phy and one u3phy) when port1 support
>> > dual-role mode? leave them to peripheral side as felipe suggested
>> > before? If so, no port1 node for roothub, is there any problem when
>> > change the port1 to host-only mode?
>> on Amlogic Meson GXL we have the following IP blocks:
>> - 2x USB2 PHYs, some external component has to tell them which mode
>> (host/device) they should operate in
>> - there is an additional (1x) USB3 PHY with built-in OTG detection logic
>>
>> on Amlogic Meson GXL it could work like this:
>> USB2 and USB3 phy0 can be passed to the root-hub. Additionally the
>> USB2 phy0 could be passed to the USB3 PHY. The USB3 PHY would then
>> tell the USB2 PHY in which mode it should operate.
>>
>> please note that device mode and OTG support on Amlogic Meson GXL is
>> more complicated, as it uses dwc2 and dwc3 controllers in combination:
>> - dwc3 is reponsible for host-only mode
>> - dwc2 is responsible for device-only mode
>> - OTG detection is done by the USB3 PHY
>>
>> would you mind sharing a short overview of host/device/OTG support
>> works on Mediatek SoCs? I assume that the Amlogic Meson GXL
>> implementation is quite special, so comparing this with another
>> implementation (for example the Mediatek one) may help spotting
>> potential issues.
>>
> MTK's mtu3 IP supports at most 5x USB2 phys and 4x USB3 phys. They work
> as following:
thank you for sharing this!

> 1. device mode works as HS only:
>
> u2phy0 --- dual-role/OTG
>
> u2phy1 ---|
>           +  U3 host-only
> u3phy0 ---|
>
> ...
> u2phy4 ---|
>           +  U3 host-only
> u3phy3 ---|
> (e.g. MT8173 supports 2x u2phys and 1x u3phy, u2phy0 can work as
> dual-role mode, u2phy1 & u3phy0 are host-only)
>
> 2. device mode works as HS & SS, or host only:
>
> u2phy0 ---|
>           + dual-role or host-only
> u3phy0 ---|
>
> ...
> u2phy3 ---|
>           + U3 host-only
> u3phy3 ---|
>
> u2phy4 --- U2 host-only
> (e.g. on MT2701, u2phy0 and u3phy0 work as host-only mode)
OK, so in both cases only one port (with one u2phy and one u3phy) is
dual-role capable

> mtu3 driver supports host-only, device-only and dual-role mode(use IDDIG
> pin), and will take all phys it needed, include host-only phys;
> But if just host-only mode is supported, we can skip mtu3 driver and
> make use of xhci-mtk driver directly, then xhci-mtk will take all phys.
I see, in your example it's the mtu3
(Documentation/devicetree/bindings/usb/mt8173-mtu3.txt) which does the
mode switching. I assume that you're doing the host/device mode
switching through the extcon phandle (for example together with the
extcon-usb-gpio driver).

in you example: can't we *always* describe the roothub via devicetree
(just like in my example: [0])?
this means that (as you already mentioned) USB host-only support is now covered.
to handle dual-role (host/device switching) we now need to pass the
dual-role capable PHYs to whatever IP can detect the mode it should
operate in (in your case: mtu3, in Amlogic's case: the u3phy with
built-in mode detection logic -> the driver for this IP block should
call phy_set_mode(phy, PHY_MODE_USB_{HOST,DEVICE,OTG} accordingly).
here's a skeleton (stripped-down) of how the .dts could look like:
mtu3: usb@11271000 {
    compatible = "mediatek,mt8173-mtu3";
...
    /* MT2701 = 2nd example, for MT8173 = 1st example we would skip
the u3phy0 */
    /* only list the dual role capable PHYs here */
    phys = <&u3phy0>, <&u2phy0>;
    phy-names = "usb3-phy", "usb2-phy";

    usb_host: xhci@11270000 {
        compatible = "mediatek,mt8173-xhci";
        ...
        roothub@0 {
            /* includes all PHYs, including the dual role capable ones */
        };
    };
};

do you think that this would work for the Mediatek SoCs?
I've seen that the phy-mt65xx-usb3.c PHY driver does not have any
.set_mode callback - I assume it's simply because it doesn't need it
(as this is either managed by the hardware/IP block internally, or
through some firmware/mailbox mechanism).


Regards,
Mart

[0] http://lists.infradead.org/pipermail/linux-amlogic/2017-July/004305.html
--
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

Re: [RFCv2 usb-next 0/3] initialize (multiple) PHYs in xhci-plat

[Chunfeng Yun]

Hi,
On Mon, 2017-07-17 at 11:27 +0200, Martin Blumenstingl wrote:
> Hi,
> 
> On Mon, Jul 17, 2017 at 9:21 AM, Chunfeng Yun <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> > Hi,
> > On Sat, 2017-07-15 at 14:11 +0200, Martin Blumenstingl wrote:
> >> Hi,
> >>
> >> On Sat, Jul 15, 2017 at 11:33 AM, Chunfeng Yun
> >> <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> >> > Hi Martin,
> >> >
> >> > On Thu, 2017-07-13 at 12:59 +0200, Martin Blumenstingl 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).
> >> >>
> >> > How to handle the phy0(one u2phy and one u3phy) when port1 support
> >> > dual-role mode? leave them to peripheral side as felipe suggested
> >> > before? If so, no port1 node for roothub, is there any problem when
> >> > change the port1 to host-only mode?
> >> on Amlogic Meson GXL we have the following IP blocks:
> >> - 2x USB2 PHYs, some external component has to tell them which mode
> >> (host/device) they should operate in
> >> - there is an additional (1x) USB3 PHY with built-in OTG detection logic
> >>
> >> on Amlogic Meson GXL it could work like this:
> >> USB2 and USB3 phy0 can be passed to the root-hub. Additionally the
> >> USB2 phy0 could be passed to the USB3 PHY. The USB3 PHY would then
> >> tell the USB2 PHY in which mode it should operate.
> >>
> >> please note that device mode and OTG support on Amlogic Meson GXL is
> >> more complicated, as it uses dwc2 and dwc3 controllers in combination:
> >> - dwc3 is reponsible for host-only mode
> >> - dwc2 is responsible for device-only mode
> >> - OTG detection is done by the USB3 PHY
> >>
> >> would you mind sharing a short overview of host/device/OTG support
> >> works on Mediatek SoCs? I assume that the Amlogic Meson GXL
> >> implementation is quite special, so comparing this with another
> >> implementation (for example the Mediatek one) may help spotting
> >> potential issues.
> >>
> > MTK's mtu3 IP supports at most 5x USB2 phys and 4x USB3 phys. They work
> > as following:
> thank you for sharing this!
> 
> > 1. device mode works as HS only:
> >
> > u2phy0 --- dual-role/OTG
> >
> > u2phy1 ---|
> >           +  U3 host-only
> > u3phy0 ---|
> >
> > ...
> > u2phy4 ---|
> >           +  U3 host-only
> > u3phy3 ---|
> > (e.g. MT8173 supports 2x u2phys and 1x u3phy, u2phy0 can work as
> > dual-role mode, u2phy1 & u3phy0 are host-only)
> >
> > 2. device mode works as HS & SS, or host only:
> >
> > u2phy0 ---|
> >           + dual-role or host-only
> > u3phy0 ---|
> >
> > ...
> > u2phy3 ---|
> >           + U3 host-only
> > u3phy3 ---|
> >
> > u2phy4 --- U2 host-only
> > (e.g. on MT2701, u2phy0 and u3phy0 work as host-only mode)
> OK, so in both cases only one port (with one u2phy and one u3phy) is
> dual-role capable
Yes
> 
> > mtu3 driver supports host-only, device-only and dual-role mode(use IDDIG
> > pin), and will take all phys it needed, include host-only phys;
> > But if just host-only mode is supported, we can skip mtu3 driver and
> > make use of xhci-mtk driver directly, then xhci-mtk will take all phys.
> I see, in your example it's the mtu3
> (Documentation/devicetree/bindings/usb/mt8173-mtu3.txt) which does the
> mode switching. I assume that you're doing the host/device mode
> switching through the extcon phandle (for example together with the
> extcon-usb-gpio driver).
> 
> in you example: can't we *always* describe the roothub via devicetree
> (just like in my example: [0])?
> this means that (as you already mentioned) USB host-only support is now covered.
> to handle dual-role (host/device switching) we now need to pass the
> dual-role capable PHYs to whatever IP can detect the mode it should
> operate in (in your case: mtu3, in Amlogic's case: the u3phy with
> built-in mode detection logic -> the driver for this IP block should
> call phy_set_mode(phy, PHY_MODE_USB_{HOST,DEVICE,OTG} accordingly).
> here's a skeleton (stripped-down) of how the .dts could look like:
> mtu3: usb@11271000 {
>     compatible = "mediatek,mt8173-mtu3";
> ...
>     /* MT2701 = 2nd example, for MT8173 = 1st example we would skip
> the u3phy0 */
>     /* only list the dual role capable PHYs here */
>     phys = <&u3phy0>, <&u2phy0>;
>     phy-names = "usb3-phy", "usb2-phy";
> 
>     usb_host: xhci@11270000 {
>         compatible = "mediatek,mt8173-xhci";
>         ...
>         roothub@0 {
>             /* includes all PHYs, including the dual role capable ones */
If here include dual-role capable phys which are also taken by parent
node(11271000), will cause phy_init/_power_on them twice.
it seems ok if here include host-only phys, but I need test it.
If here include host-only phys, there will not be port1 for roothub
node, maybe it's a problem.
>         };
>     };
> };
> 
> do you think that this would work for the Mediatek SoCs?
> I've seen that the phy-mt65xx-usb3.c PHY driver does not have any
> .set_mode callback - I assume it's simply because it doesn't need it
> (as this is either managed by the hardware/IP block internally, or
> through some firmware/mailbox mechanism).
Hardware can automatically switch host/device mode by IDDIG pin, but
also we can manually switch them. Phy driver makes use of auto way.
> 
> 
> Regards,
> Mart
> 
> [0] http://lists.infradead.org/pipermail/linux-amlogic/2017-July/004305.html


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

Re: [RFCv2 usb-next 0/3] initialize (multiple) PHYs in xhci-plat

[Martin Blumenstingl]

Hi,

On Tue, Jul 18, 2017 at 3:40 AM, Chunfeng Yun <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> Hi,
> On Mon, 2017-07-17 at 11:27 +0200, Martin Blumenstingl wrote:
>> Hi,
>>
>> On Mon, Jul 17, 2017 at 9:21 AM, Chunfeng Yun <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
>> > Hi,
>> > On Sat, 2017-07-15 at 14:11 +0200, Martin Blumenstingl wrote:
>> >> Hi,
>> >>
>> >> On Sat, Jul 15, 2017 at 11:33 AM, Chunfeng Yun
>> >> <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
>> >> > Hi Martin,
>> >> >
>> >> > On Thu, 2017-07-13 at 12:59 +0200, Martin Blumenstingl 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).
>> >> >>
>> >> > How to handle the phy0(one u2phy and one u3phy) when port1 support
>> >> > dual-role mode? leave them to peripheral side as felipe suggested
>> >> > before? If so, no port1 node for roothub, is there any problem when
>> >> > change the port1 to host-only mode?
>> >> on Amlogic Meson GXL we have the following IP blocks:
>> >> - 2x USB2 PHYs, some external component has to tell them which mode
>> >> (host/device) they should operate in
>> >> - there is an additional (1x) USB3 PHY with built-in OTG detection logic
>> >>
>> >> on Amlogic Meson GXL it could work like this:
>> >> USB2 and USB3 phy0 can be passed to the root-hub. Additionally the
>> >> USB2 phy0 could be passed to the USB3 PHY. The USB3 PHY would then
>> >> tell the USB2 PHY in which mode it should operate.
>> >>
>> >> please note that device mode and OTG support on Amlogic Meson GXL is
>> >> more complicated, as it uses dwc2 and dwc3 controllers in combination:
>> >> - dwc3 is reponsible for host-only mode
>> >> - dwc2 is responsible for device-only mode
>> >> - OTG detection is done by the USB3 PHY
>> >>
>> >> would you mind sharing a short overview of host/device/OTG support
>> >> works on Mediatek SoCs? I assume that the Amlogic Meson GXL
>> >> implementation is quite special, so comparing this with another
>> >> implementation (for example the Mediatek one) may help spotting
>> >> potential issues.
>> >>
>> > MTK's mtu3 IP supports at most 5x USB2 phys and 4x USB3 phys. They work
>> > as following:
>> thank you for sharing this!
>>
>> > 1. device mode works as HS only:
>> >
>> > u2phy0 --- dual-role/OTG
>> >
>> > u2phy1 ---|
>> >           +  U3 host-only
>> > u3phy0 ---|
>> >
>> > ...
>> > u2phy4 ---|
>> >           +  U3 host-only
>> > u3phy3 ---|
>> > (e.g. MT8173 supports 2x u2phys and 1x u3phy, u2phy0 can work as
>> > dual-role mode, u2phy1 & u3phy0 are host-only)
>> >
>> > 2. device mode works as HS & SS, or host only:
>> >
>> > u2phy0 ---|
>> >           + dual-role or host-only
>> > u3phy0 ---|
>> >
>> > ...
>> > u2phy3 ---|
>> >           + U3 host-only
>> > u3phy3 ---|
>> >
>> > u2phy4 --- U2 host-only
>> > (e.g. on MT2701, u2phy0 and u3phy0 work as host-only mode)
>> OK, so in both cases only one port (with one u2phy and one u3phy) is
>> dual-role capable
> Yes
>>
>> > mtu3 driver supports host-only, device-only and dual-role mode(use IDDIG
>> > pin), and will take all phys it needed, include host-only phys;
>> > But if just host-only mode is supported, we can skip mtu3 driver and
>> > make use of xhci-mtk driver directly, then xhci-mtk will take all phys.
>> I see, in your example it's the mtu3
>> (Documentation/devicetree/bindings/usb/mt8173-mtu3.txt) which does the
>> mode switching. I assume that you're doing the host/device mode
>> switching through the extcon phandle (for example together with the
>> extcon-usb-gpio driver).
>>
>> in you example: can't we *always* describe the roothub via devicetree
>> (just like in my example: [0])?
>> this means that (as you already mentioned) USB host-only support is now covered.
>> to handle dual-role (host/device switching) we now need to pass the
>> dual-role capable PHYs to whatever IP can detect the mode it should
>> operate in (in your case: mtu3, in Amlogic's case: the u3phy with
>> built-in mode detection logic -> the driver for this IP block should
>> call phy_set_mode(phy, PHY_MODE_USB_{HOST,DEVICE,OTG} accordingly).
>> here's a skeleton (stripped-down) of how the .dts could look like:
>> mtu3: usb@11271000 {
>>     compatible = "mediatek,mt8173-mtu3";
>> ...
>>     /* MT2701 = 2nd example, for MT8173 = 1st example we would skip
>> the u3phy0 */
>>     /* only list the dual role capable PHYs here */
>>     phys = <&u3phy0>, <&u2phy0>;
>>     phy-names = "usb3-phy", "usb2-phy";
>>
>>     usb_host: xhci@11270000 {
>>         compatible = "mediatek,mt8173-xhci";
>>         ...
>>         roothub@0 {
>>             /* includes all PHYs, including the dual role capable ones */
> If here include dual-role capable phys which are also taken by parent
> node(11271000), will cause phy_init/_power_on them twice.
the PHY framework does ref-counting for all calls [0], so
phy_ops.init/power_on will only be called for the first time
phy_{init,power_on} is called
also phy_ops.exit/power_off is only called when the last consumer
calls phy_{exit,power_off}

> it seems ok if here include host-only phys, but I need test it.
> If here include host-only phys, there will not be port1 for roothub
> node, maybe it's a problem.
on Amlogic's GXL SoC it also makes sense to pass the OTG capable USB2
PHY to the roothub (remember: the USB3 PHY does mode-detection for the
USB2 PHY)
this way host-only mode works even if the USB3 PHY driver is not
loaded (or the kernel module is unloaded). and it gets even worse: if
one of the USB2 PHYs is powered off then host-only mode is broken for
the other PHYs as well!

additionally the device-tree should describe the hardware - and the if
the dual-role capable PHYs are controlled by two different IP blocks:
why not add them to both (and let the PHY framework or the IP block -
like the Mediatek USB PHY IP does - handle this)

>>         };
>>     };
>> };
>>
>> do you think that this would work for the Mediatek SoCs?
>> I've seen that the phy-mt65xx-usb3.c PHY driver does not have any
>> .set_mode callback - I assume it's simply because it doesn't need it
>> (as this is either managed by the hardware/IP block internally, or
>> through some firmware/mailbox mechanism).
> Hardware can automatically switch host/device mode by IDDIG pin, but
> also we can manually switch them. Phy driver makes use of auto way.
OK, that makes it easy for the driver developers :)

>>
>>
>> Regards,
>> Mart
>>
>> [0] http://lists.infradead.org/pipermail/linux-amlogic/2017-July/004305.html
>
>

Regards,
Martin

[0] http://elixir.free-electrons.com/linux/latest/source/drivers/phy/phy-core.c#L218
--
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

Re: [RFCv2 usb-next 0/3] initialize (multiple) PHYs in xhci-plat

[Chunfeng Yun]

On Tue, 2017-07-18 at 10:19 +0200, Martin Blumenstingl wrote:
> Hi,
> 
> On Tue, Jul 18, 2017 at 3:40 AM, Chunfeng Yun <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> > Hi,
> > On Mon, 2017-07-17 at 11:27 +0200, Martin Blumenstingl wrote:
> >> Hi,
> >>
> >> On Mon, Jul 17, 2017 at 9:21 AM, Chunfeng Yun <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> >> > Hi,
> >> > On Sat, 2017-07-15 at 14:11 +0200, Martin Blumenstingl wrote:
> >> >> Hi,
> >> >>
> >> >> On Sat, Jul 15, 2017 at 11:33 AM, Chunfeng Yun
> >> >> <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> >> >> > Hi Martin,
> >> >> >
> >> >> > On Thu, 2017-07-13 at 12:59 +0200, Martin Blumenstingl 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).
> >> >> >>
> >> >> > How to handle the phy0(one u2phy and one u3phy) when port1 support
> >> >> > dual-role mode? leave them to peripheral side as felipe suggested
> >> >> > before? If so, no port1 node for roothub, is there any problem when
> >> >> > change the port1 to host-only mode?
> >> >> on Amlogic Meson GXL we have the following IP blocks:
> >> >> - 2x USB2 PHYs, some external component has to tell them which mode
> >> >> (host/device) they should operate in
> >> >> - there is an additional (1x) USB3 PHY with built-in OTG detection logic
> >> >>
> >> >> on Amlogic Meson GXL it could work like this:
> >> >> USB2 and USB3 phy0 can be passed to the root-hub. Additionally the
> >> >> USB2 phy0 could be passed to the USB3 PHY. The USB3 PHY would then
> >> >> tell the USB2 PHY in which mode it should operate.
> >> >>
> >> >> please note that device mode and OTG support on Amlogic Meson GXL is
> >> >> more complicated, as it uses dwc2 and dwc3 controllers in combination:
> >> >> - dwc3 is reponsible for host-only mode
> >> >> - dwc2 is responsible for device-only mode
> >> >> - OTG detection is done by the USB3 PHY
> >> >>
> >> >> would you mind sharing a short overview of host/device/OTG support
> >> >> works on Mediatek SoCs? I assume that the Amlogic Meson GXL
> >> >> implementation is quite special, so comparing this with another
> >> >> implementation (for example the Mediatek one) may help spotting
> >> >> potential issues.
> >> >>
> >> > MTK's mtu3 IP supports at most 5x USB2 phys and 4x USB3 phys. They work
> >> > as following:
> >> thank you for sharing this!
> >>
> >> > 1. device mode works as HS only:
> >> >
> >> > u2phy0 --- dual-role/OTG
> >> >
> >> > u2phy1 ---|
> >> >           +  U3 host-only
> >> > u3phy0 ---|
> >> >
> >> > ...
> >> > u2phy4 ---|
> >> >           +  U3 host-only
> >> > u3phy3 ---|
> >> > (e.g. MT8173 supports 2x u2phys and 1x u3phy, u2phy0 can work as
> >> > dual-role mode, u2phy1 & u3phy0 are host-only)
> >> >
> >> > 2. device mode works as HS & SS, or host only:
> >> >
> >> > u2phy0 ---|
> >> >           + dual-role or host-only
> >> > u3phy0 ---|
> >> >
> >> > ...
> >> > u2phy3 ---|
> >> >           + U3 host-only
> >> > u3phy3 ---|
> >> >
> >> > u2phy4 --- U2 host-only
> >> > (e.g. on MT2701, u2phy0 and u3phy0 work as host-only mode)
> >> OK, so in both cases only one port (with one u2phy and one u3phy) is
> >> dual-role capable
> > Yes
> >>
> >> > mtu3 driver supports host-only, device-only and dual-role mode(use IDDIG
> >> > pin), and will take all phys it needed, include host-only phys;
> >> > But if just host-only mode is supported, we can skip mtu3 driver and
> >> > make use of xhci-mtk driver directly, then xhci-mtk will take all phys.
> >> I see, in your example it's the mtu3
> >> (Documentation/devicetree/bindings/usb/mt8173-mtu3.txt) which does the
> >> mode switching. I assume that you're doing the host/device mode
> >> switching through the extcon phandle (for example together with the
> >> extcon-usb-gpio driver).
> >>
> >> in you example: can't we *always* describe the roothub via devicetree
> >> (just like in my example: [0])?
> >> this means that (as you already mentioned) USB host-only support is now covered.
> >> to handle dual-role (host/device switching) we now need to pass the
> >> dual-role capable PHYs to whatever IP can detect the mode it should
> >> operate in (in your case: mtu3, in Amlogic's case: the u3phy with
> >> built-in mode detection logic -> the driver for this IP block should
> >> call phy_set_mode(phy, PHY_MODE_USB_{HOST,DEVICE,OTG} accordingly).
> >> here's a skeleton (stripped-down) of how the .dts could look like:
> >> mtu3: usb@11271000 {
> >>     compatible = "mediatek,mt8173-mtu3";
> >> ...
> >>     /* MT2701 = 2nd example, for MT8173 = 1st example we would skip
> >> the u3phy0 */
> >>     /* only list the dual role capable PHYs here */
> >>     phys = <&u3phy0>, <&u2phy0>;
> >>     phy-names = "usb3-phy", "usb2-phy";
> >>
> >>     usb_host: xhci@11270000 {
> >>         compatible = "mediatek,mt8173-xhci";
> >>         ...
> >>         roothub@0 {
> >>             /* includes all PHYs, including the dual role capable ones */
> > If here include dual-role capable phys which are also taken by parent
> > node(11271000), will cause phy_init/_power_on them twice.
> the PHY framework does ref-counting for all calls [0], so
> phy_ops.init/power_on will only be called for the first time
> phy_{init,power_on} is called
> also phy_ops.exit/power_off is only called when the last consumer
> calls phy_{exit,power_off}
Sorry, I forgot it
> 
> > it seems ok if here include host-only phys, but I need test it.
> > If here include host-only phys, there will not be port1 for roothub
> > node, maybe it's a problem.
> on Amlogic's GXL SoC it also makes sense to pass the OTG capable USB2
> PHY to the roothub (remember: the USB3 PHY does mode-detection for the
> USB2 PHY)
> this way host-only mode works even if the USB3 PHY driver is not
> loaded (or the kernel module is unloaded). and it gets even worse: if
> one of the USB2 PHYs is powered off then host-only mode is broken for
> the other PHYs as well!
It's better if all phys are independent 
> 
> additionally the device-tree should describe the hardware - and the if
> the dual-role capable PHYs are controlled by two different IP blocks:
> why not add them to both (and let the PHY framework or the IP block -
> like the Mediatek USB PHY IP does - handle this)
Ok, I will test these patches.
thanks a lot
> 
> >>         };
> >>     };
> >> };
> >>
> >> do you think that this would work for the Mediatek SoCs?
> >> I've seen that the phy-mt65xx-usb3.c PHY driver does not have any
> >> .set_mode callback - I assume it's simply because it doesn't need it
> >> (as this is either managed by the hardware/IP block internally, or
> >> through some firmware/mailbox mechanism).
> > Hardware can automatically switch host/device mode by IDDIG pin, but
> > also we can manually switch them. Phy driver makes use of auto way.
> OK, that makes it easy for the driver developers :)
> 
> >>
> >>
> >> Regards,
> >> Mart
> >>
> >> [0] http://lists.infradead.org/pipermail/linux-amlogic/2017-July/004305.html
> >
> >
> 
> Regards,
> Martin
> 
> [0] http://elixir.free-electrons.com/linux/latest/source/drivers/phy/phy-core.c#L218


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

Re: [RFCv2 usb-next 0/3] initialize (multiple) PHYs in xhci-plat

[Martin Blumenstingl]

Hi,

On Tue, Jul 18, 2017 at 12:33 PM, Chunfeng Yun
<chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> On Tue, 2017-07-18 at 10:19 +0200, Martin Blumenstingl wrote:
>> Hi,
>>
>> On Tue, Jul 18, 2017 at 3:40 AM, Chunfeng Yun <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
>> > Hi,
>> > On Mon, 2017-07-17 at 11:27 +0200, Martin Blumenstingl wrote:
>> >> Hi,
>> >>
>> >> On Mon, Jul 17, 2017 at 9:21 AM, Chunfeng Yun <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
>> >> > Hi,
>> >> > On Sat, 2017-07-15 at 14:11 +0200, Martin Blumenstingl wrote:
>> >> >> Hi,
>> >> >>
>> >> >> On Sat, Jul 15, 2017 at 11:33 AM, Chunfeng Yun
>> >> >> <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
>> >> >> > Hi Martin,
>> >> >> >
>> >> >> > On Thu, 2017-07-13 at 12:59 +0200, Martin Blumenstingl 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).
>> >> >> >>
>> >> >> > How to handle the phy0(one u2phy and one u3phy) when port1 support
>> >> >> > dual-role mode? leave them to peripheral side as felipe suggested
>> >> >> > before? If so, no port1 node for roothub, is there any problem when
>> >> >> > change the port1 to host-only mode?
>> >> >> on Amlogic Meson GXL we have the following IP blocks:
>> >> >> - 2x USB2 PHYs, some external component has to tell them which mode
>> >> >> (host/device) they should operate in
>> >> >> - there is an additional (1x) USB3 PHY with built-in OTG detection logic
>> >> >>
>> >> >> on Amlogic Meson GXL it could work like this:
>> >> >> USB2 and USB3 phy0 can be passed to the root-hub. Additionally the
>> >> >> USB2 phy0 could be passed to the USB3 PHY. The USB3 PHY would then
>> >> >> tell the USB2 PHY in which mode it should operate.
>> >> >>
>> >> >> please note that device mode and OTG support on Amlogic Meson GXL is
>> >> >> more complicated, as it uses dwc2 and dwc3 controllers in combination:
>> >> >> - dwc3 is reponsible for host-only mode
>> >> >> - dwc2 is responsible for device-only mode
>> >> >> - OTG detection is done by the USB3 PHY
>> >> >>
>> >> >> would you mind sharing a short overview of host/device/OTG support
>> >> >> works on Mediatek SoCs? I assume that the Amlogic Meson GXL
>> >> >> implementation is quite special, so comparing this with another
>> >> >> implementation (for example the Mediatek one) may help spotting
>> >> >> potential issues.
>> >> >>
>> >> > MTK's mtu3 IP supports at most 5x USB2 phys and 4x USB3 phys. They work
>> >> > as following:
>> >> thank you for sharing this!
>> >>
>> >> > 1. device mode works as HS only:
>> >> >
>> >> > u2phy0 --- dual-role/OTG
>> >> >
>> >> > u2phy1 ---|
>> >> >           +  U3 host-only
>> >> > u3phy0 ---|
>> >> >
>> >> > ...
>> >> > u2phy4 ---|
>> >> >           +  U3 host-only
>> >> > u3phy3 ---|
>> >> > (e.g. MT8173 supports 2x u2phys and 1x u3phy, u2phy0 can work as
>> >> > dual-role mode, u2phy1 & u3phy0 are host-only)
>> >> >
>> >> > 2. device mode works as HS & SS, or host only:
>> >> >
>> >> > u2phy0 ---|
>> >> >           + dual-role or host-only
>> >> > u3phy0 ---|
>> >> >
>> >> > ...
>> >> > u2phy3 ---|
>> >> >           + U3 host-only
>> >> > u3phy3 ---|
>> >> >
>> >> > u2phy4 --- U2 host-only
>> >> > (e.g. on MT2701, u2phy0 and u3phy0 work as host-only mode)
>> >> OK, so in both cases only one port (with one u2phy and one u3phy) is
>> >> dual-role capable
>> > Yes
>> >>
>> >> > mtu3 driver supports host-only, device-only and dual-role mode(use IDDIG
>> >> > pin), and will take all phys it needed, include host-only phys;
>> >> > But if just host-only mode is supported, we can skip mtu3 driver and
>> >> > make use of xhci-mtk driver directly, then xhci-mtk will take all phys.
>> >> I see, in your example it's the mtu3
>> >> (Documentation/devicetree/bindings/usb/mt8173-mtu3.txt) which does the
>> >> mode switching. I assume that you're doing the host/device mode
>> >> switching through the extcon phandle (for example together with the
>> >> extcon-usb-gpio driver).
>> >>
>> >> in you example: can't we *always* describe the roothub via devicetree
>> >> (just like in my example: [0])?
>> >> this means that (as you already mentioned) USB host-only support is now covered.
>> >> to handle dual-role (host/device switching) we now need to pass the
>> >> dual-role capable PHYs to whatever IP can detect the mode it should
>> >> operate in (in your case: mtu3, in Amlogic's case: the u3phy with
>> >> built-in mode detection logic -> the driver for this IP block should
>> >> call phy_set_mode(phy, PHY_MODE_USB_{HOST,DEVICE,OTG} accordingly).
>> >> here's a skeleton (stripped-down) of how the .dts could look like:
>> >> mtu3: usb@11271000 {
>> >>     compatible = "mediatek,mt8173-mtu3";
>> >> ...
>> >>     /* MT2701 = 2nd example, for MT8173 = 1st example we would skip
>> >> the u3phy0 */
>> >>     /* only list the dual role capable PHYs here */
>> >>     phys = <&u3phy0>, <&u2phy0>;
>> >>     phy-names = "usb3-phy", "usb2-phy";
>> >>
>> >>     usb_host: xhci@11270000 {
>> >>         compatible = "mediatek,mt8173-xhci";
>> >>         ...
>> >>         roothub@0 {
>> >>             /* includes all PHYs, including the dual role capable ones */
>> > If here include dual-role capable phys which are also taken by parent
>> > node(11271000), will cause phy_init/_power_on them twice.
>> the PHY framework does ref-counting for all calls [0], so
>> phy_ops.init/power_on will only be called for the first time
>> phy_{init,power_on} is called
>> also phy_ops.exit/power_off is only called when the last consumer
>> calls phy_{exit,power_off}
> Sorry, I forgot it
>>
>> > it seems ok if here include host-only phys, but I need test it.
>> > If here include host-only phys, there will not be port1 for roothub
>> > node, maybe it's a problem.
>> on Amlogic's GXL SoC it also makes sense to pass the OTG capable USB2
>> PHY to the roothub (remember: the USB3 PHY does mode-detection for the
>> USB2 PHY)
>> this way host-only mode works even if the USB3 PHY driver is not
>> loaded (or the kernel module is unloaded). and it gets even worse: if
>> one of the USB2 PHYs is powered off then host-only mode is broken for
>> the other PHYs as well!
> It's better if all phys are independent
>>
>> additionally the device-tree should describe the hardware - and the if
>> the dual-role capable PHYs are controlled by two different IP blocks:
>> why not add them to both (and let the PHY framework or the IP block -
>> like the Mediatek USB PHY IP does - handle this)
> Ok, I will test these patches.
it would be awesome if you could test my patches and report if they
work fine for the Mediatek SoCs as well!

> thanks a lot
thank you again for sharing details about your SoC and starting a
discussion about my implementation

>>
>> >>         };
>> >>     };
>> >> };
>> >>
>> >> do you think that this would work for the Mediatek SoCs?
>> >> I've seen that the phy-mt65xx-usb3.c PHY driver does not have any
>> >> .set_mode callback - I assume it's simply because it doesn't need it
>> >> (as this is either managed by the hardware/IP block internally, or
>> >> through some firmware/mailbox mechanism).
>> > Hardware can automatically switch host/device mode by IDDIG pin, but
>> > also we can manually switch them. Phy driver makes use of auto way.
>> OK, that makes it easy for the driver developers :)
>>
>> >>
>> >>
>> >> Regards,
>> >> Mart
>> >>
>> >> [0] http://lists.infradead.org/pipermail/linux-amlogic/2017-July/004305.html
>> >
>> >
>>
>> Regards,
>> Martin
>>
>> [0] http://elixir.free-electrons.com/linux/latest/source/drivers/phy/phy-core.c#L218
>
>

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