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

Sumit Gupta
2021-12-16T18:35:38Z

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