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@xxxxxxxxxx>
> Sent: 08 May 2023 17:04
> To: Peter De Schrijver; thierry.reding@xxxxxxxxx; Jonathan Hunter
> Cc: robh+dt@xxxxxxxxxx; krzysztof.kozlowski+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-tegra@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; 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@xxxxxxxxxx>
>> Signed-off-by: Stefan Kristiansson <stefank@xxxxxxxxxx>
>> Signed-off-by: Peter De Schrijver <pdeschrijver@xxxxxxxxxx>
>> ---
>>  .../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