Re: [PATCH v2 1/2] ethtool: implement Energy Detect Powerdown support via phy-tunable

From: Andrew Lunn
Date: Wed Sep 04 2019 - 15:54:05 EST


On Wed, Sep 04, 2019 at 07:23:21PM +0300, Alexandru Ardelean wrote:

Hi Alexandru

Somewhere we need a comment stating what EDPD means. Here would be a
good place.

> +#define ETHTOOL_PHY_EDPD_DFLT_TX_INTERVAL 0x7fff
> +#define ETHTOOL_PHY_EDPD_NO_TX 0x8000
> +#define ETHTOOL_PHY_EDPD_DISABLE 0

I think you are passing a u16. So why not 0xfffe and 0xffff? We also
need to make it clear what the units are for interval. This file
specifies the contract between the kernel and user space. So we need
to clearly define what we mean here. Lots of comments are better than
no comments.

Andrew