| Message ID | 20250306210950.1686713-2-jonas@kwiboo.se (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: stmmac: dwmac-rk: Validate rockchip,grf and php-grf during probe | expand |
On Thu, Mar 06, 2025 at 09:09:45PM +0000, Jonas Karlman wrote: > All Rockchip GMAC variants require writing to GRF to configure e.g. > interface mode and MAC rx/tx delay. > > Change binding to require rockchip,grf and rockchip,php-grf to reflect > that GRF (and PHP-GRF for RK3576/RK3588) control part of GMAC. It is pretty unusual to change the binding such that something optional becomes mandatory. I would expect a bit more of a comment explaining why this does not cause backwards compatibility issues. Have all the .dtsi files always had these properties? Andrew --- pw-bot: cr
Hi Andrew, On 2025-03-06 23:32, Andrew Lunn wrote: > On Thu, Mar 06, 2025 at 09:09:45PM +0000, Jonas Karlman wrote: >> All Rockchip GMAC variants require writing to GRF to configure e.g. >> interface mode and MAC rx/tx delay. >> >> Change binding to require rockchip,grf and rockchip,php-grf to reflect >> that GRF (and PHP-GRF for RK3576/RK3588) control part of GMAC. > > It is pretty unusual to change the binding such that something > optional becomes mandatory. I would expect a bit more of a comment > explaining why this does not cause backwards compatibility > issues. Have all the .dtsi files always had these properties? rockchip,grf was listed under required properties prior to the commit b331b8ef86f0 ("dt-bindings: net: convert rockchip-dwmac to json-schema"), maybe this was just lost during the conversion to yaml schema. The DT's I have managed to check all seem to have the rockchip,grf prop and the old .txt schema listed "phandle to the syscon grf used to control speed and mode". Without the rockchip,grf the driver just logged an error and ignored trying to configure speed or mode. We could possible leave it as optional, but when it is missing speed and mode cannot be configured by the driver. Today this just result in an error message, after this series there will instead be a probe error. Regards, Jonas > > Andrew > > --- > pw-bot: cr
diff --git a/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml b/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml index f8a576611d6c..05a5605f1b51 100644 --- a/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml +++ b/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml @@ -32,9 +32,6 @@ select: required: - compatible -allOf: - - $ref: snps,dwmac.yaml# - properties: compatible: oneOf: @@ -114,6 +111,20 @@ required: - compatible - clocks - clock-names + - rockchip,grf + +allOf: + - $ref: snps,dwmac.yaml# + - if: + properties: + compatible: + contains: + enum: + - rockchip,rk3576-gmac + - rockchip,rk3588-gmac + then: + required: + - rockchip,php-grf unevaluatedProperties: false
All Rockchip GMAC variants require writing to GRF to configure e.g. interface mode and MAC rx/tx delay. Change binding to require rockchip,grf and rockchip,php-grf to reflect that GRF (and PHP-GRF for RK3576/RK3588) control part of GMAC. Signed-off-by: Jonas Karlman <jonas@kwiboo.se> --- --- .../devicetree/bindings/net/rockchip-dwmac.yaml | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)