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 indicationquoted
I know it is set because someone does put some work in designing asystem,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