Re: [PATCH v2 net-next 3/9] net: enetc: only commit preemptible TCs to hardware when MM TX is active

[Simon Horman]

On Tue, Apr 18, 2023 at 02:14:53PM +0300, Vladimir Oltean wrote:
> This was left as TODO in commit 01e23b2b3bad ("net: enetc: add support
> for preemptible traffic classes") since it's relatively complicated.
> 
> Where this makes a difference is with a configuration as follows:
> 
> ethtool --set-mm eno0 pmac-enabled on tx-enabled on verify-enabled on
> 
> Preemptible packets should only be sent when the MAC Merge TX direction
> becomes active (i.o.w. when the verification process succeeds, aka when
> the link partner confirms it can process preemptible traffic). But the
> tc qdisc with the preemptible traffic classes is offloaded completely
> asynchronously w.r.t. the MM becoming active.
> 
> The ENETC manual does suggest that this should be handled in the driver:
> "On startup, software should wait for the verification process to
> complete (MMCSR[VSTS]=011) before initiating traffic".
> 
> Adding the necessary logic allows future selftests to uphold the claim
> that an inactive or disabled MAC Merge layer should never send data
> packets through the pMAC.
> 
> This change moves enetc_set_ptcfpr() from enetc.c to enetc_ethtool.c,
> where its only caller is now - enetc_mm_commit_preemptible_tcs().
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@xxxxxxx>

Reviewed-by: Simon Horman <simon.horman@xxxxxxxxxxxx>

>  int enetc_qos_query_caps(struct net_device *ndev, void *type_data);
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c b/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c
> index deb674752851..838a92131963 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c
> @@ -991,6 +991,64 @@ static int enetc_get_mm(struct net_device *ndev, struct ethtool_mm_state *state)
>  	return 0;
>  }
>  
> +static int enetc_mm_wait_tx_active(struct enetc_hw *hw, int verify_time)
> +{
> +	int timeout = verify_time * USEC_PER_MSEC * ENETC_MM_VERIFY_RETRIES;
> +	u32 val;
> +
> +	/* This will time out after the standard value of 3 verification
> +	 * attempts. To not sleep forever, it relies on a non-zero verify_time,
> +	 * guarantee which is provided by the ethtool nlattr policy.
> +	 */
> +	return read_poll_timeout(enetc_port_rd, val,
> +				 ENETC_MMCSR_GET_VSTS(val) == 3,

nit: 3 is doing a lot of work here.
     As a follow-up, perhaps it could become part of an enum?

> +				 ENETC_MM_VERIFY_SLEEP_US, timeout,
> +				 true, hw, ENETC_MMCSR);
> +}

...