On Mon, 13 Jan 2025 16:22:39 +0800 Wei Fang wrote:
> Add Receive side scaling (RSS) support for i.MX95 ENETC PF to improve the
> network performance and balance the CPU loading. In addition, since both
> ENETC v1 and ENETC v4 only support the toeplitz algorithm, so a check for
> hfunc was added.
This and previous commits are a bi hard to follow. You plumb some
stuff thru in the previous commit. In this one you reshuffle things,
again. Try to separate code movement / restructuring in one commit.
And new additions more clearly in the next.
> +static void enetc4_set_rss_key(struct enetc_hw *hw, const u8 *key)
> +{
> + int i;
> +
> + for (i = 0; i < ENETC_RSSHASH_KEY_SIZE / 4; i++)
> + enetc_port_wr(hw, ENETC4_PRSSKR(i), ((u32 *)key)[i]);
> +}
> +
> +static void enetc4_get_rss_key(struct enetc_hw *hw, u8 *key)
> +{
> + int i;
> +
> + for (i = 0; i < ENETC_RSSHASH_KEY_SIZE / 4; i++)
> + ((u32 *)key)[i] = enetc_port_rd(hw, ENETC4_PRSSKR(i));
> +}
Isn't the only difference between the chips the register offset?
Why create full ops for something this trivial?
> +static int enetc4_get_rxnfc(struct net_device *ndev, struct ethtool_rxnfc
> *rxnfc,
> + u32 *rule_locs)
> +{
> + struct enetc_ndev_priv *priv = netdev_priv(ndev);
> +
> + switch (rxnfc->cmd) {
> + case ETHTOOL_GRXRINGS:
> + rxnfc->data = priv->num_rx_rings;
> + break;
> + case ETHTOOL_GRXFH:
> + return enetc_get_rsshash(rxnfc);
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return 0;
> +}
Why add a new function instead of returning EOPNOTSUPP for new chips
in the existing one?
> @@ -712,6 +730,12 @@ static int enetc_set_rxfh(struct net_device *ndev,
> struct enetc_hw *hw = &si->hw;
> int err = 0;
>
> + if (rxfh->hfunc != ETH_RSS_HASH_NO_CHANGE &&
> + rxfh->hfunc != ETH_RSS_HASH_TOP) {
> + netdev_err(ndev, "Only toeplitz hash function is supported\n");
> + return -EOPNOTSUPP;
Should be a separate commit.
--
pw-bot: cr
