This patch set addresses some of the feedback received on the dpaa2-ethsw
driver: https://lore.kernel.org/patchwork/patch/1113335/.
There are no functional changes but rather just code cleanup.
Changes in v2:
- added Reported-by and Suggested-by tags
Ioana Ciornei (10):
staging: fsl-dpaa2/ethsw: remove IGMP default address
staging: fsl-dpaa2/ethsw: enable switch ports only on dev_open
staging: fsl-dpaa2/ethsw: add line terminator to all formats
staging: fsl-dpaa2/ethsw: remove debug message
staging: fsl-dpaa2/ethsw: use bool when encoding learning/flooding
state
staging: fsl-dpaa2/ethsw: remove unnecessary memset
staging: fsl-dpaa2/ethsw: remove redundant VLAN check
staging: fsl-dpaa2/ethsw: reword error message
staging: fsl-dpaa2/ethsw: register_netdev only when ready
staging: fsl-dpaa2/ethsw: do not force user to bring interface down
drivers/staging/fsl-dpaa2/ethsw/ethsw-ethtool.c | 44 +++++-----
drivers/staging/fsl-dpaa2/ethsw/ethsw.c | 110 ++++++------------------
2 files changed, 50 insertions(+), 104 deletions(-)
--
1.9.1
Do not add an IGMP multicast address by default since we do not support
Rx/Tx ar the moment.
Reported-by: Andrew Lunn <[email protected]>
Signed-off-by: Ioana Ciornei <[email protected]>
---
Changes in v2:
- added Reported-by tag
drivers/staging/fsl-dpaa2/ethsw/ethsw.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
index aac98ece2335..8032314d5cae 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
+++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
@@ -1506,7 +1506,6 @@ static int ethsw_init(struct fsl_mc_device *sw_dev)
static int ethsw_port_init(struct ethsw_port_priv *port_priv, u16 port)
{
- const char def_mcast[ETH_ALEN] = {0x01, 0x00, 0x5e, 0x00, 0x00, 0x01};
struct net_device *netdev = port_priv->netdev;
struct ethsw_core *ethsw = port_priv->ethsw_data;
struct dpsw_vlan_if_cfg vcfg;
@@ -1532,12 +1531,10 @@ static int ethsw_port_init(struct ethsw_port_priv *port_priv, u16 port)
err = dpsw_vlan_remove_if(ethsw->mc_io, 0, ethsw->dpsw_handle,
DEFAULT_VLAN_ID, &vcfg);
- if (err) {
+ if (err)
netdev_err(netdev, "dpsw_vlan_remove_if err %d\n", err);
- return err;
- }
- return ethsw_port_fdb_add_mc(port_priv, def_mcast);
+ return err;
}
static void ethsw_unregister_notifier(struct device *dev)
--
1.9.1
The ethtool core already zeroes the memory before calling
.get_ethtool_stats() thus making the memset unnecessary. Remove it.
Reported-by: Andrew Lunn <[email protected]>
Signed-off-by: Ioana Ciornei <[email protected]>
---
Changes in v2:
- added Reported-by tag
drivers/staging/fsl-dpaa2/ethsw/ethsw-ethtool.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw-ethtool.c b/drivers/staging/fsl-dpaa2/ethsw/ethsw-ethtool.c
index 12e33a3a1bf1..0f9f8345e534 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/ethsw-ethtool.c
+++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw-ethtool.c
@@ -149,9 +149,6 @@ static void ethsw_ethtool_get_stats(struct net_device *netdev,
struct ethsw_port_priv *port_priv = netdev_priv(netdev);
int i, err;
- memset(data, 0,
- sizeof(u64) * ETHSW_NUM_COUNTERS);
-
for (i = 0; i < ETHSW_NUM_COUNTERS; i++) {
err = dpsw_if_get_counter(port_priv->ethsw_data->mc_io, 0,
port_priv->ethsw_data->dpsw_handle,
--
1.9.1
In the current state, the dpaa2-ethsw driver supports only one bridge
per DPSW object. Reword the error message so that this information is
much more clear.
Suggested-by: Andrew Lunn <[email protected]>
Signed-off-by: Ioana Ciornei <[email protected]>
---
Changes in v2:
- added Suggested-by tag
drivers/staging/fsl-dpaa2/ethsw/ethsw.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
index b69b2b7be972..28da109aef5e 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
+++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
@@ -1119,8 +1119,7 @@ static int port_bridge_join(struct net_device *netdev,
if (ethsw->ports[i]->bridge_dev &&
(ethsw->ports[i]->bridge_dev != upper_dev)) {
netdev_err(netdev,
- "Another switch port is connected to %s\n",
- ethsw->ports[i]->bridge_dev->name);
+ "Only one bridge supported per DPSW object!\n");
return -EINVAL;
}
--
1.9.1
Use a bool instead of an u8 in ethsw_set_learning() and
ethsw_port_set_flood() to encode an binary type property.
Reported-by: Andrew Lunn <[email protected]>
Signed-off-by: Ioana Ciornei <[email protected]>
---
Changes in v2:
- added Reported-by tag
drivers/staging/fsl-dpaa2/ethsw/ethsw.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
index 9ade73928e60..20519af4d804 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
+++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
@@ -149,12 +149,12 @@ static int ethsw_port_add_vlan(struct ethsw_port_priv *port_priv,
return 0;
}
-static int ethsw_set_learning(struct ethsw_core *ethsw, u8 flag)
+static int ethsw_set_learning(struct ethsw_core *ethsw, bool enable)
{
enum dpsw_fdb_learning_mode learn_mode;
int err;
- if (flag)
+ if (enable)
learn_mode = DPSW_FDB_LEARNING_MODE_HW;
else
learn_mode = DPSW_FDB_LEARNING_MODE_DIS;
@@ -165,24 +165,24 @@ static int ethsw_set_learning(struct ethsw_core *ethsw, u8 flag)
dev_err(ethsw->dev, "dpsw_fdb_set_learning_mode err %d\n", err);
return err;
}
- ethsw->learning = !!flag;
+ ethsw->learning = enable;
return 0;
}
-static int ethsw_port_set_flood(struct ethsw_port_priv *port_priv, u8 flag)
+static int ethsw_port_set_flood(struct ethsw_port_priv *port_priv, bool enable)
{
int err;
err = dpsw_if_set_flooding(port_priv->ethsw_data->mc_io, 0,
port_priv->ethsw_data->dpsw_handle,
- port_priv->idx, flag);
+ port_priv->idx, enable);
if (err) {
netdev_err(port_priv->netdev,
"dpsw_if_set_flooding err %d\n", err);
return err;
}
- port_priv->flood = !!flag;
+ port_priv->flood = enable;
return 0;
}
--
1.9.1
Since ethtool will be loud enough if the .set_link_ksettings() callback
fails, remove the debug messages which do not add additional
information.
Reported-by: Andrew Lunn <[email protected]>
Signed-off-by: Ioana Ciornei <[email protected]>
---
Changes in v2:
- added Reported-by tag
drivers/staging/fsl-dpaa2/ethsw/ethsw-ethtool.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw-ethtool.c b/drivers/staging/fsl-dpaa2/ethsw/ethsw-ethtool.c
index 95e9f1096999..12e33a3a1bf1 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/ethsw-ethtool.c
+++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw-ethtool.c
@@ -91,8 +91,6 @@ static void ethsw_get_drvinfo(struct net_device *netdev,
struct dpsw_link_cfg cfg = {0};
int err = 0;
- netdev_dbg(netdev, "Setting link parameters...");
-
/* Due to a temporary MC limitation, the DPSW port must be down
* in order to be able to change link settings. Taking steps to let
* the user know that.
@@ -116,11 +114,6 @@ static void ethsw_get_drvinfo(struct net_device *netdev,
port_priv->ethsw_data->dpsw_handle,
port_priv->idx,
&cfg);
- if (err)
- /* ethtool will be loud enough if we return an error; no point
- * in putting our own error message on the console by default
- */
- netdev_dbg(netdev, "ERROR %d setting link cfg", err);
return err;
}
--
1.9.1
At probe time, only the DPSW object should be enabled without the
associated ports, which will get enabled on dev_open. Remove the
ethsw_open() and ethsw_stop() functions and replace them only with
dpsw_enable()/_disable().
Reported-by: Andrew Lunn <[email protected]>
Signed-off-by: Ioana Ciornei <[email protected]>
---
Changes in v2:
- added Reported-by tag
drivers/staging/fsl-dpaa2/ethsw/ethsw.c | 59 ++++-----------------------------
1 file changed, 6 insertions(+), 53 deletions(-)
diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
index 8032314d5cae..302842c3bdfe 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
+++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
@@ -1363,48 +1363,6 @@ static int ethsw_register_notifier(struct device *dev)
return err;
}
-static int ethsw_open(struct ethsw_core *ethsw)
-{
- struct ethsw_port_priv *port_priv = NULL;
- int i, err;
-
- err = dpsw_enable(ethsw->mc_io, 0, ethsw->dpsw_handle);
- if (err) {
- dev_err(ethsw->dev, "dpsw_enable err %d\n", err);
- return err;
- }
-
- for (i = 0; i < ethsw->sw_attr.num_ifs; i++) {
- port_priv = ethsw->ports[i];
- err = dev_open(port_priv->netdev, NULL);
- if (err) {
- netdev_err(port_priv->netdev, "dev_open err %d\n", err);
- return err;
- }
- }
-
- return 0;
-}
-
-static int ethsw_stop(struct ethsw_core *ethsw)
-{
- struct ethsw_port_priv *port_priv = NULL;
- int i, err;
-
- for (i = 0; i < ethsw->sw_attr.num_ifs; i++) {
- port_priv = ethsw->ports[i];
- dev_close(port_priv->netdev);
- }
-
- err = dpsw_disable(ethsw->mc_io, 0, ethsw->dpsw_handle);
- if (err) {
- dev_err(ethsw->dev, "dpsw_disable err %d\n", err);
- return err;
- }
-
- return 0;
-}
-
static int ethsw_init(struct fsl_mc_device *sw_dev)
{
struct device *dev = &sw_dev->dev;
@@ -1586,9 +1544,7 @@ static int ethsw_remove(struct fsl_mc_device *sw_dev)
destroy_workqueue(ethsw_owq);
- rtnl_lock();
- ethsw_stop(ethsw);
- rtnl_unlock();
+ dpsw_disable(ethsw->mc_io, 0, ethsw->dpsw_handle);
for (i = 0; i < ethsw->sw_attr.num_ifs; i++) {
port_priv = ethsw->ports[i];
@@ -1708,12 +1664,11 @@ static int ethsw_probe(struct fsl_mc_device *sw_dev)
goto err_free_ports;
}
- /* Switch starts up enabled */
- rtnl_lock();
- err = ethsw_open(ethsw);
- rtnl_unlock();
- if (err)
+ err = dpsw_enable(ethsw->mc_io, 0, ethsw->dpsw_handle);
+ if (err) {
+ dev_err(ethsw->dev, "dpsw_enable err %d\n", err);
goto err_free_ports;
+ }
/* Setup IRQs */
err = ethsw_setup_irqs(sw_dev);
@@ -1724,9 +1679,7 @@ static int ethsw_probe(struct fsl_mc_device *sw_dev)
return 0;
err_stop:
- rtnl_lock();
- ethsw_stop(ethsw);
- rtnl_unlock();
+ dpsw_disable(ethsw->mc_io, 0, ethsw->dpsw_handle);
err_free_ports:
/* Cleanup registered ports only */
--
1.9.1