* [PATCH 0/2] Provide a hook to check port init status @ 2022-08-16 8:38 Ray Chi 2022-08-16 8:38 ` [PATCH 1/2] usb: core: add " Ray Chi 2022-08-16 8:38 ` [PATCH 2/2] usb: xhci: add check_init_status hook support Ray Chi 0 siblings, 2 replies; 6+ messages in thread From: Ray Chi @ 2022-08-16 8:38 UTC (permalink / raw) To: gregkh, mathias.nyman, stern Cc: badhri, albertccwang, linux-usb, linux-kernel, Ray Chi Currently, only usbcore knows port init status even if the result is bad, so it will keep doing USB enumeration and other drivers can't do anything. Add the hook to let other drivers know the status and do error handling. Ray Chi (2): usb: core: add a hook to check port init status usb: xhci: add check_init_status hook support drivers/usb/core/hub.c | 14 ++++++++++++++ drivers/usb/host/xhci.c | 17 +++++++++++++++++ drivers/usb/host/xhci.h | 1 + include/linux/usb/hcd.h | 8 ++++++++ 4 files changed, 40 insertions(+) -- 2.37.1.595.g718a3a8f04-goog ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] usb: core: add a hook to check port init status 2022-08-16 8:38 [PATCH 0/2] Provide a hook to check port init status Ray Chi @ 2022-08-16 8:38 ` Ray Chi 2022-08-16 13:53 ` Alan Stern 2022-08-16 8:38 ` [PATCH 2/2] usb: xhci: add check_init_status hook support Ray Chi 1 sibling, 1 reply; 6+ messages in thread From: Ray Chi @ 2022-08-16 8:38 UTC (permalink / raw) To: gregkh, mathias.nyman, stern Cc: badhri, albertccwang, linux-usb, linux-kernel, Ray Chi This patch add a hook to check the port init status. Currently, only usbcore knows port init status even if the result is bad. It will cause a USB host keep doing USB enumeration for a long time when the USB host connects to a broken USB accessory. The hc_driver could use the hook to know port init status and do possible error handling according to platform requirements or limitations. Signed-off-by: Ray Chi <raychi@google.com> --- drivers/usb/core/hub.c | 14 ++++++++++++++ include/linux/usb/hcd.h | 8 ++++++++ 2 files changed, 22 insertions(+) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 2633acde7ac1..6ce6092816cb 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -4659,6 +4659,16 @@ static int hub_enable_device(struct usb_device *udev) return hcd->driver->enable_device(hcd, udev); } +static int hub_port_check_init_status(struct usb_device *udev, int r) +{ + struct usb_hcd *hcd = bus_to_hcd(udev->bus); + + if (!hcd->driver->check_init_status) + return 0; + + return hcd->driver->check_init_status(hcd, udev, r); +} + /* Reset device, (re)assign address, get device descriptor. * Device connection must be stable, no more debouncing needed. * Returns device in USB_STATE_ADDRESS, except on error. @@ -4855,6 +4865,10 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1, buf->bMaxPacketSize0; kfree(buf); + retval = hub_port_check_init_status(udev, r); + if (retval < 0) + goto fail; + retval = hub_port_reset(hub, port1, udev, delay, false); if (retval < 0) /* error or disconnect */ goto fail; diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h index 67f8713d3fa3..8fa30b4a6b7d 100644 --- a/include/linux/usb/hcd.h +++ b/include/linux/usb/hcd.h @@ -297,6 +297,14 @@ struct hc_driver { gfp_t mem_flags); void (*unmap_urb_for_dma)(struct usb_hcd *hcd, struct urb *urb); + /* + * (optional) HCD could get the information of GET_DESCRIPTOR by this hook. + * In general, it is not necessary unless the enumeration takes long + * time to do. The host controller could know the enumeration status by + * this hook and do some error handlings. + */ + int (*check_init_status)(struct usb_hcd *hcd, struct usb_device *udev, int r); + /* hw synch, freeing endpoint resources that urb_dequeue can't */ void (*endpoint_disable)(struct usb_hcd *hcd, struct usb_host_endpoint *ep); -- 2.37.1.595.g718a3a8f04-goog ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] usb: core: add a hook to check port init status 2022-08-16 8:38 ` [PATCH 1/2] usb: core: add " Ray Chi @ 2022-08-16 13:53 ` Alan Stern 0 siblings, 0 replies; 6+ messages in thread From: Alan Stern @ 2022-08-16 13:53 UTC (permalink / raw) To: Ray Chi Cc: gregkh, mathias.nyman, badhri, albertccwang, linux-usb, linux-kernel On Tue, Aug 16, 2022 at 04:38:53PM +0800, Ray Chi wrote: > This patch add a hook to check the port init status. Currently, only > usbcore knows port init status even if the result is bad. It will cause > a USB host keep doing USB enumeration for a long time when the USB host > connects to a broken USB accessory. > > The hc_driver could use the hook to know port init status and do possible > error handling according to platform requirements or limitations. > > Signed-off-by: Ray Chi <raychi@google.com> > --- > drivers/usb/core/hub.c | 14 ++++++++++++++ > include/linux/usb/hcd.h | 8 ++++++++ > 2 files changed, 22 insertions(+) > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c > index 2633acde7ac1..6ce6092816cb 100644 > --- a/drivers/usb/core/hub.c > +++ b/drivers/usb/core/hub.c > @@ -4855,6 +4865,10 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1, > buf->bMaxPacketSize0; > kfree(buf); > > + retval = hub_port_check_init_status(udev, r); > + if (retval < 0) > + goto fail; For future reference, you should be aware that this code won't get executed if do_new_scheme is false. Alan Stern ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] usb: xhci: add check_init_status hook support 2022-08-16 8:38 [PATCH 0/2] Provide a hook to check port init status Ray Chi 2022-08-16 8:38 ` [PATCH 1/2] usb: core: add " Ray Chi @ 2022-08-16 8:38 ` Ray Chi 2022-08-16 8:57 ` Greg KH 1 sibling, 1 reply; 6+ messages in thread From: Ray Chi @ 2022-08-16 8:38 UTC (permalink / raw) To: gregkh, mathias.nyman, stern Cc: badhri, albertccwang, linux-usb, linux-kernel, Ray Chi In general, xHCI didn't do anything for port initialization. However, there are some requirement or limitation on various platforms, so vendors need to do some error handlings if the device connected to a broken USB accessory. This patch also add the hook to xhci_driver_overrides so that vendors can add their specific protection easily if needed. Signed-off-by: Ray Chi <raychi@google.com> --- drivers/usb/host/xhci.c | 17 +++++++++++++++++ drivers/usb/host/xhci.h | 1 + 2 files changed, 18 insertions(+) diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 65858f607437..f237af9d6e2e 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -4358,6 +4358,20 @@ static int xhci_enable_device(struct usb_hcd *hcd, struct usb_device *udev) return xhci_setup_device(hcd, udev, SETUP_CONTEXT_ONLY); } +/* + * The function could get the status of port initialization. + */ +static int xhci_check_init_status(struct usb_hcd *hcd, struct usb_device *udev, int r) +{ + /* + * In general, this function is not necessory. Some platforms may + * need doing error handling when the port initialization takes a + * long time to do. The device can use the override callback to + * do specific handlings. + */ + return 0; +} + /* * Transfer the port index into real index in the HW port status * registers. Caculate offset between the port's PORTSC register @@ -5455,6 +5469,7 @@ static const struct hc_driver xhci_hc_driver = { .enable_device = xhci_enable_device, .update_hub_device = xhci_update_hub_device, .reset_device = xhci_discover_or_reset_device, + .check_init_status = xhci_check_init_status, /* * scheduling support @@ -5503,6 +5518,8 @@ void xhci_init_driver(struct hc_driver *drv, drv->check_bandwidth = over->check_bandwidth; if (over->reset_bandwidth) drv->reset_bandwidth = over->reset_bandwidth; + if (over->check_init_status) + drv->check_init_status = over->check_init_status; } } EXPORT_SYMBOL_GPL(xhci_init_driver); diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 1960b47acfb2..33ce873236e9 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1944,6 +1944,7 @@ struct xhci_driver_overrides { struct usb_host_endpoint *ep); int (*check_bandwidth)(struct usb_hcd *, struct usb_device *); void (*reset_bandwidth)(struct usb_hcd *, struct usb_device *); + int (*check_init_status)(struct usb_hcd *hcd, struct usb_device *udev, int r); }; #define XHCI_CFC_DELAY 10 -- 2.37.1.595.g718a3a8f04-goog ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] usb: xhci: add check_init_status hook support 2022-08-16 8:38 ` [PATCH 2/2] usb: xhci: add check_init_status hook support Ray Chi @ 2022-08-16 8:57 ` Greg KH 2022-08-22 15:42 ` Ray Chi 0 siblings, 1 reply; 6+ messages in thread From: Greg KH @ 2022-08-16 8:57 UTC (permalink / raw) To: Ray Chi Cc: mathias.nyman, stern, badhri, albertccwang, linux-usb, linux-kernel On Tue, Aug 16, 2022 at 04:38:54PM +0800, Ray Chi wrote: > In general, xHCI didn't do anything for port initialization. However, > there are some requirement or limitation on various platforms, so > vendors need to do some error handlings if the device connected to a > broken USB accessory. > > This patch also add the hook to xhci_driver_overrides so that vendors > can add their specific protection easily if needed. > > Signed-off-by: Ray Chi <raychi@google.com> > --- > drivers/usb/host/xhci.c | 17 +++++++++++++++++ > drivers/usb/host/xhci.h | 1 + > 2 files changed, 18 insertions(+) > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > index 65858f607437..f237af9d6e2e 100644 > --- a/drivers/usb/host/xhci.c > +++ b/drivers/usb/host/xhci.c > @@ -4358,6 +4358,20 @@ static int xhci_enable_device(struct usb_hcd *hcd, struct usb_device *udev) > return xhci_setup_device(hcd, udev, SETUP_CONTEXT_ONLY); > } > > +/* > + * The function could get the status of port initialization. > + */ > +static int xhci_check_init_status(struct usb_hcd *hcd, struct usb_device *udev, int r) > +{ > + /* > + * In general, this function is not necessory. Some platforms may > + * need doing error handling when the port initialization takes a > + * long time to do. The device can use the override callback to > + * do specific handlings. > + */ > + return 0; > +} For obvious technical and legal reasons, we are not allowed to add "hooks" to the kernel where there are no in-kernel users. Nor would you want us to do so. So I really do not understand this patch series at all. What driver wants to do odd things here? What needs to happen that the in-tree drivers are not doing properly? Why not get the needed fixes in the in-kernel drivers instead of trying to add random hooks that some out-of-tree code would use instead. confused, greg k-h ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] usb: xhci: add check_init_status hook support 2022-08-16 8:57 ` Greg KH @ 2022-08-22 15:42 ` Ray Chi 0 siblings, 0 replies; 6+ messages in thread From: Ray Chi @ 2022-08-22 15:42 UTC (permalink / raw) To: Greg KH Cc: mathias.nyman, stern, Badhri Jagan Sridharan, Albert Wang, linux-usb, linux-kernel On Tue, Aug 16, 2022 at 4:57 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > On Tue, Aug 16, 2022 at 04:38:54PM +0800, Ray Chi wrote: > > In general, xHCI didn't do anything for port initialization. However, > > there are some requirement or limitation on various platforms, so > > vendors need to do some error handlings if the device connected to a > > broken USB accessory. > > > > This patch also add the hook to xhci_driver_overrides so that vendors > > can add their specific protection easily if needed. > > > > Signed-off-by: Ray Chi <raychi@google.com> > > --- > > drivers/usb/host/xhci.c | 17 +++++++++++++++++ > > drivers/usb/host/xhci.h | 1 + > > 2 files changed, 18 insertions(+) > > > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > > index 65858f607437..f237af9d6e2e 100644 > > --- a/drivers/usb/host/xhci.c > > +++ b/drivers/usb/host/xhci.c > > @@ -4358,6 +4358,20 @@ static int xhci_enable_device(struct usb_hcd *hcd, struct usb_device *udev) > > return xhci_setup_device(hcd, udev, SETUP_CONTEXT_ONLY); > > } > > > > +/* > > + * The function could get the status of port initialization. > > + */ > > +static int xhci_check_init_status(struct usb_hcd *hcd, struct usb_device *udev, int r) > > +{ > > + /* > > + * In general, this function is not necessory. Some platforms may > > + * need doing error handling when the port initialization takes a > > + * long time to do. The device can use the override callback to > > + * do specific handlings. > > + */ > > + return 0; > > +} > > For obvious technical and legal reasons, we are not allowed to add > "hooks" to the kernel where there are no in-kernel users. Nor would you > want us to do so. > Agree on this. I am trying another way to achieve the same goal. > So I really do not understand this patch series at all. > > What driver wants to do odd things here? What needs to happen that the > in-tree drivers are not doing properly? Why not get the needed fixes in > the in-kernel drivers instead of trying to add random hooks that some > out-of-tree code would use instead. > > confused, > > greg k-h I will prepare a new commit to do it. Thanks, Ray ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-08-22 15:43 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-08-16 8:38 [PATCH 0/2] Provide a hook to check port init status Ray Chi 2022-08-16 8:38 ` [PATCH 1/2] usb: core: add " Ray Chi 2022-08-16 13:53 ` Alan Stern 2022-08-16 8:38 ` [PATCH 2/2] usb: xhci: add check_init_status hook support Ray Chi 2022-08-16 8:57 ` Greg KH 2022-08-22 15:42 ` Ray Chi
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.