usb: xhci: Enable LPM for VIA LABS VL805
diff mbox series

Message ID 20200120142422.3907-1-nsaenzjulienne@suse.de
State Accepted
Commit 2170a98d56cc86cef71134ac213aede2fdf64021
Headers show
Series
  • usb: xhci: Enable LPM for VIA LABS VL805
Related show

Commit Message

Nicolas Saenz Julienne Jan. 20, 2020, 2:24 p.m. UTC
This PCIe controller chip is used on the Raspberry Pi 4 and multiple
adapter cards. There is no publicly available documentation for the
chip, yet both the downstream RPi4 kernel and the controller cards
support/advertise LPM support.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
 drivers/usb/host/xhci-pci.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Greg Kroah-Hartman Feb. 10, 2020, 6:59 p.m. UTC | #1
On Mon, Jan 20, 2020 at 03:24:22PM +0100, Nicolas Saenz Julienne wrote:
> This PCIe controller chip is used on the Raspberry Pi 4 and multiple
> adapter cards. There is no publicly available documentation for the
> chip, yet both the downstream RPi4 kernel and the controller cards
> support/advertise LPM support.
> 
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> ---
>  drivers/usb/host/xhci-pci.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index 4917c5b033fa..c1976e98992b 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -241,6 +241,9 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
>  			pdev->device == 0x3432)
>  		xhci->quirks |= XHCI_BROKEN_STREAMS;
>  
> +	if (pdev->vendor == PCI_VENDOR_ID_VIA && pdev->device == 0x3483)
> +		xhci->quirks |= XHCI_LPM_SUPPORT;
> +
>  	if (pdev->vendor == PCI_VENDOR_ID_ASMEDIA &&
>  			pdev->device == 0x1042)
>  		xhci->quirks |= XHCI_BROKEN_STREAMS;

Mathias, is this in your review queue?

thanks,

greg k-h
Mathias Nyman Feb. 11, 2020, 9:34 a.m. UTC | #2
On 10.2.2020 20.59, Greg Kroah-Hartman wrote:
> On Mon, Jan 20, 2020 at 03:24:22PM +0100, Nicolas Saenz Julienne wrote:
>> This PCIe controller chip is used on the Raspberry Pi 4 and multiple
>> adapter cards. There is no publicly available documentation for the
>> chip, yet both the downstream RPi4 kernel and the controller cards
>> support/advertise LPM support.
>>
>> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
>> ---
>>  drivers/usb/host/xhci-pci.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
>> index 4917c5b033fa..c1976e98992b 100644
>> --- a/drivers/usb/host/xhci-pci.c
>> +++ b/drivers/usb/host/xhci-pci.c
>> @@ -241,6 +241,9 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
>>  			pdev->device == 0x3432)
>>  		xhci->quirks |= XHCI_BROKEN_STREAMS;
>>  
>> +	if (pdev->vendor == PCI_VENDOR_ID_VIA && pdev->device == 0x3483)
>> +		xhci->quirks |= XHCI_LPM_SUPPORT;
>> +
>>  	if (pdev->vendor == PCI_VENDOR_ID_ASMEDIA &&
>>  			pdev->device == 0x1042)
>>  		xhci->quirks |= XHCI_BROKEN_STREAMS;
> 
> Mathias, is this in your review queue?
> 

Ah yes, before adding link power management support for this controller we
should check that it has sane (or any) exit latency values set in its
HCSPARAMS3 capability register.

Nicolas, if you have this controller could you show the capability registers:

cat /sys/kernel/debug/usb/xhci/*/reg-cap

-Mathias
Stefan Wahren Feb. 11, 2020, 9:49 a.m. UTC | #3
Hi Mathias,

On 11.02.20 10:34, Mathias Nyman wrote:
> On 10.2.2020 20.59, Greg Kroah-Hartman wrote:
>> On Mon, Jan 20, 2020 at 03:24:22PM +0100, Nicolas Saenz Julienne wrote:
>>> This PCIe controller chip is used on the Raspberry Pi 4 and multiple
>>> adapter cards. There is no publicly available documentation for the
>>> chip, yet both the downstream RPi4 kernel and the controller cards
>>> support/advertise LPM support.
>>>
>>> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
>>> ---
>>>  drivers/usb/host/xhci-pci.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
>>> index 4917c5b033fa..c1976e98992b 100644
>>> --- a/drivers/usb/host/xhci-pci.c
>>> +++ b/drivers/usb/host/xhci-pci.c
>>> @@ -241,6 +241,9 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
>>>  			pdev->device == 0x3432)
>>>  		xhci->quirks |= XHCI_BROKEN_STREAMS;
>>>  
>>> +	if (pdev->vendor == PCI_VENDOR_ID_VIA && pdev->device == 0x3483)
>>> +		xhci->quirks |= XHCI_LPM_SUPPORT;
>>> +
>>>  	if (pdev->vendor == PCI_VENDOR_ID_ASMEDIA &&
>>>  			pdev->device == 0x1042)
>>>  		xhci->quirks |= XHCI_BROKEN_STREAMS;
>> Mathias, is this in your review queue?
>>
> Ah yes, before adding link power management support for this controller we
> should check that it has sane (or any) exit latency values set in its
> HCSPARAMS3 capability register.
>
> Nicolas, if you have this controller could you show the capability registers:
>
> cat /sys/kernel/debug/usb/xhci/*/reg-cap
sorry for the naive question, but do you need the dump with or without
this patch applied?
>
> -Mathias
>
> _______________________________________________
> linux-rpi-kernel mailing list
> linux-rpi-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rpi-kernel
Nicolas Saenz Julienne Feb. 11, 2020, 10:02 a.m. UTC | #4
Hi Stefan, Mathias.

On Tue, 2020-02-11 at 10:49 +0100, Stefan Wahren wrote:
> Hi Mathias,
> 
> On 11.02.20 10:34, Mathias Nyman wrote:
> > On 10.2.2020 20.59, Greg Kroah-Hartman wrote:
> > > On Mon, Jan 20, 2020 at 03:24:22PM +0100, Nicolas Saenz Julienne wrote:
> > > > This PCIe controller chip is used on the Raspberry Pi 4 and multiple
> > > > adapter cards. There is no publicly available documentation for the
> > > > chip, yet both the downstream RPi4 kernel and the controller cards
> > > > support/advertise LPM support.
> > > > 
> > > > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > > > ---
> > > >  drivers/usb/host/xhci-pci.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > > 
> > > > diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> > > > index 4917c5b033fa..c1976e98992b 100644
> > > > --- a/drivers/usb/host/xhci-pci.c
> > > > +++ b/drivers/usb/host/xhci-pci.c
> > > > @@ -241,6 +241,9 @@ static void xhci_pci_quirks(struct device *dev,
> > > > struct xhci_hcd *xhci)
> > > >  			pdev->device == 0x3432)
> > > >  		xhci->quirks |= XHCI_BROKEN_STREAMS;
> > > >  
> > > > +	if (pdev->vendor == PCI_VENDOR_ID_VIA && pdev->device == 0x3483)
> > > > +		xhci->quirks |= XHCI_LPM_SUPPORT;
> > > > +
> > > >  	if (pdev->vendor == PCI_VENDOR_ID_ASMEDIA &&
> > > >  			pdev->device == 0x1042)
> > > >  		xhci->quirks |= XHCI_BROKEN_STREAMS;
> > > Mathias, is this in your review queue?
> > > 
> > Ah yes, before adding link power management support for this controller we
> > should check that it has sane (or any) exit latency values set in its
> > HCSPARAMS3 capability register.

I did some checks myself before sending the patch, and tested with some devices
I own. The latencies seemd reasonable. For example I just hooked up an USB3 HD,
the root HUB exposes:

	bU1DevExitLat           4 micro seconds
	bU2DevExitLat         231 micro seconds

And xhci configured the device with:

	bU1DevExitLat          10 micro seconds
	bU2DevExitLat        2047 micro seconds

> > Nicolas, if you have this controller could you show the capability
> > registers:
> > 
> > cat /sys/kernel/debug/usb/xhci/*/reg-cap

CAPLENGTH = 0x01000020
HCSPARAMS1 = 0x05000420
HCSPARAMS2 = 0xfc000031
HCSPARAMS3 = 0x00e70004
HCCPARAMS1 = 0x002841eb
DOORBELLOFF = 0x00000100
RUNTIMEOFF = 0x00000200
HCCPARAMS2 = 0x00000000

> sorry for the naive question, but do you need the dump with or without
> this patch applied?

IIRC these are dumps from xhci's extended registers. Shouldn't matter at all. 

Regards,
Nicolas
Mathias Nyman Feb. 11, 2020, 12:13 p.m. UTC | #5
On 11.2.2020 12.02, Nicolas Saenz Julienne wrote:
> Hi Stefan, Mathias.
> 
> On Tue, 2020-02-11 at 10:49 +0100, Stefan Wahren wrote:
>> Hi Mathias,
>>
>> On 11.02.20 10:34, Mathias Nyman wrote:
>>> On 10.2.2020 20.59, Greg Kroah-Hartman wrote:
>>>> On Mon, Jan 20, 2020 at 03:24:22PM +0100, Nicolas Saenz Julienne wrote:
>>>>> This PCIe controller chip is used on the Raspberry Pi 4 and multiple
>>>>> adapter cards. There is no publicly available documentation for the
>>>>> chip, yet both the downstream RPi4 kernel and the controller cards
>>>>> support/advertise LPM support.
>>>>>
>>>>> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
>>>>> ---
>>>>>  drivers/usb/host/xhci-pci.c | 3 +++
>>>>>  1 file changed, 3 insertions(+)
>>>>>
>>>>> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
>>>>> index 4917c5b033fa..c1976e98992b 100644
>>>>> --- a/drivers/usb/host/xhci-pci.c
>>>>> +++ b/drivers/usb/host/xhci-pci.c
>>>>> @@ -241,6 +241,9 @@ static void xhci_pci_quirks(struct device *dev,
>>>>> struct xhci_hcd *xhci)
>>>>>  			pdev->device == 0x3432)
>>>>>  		xhci->quirks |= XHCI_BROKEN_STREAMS;
>>>>>  
>>>>> +	if (pdev->vendor == PCI_VENDOR_ID_VIA && pdev->device == 0x3483)
>>>>> +		xhci->quirks |= XHCI_LPM_SUPPORT;
>>>>> +
>>>>>  	if (pdev->vendor == PCI_VENDOR_ID_ASMEDIA &&
>>>>>  			pdev->device == 0x1042)
>>>>>  		xhci->quirks |= XHCI_BROKEN_STREAMS;
>>>> Mathias, is this in your review queue?
>>>>
>>> Ah yes, before adding link power management support for this controller we
>>> should check that it has sane (or any) exit latency values set in its
>>> HCSPARAMS3 capability register.
> 
> I did some checks myself before sending the patch, and tested with some devices
> I own. The latencies seemd reasonable. For example I just hooked up an USB3 HD,
> the root HUB exposes:
> 
> 	bU1DevExitLat           4 micro seconds
> 	bU2DevExitLat         231 micro seconds
> 
> And xhci configured the device with:
> 
> 	bU1DevExitLat          10 micro seconds
> 	bU2DevExitLat        2047 micro seconds
> 
>>> Nicolas, if you have this controller could you show the capability
>>> registers:
>>>
>>> cat /sys/kernel/debug/usb/xhci/*/reg-cap
> 
> CAPLENGTH = 0x01000020
> HCSPARAMS1 = 0x05000420
> HCSPARAMS2 = 0xfc000031
> HCSPARAMS3 = 0x00e70004

Thanks, looks sane, U1 Device exit latency is 4us, and U2 is 231us, and as
showed above these were set correctly to the roothub.

Greg, if you want you can pick this patch as is, otherwise I'll send it later
with other usb-next patches.

Acked-by: Mathias Nyman <mathias.nyman@linux.intel.com>

Patch
diff mbox series

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 4917c5b033fa..c1976e98992b 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -241,6 +241,9 @@  static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
 			pdev->device == 0x3432)
 		xhci->quirks |= XHCI_BROKEN_STREAMS;
 
+	if (pdev->vendor == PCI_VENDOR_ID_VIA && pdev->device == 0x3483)
+		xhci->quirks |= XHCI_LPM_SUPPORT;
+
 	if (pdev->vendor == PCI_VENDOR_ID_ASMEDIA &&
 			pdev->device == 0x1042)
 		xhci->quirks |= XHCI_BROKEN_STREAMS;