Re: [PATCH v1 4/9] soc: qcom: geni-se: Enable QUPs on SA8255p Qualcomm platforms

Praveen Talari
2025-04-14T18:00:10Z

On 4/14/2025 1:26 PM, Jiri Slaby wrote:
> On 10. 04. 25, 19:40, Praveen Talari wrote:
>> On the sa8255p platform, resources such as clocks,interconnects
>> and TLMM (GPIO) configurations are managed by firmware.
>>
>> Introduce a platform data function callback to distinguish whether
>> resource control is performed by firmware or directly by the driver
>> in linux.
>>
>> The refactor ensures clear differentiation of resource
>> management mechanisms, improving maintainability and flexibility
>> in handling platform-specific configurations.
>>
>> Signed-off-by: Praveen Talari <quic_ptalari@quicinc.com>
>> ---
>>   drivers/soc/qcom/qcom-geni-se.c | 78 +++++++++++++++++++++------------
>>   1 file changed, 50 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/soc/qcom/qcom-geni-se.c
>> b/drivers/soc/qcom/qcom-geni-se.c
>> index 4cb959106efa..5e2add1e04d3 100644
>> --- a/drivers/soc/qcom/qcom-geni-se.c
>> +++ b/drivers/soc/qcom/qcom-geni-se.c
>> @@ -105,6 +105,8 @@ struct geni_wrapper {
>>   struct geni_se_desc {
>>       unsigned int num_clks;
>>       const char * const *clks;
>> +    int (*geni_se_rsc_init)(struct geni_wrapper *wrapper,
>> +                const struct geni_se_desc *desc);
>>   };
>>     static const char * const icc_path_names[] = {"qup-core",
>> "qup-config",
>> @@ -891,10 +893,44 @@ int geni_icc_disable(struct geni_se *se)
>>   }
>>   EXPORT_SYMBOL_GPL(geni_icc_disable);
>>   +static int geni_se_resource_init(struct geni_wrapper *wrapper,
>> +                 const struct geni_se_desc *desc)
>> +{
>> +    struct device *dev = wrapper->dev;
>> +    int ret;
>> +    int i;
>> +
>> +    wrapper->num_clks = min_t(unsigned int, desc->num_clks, MAX_CLKS);
>
> I thought min_t is no longer needed in these cases?

it is needed since there are two types of QUPs - the normal QUP which
uses 2 clocks,

and HUB QUP which uses only 1 clock.

>
>> +
>> +    for (i = 0; i < wrapper->num_clks; ++i)
>
> FWIW i should be unsigned too.
>
> thanks,