On Fri, Dec 08, 2017 at 05:49:41PM +0800, [email protected] wrote:
> From: "yinbo.zhu" <[email protected]>
>
> Description: This is a occasional problem where the software
No need for a "Description:" word. That's just assumed here, right?
> issues an End Transfer command while a USB transfer is in progress,
> resulting in the TxFIFO being flushed when the lower layer is waiting
> for data,causing the super speed (SS) transmit to get blocked.
> If the End Transfer command is issued on an IN endpoint to
> flush out the pending transfers when the same IN endpoint
> is doing transfers on the USB, then depending upon the timing
> of the End Transfer (and the resulting internal FIFO flush),the
> lower layer (U3PTL/U3MAC) could get stuck waiting for data
> indefinitely. This blocks the transmission path on the SS, and no
> DP/ACK/ERDY/DEVNOTIF packets can be sent from the device.
> Impact: If this issue happens and the transmission gets blocked,
> then the USB host aborts and resets/re-enumerates the device.
> This unblocks the transmitt engine and the device functions normally.
>
> Workaround: Software must wait for all existing TRBs to complete before
> issuing End transfer command.
>
> Configs Affected:
> LS1088-48A-R1.0, LS2081A-R1.1, LS2088-48A-R1.0, LS2088-48A-R1.1,
> LX2160-2120-2080A-R1.
What are these Configs? That doesn't seem to match up with anything
that is in the kernel tree that I can see.
>
> Signed-off-by: yinbo.zhu <[email protected]>
> ---
> drivers/usb/dwc3/core.c | 3 +++
> drivers/usb/dwc3/core.h | 3 +++
> drivers/usb/dwc3/host.c | 3 +++
> drivers/usb/host/xhci-plat.c | 4 ++++
> drivers/usb/host/xhci.c | 24 ++++++++++++++++++------
> drivers/usb/host/xhci.h | 1 +
> 6 files changed, 32 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 5cb3f6795b0b..071e7cea8cbb 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1106,6 +1106,9 @@ static void dwc3_get_properties(struct dwc3 *dwc)
>
> dwc->quirk_reverse_in_out = device_property_read_bool(dev,
> "snps,quirk_reverse_in_out");
> + dwc->quirk_stop_transfer_in_block = device_property_read_bool(dev,
> + "snps,quirk_stop_transfer_in_block");
Have you documented this new DT value somewhere?
> +
> dwc->needs_fifo_resize = of_property_read_bool(node, "tx-fifo-resize");
>
> dwc->configure_gfladj =
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 6c530cbedf49..b2425799ecb6 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -900,6 +900,8 @@ struct dwc3_scratchpad_array {
> * 3 - Reserved
> * @disable_devinit_u1u2_quirk: disable device-initiated U1/U2 request.
> * @quirk_reverse_in_out: prevent tx fifo reverse the data direction.
> + * @quirk_stop_transfer_in_block: prevent block transmission from being
> + * interrupted.
> * @imod_interval: set the interrupt moderation interval in 250ns
> * increments or 0 to disable.
> */
> @@ -1063,6 +1065,7 @@ struct dwc3 {
> unsigned tx_de_emphasis:2;
> unsigned disable_devinit_u1u2_quirk:1;
> unsigned quirk_reverse_in_out:1;
> + unsigned quirk_stop_transfer_in_block:1;
>
> u16 imod_interval;
> };
> diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
> index 2cd48633d3fa..a9ccbf1b9871 100644
> --- a/drivers/usb/dwc3/host.c
> +++ b/drivers/usb/dwc3/host.c
> @@ -110,6 +110,9 @@ int dwc3_host_init(struct dwc3 *dwc)
> if (dwc->quirk_reverse_in_out)
> props[prop_idx++].name = "quirk-reverse-in-out";
>
> + if (dwc->quirk_stop_transfer_in_block)
> + props[prop_idx++].name = "quirk-stop-transfer-in-block";
> +
> if (dwc->usb3_lpm_capable)
> props[prop_idx++].name = "usb3-lpm-capable";
>
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index d1c1e882e6d7..5721d4ece625 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -272,6 +272,10 @@ static int xhci_plat_probe(struct platform_device *pdev)
> if (device_property_read_bool(&pdev->dev, "quirk-reverse-in-out"))
> xhci->quirks |= XHCI_REVERSE_IN_OUT;
>
> + if (device_property_read_bool(&pdev->dev,
> + "quirk-stop-transfer-in-block"))
> + xhci->quirks |= XHCI_STOP_TRANSFER_IN_BLOCK;
> +
> if (device_property_read_bool(&pdev->dev, "quirk-broken-port-ped"))
> xhci->quirks |= XHCI_BROKEN_PORT_PED;
>
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 21dd1d98508f..925c8d171c0b 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -1515,13 +1515,25 @@ static int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
> ret = -ENOMEM;
> goto done;
> }
> - ep->ep_state |= EP_STOP_CMD_PENDING;
> - ep->stop_cmd_timer.expires = jiffies +
> + /*
> + *A-009611: Issuing an End Transfer command on an IN endpoint.
> + *when a transfer is in progress on USB blocks the transmission
> + *Workaround: Software must wait for all existing TRBs to
> + *complete before issuing End transfer command.
What is "A-009611:" mean?
Also please properly format your comments (look at other ones for
examples of how to do it.)
> + */
> + if ((ep_ring->enqueue == ep_ring->dequeue &&
> + (xhci->quirks & XHCI_STOP_TRANSFER_IN_BLOCK)) ||
> + !(xhci->quirks & XHCI_STOP_TRANSFER_IN_BLOCK)) {
> + ep->ep_state |= EP_STOP_CMD_PENDING;
> + ep->stop_cmd_timer.expires = jiffies +
> XHCI_STOP_EP_CMD_TIMEOUT * HZ;
> - add_timer(&ep->stop_cmd_timer);
> - xhci_queue_stop_endpoint(xhci, command, urb->dev->slot_id,
> - ep_index, 0);
> - xhci_ring_cmd_db(xhci);
> + add_timer(&ep->stop_cmd_timer);
> + xhci_queue_stop_endpoint(xhci, command,
> + urb->dev->slot_id,
> + ep_index, 0);
> + xhci_ring_cmd_db(xhci);
> + }
> +
> }
> done:
> spin_unlock_irqrestore(&xhci->lock, flags);
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index 78d14ff0b811..bff47d6582a8 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -1836,6 +1836,7 @@ struct xhci_hcd {
> /* Reserved. It was XHCI_U2_DISABLE_WAKE */
> #define XHCI_ASMEDIA_MODIFY_FLOWCONTROL (1 << 28)
> #define XHCI_HW_LPM_DISABLE (1 << 29)
> +#define XHCI_STOP_TRANSFER_IN_BLOCK (1 << 30)
Meta-comment, why are these not using the BIT() macro?
thanks,
greg k-h
Hi,
Greg Kroah-Hartman <[email protected]> writes:
> On Fri, Dec 08, 2017 at 05:49:41PM +0800, [email protected] wrote:
>> From: "yinbo.zhu" <[email protected]>
>>
>> Description: This is a occasional problem where the software
>
> No need for a "Description:" word. That's just assumed here, right?
>
>> issues an End Transfer command while a USB transfer is in progress,
>> resulting in the TxFIFO being flushed when the lower layer is waiting
>> for data,causing the super speed (SS) transmit to get blocked.
>> If the End Transfer command is issued on an IN endpoint to
>> flush out the pending transfers when the same IN endpoint
>> is doing transfers on the USB, then depending upon the timing
>> of the End Transfer (and the resulting internal FIFO flush),the
>> lower layer (U3PTL/U3MAC) could get stuck waiting for data
>> indefinitely. This blocks the transmission path on the SS, and no
>> DP/ACK/ERDY/DEVNOTIF packets can be sent from the device.
>> Impact: If this issue happens and the transmission gets blocked,
>> then the USB host aborts and resets/re-enumerates the device.
>> This unblocks the transmitt engine and the device functions normally.
>>
>> Workaround: Software must wait for all existing TRBs to complete before
>> issuing End transfer command.
>>
>> Configs Affected:
>> LS1088-48A-R1.0, LS2081A-R1.1, LS2088-48A-R1.0, LS2088-48A-R1.1,
>> LX2160-2120-2080A-R1.
>
> What are these Configs? That doesn't seem to match up with anything
> that is in the kernel tree that I can see.
>
>>
>> Signed-off-by: yinbo.zhu <[email protected]>
>> ---
>> drivers/usb/dwc3/core.c | 3 +++
>> drivers/usb/dwc3/core.h | 3 +++
>> drivers/usb/dwc3/host.c | 3 +++
>> drivers/usb/host/xhci-plat.c | 4 ++++
>> drivers/usb/host/xhci.c | 24 ++++++++++++++++++------
>> drivers/usb/host/xhci.h | 1 +
>> 6 files changed, 32 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 5cb3f6795b0b..071e7cea8cbb 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -1106,6 +1106,9 @@ static void dwc3_get_properties(struct dwc3 *dwc)
>>
>> dwc->quirk_reverse_in_out = device_property_read_bool(dev,
>> "snps,quirk_reverse_in_out");
This was generated on vendor tree. This quirk doesn't exist in
dwc3. Also, update your tree and review MAINTAINERS file. It has been
almost 2 years since I left TI :-)
--
balbi
-----Original Message-----
From: Felipe Balbi [mailto:[email protected]]
Sent: Friday, December 08, 2017 6:44 PM
To: Greg Kroah-Hartman <[email protected]>; Yinbo Zhu <[email protected]>
Cc: Mathias Nyman <[email protected]>; open list:DESIGNWARE USB3 DRD IP DRIVER <[email protected]>; open list:DESIGNWARE USB3 DRD IP DRIVER <[email protected]>; open list <[email protected]>; Xiaobo Xie <[email protected]>; Jerry Huang <[email protected]>; Ran Wang <[email protected]>
Subject: Re: [PATCH v2] usb: host: Implement workaround for Erratum A-009611
>Hi,
>Greg Kroah-Hartman <[email protected]> writes:
> On Fri, Dec 08, 2017 at 05:49:41PM +0800, [email protected] wrote:
>> From: "yinbo.zhu" <[email protected]>
>>
>> Description: This is a occasional problem where the software
>
> No need for a "Description:" word. That's just assumed here, right?
I will remove "Description:" thanks.
>> issues an End Transfer command while a USB transfer is in progress,
>> resulting in the TxFIFO being flushed when the lower layer is
>> waiting for data,causing the super speed (SS) transmit to get blocked.
>> If the End Transfer command is issued on an IN endpoint to flush out
>> the pending transfers when the same IN endpoint is doing transfers on
>> the USB, then depending upon the timing of the End Transfer (and the
>> resulting internal FIFO flush),the lower layer (U3PTL/U3MAC) could
>> get stuck waiting for data indefinitely. This blocks the transmission
>> path on the SS, and no DP/ACK/ERDY/DEVNOTIF packets can be sent from
>> the device.
>> Impact: If this issue happens and the transmission gets blocked, then
>> the USB host aborts and resets/re-enumerates the device.
>> This unblocks the transmitt engine and the device functions normally.
>>
>> Workaround: Software must wait for all existing TRBs to complete
>> before issuing End transfer command.
>>
>> Configs Affected:
>> LS1088-48A-R1.0, LS2081A-R1.1, LS2088-48A-R1.0, LS2088-48A-R1.1,
>> LX2160-2120-2080A-R1.
>
> What are these Configs? That doesn't seem to match up with anything
> that is in the kernel tree that I can see.
These configs is soc information, I don't enable it on these platform dts.
Although the erratum issue can't be reproduced.
>>
>> Signed-off-by: yinbo.zhu <[email protected]>
>> ---
>> drivers/usb/dwc3/core.c | 3 +++
>> drivers/usb/dwc3/core.h | 3 +++
>> drivers/usb/dwc3/host.c | 3 +++
>> drivers/usb/host/xhci-plat.c | 4 ++++
>> drivers/usb/host/xhci.c | 24 ++++++++++++++++++------
>> drivers/usb/host/xhci.h | 1 +
>> 6 files changed, 32 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index
>> 5cb3f6795b0b..071e7cea8cbb 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -1106,6 +1106,9 @@ static void dwc3_get_properties(struct dwc3
>> *dwc)
>>
>> dwc->quirk_reverse_in_out = device_property_read_bool(dev,
>> "snps,quirk_reverse_in_out");
>This was generated on vendor tree. This quirk doesn't exist in dwc3. Also,
>update your tree and review MAINTAINERS file. It has been almost 2 years since I left TI :-)
>--
>Balbi
Hi Balbi,
The quirk that I had add it in dwc3. Your meaning is that I can't use quirk to enable or disable the erratum, isn't it? The tree is git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git, I had updated it.
Thanks.
Yinbo
On Mon, Dec 11, 2017 at 03:15:37AM +0000, Yinbo Zhu wrote:
>
>
> -----Original Message-----
> From: Felipe Balbi [mailto:[email protected]]
> Sent: Friday, December 08, 2017 6:44 PM
> To: Greg Kroah-Hartman <[email protected]>; Yinbo Zhu <[email protected]>
> Cc: Mathias Nyman <[email protected]>; open list:DESIGNWARE USB3 DRD IP DRIVER <[email protected]>; open list:DESIGNWARE USB3 DRD IP DRIVER <[email protected]>; open list <[email protected]>; Xiaobo Xie <[email protected]>; Jerry Huang <[email protected]>; Ran Wang <[email protected]>
> Subject: Re: [PATCH v2] usb: host: Implement workaround for Erratum A-009611
>
>
> >Hi,
>
> >Greg Kroah-Hartman <[email protected]> writes:
> > On Fri, Dec 08, 2017 at 05:49:41PM +0800, [email protected] wrote:
> >> From: "yinbo.zhu" <[email protected]>
> >>
> >> Description: This is a occasional problem where the software
> >
> > No need for a "Description:" word. That's just assumed here, right?
>
> I will remove "Description:" thanks.
> >> issues an End Transfer command while a USB transfer is in progress,
> >> resulting in the TxFIFO being flushed when the lower layer is
> >> waiting for data,causing the super speed (SS) transmit to get blocked.
> >> If the End Transfer command is issued on an IN endpoint to flush out
> >> the pending transfers when the same IN endpoint is doing transfers on
> >> the USB, then depending upon the timing of the End Transfer (and the
> >> resulting internal FIFO flush),the lower layer (U3PTL/U3MAC) could
> >> get stuck waiting for data indefinitely. This blocks the transmission
> >> path on the SS, and no DP/ACK/ERDY/DEVNOTIF packets can be sent from
> >> the device.
> >> Impact: If this issue happens and the transmission gets blocked, then
> >> the USB host aborts and resets/re-enumerates the device.
> >> This unblocks the transmitt engine and the device functions normally.
> >>
> >> Workaround: Software must wait for all existing TRBs to complete
> >> before issuing End transfer command.
> >>
> >> Configs Affected:
> >> LS1088-48A-R1.0, LS2081A-R1.1, LS2088-48A-R1.0, LS2088-48A-R1.1,
> >> LX2160-2120-2080A-R1.
> >
> > What are these Configs? That doesn't seem to match up with anything
> > that is in the kernel tree that I can see.
>
> These configs is soc information, I don't enable it on these platform dts.
> Although the erratum issue can't be reproduced.
I do not understand what this means, please explain it a bit better.
thanks,
greg k-h
-----Original Message-----
From: Greg Kroah-Hartman [mailto:[email protected]]
Sent: Monday, December 11, 2017 3:35 PM
To: Yinbo Zhu <[email protected]>
Cc: Felipe Balbi <[email protected]>; Mathias Nyman <[email protected]>; open list:DESIGNWARE USB3 DRD IP DRIVER <[email protected]>; open list:DESIGNWARE USB3 DRD IP DRIVER <[email protected]>; open list <[email protected]>; Xiaobo Xie <[email protected]>; Jerry Huang <[email protected]>; Ran Wang <[email protected]>
Subject: Re: [PATCH v2] usb: host: Implement workaround for Erratum A-009611
On Mon, Dec 11, 2017 at 03:15:37AM +0000, Yinbo Zhu wrote:
>
>
> -----Original Message-----
> From: Felipe Balbi [mailto:[email protected]]
> Sent: Friday, December 08, 2017 6:44 PM
> To: Greg Kroah-Hartman <[email protected]>; Yinbo Zhu
> <[email protected]>
> Cc: Mathias Nyman <[email protected]>; open list:DESIGNWARE USB3
> DRD IP DRIVER <[email protected]>; open list:DESIGNWARE USB3
> DRD IP DRIVER <[email protected]>; open list
> <[email protected]>; Xiaobo Xie <[email protected]>; Jerry
> Huang <[email protected]>; Ran Wang <[email protected]>
> Subject: Re: [PATCH v2] usb: host: Implement workaround for Erratum
> A-009611
>
>
> >Hi,
>
> >Greg Kroah-Hartman <[email protected]> writes:
> > On Fri, Dec 08, 2017 at 05:49:41PM +0800, [email protected] wrote:
> >> From: "yinbo.zhu" <[email protected]>
> >>
> >> Description: This is a occasional problem where the software
> >
> > No need for a "Description:" word. That's just assumed here, right?
>
> I will remove "Description:" thanks.
> >> issues an End Transfer command while a USB transfer is in progress,
> >> resulting in the TxFIFO being flushed when the lower layer is
> >> waiting for data,causing the super speed (SS) transmit to get blocked.
> >> If the End Transfer command is issued on an IN endpoint to flush
> >> out the pending transfers when the same IN endpoint is doing
> >> transfers on the USB, then depending upon the timing of the End
> >> Transfer (and the resulting internal FIFO flush),the lower layer
> >> (U3PTL/U3MAC) could get stuck waiting for data indefinitely. This
> >> blocks the transmission path on the SS, and no DP/ACK/ERDY/DEVNOTIF
> >> packets can be sent from the device.
> >> Impact: If this issue happens and the transmission gets blocked,
> >> then the USB host aborts and resets/re-enumerates the device.
> >> This unblocks the transmitt engine and the device functions normally.
> >>
> >> Workaround: Software must wait for all existing TRBs to complete
> >> before issuing End transfer command.
> >>
> >> Configs Affected:
> >> LS1088-48A-R1.0, LS2081A-R1.1, LS2088-48A-R1.0, LS2088-48A-R1.1,
> >> LX2160-2120-2080A-R1.
> >
> > What are these Configs? That doesn't seem to match up with anything
> > that is in the kernel tree that I can see.
>
> These configs is soc information, I don't enable it on these platform dts.
> Although the erratum issue can't be reproduced.
>I do not understand what this means, please explain it a bit better.
>thanks,
>greg k-h
Maybe I have a problem with your words, Your meaning is that you want to ask me why I didn't add an attribute in the device tree to match kernel for every platform, right?
-----Original Message-----
From: Greg Kroah-Hartman [mailto:[email protected]]
Sent: Friday, December 08, 2017 6:21 PM
To: Yinbo Zhu <[email protected]>
Cc: Felipe Balbi <[email protected]>; Mathias Nyman <[email protected]>; open list:DESIGNWARE USB3 DRD IP DRIVER <[email protected]>; open list:DESIGNWARE USB3 DRD IP DRIVER <[email protected]>; open list <[email protected]>; Xiaobo Xie <[email protected]>; Jerry Huang <[email protected]>; Ran Wang <[email protected]>
Subject: Re: [PATCH v2] usb: host: Implement workaround for Erratum A-009611
On Fri, Dec 08, 2017 at 05:49:41PM +0800, [email protected] wrote:
> From: "yinbo.zhu" <[email protected]>
>
> Description: This is a occasional problem where the software
>No need for a "Description:" word. That's just assumed here, right?
> issues an End Transfer command while a USB transfer is in progress,
> resulting in the TxFIFO being flushed when the lower layer is waiting
> for data,causing the super speed (SS) transmit to get blocked.
> If the End Transfer command is issued on an IN endpoint to flush out
> the pending transfers when the same IN endpoint is doing transfers on
> the USB, then depending upon the timing of the End Transfer (and the
> resulting internal FIFO flush),the lower layer (U3PTL/U3MAC) could get
> stuck waiting for data indefinitely. This blocks the transmission path
> on the SS, and no DP/ACK/ERDY/DEVNOTIF packets can be sent from the
> device.
> Impact: If this issue happens and the transmission gets blocked, then
> the USB host aborts and resets/re-enumerates the device.
> This unblocks the transmitt engine and the device functions normally.
>
> Workaround: Software must wait for all existing TRBs to complete
> before issuing End transfer command.
>
> Configs Affected:
> LS1088-48A-R1.0, LS2081A-R1.1, LS2088-48A-R1.0, LS2088-48A-R1.1,
> LX2160-2120-2080A-R1.
What are these Configs? That doesn't seem to match up with anything that is in the kernel tree that I can see.
>
> Signed-off-by: yinbo.zhu <[email protected]>
> ---
> drivers/usb/dwc3/core.c | 3 +++
> drivers/usb/dwc3/core.h | 3 +++
> drivers/usb/dwc3/host.c | 3 +++
> drivers/usb/host/xhci-plat.c | 4 ++++
> drivers/usb/host/xhci.c | 24 ++++++++++++++++++------
> drivers/usb/host/xhci.h | 1 +
> 6 files changed, 32 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index
> 5cb3f6795b0b..071e7cea8cbb 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1106,6 +1106,9 @@ static void dwc3_get_properties(struct dwc3
> *dwc)
>
> dwc->quirk_reverse_in_out = device_property_read_bool(dev,
> "snps,quirk_reverse_in_out");
> + dwc->quirk_stop_transfer_in_block = device_property_read_bool(dev,
> + "snps,quirk_stop_transfer_in_block");
>Have you documented this new DT value somewhere?
I had add some description in drivers/usb/dwc3/core.h.
Is it okay?
" + * @quirk_stop_transfer_in_block: prevent block transmission from being
+ * interrupted."
> +
> dwc->needs_fifo_resize = of_property_read_bool(node,
> "tx-fifo-resize");
>
> dwc->configure_gfladj =
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index
> 6c530cbedf49..b2425799ecb6 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -900,6 +900,8 @@ struct dwc3_scratchpad_array {
> * 3 - Reserved
> * @disable_devinit_u1u2_quirk: disable device-initiated U1/U2 request.
> * @quirk_reverse_in_out: prevent tx fifo reverse the data direction.
> + * @quirk_stop_transfer_in_block: prevent block transmission from being
> + * interrupted.
> * @imod_interval: set the interrupt moderation interval in 250ns
> * increments or 0 to disable.
> */
> @@ -1063,6 +1065,7 @@ struct dwc3 {
> unsigned tx_de_emphasis:2;
> unsigned disable_devinit_u1u2_quirk:1;
> unsigned quirk_reverse_in_out:1;
> + unsigned quirk_stop_transfer_in_block:1;
>
> u16 imod_interval;
> };
> diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c index
> 2cd48633d3fa..a9ccbf1b9871 100644
> --- a/drivers/usb/dwc3/host.c
> +++ b/drivers/usb/dwc3/host.c
> @@ -110,6 +110,9 @@ int dwc3_host_init(struct dwc3 *dwc)
> if (dwc->quirk_reverse_in_out)
> props[prop_idx++].name = "quirk-reverse-in-out";
>
> + if (dwc->quirk_stop_transfer_in_block)
> + props[prop_idx++].name = "quirk-stop-transfer-in-block";
> +
> if (dwc->usb3_lpm_capable)
> props[prop_idx++].name = "usb3-lpm-capable";
>
> diff --git a/drivers/usb/host/xhci-plat.c
> b/drivers/usb/host/xhci-plat.c index d1c1e882e6d7..5721d4ece625 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -272,6 +272,10 @@ static int xhci_plat_probe(struct platform_device *pdev)
> if (device_property_read_bool(&pdev->dev, "quirk-reverse-in-out"))
> xhci->quirks |= XHCI_REVERSE_IN_OUT;
>
> + if (device_property_read_bool(&pdev->dev,
> + "quirk-stop-transfer-in-block"))
> + xhci->quirks |= XHCI_STOP_TRANSFER_IN_BLOCK;
> +
> if (device_property_read_bool(&pdev->dev, "quirk-broken-port-ped"))
> xhci->quirks |= XHCI_BROKEN_PORT_PED;
>
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index
> 21dd1d98508f..925c8d171c0b 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -1515,13 +1515,25 @@ static int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
> ret = -ENOMEM;
> goto done;
> }
> - ep->ep_state |= EP_STOP_CMD_PENDING;
> - ep->stop_cmd_timer.expires = jiffies +
> + /*
> + *A-009611: Issuing an End Transfer command on an IN endpoint.
> + *when a transfer is in progress on USB blocks the transmission
> + *Workaround: Software must wait for all existing TRBs to
> + *complete before issuing End transfer command.
>What is "A-009611:" mean?
>Also please properly format your comments (look at other ones for examples of how to do it.)
A-009611 is erratum name, about the comments that I will had a change. Thanks.
> + */
> + if ((ep_ring->enqueue == ep_ring->dequeue &&
> + (xhci->quirks & XHCI_STOP_TRANSFER_IN_BLOCK)) ||
> + !(xhci->quirks & XHCI_STOP_TRANSFER_IN_BLOCK)) {
> + ep->ep_state |= EP_STOP_CMD_PENDING;
> + ep->stop_cmd_timer.expires = jiffies +
> XHCI_STOP_EP_CMD_TIMEOUT * HZ;
> - add_timer(&ep->stop_cmd_timer);
> - xhci_queue_stop_endpoint(xhci, command, urb->dev->slot_id,
> - ep_index, 0);
> - xhci_ring_cmd_db(xhci);
> + add_timer(&ep->stop_cmd_timer);
> + xhci_queue_stop_endpoint(xhci, command,
> + urb->dev->slot_id,
> + ep_index, 0);
> + xhci_ring_cmd_db(xhci);
> + }
> +
> }
> done:
> spin_unlock_irqrestore(&xhci->lock, flags); diff --git
> a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index
> 78d14ff0b811..bff47d6582a8 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -1836,6 +1836,7 @@ struct xhci_hcd {
> /* Reserved. It was XHCI_U2_DISABLE_WAKE */
> #define XHCI_ASMEDIA_MODIFY_FLOWCONTROL (1 << 28)
> #define XHCI_HW_LPM_DISABLE (1 << 29)
> +#define XHCI_STOP_TRANSFER_IN_BLOCK (1 << 30)
>Meta-comment, why are these not using the BIT() macro?
>thanks,
>greg k-h
Okay , I will use the BIT() .
Thanks
yinbo
Hi,
(please break your lines at 80-characters)
Yinbo Zhu <[email protected]> writes:
>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index
>>> 5cb3f6795b0b..071e7cea8cbb 100644
>>> --- a/drivers/usb/dwc3/core.c
>>> +++ b/drivers/usb/dwc3/core.c
>>> @@ -1106,6 +1106,9 @@ static void dwc3_get_properties(struct dwc3
>>> *dwc)
>>>
>>> dwc->quirk_reverse_in_out = device_property_read_bool(dev,
>>> "snps,quirk_reverse_in_out");
>
>>This was generated on vendor tree. This quirk doesn't exist in dwc3. Also,
> >update your tree and review MAINTAINERS file. It has been almost 2 years since I left TI :-)
>
>>--
>>Balbi
>
> Hi Balbi,
>
> The quirk that I had add it in dwc3. Your meaning is that I can't use
> quirk to enable or disable the erratum, isn't it? The tree is
> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git, I had
> updated it.
-*- mode: grep; default-directory: "~/workspace/linux/" -*-
Grep started at Mon Dec 11 10:50:47
git --no-pager grep --color -nH -e quirk_reverse_in_out
Grep finished with no matches found at Mon Dec 11 10:50:48
--
balbi
-----Original Message-----
From: Felipe Balbi [mailto:[email protected]]
Sent: Monday, December 11, 2017 4:52 PM
To: Yinbo Zhu <[email protected]>; Greg Kroah-Hartman <[email protected]>
Cc: Mathias Nyman <[email protected]>; open list:DESIGNWARE USB3 DRD IP DRIVER <[email protected]>; open list:DESIGNWARE USB3 DRD IP DRIVER <[email protected]>; open list <[email protected]>; Xiaobo Xie <[email protected]>; Jerry Huang <[email protected]>; Ran Wang <[email protected]>
Subject: RE: [PATCH v2] usb: host: Implement workaround for Erratum A-009611
>>Hi,
>>(please break your lines at 80-characters)
>>Yinbo Zhu <[email protected]> writes:
I had check it. Every line is less than 80-characters.
>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index
>>> 5cb3f6795b0b..071e7cea8cbb 100644
>>> --- a/drivers/usb/dwc3/core.c
>>> +++ b/drivers/usb/dwc3/core.c
>>> @@ -1106,6 +1106,9 @@ static void dwc3_get_properties(struct dwc3
>>> *dwc)
>>>
>>> dwc->quirk_reverse_in_out = device_property_read_bool(dev,
>>> "snps,quirk_reverse_in_out");
>
>>This was generated on vendor tree. This quirk doesn't exist in dwc3.
>>Also,
> >update your tree and review MAINTAINERS file. It has been almost 2
> years since I left TI :-)
>
>>--
>>Balbi
>
> Hi Balbi,
>
> The quirk that I had add it in dwc3. Your meaning is that I can't use
> quirk to enable or disable the erratum, isn't it? The tree is
> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git, I had
> updated it.
>-*- mode: grep; default-directory: "~/workspace/linux/" -*- Grep started at Mon Dec 11 10:50:47
>git --no-pager grep --color -nH -e quirk_reverse_in_out
>Grep finished with no matches found at Mon Dec 11 10:50:48
>--
>balbi
Hi Balbi,
You can't find the quirk that it is normal. There's no one in the previous code.
The quirk that I added to control the new erratum
Please you note.
Thanks.
Yinbo.