Re: [PATCH v4 72/73] xfs: Convert mru cache to XArray

[Matthew Wilcox]

On Thu, Dec 07, 2017 at 11:38:43AM +1100, Dave Chinner wrote:
> > > cmpxchg is for replacing a known object in a store - it's not really
> > > intended for doing initial inserts after a lookup tells us there is
> > > nothing in the store.  The radix tree "insert only if empty" makes
> > > sense here, because it naturally takes care of lookup/insert races
> > > via the -EEXIST mechanism.
> > > 
> > > I think that providing xa_store_excl() (which would return -EEXIST
> > > if the entry is not empty) would be a better interface here, because
> > > it matches the semantics of lookup cache population used all over
> > > the kernel
> > 
> > I'm not thrilled with xa_store_excl(), but I need to think about that
> > a bit more.
> 
> Not fussed about the name - I just think we need a function that
> matches the insert semantics of the code

I think I have something that works better for you than returning -EEXIST
(because you don't actually want -EEXIST, you want -EAGAIN):

/* insert the new inode */
-   spin_lock(>pag_ici_lock);
-   error = radix_tree_insert(>pag_ici_root, agino, ip);
-   if (unlikely(error)) {
-   WARN_ON(error != -EEXIST);
-   XFS_STATS_INC(mp, xs_ig_dup);
-   error = -EAGAIN;
-   goto out_preload_end;
-   }
-   spin_unlock(>pag_ici_lock);
-   radix_tree_preload_end();
+   curr = xa_cmpxchg(>pag_ici_xa, agino, NULL, ip, GFP_NOFS);
+   error = __xa_race(curr, -EAGAIN);
+   if (error)
+   goto out_unlock;

...

-out_preload_end:
-   spin_unlock(>pag_ici_lock);
-   radix_tree_preload_end();
+out_unlock:
+   if (error == -EAGAIN)
+   XFS_STATS_INC(mp, xs_ig_dup);

I've changed the behaviour slightly in that returning an -ENOMEM used to
hit a WARN_ON, and I don't think that's the right response -- GFP_NOFS
returning -ENOMEM probably gets you a nice warning already from the
mm code.

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

Dual-role behavior with USB-C?

[Takashi Matsuzawa]

Hello.

Just in case if you already have pointer or experience with this, any 
suggestion is highly appreciated.

Some of the recent i.MX boards has USB-C socket for USB-OTG port, and I wonder 
how I can try the following steps (Dual-role behavior testing) with USB-C 
sockets.

https://www.kernel.org/doc/Documentation/usb/chipidea.txt

The steps involves micro-A and micro-B plugs, but for my board only USB-C plug 
can be inserted.
(I tried C-C cable to connect two boards, but nothing happened - both of the 
boards stayed in gadget role, neither becoming the host).

I have confirmed my board can act as host (can detect a device attached and 
shown on lsusb output) or gadget (by loading mass storage driver, it can be 
shown as external disk from my PC).

(Well, however these behaviors are not stable yet with my local build image.)

Anyway:

- How can I conduct test steps outlined in chipidea.txt memo.
How can I interpret it with UCB-C ports (may be using appropriate cables) or is 
there alternative test steps?

--
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: Lockdep is less useful than it was

[Dave Chinner]

On Fri, Dec 08, 2017 at 10:14:38AM -0800, Matthew Wilcox wrote:
> At the moment, the radix tree actively disables the RCU checking that
> enabling lockdep would give us.  It has to, because it has no idea what
> lock protects any individual access to the radix tree.  The XArray can
> use the RCU checking because it knows that every reference is protected
> by either the spinlock or the RCU lock.
> 
> Dave was saying that he has a tree which has to be protected by a mutex
> because of where it is in the locking hierarchy, and I was vigorously
> declining his proposal of allowing him to skip taking the spinlock.

Oh, I wasn't suggesting that you remove the internal tree locking
because we need external locking.

I was trying to point out that the internal locking doesn't remove
the need for external locking,  and that there are cases where
smearing the internal lock outside the XA tree doesn't work, either.
i.e. internal locking doesn't replace all the cases where external
locking is required, and so it's less efficient than the existing
radix tree code.

What I was questioning was the value of replacing the radix tree
code with a less efficient structure just to add lockdep validation
to a tree that doesn't actually need any extra locking validation...

Cheers,

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

Re: [PATCH v4 72/73] xfs: Convert mru cache to XArray

[Dave Chinner]

On Fri, Dec 08, 2017 at 12:35:07PM -0500, Alan Stern wrote:
> On Fri, 8 Dec 2017, Byungchul Park wrote:
> 
> > I'm sorry to hear that.. If I were you, I would also get
> > annoyed. And.. thanks for explanation.
> > 
> > But, I think assigning lock classes properly and checking
> > relationship of the classes to detect deadlocks is reasonable.
> > 
> > In my opinion about the common lockdep stuff, there are 2
> > problems on it.
> > 
> > 1) Firstly, it's hard to assign lock classes *properly*. By
> > default, it relies on the caller site of lockdep_init_map(),
> > but we need to assign another class manually, where ordering
> > rules are complicated so cannot rely on the caller site. That
> > *only* can be done by experts of the subsystem.

Sure, but that's not the issue here. The issue here is the lack of
communication with subsystem experts and that the annotation
complexity warnings given immediately by the subsystem experts were
completely ignored...

> > I think if they want to get benifit from lockdep, they have no
> > choice but to assign classes manually with the domain knowledge,
> > or use *lockdep_set_novalidate_class()* to invalidate locks
> > making the developers annoyed and not want to use the checking
> > for them.
> 
> Lockdep's no_validate class is used when the locking patterns are too
> complicated for lockdep to understand.  Basically, it tells lockdep to
> ignore those locks.

Let me just point out two things here:

1. Using lockdep_set_novalidate_class() for anything other
than device->mutex will throw checkpatch warnings. Nice. (*)

2. lockdep_set_novalidate_class() is completely undocumented
- it's the first I've ever heard of this functionality. i.e.
nobody has ever told us there is a mechanism to turn off
validation of an object; we've *always* been told to "change
your code and/or fix your annotations" when discussing
lockdep deficiencies. (**)

> The device core uses that class.  The tree of struct devices, each with
> its own lock, gets used in many different and complicated ways.  
> Lockdep can't understand this -- it doesn't have the ability to
> represent an arbitrarily deep hierarchical tree of locks -- so we tell
^^

That largely describes the in-memory structure of XFS, except we
have a forest of lock trees, not just one

> it to ignore the device locks.
> 
> It sounds like XFS may need to do the same thing with its semaphores.

Who-ever adds semaphore checking to lockdep can add those
annotations. The externalisation of the development cost of new
lockdep functionality is one of the problems here.

-Dave.

(*) checkpatch.pl is considered mostly harmful round here, too,
but that's another rant

(**) the frequent occurrence of "core code/devs aren't held to the
same rules/standard as everyone else" is another rant I have stored
up for a rainy day.

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

[PATCH 1/2] usb: musb: un-break davinci glue layer

[Alejandro Mery]

MUSB's davinci glue was made to depend on BROKEN by Felipe Balbi back in
2013 because of lack of active development. needed changes were actually trivial

Fixes: 787f5627bec8 (usb: musb: make davinci and da8xx glues depend on BROKEN)
Signed-off-by: Alejandro Mery 
---
 drivers/usb/musb/Kconfig   |  1 -
 drivers/usb/musb/davinci.c | 20 ++--
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
index 5506a9c03c1f..e13320eebbbf 100644
--- a/drivers/usb/musb/Kconfig
+++ b/drivers/usb/musb/Kconfig
@@ -76,7 +76,6 @@ config USB_MUSB_DAVINCI
tristate "DaVinci"
depends on ARCH_DAVINCI_DMx
depends on NOP_USB_XCEIV
-   depends on BROKEN
 
 config USB_MUSB_DA8XX
tristate "DA8xx/OMAP-L1x"
diff --git a/drivers/usb/musb/davinci.c b/drivers/usb/musb/davinci.c
index 2ad39dcd2f4c..6571f9e59f8f 100644
--- a/drivers/usb/musb/davinci.c
+++ b/drivers/usb/musb/davinci.c
@@ -39,6 +39,7 @@
 struct davinci_glue {
struct device   *dev;
struct platform_device  *musb;
+   struct platform_device  *phy;
struct clk  *clk;
 };
 
@@ -363,10 +364,8 @@ static int davinci_musb_init(struct musb *musb)
int ret = -ENODEV;
 
musb->xceiv = usb_get_phy(USB_PHY_TYPE_USB2);
-   if (IS_ERR_OR_NULL(musb->xceiv)) {
-   ret = -EPROBE_DEFER;
-   goto unregister;
-   }
+   if (IS_ERR_OR_NULL(musb->xceiv))
+   return -EPROBE_DEFER;
 
musb->mregs += DAVINCI_BASE_OFFSET;
 
@@ -418,8 +417,6 @@ static int davinci_musb_init(struct musb *musb)
 
 fail:
usb_put_phy(musb->xceiv);
-unregister:
-   usb_phy_generic_unregister();
return ret;
 }
 
@@ -527,7 +524,9 @@ static int davinci_probe(struct platform_device *pdev)
 
pdata->platform_ops = _ops;
 
-   usb_phy_generic_register();
+   glue->phy = usb_phy_generic_register();
+   if (IS_ERR(glue->phy))
+   goto err1;
platform_set_drvdata(pdev, glue);
 
memset(musb_resources, 0x00, sizeof(*musb_resources) *
@@ -563,14 +562,15 @@ static int davinci_probe(struct platform_device *pdev)
if (IS_ERR(musb)) {
ret = PTR_ERR(musb);
dev_err(>dev, "failed to register musb device: %d\n", 
ret);
-   goto err1;
+   goto err2;
}
 
return 0;
 
+err2:
+   usb_phy_generic_unregister(glue->phy);
 err1:
clk_disable(clk);
-
 err0:
return ret;
 }
@@ -580,7 +580,7 @@ static int davinci_remove(struct platform_device *pdev)
struct davinci_glue *glue = platform_get_drvdata(pdev);
 
platform_device_unregister(glue->musb);
-   usb_phy_generic_unregister();
+   usb_phy_generic_unregister(glue->phy);
clk_disable(glue->clk);
 
return 0;
-- 
2.15.0

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

[PATCH 2/2] usb: musb: davinci: Pass file_mode in platform data

[Alejandro Mery]

as it was done for glues to allow setting the correct
fifo_mode when multiple MUSB glue layers are built-in

Fixes: 8a77f05aa39b ("usb: musb: Pass fifo_mode in platform data")
Signed-off-by: Alejandro Mery 
---
 drivers/usb/musb/davinci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/musb/davinci.c b/drivers/usb/musb/davinci.c
index 6571f9e59f8f..2632efd3d598 100644
--- a/drivers/usb/musb/davinci.c
+++ b/drivers/usb/musb/davinci.c
@@ -472,6 +472,7 @@ static const struct musb_platform_ops davinci_ops = {
.quirks = MUSB_DMA_CPPI,
.init   = davinci_musb_init,
.exit   = davinci_musb_exit,
+   .fifo_mode  = 2,
 
 #ifdef CONFIG_USB_TI_CPPI_DMA
.dma_init   = cppi_dma_controller_create,
-- 
2.15.0

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

Re: Lockdep is less useful than it was

[Matthew Wilcox]

On Fri, Dec 08, 2017 at 10:27:17AM -0500, Theodore Ts'o wrote:
> So if you are adding complexity to the kernel with the argument,
> "lockdep will save us", I'm with Dave --- it's just not a believable
> argument.

I think that's a gross misrepresentation of what I'm doing.

At the moment, the radix tree actively disables the RCU checking that
enabling lockdep would give us.  It has to, because it has no idea what
lock protects any individual access to the radix tree.  The XArray can
use the RCU checking because it knows that every reference is protected
by either the spinlock or the RCU lock.

Dave was saying that he has a tree which has to be protected by a mutex
because of where it is in the locking hierarchy, and I was vigorously
declining his proposal of allowing him to skip taking the spinlock.

And yes, we have bugs today that I assume we only stumble across every
few billion years (or only on alpha, or only if our compiler gets more
aggressive) because we have missing rcu_dereference annotations.
--
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 09/14] usb: xhci: Add DbC support in xHCI driver

[Greg KH]

On Fri, Dec 08, 2017 at 07:44:49PM +0200, Mathias Nyman wrote:
> From: Lu Baolu 
> 
> xHCI compatible USB host controllers(i.e. super-speed USB3 controllers)
> can be implemented with the Debug Capability(DbC). It presents a debug
> device which is fully compliant with the USB framework and provides the
> equivalent of a very high performance full-duplex serial link. The debug
> capability operation model and registers interface are defined in 7.6.8
> of the xHCI specification, revision 1.1.
> 
> The DbC debug device shares a root port with the xHCI host. By default,
> the debug capability is disabled and the root port is assigned to xHCI.
> When the DbC is enabled, the root port will be assigned to the DbC debug
> device, and the xHCI sees nothing on this port. This implementation uses
> a sysfs node named  under the xHCI device to manage the enabling
> and disabling of the debug capability.
> 
> When the debug capability is enabled, it will present a debug device
> through the debug port. This debug device is fully compliant with the
> USB3 framework, and it can be enumerated by a debug host on the other
> end of the USB link. As soon as the debug device is configured, a TTY
> serial device named /dev/ttyDBC0 will be created.
> 
> Signed-off-by: Lu Baolu 
> Signed-off-by: Mathias Nyman 
> ---
> 
> v2: memset correct amount of bytes to zero in xhci_dbc_eps_exit()

I've already applied this, can you send me a fix-up patch instead?

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

[Alan Stern]

On Fri, 8 Dec 2017, Geert Uytterhoeven wrote:

> Hi Alan,
> 
> On Thu, Dec 7, 2017 at 10:26 PM, Alan Stern  wrote:
> > On Thu, 7 Dec 2017, Dan Carpenter wrote:
> >> The standard is to treat them like errors and try press forward in a
> >> degraded mode but don't print a message.  Checkpatch.pl complains if you
> >> print a warning for allocation failures.  A lot of low level functions
> >> handle their own messages pretty well but especially kmalloc() does.
> >
> > Which brings us back to my original objection.  If an allocation
> > failure has localized effects, but the low-level warning is unable to
> > specify what will be affected, how is the user supposed to connect the
> > effect to the cause?
> 
> The backtrace would include usb_hub_clear_tt_buffer, so the user will
> know USB is affected.
> Note that the cause of the memory exhaustion is usually not the caller.
> Unless it requests a real big block of memory, in which case that will be
> mentioned in the backtrace, too.
> 
> In this particular case, the driver uses GFP_ATOMIC, so allocation failures
> aren't that rare, and I think the driver should be prepared for that, and try
> to recover gracefully.
> 
> The comment
> 
> /* FIXME recover somehow ... RESET_TT? */
> 
> makes me think it isn't.
> 
> As this is a pretty small allocation, perhaps it can be done beforehand, 
> without
> GFP_ATOMIC?

I can't see how to make that work.  We don't know beforehand how many
structures will be needed at any time.

Alan Stern

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

[PATCH v2 09/14] usb: xhci: Add DbC support in xHCI driver

[Mathias Nyman]

From: Lu Baolu 

xHCI compatible USB host controllers(i.e. super-speed USB3 controllers)
can be implemented with the Debug Capability(DbC). It presents a debug
device which is fully compliant with the USB framework and provides the
equivalent of a very high performance full-duplex serial link. The debug
capability operation model and registers interface are defined in 7.6.8
of the xHCI specification, revision 1.1.

The DbC debug device shares a root port with the xHCI host. By default,
the debug capability is disabled and the root port is assigned to xHCI.
When the DbC is enabled, the root port will be assigned to the DbC debug
device, and the xHCI sees nothing on this port. This implementation uses
a sysfs node named  under the xHCI device to manage the enabling
and disabling of the debug capability.

When the debug capability is enabled, it will present a debug device
through the debug port. This debug device is fully compliant with the
USB3 framework, and it can be enumerated by a debug host on the other
end of the USB link. As soon as the debug device is configured, a TTY
serial device named /dev/ttyDBC0 will be created.

Signed-off-by: Lu Baolu 
Signed-off-by: Mathias Nyman 
---

v2: memset correct amount of bytes to zero in xhci_dbc_eps_exit()

 .../ABI/testing/sysfs-bus-pci-drivers-xhci_hcd |  25 +
 drivers/usb/host/Kconfig   |   8 +
 drivers/usb/host/Makefile  |   5 +
 drivers/usb/host/xhci-dbgcap.c | 996 +
 drivers/usb/host/xhci-dbgcap.h | 229 +
 drivers/usb/host/xhci-dbgtty.c | 497 ++
 drivers/usb/host/xhci-trace.h  |  59 ++
 drivers/usb/host/xhci.c|   9 +
 drivers/usb/host/xhci.h|   1 +
 9 files changed, 1829 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
 create mode 100644 drivers/usb/host/xhci-dbgcap.c
 create mode 100644 drivers/usb/host/xhci-dbgcap.h
 create mode 100644 drivers/usb/host/xhci-dbgtty.c

diff --git a/Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd 
b/Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
new file mode 100644
index 000..0088aba
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
@@ -0,0 +1,25 @@
+What:  /sys/bus/pci/drivers/xhci_hcd/.../dbc
+Date:  June 2017
+Contact:   Lu Baolu 
+Description:
+   xHCI compatible USB host controllers (i.e. super-speed
+   USB3 controllers) are often implemented with the Debug
+   Capability (DbC). It can present a debug device which
+   is fully compliant with the USB framework and provides
+   the equivalent of a very high performance full-duplex
+   serial link for debug purpose.
+
+   The DbC debug device shares a root port with xHCI host.
+   When the DbC is enabled, the root port will be assigned
+   to the Debug Capability. Otherwise, it will be assigned
+   to xHCI.
+
+   Writing "enable" to this attribute will enable the DbC
+   functionality and the shared root port will be assigned
+   to the DbC device. Writing "disable" to this attribute
+   will disable the DbC functionality and the shared root
+   port will roll back to the xHCI.
+
+   Reading this attribute gives the state of the DbC. It
+   can be one of the following states: disabled, enabled,
+   initialized, connected, configured and stalled.
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index b80a94e..6150bed 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -27,6 +27,14 @@ config USB_XHCI_HCD
  module will be called xhci-hcd.
 
 if USB_XHCI_HCD
+config USB_XHCI_DBGCAP
+   bool "xHCI support for debug capability"
+   depends on TTY
+   ---help---
+ Say 'Y' to enable the support for the xHCI debug capability. Make
+ sure that your xHCI host supports the extended debug capability and
+ you want a TTY serial device based on the xHCI debug capability
+ before enabling this option. If unsure, say 'N'.
 
 config USB_XHCI_PCI
tristate
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index 32b036e..4ede4ce 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -14,6 +14,11 @@ fhci-$(CONFIG_FHCI_DEBUG) += fhci-dbg.o
 xhci-hcd-y := xhci.o xhci-mem.o
 xhci-hcd-y += xhci-ring.o xhci-hub.o xhci-dbg.o
 xhci-hcd-y += xhci-trace.o
+
+ifneq ($(CONFIG_USB_XHCI_DBGCAP), )
+   xhci-hcd-y += xhci-dbgcap.o xhci-dbgtty.o
+endif
+
 ifneq ($(CONFIG_USB_XHCI_MTK), )
xhci-hcd-y += xhci-mtk-sch.o
 

Re: [PATCH v4 72/73] xfs: Convert mru cache to XArray

[Alan Stern]

On Fri, 8 Dec 2017, Byungchul Park wrote:

> I'm sorry to hear that.. If I were you, I would also get
> annoyed. And.. thanks for explanation.
> 
> But, I think assigning lock classes properly and checking
> relationship of the classes to detect deadlocks is reasonable.
> 
> In my opinion about the common lockdep stuff, there are 2
> problems on it.
> 
> 1) Firstly, it's hard to assign lock classes *properly*. By
> default, it relies on the caller site of lockdep_init_map(),
> but we need to assign another class manually, where ordering
> rules are complicated so cannot rely on the caller site. That
> *only* can be done by experts of the subsystem.
> 
> I think if they want to get benifit from lockdep, they have no
> choice but to assign classes manually with the domain knowledge,
> or use *lockdep_set_novalidate_class()* to invalidate locks
> making the developers annoyed and not want to use the checking
> for them.

Lockdep's no_validate class is used when the locking patterns are too
complicated for lockdep to understand.  Basically, it tells lockdep to
ignore those locks.

The device core uses that class.  The tree of struct devices, each with
its own lock, gets used in many different and complicated ways.  
Lockdep can't understand this -- it doesn't have the ability to
represent an arbitrarily deep hierarchical tree of locks -- so we tell
it to ignore the device locks.

It sounds like XFS may need to do the same thing with its semaphores.

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-core: Fix potential null pointer dereference in xhci-debugfs.c

[Mathias Nyman]

On 08.12.2017 13:06, Alexander Kappner wrote:

Hi,


I think we need to dig a bit deeper. It's good to check if spriv is
valid
but there are probably other reasons than kzalloc failing.


I agree -- this small allocation is  unlikely to fail in practice.
Also, while my patch prevents the kernel oops, it also prevents the
debugfs entries from being created.

I've been debugging this more trying to come up with a better
solution, but I might need some guidance as I'm not too familiar with
the USB subsystem. The immediate cause of the crash was usbmuxd
sending a USBDEVFS_SETCONFIGURATION ioctl to a device, which _only if
it fails_ calls usb_hcd_alloc_bandwidth to try and reset the device,
which in turn calls xhci_debugfs_create_endpoint. The ioctl handler
acquires a device-specific lock via usb_lock_device.

When the system resumed from hibernate, xhci_resume was called. This
in turn called xhci_mem_cleanup to deallocate the device structures,
which include setting the debugfs_private pointer to NULL  (via
xhci_free_virt_devices_depth_first). It thus seems likely that the
ioctl is somehow racing with the hibernate. The call to xhci_resume
is protected by a host-controller specific lock (xhci->lock) but it
doesn't attempt to take the usb_lock_device device-specific lock.

Now my suspicion is that xhci_resume freed and zeroed the device
structures while racing with the ioctl handler. I can't seem to find
any exclusion mechanism that would prevent xhci_resume from racing
with the USBDEVFS_SETCONFIGURATION ioctl (or any other ioctl, for
that matter). Am I missing something? If not, is there any reason why
an ioctl might need to execute in parallel with the xhci_resume? If
not, can we just do a busy wait in xhci_resume until all pending
ioctls have returned?


I'm not sure, but if I recall correctly then power management is supposed
to make sure a driver doesn't access usb devices while the host controller
is still resuming.

The odd thing here is that
xhci_debugfs_remove_slot(xhci, slot_id), and
xhci_free_virt_device(xhci, slot_id) are called together when
xhci_mem_cleanup() calls xhci_free_virt_devices_depth_first()

That means both the xhci_virt_device *dev and dev->debugfs_private
should both be freed and xhci->devs[slot_id] set NULL for that virt_device.

so xhci_add_endpoint() should fail a lot earlier because the xhci->devs[slot_id]
should be a null pointer as well.

Allocation is also done together in xhci_alloc_dev()

Looking at it more closely there is actually the .free_dev callback that
first frees the dev->debugs_private but the virt_dev is only freed
conditionally later

Attached a patch that frees them together, can you try it out?

If it doesn't help we need to add some elaborate tracing

Thanks
-Mathias


>From 811aad86287e78879b7e7f28e0c266bcd2a2f73e Mon Sep 17 00:00:00 2001
From: Mathias Nyman 
Date: Fri, 8 Dec 2017 18:50:18 +0200
Subject: [PATCH] xhci: Only free xhci_virt_device->debugfs_private if device
 is freed

freeing device is conditional, we otherwise might end up with a
xhci virt_device that doesnt have a valid debugs_private pointer.
For testing only

Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 2424d30..da6dbe3 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3525,8 +3525,6 @@ static void xhci_free_dev(struct usb_hcd *hcd, struct usb_device *udev)
 	struct xhci_slot_ctx *slot_ctx;
 	int i, ret;
 
-	xhci_debugfs_remove_slot(xhci, udev->slot_id);
-
 #ifndef CONFIG_USB_DEFAULT_PERSIST
 	/*
 	 * We called pm_runtime_get_noresume when the device was attached.
@@ -3555,8 +3553,10 @@ static void xhci_free_dev(struct usb_hcd *hcd, struct usb_device *udev)
 	}
 
 	ret = xhci_disable_slot(xhci, udev->slot_id);
-	if (ret)
+	if (ret) {
+		xhci_debugfs_remove_slot(xhci, udev->slot_id);
 		xhci_free_virt_device(xhci, udev->slot_id);
+	}
 }
 
 int xhci_disable_slot(struct xhci_hcd *xhci, u32 slot_id)
-- 
2.7.4

Re: [PATCH 09/14] usb: xhci: Add DbC support in xHCI driver

[Greg KH]

On Fri, Dec 08, 2017 at 05:59:10PM +0200, Mathias Nyman wrote:
> From: Lu Baolu 
> 
> xHCI compatible USB host controllers(i.e. super-speed USB3 controllers)
> can be implemented with the Debug Capability(DbC). It presents a debug
> device which is fully compliant with the USB framework and provides the
> equivalent of a very high performance full-duplex serial link. The debug
> capability operation model and registers interface are defined in 7.6.8
> of the xHCI specification, revision 1.1.
> 
> The DbC debug device shares a root port with the xHCI host. By default,
> the debug capability is disabled and the root port is assigned to xHCI.
> When the DbC is enabled, the root port will be assigned to the DbC debug
> device, and the xHCI sees nothing on this port. This implementation uses
> a sysfs node named  under the xHCI device to manage the enabling
> and disabling of the debug capability.
> 
> When the debug capability is enabled, it will present a debug device
> through the debug port. This debug device is fully compliant with the
> USB3 framework, and it can be enumerated by a debug host on the other
> end of the USB link. As soon as the debug device is configured, a TTY
> serial device named /dev/ttyDBC0 will be created.
> 
> Signed-off-by: Lu Baolu 
> Signed-off-by: Mathias Nyman 

This patch produces the following build warning for me:

drivers/usb/host/xhci-dbgcap.c: In function ‘xhci_dbc_eps_exit’:
drivers/usb/host/xhci-dbgcap.c:369:2: warning: ‘memset’ used with length equal 
to number of elements without multiplication by element size [-Wmemset-elt-size]
  memset(dbc->eps, 0, ARRAY_SIZE(dbc->eps));
  ^~

Can you send a follow-on patch to fix it up?

And you might want to update to a newer version of gcc :)

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: [PATCH 0/4] USB over IP Secuurity fixes

[Shuah Khan]

On 12/08/2017 09:33 AM, Greg KH wrote:
> On Fri, Dec 08, 2017 at 08:44:58AM -0700, Shuah Khan wrote:
>> Hi Jakub,
>>
>> On 12/08/2017 08:14 AM, Secunia Research wrote:
>>> Hi Shuah,
>>>
>>> Thanks a lot for the quick fixes.
>>
>> Thanks for finding them and doing all the leg work in
>> pin pointing the issues.
>>
>>>
>>> Please, use this email address: v...@secunia.com
>>>
>>> We have assigned the following CVEs to the issues:
>>> CVE-2017-16911 usbip: prevent vhci_hcd driver from leaking a socket pointer
>>> address
>>> CVE-2017-16912 usbip: fix stub_rx: get_pipe() to validate endpoint number
>>> CVE-2017-16913 usbip: fix stub_rx: harden CMD_SUBMIT path to handle
>>> malicious input
>>> CVE-2017-16914 usbip: fix stub_send_ret_submit() vulnerability to null
>>> transfer_buffer
>>>
>>> Please, let me know if we should proceed with a coordinated disclosure. I'm
>>> not quite sure how many distros / downstreams actually use this
>>> functionality.
>>
>> I believe so. We have to get these into mainline and propagate them into
>> stables first which could take a couple of weeks.
>>
>> I will defer to Greg KH on this to comment and weigh in.
> 
> I've queued them all up and will send them to Linus in a few days.

Thanks.

> 
> As for "disclosure", well, you all are talking about this on a public
> mailing list, so I think there's really not much else that needs to be
> "disclosed" :)

Yes :)

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] musb-fixes for v4.15-rc3

[Greg Kroah-Hartman]

On Tue, Dec 05, 2017 at 08:42:44AM -0600, Bin Liu wrote:
> Hi Greg,
> 
> Here is only one musb patch for the next v4.15 -rc, which fixes the babble
> condition handling for DA8xx. Please let me know if any change is needed.

Now applied, 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: [PATCH 0/4] USB over IP Secuurity fixes

[Greg KH]

On Fri, Dec 08, 2017 at 08:44:58AM -0700, Shuah Khan wrote:
> Hi Jakub,
> 
> On 12/08/2017 08:14 AM, Secunia Research wrote:
> > Hi Shuah,
> > 
> > Thanks a lot for the quick fixes.
> 
> Thanks for finding them and doing all the leg work in
> pin pointing the issues.
> 
> > 
> > Please, use this email address: v...@secunia.com
> > 
> > We have assigned the following CVEs to the issues:
> > CVE-2017-16911 usbip: prevent vhci_hcd driver from leaking a socket pointer
> > address
> > CVE-2017-16912 usbip: fix stub_rx: get_pipe() to validate endpoint number
> > CVE-2017-16913 usbip: fix stub_rx: harden CMD_SUBMIT path to handle
> > malicious input
> > CVE-2017-16914 usbip: fix stub_send_ret_submit() vulnerability to null
> > transfer_buffer
> > 
> > Please, let me know if we should proceed with a coordinated disclosure. I'm
> > not quite sure how many distros / downstreams actually use this
> > functionality.
> 
> I believe so. We have to get these into mainline and propagate them into
> stables first which could take a couple of weeks.
> 
> I will defer to Greg KH on this to comment and weigh in.

I've queued them all up and will send them to Linus in a few days.

As for "disclosure", well, you all are talking about this on a public
mailing list, so I think there's really not much else that needs to be
"disclosed" :)

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: [PATCH] usb: dwc2: Change TxFIFO and RxFIFO flushing flow

[Stefan Wahren]

Hi Minas,

Am 07.12.2017 um 17:51 schrieb Stefan Wahren:

Hi Minas,

Am 07.12.2017 um 09:40 schrieb Stefan Wahren:

Before flushing fifos required to check AHB master state and
flush when AHB master is in IDLE state.

Signed-off-by: Minas Harutyunyan 
---
  drivers/usb/dwc2/core.c | 27 +++
  1 file changed, 27 insertions(+)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index dbca3b8890da..cbc7c562477f 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -670,10 +670,23 @@ void dwc2_flush_tx_fifo(struct dwc2_hsotg *hsotg, const 
int num)
  
  	dev_vdbg(hsotg->dev, "Flush Tx FIFO %d\n", num);
  
+	/* Wait for AHB master IDLE state */

+   do {
+   udelay(1);

is this delay always necessary before reading GRSTCTL?

If yes please add a comment why.
If no please rework the loop in order to avoid this delay in case the
AHB master is already idle.



i've seen your Patch V2, but it isn't what i thought of. How about:


while (1) {
    greset = dwc2_readl(hsotg->regs + GRSTCTL);
    if (greset & GRSTCTL_AHBIDLE)
        break;

    if (++count > 1) {
        dev_warn(hsotg->dev,
             "%s() HANG! AHB Idle GRSTCTL=%0x\n",
             __func__, greset);
        return;
    }
}

Btw: Please provide a change history, so the maintainers can keep track 
of your changes.


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

[PATCH 2/2] usb: xhci: fix TDS for MTK xHCI1.1

[Mathias Nyman]

From: Chunfeng Yun 

For MTK's xHCI 1.0 or latter, TD size is the number of max
packet sized packets remaining in the TD, not including
this TRB (following spec).

For MTK's xHCI 0.96 and older, TD size is the number of max
packet sized packets remaining in the TD, including this TRB
(not following spec).

Cc: stable 
Signed-off-by: Chunfeng Yun 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-ring.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 6eb87c6..c5cbc68 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -3112,7 +3112,7 @@ static u32 xhci_td_remainder(struct xhci_hcd *xhci, int 
transferred,
 {
u32 maxp, total_packet_count;
 
-   /* MTK xHCI is mostly 0.97 but contains some features from 1.0 */
+   /* MTK xHCI 0.96 contains some features from 1.0 */
if (xhci->hci_version < 0x100 && !(xhci->quirks & XHCI_MTK_HOST))
return ((td_total_len - transferred) >> 10);
 
@@ -3121,8 +3121,8 @@ static u32 xhci_td_remainder(struct xhci_hcd *xhci, int 
transferred,
trb_buff_len == td_total_len)
return 0;
 
-   /* for MTK xHCI, TD size doesn't include this TRB */
-   if (xhci->quirks & XHCI_MTK_HOST)
+   /* for MTK xHCI 0.96, TD size include this TRB, but not in 1.x */
+   if ((xhci->quirks & XHCI_MTK_HOST) && (xhci->hci_version < 0x100))
trb_buff_len = 0;
 
maxp = usb_endpoint_maxp(>ep->desc);
-- 
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

[PATCH 0/2] xhci fixes for usb-linus

[Mathias Nyman]

Hi Greg

A couple xhci fixes for usb-linus, one sets the correct MTK quirks
based on MTK host version, the other makes sure we don't add a device
to the xhci list of devices before its properly allocated.

-Mathias

Chunfeng Yun (1):
  usb: xhci: fix TDS for MTK xHCI1.1

Mathias Nyman (1):
  xhci: Don't add a virt_dev to the devs array before it's fully
allocated

 drivers/usb/host/xhci-mem.c  | 15 +++
 drivers/usb/host/xhci-ring.c |  6 +++---
 2 files changed, 14 insertions(+), 7 deletions(-)

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

[PATCH 1/2] xhci: Don't add a virt_dev to the devs array before it's fully allocated

[Mathias Nyman]

Avoid null pointer dereference if some function is walking through the
devs array accessing members of a new virt_dev that is mid allocation.

Add the virt_dev to xhci->devs[i] _after_ the virt_device and all its
members are properly allocated.

issue found by KASAN: null-ptr-deref in xhci_find_slot_id_by_port

"Quick analysis suggests that xhci_alloc_virt_device() is not mutex
protected. If so, there is a time frame where xhci->devs[slot_id] is set
but not fully initialized. Specifically, xhci->devs[i]->udev can be NULL."

Cc: stable 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-mem.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 15f7d42..3a29b32 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -971,10 +971,9 @@ int xhci_alloc_virt_device(struct xhci_hcd *xhci, int 
slot_id,
return 0;
}
 
-   xhci->devs[slot_id] = kzalloc(sizeof(*xhci->devs[slot_id]), flags);
-   if (!xhci->devs[slot_id])
+   dev = kzalloc(sizeof(*dev), flags);
+   if (!dev)
return 0;
-   dev = xhci->devs[slot_id];
 
/* Allocate the (output) device context that will be used in the HC. */
dev->out_ctx = xhci_alloc_container_ctx(xhci, XHCI_CTX_TYPE_DEVICE, 
flags);
@@ -1015,9 +1014,17 @@ int xhci_alloc_virt_device(struct xhci_hcd *xhci, int 
slot_id,
 
trace_xhci_alloc_virt_device(dev);
 
+   xhci->devs[slot_id] = dev;
+
return 1;
 fail:
-   xhci_free_virt_device(xhci, slot_id);
+
+   if (dev->in_ctx)
+   xhci_free_container_ctx(xhci, dev->in_ctx);
+   if (dev->out_ctx)
+   xhci_free_container_ctx(xhci, dev->out_ctx);
+   kfree(dev);
+
return 0;
 }
 
-- 
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

[PATCH 13/14] xhci: add port status tracing for Get Port Status hub requests

[Mathias Nyman]

Add tracing showing the port status register content each time
the xhci roothub receives a Get Port Status request.

Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-hub.c   | 1 +
 drivers/usb/host/xhci-trace.h | 5 +
 2 files changed, 6 insertions(+)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 64e1b9b..1a1856b 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -1076,6 +1076,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, 
u16 wValue,
retval = -ENODEV;
break;
}
+   trace_xhci_get_port_status(wIndex, temp);
status = xhci_get_port_status(hcd, bus_state, port_array,
wIndex, temp, flags);
if (status == 0x)
diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index 67ff314..6c093db 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -494,6 +494,11 @@ DEFINE_EVENT(xhci_log_portsc, xhci_handle_port_status,
 TP_ARGS(portnum, portsc)
 );
 
+DEFINE_EVENT(xhci_log_portsc, xhci_get_port_status,
+TP_PROTO(u32 portnum, u32 portsc),
+TP_ARGS(portnum, portsc)
+);
+
 DECLARE_EVENT_CLASS(xhci_dbc_log_request,
TP_PROTO(struct dbc_request *req),
TP_ARGS(req),
-- 
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

[PATCH 11/14] usb: xhci: Cleanup printk debug message for ERST

[Mathias Nyman]

From: Lu Baolu 

Each event segment has been exposed through debugfs. There is no
need to dump ERST content with printk in code. Remove it to make
code more concise and readable.

Signed-off-by: Lu Baolu 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-dbg.c | 18 --
 drivers/usb/host/xhci.c |  2 --
 drivers/usb/host/xhci.h |  1 -
 3 files changed, 21 deletions(-)

diff --git a/drivers/usb/host/xhci-dbg.c b/drivers/usb/host/xhci-dbg.c
index f20ef2e..386abf2 100644
--- a/drivers/usb/host/xhci-dbg.c
+++ b/drivers/usb/host/xhci-dbg.c
@@ -10,24 +10,6 @@
 
 #include "xhci.h"
 
-void xhci_dbg_erst(struct xhci_hcd *xhci, struct xhci_erst *erst)
-{
-   u64 addr = erst->erst_dma_addr;
-   int i;
-   struct xhci_erst_entry *entry;
-
-   for (i = 0; i < erst->num_entries; i++) {
-   entry = >entries[i];
-   xhci_dbg(xhci, "@%016llx %08x %08x %08x %08x\n",
-addr,
-lower_32_bits(le64_to_cpu(entry->seg_addr)),
-upper_32_bits(le64_to_cpu(entry->seg_addr)),
-le32_to_cpu(entry->seg_size),
-le32_to_cpu(entry->rsvd));
-   addr += sizeof(*entry);
-   }
-}
-
 char *xhci_get_slot_state(struct xhci_hcd *xhci,
struct xhci_container_ctx *ctx)
 {
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index a66540d..060e5b4 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -574,8 +574,6 @@ int xhci_run(struct usb_hcd *hcd)
if (ret)
return ret;
 
-   xhci_dbg(xhci, "ERST memory map follows:\n");
-   xhci_dbg_erst(xhci, >erst);
temp_64 = xhci_read_64(xhci, >ir_set->erst_dequeue);
temp_64 &= ~ERST_PTR_MASK;
xhci_dbg_trace(xhci, trace_xhci_dbg_init,
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 8ab2d83..7c87817 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1925,7 +1925,6 @@ static inline int xhci_link_trb_quirk(struct xhci_hcd 
*xhci)
 }
 
 /* xHCI debugging */
-void xhci_dbg_erst(struct xhci_hcd *xhci, struct xhci_erst *erst);
 char *xhci_get_slot_state(struct xhci_hcd *xhci,
struct xhci_container_ctx *ctx);
 void xhci_dbg_trace(struct xhci_hcd *xhci, void (*trace)(struct va_format *),
-- 
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

[PATCH 14/14] xhci: add port status tracing for Get Hub Status requests

[Mathias Nyman]

Trace the port status of each port of a roothub when
the xhci roothub receives a Get Hub Status request.

Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-hub.c   | 2 ++
 drivers/usb/host/xhci-trace.h | 5 +
 2 files changed, 7 insertions(+)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 1a1856b..46d5e08 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -1443,6 +1443,8 @@ int xhci_hub_status_data(struct usb_hcd *hcd, char *buf)
retval = -ENODEV;
break;
}
+   trace_xhci_hub_status_data(i, temp);
+
if ((temp & mask) != 0 ||
(bus_state->port_c_suspend & 1 << i) ||
(bus_state->resume_done[i] && time_after_eq(
diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index 6c093db..410544f 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -499,6 +499,11 @@ DEFINE_EVENT(xhci_log_portsc, xhci_get_port_status,
 TP_ARGS(portnum, portsc)
 );
 
+DEFINE_EVENT(xhci_log_portsc, xhci_hub_status_data,
+TP_PROTO(u32 portnum, u32 portsc),
+TP_ARGS(portnum, portsc)
+);
+
 DECLARE_EVENT_CLASS(xhci_dbc_log_request,
TP_PROTO(struct dbc_request *req),
TP_ARGS(req),
-- 
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

[PATCH 12/14] usb: xhci: allow imod-interval to be configurable

[Mathias Nyman]

From: Adam Wallis 

The xHCI driver currently has the IMOD set to 160, which
translates to an IMOD interval of 40,000ns (160 * 250)ns

Commit 0cbd4b34cda9 ("xhci: mediatek: support MTK xHCI host controller")
introduced a QUIRK for the MTK platform to adjust this interval to 20,
which translates to an IMOD interval of 5,000ns (20 * 250)ns. This is
due to the fact that the MTK controller IMOD interval is 8 times
as much as defined in xHCI spec.

Instead of adding more quirk bits for additional platforms, this patch
introduces the ability for vendors to set the IMOD_INTERVAL as is
optimal for their platform. By using device_property_read_u32() on
"imod-interval-ns", the IMOD INTERVAL can be specified in nano seconds.
If no interval is specified, the default of 40,000ns (IMOD=160) will be
used.

No bounds checking has been implemented due to the fact that a vendor
may have violated the spec and would need to specify a value outside of
the max 8,000 IRQs/second limit specified in the xHCI spec.

Tested-by: Chunfeng Yun 
Reviewed-by: Rob Herring 
Signed-off-by: Adam Wallis 
Signed-off-by: Mathias Nyman 
---
 Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt | 2 ++
 Documentation/devicetree/bindings/usb/usb-xhci.txt  | 1 +
 drivers/usb/host/xhci-mtk.c | 9 +
 drivers/usb/host/xhci-pci.c | 3 +++
 drivers/usb/host/xhci-plat.c| 5 +
 drivers/usb/host/xhci.c | 6 +-
 drivers/usb/host/xhci.h | 2 ++
 7 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt 
b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
index 3059596..9ff5602 100644
--- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
+++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
@@ -46,6 +46,7 @@ Optional properties:
  - pinctrl-names : a pinctrl state named "default" must be defined
  - pinctrl-0 : pin control group
See: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
+ - imod-interval-ns: default interrupt moderation interval is 5000ns
 
 Example:
 usb30: usb@1127 {
@@ -66,6 +67,7 @@ usb30: usb@1127 {
usb3-lpm-capable;
mediatek,syscon-wakeup = <>;
mediatek,wakeup-src = <1>;
+   imod-interval-ns = <1>;
 };
 
 2nd: dual-role mode with xHCI driver
diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt 
b/Documentation/devicetree/bindings/usb/usb-xhci.txt
index a531071..e2ea59b 100644
--- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
+++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
@@ -31,6 +31,7 @@ Optional properties:
   - usb2-lpm-disable: indicate if we don't want to enable USB2 HW LPM
   - usb3-lpm-capable: determines if platform is USB3 LPM capable
   - quirk-broken-port-ped: set if the controller has broken port disable 
mechanism
+  - imod-interval-ns: default interrupt moderation interval is 5000ns
 
 Example:
usb@f0931000 {
diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index b62a1d2..1cb2a8b 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -674,6 +674,15 @@ static int xhci_mtk_probe(struct platform_device *pdev)
 
xhci = hcd_to_xhci(hcd);
xhci->main_hcd = hcd;
+
+   /*
+* imod_interval is the interrupt moderation value in nanoseconds.
+* The increment interval is 8 times as much as that defined in
+* the xHCI spec on MTK's controller.
+*/
+   xhci->imod_interval = 5000;
+   device_property_read_u32(dev, "imod-interval-ns", >imod_interval);
+
xhci->shared_hcd = usb_create_shared_hcd(driver, dev,
dev_name(dev), hcd);
if (!xhci->shared_hcd) {
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 7ef1274..4bcddd4 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -234,6 +234,9 @@ static int xhci_pci_setup(struct usb_hcd *hcd)
if (!xhci->sbrn)
pci_read_config_byte(pdev, XHCI_SBRN_OFFSET, >sbrn);
 
+   /* imod_interval is the interrupt moderation value in nanoseconds. */
+   xhci->imod_interval = 4;
+
retval = xhci_gen_setup(hcd, xhci_pci_quirks);
if (retval)
return retval;
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 09f164f..6f03830 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -269,6 +269,11 @@ static int xhci_plat_probe(struct platform_device *pdev)
if (device_property_read_bool(>dev, "quirk-broken-port-ped"))
xhci->quirks |= XHCI_BROKEN_PORT_PED;
 
+   /* imod_interval is the 

[PATCH 10/14] usb: xhci: Cleanup printk debug message for registers

[Mathias Nyman]

From: Lu Baolu 

The content of each register has been exposed through debugfs.
There is no need to dump register content with printk in code
lines. Remove them to make code more concise and readable.

Signed-off-by: Lu Baolu 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-dbg.c | 243 
 drivers/usb/host/xhci-mem.c |   4 -
 drivers/usb/host/xhci.c |   6 --
 drivers/usb/host/xhci.h |   5 -
 4 files changed, 258 deletions(-)

diff --git a/drivers/usb/host/xhci-dbg.c b/drivers/usb/host/xhci-dbg.c
index 584d7b9..f20ef2e 100644
--- a/drivers/usb/host/xhci-dbg.c
+++ b/drivers/usb/host/xhci-dbg.c
@@ -10,238 +10,6 @@
 
 #include "xhci.h"
 
-#define XHCI_INIT_VALUE 0x0
-
-/* Add verbose debugging later, just print everything for now */
-
-void xhci_dbg_regs(struct xhci_hcd *xhci)
-{
-   u32 temp;
-
-   xhci_dbg(xhci, "// xHCI capability registers at %p:\n",
-   xhci->cap_regs);
-   temp = readl(>cap_regs->hc_capbase);
-   xhci_dbg(xhci, "// @%p = 0x%x (CAPLENGTH AND HCIVERSION)\n",
-   >cap_regs->hc_capbase, temp);
-   xhci_dbg(xhci, "//   CAPLENGTH: 0x%x\n",
-   (unsigned int) HC_LENGTH(temp));
-   xhci_dbg(xhci, "//   HCIVERSION: 0x%x\n",
-   (unsigned int) HC_VERSION(temp));
-
-   xhci_dbg(xhci, "// xHCI operational registers at %p:\n", xhci->op_regs);
-
-   temp = readl(>cap_regs->run_regs_off);
-   xhci_dbg(xhci, "// @%p = 0x%x RTSOFF\n",
-   >cap_regs->run_regs_off,
-   (unsigned int) temp & RTSOFF_MASK);
-   xhci_dbg(xhci, "// xHCI runtime registers at %p:\n", xhci->run_regs);
-
-   temp = readl(>cap_regs->db_off);
-   xhci_dbg(xhci, "// @%p = 0x%x DBOFF\n", >cap_regs->db_off, temp);
-   xhci_dbg(xhci, "// Doorbell array at %p:\n", xhci->dba);
-}
-
-static void xhci_print_cap_regs(struct xhci_hcd *xhci)
-{
-   u32 temp;
-   u32 hci_version;
-
-   xhci_dbg(xhci, "xHCI capability registers at %p:\n", xhci->cap_regs);
-
-   temp = readl(>cap_regs->hc_capbase);
-   hci_version = HC_VERSION(temp);
-   xhci_dbg(xhci, "CAPLENGTH AND HCIVERSION 0x%x:\n",
-   (unsigned int) temp);
-   xhci_dbg(xhci, "CAPLENGTH: 0x%x\n",
-   (unsigned int) HC_LENGTH(temp));
-   xhci_dbg(xhci, "HCIVERSION: 0x%x\n", hci_version);
-
-   temp = readl(>cap_regs->hcs_params1);
-   xhci_dbg(xhci, "HCSPARAMS 1: 0x%x\n",
-   (unsigned int) temp);
-   xhci_dbg(xhci, "  Max device slots: %u\n",
-   (unsigned int) HCS_MAX_SLOTS(temp));
-   xhci_dbg(xhci, "  Max interrupters: %u\n",
-   (unsigned int) HCS_MAX_INTRS(temp));
-   xhci_dbg(xhci, "  Max ports: %u\n",
-   (unsigned int) HCS_MAX_PORTS(temp));
-
-   temp = readl(>cap_regs->hcs_params2);
-   xhci_dbg(xhci, "HCSPARAMS 2: 0x%x\n",
-   (unsigned int) temp);
-   xhci_dbg(xhci, "  Isoc scheduling threshold: %u\n",
-   (unsigned int) HCS_IST(temp));
-   xhci_dbg(xhci, "  Maximum allowed segments in event ring: %u\n",
-   (unsigned int) HCS_ERST_MAX(temp));
-
-   temp = readl(>cap_regs->hcs_params3);
-   xhci_dbg(xhci, "HCSPARAMS 3 0x%x:\n",
-   (unsigned int) temp);
-   xhci_dbg(xhci, "  Worst case U1 device exit latency: %u\n",
-   (unsigned int) HCS_U1_LATENCY(temp));
-   xhci_dbg(xhci, "  Worst case U2 device exit latency: %u\n",
-   (unsigned int) HCS_U2_LATENCY(temp));
-
-   temp = readl(>cap_regs->hcc_params);
-   xhci_dbg(xhci, "HCC PARAMS 0x%x:\n", (unsigned int) temp);
-   xhci_dbg(xhci, "  HC generates %s bit addresses\n",
-   HCC_64BIT_ADDR(temp) ? "64" : "32");
-   xhci_dbg(xhci, "  HC %s Contiguous Frame ID Capability\n",
-   HCC_CFC(temp) ? "has" : "hasn't");
-   xhci_dbg(xhci, "  HC %s generate Stopped - Short Package event\n",
-   HCC_SPC(temp) ? "can" : "can't");
-   /* FIXME */
-   xhci_dbg(xhci, "  FIXME: more HCCPARAMS debugging\n");
-
-   temp = readl(>cap_regs->run_regs_off);
-   xhci_dbg(xhci, "RTSOFF 0x%x:\n", temp & RTSOFF_MASK);
-
-   /* xhci 1.1 controllers have the HCCPARAMS2 register */
-   if (hci_version > 0x100) {
-   temp = readl(>cap_regs->hcc_params2);
-   xhci_dbg(xhci, "HCC PARAMS2 0x%x:\n", (unsigned int) temp);
-   xhci_dbg(xhci, "  HC %s Force save context capability",
-HCC2_FSC(temp) ? "supports" : "doesn't support");
-   xhci_dbg(xhci, "  HC %s Large ESIT Payload Capability",
-HCC2_LEC(temp) ? "supports" : "doesn't support");
-   

[PATCH 05/14] dt-bindings: usb-xhci: Document r8a7743 support

[Mathias Nyman]

From: Fabrizio Castro 

Document r8a7743 xhci support. The driver will use the fallback
compatible string "renesas,rcar-gen2-xhci", therefore no driver
change is needed.

Signed-off-by: Fabrizio Castro 
Reviewed-by: Yoshihiro Shimoda 
Signed-off-by: Mathias Nyman 
---
 Documentation/devicetree/bindings/usb/usb-xhci.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt 
b/Documentation/devicetree/bindings/usb/usb-xhci.txt
index ae6e484..a531071 100644
--- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
+++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
@@ -7,12 +7,14 @@ Required properties:
 - "marvell,armada3700-xhci" for Armada 37xx SoCs
 - "marvell,armada-375-xhci" for Armada 375 SoCs
 - "marvell,armada-380-xhci" for Armada 38x SoCs
+- "renesas,xhci-r8a7743" for r8a7743 SoC
 - "renesas,xhci-r8a7790" for r8a7790 SoC
 - "renesas,xhci-r8a7791" for r8a7791 SoC
 - "renesas,xhci-r8a7793" for r8a7793 SoC
 - "renesas,xhci-r8a7795" for r8a7795 SoC
 - "renesas,xhci-r8a7796" for r8a7796 SoC
-- "renesas,rcar-gen2-xhci" for a generic R-Car Gen2 compatible device
+- "renesas,rcar-gen2-xhci" for a generic R-Car Gen2 or RZ/G1 compatible
+  device
 - "renesas,rcar-gen3-xhci" for a generic R-Car Gen3 compatible device
 - "xhci-platform" (deprecated)
 
-- 
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

[PATCH 07/14] xhci: remove unnecessary boolean parameter from xhci_alloc_command

[Mathias Nyman]

commands with input contexts are allocated with the
xhci_alloc_command_with_ctx helper.

No functional changes

Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-hub.c  |  5 ++---
 drivers/usb/host/xhci-mem.c  | 17 ++---
 drivers/usb/host/xhci-ring.c |  6 +++---
 drivers/usb/host/xhci.c  | 16 
 drivers/usb/host/xhci.h  |  3 +--
 5 files changed, 16 insertions(+), 31 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 2a90229..64e1b9b 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -388,7 +388,7 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int 
slot_id, int suspend)
 
trace_xhci_stop_device(virt_dev);
 
-   cmd = xhci_alloc_command(xhci, false, true, GFP_NOIO);
+   cmd = xhci_alloc_command(xhci, true, GFP_NOIO);
if (!cmd)
return -ENOMEM;
 
@@ -404,8 +404,7 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int 
slot_id, int suspend)
if (GET_EP_CTX_STATE(ep_ctx) != EP_STATE_RUNNING)
continue;
 
-   command = xhci_alloc_command(xhci, false, false,
-GFP_NOWAIT);
+   command = xhci_alloc_command(xhci, false, GFP_NOWAIT);
if (!command) {
spin_unlock_irqrestore(>lock, flags);
ret = -ENOMEM;
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 7ab4020..824651c 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1701,8 +1701,7 @@ static void scratchpad_free(struct xhci_hcd *xhci)
 }
 
 struct xhci_command *xhci_alloc_command(struct xhci_hcd *xhci,
-   bool allocate_in_ctx, bool allocate_completion,
-   gfp_t mem_flags)
+   bool allocate_completion, gfp_t mem_flags)
 {
struct xhci_command *command;
 
@@ -1710,21 +1709,10 @@ struct xhci_command *xhci_alloc_command(struct xhci_hcd 
*xhci,
if (!command)
return NULL;
 
-   if (allocate_in_ctx) {
-   command->in_ctx =
-   xhci_alloc_container_ctx(xhci, XHCI_CTX_TYPE_INPUT,
-   mem_flags);
-   if (!command->in_ctx) {
-   kfree(command);
-   return NULL;
-   }
-   }
-
if (allocate_completion) {
command->completion =
kzalloc(sizeof(struct completion), mem_flags);
if (!command->completion) {
-   xhci_free_container_ctx(xhci, command->in_ctx);
kfree(command);
return NULL;
}
@@ -1741,8 +1729,7 @@ struct xhci_command *xhci_alloc_command_with_ctx(struct 
xhci_hcd *xhci,
 {
struct xhci_command *command;
 
-   command = xhci_alloc_command(xhci, false, allocate_completion,
-mem_flags);
+   command = xhci_alloc_command(xhci, allocate_completion, mem_flags);
if (!command)
return NULL;
 
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 6e55520..e56f1ce 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1141,7 +1141,7 @@ static void xhci_handle_cmd_reset_ep(struct xhci_hcd 
*xhci, int slot_id,
if (xhci->quirks & XHCI_RESET_EP_QUIRK) {
struct xhci_command *command;
 
-   command = xhci_alloc_command(xhci, false, false, GFP_ATOMIC);
+   command = xhci_alloc_command(xhci, false, GFP_ATOMIC);
if (!command)
return;
 
@@ -1821,7 +1821,7 @@ static void xhci_cleanup_halted_endpoint(struct xhci_hcd 
*xhci,
 {
struct xhci_virt_ep *ep = >devs[slot_id]->eps[ep_index];
struct xhci_command *command;
-   command = xhci_alloc_command(xhci, false, false, GFP_ATOMIC);
+   command = xhci_alloc_command(xhci, false, GFP_ATOMIC);
if (!command)
return;
 
@@ -4036,7 +4036,7 @@ void xhci_queue_new_dequeue_state(struct xhci_hcd *xhci,
}
 
/* This function gets called from contexts where it cannot sleep */
-   cmd = xhci_alloc_command(xhci, false, false, GFP_ATOMIC);
+   cmd = xhci_alloc_command(xhci, false, GFP_ATOMIC);
if (!cmd)
return;
 
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 5d99a60..f25b4ce3 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -610,7 +610,7 @@ int xhci_run(struct usb_hcd *hcd)
if (xhci->quirks & XHCI_NEC_HOST) {
struct xhci_command *command;
 
-   command = xhci_alloc_command(xhci, false, false, GFP_KERNEL);
+   command = xhci_alloc_command(xhci, false, 

[PATCH 09/14] usb: xhci: Add DbC support in xHCI driver

[Mathias Nyman]

From: Lu Baolu 

xHCI compatible USB host controllers(i.e. super-speed USB3 controllers)
can be implemented with the Debug Capability(DbC). It presents a debug
device which is fully compliant with the USB framework and provides the
equivalent of a very high performance full-duplex serial link. The debug
capability operation model and registers interface are defined in 7.6.8
of the xHCI specification, revision 1.1.

The DbC debug device shares a root port with the xHCI host. By default,
the debug capability is disabled and the root port is assigned to xHCI.
When the DbC is enabled, the root port will be assigned to the DbC debug
device, and the xHCI sees nothing on this port. This implementation uses
a sysfs node named  under the xHCI device to manage the enabling
and disabling of the debug capability.

When the debug capability is enabled, it will present a debug device
through the debug port. This debug device is fully compliant with the
USB3 framework, and it can be enumerated by a debug host on the other
end of the USB link. As soon as the debug device is configured, a TTY
serial device named /dev/ttyDBC0 will be created.

Signed-off-by: Lu Baolu 
Signed-off-by: Mathias Nyman 
---
 .../ABI/testing/sysfs-bus-pci-drivers-xhci_hcd |  25 +
 drivers/usb/host/Kconfig   |   8 +
 drivers/usb/host/Makefile  |   5 +
 drivers/usb/host/xhci-dbgcap.c | 996 +
 drivers/usb/host/xhci-dbgcap.h | 229 +
 drivers/usb/host/xhci-dbgtty.c | 497 ++
 drivers/usb/host/xhci-trace.h  |  59 ++
 drivers/usb/host/xhci.c|   9 +
 drivers/usb/host/xhci.h|   1 +
 9 files changed, 1829 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
 create mode 100644 drivers/usb/host/xhci-dbgcap.c
 create mode 100644 drivers/usb/host/xhci-dbgcap.h
 create mode 100644 drivers/usb/host/xhci-dbgtty.c

diff --git a/Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd 
b/Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
new file mode 100644
index 000..0088aba
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
@@ -0,0 +1,25 @@
+What:  /sys/bus/pci/drivers/xhci_hcd/.../dbc
+Date:  June 2017
+Contact:   Lu Baolu 
+Description:
+   xHCI compatible USB host controllers (i.e. super-speed
+   USB3 controllers) are often implemented with the Debug
+   Capability (DbC). It can present a debug device which
+   is fully compliant with the USB framework and provides
+   the equivalent of a very high performance full-duplex
+   serial link for debug purpose.
+
+   The DbC debug device shares a root port with xHCI host.
+   When the DbC is enabled, the root port will be assigned
+   to the Debug Capability. Otherwise, it will be assigned
+   to xHCI.
+
+   Writing "enable" to this attribute will enable the DbC
+   functionality and the shared root port will be assigned
+   to the DbC device. Writing "disable" to this attribute
+   will disable the DbC functionality and the shared root
+   port will roll back to the xHCI.
+
+   Reading this attribute gives the state of the DbC. It
+   can be one of the following states: disabled, enabled,
+   initialized, connected, configured and stalled.
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index b80a94e..6150bed 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -27,6 +27,14 @@ config USB_XHCI_HCD
  module will be called xhci-hcd.
 
 if USB_XHCI_HCD
+config USB_XHCI_DBGCAP
+   bool "xHCI support for debug capability"
+   depends on TTY
+   ---help---
+ Say 'Y' to enable the support for the xHCI debug capability. Make
+ sure that your xHCI host supports the extended debug capability and
+ you want a TTY serial device based on the xHCI debug capability
+ before enabling this option. If unsure, say 'N'.
 
 config USB_XHCI_PCI
tristate
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index 32b036e..4ede4ce 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -14,6 +14,11 @@ fhci-$(CONFIG_FHCI_DEBUG) += fhci-dbg.o
 xhci-hcd-y := xhci.o xhci-mem.o
 xhci-hcd-y += xhci-ring.o xhci-hub.o xhci-dbg.o
 xhci-hcd-y += xhci-trace.o
+
+ifneq ($(CONFIG_USB_XHCI_DBGCAP), )
+   xhci-hcd-y += xhci-dbgcap.o xhci-dbgtty.o
+endif
+
 ifneq ($(CONFIG_USB_XHCI_MTK), )
xhci-hcd-y += xhci-mtk-sch.o
 endif
diff --git a/drivers/usb/host/xhci-dbgcap.c 

[PATCH 03/14] usb: xhci: remove unused variable urb_priv

[Mathias Nyman]

From: Corentin Labbe 

Fix the build warning: variable 'urb_priv' set but not used

Signed-off-by: Corentin Labbe 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-ring.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index c239c68..9a914b6 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1878,12 +1878,10 @@ int xhci_is_vendor_info_code(struct xhci_hcd *xhci, 
unsigned int trb_comp_code)
 static int xhci_td_cleanup(struct xhci_hcd *xhci, struct xhci_td *td,
struct xhci_ring *ep_ring, int *status)
 {
-   struct urb_priv *urb_priv;
struct urb *urb = NULL;
 
/* Clean up the endpoint's TD list */
urb = td->urb;
-   urb_priv = urb->hcpriv;
 
/* if a bounce buffer was used to align this td then unmap it */
xhci_unmap_td_bounce_buffer(xhci, ep_ring, td);
-- 
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

[PATCH 04/14] usb: xhci: remove unused variable ep_ring

[Mathias Nyman]

From: Corentin Labbe 

Fix the build warning about variable 'ep_ring' set but not used

[Minor commit message change -Mathias]
Signed-off-by: Corentin Labbe 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-ring.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 9a914b6..6e55520 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1992,7 +1992,6 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct 
xhci_td *td,
struct xhci_virt_ep *ep, int *status)
 {
struct xhci_virt_device *xdev;
-   struct xhci_ring *ep_ring;
unsigned int slot_id;
int ep_index;
struct xhci_ep_ctx *ep_ctx;
@@ -2004,7 +2003,6 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct 
xhci_td *td,
slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event->flags));
xdev = xhci->devs[slot_id];
ep_index = TRB_TO_EP_ID(le32_to_cpu(event->flags)) - 1;
-   ep_ring = xhci_dma_to_transfer_ring(ep, le64_to_cpu(event->buffer));
ep_ctx = xhci_get_ep_ctx(xhci, xdev->out_ctx, ep_index);
trb_comp_code = GET_COMP_CODE(le32_to_cpu(event->transfer_len));
requested = td->urb->transfer_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

[PATCH 06/14] xhci: add helper to allocate command with input context

[Mathias Nyman]

Add a xhci_alloc_command_with_ctx() helper to get rid of
one of the boolean parameters telling if a context should
be allocated with the command.

No functional changes, improves core readability

Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-mem.c | 24 ++--
 drivers/usb/host/xhci.c |  4 ++--
 drivers/usb/host/xhci.h |  2 ++
 3 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index e1fba46..7ab4020 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -650,7 +650,7 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct 
xhci_hcd *xhci,
 
/* Allocate everything needed to free the stream rings later */
stream_info->free_streams_command =
-   xhci_alloc_command(xhci, true, true, mem_flags);
+   xhci_alloc_command_with_ctx(xhci, true, mem_flags);
if (!stream_info->free_streams_command)
goto cleanup_ctx;
 
@@ -1736,6 +1736,26 @@ struct xhci_command *xhci_alloc_command(struct xhci_hcd 
*xhci,
return command;
 }
 
+struct xhci_command *xhci_alloc_command_with_ctx(struct xhci_hcd *xhci,
+   bool allocate_completion, gfp_t mem_flags)
+{
+   struct xhci_command *command;
+
+   command = xhci_alloc_command(xhci, false, allocate_completion,
+mem_flags);
+   if (!command)
+   return NULL;
+
+   command->in_ctx = xhci_alloc_container_ctx(xhci, XHCI_CTX_TYPE_INPUT,
+  mem_flags);
+   if (!command->in_ctx) {
+   kfree(command->completion);
+   kfree(command);
+   return NULL;
+   }
+   return command;
+}
+
 void xhci_urb_free_priv(struct urb_priv *urb_priv)
 {
kfree(urb_priv);
@@ -2409,7 +2429,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
xhci_write_64(xhci, val_64, >op_regs->cmd_ring);
xhci_dbg_cmd_ptrs(xhci);
 
-   xhci->lpm_command = xhci_alloc_command(xhci, true, true, flags);
+   xhci->lpm_command = xhci_alloc_command_with_ctx(xhci, true, flags);
if (!xhci->lpm_command)
goto fail;
 
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 89ad7c0..5d99a60 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3090,7 +3090,7 @@ static int xhci_alloc_streams(struct usb_hcd *hcd, struct 
usb_device *udev,
return -ENOSYS;
}
 
-   config_cmd = xhci_alloc_command(xhci, true, true, mem_flags);
+   config_cmd = xhci_alloc_command_with_ctx(xhci, true, mem_flags);
if (!config_cmd)
return -ENOMEM;
 
@@ -4675,7 +4675,7 @@ static int xhci_update_hub_device(struct usb_hcd *hcd, 
struct usb_device *hdev,
return -EINVAL;
}
 
-   config_cmd = xhci_alloc_command(xhci, true, true, mem_flags);
+   config_cmd = xhci_alloc_command_with_ctx(xhci, true, mem_flags);
if (!config_cmd)
return -ENOMEM;
 
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 99a014a..147f1a0 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1994,6 +1994,8 @@ struct xhci_ring *xhci_stream_id_to_ring(
 struct xhci_command *xhci_alloc_command(struct xhci_hcd *xhci,
bool allocate_in_ctx, bool allocate_completion,
gfp_t mem_flags);
+struct xhci_command *xhci_alloc_command_with_ctx(struct xhci_hcd *xhci,
+   bool allocate_completion, gfp_t mem_flags);
 void xhci_urb_free_priv(struct urb_priv *urb_priv);
 void xhci_free_command(struct xhci_hcd *xhci,
struct xhci_command *command);
-- 
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

[PATCH 08/14] usb: xhci: Make some static functions global

[Mathias Nyman]

From: Lu Baolu 

This patch makes some static functions global to avoid duplications
in different files. These functions can be used in the implementation
of xHCI debug capability. There is no functional change.

Signed-off-by: Lu Baolu 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-mem.c  | 94 ++--
 drivers/usb/host/xhci-ring.c |  4 +-
 drivers/usb/host/xhci.h  | 16 +++-
 3 files changed, 72 insertions(+), 42 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 824651c..5bee81a 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -357,7 +357,7 @@ static int xhci_alloc_segments_for_ring(struct xhci_hcd 
*xhci,
  * Set the end flag and the cycle toggle bit on the last segment.
  * See section 4.9.1 and figures 15 and 16.
  */
-static struct xhci_ring *xhci_ring_alloc(struct xhci_hcd *xhci,
+struct xhci_ring *xhci_ring_alloc(struct xhci_hcd *xhci,
unsigned int num_segs, unsigned int cycle_state,
enum xhci_ring_type type, unsigned int max_packet, gfp_t flags)
 {
@@ -454,7 +454,7 @@ int xhci_ring_expansion(struct xhci_hcd *xhci, struct 
xhci_ring *ring,
return 0;
 }
 
-static struct xhci_container_ctx *xhci_alloc_container_ctx(struct xhci_hcd 
*xhci,
+struct xhci_container_ctx *xhci_alloc_container_ctx(struct xhci_hcd *xhci,
int type, gfp_t flags)
 {
struct xhci_container_ctx *ctx;
@@ -479,7 +479,7 @@ static struct xhci_container_ctx 
*xhci_alloc_container_ctx(struct xhci_hcd *xhci
return ctx;
 }
 
-static void xhci_free_container_ctx(struct xhci_hcd *xhci,
+void xhci_free_container_ctx(struct xhci_hcd *xhci,
 struct xhci_container_ctx *ctx)
 {
if (!ctx)
@@ -1757,21 +1757,61 @@ void xhci_free_command(struct xhci_hcd *xhci,
kfree(command);
 }
 
+int xhci_alloc_erst(struct xhci_hcd *xhci,
+   struct xhci_ring *evt_ring,
+   struct xhci_erst *erst,
+   gfp_t flags)
+{
+   size_t size;
+   unsigned int val;
+   struct xhci_segment *seg;
+   struct xhci_erst_entry *entry;
+
+   size = sizeof(struct xhci_erst_entry) * evt_ring->num_segs;
+   erst->entries = dma_alloc_coherent(xhci_to_hcd(xhci)->self.sysdev,
+  size,
+  >erst_dma_addr,
+  flags);
+   if (!erst->entries)
+   return -ENOMEM;
+
+   memset(erst->entries, 0, size);
+   erst->num_entries = evt_ring->num_segs;
+
+   seg = evt_ring->first_seg;
+   for (val = 0; val < evt_ring->num_segs; val++) {
+   entry = >entries[val];
+   entry->seg_addr = cpu_to_le64(seg->dma);
+   entry->seg_size = cpu_to_le32(TRBS_PER_SEGMENT);
+   entry->rsvd = 0;
+   seg = seg->next;
+   }
+
+   return 0;
+}
+
+void xhci_free_erst(struct xhci_hcd *xhci, struct xhci_erst *erst)
+{
+   size_t size;
+   struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
+
+   size = sizeof(struct xhci_erst_entry) * (erst->num_entries);
+   if (erst->entries)
+   dma_free_coherent(dev, size,
+   erst->entries,
+   erst->erst_dma_addr);
+   erst->entries = NULL;
+}
+
 void xhci_mem_cleanup(struct xhci_hcd *xhci)
 {
struct device   *dev = xhci_to_hcd(xhci)->self.sysdev;
-   int size;
int i, j, num_ports;
 
cancel_delayed_work_sync(>cmd_timer);
 
-   /* Free the Event Ring Segment Table and the actual Event Ring */
-   size = sizeof(struct xhci_erst_entry)*(xhci->erst.num_entries);
-   if (xhci->erst.entries)
-   dma_free_coherent(dev, size,
-   xhci->erst.entries, xhci->erst.erst_dma_addr);
-   xhci->erst.entries = NULL;
-   xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed ERST");
+   xhci_free_erst(xhci, >erst);
+
if (xhci->event_ring)
xhci_ring_free(xhci, xhci->event_ring);
xhci->event_ring = NULL;
@@ -2308,9 +2348,8 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
struct device   *dev = xhci_to_hcd(xhci)->self.sysdev;
unsigned intval, val2;
u64 val_64;
-   struct xhci_segment *seg;
-   u32 page_size, temp;
-   int i;
+   u32 page_size, temp;
+   int i, ret;
 
INIT_LIST_HEAD(>cmd_list);
 
@@ -2449,32 +2488,9 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
if (xhci_check_trb_in_td_math(xhci) < 0)
goto fail;
 
-   xhci->erst.entries = dma_alloc_coherent(dev,
-   sizeof(struct xhci_erst_entry) * 

[PATCH 02/14] usb: xhci: remove unused variable ep

[Mathias Nyman]

From: Corentin Labbe 

Fix the build warning: variable 'ep' set but not used

Signed-off-by: Corentin Labbe 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 31e3e46..89ad7c0 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -2838,12 +2838,10 @@ void xhci_cleanup_stalled_ring(struct xhci_hcd *xhci, 
unsigned int ep_index,
   unsigned int stream_id, struct xhci_td *td)
 {
struct xhci_dequeue_state deq_state;
-   struct xhci_virt_ep *ep;
struct usb_device *udev = td->urb->dev;
 
xhci_dbg_trace(xhci, trace_xhci_dbg_reset_ep,
"Cleaning up stalled endpoint ring");
-   ep = >devs[udev->slot_id]->eps[ep_index];
/* We need to move the HW's dequeue pointer past this TD,
 * or it will attempt to resend it on the next doorbell ring.
 */
-- 
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

[PATCH 01/14] usb: xhci: remove unused variable last_freed_endpoint

[Mathias Nyman]

From: Corentin Labbe 

Fix the build warning about variable 'last_freed_endpoint'
set but not used [-Wunused-but-set-variable]

Signed-off-by: Corentin Labbe 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 2424d30..31e3e46 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3363,7 +3363,6 @@ static int xhci_discover_or_reset_device(struct usb_hcd 
*hcd,
unsigned int slot_id;
struct xhci_virt_device *virt_dev;
struct xhci_command *reset_device_cmd;
-   int last_freed_endpoint;
struct xhci_slot_ctx *slot_ctx;
int old_active_eps = 0;
 
@@ -3478,7 +3477,6 @@ static int xhci_discover_or_reset_device(struct usb_hcd 
*hcd,
}
 
/* Everything but endpoint 0 is disabled, so free the rings. */
-   last_freed_endpoint = 1;
for (i = 1; i < 31; i++) {
struct xhci_virt_ep *ep = _dev->eps[i];
 
@@ -3493,7 +3491,6 @@ static int xhci_discover_or_reset_device(struct usb_hcd 
*hcd,
if (ep->ring) {
xhci_debugfs_remove_endpoint(xhci, virt_dev, i);
xhci_free_endpoint_ring(xhci, virt_dev, i);
-   last_freed_endpoint = i;
}
if (!list_empty(_dev->eps[i].bw_endpoint_list))
xhci_drop_ep_from_interval_table(xhci,
-- 
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

[PATCH 00/14] xhci features for usb-next

[Mathias Nyman]

Hi Greg

This series adds DbC (Debug Capaility) support to the xhci driver.
It also includes some more tracing, does some cleanup, and allows
different platfors to set their own interrupt moredation values for xchi.

-Mathias

Adam Wallis (1):
  usb: xhci: allow imod-interval to be configurable

Corentin Labbe (4):
  usb: xhci: remove unused variable last_freed_endpoint
  usb: xhci: remove unused variable ep
  usb: xhci: remove unused variable urb_priv
  usb: xhci: remove unused variable ep_ring

Fabrizio Castro (1):
  dt-bindings: usb-xhci: Document r8a7743 support

Lu Baolu (4):
  usb: xhci: Make some static functions global
  usb: xhci: Add DbC support in xHCI driver
  usb: xhci: Cleanup printk debug message for registers
  usb: xhci: Cleanup printk debug message for ERST

Mathias Nyman (4):
  xhci: add helper to allocate command with input context
  xhci: remove unnecessary boolean parameter from xhci_alloc_command
  xhci: add port status tracing for Get Port Status hub requests
  xhci: add port status tracing for Get Hub Status requests

 .../ABI/testing/sysfs-bus-pci-drivers-xhci_hcd |  25 +
 .../devicetree/bindings/usb/mediatek,mtk-xhci.txt  |   2 +
 Documentation/devicetree/bindings/usb/usb-xhci.txt |   5 +-
 drivers/usb/host/Kconfig   |   8 +
 drivers/usb/host/Makefile  |   5 +
 drivers/usb/host/xhci-dbg.c| 261 --
 drivers/usb/host/xhci-dbgcap.c | 996 +
 drivers/usb/host/xhci-dbgcap.h | 229 +
 drivers/usb/host/xhci-dbgtty.c | 497 ++
 drivers/usb/host/xhci-hub.c|   8 +-
 drivers/usb/host/xhci-mem.c| 135 +--
 drivers/usb/host/xhci-mtk.c|   9 +
 drivers/usb/host/xhci-pci.c|   3 +
 drivers/usb/host/xhci-plat.c   |   5 +
 drivers/usb/host/xhci-ring.c   |  14 +-
 drivers/usb/host/xhci-trace.h  |  69 ++
 drivers/usb/host/xhci.c|  48 +-
 drivers/usb/host/xhci.h|  30 +-
 18 files changed, 1980 insertions(+), 369 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
 create mode 100644 drivers/usb/host/xhci-dbgcap.c
 create mode 100644 drivers/usb/host/xhci-dbgcap.h
 create mode 100644 drivers/usb/host/xhci-dbgtty.c

-- 
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: [PATCH 0/4] USB over IP Secuurity fixes

[Shuah Khan]

Hi Jakub,

On 12/08/2017 08:14 AM, Secunia Research wrote:
> Hi Shuah,
> 
> Thanks a lot for the quick fixes.

Thanks for finding them and doing all the leg work in
pin pointing the issues.

> 
> Please, use this email address: v...@secunia.com
> 
> We have assigned the following CVEs to the issues:
> CVE-2017-16911 usbip: prevent vhci_hcd driver from leaking a socket pointer
> address
> CVE-2017-16912 usbip: fix stub_rx: get_pipe() to validate endpoint number
> CVE-2017-16913 usbip: fix stub_rx: harden CMD_SUBMIT path to handle
> malicious input
> CVE-2017-16914 usbip: fix stub_send_ret_submit() vulnerability to null
> transfer_buffer
> 
> Please, let me know if we should proceed with a coordinated disclosure. I'm
> not quite sure how many distros / downstreams actually use this
> functionality.

I believe so. We have to get these into mainline and propagate them into
stables first which could take a couple of weeks.

I will defer to Greg KH on this to comment and weigh in.

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 0/4] USB over IP Secuurity fixes

[Shuah Khan]

On 12/07/2017 11:25 PM, Greg KH wrote:
> On Thu, Dec 07, 2017 at 02:16:46PM -0700, Shuah Khan wrote:
>> Jakub Jirasek from Secunia Research at Flexera reported security
>> vulnerabilities in the USB over IP driver. This patch series all
>> the 4 reported problems.
> 
> Nice!
> 
> These should also all go to the stable kernels, right?
> 
> thanks,
> 
> greg k-h
> 

These should go into stables as well. I think 3/4 needs to worked as
a special patch for 4.9 - which I can take care of for 4.9 stable.

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

Re: [PATCH v2 0/8] lsusb: Add initial support for USB Audio Class 3

[Greg KH]

On Fri, Dec 08, 2017 at 03:57:30PM +0100, Greg KH wrote:
> On Fri, Dec 08, 2017 at 02:44:12PM +, Michael Drake wrote:
> > On 08/12/17 14:27, Greg KH wrote:
> > > On Fri, Dec 08, 2017 at 10:30:43AM +, Michael Drake wrote:
> > > > On 07/12/17 20:04, Greg KH wrote:
> > 
> > > > > --- lsusb-v.orig  2017-12-07 21:01:26.153185002 +0100
> > > > > +++ lsusb-v.new   2017-12-07 21:01:32.806517978 +0100
> > > > > @@ -1110,9 +1110,9 @@
> > > > >bDescriptorType36
> > > > >bDescriptorSubtype  1 (HEADER)
> > > > >bcdADC   1.00
> > > > > -wTotalLength   0x0028
> > > > > +wTotalLength   40
> > 
> > [snip]
> > 
> > > > Anyway, since it's a 2 byte field the hex representation
> > > > is expected here, and the actual value is the same.
> > > 
> > > Yes, the value is the same, but all other wTotalLength fields exported
> > > by lsusb are in decimal.  Changing just this one feels odd to me.
> > 
> > Yes, that's a good reason for wanting to change the other ones.
> > 
> > [snip]
> > 
> > > I'll look at moving those other wFields to be also in 0x mode to
> > > match up here.
> > 
> > If you like I can work up a patch for that.  I'd just be tweaking
> > the old printfs to dump wFields with "0x%04x".
> > 
> > I'm also thinking of porting more of the existing lsusb dumping
> > to use the desc_dump() function, but that would have to be piece
> > by piece, as I get time.
> 
> I think moving more over to use desc_dump() is a better idea, let me see
> if I can do one now to see how hard/easy it is...

I've just fixed up the wTotalLength fields for now, I've run out of time
to work on this.  If you want to convert more of the descriptor fields,
that would be wonderful.

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: Lockdep is less useful than it was

[Theodore Ts'o]

On Thu, Dec 07, 2017 at 02:38:03PM -0800, Matthew Wilcox wrote:
> I think it was a mistake to force these on for everybody; they have a
> much higher false-positive rate than the rest of lockdep, so as you say
> forcing them on leads to fewer people using *any* of lockdep.
> 
> The bug you're hitting isn't Byungchul's fault; it's an annotation
> problem.  The same kind of annotation problem that we used to have with
> dozens of other places in the kernel which are now fixed.

The question is whose responsibility is it to annotate the code?  On
another thread it was suggested it was ext4's responsibility to add
annotations to avoid the false positives --- never the mind the fact
that every single file system is going to have add annotations.

Also note that the documentation for how to add annotations is
***horrible***.  It's mostly, "try to figure out how other people
added magic cargo cult code which is not well defined (look at the
definitions of lockdep_set_class, lockdep_set_class_and_name,
lockdep_set_class_and_subclass, and lockdep_set_subclass, and weep) in
other subsystems and hope and pray it works for you."

And the explanation when there are failures, either false positives,
or not, are completely opaque.  For example:

[   16.190198] ext4lazyinit/648 is trying to acquire lock:
[   16.191201]  ((gendisk_completion)1 << part_shift(NUMA_NO_NODE)){+.+.}, at: 
[<8a1ebe9d>] wait_for_completion_io+0x12/0x20

Just try to tell me that:

((gendisk_completion)1 << part_shift(NUMA_NO_NODE)){+.+.}

is human comprehensible with a straight face.  And since the messages
don't even include the subclass/class/name key annotations, as lockdep
tries to handle things that are more and more complex, I'd argue it
has already crossed the boundary where unless you are a lockdep
developer, good luck trying to understand what is going on or how to
add annotations.

So if you are adding complexity to the kernel with the argument,
"lockdep will save us", I'm with Dave --- it's just not a believable
argument.

Cheers,

- Ted
--
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 0/4] USB over IP Secuurity fixes

[Secunia Research]

Hi Shuah,

Thanks a lot for the quick fixes.

Please, use this email address: v...@secunia.com

We have assigned the following CVEs to the issues:
CVE-2017-16911 usbip: prevent vhci_hcd driver from leaking a socket pointer
address
CVE-2017-16912 usbip: fix stub_rx: get_pipe() to validate endpoint number
CVE-2017-16913 usbip: fix stub_rx: harden CMD_SUBMIT path to handle
malicious input
CVE-2017-16914 usbip: fix stub_send_ret_submit() vulnerability to null
transfer_buffer

Please, let me know if we should proceed with a coordinated disclosure. I'm
not quite sure how many distros / downstreams actually use this
functionality.

Best Regards,
Jakub

-Original Message-
From: Shuah Khan [mailto:shua...@osg.samsung.com] 
Sent: Thursday, December 7, 2017 10:17 PM
To: sh...@kernel.org; valentina.mane...@gmail.com;
gre...@linuxfoundation.org
Cc: Shuah Khan ; linux-usb@vger.kernel.org;
linux-ker...@vger.kernel.org; v...@secunia.com
Subject: [PATCH 0/4] USB over IP Secuurity fixes

Jakub Jirasek from Secunia Research at Flexera reported security
vulnerabilities in the USB over IP driver. This patch series all the 4
reported problems.

Jakub, could you please suggest an email address I can use for the
Reported-by tag?

Shuah Khan (4):
  usbip: fix stub_rx: get_pipe() to validate endpoint number
  usbip: fix stub_rx: harden CMD_SUBMIT path to handle malicious input
  usbip: prevent vhci_hcd driver from leaking a socket pointer address
  usbip: fix stub_send_ret_submit() vulnerability to null
transfer_buffer

 drivers/usb/usbip/stub_rx.c  | 51
+---
 drivers/usb/usbip/stub_tx.c  |  7 +
 drivers/usb/usbip/usbip_common.h |  1 +
 drivers/usb/usbip/vhci_sysfs.c   | 25 +++---
 tools/usb/usbip/libsrc/vhci_driver.c |  8 +++---
 5 files changed, 69 insertions(+), 23 deletions(-)

--
2.14.1




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

Re: [PATCH v2 0/8] lsusb: Add initial support for USB Audio Class 3

[Greg KH]

On Fri, Dec 08, 2017 at 02:44:12PM +, Michael Drake wrote:
> On 08/12/17 14:27, Greg KH wrote:
> > On Fri, Dec 08, 2017 at 10:30:43AM +, Michael Drake wrote:
> > > On 07/12/17 20:04, Greg KH wrote:
> 
> > > > --- lsusb-v.orig2017-12-07 21:01:26.153185002 +0100
> > > > +++ lsusb-v.new 2017-12-07 21:01:32.806517978 +0100
> > > > @@ -1110,9 +1110,9 @@
> > > >bDescriptorType36
> > > >bDescriptorSubtype  1 (HEADER)
> > > >bcdADC   1.00
> > > > -wTotalLength   0x0028
> > > > +wTotalLength   40
> 
> [snip]
> 
> > > Anyway, since it's a 2 byte field the hex representation
> > > is expected here, and the actual value is the same.
> > 
> > Yes, the value is the same, but all other wTotalLength fields exported
> > by lsusb are in decimal.  Changing just this one feels odd to me.
> 
> Yes, that's a good reason for wanting to change the other ones.
> 
> [snip]
> 
> > I'll look at moving those other wFields to be also in 0x mode to
> > match up here.
> 
> If you like I can work up a patch for that.  I'd just be tweaking
> the old printfs to dump wFields with "0x%04x".
> 
> I'm also thinking of porting more of the existing lsusb dumping
> to use the desc_dump() function, but that would have to be piece
> by piece, as I get time.

I think moving more over to use desc_dump() is a better idea, let me see
if I can do one now to see how hard/easy it is...

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: [PATCH v2 0/8] lsusb: Add initial support for USB Audio Class 3

[Michael Drake]

On 08/12/17 14:27, Greg KH wrote:

On Fri, Dec 08, 2017 at 10:30:43AM +, Michael Drake wrote:

On 07/12/17 20:04, Greg KH wrote:



--- lsusb-v.orig2017-12-07 21:01:26.153185002 +0100
+++ lsusb-v.new 2017-12-07 21:01:32.806517978 +0100
@@ -1110,9 +1110,9 @@
   bDescriptorType36
   bDescriptorSubtype  1 (HEADER)
   bcdADC   1.00
-wTotalLength   0x0028
+wTotalLength   40


[snip]


Anyway, since it's a 2 byte field the hex representation
is expected here, and the actual value is the same.


Yes, the value is the same, but all other wTotalLength fields exported
by lsusb are in decimal.  Changing just this one feels odd to me.


Yes, that's a good reason for wanting to change the other ones.

[snip]


I'll look at moving those other wFields to be also in 0x mode to
match up here.


If you like I can work up a patch for that.  I'd just be tweaking
the old printfs to dump wFields with "0x%04x".

I'm also thinking of porting more of the existing lsusb dumping
to use the desc_dump() function, but that would have to be piece
by piece, as I get time.

Cheers,

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

Re: [PATCH v2 0/8] lsusb: Add initial support for USB Audio Class 3

[Greg KH]

On Fri, Dec 08, 2017 at 10:30:43AM +, Michael Drake wrote:
> On 07/12/17 20:04, Greg KH wrote:
> > On Thu, Dec 07, 2017 at 07:24:35PM +, Michael Drake wrote:
> > > On 07/12/17 17:26, Greg KH wrote:
> > > > On Thu, Dec 07, 2017 at 05:14:10PM +, Michael Drake wrote:
> > > > > On 07/12/17 15:16, Greg KH wrote:
> > > > > > On Thu, Dec 07, 2017 at 04:01:23PM +0100, Greg KH wrote:
> > > > > > > Oops, I should have tested the code, it now crashes for me with 
> > > > > > > the
> > > > > > > following error:
> > > > > > >   Floating point exception (core dumped)
> > > > > > > 
> > > > > > > Do you see this as well?
> > > > > 
> > > > > No, I don't see that.
> > > > > 
> > > > > > And it's crashing on my USB audio device.  Here's the output of it 
> > > > > > from
> > > > > > the "old" lsusb output.
> > > > > 
> > > > > [snip]
> > > > > 
> > > > > Does it still crash with the warning fixes I posted?  If so I'll
> > > > > look in detail tomorrow.
> > > > 
> > > > I will check when I get home tonight, I don't have the USB device on me
> > > > at the moment.
> > > 
> > > Thank you.  I believe I've guessed the problem and sent a patch.
> > 
> > Ok, that now works.
> > 
> > But there is a difference in the "old" and "new" outputs, here's a diff
> > of a full -v output of my laptop and a bunch of USB devices plugged in,
> > showing the fields that now look different.
> > 
> > The big one is the change from "streaming" to "control", is that
> > correct?
> 
> Replied inline below.
> 
> > thanks,
> > 
> > greg k-h
> > 
> > 
> > --- lsusb-v.orig2017-12-07 21:01:26.153185002 +0100
> > +++ lsusb-v.new 2017-12-07 21:01:32.806517978 +0100
> > @@ -1110,9 +1110,9 @@
> >   bDescriptorType36
> >   bDescriptorSubtype  1 (HEADER)
> >   bcdADC   1.00
> > -wTotalLength   0x0028
> > +wTotalLength   40
> 
> I was a bit confused by this diff at first, because the
> "-" lines are my version and the "+" lines are the old
> version.

Oops, you are right, sorry for getting them backwards.

> Anyway, this is expected.  For NUMBER type fields, my
> code uses the heuristic that 1 byte fields are shown as
> decimal and >1 byte is shown as full hexadecimal, with
> leading zeros shown.  This is consistent with the way
> the old lsusb behaved in most cases, although it was
> slightly inconsistent about it, and this is an example
> of that inconsistency.
> 
> Anyway, since it's a 2 byte field the hex representation
> is expected here, and the actual value is the same.

Yes, the value is the same, but all other wTotalLength fields exported
by lsusb are in decimal.  Changing just this one feels odd to me.

> >   bInCollection   1
> > -baInterfaceNr(0)1
> > +baInterfaceNr( 0)   1
> 
> Just a whitespace change.  My version's not using
> a fixed width for the array index, and only uses what's
> needed for the largest index to be represented.

Fair enough, I was worried things looked worse now :)

> > AudioControl Interface Descriptor:
> >   bLength12
> >   bDescriptorType36
> > @@ -1142,10 +1142,10 @@
> >   bUnitID13
> >   bSourceID   1
> >   bControlSize1
> > -bmaControls(0)   0x01
> > +bmaControls( 0)  0x01
> > Mute Control
> > -bmaControls(1)   0x00
> > -bmaControls(2)   0x00
> > +bmaControls( 1)  0x00
> > +bmaControls( 2)  0x00
> 
> Ditto for these.
> 
> >   iFeature0
> >   Interface Descriptor:
> > bLength 9
> > @@ -1173,7 +1173,7 @@
> >   bDescriptorSubtype  1 (AS_GENERAL)
> >   bTerminalLink   1
> >   bDelay  1 frames
> > -wFormatTag 0x0001 PCM
> > +wFormatTag  1 PCM
> 
> This is the >1 byte shown as hexadecimal thing.

Ok, consistancy is good, I can see that.

> > AudioStreaming Interface Descriptor:
> >   bLength20
> >   bDescriptorType36
> > @@ -1199,14 +1199,14 @@
> >   bInterval   4
> >   bRefresh0
> >   bSynchAddress 133
> > -AudioStreaming Endpoint Descriptor:
> > +AudioControl Endpoint Descriptor:
> 
> This is from the dump_audiostreaming_endpoint() function.
> The "AudioStreaming" string is correct.  Looks like a typo
> in the old lsusb output.

Hey, bugfixes, nice!  Best reason yet to take your patches :)

I'll look at moving those other wFields to be also in 0x mode to
match up 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

Re: RFC: FT232H based FPGA configuration adapter drivers

[Anatolij Gustschin]

Hi Geert,

On Fri, 8 Dec 2017 10:27:46 +0100
Geert Uytterhoeven ge...@linux-m68k.org wrote:

[...]
>A quick Google shows the FT232H chip in MPSSE mode can also support i2c.
>I guess your design can be extended for that, too, using another VID/PID
>pair?

Yes, this should be doable.

Thanks,

Anatolij
--
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] doc: usb: chipidea: Fix typo in 'enumerate'

[Fabio Estevam]

From: Fabio Estevam 

Fix the spelling of 'enumerate' in this document.

Signed-off-by: Fabio Estevam 
---
 Documentation/usb/chipidea.txt | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/Documentation/usb/chipidea.txt b/Documentation/usb/chipidea.txt
index edf7cdf..d1eedc0 100644
--- a/Documentation/usb/chipidea.txt
+++ b/Documentation/usb/chipidea.txt
@@ -23,13 +23,13 @@ cat /sys/kernel/debug/ci_hdrc.0/registers
 2) Connect 2 boards with usb cable with one end is micro A plug, the other end
is micro B plug.
 
-   The A-device(with micro A plug inserted) should enumrate B-device.
+   The A-device(with micro A plug inserted) should enumerate B-device.
 
 3) Role switch
On B-device:
echo 1 > /sys/bus/platform/devices/ci_hdrc.0/inputs/b_bus_req
 
-   B-device should take host role and enumrate A-device.
+   B-device should take host role and enumerate A-device.
 
 4) A-device switch back to host.
On B-device:
@@ -40,13 +40,13 @@ cat /sys/kernel/debug/ci_hdrc.0/registers
side by answering the polling from B-Host, this can be done on A-device:
echo 1 > /sys/bus/platform/devices/ci_hdrc.0/inputs/a_bus_req
 
-   A-device should switch back to host and enumrate B-device.
+   A-device should switch back to host and enumerate B-device.
 
 5) Remove B-device(unplug micro B plug) and insert again in 10 seconds,
-   A-device should enumrate B-device again.
+   A-device should enumerate B-device again.
 
 6) Remove B-device(unplug micro B plug) and insert again after 10 seconds,
-   A-device should NOT enumrate B-device.
+   A-device should NOT enumerate B-device.
 
if A-device wants to use bus:
On A-device:
@@ -67,7 +67,7 @@ cat /sys/kernel/debug/ci_hdrc.0/registers
On B-device:
echo 1 > /sys/bus/platform/devices/ci_hdrc.0/inputs/b_bus_req
 
-   A-device should resume usb bus and enumrate B-device.
+   A-device should resume usb bus and enumerate B-device.
 
 1.3 Reference document
 --
-- 
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

dwc2 gadget driver does not generate DISABLE event on unplugging USB cable

[Jun Sun]

Hi, all,

I have a raspberry pi zero w board and I'm currently configuring the
device as USB gadget and connects it to PC.

I'm using configfs/functionfs to send a vendor specific interface with
single buik in and bulk out endpoint each.

When I unplug the USB cable, there is no DISABLE event generated on
ep0 until next time the cable is re-plugged in again, right before the
next ENABLE event.  Obviously this is not very useful as I really need
to know exactly when a unpllugging is happening.

You can see a debugging trace below.

After looking around, I finally got it working by the attached patch.
However, I have very little confidence in the patch itself.  Would
appreciate any feedback on the issue and the correct fix.

Cheers.

Jun




[  130.406932] ffs_epfile_io_complete()
[  130.406964] == JSUN == ffs_func_disable() called
[  130.406980] __ffs_event_add(DISABLE)
[  130.406991] adding event 3
[  130.435178] ffs_epfile_release()
[  130.435193] ffs_data_closed()
[  130.435199] ffs_data_put()
[  130.435325] ffs_epfile_release()
[  130.435333] ffs_data_closed()
[  130.435338] ffs_data_put()
[  130.435758] ffs_ep0_read()
[  130.461705] dwc2 2098.usb: new device is high-speed
[  130.554583] dwc2 2098.usb: new device is high-speed
[  130.591376] dwc2 2098.usb: new address 17
[  130.827451] dwc2 2098.usb: new device is high-speed
[  130.918627] dwc2 2098.usb: new device is high-speed
[  130.962062] dwc2 2098.usb: new address 18
[  131.152381] dwc2 2098.usb: new device is high-speed
[  131.243481] dwc2 2098.usb: new device is high-speed
[  131.285993] dwc2 2098.usb: new address 19
[  131.461014] configfs-gadget gadget: high-speed config #1: c
[  131.461057] __ffs_event_add(ENABLE)
[  131.461066] adding event 2
[  131.464892] ffs_epfile_open()
[  131.464908] ffs_data_opened()
[  131.465261] ffs_epfile_open()
[  131.465272] ffs_data_opened()
[  131.488760] ffs_epfile_read_iter()
[  131.488835] ffs_ep0_read()




[  151.214987] ffs_epfile_io_complete()
[  151.215020] == JSUN == ffs_func_disable() called
[  151.215038] __ffs_event_add(DISABLE)
[  151.215048] adding event 3
[  151.246561] ffs_epfile_release()
[  151.246577] ffs_data_closed()
[  151.246583] ffs_data_put()
[  151.247797] ffs_epfile_release()
[  151.247811] ffs_data_closed()
[  151.247817] ffs_data_put()
[  151.248227] ffs_ep0_read()
[  151.269764] dwc2 2098.usb: new device is high-speed
[  151.363232] dwc2 2098.usb: new device is high-speed
[  151.400091] dwc2 2098.usb: new address 20
[  151.638217] dwc2 2098.usb: new device is high-speed
[  151.729145] dwc2 2098.usb: new device is high-speed
[  151.773919] dwc2 2098.usb: new address 21
[  151.962998] dwc2 2098.usb: new device is high-speed
[  152.054036] dwc2 2098.usb: new device is high-speed
[  152.096922] dwc2 2098.usb: new address 22
[  152.267818] configfs-gadget gadget: high-speed config #1: c
[  152.267863] __ffs_event_add(ENABLE)
[  152.267874] adding event 2
[  152.273430] ffs_epfile_open()
[  152.273446] ffs_data_opened()
[  152.273811] ffs_epfile_open()
[  152.273822] ffs_dat[  152.296847] ffs_epfile_read_iter()
[  152.296921] ffs_ep0_read()
[  251.986224] random: crng init done


00-usb-disconnect.patch
Description: Binary data

Re: [RFC PATCH 0/2] usb: typec: alternate mode bus

[Heikki Krogerus]

On Thu, Dec 07, 2017 at 05:40:40PM +0100, Hans de Goede wrote:
> Hi,
> 
> On 07-12-17 16:00, Heikki Krogerus wrote:
> > Hi Hans,
> > 
> > On Tue, Dec 05, 2017 at 03:56:05PM +0100, Hans de Goede wrote:
> > > Hi,
> > > 
> > > On 01-12-17 09:38, Heikki Krogerus wrote:
> > > > Hi,
> > > > 
> > > > Thanks for taking a look at this..
> > > > 
> > > > On Sun, Nov 26, 2017 at 12:23:31PM +0100, Hans de Goede wrote:
> > > > > Hi Heiko,
> > > > > 
> > > > > On 28-09-17 13:35, Heikki Krogerus wrote:
> > > > > > Hi guys,
> > > > > > 
> > > > > > The bus allows SVID specific communication with the partners to be
> > > > > > handled in separate drivers for each alternate mode.
> > > > > > 
> > > > > > Alternate mode handling happens with two separate logical devices:
> > > > > > 1. Partner alternate mode devices which represent the alternate 
> > > > > > modes
> > > > > >   on the partner. The driver for them will handle the alternate 
> > > > > > mode
> > > > > >   specific communication with the partner using VDMs.
> > > > > > 2. Port alternate mode devices which represent connections from the
> > > > > >   USB Type-C port to devices on the platform.
> > > > > > 
> > > > > > The drivers will be bind to the partner alternate modes. The 
> > > > > > alternate
> > > > > > mode drivers will need to deliver the result of the negotiated pin
> > > > > > configurations to the rest of the platform (towards the port 
> > > > > > alternate
> > > > > > mode devices). This series includes API for that, however, not the
> > > > > > final implementation yet.
> > > > > > 
> > > > > > The connections to the other devices on the platform the ports have
> > > > > > can be described by using the remote endpoint concept [1][2] on ACPI
> > > > > > and DT platforms, but I have no solution for the "platform data" 
> > > > > > case
> > > > > > where we have neither DT nor ACPI to describe the connections for 
> > > > > > us.
> > > > > 
> > > > > Sorry about the slow reply, I've been a bit swamped with other stuff,
> > > > > but now I would like to get back to this.
> > > > > 
> > > > > I've been trying to wrap my head around what you're proposing here and
> > > > > I see how this can help with implementing display-port alternate mode
> > > > > support, but I don't see how it is going to help with regular 
> > > > > superspeed
> > > > > USB support / the mux problem.
> > > > > 
> > > > > The problems I see / questions I have are:
> > > > > 
> > > > > 1) This seems to be driven by having a bus using svid-s as match 
> > > > > functions,
> > > > > but the standard USB function does not have any svid, or at least 
> > > > > currently
> > > > > does not show as such under e.g. /sys/class/typec/port0/port0-partner
> > > > 
> > > > USB is the "normal" mode, not alt-mode. We don't need any specific
> > > > driver for the USB mode. In alternate modes, we have to communicate
> > > > with the partner using SVID specific messages, and that is what we
> > > > need the drivers for.
> > > 
> > > Ack, but we do still need to control the mux in USB-mode I was under
> > > the impression that the alt-mode drivers would be responsible for
> > > switching the mux to the right component in the graph, if (which
> > > may not be true) the alt-mode drivers indeed will be the ones controlling
> > > the mux, then we need a dummy alt-mode for USB mode and a dummy
> > > alt-mode driver for that.
> > 
> > The Type-C/PD PHY/controller drivers (the port drivers) will be in
> > control of the dual-role (USB) mux, not an alternate mode driver. The
> > port driver will know if we need to be UFP or DFP in any case, so it
> > just needs to pass that detail to the mux driver. There is no need for
> > an extra driver in the middle just for that.
> 
> Right, but that only deals with the "USB MUX" i your ascii-art
> diagram not with the one you labelled "MUX" and the one you labelled
> "MUX" also needs to switch between "tristate(floating) / normal SuperSpeed 
> USB /
> upside-down SuperSpeed USB.

Well, I would say the cable orientation is actually a third mux,
missing from my ascii-art.

> > But logically we will have two muxes to deal with - one for the USB
> > handled by the port drivers, and one for the type-c handled by the
> > alt-mode drivers - even in case the same physical mux component
> > handles both cases on the platform.
> 
> Right, but as mentioned above even for just the non alt-mode Superspeed
> USB stuff to work we also need to control the Type-C mux/switch.

Yes, you are correct.

> > > > > 2) The alt-mode drivers you are suggesting seem to be about 2 things:
> > > > > a) Alt-mode specific PD communication
> > > > > b) Telling other components about pin-configs, e.g. telling the i915 
> > > > > driver how
> > > > > much display port lanes are configured
> > > > > 
> > > > > What this seems to miss a mechanism to control the mux between the 
> > > > > "superspeed"
> > > > > data-pairs on the port and the dp-port pins on the SoC / the 
> > > > > 

Re: USB: hub: Checking communication difficulties

[SF Markus Elfring]

> Greg maintains USB and he's has blocked Markus,

How do you think about to reconsider this blockage?


> because he never listens to feedback

I am listening …


> but instead just repsonds that he has a different opinion.

I choose such a reaction in some cases. My responses can vary.

It seems that I show change possibilities from which contributors
can get grumpy (over time).

Regards,
Markus
--
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: hub: Delete an error message for a failed memory allocation in usb_hub_clear_tt_buffer()

[Dan Carpenter]

On Wed, Dec 06, 2017 at 09:20:05PM +0100, Geert Uytterhoeven wrote:
> Hi Markus,
> 
> On Wed, Dec 6, 2017 at 6:51 PM, SF Markus Elfring
>  wrote:
> >> The system will come to a grinding halt anyway if it can't allocate 24 or 
> >> 40 bytes.
> >
> > Maybe.
> 
> Since you've been sending zillions of patches for this, perhaps the time
> is ripe to actually try to trigger this, and see what happens?
> 
> >> Which is BTW more or less the amount of memory saved by killing
> >> the useless (error) message.
> >
> > Would you dare to resend this update suggestion after such a view?
> 
> Of course. That was implied.
> 

No.  Greg maintains USB and he's has blocked Markus, because he never
listens to feedback but instead just repsonds that he has a different
opinion.

regards,
dan carpenter

--
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-core: Fix potential null pointer dereference in xhci-debugfs.c

[Alexander Kappner]

Hi,

> I think we need to dig a bit deeper. It's good to check if spriv is 
valid

but there are probably other reasons than kzalloc failing.

I agree -- this small allocation is  unlikely to fail in practice. Also, 
while my patch prevents the kernel oops, it also prevents the debugfs 
entries from being created.


I've been debugging this more trying to come up with a better solution, 
but I might need some guidance as I'm not too familiar with the USB 
subsystem. The immediate cause of the crash was usbmuxd sending a 
USBDEVFS_SETCONFIGURATION ioctl to a device, which _only if it fails_ 
calls usb_hcd_alloc_bandwidth to try and reset the device, which in turn 
calls xhci_debugfs_create_endpoint. The ioctl handler acquires a 
device-specific lock via usb_lock_device.


When the system resumed from hibernate, xhci_resume was called. This in 
turn called xhci_mem_cleanup to deallocate the device structures, which 
include setting the debugfs_private pointer to NULL  (via 
xhci_free_virt_devices_depth_first). It thus seems likely that the ioctl 
is somehow racing with the hibernate. The call to xhci_resume is 
protected by a host-controller specific lock (xhci->lock) but it doesn't 
attempt to take the usb_lock_device device-specific lock.


Now my suspicion is that xhci_resume freed and zeroed the device 
structures while racing with the ioctl handler. I can't seem to find any 
exclusion mechanism that would prevent xhci_resume from racing with the 
USBDEVFS_SETCONFIGURATION ioctl (or any other ioctl, for that matter). 
Am I missing something? If not, is there any reason why an ioctl might 
need to execute in parallel with the xhci_resume? If not, can we just do 
a busy wait in xhci_resume until all pending ioctls have returned?


> (Added this as reply subject, please remember to send mail with 
proper subject)

My apologies, thanks for fixing.


On 12/07/2017 04:47 AM, Mathias Nyman wrote:

On 07.12.2017 11:26, Alexander Kappner wrote:

Date: Wed, 6 Dec 2017 15:28:37 -0800
Subject: [PATCH] usb-core: Fix potential null pointer dereference in 
xhci-debugfs.c


(Added this as reply subject, please remember to send mail with proper 
subject)




My kernel crashed just after resuming from hibernate and starting 
usbmuxd

(a user-space daemon for iOS device pairing) with several USB devices
connected (dmesg attached).

Backtrace leads to:

0x8170465d is in xhci_debugfs_create_endpoint
(drivers/usb/host/xhci-debugfs.c:381).
376   int ep_index)
377 {
378 struct xhci_ep_priv *epriv;
379 struct xhci_slot_priv   *spriv = dev->debugfs_private;
380
381 if (spriv->eps[ep_index])
382 return;
383
384 epriv = kzalloc(sizeof(*epriv), GFP_KERNEL);
385 if (!epriv)

The read violation happens at address 0x40 and sizeof(struct
xhci_ep_priv)=0x40, so it seems ep_index is 1 and spriv is NULL here.



Thanks for reporting and debugging this, much appreciated.


spriv gets allocated in xhci_debugfs_create_slot:

...
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
if (!priv)
 return;
...

There's no separate error path if this allocation fails, so we might be
left with NULL in priv. Subsequent users of priv thus need to check for
this NULL - so this is what the patch does.



I think we need to dig a bit deeper. It's good to check if spriv is valid
but there are probably other reasons than kzalloc failing.

Resume from hibernation will reset the xhci controller, reinitializing 
a lot.

It could be related.

-Mathias

(keeping rest of message as reference)


There might be other ways of triggering this null pointer dereference,
including when xhci_resume frees the device structures (e.g. after
returning from a hibernate), but I wasn't able to find or reproduce it.

[63953.758083] BUG: unable to handle kernel NULL pointer dereference at
0040
[63953.758090] IP: xhci_debugfs_create_endpoint+0x1d/0xa0
[63953.758091] PGD bb911d067 P4D bb911d067 PUD 10500ff067 PMD 0
[63953.758093] Oops:  [#1] PREEMPT SMP
[63953.758094] Modules linked in: ipheth tun nvidia_modeset(PO) iwlmvm
mac80211 iwlwifi nvidia(PO) btusb btrtl btbcm btintel bluetooth cfg80211
qmi_wwan ecdh_generic thinkpad_acpi rfkill
[63953.758103] CPU: 4 PID: 27091 Comm: usbmuxd Tainted: P   O
4.14.0.1-12769-g1deab8c #1
[63953.758104] Hardware name: LENOVO 20ENCTO1WW/20ENCTO1WW, BIOS 
N1EET62W

(1.35 ) 11/10/2016
[63953.758105] task: 8810527ba0c0 task.stack: c9000a8ec000
[63953.758107] RIP: 0010:xhci_debugfs_create_endpoint+0x1d/0xa0
[63953.758108] RSP: 0018:c9000a8efc80 EFLAGS: 00010206
[63953.758109] RAX:  RBX: 88105a71c000 RCX:
0003
[63953.758110] RDX: 0003 RSI: 880c0b57e000 RDI:
88105a71c238
[63953.758110] RBP: 0003 R08: 881063800600 R09:
0003
[63953.758111] R10: 88105a71c238 R11: 0001 R12:
0011

Re: [PATCH v2] usb: host: Implement workaround for Erratum A-009611

[Felipe Balbi]

Hi,

Greg Kroah-Hartman  writes:
> On Fri, Dec 08, 2017 at 05:49:41PM +0800, yinbo@nxp.com wrote:
>> From: "yinbo.zhu" 
>> 
>> Description: This is a occasional problem where the software
>
> No need for a "Description:" word.  That's just assumed here, right?
>
>> issues an End Transfer command while a USB transfer is in progress,
>> resulting in the TxFIFO  being flushed when the lower layer is waiting
>> for data,causing the super speed (SS) transmit to get blocked.
>> If the End Transfer command is issued on an IN endpoint to
>> flush out the pending transfers when the same IN endpoint
>> is doing transfers on the USB, then depending upon the timing
>> of the End Transfer (and the resulting internal FIFO flush),the
>> lower layer (U3PTL/U3MAC) could get stuck waiting for data
>> indefinitely. This blocks the transmission path on the SS, and no
>> DP/ACK/ERDY/DEVNOTIF packets can be sent from the device.
>> Impact: If this issue happens and the transmission gets blocked,
>> then the USB host aborts and resets/re-enumerates the device.
>> This unblocks the transmitt engine and the device functions normally.
>> 
>> Workaround: Software must wait for all existing TRBs to complete before
>> issuing End transfer command.
>> 
>> Configs Affected:
>> LS1088-48A-R1.0, LS2081A-R1.1, LS2088-48A-R1.0, LS2088-48A-R1.1,
>> LX2160-2120-2080A-R1.
>
> What are these Configs?  That doesn't seem to match up with anything
> that is in the kernel tree that I can see.
>
>> 
>> Signed-off-by: yinbo.zhu 
>> ---
>>  drivers/usb/dwc3/core.c  |  3 +++
>>  drivers/usb/dwc3/core.h  |  3 +++
>>  drivers/usb/dwc3/host.c  |  3 +++
>>  drivers/usb/host/xhci-plat.c |  4 
>>  drivers/usb/host/xhci.c  | 24 ++--
>>  drivers/usb/host/xhci.h  |  1 +
>>  6 files changed, 32 insertions(+), 6 deletions(-)
>> 
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 5cb3f6795b0b..071e7cea8cbb 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -1106,6 +1106,9 @@ static void dwc3_get_properties(struct dwc3 *dwc)
>>  
>>  dwc->quirk_reverse_in_out = device_property_read_bool(dev,
>>  "snps,quirk_reverse_in_out");

This was generated on vendor tree. This quirk doesn't exist in
dwc3. Also, update your tree and review MAINTAINERS file. It has been
almost 2 years since I left TI :-)

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v2 0/8] lsusb: Add initial support for USB Audio Class 3

[Michael Drake]

On 07/12/17 20:04, Greg KH wrote:

On Thu, Dec 07, 2017 at 07:24:35PM +, Michael Drake wrote:

On 07/12/17 17:26, Greg KH wrote:

On Thu, Dec 07, 2017 at 05:14:10PM +, Michael Drake wrote:

On 07/12/17 15:16, Greg KH wrote:

On Thu, Dec 07, 2017 at 04:01:23PM +0100, Greg KH wrote:

Oops, I should have tested the code, it now crashes for me with the
following error:
Floating point exception (core dumped)

Do you see this as well?


No, I don't see that.


And it's crashing on my USB audio device.  Here's the output of it from
the "old" lsusb output.


[snip]

Does it still crash with the warning fixes I posted?  If so I'll
look in detail tomorrow.


I will check when I get home tonight, I don't have the USB device on me
at the moment.


Thank you.  I believe I've guessed the problem and sent a patch.


Ok, that now works.

But there is a difference in the "old" and "new" outputs, here's a diff
of a full -v output of my laptop and a bunch of USB devices plugged in,
showing the fields that now look different.

The big one is the change from "streaming" to "control", is that
correct?


Replied inline below.


thanks,

greg k-h


--- lsusb-v.orig2017-12-07 21:01:26.153185002 +0100
+++ lsusb-v.new 2017-12-07 21:01:32.806517978 +0100
@@ -1110,9 +1110,9 @@
  bDescriptorType36
  bDescriptorSubtype  1 (HEADER)
  bcdADC   1.00
-wTotalLength   0x0028
+wTotalLength   40


I was a bit confused by this diff at first, because the
"-" lines are my version and the "+" lines are the old
version.

Anyway, this is expected.  For NUMBER type fields, my
code uses the heuristic that 1 byte fields are shown as
decimal and >1 byte is shown as full hexadecimal, with
leading zeros shown.  This is consistent with the way
the old lsusb behaved in most cases, although it was
slightly inconsistent about it, and this is an example
of that inconsistency.

Anyway, since it's a 2 byte field the hex representation
is expected here, and the actual value is the same.



  bInCollection   1
-baInterfaceNr(0)1
+baInterfaceNr( 0)   1


Just a whitespace change.  My version's not using
a fixed width for the array index, and only uses what's
needed for the largest index to be represented.


AudioControl Interface Descriptor:
  bLength12
  bDescriptorType36
@@ -1142,10 +1142,10 @@
  bUnitID13
  bSourceID   1
  bControlSize1
-bmaControls(0)   0x01
+bmaControls( 0)  0x01
Mute Control
-bmaControls(1)   0x00
-bmaControls(2)   0x00
+bmaControls( 1)  0x00
+bmaControls( 2)  0x00


Ditto for these.


  iFeature0
  Interface Descriptor:
bLength 9
@@ -1173,7 +1173,7 @@
  bDescriptorSubtype  1 (AS_GENERAL)
  bTerminalLink   1
  bDelay  1 frames
-wFormatTag 0x0001 PCM
+wFormatTag  1 PCM


This is the >1 byte shown as hexadecimal thing.


AudioStreaming Interface Descriptor:
  bLength20
  bDescriptorType36
@@ -1199,14 +1199,14 @@
  bInterval   4
  bRefresh0
  bSynchAddress 133
-AudioStreaming Endpoint Descriptor:
+AudioControl Endpoint Descriptor:


This is from the dump_audiostreaming_endpoint() function.
The "AudioStreaming" string is correct.  Looks like a typo
in the old lsusb output.


bLength 7
bDescriptorType37
bDescriptorSubtype  1 (EP_GENERAL)
bmAttributes 0x01
  Sampling Frequency
bLockDelayUnits 0 Undefined
-  wLockDelay 0x
+  wLockDelay  0 Undefined


This and the remaining entries are similar to the above ones.


Endpoint Descriptor:
  bLength 9
  bDescriptorType 5
@@ -1235,7 +1235,7 @@
  bDescriptorSubtype  1 (AS_GENERAL)
  bTerminalLink   1
  bDelay  1 frames
-wFormatTag 0x0001 PCM
+wFormatTag  1 PCM
AudioStreaming Interface Descriptor:
  bLength20
  bDescriptorType36
@@ -1261,14 +1261,14 @@
  bInterval   4
  bRefresh0
  bSynchAddress 133
-AudioStreaming Endpoint Descriptor:
+AudioControl Endpoint Descriptor:
bLength 7
bDescriptorType37
bDescriptorSubtype  1 (EP_GENERAL)
bmAttributes 0x01
  Sampling Frequency
bLockDelayUnits 0 

Re: [PATCH v1 1/1] lsusb: Fix array entry count for variable sized entries.

[Greg KH]

On Fri, Dec 08, 2017 at 10:10:56AM +, Michael Drake wrote:
> On 07/12/17 20:02, Greg KH wrote:
> > On Thu, Dec 07, 2017 at 07:18:39PM +, Michael Drake wrote:
> > > This fixes a divide by zero which happened when an array,
> > > without an explicit entry count (ultimately calculated from
> > > the value in the descriptor data's bLength field) was used
> > > on field with a variable size.
> > > 
> > > The solultion is to use the get_entry_size() function on
> > > the array entry, which can get the entry size from a
> > > referenced field.
> > > 
> > > Signed-off-by: Michael Drake 
> > > ---
> > >   desc-dump.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > Nice, that fixed the problem, thanks.
> 
> Great, thanks Greg!
> 
> > Now applied and pushed out.
> 
> I can't see this commit yet on
> 
>   https://github.com/gregkh/usbutils/commits/master

Oops, forgot to sync my laptop, it's there now.

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: [PATCH v2] usb: host: Implement workaround for Erratum A-009611

[Greg Kroah-Hartman]

On Fri, Dec 08, 2017 at 05:49:41PM +0800, yinbo@nxp.com wrote:
> From: "yinbo.zhu" 
> 
> Description: This is a occasional problem where the software

No need for a "Description:" word.  That's just assumed here, right?

> issues an End Transfer command while a USB transfer is in progress,
> resulting in the TxFIFO  being flushed when the lower layer is waiting
> for data,causing the super speed (SS) transmit to get blocked.
> If the End Transfer command is issued on an IN endpoint to
> flush out the pending transfers when the same IN endpoint
> is doing transfers on the USB, then depending upon the timing
> of the End Transfer (and the resulting internal FIFO flush),the
> lower layer (U3PTL/U3MAC) could get stuck waiting for data
> indefinitely. This blocks the transmission path on the SS, and no
> DP/ACK/ERDY/DEVNOTIF packets can be sent from the device.
> Impact: If this issue happens and the transmission gets blocked,
> then the USB host aborts and resets/re-enumerates the device.
> This unblocks the transmitt engine and the device functions normally.
> 
> Workaround: Software must wait for all existing TRBs to complete before
> issuing End transfer command.
> 
> Configs Affected:
> LS1088-48A-R1.0, LS2081A-R1.1, LS2088-48A-R1.0, LS2088-48A-R1.1,
> LX2160-2120-2080A-R1.

What are these Configs?  That doesn't seem to match up with anything
that is in the kernel tree that I can see.

> 
> Signed-off-by: yinbo.zhu 
> ---
>  drivers/usb/dwc3/core.c  |  3 +++
>  drivers/usb/dwc3/core.h  |  3 +++
>  drivers/usb/dwc3/host.c  |  3 +++
>  drivers/usb/host/xhci-plat.c |  4 
>  drivers/usb/host/xhci.c  | 24 ++--
>  drivers/usb/host/xhci.h  |  1 +
>  6 files changed, 32 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 5cb3f6795b0b..071e7cea8cbb 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1106,6 +1106,9 @@ static void dwc3_get_properties(struct dwc3 *dwc)
>  
>   dwc->quirk_reverse_in_out = device_property_read_bool(dev,
>   "snps,quirk_reverse_in_out");
> + dwc->quirk_stop_transfer_in_block = device_property_read_bool(dev,
> + "snps,quirk_stop_transfer_in_block");

Have you documented this new DT value somewhere?

> +
>   dwc->needs_fifo_resize = of_property_read_bool(node, "tx-fifo-resize");
>  
>   dwc->configure_gfladj =
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 6c530cbedf49..b2425799ecb6 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -900,6 +900,8 @@ struct dwc3_scratchpad_array {
>   *   3   - Reserved
>   * @disable_devinit_u1u2_quirk: disable device-initiated U1/U2 request.
>   * @quirk_reverse_in_out: prevent tx fifo reverse the data direction.
> + * @quirk_stop_transfer_in_block: prevent block transmission from being
> + *interrupted.
>   * @imod_interval: set the interrupt moderation interval in 250ns
>   * increments or 0 to disable.
>   */
> @@ -1063,6 +1065,7 @@ struct dwc3 {
>   unsignedtx_de_emphasis:2;
>   unsigneddisable_devinit_u1u2_quirk:1;
>   unsignedquirk_reverse_in_out:1;
> + unsignedquirk_stop_transfer_in_block:1;
>  
>   u16 imod_interval;
>  };
> diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
> index 2cd48633d3fa..a9ccbf1b9871 100644
> --- a/drivers/usb/dwc3/host.c
> +++ b/drivers/usb/dwc3/host.c
> @@ -110,6 +110,9 @@ int dwc3_host_init(struct dwc3 *dwc)
>   if (dwc->quirk_reverse_in_out)
>   props[prop_idx++].name = "quirk-reverse-in-out";
>  
> + if (dwc->quirk_stop_transfer_in_block)
> + props[prop_idx++].name = "quirk-stop-transfer-in-block";
> +
>   if (dwc->usb3_lpm_capable)
>   props[prop_idx++].name = "usb3-lpm-capable";
>  
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index d1c1e882e6d7..5721d4ece625 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -272,6 +272,10 @@ static int xhci_plat_probe(struct platform_device *pdev)
>   if (device_property_read_bool(>dev, "quirk-reverse-in-out"))
>   xhci->quirks |= XHCI_REVERSE_IN_OUT;
>  
> + if (device_property_read_bool(>dev,
> + "quirk-stop-transfer-in-block"))
> + xhci->quirks |= XHCI_STOP_TRANSFER_IN_BLOCK;
> +
>   if (device_property_read_bool(>dev, "quirk-broken-port-ped"))
>   xhci->quirks |= XHCI_BROKEN_PORT_PED;
>  
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 21dd1d98508f..925c8d171c0b 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -1515,13 +1515,25 @@ static int xhci_urb_dequeue(struct usb_hcd *hcd, 
> struct 

Re: [PATCH v2] usb: host: Implement workaround for Erratum A-007463

[Greg Kroah-Hartman]

On Fri, Dec 08, 2017 at 05:49:40PM +0800, yinbo@nxp.com wrote:
> From: "yinbo.zhu" 

I need a "real name" here, I doubt you sign documents as:
"yinbo.zhu"
right?  :)

Also, you sent 3 patches, yet no way to know what order to apply them
in.  Please fix that up by sending a patch series, properly numbered.

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: [PATCH v1 1/1] lsusb: Fix array entry count for variable sized entries.

[Michael Drake]

On 07/12/17 20:02, Greg KH wrote:

On Thu, Dec 07, 2017 at 07:18:39PM +, Michael Drake wrote:

This fixes a divide by zero which happened when an array,
without an explicit entry count (ultimately calculated from
the value in the descriptor data's bLength field) was used
on field with a variable size.

The solultion is to use the get_entry_size() function on
the array entry, which can get the entry size from a
referenced field.

Signed-off-by: Michael Drake 
---
  desc-dump.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Nice, that fixed the problem, thanks.


Great, thanks Greg!


Now applied and pushed out.


I can't see this commit yet on

  https://github.com/gregkh/usbutils/commits/master

Cheers,

--
Michael Drake  http://www.codethink.co.uk/
--
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 72/73] xfs: Convert mru cache to XArray

[Byungchul Park]

On 12/8/2017 4:25 PM, Dave Chinner wrote:

On Fri, Dec 08, 2017 at 01:45:52PM +0900, Byungchul Park wrote:

On Fri, Dec 08, 2017 at 09:22:16AM +1100, Dave Chinner wrote:

On Thu, Dec 07, 2017 at 11:06:34AM -0500, Theodore Ts'o wrote:

On Wed, Dec 06, 2017 at 06:06:48AM -0800, Matthew Wilcox wrote:

Unfortunately for you, I don't find arguments along the lines of
"lockdep will save us" at all convincing.  lockdep already throws
too many false positives to be useful as a tool that reliably and
accurately points out rare, exciting, complex, intricate locking
problems.


But it does reliably and accurately point out "dude, you forgot to take
the lock".  It's caught a number of real problems in my own testing that
you never got to see.


The problem is that if it has too many false positives --- and it's
gotten *way* worse with the completion callback "feature", people will
just stop using Lockdep as being too annyoing and a waste of developer
time when trying to figure what is a legitimate locking bug versus
lockdep getting confused.

I can't even disable the new Lockdep feature which is throwing
lots of new false positives --- it's just all or nothing.

Dave has just said he's already stopped using Lockdep, as a result.


This is compeltely OT, but FYI I stopped using lockdep a long time
ago.  We've spend orders of magnitude more time and effort to shut
up lockdep false positives in the XFS code than we ever have on
locking problems that lockdep has uncovered. And still lockdep
throws too many false positives on XFS workloads to be useful to me.

But it's more than that: I understand just how much lockdep *doesn't
check* and that means *I know I can't rely on lockdep* for potential
deadlock detection. e.g.  it doesn't cover semaphores, which means


Hello,

I'm careful in saying the following since you seem to feel not good at
crossrelease and even lockdep. Now that cross-release has been
introduced, semaphores can be covered as you might know. Actually, all
general waiters can.


And all it will do is create a whole bunch more work for us XFS guys
to shut up all the the false positive crap that falls out from it
because the locking model we have is far more complex than any of
the lockdep developers thought was necessary to support, just like
happened with the XFS inode annotations all those years ago.

e.g. nobody has ever bothered to ask us what is needed to describe
XFS's semaphore locking model.  If you did that, you'd know that we
nest *thousands* of locked semaphores in compeltely random lock
order during metadata buffer writeback. And that this lock order
does not reflect the actual locking order rules we have for locking
buffers during transactions.

Oh, and you'd also know that a semaphore's lock order and context
can change multiple times during the life time of the buffer.  Say
we free a block and the reallocate it as something else before it is
reclaimed - that buffer now might have a different lock order. Or
maybe we promote a buffer to be a root btree block as a result of a
join - it's now the first buffer in a lock run, rather than a child.
Or we split a tree, and the root is now a node and so no longer is
the first buffer in a lock run. Or that we walk sideways along the
leaf nodes siblings during searches.  IOWs, there is no well defined
static lock ordering at all for buffers - and therefore semaphores -
in XFS at all.

And knowing that, you wouldn't simply mention that lockdep can
support semaphores now as though that is necessary to "make it work"
for XFS.  It's going to be much simpler for us to just turn off
lockdep and ignore whatever crap it sends our way than it is to
spend unplanned weeks of our time to try to make lockdep sorta work
again. Sure, we might get there in the end, but it's likely to take
months, if not years like it did with the XFS inode annotations.


it has zero coverage of the entire XFS metadata buffer subsystem and
the complex locking orders we have for metadata updates.

Put simply: lockdep doesn't provide me with any benefit, so I don't
use it...


Sad..


I don't think you understand. I'll try to explain.

The lockdep infrastructure by itself doesn't make lockdep a useful
tool - it mostly generates false positives because it has no
concept of locking models that don't match it's internal tracking
assumptions and/or limitations.

That means if we can't suppress the false positives, then lockdep is
going to be too noisy to find real problems.  It's taken the XFS
developers months of work over the past 7-8 years to suppress all
the *common* false positives that lockdep throws on XFS. And despite
all that work, there's still too many false positives occuring
because we can't easily suppress them with annotations. IOWs, the
signal to noise ratio is still too low for lockdep to find real
problems.

That's why lockdep isn't useful to me - the noise floor is too high,
and the effort to lower the noise floor further is too great.

This is important, because 

Re: RFC: FT232H based FPGA configuration adapter drivers

[Geert Uytterhoeven]

Hi Anatolij,

On Thu, Dec 7, 2017 at 3:31 PM, Anatolij Gustschin  wrote:
> I have to rework drivers for custom FT232H based FPGA configuration
> adapters to make them suitable for including in mainline kernel. These
> adapters should be usable via low-level drivers for FPGA Manager
> framework. Two required low-level FPGA Manager drivers (for PS-SPI and
> CvP configurations) are already in mainline. The missing parts are the
> MPSSE SPI master driver (for attaching altera-ps-spi SPI devices) and
> the FTDI FIFO FPP interface driver for FPGA Manager. I'm CCing the
> relevant subsystem mailing lists and would like to get feedback to the
> draft below before rewriting the missing driver parts.
>
> A few words about our FPGA and FPGA configuration adapter hardware. We
> have FPGA PCIe boards with two different FT232H based configuration
> adapters. The first adapter type utilizes FT232H chip in MPSSE mode to
> connect ADBUS SPI/GPIO pins to Stratix-V PS-SPI interface. Another
> adapter type connects FT232H ADBUS (in FT245 FIFO mode) and two ACBUS
> GPIOs to a CPLD, the CPLD is connected to the Arria-10 FPP interface.
> Both FPGA boards are connected to the host via PCIe and USB (FT232H).

[...]

> Instead of MFD part as in previous version I intend to add an USB misc
> driver for our FPGA configuration adapters under drivers/usb/misc.
> When probing for VID/PID assigned to FIFO-FPP adapter type, this
> driver will register CBUS GPIO controller, GPIO lookup tables for
> FIFO FPP device and will create a platform device for attaching the
> low-level FPGA manager driver for FIFO FPP interface. The attached
> FPGA manager driver will be similar to the ftdi-fifo-fpp driver and
> will reside in drivers/fpga. When probing for VID/PID assigned to
> MPSSE SPI adapter type, the USB misc driver will register MPSSE GPIO
> controller, GPIO lookup tables for altera-ps-spi control/status GPIOs
> and will create platform device for attaching MPSSE SPI master
> controller driver. The SPI master controller platform driver will
> register MPSSE SPI bus with SPI slave device from spi_board_info
> struct in its platform data (in our case altera-ps-spi SPI slave
> device for attaching altera-ps-spi driver). The intended location
> of this custom SPI master controller driver is drivers/spi.

A quick Google shows the FT232H chip in MPSSE mode can also support i2c.
I guess your design can be extended for that, too, using another VID/PID pair?

Thanks!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
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] usb: dwc2: Change TxFIFO and RxFIFO flushing flow

[Minas Harutyunyan]

Before flushing fifos required to check AHB master state and
flush when AHB master is in IDLE state.

Signed-off-by: Minas Harutyunyan 
---
 drivers/usb/dwc2/core.c | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index dbca3b8890da..4d2a8c452e6b 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -670,10 +670,23 @@ void dwc2_flush_tx_fifo(struct dwc2_hsotg *hsotg, const 
int num)
 
dev_vdbg(hsotg->dev, "Flush Tx FIFO %d\n", num);
 
+   /* Wait for AHB master IDLE state */
+   do {
+   greset = dwc2_readl(hsotg->regs + GRSTCTL);
+   if (++count > 1) {
+   dev_warn(hsotg->dev,
+"%s() HANG! AHB Idle GRSTCTL=%0x\n",
+__func__, greset);
+   return;
+   }
+   udelay(1);
+   } while (!(greset & GRSTCTL_AHBIDLE));
+
greset = GRSTCTL_TXFFLSH;
greset |= num << GRSTCTL_TXFNUM_SHIFT & GRSTCTL_TXFNUM_MASK;
dwc2_writel(greset, hsotg->regs + GRSTCTL);
 
+   count = 0;
do {
greset = dwc2_readl(hsotg->regs + GRSTCTL);
if (++count > 1) {
@@ -702,9 +715,23 @@ void dwc2_flush_rx_fifo(struct dwc2_hsotg *hsotg)
 
dev_vdbg(hsotg->dev, "%s()\n", __func__);
 
+   /* Wait for AHB master IDLE state */
+   do {
+   greset = dwc2_readl(hsotg->regs + GRSTCTL);
+   if (++count > 1) {
+   dev_warn(hsotg->dev,
+"%s() HANG! AHB Idle GRSTCTL=%0x\n",
+__func__, greset);
+   return;
+   }
+   udelay(1);
+   } while (!(greset & GRSTCTL_AHBIDLE));
+
greset = GRSTCTL_RXFFLSH;
dwc2_writel(greset, hsotg->regs + GRSTCTL);
 
+   /* Wait for RxFIFO flush done */
+   count = 0;
do {
greset = dwc2_readl(hsotg->regs + GRSTCTL);
if (++count > 1) {
-- 
2.11.0

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