Re: Ryzen7 3700U xhci fails on resume from sleep

2019-08-29 Thread Daniel Drake
On Wed, Aug 28, 2019 at 4:43 PM Rafael J. Wysocki  wrote:
> With the git branch mentioned previously merged in, you can enable
> dynamic debug in device_pm.c, repeat the PM-runtime test and collect
> the log.  There should be some additional messages from the ACPI layer
> in it.

That's useful, thanks. Runtime suspend:

usb 1-4: USB disconnect, device number 2
power-0419 __acpi_power_off  : Power resource [P0U0] turned off
device_pm-0278 device_set_power  : Device [XHC0] transitioned to D3hot

Runtime resume:
power-0363 __acpi_power_on   : Power resource [P0U0] turned on
device_pm-0278 device_set_power  : Device [XHC0] transitioned to D0
xhci_hcd :03:00.3: Refused to change power state, currently in D3
xhci_hcd :03:00.3: enabling device ( -> 0002)
xhci_hcd :03:00.3: WARN: xHC restore state timeout
xhci_hcd :03:00.3: PCI post-resume error -110!
xhci_hcd :03:00.3: HC died; cleaning up


Re: [PATCH v3 2/2] usb: roles: intel: Enable static DRD mode for role switch

2019-08-29 Thread Heikki Krogerus
Hi,

>  static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
> @@ -44,7 +52,7 @@ static int intel_xhci_usb_set_role(struct device *dev, enum 
> usb_role role)
>   struct intel_xhci_usb_data *data = dev_get_drvdata(dev);
>   unsigned long timeout;
>   acpi_status status;
> - u32 glk, val;
> + u32 glk, val, drd_config;

I guess you need to fix that. While at it, please introduce it on its
own line:

u32 drd_config = DRD_CONFIG_DYNAMIC;


thanks,

-- 
heikki


Re: fsck on ext4: "WARN Wrong bounce buffer write length: 1024 != 0"

2019-08-29 Thread Harald Dunkel

On 8/28/19 9:57 AM, Mathias Nyman wrote:


I'll submit a proper patch

-Mathias



Thanx very much
Harri


[PATCH v4 1/2] xhci-ext-caps.c: Add property to disable Intel SW switch

2019-08-29 Thread Saranya Gopal
In platforms like Cherrytrail, 'SW switch enable' bit
should not be enabled for role switch. This patch
adds a property to Intel USB Role Switch platform driver
to denote that SW switch should be disabled in
Cherrytrail devices.

Signed-off-by: Saranya Gopal 
Signed-off-by: Balaji Manoharan 
Suggested-by: Heikki Krogerus 
---
 changes since v3: none
 changes since v2: Added suggested-by tag
 changes since v1: none

 drivers/usb/host/xhci-ext-caps.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/drivers/usb/host/xhci-ext-caps.c b/drivers/usb/host/xhci-ext-caps.c
index 399113f..f498160 100644
--- a/drivers/usb/host/xhci-ext-caps.c
+++ b/drivers/usb/host/xhci-ext-caps.c
@@ -6,11 +6,20 @@
  */
 
 #include 
+#include 
+#include 
 #include "xhci.h"
 
 #define USB_SW_DRV_NAME"intel_xhci_usb_sw"
 #define USB_SW_RESOURCE_SIZE   0x400
 
+#define PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI0x22b5
+
+static const struct property_entry role_switch_props[] = {
+   PROPERTY_ENTRY_BOOL("sw_switch_disable"),
+   {},
+};
+
 static void xhci_intel_unregister_pdev(void *arg)
 {
platform_device_unregister(arg);
@@ -21,6 +30,7 @@ static int xhci_create_intel_xhci_sw_pdev(struct xhci_hcd 
*xhci, u32 cap_offset)
struct usb_hcd *hcd = xhci_to_hcd(xhci);
struct device *dev = hcd->self.controller;
struct platform_device *pdev;
+   struct pci_dev *pci = to_pci_dev(dev);
struct resource res = { 0, };
int ret;
 
@@ -43,6 +53,14 @@ static int xhci_create_intel_xhci_sw_pdev(struct xhci_hcd 
*xhci, u32 cap_offset)
return ret;
}
 
+   if (pci->device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI) {
+   ret = platform_device_add_properties(pdev, role_switch_props);
+   if (ret) {
+   dev_err(dev, "failed to register device properties\n");
+   return ret;
+   }
+   }
+
pdev->dev.parent = dev;
 
ret = platform_device_add(pdev);
-- 
1.9.1



[PATCH v4 2/2] usb: roles: intel: Enable static DRD mode for role switch

2019-08-29 Thread Saranya Gopal
Enable static DRD mode in Intel platforms which guarantees
successful role switch all the time. This fixes issues like
software role switch failure after cold boot and issue with
role switch when USB 3.0 cable is used. But, do not enable
static DRD mode for Cherrytrail devices which rely on firmware
for role switch.

Signed-off-by: Saranya Gopal 
Signed-off-by: Balaji Manoharan 
---
 changes since v3: Initialized drd_config variable to fix warning
 changes since v2: Revised SoB tags
 changes since v1: Added drd_config to avoid multiple if checks
   Other minor changes suggested by Hans
   
 drivers/usb/roles/intel-xhci-usb-role-switch.c | 27 --
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c 
b/drivers/usb/roles/intel-xhci-usb-role-switch.c
index 277de96..88d0416 100644
--- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
+++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 /* register definition */
@@ -26,6 +27,12 @@
 #define SW_VBUS_VALID  BIT(24)
 #define SW_IDPIN_ENBIT(21)
 #define SW_IDPIN   BIT(20)
+#define SW_SWITCH_EN   BIT(16)
+
+#define DRD_CONFIG_DYNAMIC 0
+#define DRD_CONFIG_STATIC_HOST 1
+#define DRD_CONFIG_STATIC_DEVICE   2
+#define DRD_CONFIG_MASK3
 
 #define DUAL_ROLE_CFG1 0x6c
 #define HOST_MODE  BIT(29)
@@ -37,6 +44,7 @@
 struct intel_xhci_usb_data {
struct usb_role_switch *role_sw;
void __iomem *base;
+   bool enable_sw_switch;
 };
 
 static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
@@ -45,6 +53,7 @@ static int intel_xhci_usb_set_role(struct device *dev, enum 
usb_role role)
unsigned long timeout;
acpi_status status;
u32 glk, val;
+   u32 drd_config = DRD_CONFIG_DYNAMIC;
 
/*
 * On many CHT devices ACPI event (_AEI) handlers read / modify /
@@ -59,24 +68,35 @@ static int intel_xhci_usb_set_role(struct device *dev, enum 
usb_role role)
 
pm_runtime_get_sync(dev);
 
-   /* Set idpin value as requested */
+   /*
+* Set idpin value as requested.
+* Since some devices rely on firmware setting DRD_CONFIG and
+* SW_SWITCH_EN bits to be zero for role switch,
+* do not set these bits for those devices.
+*/
val = readl(data->base + DUAL_ROLE_CFG0);
switch (role) {
case USB_ROLE_NONE:
val |= SW_IDPIN;
val &= ~SW_VBUS_VALID;
+   drd_config = DRD_CONFIG_DYNAMIC;
break;
case USB_ROLE_HOST:
val &= ~SW_IDPIN;
val &= ~SW_VBUS_VALID;
+   drd_config = DRD_CONFIG_STATIC_HOST;
break;
case USB_ROLE_DEVICE:
val |= SW_IDPIN;
val |= SW_VBUS_VALID;
+   drd_config = DRD_CONFIG_STATIC_DEVICE;
break;
}
val |= SW_IDPIN_EN;
-
+   if (data->enable_sw_switch) {
+   val &= ~DRD_CONFIG_MASK;
+   val |= SW_SWITCH_EN | drd_config;
+   }
writel(val, data->base + DUAL_ROLE_CFG0);
 
acpi_release_global_lock(glk);
@@ -147,6 +167,9 @@ static int intel_xhci_usb_probe(struct platform_device 
*pdev)
 
platform_set_drvdata(pdev, data);
 
+   data->enable_sw_switch = !device_property_read_bool(dev,
+   "sw_switch_disable");
+
data->role_sw = usb_role_switch_register(dev, &sw_desc);
if (IS_ERR(data->role_sw))
return PTR_ERR(data->role_sw);
-- 
1.9.1



Re: [PATCH v4 1/2] xhci-ext-caps.c: Add property to disable Intel SW switch

2019-08-29 Thread Heikki Krogerus
On Thu, Aug 29, 2019 at 04:12:06PM +0530, Saranya Gopal wrote:
> In platforms like Cherrytrail, 'SW switch enable' bit
> should not be enabled for role switch. This patch
> adds a property to Intel USB Role Switch platform driver
> to denote that SW switch should be disabled in
> Cherrytrail devices.
> 
> Signed-off-by: Saranya Gopal 
> Signed-off-by: Balaji Manoharan 
> Suggested-by: Heikki Krogerus 

Reviewed-by: Heikki Krogerus 

> ---
>  changes since v3: none
>  changes since v2: Added suggested-by tag
>  changes since v1: none
> 
>  drivers/usb/host/xhci-ext-caps.c | 18 ++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/usb/host/xhci-ext-caps.c 
> b/drivers/usb/host/xhci-ext-caps.c
> index 399113f..f498160 100644
> --- a/drivers/usb/host/xhci-ext-caps.c
> +++ b/drivers/usb/host/xhci-ext-caps.c
> @@ -6,11 +6,20 @@
>   */
>  
>  #include 
> +#include 
> +#include 
>  #include "xhci.h"
>  
>  #define USB_SW_DRV_NAME  "intel_xhci_usb_sw"
>  #define USB_SW_RESOURCE_SIZE 0x400
>  
> +#define PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI  0x22b5
> +
> +static const struct property_entry role_switch_props[] = {
> + PROPERTY_ENTRY_BOOL("sw_switch_disable"),
> + {},
> +};
> +
>  static void xhci_intel_unregister_pdev(void *arg)
>  {
>   platform_device_unregister(arg);
> @@ -21,6 +30,7 @@ static int xhci_create_intel_xhci_sw_pdev(struct xhci_hcd 
> *xhci, u32 cap_offset)
>   struct usb_hcd *hcd = xhci_to_hcd(xhci);
>   struct device *dev = hcd->self.controller;
>   struct platform_device *pdev;
> + struct pci_dev *pci = to_pci_dev(dev);
>   struct resource res = { 0, };
>   int ret;
>  
> @@ -43,6 +53,14 @@ static int xhci_create_intel_xhci_sw_pdev(struct xhci_hcd 
> *xhci, u32 cap_offset)
>   return ret;
>   }
>  
> + if (pci->device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI) {
> + ret = platform_device_add_properties(pdev, role_switch_props);
> + if (ret) {
> + dev_err(dev, "failed to register device properties\n");
> + return ret;
> + }
> + }
> +
>   pdev->dev.parent = dev;
>  
>   ret = platform_device_add(pdev);
> -- 
> 1.9.1

thanks,

-- 
heikki


Re: [PATCH v4 2/2] usb: roles: intel: Enable static DRD mode for role switch

2019-08-29 Thread Heikki Krogerus
On Thu, Aug 29, 2019 at 04:12:07PM +0530, Saranya Gopal wrote:
> Enable static DRD mode in Intel platforms which guarantees
> successful role switch all the time. This fixes issues like
> software role switch failure after cold boot and issue with
> role switch when USB 3.0 cable is used. But, do not enable
> static DRD mode for Cherrytrail devices which rely on firmware
> for role switch.
> 
> Signed-off-by: Saranya Gopal 
> Signed-off-by: Balaji Manoharan 

Reviewed-by: Heikki Krogerus 

> ---
>  changes since v3: Initialized drd_config variable to fix warning
>  changes since v2: Revised SoB tags
>  changes since v1: Added drd_config to avoid multiple if checks
>Other minor changes suggested by Hans
>
>  drivers/usb/roles/intel-xhci-usb-role-switch.c | 27 
> --
>  1 file changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c 
> b/drivers/usb/roles/intel-xhci-usb-role-switch.c
> index 277de96..88d0416 100644
> --- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
> +++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
> @@ -19,6 +19,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  /* register definition */
> @@ -26,6 +27,12 @@
>  #define SW_VBUS_VALIDBIT(24)
>  #define SW_IDPIN_EN  BIT(21)
>  #define SW_IDPIN BIT(20)
> +#define SW_SWITCH_EN BIT(16)
> +
> +#define DRD_CONFIG_DYNAMIC   0
> +#define DRD_CONFIG_STATIC_HOST   1
> +#define DRD_CONFIG_STATIC_DEVICE 2
> +#define DRD_CONFIG_MASK  3
>  
>  #define DUAL_ROLE_CFG1   0x6c
>  #define HOST_MODEBIT(29)
> @@ -37,6 +44,7 @@
>  struct intel_xhci_usb_data {
>   struct usb_role_switch *role_sw;
>   void __iomem *base;
> + bool enable_sw_switch;
>  };
>  
>  static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
> @@ -45,6 +53,7 @@ static int intel_xhci_usb_set_role(struct device *dev, enum 
> usb_role role)
>   unsigned long timeout;
>   acpi_status status;
>   u32 glk, val;
> + u32 drd_config = DRD_CONFIG_DYNAMIC;
>  
>   /*
>* On many CHT devices ACPI event (_AEI) handlers read / modify /
> @@ -59,24 +68,35 @@ static int intel_xhci_usb_set_role(struct device *dev, 
> enum usb_role role)
>  
>   pm_runtime_get_sync(dev);
>  
> - /* Set idpin value as requested */
> + /*
> +  * Set idpin value as requested.
> +  * Since some devices rely on firmware setting DRD_CONFIG and
> +  * SW_SWITCH_EN bits to be zero for role switch,
> +  * do not set these bits for those devices.
> +  */
>   val = readl(data->base + DUAL_ROLE_CFG0);
>   switch (role) {
>   case USB_ROLE_NONE:
>   val |= SW_IDPIN;
>   val &= ~SW_VBUS_VALID;
> + drd_config = DRD_CONFIG_DYNAMIC;
>   break;
>   case USB_ROLE_HOST:
>   val &= ~SW_IDPIN;
>   val &= ~SW_VBUS_VALID;
> + drd_config = DRD_CONFIG_STATIC_HOST;
>   break;
>   case USB_ROLE_DEVICE:
>   val |= SW_IDPIN;
>   val |= SW_VBUS_VALID;
> + drd_config = DRD_CONFIG_STATIC_DEVICE;
>   break;
>   }
>   val |= SW_IDPIN_EN;
> -
> + if (data->enable_sw_switch) {
> + val &= ~DRD_CONFIG_MASK;
> + val |= SW_SWITCH_EN | drd_config;
> + }
>   writel(val, data->base + DUAL_ROLE_CFG0);
>  
>   acpi_release_global_lock(glk);
> @@ -147,6 +167,9 @@ static int intel_xhci_usb_probe(struct platform_device 
> *pdev)
>  
>   platform_set_drvdata(pdev, data);
>  
> + data->enable_sw_switch = !device_property_read_bool(dev,
> + "sw_switch_disable");
> +
>   data->role_sw = usb_role_switch_register(dev, &sw_desc);
>   if (IS_ERR(data->role_sw))
>   return PTR_ERR(data->role_sw);
> -- 
> 1.9.1

thanks,

-- 
heikki


Re: [PATCH v4 2/2] usb: roles: intel: Enable static DRD mode for role switch

2019-08-29 Thread Hans de Goede

Hi Saranya,

On 29-08-19 12:42, Saranya Gopal wrote:

Enable static DRD mode in Intel platforms which guarantees
successful role switch all the time. This fixes issues like
software role switch failure after cold boot and issue with
role switch when USB 3.0 cable is used. But, do not enable
static DRD mode for Cherrytrail devices which rely on firmware
for role switch.

Signed-off-by: Saranya Gopal 
Signed-off-by: Balaji Manoharan 


Unfortunately this patch conflicts with a patch to
drivers/usb/roles/intel-xhci-usb-role-switch.c from Heikki
which is already in -next, see:

https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/log/?h=devprop

And specifically this commit:

https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/commit/?h=devprop&id=d2a90ebb65536a6deeb9daf5aa8e0700e8cbb43a

So you need to rebase on op of that branch and then the subsys
maintainers need to figure out how to merge this, I guess
the usb-next tree can merge Rafael's devprop branch for this?

I've manually resolved the conflict locally and  given this new version
a test-run on a Cherry Trail device and things still work as they should
there, so with the conflict fixed this series is:

Tested-by: Hans de Goede 
Reviewed-by: Hans de Goede 

Regards,

Hans



---
  changes since v3: Initialized drd_config variable to fix warning
  changes since v2: Revised SoB tags
  changes since v1: Added drd_config to avoid multiple if checks
Other minor changes suggested by Hans

  drivers/usb/roles/intel-xhci-usb-role-switch.c | 27 --

  1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c 
b/drivers/usb/roles/intel-xhci-usb-role-switch.c
index 277de96..88d0416 100644
--- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
+++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
@@ -19,6 +19,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  
  /* register definition */

@@ -26,6 +27,12 @@
  #define SW_VBUS_VALID BIT(24)
  #define SW_IDPIN_EN   BIT(21)
  #define SW_IDPIN  BIT(20)
+#define SW_SWITCH_EN   BIT(16)
+
+#define DRD_CONFIG_DYNAMIC 0
+#define DRD_CONFIG_STATIC_HOST 1
+#define DRD_CONFIG_STATIC_DEVICE   2
+#define DRD_CONFIG_MASK3
  
  #define DUAL_ROLE_CFG1			0x6c

  #define HOST_MODE BIT(29)
@@ -37,6 +44,7 @@
  struct intel_xhci_usb_data {
struct usb_role_switch *role_sw;
void __iomem *base;
+   bool enable_sw_switch;
  };
  
  static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)

@@ -45,6 +53,7 @@ static int intel_xhci_usb_set_role(struct device *dev, enum 
usb_role role)
unsigned long timeout;
acpi_status status;
u32 glk, val;
+   u32 drd_config = DRD_CONFIG_DYNAMIC;
  
  	/*

 * On many CHT devices ACPI event (_AEI) handlers read / modify /
@@ -59,24 +68,35 @@ static int intel_xhci_usb_set_role(struct device *dev, enum 
usb_role role)
  
  	pm_runtime_get_sync(dev);
  
-	/* Set idpin value as requested */

+   /*
+* Set idpin value as requested.
+* Since some devices rely on firmware setting DRD_CONFIG and
+* SW_SWITCH_EN bits to be zero for role switch,
+* do not set these bits for those devices.
+*/
val = readl(data->base + DUAL_ROLE_CFG0);
switch (role) {
case USB_ROLE_NONE:
val |= SW_IDPIN;
val &= ~SW_VBUS_VALID;
+   drd_config = DRD_CONFIG_DYNAMIC;
break;
case USB_ROLE_HOST:
val &= ~SW_IDPIN;
val &= ~SW_VBUS_VALID;
+   drd_config = DRD_CONFIG_STATIC_HOST;
break;
case USB_ROLE_DEVICE:
val |= SW_IDPIN;
val |= SW_VBUS_VALID;
+   drd_config = DRD_CONFIG_STATIC_DEVICE;
break;
}
val |= SW_IDPIN_EN;
-
+   if (data->enable_sw_switch) {
+   val &= ~DRD_CONFIG_MASK;
+   val |= SW_SWITCH_EN | drd_config;
+   }
writel(val, data->base + DUAL_ROLE_CFG0);
  
  	acpi_release_global_lock(glk);

@@ -147,6 +167,9 @@ static int intel_xhci_usb_probe(struct platform_device 
*pdev)
  
  	platform_set_drvdata(pdev, data);
  
+	data->enable_sw_switch = !device_property_read_bool(dev,

+   "sw_switch_disable");
+
data->role_sw = usb_role_switch_register(dev, &sw_desc);
if (IS_ERR(data->role_sw))
return PTR_ERR(data->role_sw);



RE: [PATCH v4 2/2] usb: roles: intel: Enable static DRD mode for role switch

2019-08-29 Thread Gopal, Saranya


> -Original Message-
> From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-
> ow...@vger.kernel.org] On Behalf Of Hans de Goede
> Sent: Thursday, August 29, 2019 4:34 PM
> To: Gopal, Saranya ;
> heikki.kroge...@linux.intel.com
> Cc: gre...@linuxfoundation.org; Nyman, Mathias
> ; linux-usb@vger.kernel.org; Balaji, M
> ; Rafał Psota ; Rafael J. Wysocki
> 
> Subject: Re: [PATCH v4 2/2] usb: roles: intel: Enable static DRD mode for role
> switch
> 
> Hi Saranya,
> 
> On 29-08-19 12:42, Saranya Gopal wrote:
> > Enable static DRD mode in Intel platforms which guarantees
> > successful role switch all the time. This fixes issues like
> > software role switch failure after cold boot and issue with
> > role switch when USB 3.0 cable is used. But, do not enable
> > static DRD mode for Cherrytrail devices which rely on firmware
> > for role switch.
> >
> > Signed-off-by: Saranya Gopal 
> > Signed-off-by: Balaji Manoharan 
> 
> Unfortunately this patch conflicts with a patch to
> drivers/usb/roles/intel-xhci-usb-role-switch.c from Heikki
> which is already in -next, see:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-
> pm.git/log/?h=devprop
> 
> And specifically this commit:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-
> pm.git/commit/?h=devprop&id=d2a90ebb65536a6deeb9daf5aa8e0700e8cbb43
> a
> 
> So you need to rebase on op of that branch and then the subsys
> maintainers need to figure out how to merge this, I guess
> the usb-next tree can merge Rafael's devprop branch for this?
>
Sure, let me rebase on top of that branch.
Thanks,
Saranya
 
> I've manually resolved the conflict locally and  given this new version
> a test-run on a Cherry Trail device and things still work as they should
> there, so with the conflict fixed this series is:
> 
> Tested-by: Hans de Goede 
> Reviewed-by: Hans de Goede 
> 
> Regards,
> 
> Hans
> 
> 
> > ---
> >   changes since v3: Initialized drd_config variable to fix warning
> >   changes since v2: Revised SoB tags
> >   changes since v1: Added drd_config to avoid multiple if checks
> > Other minor changes suggested by Hans
> >
> >   drivers/usb/roles/intel-xhci-usb-role-switch.c | 27
> --
> >   1 file changed, 25 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c
> b/drivers/usb/roles/intel-xhci-usb-role-switch.c
> > index 277de96..88d0416 100644
> > --- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
> > +++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
> > @@ -19,6 +19,7 @@
> >   #include 
> >   #include 
> >   #include 
> > +#include 
> >   #include 
> >
> >   /* register definition */
> > @@ -26,6 +27,12 @@
> >   #define SW_VBUS_VALID BIT(24)
> >   #define SW_IDPIN_EN   BIT(21)
> >   #define SW_IDPIN  BIT(20)
> > +#define SW_SWITCH_EN   BIT(16)
> > +
> > +#define DRD_CONFIG_DYNAMIC 0
> > +#define DRD_CONFIG_STATIC_HOST 1
> > +#define DRD_CONFIG_STATIC_DEVICE   2
> > +#define DRD_CONFIG_MASK3
> >
> >   #define DUAL_ROLE_CFG10x6c
> >   #define HOST_MODE BIT(29)
> > @@ -37,6 +44,7 @@
> >   struct intel_xhci_usb_data {
> > struct usb_role_switch *role_sw;
> > void __iomem *base;
> > +   bool enable_sw_switch;
> >   };
> >
> >   static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
> > @@ -45,6 +53,7 @@ static int intel_xhci_usb_set_role(struct device *dev,
> enum usb_role role)
> > unsigned long timeout;
> > acpi_status status;
> > u32 glk, val;
> > +   u32 drd_config = DRD_CONFIG_DYNAMIC;
> >
> > /*
> >  * On many CHT devices ACPI event (_AEI) handlers read / modify /
> > @@ -59,24 +68,35 @@ static int intel_xhci_usb_set_role(struct device *dev,
> enum usb_role role)
> >
> > pm_runtime_get_sync(dev);
> >
> > -   /* Set idpin value as requested */
> > +   /*
> > +* Set idpin value as requested.
> > +* Since some devices rely on firmware setting DRD_CONFIG and
> > +* SW_SWITCH_EN bits to be zero for role switch,
> > +* do not set these bits for those devices.
> > +*/
> > val = readl(data->base + DUAL_ROLE_CFG0);
> > switch (role) {
> > case USB_ROLE_NONE:
> > val |= SW_IDPIN;
> > val &= ~SW_VBUS_VALID;
> > +   drd_config = DRD_CONFIG_DYNAMIC;
> > break;
> > case USB_ROLE_HOST:
> > val &= ~SW_IDPIN;
> > val &= ~SW_VBUS_VALID;
> > +   drd_config = DRD_CONFIG_STATIC_HOST;
> > break;
> > case USB_ROLE_DEVICE:
> > val |= SW_IDPIN;
> > val |= SW_VBUS_VALID;
> > +   drd_config = DRD_CONFIG_STATIC_DEVICE;
> > break;
> > }
> > val |= SW_IDPIN_EN;
> > -
> > +   if (data->enable_sw_switch) {
> > +   val &= ~DRD_CONFIG_MASK;
> > +   val |= SW_SWITCH_EN | drd

[PATCH v5 1/2] xhci-ext-caps.c: Add property to disable Intel SW switch

2019-08-29 Thread Saranya Gopal
In platforms like Cherrytrail, 'SW switch enable' bit
should not be enabled for role switch. This patch
adds a property to Intel USB Role Switch platform driver
to denote that SW switch should be disabled in
Cherrytrail devices.

Signed-off-by: Saranya Gopal 
Signed-off-by: Balaji Manoharan 
Suggested-by: Heikki Krogerus 
Reviewed-by: Heikki Krogerus 
---
changes since v4: Added Reviewed-by tag
changes since v3: none
changes since v2: Added Suggested-by tag
changes since v1: none

 drivers/usb/host/xhci-ext-caps.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/drivers/usb/host/xhci-ext-caps.c b/drivers/usb/host/xhci-ext-caps.c
index 399113f..f498160 100644
--- a/drivers/usb/host/xhci-ext-caps.c
+++ b/drivers/usb/host/xhci-ext-caps.c
@@ -6,11 +6,20 @@
  */
 
 #include 
+#include 
+#include 
 #include "xhci.h"
 
 #define USB_SW_DRV_NAME"intel_xhci_usb_sw"
 #define USB_SW_RESOURCE_SIZE   0x400
 
+#define PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI0x22b5
+
+static const struct property_entry role_switch_props[] = {
+   PROPERTY_ENTRY_BOOL("sw_switch_disable"),
+   {},
+};
+
 static void xhci_intel_unregister_pdev(void *arg)
 {
platform_device_unregister(arg);
@@ -21,6 +30,7 @@ static int xhci_create_intel_xhci_sw_pdev(struct xhci_hcd 
*xhci, u32 cap_offset)
struct usb_hcd *hcd = xhci_to_hcd(xhci);
struct device *dev = hcd->self.controller;
struct platform_device *pdev;
+   struct pci_dev *pci = to_pci_dev(dev);
struct resource res = { 0, };
int ret;
 
@@ -43,6 +53,14 @@ static int xhci_create_intel_xhci_sw_pdev(struct xhci_hcd 
*xhci, u32 cap_offset)
return ret;
}
 
+   if (pci->device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI) {
+   ret = platform_device_add_properties(pdev, role_switch_props);
+   if (ret) {
+   dev_err(dev, "failed to register device properties\n");
+   return ret;
+   }
+   }
+
pdev->dev.parent = dev;
 
ret = platform_device_add(pdev);
-- 
1.9.1



[PATCH v5 2/2] usb: roles: intel: Enable static DRD mode for role switch

2019-08-29 Thread Saranya Gopal
Enable static DRD mode in Intel platforms which guarantees
successful role switch all the time. This fixes issues like
software role switch failure after cold boot and issue with
role switch when USB 3.0 cable is used. But, do not enable
static DRD mode for Cherrytrail devices which rely on firmware
for role switch.

Signed-off-by: Saranya Gopal 
Signed-off-by: Balaji Manoharan 
Reviewed-by: Heikki Krogerus 
Reviewed-by: Hans de Goede 
Tested-by: Hans de Goede 
---
changes since v4: Rebased on top of devprop branch in linux-pm.git
  Added Reviewed-by and Tested-by tags
changes since v3: Initialized drd_config variable to fix warning
changes since v2: Revised SoB tags
changes since v1: Added drd_config to avoid multiple if checks
  Other minor changes suggested by Hans

 drivers/usb/roles/intel-xhci-usb-role-switch.c | 27 --
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c 
b/drivers/usb/roles/intel-xhci-usb-role-switch.c
index 7325a84..4098513 100644
--- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
+++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 /* register definition */
@@ -26,6 +27,12 @@
 #define SW_VBUS_VALID  BIT(24)
 #define SW_IDPIN_ENBIT(21)
 #define SW_IDPIN   BIT(20)
+#define SW_SWITCH_EN   BIT(16)
+
+#define DRD_CONFIG_DYNAMIC 0
+#define DRD_CONFIG_STATIC_HOST 1
+#define DRD_CONFIG_STATIC_DEVICE   2
+#define DRD_CONFIG_MASK3
 
 #define DUAL_ROLE_CFG1 0x6c
 #define HOST_MODE  BIT(29)
@@ -37,6 +44,7 @@
 struct intel_xhci_usb_data {
struct usb_role_switch *role_sw;
void __iomem *base;
+   bool enable_sw_switch;
 };
 
 static const struct software_node intel_xhci_usb_node = {
@@ -49,6 +57,7 @@ static int intel_xhci_usb_set_role(struct device *dev, enum 
usb_role role)
unsigned long timeout;
acpi_status status;
u32 glk, val;
+   u32 drd_config = DRD_CONFIG_DYNAMIC;
 
/*
 * On many CHT devices ACPI event (_AEI) handlers read / modify /
@@ -63,24 +72,35 @@ static int intel_xhci_usb_set_role(struct device *dev, enum 
usb_role role)
 
pm_runtime_get_sync(dev);
 
-   /* Set idpin value as requested */
+   /*
+* Set idpin value as requested.
+* Since some devices rely on firmware setting DRD_CONFIG and
+* SW_SWITCH_EN bits to be zero for role switch,
+* do not set these bits for those devices.
+*/
val = readl(data->base + DUAL_ROLE_CFG0);
switch (role) {
case USB_ROLE_NONE:
val |= SW_IDPIN;
val &= ~SW_VBUS_VALID;
+   drd_config = DRD_CONFIG_DYNAMIC;
break;
case USB_ROLE_HOST:
val &= ~SW_IDPIN;
val &= ~SW_VBUS_VALID;
+   drd_config = DRD_CONFIG_STATIC_HOST;
break;
case USB_ROLE_DEVICE:
val |= SW_IDPIN;
val |= SW_VBUS_VALID;
+   drd_config = DRD_CONFIG_STATIC_DEVICE;
break;
}
val |= SW_IDPIN_EN;
-
+   if (data->enable_sw_switch) {
+   val &= ~DRD_CONFIG_MASK;
+   val |= SW_SWITCH_EN | drd_config;
+   }
writel(val, data->base + DUAL_ROLE_CFG0);
 
acpi_release_global_lock(glk);
@@ -156,6 +176,9 @@ static int intel_xhci_usb_probe(struct platform_device 
*pdev)
sw_desc.allow_userspace_control = true,
sw_desc.fwnode = software_node_fwnode(&intel_xhci_usb_node);
 
+   data->enable_sw_switch = !device_property_read_bool(dev,
+   "sw_switch_disable");
+
data->role_sw = usb_role_switch_register(dev, &sw_desc);
if (IS_ERR(data->role_sw)) {
fwnode_handle_put(sw_desc.fwnode);
-- 
1.9.1



Re: [PATCH] usb: musb: omap2430: Fix flakeyness enumerating when connected to a lapdock

2019-08-29 Thread Tony Lindgren
* Tony Lindgren  [190828 08:11]:
> With commit 594632efbb9a ("usb: musb: Adding musb support for OMAP4430")
> Looks like always calling otg_set_vbus() causes flakeyness enumerating when
> droid4 is connected to a lapdock. In this case lapdock runs in USB carkit
> type mode and feeds the VBUS.

I don't think this patch helps much actually, let's forget it for now.

Looks like the core reason for flakeyness enumerating devices is that
clocks are not idling on disconnect properly, and then the next
enumeration will fail. I'll debug more and will send a better patch
at some point.

Regards,

Tony


Re: usb-storage: Set virt_boundary_mask to avoid SG overflows

2019-08-29 Thread Alan Stern
Questions like this should always be CC'ed to the appropriate mailing 
list.  (And note that the mailing list does not accept messages in HTML 
format.)

On Thu, 29 Aug 2019, sandeep krishnaswamy wrote:

> Hi Alan,
> 
> 
> 
> We are using the Android 8.x (i.e Android ‘O’) with Android kernel 4.9.182
> (derived from open source Linux kernel 4.9.182)
> 
> 
> 
> Since Android does not support the NTFS by default, we have enabled NTFS
> support in Kernel & added changes in user space to mount NTFS USB drives
> (as part of Android Vold).
> 
> 
> 
> With the integration of K4.9.182, we have seen regression in read speed
> from NTFS mounted USB drive. FAT32 mounted USB drives read speeds are fine
> without any regression.
> 
> 
> 
> By reverting the commit -
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/usb/storage/scsiglue.c?h=v4.9.190&id=c685caf6e5d896472c67b348d23936c6dc479749
> , I could see read speed of NTFS mounted USB drives are fine.
> 
> 
> 
> Could you please help me to understand the commit and help me, how this
> commit could impact only on NTFS mounted USB drive but not with FAT32
> mounted USB drive ?

I have no idea how that commit could have affected read speeds for NTFS 
volumes.  Possibly it might cause the system to use unnecessary bounce 
buffers, but that seems unlikely to make such a large difference.

Furthermore, I now believe that the commit isn't necessary at all.  
We'll probably get rid of the blk_queue_virt_boundary() call in the
not-too-distant future.  If you want to understand better how it works,
you should ask people involved in maintaining the block layer.

Alan Stern



[PATCH RESEND] xhci: Prevent deadlock when xhci adapter breaks during init

2019-08-29 Thread Bill Kuzeja
The system can hit a deadlock if xhci adapter breaks while initializing. 
The deadlock is between two threads: thread 1 is tearing down the 
adapter and is stuck in usb_unlocked_disable_lpm waiting to lock the 
hcd->handwidth_mutex. Thread 2 is holding this mutex (while still trying 
to add a usb device), but is stuck in xhci_endpoint_reset waiting for a 
stop or config command to complete. A reboot is required to resolve. 

It turns out when calling xhci_queue_stop_endpoint and 
xhci_queue_configure_endpoint in xhci_endpoint_reset, the return code is
not checked for errors. If the timing is right and the adapter dies just
before either of these commands get issued, we hang indefinitely waiting 
for a completion on a command that didn't get issued.

This wasn't a problem before the following fix because we didn't send 
commands in xhci_endpoint_reset:

commit f5249461b504 ("xhci: Clear the host side toggle manually when endpoint 
is soft reset")

With the patch I am submitting, a duration test which breaks adapters 
during initialization (and which deadlocks with the standard kernel) runs 
without issue.

Fixes: f5249461b504 ("xhci: Clear the host side toggle manually when endpoint 
is soft reset")
Signed-off-by: Bill Kuzeja 
---
 drivers/usb/host/xhci.c | 22 +++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 248cd7a..835708d 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3132,7 +3132,16 @@ static void xhci_endpoint_reset(struct usb_hcd *hcd,
xhci_free_command(xhci, cfg_cmd);
goto cleanup;
}
-   xhci_queue_stop_endpoint(xhci, stop_cmd, udev->slot_id, ep_index, 0);
+
+   if (xhci_queue_stop_endpoint(xhci, stop_cmd, udev->slot_id,
+   ep_index, 0) < 0) {
+   spin_unlock_irqrestore(&xhci->lock, flags);
+   xhci_free_command(xhci, cfg_cmd);
+   xhci_warn(xhci, "%s: stop_cmd xhci_queue_stop_endpoint "
+   "returns error, exiting\n", __func__);
+   goto cleanup;
+   }
+
xhci_ring_cmd_db(xhci);
spin_unlock_irqrestore(&xhci->lock, flags);
 
@@ -3146,8 +3155,15 @@ static void xhci_endpoint_reset(struct usb_hcd *hcd,
   ctrl_ctx, ep_flag, ep_flag);
xhci_endpoint_copy(xhci, cfg_cmd->in_ctx, vdev->out_ctx, ep_index);
 
-   xhci_queue_configure_endpoint(xhci, cfg_cmd, cfg_cmd->in_ctx->dma,
- udev->slot_id, false);
+   if (xhci_queue_configure_endpoint(xhci, cfg_cmd, cfg_cmd->in_ctx->dma,
+ udev->slot_id, false) < 0) {
+   spin_unlock_irqrestore(&xhci->lock, flags);
+   xhci_free_command(xhci, cfg_cmd);
+   xhci_warn(xhci, "%s: cfg_cmd xhci_queue_configure_endpoint "
+   "returns error, exiting\n", __func__);
+   goto cleanup;
+   }
+
xhci_ring_cmd_db(xhci);
spin_unlock_irqrestore(&xhci->lock, flags);
 
-- 
1.8.3.1



Re: Lacie Rugged USB3-FW does not work with UAS

2019-08-29 Thread Julian Sikorski

W dniu 24.08.2019 o 09:08, Julian Sikorski pisze:

W dniu 23.08.2019 o 23:23, Oliver Neukum pisze:

Am Freitag, den 23.08.2019, 16:21 +0200 schrieb Julian Sikorski:


I did some further digging regarding whether this is a regression: the
quirk file on the laptop is from 15 July 2014. The machine is from ca.
May 2011. Looking through my earlier posts to linux-usb it appears that
the addition of the quirk is related to this thread:

https://marc.info/?l=linux-usb&m=140537519907935&w=2

At the same time, back in 2011, I reported that the drive was working
after some fixes:

https://marc.info/?l=linux-usb&m=132276407611433&w=2


Hi,

this is alarming. Was this physically the same drive? I am asking
because we have seen cases where two different devices were sold
under the same name.

Regards
Oliver


Hi,

I do indeed own two lacie rugged drives which do differ a bit. The older
one (which was definitely working without the need for the quirk) is at
work, I will bring it home and test it in a few days.
Having said that, it appears that July 2014 is about when uas was rolled
out to the public. So maybe the drive has worked using usb storage before.

Best regards,
Julian


Hi,

I have finally managed to try the second, older drive. It turns out that 
the USB IDs are different and that the older drive (059f:103e) does 
indeed appear to work with UAS whereas the newer one (059f:1031) does not.
I can now also confirm that I bought the newer drive in November 2013 
which means that the initial attempts of getting a drive to work from 
2011 must have been with the older (working) one. This makes a 
regression less likely. My educated guess is that the newer drive was 
working from November 2013 until July 2014 when linux 3.15 came out and 
uas rollout broke the drive, after which I added the quirk and have been 
using it since.
Below is the dmesg output from connecting and disconnecting both drives, 
older (working) first and newer (not working) second:


[  103.728860] usb 1-4: new full-speed USB device number 6 using xhci_hcd
[  103.933051] usb 1-4: device descriptor read/64, error -71
[  104.214040] usb 1-4: device descriptor read/64, error -71
[  104.494718] usb 1-4: new full-speed USB device number 7 using xhci_hcd
[  105.888012] usb 2-4: new SuperSpeed Gen 1 USB device number 2 using 
xhci_hcd
[  105.910020] usb 2-4: New USB device found, idVendor=059f, 
idProduct=103e, bcdDevice= 0.02
[  105.910023] usb 2-4: New USB device strings: Mfr=2, Product=3, 
SerialNumber=1

[  105.910025] usb 2-4: Product: Rugged USB 3
[  105.910027] usb 2-4: Manufacturer: LaCie
[  105.910029] usb 2-4: SerialNumber: ce0238914a4c000
[  105.960279] usb-storage 2-4:1.0: USB Mass Storage device detected
[  105.960654] scsi host12: usb-storage 2-4:1.0
[  105.960719] usbcore: registered new interface driver usb-storage
[  105.962877] usbcore: registered new interface driver uas
[  107.705420] scsi 12:0:0:0: Direct-Access ST950032 5AS 
 0002 PQ: 0 ANSI: 0
[  107.706014] sd 12:0:0:0: [sdb] 976773168 512-byte logical blocks: 
(500 GB/466 GiB)

[  107.706101] sd 12:0:0:0: Attached scsi generic sg1 type 0
[  107.706935] sd 12:0:0:0: [sdb] Write Protect is off
[  107.706939] sd 12:0:0:0: [sdb] Mode Sense: 23 00 00 00
[  107.707942] sd 12:0:0:0: [sdb] No Caching mode page found
[  107.707945] sd 12:0:0:0: [sdb] Assuming drive cache: write through
[  107.842540]  sdb: sdb1 sdb2
[  107.845196] sd 12:0:0:0: [sdb] Attached SCSI disk
[  347.637498] usb 2-4: USB disconnect, device number 2
[  362.208749] usb 2-4: new SuperSpeed Gen 1 USB device number 3 using 
xhci_hcd
[  362.230833] usb 2-4: New USB device found, idVendor=059f, 
idProduct=1061, bcdDevice= 0.01
[  362.230837] usb 2-4: New USB device strings: Mfr=2, Product=3, 
SerialNumber=1

[  362.230839] usb 2-4: Product: Rugged USB3-FW
[  362.230841] usb 2-4: Manufacturer: LaCie
[  362.230842] usb 2-4: SerialNumber: 157f928920fa
[  362.270100] scsi host12: uas
[  362.270720] scsi 12:0:0:0: Direct-Access LaCieRugged FW USB3 
 051E PQ: 0 ANSI: 6

[  362.271472] sd 12:0:0:0: Attached scsi generic sg1 type 0
[  362.280344] sd 12:0:0:0: [sdb] 1953525168 512-byte logical blocks: 
(1.00 TB/932 GiB)

[  362.280422] sd 12:0:0:0: [sdb] Write Protect is off
[  362.280423] sd 12:0:0:0: [sdb] Mode Sense: 43 00 00 00
[  362.280544] sd 12:0:0:0: [sdb] Write cache: enabled, read cache: 
enabled, doesn't support DPO or FUA
[  392.672691] sd 12:0:0:0: tag#29 uas_eh_abort_handler 0 uas-tag 1 
inflight: IN
[  392.672697] sd 12:0:0:0: tag#29 CDB: Report supported operation codes 
a3 0c 01 12 00 00 00 00 02 00 00 00

[  392.678304] scsi host12: uas_eh_device_reset_handler start
[  392.800099] usb 2-4: reset SuperSpeed Gen 1 USB device number 3 using 
xhci_hcd

[  392.848154] scsi host12: uas_eh_device_reset_handler success
[  422.875443] scsi host12: uas_eh_device_reset_handler start
[  422.875650] sd 12:0:0:0: tag#16 uas_zap_pending 0 uas-tag 1 inflight:
[  422.875654] sd 1

[PATCH v3] usb: typec: ucsi: add support for separate DP altmode devices

2019-08-29 Thread Ajay Gupta
From: Ajay Gupta 

CCGx controller used on NVIDIA GPU card has two separate display
altmode for two DP pin assignments. UCSI specification doesn't
prohibits using separate display altmode.

Current UCSI Type-C framework expects only one display altmode for
all DP pin assignment. This patch squashes two separate display
altmode into single altmode to support controllers with separate
display altmode. We first read all the alternate modes of connector
and then run through it to know if there are separate display
altmodes. If so, it prepares a new port altmode set after squashing
two or more separate altmodes into one.

Signed-off-by: Ajay Gupta 
---
Original discussion on this issue is at [1]

Change from v2->v3
- Fix review comments from Heikki
- Moved the function to squash altmode table to ucsi_ccg.c
- Moved updates to UCSI_SET_NEW_CAM and UCSI_GET_CURRENT_CAM
  to ucsi_ccg.c
- Modified ucsi.c to collect all the altmode first and then
  register

1. https://marc.info/?l=linux-usb&m=154905866830998&w=2

 drivers/usb/typec/ucsi/ucsi.c |  66 +--
 drivers/usb/typec/ucsi/ucsi.h |   8 ++
 drivers/usb/typec/ucsi/ucsi_ccg.c | 178 +-
 3 files changed, 239 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index ba288b964dc8..ca3fcbd95d57 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -346,12 +346,17 @@ static int ucsi_register_altmodes(struct ucsi_connector 
*con, u8 recipient)
int max_altmodes = UCSI_MAX_ALTMODES;
struct typec_altmode_desc desc;
struct ucsi_altmode alt[2];
+   struct ucsi_altmode orig[UCSI_MAX_ALTMODES];
+   struct ucsi_altmode updated[UCSI_MAX_ALTMODES];
+   struct ucsi *ucsi = con->ucsi;
+   bool multi_dp = false;
struct ucsi_control ctrl;
int num = 1;
int ret;
int len;
int j;
int i;
+   int k = 0;
 
if (!(con->ucsi->cap.features & UCSI_CAP_ALT_MODE_DETAILS))
return 0;
@@ -362,35 +367,72 @@ static int ucsi_register_altmodes(struct ucsi_connector 
*con, u8 recipient)
if (recipient == UCSI_RECIPIENT_CON)
max_altmodes = con->ucsi->cap.num_alt_modes;
 
+   memset(orig, 0, sizeof(orig));
+   memset(updated, 0, sizeof(updated));
+
+   /* First get all the alternate modes */
for (i = 0; i < max_altmodes;) {
memset(alt, 0, sizeof(alt));
UCSI_CMD_GET_ALTERNATE_MODES(ctrl, recipient, con->num, i, 1);
len = ucsi_run_command(con->ucsi, &ctrl, alt, sizeof(alt));
-   if (len <= 0)
+   /* We are collecting all altmodes first and then registering.
+* Some type-C device will return zero length data beyond last
+* alternate modes. We should not return if length is zero.
+*/
+   if (len < 0)
return len;
 
+   /* We got all altmodes, now break out and register them */
+   if (!len)
+   break;
+
/*
 * This code is requesting one alt mode at a time, but some PPMs
 * may still return two. If that happens both alt modes need be
-* registered and the offset for the next alt mode has to be
+* saved and the offset for the next alt mode has to be
 * incremented.
 */
num = len / sizeof(alt[0]);
i += num;
 
for (j = 0; j < num; j++) {
-   if (!alt[j].svid)
-   return 0;
-
-   memset(&desc, 0, sizeof(desc));
-   desc.vdo = alt[j].mid;
-   desc.svid = alt[j].svid;
-   desc.roles = TYPEC_PORT_DRD;
+   if (!alt[j].svid) {
+   /* break out of outer loop and register */
+   i = max_altmodes;
+   break;
+   }
+
+   orig[k].mid = alt[j].mid;
+   orig[k].svid = alt[j].svid;
+   k++;
+   }
+   }
 
-   ret = ucsi_register_altmode(con, &desc, recipient);
-   if (ret)
-   return ret;
+   /* Update the original altmode table as some ppms may report
+* multiple DP altmodes.
+*/
+   if (recipient == UCSI_RECIPIENT_CON && ucsi->ppm &&
+   ucsi->ppm->update_altmodes)
+   multi_dp = ucsi->ppm->update_altmodes(ucsi->ppm, orig, updated);
+
+   /* now register altmodes */
+   for (i = 0; i < max_altmodes; i++) {
+   memset(&desc, 0, sizeof(desc));
+   if (multi_dp && recipient == UCSI_RECIPIEN

[PATCH v2] usb: dwc3: gadget: Workaround Mirosoft's BESL check

2019-08-29 Thread Thinh Nguyen
While testing our host system using Microsoft's usb stack against our
gadget for various BESL values, we found an issue with their usb stack
when the recommended baseline BESL value is 0 (125us) or when the deep
BESL is 1 or less. The Windows host will issue a usb reset immediately
after it receives the extended BOS descriptor and the enumeration will
fail after a few attempts.

To keep compatibility with Microsoft's host usb stack, let's workaround
this issue by using the recommended baseline BESL of 1 (or 150us)
and clamp the deep BESL value within 2 to 15.

This was tested against Windows 10 build 18956.

Signed-off-by: Thinh Nguyen 
---
changes in v2:
- Add comment in code for the new change


 drivers/usb/dwc3/gadget.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 7b58e0e1e438..3754bffb378c 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2083,9 +2083,19 @@ static void dwc3_gadget_config_params(struct usb_gadget 
*g,
 
/* Recommended BESL */
if (!dwc->dis_enblslpm_quirk) {
-   params->besl_baseline = 0;
+   /*
+* If the recommended BESL baseline is 0 or if the BESL deep is
+* less than 2, Microsoft's Windows 10 host usb stack will issue
+* a usb reset immediately after it receives the extended BOS
+* descriptor and the enumeration will fail. To maintain
+* compatibility with the Windows' usb stack, let's set the
+* recommended BESL baseline to 1 and clamp the BESL deep to be
+* within 2 to 15.
+*/
+   params->besl_baseline = 1;
if (dwc->is_utmi_l1_suspend)
-   params->besl_deep = min_t(u8, dwc->hird_threshold, 15);
+   params->besl_deep =
+   clamp_t(u8, dwc->hird_threshold, 2, 15);
}
 
/* U1 Device exit Latency */
-- 
2.11.0



[PATCH v2 2/2] usb: gadget: net2280: Add workaround for AB chip Errata 11

2019-08-29 Thread Benjamin Herrenschmidt
The errata description is:

Workaround for Default Duration of LFPS Handshake Signaling for
Device-Initiated U1 Exit is too short.

The default duration of the LFPS handshake generated by USB3380 for a 
device-initiated U1-exit may not be
long enough for certain SuperSpeed downstream ports (SuperSpeed hubs/hosts) to 
recognize. This could lead
to USB3380 entering the recovery state pre-maturely and ending up in the 
SS.Inactive state.

I have observed various enumeration failures, seemingly related to
lost transactions or SETUP status phases on modern hosts (typically
thunderbolt capable systems) without this workaround.

Signed-off-by: Benjamin Herrenschmidt 
---

  - v2. Rebased on top of Felipe's tree

 drivers/usb/gadget/udc/net2280.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/usb/gadget/udc/net2280.c b/drivers/usb/gadget/udc/net2280.c
index e0191146ba22..51efee21915f 100644
--- a/drivers/usb/gadget/udc/net2280.c
+++ b/drivers/usb/gadget/udc/net2280.c
@@ -2269,6 +2269,16 @@ static void usb_reinit_338x(struct net2280 *dev)
val |= 0x3 << HOT_RX_RESET_TS2;
writel(val, &dev->llregs->ll_tsn_counters_3);
 
+   /*
+* AB errata. Errata 11. Workaround for Default Duration of LFPS
+* Handshake Signaling for Device-Initiated U1 Exit is too short.
+* Without this, various enumeration failures observed with
+* modern superspeed hosts.
+*/
+   val = readl(&dev->llregs->ll_lfps_timers_2);
+   writel((val & 0x) | LFPS_TIMERS_2_WORKAROUND_VALUE,
+  &dev->llregs->ll_lfps_timers_2);
+
/*
 * Set Recovery Idle to Recover bit:
 * - On SS connections, setting Recovery Idle to Recover Fmw improves




[PATCH v2 1/2] usb: gadget: net2280: Move all "ll" registers in one structure

2019-08-29 Thread Benjamin Herrenschmidt
The split into multiple structures of the "ll" register bank is
impractical. It makes it hard to add ll_lfps_timers_2 which is
at offset 0x794, which is outside of the existing "lfps" structure
and would require us to add yet another one.

Instead, move all the "ll" registers into a single usb338x_ll_regs
structure, and add ll_lfps_timers_2 while at it. It will be used
in a subsequent patch.

Signed-off-by: Benjamin Herrenschmidt 
---

 - v2. Rebased on top of Felipe's tree

 drivers/usb/gadget/udc/net2280.c | 28 ++---
 drivers/usb/gadget/udc/net2280.h |  3 ---
 include/linux/usb/usb338x.h  | 35 +++-
 3 files changed, 36 insertions(+), 30 deletions(-)

diff --git a/drivers/usb/gadget/udc/net2280.c b/drivers/usb/gadget/udc/net2280.c
index b6bbe2e448ba..e0191146ba22 100644
--- a/drivers/usb/gadget/udc/net2280.c
+++ b/drivers/usb/gadget/udc/net2280.c
@@ -2244,30 +2244,30 @@ static void usb_reinit_338x(struct net2280 *dev)
}
 
/* Hardware Defect and Workaround */
-   val = readl(&dev->ll_lfps_regs->ll_lfps_5);
+   val = readl(&dev->llregs->ll_lfps_5);
val &= ~(0xf << TIMER_LFPS_6US);
val |= 0x5 << TIMER_LFPS_6US;
-   writel(val, &dev->ll_lfps_regs->ll_lfps_5);
+   writel(val, &dev->llregs->ll_lfps_5);
 
-   val = readl(&dev->ll_lfps_regs->ll_lfps_6);
+   val = readl(&dev->llregs->ll_lfps_6);
val &= ~(0x << TIMER_LFPS_80US);
val |= 0x0100 << TIMER_LFPS_80US;
-   writel(val, &dev->ll_lfps_regs->ll_lfps_6);
+   writel(val, &dev->llregs->ll_lfps_6);
 
/*
 * AA_AB Errata. Issue 4. Workaround for SuperSpeed USB
 * Hot Reset Exit Handshake may Fail in Specific Case using
 * Default Register Settings. Workaround for Enumeration test.
 */
-   val = readl(&dev->ll_tsn_regs->ll_tsn_counters_2);
+   val = readl(&dev->llregs->ll_tsn_counters_2);
val &= ~(0x1f << HOT_TX_NORESET_TS2);
val |= 0x10 << HOT_TX_NORESET_TS2;
-   writel(val, &dev->ll_tsn_regs->ll_tsn_counters_2);
+   writel(val, &dev->llregs->ll_tsn_counters_2);
 
-   val = readl(&dev->ll_tsn_regs->ll_tsn_counters_3);
+   val = readl(&dev->llregs->ll_tsn_counters_3);
val &= ~(0x1f << HOT_RX_RESET_TS2);
val |= 0x3 << HOT_RX_RESET_TS2;
-   writel(val, &dev->ll_tsn_regs->ll_tsn_counters_3);
+   writel(val, &dev->llregs->ll_tsn_counters_3);
 
/*
 * Set Recovery Idle to Recover bit:
@@ -2276,10 +2276,10 @@ static void usb_reinit_338x(struct net2280 *dev)
 * - It is safe to set for all connection speeds; all chip revisions.
 * - R-M-W to leave other bits undisturbed.
 * - Reference PLX TT-7372
-*/
-   val = readl(&dev->ll_chicken_reg->ll_tsn_chicken_bit);
+   */
+   val = readl(&dev->llregs->ll_tsn_chicken_bit);
val |= BIT(RECOVERY_IDLE_TO_RECOVER_FMW);
-   writel(val, &dev->ll_chicken_reg->ll_tsn_chicken_bit);
+   writel(val, &dev->llregs->ll_tsn_chicken_bit);
 
INIT_LIST_HEAD(&dev->gadget.ep0->ep_list);
 
@@ -3669,12 +3669,6 @@ static int net2280_probe(struct pci_dev *pdev, const 
struct pci_device_id *id)
(base + 0x00b4);
dev->llregs = (struct usb338x_ll_regs __iomem *)
(base + 0x0700);
-   dev->ll_lfps_regs = (struct usb338x_ll_lfps_regs __iomem *)
-   (base + 0x0748);
-   dev->ll_tsn_regs = (struct usb338x_ll_tsn_regs __iomem *)
-   (base + 0x077c);
-   dev->ll_chicken_reg = (struct usb338x_ll_chi_regs __iomem *)
-   (base + 0x079c);
dev->plregs = (struct usb338x_pl_regs __iomem *)
(base + 0x0800);
usbstat = readl(&dev->usb->usbstat);
diff --git a/drivers/usb/gadget/udc/net2280.h b/drivers/usb/gadget/udc/net2280.h
index b65a797544d7..85d3ca1698ba 100644
--- a/drivers/usb/gadget/udc/net2280.h
+++ b/drivers/usb/gadget/udc/net2280.h
@@ -178,9 +178,6 @@ struct net2280 {
struct net2280_dep_regs __iomem *dep;
struct net2280_ep_regs  __iomem *epregs;
struct usb338x_ll_regs  __iomem *llregs;
-   struct usb338x_ll_lfps_regs __iomem *ll_lfps_regs;
-   struct usb338x_ll_tsn_regs  __iomem *ll_tsn_regs;
-   struct usb338x_ll_chi_regs  __iomem *ll_chicken_reg;
struct usb338x_pl_regs  __iomem *plregs;
 
struct dma_pool *requests;
diff --git a/include/linux/usb/usb338x.h b/include/linux/usb/usb338x.h
index 7189e3387bf9..20020c1336d5 100644
--- a/include/linux/usb/usb338x.h
+++ b/include/linux/usb/usb338x.h
@@ -113,7 +113,10 @@ struct usb338x_ll_regs {