Aw: Re: [PATCH v2] net: ethernet: mtk_eth_soc: fix mtu warning

Commit Message

Frank Wunderlich July 9, 2020, 7:48 p.m. UTC

> Gesendet: Donnerstag, 09. Juli 2020 um 15:41 Uhr
> Von: "Andrew Lunn" <[e-mail]>

> > +	eth->netdev[id]->max_mtu = 1536;
>
> I assume this is enough to make the DSA warning go away, but it is the
> true max? I have a similar patch for the FEC driver which i should
> post sometime. Reviewing the FEC code and after some testing, i found
> the real max was 2K - 64.

i tried setting only the max_mtu, but the dsa-error is still present

mt7530 mdio-bus:00: nonfatal error -95 setting MTU on port 0

but i got it too, if i revert the change...mhm, strange that these were absent last time...

the other 2 are fixed with only max_mtu.
@andrew where did you got the 2k-64 (=1984) information? sounds like orwell ;)

-95 is EOPNOTSUPP

as so far, commit 72579e14a1d3d3d561039dfe7e5f47aaf22e3fd3 introduces the warning,
but the change is making it non-fatal...so i need to adjust my fixes-tag.
i guess the real problem lies in this:

bfcb813203e6 net: dsa: configure the MTU for switch ports

it looks like dsa_slave_change_mtu failes because of missing callback in mtk_driver (mt7530 for mt7531 in my case).

net/dsa/slave.c
1405 static int dsa_slave_change_mtu(struct net_device *dev, int new_mtu)
...
1420     if (!ds->ops->port_change_mtu)
1421         return -EOPNOTSUPP;

i added an empty callback to avoid this message, but mtu should be set in hardware too...
here i will ne some assistance from mtk ethernet experts and mt7531 driver (from landen chao) to be merged first (after some needed changes)

Comments

Andrew Lunn July 9, 2020, 8:22 p.m. UTC | #1

On Thu, Jul 09, 2020 at 09:48:01PM +0200, Frank Wunderlich wrote:
> > Gesendet: Donnerstag, 09. Juli 2020 um 15:41 Uhr
> > Von: "Andrew Lunn" <[e-mail]>
> 
> > > +	eth->netdev[id]->max_mtu = 1536;
> >
> > I assume this is enough to make the DSA warning go away, but it is the
> > true max? I have a similar patch for the FEC driver which i should
> > post sometime. Reviewing the FEC code and after some testing, i found
> > the real max was 2K - 64.
> 
> i tried setting only the max_mtu, but the dsa-error is still present
> 
> mt7530 mdio-bus:00: nonfatal error -95 setting MTU on port 0
> 
> but i got it too, if i revert the change...mhm, strange that these were absent last time...
> 
> the other 2 are fixed with only max_mtu.
> @andrew where did you got the 2k-64 (=1984) information? sounds like orwell ;)

drivers/net/ethernet/freescale/fec_main.c:

/* The FEC stores dest/src/type/vlan, data, and checksum for receive packets.
 *
 * 2048 byte skbufs are allocated. However, alignment requirements
 * varies between FEC variants. Worst case is 64, so round down by 64.
 */
#define PKT_MAXBUF_SIZE         (round_down(2048 - 64, 64))

So i set the max MTU to this.

> 1405 static int dsa_slave_change_mtu(struct net_device *dev, int new_mtu)
> ...
> 1420     if (!ds->ops->port_change_mtu)
> 1421         return -EOPNOTSUPP;

Yes, i also needed to change the mv88e6xxx driver to implement this
function. These switches do support jumbo frames, so i had some real
code in there, not a dummy function.

The marketing brief for the mt7530 says it supports 1518, 1536, 1552
and 9K jumbo frames. It would be good if you can figure out how to
support that, rather than add a dummy function.

	Andrew

Patch

--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2259,6 +2259,12 @@  mt753x_phy_write(struct dsa_switch *ds, int port, int regnum, u16 val)
        return priv->info->phy_write(ds, port, regnum, val);
 }

+static int
+mt753x_port_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
+{
+       return 0;
+}
+
 static const struct dsa_switch_ops mt7530_switch_ops = {
        .get_tag_protocol       = mtk_get_tag_protocol,
        .setup                  = mt753x_setup,
@@ -2281,6 +2287,7 @@  static const struct dsa_switch_ops mt7530_switch_ops = {
        .port_vlan_del          = mt7530_port_vlan_del,
        .port_mirror_add        = mt7530_port_mirror_add,
        .port_mirror_del        = mt7530_port_mirror_del,
+       .port_change_mtu        = mt753x_port_change_mtu,
        .phylink_validate       = mt753x_phylink_validate,
        .phylink_mac_link_state = mt7530_phylink_mac_link_state,
        .phylink_mac_config     = mt753x_phylink_mac_config,