[PATCH v2 0/4] Add support of mt8183 APU

[PATCH v2 0/4] Add support of mt8183 APU

[Alexandre Bailon]

Some Mediatek's SoC have an Accelerated Processing Unit.
This adds support of the one available in the mt8183
(aswell some derivative SoC).

This series depends on two other series:
- Mediatek MT8183 scpsys support  
- arm64: dts: Add m4u and smi-larbs nodes for mt8183

Changes in v2:
- Drop the workarounds needed to load bad firmwares
- There are many name for the APU (most common one is VPU).
  Rename many functions and dts nodes to be more consistent.
- Use the bulk clock API, and enable / disable clock at a better place
- add few comments explaining how to start the APU
- update the way to use pinctl for JTAG
- fix some minors issues
- fix device tree bindings

Alexandre Bailon (4):
  dt bindings: remoteproc: Add bindings for MT8183 APU
  remoteproc: Add a remoteproc driver for the MT8183's APU
  remoteproc: mtk_vpu_rproc: Add support of JTAG
  ARM64: mt8183: Add support of APU to mt8183

 .../bindings/remoteproc/mtk,apu.yaml          | 107 +++++
 arch/arm64/boot/dts/mediatek/mt8183.dtsi      |  39 ++
 drivers/remoteproc/Kconfig                    |  19 +
 drivers/remoteproc/Makefile                   |   1 +
 drivers/remoteproc/mtk_apu.c                  | 437 ++++++++++++++++++
 5 files changed, 603 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/mtk,apu.yaml
 create mode 100644 drivers/remoteproc/mtk_apu.c

-- 
2.26.2

[PATCH v2 1/4] dt bindings: remoteproc: Add bindings for MT8183 APU

[Alexandre Bailon]

This adds dt bindings for the APU present in the MT8183.

Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
---
 .../bindings/remoteproc/mtk,apu.yaml          | 107 ++++++++++++++++++
 1 file changed, 107 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/mtk,apu.yaml

diff --git a/Documentation/devicetree/bindings/remoteproc/mtk,apu.yaml b/Documentation/devicetree/bindings/remoteproc/mtk,apu.yaml
new file mode 100644
index 000000000000..7a71d2f5c4e6
--- /dev/null
+++ b/Documentation/devicetree/bindings/remoteproc/mtk,apu.yaml
@@ -0,0 +1,107 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+
+---
+$id: "http://devicetree.org/schemas/remoteproc/mtk,apu.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: MT8183 AI Processor Unit (APU) a.k.a. Vision Processor Unit (VPU)
+
+description:
+  This document defines the binding for the APU, a co-processor that could
+  offload the CPU for machine learning and neural network.
+
+maintainers:
+  - Alexandre Bailon <abailon@bayLibre.com>
+
+properties:
+  compatible:
+    const: mediatek,mt8183-apu
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    description:
+      Three clocks are expected for AXI, IPU and JTAG.
+      The JTAG clock seems to be required to run the DSP,
+      even when JTAG is not in use."
+    maxItems: 3
+
+  clock-names:
+    items:
+      - const: axi
+      - const: ipu
+      - const: jtag
+
+  iommus:
+    maxItems: 3
+
+  memory-region:
+    maxItems: 1
+
+  power-domains:
+    maxItems: 1
+
+  pinctrl:
+    description: pinctrl handles, required to configure pins for JTAG.
+
+  pinctrl-names:
+    items:
+      - const: jtag
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - iommus
+  - memory-region
+  - power-domains
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/mt8183-clk.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/memory/mt8183-larb-port.h>
+    #include <dt-bindings/power/mt8183-power.h>
+
+    reserved-memory {
+      #address-cells = <1>;
+      #size-cells = <1>;
+      ranges;
+
+      apu_ram: apu_ram@0x60000000 {
+        compatible = "shared-dma-pool";
+        reg = <0x60000000 0x040000000>;
+        no-map;
+        linux,cma-default;
+      };
+    };
+
+    apu0: apu@19100000 {
+      compatible = "mediatek,mt8183-apu";
+      reg = <0x19180000 0x14000>;
+      interrupts = <GIC_SPI 292 IRQ_TYPE_LEVEL_LOW>;
+
+      iommus = <&iommu M4U_PORT_IMG_IPUO>,
+         <&iommu M4U_PORT_IMG_IPU3O>,
+         <&iommu M4U_PORT_IMG_IPUI>;
+
+      clocks = <&ipu_core0 CLK_IPU_CORE0_AXI>,
+         <&ipu_core0 CLK_IPU_CORE0_IPU>,
+         <&ipu_core0 CLK_IPU_CORE0_JTAG>;
+
+      clock-names = "axi", "ipu", "jtag";
+
+      power-domains = <&scpsys MT8183_POWER_DOMAIN_VPU_CORE0>;
+      memory-region = <&apu_ram>;
+    };
+...
-- 
2.26.2

Re: [PATCH v2 1/4] dt bindings: remoteproc: Add bindings for MT8183 APU

[Rob Herring]

On Thu, 10 Sep 2020 15:01:45 +0200, Alexandre Bailon wrote:
> This adds dt bindings for the APU present in the MT8183.
> 
> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
> ---
>  .../bindings/remoteproc/mtk,apu.yaml          | 107 ++++++++++++++++++
>  1 file changed, 107 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/remoteproc/mtk,apu.yaml
> 


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

Documentation/devicetree/bindings/remoteproc/mtk,apu.example.dts:22:18: fatal error: dt-bindings/power/mt8183-power.h: No such file or directory
   22 |         #include <dt-bindings/power/mt8183-power.h>
      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [scripts/Makefile.lib:342: Documentation/devicetree/bindings/remoteproc/mtk,apu.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1366: dt_binding_check] Error 2


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

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

pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade

Please check and re-submit.

Re: [PATCH v2 1/4] dt bindings: remoteproc: Add bindings for MT8183 APU

[Rob Herring]

On Thu, 10 Sep 2020 15:01:45 +0200, Alexandre Bailon wrote:
> This adds dt bindings for the APU present in the MT8183.
> 
> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
> ---
>  .../bindings/remoteproc/mtk,apu.yaml          | 107 ++++++++++++++++++
>  1 file changed, 107 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/remoteproc/mtk,apu.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v2 1/4] dt bindings: remoteproc: Add bindings for MT8183 APU

[Mathieu Poirier]

On Thu, Sep 10, 2020 at 03:01:45PM +0200, Alexandre Bailon wrote:
> This adds dt bindings for the APU present in the MT8183.
> 
> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
> ---
>  .../bindings/remoteproc/mtk,apu.yaml          | 107 ++++++++++++++++++
>  1 file changed, 107 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/remoteproc/mtk,apu.yaml
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/mtk,apu.yaml b/Documentation/devicetree/bindings/remoteproc/mtk,apu.yaml
> new file mode 100644
> index 000000000000..7a71d2f5c4e6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/remoteproc/mtk,apu.yaml
> @@ -0,0 +1,107 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +
> +---
> +$id: "http://devicetree.org/schemas/remoteproc/mtk,apu.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: MT8183 AI Processor Unit (APU) a.k.a. Vision Processor Unit (VPU)
> +
> +description:
> +  This document defines the binding for the APU, a co-processor that could
> +  offload the CPU for machine learning and neural network.
> +
> +maintainers:
> +  - Alexandre Bailon <abailon@bayLibre.com>
> +
> +properties:
> +  compatible:
> +    const: mediatek,mt8183-apu
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    description:
> +      Three clocks are expected for AXI, IPU and JTAG.
> +      The JTAG clock seems to be required to run the DSP,
> +      even when JTAG is not in use."
> +    maxItems: 3
> +
> +  clock-names:
> +    items:
> +      - const: axi
> +      - const: ipu
> +      - const: jtag
> +
> +  iommus:
> +    maxItems: 3
> +
> +  memory-region:
> +    maxItems: 1
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  pinctrl:
> +    description: pinctrl handles, required to configure pins for JTAG.
> +
> +  pinctrl-names:
> +    items:
> +      - const: jtag
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - iommus
> +  - memory-region
> +  - power-domains
> +
> +additionalProperties: false
> +

What about the pintctrl for the JTAG part?

> +examples:
> +  - |
> +    #include <dt-bindings/clock/mt8183-clk.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/memory/mt8183-larb-port.h>
> +    #include <dt-bindings/power/mt8183-power.h>
> +
> +    reserved-memory {
> +      #address-cells = <1>;
> +      #size-cells = <1>;
> +      ranges;
> +
> +      apu_ram: apu_ram@0x60000000 {
> +        compatible = "shared-dma-pool";
> +        reg = <0x60000000 0x040000000>;
> +        no-map;
> +        linux,cma-default;
> +      };
> +    };
> +
> +    apu0: apu@19100000 {
> +      compatible = "mediatek,mt8183-apu";
> +      reg = <0x19180000 0x14000>;
> +      interrupts = <GIC_SPI 292 IRQ_TYPE_LEVEL_LOW>;
> +
> +      iommus = <&iommu M4U_PORT_IMG_IPUO>,
> +         <&iommu M4U_PORT_IMG_IPU3O>,
> +         <&iommu M4U_PORT_IMG_IPUI>;
> +
> +      clocks = <&ipu_core0 CLK_IPU_CORE0_AXI>,
> +         <&ipu_core0 CLK_IPU_CORE0_IPU>,
> +         <&ipu_core0 CLK_IPU_CORE0_JTAG>;
> +
> +      clock-names = "axi", "ipu", "jtag";
> +
> +      power-domains = <&scpsys MT8183_POWER_DOMAIN_VPU_CORE0>;
> +      memory-region = <&apu_ram>;
> +    };
> +...
> -- 
> 2.26.2
> 

[PATCH v2 2/4] remoteproc: Add a remoteproc driver for the MT8183's APU

[Alexandre Bailon]

This adds a driver to control the APU present in the MT8183.
This loads the firmware and start the DSP.

Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
---
 drivers/remoteproc/Kconfig   |  10 ++
 drivers/remoteproc/Makefile  |   1 +
 drivers/remoteproc/mtk_apu.c | 288 +++++++++++++++++++++++++++++++++++
 3 files changed, 299 insertions(+)
 create mode 100644 drivers/remoteproc/mtk_apu.c

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index c6659dfea7c7..4ebea57bf4c8 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -51,6 +51,16 @@ config MTK_SCP
 
 	  It's safe to say N here.
 
+config MTK_APU
+	tristate "Mediatek APU remoteproc support"
+	depends on ARCH_MEDIATEK
+	depends on MTK_IOMMU
+	help
+	  Say y to support the Mediatek's Accelerated Processing Unit (APU) via
+	  the remote processor framework.
+
+	  It's safe to say N here.
+
 config OMAP_REMOTEPROC
 	tristate "OMAP remoteproc support"
 	depends on ARCH_OMAP4 || SOC_OMAP5 || SOC_DRA7XX
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index 3dfa28e6c701..174644f38fda 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_REMOTEPROC_CDEV)		+= remoteproc_cdev.o
 obj-$(CONFIG_IMX_REMOTEPROC)		+= imx_rproc.o
 obj-$(CONFIG_INGENIC_VPU_RPROC)		+= ingenic_rproc.o
 obj-$(CONFIG_MTK_SCP)			+= mtk_scp.o mtk_scp_ipi.o
+obj-$(CONFIG_MTK_APU)			+= mtk_apu.o
 obj-$(CONFIG_OMAP_REMOTEPROC)		+= omap_remoteproc.o
 obj-$(CONFIG_WKUP_M3_RPROC)		+= wkup_m3_rproc.o
 obj-$(CONFIG_DA8XX_REMOTEPROC)		+= da8xx_remoteproc.o
diff --git a/drivers/remoteproc/mtk_apu.c b/drivers/remoteproc/mtk_apu.c
new file mode 100644
index 000000000000..6d2f577cfde5
--- /dev/null
+++ b/drivers/remoteproc/mtk_apu.c
@@ -0,0 +1,288 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020 BayLibre SAS
+ */
+
+#include <linux/bitops.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/highmem.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/iommu.h>
+#include <linux/irq.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/platform_device.h>
+#include <linux/remoteproc.h>
+
+#include "remoteproc_internal.h"
+
+/* From MT8183 4.5 Vision Processor Unit (VPU).pdf datasheet */
+#define SW_RST					(0x0000000C)
+#define SW_RST_OCD_HALT_ON_RST			BIT(12)
+#define SW_RST_IPU_D_RST			BIT(8)
+#define SW_RST_IPU_B_RST			BIT(4)
+#define CORE_CTRL				(0x00000110)
+#define CORE_CTRL_PDEBUG_ENABLE			BIT(31)
+#define CORE_CTRL_SRAM_64K_iMEM			(0x00 << 27)
+#define CORE_CTRL_SRAM_96K_iMEM			(0x01 << 27)
+#define CORE_CTRL_SRAM_128K_iMEM		(0x02 << 27)
+#define CORE_CTRL_SRAM_192K_iMEM		(0x03 << 27)
+#define CORE_CTRL_SRAM_256K_iMEM		(0x04 << 27)
+#define CORE_CTRL_PBCLK_ENABLE			BIT(26)
+#define CORE_CTRL_RUN_STALL			BIT(23)
+#define CORE_CTRL_STATE_VECTOR_SELECT		BIT(19)
+#define CORE_CTRL_PIF_GATED			BIT(17)
+#define CORE_CTRL_NMI				BIT(0)
+#define CORE_XTENSA_INT				(0x00000114)
+#define CORE_CTL_XTENSA_INT			(0x00000118)
+#define CORE_DEFAULT0				(0x0000013C)
+#define CORE_DEFAULT0_QOS_SWAP_0		(0x00 << 28)
+#define CORE_DEFAULT0_QOS_SWAP_1		(0x01 << 28)
+#define CORE_DEFAULT0_QOS_SWAP_2		(0x02 << 28)
+#define CORE_DEFAULT0_QOS_SWAP_3		(0x03 << 28)
+#define CORE_DEFAULT0_ARUSER_USE_IOMMU		(0x10 << 23)
+#define CORE_DEFAULT0_AWUSER_USE_IOMMU		(0x10 << 18)
+#define CORE_DEFAULT1				(0x00000140)
+#define CORE_DEFAULT0_ARUSER_IDMA_USE_IOMMU	(0x10 << 0)
+#define CORE_DEFAULT0_AWUSER_IDMA_USE_IOMMU	(0x10 << 5)
+#define CORE_XTENSA_ALTRESETVEC			(0x000001F8)
+
+struct mtk_apu_rproc {
+	struct device *dev;
+	struct rproc *rproc;
+
+	void __iomem *base;
+	int irq;
+	struct clk_bulk_data clks[3];
+};
+
+static int mtk_apu_rproc_prepare(struct rproc *rproc)
+{
+	struct mtk_apu_rproc *apu_rproc = rproc->priv;
+	int ret;
+
+	ret = clk_bulk_prepare_enable(ARRAY_SIZE(apu_rproc->clks),
+				      apu_rproc->clks);
+	if (ret)
+		dev_err(apu_rproc->dev, "Failed to enable clocks\n");
+
+	return ret;
+}
+
+static int mtk_apu_rproc_unprepare(struct rproc *rproc)
+{
+	struct mtk_apu_rproc *apu_rproc = rproc->priv;
+
+	clk_bulk_disable_unprepare(ARRAY_SIZE(apu_rproc->clks),
+				   apu_rproc->clks);
+
+	return 0;
+}
+
+static int mtk_apu_rproc_start(struct rproc *rproc)
+{
+	struct mtk_apu_rproc *apu_rproc = rproc->priv;
+	u32 core_ctrl;
+
+	/* Set reset vector of APU firmware boot address */
+	writel(rproc->bootaddr, apu_rproc->base + CORE_XTENSA_ALTRESETVEC);
+
+	/* Turn on the clocks and stall the APU */
+	core_ctrl = readl(apu_rproc->base + CORE_CTRL);
+	core_ctrl |= CORE_CTRL_PDEBUG_ENABLE | CORE_CTRL_PBCLK_ENABLE |
+		     CORE_CTRL_STATE_VECTOR_SELECT | CORE_CTRL_RUN_STALL |
+		     CORE_CTRL_PIF_GATED;
+	writel(core_ctrl, apu_rproc->base + CORE_CTRL);
+
+	/* Reset the APU */
+	writel(SW_RST_OCD_HALT_ON_RST | SW_RST_IPU_B_RST | SW_RST_IPU_D_RST,
+		apu_rproc->base + SW_RST);
+	ndelay(27);
+	writel(0, apu_rproc->base + SW_RST);
+
+	core_ctrl &= ~CORE_CTRL_PIF_GATED;
+	writel(core_ctrl, apu_rproc->base + CORE_CTRL);
+
+	/* Configure memory accesses to go through the IOMMU */
+	writel(CORE_DEFAULT0_AWUSER_USE_IOMMU | CORE_DEFAULT0_ARUSER_USE_IOMMU |
+	      CORE_DEFAULT0_QOS_SWAP_1, apu_rproc->base + CORE_DEFAULT0);
+	writel(CORE_DEFAULT0_AWUSER_IDMA_USE_IOMMU |
+		CORE_DEFAULT0_ARUSER_IDMA_USE_IOMMU,
+		apu_rproc->base + CORE_DEFAULT1);
+
+	/* Release the APU */
+	core_ctrl &= ~CORE_CTRL_RUN_STALL;
+	writel(core_ctrl, apu_rproc->base + CORE_CTRL);
+
+	return 0;
+}
+
+static int mtk_apu_rproc_stop(struct rproc *rproc)
+{
+	struct mtk_apu_rproc *apu_rproc = rproc->priv;
+	u32 core_ctrl;
+
+	core_ctrl = readl(apu_rproc->base + CORE_CTRL);
+	writel(core_ctrl | CORE_CTRL_RUN_STALL, apu_rproc->base + CORE_CTRL);
+
+	return 0;
+}
+
+static void mtk_apu_rproc_kick(struct rproc *rproc, int vqid)
+{
+	struct mtk_apu_rproc *apu_rproc = rproc->priv;
+
+	writel(1 << vqid, apu_rproc->base + CORE_CTL_XTENSA_INT);
+}
+
+static const struct rproc_ops mtk_apu_rproc_ops = {
+	.prepare	= mtk_apu_rproc_prepare,
+	.unprepare	= mtk_apu_rproc_unprepare,
+	.start		= mtk_apu_rproc_start,
+	.stop		= mtk_apu_rproc_stop,
+	.kick		= mtk_apu_rproc_kick,
+};
+
+static irqreturn_t mtk_apu_rproc_callback(int irq, void *data)
+{
+	struct rproc *rproc = data;
+	struct mtk_apu_rproc *apu_rproc = (struct mtk_apu_rproc *)rproc->priv;
+
+	writel(1, apu_rproc->base + CORE_XTENSA_INT);
+
+	return IRQ_WAKE_THREAD;
+}
+
+static irqreturn_t handle_event(int irq, void *data)
+{
+	struct rproc *rproc = data;
+
+	rproc_vq_interrupt(rproc, 0);
+	rproc_vq_interrupt(rproc, 1);
+
+	return IRQ_HANDLED;
+}
+
+static int mtk_apu_rproc_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct mtk_apu_rproc *apu_rproc;
+	struct rproc *rproc;
+	struct resource *res;
+	int ret;
+
+	rproc = rproc_alloc(dev, dev_name(dev), &mtk_apu_rproc_ops, NULL,
+			    sizeof(*apu_rproc));
+	if (!rproc)
+		return -ENOMEM;
+
+	rproc->recovery_disabled = true;
+	rproc->has_iommu = false;
+
+	apu_rproc = rproc->priv;
+	apu_rproc->rproc = rproc;
+	apu_rproc->dev = dev;
+
+	platform_set_drvdata(pdev, rproc);
+
+	rproc->domain = iommu_get_domain_for_dev(dev);
+	if (!rproc->domain) {
+		dev_err(dev, "Failed to get the IOMMU domain\n");
+		ret = -EINVAL;
+		goto free_rproc;
+	}
+
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	apu_rproc->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(apu_rproc->base)) {
+		dev_err(dev, "Failed to map mmio\n");
+		ret = PTR_ERR(apu_rproc->base);
+		goto free_rproc;
+	}
+
+	apu_rproc->irq = platform_get_irq(pdev, 0);
+	if (apu_rproc->irq < 0) {
+		ret = apu_rproc->irq;
+		goto free_rproc;
+	}
+
+	ret = devm_request_threaded_irq(dev, apu_rproc->irq,
+					mtk_apu_rproc_callback, handle_event,
+					IRQF_SHARED | IRQF_ONESHOT,
+					NULL, rproc);
+	if (ret) {
+		dev_err(dev, "devm_request_threaded_irq error: %d\n", ret);
+		goto free_rproc;
+	}
+
+	apu_rproc->clks[0].id = "ipu";
+	apu_rproc->clks[1].id = "axi";
+	apu_rproc->clks[1].id = "jtag";
+
+	ret = devm_clk_bulk_get(dev, ARRAY_SIZE(apu_rproc->clks),
+				apu_rproc->clks);
+	if (ret) {
+		dev_err(dev, "Failed to get clocks\n");
+		goto free_rproc;
+	}
+
+	ret = of_reserved_mem_device_init(dev);
+	if (ret) {
+		dev_err(dev, "device does not have specific CMA pool\n");
+		goto free_rproc;
+	}
+
+	ret = rproc_add(rproc);
+	if (ret) {
+		dev_err(dev, "rproc_add failed: %d\n", ret);
+		goto free_mem;
+	}
+
+	return 0;
+
+free_mem:
+	of_reserved_mem_device_release(dev);
+free_rproc:
+	rproc_free(rproc);
+
+	return ret;
+}
+
+static int mtk_apu_rproc_remove(struct platform_device *pdev)
+{
+	struct rproc *rproc = platform_get_drvdata(pdev);
+	struct mtk_apu_rproc *apu_rproc = (struct mtk_apu_rproc *)rproc->priv;
+	struct device *dev = &pdev->dev;
+
+	disable_irq(apu_rproc->irq);
+
+	rproc_del(rproc);
+	of_reserved_mem_device_release(dev);
+	rproc_free(rproc);
+
+	return 0;
+}
+
+static const struct of_device_id mtk_apu_rproc_of_match[] = {
+	{ .compatible = "mediatek,mt8183-apu", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, mtk_apu_rproc_of_match);
+
+static struct platform_driver mtk_apu_rproc_driver = {
+	.probe = mtk_apu_rproc_probe,
+	.remove = mtk_apu_rproc_remove,
+	.driver = {
+		.name = "mtk_apu-rproc",
+		.of_match_table = of_match_ptr(mtk_apu_rproc_of_match),
+	},
+};
+module_platform_driver(mtk_apu_rproc_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Alexandre Bailon");
+MODULE_DESCRIPTION("MTK APU Remote Processor control driver");
-- 
2.26.2

Re: [PATCH v2 2/4] remoteproc: Add a remoteproc driver for the MT8183's APU

[Mathieu Poirier]

Hi Alexandre,

On Thu, Sep 10, 2020 at 03:01:46PM +0200, Alexandre Bailon wrote:
> This adds a driver to control the APU present in the MT8183.
> This loads the firmware and start the DSP.
> 
> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
> ---
>  drivers/remoteproc/Kconfig   |  10 ++
>  drivers/remoteproc/Makefile  |   1 +
>  drivers/remoteproc/mtk_apu.c | 288 +++++++++++++++++++++++++++++++++++
>  3 files changed, 299 insertions(+)
>  create mode 100644 drivers/remoteproc/mtk_apu.c
> 
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index c6659dfea7c7..4ebea57bf4c8 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -51,6 +51,16 @@ config MTK_SCP
>  
>  	  It's safe to say N here.
>  
> +config MTK_APU
> +	tristate "Mediatek APU remoteproc support"
> +	depends on ARCH_MEDIATEK
> +	depends on MTK_IOMMU
> +	help
> +	  Say y to support the Mediatek's Accelerated Processing Unit (APU) via
> +	  the remote processor framework.
> +
> +	  It's safe to say N here.
> +
>  config OMAP_REMOTEPROC
>  	tristate "OMAP remoteproc support"
>  	depends on ARCH_OMAP4 || SOC_OMAP5 || SOC_DRA7XX
> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> index 3dfa28e6c701..174644f38fda 100644
> --- a/drivers/remoteproc/Makefile
> +++ b/drivers/remoteproc/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_REMOTEPROC_CDEV)		+= remoteproc_cdev.o
>  obj-$(CONFIG_IMX_REMOTEPROC)		+= imx_rproc.o
>  obj-$(CONFIG_INGENIC_VPU_RPROC)		+= ingenic_rproc.o
>  obj-$(CONFIG_MTK_SCP)			+= mtk_scp.o mtk_scp_ipi.o
> +obj-$(CONFIG_MTK_APU)			+= mtk_apu.o
>  obj-$(CONFIG_OMAP_REMOTEPROC)		+= omap_remoteproc.o
>  obj-$(CONFIG_WKUP_M3_RPROC)		+= wkup_m3_rproc.o
>  obj-$(CONFIG_DA8XX_REMOTEPROC)		+= da8xx_remoteproc.o
> diff --git a/drivers/remoteproc/mtk_apu.c b/drivers/remoteproc/mtk_apu.c
> new file mode 100644
> index 000000000000..6d2f577cfde5
> --- /dev/null
> +++ b/drivers/remoteproc/mtk_apu.c
> @@ -0,0 +1,288 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 BayLibre SAS
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/highmem.h>

Not sure what this is for

> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/iommu.h>
> +#include <linux/irq.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_reserved_mem.h>
> +#include <linux/platform_device.h>
> +#include <linux/remoteproc.h>
> +
> +#include "remoteproc_internal.h"
> +
> +/* From MT8183 4.5 Vision Processor Unit (VPU).pdf datasheet */
> +#define SW_RST					(0x0000000C)
> +#define SW_RST_OCD_HALT_ON_RST			BIT(12)
> +#define SW_RST_IPU_D_RST			BIT(8)
> +#define SW_RST_IPU_B_RST			BIT(4)
> +#define CORE_CTRL				(0x00000110)
> +#define CORE_CTRL_PDEBUG_ENABLE			BIT(31)
> +#define CORE_CTRL_SRAM_64K_iMEM			(0x00 << 27)
> +#define CORE_CTRL_SRAM_96K_iMEM			(0x01 << 27)
> +#define CORE_CTRL_SRAM_128K_iMEM		(0x02 << 27)
> +#define CORE_CTRL_SRAM_192K_iMEM		(0x03 << 27)
> +#define CORE_CTRL_SRAM_256K_iMEM		(0x04 << 27)
> +#define CORE_CTRL_PBCLK_ENABLE			BIT(26)
> +#define CORE_CTRL_RUN_STALL			BIT(23)
> +#define CORE_CTRL_STATE_VECTOR_SELECT		BIT(19)
> +#define CORE_CTRL_PIF_GATED			BIT(17)
> +#define CORE_CTRL_NMI				BIT(0)
> +#define CORE_XTENSA_INT				(0x00000114)
> +#define CORE_CTL_XTENSA_INT			(0x00000118)
> +#define CORE_DEFAULT0				(0x0000013C)
> +#define CORE_DEFAULT0_QOS_SWAP_0		(0x00 << 28)
> +#define CORE_DEFAULT0_QOS_SWAP_1		(0x01 << 28)
> +#define CORE_DEFAULT0_QOS_SWAP_2		(0x02 << 28)
> +#define CORE_DEFAULT0_QOS_SWAP_3		(0x03 << 28)
> +#define CORE_DEFAULT0_ARUSER_USE_IOMMU		(0x10 << 23)
> +#define CORE_DEFAULT0_AWUSER_USE_IOMMU		(0x10 << 18)
> +#define CORE_DEFAULT1				(0x00000140)
> +#define CORE_DEFAULT0_ARUSER_IDMA_USE_IOMMU	(0x10 << 0)
> +#define CORE_DEFAULT0_AWUSER_IDMA_USE_IOMMU	(0x10 << 5)
> +#define CORE_XTENSA_ALTRESETVEC			(0x000001F8)
> +
> +struct mtk_apu_rproc {
> +	struct device *dev;
> +	struct rproc *rproc;

As far as I can tell @rproc is only used in apu_jtag_probe(), but it could just
as easily be given as a parameter to the function instead of bloating the
structure.

> +

Extra line

> +	void __iomem *base;
> +	int irq;
> +	struct clk_bulk_data clks[3];
> +};
> +
> +static int mtk_apu_rproc_prepare(struct rproc *rproc)
> +{
> +	struct mtk_apu_rproc *apu_rproc = rproc->priv;
> +	int ret;
> +
> +	ret = clk_bulk_prepare_enable(ARRAY_SIZE(apu_rproc->clks),
> +				      apu_rproc->clks);
> +	if (ret)
> +		dev_err(apu_rproc->dev, "Failed to enable clocks\n");
> +
> +	return ret;
> +}
> +
> +static int mtk_apu_rproc_unprepare(struct rproc *rproc)
> +{
> +	struct mtk_apu_rproc *apu_rproc = rproc->priv;
> +
> +	clk_bulk_disable_unprepare(ARRAY_SIZE(apu_rproc->clks),
> +				   apu_rproc->clks);
> +
> +	return 0;
> +}
> +
> +static int mtk_apu_rproc_start(struct rproc *rproc)
> +{
> +	struct mtk_apu_rproc *apu_rproc = rproc->priv;
> +	u32 core_ctrl;
> +
> +	/* Set reset vector of APU firmware boot address */
> +	writel(rproc->bootaddr, apu_rproc->base + CORE_XTENSA_ALTRESETVEC);
> +
> +	/* Turn on the clocks and stall the APU */
> +	core_ctrl = readl(apu_rproc->base + CORE_CTRL);
> +	core_ctrl |= CORE_CTRL_PDEBUG_ENABLE | CORE_CTRL_PBCLK_ENABLE |
> +		     CORE_CTRL_STATE_VECTOR_SELECT | CORE_CTRL_RUN_STALL |
> +		     CORE_CTRL_PIF_GATED;
> +	writel(core_ctrl, apu_rproc->base + CORE_CTRL);
> +
> +	/* Reset the APU */
> +	writel(SW_RST_OCD_HALT_ON_RST | SW_RST_IPU_B_RST | SW_RST_IPU_D_RST,
> +		apu_rproc->base + SW_RST);
> +	ndelay(27);

What is this for and why 27 nanosecond precicely?  Is this a board specific
setting?  Are we sure it is the same value on all platform with a MT8183? 

Matthias, what is your take on this? 

> +	writel(0, apu_rproc->base + SW_RST);
> +
> +	core_ctrl &= ~CORE_CTRL_PIF_GATED;
> +	writel(core_ctrl, apu_rproc->base + CORE_CTRL);
> +
> +	/* Configure memory accesses to go through the IOMMU */
> +	writel(CORE_DEFAULT0_AWUSER_USE_IOMMU | CORE_DEFAULT0_ARUSER_USE_IOMMU |
> +	      CORE_DEFAULT0_QOS_SWAP_1, apu_rproc->base + CORE_DEFAULT0);
> +	writel(CORE_DEFAULT0_AWUSER_IDMA_USE_IOMMU |
> +		CORE_DEFAULT0_ARUSER_IDMA_USE_IOMMU,
> +		apu_rproc->base + CORE_DEFAULT1);
> +
> +	/* Release the APU */
> +	core_ctrl &= ~CORE_CTRL_RUN_STALL;
> +	writel(core_ctrl, apu_rproc->base + CORE_CTRL);
> +
> +	return 0;
> +}
> +
> +static int mtk_apu_rproc_stop(struct rproc *rproc)
> +{
> +	struct mtk_apu_rproc *apu_rproc = rproc->priv;
> +	u32 core_ctrl;
> +
> +	core_ctrl = readl(apu_rproc->base + CORE_CTRL);
> +	writel(core_ctrl | CORE_CTRL_RUN_STALL, apu_rproc->base + CORE_CTRL);
> +
> +	return 0;
> +}
> +
> +static void mtk_apu_rproc_kick(struct rproc *rproc, int vqid)
> +{
> +	struct mtk_apu_rproc *apu_rproc = rproc->priv;
> +
> +	writel(1 << vqid, apu_rproc->base + CORE_CTL_XTENSA_INT);
> +}
> +
> +static const struct rproc_ops mtk_apu_rproc_ops = {
> +	.prepare	= mtk_apu_rproc_prepare,
> +	.unprepare	= mtk_apu_rproc_unprepare,
> +	.start		= mtk_apu_rproc_start,
> +	.stop		= mtk_apu_rproc_stop,
> +	.kick		= mtk_apu_rproc_kick,
> +};
> +
> +static irqreturn_t mtk_apu_rproc_callback(int irq, void *data)
> +{
> +	struct rproc *rproc = data;
> +	struct mtk_apu_rproc *apu_rproc = (struct mtk_apu_rproc *)rproc->priv;
> +
> +	writel(1, apu_rproc->base + CORE_XTENSA_INT);
> +
> +	return IRQ_WAKE_THREAD;
> +}
> +
> +static irqreturn_t handle_event(int irq, void *data)
> +{
> +	struct rproc *rproc = data;
> +
> +	rproc_vq_interrupt(rproc, 0);
> +	rproc_vq_interrupt(rproc, 1);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int mtk_apu_rproc_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct mtk_apu_rproc *apu_rproc;
> +	struct rproc *rproc;
> +	struct resource *res;
> +	int ret;
> +
> +	rproc = rproc_alloc(dev, dev_name(dev), &mtk_apu_rproc_ops, NULL,
> +			    sizeof(*apu_rproc));
> +	if (!rproc)
> +		return -ENOMEM;
> +
> +	rproc->recovery_disabled = true;
> +	rproc->has_iommu = false;
> +
> +	apu_rproc = rproc->priv;
> +	apu_rproc->rproc = rproc;
> +	apu_rproc->dev = dev;
> +
> +	platform_set_drvdata(pdev, rproc);
> +
> +	rproc->domain = iommu_get_domain_for_dev(dev);

Any reason why iommu_get_domain_for_dev() is explicitly called rather than
setting ->has_iommu to true and let the remoteproc core get the domain for
you?

> +	if (!rproc->domain) {
> +		dev_err(dev, "Failed to get the IOMMU domain\n");
> +		ret = -EINVAL;
> +		goto free_rproc;
> +	}
> +
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	apu_rproc->base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(apu_rproc->base)) {
> +		dev_err(dev, "Failed to map mmio\n");
> +		ret = PTR_ERR(apu_rproc->base);
> +		goto free_rproc;
> +	}
> +
> +	apu_rproc->irq = platform_get_irq(pdev, 0);
> +	if (apu_rproc->irq < 0) {
> +		ret = apu_rproc->irq;
> +		goto free_rproc;
> +	}
> +
> +	ret = devm_request_threaded_irq(dev, apu_rproc->irq,
> +					mtk_apu_rproc_callback, handle_event,
> +					IRQF_SHARED | IRQF_ONESHOT,
> +					NULL, rproc);
> +	if (ret) {
> +		dev_err(dev, "devm_request_threaded_irq error: %d\n", ret);
> +		goto free_rproc;
> +	}
> +
> +	apu_rproc->clks[0].id = "ipu";
> +	apu_rproc->clks[1].id = "axi";
> +	apu_rproc->clks[1].id = "jtag";
> +
> +	ret = devm_clk_bulk_get(dev, ARRAY_SIZE(apu_rproc->clks),
> +				apu_rproc->clks);
> +	if (ret) {
> +		dev_err(dev, "Failed to get clocks\n");
> +		goto free_rproc;
> +	}
> +
> +	ret = of_reserved_mem_device_init(dev);
> +	if (ret) {
> +		dev_err(dev, "device does not have specific CMA pool\n");
> +		goto free_rproc;
> +	}
> +
> +	ret = rproc_add(rproc);
> +	if (ret) {
> +		dev_err(dev, "rproc_add failed: %d\n", ret);
> +		goto free_mem;
> +	}
> +
> +	return 0;
> +
> +free_mem:
> +	of_reserved_mem_device_release(dev);
> +free_rproc:
> +	rproc_free(rproc);
> +
> +	return ret;
> +}
> +
> +static int mtk_apu_rproc_remove(struct platform_device *pdev)
> +{
> +	struct rproc *rproc = platform_get_drvdata(pdev);
> +	struct mtk_apu_rproc *apu_rproc = (struct mtk_apu_rproc *)rproc->priv;
> +	struct device *dev = &pdev->dev;
> +
> +	disable_irq(apu_rproc->irq);
> +
> +	rproc_del(rproc);
> +	of_reserved_mem_device_release(dev);
> +	rproc_free(rproc);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id mtk_apu_rproc_of_match[] = {
> +	{ .compatible = "mediatek,mt8183-apu", },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, mtk_apu_rproc_of_match);
> +
> +static struct platform_driver mtk_apu_rproc_driver = {
> +	.probe = mtk_apu_rproc_probe,
> +	.remove = mtk_apu_rproc_remove,
> +	.driver = {
> +		.name = "mtk_apu-rproc",
> +		.of_match_table = of_match_ptr(mtk_apu_rproc_of_match),
> +	},
> +};
> +module_platform_driver(mtk_apu_rproc_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Alexandre Bailon");
> +MODULE_DESCRIPTION("MTK APU Remote Processor control driver");
> -- 
> 2.26.2
> 

Re: [PATCH v2 2/4] remoteproc: Add a remoteproc driver for the MT8183's APU

[Alexandre Bailon]

Hi Mathieu,
> Hi Alexandre,
>
> On Thu, Sep 10, 2020 at 03:01:46PM +0200, Alexandre Bailon wrote:
>> This adds a driver to control the APU present in the MT8183.
>> This loads the firmware and start the DSP.
>>
>> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
>> ---
>>   drivers/remoteproc/Kconfig   |  10 ++
>>   drivers/remoteproc/Makefile  |   1 +
>>   drivers/remoteproc/mtk_apu.c | 288 +++++++++++++++++++++++++++++++++++
>>   3 files changed, 299 insertions(+)
>>   create mode 100644 drivers/remoteproc/mtk_apu.c
>>
>> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
>> index c6659dfea7c7..4ebea57bf4c8 100644
>> --- a/drivers/remoteproc/Kconfig
>> +++ b/drivers/remoteproc/Kconfig
>> @@ -51,6 +51,16 @@ config MTK_SCP
>>   
>>   	  It's safe to say N here.
>>   
>> +config MTK_APU
>> +	tristate "Mediatek APU remoteproc support"
>> +	depends on ARCH_MEDIATEK
>> +	depends on MTK_IOMMU
>> +	help
>> +	  Say y to support the Mediatek's Accelerated Processing Unit (APU) via
>> +	  the remote processor framework.
>> +
>> +	  It's safe to say N here.
>> +
>>   config OMAP_REMOTEPROC
>>   	tristate "OMAP remoteproc support"
>>   	depends on ARCH_OMAP4 || SOC_OMAP5 || SOC_DRA7XX
>> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
>> index 3dfa28e6c701..174644f38fda 100644
>> --- a/drivers/remoteproc/Makefile
>> +++ b/drivers/remoteproc/Makefile
>> @@ -14,6 +14,7 @@ obj-$(CONFIG_REMOTEPROC_CDEV)		+= remoteproc_cdev.o
>>   obj-$(CONFIG_IMX_REMOTEPROC)		+= imx_rproc.o
>>   obj-$(CONFIG_INGENIC_VPU_RPROC)		+= ingenic_rproc.o
>>   obj-$(CONFIG_MTK_SCP)			+= mtk_scp.o mtk_scp_ipi.o
>> +obj-$(CONFIG_MTK_APU)			+= mtk_apu.o
>>   obj-$(CONFIG_OMAP_REMOTEPROC)		+= omap_remoteproc.o
>>   obj-$(CONFIG_WKUP_M3_RPROC)		+= wkup_m3_rproc.o
>>   obj-$(CONFIG_DA8XX_REMOTEPROC)		+= da8xx_remoteproc.o
>> diff --git a/drivers/remoteproc/mtk_apu.c b/drivers/remoteproc/mtk_apu.c
>> new file mode 100644
>> index 000000000000..6d2f577cfde5
>> --- /dev/null
>> +++ b/drivers/remoteproc/mtk_apu.c
>> @@ -0,0 +1,288 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2020 BayLibre SAS
>> + */
>> +
>> +#include <linux/bitops.h>
>> +#include <linux/clk.h>
>> +#include <linux/delay.h>
>> +#include <linux/highmem.h>
> Not sure what this is for
>
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +#include <linux/iommu.h>
>> +#include <linux/irq.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of_reserved_mem.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/remoteproc.h>
>> +
>> +#include "remoteproc_internal.h"
>> +
>> +/* From MT8183 4.5 Vision Processor Unit (VPU).pdf datasheet */
>> +#define SW_RST					(0x0000000C)
>> +#define SW_RST_OCD_HALT_ON_RST			BIT(12)
>> +#define SW_RST_IPU_D_RST			BIT(8)
>> +#define SW_RST_IPU_B_RST			BIT(4)
>> +#define CORE_CTRL				(0x00000110)
>> +#define CORE_CTRL_PDEBUG_ENABLE			BIT(31)
>> +#define CORE_CTRL_SRAM_64K_iMEM			(0x00 << 27)
>> +#define CORE_CTRL_SRAM_96K_iMEM			(0x01 << 27)
>> +#define CORE_CTRL_SRAM_128K_iMEM		(0x02 << 27)
>> +#define CORE_CTRL_SRAM_192K_iMEM		(0x03 << 27)
>> +#define CORE_CTRL_SRAM_256K_iMEM		(0x04 << 27)
>> +#define CORE_CTRL_PBCLK_ENABLE			BIT(26)
>> +#define CORE_CTRL_RUN_STALL			BIT(23)
>> +#define CORE_CTRL_STATE_VECTOR_SELECT		BIT(19)
>> +#define CORE_CTRL_PIF_GATED			BIT(17)
>> +#define CORE_CTRL_NMI				BIT(0)
>> +#define CORE_XTENSA_INT				(0x00000114)
>> +#define CORE_CTL_XTENSA_INT			(0x00000118)
>> +#define CORE_DEFAULT0				(0x0000013C)
>> +#define CORE_DEFAULT0_QOS_SWAP_0		(0x00 << 28)
>> +#define CORE_DEFAULT0_QOS_SWAP_1		(0x01 << 28)
>> +#define CORE_DEFAULT0_QOS_SWAP_2		(0x02 << 28)
>> +#define CORE_DEFAULT0_QOS_SWAP_3		(0x03 << 28)
>> +#define CORE_DEFAULT0_ARUSER_USE_IOMMU		(0x10 << 23)
>> +#define CORE_DEFAULT0_AWUSER_USE_IOMMU		(0x10 << 18)
>> +#define CORE_DEFAULT1				(0x00000140)
>> +#define CORE_DEFAULT0_ARUSER_IDMA_USE_IOMMU	(0x10 << 0)
>> +#define CORE_DEFAULT0_AWUSER_IDMA_USE_IOMMU	(0x10 << 5)
>> +#define CORE_XTENSA_ALTRESETVEC			(0x000001F8)
>> +
>> +struct mtk_apu_rproc {
>> +	struct device *dev;
>> +	struct rproc *rproc;
> As far as I can tell @rproc is only used in apu_jtag_probe(), but it could just
> as easily be given as a parameter to the function instead of bloating the
> structure.
>
>> +
> Extra line
>
>> +	void __iomem *base;
>> +	int irq;
>> +	struct clk_bulk_data clks[3];
>> +};
>> +
>> +static int mtk_apu_rproc_prepare(struct rproc *rproc)
>> +{
>> +	struct mtk_apu_rproc *apu_rproc = rproc->priv;
>> +	int ret;
>> +
>> +	ret = clk_bulk_prepare_enable(ARRAY_SIZE(apu_rproc->clks),
>> +				      apu_rproc->clks);
>> +	if (ret)
>> +		dev_err(apu_rproc->dev, "Failed to enable clocks\n");
>> +
>> +	return ret;
>> +}
>> +
>> +static int mtk_apu_rproc_unprepare(struct rproc *rproc)
>> +{
>> +	struct mtk_apu_rproc *apu_rproc = rproc->priv;
>> +
>> +	clk_bulk_disable_unprepare(ARRAY_SIZE(apu_rproc->clks),
>> +				   apu_rproc->clks);
>> +
>> +	return 0;
>> +}
>> +
>> +static int mtk_apu_rproc_start(struct rproc *rproc)
>> +{
>> +	struct mtk_apu_rproc *apu_rproc = rproc->priv;
>> +	u32 core_ctrl;
>> +
>> +	/* Set reset vector of APU firmware boot address */
>> +	writel(rproc->bootaddr, apu_rproc->base + CORE_XTENSA_ALTRESETVEC);
>> +
>> +	/* Turn on the clocks and stall the APU */
>> +	core_ctrl = readl(apu_rproc->base + CORE_CTRL);
>> +	core_ctrl |= CORE_CTRL_PDEBUG_ENABLE | CORE_CTRL_PBCLK_ENABLE |
>> +		     CORE_CTRL_STATE_VECTOR_SELECT | CORE_CTRL_RUN_STALL |
>> +		     CORE_CTRL_PIF_GATED;
>> +	writel(core_ctrl, apu_rproc->base + CORE_CTRL);
>> +
>> +	/* Reset the APU */
>> +	writel(SW_RST_OCD_HALT_ON_RST | SW_RST_IPU_B_RST | SW_RST_IPU_D_RST,
>> +		apu_rproc->base + SW_RST);
>> +	ndelay(27);
> What is this for and why 27 nanosecond precicely?  Is this a board specific
> setting?  Are we sure it is the same value on all platform with a MT8183?
>
> Matthias, what is your take on this?
I have got the response recently from the HW team and this 27 nanosecond 
delay
is the minimum delay required to reset the APU, and that should work at 
any frequency.
This delay is supposed to work for the mt8183 but also for the other 
platform
including an APU.

The HW also confirmed that there are no way to know if the reset has 
been completed
or not so we must way that delay.

Maybe I should create a define instead of using directly 27 here, and 
also add some documentation
to explain why we need this delay.

What do you think about it ?
>
>> +	writel(0, apu_rproc->base + SW_RST);
>> +
>> +	core_ctrl &= ~CORE_CTRL_PIF_GATED;
>> +	writel(core_ctrl, apu_rproc->base + CORE_CTRL);
>> +
>> +	/* Configure memory accesses to go through the IOMMU */
>> +	writel(CORE_DEFAULT0_AWUSER_USE_IOMMU | CORE_DEFAULT0_ARUSER_USE_IOMMU |
>> +	      CORE_DEFAULT0_QOS_SWAP_1, apu_rproc->base + CORE_DEFAULT0);
>> +	writel(CORE_DEFAULT0_AWUSER_IDMA_USE_IOMMU |
>> +		CORE_DEFAULT0_ARUSER_IDMA_USE_IOMMU,
>> +		apu_rproc->base + CORE_DEFAULT1);
>> +
>> +	/* Release the APU */
>> +	core_ctrl &= ~CORE_CTRL_RUN_STALL;
>> +	writel(core_ctrl, apu_rproc->base + CORE_CTRL);
>> +
>> +	return 0;
>> +}
>> +
>> +static int mtk_apu_rproc_stop(struct rproc *rproc)
>> +{
>> +	struct mtk_apu_rproc *apu_rproc = rproc->priv;
>> +	u32 core_ctrl;
>> +
>> +	core_ctrl = readl(apu_rproc->base + CORE_CTRL);
>> +	writel(core_ctrl | CORE_CTRL_RUN_STALL, apu_rproc->base + CORE_CTRL);
>> +
>> +	return 0;
>> +}
>> +
>> +static void mtk_apu_rproc_kick(struct rproc *rproc, int vqid)
>> +{
>> +	struct mtk_apu_rproc *apu_rproc = rproc->priv;
>> +
>> +	writel(1 << vqid, apu_rproc->base + CORE_CTL_XTENSA_INT);
>> +}
>> +
>> +static const struct rproc_ops mtk_apu_rproc_ops = {
>> +	.prepare	= mtk_apu_rproc_prepare,
>> +	.unprepare	= mtk_apu_rproc_unprepare,
>> +	.start		= mtk_apu_rproc_start,
>> +	.stop		= mtk_apu_rproc_stop,
>> +	.kick		= mtk_apu_rproc_kick,
>> +};
>> +
>> +static irqreturn_t mtk_apu_rproc_callback(int irq, void *data)
>> +{
>> +	struct rproc *rproc = data;
>> +	struct mtk_apu_rproc *apu_rproc = (struct mtk_apu_rproc *)rproc->priv;
>> +
>> +	writel(1, apu_rproc->base + CORE_XTENSA_INT);
>> +
>> +	return IRQ_WAKE_THREAD;
>> +}
>> +
>> +static irqreturn_t handle_event(int irq, void *data)
>> +{
>> +	struct rproc *rproc = data;
>> +
>> +	rproc_vq_interrupt(rproc, 0);
>> +	rproc_vq_interrupt(rproc, 1);
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static int mtk_apu_rproc_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct mtk_apu_rproc *apu_rproc;
>> +	struct rproc *rproc;
>> +	struct resource *res;
>> +	int ret;
>> +
>> +	rproc = rproc_alloc(dev, dev_name(dev), &mtk_apu_rproc_ops, NULL,
>> +			    sizeof(*apu_rproc));
>> +	if (!rproc)
>> +		return -ENOMEM;
>> +
>> +	rproc->recovery_disabled = true;
>> +	rproc->has_iommu = false;
>> +
>> +	apu_rproc = rproc->priv;
>> +	apu_rproc->rproc = rproc;
>> +	apu_rproc->dev = dev;
>> +
>> +	platform_set_drvdata(pdev, rproc);
>> +
>> +	rproc->domain = iommu_get_domain_for_dev(dev);
> Any reason why iommu_get_domain_for_dev() is explicitly called rather than
> setting ->has_iommu to true and let the remoteproc core get the domain for
> you?

If I set ->has_iommu to true, the remoteproc framework will try to get a 
unmanaged domain
which is actually not supported by the iommu driver.
I don't like that solution but when I sent this patch, it was the only 
solution I had to make it work.

Now, I have to options:
- I update the iommu driver to support unmanaged domain
   (I made a PoC and this works but I am not sure that I made it correctly)
- I don't let the remoteproc framework manage the iommu 
(has_iommu=false, and domain=NULL)
   and I manage the iommu in this driver.
I have chosen the second options the v3 because this also make easier to 
manage some hardware
constraint.

Best Regards,
Alexandre

>
>> +	if (!rproc->domain) {
>> +		dev_err(dev, "Failed to get the IOMMU domain\n");
>> +		ret = -EINVAL;
>> +		goto free_rproc;
>> +	}
>> +
>> +
>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	apu_rproc->base = devm_ioremap_resource(dev, res);
>> +	if (IS_ERR(apu_rproc->base)) {
>> +		dev_err(dev, "Failed to map mmio\n");
>> +		ret = PTR_ERR(apu_rproc->base);
>> +		goto free_rproc;
>> +	}
>> +
>> +	apu_rproc->irq = platform_get_irq(pdev, 0);
>> +	if (apu_rproc->irq < 0) {
>> +		ret = apu_rproc->irq;
>> +		goto free_rproc;
>> +	}
>> +
>> +	ret = devm_request_threaded_irq(dev, apu_rproc->irq,
>> +					mtk_apu_rproc_callback, handle_event,
>> +					IRQF_SHARED | IRQF_ONESHOT,
>> +					NULL, rproc);
>> +	if (ret) {
>> +		dev_err(dev, "devm_request_threaded_irq error: %d\n", ret);
>> +		goto free_rproc;
>> +	}
>> +
>> +	apu_rproc->clks[0].id = "ipu";
>> +	apu_rproc->clks[1].id = "axi";
>> +	apu_rproc->clks[1].id = "jtag";
>> +
>> +	ret = devm_clk_bulk_get(dev, ARRAY_SIZE(apu_rproc->clks),
>> +				apu_rproc->clks);
>> +	if (ret) {
>> +		dev_err(dev, "Failed to get clocks\n");
>> +		goto free_rproc;
>> +	}
>> +
>> +	ret = of_reserved_mem_device_init(dev);
>> +	if (ret) {
>> +		dev_err(dev, "device does not have specific CMA pool\n");
>> +		goto free_rproc;
>> +	}
>> +
>> +	ret = rproc_add(rproc);
>> +	if (ret) {
>> +		dev_err(dev, "rproc_add failed: %d\n", ret);
>> +		goto free_mem;
>> +	}
>> +
>> +	return 0;
>> +
>> +free_mem:
>> +	of_reserved_mem_device_release(dev);
>> +free_rproc:
>> +	rproc_free(rproc);
>> +
>> +	return ret;
>> +}
>> +
>> +static int mtk_apu_rproc_remove(struct platform_device *pdev)
>> +{
>> +	struct rproc *rproc = platform_get_drvdata(pdev);
>> +	struct mtk_apu_rproc *apu_rproc = (struct mtk_apu_rproc *)rproc->priv;
>> +	struct device *dev = &pdev->dev;
>> +
>> +	disable_irq(apu_rproc->irq);
>> +
>> +	rproc_del(rproc);
>> +	of_reserved_mem_device_release(dev);
>> +	rproc_free(rproc);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct of_device_id mtk_apu_rproc_of_match[] = {
>> +	{ .compatible = "mediatek,mt8183-apu", },
>> +	{ /* sentinel */ },
>> +};
>> +MODULE_DEVICE_TABLE(of, mtk_apu_rproc_of_match);
>> +
>> +static struct platform_driver mtk_apu_rproc_driver = {
>> +	.probe = mtk_apu_rproc_probe,
>> +	.remove = mtk_apu_rproc_remove,
>> +	.driver = {
>> +		.name = "mtk_apu-rproc",
>> +		.of_match_table = of_match_ptr(mtk_apu_rproc_of_match),
>> +	},
>> +};
>> +module_platform_driver(mtk_apu_rproc_driver);
>> +
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_AUTHOR("Alexandre Bailon");
>> +MODULE_DESCRIPTION("MTK APU Remote Processor control driver");
>> -- 
>> 2.26.2
>>

[PATCH v2 3/4] remoteproc: mtk_vpu_rproc: Add support of JTAG

[Alexandre Bailon]

The DSP could be debugged using JTAG.
The support of JTAG could enabled at build time and it could be enabled
using debugfs.

Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
---
 drivers/remoteproc/Kconfig   |   9 +++
 drivers/remoteproc/mtk_apu.c | 151 ++++++++++++++++++++++++++++++++++-
 2 files changed, 159 insertions(+), 1 deletion(-)

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 4ebea57bf4c8..310462346bd8 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -61,6 +61,15 @@ config MTK_APU
 
 	  It's safe to say N here.
 
+config MTK_APU_JTAG
+	bool "Enable support of JTAG"
+	depends on MTK_APU
+	help
+	  Say y to enable support of JTAG.
+	  By default, JTAG will remain disabled until it is enabled using
+	  debugfs: remoteproc/remoteproc0/jtag. Write 1 to enable it and
+	  0 to disable it.
+
 config OMAP_REMOTEPROC
 	tristate "OMAP remoteproc support"
 	depends on ARCH_OMAP4 || SOC_OMAP5 || SOC_DRA7XX
diff --git a/drivers/remoteproc/mtk_apu.c b/drivers/remoteproc/mtk_apu.c
index 6d2f577cfde5..07157fdc24ba 100644
--- a/drivers/remoteproc/mtk_apu.c
+++ b/drivers/remoteproc/mtk_apu.c
@@ -5,6 +5,7 @@
 
 #include <linux/bitops.h>
 #include <linux/clk.h>
+#include <linux/debugfs.h>
 #include <linux/delay.h>
 #include <linux/highmem.h>
 #include <linux/interrupt.h>
@@ -14,6 +15,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of_reserved_mem.h>
+#include <linux/pinctrl/consumer.h>
 #include <linux/platform_device.h>
 #include <linux/remoteproc.h>
 
@@ -48,6 +50,11 @@
 #define CORE_DEFAULT1				(0x00000140)
 #define CORE_DEFAULT0_ARUSER_IDMA_USE_IOMMU	(0x10 << 0)
 #define CORE_DEFAULT0_AWUSER_IDMA_USE_IOMMU	(0x10 << 5)
+#define CORE_DEFAULT2				(0x00000144)
+#define CORE_DEFAULT2_DBG_EN			BIT(3)
+#define CORE_DEFAULT2_NIDEN			BIT(2)
+#define CORE_DEFAULT2_SPNIDEN			BIT(1)
+#define CORE_DEFAULT2_SPIDEN			BIT(0)
 #define CORE_XTENSA_ALTRESETVEC			(0x000001F8)
 
 struct mtk_apu_rproc {
@@ -57,6 +64,13 @@ struct mtk_apu_rproc {
 	void __iomem *base;
 	int irq;
 	struct clk_bulk_data clks[3];
+
+#ifdef CONFIG_MTK_APU_JTAG
+	struct pinctrl *pinctrl;
+	struct pinctrl_state *pinctrl_jtag;
+	bool jtag_enabled;
+	struct mutex jtag_mutex;
+#endif
 };
 
 static int mtk_apu_rproc_prepare(struct rproc *rproc)
@@ -166,6 +180,137 @@ static irqreturn_t handle_event(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
+#ifdef CONFIG_MTK_APU_JTAG
+
+static int apu_enable_jtag(struct mtk_apu_rproc *apu_rproc)
+{
+	int ret = 0;
+
+	mutex_lock(&apu_rproc->jtag_mutex);
+	if (apu_rproc->jtag_enabled) {
+		ret = -EINVAL;
+		goto err_mutex_unlock;
+	}
+
+	writel(CORE_DEFAULT2_SPNIDEN | CORE_DEFAULT2_SPIDEN |
+		CORE_DEFAULT2_NIDEN | CORE_DEFAULT2_DBG_EN,
+		apu_rproc->base + CORE_DEFAULT2);
+
+	apu_rproc->jtag_enabled = 1;
+
+err_mutex_unlock:
+	mutex_unlock(&apu_rproc->jtag_mutex);
+
+	return ret;
+}
+
+static int apu_disable_jtag(struct mtk_apu_rproc *apu_rproc)
+{
+	int ret = 0;
+
+	mutex_lock(&apu_rproc->jtag_mutex);
+	if (!apu_rproc->jtag_enabled) {
+		ret = -EINVAL;
+		goto err_mutex_unlock;
+	}
+
+	writel(0, apu_rproc->base + CORE_DEFAULT2);
+
+	apu_rproc->jtag_enabled = 0;
+
+err_mutex_unlock:
+	mutex_unlock(&apu_rproc->jtag_mutex);
+
+	return ret;
+}
+
+static ssize_t rproc_jtag_read(struct file *filp, char __user *userbuf,
+			       size_t count, loff_t *ppos)
+{
+	struct rproc *rproc = filp->private_data;
+	struct mtk_apu_rproc *apu_rproc = (struct mtk_apu_rproc *)rproc->priv;
+	char *buf = apu_rproc->jtag_enabled ? "enabled\n" : "disabled\n";
+
+	return simple_read_from_buffer(userbuf, count, ppos, buf, strlen(buf));
+}
+
+static ssize_t rproc_jtag_write(struct file *filp, const char __user *user_buf,
+				size_t count, loff_t *ppos)
+{
+	struct rproc *rproc = filp->private_data;
+	struct mtk_apu_rproc *apu_rproc = (struct mtk_apu_rproc *)rproc->priv;
+	char buf[10];
+	int ret;
+
+	if (count < 1 || count > sizeof(buf))
+		return -EINVAL;
+
+	ret = copy_from_user(buf, user_buf, count);
+	if (ret)
+		return -EFAULT;
+
+	/* remove end of line */
+	if (buf[count - 1] == '\n')
+		buf[count - 1] = '\0';
+
+	if (!strncmp(buf, "enabled", count))
+		ret = apu_enable_jtag(apu_rproc);
+	else if (!strncmp(buf, "disabled", count))
+		ret = apu_disable_jtag(apu_rproc);
+	else
+		return -EINVAL;
+
+	return ret ? ret : count;
+}
+
+static const struct file_operations rproc_jtag_ops = {
+	.read = rproc_jtag_read,
+	.write = rproc_jtag_write,
+	.open = simple_open,
+};
+
+static int apu_jtag_probe(struct mtk_apu_rproc *apu_rproc)
+{
+	int ret;
+
+	if (!apu_rproc->rproc->dbg_dir)
+		return -ENODEV;
+
+	apu_rproc->pinctrl = devm_pinctrl_get(apu_rproc->dev);
+	if (IS_ERR(apu_rproc->pinctrl)) {
+		dev_warn(apu_rproc->dev, "Failed to find JTAG pinctrl\n");
+		return PTR_ERR(apu_rproc->pinctrl);
+	}
+
+	apu_rproc->pinctrl_jtag = pinctrl_lookup_state(apu_rproc->pinctrl,
+						       "jtag");
+	if (IS_ERR(apu_rproc->pinctrl_jtag))
+		return PTR_ERR(apu_rproc->pinctrl_jtag);
+
+	ret = pinctrl_select_state(apu_rproc->pinctrl,
+				   apu_rproc->pinctrl_jtag);
+	if (ret < 0)
+		return ret;
+
+	mutex_init(&apu_rproc->jtag_mutex);
+
+	debugfs_create_file("jtag", 0600, apu_rproc->rproc->dbg_dir,
+			    apu_rproc->rproc, &rproc_jtag_ops);
+
+	return 0;
+}
+#else
+static int apu_jtag_probe(struct mtk_apu_rproc *apu_rproc)
+{
+	return 0;
+}
+
+static int apu_disable_jtag(struct mtk_apu_rproc *apu_rproc)
+{
+	return 0;
+}
+#endif /* CONFIG_MTK_APU_JTAG */
+
 static int mtk_apu_rproc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -242,6 +387,10 @@ static int mtk_apu_rproc_probe(struct platform_device *pdev)
 		goto free_mem;
 	}
 
+	ret = apu_jtag_probe(apu_rproc);
+	if (ret)
+		dev_warn(dev, "Failed to configure jtag\n");
+
 	return 0;
 
 free_mem:
@@ -259,7 +408,7 @@ static int mtk_apu_rproc_remove(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 
 	disable_irq(apu_rproc->irq);
-
+	apu_disable_jtag(apu_rproc);
 	rproc_del(rproc);
 	of_reserved_mem_device_release(dev);
 	rproc_free(rproc);
-- 
2.26.2

Re: [PATCH v2 3/4] remoteproc: mtk_vpu_rproc: Add support of JTAG

[Mathieu Poirier]

On Thu, Sep 10, 2020 at 03:01:47PM +0200, Alexandre Bailon wrote:
> The DSP could be debugged using JTAG.
> The support of JTAG could enabled at build time and it could be enabled
> using debugfs.
> 
> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
> ---
>  drivers/remoteproc/Kconfig   |   9 +++
>  drivers/remoteproc/mtk_apu.c | 151 ++++++++++++++++++++++++++++++++++-
>  2 files changed, 159 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index 4ebea57bf4c8..310462346bd8 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -61,6 +61,15 @@ config MTK_APU
>  
>  	  It's safe to say N here.
>  
> +config MTK_APU_JTAG
> +	bool "Enable support of JTAG"

I think it is better to simply go with "Enable JTAG support"

> +	depends on MTK_APU
> +	help
> +	  Say y to enable support of JTAG.

Same here.

> +	  By default, JTAG will remain disabled until it is enabled using
> +	  debugfs: remoteproc/remoteproc0/jtag. Write 1 to enable it and

s/remoteproc0/remoteprocX

> +	  0 to disable it.
> +
>  config OMAP_REMOTEPROC
>  	tristate "OMAP remoteproc support"
>  	depends on ARCH_OMAP4 || SOC_OMAP5 || SOC_DRA7XX
> diff --git a/drivers/remoteproc/mtk_apu.c b/drivers/remoteproc/mtk_apu.c
> index 6d2f577cfde5..07157fdc24ba 100644
> --- a/drivers/remoteproc/mtk_apu.c
> +++ b/drivers/remoteproc/mtk_apu.c
> @@ -5,6 +5,7 @@
>  
>  #include <linux/bitops.h>
>  #include <linux/clk.h>
> +#include <linux/debugfs.h>
>  #include <linux/delay.h>
>  #include <linux/highmem.h>
>  #include <linux/interrupt.h>
> @@ -14,6 +15,7 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/of_reserved_mem.h>
> +#include <linux/pinctrl/consumer.h>
>  #include <linux/platform_device.h>
>  #include <linux/remoteproc.h>
>  
> @@ -48,6 +50,11 @@
>  #define CORE_DEFAULT1				(0x00000140)
>  #define CORE_DEFAULT0_ARUSER_IDMA_USE_IOMMU	(0x10 << 0)
>  #define CORE_DEFAULT0_AWUSER_IDMA_USE_IOMMU	(0x10 << 5)
> +#define CORE_DEFAULT2				(0x00000144)
> +#define CORE_DEFAULT2_DBG_EN			BIT(3)
> +#define CORE_DEFAULT2_NIDEN			BIT(2)
> +#define CORE_DEFAULT2_SPNIDEN			BIT(1)
> +#define CORE_DEFAULT2_SPIDEN			BIT(0)
>  #define CORE_XTENSA_ALTRESETVEC			(0x000001F8)
>  
>  struct mtk_apu_rproc {
> @@ -57,6 +64,13 @@ struct mtk_apu_rproc {
>  	void __iomem *base;
>  	int irq;
>  	struct clk_bulk_data clks[3];
> +
> +#ifdef CONFIG_MTK_APU_JTAG
> +	struct pinctrl *pinctrl;
> +	struct pinctrl_state *pinctrl_jtag;
> +	bool jtag_enabled;
> +	struct mutex jtag_mutex;

Move this up to keep all the struct together.

> +#endif
>  };
>  
>  static int mtk_apu_rproc_prepare(struct rproc *rproc)
> @@ -166,6 +180,137 @@ static irqreturn_t handle_event(int irq, void *data)
>  	return IRQ_HANDLED;
>  }
>  
> +#ifdef CONFIG_MTK_APU_JTAG
> +
> +static int apu_enable_jtag(struct mtk_apu_rproc *apu_rproc)
> +{
> +	int ret = 0;
> +
> +	mutex_lock(&apu_rproc->jtag_mutex);
> +	if (apu_rproc->jtag_enabled) {
> +		ret = -EINVAL;

The JTAG is already enabled, I think enabling it again isn't a big deal and
should simply return 0 rather than an error.

> +		goto err_mutex_unlock;
> +	}
> +
> +	writel(CORE_DEFAULT2_SPNIDEN | CORE_DEFAULT2_SPIDEN |
> +		CORE_DEFAULT2_NIDEN | CORE_DEFAULT2_DBG_EN,
> +		apu_rproc->base + CORE_DEFAULT2);
> +
> +	apu_rproc->jtag_enabled = 1;

s/1/true

> +
> +err_mutex_unlock:
> +	mutex_unlock(&apu_rproc->jtag_mutex);
> +
> +	return ret;
> +}
> +
> +static int apu_disable_jtag(struct mtk_apu_rproc *apu_rproc)
> +{
> +	int ret = 0;
> +
> +	mutex_lock(&apu_rproc->jtag_mutex);
> +	if (!apu_rproc->jtag_enabled) {
> +		ret = -EINVAL;

Same as above

> +		goto err_mutex_unlock;
> +	}
> +
> +	writel(0, apu_rproc->base + CORE_DEFAULT2);
> +
> +	apu_rproc->jtag_enabled = 0;

s/0/false

Thanks for the patience,
Mathieu

> +
> +err_mutex_unlock:
> +	mutex_unlock(&apu_rproc->jtag_mutex);
> +
> +	return ret;
> +}
> +
> +static ssize_t rproc_jtag_read(struct file *filp, char __user *userbuf,
> +			       size_t count, loff_t *ppos)
> +{
> +	struct rproc *rproc = filp->private_data;
> +	struct mtk_apu_rproc *apu_rproc = (struct mtk_apu_rproc *)rproc->priv;
> +	char *buf = apu_rproc->jtag_enabled ? "enabled\n" : "disabled\n";
> +
> +	return simple_read_from_buffer(userbuf, count, ppos, buf, strlen(buf));
> +}
> +
> +static ssize_t rproc_jtag_write(struct file *filp, const char __user *user_buf,
> +				size_t count, loff_t *ppos)
> +{
> +	struct rproc *rproc = filp->private_data;
> +	struct mtk_apu_rproc *apu_rproc = (struct mtk_apu_rproc *)rproc->priv;
> +	char buf[10];
> +	int ret;
> +
> +	if (count < 1 || count > sizeof(buf))
> +		return -EINVAL;
> +
> +	ret = copy_from_user(buf, user_buf, count);
> +	if (ret)
> +		return -EFAULT;
> +
> +	/* remove end of line */
> +	if (buf[count - 1] == '\n')
> +		buf[count - 1] = '\0';
> +
> +	if (!strncmp(buf, "enabled", count))
> +		ret = apu_enable_jtag(apu_rproc);
> +	else if (!strncmp(buf, "disabled", count))
> +		ret = apu_disable_jtag(apu_rproc);
> +	else
> +		return -EINVAL;
> +
> +	return ret ? ret : count;
> +}
> +
> +static const struct file_operations rproc_jtag_ops = {
> +	.read = rproc_jtag_read,
> +	.write = rproc_jtag_write,
> +	.open = simple_open,
> +};
> +
> +static int apu_jtag_probe(struct mtk_apu_rproc *apu_rproc)
> +{
> +	int ret;
> +
> +	if (!apu_rproc->rproc->dbg_dir)
> +		return -ENODEV;
> +
> +	apu_rproc->pinctrl = devm_pinctrl_get(apu_rproc->dev);
> +	if (IS_ERR(apu_rproc->pinctrl)) {
> +		dev_warn(apu_rproc->dev, "Failed to find JTAG pinctrl\n");
> +		return PTR_ERR(apu_rproc->pinctrl);
> +	}
> +
> +	apu_rproc->pinctrl_jtag = pinctrl_lookup_state(apu_rproc->pinctrl,
> +						       "jtag");
> +	if (IS_ERR(apu_rproc->pinctrl_jtag))
> +		return PTR_ERR(apu_rproc->pinctrl_jtag);
> +
> +	ret = pinctrl_select_state(apu_rproc->pinctrl,
> +				   apu_rproc->pinctrl_jtag);
> +	if (ret < 0)
> +		return ret;
> +
> +	mutex_init(&apu_rproc->jtag_mutex);
> +
> +	debugfs_create_file("jtag", 0600, apu_rproc->rproc->dbg_dir,
> +			    apu_rproc->rproc, &rproc_jtag_ops);
> +
> +	return 0;
> +}
> +#else
> +static int apu_jtag_probe(struct mtk_apu_rproc *apu_rproc)
> +{
> +	return 0;
> +}
> +
> +static int apu_disable_jtag(struct mtk_apu_rproc *apu_rproc)
> +{
> +	return 0;
> +}
> +#endif /* CONFIG_MTK_APU_JTAG */
> +
>  static int mtk_apu_rproc_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -242,6 +387,10 @@ static int mtk_apu_rproc_probe(struct platform_device *pdev)
>  		goto free_mem;
>  	}
>  
> +	ret = apu_jtag_probe(apu_rproc);
> +	if (ret)
> +		dev_warn(dev, "Failed to configure jtag\n");
> +
>  	return 0;
>  
>  free_mem:
> @@ -259,7 +408,7 @@ static int mtk_apu_rproc_remove(struct platform_device *pdev)
>  	struct device *dev = &pdev->dev;
>  
>  	disable_irq(apu_rproc->irq);
> -
> +	apu_disable_jtag(apu_rproc);
>  	rproc_del(rproc);
>  	of_reserved_mem_device_release(dev);
>  	rproc_free(rproc);
> -- 
> 2.26.2
> 

[PATCH v2 4/4] ARM64: mt8183: Add support of APU to mt8183

[Alexandre Bailon]

This adds the support of APU to mt8183.

Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
---
 arch/arm64/boot/dts/mediatek/mt8183.dtsi | 39 ++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index e215f1eb3eb2..28f75452961c 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -967,12 +967,51 @@ ipu_adl: syscon@19010000 {
 			#clock-cells = <1>;
 		};
 
+		apu0: apu@0x19100000 {
+			compatible = "mediatek,mt8183-apu";
+			reg = <0 0x19180000 0 0x14000>;
+			interrupts = <GIC_SPI 292 IRQ_TYPE_LEVEL_LOW>;
+
+			iommus = <&iommu M4U_PORT_IMG_IPUO>,
+				 <&iommu M4U_PORT_IMG_IPU3O>,
+				 <&iommu M4U_PORT_IMG_IPUI>;
+
+			clocks = <&ipu_core0 CLK_IPU_CORE0_AXI>,
+				 <&ipu_core0 CLK_IPU_CORE0_IPU>,
+				 <&ipu_core0 CLK_IPU_CORE0_JTAG>;
+
+			clock-names = "axi", "ipu", "jtag";
+
+			power-domains = <&scpsys MT8183_POWER_DOMAIN_VPU_CORE0>;
+			status = "disabled";
+		};
+
 		ipu_core0: syscon@19180000 {
 			compatible = "mediatek,mt8183-ipu_core0", "syscon";
 			reg = <0 0x19180000 0 0x1000>;
 			#clock-cells = <1>;
 		};
 
+		apu1: apu@19200000 {
+			compatible = "mediatek,mt8183-apu";
+			reg = <0 0x19280000 0 0x14000>;
+			interrupts = <GIC_SPI 293 IRQ_TYPE_LEVEL_LOW>;
+
+			iommus = <&iommu M4U_PORT_CAM_IPUO>,
+				 <&iommu M4U_PORT_CAM_IPU2O>,
+				 <&iommu M4U_PORT_CAM_IPU3O>,
+				 <&iommu M4U_PORT_CAM_IPUI>,
+				 <&iommu M4U_PORT_CAM_IPU2I>;
+
+			clocks = <&ipu_core0 CLK_IPU_CORE1_AXI>,
+				 <&ipu_core0 CLK_IPU_CORE1_IPU>,
+				 <&ipu_core0 CLK_IPU_CORE1_JTAG>;
+
+			clock-names = "axi", "ipu", "jtag";
+
+			power-domains = <&scpsys MT8183_POWER_DOMAIN_VPU_CORE1>;
+		};
+
 		ipu_core1: syscon@19280000 {
 			compatible = "mediatek,mt8183-ipu_core1", "syscon";
 			reg = <0 0x19280000 0 0x1000>;
-- 
2.26.2

Re: [PATCH v2 4/4] ARM64: mt8183: Add support of APU to mt8183

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 1574 bytes --]

Hi Alexandre,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on robh/for-next]
[also build test ERROR on linus/master v5.9-rc4 next-20200911]
[cannot apply to remoteproc/for-next rpmsg/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Alexandre-Bailon/Add-support-of-mt8183-APU/20200911-055649
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm64-randconfig-r006-20200911 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 0448d11a06b451a63a8f60408fec613ad24801ba)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm64 cross compiling tool for clang build
        # apt-get install binutils-aarch64-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> Error: arch/arm64/boot/dts/mediatek/mt8183.dtsi:792.21-22 syntax error
>> FATAL ERROR: Unable to parse input tree

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36422 bytes --]

Re: [PATCH v2 0/4] Add support of mt8183 APU

[Mathieu Poirier]

Hi Alexander,

Things have been quite busy over the last 3 weeks, preventing me from
giving your work the attention it deserves.  It is on my radar and will get to
it in the next two weeks.

Thanks,
Mathieu
 
On Thu, Sep 10, 2020 at 03:01:44PM +0200, Alexandre Bailon wrote:
> Some Mediatek's SoC have an Accelerated Processing Unit.
> This adds support of the one available in the mt8183
> (aswell some derivative SoC).
> 
> This series depends on two other series:
> - Mediatek MT8183 scpsys support  
> - arm64: dts: Add m4u and smi-larbs nodes for mt8183
> 
> Changes in v2:
> - Drop the workarounds needed to load bad firmwares
> - There are many name for the APU (most common one is VPU).
>   Rename many functions and dts nodes to be more consistent.
> - Use the bulk clock API, and enable / disable clock at a better place
> - add few comments explaining how to start the APU
> - update the way to use pinctl for JTAG
> - fix some minors issues
> - fix device tree bindings
> 
> Alexandre Bailon (4):
>   dt bindings: remoteproc: Add bindings for MT8183 APU
>   remoteproc: Add a remoteproc driver for the MT8183's APU
>   remoteproc: mtk_vpu_rproc: Add support of JTAG
>   ARM64: mt8183: Add support of APU to mt8183
> 
>  .../bindings/remoteproc/mtk,apu.yaml          | 107 +++++
>  arch/arm64/boot/dts/mediatek/mt8183.dtsi      |  39 ++
>  drivers/remoteproc/Kconfig                    |  19 +
>  drivers/remoteproc/Makefile                   |   1 +
>  drivers/remoteproc/mtk_apu.c                  | 437 ++++++++++++++++++
>  5 files changed, 603 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/remoteproc/mtk,apu.yaml
>  create mode 100644 drivers/remoteproc/mtk_apu.c
> 
> -- 
> 2.26.2
>