Re: [PATCH v3 1/9] dt-binding: soc: xilinx: ai-engine: Add AI engine binding

On Sun, Nov 29, 2020 at 11:48:17PM -0800, Wendy Liang wrote:

Xilinx AI engine array can be partitioned statically for different applications. In the device tree, there will be device node for the AI engine device, and device nodes for the statically configured AI engine partitions. Each of the statically configured partition has a partition ID in the system.

Signed-off-by: Wendy Liang wendy.liang@xilinx.com

.../bindings/soc/xilinx/xlnx,ai-engine.yaml | 126 +++++++++++++++++++++ 1 file changed, 126 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 0000000..1de5623 --- /dev/null +++ b/Documentation/devicetree/bindings/soc/xilinx/xlnx,ai-engine.yaml @@ -0,0 +1,126 @@ +# 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: Xilinx AI Engine

+maintainers:

• • Wendy Liang wendy.liang@xilinx.com

+description: |+

You don't need '|' unless there's formatting to preserve.

• The Xilinx 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.

+properties:

• compatible:
• const: xlnx,ai-engine-v1.0

This is soft logic? If not, don't use version numbers.

• reg:
• description: |

•  Physical base address and length of the device registers.
  

That's every 'reg' property. Drop.

•  The AI engine address space assigned to Linux is defined by Xilinx
  

•  platform design tool.
  

• '#address-cells':
• enum: [2]

const: 2

• description: |

•  size of cell to describe AI engine range of tiles address.
  

•  It is the location of the starting tile of the range.
  

•  As the AI engine tiles are 2D array, the location of a tile
  

•  is presented as (column, row), the address cell is 2.
  

• '#size-cells':
• enum: [2]
• description: |

•  size of cell to describe AI engine range of tiles size.
  

•  As the AI engine tiles are 2D array, the size cell is 2.
  

• power-domains:
• maxItems: 1
• description: phandle to the associated power domain
• interrupts:
• maxItems: 3
• interrupt-names:
• description: |

•  Should be "interrupt1", "interrupt2" or "interrupt3".
  

Really, not useful names. If you do have names, they should be a schema, not freeform text.

+required:

• • compatible
• • reg
• • '#address-cells'
• • '#size-cells'
• • power-domains
• • interrupt-parent

Generally, never required because it could be in the parent node.

• • interrupts
• • interrupt-names

+patternProperties:

• "^aie_partition@[0-9]+$":

aie-partition@

The unit-address is just the 1st cell of reg (the row)? Or needs to be row and column, in which case you'd want something like '@0,0'. Also, unit-address values are typically hex, not decimal.

• type: object
• description: |

•  AI engine partition which is a group of column based tiles of the AI
  

•  engine device. Each AI engine partition is isolated from the other
  

•  AI engine partitions. An AI engine partition is defined by Xilinx
  

•  platform design tools. Each partition has a SHIM row and core tiles rows.
  

•  A SHIM row contains SHIM tiles which are the interface to external
  

•  components. AXI master can access AI engine registers, push data to and
  

•  fetch data from AI engine through the SHIM tiles. Core tiles are the
  

•  compute tiles.
  

• properties:

•  reg:
  

•    description: |
  

•      It describes the group of tiles of the AI engine partition. It needs
  

•      to include the SHIM row. The format is defined by the parent AI engine
  

•      device node's '#address-cells' and '#size-cells' properties. e.g. a v1
  

•      AI engine device has 2D tiles array, the first row is SHIM row. A
  

•      partition which has 50 columns and 8 rows of core tiles and 1 row of
  

•      SHIM tiles will be presented as <0 0 50 9>.
  

You should be able to write some constraints like max row and column values?

•  label:
  

•    maxItems: 1
  

'label' is not an array. Why do you need label?

•  xlnx,partition-id:
  

•    $ref: /schemas/types.yaml#/definitions/uint32
  

•    description: |
  

•      AI engine partition ID, which is defined by Xilinx platform design
  

•      tool to identify the AI engine partition in the system.
  

I find the use of 'reg' a bit odd here. Maybe using 'reg' for partition would make more sense? Which is more closely associated with how you address the partition?

• required:

•  - reg
  

•  - xlnx,partition-id
  

• additionalProperties: false

+additionalProperties: false

+examples:

• • |
• bus {

•  #address-cells = <2>;
  

•  #size-cells = <2>;
  

•  ai_engine: ai-engine@20000000000 {
  

•    compatible = "xlnx,ai-engine-v1.0";
  

•    reg = <0x200 0x0 0x1 0x0>;
  

•    #address-cells = <2>;
  

•    #size-cells = <2>;
  

•    power-domains = <&versal_firmware 0x18224072>;
  

•    interrupt-parent = <&gic>;
  

•    interrupts = <0x0 0x94 0x4>,
  

•                 <0x0 0x95 0x4>,
  

•                 <0x0 0x96 0x4>;
  

•    interrupt-names = "interrupt1", "interrupt2", "interrupt3";
  

•    aie_partition0: aie_partition@0 {
  

•            /* 50 columns and 8 core tile rows + 1 SHIM row */
  

•            reg = <0 0 50 9>;
  

•            xlnx,partition-id = <1>;
  

•    };
  

•  };
  

• };

-- 2.7.4