Re: [PATCHv12 2/3] usb: USB Type-C connector class

[Oliver Neukum]

On Mon, 2016-11-28 at 12:11 -0800, Guenter Roeck wrote:
> On Mon, Nov 28, 2016 at 04:23:23PM +0200, Heikki Krogerus wrote:
> > On Mon, Nov 28, 2016 at 11:19:32AM +0100, Oliver Neukum wrote:

> The Type-C specification states (in 4.5.2.2.11):
> 
> "Note: if both Try.SRC and Try.SNK mechanisms are implemented, only one shall 
> be
> enabled by the port at any given time. Deciding which of these two mechanisms
> is enabled is product design-specific."
> 
> I read into this:
> - The class code can not assume that either of those mechanisms are 
> implemented.

Total agreement

> - "product-design specific" means that the product designer determines which 
> of
>   the two mechanisms (if any) is enabled. While not explicitly stated, I would
>   assume this to be set either by hardware or via devicetree / ACPI, and not
>   from user space.

I read this as spec-speak for "not our problem"

> I don't mind to have user space control; all I am asking for is to have the
> means for lower level code (which is the most likely entity to know about
> product design) to be able to inform higher layers about its initial
> preferences. We have this now, so I am happy.

So am I.

> Personally I don't really care about a module parameter; as mentioned above,
> I would expect the preference, if it needs to be selectable, to be configured
> with devicetree or ACPI properties (or by a platform driver which sets a 
> device
> property).

Well, as a distro for generic desktops and servers you will face an XHCI
on PCI and UCSI telling you that your ports can express preferences. Now
deal with it. And it must be said that such distros have over a decade
of experience in acting as a master. The slave capability is less well
developed. We'd like to be master. And if possible we'd like to avoid
a later switch of roles, so the choice should be easily made in initrd.

Regards
Oliver


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

[PATCH v4] fsl/usb: Workarourd for USB erratum-A005697

[Changming Huang]

The EHCI specification states the following in the SUSP bit description:
In the Suspend state, the port is sensitive to resume detection.
Note that the bit status does not change until the port is suspended and
that there may be a delay in suspending a port if there is a transaction
currently in progress on the USB.

However, in NXP USBDR controller, the PORTSCx[SUSP] bit changes immediately
when the application sets it and not when the port is actually suspended.

So the application must wait for at least 10 milliseconds after a port
indicates that it is suspended, to make sure this port has entered
suspended state before initiating this port resume using the Force Port
Resume bit. This bit is for NXP controller, not EHCI compatible.

Signed-off-by: Changming Huang 
Signed-off-by: Ramneek Mehresh 
---
Changes in v4:
  - release spinlock before sleeping
Changes in v3:
  - add 10ms delay in function ehci_hub_control
  - fix typos
Changes in v2:
  - move sleep out of spin-lock
  - add more comment for this workaround

 drivers/usb/host/ehci-fsl.c  |3 +++
 drivers/usb/host/ehci-hub.c  |   14 ++
 drivers/usb/host/ehci.h  |8 
 drivers/usb/host/fsl-mph-dr-of.c |2 ++
 include/linux/fsl_devices.h  |1 +
 5 files changed, 28 insertions(+)

diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c
index 9f5ffb6..91701cc 100644
--- a/drivers/usb/host/ehci-fsl.c
+++ b/drivers/usb/host/ehci-fsl.c
@@ -286,6 +286,9 @@ static int ehci_fsl_usb_setup(struct ehci_hcd *ehci)
if (pdata->has_fsl_erratum_a005275 == 1)
ehci->has_fsl_hs_errata = 1;
 
+   if (pdata->has_fsl_erratum_a005697 == 1)
+   ehci->has_fsl_susp_errata = 1;
+
if ((pdata->operating_mode == FSL_USB2_DR_HOST) ||
(pdata->operating_mode == FSL_USB2_DR_OTG))
if (ehci_fsl_setup_phy(hcd, pdata->phy_mode, 0))
diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
index 74f62d6..df169c8 100644
--- a/drivers/usb/host/ehci-hub.c
+++ b/drivers/usb/host/ehci-hub.c
@@ -310,6 +310,14 @@ static int ehci_bus_suspend (struct usb_hcd *hcd)
}
spin_unlock_irq(>lock);
 
+   if (changed && ehci_has_fsl_susp_errata(ehci))
+   /*
+* Wait for at least 10 millisecondes to ensure the controller
+* enter the suspend status before initiating a port resume
+* using the Force Port Resume bit (Not-EHCI compatible).
+*/
+   usleep_range(1, 2);
+
if ((changed && ehci->has_tdi_phy_lpm) || fs_idle_delay) {
/*
 * Wait for HCD to enter low-power mode or for the bus
@@ -1200,6 +1208,12 @@ int ehci_hub_control(
wIndex, (temp1 & HOSTPC_PHCD) ?
"succeeded" : "failed");
}
+   if (ehci_has_fsl_susp_errata(ehci)) {
+   /* 10ms for HCD enter suspend */
+   spin_unlock_irqrestore(>lock, flags);
+   usleep_range(1, 2);
+   spin_lock_irqsave(>lock, flags);
+   }
set_bit(wIndex, >suspended_ports);
break;
case USB_PORT_FEAT_POWER:
diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index 3f3b74a..a8e3617 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -219,6 +219,7 @@ struct ehci_hcd {   /* one per controller */
unsignedno_selective_suspend:1;
unsignedhas_fsl_port_bug:1; /* FreeScale */
unsignedhas_fsl_hs_errata:1;/* Freescale HS quirk */
+   unsignedhas_fsl_susp_errata:1;  /* NXP SUSP quirk */
unsignedbig_endian_mmio:1;
unsignedbig_endian_desc:1;
unsignedbig_endian_capbase:1;
@@ -710,6 +711,13 @@ struct ehci_tt {
 #endif
 
 /*
+ * Some Freescale/NXP processors have an erratum (USB A-005697)
+ * in which we need to wait for 10ms for bus to enter suspend mode
+ * after setting SUSP bit.
+ */
+#define ehci_has_fsl_susp_errata(e)((e)->has_fsl_susp_errata)
+
+/*
  * While most USB host controllers implement their registers in
  * little-endian format, a minority (celleb companion chip) implement
  * them in big endian format.
diff --git a/drivers/usb/host/fsl-mph-dr-of.c b/drivers/usb/host/fsl-mph-dr-of.c
index f07ccb2..e90ddb5 100644
--- a/drivers/usb/host/fsl-mph-dr-of.c
+++ b/drivers/usb/host/fsl-mph-dr-of.c
@@ -226,6 +226,8 @@ static int fsl_usb2_mph_dr_of_probe(struct platform_device 
*ofdev)
of_property_read_bool(np, "fsl,usb-erratum-a007792");
pdata->has_fsl_erratum_a005275 =
   

Re: [PATCH v2 1/4] usb: host: xhci: dynamically allocate devs array

[Baolin Wang]

On 28 November 2016 at 19:00, Felipe Balbi  wrote:
> Instead of always defaulting to a 256-entry array,
> we can dynamically allocate devs based on what HW
> tells us it supports.
>
> Note that we can't, yet, purge MAX_HC_SLOTS
> completely because of struct
> xhci_device_context_array reliance on it.
>
> Signed-off-by: Felipe Balbi 
> ---
>  drivers/usb/host/xhci-hub.c  |  2 +-
>  drivers/usb/host/xhci-mem.c  |  4 ++--
>  drivers/usb/host/xhci-ring.c |  2 +-
>  drivers/usb/host/xhci.c  | 19 +++
>  drivers/usb/host/xhci.h  |  2 +-
>  5 files changed, 20 insertions(+), 9 deletions(-)
>
> Changes since v1:
> - accounted for invalid slot 0 which driver assumes to exist.

Tested on my dwc3 platform, it can work well as host role.
Tested-by: Baolin Wang 

-- 
Baolin.wang
Best Regards
--
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] b: re-queue tx dma request on herror

[Bin Liu]

Hi,

On Wed, Nov 16, 2016 at 10:42:03AM +0300, Max Uvarov wrote:
> Some times dma transfer to usb endpoint fails:
> 
> [ 78.378283] musb-hdrc musb-hdrc.1.auto: Start TX10 dma
> [ 78.410763] musb-hdrc musb-hdrc.1.auto: OUT/TX10 end, csr 3400, dma
> [ 78.410896] musb-hdrc musb-hdrc.1.auto: complete dc01eb00 
> usb_api_blocking_completion+0x0/0x24 [usbcore] (0), dev4 ep1out, 10/10
> [ 78.411181] musb-hdrc musb-hdrc.1.auto: qh dc01ed00 periodic slot 10
> [ 78.411205] musb-hdrc musb-hdrc.1.auto: qh dc01ed00 urb dc01eb00 dev4 
> ep1out-intr, hw_ep 10, dd624d00/10
> [ 78.411223] musb-hdrc musb-hdrc.1.auto: --> hw10 urb dc01eb00 spd2 dev4 
> ep1out h_addr83 h_port02 bytes 10
> [ 78.411244] musb-hdrc musb-hdrc.1.auto: check whether there's still time for 
> periodic Tx
> [ 78.411256] musb-hdrc musb-hdrc.1.auto: Start TX10 dma
> [ 78.443762] musb-hdrc musb-hdrc.1.auto: OUT/TX10 end, csr 3500, dma

What kernel version is this? Log format looks old.

> success transmition:
> 
> [ 78.443889] musb-hdrc musb-hdrc.1.auto: complete dc01eb00 
> usb_api_blocking_completion+0x0/0x24 [usbcore] (0), dev4 ep1out, 10/10
> [ 78.444170] musb-hdrc musb-hdrc.1.auto: qh dc01ed00 periodic slot 10
> [ 78.444195] musb-hdrc musb-hdrc.1.auto: qh dc01ed00 urb dc01eb00 dev4 
> ep1out-intr, hw_ep 10, dd624d00/10
> [ 78.444213] musb-hdrc musb-hdrc.1.auto: --> hw10 urb dc01eb00 spd2 dev4 
> ep1out h_addr83 h_port02 bytes 10
> [ 78.444232] musb-hdrc musb-hdrc.1.auto: check whether there's still time for 
> periodic Tx
> [ 78.444245] musb-hdrc musb-hdrc.1.auto: Start TX10 dma
> [ 78.540761] musb-hdrc musb-hdrc.1.auto: OUT/TX10 end, csr 3504, dma
> 
> failed transmission:
> 
> [ 78.540780] musb-hdrc musb-hdrc.1.auto: TX 3strikes on ep=10 set ETIMEDOUT
> [ 78.540897] musb-hdrc musb-hdrc.1.auto: complete dc01eb00 
> usb_api_blocking_completion+0x0/0x24 [usbcore] (-110), dev4 ep1out, 10/10
> [ 78.540945] musb-hdrc musb-hdrc.1.auto: extra TX10 ready, csr 2500
> [ 78.540989] usb 2-1.1.2: urb wait ok but retval -110
> 
> Use to reproduce is writes to /dev/hidraw0 device which end up with early
> unexpected timeout and -110 errno set.
> 
> Code sets timeout to 5 seconds
> vfs_write()->hidraw_write()->hidraw_send_report()->usbhid_output_report()
>ret = usb_interrupt_msg(dev, usbhid->urbout->pipe,
>buf, count, _length,
>USB_CTRL_SET_TIMEOUT);
> where is set to 5 second:

The two timeout here are completely different - ETIMEOUT on 3strikes vs.
USB_CTRL_SET_TIMEOUT.

USB_CTRL_SET_TIMEOUT only takes effects when the usb device NAK the
interrupt transaction for 5 seconds.

But when MUSB reports 3strikes ETIMEOUT, it means the usb device doesn't
respond, no handshake packet at all. The host tried TX 3 times, all
received no response handshake, so ETIMEOUT.
 
> when it wents to usb_start_wait_urb() where waits for completition:
>retval = usb_submit_urb(urb, GFP_NOIO);
> 
>expire = timeout ? msecs_to_jiffies(timeout) : MAX_SCHEDULE_TIMEOUT;
>if (!wait_for_completion_timeout(, expire)) {
> 
> So user space application expects that write will be done in 5 seconds or
> error will happen. But musb driver exists this logic on first dma error

This ETIMEOUT is not a dma error, but something wrong on the wire which
causes the usb device is unable to respond, typically not received the
TX packet at all.

> without trying to retransmit current urb. This patch adds current request

The host does trying to retransmit the TX packet 3 times, before reports
ETIMEOUT.

> to the end of list, destroys current dma transfer and renew transmission.
> In that case this urb transmitted in next cycle and not failing with error
> before timeout.

NAK.

It is up to the application to retry the transaction, 

Regards,
-Bin.

> 
> Signed-off-by: Max Uvarov 
> ---
>  drivers/usb/musb/musb_host.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
> index 53bc4ce..e44ae95 100644
> --- a/drivers/usb/musb/musb_host.c
> +++ b/drivers/usb/musb/musb_host.c
> @@ -1293,11 +1293,11 @@ void musb_host_tx(struct musb *musb, u8 epnum)
>   status = -EPIPE;
>  
>   } else if (tx_csr & MUSB_TXCSR_H_ERROR) {
> - /* (NON-ISO) dma was disabled, fifo flushed */
>   musb_dbg(musb, "TX 3strikes on ep=%d", epnum);
> -
> - status = -ETIMEDOUT;
> -
> + if (dma)
> + musb_bulk_nak_timeout(musb, hw_ep, 0);
> + else
> + status = -ETIMEDOUT;
>   } else if (tx_csr & MUSB_TXCSR_H_NAKTIMEOUT) {
>   if (USB_ENDPOINT_XFER_BULK == qh->type && qh->mux == 1
>   && !list_is_singular(>out_bulk)) {
> -- 
> 1.9.1
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  

Re: [PATCH v2 2/4] usb: dwc2: Add binding for AHB burst

[John Youn]

On 11/22/2016 12:51 PM, Christian Lamparter wrote:
> On Monday, November 21, 2016 7:32:30 PM CET John Youn wrote:
>> On 11/21/2016 1:10 PM, Christian Lamparter wrote:
>>> On Monday, November 21, 2016 12:16:31 PM CET John Youn wrote:
 On 11/18/2016 12:18 PM, Christian Lamparter wrote:
> On Friday, November 18, 2016 8:16:08 AM CET Rob Herring wrote:
>> Also, perhaps you should allow that the compatible string can define the 
>> default.
>>
> I hoped you would say that :).
>
> I've attached a patch (on top of John Youn changes) [...]
> ---
> Subject: [PATCH] usb: dwc2: add a default ahb-burst setting for 
> amcc,dwc-otg
> [...]
> @@ -1097,6 +1097,22 @@ static const char *const ahb_bursts[] = {
> +/* [...] */
> +static const struct of_device_id dwc2_compat_ahb_bursts[] = {
> + {
> + .compatible = "amcc,dwc-otg",
> + .data = (void *) GAHBCFG_HBSTLEN_INCR16,
> + },
> +};
>> [...]
>> @@ -1107,6 +1123,12 @@ static int dwc2_get_property_ahb_burst(struct 
>> dwc2_hsotg *hsotg)
>   ret = device_property_read_string(hsotg->dev, "snps,ahb-burst", );
>   if (ret < 0) {
> + const struct of_device_id *match;
> +
> + match = of_match_node(dwc2_compat_ahb_bursts, node);
> + if (match)
> + ret = (int)match->data;
> +
>> [...]
 I'd prefer if you use the binding which requires no extra code in
 dwc2.
>>> I'm fine with either option. However it think that this would require
>>> that either Mark or Rob would allow an exception to the "keep existing
>>> dts the way they are) and ack the following change to the canyonlands.dts. 
>>
>> I don't know about that. Under what circumstance can the dts change?
> As far as I know, the justification for not changing the DTS is that a
> compiled DTB might be stored in an read-only ROM on a board. So it would
> be impossible to update it. Hence, the driver have work with the existing
> (and sometimes buggy or incomplete) information to stay compatible.
> 
> (Note: Thankfully, the canyonlands dtb is stored in flash, it's possible
> to update it. But it is an extra step that's not done automatically
> with make install). 
> 
>> The canyonlands dts was binding to an external vendor driver. So it
>> wasn't documented nor expected to work with dwc2 until your recent
>> patch adding the compatible string.
> 
> Oh, no that's not what happend. Let me explain why there was no "external 
> vendor driver": AMCC/APM were planing to upstream their hole platform. And
> in fact, the devs tried very hard to include their driver back in 2011 [0].
> But this driver was denied inclusion back then due to:
> 
> "[...]
> I would also like to point out that the same Synopsys USB controller
> is used in a number of other SoCs (especially ARM chips), and
> supported by other drivers, some of these even in mainline.
> 
> See http://thread.gmane.org/gmane.linux.usb.general/61714/focus=62139
> for a related thread.
> 
> Instead of trying to add a completely new driver to mainline (and one
> which has been repeatedly been rejected), I vote for focusing on the
> existing driver code that is already in mainline, and testing and
> improving this so we can use a single implementation of this driver
> code for all SoCs that use the same IP block." [1]
> 
> Of course: The listed link goes the "USB Host driver for i.MX28" driver.
> And this is an ehci-hcd like driver... Which is as you are well aware not
> that similar to the dwc2 OTG. And as far as I can tell: AMCC abandoned
> the patch series right there. 
> 
> Note: AMCC did however succeed in pushing your employer's Synopsys
> DesignWare SATA and DMA drivers to the kernel back then. And I'm happy
> to report that both drivers are still around and working fine for the 460EX 
> (sata_dwc_460ex.c[2] and the DW AHB DMA [3]). (The drivers also work for
> different platforms than the original PPC. I know that because I helped
> Andy Shevchenko with testing and pushing some fixes to it when he was
> adding support for the Intel Quark SoC, which uses the DWC SATA and DMA).

Ok thanks for clearing that up. I understand.

For now we can just set the property to "INCR16" based on the
compatible string. Perhaps in the future do this from a glue-layer
driver which binds to all compatible strings other than "snps,dwc2".

I won't be able to do anything with this until next week though.

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

RE: [PATCH v3] fsl/usb: Workarourd for USB erratum-A005697

[Jerry Huang]

Thanks a lot, Alan.
That is my fault, will fix it and send one new version.

Best Regards
Jerry Huang


-Original Message-
From: Alan Stern [mailto:st...@rowland.harvard.edu] 
Sent: Monday, November 28, 2016 11:58 PM
To: Jerry Huang 
Cc: gre...@linuxfoundation.org; Ramneek Mehresh ; 
julia.law...@lip6.fr; Sriram Dash ; 
linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org
Subject: Re: [PATCH v3] fsl/usb: Workarourd for USB erratum-A005697

On Mon, 28 Nov 2016, Changming Huang wrote:

> The EHCI specification states the following in the SUSP bit description:
> In the Suspend state, the port is sensitive to resume detection.
> Note that the bit status does not change until the port is suspended 
> and that there may be a delay in suspending a port if there is a 
> transaction currently in progress on the USB.
> 
> However, in NXP USBDR controller, the PORTSCx[SUSP] bit changes 
> immediately when the application sets it and not when the port is actually 
> suspended.
> 
> So the application must wait for at least 10 milliseconds after a port 
> indicates that it is suspended, to make sure this port has entered 
> suspended state before initiating this port resume using the Force 
> Port Resume bit. This bit is for NXP controller, not EHCI compatible.
> 
> Signed-off-by: Changming Huang 
> Signed-off-by: Ramneek Mehresh 
> ---
> Changes in v3:
>   - add 10ms delay in function ehci_hub_control
>   - fix typos
> Changes in v2:
>   - move sleep out of spin-lock
>   - add more comment for this workaround

> --- a/drivers/usb/host/ehci-hub.c
> +++ b/drivers/usb/host/ehci-hub.c
> @@ -310,6 +310,14 @@ static int ehci_bus_suspend (struct usb_hcd *hcd)
>   }
>   spin_unlock_irq(>lock);
>  
> + if (changed && ehci_has_fsl_susp_errata(ehci))
> + /*
> +  * Wait for at least 10 millisecondes to ensure the controller
> +  * enter the suspend status before initiating a port resume
> +  * using the Force Port Resume bit (Not-EHCI compatible).
> +  */
> + usleep_range(1, 2);
> +
>   if ((changed && ehci->has_tdi_phy_lpm) || fs_idle_delay) {
>   /*
>* Wait for HCD to enter low-power mode or for the bus @@ 
> -1200,6 
> +1208,9 @@ int ehci_hub_control(
>   wIndex, (temp1 & HOSTPC_PHCD) ?
>   "succeeded" : "failed");
>   }
> + if (ehci_has_fsl_susp_errata(ehci))
> + /* 10ms for HCD enter suspend */
> + usleep_range(1, 2);
>   set_bit(wIndex, >suspended_ports);
>   break;

Just like before, you have to release the spinlock before sleeping.  
Look at the code 10 lines above this.

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 V12 1/1] usb:serial: Add Fintek F81532/534 driver

[Ji-Ze Hong (Peter Hong)]

Hi Johan,

Johan Hovold 於 2016/11/24 下午 11:04 寫道:

On Mon, Nov 14, 2016 at 01:37:59PM +0800, Ji-Ze Hong (Peter Hong) wrote:

This driver is for Fintek F81532/F81534 USB to Serial Ports IC.



+   }
+
+   if (size_bulk_out != F81534_WRITE_BUFFER_SIZE ||
+   size_bulk_in != F81534_MAX_RECEIVE_BLOCK_SIZE) {
+   dev_err(dev, "%s: endpoints packet size not matched\n",
+   __func__);


Similarly: "unsupported endpoint max packet size\n".

But just to be clear: You do want to bail out if connected at full
speed? You could also ask usb-serial core to allocate large enough
buffers (e.g. by setting the bulk_out_size driver field) and the host
controller will handle partitioning.


Yes, this product cannot run on USB full-speed currently due to the
bulk-out endpoint design. It need use 512 bytes packet size to transmit
4 ports TX data, but USB Full-speed only provide 64 byte. The F81534
will treat the all bulk-out data as port 0 although the host will spilt
512byte to 8 x 64bytes packets.

--
With Best Regards,
Peter Hong
--
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: xhci: Remove unuseful 'return' and 'break' statement

[Baolin Wang]

On 28 November 2016 at 23:14, Mathias Nyman
 wrote:
> On 28.11.2016 09:41, Baolin Wang wrote:
>>
>> On 28 November 2016 at 15:21, Greg KH  wrote:
>>>
>>> On Mon, Nov 28, 2016 at 02:29:25PM +0800, Baolin Wang wrote:

 Hi Mathias,

 On 24 November 2016 at 19:16, Baolin Wang 
 wrote:
>
> Since these 'return' statements are not generally useful in void
> function, remove them. Also remove one unuseful 'break' statement
> in xhci_setup_addressable_virt_dev() function.
>
> Signed-off-by: Baolin Wang 
> ---
> Changes since v1:
>   - Add description of removing 'break' statement in commitlog.
> ---


 Could you apply this patch if there are no other comments? Thanks.
>>>
>>>
>>> Less than a week response for a simple cleanup patch?  Why the rush and
>>> pressure?  Relax, this really isn't an important patch...
>>
>>
>> I am sorry for the pressure, I just thought it is one simple cleanup
>> patch. It is okay for me to wait for.
>>
>
> Looks ok.
>
> If it applies I'll send it forward to usb-next after 4.10-rc1,
> It should end up in 4.11

Thanks.

-- 
Baolin.wang
Best Regards
--
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 4.9] New led trigger usbport gets the kernel to panic

[Rafal Milecki]

Hi Ralph,

On 11/21/2016 09:40 AM, Ralph Sennhauser wrote:

I tried your new usbport trigger in Linux 4.9 with little luck as can be seen in
the following output of the serial console.


I'm really happy my trigger got some attention. I'm sorry it crashes for you,
I never experienced any crash so far. I also got few ppl using it as part of
LEDE project but no reports from them neither.

Also sorry for the late reply, my private life took all my time previous week.



  root@wrt1900acs:/# cd /sys/class/leds/pca963x\:shelby\:white\:usb2/
  
root@wrt1900acs:/sys/devices/platform/soc/soc:internal-regs/f1011000.i2c/i2c-0/0-0068/leds/pca963x:shelby:white:usb2#
 echo usbport > trigger
  
root@wrt1900acs:/sys/devices/platform/soc/soc:internal-regs/f1011000.i2c/i2c-0/0-0068/leds/pca963x:shelby:white:usb2#
 echo 1 > ports/usb2-port1
  [ 1461.761528] Unable to handle kernel NULL pointer dereference at virtual 
address 
  [ 1461.769734] pgd = de33c000
  [ 1461.772454] [] *pgd=1cb2a831, *pte=, *ppte=
  [ 1461.778791] Internal error: Oops: 8007 [#1] SMP ARM
  [ 1461.784036] Modules linked in: iptable_nat nft_chain_nat_ipv4 
nf_tables_inet
  nf_nat_ipv4 nf_conntrack_ipv6 nf_conntrack_ipv4 ipt_REJECT ipt_MASQUERADE
  xt_time xt_tcpudp xt_state xt_nat xt_multiport xt_mark xt_mac xt_limit
  xt_conntrack xt_comment xt_TCP MSS xt_REDIRECT xt_LOG xt_CT nft_set_rbtree
  nft_set_hash nft_reject_inet nft_reject nft_redir_ipv4 nft_redir nft_nat
  nft_meta nft_masq_ipv4 nft_masq nft_log nft_limit nft_hash nft_exthdr nft_ct
  nft_counter nft_chain_route_ipv6 nft_chain_route_ipv4 nf_tabl es_ipv6
  nf_tables_ipv4 nf_tables nf_reject_ipv4 nf_nat_redirect nf_nat_masquerade_ipv4
  nf_nat nf_log_ipv4 nf_defrag_ipv6 nf_defrag_ipv4 nf_conntrack_netlink
  nf_conntrack macvlan libcrc32c iptable_raw iptable_mangle iptable_filter
  ip_tables act_skbedit act _mirred em_u32 cls_u32 cls_tcindex cls_flow 
cls_route
  cls_fw sch_hfsc sch_ingress mwlwifi(O) mac80211 cfg80211 ledtrig_netdev(O)
  ip_set_list_set ip_set_hash_netiface ip_set_hash_netport ip_set_hash_netnet
  ip_set_hash_net ip_set_hash_netportnet ip_set_hash _mac ip_set_hash_ipportnet
  ip_set_hash_ipportip ip_set_hash_ipport ip_set_hash_ipmark ip_set_hash_ip
  ip_set_bitmap_port ip_set_bitmap_ipmac ip_set_bitmap_ip ip_set nfnetlink
  ip6t_REJECT nf_reject_ipv6 nf_log_ipv6 nf_log_common ip6table_raw
  ip6table_mangle ip6table_filter ip6_tables x_tables ifb sit tunnel4 ip_tunnel
  vfat fat nls_utf8 nls_iso8859_1 nls_cp850 nls_cp437 rfkill input_core
  sha256_generic seqiv jitterentropy_rng drbg hmac ghash_generic gf128mul gcm 
ctr
  ccm ledtrig_usbport ext4 jbd2 mbcache btrf s xor raid6_pq crc32c_generic ubifs
  ubi
  [ 1461.922401] CPU: 0 PID: 759 Comm: ash Tainted: G   O4.9.0-rc6 
#2
  [ 1461.929476] Hardware name: Marvell Armada 380/385 (Device Tree)
  [ 1461.935418] task: df621080 task.stack: dee0e000
  [ 1461.939966] PC is at 0x0
  [ 1461.942510] LR is at usbport_trig_port_store+0x8c/0xd8 [ledtrig_usbport]
  [ 1461.949238] pc : [<>]lr : []psr: 6013
  [ 1461.949238] sp : dee0fe50  ip : dee0fe10  fp : dee0fe6c
  [ 1461.960761] r10:   r9 :   r8 : 
  [ 1461.966006] r7 : dcabc7c0  r6 : df4164ec  r5 : 0002  r4 : dcabc040
  [ 1461.972558] r3 :   r2 : 001a  r1 :   r0 : df4164ec
  [ 1461.979112] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment 
none
  [ 1461.986275] Control: 10c5387d  Table: 1e33c04a  DAC: 0051
  [ 1461.992043] Process ash (pid: 759, stack limit = 0xdee0e210)
  [ 1461.997724] Stack: (0xdee0fe50 to 0xdee1)
  [ 1462.002097] fe40: bf228164 0002 
dee0ff80 dcabc7c0
  [ 1462.010309] fe60: dee0fe84 dee0fe70 c023f758 bf228170 c023f738 0002 
dee0fe9c dee0fe88
  [ 1462.018520] fe80: c0169310 c023f744 df4a1000 0002 dee0fedc dee0fea0 
c0168ab8 c01692d4
  [ 1462.026731] fea0:   df747c10 df4a100c d4aca1c9 df727a80 
c01689c0 dee0ff80
  [ 1462.034943] fec0: 0002 c000fc24 dee0e000  dee0ff4c dee0fee0 
c0100c78 c01689cc
  [ 1462.043153] fee0:  dee0fee8 dee0ff14 dee0fef8 dee0ff14 dee0ff00 
c0060668 c04640a0
  [ 1462.051365] ff00: dee0ff50 df621350 dee0ff4c dee0ff18 c00298f8 c0060630 
fff6 dee0ff68
  [ 1462.059576] ff20: c00ff23c 0007 0002 df727a80 b6f092d0 dee0ff80 
c000fc24 dee0e000
  [ 1462.067786] ff40: dee0ff7c dee0ff50 c0101ae4 c0100c50 0003 0007 
df727a80 df727a80
  [ 1462.075997] ff60: b6f092d0 0002 c000fc24 dee0e000 dee0ffa4 dee0ff80 
c01028dc c0101a44
  [ 1462.084208] ff80:      0004 
 dee0ffa8
  [ 1462.092419] ffa0: c000fa60 c010289c   0001 b6f092d0 
0002 
  [ 1462.100630] ffc0:    0004 b6f092d0 0020 
00059ec0 00059ea0
  [ 1462.108841] ffe0: beb744f8 beb744e4 b6ee1904 b6ee0dc8 6010 0001 
 
  [ 1462.117049] 

Re: [PATCH v2 2/3] Revert "usb: dwc2: gadget: fix TX FIFO size and address initialization"

[John Youn]

On 11/24/2016 1:15 PM, Stefan Wahren wrote:
> Hi John,
> 
>> John Youn  hat am 18. Oktober 2016 um 02:36
>> geschrieben:
>>
>>
>> This reverts commit aa381a7259c3 ("usb: dwc2: gadget: fix TX FIFO size
>> and address initialization").
>>
>> The original commit removed the FIFO size programming per endpoint. The
>> DPTXFSIZn register is also used for DIEPTXFn and the SIZE field is r/w
>> in dedicated fifo mode. So it isn't appropriate to simply remove this
>> initialization as it might break existing behavior.
>>
>> Also, some cores might not have enough fifo space to handle the
>> programming method used in the reverted patch, resulting in fifo
>> initialization failure.
> 
> since the original patch is still necessary for bcm2835 gadget support i want 
> to
> know if there is an alternative solution?
> 
> Stefan
> 

Hi Stefan,

We are working on a solution to have the driver initialize the FIFOs
more intelligently as the default behavior so it should never fail.

Regards,
John
--
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: dwc3: pci: Add "linux,sysdev_is_parent" property

[John Youn]

Calling platform_device_add_properties() replaces existing properties so
the "linux,sysdev_is_parent" property doesn't get set. Add this property
to each platform.

Fixes: d64ff406e51e ("usb: dwc3: use bus->sysdev for DMA configuration")
Signed-off-by: John Youn 
---

Hi Felipe,

Can you queue for 4.10 fixes? The original commit breaks HAPS.

v2:
* Resend with fixed email format issues

Regards,
John


 drivers/usb/dwc3/dwc3-pci.c | 13 +++--
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
index 2b73339..d13f771 100644
--- a/drivers/usb/dwc3/dwc3-pci.c
+++ b/drivers/usb/dwc3/dwc3-pci.c
@@ -73,16 +73,6 @@ static int dwc3_pci_quirks(struct dwc3_pci *dwc)
 {
struct platform_device  *dwc3 = dwc->dwc3;
struct pci_dev  *pdev = dwc->pci;
-   int ret;
-
-   struct property_entry sysdev_property[] = {
-   PROPERTY_ENTRY_BOOL("linux,sysdev_is_parent"),
-   { },
-   };
-
-   ret = platform_device_add_properties(dwc3, sysdev_property);
-   if (ret)
-   return ret;
 
if (pdev->vendor == PCI_VENDOR_ID_AMD &&
pdev->device == PCI_DEVICE_ID_AMD_NL_USB) {
@@ -105,6 +95,7 @@ static int dwc3_pci_quirks(struct dwc3_pci *dwc)
PROPERTY_ENTRY_BOOL("snps,disable_scramble_quirk"),
PROPERTY_ENTRY_BOOL("snps,dis_u3_susphy_quirk"),
PROPERTY_ENTRY_BOOL("snps,dis_u2_susphy_quirk"),
+   PROPERTY_ENTRY_BOOL("linux,sysdev_is_parent"),
{ },
};
 
@@ -116,6 +107,7 @@ static int dwc3_pci_quirks(struct dwc3_pci *dwc)
 
struct property_entry properties[] = {
PROPERTY_ENTRY_STRING("dr-mode", "peripheral"),
+   PROPERTY_ENTRY_BOOL("linux,sysdev_is_parent"),
{ }
};
 
@@ -167,6 +159,7 @@ static int dwc3_pci_quirks(struct dwc3_pci *dwc)
PROPERTY_ENTRY_BOOL("snps,usb3_lpm_capable"),
PROPERTY_ENTRY_BOOL("snps,has-lpm-erratum"),
PROPERTY_ENTRY_BOOL("snps,dis_enblslpm_quirk"),
+   PROPERTY_ENTRY_BOOL("linux,sysdev_is_parent"),
{ },
};
 
-- 
2.10.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

[no subject]

[John Youn]

>From 1ea511f4fd3ed082beee53130625570a6b6b8d82 Mon Sep 17 00:00:00 2001
Message-Id: 
<1ea511f4fd3ed082beee53130625570a6b6b8d82.1480378291.git.johny...@synopsys.com>
Subject: [PATCH] usb: dwc3: pci: Add "linux,sysdev_is_parent" property
To: Felipe Balbi , linux-usb@vger.kernel.org
Cc: John Youn , Sriram Dash , 
Baolin Wang , Arnd Bergmann 

Calling platform_device_add_properties() replaces existing properties so
the "linux,sysdev_is_parent" property doesn't get set. Add this property
to each platform.

Fixes: d64ff406e51e ("usb: dwc3: use bus->sysdev for DMA configuration")
Signed-off-by: John Youn 
---

Hi Felipe,

Can you queue for 4.10 fixes? The original commit breaks HAPS.

Regards,
John



 drivers/usb/dwc3/dwc3-pci.c | 13 +++--
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
index 2b73339..d13f771 100644
--- a/drivers/usb/dwc3/dwc3-pci.c
+++ b/drivers/usb/dwc3/dwc3-pci.c
@@ -73,16 +73,6 @@ static int dwc3_pci_quirks(struct dwc3_pci *dwc)
 {
struct platform_device  *dwc3 = dwc->dwc3;
struct pci_dev  *pdev = dwc->pci;
-   int ret;
-
-   struct property_entry sysdev_property[] = {
-   PROPERTY_ENTRY_BOOL("linux,sysdev_is_parent"),
-   { },
-   };
-
-   ret = platform_device_add_properties(dwc3, sysdev_property);
-   if (ret)
-   return ret;
 
if (pdev->vendor == PCI_VENDOR_ID_AMD &&
pdev->device == PCI_DEVICE_ID_AMD_NL_USB) {
@@ -105,6 +95,7 @@ static int dwc3_pci_quirks(struct dwc3_pci *dwc)
PROPERTY_ENTRY_BOOL("snps,disable_scramble_quirk"),
PROPERTY_ENTRY_BOOL("snps,dis_u3_susphy_quirk"),
PROPERTY_ENTRY_BOOL("snps,dis_u2_susphy_quirk"),
+   PROPERTY_ENTRY_BOOL("linux,sysdev_is_parent"),
{ },
};
 
@@ -116,6 +107,7 @@ static int dwc3_pci_quirks(struct dwc3_pci *dwc)
 
struct property_entry properties[] = {
PROPERTY_ENTRY_STRING("dr-mode", "peripheral"),
+   PROPERTY_ENTRY_BOOL("linux,sysdev_is_parent"),
{ }
};
 
@@ -167,6 +159,7 @@ static int dwc3_pci_quirks(struct dwc3_pci *dwc)
PROPERTY_ENTRY_BOOL("snps,usb3_lpm_capable"),
PROPERTY_ENTRY_BOOL("snps,has-lpm-erratum"),
PROPERTY_ENTRY_BOOL("snps,dis_enblslpm_quirk"),
+   PROPERTY_ENTRY_BOOL("linux,sysdev_is_parent"),
{ },
};
 
-- 
2.10.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: [RFT PATCH 1/1] xhci: free xhci virtual devices with leaf nodes first

[Guenter Roeck]

On Wed, Nov 23, 2016 at 02:24:27PM +0200, Mathias Nyman wrote:
> the tt_info provided by a HS hub might be in use to by a child device
> Make sure we free the devices in the correct order.
> 
> This is needed in special cases such as when xhci controller is
> reset when resuming from hibernate, and all virt_devices are freed.
> 
> Also free the virt_devices starting from max slot_id as children
> more commonly have higher slot_id than parent.
> 
> CC: 
> Signed-off-by: Mathias Nyman 
> 
> ---
> 
> Guenter Roeck, does this work for you?
> 
Yes, it does.

Tested-by: Guenter Roeck 

Thanks,
Guenter

> A rework of how tt_info is stored and used might be needed,
> but that will take some time and won't go to stable as easily.
> ---
>  drivers/usb/host/xhci-mem.c | 38 --
>  1 file changed, 36 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index 6afe323..b3a5cd8 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -979,6 +979,40 @@ void xhci_free_virt_device(struct xhci_hcd *xhci, int 
> slot_id)
>   xhci->devs[slot_id] = NULL;
>  }
>  
> +/*
> + * Free a virt_device structure.
> + * If the virt_device added a tt_info (a hub) and has children pointing to
> + * that tt_info, then free the child first. Recursive.
> + * We can't rely on udev at this point to find child-parent relationships.
> + */
> +void xhci_free_virt_devices_depth_first(struct xhci_hcd *xhci, int slot_id)
> +{
> + struct xhci_virt_device *vdev;
> + struct list_head *tt_list_head;
> + struct xhci_tt_bw_info *tt_info, *next;
> + int i;
> +
> + vdev = xhci->devs[slot_id];
> + if (!vdev)
> + return;
> +
> + tt_list_head = &(xhci->rh_bw[vdev->real_port - 1].tts);
> + list_for_each_entry_safe(tt_info, next, tt_list_head, tt_list) {
> + /* is this a hub device that added a tt_info to the tts list */
> + if (tt_info->slot_id == slot_id) {
> + /* are any devices using this tt_info? */
> + for (i = 1; i < HCS_MAX_SLOTS(xhci->hcs_params1); i++) {
> + vdev = xhci->devs[i];
> + if (vdev && (vdev->tt_info == tt_info))
> + xhci_free_virt_devices_depth_first(
> + xhci, i);
> + }
> + }
> + }
> + /* we are now at a leaf device */
> + xhci_free_virt_device(xhci, slot_id);
> +}
> +
>  int xhci_alloc_virt_device(struct xhci_hcd *xhci, int slot_id,
>   struct usb_device *udev, gfp_t flags)
>  {
> @@ -1829,8 +1863,8 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
>   }
>   }
>  
> - for (i = 1; i < MAX_HC_SLOTS; ++i)
> - xhci_free_virt_device(xhci, i);
> + for (i = HCS_MAX_SLOTS(xhci->hcs_params1); i > 0; i--)
> + xhci_free_virt_devices_depth_first(xhci, i);
>  
>   dma_pool_destroy(xhci->segment_pool);
>   xhci->segment_pool = NULL;
> -- 
> 1.9.1
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv12 2/3] usb: USB Type-C connector class

[Guenter Roeck]

On Mon, Nov 28, 2016 at 04:23:23PM +0200, Heikki Krogerus wrote:
> On Mon, Nov 28, 2016 at 11:19:32AM +0100, Oliver Neukum wrote:
> > On Thu, 2016-11-24 at 11:57 +0200, Heikki Krogerus wrote:
> > > On Wed, Nov 23, 2016 at 09:12:04PM -0800, Guenter Roeck wrote:
> > 
> > > > In our implementation, the default preferred role is determined by the
> > > > low level driver (as, in my understanding, is suggested by the 
> > > > standard).
> > > > This means that the ABI will report "no preferred role", unless user 
> > > > space
> > > > overwrites it, even though there _is_ in fact a preferred role, and the
> > > > low level driver will execute try.src or try.snk based on that role.
> > > 
> > > I'm not sure which standard are you referring? Try.SNK and Try.SRC are
> > > optional mechanisms for *policy-based* role preference according to
> > > the USB Type-C spec. The policy really should always come from the
> > 

The Type-C specification states (in 4.5.2.2.11):

"Note: if both Try.SRC and Try.SNK mechanisms are implemented, only one shall be
enabled by the port at any given time. Deciding which of these two mechanisms
is enabled is product design-specific."

I read into this:
- The class code can not assume that either of those mechanisms are implemented.
- "product-design specific" means that the product designer determines which of
  the two mechanisms (if any) is enabled. While not explicitly stated, I would
  assume this to be set either by hardware or via devicetree / ACPI, and not
  from user space.

I don't mind to have user space control; all I am asking for is to have the
means for lower level code (which is the most likely entity to know about
product design) to be able to inform higher layers about its initial
preferences. We have this now, so I am happy.

> > Not all that obvious. If you are looking at it from a distro view
> > point if you know that you are booting on basically a gadget, you'll
> > be happy to take the hint. And if the hardware knows it is better
> > as a sink or source, we should take the hint.
> > 
> > > user space in our case, but I don't think that rules out for example
> > > initial role preferences coming from the lower level drivers.
> > 
> > Indeed. That should not be a hindrance to submission and inclusion.
> > 
> > > We will need a way the OS can set the initial preference for every
> > > port. Note that once we can support that, what ever the lower level
> > > drivers request will be overridden by it. So if for example the
> > > platform has preference for an initial role, we will simply ignore it
> > > if the policy says otherwise.
> > 
> > Again, not obvious in a distro. I would actually prefer a module
> > parameter that would allow us to prefer try.src, as we know how
> > to be a master.
> 
> I would be happy with module parameter.
> 
Personally I don't really care about a module parameter; as mentioned above,
I would expect the preference, if it needs to be selectable, to be configured
with devicetree or ACPI properties (or by a platform driver which sets a device
property).

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

Re: [PATCH 2/3 v3] xhci: Fix race related to abort operation

[OGAWA Hirofumi]

Vincent Pelletier  writes:

>> I couldn't reproduce that "no CRC"
>> issue with debian kernel's .config
>
> I did not diff recently my .config with debian's. I think I based it on
> Debian's 4.2 and then let it evolve "naturally" following vanilla.
> I checked all diffs against Debian's 4.8 and reduced it a lot (mostly
> new drivers which I did not enable). Doing so, I discovered that I
> somehow did not have "CONFIG_MODVERSIONS" set at all (not even the
> "is not set" comment).
>
> 4.9-rc7 is done building, there is no warning about CRC anymore -
> I'm not sure whether this comes from rc7 or MODVERSIONS. I can check if
> you think it is valuable.

At 4.9-rc7, MODVERSIONS was marked as BROKEN. It might fix that
issue. Well, anyway, it doesn't matter anymore.

> Resulting kernel booted successfully once, but I must do several more
> tries before being able to tell if it reliably does so.
> I also want to check rc7 without your patches, to compare boot failure
> rate.
>
> For reference, the I pushed the source I built in
>   https://github.com/vpelletier/linux/tree/ts651_4.9-rc7
> Please do check I did not do mistakes when resolving conflicts (the 2
> last commits).

Your ts651_4.9-rc7 (latest commit) is broken somehow. The following
incremental patch for ts651_4.9-rc7 is to fix it. With this, xhci will
become same with mine.

Thanks.
-- 
OGAWA Hirofumi 



---

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

diff -puN drivers/usb/host/xhci-ring.c~ts651_4.9-rc7-fix 
drivers/usb/host/xhci-ring.c
--- linux/drivers/usb/host/xhci-ring.c~ts651_4.9-rc7-fix2016-11-29 
01:58:29.103923074 +0900
+++ linux-hirofumi/drivers/usb/host/xhci-ring.c 2016-11-29 02:02:48.958281665 
+0900
@@ -327,16 +327,9 @@ static int xhci_abort_cmd_ring(struct xh
 
xhci_dbg(xhci, "Abort command ring\n");
 
-   temp_64 = xhci_read_64(xhci, >op_regs->cmd_ring);
-   xhci->cmd_ring_state = CMD_RING_STATE_ABORTED;
+   reinit_completion(>stop_completion);
 
-   /*
-* Writing the CMD_RING_ABORT bit should cause a cmd completion event,
-* however on some host hw the CMD_RING_RUNNING bit is correctly cleared
-* but the completion event in never sent. Use the cmd timeout timer to
-* handle those cases. Use twice the time to cover the bit polling retry
-*/
-   xhci_mod_cmd_timer(xhci, 2 * XHCI_CMD_DEFAULT_TIMEOUT);
+   temp_64 = xhci_read_64(xhci, >op_regs->cmd_ring);
xhci_write_64(xhci, temp_64 | CMD_RING_ABORT,
>op_regs->cmd_ring);
 
@@ -357,6 +350,7 @@ static int xhci_abort_cmd_ring(struct xh
xhci_halt(xhci);
return -ESHUTDOWN;
}
+
/*
 * Writing the CMD_RING_ABORT bit should cause a cmd completion event,
 * however on some host hw the CMD_RING_RUNNING bit is correctly cleared
@@ -1318,7 +1312,7 @@ void xhci_handle_command_timeout(struct
 
/* command timeout on stopped ring, ring can't be aborted */
xhci_dbg(xhci, "Command timeout on stopped ring\n");
-   complete_all(>stop_completion);
+   xhci_handle_stopped_cmd_ring(xhci, xhci->current_cmd);
spin_unlock_irqrestore(>lock, flags);
return;
 }
@@ -1359,7 +1353,7 @@ static void handle_cmd_completion(struct
 
/* If CMD ring stopped we own the trbs between enqueue and dequeue */
if (cmd_comp_code == COMP_CMD_STOP) {
-   xhci_handle_stopped_cmd_ring(xhci, cmd);
+   complete_all(>stop_completion);
return;
}
 
diff -puN drivers/usb/host/xhci-mem.c~ts651_4.9-rc7-fix 
drivers/usb/host/xhci-mem.c
--- linux/drivers/usb/host/xhci-mem.c~ts651_4.9-rc7-fix 2016-11-29 
02:01:48.178963612 +0900
+++ linux-hirofumi/drivers/usb/host/xhci-mem.c  2016-11-29 02:02:02.341037706 
+0900
@@ -1796,7 +1796,7 @@ void xhci_mem_cleanup(struct xhci_hcd *x
int size;
int i, j, num_ports;
 
-   cancel_delayed_work_sync(>cmd_timer);;
+   cancel_delayed_work_sync(>cmd_timer);
 
/* Free the Event Ring Segment Table and the actual Event Ring */
size = sizeof(struct xhci_erst_entry)*(xhci->erst.num_entries);
_
--
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/3] usb: musb: Add a quirk to preserve MUSB_DEVCTL during suspend

[Alexandre Bailon]

On da8xx, VBUS is not maintained during suspend when musb is in host mode.
On resume, all the connected devices will be disconnected and then will
be enumerated again.
This happens because MUSB_DEVCTL is cleared during suspend.
Add a quirk to preserve MUSB_DEVCTL during a suspend.

Signed-off-by: Alexandre Bailon 
---
 drivers/usb/musb/musb_core.c | 13 +++--
 drivers/usb/musb/musb_core.h |  1 +
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 9e22646..7e2cd98 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1040,14 +1040,15 @@ static void musb_enable_interrupts(struct musb *musb)
 
 }
 
-static void musb_generic_disable(struct musb *musb)
+static void musb_generic_disable(struct musb *musb, bool suspend)
 {
void __iomem*mbase = musb->mregs;
 
musb_disable_interrupts(musb);
 
/* off */
-   musb_writeb(mbase, MUSB_DEVCTL, 0);
+   if (!suspend || !(musb->io.quirks & MUSB_PRESERVE_DEVCTL))
+   musb_writeb(mbase, MUSB_DEVCTL, 0);
 }
 
 /*
@@ -1106,7 +1107,7 @@ void musb_stop(struct musb *musb)
 {
/* stop IRQs, timers, ... */
musb_platform_disable(musb);
-   musb_generic_disable(musb);
+   musb_generic_disable(musb, false);
musb_dbg(musb, "HDRC disabled");
 
/* FIXME
@@ -2310,7 +2311,7 @@ musb_init_controller(struct device *dev, int nIrq, void 
__iomem *ctrl)
 
/* be sure interrupts are disabled before connecting ISR */
musb_platform_disable(musb);
-   musb_generic_disable(musb);
+   musb_generic_disable(musb, false);
 
/* Init IRQ workqueue before request_irq */
INIT_DELAYED_WORK(>irq_work, musb_irq_work);
@@ -2486,7 +2487,7 @@ static int musb_remove(struct platform_device *pdev)
musb_gadget_cleanup(musb);
spin_lock_irqsave(>lock, flags);
musb_platform_disable(musb);
-   musb_generic_disable(musb);
+   musb_generic_disable(musb, false);
spin_unlock_irqrestore(>lock, flags);
musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
pm_runtime_dont_use_autosuspend(musb->controller);
@@ -2663,7 +2664,7 @@ static int musb_suspend(struct device *dev)
unsigned long   flags;
 
musb_platform_disable(musb);
-   musb_generic_disable(musb);
+   musb_generic_disable(musb, true);
WARN_ON(!list_empty(>pending_list));
 
spin_lock_irqsave(>lock, flags);
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index a611e2f..22961ef 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -172,6 +172,7 @@ struct musb_io;
  */
 struct musb_platform_ops {
 
+#define MUSB_PRESERVE_DEVCTL   BIT(7)
 #define MUSB_DMA_UX500 BIT(6)
 #define MUSB_DMA_CPPI41BIT(5)
 #define MUSB_DMA_CPPI  BIT(4)
-- 
2.7.3

--
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/3] usb: musb: da8xx: Fix host mode suspend

[Alexandre Bailon]

On da8xx, VBUS is not maintained during suspend when musb is in host mode.
On resume, all the connected devices will be disconnected and then will
be enumerated again.
This happens because MUSB_DEVCTL is cleared during suspend.
MUSB_DEVCTL is clear twice: once by da8xx_musb_disable()
and once musb_generic_disable().

Don't clear MUSB_DEVCTL in da8xx_musb_disable() and use the quirk
MUSB_PRESERVE_DEVCTL to preseve MUSB_DEVCTL during suspend.

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

diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index 905f0d9..90f0c06 100644
--- a/drivers/usb/musb/da8xx.c
+++ b/drivers/usb/musb/da8xx.c
@@ -125,7 +125,6 @@ static void da8xx_musb_disable(struct musb *musb)
musb_writel(reg_base, DA8XX_USB_INTR_MASK_CLEAR_REG,
DA8XX_INTR_USB_MASK |
DA8XX_INTR_TX_MASK | DA8XX_INTR_RX_MASK);
-   musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
musb_writel(reg_base, DA8XX_USB_END_OF_INTR_REG, 0);
 }
 
@@ -458,7 +457,8 @@ static inline u8 get_vbus_power(struct device *dev)
 }
 
 static const struct musb_platform_ops da8xx_ops = {
-   .quirks = MUSB_DMA_CPPI | MUSB_INDEXED_EP,
+   .quirks = MUSB_DMA_CPPI | MUSB_INDEXED_EP |
+ MUSB_PRESERVE_DEVCTL,
.init   = da8xx_musb_init,
.exit   = da8xx_musb_exit,
 
-- 
2.7.3

--
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/3] usb: musb: da8xx: Add support of suspend / resume

[Alexandre Bailon]

Implement PM methods specifics for da8xx glue.
The only thing to do is to power off the phy.
As the registers are in retention during suspend,
there is no need to save them.

Signed-off-by: Alexandre Bailon 
---
 drivers/usb/musb/da8xx.c | 29 +
 1 file changed, 29 insertions(+)

diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index e89708d..905f0d9 100644
--- a/drivers/usb/musb/da8xx.c
+++ b/drivers/usb/musb/da8xx.c
@@ -578,6 +578,34 @@ static int da8xx_remove(struct platform_device *pdev)
return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int da8xx_suspend(struct device *dev)
+{
+   int ret;
+   struct da8xx_glue *glue = dev_get_drvdata(dev);
+
+   ret = phy_power_off(glue->phy);
+   if (ret)
+   return ret;
+   clk_disable_unprepare(glue->clk);
+
+   return 0;
+}
+
+static int da8xx_resume(struct device *dev)
+{
+   int ret;
+   struct da8xx_glue *glue = dev_get_drvdata(dev);
+
+   ret = clk_prepare_enable(glue->clk);
+   if (ret)
+   return ret;
+   return phy_power_on(glue->phy);
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(da8xx_pm_ops, da8xx_suspend, da8xx_resume);
+
 #ifdef CONFIG_OF
 static const struct of_device_id da8xx_id_table[] = {
{
@@ -593,6 +621,7 @@ static struct platform_driver da8xx_driver = {
.remove = da8xx_remove,
.driver = {
.name   = "musb-da8xx",
+   .pm = _pm_ops,
.of_match_table = of_match_ptr(da8xx_id_table),
},
 };
-- 
2.7.3

--
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/3] usb: musb: da8xx: Add support of suspend / resume

[Alexandre Bailon]

There is no suspend / resume support for da8xx.
Add the PM methods to da8xx glue.
In addition, introduce a new quirk to not clear the devctl register.
Clearing devctl will power off vbus on da8xx platform
(when it is host mode) and then devices will be disconnected on resume.

The quirk name (MUSB_PRESERVE_DEVCTL) doesn't seem good to me,
so if anyone have better name, I will be happy to rename it.
I think there is other ways to fix the VBUS issue but adding a quirk
make the required changes harmless to other platform.

Alexandre Bailon (3):
  usb: musb: da8xx: Add support of suspend / resume
  usb: musb: Add a quirk to preserve MUSB_DEVCTL during suspend
  usb: musb: da8xx: Fix host mode suspend

 drivers/usb/musb/da8xx.c | 33 +++--
 drivers/usb/musb/musb_core.c | 13 +++--
 drivers/usb/musb/musb_core.h |  1 +
 3 files changed, 39 insertions(+), 8 deletions(-)

-- 
2.7.3

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

Re: [PATCH v3] fsl/usb: Workarourd for USB erratum-A005697

[Alan Stern]

On Mon, 28 Nov 2016, Changming Huang wrote:

> The EHCI specification states the following in the SUSP bit description:
> In the Suspend state, the port is sensitive to resume detection.
> Note that the bit status does not change until the port is suspended and
> that there may be a delay in suspending a port if there is a transaction
> currently in progress on the USB.
> 
> However, in NXP USBDR controller, the PORTSCx[SUSP] bit changes immediately
> when the application sets it and not when the port is actually suspended.
> 
> So the application must wait for at least 10 milliseconds after a port
> indicates that it is suspended, to make sure this port has entered
> suspended state before initiating this port resume using the Force Port
> Resume bit. This bit is for NXP controller, not EHCI compatible.
> 
> Signed-off-by: Changming Huang 
> Signed-off-by: Ramneek Mehresh 
> ---
> Changes in v3:
>   - add 10ms delay in function ehci_hub_control
>   - fix typos 
> Changes in v2:
>   - move sleep out of spin-lock
>   - add more comment for this workaround

> --- a/drivers/usb/host/ehci-hub.c
> +++ b/drivers/usb/host/ehci-hub.c
> @@ -310,6 +310,14 @@ static int ehci_bus_suspend (struct usb_hcd *hcd)
>   }
>   spin_unlock_irq(>lock);
>  
> + if (changed && ehci_has_fsl_susp_errata(ehci))
> + /*
> +  * Wait for at least 10 millisecondes to ensure the controller
> +  * enter the suspend status before initiating a port resume
> +  * using the Force Port Resume bit (Not-EHCI compatible).
> +  */
> + usleep_range(1, 2);
> +
>   if ((changed && ehci->has_tdi_phy_lpm) || fs_idle_delay) {
>   /*
>* Wait for HCD to enter low-power mode or for the bus
> @@ -1200,6 +1208,9 @@ int ehci_hub_control(
>   wIndex, (temp1 & HOSTPC_PHCD) ?
>   "succeeded" : "failed");
>   }
> + if (ehci_has_fsl_susp_errata(ehci))
> + /* 10ms for HCD enter suspend */
> + usleep_range(1, 2);
>   set_bit(wIndex, >suspended_ports);
>   break;

Just like before, you have to release the spinlock before sleeping.  
Look at the code 10 lines above this.

Alan Stern

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

Re: [PATCH v2] usb: xhci: Remove unuseful 'return' and 'break' statement

[Mathias Nyman]

On 28.11.2016 09:41, Baolin Wang wrote:

On 28 November 2016 at 15:21, Greg KH  wrote:

On Mon, Nov 28, 2016 at 02:29:25PM +0800, Baolin Wang wrote:

Hi Mathias,

On 24 November 2016 at 19:16, Baolin Wang  wrote:

Since these 'return' statements are not generally useful in void
function, remove them. Also remove one unuseful 'break' statement
in xhci_setup_addressable_virt_dev() function.

Signed-off-by: Baolin Wang 
---
Changes since v1:
  - Add description of removing 'break' statement in commitlog.
---


Could you apply this patch if there are no other comments? Thanks.


Less than a week response for a simple cleanup patch?  Why the rush and
pressure?  Relax, this really isn't an important patch...


I am sorry for the pressure, I just thought it is one simple cleanup
patch. It is okay for me to wait for.



Looks ok.

If it applies I'll send it forward to usb-next after 4.10-rc1,
It should end up in 4.11

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

Re: [PATCH 2/3 v3] xhci: Fix race related to abort operation

[Vincent Pelletier]

On Mon, 28 Nov 2016 21:57:11 +0900, OGAWA Hirofumi
 wrote:
> OGAWA Hirofumi  writes:
> Hm, this issue might be same with 4efca4ed05cbdfd13ec3e8cb623fb77d6e4ab187.
> But it was already including into 4.9-rc7.

My bad, it was rc6, not rc9.

> can you try 4.9-rc7 if older one?

Updated & rebased, building.

> I couldn't reproduce that "no CRC"
> issue with debian kernel's .config

I did not diff recently my .config with debian's. I think I based it on
Debian's 4.2 and then let it evolve "naturally" following vanilla.
I checked all diffs against Debian's 4.8 and reduced it a lot (mostly
new drivers which I did not enable). Doing so, I discovered that I
somehow did not have "CONFIG_MODVERSIONS" set at all (not even the
"is not set" comment).

4.9-rc7 is done building, there is no warning about CRC anymore -
I'm not sure whether this comes from rc7 or MODVERSIONS. I can check if
you think it is valuable.

Resulting kernel booted successfully once, but I must do several more
tries before being able to tell if it reliably does so.
I also want to check rc7 without your patches, to compare boot failure
rate.

For reference, the I pushed the source I built in
  https://github.com/vpelletier/linux/tree/ts651_4.9-rc7
Please do check I did not do mistakes when resolving conflicts (the 2
last commits).

Regards,
-- 
Vincent Pelletier
--
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: [PATCHv12 2/3] usb: USB Type-C connector class

[Heikki Krogerus]

On Mon, Nov 28, 2016 at 11:19:32AM +0100, Oliver Neukum wrote:
> On Thu, 2016-11-24 at 11:57 +0200, Heikki Krogerus wrote:
> > On Wed, Nov 23, 2016 at 09:12:04PM -0800, Guenter Roeck wrote:
> 
> > > In our implementation, the default preferred role is determined by the
> > > low level driver (as, in my understanding, is suggested by the standard).
> > > This means that the ABI will report "no preferred role", unless user space
> > > overwrites it, even though there _is_ in fact a preferred role, and the
> > > low level driver will execute try.src or try.snk based on that role.
> > 
> > I'm not sure which standard are you referring? Try.SNK and Try.SRC are
> > optional mechanisms for *policy-based* role preference according to
> > the USB Type-C spec. The policy really should always come from the
> 
> Not all that obvious. If you are looking at it from a distro view
> point if you know that you are booting on basically a gadget, you'll
> be happy to take the hint. And if the hardware knows it is better
> as a sink or source, we should take the hint.
> 
> > user space in our case, but I don't think that rules out for example
> > initial role preferences coming from the lower level drivers.
> 
> Indeed. That should not be a hindrance to submission and inclusion.
> 
> > We will need a way the OS can set the initial preference for every
> > port. Note that once we can support that, what ever the lower level
> > drivers request will be overridden by it. So if for example the
> > platform has preference for an initial role, we will simply ignore it
> > if the policy says otherwise.
> 
> Again, not obvious in a distro. I would actually prefer a module
> parameter that would allow us to prefer try.src, as we know how
> to be a master.

I would be happy with module parameter.

> None of that should hinder submission and inclusion.

It's already there in v13.

I better ping Greg already after rc1 this time, just so we don't start
the review again when we are already at rc5 (and I have forgotten
about this whole thing). I'll ask about the module parameter idea
then, though I'm guessing he won't like it. But maybe he has some
suggestions.


Thanks Oliver,

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

Re: [PATCH 2/3 v3] xhci: Fix race related to abort operation

[OGAWA Hirofumi]

OGAWA Hirofumi  writes:

>> I succeed to build 4.9-rc9 vanilla (starting from the same .config as
>> with 4.8.9) but it then fails to load produced modules. During the
>> build I have the following warnings:
>>   WARNING: "_copy_from_user" [drivers/hid/hid.ko] has no CRC!
>
> Looks like, CONFIG_MODVERSIONS is broken. CONFIG_MODVERSIONS=n may fix it.

Hm, this issue might be same with 4efca4ed05cbdfd13ec3e8cb623fb77d6e4ab187.
But it was already including into 4.9-rc7.

4.9-rc9 was actually -rc1? Well, can you try 4.9-rc7 if older one? Then
if not working, can you send .config? I couldn't reproduce that "no CRC"
issue with debian kernel's .config (BTW, compiler is debian/testing
gcc-6.2.0-13).

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

[RFC/PATCH 0/2] usb: host: xhci: couple more cleanups

[Felipe Balbi]

Hi Mathias,

these two cleanups are proposals for now, hence RFC on subject.
Let me know what you think of the idea.

Felipe Balbi (2):
  usb: host: xhci: rename completion codes to match spec
  usb: host: xhci: WARN on unexpected COMP_SUCCESS

 drivers/usb/host/xhci-hub.c  |   3 +-
 drivers/usb/host/xhci-ring.c | 154 ++-
 drivers/usb/host/xhci.c  |  48 +++---
 drivers/usb/host/xhci.h  | 106 ++---
 4 files changed, 140 insertions(+), 171 deletions(-)

-- 
2.10.1

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

[RFC/PATCH 2/2] usb: host: xhci: WARN on unexpected COMP_SUCCESS

[Felipe Balbi]

COMP_SUCCESS should only be asserted on a *true* sucessful transfer. Any
other cases are bogus and we should aim to catch them. One easy way to
get bug reports is to trigger a WARN_ONCE() on such cases.

Signed-off-by: Felipe Balbi 
---
 drivers/usb/host/xhci-ring.c | 28 +++-
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 5411af983c4e..4dd2e438467f 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1934,6 +1934,7 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct 
xhci_td *td,
 {
struct xhci_virt_device *xdev;
struct xhci_ring *ep_ring;
+   struct device *dev;
unsigned int slot_id;
int ep_index;
struct xhci_ep_ctx *ep_ctx;
@@ -1950,15 +1951,15 @@ static int process_ctrl_td(struct xhci_hcd *xhci, 
struct xhci_td *td,
trb_comp_code = GET_COMP_CODE(le32_to_cpu(event->transfer_len));
requested = td->urb->transfer_buffer_length;
remaining = EVENT_TRB_LEN(le32_to_cpu(event->transfer_len));
+   dev = xhci_to_hcd(xhci)->self.controller;
 
switch (trb_comp_code) {
case COMP_SUCCESS:
-   if (trb_type != TRB_STATUS) {
-   xhci_warn(xhci, "WARN: Success on ctrl %s TRB without 
IOC set?\n",
-   (trb_type == TRB_DATA) ? "data" : 
"setup");
-   *status = -ESHUTDOWN;
-   break;
-   }
+   dev_WARN_ONCE(dev, trb_type != TRB_STATUS,
+   "ep%d%s: unexpected success! TRB Type %d\n",
+   usb_endpoint_num(>urb->ep->desc),
+   usb_endpoint_dir_in(>urb->ep->desc) ?
+   "in" : "out", trb_type);
*status = 0;
break;
case COMP_SHORT_PACKET:
@@ -2146,6 +2147,7 @@ static int process_bulk_intr_td(struct xhci_hcd *xhci, 
struct xhci_td *td,
struct xhci_virt_ep *ep, int *status)
 {
struct xhci_ring *ep_ring;
+   struct device *dev;
u32 trb_comp_code;
u32 remaining, requested, ep_trb_len;
 
@@ -2154,16 +2156,16 @@ static int process_bulk_intr_td(struct xhci_hcd *xhci, 
struct xhci_td *td,
remaining = EVENT_TRB_LEN(le32_to_cpu(event->transfer_len));
ep_trb_len = TRB_LEN(le32_to_cpu(ep_trb->generic.field[2]));
requested = td->urb->transfer_buffer_length;
+   dev = xhci_to_hcd(xhci)->self.controller;
 
switch (trb_comp_code) {
case COMP_SUCCESS:
-   /* handle success with untransferred data as short packet */
-   if (ep_trb != td->last_trb || remaining) {
-   xhci_warn(xhci, "WARN Successful completion on short 
TX\n");
-   xhci_dbg(xhci, "ep %#x - asked for %d bytes, %d bytes 
untransferred\n",
-td->urb->ep->desc.bEndpointAddress,
-requested, remaining);
-   }
+   dev_WARN_ONCE(dev, (ep_trb != td->last_trb) || remaining,
+   "ep%d%s: unexpected success! TRB %p/%p size 
%d/%d\n",
+   usb_endpoint_num(>urb->ep->desc),
+   usb_endpoint_dir_in(>urb->ep->desc) ?
+   "in" : "out", ep_trb, td->last_trb, remaining,
+   requested);
*status = 0;
break;
case COMP_SHORT_PACKET:
-- 
2.10.1

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

[RFC/PATCH 1/2] usb: host: xhci: rename completion codes to match spec

[Felipe Balbi]

Cleanup only. This patch is a mechaninal rename to make sure our macros
for TRB completion codes match what the specification uses to refer to
such errors. The idea behind this is that it makes it far easier to grep
the specification and match it with implementation.

Signed-off-by: Felipe Balbi 
---
 drivers/usb/host/xhci-hub.c  |   3 +-
 drivers/usb/host/xhci-ring.c | 126 +--
 drivers/usb/host/xhci.c  |  48 -
 drivers/usb/host/xhci.h  | 106 +---
 4 files changed, 125 insertions(+), 158 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index ba5ffeef305d..dd6de282e48f 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -418,7 +418,8 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int 
slot_id, int suspend)
/* Wait for last stop endpoint command to finish */
wait_for_completion(cmd->completion);
 
-   if (cmd->status == COMP_CMD_ABORT || cmd->status == COMP_CMD_STOP) {
+   if (cmd->status == COMP_COMMAND_ABORTED ||
+   cmd->status == COMP_STOPPED) {
xhci_warn(xhci, "Timeout while waiting for stop endpoint 
command\n");
ret = -ETIME;
}
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 70067b81cc8f..5411af983c4e 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -995,10 +995,10 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd 
*xhci, int slot_id,
unsigned int slot_state;
 
switch (cmd_comp_code) {
-   case COMP_TRB_ERR:
+   case COMP_TRB_ERROR:
xhci_warn(xhci, "WARN Set TR Deq Ptr cmd invalid 
because of stream ID configuration\n");
break;
-   case COMP_CTX_STATE:
+   case COMP_CONTEXT_STATE_ERROR:
xhci_warn(xhci, "WARN Set TR Deq Ptr cmd failed due to 
incorrect slot or ep state.\n");
ep_state = GET_EP_CTX_STATE(ep_ctx);
slot_state = le32_to_cpu(slot_ctx->dev_state);
@@ -1007,7 +1007,7 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd 
*xhci, int slot_id,
"Slot state = %u, EP state = %u",
slot_state, ep_state);
break;
-   case COMP_EBADSLT:
+   case COMP_SLOT_NOT_ENABLED_ERROR:
xhci_warn(xhci, "WARN Set TR Deq Ptr cmd failed because 
slot %u was not enabled.\n",
slot_id);
break;
@@ -1204,7 +1204,7 @@ void xhci_cleanup_command_queue(struct xhci_hcd *xhci)
 {
struct xhci_command *cur_cmd, *tmp_cmd;
list_for_each_entry_safe(cur_cmd, tmp_cmd, >cmd_list, cmd_list)
-   xhci_complete_del_and_free_cmd(cur_cmd, COMP_CMD_ABORT);
+   xhci_complete_del_and_free_cmd(cur_cmd, COMP_COMMAND_ABORTED);
 }
 
 /*
@@ -1222,10 +1222,10 @@ static void xhci_handle_stopped_cmd_ring(struct 
xhci_hcd *xhci,
list_for_each_entry_safe(i_cmd, tmp_cmd, >cmd_list,
 cmd_list) {
 
-   if (i_cmd->status != COMP_CMD_ABORT)
+   if (i_cmd->status != COMP_COMMAND_ABORTED)
continue;
 
-   i_cmd->status = COMP_CMD_STOP;
+   i_cmd->status = COMP_STOPPED;
 
xhci_dbg(xhci, "Turn aborted command %p to no-op\n",
 i_cmd->command_trb);
@@ -1270,9 +1270,9 @@ void xhci_handle_command_timeout(unsigned long data)
/* mark this command to be cancelled */
spin_lock_irqsave(>lock, flags);
if (xhci->current_cmd) {
-   if (xhci->current_cmd->status == COMP_CMD_ABORT)
+   if (xhci->current_cmd->status == COMP_COMMAND_ABORTED)
second_timeout = true;
-   xhci->current_cmd->status = COMP_CMD_ABORT;
+   xhci->current_cmd->status = COMP_COMMAND_ABORTED;
}
 
/* Make sure command ring is running before aborting it */
@@ -1340,7 +1340,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
cmd_comp_code = GET_COMP_CODE(le32_to_cpu(event->status));
 
/* If CMD ring stopped we own the trbs between enqueue and dequeue */
-   if (cmd_comp_code == COMP_CMD_STOP) {
+   if (cmd_comp_code == COMP_STOPPED) {
xhci_handle_stopped_cmd_ring(xhci, cmd);
return;
}
@@ -1357,9 +1357,9 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
 * The command ring is stopped now, but the xHC will issue a Command
 * Ring Stopped event which will cause us to restart it.
 */
-   if (cmd_comp_code == COMP_CMD_ABORT) {
+   if (cmd_comp_code == 

Re: [PATCH 2/3 v3] xhci: Fix race related to abort operation

[OGAWA Hirofumi]

Vincent Pelletier  writes:

> I fail to build 4.8.9 vanilla:
>   Cannot use CONFIG_CC_STACKPROTECTOR_REGULAR: -fstack-protector not 
> supported by compiler

Probably, debian's gcc default-pie issue. Below patch may fix it.

> I succeed to build 4.9-rc9 vanilla (starting from the same .config as
> with 4.8.9) but it then fails to load produced modules. During the
> build I have the following warnings:
>   WARNING: "_copy_from_user" [drivers/hid/hid.ko] has no CRC!

Looks like, CONFIG_MODVERSIONS is broken. CONFIG_MODVERSIONS=n may fix it.

Thanks.
-- 
OGAWA Hirofumi 


8ae94224c9d72fc4d9aaac93b2d7833cf46d7141
82031ea29e454b574bc6f49a33683a693ca5d907
90944e40ba1838de4b2a9290cf273f9d76bd3bdd
c6a385539175ebc603da53aafb7753d39089f32e

Signed-off-by: OGAWA Hirofumi 
---

 Makefile  |5 +++--
 arch/x86/purgatory/Makefile   |1 +
 scripts/gcc-x86_64-has-stack-protector.sh |2 +-
 3 files changed, 5 insertions(+), 3 deletions(-)

diff -puN Makefile~debian-pie-default-fix Makefile
--- linux/Makefile~debian-pie-default-fix   2016-11-26 22:57:00.681881071 
+0900
+++ linux-hirofumi/Makefile 2016-11-26 22:57:00.683881081 +0900
@@ -399,11 +399,12 @@ KBUILD_CFLAGS   := -Wall -Wundef -Wstric
   -fno-strict-aliasing -fno-common \
   -Werror-implicit-function-declaration \
   -Wno-format-security \
-  -std=gnu89
+  -std=gnu89 $(call cc-option,-fno-PIE)
+
 
 KBUILD_AFLAGS_KERNEL :=
 KBUILD_CFLAGS_KERNEL :=
-KBUILD_AFLAGS   := -D__ASSEMBLY__
+KBUILD_AFLAGS   := -D__ASSEMBLY__ $(call cc-option,-fno-PIE)
 KBUILD_AFLAGS_MODULE  := -DMODULE
 KBUILD_CFLAGS_MODULE  := -DMODULE
 KBUILD_LDFLAGS_MODULE := -T $(srctree)/scripts/module-common.lds
diff -puN scripts/gcc-x86_64-has-stack-protector.sh~debian-pie-default-fix 
scripts/gcc-x86_64-has-stack-protector.sh
--- linux/scripts/gcc-x86_64-has-stack-protector.sh~debian-pie-default-fix  
2016-11-26 22:57:00.681881071 +0900
+++ linux-hirofumi/scripts/gcc-x86_64-has-stack-protector.sh2016-11-26 
22:57:00.683881081 +0900
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-echo "int foo(void) { char X[200]; return 3; }" | $* -S -x c -c -O0 
-mcmodel=kernel -fstack-protector - -o - 2> /dev/null | grep -q "%gs"
+echo "int foo(void) { char X[200]; return 3; }" | $* -S -x c -c -O0 
-mcmodel=kernel -fno-PIE -fstack-protector - -o - 2> /dev/null | grep -q "%gs"
 if [ "$?" -eq "0" ] ; then
echo y
 else
diff -puN arch/x86/purgatory/Makefile~debian-pie-default-fix 
arch/x86/purgatory/Makefile
--- linux/arch/x86/purgatory/Makefile~debian-pie-default-fix2016-11-26 
22:57:00.682881076 +0900
+++ linux-hirofumi/arch/x86/purgatory/Makefile  2016-11-26 22:57:00.683881081 
+0900
@@ -16,6 +16,7 @@ KCOV_INSTRUMENT := n
 
 KBUILD_CFLAGS := -fno-strict-aliasing -Wall -Wstrict-prototypes 
-fno-zero-initialized-in-bss -fno-builtin -ffreestanding -c -MD -Os 
-mcmodel=large
 KBUILD_CFLAGS += -m$(BITS)
+KBUILD_CFLAGS += $(call cc-option,-fno-PIE)
 
 $(obj)/purgatory.ro: $(PURGATORY_OBJS) FORCE
$(call if_changed,ld)
_
--
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] fs: configfs: don't return anything from drop_link

[Andrzej Pietrasiewicz]

Documentation/filesystems/configfs/configfs.txt says:

"When unlink(2) is called on the symbolic link, the source item is
notified via the ->drop_link() method.  Like the ->drop_item() method,
this is a void function and cannot return failure."

The ->drop_item() is indeed a void function, the ->drop_link() is
actually not. This, together with the fact that the value of ->drop_link()
is silently ignored suggests, that it is the ->drop_link() return
type that should be corrected and changed to void.

This patch changes drop_link() signature and all its users.

Compile-tested only! It needs Tested-by from respective subsystems.

Signed-off-by: Andrzej Pietrasiewicz 
---
 Documentation/filesystems/configfs/configfs.txt |  2 +-
 drivers/nvme/target/configfs.c  | 46 ++---
 drivers/target/target_core_fabric_configfs.c|  7 ++--
 drivers/usb/gadget/configfs.c   |  8 ++---
 drivers/usb/gadget/function/uvc_configfs.c  | 25 +++---
 include/linux/configfs.h|  2 +-
 6 files changed, 31 insertions(+), 59 deletions(-)

diff --git a/Documentation/filesystems/configfs/configfs.txt 
b/Documentation/filesystems/configfs/configfs.txt
index 8ec9136..3828e85 100644
--- a/Documentation/filesystems/configfs/configfs.txt
+++ b/Documentation/filesystems/configfs/configfs.txt
@@ -174,7 +174,7 @@ among other things.  For that, it needs a type.
void (*release)(struct config_item *);
int (*allow_link)(struct config_item *src,
  struct config_item *target);
-   int (*drop_link)(struct config_item *src,
+   void (*drop_link)(struct config_item *src,
 struct config_item *target);
};
 
diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index af5e2dc..9ee1490 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -466,7 +466,7 @@ static int nvmet_port_subsys_allow_link(struct config_item 
*parent,
return ret;
 }
 
-static int nvmet_port_subsys_drop_link(struct config_item *parent,
+static void nvmet_port_subsys_drop_link(struct config_item *parent,
struct config_item *target)
 {
struct nvmet_port *port = to_nvmet_port(parent->ci_parent);
@@ -474,21 +474,16 @@ static int nvmet_port_subsys_drop_link(struct config_item 
*parent,
struct nvmet_subsys_link *p;
 
down_write(_config_sem);
-   list_for_each_entry(p, >subsystems, entry) {
-   if (p->subsys == subsys)
-   goto found;
-   }
-   up_write(_config_sem);
-   return -EINVAL;
-
-found:
-   list_del(>entry);
-   nvmet_genctr++;
-   if (list_empty(>subsystems))
-   nvmet_disable_port(port);
+   list_for_each_entry(p, >subsystems, entry)
+   if (p->subsys == subsys) {
+   list_del(>entry);
+   nvmet_genctr++;
+   if (list_empty(>subsystems))
+   nvmet_disable_port(port);
+   kfree(p);
+   break;
+   }
up_write(_config_sem);
-   kfree(p);
-   return 0;
 }
 
 static struct configfs_item_operations nvmet_port_subsys_item_ops = {
@@ -542,7 +537,7 @@ static int nvmet_allowed_hosts_allow_link(struct 
config_item *parent,
return ret;
 }
 
-static int nvmet_allowed_hosts_drop_link(struct config_item *parent,
+static void nvmet_allowed_hosts_drop_link(struct config_item *parent,
struct config_item *target)
 {
struct nvmet_subsys *subsys = to_subsys(parent->ci_parent);
@@ -550,19 +545,14 @@ static int nvmet_allowed_hosts_drop_link(struct 
config_item *parent,
struct nvmet_host_link *p;
 
down_write(_config_sem);
-   list_for_each_entry(p, >hosts, entry) {
-   if (!strcmp(nvmet_host_name(p->host), nvmet_host_name(host)))
-   goto found;
-   }
-   up_write(_config_sem);
-   return -EINVAL;
-
-found:
-   list_del(>entry);
-   nvmet_genctr++;
+   list_for_each_entry(p, >hosts, entry)
+   if (!strcmp(nvmet_host_name(p->host), nvmet_host_name(host))) {
+   list_del(>entry);
+   nvmet_genctr++;
+   kfree(p);
+   break;
+   }
up_write(_config_sem);
-   kfree(p);
-   return 0;
 }
 
 static struct configfs_item_operations nvmet_allowed_hosts_item_ops = {
diff --git a/drivers/target/target_core_fabric_configfs.c 
b/drivers/target/target_core_fabric_configfs.c
index 31a096a..d8a16ca 100644
--- a/drivers/target/target_core_fabric_configfs.c
+++ b/drivers/target/target_core_fabric_configfs.c
@@ -137,7 +137,7 @@ static int target_fabric_mappedlun_link(
return core_dev_add_initiator_node_lun_acl(se_tpg, lacl, 

Re: [PATCH 2/3 v3] xhci: Fix race related to abort operation

[Vincent Pelletier]

On Thu, 24 Nov 2016 19:42:04 +0900, OGAWA Hirofumi
 wrote:
> If you need my help for something, let me know either publicly or
> privately what way you want.

So, I fail a lot. And in ways totally unrelated to this patchset.

My goal is to get a kernel image built with a trivial platform patch
for my hardware (declare a few GPIO with leds and keys), a revert of
  "xhci: give command abortion one more chance before killing xhci"
  a6809ffd1687b3a8c192960e69add559b9d32649
to be able to tell if your changes improve the situation, plus a
backport of changes 1 & 2 (which conflict with the revert), as I saw 3
is not essential and maybe on the way out.

I fail to build 4.8.9 vanilla:
  Cannot use CONFIG_CC_STACKPROTECTOR_REGULAR: -fstack-protector not supported 
by compiler
I succeed to build 4.9-rc9 vanilla (starting from the same .config as
with 4.8.9) but it then fails to load produced modules. During the
build I have the following warnings:
  WARNING: "_copy_from_user" [drivers/hid/hid.ko] has no CRC!

Any idea on either ?

I am building from Debian sid, build & target are x86_64.
$ gcc --version
gcc (Debian 6.2.0-13) 6.2.0 20161109

Build command line:
$ make deb-pkg

Regards,
-- 
Vincent Pelletier
--
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/1] NET: usb: cdc_ncm: adding MBIM RESET_FUNCTION request and modifying ncm bind common code

[Daniele Palmas]

2016-11-26 22:17 GMT+01:00 Bjørn Mork :
> Bjørn Mork  writes:
>
>> Finally, I found my modems (or at least a number of them) again today.
>> But I'm sorry to say, that the troublesome Huawei E3372h-153 is still
>> giving us a hard time.  It does not work with your patch. The symptom is
>> the same as earlier:  The modem returns MBIM frames with 32bit headers.
>>
>> So for now, I have to NAK this patch.
>>
>> I am sure we can find a good solution that makes all of these modems
>> work, but I cannot support a patch that breaks previously working
>> configurations. Sorry.  I'll do a few experiments and see if there is a
>> simple fix for this.  Otherwise we'll probably have to do the quirk
>> game.
>
>
> This is a proof-of-concept only, but it appears to be working.  Please
> test with your device(s) too.  It's still mostly your code, as you can
> see.

Sorry, this does not work :-(

The problem is always in the altsetting toggle: if I comment that
part, your patch is working fine.

I'm now wondering if the safest thing is to add a very simple quirk in
cdc_mbim that makes this toggle not to be applied with the buggy
modems and then unconditionally add the RESET_FUNCTION request in
cdc_mbim_bind after cdc_ncm_bind_common has been executed.

Daniele

>
> If this turns out to work, then I believe we should refactor
> cdc_ncm_init() and cdc_ncm_bind_common() to make the whole
> initialisation sequence a bit cleaner.  And maybe also include
> cdc_mbim_bind().  Ideally, the MBIM specific RESET should happen there
> instead of "polluting" the NCM driver with MBIM specific code.
>
> But anyway:  The sequence that seems to work for both the  E3372h-153
> and the EM7455 is
>
>  USB_CDC_GET_NTB_PARAMETERS
>  USB_CDC_RESET_FUNCTION
>  usb_set_interface(dev->udev, 'data interface no', 0);
>  remaining parts of cdc_ncm_init(), excluding USB_CDC_GET_NTB_PARAMETERS
>  usb_set_interface(dev->udev, 'data interface no', 'data alt setting');
>
> without any additional delay between the two usb_set_interface() calls.
> So the major difference from your patch is that I moved the two control
> requests out of cdc_ncm_init() to allow running them _before_ setting
> the data interface to altsetting 0.
>
> But maybe I was just lucky.  This was barely proof tested.  Needs a lot
> more testing and cleanups as suggested.  I'd appreciate it if you
> continued that, as I don't really have any time for it...
>
> FWIW, I also ran a quick test with a D-Link DWM-156A7 (Mediatek MBIM
> firmware) and a Huawei E367 (Qualcomm device with early Huawei MBIM
> firmware, distinctly different from the E3372h-153 and most other
> MBIM devices I've seen)
>
>
>
> Bjørn
>
> ---
>  drivers/net/usb/cdc_ncm.c| 48 
> 
>  include/uapi/linux/usb/cdc.h |  1 +
>  2 files changed, 32 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
> index 877c9516e781..be019cbf1719 100644
> --- a/drivers/net/usb/cdc_ncm.c
> +++ b/drivers/net/usb/cdc_ncm.c
> @@ -488,16 +488,6 @@ static int cdc_ncm_init(struct usbnet *dev)
> u8 iface_no = ctx->control->cur_altsetting->desc.bInterfaceNumber;
> int err;
>
> -   err = usbnet_read_cmd(dev, USB_CDC_GET_NTB_PARAMETERS,
> - USB_TYPE_CLASS | USB_DIR_IN
> - |USB_RECIP_INTERFACE,
> - 0, iface_no, >ncm_parm,
> - sizeof(ctx->ncm_parm));
> -   if (err < 0) {
> -   dev_err(>intf->dev, "failed GET_NTB_PARAMETERS\n");
> -   return err; /* GET_NTB_PARAMETERS is required */
> -   }
> -
> /* set CRC Mode */
> if (cdc_ncm_flags(dev) & USB_CDC_NCM_NCAP_CRC_MODE) {
> dev_dbg(>intf->dev, "Setting CRC mode off\n");
> @@ -837,12 +827,43 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct 
> usb_interface *intf, u8 data_
> }
> }
>
> +   iface_no = ctx->control->cur_altsetting->desc.bInterfaceNumber;
> +   temp = usbnet_read_cmd(dev, USB_CDC_GET_NTB_PARAMETERS,
> +  USB_TYPE_CLASS | USB_DIR_IN
> +  | USB_RECIP_INTERFACE,
> +  0, iface_no, >ncm_parm,
> +  sizeof(ctx->ncm_parm));
> +   if (temp < 0) {
> +   dev_err(>intf->dev, "failed GET_NTB_PARAMETERS\n");
> +   goto error; /* GET_NTB_PARAMETERS is required */
> +   }
> +
> +   /* Some modems (e.g. Telit LE922A6) need to reset the MBIM function
> +* or they will fail to work properly.
> +* For details on RESET_FUNCTION request see document
> +* "USB Communication Class Subclass Specification for MBIM"
> +* RESET_FUNCTION should be harmless for all the other MBIM modems
> +*/
> +   if (cdc_ncm_comm_intf_is_mbim(ctx->control->cur_altsetting)) {
> +   temp 

Re: [PATCH] usb: musb: mark PM functions as __maybe_unused

[Geert Uytterhoeven]

Hi Arnd,

On Mon, Nov 28, 2016 at 11:54 AM, Arnd Bergmann  wrote:
> On Monday, November 28, 2016 11:51:37 AM CET Geert Uytterhoeven wrote:
>> On Tue, Nov 22, 2016 at 3:30 PM, Arnd Bergmann  wrote:
>> > Building without CONFIG_PM causes a harmless warning:
>> >
>> > drivers/usb/musb/musb_core.c:2041:12: error: ‘musb_run_resume_work’ 
>> > defined but not used [-Werror=unused-function]
>> >
>> > Removing the #ifdef around the PM code and instead marking the 
>> > suspend/resume
>> > functions as __maybe_unused will do the right thing without warning.
>> >
>> > Fixes: ea2f35c01d5e ("usb: musb: Fix sleeping function called from invalid 
>> > context for hdrc glue")
>> > Signed-off-by: Arnd Bergmann 
>>
>> linux-m68k-allmodconfig$ bloat-o-meter drivers/usb/musb/musb_core.o{.orig,}
>> add/remove: 8/0 grow/shrink: 0/0 up/down: 2344/0 (2344)
>> function old new   delta
>> musb_restore_context   - 892+892
>> musb_save_context  - 690+690
>> musb_run_resume_work   - 190+190
>> musb_resume- 182+182
>> musb_runtime_resume- 148+148
>> musb_suspend   - 114+114
>> musb_dev_pm_ops-  92 +92
>> musb_runtime_suspend   -  36 +36
>> Total: Before=13091, After=15435, chg +17.91%
>
> Well, in allmodconfig, you have CONFIG_PM enabled, so this is not
> dead code but actually does what was intended (though possibly
> not written as efficiently as it could have been).

M68k does not have CONFIG_PM.

> In a configuration without CONFIG_PM, there should ideally be
> no added code.

Apparently not :-(

BTW, same result for allyesconfig.

Gr{oetje,eeting}s,

Geert

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

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/4] usb: host: xhci: dynamically allocate devs array

[Felipe Balbi]

Instead of always defaulting to a 256-entry array,
we can dynamically allocate devs based on what HW
tells us it supports.

Note that we can't, yet, purge MAX_HC_SLOTS
completely because of struct
xhci_device_context_array reliance on it.

Signed-off-by: Felipe Balbi 
---
 drivers/usb/host/xhci-hub.c  |  2 +-
 drivers/usb/host/xhci-mem.c  |  4 ++--
 drivers/usb/host/xhci-ring.c |  2 +-
 drivers/usb/host/xhci.c  | 19 +++
 drivers/usb/host/xhci.h  |  2 +-
 5 files changed, 20 insertions(+), 9 deletions(-)

Changes since v1:
- accounted for invalid slot 0 which driver assumes to exist.

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 0ef16900efed..ba5ffeef305d 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -356,7 +356,7 @@ int xhci_find_slot_id_by_port(struct usb_hcd *hcd, struct 
xhci_hcd *xhci,
enum usb_device_speed speed;
 
slot_id = 0;
-   for (i = 0; i < MAX_HC_SLOTS; i++) {
+   for (i = 0; i <= HCS_MAX_SLOTS(xhci->hcs_params1); i++) {
if (!xhci->devs[i])
continue;
speed = xhci->devs[i]->udev->speed;
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 321de2e0161b..3d9d6e893c79 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1828,7 +1828,7 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
}
}
 
-   for (i = 1; i < MAX_HC_SLOTS; ++i)
+   for (i = 1; i <= HCS_MAX_SLOTS(xhci->hcs_params1); ++i)
xhci_free_virt_device(xhci, i);
 
dma_pool_destroy(xhci->segment_pool);
@@ -2535,7 +2535,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 * something other than the default (~1ms minimum between interrupts).
 * See section 5.5.1.2.
 */
-   for (i = 0; i < MAX_HC_SLOTS; ++i)
+   for (i = 0; i <= HCS_MAX_SLOTS(xhci->hcs_params1); ++i)
xhci->devs[i] = NULL;
for (i = 0; i < USB_MAXCHILDREN; ++i) {
xhci->bus_state[0].resume_done[i] = 0;
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index bdf6b13d9b67..76402b44f832 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -895,7 +895,7 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg)
 * doesn't touch the memory.
 */
}
-   for (i = 0; i < MAX_HC_SLOTS; i++) {
+   for (i = 0; i <= HCS_MAX_SLOTS(xhci->hcs_params1); i++) {
if (!xhci->devs[i])
continue;
for (j = 0; j < 31; j++)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 1cd56417cbec..1113b5fea7b4 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -766,6 +766,8 @@ void xhci_shutdown(struct usb_hcd *hcd)
/* Yet another workaround for spurious wakeups at shutdown with HSW */
if (xhci->quirks & XHCI_SPURIOUS_WAKEUP)
pci_set_power_state(to_pci_dev(hcd->self.controller), 
PCI_D3hot);
+
+   kfree(xhci->devs);
 }
 
 #ifdef CONFIG_PM
@@ -4896,6 +4898,11 @@ int xhci_gen_setup(struct usb_hcd *hcd, 
xhci_get_quirks_t get_quirks)
 
xhci->quirks |= quirks;
 
+   xhci->devs = kcalloc(HCS_MAX_SLOTS(xhci->hcs_params1) + 1,
+   sizeof(struct xhci_virt_device *), GFP_KERNEL);
+   if (!xhci->devs)
+   return -ENOMEM;
+
get_quirks(dev, xhci);
 
/* In xhci controllers which follow xhci 1.0 spec gives a spurious
@@ -4908,13 +4915,13 @@ int xhci_gen_setup(struct usb_hcd *hcd, 
xhci_get_quirks_t get_quirks)
/* Make sure the HC is halted. */
retval = xhci_halt(xhci);
if (retval)
-   return retval;
+   goto err;
 
xhci_dbg(xhci, "Resetting HCD\n");
/* Reset the internal HC memory state and registers. */
retval = xhci_reset(xhci);
if (retval)
-   return retval;
+   goto err;
xhci_dbg(xhci, "Reset complete\n");
 
/*
@@ -4940,7 +4947,7 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t 
get_quirks)
 */
retval = dma_set_mask(dev, DMA_BIT_MASK(32));
if (retval)
-   return retval;
+   goto err;
xhci_dbg(xhci, "Enabling 32-bit DMA addresses.\n");
dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
}
@@ -4949,13 +4956,17 @@ int xhci_gen_setup(struct usb_hcd *hcd, 
xhci_get_quirks_t get_quirks)
/* Initialize HCD and host controller data structures. */
retval = xhci_init(hcd);
if (retval)
-   return retval;
+   goto err;
xhci_dbg(xhci, "Called HCD init\n");
 
xhci_info(xhci, "hcc params 0x%08x hci version 0x%x quirks 0x%08x\n",
  

[PATCH v2 2/4] usb: host: xhci: handle COMP_STOP from SETUP phase too

[Felipe Balbi]

Stop Endpoint command can come at any point and we
have no control of that. We should make sure to
handle COMP_STOP on SETUP phase as well, otherwise
urb->actual_lenght might be set to negative values
in some occasions such as below:

 urb->length = 4;
 build_control_transfer_td_for(urb, ep);

stop_endpoint(ep);

COMP_STOP:
[...]
urb->actual_length = urb->length - trb->length;

trb->length is 8 for SETUP stage (8 control request
bytes), so actual_length would be set to -4 in this
case.

While doing that, also make sure to use TRB_TYPE
field of the actual TRB instead of matching pointers
to figure out in which stage of the control transfer
we got our completion event.

Signed-off-by: Felipe Balbi 
---
 drivers/usb/host/xhci-ring.c | 30 ++
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 76402b44f832..70067b81cc8f 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1939,8 +1939,9 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct 
xhci_td *td,
struct xhci_ep_ctx *ep_ctx;
u32 trb_comp_code;
u32 remaining, requested;
-   bool on_data_stage;
+   u32 trb_type;
 
+   trb_type = TRB_FIELD_TO_TYPE(le32_to_cpu(ep_trb->generic.field[3]));
slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event->flags));
xdev = xhci->devs[slot_id];
ep_index = TRB_TO_EP_ID(le32_to_cpu(event->flags)) - 1;
@@ -1950,14 +1951,11 @@ static int process_ctrl_td(struct xhci_hcd *xhci, 
struct xhci_td *td,
requested = td->urb->transfer_buffer_length;
remaining = EVENT_TRB_LEN(le32_to_cpu(event->transfer_len));
 
-   /* not setup (dequeue), or status stage means we are at data stage */
-   on_data_stage = (ep_trb != ep_ring->dequeue && ep_trb != td->last_trb);
-
switch (trb_comp_code) {
case COMP_SUCCESS:
-   if (ep_trb != td->last_trb) {
+   if (trb_type != TRB_STATUS) {
xhci_warn(xhci, "WARN: Success on ctrl %s TRB without 
IOC set?\n",
- on_data_stage ? "data" : "setup");
+   (trb_type == TRB_DATA) ? "data" : 
"setup");
*status = -ESHUTDOWN;
break;
}
@@ -1967,15 +1965,23 @@ static int process_ctrl_td(struct xhci_hcd *xhci, 
struct xhci_td *td,
*status = 0;
break;
case COMP_STOP_SHORT:
-   if (on_data_stage)
+   if (trb_type == TRB_DATA)
td->urb->actual_length = remaining;
else
xhci_warn(xhci, "WARN: Stopped Short Packet on ctrl 
setup or status TRB\n");
goto finish_td;
case COMP_STOP:
-   if (on_data_stage)
+   switch (trb_type) {
+   case TRB_SETUP:
+   td->urb->actual_length = 0;
+   goto finish_td;
+   case TRB_DATA:
td->urb->actual_length = requested - remaining;
-   goto finish_td;
+   goto finish_td;
+   default:
+   xhci_warn(xhci, "WARN: unexpected TRB Type %d\n", 
trb_type);
+   goto finish_td;
+   }
case COMP_STOP_INVAL:
goto finish_td;
default:
@@ -1987,7 +1993,7 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct 
xhci_td *td,
/* else fall through */
case COMP_STALL:
/* Did we transfer part of the data (middle) phase? */
-   if (on_data_stage)
+   if (trb_type == TRB_DATA)
td->urb->actual_length = requested - remaining;
else if (!td->urb_length_set)
td->urb->actual_length = 0;
@@ -1995,14 +2001,14 @@ static int process_ctrl_td(struct xhci_hcd *xhci, 
struct xhci_td *td,
}
 
/* stopped at setup stage, no data transferred */
-   if (ep_trb == ep_ring->dequeue)
+   if (trb_type == TRB_SETUP)
goto finish_td;
 
/*
 * if on data stage then update the actual_length of the URB and flag it
 * as set, so it won't be overwritten in the event for the last TRB.
 */
-   if (on_data_stage) {
+   if (trb_type == TRB_DATA) {
td->urb_length_set = true;
td->urb->actual_length = requested - remaining;
xhci_dbg(xhci, "Waiting for status stage event\n");
-- 
2.10.1

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

[PATCH v2 4/4] usb: host: xhci: print HCIVERSION on debug

[Felipe Balbi]

When calling xhci_dbg_regs() we actually _do_ want to know XHCI's
version. This might help figure out why certain problems only happen
in some cases.

Signed-off-by: Felipe Balbi 
---
 drivers/usb/host/xhci-dbg.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/usb/host/xhci-dbg.c b/drivers/usb/host/xhci-dbg.c
index a3b67f33d4d8..363d125300ea 100644
--- a/drivers/usb/host/xhci-dbg.c
+++ b/drivers/usb/host/xhci-dbg.c
@@ -37,10 +37,8 @@ void xhci_dbg_regs(struct xhci_hcd *xhci)
>cap_regs->hc_capbase, temp);
xhci_dbg(xhci, "//   CAPLENGTH: 0x%x\n",
(unsigned int) HC_LENGTH(temp));
-#if 0
xhci_dbg(xhci, "//   HCIVERSION: 0x%x\n",
(unsigned int) HC_VERSION(temp));
-#endif
 
xhci_dbg(xhci, "// xHCI operational registers at %p:\n", xhci->op_regs);
 
-- 
2.10.1

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

[PATCH v2 0/4] usb: host: xhci: cleanups and couple fixes

[Felipe Balbi]

Hi Mathias,

These four patches help with two minor details in XHCI and also
implement a couple cleanups.

Patch 1 reduces the amount of memory used by our virt_device array by
discovering from the HW how many slots we can support.

Patch 2 makes COMP_STOP work when a Stop Endpoint command is issued
while we're still in SETUP stage. Note how I have also converted
on_data_stage and all the pointer mathing to more robust TRB type
matching.

Patch 3 just changes all pre-increments to post-increments.

Patch 4 removes a #if 0 from dbg code so we also print out XHCI's
revision when debugging.

Tested (albeit lightly with USB storage and keyboard + rootfs on another
USB storage) with a SKL box. Patches on top of today's next/master

Felipe Balbi (4):
  usb: host: xhci: dynamically allocate devs array
  usb: host: xhci: handle COMP_STOP from SETUP phase too
  usb: host: xhci: change pre-increments to post-increments
  usb: host: xhci: print HCIVERSION on debug

 drivers/usb/host/xhci-dbg.c  | 22 ++
 drivers/usb/host/xhci-hub.c  |  2 +-
 drivers/usb/host/xhci-mem.c  | 10 +-
 drivers/usb/host/xhci-ring.c | 32 +++-
 drivers/usb/host/xhci.c  | 33 ++---
 drivers/usb/host/xhci.h  |  2 +-
 6 files changed, 58 insertions(+), 43 deletions(-)

-- 
2.10.1

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

Re: [PATCH] usb: xhci: apply XHCI_PME_STUCK_QUIRK to Intel Apollo Lake

[Mathias Nyman]

On 28.11.2016 09:24, Greg KH wrote:

On Mon, Nov 28, 2016 at 09:53:52AM +0800, 
wan.ahmad.zainie.wan.moha...@intel.com wrote:

From: Wan Ahmad Zainie 

Intel Apollo Lake also requires XHCI_PME_STUCK_QUIRK.
Adding its PCI ID to quirk.

Signed-off-by: Wan Ahmad Zainie 
---
  drivers/usb/host/xhci-pci.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index e96ae80..954abfd 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -165,7 +165,8 @@ static void xhci_pci_quirks(struct device *dev, struct 
xhci_hcd *xhci)
 pdev->device == PCI_DEVICE_ID_INTEL_SUNRISEPOINT_H_XHCI ||
 pdev->device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI ||
 pdev->device == PCI_DEVICE_ID_INTEL_BROXTON_M_XHCI ||
-pdev->device == PCI_DEVICE_ID_INTEL_BROXTON_B_XHCI)) {
+pdev->device == PCI_DEVICE_ID_INTEL_BROXTON_B_XHCI ||
+pdev->device == PCI_DEVICE_ID_INTEL_APL_XHCI)) {
xhci->quirks |= XHCI_PME_STUCK_QUIRK;


When is Intel going to fix this?  Why don't we just blacklist all intel
hosts :(



Apollo Lake is Broxton-based/a Broxton derivative, with, well pretty much the 
same xHCI IP with
the same flaws, but different PCI ID.

I agree the quirk list is getting long. And annoying to maintain.
I'll see if there's a better way to identify the hosts that need this quirk.

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

[PATCH v2 3/4] usb: host: xhci: change pre-increments to post-increments

[Felipe Balbi]

This is a cleanup patch only, no functional changes. The idea is just to
make sure for loops look the same all over the driver.

Signed-off-by: Felipe Balbi 
---
 drivers/usb/host/xhci-dbg.c | 20 ++--
 drivers/usb/host/xhci-mem.c | 10 +-
 drivers/usb/host/xhci.c | 14 +++---
 3 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/host/xhci-dbg.c b/drivers/usb/host/xhci-dbg.c
index 74c42f722678..a3b67f33d4d8 100644
--- a/drivers/usb/host/xhci-dbg.c
+++ b/drivers/usb/host/xhci-dbg.c
@@ -177,7 +177,7 @@ static void xhci_print_ports(struct xhci_hcd *xhci)
ports = HCS_MAX_PORTS(xhci->hcs_params1);
addr = >op_regs->port_status_base;
for (i = 0; i < ports; i++) {
-   for (j = 0; j < NUM_PORT_REGS; ++j) {
+   for (j = 0; j < NUM_PORT_REGS; j++) {
xhci_dbg(xhci, "%p port %s reg = 0x%x\n",
addr, names[j],
(unsigned int) readl(addr));
@@ -240,7 +240,7 @@ void xhci_print_run_regs(struct xhci_hcd *xhci)
xhci_dbg(xhci, "  %p: Microframe index = 0x%x\n",
>run_regs->microframe_index,
(unsigned int) temp);
-   for (i = 0; i < 7; ++i) {
+   for (i = 0; i < 7; i++) {
temp = readl(>run_regs->rsvd[i]);
if (temp != XHCI_INIT_VALUE)
xhci_dbg(xhci, "  WARN: %p: Rsvd[%i] = 0x%x\n",
@@ -259,7 +259,7 @@ void xhci_print_registers(struct xhci_hcd *xhci)
 void xhci_print_trb_offsets(struct xhci_hcd *xhci, union xhci_trb *trb)
 {
int i;
-   for (i = 0; i < 4; ++i)
+   for (i = 0; i < 4; i++)
xhci_dbg(xhci, "Offset 0x%x = 0x%x\n",
i*4, trb->generic.field[i]);
 }
@@ -332,7 +332,7 @@ void xhci_debug_segment(struct xhci_hcd *xhci, struct 
xhci_segment *seg)
u64 addr = seg->dma;
union xhci_trb *trb = seg->trbs;
 
-   for (i = 0; i < TRBS_PER_SEGMENT; ++i) {
+   for (i = 0; i < TRBS_PER_SEGMENT; i++) {
trb = >trbs[i];
xhci_dbg(xhci, "@%016llx %08x %08x %08x %08x\n", addr,
 lower_32_bits(le64_to_cpu(trb->link.segment_ptr)),
@@ -413,7 +413,7 @@ void xhci_dbg_erst(struct xhci_hcd *xhci, struct xhci_erst 
*erst)
int i;
struct xhci_erst_entry *entry;
 
-   for (i = 0; i < erst->num_entries; ++i) {
+   for (i = 0; i < erst->num_entries; i++) {
entry = >entries[i];
xhci_dbg(xhci, "@%016llx %08x %08x %08x %08x\n",
 addr,
@@ -440,7 +440,7 @@ void xhci_dbg_cmd_ptrs(struct xhci_hcd *xhci)
 static void dbg_rsvd64(struct xhci_hcd *xhci, u64 *ctx, dma_addr_t dma)
 {
int i;
-   for (i = 0; i < 4; ++i) {
+   for (i = 0; i < 4; i++) {
xhci_dbg(xhci, "@%p (virt) @%08llx "
 "(dma) %#08llx - rsvd64[%d]\n",
 [4 + i], (unsigned long long)dma,
@@ -496,7 +496,7 @@ static void xhci_dbg_slot_ctx(struct xhci_hcd *xhci, struct 
xhci_container_ctx *
_ctx->dev_state,
(unsigned long long)dma, slot_ctx->dev_state);
dma += field_size;
-   for (i = 0; i < 4; ++i) {
+   for (i = 0; i < 4; i++) {
xhci_dbg(xhci, "@%p (virt) @%08llx (dma) %#08x - rsvd[%d]\n",
_ctx->reserved[i], (unsigned long long)dma,
slot_ctx->reserved[i], i);
@@ -519,7 +519,7 @@ static void xhci_dbg_ep_ctx(struct xhci_hcd *xhci,
 
if (last_ep < 31)
last_ep_ctx = last_ep + 1;
-   for (i = 0; i < last_ep_ctx; ++i) {
+   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 +
@@ -544,7 +544,7 @@ static void xhci_dbg_ep_ctx(struct xhci_hcd *xhci,
_ctx->tx_info,
(unsigned long long)dma, ep_ctx->tx_info);
dma += field_size;
-   for (j = 0; j < 3; ++j) {
+   for (j = 0; j < 3; j++) {
xhci_dbg(xhci, "@%p (virt) @%08llx (dma) %#08x - 
rsvd[%d]\n",
_ctx->reserved[j],
(unsigned long long)dma,
@@ -583,7 +583,7 @@ void xhci_dbg_ctx(struct xhci_hcd *xhci,
 _ctx->add_flags, (unsigned long long)dma,
 ctrl_ctx->add_flags);
dma += field_size;
-   for (i = 0; i < 6; ++i) {
+   for (i = 0; i < 6; i++) {
xhci_dbg(xhci, "@%p (virt) @%08llx (dma) %#08x - 
rsvd2[%d]\n",
 _ctx->rsvd2[i], (unsigned 

Re: [PATCH] usb: musb: mark PM functions as __maybe_unused

[Arnd Bergmann]

On Monday, November 28, 2016 11:51:37 AM CET Geert Uytterhoeven wrote:
> On Tue, Nov 22, 2016 at 3:30 PM, Arnd Bergmann  wrote:
> > Building without CONFIG_PM causes a harmless warning:
> >
> > drivers/usb/musb/musb_core.c:2041:12: error: ‘musb_run_resume_work’ defined 
> > but not used [-Werror=unused-function]
> >
> > Removing the #ifdef around the PM code and instead marking the 
> > suspend/resume
> > functions as __maybe_unused will do the right thing without warning.
> >
> > Fixes: ea2f35c01d5e ("usb: musb: Fix sleeping function called from invalid 
> > context for hdrc glue")
> > Signed-off-by: Arnd Bergmann 
> 
> linux-m68k-allmodconfig$ bloat-o-meter drivers/usb/musb/musb_core.o{.orig,}
> add/remove: 8/0 grow/shrink: 0/0 up/down: 2344/0 (2344)
> function old new   delta
> musb_restore_context   - 892+892
> musb_save_context  - 690+690
> musb_run_resume_work   - 190+190
> musb_resume- 182+182
> musb_runtime_resume- 148+148
> musb_suspend   - 114+114
> musb_dev_pm_ops-  92 +92
> musb_runtime_suspend   -  36 +36
> Total: Before=13091, After=15435, chg +17.91%

Well, in allmodconfig, you have CONFIG_PM enabled, so this is not
dead code but actually does what was intended (though possibly
not written as efficiently as it could have been).

In a configuration without CONFIG_PM, there should ideally be
no added code.

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: [PATCH] usb: musb: mark PM functions as __maybe_unused

[Geert Uytterhoeven]

On Tue, Nov 22, 2016 at 3:30 PM, Arnd Bergmann  wrote:
> Building without CONFIG_PM causes a harmless warning:
>
> drivers/usb/musb/musb_core.c:2041:12: error: ‘musb_run_resume_work’ defined 
> but not used [-Werror=unused-function]
>
> Removing the #ifdef around the PM code and instead marking the suspend/resume
> functions as __maybe_unused will do the right thing without warning.
>
> Fixes: ea2f35c01d5e ("usb: musb: Fix sleeping function called from invalid 
> context for hdrc glue")
> Signed-off-by: Arnd Bergmann 

linux-m68k-allmodconfig$ bloat-o-meter drivers/usb/musb/musb_core.o{.orig,}
add/remove: 8/0 grow/shrink: 0/0 up/down: 2344/0 (2344)
function old new   delta
musb_restore_context   - 892+892
musb_save_context  - 690+690
musb_run_resume_work   - 190+190
musb_resume- 182+182
musb_runtime_resume- 148+148
musb_suspend   - 114+114
musb_dev_pm_ops-  92 +92
musb_runtime_suspend   -  36 +36
Total: Before=13091, After=15435, chg +17.91%

Doh...

Gr{oetje,eeting}s,

Geert

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

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv12 2/3] usb: USB Type-C connector class

[Oliver Neukum]

On Thu, 2016-11-24 at 11:57 +0200, Heikki Krogerus wrote:
> On Wed, Nov 23, 2016 at 09:12:04PM -0800, Guenter Roeck wrote:

> > In our implementation, the default preferred role is determined by the
> > low level driver (as, in my understanding, is suggested by the standard).
> > This means that the ABI will report "no preferred role", unless user space
> > overwrites it, even though there _is_ in fact a preferred role, and the
> > low level driver will execute try.src or try.snk based on that role.
> 
> I'm not sure which standard are you referring? Try.SNK and Try.SRC are
> optional mechanisms for *policy-based* role preference according to
> the USB Type-C spec. The policy really should always come from the

Not all that obvious. If you are looking at it from a distro view
point if you know that you are booting on basically a gadget, you'll
be happy to take the hint. And if the hardware knows it is better
as a sink or source, we should take the hint.

> user space in our case, but I don't think that rules out for example
> initial role preferences coming from the lower level drivers.

Indeed. That should not be a hindrance to submission and inclusion.

> We will need a way the OS can set the initial preference for every
> port. Note that once we can support that, what ever the lower level
> drivers request will be overridden by it. So if for example the
> platform has preference for an initial role, we will simply ignore it
> if the policy says otherwise.

Again, not obvious in a distro. I would actually prefer a module
parameter that would allow us to prefer try.src, as we know how
to be a master.

None of that should hinder submission and inclusion.

Regards
Oliver


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

Re: [PATCH 1/2] usb: host: xhci: dynamically allocate devs array

[Felipe Balbi]

Hi,

Mathias Nyman  writes:
> On 24.11.2016 15:33, Felipe Balbi wrote:
>> Instead of always defaulting to a 256-entry array,
>> we can dynamically allocate devs based on what HW
>> tells us it supports.
>>
>> Note that we can't, yet, purge MAX_HC_SLOTS
>> completely because of struct
>> xhci_device_context_array reliance on it.
>>
>> Signed-off-by: Felipe Balbi 
>> ---
>>   drivers/usb/host/xhci-hub.c  |  2 +-
>>   drivers/usb/host/xhci-mem.c  |  4 ++--
>>   drivers/usb/host/xhci-ring.c |  2 +-
>>   drivers/usb/host/xhci.c  | 19 +++
>>   drivers/usb/host/xhci.h  |  2 +-
>>   5 files changed, 20 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
>> index 0ef16900efed..7b1b58ad6aac 100644
>> --- a/drivers/usb/host/xhci-hub.c
>> +++ b/drivers/usb/host/xhci-hub.c
>> @@ -356,7 +356,7 @@ int xhci_find_slot_id_by_port(struct usb_hcd *hcd, 
>> struct xhci_hcd *xhci,
>>  enum usb_device_speed speed;
>>
>>  slot_id = 0;
>> -for (i = 0; i < MAX_HC_SLOTS; i++) {
>> +for (i = 0; i < HCS_MAX_SLOTS(xhci->hcs_params1); i++) {
>
> Normally that is what it should look like, but as we in xhci driver don't use
> xhci->devs[0], and xhci->devs[HCS_MAX_SLOTS(xhci->hcs_params1)] can be a valid
> virt_device it needs a +1.

Should HCS_MAX_SLOTS() already handle that, then?

> Same goes for everywhere else (also when allocating)
>
> This is probably originally due to that xhci hw returns a slot_id 0 for 
> failure, and
> 1 to, including HCD_MAX_SLOTS[hcs_params1) for successful enable device 
> command.
>
> the virt_dev is then straight allocated to xhci->devs[slot_id] = kzalloc(..)
> 
> -Mathias

-- 
balbi


signature.asc
Description: PGP signature

[PATCH v3] fsl/usb: Workarourd for USB erratum-A005697

[Changming Huang]

The EHCI specification states the following in the SUSP bit description:
In the Suspend state, the port is sensitive to resume detection.
Note that the bit status does not change until the port is suspended and
that there may be a delay in suspending a port if there is a transaction
currently in progress on the USB.

However, in NXP USBDR controller, the PORTSCx[SUSP] bit changes immediately
when the application sets it and not when the port is actually suspended.

So the application must wait for at least 10 milliseconds after a port
indicates that it is suspended, to make sure this port has entered
suspended state before initiating this port resume using the Force Port
Resume bit. This bit is for NXP controller, not EHCI compatible.

Signed-off-by: Changming Huang 
Signed-off-by: Ramneek Mehresh 
---
Changes in v3:
  - add 10ms delay in function ehci_hub_control
  - fix typos 
Changes in v2:
  - move sleep out of spin-lock
  - add more comment for this workaround

 drivers/usb/host/ehci-fsl.c  |3 +++
 drivers/usb/host/ehci-hub.c  |   11 +++
 drivers/usb/host/ehci.h  |8 
 drivers/usb/host/fsl-mph-dr-of.c |2 ++
 include/linux/fsl_devices.h  |1 +
 5 files changed, 25 insertions(+)

diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c
index 9f5ffb6..91701cc 100644
--- a/drivers/usb/host/ehci-fsl.c
+++ b/drivers/usb/host/ehci-fsl.c
@@ -286,6 +286,9 @@ static int ehci_fsl_usb_setup(struct ehci_hcd *ehci)
if (pdata->has_fsl_erratum_a005275 == 1)
ehci->has_fsl_hs_errata = 1;
 
+   if (pdata->has_fsl_erratum_a005697 == 1)
+   ehci->has_fsl_susp_errata = 1;
+
if ((pdata->operating_mode == FSL_USB2_DR_HOST) ||
(pdata->operating_mode == FSL_USB2_DR_OTG))
if (ehci_fsl_setup_phy(hcd, pdata->phy_mode, 0))
diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
index 74f62d6..d79b824 100644
--- a/drivers/usb/host/ehci-hub.c
+++ b/drivers/usb/host/ehci-hub.c
@@ -310,6 +310,14 @@ static int ehci_bus_suspend (struct usb_hcd *hcd)
}
spin_unlock_irq(>lock);
 
+   if (changed && ehci_has_fsl_susp_errata(ehci))
+   /*
+* Wait for at least 10 millisecondes to ensure the controller
+* enter the suspend status before initiating a port resume
+* using the Force Port Resume bit (Not-EHCI compatible).
+*/
+   usleep_range(1, 2);
+
if ((changed && ehci->has_tdi_phy_lpm) || fs_idle_delay) {
/*
 * Wait for HCD to enter low-power mode or for the bus
@@ -1200,6 +1208,9 @@ int ehci_hub_control(
wIndex, (temp1 & HOSTPC_PHCD) ?
"succeeded" : "failed");
}
+   if (ehci_has_fsl_susp_errata(ehci))
+   /* 10ms for HCD enter suspend */
+   usleep_range(1, 2);
set_bit(wIndex, >suspended_ports);
break;
case USB_PORT_FEAT_POWER:
diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index 3f3b74a..a8e3617 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -219,6 +219,7 @@ struct ehci_hcd {   /* one per controller */
unsignedno_selective_suspend:1;
unsignedhas_fsl_port_bug:1; /* FreeScale */
unsignedhas_fsl_hs_errata:1;/* Freescale HS quirk */
+   unsignedhas_fsl_susp_errata:1;  /* NXP SUSP quirk */
unsignedbig_endian_mmio:1;
unsignedbig_endian_desc:1;
unsignedbig_endian_capbase:1;
@@ -710,6 +711,13 @@ struct ehci_tt {
 #endif
 
 /*
+ * Some Freescale/NXP processors have an erratum (USB A-005697)
+ * in which we need to wait for 10ms for bus to enter suspend mode
+ * after setting SUSP bit.
+ */
+#define ehci_has_fsl_susp_errata(e)((e)->has_fsl_susp_errata)
+
+/*
  * While most USB host controllers implement their registers in
  * little-endian format, a minority (celleb companion chip) implement
  * them in big endian format.
diff --git a/drivers/usb/host/fsl-mph-dr-of.c b/drivers/usb/host/fsl-mph-dr-of.c
index f07ccb2..e90ddb5 100644
--- a/drivers/usb/host/fsl-mph-dr-of.c
+++ b/drivers/usb/host/fsl-mph-dr-of.c
@@ -226,6 +226,8 @@ static int fsl_usb2_mph_dr_of_probe(struct platform_device 
*ofdev)
of_property_read_bool(np, "fsl,usb-erratum-a007792");
pdata->has_fsl_erratum_a005275 =
of_property_read_bool(np, "fsl,usb-erratum-a005275");
+   pdata->has_fsl_erratum_a005697 =
+   of_property_read_bool(np, "fsl,usb_erratum-a005697");
 
/*
 * Determine 

Re: [PATCH] usb: gadget: uvc: fix returnvar.cocci warnings

[Christoph Hellwig]

On Wed, Nov 23, 2016 at 09:35:36AM +0100, Andrzej Pietrasiewicz wrote:
> The ->drop_item() is indeed a void function, the ->drop_link() is
> actually not. This, together with the fact that the value of ->drop_link()
> is silently ignored suggests, that it is the ->drop_link() return
> type that should be corrected and changed to void.

Please send a patch to change it.
--
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