Thread (28 messages) 28 messages, 4 authors, 2022-09-09

Re: [PATCH 01/13] dt-bindings: memory: snps: Extend schema with IRQs/resets/clocks props

From: Krzysztof Kozlowski <hidden>
Date: 2022-08-30 15:02:06
Also in: linux-devicetree, linux-edac, lkml

On 26/08/2022 11:47, Serge Semin wrote:
quoted
quoted
+
+  interrupt-names:
+    minItems: 1
+    maxItems: 5
+    oneOf:
+      - description: Common ECC CE/UE/Scrubber/DFI Errors IRQ
+        items:
+          - const: ecc
+      - description: Individual ECC CE/UE/Scrubber/DFI Errors IRQs
+        items:
+          enum: [ ecc_ce, ecc_ue, ecc_ap, ecc_sbr, dfi_e ]
 
   reg:
     maxItems: 1
 
+  clocks:
+    description:
+      A standard set of the clock sources contains CSRs bus clock, AXI-ports
+      reference clock, DDRC core clock, Scrubber standalone clock
+      (synchronous to the DDRC clock).
+    minItems: 1
+    maxItems: 4
quoted
I expect list to be strictly defined, not flexible.
Some of the clock sources might be absent or tied up to another one
(for instance pclk, aclk and sbr can be clocked from a single core
clock source). It depends on the IP-core synthesize parameters.
Yet still your device has clock lines - clock inputs, right? Therefore
still 4 clocks will be going there or you want to say that the pin is
not connected (or pulled down)?
quoted
quoted
+
+  clock-names:
+    minItems: 1
+    maxItems: 4
+    items:
+      enum: [ pclk, aclk, core, sbr ]
+
+  resets:
+    description:
+      Each clock domain can have separate reset signal.
+    minItems: 1
+    maxItems: 4
+
+  reset-names:
+    minItems: 1
+    maxItems: 4
+    items:
+      enum: [ prst, arst, core, sbr ]
quoted
The same.
The same as for the clock.
quoted
quoted
+
 required:
   - compatible
   - reg
@@ -48,4 +92,15 @@ examples:
       interrupt-parent = <&gic>;
       interrupts = <0 112 4>;
     };
+  - |
+    memory-controller@fd070000 {
+      compatible = "snps,ddrc-3.80a";
+      reg = <0x3d400000 0x400000>;
+
+      interrupts = <0 147 4>, <0 148 4>, <0 149 4>, <0 150 4>;
quoted
Use proper defines.
What do you mean? Which defines do you think would be proper? If you
meant the IRQ DT-bindings macros, then what difference does it make
for a generic device in the DT-binding example? 
The macros/defines representing these numbers.

Note since the device
is defined as generic it can be placed on different platforms with
different interrupt controller requirements. So what do you mean by
"proper" in this case?
Proper means text instead of hard-coded number. This piece of code has
meaning in a specific context, because you used interrupts matching some
specific interrupt controllers. In that controller context, the "4" has
a meaning. Just like "0". You cannot say that this piece of code is
interrupt-controller-independent, because it is not. 4 has meaning.

If you want it to be independent, drop all the flags... If you use flags
from a specific implementation, then use proper defines matching them,
not hard-coded numbers.

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help