On Tue, Jan 07, 2014 at 12:30:00PM -0800, Dan Williams wrote:
> The port pm_runtime implementation unconditionally clears FEAT_C_ENABLE
> after clearing PORT_POWER, but the bit is reserved on usb3 hub ports.
> 
> It also takes steps to re-disable the port if it fails to reconnect
> which again is broken on usb3 ports and unnecessary on usb2 ports.

The subject is misleading, since you don't just change when you clear
FEAT_C_ENABLE for USB 2.0 hubs; you also change it for USB 3.0 hubs.
Suggest:

"usb: Clear FEAT_C_ENABLE on port suspend for usb2, not usb3"

Actually, that's still not clear, because the patch really fixes two
distinct bugs:

1. Don't clear FEAT_C_ENABLE for USB 3.0 hubs at all.
2. Don't clear FEAT_C_ENABLE for USB 2.0 ports on failed resume.

That really suggests it should be two separate patches, in case there's
say, a bug in patch #2 and we really do want to clear FEAT_C_ENABLE on
USB 2.0 port resume.

Can you split this patch up?

Sarah Sharp

> Signed-off-by: Dan Williams <dan.j.willi...@intel.com>
> ---
>  drivers/usb/core/port.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
> index 217a3c6df29e..97e4939fee1a 100644
> --- a/drivers/usb/core/port.c
> +++ b/drivers/usb/core/port.c
> @@ -102,7 +102,6 @@ static int usb_port_runtime_resume(struct device *dev)
>               if (retval < 0)
>                       dev_dbg(&port_dev->dev, "can't get reconnection after 
> setting port  power on, status %d\n",
>                                       retval);
> -             usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_ENABLE);
>               retval = 0;
>       }
>  
> @@ -142,7 +141,8 @@ static int usb_port_runtime_suspend(struct device *dev)
>       set_bit(port1, hub->busy_bits);
>       retval = usb_hub_set_port_power(hdev, hub, port1, false);
>       usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_CONNECTION);
> -     usb_clear_port_feature(hdev, port1,     USB_PORT_FEAT_C_ENABLE);
> +     if (!hub_is_superspeed(hdev))
> +             usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_ENABLE);
>       clear_bit(port1, hub->busy_bits);
>       usb_autopm_put_interface(intf);
>  
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to