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

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

[Longfang Liu]

A Synopsys USB2.0 core used in Huawei Kunpeng920 SoC has a bug which
might cause the host controller not issuing ping.

The detailed description and solution:
Link: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/commit/?id=2f7ac6c199978d0a0e407a12534201aa675a6482

Signed-off-by: Longfang Liu <liulongfang@huawei.com>
Reviewed-by: Yicong Yang <yangyicong@hisilicon.com>
---
 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

Re: [PATCH] USB: ehci: reopen solution for Synopsys HC bug

[Greg KH]

On Tue, Jun 02, 2020 at 07:14:49PM +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.
> 
> The detailed description and solution:
> Link: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/commit/?id=2f7ac6c199978d0a0e407a12534201aa675a6482

That just points to an existing patch in the tree.  Why link to it here
in another patch?  And that patch is from 2011?

If you want to refer to an id already in the tree, you can do it like:
2f7ac6c19997 ("USB: ehci: add workaround for Synopsys HC bug")

> Signed-off-by: Longfang Liu <liulongfang@huawei.com>
> Reviewed-by: Yicong Yang <yangyicong@hisilicon.com>
> ---
>  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;

Ah, you are adding another "broken" device to the tree, that makes more
sense.  Can you reword the above description a bit?

thanks,

greg k-h

Re: [PATCH] USB: ehci: reopen solution for Synopsys HC bug

[liulongfang]

On 2020/6/2 22:07, Greg KH wrote:
> On Tue, Jun 02, 2020 at 07:14:49PM +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.
>>
>> The detailed description and solution:
>> Link: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/commit/?id=2f7ac6c199978d0a0e407a12534201aa675a6482
> That just points to an existing patch in the tree.  Why link to it here
> in another patch?  And that patch is from 2011?
>
> If you want to refer to an id already in the tree, you can do it like:
> 2f7ac6c19997 ("USB: ehci: add workaround for Synopsys HC bug")

Yes,  It's necessary to add this reference.

I will change it in the next patch.

>> Signed-off-by: Longfang Liu <liulongfang@huawei.com>
>> Reviewed-by: Yicong Yang <yangyicong@hisilicon.com>
>> ---
>>  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;
> Ah, you are adding another "broken" device to the tree, that makes more
> sense.  Can you reword the above description a bit?

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.

>
> thanks,
>
> greg k-h
>
> .

Thanks,

Longfang