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

[Konrad Dybcio]

On 2/23/26 2:39 PM, Praveen Talari wrote:
> Hi Konrad,
> 
> On 2/17/2026 5:32 PM, Konrad Dybcio wrote:
>> On 2/6/26 6:41 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@xxxxxxxxxxxxxxxx>
>>> ---
>>> v4 -> v5
>>> Konrad
>>> - updated return value as 0 for geni_se_resources_activate().
>>
>> I would really like you to include that kerneldoc bit I mentioned it
>>
>> https://lore.kernel.org/linux-arm-msm/ff9a2aa3-1b2b-4001-82ce-198c61a4b637@xxxxxxxxxxxxxxxx/
> 
> I hope the below works for you.

Tracking the rate internally with drivers potentially doing more work to
change it introduces more room for error. I'm fine with the current
state of this patch, so long as you

> +/**
> + * geni_se_resources_activate() - Activate GENI SE device resources
> + * @se: Pointer to the geni_se structure
> + *
> + * Activates device resources for operation: enables interconnect, prepares clocks,
> + * and sets pin control to default state. Includes error cleanup. Skips ACPI devices.

use this kerneldoc and include a paragraph saying:

"Unlike geni_se_resources_deactivate(), this function doesn't alter the
connected genpds' performance states, which must be additionally handled."

We may find a better solution to this down the road, perhaps including a
factoring-out of the clock configuration writes in the non-scmi case (they
seem largely similar), but that's an improvement that doesn't have to block
this series

Konrad