On Sun, Jul 12, 2026 at 06:16, Conor Dooley <[email protected]> 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.
> > > 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.
> > > 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.
> > > 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.
> > > 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
