Re: [Patch Resend v1 5/8] dt-bindings: arm: tegra: Add NVIDIA Tegra234 CBB2.0 binding

[Sumit Gupta <sumitg@nvidia.com>]

>>> On Thu, Dec 09, 2021 at 10:52:03PM +0530, Sumit Gupta wrote:
>>>> Add device-tree binding documentation to represent CBB2.0 (Control
>>>> Backbone) error handling driver. The driver prints debug information
>>>> about failed transaction on receiving interrupt from CBB2.0.
>>>>
>>>> Signed-off-by: Sumit Gupta<sumitg@nvidia.com>
>>>> ---
>>>>    .../arm/tegra/nvidia,tegra234-cbb.yaml        | 80 +++++++++++++++++++
>>>>    1 file changed, 80 insertions(+)
>>>>    create mode 100644 Documentation/devicetree/bindings/arm/tegra/nvidia,tegra234-cbb.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra234-cbb.yaml b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra234-cbb.yaml
>>>> new file mode 100644
>>>> index 000000000000..ad8177255e6c
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra234-cbb.yaml
>>>> @@ -0,0 +1,80 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +
>>>> +$id:"http://devicetree.org/schemas/arm/tegra/tegra23_cbb.yaml#"
>>>> +$schema:"http://devicetree.org/meta-schemas/core.yaml#"
>>>> +
>>>> +title: NVIDIA Tegra CBB2.0 Error handling driver device tree bindings
>>>> +
>>>> +maintainers:
>>>> +  - Sumit Gupta<sumitg@nvidia.com>
>>>> +
>>>> +description: |+
>>>> +  Control Backbone (CBB) comprises of the physical path from an
>>>> +  initiator to a target's register configuration space.
>>>> +  CBB2.0 consists of multiple sub-blocks connected to each other
>>>> +  to create a topology.
>>>> +  Tegra234 SOC has different fabrics based on CBB2.0 architecture
>>>> +  which include cluster fabrics BPMP, AON, PSC, SCE, RCE, DCE, FSI
>>>> +  and "CBB central fabric".
>>>> +
>>>> +  In CBB2.0, each initiator which can issue transactions connects to
>>>> +  a Root Master Node (MN) before it connects to any other element of
>>>> +  the fabric. Each Root MN contains a Error Monitor (EM) which detects
>>>> +  and logs error. Interrupts from various EM blocks are collated by
>>>> +  Error Notifier (EN) which is per fabric and presents a single
>>>> +  interrupt from fabric to the SOC interrupt controller.
>>>> +
>>>> +  The driver handles errors from CBB due to illegal register accesses
>>>> +  and prints debug information about failed transaction on receiving
>>>> +  the interrupt from EN. Debug information includes Error Code, Error
>>>> +  Description, MasterID, Fabric, SlaveID, Address, Cache, Protection,
>>>> +  Security Group etc on receiving error notification.
>>>> +
>>>> +  If the Error Response Disable (ERD) is set/enabled for an initiator,
>>>> +  then SError or Data abort exception error response is masked and an
>>>> +  interrupt is used for reporting errors due to illegal accesses from
>>>> +  that initiator. The value returned on read failures is '0xFFFFFFFF'
>>>> +  for compatibility with PCIE.
>>>> +
>>>> +properties:
>>>> +  $nodename:
>>>> +    pattern: "^[a-f]+-en@[0-9a-f]+$"
>>>> +
>>>> +  compatible:
>>>> +    enum:
>>>> +      - nvidia,tegra234-aon-fabric
>>>> +      - nvidia,tegra234-bpmp-fabric
>>>> +      - nvidia,tegra234-cbb-fabric
>>>> +      - nvidia,tegra234-dce-fabric
>>>> +      - nvidia,tegra234-rce-fabric
>>>> +      - nvidia,tegra234-sce-fabric
>>>> +
>>>> +  reg:
>>>> +    maxItems: 1
>>>> +
>>>> +  interrupts:
>>>> +    maxItems: 1
>>>> +    items:
>>>> +      - description: secure interrupt from error notifier.
>>>> +
>>>> +  nvidia,err-notifier-base:
>>>> +    description: address of error notifier inside a fabric.
>>>> +
>>>> +  nvidia,off-mask-erd:
>>>> +    description: offset of register having ERD bit.
>>> I was wondering about these two properties. Do we really need them? I
>>> see that they are set on a per-SoC basic and they only differ between
>>> the various fabrics. If they don't need to be configured on a per-board
>>> basis, then I don't think we need to specify these explicitly. Instead I
>>> think we could derive them from the compatible string
>> The CBB 2.0 based fabric's error handling driver remains same across
>> different SOC's and their variants. Only these fields change.
>> e.g: "off-mask-erd" value is different for T23x SOC variants.
>> "err-notifier-base" also changed multiple times during simulator stage.
>> So, keeping them in DT to avoid changing the driver code for different
>> variants of an SOC and to change them during bring up stages with DT change
>> only.
> For different SoC variants I would expect this to be implied by a new
> compatible string. A hypothetical Tegra235 SoC that is largely the same
> as Tegra234 but required slight changes in these values would also get a
> different set of compatible strings. So the fabrics in that case would
> be called:
> 
> 	- nvidia,tegra235-aon-fabric
> 	- nvidia,tegra235-bpmp-fabric
> 	- nvidia,tegra235-cbb-fabric
> 	...
> 
> and then that new value can be derived from that new compatible string.
> In general we only want to provide data in device tree if it can't be
> implied from the compatible string. Most of the time that's only for
> things that are somehow dependent on the board design. Data that is
> fixed for a given SoC can be derived from the compatible string.
> 
Ok. Will move them from DT to driver and send v2 tomorrow.

> Thierry
>