Thread (25 messages) 25 messages, 5 authors, 1d ago

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help