Re: [PATCH resend v3 1/4] usb: usbip tool: Check the return of get_nports()

2017-05-19 Thread Shuah Khan
On 05/19/2017 12:46 AM, Yuyang Du wrote:
> On Thu, May 18, 2017 at 03:53:04PM +0300, Sergei Shtylyov wrote:
>>> +   if (vhci_driver->nports <=0) {
>>
>>Please add a space after <= too.
> 
> Indeed. Thanks.
> 
> Hi Greg,
> 
> Do you want me to send another version to fix this?
> 

Yes. Please resend fixing this. I am surprised checkpatch.pl
didn't catch it!

thanks,
-- Shuah

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


Re: [PATCH v2 0/9] usbip: Enable USB3 SuperSpeed

2017-05-19 Thread Shuah Khan
On 05/18/2017 04:08 AM, Yuyang Du wrote:
> Hi Shuah,
> 
> SuperSpeed (only) USB devices cannot be shared via usbip. This
> patch series attempts to fix it.
> 
> The first 5 patches refactors the existing code to prepare for the
> SuperSpeed addition. With this series, our SuperSpeed device works
> fine.
> 
> Many thanks to Greg and Krzysztof for their patience to answer my
> non-usb-professional questions, and special thanks to Krzysztof for
> the pointer to the SuperSpeed patch in dummy_hcd.
> 
> This series is based on the series: "usb: usbip: Fix ports and port
> status v3" (https://www.spinics.net/lists/linux-usb/msg155834.html).
> 
> v2:
>  - Remove set_link_state() since it is not used
> 
> Regards,
> Yuyang

Hi Yuyang,

Thanks for the patch series. I will try to review this next week
on my business trip. If not, I will definitely get to it the week
of May 29th.

thanks,
-- Shuah

> 
> --
> 
> Yuyang Du (9):
>   usbip: vhci-hcd: Rename function names to reflect their struct names
>   usbip: vhci-hcd: Add vhci struct
>   usbip: vhci-hcd: Move VHCI platform device into vhci struct
>   usbip: vhci-hcd: Rework vhci_hcd_init
>   usbip: vhci-hcd: Set the vhci structure up to work
>   usbip: vhci-hcd: Add USB3 SuperSpeed support
>   usbip: Add USB_SPEED_SUPER as valid arg
>   usbip: vhci-hcd: Add USB3 port status bits
>   usbip: vhci-hcd: Clean up the code by adding a new macro
> 
>  drivers/usb/usbip/vhci.h |  36 ++-
>  drivers/usb/usbip/vhci_hcd.c | 605 
> +--
>  drivers/usb/usbip/vhci_rx.c  |  16 +-
>  drivers/usb/usbip/vhci_sysfs.c   | 138 +---
>  tools/usb/usbip/libsrc/vhci_driver.c |  25 +-
>  tools/usb/usbip/libsrc/vhci_driver.h |   9 +-
>  tools/usb/usbip/src/usbip_attach.c   |   3 +-
>  7 files changed, 601 insertions(+), 231 deletions(-)
> 

--
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] ezusb: Delete an error message for a failed memory allocation in ezusb_writememory()

2017-05-19 Thread SF Markus Elfring
From: Markus Elfring 
Date: Fri, 19 May 2017 22:30:21 +0200

Omit an extra message for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Link: 
http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
Signed-off-by: Markus Elfring 
---
 drivers/usb/misc/ezusb.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/misc/ezusb.c b/drivers/usb/misc/ezusb.c
index 837208f14f86..6d5e468dc897 100644
--- a/drivers/usb/misc/ezusb.c
+++ b/drivers/usb/misc/ezusb.c
@@ -41,11 +41,9 @@ static int ezusb_writememory(struct usb_device *dev, int 
address,
return -ENODEV;
 
transfer_buffer = kmemdup(data, length, GFP_KERNEL);
-   if (!transfer_buffer) {
-   dev_err(>dev, "%s - kmalloc(%d) failed.\n",
-   __func__, length);
+   if (!transfer_buffer)
return -ENOMEM;
-   }
+
result = usb_control_msg(dev, usb_sndctrlpipe(dev, 0), request,
 USB_DIR_OUT | USB_TYPE_VENDOR | 
USB_RECIP_DEVICE,
 address, 0, transfer_buffer, length, 3000);
-- 
2.13.0

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


[PATCH 3/4] ueagle-atm: Delete unnecessary return statements in two functions

2017-05-19 Thread SF Markus Elfring
From: Markus Elfring 
Date: Fri, 19 May 2017 19:22:12 +0200

The script "checkpatch.pl" pointed information out like the following.

WARNING: void function return statements are not generally useful

Thus remove such a statement in the affected functions.

Signed-off-by: Markus Elfring 
---
 drivers/usb/atm/ueagle-atm.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/usb/atm/ueagle-atm.c b/drivers/usb/atm/ueagle-atm.c
index 9725e6f2f301..0ca4ff3e6683 100644
--- a/drivers/usb/atm/ueagle-atm.c
+++ b/drivers/usb/atm/ueagle-atm.c
@@ -1058,7 +1058,6 @@ static void __uea_load_page_e4(struct uea_softc *sc, u8 
pageno, int boot)
 
 bad:
uea_err(INS_TO_USBDEV(sc), "sending DSP block %u failed\n", blockno);
-   return;
 }
 
 static void uea_load_page_e4(struct work_struct *work)
@@ -2101,7 +2100,6 @@ static void uea_dispatch_cmv_e4(struct uea_softc *sc, 
struct intr_pkt *intr)
E4_FUNCTION_TYPE(cmv->wFunction),
E4_FUNCTION_SUBTYPE(cmv->wFunction));
uea_leaves(INS_TO_USBDEV(sc));
-   return;
 }
 
 static void uea_schedule_load_page_e1(struct uea_softc *sc,
-- 
2.13.0

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


[PATCH 2/4] ueagle-atm: Improve a size determination in uea_bind()

2017-05-19 Thread SF Markus Elfring
From: Markus Elfring 
Date: Fri, 19 May 2017 19:09:28 +0200

Replace the specification of a data structure by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

Signed-off-by: Markus Elfring 
---
 drivers/usb/atm/ueagle-atm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/atm/ueagle-atm.c b/drivers/usb/atm/ueagle-atm.c
index 16fd87df69e1..9725e6f2f301 100644
--- a/drivers/usb/atm/ueagle-atm.c
+++ b/drivers/usb/atm/ueagle-atm.c
@@ -2551,7 +2551,7 @@ static int uea_bind(struct usbatm_data *usbatm, struct 
usb_interface *intf,
return ret;
}
 
-   sc = kzalloc(sizeof(struct uea_softc), GFP_KERNEL);
+   sc = kzalloc(sizeof(*sc), GFP_KERNEL);
if (!sc)
return -ENOMEM;
 
-- 
2.13.0

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


Re: [PATCH 1/4] usb: host: ohci-platform: Add basic runtime PM support

2017-05-19 Thread Tony Lindgren
* Alan Stern  [170518 10:24]:
> On Wed, 17 May 2017, Tony Lindgren wrote:
> 
> > This is needed in preparation of adding support for omap3 and
> > later OHCI. The runtime PM will only do something on platforms
> > that implement it.
> 
> > @@ -51,6 +52,10 @@ static int ohci_platform_power_on(struct platform_device 
> > *dev)
> > struct ohci_platform_priv *priv = hcd_to_ohci_priv(hcd);
> > int clk, ret, phy_num;
> >  
> > +   ret = pm_runtime_get_sync(>dev);
> > +   if (ret < 0)
> > +   return ret;
> > +
> 
> I don't remember how probing for platform device drivers is set up.
> So although this is definitely the wrong place for 
> pm_runtime_get_sync(), it may turn out that you need to do a 
> pm_runtime_get_noresume() before calling pm_runtime_enable(), and a 
> pm_runtime_put() at the end of ohci_platform_probe().  Otherwise 
> runtime PM might kick in during the middle of the probe sequence -- not 
> what you want.

Just removing those parts seems to work for probing.

> Similarly, you may need pm_runtime_get_sync() at the start of 
> ohci_platform_remove() and pm_runtime_put_noidle() at the end.

Yup that I noticed that too trying to rmmod it with no devices
connected.

While testing I also saw once "external abort on non-linefetch":

(ohci_hub_status_data [ohci_hcd]) from []
(usb_hcd_poll_rh_status+0x3c/0x128 [usbcore])
(usb_hcd_poll_rh_status [usbcore]) from []
(call_timer_fn+0xb0/0x3f8)
(call_timer_fn) from [] (expire_timers+0xe4/0x220)
(expire_timers) from [] (run_timer_softirq+0x94/0x19c)
(run_timer_softirq) from [] (__do_softirq+0x140/0x570)

Also I'm still checking if legacy usb_phy vs phy needs handling.
So far it seems that no need to do anything because most devices
need a USB hub anyways unless they have USB serial transceiver
configured with drivers/mfd/omap-usb-host.c for LS/FS.

Regards,

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


[PATCH 4/4] ueagle-atm: Adjust three checks for null pointers

2017-05-19 Thread SF Markus Elfring
From: Markus Elfring 
Date: Fri, 19 May 2017 19:29:08 +0200
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The script “checkpatch.pl” pointed information out like the following.

Comparison to NULL could be written !…

Thus fix the affected source code places.

Signed-off-by: Markus Elfring 
---
 drivers/usb/atm/ueagle-atm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/atm/ueagle-atm.c b/drivers/usb/atm/ueagle-atm.c
index 0ca4ff3e6683..5c9517bfb82c 100644
--- a/drivers/usb/atm/ueagle-atm.c
+++ b/drivers/usb/atm/ueagle-atm.c
@@ -955,7 +955,7 @@ static void uea_load_page_e1(struct work_struct *work)
sc->dsp_firm = NULL;
}
 
-   if (sc->dsp_firm == NULL && request_dsp(sc) < 0)
+   if (!sc->dsp_firm && request_dsp(sc) < 0)
return;
 
p = sc->dsp_firm->data;
@@ -1076,7 +1076,7 @@ static void uea_load_page_e4(struct work_struct *work)
sc->dsp_firm = NULL;
}
 
-   if (sc->dsp_firm == NULL && request_dsp(sc) < 0)
+   if (!sc->dsp_firm && request_dsp(sc) < 0)
return;
 
p = (struct l1_code *) sc->dsp_firm->data;
@@ -1596,7 +1596,7 @@ static void cmvs_file_name(struct uea_softc *sc, char 
*const cmv_name, int ver)
 
kernel_param_lock(THIS_MODULE);
/* set proper name corresponding modem version and line type */
-   if (cmv_file[sc->modem_index] == NULL) {
+   if (!cmv_file[sc->modem_index]) {
if (UEA_CHIP_VERSION(sc) == ADI930)
file_arr[3] = '9';
else if (UEA_CHIP_VERSION(sc) == EAGLE_IV)
-- 
2.13.0

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


[PATCH 1/4] ueagle-atm: Delete error messages for failed memory allocations in two functions

2017-05-19 Thread SF Markus Elfring
From: Markus Elfring 
Date: Fri, 19 May 2017 19:02:01 +0200

Omit two extra messages for memory allocation failures in these functions.

This issue was detected by using the Coccinelle software.

Link: 
http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
Signed-off-by: Markus Elfring 
---
 drivers/usb/atm/ueagle-atm.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/atm/ueagle-atm.c b/drivers/usb/atm/ueagle-atm.c
index df67815f74e6..16fd87df69e1 100644
--- a/drivers/usb/atm/ueagle-atm.c
+++ b/drivers/usb/atm/ueagle-atm.c
@@ -868,10 +868,8 @@ static int uea_idma_write(struct uea_softc *sc, const void 
*data, u32 size)
int bytes_read;
 
xfer_buff = kmemdup(data, size, GFP_KERNEL);
-   if (!xfer_buff) {
-   uea_err(INS_TO_USBDEV(sc), "can't allocate xfer_buff\n");
+   if (!xfer_buff)
return ret;
-   }
 
ret = usb_bulk_msg(sc->usb_dev,
 usb_sndbulkpipe(sc->usb_dev, UEA_IDMA_PIPE),
@@ -1147,10 +1145,8 @@ static int uea_request(struct uea_softc *sc,
int ret = -ENOMEM;
 
xfer_buff = kmemdup(data, size, GFP_KERNEL);
-   if (!xfer_buff) {
-   uea_err(INS_TO_USBDEV(sc), "can't allocate xfer_buff\n");
+   if (!xfer_buff)
return ret;
-   }
 
ret = usb_control_msg(sc->usb_dev, usb_sndctrlpipe(sc->usb_dev, 0),
  UCDC_SEND_ENCAPSULATED_COMMAND,
-- 
2.13.0

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


[PATCH 0/4] ueagle-atm: Adjustments for eight function implementations

2017-05-19 Thread SF Markus Elfring
From: Markus Elfring 
Date: Fri, 19 May 2017 19:41:23 +0200

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (4):
  Delete error messages for failed memory allocations in two functions
  Improve a size determination in uea_bind()
  Delete unnecessary return statements in two functions
  Adjust three checks for null pointers

 drivers/usb/atm/ueagle-atm.c | 18 ++
 1 file changed, 6 insertions(+), 12 deletions(-)

-- 
2.13.0

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


Re: [PATCH 3/4] usb: host: ohci-omap3: Print warning to get people to use ohci-platform

2017-05-19 Thread Tony Lindgren
* Sebastian Reichel  [170518 10:25]:
> Hi,
> 
> On Thu, May 18, 2017 at 07:08:58AM -0700, Tony Lindgren wrote:
> > * Sebastian Reichel  [170518 02:18]:
> > > On Wed, May 17, 2017 at 03:59:21PM -0700, Tony Lindgren wrote:
> > > > We can't just remove ohci-omap3 as that could make some people's
> > > > systems unusable for keyboard and mouse. Let's print a warning for
> > > > now, and then remove the driver in a year or so.
> > >
> > > I guess you could drop the driver and modify the Kconfig entry,
> > > so that it selects USB_OHCI_HCD_PLATFORM? I just checked and
> > > there are already a few examples of this in the USB host Kconfig :)
> > 
> > Right but there's still nothing telling people to load ohci-platform
> > instead of ohci-omap3 if we just remove ohci-omap3 without any warnings.
> 
> Shouldn't the driver be loaded automatically due to the DT
> compatible value?

Doesn't that depend on a collection of user space software of
varying quality and usability? :)

But I think we can actually ohci-omap3 once the issues are sorted
out. That's because for OHCI omaps need an OHCI PHY. In most cases
there is just EHCI PHY and an external hub is needed. I'll do some
more testing.

Regards,

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


HELLO!!!!!

2017-05-19 Thread MR. NEIL TROTTER
HELLO!

I am Mr  Neil Trotter, the current winner of 108  Euro millions jackpot, if you 
have received this email then you are of the lucky fellows to benefit from 
me,so do get back to me for a better understanding. Here is the website for 
proof
http://www.huffingtonpost.co.uk/2014/03/18/neil-trotter-euromillions-winner_n_4984234.html}
Contact Email;(mr.neiltrotter...@outlook.com)

THANKS,
MR. NEIL TROTTER.
--
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: Runtime PM enabled EHCI does not respond to device plugging.

2017-05-19 Thread Alan Stern
On Fri, 19 May 2017, Kai-Heng Feng wrote:

> Hi,
> 
> This issue happens on Carrizo AMD laptops, only EHCI is affected, XHCI
> works fine on the same machine.
> 
> I can see lots of USB wakeup and resume messages showed every two seconds.
> Plug USB devices to the EHCI port does not change anything.
> 
> dmesg with ehci-hcd, ehci-pci and usbcore dynamic debug enabled is attached:
> 
> [   88.556535] ehci-pci :00:13.0: hcd_pci_runtime_resume: 0
> [   88.556563] usb usb2: usb auto-resume
> [   88.556588] hub 2-0:1.0: hub_resume
> [   88.556676] usb usb2-port1: status 0507 change 
> [   88.556794] hub 2-0:1.0: state 7 ports 2 chg  evt 
> [   88.556862] usb 2-1: usb auto-resume
> [   88.624577] usb 2-1: Waited 0ms for CONNECT
> [   88.624585] usb 2-1: finish resume
> [   88.624838] hub 2-1:1.0: hub_resume
> [   88.624964] usb 2-1-port1: status 0507 change 
> [   88.625085] usb 2-1-port2: status 0107 change 
> [   88.625354] usb 2-1-port4: status 0507 change 
> [   88.625403] hub 2-1:1.0: state 7 ports 4 chg  evt 
> [   88.625605] usb 2-1.1: usb auto-resume
> [   88.692635] usb 2-1.1: Waited 0ms for CONNECT
> [   88.692640] usb 2-1.1: finish resume
> [   90.800874] usb 2-1.1: usb auto-suspend, wakeup 1
> [   90.820326] hub 2-1:1.0: hub_suspend
> [   90.823161] usb 2-1: usb auto-suspend, wakeup 1
> [   90.840362] hub 2-0:1.0: hub_suspend
> [   90.840399] usb usb2: bus auto-suspend, wakeup 1
> [   90.860346] ehci-pci :00:13.0: hcd_pci_runtime_suspend: 0

Your log shows that the 2-1.1 device is a USB2.0-CRW card reader, and
it uses the rtsx_usb driver.  This driver probes the reader every few
seconds to see whether a card has been inserted (actually, it probes
_twice_: once to see if an SD card is present and once to see if a
MemoryStick card is present).  Each probe (or pair of probes) causes a
resume followed by a suspend.

The way to prevent all those resumes and suspends -- if that's what you
want to do -- is to disable autosuspend for the card reader.  The best
way to do this depends on your system, but

echo on >/sys/bus/usb/devices/2-1.1/power/control

should work.

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: [RFC V1 1/1] net: cdc_ncm: Reduce memory use when kernel memory low

2017-05-19 Thread David Laight
From: Bjørn Mork
> Sent: 19 May 2017 14:56
...
> Unless someone has a nice way to just collect a list of skbs and have
> them converted to proper framing on the fly when transmitting, without
> having to care about USB packet boundaries.

skb can be linked into arbitrary chains (or even trees), but I suspect
the usbnet code would need to be taught about them.

For XHCI it isn't too bad because it will do arbitrary scatter-gather
(apart from the ring end).
But I believe the earlier controllers only support fragments that
end on usb packet boundaries.

I looked at the usbnet code a few years ago, I'm sure it ought to
be possible to shortcut most of the code that uses URB and directly
write to the xhci (in particular) ring descriptors.

For receive you probably want to feed the USB stack multiple (probably)
2k buffers, and handle the debatching into ethernet frames yourself.

One of the ASIX drivers used to be particularly bad, it allocated 64k
skb for receive (the hardware can merge IP packets) and then hacked
the true_size before passing upstream.

David

N�r��yb�X��ǧv�^�)޺{.n�+{��^n�r���z���h�&���G���h�(�階�ݢj"���m��z�ޖ���f���h���~�m�

[PATCH v4] usb: musb: musb_cppi41: Defer probe only if DMA is not ready

2017-05-19 Thread Alexandre Bailon
If dma_request_slave_channel() failed to return a channel,
then the driver will print an error and request to defer probe,
regardless of the cause of the failure.
Defer if the DMA is not ready yet otherwise print an error.

Signed-off-by: Alexandre Bailon 
Reviewed-by: Johan Hovold 
---
 drivers/usb/musb/musb_cppi41.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
index e7c8b1b..ba25528 100644
--- a/drivers/usb/musb/musb_cppi41.c
+++ b/drivers/usb/musb/musb_cppi41.c
@@ -673,12 +673,15 @@ static int cppi41_dma_controller_start(struct 
cppi41_dma_controller *controller)
musb_dma->status = MUSB_DMA_STATUS_FREE;
musb_dma->max_len = SZ_4M;
 
-   dc = dma_request_slave_channel(dev->parent, str);
-   if (!dc) {
-   dev_err(dev, "Failed to request %s.\n", str);
-   ret = -EPROBE_DEFER;
+   dc = dma_request_chan(dev->parent, str);
+   if (IS_ERR(dc)) {
+   ret = PTR_ERR(dc);
+   if (ret != -EPROBE_DEFER)
+   dev_err(dev, "Failed to request %s: %d.\n",
+   str, ret);
goto err;
}
+
cppi41_channel->dc = dc;
}
return 0;
-- 
2.10.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: [PATCH v3] usb: musb: musb_cppi41: Update an error message

2017-05-19 Thread Johan Hovold
On Fri, May 19, 2017 at 03:27:47PM +0200, Alexandre Bailon wrote:
> If dma_request_slave_channel() failed to return a channel,
> then the driver will print an error and request to defer probe,
> regardless of the cause of the failure.
> Defer if the DMA is not ready yet otherwise print an error.

The patch summary (Subject) doesn't really reflect what you're doing
here (only deferring probe when actually requested).

But the patch itself otherwise looks good now.

> Signed-off-by: Alexandre Bailon 

With a more descriptive summary you can add my:

Reviewed-by: Johan Hovold 

> ---
>  drivers/usb/musb/musb_cppi41.c | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
> index e7c8b1b..2c6ad93 100644
> --- a/drivers/usb/musb/musb_cppi41.c
> +++ b/drivers/usb/musb/musb_cppi41.c
> @@ -673,12 +673,14 @@ static int cppi41_dma_controller_start(struct 
> cppi41_dma_controller *controller)
>   musb_dma->status = MUSB_DMA_STATUS_FREE;
>   musb_dma->max_len = SZ_4M;
>  
> - dc = dma_request_slave_channel(dev->parent, str);
> - if (!dc) {
> - dev_err(dev, "Failed to request %s.\n", str);
> - ret = -EPROBE_DEFER;
> + dc = dma_request_chan(dev->parent, str);
> + if (IS_ERR(dc)) {
> + ret = PTR_ERR(dc);
> + if (ret != -EPROBE_DEFER)
> + dev_err(dev, "Failed to request %s.\n", str);

Perhaps also include the errno here now?

>   goto err;
>   }
> +
>   cppi41_channel->dc = dc;
>   }
>   return 0;

Thanks,
Johan
--
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: [RFC V1 1/1] net: cdc_ncm: Reduce memory use when kernel memory low

2017-05-19 Thread Bjørn Mork
David Laight  writes:

> From: linux-usb-ow...@vger.kernel.org 
> [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of Jim Baxter
>> Sent: 16 May 2017 18:41
>> 
>> The CDC-NCM driver can require large amounts of memory to create
>> skb's and this can be a problem when the memory becomes fragmented.
>> 
>> This especially affects embedded systems that have constrained
>> resources but wish to maximise the throughput of CDC-NCM with 16KiB
>> NTB's.
>
> Why is this driver copying multiple tx messages into a single skb.

Mostly becasue it already did that when I started messing with it, and I
didn't know how to avoid that.

> Surely there are better ways to do this??

But I have been there thinking this exact thought a couple of times.
Suggestions are appreciated.

> I think it is generating a 'multi-ethernet frame' URB with an
> overall header for each URB and a header for each ethernet frame.

With some negotiated alignment restrictions, and a linked list of frame
pointer arrays.  But yes, that is basically it.

(it's not always ethernet - with MBIM it can be IP or arbitrary as well,
but I don't think that makes any difference)

> Given that the USB stack allows multiple concurrent transmits I'm
> surprised that batching large ethernet frames makes much difference.

Me too.  Actually, I don't think it does.  The protocol was developed
with specific device restrictions in mind. These might be invalid today.
There is no reason to believe that using simple CDC ECM framing
(i.e. one ethernet frame per URB) is any problem.

> Also the USB target can't actually tell when URB that contain
> multiples of the USB packet size end.
> So it is possible to send a single NTB as multiple URB.

Nice idea!  Never thought of that.  Yes, the driver could use a number
smaller buffers regardless of the NTB size, by abusing the fact that the
device will see them as a contigious USB transfer as long as they fall
on USB packet boundaries.

Started thinking about how to do this in practice.  It seemed obviously
simply to jsut fire off the buffers as they fill up until the the max
aggregation size or time has been exceeded.  But the header makes this
harder than necessary.  It contains both a length and a pointer to the
first frame pointer array (NDP).  So we will have to decide the size of
the NTB and where to put the first NDP before sending the first USB
packet. This is possible if we always go for the pad-to-max strategy.
We'll also have to make some assumptions about the size of the NDP(s) as
we add them, but we already do that so I don't think it is a big deal.

Might be the way to go.

Unless someone has a nice way to just collect a list of skbs and have
them converted to proper framing on the fly when transmitting, without
having to care about USB packet boundaries.



Bjørn
--
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 6/8] xhci: remove GFP_DMA flag from allocation

2017-05-19 Thread David Laight
From: Mathias Nyman
> Sent: 19 May 2017 10:49
> To: David Laight; gre...@linuxfoundation.org
> Cc: linux-usb@vger.kernel.org; Matthias Lange; sta...@vger.kernel.org
> Subject: Re: [PATCH 6/8] xhci: remove GFP_DMA flag from allocation
> 
> On 19.05.2017 12:10, David Laight wrote:
> > From: Mathias Nyman
> >> Sent: 17 May 2017 16:32
> >> There is no reason to restrict allocations to the first 16MB ISA DMA
> >> addresses.
> >>
> >> It is causing problems in a virtualization setup with enabled IOMMU
> >> (x86_64). The result is that USB is not working in the VM.
> > ...
> >> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> >> index 12b573c..1f1687e 100644
> >> --- a/drivers/usb/host/xhci-mem.c
> >> +++ b/drivers/usb/host/xhci-mem.c
> >> @@ -56,7 +56,7 @@ static struct xhci_segment *xhci_segment_alloc(struct 
> >> xhci_hcd *xhci,
> >>}
> >>
> >>if (max_packet) {
> >> -  seg->bounce_buf = kzalloc(max_packet, flags | GFP_DMA);
> >> +  seg->bounce_buf = kzalloc(max_packet, flags);
> >
> > This might allocate memory that the device cannot access.
> > So can only work if dma_map_single() itself allocates a bounce buffer.
> > There must be a sane way to do this that doesn't ever require
> > double copies.
> >
> 
> We are using dma_map_single()
> This allocated memory is used as the processor virtual memory required by 
> dma_map_single()
> i.e. the "void *cpu_addr" part of dma_map_single, see DMA-API.txt:
> 
> 
> dma_map_single(struct device *dev, void *cpu_addr, size_t size,
>enum dma_data_direction direction)
> 
> Maps a piece of processor virtual memory so it can be accessed by the
> device and returns the DMA address of the memory.
> 
> ...
> 
> Notes:  Not all memory regions in a machine can be mapped by this API.
> Further, contiguous kernel virtual space may not be contiguous as
> physical memory.  Since this API does not provide any scatter/gather
> capability, it will fail if the user tries to map a non-physically
> contiguous piece of memory.  For this reason, memory to be mapped by
> this API should be obtained from sources which guarantee it to be
> physically contiguous (like kmalloc).
> 
> I'm not fully sure I understand yout concern, are you thinking the driver 
> should
> doublecheck the dma address returned by dma_map_single() and make sure it's 
> within the
> dma mask set for the device?

The physical memory returned by kzalloc() (without GFP_DMA) can definitely be
outside the dma mask for the device.
If that happens something must allocate a dma bounce buffer, I'm guessing that
dma_map_single() does so - it certainly can since it can copy the data during
the unmap (for a read transfer).
So you really want to allocate a buffer that is within the devices dma window.

The buffer you are allocating isn't really a 'bounce' buffer at all - is it?
It is more of a de_fragmentation buffer.
It is there so that the LINK TRB will always be at a USB segment boundary.
(I'm guessing the bug I found several years ago still isn't fixed??)

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: [REGRESSION] Failed network caused by: xhci: switch to pci_alloc_irq_vectors

2017-05-19 Thread Steven Rostedt
On Fri, 19 May 2017 14:46:25 +0200
Christoph Hellwig  wrote:

> On Fri, May 19, 2017 at 08:37:21AM -0400, Steven Rostedt wrote:
> > ktest config bisect ended with:
> > 
> > ***
> > Found bad config: CONFIG_PCI_MSI
> > ***  
> 
> Oh, that's interesting.  I think there's been a bug in the !CONFIG_PCI_MSI
> fallback for pci_alloc_irq_vectors since the very beginning.  And it
> didn't matter for any driver so far, but xhci has a very odd way
> to set MSI(-X) vs legacy interrupts.
> 
> Can you try the patch below?

Works. Thanks!

Tested-by: Steven Rostedt (VMware) 

-- Steve

> 
> 
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 33c2b0b77429..5a7fd3b6a7b9 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1342,7 +1342,7 @@ pci_alloc_irq_vectors_affinity(struct pci_dev *dev, 
> unsigned int min_vecs,
>  unsigned int max_vecs, unsigned int flags,
>  const struct irq_affinity *aff_desc)
>  {
> - if (min_vecs > 1)
> + if (min_vecs > 1 || !(flags & PCI_IRQ_LEGACY))
>   return -EINVAL;
>   return 1;
>  }

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


[RESEND PATCH v5] ARM: dts: da850: Add the CPPI 4.1 DMA to the USB OTG controller

2017-05-19 Thread Alexandre Bailon
This adds the CPPI 4.1 DMA controller to the USB OTG controller.

Changes since v2:
- Fixed the the property reg-names (had glue register defined)
- Removed few useless property

Signed-off-by: Alexandre Bailon 
---
 arch/arm/boot/dts/da850.dtsi | 27 ++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index 941d455..0bd048e 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -446,13 +446,38 @@
};
usb0: usb@20 {
compatible = "ti,da830-musb";
-   reg = <0x20 0x1>;
+   reg = <0x20 0x1000>;
+   ranges;
interrupts = <58>;
interrupt-names = "mc";
dr_mode = "otg";
phys = <_phy 0>;
phy-names = "usb-phy";
status = "disabled";
+
+   #address-cells = <1>;
+   #size-cells = <1>;
+
+   dmas = < 0 0  1 0
+2 0  3 0
+0 1  1 1
+2 1  3 1>;
+   dma-names =
+   "rx1", "rx2", "rx3", "rx4",
+   "tx1", "tx2", "tx3", "tx4";
+
+   cppi41dma: dma-controller@201000 {
+   compatible = "ti,da830-cppi41";
+   reg =  <0x201000 0x1000
+   0x202000 0x1000
+   0x204000 0x4000>;
+   reg-names = "controller",
+   "scheduler", "queuemgr";
+   interrupts = <58>;
+   #dma-cells = <2>;
+   #dma-channels = <4>;
+   status = "okay";
+   };
};
sata: sata@218000 {
compatible = "ti,da850-ahci";
-- 
2.10.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: [PATCH] usb: typec: Defer checking of valid power role swap to low level drivers

2017-05-19 Thread Guenter Roeck

On 05/19/2017 03:35 AM, Heikki Krogerus wrote:

On Thu, May 18, 2017 at 02:08:53PM -0700, Badhri Jagan Sridharan wrote:

On Thu, May 18, 2017 at 9:51 AM, Guenter Roeck  wrote:

On Thu, May 18, 2017 at 11:13:51AM +0200, Oliver Neukum wrote:

Am Mittwoch, den 17.05.2017, 02:36 -0700 schrieb Guenter Roeck:

On 05/17/2017 12:34 AM, Oliver Neukum wrote:


Am Mittwoch, den 17.05.2017, 00:32 -0700 schrieb Badhri Jagan
Sridharan:

Hi,



"Two independent set of mechanisms are defined to allow a USB Type-C
DRP to functionally swap power and data roles. When USB PD is
supported, power and data role swapping is performed as a subsequent
step following the initial connection process. For non-PD implementations,
power/data role swapping can optionally be dealt with as part of the initial
connection process."


Well, as I read it, without PD once a connection is established, you
are stuck with your role. So it seems to me that blocking a later
attempt to change it makes sense.



That seems to be a harsh and not very user friendly reading of the 
specification.

I would argue that the user doesn't care if the partner supports PD or not
when selecting a role, and I would prefer to provide an implementation which is
as user friendly as possible.


Data role, no question, you are right.
Power role is a different question. A switch of power role with PD should
not lead to a disconnect. Any other method might. So equating them does
not look like a good idea.



Not really sure I can follow. If a partner does not support PD, there is no
real distinction between data role and power role, or am I missing something ?

Are you saying that, if a partner does not support PD, user space should
request a data role swap instead, and that this would be acceptable for you ?

I don't really understand the difference - a data role swap doesn't cause
a disconnect either if the partner supports PD, and it would still result
in a disconnect/reconnect sequence if the partner does not support PD -
but if it works for you, fine with me.

Badhri, would that work for us ?


Yes Geunter that should work as well. Requesting non-pd role swap either through
current_power_role or current_data_role is virtually the same.


So if I understood this correctly, we'll skip this change, right?


Yes.

Thanks,
Guenter
--
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 v3] usb: musb: musb_cppi41: Update an error message

2017-05-19 Thread Alexandre Bailon
If dma_request_slave_channel() failed to return a channel,
then the driver will print an error and request to defer probe,
regardless of the cause of the failure.
Defer if the DMA is not ready yet otherwise print an error.

Signed-off-by: Alexandre Bailon 
---
 drivers/usb/musb/musb_cppi41.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
index e7c8b1b..2c6ad93 100644
--- a/drivers/usb/musb/musb_cppi41.c
+++ b/drivers/usb/musb/musb_cppi41.c
@@ -673,12 +673,14 @@ static int cppi41_dma_controller_start(struct 
cppi41_dma_controller *controller)
musb_dma->status = MUSB_DMA_STATUS_FREE;
musb_dma->max_len = SZ_4M;
 
-   dc = dma_request_slave_channel(dev->parent, str);
-   if (!dc) {
-   dev_err(dev, "Failed to request %s.\n", str);
-   ret = -EPROBE_DEFER;
+   dc = dma_request_chan(dev->parent, str);
+   if (IS_ERR(dc)) {
+   ret = PTR_ERR(dc);
+   if (ret != -EPROBE_DEFER)
+   dev_err(dev, "Failed to request %s.\n", str);
goto err;
}
+
cppi41_channel->dc = dc;
}
return 0;
-- 
2.10.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


drivers/usb/host/xhci-ring.c:1390 handle_cmd_completion

2017-05-19 Thread Jason A. Donenfeld
I'm having this issue on kernel 4.11.0 and 4.11.1. It usually happens
after a while of ordinary USB use. Afterwards, USB does not work. If I
rmmod all the modules and reinsert them, it works for a while, but
even sooner after it stops working again.

[48112.422418] [ cut here ]
[48112.422441] WARNING: CPU: 0 PID: 14420 at
drivers/usb/host/xhci-ring.c:1390 handle_cmd_completion+0xb17/0xc00
[xhci_hcd]
[48112.422446] Modules linked in: xt_hashlimit ip6_udp_tunnel
udp_tunnel rfcomm pl2303 hid_lenovo bnep cdc_mbim cdc_ncm qcserial
cdc_wdm usb_wwan usbnet usbserial mii uvcvideo videobuf2_vmalloc
videobuf2_memops
[48112.422480] xhci_hcd :00:14.0: Timeout while waiting for setup
device command
[48112.422481] videobuf2_v4l2 videobuf2_core cdc_acm videodev btusb
btintel usbhid bluetooth af_packet nf_conntrack_ipv4 nf_defrag_ipv4
iptable_filter xt_hl nf_conntrack_ipv6 nf_defrag_ipv6 xt_multiport
8021q xt_conntrack nf_conntrack ip6table_filter ip6_tables
algif_skcipher joydev mousedev snd_hda_codec_realtek
snd_hda_codec_generic arc4 iwlmvm mac80211 rtsx_pci_sdmmc mmc_core
intel_rapl iosf_mbi x86_pkg_temp_thermal intel_powerclamp coretemp
kvm_intel kvm iwlwifi snd_hda_intel ahci irqbypass snd_hda_codec
crc32_pclmul snd_hwdep xhci_pci xhci_hcd libahci crc32c_intel
snd_hda_core mei_me cfg80211 usbcore snd_pcm rtsx_pci ie31200_edac
input_leds mfd_core e1000e libata usb_common mei snd_timer psmouse
edac_core intel_pch_thermal thinkpad_acpi snd soundcore led_class
rfkill tpm_tis tpm_tis_core evdev
[48112.422556] tpm sch_fq_codel
[48112.422565] CPU: 0 PID: 14420 Comm: kworker/0:7 Tainted: P W O
4.11.1-gentoo #1
[48112.422567] Hardware name: LENOVO 20ENCTO1WW/20ENCTO1WW, BIOS
N1EET65W (1.38 ) 02/09/2017
[48112.422577] Workqueue: events xhci_handle_command_timeout [xhci_hcd]
[48112.422580] Call Trace:
[48112.422583] 
[48112.422589] ? dump_stack+0x46/0x5e
[48112.422595] ? __warn+0xb9/0xe0
[48112.422603] ? handle_cmd_completion+0xb17/0xc00 [xhci_hcd]
[48112.422609] ? try_to_wake_up+0x22e/0x390
[48112.422617] ? xhci_irq+0x38f/0x1460 [xhci_hcd]
[48112.422624] ? run_timer_softirq.part.2+0x4c/0xa0
[48112.422629] ? expire_timers+0x6e/0xe0
[48112.422634] ? __handle_irq_event_percpu+0x36/0x190
[48112.422637] ? handle_irq_event_percpu+0x1b/0x50
[48112.422640] ? handle_irq_event+0x22/0x40
[48112.422644] ? handle_edge_irq+0x65/0x120
[48112.422649] ? handle_irq+0x11/0x20
[48112.422653] ? do_IRQ+0x3c/0xc0
[48112.422658] ? common_interrupt+0x7f/0x7f
[48112.422660] 
[48112.422664] ? _raw_spin_unlock_irqrestore+0x5/0x10
[48112.422671] ? xhci_handle_command_timeout+0xf4/0x1b0 [xhci_hcd]
[48112.422684] ? process_one_work+0x1d9/0x450
[48112.422689] ? worker_thread+0x42/0x4b0
[48112.422695] ? process_one_work+0x450/0x450
[48112.422698] ? kthread+0x112/0x130
[48112.422702] ? kthread_create_on_node+0x40/0x40
[48112.422705] ? ret_from_fork+0x23/0x30
[48112.422709] ---[ end trace eb9505885b6e349e ]---
[48113.446247] xhci_hcd :00:14.0: xHCI host not responding to stop
endpoint command.
[48113.446250] xhci_hcd :00:14.0: Assuming host is dying, halting host.
[48113.446348] xhci_hcd :00:14.0: HC died; cleaning up
--
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: [REGRESSION] Failed network caused by: xhci: switch to pci_alloc_irq_vectors

2017-05-19 Thread Christoph Hellwig
On Fri, May 19, 2017 at 08:37:21AM -0400, Steven Rostedt wrote:
> ktest config bisect ended with:
> 
> ***
> Found bad config: CONFIG_PCI_MSI
> ***

Oh, that's interesting.  I think there's been a bug in the !CONFIG_PCI_MSI
fallback for pci_alloc_irq_vectors since the very beginning.  And it
didn't matter for any driver so far, but xhci has a very odd way
to set MSI(-X) vs legacy interrupts.

Can you try the patch below?


diff --git a/include/linux/pci.h b/include/linux/pci.h
index 33c2b0b77429..5a7fd3b6a7b9 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1342,7 +1342,7 @@ pci_alloc_irq_vectors_affinity(struct pci_dev *dev, 
unsigned int min_vecs,
   unsigned int max_vecs, unsigned int flags,
   const struct irq_affinity *aff_desc)
 {
-   if (min_vecs > 1)
+   if (min_vecs > 1 || !(flags & PCI_IRQ_LEGACY))
return -EINVAL;
return 1;
 }
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [REGRESSION] Failed network caused by: xhci: switch to pci_alloc_irq_vectors

2017-05-19 Thread Steven Rostedt
On Fri, 19 May 2017 06:08:56 -0400
Steven Rostedt  wrote:

> > But other configs on this same hardware work, can you do a diff of a
> > working vs. not working?  
> 
> I could probably run my config-bisect and see what it comes up with.

ktest config bisect ended with:

***
Found bad config: CONFIG_PCI_MSI
***

diffconfig good_config bad_config
-ENA_ETHERNET n
-FM10K n
-GENERIC_MSI_IRQ y
-GENERIC_MSI_IRQ_DOMAIN y
-I40EVF n
-INTEL_IOMMU n
-IRQ_REMAP n
-IXGBEVF n
-LIQUIDIO_VF n
-NFP n
-PCIE_DW_PLAT n
-PCI_MSI_IRQ_DOMAIN y
-VMD n
 PCI_MSI y -> n


When that is not set, it fails to boot. It boots fine if I enable it.

-- Steve
--
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: [RFC V1 1/1] net: cdc_ncm: Reduce memory use when kernel memory low

2017-05-19 Thread David Laight
From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-ow...@vger.kernel.org] 
On Behalf Of Jim Baxter
> Sent: 16 May 2017 18:41
> 
> The CDC-NCM driver can require large amounts of memory to create
> skb's and this can be a problem when the memory becomes fragmented.
> 
> This especially affects embedded systems that have constrained
> resources but wish to maximise the throughput of CDC-NCM with 16KiB
> NTB's.

Why is this driver copying multiple tx messages into a single skb.
Surely there are better ways to do this??

I think it is generating a 'multi-ethernet frame' URB with an
overall header for each URB and a header for each ethernet frame.

Given that the USB stack allows multiple concurrent transmits I'm
surprised that batching large ethernet frames makes much difference.

Also the USB target can't actually tell when URB that contain
multiples of the USB packet size end.
So it is possible to send a single NTB as multiple URB.
Of course, the usb_net might have other ideas.

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: [PATCH] usb: typec: Defer checking of valid power role swap to low level drivers

2017-05-19 Thread Oliver Neukum
Am Donnerstag, den 18.05.2017, 14:08 -0700 schrieb Badhri Jagan
Sridharan:
> > Badhri, would that work for us ?
> 
> Yes Geunter that should work as well. Requesting non-pd role swap either 
> through
> current_power_role or current_data_role is virtually the same.

Yes and that is the issue.
If you can do PD and swap power roles, power roles will be swapped
and data connection and driver assignments stay put. However without
PD drivers will be unbound. That means that user space must be made
aware that requesting a power role change will have side effects.

Regards
Oliver

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


Re: [PATCH] usb: typec: Defer checking of valid power role swap to low level drivers

2017-05-19 Thread Heikki Krogerus
On Thu, May 18, 2017 at 02:08:53PM -0700, Badhri Jagan Sridharan wrote:
> On Thu, May 18, 2017 at 9:51 AM, Guenter Roeck  wrote:
> > On Thu, May 18, 2017 at 11:13:51AM +0200, Oliver Neukum wrote:
> >> Am Mittwoch, den 17.05.2017, 02:36 -0700 schrieb Guenter Roeck:
> >> > On 05/17/2017 12:34 AM, Oliver Neukum wrote:
> >> > >
> >> > > Am Mittwoch, den 17.05.2017, 00:32 -0700 schrieb Badhri Jagan
> >> > > Sridharan:
> >> > >
> >> > > Hi,
> >> > >
> >> > > >
> >> > > > "Two independent set of mechanisms are defined to allow a USB Type-C
> >> > > > DRP to functionally swap power and data roles. When USB PD is
> >> > > > supported, power and data role swapping is performed as a subsequent
> >> > > > step following the initial connection process. For non-PD 
> >> > > > implementations,
> >> > > > power/data role swapping can optionally be dealt with as part of the 
> >> > > > initial
> >> > > > connection process."
> >> > >
> >> > > Well, as I read it, without PD once a connection is established, you
> >> > > are stuck with your role. So it seems to me that blocking a later
> >> > > attempt to change it makes sense.
> >> > >
> >> >
> >> > That seems to be a harsh and not very user friendly reading of the 
> >> > specification.
> >> >
> >> > I would argue that the user doesn't care if the partner supports PD or 
> >> > not
> >> > when selecting a role, and I would prefer to provide an implementation 
> >> > which is
> >> > as user friendly as possible.
> >>
> >> Data role, no question, you are right.
> >> Power role is a different question. A switch of power role with PD should
> >> not lead to a disconnect. Any other method might. So equating them does
> >> not look like a good idea.
> >>
> >
> > Not really sure I can follow. If a partner does not support PD, there is no
> > real distinction between data role and power role, or am I missing 
> > something ?
> >
> > Are you saying that, if a partner does not support PD, user space should
> > request a data role swap instead, and that this would be acceptable for you 
> > ?
> >
> > I don't really understand the difference - a data role swap doesn't cause
> > a disconnect either if the partner supports PD, and it would still result
> > in a disconnect/reconnect sequence if the partner does not support PD -
> > but if it works for you, fine with me.
> >
> > Badhri, would that work for us ?
> 
> Yes Geunter that should work as well. Requesting non-pd role swap either 
> through
> current_power_role or current_data_role is virtually the same.

So if I understood this correctly, we'll skip this change, right?


Thanks,

-- 
heikki
--
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: [REGRESSION] Failed network caused by: xhci: switch to pci_alloc_irq_vectors

2017-05-19 Thread Steven Rostedt
On Fri, 19 May 2017 07:42:23 +0200
Greg Kroah-Hartman  wrote:

> On Thu, May 18, 2017 at 11:42:34PM -0400, Steven Rostedt wrote:
> > 
> > One of my the configs I use to test ftrace with (configs that have
> > caused failures in the past), has lots of irq issues and fails to
> > initialize the network of my box. I bisected the problem down to a
> > single commit, and when I revert that commit, my box boots without any
> > network or irq issues.
> > 
> > Note, my other configs work fine on this box. I haven't investigated
> > which config is also the culprit. But since it use to work with this
> > config, I want to report it.  
> 
> So what commit is causing the problem?

Ug, I forgot to cut and paste the sha1. I thought I did, but I only cut
and pasted the subject into the subject of this email.

commit 77d45b4500967de674b8f75a9a91f58d57d5704d

> 
> It looks like the ehci driver is having problems, but first, your
> interrupts are whack:

Could be. It's an old board.

> 
> >  irq 16: nobody cared (try booting with the "irqpoll" option)
> >  CPU: 2 PID: 0 Comm: swapper/2 Not tainted 4.12.0-rc1-test-dirty #24
> >  Hardware name: MSI MS-7823/CSM-H87M-G43 (MS-7823), BIOS V1.6 02/22/2014
> >  Call Trace:
> >   
> >  devtmpfs: mounted
> >   dump_stack+0x9a/0xd6
> >   __report_bad_irq+0x35/0xc0
> >   note_interrupt+0x234/0x270
> >   handle_irq_event_percpu+0x45/0x60
> >   handle_irq_event+0x39/0x60
> >   handle_fasteoi_irq+0x8f/0x160
> >   handle_irq+0x6f/0x110
> >   do_IRQ+0x46/0xd0
> >   common_interrupt+0x93/0x93
> >  RIP: 0010:native_safe_halt+0x6/0x10
> >  RSP: :b54240cd7e90 EFLAGS: 0286 ORIG_RAX: ff7e
> >  RAX:  RBX: 8ea214498040 RCX: 
> >  RDX:  RSI:  RDI: 
> >  RBP: b54240cd7e90 R08: 0001 R09: 41129b0c
> >  R10: b54240cd7d68 R11: 0001 R12: 0002
> >  R13: 8ea214498040 R14:  R15: 8ea214498040
> >   
> >   default_idle+0x38/0x160
> >   arch_cpu_idle+0xf/0x20
> >   default_idle_call+0x28/0x50
> >   do_idle+0x182/0x220
> >   cpu_startup_entry+0x1d/0x20
> >   start_secondary+0x132/0x160
> >   secondary_startup_64+0x9f/0x9f
> >  handlers:
> >  [] xhci_msi_irq
> >  Disabling IRQ #16  
> 
> Have you tried taking the kernel's advice?  :)

You mean the "irqpoll"?  No. This works fine without that commit. Why
should I have to change?

> 
> >  ehci-pci :00:1a.0: new USB bus registered, assigned bus number 3
> >  ehci-pci :00:1a.0: debug port 2
> >  ehci-pci :00:1a.0: cache line size of 64 is not supported
> >  genirq: Flags mismatch irq 16. 0080 (ehci_hcd:usb3) vs.  
> > (xhci_hcd)  
> 
> What does that mean?

No idea ;-)

> 
> >  CPU: 0 PID: 307 Comm: modprobe Tainted: GE   
> > 4.12.0-rc1-test-dirty #24
> >  Hardware name: MSI MS-7823/CSM-H87M-G43 (MS-7823), BIOS V1.6 02/22/2014
> >  Call Trace:
> >   dump_stack+0x9a/0xd6
> >   __setup_irq+0x5d4/0x630
> >   request_threaded_irq+0x10d/0x190
> >   usb_add_hcd+0x658/0x970
> >   ? for_each_companion+0x3e/0xb0
> >   usb_hcd_pci_probe+0x3e4/0x490
> >   ehci_pci_probe+0x36/0x40 [ehci_pci]
> >   local_pci_probe+0x45/0xa0
> >   ? pci_match_device+0xca/0x110
> >   pci_device_probe+0xdb/0x130
> >   driver_probe_device+0x2ed/0x480
> >   __driver_attach+0xd5/0x100
> >   ? driver_probe_device+0x480/0x480
> >   bus_for_each_dev+0x62/0xa0
> >   driver_attach+0x1e/0x20
> >   bus_add_driver+0x1c6/0x290
> >   driver_register+0x60/0xe0
> >   __pci_register_driver+0x60/0x70
> >   ? 0xc0346000
> >   ehci_pci_init+0x6a/0x1000 [ehci_pci]
> >   do_one_initcall+0x43/0x190
> >   ? kmem_cache_alloc_trace+0x1be/0x200
> >   do_init_module+0x7d/0x210
> >   load_module+0x1891/0x1eb0
> >   ? vmap_page_range_noflush+0x29b/0x370
> >   ? show_coresize+0x30/0x30
> >   SYSC_init_module+0x143/0x180
> >   ? load_module+0x5/0x1eb0
> >   ? SYSC_init_module+0x143/0x180
> >   SyS_init_module+0xe/0x10
> >   entry_SYSCALL_64_fastpath+0x23/0xc2
> >  RIP: 0033:0x3b918e0ffa
> >  RSP: 002b:7ffd11d575c8 EFLAGS: 0246 ORIG_RAX: 00af
> >  RAX: ffda RBX: 0061f950 RCX: 003b918e0ffa
> >  RDX: 0061f7d0 RSI: 36b0 RDI: 0062c9e0
> >  RBP:  R08: 00630090 R09: 7f019c07c700
> >  R10: 7ffd11d574f0 R11: 0246 R12: 00626200
> >  R13: 0061f930 R14:  R15: 0061f420
> >  ehci-pci :00:1a.0: request interrupt 16 failed  
> 
> So ehci can't use the same irq line as xhci?  No sharing allowed?
> 
> But other configs on this same hardware work, can you do a diff of a
> working vs. not working?

I could probably run my config-bisect and see what it comes up with.

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

Re: [PATCH 6/8] xhci: remove GFP_DMA flag from allocation

2017-05-19 Thread Mathias Nyman

On 19.05.2017 12:10, David Laight wrote:

From: Mathias Nyman

Sent: 17 May 2017 16:32
There is no reason to restrict allocations to the first 16MB ISA DMA
addresses.

It is causing problems in a virtualization setup with enabled IOMMU
(x86_64). The result is that USB is not working in the VM.

...

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 12b573c..1f1687e 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -56,7 +56,7 @@ static struct xhci_segment *xhci_segment_alloc(struct 
xhci_hcd *xhci,
}

if (max_packet) {
-   seg->bounce_buf = kzalloc(max_packet, flags | GFP_DMA);
+   seg->bounce_buf = kzalloc(max_packet, flags);


This might allocate memory that the device cannot access.
So can only work if dma_map_single() itself allocates a bounce buffer.
There must be a sane way to do this that doesn't ever require
double copies.



We are using dma_map_single()
This allocated memory is used as the processor virtual memory required by 
dma_map_single()
i.e. the "void *cpu_addr" part of dma_map_single, see DMA-API.txt:


dma_map_single(struct device *dev, void *cpu_addr, size_t size,
  enum dma_data_direction direction)

Maps a piece of processor virtual memory so it can be accessed by the
device and returns the DMA address of the memory.

...

Notes:  Not all memory regions in a machine can be mapped by this API.
Further, contiguous kernel virtual space may not be contiguous as
physical memory.  Since this API does not provide any scatter/gather
capability, it will fail if the user tries to map a non-physically
contiguous piece of memory.  For this reason, memory to be mapped by
this API should be obtained from sources which guarantee it to be
physically contiguous (like kmalloc).

I'm not fully sure I understand yout concern, are you thinking the driver should
doublecheck the dma address returned by dma_map_single() and make sure it's 
within the
dma mask set for the device?

-Mathias

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


RE: [PATCH 6/8] xhci: remove GFP_DMA flag from allocation

2017-05-19 Thread David Laight
From: Mathias Nyman
> Sent: 17 May 2017 16:32
> There is no reason to restrict allocations to the first 16MB ISA DMA
> addresses.
> 
> It is causing problems in a virtualization setup with enabled IOMMU
> (x86_64). The result is that USB is not working in the VM.
...
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index 12b573c..1f1687e 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -56,7 +56,7 @@ static struct xhci_segment *xhci_segment_alloc(struct 
> xhci_hcd *xhci,
>   }
> 
>   if (max_packet) {
> - seg->bounce_buf = kzalloc(max_packet, flags | GFP_DMA);
> + seg->bounce_buf = kzalloc(max_packet, flags);

This might allocate memory that the device cannot access.
So can only work if dma_map_single() itself allocates a bounce buffer.
There must be a sane way to do this that doesn't ever require
double copies.

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


[PATCH] uwb: i1480: add missing goto

2017-05-19 Thread Gustavo A. R. Silva
Add missing goto.

Addresses-Coverity-ID: 1226913
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/uwb/i1480/dfu/phy.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/uwb/i1480/dfu/phy.c b/drivers/uwb/i1480/dfu/phy.c
index 3b1a87d..1ac8526 100644
--- a/drivers/uwb/i1480/dfu/phy.c
+++ b/drivers/uwb/i1480/dfu/phy.c
@@ -126,6 +126,7 @@ int i1480_mpi_read(struct i1480 *i1480, u8 *data, u16 
srcaddr, size_t size)
dev_err(i1480->dev, "MPI-READ: command execution failed: %d\n",
reply->bResultCode);
result = -EIO;
+   goto out;
}
for (cnt = 0; cnt < size; cnt++) {
if (reply->data[cnt].page != (srcaddr + cnt) >> 8)
-- 
2.5.0

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


Re: [uwb-i1480] question about value overwrite

2017-05-19 Thread Gustavo A. R. Silva

Hi Greg,

Quoting Greg KH :


On Thu, May 18, 2017 at 06:00:06PM -0500, Gustavo A. R. Silva wrote:


Hello everybody,

While looking into Coverity ID 1226913 I ran into the following piece of
code at drivers/uwb/i1480/dfu/phy.c:99:

 99static
100int i1480_mpi_read(struct i1480 *i1480, u8 *data, u16 srcaddr,  
size_t size)

101{
102int result;
103struct i1480_cmd_mpi_read *cmd = i1480->cmd_buf;
104struct i1480_evt_mpi_read *reply = i1480->evt_buf;
105unsigned cnt;
106
107memset(i1480->cmd_buf, 0x69, 512);
108memset(i1480->evt_buf, 0x69, 512);
109
110BUG_ON(size > (i1480->buf_size - sizeof(*reply)) / 3);
111result = -ENOMEM;
112cmd->rccb.bCommandType = i1480_CET_VS1;
113cmd->rccb.wCommand = cpu_to_le16(i1480_CMD_MPI_READ);
114cmd->size = cpu_to_le16(3*size);
115for (cnt = 0; cnt < size; cnt++) {
116cmd->data[cnt].page = (srcaddr + cnt) >> 8;
117cmd->data[cnt].offset = (srcaddr + cnt) & 0xff;
118}
119reply->rceb.bEventType = i1480_CET_VS1;
120reply->rceb.wEvent = i1480_CMD_MPI_READ;
121result = i1480_cmd(i1480, "MPI-READ", sizeof(*cmd) + 2*size,
122sizeof(*reply) + 3*size);
123if (result < 0)
124goto out;
125if (reply->bResultCode != UWB_RC_RES_SUCCESS) {
126dev_err(i1480->dev, "MPI-READ: command execution failed:
%d\n",
127reply->bResultCode);
128result = -EIO;
129}
130for (cnt = 0; cnt < size; cnt++) {
131if (reply->data[cnt].page != (srcaddr + cnt) >> 8)
132dev_err(i1480->dev, "MPI-READ: page inconsistency
at "
133"index %u: expected 0x%02x, got
0x%02x\n", cnt,
134(srcaddr + cnt) >> 8,
reply->data[cnt].page);
135if (reply->data[cnt].offset != ((srcaddr + cnt)  
& 0x00ff))

136dev_err(i1480->dev, "MPI-READ: offset
inconsistency at "
137"index %u: expected 0x%02x, got
0x%02x\n", cnt,
138(srcaddr + cnt) & 0x00ff,
139reply->data[cnt].offset);
140data[cnt] = reply->data[cnt].value;
141}
142result = 0;
143out:
144return result;
145}

The issue is that the value store in variable _result_ at line 128 is
overwritten by the one stored at line 142, before it can be used.

My question is if the original intention was to return this value
inmediately after the assignment at line 128, something like in the
following patch:

index 3b1a87d..1ac8526 100644
--- a/drivers/uwb/i1480/dfu/phy.c
+++ b/drivers/uwb/i1480/dfu/phy.c
@@ -126,6 +126,7 @@ int i1480_mpi_read(struct i1480 *i1480, u8 *data, u16
srcaddr, size_t size)
dev_err(i1480->dev, "MPI-READ: command execution failed:
%d\n",
reply->bResultCode);
result = -EIO;
+   goto out;
}
for (cnt = 0; cnt < size; cnt++) {
if (reply->data[cnt].page != (srcaddr + cnt) >> 8)

What do you think?

I'd really appreciate any comment on this.


I think you are correct, I'll take a patch to fix this up if you want to
write one :)



Absolutely, I'll send it shortly.

Thanks!
--
Gustavo A. R. Silva






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


[PATCH] usb: chipidea: core: check before accessing ci_role in ci_role_show

2017-05-19 Thread Michael Thalmeier
ci_role BUGs when the role is >= CI_ROLE_END.
This is the case while the role is changing.

Signed-off-by: Michael Thalmeier 
---
 drivers/usb/chipidea/core.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 9e217b1..fe4fe24 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -843,7 +843,10 @@ static ssize_t ci_role_show(struct device *dev, struct 
device_attribute *attr,
 {
struct ci_hdrc *ci = dev_get_drvdata(dev);
 
-   return sprintf(buf, "%s\n", ci_role(ci)->name);
+   if (ci->role != CI_ROLE_END)
+   return sprintf(buf, "%s\n", ci_role(ci)->name);
+
+   return 0;
 }
 
 static ssize_t ci_role_store(struct device *dev,
-- 
2.9.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: [PATCH] usb: chipidea: debug: check before accessing ci_role

2017-05-19 Thread Michael Thalmeier
On Fri, May 19, 2017 at 09:15:40AM +0800, Peter Chen wrote:
> On Thu, May 18, 2017 at 04:14:14PM +0200, Michael Thalmeier wrote:
> > ci_role BUGs when the role is >= CI_ROLE_END.
> > 
> > Signed-off-by: Michael Thalmeier 
> > ---
> >  drivers/usb/chipidea/debug.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/usb/chipidea/debug.c b/drivers/usb/chipidea/debug.c
> > index 6d23eed..1c31e8a 100644
> > --- a/drivers/usb/chipidea/debug.c
> > +++ b/drivers/usb/chipidea/debug.c
> > @@ -294,7 +294,8 @@ static int ci_role_show(struct seq_file *s, void *data)
> >  {
> > struct ci_hdrc *ci = s->private;
> >  
> > -   seq_printf(s, "%s\n", ci_role(ci)->name);
> > +   if (ci->role != CI_ROLE_END)
> > +   seq_printf(s, "%s\n", ci_role(ci)->name);
> >  
> > return 0;
> >  }
> 
> By the way, how can you trigger this issue?

It is quite easy to trigger.
I have an USB OTG adapter cable connected to the micro-USB port of my device.
When the cable is connected, the port has the "host" role. When
disconnected, the role changes to "gadget".

When polling (1 second interval) the role file while
connecting/disconnecting the USB OTG cable, the error occurs within
the first few tries.

> Do you mind sending another patch to fix the same issue for ci_role_show
> at core.c?

I did not see, that there is now another place this is used. Will send
a patch later.

> 
> -- 
> 
> Best Regards,
> Peter Chen

Regards,
Michael
--
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: [GIT PULL] USB-serial fixes for v4.12-rc2

2017-05-19 Thread Greg Kroah-Hartman
On Fri, May 19, 2017 at 09:52:23AM +0200, Johan Hovold wrote:
> The following changes since commit 2ea659a9ef488125eb46da6eb571de5eae5c43f6:
> 
>   Linux 4.12-rc1 (2017-05-13 13:19:49 -0700)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git 
> tags/usb-serial-4.12-rc2

pulled and pushed out, thanks.

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


Re: uas failing on multiple disk access on a jmicron JMS567 bridge

2017-05-19 Thread Mathias Nyman

On 18.05.2017 17:30, Christoph Gohle wrote:

Hi,

I have seen several reports around the internet regarding failing io on 
USB-SATA bridges. However, these reports seem to be partially old and/or fixes 
proposed are implemented in my kernel but don’t fix things. Therefore I thought 
I report here again. If this is know/duplicate please apologise.

I am running on ubuntu 16.04 LTS with kernel

$ ~> uname -a
Linux gserv 4.8.0-52-generic #55~16.04.1-Ubuntu SMP Fri Apr 28 14:36:29 UTC 
2017 x86_64 x86_64 x86_64 GNU/Linux

I was also using the 4.4.0-75-generic version of the kernel before, with same 
results. I am having a JMS567 bridge with (currently) three disks attached.

$ ~> lsusb
Bus 002 Device 002: ID 152d:0567 JMicron Technology Corp. / JMicron USA 
Technology Corp. JMS567 SATA 6Gb/s bridge
Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
…

$ ~> ls -l /dev/disk/by-id/
total 0
…
lrwxrwxrwx 1 root root  9 May 18 00:20 
usb-JMicron_Generic_DISK00_0123456789ABCDEF-0:0 -> ../../sdc
lrwxrwxrwx 1 root root 10 May 18 00:20 
usb-JMicron_Generic_DISK00_0123456789ABCDEF-0:0-part1 -> ../../sdc1
lrwxrwxrwx 1 root root 10 May 18 00:20 
usb-JMicron_Generic_DISK00_0123456789ABCDEF-0:0-part9 -> ../../sdc9
lrwxrwxrwx 1 root root  9 May 18 00:20 
usb-JMicron_Generic_DISK01_0123456789ABCDEF-0:1 -> ../../sdd
lrwxrwxrwx 1 root root 10 May 18 00:20 
usb-JMicron_Generic_DISK01_0123456789ABCDEF-0:1-part1 -> ../../sdd1
lrwxrwxrwx 1 root root  9 May 18 00:20 
usb-JMicron_Generic_DISK02_0123456789ABCDEF-0:2 -> ../../sde
lrwxrwxrwx 1 root root 10 May 18 00:20 
usb-JMicron_Generic_DISK02_0123456789ABCDEF-0:2-part1 -> ../../sde1
lrwxrwxrwx 1 root root 10 May 18 00:20 
usb-JMicron_Generic_DISK02_0123456789ABCDEF-0:2-part9 -> ../../sde9
…

I am running a ZFS filesystem on those… (actually two pools…). The system seems 
to run fine as long as there is only reading going on or only writing to one 
disk (possibly from somewhere else)….

However once I start a copy operation (here data coming from one of the above 
devices and going to the two others (in a mirror configuration), I get frequent 
io errors from the kernel:

May 17 22:53:13 gserv kernel: [  474.505548] xhci_hcd :00:14.0: ERROR 
Transfer event for disabled endpoint or incorrect stream ring
May 17 22:53:13 gserv kernel: [  474.505670] xhci_hcd :00:14.0: 
@00026e54c460   0400 02088001


4.11 kernel has xhci traces enabled, could you try to reproduce it with 4.11?
xhci traces can be enabled with:

echo 1 > /sys/kernel/debug/tracing/events/xhci-hcd/enable

If you know how to reliably reproduce this then please enable tracing just 
before
triggering this. It generates a lot of data.

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


[GIT PULL] USB-serial fixes for v4.12-rc2

2017-05-19 Thread Johan Hovold
The following changes since commit 2ea659a9ef488125eb46da6eb571de5eae5c43f6:

  Linux 4.12-rc1 (2017-05-13 13:19:49 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git 
tags/usb-serial-4.12-rc2

for you to fetch changes up to 8d7a10dd323993cc40bd37bce8bc570133b0c396:

  USB: serial: qcserial: add more Lenovo EM74xx device IDs (2017-05-17 17:10:57 
+0200)


USB-serial fixes for v4.12-rc2

Here's a fix for a long-standing issue in the ftdi_sio driver that
prevented unprivileged users from updating the low-latency flag,
something which became apparent after a recent change that restored the
older setting of not using low-latency mode by default.

A run of sparse revealed a couple of endianness issues that are now
fixed, and addressed is also a user-triggerable division-by-zero in
io_ti when debugging is enabled.

Finally there are some new device ids, including a simplification of how
we deal with a couple of older Olimex JTAG adapters.

All have been in linux-next with no reported issues.

Signed-off-by: Johan Hovold 


Andrey Korolyov (1):
  USB: serial: ftdi_sio: add Olimex ARM-USB-TINY(H) PIDs

Anthony Mallet (1):
  USB: serial: ftdi_sio: fix setting latency for unprivileged users

Bjørn Mork (1):
  USB: serial: qcserial: add more Lenovo EM74xx device IDs

Daniele Palmas (1):
  usb: serial: option: add Telit ME910 support

Johan Hovold (3):
  USB: serial: ir-usb: fix big-endian baud-rate debug printk
  USB: serial: mct_u232: fix big-endian baud-rate handling
  USB: serial: io_ti: fix div-by-zero in set_termios

 drivers/usb/serial/ftdi_sio.c | 10 +-
 drivers/usb/serial/ftdi_sio_ids.h |  2 ++
 drivers/usb/serial/io_ti.c|  5 -
 drivers/usb/serial/ir-usb.c   | 21 -
 drivers/usb/serial/mct_u232.c |  2 +-
 drivers/usb/serial/option.c   |  8 
 drivers/usb/serial/qcserial.c |  2 ++
 7 files changed, 34 insertions(+), 16 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] usb: xhci: add quirk flag for broken stop command on AMD platforms

2017-05-19 Thread Mathias Nyman

On 18.05.2017 16:46, Alan Stern wrote:

On Thu, 18 May 2017, Shyam Sundar S K wrote:


on AMD platforms with SNPS 3.1 USB controller has an issue
if the stop EP command is issued when the controller is not
in running state. If issued, it is leading to a critical RTL bug
because of which controller goes into irrecoverable state.

This patch adds a appropriate checks to make sure that scenario
does not happen.

Signed-off-by: Shyam Sundar S K 
Signed-off-by: Nehal Shah 
---



--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1819,6 +1819,7 @@ struct xhci_hcd {
  /* For controller with a broken Port Disable implementation */
  #define XHCI_BROKEN_PORT_PED  (1 << 25)
  #define XHCI_LIMIT_ENDPOINT_INTERVAL_7(1 << 26)
+#define XHCI_BROKEN_STOP   (1 << 27)


Does there really need to be a quirk flag for this?  I should think
that you never want to issue a STOP EP command while the controller is
not running, no matter what kind of controller it is.

Alan Stern



If it's about controller not running then there shouldn't be any problems.
We shouldn't issue a stop endpoint command if controller is halted.

If this is about issuing a stop endpoint command while endpoint isn't
running, then fully working controllers should just respond with a command
completion with "context state error" status.

Anyway, as Alan said the quirk is probably unnecessary here. We shouldn't need 
to
stop endpoints that are not running. Only problem I see here is that the
endpoint state in the output endpoint context might not be up to date. If driver
just restarted the endpoint by ringing the doorbell, the output context state
might not be updated yet.

How does this SNPS 3.1 controller react if the endpoint just halted or moved to
error state just before controller runs the stop endpoint command? Still 
triggers
the RTL bug?

I'm talking about the in xhci spec 4.6.9:

" A Busy endpoint may asynchronously transition from the Running to the Halted 
or Error state due
to error conditions detected while processing TRBs. A possible race condition 
may occur if
software, thinking an endpoint is in the Running state, issues a Stop Endpoint 
Command however
at the same time the xHC asynchronously transitions the endpoint to the Halted 
or Error state. In
this case, a Context State Error may be generated for the command completion. 
Software may
verify that this case occurred by inspecting the EP State for Halted or Error 
when a Stop Endpoint
Command results in a Context State Error."

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


Re: [PATCH resend v3 1/4] usb: usbip tool: Check the return of get_nports()

2017-05-19 Thread Yuyang Du
On Thu, May 18, 2017 at 03:53:04PM +0300, Sergei Shtylyov wrote:
> >+if (vhci_driver->nports <=0) {
> 
>Please add a space after <= too.

Indeed. Thanks.

Hi Greg,

Do you want me to send another version to fix this?

Thanks,
Yuyang
--
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