From: Baruch Siach <baruch@tkos.co.il>
To: Russell King - ARM Linux <linux@armlinux.org.uk>
Cc: Andrew Lunn <andrew@lunn.ch>,
Florian Fainelli <f.fainelli@gmail.com>,
netdev@vger.kernel.org,
Antoine Tenart <antoine.tenart@bootlin.com>,
Gregory CLEMENT <gregory.clement@bootlin.com>
Subject: Re: [PATCH] net: phy: phylink: fix SFP interface autodetection
Date: Mon, 17 Sep 2018 18:39:36 +0300 [thread overview]
Message-ID: <87work2kw7.fsf@tkos.co.il> (raw)
In-Reply-To: <20180917151230.GF30658@n2100.armlinux.org.uk>
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 -
next prev parent reply other threads:[~2018-09-17 21:07 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
-- strict thread matches above, loose matches on Subject: below --
2018-09-22 19:09 Baruch Siach
2018-09-22 19:18 ` Russell King - ARM Linux
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=87work2kw7.fsf@tkos.co.il \
--to=baruch@tkos.co.il \
--cc=andrew@lunn.ch \
--cc=antoine.tenart@bootlin.com \
--cc=f.fainelli@gmail.com \
--cc=gregory.clement@bootlin.com \
--cc=linux@armlinux.org.uk \
--cc=netdev@vger.kernel.org \
/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.