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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 01 Jul 2021 01:26:25 +0530
From:   Sibi Sankar <sibis@...eaurora.org>
To:     Matthias Kaehlcke <mka@...omium.org>
Cc:     bjorn.andersson@...aro.org, robh+dt@...nel.org,
        swboyd@...omium.org, ulf.hansson@...aro.org, rjw@...ysocki.net,
        agross@...nel.org, ohad@...ery.com, mathieu.poirier@...aro.org,
        linux-arm-msm@...r.kernel.org, linux-remoteproc@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        dianders@...omium.org, rishabhb@...eaurora.org,
        sidgup@...eaurora.org
Subject: Re: [PATCH v3 11/13] soc: qcom: aoss: Drop power domain support

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@...eaurora.org>
>> ---
>>  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.