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

On Thu, Aug 27, 2026 at 01:39:35AM +0000, Ousherovitch, Alex wrote:

On Wed, Aug 26, 2026 at 06:02:16PM +0100, Conor Dooley wrote:

quoted

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.

With the correct syntax, which I think the sifive example I copied from
doesn't have, but
Documentation/devicetree/bindings/crypto/inside-secure,safexcel-eip93.yaml
does, fake compatibles won't be permitted (and we would probably object
to seeing them too). What we want is the binding to force people to use
a soc-specific one to pass validation. Unfortunately this does mean you
can't have an example because it won't pass the validation, which is
annoying!

quoted

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

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.

Please do, yeah.

Thanks,
Conor.