RE: [PATCH v2 01/19] dt-bindings: crypto: add Rambus CryptoManager Hub
From: Ousherovitch, Alex <hidden>
Date: 2026-07-13 23:17:26
Also in:
linux-crypto, linux-devicetree, linux-doc, linux-kselftest, linux-riscv, lkml
On Sun, Jul 12, 2026 at 06:16, Conor Dooley [off-list ref] wrote:
Please fix your quoting, you need to retain context beyond what I said so that people who get 100s of mails per day (me) remember what it was in response to.
Apologies -- retained the context here, and I'll keep it in future replies. Short version: v3 switches the vendor prefix to "rambus", makes each mailbox a subnode with its own reg (as agreed), adds a device-specific compatible, and adds clocks + an optional reset. One property (the VCQ ring geometry) I'd like to keep in DT -- reasoning below, happy to be overruled.
quoted
quoted
This company no longer exists, you should probably introduce a rambus vendor prefix instead.Cryptography Research, Inc. does still exist -- it's now a wholly-owned subsidiary of Rambus (our co-maintainer is @cryptography.com). The prefix names the IP originator, which is consistent with existing subsidiary/acquired-vendor prefixes in the tree (e.g. al = Annapurna Labs under Amazon, mstar noted as acquired by MediaTek, fsl, cavium, xlnx). We'd prefer to keep "cri" on that basis, and can annotate the description as "Cryptography Research, Inc. (a Rambus company)" to make the ownership explicit. Happy to switch if you feel strongly.I'm not sure that these examples actually aid your cause. al has been replaced by amazon, fsl is not used for new devices, new xlnx devices use amd (only example for now is the riscv stuff I think), cavium has had nothing added in donkey's years etc. mstar I don't see anything new in years either.
Fair enough -- point taken. v3 uses a "rambus" vendor prefix (added to vendor-prefixes.yaml) and drops "cri". The description still credits Cryptography Research, Inc. as the IP originator.
quoted
quoted
This whole subnode thing seems like it is only required because you don't have device-specific compatibles [cores].Core presence is actually discoverable at runtime from the CORE_ENABLE register, so v3 will drop the per-core child nodes entirely and probe for enabled cores -- no per-variant compatible needed.No, per-variant compatibles (for the devices/socs that this IP is integrated into) are a requirement. While it would have been handy for detecting capabilities, it's a requirement for other reasons: differences between integrations be that functional or enforcing the correct constraints on properties, issues only present on select devices, etc.
Agreed, and done. Runtime CORE_ENABLE probing stays only for capability
detection; it does not replace the compatible. v3 adds a device-specific
compatible keyed to the IP revision:
compatible = "rambus,cmh-v1030";
with an SoC integration expected to list its own compatible first and
fall back to the IP one:
compatible = "<vendor>,<soc>-cmh", "rambus,cmh-v1030";
This follows the pattern already used for licensable security IP, e.g.
inside-secure,safexcel-eip197b and arm,cryptocell-712-ree.
On that note, I see there's no clocks or resets properties added by your patch. While the IP may not have a reset (although I suspect it probably does) there's no way it functions without a clock.
Correct on both. v3 adds them: - clocks: the hub has a main functional input clock (plus, on side-channel-core configurations, a half-rate variant, and a real-time-tick clock). The block gates its clocks internally, so the host doesn't gate them -- the property just describes the input pin(s). Where a secure power controller owns the clock and Linux has no handle, it's absent, as drivers/crypto/xilinx/zynqmp-* do. - reset: the hub has two external, active-low reset inputs -- a power-on reset and a hard reset. On an integration these are top-level wires driven by a reset controller in the management domain, not by Linux. Where the Linux host is the management host and one of those lines is exposed to it as a GPIO, v3 describes it via reset-gpios; otherwise it's absent and Linux doesn't reset the block.
quoted
quoted
This looks like it should be deducible from a device-specific compatible. [slots/strides]These aren't fixed per silicon -- they're the per-mailbox layout of the VCQ rings in host DMA memory, chosen at platform integration and programmed by the driver into the mailbox QUEUE/SLOTS/STRIDE registers. They can differ per mailbox on the same silicon, so a compatible can't encode them. v3 will keep them as optional, defaulted properties on the per-mailbox subnodes.I'm not sure. Unless there's more than one instance, this definitely sounds like something that you can determine from the compatible. Generally these kinds of accelerators tend not to have multiple instances though, so each platform will have a different compatible, and the driver can store an array of mailbox configurations.
This is the one I'd like to keep in DT. The slot count and stride are the host-DMA ring geometry the driver programs into QUEUE_SLOTS / QUEUE_STRIDE at probe. They're fixed neither by the silicon nor by the integration: the same SoC (same compatible) ships to multiple customers who provision different amounts of host memory for the rings, and a customer may tune it further per board -- so two boards can share a compatible yet need different geometry. They're optional with defaults (6 / 9), so the common case sets nothing. The reason I reached for DT is that the alternatives are each worse for a per-board value that has to be known at probe: per-compatible match_data means a new compatible and a driver patch for every board that tunes it (two boards on the same SoC can't differ), and a module parameter is exactly the sort of global knob I'd expect you'd rather not add. DT lets the integrator set it without touching the driver. If you'd still prefer one of those, tell me which and I'll switch.
quoted
quoted
this could probably be handled via reg-names? [affinity]Yes -- v3 will express affinity per mailbox (a "role" of a specific core type for a dedicated mailbox, or "generic" for the round-robin pool), which is the subnode analog of your reg-names idea. One caveat: this cleanly covers 1:1 core-to-mailbox dedication plus a shared pool; a mailbox dedicated to several specific cores would need multiple role tokens.
Following up on my own reply: I kept an explicit core-ID affinity list on each mailbox node rather than roles/reg-names. As the caveat above hinted, roles get awkward once a mailbox serves several cores, and they can't distinguish separate instances of the same core type -- the core ID handles both, and the driver would map a role back to an ID anyway. Thanks for the review, Alex