diff mbox series

[net-next,03/12] net: mscc: ocelot: move invariant configs out of adjust_link

Message ID 20191112124420.6225-4-olteanv@gmail.com
State Changes Requested
Delegated to: David Miller
Headers show
Series [net-next,01/12] net: mscc: ocelot: move resource ioremap and regmap init to common code | expand

Commit Message

Vladimir Oltean Nov. 12, 2019, 12:44 p.m. UTC
From: Vladimir Oltean <vladimir.oltean@nxp.com>

It doesn't make sense to rewrite all these registers every time the PHY
library notifies us about a link state change.

In a future patch we will customize the MTU for the CPU port, and since
the MTU was previously configured from adjust_link, if we don't make
this change, its value would have got overridden.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/ethernet/mscc/ocelot.c | 85 +++++++++++++++---------------
 1 file changed, 43 insertions(+), 42 deletions(-)

Comments

Andrew Lunn Nov. 12, 2019, 1:35 p.m. UTC | #1
On Tue, Nov 12, 2019 at 02:44:11PM +0200, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> It doesn't make sense to rewrite all these registers every time the PHY
> library notifies us about a link state change.
> 
> In a future patch we will customize the MTU for the CPU port, and since
> the MTU was previously configured from adjust_link, if we don't make
> this change, its value would have got overridden.

This is also a good change in preparation of PHYLINK.  When you do
that conversion, ocelot_adjust_link() is likely to become
ocelot_mac_config(). It should only change hardware state when there
actually is a change in link state. This is something drivers often
get wrong.

> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

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

    Andrew



> ---
>  drivers/net/ethernet/mscc/ocelot.c | 85 +++++++++++++++---------------
>  1 file changed, 43 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
> index 2b6792ab0eda..4558c09e2e8a 100644
> --- a/drivers/net/ethernet/mscc/ocelot.c
> +++ b/drivers/net/ethernet/mscc/ocelot.c
> @@ -408,7 +408,7 @@ static void ocelot_adjust_link(struct ocelot *ocelot, int port,
>  			       struct phy_device *phydev)
>  {
>  	struct ocelot_port *ocelot_port = ocelot->ports[port];
> -	int speed, atop_wm, mode = 0;
> +	int speed, mode = 0;
>  
>  	switch (phydev->speed) {
>  	case SPEED_10:
> @@ -440,32 +440,9 @@ static void ocelot_adjust_link(struct ocelot *ocelot, int port,
>  	ocelot_port_writel(ocelot_port, DEV_MAC_MODE_CFG_FDX_ENA |
>  			   mode, DEV_MAC_MODE_CFG);
>  
> -	/* Set MAC IFG Gaps
> -	 * FDX: TX_IFG = 5, RX_IFG1 = RX_IFG2 = 0
> -	 * !FDX: TX_IFG = 5, RX_IFG1 = RX_IFG2 = 5
> -	 */
> -	ocelot_port_writel(ocelot_port, DEV_MAC_IFG_CFG_TX_IFG(5),
> -			   DEV_MAC_IFG_CFG);
> -
> -	/* Load seed (0) and set MAC HDX late collision  */
> -	ocelot_port_writel(ocelot_port, DEV_MAC_HDX_CFG_LATE_COL_POS(67) |
> -			   DEV_MAC_HDX_CFG_SEED_LOAD,
> -			   DEV_MAC_HDX_CFG);
> -	mdelay(1);
> -	ocelot_port_writel(ocelot_port, DEV_MAC_HDX_CFG_LATE_COL_POS(67),
> -			   DEV_MAC_HDX_CFG);
> -
>  	if (ocelot->ops->pcs_init)
>  		ocelot->ops->pcs_init(ocelot, port);
>  
> -	/* Set Max Length and maximum tags allowed */
> -	ocelot_port_writel(ocelot_port, VLAN_ETH_FRAME_LEN,
> -			   DEV_MAC_MAXLEN_CFG);
> -	ocelot_port_writel(ocelot_port, DEV_MAC_TAGS_CFG_TAG_ID(ETH_P_8021AD) |
> -			   DEV_MAC_TAGS_CFG_VLAN_AWR_ENA |
> -			   DEV_MAC_TAGS_CFG_VLAN_LEN_AWR_ENA,
> -			   DEV_MAC_TAGS_CFG);
> -
>  	/* Enable MAC module */
>  	ocelot_port_writel(ocelot_port, DEV_MAC_ENA_CFG_RX_ENA |
>  			   DEV_MAC_ENA_CFG_TX_ENA, DEV_MAC_ENA_CFG);
> @@ -475,22 +452,10 @@ static void ocelot_adjust_link(struct ocelot *ocelot, int port,
>  	ocelot_port_writel(ocelot_port, DEV_CLOCK_CFG_LINK_SPEED(speed),
>  			   DEV_CLOCK_CFG);
>  
> -	/* Set SMAC of Pause frame (00:00:00:00:00:00) */
> -	ocelot_port_writel(ocelot_port, 0, DEV_MAC_FC_MAC_HIGH_CFG);
> -	ocelot_port_writel(ocelot_port, 0, DEV_MAC_FC_MAC_LOW_CFG);
> -
>  	/* No PFC */
>  	ocelot_write_gix(ocelot, ANA_PFC_PFC_CFG_FC_LINK_SPEED(speed),
>  			 ANA_PFC_PFC_CFG, port);
>  
> -	/* Set Pause WM hysteresis
> -	 * 152 = 6 * VLAN_ETH_FRAME_LEN / OCELOT_BUFFER_CELL_SZ
> -	 * 101 = 4 * VLAN_ETH_FRAME_LEN / OCELOT_BUFFER_CELL_SZ
> -	 */
> -	ocelot_write_rix(ocelot, SYS_PAUSE_CFG_PAUSE_ENA |
> -			 SYS_PAUSE_CFG_PAUSE_STOP(101) |
> -			 SYS_PAUSE_CFG_PAUSE_START(152), SYS_PAUSE_CFG, port);
> -
>  	/* Core: Enable port for frame transfer */
>  	ocelot_write_rix(ocelot, QSYS_SWITCH_PORT_MODE_INGRESS_DROP_MODE |
>  			 QSYS_SWITCH_PORT_MODE_SCH_NEXT_CFG(1) |
> @@ -505,12 +470,6 @@ static void ocelot_adjust_link(struct ocelot *ocelot, int port,
>  			 SYS_MAC_FC_CFG_FC_LINK_SPEED(speed),
>  			 SYS_MAC_FC_CFG, port);
>  	ocelot_write_rix(ocelot, 0, ANA_POL_FLOWC, port);
> -
> -	/* Tail dropping watermark */
> -	atop_wm = (ocelot->shared_queue_sz - 9 * VLAN_ETH_FRAME_LEN) / OCELOT_BUFFER_CELL_SZ;
> -	ocelot_write_rix(ocelot, ocelot_wm_enc(9 * VLAN_ETH_FRAME_LEN),
> -			 SYS_ATOP, port);
> -	ocelot_write(ocelot, ocelot_wm_enc(atop_wm), SYS_ATOP_TOT_CFG);
>  }
>  
>  static void ocelot_port_adjust_link(struct net_device *dev)
> @@ -2141,11 +2100,53 @@ static int ocelot_init_timestamp(struct ocelot *ocelot)
>  static void ocelot_init_port(struct ocelot *ocelot, int port)
>  {
>  	struct ocelot_port *ocelot_port = ocelot->ports[port];
> +	int atop_wm;
>  
>  	INIT_LIST_HEAD(&ocelot_port->skbs);
>  
>  	/* Basic L2 initialization */
>  
> +	/* Set MAC IFG Gaps
> +	 * FDX: TX_IFG = 5, RX_IFG1 = RX_IFG2 = 0
> +	 * !FDX: TX_IFG = 5, RX_IFG1 = RX_IFG2 = 5
> +	 */
> +	ocelot_port_writel(ocelot_port, DEV_MAC_IFG_CFG_TX_IFG(5),
> +			   DEV_MAC_IFG_CFG);
> +
> +	/* Load seed (0) and set MAC HDX late collision  */
> +	ocelot_port_writel(ocelot_port, DEV_MAC_HDX_CFG_LATE_COL_POS(67) |
> +			   DEV_MAC_HDX_CFG_SEED_LOAD,
> +			   DEV_MAC_HDX_CFG);
> +	mdelay(1);
> +	ocelot_port_writel(ocelot_port, DEV_MAC_HDX_CFG_LATE_COL_POS(67),
> +			   DEV_MAC_HDX_CFG);
> +
> +	/* Set Max Length and maximum tags allowed */
> +	ocelot_port_writel(ocelot_port, VLAN_ETH_FRAME_LEN,
> +			   DEV_MAC_MAXLEN_CFG);
> +	ocelot_port_writel(ocelot_port, DEV_MAC_TAGS_CFG_TAG_ID(ETH_P_8021AD) |
> +			   DEV_MAC_TAGS_CFG_VLAN_AWR_ENA |
> +			   DEV_MAC_TAGS_CFG_VLAN_LEN_AWR_ENA,
> +			   DEV_MAC_TAGS_CFG);
> +
> +	/* Set SMAC of Pause frame (00:00:00:00:00:00) */
> +	ocelot_port_writel(ocelot_port, 0, DEV_MAC_FC_MAC_HIGH_CFG);
> +	ocelot_port_writel(ocelot_port, 0, DEV_MAC_FC_MAC_LOW_CFG);
> +
> +	/* Set Pause WM hysteresis
> +	 * 152 = 6 * VLAN_ETH_FRAME_LEN / OCELOT_BUFFER_CELL_SZ
> +	 * 101 = 4 * VLAN_ETH_FRAME_LEN / OCELOT_BUFFER_CELL_SZ
> +	 */
> +	ocelot_write_rix(ocelot, SYS_PAUSE_CFG_PAUSE_ENA |
> +			 SYS_PAUSE_CFG_PAUSE_STOP(101) |
> +			 SYS_PAUSE_CFG_PAUSE_START(152), SYS_PAUSE_CFG, port);
> +
> +	/* Tail dropping watermark */
> +	atop_wm = (ocelot->shared_queue_sz - 9 * VLAN_ETH_FRAME_LEN) / OCELOT_BUFFER_CELL_SZ;
> +	ocelot_write_rix(ocelot, ocelot_wm_enc(9 * VLAN_ETH_FRAME_LEN),
> +			 SYS_ATOP, port);
> +	ocelot_write(ocelot, ocelot_wm_enc(atop_wm), SYS_ATOP_TOT_CFG);
> +
>  	/* Drop frames with multicast source address */
>  	ocelot_rmw_gix(ocelot, ANA_PORT_DROP_CFG_DROP_MC_SMAC_ENA,
>  		       ANA_PORT_DROP_CFG_DROP_MC_SMAC_ENA,
> -- 
> 2.17.1
>
Vladimir Oltean Nov. 12, 2019, 1:38 p.m. UTC | #2
On Tue, 12 Nov 2019 at 15:35, Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Tue, Nov 12, 2019 at 02:44:11PM +0200, Vladimir Oltean wrote:
> > From: Vladimir Oltean <vladimir.oltean@nxp.com>
> >
> > It doesn't make sense to rewrite all these registers every time the PHY
> > library notifies us about a link state change.
> >
> > In a future patch we will customize the MTU for the CPU port, and since
> > the MTU was previously configured from adjust_link, if we don't make
> > this change, its value would have got overridden.
>
> This is also a good change in preparation of PHYLINK.  When you do
> that conversion, ocelot_adjust_link() is likely to become
> ocelot_mac_config(). It should only change hardware state when there
> actually is a change in link state. This is something drivers often
> get wrong.
>

Yes. We'll need PHYLINK because the CPU port is 2.5G fixed-link, which
is something PHYLIB can't describe. One at a time, though.

> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
>
>     Andrew
>
>

[snip]
Florian Fainelli Nov. 12, 2019, 9:30 p.m. UTC | #3
On 11/12/19 4:44 AM, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> It doesn't make sense to rewrite all these registers every time the PHY
> library notifies us about a link state change.
> 
> In a future patch we will customize the MTU for the CPU port, and since
> the MTU was previously configured from adjust_link, if we don't make
> this change, its value would have got overridden.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
diff mbox series

Patch

diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index 2b6792ab0eda..4558c09e2e8a 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -408,7 +408,7 @@  static void ocelot_adjust_link(struct ocelot *ocelot, int port,
 			       struct phy_device *phydev)
 {
 	struct ocelot_port *ocelot_port = ocelot->ports[port];
-	int speed, atop_wm, mode = 0;
+	int speed, mode = 0;
 
 	switch (phydev->speed) {
 	case SPEED_10:
@@ -440,32 +440,9 @@  static void ocelot_adjust_link(struct ocelot *ocelot, int port,
 	ocelot_port_writel(ocelot_port, DEV_MAC_MODE_CFG_FDX_ENA |
 			   mode, DEV_MAC_MODE_CFG);
 
-	/* Set MAC IFG Gaps
-	 * FDX: TX_IFG = 5, RX_IFG1 = RX_IFG2 = 0
-	 * !FDX: TX_IFG = 5, RX_IFG1 = RX_IFG2 = 5
-	 */
-	ocelot_port_writel(ocelot_port, DEV_MAC_IFG_CFG_TX_IFG(5),
-			   DEV_MAC_IFG_CFG);
-
-	/* Load seed (0) and set MAC HDX late collision  */
-	ocelot_port_writel(ocelot_port, DEV_MAC_HDX_CFG_LATE_COL_POS(67) |
-			   DEV_MAC_HDX_CFG_SEED_LOAD,
-			   DEV_MAC_HDX_CFG);
-	mdelay(1);
-	ocelot_port_writel(ocelot_port, DEV_MAC_HDX_CFG_LATE_COL_POS(67),
-			   DEV_MAC_HDX_CFG);
-
 	if (ocelot->ops->pcs_init)
 		ocelot->ops->pcs_init(ocelot, port);
 
-	/* Set Max Length and maximum tags allowed */
-	ocelot_port_writel(ocelot_port, VLAN_ETH_FRAME_LEN,
-			   DEV_MAC_MAXLEN_CFG);
-	ocelot_port_writel(ocelot_port, DEV_MAC_TAGS_CFG_TAG_ID(ETH_P_8021AD) |
-			   DEV_MAC_TAGS_CFG_VLAN_AWR_ENA |
-			   DEV_MAC_TAGS_CFG_VLAN_LEN_AWR_ENA,
-			   DEV_MAC_TAGS_CFG);
-
 	/* Enable MAC module */
 	ocelot_port_writel(ocelot_port, DEV_MAC_ENA_CFG_RX_ENA |
 			   DEV_MAC_ENA_CFG_TX_ENA, DEV_MAC_ENA_CFG);
@@ -475,22 +452,10 @@  static void ocelot_adjust_link(struct ocelot *ocelot, int port,
 	ocelot_port_writel(ocelot_port, DEV_CLOCK_CFG_LINK_SPEED(speed),
 			   DEV_CLOCK_CFG);
 
-	/* Set SMAC of Pause frame (00:00:00:00:00:00) */
-	ocelot_port_writel(ocelot_port, 0, DEV_MAC_FC_MAC_HIGH_CFG);
-	ocelot_port_writel(ocelot_port, 0, DEV_MAC_FC_MAC_LOW_CFG);
-
 	/* No PFC */
 	ocelot_write_gix(ocelot, ANA_PFC_PFC_CFG_FC_LINK_SPEED(speed),
 			 ANA_PFC_PFC_CFG, port);
 
-	/* Set Pause WM hysteresis
-	 * 152 = 6 * VLAN_ETH_FRAME_LEN / OCELOT_BUFFER_CELL_SZ
-	 * 101 = 4 * VLAN_ETH_FRAME_LEN / OCELOT_BUFFER_CELL_SZ
-	 */
-	ocelot_write_rix(ocelot, SYS_PAUSE_CFG_PAUSE_ENA |
-			 SYS_PAUSE_CFG_PAUSE_STOP(101) |
-			 SYS_PAUSE_CFG_PAUSE_START(152), SYS_PAUSE_CFG, port);
-
 	/* Core: Enable port for frame transfer */
 	ocelot_write_rix(ocelot, QSYS_SWITCH_PORT_MODE_INGRESS_DROP_MODE |
 			 QSYS_SWITCH_PORT_MODE_SCH_NEXT_CFG(1) |
@@ -505,12 +470,6 @@  static void ocelot_adjust_link(struct ocelot *ocelot, int port,
 			 SYS_MAC_FC_CFG_FC_LINK_SPEED(speed),
 			 SYS_MAC_FC_CFG, port);
 	ocelot_write_rix(ocelot, 0, ANA_POL_FLOWC, port);
-
-	/* Tail dropping watermark */
-	atop_wm = (ocelot->shared_queue_sz - 9 * VLAN_ETH_FRAME_LEN) / OCELOT_BUFFER_CELL_SZ;
-	ocelot_write_rix(ocelot, ocelot_wm_enc(9 * VLAN_ETH_FRAME_LEN),
-			 SYS_ATOP, port);
-	ocelot_write(ocelot, ocelot_wm_enc(atop_wm), SYS_ATOP_TOT_CFG);
 }
 
 static void ocelot_port_adjust_link(struct net_device *dev)
@@ -2141,11 +2100,53 @@  static int ocelot_init_timestamp(struct ocelot *ocelot)
 static void ocelot_init_port(struct ocelot *ocelot, int port)
 {
 	struct ocelot_port *ocelot_port = ocelot->ports[port];
+	int atop_wm;
 
 	INIT_LIST_HEAD(&ocelot_port->skbs);
 
 	/* Basic L2 initialization */
 
+	/* Set MAC IFG Gaps
+	 * FDX: TX_IFG = 5, RX_IFG1 = RX_IFG2 = 0
+	 * !FDX: TX_IFG = 5, RX_IFG1 = RX_IFG2 = 5
+	 */
+	ocelot_port_writel(ocelot_port, DEV_MAC_IFG_CFG_TX_IFG(5),
+			   DEV_MAC_IFG_CFG);
+
+	/* Load seed (0) and set MAC HDX late collision  */
+	ocelot_port_writel(ocelot_port, DEV_MAC_HDX_CFG_LATE_COL_POS(67) |
+			   DEV_MAC_HDX_CFG_SEED_LOAD,
+			   DEV_MAC_HDX_CFG);
+	mdelay(1);
+	ocelot_port_writel(ocelot_port, DEV_MAC_HDX_CFG_LATE_COL_POS(67),
+			   DEV_MAC_HDX_CFG);
+
+	/* Set Max Length and maximum tags allowed */
+	ocelot_port_writel(ocelot_port, VLAN_ETH_FRAME_LEN,
+			   DEV_MAC_MAXLEN_CFG);
+	ocelot_port_writel(ocelot_port, DEV_MAC_TAGS_CFG_TAG_ID(ETH_P_8021AD) |
+			   DEV_MAC_TAGS_CFG_VLAN_AWR_ENA |
+			   DEV_MAC_TAGS_CFG_VLAN_LEN_AWR_ENA,
+			   DEV_MAC_TAGS_CFG);
+
+	/* Set SMAC of Pause frame (00:00:00:00:00:00) */
+	ocelot_port_writel(ocelot_port, 0, DEV_MAC_FC_MAC_HIGH_CFG);
+	ocelot_port_writel(ocelot_port, 0, DEV_MAC_FC_MAC_LOW_CFG);
+
+	/* Set Pause WM hysteresis
+	 * 152 = 6 * VLAN_ETH_FRAME_LEN / OCELOT_BUFFER_CELL_SZ
+	 * 101 = 4 * VLAN_ETH_FRAME_LEN / OCELOT_BUFFER_CELL_SZ
+	 */
+	ocelot_write_rix(ocelot, SYS_PAUSE_CFG_PAUSE_ENA |
+			 SYS_PAUSE_CFG_PAUSE_STOP(101) |
+			 SYS_PAUSE_CFG_PAUSE_START(152), SYS_PAUSE_CFG, port);
+
+	/* Tail dropping watermark */
+	atop_wm = (ocelot->shared_queue_sz - 9 * VLAN_ETH_FRAME_LEN) / OCELOT_BUFFER_CELL_SZ;
+	ocelot_write_rix(ocelot, ocelot_wm_enc(9 * VLAN_ETH_FRAME_LEN),
+			 SYS_ATOP, port);
+	ocelot_write(ocelot, ocelot_wm_enc(atop_wm), SYS_ATOP_TOT_CFG);
+
 	/* Drop frames with multicast source address */
 	ocelot_rmw_gix(ocelot, ANA_PORT_DROP_CFG_DROP_MC_SMAC_ENA,
 		       ANA_PORT_DROP_CFG_DROP_MC_SMAC_ENA,