Re: [PATCH v6 06/11] drm/xe/configfs: Rename configfs_find_group() helper

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

On 8/5/2025 3:14 PM, Lucas De Marchi wrote:
> On Thu, Jul 31, 2025 at 09:33:34PM +0200, Michal Wajdeczko wrote:
>> This helper name shouldn't suggest that it iss a part of the core
>> configfs API family. While around switch to use different helper
>> to release a reference.
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> ---
>> drivers/gpu/drm/xe/xe_configfs.c | 14 +++++++-------
>> 1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_configfs.c b/drivers/gpu/drm/xe/xe_configfs.c
>> index d69b5f248e59..acdee616d118 100644
>> --- a/drivers/gpu/drm/xe/xe_configfs.c
>> +++ b/drivers/gpu/drm/xe/xe_configfs.c
>> @@ -309,7 +309,7 @@ static struct configfs_subsystem xe_configfs = {
>>     },
>> };
>>
>> -static struct xe_config_group_device *configfs_find_group(struct pci_dev *pdev)
>> +static struct xe_config_group_device *find_xe_config_group_device(struct pci_dev *pdev)
>> {
>>     struct config_item *item;
>>
>> @@ -334,14 +334,14 @@ static struct xe_config_group_device *configfs_find_group(struct pci_dev *pdev)
>>  */
>> bool xe_configfs_get_survivability_mode(struct pci_dev *pdev)
>> {
>> -    struct xe_config_group_device *dev = configfs_find_group(pdev);
>> +    struct xe_config_group_device *dev = find_xe_config_group_device(pdev);
> 
> looking at this now, I think it would make more sense to call them,
> differently, which would affect the previous patches too:
> 
>     struct xe_config_group *grp = find_xe_config_group(pdev);
> 
> and:
> 
>     struct xe_config_group {
>         struct config_group base;
>         ...
>     };

I was assuming that one day we would like to add some config
groups that will target "tile" or "GT", unlike simple "device"
as it was introduced today, so we would have 

* xe_device <-> xe_device_config <-> xe_device_config_group
* xe_tile   <-> xe_tile_config   <-> xe_tile_config_group
* xe_gt     <-> xe_gt_config     <-> xe_gt_config_group

but I guess simplified name works too

* xe_device <-> xe_config        <-> xe_config_group

(I can rename, but no earlier than 2w from now)

> 
> but I'm ok with what you did here.
> 
> Lucas De Marchi
> 
>>     bool mode;
>>
>>     if (!dev)
>>         return false;
>>
>>     mode = dev->survivability_mode;
>> -    config_item_put(&dev->group.cg_item);
>> +    config_group_put(&dev->group);
>>
>>     return mode;
>> }
>> @@ -355,7 +355,7 @@ bool xe_configfs_get_survivability_mode(struct pci_dev *pdev)
>>  */
>> void xe_configfs_clear_survivability_mode(struct pci_dev *pdev)
>> {
>> -    struct xe_config_group_device *dev = configfs_find_group(pdev);
>> +    struct xe_config_group_device *dev = find_xe_config_group_device(pdev);
>>
>>     if (!dev)
>>         return;
>> @@ -364,7 +364,7 @@ void xe_configfs_clear_survivability_mode(struct pci_dev *pdev)
>>     dev->survivability_mode = 0;
>>     mutex_unlock(&dev->lock);
>>
>> -    config_item_put(&dev->group.cg_item);
>> +    config_group_put(&dev->group);
>> }
>>
>> /**
>> @@ -378,14 +378,14 @@ void xe_configfs_clear_survivability_mode(struct pci_dev *pdev)
>>  */
>> u64 xe_configfs_get_engines_allowed(struct pci_dev *pdev)
>> {
>> -    struct xe_config_group_device *dev = configfs_find_group(pdev);
>> +    struct xe_config_group_device *dev = find_xe_config_group_device(pdev);
>>     u64 engines_allowed;
>>
>>     if (!dev)
>>         return U64_MAX;
>>
>>     engines_allowed = dev->engines_allowed;
>> -    config_item_put(&dev->group.cg_item);
>> +    config_group_put(&dev->group);
>>
>>     return engines_allowed;
>> }
>> -- 
>> 2.47.1
>>