USB: phy: phy-nop: Manage RESET GPIO in the driver

Modelling the RESET line as a regulator supply wasn't a good idea as it kind of abuses the regulator framework and also makes adaptation code more complex.

Instead, manage the RESET gpio line directly in the driver. Update the device tree binding information.

This also makes us easy to migrate to a dedicated GPIO RESET controller whenever it becomes available.

Signed-off-by Roger Quadros <[email protected]>

---
 .../devicetree/bindings/usb/usb-nop-xceiv.txt      |    7 +-
 drivers/usb/phy/phy-nop.c                          |   71 ++++++++++++++-----
 2 files changed, 55 insertions(+), 23 deletions(-)

Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt +3 -4

index d7e2726..1bd37fa 100644
@@ -15,7 +15,7 @@ Optional properties:
 
 - vcc-supply: phandle to the regulator that provides RESET to the PHY.
 
-- reset-supply: phandle to the regulator that provides power to the PHY.
+- reset-gpios: Should specify the GPIO for reset.
 
 Example:
 

@@ -25,10 +25,9 @@ Example:
 		clocks = <&osc 0>;
 		clock-names = "main_clk";
 		vcc-supply = <&hsusb1_vcc_regulator>;
-		reset-supply = <&hsusb1_reset_regulator>;
+		reset-gpios = <&gpio1 7 GPIO_ACTIVE_LOW>;
 	};
 
 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.
+hsusb1_vcc_regulator provides power to the PHY and GPIO 7 controls RESET.

drivers/usb/phy/phy-nop.c +36 -17

index 55445e5d..1f88c32 100644
@@ -35,6 +35,9 @@
 #include <linux/clk.h>
 #include <linux/regulator/consumer.h>
 #include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/gpio.h>
+#include <linux/delay.h>
 
 struct nop_usb_xceiv {
 	struct usb_phy phy;

@@ -42,6 +45,8 @@ struct nop_usb_xceiv {
 	struct clk *clk;
 	struct regulator *vcc;
 	struct regulator *reset;
+	int gpio_reset;
+	bool reset_active_low;
 };
 
 static struct platform_device *pd;

@@ -70,6 +75,23 @@ static int nop_set_suspend(struct usb_phy *x, int suspend)
 	return 0;
 }
 
+static void nop_reset_set(struct nop_usb_xceiv *nop, int asserted)
+{
+	int value;
+
+	if (!gpio_is_valid(nop->gpio_reset))
+		return;
+
+	value = asserted;
+	if (nop->reset_active_low)
+		value = !value;
+
+	gpio_set_value_cansleep(nop->gpio_reset, value);
+
+	if (!asserted)
+		usleep_range(10000, 20000);
+}
+
 static int nop_init(struct usb_phy *phy)
 {
 	struct nop_usb_xceiv *nop = dev_get_drvdata(phy->dev);

@@ -82,11 +104,8 @@ 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");
-	}
+	/* De-assert RESET */
+	nop_reset_set(nop, 0);
 
 	return 0;
 }

@@ -95,11 +114,8 @@ 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");
-	}
+	/* Assert RESET */
+	nop_reset_set(nop, 1);
 
 	if (!IS_ERR(nop->clk))
 		clk_disable(nop->clk);

@@ -148,7 +164,6 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev)
 	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)

@@ -159,20 +174,28 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev)
 	if (!nop->phy.otg)
 		return -ENOMEM;
 
+	nop->reset_active_low = true;	/* default behaviour */
+
 	if (dev->of_node) {
 		struct device_node *node = dev->of_node;
+		enum of_gpio_flags flags;
 
 		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");
+		nop->gpio_reset = of_get_named_gpio_flags(node, "reset-gpios",
+								0, &flags);
+		if (nop->gpio_reset == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+
+		nop->reset_active_low = flags & OF_GPIO_ACTIVE_LOW;
 
 	} else if (pdata) {
 		type = pdata->type;
 		clk_rate = pdata->clk_rate;
 		needs_vcc = pdata->needs_vcc;
-		needs_reset = pdata->needs_reset;
+		nop->gpio_reset = pdata->gpio_reset;
 	}
 
 	nop->clk = devm_clk_get(&pdev->dev, "main_clk");

@@ -205,12 +228,22 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev)
 			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));

The platform data bits can be inferred from the other members of struct usbhs_phy_data. So get rid of the platform_data member.

Build the platform data for the PHY device in usbhs_init_phys() instead.

Signed-off-by Roger Quadros <[email protected]>

Hi,

Quoted 3 lines · Roger Quadros

The platform data bits can be inferred from the other members of struct usbhs_phy_data. So get rid of the platform_data member.

Quoted 3 lines

Build the platform data for the PHY device in usbhs_init_phys() instead.

Signed-off-by Roger Quadros <[email protected]>

Tony, do I get your Acked-by here and all following arch/arm/mach-omap2* patches in this series ?

Quoted 6 lines · Felipe Balbi

Hi,

Quoted 3 lines · Roger Quadros

The platform data bits can be inferred from the other members of struct usbhs_phy_data. So get rid of the platform_data member.

Quoted 6 lines

Quoted 3 lines

Build the platform data for the PHY device in usbhs_init_phys() instead.

Signed-off-by Roger Quadros <[email protected]>

Tony, do I get your Acked-by here and all following arch/arm/mach-omap2* patches in this series ?

If you can merge just this series without the .dts changes into an immutable topic branch against let's say v3.12-rc2 when it comes out, then yes I can ack them:

Acked-by Tony Lindgren <[email protected]>

That way I can also pull in the topic branch as needed to avoid self inflicted merge conflicts ;) And Benoit can set up a separate .dts branch based on your branch to make sure the dependencies are in place before merging that one upstream.

Hi,

Modelling the RESET line as a regulator supply wasn't a good idea as it abuses the regulator framework and makes adaptation code/data more complex.

Instead, manage the RESET gpio line directly in the driver.

This also makes us easy to migrate to a dedicated GPIO RESET controller whenever it becomes available. As of now, it doesn't seem to be making it into 3.12.

*NOTE:* As there are changes to platform data, Patch 1 needs to be shared between the arm-soc tree and usb tree.

Patch 1 is available at repo
git://github.com/rogerq/linux.git
in branch
phy-reset-common

Patch 2 contains the phy-nop driver changes
Patches 3 and 4 adapt legacy boot code to the phy-nop driver changes. Patches 5, 6 and 7 adapt DT data to the binding changes. Patch 8 is cleanup of omap3-beagle DT.
Patch 9 adds USB host support to omap3-beagle-xm using the new binding.

Patches are based on v3.11-rc5.
Tested leacy boot on omap3-beagle and omap3-beagle-xm Tested DT boot on omap3-beagle, omap3-beagle-xm and omap4-panda-es

v2:
- Added RESET GPIO polarity feature
- Changed to gpio_set_value_cansleep()

cheers,
-roger

---
Roger Quadros (9):
usb: phy: nop: Add gpio_reset to platform data
usb: phy: nop: Don't use regulator framework for RESET line
ARM: OMAP2+: omap-usb-host: Get rid of platform_data from struct
usbhs_phy_data
ARM: OMAP2+: usb-host: Adapt to USB phy-nop RESET line changes
ARM: dts: omap3-beagle: Use reset-gpios for hsusb2_reset
ARM: dts: omap4-panda: Use reset-gpios for hsusb1_reset
ARM: dts: omap5-uevm: Use reset-gpios for hsusb2_reset
ARM: dts: omap3-beagle: Make USB host pin naming consistent
ARM: dts: omap3-beagle-xm: Add USB Host support

Hi Roger,

Quoted 37 lines · Roger Quadros

Hi,

Modelling the RESET line as a regulator supply wasn't a good idea as it abuses the regulator framework and makes adaptation code/data more complex.

Instead, manage the RESET gpio line directly in the driver.

This also makes us easy to migrate to a dedicated GPIO RESET controller whenever it becomes available. As of now, it doesn't seem to be making it into 3.12.

*NOTE:* As there are changes to platform data, Patch 1 needs to be shared between the arm-soc tree and usb tree.

Patch 1 is available at repo
git://github.com/rogerq/linux.git
in branch
phy-reset-common

Patch 2 contains the phy-nop driver changes
Patches 3 and 4 adapt legacy boot code to the phy-nop driver changes. Patches 5, 6 and 7 adapt DT data to the binding changes. Patch 8 is cleanup of omap3-beagle DT.
Patch 9 adds USB host support to omap3-beagle-xm using the new binding.

Patches are based on v3.11-rc5.
Tested leacy boot on omap3-beagle and omap3-beagle-xm Tested DT boot on omap3-beagle, omap3-beagle-xm and omap4-panda-es

v2:
- Added RESET GPIO polarity feature
- Changed to gpio_set_value_cansleep()

cheers,
-roger

Quoted 12 lines

---
Roger Quadros (9):
usb: phy: nop: Add gpio_reset to platform data
usb: phy: nop: Don't use regulator framework for RESET line
ARM: OMAP2+: omap-usb-host: Get rid of platform_data from struct
usbhs_phy_data
ARM: OMAP2+: usb-host: Adapt to USB phy-nop RESET line changes
ARM: dts: omap3-beagle: Use reset-gpios for hsusb2_reset
ARM: dts: omap4-panda: Use reset-gpios for hsusb1_reset
ARM: dts: omap5-uevm: Use reset-gpios for hsusb2_reset
ARM: dts: omap3-beagle: Make USB host pin naming consistent
ARM: dts: omap3-beagle-xm: Add USB Host support

The 5 DTS patches looks good to me, and I can apply them, but you need to be sure that the driver will be merged for 3.12 as well.