diff mbox series

[net-next,v4,6/9] net: dsa: microchip: dcb: add special handling for KSZ88X3 family

Message ID 20240408074758.1825674-7-o.rempel@pengutronix.de (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series Enhanced DCB and DSCP Support for KSZ Switches | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 942 this patch: 942
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 9 of 9 maintainers
netdev/build_clang success Errors and warnings before: 953 this patch: 953
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 953 this patch: 953
netdev/checkpatch warning CHECK: Please don't use multiple blank lines WARNING: Missing a blank line after declarations WARNING: braces {} are not necessary for any arm of this statement WARNING: line length of 81 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-04-08--21-00 (tests: 956)

Commit Message

Oleksij Rempel April 8, 2024, 7:47 a.m. UTC
KSZ88X3 switches have different behavior on different ports:
- It seems to be not possible to disable VLAN PCP classification on port
  2. It means, as soon as mutliqueue support is enabled, frames with
     VLAN tag will get PCP prios. This behavior do not affect Port 1 -
     it is possible to disable PCP prios.
- DSCP classification is not working on Port 2.

Since there are still usable configuration combinations, I added some
quirks to make sure user will get appropriate error message if not
possible configuration is chosen.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
changes v3:
- move ksz8_port2_supported_apptrust[] to this patch, where it is
  actually used.
---
 drivers/net/dsa/microchip/ksz8.h    |   1 +
 drivers/net/dsa/microchip/ksz8795.c |  14 +++
 drivers/net/dsa/microchip/ksz_dcb.c | 189 +++++++++++++++++++++++++++-
 3 files changed, 201 insertions(+), 3 deletions(-)

Comments

Arun Ramadoss April 8, 2024, 4:26 p.m. UTC | #1
Hi Oleksij,

> 
> diff --git a/drivers/net/dsa/microchip/ksz_dcb.c
> b/drivers/net/dsa/microchip/ksz_dcb.c
> index 46491c6dd6f60..2c28c4ed288a9 100644
> --- a/drivers/net/dsa/microchip/ksz_dcb.c
> +++ b/drivers/net/dsa/microchip/ksz_dcb.c
> @@ -83,6 +83,10 @@ static const u8 ksz_supported_apptrust[] = {
>         IEEE_8021QAZ_APP_SEL_DSCP,
>  };
> 
> +static const u8 ksz8_port2_supported_apptrust[] = {
> +       DCB_APP_SEL_PCP,
> +};
> +
>  static const char * const ksz_supported_apptrust_variants[] = {
>         "empty", "dscp", "pcp", "dscp pcp"
>  };
> @@ -128,6 +132,48 @@ int ksz_port_get_default_prio(struct dsa_switch
> *ds, int port)
>         return (data & mask) >> shift;
>  }
> 
> 
> 
> +/**
> + * ksz88x3_port0_apptrust_quirk - Quirk for apptrust configuration
> on Port 1
> + *                               (port == 0) of KSZ88x3 devices
> + * @dev: Pointer to the KSZ switch device structure
> + * @port: Port number for which to set the apptrust selectors
> + * @reg: Register address for the apptrust configuration
> + * @data: Data to set for the apptrust configuration
> + *
> + * This function implements a quirk for apptrust configuration on
> Port 1 of
> + * KSZ88x3 devices. It ensures that apptrust configuration on Port 1
> is not
> + * possible if PCP apptrust on Port 2 is disabled. This is because
> the Port 2
> + * seems to be permanently hardwired to PCP classification, so we
> need to
> + * do Port 1 configuration always in agreement with Port 2
> configuration.
> + *
> + * Return: 0 on success, or a negative error code on failure
> + */
> +static int ksz88x3_port0_apptrust_quirk(struct ksz_device *dev, int
> port,
> +                                       int reg, u8 data)
> +{
> +       u8 port1_data;

why can't we have some common reference, because it is somewhat
confusing. function name is port0, but apptrust config is for port1 and
u8 port1_data. atleast instead of port1_data, port0_data, we can have
variable name as data, since they are handled in two different
functions. 

> +       int ret;
> +
> +       if (!(data & (KSZ8_PORT_802_1P_ENABLE |
> KSZ8_PORT_DIFFSERV_ENABLE)))
> +               return 0;
> +
> +       ret = ksz_pread8(dev, 1, reg, &port1_data);

Having macros for magic number for port 1 and 0 will be good. 
> --
> 2.39.2
>
Oleksij Rempel April 9, 2024, 8:02 a.m. UTC | #2
Hi Arun,

On Mon, Apr 08, 2024 at 04:26:34PM +0000, Arun.Ramadoss@microchip.com wrote:
> Hi Oleksij,
> > + * Return: 0 on success, or a negative error code on failure
> > + */
> > +static int ksz88x3_port0_apptrust_quirk(struct ksz_device *dev, int
> > port,
> > +                                       int reg, u8 data)
> > +{
> > +       u8 port1_data;
> 
> why can't we have some common reference, because it is somewhat
> confusing. function name is port0, but apptrust config is for port1 and
> u8 port1_data. atleast instead of port1_data, port0_data, we can have
> variable name as data, since they are handled in two different
> functions. 

Ack, I renamed variables and functions to be more in sync with the
documentation and add defines for ports

Is it possible to add this erratum to the chip errata documentation?

Regards,
Oleksij
diff mbox series

Patch

diff --git a/drivers/net/dsa/microchip/ksz8.h b/drivers/net/dsa/microchip/ksz8.h
index 571c26ce71e47..6332751f96bdb 100644
--- a/drivers/net/dsa/microchip/ksz8.h
+++ b/drivers/net/dsa/microchip/ksz8.h
@@ -58,5 +58,6 @@  void ksz8_phylink_mac_link_up(struct ksz_device *dev, int port,
 			      unsigned int mode, phy_interface_t interface,
 			      struct phy_device *phydev, int speed, int duplex,
 			      bool tx_pause, bool rx_pause);
+int ksz8_all_queues_split(struct ksz_device *dev, int queues);
 
 #endif
diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index afe61c75944d4..b3e256ffe98f8 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -179,6 +179,20 @@  static int ksz8_port_queue_split(struct ksz_device *dev, int port, int queues)
 	return ksz_prmw8(dev, port, reg_2q, mask_2q, data_2q);
 }
 
+int ksz8_all_queues_split(struct ksz_device *dev, int queues)
+{
+	struct dsa_switch *ds = dev->ds;
+	const struct dsa_port *dp;
+
+	dsa_switch_for_each_port(dp, ds) {
+		int ret = ksz8_port_queue_split(dev, dp->index, queues);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 void ksz8_r_mib_cnt(struct ksz_device *dev, int port, u16 addr, u64 *cnt)
 {
 	const u32 *masks;
diff --git a/drivers/net/dsa/microchip/ksz_dcb.c b/drivers/net/dsa/microchip/ksz_dcb.c
index 46491c6dd6f60..2c28c4ed288a9 100644
--- a/drivers/net/dsa/microchip/ksz_dcb.c
+++ b/drivers/net/dsa/microchip/ksz_dcb.c
@@ -83,6 +83,10 @@  static const u8 ksz_supported_apptrust[] = {
 	IEEE_8021QAZ_APP_SEL_DSCP,
 };
 
+static const u8 ksz8_port2_supported_apptrust[] = {
+	DCB_APP_SEL_PCP,
+};
+
 static const char * const ksz_supported_apptrust_variants[] = {
 	"empty", "dscp", "pcp", "dscp pcp"
 };
@@ -128,6 +132,48 @@  int ksz_port_get_default_prio(struct dsa_switch *ds, int port)
 	return (data & mask) >> shift;
 }
 
+/**
+ * ksz88x3_port_set_default_prio_quirks - Quirks for default priority
+ * @dev: Pointer to the KSZ switch device structure
+ * @port: Port number for which to set the default priority
+ * @prio: Priority value to set
+ *
+ * This function implements quirks for setting the default priority on KSZ88x3
+ * devices. On Port 2 (port == 1), no other priority providers are working
+ * except of PCP. So, configuring default priority on Port 2 is not possible.
+ * On Port 1 (port == 0), it is not possible to configure port priority if PCP
+ * apptrust on Port 2 is disabled. Since we disable multiple queues on the
+ * switch to disable PCP on Port 2, we need to ensure that the default priority
+ * configuration on Port 1 is in agreement with the configuration on Port 2.
+ *
+ * Return: 0 on success, or a negative error code on failure
+ */
+static int ksz88x3_port_set_default_prio_quirks(struct ksz_device *dev, int port,
+						u8 prio)
+{
+	if (!prio)
+		return 0;
+
+	if (port == 1) {
+		dev_err(dev->dev, "Port priority configuration is not working on Port 2\n");
+		return -EINVAL;
+	} else if (port == 0) {
+		u8 port1_data;
+		int ret;
+
+		ret = ksz_pread8(dev, 1, KSZ8_REG_PORT_1_CTRL_0, &port1_data);
+		if (ret)
+			return ret;
+
+		if (!(port1_data & KSZ8_PORT_802_1P_ENABLE)) {
+			dev_err(dev->dev, "Not possible to configur port priority on Port 1 if PCP apptrust on Port 2 is disabled\n");
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
 /**
  * ksz_port_set_default_prio - Sets the default priority for a port on a KSZ
  *			       switch
@@ -143,12 +189,19 @@  int ksz_port_get_default_prio(struct dsa_switch *ds, int port)
 int ksz_port_set_default_prio(struct dsa_switch *ds, int port, u8 prio)
 {
 	struct ksz_device *dev = ds->priv;
-	int reg, shift;
+	int reg, shift, ret;
 	u8 mask;
 
 	if (prio >= dev->info->num_tx_queues)
 		return -EINVAL;
 
+
+	if (ksz_is_ksz88x3(dev)) {
+		ret = ksz88x3_port_set_default_prio_quirks(dev, port, prio);
+		if (ret)
+			return ret;
+	}
+
 	ksz_get_defult_port_prio_reg(dev, &reg, &mask, &shift);
 
 	return ksz_prmw8(dev, port, reg, mask, (prio << shift) & mask);
@@ -409,6 +462,118 @@  static void ksz_get_apptrus_map_and_reg(struct ksz_device *dev,
 	}
 }
 
+/**
+ * ksz88x3_port0_apptrust_quirk - Quirk for apptrust configuration on Port 1
+ *				  (port == 0) of KSZ88x3 devices
+ * @dev: Pointer to the KSZ switch device structure
+ * @port: Port number for which to set the apptrust selectors
+ * @reg: Register address for the apptrust configuration
+ * @data: Data to set for the apptrust configuration
+ *
+ * This function implements a quirk for apptrust configuration on Port 1 of
+ * KSZ88x3 devices. It ensures that apptrust configuration on Port 1 is not
+ * possible if PCP apptrust on Port 2 is disabled. This is because the Port 2
+ * seems to be permanently hardwired to PCP classification, so we need to
+ * do Port 1 configuration always in agreement with Port 2 configuration.
+ *
+ * Return: 0 on success, or a negative error code on failure
+ */
+static int ksz88x3_port0_apptrust_quirk(struct ksz_device *dev, int port,
+					int reg, u8 data)
+{
+	u8 port1_data;
+	int ret;
+
+	if (!(data & (KSZ8_PORT_802_1P_ENABLE | KSZ8_PORT_DIFFSERV_ENABLE)))
+		return 0;
+
+	ret = ksz_pread8(dev, 1, reg, &port1_data);
+	if (ret)
+		return ret;
+
+	if (!(port1_data & KSZ8_PORT_802_1P_ENABLE)) {
+		dev_err(dev->dev, "Not possible to enable any apptrust on Port 1 if PCP apptrust on Port 2 is disabled\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/**
+ * ksz88x3_port1_apptrust_quirk - Quirk for apptrust configuration on Port 2
+ *				  (port == 1) of KSZ88x3 devices
+ * @dev: Pointer to the KSZ switch device structure
+ * @port: Port number for which to set the apptrust selectors
+ * @reg: Register address for the apptrust configuration
+ * @data: Data to set for the apptrust configuration
+ *
+ * This function implements a quirk for apptrust configuration on Port 2 of
+ * KSZ88x3 devices. It ensures that DSCP apptrust is not working on Port 2 and
+ * that it is not possible to disable PCP on Port 2. The only way to disable PCP
+ * on Port 2 is to disable multiple queues on the switch.
+ *
+ * Return: 0 on success, or a negative error code on failure
+ */
+static int ksz88x3_port1_apptrust_quirk(struct ksz_device *dev, int port,
+					int reg, u8 data)
+{
+	struct dsa_switch *ds = dev->ds;
+	u8 port0_data;
+	int ret;
+
+	if (data & KSZ8_PORT_DIFFSERV_ENABLE) {
+		dev_err(dev->dev, "DSCP apptrust is not working on Port 2\n");
+		return -EINVAL;
+	}
+
+	if (data & KSZ8_PORT_802_1P_ENABLE)
+		return ksz8_all_queues_split(dev, dev->info->num_tx_queues);
+
+	ret = ksz_pread8(dev, 0, reg, &port0_data);
+	if (ret)
+		return ret;
+
+	if (port0_data & (KSZ8_PORT_802_1P_ENABLE | KSZ8_PORT_DIFFSERV_ENABLE)) {
+		dev_err(dev->dev, "Not possible to disable PCP on Port 2 if any apptrust is enabled on Port 1\n");
+		return -EINVAL;
+	}
+
+	ret = ksz_port_get_default_prio(ds, 0);
+	if (ret < 0) {
+		return ret;
+	} else if (ret) {
+		dev_err(dev->dev, "Not possible to disable PCP on Port 2 if non zero default priority is set on Port 1\n");
+		return -EINVAL;
+	}
+
+	return ksz8_all_queues_split(dev, 1);
+}
+
+/**
+ * ksz88x3_port_apptrust_quirk - Quirk for apptrust configuration on KSZ88x3
+ *			       devices
+ * @dev: Pointer to the KSZ switch device structure
+ * @port: Port number for which to set the apptrust selectors
+ * @reg: Register address for the apptrust configuration
+ * @data: Data to set for the apptrust configuration
+ *
+ * This function implements a quirk for apptrust configuration on KSZ88x3
+ * devices. It ensures that apptrust configuration on Port 1 (port == 0) and
+ * Port 2 (port == 1) is done in agreement with each other.
+ *
+ * Return: 0 on success, or a negative error code on failure
+ */
+static int ksz88x3_port_apptrust_quirk(struct ksz_device *dev, int port,
+				       int reg, u8 data)
+{
+	if (port == 0) {
+		return ksz88x3_port0_apptrust_quirk(dev, port, reg, data);
+	} else if (port == 1)
+		return ksz88x3_port1_apptrust_quirk(dev, port, reg, data);
+
+	return 0;
+}
+
 /**
  * ksz_port_set_apptrust - Sets the apptrust selectors for a port on a KSZ
  *			   switch
@@ -449,6 +614,12 @@  int ksz_port_set_apptrust(struct dsa_switch *ds, int port,
 		}
 	}
 
+	if (ksz_is_ksz88x3(dev)) {
+		ret = ksz88x3_port_apptrust_quirk(dev, port, reg, data);
+		if (ret)
+			return ret;
+	}
+
 	return ksz_prmw8(dev, port, reg, mask, data);
 }
 
@@ -503,7 +674,9 @@  int ksz_port_get_apptrust(struct dsa_switch *ds, int port, u8 *sel, int *nsel)
  */
 int ksz_dcb_init_port(struct ksz_device *dev, int port)
 {
+	const u8 *sel;
 	int ret, ipv;
+	int sel_len;
 
 	if (is_ksz8(dev)) {
 		ipv = ieee8021q_tt_to_tc(IEEE8021Q_TT_BE,
@@ -519,8 +692,18 @@  int ksz_dcb_init_port(struct ksz_device *dev, int port)
 	if (ret)
 		return ret;
 
-	return ksz_port_set_apptrust(dev->ds, port, ksz_supported_apptrust,
-				     ARRAY_SIZE(ksz_supported_apptrust));
+	if (ksz_is_ksz88x3(dev) && port == 1) {
+		/* KSZ88x3 devices do not support DSCP classification on
+		 * "Port 2" (port == 1).
+		 */
+		sel = ksz8_port2_supported_apptrust;
+		sel_len = ARRAY_SIZE(ksz8_port2_supported_apptrust);
+	} else {
+		sel = ksz_supported_apptrust;
+		sel_len = ARRAY_SIZE(ksz_supported_apptrust);
+	}
+
+	return ksz_port_set_apptrust(dev->ds, port, sel, sel_len);
 }
 
 /**