Re: [PATCH net-next 3/3] net: atlantic: implement media detect feature via phy tunables

[Andrew Lunn <andrew@lunn.ch>]

> @@ -923,6 +923,12 @@ static int aq_ethtool_get_phy_tunable(struct net_device *ndev,
>  	struct aq_nic_s *aq_nic = netdev_priv(ndev);
>  
>  	switch (tuna->id) {
> +	case ETHTOOL_PHY_EDPD: {
> +		u16 *val = data;
> +
> +		*val = (u16)aq_nic->aq_nic_cfg.is_media_detect;
> +		break;
> +	}
>  	case ETHTOOL_PHY_DOWNSHIFT: {
>  		u8 *val = data;
>  
> @@ -943,6 +949,14 @@ static int aq_ethtool_set_phy_tunable(struct net_device *ndev,
>  	struct aq_nic_s *aq_nic = netdev_priv(ndev);
>  
>  	switch (tuna->id) {
> +	case ETHTOOL_PHY_EDPD: {
> +		const u16 *val = data;
> +
> +		/* msecs plays no role - configuration is always fixed in PHY */
> +		aq_nic->aq_nic_cfg.is_media_detect = *val ? 1 : 0;

This is the wrong usage of the API:

include/uapi/linux/ethtool.h:

* The interval units for TX wake-up are in milliseconds, since this should
 * cover a reasonable range of intervals:
 *  - from 1 millisecond, which does not sound like much of a power-saver
 *  - to ~65 seconds which is quite a lot to wait for a link to come up when
 *    plugging a cable
 */

I guess your PHY is not hard coded to 1 millisecond? Please return the
real value. And the set call should really only allow 0, or the value
the PHY is using.

    Andrew