diff mbox series

[v1] TSO and TBS cannot co-exist. TBS requires special descriptor to be allocated at bootup. Initialising Tx queues at probe to support TSO and TBS can help in allocating those resources at bootup.

Message ID 20240206212734.1209920-1-quic_abchauha@quicinc.com
State New
Headers
Series [v1] TSO and TBS cannot co-exist. TBS requires special descriptor to be allocated at bootup. Initialising Tx queues at probe to support TSO and TBS can help in allocating those resources at bootup. |

Commit Message

Abhishek Chauhan Feb. 6, 2024, 9:27 p.m. UTC 
  TX queues with TBS can support etf qdisc hw offload.

Signed-off-by: Abhishek Chauhan <quic_abchauha@quicinc.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Andrew Halaney Feb. 6, 2024, 11:29 p.m. UTC | #1 
Hey Abhishek,

Thanks for the patch!

This is a new feature for netdev, so the Subject should have "net-next"
in it: https://docs.kernel.org/process/maintainer-netdev.html#tl-dr

Another thing, the kernel is very particular about the commit messages.
I thought checkpatch.pl would complain about the subject line, but
surprisingly it didn't.

Usually going with a "when in rome" approach is good (i.e. take a look
at the git log). So here something like:

    "net: stmmac: dwmac-qcom-ethqos: Enable TBS on all queues but 0"

would be better. Its a short one line subject, that has the appropriate
prefix, etc. You could then embed the information about TSO and TBS
being exclusive, and maybe explain your reasoning on why this allocation
of queues (TSO on 0, TBS on the rest) was done.. etc in the body. Maybe even
pointing to the similar NXP related patch Esben posted recently would be
smart since your motivation is the same:

    commit 3b12ec8f618ebaccfe43ea4621a6f5fb586edef8
    Author: Esben Haabendal <esben@geanix.com>
    Date:   Fri Jan 26 10:10:42 2024 +0100

        net: stmmac: dwmac-imx: set TSO/TBS TX queues default settings

        TSO and TBS cannot coexist. For now we set i.MX Ethernet QOS controller to
        use the first TX queue with TSO and the rest for TBS.

        TX queues with TBS can support etf qdisc hw offload.

        Signed-off-by: Esben Haabendal <esben@geanix.com>
        Reviewed-by: Kurt Kanzenbach <kurt@linutronix.de>
        Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
        Signed-off-by: Paolo Abeni <pabeni@redhat.com>

i.e. something like (please make it your own):

    In a similar vein to 3b12ec8f618e
    ("net: stmmac: dwmac-imx: set TSO/TBS TX queues default settings"),
    let's leave TSO enabled on queue 0 and enable TBS on all other queues.
    This allows using the etf qdisc with hw offload on the TBS enabled
    queues.

On Tue, Feb 06, 2024 at 01:27:34PM -0800, Abhishek Chauhan wrote:
> TX queues with TBS can support etf qdisc hw offload.
> 
> Signed-off-by: Abhishek Chauhan <quic_abchauha@quicinc.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> index 31631e3f89d0..d2f9b8f6c027 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> @@ -728,7 +728,7 @@ static int qcom_ethqos_probe(struct platform_device *pdev)
>  	struct stmmac_resources stmmac_res;
>  	struct device *dev = &pdev->dev;
>  	struct qcom_ethqos *ethqos;
> -	int ret;
> +	int ret, i;
>  
>  	ret = stmmac_get_platform_resources(pdev, &stmmac_res);
>  	if (ret)
> @@ -822,6 +822,10 @@ static int qcom_ethqos_probe(struct platform_device *pdev)
>  		plat_dat->serdes_powerdown  = qcom_ethqos_serdes_powerdown;
>  	}
>  
> +	/*Enable TSO on queue0 and enable TBS on rest of the queues*/

nitpicky: Please put spaces between the comments

> +	for (i = 1; i < plat_dat->tx_queues_to_use; i++)
> +		plat_dat->tx_queues_cfg[i].tbs_en = 1;
> +
>  	return devm_stmmac_pltfr_probe(pdev, plat_dat, &stmmac_res);
>  }
>  
> -- 
> 2.25.1
> 
>
diff mbox series

Patch

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
index 31631e3f89d0..d2f9b8f6c027 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
@@ -728,7 +728,7 @@  static int qcom_ethqos_probe(struct platform_device *pdev)
 	struct stmmac_resources stmmac_res;
 	struct device *dev = &pdev->dev;
 	struct qcom_ethqos *ethqos;
-	int ret;
+	int ret, i;
 
 	ret = stmmac_get_platform_resources(pdev, &stmmac_res);
 	if (ret)
@@ -822,6 +822,10 @@  static int qcom_ethqos_probe(struct platform_device *pdev)
 		plat_dat->serdes_powerdown  = qcom_ethqos_serdes_powerdown;
 	}
 
+	/*Enable TSO on queue0 and enable TBS on rest of the queues*/
+	for (i = 1; i < plat_dat->tx_queues_to_use; i++)
+		plat_dat->tx_queues_cfg[i].tbs_en = 1;
+
 	return devm_stmmac_pltfr_probe(pdev, plat_dat, &stmmac_res);
 }