Re: [PATCH v1 1/1] dt-bindings: crypto: Document support for SPAcc

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
> >





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux