[RFC PATCH] usb: host: xhci: plat: add support for otg_set_host() call

[Manish Narani]

This patch will add support for OTG host initialization. This will
help OTG drivers to populate their host subsystem.

Signed-off-by: Manish Narani 
---
 drivers/usb/host/xhci-plat.c | 39 +++
 1 file changed, 39 insertions(+)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index ddfab30..aa08bdd 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "xhci.h"
 #include "xhci-plat.h"
@@ -144,6 +145,37 @@ static const struct of_device_id usb_xhci_of_match[] = {
 MODULE_DEVICE_TABLE(of, usb_xhci_of_match);
 #endif
 
+static int usb_otg_set_host(struct device *dev, struct usb_hcd *hcd, bool yes)
+{
+   int ret = 0;
+
+   hcd->usb_phy = usb_get_phy(USB_PHY_TYPE_USB3);
+   if (!IS_ERR_OR_NULL(hcd->usb_phy) && hcd->usb_phy->otg) {
+   dev_dbg(dev, "%s otg support available\n", __func__);
+   if (yes) {
+   if (otg_set_host(hcd->usb_phy->otg, >self)) {
+   dev_err(dev, "%s otg_set_host failed\n",
+   __func__);
+   usb_put_phy(hcd->usb_phy);
+   goto disable_phy;
+   }
+   } else {
+   ret = otg_set_host(hcd->usb_phy->otg, NULL);
+   usb_put_phy(hcd->usb_phy);
+   goto disable_phy;
+   }
+
+   } else
+   goto disable_phy;
+
+   return 0;
+
+disable_phy:
+   hcd->usb_phy = NULL;
+
+   return ret;
+}
+
 static int xhci_plat_probe(struct platform_device *pdev)
 {
const struct of_device_id *match;
@@ -255,6 +287,11 @@ static int xhci_plat_probe(struct platform_device *pdev)
if (ret)
goto dealloc_usb2_hcd;
 
+   ret = usb_otg_set_host(>dev, hcd, 1);
+   if (ret)
+   goto dealloc_usb2_hcd;
+
+
return 0;
 
 
@@ -283,6 +320,8 @@ static int xhci_plat_remove(struct platform_device *dev)
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
struct clk *clk = xhci->clk;
 
+   usb_otg_set_host(>dev, hcd, 0);
+
usb_remove_hcd(xhci->shared_hcd);
usb_phy_shutdown(hcd->usb_phy);
 
-- 
2.1.1

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

[PATCH v2 1/1] Increase USB transfer limit

[Mateusz Berezecki]

Promote a variable keeping track of USB transfer memory usage to a
wider data type and allow for higher bandwidth transfers from a large
number of USB devices connected to a single host.

Signed-off-by: Mateusz Berezecki 
---
 drivers/usb/core/devio.c | 43 ---
 1 file changed, 16 insertions(+), 27 deletions(-)

diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index 4016dae..52747b6 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -134,42 +134,35 @@ enum snoop_when {
 #define USB_DEVICE_DEV MKDEV(USB_DEVICE_MAJOR, 0)
 
 /* Limit on the total amount of memory we can allocate for transfers */
-static unsigned usbfs_memory_mb = 16;
+static u32 usbfs_memory_mb = 16;
 module_param(usbfs_memory_mb, uint, 0644);
 MODULE_PARM_DESC(usbfs_memory_mb,
"maximum MB allowed for usbfs buffers (0 = no limit)");
 
-/* Hard limit, necessary to avoid arithmetic overflow */
-#define USBFS_XFER_MAX (UINT_MAX / 2 - 100)
-
-static atomic_t usbfs_memory_usage;/* Total memory currently allocated */
+static atomic64_t usbfs_memory_usage;  /* Total memory currently allocated */
 
 /* Check whether it's okay to allocate more memory for a transfer */
-static int usbfs_increase_memory_usage(unsigned amount)
+static int usbfs_increase_memory_usage(u64 amount)
 {
-   unsigned lim;
+   u64 lim;
 
-   /*
-* Convert usbfs_memory_mb to bytes, avoiding overflows.
-* 0 means use the hard limit (effectively unlimited).
-*/
lim = ACCESS_ONCE(usbfs_memory_mb);
-   if (lim == 0 || lim > (USBFS_XFER_MAX >> 20))
-   lim = USBFS_XFER_MAX;
-   else
-   lim <<= 20;
+   lim <<= 20;
 
-   atomic_add(amount, _memory_usage);
-   if (atomic_read(_memory_usage) <= lim)
-   return 0;
-   atomic_sub(amount, _memory_usage);
-   return -ENOMEM;
+   atomic64_add(amount, _memory_usage);
+
+   if (lim > 0 && atomic64_read(_memory_usage) > lim) {
+   atomic64_sub(amount, _memory_usage);
+   return -ENOMEM;
+   }
+
+   return 0;
 }
 
 /* Memory for a transfer is being deallocated */
-static void usbfs_decrease_memory_usage(unsigned amount)
+static void usbfs_decrease_memory_usage(u64 amount)
 {
-   atomic_sub(amount, _memory_usage);
+   atomic64_sub(amount, _memory_usage);
 }
 
 static int connected(struct usb_dev_state *ps)
@@ -1191,7 +1184,7 @@ static int proc_bulk(struct usb_dev_state *ps, void 
__user *arg)
if (!usb_maxpacket(dev, pipe, !(bulk.ep & USB_DIR_IN)))
return -EINVAL;
len1 = bulk.len;
-   if (len1 >= USBFS_XFER_MAX)
+   if (len1 >= (INT_MAX - sizeof(struct urb)))
return -EINVAL;
ret = usbfs_increase_memory_usage(len1 + sizeof(struct urb));
if (ret)
@@ -1584,10 +1577,6 @@ static int proc_do_submiturb(struct usb_dev_state *ps, 
struct usbdevfs_urb *uurb
return -EINVAL;
}
 
-   if (uurb->buffer_length >= USBFS_XFER_MAX) {
-   ret = -EINVAL;
-   goto error;
-   }
if (uurb->buffer_length > 0 &&
!access_ok(is_in ? VERIFY_WRITE : VERIFY_READ,
uurb->buffer, uurb->buffer_length)) {
-- 
2.7.4

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

Re: [RFC PATCH] usb: host: xhci: plat: add support for otg_set_host() call

[Greg KH]

On Thu, Dec 15, 2016 at 12:25:08AM +0530, Manish Narani wrote:
> This patch will add support for OTG host initialization. This will
> help OTG drivers to populate their host subsystem.
> 
> Signed-off-by: Manish Narani 
> ---
>  drivers/usb/host/xhci-plat.c |   35 +++
>  1 files changed, 35 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index ddfab30..b4cadbd 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -20,6 +20,10 @@
>  #include 
>  #include 
>  
> +#ifdef CONFIG_USB_OTG
> +#include 
> +#endif

never use a #ifdef in a .c file if at all possible.  Here you don't need
it at all.

> +
>  #include "xhci.h"
>  #include "xhci-plat.h"
>  #include "xhci-mvebu.h"
> @@ -255,6 +259,24 @@ static int xhci_plat_probe(struct platform_device *pdev)
>   if (ret)
>   goto dealloc_usb2_hcd;
>  
> +#ifdef CONFIG_USB_OTG
> + hcd->usb_phy = usb_get_phy(USB_PHY_TYPE_USB3);
> + if (!IS_ERR_OR_NULL(hcd->usb_phy) && hcd->usb_phy->otg) {
> + dev_dbg(>dev, "%s otg support available\n", __func__);
> + ret = otg_set_host(hcd->usb_phy->otg, >self);
> + if (ret) {
> + dev_err(>dev, "%s otg_set_host failed\n",
> + __func__);
> + usb_put_phy(hcd->usb_phy);
> + hcd->usb_phy = NULL;
> + goto dealloc_usb2_hcd;
> + }
> + } else {
> + usb_put_phy(hcd->usb_phy);
> + hcd->usb_phy = NULL;
> + }
> +#endif

Can't you wrap this in a function to get rid of this #ifdef mess?

> +
>   return 0;
>  
>  
> @@ -283,6 +305,19 @@ static int xhci_plat_remove(struct platform_device *dev)
>   struct xhci_hcd *xhci = hcd_to_xhci(hcd);
>   struct clk *clk = xhci->clk;
>  
> +#ifdef CONFIG_USB_OTG
> + if (hcd->usb_phy) {
> + if (!IS_ERR(hcd->usb_phy)) {
> + if (hcd->usb_phy->otg)
> + otg_set_host(hcd->usb_phy->otg, NULL);
> + usb_put_phy(hcd->usb_phy);
> + }
> + hcd->usb_phy = NULL;
> + if (xhci->shared_hcd)
> + xhci->shared_hcd->usb_phy = NULL;
> + }
> +#endif

same here.

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

[RFC PATCH] usb: host: xhci: plat: add support for otg_set_host() call

[Manish Narani]

This patch will add support for OTG host initialization. This will
help OTG drivers to populate their host subsystem.

Signed-off-by: Manish Narani 
---
 drivers/usb/host/xhci-plat.c |   35 +++
 1 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index ddfab30..b4cadbd 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -20,6 +20,10 @@
 #include 
 #include 
 
+#ifdef CONFIG_USB_OTG
+#include 
+#endif
+
 #include "xhci.h"
 #include "xhci-plat.h"
 #include "xhci-mvebu.h"
@@ -255,6 +259,24 @@ static int xhci_plat_probe(struct platform_device *pdev)
if (ret)
goto dealloc_usb2_hcd;
 
+#ifdef CONFIG_USB_OTG
+   hcd->usb_phy = usb_get_phy(USB_PHY_TYPE_USB3);
+   if (!IS_ERR_OR_NULL(hcd->usb_phy) && hcd->usb_phy->otg) {
+   dev_dbg(>dev, "%s otg support available\n", __func__);
+   ret = otg_set_host(hcd->usb_phy->otg, >self);
+   if (ret) {
+   dev_err(>dev, "%s otg_set_host failed\n",
+   __func__);
+   usb_put_phy(hcd->usb_phy);
+   hcd->usb_phy = NULL;
+   goto dealloc_usb2_hcd;
+   }
+   } else {
+   usb_put_phy(hcd->usb_phy);
+   hcd->usb_phy = NULL;
+   }
+#endif
+
return 0;
 
 
@@ -283,6 +305,19 @@ static int xhci_plat_remove(struct platform_device *dev)
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
struct clk *clk = xhci->clk;
 
+#ifdef CONFIG_USB_OTG
+   if (hcd->usb_phy) {
+   if (!IS_ERR(hcd->usb_phy)) {
+   if (hcd->usb_phy->otg)
+   otg_set_host(hcd->usb_phy->otg, NULL);
+   usb_put_phy(hcd->usb_phy);
+   }
+   hcd->usb_phy = NULL;
+   if (xhci->shared_hcd)
+   xhci->shared_hcd->usb_phy = NULL;
+   }
+#endif
+
usb_remove_hcd(xhci->shared_hcd);
usb_phy_shutdown(hcd->usb_phy);
 
-- 
1.7.1

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

[PATCH] USB: dummy-hcd: fix bug in stop_activity (handle ep0)

[Alan Stern]

The stop_activity() routine in dummy-hcd is supposed to unlink all
active requests for every endpoint, among other things.  But it
doesn't handle ep0.  As a result, fuzz testing can generate a WARNING
like the following:

WARNING: CPU: 0 PID: 4410 at drivers/usb/gadget/udc/dummy_hcd.c:672 
dummy_free_request+0x153/0x170
Modules linked in:
CPU: 0 PID: 4410 Comm: syz-executor Not tainted 4.9.0-rc7+ #32
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
 88006a64ed10 81f96b8a 41b58ab3 11000d4c9d35
 ed000d4c9d2d 880065f8ac00 41b58ab3 8598b510
 81f968f8 41b58ab3 859410e0 813f0590
Call Trace:
 [< inline >] __dump_stack lib/dump_stack.c:15
 [] dump_stack+0x292/0x398 lib/dump_stack.c:51
 [] __warn+0x19f/0x1e0 kernel/panic.c:550
 [] warn_slowpath_null+0x2c/0x40 kernel/panic.c:585
 [] dummy_free_request+0x153/0x170 
drivers/usb/gadget/udc/dummy_hcd.c:672
 [] usb_ep_free_request+0xc0/0x420 
drivers/usb/gadget/udc/core.c:195
 [] gadgetfs_unbind+0x131/0x190 
drivers/usb/gadget/legacy/inode.c:1612
 [] usb_gadget_remove_driver+0x10f/0x2b0 
drivers/usb/gadget/udc/core.c:1228
 [] usb_gadget_unregister_driver+0x154/0x240 
drivers/usb/gadget/udc/core.c:1357

This patch fixes the problem by iterating over all the endpoints in
the driver's ep array instead of iterating over the gadget's ep_list,
which explicitly leaves out ep0.

Signed-off-by: Alan Stern 
Reported-by: Andrey Konovalov 
CC: 

---


[as1820]


 drivers/usb/gadget/udc/dummy_hcd.c |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Index: usb-4.x/drivers/usb/gadget/udc/dummy_hcd.c
===
--- usb-4.x.orig/drivers/usb/gadget/udc/dummy_hcd.c
+++ usb-4.x/drivers/usb/gadget/udc/dummy_hcd.c
@@ -330,7 +330,7 @@ static void nuke(struct dummy *dum, stru
 /* caller must hold lock */
 static void stop_activity(struct dummy *dum)
 {
-   struct dummy_ep *ep;
+   int i;
 
/* prevent any more requests */
dum->address = 0;
@@ -338,8 +338,8 @@ static void stop_activity(struct dummy *
/* The timer is left running so that outstanding URBs can fail */
 
/* nuke any pending requests first, so driver i/o is quiesced */
-   list_for_each_entry(ep, >gadget.ep_list, ep.ep_list)
-   nuke(dum, ep);
+   for (i = 0; i < DUMMY_ENDPOINTS; ++i)
+   nuke(dum, >ep[i]);
 
/* driver now does any non-usb quiescing necessary */
 }

--
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: musb: debugfs: allow forcing host mode together with speed in testmode

[Pali Rohár]

Based on the musb ug, force_host bit is allowed to be set along with
force_hs or force_fs bit.

It could help to implement forced host mode via testmode on Nokia N900.

Signed-off-by: Pali Rohár 
---
 drivers/usb/musb/musb_debugfs.c |   44 +--
 1 file changed, 28 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/musb/musb_debugfs.c b/drivers/usb/musb/musb_debugfs.c
index 9b22d94..62c13a2 100644
--- a/drivers/usb/musb/musb_debugfs.c
+++ b/drivers/usb/musb/musb_debugfs.c
@@ -147,28 +147,34 @@ static int musb_test_mode_show(struct seq_file *s, void 
*unused)
 
test = musb_readb(musb->mregs, MUSB_TESTMODE);
 
-   if (test & MUSB_TEST_FORCE_HOST)
+   if (test & (MUSB_TEST_FORCE_HOST | MUSB_TEST_FORCE_FS))
+   seq_printf(s, "force host full-speed\n");
+
+   else if (test & (MUSB_TEST_FORCE_HOST | MUSB_TEST_FORCE_HS))
+   seq_printf(s, "force host high-speed\n");
+
+   else if (test & MUSB_TEST_FORCE_HOST)
seq_printf(s, "force host\n");
 
-   if (test & MUSB_TEST_FIFO_ACCESS)
+   else if (test & MUSB_TEST_FIFO_ACCESS)
seq_printf(s, "fifo access\n");
 
-   if (test & MUSB_TEST_FORCE_FS)
+   else if (test & MUSB_TEST_FORCE_FS)
seq_printf(s, "force full-speed\n");
 
-   if (test & MUSB_TEST_FORCE_HS)
+   else if (test & MUSB_TEST_FORCE_HS)
seq_printf(s, "force high-speed\n");
 
-   if (test & MUSB_TEST_PACKET)
+   else if (test & MUSB_TEST_PACKET)
seq_printf(s, "test packet\n");
 
-   if (test & MUSB_TEST_K)
+   else if (test & MUSB_TEST_K)
seq_printf(s, "test K\n");
 
-   if (test & MUSB_TEST_J)
+   else if (test & MUSB_TEST_J)
seq_printf(s, "test J\n");
 
-   if (test & MUSB_TEST_SE0_NAK)
+   else if (test & MUSB_TEST_SE0_NAK)
seq_printf(s, "test SE0 NAK\n");
 
return 0;
@@ -206,30 +212,36 @@ static ssize_t musb_test_mode_write(struct file *file,
if (copy_from_user(buf, ubuf, min_t(size_t, sizeof(buf) - 1, count)))
return -EFAULT;
 
-   if (strstarts(buf, "force host"))
+   if (strstarts(buf, "force host full-speed"))
+   test = MUSB_TEST_FORCE_HOST | MUSB_TEST_FORCE_FS;
+
+   else if (strstarts(buf, "force host high-speed"))
+   test = MUSB_TEST_FORCE_HOST | MUSB_TEST_FORCE_HS;
+
+   else if (strstarts(buf, "force host"))
test = MUSB_TEST_FORCE_HOST;
 
-   if (strstarts(buf, "fifo access"))
+   else if (strstarts(buf, "fifo access"))
test = MUSB_TEST_FIFO_ACCESS;
 
-   if (strstarts(buf, "force full-speed"))
+   else if (strstarts(buf, "force full-speed"))
test = MUSB_TEST_FORCE_FS;
 
-   if (strstarts(buf, "force high-speed"))
+   else if (strstarts(buf, "force high-speed"))
test = MUSB_TEST_FORCE_HS;
 
-   if (strstarts(buf, "test packet")) {
+   else if (strstarts(buf, "test packet")) {
test = MUSB_TEST_PACKET;
musb_load_testpacket(musb);
}
 
-   if (strstarts(buf, "test K"))
+   else if (strstarts(buf, "test K"))
test = MUSB_TEST_K;
 
-   if (strstarts(buf, "test J"))
+   else if (strstarts(buf, "test J"))
test = MUSB_TEST_J;
 
-   if (strstarts(buf, "test SE0 NAK"))
+   else if (strstarts(buf, "test SE0 NAK"))
test = MUSB_TEST_SE0_NAK;
 
musb_writeb(musb->mregs, MUSB_TESTMODE, test);
-- 
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: [PATCH v4 1/2] USB: add switch to turn off padding of resume time delays

[Todd Brandt]

On Wed, 2016-12-14 at 11:36 -0500, Alan Stern wrote:
> On Wed, 14 Dec 2016, Todd Brandt wrote:
> 
> > Add a kernel parameter that replaces the USB_RESUME_TIMEOUT
> > and other hardcoded delay numbers with the USB spec minimums.
> > 
> > The USB subsystem currently uses heavily padded values for TDRSMDN
> > and TRSTRCY. This patch keeps the current values by default, but if
> > the kernel is booted with usb_timing_minimum=1 they are set to the
> > spec minimums with no padding. The result is significant performance
> > improvement in usb device resume.
> > 
> > Example analyze_suspend runs are provided here showing the benefits:
> > https://01.org/suspendresume/blogs/tebrandt/2016/usb-resume-optimization-using-spec-minimum-delays
> > 
> > Signed-off-by: Todd Brandt 
> 
> ...
> 
> > --- a/drivers/usb/common/common.c
> > +++ b/drivers/usb/common/common.c
> > @@ -19,6 +19,13 @@
> >  #include 
> >  #include 
> >  
> > +struct _usb_timing_config usb_timing = {
> 
> Initial '_'?  Ugh.  How about just struct usb_timing_config?
ok, will do.

> 
> > +   .tdrsmdn = USB_TIMING_TDRSMDN_DEF,
> > +   .trsmrcy = USB_TIMING_TRSMRCY_DEF,
> > +   .trstrcy = USB_TIMING_TRSTRCY_DEF
> > +};
> > +EXPORT_SYMBOL_GPL(usb_timing);
> 
> > --- a/drivers/usb/host/uhci-hub.c
> > +++ b/drivers/usb/host/uhci-hub.c
> 
> > @@ -339,7 +339,7 @@ static int uhci_hub_control(struct usb_hcd *hcd, u16 
> > typeReq, u16 wValue,
> >  
> > /* USB v2.0 7.1.7.5 */
> > uhci->ports_timeout = jiffies +
> > -   msecs_to_jiffies(USB_RESUME_TIMEOUT);
> > +   msecs_to_jiffies(usb_timing.trstrcy);
> 
> Actually this was wrong from the beginning (it was a bug).  It should
> be 50 ms (TDRSTR), not TRSTRCY.  I suppose that could be fixed in a 
> separate patch.

I could add tdrstr to the usb_timing struct itself. It has a minimum of
50ms, but it might be useful to tweak it upwards with the debugfs
interface for buggy hardware. (I'll do a v5)

> 
> 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 v2] keys/encrypted: Fix two crypto-on-the-stack bugs

[David Howells]

Andy Lutomirski  wrote:

> David, are these encrypted keys ever exported anywhere?  If not, could
> the code use a mode that doesn't need padding?

ecryptfs uses them, I think.

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] keys/encrypted: Fix two crypto-on-the-stack bugs

[Joerg Roedel]

On Tue, Dec 13, 2016 at 08:40:00AM -0800, Andy Lutomirski wrote:
> But I think this is rather silly.  Joerg, Linus, etc: would it be okay
> to change lib/dma-debug.c to allow DMA *from* rodata?

Yeah, this would be fine for DMA_TO_DEVICE mappings. At least I can't
think of a reason right now to not allow it, in the end its also
read-only memory for the CPU, so it can be readable by devices too.
There is no danger of race conditions like on stacks or data leaks, as
there is only compile-time data in rodata.



Joerg

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

Re: [PATCH v4 2/2] USB: usb resume time delay values debug

[Todd Brandt]

On Wed, 2016-12-14 at 10:33 -0500, Alan Stern wrote:
> On Wed, 14 Dec 2016, Todd Brandt wrote:
> 
> > add debugfs support for experimenting with USB timing delay
> > values on the fly. Values are read/written from debugfs at
> > /sys/kernel/debug/usb/timing.
> > 
> > Signed-off-by: Todd Brandt 
> > ---
> > v2 changes:
> >  - moved the debug code from hub.c to usb.c
> >  - use debugfs instead of /sys/kernel/usb
> 
> ...
> 
> > +static int usb_timing_show(struct seq_file *s, void *unused)
> > +{
> > +   seq_printf(s, "tdrsmdn=%d\n", usb_timing.tdrsmdn);
> > +   seq_printf(s, "trsmrcy=%d\n", usb_timing.trsmrcy);
> > +   seq_printf(s, "trstrcy=%d\n", usb_timing.trstrcy);
> > +   return 0;
> > +}
> > +
> > +static int usb_timing_open(struct inode *inode, struct file *file)
> > +{
> > +   return single_open(file, usb_timing_show, inode->i_private);
> > +}
> > +
> > +static ssize_t usb_timing_write(struct file *file,
> > +   const char __user *ubuf, size_t count, loff_t *ppos)
> > +{
> > +   int val;
> > +   char buf[32];
> > +
> > +   if (copy_from_user(, ubuf, min_t(size_t, sizeof(buf) - 1, count)))
> > +   return -EFAULT;
> > +
> > +   if (sscanf(buf, "tdrsmdn=%u", ) == 1)
> > +   usb_timing.tdrsmdn = max(val, USB_TIMING_TDRSMDN_MIN);
> > +   else if (sscanf(buf, "trsmrcy=%u", ) == 1)
> > +   usb_timing.trsmrcy = max(val, USB_TIMING_TRSMRCY_MIN);
> > +   else if (sscanf(buf, "trstrcy=%u", ) == 1)
> > +   usb_timing.trstrcy = max(val, USB_TIMING_TRSTRCY_MIN);
> 
> Nit: Since the values in usb_timing are signed integers, and since val 
> is a signed integer, why do the sscanf calls use %u rather than %d?

I just want the extra format checking for sscanf, it refuses negative
integers (I prefer an -EINVAL in the event of a negative time input
rather than a silient setting to VAL_MIN). I could declare val an
unsigned int, but then "max(" throws a warning about a type mismatch, so
I figured this was the simplest way to implement it without having to
use any casts.

> 
> 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] usb: musb: debugfs: allow forcing host mode together with speed in testmode

[Greg Kroah-Hartman]

On Wed, Dec 14, 2016 at 03:47:57PM +0100, Pali Rohár wrote:
> Signed-off-by: Pali Rohár 
> ---
>  drivers/usb/musb/musb_debugfs.c |   44 
> +--
>  1 file changed, 28 insertions(+), 16 deletions(-)

I don't accept patches without any changelog information, nor would I
ever expect any other maintainer to do so. Please read
Documentation/SubmittingPatches for how to write a good changelog
message.

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: usb/gadget: warning in ep_write_iter/__alloc_pages_nodemask

[Alan Stern]

On Wed, 14 Dec 2016, Michal Hocko wrote:

> On Tue 13-12-16 08:33:34, Alan Stern wrote:
> > On Tue, 13 Dec 2016, Michal Hocko wrote:
> > 
> > > > > That being said, what ep_write_iter does sounds quite stupit. It just
> > > > > allocates a large continuous buffer which seems to be under user
> > > > > control...  Aka no good! It should do that per pages or something like
> > > > > that. Something worth fixing
> > > > 
> > > > It's not important enough to make the driver do all this work.  If
> > > > users want to send large amounts of data, they can send it a page at a
> > > > time (or something like that).
> > > 
> > > Is it really necessary to allocate the full iov_iter_count? Why cannot
> > > we process the from buffer one page at a time?
> > 
> > We could (although one page is really too small -- USB 3.1 can transfer
> > 800 KB per ms so we ought to handle at least 128 KB at a time).
> 
> Is there any problem to submit larger transfers without having the
> buffer physically contiguous?

Async I/O would be rather awkward; it would have to use a work queue
routine.  But it could be done.

And we would still end up allocating the same total space (more
actually, because we would need to store the scatter-gather table too).  
It just wouldn't be contiguous.

> > But
> > turn the argument around: If the user wants to transfer that much data,
> > why can't he _submit_ it one page at a time?
> 
> Not sure I understand.
>  
> > > > If you really want to prevent the driver from attempting to allocate a
> > > > large buffer, all that's needed is an upper limit on the total size.  
> > > > For example, 64 KB.
> > > 
> > > Well, my point was that it is not really hard to imagine to deplete
> > > larger contiguous memory blocks (say PAGE_ALLOC_COSTLY_ORDER). Those are
> > > still causing the OOM killer and chances are that a controlled flood of
> > > these requests could completely DoS the system.
> > 
> > Putting a limit on the total size of a single transfer would prevent 
> > this.
> 
> Dunno, putting a limit to the user visible interface sounds wrong to me.

In practice, I think the data transfer sizes tend to be not very large.  
But I could be wrong about that.

Alan Stern

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

Re: [PATCH v4 1/2] USB: add switch to turn off padding of resume time delays

[Alan Stern]

On Wed, 14 Dec 2016, Todd Brandt wrote:

> Add a kernel parameter that replaces the USB_RESUME_TIMEOUT
> and other hardcoded delay numbers with the USB spec minimums.
> 
> The USB subsystem currently uses heavily padded values for TDRSMDN
> and TRSTRCY. This patch keeps the current values by default, but if
> the kernel is booted with usb_timing_minimum=1 they are set to the
> spec minimums with no padding. The result is significant performance
> improvement in usb device resume.
> 
> Example analyze_suspend runs are provided here showing the benefits:
> https://01.org/suspendresume/blogs/tebrandt/2016/usb-resume-optimization-using-spec-minimum-delays
> 
> Signed-off-by: Todd Brandt 

...

> --- a/drivers/usb/common/common.c
> +++ b/drivers/usb/common/common.c
> @@ -19,6 +19,13 @@
>  #include 
>  #include 
>  
> +struct _usb_timing_config usb_timing = {

Initial '_'?  Ugh.  How about just struct usb_timing_config?

> + .tdrsmdn = USB_TIMING_TDRSMDN_DEF,
> + .trsmrcy = USB_TIMING_TRSMRCY_DEF,
> + .trstrcy = USB_TIMING_TRSTRCY_DEF
> +};
> +EXPORT_SYMBOL_GPL(usb_timing);

> --- a/drivers/usb/host/uhci-hub.c
> +++ b/drivers/usb/host/uhci-hub.c

> @@ -339,7 +339,7 @@ static int uhci_hub_control(struct usb_hcd *hcd, u16 
> typeReq, u16 wValue,
>  
>   /* USB v2.0 7.1.7.5 */
>   uhci->ports_timeout = jiffies +
> - msecs_to_jiffies(USB_RESUME_TIMEOUT);
> + msecs_to_jiffies(usb_timing.trstrcy);

Actually this was wrong from the beginning (it was a bug).  It should
be 50 ms (TDRSTR), not TRSTRCY.  I suppose that could be fixed in a 
separate patch.

Alan Stern

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

Re: [PATCH v2] keys/encrypted: Fix two crypto-on-the-stack bugs

[Andy Lutomirski]

On Wed, Dec 14, 2016 at 12:37 AM, David Howells  wrote:
> Andy Lutomirski  wrote:
>
>> > -   sg_set_buf(_out[1], pad, sizeof pad);
>> > +   sg_set_buf(_out[1], empty_zero_page, 16);
>>
>> My fix here is obviously bogus (I meant to use ZERO_PAGE(0)), but what
>> exactly is the code trying to do?  The old code makes no sense.  It's
>> setting the *output* buffer to zeroed padding.
>
> Padding goes into the encrypt function and is going to come out of the decrypt
> function.  Possibly derived_key_decrypt() should be checking that the padding
> that comes out is actually a bunch of zeros.  Maybe we don't actually need to
> get the padding out, but I'm not sure whether the crypto layer will
> malfunction if we don't give it a buffer for the padding.

It was the memset that threw me for a loop.

David, are these encrypted keys ever exported anywhere?  If not, could
the code use a mode that doesn't need padding?

--Andy

>
> David



-- 
Andy Lutomirski
AMA Capital Management, LLC
--
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: usb/gadget: warning in ep_write_iter/__alloc_pages_nodemask

[Michal Hocko]

On Wed 14-12-16 11:13:11, Alan Stern wrote:
> On Wed, 14 Dec 2016, Michal Hocko wrote:
> 
> > On Tue 13-12-16 08:33:34, Alan Stern wrote:
> > > On Tue, 13 Dec 2016, Michal Hocko wrote:
[...]
> > > > Well, my point was that it is not really hard to imagine to deplete
> > > > larger contiguous memory blocks (say PAGE_ALLOC_COSTLY_ORDER). Those are
> > > > still causing the OOM killer and chances are that a controlled flood of
> > > > these requests could completely DoS the system.
> > > 
> > > Putting a limit on the total size of a single transfer would prevent 
> > > this.
> > 
> > Dunno, putting a limit to the user visible interface sounds wrong to me.
> 
> In practice, I think the data transfer sizes tend to be not very large.  
> But I could be wrong about that.

That is one part the other is whether a malicious user can abuse this to
DoS the kernel which is the point I am trying to make here. Depleting
non-costly high orders can be quite dangerious so allowing a free ticket
to them to arbitrary user in an arbitrary amount is definitely not good.
-- 
Michal Hocko
SUSE Labs
--
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] tools: usb: usbip: Update README

[Shuah Khan]

Hi Krzysztof,

Thanks for the patch.

On 12/13/2016 12:52 PM, Krzysztof Opasiak wrote:
> Update README file:
> - remove outdated parts
> - clarify terminology and general structure
> - add some description of vUDC
> 
> Signed-off-by: Krzysztof Opasiak 
> ---
>  tools/usb/usbip/README | 56 
> +-
>  1 file changed, 55 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/usb/usbip/README b/tools/usb/usbip/README
> index 831f49fea3ce..148d13814306 100644
> --- a/tools/usb/usbip/README
> +++ b/tools/usb/usbip/README
> @@ -4,10 +4,32 @@
>  # Copyright (C) 2011 matt mooney 
>  #   2005-2008 Takahiro Hirofuchi
>  
> +[Overview]
> +USB/IP protocol allows to pass USB device from server to client over the
> +network. The USB device may be either physical device connected to a server 
> or
> +software entity created on a server using USB gadget subsystem.
> +Whole project consists of four parts:
> +

Please add definitions for client and server.

> +- usbip-vhci
> +Kernel module which provides a virtual USB Host Controller and allows
> +to import a USB device from a remote machine. Used on a client side.

Could be rephrased as:

A client side Kernel module which provides a virtual USB Host Controller and 
allows
import of a USB device from a remote server machine.

> +
> +- usbip-host (stub driver)
> +Kernel module which provides a USB device driver which can be bound 
> to
> +a physical USB device to make it exportable. Used on a server side.
> +

A server side Kernel module which provides a USB device driver that exports
physical USB devices that are bound to it.

> +- usbip-vudc
> +Kernel module which provides a virtual USB Device Controller and 
> allows
> +to export a USB device created using USB Gadget Subsystem. Used on
> +a server side.
> +

Similar changes as above.

> +- usbip-utils
> +A set of userspace tools used to handle connection and management.
> +Used on both sides.
>  
>  [Requirements]
>  - USB/IP device drivers
> - Found in the staging directory of the Linux kernel.
> +Found in the drivers/usb/usbip/ directory of the Linux kernel tree.
>  
>  - libudev >= 2.0
>   libudev library
> @@ -36,6 +58,10 @@
>  
>  
>  [Usage]
> +On a server side there are two entities which can be shared.
> +First of them is physical usb device connected to the machine.
> +To make it available below steps should be executed:
> +
>  server:# (Physically attach your USB device.)
>  
>  server:# insmod usbip-core.ko
> @@ -52,6 +78,30 @@
>   - The USB device 1-2 is now exportable to other hosts!
>   - Use `usbip unbind --busid 1-2' to stop exporting the device.
>  
> +Second of shareable entities is USB Gadget created using USB Gadget Subsystem
> +on a server machine. To make it available below steps should be executed:
> +
> +server:# (Create your USB gadget)
> +- Currently the most preferable way of creating a new USB gadget
> +  is ConfigFS Composite Gadget. Please refer to its documentation
> +  for details.
> +- See vudc_server_example.sh for a short example of USB gadget 
> creation
> +
> +server:# insmod usbip-core.ko
> +server:# insmod usbip-vudc.ko
> +- To create more than one instance of vudc use num module param
> +
> +server:# (Bind gadget to one of available vudc)
> +- Assign your new gadget to USB/IP UDC
> +- Using ConfigFS interface you may do this simply by:
> +server:# cd /sys/kernel/config/usb_gadget/
> +server:# echo "usbip-vudc.0" > UDC
> +
> +server:# usbipd -D --device
> +- Start usbip daemon.
> +
> +To attach new device to client machine below commands should be used:
> +
>  client:# insmod usbip-core.ko
>  client:# insmod vhci-hcd.ko
>  
> @@ -60,6 +110,8 @@
>  
>  client:# usbip attach --remote  --busid 1-2
>   - Connect the remote USB device.
> + - When using vudc on a server side busid is really vudc instance name.
> +   For example: usbip-vudc.0
>  
>  client:# usbip port
>   - Show virtual port status.
> @@ -192,6 +244,8 @@ Detach the imported device:
>   - http://usbip.wiki.sourceforge.net/how-to-debug-usbip
>  - usbip-host.ko must be bound to the target device.
>   - See /proc/bus/usb/devices and find "Driver=..." lines of the device.
> +- Target USB gadget must be bound to vudc
> +  (using USB gadget susbsys, not usbip bind command)
>  - Shutdown firewall.
>   - usbip now uses TCP port 3240.
>  - Disable SELinux.
> 

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 1/1] Increase USB transfer limit

[Alan Stern]

On Tue, 13 Dec 2016, Mateusz Berezecki wrote:

> Promote a variable keeping track of USB transfer memory usage to a
> wider data type and allow for higher bandwidth transfers from a large
> number of USB devices connected to a single host.
> 
> Signed-off-by: Mateusz Berezecki 
> ---

...

>  /* Check whether it's okay to allocate more memory for a transfer */
> -static int usbfs_increase_memory_usage(unsigned amount)
> +static int usbfs_increase_memory_usage(u64 amount)
>  {
> - unsigned lim;
> + u64 lim;
>  
> - /*
> -  * Convert usbfs_memory_mb to bytes, avoiding overflows.
> -  * 0 means use the hard limit (effectively unlimited).
> -  */
>   lim = ACCESS_ONCE(usbfs_memory_mb);
> - if (lim == 0 || lim > (USBFS_XFER_MAX >> 20))
> - lim = USBFS_XFER_MAX;
> - else
> - lim <<= 20;
> + lim <<= 20;
>  
> - atomic_add(amount, _memory_usage);
> - if (atomic_read(_memory_usage) <= lim)
> - return 0;
> - atomic_sub(amount, _memory_usage);
> - return -ENOMEM;
> + atomic64_add(amount, _memory_usage);
> +
> + if (lim > 0) {
> + if (atomic64_read(_memory_usage) <= lim)
> + return 0;
> + atomic64_sub(amount, _memory_usage);
> + return -ENOMEM;
> + }
> +
> + return 0;

I would have written:

if (lim > 0 && atomic64_read(_memory_usage) > lim) {
atomic64_sub(amount, _memory_usage);
return -ENOMEM;
}
return 0;

But that's merely a matter of personal style and taste.

> @@ -1458,6 +1453,7 @@ static int proc_do_submiturb(struct usb_dev_state *ps, 
> struct usbdevfs_urb *uurb
>   int number_of_packets = 0;
>   unsigned int stream_id = 0;
>   void *buf;
> + u32 overhead;
>  
>   if (uurb->flags & ~(USBDEVFS_URB_ISO_ASAP |
>   USBDEVFS_URB_SHORT_NOT_OK |
> @@ -1584,7 +1580,11 @@ static int proc_do_submiturb(struct usb_dev_state *ps, 
> struct usbdevfs_urb *uurb
>   return -EINVAL;
>   }
>  
> - if (uurb->buffer_length >= USBFS_XFER_MAX) {
> + /* check for overflow */
> + overhead = u + sizeof(struct async) + sizeof(struct urb)
> +   + num_sgs * sizeof(struct scatterlist);
> +
> + if (uurb->buffer_length + overhead < uurb->buffer_length) {
>   ret = -EINVAL;
>   goto error;
>   }

I just realized that this part isn't necessary after all.  u is
unsigned, but uurb->buffer_length is a signed integer.  Since num_sgs
is limited to 128, the computation cannot overflow in any case.

Sorry for the confusion.

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] usb: musb: debugfs: allow forcing host mode together with speed in testmode

[Pali Rohár]

On Wednesday 14 December 2016 16:34:31 Tony Lindgren wrote:
> * Pali Rohár  [161214 06:48]:
> 
> We need this because  and this allows debugging problems related
> to ...?

Hi! We have already discussion about it, search for older email with 
Message-Id: <201601231357.32629@pali>

After that in June, Bin wanted from me to resend patch with changed 
subject... but after that I forgot. Now I resent it.

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: [PATCH] usb: musb: debugfs: allow forcing host mode together with speed in testmode

[Tony Lindgren]

* Pali Rohár  [161214 07:40]:
> On Wednesday 14 December 2016 16:34:31 Tony Lindgren wrote:
> > * Pali Rohár  [161214 06:48]:
> > 
> > We need this because  and this allows debugging problems related
> > to ...?
> 
> Hi! We have already discussion about it, search for older email with 
> Message-Id: <201601231357.32629@pali>
> 
> After that in June, Bin wanted from me to resend patch with changed 
> subject... but after that I forgot. Now I resent it.

What I meant is that the patch description is missing :)

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 09/13] USB: serial: ch341: fix line settings after reset-resume

[Johan Hovold]

A recent change added support for modifying the default line-control
settings, but did not make sure that the modified settings were used as
part of reconfiguration after a device has been reset during resume.

This caused a port that was open before suspend to be unusable until
being closed and reopened.

Fixes: ba781bdf8662 ("USB: serial: ch341: add support for parity, frame
length, stop bits")
Signed-off-by: Johan Hovold 
---
 drivers/usb/serial/ch341.c | 18 ++
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
index 2d07fa037f02..e1dd096106d1 100644
--- a/drivers/usb/serial/ch341.c
+++ b/drivers/usb/serial/ch341.c
@@ -95,6 +95,7 @@ struct ch341_private {
unsigned baud_rate; /* set baud rate */
u8 line_control; /* set line control value RTS/DTR */
u8 line_status; /* active status of modem control inputs */
+   u8 lcr;
 };
 
 static void ch341_set_termios(struct tty_struct *tty,
@@ -213,7 +214,6 @@ static int ch341_configure(struct usb_device *dev, struct 
ch341_private *priv)
const unsigned int size = 2;
char *buffer;
int r;
-   u8 lcr;
 
buffer = kmalloc(size, GFP_KERNEL);
if (!buffer)
@@ -229,16 +229,11 @@ static int ch341_configure(struct usb_device *dev, struct 
ch341_private *priv)
if (r < 0)
goto out;
 
-   /*
-* Some CH340 devices appear unable to change the initial LCR
-* settings, so set a sane 8N1 default.
-*/
-   lcr = CH341_LCR_ENABLE_RX | CH341_LCR_ENABLE_TX | CH341_LCR_CS8;
-   r = ch341_control_out(dev, CH341_REQ_WRITE_REG, 0x2518, lcr);
+   r = ch341_control_out(dev, CH341_REQ_WRITE_REG, 0x2518, priv->lcr);
if (r < 0)
goto out;
 
-   r = ch341_init_set_baudrate(dev, priv, lcr);
+   r = ch341_init_set_baudrate(dev, priv, priv->lcr);
if (r < 0)
goto out;
 
@@ -259,6 +254,11 @@ static int ch341_port_probe(struct usb_serial_port *port)
 
spin_lock_init(>lock);
priv->baud_rate = DEFAULT_BAUD_RATE;
+   /*
+* Some CH340 devices appear unable to change the initial LCR
+* settings, so set a sane 8N1 default.
+*/
+   priv->lcr = CH341_LCR_ENABLE_RX | CH341_LCR_ENABLE_TX | CH341_LCR_CS8;
 
r = ch341_configure(port->serial->dev, priv);
if (r < 0)
@@ -399,6 +399,8 @@ static void ch341_set_termios(struct tty_struct *tty,
if (r < 0 && old_termios) {
priv->baud_rate = tty_termios_baud_rate(old_termios);
tty_termios_copy_hw(>termios, old_termios);
+   } else if (r == 0) {
+   priv->lcr = ctrl;
}
}
 
-- 
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: musb: debugfs: allow forcing host mode together with speed in testmode

[Tony Lindgren]

* Pali Rohár  [161214 06:48]:

We need this because  and this allows debugging problems related to
...?

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

Re: [PATCH v4 2/2] USB: usb resume time delay values debug

[Alan Stern]

On Wed, 14 Dec 2016, Todd Brandt wrote:

> add debugfs support for experimenting with USB timing delay
> values on the fly. Values are read/written from debugfs at
> /sys/kernel/debug/usb/timing.
> 
> Signed-off-by: Todd Brandt 
> ---
> v2 changes:
>  - moved the debug code from hub.c to usb.c
>  - use debugfs instead of /sys/kernel/usb

...

> +static int usb_timing_show(struct seq_file *s, void *unused)
> +{
> + seq_printf(s, "tdrsmdn=%d\n", usb_timing.tdrsmdn);
> + seq_printf(s, "trsmrcy=%d\n", usb_timing.trsmrcy);
> + seq_printf(s, "trstrcy=%d\n", usb_timing.trstrcy);
> + return 0;
> +}
> +
> +static int usb_timing_open(struct inode *inode, struct file *file)
> +{
> + return single_open(file, usb_timing_show, inode->i_private);
> +}
> +
> +static ssize_t usb_timing_write(struct file *file,
> + const char __user *ubuf, size_t count, loff_t *ppos)
> +{
> + int val;
> + char buf[32];
> +
> + if (copy_from_user(, ubuf, min_t(size_t, sizeof(buf) - 1, count)))
> + return -EFAULT;
> +
> + if (sscanf(buf, "tdrsmdn=%u", ) == 1)
> + usb_timing.tdrsmdn = max(val, USB_TIMING_TDRSMDN_MIN);
> + else if (sscanf(buf, "trsmrcy=%u", ) == 1)
> + usb_timing.trsmrcy = max(val, USB_TIMING_TRSMRCY_MIN);
> + else if (sscanf(buf, "trstrcy=%u", ) == 1)
> + usb_timing.trstrcy = max(val, USB_TIMING_TRSTRCY_MIN);

Nit: Since the values in usb_timing are signed integers, and since val 
is a signed integer, why do the sscanf calls use %u rather than %d?

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

[Johan Hovold]

On Mon, Dec 12, 2016 at 04:15:29PM -0800, Russell Senior wrote:
> On Mon, Dec 12, 2016 at 10:19 AM, Johan Hovold  wrote:

> > Can you try also the below diagnostics patch on top of usb-next (not
> > usb-linus this time)?
> >
> > It seems we should be able to revert to setting the divisors and lcr
> > directly for all CH340 devices, while using the new method for CH341
> > only (which requires it).
> >
> > Just want to make sure that keeping a common init sequence would still
> > work first.
> >
> > Note that I don't expect you to be able to change word size, but
> > hopefully a default 8N1 will work while the baud rate would be
> > configurable.
> 
> If both sides (ch341 and pl2303) are configured 8N1, your new patch
> works.  When both sides are configured for the same baud rate, it
> works.  Changing ch341 to a different baud rate from the pl2303 is
> stops working, which implies that baud rate changes are working.

Good, thanks for testing. I've just submitted a series that should
enable (basic) support also for your device, but I'll need to your help
to verify it.

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

[PATCH 12/13] USB: serial: ch341: rename shadow modem-control register

[Johan Hovold]

Rename the shadow modem-control register currently named "line_control"
to the less confusing "mcr".

Signed-off-by: Johan Hovold 
---
 drivers/usb/serial/ch341.c | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
index 4c5e61979e38..a742ed98e663 100644
--- a/drivers/usb/serial/ch341.c
+++ b/drivers/usb/serial/ch341.c
@@ -95,7 +95,7 @@ MODULE_DEVICE_TABLE(usb, id_table);
 struct ch341_private {
spinlock_t lock; /* access lock */
unsigned baud_rate; /* set baud rate */
-   u8 line_control; /* set line control value RTS/DTR */
+   u8 mcr;
u8 line_status; /* active status of modem control inputs */
u8 lcr;
 
@@ -258,7 +258,7 @@ static int ch341_configure(struct usb_device *dev, struct 
ch341_private *priv)
if (r < 0)
goto out;
 
-   r = ch341_set_handshake(dev, priv->line_control);
+   r = ch341_set_handshake(dev, priv->mcr);
 
 out:   kfree(buffer);
return r;
@@ -319,11 +319,11 @@ static void ch341_dtr_rts(struct usb_serial_port *port, 
int on)
/* drop DTR and RTS */
spin_lock_irqsave(>lock, flags);
if (on)
-   priv->line_control |= CH341_BIT_RTS | CH341_BIT_DTR;
+   priv->mcr |= CH341_BIT_RTS | CH341_BIT_DTR;
else
-   priv->line_control &= ~(CH341_BIT_RTS | CH341_BIT_DTR);
+   priv->mcr &= ~(CH341_BIT_RTS | CH341_BIT_DTR);
spin_unlock_irqrestore(>lock, flags);
-   ch341_set_handshake(port->serial->dev, priv->line_control);
+   ch341_set_handshake(port->serial->dev, priv->mcr);
 }
 
 static void ch341_close(struct usb_serial_port *port)
@@ -428,12 +428,12 @@ static void ch341_set_termios(struct tty_struct *tty,
 
spin_lock_irqsave(>lock, flags);
if (C_BAUD(tty) == B0)
-   priv->line_control &= ~(CH341_BIT_DTR | CH341_BIT_RTS);
+   priv->mcr &= ~(CH341_BIT_DTR | CH341_BIT_RTS);
else if (old_termios && (old_termios->c_cflag & CBAUD) == B0)
-   priv->line_control |= (CH341_BIT_DTR | CH341_BIT_RTS);
+   priv->mcr |= (CH341_BIT_DTR | CH341_BIT_RTS);
spin_unlock_irqrestore(>lock, flags);
 
-   ch341_set_handshake(port->serial->dev, priv->line_control);
+   ch341_set_handshake(port->serial->dev, priv->mcr);
 }
 
 static void ch341_break_ctl(struct tty_struct *tty, int break_state)
@@ -489,14 +489,14 @@ static int ch341_tiocmset(struct tty_struct *tty,
 
spin_lock_irqsave(>lock, flags);
if (set & TIOCM_RTS)
-   priv->line_control |= CH341_BIT_RTS;
+   priv->mcr |= CH341_BIT_RTS;
if (set & TIOCM_DTR)
-   priv->line_control |= CH341_BIT_DTR;
+   priv->mcr |= CH341_BIT_DTR;
if (clear & TIOCM_RTS)
-   priv->line_control &= ~CH341_BIT_RTS;
+   priv->mcr &= ~CH341_BIT_RTS;
if (clear & TIOCM_DTR)
-   priv->line_control &= ~CH341_BIT_DTR;
-   control = priv->line_control;
+   priv->mcr &= ~CH341_BIT_DTR;
+   control = priv->mcr;
spin_unlock_irqrestore(>lock, flags);
 
return ch341_set_handshake(port->serial->dev, control);
@@ -590,7 +590,7 @@ static int ch341_tiocmget(struct tty_struct *tty)
unsigned int result;
 
spin_lock_irqsave(>lock, flags);
-   mcr = priv->line_control;
+   mcr = priv->mcr;
status = priv->line_status;
spin_unlock_irqrestore(>lock, flags);
 
-- 
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

[PATCH 08/13] USB: serial: ch341: fix control-message error handling

[Johan Hovold]

A short control transfer would currently fail to be detected, something
which could lead to stale buffer data being used as valid input.

Check for short transfers, and make sure to log any transfer errors.

Fixes: 6ce76104781a ("USB: Driver for CH341 USB-serial adaptor")
Signed-off-by: Johan Hovold 
---
 drivers/usb/serial/ch341.c | 32 +---
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
index 74d21171d22a..2d07fa037f02 100644
--- a/drivers/usb/serial/ch341.c
+++ b/drivers/usb/serial/ch341.c
@@ -112,6 +112,8 @@ static int ch341_control_out(struct usb_device *dev, u8 
request,
r = usb_control_msg(dev, usb_sndctrlpipe(dev, 0), request,
USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
value, index, NULL, 0, DEFAULT_TIMEOUT);
+   if (r < 0)
+   dev_err(>dev, "failed to send control message: %d\n", r);
 
return r;
 }
@@ -129,7 +131,20 @@ static int ch341_control_in(struct usb_device *dev,
r = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0), request,
USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
value, index, buf, bufsize, DEFAULT_TIMEOUT);
-   return r;
+   if (r < bufsize) {
+   if (r >= 0) {
+   dev_err(>dev,
+   "short control message received (%d < %u)\n",
+   r, bufsize);
+   r = -EIO;
+   }
+
+   dev_err(>dev, "failed to receive control message: %d\n",
+   r);
+   return r;
+   }
+
+   return 0;
 }
 
 static int ch341_init_set_baudrate(struct usb_device *dev,
@@ -170,9 +185,9 @@ static int ch341_set_handshake(struct usb_device *dev, u8 
control)
 
 static int ch341_get_status(struct usb_device *dev, struct ch341_private *priv)
 {
+   const unsigned int size = 2;
char *buffer;
int r;
-   const unsigned size = 8;
unsigned long flags;
 
buffer = kmalloc(size, GFP_KERNEL);
@@ -183,14 +198,9 @@ static int ch341_get_status(struct usb_device *dev, struct 
ch341_private *priv)
if (r < 0)
goto out;
 
-   /* setup the private status if available */
-   if (r == 2) {
-   r = 0;
-   spin_lock_irqsave(>lock, flags);
-   priv->line_status = (~(*buffer)) & CH341_BITS_MODEM_STAT;
-   spin_unlock_irqrestore(>lock, flags);
-   } else
-   r = -EPROTO;
+   spin_lock_irqsave(>lock, flags);
+   priv->line_status = (~(*buffer)) & CH341_BITS_MODEM_STAT;
+   spin_unlock_irqrestore(>lock, flags);
 
 out:   kfree(buffer);
return r;
@@ -200,9 +210,9 @@ out:kfree(buffer);
 
 static int ch341_configure(struct usb_device *dev, struct ch341_private *priv)
 {
+   const unsigned int size = 2;
char *buffer;
int r;
-   const unsigned size = 8;
u8 lcr;
 
buffer = kmalloc(size, GFP_KERNEL);
-- 
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

[PATCH 05/13] USB: serial: ch341: fix resume after reset

[Johan Hovold]

Fix reset-resume handling which failed to resubmit the read and
interrupt URBs, thereby leaving a port that was open before suspend in a
broken state until closed and reopened.

Fixes: 1ded7ea47b88 ("USB: ch341 serial: fix port number changed after
resume")
Fixes: 2bfd1c96a9fb ("USB: serial: ch341: remove reset_resume callback")
Cc: stable 
Signed-off-by: Johan Hovold 
---
 drivers/usb/serial/ch341.c | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
index 8f41d4385f1c..cbe91b232828 100644
--- a/drivers/usb/serial/ch341.c
+++ b/drivers/usb/serial/ch341.c
@@ -582,14 +582,23 @@ static int ch341_tiocmget(struct tty_struct *tty)
 
 static int ch341_reset_resume(struct usb_serial *serial)
 {
-   struct ch341_private *priv;
-
-   priv = usb_get_serial_port_data(serial->port[0]);
+   struct usb_serial_port *port = serial->port[0];
+   struct ch341_private *priv = usb_get_serial_port_data(port);
+   int ret;
 
/* reconfigure ch341 serial port after bus-reset */
ch341_configure(serial->dev, priv);
 
-   return 0;
+   if (tty_port_initialized(>port)) {
+   ret = usb_submit_urb(port->interrupt_in_urb, GFP_KERNEL);
+   if (ret) {
+   dev_err(>dev, "failed to submit interrupt urb: 
%d\n",
+   ret);
+   return ret;
+   }
+   }
+
+   return usb_serial_generic_resume(serial);
 }
 
 static struct usb_serial_driver ch341_device = {
-- 
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

[PATCH 10/13] USB: serial: ch341: fix baud rate and line-control handling

[Johan Hovold]

Some CH341 devices appear to require the use of the init vendor command
to set the baud rate and line-control register. Specifically, while
using direct register updates for speed and LCR seem to update those
settings correctly, not using the init command causes received data to
be buffered until a full endpoint-size packet (32 bytes) have been
received (i.e. the init command has some undocumented side-effect we
need).

On the other hand, some CH340 devices have been reported to require the
use of direct register manipulations to set the line speed, while not
suffering from the above mentioned buffering effect.

Let's use the init vendor command only for CH341 devices to be able to
support also such (quirky?) CH340 devices.

Signed-off-by: Johan Hovold 
---
 drivers/usb/serial/ch341.c | 46 +++---
 1 file changed, 35 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
index e1dd096106d1..05fd32a52048 100644
--- a/drivers/usb/serial/ch341.c
+++ b/drivers/usb/serial/ch341.c
@@ -82,10 +82,12 @@
 #define CH341_LCR_CS6  0x01
 #define CH341_LCR_CS5  0x00
 
+#define CH341_QUIRK_INIT   BIT(0)
+
 static const struct usb_device_id id_table[] = {
{ USB_DEVICE(0x4348, 0x5523) },
{ USB_DEVICE(0x1a86, 0x7523) },
-   { USB_DEVICE(0x1a86, 0x5523) },
+   { USB_DEVICE(0x1a86, 0x5523), .driver_info = CH341_QUIRK_INIT },
{ },
 };
 MODULE_DEVICE_TABLE(usb, id_table);
@@ -96,6 +98,8 @@ struct ch341_private {
u8 line_control; /* set line control value RTS/DTR */
u8 line_status; /* active status of modem control inputs */
u8 lcr;
+
+   unsigned long quirks;
 };
 
 static void ch341_set_termios(struct tty_struct *tty,
@@ -148,7 +152,7 @@ static int ch341_control_in(struct usb_device *dev,
return 0;
 }
 
-static int ch341_init_set_baudrate(struct usb_device *dev,
+static int ch341_set_baudrate_lcr(struct usb_device *dev,
   struct ch341_private *priv, unsigned ctrl)
 {
short a;
@@ -172,9 +176,23 @@ static int ch341_init_set_baudrate(struct usb_device *dev,
factor = 0x1 - factor;
a = (factor & 0xff00) | divisor;
 
-   /* 0x9c is "enable SFR_UART Control register and timer" */
-   r = ch341_control_out(dev, CH341_REQ_SERIAL_INIT,
- 0x9c | (ctrl << 8), a | 0x80);
+   /*
+* Some CH341 devices require us to use the init vendor command to set
+* the baud rate and LCR.
+*/
+   if (priv->quirks & CH341_QUIRK_INIT) {
+   /* 0x9c is "enable SFR_UART Control register and timer" */
+   r = ch341_control_out(dev, CH341_REQ_SERIAL_INIT,
+ 0x9c | (ctrl << 8), a | 0x80);
+   } else {
+   r = ch341_control_out(dev, CH341_REQ_WRITE_REG, 0x1312, a);
+   if (r)
+   return r;
+
+   r = ch341_control_out(dev, CH341_REQ_WRITE_REG, 0x2518, ctrl);
+   if (r)
+   return r;
+   }
 
return r;
 }
@@ -209,6 +227,14 @@ out:   kfree(buffer);
 
 /* -- 
*/
 
+static int ch341_probe(struct usb_serial *serial,
+  const struct usb_device_id *id)
+{
+   usb_set_serial_data(serial, (void *)id->driver_info);
+
+   return 0;
+}
+
 static int ch341_configure(struct usb_device *dev, struct ch341_private *priv)
 {
const unsigned int size = 2;
@@ -229,11 +255,7 @@ static int ch341_configure(struct usb_device *dev, struct 
ch341_private *priv)
if (r < 0)
goto out;
 
-   r = ch341_control_out(dev, CH341_REQ_WRITE_REG, 0x2518, priv->lcr);
-   if (r < 0)
-   goto out;
-
-   r = ch341_init_set_baudrate(dev, priv, priv->lcr);
+   r = ch341_set_baudrate_lcr(dev, priv, priv->lcr);
if (r < 0)
goto out;
 
@@ -259,6 +281,7 @@ static int ch341_port_probe(struct usb_serial_port *port)
 * settings, so set a sane 8N1 default.
 */
priv->lcr = CH341_LCR_ENABLE_RX | CH341_LCR_ENABLE_TX | CH341_LCR_CS8;
+   priv->quirks = (unsigned long)usb_get_serial_data(port->serial);
 
r = ch341_configure(port->serial->dev, priv);
if (r < 0)
@@ -395,7 +418,7 @@ static void ch341_set_termios(struct tty_struct *tty,
if (baud_rate) {
priv->baud_rate = baud_rate;
 
-   r = ch341_init_set_baudrate(port->serial->dev, priv, ctrl);
+   r = ch341_set_baudrate_lcr(port->serial->dev, priv, ctrl);
if (r < 0 && old_termios) {
priv->baud_rate = tty_termios_baud_rate(old_termios);
tty_termios_copy_hw(>termios, old_termios);
@@ -628,6 +651,7 @@ static struct usb_serial_driver ch341_device = {

[PATCH 11/13] USB: serial: ch341: clean up control debug messages

[Johan Hovold]

Clean up the control-transfer debug messages by dropping redundant
information and unnecessary casts.

Signed-off-by: Johan Hovold 
---
 drivers/usb/serial/ch341.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
index 05fd32a52048..4c5e61979e38 100644
--- a/drivers/usb/serial/ch341.c
+++ b/drivers/usb/serial/ch341.c
@@ -111,8 +111,8 @@ static int ch341_control_out(struct usb_device *dev, u8 
request,
 {
int r;
 
-   dev_dbg(>dev, "ch341_control_out(%02x,%02x,%04x,%04x)\n",
-   USB_DIR_OUT|0x40, (int)request, (int)value, (int)index);
+   dev_dbg(>dev, "%s - (%02x,%04x,%04x)\n", __func__,
+   request, value, index);
 
r = usb_control_msg(dev, usb_sndctrlpipe(dev, 0), request,
USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
@@ -129,9 +129,8 @@ static int ch341_control_in(struct usb_device *dev,
 {
int r;
 
-   dev_dbg(>dev, "ch341_control_in(%02x,%02x,%04x,%04x,%p,%u)\n",
-   USB_DIR_IN|0x40, (int)request, (int)value, (int)index, buf,
-   (int)bufsize);
+   dev_dbg(>dev, "%s - (%02x,%04x,%04x,%u)\n", __func__,
+   request, value, index, bufsize);
 
r = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0), request,
USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
-- 
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

[PATCH 07/13] USB: serial: ch341: fix modem-status handling

[Johan Hovold]

The modem-status register was read as part of device configuration at
port_probe and then again at open (and reset-resume). During open (and
reset-resume) the MSR was read before submitting the interrupt URB,
something which could lead to an MSR-change going unnoticed when it
races with open (reset-resume).

Fix this by dropping the redundant reconfiguration of the port at every
open, and only read the MSR after the interrupt URB has been submitted.

Fixes: 664d5df92e88 ("USB: usb-serial ch341: support for DTR/RTS/CTS")
Signed-off-by: Johan Hovold 
---
 drivers/usb/serial/ch341.c | 27 ---
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
index 3d86272a4b31..74d21171d22a 100644
--- a/drivers/usb/serial/ch341.c
+++ b/drivers/usb/serial/ch341.c
@@ -228,21 +228,11 @@ static int ch341_configure(struct usb_device *dev, struct 
ch341_private *priv)
if (r < 0)
goto out;
 
-   /* expect 0xff 0xee */
-   r = ch341_get_status(dev, priv);
-   if (r < 0)
-   goto out;
-
r = ch341_init_set_baudrate(dev, priv, lcr);
if (r < 0)
goto out;
 
r = ch341_set_handshake(dev, priv->line_control);
-   if (r < 0)
-   goto out;
-
-   /* expect 0x9f 0xee */
-   r = ch341_get_status(dev, priv);
 
 out:   kfree(buffer);
return r;
@@ -314,14 +304,9 @@ static void ch341_close(struct usb_serial_port *port)
 /* open this device, set default parameters */
 static int ch341_open(struct tty_struct *tty, struct usb_serial_port *port)
 {
-   struct usb_serial *serial = port->serial;
struct ch341_private *priv = usb_get_serial_port_data(port);
int r;
 
-   r = ch341_configure(serial->dev, priv);
-   if (r)
-   return r;
-
if (tty)
ch341_set_termios(tty, port, NULL);
 
@@ -333,6 +318,12 @@ static int ch341_open(struct tty_struct *tty, struct 
usb_serial_port *port)
return r;
}
 
+   r = ch341_get_status(port->serial->dev, priv);
+   if (r < 0) {
+   dev_err(>dev, "failed to read modem status: %d\n", r);
+   goto err_kill_interrupt_urb;
+   }
+
r = usb_serial_generic_open(tty, port);
if (r)
goto err_kill_interrupt_urb;
@@ -597,6 +588,12 @@ static int ch341_reset_resume(struct usb_serial *serial)
ret);
return ret;
}
+
+   ret = ch341_get_status(port->serial->dev, priv);
+   if (ret < 0) {
+   dev_err(>dev, "failed to read modem status: %d\n",
+   ret);
+   }
}
 
return usb_serial_generic_resume(serial);
-- 
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

[PATCH 06/13] USB: serial: ch341: fix initial line settings

[Johan Hovold]

The ch341 driver is based on reverse-engineering and contains some bits
which appear to be redundant and some which appear incompatible which
certain devices.

Specifically, some CH340 devices seem unable to change the initial line
settings, which have so far been set to 5N1. Let's use a more reasonable
8N1 default instead.

Note that we also need to set the ENABLE_RX bit or receive will be
disabled after a reset during resume.

Also drop a redundant LCR-register read.

Fixes: 6ce76104781a ("USB: Driver for CH341 USB-serial adaptor")
Cc: stable 
Signed-off-by: Johan Hovold 
---
 drivers/usb/serial/ch341.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
index cbe91b232828..3d86272a4b31 100644
--- a/drivers/usb/serial/ch341.c
+++ b/drivers/usb/serial/ch341.c
@@ -203,6 +203,7 @@ static int ch341_configure(struct usb_device *dev, struct 
ch341_private *priv)
char *buffer;
int r;
const unsigned size = 8;
+   u8 lcr;
 
buffer = kmalloc(size, GFP_KERNEL);
if (!buffer)
@@ -218,12 +219,12 @@ static int ch341_configure(struct usb_device *dev, struct 
ch341_private *priv)
if (r < 0)
goto out;
 
-   /* expect two bytes 0x56 0x00 */
-   r = ch341_control_in(dev, CH341_REQ_READ_REG, 0x2518, 0, buffer, size);
-   if (r < 0)
-   goto out;
-
-   r = ch341_control_out(dev, CH341_REQ_WRITE_REG, 0x2518, 0x0050);
+   /*
+* Some CH340 devices appear unable to change the initial LCR
+* settings, so set a sane 8N1 default.
+*/
+   lcr = CH341_LCR_ENABLE_RX | CH341_LCR_ENABLE_TX | CH341_LCR_CS8;
+   r = ch341_control_out(dev, CH341_REQ_WRITE_REG, 0x2518, lcr);
if (r < 0)
goto out;
 
@@ -232,7 +233,7 @@ static int ch341_configure(struct usb_device *dev, struct 
ch341_private *priv)
if (r < 0)
goto out;
 
-   r = ch341_init_set_baudrate(dev, priv, 0);
+   r = ch341_init_set_baudrate(dev, priv, lcr);
if (r < 0)
goto out;
 
-- 
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

[PATCH 13/13] USB: serial: ch341: rename modem-status register

[Johan Hovold]

Rename the shadow modem-status register currently named "line_status" to
the less confusing "msr".

Also rename the helper function used to parse the interrupt data.

Signed-off-by: Johan Hovold 
---
 drivers/usb/serial/ch341.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
index a742ed98e663..42eadee56a13 100644
--- a/drivers/usb/serial/ch341.c
+++ b/drivers/usb/serial/ch341.c
@@ -96,7 +96,7 @@ struct ch341_private {
spinlock_t lock; /* access lock */
unsigned baud_rate; /* set baud rate */
u8 mcr;
-   u8 line_status; /* active status of modem control inputs */
+   u8 msr;
u8 lcr;
 
unsigned long quirks;
@@ -217,7 +217,7 @@ static int ch341_get_status(struct usb_device *dev, struct 
ch341_private *priv)
goto out;
 
spin_lock_irqsave(>lock, flags);
-   priv->line_status = (~(*buffer)) & CH341_BITS_MODEM_STAT;
+   priv->msr = (~(*buffer)) & CH341_BITS_MODEM_STAT;
spin_unlock_irqrestore(>lock, flags);
 
 out:   kfree(buffer);
@@ -306,7 +306,7 @@ static int ch341_port_remove(struct usb_serial_port *port)
 static int ch341_carrier_raised(struct usb_serial_port *port)
 {
struct ch341_private *priv = usb_get_serial_port_data(port);
-   if (priv->line_status & CH341_BIT_DCD)
+   if (priv->msr & CH341_BIT_DCD)
return 1;
return 0;
 }
@@ -502,7 +502,7 @@ static int ch341_tiocmset(struct tty_struct *tty,
return ch341_set_handshake(port->serial->dev, control);
 }
 
-static void ch341_update_line_status(struct usb_serial_port *port,
+static void ch341_update_status(struct usb_serial_port *port,
unsigned char *data, size_t len)
 {
struct ch341_private *priv = usb_get_serial_port_data(port);
@@ -517,8 +517,8 @@ static void ch341_update_line_status(struct usb_serial_port 
*port,
status = ~data[2] & CH341_BITS_MODEM_STAT;
 
spin_lock_irqsave(>lock, flags);
-   delta = status ^ priv->line_status;
-   priv->line_status = status;
+   delta = status ^ priv->msr;
+   priv->msr = status;
spin_unlock_irqrestore(>lock, flags);
 
if (data[1] & CH341_MULT_STAT)
@@ -571,7 +571,7 @@ static void ch341_read_int_callback(struct urb *urb)
}
 
usb_serial_debug_data(>dev, __func__, len, data);
-   ch341_update_line_status(port, data, len);
+   ch341_update_status(port, data, len);
 exit:
status = usb_submit_urb(urb, GFP_ATOMIC);
if (status) {
@@ -591,7 +591,7 @@ static int ch341_tiocmget(struct tty_struct *tty)
 
spin_lock_irqsave(>lock, flags);
mcr = priv->mcr;
-   status = priv->line_status;
+   status = priv->msr;
spin_unlock_irqrestore(>lock, flags);
 
result = ((mcr & CH341_BIT_DTR) ? TIOCM_DTR : 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

[PATCH 01/13] USB: serial: ch341: fix initial modem-control state

[Johan Hovold]

DTR and RST will be asserted by the tty-layer when the port is opened
and deasserted on close (if HUPCL is set). Make sure the initial state
is not-asserted before the port is first opened as well.

Fixes: 6ce76104781a ("USB: Driver for CH341 USB-serial adaptor")
Cc: stable 
Signed-off-by: Johan Hovold 
---
 drivers/usb/serial/ch341.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
index 2597b83a8ae2..d133e72fe888 100644
--- a/drivers/usb/serial/ch341.c
+++ b/drivers/usb/serial/ch341.c
@@ -258,7 +258,6 @@ static int ch341_port_probe(struct usb_serial_port *port)
 
spin_lock_init(>lock);
priv->baud_rate = DEFAULT_BAUD_RATE;
-   priv->line_control = CH341_BIT_RTS | CH341_BIT_DTR;
 
r = ch341_configure(port->serial->dev, priv);
if (r < 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

[PATCH 03/13] USB: serial: ch341: fix modem-control and B0 handling

[Johan Hovold]

The modem-control signals are managed by the tty-layer during open and
should not be asserted prematurely when set_termios is called from
driver open.

Also make sure that the signals are asserted only when changing speed
from B0.

Fixes: 664d5df92e88 ("USB: usb-serial ch341: support for DTR/RTS/CTS")
Cc: stable 
Signed-off-by: Johan Hovold 
---
 drivers/usb/serial/ch341.c | 16 +++-
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
index 6279df905c14..0cc5056b304d 100644
--- a/drivers/usb/serial/ch341.c
+++ b/drivers/usb/serial/ch341.c
@@ -384,10 +384,6 @@ static void ch341_set_termios(struct tty_struct *tty,
ctrl |= CH341_LCR_STOP_BITS_2;
 
if (baud_rate) {
-   spin_lock_irqsave(>lock, flags);
-   priv->line_control |= (CH341_BIT_DTR | CH341_BIT_RTS);
-   spin_unlock_irqrestore(>lock, flags);
-
priv->baud_rate = baud_rate;
 
r = ch341_init_set_baudrate(port->serial->dev, priv, ctrl);
@@ -395,14 +391,16 @@ static void ch341_set_termios(struct tty_struct *tty,
priv->baud_rate = tty_termios_baud_rate(old_termios);
tty_termios_copy_hw(>termios, old_termios);
}
-   } else {
-   spin_lock_irqsave(>lock, flags);
-   priv->line_control &= ~(CH341_BIT_DTR | CH341_BIT_RTS);
-   spin_unlock_irqrestore(>lock, flags);
}
 
-   ch341_set_handshake(port->serial->dev, priv->line_control);
+   spin_lock_irqsave(>lock, flags);
+   if (C_BAUD(tty) == B0)
+   priv->line_control &= ~(CH341_BIT_DTR | CH341_BIT_RTS);
+   else if (old_termios && (old_termios->c_cflag & CBAUD) == B0)
+   priv->line_control |= (CH341_BIT_DTR | CH341_BIT_RTS);
+   spin_unlock_irqrestore(>lock, flags);
 
+   ch341_set_handshake(port->serial->dev, priv->line_control);
 }
 
 static void ch341_break_ctl(struct tty_struct *tty, int break_state)
-- 
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

[PATCH 00/13] USB: serial: ch341: fixes and improved CH340 support

[Johan Hovold]

This series fixes a number of problems with the ch341 driver, and adds
support for a class of CH340 devices which does not seem to support
the new method of configuring the line settings (using the init vendor
command).

These patches have been verified against CH340G and CH341A, but I need
help with testing the quirky class of CH340 devices (which Russel and
possibly also Eddie has), and any other devices which you may have
access too.

The series is against my usb-next branch

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

Thanks,
Johan


Johan Hovold (13):
  USB: serial: ch341: fix initial modem-control state
  USB: serial: ch341: fix open and resume after B0
  USB: serial: ch341: fix modem-control and B0 handling
  USB: serial: ch341: fix open error handling
  USB: serial: ch341: fix resume after reset
  USB: serial: ch341: fix initial line settings
  USB: serial: ch341: fix modem-status handling
  USB: serial: ch341: fix control-message error handling
  USB: serial: ch341: fix line settings after reset-resume
  USB: serial: ch341: fix baud rate and line-control handling
  USB: serial: ch341: clean up control debug messages
  USB: serial: ch341: rename shadow modem-control register
  USB: serial: ch341: rename modem-status register

 drivers/usb/serial/ch341.c | 210 -
 1 file changed, 129 insertions(+), 81 deletions(-)

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

[PATCH 02/13] USB: serial: ch341: fix open and resume after B0

[Johan Hovold]

The private baud_rate variable is used to configure the port at open and
reset-resume, which means it must never be set to (and left at) zero or
reset-resume and all further open attempts will fail.

Fixes: 664d5df92e88 ("USB: usb-serial ch341: support for DTR/RTS/CTS")
Cc: stable 
Signed-off-by: Johan Hovold 
---
 drivers/usb/serial/ch341.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
index d133e72fe888..6279df905c14 100644
--- a/drivers/usb/serial/ch341.c
+++ b/drivers/usb/serial/ch341.c
@@ -355,7 +355,6 @@ static void ch341_set_termios(struct tty_struct *tty,
 
baud_rate = tty_get_baud_rate(tty);
 
-   priv->baud_rate = baud_rate;
ctrl = CH341_LCR_ENABLE_RX | CH341_LCR_ENABLE_TX;
 
switch (C_CSIZE(tty)) {
@@ -388,6 +387,9 @@ static void ch341_set_termios(struct tty_struct *tty,
spin_lock_irqsave(>lock, flags);
priv->line_control |= (CH341_BIT_DTR | CH341_BIT_RTS);
spin_unlock_irqrestore(>lock, flags);
+
+   priv->baud_rate = baud_rate;
+
r = ch341_init_set_baudrate(port->serial->dev, priv, ctrl);
if (r < 0 && old_termios) {
priv->baud_rate = tty_termios_baud_rate(old_termios);
-- 
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

[PATCH] usb: musb: debugfs: allow forcing host mode together with speed in testmode

[Pali Rohár]

Signed-off-by: Pali Rohár 
---
 drivers/usb/musb/musb_debugfs.c |   44 +--
 1 file changed, 28 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/musb/musb_debugfs.c b/drivers/usb/musb/musb_debugfs.c
index 9b22d94..62c13a2 100644
--- a/drivers/usb/musb/musb_debugfs.c
+++ b/drivers/usb/musb/musb_debugfs.c
@@ -147,28 +147,34 @@ static int musb_test_mode_show(struct seq_file *s, void 
*unused)
 
test = musb_readb(musb->mregs, MUSB_TESTMODE);
 
-   if (test & MUSB_TEST_FORCE_HOST)
+   if (test & (MUSB_TEST_FORCE_HOST | MUSB_TEST_FORCE_FS))
+   seq_printf(s, "force host full-speed\n");
+
+   else if (test & (MUSB_TEST_FORCE_HOST | MUSB_TEST_FORCE_HS))
+   seq_printf(s, "force host high-speed\n");
+
+   else if (test & MUSB_TEST_FORCE_HOST)
seq_printf(s, "force host\n");
 
-   if (test & MUSB_TEST_FIFO_ACCESS)
+   else if (test & MUSB_TEST_FIFO_ACCESS)
seq_printf(s, "fifo access\n");
 
-   if (test & MUSB_TEST_FORCE_FS)
+   else if (test & MUSB_TEST_FORCE_FS)
seq_printf(s, "force full-speed\n");
 
-   if (test & MUSB_TEST_FORCE_HS)
+   else if (test & MUSB_TEST_FORCE_HS)
seq_printf(s, "force high-speed\n");
 
-   if (test & MUSB_TEST_PACKET)
+   else if (test & MUSB_TEST_PACKET)
seq_printf(s, "test packet\n");
 
-   if (test & MUSB_TEST_K)
+   else if (test & MUSB_TEST_K)
seq_printf(s, "test K\n");
 
-   if (test & MUSB_TEST_J)
+   else if (test & MUSB_TEST_J)
seq_printf(s, "test J\n");
 
-   if (test & MUSB_TEST_SE0_NAK)
+   else if (test & MUSB_TEST_SE0_NAK)
seq_printf(s, "test SE0 NAK\n");
 
return 0;
@@ -206,30 +212,36 @@ static ssize_t musb_test_mode_write(struct file *file,
if (copy_from_user(buf, ubuf, min_t(size_t, sizeof(buf) - 1, count)))
return -EFAULT;
 
-   if (strstarts(buf, "force host"))
+   if (strstarts(buf, "force host full-speed"))
+   test = MUSB_TEST_FORCE_HOST | MUSB_TEST_FORCE_FS;
+
+   else if (strstarts(buf, "force host high-speed"))
+   test = MUSB_TEST_FORCE_HOST | MUSB_TEST_FORCE_HS;
+
+   else if (strstarts(buf, "force host"))
test = MUSB_TEST_FORCE_HOST;
 
-   if (strstarts(buf, "fifo access"))
+   else if (strstarts(buf, "fifo access"))
test = MUSB_TEST_FIFO_ACCESS;
 
-   if (strstarts(buf, "force full-speed"))
+   else if (strstarts(buf, "force full-speed"))
test = MUSB_TEST_FORCE_FS;
 
-   if (strstarts(buf, "force high-speed"))
+   else if (strstarts(buf, "force high-speed"))
test = MUSB_TEST_FORCE_HS;
 
-   if (strstarts(buf, "test packet")) {
+   else if (strstarts(buf, "test packet")) {
test = MUSB_TEST_PACKET;
musb_load_testpacket(musb);
}
 
-   if (strstarts(buf, "test K"))
+   else if (strstarts(buf, "test K"))
test = MUSB_TEST_K;
 
-   if (strstarts(buf, "test J"))
+   else if (strstarts(buf, "test J"))
test = MUSB_TEST_J;
 
-   if (strstarts(buf, "test SE0 NAK"))
+   else if (strstarts(buf, "test SE0 NAK"))
test = MUSB_TEST_SE0_NAK;
 
musb_writeb(musb->mregs, MUSB_TESTMODE, test);
-- 
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] usb: hub: Move hub_port_disable() to fix warning if PM is disabled

[Geert Uytterhoeven]

If CONFIG_PM=n:

drivers/usb/core/hub.c:107: warning: ‘hub_usb3_port_prepare_disable’ 
declared inline after being called
drivers/usb/core/hub.c:107: warning: previous declaration of 
‘hub_usb3_port_prepare_disable’ was here

To fix this, move hub_port_disable() after
hub_usb3_port_prepare_disable(), and adjust forward declarations.

Fixes: 37be66767e3cae4f ("usb: hub: Fix auto-remount of safely removed or 
ejected USB-3 devices")
Signed-off-by: Geert Uytterhoeven 
---
 drivers/usb/core/hub.c | 59 +-
 1 file changed, 29 insertions(+), 30 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 143454ea385b72ff..20573a739d2681a9 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -103,8 +103,7 @@
 
 static void hub_release(struct kref *kref);
 static int usb_reset_and_verify_device(struct usb_device *udev);
-static void hub_usb3_port_prepare_disable(struct usb_hub *hub,
- struct usb_port *port_dev);
+static int hub_port_disable(struct usb_hub *hub, int port1, int set_state);
 
 static inline char *portspeed(struct usb_hub *hub, int portstatus)
 {
@@ -903,34 +902,6 @@ static int hub_set_port_link_state(struct usb_hub *hub, 
int port1,
 }
 
 /*
- * USB-3 does not have a similar link state as USB-2 that will avoid 
negotiating
- * a connection with a plugged-in cable but will signal the host when the cable
- * is unplugged. Disable remote wake and set link state to U3 for USB-3 devices
- */
-static int hub_port_disable(struct usb_hub *hub, int port1, int set_state)
-{
-   struct usb_port *port_dev = hub->ports[port1 - 1];
-   struct usb_device *hdev = hub->hdev;
-   int ret = 0;
-
-   if (!hub->error) {
-   if (hub_is_superspeed(hub->hdev)) {
-   hub_usb3_port_prepare_disable(hub, port_dev);
-   ret = hub_set_port_link_state(hub, port_dev->portnum,
- USB_SS_PORT_LS_U3);
-   } else {
-   ret = usb_clear_port_feature(hdev, port1,
-   USB_PORT_FEAT_ENABLE);
-   }
-   }
-   if (port_dev->child && set_state)
-   usb_set_device_state(port_dev->child, USB_STATE_NOTATTACHED);
-   if (ret && ret != -ENODEV)
-   dev_err(_dev->dev, "cannot disable (err = %d)\n", ret);
-   return ret;
-}
-
-/*
  * Disable a port and mark a logical connect-change event, so that some
  * time later hub_wq will disconnect() any existing usb_device on the port
  * and will re-enumerate if there actually is a device attached.
@@ -4162,6 +4133,34 @@ static int hub_handle_remote_wakeup(struct usb_hub *hub, 
unsigned int port,
 
 #endif /* CONFIG_PM */
 
+/*
+ * USB-3 does not have a similar link state as USB-2 that will avoid 
negotiating
+ * a connection with a plugged-in cable but will signal the host when the cable
+ * is unplugged. Disable remote wake and set link state to U3 for USB-3 devices
+ */
+static int hub_port_disable(struct usb_hub *hub, int port1, int set_state)
+{
+   struct usb_port *port_dev = hub->ports[port1 - 1];
+   struct usb_device *hdev = hub->hdev;
+   int ret = 0;
+
+   if (!hub->error) {
+   if (hub_is_superspeed(hub->hdev)) {
+   hub_usb3_port_prepare_disable(hub, port_dev);
+   ret = hub_set_port_link_state(hub, port_dev->portnum,
+ USB_SS_PORT_LS_U3);
+   } else {
+   ret = usb_clear_port_feature(hdev, port1,
+   USB_PORT_FEAT_ENABLE);
+   }
+   }
+   if (port_dev->child && set_state)
+   usb_set_device_state(port_dev->child, USB_STATE_NOTATTACHED);
+   if (ret && ret != -ENODEV)
+   dev_err(_dev->dev, "cannot disable (err = %d)\n", ret);
+   return ret;
+}
+
 
 /* USB 2.0 spec, 7.1.7.3 / fig 7-29:
  *
-- 
1.9.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: xhci_reset_endpoint() doesn't reset endpoint

[Mathias Nyman]

On 14.12.2016 12:58, Michal Necasek wrote:

prior to the endpoint reset. SetFeature(CLEAR_HALT) resets the toggle
on the device, but not on the host. But we know for a fact that the
device sends a packet (with data toggle 0) which the host USB stack
never sees, and a data toggle mismatch explains that quite well.

We are using USBFS to talk to the printer, but that shouldn't matter
much. I will note that the available documentation<1> explicitly says
that USBDEVFS_RESETEP and USBDEVFS_CLEAR_HALT both reset the data
toggle. That is indeed the case for the Linux EHCI driver but not
xHCI. Both of the USBFS IOCTLs call into xhci_reset_endpoint() which
does nothing.



This is very likely the case.

xhci can not reset the host side of the endpoint unless it really is halted.
xhci 4.6.8:

"If the endpoint is not in the Halted state when an Reset Endpoint  Command
is executed -The xHC shall reject the command and generate a  Command
Completion Event with the Completion Code set to Context State Error."


Normal halt/stall case is that xhci receives a STALL from the device,
and immediately resets the endpoint (clears toggle, host side) then
propagates the HALT status to usb core.
USB core then sends SetFeature(CLEAR_HALT) to the device which will reset the
toggle for the device side of the endpoint, and host and device toggles
will be in sync.

After this xhci_endpoint_reset() is called by usb core to inform xhci that the
endpoint was reset, but currently we don't do anything in it.

If SetFeature(CLEAR_HALT) is called without endpoint actually being HALTED we 
can not
reset it from xhci. we should issue a config endpoint command to reset the host 
side
toggle, as mentioned in xhci 1.0 120814 as a last note:

"Note: The Reset Endpoint Command may only be issued to endpoints in the Halted 
state.
If software wishes reset the Data Toggle or Sequence Number of an endpoint that 
isn't
in the Halted state, then software may issue a Configure Endpoint Command with 
the Drop
and Add bits set for the target endpoint. that is in the Stopped state."

There was a case with a scanner we believed had the same issue, and we tried to
resolve it by issuing the configure endpoint command in xhci_endpoint_reset() 
but
if I remember correctly It did not resolve the case and code never got anywhere.

I might have some really old implementation somewhere for this, at least there 
is
a really old and outdated hack at


git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git ep_reset_halt_test
https://git.kernel.org/cgit/linux/kernel/git/mnyman/xhci.git/log/?h=ep_reset_halt_test

which really is quite a hack, and based on 3.19 kernel so it's probably only 
useful
as an Idea to base a real solution on.

-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 3/3] xhci: Remove busy loop from xhci_abort_cmd_ring()

[OGAWA Hirofumi]

Mathias Nyman  writes:

> On 14.12.2016 01:40, OGAWA Hirofumi wrote:
>> ping about [PATCH 1/3, 2/3, 3/3]?
>>
>
> 1/3 and 2/3 will be sent to 4.10 usb-linus after rc1,
> 3/3 maybe to usb-next after that

Thanks.
-- 
OGAWA Hirofumi 
--
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/3] xhci: Remove busy loop from xhci_abort_cmd_ring()

[Mathias Nyman]

On 14.12.2016 01:40, OGAWA Hirofumi wrote:

ping about [PATCH 1/3, 2/3, 3/3]?



1/3 and 2/3 will be sent to 4.10 usb-linus after rc1,
3/3 maybe to usb-next after that

-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

[PATCH v4 0/2] USB: resume time optimization by using spec minimums

[Todd Brandt]

The USB resume code in the kernel currently uses a set of hard coded
delay values that are defined in the USB 2.0 spec. Specifically these
three have the most effect on resume time:

 - tdrsmdn: resume signal time (20ms - infinity) usb 2.0 spec 7.1.7.7
 - trsmrcy: resume recovery time (10ms) usb 2.0 spec 7.1.7.7
 - trstrcy: reset recovery time (0ms - infinity) usb 2.0 spec 7.1.7.5

These values have been padded considerably in order to accomodate
non-compliant devices.

 - tdrsmdn: resume signal time = 40ms
 - trsmrcy: resume recovery time = 10ms
 - trstrcy: reset recovery time = 50ms

I propose that we provide a kernel parameter that sets the USB timing
values to their spec minimums. The following patches do this by creating
a global struct which contains these values. By default the values remain
as they are now, but if usbcore.timing_minimum=1 is added to the kernel cmd
line they're set to their minimums. This struct can be expanded over time
to include other hardcoded values that have padding we can remove.

I've created a blog entry on 01.org with some analyze_suspend test
cases illustrating the benefits:

 - 
https://01.org/suspendresume/blogs/tebrandt/2016/usb-resume-optimization-using-spec-minimum-delays

Todd Brandt (2):
  USB: add switch to turn off padding of resume time delays
  USB: usb resume time delay values debug

 Documentation/admin-guide/kernel-parameters.txt |  7 +++
 drivers/usb/common/common.c |  7 +++
 drivers/usb/core/hub.c  | 12 ++---
 drivers/usb/core/usb.c  | 64 +
 drivers/usb/dwc2/hcd.c  |  2 +-
 drivers/usb/host/ehci-hcd.c |  4 +-
 drivers/usb/host/ehci-hub.c |  6 +--
 drivers/usb/host/fotg210-hcd.c  |  2 +-
 drivers/usb/host/isp116x-hcd.c  |  2 +-
 drivers/usb/host/isp1362-hcd.c  |  2 +-
 drivers/usb/host/ohci-hub.c |  2 +-
 drivers/usb/host/oxu210hp-hcd.c |  4 +-
 drivers/usb/host/r8a66597-hcd.c |  2 +-
 drivers/usb/host/sl811-hcd.c|  2 +-
 drivers/usb/host/uhci-hub.c |  6 +--
 drivers/usb/host/xhci-hub.c |  6 +--
 drivers/usb/host/xhci-ring.c|  2 +-
 drivers/usb/isp1760/isp1760-hcd.c   |  2 +-
 drivers/usb/musb/musb_core.c|  6 +--
 drivers/usb/musb/musb_virthub.c |  2 +-
 include/linux/usb.h | 23 -
 21 files changed, 132 insertions(+), 33 deletions(-)

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

[PATCH v4 1/2] USB: add switch to turn off padding of resume time delays

[Todd Brandt]

Add a kernel parameter that replaces the USB_RESUME_TIMEOUT
and other hardcoded delay numbers with the USB spec minimums.

The USB subsystem currently uses heavily padded values for TDRSMDN
and TRSTRCY. This patch keeps the current values by default, but if
the kernel is booted with usb_timing_minimum=1 they are set to the
spec minimums with no padding. The result is significant performance
improvement in usb device resume.

Example analyze_suspend runs are provided here showing the benefits:
https://01.org/suspendresume/blogs/tebrandt/2016/usb-resume-optimization-using-spec-minimum-delays

Signed-off-by: Todd Brandt 
---
v2 changes:
 - moved the core code from hub.c to usb.c
 - param name is now usb_timing_minimum
 - configured isp1362-hcd and ohci-hub to use the new values

v3 changes:
 - changed param to usbcore.timing_minimum

v4 changes:
 - moved usb_timing object to usb-common

 Documentation/admin-guide/kernel-parameters.txt |  7 +++
 drivers/usb/common/common.c |  7 +++
 drivers/usb/core/hub.c  | 12 ++--
 drivers/usb/core/usb.c  |  9 +
 drivers/usb/dwc2/hcd.c  |  2 +-
 drivers/usb/host/ehci-hcd.c |  4 ++--
 drivers/usb/host/ehci-hub.c |  6 +++---
 drivers/usb/host/fotg210-hcd.c  |  2 +-
 drivers/usb/host/isp116x-hcd.c  |  2 +-
 drivers/usb/host/isp1362-hcd.c  |  2 +-
 drivers/usb/host/ohci-hub.c |  2 +-
 drivers/usb/host/oxu210hp-hcd.c |  4 ++--
 drivers/usb/host/r8a66597-hcd.c |  2 +-
 drivers/usb/host/sl811-hcd.c|  2 +-
 drivers/usb/host/uhci-hub.c |  6 +++---
 drivers/usb/host/xhci-hub.c |  6 +++---
 drivers/usb/host/xhci-ring.c|  2 +-
 drivers/usb/isp1760/isp1760-hcd.c   |  2 +-
 drivers/usb/musb/musb_core.c|  6 +++---
 drivers/usb/musb/musb_virthub.c |  2 +-
 include/linux/usb.h | 23 ++-
 21 files changed, 77 insertions(+), 33 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index be2d6d0..75f65c6 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4024,6 +4024,13 @@
unknown_nmi_panic
[X86] Cause panic on unknown NMI.
 
+   usbcore.timing_minimum=
+   Set USB timing values to USB 2.0 spec minimums (default 
0 = off).
+   This removes any padding added to the values to 
accommodate
+   older or troublesome hardware. This will reduce usb 
resume
+   time. Use this only if you know your USB hardware and 
device
+   setup can handle the spec minimums.
+
usbcore.authorized_default=
[USB] Default USB device authorization:
(default -1 = authorized except for wireless USB,
diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
index 5ef8da6..05b20f12 100644
--- a/drivers/usb/common/common.c
+++ b/drivers/usb/common/common.c
@@ -19,6 +19,13 @@
 #include 
 #include 
 
+struct _usb_timing_config usb_timing = {
+   .tdrsmdn = USB_TIMING_TDRSMDN_DEF,
+   .trsmrcy = USB_TIMING_TRSMRCY_DEF,
+   .trstrcy = USB_TIMING_TRSTRCY_DEF
+};
+EXPORT_SYMBOL_GPL(usb_timing);
+
 const char *usb_otg_state_string(enum usb_otg_state state)
 {
static const char *const names[] = {
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 143454e..10c0bea 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2841,7 +2841,7 @@ static int hub_port_reset(struct usb_hub *hub, int port1,
 done:
if (status == 0) {
/* TRSTRCY = 10 ms; plus some extra */
-   msleep(10 + 40);
+   msleep(usb_timing.trstrcy);
if (udev) {
struct usb_hcd *hcd = bus_to_hcd(udev->bus);
 
@@ -3433,10 +3433,10 @@ int usb_port_resume(struct usb_device *udev, 
pm_message_t msg)
if (status) {
dev_dbg(_dev->dev, "can't resume, status %d\n", status);
} else {
-   /* drive resume for USB_RESUME_TIMEOUT msec */
+   /* drive resume for TDRSMDN msec */
dev_dbg(>dev, "usb %sresume\n",
(PMSG_IS_AUTO(msg) ? "auto-" : ""));
-   msleep(USB_RESUME_TIMEOUT);
+   msleep(usb_timing.tdrsmdn);
 
/* Virtual root hubs can trigger on GET_PORT_STATUS to
 * stop resume signaling.  Then finish the resume
@@ -3445,7 +3445,7 @@ int usb_port_resume(struct usb_device *udev, 

[PATCH v4 2/2] USB: usb resume time delay values debug

[Todd Brandt]

add debugfs support for experimenting with USB timing delay
values on the fly. Values are read/written from debugfs at
/sys/kernel/debug/usb/timing.

Signed-off-by: Todd Brandt 
---
v2 changes:
 - moved the debug code from hub.c to usb.c
 - use debugfs instead of /sys/kernel/usb

 drivers/usb/core/usb.c | 55 ++
 1 file changed, 55 insertions(+)

diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 2120c34..4f45288 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -39,6 +39,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 
 #include 
@@ -1036,6 +1038,49 @@ struct dentry *usb_debug_root;
 EXPORT_SYMBOL_GPL(usb_debug_root);
 
 static struct dentry *usb_debug_devices;
+static struct dentry *usb_debug_timing;
+
+static int usb_timing_show(struct seq_file *s, void *unused)
+{
+   seq_printf(s, "tdrsmdn=%d\n", usb_timing.tdrsmdn);
+   seq_printf(s, "trsmrcy=%d\n", usb_timing.trsmrcy);
+   seq_printf(s, "trstrcy=%d\n", usb_timing.trstrcy);
+   return 0;
+}
+
+static int usb_timing_open(struct inode *inode, struct file *file)
+{
+   return single_open(file, usb_timing_show, inode->i_private);
+}
+
+static ssize_t usb_timing_write(struct file *file,
+   const char __user *ubuf, size_t count, loff_t *ppos)
+{
+   int val;
+   char buf[32];
+
+   if (copy_from_user(, ubuf, min_t(size_t, sizeof(buf) - 1, count)))
+   return -EFAULT;
+
+   if (sscanf(buf, "tdrsmdn=%u", ) == 1)
+   usb_timing.tdrsmdn = max(val, USB_TIMING_TDRSMDN_MIN);
+   else if (sscanf(buf, "trsmrcy=%u", ) == 1)
+   usb_timing.trsmrcy = max(val, USB_TIMING_TRSMRCY_MIN);
+   else if (sscanf(buf, "trstrcy=%u", ) == 1)
+   usb_timing.trstrcy = max(val, USB_TIMING_TRSTRCY_MIN);
+   else
+   return -EINVAL;
+
+   return count;
+}
+
+static const struct file_operations usbfs_timing_fops = {
+   .open = usb_timing_open,
+   .write= usb_timing_write,
+   .read = seq_read,
+   .llseek   = seq_lseek,
+   .release  = single_release,
+};
 
 static int usb_debugfs_init(void)
 {
@@ -1052,11 +1097,21 @@ static int usb_debugfs_init(void)
return -ENOENT;
}
 
+   usb_debug_timing = debugfs_create_file("timing", 0644,
+   usb_debug_root, NULL,
+   _timing_fops);
+   if (!usb_debug_timing) {
+   debugfs_remove(usb_debug_root);
+   usb_debug_root = NULL;
+   return -ENOENT;
+   }
+
return 0;
 }
 
 static void usb_debugfs_cleanup(void)
 {
+   debugfs_remove(usb_debug_timing);
debugfs_remove(usb_debug_devices);
debugfs_remove(usb_debug_root);
 }
-- 
2.1.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: [v5,1/6] usb: separate out sysdev pointer from usb_bus

[Alexander Sverdlin]

Hi!

On 17/11/16 12:43, Sriram Dash wrote:
> From: Arnd Bergmann 
> 
> For xhci-hcd platform device, all the DMA parameters are not
> configured properly, notably dma ops for dwc3 devices.
> 
> The idea here is that you pass in the parent of_node along with
> the child device pointer, so it would behave exactly like the
> parent already does. The difference is that it also handles all
> the other attributes besides the mask.
> 
> sysdev will represent the physical device, as seen from firmware
> or bus.Splitting the usb_bus->controller field into the
> Linux-internal device (used for the sysfs hierarchy, for printks
> and for power management) and a new pointer (used for DMA,
> DT enumeration and phy lookup) probably covers all that we really
> need.
> 
> Signed-off-by: Arnd Bergmann 
> Signed-off-by: Sriram Dash 
> Tested-by: Baolin Wang 

Successfully tested on arm64/axxia with DWC3 USB host, XHCIs properly inherit
DMA configuration. Therefore:

Tested-by: Alexander Sverdlin 

> Cc: Felipe Balbi 
> Cc: Grygorii Strashko 
> Cc: Sinjan Kumar 
> Cc: David Fisher 
> Cc: Catalin Marinas 
> Cc: "Thang Q. Nguyen" 
> Cc: Yoshihiro Shimoda 
> Cc: Stephen Boyd 
> Cc: Bjorn Andersson 
> Cc: Ming Lei 
> Cc: Jon Masters 
> Cc: Dann Frazier 
> Cc: Peter Chen 
> Cc: Leo Li 
> Tested-by: Brian Norris 
> ---
> Changes in v5:
>   - No update
> 
> Changes in v4:
>   - No update
> 
> Changes in v3: 
>   - usb is_device_dma_capable instead of directly accessing 
> dma props. 
>  
> Changes in v2: 
>   - Split the patch wrt driver
> 
>  drivers/usb/core/buffer.c | 12 ++--
>  drivers/usb/core/hcd.c| 48 
> ---
>  drivers/usb/core/usb.c| 18 +-
>  include/linux/usb.h   |  1 +
>  include/linux/usb/hcd.h   |  3 +++
>  5 files changed, 48 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/usb/core/buffer.c b/drivers/usb/core/buffer.c
> index 98e39f9..a6cd44a 100644
> --- a/drivers/usb/core/buffer.c
> +++ b/drivers/usb/core/buffer.c
> @@ -63,7 +63,7 @@ int hcd_buffer_create(struct usb_hcd *hcd)
>   int i, size;
>  
>   if (!IS_ENABLED(CONFIG_HAS_DMA) ||
> - (!hcd->self.controller->dma_mask &&
> + (!is_device_dma_capable(hcd->self.sysdev) &&
>!(hcd->driver->flags & HCD_LOCAL_MEM)))
>   return 0;
>  
> @@ -72,7 +72,7 @@ int hcd_buffer_create(struct usb_hcd *hcd)
>   if (!size)
>   continue;
>   snprintf(name, sizeof(name), "buffer-%d", size);
> - hcd->pool[i] = dma_pool_create(name, hcd->self.controller,
> + hcd->pool[i] = dma_pool_create(name, hcd->self.sysdev,
>   size, size, 0);
>   if (!hcd->pool[i]) {
>   hcd_buffer_destroy(hcd);
> @@ -127,7 +127,7 @@ void *hcd_buffer_alloc(
>  
>   /* some USB hosts just use PIO */
>   if (!IS_ENABLED(CONFIG_HAS_DMA) ||
> - (!bus->controller->dma_mask &&
> + (!is_device_dma_capable(bus->sysdev) &&
>!(hcd->driver->flags & HCD_LOCAL_MEM))) {
>   *dma = ~(dma_addr_t) 0;
>   return kmalloc(size, mem_flags);
> @@ -137,7 +137,7 @@ void *hcd_buffer_alloc(
>   if (size <= pool_max[i])
>   return dma_pool_alloc(hcd->pool[i], mem_flags, dma);
>   }
> - return dma_alloc_coherent(hcd->self.controller, size, dma, mem_flags);
> + return dma_alloc_coherent(hcd->self.sysdev, size, dma, mem_flags);
>  }
>  
>  void hcd_buffer_free(
> @@ -154,7 +154,7 @@ void hcd_buffer_free(
>   return;
>  
>   if (!IS_ENABLED(CONFIG_HAS_DMA) ||
> - (!bus->controller->dma_mask &&
> + (!is_device_dma_capable(bus->sysdev) &&
>!(hcd->driver->flags & HCD_LOCAL_MEM))) {
>   kfree(addr);
>   return;
> @@ -166,5 +166,5 @@ void hcd_buffer_free(
>   return;
>   }
>   }
> - dma_free_coherent(hcd->self.controller, size, addr, dma);
> + dma_free_coherent(hcd->self.sysdev, size, addr, dma);
>  }
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 479e223..f8feb08 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -1073,6 +1073,7 @@ static void usb_deregister_bus (struct usb_bus *bus)
>  static int register_root_hub(struct usb_hcd *hcd)
>  {
>   struct device *parent_dev = hcd->self.controller;
> + struct device *sysdev = hcd->self.sysdev;
> 

Re: dma-coherent and snooping set

[Felipe Balbi]

Hi,

Jerry Huang  writes:
> hi, Balbi,
> sorry to disturb you.
>
> some NXP platforms need dma-coherent property.
>
> I want to add "dma-coherent" into usb node in DTS file like other
> platform done
> (e.g. arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi).
>
> Could you please give some comment about this?

well, just send a patch ;-)

-- 
balbi


signature.asc
Description: PGP signature

[PATCH] USB: serial: f81534: Detect errors from f81534_logic_to_phy_port()

[Geert Uytterhoeven]

With gcc 4.1.2:

drivers/usb/serial/f81534.c: In function ‘f81534_port_probe’:
drivers/usb/serial/f81534.c:1250: warning: comparison is always false due 
to limited range of data type

f81534_logic_to_phy_port() may return a negative error value, which is
ignored by assigning it to u8 f81534_port_private.phy_num.

Use an intermediate variable of type int to fix this.
While at it, forward the actual error code instead of converting it to
-ENODEV.

Fixes: 0c9bd6004d258d46 ("USB: serial: add Fintek F81532/534 driver")
Signed-off-by: Geert Uytterhoeven 
---
 drivers/usb/serial/f81534.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/serial/f81534.c b/drivers/usb/serial/f81534.c
index 8282a6a18fee83f6..2acad268f9d15f43 100644
--- a/drivers/usb/serial/f81534.c
+++ b/drivers/usb/serial/f81534.c
@@ -1237,6 +1237,7 @@ static int f81534_attach(struct usb_serial *serial)
 static int f81534_port_probe(struct usb_serial_port *port)
 {
struct f81534_port_private *port_priv;
+   int ret;
 
port_priv = devm_kzalloc(>dev, sizeof(*port_priv), GFP_KERNEL);
if (!port_priv)
@@ -1246,10 +1247,14 @@ static int f81534_port_probe(struct usb_serial_port 
*port)
mutex_init(_priv->mcr_mutex);
 
/* Assign logic-to-phy mapping */
-   port_priv->phy_num = f81534_logic_to_phy_port(port->serial, port);
-   if (port_priv->phy_num < 0 || port_priv->phy_num >= F81534_NUM_PORT)
+   ret = f81534_logic_to_phy_port(port->serial, port);
+   if (ret < 0)
+   return ret;
+
+   if (ret >= F81534_NUM_PORT)
return -ENODEV;
 
+   port_priv->phy_num = ret;
usb_set_serial_port_data(port, port_priv);
dev_dbg(>dev, "%s: port_number: %d, phy_num: %d\n", __func__,
port->port_number, port_priv->phy_num);
-- 
1.9.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

xhci_reset_endpoint() doesn't reset endpoint

[Michal Necasek]

Hi Mathias,

 We have run into a problem with a USB printer which we're quite confident is a 
bug in the Linux xHCI driver. There is no problem when the same printer is 
plugged into a port managed by the EHCI driver.

 The core problem is that xhci_reset_endpoint() doesn't do anything, and more 
specifically does not reset the xHC's data toggle/sequence number. That is not 
normally an issue, because the reset does happen in response to a STALL; in our 
scenario, there is no STALL or any other error. That can lead to the data 
toggle getting out of sync and the host dropping a packet sent by the device.

 Now a detailed problem description. We have a USB printer passed through to a 
VM. The VM runs Windows 8.1 or 10 (other versions may be affected too), and 
uses Microsoft's standard usbprint.sys to talk to the printer. The vendor 
printer driver tries to query the printer's configuration, using the control 
endpoint, one OUT endpoint, and one IN endpoint. The query always times 
out/fails when printer is plugged into a port managed by xHCI, yet works in 
EHCI ports.

 The usbprint.sys driver is a bit funny and in many cases (though not always) 
queues up URBs on the IN endpoint in advance, and once it decides that it has 
received the entire response, cancels the last URB and resets the IN endpoint 
(issuing SetFeature(CLEAR_HALT)). After much head scratching, we realized, and 
later confirmed with a USB analyzer, that the next IN packet that the printer 
sends is not seen by the host's USB stack at all, let alone the guest OS. Other 
packets arrive just fine, but the guest OS keeps waiting for more data to 
arrive, eventually loses patience and fails.

 We cannot observe the data toggle state of the xHC but we are fairly certain 
that things go wrong when the data toggle is set (on both ends) prior to the 
endpoint reset. SetFeature(CLEAR_HALT) resets the toggle on the device, but not 
on the host. But we know for a fact that the device sends a packet (with data 
toggle 0) which the host USB stack never sees, and a data toggle mismatch 
explains that quite well.

 We are using USBFS to talk to the printer, but that shouldn't matter much. I 
will note that the available documentation<1> explicitly says that 
USBDEVFS_RESETEP and USBDEVFS_CLEAR_HALT both reset the data toggle. That is 
indeed the case for the Linux EHCI driver but not xHCI. Both of the USBFS 
IOCTLs call into xhci_reset_endpoint() which does nothing.

 We believe that xhci_reset_endpoint() needs to reset the data toggle/sequence 
number to match the documentation and for compatibility with the EHCI driver. 
We tried but failed to find a workaround which would reset the data toggle 
without side effects (e.g. USBDEVFS_SETINTERFACE does reset the toggle on the 
IN endpoint, but also resets it on the OUT endpoint and talks to the device, so 
that's no good).

 The data toggle management is not terribly well documented in the xHCI spec so 
we hope you know about it more than we do. Based on our understanding of the 
xHCI specification, xhci_reset_endpoint() should issue either a Reset Endpoint 
command with TSP=0 or a dummy Configure Endpoint command dropping/re-adding the 
specified endpoint (as the xHCI 1.1 spec suggests at the end of 4.6.8). Please 
confirm if that should solve the problem.

 We don't know how many devices this problem affects. We suspect it affects 
many USB printers and could in theory affect more or less any device, but few 
drivers reset endpoints when there are no errors. The problem scenario can 
probably be artificially reproduced with more or less any USB device (when data 
toggle is set, issue USBDEVFS_CLEAR_HALT, see if next packet arrives at 
destination).


  Regards,
 Michal


1:
  https://www.kernel.org/doc/htmldocs/usb/usbfs-ioctl.html
--
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: usb/gadget: warning in ep_write_iter/__alloc_pages_nodemask

[Michal Hocko]

On Tue 13-12-16 08:33:34, Alan Stern wrote:
> On Tue, 13 Dec 2016, Michal Hocko wrote:
> 
> > > > That being said, what ep_write_iter does sounds quite stupit. It just
> > > > allocates a large continuous buffer which seems to be under user
> > > > control...  Aka no good! It should do that per pages or something like
> > > > that. Something worth fixing
> > > 
> > > It's not important enough to make the driver do all this work.  If
> > > users want to send large amounts of data, they can send it a page at a
> > > time (or something like that).
> > 
> > Is it really necessary to allocate the full iov_iter_count? Why cannot
> > we process the from buffer one page at a time?
> 
> We could (although one page is really too small -- USB 3.1 can transfer
> 800 KB per ms so we ought to handle at least 128 KB at a time).

Is there any problem to submit larger transfers without having the
buffer physically contiguous?

> But
> turn the argument around: If the user wants to transfer that much data,
> why can't he _submit_ it one page at a time?

Not sure I understand.
 
> > > If you really want to prevent the driver from attempting to allocate a
> > > large buffer, all that's needed is an upper limit on the total size.  
> > > For example, 64 KB.
> > 
> > Well, my point was that it is not really hard to imagine to deplete
> > larger contiguous memory blocks (say PAGE_ALLOC_COSTLY_ORDER). Those are
> > still causing the OOM killer and chances are that a controlled flood of
> > these requests could completely DoS the system.
> 
> Putting a limit on the total size of a single transfer would prevent 
> this.

Dunno, putting a limit to the user visible interface sounds wrong to me.
-- 
Michal Hocko
SUSE Labs
--
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] keys/encrypted: Fix two crypto-on-the-stack bugs

[David Howells]

Andy Lutomirski  wrote:

> > -   sg_set_buf(_out[1], pad, sizeof pad);
> > +   sg_set_buf(_out[1], empty_zero_page, 16);
> 
> My fix here is obviously bogus (I meant to use ZERO_PAGE(0)), but what
> exactly is the code trying to do?  The old code makes no sense.  It's
> setting the *output* buffer to zeroed padding.

Padding goes into the encrypt function and is going to come out of the decrypt
function.  Possibly derived_key_decrypt() should be checking that the padding
that comes out is actually a bunch of zeros.  Maybe we don't actually need to
get the padding out, but I'm not sure whether the crypto layer will
malfunction if we don't give it a buffer for the padding.

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] cifs: Fix smbencrypt() to stop pointing a scatterlist at the stack

[Steve French]

merged into cifs-2.6.git for-next

On Tue, Dec 13, 2016 at 7:07 AM, Jeff Layton  wrote:
> On Mon, 2016-12-12 at 12:54 -0800, Andy Lutomirski wrote:
>> smbencrypt() points a scatterlist to the stack, which is breaks if
>> CONFIG_VMAP_STACK=y.
>>
>> Fix it by switching to crypto_cipher_encrypt_one().  The new code
>> should be considerably faster as an added benefit.
>>
>> This code is nearly identical to some code that Eric Biggers
>> suggested.
>>
>> Cc: sta...@vger.kernel.org # 4.9 only
>> Reported-by: Eric Biggers 
>> Signed-off-by: Andy Lutomirski 
>> ---
>>
>> Compile-tested only.
>>
>> fs/cifs/smbencrypt.c | 40 
>>  1 file changed, 8 insertions(+), 32 deletions(-)
>>
>> diff --git a/fs/cifs/smbencrypt.c b/fs/cifs/smbencrypt.c
>> index 699b7868108f..c12bffefa3c9 100644
>> --- a/fs/cifs/smbencrypt.c
>> +++ b/fs/cifs/smbencrypt.c
>> @@ -23,7 +23,7 @@
>> Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
>>  */
>>
>> -#include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -69,46 +69,22 @@ str_to_key(unsigned char *str, unsigned char *key)
>>  static int
>>  smbhash(unsigned char *out, const unsigned char *in, unsigned char *key)
>>  {
>> - int rc;
>>   unsigned char key2[8];
>> - struct crypto_skcipher *tfm_des;
>> - struct scatterlist sgin, sgout;
>> - struct skcipher_request *req;
>> + struct crypto_cipher *tfm_des;
>>
>>   str_to_key(key, key2);
>>
>> - tfm_des = crypto_alloc_skcipher("ecb(des)", 0, CRYPTO_ALG_ASYNC);
>> + tfm_des = crypto_alloc_cipher("des", 0, 0);
>>   if (IS_ERR(tfm_des)) {
>> - rc = PTR_ERR(tfm_des);
>> - cifs_dbg(VFS, "could not allocate des crypto API\n");
>> - goto smbhash_err;
>> - }
>> -
>> - req = skcipher_request_alloc(tfm_des, GFP_KERNEL);
>> - if (!req) {
>> - rc = -ENOMEM;
>>   cifs_dbg(VFS, "could not allocate des crypto API\n");
>> - goto smbhash_free_skcipher;
>> + return PTR_ERR(tfm_des);
>>   }
>>
>> - crypto_skcipher_setkey(tfm_des, key2, 8);
>> -
>> - sg_init_one(, in, 8);
>> - sg_init_one(, out, 8);
>> + crypto_cipher_setkey(tfm_des, key2, 8);
>> + crypto_cipher_encrypt_one(tfm_des, out, in);
>> + crypto_free_cipher(tfm_des);
>>
>> - skcipher_request_set_callback(req, 0, NULL, NULL);
>> - skcipher_request_set_crypt(req, , , 8, NULL);
>> -
>> - rc = crypto_skcipher_encrypt(req);
>> - if (rc)
>> - cifs_dbg(VFS, "could not encrypt crypt key rc: %d\n", rc);
>> -
>> - skcipher_request_free(req);
>> -
>> -smbhash_free_skcipher:
>> - crypto_free_skcipher(tfm_des);
>> -smbhash_err:
>> - return rc;
>> + return 0;
>>  }
>>
>>  static int
>
> Acked-by: Jeff Layton 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Thanks,

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: [REGRESSION] dwc2: 5f54c54b0ba8 Add DDMA chain pointers to dwc2_hsotg_ep structure

[Vardan Mikayelyan]

On 12/14/2016 6:25 AM, John Stultz wrote:
> On Tue, Dec 13, 2016 at 3:51 PM, John Youn  wrote:
>> On 12/13/2016 3:23 PM, John Stultz wrote:
>>> I've found trying to boot Linus' HEAD today on HiKey, I'm hitting the
>>> following WARNINGS/panics, which I bisected down to being introduced
>>> with 5f54c54b0ba8 ("Add DDMA chain pointers to dwc2_hsotg_ep
>>> structure").
>>>
>>> It seems the issue is calling dma_alloc_coherent() in
>>> dwc2_hsotg_ep_enable() isn't safe, as we may get there from irq
>>> context with irqs off.
>>>
>>> Any thoughts on a fix?
>>>
>>
>> Hi John,
>>
>> Check this:
>> http://marc.info/?l=linux-usb=148058361526211=2
>
> So digging further here, it seems that patch addresses the allocation
> side, but I'm also seeing warnings on the free side, as irqs are
> disabled in dwc2_hsotg_vbus_session() before calling
> dwc2_hsotg_disconnect().
>
> (Note that this is triggered using the extcon patch I have, but the
> problematic logic seems to be pre-existing).
>
> thanks
> -john

Hi John Stultz,

Related to free side, we have fix which will be submitted after internal 
review and testing. You'll see it here soon.

Thanks,
Vardan.
--
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] keys/encrypted: Fix two crypto-on-the-stack bugs

[Eric Biggers]

On Wed, Dec 14, 2016 at 01:04:04PM +0800, Herbert Xu wrote:
> On Tue, Dec 13, 2016 at 06:53:03PM -0800, Andy Lutomirski wrote:
> > On Tue, Dec 13, 2016 at 6:48 PM, Andy Lutomirski  wrote:
> > > The driver put a constant buffer of all zeros on the stack and
> > > pointed a scatterlist entry at it in two places.  This doesn't work
> > > with virtual stacks.  Use ZERO_PAGE instead.
> > 
> > Wait a second...
> > 
> > > -   sg_set_buf(_out[1], pad, sizeof pad);
> > > +   sg_set_buf(_out[1], empty_zero_page, 16);
> > 
> > My fix here is obviously bogus (I meant to use ZERO_PAGE(0)), but what
> > exactly is the code trying to do?  The old code makes no sense.  It's
> > setting the *output* buffer to zeroed padding.
> 
> It's decrypting so I presume it just needs to the extra space for
> the padding and the result will be thrown away.
> 

It looks like the data is zero-padded to a 16-byte AES block boundary before
being encrypted with CBC, so the encrypted data may be longer than the original
data.  (I don't know why it doesn't use CTS mode instead, which would make the
lengths the same.)

Since the crypto API can do in-place operations, when *encrypting* I suggest
copying the decrypted data to the output buffer, padding it with 0's, and
encrypting it in-place.  This eliminates the need for the stack buffer.

But when *decrypting* there will be up to 15 extra throwaway bytes of output
produced by the decryption.  There must be space made for these, and since it's
output it can't be empty_zero_page.  I guess either check whether space can be
made for it in-place, or allocate a temporary buffer...

Eric
--
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 1/2] USB: add switch to turn off padding of resume time delays

[kbuild test robot]

Hi Todd,

[auto build test ERROR on linus/master]
[also build test ERROR on next-20161214]
[cannot apply to usb/usb-testing balbi-usb/next v4.9]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Todd-Brandt/USB-resume-time-optimization-by-using-spec-minimums/20161214-115712
config: i386-randconfig-b0-12141419 (attached as .config)
compiler: gcc-5 (Debian 5.4.1-2) 5.4.1 20160904
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/built-in.o: In function `musb_runtime_resume':
>> musb_core.c:(.text+0x109ee7): undefined reference to `usb_timing'
   drivers/built-in.o: In function `musb_stage0_irq':
   musb_core.c:(.text+0x10b52e): undefined reference to `usb_timing'
   drivers/built-in.o: In function `musb_resume':
   musb_core.c:(.text+0x10be58): undefined reference to `usb_timing'

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip