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$(echo .)co.il \
--cc=andrew@lunn$(echo .)ch \
--cc=antoine.tenart@bootlin$(echo .)com \
--cc=f.fainelli@gmail$(echo .)com \
--cc=gregory.clement@bootlin$(echo .)com \
--cc=linux@armlinux$(echo .)org.uk \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox