[PATCH 1/5] phy: add support for USB cluster on the Armada 375 SoC

[PATCH 1/5] phy: add support for USB cluster on the Armada 375 SoC

[Gregory CLEMENT]

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                 | 140 +++++++++++++++++++++++
 include/dt-bindings/phy/armada-375-usb-cluster.h |  18 +++
 4 files changed, 165 insertions(+)
 create mode 100644 drivers/phy/phy-armada375-usb2.c
 create mode 100644 include/dt-bindings/phy/armada-375-usb-cluster.h

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 3bb05f17b9b4..e63cf9d15489 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 2faf78edc864..47d5a86807b6 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 000000000000..6f8b274693de
--- /dev/null
+++ b/drivers/phy/phy-armada375-usb2.c
@@ -0,0 +1,140 @@
+/*
+ * 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 <dt-bindings/phy/armada-375-usb-cluster.h>
+#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>
+
+#define USB2_PHY_CONFIG_DISABLE BIT(0)
+
+struct armada375_cluster_phy {
+	struct phy *phy;
+	void __iomem *reg;
+	bool use_usb3;
+	bool phy_provided;
+};
+
+static struct armada375_cluster_phy usb_cluster_phy;
+
+static int armada375_usb_phy_init(struct phy *phy)
+{
+	struct armada375_cluster_phy *cluster_phy = phy_get_drvdata(phy);
+	u32 reg;
+
+	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,
+};
+
+/*
+ * Only one controller can use this PHY. We shouldn't have the case
+ * when two controllers want to use this PHY. But if this case occurs
+ * then we provide a phy to the first one and return an error for the
+ * next one. This error has also to be an error returned by
+ * devm_phy_optional_get() so different from ENODEV for USB2. In the
+ * USB3 case it still optional and we use ENODEV.
+ */
+static struct phy *armada375_usb_phy_xlate(struct device *dev,
+					struct of_phandle_args *args)
+{
+	if (WARN_ON(usb_cluster_phy.phy_provided)) {
+		dev_err(dev, "This PHY has already been provided!\n");
+		dev_err(dev, "Check your device tree, only one controller can use it\n.");
+		if (args->args[0] == PHY_USB2)
+			return ERR_PTR(-EBUSY);
+		else
+			return ERR_PTR(-ENODEV);
+	}
+
+	if (args->args[0] == PHY_USB2)
+		usb_cluster_phy.use_usb3 = false;
+	else if (args->args[0] == PHY_USB3)
+		usb_cluster_phy.use_usb3 = true;
+	else {
+		dev_err(dev, "Invalid PHY mode\n");
+		return ERR_PTR(-ENODEV);
+	}
+
+	usb_cluster_phy.phy_provided = true;
+
+	return usb_cluster_phy.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 resource *res;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	usb_cluster_base = devm_ioremap_resource(&pdev->dev, res);
+	if (!usb_cluster_base)
+		return -ENOMEM;
+
+	phy = devm_phy_create(dev, &armada375_usb_phy_ops, NULL);
+	if (IS_ERR(phy)) {
+		dev_err(dev, "failed to create PHY \n");
+		return PTR_ERR(phy);
+	}
+
+	usb_cluster_phy.phy = phy;
+	usb_cluster_phy.reg = usb_cluster_base;
+	phy_set_drvdata(phy, &usb_cluster_phy);
+
+	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");
diff --git a/include/dt-bindings/phy/armada-375-usb-cluster.h b/include/dt-bindings/phy/armada-375-usb-cluster.h
new file mode 100644
index 000000000000..8cf037bb6408
--- /dev/null
+++ b/include/dt-bindings/phy/armada-375-usb-cluster.h
@@ -0,0 +1,18 @@
+/*
+ * Copyright (C) 2014 Marvell
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ */
+
+#ifndef __ARMADA_375_USB_CLUSTER__
+#define __ARMADA_375_USB_CLUSTER__
+
+/* The supported mode passed through by the phandle are the following */
+
+#define PHY_USB2    0
+#define PHY_USB3    1
+
+#endif /* __DT_BINDINGS_CLOCK_R8A7791_H__ */
-- 
1.8.1.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

[PATCH 3/5] ARM: mvebu: add Device Tree description of USB cluster controller on Armada 375

[Gregory CLEMENT]

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@free-electrons.com>
---
 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 53baeb2f1fe8..31b3ac0f9c77 100644
--- a/arch/arm/boot/dts/armada-375.dtsi
+++ b/arch/arm/boot/dts/armada-375.dtsi
@@ -293,6 +293,12 @@
 				#clock-cells = <1>;
 			};
 
+			usbcluster: usb-cluster@18400 {
+				compatible = "marvell,armada-375-usb-cluster";
+				reg = <0x18400 0x4>;
+				#phy-cells = <1>;
+			};
+
 			mbusc: mbus-controller@20000 {
 				compatible = "marvell,mbus-controller";
 				reg = <0x20000 0x100>, <0x20180 0x20>;
-- 
1.8.1.2

[PATCH 4/5] usb: host: xhci-plat: add optional PHY support

[Gregory CLEMENT]

This commit extends the xhci-plat 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.

As for the clock, to achieve this, it adds a 'struct phy *' member in
xhci_hcd. While only used for now in xhci-plat, here again, it might
be used by other drivers in the future.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 drivers/usb/host/xhci-plat.c | 29 ++++++++++++++++++++++++++++-
 drivers/usb/host/xhci.h      |  2 ++
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 0f5f4c8f5bf6..34239b582621 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -15,6 +15,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/phy/phy.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 
@@ -94,6 +95,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	struct resource         *res;
 	struct usb_hcd		*hcd;
 	struct clk              *clk;
+	struct phy              *phy;
 	int			ret;
 	int			irq;
 
@@ -160,9 +162,23 @@ static int xhci_plat_probe(struct platform_device *pdev)
 			goto unmap_registers;
 	}
 
+	phy = devm_phy_optional_get(&pdev->dev, "usb");
+	if (IS_ERR(phy)) {
+		ret = PTR_ERR(phy);
+		goto disable_clk;
+	} else {
+		ret = phy_init(phy);
+		if (ret)
+			goto disable_phy;
+
+		ret = phy_power_on(phy);
+		if (ret)
+			goto disable_phy;
+	}
+
 	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
 	if (ret)
-		goto disable_clk;
+		goto power_off_phy;
 
 	device_wakeup_enable(hcd->self.controller);
 
@@ -198,6 +214,12 @@ put_usb3_hcd:
 dealloc_usb2_hcd:
 	usb_remove_hcd(hcd);
 
+power_off_phy:
+	if (!IS_ERR(phy))
+		phy_power_off(phy);
+disable_phy:
+	if (!IS_ERR(phy))
+		phy_exit(phy);
 disable_clk:
 	if (!IS_ERR(clk))
 		clk_disable_unprepare(clk);
@@ -219,6 +241,7 @@ 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;
+	struct phy *phy = xhci->phy;
 
 	usb_remove_hcd(xhci->shared_hcd);
 	usb_put_hcd(xhci->shared_hcd);
@@ -226,6 +249,10 @@ static int xhci_plat_remove(struct platform_device *dev)
 	usb_remove_hcd(hcd);
 	if (!IS_ERR(clk))
 		clk_disable_unprepare(clk);
+	if (!IS_ERR(phy)) {
+		phy_power_off(phy);
+		phy_exit(phy);
+	}
 	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 003dc094ca37..4d2e084f6d2d 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1480,6 +1480,8 @@ struct xhci_hcd {
 	struct msix_entry	*msix_entries;
 	/* optional clock */
 	struct clk		*clk;
+	/* optional phy */
+	struct phy		*phy;
 	/* data structures */
 	struct xhci_device_context_array *dcbaa;
 	struct xhci_ring	*cmd_ring;
-- 
1.8.1.2

Re: [PATCH 4/5] usb: host: xhci-plat: add optional PHY support

[Kishon Vijay Abraham I]

Hi,

On Friday 16 May 2014 09:52 PM, Gregory CLEMENT wrote:
> This commit extends the xhci-plat 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.
> 
> As for the clock, to achieve this, it adds a 'struct phy *' member in
> xhci_hcd. While only used for now in xhci-plat, here again, it might
> be used by other drivers in the future.
> 
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
>  drivers/usb/host/xhci-plat.c | 29 ++++++++++++++++++++++++++++-
>  drivers/usb/host/xhci.h      |  2 ++
>  2 files changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 0f5f4c8f5bf6..34239b582621 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -15,6 +15,7 @@
>  #include <linux/dma-mapping.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> +#include <linux/phy/phy.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  
> @@ -94,6 +95,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
>  	struct resource         *res;
>  	struct usb_hcd		*hcd;
>  	struct clk              *clk;
> +	struct phy              *phy;
>  	int			ret;
>  	int			irq;
>  
> @@ -160,9 +162,23 @@ static int xhci_plat_probe(struct platform_device *pdev)
>  			goto unmap_registers;
>  	}
>  
> +	phy = devm_phy_optional_get(&pdev->dev, "usb");
> +	if (IS_ERR(phy)) {
> +		ret = PTR_ERR(phy);
> +		goto disable_clk;
> +	} else {
> +		ret = phy_init(phy);
> +		if (ret)
> +			goto disable_phy;

I think you meant disable_clk here?
> +
> +		ret = phy_power_on(phy);
> +		if (ret)
> +			goto disable_phy;
> +	}
> +
>  	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
>  	if (ret)
> -		goto disable_clk;
> +		goto power_off_phy;
>  
>  	device_wakeup_enable(hcd->self.controller);
>  
> @@ -198,6 +214,12 @@ put_usb3_hcd:
>  dealloc_usb2_hcd:
>  	usb_remove_hcd(hcd);
>  
> +power_off_phy:
> +	if (!IS_ERR(phy))

This check is unnecessary here since you do power_off only if PHY is not error.
> +		phy_power_off(phy);
> +disable_phy:
> +	if (!IS_ERR(phy))

same here..
> +		phy_exit(phy);
>  disable_clk:
>  	if (!IS_ERR(clk))
>  		clk_disable_unprepare(clk);
> @@ -219,6 +241,7 @@ 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;
> +	struct phy *phy = xhci->phy;
>  
>  	usb_remove_hcd(xhci->shared_hcd);
>  	usb_put_hcd(xhci->shared_hcd);
> @@ -226,6 +249,10 @@ static int xhci_plat_remove(struct platform_device *dev)
>  	usb_remove_hcd(hcd);
>  	if (!IS_ERR(clk))
>  		clk_disable_unprepare(clk);
> +	if (!IS_ERR(phy)) {

same here..

Thanks
Kishon

Re: [PATCH 4/5] usb: host: xhci-plat: add optional PHY support

[Gregory CLEMENT]

On 23/05/2014 11:28, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Friday 16 May 2014 09:52 PM, Gregory CLEMENT wrote:
>> This commit extends the xhci-plat 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.
>>
>> As for the clock, to achieve this, it adds a 'struct phy *' member in
>> xhci_hcd. While only used for now in xhci-plat, here again, it might
>> be used by other drivers in the future.
>>
>> Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>> ---
>>  drivers/usb/host/xhci-plat.c | 29 ++++++++++++++++++++++++++++-
>>  drivers/usb/host/xhci.h      |  2 ++
>>  2 files changed, 30 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
>> index 0f5f4c8f5bf6..34239b582621 100644
>> --- a/drivers/usb/host/xhci-plat.c
>> +++ b/drivers/usb/host/xhci-plat.c
>> @@ -15,6 +15,7 @@
>>  #include <linux/dma-mapping.h>
>>  #include <linux/module.h>
>>  #include <linux/of.h>
>> +#include <linux/phy/phy.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/slab.h>
>>  
>> @@ -94,6 +95,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
>>  	struct resource         *res;
>>  	struct usb_hcd		*hcd;
>>  	struct clk              *clk;
>> +	struct phy              *phy;
>>  	int			ret;
>>  	int			irq;
>>  
>> @@ -160,9 +162,23 @@ static int xhci_plat_probe(struct platform_device *pdev)
>>  			goto unmap_registers;
>>  	}
>>  
>> +	phy = devm_phy_optional_get(&pdev->dev, "usb");
>> +	if (IS_ERR(phy)) {
>> +		ret = PTR_ERR(phy);
>> +		goto disable_clk;
>> +	} else {
>> +		ret = phy_init(phy);
>> +		if (ret)
>> +			goto disable_phy;
> s
> I think you meant disable_clk here?

yes indeed!


>> +
>> +		ret = phy_power_on(phy);
>> +		if (ret)
>> +			goto disable_phy;
>> +	}
>> +
>>  	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
>>  	if (ret)
>> -		goto disable_clk;
>> +		goto power_off_phy;
>>  
>>  	device_wakeup_enable(hcd->self.controller);
>>  
>> @@ -198,6 +214,12 @@ put_usb3_hcd:
>>  dealloc_usb2_hcd:
>>  	usb_remove_hcd(hcd);
>>  
>> +power_off_phy:
>> +	if (!IS_ERR(phy))
> 
> This check is unnecessary here since you do power_off only if PHY is not error.

Good catch again!


>> +		phy_power_off(phy);
>> +disable_phy:
>> +	if (!IS_ERR(phy))
> 
> same here..

I agree

>> +		phy_exit(phy);
>>  disable_clk:
>>  	if (!IS_ERR(clk))
>>  		clk_disable_unprepare(clk);
>> @@ -219,6 +241,7 @@ 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;
>> +	struct phy *phy = xhci->phy;
>>  
>>  	usb_remove_hcd(xhci->shared_hcd);
>>  	usb_put_hcd(xhci->shared_hcd);
>> @@ -226,6 +249,10 @@ static int xhci_plat_remove(struct platform_device *dev)
>>  	usb_remove_hcd(hcd);
>>  	if (!IS_ERR(clk))
>>  		clk_disable_unprepare(clk);
>> +	if (!IS_ERR(phy)) {
> 
> same here..

I agree too

Thanks for you review,

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

[PATCH 5/5] ARM: mvebu: add PHY support to the dts for the USB controllers on Armada 375

[Gregory CLEMENT]

Now that the USB cluster node has been added, use it as a PHY provider
for the USB controller linked to it: the first EHCI and the xHCI.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 arch/arm/boot/dts/armada-375.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/armada-375.dtsi b/arch/arm/boot/dts/armada-375.dtsi
index 31b3ac0f9c77..0896b0daf7be 100644
--- a/arch/arm/boot/dts/armada-375.dtsi
+++ b/arch/arm/boot/dts/armada-375.dtsi
@@ -14,6 +14,7 @@
 #include "skeleton.dtsi"
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/phy/armada-375-usb-cluster.h>
 
 #define MBUS_ID(target,attributes) (((target) << 24) | ((attributes) << 16))
 
@@ -331,6 +332,8 @@
 				reg = <0x50000 0x500>;
 				interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&gateclk 18>;
+				phys = <&usbcluster PHY_USB2>;
+				phy-names = "usb";
 				status = "disabled";
 			};
 
@@ -347,6 +350,8 @@
 				reg = <0x58000 0x20000>,<0x5b880 0x80>;
 				interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&gateclk 16>;
+				phys = <&usbcluster PHY_USB3>;
+				phy-names = "usb";
 				status = "disabled";
 			};
 
-- 
1.8.1.2

[PATCH 2/5] Documentation: dt-bindings: document the Armada 375 USB cluster binding

[Gregory CLEMENT]

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>
---
 .../bindings/phy/armada-375-usb-phy-cluster.txt      | 20 ++++++++++++++++++++
 1 file changed, 20 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 000000000000..9bd1502e1f33
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/armada-375-usb-phy-cluster.txt
@@ -0,0 +1,20 @@
+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. Possible
+values are 0 (USB2), 1 (USB3).
+
+Example:
+	usbcluster: usb-cluster@18400 {
+		compatible = "marvell,armada-375-usb-cluster";
+		reg = <0x18400 0x4>;
+		#phy-cells = <1>
+	};
-- 
1.8.1.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: [PATCH 2/5] Documentation: dt-bindings: document the Armada 375 USB cluster binding

[Kishon Vijay Abraham I]

Hi,

On Friday 16 May 2014 09:52 PM, Gregory CLEMENT wrote:
> 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.

Pls re-order so that the Documentation patch comes before the driver patch..
> 
> Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
>  .../bindings/phy/armada-375-usb-phy-cluster.txt      | 20 ++++++++++++++++++++
>  1 file changed, 20 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

simpler file name? armada-phy?

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: [PATCH 2/5] Documentation: dt-bindings: document the Armada 375 USB cluster binding

[Andrew Lunn]

On Fri, May 23, 2014 at 02:54:02PM +0530, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Friday 16 May 2014 09:52 PM, Gregory CLEMENT wrote:
> > 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.
> 
> Pls re-order so that the Documentation patch comes before the driver patch..
> > 
> > Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> > ---
> >  .../bindings/phy/armada-375-usb-phy-cluster.txt      | 20 ++++++++++++++++++++
> >  1 file changed, 20 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
> 
> simpler file name? armada-phy?

Armada is a collection of families of SoCs. This driver is very
specific to one SoC, in one family of SoCs.

Do you want one .txt file per driver, or can we combine binding
documentations into one file? There should already be a mvebu-phy.txt,
which contains the sata phy usable on some of the Armada SoC families.
This binding could be appended to it.

      Andrew

Re: [PATCH 2/5] Documentation: dt-bindings: document the Armada 375 USB cluster binding

[Andrew Lunn]

> Do you want one .txt file per driver, or can we combine binding
> documentations into one file? There should already be a mvebu-phy.txt,
> which contains the sata phy usable on some of the Armada SoC families.
> This binding could be appended to it.

Ah. Humm, well! It seems the patch adding the mvebu-phy.txt fell
through a crack when adding the driver.

Kishon could you take
http://www.spinics.net/lists/devicetree/msg17018.html into your tree?
It should of been taken the same time you took the actual driver,
http://www.spinics.net/lists/devicetree/msg17011.html into your tree.

I can resend that one missing patch if you want.

  Andrew
--
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: [PATCH 2/5] Documentation: dt-bindings: document the Armada 375 USB cluster binding

[Kishon Vijay Abraham I]

hI,

On Friday 23 May 2014 07:06 PM, Andrew Lunn wrote:
>> Do you want one .txt file per driver, or can we combine binding
>> documentations into one file? There should already be a mvebu-phy.txt,
>> which contains the sata phy usable on some of the Armada SoC families.
>> This binding could be appended to it.
> 
> Ah. Humm, well! It seems the patch adding the mvebu-phy.txt fell
> through a crack when adding the driver.
> 
> Kishon could you take
> http://www.spinics.net/lists/devicetree/msg17018.html into your tree?
> It should of been taken the same time you took the actual driver,
> http://www.spinics.net/lists/devicetree/msg17011.html into your tree.
> 
> I can resend that one missing patch if you want.

yes please. But it's already too late to go in the next merge window.

-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: [PATCH 2/5] Documentation: dt-bindings: document the Armada 375 USB cluster binding

[Andrew Lunn]

On Fri, May 23, 2014 at 07:22:48PM +0530, Kishon Vijay Abraham I wrote:
> hI,
> 
> On Friday 23 May 2014 07:06 PM, Andrew Lunn wrote:
> >> Do you want one .txt file per driver, or can we combine binding
> >> documentations into one file? There should already be a mvebu-phy.txt,
> >> which contains the sata phy usable on some of the Armada SoC families.
> >> This binding could be appended to it.
> > 
> > Ah. Humm, well! It seems the patch adding the mvebu-phy.txt fell
> > through a crack when adding the driver.
> > 
> > Kishon could you take
> > http://www.spinics.net/lists/devicetree/msg17018.html into your tree?
> > It should of been taken the same time you took the actual driver,
> > http://www.spinics.net/lists/devicetree/msg17011.html into your tree.
> > 
> > I can resend that one missing patch if you want.
> 
> yes please. But it's already too late to go in the next merge window.

Gregory, could you pick it up and append your 375 binding to it? We
can avoid merge conflicts that way.

Thanks
	Andrew
--
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: [PATCH 2/5] Documentation: dt-bindings: document the Armada 375 USB cluster binding

[Gregory CLEMENT]

On 23/05/2014 15:59, Andrew Lunn wrote:
> On Fri, May 23, 2014 at 07:22:48PM +0530, Kishon Vijay Abraham I wrote:
>> hI,
>>
>> On Friday 23 May 2014 07:06 PM, Andrew Lunn wrote:
>>>> Do you want one .txt file per driver, or can we combine binding
>>>> documentations into one file? There should already be a mvebu-phy.txt,
>>>> which contains the sata phy usable on some of the Armada SoC families.
>>>> This binding could be appended to it.
>>>
>>> Ah. Humm, well! It seems the patch adding the mvebu-phy.txt fell
>>> through a crack when adding the driver.
>>>
>>> Kishon could you take
>>> http://www.spinics.net/lists/devicetree/msg17018.html into your tree?
>>> It should of been taken the same time you took the actual driver,
>>> http://www.spinics.net/lists/devicetree/msg17011.html into your tree.
>>>
>>> I can resend that one missing patch if you want.
>>
>> yes please. But it's already too late to go in the next merge window.
> 
> Gregory, could you pick it up and append your 375 binding to it? We
> can avoid merge conflicts that way.

yes sure, I will do it

> 
> Thanks
> 	Andrew
> 


-- 
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: [PATCH 2/5] Documentation: dt-bindings: document the Armada 375 USB cluster binding

[Kishon Vijay Abraham I]

Hi,

On Friday 23 May 2014 06:51 PM, Andrew Lunn wrote:
> On Fri, May 23, 2014 at 02:54:02PM +0530, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Friday 16 May 2014 09:52 PM, Gregory CLEMENT wrote:
>>> 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.
>>
>> Pls re-order so that the Documentation patch comes before the driver patch..
>>>
>>> Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>> ---
>>>  .../bindings/phy/armada-375-usb-phy-cluster.txt      | 20 ++++++++++++++++++++
>>>  1 file changed, 20 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
>>
>> simpler file name? armada-phy?
> 
> Armada is a collection of families of SoCs. This driver is very
> specific to one SoC, in one family of SoCs.
> 
> Do you want one .txt file per driver, or can we combine binding
> documentations into one file? There should already be a mvebu-phy.txt,
> which contains the sata phy usable on some of the Armada SoC families.
> This binding could be appended to it.

So far we've been creating Documentation file per manufacturer (ti-phy.txt,
samsung-phy.txt..), so would prefer if you could combine binding documentation
in a single file.

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: [PATCH 2/5] Documentation: dt-bindings: document the Armada 375 USB cluster binding

[Gregory CLEMENT]

Hi Kishon,

On 23/05/2014 11:24, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Friday 16 May 2014 09:52 PM, Gregory CLEMENT wrote:
>> 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.
> 
> Pls re-order so that the Documentation patch comes before the driver patch..

OK I will do it

>>
>> Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>> ---
>>  .../bindings/phy/armada-375-usb-phy-cluster.txt      | 20 ++++++++++++++++++++
>>  1 file changed, 20 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
> 
> simpler file name? armada-phy?

I can remove the "cluster" part but not the 375, there are many SoCs
from Marvell called Armada, and some of then have nearly nothing in
common, so I prefer keep this name. If your intent is to have a file
with all the PHY binding related to an SoC family, then we should call it
mvebu-phy.


Thanks for your review,

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 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: [PATCH 1/5] phy: add support for USB cluster on the Armada 375 SoC

[Kishon Vijay Abraham I]

Hi,

On Friday 16 May 2014 09:52 PM, Gregory CLEMENT wrote:
> 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                 | 140 +++++++++++++++++++++++
>  include/dt-bindings/phy/armada-375-usb-cluster.h |  18 +++
>  4 files changed, 165 insertions(+)
>  create mode 100644 drivers/phy/phy-armada375-usb2.c
>  create mode 100644 include/dt-bindings/phy/armada-375-usb-cluster.h
> 
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 3bb05f17b9b4..e63cf9d15489 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 2faf78edc864..47d5a86807b6 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 000000000000..6f8b274693de
> --- /dev/null
> +++ b/drivers/phy/phy-armada375-usb2.c
> @@ -0,0 +1,140 @@
> +/*
> + * 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 <dt-bindings/phy/armada-375-usb-cluster.h>
> +#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>
> +
> +#define USB2_PHY_CONFIG_DISABLE BIT(0)
> +
> +struct armada375_cluster_phy {
> +	struct phy *phy;
> +	void __iomem *reg;
> +	bool use_usb3;
> +	bool phy_provided;
> +};
> +
> +static struct armada375_cluster_phy usb_cluster_phy;
> +
> +static int armada375_usb_phy_init(struct phy *phy)
> +{
> +	struct armada375_cluster_phy *cluster_phy = phy_get_drvdata(phy);
> +	u32 reg;
> +
> +	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,

nitpick: unnecessary tab.
> +};
> +
> +/*
> + * Only one controller can use this PHY. We shouldn't have the case
> + * when two controllers want to use this PHY. But if this case occurs
> + * then we provide a phy to the first one and return an error for the
> + * next one. This error has also to be an error returned by
> + * devm_phy_optional_get() so different from ENODEV for USB2. In the
> + * USB3 case it still optional and we use ENODEV.
> + */
> +static struct phy *armada375_usb_phy_xlate(struct device *dev,
> +					struct of_phandle_args *args)
> +{
> +	if (WARN_ON(usb_cluster_phy.phy_provided)) {
> +		dev_err(dev, "This PHY has already been provided!\n");
> +		dev_err(dev, "Check your device tree, only one controller can use it\n.");
> +		if (args->args[0] == PHY_USB2)
> +			return ERR_PTR(-EBUSY);
> +		else
> +			return ERR_PTR(-ENODEV);
> +	}
> +
> +	if (args->args[0] == PHY_USB2)
> +		usb_cluster_phy.use_usb3 = false;
> +	else if (args->args[0] == PHY_USB3)
> +		usb_cluster_phy.use_usb3 = true;
> +	else {
> +		dev_err(dev, "Invalid PHY mode\n");
> +		return ERR_PTR(-ENODEV);
> +	}

how will this behave if there is a phy_put and then a phy_get?
> +
> +	usb_cluster_phy.phy_provided = true;
> +
> +	return usb_cluster_phy.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 resource *res;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	usb_cluster_base = devm_ioremap_resource(&pdev->dev, res);
> +	if (!usb_cluster_base)
> +		return -ENOMEM;
> +
> +	phy = devm_phy_create(dev, &armada375_usb_phy_ops, NULL);
> +	if (IS_ERR(phy)) {
> +		dev_err(dev, "failed to create PHY \n");
> +		return PTR_ERR(phy);
> +	}
> +
> +	usb_cluster_phy.phy = phy;
> +	usb_cluster_phy.reg = usb_cluster_base;
> +	phy_set_drvdata(phy, &usb_cluster_phy);
> +
> +	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 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: [PATCH 1/5] phy: add support for USB cluster on the Armada 375 SoC

[Gregory CLEMENT]

On 23/05/2014 11:20, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Friday 16 May 2014 09:52 PM, Gregory CLEMENT wrote:
>> 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                 | 140 +++++++++++++++++++++++
>>  include/dt-bindings/phy/armada-375-usb-cluster.h |  18 +++
[...]

>> +static struct phy_ops armada375_usb_phy_ops = {
>> +	.init = armada375_usb_phy_init,
>> +	.owner		= THIS_MODULE,
> 
> nitpick: unnecessary tab.

OK

>> +};
>> +
>> +/*
>> + * Only one controller can use this PHY. We shouldn't have the case
>> + * when two controllers want to use this PHY. But if this case occurs
>> + * then we provide a phy to the first one and return an error for the
>> + * next one. This error has also to be an error returned by
>> + * devm_phy_optional_get() so different from ENODEV for USB2. In the
>> + * USB3 case it still optional and we use ENODEV.
>> + */
>> +static struct phy *armada375_usb_phy_xlate(struct device *dev,
>> +					struct of_phandle_args *args)
>> +{
>> +	if (WARN_ON(usb_cluster_phy.phy_provided)) {
>> +		dev_err(dev, "This PHY has already been provided!\n");
>> +		dev_err(dev, "Check your device tree, only one controller can use it\n.");
>> +		if (args->args[0] == PHY_USB2)
>> +			return ERR_PTR(-EBUSY);
>> +		else
>> +			return ERR_PTR(-ENODEV);
>> +	}
>> +
>> +	if (args->args[0] == PHY_USB2)
>> +		usb_cluster_phy.use_usb3 = false;
>> +	else if (args->args[0] == PHY_USB3)
>> +		usb_cluster_phy.use_usb3 = true;
>> +	else {
>> +		dev_err(dev, "Invalid PHY mode\n");
>> +		return ERR_PTR(-ENODEV);
>> +	}
> 
> how will this behave if there is a phy_put and then a phy_get?

Badly I think :(

I have to think about a better solution then.

>> +
>> +	usb_cluster_phy.phy_provided = true;
>> +
>> +	return usb_cluster_phy.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 resource *res;
>> +
>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	usb_cluster_base = devm_ioremap_resource(&pdev->dev, res);
>> +	if (!usb_cluster_base)
>> +		return -ENOMEM;
>> +
>> +	phy = devm_phy_create(dev, &armada375_usb_phy_ops, NULL);
>> +	if (IS_ERR(phy)) {
>> +		dev_err(dev, "failed to create PHY \n");
>> +		return PTR_ERR(phy);
>> +	}
>> +
>> +	usb_cluster_phy.phy = phy;
>> +	usb_cluster_phy.reg = usb_cluster_base;
>> +	phy_set_drvdata(phy, &usb_cluster_phy);
>> +
>> +	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?

See the header, I chose "GNU General Public License version 2 or later."
so "GPL" match it.


Thanks for your review, once I will have found a proper solution for the
phy_put/phy_get scenario, I will sent a new version with all the other issues
fixed.


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 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: [PATCH 1/5] phy: add support for USB cluster on the Armada 375 SoC

[Thomas Petazzoni]

Gregory, Kishon,

On Fri, 23 May 2014 23:50:54 +0200, Gregory CLEMENT wrote:

> >> +MODULE_DESCRIPTION("Armada 375 USB cluster driver");
> >> +MODULE_AUTHOR("Gregory CLEMENT <gregory.clement@free-electrons.com>");
> >> +MODULE_LICENSE("GPL");
> > 
> > GPL v2?
> 
> See the header, I chose "GNU General Public License version 2 or later."
> so "GPL" match it.

And also, the vast majority of kernel drivers use
MODULE_LICENSE("GPL") :

$ git grep 'MODULE_LICENSE("GPL")' | wc -l
5615
$ git grep 'MODULE_LICENSE("GPLv2")' | wc -l
5
$ git grep 'MODULE_LICENSE("GPL v2")' | wc -l
932

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 1/5] phy: add support for USB cluster on the Armada 375 SoC

[Kishon Vijay Abraham I]

hI,

On Saturday 24 May 2014 03:20 AM, Gregory CLEMENT wrote:
> On 23/05/2014 11:20, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Friday 16 May 2014 09:52 PM, Gregory CLEMENT wrote:
>>> 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                 | 140 +++++++++++++++++++++++
>>>  include/dt-bindings/phy/armada-375-usb-cluster.h |  18 +++
> [...]
> 
>>> +static struct phy_ops armada375_usb_phy_ops = {
>>> +	.init = armada375_usb_phy_init,
>>> +	.owner		= THIS_MODULE,
>>
>> nitpick: unnecessary tab.
> 
> OK
> 
>>> +};
>>> +
>>> +/*
>>> + * Only one controller can use this PHY. We shouldn't have the case
>>> + * when two controllers want to use this PHY. But if this case occurs
>>> + * then we provide a phy to the first one and return an error for the
>>> + * next one. This error has also to be an error returned by
>>> + * devm_phy_optional_get() so different from ENODEV for USB2. In the
>>> + * USB3 case it still optional and we use ENODEV.
>>> + */
>>> +static struct phy *armada375_usb_phy_xlate(struct device *dev,
>>> +					struct of_phandle_args *args)
>>> +{
>>> +	if (WARN_ON(usb_cluster_phy.phy_provided)) {
>>> +		dev_err(dev, "This PHY has already been provided!\n");
>>> +		dev_err(dev, "Check your device tree, only one controller can use it\n.");
>>> +		if (args->args[0] == PHY_USB2)
>>> +			return ERR_PTR(-EBUSY);
>>> +		else
>>> +			return ERR_PTR(-ENODEV);
>>> +	}
>>> +
>>> +	if (args->args[0] == PHY_USB2)
>>> +		usb_cluster_phy.use_usb3 = false;
>>> +	else if (args->args[0] == PHY_USB3)
>>> +		usb_cluster_phy.use_usb3 = true;
>>> +	else {
>>> +		dev_err(dev, "Invalid PHY mode\n");
>>> +		return ERR_PTR(-ENODEV);
>>> +	}
>>
>> how will this behave if there is a phy_put and then a phy_get?
> 
> Badly I think :(
> 
> I have to think about a better solution then.
> 
>>> +
>>> +	usb_cluster_phy.phy_provided = true;
>>> +
>>> +	return usb_cluster_phy.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 resource *res;
>>> +
>>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> +	usb_cluster_base = devm_ioremap_resource(&pdev->dev, res);
>>> +	if (!usb_cluster_base)
>>> +		return -ENOMEM;
>>> +
>>> +	phy = devm_phy_create(dev, &armada375_usb_phy_ops, NULL);
>>> +	if (IS_ERR(phy)) {
>>> +		dev_err(dev, "failed to create PHY \n");
>>> +		return PTR_ERR(phy);
>>> +	}
>>> +
>>> +	usb_cluster_phy.phy = phy;
>>> +	usb_cluster_phy.reg = usb_cluster_base;
>>> +	phy_set_drvdata(phy, &usb_cluster_phy);
>>> +
>>> +	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?
> 
> See the header, I chose "GNU General Public License version 2 or later."
> so "GPL" match it.

Indeed! Just figured it's documented in include/linux/module.h.

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