Re: [PATCH] ASoC: Intel: SST: fix platform device leak on probe failure

[Cezary Rojewski <cezary.rojewski@intel.com>]

On 9/22/2026 10:17 AM, Guangshuo Li wrote:
> Hi Czarek,
> 
> Thanks for the review.

>>> @@ -361,15 +362,21 @@ static int sst_acpi_probe(struct platform_device *pdev)
>>>
>>>       ret = sst_platform_get_resources(ctx);
>>>       if (ret)
>>> -             return ret;
>>> +             goto err_unregister_mdev;
>>>
>>>       ret = sst_context_init(ctx);
>>>       if (ret < 0)
>>> -             return ret;
>>> +             goto err_unregister_mdev;
>>>
>>>       sst_configure_runtime_pm(ctx);
>>>       platform_set_drvdata(pdev, ctx);
>>>       return ret;
>>> +
>>> +err_unregister_mdev:
>>> +     platform_device_unregister(mdev);
>>> +err_unregister_plat_dev:
>>> +     platform_device_unregister(plat_dev);
>>> +     return ret;
>>>  }
>>
>> Looks like sst_acpi_remove() does not unregister the devices either.
>> That could be fixed by enlisting devm_add_action_or_reset() in
>> sst_acpi_probe() without altering sst_acpi_remove() at all.
>>
>> Would you mind sending a separate patch updating the function so both
>> the error path and the driver-unload clean up the device objects?
>>
>>
>> Kind regards,
>> Czarek
> 
> I'll update the title to:
> 
> ASoC: Intel: atom: Fix platform device leak on probe failure
> 
> and trim the commit message as suggested, including the
> s/machine drivers/machine board/ change. I'll also add your Reviewed-by.
> 
> For the separate cleanup patch, do you mean something like this?
> 
> +static void sst_unregister_platform_device(void *data)
> +{
> +        platform_device_unregister(data);
> +}
> +
>         plat_dev = platform_device_register_data(...);
>         if (IS_ERR(plat_dev))
>                 return PTR_ERR(plat_dev);
> +
> +       ret = devm_add_action_or_reset(dev, sst_unregister_platform_device,
> +                                      plat_dev);
> +       if (ret)
> +               return ret;
> 
>         ...
> 
>         mdev = platform_device_register_data(...);
>         if (IS_ERR(mdev))
>                 return PTR_ERR(mdev);
> +
> +       ret = devm_add_action_or_reset(dev, sst_unregister_platform_device,
> +                                      mdev);
> +       if (ret)
> +               return ret;
> 
> This would let devres clean up both devices on later probe failure and on
> driver unload, so the explicit unregister error paths from the first patch
> would no longer be needed after this patch.
> 
> Would this be what you had in mind? If so, I'll follow Krzysztof's
> suggestion, organize the related changes into a proper patch series,
> and send a two-patch v2.

I do not mind either approach. There are two problems here:
	1) leak on probe() failure
	2) leak on driver remove()

So, two patches-approach IMHO is perfectly fine. And yes, enlisting
devm_add_action_or_reset() closes both 1) and 2) in one go.

Kind regards,
Czarek