mbox series

[0/3] Add EPSS L3 provider support on SA8775P SoC

Message ID 20240904171209.29120-1-quic_rlaggysh@quicinc.com
Headers show
Series Add EPSS L3 provider support on SA8775P SoC | expand

Message

Raviteja Laggyshetty Sept. 4, 2024, 5:12 p.m. UTC 
Add Epoch Subsystem (EPSS) L3 provider support on SA8775P SoCs.

Raviteja Laggyshetty (3):
  dt-bindings: interconnect: Add EPSS L3 compatible for SA8775P
  arm64: dts: qcom: sa8775p: add EPSS l3 interconnect provider
  interconnect: qcom: Add EPSS L3 support on SA8775P

 .../bindings/interconnect/qcom,osm-l3.yaml    |  2 ++
 arch/arm64/boot/dts/qcom/sa8775p.dtsi         | 19 +++++++++++++
 drivers/interconnect/qcom/osm-l3.c            | 27 +++++++++++++++++++
 3 files changed, 48 insertions(+)

Comments

Krzysztof Kozlowski Sept. 4, 2024, 6:22 p.m. UTC | #1 
On 04/09/2024 19:12, Raviteja Laggyshetty wrote:
> +
>  static const struct qcom_osm_l3_desc epss_l3_l3_vote = {
>  	.nodes = epss_l3_nodes,
>  	.num_nodes = ARRAY_SIZE(epss_l3_nodes),
> @@ -284,6 +307,10 @@ static const struct of_device_id osm_l3_of_match[] = {
>  	{ .compatible = "qcom,sm8150-osm-l3", .data = &osm_l3 },
>  	{ .compatible = "qcom,sc8180x-osm-l3", .data = &osm_l3 },
>  	{ .compatible = "qcom,sm8250-epss-l3", .data = &epss_l3_perf_state },
> +	{ .compatible = "qcom,sa8775p-epss-l3-cl0",
> +	  .data = &epss_l3_perf_state },

Don't grow it but express compatibility.

Best regards,
Krzysztof
Krzysztof Kozlowski Sept. 6, 2024, 4:30 p.m. UTC | #2 
On 06/09/2024 17:32, Raviteja Laggyshetty wrote:
> 
> On 9/4/2024 11:52 PM, Krzysztof Kozlowski wrote:
>> On 04/09/2024 19:12, Raviteja Laggyshetty wrote:
>>> +
>>>  static const struct qcom_osm_l3_desc epss_l3_l3_vote = {
>>>  	.nodes = epss_l3_nodes,
>>>  	.num_nodes = ARRAY_SIZE(epss_l3_nodes),
>>> @@ -284,6 +307,10 @@ static const struct of_device_id osm_l3_of_match[] = {
>>>  	{ .compatible = "qcom,sm8150-osm-l3", .data = &osm_l3 },
>>>  	{ .compatible = "qcom,sc8180x-osm-l3", .data = &osm_l3 },
>>>  	{ .compatible = "qcom,sm8250-epss-l3", .data = &epss_l3_perf_state },
>>> +	{ .compatible = "qcom,sa8775p-epss-l3-cl0",
>>> +	  .data = &epss_l3_perf_state },
>> Don't grow it but express compatibility.
> ok. Will rename compatible from "qcom,sa8775p-epss-l3-cl0" to "qcom,sa8775p-epss-l3".

This won't solve the problem. You still grow the table, right?

Best regards,
Krzysztof
Konrad Dybcio Sept. 9, 2024, 11:44 a.m. UTC | #3 
On 4.09.2024 7:12 PM, Raviteja Laggyshetty wrote:
> Add Epoch Subsystem (EPSS) L3 interconnect provider support on
> SA8775P SoCs.
> 
> Signed-off-by: Raviteja Laggyshetty <quic_rlaggysh@quicinc.com>
> ---
>  drivers/interconnect/qcom/osm-l3.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/interconnect/qcom/osm-l3.c b/drivers/interconnect/qcom/osm-l3.c
> index 61a8695a9adc..e97d61a9d8d7 100644
> --- a/drivers/interconnect/qcom/osm-l3.c
> +++ b/drivers/interconnect/qcom/osm-l3.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0
>  /*
>   * Copyright (c) 2020-2021, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
>   */
>  
>  #include <linux/args.h>
> @@ -74,6 +75,11 @@ enum {
>  	OSM_L3_SLAVE_NODE,
>  };
>  
> +enum {
> +	EPSS_L3_CL1_MASTER_NODE = 20000,
> +	EPSS_L3_CL1_SLAVE_NODE,
> +};
> +
>  #define DEFINE_QNODE(_name, _id, _buswidth, ...)			\
>  	static const struct qcom_osm_l3_node _name = {			\
>  		.name = #_name,						\
> @@ -99,6 +105,15 @@ static const struct qcom_osm_l3_node * const epss_l3_nodes[] = {
>  	[SLAVE_EPSS_L3_SHARED] = &epss_l3_slave,
>  };
>  
> +DEFINE_QNODE(epss_l3_cl1_master, EPSS_L3_CL1_MASTER_NODE, 32,
> +	     EPSS_L3_CL1_SLAVE_NODE);
> +DEFINE_QNODE(epss_l3_cl1_slave, EPSS_L3_CL1_SLAVE_NODE, 32);
> +
> +static const struct qcom_osm_l3_node * const epss_l3_cl1_nodes[] = {
> +	[MASTER_EPSS_L3_APPS] = &epss_l3_cl1_master,
> +	[SLAVE_EPSS_L3_SHARED] = &epss_l3_cl1_slave,
> +};
> +
>  static const struct qcom_osm_l3_desc osm_l3 = {
>  	.nodes = osm_l3_nodes,
>  	.num_nodes = ARRAY_SIZE(osm_l3_nodes),
> @@ -115,6 +130,14 @@ static const struct qcom_osm_l3_desc epss_l3_perf_state = {
>  	.reg_perf_state = EPSS_REG_PERF_STATE,
>  };
>  
> +static const struct qcom_osm_l3_desc epss_l3_cl1_perf_state = {
> +	.nodes = epss_l3_cl1_nodes,
> +	.num_nodes = ARRAY_SIZE(epss_l3_cl1_nodes),
> +	.lut_row_size = EPSS_LUT_ROW_SIZE,
> +	.reg_freq_lut = EPSS_REG_FREQ_LUT,
> +	.reg_perf_state = EPSS_REG_PERF_STATE,
> +};

This is a bad workaround for the unfortunate interconnect API choices
(conflicting ICC IDs), in no way specific to this platform

> +
>  static const struct qcom_osm_l3_desc epss_l3_l3_vote = {
>  	.nodes = epss_l3_nodes,
>  	.num_nodes = ARRAY_SIZE(epss_l3_nodes),
> @@ -284,6 +307,10 @@ static const struct of_device_id osm_l3_of_match[] = {
>  	{ .compatible = "qcom,sm8150-osm-l3", .data = &osm_l3 },
>  	{ .compatible = "qcom,sc8180x-osm-l3", .data = &osm_l3 },
>  	{ .compatible = "qcom,sm8250-epss-l3", .data = &epss_l3_perf_state },
> +	{ .compatible = "qcom,sa8775p-epss-l3-cl0",
> +	  .data = &epss_l3_perf_state },

Reuse qcom,sm8250-epss-l3, like:

compatible = "qcom,foobar-epss-l3", "qcom,sm8250-epss-l3";

Konrad
Raviteja Laggyshetty Oct. 25, 2024, 3:38 p.m. UTC | #4 
On 9/6/2024 10:00 PM, Krzysztof Kozlowski wrote:
> On 06/09/2024 17:32, Raviteja Laggyshetty wrote:
>>
>> On 9/4/2024 11:52 PM, Krzysztof Kozlowski wrote:
>>> On 04/09/2024 19:12, Raviteja Laggyshetty wrote:
>>>> +
>>>>  static const struct qcom_osm_l3_desc epss_l3_l3_vote = {
>>>>  	.nodes = epss_l3_nodes,
>>>>  	.num_nodes = ARRAY_SIZE(epss_l3_nodes),
>>>> @@ -284,6 +307,10 @@ static const struct of_device_id osm_l3_of_match[] = {
>>>>  	{ .compatible = "qcom,sm8150-osm-l3", .data = &osm_l3 },
>>>>  	{ .compatible = "qcom,sc8180x-osm-l3", .data = &osm_l3 },
>>>>  	{ .compatible = "qcom,sm8250-epss-l3", .data = &epss_l3_perf_state },
>>>> +	{ .compatible = "qcom,sa8775p-epss-l3-cl0",
>>>> +	  .data = &epss_l3_perf_state },
>>> Don't grow it but express compatibility.
>> ok. Will rename compatible from "qcom,sa8775p-epss-l3-cl0" to "qcom,sa8775p-epss-l3".
> 
> This won't solve the problem. You still grow the table, right?

Falling back to "qcom,epss-l3" won't work because we need to vote into perf state register.
I am introducing a new fallback compatible "qcom,epss-l3-perf" for perf voting, which can be used for upcoming qcs8300.

epss_l3_cl0: interconnect@18590000 {
       compatible = "qcom,sa8775p-epss-l3", "qcom,epss-l3-perf";	   
};

> 
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski Oct. 26, 2024, 11:28 a.m. UTC | #5 
On 25/10/2024 17:38, Raviteja Laggyshetty wrote:
> 
> 
> On 9/6/2024 10:00 PM, Krzysztof Kozlowski wrote:
>> On 06/09/2024 17:32, Raviteja Laggyshetty wrote:
>>>
>>> On 9/4/2024 11:52 PM, Krzysztof Kozlowski wrote:
>>>> On 04/09/2024 19:12, Raviteja Laggyshetty wrote:
>>>>> +
>>>>>  static const struct qcom_osm_l3_desc epss_l3_l3_vote = {
>>>>>  	.nodes = epss_l3_nodes,
>>>>>  	.num_nodes = ARRAY_SIZE(epss_l3_nodes),
>>>>> @@ -284,6 +307,10 @@ static const struct of_device_id osm_l3_of_match[] = {
>>>>>  	{ .compatible = "qcom,sm8150-osm-l3", .data = &osm_l3 },
>>>>>  	{ .compatible = "qcom,sc8180x-osm-l3", .data = &osm_l3 },
>>>>>  	{ .compatible = "qcom,sm8250-epss-l3", .data = &epss_l3_perf_state },
>>>>> +	{ .compatible = "qcom,sa8775p-epss-l3-cl0",
>>>>> +	  .data = &epss_l3_perf_state },
>>>> Don't grow it but express compatibility.
>>> ok. Will rename compatible from "qcom,sa8775p-epss-l3-cl0" to "qcom,sa8775p-epss-l3".
>>
>> This won't solve the problem. You still grow the table, right?
> 
> Falling back to "qcom,epss-l3" won't work because we need to vote into perf state register.
> I am introducing a new fallback compatible "qcom,epss-l3-perf" for perf voting, which can be used for upcoming qcs8300.

Maybe, no clue, this was 1.5 months ago. I don't have original patches
in the inbox anymore.

Just choose something sensible following writing bindings guideline.

Best regards,
Krzysztof
Konrad Dybcio Oct. 26, 2024, 12:24 p.m. UTC | #6 
On 26.10.2024 1:28 PM, Krzysztof Kozlowski wrote:
> On 25/10/2024 17:38, Raviteja Laggyshetty wrote:
>>
>>
>> On 9/6/2024 10:00 PM, Krzysztof Kozlowski wrote:
>>> On 06/09/2024 17:32, Raviteja Laggyshetty wrote:
>>>>
>>>> On 9/4/2024 11:52 PM, Krzysztof Kozlowski wrote:
>>>>> On 04/09/2024 19:12, Raviteja Laggyshetty wrote:
>>>>>> +
>>>>>>  static const struct qcom_osm_l3_desc epss_l3_l3_vote = {
>>>>>>  	.nodes = epss_l3_nodes,
>>>>>>  	.num_nodes = ARRAY_SIZE(epss_l3_nodes),
>>>>>> @@ -284,6 +307,10 @@ static const struct of_device_id osm_l3_of_match[] = {
>>>>>>  	{ .compatible = "qcom,sm8150-osm-l3", .data = &osm_l3 },
>>>>>>  	{ .compatible = "qcom,sc8180x-osm-l3", .data = &osm_l3 },
>>>>>>  	{ .compatible = "qcom,sm8250-epss-l3", .data = &epss_l3_perf_state },
>>>>>> +	{ .compatible = "qcom,sa8775p-epss-l3-cl0",
>>>>>> +	  .data = &epss_l3_perf_state },
>>>>> Don't grow it but express compatibility.
>>>> ok. Will rename compatible from "qcom,sa8775p-epss-l3-cl0" to "qcom,sa8775p-epss-l3".
>>>
>>> This won't solve the problem. You still grow the table, right?
>>
>> Falling back to "qcom,epss-l3" won't work because we need to vote into perf state register.
>> I am introducing a new fallback compatible "qcom,epss-l3-perf" for perf voting, which can be used for upcoming qcs8300.
> 
> Maybe, no clue, this was 1.5 months ago. I don't have original patches
> in the inbox anymore.
> 
> Just choose something sensible following writing bindings guideline.

You can see that qcom,sm8250-epss-l3 uses the same match data, so that
sounds like a good fit

Konrad
Krzysztof Kozlowski Oct. 26, 2024, 12:26 p.m. UTC | #7 
On 26/10/2024 14:24, Konrad Dybcio wrote:
> On 26.10.2024 1:28 PM, Krzysztof Kozlowski wrote:
>> On 25/10/2024 17:38, Raviteja Laggyshetty wrote:
>>>
>>>
>>> On 9/6/2024 10:00 PM, Krzysztof Kozlowski wrote:
>>>> On 06/09/2024 17:32, Raviteja Laggyshetty wrote:
>>>>>
>>>>> On 9/4/2024 11:52 PM, Krzysztof Kozlowski wrote:
>>>>>> On 04/09/2024 19:12, Raviteja Laggyshetty wrote:
>>>>>>> +
>>>>>>>  static const struct qcom_osm_l3_desc epss_l3_l3_vote = {
>>>>>>>  	.nodes = epss_l3_nodes,
>>>>>>>  	.num_nodes = ARRAY_SIZE(epss_l3_nodes),
>>>>>>> @@ -284,6 +307,10 @@ static const struct of_device_id osm_l3_of_match[] = {
>>>>>>>  	{ .compatible = "qcom,sm8150-osm-l3", .data = &osm_l3 },
>>>>>>>  	{ .compatible = "qcom,sc8180x-osm-l3", .data = &osm_l3 },
>>>>>>>  	{ .compatible = "qcom,sm8250-epss-l3", .data = &epss_l3_perf_state },
>>>>>>> +	{ .compatible = "qcom,sa8775p-epss-l3-cl0",
>>>>>>> +	  .data = &epss_l3_perf_state },
>>>>>> Don't grow it but express compatibility.
>>>>> ok. Will rename compatible from "qcom,sa8775p-epss-l3-cl0" to "qcom,sa8775p-epss-l3".
>>>>
>>>> This won't solve the problem. You still grow the table, right?
>>>
>>> Falling back to "qcom,epss-l3" won't work because we need to vote into perf state register.
>>> I am introducing a new fallback compatible "qcom,epss-l3-perf" for perf voting, which can be used for upcoming qcs8300.
>>
>> Maybe, no clue, this was 1.5 months ago. I don't have original patches
>> in the inbox anymore.
>>
>> Just choose something sensible following writing bindings guideline.
> 
> You can see that qcom,sm8250-epss-l3 uses the same match data, so that
> sounds like a good fit

Yep, so probably this was obvious to me when I wrote above comment and I
just don't get why fallbacking to qcom,sa8775p-epss-l3...

Best regards,
Krzysztof