Re: [PATCH net-next v2 07/11] net: ethernet: qualcomm: ppe: Add IPQ5424 QM configuration

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit message and the new code comment above
  ipq5424_ppe_qm_group_config in…

Pre-existing issues:
- [Low] At this commit, ppe_port_resource_get() checks `port >
  ppe_dev->num_ports` and then indexes the single IPQ9574-shaped…

--

net: ethernet: qualcomm: ppe: Add IPQ5424 QM configuration

This commit adds IPQ5424-specific QM settings, which ppe_config_qm() selects
based on ppe_dev->type. On IPQ5424 the multicast queue range ends at 283
instead of 299. The group0 shared buffer pool is set to 2010 buffers
instead of 2000.

> diff --git a/drivers/net/ethernet/qualcomm/ppe/ppe_config.c b/drivers/net/ethernet/qualcomm/ppe/ppe_config.c
> index ef46e9b133fe..d2bd78853b98 100644
> --- a/drivers/net/ethernet/qualcomm/ppe/ppe_config.c
> +++ b/drivers/net/ethernet/qualcomm/ppe/ppe_config.c
> @@ -314,6 +314,37 @@ static const struct ppe_qm_queue_config ipq9574_ppe_qm_queue_config[] = {
>  	},
>  };
>  
> +/* Multicast queues are dedicated per PPE port. IPQ5424 has only 3 physical
> + * PPE ports, versus IPQ9574's 6 PPE ports, so multicast queues 284 to 299
> + * are not available on IPQ5424, and the buffer pool assigned to the general
> + * 'group0' is larger here, 2010 versus IPQ9574's 2000.
> + */

[Severity: Low]
Is this explanation complete? The commit message uses the same wording.

In ppe_scheduler_res[], ports 4 to 7 each get four multicast queues:

    port 4: mcastq 284-287
    port 5: mcastq 288-291
    port 6: mcastq 292-295
    port 7: mcastq 296-299

The three missing physical ports only cover queues 284-295. Queues 296-299
belong to port 7, the EIP FIFO port, which IPQ5424 also does not have.

The queue range in ipq5424_ppe_qm_queue_config[] looks correct. Could the
comment and the commit message also mention the missing EIP port?

> +static const int ipq5424_ppe_qm_group_config = 2010;
> +
> +/* Default QM settings for unicast and multicast queues for IPQ5424. */
> +static const struct ppe_qm_queue_config ipq5424_ppe_qm_queue_config[] = {

[ ... ]

> +	{
> +		/* QM settings for multicast queues 256 to 283. */
> +		.queue_start	= 256,
> +		.queue_end	= 283,
> +		.prealloc_buf	= 0,
> +		.ceil		= 250,
> +		.weight		= 0,
> +		.resume_offset	= 36,
> +		.dynamic	= false,
> +	},
> +};

[Severity: Low]
This is a pre-existing issue, not one introduced by this patch, but it
interacts with the multicast range set here. ppe_port_resource_get() still
indexes the single IPQ9574-shaped ppe_scheduler_res[] table:

drivers/net/ethernet/qualcomm/ppe/ppe_config.c:ppe_port_resource_get() {
    ...
	/* The reserved resource with the maximum port ID of PPE is
	 * also allowed to be acquired.
	 */
	if (port > ppe_dev->num_ports)
		return -EINVAL;

	res = ppe_scheduler_res[port];
    ...
}

On IPQ5424, num_ports is 4. A request for the reserved resource
(port == 4) would return the IPQ9574 port 4 entry, which has unicast
queues 192-207 and multicast queues 284-287.

This patch leaves multicast queues 284-287 unconfigured in QM, so
enqueue and dequeue stay disabled for them. The correct reserved entry for
IPQ5424 would be unicast queues 64-143 with no multicast queues.

Can ppe_port_resource_get() hand out multicast queue IDs that are disabled
in QM on IPQ5424?

No in-tree caller passes port == num_ports at this commit, so the problem
is latent. ppe_queue_dest_init() loops while port_id < num_ports, and the
other caller passes port 0.

A later patch in this series, "net: ethernet: qualcomm: ppe: Add IPQ5424
scheduler resource configuration", fixes this. It adds
ipq5424_ppe_scheduler_res[], where index 4 is the reserved entry. It also
changes ppe_port_resource_get() to pick the table by ppe_dev->type and to
bound the index by the table size.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261011-qcom_ipq5424_ppe-v2-0-8c7b057e2c43%40oss.qualcomm.com