[PATCH v2 1/4] drm/xe: Simplify function return using drmm_add_action_or_reset()

[Lucas De Marchi]

On Mon, Apr 08, 2024 at 06:08:10PM +0300, Jani Nikula wrote:
>On Mon, 08 Apr 2024, Himal Prasad Ghimiray <himal.prasad.ghimiray at intel.com> wrote:
>> Instead of assigning the value of drmm_add_action_or_reset() to err and
>> returning err in case of failure and 0 in case of success, simply return
>> the result of drmm_add_action_or_reset().
>>
>> -v2:
>> cleanup in xe_display too.
>>
>> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
>> Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>> Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray at intel.com>
>> ---
>>  drivers/gpu/drm/xe/display/xe_display.c | 8 +-------
>>  drivers/gpu/drm/xe/xe_device.c          | 6 +-----
>>  drivers/gpu/drm/xe/xe_gsc_proxy.c       | 7 +------
>>  drivers/gpu/drm/xe/xe_gt.c              | 6 +-----
>>  drivers/gpu/drm/xe/xe_guc_pc.c          | 6 +-----
>>  drivers/gpu/drm/xe/xe_hw_engine.c       | 6 +-----
>>  6 files changed, 6 insertions(+), 33 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
>> index 6ec375c1c4b6..63b27fbcdaca 100644
>> --- a/drivers/gpu/drm/xe/display/xe_display.c
>> +++ b/drivers/gpu/drm/xe/display/xe_display.c
>> @@ -101,8 +101,6 @@ static void display_destroy(struct drm_device *dev, void *dummy)
>>   */
>>  int xe_display_create(struct xe_device *xe)
>>  {
>> -	int err;
>> -
>>  	spin_lock_init(&xe->display.fb_tracking.lock);
>>
>>  	xe->display.hotplug.dp_wq = alloc_ordered_workqueue("xe-dp", 0);
>> @@ -110,11 +108,7 @@ int xe_display_create(struct xe_device *xe)
>>  	drmm_mutex_init(&xe->drm, &xe->sb_lock);
>>  	xe->enabled_irq_mask = ~0;
>>
>> -	err = drmm_add_action_or_reset(&xe->drm, display_destroy, NULL);
>> -	if (err)
>> -		return err;
>> -
>> -	return 0;
>> +	return drmm_add_action_or_reset(&xe->drm, display_destroy, NULL);
>
>Up to the xe maintainers, but personally I prefer these as they were,
>throughout the series.
>
>The thing is, these functions will be maintained for a long time, and
>there'll inevitably be a need to add more initializations and error
>returns and cleanups. All of these "return foo()" style things make
>future changes harder.

I'd prefer to keep the code tidy rather than these hypothetical
additions.

But even if we extend, drmm_add_action_or_reset() will always end up
being the last thing we do and other things will be added before that.
 From that POV the `return drmm_add_action_or_reset` will always remain
as is.

Lucas De Marchi