| Message ID | 20190417190316.10032-1-rrangel@chromium.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3] usb/hcd: Send a uevent signaling that the host controller had died | expand |
On Wed, 17 Apr 2019, Raul E Rangel wrote: > +/* Workqueue routine for when the root-hub has died. */ > +static void hcd_died_work(struct work_struct *work) > +{ > + struct usb_hcd *hcd = container_of(work, struct usb_hcd, died_work); > + char *env[] = { > + "ERROR=DEAD", > + NULL > + }; This can now be static const char *env[] = ... right? There's no need for the array to be reinitialized every time the routine runs. Alan Stern
On Wed, Apr 17, 2019 at 03:14:14PM -0400, Alan Stern wrote: > On Wed, 17 Apr 2019, Raul E Rangel wrote: > > > +/* Workqueue routine for when the root-hub has died. */ > > +static void hcd_died_work(struct work_struct *work) > > +{ > > + struct usb_hcd *hcd = container_of(work, struct usb_hcd, died_work); > > + char *env[] = { > > + "ERROR=DEAD", > > + NULL > > + }; > > This can now be > > static const char *env[] = ... > > right? There's no need for the array to be reinitialized every time > the routine runs. I originally tried to make it const, but kobject_uevent_env doesn't declare the parameter as const, so the compiler yelled at me. I could make it static, but a static without a const makes me wary. I can add it if you think it's fine. > > Alan Stern >
On Wed, 17 Apr 2019, Raul Rangel wrote: > On Wed, Apr 17, 2019 at 03:14:14PM -0400, Alan Stern wrote: > > On Wed, 17 Apr 2019, Raul E Rangel wrote: > > > > > +/* Workqueue routine for when the root-hub has died. */ > > > +static void hcd_died_work(struct work_struct *work) > > > +{ > > > + struct usb_hcd *hcd = container_of(work, struct usb_hcd, died_work); > > > + char *env[] = { > > > + "ERROR=DEAD", > > > + NULL > > > + }; > > > > This can now be > > > > static const char *env[] = ... > > > > right? There's no need for the array to be reinitialized every time > > the routine runs. > I originally tried to make it const, but kobject_uevent_env doesn't > declare the parameter as const, so the compiler yelled at me. I could > make it static, but a static without a const makes me wary. I can add it > if you think it's fine. This sounds like a golden opportunity! Submit a separate patch making the parameter to kobject_uevent_env be const (actually const char * const []), then submit this patch on top of that one. Alan Stern
On Wed, Apr 17, 2019 at 04:39:23PM -0400, Alan Stern wrote: > > This sounds like a golden opportunity! Submit a separate patch making > the parameter to kobject_uevent_env be const (actually const char * > const []), then submit this patch on top of that one. So there are other parts of the code base that dynamically create their array values. So by making the function take const, it breaks :( > > Alan Stern >
On Wed, Apr 17, 2019 at 3:11 PM Raul Rangel <rrangel@chromium.org> wrote: > > On Wed, Apr 17, 2019 at 04:39:23PM -0400, Alan Stern wrote: > > > > This sounds like a golden opportunity! Submit a separate patch making > > the parameter to kobject_uevent_env be const (actually const char * > > const []), then submit this patch on top of that one. > So there are other parts of the code base that dynamically create their > array values. So by making the function take const, it breaks :( Confused. The calling code can still be non-const. I don't see the parameter modified in kobject_uevent_env(), so declaring it const should be possible. Can you give an example of code that no longer works ? Thanks, Guenter > > > > Alan Stern > >
On Wed, Apr 17, 2019 at 03:23:52PM -0700, Guenter Roeck wrote: > On Wed, Apr 17, 2019 at 3:11 PM Raul Rangel <rrangel@chromium.org> wrote: > > > > On Wed, Apr 17, 2019 at 04:39:23PM -0400, Alan Stern wrote: > > > > > > This sounds like a golden opportunity! Submit a separate patch making > > > the parameter to kobject_uevent_env be const (actually const char * > > > const []), then submit this patch on top of that one. > > So there are other parts of the code base that dynamically create their > > array values. So by making the function take const, it breaks :( > > Confused. The calling code can still be non-const. I don't see the > parameter modified in kobject_uevent_env(), so declaring it const > should be possible. Can you give an example of code that no longer > works ? static int notify_user_space(struct thermal_zone_device *tz, int trip) { char *thermal_prop[5]; int i; mutex_lock(&tz->lock); thermal_prop[0] = kasprintf(GFP_KERNEL, "NAME=%s", tz->type); thermal_prop[1] = kasprintf(GFP_KERNEL, "TEMP=%d", tz->temperature); thermal_prop[2] = kasprintf(GFP_KERNEL, "TRIP=%d", trip); thermal_prop[3] = kasprintf(GFP_KERNEL, "EVENT=%d", tz->notify_event); thermal_prop[4] = NULL; kobject_uevent_env(&tz->device.kobj, KOBJ_CHANGE, thermal_prop); for (i = 0; i < 4; ++i) kfree(thermal_prop[i]); mutex_unlock(&tz->lock); return 0; } drivers/thermal/user_space.c:48:52: error: passing 'char *[5]' to parameter of type 'const char *const *' discards qualifiers in nested pointer types [-Werror,-Wincompatible-pointer-types-discards-qualifiers] kobject_uevent_env(&tz->device.kobj, KOBJ_CHANGE, thermal_prop); ^~~~~~~~~~~~ include/linux/kobject.h:238:22: note: passing argument to parameter 'envp' here const char *const envp[]); ^ http://c-faq.com/ansi/constmismatch.html explains why it fails. Raul > > Thanks, > Guenter > > > > > > > Alan Stern > > >
On Wed, Apr 17, 2019 at 3:41 PM Raul Rangel <rrangel@chromium.org> wrote: > > On Wed, Apr 17, 2019 at 03:23:52PM -0700, Guenter Roeck wrote: > > On Wed, Apr 17, 2019 at 3:11 PM Raul Rangel <rrangel@chromium.org> wrote: > > > > > > On Wed, Apr 17, 2019 at 04:39:23PM -0400, Alan Stern wrote: > > > > > > > > This sounds like a golden opportunity! Submit a separate patch making > > > > the parameter to kobject_uevent_env be const (actually const char * > > > > const []), then submit this patch on top of that one. > > > So there are other parts of the code base that dynamically create their > > > array values. So by making the function take const, it breaks :( > > > > Confused. The calling code can still be non-const. I don't see the > > parameter modified in kobject_uevent_env(), so declaring it const > > should be possible. Can you give an example of code that no longer > > works ? > static int notify_user_space(struct thermal_zone_device *tz, int trip) > { > char *thermal_prop[5]; > int i; > > mutex_lock(&tz->lock); > thermal_prop[0] = kasprintf(GFP_KERNEL, "NAME=%s", tz->type); > thermal_prop[1] = kasprintf(GFP_KERNEL, "TEMP=%d", tz->temperature); > thermal_prop[2] = kasprintf(GFP_KERNEL, "TRIP=%d", trip); > thermal_prop[3] = kasprintf(GFP_KERNEL, "EVENT=%d", tz->notify_event); > thermal_prop[4] = NULL; > kobject_uevent_env(&tz->device.kobj, KOBJ_CHANGE, thermal_prop); > for (i = 0; i < 4; ++i) > kfree(thermal_prop[i]); > mutex_unlock(&tz->lock); > return 0; > } > > drivers/thermal/user_space.c:48:52: error: passing 'char *[5]' to parameter of type 'const char *const *' discards qualifiers in nested pointer types [-Werror,-Wincompatible-pointer-types-discards-qualifiers] > kobject_uevent_env(&tz->device.kobj, KOBJ_CHANGE, thermal_prop); > ^~~~~~~~~~~~ > include/linux/kobject.h:238:22: note: passing argument to parameter 'envp' here > const char *const envp[]); > ^ > > http://c-faq.com/ansi/constmismatch.html explains why it fails. > Interesting. One never stops learning. So the best you could do would be char * const envp[], but I guess that doesn't help much. Guenter > Raul > > > > > Thanks, > > Guenter > > > > > > > > > > Alan Stern > > > >
On Wed, Apr 17, 2019 at 04:20:09PM -0700, Guenter Roeck wrote: > On Wed, Apr 17, 2019 at 3:41 PM Raul Rangel <rrangel@chromium.org> wrote: > > > > On Wed, Apr 17, 2019 at 03:23:52PM -0700, Guenter Roeck wrote: > > > On Wed, Apr 17, 2019 at 3:11 PM Raul Rangel <rrangel@chromium.org> wrote: > > > > > > > > On Wed, Apr 17, 2019 at 04:39:23PM -0400, Alan Stern wrote: > > > > > > > > > > This sounds like a golden opportunity! Submit a separate patch making > > > > > the parameter to kobject_uevent_env be const (actually const char * > > > > > const []), then submit this patch on top of that one. > > > > So there are other parts of the code base that dynamically create their > > > > array values. So by making the function take const, it breaks :( > > > > > > Confused. The calling code can still be non-const. I don't see the > > > parameter modified in kobject_uevent_env(), so declaring it const > > > should be possible. Can you give an example of code that no longer > > > works ? > > static int notify_user_space(struct thermal_zone_device *tz, int trip) > > { > > char *thermal_prop[5]; > > int i; > > > > mutex_lock(&tz->lock); > > thermal_prop[0] = kasprintf(GFP_KERNEL, "NAME=%s", tz->type); > > thermal_prop[1] = kasprintf(GFP_KERNEL, "TEMP=%d", tz->temperature); > > thermal_prop[2] = kasprintf(GFP_KERNEL, "TRIP=%d", trip); > > thermal_prop[3] = kasprintf(GFP_KERNEL, "EVENT=%d", tz->notify_event); > > thermal_prop[4] = NULL; > > kobject_uevent_env(&tz->device.kobj, KOBJ_CHANGE, thermal_prop); > > for (i = 0; i < 4; ++i) > > kfree(thermal_prop[i]); > > mutex_unlock(&tz->lock); > > return 0; > > } > > > > drivers/thermal/user_space.c:48:52: error: passing 'char *[5]' to parameter of type 'const char *const *' discards qualifiers in nested pointer types [-Werror,-Wincompatible-pointer-types-discards-qualifiers] > > kobject_uevent_env(&tz->device.kobj, KOBJ_CHANGE, thermal_prop); > > ^~~~~~~~~~~~ > > include/linux/kobject.h:238:22: note: passing argument to parameter 'envp' here > > const char *const envp[]); > > ^ > > > > http://c-faq.com/ansi/constmismatch.html explains why it fails. > > > Interesting. One never stops learning. So the best you could do would > be char * const envp[], but I guess that doesn't help much. Yeah, I went down this path a year or so ago and had to give it up as well :(
On Thu, 18 Apr 2019, Greg Kroah-Hartman wrote: > On Wed, Apr 17, 2019 at 04:20:09PM -0700, Guenter Roeck wrote: > > On Wed, Apr 17, 2019 at 3:41 PM Raul Rangel <rrangel@chromium.org> wrote: > > > > > > On Wed, Apr 17, 2019 at 03:23:52PM -0700, Guenter Roeck wrote: > > > > On Wed, Apr 17, 2019 at 3:11 PM Raul Rangel <rrangel@chromium.org> wrote: > > > > > > > > > > On Wed, Apr 17, 2019 at 04:39:23PM -0400, Alan Stern wrote: > > > > > > > > > > > > This sounds like a golden opportunity! Submit a separate patch making > > > > > > the parameter to kobject_uevent_env be const (actually const char * > > > > > > const []), then submit this patch on top of that one. > > > > > So there are other parts of the code base that dynamically create their > > > > > array values. So by making the function take const, it breaks :( > > > > > > > > Confused. The calling code can still be non-const. I don't see the > > > > parameter modified in kobject_uevent_env(), so declaring it const > > > > should be possible. Can you give an example of code that no longer > > > > works ? > > > static int notify_user_space(struct thermal_zone_device *tz, int trip) > > > { > > > char *thermal_prop[5]; > > > int i; > > > > > > mutex_lock(&tz->lock); > > > thermal_prop[0] = kasprintf(GFP_KERNEL, "NAME=%s", tz->type); > > > thermal_prop[1] = kasprintf(GFP_KERNEL, "TEMP=%d", tz->temperature); > > > thermal_prop[2] = kasprintf(GFP_KERNEL, "TRIP=%d", trip); > > > thermal_prop[3] = kasprintf(GFP_KERNEL, "EVENT=%d", tz->notify_event); > > > thermal_prop[4] = NULL; > > > kobject_uevent_env(&tz->device.kobj, KOBJ_CHANGE, thermal_prop); > > > for (i = 0; i < 4; ++i) > > > kfree(thermal_prop[i]); > > > mutex_unlock(&tz->lock); > > > return 0; > > > } > > > > > > drivers/thermal/user_space.c:48:52: error: passing 'char *[5]' to parameter of type 'const char *const *' discards qualifiers in nested pointer types [-Werror,-Wincompatible-pointer-types-discards-qualifiers] > > > kobject_uevent_env(&tz->device.kobj, KOBJ_CHANGE, thermal_prop); > > > ^~~~~~~~~~~~ > > > include/linux/kobject.h:238:22: note: passing argument to parameter 'envp' here > > > const char *const envp[]); > > > ^ > > > > > > http://c-faq.com/ansi/constmismatch.html explains why it fails. > > > > > Interesting. One never stops learning. So the best you could do would > > be char * const envp[], but I guess that doesn't help much. > > Yeah, I went down this path a year or so ago and had to give it up as > well :( Well, the signature could still be changed as Guenter suggests. And the array being added in the new code could still be static. After all, there isn't really any danger that the contents of those strings will be modified, right? It's just that the const modifiers weren't put in until it was too late and there were too many existing callers. Perhaps a comment about this could be included in the kerneldoc for kobject_uevent_env. Alan Stern
On Thu, Apr 18, 2019 at 10:21:32AM -0400, Alan Stern wrote: > On Thu, 18 Apr 2019, Greg Kroah-Hartman wrote: > > > On Wed, Apr 17, 2019 at 04:20:09PM -0700, Guenter Roeck wrote: > > > On Wed, Apr 17, 2019 at 3:41 PM Raul Rangel <rrangel@chromium.org> wrote: > > > > > > > > On Wed, Apr 17, 2019 at 03:23:52PM -0700, Guenter Roeck wrote: > > > > > On Wed, Apr 17, 2019 at 3:11 PM Raul Rangel <rrangel@chromium.org> wrote: > > > > > > > > > > > > On Wed, Apr 17, 2019 at 04:39:23PM -0400, Alan Stern wrote: > > > > > > > > > > > > > > This sounds like a golden opportunity! Submit a separate patch making > > > > > > > the parameter to kobject_uevent_env be const (actually const char * > > > > > > > const []), then submit this patch on top of that one. > > > > > > So there are other parts of the code base that dynamically create their > > > > > > array values. So by making the function take const, it breaks :( > > > > > > > > > > Confused. The calling code can still be non-const. I don't see the > > > > > parameter modified in kobject_uevent_env(), so declaring it const > > > > > should be possible. Can you give an example of code that no longer > > > > > works ? > > > > static int notify_user_space(struct thermal_zone_device *tz, int trip) > > > > { > > > > char *thermal_prop[5]; > > > > int i; > > > > > > > > mutex_lock(&tz->lock); > > > > thermal_prop[0] = kasprintf(GFP_KERNEL, "NAME=%s", tz->type); > > > > thermal_prop[1] = kasprintf(GFP_KERNEL, "TEMP=%d", tz->temperature); > > > > thermal_prop[2] = kasprintf(GFP_KERNEL, "TRIP=%d", trip); > > > > thermal_prop[3] = kasprintf(GFP_KERNEL, "EVENT=%d", tz->notify_event); > > > > thermal_prop[4] = NULL; > > > > kobject_uevent_env(&tz->device.kobj, KOBJ_CHANGE, thermal_prop); > > > > for (i = 0; i < 4; ++i) > > > > kfree(thermal_prop[i]); > > > > mutex_unlock(&tz->lock); > > > > return 0; > > > > } > > > > > > > > drivers/thermal/user_space.c:48:52: error: passing 'char *[5]' to parameter of type 'const char *const *' discards qualifiers in nested pointer types [-Werror,-Wincompatible-pointer-types-discards-qualifiers] > > > > kobject_uevent_env(&tz->device.kobj, KOBJ_CHANGE, thermal_prop); > > > > ^~~~~~~~~~~~ > > > > include/linux/kobject.h:238:22: note: passing argument to parameter 'envp' here > > > > const char *const envp[]); > > > > ^ > > > > > > > > http://c-faq.com/ansi/constmismatch.html explains why it fails. > > > > > > > Interesting. One never stops learning. So the best you could do would > > > be char * const envp[], but I guess that doesn't help much. > > > > Yeah, I went down this path a year or so ago and had to give it up as > > well :( > > Well, the signature could still be changed as Guenter suggests. > > And the array being added in the new code could still be static. > After all, there isn't really any danger that the contents of those > strings will be modified, right? It's just that the const modifiers > weren't put in until it was too late and there were too many existing > callers. Perhaps a comment about this could be included in the > kerneldoc for kobject_uevent_env. I am all for changing this, but I remember I tried to, and somehow failed, but I don't remember the full details sorry, it was a while ago. If someone figures out how to make this all const, I will gladly take that patch. thanks, greg k-h
On Thu, Apr 18, 2019 at 04:30:48PM +0200, Greg Kroah-Hartman wrote: > On Thu, Apr 18, 2019 at 10:21:32AM -0400, Alan Stern wrote: > > On Thu, 18 Apr 2019, Greg Kroah-Hartman wrote: > > > > > On Wed, Apr 17, 2019 at 04:20:09PM -0700, Guenter Roeck wrote: > > > > On Wed, Apr 17, 2019 at 3:41 PM Raul Rangel <rrangel@chromium.org> wrote: > > > > > > > > > > On Wed, Apr 17, 2019 at 03:23:52PM -0700, Guenter Roeck wrote: > > > > > > On Wed, Apr 17, 2019 at 3:11 PM Raul Rangel <rrangel@chromium.org> wrote: > > > > > > > > > > > > > > On Wed, Apr 17, 2019 at 04:39:23PM -0400, Alan Stern wrote: > > > > > > > > > > > > > > > > This sounds like a golden opportunity! Submit a separate patch making > > > > > > > > the parameter to kobject_uevent_env be const (actually const char * > > > > > > > > const []), then submit this patch on top of that one. > > > > > > > So there are other parts of the code base that dynamically create their > > > > > > > array values. So by making the function take const, it breaks :( > > > > > > > > > > > > Confused. The calling code can still be non-const. I don't see the > > > > > > parameter modified in kobject_uevent_env(), so declaring it const > > > > > > should be possible. Can you give an example of code that no longer > > > > > > works ? > > > > > static int notify_user_space(struct thermal_zone_device *tz, int trip) > > > > > { > > > > > char *thermal_prop[5]; > > > > > int i; > > > > > > > > > > mutex_lock(&tz->lock); > > > > > thermal_prop[0] = kasprintf(GFP_KERNEL, "NAME=%s", tz->type); > > > > > thermal_prop[1] = kasprintf(GFP_KERNEL, "TEMP=%d", tz->temperature); > > > > > thermal_prop[2] = kasprintf(GFP_KERNEL, "TRIP=%d", trip); > > > > > thermal_prop[3] = kasprintf(GFP_KERNEL, "EVENT=%d", tz->notify_event); > > > > > thermal_prop[4] = NULL; > > > > > kobject_uevent_env(&tz->device.kobj, KOBJ_CHANGE, thermal_prop); > > > > > for (i = 0; i < 4; ++i) > > > > > kfree(thermal_prop[i]); > > > > > mutex_unlock(&tz->lock); > > > > > return 0; > > > > > } > > > > > > > > > > drivers/thermal/user_space.c:48:52: error: passing 'char *[5]' to parameter of type 'const char *const *' discards qualifiers in nested pointer types [-Werror,-Wincompatible-pointer-types-discards-qualifiers] > > > > > kobject_uevent_env(&tz->device.kobj, KOBJ_CHANGE, thermal_prop); > > > > > ^~~~~~~~~~~~ > > > > > include/linux/kobject.h:238:22: note: passing argument to parameter 'envp' here > > > > > const char *const envp[]); > > > > > ^ > > > > > > > > > > http://c-faq.com/ansi/constmismatch.html explains why it fails. > > > > > > > > > Interesting. One never stops learning. So the best you could do would > > > > be char * const envp[], but I guess that doesn't help much. > > > > > > Yeah, I went down this path a year or so ago and had to give it up as > > > well :( > > > > Well, the signature could still be changed as Guenter suggests. > > > > And the array being added in the new code could still be static. > > After all, there isn't really any danger that the contents of those > > strings will be modified, right? It's just that the const modifiers > > weren't put in until it was too late and there were too many existing > > callers. Perhaps a comment about this could be included in the > > kerneldoc for kobject_uevent_env. > > I am all for changing this, but I remember I tried to, and somehow > failed, but I don't remember the full details sorry, it was a while ago. > If someone figures out how to make this all const, I will gladly take > that patch. > Well we could use varargs... int kobject_uevent_env(struct kobject *kobj, enum kobject_action action, ...); This will accept both const char* and char *. Example https://repl.it/@RaulRangel/Const-char-var-args It seems like most callers have a fixed number of env params, so you wouldn't need a function that takes a list. > thanks, > > greg k-h
On Wed, Apr 17, 2019 at 01:03:16PM -0600, Raul E Rangel wrote: > This change will send an OFFLINE event to udev with the ERROR=DEAD > environment variable set when the HC dies. > > By notifying user space the appropriate policies can be applied. > i.e., > * Collect error logs. > * Notify the user that USB is no longer functional. > * Perform a graceful reboot. > > Signed-off-by: Raul E Rangel <rrangel@chromium.org> I'll wait for a v4 to fix up the bugs 0-day found in this before reviewing :) thanks, greg k-h
diff --git a/Documentation/ABI/testing/usb-uevent b/Documentation/ABI/testing/usb-uevent new file mode 100644 index 000000000000..d35c3cad892c --- /dev/null +++ b/Documentation/ABI/testing/usb-uevent @@ -0,0 +1,27 @@ +What: Raise a uevent when a USB Host Controller has died +Date: 2019-04-17 +KernelVersion: 5.2 +Contact: linux-usb@vger.kernel.org +Description: When the USB Host Controller has entered a state where it is no + longer functional a uevent will be raised. The uevent will + contain ACTION=offline and ERROR=DEAD. + + Here is an example taken using udevadm monitor -p: + + KERNEL[130.428945] offline /devices/pci0000:00/0000:00:10.0/usb2 (usb) + ACTION=offline + BUSNUM=002 + DEVNAME=/dev/bus/usb/002/001 + DEVNUM=001 + DEVPATH=/devices/pci0000:00/0000:00:10.0/usb2 + DEVTYPE=usb_device + DRIVER=usb + ERROR=DEAD + MAJOR=189 + MINOR=128 + PRODUCT=1d6b/2/414 + SEQNUM=2168 + SUBSYSTEM=usb + TYPE=9/0/1 + +Users: chromium-os-dev@chromium.org diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 975d7c1288e3..145b058705fe 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -2343,6 +2343,20 @@ int hcd_bus_resume(struct usb_device *rhdev, pm_message_t msg) return status; } + +/* Workqueue routine for when the root-hub has died. */ +static void hcd_died_work(struct work_struct *work) +{ + struct usb_hcd *hcd = container_of(work, struct usb_hcd, died_work); + char *env[] = { + "ERROR=DEAD", + NULL + }; + + /* Notify user space that the host controller has died */ + kobject_uevent_env(&hcd->self.root_hub->dev.kobj, KOBJ_OFFLINE, env); +} + /* Workqueue routine for root-hub remote wakeup */ static void hcd_resume_work(struct work_struct *work) { @@ -2488,6 +2502,13 @@ void usb_hc_died (struct usb_hcd *hcd) usb_kick_hub_wq(hcd->self.root_hub); } } + + /* Handle the case where this function gets called with a shared HCD */ + if (usb_hcd_is_primary_hcd(hcd)) + schedule_work(&hcd->died_work); + else + schedule_work(&hcd->primary_hcd->died_work); + spin_unlock_irqrestore (&hcd_root_hub_lock, flags); /* Make sure that the other roothub is also deallocated. */ } @@ -2555,6 +2576,8 @@ struct usb_hcd *__usb_create_hcd(const struct hc_driver *driver, INIT_WORK(&hcd->wakeup_work, hcd_resume_work); #endif + INIT_WORK(&hcd->died_work, hcd_died_work); + hcd->driver = driver; hcd->speed = driver->flags & HCD_MASK; hcd->product_desc = (driver->product_desc) ? driver->product_desc : @@ -2908,6 +2931,7 @@ int usb_add_hcd(struct usb_hcd *hcd, #ifdef CONFIG_PM cancel_work_sync(&hcd->wakeup_work); #endif + cancel_work_sync(&hcd->died_work); mutex_lock(&usb_bus_idr_lock); usb_disconnect(&rhdev); /* Sets rhdev to NULL */ mutex_unlock(&usb_bus_idr_lock); @@ -2968,6 +2992,7 @@ void usb_remove_hcd(struct usb_hcd *hcd) #ifdef CONFIG_PM cancel_work_sync(&hcd->wakeup_work); #endif + cancel_work_sync(&hcd->died_work); mutex_lock(&usb_bus_idr_lock); usb_disconnect(&rhdev); /* Sets rhdev to NULL */ diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h index 695931b03684..66a24b13e2ab 100644 --- a/include/linux/usb/hcd.h +++ b/include/linux/usb/hcd.h @@ -98,6 +98,7 @@ struct usb_hcd { #ifdef CONFIG_PM struct work_struct wakeup_work; /* for remote wakeup */ #endif + struct work_struct died_work; /* for when the device dies */ /* * hardware info/state
This change will send an OFFLINE event to udev with the ERROR=DEAD environment variable set when the HC dies. By notifying user space the appropriate policies can be applied. i.e., * Collect error logs. * Notify the user that USB is no longer functional. * Perform a graceful reboot. Signed-off-by: Raul E Rangel <rrangel@chromium.org> --- I wasn't able to find any good examples of other drivers sending a dead notification. Use an EVENT= format https://github.com/torvalds/linux/blob/master/drivers/acpi/dock.c#L302 https://github.com/torvalds/linux/blob/master/drivers/net/wireless/ath/wil6210/interrupt.c#L497 Uses SDEV_MEDIA_CHANGE= https://github.com/torvalds/linux/blob/master/drivers/scsi/scsi_lib.c#L2318 Uses ERROR=1. https://chromium.googlesource.com/chromiumos/third_party/kernel/+/7f6d8aec5803aac44192f03dce5637b66cda7abf/drivers/input/touchscreen/atmel_mxt_ts.c#1581 I'm not a fan because it doesn't signal what the error was. Changes in v3: - Added documentation - Removed use of lock and null check - Changed event to OFFLINE + ERROR=DEAD Changes in v2: - Check that the root hub still exists before sending the uevent. - Ensure died_work has completed before deallocating. Documentation/ABI/testing/usb-uevent | 27 +++++++++++++++++++++++++++ drivers/usb/core/hcd.c | 25 +++++++++++++++++++++++++ include/linux/usb/hcd.h | 1 + 3 files changed, 53 insertions(+) create mode 100644 Documentation/ABI/testing/usb-uevent