[RFC PATCH 02/10] dt-bindings: phy: rockchip: naneng-combphy: add rockchip,sgmii-mac-sel property
Coia Prant
coiaprant at gmail.com
Wed Jul 15 12:53:11 PDT 2026
Thanks for the review. I'll address both issues in v2.
> --- Patch [2]: [RFC PATCH 02/10] dt-bindings: phy: rockchip: naneng-combphy: add rockchip,sgmii-mac-sel property ---
> commit 0d950b4055dd144fecf5059f16535f2c8d94b5d3
> Author: Coia Prant <coiaprant at gmail.com>
>
> dt-bindings: phy: rockchip: naneng-combphy: add rockchip,sgmii-mac-sel property
>
> This commit adds the "rockchip,sgmii-mac-sel" property to the Rockchip
> Naneng combphy device tree binding. It allows selecting between GMAC0
> and GMAC1 for the SGMII interface on the RK3568 SoC.
>
> > This is necessary for boards such as the Ariaboard Photonicat, where
> > the SGMII interface is connected to GMAC0 and needs to be explicitly
> > configured.
>
> [Severity: Medium]
> Is it actually necessary to explicitly configure the property to 0 for
> GMAC0?
>
> The schema defines the default as 0, which implies that omitting the property
> would naturally default the hardware to GMAC0. If explicit configuration is
> required, does this mean the hardware or bootloader leaves the register at 1,
> making the `default: 0` documented in the schema incorrect?
Regarding the default value: the hardware reset value for the
`pipe_sgmii_mac_sel` bit is GMAC1 (1). I have updated the binding
to set `default: 1` and revised the commit message accordingly,
removing the contradictory "default 0" statement.
The default now matches the hardware reset value.
> [Severity: Low]
> Should this property be conditionally restricted to the RK3568 compatible
> string?
>
> Because it is defined in the top-level properties block, it will pass
> validation for all other hardware variants (like RK3528 or RK3588) that do
> not actually support SGMII. Could an `if-then` block in the `allOf` section
> be used to set this property to false for compatibles other than
> `rockchip,rk3568-naneng-combphy`?
Regarding the property scope: you are correct that this property
only makes sense for RK3568. I have added an `if-then` conditional
in the `allOf` section to restrict this property to
`rockchip,rk3568-naneng-combphy` only.
Thanks,
Coia
More information about the linux-phy
mailing list