Re: [PATCH v7 1/5] x86: add simple udelay calibration

[Sergei Shtylyov]

Hello!

On 2/14/2017 5:27 AM, Lu Baolu wrote:


Add a simple udelay calibration in x86 architecture-specific
boot-time initializations. This will get a workable estimate
for loops_per_jiffy. Hence, udelay() could be used after this
initialization.

Cc: Ingo Molnar 
Cc: x...@kernel.org
Signed-off-by: Lu Baolu 
---
 arch/x86/kernel/setup.c | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 4cfba94..aab7faa 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -835,6 +835,26 @@ dump_kernel_offset(struct notifier_block *self, unsigned 
long v, void *p)
return 0;
 }

+static void __init simple_udelay_calibration(void)
+{
+   unsigned int tsc_khz, cpu_khz;
+   unsigned long lpj;
+
+   if (!boot_cpu_has(X86_FEATURE_TSC))
+   return;
+
+   cpu_khz = x86_platform.calibrate_cpu();
+   tsc_khz = x86_platform.calibrate_tsc();
+
+   tsc_khz = tsc_khz ? : cpu_khz;


   Why not:

if (!tsc_khz)
tsc_khz = cpu_khz;

   It's more clear IMHO.


+   if (!tsc_khz)
+   return;
+
+   lpj = tsc_khz * 1000;
+   do_div(lpj, HZ);
+   loops_per_jiffy = lpj;
+}
+
 /*
  * Determine if we were loaded by an EFI loader.  If so, then we have also been
  * passed the efi memmap, systab, etc., so we should use these data structures

[...]

MBR, Sergei

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

Re: [PATCH v13 00/12] power: add power sequence library

[Roger Quadros]

Peter,

On 11/02/17 03:27, Peter Chen wrote:
> Hi all,
> 
> This is a follow-up for my last power sequence framework patch set [1].
> According to Rob Herring and Ulf Hansson's comments[2]. The kinds of
> power sequence instances will be added at postcore_initcall, the match
> criteria is compatible string first, if the compatible string is not
> matched between dts and library, it will try to use generic power sequence.
>
> The host driver just needs to call of_pwrseq_on/of_pwrseq_off
> if only one power sequence instance is needed, for more power sequences
> are used, using of_pwrseq_on_list/of_pwrseq_off_list instead (eg, USB hub 
> driver).
> 
> In future, if there are special power sequence requirements, the special
> power sequence library can be created.
> 
> This patch set is tested on i.mx6 sabresx evk using a dts change, I use
> two hot-plug devices to simulate this use case, the related binding
> change is updated at patch [1/6], The udoo board changes were tested
> using my last power sequence patch set.[3]
> 
> Except for hard-wired MMC and USB devices, I find the USB ULPI PHY also
> need to power on itself before it can be found by ULPI bus.

Can patches 3-7 can be sent independently of the power related patches?

cheers,
-roger

> 
> Changes for v13:
> - Add more design descriptions at design doc and fix one build error
>   introduced by v12 wrongly [Patch 2/12]
> - Add the last three dts patches which were forgotten at last series
> - Move the comment for usb_create_shared_hcd to correct place [Patch 3/12]
> - Add sysdev for shared hcd too for xhci-plat.c [Patch 6/12]
> 
> Rafael, if the first two power sequence patches are ok for you, would you 
> consider
> accept these first, the other USB patches can go through USB tree at 
> v4.12-rc1?
> 
> Changes for v12:
> - Add design doc and more comments at generic power sequence source file 
> [Patch 2/9]
> - Introduce four Arnd Bergmann patches and one my ehci related patches, these 
> patches
>   are used to get property DT/firmware information at USB code, and these 
> information
>   are needed for power sequence operation at USB. With these five patches, my 
> chipidea
>   hack patch in previous patch set can be removed. [Patch 3-7/9]
> - Add -ENOENT judgement to avoid USB error if no power sequence library is 
> chosen [9/9]
> 
> Changes for v11:
> - Fix warning: (USB) selects POWER_SEQUENCE which has unmet direct 
> dependencies (OF)
> - Delete redundant copyright statement.
> - Change pr_warn to pr_debug at wrseq_find_available_instance
> - Refine kerneldoc
> - %s/ENONET/ENOENT 
> - Allocate pwrseq list node before than carry out power sequence on 
> - Add mutex_lock/mutex_lock for pwrseq node browse at 
> pwrseq_find_available_instance
> - Add pwrseq_suspend/resume for API both single instance and list 
> - Add .pwrseq_suspend/resume for pwrseq_generic.c
> - Add pwrseq_suspend_list and pwrseq_resume_list for USB hub suspend
>   and resume routine
> 
> Changes for v10:
> - Improve the kernel-doc for power sequence core, including exported APIs and
>   main structure. [Patch 2/8]
> - Change Kconfig, and let the user choose power sequence. [Patch 2/8]
> - Delete EXPORT_SYMBOL and change related APIs as local, these APIs do not
>   be intended to export currently. [Patch 2/8]
> - Selete POWER_SEQUENCE at USB core's Kconfig. [Patch 4/8]
> 
> Changes for v9:
> - Add Vaibhav Hiremath's reviewed-by [Patch 4/8]
> - Rebase to v4.9-rc1
> 
> Changes for v8:
> - Allocate one extra pwrseq instance if pwrseq_get has succeed, it can avoid
>   preallocate instances problem which the number of instance is decided at
>   compile time, thanks for Heiko Stuebner's suggestion [Patch 2/8]
> - Delete pwrseq_compatible_sample.c which is the demo purpose to show 
> compatible
>   match method. [Patch 2/8]
> - Add Maciej S. Szmigiero's tested-by. [Patch 7/8]
> 
> Changes for v7:
> - Create kinds of power sequence instance at postcore_initcall, and match
>   the instance with node using compatible string, the beneit of this is
>   the host driver doesn't need to consider which pwrseq instance needs
>   to be used, and pwrseq core will match it, however, it eats some memories
>   if less power sequence instances are used. [Patch 2/8]
> - Add pwrseq_compatible_sample.c to test match pwrseq using device_id. [Patch 
> 2/8]
> - Fix the comments Vaibhav Hiremath adds for error path for clock and do not
>   use device_node for parameters at pwrseq_on. [Patch 2/8]
> - Simplify the caller to use power sequence, follows Alan's commnets [Patch 
> 4/8]
> - Tested three pwrseq instances together using both specific compatible 
> string and
>   generic libraries.
> 
> Changes for v6:
> - Add Matthias Kaehlcke's Reviewed-by and Tested-by. (patch [2/6])
> - Change chipidea core of_node assignment for coming user. (patch [5/6])
> - Applies Joshua Clayton's three dts changes for two boards,
>   the USB device's reg has only #address-cells, but without #size-cells.
> 
> Chan

Re: [PATCH v13 06/12] usb: xhci: use bus->sysdev for DMA configuration

[Roger Quadros]

Hi,

On 11/02/17 03:27, Peter Chen wrote:
> From: Arnd Bergmann 
> 
> For xhci-hcd platform device, all the DMA parameters are not
> configured properly, notably dma ops for dwc3 devices. So, set
> the dma for xhci from sysdev. sysdev is pointing to device that
> is known to the system firmware or hardware.
> 
> Cc: Baolin Wang 
> Cc: Vivek Gautam 
> Cc: Alexander Sverdlin 
> Cc: Mathias Nyman 
> 
> Signed-off-by: Arnd Bergmann 
> Signed-off-by: Sriram Dash 
> ---
> Hi, Baolin, Vivek and Alexander,
> I removed your tested-by tag due to add one change that adding sysdev
> for shared hcd too, if your test shows this change works for you or
> has no effect for you, please consider adding tested-by tag again,
> thanks.
> 
>  drivers/usb/host/xhci-mem.c  | 12 ++--
>  drivers/usb/host/xhci-plat.c | 35 +++
>  drivers/usb/host/xhci.c  | 15 +++
>  3 files changed, 44 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index ba1853f4..032a702 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -586,7 +586,7 @@ static void xhci_free_stream_ctx(struct xhci_hcd *xhci,
>   unsigned int num_stream_ctxs,
>   struct xhci_stream_ctx *stream_ctx, dma_addr_t dma)
>  {
> - struct device *dev = xhci_to_hcd(xhci)->self.controller;
> + struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
>   size_t size = sizeof(struct xhci_stream_ctx) * num_stream_ctxs;
>  
>   if (size > MEDIUM_STREAM_ARRAY_SIZE)
> @@ -614,7 +614,7 @@ static struct xhci_stream_ctx 
> *xhci_alloc_stream_ctx(struct xhci_hcd *xhci,
>   unsigned int num_stream_ctxs, dma_addr_t *dma,
>   gfp_t mem_flags)
>  {
> - struct device *dev = xhci_to_hcd(xhci)->self.controller;
> + struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
>   size_t size = sizeof(struct xhci_stream_ctx) * num_stream_ctxs;
>  
>   if (size > MEDIUM_STREAM_ARRAY_SIZE)
> @@ -1686,7 +1686,7 @@ void xhci_slot_copy(struct xhci_hcd *xhci,
>  static int scratchpad_alloc(struct xhci_hcd *xhci, gfp_t flags)
>  {
>   int i;
> - struct device *dev = xhci_to_hcd(xhci)->self.controller;
> + struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
>   int num_sp = HCS_MAX_SCRATCHPAD(xhci->hcs_params2);
>  
>   xhci_dbg_trace(xhci, trace_xhci_dbg_init,
> @@ -1758,7 +1758,7 @@ static void scratchpad_free(struct xhci_hcd *xhci)
>  {
>   int num_sp;
>   int i;
> - struct device *dev = xhci_to_hcd(xhci)->self.controller;
> + struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
>  
>   if (!xhci->scratchpad)
>   return;
> @@ -1831,7 +1831,7 @@ void xhci_free_command(struct xhci_hcd *xhci,
>  
>  void xhci_mem_cleanup(struct xhci_hcd *xhci)
>  {
> - struct device   *dev = xhci_to_hcd(xhci)->self.controller;
> + struct device   *dev = xhci_to_hcd(xhci)->self.sysdev;
>   int size;
>   int i, j, num_ports;
>  
> @@ -2373,7 +2373,7 @@ static int xhci_setup_port_arrays(struct xhci_hcd 
> *xhci, gfp_t flags)
>  int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
>  {
>   dma_addr_t  dma;
> - struct device   *dev = xhci_to_hcd(xhci)->self.controller;
> + struct device   *dev = xhci_to_hcd(xhci)->self.sysdev;
>   unsigned intval, val2;
>   u64 val_64;
>   struct xhci_segment *seg;
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 6d33b42..4ecb3fd 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -14,6 +14,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -148,6 +149,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
>  {
>   const struct of_device_id *match;
>   const struct hc_driver  *driver;
> + struct device   *sysdev;
>   struct xhci_hcd *xhci;
>   struct resource *res;
>   struct usb_hcd  *hcd;
> @@ -164,22 +166,39 @@ static int xhci_plat_probe(struct platform_device *pdev)
>   if (irq < 0)
>   return -ENODEV;
>  
> + /*
> +  * sysdev must point to a device that is known to the system firmware
> +  * or PCI hardware. We handle these three cases here:
> +  * 1. xhci_plat comes from firmware
> +  * 2. xhci_plat is child of a device from firmware (dwc3-plat)
> +  * 3. xhci_plat is grandchild of a pci device (dwc3-pci)
> +  */
> + sysdev = &pdev->dev;
> + if (sysdev->parent && !sysdev->of_node && sysdev->parent->of_node)
> + sysdev = sysdev->parent;
> +#ifdef CONFIG_PCI
> + else if (sysdev->parent && sysdev->parent->parent &&
> +  sysdev->parent->parent->bus == &pci_bus_type)
> + sysdev = sysdev->parent->parent;
> +#endif
> +
>   /* Try to set 64-bit DMA first */
> - if (!pdev->dev.dma_mask)
> + if (WARN_O

Re: [PATCH v13 06/12] usb: xhci: use bus->sysdev for DMA configuration

[Arnd Bergmann]

On Tue, Feb 14, 2017 at 11:36 AM, Roger Quadros  wrote:
> On 11/02/17 03:27, Peter Chen wrote:
>> From: Arnd Bergmann 
>>
>> For xhci-hcd platform device, all the DMA parameters are not
>> configured properly, notably dma ops for dwc3 devices. So, set
>> the dma for xhci from sysdev. sysdev is pointing to device that
>> is known to the system firmware or hardware.
>>
>> Cc: Baolin Wang 
>> Cc: Vivek Gautam 
>> Cc: Alexander Sverdlin 
>> Cc: Mathias Nyman 
>>
>> Signed-off-by: Arnd Bergmann 
>> Signed-off-by: Sriram Dash 
>> ---
>> Hi, Baolin, Vivek and Alexander,
>> I removed your tested-by tag due to add one change that adding sysdev
>> for shared hcd too, if your test shows this change works for you or
>> has no effect for you, please consider adding tested-by tag again,
>> thanks.

>> @@ -222,20 +241,20 @@ static int xhci_plat_probe(struct platform_device 
>> *pdev)
>>
>>   xhci->clk = clk;
>>   xhci->main_hcd = hcd;
>> - xhci->shared_hcd = usb_create_shared_hcd(driver, &pdev->dev,
>> + xhci->shared_hcd = __usb_create_hcd(driver, sysdev, &pdev->dev,
>>   dev_name(&pdev->dev), hcd);
>>   if (!xhci->shared_hcd) {
>>   ret = -ENOMEM;
>>   goto disable_clk;
>>   }
>>
>> - if (device_property_read_bool(&pdev->dev, "usb3-lpm-capable"))
>> + if (device_property_read_bool(sysdev, "usb3-lpm-capable"))
>
> Why are we using sysdev to read DT property? We should be using the
> XHCI device (&pdev->dev) here, no?

If I remember correctly, this is one of the cases where pdev does not
have a device node attached to it because it was created by the driver
of the parent device on the fly in case of dwc3. When you have a pure xhci
device in DT, the two pointers are the same.

Arnd
--
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: How to resolve "Waited 2000ms for CONNECT" in system resuming?

[Yoshihiro Shimoda]

Hi Alan,

> From: Alan Stern
> Sent: Tuesday, February 14, 2017 1:35 AM
> 
> On Mon, 13 Feb 2017, Yoshihiro Shimoda wrote:
> 
> > > Hmmm.  You're using platform drivers for OHCI and EHCI, not PCI,
> >
> > Yes, I'm using platform drivers for OHCI and EHCI.
> >
> > > right?  The resume_common() routine in drivers/usb/core/hcd-pci.c is
> > > careful to resume things in the correct order.  It contains this code:
> > >
> > >   /*
> > >* Only EHCI controllers have to wait for their companions.
> > >* No locking is needed because PCI controller drivers do not
> > >* get unbound during system resume.
> > >*/
> > >   if (pci_dev->class == CL_EHCI && event != PM_EVENT_AUTO_RESUME)
> > >   for_each_companion(pci_dev, hcd,
> > >   ehci_wait_for_companions);
> > >
> > > Probably the equivalent routine in the platform driver needs to do the
> > > same sort of thing.  This means it needs to know about companion
> > > controllers.
> >
> > Thank you very much for this information!
> > If I added the following code, the issue disappeared:
> >  - The ehci-platform.c calls 
> > device_enable_async_suspend(hcd->self.controller)
> >in ehci_platform_probe()
> 
> We probably should do that in all the platform drivers anyway.

I got it.

> >  - [This is a dirty code, but] hcd_bus_resume() calls 
> > device_pm_wait_for_dev(
> >rhdev->bus->controller, ohci_dev)
> >
> > I will consider how to implement such a code for [eo]hci-platform drivers.
> > Especially, like ehci_{pre,post}_add() for platform drivers are needed, I 
> > think.
> 
> The key point is that the EHCI controller must be resumed _after_ its
> companion controllers.  In order to do this properly, the platform
> driver needs to know which other devices the companions are.
> 
> There's no way it can figure this out by itself; it has to be told by
> the platform-specific code.

I understood it.
In non-DT case, if we use .id in struct platform_device, there is easy to bind
EHCI and companion controllers. However, in DT environment, there is difficult 
to
bind them.

So, I have 2 ideas for DT case.
 A) We add a new property "companion" as usb-generic.txt and EHCI node(s) have 
such a property
to bind a companion controller.
 B) We assume EHCI controller binds a companion controller if some resources 
(irq or clock)
are the same and it has a compatible strings as "generic-[uo]hci" for 
instance.

What do you think?

Best regards,
Yoshihiro Shimoda

> 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 v13 06/12] usb: xhci: use bus->sysdev for DMA configuration

[Roger Quadros]

On 14/02/17 13:44, Arnd Bergmann wrote:
> On Tue, Feb 14, 2017 at 11:36 AM, Roger Quadros  wrote:
>> On 11/02/17 03:27, Peter Chen wrote:
>>> From: Arnd Bergmann 
>>>
>>> For xhci-hcd platform device, all the DMA parameters are not
>>> configured properly, notably dma ops for dwc3 devices. So, set
>>> the dma for xhci from sysdev. sysdev is pointing to device that
>>> is known to the system firmware or hardware.
>>>
>>> Cc: Baolin Wang 
>>> Cc: Vivek Gautam 
>>> Cc: Alexander Sverdlin 
>>> Cc: Mathias Nyman 
>>>
>>> Signed-off-by: Arnd Bergmann 
>>> Signed-off-by: Sriram Dash 
>>> ---
>>> Hi, Baolin, Vivek and Alexander,
>>> I removed your tested-by tag due to add one change that adding sysdev
>>> for shared hcd too, if your test shows this change works for you or
>>> has no effect for you, please consider adding tested-by tag again,
>>> thanks.
> 
>>> @@ -222,20 +241,20 @@ static int xhci_plat_probe(struct platform_device 
>>> *pdev)
>>>
>>>   xhci->clk = clk;
>>>   xhci->main_hcd = hcd;
>>> - xhci->shared_hcd = usb_create_shared_hcd(driver, &pdev->dev,
>>> + xhci->shared_hcd = __usb_create_hcd(driver, sysdev, &pdev->dev,
>>>   dev_name(&pdev->dev), hcd);
>>>   if (!xhci->shared_hcd) {
>>>   ret = -ENOMEM;
>>>   goto disable_clk;
>>>   }
>>>
>>> - if (device_property_read_bool(&pdev->dev, "usb3-lpm-capable"))
>>> + if (device_property_read_bool(sysdev, "usb3-lpm-capable"))
>>
>> Why are we using sysdev to read DT property? We should be using the
>> XHCI device (&pdev->dev) here, no?
> 
> If I remember correctly, this is one of the cases where pdev does not
> have a device node attached to it because it was created by the driver
> of the parent device on the fly in case of dwc3. When you have a pure xhci
> device in DT, the two pointers are the same.

>From drivers/usb/dwc3/host.c

> if (dwc->usb3_lpm_capable) {
> props[0].name = "usb3-lpm-capable";
> ret = platform_device_add_properties(xhci, props);
> if (ret) {
> dev_err(dwc->dev, "failed to add properties to 
> xHCI\n");
> goto err1;
> }
> }

So it is setting the usb3-lpm-capable property into the xhci platform device
and we should be reading the property from there.

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

[PATCH] usb: add CONFIG_USB_PCI to distinguish PCI/non-PCI based USB

[yuan linyu]

From: yuan linyu 

one usage is in embedded system usb host controller is non-PCI based,
chooe N in such system will not compile PCI code/function of USB.

Signed-off-by: yuan linyu 
---
 drivers/usb/Kconfig|  9 -
 drivers/usb/Makefile   |  2 +-
 drivers/usb/chipidea/Kconfig   |  2 +-
 drivers/usb/core/Makefile  |  2 +-
 drivers/usb/dwc2/Kconfig   |  2 +-
 drivers/usb/dwc3/Kconfig   |  2 +-
 drivers/usb/gadget/udc/Kconfig |  8 
 drivers/usb/gadget/udc/bdc/Kconfig |  2 +-
 drivers/usb/gadget/udc/net2272.c   |  8 
 drivers/usb/gadget/udc/net2272.h   |  2 +-
 drivers/usb/host/Kconfig   | 10 +-
 drivers/usb/host/ehci-dbg.c|  2 +-
 drivers/usb/host/ohci-hcd.c|  2 +-
 drivers/usb/host/ohci.h|  2 +-
 drivers/usb/host/pci-quirks.h  |  4 ++--
 drivers/usb/host/uhci-hcd.c|  2 +-
 drivers/usb/host/uhci-hcd.h|  2 +-
 drivers/usb/host/xhci.c|  2 +-
 drivers/usb/isp1760/isp1760-if.c   |  8 
 include/linux/usb/hcd.h|  4 ++--
 20 files changed, 42 insertions(+), 35 deletions(-)

diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index fbe493d..d6558f2 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig
@@ -35,7 +35,6 @@ config USB_COMMON
 config USB_ARCH_HAS_HCD
def_bool y
 
-# ARM SA chips have a non-PCI based "OHCI-compatible" USB host interface.
 config USB
tristate "Support for Host-side USB"
depends on USB_ARCH_HAS_HCD
@@ -73,6 +72,14 @@ config USB
  To compile this driver as a module, choose M here: the
  module will be called usbcore.
 
+config USB_PCI
+   bool "PCI based USB host interface"
+   depends on PCI
+   default y
+   ---help---
+ if system have both PCI and USB,  but USB is non-PCI related,
+ say N here allow PCI related code not compiled.
+
 if USB
 
 source "drivers/usb/core/Kconfig"
diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
index 7791af6..4e1cf09 100644
--- a/drivers/usb/Makefile
+++ b/drivers/usb/Makefile
@@ -14,7 +14,7 @@ obj-$(CONFIG_USB_ISP1760) += isp1760/
 obj-$(CONFIG_USB_MON)  += mon/
 obj-$(CONFIG_USB_MTU3) += mtu3/
 
-obj-$(CONFIG_PCI)  += host/
+obj-$(CONFIG_USB_PCI)  += host/
 obj-$(CONFIG_USB_EHCI_HCD) += host/
 obj-$(CONFIG_USB_ISP116X_HCD)  += host/
 obj-$(CONFIG_USB_OHCI_HCD) += host/
diff --git a/drivers/usb/chipidea/Kconfig b/drivers/usb/chipidea/Kconfig
index 5e5b9eb..7e915a8 100644
--- a/drivers/usb/chipidea/Kconfig
+++ b/drivers/usb/chipidea/Kconfig
@@ -19,7 +19,7 @@ config USB_CHIPIDEA_OF
 
 config USB_CHIPIDEA_PCI
tristate
-   depends on PCI
+   depends on USB_PCI
depends on NOP_USB_XCEIV
default USB_CHIPIDEA
 
diff --git a/drivers/usb/core/Makefile b/drivers/usb/core/Makefile
index b99b871..250ec1d 100644
--- a/drivers/usb/core/Makefile
+++ b/drivers/usb/core/Makefile
@@ -8,7 +8,7 @@ usbcore-y += devio.o notify.o generic.o quirks.o devices.o
 usbcore-y += port.o
 
 usbcore-$(CONFIG_OF)   += of.o
-usbcore-$(CONFIG_PCI)  += hcd-pci.o
+usbcore-$(CONFIG_USB_PCI)  += hcd-pci.o
 usbcore-$(CONFIG_ACPI) += usb-acpi.o
 
 obj-$(CONFIG_USB)  += usbcore.o
diff --git a/drivers/usb/dwc2/Kconfig b/drivers/usb/dwc2/Kconfig
index e838701..b6a495e 100644
--- a/drivers/usb/dwc2/Kconfig
+++ b/drivers/usb/dwc2/Kconfig
@@ -54,7 +54,7 @@ endchoice
 
 config USB_DWC2_PCI
tristate "DWC2 PCI"
-   depends on PCI
+   depends on USB_PCI
depends on USB_GADGET || !USB_GADGET
default n
select NOP_USB_XCEIV
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index c5aa235..4c9e56d 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -70,7 +70,7 @@ config USB_DWC3_EXYNOS
 
 config USB_DWC3_PCI
tristate "PCIe-based Platforms"
-   depends on PCI && ACPI
+   depends on USB_PCI && ACPI
default USB_DWC3
help
  If you're using the DesignWare Core IP with a PCIe, please say
diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig
index 658b8da..5761767 100644
--- a/drivers/usb/gadget/udc/Kconfig
+++ b/drivers/usb/gadget/udc/Kconfig
@@ -263,7 +263,7 @@ source "drivers/usb/gadget/udc/bdc/Kconfig"
 
 config USB_AMD5536UDC
tristate "AMD5536 UDC"
-   depends on PCI
+   depends on USB_PCI
help
   The AMD5536 UDC is part of the AMD Geode CS5536, an x86 southbridge.
   It is a USB Highspeed DMA capable USB device controller. Beside ep0
@@ -313,7 +313,7 @@ config USB_NET2272_DMA
 
 config USB_NET2280
tristate "NetChip NET228x / PLX USB3x8x"
-   depends on PCI
+   depends on USB_PCI
help
   NetChip 2280 / 2282 is a PCI based USB peripheral controller which
   supports both full and high speed USB 2.0 data transfers.
@

Re: [PATCH v13 06/12] usb: xhci: use bus->sysdev for DMA configuration

[Arnd Bergmann]

On Tue, Feb 14, 2017 at 1:26 PM, Roger Quadros  wrote:
> On 14/02/17 13:44, Arnd Bergmann wrote:
>> On Tue, Feb 14, 2017 at 11:36 AM, Roger Quadros  wrote:

>>> Why are we using sysdev to read DT property? We should be using the
>>> XHCI device (&pdev->dev) here, no?
>>
>> If I remember correctly, this is one of the cases where pdev does not
>> have a device node attached to it because it was created by the driver
>> of the parent device on the fly in case of dwc3. When you have a pure xhci
>> device in DT, the two pointers are the same.
>
> From drivers/usb/dwc3/host.c
>
>> if (dwc->usb3_lpm_capable) {
>> props[0].name = "usb3-lpm-capable";
>> ret = platform_device_add_properties(xhci, props);
>> if (ret) {
>> dev_err(dwc->dev, "failed to add properties to 
>> xHCI\n");
>> goto err1;
>> }
>> }
>
> So it is setting the usb3-lpm-capable property into the xhci platform device
> and we should be reading the property from there.

Hmm, ideally we would only have properties on one of the two, since we
refer to the sysdev for the properties regarding DMA and PHY among other
things, but I guess that's not an option here, since we can't call
platform_device_add_properties() on a dwc_pci device and have to
use the xhci pdev instead.

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

Re: [BUG] usb: gadget: Kernel oops with UVC USB gadget and configfs

[Laurent Pinchart]

Hi Robert,

On Monday 13 Feb 2017 20:29:36 Robert Jarzmik wrote:
> Petr Cvek  writes:
> > Dne 12.2.2017 v 13:02 Robert Jarzmik napsal(a):
> > That's weird I even removed pxa_set_udc_info() from magician machine init
> > and it still fails. Host cable and/or actual camera is not required. It
> > fails without them.
> > 
> > So you binded pxa27x-udc as UDC controller (= activated it) and then
> > rmmoded it and nobody complained?
> 
> No, that usecase actually fails. I think you could submit a proper patch
> with the diff in [1], with which the pxa27x-udc unloading will work.
> 
> But it's not _your_ testcase, as per your provided callstack :
> [ 2152.826529] [] (udc_bind_to_driver [udc_core]) from
> [] (usb_add_gadget_udc_release+0x138/0x21c [udc_core]) [
> 2152.826731] [] (usb_add_gadget_udc_release [udc_core]) from
> [] (pxa_udc_probe+0x290/0x2fc [pxa27x_udc]) [ 2152.833554]
> [] (pxa_udc_probe [pxa27x_udc]) from []
> (platform_drv_probe+0x38/0x84) [ 2152.833602] []
> (platform_drv_probe) from [] (driver_probe_device+0x1e0/0x3f4)
> 
> Your problem seems in the pxa_udc_probe(), which I would presume you're
> doing for a second time, ie. after modprobe pxa27x_udc + bind UDC
> controller + rmmod pxa27x_udc + modprobe pxa27x_udc.
> 
> I suspect that in this case, the problem is that the rmmod works while udc
> is still bound to the composite. The lsmod seems to prove that the refcount
> is still 0 while usb_f_uvc is is bound to pxa27x_udc.
> 
> [@Felipe and @Laurent]
> I have no knowledge of usb_f_uvc, so would you tell me if binding usb_f_uvc
> to pxa27_udc should have incremented the module refcount of pxa27x_udc or
> not ?

I'm not familiar with how modules refcount is handled in the USB gadget 
framework, that's more a question for Felipe. I don't think the problem is 
specific to usb_f_uvc, all gadget drivers should behave the same way (and if 
they don't that should be fixed).

> > Can you try v4.10? Or send me exact commit on what you tested it so I can
> > test it too.
> 
> My top commit (not counting yours) is : 61c04572de40 ("Merge branch 'for-rc'
> of git://git.kernel.org/pub/scm/linux/kernel/git/rzhang/linux")

-- 
Regards,

Laurent Pinchart

--
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: Re: Re: Re: Subject: [PATCH v1] USB:Core: BugFix: Proper handling of Race Condition when two USB class drivers try to call init_usb_class simultaneously

[Ajay Kaher]

 At boot time, probe function of multiple connected devices
 (proprietary devices) execute simultaneously.
>>>
>>> What exactly do you mean here?  How can probe happen "simultaneously"?
>>> The USB core prevents this, right?
>> 
>> I have observed two scenarios to call probe function:
>> 
>> Scenario #1: Driver inserted and attaching USB Device:
>> Yes, you are right, two probes at same time is not happening
>> in this scenario.
>> 
>> Scenario #2: USB Device attached and inserting Driver:
>> In this case probe has been called in context of insmod,
>> refer following code flow:
>> init -> usb_register_driver -> driver_register -> bus_add_driver ->
>> driver_attach -> bus_for_each_dev -> __driver_attach ->
>> driver_probe_device -> usb_probe_interface -> probe -> usb_register_dev
>> 
>> I have observed the crash in Scenario #2, as two probes executes at
>> same time in this scenario. And init_usb_class_mutex lock require to
>> prevent race condition.
> 
> What about the fact that in __driver_attach() we call device_lock() so
> that probe never gets called at the same time for the same device?

Devices are different.

> Or are you saying that you can load multiple USB modules at the same
> time?  If so, how is insmod running on multiple cpus at the same time?
> I thought we had a global lock there to prevent that from happening
> (i.e. only one module can be loaded at a time.)  Or is that what has
> recently changed?

Yes, we can load multiple USB modules at the same time using insmod.
Tested on ARM Architecture with Linux kernel 4.1.10, that we can have 
multiple insmod on multiple cpus at same time. Also reviewed load_module and 
do_init_module functions and couldn't find any global lock.

> 
> What is causing your modules to be loaded from userspace?  What type of
> device is this happening for?  And why haven't we seen this before?
> What kernel versions have you had a problem with this?

Earlier we execute insmod in foreground as "insmod aaa.ko ; insmod bbb.ko"
and that's why insmod executed sequentially. Now we modified to execute in 
parallel/background as "insmod aaa.ko & ; insmod bbb.ko &". 

> And what for what drivers specifically?

This problem observed for drivers in which usb_register_dev called from their
probe function, and there are many such drivers.

As per my opinion, usb_class structure is global and allocated + initialized
in usb_register_dev->init_usb_class function. Also as per scenario #2
concurrency is possible, so protection using init_usb_class_mutex lock requires.
Don't you think so?

 And because of the following code path race condition happens:
 probe->usb_register_dev->init_usb_class
>>>
>>> Why is this just showing up now, and hasn't been an issue for the decade
>>> or so this code has been around?  What changed?
>>>
 Tested with these changes, and problem has been solved.
>>>
>>> What changes?
>> 
>> Tested with my patch (i.e. locking with init_usb_class_mutex).
> 
> I don't see a patch here :(

Sorry, appending the patch again with this mail.
 
thanks,
 
ajay kaher


Signed-off-by: Ajay Kaher

---

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


diff --git a/drivers/usb/core/file.c b/drivers/usb/core/file.c
index 822ced9..dedc47c 100644
--- a/drivers/usb/core/file.c
+++ b/drivers/usb/core/file.c
@@ -156,6 +156,7 @@ int usb_register_dev(struct usb_interface *intf,
  int minor_base = class_driver->minor_base;
  int minor;
  char name[20];
+ static DEFINE_MUTEX(init_usb_class_mutex);
 
 #ifdef CONFIG_USB_DYNAMIC_MINORS
  /*
@@ -171,7 +172,10 @@ int usb_register_dev(struct usb_interface *intf,
  if (intf->minor >= 0)
   return -EADDRINUSE;
 
+ mutex_lock(&init_usb_class_mutex);
  retval = init_usb_class();
+ mutex_unlock(&init_usb_class_mutex);
+
  if (retval)
   return retval;

Re: [PATCH] usb: add CONFIG_USB_PCI to distinguish PCI/non-PCI based USB

[Greg Kroah-Hartman]

On Tue, Feb 14, 2017 at 08:46:16PM +0800, yuan linyu wrote:
> From: yuan linyu 
> 
> one usage is in embedded system usb host controller is non-PCI based,
> chooe N in such system will not compile PCI code/function of USB.

I'm really sorry, but I can not parse this very well.  What exact
problem does this patch solve?  Why is it needed?  It seems to just keep
the same functionality that we currently have today, right?

We already distinguish PCI/non-PCI based USB host controllers by using
the CONFIG_PCI option, why do we need to add another one?

> 
> Signed-off-by: yuan linyu 
> ---
>  drivers/usb/Kconfig|  9 -
>  drivers/usb/Makefile   |  2 +-
>  drivers/usb/chipidea/Kconfig   |  2 +-
>  drivers/usb/core/Makefile  |  2 +-
>  drivers/usb/dwc2/Kconfig   |  2 +-
>  drivers/usb/dwc3/Kconfig   |  2 +-
>  drivers/usb/gadget/udc/Kconfig |  8 
>  drivers/usb/gadget/udc/bdc/Kconfig |  2 +-
>  drivers/usb/gadget/udc/net2272.c   |  8 
>  drivers/usb/gadget/udc/net2272.h   |  2 +-
>  drivers/usb/host/Kconfig   | 10 +-
>  drivers/usb/host/ehci-dbg.c|  2 +-
>  drivers/usb/host/ohci-hcd.c|  2 +-
>  drivers/usb/host/ohci.h|  2 +-
>  drivers/usb/host/pci-quirks.h  |  4 ++--
>  drivers/usb/host/uhci-hcd.c|  2 +-
>  drivers/usb/host/uhci-hcd.h|  2 +-
>  drivers/usb/host/xhci.c|  2 +-
>  drivers/usb/isp1760/isp1760-if.c   |  8 
>  include/linux/usb/hcd.h|  4 ++--
>  20 files changed, 42 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
> index fbe493d..d6558f2 100644
> --- a/drivers/usb/Kconfig
> +++ b/drivers/usb/Kconfig
> @@ -35,7 +35,6 @@ config USB_COMMON
>  config USB_ARCH_HAS_HCD
>   def_bool y
>  
> -# ARM SA chips have a non-PCI based "OHCI-compatible" USB host interface.
>  config USB
>   tristate "Support for Host-side USB"
>   depends on USB_ARCH_HAS_HCD
> @@ -73,6 +72,14 @@ config USB
> To compile this driver as a module, choose M here: the
> module will be called usbcore.
>  
> +config USB_PCI
> + bool "PCI based USB host interface"
> + depends on PCI
> + default y
> + ---help---
> +   if system have both PCI and USB,  but USB is non-PCI related,
> +   say N here allow PCI related code not compiled.

Again, I think this needs some more description, as I do not understand
why this new option is needed at all.

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: Re: Re: Re: Subject: [PATCH v1] USB:Core: BugFix: Proper handling of Race Condition when two USB class drivers try to call init_usb_class simultaneously

[Alan Stern]

On Thu, 2 Feb 2017, Ajay Kaher wrote:

>  At boot time, probe function of multiple connected devices
>  (proprietary devices) execute simultaneously.
> >>>
> >>> What exactly do you mean here?  How can probe happen "simultaneously"?
> >>> The USB core prevents this, right?
> >> 
> >> I have observed two scenarios to call probe function:
> >> 
> >> Scenario #1: Driver inserted and attaching USB Device:
> >> Yes, you are right, two probes at same time is not happening
> >> in this scenario.
> >> 
> >> Scenario #2: USB Device attached and inserting Driver:
> >> In this case probe has been called in context of insmod,
> >> refer following code flow:
> >> init -> usb_register_driver -> driver_register -> bus_add_driver ->
> >> driver_attach -> bus_for_each_dev -> __driver_attach ->
> >> driver_probe_device -> usb_probe_interface -> probe -> usb_register_dev
> >> 
> >> I have observed the crash in Scenario #2, as two probes executes at
> >> same time in this scenario. And init_usb_class_mutex lock require to
> >> prevent race condition.
> > 
> > What about the fact that in __driver_attach() we call device_lock() so
> > that probe never gets called at the same time for the same device?
> 
> Devices are different.
> 
> > Or are you saying that you can load multiple USB modules at the same
> > time?  If so, how is insmod running on multiple cpus at the same time?
> > I thought we had a global lock there to prevent that from happening
> > (i.e. only one module can be loaded at a time.)  Or is that what has
> > recently changed?
> 
> Yes, we can load multiple USB modules at the same time using insmod.
> Tested on ARM Architecture with Linux kernel 4.1.10, that we can have 
> multiple insmod on multiple cpus at same time. Also reviewed load_module and 
> do_init_module functions and couldn't find any global lock.
> 
> > 
> > What is causing your modules to be loaded from userspace?  What type of
> > device is this happening for?  And why haven't we seen this before?
> > What kernel versions have you had a problem with this?
> 
> Earlier we execute insmod in foreground as "insmod aaa.ko ; insmod bbb.ko"
> and that's why insmod executed sequentially. Now we modified to execute in 
> parallel/background as "insmod aaa.ko & ; insmod bbb.ko &". 
> 
> > And what for what drivers specifically?
> 
> This problem observed for drivers in which usb_register_dev called from their
> probe function, and there are many such drivers.
> 
> As per my opinion, usb_class structure is global and allocated + initialized
> in usb_register_dev->init_usb_class function. Also as per scenario #2
> concurrency is possible, so protection using init_usb_class_mutex lock 
> requires.
> Don't you think so?
> 
>  And because of the following code path race condition happens:
>  probe->usb_register_dev->init_usb_class
> >>>
> >>> Why is this just showing up now, and hasn't been an issue for the decade
> >>> or so this code has been around?  What changed?
> >>>
>  Tested with these changes, and problem has been solved.
> >>>
> >>> What changes?
> >> 
> >> Tested with my patch (i.e. locking with init_usb_class_mutex).
> > 
> > I don't see a patch here :(
> 
> Sorry, appending the patch again with this mail.
>  
> thanks,
>  
> ajay kaher
> 
> 
> Signed-off-by: Ajay Kaher

I think Ajay's argument is correct and a patch is needed.  But this
patch misses the race between init_usb_class() and release_usb_class().  

The basic problem is that reference counting doesn't work when you try
to use the same global pointer (usb_class) to refer to multiple
generations of a dynamically allocated entity.  We had the same sort of
problem many years ago with the usb_interface structure (and we
ultimately fixed it by creating a separate usb_interface_cache
structure).

The best approach here would be to forget about all the reference
counting.  Get rid of usb_class entirely, and create the "usbmisc"
class structure just once, when usbcore initializes.  Or, if you
prefer, use a mutex to protect a routine that allocates the class
structure dynamically, just once.  Either way, don't deallocate it
until usbcore is unloaded.

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: add CONFIG_USB_PCI to distinguish PCI/non-PCI based USB

[Alan Stern]

On Tue, 14 Feb 2017, Greg Kroah-Hartman wrote:

> On Tue, Feb 14, 2017 at 08:46:16PM +0800, yuan linyu wrote:
> > From: yuan linyu 
> > 
> > one usage is in embedded system usb host controller is non-PCI based,
> > chooe N in such system will not compile PCI code/function of USB.
> 
> I'm really sorry, but I can not parse this very well.  What exact
> problem does this patch solve?  Why is it needed?  It seems to just keep
> the same functionality that we currently have today, right?
> 
> We already distinguish PCI/non-PCI based USB host controllers by using
> the CONFIG_PCI option, why do we need to add another one?

The problem that this patch tries to fix arises when you have a system
where CONFIG_PCI is enabled for other reasons, but the USB hardware is
non-PCI.  In that situation, you don't want to build the PCI-based USB
drivers unnecessarily.

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 1/4 linux-next] usb: gadget: udc: atmel: Check fifo configuration values against device tree

[cristian.birsan]

From: Cristian Birsan 

Check fifo configuration values against device tree values for endpoint
fifo in auto configuration mode (fifo_mode=0).

Signed-off-by: Cristian Birsan 
---
 drivers/usb/gadget/udc/atmel_usba_udc.c | 26 +++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c 
b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 11bbce2..ce8bf8b 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -371,7 +371,7 @@ static struct usba_fifo_cfg mode_4_cfg[] = {
 };
 /* Add additional configurations here */
 
-int usba_config_fifo_table(struct usba_udc *udc)
+static int usba_config_fifo_table(struct usba_udc *udc)
 {
int n;
 
@@ -2118,14 +2118,34 @@ static struct usba_ep * atmel_udc_of_init(struct 
platform_device *pdev,
dev_err(&pdev->dev, "of_probe: fifo-size error(%d)\n", 
ret);
goto err;
}
-   ep->fifo_size = fifo_mode ? udc->fifo_cfg[i].fifo_size : val;
+   if (fifo_mode) {
+   if (val < udc->fifo_cfg[i].fifo_size) {
+   dev_warn(&pdev->dev,
+"of_probe: fifo-size table value not 
supported by HW, using DT value\n");
+   ep->fifo_size = val;
+   } else {
+   ep->fifo_size = udc->fifo_cfg[i].fifo_size;
+   }
+   } else {
+   ep->fifo_size = val;
+   }
 
ret = of_property_read_u32(pp, "atmel,nb-banks", &val);
if (ret) {
dev_err(&pdev->dev, "of_probe: nb-banks error(%d)\n", 
ret);
goto err;
}
-   ep->nr_banks = fifo_mode ? udc->fifo_cfg[i].nr_banks : val;
+   if (fifo_mode) {
+   if (val < udc->fifo_cfg[i].nr_banks) {
+   dev_warn(&pdev->dev,
+"of_probe: nb-banks table value not 
supported by HW, using DT value\n");
+   ep->nr_banks = val;
+   } else {
+   ep->nr_banks = udc->fifo_cfg[i].nr_banks;
+   }
+   } else {
+   ep->nr_banks = val;
+   }
 
ep->can_dma = of_property_read_bool(pp, "atmel,can-dma");
ep->can_isoc = of_property_read_bool(pp, "atmel,can-isoc");
-- 
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/4 linux-next] usb: gadget: udc: atmel: Endpoint allocation scheme fixes

[cristian.birsan]

From: Cristian Birsan 

This patch series provides fixes, based on the feedback received on the mailing 
list, for
the following:
- fifo table parameters validation against device tree values
- coding style
- message display for EP configuration error
- Kconfig comments for fifo_mode=0

Cristian Birsan (4):
  usb: gadget: udc: atmel: Check fifo configuration values against
device tree
  usb: gadget: udc: atmel: Minor code cleanup
  usb: gadget: udc: atmel: Use dev_warn() to display EP configuration
error
  usb: gadget: udc: atmel: Update Kconfig help for fifo_mode = 0

 drivers/usb/gadget/udc/Kconfig  |  5 ++--
 drivers/usb/gadget/udc/atmel_usba_udc.c | 47 ++---
 2 files changed, 35 insertions(+), 17 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 2/4 linux-next] usb: gadget: udc: atmel: Minor code cleanup

[cristian.birsan]

From: Cristian Birsan 

Minor code cleanup based on feedback received on mailinglist.

Signed-off-by: Cristian Birsan 
---
 drivers/usb/gadget/udc/atmel_usba_udc.c | 15 ++-
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c 
b/drivers/usb/gadget/udc/atmel_usba_udc.c
index ce8bf8b..3becb28 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -321,7 +321,6 @@ static inline void usba_cleanup_debugfs(struct usba_udc 
*udc)
 
 static ushort fifo_mode;
 
-/* "modprobe ... fifo_mode=1" etc */
 module_param(fifo_mode, ushort, 0x0);
 MODULE_PARM_DESC(fifo_mode, "Endpoint configuration mode");
 
@@ -1076,11 +1075,9 @@ static int atmel_usba_start(struct usb_gadget *gadget,
struct usb_gadget_driver *driver);
 static int atmel_usba_stop(struct usb_gadget *gadget);
 
-static struct usb_ep *atmel_usba_match_ep(
-   struct usb_gadget   *gadget,
-   struct usb_endpoint_descriptor  *desc,
-   struct usb_ss_ep_comp_descriptor *ep_comp
-)
+static struct usb_ep *atmel_usba_match_ep(struct usb_gadget *gadget,
+   struct usb_endpoint_descriptor  *desc,
+   struct usb_ss_ep_comp_descriptor *ep_comp)
 {
struct usb_ep   *_ep;
struct usba_ep *ep;
@@ -1100,7 +1097,6 @@ static struct usb_ep *atmel_usba_match_ep(
ep = to_usba_ep(_ep);
 
switch (usb_endpoint_type(desc)) {
-
case USB_ENDPOINT_XFER_CONTROL:
break;
 
@@ -1141,7 +1137,7 @@ static struct usb_ep *atmel_usba_match_ep(
ep->udc->configured_ep++;
}
 
-return _ep;
+   return _ep;
 }
 
 static const struct usb_gadget_ops usba_udc_ops = {
@@ -2089,8 +2085,9 @@ static struct usba_ep * atmel_udc_of_init(struct 
platform_device *pdev,
while ((pp = of_get_next_child(np, pp)))
udc->num_ep++;
udc->configured_ep = 1;
-   } else
+   } else {
udc->num_ep = usba_config_fifo_table(udc);
+   }
 
eps = devm_kzalloc(&pdev->dev, sizeof(struct usba_ep) * udc->num_ep,
   GFP_KERNEL);
-- 
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 4/4 linux-next] usb: gadget: udc: atmel: Update Kconfig help for fifo_mode = 0

[cristian.birsan]

From: Cristian Birsan 

Update Kconfig help for fifo_mode = 0 to explain the behavior better.

Signed-off-by: Cristian Birsan 
---
 drivers/usb/gadget/udc/Kconfig | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig
index 4b69f28..be94eb9 100644
--- a/drivers/usb/gadget/udc/Kconfig
+++ b/drivers/usb/gadget/udc/Kconfig
@@ -62,8 +62,9 @@ config USB_ATMEL_USBA
 
  The fifo_mode parameter is used to select endpoint allocation mode.
  fifo_mode = 0 is used to let the driver autoconfigure the endpoints.
- In this case 2 banks are allocated for isochronous endpoints and
- only one bank is allocated for the rest of the endpoints.
+ In this case, for ep1 2 banks are allocated if it works in isochronous
+ mode and only 1 bank otherwise. For the rest of the endpoints
+ only 1 bank is allocated.
 
  fifo_mode = 1 is a generic maximum fifo size (1024 bytes) 
configuration
  allowing the usage of ep1 - ep6
-- 
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 3/4 linux-next] usb: gadget: udc: atmel: Use dev_warn() to display EP configuration error

[cristian.birsan]

From: Cristian Birsan 

Use dev_warn() to display EP configuration error to avoid silent failure.

Signed-off-by: Cristian Birsan 
---
 drivers/usb/gadget/udc/atmel_usba_udc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c 
b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 3becb28..50f018a 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -1851,7 +1851,7 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
 * but it's clearly harmless...
 */
if (!(usba_ep_readl(ep0, CFG) & USBA_EPT_MAPPED))
-   dev_dbg(&udc->pdev->dev,
+   dev_warn(&udc->pdev->dev,
 "ODD: EP0 configuration is invalid!\n");
 
/* Preallocate other endpoints */
@@ -1860,8 +1860,8 @@ static irqreturn_t usba_udc_irq(int irq, void *devid)
ep = &udc->usba_ep[i];
usba_ep_writel(ep, CFG, ep->ept_cfg);
if (!(usba_ep_readl(ep, CFG) & USBA_EPT_MAPPED))
-   dev_dbg(&udc->pdev->dev,
-"ODD: EP%d configuration is invalid!\n", i);
+   dev_warn(&udc->pdev->dev,
+"ODD: EP%d configuration is 
invalid!\n", i);
}
}
 
-- 
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] usb: add CONFIG_USB_PCI to distinguish PCI/non-PCI based USB

[Greg Kroah-Hartman]

On Tue, Feb 14, 2017 at 10:41:06AM -0500, Alan Stern wrote:
> On Tue, 14 Feb 2017, Greg Kroah-Hartman wrote:
> 
> > On Tue, Feb 14, 2017 at 08:46:16PM +0800, yuan linyu wrote:
> > > From: yuan linyu 
> > > 
> > > one usage is in embedded system usb host controller is non-PCI based,
> > > chooe N in such system will not compile PCI code/function of USB.
> > 
> > I'm really sorry, but I can not parse this very well.  What exact
> > problem does this patch solve?  Why is it needed?  It seems to just keep
> > the same functionality that we currently have today, right?
> > 
> > We already distinguish PCI/non-PCI based USB host controllers by using
> > the CONFIG_PCI option, why do we need to add another one?
> 
> The problem that this patch tries to fix arises when you have a system
> where CONFIG_PCI is enabled for other reasons, but the USB hardware is
> non-PCI.  In that situation, you don't want to build the PCI-based USB
> drivers unnecessarily.

Ok, that would be a good start of a description of this config option :)

thanks,

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

Re: UAS not working with JMS567 based disk enclosure

[Alan Stern]

On Tue, 14 Feb 2017, Jack Coulter wrote:

> Hi,
> 
> I'm using an external multiple-disk enclosure (specifically a Hotway
> H82-SU3S2), which from lsusb appears to use a JMS567 SATA-USB bridge:
> 
> > Bus 002 Device 002: ID 152d:0567 JMicron Technology Corp. / JMicron
> > USA Technology Corp. JMS567 SATA 6Gb/s bridge
> 
> 
> According to the manufacturer's product sheet [1] for this chip, it
> supports the UAS protocol, but when connected to my system (running
> kernel 4.9.8), it falls back to the older usb-storage driver:
> 
> > /:  Bus 02.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/4p, 5000M
> > |__ Port 3: Dev 2, If 0, Class=Mass Storage, Driver=usb-storage, 5000M
> 
> 
> I had a look at uas-detect.h, specifically uas_use_uas_driver, but I
> didn't see any of the warning messages within that function printed to
> dmesg when the device is attached. I added some extra dev_warn calls
> earlier in the function and determined that uas_find_uas_alt_setting is
> returning a negative value. This prompted me to look at the output of
> lsusb -v for this device:
> 
> > Bus 002 Device 002: ID 152d:0567 JMicron Technology Corp. / JMicron
> > USA Technology Corp. JMS567 SATA 6Gb/s bridge
> > Device Descriptor:
> >   bLength18
> >   bDescriptorType 1
> >   bcdUSB   3.00
> >   bDeviceClass0
> >   bDeviceSubClass 0
> >   bDeviceProtocol 0
> >   bMaxPacketSize0 9
> >   idVendor   0x152d JMicron Technology Corp. / JMicron USA
> > Technology Corp.
> >   idProduct  0x0567 JMS567 SATA 6Gb/s bridge
> >   bcdDevice2.05
> >   iManufacturer  10 JMicron
> >   iProduct   11 USB to ATA/ATAPI Bridge
> >   iSerial 5 152D00539000
> >   bNumConfigurations  1
> >   Configuration Descriptor:
> > bLength 9
> > bDescriptorType 2
> > wTotalLength   44
> > bNumInterfaces  1
> > bConfigurationValue 1
> > iConfiguration  0
> > bmAttributes 0xc0
> >   Self Powered
> > MaxPower2mA
> > Interface Descriptor:
> >   bLength 9
> >   bDescriptorType 4
> >   bInterfaceNumber0
> >   bAlternateSetting   0
> >   bNumEndpoints   2
> >   bInterfaceClass 8 Mass Storage
> >   bInterfaceSubClass  6 SCSI
> >   bInterfaceProtocol 80 Bulk-Only
> >   iInterface  0
> >   Endpoint Descriptor:
> > bLength 7
> > bDescriptorType 5
> > bEndpointAddress 0x81  EP 1 IN
> > bmAttributes2
> >   Transfer TypeBulk
> >   Synch Type   None
> >   Usage Type   Data
> > wMaxPacketSize 0x0400  1x 1024 bytes
> > bInterval   0
> > bMaxBurst  15
> >   Endpoint Descriptor:
> > bLength 7
> > bDescriptorType 5
> > bEndpointAddress 0x02  EP 2 OUT
> > bmAttributes2
> >   Transfer TypeBulk
> >   Synch Type   None
> >   Usage Type   Data
> > wMaxPacketSize 0x0400  1x 1024 bytes
> > bInterval   0
> > bMaxBurst  15
> > Binary Object Store Descriptor:
> >   bLength 5
> >   bDescriptorType15
> >   wTotalLength   22
> >   bNumDeviceCaps  2
> >   USB 2.0 Extension Device Capability:
> > bLength 7
> > bDescriptorType16
> > bDevCapabilityType  2
> > bmAttributes   0x0002
> >   HIRD Link Power Management (LPM) Supported
> >   SuperSpeed USB Device Capability:
> > bLength10
> > bDescriptorType16
> > bDevCapabilityType  3
> > bmAttributes 0x00
> > wSpeedsSupported   0x000e
> >   Device can operate at Full Speed (12Mbps)
> >   Device can operate at High Speed (480Mbps)
> >   Device can operate at SuperSpeed (5Gbps)
> > bFunctionalitySupport   1
> >   Lowest fully-functional device speed is Full Speed (12Mbps)
> > bU1DevExitLat  10 micro seconds
> > bU2DevExitLat2047 micro seconds
> > Device Status: 0x000d
> >   Self Powered
> >   U1 Enabled
> >   U2 Enabled
> 
> It seems that it's lacking the interface descriptor for UAS, when I
> compared it to the output from a different enclosure with which UAS
> works correctly:
> 
> > Bus 002 Device 092: ID 174c:1351 ASMedia Technology Inc.
> > Device Descriptor:
> >   bLength18
> >   bDescriptorType 1
> >   bcdUSB   3.10
> >   bDeviceClass0
> >   bDeviceSubClass 0
> >   bDeviceProtocol 0
> >   bMaxPacketSize0 9
> >   idVendor   0x174c ASMedia Technology Inc.
> >

Re: [PATCH] usb: musb: add code comment for clarification

[Greg KH]

On Fri, Feb 10, 2017 at 06:57:41PM -0600, Gustavo A. R. Silva wrote:
> Add code comment to make it clear that the fall-through is intentional.
> Read the link for more details: https://lkml.org/lkml/2017/2/9/292
> 
> Signed-off-by: Gustavo A. R. Silva 
> ---
>  drivers/usb/musb/musb_core.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> index 892088f..1aec986 100644
> --- a/drivers/usb/musb/musb_core.c
> +++ b/drivers/usb/musb/musb_core.c
> @@ -1869,6 +1869,7 @@ static void musb_pm_runtime_check_session(struct musb
> *musb)
> 
> return;
> }
> +   /* fall through */
> case MUSB_QUIRK_A_DISCONNECT_19:
> if (musb->quirk_retries--) {
> musb_dbg(musb,

The tabs are all gone from this patch, and it's line-wrapped, making it
impossible to be applied :(

Can you please fix this and resend?

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: How to resolve "Waited 2000ms for CONNECT" in system resuming?

[Alan Stern]

On Tue, 14 Feb 2017, Yoshihiro Shimoda wrote:

> Hi Alan,
> 
> > From: Alan Stern
> > Sent: Tuesday, February 14, 2017 1:35 AM
> > 
> > On Mon, 13 Feb 2017, Yoshihiro Shimoda wrote:
> > 
> > > > Hmmm.  You're using platform drivers for OHCI and EHCI, not PCI,
> > >
> > > Yes, I'm using platform drivers for OHCI and EHCI.
> > >
> > > > right?  The resume_common() routine in drivers/usb/core/hcd-pci.c is
> > > > careful to resume things in the correct order.  It contains this code:
> > > >
> > > > /*
> > > >  * Only EHCI controllers have to wait for their 
> > > > companions.
> > > >  * No locking is needed because PCI controller drivers 
> > > > do not
> > > >  * get unbound during system resume.
> > > >  */
> > > > if (pci_dev->class == CL_EHCI && event != 
> > > > PM_EVENT_AUTO_RESUME)
> > > > for_each_companion(pci_dev, hcd,
> > > > ehci_wait_for_companions);
> > > >
> > > > Probably the equivalent routine in the platform driver needs to do the
> > > > same sort of thing.  This means it needs to know about companion
> > > > controllers.
> > >
> > > Thank you very much for this information!
> > > If I added the following code, the issue disappeared:
> > >  - The ehci-platform.c calls 
> > > device_enable_async_suspend(hcd->self.controller)
> > >in ehci_platform_probe()
> > 
> > We probably should do that in all the platform drivers anyway.
> 
> I got it.
> 
> > >  - [This is a dirty code, but] hcd_bus_resume() calls 
> > > device_pm_wait_for_dev(
> > >rhdev->bus->controller, ohci_dev)
> > >
> > > I will consider how to implement such a code for [eo]hci-platform drivers.
> > > Especially, like ehci_{pre,post}_add() for platform drivers are needed, I 
> > > think.
> > 
> > The key point is that the EHCI controller must be resumed _after_ its
> > companion controllers.  In order to do this properly, the platform
> > driver needs to know which other devices the companions are.
> > 
> > There's no way it can figure this out by itself; it has to be told by
> > the platform-specific code.
> 
> I understood it.
> In non-DT case, if we use .id in struct platform_device, there is easy to bind
> EHCI and companion controllers. However, in DT environment, there is 
> difficult to
> bind them.
> 
> So, I have 2 ideas for DT case.
>  A) We add a new property "companion" as usb-generic.txt and EHCI node(s) 
> have such a property
> to bind a companion controller.
>  B) We assume EHCI controller binds a companion controller if some resources 
> (irq or clock)
> are the same and it has a compatible strings as "generic-[uo]hci" for 
> instance.
> 
> What do you think?

I'm not very familiar with DT programming.  It would be better to ask 
somebody else.

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: v4.10-rc8 (-rc6) boot regression on Intel desktop, does not boot after cold boots, boots after reboot

[Pavel Machek]

Hi!

> > > > Hmm. I moved keyboard between USB ports, and now 4.10-rc6 no longer
> > > > boots. v4.6 works ok. Let me try with keyboard unplugged... no, I
> > > > could not get it to work. I believe v4.9 and some v4.10-rc's worked,
> > > > but I'll have to double check.
> > > 
> > > But all the kernel versions worked when the keyboard was plugged into
> > > its original USB port?
> > 
> > Aha. So it looks difference is probably in "where is keyboard plugged
> > in" but in "reboot" vs. "cold boot". I did not do a cold boot in quite
> > a while :-(.
> > 
> > Booting to grub, then hitting ctrl-alt-del is enough to make it work. Ouch.
> > 
> > It happens with current Linus' tree.
> 
> v4.10-rc6-feb3 : broken
> v4.9 : ok
> (v4.6 : ok)

Hmm. It hangs during PCI fixups, and it hangs in v4.10-rc8, too.   

With debug patch below, I get

...1d.7: PCI fixup... pass 2
...1d.7: PCI fixup... pass 3
...1d.7: PCI fixup... pass 3 done

...followed by hang. So yes, it looks USB related.

(Sometimes it hangs with some kind backtrace involving secondary CPU
startup, unfortunately useful info is off screen at that point).

Any ideas?
Pavel

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 1800bef..060ad79 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3510,6 +3510,8 @@ void pci_fixup_device(enum pci_fixup_pass pass, struct 
pci_dev *dev)
 {
struct pci_fixup *start, *end;
 
+   dev_info(&dev->dev, "PCI fixup device %p, pass %d\n", dev, pass);
+
switch (pass) {
case pci_fixup_early:
start = __start_pci_fixups_early;
@@ -3558,6 +3560,7 @@ void pci_fixup_device(enum pci_fixup_pass pass, struct 
pci_dev *dev)
return;
}
pci_do_fixups(dev, start, end);
+   dev_info(&dev->dev, "PCI fixup device %p, pass %d, done\n", dev, pass);
 }
 EXPORT_SYMBOL(pci_fixup_device);
 


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH] usb: musb: add code comment for clarification

[Gustavo A. R. Silva]

Quoting Greg KH :


On Fri, Feb 10, 2017 at 06:57:41PM -0600, Gustavo A. R. Silva wrote:

Add code comment to make it clear that the fall-through is intentional.
Read the link for more details: https://lkml.org/lkml/2017/2/9/292

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/usb/musb/musb_core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 892088f..1aec986 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1869,6 +1869,7 @@ static void musb_pm_runtime_check_session(struct musb
*musb)

return;
}
+   /* fall through */
case MUSB_QUIRK_A_DISCONNECT_19:
if (musb->quirk_retries--) {
musb_dbg(musb,


The tabs are all gone from this patch, and it's line-wrapped, making it
impossible to be applied :(

Can you please fix this and resend?



OK. I'll send it shortly.

Thanks
--
Gustavo A. R. Silva






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

Re: [PATCH v2] usb: musb: add code comment for clarification

[Greg KH]

On Tue, Feb 14, 2017 at 12:20:39PM -0600, Gustavo A. R. Silva wrote:
> Add code comment to make it clear that the fall-through is intentional.
> Read the link for more details: https://lkml.org/lkml/2017/2/9/292
> 
> Addresses-Coverity-ID: 1397608
> Signed-off-by: Gustavo A. R. Silva 
> ---
> Changes in v2:
>  Fix tabs and line-wrapping in previous patch.

Thanks for this.  Bin, I've applied this to my tree so it makes it into
4.11-rc1.

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

[PATCH v2] usb: musb: add code comment for clarification

[Gustavo A. R. Silva]

Add code comment to make it clear that the fall-through is intentional.
Read the link for more details: https://lkml.org/lkml/2017/2/9/292

Addresses-Coverity-ID: 1397608
Signed-off-by: Gustavo A. R. Silva 
---
Changes in v2:
 Fix tabs and line-wrapping in previous patch.

 drivers/usb/musb/musb_core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 892088f..d8bae6c 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1869,6 +1869,7 @@ static void musb_pm_runtime_check_session(struct musb 
*musb)
 
return;
}
+   /* fall through */
case MUSB_QUIRK_A_DISCONNECT_19:
if (musb->quirk_retries--) {
musb_dbg(musb,
-- 
2.5.0

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

Re: [PATCH v2] usb: musb: add code comment for clarification

[Bin Liu]

On Tue, Feb 14, 2017 at 10:25:11AM -0800, Greg KH wrote:
> On Tue, Feb 14, 2017 at 12:20:39PM -0600, Gustavo A. R. Silva wrote:
> > Add code comment to make it clear that the fall-through is intentional.
> > Read the link for more details: https://lkml.org/lkml/2017/2/9/292
> > 
> > Addresses-Coverity-ID: 1397608
> > Signed-off-by: Gustavo A. R. Silva 
> > ---
> > Changes in v2:
> >  Fix tabs and line-wrapping in previous patch.
> 
> Thanks for this.  Bin, I've applied this to my tree so it makes it into
> 4.11-rc1.

Thanks Greg for take it to v4.11.

Regards,
-Bin.
--
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: v4.10-rc8 (-rc6) boot regression on Intel desktop, does not boot after cold boots, boots after reboot

[Pavel Machek]

On Tue 2017-02-14 18:59:56, Pavel Machek wrote:
> Hi!
> 
> > > > > Hmm. I moved keyboard between USB ports, and now 4.10-rc6 no longer
> > > > > boots. v4.6 works ok. Let me try with keyboard unplugged... no, I
> > > > > could not get it to work. I believe v4.9 and some v4.10-rc's worked,
> > > > > but I'll have to double check.
> > > > 
> > > > But all the kernel versions worked when the keyboard was plugged into
> > > > its original USB port?
> > > 
> > > Aha. So it looks difference is probably in "where is keyboard plugged
> > > in" but in "reboot" vs. "cold boot". I did not do a cold boot in quite
> > > a while :-(.
> > > 
> > > Booting to grub, then hitting ctrl-alt-del is enough to make it work. 
> > > Ouch.
> > > 
> > > It happens with current Linus' tree.
> > 
> > v4.10-rc6-feb3 : broken
> > v4.9 : ok
> > (v4.6 : ok)
> 
> Hmm. It hangs during PCI fixups, and it hangs in v4.10-rc8, too.   
> 
> With debug patch below, I get
> 
> ...1d.7: PCI fixup... pass 2
> ...1d.7: PCI fixup... pass 3
> ...1d.7: PCI fixup... pass 3 done
> 
> ...followed by hang. So yes, it looks USB related.
> 
> (Sometimes it hangs with some kind backtrace involving secondary CPU
> startup, unfortunately useful info is off screen at that point).

Forgot to say, 1d.7 is EHCI controller.

00:1d.7 USB controller: Intel Corporation NM10/ICH7 Family USB2 EHCI
Controller (rev 01)

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: v4.10-rc8 (-rc6) boot regression on Intel desktop, does not boot after cold boots, boots after reboot

[Alan Stern]

On Tue, 14 Feb 2017, Pavel Machek wrote:

> On Tue 2017-02-14 18:59:56, Pavel Machek wrote:
> > Hi!
> > 
> > > > > > Hmm. I moved keyboard between USB ports, and now 4.10-rc6 no longer
> > > > > > boots. v4.6 works ok. Let me try with keyboard unplugged... no, I
> > > > > > could not get it to work. I believe v4.9 and some v4.10-rc's worked,
> > > > > > but I'll have to double check.
> > > > > 
> > > > > But all the kernel versions worked when the keyboard was plugged into
> > > > > its original USB port?
> > > > 
> > > > Aha. So it looks difference is probably in "where is keyboard plugged
> > > > in" but in "reboot" vs. "cold boot". I did not do a cold boot in quite
> > > > a while :-(.
> > > > 
> > > > Booting to grub, then hitting ctrl-alt-del is enough to make it work. 
> > > > Ouch.
> > > > 
> > > > It happens with current Linus' tree.
> > > 
> > > v4.10-rc6-feb3 : broken
> > > v4.9 : ok
> > > (v4.6 : ok)
> > 
> > Hmm. It hangs during PCI fixups, and it hangs in v4.10-rc8, too.   
> > 
> > With debug patch below, I get
> > 
> > ...1d.7: PCI fixup... pass 2
> > ...1d.7: PCI fixup... pass 3
> > ...1d.7: PCI fixup... pass 3 done
> > 
> > ...followed by hang. So yes, it looks USB related.
> > 
> > (Sometimes it hangs with some kind backtrace involving secondary CPU
> > startup, unfortunately useful info is off screen at that point).
> 
> Forgot to say, 1d.7 is EHCI controller.
> 
> 00:1d.7 USB controller: Intel Corporation NM10/ICH7 Family USB2 EHCI
> Controller (rev 01)

So this looks like a problem in the PCI subsystem affecting a USB
controller.

Linus is right; bisection is the best approach now that you know a
reliable trigger.

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 1/2] usb: gadget: udc: avoid use of freed pointer

[Gustavo A. R. Silva]

Hi Michal,

Quoting Michal Nazarewicz :


On Mon, Feb 13 2017, Gustavo A. R. Silva wrote:

Rewrite udc_free_dma_chain() function to avoid use of pointer after free.

Addresses-Coverity-ID: 1091172
Reviewed-by: Greg Kroah-Hartman 
Signed-off-by: Gustavo A. R. Silva 


Acked-by: Michal Nazarewicz 


---
 drivers/usb/gadget/udc/amd5536udc.c | 20 +++-
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c  
b/drivers/usb/gadget/udc/amd5536udc.c

index ea03ca7..ded97a3 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -611,21 +611,23 @@ udc_alloc_request(struct usb_ep *usbep, gfp_t gfp)
 static int udc_free_dma_chain(struct udc *dev, struct udc_request *req)
 {
int ret_val = 0;
-   struct udc_data_dma *td;
-   struct udc_data_dma *td_last = NULL;
+   struct udc_data_dma *td = req->td_data;
unsigned int i;

+   dma_addr_t addr_aux = 0x00;


Perhaps call it ‘addr_next’ or ‘next’?


+   dma_addr_t addr = (dma_addr_t)td->next;
+   td->next = 0x00;
+
DBG(dev, "free chain req = %p\n", req);

/* do not free first desc., will be done by free for request */
-   td_last = req->td_data;
-   td = phys_to_virt(td_last->next);
-
for (i = 1; i < req->chain_len; i++) {
-   pci_pool_free(dev->data_requests, td,
- (dma_addr_t)td_last->next);
-   td_last = td;
-   td = phys_to_virt(td_last->next);
+   td = phys_to_virt(addr);
+   addr_aux = (dma_addr_t)td->next;
+   td->next = 0x00;


This is unnecessary.


+   pci_pool_free(dev->data_requests, td, addr);
+   td = NULL;


Ditto.


+   addr = addr_aux;
}

return ret_val;
--
2.5.0





Thanks for your comments, I will send version 2 shortly.
--
Gustavo A. R. Silva






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

[PATCH v2 1/2] usb: gadget: udc: avoid use of freed pointer

[Gustavo A. R. Silva]

Rewrite udc_free_dma_chain() function to avoid use of pointer after free.

Addresses-Coverity-ID: 1091172
Acked-by: Michal Nazarewicz 
Reviewed-by: Greg Kroah-Hartman 
Signed-off-by: Gustavo A. R. Silva 
---
Changes in v2:
 Remove 'td->next = 0x00' inside for loop.
 Remove unnecessary pointer nullification after free.
 Rename variable addr_aux to addr_next.

 drivers/usb/gadget/udc/amd5536udc.c | 17 -
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index ea03ca7..821d088 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -611,21 +611,20 @@ udc_alloc_request(struct usb_ep *usbep, gfp_t gfp)
 static int udc_free_dma_chain(struct udc *dev, struct udc_request *req)
 {
int ret_val = 0;
-   struct udc_data_dma *td;
-   struct udc_data_dma *td_last = NULL;
+   struct udc_data_dma *td = req->td_data;
unsigned int i;
 
+   dma_addr_t addr_next = 0x00;
+   dma_addr_t addr = (dma_addr_t)td->next;
+
DBG(dev, "free chain req = %p\n", req);
 
/* do not free first desc., will be done by free for request */
-   td_last = req->td_data;
-   td = phys_to_virt(td_last->next);
-
for (i = 1; i < req->chain_len; i++) {
-   pci_pool_free(dev->data_requests, td,
- (dma_addr_t)td_last->next);
-   td_last = td;
-   td = phys_to_virt(td_last->next);
+   td = phys_to_virt(addr);
+   addr_next = (dma_addr_t)td->next;
+   pci_pool_free(dev->data_requests, td, addr);
+   addr = addr_next;
}
 
return ret_val;
-- 
2.5.0

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

[PATCH v2 2/2] usb: gadget: udc: remove unnecessary variable and update function prototype

[Gustavo A. R. Silva]

Remove unnecessary variable and update function prototype.

Acked-by: Michal Nazarewicz 
Reviewed-by: Greg Kroah-Hartman 
Signed-off-by: Gustavo A. R. Silva 
---
Changes in v2:
 None.

 drivers/usb/gadget/udc/amd5536udc.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index 821d088..67dd209 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -608,9 +608,8 @@ udc_alloc_request(struct usb_ep *usbep, gfp_t gfp)
 }
 
 /* frees pci pool descriptors of a DMA chain */
-static int udc_free_dma_chain(struct udc *dev, struct udc_request *req)
+static void udc_free_dma_chain(struct udc *dev, struct udc_request *req)
 {
-   int ret_val = 0;
struct udc_data_dma *td = req->td_data;
unsigned int i;
 
@@ -626,8 +625,6 @@ static int udc_free_dma_chain(struct udc *dev, struct 
udc_request *req)
pci_pool_free(dev->data_requests, td, addr);
addr = addr_next;
}
-
-   return ret_val;
 }
 
 /* Frees request packet, called by gadget driver */
-- 
2.5.0

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

Re: [PATCH v13 06/12] usb: xhci: use bus->sysdev for DMA configuration

[Peter Chen]

On Tue, Feb 14, 2017 at 01:58:40PM +0100, Arnd Bergmann wrote:
> On Tue, Feb 14, 2017 at 1:26 PM, Roger Quadros  wrote:
> > On 14/02/17 13:44, Arnd Bergmann wrote:
> >> On Tue, Feb 14, 2017 at 11:36 AM, Roger Quadros  wrote:
> 
> >>> Why are we using sysdev to read DT property? We should be using the
> >>> XHCI device (&pdev->dev) here, no?
> >>
> >> If I remember correctly, this is one of the cases where pdev does not
> >> have a device node attached to it because it was created by the driver
> >> of the parent device on the fly in case of dwc3. When you have a pure xhci
> >> device in DT, the two pointers are the same.
> >
> > From drivers/usb/dwc3/host.c
> >
> >> if (dwc->usb3_lpm_capable) {
> >> props[0].name = "usb3-lpm-capable";
> >> ret = platform_device_add_properties(xhci, props);
> >> if (ret) {
> >> dev_err(dwc->dev, "failed to add properties to 
> >> xHCI\n");
> >> goto err1;
> >> }
> >> }
> >
> > So it is setting the usb3-lpm-capable property into the xhci platform device
> > and we should be reading the property from there.

Why dwc3 needs another "snps,usb3_lpm_capable"? Why not using
"usb3-lpm-capable" at firmware directly?

Peter

> 
> Hmm, ideally we would only have properties on one of the two, since we
> refer to the sysdev for the properties regarding DMA and PHY among other
> things, but I guess that's not an option here, since we can't call
> platform_device_add_properties() on a dwc_pci device and have to
> use the xhci pdev instead.
> 
>  Arnd
> 
-- 

Best Regards,
Peter Chen
--
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 v13 00/12] power: add power sequence library

[Peter Chen]

On Tue, Feb 14, 2017 at 12:21:48PM +0200, Roger Quadros wrote:
> Peter,
> 
> On 11/02/17 03:27, Peter Chen wrote:
> > Hi all,
> > 
> > This is a follow-up for my last power sequence framework patch set [1].
> > According to Rob Herring and Ulf Hansson's comments[2]. The kinds of
> > power sequence instances will be added at postcore_initcall, the match
> > criteria is compatible string first, if the compatible string is not
> > matched between dts and library, it will try to use generic power sequence.
> >  
> > The host driver just needs to call of_pwrseq_on/of_pwrseq_off
> > if only one power sequence instance is needed, for more power sequences
> > are used, using of_pwrseq_on_list/of_pwrseq_off_list instead (eg, USB hub 
> > driver).
> > 
> > In future, if there are special power sequence requirements, the special
> > power sequence library can be created.
> > 
> > This patch set is tested on i.mx6 sabresx evk using a dts change, I use
> > two hot-plug devices to simulate this use case, the related binding
> > change is updated at patch [1/6], The udoo board changes were tested
> > using my last power sequence patch set.[3]
> > 
> > Except for hard-wired MMC and USB devices, I find the USB ULPI PHY also
> > need to power on itself before it can be found by ULPI bus.
> 
> Can patches 3-7 can be sent independently of the power related patches?
> 

Yes, I had planned to submit patches 3-7 for v4.11-rc1 today first, but it
seems you still have comments for patch 6, let's wait a conclusion first.

-- 

Best Regards,
Peter Chen
--
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

How to get related device pointer via DT?

[Yoshihiro Shimoda]

Hi,

I would like to get a related device pointer on usb EHCI drivers (or USB 
framework)
because related device (e.g. OHCI or UHCI, called "companion controllers") has 
to
finish resuming. I discussed this topic with Alan:
http://marc.info/?t=14865351421&r=1&w=2

In PCI bus, USB framework already has such a feature in 
drivers/usb/core/hcd-pci.c.
However, in platform devices, we don't have it for now. So, I would like to add 
it.

Then, I have 2 ideas to get the related device pointer:

A) We add a new property "companion" as usb-generic.txt and EHCI node(s) have
   such a property to bind a companion controller.
B) We assume EHCI controller binds a companion controller if some resources
   (irq or clock) are the same and it has a compatible strings as 
"generic-[uo]hci"
   for instance.

My environment is R-Car H3, and it has 3 EHCI and 3 OHCI controllers.
For example (I only wrote channel 0 of EHCI and OHCI):
ehci0: usb@ee080100 {
compatible = "generic-ehci";
reg = <0 0xee080100 0 0x100>;
interrupts = ;
clocks = <&cpg CPG_MOD 703>;
phys = <&usb2_phy0>;
phy-names = "usb";
power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
status = "disabled";
};

ohci0: usb@ee08 {
compatible = "generic-ohci";
reg = <0 0xee08 0 0x100>;
interrupts = ;
clocks = <&cpg CPG_MOD 703>;
phys = <&usb2_phy0>;
phy-names = "usb";
power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
status = "disabled";
};

If my idea A), ehci0 will have companion = <&ohci>;
If my idea B), no need to add any property.

What do you think?
Anyway, I will start to study DT programming :)

Best regards,
Yoshihiro Shimoda

--
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: How to resolve "Waited 2000ms for CONNECT" in system resuming?

[Yoshihiro Shimoda]

Hi Alan,

> From: Alan Stern
> Sent: Wednesday, February 15, 2017 2:57 AM
> 
> On Tue, 14 Feb 2017, Yoshihiro Shimoda wrote:
> 
> > Hi Alan,
> >
> > > From: Alan Stern
> > > Sent: Tuesday, February 14, 2017 1:35 AM
> > >
> > > On Mon, 13 Feb 2017, Yoshihiro Shimoda wrote:
> > >
> > > > > Hmmm.  You're using platform drivers for OHCI and EHCI, not PCI,
> > > >
> > > > Yes, I'm using platform drivers for OHCI and EHCI.
> > > >
> > > > > right?  The resume_common() routine in drivers/usb/core/hcd-pci.c is
> > > > > careful to resume things in the correct order.  It contains this code:
> > > > >
> > > > >   /*
> > > > >* Only EHCI controllers have to wait for their 
> > > > > companions.
> > > > >* No locking is needed because PCI controller drivers 
> > > > > do not
> > > > >* get unbound during system resume.
> > > > >*/
> > > > >   if (pci_dev->class == CL_EHCI && event != 
> > > > > PM_EVENT_AUTO_RESUME)
> > > > >   for_each_companion(pci_dev, hcd,
> > > > >   ehci_wait_for_companions);
> > > > >
> > > > > Probably the equivalent routine in the platform driver needs to do the
> > > > > same sort of thing.  This means it needs to know about companion
> > > > > controllers.
> > > >
> > > > Thank you very much for this information!
> > > > If I added the following code, the issue disappeared:
> > > >  - The ehci-platform.c calls 
> > > > device_enable_async_suspend(hcd->self.controller)
> > > >in ehci_platform_probe()
> > >
> > > We probably should do that in all the platform drivers anyway.
> >
> > I got it.
> >
> > > >  - [This is a dirty code, but] hcd_bus_resume() calls 
> > > > device_pm_wait_for_dev(
> > > >rhdev->bus->controller, ohci_dev)
> > > >
> > > > I will consider how to implement such a code for [eo]hci-platform 
> > > > drivers.
> > > > Especially, like ehci_{pre,post}_add() for platform drivers are needed, 
> > > > I think.
> > >
> > > The key point is that the EHCI controller must be resumed _after_ its
> > > companion controllers.  In order to do this properly, the platform
> > > driver needs to know which other devices the companions are.
> > >
> > > There's no way it can figure this out by itself; it has to be told by
> > > the platform-specific code.
> >
> > I understood it.
> > In non-DT case, if we use .id in struct platform_device, there is easy to 
> > bind
> > EHCI and companion controllers. However, in DT environment, there is 
> > difficult to
> > bind them.
> >
> > So, I have 2 ideas for DT case.
> >  A) We add a new property "companion" as usb-generic.txt and EHCI node(s) 
> > have such a property
> > to bind a companion controller.
> >  B) We assume EHCI controller binds a companion controller if some 
> > resources (irq or clock)
> > are the same and it has a compatible strings as "generic-[uo]hci" for 
> > instance.
> >
> > What do you think?
> 
> I'm not very familiar with DT programming.  It would be better to ask
> somebody else.

I got it. Also I'm not familiar with DT programming :)
So, I sent an email to devicetree maintainers as other email thread.

Thank you very much for your support!

Best regards.
Yoshihiro Shimoda

> 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 v7 1/5] x86: add simple udelay calibration

[Lu Baolu]

Hi,

On 02/14/2017 05:23 PM, Sergei Shtylyov wrote:
> Hello!
>
> On 2/14/2017 5:27 AM, Lu Baolu wrote:
>
>> Add a simple udelay calibration in x86 architecture-specific
>> boot-time initializations. This will get a workable estimate
>> for loops_per_jiffy. Hence, udelay() could be used after this
>> initialization.
>>
>> Cc: Ingo Molnar 
>> Cc: x...@kernel.org
>> Signed-off-by: Lu Baolu 
>> ---
>>  arch/x86/kernel/setup.c | 22 ++
>>  1 file changed, 22 insertions(+)
>>
>> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
>> index 4cfba94..aab7faa 100644
>> --- a/arch/x86/kernel/setup.c
>> +++ b/arch/x86/kernel/setup.c
>> @@ -835,6 +835,26 @@ dump_kernel_offset(struct notifier_block *self, 
>> unsigned long v, void *p)
>>  return 0;
>>  }
>>
>> +static void __init simple_udelay_calibration(void)
>> +{
>> +unsigned int tsc_khz, cpu_khz;
>> +unsigned long lpj;
>> +
>> +if (!boot_cpu_has(X86_FEATURE_TSC))
>> +return;
>> +
>> +cpu_khz = x86_platform.calibrate_cpu();
>> +tsc_khz = x86_platform.calibrate_tsc();
>> +
>> +tsc_khz = tsc_khz ? : cpu_khz;
>
>Why not:
>
> if (!tsc_khz)
> tsc_khz = cpu_khz;
>
>It's more clear IMHO.

Sure.

Best regards,
Lu Baolu

>
>> +if (!tsc_khz)
>> +return;
>> +
>> +lpj = tsc_khz * 1000;
>> +do_div(lpj, HZ);
>> +loops_per_jiffy = lpj;
>> +}
>> +
>>  /*
>>   * Determine if we were loaded by an EFI loader.  If so, then we have also 
>> been
>>   * passed the efi memmap, systab, etc., so we should use these data 
>> structures
> [...]
>
> MBR, Sergei
>
>

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

[PATCH 0/6] usb: xhci: several patches for xhci trace

[Lu Baolu]

Hi Mathias,

This patch set includes several patches for traces in
xhci driver. One trace class is for command. Several
trace events are defined to trace the life cycle of
any xhci command. The other trace class is for context.
Several trace events are defined to trace the change
in input/output device contexts of a USB device.

This also includes some cleanups to remove duplicated
code.

Best regards,
Lu Baolu

Lu Baolu (6):
  usb: xhci: add xhci_log_cmd trace events
  usb: xhci: enhance xhci_log_ctx trace events
  usb: xhci: remove xhci_debug_trb()
  usb: xhci: remove xhci_dbg_ctx()
  usb: xhci: fix link trb decoding
  usb: xhci: cleanup xhci_decode_trb() slightly

 drivers/usb/host/xhci-dbg.c   | 200 --
 drivers/usb/host/xhci-hub.c   |   2 +
 drivers/usb/host/xhci-ring.c  |  17 ++--
 drivers/usb/host/xhci-trace.h | 153 
 drivers/usb/host/xhci.c   |  67 +-
 drivers/usb/host/xhci.h   | 111 +--
 6 files changed, 252 insertions(+), 298 deletions(-)

-- 
2.1.4

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

[PATCH 5/6] usb: xhci: fix link trb decoding

[Lu Baolu]

xhci_decode_trb() treats a link trb in the same way as that for
an event trb. This patch fixes this by decoding the link trb
according to the spec.

Signed-off-by: Lu Baolu 
---
 drivers/usb/host/xhci.h | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index ef4a342..ff12c8a 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -2159,14 +2159,12 @@ static inline const char *xhci_decode_trb(u32 field0, 
u32 field1, u32 field2,
switch (type) {
case TRB_LINK:
sprintf(str,
-   "TRB %08x%08x status '%s' len %d slot %d ep %d type 
'%s' flags %c:%c",
-   field1, field0,
-   xhci_trb_comp_code_string(GET_COMP_CODE(field2)),
-   EVENT_TRB_LEN(field2), TRB_TO_SLOT_ID(field3),
-   /* Macro decrements 1, maybe it shouldn't?!? */
-   TRB_TO_EP_INDEX(field3) + 1,
+   "LINK %08x%08x intr %d type '%s' flags %c:%c:%c:%c",
+   field1, field0, GET_INTR_TARGET(field2),
xhci_trb_type_string(TRB_FIELD_TO_TYPE(field3)),
-   field3 & EVENT_DATA ? 'E' : 'e',
+   field3 & TRB_IOC ? 'I' : 'i',
+   field3 & TRB_CHAIN ? 'C' : 'c',
+   field3 & TRB_TC ? 'T' : 't',
field3 & TRB_CYCLE ? 'C' : 'c');
break;
case TRB_TRANSFER:
-- 
2.1.4

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

[PATCH 2/6] usb: xhci: enhance xhci_log_ctx trace events

[Lu Baolu]

XHCI driver has defined xhci_log_ctx trace events to trace
the change of an xhci input or output context. This patch
extends the trace class of xhci_log_ctx to print out the
contents of a context block in a human readable way.

This patch also adds some other xhci_log_ctx based events
where the xhci input or output context changes.

Signed-off-by: Lu Baolu 
---
 drivers/usb/host/xhci-trace.h | 63 ++-
 drivers/usb/host/xhci.c   | 23 +++-
 drivers/usb/host/xhci.h   | 60 +
 3 files changed, 121 insertions(+), 25 deletions(-)

diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index c31eeaf..8fe01b1 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -75,44 +75,71 @@ DEFINE_EVENT(xhci_log_msg, xhci_dbg_ring_expansion,
 );
 
 DECLARE_EVENT_CLASS(xhci_log_ctx,
-   TP_PROTO(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx,
-unsigned int ep_num),
-   TP_ARGS(xhci, ctx, ep_num),
+   TP_PROTO(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx),
+   TP_ARGS(xhci, ctx),
TP_STRUCT__entry(
__field(int, ctx_64)
__field(unsigned, ctx_type)
__field(dma_addr_t, ctx_dma)
__field(u8 *, ctx_va)
__field(unsigned, ctx_ep_num)
-   __field(int, slot_id)
__dynamic_array(u32, ctx_data,
((HCC_64BYTE_CONTEXT(xhci->hcc_params) + 1) * 8) *
-   ((ctx->type == XHCI_CTX_TYPE_INPUT) + ep_num + 1))
+   ((ctx->type == XHCI_CTX_TYPE_INPUT) + 
xhci_get_ep_num(xhci, ctx) + 1))
),
TP_fast_assign(
-   struct usb_device *udev;
-
-   udev = to_usb_device(xhci_to_hcd(xhci)->self.controller);
__entry->ctx_64 = HCC_64BYTE_CONTEXT(xhci->hcc_params);
__entry->ctx_type = ctx->type;
__entry->ctx_dma = ctx->dma;
__entry->ctx_va = ctx->bytes;
-   __entry->slot_id = udev->slot_id;
-   __entry->ctx_ep_num = ep_num;
+   __entry->ctx_ep_num = xhci_get_ep_num(xhci, ctx);
memcpy(__get_dynamic_array(ctx_data), ctx->bytes,
((HCC_64BYTE_CONTEXT(xhci->hcc_params) + 1) * 32) *
-   ((ctx->type == XHCI_CTX_TYPE_INPUT) + ep_num + 1));
+   ((ctx->type == XHCI_CTX_TYPE_INPUT) + 
xhci_get_ep_num(xhci, ctx) + 1));
),
-   TP_printk("ctx_64=%d, ctx_type=%u, ctx_dma=@%llx, ctx_va=@%p",
-   __entry->ctx_64, __entry->ctx_type,
-   (unsigned long long) __entry->ctx_dma, __entry->ctx_va
+   TP_printk("ctx @%p: ctx_64=%d, ctx_type=%u, ctx_dma=@%llx: %s",
+   __entry->ctx_va, __entry->ctx_64, __entry->ctx_type,
+   (unsigned long long)__entry->ctx_dma,
+   xhci_decode_ctx((u8 *)__get_dynamic_array(ctx_data),
+   __entry->ctx_type,
+   __entry->ctx_ep_num,
+   __entry->ctx_64 ? 64 : 32)
)
 );
 
-DEFINE_EVENT(xhci_log_ctx, xhci_address_ctx,
-   TP_PROTO(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx,
-unsigned int ep_num),
-   TP_ARGS(xhci, ctx, ep_num)
+DEFINE_EVENT(xhci_log_ctx, ctx_xhci_setup_device,
+   TP_PROTO(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx),
+   TP_ARGS(xhci, ctx)
+);
+
+DEFINE_EVENT(xhci_log_ctx, ctx_xhci_check_maxpacket,
+   TP_PROTO(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx),
+   TP_ARGS(xhci, ctx)
+);
+
+DEFINE_EVENT(xhci_log_ctx, ctx_xhci_check_bandwidth,
+   TP_PROTO(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx),
+   TP_ARGS(xhci, ctx)
+);
+
+DEFINE_EVENT(xhci_log_ctx, ctx_xhci_alloc_streams,
+   TP_PROTO(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx),
+   TP_ARGS(xhci, ctx)
+);
+
+DEFINE_EVENT(xhci_log_ctx, ctx_xhci_free_streams,
+   TP_PROTO(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx),
+   TP_ARGS(xhci, ctx)
+);
+
+DEFINE_EVENT(xhci_log_ctx, ctx_xhci_update_hub_device,
+   TP_PROTO(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx),
+   TP_ARGS(xhci, ctx)
+);
+
+DEFINE_EVENT(xhci_log_ctx, ctx_xhci_change_max_exit_latency,
+   TP_PROTO(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx),
+   TP_ARGS(xhci, ctx)
 );
 
 DECLARE_EVENT_CLASS(xhci_log_trb,
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index dff912e..304f38d 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1311,8 +1311,10 @@ static int xhci_check_maxpacket(struct xhci_hcd *xhci, 
unsigned int slot_id,
xhci_dbg(xhci, "Slot %d output context\n", slot_id);
xhci_dbg_ctx(xhci, out_ctx, ep_index);
 
+   trace_ctx_xhci_ch

[PATCH 3/6] usb: xhci: remove xhci_debug_trb()

[Lu Baolu]

Every XHCI TRB has already been traced by the trb trace events.
It is unnecessary to put the same message in kernel log. This
patch removes xhci_debug_trb().

Signed-off-by: Lu Baolu 
---
 drivers/usb/host/xhci-dbg.c  | 57 
 drivers/usb/host/xhci-ring.c |  4 
 drivers/usb/host/xhci.h  |  2 --
 3 files changed, 63 deletions(-)

diff --git a/drivers/usb/host/xhci-dbg.c b/drivers/usb/host/xhci-dbg.c
index 363d125..269089c 100644
--- a/drivers/usb/host/xhci-dbg.c
+++ b/drivers/usb/host/xhci-dbg.c
@@ -254,63 +254,6 @@ void xhci_print_registers(struct xhci_hcd *xhci)
xhci_print_ports(xhci);
 }
 
-void xhci_print_trb_offsets(struct xhci_hcd *xhci, union xhci_trb *trb)
-{
-   int i;
-   for (i = 0; i < 4; i++)
-   xhci_dbg(xhci, "Offset 0x%x = 0x%x\n",
-   i*4, trb->generic.field[i]);
-}
-
-/**
- * Debug a transfer request block (TRB).
- */
-void xhci_debug_trb(struct xhci_hcd *xhci, union xhci_trb *trb)
-{
-   u64 address;
-   u32 type = le32_to_cpu(trb->link.control) & TRB_TYPE_BITMASK;
-
-   switch (type) {
-   case TRB_TYPE(TRB_LINK):
-   xhci_dbg(xhci, "Link TRB:\n");
-   xhci_print_trb_offsets(xhci, trb);
-
-   address = le64_to_cpu(trb->link.segment_ptr);
-   xhci_dbg(xhci, "Next ring segment DMA address = 0x%llx\n", 
address);
-
-   xhci_dbg(xhci, "Interrupter target = 0x%x\n",
-GET_INTR_TARGET(le32_to_cpu(trb->link.intr_target)));
-   xhci_dbg(xhci, "Cycle bit = %u\n",
-le32_to_cpu(trb->link.control) & TRB_CYCLE);
-   xhci_dbg(xhci, "Toggle cycle bit = %u\n",
-le32_to_cpu(trb->link.control) & LINK_TOGGLE);
-   xhci_dbg(xhci, "No Snoop bit = %u\n",
-le32_to_cpu(trb->link.control) & TRB_NO_SNOOP);
-   break;
-   case TRB_TYPE(TRB_TRANSFER):
-   address = le64_to_cpu(trb->trans_event.buffer);
-   /*
-* FIXME: look at flags to figure out if it's an address or if
-* the data is directly in the buffer field.
-*/
-   xhci_dbg(xhci, "DMA address or buffer contents= %llu\n", 
address);
-   break;
-   case TRB_TYPE(TRB_COMPLETION):
-   address = le64_to_cpu(trb->event_cmd.cmd_trb);
-   xhci_dbg(xhci, "Command TRB pointer = %llu\n", address);
-   xhci_dbg(xhci, "Completion status = %u\n",
-GET_COMP_CODE(le32_to_cpu(trb->event_cmd.status)));
-   xhci_dbg(xhci, "Flags = 0x%x\n",
-le32_to_cpu(trb->event_cmd.flags));
-   break;
-   default:
-   xhci_dbg(xhci, "Unknown TRB with TRB type ID %u\n",
-   (unsigned int) type>>10);
-   xhci_print_trb_offsets(xhci, trb);
-   break;
-   }
-}
-
 /**
  * Debug a segment with an xHCI ring.
  *
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 4cdcd71..80cdb55 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2403,10 +2403,6 @@ static int handle_tx_event(struct xhci_hcd *xhci,
xhci_warn(xhci, "WARN Event TRB for slot %d ep 
%d with no TDs queued?\n",

TRB_TO_SLOT_ID(le32_to_cpu(event->flags)),
ep_index);
-   xhci_dbg(xhci, "Event TRB with TRB type ID 
%u\n",
-   (le32_to_cpu(event->flags) &
-TRB_TYPE_BITMASK)>>10);
-   xhci_print_trb_offsets(xhci, (union xhci_trb *) 
event);
}
if (ep->skip) {
ep->skip = false;
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index d6c4038..13ae2ba 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1918,8 +1918,6 @@ void xhci_print_ir_set(struct xhci_hcd *xhci, int 
set_num);
 void xhci_print_registers(struct xhci_hcd *xhci);
 void xhci_dbg_regs(struct xhci_hcd *xhci);
 void xhci_print_run_regs(struct xhci_hcd *xhci);
-void xhci_print_trb_offsets(struct xhci_hcd *xhci, union xhci_trb *trb);
-void xhci_debug_trb(struct xhci_hcd *xhci, union xhci_trb *trb);
 void xhci_debug_segment(struct xhci_hcd *xhci, struct xhci_segment *seg);
 void xhci_debug_ring(struct xhci_hcd *xhci, struct xhci_ring *ring);
 void xhci_dbg_erst(struct xhci_hcd *xhci, struct xhci_erst *erst);
-- 
2.1.4

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

[PATCH 6/6] usb: xhci: cleanup xhci_decode_trb() slightly

[Lu Baolu]

Replace 'TRB_FIELD_TO_TYPE(field3)' with 'type' to simplify
code.

Signed-off-by: Lu Baolu 
---
 drivers/usb/host/xhci.h | 36 ++--
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index ff12c8a..b97fb75 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -2161,7 +2161,7 @@ static inline const char *xhci_decode_trb(u32 field0, u32 
field1, u32 field2,
sprintf(str,
"LINK %08x%08x intr %d type '%s' flags %c:%c:%c:%c",
field1, field0, GET_INTR_TARGET(field2),
-   xhci_trb_type_string(TRB_FIELD_TO_TYPE(field3)),
+   xhci_trb_type_string(type),
field3 & TRB_IOC ? 'I' : 'i',
field3 & TRB_CHAIN ? 'C' : 'c',
field3 & TRB_TC ? 'T' : 't',
@@ -2182,7 +2182,7 @@ static inline const char *xhci_decode_trb(u32 field0, u32 
field1, u32 field2,
EVENT_TRB_LEN(field2), TRB_TO_SLOT_ID(field3),
/* Macro decrements 1, maybe it shouldn't?!? */
TRB_TO_EP_INDEX(field3) + 1,
-   xhci_trb_type_string(TRB_FIELD_TO_TYPE(field3)),
+   xhci_trb_type_string(type),
field3 & EVENT_DATA ? 'E' : 'e',
field3 & TRB_CYCLE ? 'C' : 'c');
 
@@ -2200,7 +2200,7 @@ static inline const char *xhci_decode_trb(u32 field0, u32 
field1, u32 field2,
(field1 & 0xff) >> 16,
TRB_LEN(field2), GET_TD_SIZE(field2),
GET_INTR_TARGET(field2),
-   xhci_trb_type_string(TRB_FIELD_TO_TYPE(field3)),
+   xhci_trb_type_string(type),
field3 & TRB_BEI ? 'B' : 'b',
field3 & TRB_IDT ? 'I' : 'i',
field3 & TRB_IOC ? 'I' : 'i',
@@ -2220,7 +2220,7 @@ static inline const char *xhci_decode_trb(u32 field0, u32 
field1, u32 field2,
"Buffer %08x%08x length %d TD size %d intr %d type '%s' 
flags %c:%c:%c:%c:%c:%c:%c:%c",
field1, field0, TRB_LEN(field2), GET_TD_SIZE(field2),
GET_INTR_TARGET(field2),
-   xhci_trb_type_string(TRB_FIELD_TO_TYPE(field3)),
+   xhci_trb_type_string(type),
field3 & TRB_BEI ? 'B' : 'b',
field3 & TRB_IDT ? 'I' : 'i',
field3 & TRB_IOC ? 'I' : 'i',
@@ -2235,21 +2235,21 @@ static inline const char *xhci_decode_trb(u32 field0, 
u32 field1, u32 field2,
case TRB_ENABLE_SLOT:
sprintf(str,
"%s: flags %c",
-   xhci_trb_type_string(TRB_FIELD_TO_TYPE(field3)),
+   xhci_trb_type_string(type),
field3 & TRB_CYCLE ? 'C' : 'c');
break;
case TRB_DISABLE_SLOT:
case TRB_NEG_BANDWIDTH:
sprintf(str,
"%s: slot %d flags %c",
-   xhci_trb_type_string(TRB_FIELD_TO_TYPE(field3)),
+   xhci_trb_type_string(type),
TRB_TO_SLOT_ID(field3),
field3 & TRB_CYCLE ? 'C' : 'c');
break;
case TRB_ADDR_DEV:
sprintf(str,
"%s: ctx %08x%08x slot %d flags %c:%c",
-   xhci_trb_type_string(TRB_FIELD_TO_TYPE(field3)),
+   xhci_trb_type_string(type),
field1, field0,
TRB_TO_SLOT_ID(field3),
field3 & TRB_BSR ? 'B' : 'b',
@@ -2258,7 +2258,7 @@ static inline const char *xhci_decode_trb(u32 field0, u32 
field1, u32 field2,
case TRB_CONFIG_EP:
sprintf(str,
"%s: ctx %08x%08x slot %d flags %c:%c",
-   xhci_trb_type_string(TRB_FIELD_TO_TYPE(field3)),
+   xhci_trb_type_string(type),
field1, field0,
TRB_TO_SLOT_ID(field3),
field3 & TRB_DC ? 'D' : 'd',
@@ -2267,7 +2267,7 @@ static inline const char *xhci_decode_trb(u32 field0, u32 
field1, u32 field2,
case TRB_EVAL_CONTEXT:
sprintf(str,
"%s: ctx %08x%08x slot %d flags %c",
-   xhci_trb_type_string(TRB_FIELD_TO_TYPE(field3)),
+   xhci_trb_type_string(type),
field1, field0,
TRB_TO_SLOT_ID(field3),
field3 & TRB_CYCLE ? 'C' : 'c');
@@ -2275,7 +2275,7 @@ static inline const char *xhci_decode_trb(u32 field0, u32 
field1, u32 field2,
case TRB_RESET_EP:
sprintf(str,
"%s: ct

[PATCH 4/6] usb: xhci: remove xhci_dbg_ctx()

[Lu Baolu]

XHCI context changes have already been traced by the trace
events. It's unnecessary to put the same message in kernel
log. This patch removes the use of xhci_dbg_ctx().

Signed-off-by: Lu Baolu 
---
 drivers/usb/host/xhci-dbg.c | 143 
 drivers/usb/host/xhci.c |  37 
 drivers/usb/host/xhci.h |   1 -
 3 files changed, 181 deletions(-)

diff --git a/drivers/usb/host/xhci-dbg.c b/drivers/usb/host/xhci-dbg.c
index 269089c..f832050 100644
--- a/drivers/usb/host/xhci-dbg.c
+++ b/drivers/usb/host/xhci-dbg.c
@@ -377,19 +377,6 @@ void xhci_dbg_cmd_ptrs(struct xhci_hcd *xhci)
upper_32_bits(val));
 }
 
-/* Print the last 32 bytes for 64-byte contexts */
-static void dbg_rsvd64(struct xhci_hcd *xhci, u64 *ctx, dma_addr_t dma)
-{
-   int i;
-   for (i = 0; i < 4; i++) {
-   xhci_dbg(xhci, "@%p (virt) @%08llx "
-"(dma) %#08llx - rsvd64[%d]\n",
-&ctx[4 + i], (unsigned long long)dma,
-ctx[4 + i], i);
-   dma += 8;
-   }
-}
-
 char *xhci_get_slot_state(struct xhci_hcd *xhci,
struct xhci_container_ctx *ctx)
 {
@@ -409,136 +396,6 @@ char *xhci_get_slot_state(struct xhci_hcd *xhci,
}
 }
 
-static void xhci_dbg_slot_ctx(struct xhci_hcd *xhci, struct xhci_container_ctx 
*ctx)
-{
-   /* Fields are 32 bits wide, DMA addresses are in bytes */
-   int field_size = 32 / 8;
-   int i;
-
-   struct xhci_slot_ctx *slot_ctx = xhci_get_slot_ctx(xhci, ctx);
-   dma_addr_t dma = ctx->dma +
-   ((unsigned long)slot_ctx - (unsigned long)ctx->bytes);
-   int csz = HCC_64BYTE_CONTEXT(xhci->hcc_params);
-
-   xhci_dbg(xhci, "Slot Context:\n");
-   xhci_dbg(xhci, "@%p (virt) @%08llx (dma) %#08x - dev_info\n",
-   &slot_ctx->dev_info,
-   (unsigned long long)dma, slot_ctx->dev_info);
-   dma += field_size;
-   xhci_dbg(xhci, "@%p (virt) @%08llx (dma) %#08x - dev_info2\n",
-   &slot_ctx->dev_info2,
-   (unsigned long long)dma, slot_ctx->dev_info2);
-   dma += field_size;
-   xhci_dbg(xhci, "@%p (virt) @%08llx (dma) %#08x - tt_info\n",
-   &slot_ctx->tt_info,
-   (unsigned long long)dma, slot_ctx->tt_info);
-   dma += field_size;
-   xhci_dbg(xhci, "@%p (virt) @%08llx (dma) %#08x - dev_state\n",
-   &slot_ctx->dev_state,
-   (unsigned long long)dma, slot_ctx->dev_state);
-   dma += field_size;
-   for (i = 0; i < 4; i++) {
-   xhci_dbg(xhci, "@%p (virt) @%08llx (dma) %#08x - rsvd[%d]\n",
-   &slot_ctx->reserved[i], (unsigned long long)dma,
-   slot_ctx->reserved[i], i);
-   dma += field_size;
-   }
-
-   if (csz)
-   dbg_rsvd64(xhci, (u64 *)slot_ctx, dma);
-}
-
-static void xhci_dbg_ep_ctx(struct xhci_hcd *xhci,
-struct xhci_container_ctx *ctx,
-unsigned int last_ep)
-{
-   int i, j;
-   int last_ep_ctx = 31;
-   /* Fields are 32 bits wide, DMA addresses are in bytes */
-   int field_size = 32 / 8;
-   int csz = HCC_64BYTE_CONTEXT(xhci->hcc_params);
-
-   if (last_ep < 31)
-   last_ep_ctx = last_ep + 1;
-   for (i = 0; i < last_ep_ctx; i++) {
-   unsigned int epaddr = xhci_get_endpoint_address(i);
-   struct xhci_ep_ctx *ep_ctx = xhci_get_ep_ctx(xhci, ctx, i);
-   dma_addr_t dma = ctx->dma +
-   ((unsigned long)ep_ctx - (unsigned long)ctx->bytes);
-
-   xhci_dbg(xhci, "%s Endpoint %02d Context (ep_index %02d):\n",
-   usb_endpoint_out(epaddr) ? "OUT" : "IN",
-   epaddr & USB_ENDPOINT_NUMBER_MASK, i);
-   xhci_dbg(xhci, "@%p (virt) @%08llx (dma) %#08x - ep_info\n",
-   &ep_ctx->ep_info,
-   (unsigned long long)dma, ep_ctx->ep_info);
-   dma += field_size;
-   xhci_dbg(xhci, "@%p (virt) @%08llx (dma) %#08x - ep_info2\n",
-   &ep_ctx->ep_info2,
-   (unsigned long long)dma, ep_ctx->ep_info2);
-   dma += field_size;
-   xhci_dbg(xhci, "@%p (virt) @%08llx (dma) %#08llx - deq\n",
-   &ep_ctx->deq,
-   (unsigned long long)dma, ep_ctx->deq);
-   dma += 2*field_size;
-   xhci_dbg(xhci, "@%p (virt) @%08llx (dma) %#08x - tx_info\n",
-   &ep_ctx->tx_info,
-   (unsigned long long)dma, ep_ctx->tx_info);
-   dma += field_size;
-   for (j = 0; j < 3; j++) {
-   xhci_dbg(xhci,

[PATCH 1/6] usb: xhci: add xhci_log_cmd trace events

[Lu Baolu]

This patch creates a new event class called xhci_log_cmd, and
defines the events used for tracing the life cycle of commands
issued for various purposes.

This info can be used, later, to print, in a human readable way,
the life cycle of an xHCI command using the trace-cmd tool and
the appropriate plugin.

Signed-off-by: Lu Baolu 
---
 drivers/usb/host/xhci-hub.c   |  2 +
 drivers/usb/host/xhci-ring.c  | 13 +--
 drivers/usb/host/xhci-trace.h | 90 +++
 drivers/usb/host/xhci.c   |  7 
 4 files changed, 108 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 3bddeaa..2c3f77f 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -411,9 +411,11 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int 
slot_id, int suspend)
}
xhci_queue_stop_endpoint(xhci, command, slot_id, i,
 suspend);
+   trace_cmd_xhci_stop_device(command);
}
}
xhci_queue_stop_endpoint(xhci, cmd, slot_id, 0, suspend);
+   trace_cmd_xhci_stop_device(cmd);
xhci_ring_cmd_db(xhci);
spin_unlock_irqrestore(&xhci->lock, flags);
 
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index d9936c7..4cdcd71 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1124,6 +1124,7 @@ static void xhci_handle_cmd_reset_ep(struct xhci_hcd 
*xhci, int slot_id,
xhci_queue_configure_endpoint(xhci, command,
xhci->devs[slot_id]->in_ctx->dma, slot_id,
false);
+   trace_cmd_xhci_handle_cmd_reset_ep(command);
xhci_ring_cmd_db(xhci);
} else {
/* Clear our internal halted state */
@@ -1231,13 +1232,13 @@ static void xhci_handle_cmd_nec_get_fw(struct xhci_hcd 
*xhci,
 static void xhci_complete_del_and_free_cmd(struct xhci_command *cmd, u32 
status)
 {
list_del(&cmd->cmd_list);
+   cmd->status = status;
+   trace_cmd_xhci_complete_del_and_free_cmd(cmd);
 
-   if (cmd->completion) {
-   cmd->status = status;
+   if (cmd->completion)
complete(cmd->completion);
-   } else {
+   else
kfree(cmd);
-   }
 }
 
 void xhci_cleanup_command_queue(struct xhci_hcd *xhci)
@@ -1268,6 +1269,7 @@ void xhci_handle_command_timeout(struct work_struct *work)
}
/* mark this command to be cancelled */
xhci->current_cmd->status = COMP_COMMAND_ABORTED;
+   trace_cmd_xhci_handle_command_timeout(xhci->current_cmd);
 
/* Make sure command ring is running before aborting it */
hw_ring_state = xhci_read_64(xhci, &xhci->op_regs->cmd_ring);
@@ -1432,6 +1434,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
}
 
 event_handled:
+   trace_cmd_handle_cmd_completion(cmd);
xhci_complete_del_and_free_cmd(cmd, cmd_comp_code);
 
inc_deq(xhci, xhci->cmd_ring);
@@ -1773,6 +1776,7 @@ static void xhci_cleanup_halted_endpoint(struct xhci_hcd 
*xhci,
ep->stopped_stream = stream_id;
 
xhci_queue_reset_ep(xhci, command, slot_id, ep_index);
+   trace_cmd_xhci_cleanup_halted_endpoint(command);
xhci_cleanup_stalled_ring(xhci, ep_index, td);
 
ep->stopped_stream = 0;
@@ -3956,6 +3960,7 @@ void xhci_queue_new_dequeue_state(struct xhci_hcd *xhci,
xhci_free_command(xhci, cmd);
return;
}
+   trace_cmd_xhci_queue_new_dequeue_state(cmd);
 
/* Stop the TD queueing code from ringing the doorbell until
 * this command completes.  The HC won't set the dequeue pointer
diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index 1ac2cdf..c31eeaf 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -285,6 +285,96 @@ DEFINE_EVENT(xhci_log_urb, xhci_urb_dequeue,
TP_ARGS(urb)
 );
 
+DECLARE_EVENT_CLASS(xhci_log_cmd,
+   TP_PROTO(struct xhci_command *cmd),
+   TP_ARGS(cmd),
+   TP_STRUCT__entry(
+   __field(struct xhci_command *, cmd)
+   __field(struct xhci_container_ctx *, in_ctx)
+   __field(union xhci_trb *, cmd_trb)
+   __field(int, slot_id)
+   __field(int, status)
+   __field(int, type)
+   ),
+   TP_fast_assign(
+   __entry->cmd = cmd;
+   __entry->in_ctx = cmd->in_ctx;
+   __entry->cmd_trb = cmd->command_trb;
+   __entry->slot_id = cmd->slot_id;
+   __entry->status = cmd->status;
+   __entry->type = 
TRB_FIELD_TO_TYPE(le32_to_cpu(cmd->command_trb->generic.field[3]))
+   ),
+   TP_printk("cmd @%p: %s: in_ctx=@%p, slot_id=%d, cmd_trb=@%p, status=%d",
+   __entry->cmd, xhci

[PATCH] usb: class: remove logically dead code

[Gustavo A. R. Silva]

Remove logically dead code.
'cntr' is always equal to zero when the following line of code is executed:
rv = cntr ? cntr : -EAGAIN;

Addresses-Coverity-ID: 113227
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/usb/class/cdc-wdm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
index 0a63695..8fda45a 100644
--- a/drivers/usb/class/cdc-wdm.c
+++ b/drivers/usb/class/cdc-wdm.c
@@ -531,7 +531,7 @@ static ssize_t wdm_read
i++;
if (file->f_flags & O_NONBLOCK) {
if (!test_bit(WDM_READ, &desc->flags)) {
-   rv = cntr ? cntr : -EAGAIN;
+   rv = -EAGAIN;
goto err;
}
rv = 0;
-- 
2.5.0

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

usb: storage: suspicious code

[Gustavo A. R. Silva]

Hello,

I ran into the following piece of code at drivers/usb/storage/jumpshot.c:305 
(linux-next), and it seems a little bit suspicious:

// read the result.  apparently the bulk write can complete
// before the jumpshot drive is finished writing.  so we loop
// here until we get a good return code
waitcount = 0;
do {
result = jumpshot_get_status(us);
if (result != USB_STOR_TRANSPORT_GOOD) {
// I have not experimented to find the smallest value.
//
msleep(50);
}
} while ((result != USB_STOR_TRANSPORT_GOOD) && (waitcount < 10));

if (result != USB_STOR_TRANSPORT_GOOD)
usb_stor_dbg(us, "Gah!  Waitcount = 10.  Bad write!?\n");

Variable 'waitcount' is never updated inside the do-while loop. So, either it 
isn't needed at all or line 316 should be modified (++waitcount < 10)

In case 'waitcount' isn't needed, lines 318 and 319 should be removed.

Can someone help me to clarify this so I can write a patch to fix this code?

Thank you
--
Gustavo A. R. Silva

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

Re: [usb-storage] usb: storage: suspicious code

[Oliver Neukum]

Am Dienstag, den 14.02.2017, 23:06 -0600 schrieb Gustavo A. R. Silva:

Hi,

> waitcount = 0;
> do {
> result = jumpshot_get_status(us);
> if (result != USB_STOR_TRANSPORT_GOOD) {
> // I have not experimented to find the smallest
> value.
> //
> msleep(50);
> }
> } while ((result != USB_STOR_TRANSPORT_GOOD) && (waitcount <
> 10));
> 
> if (result != USB_STOR_TRANSPORT_GOOD)
> usb_stor_dbg(us, "Gah!  Waitcount = 10.  Bad
> write!?\n");
> 
> Variable 'waitcount' is never updated inside the do-while loop. So,
> either it isn't needed at all or line 316 should be modified
> (++waitcount < 10)

you are correct. Waitcount needs to be incremented.

HTH
Oliver

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

Re: [usb-storage] usb: storage: suspicious code

[Gustavo A. R. Silva]

Hi Oliver,

Quoting Oliver Neukum :


Am Dienstag, den 14.02.2017, 23:06 -0600 schrieb Gustavo A. R. Silva:

Hi,


waitcount = 0;
do {
result = jumpshot_get_status(us);
if (result != USB_STOR_TRANSPORT_GOOD) {
// I have not experimented to find the smallest
value.
//
msleep(50);
}
} while ((result != USB_STOR_TRANSPORT_GOOD) && (waitcount <
10));

if (result != USB_STOR_TRANSPORT_GOOD)
usb_stor_dbg(us, "Gah!  Waitcount = 10.  Bad
write!?\n");

Variable 'waitcount' is never updated inside the do-while loop. So,
either it isn't needed at all or line 316 should be modified
(++waitcount < 10)


you are correct. Waitcount needs to be incremented.



Thanks for clarifying, I'll send a patch shortly.

--
Gustavo A. R. Silva




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

[PATCH] usb: storage: add missing pre-increment to variable

[Gustavo A. R. Silva]

Add missing pre-increment to 'waitcount' variable used in do-while loop.

Addresses-Coverity-ID: 1011631
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/usb/storage/jumpshot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/storage/jumpshot.c b/drivers/usb/storage/jumpshot.c
index 011e527..a26c4bb 100644
--- a/drivers/usb/storage/jumpshot.c
+++ b/drivers/usb/storage/jumpshot.c
@@ -313,7 +313,7 @@ static int jumpshot_write_data(struct us_data *us,
//
msleep(50); 
}
-   } while ((result != USB_STOR_TRANSPORT_GOOD) && (waitcount < 
10));
+   } while ((result != USB_STOR_TRANSPORT_GOOD) && (++waitcount < 
10));
 
if (result != USB_STOR_TRANSPORT_GOOD)
usb_stor_dbg(us, "Gah!  Waitcount = 10.  Bad 
write!?\n");
-- 
2.5.0

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

Re: UAS not working with JMS567 based disk enclosure

[Jack Coulter]

On 15/02/17 03:50, Alan Stern wrote:
> The problem is caused by the firwmware in the enclosure.  The UAS
> alternate setting was not included.  Perhaps it wasn't working
> correctly, or perhaps it was just left out by mistake.

Ah, I see. Thank you for the information, I'll try to get in contact
with the manufacturer.


Cheers,
Jack



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 1/6] usb: xhci: add xhci_log_cmd trace events

[Felipe Balbi]

Hi,

Lu Baolu  writes:
> diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
> index 1ac2cdf..c31eeaf 100644
> --- a/drivers/usb/host/xhci-trace.h
> +++ b/drivers/usb/host/xhci-trace.h
> @@ -285,6 +285,96 @@ DEFINE_EVENT(xhci_log_urb, xhci_urb_dequeue,
>   TP_ARGS(urb)
>  );
>  
> +DECLARE_EVENT_CLASS(xhci_log_cmd,
> + TP_PROTO(struct xhci_command *cmd),
> + TP_ARGS(cmd),
> + TP_STRUCT__entry(
> + __field(struct xhci_command *, cmd)
> + __field(struct xhci_container_ctx *, in_ctx)
> + __field(union xhci_trb *, cmd_trb)
> + __field(int, slot_id)
> + __field(int, status)
> + __field(int, type)
> + ),
> + TP_fast_assign(
> + __entry->cmd = cmd;
> + __entry->in_ctx = cmd->in_ctx;
> + __entry->cmd_trb = cmd->command_trb;
> + __entry->slot_id = cmd->slot_id;
> + __entry->status = cmd->status;
> + __entry->type = 
> TRB_FIELD_TO_TYPE(le32_to_cpu(cmd->command_trb->generic.field[3]))
> + ),
> + TP_printk("cmd @%p: %s: in_ctx=@%p, slot_id=%d, cmd_trb=@%p, status=%d",
> + __entry->cmd, xhci_trb_type_string(__entry->type),
> + __entry->in_ctx, __entry->slot_id, __entry->cmd_trb,
> + __entry->status
> + )
> +);

we already have a generic TRB tracer that decodes every single TRB. What
is this bringing that the previous doesn't provide? BTW, I also have
ready Slot and EP context tracers, I didn't send before because I
already had quite a large series pending for Mathias :-p

-- 
balbi


signature.asc
Description: PGP signature