[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Zn7OlQ4aoO2vZTrj@shell.armlinux.org.uk>
Date: Fri, 28 Jun 2024 15:54:13 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Serge Semin <fancer.lancer@...il.com>
Cc: Andrew Halaney <ahalaney@...hat.com>, 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>,
Alexei Starovoitov <ast@...nel.org>,
Jesper Dangaard Brouer <hawk@...nel.org>,
John Fastabend <john.fastabend@...il.com>,
Daniel Borkmann <daniel@...earbox.net>,
linux-arm-kernel@...ts.infradead.org,
linux-stm32@...md-mailman.stormreply.com, bpf@...r.kernel.org,
netdev@...r.kernel.org, linux-arm-msm@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC net-next v2 10/17] net: stmmac: Introduce internal
PCS offset-based CSR access
On Mon, Jun 24, 2024 at 04:26:27PM +0300, Serge Semin wrote:
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> index 80eb72bc6311..d0bcebe87ee8 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> @@ -633,7 +633,7 @@ static int ethqos_configure_sgmii(struct qcom_ethqos *ethqos)
> RGMII_CONFIG2_RGMII_CLK_SEL_CFG,
> RGMII_IO_MACRO_CONFIG2);
> ethqos_set_serdes_speed(ethqos, SPEED_2500);
> - stmmac_pcs_ctrl_ane(priv, priv->ioaddr, 0, 0, 0);
> + stmmac_pcs_ctrl_ane(priv, priv->pcsaddr, 0, 0, 0);
> break;
> case SPEED_1000:
> val &= ~ETHQOS_MAC_CTRL_PORT_SEL;
> @@ -641,12 +641,12 @@ static int ethqos_configure_sgmii(struct qcom_ethqos *ethqos)
> RGMII_CONFIG2_RGMII_CLK_SEL_CFG,
> RGMII_IO_MACRO_CONFIG2);
> ethqos_set_serdes_speed(ethqos, SPEED_1000);
> - stmmac_pcs_ctrl_ane(priv, priv->ioaddr, 1, 0, 0);
> + stmmac_pcs_ctrl_ane(priv, priv->pcsaddr, 1, 0, 0);
> break;
> case SPEED_100:
> val |= ETHQOS_MAC_CTRL_PORT_SEL | ETHQOS_MAC_CTRL_SPEED_MODE;
> ethqos_set_serdes_speed(ethqos, SPEED_1000);
> - stmmac_pcs_ctrl_ane(priv, priv->ioaddr, 1, 0, 0);
> + stmmac_pcs_ctrl_ane(priv, priv->pcsaddr, 1, 0, 0);
> break;
> case SPEED_10:
> val |= ETHQOS_MAC_CTRL_PORT_SEL;
> @@ -656,7 +656,7 @@ static int ethqos_configure_sgmii(struct qcom_ethqos *ethqos)
> SGMII_10M_RX_CLK_DVDR),
> RGMII_IO_MACRO_CONFIG);
> ethqos_set_serdes_speed(ethqos, SPEED_1000);
> - stmmac_pcs_ctrl_ane(priv, priv->ioaddr, 1, 0, 0);
> + stmmac_pcs_ctrl_ane(priv, priv->pcsaddr, 1, 0, 0);
> break;
> }
>
I think a better preparatory patch (given what you do in future patches)
would be to change all of these to:
ethqos_pcs_set_inband(priv, {false | true});
which would be:
static void ethqos_pcs_set_inband(struct stmmac_priv *priv, bool enable)
{
stmmac_pcs_ctrl_ane(priv, priv->ioaddr, enable, 0, 0);
}
which then means this patch becomes a single line, and your subsequent
patch just has to replace stmmac_pcs_ctrl_ane() with its open-coded
equivalent.
> diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.c b/drivers/net/ethernet/stmicro/stmmac/hwif.c
> index 84fd57b76fad..3666893acb69 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/hwif.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.c
> @@ -6,6 +6,7 @@
>
> #include "common.h"
> #include "stmmac.h"
> +#include "stmmac_pcs.h"
> #include "stmmac_ptp.h"
> #include "stmmac_est.h"
>
> @@ -116,6 +117,7 @@ static const struct stmmac_hwif_entry {
> const void *tc;
> const void *mmc;
> const void *est;
> + const void *pcs;
I'm not a fan of void pointers. common.h includes linux/phylink.h, which
will define struct phylink_pcs_ops, so there is no reason not to declare
this as:
const struct phylink_pcs_ops *pcs;
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Powered by blists - more mailing lists