Re: [PATCH net-next v5 08/14] net: renesas: rswitch: add locking for agent clock control

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: renesas: rswitch: add locking for agent clock control

This commit introduces an agent_lock to serialize access to the shared RCEC
and RCDC agent clock registers. This aims to prevent potential race
conditions between ETHA and GWCA ports when enabling or disabling clocks.

> diff --git a/drivers/net/ethernet/renesas/rswitch.h b/drivers/net/ethernet/renesas/rswitch.h
> --- a/drivers/net/ethernet/renesas/rswitch.h
> +++ b/drivers/net/ethernet/renesas/rswitch.h
[ ... ]
> @@ -1207,6 +1208,8 @@ struct rswitch_private {
>  	struct list_head port_list;
>  
>  	spinlock_t lock;	/* lock interrupt registers' control */
> +	spinlock_t agent_lock;	/* rswitch agent clock control */
> +

[Severity: High]
Is this lock initialized anywhere before it is used? 

It looks like agent_lock is added to the structure, but spin_lock_init() is
missing from the driver probe path. Depending on kzalloc for
zero-initialization is not valid for spinlocks and can cause issues on
architectures or kernel configurations that require explicit initialization.

> diff --git a/drivers/net/ethernet/renesas/rswitch_main.c b/drivers/net/ethernet/renesas/rswitch_main.c
> --- a/drivers/net/ethernet/renesas/rswitch_main.c
> +++ b/drivers/net/ethernet/renesas/rswitch_main.c
> @@ -173,8 +173,12 @@ int rswitch_gwca_change_mode(struct rswitch_private *priv,
>  {
>  	int ret;
>  
> -	if (!rswitch_agent_clock_is_enabled(priv->addr, priv->gwca.index))
> +	if (!rswitch_agent_clock_is_enabled(priv->addr, priv->gwca.index)) {
> +		spin_lock(&priv->agent_lock);
>  		rswitch_agent_clock_ctrl(priv->addr, priv->gwca.index, 1);
> +		spin_lock(&priv->agent_lock);
> +	}

[Severity: High]
Does this code cause an immediate deadlock? 

Because spin_lock() is called twice consecutively without a spin_unlock(), it
appears the CPU will spin indefinitely waiting for the lock it already holds.