netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
To: Vladimir Oltean <olteanv@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Antoine Tenart <atenart@kernel.org>,
	Quentin Schulz <quentin.schulz@bootlin.com>,
	Michael Walle <michael@walle.cc>,
	netdev@vger.kernel.org, Heiner Kallweit <hkallweit1@gmail.com>,
	Andrew Lunn <andrew@lunn.ch>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Ioana Ciornei <ioana.ciornei@nxp.com>,
	Maxim Kochetkov <fido_max@inbox.ru>,
	Bjarni Jonasson <bjarni.jonasson@microchip.com>,
	Steen Hegelund <steen.hegelund@microchip.com>,
	UNGLinuxDriver@microchip.com
Subject: Re: [PATCH net-next 1/2] net: phylink: explicitly configure in-band autoneg for PHYs that support it
Date: Sun, 14 Feb 2021 10:35:29 +0000	[thread overview]
Message-ID: <20210214103529.GT1463@shell.armlinux.org.uk> (raw)
In-Reply-To: <20210212172341.3489046-2-olteanv@gmail.com>

On Fri, Feb 12, 2021 at 07:23:40PM +0200, Vladimir Oltean wrote:
> +	ret = phy_config_inband_aneg(phy,
> +				     (pl->cur_link_an_mode == MLO_AN_INBAND));

Please use phylink_autoneg_inband(pl->cur_link_an_mode) here.

> +	if (ret && ret != -EOPNOTSUPP) {
> +		phylink_warn(pl, "failed to configure PHY in-band autoneg: %d\n",
> +			     ret);

Please use %pe and ERR_PTR(ret) so we can get a symbolic errno value.

As mentioned in this thread, we have at least one PHY which is unable
to provide the inband signalling in any mode (BCM84881). Currently,
phylink detects this PHY on a SFP (in phylink_phy_no_inband()) and
adjusts not to use inband mode. This would need to be addressed if we
are creating an alterative way to discover whether the PHY supports
inband mode or not.

Also, there needs to be consideration of PHYs that dynamically change
their interface type, and whether they support inband signalling.
For example, a PHY may support a mode where it dynamically selects
between 10GBASE-R, 5GBASE-R, 2500BASE-X and SGMII, where the SGMII
mode may have inband signalling enabled or disabled. This is not a
theoretical case; we have a PHY like that supported in the kernel and
boards use it. What would the semantics of your new call be for a PHY
that performs this?

Should we also have a phydev->inband tristate, taking values "unknown,
enabled, disabled" which the PHY driver is required to update in their
read_status callback if they dynamically change their interface type?
(Although then phylink will need to figure out how to deal with that.)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

  parent reply	other threads:[~2021-02-14 10:37 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-12 17:23 [PATCH net-next 0/2] Let phylink manage in-band AN for the PHY Vladimir Oltean
2021-02-12 17:23 ` [PATCH net-next 1/2] net: phylink: explicitly configure in-band autoneg for PHYs that support it Vladimir Oltean
2021-02-12 22:40   ` Michael Walle
2021-02-13  0:18     ` Russell King - ARM Linux admin
2021-02-13 16:41       ` Michael Walle
2021-02-13 16:59         ` Andrew Lunn
2021-02-13 17:06         ` Russell King - ARM Linux admin
2021-02-13  0:36     ` Vladimir Oltean
2021-02-13 16:53       ` Michael Walle
2021-02-13 17:09         ` Michael Walle
2021-02-13 18:56           ` Vladimir Oltean
2021-02-13 19:57             ` Michael Walle
2021-02-13 20:12               ` Vladimir Oltean
2021-02-13 20:16               ` Russell King - ARM Linux admin
2021-02-14 10:35   ` Russell King - ARM Linux admin [this message]
2021-02-14 11:10     ` Vladimir Oltean
2021-02-14 13:18       ` Russell King - ARM Linux admin
2021-02-12 17:23 ` [PATCH net-next 2/2] net: phy: mscc: configure in-band auto-negotiation for VSC8514 Vladimir Oltean

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210214103529.GT1463@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=andrew@lunn.ch \
    --cc=atenart@kernel.org \
    --cc=bjarni.jonasson@microchip.com \
    --cc=davem@davemloft.net \
    --cc=f.fainelli@gmail.com \
    --cc=fido_max@inbox.ru \
    --cc=hkallweit1@gmail.com \
    --cc=ioana.ciornei@nxp.com \
    --cc=kuba@kernel.org \
    --cc=michael@walle.cc \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=quentin.schulz@bootlin.com \
    --cc=steen.hegelund@microchip.com \
    --subject='Re: [PATCH net-next 1/2] net: phylink: explicitly configure in-band autoneg for PHYs that support it' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).