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

[Mukesh Ojha]

On Mon, Mar 23, 2026 at 09:36:49PM -0500, Bjorn Andersson wrote:
> 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.

Agree..

I was not sure about these text usage "next commit" and "will be modified",
Hence, was asking if we can accomodate it in one commit as both are Cced to
stable


> 
> Regards,
> Bjorn
> 
> > > 
> > > Fixes: 917809e2280b ("slimbus: ngd: Add qcom SLIMBus NGD driver")
> > > Cc: stable@xxxxxxxxxxxxxxx
> > > Signed-off-by: Bjorn Andersson <bjorn.andersson@xxxxxxxxxxxxxxxx>
> > 
> > Reviewed-by: Mukesh Ojha <mukesh.ojha@xxxxxxxxxxxxxxxx>
> > 
> > > ---
> > >  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

-- 
-Mukesh Ojha