[PATCH v2] phy: rcar-gen2 usb: Add Host/Function switching for USB0

[PATCH v2] phy: rcar-gen2 usb: Add Host/Function switching for USB0

[Phil Edworthy]

Instead of statically selecting the PHY connection to either the
USBHS (Function) or PCI0 (Host) IP blocks, this change allows the
dts to specifiy gpio pins for the vbus and id signals. Additional
gpio pins are used to control power to an external OTG device and
an override to turn vbus on/off.

Note: the R-Car USB PHY only allows this Host/Function switching
on channel 0.

This has been tested on a r8a7791 based Koelsch board, which uses
a MAX3355 device to supply vbus power when needed.

Signed-off-by: Phil Edworthy <phil.edworthy-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>

---
Tested with patch "usb: renesas_usbhs: Allow an OTG PHY driver to provide VBUS"
and changes to koelsch dts.

v2:
  - Added -gpio to dts prop names of GPIO pins.
  - Document the new bindings.
---
 .../devicetree/bindings/phy/rcar-gen2-phy.txt      |  10 +
 drivers/phy/phy-rcar-gen2.c                        | 269 +++++++++++++++++++--
 2 files changed, 257 insertions(+), 22 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/rcar-gen2-phy.txt b/Documentation/devicetree/bindings/phy/rcar-gen2-phy.txt
index 00fc52a..3a501fe 100644
--- a/Documentation/devicetree/bindings/phy/rcar-gen2-phy.txt
+++ b/Documentation/devicetree/bindings/phy/rcar-gen2-phy.txt
@@ -30,6 +30,16 @@ the USB channel; see the selector meanings below:
 | 2         | PCI EHCI/OHCI | xHCI          |
 +-----------+---------------+---------------+
 
+Optional properties:
+  - renesas,pwr-gpio: 		A gpio specifier that will be active when the
+				PHY is powered on.
+  - renesas,id-gpio:		A gpio specifier that is read to get the USB
+				ID signal.
+  - renesas,vbus-gpio:		A gpio specifier that is read to get the USB
+				VBUS signal.
+  - renesas,vbus-pwr-gpio:	A gpio specifier that will be active when VBUS
+				is required to be powered.
+
 Example (Lager board):
 
 	usb-phy@e6590100 {
diff --git a/drivers/phy/phy-rcar-gen2.c b/drivers/phy/phy-rcar-gen2.c
index 97d45f4..1e7a4d9 100644
--- a/drivers/phy/phy-rcar-gen2.c
+++ b/drivers/phy/phy-rcar-gen2.c
@@ -1,5 +1,5 @@
 /*
- * Renesas R-Car Gen2 PHY driver
+ * Renesas R-Car Gen2 USB PHY driver
  *
  * Copyright (C) 2014 Renesas Solutions Corp.
  * Copyright (C) 2014 Cogent Embedded, Inc.
@@ -12,11 +12,16 @@
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/io.h>
+#include <linux/interrupt.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_gpio.h>
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
 #include <linux/spinlock.h>
+#include <linux/usb/gadget.h>
+#include <linux/usb/otg.h>
+#include <linux/workqueue.h>
 
 #include <asm/cmpxchg.h>
 
@@ -58,6 +63,18 @@ struct rcar_gen2_channel {
 	struct rcar_gen2_phy phys[PHYS_PER_CHANNEL];
 	int selected_phy;
 	u32 select_mask;
+
+	/* external power enable pin */
+	int			gpio_pwr;
+
+	/* Host/Function switching */
+	struct delayed_work	work;
+	int			use_otg;
+	int			gpio_vbus;
+	int			gpio_id;
+	int			gpio_vbus_pwr;
+	struct usb_phy		*usbphy;
+	struct usb_otg		*otg;
 };
 
 struct rcar_gen2_phy_driver {
@@ -68,31 +85,50 @@ struct rcar_gen2_phy_driver {
 	struct rcar_gen2_channel *channels;
 };
 
-static int rcar_gen2_phy_init(struct phy *p)
+static void rcar_gen2_phy_switch(struct rcar_gen2_channel *channel,
+	u32 select_value)
 {
-	struct rcar_gen2_phy *phy = phy_get_drvdata(p);
-	struct rcar_gen2_channel *channel = phy->channel;
 	struct rcar_gen2_phy_driver *drv = channel->drv;
 	unsigned long flags;
 	u32 ugctrl2;
 
-	/*
-	 * Try to acquire exclusive access to PHY.  The first driver calling
-	 * phy_init()  on a given channel wins, and all attempts  to use another
-	 * PHY on this channel will fail until phy_exit() is called by the first
-	 * driver.   Achieving this with cmpxcgh() should be SMP-safe.
-	 */
-	if (cmpxchg(&channel->selected_phy, -1, phy->number) != -1)
-		return -EBUSY;
-
-	clk_prepare_enable(drv->clk);
-
 	spin_lock_irqsave(&drv->lock, flags);
 	ugctrl2 = readl(drv->base + USBHS_UGCTRL2);
 	ugctrl2 &= ~channel->select_mask;
-	ugctrl2 |= phy->select_value;
+	ugctrl2 |= select_value;
 	writel(ugctrl2, drv->base + USBHS_UGCTRL2);
 	spin_unlock_irqrestore(&drv->lock, flags);
+}
+
+static int rcar_gen2_phy_init(struct phy *p)
+{
+	struct rcar_gen2_phy *phy = phy_get_drvdata(p);
+	struct rcar_gen2_channel *channel = phy->channel;
+	struct rcar_gen2_phy_driver *drv = channel->drv;
+
+	if (!channel->use_otg) {
+		/*
+		 * Static Host/Function role.
+		 * Try to acquire exclusive access to PHY. The first driver
+		 * calling phy_init() on a given channel wins, and all attempts
+		 * to use another PHY on this channel will fail until
+		 * phy_exit() is called by the first driver. Achieving this
+		 * with cmpxcgh() should be SMP-safe.
+		 */
+		if (cmpxchg(&channel->selected_phy, -1, phy->number) != -1)
+			return -EBUSY;
+
+		clk_prepare_enable(drv->clk);
+		rcar_gen2_phy_switch(channel, phy->select_value);
+	} else {
+		/*
+		 * Using Host/Function switching, so schedule work to determine
+		 * which role to use.
+		 */
+		clk_prepare_enable(drv->clk);
+		schedule_delayed_work(&channel->work, 100);
+	}
+
 	return 0;
 }
 
@@ -117,9 +153,9 @@ static int rcar_gen2_phy_power_on(struct phy *p)
 	u32 value;
 	int err = 0, i;
 
-	/* Skip if it's not USBHS */
-	if (phy->select_value != USBHS_UGCTRL2_USB0SEL_HS_USB)
-		return 0;
+	/* Optional external power pin */
+	if (gpio_is_valid(phy->channel->gpio_pwr))
+		gpio_direction_output(phy->channel->gpio_pwr, 1);
 
 	spin_lock_irqsave(&drv->lock, flags);
 
@@ -160,9 +196,9 @@ static int rcar_gen2_phy_power_off(struct phy *p)
 	unsigned long flags;
 	u32 value;
 
-	/* Skip if it's not USBHS */
-	if (phy->select_value != USBHS_UGCTRL2_USB0SEL_HS_USB)
-		return 0;
+	/* External power pin */
+	if (gpio_is_valid(phy->channel->gpio_pwr))
+		gpio_direction_output(phy->channel->gpio_pwr, 0);
 
 	spin_lock_irqsave(&drv->lock, flags);
 
@@ -236,6 +272,132 @@ static const u32 select_value[][PHYS_PER_CHANNEL] = {
 	[2]	= { USBHS_UGCTRL2_USB2SEL_PCI, USBHS_UGCTRL2_USB2SEL_USB30 },
 };
 
+
+#define VBUS_IRQ_FLAGS \
+	(IRQF_SHARED | IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING)
+
+static void gpio_vbus_work(struct work_struct *work)
+{
+	struct rcar_gen2_channel *channel = container_of(work,
+					struct rcar_gen2_channel, work.work);
+	struct usb_phy *usbphy = channel->usbphy;
+	int status, vbus, id;
+
+	vbus = !!gpio_get_value(channel->gpio_vbus);
+	id = !!gpio_get_value(channel->gpio_id);
+
+	/* Switch the PHY over */
+	if (id)
+		rcar_gen2_phy_switch(channel, USBHS_UGCTRL2_USB0SEL_HS_USB);
+	else
+		rcar_gen2_phy_switch(channel, USBHS_UGCTRL2_USB0SEL_PCI);
+
+	/* If VBUS is powered and we are not the initial Host, turn off VBUS */
+	if (gpio_is_valid(channel->gpio_vbus_pwr))
+		gpio_direction_output(channel->gpio_vbus_pwr, !(id && vbus));
+
+	if (!channel->otg->gadget)
+		return;
+
+	/* Function handling: vbus=1 when initially plugged into a Host */
+	if (vbus) {
+		status = USB_EVENT_VBUS;
+		usbphy->otg->state = OTG_STATE_B_PERIPHERAL;
+		usbphy->last_event = status;
+		usb_gadget_vbus_connect(usbphy->otg->gadget);
+
+		atomic_notifier_call_chain(&usbphy->notifier,
+					   status, usbphy->otg->gadget);
+	} else {
+		usb_gadget_vbus_disconnect(usbphy->otg->gadget);
+		status = USB_EVENT_NONE;
+		usbphy->otg->state = OTG_STATE_B_IDLE;
+		usbphy->last_event = status;
+
+		atomic_notifier_call_chain(&usbphy->notifier,
+					   status, usbphy->otg->gadget);
+	}
+}
+
+/* VBUS change IRQ handler */
+static irqreturn_t gpio_vbus_irq(int irq, void *data)
+{
+	struct rcar_gen2_channel *channel = data;
+
+	/* Wait 20ms before doing anything as VBUS needs to settle */
+	schedule_delayed_work(&channel->work, msecs_to_jiffies(20));
+
+	return IRQ_HANDLED;
+}
+
+static int probe_otg(struct platform_device *pdev,
+	struct rcar_gen2_phy_driver *drv)
+{
+	struct device *dev = &pdev->dev;
+	struct rcar_gen2_channel *ch = drv->channels;
+	int irq;
+	int ret;
+
+	/* GPIOs for Host/Fn switching */
+	ch->gpio_id = of_get_named_gpio_flags(dev->of_node,
+				"renesas,id-gpio", 0, NULL);
+	ch->gpio_vbus = of_get_named_gpio_flags(dev->of_node,
+				"renesas,vbus-gpio", 0, NULL);
+
+	/* Need valid ID and VBUS gpios for Host/Fn switching */
+	if (gpio_is_valid(ch->gpio_id) && gpio_is_valid(ch->gpio_vbus)) {
+		ch->use_otg = 1;
+
+		/* GPIO for ID input */
+		ret = devm_gpio_request_one(dev, ch->gpio_id, GPIOF_IN, "id");
+		if (ret)
+			return ret;
+
+		/* GPIO for VBUS input */
+		ret = devm_gpio_request_one(dev, ch->gpio_vbus, GPIOF_IN, "vbus");
+		if (ret)
+			return ret;
+
+		irq = gpio_to_irq(ch->gpio_vbus);
+		ret = devm_request_irq(dev, irq, gpio_vbus_irq, VBUS_IRQ_FLAGS,
+				"vbus_detect", ch);
+		if (ret)
+			return ret;
+
+		/* Optional GPIO for VBUS power */
+		ch->gpio_vbus_pwr = of_get_named_gpio_flags(dev->of_node,
+						"renesas,vbus-pwr-gpio", 0, NULL);
+		if (gpio_is_valid(ch->gpio_id)) {
+			ret = devm_gpio_request_one(dev, ch->gpio_vbus_pwr,
+					GPIOF_OUT_INIT_LOW, "vbus-pwr");
+			if (ret)
+				return ret;
+		}
+
+	} else if (gpio_is_valid(ch->gpio_id)) {
+		dev_err(dev, "Failed to get VBUS gpio\n");
+		return ch->gpio_vbus;
+	} else if (gpio_is_valid(ch->gpio_vbus)) {
+		dev_err(dev, "Failed to get ID gpio\n");
+		return ch->gpio_id;
+	}
+
+	return 0;
+}
+
+/* bind/unbind the peripheral controller */
+static int rcar_gen2_usb_set_peripheral(struct usb_otg *otg,
+					struct usb_gadget *gadget)
+{
+	otg->gadget = gadget;
+	if (!gadget) {
+		usb_gadget_vbus_disconnect(otg->gadget);
+		otg->state = OTG_STATE_UNDEFINED;
+	}
+
+	return 0;
+}
+
 static int rcar_gen2_phy_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -245,7 +407,9 @@ static int rcar_gen2_phy_probe(struct platform_device *pdev)
 	struct resource *res;
 	void __iomem *base;
 	struct clk *clk;
+	struct usb_otg *otg;
 	int i = 0;
+	int err;
 
 	if (!dev->of_node) {
 		dev_err(dev,
@@ -280,6 +444,57 @@ static int rcar_gen2_phy_probe(struct platform_device *pdev)
 	if (!drv->channels)
 		return -ENOMEM;
 
+	/* USB0 optional GPIO power pin for external devices */
+	drv->channels->gpio_pwr = of_get_named_gpio_flags(dev->of_node,
+						"renesas,pwr-gpio", 0, NULL);
+	if (drv->channels->gpio_pwr == -EPROBE_DEFER)
+		return -EPROBE_DEFER;
+
+	if (gpio_is_valid(drv->channels->gpio_pwr)) {
+		err = devm_gpio_request(dev, drv->channels->gpio_pwr, "pwr");
+		if (err)
+			return err;
+	}
+
+	/* USB0 Host/Function switching info */
+	err = probe_otg(pdev, drv);
+	if (err)
+		return err;
+
+	/*
+	 * The PHY connected to channel 0 can be used to steer signals to the
+	 * USB Host IP that stils behind a PCI bridge (pci0), or the USB Func
+	 * IP (hsusb). We can dynamically switch this based on VBUS and ID
+	 * signals connected to gpios, to get something approaching OTG.
+	 */
+	if (drv->channels->use_otg) {
+		struct usb_phy *usbphy;
+
+		usbphy = devm_kzalloc(dev, sizeof(*usbphy), GFP_KERNEL);
+		if (!usbphy)
+			return -ENOMEM;
+
+		otg = devm_kzalloc(dev, sizeof(*otg), GFP_KERNEL);
+		if (!otg)
+			return -ENOMEM;
+
+		usbphy->dev		= dev;
+		usbphy->otg		= otg;
+
+		otg->usb_phy		= usbphy;
+		otg->state		= OTG_STATE_UNDEFINED;
+		otg->set_peripheral	= rcar_gen2_usb_set_peripheral;
+
+		drv->channels->otg	= otg;
+		drv->channels->usbphy	= usbphy;
+
+		ATOMIC_INIT_NOTIFIER_HEAD(&usbphy->notifier);
+
+		INIT_DELAYED_WORK(&drv->channels->work, gpio_vbus_work);
+
+		usb_add_phy_dev(usbphy);
+	}
+
 	for_each_child_of_node(dev->of_node, np) {
 		struct rcar_gen2_channel *channel = drv->channels + i;
 		u32 channel_num;
@@ -288,6 +503,8 @@ static int rcar_gen2_phy_probe(struct platform_device *pdev)
 		channel->of_node = np;
 		channel->drv = drv;
 		channel->selected_phy = -1;
+		if (i != 0)
+			channel->gpio_pwr = -ENOENT;
 
 		error = of_property_read_u32(np, "reg", &channel_num);
 		if (error || channel_num > 2) {
@@ -323,6 +540,14 @@ static int rcar_gen2_phy_probe(struct platform_device *pdev)
 
 	dev_set_drvdata(dev, drv);
 
+	/*
+	 * If we already have something plugged into USB0, we won't get an edge
+	 * on VBUS, so we have to manually schedule work to look at the VBUS
+	 * and ID signals.
+	 */
+	if (drv->channels->use_otg)
+		schedule_delayed_work(&drv->channels->work, msecs_to_jiffies(100));
+
 	return 0;
 }
 
-- 
1.9.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

RE: [PATCH v2] phy: rcar-gen2 usb: Add Host/Function switching for USB0

[Yoshihiro Shimoda]

Hi Phil-san,

Thank you very much for the patch!

> Sent: Thursday, July 02, 2015 5:06 PM
< snip >
> +/* VBUS change IRQ handler */
> +static irqreturn_t gpio_vbus_irq(int irq, void *data)
> +{
> +	struct rcar_gen2_channel *channel = data;
> +
> +	/* Wait 20ms before doing anything as VBUS needs to settle */
> +	schedule_delayed_work(&channel->work, msecs_to_jiffies(20));
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int probe_otg(struct platform_device *pdev,
> +	struct rcar_gen2_phy_driver *drv)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct rcar_gen2_channel *ch = drv->channels;
> +	int irq;
> +	int ret;
> +
> +	/* GPIOs for Host/Fn switching */
> +	ch->gpio_id = of_get_named_gpio_flags(dev->of_node,
> +				"renesas,id-gpio", 0, NULL);
> +	ch->gpio_vbus = of_get_named_gpio_flags(dev->of_node,
> +				"renesas,vbus-gpio", 0, NULL);
> +
> +	/* Need valid ID and VBUS gpios for Host/Fn switching */
> +	if (gpio_is_valid(ch->gpio_id) && gpio_is_valid(ch->gpio_vbus)) {
> +		ch->use_otg = 1;
> +
> +		/* GPIO for ID input */
> +		ret = devm_gpio_request_one(dev, ch->gpio_id, GPIOF_IN, "id");
> +		if (ret)
> +			return ret;
> +
> +		/* GPIO for VBUS input */
> +		ret = devm_gpio_request_one(dev, ch->gpio_vbus, GPIOF_IN, "vbus");

According to the checkpatch.pl, "line over 80 characters".

> +		if (ret)
> +			return ret;
> +
> +		irq = gpio_to_irq(ch->gpio_vbus);
> +		ret = devm_request_irq(dev, irq, gpio_vbus_irq, VBUS_IRQ_FLAGS,
> +				"vbus_detect", ch);
> +		if (ret)
> +			return ret;
> +
> +		/* Optional GPIO for VBUS power */
> +		ch->gpio_vbus_pwr = of_get_named_gpio_flags(dev->of_node,
> +						"renesas,vbus-pwr-gpio", 0, NULL);

Same above.

> +		if (gpio_is_valid(ch->gpio_id)) {
> +			ret = devm_gpio_request_one(dev, ch->gpio_vbus_pwr,
> +					GPIOF_OUT_INIT_LOW, "vbus-pwr");
> +			if (ret)
> +				return ret;
> +		}
> +
> +	} else if (gpio_is_valid(ch->gpio_id)) {
> +		dev_err(dev, "Failed to get VBUS gpio\n");
> +		return ch->gpio_vbus;
> +	} else if (gpio_is_valid(ch->gpio_vbus)) {
> +		dev_err(dev, "Failed to get ID gpio\n");
> +		return ch->gpio_id;
> +	}
> +
> +	return 0;
> +}
> +
> +/* bind/unbind the peripheral controller */
> +static int rcar_gen2_usb_set_peripheral(struct usb_otg *otg,
> +					struct usb_gadget *gadget)
> +{
> +	otg->gadget = gadget;
> +	if (!gadget) {
> +		usb_gadget_vbus_disconnect(otg->gadget);

Since the otg->gadget is NULL, this driver should not call this function here.

> +		otg->state = OTG_STATE_UNDEFINED;
> +	}
> +
> +	return 0;
> +}
> +
>  static int rcar_gen2_phy_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
< snip >
> @@ -323,6 +540,14 @@ static int rcar_gen2_phy_probe(struct platform_device *pdev)
> 
>  	dev_set_drvdata(dev, drv);
> 
> +	/*
> +	 * If we already have something plugged into USB0, we won't get an edge
> +	 * on VBUS, so we have to manually schedule work to look at the VBUS
> +	 * and ID signals.
> +	 */
> +	if (drv->channels->use_otg)
> +		schedule_delayed_work(&drv->channels->work, msecs_to_jiffies(100));

This line is also "line over 80 characters".

Best regards,
Yoshihiro Shimoda

> +
>  	return 0;
>  }
> 
> --
> 1.9.1

RE: [PATCH v2] phy: rcar-gen2 usb: Add Host/Function switching for USB0

[Phil Edworthy]

Hi Shimoda-san,

On 06 July 2015 08:18, Shimoda-san wrote:
> Hi Phil-san,
> 
> Thank you very much for the patch!
> 
> > Sent: Thursday, July 02, 2015 5:06 PM
> < snip >
> > +/* VBUS change IRQ handler */
> > +static irqreturn_t gpio_vbus_irq(int irq, void *data)
> > +{
> > +	struct rcar_gen2_channel *channel = data;
> > +
> > +	/* Wait 20ms before doing anything as VBUS needs to settle */
> > +	schedule_delayed_work(&channel->work, msecs_to_jiffies(20));
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static int probe_otg(struct platform_device *pdev,
> > +	struct rcar_gen2_phy_driver *drv)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	struct rcar_gen2_channel *ch = drv->channels;
> > +	int irq;
> > +	int ret;
> > +
> > +	/* GPIOs for Host/Fn switching */
> > +	ch->gpio_id = of_get_named_gpio_flags(dev->of_node,
> > +				"renesas,id-gpio", 0, NULL);
> > +	ch->gpio_vbus = of_get_named_gpio_flags(dev->of_node,
> > +				"renesas,vbus-gpio", 0, NULL);
> > +
> > +	/* Need valid ID and VBUS gpios for Host/Fn switching */
> > +	if (gpio_is_valid(ch->gpio_id) && gpio_is_valid(ch->gpio_vbus)) {
> > +		ch->use_otg = 1;
> > +
> > +		/* GPIO for ID input */
> > +		ret = devm_gpio_request_one(dev, ch->gpio_id, GPIOF_IN,
> "id");
> > +		if (ret)
> > +			return ret;
> > +
> > +		/* GPIO for VBUS input */
> > +		ret = devm_gpio_request_one(dev, ch->gpio_vbus, GPIOF_IN,
> "vbus");
> 
> According to the checkpatch.pl, "line over 80 characters".
As I understand it, the 80 chars is not a hard rule. One reason for the
80 char guideline is to avoid overly complex code, but I don't think
that can be said of the above line!

> > +		if (ret)
> > +			return ret;
> > +
> > +		irq = gpio_to_irq(ch->gpio_vbus);
> > +		ret = devm_request_irq(dev, irq, gpio_vbus_irq,
> VBUS_IRQ_FLAGS,
> > +				"vbus_detect", ch);
> > +		if (ret)
> > +			return ret;
> > +
> > +		/* Optional GPIO for VBUS power */
> > +		ch->gpio_vbus_pwr = of_get_named_gpio_flags(dev->of_node,
> > +						"renesas,vbus-pwr-gpio", 0,
> NULL);
> 
> Same above.
Since this code was already split over two lines, I agree with you here.

> 
> > +		if (gpio_is_valid(ch->gpio_id)) {
> > +			ret = devm_gpio_request_one(dev, ch->gpio_vbus_pwr,
> > +					GPIOF_OUT_INIT_LOW, "vbus-pwr");
> > +			if (ret)
> > +				return ret;
> > +		}
> > +
> > +	} else if (gpio_is_valid(ch->gpio_id)) {
> > +		dev_err(dev, "Failed to get VBUS gpio\n");
> > +		return ch->gpio_vbus;
> > +	} else if (gpio_is_valid(ch->gpio_vbus)) {
> > +		dev_err(dev, "Failed to get ID gpio\n");
> > +		return ch->gpio_id;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +/* bind/unbind the peripheral controller */
> > +static int rcar_gen2_usb_set_peripheral(struct usb_otg *otg,
> > +					struct usb_gadget *gadget)
> > +{
> > +	otg->gadget = gadget;
> > +	if (!gadget) {
> > +		usb_gadget_vbus_disconnect(otg->gadget);
> 
> Since the otg->gadget is NULL, this driver should not call this function here.
Ah, yes you are correct!

> > +		otg->state = OTG_STATE_UNDEFINED;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  static int rcar_gen2_phy_probe(struct platform_device *pdev)
> >  {
> >  	struct device *dev = &pdev->dev;
> < snip >
> > @@ -323,6 +540,14 @@ static int rcar_gen2_phy_probe(struct platform_device
> *pdev)
> >
> >  	dev_set_drvdata(dev, drv);
> >
> > +	/*
> > +	 * If we already have something plugged into USB0, we won't get an edge
> > +	 * on VBUS, so we have to manually schedule work to look at the VBUS
> > +	 * and ID signals.
> > +	 */
> > +	if (drv->channels->use_otg)
> > +		schedule_delayed_work(&drv->channels->work,
> msecs_to_jiffies(100));
> 
> This line is also "line over 80 characters".
I don't think it is necessary to force this into the 80 char recommendation.

> Best regards,
> Yoshihiro Shimoda
> 
> > +
> >  	return 0;
> >  }
> >
> > --
> > 1.9.1

Best regards
Phil

[PATCH v3] phy: rcar-gen2 usb: Add Host/Function switching for USB0

[Phil Edworthy]

Instead of statically selecting the PHY connection to either the
USBHS (Function) or PCI0 (Host) IP blocks, this change allows the
dts to specifiy gpio pins for the vbus and id signals. Additional
gpio pins are used to control power to an external OTG device and
an override to turn vbus on/off.

Note: the R-Car USB PHY only allows this Host/Function switching
on channel 0.

This has been tested on a r8a7791 based Koelsch board, which uses
a MAX3355 device to supply vbus power when needed.

Signed-off-by: Phil Edworthy <phil.edworthy-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>

---
v3:
  - Do not call usb_gadget_vbus_disconnect will a NULL ptr.
  - Formatting to avoid a line length of over 80 chars.

v2:
  - Added -gpio to dts prop names of GPIO pins.
  - Document the new bindings.
---
 .../devicetree/bindings/phy/rcar-gen2-phy.txt      |  10 +
 drivers/phy/phy-rcar-gen2.c                        | 269 +++++++++++++++++++--
 2 files changed, 257 insertions(+), 22 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/rcar-gen2-phy.txt b/Documentation/devicetree/bindings/phy/rcar-gen2-phy.txt
index 00fc52a..3a501fe 100644
--- a/Documentation/devicetree/bindings/phy/rcar-gen2-phy.txt
+++ b/Documentation/devicetree/bindings/phy/rcar-gen2-phy.txt
@@ -30,6 +30,16 @@ the USB channel; see the selector meanings below:
 | 2         | PCI EHCI/OHCI | xHCI          |
 +-----------+---------------+---------------+
 
+Optional properties:
+  - renesas,pwr-gpio: 		A gpio specifier that will be active when the
+				PHY is powered on.
+  - renesas,id-gpio:		A gpio specifier that is read to get the USB
+				ID signal.
+  - renesas,vbus-gpio:		A gpio specifier that is read to get the USB
+				VBUS signal.
+  - renesas,vbus-pwr-gpio:	A gpio specifier that will be active when VBUS
+				is required to be powered.
+
 Example (Lager board):
 
 	usb-phy@e6590100 {
diff --git a/drivers/phy/phy-rcar-gen2.c b/drivers/phy/phy-rcar-gen2.c
index 97d45f4..1286892 100644
--- a/drivers/phy/phy-rcar-gen2.c
+++ b/drivers/phy/phy-rcar-gen2.c
@@ -1,5 +1,5 @@
 /*
- * Renesas R-Car Gen2 PHY driver
+ * Renesas R-Car Gen2 USB PHY driver
  *
  * Copyright (C) 2014 Renesas Solutions Corp.
  * Copyright (C) 2014 Cogent Embedded, Inc.
@@ -12,11 +12,16 @@
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/io.h>
+#include <linux/interrupt.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_gpio.h>
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
 #include <linux/spinlock.h>
+#include <linux/usb/gadget.h>
+#include <linux/usb/otg.h>
+#include <linux/workqueue.h>
 
 #include <asm/cmpxchg.h>
 
@@ -58,6 +63,18 @@ struct rcar_gen2_channel {
 	struct rcar_gen2_phy phys[PHYS_PER_CHANNEL];
 	int selected_phy;
 	u32 select_mask;
+
+	/* external power enable pin */
+	int			gpio_pwr;
+
+	/* Host/Function switching */
+	struct delayed_work	work;
+	int			use_otg;
+	int			gpio_vbus;
+	int			gpio_id;
+	int			gpio_vbus_pwr;
+	struct usb_phy		*usbphy;
+	struct usb_otg		*otg;
 };
 
 struct rcar_gen2_phy_driver {
@@ -68,31 +85,50 @@ struct rcar_gen2_phy_driver {
 	struct rcar_gen2_channel *channels;
 };
 
-static int rcar_gen2_phy_init(struct phy *p)
+static void rcar_gen2_phy_switch(struct rcar_gen2_channel *channel,
+	u32 select_value)
 {
-	struct rcar_gen2_phy *phy = phy_get_drvdata(p);
-	struct rcar_gen2_channel *channel = phy->channel;
 	struct rcar_gen2_phy_driver *drv = channel->drv;
 	unsigned long flags;
 	u32 ugctrl2;
 
-	/*
-	 * Try to acquire exclusive access to PHY.  The first driver calling
-	 * phy_init()  on a given channel wins, and all attempts  to use another
-	 * PHY on this channel will fail until phy_exit() is called by the first
-	 * driver.   Achieving this with cmpxcgh() should be SMP-safe.
-	 */
-	if (cmpxchg(&channel->selected_phy, -1, phy->number) != -1)
-		return -EBUSY;
-
-	clk_prepare_enable(drv->clk);
-
 	spin_lock_irqsave(&drv->lock, flags);
 	ugctrl2 = readl(drv->base + USBHS_UGCTRL2);
 	ugctrl2 &= ~channel->select_mask;
-	ugctrl2 |= phy->select_value;
+	ugctrl2 |= select_value;
 	writel(ugctrl2, drv->base + USBHS_UGCTRL2);
 	spin_unlock_irqrestore(&drv->lock, flags);
+}
+
+static int rcar_gen2_phy_init(struct phy *p)
+{
+	struct rcar_gen2_phy *phy = phy_get_drvdata(p);
+	struct rcar_gen2_channel *channel = phy->channel;
+	struct rcar_gen2_phy_driver *drv = channel->drv;
+
+	if (!channel->use_otg) {
+		/*
+		 * Static Host/Function role.
+		 * Try to acquire exclusive access to PHY. The first driver
+		 * calling phy_init() on a given channel wins, and all attempts
+		 * to use another PHY on this channel will fail until
+		 * phy_exit() is called by the first driver. Achieving this
+		 * with cmpxcgh() should be SMP-safe.
+		 */
+		if (cmpxchg(&channel->selected_phy, -1, phy->number) != -1)
+			return -EBUSY;
+
+		clk_prepare_enable(drv->clk);
+		rcar_gen2_phy_switch(channel, phy->select_value);
+	} else {
+		/*
+		 * Using Host/Function switching, so schedule work to determine
+		 * which role to use.
+		 */
+		clk_prepare_enable(drv->clk);
+		schedule_delayed_work(&channel->work, 100);
+	}
+
 	return 0;
 }
 
@@ -117,9 +153,9 @@ static int rcar_gen2_phy_power_on(struct phy *p)
 	u32 value;
 	int err = 0, i;
 
-	/* Skip if it's not USBHS */
-	if (phy->select_value != USBHS_UGCTRL2_USB0SEL_HS_USB)
-		return 0;
+	/* Optional external power pin */
+	if (gpio_is_valid(phy->channel->gpio_pwr))
+		gpio_direction_output(phy->channel->gpio_pwr, 1);
 
 	spin_lock_irqsave(&drv->lock, flags);
 
@@ -160,9 +196,9 @@ static int rcar_gen2_phy_power_off(struct phy *p)
 	unsigned long flags;
 	u32 value;
 
-	/* Skip if it's not USBHS */
-	if (phy->select_value != USBHS_UGCTRL2_USB0SEL_HS_USB)
-		return 0;
+	/* External power pin */
+	if (gpio_is_valid(phy->channel->gpio_pwr))
+		gpio_direction_output(phy->channel->gpio_pwr, 0);
 
 	spin_lock_irqsave(&drv->lock, flags);
 
@@ -236,6 +272,132 @@ static const u32 select_value[][PHYS_PER_CHANNEL] = {
 	[2]	= { USBHS_UGCTRL2_USB2SEL_PCI, USBHS_UGCTRL2_USB2SEL_USB30 },
 };
 
+
+#define VBUS_IRQ_FLAGS \
+	(IRQF_SHARED | IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING)
+
+static void gpio_vbus_work(struct work_struct *work)
+{
+	struct rcar_gen2_channel *channel = container_of(work,
+					struct rcar_gen2_channel, work.work);
+	struct usb_phy *usbphy = channel->usbphy;
+	int status, vbus, id;
+
+	vbus = !!gpio_get_value(channel->gpio_vbus);
+	id = !!gpio_get_value(channel->gpio_id);
+
+	/* Switch the PHY over */
+	if (id)
+		rcar_gen2_phy_switch(channel, USBHS_UGCTRL2_USB0SEL_HS_USB);
+	else
+		rcar_gen2_phy_switch(channel, USBHS_UGCTRL2_USB0SEL_PCI);
+
+	/* If VBUS is powered and we are not the initial Host, turn off VBUS */
+	if (gpio_is_valid(channel->gpio_vbus_pwr))
+		gpio_direction_output(channel->gpio_vbus_pwr, !(id && vbus));
+
+	if (!channel->otg->gadget)
+		return;
+
+	/* Function handling: vbus=1 when initially plugged into a Host */
+	if (vbus) {
+		status = USB_EVENT_VBUS;
+		usbphy->otg->state = OTG_STATE_B_PERIPHERAL;
+		usbphy->last_event = status;
+		usb_gadget_vbus_connect(usbphy->otg->gadget);
+
+		atomic_notifier_call_chain(&usbphy->notifier,
+					   status, usbphy->otg->gadget);
+	} else {
+		usb_gadget_vbus_disconnect(usbphy->otg->gadget);
+		status = USB_EVENT_NONE;
+		usbphy->otg->state = OTG_STATE_B_IDLE;
+		usbphy->last_event = status;
+
+		atomic_notifier_call_chain(&usbphy->notifier,
+					   status, usbphy->otg->gadget);
+	}
+}
+
+/* VBUS change IRQ handler */
+static irqreturn_t gpio_vbus_irq(int irq, void *data)
+{
+	struct rcar_gen2_channel *channel = data;
+
+	/* Wait 20ms before doing anything as VBUS needs to settle */
+	schedule_delayed_work(&channel->work, msecs_to_jiffies(20));
+
+	return IRQ_HANDLED;
+}
+
+static int probe_otg(struct platform_device *pdev,
+	struct rcar_gen2_phy_driver *drv)
+{
+	struct device *dev = &pdev->dev;
+	struct rcar_gen2_channel *ch = drv->channels;
+	int irq;
+	int ret;
+
+	/* GPIOs for Host/Fn switching */
+	ch->gpio_id = of_get_named_gpio_flags(dev->of_node,
+				"renesas,id-gpio", 0, NULL);
+	ch->gpio_vbus = of_get_named_gpio_flags(dev->of_node,
+				"renesas,vbus-gpio", 0, NULL);
+
+	/* Need valid ID and VBUS gpios for Host/Fn switching */
+	if (gpio_is_valid(ch->gpio_id) && gpio_is_valid(ch->gpio_vbus)) {
+		ch->use_otg = 1;
+
+		/* GPIO for ID input */
+		ret = devm_gpio_request_one(dev, ch->gpio_id, GPIOF_IN, "id");
+		if (ret)
+			return ret;
+
+		/* GPIO for VBUS input */
+		ret = devm_gpio_request_one(dev, ch->gpio_vbus, GPIOF_IN, "vbus");
+		if (ret)
+			return ret;
+
+		irq = gpio_to_irq(ch->gpio_vbus);
+		ret = devm_request_irq(dev, irq, gpio_vbus_irq, VBUS_IRQ_FLAGS,
+				"vbus_detect", ch);
+		if (ret)
+			return ret;
+
+		/* Optional GPIO for VBUS power */
+		ch->gpio_vbus_pwr = of_get_named_gpio_flags(dev->of_node,
+					"renesas,vbus-pwr-gpio", 0, NULL);
+		if (gpio_is_valid(ch->gpio_id)) {
+			ret = devm_gpio_request_one(dev, ch->gpio_vbus_pwr,
+					GPIOF_OUT_INIT_LOW, "vbus-pwr");
+			if (ret)
+				return ret;
+		}
+
+	} else if (gpio_is_valid(ch->gpio_id)) {
+		dev_err(dev, "Failed to get VBUS gpio\n");
+		return ch->gpio_vbus;
+	} else if (gpio_is_valid(ch->gpio_vbus)) {
+		dev_err(dev, "Failed to get ID gpio\n");
+		return ch->gpio_id;
+	}
+
+	return 0;
+}
+
+/* bind/unbind the peripheral controller */
+static int rcar_gen2_usb_set_peripheral(struct usb_otg *otg,
+					struct usb_gadget *gadget)
+{
+	if (!gadget) {
+		usb_gadget_vbus_disconnect(otg->gadget);
+		otg->state = OTG_STATE_UNDEFINED;
+	}
+	otg->gadget = gadget;
+
+	return 0;
+}
+
 static int rcar_gen2_phy_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -245,7 +407,9 @@ static int rcar_gen2_phy_probe(struct platform_device *pdev)
 	struct resource *res;
 	void __iomem *base;
 	struct clk *clk;
+	struct usb_otg *otg;
 	int i = 0;
+	int err;
 
 	if (!dev->of_node) {
 		dev_err(dev,
@@ -280,6 +444,57 @@ static int rcar_gen2_phy_probe(struct platform_device *pdev)
 	if (!drv->channels)
 		return -ENOMEM;
 
+	/* USB0 optional GPIO power pin for external devices */
+	drv->channels->gpio_pwr = of_get_named_gpio_flags(dev->of_node,
+						"renesas,pwr-gpio", 0, NULL);
+	if (drv->channels->gpio_pwr == -EPROBE_DEFER)
+		return -EPROBE_DEFER;
+
+	if (gpio_is_valid(drv->channels->gpio_pwr)) {
+		err = devm_gpio_request(dev, drv->channels->gpio_pwr, "pwr");
+		if (err)
+			return err;
+	}
+
+	/* USB0 Host/Function switching info */
+	err = probe_otg(pdev, drv);
+	if (err)
+		return err;
+
+	/*
+	 * The PHY connected to channel 0 can be used to steer signals to the
+	 * USB Host IP that stils behind a PCI bridge (pci0), or the USB Func
+	 * IP (hsusb). We can dynamically switch this based on VBUS and ID
+	 * signals connected to gpios, to get something approaching OTG.
+	 */
+	if (drv->channels->use_otg) {
+		struct usb_phy *usbphy;
+
+		usbphy = devm_kzalloc(dev, sizeof(*usbphy), GFP_KERNEL);
+		if (!usbphy)
+			return -ENOMEM;
+
+		otg = devm_kzalloc(dev, sizeof(*otg), GFP_KERNEL);
+		if (!otg)
+			return -ENOMEM;
+
+		usbphy->dev		= dev;
+		usbphy->otg		= otg;
+
+		otg->usb_phy		= usbphy;
+		otg->state		= OTG_STATE_UNDEFINED;
+		otg->set_peripheral	= rcar_gen2_usb_set_peripheral;
+
+		drv->channels->otg	= otg;
+		drv->channels->usbphy	= usbphy;
+
+		ATOMIC_INIT_NOTIFIER_HEAD(&usbphy->notifier);
+
+		INIT_DELAYED_WORK(&drv->channels->work, gpio_vbus_work);
+
+		usb_add_phy_dev(usbphy);
+	}
+
 	for_each_child_of_node(dev->of_node, np) {
 		struct rcar_gen2_channel *channel = drv->channels + i;
 		u32 channel_num;
@@ -288,6 +503,8 @@ static int rcar_gen2_phy_probe(struct platform_device *pdev)
 		channel->of_node = np;
 		channel->drv = drv;
 		channel->selected_phy = -1;
+		if (i != 0)
+			channel->gpio_pwr = -ENOENT;
 
 		error = of_property_read_u32(np, "reg", &channel_num);
 		if (error || channel_num > 2) {
@@ -323,6 +540,14 @@ static int rcar_gen2_phy_probe(struct platform_device *pdev)
 
 	dev_set_drvdata(dev, drv);
 
+	/*
+	 * If we already have something plugged into USB0, we won't get an edge
+	 * on VBUS, so we have to manually schedule work to look at the VBUS
+	 * and ID signals.
+	 */
+	if (drv->channels->use_otg)
+		schedule_delayed_work(&drv->channels->work, msecs_to_jiffies(100));
+
 	return 0;
 }
 
-- 
1.9.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

Re: [PATCH v3] phy: rcar-gen2 usb: Add Host/Function switching for USB0

[Sergei Shtylyov]

Hello.

On 07/07/2015 02:55 PM, Phil Edworthy wrote:

> Instead of statically selecting the PHY connection to either the
> USBHS (Function) or PCI0 (Host) IP blocks, this change allows the
> dts to specifiy gpio pins for the vbus and id signals. Additional

    These GPIOs don't have anything to do with the PHY, they're interfacing 
Maxim MAX3355 OTG chip for which I have submitted the extcon driver.

> gpio pins are used to control power to an external OTG device and
> an override to turn vbus on/off.

> Note: the R-Car USB PHY only allows this Host/Function switching
> on channel 0.

> This has been tested on a r8a7791 based Koelsch board, which uses
> a MAX3355 device to supply vbus power when needed.

> Signed-off-by: Phil Edworthy <phil.edworthy-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>

    I'm inclined to NAK this. You're modifying the wrong driver, I think.

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 v3] phy: rcar-gen2 usb: Add Host/Function switching for USB0

[Phil Edworthy]

Hi Sergei,

On 10 July 2015 17:36, Sergei wrote:
> Hello.
> 
> On 07/07/2015 02:55 PM, Phil Edworthy wrote:
> 
> > Instead of statically selecting the PHY connection to either the
> > USBHS (Function) or PCI0 (Host) IP blocks, this change allows the
> > dts to specifiy gpio pins for the vbus and id signals. Additional
> 
>     These GPIOs don't have anything to do with the PHY, they're interfacing
> Maxim MAX3355 OTG chip for which I have submitted the extcon driver.
Hmm, I see what you mean... could you post your latest version of that
driver as it seems to have stalled 6 months ago?
 

> > gpio pins are used to control power to an external OTG device and
> > an override to turn vbus on/off.
> 
> > Note: the R-Car USB PHY only allows this Host/Function switching
> > on channel 0.
> 
> > This has been tested on a r8a7791 based Koelsch board, which uses
> > a MAX3355 device to supply vbus power when needed.
> 
> > Signed-off-by: Phil Edworthy <phil.edworthy-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
> 
>     I'm inclined to NAK this. You're modifying the wrong driver, I think.
> 
> WBR, Sergei

Thanks
Phil
--
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 v3] phy: rcar-gen2 usb: Add Host/Function switching for USB0

[Sergei Shtylyov]

Hello.

On 7/13/2015 12:04 PM, Phil Edworthy wrote:

>>> Instead of statically selecting the PHY connection to either the
>>> USBHS (Function) or PCI0 (Host) IP blocks, this change allows the
>>> dts to specifiy gpio pins for the vbus and id signals. Additional

>>      These GPIOs don't have anything to do with the PHY, they're interfacing

    Perhaps that was too strong statement but nevertheless...

>> Maxim MAX3355 OTG chip for which I have submitted the extcon driver.

> Hmm, I see what you mean... could you post your latest version of that
> driver as it seems to have stalled 6 months ago?

    I haven't worked on that driver since then.

> Thanks
> Phil

WBR, Sergei

--
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 v3] phy: rcar-gen2 usb: Add Host/Function switching for USB0

[Phil Edworthy]

Hi Sergei,

On 13 July 2015 11:17, Sergei wrote:
> Hello.
> 
> On 7/13/2015 12:04 PM, Phil Edworthy wrote:
> 
> >>> Instead of statically selecting the PHY connection to either the
> >>> USBHS (Function) or PCI0 (Host) IP blocks, this change allows the
> >>> dts to specifiy gpio pins for the vbus and id signals. Additional
> 
> >>      These GPIOs don't have anything to do with the PHY, they're interfacing
> 
>     Perhaps that was too strong statement but nevertheless...
Looking at your MAX3355 extcon driver, I can't see how it would work on it's
own. The system needs to sense vbus in order to determine that the board
has been plugged into a USB Host. Since the MAX3355 device doesn't
directly provide any vbus signals, this shouldn't be part of the extcon driver,
so where should it be?

On the other hand, the MAX3355 has a pair of status pins that can be used
to get vbus instead. If these pins aren't used for other functions, maybe
it's better to use these in the extcon driver.

My intention is to make the USB PHY driver listen for extcon events instead
of directly accessing the ID and VBUS signals, but otherwise behave in the
same way it currently does.

After reading some other threads, I also intend to set up a fixed regulator
for the MAX3355 device to setup the shutdown and vbus enable pins. I know
that the vbus enable should really be controlled some other way depending
on the role, but for the moment I think it's ok just to enable it always.

Do you think that is the correct way to progress this?

> >> Maxim MAX3355 OTG chip for which I have submitted the extcon driver.
> 
> > Hmm, I see what you mean... could you post your latest version of that
> > driver as it seems to have stalled 6 months ago?
> 
>     I haven't worked on that driver since then.
> 
> > Thanks
> > Phil
> 
> WBR, Sergei

Thanks
Phil

Re: [PATCH v3] phy: rcar-gen2 usb: Add Host/Function switching for USB0

[Sergei Shtylyov]

On 07/13/2015 06:02 PM, Phil Edworthy wrote:

>>>>> Instead of statically selecting the PHY connection to either the
>>>>> USBHS (Function) or PCI0 (Host) IP blocks, this change allows the
>>>>> dts to specifiy gpio pins for the vbus and id signals. Additional

>>>>       These GPIOs don't have anything to do with the PHY, they're interfacing

>>      Perhaps that was too strong statement but nevertheless...

> Looking at your MAX3355 extcon driver, I can't see how it would work on it's
> own.

    I've added the 'extcon' support into the Renesas USBHS driver, so that it 
should be able to sense the ID pin status.

> The system needs to sense vbus in order to determine that the board
> has been plugged into a USB Host.

> Since the MAX3355 device doesn't
> directly provide any vbus signals,  this shouldn't be part of the extcon driver,
> so where should it be?

    Sure, MAX3355 does provide voltage on Vbus, controlled by -OFFVBUS input.
It's just that the driver has only ID sensing support.
    Do you have the MAX3355 datasheet at all?

> On the other hand, the MAX3355 has a pair of status pins that can be used
> to get vbus instead.

    Sure, these bits reflect 4 OTG Vbus thresholds.

> If these pins aren't used for other functions, maybe
> it's better to use these in the extcon driver.

    They are not.

> My intention is to make the USB PHY driver listen for extcon events instead
> of directly accessing the ID and VBUS signals, but otherwise behave in the
> same way it currently does.

    I'm not sure the PHY driver should be interested in that...

> After reading some other threads, I also intend to set up a fixed regulator
> for the MAX3355 device to setup the shutdown and vbus enable pins.  I know
> that the vbus enable should really be controlled some other way depending
> on the role, but for the moment I think it's ok just to enable it always.

    It's OK only in the host mode.
    I don't think we need regulators. SHDN- is already supported by the driver
(though it only drives it high at start-up time), in fact it was the only reason
not to use gpio.

> Do you think that is the correct way to progress this?

    I didn't have a clear picture how to implement the OTG support at the time 
of writing the MAX3355 driver; actually, I was tasked to only support ID p[in 
sensing.
    Note that there's some ongoing effort now on linux-usb to support OTG 
functionality.

> Thanks
> Phil

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 v3] phy: rcar-gen2 usb: Add Host/Function switching for USB0

[Phil Edworthy]

Hi Sergei,

On 13 July 2015 17:37, Sergei wrote:
> On 07/13/2015 06:02 PM, Phil Edworthy wrote:
> 
> >>>>> Instead of statically selecting the PHY connection to either the
> >>>>> USBHS (Function) or PCI0 (Host) IP blocks, this change allows the
> >>>>> dts to specifiy gpio pins for the vbus and id signals. Additional
> 
> >>>>       These GPIOs don't have anything to do with the PHY, they're interfacing
> 
> >>      Perhaps that was too strong statement but nevertheless...
> 
> > Looking at your MAX3355 extcon driver, I can't see how it would work on it's
> > own.
> 
>     I've added the 'extcon' support into the Renesas USBHS driver, so that it
> should be able to sense the ID pin status.
> 
> > The system needs to sense vbus in order to determine that the board
> > has been plugged into a USB Host.
> 
> > Since the MAX3355 device doesn't
> > directly provide any vbus signals,  this shouldn't be part of the extcon driver,
> > so where should it be?
> 
>     Sure, MAX3355 does provide voltage on Vbus, controlled by -OFFVBUS input.
> It's just that the driver has only ID sensing support.
>     Do you have the MAX3355 datasheet at all?
Sorry, I meant that the max3355 device does not directly provide vbus signals
suitable for input as a gpio. Though as it provides the vbus signal to the usb
connector, there is no reason this can't be used as a gpio input to rcar (just
a level shifter needed ala the Koelsch board).

> > On the other hand, the MAX3355 has a pair of status pins that can be used
> > to get vbus instead.
> 
>     Sure, these bits reflect 4 OTG Vbus thresholds.
> 
> > If these pins aren't used for other functions, maybe
> > it's better to use these in the extcon driver.
> 
>     They are not.
My comments were based on the view that vbus output from the max3355
device wasn't a signal that could be input to the rcar via gpio. Since it is, I agree
that we should use that!

> > My intention is to make the USB PHY driver listen for extcon events instead
> > of directly accessing the ID and VBUS signals, but otherwise behave in the
> > same way it currently does.
> 
>     I'm not sure the PHY driver should be interested in that...
Then how is the PHY going to know when to switch between pci0 (host) and
hsusb (function)?

> > After reading some other threads, I also intend to set up a fixed regulator
> > for the MAX3355 device to setup the shutdown and vbus enable pins.  I know
> > that the vbus enable should really be controlled some other way depending
> > on the role, but for the moment I think it's ok just to enable it always.
> 
>     It's OK only in the host mode.
>     I don't think we need regulators. SHDN- is already supported by the driver
> (though it only drives it high at start-up time), in fact it was the only reason
> not to use gpio.
> 
> > Do you think that is the correct way to progress this?
> 
>     I didn't have a clear picture how to implement the OTG support at the time
> of writing the MAX3355 driver; actually, I was tasked to only support ID p[in
> sensing.
Right, so the max3355 driver needs some changes to include vbus handling.
Are you working on this, or should I take it on?

>     Note that there's some ongoing effort now on linux-usb to support OTG
> functionality.
Yes, I need to spend some time reading that list!

> > Thanks
> > Phil
> 
> WBR, Sergei

Thanks
Phil
--
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 v3] phy: rcar-gen2 usb: Add Host/Function switching for USB0

[Sergei Shtylyov]

On 07/13/2015 07:55 PM, Phil Edworthy wrote:

>>>>>>> Instead of statically selecting the PHY connection to either the
>>>>>>> USBHS (Function) or PCI0 (Host) IP blocks, this change allows the
>>>>>>> dts to specifiy gpio pins for the vbus and id signals. Additional

>>>>>>        These GPIOs don't have anything to do with the PHY, they're interfacing

>>>>       Perhaps that was too strong statement but nevertheless...

>>> Looking at your MAX3355 extcon driver, I can't see how it would work on it's
>>> own.

>>      I've added the 'extcon' support into the Renesas USBHS driver, so that it
>> should be able to sense the ID pin status.

>>> The system needs to sense vbus in order to determine that the board
>>> has been plugged into a USB Host.
>>
>>> Since the MAX3355 device doesn't
>>> directly provide any vbus signals,  this shouldn't be part of the extcon driver,
>>> so where should it be?
>>
>>      Sure, MAX3355 does provide voltage on Vbus, controlled by -OFFVBUS input.
>> It's just that the driver has only ID sensing support.
>>      Do you have the MAX3355 datasheet at all?
> Sorry, I meant that the max3355 device does not directly provide vbus signals
> suitable for input as a gpio.

    What about STATUS1/2? They are fed to GPIOs.

> Though as it provides the vbus signal to the usb
> connector, there is no reason this can't be used as a gpio input to rcar (just
> a level shifter needed ala the Koelsch board).

    Well, STATUS1/2 provide better info about Vbus, OTG-wise.

>>> On the other hand, the MAX3355 has a pair of status pins that can be used
>>> to get vbus instead.

>>      Sure, these bits reflect 4 OTG Vbus thresholds.

>>> If these pins aren't used for other functions, maybe
>>> it's better to use these in the extcon driver.

>>      They are not.

> My comments were based on the view that vbus output from the max3355
> device wasn't a signal that could be input to the rcar via gpio. Since it is, I agree
> that we should use that!

    I wasn't paying enough attention to the Koelsch scheme while replying, it 
seems. Yes, USBHS can see Vbus (almost) directly when SW6 is set to 2-3.

>>> My intention is to make the USB PHY driver listen for extcon events instead
>>> of directly accessing the ID and VBUS signals, but otherwise behave in the
>>> same way it currently does.

>>      I'm not sure the PHY driver should be interested in that...

> Then how is the PHY going to know when to switch between pci0 (host) and
> hsusb (function)?

    As I said, I planted the extcon hooks directly into USBHS (merged patch) 
and USB HCD code (unmerged, low quality hack).

>>> After reading some other threads, I also intend to set up a fixed regulator
>>> for the MAX3355 device to setup the shutdown and vbus enable pins.  I know
>>> that the vbus enable should really be controlled some other way depending
>>> on the role, but for the moment I think it's ok just to enable it always.

>>      It's OK only in the host mode.
>>      I don't think we need regulators. SHDN- is already supported by the driver
>> (though it only drives it high at start-up time), in fact it was the only reason
>> not to use gpio.

>>> Do you think that is the correct way to progress this?

>>      I didn't have a clear picture how to implement the OTG support at the time
>> of writing the MAX3355 driver; actually, I was tasked to only support ID p[in
>> sensing.

> Right, so the max3355 driver needs some changes to include vbus handling.
> Are you working on this, or should I take it on?

    No, not working yet, I have other things on my plate still...

>>      Note that there's some ongoing effort now on linux-usb to support OTG
>> functionality.

> Yes, I need to spend some time reading that list!

> Thanks
> Phil

WBR, Sergei

RE: [PATCH v3] phy: rcar-gen2 usb: Add Host/Function switching for USB0

[Phil Edworthy]

Hi Sergei,

On 13 July 2015 18:10, Sergei wrote:
> On 07/13/2015 07:55 PM, Phil Edworthy wrote:
> 
> >>>>>>> Instead of statically selecting the PHY connection to either the
> >>>>>>> USBHS (Function) or PCI0 (Host) IP blocks, this change allows the
> >>>>>>> dts to specifiy gpio pins for the vbus and id signals. Additional
> 
> >>>>>>        These GPIOs don't have anything to do with the PHY, they're
> interfacing
> 
> >>>>       Perhaps that was too strong statement but nevertheless...
> 
> >>> Looking at your MAX3355 extcon driver, I can't see how it would work on it's
> >>> own.
> 
> >>      I've added the 'extcon' support into the Renesas USBHS driver, so that it
> >> should be able to sense the ID pin status.
> 
> >>> The system needs to sense vbus in order to determine that the board
> >>> has been plugged into a USB Host.
<snip>
Ok, so in summary, the max3355 device has two ways to communicate vbus
back to a processor. You can either use the vbus signal or the status1/2 signals
connected as gpios. The status1/2 signals give HNP information for OTG and
are preferred, but depending on the board, they might not be available.

 
> >>> My intention is to make the USB PHY driver listen for extcon events instead
> >>> of directly accessing the ID and VBUS signals, but otherwise behave in the
> >>> same way it currently does.
> 
> >>      I'm not sure the PHY driver should be interested in that...
> 
> > Then how is the PHY going to know when to switch between pci0 (host) and
> > hsusb (function)?
> 
>     As I said, I planted the extcon hooks directly into USBHS (merged patch)
> and USB HCD code (unmerged, low quality hack).
Unless I am missing something, the problem I see with this is that it will not
dynamically switch between Host and Gadget, hence my suggestion that the
USB PHY driver be interested in the extcon events.


> >>> After reading some other threads, I also intend to set up a fixed regulator
> >>> for the MAX3355 device to setup the shutdown and vbus enable pins.  I
> know
> >>> that the vbus enable should really be controlled some other way depending
> >>> on the role, but for the moment I think it's ok just to enable it always.
> 
> >>      It's OK only in the host mode.
> >>      I don't think we need regulators. SHDN- is already supported by the driver
> >> (though it only drives it high at start-up time), in fact it was the only reason
> >> not to use gpio.
> 
> >>> Do you think that is the correct way to progress this?
> 
> >>      I didn't have a clear picture how to implement the OTG support at the time
> >> of writing the MAX3355 driver; actually, I was tasked to only support ID p[in
> >> sensing.
> 
> > Right, so the max3355 driver needs some changes to include vbus handling.
> > Are you working on this, or should I take it on?
> 
>     No, not working yet, I have other things on my plate still...
Ok, thanks for your comments! Hopefully I'll find the time to have a stab at this.


> >>      Note that there's some ongoing effort now on linux-usb to support OTG
> >> functionality.
> 
> > Yes, I need to spend some time reading that list!
> 
> > Thanks
> > Phil
> 
> WBR, Sergei

Thanks
Phil
--
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