Re: [PATCH v2 net-next 6/7] dt-bindings: net: dsa: mediatek,mt7530: add econet,en751221

[Rob Herring <robh@kernel.org>]

On Thu, Sep 17, 2026 at 08:50:00PM +0000, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 7 potential
> issue(s) to consider:
> 
> Critical: 0 · High: 1 · Medium: 4 · Low: 2
> 
> - [High] net: dsa: lockless child_bus access causes MDIO race condition
> - [Medium] The new switch-device compatible `econet,en751221`
>   (mediatek,mt7530.yaml line 126) is the identical string already…
> - [Medium] The new `econet,en751221` conditional (mediatek,mt7530.yaml
>   lines 369-377) applies only `$ref: "#/$defs/mt7530-passthrough-dsa-…
> - [Medium] The series patch "net: dsa: mediatek: support MDIO switch
>   downstream of MMIO switch" moved `mt7530_setup_mdio_irq()` out of…
> - [Medium] In passthrough mode `mt753x_cpu_port_enable()` (added by the
>   series patch "net: dsa: mediatek: add support for passthrough mode")…
> - [Low] The paragraph added to the binding description
>   (mediatek,mt7530.yaml lines 32-36) states that "EcoNet EN751221 SoCs…
> - [Low] Concern that neither `econet,en751221-switch` nor
>   `econet,en751221` has any consumer: at this commit…
> 
> --
> 
> dt-bindings: net: dsa: mediatek,mt7530: add econet,en751221
> 
> This adds two compatible strings to the MT7530 binding: econet,en751221-switch
> for the memory mapped switch built into the EcoNet EN751221 SoC, and
> econet,en751221 for the MT7530 multi-chip module that some EN751221 packages
> attach to port 5 of the on-die switch via TRGMII. A new $defs block constrains
> the DSA link port of the multi-chip module.
> 
> A few questions on the schema below, plus two observations about other
> patches in the same series.
> 
> > diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
> > index e29fb86934981..106c4087528d7 100644
> > --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
> > +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
> > @@ -29,6 +29,12 @@ description: |
> >    and the switch registers are directly mapped into SoC's memory map rather than
> >    using MDIO. The DSA driver currently doesn't support MT7620 variants.
> >  
> > +  EcoNet EN751221 SoCs have a memory mapped MT7530 with 3x 10/100 PHYs and 1x
>                                                           ^^^^
> 
> [Severity: Low]
> Should this be 4x 10/100 PHYs?  The commit message of this patch says
> "ports 0..3 are 10/100 only, port 4 has a gigabit PHY", which is four fast
> ethernet PHYs, and en751221_mac_port_get_caps() in the later driver patch of
> this series agrees:
> 
> 	/* EN751221 ports 0..3 are connected to the integrated FE PHYs. */
> 	case 0 ... 3:
> 		__set_bit(PHY_INTERFACE_MODE_INTERNAL, config->supported_interfaces);
> 		config->mac_capabilities |= MAC_10 | MAC_100;
> 
> Since there is no in-tree EN751221 switch DTS yet, this paragraph is the only
> port reference a DT author has.
> 
> > +  gigabit PHY. Some EN751221 chips such as EN7513G and EN7526G have a multi-chip
> > +  module implementation of the MT7530 as well as the on-die switch. In these
> > +  packages, the multi-chip module is connected to port 5 of the on-die switch
> > +  via a TRGMII link.
> > +
> >    There is only the standalone version of MT7531.
> >  
> >    Port 5 on MT7530 supports various configurations:
> > @@ -111,6 +117,14 @@ properties:
> >            Built-in switch of the EcoNet EN7528 SoC
> >          const: econet,en7528-switch
> >  
> > +      - description:
> > +          Built-in switch of EcoNet EN751221 SoC
> > +        const: econet,en751221-switch
> > +
> > +      - description:
> > +          Multi-chip module MT7530 in EN751221 SoC
> > +        const: econet,en751221
> 
> [Severity: Medium]
> Can this string collide with the existing SoC compatible?  econet,en751221 is
> already documented as the EN751221 root node compatible in
> Documentation/devicetree/bindings/mips/econet.yaml:
> 
>   properties:
>     $nodename:
>       const: '/'
> 
>     compatible:
>       oneOf:
>         - description: Boards with EcoNet EN751221 family SoC
>           items:
>             - enum:
>                 - smartfiber,xp8421-b
>             - const: econet,en751221
> 
> and arch/mips/boot/dts/econet/en751221.dtsi uses it on the root node:
> 
>   / {
> 	compatible = "econet,en751221";
> 
> dtschema derives the select clause of a binding from the strings enumerated
> under properties:compatible, so after this patch mediatek,mt7530.yaml is also
> selected for that root node.  The switch schema has required: [compatible,
> reg], unevaluatedProperties: false, and via the new conditional below also
> required: [mediatek,mcm] (which in turn pulls in resets and reset-names).  Does
> make dtbs_check on arch/mips/boot/dts/econet/en751221_smartfiber_xp8421-b.dtb
> now report the root node as an invalid MT7530 switch node?
> 
> The reverse direction looks affected too: a switch@... node carrying
> compatible = "econet,en751221" is selected by mips/econet.yaml and fails its
> $nodename: const: '/' and its two-entry board/SoC compatible list.
> 
> The analogous MT7621 case does not collide because the SoC root uses
> mediatek,mt7621-soc.  Since the SoC compatible is already released, would a
> distinct string for the switch, e.g. econet,en751221-mcm-switch, work here?

This needs to be addressed.

Rob