Commit b240fc56 authored by Jakub Kicinski's avatar Jakub Kicinski
Browse files

Merge branch 'net-dsa-vsc73xx-convert-to-phylink-and-do-some-cleanup' 

Pawel Dembicki says:

====================
net: dsa: vsc73xx: convert to PHYLINK and do some cleanup

This patch series is a result of splitting a larger patch series [0],
where some parts needed to be refactored.

The first patch switches from a poll loop to read_poll_timeout.

The second patch is a simple conversion to phylink because adjust_link
won't work anymore.

The third patch is preparation for future use. Using the
"phy_interface_mode_is_rgmii" macro allows for the proper recognition
of all RGMII modes.

Patches 4-5 involve some cleanup: The fourth patch introduces
a definition with the maximum number of ports to avoid using
magic numbers. The next one fills in documentation.

[0] https://patchwork.kernel.org/project/netdevbpf/list/?series=841034&state=%2A&archive=both
====================

Link: https://lore.kernel.org/r/20240417205048.3542839-1-paweldembicki@gmail.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents c51db4ac 96944aaf

drivers/net/dsa/vitesse-vsc73xx-core.c

+118 −137
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@ 
#include <linux/kernel.h>

#include <linux/module.h>

#include <linux/device.h>

#include <linux/iopoll.h>

#include <linux/of.h>

#include <linux/of_mdio.h>

#include <linux/bitops.h>
@@ -268,6 +269,9 @@ 
#define IS_7398(a) ((a)->chipid == VSC73XX_CHIPID_ID_7398)

#define IS_739X(a) (IS_7395(a) || IS_7398(a))



#define VSC73XX_POLL_SLEEP_US		1000

#define VSC73XX_POLL_TIMEOUT_US		10000



struct vsc73xx_counter {

	u8 counter;

	const char *name;
@@ -713,51 +717,44 @@ static void vsc73xx_init_port(struct vsc73xx *vsc, int port) 
		      port, VSC73XX_C_RX0, 0);

}



static void vsc73xx_adjust_enable_port(struct vsc73xx *vsc,

				       int port, struct phy_device *phydev,

				       u32 initval)

static void vsc73xx_reset_port(struct vsc73xx *vsc, int port, u32 initval)

{

	u32 val = initval;

	u8 seed;



	/* Reset this port FIXME: break out subroutine */

	val |= VSC73XX_MAC_CFG_RESET;

	vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_MAC_CFG, val);



	/* Seed the port randomness with randomness */

	get_random_bytes(&seed, 1);

	val |= seed << VSC73XX_MAC_CFG_SEED_OFFSET;

	val |= VSC73XX_MAC_CFG_SEED_LOAD;

	val |= VSC73XX_MAC_CFG_WEXC_DIS;

	vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_MAC_CFG, val);

	int ret, err;

	u32 val;



	/* Flow control for the PHY facing ports:

	 * Use a zero delay pause frame when pause condition is left

	 * Obey pause control frames

	 * When generating pause frames, use 0xff as pause value

	 */

	vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_FCCONF,

		      VSC73XX_FCCONF_ZERO_PAUSE_EN |

		      VSC73XX_FCCONF_FLOW_CTRL_OBEY |

		      0xff);

	/* Disable RX on this port */

	vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port,

			    VSC73XX_MAC_CFG,

			    VSC73XX_MAC_CFG_RX_EN, 0);



	/* Disallow backward dropping of frames from this port */

	/* Discard packets */

	vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ARBITER, 0,

			    VSC73XX_SBACKWDROP, BIT(port), 0);

			    VSC73XX_ARBDISC, BIT(port), BIT(port));



	/* Enable TX, RX, deassert reset, stop loading seed */

	vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port,

			    VSC73XX_MAC_CFG,

			    VSC73XX_MAC_CFG_RESET | VSC73XX_MAC_CFG_SEED_LOAD |

			    VSC73XX_MAC_CFG_TX_EN | VSC73XX_MAC_CFG_RX_EN,

			    VSC73XX_MAC_CFG_TX_EN | VSC73XX_MAC_CFG_RX_EN);

	/* Wait until queue is empty */

	ret = read_poll_timeout(vsc73xx_read, err,

				err < 0 || (val & BIT(port)),

				VSC73XX_POLL_SLEEP_US,

				VSC73XX_POLL_TIMEOUT_US, false,

				vsc, VSC73XX_BLOCK_ARBITER, 0,

				VSC73XX_ARBEMPTY, &val);

	if (ret)

		dev_err(vsc->dev,

			"timeout waiting for block arbiter\n");

	else if (err < 0)

		dev_err(vsc->dev, "error reading arbiter\n");



	/* Put this port into reset */

	vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_MAC_CFG,

		      VSC73XX_MAC_CFG_RESET | initval);

}



static void vsc73xx_adjust_link(struct dsa_switch *ds, int port,

				struct phy_device *phydev)

static void vsc73xx_mac_config(struct phylink_config *config, unsigned int mode,

			       const struct phylink_link_state *state)

{

	struct vsc73xx *vsc = ds->priv;

	u32 val;

	struct dsa_port *dp = dsa_phylink_to_port(config);

	struct vsc73xx *vsc = dp->ds->priv;

	int port = dp->index;



	/* Special handling of the CPU-facing port */

	if (port == CPU_PORT) {
@@ -774,47 +771,19 @@ static void vsc73xx_adjust_link(struct dsa_switch *ds, int port, 
			      VSC73XX_ADVPORTM_ENA_GTX |

			      VSC73XX_ADVPORTM_DDR_MODE);

	}



	/* This is the MAC confiuration that always need to happen

	 * after a PHY or the CPU port comes up or down.

	 */

	if (!phydev->link) {

		int maxloop = 10;



		dev_dbg(vsc->dev, "port %d: went down\n",

			port);



		/* Disable RX on this port */

		vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port,

				    VSC73XX_MAC_CFG,

				    VSC73XX_MAC_CFG_RX_EN, 0);



		/* Discard packets */

		vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ARBITER, 0,

				    VSC73XX_ARBDISC, BIT(port), BIT(port));



		/* Wait until queue is empty */

		vsc73xx_read(vsc, VSC73XX_BLOCK_ARBITER, 0,

			     VSC73XX_ARBEMPTY, &val);

		while (!(val & BIT(port))) {

			msleep(1);

			vsc73xx_read(vsc, VSC73XX_BLOCK_ARBITER, 0,

				     VSC73XX_ARBEMPTY, &val);

			if (--maxloop == 0) {

				dev_err(vsc->dev,

					"timeout waiting for block arbiter\n");

				/* Continue anyway */

				break;

			}

}



		/* Put this port into reset */

		vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_MAC_CFG,

			      VSC73XX_MAC_CFG_RESET);

static void vsc73xx_mac_link_down(struct phylink_config *config,

				  unsigned int mode, phy_interface_t interface)

{

	struct dsa_port *dp = dsa_phylink_to_port(config);

	struct vsc73xx *vsc = dp->ds->priv;

	int port = dp->index;



		/* Accept packets again */

		vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ARBITER, 0,

				    VSC73XX_ARBDISC, BIT(port), 0);

	/* This routine is described in the datasheet (below ARBDISC register

	 * description)

	 */

	vsc73xx_reset_port(vsc, port, 0);



	/* Allow backward dropping of frames from this port */

	vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ARBITER, 0,
@@ -823,55 +792,72 @@ static void vsc73xx_adjust_link(struct dsa_switch *ds, int port, 
	/* Receive mask (disable forwarding) */

	vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0,

			    VSC73XX_RECVMASK, BIT(port), 0);



		return;

}



	/* Figure out what speed was negotiated */

	if (phydev->speed == SPEED_1000) {

		dev_dbg(vsc->dev, "port %d: 1000 Mbit mode full duplex\n",

			port);

static void vsc73xx_mac_link_up(struct phylink_config *config,

				struct phy_device *phy, unsigned int mode,

				phy_interface_t interface, int speed,

				int duplex, bool tx_pause, bool rx_pause)

{

	struct dsa_port *dp = dsa_phylink_to_port(config);

	struct vsc73xx *vsc = dp->ds->priv;

	int port = dp->index;

	u32 val;

	u8 seed;



		/* Set up default for internal port or external RGMII */

		if (phydev->interface == PHY_INTERFACE_MODE_RGMII)

			val = VSC73XX_MAC_CFG_1000M_F_RGMII;

	if (speed == SPEED_1000)

		val = VSC73XX_MAC_CFG_GIGA_MODE | VSC73XX_MAC_CFG_TX_IPG_1000M;

	else

			val = VSC73XX_MAC_CFG_1000M_F_PHY;

		vsc73xx_adjust_enable_port(vsc, port, phydev, val);

	} else if (phydev->speed == SPEED_100) {

		if (phydev->duplex == DUPLEX_FULL) {

			val = VSC73XX_MAC_CFG_100_10M_F_PHY;

			dev_dbg(vsc->dev,

				"port %d: 100 Mbit full duplex mode\n",

				port);

		} else {

			val = VSC73XX_MAC_CFG_100_10M_H_PHY;

			dev_dbg(vsc->dev,

				"port %d: 100 Mbit half duplex mode\n",

				port);

		}

		vsc73xx_adjust_enable_port(vsc, port, phydev, val);

	} else if (phydev->speed == SPEED_10) {

		if (phydev->duplex == DUPLEX_FULL) {

			val = VSC73XX_MAC_CFG_100_10M_F_PHY;

			dev_dbg(vsc->dev,

				"port %d: 10 Mbit full duplex mode\n",

				port);

		} else {

			val = VSC73XX_MAC_CFG_100_10M_H_PHY;

			dev_dbg(vsc->dev,

				"port %d: 10 Mbit half duplex mode\n",

				port);

		}

		vsc73xx_adjust_enable_port(vsc, port, phydev, val);

	} else {

		dev_err(vsc->dev,

			"could not adjust link: unknown speed\n");

	}

		val = VSC73XX_MAC_CFG_TX_IPG_100_10M;



	if (phy_interface_mode_is_rgmii(interface))

		val |= VSC73XX_MAC_CFG_CLK_SEL_1000M;

	else

		val |= VSC73XX_MAC_CFG_CLK_SEL_EXT;



	if (duplex == DUPLEX_FULL)

		val |= VSC73XX_MAC_CFG_FDX;



	/* This routine is described in the datasheet (below ARBDISC register

	 * description)

	 */

	vsc73xx_reset_port(vsc, port, val);



	/* Seed the port randomness with randomness */

	get_random_bytes(&seed, 1);

	val |= seed << VSC73XX_MAC_CFG_SEED_OFFSET;

	val |= VSC73XX_MAC_CFG_SEED_LOAD;

	val |= VSC73XX_MAC_CFG_WEXC_DIS;

	vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_MAC_CFG, val);



	/* Flow control for the PHY facing ports:

	 * Use a zero delay pause frame when pause condition is left

	 * Obey pause control frames

	 * When generating pause frames, use 0xff as pause value

	 */

	vsc73xx_write(vsc, VSC73XX_BLOCK_MAC, port, VSC73XX_FCCONF,

		      VSC73XX_FCCONF_ZERO_PAUSE_EN |

		      VSC73XX_FCCONF_FLOW_CTRL_OBEY |

		      0xff);



	/* Accept packets again */

	vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ARBITER, 0,

			    VSC73XX_ARBDISC, BIT(port), 0);



	/* Enable port (forwarding) in the receieve mask */

	/* Enable port (forwarding) in the receive mask */

	vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ANALYZER, 0,

			    VSC73XX_RECVMASK, BIT(port), BIT(port));



	/* Disallow backward dropping of frames from this port */

	vsc73xx_update_bits(vsc, VSC73XX_BLOCK_ARBITER, 0,

			    VSC73XX_SBACKWDROP, BIT(port), 0);



	/* Enable TX, RX, deassert reset, stop loading seed */

	vsc73xx_update_bits(vsc, VSC73XX_BLOCK_MAC, port,

			    VSC73XX_MAC_CFG,

			    VSC73XX_MAC_CFG_RESET | VSC73XX_MAC_CFG_SEED_LOAD |

			    VSC73XX_MAC_CFG_TX_EN | VSC73XX_MAC_CFG_RX_EN,

			    VSC73XX_MAC_CFG_TX_EN | VSC73XX_MAC_CFG_RX_EN);

}



static int vsc73xx_port_enable(struct dsa_switch *ds, int port,
@@ -1053,12 +1039,17 @@ static void vsc73xx_phylink_get_caps(struct dsa_switch *dsa, int port, 
	config->mac_capabilities = MAC_SYM_PAUSE | MAC_10 | MAC_100 | MAC_1000;

}



static const struct phylink_mac_ops vsc73xx_phylink_mac_ops = {

	.mac_config = vsc73xx_mac_config,

	.mac_link_down = vsc73xx_mac_link_down,

	.mac_link_up = vsc73xx_mac_link_up,

};



static const struct dsa_switch_ops vsc73xx_ds_ops = {

	.get_tag_protocol = vsc73xx_get_tag_protocol,

	.setup = vsc73xx_setup,

	.phy_read = vsc73xx_phy_read,

	.phy_write = vsc73xx_phy_write,

	.adjust_link = vsc73xx_adjust_link,

	.get_strings = vsc73xx_get_strings,

	.get_ethtool_stats = vsc73xx_get_ethtool_stats,

	.get_sset_count = vsc73xx_get_sset_count,
@@ -1195,26 +1186,16 @@ int vsc73xx_probe(struct vsc73xx *vsc) 
		 vsc->addr[0], vsc->addr[1], vsc->addr[2],

		 vsc->addr[3], vsc->addr[4], vsc->addr[5]);



	/* The VSC7395 switch chips have 5+1 ports which means 5

	 * ordinary ports and a sixth CPU port facing the processor

	 * with an RGMII interface. These ports are numbered 0..4

	 * and 6, so they leave a "hole" in the port map for port 5,

	 * which is invalid.

	 *

	 * The VSC7398 has 8 ports, port 7 is again the CPU port.

	 *

	 * We allocate 8 ports and avoid access to the nonexistant

	 * ports.

	 */

	vsc->ds = devm_kzalloc(dev, sizeof(*vsc->ds), GFP_KERNEL);

	if (!vsc->ds)

		return -ENOMEM;



	vsc->ds->dev = dev;

	vsc->ds->num_ports = 8;

	vsc->ds->num_ports = VSC73XX_MAX_NUM_PORTS;

	vsc->ds->priv = vsc;



	vsc->ds->ops = &vsc73xx_ds_ops;

	vsc->ds->phylink_mac_ops = &vsc73xx_phylink_mac_ops;

	ret = dsa_register_switch(vsc->ds);

	if (ret) {

		dev_err(dev, "unable to register switch (%d)\n", ret);

drivers/net/dsa/vitesse-vsc73xx.h

+26 −1
Original line number Diff line number Diff line
@@ -3,8 +3,28 @@ 
#include <linux/etherdevice.h>

#include <linux/gpio/driver.h>



/* The VSC7395 switch chips have 5+1 ports which means 5 ordinary ports and

 * a sixth CPU port facing the processor with an RGMII interface. These ports

 * are numbered 0..4 and 6, so they leave a "hole" in the port map for port 5,

 * which is invalid.

 *

 * The VSC7398 has 8 ports, port 7 is again the CPU port.

 *

 * We allocate 8 ports and avoid access to the nonexistent ports.

 */

#define VSC73XX_MAX_NUM_PORTS	8



/**

 * struct vsc73xx - VSC73xx state container

 * struct vsc73xx - VSC73xx state container: main data structure

 * @dev: The device pointer

 * @reset: The descriptor for the GPIO line tied to the reset pin

 * @ds: Pointer to the DSA core structure

 * @gc: Main structure of the GPIO controller

 * @chipid: Storage for the Chip ID value read from the CHIPID register of the

 *	switch

 * @addr: MAC address used in flow control frames

 * @ops: Structure with hardware-dependent operations

 * @priv: Pointer to the configuration interface structure

 */

struct vsc73xx {

	struct device			*dev;
@@ -17,6 +37,11 @@ struct vsc73xx { 
	void				*priv;

};



/**

 * struct vsc73xx_ops - VSC73xx methods container

 * @read: Method for register reading over the hardware-dependent interface

 * @write: Method for register writing over the hardware-dependent interface

 */

struct vsc73xx_ops {

	int (*read)(struct vsc73xx *vsc, u8 block, u8 subblock, u8 reg,

		    u32 *val);