| Message ID | 20190405125554.18070-6-jjhiblot@ti.com |
|---|---|
| State | Superseded |
| Delegated to: | Marek Vasut |
| Headers | show |
| Series | Improvement for the DWC3 USB generic driver and fixes for the K2 platforms | expand |
On 4/5/19 2:55 PM, Jean-Jacques Hiblot wrote: > This allow the phy to enter idle and then suspend. > the K2 platforms require the PHY to be suspended before the USB domain > clock can be turned off. > > Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com> > --- > > drivers/usb/dwc3/core.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > index 56e2a046bf..ae01490306 100644 > --- a/drivers/usb/dwc3/core.c > +++ b/drivers/usb/dwc3/core.c > @@ -581,6 +581,12 @@ static int dwc3_core_init_mode(struct dwc3 *dwc) > return 0; > } > > +static void dwc3_gadget_run(struct dwc3 *dwc) > +{ > + dwc3_writel(dwc->regs, DWC3_DCTL, DWC3_DCTL_RUN_STOP); > + mdelay(100); That's long, is this really needed ? > +} > + > static void dwc3_core_exit_mode(struct dwc3 *dwc) > { > switch (dwc->dr_mode) { > @@ -598,6 +604,13 @@ static void dwc3_core_exit_mode(struct dwc3 *dwc) > /* do nothing */ > break; > } > + > + /* > + * switch back to peripheral mode > + * This enables the phy to enter idle and then, if enabled, suspend. > + */ > + dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE); > + dwc3_gadget_run(dwc); > } > > #define DWC3_ALIGN_MASK (16 - 1) >
On 29/04/2019 11:56, Marek Vasut wrote: > On 4/5/19 2:55 PM, Jean-Jacques Hiblot wrote: >> This allow the phy to enter idle and then suspend. >> the K2 platforms require the PHY to be suspended before the USB domain >> clock can be turned off. >> >> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com> >> --- >> >> drivers/usb/dwc3/core.c | 13 +++++++++++++ >> 1 file changed, 13 insertions(+) >> >> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >> index 56e2a046bf..ae01490306 100644 >> --- a/drivers/usb/dwc3/core.c >> +++ b/drivers/usb/dwc3/core.c >> @@ -581,6 +581,12 @@ static int dwc3_core_init_mode(struct dwc3 *dwc) >> return 0; >> } >> >> +static void dwc3_gadget_run(struct dwc3 *dwc) >> +{ >> + dwc3_writel(dwc->regs, DWC3_DCTL, DWC3_DCTL_RUN_STOP); >> + mdelay(100); > That's long, is this really needed ? I took my queue from the old keystone code (keystone_xhci_phy_suspend(void) in v2018.11) and also from several places in this core.c. I don't know what would be the exact value required: mdelay(10) is too short, mdelay(20) works for me. mdelay(100) seems quite safe. I don't think that is a big problem here, IMHO USB init/exit are not time critical. > >> +} >> + >> static void dwc3_core_exit_mode(struct dwc3 *dwc) >> { >> switch (dwc->dr_mode) { >> @@ -598,6 +604,13 @@ static void dwc3_core_exit_mode(struct dwc3 *dwc) >> /* do nothing */ >> break; >> } >> + >> + /* >> + * switch back to peripheral mode >> + * This enables the phy to enter idle and then, if enabled, suspend. >> + */ >> + dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE); >> + dwc3_gadget_run(dwc); >> } >> >> #define DWC3_ALIGN_MASK (16 - 1) >> >
On 5/3/19 11:26 AM, Jean-Jacques Hiblot wrote: > > On 29/04/2019 11:56, Marek Vasut wrote: >> On 4/5/19 2:55 PM, Jean-Jacques Hiblot wrote: >>> This allow the phy to enter idle and then suspend. >>> the K2 platforms require the PHY to be suspended before the USB domain >>> clock can be turned off. >>> >>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com> >>> --- >>> >>> drivers/usb/dwc3/core.c | 13 +++++++++++++ >>> 1 file changed, 13 insertions(+) >>> >>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >>> index 56e2a046bf..ae01490306 100644 >>> --- a/drivers/usb/dwc3/core.c >>> +++ b/drivers/usb/dwc3/core.c >>> @@ -581,6 +581,12 @@ static int dwc3_core_init_mode(struct dwc3 *dwc) >>> return 0; >>> } >>> +static void dwc3_gadget_run(struct dwc3 *dwc) >>> +{ >>> + dwc3_writel(dwc->regs, DWC3_DCTL, DWC3_DCTL_RUN_STOP); >>> + mdelay(100); >> That's long, is this really needed ? > > I took my queue from the old keystone code > (keystone_xhci_phy_suspend(void) in v2018.11) and also from several > places in this core.c. > > I don't know what would be the exact value required: mdelay(10) is too > short, mdelay(20) works for me. mdelay(100) seems quite safe. > > I don't think that is a big problem here, IMHO USB init/exit are not > time critical. OK, it might be annoying to the user though.
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 56e2a046bf..ae01490306 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -581,6 +581,12 @@ static int dwc3_core_init_mode(struct dwc3 *dwc) return 0; } +static void dwc3_gadget_run(struct dwc3 *dwc) +{ + dwc3_writel(dwc->regs, DWC3_DCTL, DWC3_DCTL_RUN_STOP); + mdelay(100); +} + static void dwc3_core_exit_mode(struct dwc3 *dwc) { switch (dwc->dr_mode) { @@ -598,6 +604,13 @@ static void dwc3_core_exit_mode(struct dwc3 *dwc) /* do nothing */ break; } + + /* + * switch back to peripheral mode + * This enables the phy to enter idle and then, if enabled, suspend. + */ + dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE); + dwc3_gadget_run(dwc); } #define DWC3_ALIGN_MASK (16 - 1)
This allow the phy to enter idle and then suspend. the K2 platforms require the PHY to be suspended before the USB domain clock can be turned off. Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com> --- drivers/usb/dwc3/core.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)