public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next 0/3] net: dsa: yt921x: Add port police/tbf support
@ 2026-02-25  9:08 David Yang
  2026-02-25  9:08 ` [PATCH net-next 1/3] net: dsa: yt921x: Refactor long register helpers David Yang
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: David Yang @ 2026-02-25  9:08 UTC (permalink / raw)
  To: netdev
  Cc: David Yang, Andrew Lunn, Vladimir Oltean, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-kernel

David Yang (3):
  net: dsa: yt921x: Refactor long register helpers
  net: dsa: yt921x: Add port police support
  net: dsa: yt921x: Add port qdisc tbf support

 drivers/net/dsa/yt921x.c | 537 +++++++++++++++++++++++++++++++++++----
 drivers/net/dsa/yt921x.h | 159 ++++++++++--
 2 files changed, 623 insertions(+), 73 deletions(-)

--
2.51.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH net-next 1/3] net: dsa: yt921x: Refactor long register helpers
  2026-02-25  9:08 [PATCH net-next 0/3] net: dsa: yt921x: Add port police/tbf support David Yang
@ 2026-02-25  9:08 ` David Yang
  2026-02-25 13:52   ` Andrew Lunn
  2026-02-25  9:08 ` [PATCH net-next 2/3] net: dsa: yt921x: Add port police support David Yang
  2026-02-25  9:08 ` [PATCH net-next 3/3] net: dsa: yt921x: Add port qdisc tbf support David Yang
  2 siblings, 1 reply; 10+ messages in thread
From: David Yang @ 2026-02-25  9:08 UTC (permalink / raw)
  To: netdev
  Cc: David Yang, Andrew Lunn, Vladimir Oltean, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-kernel

Dealing long registers with u64 is good, until you realize there are
longer registers that are 96 bits.

Use u32 arrays instead.

Signed-off-by: David Yang <mmyangfl@gmail.com>
---
 drivers/net/dsa/yt921x.c | 152 +++++++++++++++++++++++++--------------
 drivers/net/dsa/yt921x.h |  37 +++++-----
 2 files changed, 116 insertions(+), 73 deletions(-)

diff --git a/drivers/net/dsa/yt921x.c b/drivers/net/dsa/yt921x.c
index 98e8915dd6c2..fedc72202d1e 100644
--- a/drivers/net/dsa/yt921x.c
+++ b/drivers/net/dsa/yt921x.c
@@ -255,63 +255,102 @@ yt921x_reg_toggle_bits(struct yt921x_priv *priv, u32 reg, u32 mask, bool set)
 	return yt921x_reg_update_bits(priv, reg, mask, !set ? 0 : mask);
 }
 
-/* Some registers, like VLANn_CTRL, should always be written in 64-bit, even if
- * you are to write only the lower / upper 32 bits.
+/* Some registers, like VLANn_CTRL, should always be written in chunks, even if
+ * you only want to write parts of 32 bits.
  *
- * There is no such restriction for reading, but we still provide 64-bit read
- * wrappers so that we always handle u64 values.
+ * There is no such restriction for reading, but we still provide multi-chunk
+ * read wrappers so that we can handle them consistently.
  */
 
-static int yt921x_reg64_read(struct yt921x_priv *priv, u32 reg, u64 *valp)
+static int
+yt921x_longreg_read(struct yt921x_priv *priv, u32 reg, u32 *vals,
+		    unsigned int span)
 {
-	u32 lo;
-	u32 hi;
 	int res;
 
-	res = yt921x_reg_read(priv, reg, &lo);
-	if (res)
-		return res;
-	res = yt921x_reg_read(priv, reg + 4, &hi);
-	if (res)
-		return res;
+	for (unsigned int i = 0; i < span; i++) {
+		res = yt921x_reg_read(priv, reg + 4 * i, &vals[i]);
+		if (res)
+			return res;
+	}
 
-	*valp = ((u64)hi << 32) | lo;
 	return 0;
 }
 
-static int yt921x_reg64_write(struct yt921x_priv *priv, u32 reg, u64 val)
+static int
+yt921x_longreg_write(struct yt921x_priv *priv, u32 reg, const u32 *vals,
+		     unsigned int span)
 {
 	int res;
 
-	res = yt921x_reg_write(priv, reg, (u32)val);
-	if (res)
-		return res;
-	return yt921x_reg_write(priv, reg + 4, (u32)(val >> 32));
+	for (unsigned int i = 0; i < span; i++) {
+		res = yt921x_reg_write(priv, reg + 4 * i, vals[i]);
+		if (res)
+			return res;
+	}
+
+	return 0;
 }
 
 static int
-yt921x_reg64_update_bits(struct yt921x_priv *priv, u32 reg, u64 mask, u64 val)
+yt921x_longreg_update_bits(struct yt921x_priv *priv, u32 reg, const u32 *masks,
+			   const u32 *vals, unsigned int span)
 {
+	bool changed = false;
+	u32 vs[4];
 	int res;
-	u64 v;
-	u64 u;
 
-	res = yt921x_reg64_read(priv, reg, &v);
+	BUILD_BUG_ON(span > ARRAY_SIZE(vs));
+
+	res = yt921x_longreg_read(priv, reg, vs, span);
 	if (res)
 		return res;
 
-	u = v;
-	u &= ~mask;
-	u |= val;
-	if (u == v)
+	for (unsigned int i = 0; i < span; i++) {
+		u32 u = vs[i];
+
+		u &= ~masks[i];
+		u |= vals[i];
+		if (u != vs[i])
+			changed = true;
+
+		vs[i] = u;
+	}
+
+	if (!changed)
 		return 0;
 
-	return yt921x_reg64_write(priv, reg, u);
+	return yt921x_longreg_write(priv, reg, vs, span);
 }
 
-static int yt921x_reg64_clear_bits(struct yt921x_priv *priv, u32 reg, u64 mask)
+static int
+yt921x_longreg_clear_bits(struct yt921x_priv *priv, u32 reg, const u32 *masks,
+			  unsigned int span)
 {
-	return yt921x_reg64_update_bits(priv, reg, mask, 0);
+	bool changed = false;
+	u32 vs[4];
+	int res;
+
+	BUILD_BUG_ON(span > ARRAY_SIZE(vs));
+
+	res = yt921x_longreg_read(priv, reg, vs, span);
+	if (res)
+		return res;
+
+	for (unsigned int i = 0; i < span; i++) {
+		u32 u = vs[i];
+
+		u &= ~masks[i];
+		if (u != vs[i])
+			changed = true;
+
+		vs[i] = u;
+	}
+
+	if (!changed)
+		return 0;
+
+	return yt921x_longreg_write(priv, reg, vs, span);
 }
 
 static int yt921x_reg_mdio_read(void *context, u32 reg, u32 *valp)
@@ -1844,33 +1883,32 @@ yt921x_vlan_filtering(struct yt921x_priv *priv, int port, bool vlan_filtering)
 	return 0;
 }
 
-static int
-yt921x_vlan_del(struct yt921x_priv *priv, int port, u16 vid)
+static int yt921x_vlan_del(struct yt921x_priv *priv, int port, u16 vid)
 {
-	u64 mask64;
+	u32 masks[YT921X_VLAN_CTRL_S];
 
-	mask64 = YT921X_VLAN_CTRL_PORTS(port) |
-		 YT921X_VLAN_CTRL_UNTAG_PORTn(port);
+	masks[0] = YT921X_VLAN_CTRLa_PORTn(port);
+	masks[1] = YT921X_VLAN_CTRLb_UNTAG_PORTn(port);
 
-	return yt921x_reg64_clear_bits(priv, YT921X_VLANn_CTRL(vid), mask64);
+	return yt921x_longreg_clear_bits(priv, YT921X_VLANn_CTRL(vid), masks,
+					 YT921X_VLAN_CTRL_S);
 }
 
 static int
 yt921x_vlan_add(struct yt921x_priv *priv, int port, u16 vid, bool untagged)
 {
-	u64 mask64;
-	u64 ctrl64;
+	u32 masks[YT921X_VLAN_CTRL_S];
+	u32 ctrls[YT921X_VLAN_CTRL_S];
 
-	mask64 = YT921X_VLAN_CTRL_PORTn(port) |
-		 YT921X_VLAN_CTRL_PORTS(priv->cpu_ports_mask);
-	ctrl64 = mask64;
+	masks[0] = YT921X_VLAN_CTRLa_PORTn(port) |
+		   YT921X_VLAN_CTRLa_PORTS(priv->cpu_ports_mask);
+	ctrls[0] = masks[0];
 
-	mask64 |= YT921X_VLAN_CTRL_UNTAG_PORTn(port);
-	if (untagged)
-		ctrl64 |= YT921X_VLAN_CTRL_UNTAG_PORTn(port);
+	masks[1] = YT921X_VLAN_CTRLb_UNTAG_PORTn(port);
+	ctrls[1] = untagged ? masks[1] : 0;
 
-	return yt921x_reg64_update_bits(priv, YT921X_VLANn_CTRL(vid),
-					mask64, ctrl64);
+	return yt921x_longreg_update_bits(priv, YT921X_VLANn_CTRL(vid),
+					  masks, ctrls, YT921X_VLAN_CTRL_S);
 }
 
 static int
@@ -2331,8 +2369,8 @@ yt921x_dsa_vlan_msti_set(struct dsa_switch *ds, struct dsa_bridge bridge,
 			 const struct switchdev_vlan_msti *msti)
 {
 	struct yt921x_priv *priv = to_yt921x_priv(ds);
-	u64 mask64;
-	u64 ctrl64;
+	u32 masks[YT921X_VLAN_CTRL_S];
+	u32 ctrls[YT921X_VLAN_CTRL_S];
 	int res;
 
 	if (!msti->vid)
@@ -2340,12 +2378,14 @@ yt921x_dsa_vlan_msti_set(struct dsa_switch *ds, struct dsa_bridge bridge,
 	if (!msti->msti || msti->msti >= YT921X_MSTI_NUM)
 		return -EINVAL;
 
-	mask64 = YT921X_VLAN_CTRL_STP_ID_M;
-	ctrl64 = YT921X_VLAN_CTRL_STP_ID(msti->msti);
+	masks[0] = 0;
+	ctrls[0] = 0;
+	masks[1] = YT921X_VLAN_CTRLb_STP_ID_M;
+	ctrls[1] = YT921X_VLAN_CTRLb_STP_ID(msti->msti);
 
 	mutex_lock(&priv->reg_lock);
-	res = yt921x_reg64_update_bits(priv, YT921X_VLANn_CTRL(msti->vid),
-				       mask64, ctrl64);
+	res = yt921x_longreg_update_bits(priv, YT921X_VLANn_CTRL(msti->vid),
+					 masks, ctrls, YT921X_VLAN_CTRL_S);
 	mutex_unlock(&priv->reg_lock);
 
 	return res;
@@ -3096,8 +3136,8 @@ static int yt921x_chip_reset(struct yt921x_priv *priv)
 static int yt921x_chip_setup_dsa(struct yt921x_priv *priv)
 {
 	struct dsa_switch *ds = &priv->ds;
+	u32 ctrls[YT921X_VLAN_CTRL_S];
 	unsigned long cpu_ports_mask;
-	u64 ctrl64;
 	u32 ctrl;
 	int port;
 	int res;
@@ -3158,8 +3198,10 @@ static int yt921x_chip_setup_dsa(struct yt921x_priv *priv)
 	/* Tagged VID 0 should be treated as untagged, which confuses the
 	 * hardware a lot
 	 */
-	ctrl64 = YT921X_VLAN_CTRL_LEARN_DIS | YT921X_VLAN_CTRL_PORTS_M;
-	res = yt921x_reg64_write(priv, YT921X_VLANn_CTRL(0), ctrl64);
+	ctrls[0] = YT921X_VLAN_CTRLa_LEARN_DIS | YT921X_VLAN_CTRLa_PORTS_M;
+	ctrls[1] = 0;
+	res = yt921x_longreg_write(priv, YT921X_VLANn_CTRL(0), ctrls,
+				   YT921X_VLAN_CTRL_S);
 	if (res)
 		return res;
 
diff --git a/drivers/net/dsa/yt921x.h b/drivers/net/dsa/yt921x.h
index 3f129b8d403f..361470582687 100644
--- a/drivers/net/dsa/yt921x.h
+++ b/drivers/net/dsa/yt921x.h
@@ -429,24 +429,25 @@ enum yt921x_app_selector {
 #define  YT921X_FDB_HW_FLUSH_ON_LINKDOWN	BIT(0)
 
 #define YT921X_VLANn_CTRL(vlan)		(0x188000 + 8 * (vlan))
-#define  YT921X_VLAN_CTRL_UNTAG_PORTS_M		GENMASK_ULL(50, 40)
-#define   YT921X_VLAN_CTRL_UNTAG_PORTS(x)		FIELD_PREP(YT921X_VLAN_CTRL_UNTAG_PORTS_M, (x))
-#define  YT921X_VLAN_CTRL_UNTAG_PORTn(port)	BIT_ULL((port) + 40)
-#define  YT921X_VLAN_CTRL_STP_ID_M		GENMASK_ULL(39, 36)
-#define   YT921X_VLAN_CTRL_STP_ID(x)			FIELD_PREP(YT921X_VLAN_CTRL_STP_ID_M, (x))
-#define  YT921X_VLAN_CTRL_SVLAN_EN		BIT_ULL(35)
-#define  YT921X_VLAN_CTRL_FID_M			GENMASK_ULL(34, 23)
-#define   YT921X_VLAN_CTRL_FID(x)			FIELD_PREP(YT921X_VLAN_CTRL_FID_M, (x))
-#define  YT921X_VLAN_CTRL_LEARN_DIS		BIT_ULL(22)
-#define  YT921X_VLAN_CTRL_PRIO_EN		BIT_ULL(21)
-#define  YT921X_VLAN_CTRL_PRIO_M		GENMASK_ULL(20, 18)
-#define   YT921X_VLAN_CTRL_PRIO(x)			FIELD_PREP(YT921X_VLAN_CTRL_PRIO_M, (x))
-#define  YT921X_VLAN_CTRL_PORTS_M		GENMASK_ULL(17, 7)
-#define   YT921X_VLAN_CTRL_PORTS(x)			FIELD_PREP(YT921X_VLAN_CTRL_PORTS_M, (x))
-#define  YT921X_VLAN_CTRL_PORTn(port)		BIT_ULL((port) + 7)
-#define  YT921X_VLAN_CTRL_BYPASS_1X_AC		BIT_ULL(6)
-#define  YT921X_VLAN_CTRL_METER_EN		BIT_ULL(5)
-#define  YT921X_VLAN_CTRL_METER_ID_M		GENMASK_ULL(4, 0)
+#define  YT921X_VLAN_CTRL_S			2
+#define  YT921X_VLAN_CTRLb_UNTAG_PORTS_M	GENMASK(18, 8)
+#define   YT921X_VLAN_CTRLb_UNTAG_PORTS(x)		FIELD_PREP(YT921X_VLAN_CTRLb_UNTAG_PORTS_M, (x))
+#define  YT921X_VLAN_CTRLb_UNTAG_PORTn(port)	BIT((port) + 8)
+#define  YT921X_VLAN_CTRLb_STP_ID_M		GENMASK(7, 4)
+#define   YT921X_VLAN_CTRLb_STP_ID(x)			FIELD_PREP(YT921X_VLAN_CTRLb_STP_ID_M, (x))
+#define  YT921X_VLAN_CTRLb_SVLAN_EN		BIT(3)
+#define  YT921X_VLAN_CTRLab_FID_M		GENMASK_ULL(34, 23)
+#define   YT921X_VLAN_CTRLab_FID(x)			FIELD_PREP(YT921X_VLAN_CTRLab_FID_M, (x))
+#define  YT921X_VLAN_CTRLa_LEARN_DIS		BIT(22)
+#define  YT921X_VLAN_CTRLa_PRIO_EN		BIT(21)
+#define  YT921X_VLAN_CTRLa_PRIO_M		GENMASK(20, 18)
+#define   YT921X_VLAN_CTRLa_PRIO(x)			FIELD_PREP(YT921X_VLAN_CTRLa_PRIO_M, (x))
+#define  YT921X_VLAN_CTRLa_PORTS_M		GENMASK(17, 7)
+#define   YT921X_VLAN_CTRLa_PORTS(x)			FIELD_PREP(YT921X_VLAN_CTRLa_PORTS_M, (x))
+#define  YT921X_VLAN_CTRLa_PORTn(port)		BIT((port) + 7)
+#define  YT921X_VLAN_CTRLa_BYPASS_1X_AC		BIT(6)
+#define  YT921X_VLAN_CTRLa_METER_EN		BIT(5)
+#define  YT921X_VLAN_CTRLa_METER_ID_M		GENMASK(4, 0)
 
 #define YT921X_TPID_IGRn(x)		(0x210000 + 4 * (x))	/* [0, 3] */
 #define  YT921X_TPID_IGR_TPID_M			GENMASK(15, 0)
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH net-next 2/3] net: dsa: yt921x: Add port police support
  2026-02-25  9:08 [PATCH net-next 0/3] net: dsa: yt921x: Add port police/tbf support David Yang
  2026-02-25  9:08 ` [PATCH net-next 1/3] net: dsa: yt921x: Refactor long register helpers David Yang
@ 2026-02-25  9:08 ` David Yang
  2026-02-25 14:17   ` Andrew Lunn
  2026-02-25  9:08 ` [PATCH net-next 3/3] net: dsa: yt921x: Add port qdisc tbf support David Yang
  2 siblings, 1 reply; 10+ messages in thread
From: David Yang @ 2026-02-25  9:08 UTC (permalink / raw)
  To: netdev
  Cc: David Yang, Andrew Lunn, Vladimir Oltean, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-kernel

Enable rate meter ability and support limiting the rate of incoming
traffic.

Signed-off-by: David Yang <mmyangfl@gmail.com>
---
 drivers/net/dsa/yt921x.c | 269 ++++++++++++++++++++++++++++++++++++++-
 drivers/net/dsa/yt921x.h |  55 ++++++++
 2 files changed, 323 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/yt921x.c b/drivers/net/dsa/yt921x.c
index fedc72202d1e..6fa70cea0631 100644
--- a/drivers/net/dsa/yt921x.c
+++ b/drivers/net/dsa/yt921x.c
@@ -353,6 +353,14 @@ yt921x_longreg_clear_bits(struct yt921x_priv *priv, u32 reg, const u32 *masks,
 	return yt921x_longreg_write(priv, reg, vs, span);
 }
 
+static void u32a_update_u64(u32 *arr, u64 mask, u64 val)
+{
+	arr[0] &= ~((u32)mask);
+	arr[1] &= ~((u32)(mask >> 32));
+	arr[0] |= (u32)val;
+	arr[1] |= (u32)(val >> 32);
+}
+
 static int yt921x_reg_mdio_read(void *context, u32 reg, u32 *valp)
 {
 	struct yt921x_reg_mdio *mdio = context;
@@ -1046,6 +1054,13 @@ yt921x_dsa_set_mac_eee(struct dsa_switch *ds, int port, struct ethtool_keee *e)
 	return res;
 }
 
+static int yt921x_mtu_fetch(struct yt921x_priv *priv, int port)
+{
+	struct dsa_port *dp = dsa_to_port(&priv->ds, port);
+
+	return dp->user ? READ_ONCE(dp->user->mtu) : ETH_DATA_LEN;
+}
+
 static int
 yt921x_dsa_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
 {
@@ -1077,6 +1092,218 @@ static int yt921x_dsa_port_max_mtu(struct dsa_switch *ds, int port)
 	return YT921X_FRAME_SIZE_MAX - ETH_HLEN - ETH_FCS_LEN - YT921X_TAG_LEN;
 }
 
+/* v * 2^e */
+#define ldexpi(v, e) ({ \
+	__auto_type _v = (v); \
+	__auto_type _e = (e); \
+	_e >= 0 ? _v << _e : _v >> -_e; \
+})
+
+/* slot (ns) * rate (/s) / 10^9 (ns/s) = 2^C * token * 4^unit */
+#define rate2token(rate, unit, slot_ns, C) \
+	((u32)(ldexpi((slot_ns) * (rate), \
+		      -(2 * (unit) + (C) + YT921X_TOKEN_RATE_C)) / 1000000000))
+#define token2rate(token, unit, slot_ns, C) \
+	(ldexpi(1000000000 * (u64)(token), \
+		2 * (unit) + (C) + YT921X_TOKEN_RATE_C) / (slot_ns))
+
+/* burst = 2^C * token * 4^unit */
+#define burst2token(burst, unit, C) ((u32)ldexpi((burst), -(2 * (unit) + (C))))
+#define token2burst(token, unit, C) ldexpi((u64)(token), 2 * (unit) + (C))
+
+struct yt921x_meter {
+	u32 cir;
+	u32 cbs;
+	u32 ebs;
+	int unit;
+};
+
+#define YT921X_METER_PKT_MODE		BIT(0)
+#define YT921X_METER_SINGLE_BUCKET	BIT(1)
+
+static struct yt921x_meter
+yt921x_meter_tfm(struct yt921x_priv *priv, int port, unsigned int slot_ns,
+		 u64 rate, u64 burst, unsigned int flags,
+		 u32 cir_max, u32 cbs_max, int unit_max)
+{
+	const int C = flags & YT921X_METER_PKT_MODE ? YT921X_TOKEN_PKT_C :
+		      YT921X_TOKEN_BYTE_C;
+	struct device *dev = to_device(priv);
+	struct yt921x_meter meter;
+	bool distorted;
+	u64 burst_est;
+	u64 burst_max;
+	u64 rate_max;
+
+	meter.unit = unit_max;
+	rate_max = token2rate(cir_max, meter.unit, slot_ns, C);
+	burst_max = token2burst(cbs_max, meter.unit, C);
+
+	/* Clamp rate and burst */
+	if (rate > rate_max || burst > burst_max) {
+		dev_warn(dev,
+			 "rate %llu, burst %llu too high, max is %llu, %llu, clamping...\n",
+			 rate, burst, rate_max, burst_max);
+
+		if (rate > rate_max)
+			rate = rate_max;
+		if (burst > burst_max)
+			burst = burst_max;
+
+		burst_est = slot_ns * rate / 1000000000;
+	} else {
+		u64 burst_sug;
+
+		burst_est = slot_ns * rate / 1000000000;
+		burst_sug = burst_est;
+		if (flags & YT921X_METER_PKT_MODE)
+			burst_sug++;
+		else
+			burst_sug += ETH_HLEN + yt921x_mtu_fetch(priv, port) +
+				     ETH_FCS_LEN;
+		if (burst_sug > burst)
+			dev_warn(dev,
+				 "Consider burst at least %llu to match rate %llu\n",
+				 burst_sug, rate);
+
+		/* Select unit */
+		for (; meter.unit > 0; meter.unit--) {
+			if (rate > (rate_max >> 2) || burst > (burst_max >> 2))
+				break;
+			rate_max >>= 2;
+			burst_max >>= 2;
+		}
+	}
+
+	/* Calculate information rate and bucket size */
+	distorted = false;
+	meter.cir = rate2token(rate, meter.unit, slot_ns, C);
+	if (!meter.cir) {
+		distorted = true;
+		meter.cir = 1;
+	} else if (meter.cir > cir_max) {
+		WARN_ON(1);
+		meter.cir = cir_max;
+	}
+	meter.cbs = burst2token(burst, meter.unit, C);
+	if (!meter.cbs) {
+		distorted = true;
+		meter.cbs = 1;
+	} else if (meter.cbs > cbs_max) {
+		WARN_ON(1);
+		meter.cbs = cbs_max;
+	}
+	if (distorted)
+		dev_warn(dev,
+			 "Have to increase rate %llu, burst %llu to %llu, %llu\n",
+			 rate, burst,
+			 token2rate(meter.cir, meter.unit, slot_ns, C),
+			 token2burst(meter.cbs, meter.unit, C));
+
+	/* Cut EBS */
+	meter.ebs = 0;
+	if (!(flags & YT921X_METER_SINGLE_BUCKET)) {
+		/* We don't have a chance to adjust rate when MTU is changed */
+		if (flags & YT921X_METER_PKT_MODE)
+			burst_est++;
+		else
+			burst_est += YT921X_FRAME_SIZE_MAX;
+
+		if (burst_est < burst) {
+			u32 pbs = meter.cbs;
+
+			meter.cbs = burst2token(burst_est, meter.unit, C);
+			if (!meter.cbs) {
+				meter.cbs = 1;
+			} else if (meter.cbs > cbs_max) {
+				WARN_ON(1);
+				meter.cbs = cbs_max;
+			}
+
+			if (pbs > meter.cbs)
+				meter.ebs = pbs - meter.cbs;
+		}
+	}
+
+	dev_dbg(dev,
+		"slot %u ns, rate %llu, burst %llu -> unit %d, cir %u, cbs %u, ebs %u\n",
+		slot_ns, rate, burst,
+		meter.unit, meter.cir, meter.cbs, meter.ebs);
+
+	return meter;
+}
+
+static void yt921x_dsa_port_policer_del(struct dsa_switch *ds, int port)
+{
+	struct yt921x_priv *priv = to_yt921x_priv(ds);
+	struct device *dev = to_device(priv);
+	int res;
+
+	mutex_lock(&priv->reg_lock);
+	res = yt921x_reg_write(priv, YT921X_PORTn_METER(port), 0);
+	mutex_unlock(&priv->reg_lock);
+
+	if (res)
+		dev_err(dev, "Failed to %s port %d: %i\n", "delete policer on",
+			port, res);
+}
+
+static int
+yt921x_dsa_port_policer_add(struct dsa_switch *ds, int port,
+			    const struct flow_action_police *policer)
+{
+	struct yt921x_priv *priv = to_yt921x_priv(ds);
+	struct device *dev = to_device(priv);
+	u32 ctrls[YT921X_METER_CTRL_S];
+	struct yt921x_meter meter;
+	bool pkt_mode;
+	u64 burst;
+	u64 rate;
+	u32 ctrl;
+	int res;
+
+	/* mtu defaults to unlimited but we got 2040 here, don't know why */
+	if (policer->exceed.act_id != FLOW_ACTION_DROP ||
+	    policer->notexceed.act_id != FLOW_ACTION_ACCEPT ||
+	    policer->peakrate_bytes_ps || policer->avrate ||
+	    policer->overhead) {
+		dev_err(dev, "Unsupported options specified\n");
+		return -EOPNOTSUPP;
+	}
+
+	pkt_mode = !!policer->rate_pkt_ps;
+	rate = pkt_mode ? policer->rate_pkt_ps : policer->rate_bytes_ps;
+	burst = pkt_mode ? policer->burst_pkt : policer->burst;
+	meter = yt921x_meter_tfm(priv, port, priv->meter_slot_ns, rate, burst,
+				 pkt_mode ? YT921X_METER_PKT_MODE : 0,
+				 YT921X_METER_RATE_MAX, YT921X_METER_BURST_MAX,
+				 YT921X_METER_UNIT_MAX);
+
+	mutex_lock(&priv->reg_lock);
+	ctrl = YT921X_PORT_METER_ID(port) | YT921X_PORT_METER_EN;
+	res = yt921x_reg_write(priv, YT921X_PORTn_METER(port), ctrl);
+	if (res)
+		goto end;
+
+	ctrls[0] = 0;
+	ctrls[1] = YT921X_METER_CTRLb_CIR(meter.cir);
+	ctrls[2] = YT921X_METER_CTRLc_UNIT(meter.unit) |
+		   YT921X_METER_CTRLc_DROP_R |
+		   YT921X_METER_CTRLc_TOKEN_OVERFLOW_EN |
+		   YT921X_METER_CTRLc_METER_EN;
+	u32a_update_u64(&ctrls[0], YT921X_METER_CTRLab_EBS_M,
+			YT921X_METER_CTRLab_EBS(meter.ebs));
+	u32a_update_u64(&ctrls[1], YT921X_METER_CTRLbc_CBS_M,
+			YT921X_METER_CTRLbc_CBS(meter.cbs));
+	res = yt921x_longreg_write(priv,
+				   YT921X_METERn_CTRL(port + YT921X_METER_NUM),
+				   ctrls, YT921X_METER_CTRL_S);
+end:
+	mutex_unlock(&priv->reg_lock);
+
+	return res;
+}
+
 static int
 yt921x_mirror_del(struct yt921x_priv *priv, int port, bool ingress)
 {
@@ -3046,6 +3273,7 @@ static int yt921x_chip_detect(struct yt921x_priv *priv)
 	u32 chipid;
 	u32 major;
 	u32 mode;
+	u32 val;
 	int res;
 
 	res = yt921x_reg_read(priv, YT921X_CHIP_ID, &chipid);
@@ -3080,12 +3308,27 @@ static int yt921x_chip_detect(struct yt921x_priv *priv)
 		return -ENODEV;
 	}
 
+	res = yt921x_reg_read(priv, YT921X_SYS_CLK, &val);
+	if (res)
+		return res;
+	switch (FIELD_GET(YT921X_SYS_CLK_SEL_M, val)) {
+	case 0:
+		priv->cycle_ns = info->major == YT9215_MAJOR ? 8 : 6;
+		break;
+	case YT921X_SYS_CLK_143M:
+		priv->cycle_ns = 7;
+		break;
+	default:
+		priv->cycle_ns = 8;
+	}
+
 	/* Print chipid here since we are interested in lower 16 bits */
 	dev_info(dev,
 		 "Motorcomm %s ethernet switch, chipid: 0x%x, chipmode: 0x%x 0x%x\n",
 		 info->name, chipid, mode, extmode);
 
 	priv->info = info;
+
 	return 0;
 }
 
@@ -3208,6 +3451,23 @@ static int yt921x_chip_setup_dsa(struct yt921x_priv *priv)
 	return 0;
 }
 
+static int yt921x_chip_setup_tc(struct yt921x_priv *priv)
+{
+	unsigned int op_ns;
+	u32 ctrl;
+	int res;
+
+	op_ns = 8 * priv->cycle_ns;
+
+	ctrl = max(priv->meter_slot_ns / op_ns, YT921X_METER_SLOT_MIN);
+	res = yt921x_reg_write(priv, YT921X_METER_SLOT, ctrl);
+	if (res)
+		return res;
+	priv->meter_slot_ns = ctrl * op_ns;
+
+	return 0;
+}
+
 static int __maybe_unused yt921x_chip_setup_qos(struct yt921x_priv *priv)
 {
 	u32 ctrl;
@@ -3254,7 +3514,7 @@ static int yt921x_chip_setup(struct yt921x_priv *priv)
 	u32 ctrl;
 	int res;
 
-	ctrl = YT921X_FUNC_MIB;
+	ctrl = YT921X_FUNC_MIB | YT921X_FUNC_METER;
 	res = yt921x_reg_set_bits(priv, YT921X_FUNC, ctrl);
 	if (res)
 		return res;
@@ -3263,6 +3523,10 @@ static int yt921x_chip_setup(struct yt921x_priv *priv)
 	if (res)
 		return res;
 
+	res = yt921x_chip_setup_tc(priv);
+	if (res)
+		return res;
+
 #if IS_ENABLED(CONFIG_DCB)
 	res = yt921x_chip_setup_qos(priv);
 	if (res)
@@ -3354,6 +3618,9 @@ static const struct dsa_switch_ops yt921x_dsa_switch_ops = {
 	/* mtu */
 	.port_change_mtu	= yt921x_dsa_port_change_mtu,
 	.port_max_mtu		= yt921x_dsa_port_max_mtu,
+	/* rate */
+	.port_policer_del	= yt921x_dsa_port_policer_del,
+	.port_policer_add	= yt921x_dsa_port_policer_add,
 	/* hsr */
 	.port_hsr_leave		= dsa_port_simple_hsr_leave,
 	.port_hsr_join		= dsa_port_simple_hsr_join,
diff --git a/drivers/net/dsa/yt921x.h b/drivers/net/dsa/yt921x.h
index 361470582687..b033b942b394 100644
--- a/drivers/net/dsa/yt921x.h
+++ b/drivers/net/dsa/yt921x.h
@@ -23,6 +23,7 @@
 #define  YT921X_RST_HW				BIT(31)
 #define  YT921X_RST_SW				BIT(1)
 #define YT921X_FUNC			0x80004
+#define  YT921X_FUNC_METER			BIT(4)
 #define  YT921X_FUNC_MIB			BIT(1)
 #define YT921X_CHIP_ID			0x80008
 #define  YT921X_CHIP_ID_MAJOR			GENMASK(31, 16)
@@ -239,6 +240,11 @@
 #define  YT921X_EDATA_DATA_STATUS_M		GENMASK(3, 0)
 #define   YT921X_EDATA_DATA_STATUS(x)			FIELD_PREP(YT921X_EDATA_DATA_STATUS_M, (x))
 #define   YT921X_EDATA_DATA_IDLE			YT921X_EDATA_DATA_STATUS(3)
+#define YT921X_SYS_CLK			0xe0040
+#define  YT921X_SYS_CLK_SEL_M			GENMASK(1, 0)  /* unknown: 167M */
+#define   YT9215_SYS_CLK_125M				0
+#define   YT9218_SYS_CLK_167M				0
+#define   YT921X_SYS_CLK_143M				1
 
 #define YT921X_EXT_MBUS_OP		0x6a000
 #define YT921X_INT_MBUS_OP		0xf0000
@@ -466,6 +472,43 @@ enum yt921x_app_selector {
 #define  YT921X_LAG_HASH_MAC_DA			BIT(1)
 #define  YT921X_LAG_HASH_SRC_PORT		BIT(0)
 
+#define YT921X_PORTn_RATE(port)		(0x220000 + 4 * (port))
+#define  YT921X_PORT_RATE_GAP_VALUE		GENMASK(4, 0)	/* default 20 */
+#define YT921X_METER_SLOT		0x220104
+#define  YT921X_METER_SLOT_SLOT_M		GENMASK(11, 0)
+#define YT921X_PORTn_METER(port)	(0x220108 + 4 * (port))
+#define  YT921X_PORT_METER_EN			BIT(4)
+#define  YT921X_PORT_METER_ID_M			GENMASK(3, 0)
+#define   YT921X_PORT_METER_ID(x)			FIELD_PREP(YT921X_PORT_METER_ID_M, (x))
+#define YT921X_METERn_CTRL(x)		(0x220800 + 0x10 * (x))
+#define  YT921X_METER_CTRL_S			3
+#define  YT921X_METER_CTRLc_METER_EN		BIT(14)
+#define  YT921X_METER_CTRLc_TOKEN_OVERFLOW_EN	BIT(13)	/* RFC4115: yellow use unused green bw */
+#define  YT921X_METER_CTRLc_DROP_M		GENMASK(12, 11)
+#define   YT921X_METER_CTRLc_DROP(x)			FIELD_PREP(YT921X_METER_CTRLc_DROP_M, (x))
+#define   YT921X_METER_CTRLc_DROP_GYR			YT921X_METER_CTRLc_DROP(0)
+#define   YT921X_METER_CTRLc_DROP_YR			YT921X_METER_CTRLc_DROP(1)
+#define   YT921X_METER_CTRLc_DROP_R			YT921X_METER_CTRLc_DROP(2)
+#define   YT921X_METER_CTRLc_DROP_NONE			YT921X_METER_CTRLc_DROP(3)
+#define  YT921X_METER_CTRLc_COLOR_BLIND		BIT(10)
+#define  YT921X_METER_CTRLc_UNIT_M		GENMASK(9, 7)
+#define   YT921X_METER_CTRLc_UNIT(x)			FIELD_PREP(YT921X_METER_CTRLc_UNIT_M, (x))
+#define  YT921X_METER_CTRLc_BYTE_MODE_INCLUDE_GAP	BIT(6)	/* +GAP_VALUE bytes each packet */
+#define  YT921X_METER_CTRLc_PKT_MODE		BIT(5)	/* 0: byte rate mode */
+#define  YT921X_METER_CTRLc_RFC2698		BIT(4)	/* 0: RFC4115 */
+#define  YT921X_METER_CTRLbc_CBS_M		GENMASK_ULL(35, 20)
+#define   YT921X_METER_CTRLbc_CBS(x)			FIELD_PREP(YT921X_METER_CTRLbc_CBS_M, (x))
+#define  YT921X_METER_CTRLb_CIR_M		GENMASK(19, 2)
+#define   YT921X_METER_CTRLb_CIR(x)			FIELD_PREP(YT921X_METER_CTRLb_CIR_M, (x))
+#define  YT921X_METER_CTRLab_EBS_M		GENMASK_ULL(33, 18)
+#define   YT921X_METER_CTRLab_EBS(x)			FIELD_PREP(YT921X_METER_CTRLab_EBS_M, (x))
+#define  YT921X_METER_CTRLa_EIR_M		GENMASK(17, 0)
+#define   YT921X_METER_CTRLa_EIR(x)			FIELD_PREP(YT921X_METER_CTRLa_EIR_M, (x))
+#define YT921X_METERn_STAT_EXCESS(x)	(0x221000 + 8 * (x))
+#define YT921X_METERn_STAT_COMMITTED(x)	(0x221004 + 8 * (x))
+#define  YT921X_METER_STAT_TOKEN_M		GENMASK(30, 15)
+#define  YT921X_METER_STAT_QUEUE_M		GENMASK(14, 0)
+
 #define YT921X_PORTn_VLAN_CTRL(port)	(0x230010 + 4 * (port))
 #define  YT921X_PORT_VLAN_CTRL_SVLAN_PRIO_EN	BIT(31)
 #define  YT921X_PORT_VLAN_CTRL_CVLAN_PRIO_EN	BIT(30)
@@ -509,6 +552,16 @@ enum yt921x_fdb_entry_status {
 
 #define YT921X_MSTI_NUM		16
 
+#define YT921X_TOKEN_BYTE_C	1	/* 1 token = 2^1 byte */
+#define YT921X_TOKEN_PKT_C	-6	/* 1 token = 2^-6 packets */
+#define YT921X_TOKEN_RATE_C	-15
+/* for VLAN only, not including port meters */
+#define YT921X_METER_NUM	64
+#define YT921X_METER_SLOT_MIN	80
+#define YT921X_METER_UNIT_MAX	((1 << 3) - 1)
+#define YT921X_METER_BURST_MAX	((1 << 16) - 1)
+#define YT921X_METER_RATE_MAX	((1 << 18) - 1)
+
 #define YT921X_LAG_NUM		2
 #define YT921X_LAG_PORT_NUM	4
 
@@ -603,8 +656,10 @@ struct yt921x_priv {
 	struct dsa_switch ds;
 
 	const struct yt921x_info *info;
+	unsigned int meter_slot_ns;
 	/* cache of dsa_cpu_ports(ds) */
 	u16 cpu_ports_mask;
+	u8 cycle_ns;
 
 	/* protect the access to the switch registers */
 	struct mutex reg_lock;
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH net-next 3/3] net: dsa: yt921x: Add port qdisc tbf support
  2026-02-25  9:08 [PATCH net-next 0/3] net: dsa: yt921x: Add port police/tbf support David Yang
  2026-02-25  9:08 ` [PATCH net-next 1/3] net: dsa: yt921x: Refactor long register helpers David Yang
  2026-02-25  9:08 ` [PATCH net-next 2/3] net: dsa: yt921x: Add port police support David Yang
@ 2026-02-25  9:08 ` David Yang
  2 siblings, 0 replies; 10+ messages in thread
From: David Yang @ 2026-02-25  9:08 UTC (permalink / raw)
  To: netdev
  Cc: David Yang, Andrew Lunn, Vladimir Oltean, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-kernel

Enable port shaping and support limiting the rate of outgoing traffic.

Signed-off-by: David Yang <mmyangfl@gmail.com>
---
 drivers/net/dsa/yt921x.c | 120 +++++++++++++++++++++++++++++++++++++++
 drivers/net/dsa/yt921x.h |  67 +++++++++++++++++++++-
 2 files changed, 186 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/yt921x.c b/drivers/net/dsa/yt921x.c
index 6fa70cea0631..48b322185124 100644
--- a/drivers/net/dsa/yt921x.c
+++ b/drivers/net/dsa/yt921x.c
@@ -24,6 +24,7 @@
 #include <net/dsa.h>
 #include <net/dscp.h>
 #include <net/ieee8021q.h>
+#include <net/pkt_cls.h>
 
 #include "yt921x.h"
 
@@ -1304,6 +1305,110 @@ yt921x_dsa_port_policer_add(struct dsa_switch *ds, int port,
 	return res;
 }
 
+static int
+yt921x_tbf_validate(struct yt921x_priv *priv,
+		    const struct tc_tbf_qopt_offload *qopt, int *queuep)
+{
+	struct device *dev = to_device(priv);
+	int queue = -1;
+
+	if (qopt->parent != TC_H_ROOT) {
+		dev_err(dev, "Parent should be \"root\"\n");
+		return -EOPNOTSUPP;
+	}
+
+	switch (qopt->command) {
+	case TC_TBF_REPLACE: {
+		const struct tc_tbf_qopt_offload_replace_params *p;
+
+		p = &qopt->replace_params;
+
+		if (!p->rate.mpu) {
+			dev_info(dev, "Assuming you want mpu = 64\n");
+		} else if (p->rate.mpu != 64) {
+			dev_err(dev, "mpu other than 64 not supported\n");
+			return -EINVAL;
+		}
+	}
+	default:
+		break;
+	}
+
+	*queuep = queue;
+	return 0;
+}
+
+static int
+yt921x_dsa_port_setup_tc_tbf_port(struct dsa_switch *ds, int port,
+				  const struct tc_tbf_qopt_offload *qopt)
+{
+	struct yt921x_priv *priv = to_yt921x_priv(ds);
+	u32 ctrls[YT921X_PORT_SHAPE_CTRL_S];
+	int res;
+
+	switch (qopt->command) {
+	case TC_TBF_DESTROY:
+		ctrls[0] = 0;
+		ctrls[1] = 0;
+		break;
+	case TC_TBF_REPLACE: {
+		const struct tc_tbf_qopt_offload_replace_params *p;
+		struct yt921x_meter meter;
+		u64 burst;
+
+		p = &qopt->replace_params;
+
+		/* where is burst??? */
+		burst = priv->port_shape_slot_ns * p->rate.rate_bytes_ps /
+			1000000000 + 10000;
+		meter = yt921x_meter_tfm(priv, port, priv->port_shape_slot_ns,
+					 p->rate.rate_bytes_ps, burst,
+					 YT921X_METER_SINGLE_BUCKET,
+					 YT921X_SHAPE_RATE_MAX,
+					 YT921X_SHAPE_BURST_MAX,
+					 YT921X_SHAPE_UNIT_MAX);
+
+		ctrls[0] = YT921X_PORT_SHAPE_CTRLa_CIR(meter.cir) |
+			   YT921X_PORT_SHAPE_CTRLa_CBS(meter.cbs);
+		ctrls[1] = YT921X_PORT_SHAPE_CTRLb_UNIT(meter.unit) |
+			   YT921X_PORT_SHAPE_CTRLb_EN;
+		break;
+	}
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	mutex_lock(&priv->reg_lock);
+	res = yt921x_longreg_write(priv, YT921X_PORTn_SHAPE_CTRL(port),
+				   ctrls, YT921X_PORT_SHAPE_CTRL_S);
+	mutex_unlock(&priv->reg_lock);
+
+	return res;
+}
+
+static int
+yt921x_dsa_port_setup_tc(struct dsa_switch *ds, int port,
+			 enum tc_setup_type type, void *type_data)
+{
+	struct yt921x_priv *priv = to_yt921x_priv(ds);
+	int res;
+
+	switch (type) {
+	case TC_SETUP_QDISC_TBF: {
+		const struct tc_tbf_qopt_offload *qopt = type_data;
+		int queue;
+
+		res = yt921x_tbf_validate(priv, qopt, &queue);
+		if (res)
+			return res;
+
+		return yt921x_dsa_port_setup_tc_tbf_port(ds, port, qopt);
+	}
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
 static int
 yt921x_mirror_del(struct yt921x_priv *priv, int port, bool ingress)
 {
@@ -3465,6 +3570,20 @@ static int yt921x_chip_setup_tc(struct yt921x_priv *priv)
 		return res;
 	priv->meter_slot_ns = ctrl * op_ns;
 
+	ctrl = max(priv->port_shape_slot_ns / op_ns,
+		   YT921X_PORT_SHAPE_SLOT_MIN);
+	res = yt921x_reg_write(priv, YT921X_PORT_SHAPE_SLOT, ctrl);
+	if (res)
+		return res;
+	priv->port_shape_slot_ns = ctrl * op_ns;
+
+	ctrl = max(priv->queue_shape_slot_ns / op_ns,
+		   YT921X_QUEUE_SHAPE_SLOT_MIN);
+	res = yt921x_reg_write(priv, YT921X_QUEUE_SHAPE_SLOT, ctrl);
+	if (res)
+		return res;
+	priv->queue_shape_slot_ns = ctrl * op_ns;
+
 	return 0;
 }
 
@@ -3621,6 +3740,7 @@ static const struct dsa_switch_ops yt921x_dsa_switch_ops = {
 	/* rate */
 	.port_policer_del	= yt921x_dsa_port_policer_del,
 	.port_policer_add	= yt921x_dsa_port_policer_add,
+	.port_setup_tc		= yt921x_dsa_port_setup_tc,
 	/* hsr */
 	.port_hsr_leave		= dsa_port_simple_hsr_leave,
 	.port_hsr_join		= dsa_port_simple_hsr_join,
diff --git a/drivers/net/dsa/yt921x.h b/drivers/net/dsa/yt921x.h
index b033b942b394..e18216996ee9 100644
--- a/drivers/net/dsa/yt921x.h
+++ b/drivers/net/dsa/yt921x.h
@@ -526,6 +526,12 @@ enum yt921x_app_selector {
 #define  YT921X_PORT_VLAN_CTRL1_CVLAN_DROP_TAGGED	BIT(1)
 #define  YT921X_PORT_VLAN_CTRL1_CVLAN_DROP_UNTAGGED	BIT(0)
 
+#define YT921X_PORTn_PRIO_UCAST_QUEUE(port)	(0x300200 + 4 * (port))
+#define  YT921X_PORT_PRIOm_UCAST_QUEUE_M(m)	(7 << (3 * (m)))
+#define   YT921X_PORT_PRIOm_UCAST_QUEUE(m, x)		((x) << (3 * (m)))
+#define YT921X_PORTn_PRIO_MCAST_QUEUE(port)	(0x300280 + 4 * (port))
+#define  YT921X_PORT_PRIOm_MCAST_QUEUE_M(m)	(3 << (2 * (m)))
+#define   YT921X_PORT_PRIOm_MCAST_QUEUE(m, x)		((x) << (2 * (m)))
 #define YT921X_MIRROR			0x300300
 #define  YT921X_MIRROR_IGR_PORTS_M		GENMASK(26, 16)
 #define   YT921X_MIRROR_IGR_PORTS(x)			FIELD_PREP(YT921X_MIRROR_IGR_PORTS_M, (x))
@@ -536,6 +542,49 @@ enum yt921x_app_selector {
 #define  YT921X_MIRROR_PORT_M			GENMASK(3, 0)
 #define   YT921X_MIRROR_PORT(x)				FIELD_PREP(YT921X_MIRROR_PORT_M, (x))
 
+#define YT921X_QUEUE_SHAPE_SLOT		0x340008
+#define  YT921X_QUEUE_SHAPE_SLOT_SLOT_M		GENMASK(11, 0)
+#define YT921X_PORT_SHAPE_SLOT		0x34000c
+#define  YT921X_PORT_SHAPE_SLOT_SLOT_M		GENMASK(11, 0)
+#define YT921X_QUEUEn_SCH(x)		(0x341000 + 4 * (x))
+#define  YT921X_QUEUE_SCH_E_DWRR_M		GENMASK(27, 18)
+#define   YT921X_QUEUE_SCH_E_DWRR(x)			FIELD_PREP(YT921X_QUEUE_SCH_E_DWRR_M, (x))
+#define  YT921X_QUEUE_SCH_C_DWRR_M		GENMASK(17, 8)
+#define   YT921X_QUEUE_SCH_C_DWRR(x)			FIELD_PREP(YT921X_QUEUE_SCH_C_DWRR_M, (x))
+#define  YT921X_QUEUE_SCH_E_PRIO_M		GENMASK(7, 4)
+#define   YT921X_QUEUE_SCH_E_PRIO(x)			FIELD_PREP(YT921X_QUEUE_SCH_E_PRIO_M, (x))
+#define  YT921X_QUEUE_SCH_C_PRIO_M		GENMASK(3, 0)
+#define   YT921X_QUEUE_SCH_C_PRIO(x)			FIELD_PREP(YT921X_QUEUE_SCH_C_PRIO_M, (x))
+#define YT921X_C_DWRRn(x)		(0x342000 + 4 * (x))
+#define YT921X_E_DWRRn(x)		(0x343000 + 4 * (x))
+#define  YT921X_DWRR_PKT_MODE			BIT(0)	/* 0: byte rate mode */
+#define YT921X_QUEUEn_SHAPE_CTRL(x)	(0x34c000 + 0x10 * (x))
+#define  YT921X_QUEUE_SHAPE_CTRL_S		3
+#define  YT921X_QUEUE_SHAPE_CTRLc_TOKEN_OVERFLOW_EN	BIT(6)
+#define  YT921X_QUEUE_SHAPE_CTRLc_E_EN		BIT(5)
+#define  YT921X_QUEUE_SHAPE_CTRLc_C_EN		BIT(4)
+#define  YT921X_QUEUE_SHAPE_CTRLc_PKT_MODE	BIT(3)	/* 0: byte rate mode */
+#define  YT921X_QUEUE_SHAPE_CTRLc_UNIT_M	GENMASK(2, 0)
+#define   YT921X_QUEUE_SHAPE_CTRLc_UNIT(x)		FIELD_PREP(YT921X_QUEUE_SHAPE_CTRLc_UNIT_M, (x))
+#define  YT921X_QUEUE_SHAPE_CTRLb_EBS_M		GENMASK(31, 18)
+#define   YT921X_QUEUE_SHAPE_CTRLb_EBS(x)		FIELD_PREP(YT921X_QUEUE_SHAPE_CTRLb_EBS_M, (x))
+#define  YT921X_QUEUE_SHAPE_CTRLb_EIR_M		GENMASK(17, 0)
+#define   YT921X_QUEUE_SHAPE_CTRLb_EIR(x)		FIELD_PREP(YT921X_QUEUE_SHAPE_CTRLb_EIR_M, (x))
+#define  YT921X_QUEUE_SHAPE_CTRLa_CBS_M		GENMASK(31, 18)
+#define   YT921X_QUEUE_SHAPE_CTRLa_CBS(x)		FIELD_PREP(YT921X_QUEUE_SHAPE_CTRLa_CBS_M, (x))
+#define  YT921X_QUEUE_SHAPE_CTRLa_CIR_M		GENMASK(17, 0)
+#define   YT921X_QUEUE_SHAPE_CTRLa_CIR(x)		FIELD_PREP(YT921X_QUEUE_SHAPE_CTRLa_CIR_M, (x))
+#define YT921X_PORTn_SHAPE_CTRL(port)	(0x354000 + 8 * (port))
+#define  YT921X_PORT_SHAPE_CTRL_S		2
+#define  YT921X_PORT_SHAPE_CTRLb_EN		BIT(4)
+#define  YT921X_PORT_SHAPE_CTRLb_PKT_MODE	BIT(3)	/* 0: byte rate mode */
+#define  YT921X_PORT_SHAPE_CTRLb_UNIT_M		GENMASK(2, 0)
+#define   YT921X_PORT_SHAPE_CTRLb_UNIT(x)		FIELD_PREP(YT921X_PORT_SHAPE_CTRLb_UNIT_M, (x))
+#define  YT921X_PORT_SHAPE_CTRLa_CBS_M		GENMASK(31, 18)
+#define   YT921X_PORT_SHAPE_CTRLa_CBS(x)		FIELD_PREP(YT921X_PORT_SHAPE_CTRLa_CBS_M, (x))
+#define  YT921X_PORT_SHAPE_CTRLa_CIR_M		GENMASK(17, 0)
+#define   YT921X_PORT_SHAPE_CTRLa_CIR(x)		FIELD_PREP(YT921X_PORT_SHAPE_CTRLa_CIR_M, (x))
+
 #define YT921X_EDATA_EXTMODE	0xfb
 #define YT921X_EDATA_LEN	0x100
 
@@ -561,6 +610,11 @@ enum yt921x_fdb_entry_status {
 #define YT921X_METER_UNIT_MAX	((1 << 3) - 1)
 #define YT921X_METER_BURST_MAX	((1 << 16) - 1)
 #define YT921X_METER_RATE_MAX	((1 << 18) - 1)
+#define YT921X_PORT_SHAPE_SLOT_MIN	80
+#define YT921X_QUEUE_SHAPE_SLOT_MIN	132
+#define YT921X_SHAPE_UNIT_MAX	((1 << 3) - 1)
+#define YT921X_SHAPE_BURST_MAX	((1 << 14) - 1)
+#define YT921X_SHAPE_RATE_MAX	((1 << 18) - 1)
 
 #define YT921X_LAG_NUM		2
 #define YT921X_LAG_PORT_NUM	4
@@ -578,7 +632,16 @@ enum yt921x_fdb_entry_status {
 #define YT921X_TAG_LEN	8
 
 /* 8 internal + 2 external + 1 mcu */
-#define YT921X_PORT_NUM			11
+#define YT921X_PORT_NUM		11
+#define YT921X_UCAST_QUEUE_NUM	8
+#define YT921X_MCAST_QUEUE_NUM	4
+#define YT921X_PORT_QUEUE_NUM \
+	(YT921X_UCAST_QUEUE_NUM + YT921X_MCAST_QUEUE_NUM)
+#define YT921X_UCAST_QUEUE_ID(port, queue) \
+	(YT921X_UCAST_QUEUE_NUM * (port) + (queue))
+#define YT921X_MCAST_QUEUE_ID(port, queue) \
+	(YT921X_UCAST_QUEUE_NUM * YT921X_PORT_NUM + \
+	 YT921X_MCAST_QUEUE_NUM * (port) + (queue))
 
 #define yt921x_port_is_internal(port) ((port) < 8)
 #define yt921x_port_is_external(port) (8 <= (port) && (port) < 9)
@@ -657,6 +720,8 @@ struct yt921x_priv {
 
 	const struct yt921x_info *info;
 	unsigned int meter_slot_ns;
+	unsigned int port_shape_slot_ns;
+	unsigned int queue_shape_slot_ns;
 	/* cache of dsa_cpu_ports(ds) */
 	u16 cpu_ports_mask;
 	u8 cycle_ns;
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next 1/3] net: dsa: yt921x: Refactor long register helpers
  2026-02-25  9:08 ` [PATCH net-next 1/3] net: dsa: yt921x: Refactor long register helpers David Yang
@ 2026-02-25 13:52   ` Andrew Lunn
  2026-02-25 14:12     ` David Yang
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Lunn @ 2026-02-25 13:52 UTC (permalink / raw)
  To: David Yang
  Cc: netdev, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, linux-kernel

> -/* Some registers, like VLANn_CTRL, should always be written in 64-bit, even if
> - * you are to write only the lower / upper 32 bits.
> +/* Some registers, like VLANn_CTRL, should always be written in chunks, even if
> + * you only want to write parts of 32 bits.
>   *
> - * There is no such restriction for reading, but we still provide 64-bit read
> - * wrappers so that we always handle u64 values.
> + * There is no such restriction for reading, but we still provide multi-chunk
> + * read wrappers so that we can handle them consistently.
>   */
>  
> -static int yt921x_reg64_read(struct yt921x_priv *priv, u32 reg, u64 *valp)
> +static int
> +yt921x_longreg_read(struct yt921x_priv *priv, u32 reg, u32 *vals,
> +		    unsigned int span)

Just looking at this, it is not obvious what units span is. Many bulk
read operations takes bytes, even if the underlying code works in
words. I would probably rename span to something like num_u32.

I would also keep yt921x_reg64_read, yt921x_reg64_write,
yt921x_reg64_update_bits() and just makes them wrappers which call
yt921x_longreg_read(). 

> -static int
> -yt921x_vlan_del(struct yt921x_priv *priv, int port, u16 vid)
> +static int yt921x_vlan_del(struct yt921x_priv *priv, int port, u16 vid)
>  {
> -	u64 mask64;
> +	u32 masks[YT921X_VLAN_CTRL_S];
>  
> -	mask64 = YT921X_VLAN_CTRL_PORTS(port) |
> -		 YT921X_VLAN_CTRL_UNTAG_PORTn(port);
> +	masks[0] = YT921X_VLAN_CTRLa_PORTn(port);
> +	masks[1] = YT921X_VLAN_CTRLb_UNTAG_PORTn(port);
>  
> -	return yt921x_reg64_clear_bits(priv, YT921X_VLANn_CTRL(vid), mask64);
> +	return yt921x_longreg_clear_bits(priv, YT921X_VLANn_CTRL(vid), masks,
> +					 YT921X_VLAN_CTRL_S);

By keeping the names, you don't need changes like this. And
yt921x_reg64_clear_bits() is much more obvious than
yt921x_longreg_clear_bits(). And the patch will be much smaller.

	Andrew

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next 1/3] net: dsa: yt921x: Refactor long register helpers
  2026-02-25 13:52   ` Andrew Lunn
@ 2026-02-25 14:12     ` David Yang
  2026-02-25 14:21       ` Andrew Lunn
  0 siblings, 1 reply; 10+ messages in thread
From: David Yang @ 2026-02-25 14:12 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, linux-kernel

On Wed, Feb 25, 2026 at 9:52 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > -/* Some registers, like VLANn_CTRL, should always be written in 64-bit, even if
> > - * you are to write only the lower / upper 32 bits.
> > +/* Some registers, like VLANn_CTRL, should always be written in chunks, even if
> > + * you only want to write parts of 32 bits.
> >   *
> > - * There is no such restriction for reading, but we still provide 64-bit read
> > - * wrappers so that we always handle u64 values.
> > + * There is no such restriction for reading, but we still provide multi-chunk
> > + * read wrappers so that we can handle them consistently.
> >   */
> >
> > -static int yt921x_reg64_read(struct yt921x_priv *priv, u32 reg, u64 *valp)
> > +static int
> > +yt921x_longreg_read(struct yt921x_priv *priv, u32 reg, u32 *vals,
> > +                 unsigned int span)
>
> Just looking at this, it is not obvious what units span is. Many bulk
> read operations takes bytes, even if the underlying code works in
> words. I would probably rename span to something like num_u32.
>
> I would also keep yt921x_reg64_read, yt921x_reg64_write,
> yt921x_reg64_update_bits() and just makes them wrappers which call
> yt921x_longreg_read().
>

I don't think keeping reg64_* would make it better. There will be more
96-bit regs as subsequent series progressing. It would be more messy
to have three types of regs: u32, u64, and u32[].

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next 2/3] net: dsa: yt921x: Add port police support
  2026-02-25  9:08 ` [PATCH net-next 2/3] net: dsa: yt921x: Add port police support David Yang
@ 2026-02-25 14:17   ` Andrew Lunn
  2026-02-25 14:47     ` David Yang
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Lunn @ 2026-02-25 14:17 UTC (permalink / raw)
  To: David Yang
  Cc: netdev, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, linux-kernel

> +static void u32a_update_u64(u32 *arr, u64 mask, u64 val)
> +{
> +	arr[0] &= ~((u32)mask);
> +	arr[1] &= ~((u32)(mask >> 32));
> +	arr[0] |= (u32)val;
> +	arr[1] |= (u32)(val >> 32);
> +}

The name of this function is not helping me understand what it
actually does.

> +/* v * 2^e */
> +#define ldexpi(v, e) ({ \
> +	__auto_type _v = (v); \
> +	__auto_type _e = (e); \
> +	_e >= 0 ? _v << _e : _v >> -_e; \
> +})
> +
> +/* slot (ns) * rate (/s) / 10^9 (ns/s) = 2^C * token * 4^unit */
> +#define rate2token(rate, unit, slot_ns, C) \
> +	((u32)(ldexpi((slot_ns) * (rate), \
> +		      -(2 * (unit) + (C) + YT921X_TOKEN_RATE_C)) / 1000000000))
> +#define token2rate(token, unit, slot_ns, C) \
> +	(ldexpi(1000000000 * (u64)(token), \
> +		2 * (unit) + (C) + YT921X_TOKEN_RATE_C) / (slot_ns))
> +
> +/* burst = 2^C * token * 4^unit */
> +#define burst2token(burst, unit, C) ((u32)ldexpi((burst), -(2 * (unit) + (C))))
> +#define token2burst(token, unit, C) ldexpi((u64)(token), 2 * (unit) + (C))

Please avoid macros like this. Write functions. The compiler will
inline them if that makes sense. But you gain type checking, etc.

> +static struct yt921x_meter
> +yt921x_meter_tfm(struct yt921x_priv *priv, int port, unsigned int slot_ns,
> +		 u64 rate, u64 burst, unsigned int flags,
> +		 u32 cir_max, u32 cbs_max, int unit_max)
> +{
> +	const int C = flags & YT921X_METER_PKT_MODE ? YT921X_TOKEN_PKT_C :
> +		      YT921X_TOKEN_BYTE_C;
> +	struct device *dev = to_device(priv);
> +	struct yt921x_meter meter;
> +	bool distorted;
> +	u64 burst_est;
> +	u64 burst_max;
> +	u64 rate_max;
> +
> +	meter.unit = unit_max;
> +	rate_max = token2rate(cir_max, meter.unit, slot_ns, C);
> +	burst_max = token2burst(cbs_max, meter.unit, C);
> +
> +	/* Clamp rate and burst */
> +	if (rate > rate_max || burst > burst_max) {
> +		dev_warn(dev,
> +			 "rate %llu, burst %llu too high, max is %llu, %llu, clamping...\n",
> +			 rate, burst, rate_max, burst_max);

I'm not sure clamping is the correct thing to do here. -ERANGE would
make it clearer that what the user requests is not possible.

> +
> +		if (rate > rate_max)
> +			rate = rate_max;
> +		if (burst > burst_max)
> +			burst = burst_max;
> +
> +		burst_est = slot_ns * rate / 1000000000;
> +	} else {
> +		u64 burst_sug;
> +
> +		burst_est = slot_ns * rate / 1000000000;
> +		burst_sug = burst_est;
> +		if (flags & YT921X_METER_PKT_MODE)
> +			burst_sug++;
> +		else
> +			burst_sug += ETH_HLEN + yt921x_mtu_fetch(priv, port) +
> +				     ETH_FCS_LEN;
> +		if (burst_sug > burst)
> +			dev_warn(dev,
> +				 "Consider burst at least %llu to match rate %llu\n",
> +				 burst_sug, rate);
> +
> +		/* Select unit */
> +		for (; meter.unit > 0; meter.unit--) {
> +			if (rate > (rate_max >> 2) || burst > (burst_max >> 2))
> +				break;
> +			rate_max >>= 2;
> +			burst_max >>= 2;
> +		}
> +	}
> +
> +	/* Calculate information rate and bucket size */
> +	distorted = false;
> +	meter.cir = rate2token(rate, meter.unit, slot_ns, C);
> +	if (!meter.cir) {
> +		distorted = true;
> +		meter.cir = 1;
> +	} else if (meter.cir > cir_max) {
> +		WARN_ON(1);
> +		meter.cir = cir_max;

What does this WARN_ON() indicate? If this fires, you want somebody
who is debugging it to be able to understand it.

> +	}
> +	meter.cbs = burst2token(burst, meter.unit, C);
> +	if (!meter.cbs) {
> +		distorted = true;
> +		meter.cbs = 1;
> +	} else if (meter.cbs > cbs_max) {
> +		WARN_ON(1);
> +		meter.cbs = cbs_max;
> +	}
> +	if (distorted)
> +		dev_warn(dev,
> +			 "Have to increase rate %llu, burst %llu to %llu, %llu\n",
> +			 rate, burst,
> +			 token2rate(meter.cir, meter.unit, slot_ns, C),
> +			 token2burst(meter.cbs, meter.unit, C));

I see a difference between clamping, and the granularity the hardware
can do. Clamping should be an error. However, rounding to what the
hardware can actually do is expected, so i would not spam the logs
with it.

> +static int
> +yt921x_dsa_port_policer_add(struct dsa_switch *ds, int port,
> +			    const struct flow_action_police *policer)
> +{
> +	struct yt921x_priv *priv = to_yt921x_priv(ds);
> +	struct device *dev = to_device(priv);
> +	u32 ctrls[YT921X_METER_CTRL_S];
> +	struct yt921x_meter meter;
> +	bool pkt_mode;
> +	u64 burst;
> +	u64 rate;
> +	u32 ctrl;
> +	int res;
> +
> +	/* mtu defaults to unlimited but we got 2040 here, don't know why */
> +	if (policer->exceed.act_id != FLOW_ACTION_DROP ||
> +	    policer->notexceed.act_id != FLOW_ACTION_ACCEPT ||
> +	    policer->peakrate_bytes_ps || policer->avrate ||
> +	    policer->overhead) {
> +		dev_err(dev, "Unsupported options specified\n");

dev_dbg()

You might also want to extend .port_policer_add() to pass the extack
dsa_user_add_cls_matchall_police() has, so you can give a user
friendly error message.

	Andrew

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next 1/3] net: dsa: yt921x: Refactor long register helpers
  2026-02-25 14:12     ` David Yang
@ 2026-02-25 14:21       ` Andrew Lunn
  0 siblings, 0 replies; 10+ messages in thread
From: Andrew Lunn @ 2026-02-25 14:21 UTC (permalink / raw)
  To: David Yang
  Cc: netdev, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, linux-kernel

On Wed, Feb 25, 2026 at 10:12:36PM +0800, David Yang wrote:
> On Wed, Feb 25, 2026 at 9:52 PM Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > > -/* Some registers, like VLANn_CTRL, should always be written in 64-bit, even if
> > > - * you are to write only the lower / upper 32 bits.
> > > +/* Some registers, like VLANn_CTRL, should always be written in chunks, even if
> > > + * you only want to write parts of 32 bits.
> > >   *
> > > - * There is no such restriction for reading, but we still provide 64-bit read
> > > - * wrappers so that we always handle u64 values.
> > > + * There is no such restriction for reading, but we still provide multi-chunk
> > > + * read wrappers so that we can handle them consistently.
> > >   */
> > >
> > > -static int yt921x_reg64_read(struct yt921x_priv *priv, u32 reg, u64 *valp)
> > > +static int
> > > +yt921x_longreg_read(struct yt921x_priv *priv, u32 reg, u32 *vals,
> > > +                 unsigned int span)
> >
> > Just looking at this, it is not obvious what units span is. Many bulk
> > read operations takes bytes, even if the underlying code works in
> > words. I would probably rename span to something like num_u32.
> >
> > I would also keep yt921x_reg64_read, yt921x_reg64_write,
> > yt921x_reg64_update_bits() and just makes them wrappers which call
> > yt921x_longreg_read().
> >
> 
> I don't think keeping reg64_* would make it better. There will be more
> 96-bit regs as subsequent series progressing. It would be more messy
> to have three types of regs: u32, u64, and u32[].

The lower level helper takes a u32[], but your higher level is always
yt921x_reg32_read(), yt921x_reg64_read(), or yt921x_reg96_read(). That
makes it obvious what size you are reading, and keeps your patch
small.

      Andrew

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next 2/3] net: dsa: yt921x: Add port police support
  2026-02-25 14:17   ` Andrew Lunn
@ 2026-02-25 14:47     ` David Yang
  2026-02-25 15:06       ` Andrew Lunn
  0 siblings, 1 reply; 10+ messages in thread
From: David Yang @ 2026-02-25 14:47 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, linux-kernel

On Wed, Feb 25, 2026 at 10:17 PM Andrew Lunn <andrew@lunn.ch> wrote:
...
> > +/* v * 2^e */
> > +#define ldexpi(v, e) ({ \
> > +     __auto_type _v = (v); \
> > +     __auto_type _e = (e); \
> > +     _e >= 0 ? _v << _e : _v >> -_e; \
> > +})
> > +
> > +/* slot (ns) * rate (/s) / 10^9 (ns/s) = 2^C * token * 4^unit */
> > +#define rate2token(rate, unit, slot_ns, C) \
> > +     ((u32)(ldexpi((slot_ns) * (rate), \
> > +                   -(2 * (unit) + (C) + YT921X_TOKEN_RATE_C)) / 1000000000))
> > +#define token2rate(token, unit, slot_ns, C) \
> > +     (ldexpi(1000000000 * (u64)(token), \
> > +             2 * (unit) + (C) + YT921X_TOKEN_RATE_C) / (slot_ns))
> > +
> > +/* burst = 2^C * token * 4^unit */
> > +#define burst2token(burst, unit, C) ((u32)ldexpi((burst), -(2 * (unit) + (C))))
> > +#define token2burst(token, unit, C) ldexpi((u64)(token), 2 * (unit) + (C))
>
> Please avoid macros like this. Write functions. The compiler will
> inline them if that makes sense. But you gain type checking, etc.
>

ldexpi is generic over integer types and we do type conversions in the
formulas below. So I think it cannot be expressed as a function.

> > +static struct yt921x_meter
> > +yt921x_meter_tfm(struct yt921x_priv *priv, int port, unsigned int slot_ns,
> > +              u64 rate, u64 burst, unsigned int flags,
> > +              u32 cir_max, u32 cbs_max, int unit_max)
> > +{
> > +     const int C = flags & YT921X_METER_PKT_MODE ? YT921X_TOKEN_PKT_C :
> > +                   YT921X_TOKEN_BYTE_C;
> > +     struct device *dev = to_device(priv);
> > +     struct yt921x_meter meter;
> > +     bool distorted;
> > +     u64 burst_est;
> > +     u64 burst_max;
> > +     u64 rate_max;
> > +
> > +     meter.unit = unit_max;
> > +     rate_max = token2rate(cir_max, meter.unit, slot_ns, C);
> > +     burst_max = token2burst(cbs_max, meter.unit, C);
> > +
> > +     /* Clamp rate and burst */
> > +     if (rate > rate_max || burst > burst_max) {
> > +             dev_warn(dev,
> > +                      "rate %llu, burst %llu too high, max is %llu, %llu, clamping...\n",
> > +                      rate, burst, rate_max, burst_max);
>
> I'm not sure clamping is the correct thing to do here. -ERANGE would
> make it clearer that what the user requests is not possible.
>

The maxima are way larger than the physical limitations of GbE, so
they are also acceptable. Still it would be quite unusual for users to
supply such values.

> > +
> > +             if (rate > rate_max)
> > +                     rate = rate_max;
> > +             if (burst > burst_max)
> > +                     burst = burst_max;
> > +
> > +             burst_est = slot_ns * rate / 1000000000;
> > +     } else {
> > +             u64 burst_sug;
> > +
> > +             burst_est = slot_ns * rate / 1000000000;
> > +             burst_sug = burst_est;
> > +             if (flags & YT921X_METER_PKT_MODE)
> > +                     burst_sug++;
> > +             else
> > +                     burst_sug += ETH_HLEN + yt921x_mtu_fetch(priv, port) +
> > +                                  ETH_FCS_LEN;
> > +             if (burst_sug > burst)
> > +                     dev_warn(dev,
> > +                              "Consider burst at least %llu to match rate %llu\n",
> > +                              burst_sug, rate);
> > +
> > +             /* Select unit */
> > +             for (; meter.unit > 0; meter.unit--) {
> > +                     if (rate > (rate_max >> 2) || burst > (burst_max >> 2))
> > +                             break;
> > +                     rate_max >>= 2;
> > +                     burst_max >>= 2;
> > +             }
> > +     }
> > +
> > +     /* Calculate information rate and bucket size */
> > +     distorted = false;
> > +     meter.cir = rate2token(rate, meter.unit, slot_ns, C);
> > +     if (!meter.cir) {
> > +             distorted = true;
> > +             meter.cir = 1;
> > +     } else if (meter.cir > cir_max) {
> > +             WARN_ON(1);
> > +             meter.cir = cir_max;
>
> What does this WARN_ON() indicate? If this fires, you want somebody
> who is debugging it to be able to understand it.
>

It means logical error during unit selection. Surely internal error
and the debugger must be familiar with the implementation details, I
think.

...

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net-next 2/3] net: dsa: yt921x: Add port police support
  2026-02-25 14:47     ` David Yang
@ 2026-02-25 15:06       ` Andrew Lunn
  0 siblings, 0 replies; 10+ messages in thread
From: Andrew Lunn @ 2026-02-25 15:06 UTC (permalink / raw)
  To: David Yang
  Cc: netdev, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, linux-kernel

On Wed, Feb 25, 2026 at 10:47:18PM +0800, David Yang wrote:
> On Wed, Feb 25, 2026 at 10:17 PM Andrew Lunn <andrew@lunn.ch> wrote:
> ...
> > > +/* v * 2^e */
> > > +#define ldexpi(v, e) ({ \
> > > +     __auto_type _v = (v); \
> > > +     __auto_type _e = (e); \
> > > +     _e >= 0 ? _v << _e : _v >> -_e; \
> > > +})
> > > +
> > > +/* slot (ns) * rate (/s) / 10^9 (ns/s) = 2^C * token * 4^unit */
> > > +#define rate2token(rate, unit, slot_ns, C) \
> > > +     ((u32)(ldexpi((slot_ns) * (rate), \
> > > +                   -(2 * (unit) + (C) + YT921X_TOKEN_RATE_C)) / 1000000000))
> > > +#define token2rate(token, unit, slot_ns, C) \
> > > +     (ldexpi(1000000000 * (u64)(token), \
> > > +             2 * (unit) + (C) + YT921X_TOKEN_RATE_C) / (slot_ns))
> > > +
> > > +/* burst = 2^C * token * 4^unit */
> > > +#define burst2token(burst, unit, C) ((u32)ldexpi((burst), -(2 * (unit) + (C))))
> > > +#define token2burst(token, unit, C) ldexpi((u64)(token), 2 * (unit) + (C))
> >
> > Please avoid macros like this. Write functions. The compiler will
> > inline them if that makes sense. But you gain type checking, etc.
> >
> 
> ldexpi is generic over integer types and we do type conversions in the
> formulas below. So I think it cannot be expressed as a function.

Writing functions will then help makes the types clearer. Why are type
conversions needed? Making them explicit would make the code better.

> > > +static struct yt921x_meter
> > > +yt921x_meter_tfm(struct yt921x_priv *priv, int port, unsigned int slot_ns,
> > > +              u64 rate, u64 burst, unsigned int flags,
> > > +              u32 cir_max, u32 cbs_max, int unit_max)
> > > +{
> > > +     const int C = flags & YT921X_METER_PKT_MODE ? YT921X_TOKEN_PKT_C :
> > > +                   YT921X_TOKEN_BYTE_C;
> > > +     struct device *dev = to_device(priv);
> > > +     struct yt921x_meter meter;
> > > +     bool distorted;
> > > +     u64 burst_est;
> > > +     u64 burst_max;
> > > +     u64 rate_max;
> > > +
> > > +     meter.unit = unit_max;
> > > +     rate_max = token2rate(cir_max, meter.unit, slot_ns, C);
> > > +     burst_max = token2burst(cbs_max, meter.unit, C);
> > > +
> > > +     /* Clamp rate and burst */
> > > +     if (rate > rate_max || burst > burst_max) {
> > > +             dev_warn(dev,
> > > +                      "rate %llu, burst %llu too high, max is %llu, %llu, clamping...\n",
> > > +                      rate, burst, rate_max, burst_max);
> >
> > I'm not sure clamping is the correct thing to do here. -ERANGE would
> > make it clearer that what the user requests is not possible.
> >
> 
> The maxima are way larger than the physical limitations of GbE, so
> they are also acceptable. Still it would be quite unusual for users to
> supply such values.

If they are unusual, then maybe ERANGE makes more sense, the user has
probably misunderstood the API, which can be quite easy with tc :-(

> > > +     } else if (meter.cir > cir_max) {
> > > +             WARN_ON(1);
> > > +             meter.cir = cir_max;
> >
> > What does this WARN_ON() indicate? If this fires, you want somebody
> > who is debugging it to be able to understand it.
> >
> 
> It means logical error during unit selection. Surely internal error
> and the debugger must be familiar with the implementation details, I
> think.

     } else if (WARN_ON(meter.cir > cir_max)) {
             meter.cir = cir_max;

would be more normal.

      Andrew

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-02-25 15:06 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-25  9:08 [PATCH net-next 0/3] net: dsa: yt921x: Add port police/tbf support David Yang
2026-02-25  9:08 ` [PATCH net-next 1/3] net: dsa: yt921x: Refactor long register helpers David Yang
2026-02-25 13:52   ` Andrew Lunn
2026-02-25 14:12     ` David Yang
2026-02-25 14:21       ` Andrew Lunn
2026-02-25  9:08 ` [PATCH net-next 2/3] net: dsa: yt921x: Add port police support David Yang
2026-02-25 14:17   ` Andrew Lunn
2026-02-25 14:47     ` David Yang
2026-02-25 15:06       ` Andrew Lunn
2026-02-25  9:08 ` [PATCH net-next 3/3] net: dsa: yt921x: Add port qdisc tbf support David Yang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox