Re: [PATCH v4 2/2] net: stmmac: Add interconnect support
From: Serge Semin <hidden>
Date: 2024-08-01 18:32:38
Also in:
linux-arm-kernel, linux-arm-msm, linux-devicetree, lkml
Hi Sagar On Mon, Jul 08, 2024 at 02:30:01PM -0700, Sagar Cheluvegowda wrote:
quoted hunk ↗ jump to hunk
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 <redacted> Signed-off-by: Sagar Cheluvegowda <redacted> --- 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?
quoted hunk ↗ jump to hunk
+} + 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; elsediff --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