| Message ID | 1460374506-9779-4-git-send-email-rogerq@ti.com |
|---|---|
| State | New |
| Headers | show |
On 11/04/16 15:18, Felipe Balbi wrote: > > Hi, > > Roger Quadros <rogerq@ti.com> writes: >> Don't make any decisions regarding VBUS session based on ID >> status. That is best left to the OTG core. > > what about builds who don't want OTG and/or dual-role ? > >> Pass ID and VBUS events independent of each other so that OTG >> core knows exactly what to do. >> >> This makes dual-role with extcon work with OTG irq on OMAP platforms. >> >> Signed-off-by: Roger Quadros <rogerq@ti.com> >> --- >> drivers/usb/dwc3/dwc3-omap.c | 15 ++++++--------- >> 1 file changed, 6 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c >> index 51ca098..c9b918d 100644 >> --- a/drivers/usb/dwc3/dwc3-omap.c >> +++ b/drivers/usb/dwc3/dwc3-omap.c >> @@ -233,19 +233,14 @@ static void dwc3_omap_set_mailbox(struct dwc3_omap *omap, >> } >> >> val = dwc3_omap_read_utmi_ctrl(omap); >> - val &= ~(USBOTGSS_UTMI_OTG_CTRL_IDDIG >> - | USBOTGSS_UTMI_OTG_CTRL_VBUSVALID >> - | USBOTGSS_UTMI_OTG_CTRL_SESSEND); >> - val |= USBOTGSS_UTMI_OTG_CTRL_SESSVALID >> - | USBOTGSS_UTMI_OTG_CTRL_POWERPRESENT; >> + val &= ~USBOTGSS_UTMI_OTG_CTRL_IDDIG; > > this creates the possibility of having a USB peripheral with VBUS_VALID, > right Sorry, I didn't get what you meant. > >> dwc3_omap_write_utmi_ctrl(omap, val); >> break; >> >> case OMAP_DWC3_VBUS_VALID: >> val = dwc3_omap_read_utmi_ctrl(omap); >> val &= ~USBOTGSS_UTMI_OTG_CTRL_SESSEND; >> - val |= USBOTGSS_UTMI_OTG_CTRL_IDDIG >> - | USBOTGSS_UTMI_OTG_CTRL_VBUSVALID >> + val |= USBOTGSS_UTMI_OTG_CTRL_VBUSVALID >> | USBOTGSS_UTMI_OTG_CTRL_SESSVALID >> | USBOTGSS_UTMI_OTG_CTRL_POWERPRESENT; > > I remember discussing this with TI's IP owner back in OMAP5 days. This > code was a result of talking to that guy and was, back then, tested by > Silicon Validation team. I would strongly advise that before changing > these bits you check with whoever's currently handling this IP inside TI > to make sure your changes are still within the expectations of the > wrapper block. > OK, I wasn't aware about this. I will check with the Silicon team. cheers, -roger
On 11/04/16 16:26, Felipe Balbi wrote: > > Hi, > > Roger Quadros <rogerq@ti.com> writes: >> On 11/04/16 15:18, Felipe Balbi wrote: >>> >>> Hi, >>> >>> Roger Quadros <rogerq@ti.com> writes: >>>> Don't make any decisions regarding VBUS session based on ID >>>> status. That is best left to the OTG core. >>> >>> what about builds who don't want OTG and/or dual-role ? >>> >>>> Pass ID and VBUS events independent of each other so that OTG >>>> core knows exactly what to do. >>>> >>>> This makes dual-role with extcon work with OTG irq on OMAP platforms. >>>> >>>> Signed-off-by: Roger Quadros <rogerq@ti.com> >>>> --- >>>> drivers/usb/dwc3/dwc3-omap.c | 15 ++++++--------- >>>> 1 file changed, 6 insertions(+), 9 deletions(-) >>>> >>>> diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c >>>> index 51ca098..c9b918d 100644 >>>> --- a/drivers/usb/dwc3/dwc3-omap.c >>>> +++ b/drivers/usb/dwc3/dwc3-omap.c >>>> @@ -233,19 +233,14 @@ static void dwc3_omap_set_mailbox(struct dwc3_omap *omap, >>>> } >>>> >>>> val = dwc3_omap_read_utmi_ctrl(omap); >>>> - val &= ~(USBOTGSS_UTMI_OTG_CTRL_IDDIG >>>> - | USBOTGSS_UTMI_OTG_CTRL_VBUSVALID >>>> - | USBOTGSS_UTMI_OTG_CTRL_SESSEND); >>>> - val |= USBOTGSS_UTMI_OTG_CTRL_SESSVALID >>>> - | USBOTGSS_UTMI_OTG_CTRL_POWERPRESENT; >>>> + val &= ~USBOTGSS_UTMI_OTG_CTRL_IDDIG; >>> >>> this creates the possibility of having a USB peripheral with VBUS_VALID, >>> right >> >> Sorry, I didn't get what you meant. > > if you're touching these bits independently from each other, won't you > have a situation where you notify IDFLOAT but don't notify that VBUS is > off ? > We're setting the mailbox state for both ID and VBUS in dwc3_omap_extcon_register() depending on the initial extcon state. After that, ID and VBUS will be signalled only when they change. cheers, -roger
diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c index 51ca098..c9b918d 100644 --- a/drivers/usb/dwc3/dwc3-omap.c +++ b/drivers/usb/dwc3/dwc3-omap.c @@ -233,19 +233,14 @@ static void dwc3_omap_set_mailbox(struct dwc3_omap *omap, } val = dwc3_omap_read_utmi_ctrl(omap); - val &= ~(USBOTGSS_UTMI_OTG_CTRL_IDDIG - | USBOTGSS_UTMI_OTG_CTRL_VBUSVALID - | USBOTGSS_UTMI_OTG_CTRL_SESSEND); - val |= USBOTGSS_UTMI_OTG_CTRL_SESSVALID - | USBOTGSS_UTMI_OTG_CTRL_POWERPRESENT; + val &= ~USBOTGSS_UTMI_OTG_CTRL_IDDIG; dwc3_omap_write_utmi_ctrl(omap, val); break; case OMAP_DWC3_VBUS_VALID: val = dwc3_omap_read_utmi_ctrl(omap); val &= ~USBOTGSS_UTMI_OTG_CTRL_SESSEND; - val |= USBOTGSS_UTMI_OTG_CTRL_IDDIG - | USBOTGSS_UTMI_OTG_CTRL_VBUSVALID + val |= USBOTGSS_UTMI_OTG_CTRL_VBUSVALID | USBOTGSS_UTMI_OTG_CTRL_SESSVALID | USBOTGSS_UTMI_OTG_CTRL_POWERPRESENT; dwc3_omap_write_utmi_ctrl(omap, val); @@ -254,14 +249,16 @@ static void dwc3_omap_set_mailbox(struct dwc3_omap *omap, case OMAP_DWC3_ID_FLOAT: if (omap->vbus_reg) regulator_disable(omap->vbus_reg); + val = dwc3_omap_read_utmi_ctrl(omap); + val |= USBOTGSS_UTMI_OTG_CTRL_IDDIG; + dwc3_omap_write_utmi_ctrl(omap, val); case OMAP_DWC3_VBUS_OFF: val = dwc3_omap_read_utmi_ctrl(omap); val &= ~(USBOTGSS_UTMI_OTG_CTRL_SESSVALID | USBOTGSS_UTMI_OTG_CTRL_VBUSVALID | USBOTGSS_UTMI_OTG_CTRL_POWERPRESENT); - val |= USBOTGSS_UTMI_OTG_CTRL_SESSEND - | USBOTGSS_UTMI_OTG_CTRL_IDDIG; + val |= USBOTGSS_UTMI_OTG_CTRL_SESSEND; dwc3_omap_write_utmi_ctrl(omap, val); break;
Don't make any decisions regarding VBUS session based on ID status. That is best left to the OTG core. Pass ID and VBUS events independent of each other so that OTG core knows exactly what to do. This makes dual-role with extcon work with OTG irq on OMAP platforms. Signed-off-by: Roger Quadros <rogerq@ti.com> --- drivers/usb/dwc3/dwc3-omap.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) -- 2.5.0