Thread (24 messages) flat view 24 messages, 2 authors, 2d ago

RE: [PATCH v4 01/19] dt-bindings: crypto: add Rambus CryptoManager Hub

From: Ousherovitch, Alex <hidden>
Date: 2026-08-27 01:39:39
Also in: linux-crypto, linux-devicetree, linux-doc, linux-kselftest, linux-riscv, lkml

On Wed, Aug 26, 2026 at 06:02:16PM +0100, Conor Dooley wrote:
On Tue, Aug 25, 2026 at 03:15:20PM -0700, Alex Ousherovitch wrote:
quoted
+      command ABI).  A specific SoC integration should list its own
+      "<vendor>,<soc>-cmh" compatible first with "rambus,cmh-v1030" as a
+      fallback; that extended form will be added when such a platform is
+      upstreamed.
This commentary should just not be here.
Instead, do what's been done in sifive,clint.yaml for the clint2.
Will do -- I'll drop the prose and model the compatible on sifive,clint2
so the schema itself requires an SoC-specific compatible ahead of the IP
fallback:

    compatible:
      items:
        - {}
        - const: rambus,cmh-v1030

Since that makes the two-item form the only valid one, I'll also move the
binding example (and our test DTs) to it, using a placeholder SoC-specific
compatible under the now-registered rambus prefix, e.g.

    compatible = "rambus,<soc>-cmh", "rambus,cmh-v1030";

and swap in the real "<vendor>,<soc>-cmh" string once the first integrating
platform is upstreamed.
quoted
+      rambus,cores:
+        $ref: /schemas/types.yaml#/definitions/uint32-array
+        items:
+          enum: [0x02, 0x03, 0x04, 0x05, 0x08, 0x09, 0x0a, 0x18]
[...]

You could just make this a string array, and then you'd not need this
mapping of unintelligible hex values. Of course then your driver has to
do it, but I'd rather see understandable (and portable, since a new
revision could change the IDs) values in DT.
Agreed, that's clearly better.  The next revision makes rambus,cores a
string array and maps the names to core IDs in the driver, so the
numbering can change across IP revisions without touching the DT:

    rambus,cores:
      $ref: /schemas/types.yaml#/definitions/string-array
      items:
        enum: [hc, aes, sm4, sm3, hcq, qse, pke, ccp]

(keeping the affinity semantics in the description -- a core appears on at
most one queue, unlisted cores are load-balanced -- just without the hex
table.)
quoted
+required:
+  - compatible
+  - reg
+  - "#address-cells"
+  - "#size-cells"
Surely clocks are also mandatory?
They can't be mandatory for this binding -- there may be no clock provider
for the node to reference:

- CMH is shared between hosts.  The SIC presents up to four host
   interfaces; a designated management host owns the privileged
   configuration (clock, reset and power) while the other hosts are
   granted only a subset of mailboxes.  A node describing a non-management
   host therefore has no clock/reset provider to point at -- those lines
   are driven by the management host or an SoC power controller.

- Even where a provider exists, only the main "core" clock is always
   fitted; "core-div2" is present only on builds that include the
   side-channel-protected slow cores, and "rt" is the real-time tick,
   which is why clock-names is a oneOf of subsets rather than a fixed
   list.

So I'd keep clocks (and reset-gpios) optional.  I can add a one-line,
hardware-framed note to the description if you'd like that rationale on
record.

Thanks,
Alex
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help