Re: [PATCH v5] xhci : AMD Promontory USB disable port support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Sep 19, 2017 at 05:40:04PM +0800, Joe Lee wrote:
> From: asmtswfae <asmt.swfae@xxxxxxxxx>

Shouldn't the name here match the name up in your From: email line?

> 
> For AMD Promontory xHCI host, although you can disable USB 2.0 ports in BIOS
> settings, those ports will be enabled anyway after you remove a device on
> that port and re-plug it in again. It's a known limitation of the chip.
> As a workaround we can clear the PORT_WAKE_BITS.
> 
> Signed-off-by: asmtswfae <asmt.swfae@xxxxxxxxx>

Same here, use your "real" name.

> v5: Add check disable port setting before set PORT_WAKE_BITS
> v4: Remove the patch code in case USB_PORT_FEAT_REMOTE_WAKE_MASK
> v3: Fix some checkpatch.pl

These v: lines go below the --- line please.

> 
> 
> 
> ---
>  drivers/usb/host/pci-quirks.c | 117 +++++++++++++++++++++++++++++++++++++++++-
>  drivers/usb/host/pci-quirks.h |   1 +
>  drivers/usb/host/xhci-hub.c   |   7 ++-
>  3 files changed, 122 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
> index 658d9d1..5926201 100644
> --- a/drivers/usb/host/pci-quirks.c
> +++ b/drivers/usb/host/pci-quirks.c
> @@ -64,7 +64,22 @@
>  #define	AB_DATA(addr)		((addr) + 0x04)
>  #define	AX_INDXC		0x30
>  #define	AX_DATAC		0x34
> -
> +#define PT_ADDR_INDEX		0xE8
> +#define PT_READ_INDEX		0xE4
> +#define PT_SIG_1_ADDR		0xA520
> +#define PT_SIG_2_ADDR		0xA521
> +#define PT_SIG_3_ADDR		0xA522
> +#define PT_SIG_4_ADDR		0xA523
> +#define PT_SIG_1_DATA		0x78
> +#define PT_SIG_2_DATA		0x56
> +#define PT_SIG_3_DATA		0x34
> +#define PT_SIG_4_DATA		0x12
> +#define PT_4_PORT_1_REG		0xB521
> +#define PT_4_PORT_2_REG		0xB522
> +#define PT_2_PORT_1_REG		0xD520
> +#define PT_2_PORT_2_REG		0xD521
> +#define PT_1_PORT_1_REG		0xD522
> +#define PT_1_PORT_2_REG		0xD523
>  #define	NB_PCIE_INDX_ADDR	0xe0
>  #define	NB_PCIE_INDX_DATA	0xe4
>  #define	PCIE_P_CNTL		0x10040
> @@ -511,6 +526,106 @@ void usb_amd_dev_put(void)
>  }
>  EXPORT_SYMBOL_GPL(usb_amd_dev_put);
>  
> +int usb_amd_pt_check_port(struct device *device, int port)
> +{
> +	unsigned char value;
> +	struct pci_dev *pdev;
> +
> +	pdev = to_pci_dev(device);
> +	pci_write_config_word(pdev, PT_ADDR_INDEX, PT_SIG_1_ADDR);
> +	pci_read_config_byte(pdev, PT_READ_INDEX, &value);
> +	if (value != PT_SIG_1_DATA)
> +		return 0;
> +	pci_write_config_word(pdev, PT_ADDR_INDEX, PT_SIG_2_ADDR);
> +	pci_read_config_byte(pdev, PT_READ_INDEX, &value);
> +	if (value != PT_SIG_2_DATA)
> +		return 0;
> +	pci_write_config_word(pdev, PT_ADDR_INDEX, PT_SIG_3_ADDR);
> +	pci_read_config_byte(pdev, PT_READ_INDEX, &value);
> +	if (value != PT_SIG_3_DATA)
> +		return 0;
> +	pci_write_config_word(pdev, PT_ADDR_INDEX, PT_SIG_4_ADDR);
> +	pci_read_config_byte(pdev, PT_READ_INDEX, &value);
> +	if (value != PT_SIG_4_DATA)
> +		return 0;
> +	if ((pdev->device == 0x43b9) || (pdev->device == 0x43ba)) {
> +		/* device is AMD_PROMONTORYA_4(0x43b9) or
> +		 * PROMONTORYA_3(0x43ba)
> +		 * disable port setting PT_4_PORT_1_REG[7..1] is
> +		 * USB2.0 port6 to 0
> +		 * PT_4_PORT_2_REG[6..0] is USB 13 to port 7
> +		 * 0 : disable ;1 : enable
> +		 */
> +		if (port > 6) {
> +			pci_write_config_word(
> +			pdev, PT_ADDR_INDEX, PT_4_PORT_2_REG);
> +			pci_read_config_byte(pdev, PT_READ_INDEX, &value);
> +			if (value & (1<<(port - 7)))
> +				return 0;
> +			else
> +				return 1;
> +		} else {
> +			pci_write_config_word(
> +			pdev, PT_ADDR_INDEX, PT_4_PORT_1_REG);
> +			pci_read_config_byte(pdev, PT_READ_INDEX, &value);
> +			if (value & (1<<(port + 1)))
> +				return 0;
> +			else
> +				return 1;
> +		}
> +	} else if (pdev->device == 0x43bb) {
> +		/* device is AMD_PROMONTORYA_2(0x43bb)
> +		 * disable port setting PT_2_PORT_1_REG[7..5] is
> +		 * USB2.0 port2 to
> +		 * PT_2_PORT_2_REG[5..0] is USB9 to port3
> +		 * 0 : disable ;1 : enable
> +		 */
> +		if (port > 2) {
> +			pci_write_config_word(
> +			pdev, PT_ADDR_INDEX, PT_2_PORT_2_REG);
> +			pci_read_config_byte(pdev, PT_READ_INDEX, &value);
> +			if (value & (1<<(port - 3)))
> +				return 0;
> +			else
> +				return 1;
> +		} else {
> +			pci_write_config_word(
> +			pdev, PT_ADDR_INDEX, PT_2_PORT_1_REG);
> +			pci_read_config_byte(pdev, PT_READ_INDEX, &value);
> +			if (value & (1<<(port + 5)))
> +				return 0;
> +			else
> +				return 1;
> +		}
> +	} else {
> +		/* device is AMD_PROMONTORYA_1(0x43bc)
> +		 * disable port setting PT_1_PORT_1_REG[7..4] is
> +		 * USB2.0 port3 to 0
> +		 * PT_1_PORT_2_REG[5..0] is USB9 to port4
> +		 * 0 : disable ;1 : enable
> +		 */
> +		if (port > 3) {
> +			pci_write_config_word(
> +			pdev, PT_ADDR_INDEX, PT_1_PORT_2_REG);

Please fix up the indentation of these types of lines, it doesn't make
any sense as-is.


> +			pci_read_config_byte(pdev, PT_READ_INDEX, &value);
> +			if (value & (1<<(port - 4)))
> +				return 0;
> +			else
> +				return 1;
> +		} else {
> +			pci_write_config_word(
> +			pdev, PT_ADDR_INDEX, PT_1_PORT_1_REG);
> +			pci_read_config_byte(
> +			pdev, PT_READ_INDEX, &value);
> +			if (value & (1<<(port + 4)))
> +				return 0;
> +			else
> +				return 1;
> +		}
> +	}
> +}
> +EXPORT_SYMBOL_GPL(usb_amd_pt_check_port);
> +
>  /*
>   * Make sure the controller is completely inactive, unable to
>   * generate interrupts or do DMA.
> diff --git a/drivers/usb/host/pci-quirks.h b/drivers/usb/host/pci-quirks.h
> index 5582cba..b883084 100644
> --- a/drivers/usb/host/pci-quirks.h
> +++ b/drivers/usb/host/pci-quirks.h
> @@ -9,6 +9,7 @@ int usb_hcd_amd_remote_wakeup_quirk(struct pci_dev *pdev);
>  bool usb_amd_hang_symptom_quirk(void);
>  bool usb_amd_prefetch_quirk(void);
>  void usb_amd_dev_put(void);
> +int usb_amd_pt_check_port(struct device *device, int port);
>  void usb_amd_quirk_pll_disable(void);
>  void usb_amd_quirk_pll_enable(void);
>  void usb_asmedia_modifyflowcontrol(struct pci_dev *pdev);
> diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
> index ad89a6d..efb689a 100644
> --- a/drivers/usb/host/xhci-hub.c
> +++ b/drivers/usb/host/xhci-hub.c
> @@ -1507,8 +1507,11 @@ int xhci_bus_suspend(struct usb_hcd *hcd)
>  				t2 &= ~PORT_WKDISC_E;
>  			}
>  			if ((xhci->quirks & XHCI_U2_DISABLE_WAKE) &&
> -			    (hcd->speed < HCD_USB3))
> -				t2 &= ~PORT_WAKE_BITS;
> +			(hcd->speed < HCD_USB3)) {

Odd indentation :(

> +				if (usb_amd_pt_check_port(
> +				hcd->self.controller, port_index))
> +					t2 &= ~PORT_WAKE_BITS;

Also horrid indentation, please make this readable.

thanks,

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



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux