Date:   Wed, 29 Jan 2020 14:36:31 +0000
From:   "Madalin Bucur (OSS)" <madalin.bucur@....nxp.com>
To:     Andrew Lunn <andrew@...n.ch>,
        "Madalin Bucur (OSS)" <madalin.bucur@....nxp.com>
CC:     Vladimir Oltean <olteanv@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "hkallweit1@...il.com" <hkallweit1@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "ykaukab@...e.de" <ykaukab@...e.de>,
        Russell King - ARM Linux admin <linux@...linux.org.uk>
Subject: RE: [PATCH net-next 1/2] net: phy: aquantia: add rate_adaptation
 indication

> -----Original Message-----
> From: Andrew Lunn <andrew@...n.ch>
> Sent: Wednesday, January 29, 2020 3:45 PM
> To: Madalin Bucur (OSS) <madalin.bucur@....nxp.com>
> Cc: Vladimir Oltean <olteanv@...il.com>; Florian Fainelli
> <f.fainelli@...il.com>; davem@...emloft.net; hkallweit1@...il.com;
> netdev@...r.kernel.org; ykaukab@...e.de; Russell King - ARM Linux admin
> <linux@...linux.org.uk>
> Subject: Re: [PATCH net-next 1/2] net: phy: aquantia: add rate_adaptation
> indication
> 
> > I know it is set because someone does put some work in designing a
> system,
> > 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

Powered by blists - more mailing lists