Linux-remoteproc Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v7 1/5] dt-bindings: Document JZ47xx VPU auxiliary processor
@ 2020-05-15 10:43 Paul Cercueil
  2020-05-15 10:43 ` [PATCH v7 2/5] remoteproc: Add device-managed variants of rproc_alloc/rproc_add Paul Cercueil
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Paul Cercueil @ 2020-05-15 10:43 UTC (permalink / raw)
  To: Bjorn Andersson, Ohad Ben-Cohen, Arnaud Pouliquen
  Cc: od, linux-remoteproc, devicetree, linux-kernel, Paul Cercueil,
	Rob Herring

Inside the Video Processing Unit (VPU) of the recent JZ47xx SoCs from
Ingenic is a second Xburst MIPS CPU very similar to the main core.
This document describes the devicetree bindings for this auxiliary
processor.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Reviewed-by: Rob Herring <robh@kernel.org>
---

Notes:
    v2: Update TCSM0 address in example
    v3: Change node name to 'video-decoder'
    v4: Convert to YAML. I didn't add Rob's Ack on v3 because of that (sorry Rob)
    v5: - Fix 'reg' not in <addr, len> pairs
    	- Add missing include to devicetree example
    v6-v7: No change

 .../bindings/remoteproc/ingenic,vpu.yaml      | 77 +++++++++++++++++++
 1 file changed, 77 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/ingenic,vpu.yaml

diff --git a/Documentation/devicetree/bindings/remoteproc/ingenic,vpu.yaml b/Documentation/devicetree/bindings/remoteproc/ingenic,vpu.yaml
new file mode 100644
index 000000000000..c019f9fbe916
--- /dev/null
+++ b/Documentation/devicetree/bindings/remoteproc/ingenic,vpu.yaml
@@ -0,0 +1,77 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/remoteproc/ingenic,vpu.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Ingenic Video Processing Unit bindings
+
+description:
+  Inside the Video Processing Unit (VPU) of the recent JZ47xx SoCs from
+  Ingenic is a second Xburst MIPS CPU very similar to the main core.
+  This document describes the devicetree bindings for this auxiliary
+  processor.
+
+maintainers:
+  - Paul Cercueil <paul@crapouillou.net>
+
+properties:
+  compatible:
+    const: ingenic,jz4770-vpu-rproc
+
+  reg:
+    items:
+      - description: aux registers
+      - description: tcsm0 registers
+      - description: tcsm1 registers
+      - description: sram registers
+
+  reg-names:
+    items:
+      - const: aux
+      - const: tcsm0
+      - const: tcsm1
+      - const: sram
+
+  clocks:
+    items:
+      - description: aux clock
+      - description: vpu clock
+
+  clock-names:
+    items:
+      - const: aux
+      - const: vpu
+
+  interrupts:
+    description: VPU hardware interrupt
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - clocks
+  - clock-names
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/jz4770-cgu.h>
+
+    vpu: video-decoder@132a0000 {
+      compatible = "ingenic,jz4770-vpu-rproc";
+
+      reg = <0x132a0000 0x20>, /* AUX */
+            <0x132b0000 0x4000>, /* TCSM0 */
+            <0x132c0000 0xc000>, /* TCSM1 */
+            <0x132f0000 0x7000>; /* SRAM */
+      reg-names = "aux", "tcsm0", "tcsm1", "sram";
+
+      clocks = <&cgu JZ4770_CLK_AUX>, <&cgu JZ4770_CLK_VPU>;
+      clock-names = "aux", "vpu";
+
+      interrupt-parent = <&cpuintc>;
+      interrupts = <3>;
+    };
-- 
2.26.2


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

* [PATCH v7 2/5] remoteproc: Add device-managed variants of rproc_alloc/rproc_add
  2020-05-15 10:43 [PATCH v7 1/5] dt-bindings: Document JZ47xx VPU auxiliary processor Paul Cercueil
@ 2020-05-15 10:43 ` Paul Cercueil
  2020-05-15 10:43 ` [PATCH v7 3/5] remoteproc: Add support for runtime PM Paul Cercueil
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Paul Cercueil @ 2020-05-15 10:43 UTC (permalink / raw)
  To: Bjorn Andersson, Ohad Ben-Cohen, Arnaud Pouliquen
  Cc: od, linux-remoteproc, devicetree, linux-kernel, Paul Cercueil

Add API functions devm_rproc_alloc() and devm_rproc_add(), which behave
like rproc_alloc() and rproc_add() respectively, but register their
respective cleanup function to be called on driver detach.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Notes:
    v3: New patch
    v4: No change
    v5: - Fix return value documentation
    	- Fix typo in documentation
    v6-v7: No change

 drivers/remoteproc/remoteproc_core.c | 67 ++++++++++++++++++++++++++++
 include/linux/remoteproc.h           |  5 +++
 2 files changed, 72 insertions(+)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index e12a54e67588..a7f96bc98406 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -1949,6 +1949,33 @@ int rproc_add(struct rproc *rproc)
 }
 EXPORT_SYMBOL(rproc_add);
 
+static void devm_rproc_remove(void *rproc)
+{
+	rproc_del(rproc);
+}
+
+/**
+ * devm_rproc_add() - resource managed rproc_add()
+ * @dev: the underlying device
+ * @rproc: the remote processor handle to register
+ *
+ * This function performs like rproc_add() but the registered rproc device will
+ * automatically be removed on driver detach.
+ *
+ * Returns: 0 on success, negative errno on failure
+ */
+int devm_rproc_add(struct device *dev, struct rproc *rproc)
+{
+	int err;
+
+	err = rproc_add(rproc);
+	if (err)
+		return err;
+
+	return devm_add_action_or_reset(dev, devm_rproc_remove, rproc);
+}
+EXPORT_SYMBOL(devm_rproc_add);
+
 /**
  * rproc_type_release() - release a remote processor instance
  * @dev: the rproc's device
@@ -2171,6 +2198,46 @@ int rproc_del(struct rproc *rproc)
 }
 EXPORT_SYMBOL(rproc_del);
 
+static void devm_rproc_free(struct device *dev, void *res)
+{
+	rproc_free(*(struct rproc **)res);
+}
+
+/**
+ * devm_rproc_alloc() - resource managed rproc_alloc()
+ * @dev: the underlying device
+ * @name: name of this remote processor
+ * @ops: platform-specific handlers (mainly start/stop)
+ * @firmware: name of firmware file to load, can be NULL
+ * @len: length of private data needed by the rproc driver (in bytes)
+ *
+ * This function performs like rproc_alloc() but the acquired rproc device will
+ * automatically be released on driver detach.
+ *
+ * Returns: new rproc instance, or NULL on failure
+ */
+struct rproc *devm_rproc_alloc(struct device *dev, const char *name,
+			       const struct rproc_ops *ops,
+			       const char *firmware, int len)
+{
+	struct rproc **ptr, *rproc;
+
+	ptr = devres_alloc(devm_rproc_free, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return ERR_PTR(-ENOMEM);
+
+	rproc = rproc_alloc(dev, name, ops, firmware, len);
+	if (rproc) {
+		*ptr = rproc;
+		devres_add(dev, ptr);
+	} else {
+		devres_free(ptr);
+	}
+
+	return rproc;
+}
+EXPORT_SYMBOL(devm_rproc_alloc);
+
 /**
  * rproc_add_subdev() - add a subdevice to a remoteproc
  * @rproc: rproc handle to add the subdevice to
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 9c07d7958c53..8c9c0dda03c3 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -599,6 +599,11 @@ int rproc_add(struct rproc *rproc);
 int rproc_del(struct rproc *rproc);
 void rproc_free(struct rproc *rproc);
 
+struct rproc *devm_rproc_alloc(struct device *dev, const char *name,
+			       const struct rproc_ops *ops,
+			       const char *firmware, int len);
+int devm_rproc_add(struct device *dev, struct rproc *rproc);
+
 void rproc_add_carveout(struct rproc *rproc, struct rproc_mem_entry *mem);
 
 struct rproc_mem_entry *
-- 
2.26.2


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

* [PATCH v7 3/5] remoteproc: Add support for runtime PM
  2020-05-15 10:43 [PATCH v7 1/5] dt-bindings: Document JZ47xx VPU auxiliary processor Paul Cercueil
  2020-05-15 10:43 ` [PATCH v7 2/5] remoteproc: Add device-managed variants of rproc_alloc/rproc_add Paul Cercueil
@ 2020-05-15 10:43 ` Paul Cercueil
  2020-05-22 16:47   ` Suman Anna
  2020-05-15 10:43 ` [PATCH v7 4/5] remoteproc: ingenic: Added remoteproc driver Paul Cercueil
  2020-05-15 10:43 ` [PATCH v7 5/5] MAINTAINERS: Add myself as reviewer for Ingenic rproc driver Paul Cercueil
  3 siblings, 1 reply; 8+ messages in thread
From: Paul Cercueil @ 2020-05-15 10:43 UTC (permalink / raw)
  To: Bjorn Andersson, Ohad Ben-Cohen, Arnaud Pouliquen
  Cc: od, linux-remoteproc, devicetree, linux-kernel, Paul Cercueil

Call pm_runtime_get_sync() before the firmware is loaded, and
pm_runtime_put() after the remote processor has been stopped.

Even though the remoteproc device has no PM callbacks, this allows the
parent device's PM callbacks to be properly called.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---

Notes:
    v2-v4: No change
    v5: Move calls to prepare/unprepare to rproc_fw_boot/rproc_shutdown
    v6: Instead of prepare/unprepare callbacks, use PM runtime callbacks
    v7: Check return value of pm_runtime_get_sync()

 drivers/remoteproc/remoteproc_core.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index a7f96bc98406..e33d1ef27981 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -29,6 +29,7 @@
 #include <linux/devcoredump.h>
 #include <linux/rculist.h>
 #include <linux/remoteproc.h>
+#include <linux/pm_runtime.h>
 #include <linux/iommu.h>
 #include <linux/idr.h>
 #include <linux/elf.h>
@@ -1382,6 +1383,12 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
 	if (ret)
 		return ret;
 
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0) {
+		dev_err(dev, "pm_runtime_get_sync failed: %d\n", ret);
+		return ret;
+	}
+
 	dev_info(dev, "Booting fw image %s, size %zd\n", name, fw->size);
 
 	/*
@@ -1391,7 +1398,7 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
 	ret = rproc_enable_iommu(rproc);
 	if (ret) {
 		dev_err(dev, "can't enable iommu: %d\n", ret);
-		return ret;
+		goto put_pm_runtime;
 	}
 
 	rproc->bootaddr = rproc_get_boot_addr(rproc, fw);
@@ -1435,6 +1442,8 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
 	rproc->table_ptr = NULL;
 disable_iommu:
 	rproc_disable_iommu(rproc);
+put_pm_runtime:
+	pm_runtime_put(dev);
 	return ret;
 }
 
@@ -1840,6 +1849,8 @@ void rproc_shutdown(struct rproc *rproc)
 
 	rproc_disable_iommu(rproc);
 
+	pm_runtime_put(dev);
+
 	/* Free the copy of the resource table */
 	kfree(rproc->cached_table);
 	rproc->cached_table = NULL;
@@ -2118,6 +2129,9 @@ struct rproc *rproc_alloc(struct device *dev, const char *name,
 
 	rproc->state = RPROC_OFFLINE;
 
+	pm_runtime_no_callbacks(&rproc->dev);
+	pm_runtime_enable(&rproc->dev);
+
 	return rproc;
 }
 EXPORT_SYMBOL(rproc_alloc);
@@ -2133,6 +2147,7 @@ EXPORT_SYMBOL(rproc_alloc);
  */
 void rproc_free(struct rproc *rproc)
 {
+	pm_runtime_disable(&rproc->dev);
 	put_device(&rproc->dev);
 }
 EXPORT_SYMBOL(rproc_free);
-- 
2.26.2


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

* [PATCH v7 4/5] remoteproc: ingenic: Added remoteproc driver
  2020-05-15 10:43 [PATCH v7 1/5] dt-bindings: Document JZ47xx VPU auxiliary processor Paul Cercueil
  2020-05-15 10:43 ` [PATCH v7 2/5] remoteproc: Add device-managed variants of rproc_alloc/rproc_add Paul Cercueil
  2020-05-15 10:43 ` [PATCH v7 3/5] remoteproc: Add support for runtime PM Paul Cercueil
@ 2020-05-15 10:43 ` Paul Cercueil
  2020-05-18 23:57   ` Bjorn Andersson
  2020-05-15 10:43 ` [PATCH v7 5/5] MAINTAINERS: Add myself as reviewer for Ingenic rproc driver Paul Cercueil
  3 siblings, 1 reply; 8+ messages in thread
From: Paul Cercueil @ 2020-05-15 10:43 UTC (permalink / raw)
  To: Bjorn Andersson, Ohad Ben-Cohen, Arnaud Pouliquen
  Cc: od, linux-remoteproc, devicetree, linux-kernel, Paul Cercueil,
	Mathieu Poirier

This driver is used to boot, communicate with and load firmwares to the
MIPS co-processor found in the VPU hardware of the JZ47xx SoCs from
Ingenic.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---

Notes:
    v2: Remove exception for always-mapped memories
    v3: - Use clk_bulk API
    	- Move device-managed code to its own patch [3/4]
    	- Move devicetree table right above ingenic_rproc_driver
    	- Removed #ifdef CONFIG_OF around devicetree table
    	- Removed .owner = THIS_MODULE in ingenic_rproc_driver
    	- Removed useless platform_set_drvdata()
    v4: - Add fix reported by Julia
    	- Change Kconfig symbol to INGENIC_VPU_RPROC
    	- Add documentation to struct vpu
    	- disable_irq_nosync() -> disable_irq()
    v5: No change
    v6: Instead of prepare/unprepare callbacks, use PM runtime callbacks
    v7: - Remove use of of_match_ptr()
    	- Move Kconfig symbol so that it's in alphabetical order
    	- Add missing doc for private structure field aux_base
    	- Don't check for (len <= 0) in da_to_va()
    	- Add missing \n in dev_info/dev_err messages

 drivers/remoteproc/Kconfig         |   9 +
 drivers/remoteproc/Makefile        |   1 +
 drivers/remoteproc/ingenic_rproc.c | 280 +++++++++++++++++++++++++++++
 3 files changed, 290 insertions(+)
 create mode 100644 drivers/remoteproc/ingenic_rproc.c

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index fbaed079b299..c4d1731295eb 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -23,6 +23,15 @@ config IMX_REMOTEPROC
 
 	  It's safe to say N here.
 
+config INGENIC_VPU_RPROC
+	tristate "Ingenic JZ47xx VPU remoteproc support"
+	depends on MIPS || COMPILE_TEST
+	help
+	  Say y or m here to support the VPU in the JZ47xx SoCs from Ingenic.
+
+	  This can be either built-in or a loadable module.
+	  If unsure say N.
+
 config MTK_SCP
 	tristate "Mediatek SCP support"
 	depends on ARCH_MEDIATEK
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index 0effd3825035..e8b886e511f0 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -10,6 +10,7 @@ remoteproc-y				+= remoteproc_sysfs.o
 remoteproc-y				+= remoteproc_virtio.o
 remoteproc-y				+= remoteproc_elf_loader.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_OMAP_REMOTEPROC)		+= omap_remoteproc.o
 obj-$(CONFIG_WKUP_M3_RPROC)		+= wkup_m3_rproc.o
diff --git a/drivers/remoteproc/ingenic_rproc.c b/drivers/remoteproc/ingenic_rproc.c
new file mode 100644
index 000000000000..189020d77b25
--- /dev/null
+++ b/drivers/remoteproc/ingenic_rproc.c
@@ -0,0 +1,280 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Ingenic JZ47xx remoteproc driver
+ * Copyright 2019, Paul Cercueil <paul@crapouillou.net>
+ */
+
+#include <linux/bitops.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/remoteproc.h>
+
+#include "remoteproc_internal.h"
+
+#define REG_AUX_CTRL		0x0
+#define REG_AUX_MSG_ACK		0x10
+#define REG_AUX_MSG		0x14
+#define REG_CORE_MSG_ACK	0x18
+#define REG_CORE_MSG		0x1C
+
+#define AUX_CTRL_SLEEP		BIT(31)
+#define AUX_CTRL_MSG_IRQ_EN	BIT(3)
+#define AUX_CTRL_NMI_RESETS	BIT(2)
+#define AUX_CTRL_NMI		BIT(1)
+#define AUX_CTRL_SW_RESET	BIT(0)
+
+struct vpu_mem_map {
+	const char *name;
+	unsigned int da;
+};
+
+struct vpu_mem_info {
+	const struct vpu_mem_map *map;
+	unsigned long len;
+	void __iomem *base;
+};
+
+static const struct vpu_mem_map vpu_mem_map[] = {
+	{ "tcsm0", 0x132b0000 },
+	{ "tcsm1", 0xf4000000 },
+	{ "sram",  0x132f0000 },
+};
+
+/**
+ * struct vpu - Ingenic VPU remoteproc private structure
+ * @irq: interrupt number
+ * @clks: pointers to the VPU and AUX clocks
+ * @aux_base: raw pointer to the AUX interface registers
+ * @mem_info: array of struct vpu_mem_info, which contain the mapping info of
+ *            each of the external memories
+ * @dev: private pointer to the device
+ */
+struct vpu {
+	int irq;
+	struct clk_bulk_data clks[2];
+	void __iomem *aux_base;
+	struct vpu_mem_info mem_info[ARRAY_SIZE(vpu_mem_map)];
+	struct device *dev;
+};
+
+static int ingenic_rproc_start(struct rproc *rproc)
+{
+	struct vpu *vpu = rproc->priv;
+	u32 ctrl;
+
+	enable_irq(vpu->irq);
+
+	/* Reset the AUX and enable message IRQ */
+	ctrl = AUX_CTRL_NMI_RESETS | AUX_CTRL_NMI | AUX_CTRL_MSG_IRQ_EN;
+	writel(ctrl, vpu->aux_base + REG_AUX_CTRL);
+
+	return 0;
+}
+
+static int ingenic_rproc_stop(struct rproc *rproc)
+{
+	struct vpu *vpu = rproc->priv;
+
+	disable_irq(vpu->irq);
+
+	/* Keep AUX in reset mode */
+	writel(AUX_CTRL_SW_RESET, vpu->aux_base + REG_AUX_CTRL);
+
+	return 0;
+}
+
+static void ingenic_rproc_kick(struct rproc *rproc, int vqid)
+{
+	struct vpu *vpu = rproc->priv;
+
+	writel(vqid, vpu->aux_base + REG_CORE_MSG);
+}
+
+static void *ingenic_rproc_da_to_va(struct rproc *rproc, u64 da, size_t len)
+{
+	struct vpu *vpu = rproc->priv;
+	void __iomem *va = NULL;
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(vpu_mem_map); i++) {
+		const struct vpu_mem_info *info = &vpu->mem_info[i];
+		const struct vpu_mem_map *map = info->map;
+
+		if (da >= map->da && (da + len) < (map->da + info->len)) {
+			va = info->base + (da - map->da);
+			break;
+		}
+	}
+
+	return (__force void *)va;
+}
+
+static struct rproc_ops ingenic_rproc_ops = {
+	.start = ingenic_rproc_start,
+	.stop = ingenic_rproc_stop,
+	.kick = ingenic_rproc_kick,
+	.da_to_va = ingenic_rproc_da_to_va,
+};
+
+static irqreturn_t vpu_interrupt(int irq, void *data)
+{
+	struct rproc *rproc = data;
+	struct vpu *vpu = rproc->priv;
+	u32 vring;
+
+	vring = readl(vpu->aux_base + REG_AUX_MSG);
+
+	/* Ack the interrupt */
+	writel(0, vpu->aux_base + REG_AUX_MSG_ACK);
+
+	return rproc_vq_interrupt(rproc, vring);
+}
+
+static void ingenic_rproc_disable_clks(void *data)
+{
+	struct vpu *vpu = data;
+
+	pm_runtime_resume(vpu->dev);
+	pm_runtime_disable(vpu->dev);
+
+	clk_bulk_disable_unprepare(ARRAY_SIZE(vpu->clks), vpu->clks);
+}
+
+static int ingenic_rproc_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct resource *mem;
+	struct rproc *rproc;
+	struct vpu *vpu;
+	unsigned int i;
+	int ret;
+
+	rproc = devm_rproc_alloc(dev, "ingenic-vpu",
+				 &ingenic_rproc_ops, NULL, sizeof(*vpu));
+	if (!rproc)
+		return -ENOMEM;
+
+	vpu = rproc->priv;
+	vpu->dev = &pdev->dev;
+	platform_set_drvdata(pdev, vpu);
+
+	mem = platform_get_resource_byname(pdev, IORESOURCE_MEM, "aux");
+	vpu->aux_base = devm_ioremap_resource(dev, mem);
+	if (IS_ERR(vpu->aux_base)) {
+		dev_err(dev, "Failed to ioremap\n");
+		return PTR_ERR(vpu->aux_base);
+	}
+
+	for (i = 0; i < ARRAY_SIZE(vpu_mem_map); i++) {
+		mem = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+						   vpu_mem_map[i].name);
+
+		vpu->mem_info[i].base = devm_ioremap_resource(dev, mem);
+		if (IS_ERR(vpu->mem_info[i].base)) {
+			ret = PTR_ERR(vpu->mem_info[i].base);
+			dev_err(dev, "Failed to ioremap\n");
+			return ret;
+		}
+
+		vpu->mem_info[i].len = resource_size(mem);
+		vpu->mem_info[i].map = &vpu_mem_map[i];
+	}
+
+	vpu->clks[0].id = "vpu";
+	vpu->clks[1].id = "aux";
+
+	ret = devm_clk_bulk_get(dev, ARRAY_SIZE(vpu->clks), vpu->clks);
+	if (ret) {
+		dev_err(dev, "Failed to get clocks\n");
+		return ret;
+	}
+
+	vpu->irq = platform_get_irq(pdev, 0);
+	if (vpu->irq < 0)
+		return vpu->irq;
+
+	ret = devm_request_irq(dev, vpu->irq, vpu_interrupt, 0, "VPU", rproc);
+	if (ret < 0) {
+		dev_err(dev, "Failed to request IRQ\n");
+		return ret;
+	}
+
+	disable_irq(vpu->irq);
+
+	/* The clocks must be enabled for the firmware to be loaded in TCSM */
+	ret = clk_bulk_prepare_enable(ARRAY_SIZE(vpu->clks), vpu->clks);
+	if (ret) {
+		dev_err(dev, "Unable to start clocks\n");
+		return ret;
+	}
+
+	pm_runtime_irq_safe(dev);
+	pm_runtime_set_active(dev);
+	pm_runtime_enable(dev);
+	pm_runtime_get_sync(dev);
+	pm_runtime_use_autosuspend(dev);
+
+	ret = devm_add_action_or_reset(dev, ingenic_rproc_disable_clks, vpu);
+	if (ret) {
+		dev_err(dev, "Unable to register action\n");
+		goto out_pm_put;
+	}
+
+	ret = devm_rproc_add(dev, rproc);
+	if (ret) {
+		dev_err(dev, "Failed to register remote processor\n");
+		goto out_pm_put;
+	}
+
+out_pm_put:
+	pm_runtime_put_autosuspend(dev);
+
+	return ret;
+}
+
+static const struct of_device_id ingenic_rproc_of_matches[] = {
+	{ .compatible = "ingenic,jz4770-vpu-rproc", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, ingenic_rproc_of_matches);
+
+static int __maybe_unused ingenic_rproc_suspend(struct device *dev)
+{
+	struct vpu *vpu = dev_get_drvdata(dev);
+
+	clk_bulk_disable(ARRAY_SIZE(vpu->clks), vpu->clks);
+
+	return 0;
+}
+
+static int __maybe_unused ingenic_rproc_resume(struct device *dev)
+{
+	struct vpu *vpu = dev_get_drvdata(dev);
+
+	return clk_bulk_enable(ARRAY_SIZE(vpu->clks), vpu->clks);
+}
+
+static const struct dev_pm_ops __maybe_unused ingenic_rproc_pm = {
+	SET_RUNTIME_PM_OPS(ingenic_rproc_suspend, ingenic_rproc_resume, NULL)
+};
+
+static struct platform_driver ingenic_rproc_driver = {
+	.probe = ingenic_rproc_probe,
+	.driver = {
+		.name = "ingenic-vpu",
+#ifdef CONFIG_PM
+		.pm = &ingenic_rproc_pm,
+#endif
+		.of_match_table = ingenic_rproc_of_matches,
+	},
+};
+module_platform_driver(ingenic_rproc_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Paul Cercueil <paul@crapouillou.net>");
+MODULE_DESCRIPTION("Ingenic JZ47xx Remote Processor control driver");
-- 
2.26.2


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

* [PATCH v7 5/5] MAINTAINERS: Add myself as reviewer for Ingenic rproc driver
  2020-05-15 10:43 [PATCH v7 1/5] dt-bindings: Document JZ47xx VPU auxiliary processor Paul Cercueil
                   ` (2 preceding siblings ...)
  2020-05-15 10:43 ` [PATCH v7 4/5] remoteproc: ingenic: Added remoteproc driver Paul Cercueil
@ 2020-05-15 10:43 ` Paul Cercueil
  3 siblings, 0 replies; 8+ messages in thread
From: Paul Cercueil @ 2020-05-15 10:43 UTC (permalink / raw)
  To: Bjorn Andersson, Ohad Ben-Cohen, Arnaud Pouliquen
  Cc: od, linux-remoteproc, devicetree, linux-kernel, Paul Cercueil

Add myself as the reviewer for the Ingenic VPU remoteproc driver.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---

Notes:
    v4: New patch
    v5-v7: No change

 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 091ec22c1a23..5d864d785574 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8414,6 +8414,7 @@ F:	drivers/mtd/nand/raw/ingenic/
 F:	drivers/pinctrl/pinctrl-ingenic.c
 F:	drivers/power/supply/ingenic-battery.c
 F:	drivers/pwm/pwm-jz4740.c
+F:	drivers/remoteproc/ingenic_rproc.c
 F:	drivers/rtc/rtc-jz4740.c
 F:	drivers/tty/serial/8250/8250_ingenic.c
 F:	drivers/usb/musb/jz4740.c
-- 
2.26.2


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

* Re: [PATCH v7 4/5] remoteproc: ingenic: Added remoteproc driver
  2020-05-15 10:43 ` [PATCH v7 4/5] remoteproc: ingenic: Added remoteproc driver Paul Cercueil
@ 2020-05-18 23:57   ` Bjorn Andersson
  0 siblings, 0 replies; 8+ messages in thread
From: Bjorn Andersson @ 2020-05-18 23:57 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: Ohad Ben-Cohen, Arnaud Pouliquen, od, linux-remoteproc,
	devicetree, linux-kernel, Mathieu Poirier

On Fri 15 May 03:43 PDT 2020, Paul Cercueil wrote:

> This driver is used to boot, communicate with and load firmwares to the
> MIPS co-processor found in the VPU hardware of the JZ47xx SoCs from
> Ingenic.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org>

Series applied

Thanks,
Bjorn

> ---
> 
> Notes:
>     v2: Remove exception for always-mapped memories
>     v3: - Use clk_bulk API
>     	- Move device-managed code to its own patch [3/4]
>     	- Move devicetree table right above ingenic_rproc_driver
>     	- Removed #ifdef CONFIG_OF around devicetree table
>     	- Removed .owner = THIS_MODULE in ingenic_rproc_driver
>     	- Removed useless platform_set_drvdata()
>     v4: - Add fix reported by Julia
>     	- Change Kconfig symbol to INGENIC_VPU_RPROC
>     	- Add documentation to struct vpu
>     	- disable_irq_nosync() -> disable_irq()
>     v5: No change
>     v6: Instead of prepare/unprepare callbacks, use PM runtime callbacks
>     v7: - Remove use of of_match_ptr()
>     	- Move Kconfig symbol so that it's in alphabetical order
>     	- Add missing doc for private structure field aux_base
>     	- Don't check for (len <= 0) in da_to_va()
>     	- Add missing \n in dev_info/dev_err messages
> 
>  drivers/remoteproc/Kconfig         |   9 +
>  drivers/remoteproc/Makefile        |   1 +
>  drivers/remoteproc/ingenic_rproc.c | 280 +++++++++++++++++++++++++++++
>  3 files changed, 290 insertions(+)
>  create mode 100644 drivers/remoteproc/ingenic_rproc.c
> 
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index fbaed079b299..c4d1731295eb 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -23,6 +23,15 @@ config IMX_REMOTEPROC
>  
>  	  It's safe to say N here.
>  
> +config INGENIC_VPU_RPROC
> +	tristate "Ingenic JZ47xx VPU remoteproc support"
> +	depends on MIPS || COMPILE_TEST
> +	help
> +	  Say y or m here to support the VPU in the JZ47xx SoCs from Ingenic.
> +
> +	  This can be either built-in or a loadable module.
> +	  If unsure say N.
> +
>  config MTK_SCP
>  	tristate "Mediatek SCP support"
>  	depends on ARCH_MEDIATEK
> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> index 0effd3825035..e8b886e511f0 100644
> --- a/drivers/remoteproc/Makefile
> +++ b/drivers/remoteproc/Makefile
> @@ -10,6 +10,7 @@ remoteproc-y				+= remoteproc_sysfs.o
>  remoteproc-y				+= remoteproc_virtio.o
>  remoteproc-y				+= remoteproc_elf_loader.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_OMAP_REMOTEPROC)		+= omap_remoteproc.o
>  obj-$(CONFIG_WKUP_M3_RPROC)		+= wkup_m3_rproc.o
> diff --git a/drivers/remoteproc/ingenic_rproc.c b/drivers/remoteproc/ingenic_rproc.c
> new file mode 100644
> index 000000000000..189020d77b25
> --- /dev/null
> +++ b/drivers/remoteproc/ingenic_rproc.c
> @@ -0,0 +1,280 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Ingenic JZ47xx remoteproc driver
> + * Copyright 2019, Paul Cercueil <paul@crapouillou.net>
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/remoteproc.h>
> +
> +#include "remoteproc_internal.h"
> +
> +#define REG_AUX_CTRL		0x0
> +#define REG_AUX_MSG_ACK		0x10
> +#define REG_AUX_MSG		0x14
> +#define REG_CORE_MSG_ACK	0x18
> +#define REG_CORE_MSG		0x1C
> +
> +#define AUX_CTRL_SLEEP		BIT(31)
> +#define AUX_CTRL_MSG_IRQ_EN	BIT(3)
> +#define AUX_CTRL_NMI_RESETS	BIT(2)
> +#define AUX_CTRL_NMI		BIT(1)
> +#define AUX_CTRL_SW_RESET	BIT(0)
> +
> +struct vpu_mem_map {
> +	const char *name;
> +	unsigned int da;
> +};
> +
> +struct vpu_mem_info {
> +	const struct vpu_mem_map *map;
> +	unsigned long len;
> +	void __iomem *base;
> +};
> +
> +static const struct vpu_mem_map vpu_mem_map[] = {
> +	{ "tcsm0", 0x132b0000 },
> +	{ "tcsm1", 0xf4000000 },
> +	{ "sram",  0x132f0000 },
> +};
> +
> +/**
> + * struct vpu - Ingenic VPU remoteproc private structure
> + * @irq: interrupt number
> + * @clks: pointers to the VPU and AUX clocks
> + * @aux_base: raw pointer to the AUX interface registers
> + * @mem_info: array of struct vpu_mem_info, which contain the mapping info of
> + *            each of the external memories
> + * @dev: private pointer to the device
> + */
> +struct vpu {
> +	int irq;
> +	struct clk_bulk_data clks[2];
> +	void __iomem *aux_base;
> +	struct vpu_mem_info mem_info[ARRAY_SIZE(vpu_mem_map)];
> +	struct device *dev;
> +};
> +
> +static int ingenic_rproc_start(struct rproc *rproc)
> +{
> +	struct vpu *vpu = rproc->priv;
> +	u32 ctrl;
> +
> +	enable_irq(vpu->irq);
> +
> +	/* Reset the AUX and enable message IRQ */
> +	ctrl = AUX_CTRL_NMI_RESETS | AUX_CTRL_NMI | AUX_CTRL_MSG_IRQ_EN;
> +	writel(ctrl, vpu->aux_base + REG_AUX_CTRL);
> +
> +	return 0;
> +}
> +
> +static int ingenic_rproc_stop(struct rproc *rproc)
> +{
> +	struct vpu *vpu = rproc->priv;
> +
> +	disable_irq(vpu->irq);
> +
> +	/* Keep AUX in reset mode */
> +	writel(AUX_CTRL_SW_RESET, vpu->aux_base + REG_AUX_CTRL);
> +
> +	return 0;
> +}
> +
> +static void ingenic_rproc_kick(struct rproc *rproc, int vqid)
> +{
> +	struct vpu *vpu = rproc->priv;
> +
> +	writel(vqid, vpu->aux_base + REG_CORE_MSG);
> +}
> +
> +static void *ingenic_rproc_da_to_va(struct rproc *rproc, u64 da, size_t len)
> +{
> +	struct vpu *vpu = rproc->priv;
> +	void __iomem *va = NULL;
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(vpu_mem_map); i++) {
> +		const struct vpu_mem_info *info = &vpu->mem_info[i];
> +		const struct vpu_mem_map *map = info->map;
> +
> +		if (da >= map->da && (da + len) < (map->da + info->len)) {
> +			va = info->base + (da - map->da);
> +			break;
> +		}
> +	}
> +
> +	return (__force void *)va;
> +}
> +
> +static struct rproc_ops ingenic_rproc_ops = {
> +	.start = ingenic_rproc_start,
> +	.stop = ingenic_rproc_stop,
> +	.kick = ingenic_rproc_kick,
> +	.da_to_va = ingenic_rproc_da_to_va,
> +};
> +
> +static irqreturn_t vpu_interrupt(int irq, void *data)
> +{
> +	struct rproc *rproc = data;
> +	struct vpu *vpu = rproc->priv;
> +	u32 vring;
> +
> +	vring = readl(vpu->aux_base + REG_AUX_MSG);
> +
> +	/* Ack the interrupt */
> +	writel(0, vpu->aux_base + REG_AUX_MSG_ACK);
> +
> +	return rproc_vq_interrupt(rproc, vring);
> +}
> +
> +static void ingenic_rproc_disable_clks(void *data)
> +{
> +	struct vpu *vpu = data;
> +
> +	pm_runtime_resume(vpu->dev);
> +	pm_runtime_disable(vpu->dev);
> +
> +	clk_bulk_disable_unprepare(ARRAY_SIZE(vpu->clks), vpu->clks);
> +}
> +
> +static int ingenic_rproc_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct resource *mem;
> +	struct rproc *rproc;
> +	struct vpu *vpu;
> +	unsigned int i;
> +	int ret;
> +
> +	rproc = devm_rproc_alloc(dev, "ingenic-vpu",
> +				 &ingenic_rproc_ops, NULL, sizeof(*vpu));
> +	if (!rproc)
> +		return -ENOMEM;
> +
> +	vpu = rproc->priv;
> +	vpu->dev = &pdev->dev;
> +	platform_set_drvdata(pdev, vpu);
> +
> +	mem = platform_get_resource_byname(pdev, IORESOURCE_MEM, "aux");
> +	vpu->aux_base = devm_ioremap_resource(dev, mem);
> +	if (IS_ERR(vpu->aux_base)) {
> +		dev_err(dev, "Failed to ioremap\n");
> +		return PTR_ERR(vpu->aux_base);
> +	}
> +
> +	for (i = 0; i < ARRAY_SIZE(vpu_mem_map); i++) {
> +		mem = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> +						   vpu_mem_map[i].name);
> +
> +		vpu->mem_info[i].base = devm_ioremap_resource(dev, mem);
> +		if (IS_ERR(vpu->mem_info[i].base)) {
> +			ret = PTR_ERR(vpu->mem_info[i].base);
> +			dev_err(dev, "Failed to ioremap\n");
> +			return ret;
> +		}
> +
> +		vpu->mem_info[i].len = resource_size(mem);
> +		vpu->mem_info[i].map = &vpu_mem_map[i];
> +	}
> +
> +	vpu->clks[0].id = "vpu";
> +	vpu->clks[1].id = "aux";
> +
> +	ret = devm_clk_bulk_get(dev, ARRAY_SIZE(vpu->clks), vpu->clks);
> +	if (ret) {
> +		dev_err(dev, "Failed to get clocks\n");
> +		return ret;
> +	}
> +
> +	vpu->irq = platform_get_irq(pdev, 0);
> +	if (vpu->irq < 0)
> +		return vpu->irq;
> +
> +	ret = devm_request_irq(dev, vpu->irq, vpu_interrupt, 0, "VPU", rproc);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to request IRQ\n");
> +		return ret;
> +	}
> +
> +	disable_irq(vpu->irq);
> +
> +	/* The clocks must be enabled for the firmware to be loaded in TCSM */
> +	ret = clk_bulk_prepare_enable(ARRAY_SIZE(vpu->clks), vpu->clks);
> +	if (ret) {
> +		dev_err(dev, "Unable to start clocks\n");
> +		return ret;
> +	}
> +
> +	pm_runtime_irq_safe(dev);
> +	pm_runtime_set_active(dev);
> +	pm_runtime_enable(dev);
> +	pm_runtime_get_sync(dev);
> +	pm_runtime_use_autosuspend(dev);
> +
> +	ret = devm_add_action_or_reset(dev, ingenic_rproc_disable_clks, vpu);
> +	if (ret) {
> +		dev_err(dev, "Unable to register action\n");
> +		goto out_pm_put;
> +	}
> +
> +	ret = devm_rproc_add(dev, rproc);
> +	if (ret) {
> +		dev_err(dev, "Failed to register remote processor\n");
> +		goto out_pm_put;
> +	}
> +
> +out_pm_put:
> +	pm_runtime_put_autosuspend(dev);
> +
> +	return ret;
> +}
> +
> +static const struct of_device_id ingenic_rproc_of_matches[] = {
> +	{ .compatible = "ingenic,jz4770-vpu-rproc", },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, ingenic_rproc_of_matches);
> +
> +static int __maybe_unused ingenic_rproc_suspend(struct device *dev)
> +{
> +	struct vpu *vpu = dev_get_drvdata(dev);
> +
> +	clk_bulk_disable(ARRAY_SIZE(vpu->clks), vpu->clks);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused ingenic_rproc_resume(struct device *dev)
> +{
> +	struct vpu *vpu = dev_get_drvdata(dev);
> +
> +	return clk_bulk_enable(ARRAY_SIZE(vpu->clks), vpu->clks);
> +}
> +
> +static const struct dev_pm_ops __maybe_unused ingenic_rproc_pm = {
> +	SET_RUNTIME_PM_OPS(ingenic_rproc_suspend, ingenic_rproc_resume, NULL)
> +};
> +
> +static struct platform_driver ingenic_rproc_driver = {
> +	.probe = ingenic_rproc_probe,
> +	.driver = {
> +		.name = "ingenic-vpu",
> +#ifdef CONFIG_PM
> +		.pm = &ingenic_rproc_pm,
> +#endif
> +		.of_match_table = ingenic_rproc_of_matches,
> +	},
> +};
> +module_platform_driver(ingenic_rproc_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Paul Cercueil <paul@crapouillou.net>");
> +MODULE_DESCRIPTION("Ingenic JZ47xx Remote Processor control driver");
> -- 
> 2.26.2
> 

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

* Re: [PATCH v7 3/5] remoteproc: Add support for runtime PM
  2020-05-15 10:43 ` [PATCH v7 3/5] remoteproc: Add support for runtime PM Paul Cercueil
@ 2020-05-22 16:47   ` Suman Anna
  2020-05-22 17:11     ` Paul Cercueil
  0 siblings, 1 reply; 8+ messages in thread
From: Suman Anna @ 2020-05-22 16:47 UTC (permalink / raw)
  To: Paul Cercueil, Bjorn Andersson, Ohad Ben-Cohen, Arnaud Pouliquen
  Cc: od, linux-remoteproc, devicetree, linux-kernel, Tero Kristo

Hi Paul,

On 5/15/20 5:43 AM, Paul Cercueil wrote:
> Call pm_runtime_get_sync() before the firmware is loaded, and
> pm_runtime_put() after the remote processor has been stopped.
> 
> Even though the remoteproc device has no PM callbacks, this allows the
> parent device's PM callbacks to be properly called.

I see this patch staged now for 5.8, and the latest -next branch has 
broken the pm-runtime autosuspend feature we have in the OMAP remoteproc 
driver. See commit 5f31b232c674 ("remoteproc/omap: Add support for 
runtime auto-suspend/resume").

What was the original purpose of this patch, because there can be 
differing backends across different SoCs.

regards
Suman

> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
> 
> Notes:
>      v2-v4: No change
>      v5: Move calls to prepare/unprepare to rproc_fw_boot/rproc_shutdown
>      v6: Instead of prepare/unprepare callbacks, use PM runtime callbacks
>      v7: Check return value of pm_runtime_get_sync()
> 
>   drivers/remoteproc/remoteproc_core.c | 17 ++++++++++++++++-
>   1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index a7f96bc98406..e33d1ef27981 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -29,6 +29,7 @@
>   #include <linux/devcoredump.h>
>   #include <linux/rculist.h>
>   #include <linux/remoteproc.h>
> +#include <linux/pm_runtime.h>
>   #include <linux/iommu.h>
>   #include <linux/idr.h>
>   #include <linux/elf.h>
> @@ -1382,6 +1383,12 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
>   	if (ret)
>   		return ret;
>   
> +	ret = pm_runtime_get_sync(dev);
> +	if (ret < 0) {
> +		dev_err(dev, "pm_runtime_get_sync failed: %d\n", ret);
> +		return ret;
> +	}
> +
>   	dev_info(dev, "Booting fw image %s, size %zd\n", name, fw->size);
>   
>   	/*
> @@ -1391,7 +1398,7 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
>   	ret = rproc_enable_iommu(rproc);
>   	if (ret) {
>   		dev_err(dev, "can't enable iommu: %d\n", ret);
> -		return ret;
> +		goto put_pm_runtime;
>   	}
>   
>   	rproc->bootaddr = rproc_get_boot_addr(rproc, fw);
> @@ -1435,6 +1442,8 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
>   	rproc->table_ptr = NULL;
>   disable_iommu:
>   	rproc_disable_iommu(rproc);
> +put_pm_runtime:
> +	pm_runtime_put(dev);
>   	return ret;
>   }
>   
> @@ -1840,6 +1849,8 @@ void rproc_shutdown(struct rproc *rproc)
>   
>   	rproc_disable_iommu(rproc);
>   
> +	pm_runtime_put(dev);
> +
>   	/* Free the copy of the resource table */
>   	kfree(rproc->cached_table);
>   	rproc->cached_table = NULL;
> @@ -2118,6 +2129,9 @@ struct rproc *rproc_alloc(struct device *dev, const char *name,
>   
>   	rproc->state = RPROC_OFFLINE;
>   
> +	pm_runtime_no_callbacks(&rproc->dev);
> +	pm_runtime_enable(&rproc->dev);
> +
>   	return rproc;
>   }
>   EXPORT_SYMBOL(rproc_alloc);
> @@ -2133,6 +2147,7 @@ EXPORT_SYMBOL(rproc_alloc);
>    */
>   void rproc_free(struct rproc *rproc)
>   {
> +	pm_runtime_disable(&rproc->dev);
>   	put_device(&rproc->dev);
>   }
>   EXPORT_SYMBOL(rproc_free);
> 


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

* Re: [PATCH v7 3/5] remoteproc: Add support for runtime PM
  2020-05-22 16:47   ` Suman Anna
@ 2020-05-22 17:11     ` Paul Cercueil
  0 siblings, 0 replies; 8+ messages in thread
From: Paul Cercueil @ 2020-05-22 17:11 UTC (permalink / raw)
  To: Suman Anna
  Cc: Bjorn Andersson, Ohad Ben-Cohen, Arnaud Pouliquen, od,
	linux-remoteproc, devicetree, linux-kernel, Tero Kristo

Hi Suman,

Le ven. 22 mai 2020 à 11:47, Suman Anna <s-anna@ti.com> a écrit :
> Hi Paul,
> 
> On 5/15/20 5:43 AM, Paul Cercueil wrote:
>> Call pm_runtime_get_sync() before the firmware is loaded, and
>> pm_runtime_put() after the remote processor has been stopped.
>> 
>> Even though the remoteproc device has no PM callbacks, this allows 
>> the
>> parent device's PM callbacks to be properly called.
> 
> I see this patch staged now for 5.8, and the latest -next branch has 
> broken the pm-runtime autosuspend feature we have in the OMAP 
> remoteproc driver. See commit 5f31b232c674 ("remoteproc/omap: Add 
> support for runtime auto-suspend/resume").
> 
> What was the original purpose of this patch, because there can be 
> differing backends across different SoCs.

Did you try pm_suspend_ignore_children()? It looks like it was made for 
your use-case.

Cheers,
-Paul

> 
> regards
> Suman
> 
>> 
>> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>> ---
>> 
>> Notes:
>>      v2-v4: No change
>>      v5: Move calls to prepare/unprepare to 
>> rproc_fw_boot/rproc_shutdown
>>      v6: Instead of prepare/unprepare callbacks, use PM runtime 
>> callbacks
>>      v7: Check return value of pm_runtime_get_sync()
>> 
>>   drivers/remoteproc/remoteproc_core.c | 17 ++++++++++++++++-
>>   1 file changed, 16 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/remoteproc/remoteproc_core.c 
>> b/drivers/remoteproc/remoteproc_core.c
>> index a7f96bc98406..e33d1ef27981 100644
>> --- a/drivers/remoteproc/remoteproc_core.c
>> +++ b/drivers/remoteproc/remoteproc_core.c
>> @@ -29,6 +29,7 @@
>>   #include <linux/devcoredump.h>
>>   #include <linux/rculist.h>
>>   #include <linux/remoteproc.h>
>> +#include <linux/pm_runtime.h>
>>   #include <linux/iommu.h>
>>   #include <linux/idr.h>
>>   #include <linux/elf.h>
>> @@ -1382,6 +1383,12 @@ static int rproc_fw_boot(struct rproc *rproc, 
>> const struct firmware *fw)
>>   	if (ret)
>>   		return ret;
>>   \x7f+	ret = pm_runtime_get_sync(dev);
>> +	if (ret < 0) {
>> +		dev_err(dev, "pm_runtime_get_sync failed: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>>   	dev_info(dev, "Booting fw image %s, size %zd\n", name, fw->size);
>>   \x7f  	/*
>> @@ -1391,7 +1398,7 @@ static int rproc_fw_boot(struct rproc *rproc, 
>> const struct firmware *fw)
>>   	ret = rproc_enable_iommu(rproc);
>>   	if (ret) {
>>   		dev_err(dev, "can't enable iommu: %d\n", ret);
>> -		return ret;
>> +		goto put_pm_runtime;
>>   	}
>>   \x7f  	rproc->bootaddr = rproc_get_boot_addr(rproc, fw);
>> @@ -1435,6 +1442,8 @@ static int rproc_fw_boot(struct rproc *rproc, 
>> const struct firmware *fw)
>>   	rproc->table_ptr = NULL;
>>   disable_iommu:
>>   	rproc_disable_iommu(rproc);
>> +put_pm_runtime:
>> +	pm_runtime_put(dev);
>>   	return ret;
>>   }
>>   \x7f@@ -1840,6 +1849,8 @@ void rproc_shutdown(struct rproc *rproc)
>>   \x7f  	rproc_disable_iommu(rproc);
>>   \x7f+	pm_runtime_put(dev);
>> +
>>   	/* Free the copy of the resource table */
>>   	kfree(rproc->cached_table);
>>   	rproc->cached_table = NULL;
>> @@ -2118,6 +2129,9 @@ struct rproc *rproc_alloc(struct device *dev, 
>> const char *name,
>>   \x7f  	rproc->state = RPROC_OFFLINE;
>>   \x7f+	pm_runtime_no_callbacks(&rproc->dev);
>> +	pm_runtime_enable(&rproc->dev);
>> +
>>   	return rproc;
>>   }
>>   EXPORT_SYMBOL(rproc_alloc);
>> @@ -2133,6 +2147,7 @@ EXPORT_SYMBOL(rproc_alloc);
>>    */
>>   void rproc_free(struct rproc *rproc)
>>   {
>> +	pm_runtime_disable(&rproc->dev);
>>   	put_device(&rproc->dev);
>>   }
>>   EXPORT_SYMBOL(rproc_free);
>> 
> 



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

end of thread, back to index

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-15 10:43 [PATCH v7 1/5] dt-bindings: Document JZ47xx VPU auxiliary processor Paul Cercueil
2020-05-15 10:43 ` [PATCH v7 2/5] remoteproc: Add device-managed variants of rproc_alloc/rproc_add Paul Cercueil
2020-05-15 10:43 ` [PATCH v7 3/5] remoteproc: Add support for runtime PM Paul Cercueil
2020-05-22 16:47   ` Suman Anna
2020-05-22 17:11     ` Paul Cercueil
2020-05-15 10:43 ` [PATCH v7 4/5] remoteproc: ingenic: Added remoteproc driver Paul Cercueil
2020-05-18 23:57   ` Bjorn Andersson
2020-05-15 10:43 ` [PATCH v7 5/5] MAINTAINERS: Add myself as reviewer for Ingenic rproc driver Paul Cercueil

Linux-remoteproc Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-remoteproc/0 linux-remoteproc/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-remoteproc linux-remoteproc/ https://lore.kernel.org/linux-remoteproc \
		linux-remoteproc@vger.kernel.org
	public-inbox-index linux-remoteproc

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-remoteproc


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git