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





[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]
  Powered by Linux