devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 02/22] dt-bindings: Add bindings for Keem Bay IPC driver
       [not found] <20201201223511.65542-1-mgross@linux.intel.com>
@ 2020-12-01 22:34 ` mgross
  2020-12-07 16:01   ` Rob Herring
  2020-12-01 22:34 ` [PATCH 04/22] dt-bindings: Add bindings for Keem Bay VPU " mgross
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 17+ messages in thread
From: mgross @ 2020-12-01 22:34 UTC (permalink / raw)
  To: markgross, mgross, arnd, bp, damien.lemoal, dragan.cvetic,
	gregkh, corbet, leonard.crestez, palmerdabbelt, paul.walmsley,
	peng.fan, robh+dt, shawnguo
  Cc: linux-kernel, Daniele Alessandrelli, devicetree

From: Daniele Alessandrelli <daniele.alessandrelli@intel.com>

Add DT binding documentation for the Intel Keem Bay IPC driver, which
enables communication between the Computing Sub-System (CSS) and the
Multimedia Sub-System (MSS) of the Intel Movidius SoC code named Keem
Bay.

Cc: devicetree@vger.kernel.org
Reviewed-by: Mark Gross <mgross@linux.intel.com>
Signed-off-by: Daniele Alessandrelli <daniele.alessandrelli@intel.com>
---
 .../bindings/soc/intel/intel,keembay-ipc.yaml | 63 +++++++++++++++++++
 1 file changed, 63 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml

diff --git a/Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml b/Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml
new file mode 100644
index 000000000000..6e21c54d8f34
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml
@@ -0,0 +1,63 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2020 Intel Corporation
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/soc/intel/intel,keembay-ipc.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Keem Bay IPC
+
+maintainers:
+  - Daniele Alessandrelli <daniele.alessandrelli@intel.com>
+
+description:
+  The Keem Bay IPC driver enables Inter-Processor Communication (IPC) with the
+  Visual Processor Unit (VPU) embedded in the Intel Movidius SoC code named
+  Keem Bay.
+
+properties:
+  compatible:
+    const: intel,keembay-ipc
+
+  reg:
+    items:
+      - description: The CSS (CPU) FIFO registers
+      - description: The MSS (VPU) FIFO registers
+
+  reg-names:
+    items:
+      - const: css_fifo
+      - const: mss_fifo
+
+  interrupts:
+    items:
+      - description: CSS FIFO not-empty interrupt
+
+  interrupt-controller: true
+
+  memory-region:
+    items:
+      - description: Reserved memory region used for CSS IPC buffers
+      - description: Reserved memory region used for MSS IPC buffers
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - interrupts
+  - memory-region
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    ipc@203300f0 {
+          compatible = "intel,keembay-ipc";
+          reg = <0x203300f0 0x310>, /* CPU TIM FIFO */
+                <0x208200f0 0x310>; /* VPU TIM FIFO */
+          reg-names = "css_fifo", "mss_fifo";
+          interrupts = <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
+          memory-region = <&css_ipc_reserved>, <&mss_ipc_reserved>;
+    };
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 04/22] dt-bindings: Add bindings for Keem Bay VPU IPC driver
       [not found] <20201201223511.65542-1-mgross@linux.intel.com>
  2020-12-01 22:34 ` [PATCH 02/22] dt-bindings: Add bindings for Keem Bay IPC driver mgross
@ 2020-12-01 22:34 ` mgross
  2020-12-07 15:57   ` Rob Herring
  2020-12-01 22:35 ` [PATCH 15/22] xlink-ipc: Add xlink ipc device tree bindings mgross
  2020-12-01 22:35 ` [PATCH 17/22] xlink-core: Add xlink core " mgross
  3 siblings, 1 reply; 17+ messages in thread
From: mgross @ 2020-12-01 22:34 UTC (permalink / raw)
  To: markgross, mgross, arnd, bp, damien.lemoal, dragan.cvetic,
	gregkh, corbet, leonard.crestez, palmerdabbelt, paul.walmsley,
	peng.fan, robh+dt, shawnguo
  Cc: linux-kernel, Paul Murphy, devicetree, Daniele Alessandrelli

From: Paul Murphy <paul.j.murphy@intel.com>

Add DT bindings documentation for the Keem Bay VPU IPC driver.

Cc: devicetree@vger.kernel.org
Reviewed-by: Mark Gross <mgross@linux.intel.com>
Signed-off-by: Paul Murphy <paul.j.murphy@intel.com>
Co-developed-by: Daniele Alessandrelli <daniele.alessandrelli@intel.com>
Signed-off-by: Daniele Alessandrelli <daniele.alessandrelli@intel.com>
---
 .../soc/intel/intel,keembay-vpu-ipc.yaml      | 151 ++++++++++++++++++
 1 file changed, 151 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/intel/intel,keembay-vpu-ipc.yaml

diff --git a/Documentation/devicetree/bindings/soc/intel/intel,keembay-vpu-ipc.yaml b/Documentation/devicetree/bindings/soc/intel/intel,keembay-vpu-ipc.yaml
new file mode 100644
index 000000000000..60d4a028563c
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/intel/intel,keembay-vpu-ipc.yaml
@@ -0,0 +1,151 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright (c) Intel Corporation. All rights reserved.
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/soc/intel/intel,keembay-vpu-ipc.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Intel Keem Bay VPU IPC
+
+maintainers:
+  - Paul Murphy <paul.j.murphy@intel.com>
+
+description:
+  The VPU IPC driver facilitates loading of firmware, control, and communication
+  with the VPU over the IPC FIFO in the Intel Keem Bay SoC.
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+        - const: intel,keembay-vpu-ipc
+
+  reg:
+    items:
+      - description: NCE WDT registers
+      - description: NCE TIM_GEN_CONFIG registers
+      - description: MSS WDT registers
+      - description: MSS TIM_GEN_CONFIG registers
+
+  reg-names:
+    items:
+      - const: nce_wdt
+      - const: nce_tim_cfg
+      - const: mss_wdt
+      - const: mss_tim_cfg
+
+  memory-region:
+    items:
+      - description: reference to the VPU reserved memory region
+      - description: reference to the X509 reserved memory region
+      - description: reference to the MSS IPC area
+
+  clocks:
+    items:
+      - description: cpu clock
+      - description: pll 0 out 0 rate
+      - description: pll 0 out 1 rate
+      - description: pll 0 out 2 rate
+      - description: pll 0 out 3 rate
+      - description: pll 1 out 0 rate
+      - description: pll 1 out 1 rate
+      - description: pll 1 out 2 rate
+      - description: pll 1 out 3 rate
+      - description: pll 2 out 0 rate
+      - description: pll 2 out 1 rate
+      - description: pll 2 out 2 rate
+      - description: pll 2 out 3 rate
+
+  clocks-names:
+    items:
+      - const: cpu_clock
+      - const: pll_0_out_0
+      - const: pll_0_out_1
+      - const: pll_0_out_2
+      - const: pll_0_out_3
+      - const: pll_1_out_0
+      - const: pll_1_out_1
+      - const: pll_1_out_2
+      - const: pll_1_out_3
+      - const: pll_2_out_0
+      - const: pll_2_out_1
+      - const: pll_2_out_2
+      - const: pll_2_out_3
+
+  interrupts:
+    items:
+      - description: number of NCE sub-system WDT timeout IRQ
+      - description: number of MSS sub-system WDT timeout IRQ
+
+  interrupt-names:
+    items:
+      - const: nce_wdt
+      - const: mss_wdt
+
+  intel,keembay-vpu-ipc-nce-wdt-redirect:
+    $ref: "/schemas/types.yaml#/definitions/uint32"
+    description:
+      Number to which we will request that the NCE sub-system
+      re-directs it's WDT timeout IRQ
+
+  intel,keembay-vpu-ipc-mss-wdt-redirect:
+    $ref: "/schemas/types.yaml#/definitions/uint32"
+    description:
+      Number to which we will request that the MSS sub-system
+      re-directs it's WDT timeout IRQ
+
+  intel,keembay-vpu-ipc-imr:
+    $ref: "/schemas/types.yaml#/definitions/uint32"
+    description:
+      IMR (isolated memory region) number which we will request
+      the runtime service uses to protect the VPU memory region
+      before authentication
+
+  intel,keembay-vpu-ipc-id:
+    $ref: "/schemas/types.yaml#/definitions/uint32"
+    description: The VPU ID to be passed to the VPU firmware.
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    vpu-ipc@3f00209c {
+        compatible = "intel,keembay-vpu-ipc";
+        reg = <0x3f00209c 0x10>,
+              <0x3f003008 0x4>,
+              <0x2082009c 0x10>,
+              <0x20821008 0x4>;
+        reg-names = "nce_wdt",
+                    "nce_tim_cfg",
+                    "mss_wdt",
+                    "mss_tim_cfg";
+        memory-region = <&vpu_reserved>,
+                        <&vpu_x509_reserved>,
+                        <&mss_ipc_reserved>;
+        clocks = <&scmi_clk 0>,
+                 <&scmi_clk 0>,
+                 <&scmi_clk 1>,
+                 <&scmi_clk 2>,
+                 <&scmi_clk 3>,
+                 <&scmi_clk 4>,
+                 <&scmi_clk 5>,
+                 <&scmi_clk 6>,
+                 <&scmi_clk 7>,
+                 <&scmi_clk 8>,
+                 <&scmi_clk 9>,
+                 <&scmi_clk 10>,
+                 <&scmi_clk 11>;
+        clock-names = "cpu_clock",
+                      "pll_0_out_0", "pll_0_out_1",
+                      "pll_0_out_2", "pll_0_out_3",
+                      "pll_1_out_0", "pll_1_out_1",
+                      "pll_1_out_2", "pll_1_out_3",
+                      "pll_2_out_0", "pll_2_out_1",
+                      "pll_2_out_2", "pll_2_out_3";
+        interrupts = <GIC_SPI 63 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>;
+        interrupt-names = "nce_wdt", "mss_wdt";
+        intel,keembay-vpu-ipc-nce-wdt-redirect = <63>;
+        intel,keembay-vpu-ipc-mss-wdt-redirect = <47>;
+        intel,keembay-vpu-ipc-imr = <9>;
+        intel,keembay-vpu-ipc-id = <0>;
+    };
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 15/22] xlink-ipc: Add xlink ipc device tree bindings
       [not found] <20201201223511.65542-1-mgross@linux.intel.com>
  2020-12-01 22:34 ` [PATCH 02/22] dt-bindings: Add bindings for Keem Bay IPC driver mgross
  2020-12-01 22:34 ` [PATCH 04/22] dt-bindings: Add bindings for Keem Bay VPU " mgross
@ 2020-12-01 22:35 ` mgross
  2020-12-07 15:58   ` Rob Herring
  2020-12-01 22:35 ` [PATCH 17/22] xlink-core: Add xlink core " mgross
  3 siblings, 1 reply; 17+ messages in thread
From: mgross @ 2020-12-01 22:35 UTC (permalink / raw)
  To: markgross, mgross, arnd, bp, damien.lemoal, dragan.cvetic,
	gregkh, corbet, leonard.crestez, palmerdabbelt, paul.walmsley,
	peng.fan, robh+dt, shawnguo
  Cc: linux-kernel, Seamus Kelly, devicetree, Ryan Carnaghi

From: Seamus Kelly <seamus.kelly@intel.com>

Add device tree bindings for the xLink IPC driver which enables xLink to
control and communicate with the VPU IP present on the Intel Keem Bay
SoC.

Cc: devicetree@vger.kernel.org
Reviewed-by: Mark Gross <mgross@linux.intel.com>
Signed-off-by: Seamus Kelly <seamus.kelly@intel.com>
Signed-off-by: Ryan Carnaghi <ryan.r.carnaghi@intel.com>
---
 .../misc/intel,keembay-xlink-ipc.yaml         | 49 +++++++++++++++++++
 1 file changed, 49 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/intel,keembay-xlink-ipc.yaml

diff --git a/Documentation/devicetree/bindings/misc/intel,keembay-xlink-ipc.yaml b/Documentation/devicetree/bindings/misc/intel,keembay-xlink-ipc.yaml
new file mode 100644
index 000000000000..699e43c4cd40
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/intel,keembay-xlink-ipc.yaml
@@ -0,0 +1,49 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright (c) Intel Corporation. All rights reserved.
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/misc/intel,keembay-xlink-ipc.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Intel Keem Bay xlink IPC
+
+maintainers:
+  - Kelly Seamus <seamus.kelly@intel.com>
+
+description: |
+  The Keem Bay xlink IPC driver enables the communication/control sub-system
+  for internal IPC communications within the Intel Keem Bay SoC.
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+        - const: intel,keembay-xlink-ipc
+
+  memory-region:
+    items:
+      - description: reference to the CSS xlink IPC reserved memory region.
+      - description: reference to the MSS xlink IPC reserved memory region.
+
+  intel,keembay-vpu-ipc-id:
+    $ref: "/schemas/types.yaml#/definitions/uint32"
+    description: The numeric ID identifying the VPU within the xLink stack.
+
+  intel,keembay-vpu-ipc-name:
+    $ref: "/schemas/types.yaml#/definitions/string"
+    description: User-friendly name for the VPU within the xLink stack.
+
+  intel,keembay-vpu-ipc:
+    $ref: "/schemas/types.yaml#/definitions/phandle"
+    description: reference to the corresponding intel,keembay-vpu-ipc node.
+
+examples:
+  - |
+    xlink-ipc {
+        compatible = "intel,keembay-xlink-ipc";
+        memory-region = <&css_xlink_reserved>,
+                        <&mss_xlink_reserved>;
+        intel,keembay-vpu-ipc-id = <0x0>;
+        intel,keembay-vpu-ipc-name = "vpu-slice-0";
+        intel,keembay-vpu-ipc = <&vpuipc>;
+    };
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 17/22] xlink-core: Add xlink core device tree bindings
       [not found] <20201201223511.65542-1-mgross@linux.intel.com>
                   ` (2 preceding siblings ...)
  2020-12-01 22:35 ` [PATCH 15/22] xlink-ipc: Add xlink ipc device tree bindings mgross
@ 2020-12-01 22:35 ` mgross
  2020-12-07 16:02   ` Rob Herring
  3 siblings, 1 reply; 17+ messages in thread
From: mgross @ 2020-12-01 22:35 UTC (permalink / raw)
  To: markgross, mgross, arnd, bp, damien.lemoal, dragan.cvetic,
	gregkh, corbet, leonard.crestez, palmerdabbelt, paul.walmsley,
	peng.fan, robh+dt, shawnguo
  Cc: linux-kernel, Seamus Kelly, devicetree, Ryan Carnaghi

From: Seamus Kelly <seamus.kelly@intel.com>

Add device tree bindings for keembay-xlink.

Cc: devicetree@vger.kernel.org
Reviewed-by: Mark Gross <mgross@linux.intel.com>
Signed-off-by: Seamus Kelly <seamus.kelly@intel.com>
Signed-off-by: Ryan Carnaghi <ryan.r.carnaghi@intel.com>
---
 .../bindings/misc/intel,keembay-xlink.yaml    | 27 +++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/intel,keembay-xlink.yaml

diff --git a/Documentation/devicetree/bindings/misc/intel,keembay-xlink.yaml b/Documentation/devicetree/bindings/misc/intel,keembay-xlink.yaml
new file mode 100644
index 000000000000..89c34018fa04
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/intel,keembay-xlink.yaml
@@ -0,0 +1,27 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright (c) Intel Corporation. All rights reserved.
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/misc/intel,keembay-xlink.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Intel Keem Bay xlink
+
+maintainers:
+  - Seamus Kelly <seamus.kelly@intel.com>
+
+description: |
+  The Keem Bay xlink driver enables the communication/control sub-system
+  for internal and external communications to the Intel Keem Bay SoC.
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+        - const: intel,keembay-xlink
+
+examples:
+  - |
+    xlink {
+        compatible = "intel,keembay-xlink";
+    };
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 04/22] dt-bindings: Add bindings for Keem Bay VPU IPC driver
  2020-12-01 22:34 ` [PATCH 04/22] dt-bindings: Add bindings for Keem Bay VPU " mgross
@ 2020-12-07 15:57   ` Rob Herring
  2020-12-07 21:28     ` mark gross
  0 siblings, 1 reply; 17+ messages in thread
From: Rob Herring @ 2020-12-07 15:57 UTC (permalink / raw)
  To: mgross
  Cc: damien.lemoal, leonard.crestez, Paul Murphy, dragan.cvetic,
	markgross, robh+dt, corbet, devicetree, Daniele Alessandrelli,
	arnd, palmerdabbelt, bp, shawnguo, peng.fan, gregkh,
	paul.walmsley, linux-kernel

On Tue, 01 Dec 2020 14:34:53 -0800, mgross@linux.intel.com wrote:
> From: Paul Murphy <paul.j.murphy@intel.com>
> 
> Add DT bindings documentation for the Keem Bay VPU IPC driver.
> 
> Cc: devicetree@vger.kernel.org
> Reviewed-by: Mark Gross <mgross@linux.intel.com>
> Signed-off-by: Paul Murphy <paul.j.murphy@intel.com>
> Co-developed-by: Daniele Alessandrelli <daniele.alessandrelli@intel.com>
> Signed-off-by: Daniele Alessandrelli <daniele.alessandrelli@intel.com>
> ---
>  .../soc/intel/intel,keembay-vpu-ipc.yaml      | 151 ++++++++++++++++++
>  1 file changed, 151 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/intel/intel,keembay-vpu-ipc.yaml
> 


My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:
./Documentation/devicetree/bindings/soc/intel/intel,keembay-vpu-ipc.yaml:21:9: [warning] wrong indentation: expected 10 but found 8 (indentation)

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/intel/intel,keembay-vpu-ipc.yaml: 'additionalProperties' is a required property
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/intel/intel,keembay-vpu-ipc.yaml: ignoring, error in schema: 
warning: no schema found in file: ./Documentation/devicetree/bindings/soc/intel/intel,keembay-vpu-ipc.yaml


See https://patchwork.ozlabs.org/patch/1409183

The base for the patch is generally the last rc1. Any dependencies
should be noted.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 15/22] xlink-ipc: Add xlink ipc device tree bindings
  2020-12-01 22:35 ` [PATCH 15/22] xlink-ipc: Add xlink ipc device tree bindings mgross
@ 2020-12-07 15:58   ` Rob Herring
  2020-12-07 21:41     ` mark gross
  0 siblings, 1 reply; 17+ messages in thread
From: Rob Herring @ 2020-12-07 15:58 UTC (permalink / raw)
  To: mgross
  Cc: robh+dt, leonard.crestez, gregkh, palmerdabbelt, devicetree,
	Ryan Carnaghi, Seamus Kelly, arnd, damien.lemoal, markgross,
	corbet, linux-kernel, dragan.cvetic, shawnguo, paul.walmsley, bp,
	peng.fan

On Tue, 01 Dec 2020 14:35:04 -0800, mgross@linux.intel.com wrote:
> From: Seamus Kelly <seamus.kelly@intel.com>
> 
> Add device tree bindings for the xLink IPC driver which enables xLink to
> control and communicate with the VPU IP present on the Intel Keem Bay
> SoC.
> 
> Cc: devicetree@vger.kernel.org
> Reviewed-by: Mark Gross <mgross@linux.intel.com>
> Signed-off-by: Seamus Kelly <seamus.kelly@intel.com>
> Signed-off-by: Ryan Carnaghi <ryan.r.carnaghi@intel.com>
> ---
>  .../misc/intel,keembay-xlink-ipc.yaml         | 49 +++++++++++++++++++
>  1 file changed, 49 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/misc/intel,keembay-xlink-ipc.yaml
> 


My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:
./Documentation/devicetree/bindings/misc/intel,keembay-xlink-ipc.yaml:21:9: [warning] wrong indentation: expected 10 but found 8 (indentation)

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/misc/intel,keembay-xlink-ipc.yaml: 'additionalProperties' is a required property
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/misc/intel,keembay-xlink-ipc.yaml: ignoring, error in schema: 
warning: no schema found in file: ./Documentation/devicetree/bindings/misc/intel,keembay-xlink-ipc.yaml


See https://patchwork.ozlabs.org/patch/1409186

The base for the patch is generally the last rc1. Any dependencies
should be noted.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 02/22] dt-bindings: Add bindings for Keem Bay IPC driver
  2020-12-01 22:34 ` [PATCH 02/22] dt-bindings: Add bindings for Keem Bay IPC driver mgross
@ 2020-12-07 16:01   ` Rob Herring
  2020-12-07 18:22     ` mark gross
  2020-12-07 18:42     ` Daniele Alessandrelli
  0 siblings, 2 replies; 17+ messages in thread
From: Rob Herring @ 2020-12-07 16:01 UTC (permalink / raw)
  To: mgross
  Cc: markgross, arnd, bp, damien.lemoal, dragan.cvetic, gregkh,
	corbet, leonard.crestez, palmerdabbelt, paul.walmsley, peng.fan,
	shawnguo, linux-kernel, Daniele Alessandrelli, devicetree

On Tue, Dec 01, 2020 at 02:34:51PM -0800, mgross@linux.intel.com wrote:
> From: Daniele Alessandrelli <daniele.alessandrelli@intel.com>
> 
> Add DT binding documentation for the Intel Keem Bay IPC driver, which
> enables communication between the Computing Sub-System (CSS) and the
> Multimedia Sub-System (MSS) of the Intel Movidius SoC code named Keem
> Bay.
> 
> Cc: devicetree@vger.kernel.org
> Reviewed-by: Mark Gross <mgross@linux.intel.com>
> Signed-off-by: Daniele Alessandrelli <daniele.alessandrelli@intel.com>
> ---
>  .../bindings/soc/intel/intel,keembay-ipc.yaml | 63 +++++++++++++++++++
>  1 file changed, 63 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml b/Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml
> new file mode 100644
> index 000000000000..6e21c54d8f34
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml
> @@ -0,0 +1,63 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) 2020 Intel Corporation
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/soc/intel/intel,keembay-ipc.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Keem Bay IPC
> +
> +maintainers:
> +  - Daniele Alessandrelli <daniele.alessandrelli@intel.com>
> +
> +description:
> +  The Keem Bay IPC driver enables Inter-Processor Communication (IPC) with the
> +  Visual Processor Unit (VPU) embedded in the Intel Movidius SoC code named
> +  Keem Bay.

Sounds like a mailbox. 

What's the relationship between this and the xlink thing?

> +
> +properties:
> +  compatible:
> +    const: intel,keembay-ipc
> +
> +  reg:
> +    items:
> +      - description: The CSS (CPU) FIFO registers
> +      - description: The MSS (VPU) FIFO registers
> +
> +  reg-names:
> +    items:
> +      - const: css_fifo
> +      - const: mss_fifo
> +
> +  interrupts:
> +    items:
> +      - description: CSS FIFO not-empty interrupt
> +
> +  interrupt-controller: true
> +
> +  memory-region:
> +    items:
> +      - description: Reserved memory region used for CSS IPC buffers
> +      - description: Reserved memory region used for MSS IPC buffers
> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - interrupts
> +  - memory-region
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    ipc@203300f0 {
> +          compatible = "intel,keembay-ipc";
> +          reg = <0x203300f0 0x310>, /* CPU TIM FIFO */
> +                <0x208200f0 0x310>; /* VPU TIM FIFO */
> +          reg-names = "css_fifo", "mss_fifo";
> +          interrupts = <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
> +          memory-region = <&css_ipc_reserved>, <&mss_ipc_reserved>;
> +    };
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 17/22] xlink-core: Add xlink core device tree bindings
  2020-12-01 22:35 ` [PATCH 17/22] xlink-core: Add xlink core " mgross
@ 2020-12-07 16:02   ` Rob Herring
  0 siblings, 0 replies; 17+ messages in thread
From: Rob Herring @ 2020-12-07 16:02 UTC (permalink / raw)
  To: mgross
  Cc: Ryan Carnaghi, peng.fan, damien.lemoal, shawnguo, paul.walmsley,
	markgross, devicetree, bp, arnd, dragan.cvetic, robh+dt,
	linux-kernel, Seamus Kelly, palmerdabbelt, gregkh,
	leonard.crestez, corbet

On Tue, 01 Dec 2020 14:35:06 -0800, mgross@linux.intel.com wrote:
> From: Seamus Kelly <seamus.kelly@intel.com>
> 
> Add device tree bindings for keembay-xlink.
> 
> Cc: devicetree@vger.kernel.org
> Reviewed-by: Mark Gross <mgross@linux.intel.com>
> Signed-off-by: Seamus Kelly <seamus.kelly@intel.com>
> Signed-off-by: Ryan Carnaghi <ryan.r.carnaghi@intel.com>
> ---
>  .../bindings/misc/intel,keembay-xlink.yaml    | 27 +++++++++++++++++++
>  1 file changed, 27 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/misc/intel,keembay-xlink.yaml
> 


My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:
./Documentation/devicetree/bindings/misc/intel,keembay-xlink.yaml:21:9: [warning] wrong indentation: expected 10 but found 8 (indentation)

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/misc/intel,keembay-xlink.yaml: 'additionalProperties' is a required property
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/misc/intel,keembay-xlink.yaml: ignoring, error in schema: 
warning: no schema found in file: ./Documentation/devicetree/bindings/misc/intel,keembay-xlink.yaml


See https://patchwork.ozlabs.org/patch/1409184

The base for the patch is generally the last rc1. Any dependencies
should be noted.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 02/22] dt-bindings: Add bindings for Keem Bay IPC driver
  2020-12-07 16:01   ` Rob Herring
@ 2020-12-07 18:22     ` mark gross
  2020-12-07 18:42     ` Daniele Alessandrelli
  1 sibling, 0 replies; 17+ messages in thread
From: mark gross @ 2020-12-07 18:22 UTC (permalink / raw)
  To: Rob Herring
  Cc: mgross, markgross, arnd, bp, damien.lemoal, dragan.cvetic,
	gregkh, corbet, leonard.crestez, palmerdabbelt, paul.walmsley,
	peng.fan, shawnguo, linux-kernel, Daniele Alessandrelli,
	devicetree

On Mon, Dec 07, 2020 at 10:01:52AM -0600, Rob Herring wrote:
> On Tue, Dec 01, 2020 at 02:34:51PM -0800, mgross@linux.intel.com wrote:
> > From: Daniele Alessandrelli <daniele.alessandrelli@intel.com>
> > 
> > Add DT binding documentation for the Intel Keem Bay IPC driver, which
> > enables communication between the Computing Sub-System (CSS) and the
> > Multimedia Sub-System (MSS) of the Intel Movidius SoC code named Keem
> > Bay.
> > 
> > Cc: devicetree@vger.kernel.org
> > Reviewed-by: Mark Gross <mgross@linux.intel.com>
> > Signed-off-by: Daniele Alessandrelli <daniele.alessandrelli@intel.com>
> > ---
> >  .../bindings/soc/intel/intel,keembay-ipc.yaml | 63 +++++++++++++++++++
> >  1 file changed, 63 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml b/Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml
> > new file mode 100644
> > index 000000000000..6e21c54d8f34
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml
> > @@ -0,0 +1,63 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +# Copyright (C) 2020 Intel Corporation
> > +%YAML 1.2
> > +---
> > +$id: "http://devicetree.org/schemas/soc/intel/intel,keembay-ipc.yaml#"
> > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> > +
> > +title: Keem Bay IPC
> > +
> > +maintainers:
> > +  - Daniele Alessandrelli <daniele.alessandrelli@intel.com>
> > +
> > +description:
> > +  The Keem Bay IPC driver enables Inter-Processor Communication (IPC) with the
> > +  Visual Processor Unit (VPU) embedded in the Intel Movidius SoC code named
> > +  Keem Bay.
> 
> Sounds like a mailbox. 
Its a multi-channel mailbox like thing with priority channel support.

> 
> What's the relationship between this and the xlink thing?
Xlink is a SW abstraction to allow multiple user access to the VPU as well as
enabling use cases where a Keem Bay is used as an accelerator add in card as
well as a simple SBC type of design.  The xlink stuff sits on top of the IPC
stuff.

--mark


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 02/22] dt-bindings: Add bindings for Keem Bay IPC driver
  2020-12-07 16:01   ` Rob Herring
  2020-12-07 18:22     ` mark gross
@ 2020-12-07 18:42     ` Daniele Alessandrelli
  2020-12-07 20:31       ` Jassi Brar
  2020-12-09 17:33       ` Rob Herring
  1 sibling, 2 replies; 17+ messages in thread
From: Daniele Alessandrelli @ 2020-12-07 18:42 UTC (permalink / raw)
  To: Rob Herring, mgross, daniele.alessandrelli
  Cc: markgross, arnd, bp, damien.lemoal, gregkh, corbet,
	leonard.crestez, palmerdabbelt, paul.walmsley, peng.fan,
	shawnguo, linux-kernel, devicetree, Jassi Brar

Hi Rob,

Thanks for the feedback.

On Mon, 2020-12-07 at 10:01 -0600, Rob Herring wrote:
> On Tue, Dec 01, 2020 at 02:34:51PM -0800, mgross@linux.intel.com wrote:
> > From: Daniele Alessandrelli <daniele.alessandrelli@intel.com>
> > 
> > Add DT binding documentation for the Intel Keem Bay IPC driver, which
> > enables communication between the Computing Sub-System (CSS) and the
> > Multimedia Sub-System (MSS) of the Intel Movidius SoC code named Keem
> > Bay.
> > 

[cut]

> > +
> > +description:
> > +  The Keem Bay IPC driver enables Inter-Processor Communication (IPC) with the
> > +  Visual Processor Unit (VPU) embedded in the Intel Movidius SoC code named
> > +  Keem Bay.
> 
> Sounds like a mailbox.

We did consider using the mailbox framework, but eventually decided
against it; mainly because of the following two reasons:

1. The channel concept in the Mailbox framework is different than the
   channel concept in Keem Bay IPC:

   a. My understanding is that Mailbox channels are meant to be SW
      representation of physical HW channels, while in Keem Bay IPC
      channels are software abstractions to achieve communication
      multiplexing over a single HW link

   b. Additionally, Keem Bay IPC has two different classes of channels 
      (high-speed channels and general-purpose channels) that need to
      access the same HW link with different priorities.

2. The blocking / non-blocking TX behavior of mailbox channels is
   defined at channel creation time (by the tx_block value of the
   mailbox client passed to mbox_request_channel(); my understanding 
   is that the tx_block value cannot be modified after the channel is
   created), while in Keem Bay IPC the same channel can be used for
   both blocking and non-blocking TX (behavior is controlled by the
   timeout argument passed to keembay_ipc_send()).

Having said that, I guess that it could be possible to create a Mailbox
driver implementing the core communication mechanism used by the Keem
Bay IPC and then build our API around it (basically having two
drivers). But I'm not sure that would make the code simpler or easier
to maintain. Any thoughts on this?


>  
> 
> What's the relationship between this and the xlink thing?
> 

xLink internally uses Keem Bay IPC to communicate with the VPU.
Basically, Keem Bay IPC is the lowest layer of the xLink stack.





^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 02/22] dt-bindings: Add bindings for Keem Bay IPC driver
  2020-12-07 18:42     ` Daniele Alessandrelli
@ 2020-12-07 20:31       ` Jassi Brar
  2020-12-09  0:12         ` mark gross
  2020-12-09 17:33       ` Rob Herring
  1 sibling, 1 reply; 17+ messages in thread
From: Jassi Brar @ 2020-12-07 20:31 UTC (permalink / raw)
  To: Daniele Alessandrelli
  Cc: Rob Herring, mgross, Daniele Alessandrelli, markgross, arnd, bp,
	damien.lemoal, Greg KH, Jonathan Corbet, Leonard Crestez,
	palmerdabbelt, paul.walmsley, Peng Fan, Shawn Guo,
	Linux Kernel Mailing List, Devicetree List

On Mon, Dec 7, 2020 at 12:43 PM Daniele Alessandrelli
<daniele.alessandrelli@linux.intel.com> wrote:
>
> Hi Rob,
>
> Thanks for the feedback.
>
> On Mon, 2020-12-07 at 10:01 -0600, Rob Herring wrote:
> > On Tue, Dec 01, 2020 at 02:34:51PM -0800, mgross@linux.intel.com wrote:
> > > From: Daniele Alessandrelli <daniele.alessandrelli@intel.com>
> > >
> > > Add DT binding documentation for the Intel Keem Bay IPC driver, which
> > > enables communication between the Computing Sub-System (CSS) and the
> > > Multimedia Sub-System (MSS) of the Intel Movidius SoC code named Keem
> > > Bay.
> > >
>
> [cut]
>
> > > +
> > > +description:
> > > +  The Keem Bay IPC driver enables Inter-Processor Communication (IPC) with the
> > > +  Visual Processor Unit (VPU) embedded in the Intel Movidius SoC code named
> > > +  Keem Bay.
> >
> > Sounds like a mailbox.
>
> We did consider using the mailbox framework, but eventually decided
> against it; mainly because of the following two reasons:
>
> 1. The channel concept in the Mailbox framework is different than the
>    channel concept in Keem Bay IPC:
>
>    a. My understanding is that Mailbox channels are meant to be SW
>       representation of physical HW channels, while in Keem Bay IPC
>       channels are software abstractions to achieve communication
>       multiplexing over a single HW link
>
In mailbox api, that would be a physical channel shared between various clients.

>    b. Additionally, Keem Bay IPC has two different classes of channels
>       (high-speed channels and general-purpose channels) that need to
>       access the same HW link with different priorities.
>
If the priorities are hard (programmed into some register), you could
do that via dt during channel population.
If they are soft, that would be handled in the shared channel implementation.

> 2. The blocking / non-blocking TX behavior of mailbox channels is
>    defined at channel creation time (by the tx_block value of the
>    mailbox client passed to mbox_request_channel();
>
No, that is checked at mbox_send_message()

> my understanding
>    is that the tx_block value cannot be modified after the channel is
>    created),
>
Again no. If you don't queue more than one message at any time you can
change it between transfers. To be safe you can always change it
between channel release - request calls.

>  while in Keem Bay IPC the same channel can be used for
>    both blocking and non-blocking TX (behavior is controlled by the
>    timeout argument passed to keembay_ipc_send()).
>
> Having said that, I guess that it could be possible to create a Mailbox
> driver implementing the core communication mechanism used by the Keem
> Bay IPC and then build our API around it (basically having two
> drivers). But I'm not sure that would make the code simpler or easier
> to maintain. Any thoughts on this?
>
I think so. Most of KeemBay specific behaviour would be implemented in
the shared channel above the mailbox api.

cheers!

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 04/22] dt-bindings: Add bindings for Keem Bay VPU IPC driver
  2020-12-07 15:57   ` Rob Herring
@ 2020-12-07 21:28     ` mark gross
  0 siblings, 0 replies; 17+ messages in thread
From: mark gross @ 2020-12-07 21:28 UTC (permalink / raw)
  To: Rob Herring
  Cc: mgross, damien.lemoal, leonard.crestez, Paul Murphy,
	dragan.cvetic, markgross, robh+dt, corbet, devicetree,
	Daniele Alessandrelli, arnd, palmerdabbelt, bp, shawnguo,
	peng.fan, gregkh, paul.walmsley, linux-kernel

On Mon, Dec 07, 2020 at 09:57:26AM -0600, Rob Herring wrote:
> On Tue, 01 Dec 2020 14:34:53 -0800, mgross@linux.intel.com wrote:
> > From: Paul Murphy <paul.j.murphy@intel.com>
> > 
> > Add DT bindings documentation for the Keem Bay VPU IPC driver.
> > 
> > Cc: devicetree@vger.kernel.org
> > Reviewed-by: Mark Gross <mgross@linux.intel.com>
> > Signed-off-by: Paul Murphy <paul.j.murphy@intel.com>
> > Co-developed-by: Daniele Alessandrelli <daniele.alessandrelli@intel.com>
> > Signed-off-by: Daniele Alessandrelli <daniele.alessandrelli@intel.com>
> > ---
> >  .../soc/intel/intel,keembay-vpu-ipc.yaml      | 151 ++++++++++++++++++
> >  1 file changed, 151 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/soc/intel/intel,keembay-vpu-ipc.yaml
> > 
> 
> 
> My bot found errors running 'make dt_binding_check' on your patch:
> 
> yamllint warnings/errors:
> ./Documentation/devicetree/bindings/soc/intel/intel,keembay-vpu-ipc.yaml:21:9: [warning] wrong indentation: expected 10 but found 8 (indentation)
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/intel/intel,keembay-vpu-ipc.yaml: 'additionalProperties' is a required property
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/intel/intel,keembay-vpu-ipc.yaml: ignoring, error in schema: 
> warning: no schema found in file: ./Documentation/devicetree/bindings/soc/intel/intel,keembay-vpu-ipc.yaml
> 
> 
> See https://patchwork.ozlabs.org/patch/1409183
> 
> The base for the patch is generally the last rc1. Any dependencies
> should be noted.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit.

Thanks!  I will fix on the next update.

--mark


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 15/22] xlink-ipc: Add xlink ipc device tree bindings
  2020-12-07 15:58   ` Rob Herring
@ 2020-12-07 21:41     ` mark gross
  0 siblings, 0 replies; 17+ messages in thread
From: mark gross @ 2020-12-07 21:41 UTC (permalink / raw)
  To: Rob Herring
  Cc: mgross, robh+dt, leonard.crestez, gregkh, palmerdabbelt,
	devicetree, Ryan Carnaghi, Seamus Kelly, arnd, damien.lemoal,
	markgross, corbet, linux-kernel, dragan.cvetic, shawnguo,
	paul.walmsley, bp, peng.fan

On Mon, Dec 07, 2020 at 09:58:14AM -0600, Rob Herring wrote:
> On Tue, 01 Dec 2020 14:35:04 -0800, mgross@linux.intel.com wrote:
> > From: Seamus Kelly <seamus.kelly@intel.com>
> > 
> > Add device tree bindings for the xLink IPC driver which enables xLink to
> > control and communicate with the VPU IP present on the Intel Keem Bay
> > SoC.
> > 
> > Cc: devicetree@vger.kernel.org
> > Reviewed-by: Mark Gross <mgross@linux.intel.com>
> > Signed-off-by: Seamus Kelly <seamus.kelly@intel.com>
> > Signed-off-by: Ryan Carnaghi <ryan.r.carnaghi@intel.com>
> > ---
> >  .../misc/intel,keembay-xlink-ipc.yaml         | 49 +++++++++++++++++++
> >  1 file changed, 49 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/misc/intel,keembay-xlink-ipc.yaml
> > 
> 
> 
> My bot found errors running 'make dt_binding_check' on your patch:
> 
> yamllint warnings/errors:
> ./Documentation/devicetree/bindings/misc/intel,keembay-xlink-ipc.yaml:21:9: [warning] wrong indentation: expected 10 but found 8 (indentation)
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/misc/intel,keembay-xlink-ipc.yaml: 'additionalProperties' is a required property
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/misc/intel,keembay-xlink-ipc.yaml: ignoring, error in schema: 
> warning: no schema found in file: ./Documentation/devicetree/bindings/misc/intel,keembay-xlink-ipc.yaml
> 
> 
> See https://patchwork.ozlabs.org/patch/1409186
> 
> The base for the patch is generally the last rc1. Any dependencies
> should be noted.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit.
Thank you!  will do.

--mark

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 02/22] dt-bindings: Add bindings for Keem Bay IPC driver
  2020-12-07 20:31       ` Jassi Brar
@ 2020-12-09  0:12         ` mark gross
  2020-12-09 16:49           ` Jassi Brar
  0 siblings, 1 reply; 17+ messages in thread
From: mark gross @ 2020-12-09  0:12 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Daniele Alessandrelli, Rob Herring, mgross,
	Daniele Alessandrelli, markgross, arnd, bp, damien.lemoal,
	Greg KH, Jonathan Corbet, Leonard Crestez, palmerdabbelt,
	paul.walmsley, Peng Fan, Shawn Guo, Linux Kernel Mailing List,
	Devicetree List

On Mon, Dec 07, 2020 at 02:31:37PM -0600, Jassi Brar wrote:
> On Mon, Dec 7, 2020 at 12:43 PM Daniele Alessandrelli
> <daniele.alessandrelli@linux.intel.com> wrote:
> >
> > Hi Rob,
> >
> > Thanks for the feedback.
> >
> > On Mon, 2020-12-07 at 10:01 -0600, Rob Herring wrote:
> > > On Tue, Dec 01, 2020 at 02:34:51PM -0800, mgross@linux.intel.com wrote:
> > > > From: Daniele Alessandrelli <daniele.alessandrelli@intel.com>
> > > >
> > > > Add DT binding documentation for the Intel Keem Bay IPC driver, which
> > > > enables communication between the Computing Sub-System (CSS) and the
> > > > Multimedia Sub-System (MSS) of the Intel Movidius SoC code named Keem
> > > > Bay.
> > > >
> >
> > [cut]
> >
> > > > +
> > > > +description:
> > > > +  The Keem Bay IPC driver enables Inter-Processor Communication (IPC) with the
> > > > +  Visual Processor Unit (VPU) embedded in the Intel Movidius SoC code named
> > > > +  Keem Bay.
> > >
> > > Sounds like a mailbox.
> >
> > We did consider using the mailbox framework, but eventually decided
> > against it; mainly because of the following two reasons:
> >
> > 1. The channel concept in the Mailbox framework is different than the
> >    channel concept in Keem Bay IPC:
> >
> >    a. My understanding is that Mailbox channels are meant to be SW
> >       representation of physical HW channels, while in Keem Bay IPC
> >       channels are software abstractions to achieve communication
> >       multiplexing over a single HW link
> >
> In mailbox api, that would be a physical channel shared between various clients.
> 
> >    b. Additionally, Keem Bay IPC has two different classes of channels
> >       (high-speed channels and general-purpose channels) that need to
> >       access the same HW link with different priorities.
> >
> If the priorities are hard (programmed into some register), you could
> do that via dt during channel population.
> If they are soft, that would be handled in the shared channel implementation.
> 
> > 2. The blocking / non-blocking TX behavior of mailbox channels is
> >    defined at channel creation time (by the tx_block value of the
> >    mailbox client passed to mbox_request_channel();
> >
> No, that is checked at mbox_send_message()
> 
> > my understanding
> >    is that the tx_block value cannot be modified after the channel is
> >    created),
> >
> Again no. If you don't queue more than one message at any time you can
> change it between transfers. To be safe you can always change it
> between channel release - request calls.
> 
> >  while in Keem Bay IPC the same channel can be used for
> >    both blocking and non-blocking TX (behavior is controlled by the
> >    timeout argument passed to keembay_ipc_send()).
> >
> > Having said that, I guess that it could be possible to create a Mailbox
> > driver implementing the core communication mechanism used by the Keem
> > Bay IPC and then build our API around it (basically having two
> > drivers). But I'm not sure that would make the code simpler or easier
> > to maintain. Any thoughts on this?
> >
> I think so. Most of KeemBay specific behaviour would be implemented in
> the shared channel above the mailbox api.

Quick question.  By "I think so" do you mean that you feel the keem bay IPC
code will be simpler and easier to maintain if we make yet another driver at the
Keem Bay IPC driver sits on top off?  Or, the current implementation would be
simpler if we rework the implementation to use the mailbox api?

I'm just now ramping on the common mailbox framework so that may be a dumb
question.  I would like to be confident reworking the driver to use the mailbox
api will not lead to blocking issues before we start that.

thanks!

--mark

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 02/22] dt-bindings: Add bindings for Keem Bay IPC driver
  2020-12-09  0:12         ` mark gross
@ 2020-12-09 16:49           ` Jassi Brar
  0 siblings, 0 replies; 17+ messages in thread
From: Jassi Brar @ 2020-12-09 16:49 UTC (permalink / raw)
  To: mgross
  Cc: Daniele Alessandrelli, Rob Herring, Daniele Alessandrelli,
	markgross, arnd, bp, damien.lemoal, Greg KH, Jonathan Corbet,
	Leonard Crestez, palmerdabbelt, paul.walmsley, Peng Fan,
	Shawn Guo, Linux Kernel Mailing List, Devicetree List

On Tue, Dec 8, 2020 at 6:12 PM mark gross <mgross@linux.intel.com> wrote:
>
> On Mon, Dec 07, 2020 at 02:31:37PM -0600, Jassi Brar wrote:
> > On Mon, Dec 7, 2020 at 12:43 PM Daniele Alessandrelli
> > <daniele.alessandrelli@linux.intel.com> wrote:
> > >
> > > Hi Rob,
> > >
> > > Thanks for the feedback.
> > >
> > > On Mon, 2020-12-07 at 10:01 -0600, Rob Herring wrote:
> > > > On Tue, Dec 01, 2020 at 02:34:51PM -0800, mgross@linux.intel.com wrote:
> > > > > From: Daniele Alessandrelli <daniele.alessandrelli@intel.com>
> > > > >
> > > > > Add DT binding documentation for the Intel Keem Bay IPC driver, which
> > > > > enables communication between the Computing Sub-System (CSS) and the
> > > > > Multimedia Sub-System (MSS) of the Intel Movidius SoC code named Keem
> > > > > Bay.
> > > > >
> > >
> > > [cut]
> > >
> > > > > +
> > > > > +description:
> > > > > +  The Keem Bay IPC driver enables Inter-Processor Communication (IPC) with the
> > > > > +  Visual Processor Unit (VPU) embedded in the Intel Movidius SoC code named
> > > > > +  Keem Bay.
> > > >
> > > > Sounds like a mailbox.
> > >
> > > We did consider using the mailbox framework, but eventually decided
> > > against it; mainly because of the following two reasons:
> > >
> > > 1. The channel concept in the Mailbox framework is different than the
> > >    channel concept in Keem Bay IPC:
> > >
> > >    a. My understanding is that Mailbox channels are meant to be SW
> > >       representation of physical HW channels, while in Keem Bay IPC
> > >       channels are software abstractions to achieve communication
> > >       multiplexing over a single HW link
> > >
> > In mailbox api, that would be a physical channel shared between various clients.
> >
> > >    b. Additionally, Keem Bay IPC has two different classes of channels
> > >       (high-speed channels and general-purpose channels) that need to
> > >       access the same HW link with different priorities.
> > >
> > If the priorities are hard (programmed into some register), you could
> > do that via dt during channel population.
> > If they are soft, that would be handled in the shared channel implementation.
> >
> > > 2. The blocking / non-blocking TX behavior of mailbox channels is
> > >    defined at channel creation time (by the tx_block value of the
> > >    mailbox client passed to mbox_request_channel();
> > >
> > No, that is checked at mbox_send_message()
> >
> > > my understanding
> > >    is that the tx_block value cannot be modified after the channel is
> > >    created),
> > >
> > Again no. If you don't queue more than one message at any time you can
> > change it between transfers. To be safe you can always change it
> > between channel release - request calls.
> >
> > >  while in Keem Bay IPC the same channel can be used for
> > >    both blocking and non-blocking TX (behavior is controlled by the
> > >    timeout argument passed to keembay_ipc_send()).
> > >
> > > Having said that, I guess that it could be possible to create a Mailbox
> > > driver implementing the core communication mechanism used by the Keem
> > > Bay IPC and then build our API around it (basically having two
> > > drivers). But I'm not sure that would make the code simpler or easier
> > > to maintain. Any thoughts on this?
> > >
> > I think so. Most of KeemBay specific behaviour would be implemented in
> > the shared channel above the mailbox api.
>
> Quick question.  By "I think so"
>
From what I have read, it seems it should be doable as a mailbox.

> do you mean that you feel the keem bay IPC
> code will be simpler and easier to maintain if we make yet another driver at the
> Keem Bay IPC driver sits on top off?  Or, the current implementation would be
> simpler if we rework the implementation to use the mailbox api?
>
Most of the queue management code would be taken over my mailbox api.
And you'll have
separated out the pure controller driver from protocol assumptions.
So there shouldn't be extra code... just this code reorganised and some reduced.

cheers!

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 02/22] dt-bindings: Add bindings for Keem Bay IPC driver
  2020-12-07 18:42     ` Daniele Alessandrelli
  2020-12-07 20:31       ` Jassi Brar
@ 2020-12-09 17:33       ` Rob Herring
  1 sibling, 0 replies; 17+ messages in thread
From: Rob Herring @ 2020-12-09 17:33 UTC (permalink / raw)
  To: Daniele Alessandrelli
  Cc: mgross, daniele.alessandrelli, markgross, arnd, bp,
	damien.lemoal, gregkh, corbet, leonard.crestez, palmerdabbelt,
	paul.walmsley, peng.fan, shawnguo, linux-kernel, devicetree,
	Jassi Brar

On Mon, Dec 07, 2020 at 06:42:07PM +0000, Daniele Alessandrelli wrote:
> Hi Rob,
> 
> Thanks for the feedback.
> 
> On Mon, 2020-12-07 at 10:01 -0600, Rob Herring wrote:
> > On Tue, Dec 01, 2020 at 02:34:51PM -0800, mgross@linux.intel.com wrote:
> > > From: Daniele Alessandrelli <daniele.alessandrelli@intel.com>
> > > 
> > > Add DT binding documentation for the Intel Keem Bay IPC driver, which
> > > enables communication between the Computing Sub-System (CSS) and the
> > > Multimedia Sub-System (MSS) of the Intel Movidius SoC code named Keem
> > > Bay.
> > > 
> 
> [cut]
> 
> > > +
> > > +description:
> > > +  The Keem Bay IPC driver enables Inter-Processor Communication (IPC) with the
> > > +  Visual Processor Unit (VPU) embedded in the Intel Movidius SoC code named
> > > +  Keem Bay.
> > 
> > Sounds like a mailbox.
> 
> We did consider using the mailbox framework, but eventually decided
> against it; mainly because of the following two reasons:
> 
> 1. The channel concept in the Mailbox framework is different than the
>    channel concept in Keem Bay IPC:
> 
>    a. My understanding is that Mailbox channels are meant to be SW
>       representation of physical HW channels, while in Keem Bay IPC
>       channels are software abstractions to achieve communication
>       multiplexing over a single HW link
> 
>    b. Additionally, Keem Bay IPC has two different classes of channels 
>       (high-speed channels and general-purpose channels) that need to
>       access the same HW link with different priorities.
> 
> 2. The blocking / non-blocking TX behavior of mailbox channels is
>    defined at channel creation time (by the tx_block value of the
>    mailbox client passed to mbox_request_channel(); my understanding 
>    is that the tx_block value cannot be modified after the channel is
>    created), while in Keem Bay IPC the same channel can be used for
>    both blocking and non-blocking TX (behavior is controlled by the
>    timeout argument passed to keembay_ipc_send()).
> 
> Having said that, I guess that it could be possible to create a Mailbox
> driver implementing the core communication mechanism used by the Keem
> Bay IPC and then build our API around it (basically having two
> drivers). But I'm not sure that would make the code simpler or easier
> to maintain. Any thoughts on this?

The use of the mailbox binding and the kernel's mailbox framework are 
independent questions. I'm only concerned with the former (for purposes 
of this review).

Rob

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 02/22] dt-bindings: Add bindings for Keem Bay IPC driver
       [not found] <20201130230707.46351-1-mgross@linux.intel.com>
@ 2020-11-30 23:06 ` mgross
  0 siblings, 0 replies; 17+ messages in thread
From: mgross @ 2020-11-30 23:06 UTC (permalink / raw)
  To: linux-kernel
  Cc: markgross, mgross, adam.r.gretzinger, Daniele Alessandrelli,
	Rob Herring, devicetree

From: Daniele Alessandrelli <daniele.alessandrelli@intel.com>

Add DT binding documentation for the Intel Keem Bay IPC driver, which
enables communication between the Computing Sub-System (CSS) and the
Multimedia Sub-System (MSS) of the Intel Movidius SoC code named Keem
Bay.

Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org
Reviewed-by: Mark Gross <mgross@linux.intel.com>
Signed-off-by: Daniele Alessandrelli <daniele.alessandrelli@intel.com>
---
 .../bindings/soc/intel/intel,keembay-ipc.yaml | 63 +++++++++++++++++++
 1 file changed, 63 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml

diff --git a/Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml b/Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml
new file mode 100644
index 000000000000..6e21c54d8f34
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/intel/intel,keembay-ipc.yaml
@@ -0,0 +1,63 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2020 Intel Corporation
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/soc/intel/intel,keembay-ipc.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Keem Bay IPC
+
+maintainers:
+  - Daniele Alessandrelli <daniele.alessandrelli@intel.com>
+
+description:
+  The Keem Bay IPC driver enables Inter-Processor Communication (IPC) with the
+  Visual Processor Unit (VPU) embedded in the Intel Movidius SoC code named
+  Keem Bay.
+
+properties:
+  compatible:
+    const: intel,keembay-ipc
+
+  reg:
+    items:
+      - description: The CSS (CPU) FIFO registers
+      - description: The MSS (VPU) FIFO registers
+
+  reg-names:
+    items:
+      - const: css_fifo
+      - const: mss_fifo
+
+  interrupts:
+    items:
+      - description: CSS FIFO not-empty interrupt
+
+  interrupt-controller: true
+
+  memory-region:
+    items:
+      - description: Reserved memory region used for CSS IPC buffers
+      - description: Reserved memory region used for MSS IPC buffers
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - interrupts
+  - memory-region
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    ipc@203300f0 {
+          compatible = "intel,keembay-ipc";
+          reg = <0x203300f0 0x310>, /* CPU TIM FIFO */
+                <0x208200f0 0x310>; /* VPU TIM FIFO */
+          reg-names = "css_fifo", "mss_fifo";
+          interrupts = <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
+          memory-region = <&css_ipc_reserved>, <&mss_ipc_reserved>;
+    };
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2020-12-09 17:34 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20201201223511.65542-1-mgross@linux.intel.com>
2020-12-01 22:34 ` [PATCH 02/22] dt-bindings: Add bindings for Keem Bay IPC driver mgross
2020-12-07 16:01   ` Rob Herring
2020-12-07 18:22     ` mark gross
2020-12-07 18:42     ` Daniele Alessandrelli
2020-12-07 20:31       ` Jassi Brar
2020-12-09  0:12         ` mark gross
2020-12-09 16:49           ` Jassi Brar
2020-12-09 17:33       ` Rob Herring
2020-12-01 22:34 ` [PATCH 04/22] dt-bindings: Add bindings for Keem Bay VPU " mgross
2020-12-07 15:57   ` Rob Herring
2020-12-07 21:28     ` mark gross
2020-12-01 22:35 ` [PATCH 15/22] xlink-ipc: Add xlink ipc device tree bindings mgross
2020-12-07 15:58   ` Rob Herring
2020-12-07 21:41     ` mark gross
2020-12-01 22:35 ` [PATCH 17/22] xlink-core: Add xlink core " mgross
2020-12-07 16:02   ` Rob Herring
     [not found] <20201130230707.46351-1-mgross@linux.intel.com>
2020-11-30 23:06 ` [PATCH 02/22] dt-bindings: Add bindings for Keem Bay IPC driver mgross

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).