Messages in this thread | | | Date | Tue, 14 Mar 2023 15:03:47 +0300 | From | Serge Semin <> | Subject | Re: [PATCH net 09/13] net: stmmac: Remove default maxmtu DT-platform setting |
| |
On Tue, Mar 14, 2023 at 12:20:18PM +0100, Piotr Raczynski wrote: > On Tue, Mar 14, 2023 at 01:42:33AM +0300, Serge Semin wrote: > > Initializing maxmtu platform parameter in the stmmac_probe_config_dt() > > method by default makes being pointless the DW MAC-specific maximum MTU > > selection algorithm implemented in the stmmac_dvr_probe() method. At least > > for xGMAC we'll always have a frame MTU limited with 9000 while it > > supports units up to 16KB. Let's remove the default initialization of > > the maxmtu platform setting then. We don't replace it with setting the > > maxmtu with some greater value because a default maximum MTU is > > calculated later in the stmmac_dvr_probe() anyway. That would have been a > > pointless limitation too. Instead from now the main STMMAC driver code > > will consider the out of bounds maxmtu value as invalid and will silently > > replace it with a maximum MTU value specific to the corresponding DW MAC. > > > > Note this alteration will only affect the xGMAC IP-cores due to the way > > the MTU autodetecion algorithm is implemented. So from now the driver will > > permit DW xGMACs to handle frames up to 16KB length (XGMAC_JUMBO_LEN). As > > before DW GMAC IP-cores of v4.0 and higher and IP-cores with enhanced > > descriptor support will be able to work with frames up to 8KB (JUMBO_LEN). > > The rest of the NICs will support frames of SKB_MAX_HEAD(NET_SKB_PAD + > > NET_IP_ALIGN) size. > > > > Fixes: 7d9e6c5afab6 ("net: stmmac: Integrate XGMAC into main driver flow") > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > --- > > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 ---- > > drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 5 ----- > > 2 files changed, 9 deletions(-) > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > index 32aa7953d296..e5cb4edc4e23 100644 > > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > @@ -7252,10 +7252,6 @@ int stmmac_dvr_probe(struct device *device, > > if ((priv->plat->maxmtu < ndev->max_mtu) && > > (priv->plat->maxmtu >= ndev->min_mtu)) > > ndev->max_mtu = priv->plat->maxmtu;
> > - else if (priv->plat->maxmtu < ndev->min_mtu) > > - dev_warn(priv->device, > > - "%s: warning: maxmtu having invalid value (%d)\n", > > - __func__, priv->plat->maxmtu); > > Looks fine but by removing plat->maxmtu = JUMBO_LEN; you eliminate the > case of dev_warn here or you remove dev_warn since the driver will be > able to fix the mtu value?
That warning is kind of odd if not to say contradicting. Why having max_mtu being less than the HW-specific min-value is more dangerous than having it being greater than the max-value for which there is no warning? The driver gets to the HW-specific min and max MTU values in anyway. Why to warn in one case and leaving unnoticed another?..
Anyway getting back to this patch change I remove the warning since plat_stmmaceth_data.maxmtu is now will be initialized with zero most of the time (if glue-drivers leave it uninitialized or there is no "max-frame-size" DT-property specified) which will falsely trigger that warning.
-Serge(y)
> > > > if (flow_ctrl) > > priv->flow_ctrl = FLOW_AUTO; /* RX/TX pause on */ > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c > > index 067a40fe0a23..857411105a0a 100644 > > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c > > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c > > @@ -468,11 +468,6 @@ stmmac_probe_config_dt(struct platform_device *pdev, u8 *mac) > > plat->en_tx_lpi_clockgating = > > of_property_read_bool(np, "snps,en-tx-lpi-clockgating"); > > > > - /* Set the maxmtu to a default of JUMBO_LEN in case the > > - * parameter is not present in the device tree. > > - */ > > - plat->maxmtu = JUMBO_LEN; > > - > > /* Set default value for multicast hash bins */ > > plat->multicast_filter_bins = HASH_TABLE_SIZE; > > > > -- > > 2.39.2 > > > >
| |