diff mbox

[v6,23/25] usb: chipidea: Pullup D+ in device mode via phy APIs

Message ID 20161228225711.698-24-stephen.boyd@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Stephen Boyd Dec. 28, 2016, 10:57 p.m. UTC
If the phy supports it, call phy_set_mode() to pull up D+ when
required by setting the mode to PHY_MODE_USB_DEVICE. If we want
to remove the pullup, set the mode to PHY_MODE_USB_HOST.

Cc: Peter Chen <peter.chen@nxp.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
---
 drivers/usb/chipidea/udc.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Comments

Peter Chen Jan. 3, 2017, 6:53 a.m. UTC | #1
On Wed, Dec 28, 2016 at 02:57:09PM -0800, Stephen Boyd wrote:
> If the phy supports it, call phy_set_mode() to pull up D+ when
> required by setting the mode to PHY_MODE_USB_DEVICE. If we want
> to remove the pullup, set the mode to PHY_MODE_USB_HOST.
> 
> Cc: Peter Chen <peter.chen@nxp.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> ---
>  drivers/usb/chipidea/udc.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index 0d532a724d48..6d61fa0689b0 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -18,6 +18,7 @@
>  #include <linux/kernel.h>
>  #include <linux/slab.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/phy/phy.h>
>  #include <linux/usb/ch9.h>
>  #include <linux/usb/gadget.h>
>  #include <linux/usb/otg-fsm.h>
> @@ -1609,10 +1610,15 @@ static int ci_udc_pullup(struct usb_gadget *_gadget, int is_on)
>  		return 0;
>  
>  	pm_runtime_get_sync(&ci->gadget.dev);
> -	if (is_on)
> +	if (is_on) {
> +		if (ci->phy)
> +			phy_set_mode(ci->phy, PHY_MODE_USB_DEVICE);
>  		hw_write(ci, OP_USBCMD, USBCMD_RS, USBCMD_RS);
> -	else
> +	} else {
>  		hw_write(ci, OP_USBCMD, USBCMD_RS, 0);
> +		if (ci->phy)
> +			phy_set_mode(ci->phy, PHY_MODE_USB_HOST);
> +	}
>  	pm_runtime_put_sync(&ci->gadget.dev);
>  
>  	return 0;
> -- 

Would you describe the use case for it? Why not adding it at
role switch routine?
Stephen Boyd Jan. 12, 2017, 12:19 a.m. UTC | #2
Quoting Peter Chen (2017-01-02 22:53:19)
> On Wed, Dec 28, 2016 at 02:57:09PM -0800, Stephen Boyd wrote:
> > If the phy supports it, call phy_set_mode() to pull up D+ when
> > required by setting the mode to PHY_MODE_USB_DEVICE. If we want
> > to remove the pullup, set the mode to PHY_MODE_USB_HOST.
> > 
[..]
> > diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> > index 0d532a724d48..6d61fa0689b0 100644
> > --- a/drivers/usb/chipidea/udc.c
> > +++ b/drivers/usb/chipidea/udc.c
> > @@ -1609,10 +1610,15 @@ static int ci_udc_pullup(struct usb_gadget *_gadget, int is_on)
> >               return 0;
> >  
> >       pm_runtime_get_sync(&ci->gadget.dev);
> > -     if (is_on)
> > +     if (is_on) {
> > +             if (ci->phy)
> > +                     phy_set_mode(ci->phy, PHY_MODE_USB_DEVICE);
> >               hw_write(ci, OP_USBCMD, USBCMD_RS, USBCMD_RS);
> > -     else
> > +     } else {
> >               hw_write(ci, OP_USBCMD, USBCMD_RS, 0);
> > +             if (ci->phy)
> > +                     phy_set_mode(ci->phy, PHY_MODE_USB_HOST);
> > +     }
> >       pm_runtime_put_sync(&ci->gadget.dev);
> >  
> >       return 0;
> 
> Would you describe the use case for it? Why not adding it at
> role switch routine?
> 

This is about pulling up D+. The phy I have requires that we manually
pull up D+ by writing a ULPI register before we set the run/stop bit. I
thought it would be appropriate to do so in ci_udc_pullup(), where we're
supposed to put that pullup code, unless I'm mistaken?

It's not exactly about putting the phy into device or host mode, so
phy_set_mode() may not actually be the best API to use. Perhaps we need
some sort of phy_pullup_usb() API here?
Peter Chen Jan. 12, 2017, 9:50 a.m. UTC | #3
On Wed, Jan 11, 2017 at 04:19:53PM -0800, Stephen Boyd wrote:
> Quoting Peter Chen (2017-01-02 22:53:19)
> > On Wed, Dec 28, 2016 at 02:57:09PM -0800, Stephen Boyd wrote:
> > > If the phy supports it, call phy_set_mode() to pull up D+ when
> > > required by setting the mode to PHY_MODE_USB_DEVICE. If we want
> > > to remove the pullup, set the mode to PHY_MODE_USB_HOST.
> > > 
> [..]
> > > diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> > > index 0d532a724d48..6d61fa0689b0 100644
> > > --- a/drivers/usb/chipidea/udc.c
> > > +++ b/drivers/usb/chipidea/udc.c
> > > @@ -1609,10 +1610,15 @@ static int ci_udc_pullup(struct usb_gadget *_gadget, int is_on)
> > >               return 0;
> > >  
> > >       pm_runtime_get_sync(&ci->gadget.dev);
> > > -     if (is_on)
> > > +     if (is_on) {
> > > +             if (ci->phy)
> > > +                     phy_set_mode(ci->phy, PHY_MODE_USB_DEVICE);
> > >               hw_write(ci, OP_USBCMD, USBCMD_RS, USBCMD_RS);
> > > -     else
> > > +     } else {
> > >               hw_write(ci, OP_USBCMD, USBCMD_RS, 0);
> > > +             if (ci->phy)
> > > +                     phy_set_mode(ci->phy, PHY_MODE_USB_HOST);
> > > +     }
> > >       pm_runtime_put_sync(&ci->gadget.dev);
> > >  
> > >       return 0;
> > 
> > Would you describe the use case for it? Why not adding it at
> > role switch routine?
> > 
> 
> This is about pulling up D+. The phy I have requires that we manually
> pull up D+ by writing a ULPI register before we set the run/stop bit.

Afaik, only controller can pull up dp when it is at device mode by
setting USBCMD_RS. At host mode, clear USBCMD_RS will only stopping
sending SoF from controller side.

I am puzzled why you can pull up D+ by writing an ULPI register, perhaps,
your phy needs DP to change before switching the mode? Would you
double confirm that?

> I
> thought it would be appropriate to do so in ci_udc_pullup(), where we're
> supposed to put that pullup code, unless I'm mistaken?
> 
> It's not exactly about putting the phy into device or host mode, so
> phy_set_mode() may not actually be the best API to use. Perhaps we need
> some sort of phy_pullup_usb() API here?
Stephen Boyd Jan. 12, 2017, 10:49 p.m. UTC | #4
Quoting Peter Chen (2017-01-12 01:50:40)
> On Wed, Jan 11, 2017 at 04:19:53PM -0800, Stephen Boyd wrote:
> > Quoting Peter Chen (2017-01-02 22:53:19)
> > > On Wed, Dec 28, 2016 at 02:57:09PM -0800, Stephen Boyd wrote:
> > > > If the phy supports it, call phy_set_mode() to pull up D+ when
> > > > required by setting the mode to PHY_MODE_USB_DEVICE. If we want
> > > > to remove the pullup, set the mode to PHY_MODE_USB_HOST.
> > > > 
> > [..]
> > > > diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> > > > index 0d532a724d48..6d61fa0689b0 100644
> > > > --- a/drivers/usb/chipidea/udc.c
> > > > +++ b/drivers/usb/chipidea/udc.c
> > > > @@ -1609,10 +1610,15 @@ static int ci_udc_pullup(struct usb_gadget *_gadget, int is_on)
> > > >               return 0;
> > > >  
> > > >       pm_runtime_get_sync(&ci->gadget.dev);
> > > > -     if (is_on)
> > > > +     if (is_on) {
> > > > +             if (ci->phy)
> > > > +                     phy_set_mode(ci->phy, PHY_MODE_USB_DEVICE);
> > > >               hw_write(ci, OP_USBCMD, USBCMD_RS, USBCMD_RS);
> > > > -     else
> > > > +     } else {
> > > >               hw_write(ci, OP_USBCMD, USBCMD_RS, 0);
> > > > +             if (ci->phy)
> > > > +                     phy_set_mode(ci->phy, PHY_MODE_USB_HOST);
> > > > +     }
> > > >       pm_runtime_put_sync(&ci->gadget.dev);
> > > >  
> > > >       return 0;
> > > 
> > > Would you describe the use case for it? Why not adding it at
> > > role switch routine?
> > > 
> > 
> > This is about pulling up D+. The phy I have requires that we manually
> > pull up D+ by writing a ULPI register before we set the run/stop bit.
> 
> Afaik, only controller can pull up dp when it is at device mode by
> setting USBCMD_RS. At host mode, clear USBCMD_RS will only stopping
> sending SoF from controller side.
> 
> I am puzzled why you can pull up D+ by writing an ULPI register, perhaps,
> your phy needs DP to change before switching the mode? Would you
> double confirm that?

With the boards I have, vbus is not routed to the phy. Instead, there's
a vbus comparator on the PMIC where the vbus line from the usb
receptacle is sent. The vbus extcon driver probes the comparator on the
PMIC to see if vbus is present or not and then notifies extcon users
when vbus changes.

The ULPI register we write in the phy is a vendor specific register
(called MISC_A) that has two bits. If you look at
qcom_usb_hs_phy_set_mode() in this series you'll see that we set
VBUSVLDEXTSEL and VBUSVLDEXT. VBUSVLDEXTSEL controls a mux in the phy
that chooses between an internal comparator, in the case where vbus goes
to the phy, or an external signal input to the phy, VBUSVLDEXT, to
consider as the "session valid" signal. It looks like the session valid
signal drives the D+ pullup resistor in the phy. These bits in MISC_A
don't matter when the phy is in host mode.

So when the board doesn't route vbus to the phy, we have to toggle the
VBUSVLDEXT bit to signal to the phy that the vbus is there or not. I
also see that we're not supposed to toggle the VBUSVLDEXTSEL bit when in
"normal" operating mode. So perhaps we should do everything in the
qcom_usb_hs_phy_set_mode() routine during the role switch as you
suggest, except toggle the VBUSVLDEXT bit. Toggling the VBUSVLDEXT bit
can be done via some new phy op when the extcon triggers?
Peter Chen Jan. 13, 2017, 3:35 a.m. UTC | #5
On Thu, Jan 12, 2017 at 02:49:51PM -0800, Stephen Boyd wrote:
> Quoting Peter Chen (2017-01-12 01:50:40)
> > On Wed, Jan 11, 2017 at 04:19:53PM -0800, Stephen Boyd wrote:
> > > Quoting Peter Chen (2017-01-02 22:53:19)
> > > > On Wed, Dec 28, 2016 at 02:57:09PM -0800, Stephen Boyd wrote:
> > > > > If the phy supports it, call phy_set_mode() to pull up D+ when
> > > > > required by setting the mode to PHY_MODE_USB_DEVICE. If we want
> > > > > to remove the pullup, set the mode to PHY_MODE_USB_HOST.
> > > > > 
> > > [..]
> > > > > diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> > > > > index 0d532a724d48..6d61fa0689b0 100644
> > > > > --- a/drivers/usb/chipidea/udc.c
> > > > > +++ b/drivers/usb/chipidea/udc.c
> > > > > @@ -1609,10 +1610,15 @@ static int ci_udc_pullup(struct usb_gadget *_gadget, int is_on)
> > > > >               return 0;
> > > > >  
> > > > >       pm_runtime_get_sync(&ci->gadget.dev);
> > > > > -     if (is_on)
> > > > > +     if (is_on) {
> > > > > +             if (ci->phy)
> > > > > +                     phy_set_mode(ci->phy, PHY_MODE_USB_DEVICE);
> > > > >               hw_write(ci, OP_USBCMD, USBCMD_RS, USBCMD_RS);
> > > > > -     else
> > > > > +     } else {
> > > > >               hw_write(ci, OP_USBCMD, USBCMD_RS, 0);
> > > > > +             if (ci->phy)
> > > > > +                     phy_set_mode(ci->phy, PHY_MODE_USB_HOST);
> > > > > +     }
> > > > >       pm_runtime_put_sync(&ci->gadget.dev);
> > > > >  
> > > > >       return 0;
> > > > 
> > > > Would you describe the use case for it? Why not adding it at
> > > > role switch routine?
> > > > 
> > > 
> > > This is about pulling up D+. The phy I have requires that we manually
> > > pull up D+ by writing a ULPI register before we set the run/stop bit.
> > 
> > Afaik, only controller can pull up dp when it is at device mode by
> > setting USBCMD_RS. At host mode, clear USBCMD_RS will only stopping
> > sending SoF from controller side.
> > 
> > I am puzzled why you can pull up D+ by writing an ULPI register, perhaps,
> > your phy needs DP to change before switching the mode? Would you
> > double confirm that?
> 
> With the boards I have, vbus is not routed to the phy. Instead, there's
> a vbus comparator on the PMIC where the vbus line from the usb
> receptacle is sent. The vbus extcon driver probes the comparator on the
> PMIC to see if vbus is present or not and then notifies extcon users
> when vbus changes.
> 
> The ULPI register we write in the phy is a vendor specific register
> (called MISC_A) that has two bits. If you look at
> qcom_usb_hs_phy_set_mode() in this series you'll see that we set
> VBUSVLDEXTSEL and VBUSVLDEXT. VBUSVLDEXTSEL controls a mux in the phy
> that chooses between an internal comparator, in the case where vbus goes
> to the phy, or an external signal input to the phy, VBUSVLDEXT, to
> consider as the "session valid" signal. It looks like the session valid
> signal drives the D+ pullup resistor in the phy. These bits in MISC_A
> don't matter when the phy is in host mode.
> 
> So when the board doesn't route vbus to the phy, we have to toggle the
> VBUSVLDEXT bit to signal to the phy that the vbus is there or not. I
> also see that we're not supposed to toggle the VBUSVLDEXTSEL bit when in
> "normal" operating mode. So perhaps we should do everything in the
> qcom_usb_hs_phy_set_mode() routine during the role switch as you
> suggest, except toggle the VBUSVLDEXT bit. Toggling the VBUSVLDEXT bit
> can be done via some new phy op when the extcon triggers?

Why not call phy_set_mode(phy, DEVICE) directly at ci_handle_vbus_change when
you get extcon vbus event?
Stephen Boyd Jan. 13, 2017, 8:03 p.m. UTC | #6
Quoting Peter Chen (2017-01-12 19:35:36)
> On Thu, Jan 12, 2017 at 02:49:51PM -0800, Stephen Boyd wrote:
> > 
> > With the boards I have, vbus is not routed to the phy. Instead, there's
> > a vbus comparator on the PMIC where the vbus line from the usb
> > receptacle is sent. The vbus extcon driver probes the comparator on the
> > PMIC to see if vbus is present or not and then notifies extcon users
> > when vbus changes.
> > 
> > The ULPI register we write in the phy is a vendor specific register
> > (called MISC_A) that has two bits. If you look at
> > qcom_usb_hs_phy_set_mode() in this series you'll see that we set
> > VBUSVLDEXTSEL and VBUSVLDEXT. VBUSVLDEXTSEL controls a mux in the phy
> > that chooses between an internal comparator, in the case where vbus goes
> > to the phy, or an external signal input to the phy, VBUSVLDEXT, to
> > consider as the "session valid" signal. It looks like the session valid
> > signal drives the D+ pullup resistor in the phy. These bits in MISC_A
> > don't matter when the phy is in host mode.
> > 
> > So when the board doesn't route vbus to the phy, we have to toggle the
> > VBUSVLDEXT bit to signal to the phy that the vbus is there or not. I
> > also see that we're not supposed to toggle the VBUSVLDEXTSEL bit when in
> > "normal" operating mode. So perhaps we should do everything in the
> > qcom_usb_hs_phy_set_mode() routine during the role switch as you
> > suggest, except toggle the VBUSVLDEXT bit. Toggling the VBUSVLDEXT bit
> > can be done via some new phy op when the extcon triggers?
> 
> Why not call phy_set_mode(phy, DEVICE) directly at ci_handle_vbus_change when
> you get extcon vbus event?
> 

Right, I can call phy_set_mode(phy, DEVICE) there, but is that correct?
How do we signal vbus is gone, with phy_set_mode(phy, HOST)? Mode
doesn't seem the same as "vbus status changed" so this feels wrong.
Peter Chen Jan. 16, 2017, 3:45 a.m. UTC | #7
On Fri, Jan 13, 2017 at 12:03:00PM -0800, Stephen Boyd wrote:
> Quoting Peter Chen (2017-01-12 19:35:36)
> > On Thu, Jan 12, 2017 at 02:49:51PM -0800, Stephen Boyd wrote:
> > > 
> > > With the boards I have, vbus is not routed to the phy. Instead, there's
> > > a vbus comparator on the PMIC where the vbus line from the usb
> > > receptacle is sent. The vbus extcon driver probes the comparator on the
> > > PMIC to see if vbus is present or not and then notifies extcon users
> > > when vbus changes.
> > > 
> > > The ULPI register we write in the phy is a vendor specific register
> > > (called MISC_A) that has two bits. If you look at
> > > qcom_usb_hs_phy_set_mode() in this series you'll see that we set
> > > VBUSVLDEXTSEL and VBUSVLDEXT. VBUSVLDEXTSEL controls a mux in the phy
> > > that chooses between an internal comparator, in the case where vbus goes
> > > to the phy, or an external signal input to the phy, VBUSVLDEXT, to
> > > consider as the "session valid" signal. It looks like the session valid
> > > signal drives the D+ pullup resistor in the phy. These bits in MISC_A
> > > don't matter when the phy is in host mode.
> > > 
> > > So when the board doesn't route vbus to the phy, we have to toggle the
> > > VBUSVLDEXT bit to signal to the phy that the vbus is there or not. I
> > > also see that we're not supposed to toggle the VBUSVLDEXTSEL bit when in
> > > "normal" operating mode. So perhaps we should do everything in the
> > > qcom_usb_hs_phy_set_mode() routine during the role switch as you
> > > suggest, except toggle the VBUSVLDEXT bit. Toggling the VBUSVLDEXT bit
> > > can be done via some new phy op when the extcon triggers?
> > 
> > Why not call phy_set_mode(phy, DEVICE) directly at ci_handle_vbus_change when
> > you get extcon vbus event?
> > 
> 
> Right, I can call phy_set_mode(phy, DEVICE) there, but is that correct?
> How do we signal vbus is gone, with phy_set_mode(phy, HOST)? Mode
> doesn't seem the same as "vbus status changed" so this feels wrong.


> So when the board doesn't route vbus to the phy, we have to toggle the
> VBUSVLDEXT bit to signal to the phy that the vbus is there or not. I
> also see that we're not supposed to toggle the VBUSVLDEXTSEL bit when in
> "normal" operating mode. So perhaps we should do everything in the
> qcom_usb_hs_phy_set_mode() routine during the role switch as you
> suggest, except toggle the VBUSVLDEXT bit. Toggling the VBUSVLDEXT bit
> can be done via some new phy op when the extcon triggers?

So, you need to call phy_set_mode when switching between host and device.
Besides, you also need to toggle VBUSVLDEXT when the external vbus
is on or off at device mode (doesn't need for host mode), is it correct?

At include/linux/usb/phy.h, we have .set_vbus interface, maybe you need
to port it to generic phy framework.
Stephen Boyd Jan. 17, 2017, 5:58 p.m. UTC | #8
Quoting Peter Chen (2017-01-15 19:45:51)
> 
> So, you need to call phy_set_mode when switching between host and device.
> Besides, you also need to toggle VBUSVLDEXT when the external vbus
> is on or off at device mode (doesn't need for host mode), is it correct?

Correct.

> 
> At include/linux/usb/phy.h, we have .set_vbus interface, maybe you need
> to port it to generic phy framework.
> 

Ok. I'll look into that. Can the other patches in this series be picked
up? Otherwise I can resend them all again once I fix the phy_set_mode()
call location and introduce a new phy op.
Peter Chen Jan. 18, 2017, 7:34 a.m. UTC | #9
On Tue, Jan 17, 2017 at 09:58:33AM -0800, Stephen Boyd wrote:
> Quoting Peter Chen (2017-01-15 19:45:51)
> > 
> > So, you need to call phy_set_mode when switching between host and device.
> > Besides, you also need to toggle VBUSVLDEXT when the external vbus
> > is on or off at device mode (doesn't need for host mode), is it correct?
> 
> Correct.
> 
> > 
> > At include/linux/usb/phy.h, we have .set_vbus interface, maybe you need
> > to port it to generic phy framework.
> > 
> 
> Ok. I'll look into that. Can the other patches in this series be picked
> up? Otherwise I can resend them all again once I fix the phy_set_mode()
> call location and introduce a new phy op.

I can pick up chipidea patches after you test the patch I supplied at:

	[PATCH v6 11/25] usb: chipidea: vbus event may exist before starting
gadget

You may ping other maintainers to pick up other patches.
Stephen Boyd Jan. 18, 2017, 8:54 p.m. UTC | #10
Quoting Peter Chen (2017-01-17 23:34:32)
> On Tue, Jan 17, 2017 at 09:58:33AM -0800, Stephen Boyd wrote:
> > Quoting Peter Chen (2017-01-15 19:45:51)
> > > 
> > > At include/linux/usb/phy.h, we have .set_vbus interface, maybe you need
> > > to port it to generic phy framework.
> > > 
> > 
> > Ok. I'll look into that. Can the other patches in this series be picked
> > up? Otherwise I can resend them all again once I fix the phy_set_mode()
> > call location and introduce a new phy op.
> 
> I can pick up chipidea patches after you test the patch I supplied at:
> 
>         [PATCH v6 11/25] usb: chipidea: vbus event may exist before starting
> gadget

Ok. I've confirmed that this updated patch works fine for me. You can
have my Tested-by and Reviewed-by there.

> 
> You may ping other maintainers to pick up other patches.
> 

I was hoping you could pick the beginning of the series up until the PHY
drivers, which can go via Kishon's tree. That would mean applying the
drivers/of/ part. Rob is that ok?
Rob Herring Jan. 18, 2017, 8:57 p.m. UTC | #11
On Wed, Jan 18, 2017 at 2:54 PM, Stephen Boyd <stephen.boyd@linaro.org> wrote:
> Quoting Peter Chen (2017-01-17 23:34:32)
>> On Tue, Jan 17, 2017 at 09:58:33AM -0800, Stephen Boyd wrote:
>> > Quoting Peter Chen (2017-01-15 19:45:51)
>> > >
>> > > At include/linux/usb/phy.h, we have .set_vbus interface, maybe you need
>> > > to port it to generic phy framework.
>> > >
>> >
>> > Ok. I'll look into that. Can the other patches in this series be picked
>> > up? Otherwise I can resend them all again once I fix the phy_set_mode()
>> > call location and introduce a new phy op.
>>
>> I can pick up chipidea patches after you test the patch I supplied at:
>>
>>         [PATCH v6 11/25] usb: chipidea: vbus event may exist before starting
>> gadget
>
> Ok. I've confirmed that this updated patch works fine for me. You can
> have my Tested-by and Reviewed-by there.
>
>>
>> You may ping other maintainers to pick up other patches.
>>
>
> I was hoping you could pick the beginning of the series up until the PHY
> drivers, which can go via Kishon's tree. That would mean applying the
> drivers/of/ part. Rob is that ok?

Peter, If there's a dependency then please take the patches I've
acked. That's why I acked them.

Rob
Peter Chen Jan. 19, 2017, 6:29 a.m. UTC | #12
On Wed, Jan 18, 2017 at 02:57:27PM -0600, Rob Herring wrote:
> On Wed, Jan 18, 2017 at 2:54 PM, Stephen Boyd <stephen.boyd@linaro.org> wrote:
> > Quoting Peter Chen (2017-01-17 23:34:32)
> >> On Tue, Jan 17, 2017 at 09:58:33AM -0800, Stephen Boyd wrote:
> >> > Quoting Peter Chen (2017-01-15 19:45:51)
> >> > >
> >> > > At include/linux/usb/phy.h, we have .set_vbus interface, maybe you need
> >> > > to port it to generic phy framework.
> >> > >
> >> >
> >> > Ok. I'll look into that. Can the other patches in this series be picked
> >> > up? Otherwise I can resend them all again once I fix the phy_set_mode()
> >> > call location and introduce a new phy op.
> >>
> >> I can pick up chipidea patches after you test the patch I supplied at:
> >>
> >>         [PATCH v6 11/25] usb: chipidea: vbus event may exist before starting
> >> gadget
> >
> > Ok. I've confirmed that this updated patch works fine for me. You can
> > have my Tested-by and Reviewed-by there.
> >
> >>
> >> You may ping other maintainers to pick up other patches.
> >>
> >
> > I was hoping you could pick the beginning of the series up until the PHY
> > drivers, which can go via Kishon's tree. That would mean applying the
> > drivers/of/ part. Rob is that ok?
> 
> Peter, If there's a dependency then please take the patches I've
> acked. That's why I acked them.
> 

Ok, I will do it.
diff mbox

Patch

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 0d532a724d48..6d61fa0689b0 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -18,6 +18,7 @@ 
 #include <linux/kernel.h>
 #include <linux/slab.h>
 #include <linux/pm_runtime.h>
+#include <linux/phy/phy.h>
 #include <linux/usb/ch9.h>
 #include <linux/usb/gadget.h>
 #include <linux/usb/otg-fsm.h>
@@ -1609,10 +1610,15 @@  static int ci_udc_pullup(struct usb_gadget *_gadget, int is_on)
 		return 0;
 
 	pm_runtime_get_sync(&ci->gadget.dev);
-	if (is_on)
+	if (is_on) {
+		if (ci->phy)
+			phy_set_mode(ci->phy, PHY_MODE_USB_DEVICE);
 		hw_write(ci, OP_USBCMD, USBCMD_RS, USBCMD_RS);
-	else
+	} else {
 		hw_write(ci, OP_USBCMD, USBCMD_RS, 0);
+		if (ci->phy)
+			phy_set_mode(ci->phy, PHY_MODE_USB_HOST);
+	}
 	pm_runtime_put_sync(&ci->gadget.dev);
 
 	return 0;