Re: [PATCH 3/5] drm/xe/configfs: Use pci_name() for lookup

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

On 17.07.2025 23:18, Cavitt, Jonathan wrote:
> -----Original Message-----
> From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Michal Wajdeczko
> Sent: Thursday, July 17, 2025 11:48 AM
> To: intel-xe@lists.freedesktop.org
> Cc: Wajdeczko, Michal <Michal.Wajdeczko@intel.com>; De Marchi, Lucas <lucas.demarchi@intel.com>
> Subject: [PATCH 3/5] drm/xe/configfs: Use pci_name() for lookup
>>
>> There is no need to manually build PCI device name from BDF data,
>> since it was already prepared and assigned and can be accessed by
>> calling pci_name() function.
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>> ---
>>  drivers/gpu/drm/xe/xe_configfs.c | 6 +-----
>>  1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_configfs.c b/drivers/gpu/drm/xe/xe_configfs.c
>> index 90b4fe92a611..00bb4e412c12 100644
>> --- a/drivers/gpu/drm/xe/xe_configfs.c
>> +++ b/drivers/gpu/drm/xe/xe_configfs.c
>> @@ -312,13 +312,9 @@ static struct configfs_subsystem xe_configfs = {
>>  static struct xe_config_device *configfs_find_group(struct pci_dev *pdev)
>>  {
>>  	struct config_item *item;
>> -	char name[64];
>> -
>> -	snprintf(name, sizeof(name), "%04x:%02x:%02x.%x", pci_domain_nr(pdev->bus),
>> -		 pdev->bus->number, PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
>>  
>>  	mutex_lock(&xe_configfs.su_mutex);
>> -	item = config_group_find_item(&xe_configfs.su_group, name);
>> +	item = config_group_find_item(&xe_configfs.su_group, pci_name(pdev));
> 
> Oddly, it doesn't seem like pci_name is a helper function that generates the name we were
> previously constructing manually.  Instead, it looks like it returns either pdev->dev->init_name
> or pdev->dev->kobj->name.
> 
> On the other hand, I can't imagine a function called "pci_name" returning anything other
> than what we were looking for prior, so I'm going to trust you did your due diligence
> and that pci_name returns the same thing during runtime that the snprintf was previously
> constructing.

you missed dev_set_name() called from pci_setup_device() here [1]

[1]
https://elixir.bootlin.com/linux/v6.16-rc6/source/drivers/pci/probe.c#L2006

> 
> Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> -Jonathan Cavitt
>  
>>  	mutex_unlock(&xe_configfs.su_mutex);
>>  
>>  	if (!item)
>> -- 
>> 2.47.1
>>
>>