| Message ID | 1414497280-3126-2-git-send-email-ray.huang@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
> From: Huang Rui [mailto:ray.huang@amd.com] > Sent: Tuesday, October 28, 2014 4:54 AM > > It enables hibernation if the function is set in coreConsultant. > > Suggested-by: Felipe Balbi <balbi@ti.com> > Signed-off-by: Huang Rui <ray.huang@amd.com> > --- > drivers/usb/dwc3/core.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > index fa396fc..bf77509 100644 > --- a/drivers/usb/dwc3/core.c > +++ b/drivers/usb/dwc3/core.c > @@ -449,6 +449,7 @@ static int dwc3_core_init(struct dwc3 *dwc) > case DWC3_GHWPARAMS1_EN_PWROPT_HIB: > /* enable hibernation here */ > dwc->nr_scratch = DWC3_GHWPARAMS4_HIBER_SCRATCHBUFS(hwparams4); > + reg |= DWC3_GCTL_GBLHIBERNATIONEN; > break; > default: > dev_dbg(dwc->dev, "No power optimization available\n"); What effect does this have when the controller is in device mode? I expect it will start generating DWC3_DEVICE_EVENT_HIBER_REQ interrupt events when this register bit is set. So the dev_WARN_ONCE in dwc3_gadget_interrupt() will start firing, I think.
On Tue, Oct 28, 2014 at 06:47:08PM +0000, Paul Zimmerman wrote: > > From: Huang Rui [mailto:ray.huang@amd.com] > > Sent: Tuesday, October 28, 2014 4:54 AM > > > > It enables hibernation if the function is set in coreConsultant. > > > > Suggested-by: Felipe Balbi <balbi@ti.com> > > Signed-off-by: Huang Rui <ray.huang@amd.com> > > --- > > drivers/usb/dwc3/core.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > > index fa396fc..bf77509 100644 > > --- a/drivers/usb/dwc3/core.c > > +++ b/drivers/usb/dwc3/core.c > > @@ -449,6 +449,7 @@ static int dwc3_core_init(struct dwc3 *dwc) > > case DWC3_GHWPARAMS1_EN_PWROPT_HIB: > > /* enable hibernation here */ > > dwc->nr_scratch = DWC3_GHWPARAMS4_HIBER_SCRATCHBUFS(hwparams4); > > + reg |= DWC3_GCTL_GBLHIBERNATIONEN; > > break; > > default: > > dev_dbg(dwc->dev, "No power optimization available\n"); > > What effect does this have when the controller is in device mode? I > expect it will start generating DWC3_DEVICE_EVENT_HIBER_REQ interrupt > events when this register bit is set. So the dev_WARN_ONCE in > dwc3_gadget_interrupt() will start firing, I think. Ok, so this *has* to wait for proper hibernation support ?
> From: Felipe Balbi [mailto:balbi@ti.com] > Sent: Tuesday, October 28, 2014 11:51 AM > > On Tue, Oct 28, 2014 at 06:47:08PM +0000, Paul Zimmerman wrote: > > > From: Huang Rui [mailto:ray.huang@amd.com] > > > Sent: Tuesday, October 28, 2014 4:54 AM > > > > > > It enables hibernation if the function is set in coreConsultant. > > > > > > Suggested-by: Felipe Balbi <balbi@ti.com> > > > Signed-off-by: Huang Rui <ray.huang@amd.com> > > > --- > > > drivers/usb/dwc3/core.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > > > index fa396fc..bf77509 100644 > > > --- a/drivers/usb/dwc3/core.c > > > +++ b/drivers/usb/dwc3/core.c > > > @@ -449,6 +449,7 @@ static int dwc3_core_init(struct dwc3 *dwc) > > > case DWC3_GHWPARAMS1_EN_PWROPT_HIB: > > > /* enable hibernation here */ > > > dwc->nr_scratch = DWC3_GHWPARAMS4_HIBER_SCRATCHBUFS(hwparams4); > > > + reg |= DWC3_GCTL_GBLHIBERNATIONEN; > > > break; > > > default: > > > dev_dbg(dwc->dev, "No power optimization available\n"); > > > > What effect does this have when the controller is in device mode? I > > expect it will start generating DWC3_DEVICE_EVENT_HIBER_REQ interrupt > > events when this register bit is set. So the dev_WARN_ONCE in > > dwc3_gadget_interrupt() will start firing, I think. > > Ok, so this *has* to wait for proper hibernation support ? Ah, never mind. Since the hibernation event is not enabled in the DEVTEN register, the controller shouldn't generate that event after all. So I think it should be OK. Sorry for the noise.
On Tue, Oct 28, 2014 at 06:55:50PM +0000, Paul Zimmerman wrote: > > From: Felipe Balbi [mailto:balbi@ti.com] > > Sent: Tuesday, October 28, 2014 11:51 AM > > > > On Tue, Oct 28, 2014 at 06:47:08PM +0000, Paul Zimmerman wrote: > > > > From: Huang Rui [mailto:ray.huang@amd.com] > > > > Sent: Tuesday, October 28, 2014 4:54 AM > > > > > > > > It enables hibernation if the function is set in coreConsultant. > > > > > > > > Suggested-by: Felipe Balbi <balbi@ti.com> > > > > Signed-off-by: Huang Rui <ray.huang@amd.com> > > > > --- > > > > drivers/usb/dwc3/core.c | 1 + > > > > 1 file changed, 1 insertion(+) > > > > > > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > > > > index fa396fc..bf77509 100644 > > > > --- a/drivers/usb/dwc3/core.c > > > > +++ b/drivers/usb/dwc3/core.c > > > > @@ -449,6 +449,7 @@ static int dwc3_core_init(struct dwc3 *dwc) > > > > case DWC3_GHWPARAMS1_EN_PWROPT_HIB: > > > > /* enable hibernation here */ > > > > dwc->nr_scratch = DWC3_GHWPARAMS4_HIBER_SCRATCHBUFS(hwparams4); > > > > + reg |= DWC3_GCTL_GBLHIBERNATIONEN; > > > > break; > > > > default: > > > > dev_dbg(dwc->dev, "No power optimization available\n"); > > > > > > What effect does this have when the controller is in device mode? I > > > expect it will start generating DWC3_DEVICE_EVENT_HIBER_REQ interrupt > > > events when this register bit is set. So the dev_WARN_ONCE in > > > dwc3_gadget_interrupt() will start firing, I think. > > > > Ok, so this *has* to wait for proper hibernation support ? > > Ah, never mind. Since the hibernation event is not enabled in the > DEVTEN register, the controller shouldn't generate that event after > all. So I think it should be OK. Sorry for the noise. do you think it's still nice to have a comment here ?
> From: Felipe Balbi [mailto:balbi@ti.com] > Sent: Tuesday, October 28, 2014 12:01 PM > > On Tue, Oct 28, 2014 at 06:55:50PM +0000, Paul Zimmerman wrote: > > > From: Felipe Balbi [mailto:balbi@ti.com] > > > Sent: Tuesday, October 28, 2014 11:51 AM > > > > > > On Tue, Oct 28, 2014 at 06:47:08PM +0000, Paul Zimmerman wrote: > > > > > From: Huang Rui [mailto:ray.huang@amd.com] > > > > > Sent: Tuesday, October 28, 2014 4:54 AM > > > > > > > > > > It enables hibernation if the function is set in coreConsultant. > > > > > > > > > > Suggested-by: Felipe Balbi <balbi@ti.com> > > > > > Signed-off-by: Huang Rui <ray.huang@amd.com> > > > > > --- > > > > > drivers/usb/dwc3/core.c | 1 + > > > > > 1 file changed, 1 insertion(+) > > > > > > > > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > > > > > index fa396fc..bf77509 100644 > > > > > --- a/drivers/usb/dwc3/core.c > > > > > +++ b/drivers/usb/dwc3/core.c > > > > > @@ -449,6 +449,7 @@ static int dwc3_core_init(struct dwc3 *dwc) > > > > > case DWC3_GHWPARAMS1_EN_PWROPT_HIB: > > > > > /* enable hibernation here */ > > > > > dwc->nr_scratch = DWC3_GHWPARAMS4_HIBER_SCRATCHBUFS(hwparams4); > > > > > + reg |= DWC3_GCTL_GBLHIBERNATIONEN; > > > > > break; > > > > > default: > > > > > dev_dbg(dwc->dev, "No power optimization available\n"); > > > > > > > > What effect does this have when the controller is in device mode? I > > > > expect it will start generating DWC3_DEVICE_EVENT_HIBER_REQ interrupt > > > > events when this register bit is set. So the dev_WARN_ONCE in > > > > dwc3_gadget_interrupt() will start firing, I think. > > > > > > Ok, so this *has* to wait for proper hibernation support ? > > > > Ah, never mind. Since the hibernation event is not enabled in the > > DEVTEN register, the controller shouldn't generate that event after > > all. So I think it should be OK. Sorry for the noise. > > do you think it's still nice to have a comment here ? Maybe something along the lines of "enabling this bit so that host-mode hibernation will work, device-mode hibernation is not implemented yet"?
On Tue, Oct 28, 2014 at 07:15:29PM +0000, Paul Zimmerman wrote: > > From: Felipe Balbi [mailto:balbi@ti.com] > > Sent: Tuesday, October 28, 2014 12:01 PM > > > > On Tue, Oct 28, 2014 at 06:55:50PM +0000, Paul Zimmerman wrote: > > > > From: Felipe Balbi [mailto:balbi@ti.com] > > > > Sent: Tuesday, October 28, 2014 11:51 AM > > > > > > > > On Tue, Oct 28, 2014 at 06:47:08PM +0000, Paul Zimmerman wrote: > > > > > > From: Huang Rui [mailto:ray.huang@amd.com] > > > > > > Sent: Tuesday, October 28, 2014 4:54 AM > > > > > > > > > > > > It enables hibernation if the function is set in coreConsultant. > > > > > > > > > > > > Suggested-by: Felipe Balbi <balbi@ti.com> > > > > > > Signed-off-by: Huang Rui <ray.huang@amd.com> > > > > > > --- > > > > > > drivers/usb/dwc3/core.c | 1 + > > > > > > 1 file changed, 1 insertion(+) > > > > > > > > > > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > > > > > > index fa396fc..bf77509 100644 > > > > > > --- a/drivers/usb/dwc3/core.c > > > > > > +++ b/drivers/usb/dwc3/core.c > > > > > > @@ -449,6 +449,7 @@ static int dwc3_core_init(struct dwc3 *dwc) > > > > > > case DWC3_GHWPARAMS1_EN_PWROPT_HIB: > > > > > > /* enable hibernation here */ > > > > > > dwc->nr_scratch = DWC3_GHWPARAMS4_HIBER_SCRATCHBUFS(hwparams4); > > > > > > + reg |= DWC3_GCTL_GBLHIBERNATIONEN; > > > > > > break; > > > > > > default: > > > > > > dev_dbg(dwc->dev, "No power optimization available\n"); > > > > > > > > > > What effect does this have when the controller is in device mode? I > > > > > expect it will start generating DWC3_DEVICE_EVENT_HIBER_REQ interrupt > > > > > events when this register bit is set. So the dev_WARN_ONCE in > > > > > dwc3_gadget_interrupt() will start firing, I think. > > > > > > > > Ok, so this *has* to wait for proper hibernation support ? > > > > > > Ah, never mind. Since the hibernation event is not enabled in the > > > DEVTEN register, the controller shouldn't generate that event after > > > all. So I think it should be OK. Sorry for the noise. > > > > do you think it's still nice to have a comment here ? > > Maybe something along the lines of "enabling this bit so that host-mode > hibernation will work, device-mode hibernation is not implemented yet"? sounds good to me. Huang, can you update your patch to add this comment Paul suggested ? thanks
On Tue, Oct 28, 2014 at 02:18:52PM -0500, Felipe Balbi wrote: > On Tue, Oct 28, 2014 at 07:15:29PM +0000, Paul Zimmerman wrote: > > > From: Felipe Balbi [mailto:balbi@ti.com] > > > Sent: Tuesday, October 28, 2014 12:01 PM > > > > > > On Tue, Oct 28, 2014 at 06:55:50PM +0000, Paul Zimmerman wrote: > > > > > From: Felipe Balbi [mailto:balbi@ti.com] > > > > > Sent: Tuesday, October 28, 2014 11:51 AM > > > > > > > > > > On Tue, Oct 28, 2014 at 06:47:08PM +0000, Paul Zimmerman wrote: > > > > > > > From: Huang Rui [mailto:ray.huang@amd.com] > > > > > > > Sent: Tuesday, October 28, 2014 4:54 AM > > > > > > > > > > > > > > It enables hibernation if the function is set in coreConsultant. > > > > > > > > > > > > > > Suggested-by: Felipe Balbi <balbi@ti.com> > > > > > > > Signed-off-by: Huang Rui <ray.huang@amd.com> > > > > > > > --- > > > > > > > drivers/usb/dwc3/core.c | 1 + > > > > > > > 1 file changed, 1 insertion(+) > > > > > > > > > > > > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > > > > > > > index fa396fc..bf77509 100644 > > > > > > > --- a/drivers/usb/dwc3/core.c > > > > > > > +++ b/drivers/usb/dwc3/core.c > > > > > > > @@ -449,6 +449,7 @@ static int dwc3_core_init(struct dwc3 *dwc) > > > > > > > case DWC3_GHWPARAMS1_EN_PWROPT_HIB: > > > > > > > /* enable hibernation here */ > > > > > > > dwc->nr_scratch = DWC3_GHWPARAMS4_HIBER_SCRATCHBUFS(hwparams4); > > > > > > > + reg |= DWC3_GCTL_GBLHIBERNATIONEN; > > > > > > > break; > > > > > > > default: > > > > > > > dev_dbg(dwc->dev, "No power optimization available\n"); > > > > > > > > > > > > What effect does this have when the controller is in device mode? I > > > > > > expect it will start generating DWC3_DEVICE_EVENT_HIBER_REQ interrupt > > > > > > events when this register bit is set. So the dev_WARN_ONCE in > > > > > > dwc3_gadget_interrupt() will start firing, I think. > > > > > > > > > > Ok, so this *has* to wait for proper hibernation support ? > > > > > > > > Ah, never mind. Since the hibernation event is not enabled in the > > > > DEVTEN register, the controller shouldn't generate that event after > > > > all. So I think it should be OK. Sorry for the noise. > > > > > > do you think it's still nice to have a comment here ? > > > > Maybe something along the lines of "enabling this bit so that host-mode > > hibernation will work, device-mode hibernation is not implemented yet"? > > sounds good to me. Huang, can you update your patch to add this comment > Paul suggested ? > OK, will update in V4. Thanks, Rui -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index fa396fc..bf77509 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -449,6 +449,7 @@ static int dwc3_core_init(struct dwc3 *dwc) case DWC3_GHWPARAMS1_EN_PWROPT_HIB: /* enable hibernation here */ dwc->nr_scratch = DWC3_GHWPARAMS4_HIBER_SCRATCHBUFS(hwparams4); + reg |= DWC3_GCTL_GBLHIBERNATIONEN; break; default: dev_dbg(dwc->dev, "No power optimization available\n");
It enables hibernation if the function is set in coreConsultant. Suggested-by: Felipe Balbi <balbi@ti.com> Signed-off-by: Huang Rui <ray.huang@amd.com> --- drivers/usb/dwc3/core.c | 1 + 1 file changed, 1 insertion(+)