[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c2ccbdd3-cd5b-df02-7b69-7ec695ddc743@gmail.com>
Date: Mon, 25 Mar 2019 12:19:00 -0700
From: Florian Fainelli <f.fainelli@...il.com>
To: Heiner Kallweit <hkallweit1@...il.com>,
Andrew Lunn <andrew@...n.ch>,
David Miller <davem@...emloft.net>,
"John W. Linville" <linville@...driver.com>
Cc: "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next v2 2/2] net: phy: marvell: add PHY tunable fast
link down support for 88E1540
On 3/25/19 12:13 PM, Heiner Kallweit wrote:
> On 25.03.2019 19:44, Florian Fainelli wrote:
>> On 3/25/19 11:35 AM, Heiner Kallweit wrote:
>>> 1000BaseT standard requires that a link is reported as down earliest
>>> after 750ms. Several use case however require a much faster detecion
>>> of a broken link. Fast Link Down supports this by intentionally
>>> violating a the standard. This patch exposes the Fast Link Down
>>> feature of 88E1540 and 88E6390. These PHY's can be found as internal
>>> PHY's in several switches: 88E6352, 88E6240, 88E6176, 88E6172,
>>> and 88E6390(X). Fast Link Down and EEE are mutually exclusive.
>>>
>>> Signed-off-by: Heiner Kallweit <hkallweit1@...il.com>
>>
>> This looks fine, just one question though: do not you need to verify
>> that the phy_interface_t maps to a copper medium somehow?
>>
> Good question. The register we configure is used in copper mode
> only (see name: COPPER_CTRL3). So we don't cause any harm at least
> if we configure Fast Link Down when being, let's say, in a fiber mode.
> Fast Link Down is applicable for 1000BaseT only, so in theory what
> you say would apply also to an attempt to configure Fast Link Down
> when being in 100BaseT or 10GBaseT copper modes.
> Being allowed to configure a feature for 1000BaseT whilst being in
> a different mode can been seen as a feature (what I do) or as a bug.
> Most likely there is no 100% answer to this question.
>
Agree, and the reasoning about the register we are reading/writing to is
indeed not a problem, it is more about allowing the tunable to be
configured or not, and in fact, this is a policy check that can be done
above the PHY driver itself.
--
Florian
Powered by blists - more mailing lists