2017-08-04 22:26:26

by Vivien Didelot

[permalink] [raw]
Subject: [PATCH net-next 0/3] net: dsa: remove unnecessary arguments

Several DSA core functions take many arguments, mostly because the
legacy code does not assign ds->dev. This patch series assigns ds->dev
in legacy and removes the unnecessary arguments of these functions,
where either the dsa_switch or dsa_port argument is enough.

Vivien Didelot (3):
net: dsa: assign switch device in legacy code
net: dsa: remove useless args of dsa_cpu_dsa_setup
net: dsa: remove useless args of dsa_slave_create

net/dsa/dsa.c | 10 +++++-----
net/dsa/dsa2.c | 6 +++---
net/dsa/dsa_priv.h | 6 ++----
net/dsa/legacy.c | 19 +++++++++----------
net/dsa/slave.c | 14 +++++++-------
5 files changed, 26 insertions(+), 29 deletions(-)

--
2.13.3


2017-08-04 22:26:29

by Vivien Didelot

[permalink] [raw]
Subject: [PATCH net-next 3/3] net: dsa: remove useless args of dsa_slave_create

dsa_slave_create currently takes 4 arguments while it only needs the
related dsa_port and its name. Remove all other arguments.

Signed-off-by: Vivien Didelot <[email protected]>
---
net/dsa/dsa2.c | 2 +-
net/dsa/dsa_priv.h | 3 +--
net/dsa/legacy.c | 2 +-
net/dsa/slave.c | 14 +++++++-------
4 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 2a0120493cf1..cceaa4dd9f53 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -275,7 +275,7 @@ static int dsa_user_port_apply(struct dsa_port *port)
if (!name)
name = "eth%d";

- err = dsa_slave_create(ds, ds->dev, port->index, name);
+ err = dsa_slave_create(port, name);
if (err) {
dev_warn(ds->dev, "Failed to create slave %d: %d\n",
port->index, err);
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 46851c91c7fe..73426f9c2cca 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -148,8 +148,7 @@ int dsa_port_vlan_dump(struct dsa_port *dp,
extern const struct dsa_device_ops notag_netdev_ops;
void dsa_slave_mii_bus_init(struct dsa_switch *ds);
void dsa_cpu_port_ethtool_init(struct ethtool_ops *ops);
-int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
- int port, const char *name);
+int dsa_slave_create(struct dsa_port *port, const char *name);
void dsa_slave_destroy(struct net_device *slave_dev);
int dsa_slave_suspend(struct net_device *slave_dev);
int dsa_slave_resume(struct net_device *slave_dev);
diff --git a/net/dsa/legacy.c b/net/dsa/legacy.c
index 05be0bc10735..db7c1eaad078 100644
--- a/net/dsa/legacy.c
+++ b/net/dsa/legacy.c
@@ -194,7 +194,7 @@ static int dsa_switch_setup_one(struct dsa_switch *ds,
if (!(ds->enabled_port_mask & (1 << i)))
continue;

- ret = dsa_slave_create(ds, ds->dev, i, cd->port_names[i]);
+ ret = dsa_slave_create(&ds->ports[i], cd->port_names[i]);
if (ret < 0)
netdev_err(master, "[%d]: can't create dsa slave device for port %d(%s): %d\n",
index, i, cd->port_names[i], ret);
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index e196562035b1..c8eb33746850 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1165,9 +1165,9 @@ int dsa_slave_resume(struct net_device *slave_dev)
return 0;
}

-int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
- int port, const char *name)
+int dsa_slave_create(struct dsa_port *port, const char *name)
{
+ struct dsa_switch *ds = port->ds;
struct dsa_switch_tree *dst = ds->dst;
struct net_device *master;
struct net_device *slave_dev;
@@ -1197,13 +1197,13 @@ int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
netdev_for_each_tx_queue(slave_dev, dsa_slave_set_lockdep_class_one,
NULL);

- SET_NETDEV_DEV(slave_dev, parent);
- slave_dev->dev.of_node = ds->ports[port].dn;
+ SET_NETDEV_DEV(slave_dev, port->ds->dev);
+ slave_dev->dev.of_node = port->dn;
slave_dev->vlan_features = master->vlan_features;

p = netdev_priv(slave_dev);
u64_stats_init(&p->stats64.syncp);
- p->dp = &ds->ports[port];
+ p->dp = port;
INIT_LIST_HEAD(&p->mall_tc_list);
p->xmit = dst->tag_ops->xmit;

@@ -1211,12 +1211,12 @@ int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
p->old_link = -1;
p->old_duplex = -1;

- ds->ports[port].netdev = slave_dev;
+ port->netdev = slave_dev;
ret = register_netdev(slave_dev);
if (ret) {
netdev_err(master, "error %d registering interface %s\n",
ret, slave_dev->name);
- ds->ports[port].netdev = NULL;
+ port->netdev = NULL;
free_netdev(slave_dev);
return ret;
}
--
2.13.3

2017-08-04 22:26:28

by Vivien Didelot

[permalink] [raw]
Subject: [PATCH net-next 1/3] net: dsa: assign switch device in legacy code

Assign the parent device to the dev member of the newly allocated
dsa_switch structure in the legacy dsa_switch_setup function, so that
the underlying dsa_switch_setup_one and dsa_cpu_dsa_setups functions can
access it instead of requiring an additional struct device argument.

Signed-off-by: Vivien Didelot <[email protected]>
---
net/dsa/legacy.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/net/dsa/legacy.c b/net/dsa/legacy.c
index 1d7a3282f2a7..c565787e1c78 100644
--- a/net/dsa/legacy.c
+++ b/net/dsa/legacy.c
@@ -78,7 +78,7 @@ dsa_switch_probe(struct device *parent, struct device *host_dev, int sw_addr,
}

/* basic switch operations **************************************************/
-static int dsa_cpu_dsa_setups(struct dsa_switch *ds, struct device *dev)
+static int dsa_cpu_dsa_setups(struct dsa_switch *ds)
{
struct dsa_port *dport;
int ret, port;
@@ -88,15 +88,15 @@ static int dsa_cpu_dsa_setups(struct dsa_switch *ds, struct device *dev)
continue;

dport = &ds->ports[port];
- ret = dsa_cpu_dsa_setup(ds, dev, dport, port);
+ ret = dsa_cpu_dsa_setup(ds, ds->dev, dport, port);
if (ret)
return ret;
}
return 0;
}

-static int dsa_switch_setup_one(struct dsa_switch *ds, struct net_device *master,
- struct device *parent)
+static int dsa_switch_setup_one(struct dsa_switch *ds,
+ struct net_device *master)
{
const struct dsa_switch_ops *ops = ds->ops;
struct dsa_switch_tree *dst = ds->dst;
@@ -176,7 +176,7 @@ static int dsa_switch_setup_one(struct dsa_switch *ds, struct net_device *master
}

if (!ds->slave_mii_bus && ops->phy_read) {
- ds->slave_mii_bus = devm_mdiobus_alloc(parent);
+ ds->slave_mii_bus = devm_mdiobus_alloc(ds->dev);
if (!ds->slave_mii_bus)
return -ENOMEM;
dsa_slave_mii_bus_init(ds);
@@ -196,14 +196,14 @@ static int dsa_switch_setup_one(struct dsa_switch *ds, struct net_device *master
if (!(ds->enabled_port_mask & (1 << i)))
continue;

- ret = dsa_slave_create(ds, parent, i, cd->port_names[i]);
+ ret = dsa_slave_create(ds, ds->dev, i, cd->port_names[i]);
if (ret < 0)
netdev_err(master, "[%d]: can't create dsa slave device for port %d(%s): %d\n",
index, i, cd->port_names[i], ret);
}

/* Perform configuration of the CPU and DSA ports */
- ret = dsa_cpu_dsa_setups(ds, parent);
+ ret = dsa_cpu_dsa_setups(ds);
if (ret < 0)
netdev_err(master, "[%d] : can't configure CPU and DSA ports\n",
index);
@@ -251,8 +251,9 @@ dsa_switch_setup(struct dsa_switch_tree *dst, struct net_device *master,
ds->cd = cd;
ds->ops = ops;
ds->priv = priv;
+ ds->dev = parent;

- ret = dsa_switch_setup_one(ds, master, parent);
+ ret = dsa_switch_setup_one(ds, master);
if (ret)
return ERR_PTR(ret);

--
2.13.3

2017-08-04 22:27:37

by Vivien Didelot

[permalink] [raw]
Subject: [PATCH net-next 2/3] net: dsa: remove useless args of dsa_cpu_dsa_setup

dsa_cpu_dsa_setup currently takes 4 arguments but they are all available
from the dsa_port argument. Remove all others.

Signed-off-by: Vivien Didelot <[email protected]>
---
net/dsa/dsa.c | 10 +++++-----
net/dsa/dsa2.c | 4 ++--
net/dsa/dsa_priv.h | 3 +--
net/dsa/legacy.c | 4 +---
4 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 0ba842c08dd3..64db6eece3c1 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -67,17 +67,17 @@ const struct dsa_device_ops *dsa_device_ops[DSA_TAG_LAST] = {
[DSA_TAG_PROTO_NONE] = &none_ops,
};

-int dsa_cpu_dsa_setup(struct dsa_switch *ds, struct device *dev,
- struct dsa_port *dport, int port)
+int dsa_cpu_dsa_setup(struct dsa_port *port)
{
- struct device_node *port_dn = dport->dn;
+ struct device_node *port_dn = port->dn;
+ struct dsa_switch *ds = port->ds;
struct phy_device *phydev;
int ret, mode;

if (of_phy_is_fixed_link(port_dn)) {
ret = of_phy_register_fixed_link(port_dn);
if (ret) {
- dev_err(dev, "failed to register fixed PHY\n");
+ dev_err(ds->dev, "failed to register fixed PHY\n");
return ret;
}
phydev = of_phy_find_device(port_dn);
@@ -90,7 +90,7 @@ int dsa_cpu_dsa_setup(struct dsa_switch *ds, struct device *dev,
genphy_config_init(phydev);
genphy_read_status(phydev);
if (ds->ops->adjust_link)
- ds->ops->adjust_link(ds, port, phydev);
+ ds->ops->adjust_link(ds, port->index, phydev);

put_device(&phydev->mdio.dev);
}
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index c442051d5a55..2a0120493cf1 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -219,7 +219,7 @@ static int dsa_dsa_port_apply(struct dsa_port *port)
struct dsa_switch *ds = port->ds;
int err;

- err = dsa_cpu_dsa_setup(ds, ds->dev, port, port->index);
+ err = dsa_cpu_dsa_setup(port);
if (err) {
dev_warn(ds->dev, "Failed to setup dsa port %d: %d\n",
port->index, err);
@@ -243,7 +243,7 @@ static int dsa_cpu_port_apply(struct dsa_port *port)
struct dsa_switch *ds = port->ds;
int err;

- err = dsa_cpu_dsa_setup(ds, ds->dev, port, port->index);
+ err = dsa_cpu_dsa_setup(port);
if (err) {
dev_warn(ds->dev, "Failed to setup cpu port %d: %d\n",
port->index, err);
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 7aa0656296c2..46851c91c7fe 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -101,8 +101,7 @@ struct dsa_slave_priv {
};

/* dsa.c */
-int dsa_cpu_dsa_setup(struct dsa_switch *ds, struct device *dev,
- struct dsa_port *dport, int port);
+int dsa_cpu_dsa_setup(struct dsa_port *port);
void dsa_cpu_dsa_destroy(struct dsa_port *dport);
const struct dsa_device_ops *dsa_resolve_tag_protocol(int tag_protocol);
int dsa_cpu_port_ethtool_setup(struct dsa_port *cpu_dp);
diff --git a/net/dsa/legacy.c b/net/dsa/legacy.c
index c565787e1c78..05be0bc10735 100644
--- a/net/dsa/legacy.c
+++ b/net/dsa/legacy.c
@@ -80,15 +80,13 @@ dsa_switch_probe(struct device *parent, struct device *host_dev, int sw_addr,
/* basic switch operations **************************************************/
static int dsa_cpu_dsa_setups(struct dsa_switch *ds)
{
- struct dsa_port *dport;
int ret, port;

for (port = 0; port < ds->num_ports; port++) {
if (!(dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)))
continue;

- dport = &ds->ports[port];
- ret = dsa_cpu_dsa_setup(ds, ds->dev, dport, port);
+ ret = dsa_cpu_dsa_setup(&ds->ports[port]);
if (ret)
return ret;
}
--
2.13.3

2017-08-05 00:55:33

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net-next 1/3] net: dsa: assign switch device in legacy code

> @@ -251,8 +251,9 @@ dsa_switch_setup(struct dsa_switch_tree *dst, struct net_device *master,
> ds->cd = cd;
> ds->ops = ops;
> ds->priv = priv;
> + ds->dev = parent;

Hi Vivien

Is this even needed? dsa_switch_alloc() does ds->dev = dev.

Andrew

2017-08-05 20:12:32

by Vivien Didelot

[permalink] [raw]
Subject: Re: [PATCH net-next 1/3] net: dsa: assign switch device in legacy code

Hi Andrew,

Andrew Lunn <[email protected]> writes:

>> @@ -251,8 +251,9 @@ dsa_switch_setup(struct dsa_switch_tree *dst, struct net_device *master,
>> ds->cd = cd;
>> ds->ops = ops;
>> ds->priv = priv;
>> + ds->dev = parent;
>
> Is this even needed? dsa_switch_alloc() does ds->dev = dev.

You are correct! Respinning.


Thanks,

Vivien