lkml.org 
[lkml]   [2023]   [Jan]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH net-next v5 4/4] phy: aquantia: Determine rate adaptation support from registers
    From
    On 1/5/23 09:04, Vladimir Oltean wrote:
    > On Tue, Jan 03, 2023 at 05:05:11PM -0500, Sean Anderson wrote:
    >> static int aqr107_get_rate_matching(struct phy_device *phydev,
    >> phy_interface_t iface)
    >> {
    >> - if (iface == PHY_INTERFACE_MODE_10GBASER ||
    >> - iface == PHY_INTERFACE_MODE_2500BASEX ||
    >> - iface == PHY_INTERFACE_MODE_NA)
    >> - return RATE_MATCH_PAUSE;
    >> - return RATE_MATCH_NONE;
    >> + static const struct aqr107_link_speed_cfg speed_table[] = {
    >> + {
    >> + .speed = SPEED_10,
    >> + .reg = VEND1_GLOBAL_CFG_10M,
    >> + .speed_bit = MDIO_PMA_SPEED_10,
    >> + },
    >> + {
    >> + .speed = SPEED_100,
    >> + .reg = VEND1_GLOBAL_CFG_100M,
    >> + .speed_bit = MDIO_PMA_SPEED_100,
    >> + },
    >> + {
    >> + .speed = SPEED_1000,
    >> + .reg = VEND1_GLOBAL_CFG_1G,
    >> + .speed_bit = MDIO_PMA_SPEED_1000,
    >> + },
    >> + {
    >> + .speed = SPEED_2500,
    >> + .reg = VEND1_GLOBAL_CFG_2_5G,
    >> + .speed_bit = MDIO_PMA_SPEED_2_5G,
    >> + },
    >> + {
    >> + .speed = SPEED_5000,
    >> + .reg = VEND1_GLOBAL_CFG_5G,
    >> + .speed_bit = MDIO_PMA_SPEED_5G,
    >> + },
    >> + {
    >> + .speed = SPEED_10000,
    >> + .reg = VEND1_GLOBAL_CFG_10G,
    >> + .speed_bit = MDIO_PMA_SPEED_10G,
    >> + },
    >> + };
    >> + int speed = phy_interface_max_speed(iface);
    >> + bool got_one = false;
    >> + int i;
    >> +
    >> + for (i = 0; i < ARRAY_SIZE(speed_table) &&
    >> + speed_table[i].speed <= speed; i++) {
    >> + if (!aqr107_rate_adapt_ok(phydev, speed, &speed_table[i]))
    >> + return RATE_MATCH_NONE;
    >> + got_one = true;
    >> + }
    >
    > Trying to wrap my head around the API for rate matching that was
    > originally proposed and how it applies to what we read from Aquantia
    > registers now.
    >
    > IIUC, phylink (via the PHY library) asks "what kind of rate matching is
    > supported for this SERDES protocol?". It doesn't ask "via what kind of
    > rate matching can this SERDES protocol support this particular media
    > side speed?".

    We ask "What kind of rate matching is supported for this phy interface?"

    > Your code walks through the speed_table[] of media speeds (from 10M up
    > until the max speed of the SERDES) and sees whether the PHY was
    > provisioned, for that speed, to use PAUSE rate adaptation.

    This is because we assume that if a phy supports rate matching for a phy
    interface mode, then it supports rate matching to all slower speeds that
    it otherwise supports. This seemed like a pretty reasonable assumption
    when I wrote the code, but it turns out that some firmwares don't abide
    by this. This is firstly a problem with the firmware (as it should be
    configured so that Linux can use the phy's features), but we have to be
    careful not to end up with an unsupported combination.

    > If the PHY firmware uses a combination like this: 10GBASE-R/XFI for
    > media speeds of 10G, 5G, 2.5G (rate adapted), and SGMII for 1G, 100M
    > and 10M, a call to your implementation of
    > aqr107_get_rate_matching(PHY_INTERFACE_MODE_10GBASER) would return
    > RATE_MATCH_NONE, right?

    Correct.

    > So only ETHTOOL_LINK_MODE_10000baseT_Full_BIT
    > would be advertised on the media side?


    If the host only supports 10GBASE-R and nothing else. If the host
    supports SGMII as well, we will advertise 10G, 1G, 100M, and 10M. But
    really, this is a problem with the firmware, since if the host supports
    only 10GBASE-R, then the firmware should be set up to rate adapt to all
    speeds.

    > Shouldn't you take into consideration in your aqr107_rate_adapt_ok()
    > function only the media side link speeds for which the PHY was actually
    > *configured* to use the SERDES protocol @iface?

    No, because we don't know what phy interface modes are actually going to
    be used. The phy doesn't know whether e.g. the host supports both
    10GBASE-R and SGMII or whether it only supports 10GBASE-R. With the
    current API we cannot say "I support 5G" without also saying "I support
    1G". If you don't like this, please send a patch for an API returning
    supported speeds for a phy interface mode.

    --Sean

    \
     
     \ /
      Last update: 2023-03-26 23:28    [W:4.185 / U:0.032 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site