Re: [PATCH v12 4/6] PCI: dwc: Use common PCI host bridge APIs for finding the capabilities

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Thu, 15 May 2025, Hans Zhang wrote:

> Use the PCI core is now exposing generic macros for the host bridges to
> search for the PCIe capabilities, make use of them in the DWC driver.
> 
> Signed-off-by: Hans Zhang <18255117159@163.com>
> ---
> Changes since v11:
> - Resolve compilation errors. s/dw_pcie_read_dbi/dw_pcie_read*_dbi
> 
> Changes since v10:
> - None
> 
> Changes since v9:
> - Resolved [v9 4/6] compilation error.
>   The latest 6.15 rc1 merge __dw_pcie_find_vsec_capability, which uses 
>   dw_pcie_find_next_ext_capability.
> 
> Changes since v8:
> - None
> 
> Changes since v7:
> - Resolve compilation errors.
> 
> Changes since v6:
> https://lore.kernel.org/linux-pci/20250323164852.430546-3-18255117159@163.com/
> 
> - The patch commit message were modified.
> 
> Changes since v5:
> https://lore.kernel.org/linux-pci/20250321163803.391056-3-18255117159@163.com/
> 
> - Kconfig add "select PCI_HOST_HELPERS"
> ---
>  drivers/pci/controller/dwc/pcie-designware.c | 81 ++++----------------
>  1 file changed, 14 insertions(+), 67 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 97d76d3dc066..7939411a24eb 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -205,83 +205,30 @@ void dw_pcie_version_detect(struct dw_pcie *pci)
>  		pci->type = ver;
>  }
>  
> -/*
> - * These interfaces resemble the pci_find_*capability() interfaces, but these
> - * are for configuring host controllers, which are bridges *to* PCI devices but
> - * are not PCI devices themselves.
> - */
> -static u8 __dw_pcie_find_next_cap(struct dw_pcie *pci, u8 cap_ptr,
> -				  u8 cap)
> +static int dw_pcie_read_cfg(void *priv, int where, int size, u32 *val)
>  {
> -	u8 cap_id, next_cap_ptr;
> -	u16 reg;
> -
> -	if (!cap_ptr)
> -		return 0;
> +	struct dw_pcie *pci = priv;
>  
> -	reg = dw_pcie_readw_dbi(pci, cap_ptr);
> -	cap_id = (reg & 0x00ff);
> -
> -	if (cap_id > PCI_CAP_ID_MAX)
> -		return 0;
> -
> -	if (cap_id == cap)
> -		return cap_ptr;
> +	if (size == 4)
> +		*val = dw_pcie_readl_dbi(pci, where);
> +	else if (size == 2)
> +		*val = dw_pcie_readw_dbi(pci, where);
> +	else if (size == 1)
> +		*val = dw_pcie_readb_dbi(pci, where);

Maybe here as well return error if the given size is invalid.
>  
> -	next_cap_ptr = (reg & 0xff00) >> 8;
> -	return __dw_pcie_find_next_cap(pci, next_cap_ptr, cap);
> +	return PCIBIOS_SUCCESSFUL;
>  }
>  
>  u8 dw_pcie_find_capability(struct dw_pcie *pci, u8 cap)
>  {
> -	u8 next_cap_ptr;
> -	u16 reg;
> -
> -	reg = dw_pcie_readw_dbi(pci, PCI_CAPABILITY_LIST);
> -	next_cap_ptr = (reg & 0x00ff);
> -
> -	return __dw_pcie_find_next_cap(pci, next_cap_ptr, cap);
> +	return PCI_FIND_NEXT_CAP_TTL(dw_pcie_read_cfg, PCI_CAPABILITY_LIST, cap,
> +				     pci);
>  }
>  EXPORT_SYMBOL_GPL(dw_pcie_find_capability);
>  
> -static u16 dw_pcie_find_next_ext_capability(struct dw_pcie *pci, u16 start,
> -					    u8 cap)
> -{
> -	u32 header;
> -	int ttl;
> -	int pos = PCI_CFG_SPACE_SIZE;
> -
> -	/* minimum 8 bytes per capability */
> -	ttl = (PCI_CFG_SPACE_EXP_SIZE - PCI_CFG_SPACE_SIZE) / 8;
> -
> -	if (start)
> -		pos = start;
> -
> -	header = dw_pcie_readl_dbi(pci, pos);
> -	/*
> -	 * If we have no capabilities, this is indicated by cap ID,
> -	 * cap version and next pointer all being 0.
> -	 */
> -	if (header == 0)
> -		return 0;
> -
> -	while (ttl-- > 0) {
> -		if (PCI_EXT_CAP_ID(header) == cap && pos != start)
> -			return pos;
> -
> -		pos = PCI_EXT_CAP_NEXT(header);
> -		if (pos < PCI_CFG_SPACE_SIZE)
> -			break;
> -
> -		header = dw_pcie_readl_dbi(pci, pos);
> -	}
> -
> -	return 0;
> -}
> -
>  u16 dw_pcie_find_ext_capability(struct dw_pcie *pci, u8 cap)
>  {
> -	return dw_pcie_find_next_ext_capability(pci, 0, cap);
> +	return PCI_FIND_NEXT_EXT_CAPABILITY(dw_pcie_read_cfg, 0, cap, pci);
>  }
>  EXPORT_SYMBOL_GPL(dw_pcie_find_ext_capability);
>  
> @@ -294,8 +241,8 @@ static u16 __dw_pcie_find_vsec_capability(struct dw_pcie *pci, u16 vendor_id,
>  	if (vendor_id != dw_pcie_readw_dbi(pci, PCI_VENDOR_ID))
>  		return 0;
>  
> -	while ((vsec = dw_pcie_find_next_ext_capability(pci, vsec,
> -						       PCI_EXT_CAP_ID_VNDR))) {
> +	while ((vsec = PCI_FIND_NEXT_EXT_CAPABILITY(
> +			dw_pcie_read_cfg, vsec, PCI_EXT_CAP_ID_VNDR, pci))) {

Start the arguments from the first line and align the continuations to (.

>  		header = dw_pcie_readl_dbi(pci, vsec + PCI_VNDR_HEADER);
>  		if (PCI_VNDR_HEADER_ID(header) == vsec_id)
>  			return vsec;
> 

-- 
 i.