Hi Rob, Thanks for the review, I will update and push the patches accordingly. Warm regards, PK On Fri, Sep 6, 2024 at 1:47 AM Rob Herring <robh@xxxxxxxxxx> wrote: > > On Thu, Sep 5, 2024 at 2:28 PM Pavitrakumar Managutte > <pavitrakumarm@xxxxxxxxxxxxxxx> wrote: > > > > 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. > > Also, please use full names. PK: Will do that > > > > > 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) > > Not testing BE is clear from reading the driver... > > It's probably safe to assume the IP will be configured to match the > processor. The endian properties are for the exceptions where the > peripherals don't match the CPU's endianness. This property can be > added when and if there's a platform needing it. Any specific platform > should have a specific compatible added as well. PK: Agreed, I will remove "little-endian" property but the code defaults to little endian implementation. In case the big-endian support comes, we will add a "big-endian" property. > > > > > > > > + > > > > + 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. > > Okay, I'd use 'id' rather than 'index' in that case. PK: Sure, I will change it to "vspacc-id". > > Rob
