* Re: Moving Octeon ethernet out of staging
2023-01-08 20:59 ` Chris Packham
@ 2023-04-08 14:20 ` Ladislav Michl
0 siblings, 0 replies; 3+ messages in thread
From: Ladislav Michl @ 2023-04-08 14:20 UTC (permalink / raw)
To: Chris Packham
Cc: linux-mips@vger.kernel.org, Aaro Koskinen, YunQiang Su,
Anushka Singh, Lukasz Majczak, Peter Swain
Hi Chris,
[snip]
On Sun, Jan 08, 2023 at 08:59:55PM +0000, Chris Packham wrote:
> From our end we still have our Octeon MIPS based routers which we still
> want to support but for various reasons these aren't seeing the kernel
> updates that I might like. And because these are technically legacy I'm
> having a hard time convincing anyone to dedicate any resources to
> helping with any effort to improve these drivers (whatever is happening
> is just "spare time" work).
At this end, the intention is still to use mainline kernel. So far the
show-stoppers are SFP and AGL support. Therefore TODO list looks like:
1. Covert driver to use PHYLINK (see attached patch)
2. Implement phylink_mac_ops using code from arch/mips/cavium-octeon/executive
(cvmx-helper-rgmii.c, cvmx-helper-sgmii.c, etc...)
3. Now it should be easy to add AGL after DT representation is decided.
4. Fix GPIO driver as it does not currently service interrupts, causing
troubles to both SFP and MDIO hanging there.
5. Let people test it.
6. Move out of staging ;-)
> As a user I can say that we'd probably be OK if we had to make some dts
> updates as we ship our kernel+dtb+initrd as a FIT image so we can update
> the kernel+dtb at the same time if necessary.
Indeed, it all boils down to DT representation. I'd probably go with
topology, interfaces and port register address taken from device tree.
Patch bellow does not make things worse and you'll get SFP for free.
There are some glitches, for example setting ports up:
octeon_ethernet 11800a0000000.pip eth0: configuring for inband/1000base-x link mode
octeon_ethernet 11800a0000000.pip eth1: PHY [8001180000001800:00] driver [Generic PHY] (irq=POLL)
octeon_ethernet 11800a0000000.pip eth1: configuring for phy/sgmii link mode
octeon_ethernet 11800a0000000.pip eth2: PHY [8001180000001800:01] driver [Generic PHY] (irq=POLL)
octeon_ethernet 11800a0000000.pip eth2: configuring for phy/sgmii link mode
octeon_ethernet 11800a0000000.pip eth0: switched to inband/sgmii link mode
octeon_ethernet 11800a0000000.pip eth0: PHY [i2c:sfp:16] driver [Marvell 88E1111] (irq=POLL)
octeon_ethernet 11800a0000000.pip eth3: PHY [8001180000001800:02] driver [Marvell 88E1340S] (irq=25)
octeon_ethernet 11800a0000000.pip eth3: configuring for phy/sgmii link mode
octeon_ethernet 11800a0000000.pip eth4: PHY [8001180000001800:03] driver [Marvell 88E1340S] (irq=25)
octeon_ethernet 11800a0000000.pip eth4: configuring for phy/sgmii link mode
octeon_ethernet 11800a0000000.pip eth1: Link is Up - 100Mbps/Full - flow control off
I do not know why are eth1 and eth2 attached to Generic PHY, while
the rest to the Marvell 88E1111 - all nodes are the same.
Comments welcome,
ladis
From: Ladislav Michl <ladis@linux-mips.org>
Subject: [PATCH] staging: octeon: phylink
---
drivers/staging/octeon/Kconfig | 2 +-
drivers/staging/octeon/ethernet-mdio.c | 171 ++++++++++++++---------
drivers/staging/octeon/ethernet-rgmii.c | 13 +-
drivers/staging/octeon/ethernet.c | 55 +++-----
drivers/staging/octeon/octeon-ethernet.h | 8 +-
5 files changed, 131 insertions(+), 118 deletions(-)
diff --git a/drivers/staging/octeon/Kconfig b/drivers/staging/octeon/Kconfig
index 5319909eb2f6..fda90025710d 100644
--- a/drivers/staging/octeon/Kconfig
+++ b/drivers/staging/octeon/Kconfig
@@ -3,7 +3,7 @@ config OCTEON_ETHERNET
tristate "Cavium Networks Octeon Ethernet support"
depends on CAVIUM_OCTEON_SOC || COMPILE_TEST
depends on NETDEVICES
- select PHYLIB
+ select PHYLINK
select MDIO_OCTEON
help
This driver supports the builtin ethernet ports on Cavium
diff --git a/drivers/staging/octeon/ethernet-mdio.c b/drivers/staging/octeon/ethernet-mdio.c
index b3049108edc4..a14fb4dbb2fd 100644
--- a/drivers/staging/octeon/ethernet-mdio.c
+++ b/drivers/staging/octeon/ethernet-mdio.c
@@ -9,7 +9,7 @@
#include <linux/ethtool.h>
#include <linux/phy.h>
#include <linux/ratelimit.h>
-#include <linux/of_mdio.h>
+#include <linux/of_net.h>
#include <generated/utsrelease.h>
#include <net/dst.h>
@@ -26,23 +26,27 @@ static void cvm_oct_get_drvinfo(struct net_device *dev,
strscpy(info->bus_info, "Builtin", sizeof(info->bus_info));
}
-static int cvm_oct_nway_reset(struct net_device *dev)
+static int cvm_oct_get_link_ksettings(struct net_device *dev,
+ struct ethtool_link_ksettings *cmd)
{
- if (!capable(CAP_NET_ADMIN))
- return -EPERM;
+ struct octeon_ethernet *priv = netdev_priv(dev);
+
+ return phylink_ethtool_ksettings_get(priv->phylink, cmd);
+}
- if (dev->phydev)
- return phy_start_aneg(dev->phydev);
+static int cvm_oct_set_link_ksettings(struct net_device *dev,
+ const struct ethtool_link_ksettings *cmd)
+{
+ struct octeon_ethernet *priv = netdev_priv(dev);
- return -EINVAL;
+ return phylink_ethtool_ksettings_set(priv->phylink, cmd);
}
const struct ethtool_ops cvm_oct_ethtool_ops = {
.get_drvinfo = cvm_oct_get_drvinfo,
- .nway_reset = cvm_oct_nway_reset,
.get_link = ethtool_op_get_link,
- .get_link_ksettings = phy_ethtool_get_link_ksettings,
- .set_link_ksettings = phy_ethtool_set_link_ksettings,
+ .get_link_ksettings = cvm_oct_get_link_ksettings,
+ .set_link_ksettings = cvm_oct_set_link_ksettings,
};
/**
@@ -55,53 +59,80 @@ const struct ethtool_ops cvm_oct_ethtool_ops = {
*/
int cvm_oct_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
{
- if (!netif_running(dev))
- return -EINVAL;
+ struct octeon_ethernet *priv = netdev_priv(dev);
- if (!dev->phydev)
- return -EINVAL;
+ return phylink_mii_ioctl(priv->phylink, rq, cmd);
+}
- return phy_mii_ioctl(dev->phydev, rq, cmd);
+static void cvm_oct_mac_get_state(struct phylink_config *config,
+ struct phylink_link_state *state)
+{
+ union cvmx_helper_link_info link_info;
+ struct net_device *dev = to_net_dev(config->dev);
+ struct octeon_ethernet *priv = netdev_priv(dev);
+
+ link_info = cvmx_helper_link_get(priv->port);
+ state->link = link_info.s.link_up;
+ state->duplex = link_info.s.full_duplex ? DUPLEX_FULL : DUPLEX_HALF;
+ state->speed = link_info.s.speed;
}
-void cvm_oct_note_carrier(struct octeon_ethernet *priv,
- union cvmx_helper_link_info li)
+static void cvm_oct_mac_config(struct phylink_config *config,
+ unsigned int mode,
+ const struct phylink_link_state *state)
{
- if (li.s.link_up) {
- pr_notice_ratelimited("%s: %u Mbps %s duplex, port %d, queue %d\n",
- netdev_name(priv->netdev), li.s.speed,
- (li.s.full_duplex) ? "Full" : "Half",
- priv->port, priv->queue);
- } else {
- pr_notice_ratelimited("%s: Link down\n",
- netdev_name(priv->netdev));
- }
}
-void cvm_oct_adjust_link(struct net_device *dev)
+static void cvm_oct_mac_link_down(struct phylink_config *config,
+ unsigned int mode, phy_interface_t interface)
{
+ union cvmx_helper_link_info link_info;
+ struct net_device *dev = to_net_dev(config->dev);
struct octeon_ethernet *priv = netdev_priv(dev);
+
+ link_info.u64 = 0;
+ link_info.s.link_up = 0;
+ link_info.s.full_duplex = 0;
+ link_info.s.speed = 0;
+ priv->link_info = link_info.u64;
+
+ cvmx_helper_link_set(priv->port, link_info);
+
+ priv->poll_used = false;
+}
+
+static void cvm_oct_mac_link_up(struct phylink_config *config,
+ struct phy_device *phy,
+ unsigned int mode, phy_interface_t interface,
+ int speed, int duplex,
+ bool tx_pause, bool rx_pause)
+{
union cvmx_helper_link_info link_info;
+ struct net_device *dev = to_net_dev(config->dev);
+ struct octeon_ethernet *priv = netdev_priv(dev);
link_info.u64 = 0;
- link_info.s.link_up = dev->phydev->link ? 1 : 0;
- link_info.s.full_duplex = dev->phydev->duplex ? 1 : 0;
- link_info.s.speed = dev->phydev->speed;
+ link_info.s.link_up = 1;
+ link_info.s.full_duplex = duplex == DUPLEX_FULL ? 1 : 0;
+ link_info.s.speed = speed;
priv->link_info = link_info.u64;
- /*
- * The polling task need to know about link status changes.
- */
- if (priv->poll)
- priv->poll(dev);
+ cvmx_helper_link_set(priv->port, link_info);
- if (priv->last_link != dev->phydev->link) {
- priv->last_link = dev->phydev->link;
- cvmx_helper_link_set(priv->port, link_info);
- cvm_oct_note_carrier(priv, link_info);
+ if (!phy && priv->poll) {
+ priv->poll_used = true;
+ priv->poll(dev);
}
}
+static const struct phylink_mac_ops cvm_oct_phylink_ops = {
+ .validate = phylink_generic_validate,
+ .mac_pcs_get_state = cvm_oct_mac_get_state,
+ .mac_config = cvm_oct_mac_config,
+ .mac_link_down = cvm_oct_mac_link_down,
+ .mac_link_up = cvm_oct_mac_link_up,
+};
+
int cvm_oct_common_stop(struct net_device *dev)
{
struct octeon_ethernet *priv = netdev_priv(dev);
@@ -116,15 +147,14 @@ int cvm_oct_common_stop(struct net_device *dev)
priv->poll = NULL;
- if (dev->phydev)
- phy_disconnect(dev->phydev);
+ phylink_stop(priv->phylink);
+ phylink_disconnect_phy(priv->phylink);
if (priv->last_link) {
link_info.u64 = 0;
priv->last_link = 0;
cvmx_helper_link_set(priv->port, link_info);
- cvm_oct_note_carrier(priv, link_info);
}
return 0;
}
@@ -138,34 +168,45 @@ int cvm_oct_common_stop(struct net_device *dev)
*/
int cvm_oct_phy_setup_device(struct net_device *dev)
{
+ phy_interface_t phy_mode;
+ struct phylink *phylink;
struct octeon_ethernet *priv = netdev_priv(dev);
- struct device_node *phy_node;
- struct phy_device *phydev = NULL;
- if (!priv->of_node)
- goto no_phy;
-
- phy_node = of_parse_phandle(priv->of_node, "phy-handle", 0);
- if (!phy_node && of_phy_is_fixed_link(priv->of_node))
- phy_node = of_node_get(priv->of_node);
- if (!phy_node)
- goto no_phy;
+ priv->phylink_config.dev = &dev->dev;
+ priv->phylink_config.type = PHYLINK_NETDEV;
+ priv->phylink_config.mac_capabilities = MAC_ASYM_PAUSE |
+ MAC_10 | MAC_100 | MAC_1000;
+ __set_bit(PHY_INTERFACE_MODE_MII,
+ priv->phylink_config.supported_interfaces);
+ __set_bit(PHY_INTERFACE_MODE_RMII,
+ priv->phylink_config.supported_interfaces);
+
+ switch (priv->imode) {
+ case CVMX_HELPER_INTERFACE_MODE_RGMII:
+ phy_interface_set_rgmii(priv->phylink_config.supported_interfaces);
+ break;
+ case CVMX_HELPER_INTERFACE_MODE_SGMII:
+ __set_bit(PHY_INTERFACE_MODE_1000BASEX,
+ priv->phylink_config.supported_interfaces);
+ __set_bit(PHY_INTERFACE_MODE_SGMII,
+ priv->phylink_config.supported_interfaces);
+ __set_bit(PHY_INTERFACE_MODE_QSGMII,
+ priv->phylink_config.supported_interfaces);
+ break;
+ default:
+ break;
+ }
- phydev = of_phy_connect(dev, phy_node, cvm_oct_adjust_link, 0,
- priv->phy_mode);
- of_node_put(phy_node);
+ if (of_get_phy_mode(priv->of_node, &phy_mode) == 0)
+ priv->phy_mode = phy_mode;
- if (!phydev)
- return -EPROBE_DEFER;
+ phylink = phylink_create(&priv->phylink_config,
+ of_fwnode_handle(priv->of_node),
+ priv->phy_mode, &cvm_oct_phylink_ops);
+ if (IS_ERR(phylink))
+ return PTR_ERR(phylink);
- priv->last_link = 0;
- phy_start(phydev);
+ priv->phylink = phylink;
- return 0;
-no_phy:
- /* If there is no phy, assume a direct MAC connection and that
- * the link is up.
- */
- netif_carrier_on(dev);
return 0;
}
diff --git a/drivers/staging/octeon/ethernet-rgmii.c b/drivers/staging/octeon/ethernet-rgmii.c
index 0c4fac31540a..8c6eb0b87254 100644
--- a/drivers/staging/octeon/ethernet-rgmii.c
+++ b/drivers/staging/octeon/ethernet-rgmii.c
@@ -115,17 +115,8 @@ static void cvm_oct_rgmii_poll(struct net_device *dev)
cvm_oct_check_preamble_errors(dev);
- if (likely(!status_change))
- return;
-
- /* Tell core. */
- if (link_info.s.link_up) {
- if (!netif_carrier_ok(dev))
- netif_carrier_on(dev);
- } else if (netif_carrier_ok(dev)) {
- netif_carrier_off(dev);
- }
- cvm_oct_note_carrier(priv, link_info);
+ if (likely(status_change))
+ phylink_mac_change(priv->phylink, link_info.s.link_up);
}
int cvm_oct_rgmii_open(struct net_device *dev)
diff --git a/drivers/staging/octeon/ethernet.c b/drivers/staging/octeon/ethernet.c
index 949ef51bf896..20b06fb3649c 100644
--- a/drivers/staging/octeon/ethernet.c
+++ b/drivers/staging/octeon/ethernet.c
@@ -10,7 +10,7 @@
#include <linux/module.h>
#include <linux/netdevice.h>
#include <linux/etherdevice.h>
-#include <linux/phy.h>
+#include <linux/phylink.h>
#include <linux/slab.h>
#include <linux/interrupt.h>
#include <linux/of_mdio.h>
@@ -128,7 +128,7 @@ static void cvm_oct_periodic_worker(struct work_struct *work)
struct octeon_ethernet,
port_periodic_work.work);
- if (priv->poll)
+ if (priv->poll_used && priv->poll)
priv->poll(cvm_oct_device[priv->port]);
cvm_oct_device[priv->port]->netdev_ops->ndo_get_stats
@@ -446,23 +446,19 @@ int cvm_oct_common_init(struct net_device *dev)
void cvm_oct_common_uninit(struct net_device *dev)
{
- if (dev->phydev)
- phy_disconnect(dev->phydev);
+ struct octeon_ethernet *priv = netdev_priv(dev);
+
+ phylink_destroy(priv->phylink);
}
int cvm_oct_common_open(struct net_device *dev,
void (*link_poll)(struct net_device *))
{
+ int err;
union cvmx_gmxx_prtx_cfg gmx_cfg;
struct octeon_ethernet *priv = netdev_priv(dev);
int interface = INTERFACE(priv->port);
int index = INDEX(priv->port);
- union cvmx_helper_link_info link_info;
- int rv;
-
- rv = cvm_oct_phy_setup_device(dev);
- if (rv)
- return rv;
gmx_cfg.u64 = cvmx_read_csr(CVMX_GMXX_PRTX_CFG(index, interface));
gmx_cfg.s.en = 1;
@@ -473,20 +469,17 @@ int cvm_oct_common_open(struct net_device *dev,
if (octeon_is_simulation())
return 0;
- if (dev->phydev) {
- int r = phy_read_status(dev->phydev);
-
- if (r == 0 && dev->phydev->link == 0)
- netif_carrier_off(dev);
- cvm_oct_adjust_link(dev);
- } else {
- link_info = cvmx_helper_link_get(priv->port);
- if (!link_info.s.link_up)
- netif_carrier_off(dev);
- priv->poll = link_poll;
- link_poll(dev);
+ err = phylink_of_phy_connect(priv->phylink, priv->of_node, 0);
+ if (err) {
+ netdev_err(dev, "Could not attach PHY (%d)\n", err);
+ return err;
}
+ priv->poll_used = false;
+ priv->poll = link_poll;
+
+ phylink_start(priv->phylink);
+
return 0;
}
@@ -504,13 +497,7 @@ void cvm_oct_link_poll(struct net_device *dev)
else
priv->link_info = link_info.u64;
- if (link_info.s.link_up) {
- if (!netif_carrier_ok(dev))
- netif_carrier_on(dev);
- } else if (netif_carrier_ok(dev)) {
- netif_carrier_off(dev);
- }
- cvm_oct_note_carrier(priv, link_info);
+ phylink_mac_change(priv->phylink, link_info.s.link_up);
}
static int cvm_oct_xaui_open(struct net_device *dev)
@@ -797,7 +784,6 @@ static int cvm_oct_probe(struct platform_device *pdev)
}
}
- num_interfaces = cvmx_helper_get_number_of_interfaces();
for (interface = 0; interface < num_interfaces; interface++) {
cvmx_helper_interface_mode_t imode =
cvmx_helper_interface_get_mode(interface);
@@ -886,13 +872,8 @@ static int cvm_oct_probe(struct platform_device *pdev)
break;
}
- if (priv->of_node && of_phy_is_fixed_link(priv->of_node)) {
- if (of_phy_register_fixed_link(priv->of_node)) {
- netdev_err(dev, "Failed to register fixed link for interface %d, port %d\n",
- interface, priv->port);
- dev->netdev_ops = NULL;
- }
- }
+ if (cvm_oct_phy_setup_device(dev))
+ dev->netdev_ops = NULL;
if (!dev->netdev_ops) {
free_netdev(dev);
diff --git a/drivers/staging/octeon/octeon-ethernet.h b/drivers/staging/octeon/octeon-ethernet.h
index a6140705706f..fc9bc0974a2a 100644
--- a/drivers/staging/octeon/octeon-ethernet.h
+++ b/drivers/staging/octeon/octeon-ethernet.h
@@ -12,7 +12,7 @@
#define OCTEON_ETHERNET_H
#include <linux/of.h>
-#include <linux/phy.h>
+#include <linux/phylink.h>
#ifdef CONFIG_CAVIUM_OCTEON_SOC
@@ -62,6 +62,8 @@ struct octeon_ethernet {
int imode;
/* PHY mode */
phy_interface_t phy_mode;
+ struct phylink *phylink;
+ struct phylink_config phylink_config;
/* List of outstanding tx buffers per queue */
struct sk_buff_head tx_free_list[16];
unsigned int last_speed;
@@ -69,6 +71,7 @@ struct octeon_ethernet {
/* Last negotiated link state */
u64 link_info;
/* Called periodically to check link status */
+ bool poll_used;
void (*poll)(struct net_device *dev);
struct delayed_work port_periodic_work;
struct device_node *of_node;
@@ -86,12 +89,9 @@ void cvm_oct_spi_uninit(struct net_device *dev);
int cvm_oct_common_init(struct net_device *dev);
void cvm_oct_common_uninit(struct net_device *dev);
-void cvm_oct_adjust_link(struct net_device *dev);
int cvm_oct_common_stop(struct net_device *dev);
int cvm_oct_common_open(struct net_device *dev,
void (*link_poll)(struct net_device *));
-void cvm_oct_note_carrier(struct octeon_ethernet *priv,
- union cvmx_helper_link_info li);
void cvm_oct_link_poll(struct net_device *dev);
extern int always_use_pow;
--
2.32.0
^ permalink raw reply related [flat|nested] 3+ messages in thread