netdev - Re: [PATCH net-next 6/6] net: macb: Add MACB_CAPS_QBV capability flag for IEEE 802.1Qbv support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250723190500.GM1036606@horms.kernel.org>
Date: Wed, 23 Jul 2025 20:05:00 +0100
From: Simon Horman <horms@...nel.org>
To: Vineeth Karumanchi <vineeth.karumanchi@....com>
Cc: nicolas.ferre@...rochip.com, claudiu.beznea@...on.dev,
	andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com,
	kuba@...nel.org, pabeni@...hat.com, git@....com,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next 6/6] net: macb: Add MACB_CAPS_QBV capability
 flag for IEEE 802.1Qbv support

On Tue, Jul 22, 2025 at 09:11:11PM +0530, Vineeth Karumanchi wrote:
> The "exclude_qbv" bit in designcfg_debug1 register varies between MACB/GEM
> IP revisions, making direct register probing unreliable for
> feature detection. A capability-based approach provides consistent
> QBV support identification across the IP family
> 
> Platform support:
> - Enable MACB_CAPS_QBV for Xilinx Versal platform configuration
> - Foundation for QBV feature detection in TAPRIO implementation
> 
> Signed-off-by: Vineeth Karumanchi <vineeth.karumanchi@....com>

...

> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index cc33491930e3..98e56697661c 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -4601,6 +4601,10 @@ static int macb_init(struct platform_device *pdev)
>  		dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_RXCSUM;
>  	if (bp->caps & MACB_CAPS_SG_DISABLED)
>  		dev->hw_features &= ~NETIF_F_SG;
> +	/* Enable HW_TC if hardware supports QBV */
> +	if (bp->caps & MACB_CAPS_QBV)
> +		dev->hw_features |= NETIF_F_HW_TC;
> +
>  	dev->features = dev->hw_features;
>  
>  	/* Check RX Flow Filters support.
> @@ -5345,7 +5349,7 @@ static const struct macb_config sama7g5_emac_config = {
>  static const struct macb_config versal_config = {
>  	.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_JUMBO |
>  		MACB_CAPS_GEM_HAS_PTP | MACB_CAPS_BD_RD_PREFETCH | MACB_CAPS_NEED_TSUCLK |
> -		MACB_CAPS_QUEUE_DISABLE,
> +		MACB_CAPS_QUEUE_DISABLE, MACB_CAPS_QBV,

Hi Vineeth,

TL;DR: I think you mean

		MACB_CAPS_QUEUE_DISABLE | MACB_CAPS_QBV,
		                       ^^^

I assume that the intention here is to set the MACB_CAPS_QBV bit of .caps.
However, because there is a comma rather than a pipe between
it and MACB_CAPS_QUEUE_DISABLE the effect is to leave .caps as
it was before, and set .dma_burst_length to MACB_CAPS_QBV.
.dma_burst_length is then overwritten on the following line.

Flagged by W=1 builds with Clang 20.1.8 and 15.1.0.

Please build your patches with W=1 and try to avoid adding warnings
it flags.

Also, while we are here, it would be nice to fix up the line wrapping so
the adjacent code is 80 columns wide or less, as is still preferred in
Networking code.

	.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_JUMBO |
		MACB_CAPS_GEM_HAS_PTP | MACB_CAPS_BD_RD_PREFETCH |
		MACB_CAPS_NEED_TSUCLK | MACB_CAPS_QUEUE_DISABLE |
		MACB_CAPS_QBV,

>  	.dma_burst_length = 16,
>  	.clk_init = macb_clk_init,
>  	.init = init_reset_optional,
> -- 
> 2.34.1
> 
>