In the device tree, there will be device node for the AI engine device,
and device nodes for the statically configured AI engine apertures.
Apertures are an isolated set of columns with in the AI engine device
with their own address space and interrupt.
Signed-off-by: Gregory Williams <gregory.williams@amd.com>
---
.../bindings/soc/xilinx/xlnx,ai-engine.yaml | 151 ++++++++++++++++++
1 file changed, 151 insertions(+)
create mode 100644 Documentation/devicetree/bindings/soc/xilinx/xlnx,ai-engine.yaml
diff --git a/Documentation/devicetree/bindings/soc/xilinx/xlnx,ai-engine.yaml b/Documentation/devicetree/bindings/soc/xilinx/xlnx,ai-engine.yaml
new file mode 100644
index 000000000000..7d9a36c56366
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/xilinx/xlnx,ai-engine.yaml
@@ -0,0 +1,151 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/xilinx/xlnx,ai-engine.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: AMD AI Engine
+
+maintainers:
+ - Gregory Williams <gregory.williams@amd.com>
+
+description:
+ The AMD AI Engine is a tile processor with many cores (up to 400) that
+ can run in parallel. The data routing between cores is configured through
+ internal switches, and shim tiles interface with external interconnect, such
+ as memory or PL. One AI engine device can have multiple apertures, each
+ has its own address space and interrupt. At runtime application can create
+ multiple partitions within an aperture which are groups of columns of AI
+ engine tiles. Each AI engine partition is the minimum resetable unit for an
+ AI engine application.
+
+properties:
+ compatible:
+ const: xlnx,ai-engine-v2.0
+
+ reg:
+ maxItems: 1
+
+ '#address-cells':
+ const: 2
+
+ '#size-cells':
+ const: 2
+
+ power-domains:
+ description:
+ Platform management node id used to request power management services
+ from the firmware driver.
+
+ xlnx,aie-gen:
+ $ref: /schemas/types.yaml#/definitions/uint8
+ description:
+ Hardware generation of AI engine device. E.g. the current values supported
+ are 1 (AIE) and 2 (AIEML).
+
+ xlnx,shim-rows:
+ $ref: /schemas/types.yaml#/definitions/uint8-array
+ description:
+ start row and the number of rows of SHIM tiles of the AI engine device
+
+ xlnx,core-rows:
+ $ref: /schemas/types.yaml#/definitions/uint8-array
+ description:
+ start row and the number of rows of core tiles of the AI engine device
+
+ xlnx,mem-rows:
+ $ref: /schemas/types.yaml#/definitions/uint8-array
+ description:
+ start row and the number of rows of memory tiles of the AI engine device
+
+required:
+ - compatible
+ - reg
+ - power-domains
+ - xlnx,aie-gen
+ - xlnx,shim-rows
+ - xlnx,core-rows
+ - xlnx,mem-rows
+
+patternProperties:
+ "^aperture@[0-9]+$":
+ type: object
+ description:
+ AI engine aperture which is a group of column based tiles of the
+ AI engine device. Each AI engine apertures isolated from the
+ other AI engine apertures. An AI engine aperture is defined by
+ AMD/Xilinx platform design tools.
+
+ properties:
+ compatible:
+ const: xlnx,ai-engine-aperture
+
+ reg:
+ description:
+ Physical base address and length of the aperture registers.
+ The AI engine address space assigned to Linux is defined by
+ Xilinx/AMD platform design tool.
+
+ interrupts:
+ maxItems: 3
+
+ interrupt-names:
+ items:
+ - const: interrupt1
+ - const: interrupt2
+ - const: interrupt3
+
+ xlnx,columns:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ description:
+ It describes the location of the aperture. It specifies the start
+ column and the number of columns. E.g. an aperture starts from
+ column 0 and there are 50 columns, it will be presented as <0 50>.
+
+ xlnx,node-id:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ AI engine aperture node ID, which is defined by AMD/Xilinx platform
+ design tool to identify the AI engine aperture in the firmware.
+
+ required:
+ - compatible
+ - reg
+ - xlnx,columns
+ - xlnx,node-id
+
+ additionalProperties: false
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/power/xlnx-versal-power.h>
+ bus {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ai_engine: ai-engine@20000000000 {
+ compatible = "xlnx,ai-engine-v2.0";
+ reg = <0x200 0x00 0x01 0x00>;
+ #address-cells = <2>;
+ #size-cells = <2>;
+ power-domains = <&versal_firmware PM_DEV_AI>;
+ xlnx,aie-gen = /bits/ 8 <0x1>;
+ xlnx,core-rows = /bits/ 8 <1 8>;
+ xlnx,mem-rows = /bits/ 8 <0 0>;
+ xlnx,shim-rows = /bits/ 8 <0 1>;
+
+ aperture0: aperture@200000000000 {
+ /* 50 columns and 8 core tile rows + 1 SHIM row */
+ compatible = "xlnx,ai-engine-aperture";
+ reg = <0x200 0x0 0x1 0x0>;
+ interrupts = <0x0 0x94 0x4>,
+ <0x0 0x95 0x4>,
+ <0x0 0x96 0x4>;
+ interrupt-names = "interrupt1", "interrupt2", "interrupt3";
+ interrupt-parent = <&gic>;
+ xlnx,columns = <0 50>;
+ xlnx,node-id = <1>;
+ };
+ };
+ };
--
2.34.1On 02/07/2025 17:56, Gregory Williams wrote:
> In the device tree, there will be device node for the AI engine device,
> and device nodes for the statically configured AI engine apertures.
No, describe the hardware, not DTS.
> Apertures are an isolated set of columns with in the AI engine device
> with their own address space and interrupt.
>
> Signed-off-by: Gregory Williams <gregory.williams@amd.com>
> ---
> .../bindings/soc/xilinx/xlnx,ai-engine.yaml | 151 ++++++++++++++++++
> 1 file changed, 151 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/soc/xilinx/xlnx,ai-engine.yaml
>
> diff --git a/Documentation/devicetree/bindings/soc/xilinx/xlnx,ai-engine.yaml b/Documentation/devicetree/bindings/soc/xilinx/xlnx,ai-engine.yaml
> new file mode 100644
> index 000000000000..7d9a36c56366
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/xilinx/xlnx,ai-engine.yaml
Filename matching compatible.
> @@ -0,0 +1,151 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/soc/xilinx/xlnx,ai-engine.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: AMD AI Engine
That's really too generic...
> +
> +maintainers:
> + - Gregory Williams <gregory.williams@amd.com>
> +
> +description:
> + The AMD AI Engine is a tile processor with many cores (up to 400) that
> + can run in parallel. The data routing between cores is configured through
> + internal switches, and shim tiles interface with external interconnect, such
> + as memory or PL. One AI engine device can have multiple apertures, each
> + has its own address space and interrupt. At runtime application can create
> + multiple partitions within an aperture which are groups of columns of AI
> + engine tiles. Each AI engine partition is the minimum resetable unit for an
> + AI engine application.
> +
> +properties:
> + compatible:
> + const: xlnx,ai-engine-v2.0
What does v2.0 stands for? Versioning is discouraged, unless mapping is
well documented.
> +
> + reg:
> + maxItems: 1
> +
> + '#address-cells':
> + const: 2
> +
> + '#size-cells':
> + const: 2
> +
> + power-domains:
Missing constraints.
> + description:
> + Platform management node id used to request power management services
> + from the firmware driver.
Drop description, redundant.
> +
> + xlnx,aie-gen:
> + $ref: /schemas/types.yaml#/definitions/uint8
Why uint8?
> + description:
> + Hardware generation of AI engine device. E.g. the current values supported
> + are 1 (AIE) and 2 (AIEML).
No clue what's that, but it is implied by compatible, isn't it?
Missing constraints.
> +
> + xlnx,shim-rows:
> + $ref: /schemas/types.yaml#/definitions/uint8-array
> + description:
> + start row and the number of rows of SHIM tiles of the AI engine device
Implied by compatible.
Missing constraints.
> +
> + xlnx,core-rows:
> + $ref: /schemas/types.yaml#/definitions/uint8-array
> + description:
> + start row and the number of rows of core tiles of the AI engine device
> +
> + xlnx,mem-rows:
> + $ref: /schemas/types.yaml#/definitions/uint8-array
> + description:
> + start row and the number of rows of memory tiles of the AI engine device
> +
Same comments everywhere.
> +required:
> + - compatible
> + - reg
> + - power-domains
> + - xlnx,aie-gen
> + - xlnx,shim-rows
> + - xlnx,core-rows
> + - xlnx,mem-rows
> +
> +patternProperties:
This goes after properties.
> + "^aperture@[0-9]+$":
> + type: object
> + description:
> + AI engine aperture which is a group of column based tiles of the
> + AI engine device. Each AI engine apertures isolated from the
> + other AI engine apertures. An AI engine aperture is defined by
> + AMD/Xilinx platform design tools.
> +
> + properties:
> + compatible:
> + const: xlnx,ai-engine-aperture
> +
> + reg:
> + description:
> + Physical base address and length of the aperture registers.
> + The AI engine address space assigned to Linux is defined by
> + Xilinx/AMD platform design tool.
Missing constraints. Description is redundant - can it be anything else?
Plus you clearly miss ranges.
> +
> + interrupts:
> + maxItems: 3
> +
> + interrupt-names:
> + items:
> + - const: interrupt1
> + - const: interrupt2
> + - const: interrupt3
Useless names, drop entirely.
> +
> + xlnx,columns:
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + description:
> + It describes the location of the aperture. It specifies the start
> + column and the number of columns. E.g. an aperture starts from
> + column 0 and there are 50 columns, it will be presented as <0 50>.
Same comments as before
> +
> + xlnx,node-id:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + AI engine aperture node ID, which is defined by AMD/Xilinx platform
> + design tool to identify the AI engine aperture in the firmware.
No, you do not get node ID. Recently every day a patch comes for that...
> +
> + required:
> + - compatible
> + - reg
> + - xlnx,columns
> + - xlnx,node-id
> +
> + additionalProperties: false
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/power/xlnx-versal-power.h>
> + bus {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ai_engine: ai-engine@20000000000 {
> + compatible = "xlnx,ai-engine-v2.0";
> + reg = <0x200 0x00 0x01 0x00>;
> + #address-cells = <2>;
> + #size-cells = <2>;
> + power-domains = <&versal_firmware PM_DEV_AI>;
> + xlnx,aie-gen = /bits/ 8 <0x1>;
> + xlnx,core-rows = /bits/ 8 <1 8>;
> + xlnx,mem-rows = /bits/ 8 <0 0>;
> + xlnx,shim-rows = /bits/ 8 <0 1>
This cannot be without ranges... I am surprised it actually works, but
for sure was not tested and produces warnings.
> +
> + aperture0: aperture@200000000000 {
> + /* 50 columns and 8 core tile rows + 1 SHIM row */
> + compatible = "xlnx,ai-engine-aperture";
> + reg = <0x200 0x0 0x1 0x0>;
> + interrupts = <0x0 0x94 0x4>,
> + <0x0 0x95 0x4>,
> + <0x0 0x96 0x4>;
Use proper flags.
Best regards,
Krzysztof
On 7/3/2025 12:48 AM, Krzysztof Kozlowski wrote:
> On 02/07/2025 17:56, Gregory Williams wrote:
>> In the device tree, there will be device node for the AI engine device,
>> and device nodes for the statically configured AI engine apertures.
>
> No, describe the hardware, not DTS.
>
>> Apertures are an isolated set of columns with in the AI engine device
>> with their own address space and interrupt.
>>
>> Signed-off-by: Gregory Williams <gregory.williams@amd.com>
>> ---
>> .../bindings/soc/xilinx/xlnx,ai-engine.yaml | 151 ++++++++++++++++++
>> 1 file changed, 151 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/soc/xilinx/xlnx,ai-engine.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/soc/xilinx/xlnx,ai-engine.yaml b/Documentation/devicetree/bindings/soc/xilinx/xlnx,ai-engine.yaml
>> new file mode 100644
>> index 000000000000..7d9a36c56366
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/soc/xilinx/xlnx,ai-engine.yaml
>
> Filename matching compatible.
>
>> @@ -0,0 +1,151 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/soc/xilinx/xlnx,ai-engine.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: AMD AI Engine
>
> That's really too generic...
>
>> +
>> +maintainers:
>> + - Gregory Williams <gregory.williams@amd.com>
>> +
>> +description:
>> + The AMD AI Engine is a tile processor with many cores (up to 400) that
>> + can run in parallel. The data routing between cores is configured through
>> + internal switches, and shim tiles interface with external interconnect, such
>> + as memory or PL. One AI engine device can have multiple apertures, each
>> + has its own address space and interrupt. At runtime application can create
>> + multiple partitions within an aperture which are groups of columns of AI
>> + engine tiles. Each AI engine partition is the minimum resetable unit for an
>> + AI engine application.
>> +
>> +properties:
>> + compatible:
>> + const: xlnx,ai-engine-v2.0
>
> What does v2.0 stands for? Versioning is discouraged, unless mapping is
> well documented.
Sure, I will remove the versioning in V2 patch.
>
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + '#address-cells':
>> + const: 2
>> +
>> + '#size-cells':
>> + const: 2
>> +
>> + power-domains:
>
> Missing constraints.
>
>> + description:
>> + Platform management node id used to request power management services
>> + from the firmware driver.
>
> Drop description, redundant.
>
>> +
>> + xlnx,aie-gen:
>> + $ref: /schemas/types.yaml#/definitions/uint8
>
> Why uint8?
>
>> + description:
>> + Hardware generation of AI engine device. E.g. the current values supported
>> + are 1 (AIE) and 2 (AIEML).
>
> No clue what's that, but it is implied by compatible, isn't it?
The driver supports multiple hardware generations. During driver probe, this value is read from the device tree and hardware generation specific
data structures are loaded based on this value. The compatible string is the same between devices.
>
> Missing constraints.
>
>> +
>> + xlnx,shim-rows:
>> + $ref: /schemas/types.yaml#/definitions/uint8-array
>> + description:
>> + start row and the number of rows of SHIM tiles of the AI engine device
>
> Implied by compatible.
The AI Engine device can have different configurations for number of rows and column (even if it is the same hardware generation). This property
tells the driver the size and layout of the array, this is not implied by compatible.
>
> Missing constraints.
>
>
>> +
>> + xlnx,core-rows:
>> + $ref: /schemas/types.yaml#/definitions/uint8-array
>> + description:
>> + start row and the number of rows of core tiles of the AI engine device
>> +
>> + xlnx,mem-rows:
>> + $ref: /schemas/types.yaml#/definitions/uint8-array
>> + description:
>> + start row and the number of rows of memory tiles of the AI engine device
>> +
>
> Same comments everywhere.
>
>> +required:
>> + - compatible
>> + - reg
>> + - power-domains
>> + - xlnx,aie-gen
>> + - xlnx,shim-rows
>> + - xlnx,core-rows
>> + - xlnx,mem-rows
>> +
>> +patternProperties:
>
> This goes after properties.
>
>> + "^aperture@[0-9]+$":
>> + type: object
>> + description:
>> + AI engine aperture which is a group of column based tiles of the
>> + AI engine device. Each AI engine apertures isolated from the
>> + other AI engine apertures. An AI engine aperture is defined by
>> + AMD/Xilinx platform design tools.
>> +
>> + properties:
>> + compatible:
>> + const: xlnx,ai-engine-aperture
>> +
>> + reg:
>> + description:
>> + Physical base address and length of the aperture registers.
>> + The AI engine address space assigned to Linux is defined by
>> + Xilinx/AMD platform design tool.
>
> Missing constraints. Description is redundant - can it be anything else?
>
> Plus you clearly miss ranges.
>
>
>> +
>> + interrupts:
>> + maxItems: 3
>> +
>> + interrupt-names:
>> + items:
>> + - const: interrupt1
>> + - const: interrupt2
>> + - const: interrupt3
>
> Useless names, drop entirely.
>
>> +
>> + xlnx,columns:
>> + $ref: /schemas/types.yaml#/definitions/uint32-array
>> + description:
>> + It describes the location of the aperture. It specifies the start
>> + column and the number of columns. E.g. an aperture starts from
>> + column 0 and there are 50 columns, it will be presented as <0 50>.
>
> Same comments as before
>
>> +
>> + xlnx,node-id:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description:
>> + AI engine aperture node ID, which is defined by AMD/Xilinx platform
>> + design tool to identify the AI engine aperture in the firmware.
>
> No, you do not get node ID. Recently every day a patch comes for that...
>
>> +
>> + required:
>> + - compatible
>> + - reg
>> + - xlnx,columns
>> + - xlnx,node-id
>> +
>> + additionalProperties: false
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + #include <dt-bindings/power/xlnx-versal-power.h>
>> + bus {
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> + ai_engine: ai-engine@20000000000 {
>> + compatible = "xlnx,ai-engine-v2.0";
>> + reg = <0x200 0x00 0x01 0x00>;
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> + power-domains = <&versal_firmware PM_DEV_AI>;
>> + xlnx,aie-gen = /bits/ 8 <0x1>;
>> + xlnx,core-rows = /bits/ 8 <1 8>;
>> + xlnx,mem-rows = /bits/ 8 <0 0>;
>> + xlnx,shim-rows = /bits/ 8 <0 1>
>
> This cannot be without ranges... I am surprised it actually works, but
> for sure was not tested and produces warnings.
>
>> +
>> + aperture0: aperture@200000000000 {
>> + /* 50 columns and 8 core tile rows + 1 SHIM row */
>> + compatible = "xlnx,ai-engine-aperture";
>> + reg = <0x200 0x0 0x1 0x0>;
>> + interrupts = <0x0 0x94 0x4>,
>> + <0x0 0x95 0x4>,
>> + <0x0 0x96 0x4>;
> Use proper flags.
>
> Best regards,
> Krzysztof
Thanks again for the review Krzysztof, I appreciate your time. I will address the remaining comments in a V2 patch.
Thanks,
Greg
On 10/07/2025 21:03, Williams, Gregory wrote: > On 7/3/2025 12:48 AM, Krzysztof Kozlowski wrote: >> On 02/07/2025 17:56, Gregory Williams wrote: >>> In the device tree, there will be device node for the AI engine device, >>> and device nodes for the statically configured AI engine apertures. >> >> No, describe the hardware, not DTS. >> >>> Apertures are an isolated set of columns with in the AI engine device >>> with their own address space and interrupt. >>> >>> Signed-off-by: Gregory Williams <gregory.williams@amd.com> >>> --- >>> .../bindings/soc/xilinx/xlnx,ai-engine.yaml | 151 ++++++++++++++++++ >>> 1 file changed, 151 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/soc/xilinx/xlnx,ai-engine.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/soc/xilinx/xlnx,ai-engine.yaml b/Documentation/devicetree/bindings/soc/xilinx/xlnx,ai-engine.yaml >>> new file mode 100644 >>> index 000000000000..7d9a36c56366 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/soc/xilinx/xlnx,ai-engine.yaml >> >> Filename matching compatible. >> >>> @@ -0,0 +1,151 @@ >>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/soc/xilinx/xlnx,ai-engine.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: AMD AI Engine >> >> That's really too generic... You did not answer to other comments here and other patches, so I just assume you did not ignore them. >> >>> + >>> +maintainers: >>> + - Gregory Williams <gregory.williams@amd.com> >>> + >>> +description: >>> + The AMD AI Engine is a tile processor with many cores (up to 400) that >>> + can run in parallel. The data routing between cores is configured through >>> + internal switches, and shim tiles interface with external interconnect, such >>> + as memory or PL. One AI engine device can have multiple apertures, each >>> + has its own address space and interrupt. At runtime application can create >>> + multiple partitions within an aperture which are groups of columns of AI >>> + engine tiles. Each AI engine partition is the minimum resetable unit for an >>> + AI engine application. >>> + >>> +properties: >>> + compatible: >>> + const: xlnx,ai-engine-v2.0 >> >> What does v2.0 stands for? Versioning is discouraged, unless mapping is >> well documented. > > Sure, I will remove the versioning in V2 patch. This should be specific to product, so use the actual product/model name. Is this part of a Soc? Then standard rules apply... but I could not deduce it from the descriptions or commit msgs. > >> >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + '#address-cells': >>> + const: 2 >>> + >>> + '#size-cells': >>> + const: 2 >>> + >>> + power-domains: >> >> Missing constraints. >> >>> + description: >>> + Platform management node id used to request power management services >>> + from the firmware driver. >> >> Drop description, redundant. >> >>> + >>> + xlnx,aie-gen: >>> + $ref: /schemas/types.yaml#/definitions/uint8 >> >> Why uint8? >> >>> + description: >>> + Hardware generation of AI engine device. E.g. the current values supported >>> + are 1 (AIE) and 2 (AIEML). >> >> No clue what's that, but it is implied by compatible, isn't it? > > The driver supports multiple hardware generations. During driver probe, this value is read from the device tree and hardware generation specific Bindings are about hardware, not driver, so your driver arguments are not valid. > data structures are loaded based on this value. The compatible string is the same between devices. No. See writing bindings. > >> >> Missing constraints. >> >>> + >>> + xlnx,shim-rows: >>> + $ref: /schemas/types.yaml#/definitions/uint8-array >>> + description: >>> + start row and the number of rows of SHIM tiles of the AI engine device >> >> Implied by compatible. > > The AI Engine device can have different configurations for number of rows and column (even if it is the same hardware generation). This property > tells the driver the size and layout of the array, this is not implied by compatible. Wrap your emails correctly. Again driver.. no, please describe the hardware, not your drivers. Best regards, Krzysztof
On 7/10/2025 3:38 PM, Krzysztof Kozlowski wrote: > On 10/07/2025 21:03, Williams, Gregory wrote: >> On 7/3/2025 12:48 AM, Krzysztof Kozlowski wrote: >>> On 02/07/2025 17:56, Gregory Williams wrote: >>>> In the device tree, there will be device node for the AI engine device, >>>> and device nodes for the statically configured AI engine apertures. >>> >>> No, describe the hardware, not DTS. >>> >>>> Apertures are an isolated set of columns with in the AI engine device >>>> with their own address space and interrupt. >>>> >>>> Signed-off-by: Gregory Williams <gregory.williams@amd.com> >>>> --- >>>> .../bindings/soc/xilinx/xlnx,ai-engine.yaml | 151 ++++++++++++++++++ >>>> 1 file changed, 151 insertions(+) >>>> create mode 100644 Documentation/devicetree/bindings/soc/xilinx/xlnx,ai-engine.yaml >>>> >>>> diff --git a/Documentation/devicetree/bindings/soc/xilinx/xlnx,ai-engine.yaml b/Documentation/devicetree/bindings/soc/xilinx/xlnx,ai-engine.yaml >>>> new file mode 100644 >>>> index 000000000000..7d9a36c56366 >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/soc/xilinx/xlnx,ai-engine.yaml >>> >>> Filename matching compatible. >>> >>>> @@ -0,0 +1,151 @@ >>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >>>> +%YAML 1.2 >>>> +--- >>>> +$id: http://devicetree.org/schemas/soc/xilinx/xlnx,ai-engine.yaml# >>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>>> + >>>> +title: AMD AI Engine >>> >>> That's really too generic... > > You did not answer to other comments here and other patches, so I just > assume you did not ignore them. No, they were not ignored. I will make sure to address in a V2 patch. > >>> >>>> + >>>> +maintainers: >>>> + - Gregory Williams <gregory.williams@amd.com> >>>> + >>>> +description: >>>> + The AMD AI Engine is a tile processor with many cores (up to 400) that >>>> + can run in parallel. The data routing between cores is configured through >>>> + internal switches, and shim tiles interface with external interconnect, such >>>> + as memory or PL. One AI engine device can have multiple apertures, each >>>> + has its own address space and interrupt. At runtime application can create >>>> + multiple partitions within an aperture which are groups of columns of AI >>>> + engine tiles. Each AI engine partition is the minimum resetable unit for an >>>> + AI engine application. >>>> + >>>> +properties: >>>> + compatible: >>>> + const: xlnx,ai-engine-v2.0 >>> >>> What does v2.0 stands for? Versioning is discouraged, unless mapping is >>> well documented. >> >> Sure, I will remove the versioning in V2 patch. > > This should be specific to product, so use the actual product/model name. > > Is this part of a Soc? Then standard rules apply... but I could not > deduce it from the descriptions or commit msgs. Yes this is part of an SoC. I will be more descriptive in V2 patch. > > >> >>> >>>> + >>>> + reg: >>>> + maxItems: 1 >>>> + >>>> + '#address-cells': >>>> + const: 2 >>>> + >>>> + '#size-cells': >>>> + const: 2 >>>> + >>>> + power-domains: >>> >>> Missing constraints. >>> >>>> + description: >>>> + Platform management node id used to request power management services >>>> + from the firmware driver. >>> >>> Drop description, redundant. >>> >>>> + >>>> + xlnx,aie-gen: >>>> + $ref: /schemas/types.yaml#/definitions/uint8 >>> >>> Why uint8? >>> >>>> + description: >>>> + Hardware generation of AI engine device. E.g. the current values supported >>>> + are 1 (AIE) and 2 (AIEML). >>> >>> No clue what's that, but it is implied by compatible, isn't it? >> >> The driver supports multiple hardware generations. During driver probe, this value is read from the device tree and hardware generation specific > > Bindings are about hardware, not driver, so your driver arguments are > not valid. Understood. > >> data structures are loaded based on this value. The compatible string is the same between devices. > > No. See writing bindings. Ok so there should be a different compatible strings based on hardware generation. I will fix this for a V2 patch. > >> >>> >>> Missing constraints. >>> >>>> + >>>> + xlnx,shim-rows: >>>> + $ref: /schemas/types.yaml#/definitions/uint8-array >>>> + description: >>>> + start row and the number of rows of SHIM tiles of the AI engine device >>> >>> Implied by compatible. >> >> The AI Engine device can have different configurations for number of rows and column (even if it is the same hardware generation). This property >> tells the driver the size and layout of the array, this is not implied by compatible. > > Wrap your emails correctly. > > Again driver.. no, please describe the hardware, not your drivers. I see in 'writing bindings' that I should use device-based compatible string. I will do this and remove these nodes for V2 patch. Thanks again for your time, Greg > > > Best regards, > Krzysztof
On 11/07/2025 20:33, Williams, Gregory wrote: >>>>> + >>>>> +maintainers: >>>>> + - Gregory Williams <gregory.williams@amd.com> >>>>> + >>>>> +description: >>>>> + The AMD AI Engine is a tile processor with many cores (up to 400) that >>>>> + can run in parallel. The data routing between cores is configured through >>>>> + internal switches, and shim tiles interface with external interconnect, such >>>>> + as memory or PL. One AI engine device can have multiple apertures, each >>>>> + has its own address space and interrupt. At runtime application can create >>>>> + multiple partitions within an aperture which are groups of columns of AI >>>>> + engine tiles. Each AI engine partition is the minimum resetable unit for an >>>>> + AI engine application. >>>>> + >>>>> +properties: >>>>> + compatible: >>>>> + const: xlnx,ai-engine-v2.0 >>>> >>>> What does v2.0 stands for? Versioning is discouraged, unless mapping is >>>> well documented. >>> >>> Sure, I will remove the versioning in V2 patch. >> >> This should be specific to product, so use the actual product/model name. >> >> Is this part of a Soc? Then standard rules apply... but I could not >> deduce it from the descriptions or commit msgs. > > Yes this is part of an SoC. I will be more descriptive in V2 patch. Huh... so you MUST use SoC compatibles. Don't upstream things entirely different than everything else. Best regards, Krzysztof
On 7/12/2025 1:33 AM, Krzysztof Kozlowski wrote: > On 11/07/2025 20:33, Williams, Gregory wrote: >>>>>> + >>>>>> +maintainers: >>>>>> + - Gregory Williams <gregory.williams@amd.com> >>>>>> + >>>>>> +description: >>>>>> + The AMD AI Engine is a tile processor with many cores (up to 400) that >>>>>> + can run in parallel. The data routing between cores is configured through >>>>>> + internal switches, and shim tiles interface with external interconnect, such >>>>>> + as memory or PL. One AI engine device can have multiple apertures, each >>>>>> + has its own address space and interrupt. At runtime application can create >>>>>> + multiple partitions within an aperture which are groups of columns of AI >>>>>> + engine tiles. Each AI engine partition is the minimum resetable unit for an >>>>>> + AI engine application. >>>>>> + >>>>>> +properties: >>>>>> + compatible: >>>>>> + const: xlnx,ai-engine-v2.0 >>>>> >>>>> What does v2.0 stands for? Versioning is discouraged, unless mapping is >>>>> well documented. >>>> >>>> Sure, I will remove the versioning in V2 patch. >>> >>> This should be specific to product, so use the actual product/model name. >>> >>> Is this part of a Soc? Then standard rules apply... but I could not >>> deduce it from the descriptions or commit msgs. >> >> Yes this is part of an SoC. I will be more descriptive in V2 patch. > > Huh... so you MUST use SoC compatibles. Don't upstream things entirely > different than everything else. I will fix this in V2 patch. Thanks, Greg > > Best regards, > Krzysztof
© 2016 - 2025 Red Hat, Inc.