linux-kernel - Re: [PATCH v3 3/4] soc: qcom: geni-se: Do not keep GPIOs to sleep state for shared SE usecase

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]

Message-ID: <b6793406-1527-4413-addd-e4d5dc73c5e3@quicinc.com>
Date: Wed, 13 Nov 2024 21:37:24 +0530
From: Mukesh Kumar Savaliya <quic_msavaliy@...cinc.com>
To: Konrad Dybcio <konrad.dybcio@....qualcomm.com>, <konrad.dybcio@...aro.org>,
        <andersson@...nel.org>, <andi.shyti@...nel.org>,
        <linux-arm-msm@...r.kernel.org>, <dmaengine@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <linux-i2c@...r.kernel.org>,
        <conor+dt@...nel.org>, <agross@...nel.org>,
        <devicetree@...r.kernel.org>, <vkoul@...nel.org>, <linux@...blig.org>,
        <dan.carpenter@...aro.org>, <Frank.Li@....com>,
        <konradybcio@...nel.org>, <bryan.odonoghue@...aro.org>,
        <krzk+dt@...nel.org>, <robh@...nel.org>
Subject: Re: [PATCH v3 3/4] soc: qcom: geni-se: Do not keep GPIOs to sleep
 state for shared SE usecase

Thanks Konrad for the review.

On 10/26/2024 12:23 AM, Konrad Dybcio wrote:
> On 27.09.2024 8:31 AM, Mukesh Kumar Savaliya wrote:
>> Currently the driver provides a function called geni_serial_resources_off()
>> to turn off resources like clocks and  pinctrl.
>>
>> For shared SE between two SS, we don't need to keep pinctrl to sleep state
>> as other SS may be actively transferring data over SE. Hence,bypass keeping
>> pinctrl to sleep state conditionally using shared_geni_se flag.
>>
>> Signed-off-by: Mukesh Kumar Savaliya <quic_msavaliy@...cinc.com>
>> ---
>>   drivers/soc/qcom/qcom-geni-se.c  | 14 ++++++++++----
>>   include/linux/soc/qcom/geni-se.h |  3 +++
>>   2 files changed, 13 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
>> index 2e8f24d5da80..89cf18699336 100644
>> --- a/drivers/soc/qcom/qcom-geni-se.c
>> +++ b/drivers/soc/qcom/qcom-geni-se.c
>> @@ -1,5 +1,6 @@
>>   // SPDX-License-Identifier: GPL-2.0
>>   // Copyright (c) 2017-2018, The Linux Foundation. All rights reserved.
>> +// Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
>>   
>>   /* Disable MMIO tracing to prevent excessive logging of unwanted MMIO traces */
>>   #define __DISABLE_TRACE_MMIO__
>> @@ -503,10 +504,15 @@ int geni_se_resources_off(struct geni_se *se)
>>   
>>   	if (has_acpi_companion(se->dev))
>>   		return 0;
>> -
>> -	ret = pinctrl_pm_select_sleep_state(se->dev);
>> -	if (ret)
>> -		return ret;
>> +	/* Keep pinctrl to sleep state only for regular usecase.
>> +	 * Do not sleep pinctrl for shared SE because other SS(subsystems)
>> +	 * may continueto perform transfer.
>> +	 */
> 
> /*
>   * Don't alter pin states on shared SEs to avoid potentially
>   * interrupting transfers started by other subsystems
>   */
> 
Done
> 
>> +	if (se->shared_geni_se == false) {
> 
> if (!se->shared_geni_se)
Done
> 
>> +		ret = pinctrl_pm_select_sleep_state(se->dev);
>> +		if (ret)
>> +			return ret;
>> +	}
>>   
>>   	geni_se_clks_off(se);
> 
> Should the clocks be turned off?
> 
Yes, it's required to be turned off.
> Konrad