[PATCH v4 0/2] Add interconnect support for stmmac driver.

[PATCH v4 0/2] Add interconnect support for stmmac driver.

[Sagar Cheluvegowda]

Interconnect is a software framework to access NOC bus topology
of the system, this framework is designed to provide a standard
kernel interface to control the settings of the interconnects on
an SoC.
The interconnect support is now being added to the stmmac driver
so that any vendors who wants to use this feature can just
define corresponging dtsi properties according to their
NOC bus topologies.

here is a patch series which is enabling interconnect support
for ethernet node of SA8775P
https://lore.kernel.org/all/20240708-icc_bw_voting_emac_dtsi-v1-1-4b091b3150c0@quicinc.com/ 

Signed-off-by: Sagar Cheluvegowda <quic_scheluve@quicinc.com>
---
Changes in v4:
- Add reference to the series which is enabling interconnect-properties defined in this series
- Link to v3: https://lore.kernel.org/r/20240703-icc_bw_voting_from_ethqos-v3-0-8f9148ac60a3@quicinc.com

Changes in v3:
- Drop the patch:
	[PATCH v2 3/3] net: stmmac: Bring down the clocks to lower frequencies when mac link goes down.
- Modify the dt-bindings property names from "axi" and "ahb" to "mac-mem" and
  "cpu-mac" respectively.
- Link to v2: https://lore.kernel.org/r/20240625-icc_bw_voting_from_ethqos-v2-0-eaa7cf9060f0@quicinc.com

Changes in v2:
- Edit the cover letter to give a big picture of this change.
- Move the interconnect changes from ethqos driver to stmmac driver.
- Reorder the the patches to place bindings patch on the top.
- Remove "_icc_path" redundant string from the "interconnect-names" property.
- Link to v1: https://lore.kernel.org/r/20240619-icc_bw_voting_from_ethqos-v1-0-6112948b825e@quicinc.com

---
Sagar Cheluvegowda (2):
      dt-bindings: net: qcom: ethernet: Add interconnect properties
      net: stmmac: Add interconnect support

 Documentation/devicetree/bindings/net/qcom,ethqos.yaml |  8 ++++++++
 drivers/net/ethernet/stmicro/stmmac/stmmac.h           |  1 +
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c      |  8 ++++++++
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c  | 12 ++++++++++++
 include/linux/stmmac.h                                 |  2 ++
 5 files changed, 31 insertions(+)
---
base-commit: 8a92980606e3585d72d510a03b59906e96755b8a
change-id: 20240610-icc_bw_voting_from_ethqos-12f5c6ed46c2

Best regards,
-- 
Sagar Cheluvegowda <quic_scheluve@quicinc.com>

[PATCH v4 1/2] dt-bindings: net: qcom: ethernet: Add interconnect properties

[Sagar Cheluvegowda]

Add documentation for the interconnect and interconnect-names
properties required when voting for AHB and AXI buses.

Suggested-by: Andrew Halaney <ahalaney@redhat.com>
Signed-off-by: Sagar Cheluvegowda <quic_scheluve@quicinc.com>
---
 Documentation/devicetree/bindings/net/qcom,ethqos.yaml | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/qcom,ethqos.yaml b/Documentation/devicetree/bindings/net/qcom,ethqos.yaml
index 6672327358bc..f0e8eaf51137 100644
--- a/Documentation/devicetree/bindings/net/qcom,ethqos.yaml
+++ b/Documentation/devicetree/bindings/net/qcom,ethqos.yaml
@@ -63,6 +63,14 @@ properties:
 
   dma-coherent: true
 
+  interconnects:
+    maxItems: 2
+
+  interconnect-names:
+    items:
+      - const: cpu-mac
+      - const: mac-mem
+
   phys: true
 
   phy-names:

-- 
2.34.1

[PATCH v4 2/2] net: stmmac: Add interconnect support

[Sagar Cheluvegowda]

Add interconnect support to vote for bus bandwidth based
on the current speed of the driver.
Adds support for two different paths - one from ethernet to
DDR and the other from CPU to ethernet, Vote from each
interconnect client is aggregated and the on-chip interconnect
hardware is configured to the most appropriate bandwidth profile.

Suggested-by: Andrew Halaney <ahalaney@redhat.com>
Signed-off-by: Sagar Cheluvegowda <quic_scheluve@quicinc.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac.h          |  1 +
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c     |  8 ++++++++
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 12 ++++++++++++
 include/linux/stmmac.h                                |  2 ++
 4 files changed, 23 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index b23b920eedb1..56a282d2b8cd 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -21,6 +21,7 @@
 #include <linux/ptp_clock_kernel.h>
 #include <linux/net_tstamp.h>
 #include <linux/reset.h>
+#include <linux/interconnect.h>
 #include <net/page_pool/types.h>
 #include <net/xdp.h>
 #include <uapi/linux/bpf.h>
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index b3afc7cb7d72..ec7c61ee44d4 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -985,6 +985,12 @@ static void stmmac_fpe_link_state_handle(struct stmmac_priv *priv, bool is_up)
 	}
 }
 
+static void stmmac_set_icc_bw(struct stmmac_priv *priv, unsigned int speed)
+{
+	icc_set_bw(priv->plat->axi_icc_path, Mbps_to_icc(speed), Mbps_to_icc(speed));
+	icc_set_bw(priv->plat->ahb_icc_path, Mbps_to_icc(speed), Mbps_to_icc(speed));
+}
+
 static void stmmac_mac_link_down(struct phylink_config *config,
 				 unsigned int mode, phy_interface_t interface)
 {
@@ -1080,6 +1086,8 @@ static void stmmac_mac_link_up(struct phylink_config *config,
 	if (priv->plat->fix_mac_speed)
 		priv->plat->fix_mac_speed(priv->plat->bsp_priv, speed, mode);
 
+	stmmac_set_icc_bw(priv, speed);
+
 	if (!duplex)
 		ctrl &= ~priv->hw->link.duplex;
 	else
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 54797edc9b38..201f9dea6da9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -642,6 +642,18 @@ stmmac_probe_config_dt(struct platform_device *pdev, u8 *mac)
 		dev_dbg(&pdev->dev, "PTP rate %d\n", plat->clk_ptp_rate);
 	}
 
+	plat->axi_icc_path = devm_of_icc_get(&pdev->dev, "mac-mem");
+	if (IS_ERR(plat->axi_icc_path)) {
+		ret = ERR_CAST(plat->axi_icc_path);
+		goto error_hw_init;
+	}
+
+	plat->ahb_icc_path = devm_of_icc_get(&pdev->dev, "cpu-mac");
+	if (IS_ERR(plat->ahb_icc_path)) {
+		ret = ERR_CAST(plat->ahb_icc_path);
+		goto error_hw_init;
+	}
+
 	plat->stmmac_rst = devm_reset_control_get_optional(&pdev->dev,
 							   STMMAC_RESOURCE_NAME);
 	if (IS_ERR(plat->stmmac_rst)) {
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index f92c195c76ed..385f352a0c23 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -283,6 +283,8 @@ struct plat_stmmacenet_data {
 	struct reset_control *stmmac_rst;
 	struct reset_control *stmmac_ahb_rst;
 	struct stmmac_axi *axi;
+	struct icc_path *axi_icc_path;
+	struct icc_path *ahb_icc_path;
 	int has_gmac4;
 	int rss_en;
 	int mac_port_sel_speed;

-- 
2.34.1

Re: [PATCH v4 1/2] dt-bindings: net: qcom: ethernet: Add interconnect properties

[Andrew Lunn]

On Mon, Jul 08, 2024 at 02:30:00PM -0700, Sagar Cheluvegowda wrote:
> Add documentation for the interconnect and interconnect-names
> properties required when voting for AHB and AXI buses.
> 
> Suggested-by: Andrew Halaney <ahalaney@redhat.com>
> Signed-off-by: Sagar Cheluvegowda <quic_scheluve@quicinc.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH v4 2/2] net: stmmac: Add interconnect support

[Andrew Lunn]

On Mon, Jul 08, 2024 at 02:30:01PM -0700, Sagar Cheluvegowda wrote:
> Add interconnect support to vote for bus bandwidth based
> on the current speed of the driver.
> Adds support for two different paths - one from ethernet to
> DDR and the other from CPU to ethernet, Vote from each
> interconnect client is aggregated and the on-chip interconnect
> hardware is configured to the most appropriate bandwidth profile.
> 
> Suggested-by: Andrew Halaney <ahalaney@redhat.com>
> Signed-off-by: Sagar Cheluvegowda <quic_scheluve@quicinc.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH v4 1/2] dt-bindings: net: qcom: ethernet: Add interconnect properties

[Krzysztof Kozlowski]

On 08/07/2024 23:30, Sagar Cheluvegowda wrote:
> Add documentation for the interconnect and interconnect-names
> properties required when voting for AHB and AXI buses.
> 
> Suggested-by: Andrew Halaney <ahalaney@redhat.com>
> Signed-off-by: Sagar Cheluvegowda <quic_scheluve@quicinc.com>

<form letter>
This is a friendly reminder during the review process.

It looks like you received a tag and forgot to add it.

If you do not know the process, here is a short explanation:
Please add Acked-by/Reviewed-by/Tested-by tags when posting new
versions, under or above your Signed-off-by tag. Tag is "received", when
provided in a message replied to you on the mailing list. Tools like b4
can help here. However, there's no need to repost patches *only* to add
the tags. The upstream maintainer will do that for tags received on the
version they apply.

https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577

If a tag was not added on purpose, please state why and what changed.
</form letter>

Best regards,
Krzysztof

Re: [PATCH v4 0/2] Add interconnect support for stmmac driver.

[Krzysztof Kozlowski]

On 08/07/2024 23:29, Sagar Cheluvegowda wrote:
> Interconnect is a software framework to access NOC bus topology
> of the system, this framework is designed to provide a standard
> kernel interface to control the settings of the interconnects on
> an SoC.
> The interconnect support is now being added to the stmmac driver
> so that any vendors who wants to use this feature can just
> define corresponging dtsi properties according to their
> NOC bus topologies.
> 
> here is a patch series which is enabling interconnect support
> for ethernet node of SA8775P
> https://lore.kernel.org/all/20240708-icc_bw_voting_emac_dtsi-v1-1-4b091b3150c0@quicinc.com/ 
> 
> Signed-off-by: Sagar Cheluvegowda <quic_scheluve@quicinc.com>
> ---
> Changes in v4:
> - Add reference to the series which is enabling interconnect-properties defined in this series
> - Link to v3: https://lore.kernel.org/r/20240703-icc_bw_voting_from_ethqos-v3-0-8f9148ac60a3@quicinc.com

You got two or three times review, but you keep ignoring it. You are
expecting the community to keep doing the same work, which is waste of
our time and resources.

Best regards,
Krzysztof

Re: [PATCH v4 0/2] Add interconnect support for stmmac driver.

[Sagar Cheluvegowda]

On 7/9/2024 1:58 AM, Krzysztof Kozlowski wrote:
> On 08/07/2024 23:29, Sagar Cheluvegowda wrote:
>> Interconnect is a software framework to access NOC bus topology
>> of the system, this framework is designed to provide a standard
>> kernel interface to control the settings of the interconnects on
>> an SoC.
>> The interconnect support is now being added to the stmmac driver
>> so that any vendors who wants to use this feature can just
>> define corresponging dtsi properties according to their
>> NOC bus topologies.
>>
>> here is a patch series which is enabling interconnect support
>> for ethernet node of SA8775P
>> https://lore.kernel.org/all/20240708-icc_bw_voting_emac_dtsi-v1-1-4b091b3150c0@quicinc.com/ 
>>
>> Signed-off-by: Sagar Cheluvegowda <quic_scheluve@quicinc.com>
>> ---
>> Changes in v4:
>> - Add reference to the series which is enabling interconnect-properties defined in this series
>> - Link to v3: https://lore.kernel.org/r/20240703-icc_bw_voting_from_ethqos-v3-0-8f9148ac60a3@quicinc.com
> 
> You got two or three times review, but you keep ignoring it. You are
> expecting the community to keep doing the same work, which is waste of
> our time and resources.
> 
> Best regards,
> Krzysztof
> 
I will make sure to add the required Reviewed by or any other tags on the patches in future.

Re: [PATCH v4 1/2] dt-bindings: net: qcom: ethernet: Add interconnect properties

[Sagar Cheluvegowda]

On 7/9/2024 1:56 AM, Krzysztof Kozlowski wrote:
> On 08/07/2024 23:30, Sagar Cheluvegowda wrote:
>> Add documentation for the interconnect and interconnect-names
>> properties required when voting for AHB and AXI buses.
>>
>> Suggested-by: Andrew Halaney <ahalaney@redhat.com>
>> Signed-off-by: Sagar Cheluvegowda <quic_scheluve@quicinc.com>
> 
> <form letter>
> This is a friendly reminder during the review process.
> 
> It looks like you received a tag and forgot to add it.
> 
> If you do not know the process, here is a short explanation:
> Please add Acked-by/Reviewed-by/Tested-by tags when posting new
> versions, under or above your Signed-off-by tag. Tag is "received", when
> provided in a message replied to you on the mailing list. Tools like b4
> can help here. However, there's no need to repost patches *only* to add
> the tags. The upstream maintainer will do that for tags received on the
> version they apply.
> 
> https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577
> 
> If a tag was not added on purpose, please state why and what changed.
> </form letter>
> 
> Best regards,
> Krzysztof
> 


Thanks Krzysztof for the explanation, i will make a note of it.

Re: [PATCH v4 2/2] net: stmmac: Add interconnect support

[Serge Semin]

Hi Sagar

On Mon, Jul 08, 2024 at 02:30:01PM -0700, Sagar Cheluvegowda wrote:
> Add interconnect support to vote for bus bandwidth based
> on the current speed of the driver.
> Adds support for two different paths - one from ethernet to
> DDR and the other from CPU to ethernet, Vote from each
> interconnect client is aggregated and the on-chip interconnect
> hardware is configured to the most appropriate bandwidth profile.
> 
> Suggested-by: Andrew Halaney <ahalaney@redhat.com>
> Signed-off-by: Sagar Cheluvegowda <quic_scheluve@quicinc.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac.h          |  1 +
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c     |  8 ++++++++
>  drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 12 ++++++++++++
>  include/linux/stmmac.h                                |  2 ++
>  4 files changed, 23 insertions(+)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> index b23b920eedb1..56a282d2b8cd 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> @@ -21,6 +21,7 @@
>  #include <linux/ptp_clock_kernel.h>
>  #include <linux/net_tstamp.h>
>  #include <linux/reset.h>
> +#include <linux/interconnect.h>
>  #include <net/page_pool/types.h>
>  #include <net/xdp.h>
>  #include <uapi/linux/bpf.h>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index b3afc7cb7d72..ec7c61ee44d4 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -985,6 +985,12 @@ static void stmmac_fpe_link_state_handle(struct stmmac_priv *priv, bool is_up)
>  	}
>  }
>  
> +static void stmmac_set_icc_bw(struct stmmac_priv *priv, unsigned int speed)
> +{

> +	icc_set_bw(priv->plat->axi_icc_path, Mbps_to_icc(speed), Mbps_to_icc(speed));
> +	icc_set_bw(priv->plat->ahb_icc_path, Mbps_to_icc(speed), Mbps_to_icc(speed));

I've got two questions in this regard:

1. Don't we need to call icc_enable()/icc_disable() in someplace in
the driver? For instance the CPU-MEM path must be enabled before even
the stmmac_dvr_probe() is called, otherwise the CSR won't be
accessible. Right? For the same reason the CPU-MEM bandwidth should be
set in sync with that.

2. Why is the CPU-MAC speed is specified to match the Ethernet link
speed? It doesn't seem reasonable. It's the CSR's access speed and
should be done as fast as possible. Shouldn't it?

> +}
> +
>  static void stmmac_mac_link_down(struct phylink_config *config,
>  				 unsigned int mode, phy_interface_t interface)
>  {
> @@ -1080,6 +1086,8 @@ static void stmmac_mac_link_up(struct phylink_config *config,
>  	if (priv->plat->fix_mac_speed)
>  		priv->plat->fix_mac_speed(priv->plat->bsp_priv, speed, mode);
>  
> +	stmmac_set_icc_bw(priv, speed);
> +
>  	if (!duplex)
>  		ctrl &= ~priv->hw->link.duplex;
>  	else
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index 54797edc9b38..201f9dea6da9 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -642,6 +642,18 @@ stmmac_probe_config_dt(struct platform_device *pdev, u8 *mac)
>  		dev_dbg(&pdev->dev, "PTP rate %d\n", plat->clk_ptp_rate);
>  	}
>  
> +	plat->axi_icc_path = devm_of_icc_get(&pdev->dev, "mac-mem");
> +	if (IS_ERR(plat->axi_icc_path)) {
> +		ret = ERR_CAST(plat->axi_icc_path);
> +		goto error_hw_init;
> +	}
> +
> +	plat->ahb_icc_path = devm_of_icc_get(&pdev->dev, "cpu-mac");
> +	if (IS_ERR(plat->ahb_icc_path)) {
> +		ret = ERR_CAST(plat->ahb_icc_path);
> +		goto error_hw_init;
> +	}
> +
>  	plat->stmmac_rst = devm_reset_control_get_optional(&pdev->dev,
>  							   STMMAC_RESOURCE_NAME);
>  	if (IS_ERR(plat->stmmac_rst)) {
> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
> index f92c195c76ed..385f352a0c23 100644
> --- a/include/linux/stmmac.h
> +++ b/include/linux/stmmac.h
> @@ -283,6 +283,8 @@ struct plat_stmmacenet_data {
>  	struct reset_control *stmmac_rst;
>  	struct reset_control *stmmac_ahb_rst;
>  	struct stmmac_axi *axi;

> +	struct icc_path *axi_icc_path;

The MAC<->MEM interface isn't always AXI (it can be AHB or custom) and

> +	struct icc_path *ahb_icc_path;

the CPU<->MAC isn't always AHB (it can also be APB, AXI, custom). So
the more generic naming would be:

axi_icc_path -> dma_icc_path
and
ahb_icc_path -> csr_icc_path

-Serge(y)

>  	int has_gmac4;
>  	int rss_en;
>  	int mac_port_sel_speed;
> 
> -- 
> 2.34.1
> 
> 

Re: [PATCH v4 2/2] net: stmmac: Add interconnect support

[Sagar Cheluvegowda]

On 8/1/2024 11:32 AM, Serge Semin wrote:
> Hi Sagar
> 
> On Mon, Jul 08, 2024 at 02:30:01PM -0700, Sagar Cheluvegowda wrote:
>> Add interconnect support to vote for bus bandwidth based
>> on the current speed of the driver.
>> Adds support for two different paths - one from ethernet to
>> DDR and the other from CPU to ethernet, Vote from each
>> interconnect client is aggregated and the on-chip interconnect
>> hardware is configured to the most appropriate bandwidth profile.
>>
>> Suggested-by: Andrew Halaney <ahalaney@redhat.com>
>> Signed-off-by: Sagar Cheluvegowda <quic_scheluve@quicinc.com>
>> ---
>>  drivers/net/ethernet/stmicro/stmmac/stmmac.h          |  1 +
>>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c     |  8 ++++++++
>>  drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 12 ++++++++++++
>>  include/linux/stmmac.h                                |  2 ++
>>  4 files changed, 23 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
>> index b23b920eedb1..56a282d2b8cd 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
>> @@ -21,6 +21,7 @@
>>  #include <linux/ptp_clock_kernel.h>
>>  #include <linux/net_tstamp.h>
>>  #include <linux/reset.h>
>> +#include <linux/interconnect.h>
>>  #include <net/page_pool/types.h>
>>  #include <net/xdp.h>
>>  #include <uapi/linux/bpf.h>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> index b3afc7cb7d72..ec7c61ee44d4 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> @@ -985,6 +985,12 @@ static void stmmac_fpe_link_state_handle(struct stmmac_priv *priv, bool is_up)
>>  	}
>>  }
>>  
>> +static void stmmac_set_icc_bw(struct stmmac_priv *priv, unsigned int speed)
>> +{
> 
>> +	icc_set_bw(priv->plat->axi_icc_path, Mbps_to_icc(speed), Mbps_to_icc(speed));
>> +	icc_set_bw(priv->plat->ahb_icc_path, Mbps_to_icc(speed), Mbps_to_icc(speed));
> 
> I've got two questions in this regard:
> 
> 1. Don't we need to call icc_enable()/icc_disable() in someplace in
> the driver? For instance the CPU-MEM path must be enabled before even
> the stmmac_dvr_probe() is called, otherwise the CSR won't be
> accessible. Right? For the same reason the CPU-MEM bandwidth should be
> set in sync with that.
> 
> 2. Why is the CPU-MAC speed is specified to match the Ethernet link
> speed? It doesn't seem reasonable. It's the CSR's access speed and
> should be done as fast as possible. Shouldn't it?
> 
>> +}

I am having internal discussions with clocks team, I will revert back soon with answers.
>> +
>>  static void stmmac_mac_link_down(struct phylink_config *config,
>>  				 unsigned int mode, phy_interface_t interface)
>>  {
>> @@ -1080,6 +1086,8 @@ static void stmmac_mac_link_up(struct phylink_config *config,
>>  	if (priv->plat->fix_mac_speed)
>>  		priv->plat->fix_mac_speed(priv->plat->bsp_priv, speed, mode);
>>  
>> +	stmmac_set_icc_bw(priv, speed);
>> +
>>  	if (!duplex)
>>  		ctrl &= ~priv->hw->link.duplex;
>>  	else
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> index 54797edc9b38..201f9dea6da9 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> @@ -642,6 +642,18 @@ stmmac_probe_config_dt(struct platform_device *pdev, u8 *mac)
>>  		dev_dbg(&pdev->dev, "PTP rate %d\n", plat->clk_ptp_rate);
>>  	}
>>  
>> +	plat->axi_icc_path = devm_of_icc_get(&pdev->dev, "mac-mem");
>> +	if (IS_ERR(plat->axi_icc_path)) {
>> +		ret = ERR_CAST(plat->axi_icc_path);
>> +		goto error_hw_init;
>> +	}
>> +
>> +	plat->ahb_icc_path = devm_of_icc_get(&pdev->dev, "cpu-mac");
>> +	if (IS_ERR(plat->ahb_icc_path)) {
>> +		ret = ERR_CAST(plat->ahb_icc_path);
>> +		goto error_hw_init;
>> +	}
>> +
>>  	plat->stmmac_rst = devm_reset_control_get_optional(&pdev->dev,
>>  							   STMMAC_RESOURCE_NAME);
>>  	if (IS_ERR(plat->stmmac_rst)) {
>> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
>> index f92c195c76ed..385f352a0c23 100644
>> --- a/include/linux/stmmac.h
>> +++ b/include/linux/stmmac.h
>> @@ -283,6 +283,8 @@ struct plat_stmmacenet_data {
>>  	struct reset_control *stmmac_rst;
>>  	struct reset_control *stmmac_ahb_rst;
>>  	struct stmmac_axi *axi;
> 
>> +	struct icc_path *axi_icc_path;
> 
> The MAC<->MEM interface isn't always AXI (it can be AHB or custom) and
> 
>> +	struct icc_path *ahb_icc_path;
> 
> the CPU<->MAC isn't always AHB (it can also be APB, AXI, custom). So
> the more generic naming would be:
> 
> axi_icc_path -> dma_icc_path
> and
> ahb_icc_path -> csr_icc_path
> 
> -Serge(y)
> 
>>  	int has_gmac4;
>>  	int rss_en;
>>  	int mac_port_sel_speed;
>>
>> -- 
>> 2.34.1
>>
>>