Message-ID: <5d10fa17-364c-9d90-c083-f338957631a6@linaro.org>
Date:   Fri, 3 Mar 2023 10:53:46 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: [PATCH 2/9] dt-bindings: crypto: fsl-sec4: convert to DT schema
Content-Language: en-US
To:     "Peng Fan (OSS)" <peng.fan@oss.nxp.com>,
        herbert@gondor.apana.org.au, davem@davemloft.net,
        robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
        horia.geanta@nxp.com, pankaj.gupta@nxp.com, gaurav.jain@nxp.com,
        shawnguo@kernel.org, s.hauer@pengutronix.de
Cc:     kernel@pengutronix.de, stefan@agner.ch,
        linux-crypto@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        Peng Fan <peng.fan@nxp.com>
References: <20230301015702.3388458-1-peng.fan@oss.nxp.com>
 <20230301015702.3388458-3-peng.fan@oss.nxp.com>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <20230301015702.3388458-3-peng.fan@oss.nxp.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 01/03/2023 02:56, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> Convert the fsl-sec4 binding to DT schema

I don't see the removal part of conversion.

> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  .../devicetree/bindings/crypto/fsl-sec4.yaml  | 324 ++++++++++++++++++
>  1 file changed, 324 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/crypto/fsl-sec4.yaml

Filename matching compatible.

> 
> diff --git a/Documentation/devicetree/bindings/crypto/fsl-sec4.yaml b/Documentation/devicetree/bindings/crypto/fsl-sec4.yaml
> new file mode 100644
> index 000000000000..678c8389ef49
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/crypto/fsl-sec4.yaml
> @@ -0,0 +1,324 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/crypto/fsl-sec4.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP SEC4 Crypto Binding

Drop "Binding"

> +
> +description:
> +  CONTENTS
> +    -Overview
> +    -SEC 4 Node
> +    -Job Ring Node
> +    -Run Time Integrity Check (RTIC) Node
> +    -Run Time Integrity Check (RTIC) Memory Node

????

> +  NOTE, the SEC 4 is also known as Freescale's Cryptographic Accelerator
> +  Accelerator and Assurance Module (CAAM).
> +  For information on SEC4 SNVS, ref fsl-sec4-snvs.yaml
> +
> +  =====================================================================
> +  Overview
> +
> +  DESCRIPTION

Reformat it to look like normal text. Drop "====", fix title case.

> +
> +  SEC 4 h/w can process requests from 2 types of sources.
> +  1. DPAA Queue Interface (HW interface between Queue Manager & SEC 4).
> +  2. Job Rings (HW interface between cores & SEC 4 registers).
> +
> +  High Speed Data Path Configuration,
> +
> +  HW interface between QM & SEC 4 and also BM & SEC 4, on DPAA-enabled parts
> +  such as the P4080.  The number of simultaneous dequeues the QI can make is
> +  equal to the number of Descriptor Controller (DECO) engines in a particular
> +  SEC version.  E.g., the SEC 4.0 in the P4080 has 5 DECOs and can thus
> +  dequeue from 5 subportals simultaneously.
> +
> +  Job Ring Data Path Configuration,
> +
> +  Each JR is located on a separate 4k page, they may (or may not) be made visible
> +  in the memory partition devoted to a particular core.  The P4080 has 4 JRs, so
> +  up to 4 JRs can be configured; and all 4 JRs process requests in parallel.
> +
> +maintainers:
> +  - Peng Fan <peng.fan@nxp.com>

Keep the order of properties like in example-schema.

> +
> +properties:
> +  compatible:
> +    enum:
> +      - fsl,sec-v4.0
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 1
> +
> +  reg:
> +    maxItems: 1
> +
> +  ranges:
> +    description:
> +      A standard property. Specifies the physical address range of the SEC
> +      4.0 register space (-SNVS not included).  A triplet that includes the
> +      child address, parent address, & length.
> +
> +  interrupts:
> +    description:
> +      Specifies the interrupts generated by this device.  The value of the
> +      interrupts property consists of one interrupt specifier. The format
> +      of the specifier is defined by the binding document describing the
> +      node's interrupt parent.
> +    maxItems: 1
> +
> +  clocks:
> +    minItems: 1
> +    maxItems: 4
> +
> +  clock-names:
> +    oneOf:
> +      - items:
> +          - const: mem
> +          - const: aclk
> +          - const: ipg
> +          - const: emi_slow
> +      - items:
> +          - const: aclk
> +          - const: ipg
> +      - items:
> +          - const: ipg
> +          - const: aclk
> +          - const: mem
> +
> +  fsl,sec-era:
> +    description:
> +      Optional. A standard property. Define the 'ERA' of the SEC device.

Drop redundant, free form text - optional.

What is a "standard property"? So all others are non-standard? They
violate standard? What does it mean?

I will actually skip the review as I really do not know if this is new
binding all conversion.

(...)

> +
> +required:
> +  - compatible
> +  - reg
> +
> +unevaluatedProperties: false

additionalProperties instead


Best regards,
Krzysztof