On 06/04/2012 04:46 PM, Stanislaw Ledwon wrote:
> The host controller port status register supports CAS (Could Attach
> Status) bit. This bit could be set when USB3.0 device is connectd
> when system is in Sx state. When the system wakes to S0 this port
> status with CAS bit is reported and this port can't be used by any
> device.
>
> When CAS bit is set the port should be reset by warm reset. This
> was not supported by xhci driver.
>
> The issue was found when pendrive was connected to suspended
> platfrom. The link state of "Compliance Mode" was reported togather
> with CAS bit. This link state was also not supported by xhci and
> core/hub.c.
>
> The CAS bit is defined only for xhci root hub port and it is
> not supported on regular hubs. The link status is used to force
> warm reset on port. Make the USB core issue a warm reset when port
> is in ether the 'inactive' or 'compliance mode'. Change the xHCI driver
> to report 'compliance mode' when the CAS is set. This force warm reset
> on the root hub port.
>
Why fake the port status to compliance mode? All you want to do is issue
a warm reset when finding a CAS bit set. Fake a bogus USB3 wPortStatus
bit is simpler and clearer.
It's like this:
1. define a bogus bit in ch11.h. You can use bit13 since it's reserved.
#define USB_SS_PORT_STAT_CAS_BOGUS 0x2000 /* bit 13 */
2. In GetPortStatus in xhci_hub_control, report the CAS.
If (temp & PORT_CAS)
status |= USB_SS_PORT_STAT_CAS_BOGUS;
3. in hub.c, issue a warm reset if CAS is found.
+static bool hub_port_warm_reset_required(struct usb_hub *hub, u16
portstatus)
{
return hub_is_superspeed(hub->hdev) &&
- (portstatus & USB_PORT_STAT_LINK_STATE) ==
- USB_SS_PORT_LS_SS_INACTIVE;
+ (((portstatus & USB_PORT_STAT_LINK_STATE) ==
+ USB_SS_PORT_LS_SS_INACTIVE) ||
+ (portstatus & USB_PORT_STAT_CAS_BOGUS));
}
> Signed-off-by: Stanislaw Ledwon <staszek.ledwon@xxxxxxxxxxxxxxx>
> ---
> drivers/usb/core/hub.c | 18 +++++++++-------
> drivers/usb/host/xhci-hub.c | 44 +++++++++++++++++++++++++++++++++++++-----
> drivers/usb/host/xhci.h | 6 ++++-
> 3 files changed, 53 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index ec6c97d..572769b 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -2102,12 +2102,16 @@ static unsigned hub_is_wusb(struct usb_hub *hub)
> static int hub_port_reset(struct usb_hub *hub, int port1,
> struct usb_device *udev, unsigned int delay, bool warm);
>
> -/* Is a USB 3.0 port in the Inactive state? */
> -static bool hub_port_inactive(struct usb_hub *hub, u16 portstatus)
> +/* Is a USB 3.0 port in the Inactive or Complinance Mode state?
> + * Port worm reset is required to recover
> + */
> +static bool hub_port_warm_reset_required(struct usb_hub *hub, u16 portstatus)
> {
> return hub_is_superspeed(hub->hdev) &&
> - (portstatus & USB_PORT_STAT_LINK_STATE) ==
> - USB_SS_PORT_LS_SS_INACTIVE;
> + (((portstatus & USB_PORT_STAT_LINK_STATE) ==
> + USB_SS_PORT_LS_SS_INACTIVE) ||
> + ((portstatus & USB_PORT_STAT_LINK_STATE) ==
> + USB_SS_PORT_LS_COMP_MOD)) ;
> }
>
> static int hub_port_wait_reset(struct usb_hub *hub, int port1,
> @@ -2143,7 +2147,7 @@ static int hub_port_wait_reset(struct usb_hub *hub, int port1,
> *
> * See https://bugzilla.kernel.org/show_bug.cgi?id=41752
> */
> - if (hub_port_inactive(hub, portstatus)) {
> + if (hub_port_warm_reset_required(hub, portstatus)) {
> int ret;
>
> if ((portchange & USB_PORT_STAT_C_CONNECTION))
> @@ -3753,9 +3757,7 @@ static void hub_events(void)
> /* Warm reset a USB3 protocol port if it's in
> * SS.Inactive state.
> */
> - if (hub_is_superspeed(hub->hdev) &&
> - (portstatus & USB_PORT_STAT_LINK_STATE)
> - == USB_SS_PORT_LS_SS_INACTIVE) {
> + if (hub_port_warm_reset_required(hub, portstatus)) {
> dev_dbg(hub_dev, "warm reset port %d\n", i);
> hub_port_reset(hub, i, NULL,
> HUB_BH_RESET_TIME, true);
> diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
> index 673ad12..58e1e73 100644
> --- a/drivers/usb/host/xhci-hub.c
> +++ b/drivers/usb/host/xhci-hub.c
> @@ -462,6 +462,42 @@ void xhci_test_and_clear_bit(struct xhci_hcd *xhci, __le32 __iomem **port_array,
> }
> }
>
> +/* Updates Link Status for super Speed port */
> +void xhci_hub_set_link_status(u32 *status, u32 status_reg)
The function name is ambiguous. Its purpose is to report a bogus port
status, not "set_link_status". There is another function named
xhci_set_link_state() in the same file, which does complete different
things.
Also, this should be a static function.
> +{
> + u32 pls = status_reg & PORT_PLS_MASK;
> +
> + /* resume state is a xHCI internal state.
> + * Do not report it to usb core.
> + */
> + if (pls == XDEV_RESUME)
> + return;
> +
> + /* When the CAS bit is set then warm reset
> + * should be performed on port
> + */
> + if (status_reg & PORT_CAS) {
> + /* The CAS bit can be set while the port is
> + * in any link state.
> + * Only roothubs have CAS bit, so we
> + * pretend to be in compliance mode
> + * unless we're already in compliance
> + * or the inactive state.
> + */
> + if (pls != USB_SS_PORT_LS_COMP_MOD &&
> + pls != USB_SS_PORT_LS_SS_INACTIVE) {
> + pls = USB_SS_PORT_LS_COMP_MOD;
> + }
> + /* Return also connection bit -
> + * hub state machine resets port
> + * when this bit is set.
> + */
> + pls |= USB_PORT_STAT_CONNECTION;
Why do you need this setting? Does PORTSC return a PORT_CONNECT when CAS
is set?
Thanks,
Andiry
> + }
> + /* update status field */
> + *status |= pls;
> +}
> +
> int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
> u16 wIndex, char *buf, u16 wLength)
> {
> @@ -604,13 +640,9 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
> else
> status |= USB_PORT_STAT_POWER;
> }
> - /* Port Link State */
> + /* Update Port Link State for super speed ports*/
> if (hcd->speed == HCD_USB3) {
> - /* resume state is a xHCI internal state.
> - * Do not report it to usb core.
> - */
> - if ((temp & PORT_PLS_MASK) != XDEV_RESUME)
> - status |= (temp & PORT_PLS_MASK);
> + xhci_hub_set_link_status(&status, temp);
> }
> if (bus_state->port_c_suspend & (1 << wIndex))
> status |= 1 << USB_PORT_FEAT_C_SUSPEND;
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index 3d69c4b..7e8280a 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -341,7 +341,11 @@ struct xhci_op_regs {
> #define PORT_PLC (1 << 22)
> /* port configure error change - port failed to configure its link partner */
> #define PORT_CEC (1 << 23)
> -/* bit 24 reserved */
> +/* Cold Attach Status - xHC can set this bit to report device attached during
> + * Sx state. Warm port reset should be perfomed to clear this bit and move port
> + * to connected state.
> + */
> +#define PORT_CAS (1 << 24)
> /* wake on connect (enable) */
> #define PORT_WKCONN_E (1 << 25)
> /* wake on disconnect (enable) */
--
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
