linux-mips
[Top] [All Lists]

Re: [linux-usb-devel] [PATCH 11/12] drivers: PMC MSP71xx USB driver

To: Marc St-Jean <stjeanma@pmc-sierra.com>
Subject: Re: [linux-usb-devel] [PATCH 11/12] drivers: PMC MSP71xx USB driver
From: Alan Stern <stern@rowland.harvard.edu>
Date: Wed, 6 Jun 2007 10:42:23 -0400 (EDT)
Cc: gregkh@suse.de, <linux-mips@linux-mips.org>, <akpm@linux-foundation.org>, <dbrownell@users.sourceforge.net>, <linux-usb-devel@lists.sourceforge.net>
In-reply-to: <200706060048.l560moU4007288@pasqua.pmc-sierra.bc.ca>
Original-recipient: rfc822;linux-mips@linux-mips.org
Sender: linux-mips-bounce@linux-mips.org
On Tue, 5 Jun 2007, Marc St-Jean wrote:

> @@ -2749,12 +2749,33 @@ 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 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);
> +
> +                             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);
> +                             }
>                       }

Quite apart from all the issues David mentioned, you shouldn't change 
the way errors are reported.  The dev_err() statement should always be 
executed when there is an overcurrent change; it shouldn't depend on 
whether the overcurrent feature is set at the moment.

Remember, the message reports an overcurrent _change_, not an 
overcurrent _state_.

Alan Stern


<Prev in Thread] Current Thread [Next in Thread>