* [PATCH net-next v13 0/9] net: dsa: add DSA support for the LAN9645x switch chip family
@ 2026-09-29 7:48 Jens Emil Schulz Østergaard
2026-09-29 7:48 ` [PATCH net-next v13 1/9] net: dsa: add tag driver for LAN9645X Jens Emil Schulz Østergaard
` (8 more replies)
0 siblings, 9 replies; 29+ messages in thread
From: Jens Emil Schulz Østergaard @ 2026-09-29 7:48 UTC (permalink / raw)
To: UNGLinuxDriver, Andrew Lunn, Vladimir Oltean, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Woojung Huh,
Russell King, Steen Hegelund, Daniel Machon, Geert Uytterhoeven,
Magnus Damm
Cc: linux-kernel, netdev, devicetree, linux-renesas-soc,
Jens Emil Schulz Østergaard
This series provides the Microchip LAN9645X Switch driver.
The LAN9645x is a family of chips with ethernet switch functionality and
multiple peripheral functions. The switch delivers up to 9 ethernet
ports and 12 Gbps switching bandwidth.
The switch chip has 5 integrated copper PHYs, support for 2x RGMII
interfaces, 2x SGMII and one QSGMII interface.
The switch chip is from the same design architecture family as ocelot
and lan966x, and the driver reflects this similarity. However, LAN9645x
does not have an internal CPU in any package, and must be driven
externally. For register IO it supports interfaces such as SPI, I2C and
MDIO.
The chip supports a variety of network features such as
* Mactable for MDB/FDB functionality
* Bridge forwarding offload
* VLAN-aware bridging
* IGMP/MLD snooping
* Link aggregation
* PTP timestamping
* FRER (802.1CB)
* Media Redundancy Protocol
* Parallel Redundancy and High-Availability Seamless Redundancy
(HSR/PRP) in DANH/DANP mode
* Per stream filtering and policing
* Shapers such as Credit Based Shaping and Time Aware Shaing
* Frame preemption
* A TCAM (VCAP) for line-rate frame processing
The LAN9645x family consists of the following SKUs:
LAN96455F
LAN96457F
LAN96459F
LAN96455S
LAN96457S
LAN96459S
The difference between the SKUs is the number of supported ports (5, 7
or 9) and features supported. The F subfamily supports HSR/PRP and TSN,
while the S subfamily does not.
The intended way to bind this driver is using a parent MFD driver,
responsible for the register IO protocol, and distributing regmaps to
child devices. The goal is to use the same approach as the MFD driver in
drivers/mfd/ocelot-spi.c, which also resets the chip before it
instantiates its children.
This driver expects to request named regmaps from a parent device. This
approach is similar to the DSA driver
drivers/net/dsa/ocelot/ocelot_ext.c
which supports being driven by an external CPU via SPI with parent
device drivers/mfd/ocelot-spi.c.
The MFD driver will come in a later series, because there are
requirements on the number of child devices before a driver qualifies as
a MFD device.
Development is done using the LAN966x as a host CPU, running the lan966x
swichdev driver, using the EVB-LAN9668 EDS2 board.
The datasheet is available here:
https://ww1.microchip.com/downloads/aemDocuments/documents/UNG/ProductDocuments/DataSheets/LAN9645xF-Data-Sheet-DS00006065.pdf
This series will deliver the following features:
* Standalone ports
* Bridge forwarding and FDB offloading
* Multicast forwarding and MDB offloading
* VLAN-aware bridge
* Stats integration
More support will be added at a later stage. Here is a tentative plan of
future patches for this DSA driver:
* Add LAG support.
* Add TC matchall mirror support.
* Add TC matchall police support.
* Add DCB/qos support.
* Add simple TC support: mqprio, cbs, tbf, ebf.
* Add TC flower filter support.
* Add HSR/PRP offloading support.
* Add PTP support.
* Add TC taprio support.
For completeness I include tentative plan of planned patches for
LAN9645x peripherals:
* Extend pinctrl-ocelot for LAN9645x:
https://lore.kernel.org/linux-gpio/20260119-pinctrl_ocelot_extend_support_for_lan9645x-v1-0-1228155ed0ee@microchip.com/
* Add driver for internal PHY:
https://lore.kernel.org/netdev/20260123-phy_micrel_add_support_for_lan9645x_internal_phy-v1-1-8484b1a5a7fd@microchip.com/
* MFD driver for managing register IO protocol and child device
initialization.
* Extend pinctrl-microchip-sgpio for LAN9645x support.
* Extend i2c_designware for LAN9645x support.
* Add driver for outbound interrupt controller.
* Add serdes driver for lan9645x.
Signed-off-by: Jens Emil Schulz Østergaard <jensemil.schulzostergaard@microchip.com>
---
Changes in v13:
- Individual patches mention specific v13 changes
- bindings: Drop the commit message paragraph citing
Documentation/networking/phy.rst.
- basic: only redirect the CPU extraction queues to the NPI port while
its link is up, and flush the CPU port module on NPI link up.
- Rebased on net-next, bumping DSA_TAG_PROTO_LAN9645X_VALUE to 37
- Link to v12: https://lore.kernel.org/r/20260908-dsa_lan9645x_switch_driver_base-v12-0-2d6aa59350cd@microchip.com
Changes in v12:
- reorder the series so bridge support lands after vlan, mac table and
mdb support. port_fast_age() now exists before the STP state handling
that needs it, and host address filtering is in place before bridging.
- include the CPU port module in the flood masks from the start, and
remove it again in the bridge patch where port_set_host_flood() takes
over on demand host flooding. Previously the host could not receive
unicast until the mac table patch.
- vlan: serialize the VLAN table with fwd_domain_lock, and describe the
untagged VLAN, TPID and C-component conformance limitations.
- tag driver: move an out-of-band VLAN tag into the payload before
reading it, instead of relying on the conduit never advertising a VLAN
TX offload.
- tag driver: restore the classified VLAN as a C-tag rather than following
the IFH tag type, so a terminated frame lands on the VID the hardware
forwarded it on.
- basic: drop the tail drop watermark setup, which programmed a
condition that could not trigger.
- mdb: handle PGID exhaustion on both the add and delete paths, rather
than leaving a departed port in the group.
- mdb: classify IPv4 multicast on the 01:00:5E prefix alone. Hardware does
not apply the 23-bit rule, so some groups were programmed as entries it
never matches.
- stats: give the hardware counters and the software shadow a common
baseline, fix some rtnl_link_stats64 fields, and stop reporting the
three eth-mac fields the hardware cannot measure.
- stats: stop the counter poller in .shutdown, which kept reading
registers after the parent had been shut down.
- Link to v11: https://lore.kernel.org/r/20260805-dsa_lan9645x_switch_driver_base-v11-0-007ebc983a0a@microchip.com
Changes in v11:
- Update comment about vlan tag hwaccel use in tag driver.
- Bump DSA_TAG_PROTO value in include/net/dsa.h.
- stats: add comment about calling contexts of sw_lock in
lan9645x_stats_get_stats64
- Link to v10: https://lore.kernel.org/r/20260713-dsa_lan9645x_switch_driver_base-v10-0-a4886a08fb15@microchip.com
Changes in v10:
- Individual patches mention specific v10 changes
- Update tag driver after skb ownership model change in DSA taggers.
- Link to v9: https://lore.kernel.org/r/20260708-dsa_lan9645x_switch_driver_base-v9-0-0d1512a326d7@microchip.com
Changes in v9:
- Individual patches mention specific v9 changes
- Drop RGMII muxing to port 4.
- Link to v8: https://lore.kernel.org/r/20260702-dsa_lan9645x_switch_driver_base-v8-0-90228d8bba58@microchip.com
Changes in v8:
- Individual patches mention specific v8 changes
- Do not use the name CPU_PORT for the chips internal CPU port module,
as it collides with the DSA CPU port (the NPI front port). Drop the
CPU_PORT macro and reference the module as lan9645x->num_phys_ports,
mirroring ocelot/felix.
- Reword the port map and related comments to distinguish the chip CPU
port modules (indices 9-10) from the DSA CPU port.
- Add a port_mux_lock mutex to serialize port mux arbitration in
the phylink mac_prepare path.
- Link to v7: https://lore.kernel.org/r/20260603-dsa_lan9645x_switch_driver_base-v7-0-b2f90e676707@microchip.com
Changes in v7:
- Individual patches mention specific v7 changes
- Do not mark the BPDU range 01:80:C2:00:00:0X as offloaded in the tag driver.
- Refactor cpu queue based frame classification to use categories default, trap
and copy, which mirrors usage instead of being based on frame types.
- Add registers ANA:COMMON:CPUQ_8021_CFG for bpdu cpu queue configuration.
- Use cpu queue LAN9645X_CPUQ_TRAP for bpdu frames.
- Refactor IGMP/MLD/IPMC_CTRL to use new LAN9645X_CPUQ_DEF,
LAN9645X_CPUQ_TRAP and LAN9645X_CPUQ_COPY queues.
- Add __aligned(2) to mac variable in lan9645x_mdb_update_dest.
- Link to v6: https://lore.kernel.org/r/20260527-dsa_lan9645x_switch_driver_base-v6-0-4d409ae64f3c@microchip.com
Changes in v6:
- Individual patches mention specific v6 changes
- Rebased on net-next, bumping DSA_TAG_PROTO_LAN9645X_VALUE to 34
- Link to v5: https://lore.kernel.org/r/20260518-dsa_lan9645x_switch_driver_base-v5-0-968fbf34ffa3@microchip.com
Changes in v5:
- Individual patches mention specific v5 changes
- Undo offset fix in postpull_rcsum. The original logic was correct for
CHECKSUM_COMPLETE host NICs
- Use __always_inline in lan9645x_ifh_{get,set}
- remove double space after = in set_merge_mask
- remove unused fields dd_dis and tsn_dis, and add SKU supported port validation
during setup
- phylink: remove MAC_2500FD
- phylink: add comment about empty supported_interfaces for port 5-6.
- phylink: fix 2:1 rgmii port muxing for port module 4 and 7 to be fully
dynamic and validate requested mux settings.
- phylink: add comment about 2:1 rgmii port muxing for port module 4
and 7.
- rx/tx-internal-delay-ps checked against supported 2ns value
- init lan9645x->npi = -1 at probe and check port < 0 in npi_deinit
- use ds->ageing_time_max
- use packed p->host_flood_req for atomic r/w
- fix typo in set_ageing_time comment
- include lan9645x->bridge deref under lock in brige_join
- switch -EBUSY to -EINVAL for vlan add/del in the reserved range.
- remove reserved HSR vlan
- lan9645x_mac_init returns error on table init timeout
- add comment about skipping LOCKED entries on fdb dump
- skip igmp/mld redir for npi port
- make lan9645x_stats_init void
- add SCNT_TX_BUFDROP to tx_dropped
- change rmon range {0,64} -> {64, 64}. Runt frames counted elsewhere.
- remove rx_crc, rx_symbol_err from rx_packets, as they are already
counted in SZ_* buckets.
- add defensive cancel_delayed_work_sync in stats_free
- Link to v4: https://lore.kernel.org/r/20260430-dsa_lan9645x_switch_driver_base-v4-0-f1b6005fa8b7@microchip.com
Changes in v4:
- v3 was deferred, but I made some changes based on the Sashiko review
- Individual patches mention specific v4 changes
- Fix offset in postpull_rcsum so prefix eth header is cleared, not
actual eth header, so tag driver works with CHECKSUM_COMPLETE host
NICs
- Fix untagged rx on vlan aware port with pvid
- Add comment to QSYS_RES_CFG configuration
- Phylink_mac_prepare: fix to make sure we can dynamically change rgmii
on port 4
- Move ports allocation to probe
- tag_npi_setup: reject cascaded setups
- Skip WARN_ON in lan9645x_to_port
- set_host_flood changed to per port work to coalesce values and skip
atomic allocations
- Fix clear HOST_PVID vlan membership when a port joins a bridge.
- Explicit default value write to tag type register for untagged frames
- Use lan_rmw for ANA_DROP_CFG
- Add comment for error path in lan9645x_vlan_hw_wr
- Remove mac_entries list and just do direct IO to mac table from
fdb_add/fdb_del.
- Clean up fresh mdb when hw mac table write fails.
- Remove rx_uc and tx_uc from ethtool stats list, as they are derivable
from the eth-mac group
- Split stats_init into stats_alloc and stats_init, use alloc in probe
and init in dsa_setup
- Link to v3: https://lore.kernel.org/r/20260410-dsa_lan9645x_switch_driver_base-v3-0-aadc8595306d@microchip.com
Changes in v3:
- Individual patches mention specific v3 changes.
- Add guard before vlan_remove_tag on xmit
- Add pskb_may_pull checks on rx
- Remove additionalProperties: true in bindings
- Remove unnecessary | from description in bindings
- Change top level $ref to dsa.yaml#/$defs/ethernet-ports
- Use ethernet-ports and ethernet-port
- Move ethernet-ports under properties instead of patternProperties
- Move unevaluatedProperties: false after $ref
- Update bindings example to use ethernet-ports and ethernet-port
- Move DEV_MAC_TAGS_CFG to port setup, instead of vlan config, so vlan
overhead is always included in port frame maxlen calculation.
- Remove code disabling ipv6 on conduit
- Use of_property_read_u32 for {rx,tx}-internal-delay-ps
- Use dsa_user_ports(ds) instead of
GENMASK(lan9645x->num_phys_ports - 1, 0) as base flood mask.
- Add comment explaining obey vlan
- Allow disabling aging with explicit zero parameters.
- Fix non-forwarding STP states in bridge fwd calculation.
- Restore host flood state on bridge leave.
- Avoid mac_entry dealloc when mac table writes fail.
- Avoid mdb_entry dealloc when mac table writes fail.
- Dealloc mac_entries on deinit.
- Dealloc mdb_entries on deinit.
- Link to v2: https://lore.kernel.org/r/20260324-dsa_lan9645x_switch_driver_base-v2-0-f7504e3b0681@microchip.com
Changes in v2:
- Individual patches have specific v2 changes.
- Ran DSA, and std counters, selftests, which prompted several changes.
The following selftests pass, except for some expected failures:
- bridge_vlan_aware.sh
- bridge_vlan_unaware.sh
- bridge_vlan_mcast.sh
- no_forwarding.sh
- bridge_mdb.sh
- bridge_mld.sh
- test_fdb_stress_test.sh
- .../drivers/net/hw/ethtool_rmon.sh
- .../drivers/net/hw/ethtool_std_stats.sh (from Ioana's series)
- Added new patch for MDB management, as this was required for selftests.
- Added port_set_host_flood to enable unknown traffic to standalone during
promisc/ALL_MULTI (selftests).
- Remove the dubugfs.
- Link to v1: https://lore.kernel.org/r/20260303-dsa_lan9645x_switch_driver_base-v1-0-bff8ca1396f5@microchip.com
---
Jens Emil Schulz Østergaard (9):
net: dsa: add tag driver for LAN9645X
dt-bindings: net: lan9645x: add LAN9645X switch bindings
net: dsa: lan9645x: add autogenerated register macros
net: dsa: lan9645x: add basic dsa driver for LAN9645X
net: dsa: lan9645x: add vlan support
net: dsa: lan9645x: add mac table integration
net: dsa: lan9645x: add mdb management
net: dsa: lan9645x: add bridge support
net: dsa: lan9645x: add port statistics
.../net/dsa/microchip,lan96455s-switch.yaml | 177 ++
MAINTAINERS | 10 +
drivers/net/dsa/Kconfig | 2 +
drivers/net/dsa/microchip/Makefile | 1 +
drivers/net/dsa/microchip/lan9645x/Kconfig | 12 +
drivers/net/dsa/microchip/lan9645x/Makefile | 12 +
drivers/net/dsa/microchip/lan9645x/lan9645x_mac.c | 268 +++
drivers/net/dsa/microchip/lan9645x/lan9645x_main.c | 1031 +++++++++++
drivers/net/dsa/microchip/lan9645x/lan9645x_main.h | 435 +++++
drivers/net/dsa/microchip/lan9645x/lan9645x_mdb.c | 569 ++++++
drivers/net/dsa/microchip/lan9645x/lan9645x_npi.c | 116 ++
.../net/dsa/microchip/lan9645x/lan9645x_phylink.c | 371 ++++
drivers/net/dsa/microchip/lan9645x/lan9645x_port.c | 180 ++
drivers/net/dsa/microchip/lan9645x/lan9645x_regs.h | 1932 ++++++++++++++++++++
.../net/dsa/microchip/lan9645x/lan9645x_stats.c | 856 +++++++++
.../net/dsa/microchip/lan9645x/lan9645x_stats.h | 229 +++
drivers/net/dsa/microchip/lan9645x/lan9645x_vlan.c | 411 +++++
include/linux/dsa/lan9645x.h | 54 +
include/net/dsa.h | 2 +
net/dsa/Kconfig | 11 +
net/dsa/Makefile | 1 +
net/dsa/tag_lan9645x.c | 474 +++++
22 files changed, 7154 insertions(+)
---
base-commit: 014d795c73837ea2339a4ea8e8f82c6e959b845d
change-id: 20260210-dsa_lan9645x_switch_driver_base-312bbfc37edb
Best regards,
--
Jens Emil Schulz Østergaard <jensemil.schulzostergaard@microchip.com>
^ permalink raw reply [flat|nested] 29+ messages in thread* [PATCH net-next v13 1/9] net: dsa: add tag driver for LAN9645X 2026-09-29 7:48 [PATCH net-next v13 0/9] net: dsa: add DSA support for the LAN9645x switch chip family Jens Emil Schulz Østergaard @ 2026-09-29 7:48 ` Jens Emil Schulz Østergaard 2026-09-30 7:50 ` sashiko-bot 2026-10-02 21:14 ` netdev-bot+sashiko 2026-09-29 7:48 ` [PATCH net-next v13 2/9] dt-bindings: net: lan9645x: add LAN9645X switch bindings Jens Emil Schulz Østergaard ` (7 subsequent siblings) 8 siblings, 2 replies; 29+ messages in thread From: Jens Emil Schulz Østergaard @ 2026-09-29 7:48 UTC (permalink / raw) To: UNGLinuxDriver, Andrew Lunn, Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Woojung Huh, Russell King, Steen Hegelund, Daniel Machon, Geert Uytterhoeven, Magnus Damm Cc: linux-kernel, netdev, devicetree, linux-renesas-soc, Jens Emil Schulz Østergaard Add tag driver for LAN9645x using a front port as CPU port. This mode is called an NPI port in the datasheet. Use long prefix on extraction (RX) and no prefix on injection (TX). A long prefix on extraction helps get through the conduit port on host side, since it will see a broadcast MAC. The LAN9645x chip is in the same design architecture family as ocelot and lan966x. The tagging protocol has the same structure as these chips, but the particular fields are different or have different sizes. Therefore, this tag driver is similar to tag_ocelot.c, but the differences in fields makes it hard to reuse. LAN9645x supports 3 different tag formats for extraction/injection of frames from a CPU port: long prefix, short prefix and no prefix. The tag is prepended to the frame. The critical data for the chip is contained in an internal frame header (IFH) which is 28 bytes. The prefix formats look like this: Long prefix (16 bytes) + IFH: - DMAC = 0xffffffffffff on extraction. - SMAC = 0xfeffffffffff on extraction. - ETYPE = 0x8880 - payload = 0x0011 - IFH Short prefix (4 bytes) + IFH: - 0x8880 - 0x0011 - IFH No prefix: - IFH The format can be configured asymmetrically on RX and TX. The IFH get/set functions are declared as inline. All the field constants are compile-time known, so when these calls are inlined efficient code is generated with branches pruned and loops unrolled. During testing it was observed that without explicit inlining GCC would have trouble inlining the functions, which hurt performance. A frame extracted from a VLAN-aware port arrives without its VLAN tag. REW_PORT_CFG.NO_REWRITE is clear on the NPI port, so the rewriter applies the classified VLAN_POP_CNT, and the classified VID reaches the CPU only through the IFH. The tagger restores it as a hwaccel tag, except when it equals the port pvid, where a frame tagged with the pvid is indistinguishable from an untagged one. The restored tag is always a C-tag, not the TPID the IFH TAG_TYPE reports. Both TPIDs are recognized as VLAN tags unconditionally, with no per port control to treat one as untagged customer data, so an S-tag is already consumed as the VLAN tag before extraction. Restoring 802.1AD would make the bridge push the tag back into the payload and reclassify the frame to the port pvid, placing it on a different VID than the one the hardware forwarded it on. Normalizing to a C-tag keeps the terminated path consistent with the forwarded path, at the cost of not preserving the S-tag TPID towards the CPU. This assumes a bridge vlan_protocol of 802.1Q, which is the only protocol the driver offloads. Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com> Signed-off-by: Jens Emil Schulz Østergaard <jensemil.schulzostergaard@microchip.com> --- Changes in v12: - Move an out-of-band VLAN tag into the payload with __vlan_hwaccel_push_inside() before reading it, rather than relying on the conduit never advertising NETIF_F_HW_VLAN_CTAG_TX or NETIF_F_HW_VLAN_STAG_TX in vlan_features. - Return the skb from lan9645x_xmit_get_vlan_info(), since __vlan_hwaccel_push_inside() may reallocate and frees the skb on failure. - Annotate the IFH field defines with injection/extraction applicability - Drop IFH_TIMESTAMP because it is 38 bits wide. Rely on NS/SUB_NS subfields. - Document how HW derives the DMAC offset from pop_cnt and etype_ofs, and drop frames whose encoding only a tag push could produce. - Drop unncessary IFH_SRCPORT on injection. - Drop the WARN_ON_ONCE() when the source port does not resolve to a user port. - Move the IFH field position and size defines from include/linux/dsa/lan9645x.h into net/dsa/tag_lan9645x.c. - Rename LAN9645X_IFH_LEN to LAN9645X_IFH_LEN_BYTES, so it does not read as the position of the IFH LEN field. - Use skb_vlan_eth_hdr() in lan9645x_xmit_get_vlan_info(). - Drop BTM_MSK() TOP_MSK() helpers for GENMASK(). - Rename set_merge_mask() to lan9645x_ifh_merge_byte(). - Name the vendor in the Kconfig prompt, spell the part LAN9645x, and move the entry to its sorted position after NET_DSA_TAG_KSZ. - Clamp the classified QoS class to the width of IFH_QOS_CLASS. A skb->priority above 7 aliased down onto an unrelated class. - Comment why needed_headroom covers the extraction prefix. - Restore the classified VLAN as a C-tag rather than following the IFH TAG_TYPE, so a terminated S-tagged frame lands on the same VID the hardware forwards it on. Changes in v11: - Update comment about vlan tag hwaccel use in tag driver. - Bump DSA_TAG_PROTO value in include/net/dsa.h. Changes in v10: - Update tag driver after skb ownership model change in DSA taggers. Explicitly freeing the skb at rcv/xmit return points. Changes in v8: - Drop the cpu_port local and use ds->num_ports directly as the IFH source port (the CPU port module index). - Reword the reflection comment to refer to "the CPU". Changes in v7: - Introduce cpu queue based frame classification, and refactor to the categories default, trap and copy, which mirrors usage instead of being based on frame types. Changes in v6: - rebased on net-next, bumping DSA_TAG_PROTO_LAN9645X_VALUE to 34 Changes in v5: - Undo offset fix in postpull_rcsum. The original logic was correct for CHECKSUM_COMPLETE host NICs - Use __always_inline in lan9645x_ifh_{get,set} - remove double space after = in set_merge_mask Changes in v4: - Fix offset in postpull_rcsum so prefix eth header is cleared, not actual eth header, so tag driver works with CHECKSUM_COMPLETE host NICs - Fix untagged rx on vlan aware port with pvid Changes in v3: - guard vlan_remove_tag behind skb_headlen(skb) >= VLAN_ETH_HLEN on xmit - add pskb_may_pull checks in rx path Changes in v2: - sorting in net/dsa/Kconfig - sorting in net/dsa/Makefile - remove default zero promisc_on_conduit - move functions to to .c file - add justification for inline usage to commit message - add __skb_put_padto on xmit path - fix hwaccel_put_tag --- MAINTAINERS | 8 + include/linux/dsa/lan9645x.h | 54 +++++ include/net/dsa.h | 2 + net/dsa/Kconfig | 11 + net/dsa/Makefile | 1 + net/dsa/tag_lan9645x.c | 474 +++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 550 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 6de1ff058db6..e2cbcf89d36e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -17800,6 +17800,14 @@ L: netdev@vger.kernel.org S: Maintained F: drivers/net/phy/microchip_t1.c +MICROCHIP LAN9645X ETHERNET SWITCH DRIVER +M: Jens Emil Schulz Østergaard <jensemil.schulzostergaard@microchip.com> +M: UNGLinuxDriver@microchip.com +L: netdev@vger.kernel.org +S: Maintained +F: include/linux/dsa/lan9645x.h +F: net/dsa/tag_lan9645x.c + MICROCHIP LAN966X ETHERNET DRIVER M: Horatiu Vultur <horatiu.vultur@microchip.com> M: UNGLinuxDriver@microchip.com diff --git a/include/linux/dsa/lan9645x.h b/include/linux/dsa/lan9645x.h new file mode 100644 index 000000000000..e774aa137092 --- /dev/null +++ b/include/linux/dsa/lan9645x.h @@ -0,0 +1,54 @@ +/* SPDX-License-Identifier: GPL-2.0 + * Copyright (C) 2026 Microchip Technology Inc. + */ + +#ifndef _NET_DSA_TAG_LAN9645X_H_ +#define _NET_DSA_TAG_LAN9645X_H_ + +#include <linux/bits.h> +#include <linux/types.h> + +/* LAN9645x supports 3 different formats on an NPI port, long prefix, short + * prefix and no prefix. The format can be configured asymmetrically on RX and + * TX. We use long prefix on extraction (RX), and no prefix on injection. + * The long prefix on extraction helps get through the conduit port on host + * side, since it will see a broadcast MAC. + * + * The internal frame header (IFH) is 28 bytes. + * + * Long prefix, 16 bytes + IFH: + * - DMAC = 0xFFFFFFFFFFFF on extraction. + * - SMAC = 0xFEFFFFFFFFFF on extraction. + * - ETYPE = 0x8880 + * - payload = 0x0011 + * - IFH + * + * Short prefix, 4 bytes + IFH: + * - 0x8880 + * - 0x0011 + * - IFH + * + * No prefix: + * - IFH + * + */ +#define LAN9645X_IFH_TAG_TYPE_C 0 +#define LAN9645X_IFH_TAG_TYPE_S 1 +#define LAN9645X_IFH_LEN_U32 7 +#define LAN9645X_IFH_LEN_BYTES (LAN9645X_IFH_LEN_U32 * sizeof(u32)) +#define LAN9645X_IFH_BITS (LAN9645X_IFH_LEN_BYTES * BITS_PER_BYTE) +#define LAN9645X_LONG_PREFIX_LEN 16 +#define LAN9645X_TOTAL_TAG_LEN \ + (LAN9645X_LONG_PREFIX_LEN + LAN9645X_IFH_LEN_BYTES) + +/* Chip has 8 cpu queues. The cpu queues used by a frame are passed as a mask in + * the IFH on extraction. We use this to avoid classifying BPDU, IGMP and MLD + * frames in the tag driver. + */ +enum { + LAN9645X_CPUQ_DEF = 0, + LAN9645X_CPUQ_TRAP = 1, + LAN9645X_CPUQ_COPY = 2, +}; + +#endif /* _NET_DSA_TAG_LAN9645X_H_ */ diff --git a/include/net/dsa.h b/include/net/dsa.h index 5d12191b6f6f..09f3043d57f0 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -62,6 +62,7 @@ struct tc_action; #define DSA_TAG_PROTO_KSZ8463_VALUE 34 #define DSA_TAG_PROTO_MT7628_VALUE 35 #define DSA_TAG_PROTO_KS8995_VALUE 36 +#define DSA_TAG_PROTO_LAN9645X_VALUE 37 enum dsa_tag_protocol { DSA_TAG_PROTO_NONE = DSA_TAG_PROTO_NONE_VALUE, @@ -101,6 +102,7 @@ enum dsa_tag_protocol { DSA_TAG_PROTO_KSZ8463 = DSA_TAG_PROTO_KSZ8463_VALUE, DSA_TAG_PROTO_MT7628 = DSA_TAG_PROTO_MT7628_VALUE, DSA_TAG_PROTO_KS8995 = DSA_TAG_PROTO_KS8995_VALUE, + DSA_TAG_PROTO_LAN9645X = DSA_TAG_PROTO_LAN9645X_VALUE, }; struct dsa_switch; diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig index 4f44bf3ede23..bdeb60af764b 100644 --- a/net/dsa/Kconfig +++ b/net/dsa/Kconfig @@ -137,6 +137,17 @@ config NET_DSA_TAG_KSZ Say Y if you want to enable support for tagging frames for the Microchip 8795/937x/9477/9893 families of switches. +config NET_DSA_TAG_LAN9645X + tristate "Tag driver for Microchip LAN9645x switches" + help + Say Y or M if you want to enable NPI tagging for the Microchip + LAN9645x switches. In this mode, the frames over the Ethernet CPU + port are prepended with a hardware-defined injection/extraction frame + header. On injection a 28 byte internal frame header (IFH) is used. + On extraction a 16 byte prefix is prepended before the internal frame + header. This prefix starts with a broadcast MAC, to ease passage + through the host side RX filter. + config NET_DSA_TAG_NETC tristate "Tag driver for NXP NETC switches" help diff --git a/net/dsa/Makefile b/net/dsa/Makefile index 1f9cc30e9988..3cd53124448d 100644 --- a/net/dsa/Makefile +++ b/net/dsa/Makefile @@ -28,6 +28,7 @@ obj-$(CONFIG_NET_DSA_TAG_HELLCREEK) += tag_hellcreek.o obj-$(CONFIG_NET_DSA_TAG_KS8995) += tag_ks8995.o obj-$(CONFIG_NET_DSA_TAG_KSZ) += tag_ksz.o obj-$(CONFIG_NET_DSA_TAG_LAN9303) += tag_lan9303.o +obj-$(CONFIG_NET_DSA_TAG_LAN9645X) += tag_lan9645x.o obj-$(CONFIG_NET_DSA_TAG_MT7628) += tag_mt7628.o obj-$(CONFIG_NET_DSA_TAG_MTK) += tag_mtk.o obj-$(CONFIG_NET_DSA_TAG_MXL_862XX) += tag_mxl862xx.o diff --git a/net/dsa/tag_lan9645x.c b/net/dsa/tag_lan9645x.c new file mode 100644 index 000000000000..f54646d4b394 --- /dev/null +++ b/net/dsa/tag_lan9645x.c @@ -0,0 +1,474 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (C) 2026 Microchip Technology Inc. + */ + +#include <linux/dsa/lan9645x.h> + +#include "tag.h" + +#define LAN9645X_NAME "lan9645x" + +/* The internal frame header (IFH) is 28 bytes, and the fields are documented + * below. Some fields are only used on either injection or extraction. + * + * Injection header + */ +#define IFH_INJ_TIMESTAMP 192 +#define IFH_BYPASS 191 +#define IFH_MASQ 190 +/* Extraction header */ +#define IFH_TIMESTAMP_NS 194 +#define IFH_TIMESTAMP_SUBNS 186 +/* Injection header */ +#define IFH_MASQ_PORT 186 +#define IFH_RCT_INJ 185 +/* Extraction header */ +#define IFH_LEN 171 +#define IFH_WRDMODE 169 +/* Extraction/Injection header */ +#define IFH_RTAGD 167 +/* Extraction header */ +#define IFH_CUTTHRU 166 +/* Extraction/Injection header */ +#define IFH_REW_CMD 156 +#define IFH_REW_OAM 155 +#define IFH_PDU_TYPE 151 +#define IFH_FCS_UPD 150 +#define IFH_DP 149 +/* Reserved */ +#define IFH_RTE_INB_UPDATE 148 +/* Extraction/Injection header */ +#define IFH_POP_CNT 146 +#define IFH_ETYPE_OFS 144 +/* Extraction header */ +#define IFH_SRCPORT 140 +/* Extraction/Injection header */ +#define IFH_SEQ_NUM 120 +#define IFH_TAG_TYPE 119 +#define IFH_TCI 103 +#define IFH_DSCP 97 +#define IFH_QOS_CLASS 94 +#define IFH_CPUQ 86 +/* Extraction header */ +#define IFH_LEARN_FLAGS 84 +/* Extraction/Injection header */ +#define IFH_SFLOW_ID 80 +#define IFH_ACL_HIT 79 +#define IFH_ACL_IDX 73 +#define IFH_ISDX 65 +#define IFH_DSTS 55 +/* Extraction header */ +#define IFH_FLOOD 53 +/* Extraction/Injection header */ +#define IFH_SEQ_OP 51 +#define IFH_IPV 48 +/* Injection header */ +#define IFH_AFI 47 +/* Reserved */ +#define IFH_RTP_ID 37 +#define IFH_RTP_SUBID 36 +#define IFH_PN_DATA_STATUS 28 +#define IFH_PN_TRANSF_STATUS_ZERO 27 +#define IFH_PN_CC 11 +/* Extraction/Injection header */ +#define IFH_DUPL_DISC_ENA 10 +/* Extraction header */ +#define IFH_RCT_AVAIL 9 + +#define IFH_INJ_TIMESTAMP_SZ 32 +#define IFH_BYPASS_SZ 1 +#define IFH_MASQ_SZ 1 +#define IFH_TIMESTAMP_NS_SZ 30 +#define IFH_TIMESTAMP_SUBNS_SZ 8 +#define IFH_MASQ_PORT_SZ 4 +#define IFH_RCT_INJ_SZ 1 +#define IFH_LEN_SZ 14 +#define IFH_WRDMODE_SZ 2 +#define IFH_RTAGD_SZ 2 +#define IFH_CUTTHRU_SZ 1 +#define IFH_REW_CMD_SZ 10 +#define IFH_REW_OAM_SZ 1 +#define IFH_PDU_TYPE_SZ 4 +#define IFH_FCS_UPD_SZ 1 +#define IFH_DP_SZ 1 +#define IFH_RTE_INB_UPDATE_SZ 1 +#define IFH_POP_CNT_SZ 2 +#define IFH_ETYPE_OFS_SZ 2 +#define IFH_SRCPORT_SZ 4 +#define IFH_SEQ_NUM_SZ 16 +#define IFH_TAG_TYPE_SZ 1 +#define IFH_TCI_SZ 16 +#define IFH_DSCP_SZ 6 +#define IFH_QOS_CLASS_SZ 3 +#define IFH_CPUQ_SZ 8 +#define IFH_LEARN_FLAGS_SZ 2 +#define IFH_SFLOW_ID_SZ 4 +#define IFH_ACL_HIT_SZ 1 +#define IFH_ACL_IDX_SZ 6 +#define IFH_ISDX_SZ 8 +#define IFH_DSTS_SZ 10 +#define IFH_FLOOD_SZ 2 +#define IFH_SEQ_OP_SZ 2 +#define IFH_IPV_SZ 3 +#define IFH_AFI_SZ 1 +#define IFH_RTP_ID_SZ 10 +#define IFH_RTP_SUBID_SZ 1 +#define IFH_PN_DATA_STATUS_SZ 8 +#define IFH_PN_TRANSF_STATUS_ZERO_SZ 1 +#define IFH_PN_CC_SZ 16 +#define IFH_DUPL_DISC_ENA_SZ 1 +#define IFH_RCT_AVAIL_SZ 1 + +static __always_inline void lan9645x_ifh_merge_byte(u8 *dst, u8 src, u8 mask) +{ + *dst = *dst ^ ((*dst ^ src) & mask); +} + +/* The internal frame header (IFH) is a big-endian 28 byte unpadded bit array. + * Frames can be prepended with an IFH on injection and extraction. There + * are two field layouts, one for extraction and one for injection. + * + * IFH bits go from high to low, for instance + * ifh[0] = [223:216] + * ifh[27] = [7:0] + * + * Here is an example of setting a value starting at bit 13 of bit length 17. + * + * val = 0x1ff + * pos = 13 + * length = 17 + * + * + * IFH[] 0 23 24 25 26 27 + * + * end_u8 start_u8 + * +--------+----------------+--------+--------+--------+--------+--------+ + * | | | | | | | | + * IFH | | .... | | vvvvvvvvvvvvvvvvvvv | | + * | | | | | | | | | | + * +--------+----------------+--------+--+-----+--------+--+-----+--------+ + * Bits 223 39 32 31| 24 23 16 15| 8 7 0 + * | | + * | | + * | | + * v v + * end = 29 pos = 13 + * end_rem = 5 pos_rem = 5 + * end_u8 = 3 start_u8 = 1 + * GENMASK(5, 0) = 0x3f GENMASK(7, 5) = 0xe0 + * + * + * In end_u8 and start_u8 we must merge the existing IFH byte with the new + * value. In the 'middle' bytes of the value we can overwrite the corresponding + * IFH byte. + */ +static __always_inline void lan9645x_ifh_set(u8 *ifh, u32 val, size_t pos, + size_t length) +{ + size_t end = (pos + length) - 1; + size_t end_rem = end & 0x7; + size_t pos_rem = pos & 0x7; + size_t start_u8 = pos >> 3; + size_t end_u8 = end >> 3; + u8 end_mask, start_mask; + size_t vshift; + u8 *ptr; + + BUILD_BUG_ON_MSG(length > 32, "IFH field size wider than 32."); + BUILD_BUG_ON_MSG(length == 0, "IFH field size of 0."); + BUILD_BUG_ON_MSG(pos + length > LAN9645X_IFH_BITS, + "IFH field overflows IFH"); + + end_mask = GENMASK(end_rem, 0); + start_mask = GENMASK(7, pos_rem); + + ptr = &ifh[LAN9645X_IFH_LEN_BYTES - 1 - end_u8]; + + if (end_u8 == start_u8) + return lan9645x_ifh_merge_byte(ptr, val << pos_rem, + end_mask & start_mask); + + vshift = length - end_rem - 1; + lan9645x_ifh_merge_byte(ptr++, val >> vshift, end_mask); + + for (size_t j = 1; j < end_u8 - start_u8; j++) { + vshift -= 8; + *ptr++ = val >> vshift; + } + + lan9645x_ifh_merge_byte(ptr, val << pos_rem, start_mask); +} + +static __always_inline u32 lan9645x_ifh_get(const u8 *ifh, size_t pos, + size_t length) +{ + size_t end = (pos + length) - 1; + size_t end_rem = end & 0x7; + size_t pos_rem = pos & 0x7; + size_t start_u8 = pos >> 3; + size_t end_u8 = end >> 3; + u8 end_mask, start_mask; + const u8 *ptr; + u32 val; + + BUILD_BUG_ON_MSG(length > 32, "IFH field size wider than 32."); + BUILD_BUG_ON_MSG(length == 0, "IFH field size of 0."); + BUILD_BUG_ON_MSG(pos + length > LAN9645X_IFH_BITS, + "IFH field overflows IFH"); + + end_mask = GENMASK(end_rem, 0); + start_mask = GENMASK(7, pos_rem); + + ptr = &ifh[LAN9645X_IFH_LEN_BYTES - 1 - end_u8]; + + if (end_u8 == start_u8) + return (*ptr & end_mask & start_mask) >> pos_rem; + + val = *ptr++ & end_mask; + + for (size_t j = 1; j < end_u8 - start_u8; j++) + val = val << 8 | *ptr++; + + return val << (8 - pos_rem) | (*ptr & start_mask) >> pos_rem; +} + +static struct sk_buff *lan9645x_xmit_get_vlan_info(struct sk_buff *skb, + struct net_device *br, + u32 *vlan_tci, + u32 *tag_type) +{ + struct vlan_ethhdr *hdr; + u16 proto, tci; + + /* If the VLAN tag is in the hwaccel area, move it to the payload so + * that both cases are handled uniformly below, and so that the conduit + * cannot insert it into the middle of the IFH we are about to prepend. + */ + if (unlikely(skb_vlan_tag_present(skb))) { + skb = __vlan_hwaccel_push_inside(skb); + if (!skb) + return NULL; + } + + if (!br || !br_vlan_enabled(br)) { + *vlan_tci = 0; + *tag_type = LAN9645X_IFH_TAG_TYPE_C; + return skb; + } + + hdr = skb_vlan_eth_hdr(skb); + br_vlan_get_proto(br, &proto); + + if (skb_headlen(skb) >= VLAN_ETH_HLEN && + ntohs(hdr->h_vlan_proto) == proto) { + vlan_remove_tag(skb, &tci); + *vlan_tci = tci; + } else { + rcu_read_lock(); + br_vlan_get_pvid_rcu(br, &tci); + rcu_read_unlock(); + *vlan_tci = tci; + } + + *tag_type = (proto != ETH_P_8021Q) ? LAN9645X_IFH_TAG_TYPE_S : + LAN9645X_IFH_TAG_TYPE_C; + + return skb; +} + +static void lan9645x_offload_fwd_mark(struct sk_buff *skb, u32 cpuq) +{ + /* Trapped frames must be forwarded by the stack. */ + if (cpuq & BIT(LAN9645X_CPUQ_TRAP)) { + skb->offload_fwd_mark = 0; + return; + } + + dsa_default_offload_fwd_mark(skb); +} + +static struct sk_buff *lan9645x_xmit(struct sk_buff *skb, + struct net_device *ndev) +{ + struct dsa_port *dp = dsa_user_to_port(ndev); + u32 vlan_tci, tag_type; + u32 qos_class; + void *ifh; + + skb = lan9645x_xmit_get_vlan_info(skb, dsa_port_bridge_dev_get(dp), + &vlan_tci, &tag_type); + if (!skb) + return NULL; + + /* We need to make sure frame has the proper size after IFH is stripped + * by hw. + */ + if (skb_put_padto(skb, ETH_ZLEN)) + return NULL; + + qos_class = netdev_get_num_tc(ndev) ? + netdev_get_prio_tc_map(ndev, skb->priority) : + skb->priority; + qos_class = min_t(u32, qos_class, GENMASK(IFH_QOS_CLASS_SZ - 1, 0)); + + /* Make room for IFH */ + ifh = skb_push(skb, LAN9645X_IFH_LEN_BYTES); + memset(ifh, 0, LAN9645X_IFH_LEN_BYTES); + + lan9645x_ifh_set(ifh, 1, IFH_BYPASS, IFH_BYPASS_SZ); + lan9645x_ifh_set(ifh, tag_type, IFH_TAG_TYPE, IFH_TAG_TYPE_SZ); + lan9645x_ifh_set(ifh, vlan_tci, IFH_TCI, IFH_TCI_SZ); + lan9645x_ifh_set(ifh, qos_class, IFH_QOS_CLASS, IFH_QOS_CLASS_SZ); + lan9645x_ifh_set(ifh, BIT(dp->index), IFH_DSTS, IFH_DSTS_SZ); + + return skb; +} + +static struct sk_buff *lan9645x_rcv(struct sk_buff *skb, + struct net_device *ndev) +{ + u32 src_port, qos_class, vlan_tci, popcnt, etype_ofs, cpuq; + struct dsa_port *dp; + u32 ifh_gap_len = 0; + u8 *ifh; + + /* Conduit already consumed DMAC,SMAC,ETYPE from long prefix. Go back + * to beginning of frame. + */ + skb_push(skb, ETH_HLEN); + + if (unlikely(!pskb_may_pull(skb, LAN9645X_TOTAL_TAG_LEN))) { + kfree_skb(skb); + return NULL; + } + + /* IFH starts after our long prefix */ + ifh = skb_pull(skb, LAN9645X_LONG_PREFIX_LEN); + + popcnt = lan9645x_ifh_get(ifh, IFH_POP_CNT, IFH_POP_CNT_SZ); + etype_ofs = lan9645x_ifh_get(ifh, IFH_ETYPE_OFS, IFH_ETYPE_OFS_SZ); + src_port = lan9645x_ifh_get(ifh, IFH_SRCPORT, IFH_SRCPORT_SZ); + vlan_tci = lan9645x_ifh_get(ifh, IFH_TCI, IFH_TCI_SZ); + qos_class = lan9645x_ifh_get(ifh, IFH_QOS_CLASS, IFH_QOS_CLASS_SZ); + cpuq = lan9645x_ifh_get(ifh, IFH_CPUQ, IFH_CPUQ_SZ); + + /* Tag pushing is disabled on the NPI port via REW_TAG_CFG, so if this + * fires REW_TAG_CFG is misconfigured. + */ + if (popcnt == 1 || + (popcnt == 0 && etype_ofs > 0)) { + kfree_skb(skb); + return NULL; + } + + /* Since REW_PORT_CFG_NO_REWRITE=0 is required on the NPI port, we need + * to account for any tags popped by the hardware, as that will leave a + * gap between the IFH and DMAC. Tag pushing is disabled. + * + * The IFH fields do not have intuitive values. This is how HW does the + * calculation: + * + * DMAC_DT = (ifh.pop_cnt == 0 && ifh.etype_ofs == 0) ? 4 : ifh.pop_cnt + * DMAC_OFFSET = TAG_SIZE + 4*(DMAC_DT - 2) + * + * With tag pushing disabled we have either + * + * popcnt=0 and etype_ofs=0 => 2x pop + * popcnt=3 and etype_ofs=* => 1x pop + * popcnt=2 and etype_ofs=* => no pop + * + * The remaining combinations indicate a push and will not occur. + */ + if (popcnt == 0 && etype_ofs == 0) + ifh_gap_len = 2 * VLAN_HLEN; + else if (popcnt == 3) + ifh_gap_len = VLAN_HLEN; + + /* Set skb->data at start of real header */ + skb_pull(skb, LAN9645X_IFH_LEN_BYTES); + + if (unlikely(!pskb_may_pull(skb, ifh_gap_len + ETH_HLEN))) { + kfree_skb(skb); + return NULL; + } + + skb_pull(skb, ifh_gap_len); + skb_reset_mac_header(skb); + skb_set_network_header(skb, ETH_HLEN); + skb_reset_mac_len(skb); + + /* Reset skb->data past the actual ethernet header. */ + skb_pull(skb, ETH_HLEN); + + /* We must deliver the skb so skb->csum only covers the data beyond the + * real ethernet header. The fake ethernet header in the prefix is + * not part of skb->csum already. We must subtract what remains of the + * prefix, the ifh and the gap. The start is derived from the current + * skb->data rather than saved on entry, because the pskb_may_pull() + * calls above may have reallocated skb->head. + */ + skb_postpull_rcsum(skb, + skb->data - LAN9645X_TOTAL_TAG_LEN - ifh_gap_len, + LAN9645X_TOTAL_TAG_LEN + ifh_gap_len); + + skb->dev = dsa_conduit_find_user(ndev, 0, src_port); + if (!skb->dev) { + /* Reflection is disabled for frames from the tag driver itself, + * however it is possible that a frame sent directly on the + * conduit gets reflected, so we drop it here. + */ + kfree_skb(skb); + return NULL; + } + + lan9645x_offload_fwd_mark(skb, cpuq); + + skb->priority = qos_class; + + /* While we have REW_PORT_CFG_NO_REWRITE=0 on the NPI port, we still + * disable port VLAN tag pushing with REW_TAG_CFG. A frame ingressing + * on a vlan aware port, which is forwarded to the CPU, will not carry + * vlan info in the frame data, because the tag is popped. The + * classified VID is only communicated via the IFH, never in the + * payload. We therefore restore it via hwaccel and must not pop an + * in-band tag here. + */ + dp = dsa_user_to_port(skb->dev); + + if (dsa_port_is_vlan_filtering(dp) && vlan_tci) { + u16 port_pvid = 0; + + br_vlan_get_pvid_rcu(skb->dev, &port_pvid); + + /* The tag is restored as a C-tag, not as the TAG_TYPE the IFH + * reports. The classifier recognizes both TPIDs as VLAN tags, + * so an S-tag has already been used for classification by the + * time we get here. Restoring it as 802.1AD would make the + * bridge push it back into the payload and reclassify the frame + * to the port pvid, on a different VID than the one the + * hardware forwarded it on. + */ + if ((vlan_tci & VLAN_VID_MASK) != port_pvid) + __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), + vlan_tci); + } + + return skb; +} + +static const struct dsa_device_ops lan9645x_netdev_ops = { + .name = LAN9645X_NAME, + .proto = DSA_TAG_PROTO_LAN9645X, + .xmit = lan9645x_xmit, + .rcv = lan9645x_rcv, + /* Covers the extraction prefix too, since dsa_tag_protocol_overhead() + * sizes the conduit MTU from this. + */ + .needed_headroom = LAN9645X_TOTAL_TAG_LEN, +}; + +MODULE_DESCRIPTION("DSA tag driver for LAN9645x family of switches, using NPI port"); +MODULE_LICENSE("GPL"); +MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_LAN9645X, LAN9645X_NAME); + +module_dsa_tag_driver(lan9645x_netdev_ops); -- 2.52.0 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH net-next v13 1/9] net: dsa: add tag driver for LAN9645X 2026-09-29 7:48 ` [PATCH net-next v13 1/9] net: dsa: add tag driver for LAN9645X Jens Emil Schulz Østergaard @ 2026-09-30 7:50 ` sashiko-bot 2026-10-02 21:14 ` netdev-bot+sashiko 1 sibling, 0 replies; 29+ messages in thread From: sashiko-bot @ 2026-09-30 7:50 UTC (permalink / raw) To: Jens Emil Schulz Østergaard; +Cc: conor+dt, wsa+renesas, robh, devicetree Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Critical] net: dsa: lan9645x: VLAN hopping via fragmented in-band VLAN tags -- commit f7552b4bce99ac2975ea8145332463c0a3772965 Author: Jens Emil Schulz Østergaard <jensemil.schulzostergaard@microchip.com> net: dsa: add tag driver for LAN9645X This commit adds a DSA tag driver for LAN9645x switches using an NPI port. It implements asymmetric prefix configurations, using a long prefix for extraction and an internal frame header (IFH) for both extraction and injection. > diff --git a/net/dsa/tag_lan9645x.c b/net/dsa/tag_lan9645x.c > new file mode 100644 > index 0000000000000..f54646d4b3947 > --- /dev/null > +++ b/net/dsa/tag_lan9645x.c [ ... ] > +static struct sk_buff *lan9645x_xmit_get_vlan_info(struct sk_buff *skb, > + struct net_device *br, > + u32 *vlan_tci, > + u32 *tag_type) > +{ > + struct vlan_ethhdr *hdr; > + u16 proto, tci; [ ... ] > + if (skb_headlen(skb) >= VLAN_ETH_HLEN && > + ntohs(hdr->h_vlan_proto) == proto) { > + vlan_remove_tag(skb, &tci); > + *vlan_tci = tci; > + } else { [Severity: Critical] Does this logic allow VLAN isolation bypass for fragmented in-band VLAN tags? If a user with raw socket access constructs a frame where the 14-byte MAC header is linear, but the in-band 802.1Q VLAN tag is placed in paged fragments, skb_headlen(skb) >= VLAN_ETH_HLEN will evaluate to false. Instead of linearizing the header to securely process the VLAN tag using pskb_may_pull(), the code takes this else branch. This misclassifies the packet as untagged and assigns it the bridge's PVID, leaving the original spoofed 802.1Q tag untouched in the payload. When the frame is later injected into the switch, the hardware rewriter won't pop the payload tag since the switch believes the frame is untagged. The packet would then be transmitted on the wire retaining the user's spoofed 802.1Q tag, completely bypassing the bridge's VLAN filtering and isolation. > + rcu_read_lock(); > + br_vlan_get_pvid_rcu(br, &tci); > + rcu_read_unlock(); > + *vlan_tci = tci; > + } > + > + *tag_type = (proto != ETH_P_8021Q) ? LAN9645X_IFH_TAG_TYPE_S : > + LAN9645X_IFH_TAG_TYPE_C; > + > + return skb; > +} -- Sashiko AI review · https://sashiko.dev/#/patchset/20260929-dsa_lan9645x_switch_driver_base-v13-0-827c2d3617f2@microchip.com?part=1 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH net-next v13 1/9] net: dsa: add tag driver for LAN9645X 2026-09-29 7:48 ` [PATCH net-next v13 1/9] net: dsa: add tag driver for LAN9645X Jens Emil Schulz Østergaard 2026-09-30 7:50 ` sashiko-bot @ 2026-10-02 21:14 ` netdev-bot+sashiko 2026-10-06 7:19 ` Jens Emil Schulz Ostergaard 1 sibling, 1 reply; 29+ messages in thread From: netdev-bot+sashiko @ 2026-10-02 21:14 UTC (permalink / raw) To: jensemil.schulzostergaard Cc: UNGLinuxDriver, andrew, olteanv, davem, edumazet, kuba, pabeni, horms, robh, krzk+dt, conor+dt, woojung.huh, linux, Steen.Hegelund, daniel.machon, geert+renesas, magnus.damm, linux-kernel, netdev, devicetree, linux-renesas-soc Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider. Critical: 0 · High: 1 (1) · Medium: 2 · Low: 3 If you believe none of the issues are legitimate, please reply to each issue of Medium severity or higher and explain why the AI is wrong. If any of them are legitimate and you plan to respin, please let patchwork know by including "pw-bot: cr" as a separate line at the end of your reply (one such reply per series is enough). - [High] In lan9645x_xmit_get_vlan_info() (net/dsa/tag_lan9645x.c), any frame with no in-band tag matching the bridge protocol gets IFH_TCI… - [Medium] lan9645x_netdev_ops sets no .flow_dissect, so __skb_flow_dissect() (net/core/flow_dissector.c:1083-1098) falls back… - [Medium] When the IFH-classified VID equals the port pvid, lan9645x_rcv() skips restoring the tag (tag_lan9645x.c:451-453). - [Low] In lan9645x_xmit_get_vlan_info(), the condition `skb_headlen(skb) >= VLAN_ETH_HLEN && ntohs(hdr->h_vlan_proto) == proto` treats a VLAN… - [Low] In lan9645x_xmit_get_vlan_info(), the guard `skb_headlen(skb) >= VLAN_ETH_HLEN` in front of vlan_remove_tag() does not cover every byte… - [Low] lan9645x_rcv() always restores the classified VLAN as ETH_P_8021Q. Pre-existing issues: - [High] lan9645x_xmit() calls dsa_port_bridge_dev_get(dp) (include/net/dsa.h:774-778, `dp->bridge ? -- net: dsa: add tag driver for LAN9645X This adds a DSA tag driver for the Microchip LAN9645x switch, which uses a front port as the NPI CPU port. On extraction, frames carry a 16 byte long prefix plus a 28 byte IFH. On injection, frames carry only the IFH. On receive, the classified VLAN is restored as a hwaccel C-tag unless it equals the port pvid. > diff --git a/net/dsa/tag_lan9645x.c b/net/dsa/tag_lan9645x.c > new file mode 100644 > index 0000000000000..f54646d4b3947 > --- /dev/null > +++ b/net/dsa/tag_lan9645x.c > @@ -0,0 +1,474 @@ [ ... ] > +static struct sk_buff *lan9645x_xmit_get_vlan_info(struct sk_buff *skb, > + struct net_device *br, > + u32 *vlan_tci, > + u32 *tag_type) > +{ [ ... ] > + hdr = skb_vlan_eth_hdr(skb); > + br_vlan_get_proto(br, &proto); > + > + if (skb_headlen(skb) >= VLAN_ETH_HLEN && > + ntohs(hdr->h_vlan_proto) == proto) { > + vlan_remove_tag(skb, &tci); [Severity: Low] What happens when the VLAN header is not fully in the linear area? Because of the skb_headlen() check, a tagged frame with a short linear head goes to the else branch. The original 802.1Q tag then stays in the payload, and IFH_TCI is set to br0's pvid. dsa_user_xmit() does not linearize the skb here. The user netdev inherits NETIF_F_SG from conduit->vlan_features in dsa_user_setup_tagger(). The AF_PACKET PACKET_TX_RING path, tpacket_snd()->tpacket_fill_skb(), copies only hard_header_len (14) bytes into the linear area. Could the rewriter then push a second tag, so the frame goes out double-tagged or in the wrong VLAN? Would a pskb_may_pull() of VLAN_ETH_HLEN before this check avoid that? [Severity: Low] Is the VLAN_ETH_HLEN check enough for vlan_remove_tag()? When h_vlan_encapsulated_proto is an 802.3 length, vlan_set_encap_proto() also reads the two bytes after the VLAN header: include/linux/if_vlan.h:vlan_set_encap_proto() { ... rawp = (unsigned short *)(vhdr + 1); if (*rawp == 0xFFFF) ... } lan9645x has no needed_tailroom, so dsa_user_xmit() does not pad the frame. skb_put_padto() in lan9645x_xmit() only runs after this function returns. Take an 18 or 19 byte frame such as DA SA 8100 TCI <len>. Can this read uninitialized tailroom past skb_tail_pointer()? The only effect is whether skb->protocol becomes ETH_P_802_3 or ETH_P_802_2, but KMSAN would likely report an uninit-value. > + *vlan_tci = tci; > + } else { > + rcu_read_lock(); > + br_vlan_get_pvid_rcu(br, &tci); > + rcu_read_unlock(); [Severity: High] Is br_vlan_get_pvid_rcu() being passed the right device here? br comes from dsa_port_bridge_dev_get(), so it is the bridge master. This call returns the pvid of br0's own VLAN group. That VID is unrelated to the egress port and to the VLAN the bridge forwarded the frame in, because br_handle_vlan() already cleared the tag for an egress-untagged VLAN. lan9645x_xmit() then writes that VID into IFH_TCI with IFH_BYPASS set, so the rewriter uses it as the classified VID: lan9645x_ifh_set(ifh, 1, IFH_BYPASS, IFH_BYPASS_SZ); ... lan9645x_ifh_set(ifh, vlan_tci, IFH_TCI, IFH_TCI_SZ); The later "net: dsa: lan9645x: add vlan support" patch changes lan9645x_vlan_port_apply_egress(). It programs a hybrid port (one untagged VLAN plus tagged VLANs) as LAN9645X_TAG_NO_PVID_NO_UNAWARE, with PORT_VID set to the untagged VID. In that mode every frame is tagged unless VID == PORT_VID or VID == 0. For example, say br0 has pvid 1 (the default), and swp1 has VLAN 10 as pvid/untagged plus VLAN 20 tagged. A frame sent by the host in VLAN 10 (from br0.10, or ARP flooded by the bridge) reaches this code untagged and gets VID 1. So does a frame forwarded in software in VLAN 10. Would the switch then send it on the wire tagged with VLAN 1 instead of untagged in VLAN 10? > + *vlan_tci = tci; > + } [ ... ] > +static struct sk_buff *lan9645x_xmit(struct sk_buff *skb, > + struct net_device *ndev) > +{ > + struct dsa_port *dp = dsa_user_to_port(ndev); > + u32 vlan_tci, tag_type; > + u32 qos_class; > + void *ifh; > + > + skb = lan9645x_xmit_get_vlan_info(skb, dsa_port_bridge_dev_get(dp), > + &vlan_tci, &tag_type); [Severity: High] This isn't a bug introduced by this patch. The racy pattern is in the DSA core and is already used by tag_ocelot, tag_sja1105, tag_dsa and others; this tagger adds another user of it. dsa_port_bridge_dev_get() reads dp->bridge twice on the TX path. It holds no lock and uses neither READ_ONCE() nor RCU: return dp->bridge ? dp->bridge->dev : NULL; The following path runs under rtnl: del_nbp() netdev_upper_dev_unlink() dsa_port_bridge_leave() dsa_port_bridge_destroy() It clears dp->bridge and, on the last reference, frees the dsa_bridge without waiting for a grace period: net/dsa/port.c:dsa_port_bridge_destroy() { ... dp->bridge = NULL; if (!refcount_dec_and_test(&bridge->refcount)) return; ... kfree(bridge); } TX on the user port (AF_PACKET or locally generated traffic) is not quiesced at that point. Can a concurrent lan9645x_xmit() read ->dev from freed memory, or reload dp->bridge after it became NULL? The result is then passed to br_vlan_enabled(), br_vlan_get_proto() and br_vlan_get_pvid_rcu(). [ ... ] > +static struct sk_buff *lan9645x_rcv(struct sk_buff *skb, > + struct net_device *ndev) > +{ [ ... ] > + dp = dsa_user_to_port(skb->dev); > + > + if (dsa_port_is_vlan_filtering(dp) && vlan_tci) { > + u16 port_pvid = 0; > + > + br_vlan_get_pvid_rcu(skb->dev, &port_pvid); > + > + /* The tag is restored as a C-tag, not as the TAG_TYPE the IFH > + * reports. The classifier recognizes both TPIDs as VLAN tags, > + * so an S-tag has already been used for classification by the > + * time we get here. Restoring it as 802.1AD would make the > + * bridge push it back into the payload and reclassify the frame > + * to the port pvid, on a different VID than the one the > + * hardware forwarded it on. > + */ > + if ((vlan_tci & VLAN_VID_MASK) != port_pvid) > + __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), > + vlan_tci); [Severity: Medium] What happens to a double-tagged frame whose outer VID equals the pvid? The later "net: dsa: lan9645x: add vlan support" patch sets ANA_VLAN_CFG_VLAN_POP_CNT=1 without VLAN_INNER_TAG_ENA, so VLAN-aware ports pop exactly one tag. Take outer VID 100 (the pvid) with an inner C-tag 200. The frame is extracted with popcnt == 3, the inner tag is still in the payload, and IFH_TCI reports VID 100. No tag is restored here in that case. eth_type_trans() in dsa_switch_rcv() then sets skb->protocol to 0x8100, and __netif_receive_skb_core() turns the inner tag into hwaccel VID 200 via skb_vlan_untag(). Would the software bridge then classify the frame to VLAN 200, while the hardware forwarded it in VLAN 100? [Severity: Low] The commit message says this assumes an 802.1Q bridge: This assumes a bridge vlan_protocol of 802.1Q, which is the only protocol the driver offloads. The later "net: dsa: lan9645x: add vlan support" patch documents the same limitation. Is anything enforcing it? DSA does not handle SWITCHDEV_ATTR_ID_BRIDGE_VLAN_PROTOCOL, and __br_vlan_set_proto() sends it with SWITCHDEV_F_SKIP_EOPNOTSUPP. So an 802.1ad bridge cannot be refused that way. On such a bridge, __allowed_ingress() sees skb->vlan_proto != br->vlan_proto. It pushes the tag back into the payload and classifies the frame to the pvid. Terminated frames on non-pvid VLANs would then land on the pvid, or be dropped if the port has no pvid. lan9645x_xmit_get_vlan_info(), on the other hand, handles 802.1ad explicitly: *tag_type = (proto != ETH_P_8021Q) ? LAN9645X_IFH_TAG_TYPE_S : LAN9645X_IFH_TAG_TYPE_C; Should 802.1ad bridges be refused in port_bridge_join? Should xmit and rcv also handle an 802.1ad bridge the same way? > + } > + > + return skb; > +} > + > +static const struct dsa_device_ops lan9645x_netdev_ops = { > + .name = LAN9645X_NAME, > + .proto = DSA_TAG_PROTO_LAN9645X, > + .xmit = lan9645x_xmit, > + .rcv = lan9645x_rcv, > + /* Covers the extraction prefix too, since dsa_tag_protocol_overhead() > + * sizes the conduit MTU from this. > + */ > + .needed_headroom = LAN9645X_TOTAL_TAG_LEN, > +}; [Severity: Medium] There is no .flow_dissect callback here, so __skb_flow_dissect() falls back to dsa_tag_generic_flow_dissect(). That helper assumes the tag overhead is exactly needed_headroom: include/net/dsa.h:dsa_tag_generic_flow_dissect() { ... *offset = tag_len; *proto = ((__be16 *)skb->data)[(tag_len / 2) - 1]; } Does that hold for this tagger? Whenever the rewriter popped tags, lan9645x_rcv() skips a 4 or 8 byte ifh_gap_len between the IFH and the real DMAC. The later "net: dsa: lan9645x: add vlan support" patch sets ANA_VLAN_CFG_VLAN_POP_CNT=1 on VLAN-aware ports. So every tagged frame received on those ports has a 4 byte gap. For those frames, would the generic dissector take SMAC or gap bytes as the EtherType and use the wrong network header offset? That would misdirect RPS and skb_get_hash() on the conduit. -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260929-dsa_lan9645x_switch_driver_base-v13-0-827c2d3617f2%40microchip.com ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH net-next v13 1/9] net: dsa: add tag driver for LAN9645X 2026-10-02 21:14 ` netdev-bot+sashiko @ 2026-10-06 7:19 ` Jens Emil Schulz Ostergaard 0 siblings, 0 replies; 29+ messages in thread From: Jens Emil Schulz Ostergaard @ 2026-10-06 7:19 UTC (permalink / raw) To: netdev-bot+sashiko Cc: UNGLinuxDriver, andrew, olteanv, davem, edumazet, kuba, pabeni, horms, robh, krzk+dt, conor+dt, woojung.huh, linux, Steen.Hegelund, daniel.machon, geert+renesas, magnus.damm, linux-kernel, netdev, devicetree, linux-renesas-soc On Fri, 2026-10-02 at 21:14 +0000, netdev-bot+sashiko@kernel.org wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Thank you for your contribution! Sashiko AI review found 6 potential > issue(s) to consider. > > Critical: 0 · High: 1 (1) · Medium: 2 · Low: 3 > > If you believe none of the issues are legitimate, please reply to each > issue of Medium severity or higher and explain why the AI is wrong. > If any of them are legitimate and you plan to respin, please let patchwork > know by including "pw-bot: cr" as a separate line at the end of your reply > (one such reply per series is enough). > > - [High] In lan9645x_xmit_get_vlan_info() (net/dsa/tag_lan9645x.c), any > frame with no in-band tag matching the bridge protocol gets IFH_TCI… > - [Medium] lan9645x_netdev_ops sets no .flow_dissect, so > __skb_flow_dissect() (net/core/flow_dissector.c:1083-1098) falls back… > - [Medium] When the IFH-classified VID equals the port pvid, > lan9645x_rcv() skips restoring the tag (tag_lan9645x.c:451-453). > - [Low] In lan9645x_xmit_get_vlan_info(), the condition `skb_headlen(skb) > >= VLAN_ETH_HLEN && ntohs(hdr->h_vlan_proto) == proto` treats a VLAN… > - [Low] In lan9645x_xmit_get_vlan_info(), the guard `skb_headlen(skb) >= > VLAN_ETH_HLEN` in front of vlan_remove_tag() does not cover every byte… > - [Low] lan9645x_rcv() always restores the classified VLAN as > ETH_P_8021Q. > > Pre-existing issues: > - [High] lan9645x_xmit() calls dsa_port_bridge_dev_get(dp) > (include/net/dsa.h:774-778, `dp->bridge ? > > -- > > net: dsa: add tag driver for LAN9645X > > This adds a DSA tag driver for the Microchip LAN9645x switch, which uses a > front port as the NPI CPU port. On extraction, frames carry a 16 byte long > prefix plus a 28 byte IFH. On injection, frames carry only the IFH. On > receive, the classified VLAN is restored as a hwaccel C-tag unless it > equals the port pvid. > > > diff --git a/net/dsa/tag_lan9645x.c b/net/dsa/tag_lan9645x.c > > new file mode 100644 > > index 0000000000000..f54646d4b3947 > > --- /dev/null > > +++ b/net/dsa/tag_lan9645x.c > > @@ -0,0 +1,474 @@ > > [ ... ] > > > +static struct sk_buff *lan9645x_xmit_get_vlan_info(struct sk_buff *skb, > > + struct net_device *br, > > + u32 *vlan_tci, > > + u32 *tag_type) > > +{ > > [ ... ] > > > + hdr = skb_vlan_eth_hdr(skb); > > + br_vlan_get_proto(br, &proto); > > + > > + if (skb_headlen(skb) >= VLAN_ETH_HLEN && > > + ntohs(hdr->h_vlan_proto) == proto) { > > + vlan_remove_tag(skb, &tci); > > [Severity: Low] > What happens when the VLAN header is not fully in the linear area? > > Because of the skb_headlen() check, a tagged frame with a short linear head > goes to the else branch. The original 802.1Q tag then stays in the payload, > and IFH_TCI is set to br0's pvid. > > dsa_user_xmit() does not linearize the skb here. The user netdev inherits > NETIF_F_SG from conduit->vlan_features in dsa_user_setup_tagger(). The > AF_PACKET PACKET_TX_RING path, tpacket_snd()->tpacket_fill_skb(), copies > only hard_header_len (14) bytes into the linear area. > > Could the rewriter then push a second tag, so the frame goes out > double-tagged or in the wrong VLAN? Would a pskb_may_pull() of > VLAN_ETH_HLEN before this check avoid that? > > [Severity: Low] > Is the VLAN_ETH_HLEN check enough for vlan_remove_tag()? > > When h_vlan_encapsulated_proto is an 802.3 length, vlan_set_encap_proto() > also reads the two bytes after the VLAN header: > > include/linux/if_vlan.h:vlan_set_encap_proto() { > ... > rawp = (unsigned short *)(vhdr + 1); > if (*rawp == 0xFFFF) > ... > } > > lan9645x has no needed_tailroom, so dsa_user_xmit() does not pad the frame. > skb_put_padto() in lan9645x_xmit() only runs after this function returns. > > Take an 18 or 19 byte frame such as DA SA 8100 TCI <len>. Can this read > uninitialized tailroom past skb_tail_pointer()? The only effect is whether > skb->protocol becomes ETH_P_802_3 or ETH_P_802_2, but KMSAN would likely > report an uninit-value. > > > + *vlan_tci = tci; > > + } else { > > + rcu_read_lock(); > > + br_vlan_get_pvid_rcu(br, &tci); > > + rcu_read_unlock(); > > [Severity: High] > Is br_vlan_get_pvid_rcu() being passed the right device here? > > br comes from dsa_port_bridge_dev_get(), so it is the bridge master. This > call returns the pvid of br0's own VLAN group. That VID is unrelated to the > egress port and to the VLAN the bridge forwarded the frame in, because > br_handle_vlan() already cleared the tag for an egress-untagged VLAN. > > lan9645x_xmit() then writes that VID into IFH_TCI with IFH_BYPASS set, so > the rewriter uses it as the classified VID: > > lan9645x_ifh_set(ifh, 1, IFH_BYPASS, IFH_BYPASS_SZ); > ... > lan9645x_ifh_set(ifh, vlan_tci, IFH_TCI, IFH_TCI_SZ); > > The later "net: dsa: lan9645x: add vlan support" patch changes > lan9645x_vlan_port_apply_egress(). It programs a hybrid port (one untagged > VLAN plus tagged VLANs) as LAN9645X_TAG_NO_PVID_NO_UNAWARE, with PORT_VID > set to the untagged VID. In that mode every frame is tagged unless > VID == PORT_VID or VID == 0. > > For example, say br0 has pvid 1 (the default), and swp1 has VLAN 10 as > pvid/untagged plus VLAN 20 tagged. A frame sent by the host in VLAN 10 (from > br0.10, or ARP flooded by the bridge) reaches this code untagged and gets > VID 1. So does a frame forwarded in software in VLAN 10. > > Would the switch then send it on the wire tagged with VLAN 1 instead of > untagged in VLAN 10? This is legitimate. I will fix this. > > > + *vlan_tci = tci; > > + } > > [ ... ] > > > +static struct sk_buff *lan9645x_xmit(struct sk_buff *skb, > > + struct net_device *ndev) > > +{ > > + struct dsa_port *dp = dsa_user_to_port(ndev); > > + u32 vlan_tci, tag_type; > > + u32 qos_class; > > + void *ifh; > > + > > + skb = lan9645x_xmit_get_vlan_info(skb, dsa_port_bridge_dev_get(dp), > > + &vlan_tci, &tag_type); > > [Severity: High] > This isn't a bug introduced by this patch. The racy pattern is in the DSA > core and is already used by tag_ocelot, tag_sja1105, tag_dsa and others; > this tagger adds another user of it. > > dsa_port_bridge_dev_get() reads dp->bridge twice on the TX path. It holds > no lock and uses neither READ_ONCE() nor RCU: > > return dp->bridge ? dp->bridge->dev : NULL; > > The following path runs under rtnl: > > del_nbp() > netdev_upper_dev_unlink() > dsa_port_bridge_leave() > dsa_port_bridge_destroy() > > It clears dp->bridge and, on the last reference, frees the dsa_bridge > without waiting for a grace period: > > net/dsa/port.c:dsa_port_bridge_destroy() { > ... > dp->bridge = NULL; > > if (!refcount_dec_and_test(&bridge->refcount)) > return; > ... > kfree(bridge); > } > > TX on the user port (AF_PACKET or locally generated traffic) is not > quiesced at that point. > > Can a concurrent lan9645x_xmit() read ->dev from freed memory, or reload > dp->bridge after it became NULL? The result is then passed to > br_vlan_enabled(), br_vlan_get_proto() and br_vlan_get_pvid_rcu(). > > [ ... ] > Dismissed. This is preexisting and a DSA core pattern shared with tag_ocelot, tag_dsa, tag_sja1105, tag_vsc73xx_8021q and dsa_software_vlan_untag(). dsa_bridge is freed in dsa_port_bridge_destroy() without a grace period, so no tagger can fix it locally. > > +static struct sk_buff *lan9645x_rcv(struct sk_buff *skb, > > + struct net_device *ndev) > > +{ > > [ ... ] > > > + dp = dsa_user_to_port(skb->dev); > > + > > + if (dsa_port_is_vlan_filtering(dp) && vlan_tci) { > > + u16 port_pvid = 0; > > + > > + br_vlan_get_pvid_rcu(skb->dev, &port_pvid); > > + > > + /* The tag is restored as a C-tag, not as the TAG_TYPE the IFH > > + * reports. The classifier recognizes both TPIDs as VLAN tags, > > + * so an S-tag has already been used for classification by the > > + * time we get here. Restoring it as 802.1AD would make the > > + * bridge push it back into the payload and reclassify the frame > > + * to the port pvid, on a different VID than the one the > > + * hardware forwarded it on. > > + */ > > + if ((vlan_tci & VLAN_VID_MASK) != port_pvid) > > + __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), > > + vlan_tci); > > [Severity: Medium] > What happens to a double-tagged frame whose outer VID equals the pvid? > > The later "net: dsa: lan9645x: add vlan support" patch sets > ANA_VLAN_CFG_VLAN_POP_CNT=1 without VLAN_INNER_TAG_ENA, so VLAN-aware ports > pop exactly one tag. > > Take outer VID 100 (the pvid) with an inner C-tag 200. The frame is > extracted with popcnt == 3, the inner tag is still in the payload, and > IFH_TCI reports VID 100. > > No tag is restored here in that case. eth_type_trans() in dsa_switch_rcv() > then sets skb->protocol to 0x8100, and __netif_receive_skb_core() turns the > inner tag into hwaccel VID 200 via skb_vlan_untag(). > > Would the software bridge then classify the frame to VLAN 200, while the > hardware forwarded it in VLAN 100? > Correct. The pvid comparison in lan9645x_rcv() assumed a frame tagged with the pvid cannot be told apart from an untagged one, but the IFH reports whether the rewriter popped a tag. v14 restores the classified VID as a hwaccel tag whenever a tag was popped. > [Severity: Low] > The commit message says this assumes an 802.1Q bridge: > > This assumes a bridge vlan_protocol of 802.1Q, which is the only > protocol the driver offloads. > > The later "net: dsa: lan9645x: add vlan support" patch documents the same > limitation. Is anything enforcing it? > > DSA does not handle SWITCHDEV_ATTR_ID_BRIDGE_VLAN_PROTOCOL, and > __br_vlan_set_proto() sends it with SWITCHDEV_F_SKIP_EOPNOTSUPP. So an > 802.1ad bridge cannot be refused that way. > > On such a bridge, __allowed_ingress() sees skb->vlan_proto != > br->vlan_proto. It pushes the tag back into the payload and classifies the > frame to the pvid. Terminated frames on non-pvid VLANs would then land on > the pvid, or be dropped if the port has no pvid. > > lan9645x_xmit_get_vlan_info(), on the other hand, handles 802.1ad > explicitly: > > *tag_type = (proto != ETH_P_8021Q) ? LAN9645X_IFH_TAG_TYPE_S : > LAN9645X_IFH_TAG_TYPE_C; > > Should 802.1ad bridges be refused in port_bridge_join? Should xmit and rcv > also handle an 802.1ad bridge the same way? > > > + } > > + > > + return skb; > > +} > > + > > +static const struct dsa_device_ops lan9645x_netdev_ops = { > > + .name = LAN9645X_NAME, > > + .proto = DSA_TAG_PROTO_LAN9645X, > > + .xmit = lan9645x_xmit, > > + .rcv = lan9645x_rcv, > > + /* Covers the extraction prefix too, since dsa_tag_protocol_overhead() > > + * sizes the conduit MTU from this. > > + */ > > + .needed_headroom = LAN9645X_TOTAL_TAG_LEN, > > +}; > > [Severity: Medium] > There is no .flow_dissect callback here, so __skb_flow_dissect() falls back > to dsa_tag_generic_flow_dissect(). That helper assumes the tag overhead is > exactly needed_headroom: > > include/net/dsa.h:dsa_tag_generic_flow_dissect() { > ... > *offset = tag_len; > *proto = ((__be16 *)skb->data)[(tag_len / 2) - 1]; > } > > Does that hold for this tagger? Whenever the rewriter popped tags, > lan9645x_rcv() skips a 4 or 8 byte ifh_gap_len between the IFH and the > real DMAC. > > The later "net: dsa: lan9645x: add vlan support" patch sets > ANA_VLAN_CFG_VLAN_POP_CNT=1 on VLAN-aware ports. So every tagged frame > received on those ports has a 4 byte gap. > > For those frames, would the generic dissector take SMAC or gap bytes as the > EtherType and use the wrong network header offset? That would misdirect RPS > and skb_get_hash() on the conduit. > Correct, the generic dissector takes needed_headroom as the rx tag length, which only holds when the rewriter did not pop a tag on extraction. v14 adds a .flow_dissect callback. > -- > Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260929-dsa_lan9645x_switch_driver_base-v13-0-827c2d3617f2%40microchip.com pw-bot: cr ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH net-next v13 2/9] dt-bindings: net: lan9645x: add LAN9645X switch bindings 2026-09-29 7:48 [PATCH net-next v13 0/9] net: dsa: add DSA support for the LAN9645x switch chip family Jens Emil Schulz Østergaard 2026-09-29 7:48 ` [PATCH net-next v13 1/9] net: dsa: add tag driver for LAN9645X Jens Emil Schulz Østergaard @ 2026-09-29 7:48 ` Jens Emil Schulz Østergaard 2026-09-30 7:50 ` sashiko-bot ` (2 more replies) 2026-09-29 7:48 ` [PATCH net-next v13 3/9] net: dsa: lan9645x: add autogenerated register macros Jens Emil Schulz Østergaard ` (6 subsequent siblings) 8 siblings, 3 replies; 29+ messages in thread From: Jens Emil Schulz Østergaard @ 2026-09-29 7:48 UTC (permalink / raw) To: UNGLinuxDriver, Andrew Lunn, Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Woojung Huh, Russell King, Steen Hegelund, Daniel Machon, Geert Uytterhoeven, Magnus Damm Cc: linux-kernel, netdev, devicetree, linux-renesas-soc, Jens Emil Schulz Østergaard Add bindings for LAN9645X switch. We use a fallback compatible for the smallest SKU microchip,lan96455s-switch. The last digit of the part number is the number of usable front ports and the letter is the feature set. The switch is a child of an externally controlled parent that hands out one regmap per register target, so reg and reg-names describe the target layout the way mscc,vsc7514-switch.yaml does for the equivalent mscc,vsc7512-switch node. That binding, and the mfd parent it hangs off in mscc,ocelot.yaml, is the model this driver and its parent follow. Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com> Signed-off-by: Jens Emil Schulz Østergaard <jensemil.schulzostergaard@microchip.com> --- Changes in v13: - Remove section quoting Documentation/networking/phy.rst from commit message. - Remove rx/tx-internal-delay-ps from example. Changes in v12: - Remove Rob's Reviewed-by - Match both the port@ and ethernet-port@ spellings, so the per port constraints apply. dsa.yaml accepts either, and the local pattern only matched the long form. - Allow 0 as well as 2000 for rx/tx-internal-delay-ps, matching what the driver accepts, and restrict them to those two values on RGMII ports. - Constrain the ethernet-ports node the way renesas,rzn1-a5psw.yaml does, with additionalProperties instead of a local dsa-port.yaml $ref. - Put the example properties in canonical order, so dt-check-style --mode=strict is clean. - Describe the switch register targets in reg, with a matching reg-names, following mscc,vsc7514-switch.yaml. - Document the default of rx/tx-internal-delay-ps, as microchip,lan937x.yaml does. - Describe the SKU naming in the binding description. Changes in v5: - No changes. Changes in v4: - No changes. Changes in v3: - remove additionalProperties: true - remove unnecessary | from description - change top level $ref to dsa.yaml#/$defs/ethernet-ports - use ethernet-ports and ethernet-port - move ethernet-ports under properties instead of patternProperties - move unevaluatedProperties: false after $ref - update example to use ethernet-ports and ethernet-port Changes in v2: - rename file to microchip,lan96455s-switch.yaml - remove led vendor property - add {rx,tx}-internal-delay-ps for rgmii delay - remove labels from example - remove container node from example --- .../net/dsa/microchip,lan96455s-switch.yaml | 177 +++++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 178 insertions(+) diff --git a/Documentation/devicetree/bindings/net/dsa/microchip,lan96455s-switch.yaml b/Documentation/devicetree/bindings/net/dsa/microchip,lan96455s-switch.yaml new file mode 100644 index 000000000000..9deb7a427804 --- /dev/null +++ b/Documentation/devicetree/bindings/net/dsa/microchip,lan96455s-switch.yaml @@ -0,0 +1,177 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/net/dsa/microchip,lan96455s-switch.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Microchip LAN9645x Ethernet switch + +maintainers: + - Jens Emil Schulz Østergaard <jensemil.schulzostergaard@microchip.com> + +description: | + The LAN9645x switch is a multi-port Gigabit AVB/TSN Ethernet switch with + five integrated 10/100/1000Base-T PHYs. In addition to the integrated PHYs, + it supports up to 2 RGMII/RMII, up to 2 BASE-X/SERDES/2.5GBASE-X and one + Quad-SGMII interfaces. + + The part number encodes the SKU. The last digit is the number of usable + front ports, 5, 7 or 9. The letter is the feature set, f for full and s for + standard, where full adds HSR, PRP, TAS, frame preemption and PSFP. + +properties: + compatible: + oneOf: + - enum: + - microchip,lan96455s-switch + - items: + - enum: + - microchip,lan96455f-switch + - microchip,lan96457f-switch + - microchip,lan96459f-switch + - microchip,lan96457s-switch + - microchip,lan96459s-switch + - const: microchip,lan96455s-switch + + reg: + items: + - description: General configuration block target + - description: + CPU device queue system target, the register based frame injection + and extraction interface + - description: Chip top level target + - description: Rewriter target + - description: Switching engine target + - description: HSIO target + - description: Port 0 device target + - description: Port 1 device target + - description: Port 2 device target + - description: Port 3 device target + - description: Port 4 device target + - description: Port 5 device target + - description: Port 6 device target + - description: Port 7 device target + - description: Port 8 device target + - description: Queue system target + - description: Analyzer target + + reg-names: + items: + - const: gcb + - const: qs + - const: chip_top + - const: rew + - const: sys + - const: hsio + - const: dev0 + - const: dev1 + - const: dev2 + - const: dev3 + - const: dev4 + - const: dev5 + - const: dev6 + - const: dev7 + - const: dev8 + - const: qsys + - const: ana + + ethernet-ports: + type: object + additionalProperties: true + patternProperties: + "^(ethernet-)?port@[0-8]$": + type: object + description: Ethernet switch ports + additionalProperties: true + + allOf: + - if: + properties: + phy-mode: + contains: + enum: + - rgmii + - rgmii-rxid + - rgmii-txid + - rgmii-id + then: + properties: + rx-internal-delay-ps: + $ref: "#/$defs/internal-delay-ps" + tx-internal-delay-ps: + $ref: "#/$defs/internal-delay-ps" + +$ref: dsa.yaml#/$defs/ethernet-ports + +required: + - compatible + - reg + - reg-names + - ethernet-ports + +unevaluatedProperties: false + +$defs: + internal-delay-ps: + description: + Disable the delay line using 0 ps, or enable the 2000 ps delay. The + delay line is not tunable, so no other phase can be selected. + enum: [0, 2000] + default: 0 + +examples: + - | + ethernet-switch@4000 { + compatible = "microchip,lan96459f-switch", "microchip,lan96455s-switch"; + reg = <0x4000 0x244>, <0x8000 0x4c>, <0x10000 0x134>, + <0x18000 0x1910>, <0x2c000 0x12d4>, <0x30000 0xe4>, + <0x38000 0xb8>, <0x3c000 0xb8>, <0x40000 0xb8>, + <0x44000 0xb8>, <0x48000 0xb8>, <0x4c000 0xb8>, + <0x50000 0xb8>, <0x54000 0xb8>, <0x58000 0xb8>, + <0x100000 0xa000>, <0x140000 0x7468>; + reg-names = "gcb", "qs", "chip_top", "rew", "sys", "hsio", "dev0", + "dev1", "dev2", "dev3", "dev4", "dev5", "dev6", "dev7", + "dev8", "qsys", "ana"; + + ethernet-ports { + #address-cells = <1>; + #size-cells = <0>; + + ethernet-port@0 { + reg = <0>; + phy-handle = <&cuphy0>; + phy-mode = "gmii"; + }; + + ethernet-port@1 { + reg = <1>; + phy-handle = <&cuphy1>; + phy-mode = "gmii"; + }; + + ethernet-port@2 { + reg = <2>; + phy-handle = <&cuphy2>; + phy-mode = "gmii"; + }; + + ethernet-port@3 { + reg = <3>; + phy-handle = <&cuphy3>; + phy-mode = "gmii"; + }; + + ethernet-port@7 { + reg = <7>; + ethernet = <&cpu_host_port>; + phy-mode = "rgmii"; + + fixed-link { + full-duplex; + pause; + speed = <1000>; + }; + }; + }; + }; +... diff --git a/MAINTAINERS b/MAINTAINERS index e2cbcf89d36e..d9632ba3419a 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -17805,6 +17805,7 @@ M: Jens Emil Schulz Østergaard <jensemil.schulzostergaard@microchip.com> M: UNGLinuxDriver@microchip.com L: netdev@vger.kernel.org S: Maintained +F: Documentation/devicetree/bindings/net/dsa/microchip,lan96455s-switch.yaml F: include/linux/dsa/lan9645x.h F: net/dsa/tag_lan9645x.c -- 2.52.0 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH net-next v13 2/9] dt-bindings: net: lan9645x: add LAN9645X switch bindings 2026-09-29 7:48 ` [PATCH net-next v13 2/9] dt-bindings: net: lan9645x: add LAN9645X switch bindings Jens Emil Schulz Østergaard @ 2026-09-30 7:50 ` sashiko-bot 2026-10-02 21:14 ` netdev-bot+sashiko 2026-10-06 3:19 ` Rob Herring (Arm) 2 siblings, 0 replies; 29+ messages in thread From: sashiko-bot @ 2026-09-30 7:50 UTC (permalink / raw) To: Jens Emil Schulz Østergaard; +Cc: robh, wsa+renesas, conor+dt, devicetree Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] dt-bindings: strict property validation disabled by additionalProperties: true -- commit 37a55f2415392987ad4b328233f6c32282b21103 Author: Jens Emil Schulz Østergaard <jensemil.schulzostergaard@microchip.com> dt-bindings: net: lan9645x: add LAN9645X switch bindings This commit adds bindings for the LAN9645X switch, which uses a fallback compatible for the smallest SKU microchip,lan96455s-switch. The switch is a child of an externally controlled parent that hands out one regmap per register target. > diff --git a/Documentation/devicetree/bindings/net/dsa/microchip,lan96455s-switch.yaml b/Documentation/devicetree/bindings/net/dsa/microchip,lan96455s-switch.yaml > new file mode 100644 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/dsa/microchip,lan96455s-switch.yaml [ ... ] > + ethernet-ports: > + type: object > + additionalProperties: true > + patternProperties: > + "^(ethernet-)?port@[0-8]$": > + type: object > + description: Ethernet switch ports > + additionalProperties: true > + > + allOf: > + - if: > + properties: > + phy-mode: [Severity: Low] Does setting additionalProperties: true here introduce a validation regression by unintentionally defeating the unevaluatedProperties: false strictness checks inherited from dsa.yaml? Because additionalProperties: true causes dt-schema to mark all unknown properties as evaluated, it allows any undefined or misspelled properties inside the child port nodes to silently pass validation. Could this also cause a regression in the conditional if-then logic below? If restricted properties like rx-internal-delay-ps are specified on non-RGMII ports, the if condition fails and the constraint is skipped, but instead of being rejected, the property is silently allowed unrestricted by additionalProperties: true. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260929-dsa_lan9645x_switch_driver_base-v13-0-827c2d3617f2@microchip.com?part=2 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH net-next v13 2/9] dt-bindings: net: lan9645x: add LAN9645X switch bindings 2026-09-29 7:48 ` [PATCH net-next v13 2/9] dt-bindings: net: lan9645x: add LAN9645X switch bindings Jens Emil Schulz Østergaard 2026-09-30 7:50 ` sashiko-bot @ 2026-10-02 21:14 ` netdev-bot+sashiko 2026-10-07 14:34 ` Jens Emil Schulz Ostergaard 2026-10-06 3:19 ` Rob Herring (Arm) 2 siblings, 1 reply; 29+ messages in thread From: netdev-bot+sashiko @ 2026-10-02 21:14 UTC (permalink / raw) To: jensemil.schulzostergaard Cc: UNGLinuxDriver, andrew, olteanv, davem, edumazet, kuba, pabeni, horms, robh, krzk+dt, conor+dt, woojung.huh, linux, Steen.Hegelund, daniel.machon, geert+renesas, magnus.damm, linux-kernel, netdev, devicetree, linux-renesas-soc Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider. Critical: 0 · High: 0 · Medium: 1 · Low: 2 If you believe none of the issues are legitimate, please reply to each issue of Medium severity or higher and explain why the AI is wrong. If any of them are legitimate and you plan to respin, please let patchwork know by including "pw-bot: cr" as a separate line at the end of your reply (one such reply per series is enough). - [Medium] The binding makes rx/tx-internal-delay-ps on the MAC-side switch port nodes an on/off switch for the full 2 ns RGMII delay… - [Low] The per-port pattern "^(ethernet-)?port@[0-8]$" in properties/ethernet-ports does not actually limit ports to the 9… - [Low] The subject "dt-bindings: net: lan9645x: add LAN9645X switch bindings" does not follow… -- dt-bindings: net: lan9645x: add LAN9645X switch bindings This commit adds a DT binding for the Microchip LAN9645x DSA switch. The smallest SKU, microchip,lan96455s-switch, is the fallback compatible. The switch register targets are described through reg and reg-names, following mscc,vsc7514-switch.yaml. > dt-bindings: net: lan9645x: add LAN9645X switch bindings [Severity: Low] This isn't a bug, but the subject doesn't match what Documentation/devicetree/bindings/submitting-patches.rst asks for. That file says "Repeating "binding" again should also be avoided". Its example form is "dt-bindings: iio: adc: Add ROHM BD79100G". The file lives under net/dsa/, so the usual prefix is "dt-bindings: net: dsa:". Something like "dt-bindings: net: dsa: Add Microchip LAN9645x switch" would fit better. > diff --git a/Documentation/devicetree/bindings/net/dsa/microchip,lan96455s-switch.yaml b/Documentation/devicetree/bindings/net/dsa/microchip,lan96455s-switch.yaml > new file mode 100644 > index 0000000000000..9deb7a427804f > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/dsa/microchip,lan96455s-switch.yaml [ ... ] > + ethernet-ports: > + type: object > + additionalProperties: true > + patternProperties: > + "^(ethernet-)?port@[0-8]$": [Severity: Low] Does this pattern actually restrict port nodes to the 9 hardware ports (dev0..dev8)? ethernet-ports has additionalProperties: true, so nodes that don't match "^(ethernet-)?port@[0-8]$" are not rejected here. The inherited dsa.yaml#/$defs/ethernet-ports and ethernet-switch.yaml both accept any "^(ethernet-)?port@[0-9a-f]+$" node through dsa-port.yaml. So a node like ethernet-port@9 { reg = <9>; ... } or ethernet-port@a passes dt_binding_check and dtbs_check. It also skips the local RGMII delay constraints. The problem only shows up at probe time: net/dsa/dsa.c:dsa_switch_parse_ports_of() { ... if (reg >= ds->num_ports) { dev_err(ds->dev, "port %pOF index %u exceeds num_ports (%u)\n", port, reg, ds->num_ports); ... } Should the schema reject port nodes outside 0-8? > + type: object > + description: Ethernet switch ports > + additionalProperties: true > + > + allOf: > + - if: > + properties: > + phy-mode: > + contains: > + enum: > + - rgmii > + - rgmii-rxid > + - rgmii-txid > + - rgmii-id > + then: > + properties: > + rx-internal-delay-ps: > + $ref: "#/$defs/internal-delay-ps" > + tx-internal-delay-ps: > + $ref: "#/$defs/internal-delay-ps" [ ... ] > +$defs: > + internal-delay-ps: > + description: > + Disable the delay line using 0 ps, or enable the 2000 ps delay. The > + delay line is not tunable, so no other phase can be selected. > + enum: [0, 2000] > + default: 0 [Severity: Medium] Is it intended that rx/tx-internal-delay-ps on the MAC side port nodes turns the full 2 ns RGMII delay on and off, for all four rgmii phy-modes? Documentation/devicetree/bindings/net/ethernet-controller.yaml says these MAC node properties "should be used to indicate fine tuning performed by the MAC. The values expected here are small. A value of 2000ps, i.e 2ns, and a phy-mode of 'rgmii' will not be accepted by Reviewers." The same document says that when the MAC adds the delay, it "must modify the phy-mode it passes to the PHY, removing the delay it has added. Failure to remove the delay will result in a non-functioning link." This schema accepts phy-mode = "rgmii-id" together with rx-internal-delay-ps = <2000>. In the driver later in this series, lan9645x_parse_mac_delay() in lan9645x_port.c only checks for 0 or 2000. lan9645x_rgmii_dll_config() in lan9645x_phylink.c then enables the delay line directly from the parsed value: drivers/net/dsa/microchip/lan9645x/lan9645x_phylink.c:lan9645x_rgmii_dll_config() { ... HSIO_DLL_CFG_DLL_ENA_SET(p->rx_internal_delay) | HSIO_DLL_CFG_DELAY_ENA_SET(p->rx_internal_delay), ... } Nothing there checks phy-mode or removes the delay from the interface that phylink passes to the PHY. Take port 7 or 8 with a phy-handle, phy-mode "rgmii-id" and rx-internal-delay-ps = <2000>. Wouldn't the MAC and the PHY each add 2 ns, leaving a DT that passes validation with a link that doesn't work? The example also uses phy-mode = "rgmii" on the CPU port. For any RGMII mode other than 'rgmii-id', ethernet-controller.yaml says it "is probably wrong" unless justified. > + > +examples: [ ... ] > + ethernet-port@7 { > + reg = <7>; > + ethernet = <&cpu_host_port>; > + phy-mode = "rgmii"; [ ... ] -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260929-dsa_lan9645x_switch_driver_base-v13-0-827c2d3617f2%40microchip.com ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH net-next v13 2/9] dt-bindings: net: lan9645x: add LAN9645X switch bindings 2026-10-02 21:14 ` netdev-bot+sashiko @ 2026-10-07 14:34 ` Jens Emil Schulz Ostergaard 0 siblings, 0 replies; 29+ messages in thread From: Jens Emil Schulz Ostergaard @ 2026-10-07 14:34 UTC (permalink / raw) To: netdev-bot+sashiko Cc: UNGLinuxDriver, andrew, olteanv, davem, edumazet, kuba, pabeni, horms, robh, krzk+dt, conor+dt, woojung.huh, linux, Steen.Hegelund, daniel.machon, geert+renesas, magnus.damm, linux-kernel, netdev, devicetree, linux-renesas-soc On Fri, 2026-10-02 at 21:14 +0000, netdev-bot+sashiko@kernel.org wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Thank you for your contribution! Sashiko AI review found 3 potential > issue(s) to consider. > > Critical: 0 · High: 0 · Medium: 1 · Low: 2 > > If you believe none of the issues are legitimate, please reply to each > issue of Medium severity or higher and explain why the AI is wrong. > If any of them are legitimate and you plan to respin, please let patchwork > know by including "pw-bot: cr" as a separate line at the end of your reply > (one such reply per series is enough). > > - [Medium] The binding makes rx/tx-internal-delay-ps on the MAC-side > switch port nodes an on/off switch for the full 2 ns RGMII delay… > - [Low] The per-port pattern "^(ethernet-)?port@[0-8]$" in > properties/ethernet-ports does not actually limit ports to the 9… > - [Low] The subject "dt-bindings: net: lan9645x: add LAN9645X switch > bindings" does not follow… > > -- > > dt-bindings: net: lan9645x: add LAN9645X switch bindings > > This commit adds a DT binding for the Microchip LAN9645x DSA switch. The > smallest SKU, microchip,lan96455s-switch, is the fallback compatible. The > switch register targets are described through reg and reg-names, following > mscc,vsc7514-switch.yaml. > > > dt-bindings: net: lan9645x: add LAN9645X switch bindings > > [Severity: Low] > This isn't a bug, but the subject doesn't match what > Documentation/devicetree/bindings/submitting-patches.rst asks for. That file > says "Repeating "binding" again should also be avoided". Its example form is > "dt-bindings: iio: adc: Add ROHM BD79100G". > > The file lives under net/dsa/, so the usual prefix is "dt-bindings: net: dsa:". > Something like "dt-bindings: net: dsa: Add Microchip LAN9645x switch" would > fit better. > I will retitle. > > diff --git a/Documentation/devicetree/bindings/net/dsa/microchip,lan96455s-switch.yaml b/Documentation/devicetree/bindings/net/dsa/microchip,lan96455s-switch.yaml > > new file mode 100644 > > index 0000000000000..9deb7a427804f > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/net/dsa/microchip,lan96455s-switch.yaml > > [ ... ] > > > + ethernet-ports: > > + type: object > > + additionalProperties: true > > + patternProperties: > > + "^(ethernet-)?port@[0-8]$": > > [Severity: Low] > Does this pattern actually restrict port nodes to the 9 hardware ports > (dev0..dev8)? > > ethernet-ports has additionalProperties: true, so nodes that don't match > "^(ethernet-)?port@[0-8]$" are not rejected here. The inherited > dsa.yaml#/$defs/ethernet-ports and ethernet-switch.yaml both accept any > "^(ethernet-)?port@[0-9a-f]+$" node through dsa-port.yaml. > > So a node like ethernet-port@9 { reg = <9>; ... } or ethernet-port@a passes > dt_binding_check and dtbs_check. It also skips the local RGMII delay > constraints. The problem only shows up at probe time: > > net/dsa/dsa.c:dsa_switch_parse_ports_of() { > ... > if (reg >= ds->num_ports) { > dev_err(ds->dev, "port %pOF index %u exceeds num_ports (%u)\n", > port, reg, ds->num_ports); > ... > } > > Should the schema reject port nodes outside 0-8? > > > + type: object > > + description: Ethernet switch ports > > + additionalProperties: true > > + > > + allOf: > > + - if: > > + properties: > > + phy-mode: > > + contains: > > + enum: > > + - rgmii > > + - rgmii-rxid > > + - rgmii-txid > > + - rgmii-id > > + then: > > + properties: > > + rx-internal-delay-ps: > > + $ref: "#/$defs/internal-delay-ps" > > + tx-internal-delay-ps: > > + $ref: "#/$defs/internal-delay-ps" > > [ ... ] > > > +$defs: > > + internal-delay-ps: > > + description: > > + Disable the delay line using 0 ps, or enable the 2000 ps delay. The > > + delay line is not tunable, so no other phase can be selected. > > + enum: [0, 2000] > > + default: 0 > > [Severity: Medium] > Is it intended that rx/tx-internal-delay-ps on the MAC side port nodes turns > the full 2 ns RGMII delay on and off, for all four rgmii phy-modes? > > Documentation/devicetree/bindings/net/ethernet-controller.yaml says these > MAC node properties "should be used to indicate fine tuning performed by the > MAC. The values expected here are small. A value of 2000ps, i.e 2ns, and a > phy-mode of 'rgmii' will not be accepted by Reviewers." > > The same document says that when the MAC adds the delay, it "must modify the > phy-mode it passes to the PHY, removing the delay it has added. Failure to > remove the delay will result in a non-functioning link." > > This schema accepts phy-mode = "rgmii-id" together with > rx-internal-delay-ps = <2000>. In the driver later in this series, > lan9645x_parse_mac_delay() in lan9645x_port.c only checks for 0 or 2000. > lan9645x_rgmii_dll_config() in lan9645x_phylink.c then enables the delay line > directly from the parsed value: > > drivers/net/dsa/microchip/lan9645x/lan9645x_phylink.c:lan9645x_rgmii_dll_config() { > ... > HSIO_DLL_CFG_DLL_ENA_SET(p->rx_internal_delay) | > HSIO_DLL_CFG_DELAY_ENA_SET(p->rx_internal_delay), > ... > } > > Nothing there checks phy-mode or removes the delay from the interface that > phylink passes to the PHY. > > Take port 7 or 8 with a phy-handle, phy-mode "rgmii-id" and > rx-internal-delay-ps = <2000>. Wouldn't the MAC and the PHY each add 2 ns, > leaving a DT that passes validation with a link that doesn't work? > > The example also uses phy-mode = "rgmii" on the CPU port. For any RGMII mode > other than 'rgmii-id', ethernet-controller.yaml says it "is probably wrong" > unless justified. > Dismissed. This follows the binding model introduced for sja1105 in commit ac41ac81e331 ("dt-bindings: net: dsa: sja1105: add {rx,tx}-internal-delay-ps") and used by microchip,lan937x.yaml since commit 8926d94e5c50, which also restricts the values to 0 and 2000 ps. The MAC delay lines are configured only from rx/tx-internal-delay-ps, independently of phy-mode and of whether the port uses a PHY or a fixed-link, and phy-mode is passed unmodified to the PHY. The hardware has a fixed 2 ns delay line, 0 and 2000 are the only values it can express. For the CPU port, phy-mode "rgmii" with a fixed-link matches the sja1105 and lan937x examples. > > + > > +examples: > > [ ... ] > > > + ethernet-port@7 { > > + reg = <7>; > > + ethernet = <&cpu_host_port>; > > + phy-mode = "rgmii"; > > [ ... ] > > -- > Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260929-dsa_lan9645x_switch_driver_base-v13-0-827c2d3617f2%40microchip.com ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH net-next v13 2/9] dt-bindings: net: lan9645x: add LAN9645X switch bindings 2026-09-29 7:48 ` [PATCH net-next v13 2/9] dt-bindings: net: lan9645x: add LAN9645X switch bindings Jens Emil Schulz Østergaard 2026-09-30 7:50 ` sashiko-bot 2026-10-02 21:14 ` netdev-bot+sashiko @ 2026-10-06 3:19 ` Rob Herring (Arm) 2 siblings, 0 replies; 29+ messages in thread From: Rob Herring (Arm) @ 2026-10-06 3:19 UTC (permalink / raw) To: Jens Emil Schulz Østergaard Cc: David S. Miller, Russell King, Andrew Lunn, Steen Hegelund, netdev, UNGLinuxDriver, Vladimir Oltean, devicetree, Magnus Damm, Daniel Machon, Jakub Kicinski, Eric Dumazet, Geert Uytterhoeven, linux-renesas-soc, Paolo Abeni, Krzysztof Kozlowski, linux-kernel, Conor Dooley, Woojung Huh, Simon Horman On Tue, 29 Sep 2026 09:48:23 +0200, Jens Emil Schulz Østergaard wrote: > Add bindings for LAN9645X switch. We use a fallback compatible for the > smallest SKU microchip,lan96455s-switch. The last digit of the part number > is the number of usable front ports and the letter is the feature set. > > The switch is a child of an externally controlled parent that hands out > one regmap per register target, so reg and reg-names describe the target > layout the way mscc,vsc7514-switch.yaml does for the equivalent > mscc,vsc7512-switch node. That binding, and the mfd parent it hangs off > in mscc,ocelot.yaml, is the model this driver and its parent follow. > > Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com> > Signed-off-by: Jens Emil Schulz Østergaard <jensemil.schulzostergaard@microchip.com> > --- > Changes in v13: > - Remove section quoting Documentation/networking/phy.rst from commit > message. > - Remove rx/tx-internal-delay-ps from example. > > Changes in v12: > - Remove Rob's Reviewed-by > - Match both the port@ and ethernet-port@ spellings, so the per port > constraints apply. dsa.yaml accepts either, and the local pattern only > matched the long form. > - Allow 0 as well as 2000 for rx/tx-internal-delay-ps, matching what the > driver accepts, and restrict them to those two values on RGMII ports. > - Constrain the ethernet-ports node the way renesas,rzn1-a5psw.yaml does, > with additionalProperties instead of a local dsa-port.yaml $ref. > - Put the example properties in canonical order, so dt-check-style > --mode=strict is clean. > - Describe the switch register targets in reg, with a matching reg-names, > following mscc,vsc7514-switch.yaml. > - Document the default of rx/tx-internal-delay-ps, as > microchip,lan937x.yaml does. > - Describe the SKU naming in the binding description. > > Changes in v5: > - No changes. > > Changes in v4: > - No changes. > > Changes in v3: > - remove additionalProperties: true > - remove unnecessary | from description > - change top level $ref to dsa.yaml#/$defs/ethernet-ports > - use ethernet-ports and ethernet-port > - move ethernet-ports under properties instead of patternProperties > - move unevaluatedProperties: false after $ref > - update example to use ethernet-ports and ethernet-port > > Changes in v2: > - rename file to microchip,lan96455s-switch.yaml > - remove led vendor property > - add {rx,tx}-internal-delay-ps for rgmii delay > - remove labels from example > - remove container node from example > --- > .../net/dsa/microchip,lan96455s-switch.yaml |