lkml.org 
[lkml]   [2020]   [Dec]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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
>

\
 
 \ /
  Last update: 2020-12-09 00:43    [W:0.136 / U:0.688 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site