diff mbox series

[v2] USB: ehci: reopen solution for Synopsys HC bug

Message ID 1591236678-36009-1-git-send-email-liulongfang@huawei.com (mailing list archive)
State Superseded
Headers show
Series [v2] USB: ehci: reopen solution for Synopsys HC bug | expand

Commit Message

liulongfang June 4, 2020, 2:11 a.m. UTC
A Synopsys USB2.0 core used in Huawei Kunpeng920 SoC has a bug which
might cause the host controller not issuing ping.

Bug description:
After indicating an Interrupt on Async Advance, the software uses the
doorbell mechanism to delete the Next Link queue head of the last
executed queue head. At this time, the host controller still references
the removed queue head(the queue head is NULL). NULL reference causes
the host controller to lose the USB device.

Solution:
After deleting the Next Link queue head, the software can write one of the
valid queue head addresses to the ASYNCLISTADDR register to allow
the host controller to get the valid queue head.

There are detailed instructions and solutions in this patch:
commit 2f7ac6c19997 ("USB: ehci: add workaround for Synopsys HC bug")

Signed-off-by: Longfang Liu <liulongfang@huawei.com>
---

Changes in V2:
- updated comment message

 drivers/usb/host/ehci-pci.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Alan Stern June 4, 2020, 2:26 p.m. UTC | #1
On Thu, Jun 04, 2020 at 10:11:18AM +0800, Longfang Liu wrote:
> A Synopsys USB2.0 core used in Huawei Kunpeng920 SoC has a bug which
> might cause the host controller not issuing ping.
> 
> Bug description:
> After indicating an Interrupt on Async Advance, the software uses the
> doorbell mechanism to delete the Next Link queue head of the last
> executed queue head. At this time, the host controller still references
> the removed queue head(the queue head is NULL). NULL reference causes
> the host controller to lose the USB device.
> 
> Solution:
> After deleting the Next Link queue head, the software can write one of the
> valid queue head addresses to the ASYNCLISTADDR register to allow
> the host controller to get the valid queue head.
> 
> There are detailed instructions and solutions in this patch:
> commit 2f7ac6c19997 ("USB: ehci: add workaround for Synopsys HC bug")
> 
> Signed-off-by: Longfang Liu <liulongfang@huawei.com>

This is a very bad patch description.  Do you understand why?  Because 
it doesn't describe what the patch actually does!

The description talks about the original bug -- what it was and how it 
was fixed.  But this patch doesn't contain the bug fix.  Instead, all 
this patch does is set a quirk flag for the Huawei Kunpeng920.  The 
patch description should explain why the quirk flag needs to be set; it 
doesn't have to explain the original bug.

Alan Stern

> ---
> 
> Changes in V2:
> - updated comment message
> 
>  drivers/usb/host/ehci-pci.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
> index 1a48ab1..7ff2cbd 100644
> --- a/drivers/usb/host/ehci-pci.c
> +++ b/drivers/usb/host/ehci-pci.c
> @@ -216,6 +216,13 @@ static int ehci_pci_setup(struct usb_hcd *hcd)
>  		ehci_info(ehci, "applying MosChip frame-index workaround\n");
>  		ehci->frame_index_bug = 1;
>  		break;
> +	case PCI_VENDOR_ID_HUAWEI:
> +		/* Synopsys HC bug */
> +		if (pdev->device == 0xa239) {
> +			ehci_info(ehci, "applying Synopsys HC workaround\n");
> +			ehci->has_synopsys_hc_bug = 1;
> +		}
> +		break;
>  	}
>  
>  	/* optional debug port, normally in the first BAR */
> -- 
> 2.8.1
>
liulongfang June 5, 2020, 3:44 a.m. UTC | #2
On 2020/6/4 22:26, Alan Stern Wrote:
> On Thu, Jun 04, 2020 at 10:11:18AM +0800, Longfang Liu wrote:
>> A Synopsys USB2.0 core used in Huawei Kunpeng920 SoC has a bug which
>> might cause the host controller not issuing ping.
>>
>> Bug description:
>> After indicating an Interrupt on Async Advance, the software uses the
>> doorbell mechanism to delete the Next Link queue head of the last
>> executed queue head. At this time, the host controller still references
>> the removed queue head(the queue head is NULL). NULL reference causes
>> the host controller to lose the USB device.
>>
>> Solution:
>> After deleting the Next Link queue head, the software can write one of the
>> valid queue head addresses to the ASYNCLISTADDR register to allow
>> the host controller to get the valid queue head.
>>
>> There are detailed instructions and solutions in this patch:
>> commit 2f7ac6c19997 ("USB: ehci: add workaround for Synopsys HC bug")
>>
>> Signed-off-by: Longfang Liu <liulongfang@huawei.com>
> This is a very bad patch description.  Do you understand why?  Because 
> it doesn't describe what the patch actually does!
>
> The description talks about the original bug -- what it was and how it 
> was fixed.  But this patch doesn't contain the bug fix.  Instead, all 
> this patch does is set a quirk flag for the Huawei Kunpeng920.  The 
> patch description should explain why the quirk flag needs to be set; it 
> doesn't have to explain the original bug.
>
> Alan Stern

Solution:
After deleting the Next Link queue head, when has_synopsys_hc_bug set
to 1,the software can write one of the valid queue head addresses to
the ASYNCLISTADDR register to allow the host controller to get
the valid queue head. in order to solve that problem, this patch set
the flag for Huawei Kunpeng920

Thanks.
Longfang

>> ---
>>
>> Changes in V2:
>> - updated comment message
>>
>>  drivers/usb/host/ehci-pci.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
>> index 1a48ab1..7ff2cbd 100644
>> --- a/drivers/usb/host/ehci-pci.c
>> +++ b/drivers/usb/host/ehci-pci.c
>> @@ -216,6 +216,13 @@ static int ehci_pci_setup(struct usb_hcd *hcd)
>>  		ehci_info(ehci, "applying MosChip frame-index workaround\n");
>>  		ehci->frame_index_bug = 1;
>>  		break;
>> +	case PCI_VENDOR_ID_HUAWEI:
>> +		/* Synopsys HC bug */
>> +		if (pdev->device == 0xa239) {
>> +			ehci_info(ehci, "applying Synopsys HC workaround\n");
>> +			ehci->has_synopsys_hc_bug = 1;
>> +		}
>> +		break;
>>  	}
>>  
>>  	/* optional debug port, normally in the first BAR */
>> -- 
>> 2.8.1
>>
> .
>
diff mbox series

Patch

diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
index 1a48ab1..7ff2cbd 100644
--- a/drivers/usb/host/ehci-pci.c
+++ b/drivers/usb/host/ehci-pci.c
@@ -216,6 +216,13 @@  static int ehci_pci_setup(struct usb_hcd *hcd)
 		ehci_info(ehci, "applying MosChip frame-index workaround\n");
 		ehci->frame_index_bug = 1;
 		break;
+	case PCI_VENDOR_ID_HUAWEI:
+		/* Synopsys HC bug */
+		if (pdev->device == 0xa239) {
+			ehci_info(ehci, "applying Synopsys HC workaround\n");
+			ehci->has_synopsys_hc_bug = 1;
+		}
+		break;
 	}
 
 	/* optional debug port, normally in the first BAR */