On Thu, Feb 14, 2002 at 11:18:57AM +0100, Martin Diehl wrote:
>
> +static int usb_hub_port_status(struct usb_device *hub, int port,
> + unsigned short *status, unsigned short *change)
status and change should both be u16 types, right?
> +{
> + struct usb_port_status *portsts;
> + int ret = -1;
Should be:
int ret = -ENOMEM;
> +
> + portsts = kmalloc(sizeof(*portsts), GFP_KERNEL);
> + if (portsts) {
> + ret = usb_get_port_status(hub, port + 1, portsts);
> + if (ret < 0)
> + err("get_port_status(%d) failed (err = %d)", port + 1, ret);
> + else {
> + *status = le16_to_cpu(portsts->wPortStatus);
> + *change = le16_to_cpu(portsts->wPortChange);
> + dbg("port %d, portstatus %x, change %x, %s", port + 1,
> + *status, *change, portspeed(*status));
> + ret = 0;
> + }
> + kfree(portsts);
> + }
> + return ret;
> +}
> +
> #define HUB_RESET_TRIES 5
> #define HUB_PROBE_TRIES 2
> #define HUB_SHORT_RESET_TIME 10
> @@ -507,7 +530,6 @@
> struct usb_device *dev, unsigned int delay)
> {
> int delay_time, ret;
> - struct usb_port_status portsts;
> unsigned short portchange, portstatus;
>
> for (delay_time = 0; delay_time < HUB_RESET_TIMEOUT; delay_time += delay) {
> @@ -515,17 +537,11 @@
> wait_ms(delay);
>
> /* read and decode port status */
> - ret = usb_get_port_status(hub, port + 1, &portsts);
> + ret = usb_hub_port_status(hub, port, &portstatus, &portchange);
> if (ret < 0) {
> - err("get_port_status(%d) failed (err = %d)", port + 1, ret);
> return -1;
> }
>
> - portstatus = le16_to_cpu(portsts.wPortStatus);
> - portchange = le16_to_cpu(portsts.wPortChange);
> - dbg("port %d, portstatus %x, change %x, %s", port + 1,
> - portstatus, portchange, portspeed (portstatus));
> -
> /* bomb out completely if something weird happened */
> if ((portchange & USB_PORT_STAT_C_CONNECTION))
> return -1;
> @@ -592,17 +608,43 @@
> port + 1, hub->devnum, ret);
> }
>
> +#define HUB_DEBOUNCE_TIMEOUT 200 /* 7.1.7.1: min 100ms - restart if disconnect
>*/
> +#define HUB_DEBOUNCE_STEP 10 /* interval when to check for spurious
>disconnect */
> +
> +/* return: -1 on error, 0 on success, 1 on disconnect. */
As no one is checking for the return value of 1, and a disconnect is
still an error, shouldn't the code be the following:
static int usb_hub_port_debounce(struct usb_device *hub, int port)
{
int delay_time;
int ret;
u16 portchange;
u16 portstatus;
for (delay_time = 0; delay_time < HUB_DEBOUNCE_TIMEOUT; /* empty */ ) {
/* wait for debounce and for device getting stable buspower */
wait_ms(HUB_DEBOUNCE_STEP);
ret = usb_hub_port_status(hub, port, &portstatus, &portchange);
if (ret)
return ret;
if ((portchange & USB_PORT_STAT_C_CONNECTION)) {
usb_clear_port_feature(hub, port+1,
USB_PORT_FEAT_C_CONNECTION);
delay_time = 0;
}
else
delay_time += HUB_DEBOUNCE_STEP;
}
return ((portstatus&USB_PORT_STAT_CONNECTION)) ? 0 : -ENODEV;
}
And what's the odds that we can get stuck in this loop for a very noisy
connection?
thanks,
greg k-h
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
