Messages in this thread | | | Date | Tue, 22 Mar 2022 13:58:12 +0200 | From | Vladimir Oltean <> | Subject | Re: [net-next PATCH 1/4] drivers: net: dsa: qca8k: drop MTU tracking from qca8k_priv |
| |
On Tue, Mar 22, 2022 at 02:45:03AM +0100, Ansuel Smith wrote: > Drop the MTU array from qca8k_priv and use slave net dev to get the max > MTU across all user port. CPU port can be skipped as DSA already make > sure CPU port are set to the max MTU across all ports. > > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com> > ---
I hardly find this to be an improvement and I would rather not see such unjustified complexity in a device driver. What are the concrete benefits, size wise?
> drivers/net/dsa/qca8k.c | 38 +++++++++++++++++++++++--------------- > drivers/net/dsa/qca8k.h | 1 - > 2 files changed, 23 insertions(+), 16 deletions(-) > > diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c > index d3ed0a7f8077..4366d87b4bbd 100644 > --- a/drivers/net/dsa/qca8k.c > +++ b/drivers/net/dsa/qca8k.c > @@ -2367,13 +2367,31 @@ static int > qca8k_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu) > { > struct qca8k_priv *priv = ds->priv; > - int i, mtu = 0; > + struct dsa_port *dp; > + int mtu = new_mtu; > > - priv->port_mtu[port] = new_mtu; > + /* We have only have a general MTU setting. So check > + * every port and set the max across all port. > + */ > + list_for_each_entry(dp, &ds->dst->ports, list) { > + /* We can ignore cpu port, DSA will itself chose > + * the max MTU across all port > + */ > + if (!dsa_port_is_user(dp)) > + continue; > > - for (i = 0; i < QCA8K_NUM_PORTS; i++) > - if (priv->port_mtu[i] > mtu) > - mtu = priv->port_mtu[i]; > + if (dp->index == port) > + continue; > + > + /* Address init phase where not every port have > + * a slave device > + */ > + if (!dp->slave) > + continue; > + > + if (mtu < dp->slave->mtu) > + mtu = dp->slave->mtu; > + } > > /* Include L2 header / FCS length */ > return qca8k_write(priv, QCA8K_MAX_FRAME_SIZE, mtu + ETH_HLEN + ETH_FCS_LEN); > @@ -3033,16 +3051,6 @@ qca8k_setup(struct dsa_switch *ds) > QCA8K_PORT_HOL_CTRL1_WRED_EN, > mask); > } > - > - /* Set initial MTU for every port. > - * We have only have a general MTU setting. So track > - * every port and set the max across all port. > - * Set per port MTU to 1500 as the MTU change function > - * will add the overhead and if its set to 1518 then it > - * will apply the overhead again and we will end up with > - * MTU of 1536 instead of 1518 > - */ > - priv->port_mtu[i] = ETH_DATA_LEN; > } > > /* Special GLOBAL_FC_THRESH value are needed for ar8327 switch */ > diff --git a/drivers/net/dsa/qca8k.h b/drivers/net/dsa/qca8k.h > index f375627174c8..562d75997e55 100644 > --- a/drivers/net/dsa/qca8k.h > +++ b/drivers/net/dsa/qca8k.h > @@ -398,7 +398,6 @@ struct qca8k_priv { > struct device *dev; > struct dsa_switch_ops ops; > struct gpio_desc *reset_gpio; > - unsigned int port_mtu[QCA8K_NUM_PORTS]; > struct net_device *mgmt_master; /* Track if mdio/mib Ethernet is available */ > struct qca8k_mgmt_eth_data mgmt_eth_data; > struct qca8k_mib_eth_data mib_eth_data; > -- > 2.34.1 >
| |