On Wed, Feb 25, 2026 at 10:17 PM Andrew Lunn [off-list ref] wrote:
...

quoted

+/* 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.

quoted

+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.

quoted

+
+             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.

...