linux-kernel - Re: [PATCH v4 05/13] soc: qcom: geni-se: Add resources activation/deactivation helpers

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: <06d16c97-5db0-4c2d-9d30-68fc2ef1c5c0@oss.qualcomm.com>
Date: Wed, 4 Feb 2026 10:48:47 +0530
From: Praveen Talari <praveen.talari@....qualcomm.com>
To: Konrad Dybcio <konrad.dybcio@....qualcomm.com>,
        Andi Shyti <andi.shyti@...nel.org>, Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley
 <conor+dt@...nel.org>,
        Mukesh Kumar Savaliya <mukesh.savaliya@....qualcomm.com>,
        Viken Dadhaniya <viken.dadhaniya@....qualcomm.com>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konradybcio@...nel.org>, linux-arm-msm@...r.kernel.org,
        linux-i2c@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, bjorn.andersson@....qualcomm.com,
        dmitry.baryshkov@....qualcomm.com
Cc: prasad.sodagudi@....qualcomm.com, quic_vtanuku@...cinc.com,
        aniket.randive@....qualcomm.com, chandana.chiluveru@....qualcomm.com,
        jyothi.seerapu@....qualcomm.com
Subject: Re: [PATCH v4 05/13] soc: qcom: geni-se: Add resources
 activation/deactivation helpers

Hi Konrad,

On 2/3/2026 5:50 PM, Konrad Dybcio wrote:
> On 2/2/26 7:09 PM, Praveen Talari wrote:
>> The GENI SE protocol drivers (I2C, SPI, UART) implement similar resource
>> activation/deactivation sequences independently, leading to code
>> duplication.
>>
>> Introduce geni_se_resources_activate()/geni_se_resources_deactivate() to
>> power on/off resources.The activate function enables ICC, clocks, and TLMM
>> whereas the deactivate function disables resources in reverse order
>> including OPP rate reset, clocks, ICC and TLMM.
>>
>> Signed-off-by: Praveen Talari <praveen.talari@....qualcomm.com>
>> ---
> 
> [...]
> 
>> +int geni_se_resources_deactivate(struct geni_se *se)
>> +{
>> +	int ret;
>> +
>> +	if (has_acpi_companion(se->dev))
>> +		return 0;
>> +
>> +	if (se->has_opp)
>> +		dev_pm_opp_set_rate(se->dev, 0);
> 
> This is still unbalanced at this point of abstraction, notably
> keeping the RPMh vote at 0 permanently after the first
> geni_se_resources_deactivate()  since there's no counterpart in
> _activate()

I don’t think we need a counterpart for this in the activate path, since 
it is specific to dropping the vote during suspend. The vote will anyway 
be taken again as part of the transfer after the device resumes.

Thanks,
Praveen Talari

> 
> That said, the serial and UART drivers do rate calculations internally,
> so perhaps trying to be overly smart about it wouldn't be a good thing
> either.. Let's add a note in kerneldoc that the activate must be preceded
> by a dev_pm_opp_set_xyz()
> 
> [...]
> 
>> +int geni_se_resources_activate(struct geni_se *se)
>> +{
>> +	int ret;
>> +
>> +	if (has_acpi_companion(se->dev))
>> +		return 0;
>> +
>> +	ret = geni_icc_enable(se);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = geni_se_clks_on(se);
>> +	if (ret)
>> +		goto out_icc_disable;
>> +
>> +	ret = pinctrl_pm_select_default_state(se->dev);
>> +	if (ret) {
>> +		geni_se_clks_off(se);
>> +		goto out_icc_disable;
>> +	}
>> +
>> +	return ret;
> 
> nit: this 'return' always returns 0
> 
> Konrad
> 
>> +
>> +out_icc_disable:
>> +	geni_icc_disable(se);
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(geni_se_resources_activate);
>> +
>>   /**
>>    * geni_se_resources_init() - Initialize resources for a GENI SE device.
>>    * @se: Pointer to the geni_se structure representing the GENI SE device.
>> diff --git a/include/linux/soc/qcom/geni-se.h b/include/linux/soc/qcom/geni-se.h
>> index c182dd0f0bde..36a68149345c 100644
>> --- a/include/linux/soc/qcom/geni-se.h
>> +++ b/include/linux/soc/qcom/geni-se.h
>> @@ -541,6 +541,10 @@ int geni_icc_disable(struct geni_se *se);
>>   
>>   int geni_se_resources_init(struct geni_se *se);
>>   
>> +int geni_se_resources_activate(struct geni_se *se);
>> +
>> +int geni_se_resources_deactivate(struct geni_se *se);
>> +
>>   int geni_load_se_firmware(struct geni_se *se, enum geni_se_protocol_type protocol);
>>   #endif
>>   #endif