[PATCHv5 00/20] USB support for Armada 38x and Armada 375

[PATCHv5 00/20] USB support for Armada 38x and Armada 375

[Thomas Petazzoni]

Hello,

This patch set adds the USB support for the Armada 38x and Armada 375
SOCs. These SoCs use an xHCI but still need specific initialization,
mainly to setup the MBus memory windows. They also have another USB
controller for EHCI, identical to the one used on other mvebu SOCs.

This series is also available in
the branch USB-375-38x-3.15-rc1-V5
https://github.com/MISL-EBU-System-SW/mainline-public.git

Changes between v4 and v5:

 * Fixed a compile time problem when CONFIG_USB_XHCI_MVEBU was
   disabled. Noticed by Shimoda, Yoshihiro
   <yoshihiro.shimoda.uh-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>.

 * Added Acked-by from Alan Stern on patches 1 to 4.

Changes between v3 and v4:

 * Additional patch that removes the use of of_irq_parse_and_map() in
   ehci-orion, and just uses platform_get_irq() instead, to avoid a
   different case between DT and non-T.

 * Additional patch that renames the ehci-orion error handling goto
   labels to have more meaningful names, in preparation for handling
   of additional error cases in followup patches.

 * Additional patch to update the ehci-orion Device Tree binding
   documentation.

 * Merge of 375/38x XHCI support patches: one patch for the driver,
   one patch for the Device Tree binding documentation (instead of one
   patch of these, for each SoC).

 * Update of the XHCI Device Tree binding documentation to indicate
   the new optional clocks property.

 * Update of armada-375.dtsi to add missing spaces in the phy-names
   definition.

 * Update of armada-38x.dtsi to use 0x4000 instead of 0x3fff for the
   register area length. Noticed by Andrew Lunn.

 * Changed the PHY driver Kconfig option to only be enabled either
   when MACH_ARMADA_375 or COMPILE_TEST are enabled. In the previous
   version, the PHY driver was always enabled, regardless of the
   platform.

 * Various improvements to the PHY driver:

    - Rename USB2_PHY_CONFIG_ENABLE to USB2_PHY_CONFIG_DISABLE, as
      suggested by Ezequiel Garcia.
    - Simplify the logic of armada375_usb_phy_init() by handling the
      !cluster_phy->enable case first.
    - Use devm_ioremap_resource() in the ->probe() function instead of
      of_iomap().
    - Bail out from ->probe() when a PHY cannot be created.
    - Fix typos in comments.

 * Reworded the Kconfig prompt and help text of XHCI_MVEBU to indicate
   that it's for both 375 and 38x (it was only indicating 38x until
   now).

 * Changed the Makefile bit of the xhci-mvebu driver according to the
   suggestion of Felipe Balbi, so that things work properly when
   CONFIG_USB_XHCI_MVEBU is 'm'.

 * Various improvements in the ehci-orion driver:

    - Use better goto labels for error handling in ->probe().
    - Use devm_phy_optional_get() instead of devm_phy_get(), which
      allows to handle EPROBE_DEFER cases nicely.
    - Call phy_power_off() when needed (probe error handling, and
      remove).
    - Use __initconst instead of __initdata for override structure, as
      noticed by checkpatch.
    - Fix use after free errors noticed by Felipe Balbi.

 * Various improvements to the XHCI driver:

    - Don't make xhci_mvebu_mbus_config() an __init function, since
      it's called from probe(), which isn't in __init.
    - Don't use the buggy 'priv[0]' solution implemented in the
      previous version of the patch set (see in
      http://lists.infradead.org/pipermail/linux-arm-kernel/2014-May/253871.html
      the details of why it doesn't work). Instead, add a 'struct
      clk*' field in xhci_hcd to support the clock in xhci-plat,
      exactly like xhci_hcd has msix_count and msix_entries for
      xhci-pci.
    - Misc minor code style improvements.

Thomas

Gregory CLEMENT (17):
  usb: ehci-orion: fix clock reference leaking
  usb: ehci-orion: add optional PHY support
  usb: host: xhci-plat: sort the headers in alphabetic order
  usb: host: xhci-plat: add clock support
  usb: host: xhci-plat: add support for the Armada 375/38x XHCI
    controllers
  Documentation: dt-bindings: update xhci-platform DT binding
  phy: add support for USB cluster on the Armada 375 SoC
  Documentation: dt-bindings: document the Armada 375 USB cluster
    binding
  ARM: mvebu: add USB3 support for Armada 38x
  ARM: mvebu: add USB3 support for Armada 375
  ARM: configs: enable XHCI mvebu support in mvebu_v7_defconfig
  ARM: configs: enable XHCI mvebu support in multi_v7_defconfig
  ARM: mvebu: add Device Tree description of xHCI controllers on Armada
    38x
  ARM: mvebu: add Device Tree description of the EHCI controller on
    Armada 38x
  ARM: mvebu: add Device Tree description of USB cluster controller on
    Armada 375
  ARM: mvebu: add Device Tree description of the xHCI controller on
    Armada 375
  ARM: mvebu: add Device Tree description of the EHCI controller on
    Armada 375

Thomas Petazzoni (3):
  usb: ehci-orion: use platform_get_irq() for DT probing
  usb: ehci-orion: rename error goto labels in ehci_orion_drv_probe()
  Documentation: dt-bindings: update ehci-orion binding documentation

 .../bindings/phy/armada-375-usb-phy-cluster.txt    |  19 +++
 .../devicetree/bindings/usb/ehci-orion.txt         |   5 +
 Documentation/devicetree/bindings/usb/usb-xhci.txt |   7 +-
 arch/arm/boot/dts/armada-375-db.dts                |   8 ++
 arch/arm/boot/dts/armada-375.dtsi                  |  34 +++++
 arch/arm/boot/dts/armada-385-db.dts                |  12 ++
 arch/arm/boot/dts/armada-385-rd.dts                |   4 +
 arch/arm/boot/dts/armada-38x.dtsi                  |  25 ++++
 arch/arm/configs/multi_v7_defconfig                |   1 +
 arch/arm/configs/mvebu_v7_defconfig                |   1 +
 arch/arm/mach-mvebu/Kconfig                        |   2 +
 drivers/phy/Kconfig                                |   6 +
 drivers/phy/Makefile                               |   1 +
 drivers/phy/phy-armada375-usb2.c                   | 157 +++++++++++++++++++++
 drivers/usb/host/Kconfig                           |   8 ++
 drivers/usb/host/Makefile                          |   3 +
 drivers/usb/host/ehci-orion.c                      |  92 ++++++++----
 drivers/usb/host/xhci-mvebu.c                      |  70 +++++++++
 drivers/usb/host/xhci-mvebu.h                      |  21 +++
 drivers/usb/host/xhci-plat.c                       |  42 +++++-
 drivers/usb/host/xhci.h                            |   2 +
 21 files changed, 488 insertions(+), 32 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/phy/armada-375-usb-phy-cluster.txt
 create mode 100644 drivers/phy/phy-armada375-usb2.c
 create mode 100644 drivers/usb/host/xhci-mvebu.c
 create mode 100644 drivers/usb/host/xhci-mvebu.h

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

[PATCHv5 01/20] usb: ehci-orion: use platform_get_irq() for DT probing

[Thomas Petazzoni]

Commit 77dae54ab385033e488d8b07045bc7f8d931740f ('ARM: Kirkwood:
ehci-orion: Add device tree binding') added the Device Tree binding
for the ehci-orion driver. To achieve that with the irq, it used the
irq_of_parse_and_map() function when probed in DT-mode, and
platform_get_irq() when probed in non-DT mode.

This is not necessary: platform_get_irq() works just as fine in
DT-mode, since the conversion from DT information to 'struct resource'
is done by the generic layers of the kernel.

Therefore, this commit switches back to use just platform_get_irq().

Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Acked-by: Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org>
---
 drivers/usb/host/ehci-orion.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/usb/host/ehci-orion.c b/drivers/usb/host/ehci-orion.c
index 30d35e5..7728e83 100644
--- a/drivers/usb/host/ehci-orion.c
+++ b/drivers/usb/host/ehci-orion.c
@@ -154,10 +154,7 @@ static int ehci_orion_drv_probe(struct platform_device *pdev)
 
 	pr_debug("Initializing Orion-SoC USB Host Controller\n");
 
-	if (pdev->dev.of_node)
-		irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
-	else
-		irq = platform_get_irq(pdev, 0);
+	irq = platform_get_irq(pdev, 0);
 	if (irq <= 0) {
 		dev_err(&pdev->dev,
 			"Found HC with no IRQ. Check %s setup!\n",
-- 
1.9.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

[PATCHv5 02/20] usb: ehci-orion: rename error goto labels in ehci_orion_drv_probe()

[Thomas Petazzoni]

In preparation to the introduction of additional initialization steps
in ehci_orion_drv_probe(), we rename the error goto labels from err1,
err2 and err3 names to some more meaningful names.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Acked-by: Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org>
---
 drivers/usb/host/ehci-orion.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/host/ehci-orion.c b/drivers/usb/host/ehci-orion.c
index 7728e83..9298be7 100644
--- a/drivers/usb/host/ehci-orion.c
+++ b/drivers/usb/host/ehci-orion.c
@@ -160,7 +160,7 @@ static int ehci_orion_drv_probe(struct platform_device *pdev)
 			"Found HC with no IRQ. Check %s setup!\n",
 			dev_name(&pdev->dev));
 		err = -ENODEV;
-		goto err1;
+		goto err;
 	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -169,7 +169,7 @@ static int ehci_orion_drv_probe(struct platform_device *pdev)
 			"Found HC with no register addr. Check %s setup!\n",
 			dev_name(&pdev->dev));
 		err = -ENODEV;
-		goto err1;
+		goto err;
 	}
 
 	/*
@@ -179,12 +179,12 @@ static int ehci_orion_drv_probe(struct platform_device *pdev)
 	 */
 	err = dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
 	if (err)
-		goto err1;
+		goto err;
 
 	regs = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(regs)) {
 		err = PTR_ERR(regs);
-		goto err1;
+		goto err;
 	}
 
 	/* Not all platforms can gate the clock, so it is not
@@ -197,7 +197,7 @@ static int ehci_orion_drv_probe(struct platform_device *pdev)
 			&pdev->dev, dev_name(&pdev->dev));
 	if (!hcd) {
 		err = -ENOMEM;
-		goto err2;
+		goto err_create_hcd;
 	}
 
 	hcd->rsrc_start = res->start;
@@ -237,17 +237,17 @@ static int ehci_orion_drv_probe(struct platform_device *pdev)
 
 	err = usb_add_hcd(hcd, irq, IRQF_SHARED);
 	if (err)
-		goto err3;
+		goto err_add_hcd;
 
 	device_wakeup_enable(hcd->self.controller);
 	return 0;
 
-err3:
+err_add_hcd:
 	usb_put_hcd(hcd);
-err2:
+err_create_hcd:
 	if (!IS_ERR(clk))
 		clk_disable_unprepare(clk);
-err1:
+err:
 	dev_err(&pdev->dev, "init %s fail, %d\n",
 		dev_name(&pdev->dev), err);
 
-- 
1.9.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

[PATCHv5 03/20] usb: ehci-orion: fix clock reference leaking

[Thomas Petazzoni]

From: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

In order to disable the clock in the ->remove() function, a call to
devm_clk_get() is being made, which further increases the reference
count of the clock.

In order to clean this up, a private structure holding a pointer to
the clock is added using the override mechanism provided by the ehci
framework. This makes the driver clock handling much more logical.

The bug was introduced in v3.6, however the ehci framework allowing to
use the override mechanism has only been introduced in v3.8, so this
patch won't apply before it.

[Thomas: reword commit log, fix goto label names.]

Fixes: 8c869edaee07c623066266827371235fb9c12e01 ('ARM: Orion: EHCI: Add support for enabling clocks')
Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> # v3.8+
Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Acked-by: Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org>
---
 drivers/usb/host/ehci-orion.c | 45 ++++++++++++++++++++++++++++---------------
 1 file changed, 29 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/host/ehci-orion.c b/drivers/usb/host/ehci-orion.c
index 9298be7..9c98bac 100644
--- a/drivers/usb/host/ehci-orion.c
+++ b/drivers/usb/host/ehci-orion.c
@@ -42,6 +42,12 @@
 
 #define DRIVER_DESC "EHCI orion driver"
 
+#define hcd_to_orion_priv(h) ((struct orion_ehci_hcd *)hcd_to_ehci(h)->priv)
+
+struct orion_ehci_hcd {
+	struct clk *clk;
+};
+
 static const char hcd_name[] = "ehci-orion";
 
 static struct hc_driver __read_mostly ehci_orion_hc_driver;
@@ -137,6 +143,10 @@ ehci_orion_conf_mbus_windows(struct usb_hcd *hcd,
 	}
 }
 
+static const struct ehci_driver_overrides orion_overrides __initconst = {
+	.extra_priv_size =	sizeof(struct orion_ehci_hcd),
+};
+
 static int ehci_orion_drv_probe(struct platform_device *pdev)
 {
 	struct orion_ehci_data *pd = dev_get_platdata(&pdev->dev);
@@ -144,10 +154,10 @@ static int ehci_orion_drv_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct usb_hcd *hcd;
 	struct ehci_hcd *ehci;
-	struct clk *clk;
 	void __iomem *regs;
 	int irq, err;
 	enum orion_ehci_phy_ver phy_version;
+	struct orion_ehci_hcd *priv;
 
 	if (usb_disabled())
 		return -ENODEV;
@@ -187,17 +197,11 @@ static int ehci_orion_drv_probe(struct platform_device *pdev)
 		goto err;
 	}
 
-	/* Not all platforms can gate the clock, so it is not
-	   an error if the clock does not exists. */
-	clk = devm_clk_get(&pdev->dev, NULL);
-	if (!IS_ERR(clk))
-		clk_prepare_enable(clk);
-
 	hcd = usb_create_hcd(&ehci_orion_hc_driver,
 			&pdev->dev, dev_name(&pdev->dev));
 	if (!hcd) {
 		err = -ENOMEM;
-		goto err_create_hcd;
+		goto err;
 	}
 
 	hcd->rsrc_start = res->start;
@@ -208,6 +212,15 @@ static int ehci_orion_drv_probe(struct platform_device *pdev)
 	ehci->caps = hcd->regs + 0x100;
 	hcd->has_tt = 1;
 
+	priv = hcd_to_orion_priv(hcd);
+	/*
+	 * Not all platforms can gate the clock, so it is not an error if
+	 * the clock does not exists.
+	 */
+	priv->clk = devm_clk_get(&pdev->dev, NULL);
+	if (!IS_ERR(priv->clk))
+		clk_prepare_enable(priv->clk);
+
 	/*
 	 * (Re-)program MBUS remapping windows if we are asked to.
 	 */
@@ -243,10 +256,9 @@ static int ehci_orion_drv_probe(struct platform_device *pdev)
 	return 0;
 
 err_add_hcd:
+	if (!IS_ERR(priv->clk))
+		clk_disable_unprepare(priv->clk);
 	usb_put_hcd(hcd);
-err_create_hcd:
-	if (!IS_ERR(clk))
-		clk_disable_unprepare(clk);
 err:
 	dev_err(&pdev->dev, "init %s fail, %d\n",
 		dev_name(&pdev->dev), err);
@@ -257,14 +269,15 @@ err:
 static int ehci_orion_drv_remove(struct platform_device *pdev)
 {
 	struct usb_hcd *hcd = platform_get_drvdata(pdev);
-	struct clk *clk;
+	struct orion_ehci_hcd *priv = hcd_to_orion_priv(hcd);
 
 	usb_remove_hcd(hcd);
+
+	if (!IS_ERR(priv->clk))
+		clk_disable_unprepare(priv->clk);
+
 	usb_put_hcd(hcd);
 
-	clk = devm_clk_get(&pdev->dev, NULL);
-	if (!IS_ERR(clk))
-		clk_disable_unprepare(clk);
 	return 0;
 }
 
@@ -292,7 +305,7 @@ static int __init ehci_orion_init(void)
 
 	pr_info("%s: " DRIVER_DESC "\n", hcd_name);
 
-	ehci_init_driver(&ehci_orion_hc_driver, NULL);
+	ehci_init_driver(&ehci_orion_hc_driver, &orion_overrides);
 	return platform_driver_register(&ehci_orion_driver);
 }
 module_init(ehci_orion_init);
-- 
1.9.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

[PATCHv5 04/20] usb: ehci-orion: add optional PHY support

[Thomas Petazzoni]

From: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

This commit extends the ehci-orion so that it can optionally be passed
a reference to a PHY through the Device Tree. It will be useful for
the Armada 375 SoCs. If no PHY is provided then the behavior of the
driver is unchanged.

[Thomas: use devm_phy_optional_get() so that we handle -EPROBE_DEFER
properly. Also call phy_power_off() when needed, and rename goto
labels.]

Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Acked-by: Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org>
---
 drivers/usb/host/ehci-orion.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/usb/host/ehci-orion.c b/drivers/usb/host/ehci-orion.c
index 9c98bac..22e15ca 100644
--- a/drivers/usb/host/ehci-orion.c
+++ b/drivers/usb/host/ehci-orion.c
@@ -15,6 +15,7 @@
 #include <linux/clk.h>
 #include <linux/platform_data/usb-ehci-orion.h>
 #include <linux/of.h>
+#include <linux/phy/phy.h>
 #include <linux/of_device.h>
 #include <linux/of_irq.h>
 #include <linux/usb.h>
@@ -46,6 +47,7 @@
 
 struct orion_ehci_hcd {
 	struct clk *clk;
+	struct phy *phy;
 };
 
 static const char hcd_name[] = "ehci-orion";
@@ -221,6 +223,20 @@ static int ehci_orion_drv_probe(struct platform_device *pdev)
 	if (!IS_ERR(priv->clk))
 		clk_prepare_enable(priv->clk);
 
+	priv->phy = devm_phy_optional_get(&pdev->dev, "usb");
+	if (IS_ERR(priv->phy)) {
+		err = PTR_ERR(priv->phy);
+		goto err_phy_get;
+	} else {
+		err = phy_init(priv->phy);
+		if (err)
+			goto err_phy_init;
+
+		err = phy_power_on(priv->phy);
+		if (err)
+			goto err_phy_power_on;
+	}
+
 	/*
 	 * (Re-)program MBUS remapping windows if we are asked to.
 	 */
@@ -256,6 +272,13 @@ static int ehci_orion_drv_probe(struct platform_device *pdev)
 	return 0;
 
 err_add_hcd:
+	if (!IS_ERR(priv->phy))
+		phy_power_off(priv->phy);
+err_phy_power_on:
+	if (!IS_ERR(priv->phy))
+		phy_exit(priv->phy);
+err_phy_init:
+err_phy_get:
 	if (!IS_ERR(priv->clk))
 		clk_disable_unprepare(priv->clk);
 	usb_put_hcd(hcd);
@@ -273,6 +296,11 @@ static int ehci_orion_drv_remove(struct platform_device *pdev)
 
 	usb_remove_hcd(hcd);
 
+	if (!IS_ERR(priv->phy)) {
+		phy_power_off(priv->phy);
+		phy_exit(priv->phy);
+	}
+
 	if (!IS_ERR(priv->clk))
 		clk_disable_unprepare(priv->clk);
 
-- 
1.9.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

[PATCHv5 05/20] Documentation: dt-bindings: update ehci-orion binding documentation

[Thomas Petazzoni]

This commit updates the Device Tree binding documentation of
ehci-orion to take into account the fact that we can now optionally
pass a clock and a PHY reference.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 Documentation/devicetree/bindings/usb/ehci-orion.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/ehci-orion.txt b/Documentation/devicetree/bindings/usb/ehci-orion.txt
index 6bc09ec..17c3bc8 100644
--- a/Documentation/devicetree/bindings/usb/ehci-orion.txt
+++ b/Documentation/devicetree/bindings/usb/ehci-orion.txt
@@ -6,6 +6,11 @@ Required properties:
   region.
 - interrupts: The EHCI interrupt
 
+Optional properties:
+- clocks: reference to the clock
+- phys: reference to the USB PHY
+- phy-names: name of the USB PHY, should be "usb"
+
 Example:
 
 	ehci@50000 {
-- 
1.9.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: [PATCHv5 05/20] Documentation: dt-bindings: update ehci-orion binding documentation

[Gregory CLEMENT]

Hi Alan,

On 11/05/2014 20:17, Thomas Petazzoni wrote:
> This commit updates the Device Tree binding documentation of
> ehci-orion to take into account the fact that we can now optionally
> pass a clock and a PHY reference.

Would you agree to take this patch with the 4 first ones?

For this kind of change in the device tree the agreement was that it
can be directly taken by the maintainer of the subsystem. Indeed here
we don't introduce a new kind of device tree node, we just add a optional
resource already used in many place.

Thanks,

Gregory


> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/usb/ehci-orion.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/ehci-orion.txt b/Documentation/devicetree/bindings/usb/ehci-orion.txt
> index 6bc09ec..17c3bc8 100644
> --- a/Documentation/devicetree/bindings/usb/ehci-orion.txt
> +++ b/Documentation/devicetree/bindings/usb/ehci-orion.txt
> @@ -6,6 +6,11 @@ Required properties:
>    region.
>  - interrupts: The EHCI interrupt
>  
> +Optional properties:
> +- clocks: reference to the clock
> +- phys: reference to the USB PHY
> +- phy-names: name of the USB PHY, should be "usb"
> +
>  Example:
>  
>  	ehci@50000 {
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
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: [PATCHv5 05/20] Documentation: dt-bindings: update ehci-orion binding documentation

[Alan Stern]

On Mon, 12 May 2014, Gregory CLEMENT wrote:

> Hi Alan,
> 
> On 11/05/2014 20:17, Thomas Petazzoni wrote:
> > This commit updates the Device Tree binding documentation of
> > ehci-orion to take into account the fact that we can now optionally
> > pass a clock and a PHY reference.
> 
> Would you agree to take this patch with the 4 first ones?
> 
> For this kind of change in the device tree the agreement was that it
> can be directly taken by the maintainer of the subsystem. Indeed here
> we don't introduce a new kind of device tree node, we just add a optional
> resource already used in many place.

Sorry, I missed this one.  Is it okay with the DT people?  If it is, 
you can submit it directly to Greg KH and he'll merge it.

Alan Stern

> > Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> > ---
> >  Documentation/devicetree/bindings/usb/ehci-orion.txt | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/usb/ehci-orion.txt b/Documentation/devicetree/bindings/usb/ehci-orion.txt
> > index 6bc09ec..17c3bc8 100644
> > --- a/Documentation/devicetree/bindings/usb/ehci-orion.txt
> > +++ b/Documentation/devicetree/bindings/usb/ehci-orion.txt
> > @@ -6,6 +6,11 @@ Required properties:
> >    region.
> >  - interrupts: The EHCI interrupt
> >  
> > +Optional properties:
> > +- clocks: reference to the clock
> > +- phys: reference to the USB PHY
> > +- phy-names: name of the USB PHY, should be "usb"
> > +
> >  Example:
> >  
> >  	ehci@50000 {
> > 

--
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: [PATCHv5 05/20] Documentation: dt-bindings: update ehci-orion binding documentation

[Gregory CLEMENT]

On 12/05/2014 17:46, Alan Stern wrote:
> On Mon, 12 May 2014, Gregory CLEMENT wrote:
> 
>> Hi Alan,
>>
>> On 11/05/2014 20:17, Thomas Petazzoni wrote:
>>> This commit updates the Device Tree binding documentation of
>>> ehci-orion to take into account the fact that we can now optionally
>>> pass a clock and a PHY reference.
>>
>> Would you agree to take this patch with the 4 first ones?
>>
>> For this kind of change in the device tree the agreement was that it
>> can be directly taken by the maintainer of the subsystem. Indeed here
>> we don't introduce a new kind of device tree node, we just add a optional
>> resource already used in many place.
> 
> Sorry, I missed this one.  Is it okay with the DT people?  If it is, 

Rob, Mark,

could you confirm that for this kind of change in the dts documentation
you agree that the maintainer take it?


> you can submit it directly to Greg KH and he'll merge it.

OK thanks!

> 
> Alan Stern
> 
>>> Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>> ---
>>>  Documentation/devicetree/bindings/usb/ehci-orion.txt | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/ehci-orion.txt b/Documentation/devicetree/bindings/usb/ehci-orion.txt
>>> index 6bc09ec..17c3bc8 100644
>>> --- a/Documentation/devicetree/bindings/usb/ehci-orion.txt
>>> +++ b/Documentation/devicetree/bindings/usb/ehci-orion.txt
>>> @@ -6,6 +6,11 @@ Required properties:
>>>    region.
>>>  - interrupts: The EHCI interrupt
>>>  
>>> +Optional properties:
>>> +- clocks: reference to the clock
>>> +- phys: reference to the USB PHY
>>> +- phy-names: name of the USB PHY, should be "usb"
>>> +
>>>  Example:
>>>  
>>>  	ehci@50000 {
>>>
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
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

[PATCHv5 06/20] usb: host: xhci-plat: sort the headers in alphabetic order

[Thomas Petazzoni]

From: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

Sorting the headers in alphabetic order will help to reduce the conflict
when adding new headers later.

Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Acked-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 drivers/usb/host/xhci-plat.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 151901c..f5351af 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -11,11 +11,11 @@
  * version 2 as published by the Free Software Foundation.
  */
 
-#include <linux/platform_device.h>
+#include <linux/dma-mapping.h>
 #include <linux/module.h>
-#include <linux/slab.h>
 #include <linux/of.h>
-#include <linux/dma-mapping.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
 
 #include "xhci.h"
 
-- 
1.9.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

[PATCHv5 07/20] usb: host: xhci-plat: add clock support

[Thomas Petazzoni]

From: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

Some platforms (such as the Armada 38x ones) can gate the clock of
their USB controller. This patch adds the support for one clock in
xhci-plat, by enabling it during probe and disabling it on remove.

To achieve this, it adds a 'struct clk *' member in xhci_hcd. While
only used for now in xhci-plat, it might be used by other drivers in
the future. Moreover, the xhci_hcd structure already holds other
members such as msix_count and msix_entries, which are MSI-X specific,
and therefore only used by xhci-pci.

Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 drivers/usb/host/xhci-plat.c | 24 +++++++++++++++++++++++-
 drivers/usb/host/xhci.h      |  2 ++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index f5351af..8108e58 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -11,6 +11,7 @@
  * version 2 as published by the Free Software Foundation.
  */
 
+#include <linux/clk.h>
 #include <linux/dma-mapping.h>
 #include <linux/module.h>
 #include <linux/of.h>
@@ -91,6 +92,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	struct xhci_hcd		*xhci;
 	struct resource         *res;
 	struct usb_hcd		*hcd;
+	struct clk              *clk;
 	int			ret;
 	int			irq;
 
@@ -137,14 +139,27 @@ static int xhci_plat_probe(struct platform_device *pdev)
 		goto release_mem_region;
 	}
 
+	/*
+	 * Not all platforms have a clk so it is not an error if the
+	 * clock does not exists.
+	 */
+	clk = devm_clk_get(&pdev->dev, NULL);
+	if (!IS_ERR(clk)) {
+		ret = clk_prepare_enable(clk);
+		if (ret)
+			goto unmap_registers;
+	}
+
 	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
 	if (ret)
-		goto unmap_registers;
+		goto disable_clk;
+
 	device_wakeup_enable(hcd->self.controller);
 
 	/* USB 2.0 roothub is stored in the platform_device now. */
 	hcd = platform_get_drvdata(pdev);
 	xhci = hcd_to_xhci(hcd);
+	xhci->clk = clk;
 	xhci->shared_hcd = usb_create_shared_hcd(driver, &pdev->dev,
 			dev_name(&pdev->dev), hcd);
 	if (!xhci->shared_hcd) {
@@ -173,6 +188,10 @@ put_usb3_hcd:
 dealloc_usb2_hcd:
 	usb_remove_hcd(hcd);
 
+disable_clk:
+	if (!IS_ERR(clk))
+		clk_disable_unprepare(clk);
+
 unmap_registers:
 	iounmap(hcd->regs);
 
@@ -189,11 +208,14 @@ static int xhci_plat_remove(struct platform_device *dev)
 {
 	struct usb_hcd	*hcd = platform_get_drvdata(dev);
 	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
+	struct clk *clk = xhci->clk;
 
 	usb_remove_hcd(xhci->shared_hcd);
 	usb_put_hcd(xhci->shared_hcd);
 
 	usb_remove_hcd(hcd);
+	if (!IS_ERR(clk))
+		clk_disable_unprepare(clk);
 	iounmap(hcd->regs);
 	release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
 	usb_put_hcd(hcd);
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index d280e92..003dc09 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1478,6 +1478,8 @@ struct xhci_hcd {
 	/* msi-x vectors */
 	int		msix_count;
 	struct msix_entry	*msix_entries;
+	/* optional clock */
+	struct clk		*clk;
 	/* data structures */
 	struct xhci_device_context_array *dcbaa;
 	struct xhci_ring	*cmd_ring;
-- 
1.9.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: [PATCHv5 07/20] usb: host: xhci-plat: add clock support

[Gregory CLEMENT]

Hi Mathias, Felipe,

On 11/05/2014 20:17, Thomas Petazzoni wrote:
> From: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> 
> Some platforms (such as the Armada 38x ones) can gate the clock of
> their USB controller. This patch adds the support for one clock in
> xhci-plat, by enabling it during probe and disabling it on remove.
> 
> To achieve this, it adds a 'struct clk *' member in xhci_hcd. While
> only used for now in xhci-plat, it might be used by other drivers in
> the future. Moreover, the xhci_hcd structure already holds other
> members such as msix_count and msix_entries, which are MSI-X specific,
> and therefore only used by xhci-pci.

Felpie,

would you agree to add your acked-by on this patch. You gave it on
the 3rd version however as pointed by Thomas there was some issues,
what do you think of this version?


Mathias,

as the xHCI maintainer what is your opinion on this patch. Do you plan
to take it? do you expect a pull request?

We submitted the first version of this series one month ago and we didn't
receive any feedback from you. Maybe it was just because you were happy
with the patch sent, but I would like to be sure that you noticed this
series.

I have the same question for the patch before: "[PATCHv5 06/20] usb: host:
xhci-plat: sort the headers in alphabetic order" and the three following:
"[PATCHv5 07/20] usb: host: xhci-plat: add clock support", "[PATCHv5 08/20]
 usb: host: xhci-plat: add support for the Armada 375/38x XHCI controllers"
and "[PATCHv5 09/20] Documentation: dt-bindings: update xhci-platform DT
binding".


Thanks,

Gregory


> 
> Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
>  drivers/usb/host/xhci-plat.c | 24 +++++++++++++++++++++++-
>  drivers/usb/host/xhci.h      |  2 ++
>  2 files changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index f5351af..8108e58 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -11,6 +11,7 @@
>   * version 2 as published by the Free Software Foundation.
>   */
>  
> +#include <linux/clk.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> @@ -91,6 +92,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
>  	struct xhci_hcd		*xhci;
>  	struct resource         *res;
>  	struct usb_hcd		*hcd;
> +	struct clk              *clk;
>  	int			ret;
>  	int			irq;
>  
> @@ -137,14 +139,27 @@ static int xhci_plat_probe(struct platform_device *pdev)
>  		goto release_mem_region;
>  	}
>  
> +	/*
> +	 * Not all platforms have a clk so it is not an error if the
> +	 * clock does not exists.
> +	 */
> +	clk = devm_clk_get(&pdev->dev, NULL);
> +	if (!IS_ERR(clk)) {
> +		ret = clk_prepare_enable(clk);
> +		if (ret)
> +			goto unmap_registers;
> +	}
> +
>  	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
>  	if (ret)
> -		goto unmap_registers;
> +		goto disable_clk;
> +
>  	device_wakeup_enable(hcd->self.controller);
>  
>  	/* USB 2.0 roothub is stored in the platform_device now. */
>  	hcd = platform_get_drvdata(pdev);
>  	xhci = hcd_to_xhci(hcd);
> +	xhci->clk = clk;
>  	xhci->shared_hcd = usb_create_shared_hcd(driver, &pdev->dev,
>  			dev_name(&pdev->dev), hcd);
>  	if (!xhci->shared_hcd) {
> @@ -173,6 +188,10 @@ put_usb3_hcd:
>  dealloc_usb2_hcd:
>  	usb_remove_hcd(hcd);
>  
> +disable_clk:
> +	if (!IS_ERR(clk))
> +		clk_disable_unprepare(clk);
> +
>  unmap_registers:
>  	iounmap(hcd->regs);
>  
> @@ -189,11 +208,14 @@ static int xhci_plat_remove(struct platform_device *dev)
>  {
>  	struct usb_hcd	*hcd = platform_get_drvdata(dev);
>  	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
> +	struct clk *clk = xhci->clk;
>  
>  	usb_remove_hcd(xhci->shared_hcd);
>  	usb_put_hcd(xhci->shared_hcd);
>  
>  	usb_remove_hcd(hcd);
> +	if (!IS_ERR(clk))
> +		clk_disable_unprepare(clk);
>  	iounmap(hcd->regs);
>  	release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
>  	usb_put_hcd(hcd);
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index d280e92..003dc09 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -1478,6 +1478,8 @@ struct xhci_hcd {
>  	/* msi-x vectors */
>  	int		msix_count;
>  	struct msix_entry	*msix_entries;
> +	/* optional clock */
> +	struct clk		*clk;
>  	/* data structures */
>  	struct xhci_device_context_array *dcbaa;
>  	struct xhci_ring	*cmd_ring;
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
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: [PATCHv5 07/20] usb: host: xhci-plat: add clock support

[Mathias Nyman]

On 05/12/2014 05:46 PM, Gregory CLEMENT wrote:
> Hi Mathias, Felipe,
>
> On 11/05/2014 20:17, Thomas Petazzoni wrote:
>> From: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>
>> Some platforms (such as the Armada 38x ones) can gate the clock of
>> their USB controller. This patch adds the support for one clock in
>> xhci-plat, by enabling it during probe and disabling it on remove.
>>
>> To achieve this, it adds a 'struct clk *' member in xhci_hcd. While
>> only used for now in xhci-plat, it might be used by other drivers in
>> the future. Moreover, the xhci_hcd structure already holds other
>> members such as msix_count and msix_entries, which are MSI-X specific,
>> and therefore only used by xhci-pci.
>
> Felpie,
>
> would you agree to add your acked-by on this patch. You gave it on
> the 3rd version however as pointed by Thomas there was some issues,
> what do you think of this version?
>
>
> Mathias,
>
> as the xHCI maintainer what is your opinion on this patch. Do you plan
> to take it? do you expect a pull request?
>
> We submitted the first version of this series one month ago and we didn't
> receive any feedback from you. Maybe it was just because you were happy
> with the patch sent, but I would like to be sure that you noticed this
> series.
>
> I have the same question for the patch before: "[PATCHv5 06/20] usb: host:
> xhci-plat: sort the headers in alphabetic order" and the three following:
> "[PATCHv5 07/20] usb: host: xhci-plat: add clock support", "[PATCHv5 08/20]
>   usb: host: xhci-plat: add support for the Armada 375/38x XHCI controllers"
> and "[PATCHv5 09/20] Documentation: dt-bindings: update xhci-platform DT
> binding".
>

Hi

Sorry about the delay, Only thing I found was a sparse warning, otherwise looks ok.

-Mathias

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

Re: [PATCHv5 07/20] usb: host: xhci-plat: add clock support

[Felipe Balbi]

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

On Sun, May 11, 2014 at 08:17:55PM +0200, Thomas Petazzoni wrote:
> From: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> 
> Some platforms (such as the Armada 38x ones) can gate the clock of
> their USB controller. This patch adds the support for one clock in
> xhci-plat, by enabling it during probe and disabling it on remove.
> 
> To achieve this, it adds a 'struct clk *' member in xhci_hcd. While
> only used for now in xhci-plat, it might be used by other drivers in
> the future. Moreover, the xhci_hcd structure already holds other
> members such as msix_count and msix_entries, which are MSI-X specific,
> and therefore only used by xhci-pci.
> 
> Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

FWIW

Acked-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>

> ---
>  drivers/usb/host/xhci-plat.c | 24 +++++++++++++++++++++++-
>  drivers/usb/host/xhci.h      |  2 ++
>  2 files changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index f5351af..8108e58 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -11,6 +11,7 @@
>   * version 2 as published by the Free Software Foundation.
>   */
>  
> +#include <linux/clk.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> @@ -91,6 +92,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
>  	struct xhci_hcd		*xhci;
>  	struct resource         *res;
>  	struct usb_hcd		*hcd;
> +	struct clk              *clk;
>  	int			ret;
>  	int			irq;
>  
> @@ -137,14 +139,27 @@ static int xhci_plat_probe(struct platform_device *pdev)
>  		goto release_mem_region;
>  	}
>  
> +	/*
> +	 * Not all platforms have a clk so it is not an error if the
> +	 * clock does not exists.
> +	 */
> +	clk = devm_clk_get(&pdev->dev, NULL);
> +	if (!IS_ERR(clk)) {
> +		ret = clk_prepare_enable(clk);
> +		if (ret)
> +			goto unmap_registers;
> +	}
> +
>  	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
>  	if (ret)
> -		goto unmap_registers;
> +		goto disable_clk;
> +
>  	device_wakeup_enable(hcd->self.controller);
>  
>  	/* USB 2.0 roothub is stored in the platform_device now. */
>  	hcd = platform_get_drvdata(pdev);
>  	xhci = hcd_to_xhci(hcd);
> +	xhci->clk = clk;
>  	xhci->shared_hcd = usb_create_shared_hcd(driver, &pdev->dev,
>  			dev_name(&pdev->dev), hcd);
>  	if (!xhci->shared_hcd) {
> @@ -173,6 +188,10 @@ put_usb3_hcd:
>  dealloc_usb2_hcd:
>  	usb_remove_hcd(hcd);
>  
> +disable_clk:
> +	if (!IS_ERR(clk))
> +		clk_disable_unprepare(clk);
> +
>  unmap_registers:
>  	iounmap(hcd->regs);
>  
> @@ -189,11 +208,14 @@ static int xhci_plat_remove(struct platform_device *dev)
>  {
>  	struct usb_hcd	*hcd = platform_get_drvdata(dev);
>  	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
> +	struct clk *clk = xhci->clk;
>  
>  	usb_remove_hcd(xhci->shared_hcd);
>  	usb_put_hcd(xhci->shared_hcd);
>  
>  	usb_remove_hcd(hcd);
> +	if (!IS_ERR(clk))
> +		clk_disable_unprepare(clk);
>  	iounmap(hcd->regs);
>  	release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
>  	usb_put_hcd(hcd);
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index d280e92..003dc09 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -1478,6 +1478,8 @@ struct xhci_hcd {
>  	/* msi-x vectors */
>  	int		msix_count;
>  	struct msix_entry	*msix_entries;
> +	/* optional clock */
> +	struct clk		*clk;
>  	/* data structures */
>  	struct xhci_device_context_array *dcbaa;
>  	struct xhci_ring	*cmd_ring;
> -- 
> 1.9.2
> 

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCHv5 08/20] usb: host: xhci-plat: add support for the Armada 375/38x XHCI controllers

[Thomas Petazzoni]

From: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

The Armada 375 and 38x SoCs come with an XHCI controller that requires
some specific initialization related to the MBus windows
configuration. This patch adds the support for this special
configuration as an XHCI quirk executed during probe.

Two new compatible strings are added to identify the Armada 375 and
Armada 38x XHCI controllers, and therefore enable the relevant quirk.

Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 drivers/usb/host/Kconfig      |  8 +++++
 drivers/usb/host/Makefile     |  3 ++
 drivers/usb/host/xhci-mvebu.c | 70 +++++++++++++++++++++++++++++++++++++++++++
 drivers/usb/host/xhci-mvebu.h | 21 +++++++++++++
 drivers/usb/host/xhci-plat.c  | 12 ++++++++
 5 files changed, 114 insertions(+)
 create mode 100644 drivers/usb/host/xhci-mvebu.c
 create mode 100644 drivers/usb/host/xhci-mvebu.h

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 3d9e540..9247ad2 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -29,6 +29,14 @@ if USB_XHCI_HCD
 config USB_XHCI_PLATFORM
 	tristate
 
+config USB_XHCI_MVEBU
+	tristate "xHCI support for Marvell Armada 375/38x"
+	select USB_XHCI_PLATFORM
+	depends on ARCH_MVEBU || COMPILE_TEST
+	---help---
+	  Say 'Y' to enable the support for the xHCI host controller
+	  found in Marvell Armada 375/38x ARM SOCs.
+
 endif # USB_XHCI_HCD
 
 config USB_EHCI_HCD
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index 7530468..7c0886a 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -19,6 +19,9 @@ xhci-hcd-$(CONFIG_PCI)	+= xhci-pci.o
 
 ifneq ($(CONFIG_USB_XHCI_PLATFORM), )
 	xhci-hcd-y		+= xhci-plat.o
+ifneq ($(CONFIG_USB_XHCI_MVEBU), )
+	xhci-hcd-y		+= xhci-mvebu.o
+endif
 endif
 
 obj-$(CONFIG_USB_WHCI_HCD)	+= whci/
diff --git a/drivers/usb/host/xhci-mvebu.c b/drivers/usb/host/xhci-mvebu.c
new file mode 100644
index 0000000..fab9d6f
--- /dev/null
+++ b/drivers/usb/host/xhci-mvebu.c
@@ -0,0 +1,70 @@
+/*
+ * Copyright (C) 2014 Marvell
+ * Author: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@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.
+ */
+
+#include <linux/io.h>
+#include <linux/mbus.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+#define USB3_MAX_WINDOWS	4
+#define USB3_WIN_CTRL(w)	(0x0 + ((w) * 8))
+#define USB3_WIN_BASE(w)	(0x4 + ((w) * 8))
+
+static void xhci_mvebu_mbus_config(void __iomem *base,
+			const struct mbus_dram_target_info *dram)
+{
+	int win;
+
+	/* Clear all existing windows */
+	for (win = 0; win < USB3_MAX_WINDOWS; win++) {
+		writel(0, base + USB3_WIN_CTRL(win));
+		writel(0, base + USB3_WIN_BASE(win));
+	}
+
+	/* Program each DRAM CS in a seperate window */
+	for (win = 0; win < dram->num_cs; win++) {
+		const struct mbus_dram_window *cs = dram->cs + win;
+
+		writel(((cs->size - 1) & 0xffff0000) | (cs->mbus_attr << 8) |
+		       (dram->mbus_dram_target_id << 4) | 1,
+		       base + USB3_WIN_CTRL(win));
+
+		writel((cs->base & 0xffff0000), base + USB3_WIN_BASE(win));
+	}
+}
+
+int xhci_mvebu_mbus_init_quirk(struct platform_device *pdev)
+{
+	struct resource	*res;
+	void __iomem *base;
+	const struct mbus_dram_target_info *dram;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (!res)
+		return -ENODEV;
+
+	/*
+	 * We don't use devm_ioremap() because this mapping should
+	 * only exists for the duration of this probe function.
+	 */
+	base = ioremap(res->start, resource_size(res));
+	if (!base)
+		return -ENODEV;
+
+	dram = mv_mbus_dram_info();
+	xhci_mvebu_mbus_config(base, dram);
+
+	/*
+	 * This memory area was only needed to configure the MBus
+	 * windows, and is therefore no longer useful.
+	 */
+	iounmap(base);
+
+	return 0;
+}
diff --git a/drivers/usb/host/xhci-mvebu.h b/drivers/usb/host/xhci-mvebu.h
new file mode 100644
index 0000000..7ede92a
--- /dev/null
+++ b/drivers/usb/host/xhci-mvebu.h
@@ -0,0 +1,21 @@
+/*
+ * Copyright (C) 2014 Marvell
+ *
+ * Gregory Clement <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#ifndef __LINUX_XHCI_MVEBU_H
+#define __LINUX_XHCI_MVEBU_H
+#if IS_ENABLED(CONFIG_USB_XHCI_MVEBU)
+int xhci_mvebu_mbus_init_quirk(struct platform_device *pdev);
+#else
+static inline int xhci_mvebu_mbus_init_quirk(struct platform_device *pdev)
+{
+	return 0;
+}
+#endif
+#endif /* __LINUX_XHCI_MVEBU_H */
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 8108e58..0f5f4c8 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -19,6 +19,7 @@
 #include <linux/slab.h>
 
 #include "xhci.h"
+#include "xhci-mvebu.h"
 
 static void xhci_plat_quirks(struct device *dev, struct xhci_hcd *xhci)
 {
@@ -109,6 +110,15 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	if (!res)
 		return -ENODEV;
 
+	if (of_device_is_compatible(pdev->dev.of_node,
+				    "marvell,armada-375-xhci") ||
+	    of_device_is_compatible(pdev->dev.of_node,
+				    "marvell,armada-380-xhci")) {
+		ret = xhci_mvebu_mbus_init_quirk(pdev);
+		if (ret)
+			return ret;
+	}
+
 	/* Initialize dma_mask and coherent_dma_mask to 32-bits */
 	ret = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
 	if (ret)
@@ -253,6 +263,8 @@ static const struct dev_pm_ops xhci_plat_pm_ops = {
 static const struct of_device_id usb_xhci_of_match[] = {
 	{ .compatible = "generic-xhci" },
 	{ .compatible = "xhci-platform" },
+	{ .compatible = "marvell,armada-375-xhci"},
+	{ .compatible = "marvell,armada-380-xhci"},
 	{ },
 };
 MODULE_DEVICE_TABLE(of, usb_xhci_of_match);
-- 
1.9.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: [PATCHv5 08/20] usb: host: xhci-plat: add support for the Armada 375/38x XHCI controllers

[Mathias Nyman]

On 05/11/2014 09:17 PM, Thomas Petazzoni wrote:
> From: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>
> The Armada 375 and 38x SoCs come with an XHCI controller that requires
> some specific initialization related to the MBus windows
> configuration. This patch adds the support for this special
> configuration as an XHCI quirk executed during probe.
>
> Two new compatible strings are added to identify the Armada 375 and
> Armada 38x XHCI controllers, and therefore enable the relevant quirk.
>
> Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
>   drivers/usb/host/Kconfig      |  8 +++++
>   drivers/usb/host/Makefile     |  3 ++
>   drivers/usb/host/xhci-mvebu.c | 70 +++++++++++++++++++++++++++++++++++++++++++
>   drivers/usb/host/xhci-mvebu.h | 21 +++++++++++++
>   drivers/usb/host/xhci-plat.c  | 12 ++++++++
>   5 files changed, 114 insertions(+)
>   create mode 100644 drivers/usb/host/xhci-mvebu.c
>   create mode 100644 drivers/usb/host/xhci-mvebu.h
>
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index 3d9e540..9247ad2 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -29,6 +29,14 @@ if USB_XHCI_HCD
>   config USB_XHCI_PLATFORM
>   	tristate
>
> +config USB_XHCI_MVEBU
> +	tristate "xHCI support for Marvell Armada 375/38x"
> +	select USB_XHCI_PLATFORM
> +	depends on ARCH_MVEBU || COMPILE_TEST
> +	---help---
> +	  Say 'Y' to enable the support for the xHCI host controller
> +	  found in Marvell Armada 375/38x ARM SOCs.
> +
>   endif # USB_XHCI_HCD
>
>   config USB_EHCI_HCD
> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
> index 7530468..7c0886a 100644
> --- a/drivers/usb/host/Makefile
> +++ b/drivers/usb/host/Makefile
> @@ -19,6 +19,9 @@ xhci-hcd-$(CONFIG_PCI)	+= xhci-pci.o
>
>   ifneq ($(CONFIG_USB_XHCI_PLATFORM), )
>   	xhci-hcd-y		+= xhci-plat.o
> +ifneq ($(CONFIG_USB_XHCI_MVEBU), )
> +	xhci-hcd-y		+= xhci-mvebu.o
> +endif
>   endif
>
>   obj-$(CONFIG_USB_WHCI_HCD)	+= whci/
> diff --git a/drivers/usb/host/xhci-mvebu.c b/drivers/usb/host/xhci-mvebu.c
> new file mode 100644
> index 0000000..fab9d6f
> --- /dev/null
> +++ b/drivers/usb/host/xhci-mvebu.c
> @@ -0,0 +1,70 @@
> +/*
> + * Copyright (C) 2014 Marvell
> + * Author: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@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.
> + */
> +
> +#include <linux/io.h>
> +#include <linux/mbus.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +
> +#define USB3_MAX_WINDOWS	4
> +#define USB3_WIN_CTRL(w)	(0x0 + ((w) * 8))
> +#define USB3_WIN_BASE(w)	(0x4 + ((w) * 8))
> +
> +static void xhci_mvebu_mbus_config(void __iomem *base,
> +			const struct mbus_dram_target_info *dram)
> +{
> +	int win;
> +
> +	/* Clear all existing windows */
> +	for (win = 0; win < USB3_MAX_WINDOWS; win++) {
> +		writel(0, base + USB3_WIN_CTRL(win));
> +		writel(0, base + USB3_WIN_BASE(win));
> +	}
> +
> +	/* Program each DRAM CS in a seperate window */
> +	for (win = 0; win < dram->num_cs; win++) {
> +		const struct mbus_dram_window *cs = dram->cs + win;
> +
> +		writel(((cs->size - 1) & 0xffff0000) | (cs->mbus_attr << 8) |
> +		       (dram->mbus_dram_target_id << 4) | 1,
> +		       base + USB3_WIN_CTRL(win));
> +
> +		writel((cs->base & 0xffff0000), base + USB3_WIN_BASE(win));
> +	}
> +}
> +
> +int xhci_mvebu_mbus_init_quirk(struct platform_device *pdev)
> +{
> +	struct resource	*res;
> +	void __iomem *base;
> +	const struct mbus_dram_target_info *dram;

Hi

Sparse warns about this:

drivers/usb/host/xhci-mvebu.c:42:5: warning: symbol 'xhci_mvebu_mbus_init_quirk' 
was not declared. Should it be static?

Otherwise I think it looks good

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

Re: [PATCHv5 08/20] usb: host: xhci-plat: add support for the Armada 375/38x XHCI controllers

[Thomas Petazzoni]

Dear Mathias Nyman,

On Mon, 12 May 2014 20:24:45 +0300, Mathias Nyman wrote:

> > +int xhci_mvebu_mbus_init_quirk(struct platform_device *pdev)
> > +{
> > +	struct resource	*res;
> > +	void __iomem *base;
> > +	const struct mbus_dram_target_info *dram;
> 
> Hi
> 
> Sparse warns about this:
> 
> drivers/usb/host/xhci-mvebu.c:42:5: warning: symbol 'xhci_mvebu_mbus_init_quirk' 
> was not declared. Should it be static?

Not, it should not: it gets called from xhci-plat.c. However,
xhci-mvebu.c should include its header xhci-mvebu.h so that sparse
realize that the function is voluntarily exported, and that therefore
not having the static qualifier is expected.

Should we resend a v6 with just this change?

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
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: [PATCHv5 08/20] usb: host: xhci-plat: add support for the Armada 375/38x XHCI controllers

[Mathias Nyman]

On 05/12/2014 08:36 PM, Thomas Petazzoni wrote:
> Dear Mathias Nyman,
>
> On Mon, 12 May 2014 20:24:45 +0300, Mathias Nyman wrote:
>
>>> +int xhci_mvebu_mbus_init_quirk(struct platform_device *pdev)
>>> +{
>>> +	struct resource	*res;
>>> +	void __iomem *base;
>>> +	const struct mbus_dram_target_info *dram;
>>
>> Hi
>>
>> Sparse warns about this:
>>
>> drivers/usb/host/xhci-mvebu.c:42:5: warning: symbol 'xhci_mvebu_mbus_init_quirk'
>> was not declared. Should it be static?
>
> Not, it should not: it gets called from xhci-plat.c. However,
> xhci-mvebu.c should include its header xhci-mvebu.h so that sparse
> realize that the function is voluntarily exported, and that therefore
> not having the static qualifier is expected.
>
> Should we resend a v6 with just this change?
>

Not if this is the only thing that needs fixing.
But if you anyway need to make a new version (Kishon had some remarks) then please 
include the header to xhci-mvebu at the same time.

-Mathias

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

[PATCHv5 09/20] Documentation: dt-bindings: update xhci-platform DT binding

[Thomas Petazzoni]

From: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

This commit extends the compatible string list of the xhci-platform
binding with the new "armada-375-xhci" and "armada-380-xhci"
compatible strings. It is used to describe the XHCI controller which
is available in the Armada 375 and 38x SoCs.

It also indicates that an optional 'clocks' property is now supported.

Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 Documentation/devicetree/bindings/usb/usb-xhci.txt | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt
index 90f8f60..999be5c 100644
--- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
+++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
@@ -1,11 +1,16 @@
 USB xHCI controllers
 
 Required properties:
-  - compatible: should be "generic-xhci" (deprecated: "xhci-platform").
+  - compatible: should be one of "generic-xhci",
+    "marvell,armada-375-xhci", "marvell,armada-380-xhci" (deprecated:
+    "xhci-platform").
   - reg: should contain address and length of the standard XHCI
     register set for the device.
   - interrupts: one XHCI interrupt should be described here.
 
+Optional property:
+  - clocks: reference to a clock
+
 Example:
 	usb@f0931000 {
 		compatible = "generic-xhci";
-- 
1.9.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: [PATCHv5 09/20] Documentation: dt-bindings: update xhci-platform DT binding

[Mathias Nyman]

On 05/11/2014 09:17 PM, Thomas Petazzoni wrote:
> From: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>
> This commit extends the compatible string list of the xhci-platform
> binding with the new "armada-375-xhci" and "armada-380-xhci"
> compatible strings. It is used to describe the XHCI controller which
> is available in the Armada 375 and 38x SoCs.
>
> It also indicates that an optional 'clocks' property is now supported.
>
> Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
>   Documentation/devicetree/bindings/usb/usb-xhci.txt | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
>

Hi

Except for the sparse warning in PATCH [8/20], and if DT people are ok with this I 
think everything is in order.

otherwise, for patches 6-9:
Acked-by: Mathias Nyman <mathias.nyman-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

Greg, should this entire series go directly to you, or should we pick patches
to our own trees first (patches 6-9 for me)?

-Mathias

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

[PATCHv5 10/20] phy: add support for USB cluster on the Armada 375 SoC

[Thomas Petazzoni]

From: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

The Armada 375 SoC comes with an USB2 host and device controller and
an USB3 controller. The USB cluster control register allows to manage
common features of both USB controllers.

This commit adds a driver integrated in the generic PHY framework to
control this USB cluster feature.

Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 drivers/phy/Kconfig              |   6 ++
 drivers/phy/Makefile             |   1 +
 drivers/phy/phy-armada375-usb2.c | 157 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 164 insertions(+)
 create mode 100644 drivers/phy/phy-armada375-usb2.c

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 3bb05f1..e63cf9d 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -15,6 +15,12 @@ config GENERIC_PHY
 	  phy users can obtain reference to the PHY. All the users of this
 	  framework should select this config.
 
+config ARMADA375_USBCLUSTER_PHY
+	def_bool y
+	depends on MACH_ARMADA_375 || COMPILE_TEST
+	depends on OF
+	select GENERIC_PHY
+
 config PHY_EXYNOS_MIPI_VIDEO
 	tristate "S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver"
 	depends on HAS_IOMEM
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 2faf78e..47d5a86 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -3,6 +3,7 @@
 #
 
 obj-$(CONFIG_GENERIC_PHY)		+= phy-core.o
+obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY)	+= phy-armada375-usb2.o
 obj-$(CONFIG_BCM_KONA_USB2_PHY)		+= phy-bcm-kona-usb2.o
 obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)	+= phy-exynos-dp-video.o
 obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
diff --git a/drivers/phy/phy-armada375-usb2.c b/drivers/phy/phy-armada375-usb2.c
new file mode 100644
index 0000000..a6f746d
--- /dev/null
+++ b/drivers/phy/phy-armada375-usb2.c
@@ -0,0 +1,157 @@
+/*
+ * USB cluster support for Armada 375 platform.
+ *
+ * Copyright (C) 2014 Marvell
+ *
+ * Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2 or later. This program is licensed "as is"
+ * without any warranty of any kind, whether express or implied.
+ *
+ * Armada 375 comes with an USB2 host and device controller and an
+ * USB3 controller. The USB cluster control register allows to manage
+ * common features of both USB controllers.
+ */
+
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#define USB2_PHY_CONFIG_DISABLE BIT(0)
+
+/* The USB cluster allows to choose between two PHYs */
+#define NB_PHY 2
+
+enum {
+	PHY_USB2 = 0,
+	PHY_USB3 = 1,
+};
+
+struct armada375_cluster_phy {
+	struct phy *phy;
+	void __iomem *reg;
+	bool enable;
+	bool use_usb3;
+};
+
+struct armada375_cluster_phy usb_cluster_phy[NB_PHY];
+
+static int armada375_usb_phy_init(struct phy *phy)
+{
+	struct armada375_cluster_phy *cluster_phy = phy_get_drvdata(phy);
+	u32 reg;
+
+	if (!cluster_phy->enable)
+		return -ENODEV;
+
+	reg = readl(cluster_phy->reg);
+	if (cluster_phy->use_usb3)
+		reg |= USB2_PHY_CONFIG_DISABLE;
+	else
+		reg &= ~USB2_PHY_CONFIG_DISABLE;
+	writel(reg, cluster_phy->reg);
+
+	return 0;
+}
+
+static struct phy_ops armada375_usb_phy_ops = {
+	.init = armada375_usb_phy_init,
+	.owner		= THIS_MODULE,
+};
+
+static struct phy *armada375_usb_phy_xlate(struct device *dev,
+					struct of_phandle_args *args)
+{
+	if (WARN_ON(args->args[0] >= NB_PHY))
+		return ERR_PTR(-ENODEV);
+
+	return usb_cluster_phy[args->args[0]].phy;
+}
+
+static int armada375_usb_phy_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct phy *phy;
+	struct phy_provider *phy_provider;
+	void __iomem *usb_cluster_base;
+	struct device_node *xhci_node;
+	struct resource *res;
+	int i;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	usb_cluster_base = devm_ioremap_resource(&pdev->dev, res);
+	if (!usb_cluster_base)
+		return -ENOMEM;
+
+	for (i = 0; i < NB_PHY; i++) {
+		phy = devm_phy_create(dev, &armada375_usb_phy_ops, NULL);
+		if (IS_ERR(phy)) {
+			dev_err(dev, "failed to create PHY n%d\n", i);
+			return PTR_ERR(phy);
+		}
+
+		usb_cluster_phy[i].phy = phy;
+		usb_cluster_phy[i].reg = usb_cluster_base;
+		usb_cluster_phy[i].enable = false;
+		phy_set_drvdata(phy, &usb_cluster_phy[i]);
+	}
+
+	usb_cluster_phy[PHY_USB2].use_usb3 = false;
+	usb_cluster_phy[PHY_USB3].use_usb3 = true;
+
+	/*
+	 * We can't use the first usb2 unit and usb3 at the same time
+	 * to manage a USB2 device, so let's disable usb2 if usb3 is
+	 * selected. In this case the USB2 device will be managed by
+	 * the xhci controller.
+	 */
+
+	xhci_node = of_find_compatible_node(NULL, NULL,
+					"marvell,armada-375-xhci");
+
+	if (xhci_node && of_device_is_available(xhci_node)) {
+		usb_cluster_phy[PHY_USB3].enable = true;
+	} else {
+		struct device_node *ehci_node;
+		ehci_node = of_find_compatible_node(NULL, NULL,
+					"marvell,orion-ehci");
+		if (ehci_node && of_device_is_available(ehci_node))
+			usb_cluster_phy[PHY_USB2].enable = true;
+		of_node_put(ehci_node);
+	}
+
+	of_node_put(xhci_node);
+
+	phy_provider = devm_of_phy_provider_register(&pdev->dev,
+						     armada375_usb_phy_xlate);
+	if (IS_ERR(phy_provider))
+		return PTR_ERR(phy_provider);
+
+	return 0;
+}
+
+static const struct of_device_id of_usb_cluster_table[] = {
+	{ .compatible = "marvell,armada-375-usb-cluster", },
+	{ /* end of list */ },
+};
+MODULE_DEVICE_TABLE(of, of_usb_cluster_table);
+
+static struct platform_driver armada375_usb_phy_driver = {
+	.probe	= armada375_usb_phy_probe,
+	.driver = {
+		.of_match_table	= of_usb_cluster_table,
+		.name  = "armada-375-usb-cluster",
+		.owner = THIS_MODULE,
+	}
+};
+module_platform_driver(armada375_usb_phy_driver);
+
+MODULE_DESCRIPTION("Armada 375 USB cluster driver");
+MODULE_AUTHOR("Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>");
+MODULE_LICENSE("GPL");
-- 
1.9.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: [PATCHv5 10/20] phy: add support for USB cluster on the Armada 375 SoC

[Gregory CLEMENT]

Hi Felipe, Kishon,

On 11/05/2014 20:17, Thomas Petazzoni wrote:
> From: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> 
> The Armada 375 SoC comes with an USB2 host and device controller and
> an USB3 controller. The USB cluster control register allows to manage
> common features of both USB controllers.
> 
> This commit adds a driver integrated in the generic PHY framework to
> control this USB cluster feature.

Kishon,

it seems that we forgot to send you this email as the PHY framework
maintainer. Sorry for this. However did you have the opportunity to
have a look on it and do you think this driver would be OK to be
merged?

Felipe,

you are the USB PHY layer maintainer, so your opinion on this
driver would be valuable.

Thanks,

Gregory


> 
> Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
>  drivers/phy/Kconfig              |   6 ++
>  drivers/phy/Makefile             |   1 +
>  drivers/phy/phy-armada375-usb2.c | 157 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 164 insertions(+)
>  create mode 100644 drivers/phy/phy-armada375-usb2.c
> 
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 3bb05f1..e63cf9d 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -15,6 +15,12 @@ config GENERIC_PHY
>  	  phy users can obtain reference to the PHY. All the users of this
>  	  framework should select this config.
>  
> +config ARMADA375_USBCLUSTER_PHY
> +	def_bool y
> +	depends on MACH_ARMADA_375 || COMPILE_TEST
> +	depends on OF
> +	select GENERIC_PHY
> +
>  config PHY_EXYNOS_MIPI_VIDEO
>  	tristate "S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver"
>  	depends on HAS_IOMEM
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 2faf78e..47d5a86 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -3,6 +3,7 @@
>  #
>  
>  obj-$(CONFIG_GENERIC_PHY)		+= phy-core.o
> +obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY)	+= phy-armada375-usb2.o
>  obj-$(CONFIG_BCM_KONA_USB2_PHY)		+= phy-bcm-kona-usb2.o
>  obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)	+= phy-exynos-dp-video.o
>  obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
> diff --git a/drivers/phy/phy-armada375-usb2.c b/drivers/phy/phy-armada375-usb2.c
> new file mode 100644
> index 0000000..a6f746d
> --- /dev/null
> +++ b/drivers/phy/phy-armada375-usb2.c
> @@ -0,0 +1,157 @@
> +/*
> + * USB cluster support for Armada 375 platform.
> + *
> + * Copyright (C) 2014 Marvell
> + *
> + * Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2 or later. This program is licensed "as is"
> + * without any warranty of any kind, whether express or implied.
> + *
> + * Armada 375 comes with an USB2 host and device controller and an
> + * USB3 controller. The USB cluster control register allows to manage
> + * common features of both USB controllers.
> + */
> +
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#define USB2_PHY_CONFIG_DISABLE BIT(0)
> +
> +/* The USB cluster allows to choose between two PHYs */
> +#define NB_PHY 2
> +
> +enum {
> +	PHY_USB2 = 0,
> +	PHY_USB3 = 1,
> +};
> +
> +struct armada375_cluster_phy {
> +	struct phy *phy;
> +	void __iomem *reg;
> +	bool enable;
> +	bool use_usb3;
> +};
> +
> +struct armada375_cluster_phy usb_cluster_phy[NB_PHY];
> +
> +static int armada375_usb_phy_init(struct phy *phy)
> +{
> +	struct armada375_cluster_phy *cluster_phy = phy_get_drvdata(phy);
> +	u32 reg;
> +
> +	if (!cluster_phy->enable)
> +		return -ENODEV;
> +
> +	reg = readl(cluster_phy->reg);
> +	if (cluster_phy->use_usb3)
> +		reg |= USB2_PHY_CONFIG_DISABLE;
> +	else
> +		reg &= ~USB2_PHY_CONFIG_DISABLE;
> +	writel(reg, cluster_phy->reg);
> +
> +	return 0;
> +}
> +
> +static struct phy_ops armada375_usb_phy_ops = {
> +	.init = armada375_usb_phy_init,
> +	.owner		= THIS_MODULE,
> +};
> +
> +static struct phy *armada375_usb_phy_xlate(struct device *dev,
> +					struct of_phandle_args *args)
> +{
> +	if (WARN_ON(args->args[0] >= NB_PHY))
> +		return ERR_PTR(-ENODEV);
> +
> +	return usb_cluster_phy[args->args[0]].phy;
> +}
> +
> +static int armada375_usb_phy_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct phy *phy;
> +	struct phy_provider *phy_provider;
> +	void __iomem *usb_cluster_base;
> +	struct device_node *xhci_node;
> +	struct resource *res;
> +	int i;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	usb_cluster_base = devm_ioremap_resource(&pdev->dev, res);
> +	if (!usb_cluster_base)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < NB_PHY; i++) {
> +		phy = devm_phy_create(dev, &armada375_usb_phy_ops, NULL);
> +		if (IS_ERR(phy)) {
> +			dev_err(dev, "failed to create PHY n%d\n", i);
> +			return PTR_ERR(phy);
> +		}
> +
> +		usb_cluster_phy[i].phy = phy;
> +		usb_cluster_phy[i].reg = usb_cluster_base;
> +		usb_cluster_phy[i].enable = false;
> +		phy_set_drvdata(phy, &usb_cluster_phy[i]);
> +	}
> +
> +	usb_cluster_phy[PHY_USB2].use_usb3 = false;
> +	usb_cluster_phy[PHY_USB3].use_usb3 = true;
> +
> +	/*
> +	 * We can't use the first usb2 unit and usb3 at the same time
> +	 * to manage a USB2 device, so let's disable usb2 if usb3 is
> +	 * selected. In this case the USB2 device will be managed by
> +	 * the xhci controller.
> +	 */
> +
> +	xhci_node = of_find_compatible_node(NULL, NULL,
> +					"marvell,armada-375-xhci");
> +
> +	if (xhci_node && of_device_is_available(xhci_node)) {
> +		usb_cluster_phy[PHY_USB3].enable = true;
> +	} else {
> +		struct device_node *ehci_node;
> +		ehci_node = of_find_compatible_node(NULL, NULL,
> +					"marvell,orion-ehci");
> +		if (ehci_node && of_device_is_available(ehci_node))
> +			usb_cluster_phy[PHY_USB2].enable = true;
> +		of_node_put(ehci_node);
> +	}
> +
> +	of_node_put(xhci_node);
> +
> +	phy_provider = devm_of_phy_provider_register(&pdev->dev,
> +						     armada375_usb_phy_xlate);
> +	if (IS_ERR(phy_provider))
> +		return PTR_ERR(phy_provider);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id of_usb_cluster_table[] = {
> +	{ .compatible = "marvell,armada-375-usb-cluster", },
> +	{ /* end of list */ },
> +};
> +MODULE_DEVICE_TABLE(of, of_usb_cluster_table);
> +
> +static struct platform_driver armada375_usb_phy_driver = {
> +	.probe	= armada375_usb_phy_probe,
> +	.driver = {
> +		.of_match_table	= of_usb_cluster_table,
> +		.name  = "armada-375-usb-cluster",
> +		.owner = THIS_MODULE,
> +	}
> +};
> +module_platform_driver(armada375_usb_phy_driver);
> +
> +MODULE_DESCRIPTION("Armada 375 USB cluster driver");
> +MODULE_AUTHOR("Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>");
> +MODULE_LICENSE("GPL");
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
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: [PATCHv5 10/20] phy: add support for USB cluster on the Armada 375 SoC

[Kishon Vijay Abraham I]

Hi,

On Sunday 11 May 2014 11:47 PM, Thomas Petazzoni wrote:
> From: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> 
> The Armada 375 SoC comes with an USB2 host and device controller and
> an USB3 controller. The USB cluster control register allows to manage
> common features of both USB controllers.
> 
> This commit adds a driver integrated in the generic PHY framework to
> control this USB cluster feature.
> 
> Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
>  drivers/phy/Kconfig              |   6 ++
>  drivers/phy/Makefile             |   1 +
>  drivers/phy/phy-armada375-usb2.c | 157 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 164 insertions(+)
>  create mode 100644 drivers/phy/phy-armada375-usb2.c
> 
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 3bb05f1..e63cf9d 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -15,6 +15,12 @@ config GENERIC_PHY
>  	  phy users can obtain reference to the PHY. All the users of this
>  	  framework should select this config.
>  
> +config ARMADA375_USBCLUSTER_PHY
> +	def_bool y
> +	depends on MACH_ARMADA_375 || COMPILE_TEST
> +	depends on OF
> +	select GENERIC_PHY
> +
>  config PHY_EXYNOS_MIPI_VIDEO
>  	tristate "S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver"
>  	depends on HAS_IOMEM
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 2faf78e..47d5a86 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -3,6 +3,7 @@
>  #
>  
>  obj-$(CONFIG_GENERIC_PHY)		+= phy-core.o
> +obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY)	+= phy-armada375-usb2.o
>  obj-$(CONFIG_BCM_KONA_USB2_PHY)		+= phy-bcm-kona-usb2.o
>  obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)	+= phy-exynos-dp-video.o
>  obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
> diff --git a/drivers/phy/phy-armada375-usb2.c b/drivers/phy/phy-armada375-usb2.c
> new file mode 100644
> index 0000000..a6f746d
> --- /dev/null
> +++ b/drivers/phy/phy-armada375-usb2.c
> @@ -0,0 +1,157 @@
> +/*
> + * USB cluster support for Armada 375 platform.
> + *
> + * Copyright (C) 2014 Marvell
> + *
> + * Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2 or later. This program is licensed "as is"
> + * without any warranty of any kind, whether express or implied.
> + *
> + * Armada 375 comes with an USB2 host and device controller and an
> + * USB3 controller. The USB cluster control register allows to manage
> + * common features of both USB controllers.
> + */
> +
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#define USB2_PHY_CONFIG_DISABLE BIT(0)
> +
> +/* The USB cluster allows to choose between two PHYs */
> +#define NB_PHY 2
> +
> +enum {
> +	PHY_USB2 = 0,
> +	PHY_USB3 = 1,
> +};
> +
> +struct armada375_cluster_phy {
> +	struct phy *phy;
> +	void __iomem *reg;
> +	bool enable;
> +	bool use_usb3;
> +};
> +
> +struct armada375_cluster_phy usb_cluster_phy[NB_PHY];
> +
> +static int armada375_usb_phy_init(struct phy *phy)
> +{
> +	struct armada375_cluster_phy *cluster_phy = phy_get_drvdata(phy);
> +	u32 reg;

This function should be protected since both your PHYs use this ops.
> +
> +	if (!cluster_phy->enable)
> +		return -ENODEV;
> +
> +	reg = readl(cluster_phy->reg);
> +	if (cluster_phy->use_usb3)
> +		reg |= USB2_PHY_CONFIG_DISABLE;
> +	else
> +		reg &= ~USB2_PHY_CONFIG_DISABLE;
> +	writel(reg, cluster_phy->reg);

This is confusing since both your PHYs control the same bit?
> +
> +	return 0;
> +}
> +
> +static struct phy_ops armada375_usb_phy_ops = {
> +	.init = armada375_usb_phy_init,
> +	.owner		= THIS_MODULE,
> +};
> +
> +static struct phy *armada375_usb_phy_xlate(struct device *dev,
> +					struct of_phandle_args *args)
> +{
> +	if (WARN_ON(args->args[0] >= NB_PHY))
> +		return ERR_PTR(-ENODEV);
> +
> +	return usb_cluster_phy[args->args[0]].phy;
> +}
> +
> +static int armada375_usb_phy_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct phy *phy;
> +	struct phy_provider *phy_provider;
> +	void __iomem *usb_cluster_base;
> +	struct device_node *xhci_node;
> +	struct resource *res;
> +	int i;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	usb_cluster_base = devm_ioremap_resource(&pdev->dev, res);
> +	if (!usb_cluster_base)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < NB_PHY; i++) {

For devices which have multiple PHYs, each PHY should be modelled as the
sub-node of the *PHY provider* device node.
> +		phy = devm_phy_create(dev, &armada375_usb_phy_ops, NULL);
> +		if (IS_ERR(phy)) {
> +			dev_err(dev, "failed to create PHY n%d\n", i);
> +			return PTR_ERR(phy);
> +		}
> +
> +		usb_cluster_phy[i].phy = phy;
> +		usb_cluster_phy[i].reg = usb_cluster_base;
> +		usb_cluster_phy[i].enable = false;
> +		phy_set_drvdata(phy, &usb_cluster_phy[i]);
> +	}
> +
> +	usb_cluster_phy[PHY_USB2].use_usb3 = false;
> +	usb_cluster_phy[PHY_USB3].use_usb3 = true;
> +
> +	/*
> +	 * We can't use the first usb2 unit and usb3 at the same time
> +	 * to manage a USB2 device, so let's disable usb2 if usb3 is
> +	 * selected. In this case the USB2 device will be managed by
> +	 * the xhci controller.
> +	 */
> +
> +	xhci_node = of_find_compatible_node(NULL, NULL,
> +					"marvell,armada-375-xhci");

huh.. that's too much binding between the controller and the PHY.

> +
> +	if (xhci_node && of_device_is_available(xhci_node)) {
> +		usb_cluster_phy[PHY_USB3].enable = true;
> +	} else {
> +		struct device_node *ehci_node;
> +		ehci_node = of_find_compatible_node(NULL, NULL,
> +					"marvell,orion-ehci");
> +		if (ehci_node && of_device_is_available(ehci_node))
> +			usb_cluster_phy[PHY_USB2].enable = true;
> +		of_node_put(ehci_node);
> +	}
> +
> +	of_node_put(xhci_node);
> +
> +	phy_provider = devm_of_phy_provider_register(&pdev->dev,
> +						     armada375_usb_phy_xlate);
> +	if (IS_ERR(phy_provider))
> +		return PTR_ERR(phy_provider);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id of_usb_cluster_table[] = {
> +	{ .compatible = "marvell,armada-375-usb-cluster", },
> +	{ /* end of list */ },
> +};
> +MODULE_DEVICE_TABLE(of, of_usb_cluster_table);
> +
> +static struct platform_driver armada375_usb_phy_driver = {
> +	.probe	= armada375_usb_phy_probe,
> +	.driver = {
> +		.of_match_table	= of_usb_cluster_table,
> +		.name  = "armada-375-usb-cluster",
> +		.owner = THIS_MODULE,
> +	}
> +};
> +module_platform_driver(armada375_usb_phy_driver);
> +
> +MODULE_DESCRIPTION("Armada 375 USB cluster driver");
> +MODULE_AUTHOR("Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>");
> +MODULE_LICENSE("GPL");

GPL v2?

Thanks
Kishon
--
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: [PATCHv5 10/20] phy: add support for USB cluster on the Armada 375 SoC

[Gregory CLEMENT]

On 13/05/2014 07:53, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Sunday 11 May 2014 11:47 PM, Thomas Petazzoni wrote:
>> From: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>
>> The Armada 375 SoC comes with an USB2 host and device controller and
>> an USB3 controller. The USB cluster control register allows to manage
>> common features of both USB controllers.
>>
>> This commit adds a driver integrated in the generic PHY framework to
>> control this USB cluster feature.
>>
>> Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>> Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>> ---
>>  drivers/phy/Kconfig              |   6 ++
>>  drivers/phy/Makefile             |   1 +
>>  drivers/phy/phy-armada375-usb2.c | 157 +++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 164 insertions(+)
>>  create mode 100644 drivers/phy/phy-armada375-usb2.c
>>
>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>> index 3bb05f1..e63cf9d 100644
>> --- a/drivers/phy/Kconfig
>> +++ b/drivers/phy/Kconfig
>> @@ -15,6 +15,12 @@ config GENERIC_PHY
>>  	  phy users can obtain reference to the PHY. All the users of this
>>  	  framework should select this config.
>>  
>> +config ARMADA375_USBCLUSTER_PHY
>> +	def_bool y
>> +	depends on MACH_ARMADA_375 || COMPILE_TEST
>> +	depends on OF
>> +	select GENERIC_PHY
>> +
>>  config PHY_EXYNOS_MIPI_VIDEO
>>  	tristate "S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver"
>>  	depends on HAS_IOMEM
>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>> index 2faf78e..47d5a86 100644
>> --- a/drivers/phy/Makefile
>> +++ b/drivers/phy/Makefile
>> @@ -3,6 +3,7 @@
>>  #
>>  
>>  obj-$(CONFIG_GENERIC_PHY)		+= phy-core.o
>> +obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY)	+= phy-armada375-usb2.o
>>  obj-$(CONFIG_BCM_KONA_USB2_PHY)		+= phy-bcm-kona-usb2.o
>>  obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)	+= phy-exynos-dp-video.o
>>  obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
>> diff --git a/drivers/phy/phy-armada375-usb2.c b/drivers/phy/phy-armada375-usb2.c
>> new file mode 100644
>> index 0000000..a6f746d
>> --- /dev/null
>> +++ b/drivers/phy/phy-armada375-usb2.c
>> @@ -0,0 +1,157 @@
>> +/*
>> + * USB cluster support for Armada 375 platform.
>> + *
>> + * Copyright (C) 2014 Marvell
>> + *
>> + * Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>> + *
>> + * This file is licensed under the terms of the GNU General Public
>> + * License version 2 or later. This program is licensed "as is"
>> + * without any warranty of any kind, whether express or implied.
>> + *
>> + * Armada 375 comes with an USB2 host and device controller and an
>> + * USB3 controller. The USB cluster control register allows to manage
>> + * common features of both USB controllers.
>> + */
>> +
>> +#include <linux/init.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of_address.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/slab.h>
>> +
>> +#define USB2_PHY_CONFIG_DISABLE BIT(0)
>> +
>> +/* The USB cluster allows to choose between two PHYs */
>> +#define NB_PHY 2
>> +
>> +enum {
>> +	PHY_USB2 = 0,
>> +	PHY_USB3 = 1,
>> +};
>> +
>> +struct armada375_cluster_phy {
>> +	struct phy *phy;
>> +	void __iomem *reg;
>> +	bool enable;
>> +	bool use_usb3;
>> +};
>> +
>> +struct armada375_cluster_phy usb_cluster_phy[NB_PHY];
>> +
>> +static int armada375_usb_phy_init(struct phy *phy)
>> +{
>> +	struct armada375_cluster_phy *cluster_phy = phy_get_drvdata(phy);
>> +	u32 reg;
> 
> This function should be protected since both your PHYs use this ops.

Right

>> +
>> +	if (!cluster_phy->enable)
>> +		return -ENODEV;
>> +
>> +	reg = readl(cluster_phy->reg);
>> +	if (cluster_phy->use_usb3)
>> +		reg |= USB2_PHY_CONFIG_DISABLE;
>> +	else
>> +		reg &= ~USB2_PHY_CONFIG_DISABLE;
>> +	writel(reg, cluster_phy->reg);
> 
> This is confusing since both your PHYs control the same bit?
>> +
>> +	return 0;
>> +}
>> +
>> +static struct phy_ops armada375_usb_phy_ops = {
>> +	.init = armada375_usb_phy_init,
>> +	.owner		= THIS_MODULE,
>> +};
>> +
>> +static struct phy *armada375_usb_phy_xlate(struct device *dev,
>> +					struct of_phandle_args *args)
>> +{
>> +	if (WARN_ON(args->args[0] >= NB_PHY))
>> +		return ERR_PTR(-ENODEV);
>> +
>> +	return usb_cluster_phy[args->args[0]].phy;
>> +}
>> +
>> +static int armada375_usb_phy_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct phy *phy;
>> +	struct phy_provider *phy_provider;
>> +	void __iomem *usb_cluster_base;
>> +	struct device_node *xhci_node;
>> +	struct resource *res;
>> +	int i;
>> +
>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	usb_cluster_base = devm_ioremap_resource(&pdev->dev, res);
>> +	if (!usb_cluster_base)
>> +		return -ENOMEM;
>> +
>> +	for (i = 0; i < NB_PHY; i++) {
> 
> For devices which have multiple PHYs, each PHY should be modelled as the
> sub-node of the *PHY provider* device node.

Actually it is the opposite the same PHY is shared between the EHCI
and the xHCI controllers. It is more a PHY muxer than a PHY itself.

I had to create 2 logical PHYs because once the phy_init() is called
by a USB driver then the .init ops is not called anymore by the next
call to phy_init(). One of the goal of this is to disable a port for
the USB controller which can't use it due to the configuration of the
USB cluster.

But I can see how to make this two "pseudo" PHYs sub-node of the *PHY
provider* device node. It shouldn't change the internal logic of this
driver.


>> +		phy = devm_phy_create(dev, &armada375_usb_phy_ops, NULL);
>> +		if (IS_ERR(phy)) {
>> +			dev_err(dev, "failed to create PHY n%d\n", i);
>> +			return PTR_ERR(phy);
>> +		}
>> +
>> +		usb_cluster_phy[i].phy = phy;
>> +		usb_cluster_phy[i].reg = usb_cluster_base;
>> +		usb_cluster_phy[i].enable = false;
>> +		phy_set_drvdata(phy, &usb_cluster_phy[i]);
>> +	}
>> +
>> +	usb_cluster_phy[PHY_USB2].use_usb3 = false;
>> +	usb_cluster_phy[PHY_USB3].use_usb3 = true;
>> +
>> +	/*
>> +	 * We can't use the first usb2 unit and usb3 at the same time
>> +	 * to manage a USB2 device, so let's disable usb2 if usb3 is
>> +	 * selected. In this case the USB2 device will be managed by
>> +	 * the xhci controller.
>> +	 */
>> +
>> +	xhci_node = of_find_compatible_node(NULL, NULL,
>> +					"marvell,armada-375-xhci");
> 
> huh.. that's too much binding between the controller and the PHY.
> 

That's why initially it was not part of the PHY framework.  The USB
cluster is really a part managing common feature between the USB
controllers which are part of the Armada 375 SoC.

However the initial version was not really good, because this piece of
code was located in the mach- directory whereas we are trying to move
most of the files out of this directory now. The USB cluster is not a
real PHY as it is related to the PHY management this framework remains
the best place for it.

>> +
>> +	if (xhci_node && of_device_is_available(xhci_node)) {
>> +		usb_cluster_phy[PHY_USB3].enable = true;
>> +	} else {
>> +		struct device_node *ehci_node;
>> +		ehci_node = of_find_compatible_node(NULL, NULL,
>> +					"marvell,orion-ehci");
>> +		if (ehci_node && of_device_is_available(ehci_node))
>> +			usb_cluster_phy[PHY_USB2].enable = true;
>> +		of_node_put(ehci_node);
>> +	}
>> +
>> +	of_node_put(xhci_node);
>> +
>> +	phy_provider = devm_of_phy_provider_register(&pdev->dev,
>> +						     armada375_usb_phy_xlate);
>> +	if (IS_ERR(phy_provider))
>> +		return PTR_ERR(phy_provider);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct of_device_id of_usb_cluster_table[] = {
>> +	{ .compatible = "marvell,armada-375-usb-cluster", },
>> +	{ /* end of list */ },
>> +};
>> +MODULE_DEVICE_TABLE(of, of_usb_cluster_table);
>> +
>> +static struct platform_driver armada375_usb_phy_driver = {
>> +	.probe	= armada375_usb_phy_probe,
>> +	.driver = {
>> +		.of_match_table	= of_usb_cluster_table,
>> +		.name  = "armada-375-usb-cluster",
>> +		.owner = THIS_MODULE,
>> +	}
>> +};
>> +module_platform_driver(armada375_usb_phy_driver);
>> +
>> +MODULE_DESCRIPTION("Armada 375 USB cluster driver");
>> +MODULE_AUTHOR("Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>");
>> +MODULE_LICENSE("GPL");
> 
> GPL v2?

No it is really GPL v2 or latter as written at the top of this file


Thanks,

Gregory

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
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: [PATCHv5 10/20] phy: add support for USB cluster on the Armada 375 SoC

[Gregory CLEMENT]

On 13/05/2014 10:06, Gregory CLEMENT wrote:
> On 13/05/2014 07:53, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Sunday 11 May 2014 11:47 PM, Thomas Petazzoni wrote:
>>> From: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>
>>> The Armada 375 SoC comes with an USB2 host and device controller and
>>> an USB3 controller. The USB cluster control register allows to manage
>>> common features of both USB controllers.
>>>
>>> This commit adds a driver integrated in the generic PHY framework to
>>> control this USB cluster feature.
>>>
>>> Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>> Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>> ---
>>>  drivers/phy/Kconfig              |   6 ++
>>>  drivers/phy/Makefile             |   1 +
>>>  drivers/phy/phy-armada375-usb2.c | 157 +++++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 164 insertions(+)
>>>  create mode 100644 drivers/phy/phy-armada375-usb2.c
>>>
>>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>>> index 3bb05f1..e63cf9d 100644
>>> --- a/drivers/phy/Kconfig
>>> +++ b/drivers/phy/Kconfig
>>> @@ -15,6 +15,12 @@ config GENERIC_PHY
>>>  	  phy users can obtain reference to the PHY. All the users of this
>>>  	  framework should select this config.
>>>  
>>> +config ARMADA375_USBCLUSTER_PHY
>>> +	def_bool y
>>> +	depends on MACH_ARMADA_375 || COMPILE_TEST
>>> +	depends on OF
>>> +	select GENERIC_PHY
>>> +
>>>  config PHY_EXYNOS_MIPI_VIDEO
>>>  	tristate "S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver"
>>>  	depends on HAS_IOMEM
>>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>>> index 2faf78e..47d5a86 100644
>>> --- a/drivers/phy/Makefile
>>> +++ b/drivers/phy/Makefile
>>> @@ -3,6 +3,7 @@
>>>  #
>>>  
>>>  obj-$(CONFIG_GENERIC_PHY)		+= phy-core.o
>>> +obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY)	+= phy-armada375-usb2.o
>>>  obj-$(CONFIG_BCM_KONA_USB2_PHY)		+= phy-bcm-kona-usb2.o
>>>  obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)	+= phy-exynos-dp-video.o
>>>  obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
>>> diff --git a/drivers/phy/phy-armada375-usb2.c b/drivers/phy/phy-armada375-usb2.c
>>> new file mode 100644
>>> index 0000000..a6f746d
>>> --- /dev/null
>>> +++ b/drivers/phy/phy-armada375-usb2.c
>>> @@ -0,0 +1,157 @@
>>> +/*
>>> + * USB cluster support for Armada 375 platform.
>>> + *
>>> + * Copyright (C) 2014 Marvell
>>> + *
>>> + * Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>> + *
>>> + * This file is licensed under the terms of the GNU General Public
>>> + * License version 2 or later. This program is licensed "as is"
>>> + * without any warranty of any kind, whether express or implied.
>>> + *
>>> + * Armada 375 comes with an USB2 host and device controller and an
>>> + * USB3 controller. The USB cluster control register allows to manage
>>> + * common features of both USB controllers.
>>> + */
>>> +
>>> +#include <linux/init.h>
>>> +#include <linux/io.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/module.h>
>>> +#include <linux/of_address.h>
>>> +#include <linux/phy/phy.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/slab.h>
>>> +
>>> +#define USB2_PHY_CONFIG_DISABLE BIT(0)
>>> +
>>> +/* The USB cluster allows to choose between two PHYs */
>>> +#define NB_PHY 2
>>> +
>>> +enum {
>>> +	PHY_USB2 = 0,
>>> +	PHY_USB3 = 1,
>>> +};
>>> +
>>> +struct armada375_cluster_phy {
>>> +	struct phy *phy;
>>> +	void __iomem *reg;
>>> +	bool enable;
>>> +	bool use_usb3;
>>> +};
>>> +
>>> +struct armada375_cluster_phy usb_cluster_phy[NB_PHY];
>>> +
>>> +static int armada375_usb_phy_init(struct phy *phy)
>>> +{
>>> +	struct armada375_cluster_phy *cluster_phy = phy_get_drvdata(phy);
>>> +	u32 reg;
>>
>> This function should be protected since both your PHYs use this ops.
> 
> Right

Actually only one PHY can access this register. See the probe function,
cluster_phy->enable is only set to true for one PHY.

> 
>>> +
>>> +	if (!cluster_phy->enable)
>>> +		return -ENODEV;
>>> +
>>> +	reg = readl(cluster_phy->reg);
>>> +	if (cluster_phy->use_usb3)
>>> +		reg |= USB2_PHY_CONFIG_DISABLE;
>>> +	else
>>> +		reg &= ~USB2_PHY_CONFIG_DISABLE;
>>> +	writel(reg, cluster_phy->reg);
>>
>> This is confusing since both your PHYs control the same bit?

Same here at the end the bit is accessed by only one PHY.

>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static struct phy_ops armada375_usb_phy_ops = {
>>> +	.init = armada375_usb_phy_init,
>>> +	.owner		= THIS_MODULE,
>>> +};
>>> +
>>> +static struct phy *armada375_usb_phy_xlate(struct device *dev,
>>> +					struct of_phandle_args *args)
>>> +{
>>> +	if (WARN_ON(args->args[0] >= NB_PHY))
>>> +		return ERR_PTR(-ENODEV);
>>> +
>>> +	return usb_cluster_phy[args->args[0]].phy;
>>> +}
>>> +
>>> +static int armada375_usb_phy_probe(struct platform_device *pdev)
>>> +{
>>> +	struct device *dev = &pdev->dev;
>>> +	struct phy *phy;
>>> +	struct phy_provider *phy_provider;
>>> +	void __iomem *usb_cluster_base;
>>> +	struct device_node *xhci_node;
>>> +	struct resource *res;
>>> +	int i;
>>> +
>>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> +	usb_cluster_base = devm_ioremap_resource(&pdev->dev, res);
>>> +	if (!usb_cluster_base)
>>> +		return -ENOMEM;
>>> +
>>> +	for (i = 0; i < NB_PHY; i++) {
>>
>> For devices which have multiple PHYs, each PHY should be modelled as the
>> sub-node of the *PHY provider* device node.
> 
> Actually it is the opposite the same PHY is shared between the EHCI
> and the xHCI controllers. It is more a PHY muxer than a PHY itself.
> 
> I had to create 2 logical PHYs because once the phy_init() is called
> by a USB driver then the .init ops is not called anymore by the next
> call to phy_init(). One of the goal of this is to disable a port for
> the USB controller which can't use it due to the configuration of the
> USB cluster.
> 
> But I can see how to make this two "pseudo" PHYs sub-node of the *PHY
> provider* device node. It shouldn't change the internal logic of this
> driver.

I need to make a distinction when the PHY access by the xHCI or when
it was access by the EHCI. If I create two new sub-node then I will
also need to add a property to make this distinction. It seems a little
overkill for the need.

> 
> 
>>> +		phy = devm_phy_create(dev, &armada375_usb_phy_ops, NULL);
>>> +		if (IS_ERR(phy)) {
>>> +			dev_err(dev, "failed to create PHY n%d\n", i);
>>> +			return PTR_ERR(phy);
>>> +		}
>>> +
>>> +		usb_cluster_phy[i].phy = phy;
>>> +		usb_cluster_phy[i].reg = usb_cluster_base;
>>> +		usb_cluster_phy[i].enable = false;
>>> +		phy_set_drvdata(phy, &usb_cluster_phy[i]);
>>> +	}
>>> +
>>> +	usb_cluster_phy[PHY_USB2].use_usb3 = false;
>>> +	usb_cluster_phy[PHY_USB3].use_usb3 = true;
>>> +
>>> +	/*
>>> +	 * We can't use the first usb2 unit and usb3 at the same time
>>> +	 * to manage a USB2 device, so let's disable usb2 if usb3 is
>>> +	 * selected. In this case the USB2 device will be managed by
>>> +	 * the xhci controller.
>>> +	 */
>>> +
>>> +	xhci_node = of_find_compatible_node(NULL, NULL,
>>> +					"marvell,armada-375-xhci");
>>
>> huh.. that's too much binding between the controller and the PHY.
>>
> 
> That's why initially it was not part of the PHY framework.  The USB
> cluster is really a part managing common feature between the USB
> controllers which are part of the Armada 375 SoC.
> 
> However the initial version was not really good, because this piece of
> code was located in the mach- directory whereas we are trying to move
> most of the files out of this directory now. The USB cluster is not a
> real PHY as it is related to the PHY management this framework remains
> the best place for it.
> 
>>> +
>>> +	if (xhci_node && of_device_is_available(xhci_node)) {
>>> +		usb_cluster_phy[PHY_USB3].enable = true;
>>> +	} else {
>>> +		struct device_node *ehci_node;
>>> +		ehci_node = of_find_compatible_node(NULL, NULL,
>>> +					"marvell,orion-ehci");
>>> +		if (ehci_node && of_device_is_available(ehci_node))
>>> +			usb_cluster_phy[PHY_USB2].enable = true;
>>> +		of_node_put(ehci_node);
>>> +	}
>>> +
>>> +	of_node_put(xhci_node);
>>> +
>>> +	phy_provider = devm_of_phy_provider_register(&pdev->dev,
>>> +						     armada375_usb_phy_xlate);
>>> +	if (IS_ERR(phy_provider))
>>> +		return PTR_ERR(phy_provider);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static const struct of_device_id of_usb_cluster_table[] = {
>>> +	{ .compatible = "marvell,armada-375-usb-cluster", },
>>> +	{ /* end of list */ },
>>> +};
>>> +MODULE_DEVICE_TABLE(of, of_usb_cluster_table);
>>> +
>>> +static struct platform_driver armada375_usb_phy_driver = {
>>> +	.probe	= armada375_usb_phy_probe,
>>> +	.driver = {
>>> +		.of_match_table	= of_usb_cluster_table,
>>> +		.name  = "armada-375-usb-cluster",
>>> +		.owner = THIS_MODULE,
>>> +	}
>>> +};
>>> +module_platform_driver(armada375_usb_phy_driver);
>>> +
>>> +MODULE_DESCRIPTION("Armada 375 USB cluster driver");
>>> +MODULE_AUTHOR("Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>");
>>> +MODULE_LICENSE("GPL");
>>
>> GPL v2?
> 
> No it is really GPL v2 or latter as written at the top of this file
> 
> 
> Thanks,
> 
> Gregory
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
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: [PATCHv5 10/20] phy: add support for USB cluster on the Armada 375 SoC

[Gregory CLEMENT]

Hi Kishon,

Given the answers I provided to your concerns. I don't see any modification
to do to this driver. Do you agree?

If not which change is mandatory from your point of view?


Thanks,

Gregory


On 13/05/2014 11:41, Gregory CLEMENT wrote:
> On 13/05/2014 10:06, Gregory CLEMENT wrote:
>> On 13/05/2014 07:53, Kishon Vijay Abraham I wrote:
>>> Hi,
>>>
>>> On Sunday 11 May 2014 11:47 PM, Thomas Petazzoni wrote:
>>>> From: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>>
>>>> The Armada 375 SoC comes with an USB2 host and device controller and
>>>> an USB3 controller. The USB cluster control register allows to manage
>>>> common features of both USB controllers.
>>>>
>>>> This commit adds a driver integrated in the generic PHY framework to
>>>> control this USB cluster feature.
>>>>
>>>> Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>> Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>> ---
>>>>  drivers/phy/Kconfig              |   6 ++
>>>>  drivers/phy/Makefile             |   1 +
>>>>  drivers/phy/phy-armada375-usb2.c | 157 +++++++++++++++++++++++++++++++++++++++
>>>>  3 files changed, 164 insertions(+)
>>>>  create mode 100644 drivers/phy/phy-armada375-usb2.c
>>>>
>>>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>>>> index 3bb05f1..e63cf9d 100644
>>>> --- a/drivers/phy/Kconfig
>>>> +++ b/drivers/phy/Kconfig
>>>> @@ -15,6 +15,12 @@ config GENERIC_PHY
>>>>  	  phy users can obtain reference to the PHY. All the users of this
>>>>  	  framework should select this config.
>>>>  
>>>> +config ARMADA375_USBCLUSTER_PHY
>>>> +	def_bool y
>>>> +	depends on MACH_ARMADA_375 || COMPILE_TEST
>>>> +	depends on OF
>>>> +	select GENERIC_PHY
>>>> +
>>>>  config PHY_EXYNOS_MIPI_VIDEO
>>>>  	tristate "S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver"
>>>>  	depends on HAS_IOMEM
>>>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>>>> index 2faf78e..47d5a86 100644
>>>> --- a/drivers/phy/Makefile
>>>> +++ b/drivers/phy/Makefile
>>>> @@ -3,6 +3,7 @@
>>>>  #
>>>>  
>>>>  obj-$(CONFIG_GENERIC_PHY)		+= phy-core.o
>>>> +obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY)	+= phy-armada375-usb2.o
>>>>  obj-$(CONFIG_BCM_KONA_USB2_PHY)		+= phy-bcm-kona-usb2.o
>>>>  obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)	+= phy-exynos-dp-video.o
>>>>  obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
>>>> diff --git a/drivers/phy/phy-armada375-usb2.c b/drivers/phy/phy-armada375-usb2.c
>>>> new file mode 100644
>>>> index 0000000..a6f746d
>>>> --- /dev/null
>>>> +++ b/drivers/phy/phy-armada375-usb2.c
>>>> @@ -0,0 +1,157 @@
>>>> +/*
>>>> + * USB cluster support for Armada 375 platform.
>>>> + *
>>>> + * Copyright (C) 2014 Marvell
>>>> + *
>>>> + * Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>> + *
>>>> + * This file is licensed under the terms of the GNU General Public
>>>> + * License version 2 or later. This program is licensed "as is"
>>>> + * without any warranty of any kind, whether express or implied.
>>>> + *
>>>> + * Armada 375 comes with an USB2 host and device controller and an
>>>> + * USB3 controller. The USB cluster control register allows to manage
>>>> + * common features of both USB controllers.
>>>> + */
>>>> +
>>>> +#include <linux/init.h>
>>>> +#include <linux/io.h>
>>>> +#include <linux/kernel.h>
>>>> +#include <linux/module.h>
>>>> +#include <linux/of_address.h>
>>>> +#include <linux/phy/phy.h>
>>>> +#include <linux/platform_device.h>
>>>> +#include <linux/slab.h>
>>>> +
>>>> +#define USB2_PHY_CONFIG_DISABLE BIT(0)
>>>> +
>>>> +/* The USB cluster allows to choose between two PHYs */
>>>> +#define NB_PHY 2
>>>> +
>>>> +enum {
>>>> +	PHY_USB2 = 0,
>>>> +	PHY_USB3 = 1,
>>>> +};
>>>> +
>>>> +struct armada375_cluster_phy {
>>>> +	struct phy *phy;
>>>> +	void __iomem *reg;
>>>> +	bool enable;
>>>> +	bool use_usb3;
>>>> +};
>>>> +
>>>> +struct armada375_cluster_phy usb_cluster_phy[NB_PHY];
>>>> +
>>>> +static int armada375_usb_phy_init(struct phy *phy)
>>>> +{
>>>> +	struct armada375_cluster_phy *cluster_phy = phy_get_drvdata(phy);
>>>> +	u32 reg;
>>>
>>> This function should be protected since both your PHYs use this ops.
>>
>> Right
> 
> Actually only one PHY can access this register. See the probe function,
> cluster_phy->enable is only set to true for one PHY.
> 
>>
>>>> +
>>>> +	if (!cluster_phy->enable)
>>>> +		return -ENODEV;
>>>> +
>>>> +	reg = readl(cluster_phy->reg);
>>>> +	if (cluster_phy->use_usb3)
>>>> +		reg |= USB2_PHY_CONFIG_DISABLE;
>>>> +	else
>>>> +		reg &= ~USB2_PHY_CONFIG_DISABLE;
>>>> +	writel(reg, cluster_phy->reg);
>>>
>>> This is confusing since both your PHYs control the same bit?
> 
> Same here at the end the bit is accessed by only one PHY.
> 
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static struct phy_ops armada375_usb_phy_ops = {
>>>> +	.init = armada375_usb_phy_init,
>>>> +	.owner		= THIS_MODULE,
>>>> +};
>>>> +
>>>> +static struct phy *armada375_usb_phy_xlate(struct device *dev,
>>>> +					struct of_phandle_args *args)
>>>> +{
>>>> +	if (WARN_ON(args->args[0] >= NB_PHY))
>>>> +		return ERR_PTR(-ENODEV);
>>>> +
>>>> +	return usb_cluster_phy[args->args[0]].phy;
>>>> +}
>>>> +
>>>> +static int armada375_usb_phy_probe(struct platform_device *pdev)
>>>> +{
>>>> +	struct device *dev = &pdev->dev;
>>>> +	struct phy *phy;
>>>> +	struct phy_provider *phy_provider;
>>>> +	void __iomem *usb_cluster_base;
>>>> +	struct device_node *xhci_node;
>>>> +	struct resource *res;
>>>> +	int i;
>>>> +
>>>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>> +	usb_cluster_base = devm_ioremap_resource(&pdev->dev, res);
>>>> +	if (!usb_cluster_base)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	for (i = 0; i < NB_PHY; i++) {
>>>
>>> For devices which have multiple PHYs, each PHY should be modelled as the
>>> sub-node of the *PHY provider* device node.
>>
>> Actually it is the opposite the same PHY is shared between the EHCI
>> and the xHCI controllers. It is more a PHY muxer than a PHY itself.
>>
>> I had to create 2 logical PHYs because once the phy_init() is called
>> by a USB driver then the .init ops is not called anymore by the next
>> call to phy_init(). One of the goal of this is to disable a port for
>> the USB controller which can't use it due to the configuration of the
>> USB cluster.
>>
>> But I can see how to make this two "pseudo" PHYs sub-node of the *PHY
>> provider* device node. It shouldn't change the internal logic of this
>> driver.
> 
> I need to make a distinction when the PHY access by the xHCI or when
> it was access by the EHCI. If I create two new sub-node then I will
> also need to add a property to make this distinction. It seems a little
> overkill for the need.
> 
>>
>>
>>>> +		phy = devm_phy_create(dev, &armada375_usb_phy_ops, NULL);
>>>> +		if (IS_ERR(phy)) {
>>>> +			dev_err(dev, "failed to create PHY n%d\n", i);
>>>> +			return PTR_ERR(phy);
>>>> +		}
>>>> +
>>>> +		usb_cluster_phy[i].phy = phy;
>>>> +		usb_cluster_phy[i].reg = usb_cluster_base;
>>>> +		usb_cluster_phy[i].enable = false;
>>>> +		phy_set_drvdata(phy, &usb_cluster_phy[i]);
>>>> +	}
>>>> +
>>>> +	usb_cluster_phy[PHY_USB2].use_usb3 = false;
>>>> +	usb_cluster_phy[PHY_USB3].use_usb3 = true;
>>>> +
>>>> +	/*
>>>> +	 * We can't use the first usb2 unit and usb3 at the same time
>>>> +	 * to manage a USB2 device, so let's disable usb2 if usb3 is
>>>> +	 * selected. In this case the USB2 device will be managed by
>>>> +	 * the xhci controller.
>>>> +	 */
>>>> +
>>>> +	xhci_node = of_find_compatible_node(NULL, NULL,
>>>> +					"marvell,armada-375-xhci");
>>>
>>> huh.. that's too much binding between the controller and the PHY.
>>>
>>
>> That's why initially it was not part of the PHY framework.  The USB
>> cluster is really a part managing common feature between the USB
>> controllers which are part of the Armada 375 SoC.
>>
>> However the initial version was not really good, because this piece of
>> code was located in the mach- directory whereas we are trying to move
>> most of the files out of this directory now. The USB cluster is not a
>> real PHY as it is related to the PHY management this framework remains
>> the best place for it.
>>
>>>> +
>>>> +	if (xhci_node && of_device_is_available(xhci_node)) {
>>>> +		usb_cluster_phy[PHY_USB3].enable = true;
>>>> +	} else {
>>>> +		struct device_node *ehci_node;
>>>> +		ehci_node = of_find_compatible_node(NULL, NULL,
>>>> +					"marvell,orion-ehci");
>>>> +		if (ehci_node && of_device_is_available(ehci_node))
>>>> +			usb_cluster_phy[PHY_USB2].enable = true;
>>>> +		of_node_put(ehci_node);
>>>> +	}
>>>> +
>>>> +	of_node_put(xhci_node);
>>>> +
>>>> +	phy_provider = devm_of_phy_provider_register(&pdev->dev,
>>>> +						     armada375_usb_phy_xlate);
>>>> +	if (IS_ERR(phy_provider))
>>>> +		return PTR_ERR(phy_provider);
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static const struct of_device_id of_usb_cluster_table[] = {
>>>> +	{ .compatible = "marvell,armada-375-usb-cluster", },
>>>> +	{ /* end of list */ },
>>>> +};
>>>> +MODULE_DEVICE_TABLE(of, of_usb_cluster_table);
>>>> +
>>>> +static struct platform_driver armada375_usb_phy_driver = {
>>>> +	.probe	= armada375_usb_phy_probe,
>>>> +	.driver = {
>>>> +		.of_match_table	= of_usb_cluster_table,
>>>> +		.name  = "armada-375-usb-cluster",
>>>> +		.owner = THIS_MODULE,
>>>> +	}
>>>> +};
>>>> +module_platform_driver(armada375_usb_phy_driver);
>>>> +
>>>> +MODULE_DESCRIPTION("Armada 375 USB cluster driver");
>>>> +MODULE_AUTHOR("Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>");
>>>> +MODULE_LICENSE("GPL");
>>>
>>> GPL v2?
>>
>> No it is really GPL v2 or latter as written at the top of this file
>>
>>
>> Thanks,
>>
>> Gregory
>>
> 
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
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: [PATCHv5 10/20] phy: add support for USB cluster on the Armada 375 SoC

[Kishon Vijay Abraham I]

Hi,

On Tuesday 13 May 2014 03:11 PM, Gregory CLEMENT wrote:
> On 13/05/2014 10:06, Gregory CLEMENT wrote:
>> On 13/05/2014 07:53, Kishon Vijay Abraham I wrote:
>>> Hi,
>>>
>>> On Sunday 11 May 2014 11:47 PM, Thomas Petazzoni wrote:
>>>> From: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>>
>>>> The Armada 375 SoC comes with an USB2 host and device controller and
>>>> an USB3 controller. The USB cluster control register allows to manage
>>>> common features of both USB controllers.
>>>>
>>>> This commit adds a driver integrated in the generic PHY framework to
>>>> control this USB cluster feature.
>>>>
>>>> Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>> Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>> ---
>>>>  drivers/phy/Kconfig              |   6 ++
>>>>  drivers/phy/Makefile             |   1 +
>>>>  drivers/phy/phy-armada375-usb2.c | 157 +++++++++++++++++++++++++++++++++++++++
>>>>  3 files changed, 164 insertions(+)
>>>>  create mode 100644 drivers/phy/phy-armada375-usb2.c
>>>>
>>>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>>>> index 3bb05f1..e63cf9d 100644
>>>> --- a/drivers/phy/Kconfig
>>>> +++ b/drivers/phy/Kconfig
>>>> @@ -15,6 +15,12 @@ config GENERIC_PHY
>>>>  	  phy users can obtain reference to the PHY. All the users of this
>>>>  	  framework should select this config.
>>>>  
>>>> +config ARMADA375_USBCLUSTER_PHY
>>>> +	def_bool y
>>>> +	depends on MACH_ARMADA_375 || COMPILE_TEST
>>>> +	depends on OF
>>>> +	select GENERIC_PHY
>>>> +
>>>>  config PHY_EXYNOS_MIPI_VIDEO
>>>>  	tristate "S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver"
>>>>  	depends on HAS_IOMEM
>>>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>>>> index 2faf78e..47d5a86 100644
>>>> --- a/drivers/phy/Makefile
>>>> +++ b/drivers/phy/Makefile
>>>> @@ -3,6 +3,7 @@
>>>>  #
>>>>  
>>>>  obj-$(CONFIG_GENERIC_PHY)		+= phy-core.o
>>>> +obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY)	+= phy-armada375-usb2.o
>>>>  obj-$(CONFIG_BCM_KONA_USB2_PHY)		+= phy-bcm-kona-usb2.o
>>>>  obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)	+= phy-exynos-dp-video.o
>>>>  obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
>>>> diff --git a/drivers/phy/phy-armada375-usb2.c b/drivers/phy/phy-armada375-usb2.c
>>>> new file mode 100644
>>>> index 0000000..a6f746d
>>>> --- /dev/null
>>>> +++ b/drivers/phy/phy-armada375-usb2.c
>>>> @@ -0,0 +1,157 @@
>>>> +/*
>>>> + * USB cluster support for Armada 375 platform.
>>>> + *
>>>> + * Copyright (C) 2014 Marvell
>>>> + *
>>>> + * Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>> + *
>>>> + * This file is licensed under the terms of the GNU General Public
>>>> + * License version 2 or later. This program is licensed "as is"
>>>> + * without any warranty of any kind, whether express or implied.
>>>> + *
>>>> + * Armada 375 comes with an USB2 host and device controller and an
>>>> + * USB3 controller. The USB cluster control register allows to manage
>>>> + * common features of both USB controllers.
>>>> + */
>>>> +
>>>> +#include <linux/init.h>
>>>> +#include <linux/io.h>
>>>> +#include <linux/kernel.h>
>>>> +#include <linux/module.h>
>>>> +#include <linux/of_address.h>
>>>> +#include <linux/phy/phy.h>
>>>> +#include <linux/platform_device.h>
>>>> +#include <linux/slab.h>
>>>> +
>>>> +#define USB2_PHY_CONFIG_DISABLE BIT(0)
>>>> +
>>>> +/* The USB cluster allows to choose between two PHYs */
>>>> +#define NB_PHY 2
>>>> +
>>>> +enum {
>>>> +	PHY_USB2 = 0,
>>>> +	PHY_USB3 = 1,
>>>> +};
>>>> +
>>>> +struct armada375_cluster_phy {
>>>> +	struct phy *phy;
>>>> +	void __iomem *reg;
>>>> +	bool enable;
>>>> +	bool use_usb3;
>>>> +};
>>>> +
>>>> +struct armada375_cluster_phy usb_cluster_phy[NB_PHY];
>>>> +
>>>> +static int armada375_usb_phy_init(struct phy *phy)
>>>> +{
>>>> +	struct armada375_cluster_phy *cluster_phy = phy_get_drvdata(phy);
>>>> +	u32 reg;
>>>
>>> This function should be protected since both your PHYs use this ops.
>>
>> Right
> 
> Actually only one PHY can access this register. See the probe function,
> cluster_phy->enable is only set to true for one PHY.
> 
>>
>>>> +
>>>> +	if (!cluster_phy->enable)
>>>> +		return -ENODEV;
>>>> +
>>>> +	reg = readl(cluster_phy->reg);
>>>> +	if (cluster_phy->use_usb3)
>>>> +		reg |= USB2_PHY_CONFIG_DISABLE;
>>>> +	else
>>>> +		reg &= ~USB2_PHY_CONFIG_DISABLE;
>>>> +	writel(reg, cluster_phy->reg);
>>>
>>> This is confusing since both your PHYs control the same bit?
> 
> Same here at the end the bit is accessed by only one PHY.
> 
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static struct phy_ops armada375_usb_phy_ops = {
>>>> +	.init = armada375_usb_phy_init,
>>>> +	.owner		= THIS_MODULE,
>>>> +};
>>>> +
>>>> +static struct phy *armada375_usb_phy_xlate(struct device *dev,
>>>> +					struct of_phandle_args *args)
>>>> +{
>>>> +	if (WARN_ON(args->args[0] >= NB_PHY))
>>>> +		return ERR_PTR(-ENODEV);
>>>> +
>>>> +	return usb_cluster_phy[args->args[0]].phy;
>>>> +}
>>>> +
>>>> +static int armada375_usb_phy_probe(struct platform_device *pdev)
>>>> +{
>>>> +	struct device *dev = &pdev->dev;
>>>> +	struct phy *phy;
>>>> +	struct phy_provider *phy_provider;
>>>> +	void __iomem *usb_cluster_base;
>>>> +	struct device_node *xhci_node;
>>>> +	struct resource *res;
>>>> +	int i;
>>>> +
>>>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>> +	usb_cluster_base = devm_ioremap_resource(&pdev->dev, res);
>>>> +	if (!usb_cluster_base)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	for (i = 0; i < NB_PHY; i++) {
>>>
>>> For devices which have multiple PHYs, each PHY should be modelled as the
>>> sub-node of the *PHY provider* device node.
>>
>> Actually it is the opposite the same PHY is shared between the EHCI
>> and the xHCI controllers. It is more a PHY muxer than a PHY itself.
>>
>> I had to create 2 logical PHYs because once the phy_init() is called
>> by a USB driver then the .init ops is not called anymore by the next
>> call to phy_init(). One of the goal of this is to disable a port for
>> the USB controller which can't use it due to the configuration of the
>> USB cluster.
>>
>> But I can see how to make this two "pseudo" PHYs sub-node of the *PHY
>> provider* device node. It shouldn't change the internal logic of this
>> driver.
> 
> I need to make a distinction when the PHY access by the xHCI or when
> it was access by the EHCI. If I create two new sub-node then I will
> also need to add a property to make this distinction. It seems a little
> overkill for the need.

Alright, so you have a single PHY that can be used by either XHCI or EHCI? And
the use of PHY is mutually exclusive? How should it behave if you have both
XHCI and EHCI?

One way to configure the PHY to a particular mode is by passing it as phandle
arguments. I think you can use that to enable or disable USB2_PHY_CONFIG_DISABLE?

Thanks
Kishon
--
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: [PATCHv5 10/20] phy: add support for USB cluster on the Armada 375 SoC

[Gregory CLEMENT]

On 14/05/2014 16:27, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Tuesday 13 May 2014 03:11 PM, Gregory CLEMENT wrote:
>> On 13/05/2014 10:06, Gregory CLEMENT wrote:
>>> On 13/05/2014 07:53, Kishon Vijay Abraham I wrote:
>>>> Hi,
>>>>
>>>> On Sunday 11 May 2014 11:47 PM, Thomas Petazzoni wrote:
>>>>> From: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>>>
>>>>> The Armada 375 SoC comes with an USB2 host and device controller and
>>>>> an USB3 controller. The USB cluster control register allows to manage
>>>>> common features of both USB controllers.
>>>>>
>>>>> This commit adds a driver integrated in the generic PHY framework to
>>>>> control this USB cluster feature.
>>>>>
>>>>> Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>>> Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>>> ---
>>>>>  drivers/phy/Kconfig              |   6 ++
>>>>>  drivers/phy/Makefile             |   1 +
>>>>>  drivers/phy/phy-armada375-usb2.c | 157 +++++++++++++++++++++++++++++++++++++++
>>>>>  3 files changed, 164 insertions(+)
>>>>>  create mode 100644 drivers/phy/phy-armada375-usb2.c
>>>>>
>>>>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>>>>> index 3bb05f1..e63cf9d 100644
>>>>> --- a/drivers/phy/Kconfig
>>>>> +++ b/drivers/phy/Kconfig
>>>>> @@ -15,6 +15,12 @@ config GENERIC_PHY
>>>>>  	  phy users can obtain reference to the PHY. All the users of this
>>>>>  	  framework should select this config.
>>>>>  
>>>>> +config ARMADA375_USBCLUSTER_PHY
>>>>> +	def_bool y
>>>>> +	depends on MACH_ARMADA_375 || COMPILE_TEST
>>>>> +	depends on OF
>>>>> +	select GENERIC_PHY
>>>>> +
>>>>>  config PHY_EXYNOS_MIPI_VIDEO
>>>>>  	tristate "S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver"
>>>>>  	depends on HAS_IOMEM
>>>>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>>>>> index 2faf78e..47d5a86 100644
>>>>> --- a/drivers/phy/Makefile
>>>>> +++ b/drivers/phy/Makefile
>>>>> @@ -3,6 +3,7 @@
>>>>>  #
>>>>>  
>>>>>  obj-$(CONFIG_GENERIC_PHY)		+= phy-core.o
>>>>> +obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY)	+= phy-armada375-usb2.o
>>>>>  obj-$(CONFIG_BCM_KONA_USB2_PHY)		+= phy-bcm-kona-usb2.o
>>>>>  obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)	+= phy-exynos-dp-video.o
>>>>>  obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
>>>>> diff --git a/drivers/phy/phy-armada375-usb2.c b/drivers/phy/phy-armada375-usb2.c
>>>>> new file mode 100644
>>>>> index 0000000..a6f746d
>>>>> --- /dev/null
>>>>> +++ b/drivers/phy/phy-armada375-usb2.c
>>>>> @@ -0,0 +1,157 @@
>>>>> +/*
>>>>> + * USB cluster support for Armada 375 platform.
>>>>> + *
>>>>> + * Copyright (C) 2014 Marvell
>>>>> + *
>>>>> + * Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>>> + *
>>>>> + * This file is licensed under the terms of the GNU General Public
>>>>> + * License version 2 or later. This program is licensed "as is"
>>>>> + * without any warranty of any kind, whether express or implied.
>>>>> + *
>>>>> + * Armada 375 comes with an USB2 host and device controller and an
>>>>> + * USB3 controller. The USB cluster control register allows to manage
>>>>> + * common features of both USB controllers.
>>>>> + */
>>>>> +
>>>>> +#include <linux/init.h>
>>>>> +#include <linux/io.h>
>>>>> +#include <linux/kernel.h>
>>>>> +#include <linux/module.h>
>>>>> +#include <linux/of_address.h>
>>>>> +#include <linux/phy/phy.h>
>>>>> +#include <linux/platform_device.h>
>>>>> +#include <linux/slab.h>
>>>>> +
>>>>> +#define USB2_PHY_CONFIG_DISABLE BIT(0)
>>>>> +
>>>>> +/* The USB cluster allows to choose between two PHYs */
>>>>> +#define NB_PHY 2
>>>>> +
>>>>> +enum {
>>>>> +	PHY_USB2 = 0,
>>>>> +	PHY_USB3 = 1,
>>>>> +};
>>>>> +
>>>>> +struct armada375_cluster_phy {
>>>>> +	struct phy *phy;
>>>>> +	void __iomem *reg;
>>>>> +	bool enable;
>>>>> +	bool use_usb3;
>>>>> +};
>>>>> +
>>>>> +struct armada375_cluster_phy usb_cluster_phy[NB_PHY];
>>>>> +
>>>>> +static int armada375_usb_phy_init(struct phy *phy)
>>>>> +{
>>>>> +	struct armada375_cluster_phy *cluster_phy = phy_get_drvdata(phy);
>>>>> +	u32 reg;
>>>>
>>>> This function should be protected since both your PHYs use this ops.
>>>
>>> Right
>>
>> Actually only one PHY can access this register. See the probe function,
>> cluster_phy->enable is only set to true for one PHY.
>>
>>>
>>>>> +
>>>>> +	if (!cluster_phy->enable)
>>>>> +		return -ENODEV;
>>>>> +
>>>>> +	reg = readl(cluster_phy->reg);
>>>>> +	if (cluster_phy->use_usb3)
>>>>> +		reg |= USB2_PHY_CONFIG_DISABLE;
>>>>> +	else
>>>>> +		reg &= ~USB2_PHY_CONFIG_DISABLE;
>>>>> +	writel(reg, cluster_phy->reg);
>>>>
>>>> This is confusing since both your PHYs control the same bit?
>>
>> Same here at the end the bit is accessed by only one PHY.
>>
>>>>> +
>>>>> +	return 0;
>>>>> +}
>>>>> +
>>>>> +static struct phy_ops armada375_usb_phy_ops = {
>>>>> +	.init = armada375_usb_phy_init,
>>>>> +	.owner		= THIS_MODULE,
>>>>> +};
>>>>> +
>>>>> +static struct phy *armada375_usb_phy_xlate(struct device *dev,
>>>>> +					struct of_phandle_args *args)
>>>>> +{
>>>>> +	if (WARN_ON(args->args[0] >= NB_PHY))
>>>>> +		return ERR_PTR(-ENODEV);
>>>>> +
>>>>> +	return usb_cluster_phy[args->args[0]].phy;
>>>>> +}
>>>>> +
>>>>> +static int armada375_usb_phy_probe(struct platform_device *pdev)
>>>>> +{
>>>>> +	struct device *dev = &pdev->dev;
>>>>> +	struct phy *phy;
>>>>> +	struct phy_provider *phy_provider;
>>>>> +	void __iomem *usb_cluster_base;
>>>>> +	struct device_node *xhci_node;
>>>>> +	struct resource *res;
>>>>> +	int i;
>>>>> +
>>>>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>>> +	usb_cluster_base = devm_ioremap_resource(&pdev->dev, res);
>>>>> +	if (!usb_cluster_base)
>>>>> +		return -ENOMEM;
>>>>> +
>>>>> +	for (i = 0; i < NB_PHY; i++) {
>>>>
>>>> For devices which have multiple PHYs, each PHY should be modelled as the
>>>> sub-node of the *PHY provider* device node.
>>>
>>> Actually it is the opposite the same PHY is shared between the EHCI
>>> and the xHCI controllers. It is more a PHY muxer than a PHY itself.
>>>
>>> I had to create 2 logical PHYs because once the phy_init() is called
>>> by a USB driver then the .init ops is not called anymore by the next
>>> call to phy_init(). One of the goal of this is to disable a port for
>>> the USB controller which can't use it due to the configuration of the
>>> USB cluster.
>>>
>>> But I can see how to make this two "pseudo" PHYs sub-node of the *PHY
>>> provider* device node. It shouldn't change the internal logic of this
>>> driver.
>>
>> I need to make a distinction when the PHY access by the xHCI or when
>> it was access by the EHCI. If I create two new sub-node then I will
>> also need to add a property to make this distinction. It seems a little
>> overkill for the need.
> 
> Alright, so you have a single PHY that can be used by either XHCI or EHCI? And
> the use of PHY is mutually exclusive? How should it behave if you have both
> XHCI and EHCI?

if we have both XHCI and EHCI then it is the USB2_PHY_CONFIG_DISABLE which
determine which one is used. By default we decide to select the XHCI.

> 
> One way to configure the PHY to a particular mode is by passing it as phandle
> arguments. I think you can use that to enable or disable USB2_PHY_CONFIG_DISABLE?

actually it was more or less what I do:
for the XHCI I use:
phys = <&usbcluster 1>;
which enable the USB2_PHY_CONFIG_DISABLE

for the EHCI I use phys = <&usbcluster 0>;
which disable the USB2_PHY_CONFIG_DISABLE

If I had to create two PHY it was because of the behavior of
phy_init(). I need to be able to disable a controller if it can't use
the PHY. For this purpose my ops->init() exits in error. However
phy_init() will call ops->init() only one time, then the internal
counter init_count will be incremented, and the next call to phy_init
will skip the call to ops->init. And the behavior is the same for
phy_power_on().

So given this I don't see how to do in an other way except by
modifying the value of the counter in my ops.


Thanks,

Gregory


> 
> Thanks
> Kishon
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
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: [PATCHv5 10/20] phy: add support for USB cluster on the Armada 375 SoC

[Gregory CLEMENT]

Hi Kishon,

On 14/05/2014 17:35, Gregory CLEMENT wrote:
> On 14/05/2014 16:27, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Tuesday 13 May 2014 03:11 PM, Gregory CLEMENT wrote:
>>> On 13/05/2014 10:06, Gregory CLEMENT wrote:
>>>> On 13/05/2014 07:53, Kishon Vijay Abraham I wrote:
>>>>> Hi,
>>>>>
>>>>> On Sunday 11 May 2014 11:47 PM, Thomas Petazzoni wrote:
>>>>>> From: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>>>>
>>>>>> The Armada 375 SoC comes with an USB2 host and device controller and
>>>>>> an USB3 controller. The USB cluster control register allows to manage
>>>>>> common features of both USB controllers.
>>>>>>
>>>>>> This commit adds a driver integrated in the generic PHY framework to
>>>>>> control this USB cluster feature.
>>>>>>
>>>>>> Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>>>> Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>>>> ---
>>>>>>  drivers/phy/Kconfig              |   6 ++
>>>>>>  drivers/phy/Makefile             |   1 +
>>>>>>  drivers/phy/phy-armada375-usb2.c | 157 +++++++++++++++++++++++++++++++++++++++
>>>>>>  3 files changed, 164 insertions(+)
>>>>>>  create mode 100644 drivers/phy/phy-armada375-usb2.c
>>>>>>
>>>>>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>>>>>> index 3bb05f1..e63cf9d 100644
>>>>>> --- a/drivers/phy/Kconfig
>>>>>> +++ b/drivers/phy/Kconfig
>>>>>> @@ -15,6 +15,12 @@ config GENERIC_PHY
>>>>>>  	  phy users can obtain reference to the PHY. All the users of this
>>>>>>  	  framework should select this config.
>>>>>>  
>>>>>> +config ARMADA375_USBCLUSTER_PHY
>>>>>> +	def_bool y
>>>>>> +	depends on MACH_ARMADA_375 || COMPILE_TEST
>>>>>> +	depends on OF
>>>>>> +	select GENERIC_PHY
>>>>>> +
>>>>>>  config PHY_EXYNOS_MIPI_VIDEO
>>>>>>  	tristate "S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver"
>>>>>>  	depends on HAS_IOMEM
>>>>>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>>>>>> index 2faf78e..47d5a86 100644
>>>>>> --- a/drivers/phy/Makefile
>>>>>> +++ b/drivers/phy/Makefile
>>>>>> @@ -3,6 +3,7 @@
>>>>>>  #
>>>>>>  
>>>>>>  obj-$(CONFIG_GENERIC_PHY)		+= phy-core.o
>>>>>> +obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY)	+= phy-armada375-usb2.o
>>>>>>  obj-$(CONFIG_BCM_KONA_USB2_PHY)		+= phy-bcm-kona-usb2.o
>>>>>>  obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)	+= phy-exynos-dp-video.o
>>>>>>  obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
>>>>>> diff --git a/drivers/phy/phy-armada375-usb2.c b/drivers/phy/phy-armada375-usb2.c
>>>>>> new file mode 100644
>>>>>> index 0000000..a6f746d
>>>>>> --- /dev/null
>>>>>> +++ b/drivers/phy/phy-armada375-usb2.c
>>>>>> @@ -0,0 +1,157 @@
>>>>>> +/*
>>>>>> + * USB cluster support for Armada 375 platform.
>>>>>> + *
>>>>>> + * Copyright (C) 2014 Marvell
>>>>>> + *
>>>>>> + * Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>>>> + *
>>>>>> + * This file is licensed under the terms of the GNU General Public
>>>>>> + * License version 2 or later. This program is licensed "as is"
>>>>>> + * without any warranty of any kind, whether express or implied.
>>>>>> + *
>>>>>> + * Armada 375 comes with an USB2 host and device controller and an
>>>>>> + * USB3 controller. The USB cluster control register allows to manage
>>>>>> + * common features of both USB controllers.
>>>>>> + */
>>>>>> +
>>>>>> +#include <linux/init.h>
>>>>>> +#include <linux/io.h>
>>>>>> +#include <linux/kernel.h>
>>>>>> +#include <linux/module.h>
>>>>>> +#include <linux/of_address.h>
>>>>>> +#include <linux/phy/phy.h>
>>>>>> +#include <linux/platform_device.h>
>>>>>> +#include <linux/slab.h>
>>>>>> +
>>>>>> +#define USB2_PHY_CONFIG_DISABLE BIT(0)
>>>>>> +
>>>>>> +/* The USB cluster allows to choose between two PHYs */
>>>>>> +#define NB_PHY 2
>>>>>> +
>>>>>> +enum {
>>>>>> +	PHY_USB2 = 0,
>>>>>> +	PHY_USB3 = 1,
>>>>>> +};
>>>>>> +
>>>>>> +struct armada375_cluster_phy {
>>>>>> +	struct phy *phy;
>>>>>> +	void __iomem *reg;
>>>>>> +	bool enable;
>>>>>> +	bool use_usb3;
>>>>>> +};
>>>>>> +
>>>>>> +struct armada375_cluster_phy usb_cluster_phy[NB_PHY];
>>>>>> +
>>>>>> +static int armada375_usb_phy_init(struct phy *phy)
>>>>>> +{
>>>>>> +	struct armada375_cluster_phy *cluster_phy = phy_get_drvdata(phy);
>>>>>> +	u32 reg;
>>>>>
>>>>> This function should be protected since both your PHYs use this ops.
>>>>
>>>> Right
>>>
>>> Actually only one PHY can access this register. See the probe function,
>>> cluster_phy->enable is only set to true for one PHY.
>>>
>>>>
>>>>>> +
>>>>>> +	if (!cluster_phy->enable)
>>>>>> +		return -ENODEV;
>>>>>> +
>>>>>> +	reg = readl(cluster_phy->reg);
>>>>>> +	if (cluster_phy->use_usb3)
>>>>>> +		reg |= USB2_PHY_CONFIG_DISABLE;
>>>>>> +	else
>>>>>> +		reg &= ~USB2_PHY_CONFIG_DISABLE;
>>>>>> +	writel(reg, cluster_phy->reg);
>>>>>
>>>>> This is confusing since both your PHYs control the same bit?
>>>
>>> Same here at the end the bit is accessed by only one PHY.
>>>
>>>>>> +
>>>>>> +	return 0;
>>>>>> +}
>>>>>> +
>>>>>> +static struct phy_ops armada375_usb_phy_ops = {
>>>>>> +	.init = armada375_usb_phy_init,
>>>>>> +	.owner		= THIS_MODULE,
>>>>>> +};
>>>>>> +
>>>>>> +static struct phy *armada375_usb_phy_xlate(struct device *dev,
>>>>>> +					struct of_phandle_args *args)
>>>>>> +{
>>>>>> +	if (WARN_ON(args->args[0] >= NB_PHY))
>>>>>> +		return ERR_PTR(-ENODEV);
>>>>>> +
>>>>>> +	return usb_cluster_phy[args->args[0]].phy;
>>>>>> +}
>>>>>> +
>>>>>> +static int armada375_usb_phy_probe(struct platform_device *pdev)
>>>>>> +{
>>>>>> +	struct device *dev = &pdev->dev;
>>>>>> +	struct phy *phy;
>>>>>> +	struct phy_provider *phy_provider;
>>>>>> +	void __iomem *usb_cluster_base;
>>>>>> +	struct device_node *xhci_node;
>>>>>> +	struct resource *res;
>>>>>> +	int i;
>>>>>> +
>>>>>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>>>> +	usb_cluster_base = devm_ioremap_resource(&pdev->dev, res);
>>>>>> +	if (!usb_cluster_base)
>>>>>> +		return -ENOMEM;
>>>>>> +
>>>>>> +	for (i = 0; i < NB_PHY; i++) {
>>>>>
>>>>> For devices which have multiple PHYs, each PHY should be modelled as the
>>>>> sub-node of the *PHY provider* device node.
>>>>
>>>> Actually it is the opposite the same PHY is shared between the EHCI
>>>> and the xHCI controllers. It is more a PHY muxer than a PHY itself.
>>>>
>>>> I had to create 2 logical PHYs because once the phy_init() is called
>>>> by a USB driver then the .init ops is not called anymore by the next
>>>> call to phy_init(). One of the goal of this is to disable a port for
>>>> the USB controller which can't use it due to the configuration of the
>>>> USB cluster.
>>>>
>>>> But I can see how to make this two "pseudo" PHYs sub-node of the *PHY
>>>> provider* device node. It shouldn't change the internal logic of this
>>>> driver.
>>>
>>> I need to make a distinction when the PHY access by the xHCI or when
>>> it was access by the EHCI. If I create two new sub-node then I will
>>> also need to add a property to make this distinction. It seems a little
>>> overkill for the need.
>>
>> Alright, so you have a single PHY that can be used by either XHCI or EHCI? And
>> the use of PHY is mutually exclusive? How should it behave if you have both
>> XHCI and EHCI?
> 
> if we have both XHCI and EHCI then it is the USB2_PHY_CONFIG_DISABLE which
> determine which one is used. By default we decide to select the XHCI.
> 
>>
>> One way to configure the PHY to a particular mode is by passing it as phandle
>> arguments. I think you can use that to enable or disable USB2_PHY_CONFIG_DISABLE?
> 
> actually it was more or less what I do:
> for the XHCI I use:
> phys = <&usbcluster 1>;
> which enable the USB2_PHY_CONFIG_DISABLE
> 
> for the EHCI I use phys = <&usbcluster 0>;
> which disable the USB2_PHY_CONFIG_DISABLE
> 
> If I had to create two PHY it was because of the behavior of
> phy_init(). I need to be able to disable a controller if it can't use
> the PHY. For this purpose my ops->init() exits in error. However
> phy_init() will call ops->init() only one time, then the internal
> counter init_count will be incremented, and the next call to phy_init
> will skip the call to ops->init. And the behavior is the same for
> phy_power_on().
> 
> So given this I don't see how to do in an other way except by
> modifying the value of the counter in my ops.

What do you prefer here? Keep the current implementation or using
only one PHY by passing a argument through the phandle and in the
same time hacking the init_count?

Note that in both case the binding will be the same.

I would prefer the first solution because hacking the init_count (or
the power_count) don't seem to me the right thing to do.


Thanks,

Gregory



> 
> 
> Thanks,
> 
> Gregory
> 
> 
>>
>> Thanks
>> Kishon
>>
> 
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
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: [PATCHv5 10/20] phy: add support for USB cluster on the Armada 375 SoC

[Kishon Vijay Abraham I]

Hi,

On Thursday 15 May 2014 12:31 PM, Gregory CLEMENT wrote:
> Hi Kishon,
> 
> On 14/05/2014 17:35, Gregory CLEMENT wrote:
>> On 14/05/2014 16:27, Kishon Vijay Abraham I wrote:
>>> Hi,
>>>
>>> On Tuesday 13 May 2014 03:11 PM, Gregory CLEMENT wrote:
>>>> On 13/05/2014 10:06, Gregory CLEMENT wrote:
>>>>> On 13/05/2014 07:53, Kishon Vijay Abraham I wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On Sunday 11 May 2014 11:47 PM, Thomas Petazzoni wrote:
>>>>>>> From: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>>>>>
>>>>>>> The Armada 375 SoC comes with an USB2 host and device controller and
>>>>>>> an USB3 controller. The USB cluster control register allows to manage
>>>>>>> common features of both USB controllers.
>>>>>>>
>>>>>>> This commit adds a driver integrated in the generic PHY framework to
>>>>>>> control this USB cluster feature.
>>>>>>>
>>>>>>> Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>>>>> Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>>>>> ---
>>>>>>>  drivers/phy/Kconfig              |   6 ++
>>>>>>>  drivers/phy/Makefile             |   1 +
>>>>>>>  drivers/phy/phy-armada375-usb2.c | 157 +++++++++++++++++++++++++++++++++++++++
>>>>>>>  3 files changed, 164 insertions(+)
>>>>>>>  create mode 100644 drivers/phy/phy-armada375-usb2.c
>>>>>>>
>>>>>>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>>>>>>> index 3bb05f1..e63cf9d 100644
>>>>>>> --- a/drivers/phy/Kconfig
>>>>>>> +++ b/drivers/phy/Kconfig
>>>>>>> @@ -15,6 +15,12 @@ config GENERIC_PHY
>>>>>>>  	  phy users can obtain reference to the PHY. All the users of this
>>>>>>>  	  framework should select this config.
>>>>>>>  
>>>>>>> +config ARMADA375_USBCLUSTER_PHY
>>>>>>> +	def_bool y
>>>>>>> +	depends on MACH_ARMADA_375 || COMPILE_TEST
>>>>>>> +	depends on OF
>>>>>>> +	select GENERIC_PHY
>>>>>>> +
>>>>>>>  config PHY_EXYNOS_MIPI_VIDEO
>>>>>>>  	tristate "S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver"
>>>>>>>  	depends on HAS_IOMEM
>>>>>>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>>>>>>> index 2faf78e..47d5a86 100644
>>>>>>> --- a/drivers/phy/Makefile
>>>>>>> +++ b/drivers/phy/Makefile
>>>>>>> @@ -3,6 +3,7 @@
>>>>>>>  #
>>>>>>>  
>>>>>>>  obj-$(CONFIG_GENERIC_PHY)		+= phy-core.o
>>>>>>> +obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY)	+= phy-armada375-usb2.o
>>>>>>>  obj-$(CONFIG_BCM_KONA_USB2_PHY)		+= phy-bcm-kona-usb2.o
>>>>>>>  obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)	+= phy-exynos-dp-video.o
>>>>>>>  obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
>>>>>>> diff --git a/drivers/phy/phy-armada375-usb2.c b/drivers/phy/phy-armada375-usb2.c
>>>>>>> new file mode 100644
>>>>>>> index 0000000..a6f746d
>>>>>>> --- /dev/null
>>>>>>> +++ b/drivers/phy/phy-armada375-usb2.c
>>>>>>> @@ -0,0 +1,157 @@
>>>>>>> +/*
>>>>>>> + * USB cluster support for Armada 375 platform.
>>>>>>> + *
>>>>>>> + * Copyright (C) 2014 Marvell
>>>>>>> + *
>>>>>>> + * Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>>>>> + *
>>>>>>> + * This file is licensed under the terms of the GNU General Public
>>>>>>> + * License version 2 or later. This program is licensed "as is"
>>>>>>> + * without any warranty of any kind, whether express or implied.
>>>>>>> + *
>>>>>>> + * Armada 375 comes with an USB2 host and device controller and an
>>>>>>> + * USB3 controller. The USB cluster control register allows to manage
>>>>>>> + * common features of both USB controllers.
>>>>>>> + */
>>>>>>> +
>>>>>>> +#include <linux/init.h>
>>>>>>> +#include <linux/io.h>
>>>>>>> +#include <linux/kernel.h>
>>>>>>> +#include <linux/module.h>
>>>>>>> +#include <linux/of_address.h>
>>>>>>> +#include <linux/phy/phy.h>
>>>>>>> +#include <linux/platform_device.h>
>>>>>>> +#include <linux/slab.h>
>>>>>>> +
>>>>>>> +#define USB2_PHY_CONFIG_DISABLE BIT(0)
>>>>>>> +
>>>>>>> +/* The USB cluster allows to choose between two PHYs */
>>>>>>> +#define NB_PHY 2
>>>>>>> +
>>>>>>> +enum {
>>>>>>> +	PHY_USB2 = 0,
>>>>>>> +	PHY_USB3 = 1,
>>>>>>> +};
>>>>>>> +
>>>>>>> +struct armada375_cluster_phy {
>>>>>>> +	struct phy *phy;
>>>>>>> +	void __iomem *reg;
>>>>>>> +	bool enable;
>>>>>>> +	bool use_usb3;
>>>>>>> +};
>>>>>>> +
>>>>>>> +struct armada375_cluster_phy usb_cluster_phy[NB_PHY];
>>>>>>> +
>>>>>>> +static int armada375_usb_phy_init(struct phy *phy)
>>>>>>> +{
>>>>>>> +	struct armada375_cluster_phy *cluster_phy = phy_get_drvdata(phy);
>>>>>>> +	u32 reg;
>>>>>>
>>>>>> This function should be protected since both your PHYs use this ops.
>>>>>
>>>>> Right
>>>>
>>>> Actually only one PHY can access this register. See the probe function,
>>>> cluster_phy->enable is only set to true for one PHY.
>>>>
>>>>>
>>>>>>> +
>>>>>>> +	if (!cluster_phy->enable)
>>>>>>> +		return -ENODEV;
>>>>>>> +
>>>>>>> +	reg = readl(cluster_phy->reg);
>>>>>>> +	if (cluster_phy->use_usb3)
>>>>>>> +		reg |= USB2_PHY_CONFIG_DISABLE;
>>>>>>> +	else
>>>>>>> +		reg &= ~USB2_PHY_CONFIG_DISABLE;
>>>>>>> +	writel(reg, cluster_phy->reg);
>>>>>>
>>>>>> This is confusing since both your PHYs control the same bit?
>>>>
>>>> Same here at the end the bit is accessed by only one PHY.
>>>>
>>>>>>> +
>>>>>>> +	return 0;
>>>>>>> +}
>>>>>>> +
>>>>>>> +static struct phy_ops armada375_usb_phy_ops = {
>>>>>>> +	.init = armada375_usb_phy_init,
>>>>>>> +	.owner		= THIS_MODULE,
>>>>>>> +};
>>>>>>> +
>>>>>>> +static struct phy *armada375_usb_phy_xlate(struct device *dev,
>>>>>>> +					struct of_phandle_args *args)
>>>>>>> +{
>>>>>>> +	if (WARN_ON(args->args[0] >= NB_PHY))
>>>>>>> +		return ERR_PTR(-ENODEV);
>>>>>>> +
>>>>>>> +	return usb_cluster_phy[args->args[0]].phy;
>>>>>>> +}
>>>>>>> +
>>>>>>> +static int armada375_usb_phy_probe(struct platform_device *pdev)
>>>>>>> +{
>>>>>>> +	struct device *dev = &pdev->dev;
>>>>>>> +	struct phy *phy;
>>>>>>> +	struct phy_provider *phy_provider;
>>>>>>> +	void __iomem *usb_cluster_base;
>>>>>>> +	struct device_node *xhci_node;
>>>>>>> +	struct resource *res;
>>>>>>> +	int i;
>>>>>>> +
>>>>>>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>>>>> +	usb_cluster_base = devm_ioremap_resource(&pdev->dev, res);
>>>>>>> +	if (!usb_cluster_base)
>>>>>>> +		return -ENOMEM;
>>>>>>> +
>>>>>>> +	for (i = 0; i < NB_PHY; i++) {
>>>>>>
>>>>>> For devices which have multiple PHYs, each PHY should be modelled as the
>>>>>> sub-node of the *PHY provider* device node.
>>>>>
>>>>> Actually it is the opposite the same PHY is shared between the EHCI
>>>>> and the xHCI controllers. It is more a PHY muxer than a PHY itself.
>>>>>
>>>>> I had to create 2 logical PHYs because once the phy_init() is called
>>>>> by a USB driver then the .init ops is not called anymore by the next
>>>>> call to phy_init(). One of the goal of this is to disable a port for
>>>>> the USB controller which can't use it due to the configuration of the
>>>>> USB cluster.
>>>>>
>>>>> But I can see how to make this two "pseudo" PHYs sub-node of the *PHY
>>>>> provider* device node. It shouldn't change the internal logic of this
>>>>> driver.
>>>>
>>>> I need to make a distinction when the PHY access by the xHCI or when
>>>> it was access by the EHCI. If I create two new sub-node then I will
>>>> also need to add a property to make this distinction. It seems a little
>>>> overkill for the need.
>>>
>>> Alright, so you have a single PHY that can be used by either XHCI or EHCI? And
>>> the use of PHY is mutually exclusive? How should it behave if you have both
>>> XHCI and EHCI?
>>
>> if we have both XHCI and EHCI then it is the USB2_PHY_CONFIG_DISABLE which
>> determine which one is used. By default we decide to select the XHCI.
>>
>>>
>>> One way to configure the PHY to a particular mode is by passing it as phandle
>>> arguments. I think you can use that to enable or disable USB2_PHY_CONFIG_DISABLE?
>>
>> actually it was more or less what I do:
>> for the XHCI I use:
>> phys = <&usbcluster 1>;
>> which enable the USB2_PHY_CONFIG_DISABLE
>>
>> for the EHCI I use phys = <&usbcluster 0>;
>> which disable the USB2_PHY_CONFIG_DISABLE
>>
>> If I had to create two PHY it was because of the behavior of
>> phy_init(). I need to be able to disable a controller if it can't use
>> the PHY. For this purpose my ops->init() exits in error. However
>> phy_init() will call ops->init() only one time, then the internal
>> counter init_count will be incremented, and the next call to phy_init
>> will skip the call to ops->init. And the behavior is the same for
>> phy_power_on().
>>
>> So given this I don't see how to do in an other way except by
>> modifying the value of the counter in my ops.
> 
> What do you prefer here? Keep the current implementation or using
> only one PHY by passing a argument through the phandle and in the
> same time hacking the init_count?

I clearly don't prefer having of_find_compatible_node in PHY driver code. If
you can find something without using that, it would be good.

Maybe create an API in phy-core that invokes the call-backs without caring
about the ref count and implement some sort of mutual exclusivity in your
driver? However this needs to be reviewed and discussed.

Thanks
Kishon
--
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: [PATCHv5 10/20] phy: add support for USB cluster on the Armada 375 SoC

[Gregory CLEMENT]

Hi Kishon,

On 15/05/2014 11:01, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Thursday 15 May 2014 12:31 PM, Gregory CLEMENT wrote:
>> Hi Kishon,
>>
>> On 14/05/2014 17:35, Gregory CLEMENT wrote:
>>> On 14/05/2014 16:27, Kishon Vijay Abraham I wrote:
>>>> Hi,
>>>>
>>>> On Tuesday 13 May 2014 03:11 PM, Gregory CLEMENT wrote:
>>>>> On 13/05/2014 10:06, Gregory CLEMENT wrote:
>>>>>> On 13/05/2014 07:53, Kishon Vijay Abraham I wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On Sunday 11 May 2014 11:47 PM, Thomas Petazzoni wrote:
>>>>>>>> From: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>>>>>>
>>>>>>>> The Armada 375 SoC comes with an USB2 host and device controller and
>>>>>>>> an USB3 controller. The USB cluster control register allows to manage
>>>>>>>> common features of both USB controllers.
>>>>>>>>
>>>>>>>> This commit adds a driver integrated in the generic PHY framework to
>>>>>>>> control this USB cluster feature.
>>>>>>>>
>>>>>>>> Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>>>>>> Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>>>>>> ---
>>>>>>>>  drivers/phy/Kconfig              |   6 ++
>>>>>>>>  drivers/phy/Makefile             |   1 +
>>>>>>>>  drivers/phy/phy-armada375-usb2.c | 157 +++++++++++++++++++++++++++++++++++++++
>>>>>>>>  3 files changed, 164 insertions(+)
>>>>>>>>  create mode 100644 drivers/phy/phy-armada375-usb2.c
>>>>>>>>
>>>>>>>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>>>>>>>> index 3bb05f1..e63cf9d 100644
>>>>>>>> --- a/drivers/phy/Kconfig
>>>>>>>> +++ b/drivers/phy/Kconfig
>>>>>>>> @@ -15,6 +15,12 @@ config GENERIC_PHY
>>>>>>>>  	  phy users can obtain reference to the PHY. All the users of this
>>>>>>>>  	  framework should select this config.
>>>>>>>>  
>>>>>>>> +config ARMADA375_USBCLUSTER_PHY
>>>>>>>> +	def_bool y
>>>>>>>> +	depends on MACH_ARMADA_375 || COMPILE_TEST
>>>>>>>> +	depends on OF
>>>>>>>> +	select GENERIC_PHY
>>>>>>>> +
>>>>>>>>  config PHY_EXYNOS_MIPI_VIDEO
>>>>>>>>  	tristate "S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver"
>>>>>>>>  	depends on HAS_IOMEM
>>>>>>>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>>>>>>>> index 2faf78e..47d5a86 100644
>>>>>>>> --- a/drivers/phy/Makefile
>>>>>>>> +++ b/drivers/phy/Makefile
>>>>>>>> @@ -3,6 +3,7 @@
>>>>>>>>  #
>>>>>>>>  
>>>>>>>>  obj-$(CONFIG_GENERIC_PHY)		+= phy-core.o
>>>>>>>> +obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY)	+= phy-armada375-usb2.o
>>>>>>>>  obj-$(CONFIG_BCM_KONA_USB2_PHY)		+= phy-bcm-kona-usb2.o
>>>>>>>>  obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)	+= phy-exynos-dp-video.o
>>>>>>>>  obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
>>>>>>>> diff --git a/drivers/phy/phy-armada375-usb2.c b/drivers/phy/phy-armada375-usb2.c
>>>>>>>> new file mode 100644
>>>>>>>> index 0000000..a6f746d
>>>>>>>> --- /dev/null
>>>>>>>> +++ b/drivers/phy/phy-armada375-usb2.c
>>>>>>>> @@ -0,0 +1,157 @@
>>>>>>>> +/*
>>>>>>>> + * USB cluster support for Armada 375 platform.
>>>>>>>> + *
>>>>>>>> + * Copyright (C) 2014 Marvell
>>>>>>>> + *
>>>>>>>> + * Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>>>>>> + *
>>>>>>>> + * This file is licensed under the terms of the GNU General Public
>>>>>>>> + * License version 2 or later. This program is licensed "as is"
>>>>>>>> + * without any warranty of any kind, whether express or implied.
>>>>>>>> + *
>>>>>>>> + * Armada 375 comes with an USB2 host and device controller and an
>>>>>>>> + * USB3 controller. The USB cluster control register allows to manage
>>>>>>>> + * common features of both USB controllers.
>>>>>>>> + */
>>>>>>>> +
>>>>>>>> +#include <linux/init.h>
>>>>>>>> +#include <linux/io.h>
>>>>>>>> +#include <linux/kernel.h>
>>>>>>>> +#include <linux/module.h>
>>>>>>>> +#include <linux/of_address.h>
>>>>>>>> +#include <linux/phy/phy.h>
>>>>>>>> +#include <linux/platform_device.h>
>>>>>>>> +#include <linux/slab.h>
>>>>>>>> +
>>>>>>>> +#define USB2_PHY_CONFIG_DISABLE BIT(0)
>>>>>>>> +
>>>>>>>> +/* The USB cluster allows to choose between two PHYs */
>>>>>>>> +#define NB_PHY 2
>>>>>>>> +
>>>>>>>> +enum {
>>>>>>>> +	PHY_USB2 = 0,
>>>>>>>> +	PHY_USB3 = 1,
>>>>>>>> +};
>>>>>>>> +
>>>>>>>> +struct armada375_cluster_phy {
>>>>>>>> +	struct phy *phy;
>>>>>>>> +	void __iomem *reg;
>>>>>>>> +	bool enable;
>>>>>>>> +	bool use_usb3;
>>>>>>>> +};
>>>>>>>> +
>>>>>>>> +struct armada375_cluster_phy usb_cluster_phy[NB_PHY];
>>>>>>>> +
>>>>>>>> +static int armada375_usb_phy_init(struct phy *phy)
>>>>>>>> +{
>>>>>>>> +	struct armada375_cluster_phy *cluster_phy = phy_get_drvdata(phy);
>>>>>>>> +	u32 reg;
>>>>>>>
>>>>>>> This function should be protected since both your PHYs use this ops.
>>>>>>
>>>>>> Right
>>>>>
>>>>> Actually only one PHY can access this register. See the probe function,
>>>>> cluster_phy->enable is only set to true for one PHY.
>>>>>
>>>>>>
>>>>>>>> +
>>>>>>>> +	if (!cluster_phy->enable)
>>>>>>>> +		return -ENODEV;
>>>>>>>> +
>>>>>>>> +	reg = readl(cluster_phy->reg);
>>>>>>>> +	if (cluster_phy->use_usb3)
>>>>>>>> +		reg |= USB2_PHY_CONFIG_DISABLE;
>>>>>>>> +	else
>>>>>>>> +		reg &= ~USB2_PHY_CONFIG_DISABLE;
>>>>>>>> +	writel(reg, cluster_phy->reg);
>>>>>>>
>>>>>>> This is confusing since both your PHYs control the same bit?
>>>>>
>>>>> Same here at the end the bit is accessed by only one PHY.
>>>>>
>>>>>>>> +
>>>>>>>> +	return 0;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +static struct phy_ops armada375_usb_phy_ops = {
>>>>>>>> +	.init = armada375_usb_phy_init,
>>>>>>>> +	.owner		= THIS_MODULE,
>>>>>>>> +};
>>>>>>>> +
>>>>>>>> +static struct phy *armada375_usb_phy_xlate(struct device *dev,
>>>>>>>> +					struct of_phandle_args *args)
>>>>>>>> +{
>>>>>>>> +	if (WARN_ON(args->args[0] >= NB_PHY))
>>>>>>>> +		return ERR_PTR(-ENODEV);
>>>>>>>> +
>>>>>>>> +	return usb_cluster_phy[args->args[0]].phy;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +static int armada375_usb_phy_probe(struct platform_device *pdev)
>>>>>>>> +{
>>>>>>>> +	struct device *dev = &pdev->dev;
>>>>>>>> +	struct phy *phy;
>>>>>>>> +	struct phy_provider *phy_provider;
>>>>>>>> +	void __iomem *usb_cluster_base;
>>>>>>>> +	struct device_node *xhci_node;
>>>>>>>> +	struct resource *res;
>>>>>>>> +	int i;
>>>>>>>> +
>>>>>>>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>>>>>> +	usb_cluster_base = devm_ioremap_resource(&pdev->dev, res);
>>>>>>>> +	if (!usb_cluster_base)
>>>>>>>> +		return -ENOMEM;
>>>>>>>> +
>>>>>>>> +	for (i = 0; i < NB_PHY; i++) {
>>>>>>>
>>>>>>> For devices which have multiple PHYs, each PHY should be modelled as the
>>>>>>> sub-node of the *PHY provider* device node.
>>>>>>
>>>>>> Actually it is the opposite the same PHY is shared between the EHCI
>>>>>> and the xHCI controllers. It is more a PHY muxer than a PHY itself.
>>>>>>
>>>>>> I had to create 2 logical PHYs because once the phy_init() is called
>>>>>> by a USB driver then the .init ops is not called anymore by the next
>>>>>> call to phy_init(). One of the goal of this is to disable a port for
>>>>>> the USB controller which can't use it due to the configuration of the
>>>>>> USB cluster.
>>>>>>
>>>>>> But I can see how to make this two "pseudo" PHYs sub-node of the *PHY
>>>>>> provider* device node. It shouldn't change the internal logic of this
>>>>>> driver.
>>>>>
>>>>> I need to make a distinction when the PHY access by the xHCI or when
>>>>> it was access by the EHCI. If I create two new sub-node then I will
>>>>> also need to add a property to make this distinction. It seems a little
>>>>> overkill for the need.
>>>>
>>>> Alright, so you have a single PHY that can be used by either XHCI or EHCI? And
>>>> the use of PHY is mutually exclusive? How should it behave if you have both
>>>> XHCI and EHCI?
>>>
>>> if we have both XHCI and EHCI then it is the USB2_PHY_CONFIG_DISABLE which
>>> determine which one is used. By default we decide to select the XHCI.
>>>
>>>>
>>>> One way to configure the PHY to a particular mode is by passing it as phandle
>>>> arguments. I think you can use that to enable or disable USB2_PHY_CONFIG_DISABLE?
>>>
>>> actually it was more or less what I do:
>>> for the XHCI I use:
>>> phys = <&usbcluster 1>;
>>> which enable the USB2_PHY_CONFIG_DISABLE
>>>
>>> for the EHCI I use phys = <&usbcluster 0>;
>>> which disable the USB2_PHY_CONFIG_DISABLE
>>>
>>> If I had to create two PHY it was because of the behavior of
>>> phy_init(). I need to be able to disable a controller if it can't use
>>> the PHY. For this purpose my ops->init() exits in error. However
>>> phy_init() will call ops->init() only one time, then the internal
>>> counter init_count will be incremented, and the next call to phy_init
>>> will skip the call to ops->init. And the behavior is the same for
>>> phy_power_on().
>>>
>>> So given this I don't see how to do in an other way except by
>>> modifying the value of the counter in my ops.
>>
>> What do you prefer here? Keep the current implementation or using
>> only one PHY by passing a argument through the phandle and in the
>> same time hacking the init_count?
> 
> I clearly don't prefer having of_find_compatible_node in PHY driver code. If
> you can find something without using that, it would be good.
> 
> Maybe create an API in phy-core that invokes the call-backs without caring
> about the ref count and implement some sort of mutual exclusivity in your
> driver? However this needs to be reviewed and discussed.

Thanks for your feedback. This feature (usb cluster) is a nice to have
for the Armada 375 SoC, but is not mandatory for the USB support on
this SoC.

As I don't want to postpone the merge of the USB support for this. I
will resend a new patch series, without the usb-cluster support, to be
merged in 3.16 kernel. And in the same time I will submit a new series
with an extension of the phy-core API. if everything goes well, then
it will be also merged in 3.16, else it will be in the next release.


Thanks again for your time,

Gregory


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
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: [PATCHv5 10/20] phy: add support for USB cluster on the Armada 375 SoC

[Kishon Vijay Abraham I]

On Thursday 15 May 2014 03:05 PM, Gregory CLEMENT wrote:
> Hi Kishon,
> 
> On 15/05/2014 11:01, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Thursday 15 May 2014 12:31 PM, Gregory CLEMENT wrote:
>>> Hi Kishon,
>>>
>>> On 14/05/2014 17:35, Gregory CLEMENT wrote:
>>>> On 14/05/2014 16:27, Kishon Vijay Abraham I wrote:
>>>>> Hi,
>>>>>
>>>>> On Tuesday 13 May 2014 03:11 PM, Gregory CLEMENT wrote:
>>>>>> On 13/05/2014 10:06, Gregory CLEMENT wrote:
>>>>>>> On 13/05/2014 07:53, Kishon Vijay Abraham I wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> On Sunday 11 May 2014 11:47 PM, Thomas Petazzoni wrote:
>>>>>>>>> From: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>>>>>>>
>>>>>>>>> The Armada 375 SoC comes with an USB2 host and device controller and
>>>>>>>>> an USB3 controller. The USB cluster control register allows to manage
>>>>>>>>> common features of both USB controllers.
>>>>>>>>>
>>>>>>>>> This commit adds a driver integrated in the generic PHY framework to
>>>>>>>>> control this USB cluster feature.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>>>>>>> Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>>>>>>> ---
>>>>>>>>>  drivers/phy/Kconfig              |   6 ++
>>>>>>>>>  drivers/phy/Makefile             |   1 +
>>>>>>>>>  drivers/phy/phy-armada375-usb2.c | 157 +++++++++++++++++++++++++++++++++++++++
>>>>>>>>>  3 files changed, 164 insertions(+)
>>>>>>>>>  create mode 100644 drivers/phy/phy-armada375-usb2.c
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>>>>>>>>> index 3bb05f1..e63cf9d 100644
>>>>>>>>> --- a/drivers/phy/Kconfig
>>>>>>>>> +++ b/drivers/phy/Kconfig
>>>>>>>>> @@ -15,6 +15,12 @@ config GENERIC_PHY
>>>>>>>>>  	  phy users can obtain reference to the PHY. All the users of this
>>>>>>>>>  	  framework should select this config.
>>>>>>>>>  
>>>>>>>>> +config ARMADA375_USBCLUSTER_PHY
>>>>>>>>> +	def_bool y
>>>>>>>>> +	depends on MACH_ARMADA_375 || COMPILE_TEST
>>>>>>>>> +	depends on OF
>>>>>>>>> +	select GENERIC_PHY
>>>>>>>>> +
>>>>>>>>>  config PHY_EXYNOS_MIPI_VIDEO
>>>>>>>>>  	tristate "S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver"
>>>>>>>>>  	depends on HAS_IOMEM
>>>>>>>>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>>>>>>>>> index 2faf78e..47d5a86 100644
>>>>>>>>> --- a/drivers/phy/Makefile
>>>>>>>>> +++ b/drivers/phy/Makefile
>>>>>>>>> @@ -3,6 +3,7 @@
>>>>>>>>>  #
>>>>>>>>>  
>>>>>>>>>  obj-$(CONFIG_GENERIC_PHY)		+= phy-core.o
>>>>>>>>> +obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY)	+= phy-armada375-usb2.o
>>>>>>>>>  obj-$(CONFIG_BCM_KONA_USB2_PHY)		+= phy-bcm-kona-usb2.o
>>>>>>>>>  obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)	+= phy-exynos-dp-video.o
>>>>>>>>>  obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
>>>>>>>>> diff --git a/drivers/phy/phy-armada375-usb2.c b/drivers/phy/phy-armada375-usb2.c
>>>>>>>>> new file mode 100644
>>>>>>>>> index 0000000..a6f746d
>>>>>>>>> --- /dev/null
>>>>>>>>> +++ b/drivers/phy/phy-armada375-usb2.c
>>>>>>>>> @@ -0,0 +1,157 @@
>>>>>>>>> +/*
>>>>>>>>> + * USB cluster support for Armada 375 platform.
>>>>>>>>> + *
>>>>>>>>> + * Copyright (C) 2014 Marvell
>>>>>>>>> + *
>>>>>>>>> + * Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>>>>>>> + *
>>>>>>>>> + * This file is licensed under the terms of the GNU General Public
>>>>>>>>> + * License version 2 or later. This program is licensed "as is"
>>>>>>>>> + * without any warranty of any kind, whether express or implied.
>>>>>>>>> + *
>>>>>>>>> + * Armada 375 comes with an USB2 host and device controller and an
>>>>>>>>> + * USB3 controller. The USB cluster control register allows to manage
>>>>>>>>> + * common features of both USB controllers.
>>>>>>>>> + */
>>>>>>>>> +
>>>>>>>>> +#include <linux/init.h>
>>>>>>>>> +#include <linux/io.h>
>>>>>>>>> +#include <linux/kernel.h>
>>>>>>>>> +#include <linux/module.h>
>>>>>>>>> +#include <linux/of_address.h>
>>>>>>>>> +#include <linux/phy/phy.h>
>>>>>>>>> +#include <linux/platform_device.h>
>>>>>>>>> +#include <linux/slab.h>
>>>>>>>>> +
>>>>>>>>> +#define USB2_PHY_CONFIG_DISABLE BIT(0)
>>>>>>>>> +
>>>>>>>>> +/* The USB cluster allows to choose between two PHYs */
>>>>>>>>> +#define NB_PHY 2
>>>>>>>>> +
>>>>>>>>> +enum {
>>>>>>>>> +	PHY_USB2 = 0,
>>>>>>>>> +	PHY_USB3 = 1,
>>>>>>>>> +};
>>>>>>>>> +
>>>>>>>>> +struct armada375_cluster_phy {
>>>>>>>>> +	struct phy *phy;
>>>>>>>>> +	void __iomem *reg;
>>>>>>>>> +	bool enable;
>>>>>>>>> +	bool use_usb3;
>>>>>>>>> +};
>>>>>>>>> +
>>>>>>>>> +struct armada375_cluster_phy usb_cluster_phy[NB_PHY];
>>>>>>>>> +
>>>>>>>>> +static int armada375_usb_phy_init(struct phy *phy)
>>>>>>>>> +{
>>>>>>>>> +	struct armada375_cluster_phy *cluster_phy = phy_get_drvdata(phy);
>>>>>>>>> +	u32 reg;
>>>>>>>>
>>>>>>>> This function should be protected since both your PHYs use this ops.
>>>>>>>
>>>>>>> Right
>>>>>>
>>>>>> Actually only one PHY can access this register. See the probe function,
>>>>>> cluster_phy->enable is only set to true for one PHY.
>>>>>>
>>>>>>>
>>>>>>>>> +
>>>>>>>>> +	if (!cluster_phy->enable)
>>>>>>>>> +		return -ENODEV;
>>>>>>>>> +
>>>>>>>>> +	reg = readl(cluster_phy->reg);
>>>>>>>>> +	if (cluster_phy->use_usb3)
>>>>>>>>> +		reg |= USB2_PHY_CONFIG_DISABLE;
>>>>>>>>> +	else
>>>>>>>>> +		reg &= ~USB2_PHY_CONFIG_DISABLE;
>>>>>>>>> +	writel(reg, cluster_phy->reg);
>>>>>>>>
>>>>>>>> This is confusing since both your PHYs control the same bit?
>>>>>>
>>>>>> Same here at the end the bit is accessed by only one PHY.
>>>>>>
>>>>>>>>> +
>>>>>>>>> +	return 0;
>>>>>>>>> +}
>>>>>>>>> +
>>>>>>>>> +static struct phy_ops armada375_usb_phy_ops = {
>>>>>>>>> +	.init = armada375_usb_phy_init,
>>>>>>>>> +	.owner		= THIS_MODULE,
>>>>>>>>> +};
>>>>>>>>> +
>>>>>>>>> +static struct phy *armada375_usb_phy_xlate(struct device *dev,
>>>>>>>>> +					struct of_phandle_args *args)
>>>>>>>>> +{
>>>>>>>>> +	if (WARN_ON(args->args[0] >= NB_PHY))
>>>>>>>>> +		return ERR_PTR(-ENODEV);
>>>>>>>>> +
>>>>>>>>> +	return usb_cluster_phy[args->args[0]].phy;
>>>>>>>>> +}
>>>>>>>>> +
>>>>>>>>> +static int armada375_usb_phy_probe(struct platform_device *pdev)
>>>>>>>>> +{
>>>>>>>>> +	struct device *dev = &pdev->dev;
>>>>>>>>> +	struct phy *phy;
>>>>>>>>> +	struct phy_provider *phy_provider;
>>>>>>>>> +	void __iomem *usb_cluster_base;
>>>>>>>>> +	struct device_node *xhci_node;
>>>>>>>>> +	struct resource *res;
>>>>>>>>> +	int i;
>>>>>>>>> +
>>>>>>>>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>>>>>>> +	usb_cluster_base = devm_ioremap_resource(&pdev->dev, res);
>>>>>>>>> +	if (!usb_cluster_base)
>>>>>>>>> +		return -ENOMEM;
>>>>>>>>> +
>>>>>>>>> +	for (i = 0; i < NB_PHY; i++) {
>>>>>>>>
>>>>>>>> For devices which have multiple PHYs, each PHY should be modelled as the
>>>>>>>> sub-node of the *PHY provider* device node.
>>>>>>>
>>>>>>> Actually it is the opposite the same PHY is shared between the EHCI
>>>>>>> and the xHCI controllers. It is more a PHY muxer than a PHY itself.
>>>>>>>
>>>>>>> I had to create 2 logical PHYs because once the phy_init() is called
>>>>>>> by a USB driver then the .init ops is not called anymore by the next
>>>>>>> call to phy_init(). One of the goal of this is to disable a port for
>>>>>>> the USB controller which can't use it due to the configuration of the
>>>>>>> USB cluster.
>>>>>>>
>>>>>>> But I can see how to make this two "pseudo" PHYs sub-node of the *PHY
>>>>>>> provider* device node. It shouldn't change the internal logic of this
>>>>>>> driver.
>>>>>>
>>>>>> I need to make a distinction when the PHY access by the xHCI or when
>>>>>> it was access by the EHCI. If I create two new sub-node then I will
>>>>>> also need to add a property to make this distinction. It seems a little
>>>>>> overkill for the need.
>>>>>
>>>>> Alright, so you have a single PHY that can be used by either XHCI or EHCI? And
>>>>> the use of PHY is mutually exclusive? How should it behave if you have both
>>>>> XHCI and EHCI?
>>>>
>>>> if we have both XHCI and EHCI then it is the USB2_PHY_CONFIG_DISABLE which
>>>> determine which one is used. By default we decide to select the XHCI.
>>>>
>>>>>
>>>>> One way to configure the PHY to a particular mode is by passing it as phandle
>>>>> arguments. I think you can use that to enable or disable USB2_PHY_CONFIG_DISABLE?
>>>>
>>>> actually it was more or less what I do:
>>>> for the XHCI I use:
>>>> phys = <&usbcluster 1>;
>>>> which enable the USB2_PHY_CONFIG_DISABLE
>>>>
>>>> for the EHCI I use phys = <&usbcluster 0>;
>>>> which disable the USB2_PHY_CONFIG_DISABLE
>>>>
>>>> If I had to create two PHY it was because of the behavior of
>>>> phy_init(). I need to be able to disable a controller if it can't use
>>>> the PHY. For this purpose my ops->init() exits in error. However
>>>> phy_init() will call ops->init() only one time, then the internal
>>>> counter init_count will be incremented, and the next call to phy_init
>>>> will skip the call to ops->init. And the behavior is the same for
>>>> phy_power_on().
>>>>
>>>> So given this I don't see how to do in an other way except by
>>>> modifying the value of the counter in my ops.
>>>
>>> What do you prefer here? Keep the current implementation or using
>>> only one PHY by passing a argument through the phandle and in the
>>> same time hacking the init_count?
>>
>> I clearly don't prefer having of_find_compatible_node in PHY driver code. If
>> you can find something without using that, it would be good.
>>
>> Maybe create an API in phy-core that invokes the call-backs without caring
>> about the ref count and implement some sort of mutual exclusivity in your
>> driver? However this needs to be reviewed and discussed.
> 
> Thanks for your feedback. This feature (usb cluster) is a nice to have
> for the Armada 375 SoC, but is not mandatory for the USB support on
> this SoC.
> 
> As I don't want to postpone the merge of the USB support for this. I
> will resend a new patch series, without the usb-cluster support, to be
> merged in 3.16 kernel. And in the same time I will submit a new series
> with an extension of the phy-core API. if everything goes well, then
> it will be also merged in 3.16, else it will be in the next release.

Thanks for doing that.
> 
> 
> Thanks again for your time,

No problem.

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

[PATCHv5 11/20] Documentation: dt-bindings: document the Armada 375 USB cluster binding

[Thomas Petazzoni]

From: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

Armada 375 comes with an USB2 host and device controller and an USB3
controller. The USB cluster control register allows to manage common
features of both USB controllers. This commit adds the Device Tree
binding documentation for this piece of hardware.

Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 .../bindings/phy/armada-375-usb-phy-cluster.txt       | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/armada-375-usb-phy-cluster.txt

diff --git a/Documentation/devicetree/bindings/phy/armada-375-usb-phy-cluster.txt b/Documentation/devicetree/bindings/phy/armada-375-usb-phy-cluster.txt
new file mode 100644
index 0000000..258407c
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/armada-375-usb-phy-cluster.txt
@@ -0,0 +1,19 @@
+Armada 375 USB cluster
+----------------------
+
+Armada 375 comes with an USB2 host and device controller and an USB3
+controller. The USB cluster control register allows to manage common
+features of both USB controllers.
+
+Required properties:
+
+- compatible: "marvell,armada-375-usb-cluster"
+- reg: Should contain usb cluster register location and length.
+- #phy-cells : from the generic phy bindings, must be 1
+
+Example:
+	usbcluster: usb-cluster@18400 {
+		compatible = "marvell,armada-375-usb-cluster";
+		reg = <0x18400 0x4>;
+		#phy-cells = <1>
+	};
-- 
1.9.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

[PATCHv5 12/20] ARM: mvebu: add USB3 support for Armada 38x

[Thomas Petazzoni]

From: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

This patch adds the selection of the config symbol needed to build the
USB3 support for Armada 38x into mvebu_v7_defconfig.

Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 arch/arm/mach-mvebu/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
index 3f73eec..7960f21 100644
--- a/arch/arm/mach-mvebu/Kconfig
+++ b/arch/arm/mach-mvebu/Kconfig
@@ -54,6 +54,7 @@ config MACH_ARMADA_38X
 	select CPU_V7
 	select MACH_MVEBU_V7
 	select PINCTRL_ARMADA_38X
+	select USB_ARCH_HAS_XHCI
 	help
 	  Say 'Y' here if you want your kernel to support boards based
 	  on the Marvell Armada 380/385 SoC with device tree.
-- 
1.9.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

[PATCHv5 13/20] ARM: mvebu: add USB3 support for Armada 375

[Thomas Petazzoni]

From: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

This patch add the selection of the config symbol to build the USB3
support for Armada 375.

Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 arch/arm/mach-mvebu/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
index 7960f21..95afc76 100644
--- a/arch/arm/mach-mvebu/Kconfig
+++ b/arch/arm/mach-mvebu/Kconfig
@@ -41,6 +41,7 @@ config MACH_ARMADA_375
 	select CPU_V7
 	select MACH_MVEBU_V7
 	select PINCTRL_ARMADA_375
+	select USB_ARCH_HAS_XHCI
 	help
 	  Say 'Y' here if you want your kernel to support boards based
 	  on the Marvell Armada 375 SoC with device tree.
-- 
1.9.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

[PATCHv5 14/20] ARM: configs: enable XHCI mvebu support in mvebu_v7_defconfig

[Thomas Petazzoni]

From: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

The Marvell Armada 38x platform needs the xhci_mvebu driver enabled
for the xHCI USB hosts, so this commit enables the corresponding
Kconfig option in mvebu_v7_defconfig.

Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 arch/arm/configs/mvebu_v7_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/mvebu_v7_defconfig b/arch/arm/configs/mvebu_v7_defconfig
index a34713d..e881106 100644
--- a/arch/arm/configs/mvebu_v7_defconfig
+++ b/arch/arm/configs/mvebu_v7_defconfig
@@ -78,6 +78,7 @@ CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_EHCI_ROOT_HUB_TT=y
 CONFIG_USB_STORAGE=y
 CONFIG_USB_XHCI_HCD=y
+CONFIG_USB_XHCI_MVEBU=y
 CONFIG_MMC=y
 CONFIG_MMC_MVSDIO=y
 CONFIG_NEW_LEDS=y
-- 
1.9.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

[PATCHv5 15/20] ARM: configs: enable XHCI mvebu support in multi_v7_defconfig

[Thomas Petazzoni]

From: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

The Marvell Armada 38x platform needs the xhci_mvebu driver enabled
for the xHCI USB hosts, so this commit enables the corresponding
Kconfig option in multi_v7_defconfig.

Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Cc: arm-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Cc: Kevin Hilman <khilman-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>
Cc: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
---
 arch/arm/configs/multi_v7_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index d4e8a47..820cc35 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -254,6 +254,7 @@ CONFIG_SND_SOC_TEGRA_ALC5632=y
 CONFIG_SND_SOC_TEGRA_MAX98090=y
 CONFIG_USB=y
 CONFIG_USB_XHCI_HCD=y
+CONFIG_USB_XHCI_MVEBU=y
 CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_EHCI_TEGRA=y
 CONFIG_USB_EHCI_HCD_PLATFORM=y
-- 
1.9.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

[PATCHv5 16/20] ARM: mvebu: add Device Tree description of xHCI controllers on Armada 38x

[Thomas Petazzoni]

From: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

The Marvell Armada 38x SoCs contains two xHCI controllers. This commit
adds the Device Tree description of those interfaces at the SoC level,
and also enables the two USB3 ports on the Armada 385 DB platform and
one USB3 port on the Armada 385 RD platform.

Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 arch/arm/boot/dts/armada-385-db.dts |  8 ++++++++
 arch/arm/boot/dts/armada-385-rd.dts |  4 ++++
 arch/arm/boot/dts/armada-38x.dtsi   | 17 +++++++++++++++++
 3 files changed, 29 insertions(+)

diff --git a/arch/arm/boot/dts/armada-385-db.dts b/arch/arm/boot/dts/armada-385-db.dts
index 6828d77..d5db146 100644
--- a/arch/arm/boot/dts/armada-385-db.dts
+++ b/arch/arm/boot/dts/armada-385-db.dts
@@ -101,6 +101,14 @@
 					reg = <0x1000000 0x3f000000>;
 				};
 			};
+
+			usb3@f0000 {
+				status = "okay";
+			};
+
+			usb3@f8000 {
+				status = "okay";
+			};
 		};
 
 		pcie-controller {
diff --git a/arch/arm/boot/dts/armada-385-rd.dts b/arch/arm/boot/dts/armada-385-rd.dts
index 45250c8..a505fe9 100644
--- a/arch/arm/boot/dts/armada-385-rd.dts
+++ b/arch/arm/boot/dts/armada-385-rd.dts
@@ -77,6 +77,10 @@
 					reg = <1>;
 				};
 			};
+
+			usb3@f0000 {
+				status = "okay";
+			};
 		};
 
 		pcie-controller {
diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi
index a064f59..6f97f3d 100644
--- a/arch/arm/boot/dts/armada-38x.dtsi
+++ b/arch/arm/boot/dts/armada-38x.dtsi
@@ -355,6 +355,23 @@
 				clocks = <&coredivclk 0>;
 				status = "disabled";
 			};
+
+			usb3@f0000 {
+				compatible = "marvell,armada-380-xhci";
+				reg = <0xf0000 0x4000>,<0xf4000 0x4000>;
+				interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&gateclk 9>;
+				status = "disabled";
+			};
+
+			usb3@f8000 {
+				compatible = "marvell,armada-380-xhci";
+				reg = <0xf8000 0x4000>,<0xfc000 0x4000>;
+				interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&gateclk 10>;
+				status = "disabled";
+			};
+
 		};
 	};
 
-- 
1.9.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

[PATCHv5 17/20] ARM: mvebu: add Device Tree description of the EHCI controller on Armada 38x

[Thomas Petazzoni]

From: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

The Marvell Armada 38x SoCs contains one EHCI controller. This commit
adds the Device Tree description of this interface at the SoC level,
and also enables the USB2 port on the Armada 385 DB platform.

Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 arch/arm/boot/dts/armada-385-db.dts | 4 ++++
 arch/arm/boot/dts/armada-38x.dtsi   | 8 ++++++++
 2 files changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/armada-385-db.dts b/arch/arm/boot/dts/armada-385-db.dts
index d5db146..91e3e44 100644
--- a/arch/arm/boot/dts/armada-385-db.dts
+++ b/arch/arm/boot/dts/armada-385-db.dts
@@ -65,6 +65,10 @@
 				phy-mode = "rgmii-id";
 			};
 
+			usb@50000 {
+				status = "ok";
+			};
+
 			ethernet@70000 {
 				status = "okay";
 				phy = <&phy0>;
diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi
index 6f97f3d..fa2501d 100644
--- a/arch/arm/boot/dts/armada-38x.dtsi
+++ b/arch/arm/boot/dts/armada-38x.dtsi
@@ -283,6 +283,14 @@
 				status = "disabled";
 			};
 
+			usb@50000 {
+				compatible = "marvell,orion-ehci";
+				reg = <0x58000 0x500>;
+				interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&gateclk 18>;
+				status = "disabled";
+			};
+
 			xor@60800 {
 				compatible = "marvell,orion-xor";
 				reg = <0x60800 0x100
-- 
1.9.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

[PATCHv5 18/20] ARM: mvebu: add Device Tree description of USB cluster controller on Armada 375

[Thomas Petazzoni]

From: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

On Armada 375, the USB cluster allows to control the cluster composed
of the USB2 and USB3 host controllers.

Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 arch/arm/boot/dts/armada-375.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/armada-375.dtsi b/arch/arm/boot/dts/armada-375.dtsi
index 3877693..6724c10 100644
--- a/arch/arm/boot/dts/armada-375.dtsi
+++ b/arch/arm/boot/dts/armada-375.dtsi
@@ -320,6 +320,12 @@
 				clocks = <&coreclk 0>;
 			};
 
+			usbcluster: usb-cluster@18400 {
+				compatible = "marvell,armada-375-usb-cluster";
+				reg = <0x18400 0x4>;
+				#phy-cells = <1>;
+			};
+
 			xor@60800 {
 				compatible = "marvell,orion-xor";
 				reg = <0x60800 0x100
-- 
1.9.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

[PATCHv5 19/20] ARM: mvebu: add Device Tree description of the xHCI controller on Armada 375

[Thomas Petazzoni]

From: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

The Marvell Armada 375 SoCs contain a xHCI controller. This commit
adds the Device Tree description of this interfaces at the SoC level,
and also enables the USB3 port on the Armada 375 DB platform.

Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 arch/arm/boot/dts/armada-375-db.dts |  4 ++++
 arch/arm/boot/dts/armada-375.dtsi   | 10 ++++++++++
 2 files changed, 14 insertions(+)

diff --git a/arch/arm/boot/dts/armada-375-db.dts b/arch/arm/boot/dts/armada-375-db.dts
index 9378d31..0453d69 100644
--- a/arch/arm/boot/dts/armada-375-db.dts
+++ b/arch/arm/boot/dts/armada-375-db.dts
@@ -102,6 +102,10 @@
 				};
 			};
 
+			usb3@58000 {
+				status = "okay";
+			};
+
 			mvsdio@d4000 {
 				pinctrl-0 = <&sdio_pins &sdio_st_pins>;
 				pinctrl-names = "default";
diff --git a/arch/arm/boot/dts/armada-375.dtsi b/arch/arm/boot/dts/armada-375.dtsi
index 6724c10..1b81d1e 100644
--- a/arch/arm/boot/dts/armada-375.dtsi
+++ b/arch/arm/boot/dts/armada-375.dtsi
@@ -320,6 +320,16 @@
 				clocks = <&coreclk 0>;
 			};
 
+			usb3@58000 {
+				compatible = "marvell,armada-375-xhci";
+				reg = <0x58000 0x20000>,<0x5b880 0x80>;
+				interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&gateclk 16>;
+				phys = <&usbcluster 1>;
+				phy-names="usb";
+				status = "disabled";
+			};
+
 			usbcluster: usb-cluster@18400 {
 				compatible = "marvell,armada-375-usb-cluster";
 				reg = <0x18400 0x4>;
-- 
1.9.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

[PATCHv5 20/20] ARM: mvebu: add Device Tree description of the EHCI controller on Armada 375

[Thomas Petazzoni]

From: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

The Marvell Armada 375 SoCs contains one EHCI controller. This commit
adds the Device Tree description of this interfaces at the SoC level,
and also enables the USB2 port on the Armada 375 DB platform.

Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 arch/arm/boot/dts/armada-375-db.dts |  4 ++++
 arch/arm/boot/dts/armada-375.dtsi   | 18 ++++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/arch/arm/boot/dts/armada-375-db.dts b/arch/arm/boot/dts/armada-375-db.dts
index 0453d69..01bc5e8 100644
--- a/arch/arm/boot/dts/armada-375-db.dts
+++ b/arch/arm/boot/dts/armada-375-db.dts
@@ -102,6 +102,10 @@
 				};
 			};
 
+			usb@54000 {
+				status = "okay";
+			};
+
 			usb3@58000 {
 				status = "okay";
 			};
diff --git a/arch/arm/boot/dts/armada-375.dtsi b/arch/arm/boot/dts/armada-375.dtsi
index 1b81d1e..945df89 100644
--- a/arch/arm/boot/dts/armada-375.dtsi
+++ b/arch/arm/boot/dts/armada-375.dtsi
@@ -320,6 +320,24 @@
 				clocks = <&coreclk 0>;
 			};
 
+			usb@50000 {
+				compatible = "marvell,orion-ehci";
+				reg = <0x50000 0x500>;
+				interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&gateclk 18>;
+				phys = <&usbcluster 0>;
+				phy-names = "usb";
+				status = "disabled";
+			};
+
+			usb@54000 {
+				compatible = "marvell,orion-ehci";
+				reg = <0x54000 0x500>;
+				interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&gateclk 26>;
+				status = "disabled";
+			};
+
 			usb3@58000 {
 				compatible = "marvell,armada-375-xhci";
 				reg = <0x58000 0x20000>,<0x5b880 0x80>;
-- 
1.9.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: [PATCHv5 00/20] USB support for Armada 38x and Armada 375

[Gregory CLEMENT]

On 11/05/2014 20:17, Thomas Petazzoni wrote:
> Hello,
> 
> This patch set adds the USB support for the Armada 38x and Armada 375
> SOCs. These SoCs use an xHCI but still need specific initialization,
> mainly to setup the MBus memory windows. They also have another USB
> controller for EHCI, identical to the one used on other mvebu SOCs.
> 
> This series is also available in
> the branch USB-375-38x-3.15-rc1-V5
> https://github.com/MISL-EBU-System-SW/mainline-public.git


Hi,

it seems that no this series is in a pretty good shape.

Jason could you take it in mvebu/for-next? I would like see in
linux-next for a few days before sending a formal pull request.

In the mean time I would like to know how we can handle the merge of
this series.

The 5 first patches modify the EHCI part of the USB subsystem, Alan
Stern gave his acked-by for the 4th first one, and as the 5th one is
just an update of the documentation, I think Alan could take them.

The others patches don't depends on it neither for building or for
running the kernel. Of course at the end to be able to use USB on the
Armada 375/38x platform we will need some of them, but if they are
missing, then there will be no USB support as in the current kernel,
so no regression will be introduce.

Then patches from 6 to 9 are related to the xHCI part of the USB
subsystem, Felipe Balbi gave his acked-by for the former version (I am
going ta ask him his formal acked-by for the new ones), however the
xHCI maintainer Mathias Nyman didn't comment them. I hope he agree to
take them. As for the EHCI part, the others patches don't depends on
it neither for building or for running the kernel.

Patches 10 and 11 related to the USB PHY part have been reviewed but
the PHY maintainers Kishon Vijay Abraham I and Felipe Balbi didn't
comment them. For Kishon Vijay Abraham I, it was our fault he wasn't
in copy of this email. Here again no dependencies for building or
running the kernel.

Finally patches 12 to 20 are mvebu related and the patch 15 should be
handle directly by arm-soc but there is nothing controversial in it,
they are here to allow to enable the use of USB on the Armada 375/38x
platform. Here again no dependencies for building or running the
kernel.


>From my point of view these 4 chunks can be merged independently, but
if there is a need to merge the series as a whole then it should be
done through the arm-soc tree because most of the merge conflict are
likely to happen in the dts and config file.

Thanks!

Gregory

> 
> Changes between v4 and v5:
> 
>  * Fixed a compile time problem when CONFIG_USB_XHCI_MVEBU was
>    disabled. Noticed by Shimoda, Yoshihiro
>    <yoshihiro.shimoda.uh-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>.
> 
>  * Added Acked-by from Alan Stern on patches 1 to 4.
> 
> Changes between v3 and v4:
> 
>  * Additional patch that removes the use of of_irq_parse_and_map() in
>    ehci-orion, and just uses platform_get_irq() instead, to avoid a
>    different case between DT and non-T.
> 
>  * Additional patch that renames the ehci-orion error handling goto
>    labels to have more meaningful names, in preparation for handling
>    of additional error cases in followup patches.
> 
>  * Additional patch to update the ehci-orion Device Tree binding
>    documentation.
> 
>  * Merge of 375/38x XHCI support patches: one patch for the driver,
>    one patch for the Device Tree binding documentation (instead of one
>    patch of these, for each SoC).
> 
>  * Update of the XHCI Device Tree binding documentation to indicate
>    the new optional clocks property.
> 
>  * Update of armada-375.dtsi to add missing spaces in the phy-names
>    definition.
> 
>  * Update of armada-38x.dtsi to use 0x4000 instead of 0x3fff for the
>    register area length. Noticed by Andrew Lunn.
> 
>  * Changed the PHY driver Kconfig option to only be enabled either
>    when MACH_ARMADA_375 or COMPILE_TEST are enabled. In the previous
>    version, the PHY driver was always enabled, regardless of the
>    platform.
> 
>  * Various improvements to the PHY driver:
> 
>     - Rename USB2_PHY_CONFIG_ENABLE to USB2_PHY_CONFIG_DISABLE, as
>       suggested by Ezequiel Garcia.
>     - Simplify the logic of armada375_usb_phy_init() by handling the
>       !cluster_phy->enable case first.
>     - Use devm_ioremap_resource() in the ->probe() function instead of
>       of_iomap().
>     - Bail out from ->probe() when a PHY cannot be created.
>     - Fix typos in comments.
> 
>  * Reworded the Kconfig prompt and help text of XHCI_MVEBU to indicate
>    that it's for both 375 and 38x (it was only indicating 38x until
>    now).
> 
>  * Changed the Makefile bit of the xhci-mvebu driver according to the
>    suggestion of Felipe Balbi, so that things work properly when
>    CONFIG_USB_XHCI_MVEBU is 'm'.
> 
>  * Various improvements in the ehci-orion driver:
> 
>     - Use better goto labels for error handling in ->probe().
>     - Use devm_phy_optional_get() instead of devm_phy_get(), which
>       allows to handle EPROBE_DEFER cases nicely.
>     - Call phy_power_off() when needed (probe error handling, and
>       remove).
>     - Use __initconst instead of __initdata for override structure, as
>       noticed by checkpatch.
>     - Fix use after free errors noticed by Felipe Balbi.
> 
>  * Various improvements to the XHCI driver:
> 
>     - Don't make xhci_mvebu_mbus_config() an __init function, since
>       it's called from probe(), which isn't in __init.
>     - Don't use the buggy 'priv[0]' solution implemented in the
>       previous version of the patch set (see in
>       http://lists.infradead.org/pipermail/linux-arm-kernel/2014-May/253871.html
>       the details of why it doesn't work). Instead, add a 'struct
>       clk*' field in xhci_hcd to support the clock in xhci-plat,
>       exactly like xhci_hcd has msix_count and msix_entries for
>       xhci-pci.
>     - Misc minor code style improvements.
> 
> Thomas
> 
> Gregory CLEMENT (17):
>   usb: ehci-orion: fix clock reference leaking
>   usb: ehci-orion: add optional PHY support
>   usb: host: xhci-plat: sort the headers in alphabetic order
>   usb: host: xhci-plat: add clock support
>   usb: host: xhci-plat: add support for the Armada 375/38x XHCI
>     controllers
>   Documentation: dt-bindings: update xhci-platform DT binding
>   phy: add support for USB cluster on the Armada 375 SoC
>   Documentation: dt-bindings: document the Armada 375 USB cluster
>     binding
>   ARM: mvebu: add USB3 support for Armada 38x
>   ARM: mvebu: add USB3 support for Armada 375
>   ARM: configs: enable XHCI mvebu support in mvebu_v7_defconfig
>   ARM: configs: enable XHCI mvebu support in multi_v7_defconfig
>   ARM: mvebu: add Device Tree description of xHCI controllers on Armada
>     38x
>   ARM: mvebu: add Device Tree description of the EHCI controller on
>     Armada 38x
>   ARM: mvebu: add Device Tree description of USB cluster controller on
>     Armada 375
>   ARM: mvebu: add Device Tree description of the xHCI controller on
>     Armada 375
>   ARM: mvebu: add Device Tree description of the EHCI controller on
>     Armada 375
> 
> Thomas Petazzoni (3):
>   usb: ehci-orion: use platform_get_irq() for DT probing
>   usb: ehci-orion: rename error goto labels in ehci_orion_drv_probe()
>   Documentation: dt-bindings: update ehci-orion binding documentation
> 
>  .../bindings/phy/armada-375-usb-phy-cluster.txt    |  19 +++
>  .../devicetree/bindings/usb/ehci-orion.txt         |   5 +
>  Documentation/devicetree/bindings/usb/usb-xhci.txt |   7 +-
>  arch/arm/boot/dts/armada-375-db.dts                |   8 ++
>  arch/arm/boot/dts/armada-375.dtsi                  |  34 +++++
>  arch/arm/boot/dts/armada-385-db.dts                |  12 ++
>  arch/arm/boot/dts/armada-385-rd.dts                |   4 +
>  arch/arm/boot/dts/armada-38x.dtsi                  |  25 ++++
>  arch/arm/configs/multi_v7_defconfig                |   1 +
>  arch/arm/configs/mvebu_v7_defconfig                |   1 +
>  arch/arm/mach-mvebu/Kconfig                        |   2 +
>  drivers/phy/Kconfig                                |   6 +
>  drivers/phy/Makefile                               |   1 +
>  drivers/phy/phy-armada375-usb2.c                   | 157 +++++++++++++++++++++
>  drivers/usb/host/Kconfig                           |   8 ++
>  drivers/usb/host/Makefile                          |   3 +
>  drivers/usb/host/ehci-orion.c                      |  92 ++++++++----
>  drivers/usb/host/xhci-mvebu.c                      |  70 +++++++++
>  drivers/usb/host/xhci-mvebu.h                      |  21 +++
>  drivers/usb/host/xhci-plat.c                       |  42 +++++-
>  drivers/usb/host/xhci.h                            |   2 +
>  21 files changed, 488 insertions(+), 32 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/phy/armada-375-usb-phy-cluster.txt
>  create mode 100644 drivers/phy/phy-armada375-usb2.c
>  create mode 100644 drivers/usb/host/xhci-mvebu.c
>  create mode 100644 drivers/usb/host/xhci-mvebu.h
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
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