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 <[email protected]> >>>> --- >>>> .../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 <[email protected]> >>>> + >>>> +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
