All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] net: phy: phylink: fix SFP interface autodetection
@ 2018-09-17 14:19 Baruch Siach
  2018-09-17 15:12 ` Russell King - ARM Linux
  0 siblings, 1 reply; 5+ messages in thread
From: Baruch Siach @ 2018-09-17 14:19 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli
  Cc: netdev, Russell King, Antoine Tenart, Gregory CLEMENT, Baruch Siach

When the switching to the SFP detected link mode update the main
link_interface field as well. Otherwise, the link fails to come up when
the configured 'phy-mode' defers from the SFP detected mode.

This fixes 1GB SFP module link up on eth3 of the Macchiatobin board that
is configured in the DT to "2500base-x" phy-mode.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 drivers/net/phy/phylink.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 3ba5cf2a8a5f..3ece48c86841 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -1631,6 +1631,7 @@ static int phylink_sfp_module_insert(void *upstream,
 	if (pl->link_an_mode != MLO_AN_INBAND ||
 	    pl->link_config.interface != config.interface) {
 		pl->link_config.interface = config.interface;
+		pl->link_interface = config.interface;
 		pl->link_an_mode = MLO_AN_INBAND;
 
 		changed = true;
-- 
2.18.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH] net: phy: phylink: fix SFP interface autodetection
  2018-09-17 14:19 [PATCH] net: phy: phylink: fix SFP interface autodetection Baruch Siach
@ 2018-09-17 15:12 ` Russell King - ARM Linux
  2018-09-17 15:39   ` Baruch Siach
  0 siblings, 1 reply; 5+ messages in thread
From: Russell King - ARM Linux @ 2018-09-17 15:12 UTC (permalink / raw)
  To: Baruch Siach
  Cc: Andrew Lunn, Florian Fainelli, netdev, Antoine Tenart, Gregory CLEMENT

On Mon, Sep 17, 2018 at 05:19:57PM +0300, Baruch Siach wrote:
> When the switching to the SFP detected link mode update the main
> link_interface field as well. Otherwise, the link fails to come up when
> the configured 'phy-mode' defers from the SFP detected mode.
> 
> This fixes 1GB SFP module link up on eth3 of the Macchiatobin board that
> is configured in the DT to "2500base-x" phy-mode.

link_interface isn't supposed to track the SFP link mode.  In any case,
this is only used when a PHY is attached.  For a PHY on a SFP,
phylink_connect_phy() should be using link_config.interface and not
link_interface there.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 13.8Mbps down 630kbps up
According to speedtest.net: 13Mbps down 490kbps up

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] net: phy: phylink: fix SFP interface autodetection
  2018-09-17 15:12 ` Russell King - ARM Linux
@ 2018-09-17 15:39   ` Baruch Siach
  0 siblings, 0 replies; 5+ messages in thread
From: Baruch Siach @ 2018-09-17 15:39 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Andrew Lunn, Florian Fainelli, netdev, Antoine Tenart, Gregory CLEMENT

Hi Russell,

Russell King - ARM Linux writes:

> On Mon, Sep 17, 2018 at 05:19:57PM +0300, Baruch Siach wrote:
>> When the switching to the SFP detected link mode update the main
>> link_interface field as well. Otherwise, the link fails to come up when
>> the configured 'phy-mode' defers from the SFP detected mode.
>> 
>> This fixes 1GB SFP module link up on eth3 of the Macchiatobin board that
>> is configured in the DT to "2500base-x" phy-mode.
>
> link_interface isn't supposed to track the SFP link mode.  In any case,
> this is only used when a PHY is attached.  For a PHY on a SFP,
> phylink_connect_phy() should be using link_config.interface and not
> link_interface there.

You mean something like this?

@@ -750,7 +750,7 @@ int phylink_connect_phy(struct phylink *pl, struct phy_device *phy)
 		pl->link_config.interface = pl->link_interface;
 	}
 
-	ret = phy_attach_direct(pl->netdev, phy, 0, pl->link_interface);
+	ret = phy_attach_direct(pl->netdev, phy, 0, pl->link_config.interface);
 	if (ret)
 		return ret;
 
This fixes a similar issue on the Armada 8040 based Clearfog GT-8K where
the SFP interface phy-mode is set to "10gbase-kr". But the Macchiatobin
PHY-less interface has phy-mode set to "2500base-x", so I hit a separate
problem:

[   49.599455] WARNING: CPU: 1 PID: 1223 at drivers/net/phy/phylink.c:741 phylink_connect_phy+0x34/0xb4
[   49.608629] CPU: 1 PID: 1223 Comm: ifconfig Not tainted 4.19.0-rc4-00006-g18cf7eb4b625-dirty #952
[   49.617537] Hardware name: Marvell 8040 MACCHIATOBin (DT)
[   49.622957] pstate: 60000005 (nZCv daif -PAN -UAO)
[   49.627767] pc : phylink_connect_phy+0x34/0xb4
[   49.632229] lr : phylink_sfp_connect_phy+0xc/0x14
[   49.636950] sp : ffff00000b273830
[   49.640276] x29: ffff00000b273830 x28: ffff80007a342df0 
[   49.645610] x27: 00000000000004b0 x26: 0000000000000004 
[   49.650945] x25: ffff80007a342940 x24: ffff80007b90cc18 
[   49.656278] x23: ffff8000799b2538 x22: ffff000008bda000 
[   49.661613] x21: ffff80007a342000 x20: ffff80007bca4000 
[   49.666947] x19: ffff80007b814a80 x18: 000000000000000a 
[   49.672282] x17: 0000000000000000 x16: 0000000000000004 
[   49.677616] x15: 0000000000000416 x14: 0000000000000400 
[   49.682951] x13: 0000000000000400 x12: 0000000000000000 
[   49.688284] x11: 0000000000000000 x10: 0000000000000000 
[   49.693618] x9 : 00000000000002c6 x8 : 0000000000000000 
[   49.698952] x7 : ffff80007ff88b40 x6 : 0000000000000001 
[   49.704286] x5 : 0000000000000003 x4 : ffff80007bca4048 
[   49.709621] x3 : 0000000000000004 x2 : 0000000000000001 
[   49.714955] x1 : ffff80007bca4000 x0 : ffff80007a337400 
[   49.720289] Call trace:
[   49.722745]  phylink_connect_phy+0x34/0xb4
[   49.726857]  phylink_sfp_connect_phy+0xc/0x14
[   49.731231]  sfp_add_phy+0x48/0x50
[   49.734647]  sfp_sm_event+0x6e0/0x86c
[   49.738323]  sfp_start+0x10/0x18
[   49.741563]  sfp_upstream_start+0x28/0x3c
[   49.745588]  phylink_start+0x120/0x164
[   49.749352]  mvpp2_start_dev+0x184/0x260
[   49.753290]  mvpp2_open+0x394/0x3e8
[   49.756793]  __dev_open+0x110/0x124
[   49.760294]  __dev_change_flags+0xe8/0x190
[   49.764406]  dev_change_flags+0x20/0x5c
[   49.768258]  devinet_ioctl+0x270/0x528
[   49.772020]  inet_ioctl+0x13c/0x2e4
[   49.775524]  sock_do_ioctl+0x4c/0x1f4
[   49.779200]  sock_ioctl+0xf4/0x360
[   49.782616]  vfs_ioctl+0x24/0x40
[   49.785855]  do_vfs_ioctl+0xa4/0x7d4
[   49.789443]  ksys_ioctl+0x44/0x74
[   49.792770]  __arm64_sys_ioctl+0x18/0x24
[   49.796708]  el0_svc_common+0x8c/0xd0
[   49.800384]  el0_svc_handler+0x3c/0x6c
[   49.804148]  el0_svc+0x8/0xc
[   49.807039] ---[ end trace 0d1e0898561fce6e ]---
[   49.811729] sfp sfp-eth3: sfp_add_phy failed: -22

This hunk fixes that one:

@@ -738,7 +738,7 @@ int phylink_connect_phy(struct phylink *pl, struct phy_device *phy)
 
 	if (WARN_ON(pl->link_an_mode == MLO_AN_FIXED ||
 		    (pl->link_an_mode == MLO_AN_INBAND &&
-		     phy_interface_mode_is_8023z(pl->link_interface))))
+		     phy_interface_mode_is_8023z(pl->link_config.interface))))
 		return -EINVAL;
 
 	if (pl->phydev)

Is that the correct fix?

Thanks,
baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] net: phy: phylink: fix SFP interface autodetection
  2018-09-22 19:09 Baruch Siach
@ 2018-09-22 19:18 ` Russell King - ARM Linux
  0 siblings, 0 replies; 5+ messages in thread
From: Russell King - ARM Linux @ 2018-09-22 19:18 UTC (permalink / raw)
  To: Baruch Siach
  Cc: Andrew Lunn, Florian Fainelli, netdev, Antoine Tenart, Gregory CLEMENT

On Sat, Sep 22, 2018 at 10:09:44PM +0300, Baruch Siach wrote:
> When connecting a PHY to phylink use the detected interface. Otherwise,
> the link fails to come up when the configured 'phy-mode' differs from
> the SFP detected mode.
> 
> This fixes 1GB SFP module link up on eth3 of the Macchiatobin board that
> is configured in the DT to "2500base-x" phy-mode.

What I actually meant is that a PHY attached from a SFP interface
should use link_config.interface.  A PHY attached as not part of
a SFP interface should continue using the DT-parsed link_interface.

What I envisaged seeing is phylink_connect_phy() being renamed to
__phylink_connect_phy(), and taking the phy interface,
phylink_sfp_connect_phy() calling __phylink_connect_phy() with
link_config.interface, and a replacement phylink_connect_phy() which
called __phylink_connect_phy() with link_interface.

Sorry I wasn't explicit enough last time around.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 13.8Mbps down 630kbps up
According to speedtest.net: 13Mbps down 490kbps up

^ permalink raw reply	[flat|nested] 5+ messages in thread
* [PATCH] net: phy: phylink: fix SFP interface autodetection
@ 2018-09-22 19:09 Baruch Siach
  2018-09-22 19:18 ` Russell King - ARM Linux
  0 siblings, 1 reply; 5+ messages in thread
From: Baruch Siach @ 2018-09-22 19:09 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli
  Cc: netdev, Russell King, Antoine Tenart, Gregory CLEMENT, Baruch Siach

When connecting a PHY to phylink use the detected interface. Otherwise,
the link fails to come up when the configured 'phy-mode' differs from
the SFP detected mode.

This fixes 1GB SFP module link up on eth3 of the Macchiatobin board that
is configured in the DT to "2500base-x" phy-mode.

Fixes: 9525ae83959b6 ("phylink: add phylink infrastructure")
Suggested-by: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 drivers/net/phy/phylink.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 3ba5cf2a8a5f..aa745decc81d 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -738,7 +738,7 @@ int phylink_connect_phy(struct phylink *pl, struct phy_device *phy)
 
 	if (WARN_ON(pl->link_an_mode == MLO_AN_FIXED ||
 		    (pl->link_an_mode == MLO_AN_INBAND &&
-		     phy_interface_mode_is_8023z(pl->link_interface))))
+		     phy_interface_mode_is_8023z(pl->link_config.interface))))
 		return -EINVAL;
 
 	if (pl->phydev)
@@ -750,7 +750,7 @@ int phylink_connect_phy(struct phylink *pl, struct phy_device *phy)
 		pl->link_config.interface = pl->link_interface;
 	}
 
-	ret = phy_attach_direct(pl->netdev, phy, 0, pl->link_interface);
+	ret = phy_attach_direct(pl->netdev, phy, 0, pl->link_config.interface);
 	if (ret)
 		return ret;
 
-- 
2.19.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-09-23  1:12 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-17 14:19 [PATCH] net: phy: phylink: fix SFP interface autodetection Baruch Siach
2018-09-17 15:12 ` Russell King - ARM Linux
2018-09-17 15:39   ` Baruch Siach
2018-09-22 19:09 Baruch Siach
2018-09-22 19:18 ` Russell King - ARM Linux

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.