Re: [PATCH net-next v8 4/9] net: dsa: lan9645x: add basic dsa driver for LAN9645X

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Traffic from standalone ports is completely dropped in the dataplane, preventing any network communication.
- [Medium] The tail-drop watermark (SYS_ATOP) for the internal CPU port is left uninitialized, preventing CPU frame injection from using shared buffer memory.
- [Low] Redundant identical register modification in the CPU port initialization sequence.
--

Note: The format of this review is altered due to automated recitation policy
restrictions. No original patch code is quoted directly, and the review is
provided as a free-form summary of findings.

commit 6d3e640051eacab0f47d2250812bfddc2bc0f9e8
Author: Jens Emil Schulz Østergaard <jensemil.schulzostergaard@microchip.com>
Subject: net: dsa: lan9645x: add basic dsa driver for LAN9645X

This commit adds the base data switch architecture driver for the lan9645x
switch. It sets up the port modules, internal phy connections, queue systems,
and phylink integration for mac side configuration.

[Severity: High]
Will traffic from standalone ports be completely dropped in the dataplane?

In drivers/net/dsa/microchip/lan9645x/lan9645x_main.c within lan9645x_setup(),
the source port masks are initialized to zero. For standalone ports, this
mask is never updated to include any destinations when
lan9645x_update_fwd_mask() is called. Since unknown traffic is flooded to the
unicast destination mask which excludes the cpu port module, and the source
mask prevents forwarding to any port, does this mean all normal traffic
received on standalone ports will be dropped instead of reaching the cpu?

[Severity: Medium]
Does this iteration miss the internal cpu port module?

In drivers/net/dsa/microchip/lan9645x/lan9645x_port.c during
lan9645x_port_set_tail_drop_wm(), the code uses
dsa_switch_for_each_available_port() to configure the tail-drop watermarks.
This iterator limits initialization to the physical front ports, leaving the
internal cpu port module uninitialized at zero. If the cpu injects a frame
that exceeds its reserved buffer allowance, will it be immediately dropped by
the ingress tail dropper because its watermark is zero?

[Severity: Low]
This isn't a bug, but is there a redundant register modification here?

In drivers/net/dsa/microchip/lan9645x/lan9645x_port.c during
lan9645x_port_cpu_init(), the code modifies the injection gap size to zero.
Two statements later, after setting the injection group configuration, the
exact same gap size modification is executed again. Can this second redundant
bus transaction be removed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702-dsa_lan9645x_switch_driver_base-v8-0-90228d8bba58@microchip.com?part=4