RE: [PATCH 2/3] phy: rcar-gen2 usb: Add Host/Function switching for USB0
Hi Kishon,
On 02 July 2015 09:22, Kishon wrote:
> Hi,
>
> On Monday 22 June 2015 08:12 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
> > 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
> > ---
> > drivers/phy/phy-rcar-gen2.c | 269
>
> > 1 file changed, 247 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/phy/phy-rcar-gen2.c b/drivers/phy/phy-rcar-gen2.c
> > index 97d45f4..8564e7d 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
> > #include
> > #include
> > +#include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > +#include
> > +#include
> > +#include
> >
> > #include
> >
> > @@ -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;
>
> Using usb_phy is a step backwards IMO. We should rather try to get the missing
> functionality adding in Generic PHY.
Ok, that will take quite a bit more work. Do you know if anyone is working on
this?
> Thanks
> Kishon
>
> > + 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;
> >
Re: [PATCH 2/3] phy: rcar-gen2 usb: Add Host/Function switching for USB0
Hi,
On Monday 22 June 2015 08:12 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
> 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
> ---
> drivers/phy/phy-rcar-gen2.c | 269
>
> 1 file changed, 247 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/phy/phy-rcar-gen2.c b/drivers/phy/phy-rcar-gen2.c
> index 97d45f4..8564e7d 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
> #include
> #include
> +#include
> #include
> #include
> +#include
> #include
> #include
> #include
> +#include
> +#include
> +#include
>
> #include
>
> @@ -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;
Using usb_phy is a step backwards IMO. We should rather try to get the missing
functionality adding in Generic PHY.
Thanks
Kishon
> + 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_o
Re: [PATCH 2/3] phy: rcar-gen2 usb: Add Host/Function switching for USB0
+Laurent
On Monday 22 June 2015 08:12 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
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.
Looks like you are touching the part which Laurent had a problem with, so
looping him.
Thanks
Kishon
Signed-off-by: Phil Edworthy
---
drivers/phy/phy-rcar-gen2.c | 269
1 file changed, 247 insertions(+), 22 deletions(-)
diff --git a/drivers/phy/phy-rcar-gen2.c b/drivers/phy/phy-rcar-gen2.c
index 97d45f4..8564e7d 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
#include
#include
+#include
#include
#include
+#include
#include
#include
#include
+#include
+#include
+#include
#include
@@ -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)
