[PATCH 1/5] dt-bindings: mailbox: tegra: Document Tegra264 HSP

[PATCH 1/5] dt-bindings: mailbox: tegra: Document Tegra264 HSP

[Peter De Schrijver]

Add the compatible string for the HSP block found on the Tegra264 SoC.

Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
---
 .../devicetree/bindings/mailbox/nvidia,tegra186-hsp.yaml         | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/mailbox/nvidia,tegra186-hsp.yaml b/Documentation/devicetree/bindings/mailbox/nvidia,tegra186-hsp.yaml
index a3e87516d637..2d14fc948999 100644
--- a/Documentation/devicetree/bindings/mailbox/nvidia,tegra186-hsp.yaml
+++ b/Documentation/devicetree/bindings/mailbox/nvidia,tegra186-hsp.yaml
@@ -66,6 +66,7 @@ properties:
     oneOf:
       - const: nvidia,tegra186-hsp
       - const: nvidia,tegra194-hsp
+      - const: nvidia,tegra264-hsp
       - items:
           - const: nvidia,tegra234-hsp
           - const: nvidia,tegra194-hsp
-- 
2.40.0

Re: [PATCH 1/5] dt-bindings: mailbox: tegra: Document Tegra264 HSP

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 614 bytes --]

On Mon, May 08, 2023 at 03:20:46PM +0300, Peter De Schrijver wrote:
> Add the compatible string for the HSP block found on the Tegra264 SoC.
> 
> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> ---
>  .../devicetree/bindings/mailbox/nvidia,tegra186-hsp.yaml         | 1 +
>  1 file changed, 1 insertion(+)

Might be a good idea to say in the commit message that this isn't
backwards compatible with Tegra194/Tegra234, hence no fallback
compatible string. That's already in the commit message for the driver,
so either way is fine with me:

Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/5] dt-bindings: mailbox: tegra: Document Tegra264 HSP

[Krzysztof Kozlowski]

On 08/05/2023 14:20, Peter De Schrijver wrote:
> Add the compatible string for the HSP block found on the Tegra264 SoC.
> 
> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> ---
>  .../devicetree/bindings/mailbox/nvidia,tegra186-hsp.yaml         | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/mailbox/nvidia,tegra186-hsp.yaml b/Documentation/devicetree/bindings/mailbox/nvidia,tegra186-hsp.yaml
> index a3e87516d637..2d14fc948999 100644
> --- a/Documentation/devicetree/bindings/mailbox/nvidia,tegra186-hsp.yaml
> +++ b/Documentation/devicetree/bindings/mailbox/nvidia,tegra186-hsp.yaml
> @@ -66,6 +66,7 @@ properties:
>      oneOf:
>        - const: nvidia,tegra186-hsp
>        - const: nvidia,tegra194-hsp
> +      - const: nvidia,tegra264-hsp

I didn't get other patches, so I assume you know what you are doing and
the device is not compatible with others (see entire context of that diff).

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

[PATCH 4/5] dt-bindings: Add bindings to support DRAM MRQ GSCs

[Peter De Schrijver]

Add bindings for DRAM MRQ GSC support.

Co-developed-by: Stefan Kristiansson <stefank@nvidia.com>
Signed-off-by: Stefan Kristiansson <stefank@nvidia.com>
Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
---
 .../firmware/nvidia,tegra186-bpmp.yaml        | 69 ++++++++++++++++++-
 .../nvidia,tegra264-bpmp-shmem.yaml           | 40 +++++++++++
 2 files changed, 106 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/reserved-memory/nvidia,tegra264-bpmp-shmem.yaml

diff --git a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.yaml b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.yaml
index 833c07f1685c..d818cfe1d783 100644
--- a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.yaml
+++ b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.yaml
@@ -57,8 +57,11 @@ description: |
   "#address-cells" or "#size-cells" property.
 
   The shared memory area for the IPC TX and RX between CPU and BPMP are
-  predefined and work on top of sysram, which is an SRAM inside the
-  chip. See ".../sram/sram.yaml" for the bindings.
+  predefined and work on top of either sysram, which is an SRAM inside the
+  chip, or in normal SDRAM.
+  See ".../sram/sram.yaml" for the bindings for the SRAM case.
+  See "../reserved-memory/nvidia,tegra264-bpmp-shmem.yaml" for bindings for
+  the SDRAM case.
 
 properties:
   compatible:
@@ -81,6 +84,11 @@ properties:
     minItems: 2
     maxItems: 2
 
+  memory-region:
+    description: phandle to reserved memory region used for IPC between
+      CPU-NS and BPMP.
+    maxItems: 1
+
   "#clock-cells":
     const: 1
 
@@ -115,10 +123,16 @@ properties:
 
 additionalProperties: false
 
+allOf:
+  - oneOf:
+      - required:
+          - memory-region
+      - required:
+          - shmem
+
 required:
   - compatible
   - mboxes
-  - shmem
   - "#clock-cells"
   - "#power-domain-cells"
   - "#reset-cells"
@@ -184,3 +198,52 @@ examples:
             #thermal-sensor-cells = <1>;
         };
     };
+
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/mailbox/tegra186-hsp.h>
+    #include <dt-bindings/memory/tegra186-mc.h>
+
+    hsp_top0: hsp@3c00000 {
+        compatible = "nvidia,tegra186-hsp";
+        reg = <0x03c00000 0xa0000>;
+        interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>;
+        interrupt-names = "doorbell";
+        #mbox-cells = <2>;
+    };
+
+    reserved-memory {
+        dram_cpu_bpmp_mail: shmem@f1be0000  {
+            compatible = "nvidia,tegra264-bpmp-shmem";
+            reg = <0x0 0xf1be0000 0x0 0x2000>;
+            no-map;
+        };
+    };
+
+    bpmp {
+        compatible = "nvidia,tegra186-bpmp";
+        interconnects = <&mc TEGRA186_MEMORY_CLIENT_BPMPR &emc>,
+                        <&mc TEGRA186_MEMORY_CLIENT_BPMPW &emc>,
+                        <&mc TEGRA186_MEMORY_CLIENT_BPMPDMAR &emc>,
+                        <&mc TEGRA186_MEMORY_CLIENT_BPMPDMAW &emc>;
+        interconnect-names = "read", "write", "dma-mem", "dma-write";
+        iommus = <&smmu TEGRA186_SID_BPMP>;
+        mboxes = <&hsp_top0 TEGRA_HSP_MBOX_TYPE_DB
+                            TEGRA_HSP_DB_MASTER_BPMP>;
+        memory-region = <&dram_cpu_bpmp_mail>;
+        #clock-cells = <1>;
+        #power-domain-cells = <1>;
+        #reset-cells = <1>;
+
+        i2c {
+            compatible = "nvidia,tegra186-bpmp-i2c";
+            nvidia,bpmp-bus-id = <5>;
+            #address-cells = <1>;
+            #size-cells = <0>;
+        };
+
+        thermal {
+            compatible = "nvidia,tegra186-bpmp-thermal";
+            #thermal-sensor-cells = <1>;
+        };
+    };
diff --git a/Documentation/devicetree/bindings/reserved-memory/nvidia,tegra264-bpmp-shmem.yaml b/Documentation/devicetree/bindings/reserved-memory/nvidia,tegra264-bpmp-shmem.yaml
new file mode 100644
index 000000000000..6cd9a61cd31f
--- /dev/null
+++ b/Documentation/devicetree/bindings/reserved-memory/nvidia,tegra264-bpmp-shmem.yaml
@@ -0,0 +1,40 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reserved-memory/nvidia,tegra264-bpmp-shmem.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Tegra CPU-NS - BPMP IPC reserved memory binding
+
+maintainers:
+ - Peter De Schrijver <pdeschrijver@nvidia.com>
+
+description: |
+  Define a memory region used for communication between CPU-NS and BPMP.
+  Typically this node is created by the bootloader as the physical address
+  has to be known to both CPU-NS and BPMP for correct IPC operation.
+  The memory region is defined using a child node under /reserved-memory.
+  The sub-node is named shmem@<address>.
+
+properties:
+  compatible:
+    const: nvidia,tegra264-bpmp-shmem
+
+  reg:
+    description: The physical address and size of the shared SDRAM region
+
+required:
+  - compatible
+  - reg
+  - no-map
+
+examples:
+  - |
+    reserved-memory {
+       dram_cpu_bpmp_mail: shmem@f1be0000  {
+           compatible = "nvidia,tegra264-bpmp-shmem";
+           reg = <0x0 0xf1be0000 0x0 0x2000>;
+           no-map;
+       };
+    };
+...
-- 
2.40.0

Re: [PATCH 4/5] dt-bindings: Add bindings to support DRAM MRQ GSCs

[Krzysztof Kozlowski]

On Mon, 08 May 2023 15:20:52 +0300, Peter De Schrijver wrote:
> Add bindings for DRAM MRQ GSC support.
> 
> Co-developed-by: Stefan Kristiansson <stefank@nvidia.com>
> Signed-off-by: Stefan Kristiansson <stefank@nvidia.com>
> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> ---
>  .../firmware/nvidia,tegra186-bpmp.yaml        | 69 ++++++++++++++++++-
>  .../nvidia,tegra264-bpmp-shmem.yaml           | 40 +++++++++++
>  2 files changed, 106 insertions(+), 3 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/reserved-memory/nvidia,tegra264-bpmp-shmem.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/reserved-memory/nvidia,tegra264-bpmp-shmem.yaml:10:2: [warning] wrong indentation: expected 2 but found 1 (indentation)

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/reserved-memory/nvidia,tegra264-bpmp-shmem.yaml: 'oneOf' conditional failed, one must be fixed:
	'unevaluatedProperties' is a required property
	'additionalProperties' is a required property
	hint: Either unevaluatedProperties or additionalProperties must be present
	from schema $id: http://devicetree.org/meta-schemas/core.yaml#
Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.example.dts:110.31-116.11: ERROR (duplicate_label): /example-1/hsp@3c00000: Duplicate label 'hsp_top0' on /example-1/hsp@3c00000 and /example-0/hsp@3c00000
ERROR: Input tree has errors, aborting (use -f to force output)
make[1]: *** [scripts/Makefile.lib:419: Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.example.dtb] Error 2
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1512: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):
Documentation/usb/gadget_uvc.rst: Documentation/userspace-api/media/v4l/pixfmt-packed.yuv.rst
MAINTAINERS: Documentation/devicetree/bindings/pwm/pwm-apple.yaml

See https://patchwork.ozlabs.org/patch/1778345

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH 4/5] dt-bindings: Add bindings to support DRAM MRQ GSCs

[Peter De Schrijver]

No idea what the second error is about. Documentation/devicetree/bindings/reserved-memory/nvidia,tegra264-bpmp-shmem.yaml does not have a oneOf conditional?

Peter.

________________________________________
From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Sent: 08 May 2023 16:33
To: Peter De Schrijver
Cc: thierry.reding@gmail.com; krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org; linux-kernel@vger.kernel.org; robh+dt@kernel.org; Jonathan Hunter; linux-tegra@vger.kernel.org; devicetree@vger.kernel.org; Stefan Kristiansson
Subject: Re: [PATCH 4/5] dt-bindings: Add bindings to support DRAM MRQ GSCs

On Mon, 08 May 2023 15:20:52 +0300, Peter De Schrijver wrote:
> Add bindings for DRAM MRQ GSC support.
>
> Co-developed-by: Stefan Kristiansson <stefank@nvidia.com>
> Signed-off-by: Stefan Kristiansson <stefank@nvidia.com>
> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> ---
>  .../firmware/nvidia,tegra186-bpmp.yaml        | 69 ++++++++++++++++++-
>  .../nvidia,tegra264-bpmp-shmem.yaml           | 40 +++++++++++
>  2 files changed, 106 insertions(+), 3 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/reserved-memory/nvidia,tegra264-bpmp-shmem.yaml
>

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/reserved-memory/nvidia,tegra264-bpmp-shmem.yaml:10:2: [warning] wrong indentation: expected 2 but found 1 (indentation)

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/reserved-memory/nvidia,tegra264-bpmp-shmem.yaml: 'oneOf' conditional failed, one must be fixed:
        'unevaluatedProperties' is a required property
        'additionalProperties' is a required property
        hint: Either unevaluatedProperties or additionalProperties must be present
        from schema $id: http://devicetree.org/meta-schemas/core.yaml#
Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.example.dts:110.31-116.11: ERROR (duplicate_label): /example-1/hsp@3c00000: Duplicate label 'hsp_top0' on /example-1/hsp@3c00000 and /example-0/hsp@3c00000
ERROR: Input tree has errors, aborting (use -f to force output)
make[1]: *** [scripts/Makefile.lib:419: Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.example.dtb] Error 2
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1512: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):
Documentation/usb/gadget_uvc.rst: Documentation/userspace-api/media/v4l/pixfmt-packed.yuv.rst
MAINTAINERS: Documentation/devicetree/bindings/pwm/pwm-apple.yaml

See https://patchwork.ozlabs.org/patch/1778345

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

--
nvpublic

Re: [PATCH 4/5] dt-bindings: Add bindings to support DRAM MRQ GSCs

[Mikko Perttunen]

On 5/8/23 16:52, Peter De Schrijver wrote:
> No idea what the second error is about. Documentation/devicetree/bindings/reserved-memory/nvidia,tegra264-bpmp-shmem.yaml does not have a oneOf conditional?
> 
> Peter.

The oneOf conditional would be from the core schema. Also, no top posting.

Cheers,
Mikko

> 
> ________________________________________
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: 08 May 2023 16:33
> To: Peter De Schrijver
> Cc: thierry.reding@gmail.com; krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org; linux-kernel@vger.kernel.org; robh+dt@kernel.org; Jonathan Hunter; linux-tegra@vger.kernel.org; devicetree@vger.kernel.org; Stefan Kristiansson
> Subject: Re: [PATCH 4/5] dt-bindings: Add bindings to support DRAM MRQ GSCs
> 
> On Mon, 08 May 2023 15:20:52 +0300, Peter De Schrijver wrote:
>> Add bindings for DRAM MRQ GSC support.
>>
>> Co-developed-by: Stefan Kristiansson <stefank@nvidia.com>
>> Signed-off-by: Stefan Kristiansson <stefank@nvidia.com>
>> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
>> ---
>>   .../firmware/nvidia,tegra186-bpmp.yaml        | 69 ++++++++++++++++++-
>>   .../nvidia,tegra264-bpmp-shmem.yaml           | 40 +++++++++++
>>   2 files changed, 106 insertions(+), 3 deletions(-)
>>   create mode 100644 Documentation/devicetree/bindings/reserved-memory/nvidia,tegra264-bpmp-shmem.yaml
>>
> 
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> yamllint warnings/errors:
> ./Documentation/devicetree/bindings/reserved-memory/nvidia,tegra264-bpmp-shmem.yaml:10:2: [warning] wrong indentation: expected 2 but found 1 (indentation)
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/reserved-memory/nvidia,tegra264-bpmp-shmem.yaml: 'oneOf' conditional failed, one must be fixed:
>          'unevaluatedProperties' is a required property
>          'additionalProperties' is a required property
>          hint: Either unevaluatedProperties or additionalProperties must be present
>          from schema $id: http://devicetree.org/meta-schemas/core.yaml#
> Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.example.dts:110.31-116.11: ERROR (duplicate_label): /example-1/hsp@3c00000: Duplicate label 'hsp_top0' on /example-1/hsp@3c00000 and /example-0/hsp@3c00000
> ERROR: Input tree has errors, aborting (use -f to force output)
> make[1]: *** [scripts/Makefile.lib:419: Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.example.dtb] Error 2
> make[1]: *** Waiting for unfinished jobs....
> make: *** [Makefile:1512: dt_binding_check] Error 2
> 
> doc reference errors (make refcheckdocs):
> Documentation/usb/gadget_uvc.rst: Documentation/userspace-api/media/v4l/pixfmt-packed.yuv.rst
> MAINTAINERS: Documentation/devicetree/bindings/pwm/pwm-apple.yaml
> 
> See https://patchwork.ozlabs.org/patch/1778345
> 
> This check can fail if there are any dependencies. The base for a patch
> series is generally the most recent rc1.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit.
> 
> --
> nvpublic

Re: [PATCH 4/5] dt-bindings: Add bindings to support DRAM MRQ GSCs

[Krzysztof Kozlowski]

On 08/05/2023 15:52, Peter De Schrijver wrote:
> No idea what the second error is about. Documentation/devicetree/bindings/reserved-memory/nvidia,tegra264-bpmp-shmem.yaml does not have a oneOf conditional?
> 

Are you talking about the one with following explanation:

'unevaluatedProperties' is a required property
'additionalProperties' is a required property
hint: Either unevaluatedProperties or additionalProperties must be present

? Isn't it self-explanatory?

Best regards,
Krzysztof

Re: [PATCH 4/5] dt-bindings: Add bindings to support DRAM MRQ GSCs

[Krzysztof Kozlowski]

On 08/05/2023 14:20, Peter De Schrijver wrote:
> Add bindings for DRAM MRQ GSC support.
> 
> Co-developed-by: Stefan Kristiansson <stefank@nvidia.com>
> Signed-off-by: Stefan Kristiansson <stefank@nvidia.com>
> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> ---
>  .../firmware/nvidia,tegra186-bpmp.yaml        | 69 ++++++++++++++++++-
>  .../nvidia,tegra264-bpmp-shmem.yaml           | 40 +++++++++++

Why touching two files?

>  2 files changed, 106 insertions(+), 3 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/reserved-memory/nvidia,tegra264-bpmp-shmem.yaml
> 
> diff --git a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.yaml b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.yaml
> index 833c07f1685c..d818cfe1d783 100644
> --- a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.yaml
> +++ b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.yaml
> @@ -57,8 +57,11 @@ description: |
>    "#address-cells" or "#size-cells" property.
>  
>    The shared memory area for the IPC TX and RX between CPU and BPMP are
> -  predefined and work on top of sysram, which is an SRAM inside the
> -  chip. See ".../sram/sram.yaml" for the bindings.
> +  predefined and work on top of either sysram, which is an SRAM inside the
> +  chip, or in normal SDRAM.
> +  See ".../sram/sram.yaml" for the bindings for the SRAM case.
> +  See "../reserved-memory/nvidia,tegra264-bpmp-shmem.yaml" for bindings for
> +  the SDRAM case.
>  
>  properties:
>    compatible:
> @@ -81,6 +84,11 @@ properties:
>      minItems: 2
>      maxItems: 2
>  
> +  memory-region:
> +    description: phandle to reserved memory region used for IPC between
> +      CPU-NS and BPMP.
> +    maxItems: 1
> +
>    "#clock-cells":
>      const: 1
>  
> @@ -115,10 +123,16 @@ properties:
>  
>  additionalProperties: false
>  
> +allOf:
> +  - oneOf:

Keep just oneOf and drop allOf.

> +      - required:
> +          - memory-region
> +      - required:
> +          - shmem
> +
>  required:
>    - compatible
>    - mboxes
> -  - shmem
>    - "#clock-cells"
>    - "#power-domain-cells"
>    - "#reset-cells"
> @@ -184,3 +198,52 @@ examples:
>              #thermal-sensor-cells = <1>;
>          };
>      };
> +
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/mailbox/tegra186-hsp.h>
> +    #include <dt-bindings/memory/tegra186-mc.h>
> +
> +    hsp_top0: hsp@3c00000 {
> +        compatible = "nvidia,tegra186-hsp";
> +        reg = <0x03c00000 0xa0000>;
> +        interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>;
> +        interrupt-names = "doorbell";
> +        #mbox-cells = <2>;

Why HSP example is here?

> +    };
> +
> +    reserved-memory {
> +        dram_cpu_bpmp_mail: shmem@f1be0000  {
> +            compatible = "nvidia,tegra264-bpmp-shmem";
> +            reg = <0x0 0xf1be0000 0x0 0x2000>;
> +            no-map;
> +        };
> +    };

Drop, fairly obvious and should be in that binding, not here.

> +
> +    bpmp {
> +        compatible = "nvidia,tegra186-bpmp";
> +        interconnects = <&mc TEGRA186_MEMORY_CLIENT_BPMPR &emc>,
> +                        <&mc TEGRA186_MEMORY_CLIENT_BPMPW &emc>,
> +                        <&mc TEGRA186_MEMORY_CLIENT_BPMPDMAR &emc>,
> +                        <&mc TEGRA186_MEMORY_CLIENT_BPMPDMAW &emc>;
> +        interconnect-names = "read", "write", "dma-mem", "dma-write";
> +        iommus = <&smmu TEGRA186_SID_BPMP>;
> +        mboxes = <&hsp_top0 TEGRA_HSP_MBOX_TYPE_DB
> +                            TEGRA_HSP_DB_MASTER_BPMP>;
> +        memory-region = <&dram_cpu_bpmp_mail>;
> +        #clock-cells = <1>;
> +        #power-domain-cells = <1>;
> +        #reset-cells = <1>;
> +
> +        i2c {
> +            compatible = "nvidia,tegra186-bpmp-i2c";
> +            nvidia,bpmp-bus-id = <5>;
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +        };
> +
> +        thermal {
> +            compatible = "nvidia,tegra186-bpmp-thermal";
> +            #thermal-sensor-cells = <1>;
> +        };
> +    };
> diff --git a/Documentation/devicetree/bindings/reserved-memory/nvidia,tegra264-bpmp-shmem.yaml b/Documentation/devicetree/bindings/reserved-memory/nvidia,tegra264-bpmp-shmem.yaml
> new file mode 100644
> index 000000000000..6cd9a61cd31f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reserved-memory/nvidia,tegra264-bpmp-shmem.yaml
> @@ -0,0 +1,40 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/reserved-memory/nvidia,tegra264-bpmp-shmem.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Tegra CPU-NS - BPMP IPC reserved memory binding

Drop "binding"

> +
> +maintainers:
> + - Peter De Schrijver <pdeschrijver@nvidia.com>
> +
> +description: |
> +  Define a memory region used for communication between CPU-NS and BPMP.
> +  Typically this node is created by the bootloader as the physical address
> +  has to be known to both CPU-NS and BPMP for correct IPC operation.
> +  The memory region is defined using a child node under /reserved-memory.
> +  The sub-node is named shmem@<address>.
> +

Open other files there and implement it similar way. I really wonder why
this should be done differently than for example other nvidia stuff -
without reserved-memory schema?

> +properties:
> +  compatible:
> +    const: nvidia,tegra264-bpmp-shmem
> +
> +  reg:
> +    description: The physical address and size of the shared SDRAM region
> +
> +required:
> +  - compatible
> +  - reg
> +  - no-map
> +

Does not look like you tested the bindings. Please run `make
dt_binding_check` (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).

> +examples:
> +  - |
> +    reserved-memory {
> +       dram_cpu_bpmp_mail: shmem@f1be0000  {
> +           compatible = "nvidia,tegra264-bpmp-shmem";
> +           reg = <0x0 0xf1be0000 0x0 0x2000>;
> +           no-map;
> +       };
> +    };
> +...

Best regards,
Krzysztof

Re: [PATCH 4/5] dt-bindings: Add bindings to support DRAM MRQ GSCs

[Peter De Schrijver]

________________________________________
From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Sent: 08 May 2023 17:04
To: Peter De Schrijver; thierry.reding@gmail.com; Jonathan Hunter
Cc: robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org; devicetree@vger.kernel.org; linux-tegra@vger.kernel.org; linux-kernel@vger.kernel.org; Stefan Kristiansson
Subject: Re: [PATCH 4/5] dt-bindings: Add bindings to support DRAM MRQ GSCs

On 08/05/2023 14:20, Peter De Schrijver wrote:
> Add bindings for DRAM MRQ GSC support.
>
> Co-developed-by: Stefan Kristiansson <stefank@nvidia.com>
> Signed-off-by: Stefan Kristiansson <stefank@nvidia.com>
> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> ---
>  .../firmware/nvidia,tegra186-bpmp.yaml        | 69 ++++++++++++++++++-
>  .../nvidia,tegra264-bpmp-shmem.yaml           | 40 +++++++++++

> Why touching two files?

Because both are needed to support having MRQ GSCs in DRAM.

>  2 files changed, 106 insertions(+), 3 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/reserved-memory/nvidia,tegra264-bpmp-shmem.yaml
>
> diff --git a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.yaml b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.yaml
> index 833c07f1685c..d818cfe1d783 100644
> --- a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.yaml
> +++ b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.yaml
> @@ -57,8 +57,11 @@ description: |
>    "#address-cells" or "#size-cells" property.
>
>    The shared memory area for the IPC TX and RX between CPU and BPMP are
> -  predefined and work on top of sysram, which is an SRAM inside the
> -  chip. See ".../sram/sram.yaml" for the bindings.
> +  predefined and work on top of either sysram, which is an SRAM inside the
> +  chip, or in normal SDRAM.
> +  See ".../sram/sram.yaml" for the bindings for the SRAM case.
> +  See "../reserved-memory/nvidia,tegra264-bpmp-shmem.yaml" for bindings for
> +  the SDRAM case.
>
>  properties:
>    compatible:
> @@ -81,6 +84,11 @@ properties:
>      minItems: 2
>      maxItems: 2
>
> +  memory-region:
> +    description: phandle to reserved memory region used for IPC between
> +      CPU-NS and BPMP.
> +    maxItems: 1
> +
>    "#clock-cells":
>      const: 1
>
> @@ -115,10 +123,16 @@ properties:
>
>  additionalProperties: false
>
> +allOf:
> +  - oneOf:

Keep just oneOf and drop allOf.

> +      - required:
> +          - memory-region
> +      - required:
> +          - shmem
> +
>  required:
>    - compatible
>    - mboxes
> -  - shmem
>    - "#clock-cells"
>    - "#power-domain-cells"
>    - "#reset-cells"
> @@ -184,3 +198,52 @@ examples:
>              #thermal-sensor-cells = <1>;
>          };
>      };
> +
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/mailbox/tegra186-hsp.h>
> +    #include <dt-bindings/memory/tegra186-mc.h>
> +
> +    hsp_top0: hsp@3c00000 {
> +        compatible = "nvidia,tegra186-hsp";
> +        reg = <0x03c00000 0xa0000>;
> +        interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>;
> +        interrupt-names = "doorbell";
> +        #mbox-cells = <2>;

> Why HSP example is here?

Because it's referred to further down the example.

> +    };
> +
> +    reserved-memory {
> +        dram_cpu_bpmp_mail: shmem@f1be0000  {
> +            compatible = "nvidia,tegra264-bpmp-shmem";
> +            reg = <0x0 0xf1be0000 0x0 0x2000>;
> +            no-map;
> +        };
> +    };

Drop, fairly obvious and should be in that binding, not here.

> +
> +    bpmp {
> +        compatible = "nvidia,tegra186-bpmp";
> +        interconnects = <&mc TEGRA186_MEMORY_CLIENT_BPMPR &emc>,
> +                        <&mc TEGRA186_MEMORY_CLIENT_BPMPW &emc>,
> +                        <&mc TEGRA186_MEMORY_CLIENT_BPMPDMAR &emc>,
> +                        <&mc TEGRA186_MEMORY_CLIENT_BPMPDMAW &emc>;
> +        interconnect-names = "read", "write", "dma-mem", "dma-write";
> +        iommus = <&smmu TEGRA186_SID_BPMP>;
> +        mboxes = <&hsp_top0 TEGRA_HSP_MBOX_TYPE_DB

^^^ refers to hsp_top0.

> +                            TEGRA_HSP_DB_MASTER_BPMP>;
> +        memory-region = <&dram_cpu_bpmp_mail>;
> +        #clock-cells = <1>;
> +        #power-domain-cells = <1>;
> +        #reset-cells = <1>;
> +
> +        i2c {
> +            compatible = "nvidia,tegra186-bpmp-i2c";
> +            nvidia,bpmp-bus-id = <5>;
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +        };
> +
> +        thermal {
> +            compatible = "nvidia,tegra186-bpmp-thermal";
> +            #thermal-sensor-cells = <1>;
> +        };
> +    };
> diff --git a/Documentation/devicetree/bindings/reserved-memory/nvidia,tegra264-bpmp-shmem.yaml b/Documentation/devicetree/bindings/reserved-memory/nvidia,tegra264-bpmp-shmem.yaml
> new file mode 100644
> index 000000000000..6cd9a61cd31f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reserved-memory/nvidia,tegra264-bpmp-shmem.yaml
> @@ -0,0 +1,40 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/reserved-memory/nvidia,tegra264-bpmp-shmem.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Tegra CPU-NS - BPMP IPC reserved memory binding

Drop "binding"

> +
> +maintainers:
> + - Peter De Schrijver <pdeschrijver@nvidia.com>
> +
> +description: |
> +  Define a memory region used for communication between CPU-NS and BPMP.
> +  Typically this node is created by the bootloader as the physical address
> +  has to be known to both CPU-NS and BPMP for correct IPC operation.
> +  The memory region is defined using a child node under /reserved-memory.
> +  The sub-node is named shmem@<address>.
> +

> Open other files there and implement it similar way. I really wonder why
> this should be done differently than for example other nvidia stuff -
> without reserved-memory schema?

Because up to now, the GSC was kept in sysram which is considered to be a device by the kernel. Now part of the DRAM will be reserved for this, so the kernel needs to know about it.

> +properties:
> +  compatible:
> +    const: nvidia,tegra264-bpmp-shmem
> +
> +  reg:
> +    description: The physical address and size of the shared SDRAM region
> +
> +required:
> +  - compatible
> +  - reg
> +  - no-map
> +

Does not look like you tested the bindings. Please run `make
dt_binding_check` (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).

> +examples:
> +  - |
> +    reserved-memory {
> +       dram_cpu_bpmp_mail: shmem@f1be0000  {
> +           compatible = "nvidia,tegra264-bpmp-shmem";
> +           reg = <0x0 0xf1be0000 0x0 0x2000>;
> +           no-map;
> +       };
> +    };
> +...

Best regards,
Krzysztof

Re: [PATCH 4/5] dt-bindings: Add bindings to support DRAM MRQ GSCs

[Krzysztof Kozlowski]

On 08/05/2023 16:12, Peter De Schrijver wrote:
> 
> 
> ________________________________________
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: 08 May 2023 17:04
> To: Peter De Schrijver; thierry.reding@gmail.com; Jonathan Hunter
> Cc: robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org; devicetree@vger.kernel.org; linux-tegra@vger.kernel.org; linux-kernel@vger.kernel.org; Stefan Kristiansson
> Subject: Re: [PATCH 4/5] dt-bindings: Add bindings to support DRAM MRQ GSCs
> 
> On 08/05/2023 14:20, Peter De Schrijver wrote:
>> Add bindings for DRAM MRQ GSC support.
>>
>> Co-developed-by: Stefan Kristiansson <stefank@nvidia.com>
>> Signed-off-by: Stefan Kristiansson <stefank@nvidia.com>
>> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
>> ---
>>  .../firmware/nvidia,tegra186-bpmp.yaml        | 69 ++++++++++++++++++-
>>  .../nvidia,tegra264-bpmp-shmem.yaml           | 40 +++++++++++
> 
>> Why touching two files?
> 
> Because both are needed to support having MRQ GSCs in DRAM.

Yeah, but why two in one commit? All patches are needed for full support
and they are not squashed into one.

> 
>>  2 files changed, 106 insertions(+), 3 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/reserved-memory/nvidia,tegra264-bpmp-shmem.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.yaml b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.yaml
>> index 833c07f1685c..d818cfe1d783 100644
>> --- a/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.yaml
>> +++ b/Documentation/devicetree/bindings/firmware/nvidia,tegra186-bpmp.yaml
>> @@ -57,8 +57,11 @@ description: |
>>    "#address-cells" or "#size-cells" property.
>>
>>    The shared memory area for the IPC TX and RX between CPU and BPMP are
>> -  predefined and work on top of sysram, which is an SRAM inside the
>> -  chip. See ".../sram/sram.yaml" for the bindings.
>> +  predefined and work on top of either sysram, which is an SRAM inside the
>> +  chip, or in normal SDRAM.
>> +  See ".../sram/sram.yaml" for the bindings for the SRAM case.
>> +  See "../reserved-memory/nvidia,tegra264-bpmp-shmem.yaml" for bindings for
>> +  the SDRAM case.
>>
>>  properties:
>>    compatible:
>> @@ -81,6 +84,11 @@ properties:
>>      minItems: 2
>>      maxItems: 2
>>
>> +  memory-region:
>> +    description: phandle to reserved memory region used for IPC between
>> +      CPU-NS and BPMP.
>> +    maxItems: 1
>> +
>>    "#clock-cells":
>>      const: 1
>>
>> @@ -115,10 +123,16 @@ properties:
>>
>>  additionalProperties: false
>>
>> +allOf:
>> +  - oneOf:
> 
> Keep just oneOf and drop allOf.

???

I wrote this. Fix your mailer.

> 
>> +      - required:
>> +          - memory-region
>> +      - required:
>> +          - shmem
>> +
>>  required:
>>    - compatible
>>    - mboxes
>> -  - shmem
>>    - "#clock-cells"
>>    - "#power-domain-cells"
>>    - "#reset-cells"
>> @@ -184,3 +198,52 @@ examples:
>>              #thermal-sensor-cells = <1>;
>>          };
>>      };
>> +
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    #include <dt-bindings/mailbox/tegra186-hsp.h>
>> +    #include <dt-bindings/memory/tegra186-mc.h>
>> +
>> +    hsp_top0: hsp@3c00000 {
>> +        compatible = "nvidia,tegra186-hsp";
>> +        reg = <0x03c00000 0xa0000>;
>> +        interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>;
>> +        interrupt-names = "doorbell";
>> +        #mbox-cells = <2>;
> 
>> Why HSP example is here?
> 
> Because it's referred to further down the example.

Not needed.

> 
>> +    };
>> +
>> +    reserved-memory {
>> +        dram_cpu_bpmp_mail: shmem@f1be0000  {
>> +            compatible = "nvidia,tegra264-bpmp-shmem";
>> +            reg = <0x0 0xf1be0000 0x0 0x2000>;
>> +            no-map;
>> +        };
>> +    };
> 
> Drop, fairly obvious and should be in that binding, not here.

???

Please use mailing list style of replies.

Best regards,
Krzysztof