All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Eric Woudstra <ericwouds@gmail.com>
Cc: Taras Chornyi <taras.chornyi@plvision.eu>,
	Andrew Lunn <andrew@lunn.ch>,
	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>,
	Alexander Couzens <lynxis@fe80.eu>,
	Vladimir Oltean <olteanv@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Heiner Kallweit <hkallweit1@gmail.com>
Subject: Re: [PATCH net-next 5/5] net: phylink: provide fixed state for 1000base-X and 2500base-X
Date: Fri, 10 Jan 2025 11:14:50 +0000	[thread overview]
Message-ID: <Z4EBKhGVwlVhxAxw@shell.armlinux.org.uk> (raw)
In-Reply-To: <a54b564b-4f88-4783-9e8a-72289ce11c04@gmail.com>

On Fri, Jan 10, 2025 at 09:04:56AM +0100, Eric Woudstra wrote:
> 
> On 1/9/25 4:15 PM, Russell King (Oracle) wrote:
> > When decoding clause 22 state, if in-band is disabled and using either
> > 1000base-X or 2500base-X, rather than reporting link-down, we know the
> > speed, and we only support full duplex. Pause modes taken from XPCS.
> > 
> > This fixes a problem reported by Eric Woudstra.
> > 
> > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> > ---
> >  drivers/net/phy/phylink.c | 29 +++++++++++++++++++----------
> >  1 file changed, 19 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> 
> After changing 'if (pcs->neg_mode)' to 'if (pcs && pcs->neg_mode)' in
> patch 1/5, I have tested this patch-set and I get link up.
> 
> Tested-by: Eric Woudstra <ericwouds@gmail.com>

Thanks Eric. Much appreciate your patience with this tangent to the
issue you have - your report highlighted that there was this other
bug that needed fixing in addition to the problem you were experiencing.
I've fixed that slightly differently (as below) and I'll post a v2
shortly.

+       if (!pcs || pcs->neg_mode)
+               autoneg = pl->pcs_neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED;
+       else
+               autoneg = linkmode_test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
+                                           state->advertising);

there, since the "else" clause is the legacy case. I doubt that makes
any difference to your testing scenario, but please let me know if
you want to re-test with that before I add your t-b.

Next, we need to address your problem properly... I'll be looking at
that today.

-- 
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: Eric Woudstra <ericwouds@gmail.com>
Cc: Andrew Lunn <andrew@lunn.ch>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	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>,
	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: Re: [PATCH net-next 5/5] net: phylink: provide fixed state for 1000base-X and 2500base-X
Date: Fri, 10 Jan 2025 11:14:50 +0000	[thread overview]
Message-ID: <Z4EBKhGVwlVhxAxw@shell.armlinux.org.uk> (raw)
In-Reply-To: <a54b564b-4f88-4783-9e8a-72289ce11c04@gmail.com>

On Fri, Jan 10, 2025 at 09:04:56AM +0100, Eric Woudstra wrote:
> 
> On 1/9/25 4:15 PM, Russell King (Oracle) wrote:
> > When decoding clause 22 state, if in-band is disabled and using either
> > 1000base-X or 2500base-X, rather than reporting link-down, we know the
> > speed, and we only support full duplex. Pause modes taken from XPCS.
> > 
> > This fixes a problem reported by Eric Woudstra.
> > 
> > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> > ---
> >  drivers/net/phy/phylink.c | 29 +++++++++++++++++++----------
> >  1 file changed, 19 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> 
> After changing 'if (pcs->neg_mode)' to 'if (pcs && pcs->neg_mode)' in
> patch 1/5, I have tested this patch-set and I get link up.
> 
> Tested-by: Eric Woudstra <ericwouds@gmail.com>

Thanks Eric. Much appreciate your patience with this tangent to the
issue you have - your report highlighted that there was this other
bug that needed fixing in addition to the problem you were experiencing.
I've fixed that slightly differently (as below) and I'll post a v2
shortly.

+       if (!pcs || pcs->neg_mode)
+               autoneg = pl->pcs_neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED;
+       else
+               autoneg = linkmode_test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
+                                           state->advertising);

there, since the "else" clause is the legacy case. I doubt that makes
any difference to your testing scenario, but please let me know if
you want to re-test with that before I add your t-b.

Next, we need to address your problem properly... I'll be looking at
that today.

-- 
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-10 11:19 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-09 15:15 [PATCH net-next 0/5] net: phylink: fix PCS without autoneg Russell King (Oracle)
2025-01-09 15:15 ` Russell King (Oracle)
2025-01-09 15:15 ` [PATCH net-next 1/5] net: phylink: use pcs_neg_mode in phylink_mac_pcs_get_state() Russell King (Oracle)
2025-01-09 15:15   ` Russell King (Oracle)
2025-01-09 17:02   ` Simon Horman
2025-01-09 17:02     ` Simon Horman
2025-01-09 15:15 ` [PATCH net-next 2/5] net: phylink: pass neg_mode into .pcs_get_state() method Russell King (Oracle)
2025-01-09 15:15   ` Russell King (Oracle)
2025-01-09 15:15 ` [PATCH net-next 3/5] net: phylink: pass neg_mode into c22 state decoder Russell King (Oracle)
2025-01-09 15:15   ` Russell King (Oracle)
2025-01-09 15:15 ` [PATCH net-next 4/5] net: phylink: use neg_mode in phylink_mii_c22_pcs_decode_state() Russell King (Oracle)
2025-01-09 15:15   ` Russell King (Oracle)
2025-01-09 15:15 ` [PATCH net-next 5/5] net: phylink: provide fixed state for 1000base-X and 2500base-X Russell King (Oracle)
2025-01-09 15:15   ` Russell King (Oracle)
2025-01-10  8:04   ` Eric Woudstra
2025-01-10  8:04     ` Eric Woudstra
2025-01-10 11:14     ` Russell King (Oracle) [this message]
2025-01-10 11:14       ` Russell King (Oracle)
2025-01-13  9:21 ` [PATCH net-next v2 0/5] net: phylink: fix PCS without autoneg 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=Z4EBKhGVwlVhxAxw@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.