Re: [PATCH v4] Move DWC2 driver out of staging

2014-02-01 Thread Andre Heider
On Fri, Jan 31, 2014 at 11:48:37PM -0700, Stephen Warren wrote:
 On 01/31/2014 11:12 AM, Andre Heider wrote:
  On Mon, Jan 13, 2014 at 01:50:09PM -0800, Paul Zimmerman wrote:
  The DWC2 driver should now be in good enough shape to move out of
  staging. I have stress tested it overnight on RPI running mass
  storage and Ethernet transfers in parallel, and for several days
  on our proprietary PCI-based platform.
 ...
  this looks just fine, but for whatever reason it breaks sdhci on my rpi.
  With today's Linus' master the dwc2 controller seems to initialize fine,
  but I get this upon boot:
  
  [1.783316] sdhci-bcm2835 2030.sdhci: sdhci_pltfm_init failed -12
  [1.794820] sdhci-bcm2835: probe of 2030.sdhci failed with error -12
 
  That is:
  
  struct sdhci_host *sdhci_pltfm_init(struct platform_device 
  *pdev,
  const struct 
  sdhci_pltfm_data *pdata,
  size_t priv_size)
  {
  ...
  
  iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
  if (!iomem) {
  ret = -ENOMEM;
  goto err;
  }
 
 This is due to the following code:
 
 static void _dwc2_hcd_endpoint_reset(struct usb_hcd *hcd,
struct usb_host_endpoint *ep)
 {
   struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
 ...
   struct usb_device *udev;
 ...
   udev = to_usb_device(hsotg-dev);
 ...
   usb_settoggle(udev, epnum, is_out, 0);
   if (is_control)
   usb_settoggle(udev, epnum, !is_out, 0);
 
 The problem is that hsotg-dev is assigned as follows:
 
 static int dwc2_driver_probe(struct platform_device *dev)
 ...
   hsotg = devm_kzalloc(dev-dev, sizeof(*hsotg), GFP_KERNEL);
 ...
   hsotg-dev = dev-dev;
 
 As such, it's not legal to call to_usb_device() on it.
 
 What ends up happening, simply due to memory allocation order, is that
 the memory writes inside usb_settoggle() end up setting the SDHCI struct
 platform_device's num_resources to 0, so that it's call to
 platform_get_resource() fails.
 
 With the DWC2 move patch reverted, some other random piece of memory is
 being corrupted, which just happens not to cause any visible problem.
 Likely it's some other struct platform_device that's already had its
 resources read by the time DWC2 probes and corrupts them.
 
 (Yes, this was hard to find!)

Nice work, but how did you pinpoint this? Am I missing some option/tool
or did I just not stare for long enough?

 I honestly can't see how to solve this myself, since the whole DWC2
 driver doesn't seem to have a struct usb_device * hanging around that we
 can stash somewhere for it to look up later. Perhaps someone more
 familiar with the USB stack can help with that.
--
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 RFC 1/1] usb: Tell xhci when usb data might be misaligned

2014-02-01 Thread Mark Lord
On 14-02-01 02:54 AM, Ming Lei wrote:
..
 With SG enabled, for the iperf client test case, the average urb size
 for transmission will be increased from ~1500 to ~20K bytes in my
 test case:
 
iperf -c $SRV -t 30 -P 4 -w 128K
 
 So I am wondering you guys do not care the improvement ..

No, that's not it.  Simply, the recent changes killed the driver
for some users, something Linus calls a regression, and does not permit.

Far better to have it continue to work than not to work.
The plan discussed earlier calls for reintroduction of SG here
once the problems are solved outside of the main tree.
-- 
Mark Lord
Real-Time Remedies Inc.
ml...@pobox.com
--
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 RFC 1/1] usb: Tell xhci when usb data might be misaligned

2014-02-01 Thread Ming Lei
On Sat, Feb 1, 2014 at 9:30 PM, Mark Lord ml...@pobox.com wrote:
 On 14-02-01 02:54 AM, Ming Lei wrote:
 ..
 With SG enabled, for the iperf client test case, the average urb size
 for transmission will be increased from ~1500 to ~20K bytes in my
 test case:

iperf -c $SRV -t 30 -P 4 -w 128K

 So I am wondering you guys do not care the improvement ..

 No, that's not it.  Simply, the recent changes killed the driver

I just want to clarify the sg approach does improve performance,
instead of no improvement mentioned by your guys.

 for some users, something Linus calls a regression, and does not permit.

Even real regressions are easily/often introduced, and we are discussing
how to fix that. I suggest to unset the flag only for the known buggy
controllers.


Thanks,
--
Ming Lei
--
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: SCSI Sense Information in usb-storage

2014-02-01 Thread Alan Stern
On Sat, 1 Feb 2014, Steph Nguyen wrote:

 Hi
 
 It seems that when sending SCSI commands over USB, the kernel doesn't 
 necessary build a sense response. For instance, when sending REQUEST 
 SENSE over ehci, the scsi_cmnd-sense_buffer is left NULL; if I run the 
 same REQUEST SENSE command over a direct SCSI connect, the kernel builds 
 the scsi_cmnd-sense_buffer.

There's no need for the kernel to build a sense buffer when sending a 
REQUEST SENSE command.  That command always succeeds, so there's no 
reason to ask for sense data which might explain why the command 
failed.

Your question isn't very precise.  How did you run a REQUEST SENSE
command?  When using usb-storage under normal conditions, the SCSI
layer doesn't ever send that command.

 So is there a reason why usb-storage doesn't build the sense buffer?

In general, usb-storage doesn't touch scsi_cmnd except to set 
scsi_cmnd-result.  (There are a few exceptions, but this is mostly 
true.)

If the SCSI layer wants to receive sense data, it has to set the 
scsi_cmnd-sense_buffer field itself.  Normally it does so.  For 
example, see what happens when the SCSI layer issues a TEST UNIT READY 
command to usb-storage.

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 v4] Move DWC2 driver out of staging

2014-02-01 Thread Alan Stern
On Fri, 31 Jan 2014, Stephen Warren wrote:

 This is due to the following code:
 
 static void _dwc2_hcd_endpoint_reset(struct usb_hcd *hcd,
struct usb_host_endpoint *ep)
 {
   struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
 ...
   struct usb_device *udev;
 ...
   udev = to_usb_device(hsotg-dev);
 ...
   usb_settoggle(udev, epnum, is_out, 0);
   if (is_control)
   usb_settoggle(udev, epnum, !is_out, 0);
 
 The problem is that hsotg-dev is assigned as follows:
 
 static int dwc2_driver_probe(struct platform_device *dev)
 ...
   hsotg = devm_kzalloc(dev-dev, sizeof(*hsotg), GFP_KERNEL);
 ...
   hsotg-dev = dev-dev;
 
 As such, it's not legal to call to_usb_device() on it.

This clearly is a bug.  I suspect the driver has other bugs too, such
as not holding the private lock over a large enough region.

 What ends up happening, simply due to memory allocation order, is that
 the memory writes inside usb_settoggle() end up setting the SDHCI struct
 platform_device's num_resources to 0, so that it's call to
 platform_get_resource() fails.
 
 With the DWC2 move patch reverted, some other random piece of memory is
 being corrupted, which just happens not to cause any visible problem.
 Likely it's some other struct platform_device that's already had its
 resources read by the time DWC2 probes and corrupts them.
 
 (Yes, this was hard to find!)
 
 I honestly can't see how to solve this myself, since the whole DWC2
 driver doesn't seem to have a struct usb_device * hanging around that we
 can stash somewhere for it to look up later. Perhaps someone more
 familiar with the USB stack can help with that.

The correct solution is to set

qh = ep-hcpriv;
udev = qh-udev;

However, the driver doesn't store udev in qh (dwc2_qh_init() should do
this, but it doesn't).  In fact, the field doesn't even exist.

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: SCSI Sense Information in usb-storage

2014-02-01 Thread Alan Stern
On Sat, 1 Feb 2014, Steph Nguyen wrote:

  On Sat, 1 Feb 2014, Steph Nguyen wrote:
 
  Hi
 
  It seems that when sending SCSI commands over USB, the kernel doesn't
  necessary build a sense response. For instance, when sending REQUEST
  SENSE over ehci, the scsi_cmnd-sense_buffer is left NULL; if I run the
  same REQUEST SENSE command over a direct SCSI connect, the kernel builds
  the scsi_cmnd-sense_buffer.
 
  There's no need for the kernel to build a sense buffer when sending a
  REQUEST SENSE command.  That command always succeeds, so there's no
  reason to ask for sense data which might explain why the command
  failed.
 
 I'm using sg_requests. When running it against a disk attached to a usb 
 port, the sense_buffer is simply empty. When run against the same disk 
 directly attached to a scsi controller, the sense_buffer is built by the 
 kernel. So same device, same command, two different hw paths lead to two 
 different behaviors.

Hold on a second.  Above you said build a sense response and 
scsi_cmnd-sense_buffer is left NULL.  Those are two different 
things.

In one case we would have scsi_cmnd-sense_buffer == NULL.  In the
other case, we would have scsi_cmnd-sense_buffer != NULL and
scsi_cmnd-sense_buffer[i] = 0 for each i.

Which do you really mean?

  In general, usb-storage doesn't touch scsi_cmnd except to set
  scsi_cmnd-result.  (There are a few exceptions, but this is mostly
  true.)

 Right, it makes sense that the scsi subsystem takes care of this. But, 
 looking at drivers/usb/storage/cypress_atacb.c, I can see that the sense 
 buffer for ATA_16 is built by the kernel. So I suppose this is one of 
 these exceptions you are referring to.

No.  Even in cypress_atacb.c, usb-storage does not assign anything to 
scsi_cmnd-sense_buffer.  That is, it does not do anything like

scsi_cmnd-sense_buffer = X;

However, it does fill in the contents of the sense buffer.  That's as
it should be; usb-storage fills in the sense buffer contents for any
command that completes abnormally.

The reason you didn't see anything get filled in for your REQUEST SENSE 
command is because the command completed normally.

I have no idea what happens with other SCSI transports.

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: SCSI Sense Information in usb-storage

2014-02-01 Thread Steph Nguyen

On Sat, 1 Feb 2014, Steph Nguyen wrote:


On Sat, 1 Feb 2014, Steph Nguyen wrote:


Hi

It seems that when sending SCSI commands over USB, the kernel doesn't
necessary build a sense response. For instance, when sending REQUEST
SENSE over ehci, the scsi_cmnd-sense_buffer is left NULL; if I run the
same REQUEST SENSE command over a direct SCSI connect, the kernel builds
the scsi_cmnd-sense_buffer.


There's no need for the kernel to build a sense buffer when sending a
REQUEST SENSE command.  That command always succeeds, so there's no
reason to ask for sense data which might explain why the command
failed.


I'm using sg_requests. When running it against a disk attached to a usb
port, the sense_buffer is simply empty. When run against the same disk
directly attached to a scsi controller, the sense_buffer is built by the
kernel. So same device, same command, two different hw paths lead to two
different behaviors.


Hold on a second.  Above you said build a sense response and
scsi_cmnd-sense_buffer is left NULL.  Those are two different
things.

In one case we would have scsi_cmnd-sense_buffer == NULL.  In the
other case, we would have scsi_cmnd-sense_buffer != NULL and
scsi_cmnd-sense_buffer[i] = 0 for each i.

Which do you really mean?



I mean scsi_cmnd-sense_buffer == NULL;


In general, usb-storage doesn't touch scsi_cmnd except to set
scsi_cmnd-result.  (There are a few exceptions, but this is mostly
true.)



Right, it makes sense that the scsi subsystem takes care of this. But,
looking at drivers/usb/storage/cypress_atacb.c, I can see that the sense
buffer for ATA_16 is built by the kernel. So I suppose this is one of
these exceptions you are referring to.


No.  Even in cypress_atacb.c, usb-storage does not assign anything to
scsi_cmnd-sense_buffer.  That is, it does not do anything like

scsi_cmnd-sense_buffer = X;

However, it does fill in the contents of the sense buffer.  That's as
it should be; usb-storage fills in the sense buffer contents for any
command that completes abnormally.

The reason you didn't see anything get filled in for your REQUEST SENSE
command is because the command completed normally.

I have no idea what happens with other SCSI transports.



To be clear, here's what I meant by build - I'm just pasting the 
relevant bits of the code:


static void cypress_atacb_passthrough(struct scsi_cmnd *srb, struct 
us_data *us) {

...
unsigned char *sb = srb-sense_buffer;
...
	if ((srb-result != (DID_ERROR  16)  srb-result != (DID_ABORT  
16))  ...)

memset(sb, 0, SCSI_SENSE_BUFFERSIZE);
...
sb[0] = 0x72;

So, unless I'm completely misunderstanding the code, here the command is 
supposed to have completed successfully, isn't it? And the 
sense_buffer[0] is filled with 0x72 when it completes successfully.



Alan Stern




Thanks
Steph
--
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: SCSI Sense Information in usb-storage

2014-02-01 Thread Alan Stern
On Sat, 1 Feb 2014, Steph Nguyen wrote:

 To be clear, here's what I meant by build - I'm just pasting the 
 relevant bits of the code:
 
 static void cypress_atacb_passthrough(struct scsi_cmnd *srb, struct 
 us_data *us) {
   ...
   unsigned char *sb = srb-sense_buffer;
   ...
   if ((srb-result != (DID_ERROR  16)  srb-result != (DID_ABORT  
 16))  ...)
   memset(sb, 0, SCSI_SENSE_BUFFERSIZE);
   ...
   sb[0] = 0x72;
 
 So, unless I'm completely misunderstanding the code, here the command is 
 supposed to have completed successfully, isn't it? And the 
 sense_buffer[0] is filled with 0x72 when it completes successfully.

That's right.  But here the command isn't a SCSI command; it's an ATA
command getting sent over a SCSI transport.

Furthermore, this doesn't happen unless scsi_cmnd-cmnd[2]  0x20 is
nonzero (part of the if condition that you left out), which
apparently is a flag indicating that the command is always supposed to
return sense data, even when it completes normally (the comment calls
it ck_cond).

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


[DRIVER][WIP] Playstation controllers (DualShock 3/4, Navigation/Motion Controller, ...)

2014-02-01 Thread jb b
Hello,

I am a total newbie in Linux kernel development (but not in C
programming), and I started to write a USB driver for the DualShock 4
(and also others controllers/equipments for the PlayStation 3/4 while
I'm at it).

My objective is to sanitize what is reported to the userspace (most
games don't like 28+ axes all with a value of -32767), add support for
the trackpad/sound output on the DualShock 4, and (finally!) have
proper Linux support for the DualShock 3.

So far I managed to report all axes and buttons for the DualShock 4,
and also the touchpad (currently reported as 4 axis, unsure how to
deal with it for now...).

I also wrote initial support for :
Sony DualShock 3
Sony Navigation Controller
Sony Motion Controller

But it's more proof-of-concept than ready to release at this point.

To use it, download all files, run make, then run ./setup.sh as root
to install the driver (as a total newbie in Linux programming, I'm not
sure how to integrate this in the Linux build process).

ppad.c contains my driver for all controllers as it is now,
ppad_dualshock4.c is the same file with only DualShock 4 support for
ease of reviewing. I used the xpad driver (Xbox controllers) as a
starting point.

The files are hosted on Google Code :
http://code.google.com/p/linux-playstation-drivers/source/browse/

PS : do NOT pull into a Linux Git repository, this driver is NOT
integrated in the Linux source tree!

Feedback would be much appreciated :-)

Jean-Baptiste Boric
--
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 v4 00/14] port power control fixes

2014-02-01 Thread Alan Stern
On Fri, 31 Jan 2014, Dan Williams wrote:

 Toggling port power currently leads to three unintended disconnect
 scenarios that are addressed by this rework of port power recovery and
 usb device resume:
 
 1/ Superspeed devices downgrade to their hispeed connection: fix this by
preventing superspeed poweroff until the peer port is suspended.
This depends on the ability to identify peer ports (patches 1-4), and
then patch 5 implements the policy.
 
 2/ khubd prematurely disconnects ports that are in the process of
being resumed or reset.  khubd now ignores ports in the
pm-runtime-suspended state (patch 9) and holds a lock to synchronize the 
 port
status changes of usb_port_{suspend|resume} (patch 11).
 
 3/ Superspeed devices fail to reconnect: Seen during repeated toggles of
the port power state.  Fixed by forcing a full recovery cycle of the
usb_device before allowing the next suspend / khubd run (patch 12).
Also, for devices that live lock on reconnect the port runtime resume
path now has the capability to force a reset-resume to be a
warm-reset-resume (patch 13).

I haven't gone through all this in any detail.  But a couple of things 
stand out immediately, matters of style.

Although the USB stack still has plenty of old code with multiline
comments looking this this:

/* blah blah blah
 * blah blah blah
 */

the accepted style is that all new code should appear like this:

/*
 * blah blah blah
 * blah blah blah
 */

Also, the style for indentation througout most of the USB stack is that
continuation lines are indented by two tab stops beyond the first line
of the statement.  They are not aligned with opening parentheses of
function calls or anything like that.

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 v4 00/14] port power control fixes

2014-02-01 Thread Dan Williams
On Sat, Feb 1, 2014 at 10:44 AM, Alan Stern st...@rowland.harvard.edu wrote:
 On Fri, 31 Jan 2014, Dan Williams wrote:

 Toggling port power currently leads to three unintended disconnect
 scenarios that are addressed by this rework of port power recovery and
 usb device resume:

 1/ Superspeed devices downgrade to their hispeed connection: fix this by
preventing superspeed poweroff until the peer port is suspended.
This depends on the ability to identify peer ports (patches 1-4), and
then patch 5 implements the policy.

 2/ khubd prematurely disconnects ports that are in the process of
being resumed or reset.  khubd now ignores ports in the
pm-runtime-suspended state (patch 9) and holds a lock to synchronize the 
 port
status changes of usb_port_{suspend|resume} (patch 11).

 3/ Superspeed devices fail to reconnect: Seen during repeated toggles of
the port power state.  Fixed by forcing a full recovery cycle of the
usb_device before allowing the next suspend / khubd run (patch 12).
Also, for devices that live lock on reconnect the port runtime resume
path now has the capability to force a reset-resume to be a
warm-reset-resume (patch 13).

 I haven't gone through all this in any detail.  But a couple of things
 stand out immediately, matters of style.

 Although the USB stack still has plenty of old code with multiline
 comments looking this this:

 /* blah blah blah
  * blah blah blah
  */

 the accepted style is that all new code should appear like this:

 /*
  * blah blah blah
  * blah blah blah
  */

Ah, the eternal debate.  No worries.

 Also, the style for indentation througout most of the USB stack is that
 continuation lines are indented by two tab stops beyond the first line
 of the statement.  They are not aligned with opening parentheses of
 function calls or anything like that.

Unfortunate that what goes for net/, or drivers/md/ doesn't go for
drivers/usb/...  sounds like checkpatch needs subsystem specific style
rules to avoid this thrash.

If it's just the style I'll put those changes on a git branch and
spare the list if you're ok doing a pull... otherwise I'll hold off
until you have a chance to take a closer look.
--
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 RFC 1/1] usb: Tell xhci when usb data might be misaligned

2014-02-01 Thread Mark Lord
On 14-02-01 09:18 AM, Ming Lei wrote:

 Even real regressions are easily/often introduced, and we are discussing
 how to fix that. I suggest to unset the flag only for the known buggy
 controllers.


It is not the controllers that are particularly buggy here.
But rather the drivers and design of parts of the kernel.

Cheers
-- 
Mark Lord
Real-Time Remedies Inc.
ml...@pobox.com
--
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 v4 00/14] port power control fixes

2014-02-01 Thread Alan Stern
On Sat, 1 Feb 2014, Dan Williams wrote:

 Unfortunate that what goes for net/, or drivers/md/ doesn't go for
 drivers/usb/...  sounds like checkpatch needs subsystem specific style
 rules to avoid this thrash.

Yeah, this is all stuff that checkpatch ignores.

 If it's just the style I'll put those changes on a git branch and
 spare the list if you're ok doing a pull... otherwise I'll hold off
 until you have a chance to take a closer look.

Sure.  Give me a chance to look through it some.  At first glance, it
seems basically okay.

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 v4 00/14] port power control fixes

2014-02-01 Thread Dan Williams
On Sat, 2014-02-01 at 15:39 -0500, Alan Stern wrote:
 On Sat, 1 Feb 2014, Dan Williams wrote:
 
  Unfortunate that what goes for net/, or drivers/md/ doesn't go for
  drivers/usb/...  sounds like checkpatch needs subsystem specific style
  rules to avoid this thrash.
 
 Yeah, this is all stuff that checkpatch ignores.
 
  If it's just the style I'll put those changes on a git branch and
  spare the list if you're ok doing a pull... otherwise I'll hold off
  until you have a chance to take a closer look.
 
 Sure.  Give me a chance to look through it some.  At first glance, it
 seems basically okay.

Ok, if it helps:

  git://git.kernel.org/pub/scm/linux/kernel/git/djbw/usb usb3-port-power-v4

...is the same patchset with the following changes:

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 243cf5767433..5f94e3180b88 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2697,7 +2697,7 @@ static int hub_port_reset(struct usb_hub *hub, int port1,
goto done;
 
if (!hub_port_warm_reset_required(hub, port1,
- portstatus))
+   portstatus))
goto done;
 
/*
@@ -2772,7 +2772,7 @@ static int check_port_resume_type(struct usb_device *udev,
 {
/* Is a warm reset needed to recover the connection? */
if (udev-reset_resume
-hub_port_warm_reset_required(hub, port1, portstatus)) {
+hub_port_warm_reset_required(hub, port1, portstatus)) {
/* pass */;
}
/* Is the device still present? */
@@ -4403,7 +4403,7 @@ hub_power_remaining (struct usb_hub *hub)
  * always unlocked on exit
  */
 static void hub_port_connect_change_unlock(struct usb_hub *hub, int port1,
-  u16 portstatus, u16 portchange)
+   u16 portstatus, u16 portchange)
 {
struct usb_device *hdev = hub-hdev;
struct device *hub_dev = hub-intfdev;
@@ -4459,7 +4459,8 @@ static void hub_port_connect_change_unlock(struct usb_hub 
*hub, int port1,
}
}
 
-   /* from this point forward we no longer need synchronization
+   /*
+* From this point forward we no longer need synchronization
 * with usb_port_{suspend|resume} because we are about to
 * disconnect / reconnect
 */
@@ -4719,7 +4720,7 @@ static void port_event(struct usb_hub *hub, int port1)
if (portchange  USB_PORT_STAT_C_ENABLE) {
if (!connect_change)
dev_dbg(hub_dev, port%d: enable change, status %08x\n,
-   port1, portstatus);
+   port1, portstatus);
usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_ENABLE);
 
/*
@@ -4741,13 +4742,13 @@ static void port_event(struct usb_hub *hub, int port1)
 
dev_dbg(hub_dev, port%d: over-current change\n, port1);
usb_clear_port_feature(hdev, port1,
-  USB_PORT_FEAT_C_OVER_CURRENT);
+   USB_PORT_FEAT_C_OVER_CURRENT);
msleep(100);/* Cool down */
hub_power_on(hub, true);
hub_port_status(hub, port1, status, unused);
if (status  USB_PORT_STAT_OVERCURRENT)
dev_err(hub_dev, port%d: over-current condition\n,
-   port1);
+   port1);
}
 
if (portchange  USB_PORT_STAT_C_RESET) {
@@ -4758,17 +4759,17 @@ static void port_event(struct usb_hub *hub, int port1)
 hub_is_superspeed(hdev)) {
dev_dbg(hub_dev, port%d: warm reset change\n, port1);
usb_clear_port_feature(hdev, port1,
-  USB_PORT_FEAT_C_BH_PORT_RESET);
+   USB_PORT_FEAT_C_BH_PORT_RESET);
}
if (portchange  USB_PORT_STAT_C_LINK_STATE) {
dev_dbg(hub_dev, port%d: link state change\n, port1);
usb_clear_port_feature(hdev, port1,
-  USB_PORT_FEAT_C_PORT_LINK_STATE);
+   USB_PORT_FEAT_C_PORT_LINK_STATE);
}
if (portchange  USB_PORT_STAT_C_CONFIG_ERROR) {
dev_warn(hub_dev, port%d: config error\n, port1);
usb_clear_port_feature(hdev, port1,
-  USB_PORT_FEAT_C_PORT_CONFIG_ERROR);
+   USB_PORT_FEAT_C_PORT_CONFIG_ERROR);
}
 
/* take port actions that require the port to be powered on */
@@ -4777,7 +4778,8 @@ static void port_event(struct usb_hub *hub, int port1)
usb_lock_port(port_dev);
do if (pm_runtime_active(port_dev-dev)) {
 
-   /* 

Re: [PATCH v3 0/9] xhci: Add support for URB_ZERO_PACKET

2014-02-01 Thread renevant
Hello,

I applied these along with the reverting :


xhci: Set scatter-gather limit to avoid failed block writes.
usb: xhci: Link TRB must not occur within a USB payload burst


to Linus' tree. 


The excellent news is that my system no longer hard freezes with an 
ax88179_178a connected via a VIA VL800 host when doing certain types of 
network traffic. The bad news is the following:


[ 3337.877698] [ cut here ]
[ 3337.877721] WARNING: CPU: 5 PID: 0 at net/sched/sch_generic.c:264 
dev_watchdog+0x256/0x260()
[ 3337.877727] NETDEV WATCHDOG: enp2s0u1 (ax88179_178a): transmit queue 0 
timed out
[ 3337.877731] Modules linked in: nls_iso8859_1 vfat fat ax88179_178a usbnet 
mii usb_storage xhci_hcd fuse ipt_MASQUERADE iptable_nat nf_nat_ipv4 nf_nat 
nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack ipt_REJECT 
xt_CHECKSUM iptable_mangle xt_tcpudp ip6table_filter ip6_tables iptable_filter 
ip_tables ebtable_nat ebtables x_tables af_packet bridge stp llc vhost_net 
vhost macvtap macvlan vfio_pci vfio_iommu_type1 vfio it87 hwmon_vid 
hid_generic usbhid hid btrfs raid6_pq libcrc32c xor mxm_wmi ehci_pci ohci_pci 
ohci_hcd kvm_amd ehci_hcd snd_hda_codec_hdmi snd_hda_codec_realtek 
snd_hda_codec_generic kvm crct10dif_pclmul radeon fbcon ttm bitblit 
snd_hda_intel softcursor crc32_pclmul snd_hda_codec font tileblit 
ghash_clmulni_intel drm_kms_helper aesni_intel aes_x86_64 lrw gf128mul usbcore 
snd_hwdep
[ 3337.877807]  drm snd_pcm snd_timer snd agpgart cfbfillrect cfbimgblt 
cfbcopyarea i2c_algo_bit fb psmouse glue_helper fam15h_power k10temp 
usb_common serio_raw wmi ablk_helper pcspkr soundcore fbdev sp5100_tco 
i2c_piix4 cryptd edac_core tpm_infineon edac_mce_amd tpm_tis mac_hid microcode 
ipv6 autofs4 unix
[ 3337.877843] CPU: 5 PID: 0 Comm: swapper/5 Not tainted 3.13.0+ #4
[ 3337.877849] Hardware name: To be filled by O.E.M. To be filled by 
O.E.M./M5A99FX PRO R2.0, BIOS 2201 11/22/2013
[ 3337.877853]  0009 88043ed43de0 b04f07d6 
88043ed43e28
[ 3337.877860]  88043ed43e18 b0041883  
8804234f
[ 3337.877866]  0001 0005 88042856 
88043ed43e78
[ 3337.877873] Call Trace:
[ 3337.877877]  IRQ  [b04f07d6] dump_stack+0x45/0x56
[ 3337.877893]  [b0041883] warn_slowpath_common+0x73/0x90
[ 3337.877901]  [b00418e7] warn_slowpath_fmt+0x47/0x50
[ 3337.877909]  [b04677d6] dev_watchdog+0x256/0x260
[ 3337.877916]  [b0467580] ? dev_graft_qdisc+0x80/0x80
[ 3337.877924]  [b004b538] call_timer_fn.isra.36+0x18/0x80
[ 3337.877931]  [b004b70a] run_timer_softirq+0x16a/0x200
[ 3337.877939]  [b00457fc] __do_softirq+0xdc/0x1f0
[ 3337.877947]  [b0045b1d] irq_exit+0x9d/0xb0
[ 3337.877956]  [b002c21f] smp_apic_timer_interrupt+0x3f/0x50
[ 3337.877963]  [b04fb68a] apic_timer_interrupt+0x6a/0x70
[ 3337.877968]  EOI  [b03ff217] ? cpuidle_enter_state+0x47/0xc0
[ 3337.877979]  [b03ff326] cpuidle_idle_call+0x96/0x130
[ 3337.877987]  [b000b989] arch_cpu_idle+0x9/0x20
[ 3337.877995]  [b007edfa] cpu_startup_entry+0xda/0x1d0
[ 3337.878002]  [b002a861] start_secondary+0x1e1/0x240
[ 3337.878008] ---[ end trace 95a68de5267295c3 ]---
[ 3337.878915] xhci_hcd :02:00.0: HC gave bad length of 124978 bytes left



I honestly think it's worth pursuing this as my system is crashable even with 
sg disabled for the ax88179 without these patches. With this error i'm still 
able to just rmmod xhci_hcd and reload it and the whole system hasn't been 
brought down.




Regards,

Will Trives
--
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 v4] Move DWC2 driver out of staging

2014-02-01 Thread Stephen Warren
On 02/01/2014 03:00 AM, Andre Heider wrote:
 On Fri, Jan 31, 2014 at 11:48:37PM -0700, Stephen Warren wrote:
 On 01/31/2014 11:12 AM, Andre Heider wrote:
 On Mon, Jan 13, 2014 at 01:50:09PM -0800, Paul Zimmerman wrote:
 The DWC2 driver should now be in good enough shape to move out of
 staging. I have stress tested it overnight on RPI running mass
 storage and Ethernet transfers in parallel, and for several days
 on our proprietary PCI-based platform.
 ...
 this looks just fine, but for whatever reason it breaks sdhci on my rpi.
 With today's Linus' master the dwc2 controller seems to initialize fine,
 but I get this upon boot:

 [1.783316] sdhci-bcm2835 2030.sdhci: sdhci_pltfm_init failed -12
 [1.794820] sdhci-bcm2835: probe of 2030.sdhci failed with error -12
...
 This is due to the following code:
...
 What ends up happening, simply due to memory allocation order, is that
 the memory writes inside usb_settoggle() end up setting the SDHCI struct
 platform_device's num_resources to 0, so that it's call to
 platform_get_resource() fails.

 With the DWC2 move patch reverted, some other random piece of memory is
 being corrupted, which just happens not to cause any visible problem.
 Likely it's some other struct platform_device that's already had its
 resources read by the time DWC2 probes and corrupts them.

 (Yes, this was hard to find!)
 
 Nice work, but how did you pinpoint this? Am I missing some option/tool
 or did I just not stare for long enough?

Well, there was a clear place where an issue was present; the resource
lookup in sdhci_pltfm_init() was failing, so I put a bunch of printfs
into that function to dump out the data platform_get_resource() used.
This clearly pointed at num_resources==0 being the problem. Next, I
dumped the same data from the code in drivers/of that sets it up, and it
was OK there, so I knew it was getting over-written somewhere. I then
basically added hundreds of calls to the same data dumping function
throughout kernel functions like really_probe() to track down the
location of the problem. Luckily, the behaviour was stable, so I wasn't
chasing a race/timing condition. Eventually I narrowed the window to the
few lines of code I mentioned in _dwc2_hcd_endpoint_reset(). It would
have been much harder if it was e.g. the USB HW DMAing to memory that
caused the corruption, so I was lucky:-)
--
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