mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] arm64: dts: qcom: add missing cache node for cpu1
@ 2025-01-09 15:24 Neil Armstrong
  2025-01-09 18:30 ` Konrad Dybcio
  0 siblings, 1 reply; 5+ messages in thread
From: Neil Armstrong @ 2025-01-09 15:24 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Krzysztof Kozlowski, linux-arm-msm, devicetree, linux-kernel,
	Neil Armstrong

Add the missing l2-cache node for the cpu1

Fixes: 20eb2057b3e4 ("arm64: dts: qcom: sm8650: change labels to lower-case")
Fixes: d2350377997f ("arm64: dts: qcom: add initial SM8650 dtsi")
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8650.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
index 86684cb9a9325618ddb74458621cf4bbdc1cc0d1..832f3a2c400e8348847bc24b27397e2a0dc08db8 100644
--- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
@@ -119,6 +119,13 @@ cpu1: cpu@100 {
 			qcom,freq-domain = <&cpufreq_hw 0>;
 
 			#cooling-cells = <2>;
+
+			l2_100: l2-cache {
+				compatible = "cache";
+				cache-level = <2>;
+				cache-unified;
+				next-level-cache = <&l3_0>;
+			};
 		};
 
 		cpu2: cpu@200 {

---
base-commit: 6ecd20965bdc21b265a0671ccf36d9ad8043f5ab
change-id: 20250109-topic-sm8650-cpu1-missing-cache-8566b98abf39

Best regards,
-- 
Neil Armstrong <neil.armstrong@linaro.org>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] arm64: dts: qcom: add missing cache node for cpu1
  2025-01-09 15:24 [PATCH] arm64: dts: qcom: add missing cache node for cpu1 Neil Armstrong
@ 2025-01-09 18:30 ` Konrad Dybcio
  2025-01-10  9:44   ` Neil Armstrong
  0 siblings, 1 reply; 5+ messages in thread
From: Konrad Dybcio @ 2025-01-09 18:30 UTC (permalink / raw)
  To: Neil Armstrong, Bjorn Andersson, Konrad Dybcio, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: Krzysztof Kozlowski, linux-arm-msm, devicetree, linux-kernel

On 9.01.2025 4:24 PM, Neil Armstrong wrote:
> Add the missing l2-cache node for the cpu1
> 
> Fixes: 20eb2057b3e4 ("arm64: dts: qcom: sm8650: change labels to lower-case")
> Fixes: d2350377997f ("arm64: dts: qcom: add initial SM8650 dtsi")
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---

subject: missing `sm8650:`

>  arch/arm64/boot/dts/qcom/sm8650.dtsi | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> index 86684cb9a9325618ddb74458621cf4bbdc1cc0d1..832f3a2c400e8348847bc24b27397e2a0dc08db8 100644
> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> @@ -119,6 +119,13 @@ cpu1: cpu@100 {
>  			qcom,freq-domain = <&cpufreq_hw 0>;
>  
>  			#cooling-cells = <2>;
> +
> +			l2_100: l2-cache {
> +				compatible = "cache";
> +				cache-level = <2>;
> +				cache-unified;
> +				next-level-cache = <&l3_0>;
> +			};
>  		};

You likely wanted to hook up this new node to CPU1 as well.

Reading some Arm docs [1], it seems like with A520 specifically, both shared
and unique cache slices are permitted, depending on whether they're
implemented as single- or dual-core complexes (not to be confused with
multi-threading)

[2] suggests CA720s always have their own cache pools

In 8650's case, the slowest cluster has a shared L2 cache, whereas cores 2-7
have their own pools, so this patch is incorrect.

Konrad

[1] https://developer.arm.com/documentation/102517/0004/The-Cortex-A520--core/Cortex-A520--core-configuration-options
[2] https://developer.arm.com/documentation/102530/0001/L2-memory-system

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] arm64: dts: qcom: add missing cache node for cpu1
  2025-01-09 18:30 ` Konrad Dybcio
@ 2025-01-10  9:44   ` Neil Armstrong
  2025-01-10  9:49     ` Neil Armstrong
  0 siblings, 1 reply; 5+ messages in thread
From: Neil Armstrong @ 2025-01-10  9:44 UTC (permalink / raw)
  To: Konrad Dybcio, Bjorn Andersson, Konrad Dybcio, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: Krzysztof Kozlowski, linux-arm-msm, devicetree, linux-kernel

On 09/01/2025 19:30, Konrad Dybcio wrote:
> On 9.01.2025 4:24 PM, Neil Armstrong wrote:
>> Add the missing l2-cache node for the cpu1
>>
>> Fixes: 20eb2057b3e4 ("arm64: dts: qcom: sm8650: change labels to lower-case")
>> Fixes: d2350377997f ("arm64: dts: qcom: add initial SM8650 dtsi")
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>> ---
> 
> subject: missing `sm8650:`

Damn

> 
>>   arch/arm64/boot/dts/qcom/sm8650.dtsi | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>> index 86684cb9a9325618ddb74458621cf4bbdc1cc0d1..832f3a2c400e8348847bc24b27397e2a0dc08db8 100644
>> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>> @@ -119,6 +119,13 @@ cpu1: cpu@100 {
>>   			qcom,freq-domain = <&cpufreq_hw 0>;
>>   
>>   			#cooling-cells = <2>;
>> +
>> +			l2_100: l2-cache {
>> +				compatible = "cache";
>> +				cache-level = <2>;
>> +				cache-unified;
>> +				next-level-cache = <&l3_0>;
>> +			};
>>   		};
> 
> You likely wanted to hook up this new node to CPU1 as well.
> 
> Reading some Arm docs [1], it seems like with A520 specifically, both shared
> and unique cache slices are permitted, depending on whether they're
> implemented as single- or dual-core complexes (not to be confused with
> multi-threading)
> 
> [2] suggests CA720s always have their own cache pools
> 
> In 8650's case, the slowest cluster has a shared L2 cache, whereas cores 2-7
> have their own pools, so this patch is incorrect.

Damn you're right, so the cpu1 cache should be linked to the cpu0 cache somehow

Thanks,
Neil

> 
> Konrad
> 
> [1] https://developer.arm.com/documentation/102517/0004/The-Cortex-A520--core/Cortex-A520--core-configuration-options
> [2] https://developer.arm.com/documentation/102530/0001/L2-memory-system


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] arm64: dts: qcom: add missing cache node for cpu1
  2025-01-10  9:44   ` Neil Armstrong
@ 2025-01-10  9:49     ` Neil Armstrong
  2025-01-10 12:06       ` Konrad Dybcio
  0 siblings, 1 reply; 5+ messages in thread
From: Neil Armstrong @ 2025-01-10  9:49 UTC (permalink / raw)
  To: Konrad Dybcio, Bjorn Andersson, Konrad Dybcio, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: Krzysztof Kozlowski, linux-arm-msm, devicetree, linux-kernel

On 10/01/2025 10:44, Neil Armstrong wrote:
> On 09/01/2025 19:30, Konrad Dybcio wrote:
>> On 9.01.2025 4:24 PM, Neil Armstrong wrote:
>>> Add the missing l2-cache node for the cpu1
>>>
>>> Fixes: 20eb2057b3e4 ("arm64: dts: qcom: sm8650: change labels to lower-case")
>>> Fixes: d2350377997f ("arm64: dts: qcom: add initial SM8650 dtsi")
>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>>> ---
>>
>> subject: missing `sm8650:`
> 
> Damn
> 
>>
>>>   arch/arm64/boot/dts/qcom/sm8650.dtsi | 7 +++++++
>>>   1 file changed, 7 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>> index 86684cb9a9325618ddb74458621cf4bbdc1cc0d1..832f3a2c400e8348847bc24b27397e2a0dc08db8 100644
>>> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>> @@ -119,6 +119,13 @@ cpu1: cpu@100 {
>>>               qcom,freq-domain = <&cpufreq_hw 0>;
>>>               #cooling-cells = <2>;
>>> +
>>> +            l2_100: l2-cache {
>>> +                compatible = "cache";
>>> +                cache-level = <2>;
>>> +                cache-unified;
>>> +                next-level-cache = <&l3_0>;
>>> +            };
>>>           };
>>
>> You likely wanted to hook up this new node to CPU1 as well.
>>
>> Reading some Arm docs [1], it seems like with A520 specifically, both shared
>> and unique cache slices are permitted, depending on whether they're
>> implemented as single- or dual-core complexes (not to be confused with
>> multi-threading)
>>
>> [2] suggests CA720s always have their own cache pools
>>
>> In 8650's case, the slowest cluster has a shared L2 cache, whereas cores 2-7
>> have their own pools, so this patch is incorrect.
> 
> Damn you're right, so the cpu1 cache should be linked to the cpu0 cache somehow

Well, stupid me, it's already done... sorry for the noise and thx for your review

Neil

> 
> Thanks,
> Neil
> 
>>
>> Konrad
>>
>> [1] https://developer.arm.com/documentation/102517/0004/The-Cortex-A520--core/Cortex-A520--core-configuration-options
>> [2] https://developer.arm.com/documentation/102530/0001/L2-memory-system
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] arm64: dts: qcom: add missing cache node for cpu1
  2025-01-10  9:49     ` Neil Armstrong
@ 2025-01-10 12:06       ` Konrad Dybcio
  0 siblings, 0 replies; 5+ messages in thread
From: Konrad Dybcio @ 2025-01-10 12:06 UTC (permalink / raw)
  To: neil.armstrong, Konrad Dybcio, Bjorn Andersson, Konrad Dybcio,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Krzysztof Kozlowski, linux-arm-msm, devicetree, linux-kernel

On 10.01.2025 10:49 AM, Neil Armstrong wrote:
> On 10/01/2025 10:44, Neil Armstrong wrote:
>> On 09/01/2025 19:30, Konrad Dybcio wrote:
>>> On 9.01.2025 4:24 PM, Neil Armstrong wrote:
>>>> Add the missing l2-cache node for the cpu1
>>>>
>>>> Fixes: 20eb2057b3e4 ("arm64: dts: qcom: sm8650: change labels to lower-case")
>>>> Fixes: d2350377997f ("arm64: dts: qcom: add initial SM8650 dtsi")
>>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>>>> ---
>>>
>>> subject: missing `sm8650:`
>>
>> Damn
>>
>>>
>>>>   arch/arm64/boot/dts/qcom/sm8650.dtsi | 7 +++++++
>>>>   1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>>> index 86684cb9a9325618ddb74458621cf4bbdc1cc0d1..832f3a2c400e8348847bc24b27397e2a0dc08db8 100644
>>>> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>>> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>>> @@ -119,6 +119,13 @@ cpu1: cpu@100 {
>>>>               qcom,freq-domain = <&cpufreq_hw 0>;
>>>>               #cooling-cells = <2>;
>>>> +
>>>> +            l2_100: l2-cache {
>>>> +                compatible = "cache";
>>>> +                cache-level = <2>;
>>>> +                cache-unified;
>>>> +                next-level-cache = <&l3_0>;
>>>> +            };
>>>>           };
>>>
>>> You likely wanted to hook up this new node to CPU1 as well.
>>>
>>> Reading some Arm docs [1], it seems like with A520 specifically, both shared
>>> and unique cache slices are permitted, depending on whether they're
>>> implemented as single- or dual-core complexes (not to be confused with
>>> multi-threading)
>>>
>>> [2] suggests CA720s always have their own cache pools
>>>
>>> In 8650's case, the slowest cluster has a shared L2 cache, whereas cores 2-7
>>> have their own pools, so this patch is incorrect.
>>
>> Damn you're right, so the cpu1 cache should be linked to the cpu0 cache somehow
> 
> Well, stupid me, it's already done... sorry for the noise and thx for your review

cpu3 should still get its own one

Konrad

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-01-10 12:06 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-01-09 15:24 [PATCH] arm64: dts: qcom: add missing cache node for cpu1 Neil Armstrong
2025-01-09 18:30 ` Konrad Dybcio
2025-01-10  9:44   ` Neil Armstrong
2025-01-10  9:49     ` Neil Armstrong
2025-01-10 12:06       ` Konrad Dybcio

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®