Re: [PATCH net-next 2/2] net: macb: configure ENST registers for all queues

[Nicolai Buchwitz <nb@tipi-net.de>]

Hi Vineeth

On 24.7.2026 06:32, Vineeth Karumanchi wrote:
> The initial "tc" command was not overwriting the reset value of ENST
> registers if only a subset of queues were configured, leading to an
> invalid setup. To fix this, configure all queues unconditionally.
> Unconfigured queues are zero-initialized via kcalloc(), ensuring a
> complete and consistent configuration.
> 
> The queue_id field in struct macb_queue_enst_config becomes redundant
> once the array is indexed by queue id, so drop it.
> 
> Key changes:
> - Drop queue_id from struct macb_queue_enst_config
> - Allocate enst_queue[] based on bp->num_queues instead of
>   conf->num_entries
> - Index enst_queue[] directly with the queue number
> - Program ENST registers for all queues
> 
> Fixes: 89934dbf169e ("net: macb: Add TAPRIO traffic scheduling 
> support")

In addition to what Théo already mentioned:

The series targets net-next, but if this is a real bug fix it should
probably be routed via net. The offending commit is in stable since 
v6.18.

> Signed-off-by: Vineeth Karumanchi <vineeth.karumanchi@amd.com>
> ---
>  drivers/net/ethernet/cadence/macb.h      |  2 --
>  drivers/net/ethernet/cadence/macb_main.c | 20 +++++++++-----------
>  2 files changed, 9 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb.h 
> b/drivers/net/ethernet/cadence/macb.h
> index a11052565436..4cc9aa9b2f9e 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -1490,7 +1490,6 @@ struct macb_platform_data {
>   * @start_time_mask:  Bitmask representing the start time for the 
> queue
>   * @on_time_bytes:    "on" time nsec expressed in bytes
>   * @off_time_bytes:   "off" time nsec expressed in bytes
> - * @queue_id:         Identifier for the queue
>   *
>   * This structure holds the configuration parameters for an ENST 
> queue,
>   * used to control time-based transmission scheduling in the MACB 
> driver.
> @@ -1499,7 +1498,6 @@ struct macb_queue_enst_config {
>  	u32 start_time_mask;
>  	u32 on_time_bytes;
>  	u32 off_time_bytes;
> -	u8 queue_id;
>  };
> 
>  #endif /* _MACB_H */
> diff --git a/drivers/net/ethernet/cadence/macb_main.c 
> b/drivers/net/ethernet/cadence/macb_main.c
> index d394f1f43b68..d58430fe9c41 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -4329,7 +4329,7 @@ static int macb_taprio_setup_replace(struct 
> net_device *ndev,
>  	struct macb_queue *queue;
>  	u32 queue_mask;
>  	u8 queue_id;
> -	size_t i;
> +	size_t i, q;
>  	int err;
> 
>  	if (conf->num_entries > bp->num_queues) {
> @@ -4357,7 +4357,7 @@ static int macb_taprio_setup_replace(struct 
> net_device *ndev,
>  		return -EINVAL;
>  	}
> 
> -	enst_queue = kcalloc(conf->num_entries, sizeof(*enst_queue), 
> GFP_KERNEL);
> +	enst_queue = kcalloc(bp->num_queues, sizeof(*enst_queue), 
> GFP_KERNEL);
>  	if (unlikely(!enst_queue))
>  		return -ENOMEM;
> 
> @@ -4416,13 +4416,12 @@ static int macb_taprio_setup_replace(struct 
> net_device *ndev,
>  			goto cleanup;
>  		}
> 
> -		enst_queue[i].queue_id = queue_id;
> -		enst_queue[i].start_time_mask =
> +		enst_queue[queue_id].start_time_mask =
>  			(start_time_sec << GEM_START_TIME_SEC_OFFSET) |
>  			start_time_nsec;
> -		enst_queue[i].on_time_bytes =
> +		enst_queue[queue_id].on_time_bytes =
>  			enst_ns_to_hw_units(entry->interval, speed);
> -		enst_queue[i].off_time_bytes =
> +		enst_queue[queue_id].off_time_bytes =
>  			enst_ns_to_hw_units(conf->cycle_time - entry->interval, speed);
> 
>  		configured_queues |= entry->gate_mask;
> @@ -4448,15 +4447,14 @@ static int macb_taprio_setup_replace(struct 
> net_device *ndev,
>  		gem_writel(bp, ENST_CONTROL,
>  			   queue_mask << GEM_ENST_DISABLE_QUEUE_OFFSET);
> 
> -		for (i = 0; i < conf->num_entries; i++) {
> -			queue = &bp->queues[enst_queue[i].queue_id];
> +		for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
>  			/* Configure queue timing registers */
>  			queue_writel(queue, ENST_START_TIME,
> -				     enst_queue[i].start_time_mask);
> +				     enst_queue[q].start_time_mask);
>  			queue_writel(queue, ENST_ON_TIME,
> -				     enst_queue[i].on_time_bytes);
> +				     enst_queue[q].on_time_bytes);
>  			queue_writel(queue, ENST_OFF_TIME,
> -				     enst_queue[i].off_time_bytes);
> +				     enst_queue[q].off_time_bytes);
>  		}
> 
>  		/* Enable ENST for all configured queues in one write */

Regards,
Nicolai