All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Andrew Lunn <andrew@lunn.ch>, Heiner Kallweit <hkallweit1@gmail.com>
Cc: "Taras Chornyi" <taras.chornyi@plvision.eu>,
	"Marcin Wojtas" <marcin.s.wojtas@gmail.com>,
	"Madalin Bucur" <madalin.bucur@nxp.com>,
	"Alexander Duyck" <alexanderduyck@fb.com>,
	"Daniel Golle" <daniel@makrotopia.org>,
	"Eric Dumazet" <edumazet@google.com>,
	"Ioana Ciornei" <ioana.ciornei@nxp.com>,
	"Horatiu Vultur" <horatiu.vultur@microchip.com>,
	"Radhey Shyam Pandey" <radhey.shyam.pandey@amd.com>,
	"Florian Fainelli" <florian.fainelli@broadcom.com>,
	"Sean Anderson" <sean.anderson@seco.com>,
	"Steen Hegelund" <Steen.Hegelund@microchip.com>,
	"Lars Povlsen" <lars.povlsen@microchip.com>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Jose Abreu" <Jose.Abreu@synopsys.com>,
	"Sean Wang" <sean.wang@mediatek.com>,
	"Daniel Machon" <daniel.machon@microchip.com>,
	kernel-team@meta.com, "DENG Qingfang" <dqfext@gmail.com>,
	linux-mediatek@lists.infradead.org,
	"Matthias Brugger" <matthias.bgg@gmail.com>,
	"Michal Simek" <michal.simek@amd.com>,
	linux-arm-kernel@lists.infradead.org,
	"AngeloGioacchino Del Regno"
	<angelogioacchino.delregno@collabora.com>,
	"Arınç ÜNAL" <arinc.unal@arinc9.com>,
	netdev@vger.kernel.org,
	"Claudiu Beznea" <claudiu.beznea@tuxon.dev>,
	UNGLinuxDriver@microchip.com,
	"Andrew Lunn" <andrew+netdev@lunn.ch>,
	"Eric Woudstra" <ericwouds@gmail.com>,
	"Alexander Couzens" <lynxis@fe80.eu>,
	"Vladimir Oltean" <olteanv@gmail.com>,
	"David S. Miller" <davem@davemloft.net>
Subject: [PATCH net-next v2 0/5] net: phylink: fix PCS without autoneg
Date: Mon, 13 Jan 2025 09:22:15 +0000	[thread overview]
Message-ID: <Z4TbR93B-X8A8iHe@shell.armlinux.org.uk> (raw)

Hi,

Eric Woudstra reported that a PCS attached using 2500base-X does not
see link when phylink is using in-band mode, but autoneg is disabled,
despite there being a valid 2500base-X signal being received. We have
these settings:

	act_link_an_mode = MLO_AN_INBAND
	pcs_neg_mode = PHYLINK_PCS_NEG_INBAND_DISABLED

Eric diagnosed it to phylink_decode_c37_word() setting state->link
false because the full-duplex bit isn't set in the non-existent link
partner advertisement word (which doesn't exist because in-band
autoneg is disabled!)

The test in phylink_mii_c22_pcs_decode_state() is supposed to catch
this state, but since we converted PCS to use neg_mode, testing the
Autoneg in the local advertisement is no longer sufficient - we need
to be looking at the neg_mode, which currently isn't provided.

We need to provide this via the .pcs_get_state() method, and this
will require modifying all PCS implementations to add the extra
argument to this method.

Patch 1 uses the PCS neg_mode in phylink_mac_pcs_get_state() to correct
the now obsolute usage of the Autoneg bit in the advertisement.

Patch 2 passes neg_mode into the .pcs_get_state() method, and updates
all users.

Patch 3 adds neg_mode as an argument to the various clause 22 state
decoder functions in phylink, modifying drivers to pass the neg_mode
through.

Patch 4 makes use of phylink_mii_c22_pcs_decode_state() rather than
using the Autoneg bit in the advertising field.

Patch 5 may be required for Eric's case - it ensures that we report
the correct state for interface types that we support only one set
of modes for when autoneg is disabled.

Changes in v2:
- Add test for NULL pcs in patch 1

I haven't added Eric's t-b because I used a different fix in patch 1.

 drivers/net/dsa/b53/b53_serdes.c                   |  4 +-
 drivers/net/dsa/mt7530.c                           |  2 +-
 drivers/net/dsa/mv88e6xxx/pcs-6185.c               |  1 +
 drivers/net/dsa/mv88e6xxx/pcs-6352.c               |  1 +
 drivers/net/dsa/mv88e6xxx/pcs-639x.c               |  5 +-
 drivers/net/dsa/qca/qca8k-8xxx.c                   |  2 +-
 drivers/net/ethernet/cadence/macb_main.c           |  3 +-
 drivers/net/ethernet/freescale/fman/fman_dtsec.c   |  4 +-
 drivers/net/ethernet/marvell/mvneta.c              |  2 +-
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c    |  2 +
 .../net/ethernet/marvell/prestera/prestera_main.c  |  1 +
 drivers/net/ethernet/meta/fbnic/fbnic_phylink.c    |  2 +-
 .../net/ethernet/microchip/lan966x/lan966x_main.h  |  2 +-
 .../ethernet/microchip/lan966x/lan966x_phylink.c   |  3 +-
 .../net/ethernet/microchip/lan966x/lan966x_port.c  |  4 +-
 .../net/ethernet/microchip/sparx5/sparx5_phylink.c |  2 +-
 drivers/net/ethernet/xilinx/xilinx_axienet_main.c  |  3 +-
 drivers/net/pcs/pcs-lynx.c                         |  4 +-
 drivers/net/pcs/pcs-mtk-lynxi.c                    |  4 +-
 drivers/net/pcs/pcs-xpcs.c                         |  7 +--
 drivers/net/phy/phylink.c                          | 60 ++++++++++++++++------
 include/linux/phylink.h                            | 11 ++--
 22 files changed, 87 insertions(+), 42 deletions(-)

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


WARNING: multiple messages have this Message-ID (diff)
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Andrew Lunn <andrew@lunn.ch>, Heiner Kallweit <hkallweit1@gmail.com>
Cc: "Alexander Couzens" <lynxis@fe80.eu>,
	"Alexander Duyck" <alexanderduyck@fb.com>,
	"Andrew Lunn" <andrew+netdev@lunn.ch>,
	"AngeloGioacchino Del Regno"
	<angelogioacchino.delregno@collabora.com>,
	"Arınç ÜNAL" <arinc.unal@arinc9.com>,
	"Claudiu Beznea" <claudiu.beznea@tuxon.dev>,
	"Daniel Golle" <daniel@makrotopia.org>,
	"Daniel Machon" <daniel.machon@microchip.com>,
	"David S. Miller" <davem@davemloft.net>,
	"DENG Qingfang" <dqfext@gmail.com>,
	"Eric Dumazet" <edumazet@google.com>,
	"Eric Woudstra" <ericwouds@gmail.com>,
	"Florian Fainelli" <florian.fainelli@broadcom.com>,
	"Horatiu Vultur" <horatiu.vultur@microchip.com>,
	"Ioana Ciornei" <ioana.ciornei@nxp.com>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Jose Abreu" <Jose.Abreu@synopsys.com>,
	kernel-team@meta.com, "Lars Povlsen" <lars.povlsen@microchip.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org,
	"Madalin Bucur" <madalin.bucur@nxp.com>,
	"Marcin Wojtas" <marcin.s.wojtas@gmail.com>,
	"Matthias Brugger" <matthias.bgg@gmail.com>,
	"Michal Simek" <michal.simek@amd.com>,
	netdev@vger.kernel.org,
	"Nicolas Ferre" <nicolas.ferre@microchip.com>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Radhey Shyam Pandey" <radhey.shyam.pandey@amd.com>,
	"Sean Anderson" <sean.anderson@seco.com>,
	"Sean Wang" <sean.wang@mediatek.com>,
	"Steen Hegelund" <Steen.Hegelund@microchip.com>,
	"Taras Chornyi" <taras.chornyi@plvision.eu>,
	UNGLinuxDriver@microchip.com,
	"Vladimir Oltean" <olteanv@gmail.com>
Subject: [PATCH net-next v2 0/5] net: phylink: fix PCS without autoneg
Date: Mon, 13 Jan 2025 09:22:15 +0000	[thread overview]
Message-ID: <Z4TbR93B-X8A8iHe@shell.armlinux.org.uk> (raw)

Hi,

Eric Woudstra reported that a PCS attached using 2500base-X does not
see link when phylink is using in-band mode, but autoneg is disabled,
despite there being a valid 2500base-X signal being received. We have
these settings:

	act_link_an_mode = MLO_AN_INBAND
	pcs_neg_mode = PHYLINK_PCS_NEG_INBAND_DISABLED

Eric diagnosed it to phylink_decode_c37_word() setting state->link
false because the full-duplex bit isn't set in the non-existent link
partner advertisement word (which doesn't exist because in-band
autoneg is disabled!)

The test in phylink_mii_c22_pcs_decode_state() is supposed to catch
this state, but since we converted PCS to use neg_mode, testing the
Autoneg in the local advertisement is no longer sufficient - we need
to be looking at the neg_mode, which currently isn't provided.

We need to provide this via the .pcs_get_state() method, and this
will require modifying all PCS implementations to add the extra
argument to this method.

Patch 1 uses the PCS neg_mode in phylink_mac_pcs_get_state() to correct
the now obsolute usage of the Autoneg bit in the advertisement.

Patch 2 passes neg_mode into the .pcs_get_state() method, and updates
all users.

Patch 3 adds neg_mode as an argument to the various clause 22 state
decoder functions in phylink, modifying drivers to pass the neg_mode
through.

Patch 4 makes use of phylink_mii_c22_pcs_decode_state() rather than
using the Autoneg bit in the advertising field.

Patch 5 may be required for Eric's case - it ensures that we report
the correct state for interface types that we support only one set
of modes for when autoneg is disabled.

Changes in v2:
- Add test for NULL pcs in patch 1

I haven't added Eric's t-b because I used a different fix in patch 1.

 drivers/net/dsa/b53/b53_serdes.c                   |  4 +-
 drivers/net/dsa/mt7530.c                           |  2 +-
 drivers/net/dsa/mv88e6xxx/pcs-6185.c               |  1 +
 drivers/net/dsa/mv88e6xxx/pcs-6352.c               |  1 +
 drivers/net/dsa/mv88e6xxx/pcs-639x.c               |  5 +-
 drivers/net/dsa/qca/qca8k-8xxx.c                   |  2 +-
 drivers/net/ethernet/cadence/macb_main.c           |  3 +-
 drivers/net/ethernet/freescale/fman/fman_dtsec.c   |  4 +-
 drivers/net/ethernet/marvell/mvneta.c              |  2 +-
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c    |  2 +
 .../net/ethernet/marvell/prestera/prestera_main.c  |  1 +
 drivers/net/ethernet/meta/fbnic/fbnic_phylink.c    |  2 +-
 .../net/ethernet/microchip/lan966x/lan966x_main.h  |  2 +-
 .../ethernet/microchip/lan966x/lan966x_phylink.c   |  3 +-
 .../net/ethernet/microchip/lan966x/lan966x_port.c  |  4 +-
 .../net/ethernet/microchip/sparx5/sparx5_phylink.c |  2 +-
 drivers/net/ethernet/xilinx/xilinx_axienet_main.c  |  3 +-
 drivers/net/pcs/pcs-lynx.c                         |  4 +-
 drivers/net/pcs/pcs-mtk-lynxi.c                    |  4 +-
 drivers/net/pcs/pcs-xpcs.c                         |  7 +--
 drivers/net/phy/phylink.c                          | 60 ++++++++++++++++------
 include/linux/phylink.h                            | 11 ++--
 22 files changed, 87 insertions(+), 42 deletions(-)

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


             reply	other threads:[~2025-01-13  9:28 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-13  9:22 Russell King (Oracle) [this message]
2025-01-13  9:22 ` [PATCH net-next v2 0/5] net: phylink: fix PCS without autoneg Russell King (Oracle)
2025-01-13  9:22 ` [PATCH net-next v2 1/5] net: phylink: use pcs_neg_mode in phylink_mac_pcs_get_state() Russell King (Oracle)
2025-01-13  9:22   ` Russell King (Oracle)
2025-01-13  9:22 ` [PATCH net-next v2 2/5] net: phylink: pass neg_mode into .pcs_get_state() method Russell King (Oracle)
2025-01-13  9:22   ` Russell King (Oracle)
2025-01-13  9:22 ` [PATCH net-next v2 3/5] net: phylink: pass neg_mode into c22 state decoder Russell King (Oracle)
2025-01-13  9:22   ` Russell King (Oracle)
2025-01-13  9:22 ` [PATCH net-next v2 4/5] net: phylink: use neg_mode in phylink_mii_c22_pcs_decode_state() Russell King (Oracle)
2025-01-13  9:22   ` Russell King (Oracle)
2025-01-13  9:22 ` [PATCH net-next v2 5/5] net: phylink: provide fixed state for 1000base-X and 2500base-X Russell King (Oracle)
2025-01-13  9:22   ` Russell King (Oracle)
2025-01-14 12:57   ` Vladimir Oltean
2025-01-14 12:57     ` Vladimir Oltean
2025-01-14 13:31     ` Russell King (Oracle)
2025-01-14 13:31       ` Russell King (Oracle)
2025-01-13 16:22 ` [PATCH net-next v2 0/5] net: phylink: fix PCS without autoneg Maxime Chevallier
2025-01-13 16:22   ` Maxime Chevallier
2025-01-15 21:30 ` patchwork-bot+netdevbpf
2025-01-15 21:30   ` patchwork-bot+netdevbpf
  -- strict thread matches above, loose matches on Subject: below --
2025-01-09 15:15 [PATCH net-next " Russell King (Oracle)
2025-01-13  9:21 ` [PATCH net-next v2 " Russell King (Oracle)
2025-01-13  9:21   ` Russell King (Oracle)
2025-01-13 10:14   ` Russell King (Oracle)
2025-01-13 10:14     ` Russell King (Oracle)
2025-01-14 10:59   ` Eric Woudstra
2025-01-14 10:59     ` Eric Woudstra
2025-01-14 11:14     ` Russell King (Oracle)
2025-01-14 11:14       ` Russell King (Oracle)

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=Z4TbR93B-X8A8iHe@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=Jose.Abreu@synopsys.com \
    --cc=Steen.Hegelund@microchip.com \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=alexanderduyck@fb.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=andrew@lunn.ch \
    --cc=angelogioacchino.delregno@collabora.com \
    --cc=arinc.unal@arinc9.com \
    --cc=claudiu.beznea@tuxon.dev \
    --cc=daniel.machon@microchip.com \
    --cc=daniel@makrotopia.org \
    --cc=davem@davemloft.net \
    --cc=dqfext@gmail.com \
    --cc=edumazet@google.com \
    --cc=ericwouds@gmail.com \
    --cc=florian.fainelli@broadcom.com \
    --cc=hkallweit1@gmail.com \
    --cc=horatiu.vultur@microchip.com \
    --cc=ioana.ciornei@nxp.com \
    --cc=kernel-team@meta.com \
    --cc=kuba@kernel.org \
    --cc=lars.povlsen@microchip.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=lynxis@fe80.eu \
    --cc=madalin.bucur@nxp.com \
    --cc=marcin.s.wojtas@gmail.com \
    --cc=matthias.bgg@gmail.com \
    --cc=michal.simek@amd.com \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=pabeni@redhat.com \
    --cc=radhey.shyam.pandey@amd.com \
    --cc=sean.anderson@seco.com \
    --cc=sean.wang@mediatek.com \
    --cc=taras.chornyi@plvision.eu \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.