by Rob Herring (Arm)

[permalink] [raw]

Subject: Re: [Patch v5 2/9] dt-bindings: arm: tegra: Add NVIDIA Tegra194 CBB1.0 binding

On Fri, May 06, 2022 at 04:42:10PM +0530, Sumit Gupta wrote:
> Add device-tree binding documentation to represent the error handling
> driver for Control Backbone (CBB) version 1.0 used in Tegra194 SOC.

Bindings are for h/w not drivers.

> The driver prints debug information about failed transactions due to
> illegal register accesses on receiving interrupt from CBB.
>
> Signed-off-by: Sumit Gupta <[email protected]>
> ---
> .../arm/tegra/nvidia,tegra194-cbb.yaml | 93 +++++++++++++++++++
> 1 file changed, 93 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/arm/tegra/nvidia,tegra194-cbb.yaml
>
> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra194-cbb.yaml b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra194-cbb.yaml
> new file mode 100644
> index 000000000000..3167f0450298
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra194-cbb.yaml
> @@ -0,0 +1,93 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/arm/tegra/nvidia,tegra194-cbb.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: NVIDIA Tegra194 CBB 1.0 device tree bindings

'device tree bindings' is redundant

> +
> +maintainers:
> + - Sumit Gupta <[email protected]>
> +
> +description: |+
> + The Control Backbone (CBB) is comprised of the physical path from an initiator to a target's
> + register configuration space. CBB 1.0 has multiple hierarchical sub-NOCs (Network-on-Chip) and
> + connects various initiators and targets using different bridges like AXIP2P, AXI2APB.
> +
> + This driver handles errors due to illegal register accesses reported by the NOCs inside the CBB.
> + NOCs reporting errors are cluster NOCs "AON-NOC, SCE-NOC, RCE-NOC, BPMP-NOC, CV-NOC" and "CBB
> + Central NOC" which is the main NOC.
> +
> + By default, the access issuing initiator is informed about the error using SError or Data Abort
> + exception unless the ERD (Error Response Disable) is enabled/set for that initiator. If the ERD
> + is enabled, then SError or Data Abort is masked and the error is reported with interrupt.
> +
> + - For CCPLEX (CPU Complex) initiator, the driver sets ERD bit. So, the errors due to illegal
> + accesses from CCPLEX are reported by interrupts. If ERD is not set, then error is reported by
> + SError.
> + - For other initiators, the ERD is disabled. So, the access issuing initiator is informed about
> + the illegal access by Data Abort exception. In addition, an interrupt is also generated to
> + CCPLEX. These initiators include all engines using Cortex-R5 (which is ARMv7 CPU cluster) and
> + engines like TSEC (Security co-processor), NVDEC (NVIDIA Video Decoder engine) etc which can
> + initiate transactions.
> +
> + The driver prints relevant debug information like Error Code, Error Description, Master, Address,
> + AXI ID, Cache, Protection, Security Group etc on receiving error notification.

Please wrap at 80. Longer is allowed, but should still be the exception.

> +
> +properties:
> + $nodename:
> + pattern: "^[a-z]+-noc@[0-9a-f]+$"
> +
> + compatible:
> + enum:
> + - nvidia,tegra194-cbb-noc
> + - nvidia,tegra194-aon-noc
> + - nvidia,tegra194-bpmp-noc
> + - nvidia,tegra194-rce-noc
> + - nvidia,tegra194-sce-noc
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + description:
> + CCPLEX receives secure or nonsecure interrupt depending on error type. A secure interrupt is
> + received for SEC(firewall) & SLV errors and a non-secure interrupt is received for TMO & DEC
> + errors.
> + items:
> + - description: non-secure interrupt
> + - description: secure interrupt
> +
> + nvidia,axi2apb:
> + $ref: '/schemas/types.yaml#/definitions/phandle'
> + description:
> + Specifies the node having all axi2apb bridges which need to be checked for any error logged
> + in their status register.
> +
> + nvidia,apbmisc:
> + $ref: '/schemas/types.yaml#/definitions/phandle'
> + description:
> + Specifies the apbmisc node which need to be used for reading ERD register.
> +
> +additionalProperties: false
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - nvidia,axi2apb
> + - nvidia,apbmisc
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> + cbb-noc@2300000 {
> + compatible = "nvidia,tegra194-cbb-noc";
> + reg = <0x02300000 0x1000>;
> + interrupts = <GIC_SPI 230 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 231 IRQ_TYPE_LEVEL_HIGH>;
> + nvidia,axi2apb = <&axi2apb>;
> + nvidia,apbmisc = <&apbmisc>;
> + status = "okay";

Don't need status in examples.

> + };
> --
> 2.17.1
>
>