Re: [RFC PATCH] cxl: Use pci_find_vsec_capability() to simplify the code

[Andrew Donnellan <ajd@linux.ibm.com>]

On Fri, 2023-08-04 at 15:56 +0800, Xiongfeng Wang wrote:
> PCI core add pci_find_vsec_capability() to query VSEC. We can use
> that
> core API to simplify the code.
> 
> The only logical change is that pci_find_vsec_capability check the
> Vendor ID before finding the VSEC.
> 
> PCI spec rev 5.0 says in 7.9.5.2 Vendor-Specific Header:
>   VSEC ID - This field is a vendor-defined ID number that indicates
> the
>   nature and format of the VSEC structure
>   Software must qualify the Vendor ID before interpreting this field.
> 
> Signed-off-by: Xiongfeng Wang <wangxiongfeng2@huawei.com>

LGTM

The cxl driver doesn't currently bind to any devices that don't have an
IBM vendor ID, and it's very unlikely to in future. If that ever
changes, this will of course need to be updated accordingly.

Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>

> ---
>  drivers/misc/cxl/pci.c | 12 ++----------
>  1 file changed, 2 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
> index 0ff944860dda..f3108977755d 100644
> --- a/drivers/misc/cxl/pci.c
> +++ b/drivers/misc/cxl/pci.c
> @@ -150,16 +150,8 @@ static inline resource_size_t p2_size(struct
> pci_dev *dev)
>  
>  static int find_cxl_vsec(struct pci_dev *dev)
>  {
> -       int vsec = 0;
> -       u16 val;
> -
> -       while ((vsec = pci_find_next_ext_capability(dev, vsec,
> PCI_EXT_CAP_ID_VNDR))) {
> -               pci_read_config_word(dev, vsec + 0x4, &val);
> -               if (val == CXL_PCI_VSEC_ID)
> -                       return vsec;
> -       }
> -       return 0;
> -
> +       return pci_find_vsec_capability(dev, PCI_VENDOR_ID_IBM,
> +                                       CXL_PCI_VSEC_ID);
>  }
>  
>  static void dump_cxl_config_space(struct pci_dev *dev)

-- 
Andrew Donnellan    OzLabs, ADL Canberra
ajd@linux.ibm.com   IBM Australia Limited