On Fri, Sep 06, 2013 at 11:32:51PM +0800, Greg Kroah-Hartman wrote:
> On Fri, Sep 06, 2013 at 06:24:41PM +0800, Huang Rui wrote:
> > It adds all issue mice on AMD special platforms for remote wakeup quirk.
> >
> > Some mice with on Pixart controller would trigger remote wakeup quirk on
> > some AMD special platforms, this patch added all issue mice which tested.
> >
> > But some manufactures might overwrite vendor id and product id in the
> > firmware when they get their USB hardware from other suppliers. So it might
> > not cover all the issue devices, and if find another issue device in
> > future, will add to mark as USB_QUIRK_AMD_REMOTE_WAKEUP.
>
> Who should we email about these problems, so that they can be tested,
> you?
>
Probably yes at present, because these platforms doesn't release yet.
But I will list the platform name as enum type in pci-quirks at v4 to
make you clear and some one can verify this issue in future.
> > Signed-off-by: Huang Rui <ray.huang@xxxxxxx>
> > ---
> > drivers/usb/core/hub.c | 14 ++++++++++++++
> > drivers/usb/core/quirks.c | 11 +++++++++++
> > include/linux/usb.h | 1 +
> > include/linux/usb/quirks.h | 4 ++++
> > 4 files changed, 30 insertions(+)
> >
> > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > index dde4c83..d8fb0ad 100644
> > --- a/drivers/usb/core/hub.c
> > +++ b/drivers/usb/core/hub.c
> > @@ -5461,3 +5461,17 @@ acpi_handle usb_get_hub_port_acpi_handle(struct usb_device *hdev,
> > return DEVICE_ACPI_HANDLE(&hub->ports[port1 - 1]->dev);
> > }
> > #endif
> > +
> > +bool is_issue_device_for_amd_quirk(struct usb_device *udev)
> > +{
> > + if (!udev) {
> > + dev_warn(&udev->dev, "Warn: no device attached!\n");
> > + return false;
> > + }
> > +
> > + if (udev->quirks & USB_QUIRK_AMD_REMOTE_WAKEUP)
> > + return true;
> > + else
> > + return false;
> > +}
> > +EXPORT_SYMBOL_GPL(is_issue_device_for_amd_quirk);
>
> That's a really ackward name for a global symbol. Please follow the
> naming scheme of the usb global symbols please (hint, put "usb_" at the
> front, and don't use a whole sentance for a function name.)
>
> Also, as no one is calling this function in this patch, why do you have
> it here?
>
> This patch really doesn't even do anything, does it?
>
> > diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
> > index 5b44cd4..243d277 100644
> > --- a/drivers/usb/core/quirks.c
> > +++ b/drivers/usb/core/quirks.c
> > @@ -144,6 +144,17 @@ static const struct usb_device_id usb_quirk_list[] = {
> > /* INTEL VALUE SSD */
> > { USB_DEVICE(0x8086, 0xf1a5), .driver_info = USB_QUIRK_RESET_RESUME },
> >
> > + /* Lenovo Mouse with PixArt controller */
> > + { USB_DEVICE(0x17ef, 0x602e), .driver_info = USB_QUIRK_AMD_REMOTE_WAKEUP },
> > +
> > + /* Pixart Mouse */
> > + { USB_DEVICE(0x093a, 0x2500), .driver_info = USB_QUIRK_AMD_REMOTE_WAKEUP },
> > + { USB_DEVICE(0x093a, 0x2510), .driver_info = USB_QUIRK_AMD_REMOTE_WAKEUP },
> > + { USB_DEVICE(0x093a, 0x2521), .driver_info = USB_QUIRK_AMD_REMOTE_WAKEUP },
> > +
> > + /* Logitech Optical Mouse M90/M100 */
> > + { USB_DEVICE(0x046d, 0xc05a), .driver_info = USB_QUIRK_AMD_REMOTE_WAKEUP },
> > +
> > { } /* terminating entry must be last */
> > };
> >
> > diff --git a/include/linux/usb.h b/include/linux/usb.h
> > index 001629c..09e0bd8 100644
> > --- a/include/linux/usb.h
> > +++ b/include/linux/usb.h
> > @@ -591,6 +591,7 @@ extern struct usb_device *usb_get_dev(struct usb_device *dev);
> > extern void usb_put_dev(struct usb_device *dev);
> > extern struct usb_device *usb_hub_find_child(struct usb_device *hdev,
> > int port1);
> > +extern bool is_issue_device_for_amd_quirk(struct usb_device *udev);
> >
> > /**
> > * usb_hub_for_each_child - iterate over all child devices on the hub
> > diff --git a/include/linux/usb/quirks.h b/include/linux/usb/quirks.h
> > index 52f944d..741f2a9 100644
> > --- a/include/linux/usb/quirks.h
> > +++ b/include/linux/usb/quirks.h
> > @@ -30,4 +30,8 @@
> > descriptor */
> > #define USB_QUIRK_DELAY_INIT 0x00000040
> >
> > +/* device needs reset during resume phase because of remote wakeup issue on
> > + * some special AMD platforms */
>
> Can we get a listing of which AMD platforms have the problem here?
>
Yes, will do it in v4.
Thanks,
Rui
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
