Messages in this thread | | | Date | Tue, 22 Mar 2022 15:55:35 +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:38:08PM +0100, Ansuel Smith wrote: > On Tue, Mar 22, 2022 at 01:58:12PM +0200, Vladimir Oltean wrote: > > 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? > > > > The main idea here is, if the value is already present and accessible, > why should we duplicate it? Tracking the MTU in this custom way already > caused some bugs (check the comment i'm removing). We both use standard > way to track ports MTU and we save some additional space. At the cost of > 2 additional checks are are not that much of a problem.
Where is the bug?
> Also from this I discovered that (at least on ipq806x that use stmmac) > when master needs to change MTU, stmmac complains that the interface is > up and it must be put down. Wonder if that's common across other drivers > or it's only specific to stmmac.
I never had the pleasure of dealing with such DSA masters. I wonder why can't stmmac_change_mtu() check if netif_running(), call dev_close and set a bool, and at the end, if the bool was set, call dev_open back?
| |