diff mbox series

[v2] ramips: fix at803x patch again

Message ID 20210604051215.1180296-1-dqfext@gmail.com
State New
Headers show
Series [v2] ramips: fix at803x patch again | expand

Commit Message

Qingfang Deng June 4, 2021, 5:12 a.m. UTC
Commit 8222f8e1b9be overrides AR8031's PHY features, making its SFP
link status detection unavailable. To fix that, do not override it in
ramips patch.

Commit 6d4ef6792612 also moves .config_aneg to AR8035 by mistake, fix
that as well.

Reported-by: CHEN Minqiang <ptpt52@gmail.com>
Fixes: 8222f8e1b9be ("ath79: fix link mode support list on UniFi AC")
Fixes: 6d4ef6792612 ("kernel: bump 5.4 to 5.4.113")
Signed-off-by: DENG Qingfang <dqfext@gmail.com>
---
v1 -> v2: do not override .features

 .../ramips/patches-5.10/710-at803x.patch      | 13 ++++++++++--
 .../linux/ramips/patches-5.4/991-at803x.patch | 21 ++++++++++++-------
 2 files changed, 25 insertions(+), 9 deletions(-)

Comments

David Bauer June 4, 2021, 7:37 a.m. UTC | #1
Hi,

On 6/4/21 7:12 AM, DENG Qingfang wrote:
> Commit 8222f8e1b9be overrides AR8031's PHY features, making its SFP
> link status detection unavailable. To fix that, do not override it in
> ramips patch.

As the original patch still has to be backported to 21.02, I'd propose
to remove my hacks and instead properly backport c329e5af and 8f7e8762
from upstream.

This would remove the need to hack around the first hack and hopefully
less surprises when switching to the next LTS kernel.

What do your think?

Best
David

> 
> Commit 6d4ef6792612 also moves .config_aneg to AR8035 by mistake, fix
> that as well.
> 
> Reported-by: CHEN Minqiang <ptpt52@gmail.com>
> Fixes: 8222f8e1b9be ("ath79: fix link mode support list on UniFi AC")
> Fixes: 6d4ef6792612 ("kernel: bump 5.4 to 5.4.113")
> Signed-off-by: DENG Qingfang <dqfext@gmail.com>
> ---
> v1 -> v2: do not override .features
> 
>  .../ramips/patches-5.10/710-at803x.patch      | 13 ++++++++++--
>  .../linux/ramips/patches-5.4/991-at803x.patch | 21 ++++++++++++-------
>  2 files changed, 25 insertions(+), 9 deletions(-)
> 
> diff --git a/target/linux/ramips/patches-5.10/710-at803x.patch b/target/linux/ramips/patches-5.10/710-at803x.patch
> index dab62b7607..1afc266740 100644
> --- a/target/linux/ramips/patches-5.10/710-at803x.patch
> +++ b/target/linux/ramips/patches-5.10/710-at803x.patch
> @@ -9,8 +9,8 @@ Content-Transfer-Encoding: 8bit
>  
>  Signed-off-by: René van Dorst <opensource@vdorst.com>
>  ---
> - drivers/net/phy/at803x.c | 91 ++++++++++++++++++++++++++++++++++++++++
> - 1 file changed, 91 insertions(+)
> + drivers/net/phy/at803x.c | 86 +++++++++++++++++++++++++++++++++++++++-
> + 1 file changed, 85 insertions(+), 1 deletion(-)
>  
>  --- a/drivers/net/phy/at803x.c
>  +++ b/drivers/net/phy/at803x.c
> @@ -147,3 +147,12 @@ Signed-off-by: René van Dorst <opensource@vdorst.com>
>   	.flags			= PHY_POLL_CABLE_TEST,
>   	.probe			= at803x_probe,
>   	.remove			= at803x_remove,
> +@@ -1119,7 +1203,7 @@ static struct phy_driver at803x_driver[]
> + 	.get_wol		= at803x_get_wol,
> + 	.suspend		= at803x_suspend,
> + 	.resume			= at803x_resume,
> +-	.features               = PHY_GBIT_FEATURES,
> ++	/* PHY_GBIT_FEATURES */
> + 	.read_status		= at803x_read_status,
> + 	.aneg_done		= at803x_aneg_done,
> + 	.ack_interrupt		= &at803x_ack_interrupt,
> diff --git a/target/linux/ramips/patches-5.4/991-at803x.patch b/target/linux/ramips/patches-5.4/991-at803x.patch
> index 95411211b2..9148687113 100644
> --- a/target/linux/ramips/patches-5.4/991-at803x.patch
> +++ b/target/linux/ramips/patches-5.4/991-at803x.patch
> @@ -9,8 +9,8 @@ Content-Transfer-Encoding: 8bit
>  
>  Signed-off-by: René van Dorst <opensource@vdorst.com>
>  ---
> - drivers/net/phy/at803x.c | 91 ++++++++++++++++++++++++++++++++++++++++
> - 1 file changed, 91 insertions(+)
> + drivers/net/phy/at803x.c | 93 +++++++++++++++++++++++++++++++++++++++-
> + 1 file changed, 92 insertions(+), 1 deletion(-)
>  
>  --- a/drivers/net/phy/at803x.c
>  +++ b/drivers/net/phy/at803x.c
> @@ -146,11 +146,18 @@ Signed-off-by: René van Dorst <opensource@vdorst.com>
>   static struct phy_driver at803x_driver[] = {
>   {
>   	/* ATHEROS 8035 */
> -@@ -461,6 +551,7 @@ static struct phy_driver at803x_driver[]
> +@@ -485,12 +575,13 @@ static struct phy_driver at803x_driver[]
> + 	.name			= "Atheros 8031 ethernet",
> + 	.phy_id_mask		= AT803X_PHY_ID_MASK,
> + 	.probe			= at803x_probe,
> ++	.config_aneg		= at803x_config_aneg,
> + 	.config_init		= at803x_config_init,
> + 	.set_wol		= at803x_set_wol,
> + 	.get_wol		= at803x_get_wol,
>   	.suspend		= at803x_suspend,
>   	.resume			= at803x_resume,
> - 	/* PHY_GBIT_FEATURES */
> -+	.config_aneg		= at803x_config_aneg,
> +-	.features               = PHY_GBIT_FEATURES,
> ++	/* PHY_GBIT_FEATURES */
>   	.read_status		= at803x_read_status,
> - 	.ack_interrupt		= at803x_ack_interrupt,
> - 	.config_intr		= at803x_config_intr,
> + 	.aneg_done		= at803x_aneg_done,
> + 	.ack_interrupt		= &at803x_ack_interrupt,
>
Qingfang Deng June 4, 2021, 8:57 a.m. UTC | #2
Hi,

On Fri, Jun 4, 2021 at 3:37 PM David Bauer <mail@david-bauer.net> wrote:
> As the original patch still has to be backported to 21.02, I'd propose
> to remove my hacks and instead properly backport c329e5af and 8f7e8762
> from upstream.
>
> This would remove the need to hack around the first hack and hopefully
> less surprises when switching to the next LTS kernel.
>
> What do your think?

That's okay. The SFP part still needs to be tested by Minqiang and René though.

>
> Best
> David
David Bauer June 4, 2021, 4:19 p.m. UTC | #3
Hi,

On 6/4/21 10:57 AM, DENG Qingfang wrote:
> Hi,
> 
> On Fri, Jun 4, 2021 at 3:37 PM David Bauer <mail@david-bauer.net> wrote:
>> As the original patch still has to be backported to 21.02, I'd propose
>> to remove my hacks and instead properly backport c329e5af and 8f7e8762
>> from upstream.
>>
>> This would remove the need to hack around the first hack and hopefully
>> less surprises when switching to the next LTS kernel.
>>
>> What do your think?
> 
> That's okay. The SFP part still needs to be tested by Minqiang and René though.

See the commit "generic: backport at803x fixes" in my staging tree [0].

I'll test this later this weekend on my AC Lite.

[0] https://git.openwrt.org/?p=openwrt/staging/blocktrron.git;a=summary

Best
David

> 
>>
>> Best
>> David
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
>
John Thomson June 4, 2021, 10:17 p.m. UTC | #4
On Fri, 4 Jun 2021, at 16:19, David Bauer wrote:
> See the commit "generic: backport at803x fixes" in my staging tree [0].
> [0] https://git.openwrt.org/?p=openwrt/staging/blocktrron.git;a=summary

This (efbaa6c8bd4c atop master) would not compile for me with 5.4?
CONFIG_TARGET_ramips=y
CONFIG_TARGET_ramips_mt7621=y
CONFIG_TARGET_ramips_mt7621_DEVICE_mikrotik_routerboard-760igs=y
CONFIG_KERNEL_DYNAMIC_DEBUG=y

Missing functions from:
(5.5) net: phy: at803x: add device tree binding 
2f664823a47021ae029fe91272adbf0a223e477f

(5.5) net: phy: add helpers phy_(un)lock_mdio_bus 
bec170e55982c2d3b8e1beccadf16e288fe6fb5a

  CHK     include/generated/compile.h
  CC      drivers/net/phy/at803x.o
drivers/net/phy/at803x.c: In function 'at803x_probe':
drivers/net/phy/at803x.c:365:6: error: implicit declaration of function 'at803x_match_phy_id'; did you mean 'cpupid_match_pid'? [-Werror=implicit-function-declaration]
  if (at803x_match_phy_id(phydev, ATH8031_PHY_ID)) {
      ^~~~~~~~~~~~~~~~~~~
      cpupid_match_pid
drivers/net/phy/at803x.c:366:3: error: implicit declaration of function 'phy_lock_mdio_bus' [-Werror=implicit-function-declaration]
   phy_lock_mdio_bus(phydev);
   ^~~~~~~~~~~~~~~~~
drivers/net/phy/at803x.c:368:3: error: implicit declaration of function 'phy_unlock_mdio_bus' [-Werror=implicit-function-declaration]
   phy_unlock_mdio_bus(phydev);
   ^~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
make[8]: *** [scripts/Makefile.build:262: drivers/net/phy/at803x.o] Error 1

Cheers,
Qingfang Deng June 6, 2021, 1:40 p.m. UTC | #5
Hi,

On Sat, Jun 5, 2021 at 12:19 AM David Bauer <mail@david-bauer.net> wrote:
>
> See the commit "generic: backport at803x fixes" in my staging tree [0].

Your patch still adds .config_aneg to AR8035, not AR8031.

>
> I'll test this later this weekend on my AC Lite.
>
> [0] https://git.openwrt.org/?p=openwrt/staging/blocktrron.git;a=summary
>
> Best
> David
>
René van Dorst June 6, 2021, 6:44 p.m. UTC | #6
The sender domain has a DMARC Reject/Quarantine policy which disallows
sending mailing list messages using the original "From" header.

To mitigate this problem, the original message has been wrapped
automatically by the mailing list software.
Quoting DENG Qingfang <dqfext@gmail.com>:

> Commit 8222f8e1b9be overrides AR8031's PHY features, making its SFP
> link status detection unavailable. To fix that, do not override it in
> ramips patch.
>
> Commit 6d4ef6792612 also moves .config_aneg to AR8035 by mistake, fix
> that as well.
>
> Reported-by: CHEN Minqiang <ptpt52@gmail.com>
> Fixes: 8222f8e1b9be ("ath79: fix link mode support list on UniFi AC")
> Fixes: 6d4ef6792612 ("kernel: bump 5.4 to 5.4.113")
> Signed-off-by: DENG Qingfang <dqfext@gmail.com>
> ---
> v1 -> v2: do not override .features
>
>  .../ramips/patches-5.10/710-at803x.patch      | 13 ++++++++++--
>  .../linux/ramips/patches-5.4/991-at803x.patch | 21 ++++++++++++-------
>  2 files changed, 25 insertions(+), 9 deletions(-)
>
> diff --git a/target/linux/ramips/patches-5.10/710-at803x.patch  
> b/target/linux/ramips/patches-5.10/710-at803x.patch
> index dab62b7607..1afc266740 100644
> --- a/target/linux/ramips/patches-5.10/710-at803x.patch
> +++ b/target/linux/ramips/patches-5.10/710-at803x.patch
> @@ -9,8 +9,8 @@ Content-Transfer-Encoding: 8bit
>
>  Signed-off-by: René van Dorst <opensource@vdorst.com>
>  ---
> - drivers/net/phy/at803x.c | 91 ++++++++++++++++++++++++++++++++++++++++
> - 1 file changed, 91 insertions(+)
> + drivers/net/phy/at803x.c | 86 +++++++++++++++++++++++++++++++++++++++-
> + 1 file changed, 85 insertions(+), 1 deletion(-)
>
>  --- a/drivers/net/phy/at803x.c
>  +++ b/drivers/net/phy/at803x.c
> @@ -147,3 +147,12 @@ Signed-off-by: René van Dorst <opensource@vdorst.com>
>   	.flags			= PHY_POLL_CABLE_TEST,
>   	.probe			= at803x_probe,
>   	.remove			= at803x_remove,
> +@@ -1119,7 +1203,7 @@ static struct phy_driver at803x_driver[]
> + 	.get_wol		= at803x_get_wol,
> + 	.suspend		= at803x_suspend,
> + 	.resume			= at803x_resume,
> +-	.features               = PHY_GBIT_FEATURES,
> ++	/* PHY_GBIT_FEATURES */
> + 	.read_status		= at803x_read_status,
> + 	.aneg_done		= at803x_aneg_done,
> + 	.ack_interrupt		= &at803x_ack_interrupt,
> diff --git a/target/linux/ramips/patches-5.4/991-at803x.patch  
> b/target/linux/ramips/patches-5.4/991-at803x.patch
> index 95411211b2..9148687113 100644
> --- a/target/linux/ramips/patches-5.4/991-at803x.patch
> +++ b/target/linux/ramips/patches-5.4/991-at803x.patch
> @@ -9,8 +9,8 @@ Content-Transfer-Encoding: 8bit
>
>  Signed-off-by: René van Dorst <opensource@vdorst.com>
>  ---
> - drivers/net/phy/at803x.c | 91 ++++++++++++++++++++++++++++++++++++++++
> - 1 file changed, 91 insertions(+)
> + drivers/net/phy/at803x.c | 93 +++++++++++++++++++++++++++++++++++++++-
> + 1 file changed, 92 insertions(+), 1 deletion(-)
>
>  --- a/drivers/net/phy/at803x.c
>  +++ b/drivers/net/phy/at803x.c
> @@ -146,11 +146,18 @@ Signed-off-by: René van Dorst <opensource@vdorst.com>
>   static struct phy_driver at803x_driver[] = {
>   {
>   	/* ATHEROS 8035 */
> -@@ -461,6 +551,7 @@ static struct phy_driver at803x_driver[]
> +@@ -485,12 +575,13 @@ static struct phy_driver at803x_driver[]
> + 	.name			= "Atheros 8031 ethernet",
> + 	.phy_id_mask		= AT803X_PHY_ID_MASK,
> + 	.probe			= at803x_probe,
> ++	.config_aneg		= at803x_config_aneg,
> + 	.config_init		= at803x_config_init,
> + 	.set_wol		= at803x_set_wol,
> + 	.get_wol		= at803x_get_wol,
>   	.suspend		= at803x_suspend,
>   	.resume			= at803x_resume,
> - 	/* PHY_GBIT_FEATURES */
> -+	.config_aneg		= at803x_config_aneg,
> +-	.features               = PHY_GBIT_FEATURES,
> ++	/* PHY_GBIT_FEATURES */
>   	.read_status		= at803x_read_status,
> - 	.ack_interrupt		= at803x_ack_interrupt,
> - 	.config_intr		= at803x_config_intr,
> + 	.aneg_done		= at803x_aneg_done,
> + 	.ack_interrupt		= &at803x_ack_interrupt,
> --
> 2.25.1


Hi Qingfang,

Thank for fixing.
But I can't apply this patch clean on top of the current master  
04a260911ca0f10a0e37c487c220e1aae3623dda.
Which commit/branch are you using?

# patch -p1 < p.patch
patching file target/linux/ramips/patches-5.10/710-at803x.patch
Hunk #2 FAILED at 147.
1 out of 2 hunks FAILED -- saving rejects to file  
target/linux/ramips/patches-5.10/710-at803x.patch.rej
patching file target/linux/ramips/patches-5.4/991-at803x.patch
Hunk #2 FAILED at 146.
1 out of 2 hunks FAILED -- saving rejects to file  
target/linux/ramips/patches-5.4/991-at803x.patch.rej


# cat target/linux/ramips/patches-5.10/710-at803x.patch.rej
--- target/linux/ramips/patches-5.10/710-at803x.patch
+++ target/linux/ramips/patches-5.10/710-at803x.patch
@@ -147,3 +147,12 @@ Signed-off-by: René van Dorst <opensource@vdorst.com>
           .flags                        = PHY_POLL_CABLE_TEST,
           .probe                        = at803x_probe,
           .remove                        = at803x_remove,
+@@ -1119,7 +1203,7 @@ static struct phy_driver at803x_driver[]
+         .get_wol                = at803x_get_wol,
+         .suspend                = at803x_suspend,
+         .resume                        = at803x_resume,
+-        .features               = PHY_GBIT_FEATURES,
++        /* PHY_GBIT_FEATURES */
+         .read_status                = at803x_read_status,
+         .aneg_done                = at803x_aneg_done,
+         .ack_interrupt                = &at803x_ack_interrupt,

Greats,

René
Qingfang Deng June 8, 2021, 11:55 a.m. UTC | #7
Hi René,

On Mon, Jun 7, 2021 at 2:44 AM René van Dorst <opensource@vdorst.com> wrote:
> Thank for fixing.
> But I can't apply this patch clean on top of the current master
> 04a260911ca0f10a0e37c487c220e1aae3623dda.
> Which commit/branch are you using?

David's patches have been applied to master, you can test it now.

>
> # patch -p1 < p.patch
> patching file target/linux/ramips/patches-5.10/710-at803x.patch
> Hunk #2 FAILED at 147.
> 1 out of 2 hunks FAILED -- saving rejects to file
> target/linux/ramips/patches-5.10/710-at803x.patch.rej
> patching file target/linux/ramips/patches-5.4/991-at803x.patch
> Hunk #2 FAILED at 146.
> 1 out of 2 hunks FAILED -- saving rejects to file
> target/linux/ramips/patches-5.4/991-at803x.patch.rej
>
>
> # cat target/linux/ramips/patches-5.10/710-at803x.patch.rej
> --- target/linux/ramips/patches-5.10/710-at803x.patch
> +++ target/linux/ramips/patches-5.10/710-at803x.patch
> @@ -147,3 +147,12 @@ Signed-off-by: René van Dorst <opensource@vdorst.com>
>            .flags                        = PHY_POLL_CABLE_TEST,
>            .probe                        = at803x_probe,
>            .remove                        = at803x_remove,
> +@@ -1119,7 +1203,7 @@ static struct phy_driver at803x_driver[]
> +         .get_wol                = at803x_get_wol,
> +         .suspend                = at803x_suspend,
> +         .resume                        = at803x_resume,
> +-        .features               = PHY_GBIT_FEATURES,
> ++        /* PHY_GBIT_FEATURES */
> +         .read_status                = at803x_read_status,
> +         .aneg_done                = at803x_aneg_done,
> +         .ack_interrupt                = &at803x_ack_interrupt,
>
> Greats,
>
> René
>
Qingfang Deng June 11, 2021, 6:08 a.m. UTC | #8
Hi David,

Unfortunately, the issue is still not resolved, according to Minqiang.
Reverting your 2 commits and applying my patch fixes the issue.
David Bauer June 11, 2021, 8:10 a.m. UTC | #9
Hi,

On 6/11/21 8:08 AM, DENG Qingfang wrote:
> Hi David,
> 
> Unfortunately, the issue is still not resolved, according to Minqiang.
> Reverting your 2 commits and applying my patch fixes the issue.

Can you be more precise in terms of which issues are you facing? The PHY capabilities on
the AR8333 now read 1000B-X as a supported link mode, so fiber operation should be possible.

Can you share the capabilities advertised with current master and your path (ethtool).

Reverting these patches would divert from upcoming kernel versions (mind both are backports,
not downstream hacks), thus this is not a solution. Ideally, the fiber operation should be
integrated into the upstream driver.

Best
David

>
John Thomson June 11, 2021, 10:51 p.m. UTC | #10
On Fri, 11 Jun 2021, at 08:10, David Bauer wrote:
> Can you be more precise in terms of which issues are you facing? The 
> PHY capabilities on
> the AR8333 now read 1000B-X as a supported link mode, so fiber 
> operation should be possible.
> 
> Can you share the capabilities advertised with current master and your 
> path (ethtool).
> 
> Reverting these patches would divert from upcoming kernel versions 
> (mind both are backports,
> not downstream hacks), thus this is not a solution. Ideally, the fiber 
> operation should be
> integrated into the upstream driver.

I agree that this should be corrected atop the backports.
It would be great to see this SFP support upstreamed.
I have not tested SFP on OpenWrt master for some time, so I cannot blame a change yet.

This is what I am seeing on ramips 760igs:

SFP (module, and driver LOS) has link detected, but this is not reflected by the interface
(which is configured in OpenWrt as part of a bridge)

sfp: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc fq_codel master br-wan state DOWN qlen 1000

r16925+6-b721579842 master, plus my
SPI-NOR changes: https://github.com/openwrt/openwrt/pull/3271/commits

I do not remember these being looped in dmesg when I had working SFP:
sfp sfp1: SM: enter present:up:link_up event dev_up
sfp sfp1: SM: exit present:up:link_up

ethtool sfp
Settings for sfp:
	Supported ports: [ TP MII ]
	Supported link modes:   10baseT/Half 10baseT/Full 
	                        100baseT/Half 100baseT/Full 
	                        1000baseT/Full 
	                        1000baseX/Full 
	Supported pause frame use: Symmetric Receive-only
	Supports auto-negotiation: Yes
	Supported FEC modes: Not reported
	Advertised link modes:  10baseT/Half 10baseT/Full 
	                        100baseT/Half 100baseT/Full 
	                        1000baseT/Full 
	                        1000baseX/Full 
	Advertised pause frame use: Symmetric Receive-only
	Advertised auto-negotiation: Yes
	Advertised FEC modes: Not reported
	Speed: Unknown!
	Duplex: Unknown! (255)
	Port: MII
	PHYAD: 7
	Transceiver: external
	Auto-negotiation: on
	Current message level: 0x000000ff (255)
			       drv probe link timer ifdown ifup rx_err tx_err
	Link detected: no

Pieces missing from a very old (r14885+1-fe302d472a) working SFP build: link partner advertisement and link speed / duplex
        Link partner advertised link modes:  1000baseX/Full
        Link partner advertised pause frame use: Symmetric Receive-only
        Link partner advertised auto-negotiation: Yes
        Link partner advertised FEC modes: Not reported
        Speed: 1000Mb/s
        Duplex: Full
        Port: MII
        PHYAD: 7
        Transceiver: external
        Auto-negotiation: on
        Current message level: 0x000000ff (255)
                               drv probe link timer ifdown ifup rx_err tx_err
        Link detected: yes

these cannot be forced on my current build:
ethtool -s sfp speed 1000 duplex full
[  182.182182] at803x_config_aneg: fiber


Attached:
version=$(cat /etc/openwrt_version)
mkdir -p "/tmp/sfp/$version"
dmesg > "/tmp/sfp/$version/dmesg"
logread > "/tmp/sfp/$version/logread"
ethtool sfp > "/tmp/sfp/$version/ethtool_sfp"
ethtool -m sfp > "/tmp/sfp/$version/ethtool_m_sfp"
echo -n 'file sfp.c +p' > /sys/kernel/debug/dynamic_debug/control
/etc/init.d/network restart
dmesg > "/tmp/sfp/$version/dmesg_reup"
logread > "/tmp/sfp/$version/logread_reup"
# remove, and reinsert SFP module
dmesg > "/tmp/sfp/$version/dmesg_reinsert"
logread > "/tmp/sfp/$version/logread_reinsert"

diffconfig:
CONFIG_TARGET_ramips=y
CONFIG_TARGET_ramips_mt7621=y
CONFIG_TARGET_ramips_mt7621_DEVICE_mikrotik_routerboard-760igs=y
CONFIG_KERNEL_DYNAMIC_DEBUG=y
CONFIG_KERNEL_MTD_PARTITIONED_MASTER=y
CONFIG_PACKAGE_ethtool=y
CONFIG_ETHTOOL_PRETTY_DUMP=y

Cheers,
David Bauer June 12, 2021, 11:06 a.m. UTC | #11
Hi John,

On 6/12/21 12:51 AM, John Thomson wrote:
> On Fri, 11 Jun 2021, at 08:10, David Bauer wrote:
>> Can you be more precise in terms of which issues are you facing? The
>> PHY capabilities on
>> the AR8333 now read 1000B-X as a supported link mode, so fiber
>> operation should be possible.
>>
>> Can you share the capabilities advertised with current master and your
>> path (ethtool).
>>
>> Reverting these patches would divert from upcoming kernel versions
>> (mind both are backports,
>> not downstream hacks), thus this is not a solution. Ideally, the fiber
>> operation should be
>> integrated into the upstream driver.
> 
> I agree that this should be corrected atop the backports.
> It would be great to see this SFP support upstreamed.
> I have not tested SFP on OpenWrt master for some time, so I cannot blame a change yet.
> 
> This is what I am seeing on ramips 760igs:
> 
> SFP (module, and driver LOS) has link detected, but this is not reflected by the interface
> (which is configured in OpenWrt as part of a bridge)

I suppose what is happening here is the bootloader switched the PHY to 
the fiber page while linux now switches it to the copper page 
unconditionally.

Technically, this is correct from upstream perspective, as the PHY 
upstream only supports copper opmode. But it breaks the hacked fiber 
support downstream.

DENGs initial patch fixed this with the old downstream hacks, where the 
page was only switched when the PHY operated in SGMII mode, as the whole 
assumption this patch was based upon was wrong but lead to the correct 
result.

However, while we now get the correct link modes, we now switch to the 
fiber page upon probe.

Note that I haven't verified this, as i do not own this board. I'll 
prepare a patch and send it to this thread this weekend. Would be great 
if someone with this board could test it :)

Best
David

> 
> sfp: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc fq_codel master br-wan state DOWN qlen 1000
> 
> r16925+6-b721579842 master, plus my
> SPI-NOR changes: https://github.com/openwrt/openwrt/pull/3271/commits
> 
> I do not remember these being looped in dmesg when I had working SFP:
> sfp sfp1: SM: enter present:up:link_up event dev_up
> sfp sfp1: SM: exit present:up:link_up
> 
> ethtool sfp
> Settings for sfp:
> 	Supported ports: [ TP MII ]
> 	Supported link modes:   10baseT/Half 10baseT/Full
> 	                        100baseT/Half 100baseT/Full
> 	                        1000baseT/Full
> 	                        1000baseX/Full
> 	Supported pause frame use: Symmetric Receive-only
> 	Supports auto-negotiation: Yes
> 	Supported FEC modes: Not reported
> 	Advertised link modes:  10baseT/Half 10baseT/Full
> 	                        100baseT/Half 100baseT/Full
> 	                        1000baseT/Full
> 	                        1000baseX/Full
> 	Advertised pause frame use: Symmetric Receive-only
> 	Advertised auto-negotiation: Yes
> 	Advertised FEC modes: Not reported
> 	Speed: Unknown!
> 	Duplex: Unknown! (255)
> 	Port: MII
> 	PHYAD: 7
> 	Transceiver: external
> 	Auto-negotiation: on
> 	Current message level: 0x000000ff (255)
> 			       drv probe link timer ifdown ifup rx_err tx_err
> 	Link detected: no
> 
> Pieces missing from a very old (r14885+1-fe302d472a) working SFP build: link partner advertisement and link speed / duplex
>          Link partner advertised link modes:  1000baseX/Full
>          Link partner advertised pause frame use: Symmetric Receive-only
>          Link partner advertised auto-negotiation: Yes
>          Link partner advertised FEC modes: Not reported
>          Speed: 1000Mb/s
>          Duplex: Full
>          Port: MII
>          PHYAD: 7
>          Transceiver: external
>          Auto-negotiation: on
>          Current message level: 0x000000ff (255)
>                                 drv probe link timer ifdown ifup rx_err tx_err
>          Link detected: yes
> 
> these cannot be forced on my current build:
> ethtool -s sfp speed 1000 duplex full
> [  182.182182] at803x_config_aneg: fiber
> 
> 
> Attached:
> version=$(cat /etc/openwrt_version)
> mkdir -p "/tmp/sfp/$version"
> dmesg > "/tmp/sfp/$version/dmesg"
> logread > "/tmp/sfp/$version/logread"
> ethtool sfp > "/tmp/sfp/$version/ethtool_sfp"
> ethtool -m sfp > "/tmp/sfp/$version/ethtool_m_sfp"
> echo -n 'file sfp.c +p' > /sys/kernel/debug/dynamic_debug/control
> /etc/init.d/network restart
> dmesg > "/tmp/sfp/$version/dmesg_reup"
> logread > "/tmp/sfp/$version/logread_reup"
> # remove, and reinsert SFP module
> dmesg > "/tmp/sfp/$version/dmesg_reinsert"
> logread > "/tmp/sfp/$version/logread_reinsert"
> 
> diffconfig:
> CONFIG_TARGET_ramips=y
> CONFIG_TARGET_ramips_mt7621=y
> CONFIG_TARGET_ramips_mt7621_DEVICE_mikrotik_routerboard-760igs=y
> CONFIG_KERNEL_DYNAMIC_DEBUG=y
> CONFIG_KERNEL_MTD_PARTITIONED_MASTER=y
> CONFIG_PACKAGE_ethtool=y
> CONFIG_ETHTOOL_PRETTY_DUMP=y
> 
> Cheers,
> 
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
>
diff mbox series

Patch

diff --git a/target/linux/ramips/patches-5.10/710-at803x.patch b/target/linux/ramips/patches-5.10/710-at803x.patch
index dab62b7607..1afc266740 100644
--- a/target/linux/ramips/patches-5.10/710-at803x.patch
+++ b/target/linux/ramips/patches-5.10/710-at803x.patch
@@ -9,8 +9,8 @@  Content-Transfer-Encoding: 8bit
 
 Signed-off-by: René van Dorst <opensource@vdorst.com>
 ---
- drivers/net/phy/at803x.c | 91 ++++++++++++++++++++++++++++++++++++++++
- 1 file changed, 91 insertions(+)
+ drivers/net/phy/at803x.c | 86 +++++++++++++++++++++++++++++++++++++++-
+ 1 file changed, 85 insertions(+), 1 deletion(-)
 
 --- a/drivers/net/phy/at803x.c
 +++ b/drivers/net/phy/at803x.c
@@ -147,3 +147,12 @@  Signed-off-by: René van Dorst <opensource@vdorst.com>
  	.flags			= PHY_POLL_CABLE_TEST,
  	.probe			= at803x_probe,
  	.remove			= at803x_remove,
+@@ -1119,7 +1203,7 @@ static struct phy_driver at803x_driver[]
+ 	.get_wol		= at803x_get_wol,
+ 	.suspend		= at803x_suspend,
+ 	.resume			= at803x_resume,
+-	.features               = PHY_GBIT_FEATURES,
++	/* PHY_GBIT_FEATURES */
+ 	.read_status		= at803x_read_status,
+ 	.aneg_done		= at803x_aneg_done,
+ 	.ack_interrupt		= &at803x_ack_interrupt,
diff --git a/target/linux/ramips/patches-5.4/991-at803x.patch b/target/linux/ramips/patches-5.4/991-at803x.patch
index 95411211b2..9148687113 100644
--- a/target/linux/ramips/patches-5.4/991-at803x.patch
+++ b/target/linux/ramips/patches-5.4/991-at803x.patch
@@ -9,8 +9,8 @@  Content-Transfer-Encoding: 8bit
 
 Signed-off-by: René van Dorst <opensource@vdorst.com>
 ---
- drivers/net/phy/at803x.c | 91 ++++++++++++++++++++++++++++++++++++++++
- 1 file changed, 91 insertions(+)
+ drivers/net/phy/at803x.c | 93 +++++++++++++++++++++++++++++++++++++++-
+ 1 file changed, 92 insertions(+), 1 deletion(-)
 
 --- a/drivers/net/phy/at803x.c
 +++ b/drivers/net/phy/at803x.c
@@ -146,11 +146,18 @@  Signed-off-by: René van Dorst <opensource@vdorst.com>
  static struct phy_driver at803x_driver[] = {
  {
  	/* ATHEROS 8035 */
-@@ -461,6 +551,7 @@ static struct phy_driver at803x_driver[]
+@@ -485,12 +575,13 @@ static struct phy_driver at803x_driver[]
+ 	.name			= "Atheros 8031 ethernet",
+ 	.phy_id_mask		= AT803X_PHY_ID_MASK,
+ 	.probe			= at803x_probe,
++	.config_aneg		= at803x_config_aneg,
+ 	.config_init		= at803x_config_init,
+ 	.set_wol		= at803x_set_wol,
+ 	.get_wol		= at803x_get_wol,
  	.suspend		= at803x_suspend,
  	.resume			= at803x_resume,
- 	/* PHY_GBIT_FEATURES */
-+	.config_aneg		= at803x_config_aneg,
+-	.features               = PHY_GBIT_FEATURES,
++	/* PHY_GBIT_FEATURES */
  	.read_status		= at803x_read_status,
- 	.ack_interrupt		= at803x_ack_interrupt,
- 	.config_intr		= at803x_config_intr,
+ 	.aneg_done		= at803x_aneg_done,
+ 	.ack_interrupt		= &at803x_ack_interrupt,