Applied. Thanks!
On Mon, Sep 21, 2026 at 6:51 AM Guangshuo Li <[email protected]> wrote: > > acp_hw_init() registers ACP child devices with mfd_add_devices() before > attaching them to the ACP power domain and initializing the hardware. > > If attaching a child to the power domain fails, or if the ACP reset or > clock enable operation times out, the failure path frees only the > source cell, resource, and platform-data allocations. The child > platform devices already registered by mfd_add_devices() remain > registered and are never released. > > Remove the children from the power domain and unregister the MFD > devices on failures that occur after mfd_add_devices() succeeds. Keep > mfd_add_devices() failures on the existing cleanup path since the MFD > core already rolls back partially registered children itself. > > The issue was identified by a static analysis tool I developed and > confirmed by manual review. > > Fixes: 25030321ba28 ("drm/amd: add pm domain for ACP IP sub blocks") > Cc: [email protected] > Signed-off-by: Guangshuo Li <[email protected]> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c > index 9014678d75ab..be4d00ac96b3 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c > @@ -318,7 +318,7 @@ static int acp_hw_init(struct amdgpu_ip_block *ip_block) > r = device_for_each_child(adev->acp.parent, > &adev->acp.acp_genpd->gpd, > acp_genpd_add_device); > if (r) > - goto failure; > + goto failure_remove_mfd; > break; > } > default: > @@ -443,7 +443,7 @@ static int acp_hw_init(struct amdgpu_ip_block *ip_block) > r = device_for_each_child(adev->acp.parent, > &adev->acp.acp_genpd->gpd, > acp_genpd_add_device); > if (r) > - goto failure; > + goto failure_remove_mfd; > } > > /* Assert Soft reset of ACP */ > @@ -461,7 +461,7 @@ static int acp_hw_init(struct amdgpu_ip_block *ip_block) > if (--count == 0) { > dev_err(&adev->pdev->dev, "Failed to reset ACP\n"); > r = -ETIMEDOUT; > - goto failure; > + goto failure_remove_mfd; > } > udelay(100); > } > @@ -479,7 +479,7 @@ static int acp_hw_init(struct amdgpu_ip_block *ip_block) > if (--count == 0) { > dev_err(&adev->pdev->dev, "Failed to reset ACP\n"); > r = -ETIMEDOUT; > - goto failure; > + goto failure_remove_mfd; > } > udelay(100); > } > @@ -489,6 +489,11 @@ static int acp_hw_init(struct amdgpu_ip_block *ip_block) > cgs_write_register(adev->acp.cgs_device, mmACP_SOFT_RESET, val); > return 0; > > +failure_remove_mfd: > + device_for_each_child(adev->acp.parent, NULL, > + acp_genpd_remove_device); > + mfd_remove_devices(adev->acp.parent); > + > failure: > kfree(adev->acp.i2s_pdata); > kfree(adev->acp.acp_res); > -- > 2.43.0 >
