RE: [PATCH net-next v4 07/13] net: renesas: rswitch: make helper functions available to whole driver

[Michael Dege]

Hello Jakub,

Thank you for reviewing this.

> -----Original Message-----
> From: Jakub Kicinski <kuba@xxxxxxxxxx>
> Sent: Wednesday, May 13, 2026 3:36 AM
> To: Michael Dege <michael.dege@xxxxxxxxxxx>
> Cc: Jakub Kicinski <kuba@xxxxxxxxxx>; Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>;
> andrew+netdev@xxxxxxx; davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx; pabeni@xxxxxxxxxx; niklas.soderlund
> <niklas.soderlund@xxxxxxxxxxxx>; paul@xxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; linux-renesas-
> soc@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH net-next v4 07/13] net: renesas: rswitch: make helper functions available to whole
> driver
> 
> 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: make helper functions available to whole driver
> 
> This commit removes the static declaration from several helper functions and adds their prototypes to
> rswitch.h. This makes the functions available for upcoming L2 and VLAN handling features.
> 
> > 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
> [ ... ]
> >  /* Gateway CPU agent block (GWCA) */
> > -static int rswitch_gwca_change_mode(struct rswitch_private *priv,
> > -				    enum rswitch_gwca_mode mode)
> > +int rswitch_gwca_change_mode(struct rswitch_private *priv,
> > +			     enum rswitch_gwca_mode mode)
> >  {
> >  	int ret;
> [ ... ]
> >  /* Ethernet TSN Agent block (ETHA) and Ethernet MAC IP block (RMAC)
> > */ -static int rswitch_etha_change_mode(struct rswitch_etha *etha,
> > -				    enum rswitch_etha_mode mode)
> > +int rswitch_etha_change_mode(struct rswitch_etha *etha,
> > +			     enum rswitch_etha_mode mode)
> >  {
> >  	int ret;
> 
> Does exporting these functions expose unprotected read-modify-write operations on shared hardware
> clock registers to concurrent execution?
> 
> These functions modify the shared RCEC and RCDC clock registers via rswitch_agent_clock_ctrl(). Since
> all switch ports (ETHAs) and the GWCA share the exact same coma_addr space, could concurrent
> invocations for different ports cause a race condition?
> 
> For example, if multiple ports are configured concurrently via netlink, or if parallel PHY adjust_link
> worker threads call these functions:
> 
> Thread 1 (Port A)
> rswitch_etha_change_mode()
>     rswitch_agent_clock_ctrl() reads RCEC
> 
> Thread 2 (Port B)
> rswitch_etha_change_mode()
>     rswitch_agent_clock_ctrl() reads RCEC
>     rswitch_agent_clock_ctrl() writes RCEC
> 
> Thread 1 (Port A)
>     rswitch_agent_clock_ctrl() writes stale RCEC
> 
> If RCEC and RCDC are Set/Clear command registers, reading the current state and writing it back with a
> new bit could accidentally re-apply stale state, reverting concurrent clock disable commands on other
> ports.
> 
> If they are standard configuration registers, could this result in classic lost updates?
> 
> Can this lead to unpredictable port lockups or MAC failures by corrupting hardware clocks for
> unrelated ports?

I have added locking for these register accesses.

Best regards,

Michael