[PATCH] usb: gadget: fix NULL pointer dereference

2014-01-16 Thread Andrzej Pietrasiewicz
Fix possible NULL pointer dereference introduced in

219580e64f035bb9018dbb08d340f90b0ac50f8c
usb: f_fs: check quirk to pad epout buf size when not aligned to
maxpacketsize

after 3.13-rc1.

In cases we do wait with:

wait_event_interruptible(epfile-wait, (ep = epfile-ep));

for endpoint to be enabled, functionfs_bind() has not been called yet
and epfile-ffs-gadget is still NULL and the automatic variable 'gadget'
has been initialized with NULL at the point of its definition.
Later on it is used as a parameter to:

usb_ep_align_maybe(gadget, ep-ep, len)

which in turn dereferences it.

This patch fixes it by moving the actual assignment to the local 'gadget'
variable after the potential waiting has completed.

Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
---
@Michal: This has been detected with adb function implemented on top of
FunctionFS and the gadget itself has been set up with configfs.
adb's transport calls read (and usually blocks on it) very early and then
the problem manifests itself.

 drivers/usb/gadget/f_fs.c |7 ++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c
index fffda61..e84b73b 100644
--- a/drivers/usb/gadget/f_fs.c
+++ b/drivers/usb/gadget/f_fs.c
@@ -587,7 +587,7 @@ static ssize_t ffs_epfile_io(struct file *file,
 char __user *buf, size_t len, int read)
 {
struct ffs_epfile *epfile = file-private_data;
-   struct usb_gadget *gadget = epfile-ffs-gadget;
+   struct usb_gadget *gadget;
struct ffs_ep *ep;
char *data = NULL;
ssize_t ret, data_len;
@@ -613,6 +613,11 @@ static ssize_t ffs_epfile_io(struct file *file,
goto error;
}
}
+   /*
+* if we _do_ wait above, the epfile-ffs-gadget might be NULL
+* before the waiting completes, so do not assign to 'gadget' earlier
+*/
+   gadget = epfile-ffs-gadget;
 
/* Do we halt? */
halt = !read == !epfile-in;
-- 
1.7.0.4

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] usb: chipidea: usbmisc: Add support for i.MX27/i.MX31 CPUs

2014-01-16 Thread Uwe Kleine-König
Hello Chris,

On Wed, Jan 15, 2014 at 09:52:20AM +0800, Chris Ruehl wrote:
 I have a customized board running OTG/host and USB2/host (USB1 not connected)
 Both ports are connected to a ISP1504 ULPI
Similar for me here, my board has usb2 only with an ISP1504.
 a) I need a rs-gpio to reset in addition to the cs-gpio the ISP1504 (done)
I see there are several new chipidea patches in next-20140116 compared
to next-20140110 what I am using. Is there explicit support for a
cs-gpios property? If so, it's not obvious.

 b) Implement UPLI viewport (IORESOURCE_MEM) and logic to set the
 external power supply.
 The code (b) was rejected and needs rework.
The patch I'm currently using is appended below. It doesn't work yet,
but I think that's because CS isn't set yet. In my machine's dts I'm
using:

usbphy2 {
reset-gpios = gpio3 20 GPIO_ACTIVE_LOW;
};

usbh2 {
pinctrl-names = default;
pinctrl-0 = pinctrl_usbh2;
status = okay;
};

Does this look ok?

I'm out of time currently for this project, but I will search the
mailing list for your patches. If you Cc: on new attempts for b) I will
try to test it.

Best regards
Uwe

-- 
Pengutronix e.K.   | Uwe Kleine-König|
Industrial Linux Solutions | http://www.pengutronix.de/  |

 arch/arm/boot/dts/imx27-pingrp.h | 14 +
 arch/arm/boot/dts/imx27.dtsi | 67 
 2 files changed, 81 insertions(+)

diff --git a/arch/arm/boot/dts/imx27-pingrp.h b/arch/arm/boot/dts/imx27-pingrp.h
index 57ca02f89dff..c4d698fddbc9 100644
--- a/arch/arm/boot/dts/imx27-pingrp.h
+++ b/arch/arm/boot/dts/imx27-pingrp.h
@@ -148,4 +148,18 @@
MX27_PAD_UART3_CTS__UART3_CTS 0x0 \
MX27_PAD_UART3_RTS__UART3_RTS 0x0
 
+#define MX27_USBH2_PINGRP1 \
+   MX27_PAD_USBH2_CLK__USBH2_CLK 0x0 \
+   MX27_PAD_USBH2_DIR__USBH2_DIR 0x0 \
+   MX27_PAD_USBH2_DATA7__USBH2_DATA7 0x0 \
+   MX27_PAD_USBH2_NXT__USBH2_NXT 0x0 \
+   MX27_PAD_USBH2_STP__USBH2_STP 0x0 \
+   MX27_PAD_CSPI2_SS2__USBH2_DATA4 0x0 \
+   MX27_PAD_CSPI2_SS1__USBH2_DATA3 0x0 \
+   MX27_PAD_CSPI2_SS0__USBH2_DATA6 0x0 \
+   MX27_PAD_CSPI2_SCLK__USBH2_DATA0 0x0 \
+   MX27_PAD_CSPI2_MISO__USBH2_DATA2 0x0 \
+   MX27_PAD_CSPI2_MOSI__USBH2_DATA1 0x0 \
+   MX27_PAD_CSPI1_SS2__USBH2_DATA5 0x0
+
 #endif /* __DTS_IMX27_PINGRP_H */
diff --git a/arch/arm/boot/dts/imx27.dtsi b/arch/arm/boot/dts/imx27.dtsi
index 7e98966b1834..391769fb291e 100644
--- a/arch/arm/boot/dts/imx27.dtsi
+++ b/arch/arm/boot/dts/imx27.dtsi
@@ -33,6 +33,9 @@
spi0 = cspi1;
spi1 = cspi2;
spi2 = cspi3;
+   usb0 = usbotg;
+   usb1 = usbh1;
+   usb2 = usbh2;
};
 
aitc: aitc-interrupt-controller@e000 {
@@ -70,6 +73,33 @@
};
};
 
+   usbphy {
+   compatible = simple-bus;
+   #address-cells = 1;
+   #size-cells = 0;
+
+   usbphy0: usbphy@0 {
+   compatible = usb-nop-xceiv;
+   reg = 0;
+   clocks = clks 75;
+   clock-names = main_clk;
+   };
+
+   usbphy1: usbphy@1 {
+   compatible = usb-nop-xceiv;
+   reg = 1;
+   clocks = clks 75;
+   clock-names = main_clk;
+   };
+
+   usbphy2: usbphy@2 {
+   compatible = usb-nop-xceiv;
+   reg = 2;
+   clocks = clks 75;
+   clock-names = main_clk;
+   };
+   };
+
soc {
#address-cells = 1;
#size-cells = 1;
@@ -439,6 +469,43 @@
iram = iram;
};
 
+   usbotg: usb@10024000 {
+   compatible = fsl,imx27-usb;
+   reg = 0x10024000 0x200;
+   interrupts = 56;
+   clocks = clks 15;
+   fsl,usbmisc = usbmisc 0;
+   fsl,usbphy = usbphy0;
+   status = disabled;
+   };
+
+   usbh1: usb@10024200 {
+   compatible = fsl,imx27-usb;
+   reg = 0x10024200 0x200;
+   interrupts = 54;
+   clocks = clks 15;
+   fsl,usbmisc = usbmisc 1;
+   fsl,usbphy = usbphy1;
+   status = disabled;
+   };
+
+   usbh2: usb@10024400 {
+   compatible = fsl,imx27-usb;
+   reg = 0x10024400 0x200

RE: xhci ASMedia lockups - a theory and a patch

2014-01-16 Thread David Laight
From: David Laight
 From: David Laight
  From: Alan Stern
   On Wed, 15 Jan 2014, David Laight wrote:
  
I have a theory, I'll try to write a non-invasive patch.
 ...
  
   Doesn't this mean you shouldn't change the ownership of a LINK TRB
   until after you change the ownership of the TRB it points to?
 
  That is what I assume.
  In practise this means that the 'first_trb' (whose ownership is set
  last) has to be the one that is valid when prepare_ring() is called.
 
  The plan for the patch is:
  - Save the enq pointer in prepare_ring (in the ep_ring structure).
  - When writing a trb set the ownership unless it is the saved one
(ignoring the value set by the caller).
  - At the end invert the ownership on the saved entry.
 
 Below is a possible patch, I've only compile tested it.
 I've minimalised the patch by not removing all the code that saves
 'start_trb' and modifies the TRB_CYCLE bit.
 If the patch works those parts can also be tidied up.

I've had some feedback (in a private email) that the patch helps.
This was using the ASMedia controller with the asx88179_178a
ethernet device.

So the theory was definitely on the right lines.
And I managed to write the patch without any silly mistakes!

David



--
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: [linux-sunxi] Re: [PATCH] ARM: sunxi: Add driver for sunxi usb phy

2014-01-16 Thread Hans de Goede

Hi,

On 01/16/2014 08:07 AM, Chen-Yu Tsai wrote:

Hi Hans,

On Wed, Jan 15, 2014 at 11:48 PM, Hans de Goede hdego...@redhat.com wrote:

Hi,


On 01/15/2014 04:00 PM, Kishon Vijay Abraham I wrote:


On Wednesday 15 January 2014 04:28 AM, Hans de Goede wrote:

[...]

+static int sun4i_usb_phy_init(struct phy *_phy)
+{
+   struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
+   struct sun4i_usb_phy_data *data = to_sun4i_usb_phy_data(phy);
+   int ret;
+
+   ret = clk_prepare_enable(data-clk);
+   if (ret)
+   return ret;
+
+   ret = reset_control_deassert(phy-reset);
+   if (ret) {
+   clk_disable_unprepare(data-clk);
+   return ret;
+   }
+
+   /* Adjust PHY's magnitude and rate */
+   sun4i_usb_phy_write(phy, 0x20, 0x14, 5);



No magic values. Use macros instead.



We don't have docs, these values come from the Android code (and the comment
above has been translated from Chinese). I can make up some random
macros for this, but seems counter-productive, it seems best to just leave
this as magic until the day we actually have documentation and thus can use
defines with the proper register names, etc.


We have some names for the registers from Allwinner code:
https://github.com/linux-sunxi/linux-sunxi/blob/lichee-3.0.8-sun4i/drivers/usb/sun4i_usb/usbc/usbc_phy.c#L39


Ah good catch, thanks. I'll use those in the next revision of the phy driver.

Regards,

Hans
--
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 06/10] usb: chipidea: OTG fsm timers initialization.

2014-01-16 Thread b47...@freescale.com
-Original Message-
From: Michael Grzeschik [mailto:m...@pengutronix.de] 
Sent: Friday, January 10, 2014 9:48 PM
To: Li Jun-B47624
Cc: ba...@ti.com; Chen Peter-B29397; linux-usb@vger.kernel.org
Subject: Re: [PATCH 06/10] usb: chipidea: OTG fsm timers initialization.

Hi Li Jun,

On Wed, Jan 08, 2014 at 05:06:21PM +0800, Li Jun wrote:
 This patch adds OTG fsm timers initialization, which use controller's 
 1ms interrupt as time out counter, also adds some local timers which 
 are not in otg_fsm_timer list.
 
 Signed-off-by: Li Jun b47...@freescale.com
 ---
  drivers/usb/chipidea/otg_fsm.c |  111 
 +++-
  drivers/usb/chipidea/otg_fsm.h |   65 +++
  2 files changed, 175 insertions(+), 1 deletions(-)
 
 diff --git a/drivers/usb/chipidea/otg_fsm.c 
 b/drivers/usb/chipidea/otg_fsm.c index 31a046d..86bed68 100644
 --- a/drivers/usb/chipidea/otg_fsm.c
 +++ b/drivers/usb/chipidea/otg_fsm.c
 @@ -33,7 +33,23 @@ static struct list_head active_timers;
  /* FSM timers */
  struct ci_otg_fsm_timer *a_wait_vrise_tmr, *a_wait_vfall_tmr, 
 *a_wait_bcon_tmr,
   *a_aidl_bdis_tmr, *a_bidl_adis_tmr, *b_ase0_brst_tmr,
 - *b_se0_srp_tmr, *b_srp_fail_tmr, *b_data_pulse_tmr;
 + *b_se0_srp_tmr, *b_srp_fail_tmr, *b_data_pulse_tmr,
 + *b_ssend_srp_tmr, *b_sess_vld_tmr;
 +

One patch should not change code that another patch in that same series is 
introducing. These hunk seems to belong to the previous patch.
[Li Jun] You are right, I will re-split my patch set to resolve this, thanks!


-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |


--
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: [Stlinux-devel] FW: [PATCH (linux-stm) 1/2] usb: dwc3: fix kernel compilation in gadget mode

2014-01-16 Thread Carmelo Amoroso

Dear Felipe,

I'm one of the maintainer of the STLinux kernel @ST.
Let me clarify the inconvenience occurred about this patches.

We are back-porting some patches from upstream for DWC3 driver into our 
3.4.58 based kernel as our chipsets use this host controller.


The way we do it, is exactly what you referred to, by using 'git 
cherry-pick -xs' to keep authorship, upstream reference and so on, and 
when cherry-pick is not straightforward, we are used to specify what are 
the modification added by us wrt the upstream patch.


You might have a look at some recent backport stuff we did for our 3.4 
kernel:


http://git.stlinux.com/?p=stm/linux-stm.git;a=commit;h=cef7a3a85413142e73315463eae2fedea451490e

http://git.stlinux.com/?p=stm/linux-stm.git;a=commit;h=71538603d521a6e1b5d5b4ad47c77e6f9dfc8882

(feel free to browse our git.stlinux.com)

You will see authorship not changed, properly upstream commit reference 
and so on.


Regarding the merged patches sent by the colleague Giuseppe, I asked him 
to provide me an unique patch just to check locally if the issue were 
actually fixed. It should have been a private, temporary hack for testing.


It was not meant to be applied in the kernel, neither he had the 
intention to change authorship.


Due to bad git configuration (and user mistakes), when he sent the patch 
it went to our devel-list plus you as in the signed-off list, but purely 
by mistake (--suppress-xxx neither passed)


As kernel maintainer, I can guarantee such a work will not included at 
all in our kernel. It has been just due to user mistakes.


We seriously consider copyright laws.

Our apologies for such inconvenience.

With best regards,
Carmelo


-Original Message-
From: Felipe Balbi [mailto:ba...@ti.com]
Sent: mercoledì 15 gennaio 2014 19:38
To: Giuseppe CONDORELLI
Cc: stlinux-de...@lists.codex.cro.st.com; Felipe Balbi; Linux USB Mailing List
Subject: Re: [PATCH (linux-stm) 1/2] usb: dwc3: fix kernel compilation in
gadget mode

On Wed, Jan 15, 2014 at 04:36:25PM +0100, Giuseppe Condorelli wrote:

The following set of patches, from the mainline, were applied to the
tree (after reworking) to solve build time issues about implicit
declaration of irq function and on arguments number mismatch in given
functions
(example: __dwc3_gadget_ep_enable).

[giuseppe.condore...@st.com: reworked patch to fit current gadget
code)
Signed-off-by: Felipe Balbi ba...@ti.com
Signed-off-by: Giuseppe Condorelli giuseppe.condore...@st.com


wait, what ?





I don't remember seeing this patch ever been sent upstream. This patch
seems to be a merge of several other patches, none of which you Authored,
so how come you:

a) put yourself as author of the code; and
b) add my Signed-off-by without asking me permission for it ?

In fact, you have *never* authored any patch on the dwc3 driver (I just
checked). If you want to get your tree in sync with the code which is mainline,
you should be using git cherry-pick -x to make sure that you maintain
authorship and make it clear which commit you cherry-picked so things can be
tracked down.

Copyright law is a serious business, I wonder what other patches have been
taken from mainline, changed author and applied to your vendor kernel with
my SoB.

I will be very clear:

. Do *NOT* apply *any* patch you take from mainline with
yourself as author.

. Make sure to use git cherry-pick -xs and solve merge conflicts
where applicable.

. Do *NOT* use somebody else's {Signed-off,Acked,Tested,etc}-by
tags without their permission.

Patch below so other people see what ST has been doing.


---
  drivers/usb/dwc3/core.h   |1 +
  drivers/usb/dwc3/gadget.c |  101
++---
  2 files changed, 60 insertions(+), 42 deletions(-)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index
e91ddf8..610875b 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -705,6 +705,7 @@ struct dwc3 {
unsigneddelayed_status:1;
unsignedneeds_fifo_resize:1;
unsignedresize_fifos:1;
+   unsignedpullups_connected:1;

enum dwc3_ep0_next  ep0_next_event;
enum dwc3_ep0_state ep0state;
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index c0cc8f6..2c5f24b 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -433,7 +433,8 @@ static int dwc3_gadget_start_config(struct dwc3
*dwc, struct dwc3_ep *dep)

  static int dwc3_gadget_set_ep_config(struct dwc3 *dwc, struct dwc3_ep

*dep,

const struct usb_endpoint_descriptor *desc,
-   const struct usb_ss_ep_comp_descriptor *comp_desc)
+   const struct usb_ss_ep_comp_descriptor *comp_desc,
+   bool ignore)
  {
struct dwc3_gadget_ep_cmd_params params;

@@ -443,6 +444,9 @@ static int dwc3_gadget_set_ep_config(struct 

Re: [PATCH v3 00/10] Just the essential port power control fixes for 3.14

2014-01-16 Thread Alan Stern
On Wed, 15 Jan 2014, Dan Williams wrote:

 When the usb_device stays bound we do attempt recovery, fail, and
 properly trigger a disconnect of the device as expected.
 
 Question for Alan:  I'm thinking we need a hub_port_logical_disconnect()
 when the usb_device becomes unbound.  Otherwise the port state will
 remain stale until the next khubd event on that port.  I'll give it some
 more thought, but curious what is the expected recovery from that state.

No, we don't call hub_port_logical_disconnect merely because of drivers
getting bound or unbound.  We call it only when there has been an
external change, either in the device itself or its descriptors -- 
something that should cause the kernel to act as though the old device 
has been unplugged (and possibly a new device plugged in).

The kernel's assumption is that a usb_device will always have a driver.  
Originally I thought there would be alternative drivers to the standard 
one (for example, a driver that would export access to the device over 
the network), but things haven't worked out that way.

If somebody unbinds the usb_device's driver, they should be aware that
things won't work right.  The expected recovery is that the user
rebinds the driver.  (But would the rebind work if the device is 
suspended?  I don't remember ever testing it...)

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] net: sk == 0xffffffff fix - not for commit

2014-01-16 Thread Eric Dumazet
On Thu, 2014-01-16 at 16:21 +0100, Andrzej Pietrasiewicz wrote:
 W dniu 10.12.2013 15:25, Eric Dumazet pisze:
  On Tue, 2013-12-10 at 07:55 +0100, Andrzej Pietrasiewicz wrote:
  W dniu 09.12.2013 16:31, Eric Dumazet pisze:
  On Mon, 2013-12-09 at 12:47 +0100, Andrzej Pietrasiewicz wrote:
  NOT FOR COMMITTING TO MAINLINE.
 
  With g_ether loaded the sk occasionally becomes 0x.
  It happens usually after transferring few hundreds of kilobytes to few
  tens of megabytes. If sk is 0x then dereferencing it causes
  kernel panic.
 
  This is a *workaround*. I don't know enough net code to understand the 
  core
  of the problem. However, with this patch applied the problems are gone,
  or at least pushed farther away.
 
  Is it happening on SMP or UP ?
 
  UP build, S5PC110
 
  OK
 
  I believe you need additional debugging to track the exact moment
  0x is fed to 'sk'
 
  It looks like a very strange bug, involving a problem in some assembly
  helper, register save/restore, compiler bug or stack corruption or
  something.
 
 
 I started with adding WARN_ON(sk == 0x); just before return in
 __inet_lookup_established(), and the problem was gone. So this looks
 very strange, like a toolchain problem.

Or a timing issue. Adding a WARN_ON() adds extra instructions and might
really change the assembly output.

 
 I used gcc-linaro-arm-linux-gnueabihf-4.8-2013.05.
 
 If I change the toolchain to
 
 gcc-linaro-arm-linux-gnueabihf-4.7-2013.04-20130415
 
 the problem seems to have gone away.

Its totally possible some barrier was not properly handled by the
compiler. You could disassemble the function on both toolchains and
try to spot the issue.



--
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: xhci-hcd (millions of) Too many fragments error on LUKS-encrypted device

2014-01-16 Thread Johannes Stezenbach
Hi Sascha,

On Wed, Jan 15, 2014 at 08:27:58PM +0800, Sascha Weaver wrote:
 2014/1/15 David Laight david.lai...@aculab.com:
  Edit drivers/usb/host/xhci.h and change the definition of
  TRBS_PER_SEGMENT from 64 to 256.
 
 
 Thanks David! I re-compiled my kernel after changing 64 to 256 and it works!
 
 No more errors are produced!

I cannot reproduce the issue, just finished my weekly backup
to LUKS USB3 disk with unpatched linux-3.12.7.  It seems the
patches are awaiting feedback before the can go to stable,
since you seem to be able to reproduce it easily it would be
cool if you could test.  Maybe test with the first patch
only to see if it fixes the endless error loop, then confirm
the second patch fixes the error.

http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=733907#25
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=733826#69

Thanks,
Johannes
--
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 v3 03/10] usb: find internal hub tier mismatch via acpi

2014-01-16 Thread Sarah Sharp
On Wed, Jan 15, 2014 at 10:39:56AM -0500, Alan Stern wrote:
 On Tue, 14 Jan 2014, Sarah Sharp wrote:
 
   It's always possible for xhci-hcd to prevent the USB-2 root hub from 
   being suspended by calling pm_runtime_get_noresume.  The corresponding 
   _put can be done after the USB-3 root hub has been registered.
  
  Sure, it's on my todo list to fix that.  I just wondered if there were
  other race conditions present, given that I could find one off the top
  of my head.
  
  What do you think about the rest of the patchset?
 
 I regret that I haven't taken the time to look through it yet.  :-(  
 I'll do my best to go through it soon.

No worries!  I'm most interested in your comments about the changes to
khubd, so if you could find time to look at just patches 7 and 9, that
would be appreciated.

Sarah Sharp
--
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 v3 06/10] usb: gate clearing FEAT_C_ENABLE to usb2 hubs

2014-01-16 Thread Sarah Sharp
On Tue, Jan 07, 2014 at 12:30:00PM -0800, Dan Williams wrote:
 The port pm_runtime implementation unconditionally clears FEAT_C_ENABLE
 after clearing PORT_POWER, but the bit is reserved on usb3 hub ports.
 
 It also takes steps to re-disable the port if it fails to reconnect
 which again is broken on usb3 ports and unnecessary on usb2 ports.

The subject is misleading, since you don't just change when you clear
FEAT_C_ENABLE for USB 2.0 hubs; you also change it for USB 3.0 hubs.
Suggest:

usb: Clear FEAT_C_ENABLE on port suspend for usb2, not usb3

Actually, that's still not clear, because the patch really fixes two
distinct bugs:

1. Don't clear FEAT_C_ENABLE for USB 3.0 hubs at all.
2. Don't clear FEAT_C_ENABLE for USB 2.0 ports on failed resume.

That really suggests it should be two separate patches, in case there's
say, a bug in patch #2 and we really do want to clear FEAT_C_ENABLE on
USB 2.0 port resume.

Can you split this patch up?

Sarah Sharp

 Signed-off-by: Dan Williams dan.j.willi...@intel.com
 ---
  drivers/usb/core/port.c |4 ++--
  1 files changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
 index 217a3c6df29e..97e4939fee1a 100644
 --- a/drivers/usb/core/port.c
 +++ b/drivers/usb/core/port.c
 @@ -102,7 +102,6 @@ static int usb_port_runtime_resume(struct device *dev)
   if (retval  0)
   dev_dbg(port_dev-dev, can't get reconnection after 
 setting port  power on, status %d\n,
   retval);
 - usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_ENABLE);
   retval = 0;
   }
  
 @@ -142,7 +141,8 @@ static int usb_port_runtime_suspend(struct device *dev)
   set_bit(port1, hub-busy_bits);
   retval = usb_hub_set_port_power(hdev, hub, port1, false);
   usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_CONNECTION);
 - usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_ENABLE);
 + if (!hub_is_superspeed(hdev))
 + usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_ENABLE);
   clear_bit(port1, hub-busy_bits);
   usb_autopm_put_interface(intf);
  
 
--
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 06/10] usb: chipidea: OTG fsm timers initialization.

2014-01-16 Thread David Cohen
 Hi Li Jun,

Hi Li Jun,

 
 On Wed, Jan 08, 2014 at 05:06:21PM +0800, Li Jun wrote:
  This patch adds OTG fsm timers initialization, which use controller's 
  1ms interrupt as time out counter, also adds some local timers which 
  are not in otg_fsm_timer list.
  
  Signed-off-by: Li Jun b47...@freescale.com
  ---
   drivers/usb/chipidea/otg_fsm.c |  111 
  +++-
   drivers/usb/chipidea/otg_fsm.h |   65 +++
   2 files changed, 175 insertions(+), 1 deletions(-)
  
  diff --git a/drivers/usb/chipidea/otg_fsm.c 
  b/drivers/usb/chipidea/otg_fsm.c index 31a046d..86bed68 100644
  --- a/drivers/usb/chipidea/otg_fsm.c
  +++ b/drivers/usb/chipidea/otg_fsm.c
  @@ -33,7 +33,23 @@ static struct list_head active_timers;
   /* FSM timers */
   struct ci_otg_fsm_timer *a_wait_vrise_tmr, *a_wait_vfall_tmr, 
  *a_wait_bcon_tmr,
  *a_aidl_bdis_tmr, *a_bidl_adis_tmr, *b_ase0_brst_tmr,
  -   *b_se0_srp_tmr, *b_srp_fail_tmr, *b_data_pulse_tmr;
  +   *b_se0_srp_tmr, *b_srp_fail_tmr, *b_data_pulse_tmr,
  +   *b_ssend_srp_tmr, *b_sess_vld_tmr;
  +
 
 One patch should not change code that another patch in that same series is 
 introducing. These hunk seems to belong to the previous patch.
 [Li Jun] You are right, I will re-split my patch set to resolve this, thanks!

It's difficult to read the e-mails you reply. Please, consider to quote
the text messages you're replying to.

Br, David Cohen

--
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: EHCI host broken -- interrupts disabled

2014-01-16 Thread Bjorn Helgaas
[+cc linux-pci]

On Tue, Jan 14, 2014 at 11:47 AM, Alan Stern st...@rowland.harvard.edu wrote:
 On Tue, 14 Jan 2014, Sarah Sharp wrote:

 Hi Alan,

 I'm not a good person to ask about this.  Maybe Bjorn can help.

 All this info is second- or third-hand, so please excuse the extremely
 vague bug report.

 A new Intel Atom system (Baytrail) comes with both an xHCI host and an
 EHCI host, but the BIOS has an EHCI only option that hides the xHCI
 host PCI device from the operating system.  It appears that when the
 BIOS is in this mode, the EHCI PCI interrupt is disabled, and the host
 simply doesn't work.

 Jamie and Gaggery have more details about the system and the exact
 failure mode.  They can provide dmesg as well as a PCI register space
 dump, if necessary.  (Jamie and Gaggery, please note, a publicly
 archived mailing list is Cc'ed.)

 A customer reports that the following patch fixes the issue:

 diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
 index 3e86bf4371b3..e77566a021ec 100644
 --- a/drivers/usb/host/ehci-pci.c
 +++ b/drivers/usb/host/ehci-pci.c
 @@ -112,6 +112,9 @@ static int ehci_pci_setup(struct usb_hcd *hcd)
   case PCI_VENDOR_ID_INTEL:
   if (pdev-device == PCI_DEVICE_ID_INTEL_CE4100_USB)
   hcd-has_tt = 1;
 + /* Baytrail BIOS in EHCI only mode fails to enable interrupts 
 */
 + if (pdev-device == 0x0f34)
 + pci_intx(pdev, 1);
   break;
   case PCI_VENDOR_ID_TDI:
   if (pdev-device == PCI_DEVICE_ID_TDI_EHCI)

 I looked through the USB PCI initialization code, and I can't find any
 place where interrupts are enabled, so I assume the PCI core is supposed
 to enable interrupts?  Or is it assumed that the system boots with all
 PCI device interrupts enabled?  In that case, is it a BIOS issue?

 As I recall, the original PCI spec did not include any way to enable or
 disable interrupts -- a rather surprising omission.  It was added
 later on.

 Anyhow, this is all handled in the PCI core.  The device driver
 shouldn't need to do much more than pci_enable_device() (which handles
 power plus I/O and memory resources) and pci_set_master() (which
 handles DMA).  Nothing about enabling interrupt generation.

 A quick search did not show any place where the PCI core explicitly
 enables interrupts on new devices, so maybe it does assume that the
 firmware always enables them.  I don't know what guarantees (if any) a
 BIOS is supposed to fulfill.

 It seems like the best place to fix this is probably in the PCI core.

I think dev-irq is supposed to be valid after pci_enable_device(), so
it seems like it would make sense to clear PCI_COMMAND_INTX_DISABLE
there.

I don't know why a BIOS would leave PCI_COMMAND_INTX_DISABLE set for
the EHCI device.  Does it support MSI, and the BIOS assumes the OS
should use that?  I don't see any MSI support in the EHCI code.  In
any case, this doesn't seem like something we need to depend on the
BIOS to do for us.

Bjorn
--
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: [BUG] FL1009: xHCI host not responding to stop endpoint command.

2014-01-16 Thread Sarah Sharp
On Wed, Jan 15, 2014 at 08:04:45PM +0100, Arnaud Ebalard wrote:
 Hi David,
 
 David Laight david.lai...@aculab.com writes:
 
  I tried current 3.13.0-rc8 w/ 35773dac5f86 reverted and the result is
  the same:
 
  That patch only affects an error code and stops the fs code retrying
  for ever.
 
 Are you sure? ...
 
  Does everything work if you comment out the code in xhci-ring.c that adds
  NOP TRBs to the ring end in order to stop the LINK TRB appearing in the 
  middle
  of a TB.
  The ethernet code needs it, but the disk transfers are (probably) aligned
  such that they don't.
 
 ... AFAICT, this is exactly what commit 35773dac5f86 does and reverting
 it does not help. If I am mistaken, can you point which part you want me
 to remove in the code to test?
 
 I am slowly starting to see a bisect session coming ;-)

Try reverting commit 60e102ac73cd40069d077014c93c86dc7205cb68.  That was
causing issues with another Fresco Logic host.  If that doesn't help,
then yes, please git bisect.

Sarah Sharp
--
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 v3 06/10] usb: gate clearing FEAT_C_ENABLE to usb2 hubs

2014-01-16 Thread Dan Williams
On Thu, Jan 16, 2014 at 8:35 AM, Sarah Sharp
sarah.a.sh...@linux.intel.com wrote:
 On Tue, Jan 07, 2014 at 12:30:00PM -0800, Dan Williams wrote:
 The port pm_runtime implementation unconditionally clears FEAT_C_ENABLE
 after clearing PORT_POWER, but the bit is reserved on usb3 hub ports.

 It also takes steps to re-disable the port if it fails to reconnect
 which again is broken on usb3 ports and unnecessary on usb2 ports.

 The subject is misleading, since you don't just change when you clear
 FEAT_C_ENABLE for USB 2.0 hubs; you also change it for USB 3.0 hubs.
 Suggest:

 usb: Clear FEAT_C_ENABLE on port suspend for usb2, not usb3

 Actually, that's still not clear, because the patch really fixes two
 distinct bugs:

 1. Don't clear FEAT_C_ENABLE for USB 3.0 hubs at all.
 2. Don't clear FEAT_C_ENABLE for USB 2.0 ports on failed resume.

 That really suggests it should be two separate patches, in case there's
 say, a bug in patch #2 and we really do want to clear FEAT_C_ENABLE on
 USB 2.0 port resume.

 Can you split this patch up?


Of course, will do.

Given how fragile port recovery is in the current kernel I do not see
how 2/ could cause a regression especially since it will be cleared as
a matter of course by khubd.  In fact, we want khubd to service this
and not hide it with an opportunistic clearing in
usb_port_runtime_resume().  I'll say this in the split patch change
log.

--
Dan
--
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: XHCI issues: WD MyBook 1230 - reset SuperSpeed USB device

2014-01-16 Thread Peter Palúch

Alan,

I am attaching the usbmon trace after the drive has been plugged into 
the USB-2 port. Record of the drive in stall should occur around the 
file offset 87808 (decimal). The log was done on the 3.12.7 kernel 
without CONFIG_PM. Should I do a usbmon trace on my regular kernel with 
CONFIG_PM as well?


dmesg transcript:

root@bach:/tmp# dmesg
usb 4-1.2: new high-speed USB device number 5 using ehci-pci
usb 4-1.2: New USB device found, idVendor=1058, idProduct=1230
usb 4-1.2: New USB device strings: Mfr=2, Product=3, SerialNumber=1
usb 4-1.2: Product: My Book 1230
usb 4-1.2: Manufacturer: Western Digital
usb 4-1.2: SerialNumber: 574D43344E30323438393836
usb-storage 4-1.2:1.0: USB Mass Storage device detected
scsi6 : usb-storage 4-1.2:1.0
usbcore: registered new interface driver usb-storage
scsi 6:0:0:0: Direct-Access WD   My Book 1230 1050 PQ: 0 ANSI: 6
scsi 6:0:0:1: Enclosure WD   SES Device   1050 PQ: 0 ANSI: 6
sd 6:0:0:0: Attached scsi generic sg2 type 0
scsi 6:0:0:1: Attached scsi generic sg3 type 13
sd 6:0:0:0: [sdb] Spinning up disk...
.ready
sd 6:0:0:0: [sdb] 732558336 4096-byte logical blocks: (3.00 TB/2.72 TiB)
sd 6:0:0:0: [sdb] Write Protect is off
sd 6:0:0:0: [sdb] Mode Sense: 53 00 10 08
sd 6:0:0:0: [sdb] No Caching mode page found
sd 6:0:0:0: [sdb] Assuming drive cache: write through
sd 6:0:0:0: [sdb] 732558336 4096-byte logical blocks: (3.00 TB/2.72 TiB)
sd 6:0:0:0: [sdb] No Caching mode page found
sd 6:0:0:0: [sdb] Assuming drive cache: write through
 sdb: sdb1
sd 6:0:0:0: [sdb] 732558336 4096-byte logical blocks: (3.00 TB/2.72 TiB)
sd 6:0:0:0: [sdb] No Caching mode page found
sd 6:0:0:0: [sdb] Assuming drive cache: write through
sd 6:0:0:0: [sdb] Attached SCSI disk
usb 4-1.2: reset high-speed USB device number 5 using ehci-pci

Thank you!

Best regards,
Peter

On 15.01.2014 18:30, Alan Stern wrote:

On Wed, 15 Jan 2014, Peter Palúch wrote:


The interesting thing is that the same happens even if the drive is
plugged into an USB 2.0 port in the same notebook:

That is an important point.

Can you post a usbmon trace similar to the earlier one, but with the 
drive plugged into the USB-2 port?




usbmon-2.0.txt.bz2
Description: application/bzip


Re: [PATCH] xhci: make warnings greppable

2014-01-16 Thread Sarah Sharp
On Fri, Jan 10, 2014 at 01:36:17AM +0300, Sergei Shtylyov wrote:
 Hello.
 
 On 01/08/2014 07:13 PM, oli...@neukum.org wrote:
 
 From: Oliver Neukum oneu...@suse.de
 
 This changes debug messages and warnings in xhci-ring.c
 to be on a single line so grep can find them. grep must
 have precedence over the 80 column limit.
 
In the eyes of checkpatch.pl, it has been having precedence for
 quite a while already, even so that it found some of your line
 breaks premature. :-)

The patch is now applied to my for-usb-next-3.14 branch, and will be
sent out for inclusion in 3.15 after 3.14-rc1 is out.  I've fixed the
checkpatch.pl errors that Sergei commented on.

https://git.kernel.org/cgit/linux/kernel/git/sarah/xhci.git/commit/?h=for-usb-next-3.14id=5f352c30efef644f17603150ed1e38d90bfdba6d

Sarah Sharp

 Signed-off-by: Oliver Neukum oneu...@suse.de
 
 ---
   drivers/usb/host/xhci-ring.c | 21 -
   1 file changed, 8 insertions(+), 13 deletions(-)
 
 diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
 index 09b2b55..34dce53 100644
 --- a/drivers/usb/host/xhci-ring.c
 +++ b/drivers/usb/host/xhci-ring.c
 [...]
 @@ -1113,13 +1110,12 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd 
 *xhci, int slot_id,
  slot_state, ep_state);
  break;
  case COMP_EBADSLT:
 -xhci_warn(xhci, WARN Set TR Deq Ptr cmd failed because 
 
 -slot %u was not enabled.\n, slot_id);
 +xhci_warn(xhci, WARN Set TR Deq Ptr cmd failed because 
 slot 
 +%u was not enabled.\n, slot_id);
  break;
  default:
 -xhci_warn(xhci, WARN Set TR Deq Ptr cmd with unknown 
 -completion code of %u.\n,
 -  cmd_comp_code);
 +xhci_warn(xhci, WARN Set TR Deq Ptr cmd with unknown 
 completion code of 
 +%u.\n, cmd_comp_code);
  break;
  }
  /* OK what do we do now?  The endpoint state is hosed, and we
 
 $ scripts/checkpatch.pl ~/patches/xhci-make-warnings-greppable.patch
 WARNING: quoted string split across lines
 #48: FILE: drivers/usb/host/xhci-ring.c:1114:
 + xhci_warn(xhci, WARN Set TR Deq Ptr cmd failed because 
 slot 
 + %u was not enabled.\n, slot_id);
 
 WARNING: quoted string split across lines
 #55: FILE: drivers/usb/host/xhci-ring.c:1118:
 + xhci_warn(xhci, WARN Set TR Deq Ptr cmd with unknown 
 completion code of 
 + %u.\n, cmd_comp_code);
 
 total: 0 errors, 2 warnings, 49 lines checked
 
 /home/headless/patches/xhci-make-warnings-greppable.patch has style
 problems, please review.
 
 If any of these errors are false positives, please report
 them to the maintainer, see CHECKPATCH in MAINTAINERS.
 
 PS: Sarah, sorry for speaking up on xHCI patches again but I was
 sure the patch won't pass checkpatch.pl, so decided to report
 this...
 
 WBR, 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 3.12 033/118] usb: xhci: Link TRB must not occur within a USB payload burst [NEW HARDWARE]

2014-01-16 Thread Sarah Sharp
On Tue, Jan 14, 2014 at 01:27:25PM -0800, walt wrote:
 On 01/14/2014 09:20 AM, Sarah Sharp wrote:
  On Mon, Jan 13, 2014 at 03:39:07PM -0800, walt wrote:
 
  Sarah, I just fixed my xhci bug for US$19.99 :)
 
  #lspci | tail -1
  04:00.0 USB controller: NEC Corporation uPD720200 USB 3.0 Host Controller 
  (rev 03)
 
  This new NEC usb3 controller does everything the ASMedia controller should 
  have
  done from the start.
 
  
  I just got a similar report from someone with a Fresco Logic host
  controller, so I may need you to test a work-around patch for the
  ASMedia host at some point.

Hmm, the Fresco Logic host issue seems unrelated.

 Oy, Sarah! ;)  I put the ASMedia adapter in my older amd64 machine, and, well,
 the stupid thing Just Works(TM) with kernel 3.12.7!  (Yes, with the same disk
 docking station, too.)

Ugh.  Well, I suppose we can chalk it up to hardware failure?  I think
you're the only one to report a verified issue with the Link TRB patch.

You are sure you're running a vanilla 3.12.7 kernel, right?

 I can't believe the adapter works perfectly in a different computer.  Have you
 seen this kind of thing before?

No, at least not this particular host-dying-only-on-one-machine failure
mode.

 At the moment I have two machines using your xhci driver and both work 
 perfectly,
 so I thank you again :)
 
 I'm not sure where to go with this next.  I could put the adapter back in the
 other machine again if you have more patches to test.

I think any patches I was going to send are moot with this new
information.  The issue with your PCI add-in card only happens in
combination with a specific motherboard, so I don't think it makes sense
to disable the no-op TRBs for that host.

If lots of other people start reporting the same issue with the ASMedia
0.96 host, and reverting commit 35773dac5f862cb1c82ea151eba3e2f6de51ec3e
usb: xhci: Link TRB must not occur within a USB payload burst helps
them, then I'll reconsider that decision.

Thank you so much for your patience while debugging this issue, and
being willing to try all sorts of kernels and patches.  I'm glad we
finally figured out what the issue was, and you have working xHCI hosts
now.

Sarah Sharp
--
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: XHCI issues: WD MyBook 1230 - reset SuperSpeed USB device

2014-01-16 Thread Alan Stern
It's now clear that this is _not_ an XHCI issue, contrary to what 
$SUBJECT says.

On Thu, 16 Jan 2014, Peter Palúch wrote:

 Alan,
 
 I am attaching the usbmon trace after the drive has been plugged into 
 the USB-2 port. Record of the drive in stall should occur around the 
 file offset 87808 (decimal). The log was done on the 3.12.7 kernel 
 without CONFIG_PM. Should I do a usbmon trace on my regular kernel with 
 CONFIG_PM as well?

No need.

 dmesg transcript:
 
 root@bach:/tmp# dmesg
 usb 4-1.2: new high-speed USB device number 5 using ehci-pci
 usb 4-1.2: New USB device found, idVendor=1058, idProduct=1230
 usb 4-1.2: New USB device strings: Mfr=2, Product=3, SerialNumber=1
 usb 4-1.2: Product: My Book 1230
 usb 4-1.2: Manufacturer: Western Digital
 usb 4-1.2: SerialNumber: 574D43344E30323438393836
 usb-storage 4-1.2:1.0: USB Mass Storage device detected
 scsi6 : usb-storage 4-1.2:1.0
 usbcore: registered new interface driver usb-storage
 scsi 6:0:0:0: Direct-Access WD   My Book 1230 1050 PQ: 0 ANSI: 6
 scsi 6:0:0:1: Enclosure WD   SES Device   1050 PQ: 0 ANSI: 6
 sd 6:0:0:0: Attached scsi generic sg2 type 0
 scsi 6:0:0:1: Attached scsi generic sg3 type 13
 sd 6:0:0:0: [sdb] Spinning up disk...
 .ready
 sd 6:0:0:0: [sdb] 732558336 4096-byte logical blocks: (3.00 TB/2.72 TiB)
 sd 6:0:0:0: [sdb] Write Protect is off
 sd 6:0:0:0: [sdb] Mode Sense: 53 00 10 08
 sd 6:0:0:0: [sdb] No Caching mode page found
 sd 6:0:0:0: [sdb] Assuming drive cache: write through
 sd 6:0:0:0: [sdb] 732558336 4096-byte logical blocks: (3.00 TB/2.72 TiB)
 sd 6:0:0:0: [sdb] No Caching mode page found
 sd 6:0:0:0: [sdb] Assuming drive cache: write through
   sdb: sdb1
 sd 6:0:0:0: [sdb] 732558336 4096-byte logical blocks: (3.00 TB/2.72 TiB)
 sd 6:0:0:0: [sdb] No Caching mode page found
 sd 6:0:0:0: [sdb] Assuming drive cache: write through
 sd 6:0:0:0: [sdb] Attached SCSI disk
 usb 4-1.2: reset high-speed USB device number 5 using ehci-pci

It looks like the reset occurred because the computer sent an
ATA-passthru command to the disk, and the disk wasn't prepared to
handle it properly.  The firmware crashed, requiring a reset.

If anyone can explain, the command bytes in question were:

85082e00   ec00

and the sense data was:

7201001d 000e 090c 5d00 0100 0050

I don't know what either of these means, or even what software was
responsible for sending this command.  It appears to have come from
some user program, though, not the kernel.  Possibly something run by 
udev.

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 v3 03/10] usb: find internal hub tier mismatch via acpi

2014-01-16 Thread Alan Stern
On Thu, 16 Jan 2014, Sarah Sharp wrote:

   What do you think about the rest of the patchset?
  
  I regret that I haven't taken the time to look through it yet.  :-(  
  I'll do my best to go through it soon.
 
 No worries!  I'm most interested in your comments about the changes to
 khubd, so if you could find time to look at just patches 7 and 9, that
 would be appreciated.

Okay, I have looked at them (and patch 8 too).  In short, I disapprove 
of the design.  It's an ad-hoc approach that ignores the larger issues.

The real problem is that we need to guarantee mutual exclusion for 
access to a particular port on the hub.  The things we can do to a port 
include:

khubd's normal processing:
turning off various port status-change flags,
handling remote wakeups,
taking care of overcurrent events,
issuing USB-3 warm resets,
and handling connect/disconnect events;

Issuing a port reset, which can happen:
when a USB-3 port goes into SS.Inactive,
when a driver resets a device,
or when a reset-resume is needed;

Resuming a port (perhaps suspending it too);

Turning port power on or off.

These four somewhat overlapping actions need to be mutually exclusive.  
(Actually, the first of khubd's activities -- turning off status-change 
flags -- probably doesn't need to be exclusive with anything else.  But 
we might as well include it with the rest.)

The natural solution is use a per-port mutex, instead of messing around 
with atomic busy_bits stuff or PM barriers.

It turns out that we will require a particular locking order.  It will
sometimes be necessary to acquire the port's mutex while holding the
child device's lock.  This can happen when we are binding or unbinding
a driver to the child device; usb_probe_interface() is called with the
child locked, and it calls usb_autoresume_device() which will need to
lock the upstream port if the device is in runtime suspend.

As far as I can tell, we don't actually need to acquire the locks in 
the opposite order, although to make that work means we will have to 
drop the port lock in various parts of hub_port_connect_change() where 
the child device gets locked.

One possibility is to use the port's own embedded device lock as the
port mutex.  But this would preclude ever moving the child USB device
directly under the port device in the device tree, because the locking
order would be wrong.  It seems safer to embed a new mutex in struct
usb_port.

Dan, what do you think of this approach?

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] net: sk == 0xffffffff fix - not for commit

2014-01-16 Thread Andrew Ruder
On Mon, Dec 09, 2013 at 12:47:52PM +0100, Andrzej Pietrasiewicz wrote:
 With g_ether loaded the sk occasionally becomes 0x.
 It happens usually after transferring few hundreds of kilobytes to few
 tens of megabytes. If sk is 0x then dereferencing it causes
 kernel panic.

Don't know if this is relevant but I had this very similar stack trace
come up a few days ago (below).  I am working on a PXA 270/xscale with
gcc version 4.8.2 (Buildroot 2013.11-rc1-00028-gf388663).  Going to try
to see if I can reproduce it a little more readily before I start trying
to narrow down what is causing it.

===
Unable to handle kernel NULL pointer dereference at virtual address 0011
pgd = d18e
[0011] *pgd=a6d03831, *pte=, *ppte=
Internal error: Oops: 17 [#1] PREEMPT ARM
Modules linked in: zeusvirt(O) zeus16550(O) 8390p ipv6
CPU: 0 PID: 2365 Comm: sshd Tainted: G   O 3.12.0+ #201
task: d7216f00 ti: d7144000 task.ti: d7144000
PC is at tcp_v4_early_demux+0xe8/0x154
LR is at __inet_lookup_established+0x1bc/0x2e0
pc : [c0341cfc]lr : [c0329bd8]psr: a013
sp : d7145b20  ip : d7145ae8  fp : d7145b44
r10: c0576c28  r9 : 0008  r8 : d7998800
r7 : d7063800  r6 : c6cf2480  r5 :   r4 : c6cf2480
r3 : c02ec018  r2 : d7145ad0  r1 : d7b66a28  r0 : 
Flags: NzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
Control: 397f  Table: b18e  DAC: 0015
Process sshd (pid: 2365, stack limit = 0xd71441c8)
Stack: (0xd7145b20 to 0xd7146000)
5b20: 17bf3f0a 0016 0003 c0026d90 d71f4634 d71f4600 d7145b6c d7145b48
5b40: c03211b4 c0341c20 05ea d7bb0538 d7063800 0034 d71f4600 c6cf2480
5b60: d7145b9c d7145b70 c03218dc c0321158 1001  c0576c1c 
5b80: c0577e84 c0576c14   d7145be4 d7145ba0 c02fae04 c03215d4
5ba0: c0590330 c057fc08 d7145bfc c6cf2480 c02571a0 c0576c28 07e1 c05a3dc0
5bc0:  0001 c05a3d60 c05a3d74 c05a3d60 c05a3d68 d7145bfc d7145be8
5be0: c02fb990 c02fa8f0 c05a3dc0  d7145c24 d7145c00 c02fc46c c02fb968
5c00: c02fc3dc c05a3dc0 c05a3d60 0001 012c 0040 d7145c64 d7145c28
5c20: c02fbcd0 c02fc3e8  d78af3c0 d7145c5c 8d99  0001
5c40: c05a81f0 0003 0100 3fa57e1c d7144028 c05a81ec d7145cb4 d7145c68
5c60: c0026a44 c02fbc10 d7145c8c d7145c78 c00538dc c0056ce4  8d98
5c80: 00400100 000a c0228594 6093 c0590330  d7145d54 0001
5ca0: d7bb0480 05b4 d7145ccc d7145cb8 c0026ca4 c00268f4  d7144010
5cc0: d7145ce4 d7145cd0 c0026f58 c0026c58 00ab 001a d7145d04 d7145ce8
5ce0: c000f7d0 c0026ed0 0014 d7145d20 a013  d7145d1c d7145d08
5d00: c00085bc c000f768 c02f0048 c00ca7d8 d7145d7c d7145d20 c03a7dc0 c0008590
5d20: 000118ed  c05a474c c05d41cc d7bb0180 d18ed800 d7801080 06a3
5d40: 0001 d7bb0480 05b4 d7145d7c d7145d80 d7145d68 c02f0048 c00ca7d8
5d60: a013  c05a4738 d7bb0180 d7145dac d7145d80 c02f0048 c00ca7b0
5d80: 0001 00c63fc0 d7b66a00 d7b66a00 4040 05b4  d7b66a00
5da0: d7145dcc d7145db0 c032e340 c02effd0 d7145e98 4040 0008c414 
5dc0: d7145e54 d7145dd0 c032f368 c032e310 d7145e24 c02ea81c c03a6040 c03a9c6c
5de0:   d7145ee8  05b4  d7b66adc 
5e00:  d7144000 1854 05b4 27ec 0040 d7116d80 05b4
5e20:   d7145e6c d7b66a00 d7145ee8 d7145e98 4040 4040
5e40: 4040 0002 d7145e74 d7145e58 c03526c8 c032eb0c d7145e78 d7116d80
5e60: d7145ee0 d7116d80 d7145ed4 d7145e78 c02e63a4 c0352688 c05a3dc0 d7142000
5e80: 0040 4040 d76701c0 d7145ee0  d7145e98  
5ea0: d7145ee0 0001   0040 d7145ee8 c6cf2900 
5ec0:  d7145f78 d7145f44 d7145ed8 c00d1c64 c02e62e4  
5ee0: 00089c28 4040 d7116d80   d7145e78 d7216f00 
5f00:     4040   
5f20: 00089c28 d7116d80 00089c28 d7145f78 4040 00089c28 d7145f74 d7145f48
5f40: c00d23a0 c00d1bf4     d7116d80 
5f60: 00089c28 4040 d7145fa4 d7145f78 c00d2948 c00d22c0  
5f80: beed167c 0003 000614dc 0004 c000ea28 d7144000  d7145fa8
5fa0: c000e7e0 c00d2908 beed167c 0003 0003 00089c28 4040 beed167c
5fc0: beed167c 0003 000614dc 0004 00089c28 00060a88 093e beed17a0
5fe0: beed167c beed1648 00029910 b6dc821c 6010 0003  
[c0341cfc] (tcp_v4_early_demux+0xe8/0x154) from [c03211b4] 
(ip_rcv_finish+0x68/0x2c0)
[c03211b4] (ip_rcv_finish+0x68/0x2c0) from [c03218dc] (ip_rcv+0x314/0x398)
[c03218dc] (ip_rcv+0x314/0x398) from [c02fae04] 
(__netif_receive_skb_core+0x520/0x5d8)
[c02fae04] (__netif_receive_skb_core+0x520/0x5d8) from [c02fb990] 
(__netif_receive_skb+0x34/0x88)
[c02fb990] (__netif_receive_skb+0x34/0x88) from [c02fc46c] 
(process_backlog+0x90/0x148)

Re: [PATCH v3 03/10] usb: find internal hub tier mismatch via acpi

2014-01-16 Thread Dan Williams

On Thu, 2014-01-16 at 16:18 -0500, Alan Stern wrote:
 On Thu, 16 Jan 2014, Sarah Sharp wrote:
 
What do you think about the rest of the patchset?
   
   I regret that I haven't taken the time to look through it yet.  :-(  
   I'll do my best to go through it soon.
  
  No worries!  I'm most interested in your comments about the changes to
  khubd, so if you could find time to look at just patches 7 and 9, that
  would be appreciated.
 
 Okay, I have looked at them (and patch 8 too).  In short, I disapprove 
 of the design.  It's an ad-hoc approach that ignores the larger issues.
 
 The real problem is that we need to guarantee mutual exclusion for 
 access to a particular port on the hub.  The things we can do to a port 
 include:
 
   khubd's normal processing:
   turning off various port status-change flags,
   handling remote wakeups,
   taking care of overcurrent events,
   issuing USB-3 warm resets,
   and handling connect/disconnect events;
 
   Issuing a port reset, which can happen:
   when a USB-3 port goes into SS.Inactive,
   when a driver resets a device,
   or when a reset-resume is needed;
 
   Resuming a port (perhaps suspending it too);
 
   Turning port power on or off.
 
 These four somewhat overlapping actions need to be mutually exclusive.  
 (Actually, the first of khubd's activities -- turning off status-change 
 flags -- probably doesn't need to be exclusive with anything else.  But 
 we might as well include it with the rest.)
 
 The natural solution is use a per-port mutex, instead of messing around 
 with atomic busy_bits stuff or PM barriers.
 
 It turns out that we will require a particular locking order.  It will
 sometimes be necessary to acquire the port's mutex while holding the
 child device's lock.  This can happen when we are binding or unbinding
 a driver to the child device; usb_probe_interface() is called with the
 child locked, and it calls usb_autoresume_device() which will need to
 lock the upstream port if the device is in runtime suspend.
 
 As far as I can tell, we don't actually need to acquire the locks in 
 the opposite order, although to make that work means we will have to 
 drop the port lock in various parts of hub_port_connect_change() where 
 the child device gets locked.
 
 One possibility is to use the port's own embedded device lock as the
 port mutex.  But this would preclude ever moving the child USB device
 directly under the port device in the device tree, because the locking
 order would be wrong.  It seems safer to embed a new mutex in struct
 usb_port.
 
 Dan, what do you think of this approach?

Maybe I need to consider it a bit longer, but introducing a per-port
mutex adds at least 2 new lock ordering constraints.  A replacement of
hub-busy_bits with port_dev-lock now introduces a constraint with not
only the child lock, but also synchronous invocations of rpm_{suspend|
resume} for the port.

Patch 7 would look something like this:

---
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index d86548edcc36..da12d07da127 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -4740,6 +4740,13 @@ static void hub_events(void)
 
/* deal with port status changes */
for (i = 1; i = hdev-maxchild; i++) {
+   mutex_lock(port_dev-lock);
+   if (port_dev-power_is_on)
+   do_hub_event(...);
+   else
+   hub_clear_misc_change(..);
+   mutex_unlock(port_dev-lock);
+
[..]
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index 97e4939fee1a..a7f32f200d90 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -88,7 +88,7 @@ static int usb_port_runtime_resume(struct device *dev)
pm_runtime_get_sync(peer-dev);
 
usb_autopm_get_interface(intf);
-   set_bit(port1, hub-busy_bits);
+   mutex_lock(port_dev-lock);
 
retval = usb_hub_set_port_power(hdev, hub, port1, true);
if (port_dev-child  !retval) {
@@ -105,7 +105,7 @@ static int usb_port_runtime_resume(struct device *dev)
retval = 0;
}
 
-   clear_bit(port1, hub-busy_bits);
+   mutex_unlock(port_dev-lock);
usb_autopm_put_interface(intf);
 
if (!hub_is_superspeed(hdev)  peer)

@@ -138,12 +138,12 @@ static int usb_port_runtime_suspend(struct device *dev)
return -EBUSY;
 
usb_autopm_get_interface(intf);
-   set_bit(port1, hub-busy_bits);
+   mutex_lock(port_dev-lock);
retval = usb_hub_set_port_power(hdev, hub, port1, false);
usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_CONNECTION);
if (!hub_is_superspeed(hdev))
usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_ENABLE);
-   clear_bit(port1, hub-busy_bits);
+   

Re: [Stlinux-devel] FW: [PATCH (linux-stm) 1/2] usb: dwc3: fix kernel compilation in gadget mode

2014-01-16 Thread Felipe Balbi
Hi,

On Thu, Jan 16, 2014 at 01:58:30PM +0100, Carmelo Amoroso wrote:
 I'm one of the maintainer of the STLinux kernel @ST.
 Let me clarify the inconvenience occurred about this patches.
 
 We are back-porting some patches from upstream for DWC3 driver into
 our 3.4.58 based kernel as our chipsets use this host controller.

right, that's ok. Would be nice to see your glue layer hitting upstream,
though.

 The way we do it, is exactly what you referred to, by using 'git
 cherry-pick -xs' to keep authorship, upstream reference and so on,
 and when cherry-pick is not straightforward, we are used to specify
 what are the modification added by us wrt the upstream patch.

git cherry-pick would give a merge conflict, in which case you would
just solve it. In any case, if you cherry-pick every patch, you would
never have conflicts to solve, just the usual API change which is
usually very simple to sort out.

 You might have a look at some recent backport stuff we did for our
 3.4 kernel:
 
 http://git.stlinux.com/?p=stm/linux-stm.git;a=commit;h=cef7a3a85413142e73315463eae2fedea451490e
 
 http://git.stlinux.com/?p=stm/linux-stm.git;a=commit;h=71538603d521a6e1b5d5b4ad47c77e6f9dfc8882
 
 (feel free to browse our git.stlinux.com)

just did, you guys have a few things to sort out on your glue layer:

a) PHY shouldn't be directly accessed by the glue layer, just add a PHY
driver and you get all of that for free.

b) you shouldn't access DWC3_GSBUSCFG0 outside of core.c. The right way
to handle that, would be to either pass a quirk flag, or figure
out a detect that the underlying AXI needs this particular
configuration. One thing though, why wasn't that bit set
properly during coreConsultant configuration ?

c) you should be using devm_ioremap_resource() instead of
devm_request_and_ioremap()

d) that xhci_hub_on_disconnect() is nasty

 You will see authorship not changed, properly upstream commit
 reference and so on.
 
 Regarding the merged patches sent by the colleague Giuseppe, I asked
 him to provide me an unique patch just to check locally if the issue
 were actually fixed. It should have been a private, temporary hack
 for testing.

I see.

 It was not meant to be applied in the kernel, neither he had the
 intention to change authorship.
 
 Due to bad git configuration (and user mistakes), when he sent the
 patch it went to our devel-list plus you as in the signed-off list,
 but purely by mistake (--suppress-xxx neither passed)
 
 As kernel maintainer, I can guarantee such a work will not included
 at all in our kernel. It has been just due to user mistakes.
 
 We seriously consider copyright laws.
 
 Our apologies for such inconvenience.

apologies accepted, thanks for clarifying so quickly.

Hope to see your glue layer in mainline soon, believe me, it'll make
your life a lot easier in the long run.

cheers

-- 
balbi


signature.asc
Description: Digital signature


[PATCH 1/2] USB: at91: fix the number of endpoint parameter

2014-01-16 Thread Bo Shen
In sama5d3 SoC, there are 16 endpoints. As the USBA_NR_ENDPOINTS
is only 7. So, fix it for sama5d3 SoC using the udc-num_ep.

Signed-off-by: Bo Shen voice.s...@atmel.com
---

 drivers/usb/gadget/atmel_usba_udc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/atmel_usba_udc.c 
b/drivers/usb/gadget/atmel_usba_udc.c
index 2cb52e0..7e67a81 100644
--- a/drivers/usb/gadget/atmel_usba_udc.c
+++ b/drivers/usb/gadget/atmel_usba_udc.c
@@ -1670,7 +1670,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
if (ep_status) {
int i;
 
-   for (i = 0; i  USBA_NR_ENDPOINTS; i++)
+   for (i = 0; i  udc-num_ep; i++)
if (ep_status  (1  i)) {
if (ep_is_control(udc-usba_ep[i]))
usba_control_irq(udc, udc-usba_ep[i]);
-- 
1.8.5.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  http://vger.kernel.org/majordomo-info.html


[PATCH 2/2] USB: at91: using USBA_NR_DMAS for DMA channels

2014-01-16 Thread Bo Shen
When the SoC is earlier than sama5d3 SoC, which have the same number
endpoints and DMAs. However for sama5d3 SoC, it has different number
for endpoints and DMAs. So, define USBA_NR_DMAs for DMA channels

Signed-off-by: Bo Shen voice.s...@atmel.com
---

 drivers/usb/gadget/atmel_usba_udc.c | 2 +-
 drivers/usb/gadget/atmel_usba_udc.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/atmel_usba_udc.c 
b/drivers/usb/gadget/atmel_usba_udc.c
index 7e67a81..5cded1c 100644
--- a/drivers/usb/gadget/atmel_usba_udc.c
+++ b/drivers/usb/gadget/atmel_usba_udc.c
@@ -1661,7 +1661,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
if (dma_status) {
int i;
 
-   for (i = 1; i  USBA_NR_ENDPOINTS; i++)
+   for (i = 1; i  USBA_NR_DMAS; i++)
if (dma_status  (1  i))
usba_dma_irq(udc, udc-usba_ep[i]);
}
diff --git a/drivers/usb/gadget/atmel_usba_udc.h 
b/drivers/usb/gadget/atmel_usba_udc.h
index 2922db5..a70706e 100644
--- a/drivers/usb/gadget/atmel_usba_udc.h
+++ b/drivers/usb/gadget/atmel_usba_udc.h
@@ -210,7 +210,7 @@
 #define USBA_FIFO_BASE(x)  ((x)  16)
 
 /* Synth parameters */
-#define USBA_NR_ENDPOINTS  7
+#define USBA_NR_DMAS   7
 
 #define EP0_FIFO_SIZE  64
 #define EP0_EPT_SIZE   USBA_EPT_SIZE_64
-- 
1.8.5.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  http://vger.kernel.org/majordomo-info.html


Re: [BUG] FL1009: xHCI host not responding to stop endpoint command.

2014-01-16 Thread Arnaud Ebalard
Hi,

Sarah Sharp sarah.a.sh...@linux.intel.com writes:

 ... AFAICT, this is exactly what commit 35773dac5f86 does and reverting
 it does not help. If I am mistaken, can you point which part you want me
 to remove in the code to test?
 
 I am slowly starting to see a bisect session coming ;-)

 Try reverting commit 60e102ac73cd40069d077014c93c86dc7205cb68.

AFAICT, this commit does not exist in master (Linus tree), i.e. it is
not in 3.13.0-rc8.

 That was causing issues with another Fresco Logic host.  If that
 doesn't help, then yes, please git bisect.

I have started a bisect session yesterday. I will try and finish it this
evening.

Cheers,

a+
--
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 0/2] ohci and ehci-platform clks, phy and dt support

2014-01-16 Thread Florian Fainelli
Le mercredi 15 janvier 2014, 15:26:21 Alan Stern a écrit :
 On Wed, 15 Jan 2014, Hans de Goede wrote:
  Hi All,
  
  This version of my ohci and ehci-platform clks, phy and dt support
  patch-set, really fixes the 2 small bugs Alan found.
 
 All okay -- this time I can't find anything to complain about.  :-)

There is one minor issue; which is that the ehci binding claims the driver 
supports the following optional boolean properties:

- big-endian-regs : boolean, set this for hcds with big-endian registers
- big-endian-desc : boolean, set this for hcds with big-endian descriptors
- big-endian : boolean, for hcds with big-endian-regs + big-endian-desc

while it does not (yet) so this is misleading. Can we at get that fixed before 
merging? Copy pasting the PPC ehci driver should do the job.
-- 
Florian
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 0/3] usb: gadget: f_fs: add asynchronous I/O support

2014-01-16 Thread Robert Baldyga
Hello,

This is second version of patches containing improvements for FunctionFS which
allows to use it with asynchronous I/O interface. It also adds poll function
for ep0, to make it usable without creating additional thread, needed by
blocking I/O.

From last version I have made modifications suggested by Michal Nazarewicz,
and added few other fixes. A also added patch fixing setup requests handling.

More details in commit messages.

Best regards
Robert Baldyga
Samsung RD Institute Poland

Changelog:

v2:
- fix style problems
- add error handling in ffs_user_copy_worker()
- fix paremeters of aio_complete() in ffs_epfile_async_io_complete()
- in ffs_epfile_io() do copy_from_user() calls with spinlock unlocked
- in ffs_ep0_poll() remove default from switch-case block and add case
  for each enum value
- fix returned value in __ffs_ep0_queue_wait() funcion

v1: http://www.spinics.net/lists/linux-usb/msg100969.html
- initial proposal

Robert Baldyga (3):
  usb: gadget: f_fs: fix setup request handling
  usb: gadget: f_fs: add poll for endpoint 0
  usb: gadget: f_fs: add aio support

 drivers/usb/gadget/f_fs.c |  303 -
 drivers/usb/gadget/u_fs.h |1 -
 2 files changed, 275 insertions(+), 29 deletions(-)

-- 
1.7.9.5

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 3/3] usb: gadget: f_fs: add aio support

2014-01-16 Thread Robert Baldyga
This patch adds asynchronous I/O support for FunctionFS endpoint files.
It adds ffs_epfile_aio_write() and ffs_epfile_aio_read() functions responsible
for preparing AIO operations.

It also modifies ffs_epfile_io() function, adding aio handling code. Instead
of extending list of parameters of this function, there is new struct
ffs_io_data which contains all information needed to perform I/O operation.
Pointer to this struct replaces buf and len parameters of ffs_epfile_io()
function. Allocated buffer is freed immediately only after sync operation,
because in async IO it's freed in complete funcion. For each async operation
an USB request is allocated, because it allows to have more than one request
queued on single endpoint.

According to changes in ffs_epfile_io() function, functions ffs_epfile_write()
and ffs_epfile_read() are updated to use new API.

For asynchronous I/O operations there is new request complete function named
ffs_epfile_async_io_complete(), which completes AIO operation, and frees
used memory.

Signed-off-by: Robert Baldyga r.bald...@samsung.com
---
 drivers/usb/gadget/f_fs.c |  259 -
 1 file changed, 233 insertions(+), 26 deletions(-)

diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c
index 6c2294c..d4038eb 100644
--- a/drivers/usb/gadget/f_fs.c
+++ b/drivers/usb/gadget/f_fs.c
@@ -28,6 +28,8 @@
 #include linux/usb/composite.h
 #include linux/usb/functionfs.h
 
+#include linux/aio.h
+#include linux/mmu_context.h
 #include linux/poll.h
 
 #include u_fs.h
@@ -150,6 +152,25 @@ struct ffs_epfile {
unsigned char   _pad;
 };
 
+/*  ffs_io_data structure ***/
+
+struct ffs_io_data {
+   bool aio;
+   bool read;
+
+   struct kiocb *kiocb;
+   const struct iovec *iovec;
+   unsigned long nr_segs;
+   char __user *buf;
+   size_t len;
+
+   struct mm_struct *mm;
+   struct work_struct work;
+
+   struct usb_ep *ep;
+   struct usb_request *req;
+};
+
 static int  __must_check ffs_epfiles_create(struct ffs_data *ffs);
 static void ffs_epfiles_destroy(struct ffs_epfile *epfiles, unsigned count);
 
@@ -623,8 +644,55 @@ static void ffs_epfile_io_complete(struct usb_ep *_ep, 
struct usb_request *req)
}
 }
 
-static ssize_t ffs_epfile_io(struct file *file,
-char __user *buf, size_t len, int read)
+static void ffs_user_copy_worker(struct work_struct *work)
+{
+   size_t len = 0;
+   int i = 0;
+   int ret;
+
+   struct ffs_io_data *io_data = container_of(work, struct ffs_io_data,
+  work);
+   ret = io_data-len;
+
+   use_mm(io_data-mm);
+   for (i = 0; i  io_data-nr_segs; i++) {
+   if (unlikely(copy_to_user(io_data-iovec[i].iov_base,
+io_data-buf[len],
+io_data-iovec[i].iov_len))) {
+   ret = -EFAULT;
+   break;
+   }
+   len += io_data-iovec[i].iov_len;
+   }
+   unuse_mm(io_data-mm);
+
+   aio_complete(io_data-kiocb, ret, ret);
+
+   kfree(io_data-iovec);
+   kfree(io_data-buf);
+   kfree(io_data);
+}
+
+static void ffs_epfile_async_io_complete(struct usb_ep *_ep,
+struct usb_request *req)
+{
+   struct ffs_io_data *io_data = req-context;
+   int ret = req-status ? req-status : req-actual;
+   io_data-len = req-actual;
+
+   if (io_data-read  ret  0) {
+   INIT_WORK(io_data-work, ffs_user_copy_worker);
+   schedule_work(io_data-work);
+   } else {
+   aio_complete(io_data-kiocb, ret, ret);
+   kfree(io_data-buf);
+   kfree(io_data);
+   }
+
+   usb_ep_free_request(_ep, req);
+}
+
+static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
 {
struct ffs_epfile *epfile = file-private_data;
struct usb_gadget *gadget = epfile-ffs-gadget;
@@ -655,7 +723,7 @@ static ssize_t ffs_epfile_io(struct file *file,
}
 
/* Do we halt? */
-   halt = !read == !epfile-in;
+   halt = (!io_data-read == !epfile-in);
if (halt  epfile-isoc) {
ret = -EINVAL;
goto error;
@@ -667,15 +735,32 @@ static ssize_t ffs_epfile_io(struct file *file,
 * Controller may require buffer size to be aligned to
 * maxpacketsize of an out endpoint.
 */
-   data_len = read ? usb_ep_align_maybe(gadget, ep-ep, len) : len;
+   data_len = io_data-read ? usb_ep_align_maybe(gadget, ep-ep,
+io_data-len) :
+  io_data-len;
 
data = kmalloc(data_len, GFP_KERNEL);
if 

[PATCH v2 2/3] usb: gadget: f_fs: add poll for endpoint 0

2014-01-16 Thread Robert Baldyga
This patch adds poll function for file representing ep0.

Ability of read from or write to ep0 file is related with actual state of ffs:
- When desctiptors or strings are not written yet, POLLOUT flag is set.
- If there is any event to read, POLLIN flag is set.
- If setup request was read, POLLIN and POLLOUT flag is set, to allow
  send response (by performing I/O operation consistent with setup request
  direction) or set stall (by performing I/O operation opposite  setup
  request direction).

Signed-off-by: Robert Baldyga r.bald...@samsung.com
---
 drivers/usb/gadget/f_fs.c |   42 ++
 1 file changed, 42 insertions(+)

diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c
index 44a6bbe..6c2294c 100644
--- a/drivers/usb/gadget/f_fs.c
+++ b/drivers/usb/gadget/f_fs.c
@@ -28,6 +28,8 @@
 #include linux/usb/composite.h
 #include linux/usb/functionfs.h
 
+#include linux/poll.h
+
 #include u_fs.h
 #include configfs.h
 
@@ -558,6 +560,45 @@ static long ffs_ep0_ioctl(struct file *file, unsigned 
code, unsigned long value)
return ret;
 }
 
+static unsigned int ffs_ep0_poll(struct file *file, poll_table *wait)
+{
+   struct ffs_data *ffs = file-private_data;
+   unsigned int mask = POLLWRNORM;
+   int ret;
+
+   ret = ffs_mutex_lock(ffs-mutex, file-f_flags  O_NONBLOCK);
+   if (unlikely(ret  0))
+   return mask;
+
+   switch (ffs-state) {
+   case FFS_READ_DESCRIPTORS:
+   case FFS_READ_STRINGS:
+   mask |= POLLOUT;
+   break;
+
+   case FFS_ACTIVE:
+   switch (FFS_SETUP_STATE(ffs)) {
+   case FFS_NO_SETUP:
+   if (ffs-ev.count)
+   mask |= POLLIN;
+   break;
+
+   case FFS_SETUP_PENDING:
+   mask |= (POLLIN | POLLOUT);
+   break;
+
+   case FFS_SETUP_CANCELED:
+   break;
+   }
+   case FFS_CLOSING:
+   break;
+   }
+
+   mutex_unlock(ffs-mutex);
+
+   return mask;
+}
+
 static const struct file_operations ffs_ep0_operations = {
.llseek =   no_llseek,
 
@@ -566,6 +607,7 @@ static const struct file_operations ffs_ep0_operations = {
.read = ffs_ep0_read,
.release =  ffs_ep0_release,
.unlocked_ioctl =   ffs_ep0_ioctl,
+   .poll = ffs_ep0_poll,
 };
 
 
-- 
1.7.9.5

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 1/3] usb: gadget: f_fs: fix setup request handling

2014-01-16 Thread Robert Baldyga
This patch fixes __ffs_ep0_queue_wait() function, which now returns number of
bytes transferred in USB request or error code in case of failure. This is
needed by ffs_ep0_read() function, when read data is copied to userspace.

It also cleans up code by removing usused variable ep0req_status.

Signed-off-by: Robert Baldyga r.bald...@samsung.com
---
 drivers/usb/gadget/f_fs.c |2 +-
 drivers/usb/gadget/u_fs.h |1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c
index 306a2b5..44a6bbe 100644
--- a/drivers/usb/gadget/f_fs.c
+++ b/drivers/usb/gadget/f_fs.c
@@ -218,7 +218,7 @@ static int __ffs_ep0_queue_wait(struct ffs_data *ffs, char 
*data, size_t len)
}
 
ffs-setup_state = FFS_NO_SETUP;
-   return ffs-ep0req_status;
+   return req-status ? req-status : req-actual;
 }
 
 static int __ffs_ep0_stall(struct ffs_data *ffs)
diff --git a/drivers/usb/gadget/u_fs.h b/drivers/usb/gadget/u_fs.h
index bc2d371..55d2f2e 100644
--- a/drivers/usb/gadget/u_fs.h
+++ b/drivers/usb/gadget/u_fs.h
@@ -156,7 +156,6 @@ struct ffs_data {
 */
struct usb_request  *ep0req;/* P: mutex */
struct completion   ep0req_completion;  /* P: mutex */
-   int ep0req_status;  /* P: mutex */
 
/* reference counter */
atomic_tref;
-- 
1.7.9.5

--
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: XHCI issues: WD MyBook 1230 - reset SuperSpeed USB device

2014-01-16 Thread Hannes Reinecke
On 01/16/2014 09:48 PM, Alan Stern wrote:
 It's now clear that this is _not_ an XHCI issue, contrary to what 
 $SUBJECT says.
 
 On Thu, 16 Jan 2014, Peter Palúch wrote:
 
 Alan,

 I am attaching the usbmon trace after the drive has been plugged into 
 the USB-2 port. Record of the drive in stall should occur around the 
 file offset 87808 (decimal). The log was done on the 3.12.7 kernel 
 without CONFIG_PM. Should I do a usbmon trace on my regular kernel with 
 CONFIG_PM as well?
 
 No need.
 
 dmesg transcript:

 root@bach:/tmp# dmesg
 usb 4-1.2: new high-speed USB device number 5 using ehci-pci
 usb 4-1.2: New USB device found, idVendor=1058, idProduct=1230
 usb 4-1.2: New USB device strings: Mfr=2, Product=3, SerialNumber=1
 usb 4-1.2: Product: My Book 1230
 usb 4-1.2: Manufacturer: Western Digital
 usb 4-1.2: SerialNumber: 574D43344E30323438393836
 usb-storage 4-1.2:1.0: USB Mass Storage device detected
 scsi6 : usb-storage 4-1.2:1.0
 usbcore: registered new interface driver usb-storage
 scsi 6:0:0:0: Direct-Access WD   My Book 1230 1050 PQ: 0 ANSI: 6
 scsi 6:0:0:1: Enclosure WD   SES Device   1050 PQ: 0 ANSI: 6
 sd 6:0:0:0: Attached scsi generic sg2 type 0
 scsi 6:0:0:1: Attached scsi generic sg3 type 13
 sd 6:0:0:0: [sdb] Spinning up disk...
 .ready
 sd 6:0:0:0: [sdb] 732558336 4096-byte logical blocks: (3.00 TB/2.72 TiB)
 sd 6:0:0:0: [sdb] Write Protect is off
 sd 6:0:0:0: [sdb] Mode Sense: 53 00 10 08
 sd 6:0:0:0: [sdb] No Caching mode page found
 sd 6:0:0:0: [sdb] Assuming drive cache: write through
 sd 6:0:0:0: [sdb] 732558336 4096-byte logical blocks: (3.00 TB/2.72 TiB)
 sd 6:0:0:0: [sdb] No Caching mode page found
 sd 6:0:0:0: [sdb] Assuming drive cache: write through
   sdb: sdb1
 sd 6:0:0:0: [sdb] 732558336 4096-byte logical blocks: (3.00 TB/2.72 TiB)
 sd 6:0:0:0: [sdb] No Caching mode page found
 sd 6:0:0:0: [sdb] Assuming drive cache: write through
 sd 6:0:0:0: [sdb] Attached SCSI disk
 usb 4-1.2: reset high-speed USB device number 5 using ehci-pci
 
 It looks like the reset occurred because the computer sent an
 ATA-passthru command to the disk, and the disk wasn't prepared to
 handle it properly.  The firmware crashed, requiring a reset.
 
 If anyone can explain, the command bytes in question were:
 
   85082e00   ec00
 
 and the sense data was:
 
   7201001d 000e 090c 5d00 0100 0050
 
 I don't know what either of these means, or even what software was
 responsible for sending this command.  It appears to have come from
 some user program, though, not the kernel.  Possibly something run by 
 udev.
 
Probably smartd.
The logic there is _less_ than perfect.
Try to disable smartd and check if the issue remains.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke   zSeries  Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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