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

Conor Dooley <[email protected]>
Newsgroups org.infradead.lists.linux-riscv,org.kernel.vger.linux-api,org.kernel.vger.linux-crypto,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-doc,org.kernel.vger.linux-kernel,org.kernel.vger.linux-kselftest
Message-ID <20260827-crust-giddy-6888a52871a5@spud>
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:
> > On Tue, Aug 25, 2026 at 03:15:20PM -0700, Alex Ousherovitch wrote:
> > > +      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!

> 
> > > +      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.)
> 
> > > +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.

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
signature.asc (application/pgp-signature, 228 B)
-----BEGIN PGP SIGNATURE-----

iHUEABYKAB0WIQRh246EGq/8RLhDjO14tDGHoIJi0gUCapBwbQAKCRB4tDGHoIJi
0tg1AQDJxvd89lNx3GSYKYhVSI+ByKuzMH06Aat4j5i/Er7GCAD/bUIEGNkrxnPj
TriUguZYuEpl11M5MLqp9TSVpc3RUQw=
=+I7h
-----END PGP SIGNATURE-----
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.