Re: [PATCH 3/7] slimbus: qcom-ngd-ctrl: Correct PDR and SSR cleanup ownership

[Bjorn Andersson <andersson@kernel.org>]

On Tue, Mar 10, 2026 at 01:09:33PM +0530, Mukesh Ojha wrote:
> On Mon, Mar 09, 2026 at 11:09:04PM -0500, Bjorn Andersson wrote:
> > PDR and SSR callbacks are registred from the controller probe function,
> > but currently released from the child device's remove function.
> > 
> > In the next commit the controller probe function will be modified such
> > that the error path will unregister the child device, resulting in a
> > double free of these resources.
> 
> Change is fine, Could this not be accommodated in the next commit?
> 

The problem solved by patch 4 relates to the oreder that we're acquiring
the resources in probe and how the error handling of that works.

If I squash the two patches, it seems that I would have a lengthy commit
message talking about that part and then a "also, while at it move the
unregister from X to Y because...".

I.e. it doesn't feel like the same logical change to me.

Please let me know if you disagree.

Regards,
Bjorn

> > 
> > Fixes: 917809e2280b ("slimbus: ngd: Add qcom SLIMBus NGD driver")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
> 
> Reviewed-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> 
> > ---
> >  drivers/slimbus/qcom-ngd-ctrl.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/slimbus/qcom-ngd-ctrl.c b/drivers/slimbus/qcom-ngd-ctrl.c
> > index b34e727bab086c95dc7e760bf1141baac9ccf6a7..09ce3299e15c25b1b9cf6b1559850adf4aa20737 100644
> > --- a/drivers/slimbus/qcom-ngd-ctrl.c
> > +++ b/drivers/slimbus/qcom-ngd-ctrl.c
> > @@ -1685,6 +1685,9 @@ static void qcom_slim_ngd_ctrl_remove(struct platform_device *pdev)
> >  {
> >  	struct qcom_slim_ngd_ctrl *ctrl = platform_get_drvdata(pdev);
> >  
> > +	pdr_handle_release(ctrl->pdr);
> > +	qcom_unregister_ssr_notifier(ctrl->notifier, &ctrl->nb);
> > +
> >  	qcom_slim_ngd_unregister(ctrl);
> >  }
> >  
> > @@ -1693,8 +1696,6 @@ static void qcom_slim_ngd_remove(struct platform_device *pdev)
> >  	struct qcom_slim_ngd_ctrl *ctrl = platform_get_drvdata(pdev);
> >  
> >  	pm_runtime_disable(&pdev->dev);
> > -	pdr_handle_release(ctrl->pdr);
> > -	qcom_unregister_ssr_notifier(ctrl->notifier, &ctrl->nb);
> >  	qcom_slim_ngd_enable(ctrl, false);
> >  	qcom_slim_ngd_exit_dma(ctrl);
> >  	qcom_slim_ngd_qmi_svc_event_deinit(&ctrl->qmi);
> > 
> > -- 
> > 2.51.0
> > 
> 
> -- 
> -Mukesh Ojha