| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <zsdjc53fxh44bpra5cfishtvmyok2rprbtnbthimnu6quxkxyj@kvtijkxylwb3>
Date: Thu, 1 Aug 2024 21:32:32 +0300
From: Serge Semin <fancer.lancer@...il.com>
To: Sagar Cheluvegowda <quic_scheluve@...cinc.com>
Cc: Vinod Koul <vkoul@...nel.org>,
Alexandre Torgue <alexandre.torgue@...s.st.com>, Jose Abreu <joabreu@...opsys.com>,
"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Maxime Coquelin <mcoquelin.stm32@...il.com>, Russell King <linux@...linux.org.uk>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Bhupesh Sharma <bhupesh.sharma@...aro.org>, kernel@...cinc.com,
Andrew Halaney <ahalaney@...hat.com>, Andrew Lunn <andrew@...n.ch>, linux-arm-msm@...r.kernel.org,
netdev@...r.kernel.org, linux-stm32@...md-mailman.stormreply.com,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v4 2/2] net: stmmac: Add interconnect support
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@...hat.com>
> Signed-off-by: Sagar Cheluvegowda <quic_scheluve@...cinc.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
>
>
Powered by blists - more mailing lists