Re: [PATCH net-next v5 02/11] dt-bindings: phy: rockchip: naneng-combphy: add rockchip,sgmii-mac-sel property
From: netdev-bot+sashiko@kernel.org
Date: 2026-09-14 20:24:53
Also in:
linux-arm-kernel, linux-phy, linux-renesas-soc, linux-rockchip, lkml, netdev
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Critical: 0 · High: 0 · Medium: 0 · Low: 1 - [Low] Concern (validated as NOT a defect in the series context): the binding adds `rockchip,sgmii-mac-sel` while, at this commit alone, no… -- dt-bindings: phy: rockchip: naneng-combphy: add rockchip,sgmii-mac-sel property This adds an optional rockchip,sgmii-mac-sel property to the naneng-combphy binding so a board can select whether the RK3568 SGMII interface is routed to GMAC0 or GMAC1. The property is restricted to the RK3568 compatible via the existing allOf conditional and defaults to 1 (GMAC1).
quoted hunk ↗ jump to hunk
diff --git a/Documentation/devicetree/bindings/phy/phy-rockchip-naneng-combphy.yaml b/Documentation/devicetree/bindings/phy/phy-rockchip-naneng-combphy.yaml index 379b08bd9e97a..8e898bce9af73 100644 --- a/Documentation/devicetree/bindings/phy/phy-rockchip-naneng-combphy.yaml +++ b/Documentation/devicetree/bindings/phy/phy-rockchip-naneng-combphy.yaml@@ -80,6 +80,15 @@ properties: description: Some additional pipe settings are accessed through GRF regs. + rockchip,sgmii-mac-sel: + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [0, 1] + default: 1 + description: + Select gmac0 or gmac1 to be used as SGMII controller. + The hardware reset value is GMAC1 (1). Set this to 0 to route + SGMII to GMAC0. +
[Severity: Low]
At this commit alone, is rockchip,sgmii-mac-sel read by anything?
rockchip_combphy_parse_dt() in
drivers/phy/rockchip/phy-rockchip-naneng-combphy.c does not look it up,
struct rockchip_combphy_grfcfg has no pipe_sgmii_mac_sel descriptor, and
no in-tree DTS sets it, so a kernel built from exactly this commit would
silently ignore the property.
This resolves itself later in the same series: "phy: rockchip:
naneng-combphy: add SGMII MAC selection for RK3568" adds the parsing
priv->sgmii_mac_sel = 1;
device_property_read_u32(dev, "rockchip,sgmii-mac-sel", &priv->sgmii_mac_sel);
along with the RK3568 pipe-GRF descriptor
.pipe_sgmii_mac_sel = { 0x0040, 1, 1, 0x00, 0x01 },
and the write in the PHY_TYPE_SGMII path of rk3568_combphy_cfg(), and
"arm64: dts: rockchip: rk3568-photonicat: enable SGMII LAN port" adds the
first user:
&combphy2 {
rockchip,sgmii-mac-sel = <0>;
status = "okay";
};
So this is noted only for the record, since the binding and its consumer
being split across consecutive patches is the normal series layout.
The remaining point is a design question rather than a bug: is a raw
uint32 MAC index the interface preferred here, or would a phandle to the
GMAC controller node be a better fit for describing the routing?
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260913185849.907479-1-coiaprant%40gmail.com