| Message ID | 20240202-stmmac-axi-config-v2-3-64eab2bab17b@nvidia.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: stmmac: Allow driver-specific AXI configuration | expand |
On Fri, Feb 02, 2024 at 12:53:35PM +0100, Thierry Reding wrote: > From: Thierry Reding <treding@nvidia.com> > > Allow the device to use bursts and increase the maximum number of > outstanding requests to improve performance. Measurements show an > increase in throughput of around 5x on a 1 Gbps link. > > Signed-off-by: Thierry Reding <treding@nvidia.com> > --- > drivers/net/ethernet/stmicro/stmmac/dwmac-tegra.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-tegra.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-tegra.c > index bab57d1675df..b6bfa48f279d 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-tegra.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-tegra.c > @@ -199,6 +199,12 @@ static void mgbe_uphy_lane_bringup_serdes_down(struct net_device *ndev, void *mg > writel(value, mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL); > } > > +static const struct stmmac_axi tegra234_mgbe_axi = { > + .axi_wr_osr_lmt = 63, > + .axi_rd_osr_lmt = 63, > + .axi_blen = { 256, }, > +}; > + > static int tegra_mgbe_probe(struct platform_device *pdev) > { > struct plat_stmmacenet_data *plat; > @@ -284,6 +290,9 @@ static int tegra_mgbe_probe(struct platform_device *pdev) > if (err < 0) > goto disable_clks; > > + /* setup default AXI configuration */ > + res.axi = &tegra234_mgbe_axi; > + > plat = devm_stmmac_probe_config_dt(pdev, &res); > if (IS_ERR(plat)) { > err = PTR_ERR(plat); The entire series can be converted to just a few lines of change: plat = devm_stmmac_probe_config_dt(pdev, res.mac); if (IS_ERR(plat)) { err = PTR_ERR(plat); goto disable_clks; } + + if (IS_ERR_OR_NULL(plat->axi)) { + plat->axi = devm_kzalloc(&pdev->dev, sizeof(*axi), GFP_KERNEL); + if (!plat->axi) { + ret = -ENOMEM; + goto disable_clks; + } + } /* else memset plat->axi with zeros if you wish */ + + plat->axi->axi_wr_osr_lmt = 63; + plat->axi->axi_rd_osr_lmt = 63; + plat->axi->axi_blen[0] = 256; plat->has_xgmac = 1; plat->flags |= STMMAC_FLAG_TSO_EN; plat->pmt = 1; Please don't overcomplicate the already overcomplicated driver with a functionality which can be reached by the default one. In this case the easiest way is to let the generic code work and then override/replace/fix/etc the retrieved values. Thus there won't be need in adding the redundant functionality and keep the generic DT-platform code a bit simpler to read. -Serge(y) > > -- > 2.43.0 > >
On Mon Feb 5, 2024 at 1:44 AM CET, Serge Semin wrote: > On Fri, Feb 02, 2024 at 12:53:35PM +0100, Thierry Reding wrote: > > From: Thierry Reding <treding@nvidia.com> > > > > Allow the device to use bursts and increase the maximum number of > > outstanding requests to improve performance. Measurements show an > > increase in throughput of around 5x on a 1 Gbps link. > > > > Signed-off-by: Thierry Reding <treding@nvidia.com> > > --- > > drivers/net/ethernet/stmicro/stmmac/dwmac-tegra.c | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-tegra.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-tegra.c > > index bab57d1675df..b6bfa48f279d 100644 > > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-tegra.c > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-tegra.c > > @@ -199,6 +199,12 @@ static void mgbe_uphy_lane_bringup_serdes_down(struct net_device *ndev, void *mg > > writel(value, mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL); > > } > > > > +static const struct stmmac_axi tegra234_mgbe_axi = { > > + .axi_wr_osr_lmt = 63, > > + .axi_rd_osr_lmt = 63, > > + .axi_blen = { 256, }, > > +}; > > + > > static int tegra_mgbe_probe(struct platform_device *pdev) > > { > > struct plat_stmmacenet_data *plat; > > @@ -284,6 +290,9 @@ static int tegra_mgbe_probe(struct platform_device *pdev) > > if (err < 0) > > goto disable_clks; > > > > + /* setup default AXI configuration */ > > + res.axi = &tegra234_mgbe_axi; > > + > > plat = devm_stmmac_probe_config_dt(pdev, &res); > > if (IS_ERR(plat)) { > > err = PTR_ERR(plat); > > The entire series can be converted to just a few lines of change: Sorry for the delay, I missed this reply. > plat = devm_stmmac_probe_config_dt(pdev, res.mac); > if (IS_ERR(plat)) { > err = PTR_ERR(plat); > goto disable_clks; > } > + > + if (IS_ERR_OR_NULL(plat->axi)) { > + plat->axi = devm_kzalloc(&pdev->dev, sizeof(*axi), GFP_KERNEL); > + if (!plat->axi) { > + ret = -ENOMEM; > + goto disable_clks; > + } > + } /* else memset plat->axi with zeros if you wish */ > + > + plat->axi->axi_wr_osr_lmt = 63; > + plat->axi->axi_rd_osr_lmt = 63; > + plat->axi->axi_blen[0] = 256; > > plat->has_xgmac = 1; > plat->flags |= STMMAC_FLAG_TSO_EN; > plat->pmt = 1; > > Please don't overcomplicate the already overcomplicated driver with a > functionality which can be reached by the default one. In this case > the easiest way is to let the generic code work and then > override/replace/fix/etc the retrieved values. Thus there won't be > need in adding the redundant functionality and keep the generic > DT-platform code a bit simpler to read. I'm not sure I understand how this is overcomplicating things. The code is pretty much unchanged, except that the AXI configuration can now have driver-specified defaults before the DT is parsed. Perhaps I need to add comments to make that a bit clearer? While your version is certainly simpler it has the drawback that it no longer allows the platform defaults to be overridden in device tree. I would prefer if the defaults can be derived from the compatible string but if need be for those defaults to still be overridable from device tree. Thierry
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-tegra.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-tegra.c index bab57d1675df..b6bfa48f279d 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-tegra.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-tegra.c @@ -199,6 +199,12 @@ static void mgbe_uphy_lane_bringup_serdes_down(struct net_device *ndev, void *mg writel(value, mgbe->xpcs + XPCS_WRAP_UPHY_RX_CONTROL); } +static const struct stmmac_axi tegra234_mgbe_axi = { + .axi_wr_osr_lmt = 63, + .axi_rd_osr_lmt = 63, + .axi_blen = { 256, }, +}; + static int tegra_mgbe_probe(struct platform_device *pdev) { struct plat_stmmacenet_data *plat; @@ -284,6 +290,9 @@ static int tegra_mgbe_probe(struct platform_device *pdev) if (err < 0) goto disable_clks; + /* setup default AXI configuration */ + res.axi = &tegra234_mgbe_axi; + plat = devm_stmmac_probe_config_dt(pdev, &res); if (IS_ERR(plat)) { err = PTR_ERR(plat);