Re: [PATCHv2 5/7] usb: phy: twl4030-usb: check if vbus is driven by twl itself

[Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>]

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

Hi,

On Thu, Mar 21, 2013 at 03:42:46PM +0200, Grazvydas Ignotas wrote:
> >> At least on pandora, STS_VBUS gets set even when VBUS is driven by twl
> >> itself. Reporting VBUS in this case confuses OMAP musb glue and charger
> >> driver, so check if OTG VBUS charge pump is on before reporting VBUS
> >> event to avoid this problem.
> >>
> >> Signed-off-by: Grazvydas Ignotas <notasas-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >> ---
> >>  drivers/usb/phy/phy-twl4030-usb.c |   36 +++++++++++++++++++++++++++++++-----
> >>  1 file changed, 31 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/usb/phy/phy-twl4030-usb.c b/drivers/usb/phy/phy-twl4030-usb.c
> >> index 425c18a..87bf11d 100644
> >> --- a/drivers/usb/phy/phy-twl4030-usb.c
> >> +++ b/drivers/usb/phy/phy-twl4030-usb.c
> >> @@ -248,11 +248,31 @@ twl4030_usb_clear_bits(struct twl4030_usb *twl, u8 reg, u8 bits)
> >>
> >>  /*-------------------------------------------------------------------------*/
> >>
> >> +static bool twl4030_is_driving_vbus(struct twl4030_usb *twl)
> >> +{
> >> +     int ret;
> >> +
> >> +     ret = twl4030_usb_read(twl, PHY_CLK_CTRL_STS);
> >> +     if (ret < 0 || !(ret & PHY_DPLL_CLK))
> >> +             /*
> >> +              * if clocks are off, registers are not updated,
> >> +              * but we can assume we don't drive VBUS in this case
> >> +              */
> >> +             return false;
> >> +
> >> +     ret = twl4030_usb_read(twl, ULPI_OTG_CTRL);
> >> +     if (ret < 0)
> >> +             return false;
> >> +
> >> +     return (ret & (ULPI_OTG_DRVVBUS | ULPI_OTG_CHRGVBUS)) ? true : false;
> >> +}
> >> +
> >>  static enum omap_musb_vbus_id_status
> >>       twl4030_usb_linkstat(struct twl4030_usb *twl)
> >>  {
> >>       int     status;
> >>       enum omap_musb_vbus_id_status linkstat = OMAP_MUSB_UNKNOWN;
> >> +     bool    driving_vbus = false;
> >>
> >>       twl->vbus_supplied = false;
> >>
> >> @@ -270,20 +290,26 @@ static enum omap_musb_vbus_id_status
> >>       if (status < 0)
> >>               dev_err(twl->dev, "USB link status err %d\n", status);
> >>       else if (status & (BIT(7) | BIT(2))) {
> >> -             if (status & (BIT(7)))
> >> -                        twl->vbus_supplied = true;
> >> +             if (status & BIT(7)) {
> >> +                     driving_vbus = twl4030_is_driving_vbus(twl);
> >> +                     if (driving_vbus)
> >
> > how about just:
> >
> > if (twl4030_is_driving_vbus(twl))
> >         status &= ~BIT(7);
> >
> > ????
> 
> I'm logging driving_vbus below with dev_dbg(), so that it's easier to
> see what going on..

is that really necessary ? Don't we expose it through sysfs already ?

-- 
balbi

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