| Message ID | 20211208100545.1441397-3-neal_liu@aspeedtech.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Refactor Aspeed USB vhub driver | expand |
On Wed, 2021-12-08 at 18:05 +0800, Neal Liu wrote: > Signaling remote wakeup if an emulated USB device has any activity > if the device is allowed by host. > > Signed-off-by: Neal Liu <neal_liu@aspeedtech.com> I still think it should fundamentally be the device making that decision, but since they don't, this is an acceptable workaround, but please, don't write the MMIO on every EP queue. Either keep track of the bus being suspended, or turn on the AUTO bit in HW when wakeup_en is set. Cheers, Ben. > --- > drivers/usb/gadget/udc/aspeed-vhub/epn.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/usb/gadget/udc/aspeed-vhub/epn.c > b/drivers/usb/gadget/udc/aspeed-vhub/epn.c > index 917892ca8753..ccc239b5cc17 100644 > --- a/drivers/usb/gadget/udc/aspeed-vhub/epn.c > +++ b/drivers/usb/gadget/udc/aspeed-vhub/epn.c > @@ -381,6 +381,11 @@ static int ast_vhub_epn_queue(struct usb_ep* > u_ep, struct usb_request *u_req, > } else > u_req->dma = 0; > > + if (ep->dev->wakeup_en) { > + EPVDBG(ep, "Wakeup host first\n"); > + ast_vhub_hub_wake_all(vhub); > + } > + > EPVDBG(ep, "enqueue req @%p\n", req); > EPVDBG(ep, " l=%d dma=0x%x zero=%d noshort=%d noirq=%d > is_in=%d\n", > u_req->length, (u32)u_req->dma, u_req->zero,
> -----Original Message----- > From: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Sent: Thursday, December 9, 2021 8:05 AM > To: Neal Liu <neal_liu@aspeedtech.com>; Felipe Balbi <balbi@kernel.org>; > Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Joel Stanley > <joel@jms.id.au>; Andrew Jeffery <andrew@aj.id.au>; Cai Huoqing > <caihuoqing@baidu.com>; Tao Ren <rentao.bupt@gmail.com>; Julia Lawall > <julia.lawall@inria.fr>; kernel test robot <lkp@intel.com>; Sasha Levin > <sashal@kernel.org>; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org; > linux-arm-kernel@lists.infradead.org; linux-aspeed@lists.ozlabs.org > Cc: BMC-SW <BMC-SW@aspeedtech.com> > Subject: Re: [PATCH v3 2/4] usb: aspeed-vhub: fix remote wakeup failure in > iKVM use case > > On Wed, 2021-12-08 at 18:05 +0800, Neal Liu wrote: > > Signaling remote wakeup if an emulated USB device has any activity if > > the device is allowed by host. > > > > Signed-off-by: Neal Liu <neal_liu@aspeedtech.com> > > I still think it should fundamentally be the device making that decision, but > since they don't, this is an acceptable workaround, but please, don't write the > MMIO on every EP queue. Either keep track of the bus being suspended, or > turn on the AUTO bit in HW when wakeup_en is set. > > Cheers, > Ben. > I'm confused. Signaling Wakeup when wakeup_en is set if it has any ep activities is not exactly what you said? wakeup_en is set only if host allows this device have wakeup capability and bus being suspended. Normal ep activities would not write the MMIO because wakeup_en is not set. Thanks -Neal > > --- > > drivers/usb/gadget/udc/aspeed-vhub/epn.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/drivers/usb/gadget/udc/aspeed-vhub/epn.c > > b/drivers/usb/gadget/udc/aspeed-vhub/epn.c > > index 917892ca8753..ccc239b5cc17 100644 > > --- a/drivers/usb/gadget/udc/aspeed-vhub/epn.c > > +++ b/drivers/usb/gadget/udc/aspeed-vhub/epn.c > > @@ -381,6 +381,11 @@ static int ast_vhub_epn_queue(struct usb_ep* > > u_ep, struct usb_request *u_req, > > } else > > u_req->dma = 0; > > > > + if (ep->dev->wakeup_en) { > > + EPVDBG(ep, "Wakeup host first\n"); > > + ast_vhub_hub_wake_all(vhub); > > + } > > + > > EPVDBG(ep, "enqueue req @%p\n", req); > > EPVDBG(ep, " l=%d dma=0x%x zero=%d noshort=%d noirq=%d > is_in=%d\n", > > u_req->length, (u32)u_req->dma, u_req->zero,
On Wed, Dec 08, 2021 at 06:05:43PM +0800, Neal Liu wrote: > Signaling remote wakeup if an emulated USB device has any activity > if the device is allowed by host. > > Signed-off-by: Neal Liu <neal_liu@aspeedtech.com> > --- > drivers/usb/gadget/udc/aspeed-vhub/epn.c | 5 +++++ > 1 file changed, 5 insertions(+) What commit does this fix? Does it need to go to stable kernels? Should it be independent of this patch series that adds new features? thanks, greg k-h
On Thu, 2021-12-09 at 02:37 +0000, Neal Liu wrote: > I'm confused. Signaling Wakeup when wakeup_en is set if it has any ep > activities is not exactly what you said? > > wakeup_en is set only if host allows this device have wakeup > capability and bus being suspended. > > Normal ep activities would not write the MMIO because wakeup_en is > not set. Hrm... I didn't think wakeup_en was limited to the bus being suspended, but maybe I misremember, it's been a while. Cheers, Ben.
> -----Original Message----- > From: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Sent: Tuesday, December 14, 2021 10:36 AM > To: Neal Liu <neal_liu@aspeedtech.com>; Felipe Balbi <balbi@kernel.org>; > Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Joel Stanley > <joel@jms.id.au>; Andrew Jeffery <andrew@aj.id.au>; Cai Huoqing > <caihuoqing@baidu.com>; Tao Ren <rentao.bupt@gmail.com>; Julia Lawall > <julia.lawall@inria.fr>; kernel test robot <lkp@intel.com>; Sasha Levin > <sashal@kernel.org>; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org; > linux-arm-kernel@lists.infradead.org; linux-aspeed@lists.ozlabs.org > Cc: BMC-SW <BMC-SW@aspeedtech.com> > Subject: Re: [PATCH v3 2/4] usb: aspeed-vhub: fix remote wakeup failure in > iKVM use case > > On Thu, 2021-12-09 at 02:37 +0000, Neal Liu wrote: > > I'm confused. Signaling Wakeup when wakeup_en is set if it has any ep > > activities is not exactly what you said? > > > > wakeup_en is set only if host allows this device have wakeup > > capability and bus being suspended. > > > > Normal ep activities would not write the MMIO because wakeup_en is not > > set. > > Hrm... I didn't think wakeup_en was limited to the bus being suspended, but > maybe I misremember, it's been a while. > > Cheers, > Ben. > wakeup_en is only set in the case of host set USB_DEVICE_REMOTE_WAKEUP feature to vhub devices. I think this behavior only occurs during host is going to suspend, and set this feature to any device which can wakeup itself before sleep. Thanks -Neal
> -----Original Message----- > From: Neal Liu > Sent: Monday, December 20, 2021 10:23 AM > To: Benjamin Herrenschmidt <benh@kernel.crashing.org>; Felipe Balbi > <balbi@kernel.org>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Joel > Stanley <joel@jms.id.au>; Andrew Jeffery <andrew@aj.id.au>; Cai Huoqing > <caihuoqing@baidu.com>; Tao Ren <rentao.bupt@gmail.com>; Julia Lawall > <julia.lawall@inria.fr>; kernel test robot <lkp@intel.com>; Sasha Levin > <sashal@kernel.org>; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org; > linux-arm-kernel@lists.infradead.org; linux-aspeed@lists.ozlabs.org > Cc: BMC-SW <BMC-SW@aspeedtech.com> > Subject: RE: [PATCH v3 2/4] usb: aspeed-vhub: fix remote wakeup failure in > iKVM use case > > > -----Original Message----- > > From: Benjamin Herrenschmidt <benh@kernel.crashing.org> > > Sent: Tuesday, December 14, 2021 10:36 AM > > To: Neal Liu <neal_liu@aspeedtech.com>; Felipe Balbi > > <balbi@kernel.org>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>; > > Joel Stanley <joel@jms.id.au>; Andrew Jeffery <andrew@aj.id.au>; Cai > > Huoqing <caihuoqing@baidu.com>; Tao Ren <rentao.bupt@gmail.com>; Julia > > Lawall <julia.lawall@inria.fr>; kernel test robot <lkp@intel.com>; > > Sasha Levin <sashal@kernel.org>; linux-usb@vger.kernel.org; > > linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > > linux-aspeed@lists.ozlabs.org > > Cc: BMC-SW <BMC-SW@aspeedtech.com> > > Subject: Re: [PATCH v3 2/4] usb: aspeed-vhub: fix remote wakeup > > failure in iKVM use case > > > > On Thu, 2021-12-09 at 02:37 +0000, Neal Liu wrote: > > > I'm confused. Signaling Wakeup when wakeup_en is set if it has any > > > ep activities is not exactly what you said? > > > > > > wakeup_en is set only if host allows this device have wakeup > > > capability and bus being suspended. > > > > > > Normal ep activities would not write the MMIO because wakeup_en is > > > not set. > > > > Hrm... I didn't think wakeup_en was limited to the bus being > > suspended, but maybe I misremember, it's been a while. > > > > Cheers, > > Ben. > > > > wakeup_en is only set in the case of host set USB_DEVICE_REMOTE_WAKEUP > feature to vhub devices. > I think this behavior only occurs during host is going to suspend, and set this > feature to any device which can wakeup itself before sleep. > Thanks > > -Neal > Would you like to test it if you have some free time. Please feel free for any feedback. -Neal
> -----Original Message----- > From: Neal Liu > Sent: Monday, December 27, 2021 10:07 AM > To: Benjamin Herrenschmidt <benh@kernel.crashing.org>; Felipe Balbi > <balbi@kernel.org>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Joel > Stanley <joel@jms.id.au>; Andrew Jeffery <andrew@aj.id.au>; Cai Huoqing > <caihuoqing@baidu.com>; Tao Ren <rentao.bupt@gmail.com>; Julia Lawall > <julia.lawall@inria.fr>; kernel test robot <lkp@intel.com>; Sasha Levin > <sashal@kernel.org>; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org; > linux-arm-kernel@lists.infradead.org; linux-aspeed@lists.ozlabs.org > Cc: BMC-SW <BMC-SW@aspeedtech.com> > Subject: RE: [PATCH v3 2/4] usb: aspeed-vhub: fix remote wakeup failure in > iKVM use case > > > -----Original Message----- > > From: Neal Liu > > Sent: Monday, December 20, 2021 10:23 AM > > To: Benjamin Herrenschmidt <benh@kernel.crashing.org>; Felipe Balbi > > <balbi@kernel.org>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>; > > Joel Stanley <joel@jms.id.au>; Andrew Jeffery <andrew@aj.id.au>; Cai > > Huoqing <caihuoqing@baidu.com>; Tao Ren <rentao.bupt@gmail.com>; Julia > > Lawall <julia.lawall@inria.fr>; kernel test robot <lkp@intel.com>; > > Sasha Levin <sashal@kernel.org>; linux-usb@vger.kernel.org; > > linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > > linux-aspeed@lists.ozlabs.org > > Cc: BMC-SW <BMC-SW@aspeedtech.com> > > Subject: RE: [PATCH v3 2/4] usb: aspeed-vhub: fix remote wakeup > > failure in iKVM use case > > > > > -----Original Message----- > > > From: Benjamin Herrenschmidt <benh@kernel.crashing.org> > > > Sent: Tuesday, December 14, 2021 10:36 AM > > > To: Neal Liu <neal_liu@aspeedtech.com>; Felipe Balbi > > > <balbi@kernel.org>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>; > > > Joel Stanley <joel@jms.id.au>; Andrew Jeffery <andrew@aj.id.au>; Cai > > > Huoqing <caihuoqing@baidu.com>; Tao Ren <rentao.bupt@gmail.com>; > > > Julia Lawall <julia.lawall@inria.fr>; kernel test robot > > > <lkp@intel.com>; Sasha Levin <sashal@kernel.org>; > > > linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org; > > > linux-arm-kernel@lists.infradead.org; > > > linux-aspeed@lists.ozlabs.org > > > Cc: BMC-SW <BMC-SW@aspeedtech.com> > > > Subject: Re: [PATCH v3 2/4] usb: aspeed-vhub: fix remote wakeup > > > failure in iKVM use case > > > > > > On Thu, 2021-12-09 at 02:37 +0000, Neal Liu wrote: > > > > I'm confused. Signaling Wakeup when wakeup_en is set if it has any > > > > ep activities is not exactly what you said? > > > > > > > > wakeup_en is set only if host allows this device have wakeup > > > > capability and bus being suspended. > > > > > > > > Normal ep activities would not write the MMIO because wakeup_en is > > > > not set. > > > > > > Hrm... I didn't think wakeup_en was limited to the bus being > > > suspended, but maybe I misremember, it's been a while. > > > > > > Cheers, > > > Ben. > > > > > > > wakeup_en is only set in the case of host set > USB_DEVICE_REMOTE_WAKEUP > > feature to vhub devices. > > I think this behavior only occurs during host is going to suspend, and > > set this feature to any device which can wakeup itself before sleep. > > Thanks > > > > -Neal > > > Would you like to test it if you have some free time. > Please feel free for any feedback. > > -Neal Gentle ping for this patch. Thanks
diff --git a/drivers/usb/gadget/udc/aspeed-vhub/epn.c b/drivers/usb/gadget/udc/aspeed-vhub/epn.c index 917892ca8753..ccc239b5cc17 100644 --- a/drivers/usb/gadget/udc/aspeed-vhub/epn.c +++ b/drivers/usb/gadget/udc/aspeed-vhub/epn.c @@ -381,6 +381,11 @@ static int ast_vhub_epn_queue(struct usb_ep* u_ep, struct usb_request *u_req, } else u_req->dma = 0; + if (ep->dev->wakeup_en) { + EPVDBG(ep, "Wakeup host first\n"); + ast_vhub_hub_wake_all(vhub); + } + EPVDBG(ep, "enqueue req @%p\n", req); EPVDBG(ep, " l=%d dma=0x%x zero=%d noshort=%d noirq=%d is_in=%d\n", u_req->length, (u32)u_req->dma, u_req->zero,
Signaling remote wakeup if an emulated USB device has any activity if the device is allowed by host. Signed-off-by: Neal Liu <neal_liu@aspeedtech.com> --- drivers/usb/gadget/udc/aspeed-vhub/epn.c | 5 +++++ 1 file changed, 5 insertions(+)