Re: [PATCH net-next v2 3/4] net: dsa: yt921x: Add port police support

Andrew Lunn
2026-04-03T00:41:43Z

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

Please use the macros upper_32_bits() and lower_32_bits().

> + update_ctrls_unaligned(&ctrls[0], YT921X_METER_CTRLab_EBS_M,
> + YT921X_METER_CTRLab_EBS(meter.ebs));
> + update_ctrls_unaligned(&ctrls[1], YT921X_METER_CTRLbc_CBS_M,
> + YT921X_METER_CTRLbc_CBS(meter.cbs));

That looks odd. The first call to update_ctrls_unaligned() writes to
ctrls[0] and ctrl[1]. The second call ORs into ctrls[1] and writes
to ctrls[2]?

When you look at the masks:

> +#define YT921X_METER_CTRLbc_CBS_M GENMASK_ULL(35, 20)
> +#define YT921X_METER_CTRLab_EBS_M GENMASK_ULL(33, 18)

They cross the 32 bit boundary. I think it would be cleaner to deal
with these using GENMASK_U128 and construct the value in a u128. Pass
the u128 to a reg96_write() and let it discard the upper 32 bits?

Andrew