| Message ID | 20230313224237.28757-10-Sergey.Semin@baikalelectronics.ru (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: stmmac: Fixes bundle #1 | expand |
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? > > 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 > >
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 > > > >
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); 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;
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(-)