Re: [PATCH 1/4] dt-bindings: rng: add st,stm32mp25-rng support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 10/7/24 17:00, Krzysztof Kozlowski wrote:
On 07/10/2024 15:27, Gatien Chevallier wrote:
Add RNG STM32MP25x platforms compatible. Update the clock
properties management to support all versions.

Signed-off-by: Gatien Chevallier <gatien.chevallier@xxxxxxxxxxx>

You CC-ed an address, which suggests you do not work on mainline kernel
or you do not use get_maintainers.pl/b4/patman. Regardless of the
reason, process needs improvement: please CC correct address.


Hi,

I'm using get_maintainers.pl so I'll check why I have an issue.

---
  .../devicetree/bindings/rng/st,stm32-rng.yaml | 41 +++++++++++++++++--
  1 file changed, 38 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml b/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
index 340d01d481d1..c92ce92b6ac9 100644
--- a/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
+++ b/Documentation/devicetree/bindings/rng/st,stm32-rng.yaml
@@ -18,12 +18,19 @@ properties:
      enum:
        - st,stm32-rng
        - st,stm32mp13-rng
+      - st,stm32mp25-rng
reg:
      maxItems: 1
clocks:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
+
+  clock-names:

Missing minItems


Ok, will add in V2

+    items:
+      - const: rng_clk
+      - const: rng_hclk

Drop _clk and come with some reasonable names, e.g. "core" and "bus"?


Sure, makes sense. Will change in V2.

resets:
      maxItems: 1
@@ -57,15 +64,43 @@ allOf:
        properties:
          st,rng-lock-conf: false
+ - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - st,stm32mp25-rng
+    then:
+      properties:
+        clocks:
+          description: >
+            RNG bus clock must be named "rng_hclk". The RNG kernel clock
+            must be named "rng_clk".

Drop description, useless.

Missing minItems


Ok, will update in V2

+          maxItems: 2
+      required:
+        - clock-names
+    else:
+      properties:
+        clocks:
+          maxItems: 1

Missing constrain for clock-names.

+
  additionalProperties: false
examples:
    - |
-    #include <dt-bindings/clock/stm32mp1-clks.h>

Why?

      rng@54003000 {
        compatible = "st,stm32-rng";
        reg = <0x54003000 0x400>;
-      clocks = <&rcc RNG1_K>;
+      clocks = <&rcc 124>;

Why?



I have an issue with the generated st,stm32-rng.example.dts example.
Because there are 2 binding files included, I have a collision with
clock bindings names between the "dt-bindings/clock/stm32mp1-clks.h"
and the "dt-bindings/clock/st,stm32mp25-rcc.h" files. For example:
CK_MCO1 or CK_SCMI_HSE. I replaced the bindings with numbers
to avoid this conflict.

If you think this binding update does not need the addition of an
example, I'll completely drop it and we won't have the issue.

Best regards,
Gatien

      };
+ - |
+    rng: rng@42020000 {
+      compatible = "st,stm32mp25-rng";
+      reg = <0x42020000 0x400>;
+      clocks = <&clk_rcbsec>, <&rcc 110>;
+      clock-names = "rng_clk", "rng_hclk";
+      resets = <&rcc 97>;
+      access-controllers = <&rifsc 92>;


Difference in one property should not need new example, usually.

Best regards,
Krzysztof





[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]
  Powered by Linux