[Patch v2 00/14] Add power management support for MXS PHY

[Patch v2 00/14] Add power management support for MXS PHY

[Peter Chen]

Hi Felipe,

   The serial adds power management support for MXS PHY, it includes:

   - Add three common PHY API, .set_wakeup, .notify_suspend, notify_resume.
   - Related above API implementation at mxs phy driver
   - misc changes for mxs phy to support low power mode and wakeup.

Changes for v2:

- Add missing patches which introduce platform judgement code [1/14,2/14]
- re-order the patch sequence, like doc->dts->source file, 
the reviewers can know the meaning of dt properties before review source
- Add description of two IC problems exsiting at mxs PHY; change
"high speed" to "HS" and "non-high speed" to "FS/LS" [5/14]
- Change the dt property disconnect-line-without-vbus
that the separator should be "-" not the "_", meanwhile, related source
code has changed.
- Using one local variable to get the function return val to avoid long
condition statement. [13/14]

Peter Chen (14):
  usb: doc: phy-mxs: Add more compatible strings
  usb: phy-mxs: Add platform judgement code
  usb: phy-mxs: Add auto clock and power setting
  usb: doc: phy-mxs: update binding for adding anatop phandle
  ARM: dts: imx6: add anatop phandle for usbphy
  usb: phy-mxs: Add anatop regmap
  usb: phy: add notify suspend and resume callback
  usb: phy-mxs: Add implementation of nofity_suspend and notify_resume
  usb: phy-mxs: Enable IC fixes for mx6 SoC serial
  usb: doc: phy-mxs: update binding for adding disconnect line property
  ARM: dts: imx6: Add disconnect-line-without-vbus property for usbphy
  usb: phy: Add set_wakeup API
  usb: phy-mxs: Add implementation of set_wakeup
  usb: phy-mxs: Add system suspend/resume API

 Documentation/devicetree/bindings/usb/mxs-phy.txt |   12 +-
 arch/arm/boot/dts/imx6qdl.dtsi                    |    3 +
 drivers/usb/phy/phy-mxs-usb.c                     |  328 ++++++++++++++++++++-
 include/linux/usb/phy.h                           |   39 +++
 4 files changed, 367 insertions(+), 15 deletions(-)


--
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 v2 01/14] usb: doc: phy-mxs: Add more compatible strings

[Peter Chen]

Since there are three mxs PHY versions, using three compatible
strings for them.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 Documentation/devicetree/bindings/usb/mxs-phy.txt |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/mxs-phy.txt b/Documentation/devicetree/bindings/usb/mxs-phy.txt
index 5835b27..912d4ad 100644
--- a/Documentation/devicetree/bindings/usb/mxs-phy.txt
+++ b/Documentation/devicetree/bindings/usb/mxs-phy.txt
@@ -1,7 +1,11 @@
 * Freescale MXS USB Phy Device
 
 Required properties:
-- compatible: Should be "fsl,imx23-usbphy"
+- compatible: there are three compatiable strings to stand for three
+mxs PHY verions.
+"fsl,imx23-usbphy": for mx23 and mx28
+"fsl,imx6q-usbphy": for mx6q and mx6dl
+"fsl,imx6sl-usbphy": for mx6sl and later mx6 soc serial
 - reg: Should contain registers location and length
 - interrupts: Should contain phy interrupt
 
-- 
1.7.1

[Patch v2 02/14] usb: phy-mxs: Add platform judgement code

[Peter Chen]

The mxs-phy has three versions until now, each versions have
some differences among PHY operations. the 1st version is
for mx23/mx28 SoC, The 2nd version is for mx6q and mx6dl, the
3rd version is for mx6sl and later mx6 platform.

Signed-off-by: Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
 drivers/usb/phy/phy-mxs-usb.c |   65 ++++++++++++++++++++++++++++++++++++-----
 1 files changed, 57 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
index fdd33b4..a0628d6 100644
--- a/drivers/usb/phy/phy-mxs-usb.c
+++ b/drivers/usb/phy/phy-mxs-usb.c
@@ -1,5 +1,5 @@
 /*
- * Copyright 2012 Freescale Semiconductor, Inc.
+ * Copyright 2012-2013 Freescale Semiconductor, Inc.
  * Copyright (C) 2012 Marek Vasut <marex-ynQEQJNshbs@public.gmane.org>
  * on behalf of DENX Software Engineering GmbH
  *
@@ -20,6 +20,7 @@
 #include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/io.h>
+#include <linux/of_device.h>
 
 #define DRIVER_NAME "mxs_phy"
 
@@ -34,12 +35,57 @@
 #define BM_USBPHY_CTRL_ENUTMILEVEL2		BIT(14)
 #define BM_USBPHY_CTRL_ENHOSTDISCONDETECT	BIT(1)
 
+#define to_mxs_phy(p) container_of((p), struct mxs_phy, phy)
+
+enum imx_phy_type {
+	IMX6Q_USB_PHY,
+	IMX6SL_USB_PHY,
+	IMX23_USB_PHY,
+};
+
 struct mxs_phy {
 	struct usb_phy phy;
 	struct clk *clk;
+	enum imx_phy_type devtype;
 };
 
-#define to_mxs_phy(p) container_of((p), struct mxs_phy, phy)
+static inline int is_mx6q_phy(struct mxs_phy *data)
+{
+	return data->devtype == IMX6Q_USB_PHY;
+}
+
+static inline int is_mx6sl_phy(struct mxs_phy *data)
+{
+	return data->devtype == IMX6SL_USB_PHY;
+}
+
+static inline int is_mx23_phy(struct mxs_phy *data)
+{
+	return data->devtype == IMX23_USB_PHY;
+}
+
+static struct platform_device_id imx_phy_devtype[] = {
+	{
+		.name = "usb-phy-imx6q",
+		.driver_data = IMX6Q_USB_PHY,
+	}, {
+		.name = "usb-phy-imx6sl",
+		.driver_data = IMX6SL_USB_PHY,
+	}, {
+		.name = "usb-phy-imx23",
+		.driver_data = IMX23_USB_PHY,
+	}, {
+		/* sentinel */
+	}
+};
+
+static const struct of_device_id mxs_phy_dt_ids[] = {
+	{ .compatible = "fsl,imx6q-usbphy", .data = &imx_phy_devtype[IMX6Q_USB_PHY], },
+	{ .compatible = "fsl,imx6sl-usbphy", .data = &imx_phy_devtype[IMX6SL_USB_PHY], },
+	{ .compatible = "fsl,imx23-usbphy", .data = &imx_phy_devtype[IMX23_USB_PHY], },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mxs_phy_dt_ids);
 
 static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
 {
@@ -131,6 +177,14 @@ static int mxs_phy_probe(struct platform_device *pdev)
 	struct clk *clk;
 	struct mxs_phy *mxs_phy;
 	int ret;
+	const struct of_device_id *of_id =
+			of_match_device(mxs_phy_dt_ids, &pdev->dev);
+
+	/* This driver is DT-only version now */
+	if (!of_id)
+		return -ENXIO;
+
+	pdev->id_entry = of_id->data;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	base = devm_ioremap_resource(&pdev->dev, res);
@@ -163,6 +217,7 @@ static int mxs_phy_probe(struct platform_device *pdev)
 	ATOMIC_INIT_NOTIFIER_HEAD(&mxs_phy->phy.notifier);
 
 	mxs_phy->clk = clk;
+	mxs_phy->devtype = pdev->id_entry->driver_data;
 
 	platform_set_drvdata(pdev, &mxs_phy->phy);
 
@@ -182,12 +237,6 @@ static int mxs_phy_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static const struct of_device_id mxs_phy_dt_ids[] = {
-	{ .compatible = "fsl,imx23-usbphy", },
-	{ /* sentinel */ }
-};
-MODULE_DEVICE_TABLE(of, mxs_phy_dt_ids);

[Patch v2 03/14] usb: phy-mxs: Add auto clock and power setting

[Peter Chen]

With the auto setting, the PHY's clock and power can be
recovered correctly from low power mode, it is ganranteed by IC logic.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 drivers/usb/phy/phy-mxs-usb.c |   20 +++++++++++++++++---
 1 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
index a0628d6..87ba429 100644
--- a/drivers/usb/phy/phy-mxs-usb.c
+++ b/drivers/usb/phy/phy-mxs-usb.c
@@ -31,6 +31,11 @@
 
 #define BM_USBPHY_CTRL_SFTRST			BIT(31)
 #define BM_USBPHY_CTRL_CLKGATE			BIT(30)
+#define BM_USBPHY_CTRL_ENAUTOSET_USBCLKS	BIT(26)
+#define BM_USBPHY_CTRL_ENAUTOCLR_USBCLKGATE	BIT(25)
+#define BM_USBPHY_CTRL_ENAUTOCLR_PHY_PWD	BIT(20)
+#define BM_USBPHY_CTRL_ENAUTOCLR_CLKGATE	BIT(19)
+#define BM_USBPHY_CTRL_ENAUTO_PWRON_PLL		BIT(18)
 #define BM_USBPHY_CTRL_ENUTMILEVEL3		BIT(15)
 #define BM_USBPHY_CTRL_ENUTMILEVEL2		BIT(14)
 #define BM_USBPHY_CTRL_ENHOSTDISCONDETECT	BIT(1)
@@ -99,9 +104,18 @@ static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
 	/* Power up the PHY */
 	writel(0, base + HW_USBPHY_PWD);
 
-	/* enable FS/LS device */
-	writel(BM_USBPHY_CTRL_ENUTMILEVEL2 |
-	       BM_USBPHY_CTRL_ENUTMILEVEL3,
+	/*
+	 * USB PHY Ctrl Setting
+	 * - Auto clock/power on
+	 * - Enable full/low speed support
+	 */
+	writel(BM_USBPHY_CTRL_ENAUTOSET_USBCLKS |
+		BM_USBPHY_CTRL_ENAUTOCLR_USBCLKGATE |
+		BM_USBPHY_CTRL_ENAUTOCLR_PHY_PWD |
+		BM_USBPHY_CTRL_ENAUTOCLR_CLKGATE |
+		BM_USBPHY_CTRL_ENAUTO_PWRON_PLL |
+		BM_USBPHY_CTRL_ENUTMILEVEL2 |
+		BM_USBPHY_CTRL_ENUTMILEVEL3,
 	       base + HW_USBPHY_CTRL_SET);
 
 	return 0;
-- 
1.7.1

[Patch v2 04/14] usb: doc: phy-mxs: update binding for adding anatop phandle

[Peter Chen]

Add anatop phandle which is used to access anatop registers to
control PHY's power and other USB operations.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 Documentation/devicetree/bindings/usb/mxs-phy.txt |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/mxs-phy.txt b/Documentation/devicetree/bindings/usb/mxs-phy.txt
index 912d4ad..5837963 100644
--- a/Documentation/devicetree/bindings/usb/mxs-phy.txt
+++ b/Documentation/devicetree/bindings/usb/mxs-phy.txt
@@ -8,10 +8,12 @@ mxs PHY verions.
 "fsl,imx6sl-usbphy": for mx6sl and later mx6 soc serial
 - reg: Should contain registers location and length
 - interrupts: Should contain phy interrupt
+- fsl,anatop: phandle for anatop register, it is only for mx6 SoC serial
 
 Example:
 usbphy1: usbphy@020c9000 {
 	compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy";
 	reg = <0x020c9000 0x1000>;
 	interrupts = <0 44 0x04>;
+	fsl,anatop = <&anatop>;
 };
-- 
1.7.1

[Patch v2 05/14] ARM: dts: imx6: add anatop phandle for usbphy

[Peter Chen]

Add anatop phandle for usbphy

Signed-off-by: Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
 arch/arm/boot/dts/imx6qdl.dtsi |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index ef51342..145ece2 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -541,6 +541,7 @@
 				reg = <0x020c9000 0x1000>;
 				interrupts = <0 44 0x04>;
 				clocks = <&clks 182>;
+				fsl,anatop = <&anatop>;
 			};
 
 			usbphy2: usbphy@020ca000 {
@@ -548,6 +549,7 @@
 				reg = <0x020ca000 0x1000>;
 				interrupts = <0 45 0x04>;
 				clocks = <&clks 183>;
+				fsl,anatop = <&anatop>;
 			};
 
 			snvs@020cc000 {
-- 
1.7.1


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

[Patch v2 06/14] usb: phy-mxs: Add anatop regmap

[Peter Chen]

It is needed by mx6 SoC serial, but not for mx23 and mx28.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 drivers/usb/phy/phy-mxs-usb.c |   17 ++++++++++++++++-
 1 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
index 87ba429..6d48bde 100644
--- a/drivers/usb/phy/phy-mxs-usb.c
+++ b/drivers/usb/phy/phy-mxs-usb.c
@@ -21,6 +21,8 @@
 #include <linux/err.h>
 #include <linux/io.h>
 #include <linux/of_device.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
 
 #define DRIVER_NAME "mxs_phy"
 
@@ -52,6 +54,7 @@ struct mxs_phy {
 	struct usb_phy phy;
 	struct clk *clk;
 	enum imx_phy_type devtype;
+	struct regmap *regmap_anatop;
 };
 
 static inline int is_mx6q_phy(struct mxs_phy *data)
@@ -193,9 +196,10 @@ static int mxs_phy_probe(struct platform_device *pdev)
 	int ret;
 	const struct of_device_id *of_id =
 			of_match_device(mxs_phy_dt_ids, &pdev->dev);
+	struct device_node *np = pdev->dev.of_node;
 
 	/* This driver is DT-only version now */
-	if (!of_id)
+	if (!of_id || !np)
 		return -ENXIO;
 
 	pdev->id_entry = of_id->data;
@@ -218,6 +222,17 @@ static int mxs_phy_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	/* Some SoCs don't have anatop registers */
+	if (of_get_property(np, "fsl,anatop", NULL)) {
+		mxs_phy->regmap_anatop = syscon_regmap_lookup_by_phandle
+			(np, "fsl,anatop");
+		if (IS_ERR(mxs_phy->regmap_anatop)) {
+			dev_dbg(&pdev->dev,
+				"failed to find regmap for anatop\n");
+			return PTR_ERR(mxs_phy->regmap_anatop);
+		}
+	}
+
 	mxs_phy->phy.io_priv		= base;
 	mxs_phy->phy.dev		= &pdev->dev;
 	mxs_phy->phy.label		= DRIVER_NAME;
-- 
1.7.1

[Patch v2 07/14] usb: phy: add notify suspend and resume callback

[Peter Chen]

They are used to notify PHY that the controller enters suspend
or finishes resume.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 include/linux/usb/phy.h |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h
index 6c0b1c5..a747960 100644
--- a/include/linux/usb/phy.h
+++ b/include/linux/usb/phy.h
@@ -116,6 +116,11 @@ struct usb_phy {
 			enum usb_device_speed speed);
 	int	(*notify_disconnect)(struct usb_phy *x,
 			enum usb_device_speed speed);
+	int	(*notify_suspend)(struct usb_phy *x,
+			enum usb_device_speed speed);
+	int	(*notify_resume)(struct usb_phy *x,
+			enum usb_device_speed speed);
+
 };
 
 /**
@@ -282,6 +287,24 @@ usb_phy_notify_disconnect(struct usb_phy *x, enum usb_device_speed speed)
 		return 0;
 }
 
+static inline int usb_phy_notify_suspend
+	(struct usb_phy *x, enum usb_device_speed speed)
+{
+	if (x && x->notify_suspend)
+		return x->notify_suspend(x, speed);
+	else
+		return 0;
+}
+
+static inline int usb_phy_notify_resume
+	(struct usb_phy *x, enum usb_device_speed speed)
+{
+	if (x && x->notify_resume)
+		return x->notify_resume(x, speed);
+	else
+		return 0;
+}
+
 /* notifiers */
 static inline int
 usb_register_notifier(struct usb_phy *x, struct notifier_block *nb)
-- 
1.7.1

[Patch v2 08/14] usb: phy-mxs: Add implementation of nofity_suspend and notify_resume

[Peter Chen]

Add notify_suspend and notify_resume according to different SoCs.

Signed-off-by: Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
 drivers/usb/phy/phy-mxs-usb.c |   91 +++++++++++++++++++++++++++++++++++++++--
 1 files changed, 87 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
index 6d48bde..4416ed6 100644
--- a/drivers/usb/phy/phy-mxs-usb.c
+++ b/drivers/usb/phy/phy-mxs-usb.c
@@ -164,8 +164,8 @@ static int mxs_phy_suspend(struct usb_phy *x, int suspend)
 static int mxs_phy_on_connect(struct usb_phy *phy,
 		enum usb_device_speed speed)
 {
-	dev_dbg(phy->dev, "%s speed device has connected\n",
-		(speed == USB_SPEED_HIGH) ? "high" : "non-high");
+	dev_dbg(phy->dev, "%s device has connected\n",
+		(speed == USB_SPEED_HIGH) ? "HS" : "FS/LS");
 
 	if (speed == USB_SPEED_HIGH)
 		writel(BM_USBPHY_CTRL_ENHOSTDISCONDETECT,
@@ -177,8 +177,8 @@ static int mxs_phy_on_connect(struct usb_phy *phy,
 static int mxs_phy_on_disconnect(struct usb_phy *phy,
 		enum usb_device_speed speed)
 {
-	dev_dbg(phy->dev, "%s speed device has disconnected\n",
-		(speed == USB_SPEED_HIGH) ? "high" : "non-high");
+	dev_dbg(phy->dev, "%s device has disconnected\n",
+		(speed == USB_SPEED_HIGH) ? "HS" : "FS/LS");
 
 	if (speed == USB_SPEED_HIGH)
 		writel(BM_USBPHY_CTRL_ENHOSTDISCONDETECT,
@@ -187,6 +187,88 @@ static int mxs_phy_on_disconnect(struct usb_phy *phy,
 	return 0;
 }
 
+static int mxs_phy_on_suspend_workaround(struct usb_phy *phy,
+		enum usb_device_speed speed)
+{
+	dev_dbg(phy->dev, "%s device has suspended\n",
+		(speed == USB_SPEED_HIGH) ? "HS" : "FS/LS");
+
+	/* delay 4ms to wait bus entering idle */
+	usleep_range(4000, 5000);
+
+	/*
+	 * Workaround for wakeup signal between portsc.suspendM
+	 * and PHY enters low power mode.
+	 */
+	writel_relaxed(0xffffffff, phy->io_priv + HW_USBPHY_PWD);
+	writel_relaxed(0, phy->io_priv + HW_USBPHY_PWD);
+
+	if (speed == USB_SPEED_HIGH)
+		writel_relaxed(BM_USBPHY_CTRL_ENHOSTDISCONDETECT,
+				phy->io_priv + HW_USBPHY_CTRL_CLR);
+
+	return 0;
+}
+
+static int mxs_phy_on_suspend(struct usb_phy *phy,
+		enum usb_device_speed speed)
+{
+	dev_dbg(phy->dev, "%s device has suspended\n",
+		(speed == USB_SPEED_HIGH) ? "HS" : "FS/LS");
+
+	/* delay 4ms to wait bus entering idle */
+	usleep_range(4000, 5000);
+
+	if (speed == USB_SPEED_HIGH)
+		writel_relaxed(BM_USBPHY_CTRL_ENHOSTDISCONDETECT,
+				phy->io_priv + HW_USBPHY_CTRL_CLR);
+
+	return 0;
+}
+
+/*
+ * The resume signal must be finished here.
+ */
+static int mxs_phy_on_resume(struct usb_phy *phy,
+		enum usb_device_speed speed)
+{
+	dev_dbg(phy->dev, "%s device has resumed\n",
+		(speed == USB_SPEED_HIGH) ? "HS" : "FS/LS");
+
+	if (speed == USB_SPEED_HIGH) {
+		/* Make sure the device has switched to High-Speed mode */
+		udelay(500);
+		writel_relaxed(BM_USBPHY_CTRL_ENHOSTDISCONDETECT,
+				phy->io_priv + HW_USBPHY_CTRL_SET);
+	}
+
+	return 0;
+}
+
+/*
+ * For mxs PHY, there are two PHY issues related to suspend/resume.
+ *
+ * 1. The PHY will be in messy if there is an wakeup after putting
+ * bus to suspend (set portsc.suspendM) but before setting PHY to low
+ * power mode (set portsc.phcd).
+ * 2. The SOF sends too fast after resuming, it will cause disconnection
+ * between host and high speed device.
+ *
+ * For mx23 and mx28, both of two issues are existed.
+ * For mx6q and mx6dl, only the second issue is existed.
+ * For mx6 sololite and later SoCs, none issue is existed.
+ */
+static void mxs_phy_workaround(struct mxs_phy *mxs_phy)
+{
+	if (is_mx23_phy(mxs_phy)) {
+		mxs_phy->phy.notify_suspend = mxs_phy_on_suspend_workaround;
+		mxs_phy->phy.notify_resume = mxs_phy_on_resume;
+	} else if (is_mx6q_phy(mxs_phy)) {
+		mxs_phy->phy.notify_suspend = mxs_phy_on_suspend;
+		mxs_phy->phy.notify_resume = mxs_phy_on_resume;
+	}
+}
+
 static int mxs_phy_probe(struct platform_device *pdev)
 {
 	struct resource *res;
@@ -247,6 +329,7 @@ static int mxs_phy_probe(struct platform_device *pdev)
 
 	mxs_phy->clk = clk;
 	mxs_phy->devtype = pdev->id_entry->driver_data;
+	mxs_phy_workaround(mxs_phy);
 
 	platform_set_drvdata(pdev, &mxs_phy->phy);
 
-- 
1.7.1


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

[Patch v2 09/14] usb: phy-mxs: Enable IC fixes for mx6 SoC serial

[Peter Chen]

After adding IC fixes bits, some PHY bugs are fixed by
IC logic.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 drivers/usb/phy/phy-mxs-usb.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
index 4416ed6..af2a9cf 100644
--- a/drivers/usb/phy/phy-mxs-usb.c
+++ b/drivers/usb/phy/phy-mxs-usb.c
@@ -31,6 +31,10 @@
 #define HW_USBPHY_CTRL_SET			0x34
 #define HW_USBPHY_CTRL_CLR			0x38
 
+#define HW_USBPHY_IP				0x90
+#define HW_USBPHY_IP_SET			0x94
+#define HW_USBPHY_IP_CLR			0x98
+
 #define BM_USBPHY_CTRL_SFTRST			BIT(31)
 #define BM_USBPHY_CTRL_CLKGATE			BIT(30)
 #define BM_USBPHY_CTRL_ENAUTOSET_USBCLKS	BIT(26)
@@ -42,6 +46,8 @@
 #define BM_USBPHY_CTRL_ENUTMILEVEL2		BIT(14)
 #define BM_USBPHY_CTRL_ENHOSTDISCONDETECT	BIT(1)
 
+#define BM_USBPHY_IP_FIX                       (BIT(17) | BIT(18))
+
 #define to_mxs_phy(p) container_of((p), struct mxs_phy, phy)
 
 enum imx_phy_type {
@@ -121,6 +127,10 @@ static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
 		BM_USBPHY_CTRL_ENUTMILEVEL3,
 	       base + HW_USBPHY_CTRL_SET);
 
+	/* Enable IC solution */
+	if (is_mx6q_phy(mxs_phy) || is_mx6sl_phy(mxs_phy))
+		writel(BM_USBPHY_IP_FIX, base + HW_USBPHY_IP_SET);
+
 	return 0;
 }
 
-- 
1.7.1

[Patch v2 10/14] usb: doc: phy-mxs: update binding for adding disconnect line property

[Peter Chen]

This property is used to disconnect line between USB PHY and
USB controller.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 Documentation/devicetree/bindings/usb/mxs-phy.txt |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/mxs-phy.txt b/Documentation/devicetree/bindings/usb/mxs-phy.txt
index 5837963..356c9b6 100644
--- a/Documentation/devicetree/bindings/usb/mxs-phy.txt
+++ b/Documentation/devicetree/bindings/usb/mxs-phy.txt
@@ -9,6 +9,9 @@ mxs PHY verions.
 - reg: Should contain registers location and length
 - interrupts: Should contain phy interrupt
 - fsl,anatop: phandle for anatop register, it is only for mx6 SoC serial
+- disconnect-line-without-vbus: needs to disconnect
+connection between USB PHY and controller, it can avoid
+unexpected wakeup interrupt when the PHY is out of power
 
 Example:
 usbphy1: usbphy@020c9000 {
@@ -16,4 +19,5 @@ usbphy1: usbphy@020c9000 {
 	reg = <0x020c9000 0x1000>;
 	interrupts = <0 44 0x04>;
 	fsl,anatop = <&anatop>;
+	disconnect-line-without-vbus;
 };
-- 
1.7.1

[Patch v2 11/14] ARM: dts: imx6: Add disconnect-line-without-vbus property for usbphy

[Peter Chen]

When the vbus is not there and the PHY is out of power, we need to
disconnect connection between USB PHY and USB controller, otherwise
the unexpected USB wakeup will occur due to dp/dm change at device
mode.

Signed-off-by: Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
 arch/arm/boot/dts/imx6qdl.dtsi |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 145ece2..89eeda7 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -542,6 +542,7 @@
 				interrupts = <0 44 0x04>;
 				clocks = <&clks 182>;
 				fsl,anatop = <&anatop>;
+				disconnect-line-without-vbus;
 			};
 
 			usbphy2: usbphy@020ca000 {
-- 
1.7.1


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

[Patch v2 12/14] usb: phy: Add set_wakeup API

[Peter Chen]

This API is used to set wakeup enable at PHY registers, in that
case, the PHY can be waken up from suspend due to external events,
like vbus change, dp/dm change and id change.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 include/linux/usb/phy.h |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h
index a747960..c6ebe1d 100644
--- a/include/linux/usb/phy.h
+++ b/include/linux/usb/phy.h
@@ -111,6 +111,13 @@ struct usb_phy {
 	int	(*set_suspend)(struct usb_phy *x,
 				int suspend);
 
+	/*
+	 * Set wakeup enable for PHY, in that case, the PHY can be
+	 * waken up from suspend status due to external events,
+	 * like vbus change, dp/dm change and id.
+	 */
+	int	(*set_wakeup)(struct usb_phy *x, bool enabled);
+
 	/* notify phy connect status change */
 	int	(*notify_connect)(struct usb_phy *x,
 			enum usb_device_speed speed);
@@ -270,6 +277,15 @@ usb_phy_set_suspend(struct usb_phy *x, int suspend)
 }
 
 static inline int
+usb_phy_set_wakeup(struct usb_phy *x, bool enabled)
+{
+	if (x && x->set_wakeup)
+		return x->set_wakeup(x, enabled);
+	else
+		return 0;
+}
+
+static inline int
 usb_phy_notify_connect(struct usb_phy *x, enum usb_device_speed speed)
 {
 	if (x && x->notify_connect)
-- 
1.7.1

[Patch v2 13/14] usb: phy-mxs: Add implementation of set_wakeup

[Peter Chen]

When we need the PHY can be waken up by external signals,
we can call this API. Besides, we call mxs_phy_disconnect_line
at this API to close the connection between USB PHY and
controller, after that, the line state from controller is SE0.
Once the PHY is out of power, without calling mxs_phy_disconnect_line,
there are unknown wakeups due to dp/dm floating at device mode.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 drivers/usb/phy/phy-mxs-usb.c |   84 ++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 83 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
index af2a9cf..5bd53ec 100644
--- a/drivers/usb/phy/phy-mxs-usb.c
+++ b/drivers/usb/phy/phy-mxs-usb.c
@@ -31,6 +31,9 @@
 #define HW_USBPHY_CTRL_SET			0x34
 #define HW_USBPHY_CTRL_CLR			0x38
 
+#define HW_USBPHY_DEBUG_SET			0x54
+#define HW_USBPHY_DEBUG_CLR			0x58
+
 #define HW_USBPHY_IP				0x90
 #define HW_USBPHY_IP_SET			0x94
 #define HW_USBPHY_IP_CLR			0x98
@@ -39,6 +42,9 @@
 #define BM_USBPHY_CTRL_CLKGATE			BIT(30)
 #define BM_USBPHY_CTRL_ENAUTOSET_USBCLKS	BIT(26)
 #define BM_USBPHY_CTRL_ENAUTOCLR_USBCLKGATE	BIT(25)
+#define BM_USBPHY_CTRL_ENVBUSCHG_WKUP		BIT(23)
+#define BM_USBPHY_CTRL_ENIDCHG_WKUP		BIT(22)
+#define BM_USBPHY_CTRL_ENDPDMCHG_WKUP		BIT(21)
 #define BM_USBPHY_CTRL_ENAUTOCLR_PHY_PWD	BIT(20)
 #define BM_USBPHY_CTRL_ENAUTOCLR_CLKGATE	BIT(19)
 #define BM_USBPHY_CTRL_ENAUTO_PWRON_PLL		BIT(18)
@@ -46,7 +52,20 @@
 #define BM_USBPHY_CTRL_ENUTMILEVEL2		BIT(14)
 #define BM_USBPHY_CTRL_ENHOSTDISCONDETECT	BIT(1)
 
-#define BM_USBPHY_IP_FIX                       (BIT(17) | BIT(18))
+#define BM_USBPHY_IP_FIX			(BIT(17) | BIT(18))
+
+#define BM_USBPHY_DEBUG_CLKGATE			BIT(30)
+
+/* Anatop Registers */
+#define ANADIG_USB1_VBUS_DET_STAT		0x1c0
+
+#define ANADIG_USB1_LOOPBACK_SET		0x1e4
+#define ANADIG_USB1_LOOPBACK_CLR		0x1e8
+
+#define BM_ANADIG_USB1_VBUS_DET_STAT_VBUS_VALID	BIT(3)
+
+#define BM_ANADIG_USB1_LOOPBACK_UTMI_DIG_TST1	BIT(2)
+#define BM_ANADIG_USB1_LOOPBACK_TSTI_TX_EN	BIT(5)
 
 #define to_mxs_phy(p) container_of((p), struct mxs_phy, phy)
 
@@ -61,6 +80,7 @@ struct mxs_phy {
 	struct clk *clk;
 	enum imx_phy_type devtype;
 	struct regmap *regmap_anatop;
+	bool disconnect_line_without_vbus_is_needed;
 };
 
 static inline int is_mx6q_phy(struct mxs_phy *data)
@@ -134,6 +154,44 @@ static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
 	return 0;
 }
 
+static void mxs_phy_disconnect_line(struct mxs_phy *mxs_phy, bool on)
+{
+	void __iomem *base = mxs_phy->phy.io_priv;
+	bool vbus_is_on = false;
+	static bool line_is_disconnected;
+	unsigned int vbus_value = 0;
+
+	/* Only the SoCs have anatop need below operation */
+	if (!mxs_phy->disconnect_line_without_vbus_is_needed)
+		return;
+
+	regmap_read(mxs_phy->regmap_anatop, ANADIG_USB1_VBUS_DET_STAT,
+			&vbus_value);
+	if (vbus_value & BM_ANADIG_USB1_VBUS_DET_STAT_VBUS_VALID)
+		vbus_is_on = true;
+
+	if (on && !vbus_is_on) {
+		writel_relaxed(BM_USBPHY_DEBUG_CLKGATE,
+			base + HW_USBPHY_DEBUG_CLR);
+		regmap_write(mxs_phy->regmap_anatop, ANADIG_USB1_LOOPBACK_SET,
+				BM_ANADIG_USB1_LOOPBACK_UTMI_DIG_TST1 |
+				BM_ANADIG_USB1_LOOPBACK_TSTI_TX_EN);
+		/* Delay some time, and let Linestate be SE0 for controller */
+		usleep_range(500, 1000);
+		line_is_disconnected = true;
+	} else if (line_is_disconnected) {
+		regmap_write(mxs_phy->regmap_anatop, ANADIG_USB1_LOOPBACK_CLR,
+				BM_ANADIG_USB1_LOOPBACK_UTMI_DIG_TST1 |
+				BM_ANADIG_USB1_LOOPBACK_TSTI_TX_EN);
+		writel_relaxed(BM_USBPHY_DEBUG_CLKGATE,
+				base + HW_USBPHY_DEBUG_SET);
+		line_is_disconnected = false;
+	}
+
+	dev_dbg(mxs_phy->phy.dev, "line is %s\n", line_is_disconnected
+			? "disconnected" : "connected");
+}
+
 static int mxs_phy_init(struct usb_phy *phy)
 {
 	struct mxs_phy *mxs_phy = to_mxs_phy(phy);
@@ -171,6 +229,23 @@ static int mxs_phy_suspend(struct usb_phy *x, int suspend)
 	return 0;
 }
 
+static int mxs_phy_set_wakeup(struct usb_phy *x, bool enabled)
+{
+	struct mxs_phy *mxs_phy = to_mxs_phy(x);
+	u32 value = BM_USBPHY_CTRL_ENVBUSCHG_WKUP |
+			BM_USBPHY_CTRL_ENDPDMCHG_WKUP |
+				BM_USBPHY_CTRL_ENIDCHG_WKUP;
+	if (enabled) {
+		mxs_phy_disconnect_line(mxs_phy, true);
+		writel_relaxed(value, x->io_priv + HW_USBPHY_CTRL_SET);
+	} else {
+		writel_relaxed(value, x->io_priv + HW_USBPHY_CTRL_CLR);
+		mxs_phy_disconnect_line(mxs_phy, false);
+	}
+
+	return 0;
+}
+
 static int mxs_phy_on_connect(struct usb_phy *phy,
 		enum usb_device_speed speed)
 {
@@ -289,6 +364,7 @@ static int mxs_phy_probe(struct platform_device *pdev)
 	const struct of_device_id *of_id =
 			of_match_device(mxs_phy_dt_ids, &pdev->dev);
 	struct device_node *np = pdev->dev.of_node;
+	struct property *disconnect_property;
 
 	/* This driver is DT-only version now */
 	if (!of_id || !np)
@@ -325,6 +401,11 @@ static int mxs_phy_probe(struct platform_device *pdev)
 		}
 	}
 
+	disconnect_property = of_find_property
+		(np, "disconnect-line-without-vbus", NULL);
+	if (disconnect_property && mxs_phy->regmap_anatop)
+		mxs_phy->disconnect_line_without_vbus_is_needed = true;
+
 	mxs_phy->phy.io_priv		= base;
 	mxs_phy->phy.dev		= &pdev->dev;
 	mxs_phy->phy.label		= DRIVER_NAME;
@@ -334,6 +415,7 @@ static int mxs_phy_probe(struct platform_device *pdev)
 	mxs_phy->phy.notify_connect	= mxs_phy_on_connect;
 	mxs_phy->phy.notify_disconnect	= mxs_phy_on_disconnect;
 	mxs_phy->phy.type		= USB_PHY_TYPE_USB2;
+	mxs_phy->phy.set_wakeup		= mxs_phy_set_wakeup;
 
 	ATOMIC_INIT_NOTIFIER_HEAD(&mxs_phy->phy.notifier);
 
-- 
1.7.1

[Patch v2 14/14] usb: phy-mxs: Add system suspend/resume API

[Peter Chen]

We need this to keep PHY's power on or off during the system
suspend mode. If we need to enable USB wakeup, then we
must keep PHY's power being on during the system suspend mode.
Otherwise, we need to keep PHY's power being off to save power.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 drivers/usb/phy/phy-mxs-usb.c |   47 +++++++++++++++++++++++++++++++++++++++++
 1 files changed, 47 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
index 5bd53ec..d150736 100644
--- a/drivers/usb/phy/phy-mxs-usb.c
+++ b/drivers/usb/phy/phy-mxs-usb.c
@@ -57,11 +57,18 @@
 #define BM_USBPHY_DEBUG_CLKGATE			BIT(30)
 
 /* Anatop Registers */
+#define ANADIG_ANA_MISC0			0x150
+#define ANADIG_ANA_MISC0_SET			0x154
+#define ANADIG_ANA_MISC0_CLR			0x158
+
 #define ANADIG_USB1_VBUS_DET_STAT		0x1c0
 
 #define ANADIG_USB1_LOOPBACK_SET		0x1e4
 #define ANADIG_USB1_LOOPBACK_CLR		0x1e8
 
+#define BM_ANADIG_ANA_MISC0_STOP_MODE_CONFIG	BIT(12)
+#define BM_ANADIG_ANA_MISC0_STOP_MODE_CONFIG_SL BIT(11)
+
 #define BM_ANADIG_USB1_VBUS_DET_STAT_VBUS_VALID	BIT(3)
 
 #define BM_ANADIG_USB1_LOOPBACK_UTMI_DIG_TST1	BIT(2)
@@ -192,6 +199,22 @@ static void mxs_phy_disconnect_line(struct mxs_phy *mxs_phy, bool on)
 			? "disconnected" : "connected");
 }
 
+static void mxs_phy_enable_ldo_in_suspend(struct mxs_phy *mxs_phy, bool on)
+{
+	unsigned int reg = on ? ANADIG_ANA_MISC0_SET : ANADIG_ANA_MISC0_CLR;
+
+	/* Only the SoCs have anatop need below operation */
+	if (!mxs_phy->regmap_anatop)
+		return;
+
+	if (is_mx6q_phy(mxs_phy))
+		regmap_write(mxs_phy->regmap_anatop, reg,
+			BM_ANADIG_ANA_MISC0_STOP_MODE_CONFIG);
+	else if (is_mx6sl_phy(mxs_phy))
+		regmap_write(mxs_phy->regmap_anatop,
+			reg, BM_ANADIG_ANA_MISC0_STOP_MODE_CONFIG_SL);
+}
+
 static int mxs_phy_init(struct usb_phy *phy)
 {
 	struct mxs_phy *mxs_phy = to_mxs_phy(phy);
@@ -425,6 +448,7 @@ static int mxs_phy_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, &mxs_phy->phy);
 
+	device_set_wakeup_capable(&pdev->dev, true);
 	ret = usb_add_phy_dev(&mxs_phy->phy);
 	if (ret)
 		return ret;
@@ -441,6 +465,28 @@ static int mxs_phy_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static int mxs_phy_system_suspend(struct device *dev)
+{
+	struct mxs_phy *mxs_phy = dev_get_drvdata(dev);
+
+	if (device_may_wakeup(dev))
+		mxs_phy_enable_ldo_in_suspend(mxs_phy, true);
+
+	return 0;
+}
+
+static int mxs_phy_system_resume(struct device *dev)
+{
+	struct mxs_phy *mxs_phy = dev_get_drvdata(dev);
+
+	if (device_may_wakeup(dev))
+		mxs_phy_enable_ldo_in_suspend(mxs_phy, false);
+
+	return 0;
+}
+
+SIMPLE_DEV_PM_OPS(mxs_phy_pm, mxs_phy_system_suspend, mxs_phy_system_resume);
+
 static struct platform_driver mxs_phy_driver = {
 	.probe = mxs_phy_probe,
 	.remove = mxs_phy_remove,
@@ -448,6 +494,7 @@ static struct platform_driver mxs_phy_driver = {
 		.name = DRIVER_NAME,
 		.owner	= THIS_MODULE,
 		.of_match_table = mxs_phy_dt_ids,
+		.pm = &mxs_phy_pm,
 	 },
 };
 
-- 
1.7.1

Re: [Patch v2 00/14] Add power management support for MXS PHY

[Peter Chen]

Hi Marek,

I have tried this patchset as well as my chipidea runtime pm patchset
at imx28 evk, it works ok except below two problems:

- Without CONFIG_MFD_SYSCON causes build error, fixed by below patches
http://marc.info/?l=linux-arm-kernel&m=138243289017143&w=2
- Unbalanced IRQ wake disable kernel warning when system resume if
enabled wakeup setting , it may mx28 platform problem, I will discuss
with shawn about it.

On Tue, Oct 22, 2013 at 01:58:34PM +0800, Peter Chen wrote:
> Hi Felipe,
> 
>    The serial adds power management support for MXS PHY, it includes:
> 
>    - Add three common PHY API, .set_wakeup, .notify_suspend, notify_resume.
>    - Related above API implementation at mxs phy driver
>    - misc changes for mxs phy to support low power mode and wakeup.
> 
> Changes for v2:
> 
> - Add missing patches which introduce platform judgement code [1/14,2/14]
> - re-order the patch sequence, like doc->dts->source file, 
> the reviewers can know the meaning of dt properties before review source
> - Add description of two IC problems exsiting at mxs PHY; change
> "high speed" to "HS" and "non-high speed" to "FS/LS" [5/14]
> - Change the dt property disconnect-line-without-vbus
> that the separator should be "-" not the "_", meanwhile, related source
> code has changed.
> - Using one local variable to get the function return val to avoid long
> condition statement. [13/14]
> 
> Peter Chen (14):
>   usb: doc: phy-mxs: Add more compatible strings
>   usb: phy-mxs: Add platform judgement code
>   usb: phy-mxs: Add auto clock and power setting
>   usb: doc: phy-mxs: update binding for adding anatop phandle
>   ARM: dts: imx6: add anatop phandle for usbphy
>   usb: phy-mxs: Add anatop regmap
>   usb: phy: add notify suspend and resume callback
>   usb: phy-mxs: Add implementation of nofity_suspend and notify_resume
>   usb: phy-mxs: Enable IC fixes for mx6 SoC serial
>   usb: doc: phy-mxs: update binding for adding disconnect line property
>   ARM: dts: imx6: Add disconnect-line-without-vbus property for usbphy
>   usb: phy: Add set_wakeup API
>   usb: phy-mxs: Add implementation of set_wakeup
>   usb: phy-mxs: Add system suspend/resume API
> 
>  Documentation/devicetree/bindings/usb/mxs-phy.txt |   12 +-
>  arch/arm/boot/dts/imx6qdl.dtsi                    |    3 +
>  drivers/usb/phy/phy-mxs-usb.c                     |  328 ++++++++++++++++++++-
>  include/linux/usb/phy.h                           |   39 +++
>  4 files changed, 367 insertions(+), 15 deletions(-)
> 

-- 

Best Regards,
Peter Chen

Re: [Patch v2 01/14] usb: doc: phy-mxs: Add more compatible strings

[Shawn Guo]

On Tue, Oct 22, 2013 at 01:58:35PM +0800, Peter Chen wrote:
> Since there are three mxs PHY versions, using three compatible
> strings for them.
> 
> Signed-off-by: Peter Chen <peter.chen@freescale.com>
> ---
>  Documentation/devicetree/bindings/usb/mxs-phy.txt |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/mxs-phy.txt b/Documentation/devicetree/bindings/usb/mxs-phy.txt
> index 5835b27..912d4ad 100644
> --- a/Documentation/devicetree/bindings/usb/mxs-phy.txt
> +++ b/Documentation/devicetree/bindings/usb/mxs-phy.txt
> @@ -1,7 +1,11 @@
>  * Freescale MXS USB Phy Device
>  
>  Required properties:
> -- compatible: Should be "fsl,imx23-usbphy"
> +- compatible: there are three compatiable strings to stand for three

s/compatiable/compatible

> +mxs PHY verions.

s/verions/versions

> +"fsl,imx23-usbphy": for mx23 and mx28
> +"fsl,imx6q-usbphy": for mx6q and mx6dl
> +"fsl,imx6sl-usbphy": for mx6sl and later mx6 soc serial

s/serial/series

Shawn

>  - reg: Should contain registers location and length
>  - interrupts: Should contain phy interrupt
>  
> -- 
> 1.7.1
> 
> 

Re: [Patch v2 01/14] usb: doc: phy-mxs: Add more compatible strings

[Shawn Guo]

On Tue, Oct 22, 2013 at 01:58:35PM +0800, Peter Chen wrote:
> Since there are three mxs PHY versions, using three compatible
> strings for them.
> 
> Signed-off-by: Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/usb/mxs-phy.txt |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/mxs-phy.txt b/Documentation/devicetree/bindings/usb/mxs-phy.txt
> index 5835b27..912d4ad 100644
> --- a/Documentation/devicetree/bindings/usb/mxs-phy.txt
> +++ b/Documentation/devicetree/bindings/usb/mxs-phy.txt
> @@ -1,7 +1,11 @@
>  * Freescale MXS USB Phy Device
>  
>  Required properties:
> -- compatible: Should be "fsl,imx23-usbphy"
> +- compatible: there are three compatiable strings to stand for three
> +mxs PHY verions.
> +"fsl,imx23-usbphy": for mx23 and mx28
> +"fsl,imx6q-usbphy": for mx6q and mx6dl
> +"fsl,imx6sl-usbphy": for mx6sl and later mx6 soc serial

Also, please use string 'imx' rather than 'mx' for consistency.

Shawn

>  - reg: Should contain registers location and length
>  - interrupts: Should contain phy interrupt
>  
> -- 
> 1.7.1
> 
> 

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

Re: [Patch v2 02/14] usb: phy-mxs: Add platform judgement code

[Shawn Guo]

On Tue, Oct 22, 2013 at 01:58:36PM +0800, Peter Chen wrote:
> The mxs-phy has three versions until now, each versions have
> some differences among PHY operations. the 1st version is
> for mx23/mx28 SoC, The 2nd version is for mx6q and mx6dl, the
> 3rd version is for mx6sl and later mx6 platform.
> 
> Signed-off-by: Peter Chen <peter.chen@freescale.com>
> ---
>  drivers/usb/phy/phy-mxs-usb.c |   65 ++++++++++++++++++++++++++++++++++++-----
>  1 files changed, 57 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
> index fdd33b4..a0628d6 100644
> --- a/drivers/usb/phy/phy-mxs-usb.c
> +++ b/drivers/usb/phy/phy-mxs-usb.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright 2012 Freescale Semiconductor, Inc.
> + * Copyright 2012-2013 Freescale Semiconductor, Inc.
>   * Copyright (C) 2012 Marek Vasut <marex@denx.de>
>   * on behalf of DENX Software Engineering GmbH
>   *
> @@ -20,6 +20,7 @@
>  #include <linux/delay.h>
>  #include <linux/err.h>
>  #include <linux/io.h>
> +#include <linux/of_device.h>
>  
>  #define DRIVER_NAME "mxs_phy"
>  
> @@ -34,12 +35,57 @@
>  #define BM_USBPHY_CTRL_ENUTMILEVEL2		BIT(14)
>  #define BM_USBPHY_CTRL_ENHOSTDISCONDETECT	BIT(1)
>  
> +#define to_mxs_phy(p) container_of((p), struct mxs_phy, phy)
> +
> +enum imx_phy_type {
> +	IMX6Q_USB_PHY,
> +	IMX6SL_USB_PHY,
> +	IMX23_USB_PHY,
> +};
> +
>  struct mxs_phy {
>  	struct usb_phy phy;
>  	struct clk *clk;
> +	enum imx_phy_type devtype;
>  };
>  
> -#define to_mxs_phy(p) container_of((p), struct mxs_phy, phy)
> +static inline int is_mx6q_phy(struct mxs_phy *data)
> +{
> +	return data->devtype == IMX6Q_USB_PHY;
> +}
> +
> +static inline int is_mx6sl_phy(struct mxs_phy *data)
> +{
> +	return data->devtype == IMX6SL_USB_PHY;
> +}
> +
> +static inline int is_mx23_phy(struct mxs_phy *data)
> +{
> +	return data->devtype == IMX23_USB_PHY;
> +}
> +
> +static struct platform_device_id imx_phy_devtype[] = {
> +	{
> +		.name = "usb-phy-imx6q",
> +		.driver_data = IMX6Q_USB_PHY,
> +	}, {
> +		.name = "usb-phy-imx6sl",
> +		.driver_data = IMX6SL_USB_PHY,
> +	}, {
> +		.name = "usb-phy-imx23",
> +		.driver_data = IMX23_USB_PHY,
> +	}, {
> +		/* sentinel */
> +	}
> +};

I know many imx device drivers have this platform_device_id table, but
that's because they need to support both non-DT and DT probe.  Since
this driver supports DT probe only, we can save this table by passing
imx_phy_type value through of_device_id.data directly.

> +static const struct of_device_id mxs_phy_dt_ids[] = {
> +	{ .compatible = "fsl,imx6q-usbphy", .data = &imx_phy_devtype[IMX6Q_USB_PHY], },
> +	{ .compatible = "fsl,imx6sl-usbphy", .data = &imx_phy_devtype[IMX6SL_USB_PHY], },
> +	{ .compatible = "fsl,imx23-usbphy", .data = &imx_phy_devtype[IMX23_USB_PHY], },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, mxs_phy_dt_ids);
>  
>  static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
>  {
> @@ -131,6 +177,14 @@ static int mxs_phy_probe(struct platform_device *pdev)
>  	struct clk *clk;
>  	struct mxs_phy *mxs_phy;
>  	int ret;
> +	const struct of_device_id *of_id =
> +			of_match_device(mxs_phy_dt_ids, &pdev->dev);
> +
> +	/* This driver is DT-only version now */
> +	if (!of_id)
> +		return -ENXIO;

Since it's DT-only, I'm not sure you will run into the case that
mxs_phy_probe() is called with a NULL of_id.  The check looks
unnecessary to me.

> +
> +	pdev->id_entry = of_id->data;

Some imx device drivers did the same thing, but we should keep
pdev->id_entry immutable.  The removal of that platform_device_id table
will help save this.

Shawn

>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	base = devm_ioremap_resource(&pdev->dev, res);
> @@ -163,6 +217,7 @@ static int mxs_phy_probe(struct platform_device *pdev)
>  	ATOMIC_INIT_NOTIFIER_HEAD(&mxs_phy->phy.notifier);
>  
>  	mxs_phy->clk = clk;
> +	mxs_phy->devtype = pdev->id_entry->driver_data;
>  
>  	platform_set_drvdata(pdev, &mxs_phy->phy);
>  
> @@ -182,12 +237,6 @@ static int mxs_phy_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -static const struct of_device_id mxs_phy_dt_ids[] = {
> -	{ .compatible = "fsl,imx23-usbphy", },
> -	{ /* sentinel */ }
> -};
> -MODULE_DEVICE_TABLE(of, mxs_phy_dt_ids);
> -
>  static struct platform_driver mxs_phy_driver = {
>  	.probe = mxs_phy_probe,
>  	.remove = mxs_phy_remove,
> -- 
> 1.7.1
> 
> 

Re: [Patch v2 01/14] usb: doc: phy-mxs: Add more compatible strings

[Peter Chen]

On Wed, Oct 23, 2013 at 01:56:00PM +0800, Shawn Guo wrote:
> On Tue, Oct 22, 2013 at 01:58:35PM +0800, Peter Chen wrote:
> > Since there are three mxs PHY versions, using three compatible
> > strings for them.
> > 
> > Signed-off-by: Peter Chen <peter.chen@freescale.com>
> > ---
> >  Documentation/devicetree/bindings/usb/mxs-phy.txt |    6 +++++-
> >  1 files changed, 5 insertions(+), 1 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/usb/mxs-phy.txt b/Documentation/devicetree/bindings/usb/mxs-phy.txt
> > index 5835b27..912d4ad 100644
> > --- a/Documentation/devicetree/bindings/usb/mxs-phy.txt
> > +++ b/Documentation/devicetree/bindings/usb/mxs-phy.txt
> > @@ -1,7 +1,11 @@
> >  * Freescale MXS USB Phy Device
> >  
> >  Required properties:
> > -- compatible: Should be "fsl,imx23-usbphy"
> > +- compatible: there are three compatiable strings to stand for three
> > +mxs PHY verions.
> > +"fsl,imx23-usbphy": for mx23 and mx28
> > +"fsl,imx6q-usbphy": for mx6q and mx6dl
> > +"fsl,imx6sl-usbphy": for mx6sl and later mx6 soc serial
> 
> Also, please use string 'imx' rather than 'mx' for consistency.
> 

Thanks, will change your two comments.

-- 

Best Regards,
Peter Chen

Re: [Patch v2 10/14] usb: doc: phy-mxs: update binding for adding disconnect line property

[Shawn Guo]

On Tue, Oct 22, 2013 at 01:58:44PM +0800, Peter Chen wrote:
> This property is used to disconnect line between USB PHY and
> USB controller.
> 
> Signed-off-by: Peter Chen <peter.chen@freescale.com>
> ---
>  Documentation/devicetree/bindings/usb/mxs-phy.txt |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/mxs-phy.txt b/Documentation/devicetree/bindings/usb/mxs-phy.txt
> index 5837963..356c9b6 100644
> --- a/Documentation/devicetree/bindings/usb/mxs-phy.txt
> +++ b/Documentation/devicetree/bindings/usb/mxs-phy.txt
> @@ -9,6 +9,9 @@ mxs PHY verions.
>  - reg: Should contain registers location and length
>  - interrupts: Should contain phy interrupt
>  - fsl,anatop: phandle for anatop register, it is only for mx6 SoC serial
> +- disconnect-line-without-vbus: needs to disconnect
> +connection between USB PHY and controller, it can avoid
> +unexpected wakeup interrupt when the PHY is out of power

If it's a mxs-phy specific property, we may need to have vendor prefix
'fsl,' for it.

I see you set the property in imx6qdl.dtsi.  Is it true that we should
always set it for imx6q/dl?  If yes, why cannot we have the driver to
force it for imx6q type of phy.  If no, where do we need to set it and
where not?

Shawn

>  
>  Example:
>  usbphy1: usbphy@020c9000 {
> @@ -16,4 +19,5 @@ usbphy1: usbphy@020c9000 {
>  	reg = <0x020c9000 0x1000>;
>  	interrupts = <0 44 0x04>;
>  	fsl,anatop = <&anatop>;
> +	disconnect-line-without-vbus;
>  };
> -- 
> 1.7.1
> 
> 

Re: [Patch v2 13/14] usb: phy-mxs: Add implementation of set_wakeup

[Shawn Guo]

On Tue, Oct 22, 2013 at 01:58:47PM +0800, Peter Chen wrote:
> When we need the PHY can be waken up by external signals,
> we can call this API. Besides, we call mxs_phy_disconnect_line
> at this API to close the connection between USB PHY and
> controller, after that, the line state from controller is SE0.
> Once the PHY is out of power, without calling mxs_phy_disconnect_line,
> there are unknown wakeups due to dp/dm floating at device mode.
> 
> Signed-off-by: Peter Chen <peter.chen@freescale.com>
> ---
>  drivers/usb/phy/phy-mxs-usb.c |   84 ++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 83 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
> index af2a9cf..5bd53ec 100644
> --- a/drivers/usb/phy/phy-mxs-usb.c
> +++ b/drivers/usb/phy/phy-mxs-usb.c
> @@ -31,6 +31,9 @@
>  #define HW_USBPHY_CTRL_SET			0x34
>  #define HW_USBPHY_CTRL_CLR			0x38
>  
> +#define HW_USBPHY_DEBUG_SET			0x54
> +#define HW_USBPHY_DEBUG_CLR			0x58
> +
>  #define HW_USBPHY_IP				0x90
>  #define HW_USBPHY_IP_SET			0x94
>  #define HW_USBPHY_IP_CLR			0x98
> @@ -39,6 +42,9 @@
>  #define BM_USBPHY_CTRL_CLKGATE			BIT(30)
>  #define BM_USBPHY_CTRL_ENAUTOSET_USBCLKS	BIT(26)
>  #define BM_USBPHY_CTRL_ENAUTOCLR_USBCLKGATE	BIT(25)
> +#define BM_USBPHY_CTRL_ENVBUSCHG_WKUP		BIT(23)
> +#define BM_USBPHY_CTRL_ENIDCHG_WKUP		BIT(22)
> +#define BM_USBPHY_CTRL_ENDPDMCHG_WKUP		BIT(21)
>  #define BM_USBPHY_CTRL_ENAUTOCLR_PHY_PWD	BIT(20)
>  #define BM_USBPHY_CTRL_ENAUTOCLR_CLKGATE	BIT(19)
>  #define BM_USBPHY_CTRL_ENAUTO_PWRON_PLL		BIT(18)
> @@ -46,7 +52,20 @@
>  #define BM_USBPHY_CTRL_ENUTMILEVEL2		BIT(14)
>  #define BM_USBPHY_CTRL_ENHOSTDISCONDETECT	BIT(1)
>  
> -#define BM_USBPHY_IP_FIX                       (BIT(17) | BIT(18))
> +#define BM_USBPHY_IP_FIX			(BIT(17) | BIT(18))
> +
> +#define BM_USBPHY_DEBUG_CLKGATE			BIT(30)
> +
> +/* Anatop Registers */
> +#define ANADIG_USB1_VBUS_DET_STAT		0x1c0
> +
> +#define ANADIG_USB1_LOOPBACK_SET		0x1e4
> +#define ANADIG_USB1_LOOPBACK_CLR		0x1e8
> +
> +#define BM_ANADIG_USB1_VBUS_DET_STAT_VBUS_VALID	BIT(3)
> +
> +#define BM_ANADIG_USB1_LOOPBACK_UTMI_DIG_TST1	BIT(2)
> +#define BM_ANADIG_USB1_LOOPBACK_TSTI_TX_EN	BIT(5)
>  
>  #define to_mxs_phy(p) container_of((p), struct mxs_phy, phy)
>  
> @@ -61,6 +80,7 @@ struct mxs_phy {
>  	struct clk *clk;
>  	enum imx_phy_type devtype;
>  	struct regmap *regmap_anatop;
> +	bool disconnect_line_without_vbus_is_needed;
>  };
>  
>  static inline int is_mx6q_phy(struct mxs_phy *data)
> @@ -134,6 +154,44 @@ static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
>  	return 0;
>  }
>  
> +static void mxs_phy_disconnect_line(struct mxs_phy *mxs_phy, bool on)
> +{
> +	void __iomem *base = mxs_phy->phy.io_priv;
> +	bool vbus_is_on = false;
> +	static bool line_is_disconnected;
> +	unsigned int vbus_value = 0;
> +
> +	/* Only the SoCs have anatop need below operation */
> +	if (!mxs_phy->disconnect_line_without_vbus_is_needed)
> +		return;

As per the comment, shouldn't the if-clause be the following?  Or is the
comment incorrect?

	if (!mxs_phy->regmap_anatop)

Shawn

> +
> +	regmap_read(mxs_phy->regmap_anatop, ANADIG_USB1_VBUS_DET_STAT,
> +			&vbus_value);
> +	if (vbus_value & BM_ANADIG_USB1_VBUS_DET_STAT_VBUS_VALID)
> +		vbus_is_on = true;
> +
> +	if (on && !vbus_is_on) {
> +		writel_relaxed(BM_USBPHY_DEBUG_CLKGATE,
> +			base + HW_USBPHY_DEBUG_CLR);
> +		regmap_write(mxs_phy->regmap_anatop, ANADIG_USB1_LOOPBACK_SET,
> +				BM_ANADIG_USB1_LOOPBACK_UTMI_DIG_TST1 |
> +				BM_ANADIG_USB1_LOOPBACK_TSTI_TX_EN);
> +		/* Delay some time, and let Linestate be SE0 for controller */
> +		usleep_range(500, 1000);
> +		line_is_disconnected = true;
> +	} else if (line_is_disconnected) {
> +		regmap_write(mxs_phy->regmap_anatop, ANADIG_USB1_LOOPBACK_CLR,
> +				BM_ANADIG_USB1_LOOPBACK_UTMI_DIG_TST1 |
> +				BM_ANADIG_USB1_LOOPBACK_TSTI_TX_EN);
> +		writel_relaxed(BM_USBPHY_DEBUG_CLKGATE,
> +				base + HW_USBPHY_DEBUG_SET);
> +		line_is_disconnected = false;
> +	}
> +
> +	dev_dbg(mxs_phy->phy.dev, "line is %s\n", line_is_disconnected
> +			? "disconnected" : "connected");
> +}
> +
>  static int mxs_phy_init(struct usb_phy *phy)
>  {
>  	struct mxs_phy *mxs_phy = to_mxs_phy(phy);
> @@ -171,6 +229,23 @@ static int mxs_phy_suspend(struct usb_phy *x, int suspend)
>  	return 0;
>  }
>  
> +static int mxs_phy_set_wakeup(struct usb_phy *x, bool enabled)
> +{
> +	struct mxs_phy *mxs_phy = to_mxs_phy(x);
> +	u32 value = BM_USBPHY_CTRL_ENVBUSCHG_WKUP |
> +			BM_USBPHY_CTRL_ENDPDMCHG_WKUP |
> +				BM_USBPHY_CTRL_ENIDCHG_WKUP;
> +	if (enabled) {
> +		mxs_phy_disconnect_line(mxs_phy, true);
> +		writel_relaxed(value, x->io_priv + HW_USBPHY_CTRL_SET);
> +	} else {
> +		writel_relaxed(value, x->io_priv + HW_USBPHY_CTRL_CLR);
> +		mxs_phy_disconnect_line(mxs_phy, false);
> +	}
> +
> +	return 0;
> +}
> +
>  static int mxs_phy_on_connect(struct usb_phy *phy,
>  		enum usb_device_speed speed)
>  {
> @@ -289,6 +364,7 @@ static int mxs_phy_probe(struct platform_device *pdev)
>  	const struct of_device_id *of_id =
>  			of_match_device(mxs_phy_dt_ids, &pdev->dev);
>  	struct device_node *np = pdev->dev.of_node;
> +	struct property *disconnect_property;
>  
>  	/* This driver is DT-only version now */
>  	if (!of_id || !np)
> @@ -325,6 +401,11 @@ static int mxs_phy_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +	disconnect_property = of_find_property
> +		(np, "disconnect-line-without-vbus", NULL);
> +	if (disconnect_property && mxs_phy->regmap_anatop)
> +		mxs_phy->disconnect_line_without_vbus_is_needed = true;
> +
>  	mxs_phy->phy.io_priv		= base;
>  	mxs_phy->phy.dev		= &pdev->dev;
>  	mxs_phy->phy.label		= DRIVER_NAME;
> @@ -334,6 +415,7 @@ static int mxs_phy_probe(struct platform_device *pdev)
>  	mxs_phy->phy.notify_connect	= mxs_phy_on_connect;
>  	mxs_phy->phy.notify_disconnect	= mxs_phy_on_disconnect;
>  	mxs_phy->phy.type		= USB_PHY_TYPE_USB2;
> +	mxs_phy->phy.set_wakeup		= mxs_phy_set_wakeup;
>  
>  	ATOMIC_INIT_NOTIFIER_HEAD(&mxs_phy->phy.notifier);
>  
> -- 
> 1.7.1
> 
> 

Re: [Patch v2 02/14] usb: phy-mxs: Add platform judgement code

[Peter Chen]

On Wed, Oct 23, 2013 at 02:13:24PM +0800, Shawn Guo wrote:
> > +
> > +enum imx_phy_type {
> > +	IMX6Q_USB_PHY,
> > +	IMX6SL_USB_PHY,
> > +	IMX23_USB_PHY,
> > +};
> > +
> >  struct mxs_phy {
> >  	struct usb_phy phy;
> >  	struct clk *clk;
> > +	enum imx_phy_type devtype;
> >  };
> >  
> > -#define to_mxs_phy(p) container_of((p), struct mxs_phy, phy)
> > +static inline int is_mx6q_phy(struct mxs_phy *data)
> > +{
> > +	return data->devtype == IMX6Q_USB_PHY;
> > +}
> > +
> > +static inline int is_mx6sl_phy(struct mxs_phy *data)
> > +{
> > +	return data->devtype == IMX6SL_USB_PHY;
> > +}
> > +
> > +static inline int is_mx23_phy(struct mxs_phy *data)
> > +{
> > +	return data->devtype == IMX23_USB_PHY;
> > +}
> > +
> > +static struct platform_device_id imx_phy_devtype[] = {
> > +	{
> > +		.name = "usb-phy-imx6q",
> > +		.driver_data = IMX6Q_USB_PHY,
> > +	}, {
> > +		.name = "usb-phy-imx6sl",
> > +		.driver_data = IMX6SL_USB_PHY,
> > +	}, {
> > +		.name = "usb-phy-imx23",
> > +		.driver_data = IMX23_USB_PHY,
> > +	}, {
> > +		/* sentinel */
> > +	}
> > +};
> 
> I know many imx device drivers have this platform_device_id table, but
> that's because they need to support both non-DT and DT probe.  Since
> this driver supports DT probe only, we can save this table by passing
> imx_phy_type value through of_device_id.data directly.

How about compare compatible string directly at probe?

	if (of_device_is_compatible(np, "fsl,imx6q-usbphy"))
		mxs_phy->devtype = IMX6Q_USB_PHY;
	else if ((of_device_is_compatible(np, "fsl,imx6sl-usbphy"))
		mxs_phy->devtype = IMX6SL_USB_PHY;
	else if (...)
		...;

I don't know how to passing imx_phy_type value through below table
directly? imx_phy_type is a enum variable, not a arrary.

> 
> > +static const struct of_device_id mxs_phy_dt_ids[] = {
> > +	{ .compatible = "fsl,imx6q-usbphy", .data = &imx_phy_devtype[IMX6Q_USB_PHY], },
> > +	{ .compatible = "fsl,imx6sl-usbphy", .data = &imx_phy_devtype[IMX6SL_USB_PHY], },
> > +	{ .compatible = "fsl,imx23-usbphy", .data = &imx_phy_devtype[IMX23_USB_PHY], },
> > +	{ /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, mxs_phy_dt_ids);
> >  
> >  static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
> >  {
> > @@ -131,6 +177,14 @@ static int mxs_phy_probe(struct platform_device *pdev)
> >  	struct clk *clk;
> >  	struct mxs_phy *mxs_phy;
> >  	int ret;
> > +	const struct of_device_id *of_id =
> > +			of_match_device(mxs_phy_dt_ids, &pdev->dev);
> > +
> > +	/* This driver is DT-only version now */
> > +	if (!of_id)
> > +		return -ENXIO;
> 
> Since it's DT-only, I'm not sure you will run into the case that
> mxs_phy_probe() is called with a NULL of_id.  The check looks
> unnecessary to me.

Will change.

> 
> > +
> > +	pdev->id_entry = of_id->data;
> 
> Some imx device drivers did the same thing, but we should keep
> pdev->id_entry immutable.  The removal of that platform_device_id table
> will help save this.

Surely we can delete this line if no platform_device_id table.

-- 

Best Regards,
Peter Chen

Re: [Patch v2 10/14] usb: doc: phy-mxs: update binding for adding disconnect line property

[Peter Chen]

On Wed, Oct 23, 2013 at 02:31:56PM +0800, Shawn Guo wrote:
> On Tue, Oct 22, 2013 at 01:58:44PM +0800, Peter Chen wrote:
> > This property is used to disconnect line between USB PHY and
> > USB controller.
> > 
> > Signed-off-by: Peter Chen <peter.chen@freescale.com>
> > ---
> >  Documentation/devicetree/bindings/usb/mxs-phy.txt |    4 ++++
> >  1 files changed, 4 insertions(+), 0 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/usb/mxs-phy.txt b/Documentation/devicetree/bindings/usb/mxs-phy.txt
> > index 5837963..356c9b6 100644
> > --- a/Documentation/devicetree/bindings/usb/mxs-phy.txt
> > +++ b/Documentation/devicetree/bindings/usb/mxs-phy.txt
> > @@ -9,6 +9,9 @@ mxs PHY verions.
> >  - reg: Should contain registers location and length
> >  - interrupts: Should contain phy interrupt
> >  - fsl,anatop: phandle for anatop register, it is only for mx6 SoC serial
> > +- disconnect-line-without-vbus: needs to disconnect
> > +connection between USB PHY and controller, it can avoid
> > +unexpected wakeup interrupt when the PHY is out of power
> 
> If it's a mxs-phy specific property, we may need to have vendor prefix
> 'fsl,' for it.

It is mx6 version mxs-phy specific, I will add prefix "imx6"

> 
> I see you set the property in imx6qdl.dtsi.  Is it true that we should
> always set it for imx6q/dl?  If yes, why cannot we have the driver to
> force it for imx6q type of phy.  If no, where do we need to set it and
> where not?
> 

Yes, it is needed for current imx6dq, imx6dl, and imx6sl SoC.


-- 

Best Regards,
Peter Chen

Re: [Patch v2 13/14] usb: phy-mxs: Add implementation of set_wakeup

[Peter Chen]

On Wed, Oct 23, 2013 at 02:35:23PM +0800, Shawn Guo wrote:
> >  
> > +static void mxs_phy_disconnect_line(struct mxs_phy *mxs_phy, bool on)
> > +{
> > +	void __iomem *base = mxs_phy->phy.io_priv;
> > +	bool vbus_is_on = false;
> > +	static bool line_is_disconnected;
> > +	unsigned int vbus_value = 0;
> > +
> > +	/* Only the SoCs have anatop need below operation */
> > +	if (!mxs_phy->disconnect_line_without_vbus_is_needed)
> > +		return;
> 
> As per the comment, shouldn't the if-clause be the following?  Or is the
> comment incorrect?
> 
> 	if (!mxs_phy->regmap_anatop)
> 
> Shawn
> 

Thanks,

I will change the comment to 
	/* If the SoC doesn't need to disconnect line without vbus, quit */

-- 

Best Regards,
Peter Chen

Re: [Patch v2 10/14] usb: doc: phy-mxs: update binding for adding disconnect line property

[Peter Chen]

On Wed, Oct 23, 2013 at 02:31:56PM +0800, Shawn Guo wrote:
> On Tue, Oct 22, 2013 at 01:58:44PM +0800, Peter Chen wrote:
> > This property is used to disconnect line between USB PHY and
> > USB controller.
> > 
> > Signed-off-by: Peter Chen <peter.chen@freescale.com>
> > ---
> >  Documentation/devicetree/bindings/usb/mxs-phy.txt |    4 ++++
> >  1 files changed, 4 insertions(+), 0 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/usb/mxs-phy.txt b/Documentation/devicetree/bindings/usb/mxs-phy.txt
> > index 5837963..356c9b6 100644
> > --- a/Documentation/devicetree/bindings/usb/mxs-phy.txt
> > +++ b/Documentation/devicetree/bindings/usb/mxs-phy.txt
> > @@ -9,6 +9,9 @@ mxs PHY verions.
> >  - reg: Should contain registers location and length
> >  - interrupts: Should contain phy interrupt
> >  - fsl,anatop: phandle for anatop register, it is only for mx6 SoC serial
> > +- disconnect-line-without-vbus: needs to disconnect
> > +connection between USB PHY and controller, it can avoid
> > +unexpected wakeup interrupt when the PHY is out of power
> 
> If it's a mxs-phy specific property, we may need to have vendor prefix
> 'fsl,' for it.
> 
> I see you set the property in imx6qdl.dtsi.  Is it true that we should
> always set it for imx6q/dl?  If yes, why cannot we have the driver to
> force it for imx6q type of phy.  If no, where do we need to set it and
> where not?
> 

One more thing: it is a SoC feature, so I can delete it from DT, and
using compatible string (or of_device_id.data) to judge if it is needed
at driver, do you think so?

-- 

Best Regards,
Peter Chen

Re: [Patch v2 10/14] usb: doc: phy-mxs: update binding for adding disconnect line property

[Shawn Guo]

On Wed, Oct 23, 2013 at 03:08:25PM +0800, Peter Chen wrote:
> One more thing: it is a SoC feature, so I can delete it from DT, and
> using compatible string (or of_device_id.data) to judge if it is needed
> at driver, do you think so?

That's exactly my point - it's not a thing belonging to device tree.

Shawn

Re: [Patch v2 02/14] usb: phy-mxs: Add platform judgement code

[Peter Chen]

On Wed, Oct 23, 2013 at 05:04:44PM +0800, Shawn Guo wrote:
> On Wed, Oct 23, 2013 at 02:46:04PM +0800, Peter Chen wrote:
> > How about compare compatible string directly at probe?
> > 
> > 	if (of_device_is_compatible(np, "fsl,imx6q-usbphy"))
> > 		mxs_phy->devtype = IMX6Q_USB_PHY;
> > 	else if ((of_device_is_compatible(np, "fsl,imx6sl-usbphy"))
> > 		mxs_phy->devtype = IMX6SL_USB_PHY;
> > 	else if (...)
> > 		...;
> 
> It can work, but in general, we should avoid unnecessary device tree
> lookup.  I would suggest something like below.
> 
> #define MXS_FLAGS_SUSPEND_ISSUE_1	BIT(0)
> #define MXS_FLAGS_SUSPEND_ISSUE_2	BIT(1)
> #define MXS_FLAGS_LINE_DISCONNECT	BIT(2)
> 
> struct mxs_phy_data {
> 	unsigned int flags;
> };
> 
> struct mxs_phy {
> 	...
> 	mxs_phy_data *data;
> };
> 
> static struct mxs_phy_data imx6sl_usbphy_data = {
>         .flags = MXS_FLAGS_LINE_DISCONNECT,
> };
> 
> static struct mxs_phy_data imx6q_usbphy_data = {
>         .flags = MXS_FLAGS_SUSPEND_ISSUE_2 | MXS_FLAGS_LINE_DISCONNECT,
> };
> 
> static struct mxs_phy_data imx23_usbphy_data = {
>         .flags = MXS_FLAGS_SUSPEND_ISSUE_1 | MXS_FLAGS_SUSPEND_ISSUE_2,
> };
> 
> static const struct of_device_id mxs_phy_dt_ids[] = {
>        { .compatible = "fsl,imx6sl-usbphy", .data = &imx6sl_usbphy_data, },
>        { .compatible = "fsl,imx6q-usbphy", .data = &imx6q_usbphy_data, },
>        { .compatible = "fsl,imx23-usbphy", .data = &imx23_usbphy_data, },
>        { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, mxs_phy_dt_ids);
> 
> Then you can check the flags for handling different cases.  This would
> be more flexible and future proof.
> 

Great, I will use that way at chipidea driver too.

-- 

Best Regards,
Peter Chen

Re: [Patch v2 02/14] usb: phy-mxs: Add platform judgement code

[Shawn Guo]

On Wed, Oct 23, 2013 at 02:46:04PM +0800, Peter Chen wrote:
> How about compare compatible string directly at probe?
> 
> 	if (of_device_is_compatible(np, "fsl,imx6q-usbphy"))
> 		mxs_phy->devtype = IMX6Q_USB_PHY;
> 	else if ((of_device_is_compatible(np, "fsl,imx6sl-usbphy"))
> 		mxs_phy->devtype = IMX6SL_USB_PHY;
> 	else if (...)
> 		...;

It can work, but in general, we should avoid unnecessary device tree
lookup.  I would suggest something like below.

#define MXS_FLAGS_SUSPEND_ISSUE_1	BIT(0)
#define MXS_FLAGS_SUSPEND_ISSUE_2	BIT(1)
#define MXS_FLAGS_LINE_DISCONNECT	BIT(2)

struct mxs_phy_data {
	unsigned int flags;
};

struct mxs_phy {
	...
	mxs_phy_data *data;
};

static struct mxs_phy_data imx6sl_usbphy_data = {
        .flags = MXS_FLAGS_LINE_DISCONNECT,
};

static struct mxs_phy_data imx6q_usbphy_data = {
        .flags = MXS_FLAGS_SUSPEND_ISSUE_2 | MXS_FLAGS_LINE_DISCONNECT,
};

static struct mxs_phy_data imx23_usbphy_data = {
        .flags = MXS_FLAGS_SUSPEND_ISSUE_1 | MXS_FLAGS_SUSPEND_ISSUE_2,
};

static const struct of_device_id mxs_phy_dt_ids[] = {
       { .compatible = "fsl,imx6sl-usbphy", .data = &imx6sl_usbphy_data, },
       { .compatible = "fsl,imx6q-usbphy", .data = &imx6q_usbphy_data, },
       { .compatible = "fsl,imx23-usbphy", .data = &imx23_usbphy_data, },
       { /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, mxs_phy_dt_ids);

Then you can check the flags for handling different cases.  This would
be more flexible and future proof.

Shawn

Re: [Patch v2 00/14] Add power management support for MXS PHY

[Peter Chen]

On Tue, Oct 22, 2013 at 01:58:34PM +0800, Peter Chen wrote:
> Hi Felipe,

Hi Felipe, would you please give any comments for this patchset?
My another patchset chipidea's PM support also depends on this.
Thanks.

Peter

> 
>    The serial adds power management support for MXS PHY, it includes:
> 
>    - Add three common PHY API, .set_wakeup, .notify_suspend, notify_resume.
>    - Related above API implementation at mxs phy driver
>    - misc changes for mxs phy to support low power mode and wakeup.
> 
> Changes for v2:
> 
> - Add missing patches which introduce platform judgement code [1/14,2/14]
> - re-order the patch sequence, like doc->dts->source file, 
> the reviewers can know the meaning of dt properties before review source
> - Add description of two IC problems exsiting at mxs PHY; change
> "high speed" to "HS" and "non-high speed" to "FS/LS" [5/14]
> - Change the dt property disconnect-line-without-vbus
> that the separator should be "-" not the "_", meanwhile, related source
> code has changed.
> - Using one local variable to get the function return val to avoid long
> condition statement. [13/14]
> 
> Peter Chen (14):
>   usb: doc: phy-mxs: Add more compatible strings
>   usb: phy-mxs: Add platform judgement code
>   usb: phy-mxs: Add auto clock and power setting
>   usb: doc: phy-mxs: update binding for adding anatop phandle
>   ARM: dts: imx6: add anatop phandle for usbphy
>   usb: phy-mxs: Add anatop regmap
>   usb: phy: add notify suspend and resume callback
>   usb: phy-mxs: Add implementation of nofity_suspend and notify_resume
>   usb: phy-mxs: Enable IC fixes for mx6 SoC serial
>   usb: doc: phy-mxs: update binding for adding disconnect line property
>   ARM: dts: imx6: Add disconnect-line-without-vbus property for usbphy
>   usb: phy: Add set_wakeup API
>   usb: phy-mxs: Add implementation of set_wakeup
>   usb: phy-mxs: Add system suspend/resume API
> 
>  Documentation/devicetree/bindings/usb/mxs-phy.txt |   12 +-
>  arch/arm/boot/dts/imx6qdl.dtsi                    |    3 +
>  drivers/usb/phy/phy-mxs-usb.c                     |  328 ++++++++++++++++++++-
>  include/linux/usb/phy.h                           |   39 +++
>  4 files changed, 367 insertions(+), 15 deletions(-)
> 

-- 

Best Regards,
Peter Chen