Re: [PATCH net-next v6 3/9] net: dsa: realtek: rtl8365mb: set speed for family D

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] This finding was checked and rejected as a false positive,
  because later patches in the series finish the work.

--

net: dsa: realtek: rtl8365mb: set speed for family D

This commit adds family D handling to rtl8365mb_ext_config_forcemode().
On family D, 2500M uses its own speed value (5), and the top speed bit
goes into a new FORCE_SPEED2 field. The forced-mode configuration is
written to a per-port register at 0x12c0 + port, and forced mode is
enabled by writing 0xFFFF to 0x12c8 + port, while family C keeps its
existing behaviour.

> diff --git a/drivers/net/dsa/realtek/rtl8365mb_main.c b/drivers/net/dsa/realtek/rtl8365mb_main.c
> index 840db5ad32efe..2dabc4da861f4 100644
> --- a/drivers/net/dsa/realtek/rtl8365mb_main.c
> +++ b/drivers/net/dsa/realtek/rtl8365mb_main.c

[ ... ]

> @@ -1646,23 +1669,32 @@ static int rtl8365mb_ext_config_forcemode(struct realtek_priv *priv, int port,
>  	u32 r_duplex;
>  	u32 r_speed;
>  	u32 r_link;
> +	bool is_d;
>  	int val;
>  	int ret;
>  
>  	if (!extint)
>  		return -ENODEV;
>  
> +	is_d = rtl8365mb_get_family(priv) == RTL8365MB_FAMILY_D;

[Severity: Low]
This isn't a bug, but can the family D branch run at this point in the
series? Every rtl8365mb_chip_infos[] entry is still RTL8365MB_FAMILY_C
here. So is_d is always false, and rtl8365mb_ext_config_forcemode() only
takes the family C path.

Later patches in the series make the path reachable:

  "net: dsa: realtek: rtl8365mb: add family D SDS13 PCS support" adds
  the family D SerDes setup to rtl8365mb_pcs_config(). It also makes
  rtl8365mb_pcs_link_up() return early on family D.

  "net: dsa: realtek: rtl8365mb: add support for RTL8367S-VB" adds the
  first FAMILY_D chip, with extint ports 6 and 7. That keeps
  0x12c0 + port below the 0x12c8 enable window.

The other preparatory patches in the series say that the family D path
stays unreachable until the final chip table patch. Could this commit
message say the same?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/5a4715dd-89e0-4868-9ffb-c8fb73b18d04%40yahoo.com