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

[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: 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?