On Mon, Sep 10, 2018 at 11:14 AM Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Fri, Aug 31, 2018 at 10:14:19AM -0700, Jon Flatley wrote:
> > After commit 1cbd53c8cd85 ("usb: core: introduce per-port over-current
> > counters") usb ports expose a sysfs value 'over_current_count'
> > to user space. This value on its own is not very useful as it requires
> > manual polling.
> >
> > As a solution, fire a udev event from the usb hub device that specifies
> > the values 'OVER_CURRENT_PORT' and 'OVER_CURRENT_COUNT' that indicate
> > the path of the usb port where the over-current event occurred and the
> > value of 'over_current_count' in sysfs. Additionally, call
> > sysfs_notify() so the sysfs value supports poll().
> >
> > Signed-off-by: Jon Flatley <jflat@xxxxxxxxxxxx>
> > ---
> > drivers/usb/core/hub.c | 36 ++++++++++++++++++++++++++++++++++++
> > 1 file changed, 36 insertions(+)
> >
> > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > index 65feedd69323..c986b0fc2daa 100644
> > --- a/drivers/usb/core/hub.c
> > +++ b/drivers/usb/core/hub.c
> > @@ -27,6 +27,7 @@
> > #include <linux/random.h>
> > #include <linux/pm_qos.h>
> > #include <linux/pm_dark_resume.h>
> > +#include <linux/kobject.h>
> >
> > #include <asm/uaccess.h>
> > #include <asm/byteorder.h>
> > @@ -5096,6 +5097,40 @@ static void hub_port_connect_change(struct usb_hub *hub, int port1,
> > usb_lock_port(port_dev);
> > }
> >
> > +/* Handle notifying userspace about hub over-current events */
> > +static void port_over_current_notify(struct usb_port *port_dev)
> > +{
> > + static char *envp[] = { NULL, NULL, NULL };
> > + struct device *hub_dev;
> > + char *port_dev_path;
> > +
> > + sysfs_notify(&port_dev->dev.kobj, NULL, "over_current_count");
> > +
> > + hub_dev = port_dev->dev.parent;
> > +
> > + if (!hub_dev)
> > + return;
> > +
> > + port_dev_path = kobject_get_path(&port_dev->dev.kobj, GFP_KERNEL);
> > + if (!port_dev_path)
> > + return;
> > +
> > + envp[0] = kasprintf(GFP_KERNEL, "OVER_CURRENT_PORT=%s", port_dev_path);
> > + if (!envp[0])
> > + return;
> > +
> > + envp[1] = kasprintf(GFP_KERNEL, "OVER_CURRENT_COUNT=%u",
> > + port_dev->over_current_count);
> > + if (!envp[1])
> > + goto exit;
> > +
> > + kobject_uevent_env(&hub_dev->kobj, KOBJ_CHANGE, envp);
> > +
> > + kfree(envp[1]);
> > +exit:
> > + kfree(envp[0]);
> > +}
> > +
> > static void port_event(struct usb_hub *hub, int port1)
> > __must_hold(&port_dev->status_lock)
> > {
> > @@ -5138,6 +5173,7 @@ static void port_event(struct usb_hub *hub, int port1)
> > if (portchange & USB_PORT_STAT_C_OVERCURRENT) {
> > u16 status = 0, unused;
> > port_dev->over_current_count++;
> > + port_over_current_notify(port_dev);
>
> When an overcurrent "event" happens, does it just stay overloaded for
> long periods of time? Would this flood the system with lots of kobject
> messages, or is this rate-limited somehow?
>
> Also, as this is a new user/kernel abi you are creating, it needs to be
> documented somewhere. Perhaps in the sysfs file information for this
> attribute?
>
> thanks,
>
> greg k-h
The kobject message is only sent when the USB_PORT_STAT_C_OVERCURRENT bit
is set. This happens when the port enters or leaves in over-current
condition or when the port is powered off due to an over-current condition
on another port. This bit is cleared after being processed so typically
only two messages per over-current event should be expected.
Are you referring to documenting this udev event alongside the sysfs
documentation for this attribute? Is this typically where this type of
documentation goes?
Thanks,
Jon
