2022-07-14 11:16:48

by Oleksandr Mazur

[permalink] [raw]
Subject: [PATCH V3 net-next] net: marvell: prestera: add phylink support

For SFP port prestera driver will use kernel
phylink infrastucture to configure port mode based on
the module that has beed inserted

Co-developed-by: Yevhen Orlov <[email protected]>
Signed-off-by: Yevhen Orlov <[email protected]>
Co-developed-by: Taras Chornyi <[email protected]>
Signed-off-by: Taras Chornyi <[email protected]>
Signed-off-by: Oleksandr Mazur <[email protected]>

PATCH V3:
- force inband mode for SGMII
- fix >80 chars warning of checkpatch where possible (2/5)
- structure phylink_mac_change alongside phylink-related if-clause;
PATCH V2:
- fix mistreat of bitfield values as if they were bools.
- remove phylink_config ifdefs.
- remove obsolete phylink pcs / mac callbacks;
- rework mac (/pcs) config to not look for speed / duplex
parameters while link is not yet set up.
- remove unused functions.
- add phylink select cfg to prestera Kconfig.
---
drivers/net/ethernet/marvell/prestera/Kconfig | 1 +
.../net/ethernet/marvell/prestera/prestera.h | 9 +
.../marvell/prestera/prestera_ethtool.c | 28 +-
.../marvell/prestera/prestera_ethtool.h | 3 -
.../ethernet/marvell/prestera/prestera_main.c | 348 ++++++++++++++++--
5 files changed, 327 insertions(+), 62 deletions(-)

diff --git a/drivers/net/ethernet/marvell/prestera/Kconfig b/drivers/net/ethernet/marvell/prestera/Kconfig
index b6f20e2034c6..f2f7663c3d10 100644
--- a/drivers/net/ethernet/marvell/prestera/Kconfig
+++ b/drivers/net/ethernet/marvell/prestera/Kconfig
@@ -8,6 +8,7 @@ config PRESTERA
depends on NET_SWITCHDEV && VLAN_8021Q
depends on BRIDGE || BRIDGE=n
select NET_DEVLINK
+ select PHYLINK
help
This driver supports Marvell Prestera Switch ASICs family.

diff --git a/drivers/net/ethernet/marvell/prestera/prestera.h b/drivers/net/ethernet/marvell/prestera/prestera.h
index bff9651f0a89..832c27e0c284 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera.h
+++ b/drivers/net/ethernet/marvell/prestera/prestera.h
@@ -7,6 +7,7 @@
#include <linux/notifier.h>
#include <linux/skbuff.h>
#include <linux/workqueue.h>
+#include <linux/phylink.h>
#include <net/devlink.h>
#include <uapi/linux/if_ether.h>

@@ -92,6 +93,7 @@ struct prestera_lag {
struct prestera_flow_block;

struct prestera_port_mac_state {
+ bool valid;
u32 mode;
u32 speed;
bool oper;
@@ -151,6 +153,12 @@ struct prestera_port {
struct prestera_port_phy_config cfg_phy;
struct prestera_port_mac_state state_mac;
struct prestera_port_phy_state state_phy;
+
+ struct phylink_config phy_config;
+ struct phylink *phy_link;
+ struct phylink_pcs phylink_pcs;
+
+ rwlock_t state_mac_lock;
};

struct prestera_device {
@@ -291,6 +299,7 @@ struct prestera_switch {
u32 mtu_min;
u32 mtu_max;
u8 id;
+ struct device_node *np;
struct prestera_router *router;
struct prestera_lag *lags;
struct prestera_counter *counter;
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_ethtool.c b/drivers/net/ethernet/marvell/prestera/prestera_ethtool.c
index 40d5b89573bb..1da7ff889417 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_ethtool.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_ethtool.c
@@ -521,6 +521,9 @@ prestera_ethtool_get_link_ksettings(struct net_device *dev,
ecmd->base.speed = SPEED_UNKNOWN;
ecmd->base.duplex = DUPLEX_UNKNOWN;

+ if (port->phy_link)
+ return phylink_ethtool_ksettings_get(port->phy_link, ecmd);
+
ecmd->base.autoneg = port->autoneg ? AUTONEG_ENABLE : AUTONEG_DISABLE;

if (port->caps.type == PRESTERA_PORT_TYPE_TP) {
@@ -648,6 +651,9 @@ prestera_ethtool_set_link_ksettings(struct net_device *dev,
u8 adver_fec;
int err;

+ if (port->phy_link)
+ return phylink_ethtool_ksettings_set(port->phy_link, ecmd);
+
err = prestera_port_type_set(ecmd, port);
if (err)
return err;
@@ -782,28 +788,6 @@ static int prestera_ethtool_nway_reset(struct net_device *dev)
return -EINVAL;
}

-void prestera_ethtool_port_state_changed(struct prestera_port *port,
- struct prestera_port_event *evt)
-{
- struct prestera_port_mac_state *smac = &port->state_mac;
-
- smac->oper = evt->data.mac.oper;
-
- if (smac->oper) {
- smac->mode = evt->data.mac.mode;
- smac->speed = evt->data.mac.speed;
- smac->duplex = evt->data.mac.duplex;
- smac->fc = evt->data.mac.fc;
- smac->fec = evt->data.mac.fec;
- } else {
- smac->mode = PRESTERA_MAC_MODE_MAX;
- smac->speed = SPEED_UNKNOWN;
- smac->duplex = DUPLEX_UNKNOWN;
- smac->fc = 0;
- smac->fec = 0;
- }
-}
-
const struct ethtool_ops prestera_ethtool_ops = {
.get_drvinfo = prestera_ethtool_get_drvinfo,
.get_link_ksettings = prestera_ethtool_get_link_ksettings,
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_ethtool.h b/drivers/net/ethernet/marvell/prestera/prestera_ethtool.h
index 9eb18e99dea6..bd5600886bc6 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_ethtool.h
+++ b/drivers/net/ethernet/marvell/prestera/prestera_ethtool.h
@@ -11,7 +11,4 @@ struct prestera_port;

extern const struct ethtool_ops prestera_ethtool_ops;

-void prestera_ethtool_port_state_changed(struct prestera_port *port,
- struct prestera_port_event *evt);
-
#endif /* _PRESTERA_ETHTOOL_H_ */
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_main.c b/drivers/net/ethernet/marvell/prestera/prestera_main.c
index ea5bd5069826..65fded8c8f87 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_main.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_main.c
@@ -9,6 +9,7 @@
#include <linux/of.h>
#include <linux/of_net.h>
#include <linux/if_vlan.h>
+#include <linux/phylink.h>

#include "prestera.h"
#include "prestera_hw.h"
@@ -142,18 +143,24 @@ static int prestera_port_open(struct net_device *dev)
struct prestera_port_mac_config cfg_mac;
int err = 0;

- if (port->caps.transceiver == PRESTERA_PORT_TCVR_SFP) {
- err = prestera_port_cfg_mac_read(port, &cfg_mac);
- if (!err) {
- cfg_mac.admin = true;
- err = prestera_port_cfg_mac_write(port, &cfg_mac);
- }
+ if (port->phy_link) {
+ phylink_start(port->phy_link);
} else {
- port->cfg_phy.admin = true;
- err = prestera_hw_port_phy_mode_set(port, true, port->autoneg,
- port->cfg_phy.mode,
- port->adver_link_modes,
- port->cfg_phy.mdix);
+ if (port->caps.transceiver == PRESTERA_PORT_TCVR_SFP) {
+ err = prestera_port_cfg_mac_read(port, &cfg_mac);
+ if (!err) {
+ cfg_mac.admin = true;
+ err = prestera_port_cfg_mac_write(port,
+ &cfg_mac);
+ }
+ } else {
+ port->cfg_phy.admin = true;
+ err = prestera_hw_port_phy_mode_set(port, true,
+ port->autoneg,
+ port->cfg_phy.mode,
+ port->adver_link_modes,
+ port->cfg_phy.mdix);
+ }
}

netif_start_queue(dev);
@@ -169,23 +176,255 @@ static int prestera_port_close(struct net_device *dev)

netif_stop_queue(dev);

- if (port->caps.transceiver == PRESTERA_PORT_TCVR_SFP) {
+ if (port->phy_link) {
+ phylink_stop(port->phy_link);
+ phylink_disconnect_phy(port->phy_link);
err = prestera_port_cfg_mac_read(port, &cfg_mac);
if (!err) {
cfg_mac.admin = false;
prestera_port_cfg_mac_write(port, &cfg_mac);
}
} else {
- port->cfg_phy.admin = false;
- err = prestera_hw_port_phy_mode_set(port, false, port->autoneg,
- port->cfg_phy.mode,
- port->adver_link_modes,
- port->cfg_phy.mdix);
+ if (port->caps.transceiver == PRESTERA_PORT_TCVR_SFP) {
+ err = prestera_port_cfg_mac_read(port, &cfg_mac);
+ if (!err) {
+ cfg_mac.admin = false;
+ prestera_port_cfg_mac_write(port, &cfg_mac);
+ }
+ } else {
+ port->cfg_phy.admin = false;
+ err = prestera_hw_port_phy_mode_set(port, false, port->autoneg,
+ port->cfg_phy.mode,
+ port->adver_link_modes,
+ port->cfg_phy.mdix);
+ }
+ }
+
+ return err;
+}
+
+static void
+prestera_port_mac_state_cache_read(struct prestera_port *port,
+ struct prestera_port_mac_state *state)
+{
+ read_lock(&port->state_mac_lock);
+ *state = port->state_mac;
+ read_unlock(&port->state_mac_lock);
+}
+
+static void
+prestera_port_mac_state_cache_write(struct prestera_port *port,
+ struct prestera_port_mac_state *state)
+{
+ write_lock(&port->state_mac_lock);
+ port->state_mac = *state;
+ write_unlock(&port->state_mac_lock);
+}
+
+static struct prestera_port *prestera_pcs_to_port(struct phylink_pcs *pcs)
+{
+ return container_of(pcs, struct prestera_port, phylink_pcs);
+}
+
+static void prestera_mac_config(struct phylink_config *config,
+ unsigned int an_mode,
+ const struct phylink_link_state *state)
+{
+}
+
+static void prestera_mac_link_down(struct phylink_config *config,
+ unsigned int mode, phy_interface_t interface)
+{
+ struct net_device *ndev = to_net_dev(config->dev);
+ struct prestera_port *port = netdev_priv(ndev);
+ struct prestera_port_mac_state state_mac;
+
+ /* Invalidate. Parameters will update on next link event. */
+ memset(&state_mac, 0, sizeof(state_mac));
+ state_mac.valid = false;
+ prestera_port_mac_state_cache_write(port, &state_mac);
+}
+
+static void prestera_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)
+{
+}
+
+static struct phylink_pcs *
+prestera_mac_select_pcs(struct phylink_config *config,
+ phy_interface_t interface)
+{
+ struct net_device *dev = to_net_dev(config->dev);
+ struct prestera_port *port = netdev_priv(dev);
+
+ return &port->phylink_pcs;
+}
+
+static void prestera_pcs_get_state(struct phylink_pcs *pcs,
+ struct phylink_link_state *state)
+{
+ struct prestera_port *port = container_of(pcs, struct prestera_port,
+ phylink_pcs);
+ struct prestera_port_mac_state smac;
+
+ prestera_port_mac_state_cache_read(port, &smac);
+
+ if (smac.valid) {
+ state->link = smac.oper ? 1 : 0;
+ /* AN is completed, when port is up */
+ state->an_complete = (smac.oper && port->autoneg) ? 1 : 0;
+ state->speed = smac.speed;
+ state->duplex = smac.duplex;
+ } else {
+ state->link = 0;
+ state->an_complete = 0;
+ }
+}
+
+static int prestera_pcs_config(struct phylink_pcs *pcs,
+ unsigned int mode,
+ phy_interface_t interface,
+ const unsigned long *advertising,
+ bool permit_pause_to_mac)
+{
+ struct prestera_port *port = port = prestera_pcs_to_port(pcs);
+ struct prestera_port_mac_config cfg_mac;
+ int err;
+
+ err = prestera_port_cfg_mac_read(port, &cfg_mac);
+ if (err)
+ return err;
+
+ cfg_mac.admin = true;
+ cfg_mac.fec = PRESTERA_PORT_FEC_OFF;
+
+ switch (interface) {
+ case PHY_INTERFACE_MODE_10GBASER:
+ cfg_mac.speed = SPEED_10000;
+ cfg_mac.inband = 0;
+ cfg_mac.mode = PRESTERA_MAC_MODE_SR_LR;
+ break;
+ case PHY_INTERFACE_MODE_2500BASEX:
+ cfg_mac.speed = SPEED_2500;
+ cfg_mac.duplex = DUPLEX_FULL;
+ cfg_mac.inband = test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
+ advertising);
+ cfg_mac.mode = PRESTERA_MAC_MODE_SGMII;
+ break;
+ case PHY_INTERFACE_MODE_SGMII:
+ cfg_mac.inband = 1;
+ cfg_mac.mode = PRESTERA_MAC_MODE_SGMII;
+ break;
+ case PHY_INTERFACE_MODE_1000BASEX:
+ default:
+ cfg_mac.speed = SPEED_1000;
+ cfg_mac.duplex = DUPLEX_FULL;
+ cfg_mac.inband = test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
+ advertising);
+ cfg_mac.mode = PRESTERA_MAC_MODE_1000BASE_X;
+ break;
+ }
+
+ err = prestera_port_cfg_mac_write(port, &cfg_mac);
+ if (err)
+ return err;
+
+ return 0;
+}
+
+static void prestera_pcs_an_restart(struct phylink_pcs *pcs)
+{
+}
+
+static const struct phylink_mac_ops prestera_mac_ops = {
+ .validate = phylink_generic_validate,
+ .mac_select_pcs = prestera_mac_select_pcs,
+ .mac_config = prestera_mac_config,
+ .mac_link_down = prestera_mac_link_down,
+ .mac_link_up = prestera_mac_link_up,
+};
+
+static const struct phylink_pcs_ops prestera_pcs_ops = {
+ .pcs_get_state = prestera_pcs_get_state,
+ .pcs_config = prestera_pcs_config,
+ .pcs_an_restart = prestera_pcs_an_restart,
+};
+
+static int prestera_port_sfp_bind(struct prestera_port *port)
+{
+ struct prestera_switch *sw = port->sw;
+ struct device_node *ports, *node;
+ struct fwnode_handle *fwnode;
+ struct phylink *phy_link;
+ int err;
+
+ if (!sw->np)
+ return 0;
+
+ ports = of_find_node_by_name(sw->np, "ports");
+
+ for_each_child_of_node(ports, node) {
+ int num;
+
+ err = of_property_read_u32(node, "prestera,port-num", &num);
+ if (err) {
+ dev_err(sw->dev->dev,
+ "device node %pOF has no valid reg property: %d\n",
+ node, err);
+ goto out;
+ }
+
+ if (port->fp_id != num)
+ continue;
+
+ port->phylink_pcs.ops = &prestera_pcs_ops;
+
+ port->phy_config.dev = &port->dev->dev;
+ port->phy_config.type = PHYLINK_NETDEV;
+
+ fwnode = of_fwnode_handle(node);
+
+ __set_bit(PHY_INTERFACE_MODE_10GBASER,
+ port->phy_config.supported_interfaces);
+ __set_bit(PHY_INTERFACE_MODE_2500BASEX,
+ port->phy_config.supported_interfaces);
+ __set_bit(PHY_INTERFACE_MODE_SGMII,
+ port->phy_config.supported_interfaces);
+ __set_bit(PHY_INTERFACE_MODE_1000BASEX,
+ port->phy_config.supported_interfaces);
+
+ port->phy_config.mac_capabilities =
+ MAC_1000 | MAC_2500FD | MAC_10000FD;
+
+ phy_link = phylink_create(&port->phy_config, fwnode,
+ PHY_INTERFACE_MODE_INTERNAL,
+ &prestera_mac_ops);
+ if (IS_ERR(phy_link)) {
+ netdev_err(port->dev, "failed to create phylink\n");
+ err = PTR_ERR(phy_link);
+ goto out;
+ }
+
+ port->phy_link = phy_link;
+ break;
}

+out:
+ of_node_put(ports);
return err;
}

+static int prestera_port_sfp_unbind(struct prestera_port *port)
+{
+ if (port->phy_link)
+ phylink_destroy(port->phy_link);
+
+ return 0;
+}
+
static netdev_tx_t prestera_port_xmit(struct sk_buff *skb,
struct net_device *dev)
{
@@ -380,8 +619,10 @@ static int prestera_port_create(struct prestera_switch *sw, u32 id)
dev->features |= NETIF_F_NETNS_LOCAL | NETIF_F_HW_TC;
dev->netdev_ops = &prestera_netdev_ops;
dev->ethtool_ops = &prestera_ethtool_ops;
+ SET_NETDEV_DEV(dev, sw->dev->dev);

- netif_carrier_off(dev);
+ if (port->caps.transceiver != PRESTERA_PORT_TCVR_SFP)
+ netif_carrier_off(dev);

dev->mtu = min_t(unsigned int, sw->mtu_max, PRESTERA_MTU_DEFAULT);
dev->min_mtu = sw->mtu_min;
@@ -432,7 +673,7 @@ static int prestera_port_create(struct prestera_switch *sw, u32 id)
cfg_mac.admin = false;
cfg_mac.mode = PRESTERA_MAC_MODE_MAX;
}
- cfg_mac.inband = false;
+ cfg_mac.inband = 0;
cfg_mac.speed = 0;
cfg_mac.duplex = DUPLEX_UNKNOWN;
cfg_mac.fec = PRESTERA_PORT_FEC_OFF;
@@ -474,8 +715,13 @@ static int prestera_port_create(struct prestera_switch *sw, u32 id)

prestera_devlink_port_set(port);

+ err = prestera_port_sfp_bind(port);
+ if (err)
+ goto err_sfp_bind;
+
return 0;

+err_sfp_bind:
err_register_netdev:
prestera_port_list_del(port);
err_port_init:
@@ -521,8 +767,10 @@ static int prestera_create_ports(struct prestera_switch *sw)
return 0;

err_port_create:
- list_for_each_entry_safe(port, tmp, &sw->port_list, list)
+ list_for_each_entry_safe(port, tmp, &sw->port_list, list) {
+ prestera_port_sfp_unbind(port);
prestera_port_destroy(port);
+ }

return err;
}
@@ -530,25 +778,47 @@ static int prestera_create_ports(struct prestera_switch *sw)
static void prestera_port_handle_event(struct prestera_switch *sw,
struct prestera_event *evt, void *arg)
{
+ struct prestera_port_mac_state smac;
+ struct prestera_port_event *pevt;
struct delayed_work *caching_dw;
struct prestera_port *port;

- port = prestera_find_port(sw, evt->port_evt.port_id);
- if (!port || !port->dev)
- return;
-
- caching_dw = &port->cached_hw_stats.caching_dw;
-
- prestera_ethtool_port_state_changed(port, &evt->port_evt);
-
if (evt->id == PRESTERA_PORT_EVENT_MAC_STATE_CHANGED) {
+ pevt = &evt->port_evt;
+ port = prestera_find_port(sw, pevt->port_id);
+ if (!port || !port->dev)
+ return;
+
+ caching_dw = &port->cached_hw_stats.caching_dw;
+
+ if (port->phy_link) {
+ memset(&smac, 0, sizeof(smac));
+ smac.valid = true;
+ smac.oper = pevt->data.mac.oper;
+ if (smac.oper) {
+ smac.mode = pevt->data.mac.mode;
+ smac.speed = pevt->data.mac.speed;
+ smac.duplex = pevt->data.mac.duplex;
+ smac.fc = pevt->data.mac.fc;
+ smac.fec = pevt->data.mac.fec;
+ phylink_mac_change(port->phy_link, true);
+ } else {
+ phylink_mac_change(port->phy_link, false);
+ }
+ prestera_port_mac_state_cache_write(port, &smac);
+ }
+
if (port->state_mac.oper) {
- netif_carrier_on(port->dev);
+ if (!port->phy_link)
+ netif_carrier_on(port->dev);
+
if (!delayed_work_pending(caching_dw))
queue_delayed_work(prestera_wq, caching_dw, 0);
} else if (netif_running(port->dev) &&
netif_carrier_ok(port->dev)) {
- netif_carrier_off(port->dev);
+ if (!port->phy_link)
+ netif_carrier_off(port->dev);
+
if (delayed_work_pending(caching_dw))
cancel_delayed_work(caching_dw);
}
@@ -571,19 +841,20 @@ static void prestera_event_handlers_unregister(struct prestera_switch *sw)
static int prestera_switch_set_base_mac_addr(struct prestera_switch *sw)
{
struct device_node *base_mac_np;
- struct device_node *np;
int ret;

- np = of_find_compatible_node(NULL, NULL, "marvell,prestera");
- base_mac_np = of_parse_phandle(np, "base-mac-provider", 0);
+ if (sw->np) {
+ base_mac_np = of_parse_phandle(sw->np, "base-mac-provider", 0);
+ if (base_mac_np) {
+ ret = of_get_mac_address(base_mac_np, sw->base_mac);
+ of_node_put(base_mac_np);
+ }
+ }

- ret = of_get_mac_address(base_mac_np, sw->base_mac);
- if (ret) {
+ if (!is_valid_ether_addr(sw->base_mac) || ret) {
eth_random_addr(sw->base_mac);
dev_info(prestera_dev(sw), "using random base mac address\n");
}
- of_node_put(base_mac_np);
- of_node_put(np);

return prestera_hw_switch_mac_set(sw, sw->base_mac);
}
@@ -1083,6 +1354,8 @@ static int prestera_switch_init(struct prestera_switch *sw)
{
int err;

+ sw->np = of_find_compatible_node(NULL, NULL, "marvell,prestera");
+
err = prestera_hw_switch_init(sw);
if (err) {
dev_err(prestera_dev(sw), "Failed to init Switch device\n");
@@ -1183,6 +1456,7 @@ static void prestera_switch_fini(struct prestera_switch *sw)
prestera_router_fini(sw);
prestera_netdev_event_handler_unregister(sw);
prestera_hw_switch_fini(sw);
+ of_node_put(sw->np);
}

int prestera_device_register(struct prestera_device *dev)
--
2.17.1


2022-07-14 12:47:01

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH V3 net-next] net: marvell: prestera: add phylink support

Hi Oleksandr,

A process tip: don't send another version immediately after receiving
feedback from a previous version - it's really not helpful, and it
overloads reviewers with additional burden. Please wait at least a few
days to see if anyone else comments on the changes before sending
another revision of it - or a reviewer replies to your reply (as I
have just done.)

Thanks.


On Thu, Jul 14, 2022 at 01:55:16PM +0300, Oleksandr Mazur wrote:
> For SFP port prestera driver will use kernel
> phylink infrastucture to configure port mode based on
> the module that has beed inserted
>
> Co-developed-by: Yevhen Orlov <[email protected]>
> Signed-off-by: Yevhen Orlov <[email protected]>
> Co-developed-by: Taras Chornyi <[email protected]>
> Signed-off-by: Taras Chornyi <[email protected]>
> Signed-off-by: Oleksandr Mazur <[email protected]>
>
> PATCH V3:
> - force inband mode for SGMII
> - fix >80 chars warning of checkpatch where possible (2/5)
> - structure phylink_mac_change alongside phylink-related if-clause;
> PATCH V2:
> - fix mistreat of bitfield values as if they were bools.
> - remove phylink_config ifdefs.
> - remove obsolete phylink pcs / mac callbacks;
> - rework mac (/pcs) config to not look for speed / duplex
> parameters while link is not yet set up.
> - remove unused functions.
> - add phylink select cfg to prestera Kconfig.
> ---
> drivers/net/ethernet/marvell/prestera/Kconfig | 1 +
> .../net/ethernet/marvell/prestera/prestera.h | 9 +
> .../marvell/prestera/prestera_ethtool.c | 28 +-
> .../marvell/prestera/prestera_ethtool.h | 3 -
> .../ethernet/marvell/prestera/prestera_main.c | 348 ++++++++++++++++--
> 5 files changed, 327 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/prestera/Kconfig b/drivers/net/ethernet/marvell/prestera/Kconfig
> index b6f20e2034c6..f2f7663c3d10 100644
> --- a/drivers/net/ethernet/marvell/prestera/Kconfig
> +++ b/drivers/net/ethernet/marvell/prestera/Kconfig
> @@ -8,6 +8,7 @@ config PRESTERA
> depends on NET_SWITCHDEV && VLAN_8021Q
> depends on BRIDGE || BRIDGE=n
> select NET_DEVLINK
> + select PHYLINK
> help
> This driver supports Marvell Prestera Switch ASICs family.
>
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera.h b/drivers/net/ethernet/marvell/prestera/prestera.h
> index bff9651f0a89..832c27e0c284 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera.h
> +++ b/drivers/net/ethernet/marvell/prestera/prestera.h
> @@ -7,6 +7,7 @@
> #include <linux/notifier.h>
> #include <linux/skbuff.h>
> #include <linux/workqueue.h>
> +#include <linux/phylink.h>
> #include <net/devlink.h>
> #include <uapi/linux/if_ether.h>
>
> @@ -92,6 +93,7 @@ struct prestera_lag {
> struct prestera_flow_block;
>
> struct prestera_port_mac_state {
> + bool valid;
> u32 mode;
> u32 speed;
> bool oper;
> @@ -151,6 +153,12 @@ struct prestera_port {
> struct prestera_port_phy_config cfg_phy;
> struct prestera_port_mac_state state_mac;
> struct prestera_port_phy_state state_phy;
> +
> + struct phylink_config phy_config;
> + struct phylink *phy_link;
> + struct phylink_pcs phylink_pcs;
> +
> + rwlock_t state_mac_lock;
> };
>
> struct prestera_device {
> @@ -291,6 +299,7 @@ struct prestera_switch {
> u32 mtu_min;
> u32 mtu_max;
> u8 id;
> + struct device_node *np;
> struct prestera_router *router;
> struct prestera_lag *lags;
> struct prestera_counter *counter;
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_ethtool.c b/drivers/net/ethernet/marvell/prestera/prestera_ethtool.c
> index 40d5b89573bb..1da7ff889417 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_ethtool.c
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_ethtool.c
> @@ -521,6 +521,9 @@ prestera_ethtool_get_link_ksettings(struct net_device *dev,
> ecmd->base.speed = SPEED_UNKNOWN;
> ecmd->base.duplex = DUPLEX_UNKNOWN;
>
> + if (port->phy_link)
> + return phylink_ethtool_ksettings_get(port->phy_link, ecmd);
> +
> ecmd->base.autoneg = port->autoneg ? AUTONEG_ENABLE : AUTONEG_DISABLE;
>
> if (port->caps.type == PRESTERA_PORT_TYPE_TP) {
> @@ -648,6 +651,9 @@ prestera_ethtool_set_link_ksettings(struct net_device *dev,
> u8 adver_fec;
> int err;
>
> + if (port->phy_link)
> + return phylink_ethtool_ksettings_set(port->phy_link, ecmd);
> +
> err = prestera_port_type_set(ecmd, port);
> if (err)
> return err;
> @@ -782,28 +788,6 @@ static int prestera_ethtool_nway_reset(struct net_device *dev)
> return -EINVAL;
> }
>
> -void prestera_ethtool_port_state_changed(struct prestera_port *port,
> - struct prestera_port_event *evt)
> -{
> - struct prestera_port_mac_state *smac = &port->state_mac;
> -
> - smac->oper = evt->data.mac.oper;
> -
> - if (smac->oper) {
> - smac->mode = evt->data.mac.mode;
> - smac->speed = evt->data.mac.speed;
> - smac->duplex = evt->data.mac.duplex;
> - smac->fc = evt->data.mac.fc;
> - smac->fec = evt->data.mac.fec;
> - } else {
> - smac->mode = PRESTERA_MAC_MODE_MAX;
> - smac->speed = SPEED_UNKNOWN;
> - smac->duplex = DUPLEX_UNKNOWN;
> - smac->fc = 0;
> - smac->fec = 0;
> - }
> -}
> -
> const struct ethtool_ops prestera_ethtool_ops = {
> .get_drvinfo = prestera_ethtool_get_drvinfo,
> .get_link_ksettings = prestera_ethtool_get_link_ksettings,
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_ethtool.h b/drivers/net/ethernet/marvell/prestera/prestera_ethtool.h
> index 9eb18e99dea6..bd5600886bc6 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_ethtool.h
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_ethtool.h
> @@ -11,7 +11,4 @@ struct prestera_port;
>
> extern const struct ethtool_ops prestera_ethtool_ops;
>
> -void prestera_ethtool_port_state_changed(struct prestera_port *port,
> - struct prestera_port_event *evt);
> -
> #endif /* _PRESTERA_ETHTOOL_H_ */
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_main.c b/drivers/net/ethernet/marvell/prestera/prestera_main.c
> index ea5bd5069826..65fded8c8f87 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_main.c
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_main.c
> @@ -9,6 +9,7 @@
> #include <linux/of.h>
> #include <linux/of_net.h>
> #include <linux/if_vlan.h>
> +#include <linux/phylink.h>
>
> #include "prestera.h"
> #include "prestera_hw.h"
> @@ -142,18 +143,24 @@ static int prestera_port_open(struct net_device *dev)
> struct prestera_port_mac_config cfg_mac;
> int err = 0;
>
> - if (port->caps.transceiver == PRESTERA_PORT_TCVR_SFP) {
> - err = prestera_port_cfg_mac_read(port, &cfg_mac);
> - if (!err) {
> - cfg_mac.admin = true;
> - err = prestera_port_cfg_mac_write(port, &cfg_mac);
> - }
> + if (port->phy_link) {
> + phylink_start(port->phy_link);
> } else {
> - port->cfg_phy.admin = true;
> - err = prestera_hw_port_phy_mode_set(port, true, port->autoneg,
> - port->cfg_phy.mode,
> - port->adver_link_modes,
> - port->cfg_phy.mdix);
> + if (port->caps.transceiver == PRESTERA_PORT_TCVR_SFP) {
> + err = prestera_port_cfg_mac_read(port, &cfg_mac);
> + if (!err) {
> + cfg_mac.admin = true;
> + err = prestera_port_cfg_mac_write(port,
> + &cfg_mac);
> + }
> + } else {
> + port->cfg_phy.admin = true;
> + err = prestera_hw_port_phy_mode_set(port, true,
> + port->autoneg,
> + port->cfg_phy.mode,
> + port->adver_link_modes,
> + port->cfg_phy.mdix);
> + }
> }
>
> netif_start_queue(dev);
> @@ -169,23 +176,255 @@ static int prestera_port_close(struct net_device *dev)
>
> netif_stop_queue(dev);
>
> - if (port->caps.transceiver == PRESTERA_PORT_TCVR_SFP) {
> + if (port->phy_link) {
> + phylink_stop(port->phy_link);
> + phylink_disconnect_phy(port->phy_link);
> err = prestera_port_cfg_mac_read(port, &cfg_mac);
> if (!err) {
> cfg_mac.admin = false;
> prestera_port_cfg_mac_write(port, &cfg_mac);
> }
> } else {
> - port->cfg_phy.admin = false;
> - err = prestera_hw_port_phy_mode_set(port, false, port->autoneg,
> - port->cfg_phy.mode,
> - port->adver_link_modes,
> - port->cfg_phy.mdix);
> + if (port->caps.transceiver == PRESTERA_PORT_TCVR_SFP) {
> + err = prestera_port_cfg_mac_read(port, &cfg_mac);
> + if (!err) {
> + cfg_mac.admin = false;
> + prestera_port_cfg_mac_write(port, &cfg_mac);
> + }
> + } else {
> + port->cfg_phy.admin = false;
> + err = prestera_hw_port_phy_mode_set(port, false, port->autoneg,
> + port->cfg_phy.mode,
> + port->adver_link_modes,
> + port->cfg_phy.mdix);
> + }
> + }
> +
> + return err;
> +}
> +
> +static void
> +prestera_port_mac_state_cache_read(struct prestera_port *port,
> + struct prestera_port_mac_state *state)
> +{
> + read_lock(&port->state_mac_lock);
> + *state = port->state_mac;
> + read_unlock(&port->state_mac_lock);
> +}
> +
> +static void
> +prestera_port_mac_state_cache_write(struct prestera_port *port,
> + struct prestera_port_mac_state *state)
> +{
> + write_lock(&port->state_mac_lock);
> + port->state_mac = *state;
> + write_unlock(&port->state_mac_lock);
> +}
> +
> +static struct prestera_port *prestera_pcs_to_port(struct phylink_pcs *pcs)
> +{
> + return container_of(pcs, struct prestera_port, phylink_pcs);
> +}
> +
> +static void prestera_mac_config(struct phylink_config *config,
> + unsigned int an_mode,
> + const struct phylink_link_state *state)
> +{
> +}
> +
> +static void prestera_mac_link_down(struct phylink_config *config,
> + unsigned int mode, phy_interface_t interface)
> +{
> + struct net_device *ndev = to_net_dev(config->dev);
> + struct prestera_port *port = netdev_priv(ndev);
> + struct prestera_port_mac_state state_mac;
> +
> + /* Invalidate. Parameters will update on next link event. */
> + memset(&state_mac, 0, sizeof(state_mac));
> + state_mac.valid = false;
> + prestera_port_mac_state_cache_write(port, &state_mac);
> +}
> +
> +static void prestera_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)
> +{
> +}
> +
> +static struct phylink_pcs *
> +prestera_mac_select_pcs(struct phylink_config *config,
> + phy_interface_t interface)
> +{
> + struct net_device *dev = to_net_dev(config->dev);
> + struct prestera_port *port = netdev_priv(dev);
> +
> + return &port->phylink_pcs;
> +}
> +
> +static void prestera_pcs_get_state(struct phylink_pcs *pcs,
> + struct phylink_link_state *state)
> +{
> + struct prestera_port *port = container_of(pcs, struct prestera_port,
> + phylink_pcs);
> + struct prestera_port_mac_state smac;
> +
> + prestera_port_mac_state_cache_read(port, &smac);
> +
> + if (smac.valid) {
> + state->link = smac.oper ? 1 : 0;
> + /* AN is completed, when port is up */
> + state->an_complete = (smac.oper && port->autoneg) ? 1 : 0;
> + state->speed = smac.speed;
> + state->duplex = smac.duplex;
> + } else {
> + state->link = 0;
> + state->an_complete = 0;
> + }
> +}
> +
> +static int prestera_pcs_config(struct phylink_pcs *pcs,
> + unsigned int mode,
> + phy_interface_t interface,
> + const unsigned long *advertising,
> + bool permit_pause_to_mac)
> +{
> + struct prestera_port *port = port = prestera_pcs_to_port(pcs);
> + struct prestera_port_mac_config cfg_mac;
> + int err;
> +
> + err = prestera_port_cfg_mac_read(port, &cfg_mac);
> + if (err)
> + return err;
> +
> + cfg_mac.admin = true;
> + cfg_mac.fec = PRESTERA_PORT_FEC_OFF;
> +
> + switch (interface) {
> + case PHY_INTERFACE_MODE_10GBASER:
> + cfg_mac.speed = SPEED_10000;
> + cfg_mac.inband = 0;
> + cfg_mac.mode = PRESTERA_MAC_MODE_SR_LR;
> + break;
> + case PHY_INTERFACE_MODE_2500BASEX:
> + cfg_mac.speed = SPEED_2500;
> + cfg_mac.duplex = DUPLEX_FULL;
> + cfg_mac.inband = test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
> + advertising);
> + cfg_mac.mode = PRESTERA_MAC_MODE_SGMII;
> + break;
> + case PHY_INTERFACE_MODE_SGMII:
> + cfg_mac.inband = 1;
> + cfg_mac.mode = PRESTERA_MAC_MODE_SGMII;
> + break;
> + case PHY_INTERFACE_MODE_1000BASEX:
> + default:
> + cfg_mac.speed = SPEED_1000;
> + cfg_mac.duplex = DUPLEX_FULL;
> + cfg_mac.inband = test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
> + advertising);
> + cfg_mac.mode = PRESTERA_MAC_MODE_1000BASE_X;
> + break;
> + }
> +
> + err = prestera_port_cfg_mac_write(port, &cfg_mac);
> + if (err)
> + return err;
> +
> + return 0;
> +}
> +
> +static void prestera_pcs_an_restart(struct phylink_pcs *pcs)
> +{
> +}
> +
> +static const struct phylink_mac_ops prestera_mac_ops = {
> + .validate = phylink_generic_validate,
> + .mac_select_pcs = prestera_mac_select_pcs,
> + .mac_config = prestera_mac_config,
> + .mac_link_down = prestera_mac_link_down,
> + .mac_link_up = prestera_mac_link_up,
> +};
> +
> +static const struct phylink_pcs_ops prestera_pcs_ops = {
> + .pcs_get_state = prestera_pcs_get_state,
> + .pcs_config = prestera_pcs_config,
> + .pcs_an_restart = prestera_pcs_an_restart,
> +};
> +
> +static int prestera_port_sfp_bind(struct prestera_port *port)
> +{
> + struct prestera_switch *sw = port->sw;
> + struct device_node *ports, *node;
> + struct fwnode_handle *fwnode;
> + struct phylink *phy_link;
> + int err;
> +
> + if (!sw->np)
> + return 0;
> +
> + ports = of_find_node_by_name(sw->np, "ports");
> +
> + for_each_child_of_node(ports, node) {
> + int num;
> +
> + err = of_property_read_u32(node, "prestera,port-num", &num);
> + if (err) {
> + dev_err(sw->dev->dev,
> + "device node %pOF has no valid reg property: %d\n",
> + node, err);
> + goto out;
> + }
> +
> + if (port->fp_id != num)
> + continue;
> +
> + port->phylink_pcs.ops = &prestera_pcs_ops;
> +
> + port->phy_config.dev = &port->dev->dev;
> + port->phy_config.type = PHYLINK_NETDEV;
> +
> + fwnode = of_fwnode_handle(node);
> +
> + __set_bit(PHY_INTERFACE_MODE_10GBASER,
> + port->phy_config.supported_interfaces);
> + __set_bit(PHY_INTERFACE_MODE_2500BASEX,
> + port->phy_config.supported_interfaces);
> + __set_bit(PHY_INTERFACE_MODE_SGMII,
> + port->phy_config.supported_interfaces);
> + __set_bit(PHY_INTERFACE_MODE_1000BASEX,
> + port->phy_config.supported_interfaces);
> +
> + port->phy_config.mac_capabilities =
> + MAC_1000 | MAC_2500FD | MAC_10000FD;
> +
> + phy_link = phylink_create(&port->phy_config, fwnode,
> + PHY_INTERFACE_MODE_INTERNAL,
> + &prestera_mac_ops);
> + if (IS_ERR(phy_link)) {
> + netdev_err(port->dev, "failed to create phylink\n");
> + err = PTR_ERR(phy_link);
> + goto out;
> + }
> +
> + port->phy_link = phy_link;
> + break;
> }
>
> +out:
> + of_node_put(ports);
> return err;
> }
>
> +static int prestera_port_sfp_unbind(struct prestera_port *port)
> +{
> + if (port->phy_link)
> + phylink_destroy(port->phy_link);
> +
> + return 0;
> +}
> +
> static netdev_tx_t prestera_port_xmit(struct sk_buff *skb,
> struct net_device *dev)
> {
> @@ -380,8 +619,10 @@ static int prestera_port_create(struct prestera_switch *sw, u32 id)
> dev->features |= NETIF_F_NETNS_LOCAL | NETIF_F_HW_TC;
> dev->netdev_ops = &prestera_netdev_ops;
> dev->ethtool_ops = &prestera_ethtool_ops;
> + SET_NETDEV_DEV(dev, sw->dev->dev);
>
> - netif_carrier_off(dev);
> + if (port->caps.transceiver != PRESTERA_PORT_TCVR_SFP)
> + netif_carrier_off(dev);
>
> dev->mtu = min_t(unsigned int, sw->mtu_max, PRESTERA_MTU_DEFAULT);
> dev->min_mtu = sw->mtu_min;
> @@ -432,7 +673,7 @@ static int prestera_port_create(struct prestera_switch *sw, u32 id)
> cfg_mac.admin = false;
> cfg_mac.mode = PRESTERA_MAC_MODE_MAX;
> }
> - cfg_mac.inband = false;
> + cfg_mac.inband = 0;
> cfg_mac.speed = 0;
> cfg_mac.duplex = DUPLEX_UNKNOWN;
> cfg_mac.fec = PRESTERA_PORT_FEC_OFF;
> @@ -474,8 +715,13 @@ static int prestera_port_create(struct prestera_switch *sw, u32 id)
>
> prestera_devlink_port_set(port);
>
> + err = prestera_port_sfp_bind(port);
> + if (err)
> + goto err_sfp_bind;
> +
> return 0;
>
> +err_sfp_bind:
> err_register_netdev:
> prestera_port_list_del(port);
> err_port_init:
> @@ -521,8 +767,10 @@ static int prestera_create_ports(struct prestera_switch *sw)
> return 0;
>
> err_port_create:
> - list_for_each_entry_safe(port, tmp, &sw->port_list, list)
> + list_for_each_entry_safe(port, tmp, &sw->port_list, list) {
> + prestera_port_sfp_unbind(port);
> prestera_port_destroy(port);
> + }
>
> return err;
> }
> @@ -530,25 +778,47 @@ static int prestera_create_ports(struct prestera_switch *sw)
> static void prestera_port_handle_event(struct prestera_switch *sw,
> struct prestera_event *evt, void *arg)
> {
> + struct prestera_port_mac_state smac;
> + struct prestera_port_event *pevt;
> struct delayed_work *caching_dw;
> struct prestera_port *port;
>
> - port = prestera_find_port(sw, evt->port_evt.port_id);
> - if (!port || !port->dev)
> - return;
> -
> - caching_dw = &port->cached_hw_stats.caching_dw;
> -
> - prestera_ethtool_port_state_changed(port, &evt->port_evt);
> -
> if (evt->id == PRESTERA_PORT_EVENT_MAC_STATE_CHANGED) {
> + pevt = &evt->port_evt;
> + port = prestera_find_port(sw, pevt->port_id);
> + if (!port || !port->dev)
> + return;
> +
> + caching_dw = &port->cached_hw_stats.caching_dw;
> +
> + if (port->phy_link) {
> + memset(&smac, 0, sizeof(smac));
> + smac.valid = true;
> + smac.oper = pevt->data.mac.oper;
> + if (smac.oper) {
> + smac.mode = pevt->data.mac.mode;
> + smac.speed = pevt->data.mac.speed;
> + smac.duplex = pevt->data.mac.duplex;
> + smac.fc = pevt->data.mac.fc;
> + smac.fec = pevt->data.mac.fec;
> + phylink_mac_change(port->phy_link, true);
> + } else {
> + phylink_mac_change(port->phy_link, false);
> + }
> + prestera_port_mac_state_cache_write(port, &smac);
> + }
> +
> if (port->state_mac.oper) {
> - netif_carrier_on(port->dev);
> + if (!port->phy_link)
> + netif_carrier_on(port->dev);
> +
> if (!delayed_work_pending(caching_dw))
> queue_delayed_work(prestera_wq, caching_dw, 0);
> } else if (netif_running(port->dev) &&
> netif_carrier_ok(port->dev)) {
> - netif_carrier_off(port->dev);
> + if (!port->phy_link)
> + netif_carrier_off(port->dev);
> +
> if (delayed_work_pending(caching_dw))
> cancel_delayed_work(caching_dw);
> }
> @@ -571,19 +841,20 @@ static void prestera_event_handlers_unregister(struct prestera_switch *sw)
> static int prestera_switch_set_base_mac_addr(struct prestera_switch *sw)
> {
> struct device_node *base_mac_np;
> - struct device_node *np;
> int ret;
>
> - np = of_find_compatible_node(NULL, NULL, "marvell,prestera");
> - base_mac_np = of_parse_phandle(np, "base-mac-provider", 0);
> + if (sw->np) {
> + base_mac_np = of_parse_phandle(sw->np, "base-mac-provider", 0);
> + if (base_mac_np) {
> + ret = of_get_mac_address(base_mac_np, sw->base_mac);
> + of_node_put(base_mac_np);
> + }
> + }
>
> - ret = of_get_mac_address(base_mac_np, sw->base_mac);
> - if (ret) {
> + if (!is_valid_ether_addr(sw->base_mac) || ret) {
> eth_random_addr(sw->base_mac);
> dev_info(prestera_dev(sw), "using random base mac address\n");
> }
> - of_node_put(base_mac_np);
> - of_node_put(np);
>
> return prestera_hw_switch_mac_set(sw, sw->base_mac);
> }
> @@ -1083,6 +1354,8 @@ static int prestera_switch_init(struct prestera_switch *sw)
> {
> int err;
>
> + sw->np = of_find_compatible_node(NULL, NULL, "marvell,prestera");
> +
> err = prestera_hw_switch_init(sw);
> if (err) {
> dev_err(prestera_dev(sw), "Failed to init Switch device\n");
> @@ -1183,6 +1456,7 @@ static void prestera_switch_fini(struct prestera_switch *sw)
> prestera_router_fini(sw);
> prestera_netdev_event_handler_unregister(sw);
> prestera_hw_switch_fini(sw);
> + of_node_put(sw->np);
> }
>
> int prestera_device_register(struct prestera_device *dev)
> --
> 2.17.1
>
>

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

2022-07-14 17:22:12

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH V3 net-next] net: marvell: prestera: add phylink support

On Thu, 14 Jul 2022 13:55:16 +0300 Oleksandr Mazur wrote:
> For SFP port prestera driver will use kernel
> phylink infrastucture to configure port mode based on
> the module that has beed inserted
>
> Co-developed-by: Yevhen Orlov <[email protected]>
> Signed-off-by: Yevhen Orlov <[email protected]>
> Co-developed-by: Taras Chornyi <[email protected]>
> Signed-off-by: Taras Chornyi <[email protected]>
> Signed-off-by: Oleksandr Mazur <[email protected]>
>
> PATCH V3:
> - force inband mode for SGMII
> - fix >80 chars warning of checkpatch where possible (2/5)
> - structure phylink_mac_change alongside phylink-related if-clause;
> PATCH V2:
> - fix mistreat of bitfield values as if they were bools.
> - remove phylink_config ifdefs.
> - remove obsolete phylink pcs / mac callbacks;
> - rework mac (/pcs) config to not look for speed / duplex
> parameters while link is not yet set up.
> - remove unused functions.
> - add phylink select cfg to prestera Kconfig.
> ---

Please put the changelog under the --- separator, otherwise when
I apply it my tag will get added at the end, after the changelog.
Since we add links to the commits now it's okay to put the changelog
after the --- and not keep it in the tree.