Re: [PATCH net-next v4 1/3] net: ti: icssm-prueth: Add HSR and PRP HW offload mode support for AM57xx, AM437x and AM335x

[Simon Horman]

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/#/patchset/20260921134343.200426-4-parvathi@xxxxxxxxxxx?part=1
---
net: ti: icssm-prueth: Add HSR and PRP HW offload mode support for AM57xx, AM437x and AM335x

This commit extends the ICSSM PRUETH driver to support HSR and PRP redundancy
protocols on compatible TI SoCs. It enables dynamically switching PRU ethernet
ports from dual-EMAC mode into HSR/PRP slave modes via firmware reconfiguration.

> diff --git a/drivers/net/ethernet/ti/icssm/icssm_prueth.c b/drivers/net/ethernet/ti/icssm/icssm_prueth.c
> --- a/drivers/net/ethernet/ti/icssm/icssm_prueth.c
> +++ b/drivers/net/ethernet/ti/icssm/icssm_prueth.c
[ ... ]
> +static int icssm_prueth_hsr_port_link(struct net_device *ndev,
> +				      struct net_device *hsr_ndev)
> +{
[ ... ]
> +free_hsr:
> +	prueth->eth_type = prev_mode;
> +
> +	spin_lock_irqsave(&prueth->addr_lock, flags);
> +
> +	prueth->hsr_dev = NULL;
> +	prueth->hsr_members &= ~all_slaves;

[Severity: High]
Can this rollback improperly destroy the state for the already-linked first
port alongside the failing second port?

If the second port fails to transition to HSR/PRP mode, the free_hsr error
path clears the hsr_members mask for all ports by applying ~all_slaves.

Because the first port was successfully enrolled into the HSR bridge in a
previous invocation, it remains an active slave in the networking stack's
view. By wiping its membership state here, the driver drastically
desynchronizes its internal state from the kernel.

Would it be safer to only clear the bit for the specific port that is
failing to link?

> +
> +	spin_unlock_irqrestore(&prueth->addr_lock, flags);
> +	return ret;
> +}