Re: PROBLEM: lsusb -v freezes kernel on Acer ES1-111M

2015-09-01 Thread Roland Weber 
Hello Alan,

> You can try the debugging patch below.  It will tell us when the items
> on the workqueue get executed, which could be a useful clue.  You
> should test this from a VT console with the logging level set high
> enough so that the log messages show up on the screen.

No luck. I've booted into the patched kernel and set the logging level,
including .../dynamic_debug/control. I've verified that I get log output
by attaching and detaching an USB device; that gets logged. But when I
call "lsusb -v", the only output is the ID of the device that's going
to be probed. That's what I always get. The kernel freezes before the
new log statements, or any others in hub.c, are executed.

Later this week, I can enable logging with kernel boot parameters and
send the output, if that might be of help.

thanks and cheers,
  Roland
--
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/3][v2] drivers: usb: dwc3: Add frame length adjustment quirk

2015-09-01 Thread Felipe Balbi 
Hi,

On Mon, Aug 17, 2015 at 09:54:56AM +0530, Nikhil Badola wrote:
> +static void dwc3_frame_length_adjustment(struct dwc3 *dwc, u32 fladj)
> +{
> + if (dwc->revision < DWC3_REVISION_250A)
> + return;
> +
> + if (fladj == 0)
> + return;
> +
> + if (fladj) {

right here you know fladj to be non-zero.

> @@ -957,6 +990,7 @@ static int dwc3_probe(struct platform_device *pdev)
>   goto err1;
>   }
>  
> +

stray change

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH] xhci: fix warning when CONFIG_PM disabled.

2015-09-01 Thread Mathias Nyman 

On 01.09.2015 00:26, Dave Hansen wrote:

From: Dave Hansen 

I have a .config with CONFIG_PM disabled.  I get the following
whenever compiling the xhci driver:

drivers/usb/host/xhci-pci.c:192:13: warning: ‘xhci_pme_quirk’ defined but 
not used [-Wunused-function]

Looks like we just need to move xhci_pme_quirk() to be underneath
the CONFIG_PM #ifdef.



There is a patch for this already, I'll send it forward right after the merge 
window

http://marc.info/?l=linux-usb=143988457116356=2

-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 v2 1/7] usb: misc: usbtest: allocate size of urb array according to user parameter

2015-09-01 Thread Peter Chen 
 
> > ---
> >  drivers/usb/misc/usbtest.c | 9 +
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
> > index 9517812..8f294d7 100644
> > --- a/drivers/usb/misc/usbtest.c
> > +++ b/drivers/usb/misc/usbtest.c
> > @@ -95,6 +95,7 @@ static struct usb_device *testdev_to_usbdev(struct
> usbtest_dev *test)
> > dev_warn(&(tdev)->intf->dev , fmt , ## args)
> >
> >  #define GUARD_BYTE 0xA5
> > +#define MAX_SGLEN  128
> >
> >
> > /*
> > -*/
> >
> > @@ -1911,10 +1912,7 @@ test_iso_queue(struct usbtest_dev *dev, struct
> usbtest_param *param,
> > unsignedi;
> > unsigned long   packets = 0;
> > int status = 0;
> > -   struct urb  *urbs[10];  /* FIXME no limit */
> > -
> > -   if (param->sglen > 10)
> > -   return -EDOM;
> > +   struct urb  *urbs[param->sglen];
> >
> > memset(, 0, sizeof(context));
> > context.count = param->iterations * param->sglen; @@ -2061,6
> +2059,9
> > @@ usbtest_ioctl(struct usb_interface *intf, unsigned int code, void *buf)
> > if (param->iterations <= 0)
> > return -EINVAL;
> >
> > +   if (param->sglen > MAX_SGLEN)
> > +   return -EINVAL;
> 
> This will not prevent problems.  The stack space gets allocated as soon as the
> function starts, and if param->sglen is very big then the damage will already
> have occurred by this point.
> 

Sorry? It is the beginning of usbtest_ioctl, the test_iso_queue has still not
been called.

Peter

> It's probably better simply to use kmalloc()/kfree() and not try to put these
> things on the stack.
> 
> 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

[PATCH 1/5] USB: io_ti: Remove obsolete dev argument from build_i2c_fw_hdr

2015-09-01 Thread Peter E. Berger 
From: "Peter E. Berger" 

Remove unused "dev" argument from build_i2c_fw_hdr() and its caller.

Signed-off-by: Peter E. Berger 
---
 drivers/usb/serial/io_ti.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c
index 0ac1b10..fc82408 100644
--- a/drivers/usb/serial/io_ti.c
+++ b/drivers/usb/serial/io_ti.c
@@ -785,8 +785,7 @@ exit:
 }
 
 /* Build firmware header used for firmware update */
-static int build_i2c_fw_hdr(u8 *header, struct device *dev,
-   const struct firmware *fw)
+static int build_i2c_fw_hdr(u8 *header, const struct firmware *fw)
 {
__u8 *buffer;
int buffer_size;
@@ -1245,7 +1244,7 @@ static int download_fw(struct edgeport_serial *serial,
 * UMP Ram to I2C and the firmware will update the
 * record type from 0xf2 to 0x02.
 */
-   status = build_i2c_fw_hdr(header, dev, fw);
+   status = build_i2c_fw_hdr(header, fw);
if (status) {
kfree(vheader);
kfree(header);
-- 
1.8.3.1

--
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 4/5] USB: io_ti: Remove leading tabs in do_download_mode

2015-09-01 Thread Peter E. Berger 
From: "Peter E. Berger" 

Remove leading tabs in do_download_mode and fixup problems
flagged by checkpatch.pl

Signed-off-by: Peter E. Berger 
---
 drivers/usb/serial/io_ti.c | 314 +++--
 1 file changed, 162 insertions(+), 152 deletions(-)

diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c
index 58254ca..a226424 100644
--- a/drivers/usb/serial/io_ti.c
+++ b/drivers/usb/serial/io_ti.c
@@ -1054,68 +1054,131 @@ static int do_download_mode(struct edgeport_serial 
*serial,
struct edgeport_fw_hdr *fw_hdr = (struct edgeport_fw_hdr *)fw->data;
struct ti_i2c_desc *rom_desc;
 
-   dev_dbg(dev, "%s - RUNNING IN DOWNLOAD MODE\n", __func__);
+   dev_dbg(dev, "%s - RUNNING IN DOWNLOAD MODE\n", __func__);
 
-   status = check_i2c_image(serial);
-   if (status) {
-   dev_dbg(dev, "%s - DOWNLOAD MODE -- BAD I2C\n", 
__func__);
-   return status;
+   status = check_i2c_image(serial);
+   if (status) {
+   dev_dbg(dev, "%s - DOWNLOAD MODE -- BAD I2C\n", __func__);
+   return status;
+   }
+
+   /*
+* Validate Hardware version number
+* Read Manufacturing Descriptor from TI Based Edgeport
+*/
+   ti_manuf_desc = kmalloc(sizeof(*ti_manuf_desc), GFP_KERNEL);
+   if (!ti_manuf_desc)
+   return -ENOMEM;
+
+   status = get_manuf_info(serial, (__u8 *)ti_manuf_desc);
+   if (status) {
+   kfree(ti_manuf_desc);
+   return status;
+   }
+
+   /* Check version number of ION descriptor */
+   if (!ignore_cpu_rev && ti_cpu_rev(ti_manuf_desc) < 2) {
+   dev_dbg(dev, "%s - Wrong CPU Rev %d (Must be 2)\n",
+   __func__, ti_cpu_rev(ti_manuf_desc));
+   kfree(ti_manuf_desc);
+   return -EINVAL;
+   }
+
+   rom_desc = kmalloc(sizeof(*rom_desc), GFP_KERNEL);
+   if (!rom_desc) {
+   kfree(ti_manuf_desc);
+   return -ENOMEM;
+   }
+
+   /* Search for type 2 record (firmware record) */
+   start_address = get_descriptor_addr(serial,
+   I2C_DESC_TYPE_FIRMWARE_BASIC, rom_desc);
+   if (start_address != 0) {
+   struct ti_i2c_firmware_rec *firmware_version;
+   u8 *record;
+
+   dev_dbg(dev, "%s - Found Type FIRMWARE (Type 2) record\n",
+   __func__);
+
+   firmware_version = kmalloc(sizeof(*firmware_version),
+   GFP_KERNEL);
+   if (!firmware_version) {
+   kfree(rom_desc);
+   kfree(ti_manuf_desc);
+   return -ENOMEM;
}
 
/*
-* Validate Hardware version number
-* Read Manufacturing Descriptor from TI Based Edgeport
+* Validate version number
+* Read the descriptor data
 */
-   ti_manuf_desc = kmalloc(sizeof(*ti_manuf_desc), GFP_KERNEL);
-   if (!ti_manuf_desc)
-   return -ENOMEM;
-
-   status = get_manuf_info(serial, (__u8 *)ti_manuf_desc);
+   status = read_rom(serial, start_address +
+   sizeof(struct ti_i2c_desc),
+   sizeof(struct ti_i2c_firmware_rec),
+   (__u8 *)firmware_version);
if (status) {
+   kfree(firmware_version);
+   kfree(rom_desc);
kfree(ti_manuf_desc);
return status;
}
 
-   /* Check version number of ION descriptor */
-   if (!ignore_cpu_rev && ti_cpu_rev(ti_manuf_desc) < 2) {
-   dev_dbg(dev, "%s - Wrong CPU Rev %d (Must be 2)\n",
-   __func__, ti_cpu_rev(ti_manuf_desc));
-   kfree(ti_manuf_desc);
-   return -EINVAL;
-   }
-
-   rom_desc = kmalloc(sizeof(*rom_desc), GFP_KERNEL);
-   if (!rom_desc) {
-   kfree(ti_manuf_desc);
-   return -ENOMEM;
-   }
+   /*
+* Check version number of download with current
+* version in I2c
+*/
+   download_cur_ver = (firmware_version->Ver_Major << 8) +
+  (firmware_version->Ver_Minor);
+   download_new_ver = (fw_hdr->major_version << 8) +
+  (fw_hdr->minor_version);
 
-   /* Search for type 2 record (firmware record) */
-   start_address = get_descriptor_addr(serial,
-

[PATCH 5/5] USB: io_ti: Move request_firmware from edge_startup to download_fw

2015-09-01 Thread Peter E. Berger 
From: "Peter E. Berger" 

Move request_firmware from edge_startup to download_fw

Signed-off-by: Peter E. Berger 
---
 drivers/usb/serial/io_ti.c | 56 --
 1 file changed, 29 insertions(+), 27 deletions(-)

diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c
index a226424..9fe12cc 100644
--- a/drivers/usb/serial/io_ti.c
+++ b/drivers/usb/serial/io_ti.c
@@ -991,18 +991,29 @@ static int check_fw_sanity(struct edgeport_serial *serial,
  * This routine downloads the main operating code into the TI5052, using the
  * boot code already burned into E2PROM or ROM.
  */
-static int download_fw(struct edgeport_serial *serial,
-   const struct firmware *fw)
+static int download_fw(struct edgeport_serial *serial)
 {
struct device *dev = >serial->interface->dev;
int status = 0;
struct usb_interface_descriptor *interface;
-   struct edgeport_fw_hdr *fw_hdr = (struct edgeport_fw_hdr *)fw->data;
+   const struct firmware *fw;
+   const char *fw_name = "edgeport/down3.bin";
+   struct edgeport_fw_hdr *fw_hdr;
 
-   if (check_fw_sanity(serial, fw))
-   return -EINVAL;
+   status = request_firmware(, fw_name, dev);
+   if (status) {
+   dev_err(dev, "Failed to load image \"%s\" err %d\n",
+   fw_name, status);
+   return status;
+   }
+   fw_hdr = (struct edgeport_fw_hdr *)fw->data;
+
+   if (check_fw_sanity(serial, fw)) {
+   status = -EINVAL;
+   goto out;
+   }
 
-   /* If on-board version is newer, "fw_version" will be updated below. */
+   /* If on-board version is newer, "fw_version" will be updated later. */
serial->fw_version = (fw_hdr->major_version << 8) +
fw_hdr->minor_version;
 
@@ -1018,12 +1029,13 @@ static int download_fw(struct edgeport_serial *serial,
 
status = choose_config(serial->serial->dev);
if (status)
-   return status;
+   goto out;
 
interface = >serial->interface->cur_altsetting->desc;
if (!interface) {
dev_err(dev, "%s - no interface set, error!\n", __func__);
-   return -ENODEV;
+   status = -ENODEV;
+   goto out;
}
 
/*
@@ -1033,13 +1045,16 @@ static int download_fw(struct edgeport_serial *serial,
 */
if (interface->bNumEndpoints > 1) {
serial->product_info.TiMode = TI_MODE_DOWNLOAD;
-   return do_download_mode(serial, fw);
+   status = do_download_mode(serial, fw);
+   } else {
+   /* Otherwise we will remain in configuring mode */
+   serial->product_info.TiMode = TI_MODE_CONFIGURING;
+   status = do_boot_mode(serial, fw);
}
 
-   /* Otherwise we will remain in configuring mode */
-   serial->product_info.TiMode = TI_MODE_CONFIGURING;
-   return do_boot_mode(serial, fw);
-
+out:
+   release_firmware(fw);
+   return status;
 }
 
 static int do_download_mode(struct edgeport_serial *serial,
@@ -1483,7 +1498,6 @@ stayinbootmode:
return 0;
 }
 
-
 static int ti_do_config(struct edgeport_port *port, int feature, int on)
 {
int port_number = port->port->port_number;
@@ -2511,9 +2525,6 @@ static int edge_startup(struct usb_serial *serial)
 {
struct edgeport_serial *edge_serial;
int status;
-   const struct firmware *fw;
-   const char *fw_name = "edgeport/down3.bin";
-   struct device *dev = >interface->dev;
u16 product_id;
 
/* create our private serial structure */
@@ -2525,16 +2536,7 @@ static int edge_startup(struct usb_serial *serial)
edge_serial->serial = serial;
usb_set_serial_data(serial, edge_serial);
 
-   status = request_firmware(, fw_name, dev);
-   if (status) {
-   dev_err(dev, "Failed to load image \"%s\" err %d\n",
-   fw_name, status);
-   kfree(edge_serial);
-   return status;
-   }
-
-   status = download_fw(edge_serial, fw);
-   release_firmware(fw);
+   status = download_fw(edge_serial);
if (status) {
kfree(edge_serial);
return status;
-- 
1.8.3.1

--
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 0/5] USB: io_ti: Cleanup download_fw and related functions

2015-09-01 Thread Peter E. Berger 
From: "Peter E. Berger" 

While working on a previous patchset for this driver [1] we were hampered
by the fact that download_fw() is very long and its return paths are so
complicated that we were compelled to the put the request_firmware() call
in edge_startup(), when it much more naturally belongs to download_fw().

The primary purpose of this patchset is to move the bulk of the
download_fw() code into two helper functions, to cleanup the return
paths and to migrate the request_firmware() call into download_fw().

I welcome any comments or suggestions.

Thanks.
 --Peter

[1]: http://marc.info/?l=linux-usb=143832572328107=2

Peter E. Berger (5):
  USB: io_ti: Remove obsolete dev argument from build_i2c_fw_hdr
  USB: io_ti: use serial->interface for messages in download_fw
  USB: io_ti: Move download and boot mode code out of download_fw
  USB: io_ti: Remove leading tabs in do_download_mode
  USB: io_ti: Move request_firmware from edge_startup to download_fw

 drivers/usb/serial/io_ti.c | 438 +
 1 file changed, 242 insertions(+), 196 deletions(-)

-- 
1.8.3.1

--
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 3/5] USB: io_ti: Move download and boot mode code out of download_fw

2015-09-01 Thread Peter E. Berger 
From: "Peter E. Berger" 

Separate the download and boot mode code from download_fw() into two new
helper functions: do_download_mode() and do_boot_mode(), and fix formatting
in some of the comments.

Signed-off-by: Peter E. Berger 
---
 drivers/usb/serial/io_ti.c | 111 +
 1 file changed, 73 insertions(+), 38 deletions(-)

diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c
index d41ba74..58254ca 100644
--- a/drivers/usb/serial/io_ti.c
+++ b/drivers/usb/serial/io_ti.c
@@ -223,6 +223,11 @@ static void edge_set_termios(struct tty_struct *tty,
struct usb_serial_port *port, struct ktermios *old_termios);
 static void edge_send(struct usb_serial_port *port, struct tty_struct *tty);
 
+static int do_download_mode(struct edgeport_serial *serial,
+   const struct firmware *fw);
+static int do_boot_mode(struct edgeport_serial *serial,
+   const struct firmware *fw);
+
 /* sysfs attributes */
 static int edge_create_sysfs_attrs(struct usb_serial_port *port);
 static int edge_remove_sysfs_attrs(struct usb_serial_port *port);
@@ -980,7 +985,7 @@ static int check_fw_sanity(struct edgeport_serial *serial,
return 0;
 }
 
-/**
+/*
  * DownloadTIFirmware - Download run-time operating firmware to the TI5052
  *
  * This routine downloads the main operating code into the TI5052, using the
@@ -991,11 +996,7 @@ static int download_fw(struct edgeport_serial *serial,
 {
struct device *dev = >serial->interface->dev;
int status = 0;
-   int start_address;
-   struct edge_ti_manuf_descriptor *ti_manuf_desc;
struct usb_interface_descriptor *interface;
-   int download_cur_ver;
-   int download_new_ver;
struct edgeport_fw_hdr *fw_hdr = (struct edgeport_fw_hdr *)fw->data;
 
if (check_fw_sanity(serial, fw))
@@ -1005,7 +1006,8 @@ static int download_fw(struct edgeport_serial *serial,
serial->fw_version = (fw_hdr->major_version << 8) +
fw_hdr->minor_version;
 
-   /* This routine is entered by both the BOOT mode and the Download mode
+   /*
+* This routine is entered by both the BOOT mode and the Download mode
 * We can determine which code is running by the reading the config
 * descriptor and if we have only one bulk pipe it is in boot mode
 */
@@ -1029,17 +1031,28 @@ static int download_fw(struct edgeport_serial *serial,
 * if we have more than one endpoint we are definitely in download
 * mode
 */
-   if (interface->bNumEndpoints > 1)
+   if (interface->bNumEndpoints > 1) {
serial->product_info.TiMode = TI_MODE_DOWNLOAD;
-   else
-   /* Otherwise we will remain in configuring mode */
-   serial->product_info.TiMode = TI_MODE_CONFIGURING;
+   return do_download_mode(serial, fw);
+   }
 
-   //
-   /* Download Mode */
-   //
-   if (serial->product_info.TiMode == TI_MODE_DOWNLOAD) {
-   struct ti_i2c_desc *rom_desc;
+   /* Otherwise we will remain in configuring mode */
+   serial->product_info.TiMode = TI_MODE_CONFIGURING;
+   return do_boot_mode(serial, fw);
+
+}
+
+static int do_download_mode(struct edgeport_serial *serial,
+   const struct firmware *fw)
+{
+   struct device *dev = >serial->interface->dev;
+   int status = 0;
+   int start_address;
+   struct edge_ti_manuf_descriptor *ti_manuf_desc;
+   int download_cur_ver;
+   int download_new_ver;
+   struct edgeport_fw_hdr *fw_hdr = (struct edgeport_fw_hdr *)fw->data;
+   struct ti_i2c_desc *rom_desc;
 
dev_dbg(dev, "%s - RUNNING IN DOWNLOAD MODE\n", __func__);
 
@@ -1049,7 +1062,8 @@ static int download_fw(struct edgeport_serial *serial,
return status;
}
 
-   /* Validate Hardware version number
+   /*
+* Validate Hardware version number
 * Read Manufacturing Descriptor from TI Based Edgeport
 */
ti_manuf_desc = kmalloc(sizeof(*ti_manuf_desc), GFP_KERNEL);
@@ -1068,7 +1082,7 @@ static int download_fw(struct edgeport_serial *serial,
__func__, ti_cpu_rev(ti_manuf_desc));
kfree(ti_manuf_desc);
return -EINVAL;
-   }
+   }
 
rom_desc = kmalloc(sizeof(*rom_desc), GFP_KERNEL);
if (!rom_desc) {
@@ -1093,7 +1107,8 @@ static int download_fw(struct edgeport_serial *serial,
return -ENOMEM;
}
 
-   /* Validate version number
+   /*
+

[PATCH 2/5] USB: io_ti: use serial->interface for messages in download_fw

2015-09-01 Thread Peter E. Berger 
From: "Peter E. Berger" 

Use serial->interface instead of serial->dev for messages in download_fw().

Signed-off-by: Peter E. Berger 
---
 drivers/usb/serial/io_ti.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c
index fc82408..d41ba74 100644
--- a/drivers/usb/serial/io_ti.c
+++ b/drivers/usb/serial/io_ti.c
@@ -989,7 +989,7 @@ static int check_fw_sanity(struct edgeport_serial *serial,
 static int download_fw(struct edgeport_serial *serial,
const struct firmware *fw)
 {
-   struct device *dev = >serial->dev->dev;
+   struct device *dev = >serial->interface->dev;
int status = 0;
int start_address;
struct edge_ti_manuf_descriptor *ti_manuf_desc;
-- 
1.8.3.1

--
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/3][v2] drivers: usb: dwc3: Add frame length adjustment quirk

2015-09-01 Thread Badola Nikhil 
> -Original Message-
> From: Felipe Balbi [mailto:ba...@ti.com]
> Sent: Tuesday, September 01, 2015 12:51 PM
> To: Badola Nikhil-B46172 
> Cc: linux-ker...@vger.kernel.org; linux-usb@vger.kernel.org; linux-
> o...@vger.kernel.org; ba...@ti.com; gre...@linuxfoundation.org
> Subject: Re: [PATCH 2/3][v2] drivers: usb: dwc3: Add frame length
> adjustment quirk
> 
> Hi,
> 
> On Mon, Aug 17, 2015 at 09:54:56AM +0530, Nikhil Badola wrote:
> > +static void dwc3_frame_length_adjustment(struct dwc3 *dwc, u32 fladj)
> > +{
> > +   if (dwc->revision < DWC3_REVISION_250A)
> > +   return;
> > +
> > +   if (fladj == 0)
> > +   return;
> > +
> > +   if (fladj) {
> 
> right here you know fladj to be non-zero.
> 
> > @@ -957,6 +990,7 @@ static int dwc3_probe(struct platform_device
> *pdev)
> > goto err1;
> > }
> >
> > +
> 
> stray change
> 
Will make both changes and send a new version.
--
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] usbnet: Fix a race between usbnet_stop() and the BH

2015-09-01 Thread Oliver Neukum 
On Mon, 2015-08-31 at 11:50 +0300, Eugene Shatokhin wrote:
> > But I would have liked it much better if the code became simpler
> instead
> > of more complex.
> 
> Me too, but I can see no other way here. The code is simpler without 
> locking, indeed, but locking is needed to prevent the problems
> described 
> earlier.

On a practical note, will you resend the patch?

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 2/2] usbnet: Fix a race between usbnet_stop() and the BH

2015-09-01 Thread Oliver Neukum 
On Fri, 2015-08-28 at 11:09 +0300, Eugene Shatokhin wrote:
> > Exactly what problem will that result in?  The tasklet_kill() will
> wait
> > for the processing of the single element done queue, and everything
> will
> > be fine.  Or?
> 
> Given enough time, what prevents defer_bh() from calling 
> tasklet_schedule(>bh) *after* usbnet_stop() calls tasklet_kill()?
> 
> Consider the following situation (assuming '&&' are changed to '||'
> in 
> that while loop in usbnet_terminate_urbs() as they should be):
> 
> CPU0CPU1
> usbnet_stop()   defer_bh() with list == dev->rxq
>usbnet_terminate_urbs()
>  __skb_unlink() removes the last
>  skb from dev->rxq.
>  dev->rxq, dev->txq and dev->done
>  are now empty.
>while (!skb_queue_empty()...)
>  The loop ends because all 3
>  queues are now empty.
> 
>usbnet_terminate_urbs() ends.
> 
> usbnet_stop() continues:
>usbnet_status_stop(dev);
>...
>del_timer_sync (>delay);
>tasklet_kill (>bh);
>  __skb_queue_tail(>done, skb);
>  if (dev->done.qlen == 1)
>tasklet_schedule(>bh);
> 
> The BH is scheduled at this point, which is not what was intended.
> The 
> race window is small, but still.

This looks possible. I cannot come up with a better fix, although
it isn't nice. Thanks for finding this.

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 1/2] usb: chipidea: add xilinx zynq platform data

2015-09-01 Thread Subbaraya Sundeep Bhatta 
Hi,

> -Original Message-
> From: Peter Chen [mailto:peter.c...@freescale.com]
> Sent: Monday, August 31, 2015 7:59 AM
> To: Punnaiah Choudary Kalluri
> Cc: Nathan Sullivan; sundeep subbaraya; Subbaraya Sundeep Bhatta;
> robh...@kernel.org; pawel.m...@arm.com; Mark Rutland;
> ijc+devicet...@hellion.org.uk; Kumar Gala; Greg Kroah-Hartman;
> devicet...@vger.kernel.org; linux-ker...@vger.kernel.org; linux-
> u...@vger.kernel.org
> Subject: Re: [PATCH 1/2] usb: chipidea: add xilinx zynq platform data
> 
> On Mon, Aug 31, 2015 at 09:01:51AM +0530, punnaiah choudary kalluri
> wrote:
> > On Mon, Aug 31, 2015 at 6:10 AM, Peter Chen
>  wrote:
> > > On Fri, Aug 28, 2015 at 09:42:56AM -0500, Nathan Sullivan wrote:
> > >> On Fri, Aug 28, 2015 at 09:30:12AM +0800, Peter Chen wrote:
> > >> > On Thu, Aug 27, 2015 at 09:33:07AM -0500, Nathan Sullivan wrote:
> > >> > > On Thu, Aug 27, 2015 at 01:11:30PM +0530, punnaiah choudary
> kalluri wrote:
> > >> > > > Hi,
> > >> > > >
> > >> > > > On Thu, Aug 27, 2015 at 10:03 AM, Peter Chen
>  wrote:
> > >> > > > > On Thu, Aug 27, 2015 at 10:59:22AM +0530, sundeep
> subbaraya wrote:
> > >> > > > >> Hi,
> > >> > > > >>
> > >> > > > >>
> > >> > > > >> On Wed, Aug 26, 2015 at 8:57 PM, Nathan Sullivan
>  wrote:
> > >> > > > >> > The Xilinx Zynq udc does not need the
> > >> > > > >> > CI_HDRC_DISABLE_STREAMING flag, unlike the default
> > >> > > > >> > platform data.  Add platform data specific to the Zynq udc.
> > >> > > > >> >
> > >> > > > >> > Based on a patch by the same name from the Xilinx vendor
> tree.
> > >> > > > >>
> > >> > > > >> I am that Xilinx guy who sent this patch :). It is in
> > >> > > > >> Xilinx tree as temporary fix and I did not debug further
> > >> > > > >> why UDC works only when streaming is enabled.
> > >> > > > >> Probably this is right time to post my question here.
> > >> > > > >> I was expecting like:
> > >> > > > >> Streaming disabled - both low bandwidth and high bandwidth
> > >> > > > >> systems should work fine Streaming enabled - only for high
> > >> > > > >> bandwidth systems but this is not the case, Zynq UDC works
> > >> > > > >> only when Streaming is enabled.
> > >> > > > >> Please correct me if I am wrong.
> > >> > > > >
> > >> > > > > You are right, stream mode disabled should work at anytime.
> > >> > > > > It is so strange why zynq UDC only works when stream mode is
> enabled.
> > >> > > >
> > >> > > > I am referring the section 8.5.2 in Synopsys usb 2.0 HS
> > >> > > > controllervDoc 2.20a,  this is what it says about SDIS
> > >> > > > (streaming mode disable option)
> > >> > > >
> > >> > > > Before activating this mode, the user must check if the TX
> > >> > > > latency buffers per endpoint are able to accommodate at least
> > >> > > > one entire maximum size packet. The RX buffer size must, at
> > >> > > > least, double the TX buffer size per endpoint. To optimize
> > >> > > > the stream disable performance, system bus burst must be set
> > >> > > > as high as possible.
> > >> > > > When the stream disable mode is used, the burst size
> > >> > > > (VUSB_HS_RX_BURST and VUSB_HS_TX_BURST) must be a
> integer
> > >> > > > sub-multiple of the latency buffer size (VUSB_HS_RX_DEPTH for
> > >> > > > RX buffer and VUSB_HS_TX_CHAN for the TX buffer). If this is
> > >> > > > not respected the controller will not work properly in stream
> > >> > > > disable mode.
> > >> > > > The stream disable mode should just be used in situations
> > >> > > > where the available system bandwidth is low or the system bus
> > >> > > > access latency is high, in order to avoid underruns and
> > >> > > > overruns in the latency buffers. This works for all types of
> > >> > > > endpoints, except for ISO endpoints.
> > >> > > > Such a system can't ensure the real time support that the ISO
> > >> > > > endpoints require, so the ISO endpoints are not supported
> > >> > > > when the SDIS bit is set.
> > >> > > >
> > >> > > > Definitely we need to root cause why disable streaming mode
> > >> > > > is not working for zynq but from controller spec point of
> > >> > > > view it is possible that controller not work properly in
> > >> > > > stream disable mode.
> > >> > > >
> > >> > > > Regards,
> > >> > > > Punnaiah
> > >> > > >
> > >> > >
> > >> > > Maybe the burst size isn't set correctly by default?  It does
> > >> > > say the controller won't work correctly with stream disable set
> > >> > > and an invalid burst size.  Looks like TX and RX burst both default
> to 16, per the Zynq manual.
> > >> > >
> > >> > > With the stream disable bit set, the behvior we see on our
> > >> > > hardware is that priming just stops, with an outstanding
> > >> > > transfer in memory marked active in the status field by the
> > >> > > controller.  This happens at random, even when doing single
> > >> > > transfers at a time like with g_ether set to have a queue size
> > >> > > of 1.  With SDIS clear everything

Re: [PATCH v6 4/5] xhci: mediatek: support MTK xHCI host controller

2015-09-01 Thread chunfeng yun 
On Wed, 2015-08-26 at 22:18 +0100, Daniel Thompson wrote:
> On 22/08/15 02:45, Chunfeng Yun wrote:
> > MTK xhci host controller defines some extra SW scheduling
> > parameters for HW to minimize the scheduling effort for
> > synchronous and interrupt endpoints. The parameters are
> > put into reseved DWs of slot context and endpoint context
> 
> According to the covering e-mail there are additional quirks beyond the 
> scheduling algorithm. These should probably be mentioned here also (so 
> that they end up in the changelog).
> 
Ok, I will add them.

> 
> >
> > Signed-off-by: Chunfeng Yun 
> > ---
> >   drivers/usb/host/Kconfig|   9 +
> >   drivers/usb/host/Makefile   |   4 +
> >   drivers/usb/host/xhci-mtk-sch.c | 436 +
> >   drivers/usb/host/xhci-mtk.c | 831 
> > 
> >   drivers/usb/host/xhci-mtk.h | 133 +++
> >   drivers/usb/host/xhci-ring.c|  35 +-
> >   drivers/usb/host/xhci.c |  19 +-
> >   drivers/usb/host/xhci.h |   1 +
> >   8 files changed, 1461 insertions(+), 7 deletions(-)
> >   create mode 100644 drivers/usb/host/xhci-mtk-sch.c
> >   create mode 100644 drivers/usb/host/xhci-mtk.c
> >   create mode 100644 drivers/usb/host/xhci-mtk.h
> >
> > [snip]
> >
> > diff --git a/drivers/usb/host/xhci-mtk-sch.c 
> > b/drivers/usb/host/xhci-mtk-sch.c
> > new file mode 100644
> > index 000..d4b41a6
> > --- /dev/null
> > +++ b/drivers/usb/host/xhci-mtk-sch.c
> > @@ -0,0 +1,436 @@
> > +/*
> > + * Copyright (c) 2015 MediaTek Inc.
> > + * Author:
> > + *  Zhigang.Wei 
> > + *  Chunfeng.Yun 
> > + *
> > + * This software is licensed under the terms of the GNU General Public
> > + * License version 2, as published by the Free Software Foundation, and
> > + * may be copied, distributed, and modified under those terms.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + *
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include "xhci.h"
> > +#include "xhci-mtk.h"
> > +
> > +#define SS_BW_BOUNDARY 51000
> > +/* table 5-5. High-speed Isoc Transaction Limits in usb_20 spec */
> > +#define HS_BW_BOUNDARY 6144
> > +/* usb2 spec section11.18.1: at most 188 FS bytes per microframe */
> > +#define FS_PAYLOAD_MAX 188
> > +
> > +/* mtk scheduler bitmasks */
> > +#define EP_BPKTS(p)((p) & 0x3f)
> > +#define EP_BCSCOUNT(p) (((p) & 0x7) << 8)
> > +#define EP_BBM(p)  ((p) << 11)
> > +#define EP_BOFFSET(p)  ((p) & 0x3fff)
> > +#define EP_BREPEAT(p)  (((p) & 0x7fff) << 16)
> > +
> > +static int is_fs_or_ls(enum usb_device_speed speed)
> > +{
> > +   return speed == USB_SPEED_FULL || speed == USB_SPEED_LOW;
> > +}
> 
> This function is only used three times (and one of the uses would have 
> been a switch statement except for needing to make this function call). 
> Does it really make code clearer?
> 
This is mainly used to remove warnings when check patch by checkpatch.pl

> 
> > +
> > +static int get_bw_index(struct xhci_hcd *xhci, struct usb_device *udev,
> > +   struct usb_host_endpoint *ep)
> > +{
> > +   int bw_index;
> > +   int port_id;
> > +   struct xhci_virt_device *virt_dev;
> > +
> > +   virt_dev = xhci->devs[udev->slot_id];
> > +   port_id = virt_dev->real_port;
> > +
> > +   if (udev->speed == USB_SPEED_SUPER) {
> > +   if (usb_endpoint_dir_out(>desc))
> > +   bw_index = (port_id - 1) * 2;
> > +   else
> > +   bw_index = (port_id - 1) * 2 + 1;
> > +   } else {
> > +   bw_index = port_id + xhci->num_usb3_ports - 1;
> > +   }
> > +
> > +   return bw_index;
> > +}
> 
> This appears to be a function to help the scheduling code lookup 
> per-port private data held as an array in the device private structure.
> 
> Is this really the best way to lookup this data? (and if it *is* we 
> still need some comments explaining what this function does and how it 
> relates to sch_array).
> 
Yes, I will add some description for it.

> 
> > +
> > +static void setup_sch_info(struct usb_device *udev,
> > +   struct xhci_ep_ctx *ep_ctx, struct mu3h_sch_ep_info *sch_ep)
> > +{
> > +   u32 ep_type;
> > +   u32 ep_interval;
> > +   u32 max_packet_size;
> > +   u32 max_burst;
> > +   u32 mult;
> > +   u32 esit_pkts;
> > +
> > +   ep_type = CTX_TO_EP_TYPE(le32_to_cpu(ep_ctx->ep_info2));
> > +   ep_interval = CTX_TO_EP_INTERVAL(le32_to_cpu(ep_ctx->ep_info));
> > +   max_packet_size = MAX_PACKET_DECODED(le32_to_cpu(ep_ctx->ep_info2));
> > +   max_burst = CTX_TO_MAX_BURST(le32_to_cpu(ep_ctx->ep_info2));
> > +   mult = CTX_TO_EP_MULT(le32_to_cpu(ep_ctx->ep_info));
> > +
> > +   sch_ep->ep_type = ep_type;
> > +

Re: [PATCH] Revert "usb: dwc3: gadget: drop unnecessary loop when cleaning up TRBs"

2015-09-01 Thread Sergei Shtylyov 

On 8/31/2015 10:21 PM, Sergei Shtylyov wrote:


From: Ville Syrjälä 

This reverts commit 8f2c9544aba636134303105ecb164190a39dece4.

As it breaks g_ether on my Baytrail FFRD8 device. Everything starts out
fine, but after a bit of data has been transferred it just stops
flowing.

Note that I do get a bunch of these "NOHZ: local_softirq_pending 08"
when booting the machine, but I'm not really sure if they're related
to this problem.

Cc: Felipe Balbi 
Cc: Greg Kroah-Hartman 
Cc: linux-usb@vger.kernel.org
Cc: sta...@vger.kernel.org
Signed-off-by: Ville Syrjälä 
---
  drivers/usb/dwc3/gadget.c | 37 +
  1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 333a7c0..9a5de54 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1859,27 +1859,32 @@ static int dwc3_cleanup_done_reqs(struct dwc3 *dwc,
struct dwc3_ep *dep,
  unsigned inti;
  intret;

-req = next_request(>req_queued);
-if (!req) {
-WARN_ON_ONCE(1);
-return 1;
-}
-i = 0;
  do {
-slot = req->start_slot + i;
-if ((slot == DWC3_TRB_NUM - 1) &&
+req = next_request(>req_queued);
+if (!req) {
+WARN_ON_ONCE(1);
+return 1;
+}
+i = 0;
+do {
+slot = req->start_slot + i;
+if ((slot == DWC3_TRB_NUM - 1) &&
  usb_endpoint_xfer_isoc(dep->endpoint.desc))
-slot++;
-slot %= DWC3_TRB_NUM;
-trb = >trb_pool[slot];
+slot++;
+slot %= DWC3_TRB_NUM;
+trb = >trb_pool[slot];
+
+ret = __dwc3_cleanup_done_trbs(dwc, dep, req, trb,
+event, status);
+if (ret)
+break;
+}while (++i < req->request.num_mapped_sgs);


Space needed after }. And this *do {} while* loop seems replaceable with
*foor* loop...


   Sorry, didn't realize it was a revert. :-/


[...]


MBR, Sergei

--
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 v7 1/2] usb: make xhci platform driver use 64 bit or 32 bit DMA

2015-09-01 Thread Mathias Nyman 

On 31.08.2015 21:58, Duc Dang wrote:

On Thu, Aug 20, 2015 at 12:38 PM, Duc Dang  wrote:

The xhci platform driver needs to work on systems that
either only support 64-bit DMA or only support 32-bit DMA.
Attempt to set a coherent dma mask for 64-bit DMA, and
attempt again with 32-bit DMA if that fails.

[dhdang: regenerate the patch over 4.2-rc5 and address new comments]
Signed-off-by: Mark Langsdorf 
Tested-by: Mark Salter 
Signed-off-by: Duc Dang 

---
Changes from v6:
 -Add WARN_ON if dma_mask is NULL
 -Use dma_coerce_mask_and_coherent to assign
 dma_mask and coherent_dma_mask

Changes from v5:
 -Change comment
 -Assign dma_mask to coherent_dma_mask if dma_mask is NULL
 to make sure dma_set_mask_and_coherent does not fail prematurely.

Changes from v4:
 -None

Changes from v3:
 -Re-generate the patch over 4.2-rc5
 -No code change.

Changes from v2:
 -None

Changes from v1:
 -Consolidated to use dma_set_mask_and_coherent
 -Got rid of the check against sizeof(dma_addr_t)

  drivers/usb/host/xhci-plat.c | 21 +
  1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 890ad9d..e4c7f9d 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -93,14 +93,19 @@ static int xhci_plat_probe(struct platform_device *pdev)
 if (irq < 0)
 return -ENODEV;

-   /* Initialize dma_mask and coherent_dma_mask to 32-bits */
-   ret = dma_set_coherent_mask(>dev, DMA_BIT_MASK(32));
-   if (ret)
-   return ret;
-   if (!pdev->dev.dma_mask)
-   pdev->dev.dma_mask = >dev.coherent_dma_mask;
-   else
-   dma_set_mask(>dev, DMA_BIT_MASK(32));
+   /* Throw a waring if broken platform code didn't initialize dma_mask */
+   WARN_ON(!pdev->dev.dma_mask);
+   /*
+* Try setting dma_mask and coherent_dma_mask to 64 bits,
+* then try 32 bits
+*/
+   ret = dma_coerce_mask_and_coherent(>dev, DMA_BIT_MASK(64));
+   if (ret) {
+   ret = dma_coerce_mask_and_coherent(>dev,
+  DMA_BIT_MASK(32));
+   if (ret)
+   return ret;
+   }

 hcd = usb_create_hcd(driver, >dev, dev_name(>dev));
 if (!hcd)
--
1.9.1



Hi Greg, Arnd, Russell,

Do you have any more comment about this patch set?



I'm not sure I fully understand why we need to try the 64 bit DMA mask in 
platform probe.

As I understood it we just want to have some DMA mask set before calling 
usb_create_hcd()
to make sure USB core gets the "uses_dma" flag set and dma_set_mask() won't 
fail because of
missing dev->dma_mask.

The correct DMA mask is set later in xhci_gen_setup()

We also need to make sure the controller supports 64bit addressing capability 
before setting a 64 bit DMA mask.
(bit 0 in HCCPARAMS)

So for platform devices it goes look go this:

xhci_plat_probe()
  usb_create_hcd()
usb_create_shared_hcd()
  hcd->self.uses_dma = (dev->dma_mask != NULL);
  usb_add_hcd()
hcd->driver->reset()  (.reset = xhci_plat_setup)
  xhci_plat_setup()
xhci_gen_setup()
  if (HCC_64BIT_ADDR(xhci->hcc_params) && !dma_set_mask(dev, 
DMA_BIT_MASK(64))) {
xhci_dbg(xhci, "Enabling 64-bit DMA addresses.\n");
dma_set_coherent_mask(dev, DMA_BIT_MASK(64));
  }

or do we end up with dev->dma_mask = NULL on 64-bit DMA only after trying to 
set it to 32 in xhci_plat_probe(),
and thus also failing the dma_set_mask() in xhci_gen_setup()?

-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 v7 1/2] usb: make xhci platform driver use 64 bit or 32 bit DMA

2015-09-01 Thread Russell King - ARM Linux 
On Tue, Sep 01, 2015 at 02:54:17PM +0300, Mathias Nyman wrote:
> On 31.08.2015 21:58, Duc Dang wrote:
> >On Thu, Aug 20, 2015 at 12:38 PM, Duc Dang  wrote:
> >>The xhci platform driver needs to work on systems that
> >>either only support 64-bit DMA or only support 32-bit DMA.
> >>Attempt to set a coherent dma mask for 64-bit DMA, and
> >>attempt again with 32-bit DMA if that fails.
> >>
> >>[dhdang: regenerate the patch over 4.2-rc5 and address new comments]
> >>Signed-off-by: Mark Langsdorf 
> >>Tested-by: Mark Salter 
> >>Signed-off-by: Duc Dang 
> >>
> >>---
> >>Changes from v6:
> >> -Add WARN_ON if dma_mask is NULL
> >> -Use dma_coerce_mask_and_coherent to assign
> >> dma_mask and coherent_dma_mask
> >>
> >>Changes from v5:
> >> -Change comment
> >> -Assign dma_mask to coherent_dma_mask if dma_mask is NULL
> >> to make sure dma_set_mask_and_coherent does not fail prematurely.
> >>
> >>Changes from v4:
> >> -None
> >>
> >>Changes from v3:
> >> -Re-generate the patch over 4.2-rc5
> >> -No code change.
> >>
> >>Changes from v2:
> >> -None
> >>
> >>Changes from v1:
> >> -Consolidated to use dma_set_mask_and_coherent
> >> -Got rid of the check against sizeof(dma_addr_t)
> >>
> >>  drivers/usb/host/xhci-plat.c | 21 +
> >>  1 file changed, 13 insertions(+), 8 deletions(-)
> >>
> >>diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> >>index 890ad9d..e4c7f9d 100644
> >>--- a/drivers/usb/host/xhci-plat.c
> >>+++ b/drivers/usb/host/xhci-plat.c
> >>@@ -93,14 +93,19 @@ static int xhci_plat_probe(struct platform_device *pdev)
> >> if (irq < 0)
> >> return -ENODEV;
> >>
> >>-   /* Initialize dma_mask and coherent_dma_mask to 32-bits */
> >>-   ret = dma_set_coherent_mask(>dev, DMA_BIT_MASK(32));
> >>-   if (ret)
> >>-   return ret;
> >>-   if (!pdev->dev.dma_mask)
> >>-   pdev->dev.dma_mask = >dev.coherent_dma_mask;
> >>-   else
> >>-   dma_set_mask(>dev, DMA_BIT_MASK(32));
> >>+   /* Throw a waring if broken platform code didn't initialize 
> >>dma_mask */
> >>+   WARN_ON(!pdev->dev.dma_mask);
> >>+   /*
> >>+* Try setting dma_mask and coherent_dma_mask to 64 bits,
> >>+* then try 32 bits
> >>+*/
> >>+   ret = dma_coerce_mask_and_coherent(>dev, DMA_BIT_MASK(64));
> >>+   if (ret) {
> >>+   ret = dma_coerce_mask_and_coherent(>dev,
> >>+  DMA_BIT_MASK(32));
> >>+   if (ret)
> >>+   return ret;
> >>+   }

This isn't very good.  If dev.dma_mask is already set,
dma_coerce_mask_and_coherent() will always overwrite it.  There's also
no need to call it twice.  This, imho, is much better:

/* Try to set a 64-bit DMA mask first */
if (WARN_ON(!pdev->dev.dma_mask)) {
/* Eek, platform didn't initialise the streaming DMA mask */
ret = dma_coerce_mask_and_coherent(>dev, 
DMA_BIT_MASK(64));
} else {
ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(64));
}

/* If that failed, fall back to a 32-bit DMA mask */
if (ret) {
ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(32));
if (ret)
return ret;
}

since it preserves the dev.dma_mask pointer if it was properly setup

Really, drivers shouldn't be messing around with that pointer - especially
if it's already been correctly setup.  A platform may require separate
streaming and coherent masks, and we should respect that.

(The whole dma_mask being a pointer thing is a left-over from the PCI
layer which has never been cleaned up through fear of breaking something.)

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
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] Revert "usb: dwc3: gadget: drop unnecessary loop when cleaning up TRBs"

2015-09-01 Thread Ville Syrjälä 
On Tue, Sep 01, 2015 at 08:59:02AM -0500, Felipe Balbi wrote:
> Hi,
> 
> On Tue, Sep 01, 2015 at 04:17:00PM +0300, Ville Syrjälä wrote:
> > On Mon, Aug 31, 2015 at 01:50:10PM -0500, Felipe Balbi wrote:
> > > Hi,
> > > 
> > > On Mon, Aug 31, 2015 at 08:25:10PM +0300, Ville Syrjälä wrote:
> > > > On Mon, Aug 31, 2015 at 11:54:13AM -0500, Felipe Balbi wrote:
> > > > > On Mon, Aug 31, 2015 at 07:48:28PM +0300, 
> > > > > ville.syrj...@linux.intel.com wrote:
> > > > > > From: Ville Syrjälä <ville.syrj...@linux.intel.com>
> > > > > > 
> > > > > > This reverts commit 8f2c9544aba636134303105ecb164190a39dece4.
> > > > > > 
> > > > > > As it breaks g_ether on my Baytrail FFRD8 device. Everything starts 
> > > > > > out
> > > > > > fine, but after a bit of data has been transferred it just stops
> > > > > > flowing.
> > > > > > 
> > > > > > Note that I do get a bunch of these "NOHZ: local_softirq_pending 08"
> > > > > > when booting the machine, but I'm not really sure if they're related
> > > > > > to this problem.
> > > > > 
> > > > > I have a feeling your problem is elsewhere. We *are* completing one 
> > > > > TRB
> > > > > at a time. By reverting that commit you're just masking the real 
> > > > > problem
> > > > > and I'd rather get that one fixed.
> > > > > 
> > > > > How do you reproduce your issue ?
> > > > 
> > > > Just boot the system, it gets an IP from dnsmasq on my host, then I ssh
> > > > into it and do something to produce a bit of console output, after which
> > > > g_ether is dead. Eg. 'dmesg' a few times is enough to kill it.
> > > 
> > > which kernel version ?
> > 
> > Anything since the patch went in, so 4.1-rc
> > 
> > > Running as USB2 or USB3 ?
> > 
> > speed:480, so USB2 I presume?
> > 
> > > Have you tried
> > > linux-next ?
> > 
> > Tried it now (next-20150901). Equally bad as the rest.
> > 
> > > I just did 1000 dmesg iterations over ssh with g_ether and
> > > saw no issues.
> > > 
> > > Can you enable dwc3 tracepoints and try again ? (use some very large
> > > trace buffer, something around 2 or 4 MiB should be enough).
> > 
> > Attached one trace from linux-next, and another one with the revert on
> > top.
> 
> are you sure these come from next ?

Yep.

> It makes zero sense :-) Here's an
> odd snippet:
> 
> | sshd-1719  [000] d.s342.579785: dwc3_ep_queue: ep1in: req 
> 880077afa540 length 822/1514 ==> 0
> | sshd-1719  [000] d.s342.580075: dwc3_ep_queue: ep1in: req 
> 880077afa6c0 length 0/334 ==> -108
> |  systemd-network-1618  [003] d.s342.754796: dwc3_ep_queue: ep1in: req 
> 880077afa780 length 0/120 ==> -108
> 
> your requests are queued with -ESHUTDOWN!!

Looking at the code the tracepoint is before the request is queued, so
maybe there's just stale junk in req->status before it gets overwritten
by __dwc3_gadget_ep_queue()?

> 
> |   -0 [000] d.h342.877628: dwc3_readl: addr 
> c940040c value 0004
> |   -0 [000] d.h342.877635: dwc3_readl: addr 
> c9400408 value 0100
> |   -0 [000] d.h342.877638: dwc3_writel: addr 
> c9400408 value 8100
> |  irq/22-dwc3-753   [002] d..242.878300: dwc3_event: event 00c4
> 
> so you had an IRQ, fine.
> 
> |  irq/22-dwc3-753   [002] d..242.878312: dwc3_gadget: ep1out: reason 
> Transfer Not Active
> |  irq/22-dwc3-753   [002] d..242.878328: dwc3_gadget_ep_cmd: ep1out: 
> cmd 'Start Transfer' [6] params  77ad9030 
> 
> a transfer is started.
> 
> |  irq/22-dwc3-753   [002] d..242.878332: dwc3_writel: addr 
> c9400828 value 
> |  irq/22-dwc3-753   [002] d..242.878336: dwc3_writel: addr 
> c9400824 value 77ad9030
> |  irq/22-dwc3-753   [002] d..242.878339: dwc3_writel: addr 
> c9400820 value 
> |  irq/22-dwc3-753   [002] d..242.878342: dwc3_writel: addr 
> c940082c value 0406
> |  irq/22-dwc3-753   [002] d..242.878345: dwc3_readl: addr 
> c940082c value 00050006
> |  irq/22-dwc3-753   [002] d..242.878348: dwc3_gadget: Command 
> Complete --> 0
> |  irq/22-dwc3-753   [002] d..242.878350: dwc3

Re: [PATCH] Revert "usb: dwc3: gadget: drop unnecessary loop when cleaning up TRBs"

2015-09-01 Thread Felipe Balbi 
Hi,

On Tue, Sep 01, 2015 at 05:39:28PM +0300, Ville Syrjälä wrote:
> On Tue, Sep 01, 2015 at 08:59:02AM -0500, Felipe Balbi wrote:
> > Hi,
> > 
> > On Tue, Sep 01, 2015 at 04:17:00PM +0300, Ville Syrjälä wrote:
> > > On Mon, Aug 31, 2015 at 01:50:10PM -0500, Felipe Balbi wrote:
> > > > Hi,
> > > > 
> > > > On Mon, Aug 31, 2015 at 08:25:10PM +0300, Ville Syrjälä wrote:
> > > > > On Mon, Aug 31, 2015 at 11:54:13AM -0500, Felipe Balbi wrote:
> > > > > > On Mon, Aug 31, 2015 at 07:48:28PM +0300, 
> > > > > > ville.syrj...@linux.intel.com wrote:
> > > > > > > From: Ville Syrjälä <ville.syrj...@linux.intel.com>
> > > > > > > 
> > > > > > > This reverts commit 8f2c9544aba636134303105ecb164190a39dece4.
> > > > > > > 
> > > > > > > As it breaks g_ether on my Baytrail FFRD8 device. Everything 
> > > > > > > starts out
> > > > > > > fine, but after a bit of data has been transferred it just stops
> > > > > > > flowing.
> > > > > > > 
> > > > > > > Note that I do get a bunch of these "NOHZ: local_softirq_pending 
> > > > > > > 08"
> > > > > > > when booting the machine, but I'm not really sure if they're 
> > > > > > > related
> > > > > > > to this problem.
> > > > > > 
> > > > > > I have a feeling your problem is elsewhere. We *are* completing one 
> > > > > > TRB
> > > > > > at a time. By reverting that commit you're just masking the real 
> > > > > > problem
> > > > > > and I'd rather get that one fixed.
> > > > > > 
> > > > > > How do you reproduce your issue ?
> > > > > 
> > > > > Just boot the system, it gets an IP from dnsmasq on my host, then I 
> > > > > ssh
> > > > > into it and do something to produce a bit of console output, after 
> > > > > which
> > > > > g_ether is dead. Eg. 'dmesg' a few times is enough to kill it.
> > > > 
> > > > which kernel version ?
> > > 
> > > Anything since the patch went in, so 4.1-rc
> > > 
> > > > Running as USB2 or USB3 ?
> > > 
> > > speed:480, so USB2 I presume?
> > > 
> > > > Have you tried
> > > > linux-next ?
> > > 
> > > Tried it now (next-20150901). Equally bad as the rest.
> > > 
> > > > I just did 1000 dmesg iterations over ssh with g_ether and
> > > > saw no issues.
> > > > 
> > > > Can you enable dwc3 tracepoints and try again ? (use some very large
> > > > trace buffer, something around 2 or 4 MiB should be enough).
> > > 
> > > Attached one trace from linux-next, and another one with the revert on
> > > top.
> > 
> > are you sure these come from next ?
> 
> Yep.
> 
> > It makes zero sense :-) Here's an
> > odd snippet:
> > 
> > | sshd-1719  [000] d.s342.579785: dwc3_ep_queue: ep1in: req 
> > 880077afa540 length 822/1514 ==> 0
> > | sshd-1719  [000] d.s342.580075: dwc3_ep_queue: ep1in: req 
> > 880077afa6c0 length 0/334 ==> -108
> > |  systemd-network-1618  [003] d.s342.754796: dwc3_ep_queue: ep1in: req 
> > 880077afa780 length 0/120 ==> -108
> > 
> > your requests are queued with -ESHUTDOWN!!
> 
> Looking at the code the tracepoint is before the request is queued, so
> maybe there's just stale junk in req->status before it gets overwritten
> by __dwc3_gadget_ep_queue()?

right, something touched usb_request.status before and the request has
been recycled.

> > more requests are queued and that's it. No further traffic. It just
> > stopped working. No further IRQs, nothing.
> > 
> > mine looks very much different (see attached). I don't have any
> > -ESHUTDOWNs. How did you load g_ether ? Did you pass any extra options ?
> 
> g_ether is builtin, and I just pass g_ether.dev_addr= via kernel cmdline.
> 
> > Which IP version are you running ?
> 
> ipv4

I mean the SNPS IP :-) (it's 2.10a, see below)

> GSBUSCFG0 = 0x0006
> GSBUSCFG1 = 0x0f00
> GTXTHRCFG = 0x230a
> GRXTHRCFG = 0x2280
> GCTL = 0x45802002
> GEVTEN = 0x
> GSTS = 0x3e82
> GSNPSID = 0x5533210a

this could be a bug with 2.10a where completion IRQs are missed. Any
chance you can look for you Errata document and see if any exist ? I'm
using 2.40a.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v2 1/7] usb: misc: usbtest: allocate size of urb array according to user parameter

2015-09-01 Thread Alan Stern 
On Tue, 1 Sep 2015, Peter Chen wrote:

> Allocate the size of urb pointer array according to testusb's
> parameter sglen, and limits the length of sglen as MAX_SGLEN
> (128 currently).
> 
> Acked-by: Michal Nazarewicz 
> Signed-off-by: Peter Chen 
> ---
>  drivers/usb/misc/usbtest.c | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
> index 9517812..8f294d7 100644
> --- a/drivers/usb/misc/usbtest.c
> +++ b/drivers/usb/misc/usbtest.c
> @@ -95,6 +95,7 @@ static struct usb_device *testdev_to_usbdev(struct 
> usbtest_dev *test)
>   dev_warn(&(tdev)->intf->dev , fmt , ## args)
>  
>  #define GUARD_BYTE   0xA5
> +#define MAX_SGLEN128
>  
>  /*-*/
>  
> @@ -1911,10 +1912,7 @@ test_iso_queue(struct usbtest_dev *dev, struct 
> usbtest_param *param,
>   unsignedi;
>   unsigned long   packets = 0;
>   int status = 0;
> - struct urb  *urbs[10];  /* FIXME no limit */
> -
> - if (param->sglen > 10)
> - return -EDOM;
> + struct urb  *urbs[param->sglen];
>  
>   memset(, 0, sizeof(context));
>   context.count = param->iterations * param->sglen;
> @@ -2061,6 +2059,9 @@ usbtest_ioctl(struct usb_interface *intf, unsigned int 
> code, void *buf)
>   if (param->iterations <= 0)
>   return -EINVAL;
>  
> + if (param->sglen > MAX_SGLEN)
> + return -EINVAL;

This will not prevent problems.  The stack space gets allocated as soon
as the function starts, and if param->sglen is very big then the damage
will already have occurred by this point.

It's probably better simply to use kmalloc()/kfree() and not try to put
these things on the stack.

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

[PATCH] usb: dwc3: gadget: move trace_dwc3_ep_queue()

2015-09-01 Thread Felipe Balbi 
by moving trace_dwc3_ep_queue() from dwc3_gadget_ep_queue()
to __dwc3_gadget_ep_queue() after usb_request is properly
initialized, makes for a better output always showing a
request with 0 actual and -115 (-EINPROGRESS) status.

Signed-off-by: Felipe Balbi 
---
 drivers/usb/dwc3/gadget.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 0c25704dcb6b..1ea565f307e4 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1050,6 +1050,8 @@ static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep, 
struct dwc3_request *req)
req->direction  = dep->direction;
req->epnum  = dep->number;
 
+   trace_dwc3_ep_queue(req);
+
/*
 * We only add to our list of requests now and
 * start consuming the list once we get XferNotReady
@@ -1159,8 +1161,6 @@ static int dwc3_gadget_ep_queue(struct usb_ep *ep, struct 
usb_request *request,
goto out;
}
 
-   trace_dwc3_ep_queue(req);
-
ret = __dwc3_gadget_ep_queue(dep, req);
 
 out:
-- 
2.5.0

--
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] usbnet: Fix a race between usbnet_stop() and the BH

2015-09-01 Thread Eugene Shatokhin 
The race may happen when a device (e.g. YOTA 4G LTE Modem) is
unplugged while the system is downloading a large file from the Net.

Hardware breakpoints and Kprobes with delays were used to confirm that
the race does actually happen.

The race is on skb_queue ('next' pointer) between usbnet_stop()
and rx_complete(), which, in turn, calls usbnet_bh().

Here is a part of the call stack with the code where the changes to the
queue happen. The line numbers are for the kernel 4.1.0:

*0 __skb_unlink (skbuff.h:1517)
prev->next = next;
*1 defer_bh (usbnet.c:430)
spin_lock_irqsave(>lock, flags);
old_state = entry->state;
entry->state = state;
__skb_unlink(skb, list);
spin_unlock(>lock);
spin_lock(>done.lock);
__skb_queue_tail(>done, skb);
if (dev->done.qlen == 1)
tasklet_schedule(>bh);
spin_unlock_irqrestore(>done.lock, flags);
*2 rx_complete (usbnet.c:640)
state = defer_bh(dev, skb, >rxq, state);

At the same time, the following code repeatedly checks if the queue is
empty and reads these values concurrently with the above changes:

*0  usbnet_terminate_urbs (usbnet.c:765)
/* maybe wait for deletions to finish. */
while (!skb_queue_empty(>rxq)
&& !skb_queue_empty(>txq)
&& !skb_queue_empty(>done)) {
schedule_timeout(msecs_to_jiffies(UNLINK_TIMEOUT_MS));
set_current_state(TASK_UNINTERRUPTIBLE);
netif_dbg(dev, ifdown, dev->net,
  "waited for %d urb completions\n", temp);
}
*1  usbnet_stop (usbnet.c:806)
if (!(info->flags & FLAG_AVOID_UNLINK_URBS))
usbnet_terminate_urbs(dev);

As a result, it is possible, for example, that the skb is removed from
dev->rxq by __skb_unlink() before the check
"!skb_queue_empty(>rxq)" in usbnet_terminate_urbs() is made. It is
also possible in this case that the skb is added to dev->done queue
after "!skb_queue_empty(>done)" is checked. So
usbnet_terminate_urbs() may stop waiting and return while dev->done
queue still has an item.

Locking in defer_bh() and usbnet_terminate_urbs() was revisited to avoid
this race.

Signed-off-by: Eugene Shatokhin 
---
 drivers/net/usb/usbnet.c | 39 ---
 1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index e049857..b4cf107 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -428,12 +428,18 @@ static enum skb_state defer_bh(struct usbnet *dev, struct 
sk_buff *skb,
old_state = entry->state;
entry->state = state;
__skb_unlink(skb, list);
-   spin_unlock(>lock);
-   spin_lock(>done.lock);
+
+   /* defer_bh() is never called with list == >done.
+* spin_lock_nested() tells lockdep that it is OK to take
+* dev->done.lock here with list->lock held.
+*/
+   spin_lock_nested(>done.lock, SINGLE_DEPTH_NESTING);
+
__skb_queue_tail(>done, skb);
if (dev->done.qlen == 1)
tasklet_schedule(>bh);
-   spin_unlock_irqrestore(>done.lock, flags);
+   spin_unlock(>done.lock);
+   spin_unlock_irqrestore(>lock, flags);
return old_state;
 }
 
@@ -749,6 +755,20 @@ EXPORT_SYMBOL_GPL(usbnet_unlink_rx_urbs);
 
 /*-*/
 
+static void wait_skb_queue_empty(struct sk_buff_head *q)
+{
+   unsigned long flags;
+
+   spin_lock_irqsave(>lock, flags);
+   while (!skb_queue_empty(q)) {
+   spin_unlock_irqrestore(>lock, flags);
+   schedule_timeout(msecs_to_jiffies(UNLINK_TIMEOUT_MS));
+   set_current_state(TASK_UNINTERRUPTIBLE);
+   spin_lock_irqsave(>lock, flags);
+   }
+   spin_unlock_irqrestore(>lock, flags);
+}
+
 // precondition: never called in_interrupt
 static void usbnet_terminate_urbs(struct usbnet *dev)
 {
@@ -762,14 +782,11 @@ static void usbnet_terminate_urbs(struct usbnet *dev)
unlink_urbs(dev, >rxq);
 
/* maybe wait for deletions to finish. */
-   while (!skb_queue_empty(>rxq)
-   && !skb_queue_empty(>txq)
-   && !skb_queue_empty(>done)) {
-   schedule_timeout(msecs_to_jiffies(UNLINK_TIMEOUT_MS));
-   set_current_state(TASK_UNINTERRUPTIBLE);
-   netif_dbg(dev, ifdown, dev->net,
- "waited for %d urb completions\n", temp);
-   }
+   wait_skb_queue_empty(>rxq);
+   wait_skb_queue_empty(>txq);
+   wait_skb_queue_empty(>done);
+   netif_dbg(dev, ifdown, dev->net,
+ "waited for %d urb completions\n", temp);
set_current_state(TASK_RUNNING);
remove_wait_queue(>wait, );
 }
-- 
2.3.2

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

Re: [PATCH] usb: core: Fix side effect of clear port feature in hub port reset

2015-09-01 Thread Alan Stern 
On Tue, 1 Sep 2015, hyunho747.kim wrote:

> After successful port reset by set_port_feature, some devices show
> immediate link connection which generates port connect change interrupt.
> But, the next step is an unconditional usb_clear_port_feature
> and this flow always clears USB_PORT_FEAT_C_CONNECTION bit in port status
> though next kick_khubd is reserved by link connection interrupt.
> This flow can make an ambiguous state in the next port_evnet operation,
> port_status is connected state such as 0x203 but
> port_change is zero value caused by the previous clear port feature.
> So, port_event can't call hub_port_connect_change and
> there is no other way to peform connect procedure.

It sounds like you have not read this comment in hub_port_wait_reset():

/* bomb out completely if the connection bounced.  A USB 3.0
 * connection may bounce if multiple warm resets were issued,
 * but the device may have successfully re-connected. Ignore it.
 */

I agree that clearing the USB_PORT_FEAT_C_CONNECTION bit after reading
the port status can cause a race.  However I don't think your solution
is the correct one.

> Signed-off-by: hyunho747.kim 

You need to put a real name here, not an email address.  I suspect your 
real name isn't "hyunho747.kim".

> ---
>  drivers/usb/core/hub.c | 11 +++
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 73dfa19..4508aff 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -2761,18 +2761,21 @@ static int hub_port_reset(struct usb_hub *hub, int 
> port1,
>  
>   /* Check for disconnect or reset */
>   if (status == 0 || status == -ENOTCONN || status == -ENODEV) {
> - usb_clear_port_feature(hub->hdev, port1,
> + if (status)
> + usb_clear_port_feature(hub->hdev, port1,
>   USB_PORT_FEAT_C_RESET);

What is the reason for this change?  Clearing the USB_PORT_FEAT_C_RESET 
bit won't cause any problems.

>   if (!hub_is_superspeed(hub->hdev))
>   goto done;
>  
> - usb_clear_port_feature(hub->hdev, port1,
> + if (status) {
> + usb_clear_port_feature(hub->hdev, port1,
>   USB_PORT_FEAT_C_BH_PORT_RESET);

And what is the reason for this change?

> - usb_clear_port_feature(hub->hdev, port1,
> + usb_clear_port_feature(hub->hdev, port1,
>   USB_PORT_FEAT_C_PORT_LINK_STATE);
> - usb_clear_port_feature(hub->hdev, port1,
> + usb_clear_port_feature(hub->hdev, port1,
>   USB_PORT_FEAT_C_CONNECTION);
> + }

I'm not so sure about these changes.  The bits definitely do need to be 
cleared at some point, but this may not be the right place to clear 
them.  Or maybe we need to check that the port is still enabled after 
the bits have been cleared.

You need to think more carefully about this patch.

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

Re: [PATCH 2/2] usbnet: Fix a race between usbnet_stop() and the BH

2015-09-01 Thread Eugene Shatokhin 

01.09.2015 10:58, Oliver Neukum пишет:

On Mon, 2015-08-31 at 11:50 +0300, Eugene Shatokhin wrote:

But I would have liked it much better if the code became simpler

instead

of more complex.


Me too, but I can see no other way here. The code is simpler without
locking, indeed, but locking is needed to prevent the problems
described
earlier.


On a practical note, will you resend the patch?


Yes, I will do it shortly. Thanks for your help!

Regards,

Eugene



--
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: [RESEND PATCH 3/7] usb: phy: isp1301: Export I2C module alias information

2015-09-01 Thread Javier Martinez Canillas 
Hello Greg and Felipe,

On 08/25/2015 08:31 AM, Javier Martinez Canillas wrote:
> The I2C core always reports the MODALIAS uevent as "i2c: regardless if the driver was matched using the I2C id_table or the
> of_match_table. So the driver needs to export the I2C table and this
> be built into the module or udev won't have the necessary information
> to auto load the correct module when the device is added.
> 
> Signed-off-by: Javier Martinez Canillas 
> 
> ---
> 
>  drivers/usb/phy/phy-isp1301.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/usb/phy/phy-isp1301.c b/drivers/usb/phy/phy-isp1301.c
> index 8a55b37d1a02..db68156568e6 100644
> --- a/drivers/usb/phy/phy-isp1301.c
> +++ b/drivers/usb/phy/phy-isp1301.c
> @@ -31,6 +31,7 @@ static const struct i2c_device_id isp1301_id[] = {
>   { "isp1301", 0 },
>   { }
>  };
> +MODULE_DEVICE_TABLE(i2c, isp1301_id);
>  
>  static struct i2c_client *isp1301_i2c_client;
>  
>

Any comments about this patch? The first version
was posted about a month ago and is very trivial.
 
Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
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] Revert "usb: dwc3: gadget: drop unnecessary loop when cleaning up TRBs"

2015-09-01 Thread Ville Syrjälä 
On Tue, Sep 01, 2015 at 10:17:59AM -0500, Felipe Balbi wrote:
> Hi,
> 
> On Tue, Sep 01, 2015 at 05:39:28PM +0300, Ville Syrjälä wrote:
> > On Tue, Sep 01, 2015 at 08:59:02AM -0500, Felipe Balbi wrote:
> > > Hi,
> > > 
> > > On Tue, Sep 01, 2015 at 04:17:00PM +0300, Ville Syrjälä wrote:
> > > > On Mon, Aug 31, 2015 at 01:50:10PM -0500, Felipe Balbi wrote:
> > > > > Hi,
> > > > > 
> > > > > On Mon, Aug 31, 2015 at 08:25:10PM +0300, Ville Syrjälä wrote:
> > > > > > On Mon, Aug 31, 2015 at 11:54:13AM -0500, Felipe Balbi wrote:
> > > > > > > On Mon, Aug 31, 2015 at 07:48:28PM +0300, 
> > > > > > > ville.syrj...@linux.intel.com wrote:
> > > > > > > > From: Ville Syrjälä <ville.syrj...@linux.intel.com>
> > > > > > > > 
> > > > > > > > This reverts commit 8f2c9544aba636134303105ecb164190a39dece4.
> > > > > > > > 
> > > > > > > > As it breaks g_ether on my Baytrail FFRD8 device. Everything 
> > > > > > > > starts out
> > > > > > > > fine, but after a bit of data has been transferred it just stops
> > > > > > > > flowing.
> > > > > > > > 
> > > > > > > > Note that I do get a bunch of these "NOHZ: 
> > > > > > > > local_softirq_pending 08"
> > > > > > > > when booting the machine, but I'm not really sure if they're 
> > > > > > > > related
> > > > > > > > to this problem.
> > > > > > > 
> > > > > > > I have a feeling your problem is elsewhere. We *are* completing 
> > > > > > > one TRB
> > > > > > > at a time. By reverting that commit you're just masking the real 
> > > > > > > problem
> > > > > > > and I'd rather get that one fixed.
> > > > > > > 
> > > > > > > How do you reproduce your issue ?
> > > > > > 
> > > > > > Just boot the system, it gets an IP from dnsmasq on my host, then I 
> > > > > > ssh
> > > > > > into it and do something to produce a bit of console output, after 
> > > > > > which
> > > > > > g_ether is dead. Eg. 'dmesg' a few times is enough to kill it.
> > > > > 
> > > > > which kernel version ?
> > > > 
> > > > Anything since the patch went in, so 4.1-rc
> > > > 
> > > > > Running as USB2 or USB3 ?
> > > > 
> > > > speed:480, so USB2 I presume?
> > > > 
> > > > > Have you tried
> > > > > linux-next ?
> > > > 
> > > > Tried it now (next-20150901). Equally bad as the rest.
> > > > 
> > > > > I just did 1000 dmesg iterations over ssh with g_ether and
> > > > > saw no issues.
> > > > > 
> > > > > Can you enable dwc3 tracepoints and try again ? (use some very large
> > > > > trace buffer, something around 2 or 4 MiB should be enough).
> > > > 
> > > > Attached one trace from linux-next, and another one with the revert on
> > > > top.
> > > 
> > > are you sure these come from next ?
> > 
> > Yep.
> > 
> > > It makes zero sense :-) Here's an
> > > odd snippet:
> > > 
> > > | sshd-1719  [000] d.s342.579785: dwc3_ep_queue: ep1in: 
> > > req 880077afa540 length 822/1514 ==> 0
> > > | sshd-1719  [000] d.s342.580075: dwc3_ep_queue: ep1in: 
> > > req 880077afa6c0 length 0/334 ==> -108
> > > |  systemd-network-1618  [003] d.s342.754796: dwc3_ep_queue: ep1in: 
> > > req 880077afa780 length 0/120 ==> -108
> > > 
> > > your requests are queued with -ESHUTDOWN!!
> > 
> > Looking at the code the tracepoint is before the request is queued, so
> > maybe there's just stale junk in req->status before it gets overwritten
> > by __dwc3_gadget_ep_queue()?
> 
> right, something touched usb_request.status before and the request has
> been recycled.
> 
> > > more requests are queued and that's it. No further traffic. It just
> > > stopped working. No further IRQs, nothing.
> > > 
> > > mine looks very much different (see attached). I don't have any
> > > -ESHUTDOWNs. How did you load g_ether ? Did you pass any extra options ?
> > 
> > g_ether is builtin, and I just pass g_ether.dev_addr= via kernel 
> > cmdline.
> > 
> > > Which IP version are you running ?
> > 
> > ipv4
> 
> I mean the SNPS IP :-) (it's 2.10a, see below)

It's all Greek to me :)

> 
> > GSBUSCFG0 = 0x0006
> > GSBUSCFG1 = 0x0f00
> > GTXTHRCFG = 0x230a
> > GRXTHRCFG = 0x2280
> > GCTL = 0x45802002
> > GEVTEN = 0x
> > GSTS = 0x3e82
> > GSNPSID = 0x5533210a
> 
> this could be a bug with 2.10a where completion IRQs are missed. Any
> chance you can look for you Errata document and see if any exist ? I'm
> using 2.40a.

Ugh. USB isn't my thing, so I'm definitely not going to start hunting down
any obscure docs.

Cc:ing Mathias and Heikki since it looks like they've touched this beast
before. You guys have any docs and/or clue as to what's happening here?

-- 
Ville Syrjälä
Intel OTC
--
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 v6 2/5] dt-bindings: Add a binding for Mediatek xHCI host controller

2015-09-01 Thread chunfeng yun 
hi, 
On Sat, 2015-08-22 at 16:23 +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 8/22/2015 4:45 AM, Chunfeng Yun wrote:
> 
> > add a DT binding documentation of xHCI host controller for the
> > MT8173 SoC from Mediatek.
> 
> > Signed-off-by: Chunfeng Yun 
> > ---
> >   .../devicetree/bindings/usb/mt8173-xhci.txt| 52 
> > ++
> >   1 file changed, 52 insertions(+)
> >   create mode 100644 Documentation/devicetree/bindings/usb/mt8173-xhci.txt
> >
> > diff --git a/Documentation/devicetree/bindings/usb/mt8173-xhci.txt 
> > b/Documentation/devicetree/bindings/usb/mt8173-xhci.txt
> > new file mode 100644
> > index 000..8e6e463
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/mt8173-xhci.txt
> > @@ -0,0 +1,52 @@
> > +mt65XX xHCI
> 
> The file is called mt8173-xhci.txt. Are you sure MT65xx should be here?
> 
I will revise it later.

> > +
> > +The device node for Mediatek SOC usb3.0 host controller
> > +
> > +Required properties:
> > + - compatible : should contain "mediatek,mt8173-xhci"
> > + - reg : specifies physical base address and size of the registers,
> > +   the first one for MAC, the second for IPPC
> > + - interrupts : interrupt used by the controller
> > + - power-domains : a phandle to USB power domain node to control usb's
> 
> USB's -- be consistent, please.
> 
> > +   mtcmos
> > + - vusb33-supply : regulator of usb avdd3.3v
> > +
> > + - clocks : a list of phandle + clock-specifier pairs, one for each
> > +   entry in clock-names
> > + - clock-names : must contain
> > +   "sys_ck": for clock of xHCI MAC
> > +   "wakeup_deb_p0": for usb wakeup debounce clock of port0
> > +   "wakeup_deb_p0": for usb wakeup debounce clock of port1
> 
> USB.
> 
> > +
> > + - phys : a list of phandle + phy specifier pairs
> > + - usb3-lpm-capable : supports USB3 LPM
> > + - mediatek,syscon-wakeup : phandle to syscon used to access usb wakeup
> > +   control register
> > + - mediatek,wakeup-src : 1: ip sleep wakeup mode; 2: line state wakeup
> > +   mode; others means don't enable wakeup source of usb
> 
> "Others mean" and "USB".
> 
I will replace all 'usb' by "USB"

Thanks a lot, and sorry for later reply.
> [...]
> 
> MBR, Sergei
> 


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