HI Rob, Thanks for the review and feedback. On Thu, Sep 5, 2024 at 11:53 PM Rob Herring <robh@xxxxxxxxxx> wrote: > > On Thu, Sep 05, 2024 at 08:39:10PM +0530, Pavitrakumar M wrote: > > Add DT bindings related to the SPAcc driver for Documentation. > > DWC Synopsys Security Protocol Accelerator(SPAcc) Hardware Crypto > > Engine is a crypto IP designed by Synopsys. > > This belongs with the rest of your driver series. PK: I will club this with the SPAcc driver patch-set. > > > > > Signed-off-by: Bhoomika K <bhoomikak@xxxxxxxxxxxxxxx> > > Signed-off-by: Pavitrakumar M <pavitrakumarm@xxxxxxxxxxxxxxx> > > There's 2 possibilities: Bhoomika is the author and you are just > submitting it, or you both developed it. The former needs the git author > fixed to be Bhoomika. The latter needs a Co-developed-by tag for > Bhoomika. PK: Its co-developed. I will add co-developed-by tag for Bhoomika in all the patches. > > > Acked-by: Ruud Derwig <Ruud.Derwig@xxxxxxxxxxxx> > > --- > > .../bindings/crypto/snps,dwc-spacc.yaml | 79 +++++++++++++++++++ > > 1 file changed, 79 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/crypto/snps,dwc-spacc.yaml > > > > diff --git a/Documentation/devicetree/bindings/crypto/snps,dwc-spacc.yaml b/Documentation/devicetree/bindings/crypto/snps,dwc-spacc.yaml > > new file mode 100644 > > index 000000000000..a58d1b171416 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/crypto/snps,dwc-spacc.yaml > > @@ -0,0 +1,79 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/crypto/snps,dwc-spacc.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Synopsys DesignWare Security Protocol Accelerator(SPAcc) Crypto Engine > > + > > +maintainers: > > + - Ruud Derwig <Ruud.Derwig@xxxxxxxxxxxx> > > + > > +description: > > + DWC Synopsys Security Protocol Accelerator(SPAcc) Hardware Crypto Engine is > > + a crypto IP designed by Synopsys, that can accelerate cryptographic > > + operations. > > + > > +properties: > > + compatible: > > + contains: > > Drop contains. The list of compatible strings and order must be defined. PK: Will drop it as the SPAcc driver is using just "snps,dwc-spacc". > > > + enum: > > + - snps,dwc-spacc > > + - snps,dwc-spacc-6.0 > > What's the difference between these 2? The driver only had 1 compatible, > so this should too. PK: Will fix this to "snps,dwc-spacc" > > > + > > + reg: > > + maxItems: 1 > > + > > + interrupts: > > + maxItems: 1 > > + > > + clocks: > > + maxItems: 1 > > + > > + clock-names: > > + maxItems: 1 > > No, you must define the name. But really, just drop it because you don't > need names with only 1 name. PK: Will remove it. > > > + > > + little-endian: true > > Do you really need this? You have a BE CPU this is used with? PK: Its a configurable IP. Can be used in little and big-endian configurations. We have tested it on Little-endian CPUs only. (Xilinx Zynq Ultrascale ZCU104) > > > + > > + vspacc-priority: > > Custom properties need a vendor prefix (snps,). PK: Will add vendor prefix. > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + description: > > + Set priority mode on the Virtual SPAcc. This is Virtual SPAcc priority > > + weight. Its used in priority arbitration of the Virtual SPAccs. > > + minimum: 0 > > + maximum: 15 > > + default: 0 > > + > > + vspacc-index: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + description: Virtual spacc index for validation and driver functioning. > > We generally don't do indexes in DT. Need a better description of why > this is needed. PK: This is NOT for indexing into DT but more like an ID of a virtual SPAcc. The SPAcc IP can be configured as virtual SPAccs for multi-processor support, where they appear to be dedicated to each processor. The SPAcc hardware supports 2-8 virtual SPAccs (each vSPAcc has its Register bank and context-memory for crypto operations). The index here represents the vSPAcc to be referenced. > > > + minimum: 0 > > + maximum: 7 > > + > > + spacc-wdtimer: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + description: Watchdog timer count to replace the default value in driver. > > + minimum: 0x19000 > > + maximum: 0xFFFFF > > + > > +required: > > + - compatible > > + - reg > > + - interrupts > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + spacc@40000000 { > > crypto@40000000 > > > + compatible = "snps,dwc-spacc"; > > + reg = <0x40000000 0x3FFFF>; > > + interrupt-parent = <&gic>; > > + interrupts = <0 89 4>; > > + clocks = <&clock>; > > + clock-names = "ref_clk"; > > + vspacc-priority = <4>; > > + spacc-wdtimer = <0x20000>; > > + vspacc-index = <0>; > > + little-endian; > > + }; > > -- > > 2.25.1 > >