| Message ID | 20210122214247.6536-1-sbauer@blackbox.su (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v2] lan743x: add virtual PHY for PHY-less devices | expand |
On Sat, Jan 23, 2021 at 12:42:41AM +0300, Sergej Bauer wrote: > From: sbauer@blackbox.su > > v1->v2: > switch to using of fixed_phy as was suggested by Andrew and Florian > also features-related parts are removed This is not using fixed_phy, at least not in the normal way. Take a look at bgmac_phy_connect_direct() for example. Call fixed_phy_register(), and then phy_connect_direct() with the phydev. End of story. Done. > +int lan743x_set_link_ksettings(struct net_device *netdev, > + const struct ethtool_link_ksettings *cmd) > +{ > + if (!netdev->phydev) > + return -ENETDOWN; > + > + return phy_is_pseudo_fixed_link(netdev->phydev) ? > + lan743x_set_virtual_link_ksettings(netdev, cmd) > + : phy_ethtool_set_link_ksettings(netdev, cmd); > +} There should not be any need to do something different. The whole point of fixed_phy is it looks like a PHY. So calling phy_ethtool_set_link_ksettings() should work, nothing special needed. > @@ -1000,8 +1005,10 @@ static void lan743x_phy_close(struct lan743x_adapter *adapter) > struct net_device *netdev = adapter->netdev; > > phy_stop(netdev->phydev); > - phy_disconnect(netdev->phydev); > - netdev->phydev = NULL; > + if (phy_is_pseudo_fixed_link(netdev->phydev)) > + lan743x_virtual_phy_disconnect(netdev->phydev); > + else > + phy_disconnect(netdev->phydev); phy_disconnect() should work. You might want to call fixed_phy_unregister() afterwards, so you do not leak memory. > + if (phy_is_pseudo_fixed_link(phydev)) { > + ret = phy_connect_direct(netdev, phydev, > + lan743x_virtual_phy_status_change, > + PHY_INTERFACE_MODE_MII); > + } else { > + ret = phy_connect_direct(netdev, phydev, > + lan743x_phy_link_status_change, There should not be any need for a special link change callback. lan743x_phy_link_status_change() should work fine, the MAC should have no idea it is using a fixed_phy. > + PHY_INTERFACE_MODE_GMII); > + } > + > if (ret) > goto return_error; > } > @@ -1031,6 +1049,15 @@ static int lan743x_phy_open(struct lan743x_adapter *adapter) > /* MAC doesn't support 1000T Half */ > phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_1000baseT_Half_BIT); > > + if (phy_is_pseudo_fixed_link(phydev)) { > + phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_TP_BIT); > + linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, > + phydev->supported); > + linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, > + phydev->supported); > + phy_advertise_supported(phydev); > + } The fixed PHY driver will set these bits depending on the speed it has been configured for. No need to change them. The MAC should also not care if it is TP, AUI, Fibre or smoke signals. Andrew
On Saturday, January 23, 2021 12:52:48 AM MSK Florian Fainelli wrote: > On 1/22/2021 1:42 PM, Sergej Bauer wrote: > > From: sbauer@blackbox.su > > > > v1->v2: > > switch to using of fixed_phy as was suggested by Andrew and Florian > > also features-related parts are removed > > > > Previous versions can be found at: > > v1: > > initial version > > > > https://lkml.org/lkml/2020/9/17/1272 > > > > Signed-off-by: Sergej Bauer <sbauer@blackbox.su> > > You are not explaining why you need this and why you are second guessing > the fixed PHY MII emulation that already exists. You really need to do a > better job at describing your changes and why the emulation offered by > swphy.c is not enough for your use case. ok, I'll try to accomplish it with swphy.
On Saturday, January 23, 2021 1:08:03 AM MSK Andrew Lunn wrote: > On Sat, Jan 23, 2021 at 12:42:41AM +0300, Sergej Bauer wrote: > > From: sbauer@blackbox.su > > > > v1->v2: > > switch to using of fixed_phy as was suggested by Andrew and Florian > > also features-related parts are removed > > This is not using fixed_phy, at least not in the normal way. > > Take a look at bgmac_phy_connect_direct() for example. Call > fixed_phy_register(), and then phy_connect_direct() with the > phydev. End of story. Done. > > > +int lan743x_set_link_ksettings(struct net_device *netdev, > > + const struct ethtool_link_ksettings *cmd) > > +{ > > + if (!netdev->phydev) > > + return -ENETDOWN; > > + > > + return phy_is_pseudo_fixed_link(netdev->phydev) ? > > + lan743x_set_virtual_link_ksettings(netdev, cmd) > > + : phy_ethtool_set_link_ksettings(netdev, cmd); > > +} > > There should not be any need to do something different. The whole > point of fixed_phy is it looks like a PHY. So calling > phy_ethtool_set_link_ksettings() should work, nothing special needed. > > > @@ -1000,8 +1005,10 @@ static void lan743x_phy_close(struct > > lan743x_adapter *adapter)> > > struct net_device *netdev = adapter->netdev; > > > > phy_stop(netdev->phydev); > > > > - phy_disconnect(netdev->phydev); > > - netdev->phydev = NULL; > > + if (phy_is_pseudo_fixed_link(netdev->phydev)) > > + lan743x_virtual_phy_disconnect(netdev->phydev); > > + else > > + phy_disconnect(netdev->phydev); > > phy_disconnect() should work. You might want to call Andrew, this is why I was playing with my own _connect/_disconnect realizations It should work but it don't. modprobe lan743x rmmod lan743x modprobe lan743x leads to "net eth7: could not add device link to fixed-0:06 err -17" in dmesg (it does not removes corresponding phydev file in /sysfs) moreover phy_disconnect leads to kernel panic [82363.976726] BUG: kernel NULL pointer dereference, address: 00000000000003c4 [82363.977402] #PF: supervisor read access in kernel mode [82363.978077] #PF: error_code(0x0000) - not-present page [82363.978588] PGD 0 P4D 0 [82363.978588] Oops: 0000 [#1] SMP NOPTI [82363.978588] CPU: 3 PID: 2634 Comm: ifconfig Tainted: G O 5.11.0-rc4net-next-virtual_phy+ #3 [82363.978588] Hardware name: Gigabyte Technology Co., Ltd. H110-D3/H110-D3- CF, BIOS F24 04/11/2018 [82363.978588] RIP: 0010:lan743x_phy_close.isra.26+0x28/0x40 [lan743x] [82363.978588] Code: c3 90 0f 1f 44 00 00 53 48 89 fb 48 8b bf 28 08 00 00 e8 ab 5e 86 ff 48 8b bb 28 08 00 00 e8 4f 92 86 ff 48 8b bb 28 08 00 00 <f6> 87 c4 03 00 00 04 75 02 5b c3 5b e9 f7 35 ea ff 0f 1f 80 00 00 [82363.982448] RSP: 0018:ffffa528c04c7c38 EFLAGS: 00010246 [82363.982448] RAX: 000000000000000f RBX: ffff991a47e38000 RCX: 0000000000000027 [82363.982448] RDX: 0000000000000000 RSI: ffff991c76d98b30 RDI: 0000000000000000 [82363.982448] RBP: 0000000000000001 R08: 0000000000000000 R09: c0000000ffffefff [82363.982448] R10: 0000000000000001 R11: ffffa528c04c7a48 R12: ffff991a47e388c0 [82363.986855] R13: ffff991a47e390b8 R14: ffff991a47e39050 R15: ffff991a47e388c0 [82363.986855] FS: 00007f7c3ebd6540(0000) GS:ffff991c76d80000(0000) knlGS: 0000000000000000 [82363.986855] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [82363.986855] CR2: 00000000000003c4 CR3: 000000001b2ac005 CR4: 00000000003706e0 [82363.986855] Call Trace: [82363.986855] lan743x_netdev_close+0x223/0x250 [lan743x] ... > fixed_phy_unregister() afterwards, so you do not leak memory. > > > + if (phy_is_pseudo_fixed_link(phydev)) { > > + ret = phy_connect_direct(netdev, phydev, > > + lan743x_virtual_phy_status_change, > > + PHY_INTERFACE_MODE_MII); > > + } else { > > + ret = phy_connect_direct(netdev, phydev, > > + lan743x_phy_link_status_change, > > There should not be any need for a special link change > callback. lan743x_phy_link_status_change() should work fine, the MAC > should have no idea it is using a fixed_phy. > > > + PHY_INTERFACE_MODE_GMII); > > + } > > + > > > > if (ret) > > > > goto return_error; > > > > } > > > > @@ -1031,6 +1049,15 @@ static int lan743x_phy_open(struct lan743x_adapter > > *adapter)> > > /* MAC doesn't support 1000T Half */ > > phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_1000baseT_Half_BIT); > > > > + if (phy_is_pseudo_fixed_link(phydev)) { > > + phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_TP_BIT); > > + linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, > > + phydev->supported); > > + linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, > > + phydev->supported); > > + phy_advertise_supported(phydev); > > + } > > The fixed PHY driver will set these bits depending on the speed it has > been configured for. No need to change them. The MAC should also not > care if it is TP, AUI, Fibre or smoke signals. It was to make ethtool show full set of supported speeds and MII only in supported ports (without TP and the no any ports in the bare card). > > Andrew I think I should reproduce the panic with clean version of net-net -- Regards, Sergej.
> > > @@ -1000,8 +1005,10 @@ static void lan743x_phy_close(struct > > > lan743x_adapter *adapter)> > > > struct net_device *netdev = adapter->netdev; > > > > > > phy_stop(netdev->phydev); > > > > > > - phy_disconnect(netdev->phydev); > > > - netdev->phydev = NULL; > > > + if (phy_is_pseudo_fixed_link(netdev->phydev)) > > > + lan743x_virtual_phy_disconnect(netdev->phydev); > > > + else > > > + phy_disconnect(netdev->phydev); > > > > phy_disconnect() should work. You might want to call There are drivers which call phy_disconnect() on a fixed_link. e.g. https://elixir.bootlin.com/linux/v5.11-rc4/source/drivers/net/usb/lan78xx.c#L3555. It could be your missing call to fixed_phy_unregister() is leaving behind bad state. > It was to make ethtool show full set of supported speeds and MII only in > supported ports (without TP and the no any ports in the bare card). But fixed link does not support the full set of speed. It is fixed. It supports only one speed it is configured with. And by setting it wrongly, you are going to allow the user to do odd things, like use ethtool force the link speed to a speed which is not actually supported. Andrew
On Saturday, January 23, 2021 2:23:25 AM MSK Andrew Lunn wrote: > > > > @@ -1000,8 +1005,10 @@ static void lan743x_phy_close(struct > > > > lan743x_adapter *adapter)> > > > > > > > > struct net_device *netdev = adapter->netdev; > > > > > > > > phy_stop(netdev->phydev); > > > > > > > > - phy_disconnect(netdev->phydev); > > > > - netdev->phydev = NULL; > > > > + if (phy_is_pseudo_fixed_link(netdev->phydev)) > > > > + lan743x_virtual_phy_disconnect(netdev->phydev); > > > > + else > > > > + phy_disconnect(netdev->phydev); > > > > > > phy_disconnect() should work. You might want to call > > There are drivers which call phy_disconnect() on a fixed_link. e.g. > > https://elixir.bootlin.com/linux/v5.11-rc4/source/drivers/net/usb/lan78xx.c# > L3555. > > It could be your missing call to fixed_phy_unregister() is leaving > behind bad state. > lan743x_virtual_phy_disconnect removes sysfs-links and calls fixed_phy_unregister() and the reason was phydev in sysfs. > > It was to make ethtool show full set of supported speeds and MII only in > > supported ports (without TP and the no any ports in the bare card). > > But fixed link does not support the full set of speed. It is fixed. It > supports only one speed it is configured with. That's why I "re-implemented the fixed PHY driver" as Florian said. The goal of virtual phy was to make an illusion of real device working in loopback mode. So I could use ethtool and ioctl's to switch speed of device. > And by setting it > wrongly, you are going to allow the user to do odd things, like use > ethtool force the link speed to a speed which is not actually > supported. I have lan743x only and in loopback mode it allows to use speeds 10/100/1000MBps in full-duplex mode only. But the highest speed I have achived was something near 752Mbps... And I can switch speed on the fly, without reloading the module. May by I should limit the list of acceptable devices? > > Andrew
On 1/22/2021 3:58 PM, Sergej Bauer wrote: > On Saturday, January 23, 2021 2:23:25 AM MSK Andrew Lunn wrote: >>>>> @@ -1000,8 +1005,10 @@ static void lan743x_phy_close(struct >>>>> lan743x_adapter *adapter)> >>>>> >>>>> struct net_device *netdev = adapter->netdev; >>>>> >>>>> phy_stop(netdev->phydev); >>>>> >>>>> - phy_disconnect(netdev->phydev); >>>>> - netdev->phydev = NULL; >>>>> + if (phy_is_pseudo_fixed_link(netdev->phydev)) >>>>> + lan743x_virtual_phy_disconnect(netdev->phydev); >>>>> + else >>>>> + phy_disconnect(netdev->phydev); >>>> >>>> phy_disconnect() should work. You might want to call >> >> There are drivers which call phy_disconnect() on a fixed_link. e.g. >> >> https://elixir.bootlin.com/linux/v5.11-rc4/source/drivers/net/usb/lan78xx.c# >> L3555. >> > >> It could be your missing call to fixed_phy_unregister() is leaving >> behind bad state. >> > lan743x_virtual_phy_disconnect removes sysfs-links and calls > fixed_phy_unregister() > and the reason was phydev in sysfs. > >>> It was to make ethtool show full set of supported speeds and MII only in >>> supported ports (without TP and the no any ports in the bare card). >> >> But fixed link does not support the full set of speed. It is fixed. It >> supports only one speed it is configured with. > That's why I "re-implemented the fixed PHY driver" as Florian said. > The goal of virtual phy was to make an illusion of real device working in > loopback mode. So I could use ethtool and ioctl's to switch speed of device. > >> And by setting it >> wrongly, you are going to allow the user to do odd things, like use >> ethtool force the link speed to a speed which is not actually >> supported. > I have lan743x only and in loopback mode it allows to use speeds > 10/100/1000MBps > in full-duplex mode only. But the highest speed I have achived was something > near > 752Mbps... > And I can switch speed on the fly, without reloading the module. > > May by I should limit the list of acceptable devices? It is not clear what your use case is so maybe start with explaining it and we can help you define something that may be acceptable for upstream inclusion.
On Saturday, January 23, 2021 3:01:47 AM MSK Florian Fainelli wrote: > On 1/22/2021 3:58 PM, Sergej Bauer wrote: > > On Saturday, January 23, 2021 2:23:25 AM MSK Andrew Lunn wrote: > >>>>> @@ -1000,8 +1005,10 @@ static void lan743x_phy_close(struct > >>>>> lan743x_adapter *adapter)> > >>>>> > >>>>> struct net_device *netdev = adapter->netdev; > >>>>> > >>>>> phy_stop(netdev->phydev); > >>>>> > >>>>> - phy_disconnect(netdev->phydev); > >>>>> - netdev->phydev = NULL; > >>>>> + if (phy_is_pseudo_fixed_link(netdev->phydev)) > >>>>> + lan743x_virtual_phy_disconnect(netdev->phydev); > >>>>> + else > >>>>> + phy_disconnect(netdev->phydev); > >>>> > >>>> phy_disconnect() should work. You might want to call > >> > >> There are drivers which call phy_disconnect() on a fixed_link. e.g. > >> > >> https://elixir.bootlin.com/linux/v5.11-rc4/source/drivers/net/usb/lan78xx > >> .c# L3555. > >> > >> > >> It could be your missing call to fixed_phy_unregister() is leaving > >> behind bad state. > > > > lan743x_virtual_phy_disconnect removes sysfs-links and calls > > fixed_phy_unregister() > > and the reason was phydev in sysfs. > > > >>> It was to make ethtool show full set of supported speeds and MII only in > >>> supported ports (without TP and the no any ports in the bare card). > >> > >> But fixed link does not support the full set of speed. It is fixed. It > >> supports only one speed it is configured with. > > > > That's why I "re-implemented the fixed PHY driver" as Florian said. > > The goal of virtual phy was to make an illusion of real device working in > > loopback mode. So I could use ethtool and ioctl's to switch speed of > > device.> > >> And by setting it > >> wrongly, you are going to allow the user to do odd things, like use > >> ethtool force the link speed to a speed which is not actually > >> supported. > > > > I have lan743x only and in loopback mode it allows to use speeds > > 10/100/1000MBps > > in full-duplex mode only. But the highest speed I have achived was > > something near > > 752Mbps... > > And I can switch speed on the fly, without reloading the module. > > > > May by I should limit the list of acceptable devices? > > It is not clear what your use case is so maybe start with explaining it > and we can help you define something that may be acceptable for upstream > inclusion. it migth be helpful for developers work on userspace networking tools with PHY-less lan743x (the interface even could not be brought up) of course, there nothing much to do without TP port but the difference is representative. sbauer@metamini ~$ sudo ethtool eth7 Settings for eth7: Cannot get device settings: No such device Supports Wake-on: pumbag Wake-on: d Current message level: 0x00000137 (311) drv probe link ifdown ifup tx_queued Link detected: no sbauer@metamini ~$ sudo ifup eth7 sbauer@metamini ~$ sudo ethtool eth7 Settings for eth7: Supported ports: [ MII ] Supported link modes: 10baseT/Full 100baseT/Full 1000baseT/Full Supported pause frame use: Symmetric Receive-only Supports auto-negotiation: Yes Supported FEC modes: Not reported Advertised link modes: 10baseT/Full 100baseT/Full 1000baseT/Full Advertised pause frame use: Symmetric Receive-only Advertised auto-negotiation: Yes Advertised FEC modes: Not reported Speed: 1000Mb/s Duplex: Full Port: MII PHYAD: 0 Transceiver: internal Auto-negotiation: on Supports Wake-on: pumbag Wake-on: d Current message level: 0x00000137 (311) drv probe link ifdown ifup tx_queued Link detected: yes sbauer@metamini ~$ sudo mii-tool -vv eth7 Using SIOCGMIIPHY=0x8947 eth7: negotiated 1000baseT-FD, link ok registers for MII PHY 0: 5140 512d 7431 0011 4140 4140 000d 0000 0000 0200 7800 0000 0000 0000 0000 2000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000 product info: vendor 1d:0c:40, model 1 rev 1 basic mode: loopback, autonegotiation enabled basic status: autonegotiation complete, link ok capabilities: 1000baseT-FD 100baseTx-FD 10baseT-FD advertising: 1000baseT-FD 100baseTx-FD 10baseT-FD link partner: 1000baseT-FD 100baseTx-FD 10baseT-FD Regards, Sergej.
> it migth be helpful for developers work on userspace networking tools with > PHY-less lan743x (the interface even could not be brought up) > of course, there nothing much to do without TP port but the difference is > representative. > > sbauer@metamini ~$ sudo ethtool eth7 > Settings for eth7: > Cannot get device settings: No such device > Supports Wake-on: pumbag > Wake-on: d > Current message level: 0x00000137 (311) > drv probe link ifdown ifup tx_queued > Link detected: no > sbauer@metamini ~$ sudo ifup eth7 > sbauer@metamini ~$ sudo ethtool eth7 > Settings for eth7: > Supported ports: [ MII ] > Supported link modes: 10baseT/Full > 100baseT/Full > 1000baseT/Full > Supported pause frame use: Symmetric Receive-only > Supports auto-negotiation: Yes > Supported FEC modes: Not reported > Advertised link modes: 10baseT/Full > 100baseT/Full > 1000baseT/Full > Advertised pause frame use: Symmetric Receive-only > Advertised auto-negotiation: Yes > Advertised FEC modes: Not reported > Speed: 1000Mb/s > Duplex: Full > Port: MII > PHYAD: 0 > Transceiver: internal > Auto-negotiation: on > Supports Wake-on: pumbag > Wake-on: d > Current message level: 0x00000137 (311) > drv probe link ifdown ifup tx_queued > Link detected: yes > sbauer@metamini ~$ sudo mii-tool -vv eth7 > Using SIOCGMIIPHY=0x8947 > eth7: negotiated 1000baseT-FD, link ok > registers for MII PHY 0: > 5140 512d 7431 0011 4140 4140 000d 0000 > 0000 0200 7800 0000 0000 0000 0000 2000 > 0000 0000 0000 0000 0000 0000 0000 0000 > 0000 0000 0000 0000 0000 0000 0000 0000 > product info: vendor 1d:0c:40, model 1 rev 1 > basic mode: loopback, autonegotiation enabled > basic status: autonegotiation complete, link ok > capabilities: 1000baseT-FD 100baseTx-FD 10baseT-FD > advertising: 1000baseT-FD 100baseTx-FD 10baseT-FD > link partner: 1000baseT-FD 100baseTx-FD 10baseT-FD You have not shown anything i cannot do with the ethernet interfaces i have in my laptop. And since ethtool is pretty standardized, what lan743x offers should be pretty much the same as any 1G Ethernet MAC using most 1G PHYs. Andrew
On 1/22/2021 5:01 PM, Sergej Bauer wrote: > On Saturday, January 23, 2021 3:01:47 AM MSK Florian Fainelli wrote: >> On 1/22/2021 3:58 PM, Sergej Bauer wrote: >>> On Saturday, January 23, 2021 2:23:25 AM MSK Andrew Lunn wrote: >>>>>>> @@ -1000,8 +1005,10 @@ static void lan743x_phy_close(struct >>>>>>> lan743x_adapter *adapter)> >>>>>>> >>>>>>> struct net_device *netdev = adapter->netdev; >>>>>>> >>>>>>> phy_stop(netdev->phydev); >>>>>>> >>>>>>> - phy_disconnect(netdev->phydev); >>>>>>> - netdev->phydev = NULL; >>>>>>> + if (phy_is_pseudo_fixed_link(netdev->phydev)) >>>>>>> + lan743x_virtual_phy_disconnect(netdev->phydev); >>>>>>> + else >>>>>>> + phy_disconnect(netdev->phydev); >>>>>> >>>>>> phy_disconnect() should work. You might want to call >>>> >>>> There are drivers which call phy_disconnect() on a fixed_link. e.g. >>>> >>>> https://elixir.bootlin.com/linux/v5.11-rc4/source/drivers/net/usb/lan78xx >>>> .c# L3555. >>>> >>>> >>>> It could be your missing call to fixed_phy_unregister() is leaving >>>> behind bad state. >>> >>> lan743x_virtual_phy_disconnect removes sysfs-links and calls >>> fixed_phy_unregister() >>> and the reason was phydev in sysfs. >>> >>>>> It was to make ethtool show full set of supported speeds and MII only in >>>>> supported ports (without TP and the no any ports in the bare card). >>>> >>>> But fixed link does not support the full set of speed. It is fixed. It >>>> supports only one speed it is configured with. >>> >>> That's why I "re-implemented the fixed PHY driver" as Florian said. >>> The goal of virtual phy was to make an illusion of real device working in >>> loopback mode. So I could use ethtool and ioctl's to switch speed of >>> device.> >>>> And by setting it >>>> wrongly, you are going to allow the user to do odd things, like use >>>> ethtool force the link speed to a speed which is not actually >>>> supported. >>> >>> I have lan743x only and in loopback mode it allows to use speeds >>> 10/100/1000MBps >>> in full-duplex mode only. But the highest speed I have achived was >>> something near >>> 752Mbps... >>> And I can switch speed on the fly, without reloading the module. >>> >>> May by I should limit the list of acceptable devices? >> >> It is not clear what your use case is so maybe start with explaining it >> and we can help you define something that may be acceptable for upstream >> inclusion. > it migth be helpful for developers work on userspace networking tools with > PHY-less lan743x (the interface even could not be brought up) > of course, there nothing much to do without TP port but the difference is > representative. You are still not explaining why fixed PHY is not a suitable emulation and what is different, providing the output of ethtool does not tell me how you are using it. Literally everyone using Ethernet switches or MAC to MAC Ethernet connections uses fixed PHY and is reasonably happy with it.
resending answer to all: On Saturday, January 23, 2021 4:32:38 AM MSK Andrew Lunn wrote: > > it migth be helpful for developers work on userspace networking tools with > > PHY-less lan743x > > (the interface even could not be brought up) > > > of course, there nothing much to do without TP port but the difference is > > representative. > > > > sbauer@metamini ~$ sudo ethtool eth7 > > Settings for eth7: > > Cannot get device settings: No such device > > > > Supports Wake-on: pumbag > > Wake-on: d > > Current message level: 0x00000137 (311) > > > > drv probe link ifdown ifup tx_queued > > > > Link detected: no > > > > sbauer@metamini ~$ sudo ifup eth7 > > sbauer@metamini ~$ sudo ethtool eth7 > > > > Settings for eth7: > > Supported ports: [ MII ] > > Supported link modes: 10baseT/Full > > > > 100baseT/Full > > 1000baseT/Full > > > > Supported pause frame use: Symmetric Receive-only > > Supports auto-negotiation: Yes > > Supported FEC modes: Not reported > > Advertised link modes: 10baseT/Full > > > > 100baseT/Full > > 1000baseT/Full > > > > Advertised pause frame use: Symmetric Receive-only > > Advertised auto-negotiation: Yes > > Advertised FEC modes: Not reported > > Speed: 1000Mb/s > > Duplex: Full > > Port: MII > > PHYAD: 0 > > Transceiver: internal > > Auto-negotiation: on > > Supports Wake-on: pumbag > > Wake-on: d > > Current message level: 0x00000137 (311) > > > > drv probe link ifdown ifup tx_queued > > > > Link detected: yes > > > > sbauer@metamini ~$ sudo mii-tool -vv eth7 > > Using SIOCGMIIPHY=0x8947 > > eth7: negotiated 1000baseT-FD, link ok > > > > registers for MII PHY 0: > > 5140 512d 7431 0011 4140 4140 000d 0000 > > 0000 0200 7800 0000 0000 0000 0000 2000 > > 0000 0000 0000 0000 0000 0000 0000 0000 > > 0000 0000 0000 0000 0000 0000 0000 0000 > > > > product info: vendor 1d:0c:40, model 1 rev 1 > > basic mode: loopback, autonegotiation enabled > > basic status: autonegotiation complete, link ok > > capabilities: 1000baseT-FD 100baseTx-FD 10baseT-FD > > advertising: 1000baseT-FD 100baseTx-FD 10baseT-FD > > link partner: 1000baseT-FD 100baseTx-FD 10baseT-FD > > You have not shown anything i cannot do with the ethernet interfaces i > have in my laptop. And since ethtool is pretty standardized, what > lan743x offers should be pretty much the same as any 1G Ethernet MAC > using most 1G PHYs. > > Andrew Andrew, I can reproduce segfault with following changes: sbauer@metamini ~/devel/kernel-works/net-next.git master$ git diff . diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ ethernet/microchip/lan743x_main.c index 3804310c853a..6e2961f47211 100644 --- a/drivers/net/ethernet/microchip/lan743x_main.c +++ b/drivers/net/ethernet/microchip/lan743x_main.c @@ -1001,6 +1001,8 @@ static void lan743x_phy_close(struct lan743x_adapter *adapter) phy_stop(netdev->phydev); phy_disconnect(netdev->phydev); + if (phy_is_pseudo_fixed_link(netdev->phydev)) + fixed_phy_unregister(netdev->phydev); netdev->phydev = NULL; } @@ -1018,9 +1020,21 @@ static int lan743x_phy_open(struct lan743x_adapter *adapter) if (!phydev) { /* try internal phy */ phydev = phy_find_first(adapter->mdiobus); - if (!phydev) - goto return_error; - + if (!phydev) { + struct fixed_phy_status status = { + .link = 1, + .speed = SPEED_1000, + .duplex = DUPLEX_FULL, + .asym_pause = 1, + }; + + phydev = fixed_phy_register(PHY_POLL, &status, NULL); + if (!phydev || IS_ERR(phydev)) { + netif_err(adapter, probe, netdev, + "Failed to register fixed PHY device\n"); + goto return_error; + } + } ret = phy_connect_direct(netdev, phydev, lan743x_phy_link_status_change, PHY_INTERFACE_MODE_GMII); sbauer@metamini ~/devel/kernel-works/net-next.git master$ sudo modprobe lan743x sbauer@metamini ~/devel/kernel-works/net-next.git master$ sudo ifup eth7 sbauer@metamini ~/devel/kernel-works/net-next.git master$ ethtool eth7 Settings for eth7: Supported ports: [ TP MII ] Supported link modes: 1000baseT/Full Supported pause frame use: Symmetric Receive-only Supports auto-negotiation: Yes Supported FEC modes: Not reported Advertised link modes: 1000baseT/Full Advertised pause frame use: Symmetric Receive-only Advertised auto-negotiation: Yes Advertised FEC modes: Not reported Speed: 1000Mb/s Duplex: Full Port: MII PHYAD: 0 Transceiver: internal Auto-negotiation: on Cannot get wake-on-lan settings: Operation not permitted Current message level: 0x00000137 (311) drv probe link ifdown ifup tx_queued Link detected: yes sbauer@metamini ~/devel/kernel-works/net-next.git master$ sudo ifdown eth7 Killed --- dmesg: [ 365.856455] lan743x 0000:06:00.0 eth7: Link is Down [ 365.856542] BUG: kernel NULL pointer dereference, address: 00000000000003c4 [ 365.856611] #PF: supervisor read access in kernel mode [ 365.856705] #PF: error_code(0x0000) - not-present page [ 365.856772] PGD 0 P4D 0 [ 365.856832] Oops: 0000 [#1] SMP NOPTI [ 365.856898] CPU: 0 PID: 21779 Comm: ip Tainted: G O 5.11.0- rc4net-next+ #4 [ 365.857018] Hardware name: Gigabyte Technology Co., Ltd. H110-D3/H110-D3- CF, BIOS F24 04/11/2018 [ 365.857111] RIP: 0010:lan743x_phy_close.isra.26+0x28/0x50 [lan743x] [ 365.857208] Code: c3 90 0f 1f 44 00 00 53 48 89 fb 48 8b bf 28 08 00 00 e8 ab 3e da ff 48 8b bb 28 08 00 00 e8 bf 67 da ff 48 8b bb 28 08 00 00 <f6> 87 c4 03 00 00 04 75 0d 48 c7 83 28 08 00 00 00 00 00 00 5b c3 [ 365.857332] RSP: 0018:ffffb6560468b508 EFLAGS: 00010246 [ 365.857397] RAX: 0000000000000003 RBX: ffff8cfacbe30000 RCX: 0000000000000000 [ 365.857466] RDX: ffffffffc00ae2d8 RSI: 0000000000000001 RDI: 0000000000000000 [ 365.857537] RBP: 0000000000000008 R08: 0000000000000000 R09: ffffffffb2d6dee0 [ 365.857632] R10: ffff8cfb45f69bff R11: ffff8cfb057c829b R12: ffff8cfacbe308c0 [ 365.857697] R13: ffff8cfacbe310b8 R14: ffff8cfacbe31050 R15: ffff8cfacbe308c0 [ 365.857762] FS: 00007f4318318e40(0000) GS:ffff8cfbf6c00000(0000) knlGS: 0000000000000000 [ 365.857844] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 365.857907] CR2: 00000000000003c4 CR3: 0000000145a80001 CR4: 00000000003706f0 [ 365.857972] Call Trace: [ 365.858029] lan743x_netdev_close+0x223/0x250 [lan743x] [ 365.858094] __dev_close_many+0x96/0x100 [ 365.858170] __dev_change_flags+0xdc/0x210 [ 365.858264] dev_change_flags+0x21/0x60 [ 365.858325] do_setlink+0x347/0x10e0 [ 365.858385] ? __nla_validate_parse+0x5f/0xaf0 [ 365.858447] __rtnl_newlink+0x541/0x8d0 [ 365.858508] ? get_page_from_freelist+0x1194/0x1310 [ 365.858574] ? mutex_spin_on_owner+0x5c/0xb0 [ 365.858635] ? __mutex_lock.isra.9+0x46e/0x4b0 [ 365.858696] ? asm_exc_page_fault+0x1e/0x30 [ 365.858757] rtnl_newlink+0x43/0x60 [ 365.858817] rtnetlink_rcv_msg+0x134/0x380 [ 365.858878] ? _cond_resched+0x15/0x30 [ 365.858937] ? kmem_cache_alloc+0x3cf/0x7d0 [ 365.858997] ? rtnl_calcit.isra.38+0x110/0x110 [ 365.859058] netlink_rcv_skb+0x50/0x100 [ 365.859118] netlink_unicast+0x1a5/0x280 [ 365.859178] netlink_sendmsg+0x23d/0x470 [ 365.859237] sock_sendmsg+0x5b/0x60 [ 365.859298] ____sys_sendmsg+0x1ef/0x260 [ 365.859358] ? copy_msghdr_from_user+0x5c/0x90 [ 365.859418] ? mntput_no_expire+0x47/0x240 [ 365.859478] ___sys_sendmsg+0x7c/0xc0 [ 365.859537] ? tomoyo_path_number_perm+0x68/0x1e0 [ 365.859600] ? __sk_destruct+0x129/0x1b0 [ 365.859660] ? var_wake_function+0x20/0x20 [ 365.859720] ? fsnotify_grab_connector+0x46/0x80 [ 365.859782] __sys_sendmsg+0x57/0xa0 [ 365.859841] do_syscall_64+0x33/0x80 [ 365.859900] entry_SYSCALL_64_after_hwframe+0x44/0xa9 --- (gdb) l *lan743x_netdev_close+0x223 0x1f43 is in lan743x_netdev_close (drivers/net/ethernet/microchip// lan743x_main.c:2521). 2516 2517 lan743x_ptp_close(adapter); 2518 2519 lan743x_phy_close(adapter); 2520 2521 lan743x_mac_close(adapter); 2522 2523 lan743x_intr_close(adapter); 2524 2525 return 0; (gdb)
On Saturday, January 23, 2021 4:32:38 AM MSK Andrew Lunn wrote: > > it migth be helpful for developers work on userspace networking tools with > > PHY-less lan743x > > (the interface even could not be brought up) > > > of course, there nothing much to do without TP port but the difference is > > representative. > > > > sbauer@metamini ~$ sudo ethtool eth7 > > Settings for eth7: > > Cannot get device settings: No such device > > > > Supports Wake-on: pumbag > > Wake-on: d > > Current message level: 0x00000137 (311) > > > > drv probe link ifdown ifup tx_queued > > > > Link detected: no > > > > sbauer@metamini ~$ sudo ifup eth7 > > sbauer@metamini ~$ sudo ethtool eth7 > > > > Settings for eth7: > > Supported ports: [ MII ] > > Supported link modes: 10baseT/Full > > > > 100baseT/Full > > 1000baseT/Full > > > > Supported pause frame use: Symmetric Receive-only > > Supports auto-negotiation: Yes > > Supported FEC modes: Not reported > > Advertised link modes: 10baseT/Full > > > > 100baseT/Full > > 1000baseT/Full > > > > Advertised pause frame use: Symmetric Receive-only > > Advertised auto-negotiation: Yes > > Advertised FEC modes: Not reported > > Speed: 1000Mb/s > > Duplex: Full > > Port: MII > > PHYAD: 0 > > Transceiver: internal > > Auto-negotiation: on > > Supports Wake-on: pumbag > > Wake-on: d > > Current message level: 0x00000137 (311) > > > > drv probe link ifdown ifup tx_queued > > > > Link detected: yes > > > > sbauer@metamini ~$ sudo mii-tool -vv eth7 > > Using SIOCGMIIPHY=0x8947 > > eth7: negotiated 1000baseT-FD, link ok > > > > registers for MII PHY 0: > > 5140 512d 7431 0011 4140 4140 000d 0000 > > 0000 0200 7800 0000 0000 0000 0000 2000 > > 0000 0000 0000 0000 0000 0000 0000 0000 > > 0000 0000 0000 0000 0000 0000 0000 0000 > > > > product info: vendor 1d:0c:40, model 1 rev 1 > > basic mode: loopback, autonegotiation enabled > > basic status: autonegotiation complete, link ok > > capabilities: 1000baseT-FD 100baseTx-FD 10baseT-FD > > advertising: 1000baseT-FD 100baseTx-FD 10baseT-FD > > link partner: 1000baseT-FD 100baseTx-FD 10baseT-FD > > You have not shown anything i cannot do with the ethernet interfaces i > have in my laptop. And since ethtool is pretty standardized, what > lan743x offers should be pretty much the same as any 1G Ethernet MAC > using most 1G PHYs. > > Andrew Andrew, for this moment with lan743x we can get only this: sbauer@metamini ~/devel/kernel-works/net-next.git master$ sudo ethtool eth7 Settings for eth7: Cannot get device settings: No such device Supports Wake-on: pumbag Wake-on: d Current message level: 0x00000137 (311) drv probe link ifdown ifup tx_queued Link detected: no sbauer@metamini ~/devel/kernel-works/net-next.git master$ sudo mii-tool -vv eth7 Using SIOCGMIIPHY=0x8947 SIOCGMIIPHY on 'eth7' failed: Invalid argument
On Saturday, January 23, 2021 4:39:47 AM MSK Florian Fainelli wrote: > On 1/22/2021 5:01 PM, Sergej Bauer wrote: > > On Saturday, January 23, 2021 3:01:47 AM MSK Florian Fainelli wrote: > >> On 1/22/2021 3:58 PM, Sergej Bauer wrote: > >>> On Saturday, January 23, 2021 2:23:25 AM MSK Andrew Lunn wrote: > >>>>>>> @@ -1000,8 +1005,10 @@ static void lan743x_phy_close(struct > >>>>>>> lan743x_adapter *adapter)> > >>>>>>> > >>>>>>> struct net_device *netdev = adapter->netdev; > >>>>>>> > >>>>>>> phy_stop(netdev->phydev); > >>>>>>> > >>>>>>> - phy_disconnect(netdev->phydev); > >>>>>>> - netdev->phydev = NULL; > >>>>>>> + if (phy_is_pseudo_fixed_link(netdev->phydev)) > >>>>>>> + lan743x_virtual_phy_disconnect(netdev->phydev); > >>>>>>> + else > >>>>>>> + phy_disconnect(netdev->phydev); > >>>>>> > >>>>>> phy_disconnect() should work. You might want to call > >>>> > >>>> There are drivers which call phy_disconnect() on a fixed_link. e.g. > >>>> > >>>> https://elixir.bootlin.com/linux/v5.11-rc4/source/drivers/net/usb/lan78 > >>>> xx > >>>> .c# L3555. > >>>> > >>>> > >>>> It could be your missing call to fixed_phy_unregister() is leaving > >>>> behind bad state. > >>> > >>> lan743x_virtual_phy_disconnect removes sysfs-links and calls > >>> fixed_phy_unregister() > >>> and the reason was phydev in sysfs. > >>> > >>>>> It was to make ethtool show full set of supported speeds and MII only > >>>>> in > >>>>> supported ports (without TP and the no any ports in the bare card). > >>>> > >>>> But fixed link does not support the full set of speed. It is fixed. It > >>>> supports only one speed it is configured with. > >>> > >>> That's why I "re-implemented the fixed PHY driver" as Florian said. > >>> The goal of virtual phy was to make an illusion of real device working > >>> in > >>> loopback mode. So I could use ethtool and ioctl's to switch speed of > >>> device.> > >>> > >>>> And by setting it > >>>> wrongly, you are going to allow the user to do odd things, like use > >>>> ethtool force the link speed to a speed which is not actually > >>>> supported. > >>> > >>> I have lan743x only and in loopback mode it allows to use speeds > >>> 10/100/1000MBps > >>> in full-duplex mode only. But the highest speed I have achived was > >>> something near > >>> 752Mbps... > >>> And I can switch speed on the fly, without reloading the module. > >>> > >>> May by I should limit the list of acceptable devices? > >> > >> It is not clear what your use case is so maybe start with explaining it > >> and we can help you define something that may be acceptable for upstream > >> inclusion. > > > > it migth be helpful for developers work on userspace networking tools with > > PHY-less lan743x (the interface even could not be brought up) > > of course, there nothing much to do without TP port but the difference is > > representative. > > You are still not explaining why fixed PHY is not a suitable emulation > and what is different, providing the output of ethtool does not tell me > how you are using it. > > Literally everyone using Ethernet switches or MAC to MAC Ethernet > connections uses fixed PHY and is reasonably happy with it. Florian, the key idea is to make virtual phy device which acts as real 802.11 standard phy. original fixed_phy and swphy are little bit useless as they do not support write operation. in contrast of them virtual_phy supports write to all registers. and can change the speed of interface by means of SIOCSMIIREG ioctl call either ethtool. changing of appropriate bits in register 0 will change speed reflecting in ethtool and vise versa.
On Saturday, January 23, 2021 2:23:25 AM MSK Andrew Lunn wrote: > > > > @@ -1000,8 +1005,10 @@ static void lan743x_phy_close(struct > > > > lan743x_adapter *adapter)> > > > > > > > > struct net_device *netdev = adapter->netdev; > > > > > > > > phy_stop(netdev->phydev); > > > > > > > > - phy_disconnect(netdev->phydev); > > > > - netdev->phydev = NULL; > > > > + if (phy_is_pseudo_fixed_link(netdev->phydev)) > > > > + lan743x_virtual_phy_disconnect(netdev->phydev); > > > > + else > > > > + phy_disconnect(netdev->phydev); > > > > > > phy_disconnect() should work. You might want to call > > There are drivers which call phy_disconnect() on a fixed_link. e.g. > > https://elixir.bootlin.com/linux/v5.11-rc4/source/drivers/net/usb/lan78xx.c# > L3555. > > It could be your missing call to fixed_phy_unregister() is leaving > behind bad state. > fixed_phy_unregister() were inside of lan743x_virtual_phy_disconnect() > > It was to make ethtool show full set of supported speeds and MII only in > > supported ports (without TP and the no any ports in the bare card). > > But fixed link does not support the full set of speed. It is fixed. It > supports only one speed it is configured with. And by setting it > wrongly, you are going to allow the user to do odd things, like use > ethtool force the link speed to a speed which is not actually > supported. when writing virtual phy i have used "Microchip AN2948 Configuration Registers of LAN743x" document and the lan743x is designed only for LAN7430 either LAN7431 as it pointed in the document and in lan743x_pcidev_tbl. which both support speeds 10/100/1000 Mbps.
diff --git a/MAINTAINERS b/MAINTAINERS index 650deb973913..86304efd7691 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -11699,6 +11699,12 @@ L: netdev@vger.kernel.org S: Maintained F: drivers/net/ethernet/microchip/lan743x_* +MICROCHIP LAN743X VIRTUAL PHY +M: Sergej Bauer <sbauer@blackbox.su> +L: netdev@vger.kernel.org +S: Maintained +F: drivers/net/ethernet/microchip/lan743x_virtual_phy.* + MICROCHIP LCDFB DRIVER M: Nicolas Ferre <nicolas.ferre@microchip.com> L: linux-fbdev@vger.kernel.org diff --git a/drivers/net/ethernet/microchip/Kconfig b/drivers/net/ethernet/microchip/Kconfig index d0f6dfe0dcf3..fbc94cf115bd 100644 --- a/drivers/net/ethernet/microchip/Kconfig +++ b/drivers/net/ethernet/microchip/Kconfig @@ -48,6 +48,7 @@ config LAN743X select PHYLIB select CRC16 select CRC32 + select FIXED_PHY help Support for the Microchip LAN743x PCI Express Gigabit Ethernet chip diff --git a/drivers/net/ethernet/microchip/Makefile b/drivers/net/ethernet/microchip/Makefile index da603540ca57..dc3a2d66c286 100644 --- a/drivers/net/ethernet/microchip/Makefile +++ b/drivers/net/ethernet/microchip/Makefile @@ -7,4 +7,4 @@ obj-$(CONFIG_ENC28J60) += enc28j60.o obj-$(CONFIG_ENCX24J600) += encx24j600.o encx24j600-regmap.o obj-$(CONFIG_LAN743X) += lan743x.o -lan743x-objs := lan743x_main.o lan743x_ethtool.o lan743x_ptp.o +lan743x-objs := lan743x_main.o lan743x_ethtool.o lan743x_ptp.o lan743x_virtual_phy.o diff --git a/drivers/net/ethernet/microchip/lan743x_ethtool.c b/drivers/net/ethernet/microchip/lan743x_ethtool.c index c5de8f46cdd3..22636366d0db 100644 --- a/drivers/net/ethernet/microchip/lan743x_ethtool.c +++ b/drivers/net/ethernet/microchip/lan743x_ethtool.c @@ -816,6 +816,17 @@ static int lan743x_ethtool_set_wol(struct net_device *netdev, } #endif /* CONFIG_PM */ +int lan743x_set_link_ksettings(struct net_device *netdev, + const struct ethtool_link_ksettings *cmd) +{ + if (!netdev->phydev) + return -ENETDOWN; + + return phy_is_pseudo_fixed_link(netdev->phydev) ? + lan743x_set_virtual_link_ksettings(netdev, cmd) + : phy_ethtool_set_link_ksettings(netdev, cmd); +} + const struct ethtool_ops lan743x_ethtool_ops = { .get_drvinfo = lan743x_ethtool_get_drvinfo, .get_msglevel = lan743x_ethtool_get_msglevel, @@ -839,7 +850,7 @@ const struct ethtool_ops lan743x_ethtool_ops = { .get_eee = lan743x_ethtool_get_eee, .set_eee = lan743x_ethtool_set_eee, .get_link_ksettings = phy_ethtool_get_link_ksettings, - .set_link_ksettings = phy_ethtool_set_link_ksettings, + .set_link_ksettings = lan743x_set_link_ksettings, #ifdef CONFIG_PM .get_wol = lan743x_ethtool_get_wol, .set_wol = lan743x_ethtool_set_wol, diff --git a/drivers/net/ethernet/microchip/lan743x_ethtool.h b/drivers/net/ethernet/microchip/lan743x_ethtool.h index d0d11a777a58..7287474d4a5c 100644 --- a/drivers/net/ethernet/microchip/lan743x_ethtool.h +++ b/drivers/net/ethernet/microchip/lan743x_ethtool.h @@ -8,4 +8,7 @@ extern const struct ethtool_ops lan743x_ethtool_ops; +int lan743x_set_virtual_link_ksettings(struct net_device *netdev, + const struct ethtool_link_ksettings *cmd); + #endif /* _LAN743X_ETHTOOL_H */ diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c index 3804310c853a..d8c00fa298d0 100644 --- a/drivers/net/ethernet/microchip/lan743x_main.c +++ b/drivers/net/ethernet/microchip/lan743x_main.c @@ -17,6 +17,11 @@ #include <linux/crc16.h> #include "lan743x_main.h" #include "lan743x_ethtool.h" +#include "lan743x_virtual_phy.h" + +static char *mii_regs[32]; +static int mii_regs_count; +module_param_array(mii_regs, charp, &mii_regs_count, 0644); static void lan743x_pci_cleanup(struct lan743x_adapter *adapter) { @@ -821,7 +826,7 @@ static int lan743x_mac_init(struct lan743x_adapter *adapter) return 0; } -static int lan743x_mac_open(struct lan743x_adapter *adapter) +int lan743x_mac_open(struct lan743x_adapter *adapter) { u32 temp; @@ -832,7 +837,7 @@ static int lan743x_mac_open(struct lan743x_adapter *adapter) return 0; } -static void lan743x_mac_close(struct lan743x_adapter *adapter) +void lan743x_mac_close(struct lan743x_adapter *adapter) { u32 temp; @@ -1000,8 +1005,10 @@ static void lan743x_phy_close(struct lan743x_adapter *adapter) struct net_device *netdev = adapter->netdev; phy_stop(netdev->phydev); - phy_disconnect(netdev->phydev); - netdev->phydev = NULL; + if (phy_is_pseudo_fixed_link(netdev->phydev)) + lan743x_virtual_phy_disconnect(netdev->phydev); + else + phy_disconnect(netdev->phydev); } static int lan743x_phy_open(struct lan743x_adapter *adapter) @@ -1019,11 +1026,22 @@ static int lan743x_phy_open(struct lan743x_adapter *adapter) /* try internal phy */ phydev = phy_find_first(adapter->mdiobus); if (!phydev) + phydev = lan743x_virtual_phy(adapter, mii_regs, + mii_regs_count); + + if (!phydev || IS_ERR(phydev)) goto return_error; - ret = phy_connect_direct(netdev, phydev, - lan743x_phy_link_status_change, - PHY_INTERFACE_MODE_GMII); + if (phy_is_pseudo_fixed_link(phydev)) { + ret = phy_connect_direct(netdev, phydev, + lan743x_virtual_phy_status_change, + PHY_INTERFACE_MODE_MII); + } else { + ret = phy_connect_direct(netdev, phydev, + lan743x_phy_link_status_change, + PHY_INTERFACE_MODE_GMII); + } + if (ret) goto return_error; } @@ -1031,6 +1049,15 @@ static int lan743x_phy_open(struct lan743x_adapter *adapter) /* MAC doesn't support 1000T Half */ phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_1000baseT_Half_BIT); + if (phy_is_pseudo_fixed_link(phydev)) { + phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_TP_BIT); + linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, + phydev->supported); + linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, + phydev->supported); + phy_advertise_supported(phydev); + } + /* support both flow controls */ phy_support_asym_pause(phydev); phy->fc_request_control = (FLOW_CTRL_RX | FLOW_CTRL_TX); @@ -2580,11 +2607,20 @@ static netdev_tx_t lan743x_netdev_xmit_frame(struct sk_buff *skb, static int lan743x_netdev_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd) { + int ret; + if (!netif_running(netdev)) return -EINVAL; + if (cmd == SIOCSHWTSTAMP) return lan743x_ptp_ioctl(netdev, ifr, cmd); - return phy_mii_ioctl(netdev->phydev, ifr, cmd); + + if (phy_is_pseudo_fixed_link(netdev->phydev)) + ret = lan743x_virtual_phy_mii_ioctl(netdev->phydev, ifr, cmd); + else + ret = phy_mii_ioctl(netdev->phydev, ifr, cmd); + + return ret; } static void lan743x_netdev_set_multicast(struct net_device *netdev) diff --git a/drivers/net/ethernet/microchip/lan743x_main.h b/drivers/net/ethernet/microchip/lan743x_main.h index 404af3f4635e..8e16fd0f166a 100644 --- a/drivers/net/ethernet/microchip/lan743x_main.h +++ b/drivers/net/ethernet/microchip/lan743x_main.h @@ -109,6 +109,7 @@ #define MAC_CR_EEE_EN_ BIT(17) #define MAC_CR_ADD_ BIT(12) #define MAC_CR_ASD_ BIT(11) +#define MAC_CR_INT_LOOPBACK_ BIT(10) #define MAC_CR_CNTR_RST_ BIT(5) #define MAC_CR_DPX_ BIT(3) #define MAC_CR_CFG_H_ BIT(2) diff --git a/drivers/net/ethernet/microchip/lan743x_virtual_phy.c b/drivers/net/ethernet/microchip/lan743x_virtual_phy.c new file mode 100644 index 000000000000..b6bf7c016448 --- /dev/null +++ b/drivers/net/ethernet/microchip/lan743x_virtual_phy.c @@ -0,0 +1,335 @@ +// SPDX-License-Identifier: GPL-2.0+ +// Copyright (C) 2021 Sergej Bauer +#include <linux/netdevice.h> +#include <linux/phy.h> +#include <linux/phy_fixed.h> +#include <linux/ethtool.h> +#include <linux/rtnetlink.h> +#include "lan743x_main.h" +#include "lan743x_ethtool.h" +#include "lan743x_virtual_phy.h" + +static u16 regs802p3[32]; + +int lan743x_virtual_phy_mii_ioctl(struct phy_device *phydev, struct ifreq *ifr, + int cmd) +{ + struct ethtool_link_ksettings ksettings; + struct net_device *netdev = phydev->attached_dev; + struct mii_ioctl_data *mii_data = if_mii(ifr); + int ret = 0; + u16 val = mii_data->val_in; + u16 reg_num = mii_data->reg_num; + + if (reg_num > 0x1f) + return -ERANGE; + + memset(&ksettings, 0, sizeof(ksettings)); + phy_ethtool_get_link_ksettings(netdev, &ksettings); + ksettings.base.autoneg = AUTONEG_ENABLE; + + switch (cmd) { + case SIOCGMIIPHY: + break; + case SIOCGMIIREG: + mii_data->val_out = regs802p3[reg_num]; + break; + case SIOCSMIIREG: + switch (reg_num) { + case MII_BMCR: + val &= ~BMCR_RESET; + if (!(val & BMCR_FULLDPLX)) + val |= BMCR_FULLDPLX; + val &= ~0x1f; + + if (val & BMCR_ANRESTART) + val &= ~BMCR_ANRESTART; + + if ((val & BMCR_SPEED1000) && (val & BMCR_SPEED100)) { + ret = -EINVAL; + netdev_err(netdev, "invalid bits 0.6 and 0.13"); + break; + } + + ksettings.base.duplex = DUPLEX_FULL; + + if (val & BMCR_SPEED1000) + ksettings.base.speed = SPEED_1000; + else if (val & BMCR_SPEED100) + ksettings.base.speed = SPEED_100; + else + ksettings.base.speed = SPEED_10; + + break; + + case 1: + case 2: + case 3: + case 15: + default: + regs802p3[reg_num] = val; + break; + } + ret = lan743x_set_virtual_link_ksettings(netdev, &ksettings); + if (ret == 0) + regs802p3[reg_num] = val; + + break; + default: + netdev_err(netdev, "not supported ioctl %X\n", cmd); + ret = -EOPNOTSUPP; + }; + + return ret; +} + +void lan743x_virtual_phy_disconnect(struct phy_device *phydev) +{ + struct net_device *attached_dev = phydev->attached_dev; + + if (phydev->sysfs_links) { + if (phydev->attached_dev) + sysfs_remove_link(&attached_dev->dev.kobj, + "phydev"); + sysfs_remove_link(&phydev->mdio.dev.kobj, + "attached_dev"); + } + + fixed_phy_unregister(phydev); +} + +void lan743x_virtual_phy_status_change(struct net_device *netdev) +{ + struct lan743x_adapter *adapter = netdev_priv(netdev); + struct phy_device *phydev = netdev->phydev; + u32 data; + + if (!netdev->phydev) + return; + + if (phydev->state == PHY_RUNNING) { + data = lan743x_csr_read(adapter, MAC_CR); + data |= MAC_CR_INT_LOOPBACK_; + data &= ~MAC_CR_MII_EN_; + data |= MAC_CR_DPX_; + + switch (phydev->speed) { + case SPEED_10: + data &= ~MAC_CR_CFG_H_; + data &= ~MAC_CR_CFG_L_; + break; + case SPEED_100: + data &= ~MAC_CR_CFG_H_; + data |= MAC_CR_CFG_L_; + break; + case SPEED_1000: + data |= MAC_CR_CFG_H_; + data &= ~MAC_CR_CFG_L_; + break; + } + lan743x_csr_write(adapter, MAC_CR, data); + } + phy_print_status(phydev); +} + +struct phy_device *lan743x_virtual_phy(struct lan743x_adapter *adapter, + char *mii_regs[], int mii_regs_count) +{ + struct phy_device *phydev = NULL; + struct net_device *netdev = adapter->netdev; + struct mii_ioctl_data mii_data; + struct ifreq ifr; + long res; + int ret, i; + + struct fixed_phy_status status = { + .link = 1, + .speed = SPEED_1000, + .duplex = DUPLEX_FULL, + .asym_pause = 1, + }; + + phydev = fixed_phy_register(PHY_POLL, &status, NULL); + if (!phydev || IS_ERR(phydev)) { + netif_err(adapter, ifup, adapter->netdev, + "failed to regiter fixed_phy\n"); + goto out; + } + phydev->attached_dev = netdev; + netdev->phydev = phydev; + + linkmode_clear_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT, + phydev->supported); + linkmode_clear_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, + phydev->supported); + linkmode_clear_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT, + phydev->supported); + + if (mii_regs_count > 0) { + for (i = 0; i < mii_regs_count; i++) { + mii_data.reg_num = i; + ret = kstrtol(mii_regs[i], 16, &res); + + if (ret == 0) + mii_data.val_in = res; + else + mii_data.val_in = 0xffff; + + memcpy(&ifr.ifr_ifru.ifru_data, &mii_data, + sizeof(mii_data)); + ret = lan743x_virtual_phy_mii_ioctl(phydev, &ifr, + SIOCSMIIREG); + if (ret < 0) + return NULL; + } + } else { + mii_data.reg_num = MII_BMSR; + mii_data.val_in = BMSR_100FULL | BMSR_10FULL | BMSR_ESTATEN | + BMSR_ANEGCOMPLETE | BMSR_ANEGCAPABLE | BMSR_LSTATUS | + BMSR_ERCAP; + memcpy(&ifr.ifr_ifru.ifru_data, &mii_data, sizeof(mii_data)); + lan743x_virtual_phy_mii_ioctl(phydev, &ifr, SIOCSMIIREG); + + mii_data.reg_num = MII_PHYSID1; + mii_data.val_in = (adapter->csr.id_rev & ID_REV_ID_MASK_) >> 16; + memcpy(&ifr.ifr_ifru.ifru_data, &mii_data, sizeof(mii_data)); + lan743x_virtual_phy_mii_ioctl(phydev, &ifr, SIOCSMIIREG); + + mii_data.reg_num = MII_PHYSID2; + mii_data.val_in = adapter->csr.id_rev & ID_REV_CHIP_REV_MASK_; + memcpy(&ifr.ifr_ifru.ifru_data, &mii_data, sizeof(mii_data)); + lan743x_virtual_phy_mii_ioctl(phydev, &ifr, SIOCSMIIREG); + + mii_data.reg_num = MII_ADVERTISE; + mii_data.val_in = ADVERTISE_100FULL | ADVERTISE_10FULL | + ADVERTISE_LPACK; + memcpy(&ifr.ifr_ifru.ifru_data, &mii_data, sizeof(mii_data)); + lan743x_virtual_phy_mii_ioctl(phydev, &ifr, SIOCSMIIREG); + + mii_data.reg_num = MII_LPA; + mii_data.val_in = ADVERTISE_100FULL | ADVERTISE_10FULL | + ADVERTISE_LPACK; + memcpy(&ifr.ifr_ifru.ifru_data, &mii_data, sizeof(mii_data)); + lan743x_virtual_phy_mii_ioctl(phydev, &ifr, SIOCSMIIREG); + + mii_data.reg_num = MII_EXPANSION; + mii_data.val_in = EXPANSION_NWAY | EXPANSION_ENABLENPAGE | + EXPANSION_NPCAPABLE; + memcpy(&ifr.ifr_ifru.ifru_data, &mii_data, sizeof(mii_data)); + lan743x_virtual_phy_mii_ioctl(phydev, &ifr, SIOCSMIIREG); + + mii_data.reg_num = MII_CTRL1000; + mii_data.val_in = ADVERTISE_1000FULL; + memcpy(&ifr.ifr_ifru.ifru_data, &mii_data, sizeof(mii_data)); + lan743x_virtual_phy_mii_ioctl(phydev, &ifr, SIOCSMIIREG); + + mii_data.reg_num = MII_STAT1000; + mii_data.val_in = LPA_1000FULL | LPA_1000REMRXOK | + LPA_1000LOCALRXOK | LPA_1000MSRES; + memcpy(&ifr.ifr_ifru.ifru_data, &mii_data, sizeof(mii_data)); + lan743x_virtual_phy_mii_ioctl(phydev, &ifr, SIOCSMIIREG); + + mii_data.reg_num = MII_ESTATUS; + mii_data.val_in = ESTATUS_1000_TFULL; + memcpy(&ifr.ifr_ifru.ifru_data, &mii_data, sizeof(mii_data)); + lan743x_virtual_phy_mii_ioctl(phydev, &ifr, SIOCSMIIREG); + + mii_data.reg_num = MII_BMCR; + mii_data.val_in = BMCR_SPEED1000 | BMCR_FULLDPLX | + BMCR_LOOPBACK | BMCR_ANENABLE; + memcpy(&ifr.ifr_ifru.ifru_data, &mii_data, sizeof(mii_data)); + lan743x_virtual_phy_mii_ioctl(phydev, &ifr, SIOCSMIIREG); + } + + phydev->attached_dev = NULL; +out: + return phydev; +} + +int lan743x_set_virtual_link_ksettings(struct net_device *netdev, + const struct ethtool_link_ksettings *cmd) +{ + struct ethtool_link_ksettings *cmd_ref = + (struct ethtool_link_ksettings *)cmd; + struct lan743x_adapter *adapter = netdev_priv(netdev); + u32 speed = cmd->base.speed; + u8 duplex = cmd->base.duplex; + u16 v = 0; + u32 data; + + if (!netdev->phydev) + return -ENETDOWN; + if (speed && + speed != SPEED_10 && speed != SPEED_100 && speed != SPEED_1000) { + netdev_err(netdev, "%s: unknown speed %d\n", netdev->name, + speed); + return -EINVAL; + } + + v = regs802p3[0]; + v &= ~(BMCR_SPEED1000 | BMCR_SPEED100); + data = lan743x_csr_read(adapter, MAC_CR); + duplex = DUPLEX_FULL; + + if (speed != adapter->netdev->phydev->speed) { + lan743x_mac_close(adapter); + switch (speed) { + case 0: + break; + case SPEED_10: + data &= ~(MAC_CR_CFG_H_ | MAC_CR_CFG_L_); + lan743x_csr_write(adapter, MAC_CR, data); + + regs802p3[MII_ADVERTISE] = ADVERTISE_10FULL | + ADVERTISE_100FULL | + ADVERTISE_LPACK; + regs802p3[MII_LPA] = LPA_10FULL | LPA_LPACK; + regs802p3[MII_EXPANSION] = ESTATUS_1000_TFULL; + regs802p3[MII_CTRL1000] = ADVERTISE_1000FULL; + regs802p3[MII_STAT1000] = 0; + break; + case SPEED_100: + v |= BMCR_SPEED100; + data &= ~MAC_CR_CFG_H_; + data |= MAC_CR_CFG_L_; + lan743x_csr_write(adapter, MAC_CR, data); + + regs802p3[MII_ADVERTISE] = ADVERTISE_10FULL | + ADVERTISE_100FULL | + ADVERTISE_LPACK; + regs802p3[MII_LPA] = LPA_10FULL | LPA_100FULL | + LPA_LPACK; + regs802p3[MII_EXPANSION] = ESTATUS_1000_TFULL; + regs802p3[MII_CTRL1000] = ADVERTISE_1000FULL; + regs802p3[MII_STAT1000] = 0; + break; + case SPEED_1000: + v |= BMCR_SPEED1000; + data |= MAC_CR_CFG_H_; + data &= ~MAC_CR_CFG_L_; + lan743x_csr_write(adapter, MAC_CR, data); + + regs802p3[MII_ADVERTISE] = ADVERTISE_10FULL | + ADVERTISE_100FULL | + ADVERTISE_LPACK; + regs802p3[MII_LPA] = LPA_10FULL | LPA_100FULL | + LPA_LPACK; + regs802p3[MII_EXPANSION] = ESTATUS_1000_TFULL; + regs802p3[MII_CTRL1000] = ADVERTISE_1000FULL; + regs802p3[MII_STAT1000] = LPA_1000FULL | + LPA_1000REMRXOK | LPA_1000LOCALRXOK | + LPA_1000MSRES; + break; + }; + lan743x_mac_open(adapter); + + v |= BMCR_LOOPBACK | BMCR_FULLDPLX | BMCR_ANENABLE; + regs802p3[0] = v; + } + + cmd_ref->base.duplex = duplex; + cmd_ref->base.autoneg = AUTONEG_ENABLE; + + return phy_ethtool_set_link_ksettings(netdev, cmd_ref); +} + diff --git a/drivers/net/ethernet/microchip/lan743x_virtual_phy.h b/drivers/net/ethernet/microchip/lan743x_virtual_phy.h new file mode 100644 index 000000000000..caef07d73d74 --- /dev/null +++ b/drivers/net/ethernet/microchip/lan743x_virtual_phy.h @@ -0,0 +1,20 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* Copyright (C) 2021 Sergej Bauer */ +#ifndef LAN743X_FAKE_PHY_H +#define LAN743X_FAKE_PHY_H + +#include "lan743x_main.h" + +struct phy_device *lan743x_virtual_phy(struct lan743x_adapter *adapter, + char *mii_regs[], int mii_regs_count); +void lan743x_virtual_phy_disconnect(struct phy_device *phydev); +int lan743x_virtual_phy_mii_ioctl(struct phy_device *phydev, struct ifreq *ifr, + int cmd); +void lan743x_virtual_phy_status_change(struct net_device *netdev); +int lan743x_set_virtual_link_ksettings(struct net_device *netdev, + const struct ethtool_link_ksettings *cmd); + +int lan743x_mac_open(struct lan743x_adapter *adapter); +void lan743x_mac_close(struct lan743x_adapter *adapter); + +#endif