RE: [PATCH v3 net-next 12/13] net: enetc: add preliminary support for i.MX95 ENETC PF

[Claudiu Manoil]

Hi, Wei,

> -----Original Message-----
> From: Wei Fang <wei.fang@xxxxxxx>
> Sent: Thursday, October 17, 2024 10:47 AM
[...]
> Subject: [PATCH v3 net-next 12/13] net: enetc: add preliminary support for
> i.MX95 ENETC PF
> 
[...]
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c
> b/drivers/net/ethernet/freescale/enetc/enetc.c
> index bccbeb1f355c..927beccffa6b 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
> @@ -2,6 +2,7 @@
>  /* Copyright 2017-2019 NXP */
> 
>  #include "enetc.h"
> +#include <linux/clk.h>
>  #include <linux/bpf_trace.h>
>  #include <linux/tcp.h>
>  #include <linux/udp.h>
> @@ -21,7 +22,7 @@ void enetc_port_mac_wr(struct enetc_si *si, u32 reg,
> u32 val)
>  {
>  	enetc_port_wr(&si->hw, reg, val);
>  	if (si->hw_features & ENETC_SI_F_QBU)
> -		enetc_port_wr(&si->hw, reg + ENETC_PMAC_OFFSET, val);
> +		enetc_port_wr(&si->hw, reg + si->pmac_offset, val);
>  }
>  EXPORT_SYMBOL_GPL(enetc_port_mac_wr);
> 
> @@ -700,8 +701,10 @@ static void enetc_rx_dim_work(struct work_struct
> *w)
>  		net_dim_get_rx_moderation(dim->mode, dim->profile_ix);
>  	struct enetc_int_vector	*v =
>  		container_of(dim, struct enetc_int_vector, rx_dim);
> +	struct enetc_ndev_priv *priv = netdev_priv(v->rx_ring.ndev);
> +	u64 clk_freq = priv->si->clk_freq;

Not happy to access 'priv' struct here and redirect again to the 'si' struct just
to get some init time parameter value like 'clk_freq'. enetc_rx_dim_work() should
be fast and have a small footprint. Messing up caches by accessing these 2 extra
structures periodically doesn't help. Pls move 'clk_freq' to 'priv' to get rid of one
indirection at least (I don't have a better idea now).

> 
> -	v->rx_ictt = enetc_usecs_to_cycles(moder.usec);
> +	v->rx_ictt = enetc_usecs_to_cycles(moder.usec, clk_freq);
>  	dim->state = DIM_START_MEASURE;
>  }
> 
> @@ -1721,14 +1724,25 @@ void enetc_get_si_caps(struct enetc_si *si)
>  	struct enetc_hw *hw = &si->hw;
>  	u32 val;
> 
> +	if (is_enetc_rev1(si))
> +		si->clk_freq = ENETC_CLK;
> +	else
> +		si->clk_freq = ENETC_CLK_333M;
> +

[...]

> @@ -2079,10 +2096,11 @@ void enetc_init_si_rings_params(struct
> enetc_ndev_priv *priv)
>  	 * TODO: Make # of TX rings run-time configurable
>  	 */
>  	priv->num_rx_rings = min_t(int, cpus, si->num_rx_rings);
> +	priv->num_rx_rings = min_t(int, cpus, si->num_rx_rings);

Duplicated statement.
[...]