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.willi...@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.willi...@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
