Hello Jakub,
Thank you for your comment.
-----Original Message-----
From: Jakub Kicinski <kuba@kernel.org>
Sent: Monday, May 25, 2026 10:41 PM
To: Michael Dege <redacted>
Cc: Jakub Kicinski <kuba@kernel.org>; Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>;
andrew+netdev@lunn.ch; davem@davemloft.net; edumazet@google.com; pabeni@redhat.com; niklas.soderlund
[off-list ref]; paul@pbarker.dev; richardcochran@gmail.com; netdev@vger.kernel.org;
linux-renesas-soc@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next v5 08/14] net: renesas: rswitch: add locking for agent clock control
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.
quoted
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
[ ... ]
quoted
@@ -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.
Good point fixed this.
quoted
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.
This is a bug. I also fixed it.
Best regards,
Michael