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

[Sumit Gupta <sumitg@xxxxxxxxxx>]

> > > 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@xxxxxxxxxx>
> > > > ---
> > > >    .../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@xxxxxxxxxx>
> > > > +
> > > > +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