| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <c4fc1963-f4d5-8852-2119-90a631a41a0d@kapsi.fi>
Date: Tue, 17 May 2022 11:18:42 +0300
From: Mikko Perttunen <cyndis@...si.fi>
To: Rob Herring <robh@...nel.org>
Cc: thierry.reding@...il.com, jonathanh@...dia.com,
krzysztof.kozlowski+dt@...aro.org, digetx@...il.com,
dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org,
linux-tegra@...r.kernel.org, linux-kernel@...r.kernel.org,
Mikko Perttunen <mperttunen@...dia.com>
Subject: Re: [PATCH v1 01/13] dt-bindings: Add bindings for Tegra234 Host1x
and VIC
On 5/16/22 19:33, Rob Herring wrote:
> On Mon, May 16, 2022 at 01:02:01PM +0300, cyndis@...si.fi wrote:
>> From: Mikko Perttunen <mperttunen@...dia.com>
>>
>> Update VIC and Host1x bindings for changes in Tegra234.
>>
>> Namely,
>> - New compatible strings
>> - Sharded syncpoint interrupts
>> - Optional reset.
>>
>> Signed-off-by: Mikko Perttunen <mperttunen@...dia.com>
>> ---
>> .../display/tegra/nvidia,tegra124-vic.yaml | 1 +
>> .../display/tegra/nvidia,tegra20-host1x.yaml | 108 +++++++++++++++---
>> 2 files changed, 95 insertions(+), 14 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra124-vic.yaml b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra124-vic.yaml
>> index 37bb5ddc1963..7200095ef19e 100644
>> --- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra124-vic.yaml
>> +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra124-vic.yaml
>> @@ -21,6 +21,7 @@ properties:
>> - nvidia,tegra210-vic
>> - nvidia,tegra186-vic
>> - nvidia,tegra194-vic
>> + - nvidia,tegra234-vic
>>
>> - items:
>> - const: nvidia,tegra132-vic
>> diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
>> index 0adeb03b9e3a..83c58b7dae98 100644
>> --- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
>> +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
>> @@ -24,6 +24,7 @@ properties:
>> - nvidia,tegra210-host1x
>> - nvidia,tegra186-host1x
>> - nvidia,tegra194-host1x
>> + - nvidia,tegra234-host1x
>>
>> - items:
>> - const: nvidia,tegra132-host1x
>> @@ -31,23 +32,19 @@ properties:
>>
>> reg:
>> minItems: 1
>> - maxItems: 2
>> + maxItems: 3
>>
>> reg-names:
>> minItems: 1
>> - maxItems: 2
>> + maxItems: 3
>>
>> interrupts:
>> - items:
>> - - description: host1x syncpoint interrupt
>> - - description: host1x general interrupt
>> minItems: 1
>> + maxItems: 9
>>
>> interrupt-names:
>> - items:
>> - - const: syncpt
>> - - const: host1x
>> minItems: 1
>> + maxItems: 9
>>
>> '#address-cells':
>> description: The number of cells used to represent physical base addresses
>> @@ -110,13 +107,32 @@ required:
>> - reg
>> - clocks
>> - clock-names
>> - - resets
>> - - reset-names
>
> Shouldn't these still be required on some platforms?
Yes, I'll add them back in the tegra20..tegra210 conditional.
>
>>
>> additionalProperties:
>> type: object
>>
>> allOf:
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + enum:
>> + - nvidia,tegra20-host1x
>> + - nvidia,tegra30-host1x
>> + - nvidia,tegra114-host1x
>> + - nvidia,tegra124-host1x
>> + - nvidia,tegra210-host1x
>> + then:
>> + properties:
>> + interrupts:
>> + items:
>> + - description: host1x syncpoint interrupt
>> + - description: host1x general interrupt
>> +
>> + interrupt-names:
>> + items:
>> + - const: syncpt
>> + - const: host1x
>> - if:
>> properties:
>> compatible:
>> @@ -133,10 +149,10 @@ allOf:
>>
>> reg:
>> items:
>> - - description: physical base address and length of the register
>> - region assigned to the VM
>> - description: physical base address and length of the register
>> region used by the hypervisor
>> + - description: physical base address and length of the register
>> + region assigned to the VM
>
> You can't just change the order at least without a good explanation why
> in the commit message. It's an ABI.
Yeah, this doesn't change ABI, it's just a documentation bugfix, but
indeed I should have mentioned it in the commit message. In 'reg-names'
the order is given as 'hypervisor, vm' and the descriptions here were
the wrong way around.
>
>>
>> resets:
>> maxItems: 1
>> @@ -144,6 +160,70 @@ allOf:
>> reset-names:
>> maxItems: 1
>>
>> + interrupts:
>> + items:
>> + - description: host1x syncpoint interrupt
>> + - description: host1x general interrupt
>> +
>> + interrupt-names:
>> + items:
>> + - const: syncpt
>> + - const: host1x
>> +
>> + iommu-map:
>> + description: Specification of stream IDs available for memory context device
>> + use. Should be a mapping of IDs 0..n to IOMMU entries corresponding to
>> + usable stream IDs.
>> +
>> + required:
>> + - reg-names
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + enum:
>> + - nvidia,tegra234-host1x
>> + then:
>> + properties:
>> + reg-names:
>> + items:
>> + - const: common
>> + - const: hypervisor
>> + - const: vm
>> +
>> + reg:
>> + items:
>> + - description: physical base address and length of the register
>> + region used by host1x server
>> + - description: physical base address and length of the register
>> + region used by the hypervisor
>> + - description: physical base address and length of the register
>> + region assigned to the VM
>
> I guess this is just copied, but 'physical base address and length of
> the ' is redundant. That's every 'reg'.
I'll fix these up in the next revision.
Thanks,
Mikko
>
>> +
>> + interrupts:
>> + items:
>> + - description: host1x syncpoint interrupt 0
>> + - description: host1x syncpoint interrupt 1
>> + - description: host1x syncpoint interrupt 2
>> + - description: host1x syncpoint interrupt 3
>> + - description: host1x syncpoint interrupt 4
>> + - description: host1x syncpoint interrupt 5
>> + - description: host1x syncpoint interrupt 6
>> + - description: host1x syncpoint interrupt 7
>> + - description: host1x general interrupt
>> +
>> + interrupt-names:
>> + items:
>> + - const: syncpt0
>> + - const: syncpt1
>> + - const: syncpt2
>> + - const: syncpt3
>> + - const: syncpt4
>> + - const: syncpt5
>> + - const: syncpt6
>> + - const: syncpt7
>> + - const: host1x
>> +
>> iommu-map:
>> description: Specification of stream IDs available for memory context device
>> use. Should be a mapping of IDs 0..n to IOMMU entries corresponding to
>> @@ -160,8 +240,8 @@ examples:
>> host1x@...00000 {
>> compatible = "nvidia,tegra20-host1x";
>> reg = <0x50000000 0x00024000>;
>> - interrupts = <0 65 0x04 /* mpcore syncpt */
>> - 0 67 0x04>; /* mpcore general */
>> + interrupts = <0 65 0x04>, /* mpcore syncpt */
>> + <0 67 0x04>; /* mpcore general */
>> interrupt-names = "syncpt", "host1x";
>> clocks = <&tegra_car TEGRA20_CLK_HOST1X>;
>> clock-names = "host1x";
>> --
>> 2.36.1
>>
>>
Powered by blists - more mailing lists