Re: usbip port number limits

[Yuyang Du]

On Wed, Nov 22, 2017 at 09:38:27AM +0100, Juan Zea wrote:
> I don't come up with any more detail... anything I'm missing?
 
Hi Juan,

Thank you so much for your patience. Really appreciate it.

I came up with the following patch, with which at my side the
BUG doesn't happen again with both a (low speed ) mouse and a
(high speed) stick attached at the client over localhost server.

Would you be able to test it at your side?

I applied the patch right on top of the commit b891245bff7958
("usbip: vhci-hcd: Clean up the code by adding a new macro").
Anyway, the patch is pretty trivial.

Thanks,
Yuyang

--

diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
index 64c3860..732edaf 100644
--- a/drivers/usb/usbip/vhci_hcd.c
+++ b/drivers/usb/usbip/vhci_hcd.c
@@ -1112,7 +1112,6 @@ static int hcd_name_to_id(const char *name)
 static int vhci_setup(struct usb_hcd *hcd)
 {
struct vhci *vhci = *((void **)dev_get_platdata(hcd->self.controller));
-   hcd->self.sg_tablesize = ~0;
if (usb_hcd_is_primary_hcd(hcd)) {
vhci->vhci_hcd_hs = hcd_to_vhci_hcd(hcd);
vhci->vhci_hcd_hs->vhci = vhci;
--
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

Re: [PATCH] r8152: disable rx checksum offload on Dell TB dock

[Kai Heng Feng]

> On 23 Nov 2017, at 5:24 PM, Greg KH  wrote:
> 
> On Thu, Nov 23, 2017 at 04:53:41PM +0800, Kai Heng Feng wrote:
>> 
>> What I want to do here is to finding this connection:
>> Realtek r8153 <-> SMSC hub (USD ID: 0424:5537) <-> 
>> ASMedia XHCI controller (PCI ID: 1b21:1142).
>> 
>> Is there a safer way to do this?
> 
> Nope!  You can't do that at all from within a USB driver, sorry.  As you
> really should not care at all :)

Got it :)

The r8153 in Dell TB dock has version information, RTL_VER_05.
We can use it to check for workaround, but many working RTL_VER_05 devices
will also be affected.
Do you think it’s an acceptable compromise?

>> I have a r8153 <-> USB 3.0 dongle which work just fine. I can’t find any 
>> information to differentiate them. Hence I want to use the connection to
>> identify if r8153 is on a Dell TB dock.
> 
> Are you sure there is nothing different in the version or release number
> of the device?  'lsusb -v' shows the exact same information for both
> devices?

Yes. I attached `lsusb -v` for r8153 on Dell TB dock, on a RJ45 <-> USB 3.0 
dongle,
and on a RJ45 <-> USB Type-C dongle.

>> Yes. From what I know, ASMedia is working on it, but not sure how long it
>> will take. In the meantime, I’d like to workaround this issue for the users.
> 
> Again, it's a host controller bug, it should be fixed there, don't try
> to paper over the real issue in different individual drivers.
> 
> I think I've seen various patches on the linux-usb list for this
> controller already, have you tried them?

Yes. These patches are all in mainline Linux now.

>> Actually no.
>> I just plugged r8153 dongle into the same hub, surprisingly the issue
>> doesn’t happen in this scenario.
> 
> Then something seems to be wrong with the device itself, as that would
> be the same exact electrical/logical path, right?

I have no idea why externally plugged one doesn’t have this issue.
Maybe it’s related how it’s wired inside the Dell TB dock...

Kai-Heng



lsusb-a
Description: Binary data


lsusb-c
Description: Binary data


lsusb-dock
Description: Binary data

> thanks,
> 
> greg k-h

Re: [PATCH v7] xhci : AMD Promontory USB disable port support

[kbuild test robot]

Hi Joe,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on v4.14]
[also build test WARNING on next-20171122]
[cannot apply to usb/usb-testing]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Joe-Lee/xhci-AMD-Promontory-USB-disable-port-support/20171124-070642
config: i386-randconfig-x008-201747 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All warnings (new ones prefixed by >>):

   In file included from drivers/usb/host/ehci-hcd.c:110:0:
   drivers/usb/host/pci-quirks.h: In function 'usb_amd_pt_check_port':
>> drivers/usb/host/pci-quirks.h:27:48: warning: no return statement in 
>> function returning non-void [-Wreturn-type]
static inline int usb_amd_pt_check_port(struct device *device, int port) {}
   ^~
   In file included from include/uapi/linux/stddef.h:2:0,
from include/linux/stddef.h:5,
from include/uapi/linux/posix_types.h:5,
from include/uapi/linux/types.h:14,
from include/linux/types.h:6,
from include/linux/list.h:5,
from include/linux/module.h:9,
from drivers/usb/host/ehci-hcd.c:23:
   drivers/usb/host/ehci-hcd.c: At top level:
   include/linux/compiler.h:163:4: warning: '__f' is static but declared in 
inline function 'strcpy' which is not static
   __f = { \
   ^
   include/linux/compiler.h:155:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
  ^~
   include/linux/string.h:422:2: note: in expansion of macro 'if'
 if (p_size == (size_t)-1 && q_size == (size_t)-1)
 ^~
   include/linux/compiler.h:163:4: warning: '__f' is static but declared in 
inline function 'kmemdup' which is not static
   __f = { \
   ^
   include/linux/compiler.h:155:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
  ^~
   include/linux/string.h:412:2: note: in expansion of macro 'if'
 if (p_size < size)
 ^~
   include/linux/compiler.h:163:4: warning: '__f' is static but declared in 
inline function 'kmemdup' which is not static
   __f = { \
   ^
   include/linux/compiler.h:155:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
  ^~
   include/linux/string.h:410:2: note: in expansion of macro 'if'
 if (__builtin_constant_p(size) && p_size < size)
 ^~
   include/linux/compiler.h:163:4: warning: '__f' is static but declared in 
inline function 'memchr_inv' which is not static
   __f = { \
   ^
   include/linux/compiler.h:155:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
  ^~
   include/linux/string.h:401:2: note: in expansion of macro 'if'
 if (p_size < size)
 ^~
   include/linux/compiler.h:163:4: warning: '__f' is static but declared in 
inline function 'memchr_inv' which is not static
   __f = { \
   ^
   include/linux/compiler.h:155:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
  ^~
   include/linux/string.h:399:2: note: in expansion of macro 'if'
 if (__builtin_constant_p(size) && p_size < size)
 ^~
   include/linux/compiler.h:163:4: warning: '__f' is static but declared in 
inline function 'memchr' which is not static
   __f = { \
   ^
   include/linux/compiler.h:155:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
  ^~
   include/linux/string.h:390:2: note: in expansion of macro 'if'
 if (p_size < size)
 ^~
   include/linux/compiler.h:163:4: warning: '__f' is static but declared in 
inline function 'memchr' which is not static
   __f = { \
   ^
   include/linux/compiler.h:155:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
  ^~
   include/linux/string.h:388:2: note: in expansion of macro 'if'
 if (__builtin_constant_p(size) && p_size < size)
 ^~
   include/linux/compiler.h:163:4: warning: '__f' is static but declared in 
inline function 'memcmp' which is not static
   __f = { \
   ^
   include/linux/compiler.h:155:23: note: in expansion of macro '__trace_if'
#define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
   

Re: [PATCH] net: usb: hso.c: remove unneeded DRIVER_LICENSE #define

[Joe Perches]

On Thu, 2017-11-23 at 15:30 -0800, Joe Perches wrote:
> --- /dev/null 2017-11-23 06:19:12.943046739 -0800
> +++ single_use_module.pl  2017-11-23 15:23:11.729812156 -0800
> @@ -0,0 +1,15 @@
[]
> +$data =~ s~$var~$string~;

this needs to be:

$data =~ s~\b$var\b~$string~;
--
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

Re: [PATCH v2] typec: fusb302: Use dev_err during probe

[Guenter Roeck]

On 11/23/2017 03:03 PM, Mats Karrman wrote:

If probe fails, fusb302_debugfs_exit is called making it impossible
to view any logs so use normal dev_err for any error messages during
probe.

Signed-off-by: Mats Karrman 


Reviewed-by: Guenter Roeck 


---
  Changes since v1:
  - Driver is no longer in stageing.
  - Removed accidental change of gpio name.

  drivers/usb/typec/fusb302/fusb302.c | 17 -
  1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/typec/fusb302/fusb302.c 
b/drivers/usb/typec/fusb302/fusb302.c
index 72cb060..4ce1df2 100644
--- a/drivers/usb/typec/fusb302/fusb302.c
+++ b/drivers/usb/typec/fusb302/fusb302.c
@@ -1731,24 +1731,24 @@ static int init_gpio(struct fusb302_chip *chip)
chip->gpio_int_n = of_get_named_gpio(node, "fcs,int_n", 0);
if (!gpio_is_valid(chip->gpio_int_n)) {
ret = chip->gpio_int_n;
-   fusb302_log(chip, "cannot get named GPIO Int_N, ret=%d", ret);
+   dev_err(chip->dev, "cannot get named GPIO Int_N, ret=%d", ret);
return ret;
}
ret = devm_gpio_request(chip->dev, chip->gpio_int_n, "fcs,int_n");
if (ret < 0) {
-   fusb302_log(chip, "cannot request GPIO Int_N, ret=%d", ret);
+   dev_err(chip->dev, "cannot request GPIO Int_N, ret=%d", ret);
return ret;
}
ret = gpio_direction_input(chip->gpio_int_n);
if (ret < 0) {
-   fusb302_log(chip,
-   "cannot set GPIO Int_N to input, ret=%d", ret);
+   dev_err(chip->dev,
+   "cannot set GPIO Int_N to input, ret=%d", ret);
return ret;
}
ret = gpio_to_irq(chip->gpio_int_n);
if (ret < 0) {
-   fusb302_log(chip,
-   "cannot request IRQ for GPIO Int_N, ret=%d", ret);
+   dev_err(chip->dev,
+   "cannot request IRQ for GPIO Int_N, ret=%d", ret);
return ret;
}
chip->gpio_int_n_irq = ret;
@@ -1845,7 +1845,7 @@ static int fusb302_probe(struct i2c_client *client,
chip->tcpm_port = tcpm_register_port(>dev, >tcpc_dev);
if (IS_ERR(chip->tcpm_port)) {
ret = PTR_ERR(chip->tcpm_port);
-   fusb302_log(chip, "cannot register tcpm port, ret=%d", ret);
+   dev_err(dev, "cannot register tcpm port, ret=%d", ret);
goto destroy_workqueue;
}
  
@@ -1854,8 +1854,7 @@ static int fusb302_probe(struct i2c_client *client,

IRQF_ONESHOT | IRQF_TRIGGER_LOW,
"fsc_interrupt_int_n", chip);
if (ret < 0) {
-   fusb302_log(chip,
-   "cannot request IRQ for GPIO Int_N, ret=%d", ret);
+   dev_err(dev, "cannot request IRQ for GPIO Int_N, ret=%d", ret);
goto tcpm_unregister_port;
}
enable_irq_wake(chip->gpio_int_n_irq);



--
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

Re: [PATCH] net: usb: hso.c: remove unneeded DRIVER_LICENSE #define

[Joe Perches]

On Wed, 2017-11-22 at 18:12 +0100, Greg Kroah-Hartman wrote:
> On Wed, Nov 22, 2017 at 09:05:36AM -0800, Joe Perches wrote:
> > On Fri, 2017-11-17 at 15:19 +0100, Greg Kroah-Hartman wrote:
> > > There is no need to #define the license of the driver, just put it in
> > > the MODULE_LICENSE() line directly as a text string.
> > > 
> > > This allows tools that check that the module license matches the source
> > > code license to work properly, as there is no need to unwind the
> > > unneeded dereference.
> > 
> > []
> > > diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
> > 
> > []
> > > @@ -76,7 +76,6 @@
> > >  
> > >  #define MOD_AUTHOR   "Option Wireless"
> > >  #define MOD_DESCRIPTION  "USB High Speed Option driver"
> > > -#define MOD_LICENSE  "GPL"
> > >  
> > >  #define HSO_MAX_NET_DEVICES  10
> > >  #define HSO__MAX_MTU 2048
> > > @@ -3288,7 +3287,7 @@ module_exit(hso_exit);
> > >  
> > >  MODULE_AUTHOR(MOD_AUTHOR);
> > >  MODULE_DESCRIPTION(MOD_DESCRIPTION);
> > > -MODULE_LICENSE(MOD_LICENSE);
> > > +MODULE_LICENSE("GPL");
> > 
> > Probably all of these MODULE_(MOD_) uses could be
> > simplified as well.
> 
> Agreed, I did that for a bunch of USB drivers, need to do it for others
> as well.

Here's a little perl and bash script that seems to do the right thing
---
--- /dev/null   2017-11-23 06:19:12.943046739 -0800
+++ single_use_module.pl2017-11-23 15:23:11.729812156 -0800
@@ -0,0 +1,15 @@
+$/ = undef;
+my $var = $ARGV[0];
+my $file = $ARGV[1];
+print("var: <$var> file: <$file>\n");
+open my $fh, "<", $file or die;
+my $data = <$fh>;
+close $fh;
+$data =~ s/\n#[ \t]*define\s+$var\s+(.*)\n/\n/;
+my $string = $1;
+print("string: <$string>\n");
+$string =~ s/\s+\n//;
+$data =~ s~$var~$string~;
+open my $fh, ">", $file or die;
+print $fh $data;
+close $fh;

--- /dev/null   2017-11-23 06:19:12.943046739 -0800
+++ single_use_module.bash  2017-11-23 15:23:01.964676948 -0800
@@ -0,0 +1,25 @@
+#!/bin/bash
+git grep -P '\bMODULE_[A-Z]+\s*\(\s*[A-Z_]+\s*\)' $@ |
+while read line ; do
+   file=$(echo $line | cut -f1 -d":")
+   define=$(echo $line | cut -f2- -d":")
+   var=$(echo $define | sed -r -e 's/^MODULE_[A-Z_]+\s*\(\s*//' -r -e 
's/^([A-Z_]+).*$/\1/')
+
+   # see if the define exists in the file
+   count1=$(git grep -c -P "^\s*#\s*define\s+$var\s+" $file | cut -f2- 
-d":")
+   if [[ $count1 != 1 ]] ; then
+   continue
+   fi
+
+   # see if the var exists twice (once in the #define, once in the use)
+   count2=$(git grep -c -P -w $var $file | cut -f2- -d":")
+   if [[ $count2 != 2 ]] ; then
+   continue
+   fi
+
+   if [[ "${defline: -1}" == "\\" ]] ; then
+   continue
+   fi
+
+   perl single_use_module.pl $var $file
+done

--
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

[PATCH v2] typec: fusb302: Use dev_err during probe

[Mats Karrman]

If probe fails, fusb302_debugfs_exit is called making it impossible
to view any logs so use normal dev_err for any error messages during
probe.

Signed-off-by: Mats Karrman 
---
 Changes since v1:
 - Driver is no longer in stageing.
 - Removed accidental change of gpio name.

 drivers/usb/typec/fusb302/fusb302.c | 17 -
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/typec/fusb302/fusb302.c 
b/drivers/usb/typec/fusb302/fusb302.c
index 72cb060..4ce1df2 100644
--- a/drivers/usb/typec/fusb302/fusb302.c
+++ b/drivers/usb/typec/fusb302/fusb302.c
@@ -1731,24 +1731,24 @@ static int init_gpio(struct fusb302_chip *chip)
chip->gpio_int_n = of_get_named_gpio(node, "fcs,int_n", 0);
if (!gpio_is_valid(chip->gpio_int_n)) {
ret = chip->gpio_int_n;
-   fusb302_log(chip, "cannot get named GPIO Int_N, ret=%d", ret);
+   dev_err(chip->dev, "cannot get named GPIO Int_N, ret=%d", ret);
return ret;
}
ret = devm_gpio_request(chip->dev, chip->gpio_int_n, "fcs,int_n");
if (ret < 0) {
-   fusb302_log(chip, "cannot request GPIO Int_N, ret=%d", ret);
+   dev_err(chip->dev, "cannot request GPIO Int_N, ret=%d", ret);
return ret;
}
ret = gpio_direction_input(chip->gpio_int_n);
if (ret < 0) {
-   fusb302_log(chip,
-   "cannot set GPIO Int_N to input, ret=%d", ret);
+   dev_err(chip->dev,
+   "cannot set GPIO Int_N to input, ret=%d", ret);
return ret;
}
ret = gpio_to_irq(chip->gpio_int_n);
if (ret < 0) {
-   fusb302_log(chip,
-   "cannot request IRQ for GPIO Int_N, ret=%d", ret);
+   dev_err(chip->dev,
+   "cannot request IRQ for GPIO Int_N, ret=%d", ret);
return ret;
}
chip->gpio_int_n_irq = ret;
@@ -1845,7 +1845,7 @@ static int fusb302_probe(struct i2c_client *client,
chip->tcpm_port = tcpm_register_port(>dev, >tcpc_dev);
if (IS_ERR(chip->tcpm_port)) {
ret = PTR_ERR(chip->tcpm_port);
-   fusb302_log(chip, "cannot register tcpm port, ret=%d", ret);
+   dev_err(dev, "cannot register tcpm port, ret=%d", ret);
goto destroy_workqueue;
}
 
@@ -1854,8 +1854,7 @@ static int fusb302_probe(struct i2c_client *client,
IRQF_ONESHOT | IRQF_TRIGGER_LOW,
"fsc_interrupt_int_n", chip);
if (ret < 0) {
-   fusb302_log(chip,
-   "cannot request IRQ for GPIO Int_N, ret=%d", ret);
+   dev_err(dev, "cannot request IRQ for GPIO Int_N, ret=%d", ret);
goto tcpm_unregister_port;
}
enable_irq_wake(chip->gpio_int_n_irq);
-- 
2.7.4

--
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

Re: [PATCH] typec: fusb302: Use dev_err during probe

[Mats Karrman]

On 2017-11-23 23:12, Guenter Roeck wrote:

On 11/23/2017 01:41 PM, Mats Karrman wrote:

If probe fails, fusb302_debugfs_exit is called making it impossible
to view any logs so use normal dev_err for any error messages during
probe.

Signed-off-by: Mats Karrman 
---
  drivers/staging/typec/fusb302/fusb302.c | 21 +++--
  1 file changed, 11 insertions(+), 10 deletions(-)



The driver is now out of staging.

OK, missed that.
Which is the best/proper tree to base it on, kernel/git/gregkh/usb.git?


diff --git a/drivers/staging/typec/fusb302/fusb302.c 
b/drivers/staging/typec/fusb302/fusb302.c

index fc6a3cf..a9903a2 100644
--- a/drivers/staging/typec/fusb302/fusb302.c

-- 8< --

-    fusb302_log(chip,
-    "cannot request IRQ for GPIO Int_N, ret=%d", ret);
+    dev_err(>dev,
+    "cannot request IRQ for GPIO Int_N, ret=%d", ret);


You can use the 'dev' variable in the probe function.

Will do.


--
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

Re: [PATCH] typec: fusb302: Use dev_err during probe

[Guenter Roeck]

On 11/23/2017 01:41 PM, Mats Karrman wrote:

If probe fails, fusb302_debugfs_exit is called making it impossible
to view any logs so use normal dev_err for any error messages during
probe.

Signed-off-by: Mats Karrman 
---
  drivers/staging/typec/fusb302/fusb302.c | 21 +++--
  1 file changed, 11 insertions(+), 10 deletions(-)



The driver is now out of staging.


diff --git a/drivers/staging/typec/fusb302/fusb302.c 
b/drivers/staging/typec/fusb302/fusb302.c
index fc6a3cf..a9903a2 100644
--- a/drivers/staging/typec/fusb302/fusb302.c
+++ b/drivers/staging/typec/fusb302/fusb302.c
@@ -1737,27 +1737,27 @@ static int init_gpio(struct fusb302_chip *chip)
int ret = 0;
  
  	node = chip->dev->of_node;

-   chip->gpio_int_n = of_get_named_gpio(node, "fcs,int_n", 0);
+   chip->gpio_int_n = of_get_named_gpio(node, "fcs,int_n-gpios", 0);
if (!gpio_is_valid(chip->gpio_int_n)) {
ret = chip->gpio_int_n;
-   fusb302_log(chip, "cannot get named GPIO Int_N, ret=%d", ret);
+   dev_err(chip->dev, "cannot get named GPIO Int_N, ret=%d", ret);
return ret;
}
ret = devm_gpio_request(chip->dev, chip->gpio_int_n, "fcs,int_n");
if (ret < 0) {
-   fusb302_log(chip, "cannot request GPIO Int_N, ret=%d", ret);
+   dev_err(chip->dev, "cannot request GPIO Int_N, ret=%d", ret);
return ret;
}
ret = gpio_direction_input(chip->gpio_int_n);
if (ret < 0) {
-   fusb302_log(chip,
-   "cannot set GPIO Int_N to input, ret=%d", ret);
+   dev_err(chip->dev,
+   "cannot set GPIO Int_N to input, ret=%d", ret);
return ret;
}
ret = gpio_to_irq(chip->gpio_int_n);
if (ret < 0) {
-   fusb302_log(chip,
-   "cannot request IRQ for GPIO Int_N, ret=%d", ret);
+   dev_err(chip->dev,
+   "cannot request IRQ for GPIO Int_N, ret=%d", ret);
return ret;
}
chip->gpio_int_n_irq = ret;
@@ -1854,7 +1854,8 @@ static int fusb302_probe(struct i2c_client *client,
chip->tcpm_port = tcpm_register_port(>dev, >tcpc_dev);
if (IS_ERR(chip->tcpm_port)) {
ret = PTR_ERR(chip->tcpm_port);
-   fusb302_log(chip, "cannot register tcpm port, ret=%d", ret);
+   dev_err(>dev,
+   "cannot register tcpm port, ret=%d", ret);
goto destroy_workqueue;
}
  
@@ -1863,8 +1864,8 @@ static int fusb302_probe(struct i2c_client *client,

IRQF_ONESHOT | IRQF_TRIGGER_LOW,
"fsc_interrupt_int_n", chip);
if (ret < 0) {
-   fusb302_log(chip,
-   "cannot request IRQ for GPIO Int_N, ret=%d", ret);
+   dev_err(>dev,
+   "cannot request IRQ for GPIO Int_N, ret=%d", ret);


You can use the 'dev' variable in the probe function.


goto tcpm_unregister_port;
}
enable_irq_wake(chip->gpio_int_n_irq);



--
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

[PATCH] typec: fusb302: Use dev_err during probe

[Mats Karrman]

If probe fails, fusb302_debugfs_exit is called making it impossible
to view any logs so use normal dev_err for any error messages during
probe.

Signed-off-by: Mats Karrman 
---
 drivers/staging/typec/fusb302/fusb302.c | 21 +++--
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/typec/fusb302/fusb302.c 
b/drivers/staging/typec/fusb302/fusb302.c
index fc6a3cf..a9903a2 100644
--- a/drivers/staging/typec/fusb302/fusb302.c
+++ b/drivers/staging/typec/fusb302/fusb302.c
@@ -1737,27 +1737,27 @@ static int init_gpio(struct fusb302_chip *chip)
int ret = 0;
 
node = chip->dev->of_node;
-   chip->gpio_int_n = of_get_named_gpio(node, "fcs,int_n", 0);
+   chip->gpio_int_n = of_get_named_gpio(node, "fcs,int_n-gpios", 0);
if (!gpio_is_valid(chip->gpio_int_n)) {
ret = chip->gpio_int_n;
-   fusb302_log(chip, "cannot get named GPIO Int_N, ret=%d", ret);
+   dev_err(chip->dev, "cannot get named GPIO Int_N, ret=%d", ret);
return ret;
}
ret = devm_gpio_request(chip->dev, chip->gpio_int_n, "fcs,int_n");
if (ret < 0) {
-   fusb302_log(chip, "cannot request GPIO Int_N, ret=%d", ret);
+   dev_err(chip->dev, "cannot request GPIO Int_N, ret=%d", ret);
return ret;
}
ret = gpio_direction_input(chip->gpio_int_n);
if (ret < 0) {
-   fusb302_log(chip,
-   "cannot set GPIO Int_N to input, ret=%d", ret);
+   dev_err(chip->dev,
+   "cannot set GPIO Int_N to input, ret=%d", ret);
return ret;
}
ret = gpio_to_irq(chip->gpio_int_n);
if (ret < 0) {
-   fusb302_log(chip,
-   "cannot request IRQ for GPIO Int_N, ret=%d", ret);
+   dev_err(chip->dev,
+   "cannot request IRQ for GPIO Int_N, ret=%d", ret);
return ret;
}
chip->gpio_int_n_irq = ret;
@@ -1854,7 +1854,8 @@ static int fusb302_probe(struct i2c_client *client,
chip->tcpm_port = tcpm_register_port(>dev, >tcpc_dev);
if (IS_ERR(chip->tcpm_port)) {
ret = PTR_ERR(chip->tcpm_port);
-   fusb302_log(chip, "cannot register tcpm port, ret=%d", ret);
+   dev_err(>dev,
+   "cannot register tcpm port, ret=%d", ret);
goto destroy_workqueue;
}
 
@@ -1863,8 +1864,8 @@ static int fusb302_probe(struct i2c_client *client,
IRQF_ONESHOT | IRQF_TRIGGER_LOW,
"fsc_interrupt_int_n", chip);
if (ret < 0) {
-   fusb302_log(chip,
-   "cannot request IRQ for GPIO Int_N, ret=%d", ret);
+   dev_err(>dev,
+   "cannot request IRQ for GPIO Int_N, ret=%d", ret);
goto tcpm_unregister_port;
}
enable_irq_wake(chip->gpio_int_n_irq);
-- 
2.7.4

--
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

Re: [PATCH] ARM: dts: bcm283x: Fix fifo size for EP 6,7

[Stefan Wahren]

Hi Minas,

> Minas Harutyunyan  hat am 23. November 2017 
> um 11:00 geschrieben:
> 
> 
> Hi Stefan,
> 
> ...
> 
> In addition to above patches please apply this one:
> 
> diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
> index 42ac47f85bb4..7db50c27c061 100644
> --- a/drivers/usb/dwc2/core.c
> +++ b/drivers/usb/dwc2/core.c
> @@ -433,6 +433,14 @@ void dwc2_force_mode(struct dwc2_hsotg *hsotg, bool 
> host)
>  dwc2_writel(gusbcfg, hsotg->regs + GUSBCFG);
> 
>  dwc2_wait_for_mode(hsotg, host);
> +
> +   /* Reset after mode changed. Required to restore
> +* 'power on reset' values for chosen mode
> +*/
> +   if (dwc2_core_reset(hsotg, true))
> +   dev_err(hsotg->dev,
> +   "%s: Reset failed, aborting", __func__);
> +
>  return;
>   }
> 

now i get this output in the unconnected case:

[2.203265] dwc2 2098.usb: dwc2_check_params: Invalid parameter 
host_perio_tx_fifo_size=0
[2.223595] dwc2 2098.usb: EPs: 8, dedicated fifos, 4080 entries in SPRAM
[2.237313] dwc2 2098.usb: DCFG=0x0020, DCTL=0x, 
DIEPMSK=
[2.251241] dwc2 2098.usb: GAHBCFG=0x, GHWCFG1=0x
[2.264092] dwc2 2098.usb: GRXFSIZ=0x1000, GNPTXFSIZ=0x00201000
[2.276997] dwc2 2098.usb: DPTx[1] FSize=512, StAddr=0x1020
[2.289533] dwc2 2098.usb: DPTx[2] FSize=512, StAddr=0x1220
[2.301811] dwc2 2098.usb: DPTx[3] FSize=512, StAddr=0x1420
[2.313895] dwc2 2098.usb: DPTx[4] FSize=512, StAddr=0x1620
[2.325905] dwc2 2098.usb: DPTx[5] FSize=512, StAddr=0x1820
[2.337737] dwc2 2098.usb: DPTx[6] FSize=768, StAddr=0x1a20
[2.349314] dwc2 2098.usb: DPTx[7] FSize=768, StAddr=0x1d20
[2.360658] dwc2 2098.usb: ep0-in: EPCTL=0x8800, SIZ=0x, 
DMA=0xc7cc399d
[2.373898] dwc2 2098.usb: ep0-out: EPCTL=0x8000, SIZ=0x, 
DMA=0x4369542a
[2.391729] dwc2 2098.usb: ep1-in: EPCTL=0x1000, SIZ=0x, 
DMA=0x3cf1e8ff
[2.405310] dwc2 2098.usb: ep1-out: EPCTL=0x, SIZ=0x, 
DMA=0xfee68e5b
[2.424087] dwc2 2098.usb: ep2-in: EPCTL=0x1800, SIZ=0x, 
DMA=0x250c46a5
[2.438198] dwc2 2098.usb: ep2-out: EPCTL=0x, SIZ=0x, 
DMA=0x09126d05
[2.458271] dwc2 2098.usb: ep3-in: EPCTL=0x2000, SIZ=0x, 
DMA=0xad3adfdb
[2.472553] dwc2 2098.usb: ep3-out: EPCTL=0x, SIZ=0x, 
DMA=0x975be477
[2.492567] dwc2 2098.usb: ep4-in: EPCTL=0x2800, SIZ=0x, 
DMA=0x4b1c0566
[2.506976] dwc2 2098.usb: ep4-out: EPCTL=0x, SIZ=0x, 
DMA=0x1790a498
[2.527472] dwc2 2098.usb: ep5-in: EPCTL=0x3000, SIZ=0x, 
DMA=0xd73f7ded
[2.541993] dwc2 2098.usb: ep5-out: EPCTL=0x, SIZ=0x, 
DMA=0x37bc32ff
[2.562397] dwc2 2098.usb: ep6-in: EPCTL=0x3800, SIZ=0x, 
DMA=0xc6ac6b1e
[2.576850] dwc2 2098.usb: ep6-out: EPCTL=0x, SIZ=0x, 
DMA=0xdd2f97d1
[2.597308] dwc2 2098.usb: ep7-in: EPCTL=0x4000, SIZ=0x, 
DMA=0xeb84b4fb
[2.612023] dwc2 2098.usb: ep7-out: EPCTL=0x, SIZ=0x, 
DMA=0xfadeb4e6
[2.633447] dwc2 2098.usb: DVBUSDIS=0x17d7, DVBUSPULSE=05b8
[2.647954] dwc2 2098.usb: DWC OTG Controller
[2.660187] dwc2 2098.usb: new USB bus registered, assigned bus number 1
[2.674855] dwc2 2098.usb: irq 33, io mem 0x2098
[2.689352] hub 1-0:1.0: USB hub found
[2.700837] hub 1-0:1.0: 1 port detected
[2.716013] dwc2 2098.usb: Mode Mismatch Interrupt: currently in Device 
mode

and this in connected case:

[2.203259] dwc2 2098.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
parameter g_tx_fifo_size[6]=768
[2.224317] dwc2 2098.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
parameter g_tx_fifo_size[7]=768
[2.245984] dwc2 2098.usb: EPs: 8, dedicated fifos, 4080 entries in SPRAM
[2.260139] dwc2 2098.usb: DCFG=0x, DCTL=0x, 
DIEPMSK=
[2.274290] dwc2 2098.usb: GAHBCFG=0x, GHWCFG1=0x
[2.287245] dwc2 2098.usb: GRXFSIZ=0x1000, GNPTXFSIZ=0x01001000
[2.300289] dwc2 2098.usb: DPTx[1] FSize=512, StAddr=0x2000
[2.313124] dwc2 2098.usb: DPTx[2] FSize=512, StAddr=0x2000
[2.325661] dwc2 2098.usb: DPTx[3] FSize=512, StAddr=0x2000
[2.338103] dwc2 2098.usb: DPTx[4] FSize=512, StAddr=0x2000
[2.350176] dwc2 2098.usb: DPTx[5] FSize=512, StAddr=0x2000
[2.361982] dwc2 2098.usb: DPTx[6] FSize=512, StAddr=0x2000
[2.373877] dwc2 2098.usb: DPTx[7] FSize=512, StAddr=0x2000
[2.385304] dwc2 2098.usb: ep0-in: EPCTL=0x0800, SIZ=0x, 
DMA=0xc7cc3f81
[2.398589] dwc2 2098.usb: ep0-out: EPCTL=0x0800, SIZ=0x, 
DMA=0xc7cc3f81
[

Re: [PATCHv2] USB: usbfs: Filter flags passed in from user space

[Alan Stern]

On Thu, 23 Nov 2017, Oliver Neukum wrote:

> USBDEVFS_URB_ISO_ASAP must be accepted only for ISO endpoints.
> Improve sanity checking.
> 
> Reported-by: andreyk...@google.com

This should be

Reported-by: Andrey Konovalov 

> Signed-off-by: Oliver Neukum 
> CC: sta...@vger.kernel.org
> ---
>  drivers/usb/core/devio.c | 16 ++--
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
> index 705c573d0257..701ddada389a 100644
> --- a/drivers/usb/core/devio.c
> +++ b/drivers/usb/core/devio.c
> @@ -1442,14 +1442,18 @@ static int proc_do_submiturb(struct usb_dev_state 
> *ps, struct usbdevfs_urb *uurb
>   int number_of_packets = 0;
>   unsigned int stream_id = 0;
>   void *buf;
> -
> - if (uurb->flags & ~(USBDEVFS_URB_ISO_ASAP |
> - USBDEVFS_URB_SHORT_NOT_OK |
> + unsigned long mask =USBDEVFS_URB_SHORT_NOT_OK |
>   USBDEVFS_URB_BULK_CONTINUATION |
>   USBDEVFS_URB_NO_FSBR |
> - USBDEVFS_URB_ZERO_PACKET |
> - USBDEVFS_URB_NO_INTERRUPT))
> - return -EINVAL;
> + USBDEVFS_URB_ZERO_PACKET | 

Extra whitespace at end of line (doesn't checkpatch.pl catch this)?

> + USBDEVFS_URB_NO_INTERRUPT;
> + /* USBDEVFS_URB_ISO_ASAP is a special case */
> + if (uurb->type == USBDEVFS_URB_TYPE_ISO)
> + mask |= USBDEVFS_URB_ISO_ASAP;
> +
> + if (uurb->flags & ~mask)
> + return -EINVAL;
> +
>   if ((unsigned int)uurb->buffer_length >= USBFS_XFER_MAX)
>   return -EINVAL;
>   if (uurb->buffer_length > 0 && !uurb->buffer)

Aside from these issues:

Acked-by: Alan Stern 

Alan Stern

--
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

usbutils 009 release

[Greg KH]

The 009 release of usbutils happened about a month ago, but I forgot to
announce it anywhere, and also forgot to upload the tarball to
kernel.org until today.

So here's the belated announcement of the 009 release of usbutils.  It's
been a few years since the last release, sorry about that, didn't
realize that time had gone by so long.

Lots of small changes for new types of devices were added, and other
structural things were done as well.  Full details are in the shortlog
below.

The package can be downladed from kernel.org:
http://www.kernel.org/pub/linux/utils/usb/usbutils/

The source tree for usbutils can be found on both kernel.org and
github.com if you want to fork it and send us changes easier:
http://git.kernel.org/?p=linux/kernel/git/gregkh/usbutils.git
http://github.com/gregkh/usbutils/tree/master

thanks,

greg k-h

---

Bjørn Mork (1):
  usbreset: coding style

Emmanuele Bassi (1):
  Don't use C99-ism

Greg Kroah-Hartman (22):
  usbhid-dump: update submodule to latest version
  add usbreset.c example program
  update usbhid-dump to latest
  lsusb.py: Don't dump a trace dump if usb.ids is not present

Grueninger, Tobias (1):
  USB: usb-devices: Interface number can be a string

Heinrich Schuchardt (1):
  autogen.sh: checkout usbhid-dump

Jaejoong Kim (4):
  lsusb : add support for the Encoding Unit Desc for uvc 1.5 device
  lsusb: fix alignment for Video Streaming interface desc
  lsusb: parse additional control fileds in USB video control interfaces 
for UVC1.5
  lsusb: proper display hexadecimal value for UVC control interface

Jakub Wilk (1):
  Fix typos

Jo-Philipp Wich (1):
  usbreset.c: import usability improvements from OpenWrt

Justin McBride (2):
  Update lsusb.c
  Un-indent bVariableSize for Frame-Based Format descriptors

Kylie McClain (1):
  Makefile: install pkgconfig file to arch-dependent location

Mathias Nyman (2):
  lsusb: Allocate the BOS descriptor buffer dynamically
  lsusb: Add support for the USB 3.1 SuperSpeedPlus device capability desc

Muthu M (2):
  lsusb: Fix issue with lengthy string descriptors
  lsusb: Added support for Billboard Capability descriptor

Nikolai Kondrashov (2):
  Update usbhid-dump repo URL
  Update usbhid-dump to v1.4

Stephan Linz (7):
  travis-ci: add control files borrowed from libusb
  configure: remove summary about unused USE_ZLIB
  drop unused input file for usb.ids update script
  substitute usb.id location in lsusb Python script
  travis-ci: cleanup before second run
  travis-ci: rework travis-autogen.sh
  lsusb: remove unused variable procbususb

Tobias Klauser (4):
  lsusb: Report correct MaxPower for USB 3.0 devices
  lsusb: Request proper descriptor type for USB 3.1
  lsusb: Store link state descriptions without preceding space
  build: Request at least libusb 1.0.9

Torleiv Sundre (2):
  Added support for Platform Device Capability descriptor
  lsusb: change endianness of first three fields when printing UUID/GUIDs.

Vianney le Clément de Saint-Marcq (3):
  lsusb: Fix UVC STILL_IMAGE_FRAME descriptor
  lsusb: Fix UVC VideoStreaming interface header descriptor
  lsusb: Fix UVC OUTPUT_TERMINAL descriptor

Vincent Palatin (1):
  lsusb: print WebUSB platform descriptor


--
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

[PATCHv2] USB: usbfs: Filter flags passed in from user space

[Oliver Neukum]

USBDEVFS_URB_ISO_ASAP must be accepted only for ISO endpoints.
Improve sanity checking.

Reported-by: andreyk...@google.com
Signed-off-by: Oliver Neukum 
CC: sta...@vger.kernel.org
---
 drivers/usb/core/devio.c | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index 705c573d0257..701ddada389a 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -1442,14 +1442,18 @@ static int proc_do_submiturb(struct usb_dev_state *ps, 
struct usbdevfs_urb *uurb
int number_of_packets = 0;
unsigned int stream_id = 0;
void *buf;
-
-   if (uurb->flags & ~(USBDEVFS_URB_ISO_ASAP |
-   USBDEVFS_URB_SHORT_NOT_OK |
+   unsigned long mask =USBDEVFS_URB_SHORT_NOT_OK |
USBDEVFS_URB_BULK_CONTINUATION |
USBDEVFS_URB_NO_FSBR |
-   USBDEVFS_URB_ZERO_PACKET |
-   USBDEVFS_URB_NO_INTERRUPT))
-   return -EINVAL;
+   USBDEVFS_URB_ZERO_PACKET | 
+   USBDEVFS_URB_NO_INTERRUPT;
+   /* USBDEVFS_URB_ISO_ASAP is a special case */
+   if (uurb->type == USBDEVFS_URB_TYPE_ISO)
+   mask |= USBDEVFS_URB_ISO_ASAP;
+
+   if (uurb->flags & ~mask)
+   return -EINVAL;
+
if ((unsigned int)uurb->buffer_length >= USBFS_XFER_MAX)
return -EINVAL;
if (uurb->buffer_length > 0 && !uurb->buffer)
-- 
2.13.6

--
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

[PATCH] [stable only]USB: fix buffer overflows with parsing CDC headers

[Oliver Neukum]

Parsing CDC headers a buffer overflow cannot just be prevented
by checking that the remainder of the buffer is longer than minimum
length. The size of the fields to be parsed must be figured in, too.

In newer kernels this issue has been fixed at a central location with

commit 2e1c42391ff2556387b3cb6308b24f6f65619feb
Author: Greg Kroah-Hartman 
Date:   Thu Sep 21 16:58:48 2017 +0200

USB: core: harden cdc_parse_cdc_header

on anything older the parsing had not been centralised, so a separate
fix for each driver is necessary.

Signed-off-by: Oliver Neukum 
---
 drivers/net/usb/cdc_ether.c | 9 -
 drivers/usb/class/cdc-acm.c | 2 +-
 drivers/usb/class/cdc-wdm.c | 2 ++
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index d3920b54a92c..579500d69757 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -171,6 +171,8 @@ int usbnet_generic_cdc_bind(struct usbnet *dev, struct 
usb_interface *intf)
dev_dbg(>dev, "extra CDC header\n");
goto bad_desc;
}
+   if (len < sizeof(struct usb_cdc_header_desc))
+   break;
info->header = (void *) buf;
if (info->header->bLength != sizeof(*info->header)) {
dev_dbg(>dev, "CDC header len %u\n",
@@ -184,6 +186,8 @@ int usbnet_generic_cdc_bind(struct usbnet *dev, struct 
usb_interface *intf)
 */
if (rndis) {
struct usb_cdc_acm_descriptor *acm;
+   if (len < sizeof(struct usb_cdc_acm_descriptor))
+   break;
 
acm = (void *) buf;
if (acm->bmCapabilities) {
@@ -200,6 +204,8 @@ int usbnet_generic_cdc_bind(struct usbnet *dev, struct 
usb_interface *intf)
dev_dbg(>dev, "extra CDC union\n");
goto bad_desc;
}
+   if (len < sizeof(struct usb_cdc_union_desc))
+   break;
info->u = (void *) buf;
if (info->u->bLength != sizeof(*info->u)) {
dev_dbg(>dev, "CDC union len %u\n",
@@ -258,6 +264,8 @@ int usbnet_generic_cdc_bind(struct usbnet *dev, struct 
usb_interface *intf)
dev_dbg(>dev, "extra CDC ether\n");
goto bad_desc;
}
+   if (len < sizeof(struct usb_cdc_ether_desc))
+   break;
info->ether = (void *) buf;
if (info->ether->bLength != sizeof(*info->ether)) {
dev_dbg(>dev, "CDC ether len %u\n",
@@ -275,7 +283,6 @@ int usbnet_generic_cdc_bind(struct usbnet *dev, struct 
usb_interface *intf)
dev_dbg(>dev, "extra MDLM descriptor\n");
goto bad_desc;
}
-
desc = (void *)buf;
 
if (desc->bLength != sizeof(*desc))
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index d67484b3dcfa..a5e94b8fa2dc 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -1139,7 +1139,7 @@ static int acm_probe(struct usb_interface *intf,
}
}
 
-   while (buflen > 0) {
+   while (buflen >= 3) { /* minimum length making sense */
elength = buffer[0];
if (!elength) {
dev_err(>dev, "skipping garbage byte\n");
diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
index a81f9dd7ee97..df0878c4810c 100644
--- a/drivers/usb/class/cdc-wdm.c
+++ b/drivers/usb/class/cdc-wdm.c
@@ -891,6 +891,8 @@ static int wdm_probe(struct usb_interface *intf, const 
struct usb_device_id *id)
case USB_CDC_HEADER_TYPE:
break;
case USB_CDC_DMM_TYPE:
+   if (buflen < sizeof(struct usb_cdc_dmm_desc))
+   break;
dmhd = (struct usb_cdc_dmm_desc *)buffer;
maxcom = le16_to_cpu(dmhd->wMaxCommand);
dev_dbg(>dev,
-- 
2.13.6

--
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

Re: [PATCH] USB: usbfs: Filter flags passed in from user space

[Greg KH]

On Thu, Nov 23, 2017 at 04:06:28PM +0100, Oliver Neukum wrote:
> Am Donnerstag, den 23.11.2017, 15:57 +0100 schrieb Greg KH:
> > On Thu, Nov 23, 2017 at 03:28:34PM +0100, Oliver Neukum wrote:
> > > 
> > > USBDEVFS_URB_ISO_ASAP must be accepted only for ISO endpoints.
> > > Improve sanity checking.
> > > 
> > > Signed-off-by: Oliver Neukum 
> > > ---
> > >  drivers/usb/core/devio.c | 16 ++--
> > >  1 file changed, 10 insertions(+), 6 deletions(-)
> > 
> > Didn't we do something like this already?
> 
> I sent a patch. Alan found it insufficient.

Ah, ok.

> > Or was it something else?  Was this triggered by some testing?
> 
> Alexey's syzkaller tests showed that you can trigger it.

Then a nice "Reported-by:" would be good to add, as would a stable tree
inclusion, right?

thanks,

greg k-h
--
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

Re: [PATCH] USB: usbfs: Filter flags passed in from user space

[Oliver Neukum]

Am Donnerstag, den 23.11.2017, 15:57 +0100 schrieb Greg KH:
> On Thu, Nov 23, 2017 at 03:28:34PM +0100, Oliver Neukum wrote:
> > 
> > USBDEVFS_URB_ISO_ASAP must be accepted only for ISO endpoints.
> > Improve sanity checking.
> > 
> > Signed-off-by: Oliver Neukum 
> > ---
> >  drivers/usb/core/devio.c | 16 ++--
> >  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> Didn't we do something like this already?

I sent a patch. Alan found it insufficient.

> Or was it something else?  Was this triggered by some testing?

Alexey's syzkaller tests showed that you can trigger it.

Regards
Oliver

--
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

Re: [PATCH] USB: usbfs: Filter flags passed in from user space

[Greg KH]

On Thu, Nov 23, 2017 at 03:28:34PM +0100, Oliver Neukum wrote:
> USBDEVFS_URB_ISO_ASAP must be accepted only for ISO endpoints.
> Improve sanity checking.
> 
> Signed-off-by: Oliver Neukum 
> ---
>  drivers/usb/core/devio.c | 16 ++--
>  1 file changed, 10 insertions(+), 6 deletions(-)

Didn't we do something like this already?

Or was it something else?  Was this triggered by some testing?

thanks,

greg k-h
--
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

Re: [PATCH 1/2] usb: xhci: add relaxed timing quirk bit

[Adam Wallis]

On 11/23/2017 5:59 AM, Mathias Nyman wrote:
> On 23.11.2017 01:32, Adam Wallis wrote:
>> On 11/22/2017 10:24 AM, Mathias Nyman wrote:
>> [..]
>>>
>>> We know have at least two hosts/platforms that need custom interrupt 
>>> moderation
>>> values
>>>
>>> How about adding a u32 device property for xhci with the interrupt 
>>> moderation
>>> interval in
>>> nanoseconds?  And also add a u32 imod_interval variable to struct xhci_hcd?
>>>   imod_interval can be set to the current default 4ns (160*250ns) and
>>> overwritten if
>>> device_property_read_u32() returns something else.
>>>
>>
>> Isn't the 160 value quite aggressive anyway? Section 5.5.2.2 of the xHCI spec
>> says that maximum observable interrupt rate should never exceed 8000
>> interrupts/second. I believe the IMOD value in the most aggressive case would
>> then be 500 by this statement [ 1 / (250e-9 * 500) = 8000 irqs/second ]
>>
>> Perhaps I am misreading the spec or just doing the math wrong? With the 
>> default
>> value of 160, we are interrupting 25,000 irq/second...which is over 3 times 
>> the
>> maximum stated value (again, assuming I did the math right)
>>
>> Anyway, my preference would be to set the IMOD default val to 4000 (~1ms) per
>> the recommended value in Table 49 of the spec and allow platforms to adjust 
>> as
>> necessary from that point.
>>
>> Thoughts on this?
>>
> 
> I think current 40us is still reasonable, it's about ~3 times per
> usb microframe (125us) .8000 interrupts per second just covers the microframe 
> rate,
> which is the shortest interval a interrupt transfer can require service.
> 
> 1ms interrupt interval is too long. In the worst case ~8 microframes could 
> pass
> before the driver is aware of a error it needs to take care of.
> USB 3.1 devices can transfer 6 burst of 16 max sized packets (6 x 16 x 2014) 
> bytes
> per microframe.
> 
> closer to 125us could probably work as well, but unless we are fixing some 
> issue or
> getting some other bigger benefit out of it I don't think it's worth changing 
> it
> just to see if it breaks stuff.

I agree - my patch will just stick with the current default as the fallback
value if no device property is submitted.

> 
> Thanks
> -Mathias
> -- 
> 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


-- 
Adam Wallis
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
--
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

[PATCH] USB: usbfs: Filter flags passed in from user space

[Oliver Neukum]

USBDEVFS_URB_ISO_ASAP must be accepted only for ISO endpoints.
Improve sanity checking.

Signed-off-by: Oliver Neukum 
---
 drivers/usb/core/devio.c | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index 705c573d0257..701ddada389a 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -1442,14 +1442,18 @@ static int proc_do_submiturb(struct usb_dev_state *ps, 
struct usbdevfs_urb *uurb
int number_of_packets = 0;
unsigned int stream_id = 0;
void *buf;
-
-   if (uurb->flags & ~(USBDEVFS_URB_ISO_ASAP |
-   USBDEVFS_URB_SHORT_NOT_OK |
+   unsigned long mask =USBDEVFS_URB_SHORT_NOT_OK |
USBDEVFS_URB_BULK_CONTINUATION |
USBDEVFS_URB_NO_FSBR |
-   USBDEVFS_URB_ZERO_PACKET |
-   USBDEVFS_URB_NO_INTERRUPT))
-   return -EINVAL;
+   USBDEVFS_URB_ZERO_PACKET | 
+   USBDEVFS_URB_NO_INTERRUPT;
+   /* USBDEVFS_URB_ISO_ASAP is a special case */
+   if (uurb->type == USBDEVFS_URB_TYPE_ISO)
+   mask |= USBDEVFS_URB_ISO_ASAP;
+
+   if (uurb->flags & ~mask)
+   return -EINVAL;
+
if ((unsigned int)uurb->buffer_length >= USBFS_XFER_MAX)
return -EINVAL;
if (uurb->buffer_length > 0 && !uurb->buffer)
-- 
2.13.6

--
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

RE: [PATCH 2/2 v7] typec: tcpm: Only request matching pdos

[Adam Thomson]

On 16 November 2017 01:02, Badhri Jagan Sridharan wrote:

> At present, TCPM code assumes that local device supports
> variable/batt pdos and always selects the pdo with highest
> possible power within the board limit. This assumption
> might not hold good for all devices. To overcome this,
> this patch makes TCPM only accept a source_pdo when there is
> a matching sink pdo.
> 
> For Fixed pdos: The voltage should match between the
> incoming source_cap and the registered snk_pdo
> For Variable/Batt pdos: The incoming source_cap voltage
> range should fall within the registered snk_pdo's voltage
> range.
> 
> Also, when the cap_mismatch bit is set, the max_power/current
> should be set to the max_current/power of the sink_pdo.
> This is according to:
> 
> "If the Capability Mismatch bit is set to one
> The Maximum Operating Current/Power field may contain a value
> larger than the maximum current/power offered in the Source
> Capabilities message’s PDO as referenced by the Object position field.
> This enables the Sink to indicate that it requires more current/power
> than is being offered. If the Sink requires a different voltage this
> will be indicated by its Sink Capabilities message.

Hi Badhri,

I have some queries on this change. At the moment the src/snk PDOs are hard
coded in the relevant PD controller driver (e.g. fusb302.c), and the only way
to update these is to call the tcpm_update_source_capabilities() or
tcpm_update_sink_capabilities() functions. However there are no real world
examples in the kernel where this happens. Where are these functions likely to
be called from, as wherever that is it will need a reference to the port?
Actually should we be adding DT bindings to set supported src/snk PDOs from FW,
if you're taking this approach to PDO selection?

This patch also seemingly leaves 'max_snk_mv', 'max_snk_ma' and 'max_snk_mv'
redundant, although those values can be configured from a PD controller driver
(e.g. fusb302 actually supports DT bindings which allow these to be set through
FW). Now these DT bindings are basically made redundant by your change as they
have no impact. Are we expecting these to be used again in the future, or should
these be removed? FYI, as part of my PPS patch set I have been using them as
part of the PPS APDO selection criteria, based on TCPM code prior to your
modifications, as for PPS we're interested in a wide range of voltages and
currents but want to stay within the platforms limits.

> 
> Signed-off-by: Badhri Jagan Sridharan 
> Acked-by: Heikki Krogerus 
> Reviewed-by: Guenter Roeck 
> 
> ---
> Changelog since v1:
> - Rebased the patch on top of drivers/usb/type/tcpm.c
> - Added duplicate pdo check for variable/batt pdo.
> - Fixed tabs as suggested by dan.carpen...@oracle.com
> 
> Changelog since v2:
> - Rebase
> 
> Changelog since v3:
> - Refactored code fixed formatting issues as suggested
>   by heikki.kroge...@linux.intel.com
> 
> Changelog since v4:
> - Rebase
> 
> Changelog since v5:
> - handling the sink pdo index as pointer argument in tcpm_pd_select_pdo
> - ACK by heikki.kroge...@linux.intel.com
> 
> Changelog since v6:
> - Added Reviewed-by: Guenter Roeck 
> 
>  drivers/usb/typec/tcpm.c | 163 +++---
> -
>  1 file changed, 121 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
> index 8b637a4b474b..f4d563ee7690 100644
> --- a/drivers/usb/typec/tcpm.c
> +++ b/drivers/usb/typec/tcpm.c
> @@ -252,6 +252,9 @@ struct tcpm_port {
>   unsigned int nr_src_pdo;
>   u32 snk_pdo[PDO_MAX_OBJECTS];
>   unsigned int nr_snk_pdo;
> + unsigned int nr_fixed; /* number of fixed sink PDOs */
> + unsigned int nr_var; /* number of variable sink PDOs */
> + unsigned int nr_batt; /* number of battery sink PDOs */
>   u32 snk_vdo[VDO_MAX_OBJECTS];
>   unsigned int nr_snk_vdo;
> 
> @@ -1767,39 +1770,90 @@ static int tcpm_pd_check_request(struct tcpm_port
> *port)
>   return 0;
>  }
> 
> -static int tcpm_pd_select_pdo(struct tcpm_port *port)
> +#define min_power(x, y) min(pdo_max_power(x), pdo_max_power(y))
> +#define min_current(x, y) min(pdo_max_current(x), pdo_max_current(y))
> +
> +static int tcpm_pd_select_pdo(struct tcpm_port *port, int *sink_pdo,
> +   int *src_pdo)
>  {
> - unsigned int i, max_mw = 0, max_mv = 0;
> + unsigned int i, j, max_mw = 0, max_mv = 0, mw = 0, mv = 0, ma = 0;
>   int ret = -EINVAL;
> 
>   /*
> -  * Select the source PDO providing the most power while staying within
> -  * the board's voltage limits. Prefer PDO providing exp
> +  * Select the source PDO providing the most power which has a
> +  * matchig sink cap.
>*/
>   for (i = 0; i < port->nr_source_caps; i++) {
>   u32 pdo = port->source_caps[i];
>   enum pd_pdo_type type = 

Re: [PATCH 1/2] usb: xhci: add relaxed timing quirk bit

[Mathias Nyman]

On 23.11.2017 01:32, Adam Wallis wrote:

On 11/22/2017 10:24 AM, Mathias Nyman wrote:
[..]


We know have at least two hosts/platforms that need custom interrupt moderation
values

How about adding a u32 device property for xhci with the interrupt moderation
interval in
nanoseconds?  And also add a u32 imod_interval variable to struct xhci_hcd?
  
imod_interval can be set to the current default 4ns (160*250ns) and

overwritten if
device_property_read_u32() returns something else.



Isn't the 160 value quite aggressive anyway? Section 5.5.2.2 of the xHCI spec
says that maximum observable interrupt rate should never exceed 8000
interrupts/second. I believe the IMOD value in the most aggressive case would
then be 500 by this statement [ 1 / (250e-9 * 500) = 8000 irqs/second ]

Perhaps I am misreading the spec or just doing the math wrong? With the default
value of 160, we are interrupting 25,000 irq/second...which is over 3 times the
maximum stated value (again, assuming I did the math right)

Anyway, my preference would be to set the IMOD default val to 4000 (~1ms) per
the recommended value in Table 49 of the spec and allow platforms to adjust as
necessary from that point.

Thoughts on this?



I think current 40us is still reasonable, it's about ~3 times per
usb microframe (125us) .8000 interrupts per second just covers the microframe 
rate,
which is the shortest interval a interrupt transfer can require service.

1ms interrupt interval is too long. In the worst case ~8 microframes could pass
before the driver is aware of a error it needs to take care of.
USB 3.1 devices can transfer 6 burst of 16 max sized packets (6 x 16 x 2014) 
bytes
per microframe.

closer to 125us could probably work as well, but unless we are fixing some 
issue or
getting some other bigger benefit out of it I don't think it's worth changing it
just to see if it breaks stuff.

Thanks
-Mathias
--
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

Re: [PATCH] ARM: dts: bcm283x: Fix fifo size for EP 6,7

[Minas Harutyunyan]

Hi Stefan,

On 11/22/2017 6:15 PM, Stefan Wahren wrote:
> Hi Minas,
> 
>> Stefan Wahren  hat am 22. November 2017 um 12:21 
>> geschrieben:
>>
>>
>> Hi Minas,
>>
>>> Minas Harutyunyan  hat am 21. November 2017 
>>> um 13:02 geschrieben:
>>>
>>> Hi Stefan,
>>>
>>> We have prepared patch for this issue in July-August'17.
>>> Find attached 2 patch files. Please apply patches and test. If issue
>>> gone, we will send these patches to LKML by regular flow.
>>
>> thanks, but the first patch doesn't apply. My version see below, i hope i 
>> didn't break anything.
>>
>> Unfortunately after applying both patches the issue still persists (EP 1-7 
>> fifo size to 512) and the EP 1-7 TX total size increases from 3776 to 3792. 
>> I will follow my u-boot theory  ...
> 
> okay there is no influence of u-boot. If i boot directly from the RPi 
> Bootloader it shows the same behavior.
> 
> I did the following test cases with enabled debug:
> 1. boot RPI Zero (OTG configured per DT) without anything connected to the 
> OTG port
> 2. boot RPI Zero (OTG configured per DT) only a OTG cable connected to the 
> OTG port
> 
> test case 1 (nothing connected, no issue):
> dwc2 2098.usb: mapped PA 2098 to VA dc85
> dwc2 2098.usb: registering common handler for irq33
> dwc2 2098.usb: Core Release: 2.80a (snpsid=4f54280a)
> dwc2 2098.usb: Forcing mode to host
> dwc2 2098.usb: NonPeriodic TXFIFO size: 32
> dwc2 2098.usb: RXFIFO size: 256
> dwc2 2098.usb: EPs: 8, dedicated fifos, 4080 entries in SPRAM
> dwc2 2098.usb: DCFG=0x0020, DCTL=0x, DIEPMSK=
> dwc2 2098.usb: GAHBCFG=0x, GHWCFG1=0x
> dwc2 2098.usb: GRXFSIZ=0x1000, GNPTXFSIZ=0x00201000
> dwc2 2098.usb: DPTx[1] FSize=512, StAddr=0x1020
> dwc2 2098.usb: DPTx[2] FSize=512, StAddr=0x1220
> dwc2 2098.usb: DPTx[3] FSize=512, StAddr=0x1420
> dwc2 2098.usb: DPTx[4] FSize=512, StAddr=0x1620
> dwc2 2098.usb: DPTx[5] FSize=512, StAddr=0x1820
> dwc2 2098.usb: DPTx[6] FSize=768, StAddr=0x1a20
> dwc2 2098.usb: DPTx[7] FSize=768, StAddr=0x1d20
> ...
> dwc2 2098.usb: Core Global Registers
> dwc2 2098.usb: GOTGCTL@0xDC85 : 0x000D
> dwc2 2098.usb: GOTGINT@0xDC850004 : 0x
> dwc2 2098.usb: GAHBCFG@0xDC850008 : 0x0030
> dwc2 2098.usb: GUSBCFG@0xDC85000C : 0x1700
> dwc2 2098.usb: GRSTCTL@0xDC850010 : 0x8000
> dwc2 2098.usb: GINTSTS@0xDC850014 : 0x04008C22
> dwc2 2098.usb: GINTMSK@0xDC850018 : 0xD806
> dwc2 2098.usb: GRXSTSR@0xDC85001C : 0xFADEA4E0
> dwc2 2098.usb: GRXFSIZ@0xDC850024 : 0x1000
> dwc2 2098.usb: GNPTXFSIZ  @0xDC850028 : 0x00201000
> dwc2 2098.usb: GNPTXSTS   @0xDC85002C : 0x00080020
> dwc2 2098.usb: GI2CCTL@0xDC850030 : 0x
> dwc2 2098.usb: GPVNDCTL   @0xDC850034 : 0x
> dwc2 2098.usb: GGPIO  @0xDC850038 : 0x
> dwc2 2098.usb: GUID   @0xDC85003C : 0x2708A000
> dwc2 2098.usb: GSNPSID@0xDC850040 : 0x4F54280A
> dwc2 2098.usb: GHWCFG1@0xDC850044 : 0x
> dwc2 2098.usb: GHWCFG2@0xDC850048 : 0x228DDD50
> dwc2 2098.usb: GHWCFG3@0xDC85004C : 0x0FF000E8
> dwc2 2098.usb: GHWCFG4@0xDC850050 : 0x1FF00020
> dwc2 2098.usb: GLPMCFG@0xDC850054 : 0x75736230
> dwc2 2098.usb: GPWRDN @0xDC850058 : 0x
> dwc2 2098.usb: GDFIFOCFG  @0xDC85005C : 0x
> dwc2 2098.usb: HPTXFSIZ   @0xDC850100 : 0x
> dwc2 2098.usb: PCGCTL @0xDC850E00 : 0x
> ...
> dwc2 2098.usb: gintsts=04008c22  gintmsk=d806
> dwc2 2098.usb: Mode Mismatch Interrupt: currently in Device mode
> dwc2 2098.usb: USB SUSPEND
> dwc2 2098.usb: DSTS=0x3
> dwc2 2098.usb: DSTS.Suspend Status=1 HWCFG4.Power Optimize=0
> dwc2 2098.usb: dwc2_hsotg_irq: 04008420  (d806) retry 8
> 
> test case 2 (OTG cable connected, issue):
> dwc2 2098.usb: mapped PA 2098 to VA dc85
> dwc2 2098.usb: registering common handler for irq33
> dwc2 2098.usb: Core Release: 2.80a (snpsid=4f54280a)
> dwc2 2098.usb: Forcing mode to device
> dwc2 2098.usb: dwc2_check_param_tx_fifo_sizes: Invalid parameter 
> g_tx_fifo_size[6]=768
> dwc2 2098.usb: dwc2_check_param_tx_fifo_sizes: Invalid parameter 
> g_tx_fifo_size[7]=768
> dwc2 2098.usb: NonPeriodic TXFIFO size: 32
> dwc2 2098.usb: RXFIFO size: 256
> dwc2 2098.usb: EPs: 8, dedicated fifos, 4080 entries in SPRAM
> dwc2 2098.usb: DCFG=0x, DCTL=0x, DIEPMSK=
> dwc2 2098.usb: GAHBCFG=0x, GHWCFG1=0x
> dwc2 2098.usb: GRXFSIZ=0x1000, GNPTXFSIZ=0x01001000
> dwc2 2098.usb: DPTx[1] FSize=512, StAddr=0x2000
> dwc2 2098.usb: DPTx[2] FSize=512, 

Re: [PATCH] r8152: disable rx checksum offload on Dell TB dock

[Greg KH]

On Thu, Nov 23, 2017 at 04:53:41PM +0800, Kai Heng Feng wrote:
> 
> > On 23 Nov 2017, at 3:58 PM, Greg KH  wrote:
> > 
> > On Thu, Nov 23, 2017 at 01:38:38AM -0500, Kai-Heng Feng wrote:
> >> r8153 on Dell TB dock corrupts rx packets.
> >> 
> >> The root cause is not found yet, but disabling rx checksumming can
> >> workaround the issue. We can use this connection to decide if it's
> >> a Dell TB dock:
> >> Realtek r8153 <-> SMSC hub <-> ASMedia XHCI controller
> >> 
> >> BugLink: https://bugs.launchpad.net/bugs/1729674
> >> Cc: Mario Limonciello 
> >> Signed-off-by: Kai-Heng Feng 
> >> ---
> >> drivers/net/usb/r8152.c | 33 -
> >> 1 file changed, 32 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> >> index d51d9abf7986..58b80b5e7803 100644
> >> --- a/drivers/net/usb/r8152.c
> >> +++ b/drivers/net/usb/r8152.c
> >> @@ -27,6 +27,8 @@
> >> #include 
> >> #include 
> >> #include 
> >> +#include 
> >> +#include 
> >> 
> >> /* Information for net-next */
> >> #define NETNEXT_VERSION"09"
> >> @@ -5135,6 +5137,35 @@ static u8 rtl_get_version(struct usb_interface 
> >> *intf)
> >>return version;
> >> }
> >> 
> >> +/* Ethernet on Dell TB 15/16 dock is connected this way:
> >> + * Realtek r8153 <-> SMSC hub <-> ASMedia XHCI controller
> >> + * We use this connection to make sure r8153 is on the Dell TB dock.
> >> + */
> >> +static bool check_dell_tb_dock(struct usb_device *udev)
> >> +{
> >> +  struct usb_device *hub = udev->parent;
> >> +  struct usb_device *root_hub;
> >> +  struct pci_dev *controller;
> >> +
> >> +  if (!hub)
> >> +  return false;
> >> +
> >> +  if (!(le16_to_cpu(hub->descriptor.idVendor) == 0x0424 &&
> >> +le16_to_cpu(hub->descriptor.idProduct) == 0x5537))
> >> +  return false;
> >> +
> >> +  root_hub = hub->parent;
> >> +  if (!root_hub || root_hub->parent)
> >> +  return false;
> >> +
> >> +  controller = to_pci_dev(bus_to_hcd(root_hub->bus)->self.controller);
> > 
> > That's a very scary, and dangerous, cast.  You can not ever be sure that
> > the hub really is a "root hub" like this.
> 
> What I want to do here is to finding this connection:
> Realtek r8153 <-> SMSC hub (USD ID: 0424:5537) <-> 
> ASMedia XHCI controller (PCI ID: 1b21:1142).
> 
> Is there a safer way to do this?

Nope!  You can't do that at all from within a USB driver, sorry.  As you
really should not care at all :)

> >> +  if (controller->vendor == 0x1b21 && controller->device == 0x1142)
> >> +  return true;
> > 
> > Why can't you just look at the USB device itself and go off of a quirk
> > in it?  Something like a version or string or something else?
> 
> I have a r8153 <-> USB 3.0 dongle which work just fine. I can’t find any 
> information to differentiate them. Hence I want to use the connection to
> identify if r8153 is on a Dell TB dock.

Are you sure there is nothing different in the version or release number
of the device?  'lsusb -v' shows the exact same information for both
devices?

> > This sounds like a USB host controller issue, not a USB device issue,
> > can't we fix the "real" problem here instead of this crazy work-around?
> 
> Yes. From what I know, ASMedia is working on it, but not sure how long it
> will take. In the meantime, I’d like to workaround this issue for the users.

Again, it's a host controller bug, it should be fixed there, don't try
to paper over the real issue in different individual drivers.

I think I've seen various patches on the linux-usb list for this
controller already, have you tried them?

> > Odds are any device plugged into the hub should have the same issue,
> > right?
> 
> Actually no.
> I just plugged r8153 dongle into the same hub, surprisingly the issue
> doesn’t happen in this scenario.

Then something seems to be wrong with the device itself, as that would
be the same exact electrical/logical path, right?

thanks,

greg k-h
--
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

Re: [PATCH] r8152: disable rx checksum offload on Dell TB dock

[Kai Heng Feng]

> On 23 Nov 2017, at 3:58 PM, Greg KH  wrote:
> 
> On Thu, Nov 23, 2017 at 01:38:38AM -0500, Kai-Heng Feng wrote:
>> r8153 on Dell TB dock corrupts rx packets.
>> 
>> The root cause is not found yet, but disabling rx checksumming can
>> workaround the issue. We can use this connection to decide if it's
>> a Dell TB dock:
>> Realtek r8153 <-> SMSC hub <-> ASMedia XHCI controller
>> 
>> BugLink: https://bugs.launchpad.net/bugs/1729674
>> Cc: Mario Limonciello 
>> Signed-off-by: Kai-Heng Feng 
>> ---
>> drivers/net/usb/r8152.c | 33 -
>> 1 file changed, 32 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
>> index d51d9abf7986..58b80b5e7803 100644
>> --- a/drivers/net/usb/r8152.c
>> +++ b/drivers/net/usb/r8152.c
>> @@ -27,6 +27,8 @@
>> #include 
>> #include 
>> #include 
>> +#include 
>> +#include 
>> 
>> /* Information for net-next */
>> #define NETNEXT_VERSION  "09"
>> @@ -5135,6 +5137,35 @@ static u8 rtl_get_version(struct usb_interface *intf)
>>  return version;
>> }
>> 
>> +/* Ethernet on Dell TB 15/16 dock is connected this way:
>> + * Realtek r8153 <-> SMSC hub <-> ASMedia XHCI controller
>> + * We use this connection to make sure r8153 is on the Dell TB dock.
>> + */
>> +static bool check_dell_tb_dock(struct usb_device *udev)
>> +{
>> +struct usb_device *hub = udev->parent;
>> +struct usb_device *root_hub;
>> +struct pci_dev *controller;
>> +
>> +if (!hub)
>> +return false;
>> +
>> +if (!(le16_to_cpu(hub->descriptor.idVendor) == 0x0424 &&
>> +  le16_to_cpu(hub->descriptor.idProduct) == 0x5537))
>> +return false;
>> +
>> +root_hub = hub->parent;
>> +if (!root_hub || root_hub->parent)
>> +return false;
>> +
>> +controller = to_pci_dev(bus_to_hcd(root_hub->bus)->self.controller);
> 
> That's a very scary, and dangerous, cast.  You can not ever be sure that
> the hub really is a "root hub" like this.

What I want to do here is to finding this connection:
Realtek r8153 <-> SMSC hub (USD ID: 0424:5537) <-> 
ASMedia XHCI controller (PCI ID: 1b21:1142).

Is there a safer way to do this?

> 
>> +if (controller->vendor == 0x1b21 && controller->device == 0x1142)
>> +return true;
> 
> Why can't you just look at the USB device itself and go off of a quirk
> in it?  Something like a version or string or something else?

I have a r8153 <-> USB 3.0 dongle which work just fine. I can’t find any 
information to differentiate them. Hence I want to use the connection to
identify if r8153 is on a Dell TB dock.

> 
> This sounds like a USB host controller issue, not a USB device issue,
> can't we fix the "real" problem here instead of this crazy work-around?

Yes. From what I know, ASMedia is working on it, but not sure how long it
will take. In the meantime, I’d like to workaround this issue for the users.

> Odds are any device plugged into the hub should have the same issue,
> right?

Actually no.
I just plugged r8153 dongle into the same hub, surprisingly the issue
doesn’t happen in this scenario.

Kai-Heng

> 
> thanks,
> 
> greg k-h

--
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

Re: [PATCH 11/11] usb/gadget: Make it again possible to enable the legacy drivers without enabling USB_ETH

[Felipe Balbi]

Hi,

Bart Van Assche  writes:
> On Tue, 2017-10-31 at 11:03 -0700, Bart Van Assche wrote:
>> Considerable time ago the legacy gadget menu was added inside the
>> USB_ETH choice. I think this was a mistake and that the legacy
>> gadget menu should have been added after "endchoice" instead of
>> before. Hence this patch.
>> 
>> Fixes: commit 8443f2d2b778 ("usb: gadget: Gadget directory cleanup - group 
>> legacy gadgets")
>> Signed-off-by: Bart Van Assche 
>> Cc: Nicholas Bellinger 
>> Cc: Andrzej Pietrasiewicz 
>> Cc: Felipe Balbi 
>> Cc: linux-usb@vger.kernel.org
>
> Hello Andrzej and Felipe,
>
> Can one or both of you have a look at this patch?

sure thing, as soon as the merge window closes :-)

-- 
balbi


signature.asc
Description: PGP signature