[PATCH v3 0/2] Add Clocks for ICSSG

[PATCH v3 0/2] Add Clocks for ICSSG

[MD Danish Anwar]

This series adds clocks for ICSSG for AM64x.

PATCH 1/2 Adds the dt binding necessary to add clocks to the device tree.
It adds the `clocks` in the dt binding of ICSSG node. Each ICSSG instance
has 7 clocks available to them as per AM64x TRM [1] Section 6.4.3 Table 
6-398. They are not added in the dt bindings yet. This patch adds all
available clocks to ICSSG bindings. 

PATCH 2/2 Adds the required clock to the ICSSG nodes. It also changes the
clock used from clock 20 (ICSSG_ICLK) to clock 0 (ICSSG_CORE). This patch
adds the clock-names, assigned-clocks and assigned-clock-parents to icssg
nodes.

More details on clocks can be found at [2]
There is no additional driver changes needed for this binding change.

Changes from v2 to v3:
*) Modified commit message of PATCH 1/2 to state why clocks are being added
to the binding.
*) Added all available clocks to ICSSG bindings. Earlier only two clocks
were added.
*) Added all available clocks to AM64x DTS. Earlier only two clocks were
added.

Changes from v1 to v2:
*) Dropped assigned-clocks and assigned-clock-parents from DT binding as
suggested by Krzysztof Kozlowski

[1] https://www.ti.com/lit/pdf/spruim2
[2] https://software-dl.ti.com/tisci/esd/latest/5_soc_doc/am64x/clocks.html#clocks-for-pru-icssg0-device
v1 https://lore.kernel.org/all/20241107104557.1442800-1-danishanwar@ti.com/
v2 https://lore.kernel.org/all/20241108142946.2286098-1-danishanwar@ti.com/

MD Danish Anwar (2):
  dt-bindings: soc: ti: pruss: Add clocks for ICSSG
  arm64: dts: ti: k3-am64-main: Switch ICSSG clock to core clock

 .../devicetree/bindings/soc/ti/ti,pruss.yaml  | 10 +++++++++
 arch/arm64/boot/dts/ti/k3-am64-main.dtsi      | 22 +++++++++++++++++--
 2 files changed, 30 insertions(+), 2 deletions(-)


base-commit: bd05b9a700c10473c2f52bf12c5c5938c30e80b0
-- 
2.34.1

[PATCH v3 1/2] dt-bindings: soc: ti: pruss: Add clocks for ICSSG

[MD Danish Anwar]

The ICSSG module has 7 clocks for each instance.

These clocks are ICSSG0_CORE_CLK, ICSSG0_IEP_CLK, ICSSG0_ICLK,
ICSSG0_UART_CLK, RGMII_MHZ_250_CLK, RGMII_MHZ_50_CLK and RGMII_MHZ_5_CLK
These clocks are described in AM64x TRM Section 6.4.3 Table 6-398.

Add these clocks to the dt binding of ICSSG.

Link: https://www.ti.com/lit/pdf/spruim2 (AM64x TRM)
Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
---
 Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml b/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
index 3cb1471cc6b6..927b3200e29e 100644
--- a/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
+++ b/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
@@ -92,6 +92,16 @@ properties:
     description: |
       This property is as per sci-pm-domain.txt.
 
+  clocks:
+    items:
+      - description: ICSSG_CORE Clock
+      - description: ICSSG_IEP Clock
+      - description: ICSSG_RGMII_MHZ_250 Clock
+      - description: ICSSG_RGMII_MHZ_50 Clock
+      - description: ICSSG_RGMII_MHZ_5 Clock
+      - description: ICSSG_UART Clock
+      - description: ICSSG_ICLK Clock
+
 patternProperties:
 
   memories@[a-f0-9]+$:
-- 
2.34.1

[PATCH v3 2/2] arm64: dts: ti: k3-am64-main: Switch ICSSG clock to core clock

[MD Danish Anwar]

ICSSG has 7 available clocks per instance. Add all the cloks to ICSSG
nodes. ICSSG currently uses ICSSG_ICLK (clk id 20) which operates at
250MHz. Switch ICSSG clock to ICSSG_CORE clock (clk id 0) which operates at
333MHz.

ICSSG_CORE clock will help get the most out of ICSSG as more cycles are
needed to fully support all ICSSG features.

This commit also changes assigned-clock-parents of coreclk-mux to
ICSSG_CORE clock from ICSSG_ICLK.

Performance update in dual mac mode
  With ICSSG_CORE Clk @ 333MHz
    Tx throughput - 934 Mbps
    Rx throughput - 914 Mbps,

  With ICSSG_ICLK clk @ 250MHz,
    Tx throughput - 920 Mbps
    Rx throughput - 706 Mbps

Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
---
 arch/arm64/boot/dts/ti/k3-am64-main.dtsi | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-am64-main.dtsi b/arch/arm64/boot/dts/ti/k3-am64-main.dtsi
index c66289a4362b..324eb44c258d 100644
--- a/arch/arm64/boot/dts/ti/k3-am64-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am64-main.dtsi
@@ -1227,6 +1227,15 @@ icssg0: icssg@30000000 {
 		#address-cells = <1>;
 		#size-cells = <1>;
 		ranges = <0x0 0x00 0x30000000 0x80000>;
+		clocks = <&k3_clks 81 0>,  /* icssg0_core_clk */
+			 <&k3_clks 81 3>,  /* icssg0_iep_clk */
+			 <&k3_clks 81 16>, /* icssg0_rgmii_mhz_250_clk */
+			 <&k3_clks 81 17>, /* icssg0_rgmii_mhz_50_clk */
+			 <&k3_clks 81 18>, /* icssg0_rgmii_mhz_5_clk */
+			 <&k3_clks 81 19>, /* icssg0_uart_clk */
+			 <&k3_clks 81 20>; /* icssg0_iclk */
+		assigned-clocks = <&k3_clks 81 0>;
+		assigned-clock-parents = <&k3_clks 81 2>;
 
 		icssg0_mem: memories@0 {
 			reg = <0x0 0x2000>,
@@ -1252,7 +1261,7 @@ icssg0_coreclk_mux: coreclk-mux@3c {
 					clocks = <&k3_clks 81 0>,  /* icssg0_core_clk */
 						 <&k3_clks 81 20>; /* icssg0_iclk */
 					assigned-clocks = <&icssg0_coreclk_mux>;
-					assigned-clock-parents = <&k3_clks 81 20>;
+					assigned-clock-parents = <&k3_clks 81 0>;
 				};
 
 				icssg0_iepclk_mux: iepclk-mux@30 {
@@ -1397,6 +1406,15 @@ icssg1: icssg@30080000 {
 		#address-cells = <1>;
 		#size-cells = <1>;
 		ranges = <0x0 0x00 0x30080000 0x80000>;
+		clocks = <&k3_clks 82 0>,  /* icssg1_core_clk */
+			 <&k3_clks 82 3>,  /* icssg1_iep_clk */
+			 <&k3_clks 82 16>, /* icssg1_rgmii_mhz_250_clk */
+			 <&k3_clks 82 17>, /* icssg1_rgmii_mhz_50_clk */
+			 <&k3_clks 82 18>, /* icssg1_rgmii_mhz_5_clk */
+			 <&k3_clks 82 19>, /* icssg1_uart_clk */
+			 <&k3_clks 82 20>; /* icssg1_iclk */
+		assigned-clocks = <&k3_clks 82 0>;
+		assigned-clock-parents = <&k3_clks 82 2>;
 
 		icssg1_mem: memories@0 {
 			reg = <0x0 0x2000>,
@@ -1422,7 +1440,7 @@ icssg1_coreclk_mux: coreclk-mux@3c {
 					clocks = <&k3_clks 82 0>,   /* icssg1_core_clk */
 						 <&k3_clks 82 20>;  /* icssg1_iclk */
 					assigned-clocks = <&icssg1_coreclk_mux>;
-					assigned-clock-parents = <&k3_clks 82 20>;
+					assigned-clock-parents = <&k3_clks 82 0>;
 				};
 
 				icssg1_iepclk_mux: iepclk-mux@30 {
-- 
2.34.1

Re: [PATCH v3 1/2] dt-bindings: soc: ti: pruss: Add clocks for ICSSG

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 556 bytes --]

On Wed, Nov 13, 2024 at 04:39:54PM +0530, MD Danish Anwar wrote:
> The ICSSG module has 7 clocks for each instance.
> 
> These clocks are ICSSG0_CORE_CLK, ICSSG0_IEP_CLK, ICSSG0_ICLK,
> ICSSG0_UART_CLK, RGMII_MHZ_250_CLK, RGMII_MHZ_50_CLK and RGMII_MHZ_5_CLK
> These clocks are described in AM64x TRM Section 6.4.3 Table 6-398.
> 
> Add these clocks to the dt binding of ICSSG.
> 
> Link: https://www.ti.com/lit/pdf/spruim2 (AM64x TRM)
> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v3 1/2] dt-bindings: soc: ti: pruss: Add clocks for ICSSG

[Roger Quadros]

Hi,

On 13/11/2024 13:09, MD Danish Anwar wrote:
> The ICSSG module has 7 clocks for each instance.
> 
> These clocks are ICSSG0_CORE_CLK, ICSSG0_IEP_CLK, ICSSG0_ICLK,
> ICSSG0_UART_CLK, RGMII_MHZ_250_CLK, RGMII_MHZ_50_CLK and RGMII_MHZ_5_CLK
> These clocks are described in AM64x TRM Section 6.4.3 Table 6-398.
> 
> Add these clocks to the dt binding of ICSSG.
> 
> Link: https://www.ti.com/lit/pdf/spruim2 (AM64x TRM)
> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
> ---
>  Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml b/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
> index 3cb1471cc6b6..927b3200e29e 100644
> --- a/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
> +++ b/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
> @@ -92,6 +92,16 @@ properties:
>      description: |
>        This property is as per sci-pm-domain.txt.
>  
> +  clocks:
> +    items:
> +      - description: ICSSG_CORE Clock
> +      - description: ICSSG_IEP Clock
> +      - description: ICSSG_RGMII_MHZ_250 Clock
> +      - description: ICSSG_RGMII_MHZ_50 Clock
> +      - description: ICSSG_RGMII_MHZ_5 Clock
> +      - description: ICSSG_UART Clock
> +      - description: ICSSG_ICLK Clock
> +

There are actually many more clocks [1]
What is the purpose of adding all these clocks in the DT if driver doesn't
use them?

Only CORE and IEP clocks parent can be configured via clock muxes.
Those are already defined in the icssg?_cfg nodes.

[1] - https://software-dl.ti.com/tisci/esd/22_01_02/5_soc_doc/am64x/clocks.html

>  patternProperties:
>  
>    memories@[a-f0-9]+$:

-- 
cheers,
-roger

Re: [PATCH v3 1/2] dt-bindings: soc: ti: pruss: Add clocks for ICSSG

[Roger Quadros]

On 18/11/2024 15:33, Roger Quadros wrote:
> Hi,
> 
> On 13/11/2024 13:09, MD Danish Anwar wrote:
>> The ICSSG module has 7 clocks for each instance.
>>
>> These clocks are ICSSG0_CORE_CLK, ICSSG0_IEP_CLK, ICSSG0_ICLK,
>> ICSSG0_UART_CLK, RGMII_MHZ_250_CLK, RGMII_MHZ_50_CLK and RGMII_MHZ_5_CLK
>> These clocks are described in AM64x TRM Section 6.4.3 Table 6-398.
>>
>> Add these clocks to the dt binding of ICSSG.
>>
>> Link: https://www.ti.com/lit/pdf/spruim2 (AM64x TRM)
>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>> ---
>>  Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml b/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
>> index 3cb1471cc6b6..927b3200e29e 100644
>> --- a/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
>> +++ b/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
>> @@ -92,6 +92,16 @@ properties:
>>      description: |
>>        This property is as per sci-pm-domain.txt.
>>  
>> +  clocks:
>> +    items:
>> +      - description: ICSSG_CORE Clock
>> +      - description: ICSSG_IEP Clock
>> +      - description: ICSSG_RGMII_MHZ_250 Clock
>> +      - description: ICSSG_RGMII_MHZ_50 Clock
>> +      - description: ICSSG_RGMII_MHZ_5 Clock
>> +      - description: ICSSG_UART Clock
>> +      - description: ICSSG_ICLK Clock
>> +
> 
> There are actually many more clocks [1]
> What is the purpose of adding all these clocks in the DT if driver doesn't
> use them?
> 
> Only CORE and IEP clocks parent can be configured via clock muxes.
> Those are already defined in the icssg?_cfg nodes.

Actually those clock muxes are internal to ICSSG.
We still need to be able to set clock parents of CORE and IEP clock.

So pruss block needs at most 2 clocks like you had in v2 of this patch?

> 
> [1] - https://software-dl.ti.com/tisci/esd/22_01_02/5_soc_doc/am64x/clocks.html
> 
>>  patternProperties:
>>  
>>    memories@[a-f0-9]+$:
> 

-- 
cheers,
-roger

Re: [PATCH v3 1/2] dt-bindings: soc: ti: pruss: Add clocks for ICSSG

[Vignesh Raghavendra]

On 18/11/24 19:22, Roger Quadros wrote:
> 
> 
> On 18/11/2024 15:33, Roger Quadros wrote:
>> Hi,
>>
>> On 13/11/2024 13:09, MD Danish Anwar wrote:
>>> The ICSSG module has 7 clocks for each instance.
>>>
>>> These clocks are ICSSG0_CORE_CLK, ICSSG0_IEP_CLK, ICSSG0_ICLK,
>>> ICSSG0_UART_CLK, RGMII_MHZ_250_CLK, RGMII_MHZ_50_CLK and RGMII_MHZ_5_CLK
>>> These clocks are described in AM64x TRM Section 6.4.3 Table 6-398.
>>>
>>> Add these clocks to the dt binding of ICSSG.
>>>
>>> Link: https://www.ti.com/lit/pdf/spruim2 (AM64x TRM)
>>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>>> ---
>>>  Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml | 10 ++++++++++
>>>  1 file changed, 10 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml b/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
>>> index 3cb1471cc6b6..927b3200e29e 100644
>>> --- a/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
>>> +++ b/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
>>> @@ -92,6 +92,16 @@ properties:
>>>      description: |
>>>        This property is as per sci-pm-domain.txt.
>>>  
>>> +  clocks:
>>> +    items:
>>> +      - description: ICSSG_CORE Clock
>>> +      - description: ICSSG_IEP Clock
>>> +      - description: ICSSG_RGMII_MHZ_250 Clock
>>> +      - description: ICSSG_RGMII_MHZ_50 Clock
>>> +      - description: ICSSG_RGMII_MHZ_5 Clock
>>> +      - description: ICSSG_UART Clock
>>> +      - description: ICSSG_ICLK Clock
>>> +
>>
>> There are actually many more clocks [1]
>> What is the purpose of adding all these clocks in the DT if driver doesn't
>> use them?
>>

DT should completely describe the HW and not based on what Linux driver
needs. So its valid to describe all clock inputs to a module
irrespective of what driver does with it.

>> Only CORE and IEP clocks parent can be configured via clock muxes.
>> Those are already defined in the icssg?_cfg nodes.
> 
> Actually those clock muxes are internal to ICSSG.
> We still need to be able to set clock parents of CORE and IEP clock.
> 
> So pruss block needs at most 2 clocks like you had in v2 of this patch?
> 
>>
>> [1] - https://software-dl.ti.com/tisci/esd/22_01_02/5_soc_doc/am64x/clocks.html
>>
>>>  patternProperties:
>>>  
>>>    memories@[a-f0-9]+$:
>>
> 

-- 
Regards
Vignesh
https://ti.com/opensource

Re: [PATCH v3 1/2] dt-bindings: soc: ti: pruss: Add clocks for ICSSG

[Roger Quadros]

On 19/11/2024 08:12, Vignesh Raghavendra wrote:
> 
> 
> On 18/11/24 19:22, Roger Quadros wrote:
>>
>>
>> On 18/11/2024 15:33, Roger Quadros wrote:
>>> Hi,
>>>
>>> On 13/11/2024 13:09, MD Danish Anwar wrote:
>>>> The ICSSG module has 7 clocks for each instance.
>>>>
>>>> These clocks are ICSSG0_CORE_CLK, ICSSG0_IEP_CLK, ICSSG0_ICLK,
>>>> ICSSG0_UART_CLK, RGMII_MHZ_250_CLK, RGMII_MHZ_50_CLK and RGMII_MHZ_5_CLK
>>>> These clocks are described in AM64x TRM Section 6.4.3 Table 6-398.
>>>>
>>>> Add these clocks to the dt binding of ICSSG.
>>>>
>>>> Link: https://www.ti.com/lit/pdf/spruim2 (AM64x TRM)
>>>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>>>> ---
>>>>  Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml | 10 ++++++++++
>>>>  1 file changed, 10 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml b/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
>>>> index 3cb1471cc6b6..927b3200e29e 100644
>>>> --- a/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
>>>> +++ b/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
>>>> @@ -92,6 +92,16 @@ properties:
>>>>      description: |
>>>>        This property is as per sci-pm-domain.txt.
>>>>  
>>>> +  clocks:
>>>> +    items:
>>>> +      - description: ICSSG_CORE Clock
>>>> +      - description: ICSSG_IEP Clock
>>>> +      - description: ICSSG_RGMII_MHZ_250 Clock
>>>> +      - description: ICSSG_RGMII_MHZ_50 Clock
>>>> +      - description: ICSSG_RGMII_MHZ_5 Clock
>>>> +      - description: ICSSG_UART Clock
>>>> +      - description: ICSSG_ICLK Clock
>>>> +
>>>
>>> There are actually many more clocks [1]
>>> What is the purpose of adding all these clocks in the DT if driver doesn't
>>> use them?
>>>
> 
> DT should completely describe the HW and not based on what Linux driver
> needs. So its valid to describe all clock inputs to a module
> irrespective of what driver does with it.

Fair point. But there are a total 11 clocks instead of 7 in [1]

> 
>>> Only CORE and IEP clocks parent can be configured via clock muxes.
>>> Those are already defined in the icssg?_cfg nodes.
>>
>> Actually those clock muxes are internal to ICSSG.
>> We still need to be able to set clock parents of CORE and IEP clock.
>>
>> So pruss block needs at most 2 clocks like you had in v2 of this patch?
>>
>>>
>>> [1] - https://software-dl.ti.com/tisci/esd/22_01_02/5_soc_doc/am64x/clocks.html
>>>
>>>>  patternProperties:
>>>>  
>>>>    memories@[a-f0-9]+$:
>>>
>>
> 

-- 
cheers,
-roger

Re: [PATCH v3 2/2] arm64: dts: ti: k3-am64-main: Switch ICSSG clock to core clock

[Roger Quadros]

On 13/11/2024 13:09, MD Danish Anwar wrote:
> ICSSG has 7 available clocks per instance. Add all the cloks to ICSSG
> nodes. ICSSG currently uses ICSSG_ICLK (clk id 20) which operates at
> 250MHz. Switch ICSSG clock to ICSSG_CORE clock (clk id 0) which operates at
> 333MHz.
> 
> ICSSG_CORE clock will help get the most out of ICSSG as more cycles are
> needed to fully support all ICSSG features.
> 
> This commit also changes assigned-clock-parents of coreclk-mux to
> ICSSG_CORE clock from ICSSG_ICLK.
> 
> Performance update in dual mac mode
>   With ICSSG_CORE Clk @ 333MHz
>     Tx throughput - 934 Mbps
>     Rx throughput - 914 Mbps,
> 
>   With ICSSG_ICLK clk @ 250MHz,
>     Tx throughput - 920 Mbps
>     Rx throughput - 706 Mbps
> 
> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
> ---
>  arch/arm64/boot/dts/ti/k3-am64-main.dtsi | 22 ++++++++++++++++++++--

What about other platforms that have ICSSG?
e.g. k3-am65-main.dtsi and k3-j721e-main.dtsi

>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-am64-main.dtsi b/arch/arm64/boot/dts/ti/k3-am64-main.dtsi
> index c66289a4362b..324eb44c258d 100644
> --- a/arch/arm64/boot/dts/ti/k3-am64-main.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am64-main.dtsi
> @@ -1227,6 +1227,15 @@ icssg0: icssg@30000000 {
>  		#address-cells = <1>;
>  		#size-cells = <1>;
>  		ranges = <0x0 0x00 0x30000000 0x80000>;
> +		clocks = <&k3_clks 81 0>,  /* icssg0_core_clk */
> +			 <&k3_clks 81 3>,  /* icssg0_iep_clk */
> +			 <&k3_clks 81 16>, /* icssg0_rgmii_mhz_250_clk */
> +			 <&k3_clks 81 17>, /* icssg0_rgmii_mhz_50_clk */
> +			 <&k3_clks 81 18>, /* icssg0_rgmii_mhz_5_clk */
> +			 <&k3_clks 81 19>, /* icssg0_uart_clk */
> +			 <&k3_clks 81 20>; /* icssg0_iclk */
> +		assigned-clocks = <&k3_clks 81 0>;
> +		assigned-clock-parents = <&k3_clks 81 2>;
>  
>  		icssg0_mem: memories@0 {
>  			reg = <0x0 0x2000>,
> @@ -1252,7 +1261,7 @@ icssg0_coreclk_mux: coreclk-mux@3c {
>  					clocks = <&k3_clks 81 0>,  /* icssg0_core_clk */
>  						 <&k3_clks 81 20>; /* icssg0_iclk */
>  					assigned-clocks = <&icssg0_coreclk_mux>;
> -					assigned-clock-parents = <&k3_clks 81 20>;
> +					assigned-clock-parents = <&k3_clks 81 0>;
>  				};
>  
>  				icssg0_iepclk_mux: iepclk-mux@30 {
> @@ -1397,6 +1406,15 @@ icssg1: icssg@30080000 {
>  		#address-cells = <1>;
>  		#size-cells = <1>;
>  		ranges = <0x0 0x00 0x30080000 0x80000>;
> +		clocks = <&k3_clks 82 0>,  /* icssg1_core_clk */
> +			 <&k3_clks 82 3>,  /* icssg1_iep_clk */
> +			 <&k3_clks 82 16>, /* icssg1_rgmii_mhz_250_clk */
> +			 <&k3_clks 82 17>, /* icssg1_rgmii_mhz_50_clk */
> +			 <&k3_clks 82 18>, /* icssg1_rgmii_mhz_5_clk */
> +			 <&k3_clks 82 19>, /* icssg1_uart_clk */
> +			 <&k3_clks 82 20>; /* icssg1_iclk */
> +		assigned-clocks = <&k3_clks 82 0>;
> +		assigned-clock-parents = <&k3_clks 82 2>;
>  
>  		icssg1_mem: memories@0 {
>  			reg = <0x0 0x2000>,
> @@ -1422,7 +1440,7 @@ icssg1_coreclk_mux: coreclk-mux@3c {
>  					clocks = <&k3_clks 82 0>,   /* icssg1_core_clk */
>  						 <&k3_clks 82 20>;  /* icssg1_iclk */
>  					assigned-clocks = <&icssg1_coreclk_mux>;
> -					assigned-clock-parents = <&k3_clks 82 20>;
> +					assigned-clock-parents = <&k3_clks 82 0>;
>  				};
>  
>  				icssg1_iepclk_mux: iepclk-mux@30 {

-- 
cheers,
-roger

Re: [PATCH v3 2/2] arm64: dts: ti: k3-am64-main: Switch ICSSG clock to core clock

[Wadim Egorov]

Am 13.11.24 um 12:09 schrieb MD Danish Anwar:
> ICSSG has 7 available clocks per instance. Add all the cloks to ICSSG
> nodes. ICSSG currently uses ICSSG_ICLK (clk id 20) which operates at
> 250MHz. Switch ICSSG clock to ICSSG_CORE clock (clk id 0) which operates at
> 333MHz.
> 
> ICSSG_CORE clock will help get the most out of ICSSG as more cycles are
> needed to fully support all ICSSG features.
> 
> This commit also changes assigned-clock-parents of coreclk-mux to
> ICSSG_CORE clock from ICSSG_ICLK.
> 
> Performance update in dual mac mode
>    With ICSSG_CORE Clk @ 333MHz
>      Tx throughput - 934 Mbps
>      Rx throughput - 914 Mbps,
> 
>    With ICSSG_ICLK clk @ 250MHz,
>      Tx throughput - 920 Mbps
>      Rx throughput - 706 Mbps

I can see similar improvements. Thank you.

> 
> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>

Tested on a phyBOARD-Electra-AM64x board,

Tested-by: Wadim Egorov <w.egorov@phytec.de>

Re: [PATCH v3 1/2] dt-bindings: soc: ti: pruss: Add clocks for ICSSG

[Anwar, Md Danish]

Hi Roger,

On 11/19/2024 4:15 PM, Roger Quadros wrote:
> 
> 
> On 19/11/2024 08:12, Vignesh Raghavendra wrote:
>>
>>
>> On 18/11/24 19:22, Roger Quadros wrote:
>>>
>>>
>>> On 18/11/2024 15:33, Roger Quadros wrote:
>>>> Hi,
>>>>
>>>> On 13/11/2024 13:09, MD Danish Anwar wrote:
>>>>> The ICSSG module has 7 clocks for each instance.
>>>>>
>>>>> These clocks are ICSSG0_CORE_CLK, ICSSG0_IEP_CLK, ICSSG0_ICLK,
>>>>> ICSSG0_UART_CLK, RGMII_MHZ_250_CLK, RGMII_MHZ_50_CLK and RGMII_MHZ_5_CLK
>>>>> These clocks are described in AM64x TRM Section 6.4.3 Table 6-398.
>>>>>
>>>>> Add these clocks to the dt binding of ICSSG.
>>>>>
>>>>> Link: https://www.ti.com/lit/pdf/spruim2 (AM64x TRM)
>>>>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>>>>> ---
>>>>>  Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml | 10 ++++++++++
>>>>>  1 file changed, 10 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml b/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
>>>>> index 3cb1471cc6b6..927b3200e29e 100644
>>>>> --- a/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
>>>>> +++ b/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
>>>>> @@ -92,6 +92,16 @@ properties:
>>>>>      description: |
>>>>>        This property is as per sci-pm-domain.txt.
>>>>>  
>>>>> +  clocks:
>>>>> +    items:
>>>>> +      - description: ICSSG_CORE Clock
>>>>> +      - description: ICSSG_IEP Clock
>>>>> +      - description: ICSSG_RGMII_MHZ_250 Clock
>>>>> +      - description: ICSSG_RGMII_MHZ_50 Clock
>>>>> +      - description: ICSSG_RGMII_MHZ_5 Clock
>>>>> +      - description: ICSSG_UART Clock
>>>>> +      - description: ICSSG_ICLK Clock
>>>>> +
>>>>
>>>> There are actually many more clocks [1]
>>>> What is the purpose of adding all these clocks in the DT if driver doesn't
>>>> use them?
>>>>
>>
>> DT should completely describe the HW and not based on what Linux driver
>> needs. So its valid to describe all clock inputs to a module
>> irrespective of what driver does with it.
> 
> Fair point. But there are a total 11 clocks instead of 7 in [1]
> 

I took the list of clocks from AM64x TRM [1] Section 6.4.3 Table 6-398.
In the TRM only 7 clocks are mentioned per ICSSG instance which I have
mentioned in the binding.

[1] https://www.ti.com/lit/ug/spruim2h/spruim2h.pdf?ts=1732108738816

>>
>>>> Only CORE and IEP clocks parent can be configured via clock muxes.
>>>> Those are already defined in the icssg?_cfg nodes.
>>>
>>> Actually those clock muxes are internal to ICSSG.
>>> We still need to be able to set clock parents of CORE and IEP clock.
>>>
>>> So pruss block needs at most 2 clocks like you had in v2 of this patch?
>>>
>>>>
>>>> [1] - https://software-dl.ti.com/tisci/esd/22_01_02/5_soc_doc/am64x/clocks.html
>>>>
>>>>>  patternProperties:
>>>>>  
>>>>>    memories@[a-f0-9]+$:
>>>>
>>>
>>
> 

-- 
Thanks and Regards,
Md Danish Anwar

Re: [PATCH v3 1/2] dt-bindings: soc: ti: pruss: Add clocks for ICSSG

[Roger Quadros]

On 20/11/2024 15:23, Anwar, Md Danish wrote:
> Hi Roger,
> 
> On 11/19/2024 4:15 PM, Roger Quadros wrote:
>>
>>
>> On 19/11/2024 08:12, Vignesh Raghavendra wrote:
>>>
>>>
>>> On 18/11/24 19:22, Roger Quadros wrote:
>>>>
>>>>
>>>> On 18/11/2024 15:33, Roger Quadros wrote:
>>>>> Hi,
>>>>>
>>>>> On 13/11/2024 13:09, MD Danish Anwar wrote:
>>>>>> The ICSSG module has 7 clocks for each instance.
>>>>>>
>>>>>> These clocks are ICSSG0_CORE_CLK, ICSSG0_IEP_CLK, ICSSG0_ICLK,
>>>>>> ICSSG0_UART_CLK, RGMII_MHZ_250_CLK, RGMII_MHZ_50_CLK and RGMII_MHZ_5_CLK
>>>>>> These clocks are described in AM64x TRM Section 6.4.3 Table 6-398.
>>>>>>
>>>>>> Add these clocks to the dt binding of ICSSG.
>>>>>>
>>>>>> Link: https://www.ti.com/lit/pdf/spruim2 (AM64x TRM)
>>>>>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>>>>>> ---
>>>>>>  Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml | 10 ++++++++++
>>>>>>  1 file changed, 10 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml b/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
>>>>>> index 3cb1471cc6b6..927b3200e29e 100644
>>>>>> --- a/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
>>>>>> +++ b/Documentation/devicetree/bindings/soc/ti/ti,pruss.yaml
>>>>>> @@ -92,6 +92,16 @@ properties:
>>>>>>      description: |
>>>>>>        This property is as per sci-pm-domain.txt.
>>>>>>  
>>>>>> +  clocks:
>>>>>> +    items:
>>>>>> +      - description: ICSSG_CORE Clock
>>>>>> +      - description: ICSSG_IEP Clock
>>>>>> +      - description: ICSSG_RGMII_MHZ_250 Clock
>>>>>> +      - description: ICSSG_RGMII_MHZ_50 Clock
>>>>>> +      - description: ICSSG_RGMII_MHZ_5 Clock
>>>>>> +      - description: ICSSG_UART Clock
>>>>>> +      - description: ICSSG_ICLK Clock
>>>>>> +
>>>>>
>>>>> There are actually many more clocks [1]
>>>>> What is the purpose of adding all these clocks in the DT if driver doesn't
>>>>> use them?
>>>>>
>>>
>>> DT should completely describe the HW and not based on what Linux driver
>>> needs. So its valid to describe all clock inputs to a module
>>> irrespective of what driver does with it.
>>
>> Fair point. But there are a total 11 clocks instead of 7 in [1]
>>
> 
> I took the list of clocks from AM64x TRM [1] Section 6.4.3 Table 6-398.
> In the TRM only 7 clocks are mentioned per ICSSG instance which I have
> mentioned in the binding.
> 
> [1] https://www.ti.com/lit/ug/spruim2h/spruim2h.pdf?ts=1732108738816

OK thanks for the clarification. It looks like the same in AM65 and J721e TRMs as well.
So it is fine.

> 
>>>
>>>>> Only CORE and IEP clocks parent can be configured via clock muxes.
>>>>> Those are already defined in the icssg?_cfg nodes.
>>>>
>>>> Actually those clock muxes are internal to ICSSG.
>>>> We still need to be able to set clock parents of CORE and IEP clock.
>>>>
>>>> So pruss block needs at most 2 clocks like you had in v2 of this patch?
>>>>
>>>>>
>>>>> [1] - https://software-dl.ti.com/tisci/esd/22_01_02/5_soc_doc/am64x/clocks.html
>>>>>
>>>>>>  patternProperties:
>>>>>>  
>>>>>>    memories@[a-f0-9]+$:
>>>>>
>>>>
>>>
>>
> 

-- 
cheers,
-roger

Re: [PATCH v3 1/2] dt-bindings: soc: ti: pruss: Add clocks for ICSSG

[Roger Quadros]

On 13/11/2024 13:09, MD Danish Anwar wrote:
> The ICSSG module has 7 clocks for each instance.
> 
> These clocks are ICSSG0_CORE_CLK, ICSSG0_IEP_CLK, ICSSG0_ICLK,
> ICSSG0_UART_CLK, RGMII_MHZ_250_CLK, RGMII_MHZ_50_CLK and RGMII_MHZ_5_CLK
> These clocks are described in AM64x TRM Section 6.4.3 Table 6-398.
> 
> Add these clocks to the dt binding of ICSSG.
> 
> Link: https://www.ti.com/lit/pdf/spruim2 (AM64x TRM)
> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>

Reviewed-by: Roger Quadros <rogerq@kernel.org>

Re: [PATCH v3 2/2] arm64: dts: ti: k3-am64-main: Switch ICSSG clock to core clock

[Roger Quadros]

On 13/11/2024 13:09, MD Danish Anwar wrote:
> ICSSG has 7 available clocks per instance. Add all the cloks to ICSSG
> nodes. ICSSG currently uses ICSSG_ICLK (clk id 20) which operates at
> 250MHz. Switch ICSSG clock to ICSSG_CORE clock (clk id 0) which operates at
> 333MHz.
> 
> ICSSG_CORE clock will help get the most out of ICSSG as more cycles are
> needed to fully support all ICSSG features.
> 
> This commit also changes assigned-clock-parents of coreclk-mux to
> ICSSG_CORE clock from ICSSG_ICLK.
> 
> Performance update in dual mac mode
>   With ICSSG_CORE Clk @ 333MHz
>     Tx throughput - 934 Mbps
>     Rx throughput - 914 Mbps,
> 
>   With ICSSG_ICLK clk @ 250MHz,
>     Tx throughput - 920 Mbps
>     Rx throughput - 706 Mbps
> 
> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>

It would be nice if you could send patches to update
corresponding nodes in am65 and j721e platforms as well
else we will start getting dtbs_check errors.

Reviewed-by: Roger Quadros <rogerq@kernel.org>

Re: [PATCH v3 0/2] Add Clocks for ICSSG

[MD Danish Anwar]

Hi Vignesh / Nishant

On 13/11/24 4:39 pm, MD Danish Anwar wrote:
> This series adds clocks for ICSSG for AM64x.
> 
> PATCH 1/2 Adds the dt binding necessary to add clocks to the device tree.
> It adds the `clocks` in the dt binding of ICSSG node. Each ICSSG instance
> has 7 clocks available to them as per AM64x TRM [1] Section 6.4.3 Table 
> 6-398. They are not added in the dt bindings yet. This patch adds all
> available clocks to ICSSG bindings. 
> 
> PATCH 2/2 Adds the required clock to the ICSSG nodes. It also changes the
> clock used from clock 20 (ICSSG_ICLK) to clock 0 (ICSSG_CORE). This patch
> adds the clock-names, assigned-clocks and assigned-clock-parents to icssg
> nodes.
> 
> More details on clocks can be found at [2]
> There is no additional driver changes needed for this binding change.
> 
> Changes from v2 to v3:
> *) Modified commit message of PATCH 1/2 to state why clocks are being added
> to the binding.
> *) Added all available clocks to ICSSG bindings. Earlier only two clocks
> were added.
> *) Added all available clocks to AM64x DTS. Earlier only two clocks were
> added.
> 
> Changes from v1 to v2:
> *) Dropped assigned-clocks and assigned-clock-parents from DT binding as
> suggested by Krzysztof Kozlowski
> 
> [1] https://www.ti.com/lit/pdf/spruim2
> [2] https://software-dl.ti.com/tisci/esd/latest/5_soc_doc/am64x/clocks.html#clocks-for-pru-icssg0-device
> v1 https://lore.kernel.org/all/20241107104557.1442800-1-danishanwar@ti.com/
> v2 https://lore.kernel.org/all/20241108142946.2286098-1-danishanwar@ti.com/
> 
> MD Danish Anwar (2):
>   dt-bindings: soc: ti: pruss: Add clocks for ICSSG
>   arm64: dts: ti: k3-am64-main: Switch ICSSG clock to core clock
> 
>  .../devicetree/bindings/soc/ti/ti,pruss.yaml  | 10 +++++++++
>  arch/arm64/boot/dts/ti/k3-am64-main.dtsi      | 22 +++++++++++++++++--
>  2 files changed, 30 insertions(+), 2 deletions(-)
> 
> 
> base-commit: bd05b9a700c10473c2f52bf12c5c5938c30e80b0

This series still applies cleanly on latest linux-next (next-20241216)
and doesn't need a re-base.

Can you please pick this.


-- 
Thanks and Regards,
Danish

Re: [PATCH v3 0/2] Add Clocks for ICSSG

[Nishanth Menon]

Hi MD Danish Anwar,

On Wed, 13 Nov 2024 16:39:53 +0530, MD Danish Anwar wrote:
> This series adds clocks for ICSSG for AM64x.
> 
> PATCH 1/2 Adds the dt binding necessary to add clocks to the device tree.
> It adds the `clocks` in the dt binding of ICSSG node. Each ICSSG instance
> has 7 clocks available to them as per AM64x TRM [1] Section 6.4.3 Table
> 6-398. They are not added in the dt bindings yet. This patch adds all
> available clocks to ICSSG bindings.
> 
> [...]

I have applied the following to branch ti-k3-dts-next on [1].
Thank you!

[1/2] dt-bindings: soc: ti: pruss: Add clocks for ICSSG
      commit: f7ed5ae30cf395d92a3e1e3c843fa86ce96167b8
[2/2] arm64: dts: ti: k3-am64-main: Switch ICSSG clock to core clock
      commit: 25aadf5039fe8920835fb1452db08afa27a0edd9

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent up the chain during
the next merge window (or sooner if it is a relevant bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/ti/linux.git
-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D