diff mbox series

[v1] arm64: dts: rockchip: Add cache information to the Rockchip RK3566 and RK3568 SoC

Message ID 20240226182310.4032-1-linux.amoon@gmail.com (mailing list archive)
State New, archived
Headers show
Series [v1] arm64: dts: rockchip: Add cache information to the Rockchip RK3566 and RK3568 SoC | expand

Commit Message

Anand Moon Feb. 26, 2024, 6:23 p.m. UTC
As per RK3568 Datasheet and TRM add missing cache information to
the Rockchip RK3566 and RK3568 SoC.

- Each Cortex-A55 core has 32KB of L1 instruction cache available and
	32KB of L1 data cache available with ECC.
- Along with 512KB Unified L3 cache with ECC.

With adding instruction cache and data cache and a write buffer to
reduce the effect of main memory bandwidth and latency on data
access performance.

Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
[0] http://www.rock-chips.com/uploads/pdf/2022.8.26/191/RK3568%20Brief%20Datasheet.pdf
[1] https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part1%20V1.1-20210301.pdf
---
 arch/arm64/boot/dts/rockchip/rk356x.dtsi | 37 ++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

Comments

Dragan Simic Feb. 26, 2024, 7:09 p.m. UTC | #1
Hello Anand,

On 2024-02-26 19:23, Anand Moon wrote:
> As per RK3568 Datasheet and TRM add missing cache information to
> the Rockchip RK3566 and RK3568 SoC.
> 
> - Each Cortex-A55 core has 32KB of L1 instruction cache available and
> 	32KB of L1 data cache available with ECC.
> - Along with 512KB Unified L3 cache with ECC.
> 
> With adding instruction cache and data cache and a write buffer to
> reduce the effect of main memory bandwidth and latency on data
> access performance.
> 
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>

I was about to send my own patch that adds the same missing cache
information, so please allow me to describe the proposed way to move
forward.

The way I see it, your commit summary and description need a rather
complete rewrite, to be more readable, more accurate, and to avoid
including an irrelevant (and slightly misleading) description of the
general role of caches.

Also, the changes to the dtsi file would benefit from small touch-ups
here and there, for improved consistency, etc.

With all that in mind, I propose that you withdraw your patch and let
me send my patch that will addresses all these issues, of course with
a proper tag that lists you as a co-developer.  I think that would
save us a fair amount of time going back and forth.

I hope you agree.


> ---
> [0] 
> http://www.rock-chips.com/uploads/pdf/2022.8.26/191/RK3568%20Brief%20Datasheet.pdf
> [1] 
> https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part1%20V1.1-20210301.pdf
> ---
>  arch/arm64/boot/dts/rockchip/rk356x.dtsi | 37 ++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> index c19c0f1b3778..49235efefb6b 100644
> --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> @@ -56,6 +56,13 @@ cpu0: cpu@0 {
>  			clocks = <&scmi_clk 0>;
>  			#cooling-cells = <2>;
>  			enable-method = "psci";
> +			d-cache-line-size = <32>;
> +			d-cache-size = <0x8000>;
> +			d-cache-sets = <32>;
> +			i-cache-line-size = <32>;
> +			i-cache-size = <0x8000>;
> +			i-cache-sets = <32>;
> +			next-level-cache = <&l2>;
>  			operating-points-v2 = <&cpu0_opp_table>;
>  		};
> 
> @@ -65,6 +72,13 @@ cpu1: cpu@100 {
>  			reg = <0x0 0x100>;
>  			#cooling-cells = <2>;
>  			enable-method = "psci";
> +			d-cache-line-size = <32>;
> +			d-cache-size = <0x8000>;
> +			d-cache-sets = <32>;
> +			i-cache-line-size = <32>;
> +			i-cache-size = <0x8000>;
> +			i-cache-sets = <32>;
> +			next-level-cache = <&l2>;
>  			operating-points-v2 = <&cpu0_opp_table>;
>  		};
> 
> @@ -74,6 +88,13 @@ cpu2: cpu@200 {
>  			reg = <0x0 0x200>;
>  			#cooling-cells = <2>;
>  			enable-method = "psci";
> +			d-cache-line-size = <32>;
> +			d-cache-size = <0x8000>;
> +			d-cache-sets = <32>;
> +			i-cache-line-size = <32>;
> +			i-cache-size = <0x8000>;
> +			i-cache-sets = <32>;
> +			next-level-cache = <&l2>;
>  			operating-points-v2 = <&cpu0_opp_table>;
>  		};
> 
> @@ -83,8 +104,24 @@ cpu3: cpu@300 {
>  			reg = <0x0 0x300>;
>  			#cooling-cells = <2>;
>  			enable-method = "psci";
> +			d-cache-line-size = <32>;
> +			d-cache-size = <0x8000>;
> +			d-cache-sets = <32>;
> +			i-cache-line-size = <32>;
> +			i-cache-size = <0x8000>;
> +			i-cache-sets = <32>;
> +			next-level-cache = <&l2>;
>  			operating-points-v2 = <&cpu0_opp_table>;
>  		};
> +
> +		l2: l2-cache0 {
> +			compatible = "cache";
> +			cache-level = <3>;
> +			cache-unified;
> +			cache-size = <0x7d000>; /* L3. 512 KB */
> +			cache-line-size = <64>;
> +			cache-sets = <512>;
> +		};
>  	};
> 
>  	cpu0_opp_table: opp-table-0 {
Anand Moon Feb. 27, 2024, 12:49 p.m. UTC | #2
Hi Dragan

On Tue, 27 Feb 2024 at 00:39, Dragan Simic <dsimic@manjaro.org> wrote:
>
> Hello Anand,
>
> On 2024-02-26 19:23, Anand Moon wrote:
> > As per RK3568 Datasheet and TRM add missing cache information to
> > the Rockchip RK3566 and RK3568 SoC.
> >
> > - Each Cortex-A55 core has 32KB of L1 instruction cache available and
> >       32KB of L1 data cache available with ECC.
> > - Along with 512KB Unified L3 cache with ECC.
> >
> > With adding instruction cache and data cache and a write buffer to
> > reduce the effect of main memory bandwidth and latency on data
> > access performance.
> >
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
>
> I was about to send my own patch that adds the same missing cache
> information, so please allow me to describe the proposed way to move
> forward.
>
> The way I see it, your commit summary and description need a rather
> complete rewrite, to be more readable, more accurate, and to avoid
> including an irrelevant (and slightly misleading) description of the
> general role of caches.
>
> Also, the changes to the dtsi file would benefit from small touch-ups
> here and there, for improved consistency, etc.
>
> With all that in mind, I propose that you withdraw your patch and let
> me send my patch that will addresses all these issues, of course with
> a proper tag that lists you as a co-developer.  I think that would
> save us a fair amount of time going back and forth.
>
> I hope you agree.
>

I have no issue with this,.If you have a better version plz share this.

Thanks
-Anand
Dragan Simic Feb. 27, 2024, 2:58 p.m. UTC | #3
On 2024-02-27 13:49, Anand Moon wrote:
> On Tue, 27 Feb 2024 at 00:39, Dragan Simic <dsimic@manjaro.org> wrote:
>> On 2024-02-26 19:23, Anand Moon wrote:
>> > As per RK3568 Datasheet and TRM add missing cache information to
>> > the Rockchip RK3566 and RK3568 SoC.
>> >
>> > - Each Cortex-A55 core has 32KB of L1 instruction cache available and
>> >       32KB of L1 data cache available with ECC.
>> > - Along with 512KB Unified L3 cache with ECC.
>> >
>> > With adding instruction cache and data cache and a write buffer to
>> > reduce the effect of main memory bandwidth and latency on data
>> > access performance.
>> >
>> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
>> 
>> I was about to send my own patch that adds the same missing cache
>> information, so please allow me to describe the proposed way to move
>> forward.
>> 
>> The way I see it, your commit summary and description need a rather
>> complete rewrite, to be more readable, more accurate, and to avoid
>> including an irrelevant (and slightly misleading) description of the
>> general role of caches.
>> 
>> Also, the changes to the dtsi file would benefit from small touch-ups
>> here and there, for improved consistency, etc.
>> 
>> With all that in mind, I propose that you withdraw your patch and let
>> me send my patch that will addresses all these issues, of course with
>> a proper tag that lists you as a co-developer.  I think that would
>> save us a fair amount of time going back and forth.
>> 
>> I hope you agree.
> 
> I have no issue with this,.If you have a better version plz share this.

Thank you, I'll send my patch within the next couple of days.
Dragan Simic Feb. 28, 2024, 10:42 a.m. UTC | #4
Hello Anand,

On 2024-02-27 15:58, Dragan Simic wrote:
> On 2024-02-27 13:49, Anand Moon wrote:
>> On Tue, 27 Feb 2024 at 00:39, Dragan Simic <dsimic@manjaro.org> wrote:
>>> On 2024-02-26 19:23, Anand Moon wrote:
>>> > As per RK3568 Datasheet and TRM add missing cache information to
>>> > the Rockchip RK3566 and RK3568 SoC.
>>> >
>>> > - Each Cortex-A55 core has 32KB of L1 instruction cache available and
>>> >       32KB of L1 data cache available with ECC.
>>> > - Along with 512KB Unified L3 cache with ECC.
>>> >
>>> > With adding instruction cache and data cache and a write buffer to
>>> > reduce the effect of main memory bandwidth and latency on data
>>> > access performance.
>>> >
>>> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
>>> 
>>> I was about to send my own patch that adds the same missing cache
>>> information, so please allow me to describe the proposed way to move
>>> forward.
>>> 
>>> The way I see it, your commit summary and description need a rather
>>> complete rewrite, to be more readable, more accurate, and to avoid
>>> including an irrelevant (and slightly misleading) description of the
>>> general role of caches.
>>> 
>>> Also, the changes to the dtsi file would benefit from small touch-ups
>>> here and there, for improved consistency, etc.
>>> 
>>> With all that in mind, I propose that you withdraw your patch and let
>>> me send my patch that will addresses all these issues, of course with
>>> a proper tag that lists you as a co-developer.  I think that would
>>> save us a fair amount of time going back and forth.
>>> 
>>> I hope you agree.
>> 
>> I have no issue with this,.If you have a better version plz share 
>> this.
> 
> Thank you, I'll send my patch within the next couple of days.

Here's a brief update...  Basically, not all of the cache-size values
found in your patch were correct, but I've got all of them calculated
again, double-checked, and cross-compared with the way values in my
earlier patch for the RK3399 SoC dtsi were calculated. [2]

It all checked out just fine.  It's all based on the RK3566 and RK3568
SoC datasheets and a couple of ARM specifications, which I'll describe
in detail in my patch description.  I'll send the patch after I test
it a bit, to make sure it all works as expected.

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=b72633ba5cfa932405832de25d0f0a11716903b4
Dragan Simic Feb. 28, 2024, 5:50 p.m. UTC | #5
On 2024-02-28 11:42, Dragan Simic wrote:
> On 2024-02-27 15:58, Dragan Simic wrote:
>> On 2024-02-27 13:49, Anand Moon wrote:
>>> On Tue, 27 Feb 2024 at 00:39, Dragan Simic <dsimic@manjaro.org> 
>>> wrote:
>>>> On 2024-02-26 19:23, Anand Moon wrote:
>>>> > As per RK3568 Datasheet and TRM add missing cache information to
>>>> > the Rockchip RK3566 and RK3568 SoC.
>>>> >
>>>> > - Each Cortex-A55 core has 32KB of L1 instruction cache available and
>>>> >       32KB of L1 data cache available with ECC.
>>>> > - Along with 512KB Unified L3 cache with ECC.
>>>> >
>>>> > With adding instruction cache and data cache and a write buffer to
>>>> > reduce the effect of main memory bandwidth and latency on data
>>>> > access performance.
>>>> >
>>>> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
>>>> 
>>>> I was about to send my own patch that adds the same missing cache
>>>> information, so please allow me to describe the proposed way to move
>>>> forward.
>>>> 
>>>> The way I see it, your commit summary and description need a rather
>>>> complete rewrite, to be more readable, more accurate, and to avoid
>>>> including an irrelevant (and slightly misleading) description of the
>>>> general role of caches.
>>>> 
>>>> Also, the changes to the dtsi file would benefit from small 
>>>> touch-ups
>>>> here and there, for improved consistency, etc.
>>>> 
>>>> With all that in mind, I propose that you withdraw your patch and 
>>>> let
>>>> me send my patch that will addresses all these issues, of course 
>>>> with
>>>> a proper tag that lists you as a co-developer.  I think that would
>>>> save us a fair amount of time going back and forth.
>>>> 
>>>> I hope you agree.
>>> 
>>> I have no issue with this,.If you have a better version plz share 
>>> this.
>> 
>> Thank you, I'll send my patch within the next couple of days.
> 
> Here's a brief update...  Basically, not all of the cache-size values
> found in your patch were correct, but I've got all of them calculated
> again, double-checked, and cross-compared with the way values in my
> earlier patch for the RK3399 SoC dtsi were calculated. [2]
> 
> It all checked out just fine.  It's all based on the RK3566 and RK3568
> SoC datasheets and a couple of ARM specifications, which I'll describe
> in detail in my patch description.  I'll send the patch after I test
> it a bit, to make sure it all works as expected.
> 
> [1] 
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=b72633ba5cfa932405832de25d0f0a11716903b4

Pretty much the same patch for the RK3328 is also ready for testing.
Dragan Simic March 3, 2024, 7:10 p.m. UTC | #6
Hello Anand,

On 2024-02-28 18:50, Dragan Simic wrote:
> On 2024-02-28 11:42, Dragan Simic wrote:
>> On 2024-02-27 15:58, Dragan Simic wrote:
>>> On 2024-02-27 13:49, Anand Moon wrote:
>>>> On Tue, 27 Feb 2024 at 00:39, Dragan Simic <dsimic@manjaro.org> 
>>>> wrote:
>>>>> On 2024-02-26 19:23, Anand Moon wrote:
>>>>> > As per RK3568 Datasheet and TRM add missing cache information to
>>>>> > the Rockchip RK3566 and RK3568 SoC.
>>>>> >
>>>>> > - Each Cortex-A55 core has 32KB of L1 instruction cache available and
>>>>> >       32KB of L1 data cache available with ECC.
>>>>> > - Along with 512KB Unified L3 cache with ECC.
>>>>> >
>>>>> > With adding instruction cache and data cache and a write buffer to
>>>>> > reduce the effect of main memory bandwidth and latency on data
>>>>> > access performance.
>>>>> 
>>>>> I was about to send my own patch that adds the same missing cache
>>>>> information, so please allow me to describe the proposed way to 
>>>>> move
>>>>> forward.
>>>>> 
>>>>> The way I see it, your commit summary and description need a rather
>>>>> complete rewrite, to be more readable, more accurate, and to avoid
>>>>> including an irrelevant (and slightly misleading) description of 
>>>>> the
>>>>> general role of caches.
>>>>> 
>>>>> Also, the changes to the dtsi file would benefit from small 
>>>>> touch-ups
>>>>> here and there, for improved consistency, etc.
>>>>> 
>>>>> With all that in mind, I propose that you withdraw your patch and 
>>>>> let
>>>>> me send my patch that will addresses all these issues, of course 
>>>>> with
>>>>> a proper tag that lists you as a co-developer.  I think that would
>>>>> save us a fair amount of time going back and forth.
>>>>> 
>>>>> I hope you agree.
>>>> 
>>>> I have no issue with this,.If you have a better version plz share 
>>>> this.
>>> 
>>> Thank you, I'll send my patch within the next couple of days.
>> 
>> Here's a brief update...  Basically, not all of the cache-size values
>> found in your patch were correct, but I've got all of them calculated
>> again, double-checked, and cross-compared with the way values in my
>> earlier patch for the RK3399 SoC dtsi were calculated. [2]
>> 
>> It all checked out just fine.  It's all based on the RK3566 and RK3568
>> SoC datasheets and a couple of ARM specifications, which I'll describe
>> in detail in my patch description.  I'll send the patch after I test
>> it a bit, to make sure it all works as expected.
>> 
>> [1] 
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=b72633ba5cfa932405832de25d0f0a11716903b4
> 
> Pretty much the same patch for the RK3328 is also ready for testing.

Just sent the patches to the mailing list, please have a look. [2][3]

I've "downgraded" the previously proposed "Co-developed-by" tag in the
RK356x patch [3] to a "Helped-by" tag, just because the cache-size 
values
in your patch mostly weren't correct and, as a result, differed from the
cache-size values in my patch, making the "Co-developed-by" tag 
technically
not applicable.  For that tag to be applicable, the most important parts
of the patches need to be pretty much identical.

I hope you agree.

[2] 
https://lore.kernel.org/linux-rockchip/e61173d87f5f41af80e6f87f8820ce8d06f7c20c.1709491127.git.dsimic@manjaro.org/
[3] 
https://lore.kernel.org/linux-rockchip/2285ee41e165813011220f9469e28697923aa6e0.1709491108.git.dsimic@manjaro.org/
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
index c19c0f1b3778..49235efefb6b 100644
--- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
@@ -56,6 +56,13 @@  cpu0: cpu@0 {
 			clocks = <&scmi_clk 0>;
 			#cooling-cells = <2>;
 			enable-method = "psci";
+			d-cache-line-size = <32>;
+			d-cache-size = <0x8000>;
+			d-cache-sets = <32>;
+			i-cache-line-size = <32>;
+			i-cache-size = <0x8000>;
+			i-cache-sets = <32>;
+			next-level-cache = <&l2>;
 			operating-points-v2 = <&cpu0_opp_table>;
 		};
 
@@ -65,6 +72,13 @@  cpu1: cpu@100 {
 			reg = <0x0 0x100>;
 			#cooling-cells = <2>;
 			enable-method = "psci";
+			d-cache-line-size = <32>;
+			d-cache-size = <0x8000>;
+			d-cache-sets = <32>;
+			i-cache-line-size = <32>;
+			i-cache-size = <0x8000>;
+			i-cache-sets = <32>;
+			next-level-cache = <&l2>;
 			operating-points-v2 = <&cpu0_opp_table>;
 		};
 
@@ -74,6 +88,13 @@  cpu2: cpu@200 {
 			reg = <0x0 0x200>;
 			#cooling-cells = <2>;
 			enable-method = "psci";
+			d-cache-line-size = <32>;
+			d-cache-size = <0x8000>;
+			d-cache-sets = <32>;
+			i-cache-line-size = <32>;
+			i-cache-size = <0x8000>;
+			i-cache-sets = <32>;
+			next-level-cache = <&l2>;
 			operating-points-v2 = <&cpu0_opp_table>;
 		};
 
@@ -83,8 +104,24 @@  cpu3: cpu@300 {
 			reg = <0x0 0x300>;
 			#cooling-cells = <2>;
 			enable-method = "psci";
+			d-cache-line-size = <32>;
+			d-cache-size = <0x8000>;
+			d-cache-sets = <32>;
+			i-cache-line-size = <32>;
+			i-cache-size = <0x8000>;
+			i-cache-sets = <32>;
+			next-level-cache = <&l2>;
 			operating-points-v2 = <&cpu0_opp_table>;
 		};
+
+		l2: l2-cache0 {
+			compatible = "cache";
+			cache-level = <3>;
+			cache-unified;
+			cache-size = <0x7d000>; /* L3. 512 KB */
+			cache-line-size = <64>;
+			cache-sets = <512>;
+		};
 	};
 
 	cpu0_opp_table: opp-table-0 {