lkml.org 
[lkml]   [2010]   [Dec]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH V2 2/2] MSP onchip root hub over current quirk.
    On Wed, 22 Dec 2010, Anoop P.A wrote:

    > From: Anoop P A <anoop.pa@gmail.com>
    >
    > Adding chip specific code under quirk.

    NAK. See below.

    > Signed-off-by: Anoop P A <anoop.pa@gmail.com>
    > ---
    > drivers/usb/core/hub.c | 45 ++++++++++++++++++++++++++++++++++++++-----
    > drivers/usb/core/quirks.c | 3 ++
    > include/linux/usb/quirks.h | 3 ++
    > 3 files changed, 45 insertions(+), 6 deletions(-)
    >
    > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
    > index 27115b4..4bff994 100644
    > --- a/drivers/usb/core/hub.c
    > +++ b/drivers/usb/core/hub.c
    > @@ -3377,12 +3377,45 @@ static void hub_events(void)
    > }
    >
    > if (portchange & USB_PORT_STAT_C_OVERCURRENT) {
    > - dev_err (hub_dev,
    > - "over-current change on port %d\n",
    > - i);
    > - clear_port_feature(hdev, i,
    > - USB_PORT_FEAT_C_OVER_CURRENT);
    > - hub_power_on(hub, true);
    > + usb_detect_quirks(hdev);

    This line is wrong. usb_detect_quirks() gets called only once per
    device, when the device is initialized. Besides, you probably want to
    use a hub-specific flag for this rather than a device-specific flag.

    > + if (hdev->quirks & USB_QUIRK_MSP_OVERCURRENT) {

    Also, it would be better to put this code in a separate subroutine
    instead of indenting it so far.

    > + /* clear OCC bit */
    > + clear_port_feature(hdev, i,
    > + USB_PORT_FEAT_C_OVER_CURRENT);
    > +
    > + /* This step is required to toggle the
    > + * PP bit to 0 and 1 (by hub_power_on)
    > + * in order the CSC bit to be
    > + * transitioned properly for device
    > + * hotplug
    > + */
    > + /* clear PP bit */
    > + clear_port_feature(hdev, i,
    > + USB_PORT_FEAT_POWER);
    > +
    > + /* resume power */
    > + hub_power_on(hub, true);
    > +
    > + /* delay 100 usec */
    > + udelay(100);
    > +
    > + /* read OCA bit */
    > + if (portstatus &
    > + (1<<USB_PORT_FEAT_OVER_CURRENT)) {
    > + /* declare overcurrent */
    > + dev_err(hub_dev,
    > + "over-current change \
    > + on port %d\n", i);
    > + }
    > + } else {
    > + dev_err(hub_dev,
    > + "over-current change \
    > + on port %d\n", i);
    > + clear_port_feature(hdev, i,
    > + USB_PORT_FEAT_C_OVER_CURRENT);
    > + hub_power_on(hub, true);
    > + }
    > +
    > }
    >
    > if (portchange & USB_PORT_STAT_C_RESET) {
    > diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
    > index 25719da..59843b9 100644
    > --- a/drivers/usb/core/quirks.c
    > +++ b/drivers/usb/core/quirks.c
    > @@ -88,6 +88,9 @@ static const struct usb_device_id usb_quirk_list[] = {
    > /* INTEL VALUE SSD */
    > { USB_DEVICE(0x8086, 0xf1a5), .driver_info = USB_QUIRK_RESET_RESUME },
    >
    > + /* PMC MSP over current quirk */
    > + { USB_DEVICE(0x1d6b, 0x0002), .driver_info = USB_QUIRK_MSP_OVERCURRENT },
    > +

    This implementation is completely wrong. It applies to all USB-2.0
    root hubs in Linux, not just the PMC MSP.

    > { } /* terminating entry must be last */
    > };
    >
    > diff --git a/include/linux/usb/quirks.h b/include/linux/usb/quirks.h
    > index 3e93de7..97ab168 100644
    > --- a/include/linux/usb/quirks.h
    > +++ b/include/linux/usb/quirks.h
    > @@ -30,4 +30,7 @@
    > descriptor */
    > #define USB_QUIRK_DELAY_INIT 0x00000040
    >
    > +/*MSP SoC onchip EHCI overcurrent issue */
    > +#define USB_QUIRK_MSP_OVERCURRENT 0x00000080
    > +
    > #endif /* __LINUX_USB_QUIRKS_H */
    >



    \
     
     \ /
      Last update: 2010-12-23 03:21    [W:0.027 / U:32.440 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site