diff mbox series

[1/2] dt-bindings: net: rockchip-dwmac: Require rockchip,grf and rockchip,php-grf

Message ID 20250306210950.1686713-2-jonas@kwiboo.se (mailing list archive)
State New
Headers show
Series net: stmmac: dwmac-rk: Validate rockchip,grf and php-grf during probe | expand

Commit Message

Jonas Karlman March 6, 2025, 9:09 p.m. UTC
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(-)

Comments

Andrew Lunn March 6, 2025, 10:32 p.m. UTC | #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
Jonas Karlman March 6, 2025, 11:42 p.m. UTC | #2
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 mbox series

Patch

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