[PATCH 0/8] USB: PHY: nop: Device tree support for 3.10

[PATCH 0/8] USB: PHY: nop: Device tree support for 3.10

[Roger Quadros]

Hi Felipe,

These patches add device tree support as well as PHY resource handling
(i.e. clock, reset, power) for the NOP transceiver driver.

Please add these patches before your patches that move/rename the
drivers/usb/otg/nop-usb-xceiv.c file.

NOTE: The first patch that changes platform header must be common between
you and Tony's OMAP tree. Maybe you guys can share an immutable branch?

cheers,
-roger

The following changes since commit f6161aa153581da4a3867a2d1a7caf4be19b6ec9:

  Linux 3.9-rc2 (2013-03-10 16:54:19 -0700)

are available in the git repository at:
  git://github.com/rogerq/linux.git usbhost-phy-next

Roger Quadros (8):
      usb: phy: nop: Add some parameters to platform data
      usb: phy: nop: use devm_kzalloc()
      usb: phy: nop: Manage PHY clock
      usb: phy: nop: Handle power supply regulator for the PHY
      usb: phy: nop: Handle RESET for the PHY
      usb: phy: nop: use new PHY API to register PHY
      usb: phy: nop: Add device tree support and binding information
      USB: phy: nop: Defer probe if device needs VCC/RESET

 .../devicetree/bindings/usb/usb-nop-xceiv.txt      |   34 +++++
 drivers/usb/otg/nop-usb-xceiv.c                    |  143 +++++++++++++++++--
 include/linux/usb/nop-usb-xceiv.h                  |    5 +
 3 files changed, 167 insertions(+), 15 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt

-- 
1.7.4.1

[PATCH 1/8] usb: phy: nop: Add some parameters to platform data

[Roger Quadros]

Add clk_rate parameter to platform data. If supplied, the
NOP phy driver will program the clock to that rate during probe.

Also add 2 flags, needs_vcc and needs_reset.
If the flag is set and the regulator couldn't be found
then the driver will bail out with -EPROBE_DEFER.

Signed-off-by: Roger Quadros <rogerq@ti.com>
Acked-by: Felipe Balbi <balbi@ti.com>
---
 include/linux/usb/nop-usb-xceiv.h |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/include/linux/usb/nop-usb-xceiv.h b/include/linux/usb/nop-usb-xceiv.h
index 28884c7..148d351 100644
--- a/include/linux/usb/nop-usb-xceiv.h
+++ b/include/linux/usb/nop-usb-xceiv.h
@@ -5,6 +5,11 @@
 
 struct nop_usb_xceiv_platform_data {
 	enum usb_phy_type type;
+	unsigned long clk_rate;
+
+	/* if set fails with -EPROBE_DEFER if can't get regulator */
+	unsigned int needs_vcc:1;
+	unsigned int needs_reset:1;
 };
 
 #if defined(CONFIG_NOP_USB_XCEIV) || (defined(CONFIG_NOP_USB_XCEIV_MODULE) && defined(MODULE))
-- 
1.7.4.1

[PATCH 2/8] usb: phy: nop: use devm_kzalloc()

[Roger Quadros]

Use resource managed kzalloc.

Signed-off-by: Roger Quadros <rogerq@ti.com>
Acked-by: Felipe Balbi <balbi@ti.com>
---
 drivers/usb/otg/nop-usb-xceiv.c |   17 +++++------------
 1 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/otg/nop-usb-xceiv.c b/drivers/usb/otg/nop-usb-xceiv.c
index a3ce24b..af52870 100644
--- a/drivers/usb/otg/nop-usb-xceiv.c
+++ b/drivers/usb/otg/nop-usb-xceiv.c
@@ -100,15 +100,14 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev)
 	enum usb_phy_type	type = USB_PHY_TYPE_USB2;
 	int err;
 
-	nop = kzalloc(sizeof *nop, GFP_KERNEL);
+	nop = devm_kzalloc(&pdev->dev, sizeof(*nop), GFP_KERNEL);
 	if (!nop)
 		return -ENOMEM;
 
-	nop->phy.otg = kzalloc(sizeof *nop->phy.otg, GFP_KERNEL);
-	if (!nop->phy.otg) {
-		kfree(nop);
+	nop->phy.otg = devm_kzalloc(&pdev->dev, sizeof(*nop->phy.otg),
+							GFP_KERNEL);
+	if (!nop->phy.otg)
 		return -ENOMEM;
-	}
 
 	if (pdata)
 		type = pdata->type;
@@ -127,7 +126,7 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev)
 	if (err) {
 		dev_err(&pdev->dev, "can't register transceiver, err: %d\n",
 			err);
-		goto exit;
+		return err;
 	}
 
 	platform_set_drvdata(pdev, nop);
@@ -135,10 +134,6 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev)
 	ATOMIC_INIT_NOTIFIER_HEAD(&nop->phy.notifier);
 
 	return 0;
-exit:
-	kfree(nop->phy.otg);
-	kfree(nop);
-	return err;
 }
 
 static int nop_usb_xceiv_remove(struct platform_device *pdev)
@@ -148,8 +143,6 @@ static int nop_usb_xceiv_remove(struct platform_device *pdev)
 	usb_remove_phy(&nop->phy);
 
 	platform_set_drvdata(pdev, NULL);
-	kfree(nop->phy.otg);
-	kfree(nop);
 
 	return 0;
 }
-- 
1.7.4.1

[PATCH 3/8] usb: phy: nop: Manage PHY clock

[Roger Quadros]

If the PHY has a clock associated to it then manage the clock.
We just enable the clock in .init() and disable it in .shutdown().

Add clk_rate parameter in platform data and configure the
clock rate during probe if supplied.

Signed-off-by: Roger Quadros <rogerq@ti.com>
Acked-by: Felipe Balbi <balbi@ti.com>
---
 drivers/usb/otg/nop-usb-xceiv.c |   54 ++++++++++++++++++++++++++++++++++++++-
 1 files changed, 53 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/otg/nop-usb-xceiv.c b/drivers/usb/otg/nop-usb-xceiv.c
index af52870..17c174f 100644
--- a/drivers/usb/otg/nop-usb-xceiv.c
+++ b/drivers/usb/otg/nop-usb-xceiv.c
@@ -32,10 +32,12 @@
 #include <linux/usb/otg.h>
 #include <linux/usb/nop-usb-xceiv.h>
 #include <linux/slab.h>
+#include <linux/clk.h>
 
 struct nop_usb_xceiv {
 	struct usb_phy		phy;
 	struct device		*dev;
+	struct clk		*clk;
 };
 
 static struct platform_device *pd;
@@ -64,6 +66,24 @@ static int nop_set_suspend(struct usb_phy *x, int suspend)
 	return 0;
 }
 
+static int nop_init(struct usb_phy *phy)
+{
+	struct nop_usb_xceiv *nop = dev_get_drvdata(phy->dev);
+
+	if (!IS_ERR(nop->clk))
+		clk_enable(nop->clk);
+
+	return 0;
+}
+
+static void nop_shutdown(struct usb_phy *phy)
+{
+	struct nop_usb_xceiv *nop = dev_get_drvdata(phy->dev);
+
+	if (!IS_ERR(nop->clk))
+		clk_disable(nop->clk);
+}
+
 static int nop_set_peripheral(struct usb_otg *otg, struct usb_gadget *gadget)
 {
 	if (!otg)
@@ -112,10 +132,34 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev)
 	if (pdata)
 		type = pdata->type;
 
+	nop->clk = devm_clk_get(&pdev->dev, "main_clk");
+	if (IS_ERR(nop->clk)) {
+		dev_dbg(&pdev->dev, "Can't get phy clock: %ld\n",
+					PTR_ERR(nop->clk));
+	}
+
+	if (!IS_ERR(nop->clk) && pdata && pdata->clk_rate) {
+		err = clk_set_rate(nop->clk, pdata->clk_rate);
+		if (err) {
+			dev_err(&pdev->dev, "Error setting clock rate\n");
+			return err;
+		}
+	}
+
+	if (!IS_ERR(nop->clk)) {
+		err = clk_prepare(nop->clk);
+		if (err) {
+			dev_err(&pdev->dev, "Error preparing clock\n");
+			return err;
+		}
+	}
+
 	nop->dev		= &pdev->dev;
 	nop->phy.dev		= nop->dev;
 	nop->phy.label		= "nop-xceiv";
 	nop->phy.set_suspend	= nop_set_suspend;
+	nop->phy.init		= nop_init;
+	nop->phy.shutdown	= nop_shutdown;
 	nop->phy.state		= OTG_STATE_UNDEFINED;
 
 	nop->phy.otg->phy		= &nop->phy;
@@ -126,7 +170,7 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev)
 	if (err) {
 		dev_err(&pdev->dev, "can't register transceiver, err: %d\n",
 			err);
-		return err;
+		goto err_add;
 	}
 
 	platform_set_drvdata(pdev, nop);
@@ -134,12 +178,20 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev)
 	ATOMIC_INIT_NOTIFIER_HEAD(&nop->phy.notifier);
 
 	return 0;
+
+err_add:
+	if (!IS_ERR(nop->clk))
+		clk_unprepare(nop->clk);
+	return err;
 }
 
 static int nop_usb_xceiv_remove(struct platform_device *pdev)
 {
 	struct nop_usb_xceiv *nop = platform_get_drvdata(pdev);
 
+	if (!IS_ERR(nop->clk))
+		clk_unprepare(nop->clk);
+
 	usb_remove_phy(&nop->phy);
 
 	platform_set_drvdata(pdev, NULL);
-- 
1.7.4.1

[PATCH 4/8] usb: phy: nop: Handle power supply regulator for the PHY

[Roger Quadros]

We use "vcc" as the supply name for the PHY's power supply.
The power supply will be enabled during .init() and disabled
during .shutdown()

Signed-off-by: Roger Quadros <rogerq@ti.com>
Acked-by: Felipe Balbi <balbi@ti.com>
---
 drivers/usb/otg/nop-usb-xceiv.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/otg/nop-usb-xceiv.c b/drivers/usb/otg/nop-usb-xceiv.c
index 17c174f..fbdcfef 100644
--- a/drivers/usb/otg/nop-usb-xceiv.c
+++ b/drivers/usb/otg/nop-usb-xceiv.c
@@ -33,11 +33,13 @@
 #include <linux/usb/nop-usb-xceiv.h>
 #include <linux/slab.h>
 #include <linux/clk.h>
+#include <linux/regulator/consumer.h>
 
 struct nop_usb_xceiv {
 	struct usb_phy		phy;
 	struct device		*dev;
 	struct clk		*clk;
+	struct regulator	*vcc;
 };
 
 static struct platform_device *pd;
@@ -70,6 +72,11 @@ static int nop_init(struct usb_phy *phy)
 {
 	struct nop_usb_xceiv *nop = dev_get_drvdata(phy->dev);
 
+	if (!IS_ERR(nop->vcc)) {
+		if (regulator_enable(nop->vcc))
+			dev_err(phy->dev, "Failed to enable power\n");
+	}
+
 	if (!IS_ERR(nop->clk))
 		clk_enable(nop->clk);
 
@@ -82,6 +89,11 @@ static void nop_shutdown(struct usb_phy *phy)
 
 	if (!IS_ERR(nop->clk))
 		clk_disable(nop->clk);
+
+	if (!IS_ERR(nop->vcc)) {
+		if (regulator_disable(nop->vcc))
+			dev_err(phy->dev, "Failed to disable power\n");
+	}
 }
 
 static int nop_set_peripheral(struct usb_otg *otg, struct usb_gadget *gadget)
@@ -154,6 +166,12 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev)
 		}
 	}
 
+	nop->vcc = devm_regulator_get(&pdev->dev, "vcc");
+	if (IS_ERR(nop->vcc)) {
+		dev_dbg(&pdev->dev, "Error getting vcc regulator: %ld\n",
+					PTR_ERR(nop->vcc));
+	}
+
 	nop->dev		= &pdev->dev;
 	nop->phy.dev		= nop->dev;
 	nop->phy.label		= "nop-xceiv";
-- 
1.7.4.1

[PATCH 5/8] usb: phy: nop: Handle RESET for the PHY

[Roger Quadros]

We expect the RESET line to be modeled as a regulator with supply
name "reset". The regulator should be modeled such that enabling
the regulator brings the PHY device out of RESET and disabling the
regulator holds the device in RESET.

They PHY will be held in RESET in .shutdown() and brought out of
RESET in .init().

Signed-off-by: Roger Quadros <rogerq@ti.com>
Acked-by: Felipe Balbi <balbi@ti.com>
---
 drivers/usb/otg/nop-usb-xceiv.c |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/otg/nop-usb-xceiv.c b/drivers/usb/otg/nop-usb-xceiv.c
index fbdcfef..6efc9b7 100644
--- a/drivers/usb/otg/nop-usb-xceiv.c
+++ b/drivers/usb/otg/nop-usb-xceiv.c
@@ -40,6 +40,7 @@ struct nop_usb_xceiv {
 	struct device		*dev;
 	struct clk		*clk;
 	struct regulator	*vcc;
+	struct regulator	*reset;
 };
 
 static struct platform_device *pd;
@@ -80,6 +81,12 @@ static int nop_init(struct usb_phy *phy)
 	if (!IS_ERR(nop->clk))
 		clk_enable(nop->clk);
 
+	if (!IS_ERR(nop->reset)) {
+		/* De-assert RESET */
+		if (regulator_enable(nop->reset))
+			dev_err(phy->dev, "Failed to de-assert reset\n");
+	}
+
 	return 0;
 }
 
@@ -87,6 +94,12 @@ static void nop_shutdown(struct usb_phy *phy)
 {
 	struct nop_usb_xceiv *nop = dev_get_drvdata(phy->dev);
 
+	if (!IS_ERR(nop->reset)) {
+		/* Assert RESET */
+		if (regulator_disable(nop->reset))
+			dev_err(phy->dev, "Failed to assert reset\n");
+	}
+
 	if (!IS_ERR(nop->clk))
 		clk_disable(nop->clk);
 
@@ -172,6 +185,12 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev)
 					PTR_ERR(nop->vcc));
 	}
 
+	nop->reset = devm_regulator_get(&pdev->dev, "reset");
+	if (IS_ERR(nop->reset)) {
+		dev_dbg(&pdev->dev, "Error getting reset regulator: %ld\n",
+					PTR_ERR(nop->reset));
+	}
+
 	nop->dev		= &pdev->dev;
 	nop->phy.dev		= nop->dev;
 	nop->phy.label		= "nop-xceiv";
-- 
1.7.4.1

[PATCH 6/8] usb: phy: nop: use new PHY API to register PHY

[Roger Quadros]

We would need to support multiple PHYs of the same type
so use the new PHY API usb_add_phy_dev() to register the PHY.

Signed-off-by: Roger Quadros <rogerq@ti.com>
Acked-by: Felipe Balbi <balbi@ti.com>
---
 drivers/usb/otg/nop-usb-xceiv.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/otg/nop-usb-xceiv.c b/drivers/usb/otg/nop-usb-xceiv.c
index 6efc9b7..fe7a460 100644
--- a/drivers/usb/otg/nop-usb-xceiv.c
+++ b/drivers/usb/otg/nop-usb-xceiv.c
@@ -198,12 +198,13 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev)
 	nop->phy.init		= nop_init;
 	nop->phy.shutdown	= nop_shutdown;
 	nop->phy.state		= OTG_STATE_UNDEFINED;
+	nop->phy.type		= type;
 
 	nop->phy.otg->phy		= &nop->phy;
 	nop->phy.otg->set_host		= nop_set_host;
 	nop->phy.otg->set_peripheral	= nop_set_peripheral;
 
-	err = usb_add_phy(&nop->phy, type);
+	err = usb_add_phy_dev(&nop->phy);
 	if (err) {
 		dev_err(&pdev->dev, "can't register transceiver, err: %d\n",
 			err);
-- 
1.7.4.1

[PATCH 7/8] usb: phy: nop: Add device tree support and binding information

[Roger Quadros]

The PHY clock, clock rate, VCC regulator and RESET regulator
can now be provided via device tree.

Signed-off-by: Roger Quadros <rogerq@ti.com>
Acked-by: Felipe Balbi <balbi@ti.com>
---
 .../devicetree/bindings/usb/usb-nop-xceiv.txt      |   34 +++++++++++++++++++
 drivers/usb/otg/nop-usb-xceiv.c                    |   35 +++++++++++++++----
 2 files changed, 61 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt

diff --git a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
new file mode 100644
index 0000000..d7e2726
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
@@ -0,0 +1,34 @@
+USB NOP PHY
+
+Required properties:
+- compatible: should be usb-nop-xceiv
+
+Optional properties:
+- clocks: phandle to the PHY clock. Use as per Documentation/devicetree
+  /bindings/clock/clock-bindings.txt
+  This property is required if clock-frequency is specified.
+
+- clock-names: Should be "main_clk"
+
+- clock-frequency: the clock frequency (in Hz) that the PHY clock must
+  be configured to.
+
+- vcc-supply: phandle to the regulator that provides RESET to the PHY.
+
+- reset-supply: phandle to the regulator that provides power to the PHY.
+
+Example:
+
+	hsusb1_phy {
+		compatible = "usb-nop-xceiv";
+		clock-frequency = <19200000>;
+		clocks = <&osc 0>;
+		clock-names = "main_clk";
+		vcc-supply = <&hsusb1_vcc_regulator>;
+		reset-supply = <&hsusb1_reset_regulator>;
+	};
+
+hsusb1_phy is a NOP USB PHY device that gets its clock from an oscillator
+and expects that clock to be configured to 19.2MHz by the NOP PHY driver.
+hsusb1_vcc_regulator provides power to the PHY and hsusb1_reset_regulator
+controls RESET.
diff --git a/drivers/usb/otg/nop-usb-xceiv.c b/drivers/usb/otg/nop-usb-xceiv.c
index fe7a460..b26b1c2 100644
--- a/drivers/usb/otg/nop-usb-xceiv.c
+++ b/drivers/usb/otg/nop-usb-xceiv.c
@@ -34,13 +34,14 @@
 #include <linux/slab.h>
 #include <linux/clk.h>
 #include <linux/regulator/consumer.h>
+#include <linux/of.h>
 
 struct nop_usb_xceiv {
-	struct usb_phy		phy;
-	struct device		*dev;
-	struct clk		*clk;
-	struct regulator	*vcc;
-	struct regulator	*reset;
+	struct usb_phy phy;
+	struct device *dev;
+	struct clk *clk;
+	struct regulator *vcc;
+	struct regulator *reset;
 };
 
 static struct platform_device *pd;
@@ -140,10 +141,12 @@ static int nop_set_host(struct usb_otg *otg, struct usb_bus *host)
 
 static int nop_usb_xceiv_probe(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
 	struct nop_usb_xceiv_platform_data *pdata = pdev->dev.platform_data;
 	struct nop_usb_xceiv	*nop;
 	enum usb_phy_type	type = USB_PHY_TYPE_USB2;
 	int err;
+	u32 clk_rate = 0;
 
 	nop = devm_kzalloc(&pdev->dev, sizeof(*nop), GFP_KERNEL);
 	if (!nop)
@@ -154,8 +157,16 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev)
 	if (!nop->phy.otg)
 		return -ENOMEM;
 
-	if (pdata)
+	if (dev->of_node) {
+		struct device_node *node = dev->of_node;
+
+		if (of_property_read_u32(node, "clock-frequency", &clk_rate))
+			clk_rate = 0;
+
+	} else if (pdata) {
 		type = pdata->type;
+		clk_rate = pdata->clk_rate;
+	}
 
 	nop->clk = devm_clk_get(&pdev->dev, "main_clk");
 	if (IS_ERR(nop->clk)) {
@@ -163,8 +174,8 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev)
 					PTR_ERR(nop->clk));
 	}
 
-	if (!IS_ERR(nop->clk) && pdata && pdata->clk_rate) {
-		err = clk_set_rate(nop->clk, pdata->clk_rate);
+	if (!IS_ERR(nop->clk) && clk_rate) {
+		err = clk_set_rate(nop->clk, clk_rate);
 		if (err) {
 			dev_err(&pdev->dev, "Error setting clock rate\n");
 			return err;
@@ -237,12 +248,20 @@ static int nop_usb_xceiv_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id nop_xceiv_dt_ids[] = {
+	{ .compatible = "usb-nop-xceiv" },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(of, nop_xceiv_dt_ids);
+
 static struct platform_driver nop_usb_xceiv_driver = {
 	.probe		= nop_usb_xceiv_probe,
 	.remove		= nop_usb_xceiv_remove,
 	.driver		= {
 		.name	= "nop_usb_xceiv",
 		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(nop_xceiv_dt_ids),
 	},
 };
 
-- 
1.7.4.1

[PATCH 8/8] USB: phy: nop: Defer probe if device needs VCC/RESET

[Roger Quadros]

Add 2 flags, needs_vcc and needs_reset to platform data.
If the flag is set and the regulator couldn't be found
then we bail out with -EPROBE_DEFER.

For device tree boot we depend on presensce of vcc-supply/
reset-supply properties to decide if we should bail out
with -EPROBE_DEFER or just continue in case the regulator
can't be found.

This is required for proper functionality in cases where the
regulator is needed but is probed later than the PHY device.

Signed-off-by: Roger Quadros <rogerq@ti.com>
Acked-by: Felipe Balbi <balbi@ti.com>
---
 drivers/usb/otg/nop-usb-xceiv.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/otg/nop-usb-xceiv.c b/drivers/usb/otg/nop-usb-xceiv.c
index b26b1c2..2b10cc9 100644
--- a/drivers/usb/otg/nop-usb-xceiv.c
+++ b/drivers/usb/otg/nop-usb-xceiv.c
@@ -147,6 +147,8 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev)
 	enum usb_phy_type	type = USB_PHY_TYPE_USB2;
 	int err;
 	u32 clk_rate = 0;
+	bool needs_vcc = false;
+	bool needs_reset = false;
 
 	nop = devm_kzalloc(&pdev->dev, sizeof(*nop), GFP_KERNEL);
 	if (!nop)
@@ -163,9 +165,14 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev)
 		if (of_property_read_u32(node, "clock-frequency", &clk_rate))
 			clk_rate = 0;
 
+		needs_vcc = of_property_read_bool(node, "vcc-supply");
+		needs_reset = of_property_read_bool(node, "reset-supply");
+
 	} else if (pdata) {
 		type = pdata->type;
 		clk_rate = pdata->clk_rate;
+		needs_vcc = pdata->needs_vcc;
+		needs_reset = pdata->needs_reset;
 	}
 
 	nop->clk = devm_clk_get(&pdev->dev, "main_clk");
@@ -194,12 +201,16 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev)
 	if (IS_ERR(nop->vcc)) {
 		dev_dbg(&pdev->dev, "Error getting vcc regulator: %ld\n",
 					PTR_ERR(nop->vcc));
+		if (needs_vcc)
+			return -EPROBE_DEFER;
 	}
 
 	nop->reset = devm_regulator_get(&pdev->dev, "reset");
 	if (IS_ERR(nop->reset)) {
 		dev_dbg(&pdev->dev, "Error getting reset regulator: %ld\n",
 					PTR_ERR(nop->reset));
+		if (needs_reset)
+			return -EPROBE_DEFER;
 	}
 
 	nop->dev		= &pdev->dev;
-- 
1.7.4.1

Re: [PATCH 1/8] usb: phy: nop: Add some parameters to platform data

[Marc Kleine-Budde]

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

On 03/12/2013 12:24 PM, Roger Quadros wrote:
> Add clk_rate parameter to platform data. If supplied, the
> NOP phy driver will program the clock to that rate during probe.
> 
> Also add 2 flags, needs_vcc and needs_reset.
> If the flag is set and the regulator couldn't be found
> then the driver will bail out with -EPROBE_DEFER.

Is there a platform which fills out pdata.needs_vcc and
pdata.needs_reset? IMHO it makes no sense to add features for the non DT
case, if there isn't any user for it.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH 1/8] usb: phy: nop: Add some parameters to platform data

[Roger Quadros]

On 03/12/2013 01:54 PM, Marc Kleine-Budde wrote:
> On 03/12/2013 12:24 PM, Roger Quadros wrote:
>> Add clk_rate parameter to platform data. If supplied, the
>> NOP phy driver will program the clock to that rate during probe.
>>
>> Also add 2 flags, needs_vcc and needs_reset.
>> If the flag is set and the regulator couldn't be found
>> then the driver will bail out with -EPROBE_DEFER.
> 
> Is there a platform which fills out pdata.needs_vcc and
> pdata.needs_reset? IMHO it makes no sense to add features for the non DT
> case, if there isn't any user for it.
>

There can be a user in the non DT case as well. Consider the following example:
The power regulator that supplies VCC to the PHY device sits over the I2C bus.
The NOP transceiver driver is built into the kernel but the regulator driver
is built as a module.

During boot, the NOP transceiver driver's probe is called before the regulator can
be probed. So regulator_get() will fail and if the platform doesn't set the
needs_vcc flag, the driver will continue as normal and the PHY device won't work.

I know that beagleboard can benefit from this. The current workaround is to
register the regulator before we register the PHY in the board file. With the
needs_vcc flag, we don't need to have that limitation.

cheers,
-roger

Re: [PATCH 1/8] usb: phy: nop: Add some parameters to platform data

[Marc Kleine-Budde]

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

On 03/12/2013 03:12 PM, Roger Quadros wrote:
> On 03/12/2013 01:54 PM, Marc Kleine-Budde wrote:
>> On 03/12/2013 12:24 PM, Roger Quadros wrote:
>>> Add clk_rate parameter to platform data. If supplied, the
>>> NOP phy driver will program the clock to that rate during probe.
>>>
>>> Also add 2 flags, needs_vcc and needs_reset.
>>> If the flag is set and the regulator couldn't be found
>>> then the driver will bail out with -EPROBE_DEFER.
>>
>> Is there a platform which fills out pdata.needs_vcc and
>> pdata.needs_reset? IMHO it makes no sense to add features for the non DT
>> case, if there isn't any user for it.
>>
> 
> There can be a user in the non DT case as well. Consider the following example:

I'm just saying, let the implementation up to a real user for the non DT
case. Beagleboard is ARM and there's no point of implementing non DT
fall backs for ARM, IMHO.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH 1/8] usb: phy: nop: Add some parameters to platform data

[Roger Quadros]

On 03/12/2013 04:17 PM, Marc Kleine-Budde wrote:
> On 03/12/2013 03:12 PM, Roger Quadros wrote:
>> On 03/12/2013 01:54 PM, Marc Kleine-Budde wrote:
>>> On 03/12/2013 12:24 PM, Roger Quadros wrote:
>>>> Add clk_rate parameter to platform data. If supplied, the
>>>> NOP phy driver will program the clock to that rate during probe.
>>>>
>>>> Also add 2 flags, needs_vcc and needs_reset.
>>>> If the flag is set and the regulator couldn't be found
>>>> then the driver will bail out with -EPROBE_DEFER.
>>>
>>> Is there a platform which fills out pdata.needs_vcc and
>>> pdata.needs_reset? IMHO it makes no sense to add features for the non DT
>>> case, if there isn't any user for it.
>>>
>>
>> There can be a user in the non DT case as well. Consider the following example:
> 
> I'm just saying, let the implementation up to a real user for the non DT
> case. Beagleboard is ARM and there's no point of implementing non DT
> fall backs for ARM, IMHO.
> 

Why do you say so?

It doesn't depend on the CPU architecture. It depends on how the board designer
wired the board. A non ARM platform could also face the same problem as beagle.

It is not a non DT fallback. I believe many are still using non DT boot and it
needs to work at least till we move all functionality to purely DT.

cheers,
-roger

Re: [PATCH 1/8] usb: phy: nop: Add some parameters to platform data

[Marc Kleine-Budde]

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

On 03/12/2013 03:28 PM, Roger Quadros wrote:
> On 03/12/2013 04:17 PM, Marc Kleine-Budde wrote:
>> On 03/12/2013 03:12 PM, Roger Quadros wrote:
>>> On 03/12/2013 01:54 PM, Marc Kleine-Budde wrote:
>>>> On 03/12/2013 12:24 PM, Roger Quadros wrote:
>>>>> Add clk_rate parameter to platform data. If supplied, the
>>>>> NOP phy driver will program the clock to that rate during probe.
>>>>>
>>>>> Also add 2 flags, needs_vcc and needs_reset.
>>>>> If the flag is set and the regulator couldn't be found
>>>>> then the driver will bail out with -EPROBE_DEFER.
>>>>
>>>> Is there a platform which fills out pdata.needs_vcc and
>>>> pdata.needs_reset? IMHO it makes no sense to add features for the non DT
>>>> case, if there isn't any user for it.
>>>>
>>>
>>> There can be a user in the non DT case as well. Consider the following example:
>>
>> I'm just saying, let the implementation up to a real user for the non DT
>> case. Beagleboard is ARM and there's no point of implementing non DT
>> fall backs for ARM, IMHO.
>>
> 
> Why do you say so?
> 
> It doesn't depend on the CPU architecture. It depends on how the board designer
> wired the board. A non ARM platform could also face the same problem as beagle.

Using -EPROBE_DEFER ist the way to solve the problem.

> It is not a non DT fallback. I believe many are still using non DT boot and it
> needs to work at least till we move all functionality to purely DT.

From my point of view, it makes no sense today to implement new features
which have a fallback for the non-DT case, if there isn't a real user of
this feature. So IMHO don't add needs_vcc and needs_reset flags to the
pdata, just take the information from the DT. As soon as there is a
non-DT user of this feature she/he can implement it or even better
switch to DT.

just my 2¢,
Marc
-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH 1/8] usb: phy: nop: Add some parameters to platform data

[Roger Quadros]

On 03/12/2013 04:42 PM, Marc Kleine-Budde wrote:
> On 03/12/2013 03:28 PM, Roger Quadros wrote:
>> On 03/12/2013 04:17 PM, Marc Kleine-Budde wrote:
>>> On 03/12/2013 03:12 PM, Roger Quadros wrote:
>>>> On 03/12/2013 01:54 PM, Marc Kleine-Budde wrote:
>>>>> On 03/12/2013 12:24 PM, Roger Quadros wrote:
>>>>>> Add clk_rate parameter to platform data. If supplied, the
>>>>>> NOP phy driver will program the clock to that rate during probe.
>>>>>>
>>>>>> Also add 2 flags, needs_vcc and needs_reset.
>>>>>> If the flag is set and the regulator couldn't be found
>>>>>> then the driver will bail out with -EPROBE_DEFER.
>>>>>
>>>>> Is there a platform which fills out pdata.needs_vcc and
>>>>> pdata.needs_reset? IMHO it makes no sense to add features for the non DT
>>>>> case, if there isn't any user for it.
>>>>>
>>>>
>>>> There can be a user in the non DT case as well. Consider the following example:
>>>
>>> I'm just saying, let the implementation up to a real user for the non DT
>>> case. Beagleboard is ARM and there's no point of implementing non DT
>>> fall backs for ARM, IMHO.
>>>
>>
>> Why do you say so?
>>
>> It doesn't depend on the CPU architecture. It depends on how the board designer
>> wired the board. A non ARM platform could also face the same problem as beagle.
> 
> Using -EPROBE_DEFER ist the way to solve the problem.

did you mean isn't? If you did then what is the other option?
> 
>> It is not a non DT fallback. I believe many are still using non DT boot and it
>> needs to work at least till we move all functionality to purely DT.
> 
> From my point of view, it makes no sense today to implement new features
> which have a fallback for the non-DT case, if there isn't a real user of
> this feature. So IMHO don't add needs_vcc and needs_reset flags to the
> pdata, just take the information from the DT. As soon as there is a
> non-DT user of this feature she/he can implement it or even better
> switch to DT.

Adding Peter to the loop. I faintly remember him mentioning this issue before
for beagle. We really need the deferred probe mechanism or we need to resort
to device registering order.

The first user for needs_vcc flag will be the beagleboard file. I just didn't
implement it in this patch [1].

On second thoughts, since [1] does work on beagleboard without requiring the
needs_vcc flag, I think we can just live without it.

Felipe, Peter, what do you think?

cheers,
-roger

[1] https://lkml.org/lkml/2013/3/12/244

Re: [PATCH 1/8] usb: phy: nop: Add some parameters to platform data

[Marc Kleine-Budde]

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

On 03/12/2013 04:20 PM, Roger Quadros wrote:
> On 03/12/2013 04:42 PM, Marc Kleine-Budde wrote:
>> On 03/12/2013 03:28 PM, Roger Quadros wrote:
>>> On 03/12/2013 04:17 PM, Marc Kleine-Budde wrote:
>>>> On 03/12/2013 03:12 PM, Roger Quadros wrote:
>>>>> On 03/12/2013 01:54 PM, Marc Kleine-Budde wrote:
>>>>>> On 03/12/2013 12:24 PM, Roger Quadros wrote:
>>>>>>> Add clk_rate parameter to platform data. If supplied, the
>>>>>>> NOP phy driver will program the clock to that rate during probe.
>>>>>>>
>>>>>>> Also add 2 flags, needs_vcc and needs_reset.
>>>>>>> If the flag is set and the regulator couldn't be found
>>>>>>> then the driver will bail out with -EPROBE_DEFER.
>>>>>>
>>>>>> Is there a platform which fills out pdata.needs_vcc and
>>>>>> pdata.needs_reset? IMHO it makes no sense to add features for the non DT
>>>>>> case, if there isn't any user for it.
>>>>>>
>>>>>
>>>>> There can be a user in the non DT case as well. Consider the following example:
>>>>
>>>> I'm just saying, let the implementation up to a real user for the non DT
>>>> case. Beagleboard is ARM and there's no point of implementing non DT
>>>> fall backs for ARM, IMHO.
>>>>
>>>
>>> Why do you say so?
>>>
>>> It doesn't depend on the CPU architecture. It depends on how the board designer
>>> wired the board. A non ARM platform could also face the same problem as beagle.
>>
>> Using -EPROBE_DEFER ist the way to solve the problem.
> 
> did you mean isn't? If you did then what is the other option?

You're using -EPROBE_DEFER, that's all perfectly fine.

>>> It is not a non DT fallback. I believe many are still using non DT boot and it
>>> needs to work at least till we move all functionality to purely DT.
>>
>> From my point of view, it makes no sense today to implement new features
>> which have a fallback for the non-DT case, if there isn't a real user of
>> this feature. So IMHO don't add needs_vcc and needs_reset flags to the
>> pdata, just take the information from the DT. As soon as there is a
>> non-DT user of this feature she/he can implement it or even better
>> switch to DT.
> 
> Adding Peter to the loop. I faintly remember him mentioning this issue before
> for beagle. We really need the deferred probe mechanism or we need to resort
> to device registering order.

> The first user for needs_vcc flag will be the beagleboard file. I just didn't
> implement it in this patch [1].

Okay, I wrongly assumed that there wouldn't be added any new features to
the old non-DT board files.

> On second thoughts, since [1] does work on beagleboard without requiring the
> needs_vcc flag, I think we can just live without it.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH 1/8] usb: phy: nop: Add some parameters to platform data

[Peter Ujfalusi]

On 03/12/2013 04:20 PM, Roger Quadros wrote:
> Adding Peter to the loop. I faintly remember him mentioning this issue before
> for beagle. We really need the deferred probe mechanism or we need to resort
> to device registering order.

Yes, BeagleBoard is a good example. Long story short: we have external
dependency and the correct way to handle that is via deferred probe.

As of now we are not ready to kill the legacy support but over time we should
move as much as we can to DT only mode.

> The first user for needs_vcc flag will be the beagleboard file. I just didn't
> implement it in this patch [1].
> 
> On second thoughts, since [1] does work on beagleboard without requiring the
> needs_vcc flag, I think we can just live without it.

The BeagleBoard works because the bootloader happen to enable the PWM
generator which is used as a GPIO for the external HUB enable port. As soon as
the bootloader stops doing this we are going to be in trouble.

> 
> Felipe, Peter, what do you think?
> 
> cheers,
> -roger
> 
> [1] https://lkml.org/lkml/2013/3/12/244
> 


-- 
Péter

Re: [PATCH 1/8] usb: phy: nop: Add some parameters to platform data

[Roger Quadros]

On 03/13/2013 05:17 PM, Peter Ujfalusi wrote:
> On 03/12/2013 04:20 PM, Roger Quadros wrote:
>> Adding Peter to the loop. I faintly remember him mentioning this issue before
>> for beagle. We really need the deferred probe mechanism or we need to resort
>> to device registering order.
> 
> Yes, BeagleBoard is a good example. Long story short: we have external
> dependency and the correct way to handle that is via deferred probe.
> 
> As of now we are not ready to kill the legacy support but over time we should
> move as much as we can to DT only mode.
> 
>> The first user for needs_vcc flag will be the beagleboard file. I just didn't
>> implement it in this patch [1].
>>
>> On second thoughts, since [1] does work on beagleboard without requiring the
>> needs_vcc flag, I think we can just live without it.
> 
> The BeagleBoard works because the bootloader happen to enable the PWM
> generator which is used as a GPIO for the external HUB enable port. As soon as
> the bootloader stops doing this we are going to be in trouble.
> 

Wasn't it the LED A out GPO from PMIC that is used to enable the HUB? Just cross
checking so I haven't missed anything.

I will resend the board support patch for beagle implementing
needs_vcc flag for the deferred probe mechanism. Thanks.

cheers,
-roger

Re: [PATCH 1/8] usb: phy: nop: Add some parameters to platform data

[Marc Kleine-Budde]

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

On 03/13/2013 04:17 PM, Peter Ujfalusi wrote:
> On 03/12/2013 04:20 PM, Roger Quadros wrote:
>> Adding Peter to the loop. I faintly remember him mentioning this issue before
>> for beagle. We really need the deferred probe mechanism or we need to resort
>> to device registering order.
> 
> Yes, BeagleBoard is a good example. Long story short: we have external
> dependency and the correct way to handle that is via deferred probe.

Sure.

> As of now we are not ready to kill the legacy support but over time we should
> move as much as we can to DT only mode.

I was assuming you're already on DT only :)

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]