[PATCH v1 1/3] usb: phy: device-tree documentation for gpio-vbus

[PATCH v1 1/3] usb: phy: device-tree documentation for gpio-vbus

[Robert Jarzmik]

Add documentation for device-tree binding of the generic gpio-vbus phy
controller.

Signed-off-by: Robert Jarzmik <robert.jarzmik-GANU6spQydw@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
 .../devicetree/bindings/phy/gpio-vbus.txt          | 23 ++++++++++++++++++++++
 1 file changed, 23 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/gpio-vbus.txt

diff --git a/Documentation/devicetree/bindings/phy/gpio-vbus.txt b/Documentation/devicetree/bindings/phy/gpio-vbus.txt
new file mode 100644
index 0000000..ffcfd35
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/gpio-vbus.txt
@@ -0,0 +1,23 @@
+GPIO VBus phy
+=============
+
+gpio-vbus is a phy controller supporting VBus detection and pullup activation on
+GPIOs.
+
+Required properties:
+- compatible : should be "generic,phy-gpio-vbus"
+- #phy-cells : from the generic PHY bindings, must be 1.
+- gpios      : set of 2 gpio properties (see gpio.txt for gpio properties format)
+               first gpio is required, it's the VBus sensing input gpio
+	       second gpio is optional, it's the D+ pullup controlling output
+	       gpio
+
+Optional properties:
+- wakeup     : boolean, if true the VBus gpio will wakeup the platform
+
+Example:
+	usb2phy : gpio-vbus@13 {
+		compatible = "generic,phy-gpio-vbus";
+		gpios = <&gpio 13 GPIO_ACTIVE_LOW>;
+		wakeup;
+	};
-- 
2.1.0

--
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 v1 2/3] usb: phy: convert gpio-vbus to gpio_desc

[Robert Jarzmik]

In order to prepare device-tree conversion, port gpio_vbus to use
gpio_desc.

Signed-off-by: Robert Jarzmik <robert.jarzmik-GANU6spQydw@public.gmane.org>
---
 drivers/usb/phy/phy-gpio-vbus-usb.c | 105 +++++++++++++++++++-----------------
 1 file changed, 56 insertions(+), 49 deletions(-)

diff --git a/drivers/usb/phy/phy-gpio-vbus-usb.c b/drivers/usb/phy/phy-gpio-vbus-usb.c
index f4b14bd..d302ab2 100644
--- a/drivers/usb/phy/phy-gpio-vbus-usb.c
+++ b/drivers/usb/phy/phy-gpio-vbus-usb.c
@@ -10,6 +10,7 @@
 
 #include <linux/kernel.h>
 #include <linux/platform_device.h>
+#include <linux/gpio/consumer.h>
 #include <linux/gpio.h>
 #include <linux/module.h>
 #include <linux/slab.h>
@@ -40,6 +41,8 @@ struct gpio_vbus_data {
 	struct delayed_work	work;
 	int			vbus;
 	int			irq;
+	struct gpio_desc	*gpiod_vbus;
+	struct gpio_desc	*gpiod_pullup;
 };
 
 
@@ -86,28 +89,21 @@ static void set_vbus_draw(struct gpio_vbus_data *gpio_vbus, unsigned mA)
 	gpio_vbus->mA = mA;
 }
 
-static int is_vbus_powered(struct gpio_vbus_mach_info *pdata)
+static int is_vbus_powered(struct gpio_vbus_data *gpio_vbus)
 {
-	int vbus;
-
-	vbus = gpio_get_value(pdata->gpio_vbus);
-	if (pdata->gpio_vbus_inverted)
-		vbus = !vbus;
-
-	return vbus;
+	return gpiod_get_value(gpio_vbus->gpiod_vbus);
 }
 
 static void gpio_vbus_work(struct work_struct *work)
 {
 	struct gpio_vbus_data *gpio_vbus =
 		container_of(work, struct gpio_vbus_data, work.work);
-	struct gpio_vbus_mach_info *pdata = dev_get_platdata(gpio_vbus->dev);
-	int gpio, status, vbus;
+	int status, vbus;
 
 	if (!gpio_vbus->phy.otg->gadget)
 		return;
 
-	vbus = is_vbus_powered(pdata);
+	vbus = is_vbus_powered(gpio_vbus);
 	if ((vbus ^ gpio_vbus->vbus) == 0)
 		return;
 	gpio_vbus->vbus = vbus;
@@ -117,8 +113,6 @@ static void gpio_vbus_work(struct work_struct *work)
 	 * isp1301_omap::b_peripheral() does and enable the pullup here... although
 	 * that may complicate usb_gadget_{,dis}connect() support.
 	 */
-	gpio = pdata->gpio_pullup;
-
 	if (vbus) {
 		status = USB_EVENT_VBUS;
 		gpio_vbus->phy.state = OTG_STATE_B_PERIPHERAL;
@@ -129,15 +123,15 @@ static void gpio_vbus_work(struct work_struct *work)
 		set_vbus_draw(gpio_vbus, 100);
 
 		/* optionally enable D+ pullup */
-		if (gpio_is_valid(gpio))
-			gpio_set_value(gpio, !pdata->gpio_pullup_inverted);
+		if (gpio_vbus->gpiod_pullup)
+			gpiod_set_value(gpio_vbus->gpiod_pullup, 1);
 
 		atomic_notifier_call_chain(&gpio_vbus->phy.notifier,
 					   status, gpio_vbus->phy.otg->gadget);
 	} else {
 		/* optionally disable D+ pullup */
-		if (gpio_is_valid(gpio))
-			gpio_set_value(gpio, pdata->gpio_pullup_inverted);
+		if (gpio_vbus->gpiod_pullup)
+			gpiod_set_value(gpio_vbus->gpiod_pullup, 0);
 
 		set_vbus_draw(gpio_vbus, 0);
 
@@ -155,12 +149,11 @@ static void gpio_vbus_work(struct work_struct *work)
 static irqreturn_t gpio_vbus_irq(int irq, void *data)
 {
 	struct platform_device *pdev = data;
-	struct gpio_vbus_mach_info *pdata = dev_get_platdata(&pdev->dev);
 	struct gpio_vbus_data *gpio_vbus = platform_get_drvdata(pdev);
 	struct usb_otg *otg = gpio_vbus->phy.otg;
 
 	dev_dbg(&pdev->dev, "VBUS %s (gadget: %s)\n",
-		is_vbus_powered(pdata) ? "supplied" : "inactive",
+		is_vbus_powered(gpio_vbus) ? "supplied" : "inactive",
 		otg->gadget ? otg->gadget->name : "none");
 
 	if (otg->gadget)
@@ -176,22 +169,18 @@ static int gpio_vbus_set_peripheral(struct usb_otg *otg,
 					struct usb_gadget *gadget)
 {
 	struct gpio_vbus_data *gpio_vbus;
-	struct gpio_vbus_mach_info *pdata;
 	struct platform_device *pdev;
-	int gpio;
 
 	gpio_vbus = container_of(otg->phy, struct gpio_vbus_data, phy);
 	pdev = to_platform_device(gpio_vbus->dev);
-	pdata = dev_get_platdata(gpio_vbus->dev);
-	gpio = pdata->gpio_pullup;
 
 	if (!gadget) {
 		dev_dbg(&pdev->dev, "unregistering gadget '%s'\n",
 			otg->gadget->name);
 
 		/* optionally disable D+ pullup */
-		if (gpio_is_valid(gpio))
-			gpio_set_value(gpio, pdata->gpio_pullup_inverted);
+		if (gpio_vbus->gpiod_pullup)
+			gpiod_set_value(gpio_vbus->gpiod_pullup, 0);
 
 		set_vbus_draw(gpio_vbus, 0);
 
@@ -246,12 +235,29 @@ static int gpio_vbus_probe(struct platform_device *pdev)
 	struct gpio_vbus_mach_info *pdata = dev_get_platdata(&pdev->dev);
 	struct gpio_vbus_data *gpio_vbus;
 	struct resource *res;
-	int err, gpio, irq;
-	unsigned long irqflags;
-
-	if (!pdata || !gpio_is_valid(pdata->gpio_vbus))
+	struct gpio_desc *gpiod = NULL;
+	int err, gpio, irq, wakeup = 0;
+	unsigned long irqflags, gpio_flags;
+
+	if (pdata) {
+		gpio = pdata->gpio_vbus;
+		gpio_flags = pdata->gpio_vbus_inverted ? GPIOF_ACTIVE_LOW : 0;
+		if (gpio_is_valid(gpio)) {
+			err = devm_gpio_request_one(&pdev->dev, gpio,
+						    gpio_flags,
+						    "vbus_detect");
+			if (err) {
+				dev_err(&pdev->dev, "can't request vbus gpio %d, err: %d\n",
+					gpio, err);
+				return err;
+			}
+		}
+		gpiod = gpio_to_desc(gpio);
+		wakeup = pdata->wakeup;
+	}
+	if (!gpiod)
 		return -EINVAL;
-	gpio = pdata->gpio_vbus;
+	gpiod_direction_input(gpiod);
 
 	gpio_vbus = devm_kzalloc(&pdev->dev, sizeof(struct gpio_vbus_data),
 				 GFP_KERNEL);
@@ -274,37 +280,38 @@ static int gpio_vbus_probe(struct platform_device *pdev)
 	gpio_vbus->phy.otg->phy = &gpio_vbus->phy;
 	gpio_vbus->phy.otg->set_peripheral = gpio_vbus_set_peripheral;
 
-	err = devm_gpio_request(&pdev->dev, gpio, "vbus_detect");
-	if (err) {
-		dev_err(&pdev->dev, "can't request vbus gpio %d, err: %d\n",
-			gpio, err);
-		return err;
-	}
-	gpio_direction_input(gpio);
-
 	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
 	if (res) {
 		irq = res->start;
 		irqflags = (res->flags & IRQF_TRIGGER_MASK) | IRQF_SHARED;
 	} else {
-		irq = gpio_to_irq(gpio);
+		irq = gpiod_to_irq(gpiod);
 		irqflags = VBUS_IRQ_FLAGS;
 	}
 
+	gpio_vbus->gpiod_vbus = gpiod;
 	gpio_vbus->irq = irq;
 
 	/* if data line pullup is in use, initialize it to "not pulling up" */
-	gpio = pdata->gpio_pullup;
-	if (gpio_is_valid(gpio)) {
-		err = devm_gpio_request(&pdev->dev, gpio, "udc_pullup");
-		if (err) {
-			dev_err(&pdev->dev,
-				"can't request pullup gpio %d, err: %d\n",
-				gpio, err);
-			return err;
+	if (pdata) {
+		gpio = pdata->gpio_pullup;
+		gpio_flags = pdata->gpio_pullup_inverted ? GPIOF_ACTIVE_LOW : 0;
+		if (gpio_is_valid(gpio)) {
+			err = devm_gpio_request_one(&pdev->dev, gpio,
+						    gpio_flags,
+						    "udc_pullup");
+			if (err) {
+				dev_err(&pdev->dev,
+					"can't request pullup gpio %d, err: %d\n",
+					gpio, err);
+				return err;
+			}
+			gpiod = gpio_to_desc(gpio);
+			gpiod_direction_output(gpiod, 0);
 		}
-		gpio_direction_output(gpio, pdata->gpio_pullup_inverted);
 	}
+	if (!IS_ERR(gpiod))
+		gpio_vbus->gpiod_pullup = gpiod;
 
 	err = devm_request_irq(&pdev->dev, irq, gpio_vbus_irq, irqflags,
 			       "vbus_detect", pdev);
@@ -331,7 +338,7 @@ static int gpio_vbus_probe(struct platform_device *pdev)
 		return err;
 	}
 
-	device_init_wakeup(&pdev->dev, pdata->wakeup);
+	device_init_wakeup(&pdev->dev, wakeup);
 
 	return 0;
 }
-- 
2.1.0

--
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 v1 3/3] usb: phy: add device-tree support for gpio-vbus

[Robert Jarzmik]

Add device-tree support for the generic gpio-vbus driver.

Signed-off-by: Robert Jarzmik <robert.jarzmik-GANU6spQydw@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
 drivers/usb/phy/phy-gpio-vbus-usb.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/usb/phy/phy-gpio-vbus-usb.c b/drivers/usb/phy/phy-gpio-vbus-usb.c
index d302ab2..6194728 100644
--- a/drivers/usb/phy/phy-gpio-vbus-usb.c
+++ b/drivers/usb/phy/phy-gpio-vbus-usb.c
@@ -13,6 +13,7 @@
 #include <linux/gpio/consumer.h>
 #include <linux/gpio.h>
 #include <linux/module.h>
+#include <linux/of_gpio.h>
 #include <linux/slab.h>
 #include <linux/interrupt.h>
 #include <linux/usb.h>
@@ -254,6 +255,12 @@ static int gpio_vbus_probe(struct platform_device *pdev)
 		}
 		gpiod = gpio_to_desc(gpio);
 		wakeup = pdata->wakeup;
+	} else {
+		gpiod = devm_gpiod_get_index(&pdev->dev, NULL, 0, GPIOD_IN);
+		if (pdev->dev.of_node &&
+		    of_get_property(pdev->dev.of_node,
+				    "wakeup", NULL))
+			wakeup = 1;
 	}
 	if (!gpiod)
 		return -EINVAL;
@@ -309,6 +316,8 @@ static int gpio_vbus_probe(struct platform_device *pdev)
 			gpiod = gpio_to_desc(gpio);
 			gpiod_direction_output(gpiod, 0);
 		}
+	} else {
+		gpiod = devm_gpiod_get_index(&pdev->dev, NULL, 1, 0);
 	}
 	if (!IS_ERR(gpiod))
 		gpio_vbus->gpiod_pullup = gpiod;
@@ -382,12 +391,18 @@ static const struct dev_pm_ops gpio_vbus_dev_pm_ops = {
 };
 #endif
 
+static struct of_device_id gpio_vbus_dt_ids[] = {
+	{ .compatible = "generic,phy-gpio-vbus" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, gpio_vbus_dt_ids);
 MODULE_ALIAS("platform:gpio-vbus");
 
 static struct platform_driver gpio_vbus_driver = {
 	.driver = {
 		.name  = "gpio-vbus",
 		.owner = THIS_MODULE,
+		.of_match_table = of_match_ptr(gpio_vbus_dt_ids),
 #ifdef CONFIG_PM
 		.pm = &gpio_vbus_dev_pm_ops,
 #endif
-- 
2.1.0

--
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 v1 2/3] usb: phy: convert gpio-vbus to gpio_desc

[Philipp Zabel]

On Sun, Nov 2, 2014 at 7:11 PM, Robert Jarzmik <robert.jarzmik-GANU6spQydw@public.gmane.org> wrote:
> In order to prepare device-tree conversion, port gpio_vbus to use
> gpio_desc.
>
> Signed-off-by: Robert Jarzmik <robert.jarzmik-GANU6spQydw@public.gmane.org>

Acked-by: Philipp Zabel <philipp.zabel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

regards
Philipp
--
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 v1 1/3] usb: phy: device-tree documentation for gpio-vbus

[Philipp Zabel]

Hi Robert,

On Sun, Nov 2, 2014 at 7:11 PM, Robert Jarzmik <robert.jarzmik-GANU6spQydw@public.gmane.org> wrote:
> Add documentation for device-tree binding of the generic gpio-vbus phy
> controller.
>
> Signed-off-by: Robert Jarzmik <robert.jarzmik-GANU6spQydw@public.gmane.org>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---
>  .../devicetree/bindings/phy/gpio-vbus.txt          | 23 ++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/gpio-vbus.txt
>
> diff --git a/Documentation/devicetree/bindings/phy/gpio-vbus.txt b/Documentation/devicetree/bindings/phy/gpio-vbus.txt
> new file mode 100644
> index 0000000..ffcfd35
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/gpio-vbus.txt
> @@ -0,0 +1,23 @@
> +GPIO VBus phy
> +=============
> +
> +gpio-vbus is a phy controller supporting VBus detection and pullup activation on
> +GPIOs.

Maybe duplicate the comment from the driver here how his is about
B peripheral only devices

> +Required properties:
> +- compatible : should be "generic,phy-gpio-vbus"

I'm not sure about the compatible value. I have not seen the "generic,"
vendor prefix, and all the other generic gpio-something bindings don't
use any prefix: "gpio-gate-clock", "gpio-leds", "gpio-beeper",
"pps-gpio", etc.

> +- #phy-cells : from the generic PHY bindings, must be 1.
> +- gpios      : set of 2 gpio properties (see gpio.txt for gpio properties format)
> +               first gpio is required, it's the VBus sensing input gpio
> +              second gpio is optional, it's the D+ pullup controlling output
> +              gpio
> +
> +Optional properties:
> +- wakeup     : boolean, if true the VBus gpio will wakeup the platform

The vbus_draw regulator should be part of this binding, I think.

> +Example:
> +       usb2phy : gpio-vbus@13 {
> +               compatible = "generic,phy-gpio-vbus";
> +               gpios = <&gpio 13 GPIO_ACTIVE_LOW>;
> +               wakeup;

This on the other hand might be too generic.
I'd like to see just wakeup used here, but the other bindings prefix
"linux," (or "gpio-key,").

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

Re: [PATCH v1 3/3] usb: phy: add device-tree support for gpio-vbus

[Philipp Zabel]

Hi Robert,

On Sun, Nov 2, 2014 at 7:11 PM, Robert Jarzmik <robert.jarzmik-GANU6spQydw@public.gmane.org> wrote:
> Add device-tree support for the generic gpio-vbus driver.
>
> Signed-off-by: Robert Jarzmik <robert.jarzmik-GANU6spQydw@public.gmane.org>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---
>  drivers/usb/phy/phy-gpio-vbus-usb.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/drivers/usb/phy/phy-gpio-vbus-usb.c b/drivers/usb/phy/phy-gpio-vbus-usb.c
> index d302ab2..6194728 100644
> --- a/drivers/usb/phy/phy-gpio-vbus-usb.c
> +++ b/drivers/usb/phy/phy-gpio-vbus-usb.c
> @@ -13,6 +13,7 @@
>  #include <linux/gpio/consumer.h>
>  #include <linux/gpio.h>
>  #include <linux/module.h>
> +#include <linux/of_gpio.h>
>  #include <linux/slab.h>
>  #include <linux/interrupt.h>
>  #include <linux/usb.h>
> @@ -254,6 +255,12 @@ static int gpio_vbus_probe(struct platform_device *pdev)
>                 }
>                 gpiod = gpio_to_desc(gpio);
>                 wakeup = pdata->wakeup;
> +       } else {
> +               gpiod = devm_gpiod_get_index(&pdev->dev, NULL, 0, GPIOD_IN);
> +               if (pdev->dev.of_node &&
> +                   of_get_property(pdev->dev.of_node,
> +                                   "wakeup", NULL))
> +                       wakeup = 1;
>         }
>         if (!gpiod)
>                 return -EINVAL;
> @@ -309,6 +316,8 @@ static int gpio_vbus_probe(struct platform_device *pdev)
>                         gpiod = gpio_to_desc(gpio);
>                         gpiod_direction_output(gpiod, 0);
>                 }
> +       } else {
> +               gpiod = devm_gpiod_get_index(&pdev->dev, NULL, 1, 0);

The pdata path has gpiod_direction_output(gpiod, 0), so should't the flag here
be GPIOD_OUT_LOW?

>         }
>         if (!IS_ERR(gpiod))
>                 gpio_vbus->gpiod_pullup = gpiod;
> @@ -382,12 +391,18 @@ static const struct dev_pm_ops gpio_vbus_dev_pm_ops = {
>  };
>  #endif
>
> +static struct of_device_id gpio_vbus_dt_ids[] = {
> +       { .compatible = "generic,phy-gpio-vbus" },

See my reply to the binding docs, I think the "generic," might have to go.

regards
Philipp
--
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 v1 1/3] usb: phy: device-tree documentation for gpio-vbus

[Robert Jarzmik]

Philipp Zabel <philipp.zabel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:

> Hi Robert,
>
> On Sun, Nov 2, 2014 at 7:11 PM, Robert Jarzmik <robert.jarzmik-GANU6spQydw@public.gmane.org> wrote:

> Maybe duplicate the comment from the driver here how his is about
> B peripheral only devices
Yeah sure, for v2.

>> +Required properties:
>> +- compatible : should be "generic,phy-gpio-vbus"
>
> I'm not sure about the compatible value. I have not seen the "generic,"
> vendor prefix, and all the other generic gpio-something bindings don't
> use any prefix: "gpio-gate-clock", "gpio-leds", "gpio-beeper",
> "pps-gpio", etc.
Okay, so we'll probably end up on "phy-gpio-vbus" then.

>> +- #phy-cells : from the generic PHY bindings, must be 1.
>> +- gpios      : set of 2 gpio properties (see gpio.txt for gpio properties format)
>> +               first gpio is required, it's the VBus sensing input gpio
>> +              second gpio is optional, it's the D+ pullup controlling output
>> +              gpio
>> +
>> +Optional properties:
>> +- wakeup     : boolean, if true the VBus gpio will wakeup the platform
>
> The vbus_draw regulator should be part of this binding, I think.
Indeed. It should be optional, right ? Schedules for v2.

>> +Example:
>> +       usb2phy : gpio-vbus@13 {
>> +               compatible = "generic,phy-gpio-vbus";
>> +               gpios = <&gpio 13 GPIO_ACTIVE_LOW>;
>> +               wakeup;
>
> This on the other hand might be too generic.
> I'd like to see just wakeup used here, but the other bindings prefix
> "linux," (or "gpio-key,").
If I write this, would you change to better suit you ?
	usb2phy : gpio-vbus@13 {
		compatible = "phy-gpio-vbus";
		regulator-vbus: regulator@0 {
                	regulator-min-microvolt = <5000000>;
                	regulator-max-microvolt = <5000000>;
			regulator-always-on;
		};
		vbus-gpio = <&gpio 13 GPIO_ACTIVE_LOW>;
		wakeup;
	};               

Cheers.

-- 
Robert
--
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 v1 1/3] usb: phy: device-tree documentation for gpio-vbus

[Sergei Shtylyov]

Hello.

On 11/2/2014 9:11 PM, Robert Jarzmik wrote:

> Add documentation for device-tree binding of the generic gpio-vbus phy
> controller.

> Signed-off-by: Robert Jarzmik <robert.jarzmik-GANU6spQydw@public.gmane.org>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---
>   .../devicetree/bindings/phy/gpio-vbus.txt          | 23 ++++++++++++++++++++++
>   1 file changed, 23 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/phy/gpio-vbus.txt

> diff --git a/Documentation/devicetree/bindings/phy/gpio-vbus.txt b/Documentation/devicetree/bindings/phy/gpio-vbus.txt
> new file mode 100644
> index 0000000..ffcfd35
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/gpio-vbus.txt
> @@ -0,0 +1,23 @@
> +GPIO VBus phy

    It's either VBUS or Vbus.

> +=============
> +
> +gpio-vbus is a phy controller supporting VBus detection and pullup activation on

    s/phy/PHY/.

> +GPIOs.
> +
> +Required properties:
> +- compatible : should be "generic,phy-gpio-vbus"

    "generic," not needed.

> +- #phy-cells : from the generic PHY bindings, must be 1.

    However, you don't specify it in the example. It's also not clear why you 
need 1, not 0.

> +- gpios      : set of 2 gpio properties (see gpio.txt for gpio properties format)
> +               first gpio is required, it's the VBus sensing input gpio
> +	       second gpio is optional, it's the D+ pullup controlling output
> +	       gpio

    s/gpio/GPIO/.

> +
> +Optional properties:
> +- wakeup     : boolean, if true the VBus gpio will wakeup the platform
> +
> +Example:
> +	usb2phy : gpio-vbus@13 {

    Please use the generic node name ("usb-phy") in order to comply with the 
ePAPR standard.

> +		compatible = "generic,phy-gpio-vbus";
> +		gpios = <&gpio 13 GPIO_ACTIVE_LOW>;
> +		wakeup;
> +	};

WBR, Sergei

--
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 v1 2/3] usb: phy: convert gpio-vbus to gpio_desc

[Felipe Balbi]

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

Hi,

On Sun, Nov 02, 2014 at 07:11:49PM +0100, Robert Jarzmik wrote:
> In order to prepare device-tree conversion, port gpio_vbus to use
> gpio_desc.
> 
> Signed-off-by: Robert Jarzmik <robert.jarzmik-GANU6spQydw@public.gmane.org>

Can we just convert users of this to a phy-generic.c with a regulator ?

This is basically what gpio-vbus is doing, it's basically a regulator.

We can figure out how to maintain backwards compatibility with older DTS
too, no problem.

cheers

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v1 3/3] usb: phy: add device-tree support for gpio-vbus

[Felipe Balbi]

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

On Sun, Nov 02, 2014 at 07:11:50PM +0100, Robert Jarzmik wrote:
> Add device-tree support for the generic gpio-vbus driver.
> 
> Signed-off-by: Robert Jarzmik <robert.jarzmik-GANU6spQydw@public.gmane.org>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---

I would rather see you converting to a fixed-regulator with phy-generic.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v1 2/3] usb: phy: convert gpio-vbus to gpio_desc

[Robert Jarzmik]

Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org> writes:

> Hi,
>
> On Sun, Nov 02, 2014 at 07:11:49PM +0100, Robert Jarzmik wrote:
>> In order to prepare device-tree conversion, port gpio_vbus to use
>> gpio_desc.
>> 
>> Signed-off-by: Robert Jarzmik <robert.jarzmik-GANU6spQydw@public.gmane.org>
>
> Can we just convert users of this to a phy-generic.c with a regulator ?
Maybe, let's see what is missing.

> This is basically what gpio-vbus is doing, it's basically a regulator.
And a detector too. The basic thing is that it request an interrupt, and upon
this interrupt it schedules through a workqueue a usb_gadget_vbus_connect() and
the regulator stuff.

I don't see the interrupt+ usb_gadget_vbus_connect() stuff that in the
phy-generic. Am I missing something ?

Cheers.

-- 
Robert
--
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 v1 2/3] usb: phy: convert gpio-vbus to gpio_desc

[Felipe Balbi]

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

On Wed, Nov 05, 2014 at 08:46:58PM +0100, Robert Jarzmik wrote:
> Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org> writes:
> 
> > Hi,
> >
> > On Sun, Nov 02, 2014 at 07:11:49PM +0100, Robert Jarzmik wrote:
> >> In order to prepare device-tree conversion, port gpio_vbus to use
> >> gpio_desc.
> >> 
> >> Signed-off-by: Robert Jarzmik <robert.jarzmik-GANU6spQydw@public.gmane.org>
> >
> > Can we just convert users of this to a phy-generic.c with a regulator ?
> Maybe, let's see what is missing.
> 
> > This is basically what gpio-vbus is doing, it's basically a regulator.
> And a detector too. The basic thing is that it request an interrupt, and upon
> this interrupt it schedules through a workqueue a usb_gadget_vbus_connect() and
> the regulator stuff.
> 
> I don't see the interrupt+ usb_gadget_vbus_connect() stuff that in the
> phy-generic. Am I missing something ?

Well, let's add that :-) Just make it optional. It's pointless to have
80% duplicated code just because of 20% missing in phy-generic :-)

Then we avoid adding gpio-vbus specific DT properties too.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v1 2/3] usb: phy: convert gpio-vbus to gpio_desc

[Robert Jarzmik]

Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org> writes:

> On Wed, Nov 05, 2014 at 08:46:58PM +0100, Robert Jarzmik wrote:
> Well, let's add that :-) Just make it optional. It's pointless to have
> 80% duplicated code just because of 20% missing in phy-generic :-)
>
> Then we avoid adding gpio-vbus specific DT properties too.
OK, got it.

It will take me a couple of days. Philipp, am I missing something apart the
detection and connect stuff ? While I'm at making my board work with
phy-generic, let's thing ahead.

Felipe, that will mean at least this for phy-generic :
 - usb_phy_gen_create_phy() will be enhanced
   => struct usb_phy_generic_platform_data will get a :
     - int gpio_vbus field (or whatever name you wish)
     - int gpio_vbus_inverted (or maybe we could go directly for gpio desc)
     - int gpio_pullup field (I'm not sure here, maybe we should just drop that)
     - bool wakeup field (or another name)
   => device tree will get :
     - a vbus-gpio (or another name)
     - a pullup-gpio (or nothing if we drop)
 - there will be a request_irq() and a workqueue (mostly taken from gpio-vbus)
   => will call usb_gadget_vbus_connect()
   => will call usb_gadget_vbus_disconnect()

I'm writing all this just to be sure I have the good picture before I start
coding.

Cheers.

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

Re: [PATCH v1 2/3] usb: phy: convert gpio-vbus to gpio_desc

[Felipe Balbi]

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

Hi,

On Wed, Nov 05, 2014 at 09:02:04PM +0100, Robert Jarzmik wrote:
> Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org> writes:
> 
> > On Wed, Nov 05, 2014 at 08:46:58PM +0100, Robert Jarzmik wrote:
> > Well, let's add that :-) Just make it optional. It's pointless to have
> > 80% duplicated code just because of 20% missing in phy-generic :-)
> >
> > Then we avoid adding gpio-vbus specific DT properties too.
> OK, got it.
> 
> It will take me a couple of days. Philipp, am I missing something apart the
> detection and connect stuff ? While I'm at making my board work with
> phy-generic, let's thing ahead.
> 
> Felipe, that will mean at least this for phy-generic :
>  - usb_phy_gen_create_phy() will be enhanced
>    => struct usb_phy_generic_platform_data will get a :
>      - int gpio_vbus field (or whatever name you wish)
>      - int gpio_vbus_inverted (or maybe we could go directly for gpio desc)

Actually, you might want to first convert phy-generic to gpio_desc and
avoid the inverted field.

>      - int gpio_pullup field (I'm not sure here, maybe we should just drop that)
>      - bool wakeup field (or another name)

sonds good to me.

>    => device tree will get :
>      - a vbus-gpio (or another name)
>      - a pullup-gpio (or nothing if we drop)

fine by me, as long as their all optional and agreed with devicetree
folks. I think we still have time for v3.19 if you manage to finish this
before next week's end.

>  - there will be a request_irq() and a workqueue (mostly taken from gpio-vbus)
>    => will call usb_gadget_vbus_connect()
>    => will call usb_gadget_vbus_disconnect()

the workqueue should be unnecessary if you use
devm_request_threaded_irq() without a top half.

> I'm writing all this just to be sure I have the good picture before I
> start coding.

sounds good to me :-)

When it comes to DT, let's try to keep things as generic as possible so
we can just move phy-generic.c into drivers/phy/ later on without much
effort ;-)

I guess everything that you need already has existent bindings.

cheers

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v1 2/3] usb: phy: convert gpio-vbus to gpio_desc

[Robert Jarzmik]

Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org> writes:

> fine by me, as long as their all optional and agreed with devicetree
> folks. I think we still have time for v3.19 if you manage to finish this
> before next week's end.
I will try, no promise I'll succeed in this window.

At least I should fire out within the next days the gpiod patch and the DT
documentation patch if I want to respect the schedule.

Cheers.

-- 
Robert
--
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 v1 2/3] usb: phy: convert gpio-vbus to gpio_desc

[Felipe Balbi]

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

On Sat, Nov 08, 2014 at 06:45:59PM +0100, Robert Jarzmik wrote:
> Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org> writes:
> 
> > fine by me, as long as their all optional and agreed with devicetree

dumb me: s/their/they're

> > folks. I think we still have time for v3.19 if you manage to finish this
> > before next week's end.
> I will try, no promise I'll succeed in this window.

sure, no pressure either :-)

> At least I should fire out within the next days the gpiod patch and
> the DT documentation patch if I want to respect the schedule.

good, we can at least cut down some dependencies for v3.20.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]