On Thu, Sep 05, 2024 at 03:02:24PM +0800, Furong Xu wrote:

 struct stmmac_fpe_cfg {
-	bool enable;				/* FPE enable */
-	bool hs_enable;				/* FPE handshake enable */
-	enum stmmac_fpe_state lp_fpe_state;	/* Link Partner FPE state */
-	enum stmmac_fpe_state lo_fpe_state;	/* Local station FPE state */
+	/* Serialize access to MAC Merge state between ethtool requests
+	 * and link state updates.
+	 */
+	spinlock_t lock;
+
 	u32 fpe_csr;				/* MAC_FPE_CTRL_STS reg cache */
+	struct timer_list verify_timer;
+	struct ethtool_mm_state state;

I don't know what to say about keeping a full-blown struct
ethtool_mm_state copy in struct stmmac_fpe_cfg.

You don't populate two of its members: tx_active and tx_min_frag_size,
and thus they would be invalid if anyone tried to access them. And two
more of its member variables are populated with driver-constant values:
max_verify_time and rx_min_frag_size.

This leaves only verify_time, verify_status, pmac_enabled, tx_enabled,
verify_enabled. Maybe it would be better to just open-code these
variables directly in struct stmmac_fpe_cfg.