[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <96fe4e87-f51a-e3ce-ae72-a726b3db53be@gmail.com>
Date: Tue, 26 Mar 2019 19:24:57 +0100
From: Heiner Kallweit <hkallweit1@...il.com>
To: Andrew Lunn <andrew@...n.ch>, Michal Kubecek <mkubecek@...e.cz>
Cc: Florian Fainelli <f.fainelli@...il.com>,
David Miller <davem@...emloft.net>,
"John W. Linville" <linville@...driver.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 1/2] ethtool: add PHY Fast Link Down support
On 26.03.2019 09:24, Andrew Lunn wrote:
>>> +#define ETHTOOL_PHY_FAST_LINK_DOWN_ON 0
>>> +#define ETHTOOL_PHY_FAST_LINK_DOWN_OFF 0xff
>>> +
>>> enum phy_tunable_id {
>>> ETHTOOL_PHY_ID_UNSPEC,
>>> ETHTOOL_PHY_DOWNSHIFT,
>>> + ETHTOOL_PHY_FAST_LINK_DOWN,
>>> /*
>>> * Add your fresh new phy tunable attribute above and remember to update
>>> * phy_tunable_strings[] in net/core/ethtool.c
>>
>> It would be nice to have a short summary around here explaining how is
>> the value interpreted. While it's obvious from the second patch, one
>> shouldn't have to go into driver specific implementation to find out.
>>
>> I also wonder if the range 0-254 ms is sufficient. Would it be possible
>> that there is some other hardware which would support e.g. 300 ms?
>
> The default, as defined by the 802.3 standard, is i think 750ms.
>
Clause 40. From what I've found this applies to 1000BaseT only.
> The Marvel PHY also supports 50ms, 20ms and 0ms, if i remember
> correctly.
>
0, 10, 20, 40ms (at least for 88E1540 and 88E6390)
> One problem we have here is discovery. How does the user find out the
> values the driver supports. For a netlink socket API, extended errors
> could be used to pass back a string indicating the supported
> values. For the old ethtool, i think all we have is -EINVAL, which is
> not very helpful.
>
> Andrew
> .
>
Heiner
Powered by blists - more mailing lists