Linux-USB Archive on lore.kernel.org
 help / color / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
Cc: Felipe Balbi <balbi@kernel.org>,
	linux-usb@vger.kernel.org,
	Mathias Nyman <mathias.nyman@intel.com>,
	John Youn <John.Youn@synopsys.com>
Subject: Re: [PATCH 1/6] usb: xhci: Move quirks definitions to common usb header
Date: Fri, 9 Apr 2021 08:50:04 +0200
Message-ID: <YG/5HOGqQ0jFUy3N@kroah.com> (raw)
In-Reply-To: <4e1d73ed75334611578eb607bfdb5ba979abef3c.1617929509.git.Thinh.Nguyen@synopsys.com>

On Thu, Apr 08, 2021 at 06:41:59PM -0700, Thinh Nguyen wrote:
> DWC3 (and possibly others such as CDNS3) will need to access these xHCI
> quirks' definitions to initialize their hosts. Currently, to set these
> quirks, we'd need to create new DT properties matching the quirks. This
> may not be necessary as the driver can check the controller IP and
> version at runtime to determine which quirks are needed. Let's move
> these quirks' definitions to a common header under include/linux/usb so
> DWC3 can properly access them.
> 
> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
> ---
>  drivers/usb/host/xhci-plat.c    |  1 -
>  drivers/usb/host/xhci-plat.h    | 25 -----------
>  drivers/usb/host/xhci-rcar.c    |  1 -
>  drivers/usb/host/xhci.h         | 53 +----------------------
>  include/linux/usb/xhci-quirks.h | 77 +++++++++++++++++++++++++++++++++
>  5 files changed, 78 insertions(+), 79 deletions(-)
>  delete mode 100644 drivers/usb/host/xhci-plat.h
>  create mode 100644 include/linux/usb/xhci-quirks.h
> 
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index c1edcc9b13ce..716ef3a338db 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -21,7 +21,6 @@
>  #include <linux/usb/of.h>
>  
>  #include "xhci.h"
> -#include "xhci-plat.h"
>  #include "xhci-mvebu.h"
>  #include "xhci-rcar.h"
>  
> diff --git a/drivers/usb/host/xhci-plat.h b/drivers/usb/host/xhci-plat.h
> deleted file mode 100644
> index 561d0b7bce09..000000000000
> --- a/drivers/usb/host/xhci-plat.h
> +++ /dev/null
> @@ -1,25 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -/*
> - * xhci-plat.h - xHCI host controller driver platform Bus Glue.
> - *
> - * Copyright (C) 2015 Renesas Electronics Corporation
> - */
> -
> -#ifndef _XHCI_PLAT_H
> -#define _XHCI_PLAT_H
> -
> -#include "xhci.h"	/* for hcd_to_xhci() */
> -
> -struct xhci_plat_priv {
> -	const char *firmware_name;
> -	unsigned long long quirks;
> -	int (*plat_setup)(struct usb_hcd *);
> -	void (*plat_start)(struct usb_hcd *);
> -	int (*init_quirk)(struct usb_hcd *);
> -	int (*suspend_quirk)(struct usb_hcd *);
> -	int (*resume_quirk)(struct usb_hcd *);
> -};
> -
> -#define hcd_to_xhci_priv(h) ((struct xhci_plat_priv *)hcd_to_xhci(h)->priv)
> -#define xhci_to_priv(x) ((struct xhci_plat_priv *)(x)->priv)
> -#endif	/* _XHCI_PLAT_H */
> diff --git a/drivers/usb/host/xhci-rcar.c b/drivers/usb/host/xhci-rcar.c
> index 1bc4fe7b8c75..7690bee365fd 100644
> --- a/drivers/usb/host/xhci-rcar.c
> +++ b/drivers/usb/host/xhci-rcar.c
> @@ -14,7 +14,6 @@
>  #include <linux/sys_soc.h>
>  
>  #include "xhci.h"
> -#include "xhci-plat.h"
>  #include "xhci-rcar.h"
>  
>  /*
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index 2595a8f057c4..9a4e2808668b 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -17,6 +17,7 @@
>  #include <linux/kernel.h>
>  #include <linux/usb/hcd.h>
>  #include <linux/io-64-nonatomic-lo-hi.h>
> +#include <linux/usb/xhci-quirks.h>
>  
>  /* Code sharing between pci-quirks and xhci hcd */
>  #include	"xhci-ext-caps.h"
> @@ -1840,58 +1841,6 @@ struct xhci_hcd {
>  #define XHCI_STATE_HALTED	(1 << 1)
>  #define XHCI_STATE_REMOVING	(1 << 2)
>  	unsigned long long	quirks;
> -#define	XHCI_LINK_TRB_QUIRK	BIT_ULL(0)
> -#define XHCI_RESET_EP_QUIRK	BIT_ULL(1)
> -#define XHCI_NEC_HOST		BIT_ULL(2)
> -#define XHCI_AMD_PLL_FIX	BIT_ULL(3)
> -#define XHCI_SPURIOUS_SUCCESS	BIT_ULL(4)
> -/*
> - * Certain Intel host controllers have a limit to the number of endpoint
> - * contexts they can handle.  Ideally, they would signal that they can't handle
> - * anymore endpoint contexts by returning a Resource Error for the Configure
> - * Endpoint command, but they don't.  Instead they expect software to keep track
> - * of the number of active endpoints for them, across configure endpoint
> - * commands, reset device commands, disable slot commands, and address device
> - * commands.
> - */
> -#define XHCI_EP_LIMIT_QUIRK	BIT_ULL(5)
> -#define XHCI_BROKEN_MSI		BIT_ULL(6)
> -#define XHCI_RESET_ON_RESUME	BIT_ULL(7)
> -#define	XHCI_SW_BW_CHECKING	BIT_ULL(8)
> -#define XHCI_AMD_0x96_HOST	BIT_ULL(9)
> -#define XHCI_TRUST_TX_LENGTH	BIT_ULL(10)
> -#define XHCI_LPM_SUPPORT	BIT_ULL(11)
> -#define XHCI_INTEL_HOST		BIT_ULL(12)
> -#define XHCI_SPURIOUS_REBOOT	BIT_ULL(13)
> -#define XHCI_COMP_MODE_QUIRK	BIT_ULL(14)
> -#define XHCI_AVOID_BEI		BIT_ULL(15)
> -#define XHCI_PLAT		BIT_ULL(16)
> -#define XHCI_SLOW_SUSPEND	BIT_ULL(17)
> -#define XHCI_SPURIOUS_WAKEUP	BIT_ULL(18)
> -/* For controllers with a broken beyond repair streams implementation */
> -#define XHCI_BROKEN_STREAMS	BIT_ULL(19)
> -#define XHCI_PME_STUCK_QUIRK	BIT_ULL(20)
> -#define XHCI_MTK_HOST		BIT_ULL(21)
> -#define XHCI_SSIC_PORT_UNUSED	BIT_ULL(22)
> -#define XHCI_NO_64BIT_SUPPORT	BIT_ULL(23)
> -#define XHCI_MISSING_CAS	BIT_ULL(24)
> -/* For controller with a broken Port Disable implementation */
> -#define XHCI_BROKEN_PORT_PED	BIT_ULL(25)
> -#define XHCI_LIMIT_ENDPOINT_INTERVAL_7	BIT_ULL(26)
> -#define XHCI_U2_DISABLE_WAKE	BIT_ULL(27)
> -#define XHCI_ASMEDIA_MODIFY_FLOWCONTROL	BIT_ULL(28)
> -#define XHCI_HW_LPM_DISABLE	BIT_ULL(29)
> -#define XHCI_SUSPEND_DELAY	BIT_ULL(30)
> -#define XHCI_INTEL_USB_ROLE_SW	BIT_ULL(31)
> -#define XHCI_ZERO_64B_REGS	BIT_ULL(32)
> -#define XHCI_DEFAULT_PM_RUNTIME_ALLOW	BIT_ULL(33)
> -#define XHCI_RESET_PLL_ON_DISCONNECT	BIT_ULL(34)
> -#define XHCI_SNPS_BROKEN_SUSPEND    BIT_ULL(35)
> -#define XHCI_RENESAS_FW_QUIRK	BIT_ULL(36)
> -#define XHCI_SKIP_PHY_INIT	BIT_ULL(37)
> -#define XHCI_DISABLE_SPARSE	BIT_ULL(38)
> -#define XHCI_SG_TRB_CACHE_SIZE_QUIRK	BIT_ULL(39)
> -#define XHCI_NO_SOFT_RETRY	BIT_ULL(40)
>  
>  	unsigned int		num_active_eps;
>  	unsigned int		limit_active_eps;
> diff --git a/include/linux/usb/xhci-quirks.h b/include/linux/usb/xhci-quirks.h
> new file mode 100644
> index 000000000000..c2cb35c5b273
> --- /dev/null
> +++ b/include/linux/usb/xhci-quirks.h
> @@ -0,0 +1,77 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * This file holds the definitions of quirks found in xHCI USB hosts.
> + */
> +
> +#ifndef __LINUX_USB_XHCI_QUIRKS_H
> +#define __LINUX_USB_XHCI_QUIRKS_H
> +
> +#define	XHCI_LINK_TRB_QUIRK		BIT_ULL(0)
> +#define XHCI_RESET_EP_QUIRK		BIT_ULL(1)
> +#define XHCI_NEC_HOST			BIT_ULL(2)
> +#define XHCI_AMD_PLL_FIX		BIT_ULL(3)
> +#define XHCI_SPURIOUS_SUCCESS		BIT_ULL(4)
> +/*
> + * Certain Intel host controllers have a limit to the number of endpoint
> + * contexts they can handle.  Ideally, they would signal that they can't handle
> + * anymore endpoint contexts by returning a Resource Error for the Configure
> + * Endpoint command, but they don't.  Instead they expect software to keep track
> + * of the number of active endpoints for them, across configure endpoint
> + * commands, reset device commands, disable slot commands, and address device
> + * commands.
> + */
> +#define XHCI_EP_LIMIT_QUIRK		BIT_ULL(5)
> +#define XHCI_BROKEN_MSI			BIT_ULL(6)
> +#define XHCI_RESET_ON_RESUME		BIT_ULL(7)
> +#define	XHCI_SW_BW_CHECKING		BIT_ULL(8)
> +#define XHCI_AMD_0x96_HOST		BIT_ULL(9)
> +#define XHCI_TRUST_TX_LENGTH		BIT_ULL(10)
> +#define XHCI_LPM_SUPPORT		BIT_ULL(11)
> +#define XHCI_INTEL_HOST			BIT_ULL(12)
> +#define XHCI_SPURIOUS_REBOOT		BIT_ULL(13)
> +#define XHCI_COMP_MODE_QUIRK		BIT_ULL(14)
> +#define XHCI_AVOID_BEI			BIT_ULL(15)
> +#define XHCI_PLAT			BIT_ULL(16)
> +#define XHCI_SLOW_SUSPEND		BIT_ULL(17)
> +#define XHCI_SPURIOUS_WAKEUP		BIT_ULL(18)
> +/* For controllers with a broken beyond repair streams implementation */
> +#define XHCI_BROKEN_STREAMS		BIT_ULL(19)
> +#define XHCI_PME_STUCK_QUIRK		BIT_ULL(20)
> +#define XHCI_MTK_HOST			BIT_ULL(21)
> +#define XHCI_SSIC_PORT_UNUSED		BIT_ULL(22)
> +#define XHCI_NO_64BIT_SUPPORT		BIT_ULL(23)
> +#define XHCI_MISSING_CAS		BIT_ULL(24)
> +/* For controller with a broken Port Disable implementation */
> +#define XHCI_BROKEN_PORT_PED		BIT_ULL(25)
> +#define XHCI_LIMIT_ENDPOINT_INTERVAL_7	BIT_ULL(26)
> +#define XHCI_U2_DISABLE_WAKE		BIT_ULL(27)
> +#define XHCI_ASMEDIA_MODIFY_FLOWCONTROL	BIT_ULL(28)
> +#define XHCI_HW_LPM_DISABLE		BIT_ULL(29)
> +#define XHCI_SUSPEND_DELAY		BIT_ULL(30)
> +#define XHCI_INTEL_USB_ROLE_SW		BIT_ULL(31)
> +#define XHCI_ZERO_64B_REGS		BIT_ULL(32)
> +#define XHCI_DEFAULT_PM_RUNTIME_ALLOW	BIT_ULL(33)
> +#define XHCI_RESET_PLL_ON_DISCONNECT	BIT_ULL(34)
> +#define XHCI_SNPS_BROKEN_SUSPEND	BIT_ULL(35)
> +#define XHCI_RENESAS_FW_QUIRK		BIT_ULL(36)
> +#define XHCI_SKIP_PHY_INIT		BIT_ULL(37)
> +#define XHCI_DISABLE_SPARSE		BIT_ULL(38)
> +#define XHCI_SG_TRB_CACHE_SIZE_QUIRK	BIT_ULL(39)
> +#define XHCI_NO_SOFT_RETRY		BIT_ULL(40)
> +
> +struct usb_hcd;
> +
> +struct xhci_plat_priv {
> +	const char *firmware_name;
> +	unsigned long long quirks;
> +	int (*plat_setup)(struct usb_hcd *hcd);
> +	void (*plat_start)(struct usb_hcd *hcd);
> +	int (*init_quirk)(struct usb_hcd *hcd);
> +	int (*suspend_quirk)(struct usb_hcd *hcd);
> +	int (*resume_quirk)(struct usb_hcd *hcd);
> +};
> +
> +#define hcd_to_xhci_priv(h) ((struct xhci_plat_priv *)hcd_to_xhci(h)->priv)
> +#define xhci_to_priv(x) ((struct xhci_plat_priv *)(x)->priv)

Why do you need this tructure and #defines for xhci priv stuff need to
be in a public .h file?

I would think that at most you just need the xhci bit fields.

thanks,

greg k-h

  reply index

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-09  1:41 [PATCH 0/6] usb: Set quirks for xhci/dwc3 host mode Thinh Nguyen
2021-04-09  1:41 ` [PATCH 1/6] usb: xhci: Move quirks definitions to common usb header Thinh Nguyen
2021-04-09  6:50   ` Greg Kroah-Hartman [this message]
2021-04-09  8:01     ` Thinh Nguyen
2021-04-09 16:26   ` kernel test robot
2021-04-09 19:54   ` kernel test robot
2021-04-09  1:42 ` [PATCH 2/6] usb: xhci: Check for blocked disconnection Thinh Nguyen
2021-04-09  1:42 ` [PATCH 3/6] usb: xhci: Workaround undercalculated BW for fullspeed BI Thinh Nguyen
2021-04-09  1:42 ` [PATCH 4/6] usb: xhci: Rename Compliance mode timer quirk Thinh Nguyen
2021-04-09  1:42 ` [PATCH 5/6] usb: xhci: Workaround lost disconnect port status Thinh Nguyen
2021-04-09  1:42 ` [PATCH 6/6] usb: dwc3: host: Set quirks base on version Thinh Nguyen
2021-04-09  6:53   ` Greg Kroah-Hartman
2021-04-09  8:01     ` Thinh Nguyen
2021-04-09 13:22       ` Greg Kroah-Hartman
2021-04-10  0:44         ` Thinh Nguyen
2021-04-10  0:46 ` [PATCH v2 0/7] usb: Set quirks for xhci/dwc3 host mode Thinh Nguyen
2021-04-10  0:46   ` [PATCH v2 1/7] usb: xhci: Move quirks definitions to common usb header Thinh Nguyen
2021-04-10  0:46   ` [PATCH v2 2/7] usb: xhci: Move xhci-plat header " Thinh Nguyen
2021-04-10  0:47   ` [PATCH v2 3/7] usb: xhci: Check for blocked disconnection Thinh Nguyen
2021-04-27 13:08     ` Mathias Nyman
2021-04-27 22:30       ` Thinh Nguyen
2021-04-28 13:32         ` Mathias Nyman
2021-04-29  0:54           ` Thinh Nguyen
2021-04-10  0:47   ` [PATCH v2 4/7] usb: xhci: Workaround undercalculated BW for fullspeed BI Thinh Nguyen
2021-04-28 11:57     ` Mathias Nyman
2021-04-10  0:47   ` [PATCH v2 5/7] usb: xhci: Rename Compliance mode timer quirk Thinh Nguyen
2021-04-10  0:47   ` [PATCH v2 6/7] usb: xhci: Workaround lost disconnect port status Thinh Nguyen
2021-04-28 13:48     ` Mathias Nyman
2021-04-29  1:00       ` Thinh Nguyen
2021-04-10  0:47   ` [PATCH v2 7/7] usb: dwc3: host: Set quirks base on version Thinh Nguyen
2021-04-19 21:07   ` [PATCH v2 0/7] usb: Set quirks for xhci/dwc3 host mode Thinh Nguyen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YG/5HOGqQ0jFUy3N@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=John.Youn@synopsys.com \
    --cc=Thinh.Nguyen@synopsys.com \
    --cc=balbi@kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mathias.nyman@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-USB Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-usb/0 linux-usb/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-usb linux-usb/ https://lore.kernel.org/linux-usb \
		linux-usb@vger.kernel.org
	public-inbox-index linux-usb

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-usb


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git