diff mbox series

[u-boot-marvell,04/11] usb: host: make PHY handling more generic

Message ID 20200419154850.25868-5-marek.behun@nic.cz
State Deferred
Delegated to: Tom Rini
Headers show
Series Armada 37xx: port comphy to generic-phys (PLEASE TEST) | expand

Commit Message

Marek Behún April 19, 2020, 3:48 p.m. UTC 
Since XHCI may also want to use the generic-phy functions from
ehci-hcd.c, move them into separate phy.c.

Change these functions so that they enumerate end power on all
generic-phys defined in device tree for a given USB controller.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 drivers/usb/host/Makefile       |   2 +
 drivers/usb/host/ehci-generic.c |  11 +--
 drivers/usb/host/ehci-hcd.c     |  66 ------------------
 drivers/usb/host/ehci-msm.c     |   7 +-
 drivers/usb/host/ehci-pci.c     |   6 +-
 drivers/usb/host/ehci.h         |   4 --
 drivers/usb/host/phy.c          | 115 ++++++++++++++++++++++++++++++++
 drivers/usb/host/phy.h          |  31 +++++++++
 8 files changed, 161 insertions(+), 81 deletions(-)
 create mode 100644 drivers/usb/host/phy.c
 create mode 100644 drivers/usb/host/phy.h

Comments

Marek Vasut April 20, 2020, 1:41 a.m. UTC | #1 
On 4/19/20 5:48 PM, Marek Behún wrote:
[...]
> +static int usb_phy_setup(struct udevice *dev, int index)
> +{
> +	struct phy phy;
> +	int ret;
> +
> +	ret = generic_phy_get_by_index(dev, index, &phy);
> +	if (ret && ret != -ENOENT) {
> +		dev_err(dev, "failed to get usb phy %i\n", index);
> +		return ret;
> +	}
> +
> +	ret = generic_phy_init(&phy);
> +	if (ret) {
> +		dev_err(dev, "failed to init usb phy %i\n", index);
> +		return ret;
> +	}
> +
> +	ret = generic_phy_set_mode(&phy, PHY_MODE_USB_HOST_SS, 0);

How can this ever work with EHCI , which is HS/FS/LS , but not SS ?

[...]

> diff --git a/drivers/usb/host/phy.h b/drivers/usb/host/phy.h
> new file mode 100644
> index 0000000000..ba3139a714
> --- /dev/null
> +++ b/drivers/usb/host/phy.h
> @@ -0,0 +1,31 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * USB phy functions
> + *
> + * Moved from ehci-hcd.c by Marek Behun <marek.behun@nic.cz>
> + *
> + * Copyright (C) Marek Vasut <marex@denx.de>

I presume the copyright needs updating ?

[...]
Marek Behún April 20, 2020, 9:27 a.m. UTC | #2 
On Mon, 20 Apr 2020 03:41:49 +0200
Marek Vasut <marex@denx.de> wrote:

> > +	ret = generic_phy_set_mode(&phy, PHY_MODE_USB_HOST_SS, 0);  
> 
> How can this ever work with EHCI , which is HS/FS/LS , but not SS ?
> 

Marek, this is how kernel does it in
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/core/hcd.c#n2628
First PHY_MODE_USB_HOST_SS is tried, then PHY_MODE_USB_HOST.
Marek Vasut April 20, 2020, 10:20 a.m. UTC | #3 
On 4/20/20 11:27 AM, Marek Behun wrote:
> On Mon, 20 Apr 2020 03:41:49 +0200
> Marek Vasut <marex@denx.de> wrote:
> 
>>> +	ret = generic_phy_set_mode(&phy, PHY_MODE_USB_HOST_SS, 0);  
>>
>> How can this ever work with EHCI , which is HS/FS/LS , but not SS ?
>>
> 
> Marek, this is how kernel does it in
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/core/hcd.c#n2628
> First PHY_MODE_USB_HOST_SS is tried, then PHY_MODE_USB_HOST.

That seems unnecessary, considering that we already know SS-mode will fail.
Marek Behún April 20, 2020, 10:40 a.m. UTC | #4 
On Mon, 20 Apr 2020 12:20:38 +0200
Marek Vasut <marex@denx.de> wrote:

> On 4/20/20 11:27 AM, Marek Behun wrote:
> > On Mon, 20 Apr 2020 03:41:49 +0200
> > Marek Vasut <marex@denx.de> wrote:
> >   
> >>> +	ret = generic_phy_set_mode(&phy, PHY_MODE_USB_HOST_SS, 0);    
> >>
> >> How can this ever work with EHCI , which is HS/FS/LS , but not SS ?
> >>  
> > 
> > Marek, this is how kernel does it in
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/core/hcd.c#n2628
> > First PHY_MODE_USB_HOST_SS is tried, then PHY_MODE_USB_HOST.  
> 
> That seems unnecessary, considering that we already know SS-mode will fail.

Hmm, I guess phy_mode could be passed as a parameter to the
usb_phys_setup function.
diff mbox series

Patch

diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index b62f346094..f56af000fa 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -8,6 +8,8 @@  obj-y += usb-uclass.o
 obj-$(CONFIG_SANDBOX) += usb-sandbox.o
 endif
 
+obj-$(CONFIG_PHY) += phy.o
+
 # ohci
 obj-$(CONFIG_USB_OHCI_NEW) += ohci-hcd.o
 obj-$(CONFIG_USB_ATMEL) += ohci-at91.o
diff --git a/drivers/usb/host/ehci-generic.c b/drivers/usb/host/ehci-generic.c
index 0643681846..05a3b2c90b 100644
--- a/drivers/usb/host/ehci-generic.c
+++ b/drivers/usb/host/ehci-generic.c
@@ -12,9 +12,11 @@ 
 #include <reset.h>
 #include <asm/io.h>
 #include <dm.h>
-#include "ehci.h"
 #include <power/regulator.h>
 
+#include "ehci.h"
+#include "phy.h"
+
 /*
  * Even though here we don't explicitly use "struct ehci_ctrl"
  * ehci_register() expects it to be the first thing that resides in
@@ -24,7 +26,6 @@  struct generic_ehci {
 	struct ehci_ctrl ctrl;
 	struct clk *clocks;
 	struct reset_ctl *resets;
-	struct phy phy;
 #ifdef CONFIG_DM_REGULATOR
 	struct udevice *vbus_supply;
 #endif
@@ -148,7 +149,7 @@  static int ehci_usb_probe(struct udevice *dev)
 	if (err)
 		goto reset_err;
 
-	err = ehci_setup_phy(dev, &priv->phy, 0);
+	err = usb_phys_setup(dev);
 	if (err)
 		goto regulator_err;
 
@@ -163,7 +164,7 @@  static int ehci_usb_probe(struct udevice *dev)
 	return 0;
 
 phy_err:
-	ret = ehci_shutdown_phy(dev, &priv->phy);
+	ret = usb_phys_shutdown(dev);
 	if (ret)
 		dev_err(dev, "failed to shutdown usb phy\n");
 
@@ -193,7 +194,7 @@  static int ehci_usb_remove(struct udevice *dev)
 	if (ret)
 		return ret;
 
-	ret = ehci_shutdown_phy(dev, &priv->phy);
+	ret = usb_phys_shutdown(dev);
 	if (ret)
 		return ret;
 
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 1edb344d0f..ca39a0ba19 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -1744,69 +1744,3 @@  struct dm_usb_ops ehci_usb_ops = {
 };
 
 #endif
-
-#ifdef CONFIG_PHY
-int ehci_setup_phy(struct udevice *dev, struct phy *phy, int index)
-{
-	int ret;
-
-	if (!phy)
-		return 0;
-
-	ret = generic_phy_get_by_index(dev, index, phy);
-	if (ret) {
-		if (ret != -ENOENT) {
-			dev_err(dev, "failed to get usb phy\n");
-			return ret;
-		}
-	} else {
-		ret = generic_phy_init(phy);
-		if (ret) {
-			dev_err(dev, "failed to init usb phy\n");
-			return ret;
-		}
-
-		ret = generic_phy_power_on(phy);
-		if (ret) {
-			dev_err(dev, "failed to power on usb phy\n");
-			return generic_phy_exit(phy);
-		}
-	}
-
-	return 0;
-}
-
-int ehci_shutdown_phy(struct udevice *dev, struct phy *phy)
-{
-	int ret = 0;
-
-	if (!phy)
-		return 0;
-
-	if (generic_phy_valid(phy)) {
-		ret = generic_phy_power_off(phy);
-		if (ret) {
-			dev_err(dev, "failed to power off usb phy\n");
-			return ret;
-		}
-
-		ret = generic_phy_exit(phy);
-		if (ret) {
-			dev_err(dev, "failed to power off usb phy\n");
-			return ret;
-		}
-	}
-
-	return 0;
-}
-#else
-int ehci_setup_phy(struct udevice *dev, struct phy *phy, int index)
-{
-	return 0;
-}
-
-int ehci_shutdown_phy(struct udevice *dev, struct phy *phy)
-{
-	return 0;
-}
-#endif
diff --git a/drivers/usb/host/ehci-msm.c b/drivers/usb/host/ehci-msm.c
index dd92808ff7..9a9ed4ed6e 100644
--- a/drivers/usb/host/ehci-msm.c
+++ b/drivers/usb/host/ehci-msm.c
@@ -17,13 +17,14 @@ 
 #include <asm/gpio.h>
 #include <asm/io.h>
 #include <linux/compat.h>
+
 #include "ehci.h"
+#include "phy.h"
 
 struct msm_ehci_priv {
 	struct ehci_ctrl ctrl; /* Needed by EHCI */
 	struct usb_ehci *ehci; /* Start of IP core*/
 	struct ulpi_viewport ulpi_vp; /* ULPI Viewport */
-	struct phy phy;
 };
 
 static int msm_init_after_reset(struct ehci_ctrl *dev)
@@ -56,7 +57,7 @@  static int ehci_usb_probe(struct udevice *dev)
 	hcor = (struct ehci_hcor *)((phys_addr_t)hccr +
 			HC_LENGTH(ehci_readl(&(hccr)->cr_capbase)));
 
-	ret = ehci_setup_phy(dev, &p->phy, 0);
+	ret = usb_phys_setup(dev);
 	if (ret)
 		return ret;
 
@@ -81,7 +82,7 @@  static int ehci_usb_remove(struct udevice *dev)
 	/* Stop controller. */
 	clrbits_le32(&ehci->usbcmd, CMD_RUN);
 
-	ret = ehci_shutdown_phy(dev, &p->phy);
+	ret = usb_phys_shutdown(dev);
 	if (ret)
 		return ret;
 
diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
index 04e7c5e37f..8e722606a6 100644
--- a/drivers/usb/host/ehci-pci.c
+++ b/drivers/usb/host/ehci-pci.c
@@ -12,11 +12,11 @@ 
 #include <asm/io.h>
 
 #include "ehci.h"
+#include "phy.h"
 
 /* Information about a USB port */
 struct ehci_pci_priv {
 	struct ehci_ctrl ehci;
-	struct phy phy;
 };
 
 #if CONFIG_IS_ENABLED(DM_USB)
@@ -29,7 +29,7 @@  static int ehci_pci_init(struct udevice *dev, struct ehci_hccr **ret_hccr,
 	int ret;
 	u32 cmd;
 
-	ret = ehci_setup_phy(dev, &priv->phy, 0);
+	ret = usb_phys_setup(dev);
 	if (ret)
 		return ret;
 
@@ -146,7 +146,7 @@  static int ehci_pci_remove(struct udevice *dev)
 	if (ret)
 		return ret;
 
-	return ehci_shutdown_phy(dev, &priv->phy);
+	return usb_phys_shutdown(dev);
 }
 
 static const struct udevice_id ehci_pci_ids[] = {
diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index 66c1d61dbf..a4dfedfbb2 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -294,8 +294,4 @@  int ehci_register(struct udevice *dev, struct ehci_hccr *hccr,
 int ehci_deregister(struct udevice *dev);
 extern struct dm_usb_ops ehci_usb_ops;
 
-/* EHCI PHY functions */
-int ehci_setup_phy(struct udevice *dev, struct phy *phy, int index);
-int ehci_shutdown_phy(struct udevice *dev, struct phy *phy);
-
 #endif /* USB_EHCI_H */
diff --git a/drivers/usb/host/phy.c b/drivers/usb/host/phy.c
new file mode 100644
index 0000000000..a8d3789a9d
--- /dev/null
+++ b/drivers/usb/host/phy.c
@@ -0,0 +1,115 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * USB phy functions
+ *
+ * Based on code from ehci-hcd.c by Marek Vasut <marex@denx.de>
+ *
+ * Copyright (C) Marek Behun <marek.behun@nic.cz>
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <generic-phy.h>
+#include <linux/err.h>
+
+static int usb_phy_setup(struct udevice *dev, int index)
+{
+	struct phy phy;
+	int ret;
+
+	ret = generic_phy_get_by_index(dev, index, &phy);
+	if (ret && ret != -ENOENT) {
+		dev_err(dev, "failed to get usb phy %i\n", index);
+		return ret;
+	}
+
+	ret = generic_phy_init(&phy);
+	if (ret) {
+		dev_err(dev, "failed to init usb phy %i\n", index);
+		return ret;
+	}
+
+	ret = generic_phy_set_mode(&phy, PHY_MODE_USB_HOST_SS, 0);
+	if (ret) {
+		ret = generic_phy_set_mode(&phy, PHY_MODE_USB_HOST, 0);
+		if (ret) {
+			dev_err(dev, "failed to set mode on usb phy %i\n",
+				index);
+			goto err;
+		}
+	}
+
+	ret = generic_phy_power_on(&phy);
+	if (ret) {
+		dev_err(dev, "failed to power on usb phy %i\n", index);
+		goto err;
+	}
+
+	return 0;
+
+err:
+	generic_phy_exit(&phy);
+
+	return ret;
+}
+
+static int usb_phy_shutdown(struct udevice *dev, int index)
+{
+	struct phy phy;
+	int ret;
+
+	ret = generic_phy_get_by_index(dev, index, &phy);
+	if (ret && ret != -ENOENT) {
+		dev_err(dev, "failed to get usb phy %i\n", index);
+		return ret;
+	}
+
+	ret = generic_phy_power_off(&phy);
+	if (ret) {
+		dev_err(dev, "failed to power off usb phy %i\n", index);
+		return ret;
+	}
+
+	ret = generic_phy_exit(&phy);
+	if (ret) {
+		dev_err(dev, "failed to exit usb phy %i\n", index);
+		return ret;
+	}
+
+	return 0;
+}
+
+int usb_phys_setup(struct udevice *dev)
+{
+	int ret, index, count;
+
+	count = dev_count_phandle_with_args(dev, "phys", "#phy-cells");
+
+	for (index = 0; index < count; ++index) {
+		ret = usb_phy_setup(dev, index);
+		if (ret)
+			goto err;
+	}
+
+	return 0;
+err:
+	for (--index; index >= 0; --index)
+		usb_phy_shutdown(dev, index);
+
+	return ret;
+}
+
+int usb_phys_shutdown(struct udevice *dev)
+{
+	int ret, index, count;
+
+	count = dev_count_phandle_with_args(dev, "phys", "#phy-cells");
+
+	for (index = 0; index < count; ++index) {
+		ret = usb_phy_shutdown(dev, index);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
diff --git a/drivers/usb/host/phy.h b/drivers/usb/host/phy.h
new file mode 100644
index 0000000000..ba3139a714
--- /dev/null
+++ b/drivers/usb/host/phy.h
@@ -0,0 +1,31 @@ 
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * USB phy functions
+ *
+ * Moved from ehci-hcd.c by Marek Behun <marek.behun@nic.cz>
+ *
+ * Copyright (C) Marek Vasut <marex@denx.de>
+ */
+
+#ifndef __USB_HOST_PHY_H_
+#define __USB_HOST_PHY_H_
+
+#include <common.h>
+#include <dm.h>
+
+#ifdef CONFIG_PHY
+int usb_phys_setup(struct udevice *dev);
+int usb_phys_shutdown(struct udevice *dev);
+#else
+static inline int usb_phys_setup(struct udevice *dev)
+{
+	return 0;
+}
+
+static inline int usb_phys_shutdown(struct udevice *dev)
+{
+	return 0;
+}
+#endif
+
+#endif /* __USB_HOST_PHY_H_ */