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(-)

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
 
-- 
2.48.1

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