Re: [PATCH net] net: airoha: Wait for NPU PPE configuration to complete in airoha_ppe_offload_setup()

[Simon Horman <horms@kernel.org>]

From: 'Simon Horman' <horms@kernel.org>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
    net: airoha: Wait for NPU PPE configuration to complete in airoha_ppe_offload_setup()

This commit adds a poll loop in airoha_ppe_offload_setup() to wait for the NPU
PPE configuration to complete before running airoha_ppe_hw_init().

> diff --git a/drivers/net/ethernet/airoha/airoha_ppe.c b/drivers/net/ethernet/airoha/airoha_ppe.c
> index c2c32b6833df9..52199f6b39f9b 100644
> --- a/drivers/net/ethernet/airoha/airoha_ppe.c
> +++ b/drivers/net/ethernet/airoha/airoha_ppe.c
[ ... ]
> @@ -1340,6 +1341,13 @@ static int airoha_ppe_offload_setup(struct airoha_eth *eth)
>  	if (err)
>  		goto error_npu_put;
>  
> +	/* Wait for NPU PPE configuration to complete */
> +	err = read_poll_timeout(airoha_fe_rr, val, val, USEC_PER_MSEC,
> +				100 * USEC_PER_MSEC, false, eth,
> +				REG_PPE_PPE_FLOW_CFG(0));

If the SoC has multiple PPE instances, is it sufficient to only wait for
REG_PPE_PPE_FLOW_CFG(0)?

Later in airoha_ppe_hw_init(), the code iterates up to eth->soc->num_ppe,
initializing all available instances:

drivers/net/ethernet/airoha/airoha_ppe.c:airoha_ppe_hw_init() {
    ...
	for (i = 0; i < eth->soc->num_ppe; i++) {
		...
		airoha_fe_wr(eth, REG_PPE_TB_BASE(i),
			     ppe->foe_dma + sram_tb_size);
    ...
}

If we only poll PPE 0, could the driver race with the NPU firmware's
initialization of subsequent PPEs?

Additionally, does this poll condition properly ensure the hardware is ready?
Since the break condition is simply `val`, it exits immediately if the register
contains any non-zero value.

If the register retains a non-zero value from a previous session (like across
driver reloads), or if an unclocked domain returns 0xFFFFFFFF, this poll might
succeed immediately and bypass the wait.

Could we clear the register before polling, or wait for a specific bit instead?

> +	if (err)
> +		goto error_npu_put;
> +
>  	ppe_num_stats_entries = airoha_ppe_get_total_num_stats_entries(ppe);