Re: [PATCH v1 net] net: stmmac: Modify configuration method of EEE timers

[David Miller]

From: Voon Weifeng <weifeng.voon@xxxxxxxxx>
Date: Mon, 28 Sep 2020 18:02:41 +0800

> @@ -90,11 +90,12 @@ static const u32 default_msg_level = (NETIF_MSG_DRV | NETIF_MSG_PROBE |
>  				      NETIF_MSG_LINK | NETIF_MSG_IFUP |
>  				      NETIF_MSG_IFDOWN | NETIF_MSG_TIMER);
>  
> -#define STMMAC_DEFAULT_LPI_TIMER	1000
> -static int eee_timer = STMMAC_DEFAULT_LPI_TIMER;
> -module_param(eee_timer, int, 0644);
> -MODULE_PARM_DESC(eee_timer, "LPI tx expiration time in msec");
> -#define STMMAC_LPI_T(x) (jiffies + msecs_to_jiffies(x))
> +#define STMMAC_DEFAULT_LPI_TIMER	1000000
> +#define STMMAC_LPI_T(x)	(jiffies + usecs_to_jiffies(x))
> +
> +static int tw_timer = STMMAC_DEFAULT_TWT_LS;
> +module_param(tw_timer, int, 0644);
> +MODULE_PARM_DESC(tw_timer, "LPI TW timer value in msec");

This is a great example of one of the many reasons why we disallow
module parameters in networking drivers.

This is a user facing value, and now you changed the name which breaks
things for anyone who was accessing this module parameter previously.

You have to find a way to specify this value using existing kernel
infrastructure such as ethtool or devlink, and not using a module
parameter.

So please get rid of this module parameter, and add a way to set this
value portably.