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

[Andrew Lunn]

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