Re: [PATCH v3 11/13] soc: qcom: aoss: Drop power domain support

[Sibi Sankar]

On 2021-06-26 03:31, Matthias Kaehlcke wrote:
> On Fri, Jun 25, 2021 at 12:22:05AM +0530, Sibi Sankar wrote:
> > The load state resources are expected to follow the life cycle of the
> > remote processor it tracks. However, modeling load state resources as
> > power-domains result in them getting turned off during system suspend
> > and thereby falling out of sync with the remote processors that are 
> > still
> > on. Fix this by replacing load state resource control through the 
> > generic
> > qmp message send interface instead.
>
> nit: the above sounds as if this patch does all of that, when it only
> removes power domain support. Instead you could start with saying what
> the patch actually does (remove power domain support), followed by why
> PD support isn't needed anymore (now done by sending QMP messages 
> directly).

sure, will fix this up in the next
re-spin.

> > Signed-off-by: Sibi Sankar <sibis@xxxxxxxxxxxxxx>
> > ---
> >  drivers/soc/qcom/qcom_aoss.c | 109 
> > ++-----------------------------------------
> >  1 file changed, 3 insertions(+), 106 deletions(-)
> >
> > diff --git a/drivers/soc/qcom/qcom_aoss.c 
> > b/drivers/soc/qcom/qcom_aoss.c
> > index 998ee7605eb2..f0c3726e8c46 100644
> > --- a/drivers/soc/qcom/qcom_aoss.c
> > +++ b/drivers/soc/qcom/qcom_aoss.c
> >
> > ...
> >
> > @@ -650,13 +550,11 @@ static int qmp_probe(struct platform_device 
> > *pdev)
> >  	if (ret)
> >  		goto err_close_qmp;
> >
> > -	ret = qmp_pd_add(qmp);
> > -	if (ret)
> > -		goto err_remove_qdss_clk;
> > -
> >  	ret = qmp_cooling_devices_register(qmp);
> > -	if (ret)
> > +	if (ret) {
> >  		dev_err(&pdev->dev, "failed to register aoss cooling devices\n");
> > +		goto err_remove_qdss_clk;
>
> This isn't really related with the PD removal, right? I wonder if it 
> was
> intentional to have _probe() succeed even when the cooling device
> registration failed, since the cooling devices aren't essential.

Thanks for catching ^^

> If it is still desirable to fail the change should be done in a 
> separate
> patch, unless it is actually related with removing PD support.

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.