Thread (22 messages) 22 messages, 7 authors, 2020-01-30

RE: [PATCH net-next 1/2] net: phy: aquantia: add rate_adaptation indication

From: Madalin Bucur (OSS) <hidden>
Date: 2020-01-29 14:36:41

-----Original Message-----
From: Andrew Lunn <andrew@lunn.ch>
Sent: Wednesday, January 29, 2020 3:45 PM
To: Madalin Bucur (OSS) <redacted>
Cc: Vladimir Oltean <olteanv@gmail.com>; Florian Fainelli
[off-list ref]; davem@davemloft.net; hkallweit1@gmail.com;
netdev@vger.kernel.org; ykaukab@suse.de; Russell King - ARM Linux admin
[off-list ref]
Subject: Re: [PATCH net-next 1/2] net: phy: aquantia: add rate_adaptation
indication
quoted
I know it is set because someone does put some work in designing a
system,
quoted
in provisioning a correct firmware image.
Hi Madalin

That is one of the things i don't like about the aquantia PHY. It can
have all sorts of magic in its firmware, but the firmware is specific
to the board. Is this phydev->rate_adaptation going to be correct for
any other board using an Aquantia PHY?
It's a vendor policy I cannot comment upon. Probably it gives them more
visibility / control over the use-cases of their products and also
leaves less room for configuration issues on a device that's not that
simple. As far as I can tell from the public information available, the
Aquantia PHYs we have support for in the kernel have this capability.
Whether it can be disabled through various means, I cannot provide any
guarantees. I'm not aware of any way to determine if the rate adaptation
is functional or not, Vladimir seems to have some information on that.

Nor do I need to know, my approach here is "non nocere", if there is a
chance for the PHY to link with the partner at a different speed than the
one supported on the system interface, do not prevent it by deleting those
modes, as the dpaa_eth driver does now. Whether that link will be successful
or not depends upon many variables and only some are related to rate adaptation.

While it would be perfect to have total control over those variables, to check
each and every bit that could have a value conflicting with the intended use
case, this would require open documentation from the PHY vendor, a large effort 
and would have a limited benefit in the end. Somehow, these devices do work
today, without exposing all the internal knobs to the user. If we invest the
above said large effort, we'll have them still working, but we'll have users
puzzled at a myriad of options at their fingertips. Then we'd need them to spend
some time with the board schematic in hand, to make sure they feed in the correct
values for everything. We're at a certain point on the curve between good
usability and full features, I'm not saying it's the best position, that it
cannot be improved upon, just that we need to have a balanced approach and some
priorities.
I think at least you need to poke around in the Aquantia PHY registers
and ensure this feature is actually enabled before setting this bit to
true.

	Andrew
That would change this bit from "rate adaptation capable" (my intent) to
"rate adaptation functional" (outside my scope). In anyone cares to add the
latter I'm not against it but I can settle with the former.

Regards,
Madalin
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help