The mediatek remoteproc driver currently only allows bringing up a
single core SCP, e.g. MT8183. It also only bringing up the 1st
core in SoCs with a dual-core SCP, e.g. MT8195. This series support
to bring-up the 2nd core of the dual-core SCP.
v2 -> v3:
1. change the representation of dual-core SCP in dts file and update SCP yaml
2. rewrite SCP driver to reflect the change of dts node
3. add SCP core 1 node to mt8195.dtsi
4. remove redundant call of rproc_boot for SCP
5. refine IPI error message
v1 -> v2:
1. update dt-binding property description
2. remove kconfig for scp dual driver
3. merge mtk_scp_dual.c and mtk_scp_subdev.c to mtk_scp.c
Tinghan Shen (11):
dt-bindings: remoteproc: mediatek: Give the subnode a persistent name
dt-bindings: remoteproc: mediatek: Support MT8195 dual-core SCP
arm64: dts: mt8195: Add SCP core 1 node
remoteproc: mediatek: Remove redundant rproc_boot
remoteproc: mediatek: Add SCP core 1 register definitions
remoteproc: mediatek: Add MT8195 SCP core 1 operations
remoteproc: mediatek: Probe MT8195 SCP core 1
remoteproc: mediatek: Control SCP core 1 boot by rproc subdevice
remoteproc: mediatek: Setup MT8195 SCP core 1 SRAM offset
remoteproc: mediatek: Handle MT8195 SCP core 1 watchdog timeout
remoteproc: mediatek: Refine ipi handler error message
.../bindings/remoteproc/mtk,scp.yaml | 132 ++++++++--
.../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 2 +-
arch/arm64/boot/dts/mediatek/mt8195.dtsi | 14 +-
.../mediatek/vcodec/mtk_vcodec_fw_scp.c | 2 +-
drivers/remoteproc/mtk_common.h | 35 +++
drivers/remoteproc/mtk_scp.c | 241 +++++++++++++++++-
include/linux/remoteproc/mtk_scp.h | 1 +
7 files changed, 397 insertions(+), 30 deletions(-)
--
2.18.0
Add MT8195 SCP core 1 related register definitions.
Signed-off-by: Tinghan Shen <[email protected]>
Reviewed-by: Mathieu Poirier <[email protected]>
---
drivers/remoteproc/mtk_common.h | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/drivers/remoteproc/mtk_common.h b/drivers/remoteproc/mtk_common.h
index ea6fa1100a00..3778894c96f3 100644
--- a/drivers/remoteproc/mtk_common.h
+++ b/drivers/remoteproc/mtk_common.h
@@ -47,6 +47,7 @@
#define MT8192_SCP2SPM_IPC_CLR 0x4094
#define MT8192_GIPC_IN_SET 0x4098
#define MT8192_HOST_IPC_INT_BIT BIT(0)
+#define MT8195_CORE1_HOST_IPC_INT_BIT BIT(4)
#define MT8192_CORE0_SW_RSTN_CLR 0x10000
#define MT8192_CORE0_SW_RSTN_SET 0x10004
@@ -56,6 +57,26 @@
#define MT8195_L1TCM_SRAM_PDN_RESERVED_RSI_BITS GENMASK(7, 4)
+#define MT8195_CPU1_SRAM_PD 0x1084
+#define MT8195_SSHUB2APMCU_IPC_SET 0x4088
+#define MT8195_SSHUB2APMCU_IPC_CLR 0x408C
+#define MT8195_CORE1_SW_RSTN_CLR 0x20000
+#define MT8195_CORE1_SW_RSTN_SET 0x20004
+#define MT8195_CORE1_MEM_ATT_PREDEF 0x20008
+#define MT8195_CORE1_WDT_IRQ 0x20030
+#define MT8195_CORE1_WDT_CFG 0x20034
+
+#define MT8195_SEC_CTRL 0x85000
+#define MT8195_CORE_OFFSET_ENABLE_D BIT(13)
+#define MT8195_CORE_OFFSET_ENABLE_I BIT(12)
+#define MT8195_L2TCM_OFFSET_RANGE_0_LOW 0x850b0
+#define MT8195_L2TCM_OFFSET_RANGE_0_HIGH 0x850b4
+#define MT8195_L2TCM_OFFSET 0x850d0
+#define SCP_SRAM_REMAP_LOW 0
+#define SCP_SRAM_REMAP_HIGH 1
+#define SCP_SRAM_REMAP_OFFSET 2
+#define SCP_SRAM_REMAP_SIZE 3
+
#define SCP_FW_VER_LEN 32
#define SCP_SHARE_BUFFER_SIZE 288
--
2.18.0
The error message doesn't accurately reflect the cause of
the error. The error is due to a handler not being found,
not an invalid IPI ID.
Signed-off-by: Tinghan Shen <[email protected]>
---
drivers/remoteproc/mtk_scp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
index 159f3c69cd69..8b1765c61442 100644
--- a/drivers/remoteproc/mtk_scp.c
+++ b/drivers/remoteproc/mtk_scp.c
@@ -106,7 +106,7 @@ static void scp_ipi_handler(struct mtk_scp *scp)
scp_ipi_lock(scp, id);
handler = ipi_desc[id].handler;
if (!handler) {
- dev_err(scp->dev, "No such ipi id = %d\n", id);
+ dev_err(scp->dev, "No handler for ipi id = %d\n", id);
scp_ipi_unlock(scp, id);
return;
}
--
2.18.0
Register SCP core 1 as a subdevice of core 0 to control the boot sequence
and watchdog handling. The core 1 has to boot after core 0 because the
SCP clock and SRAM power is controlled by SCP core 0.
When SCP core 0 reports a watchdog timeout event, the SRAM is emptied
before rebooting the SCP core 0, forcing the reload of the SCP core 1 image.
Signed-off-by: Tinghan Shen <[email protected]>
---
drivers/remoteproc/mtk_common.h | 8 ++++
drivers/remoteproc/mtk_scp.c | 71 +++++++++++++++++++++++++++++++++
2 files changed, 79 insertions(+)
diff --git a/drivers/remoteproc/mtk_common.h b/drivers/remoteproc/mtk_common.h
index 54265c515315..dcde25f8bbf9 100644
--- a/drivers/remoteproc/mtk_common.h
+++ b/drivers/remoteproc/mtk_common.h
@@ -95,6 +95,13 @@ struct scp_ipi_desc {
void *priv;
};
+struct scp_subdev_core {
+ struct rproc_subdev subdev;
+ struct mtk_scp *scp;
+};
+
+#define to_subdev_core(d) container_of(d, struct scp_subdev_core, subdev)
+
struct mtk_scp;
struct mtk_scp_of_data {
@@ -142,6 +149,7 @@ struct mtk_scp {
struct rproc_subdev *rpmsg_subdev;
struct mtk_scp *main_scp;
+ struct rproc_subdev *core_subdev;
};
/**
diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
index f7b738743ba9..2d43338b96da 100644
--- a/drivers/remoteproc/mtk_scp.c
+++ b/drivers/remoteproc/mtk_scp.c
@@ -871,6 +871,54 @@ static void scp_remove_rpmsg_subdev(struct mtk_scp *scp)
}
}
+static int scp_core_subdev_start(struct rproc_subdev *subdev)
+{
+ struct scp_subdev_core *subdev_core = to_subdev_core(subdev);
+ struct mtk_scp *scp = subdev_core->scp;
+
+ rproc_boot(scp->rproc);
+
+ return 0;
+}
+
+static void scp_core_subdev_stop(struct rproc_subdev *subdev, bool crashed)
+{
+ struct scp_subdev_core *subdev_core = to_subdev_core(subdev);
+ struct mtk_scp *scp = subdev_core->scp;
+
+ rproc_shutdown(scp->rproc);
+}
+
+static int scp_core_subdev_register(struct mtk_scp *scp)
+{
+ struct device *dev = scp->dev;
+ struct scp_subdev_core *subdev_core;
+
+ subdev_core = devm_kzalloc(dev, sizeof(*subdev_core), GFP_KERNEL);
+ if (!subdev_core) {
+ scp->core_subdev = NULL;
+ return -ENOMEM;
+ }
+
+ subdev_core->scp = scp;
+ subdev_core->subdev.start = scp_core_subdev_start;
+ subdev_core->subdev.stop = scp_core_subdev_stop;
+
+ scp->core_subdev = &subdev_core->subdev;
+ rproc_add_subdev(scp->main_scp->rproc, scp->core_subdev);
+
+ return 0;
+}
+
+static void scp_core_subdev_unregister(struct mtk_scp *scp)
+{
+ if (scp->core_subdev) {
+ rproc_remove_subdev(scp->main_scp->rproc, scp->core_subdev);
+ devm_kfree(scp->dev, to_subdev_core(scp->core_subdev));
+ scp->core_subdev = NULL;
+ }
+}
+
static int scp_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -974,6 +1022,23 @@ static int scp_probe(struct platform_device *pdev)
goto remove_subdev;
}
+ if (of_device_is_compatible(np, "mediatek,mt8195-scp-core")) {
+ ret = scp_core_subdev_register(scp);
+ if (ret) {
+ dev_err_probe(dev, ret, "Failed to register subdev\n");
+ goto remove_subdev;
+ }
+
+ /* sub cores are booted as subdevices of main core. */
+ rproc->auto_boot = false;
+ } else {
+ ret = devm_of_platform_populate(dev);
+ if (ret) {
+ dev_err_probe(dev, ret, "Failed to probe sub cores\n");
+ goto remove_subdev;
+ }
+ }
+
ret = rproc_add(rproc);
if (ret)
goto remove_subdev;
@@ -981,6 +1046,7 @@ static int scp_probe(struct platform_device *pdev)
return 0;
remove_subdev:
+ scp_core_subdev_unregister(scp);
scp_remove_rpmsg_subdev(scp);
scp_ipi_unregister(scp, SCP_IPI_INIT);
release_dev_mem:
@@ -997,6 +1063,11 @@ static int scp_remove(struct platform_device *pdev)
struct mtk_scp *scp = platform_get_drvdata(pdev);
int i;
+ if (of_device_is_compatible(scp->dev->of_node, "mediatek,mt8195-scp-core"))
+ scp_core_subdev_unregister(scp);
+ else
+ devm_of_platform_depopulate(scp->dev);
+
rproc_del(scp->rproc);
scp_remove_rpmsg_subdev(scp);
scp_ipi_unregister(scp, SCP_IPI_INIT);
--
2.18.0
The MT8195 SCP configuration registers for core 0 and core 1 is the same.
Let SCP core 1 to reuse the mapped address requested by SCP core 0.
Signed-off-by: Tinghan Shen <[email protected]>
---
drivers/remoteproc/mtk_common.h | 2 ++
drivers/remoteproc/mtk_scp.c | 27 +++++++++++++++++++++++----
2 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/drivers/remoteproc/mtk_common.h b/drivers/remoteproc/mtk_common.h
index 3778894c96f3..54265c515315 100644
--- a/drivers/remoteproc/mtk_common.h
+++ b/drivers/remoteproc/mtk_common.h
@@ -140,6 +140,8 @@ struct mtk_scp {
size_t dram_size;
struct rproc_subdev *rpmsg_subdev;
+
+ struct mtk_scp *main_scp;
};
/**
diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
index 1d17d77b8a14..f7b738743ba9 100644
--- a/drivers/remoteproc/mtk_scp.c
+++ b/drivers/remoteproc/mtk_scp.c
@@ -917,10 +917,29 @@ static int scp_probe(struct platform_device *pdev)
scp->l1tcm_phys = res->start;
}
- scp->reg_base = devm_platform_ioremap_resource_byname(pdev, "cfg");
- if (IS_ERR(scp->reg_base))
- return dev_err_probe(dev, PTR_ERR(scp->reg_base),
- "Failed to parse and map cfg memory\n");
+ if (of_device_is_compatible(np, "mediatek,mt8195-scp-core")) {
+ struct device_node *pnp;
+ struct platform_device *scp_pdev;
+
+ pnp = of_get_parent(np);
+ if (!pnp)
+ return dev_err_probe(dev, -ENODEV, "Failed to get parent core 0\n");
+
+ scp_pdev = of_find_device_by_node(pnp);
+ of_node_put(pnp);
+ if (!scp_pdev)
+ return dev_err_probe(dev, -ENODEV, "Failed to get scp core 0 pdev\n");
+
+ scp->main_scp = platform_get_drvdata(scp_pdev);
+ scp->reg_base = scp->main_scp->reg_base;
+ } else {
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cfg");
+ scp->reg_base = devm_ioremap_resource(dev, res);
+
+ if (IS_ERR(scp->reg_base))
+ return dev_err_probe(dev, PTR_ERR(scp->reg_base),
+ "Failed to parse and map cfg memory\n");
+ }
ret = scp->data->scp_clk_get(scp);
if (ret)
--
2.18.0
The node name doesn't matter to add the subnode as a cros-ec rpmsg device.
Give it a clear persistent node name to simplify scp yaml.
Signed-off-by: Tinghan Shen <[email protected]>
---
.../bindings/remoteproc/mtk,scp.yaml | 35 ++++++++++---------
.../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 2 +-
2 files changed, 20 insertions(+), 17 deletions(-)
diff --git a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
index 7e091eaffc18..786bed897916 100644
--- a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
@@ -58,6 +58,23 @@ properties:
memory-region:
maxItems: 1
+ cros-ec-rpmsg:
+ type: object
+ description:
+ This subnode represents the rpmsg device. The names of the devices
+ are not important. The properties of this node are defined by the
+ individual bindings for the rpmsg devices.
+
+ properties:
+ mediatek,rpmsg-name:
+ $ref: /schemas/types.yaml#/definitions/string-array
+ description:
+ Contains the name for the rpmsg device. Used to match
+ the subnode to rpmsg device announced by SCP.
+
+ required:
+ - mediatek,rpmsg-name
+
required:
- compatible
- reg
@@ -89,21 +106,7 @@ allOf:
reg-names:
maxItems: 2
-additionalProperties:
- type: object
- description:
- Subnodes of the SCP represent rpmsg devices. The names of the devices
- are not important. The properties of these nodes are defined by the
- individual bindings for the rpmsg devices.
- properties:
- mediatek,rpmsg-name:
- $ref: /schemas/types.yaml#/definitions/string-array
- description:
- Contains the name for the rpmsg device. Used to match
- the subnode to rpmsg device announced by SCP.
-
- required:
- - mediatek,rpmsg-name
+additionalProperties: false
examples:
- |
@@ -118,7 +121,7 @@ examples:
clocks = <&infracfg CLK_INFRA_SCPSYS>;
clock-names = "main";
- cros_ec {
+ cros-ec-rpmsg {
mediatek,rpmsg-name = "cros-ec-rpmsg";
};
};
diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
index b4b86bb1f1a7..693ad5f2a82e 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
@@ -816,7 +816,7 @@
pinctrl-names = "default";
pinctrl-0 = <&scp_pins>;
- cros_ec {
+ cros-ec-rpmsg {
compatible = "google,cros-ec-rpmsg";
mediatek,rpmsg-name = "cros-ec-rpmsg";
};
--
2.18.0
The video codec doesn't need to explicitly boot SCP in its flow
because the SCP remote processor enables auto boot.
The redundant usage of rproc_boot increases the number of rproc.power
over 1 and prevents successfully shutting down SCP by rproc_shutdown.
Signed-off-by: Tinghan Shen <[email protected]>
---
.../mediatek/vcodec/mtk_vcodec_fw_scp.c | 2 +-
drivers/remoteproc/mtk_scp.c | 17 +++++++++++++++++
include/linux/remoteproc/mtk_scp.h | 1 +
3 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_fw_scp.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_fw_scp.c
index d8e66b645bd8..c3194f88ff31 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_fw_scp.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_fw_scp.c
@@ -6,7 +6,7 @@
static int mtk_vcodec_scp_load_firmware(struct mtk_vcodec_fw *fw)
{
- return rproc_boot(scp_get_rproc(fw->scp));
+ return scp_boot(fw->scp);
}
static unsigned int mtk_vcodec_scp_get_vdec_capa(struct mtk_vcodec_fw *fw)
diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
index d421a2ccaa1e..bf68bccab78b 100644
--- a/drivers/remoteproc/mtk_scp.c
+++ b/drivers/remoteproc/mtk_scp.c
@@ -673,6 +673,23 @@ struct rproc *scp_get_rproc(struct mtk_scp *scp)
}
EXPORT_SYMBOL_GPL(scp_get_rproc);
+/**
+ * scp_boot() - Boot SCP
+ *
+ * @scp: mtk_scp structure
+ **/
+int scp_boot(struct mtk_scp *scp)
+{
+ struct rproc *rproc = scp->rproc;
+
+ /* scp already booted when power > 0 */
+ if (atomic_read(&rproc->power) > 0)
+ return 0;
+ else
+ return rproc_boot(scp->rproc);
+}
+EXPORT_SYMBOL_GPL(scp_boot);
+
/**
* scp_get_vdec_hw_capa() - get video decoder hardware capability
*
diff --git a/include/linux/remoteproc/mtk_scp.h b/include/linux/remoteproc/mtk_scp.h
index 7c2b7cc9fe6c..e463105b351c 100644
--- a/include/linux/remoteproc/mtk_scp.h
+++ b/include/linux/remoteproc/mtk_scp.h
@@ -52,6 +52,7 @@ void scp_put(struct mtk_scp *scp);
struct device *scp_get_device(struct mtk_scp *scp);
struct rproc *scp_get_rproc(struct mtk_scp *scp);
+int scp_boot(struct mtk_scp *scp);
int scp_ipi_register(struct mtk_scp *scp, u32 id, scp_ipi_handler_t handler,
void *priv);
--
2.18.0
The SCP rproc driver has a set of chip dependent callbacks for
boot sequence and IRQ handling. Implement these callbacks for MT8195
SCP core 1.
Signed-off-by: Tinghan Shen <[email protected]>
---
drivers/remoteproc/mtk_scp.c | 57 ++++++++++++++++++++++++++++++++++++
1 file changed, 57 insertions(+)
diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
index bf68bccab78b..1d17d77b8a14 100644
--- a/drivers/remoteproc/mtk_scp.c
+++ b/drivers/remoteproc/mtk_scp.c
@@ -176,6 +176,16 @@ static void mt8192_scp_reset_deassert(struct mtk_scp *scp)
writel(1, scp->reg_base + MT8192_CORE0_SW_RSTN_CLR);
}
+static void mt8195_scp_c1_reset_assert(struct mtk_scp *scp)
+{
+ writel(1, scp->reg_base + MT8195_CORE1_SW_RSTN_SET);
+}
+
+static void mt8195_scp_c1_reset_deassert(struct mtk_scp *scp)
+{
+ writel(1, scp->reg_base + MT8195_CORE1_SW_RSTN_CLR);
+}
+
static void mt8183_scp_irq_handler(struct mtk_scp *scp)
{
u32 scp_to_host;
@@ -212,6 +222,18 @@ static void mt8192_scp_irq_handler(struct mtk_scp *scp)
}
}
+static void mt8195_scp_c1_irq_handler(struct mtk_scp *scp)
+{
+ u32 scp_to_host;
+
+ scp_to_host = readl(scp->reg_base + MT8195_SSHUB2APMCU_IPC_SET);
+
+ if (scp_to_host & MT8192_SCP_IPC_INT_BIT)
+ scp_ipi_handler(scp);
+
+ writel(scp_to_host, scp->reg_base + MT8195_SSHUB2APMCU_IPC_CLR);
+}
+
static irqreturn_t scp_irq_handler(int irq, void *priv)
{
struct mtk_scp *scp = priv;
@@ -453,6 +475,19 @@ static int mt8195_scp_before_load(struct mtk_scp *scp)
return 0;
}
+static int mt8195_scp_c1_before_load(struct mtk_scp *scp)
+{
+ scp_sram_power_on(scp->reg_base + MT8195_CPU1_SRAM_PD, 0);
+
+ /* hold SCP in reset while loading FW. */
+ scp->data->scp_reset_assert(scp);
+
+ /* enable MPU for all memory regions */
+ writel(0xff, scp->reg_base + MT8195_CORE1_MEM_ATT_PREDEF);
+
+ return 0;
+}
+
static int scp_load(struct rproc *rproc, const struct firmware *fw)
{
struct mtk_scp *scp = rproc->priv;
@@ -625,6 +660,15 @@ static void mt8195_scp_stop(struct mtk_scp *scp)
writel(0, scp->reg_base + MT8192_CORE0_WDT_CFG);
}
+static void mt8195_scp_c1_stop(struct mtk_scp *scp)
+{
+ /* Power off CPU SRAM */
+ scp_sram_power_off(scp->reg_base + MT8195_CPU1_SRAM_PD, 0);
+
+ /* Disable SCP watchdog */
+ writel(0, scp->reg_base + MT8195_CORE1_WDT_CFG);
+}
+
static int scp_stop(struct rproc *rproc)
{
struct mtk_scp *scp = (struct mtk_scp *)rproc->priv;
@@ -1007,12 +1051,25 @@ static const struct mtk_scp_of_data mt8195_of_data = {
.host_to_scp_int_bit = MT8192_HOST_IPC_INT_BIT,
};
+static const struct mtk_scp_of_data mt8195_core_of_data = {
+ .scp_clk_get = mt8195_scp_clk_get,
+ .scp_before_load = mt8195_scp_c1_before_load,
+ .scp_irq_handler = mt8195_scp_c1_irq_handler,
+ .scp_reset_assert = mt8195_scp_c1_reset_assert,
+ .scp_reset_deassert = mt8195_scp_c1_reset_deassert,
+ .scp_stop = mt8195_scp_c1_stop,
+ .scp_da_to_va = mt8192_scp_da_to_va,
+ .host_to_scp_reg = MT8192_GIPC_IN_SET,
+ .host_to_scp_int_bit = MT8195_CORE1_HOST_IPC_INT_BIT,
+};
+
static const struct of_device_id mtk_scp_of_match[] = {
{ .compatible = "mediatek,mt8183-scp", .data = &mt8183_of_data },
{ .compatible = "mediatek,mt8186-scp", .data = &mt8186_of_data },
{ .compatible = "mediatek,mt8188-scp", .data = &mt8188_of_data },
{ .compatible = "mediatek,mt8192-scp", .data = &mt8192_of_data },
{ .compatible = "mediatek,mt8195-scp", .data = &mt8195_of_data },
+ { .compatible = "mediatek,mt8195-scp-core", .data = &mt8195_core_of_data },
{},
};
MODULE_DEVICE_TABLE(of, mtk_scp_of_match);
--
2.18.0
Il 27/09/22 04:56, Tinghan Shen ha scritto:
> Add MT8195 SCP core 1 related register definitions.
>
> Signed-off-by: Tinghan Shen <[email protected]>
> Reviewed-by: Mathieu Poirier <[email protected]>
Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
Il 27/09/22 04:56, Tinghan Shen ha scritto:
> The error message doesn't accurately reflect the cause of
> the error. The error is due to a handler not being found,
> not an invalid IPI ID.
>
> Signed-off-by: Tinghan Shen <[email protected]>
Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
Il 27/09/22 04:55, Tinghan Shen ha scritto:
> The video codec doesn't need to explicitly boot SCP in its flow
> because the SCP remote processor enables auto boot.
>
> The redundant usage of rproc_boot increases the number of rproc.power
> over 1 and prevents successfully shutting down SCP by rproc_shutdown.
>
> Signed-off-by: Tinghan Shen <[email protected]>
You should Cc stable on this commit, as it's a quite important fix.
Regards,
Angelo
On 9/27/2022 10:55 AM, Tinghan Shen wrote:
> The node name doesn't matter to add the subnode as a cros-ec rpmsg device.
> Give it a clear persistent node name to simplify scp yaml.
>
> Signed-off-by: Tinghan Shen <[email protected]>
> ---
> .../bindings/remoteproc/mtk,scp.yaml | 35 ++++++++++---------
> .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 2 +-
> 2 files changed, 20 insertions(+), 17 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> index 7e091eaffc18..786bed897916 100644
> --- a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> @@ -58,6 +58,23 @@ properties:
> memory-region:
> maxItems: 1
>
> + cros-ec-rpmsg:
> + type: object
> + description:
> + This subnode represents the rpmsg device. The names of the devices
> + are not important. The properties of this node are defined by the
> + individual bindings for the rpmsg devices.
> +
> + properties:
> + mediatek,rpmsg-name:
> + $ref: /schemas/types.yaml#/definitions/string-array
> + description:
> + Contains the name for the rpmsg device. Used to match
> + the subnode to rpmsg device announced by SCP.
> +
> + required:
> + - mediatek,rpmsg-name
> +
> required:
> - compatible
> - reg
> @@ -89,21 +106,7 @@ allOf:
> reg-names:
> maxItems: 2
>
> -additionalProperties:
> - type: object
> - description:
> - Subnodes of the SCP represent rpmsg devices. The names of the devices
> - are not important. The properties of these nodes are defined by the
> - individual bindings for the rpmsg devices.
> - properties:
> - mediatek,rpmsg-name:
> - $ref: /schemas/types.yaml#/definitions/string-array
> - description:
> - Contains the name for the rpmsg device. Used to match
> - the subnode to rpmsg device announced by SCP.
> -
> - required:
> - - mediatek,rpmsg-name
> +additionalProperties: false
>
> examples:
> - |
> @@ -118,7 +121,7 @@ examples:
> clocks = <&infracfg CLK_INFRA_SCPSYS>;
> clock-names = "main";
>
> - cros_ec {
> + cros-ec-rpmsg {
> mediatek,rpmsg-name = "cros-ec-rpmsg";
> };
> };
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> index b4b86bb1f1a7..693ad5f2a82e 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
DTS changes should be in separate patch.
Regards,
Peng.
> @@ -816,7 +816,7 @@
> pinctrl-names = "default";
> pinctrl-0 = <&scp_pins>;
>
> - cros_ec {
> + cros-ec-rpmsg {
> compatible = "google,cros-ec-rpmsg";
> mediatek,rpmsg-name = "cros-ec-rpmsg";
> };
On Tue, 2022-09-27 at 13:03 +0200, AngeloGioacchino Del Regno wrote:
> Il 27/09/22 04:55, Tinghan Shen ha scritto:
> > The video codec doesn't need to explicitly boot SCP in its flow
> > because the SCP remote processor enables auto boot.
> >
> > The redundant usage of rproc_boot increases the number of rproc.power
> > over 1 and prevents successfully shutting down SCP by rproc_shutdown.
> >
> > Signed-off-by: Tinghan Shen <[email protected]>
>
> You should Cc stable on this commit, as it's a quite important fix.
>
> Regards,
> Angelo
>
Hi Angelo,
Ok, I'll add it at next version.
Thanks,
TingHan
On 9/28/2022 4:27 PM, TingHan Shen (沈廷翰) wrote:
> On Tue, 2022-09-27 at 13:03 +0200, AngeloGioacchino Del Regno wrote:
>> Il 27/09/22 04:55, Tinghan Shen ha scritto:
>>> The video codec doesn't need to explicitly boot SCP in its flow
>>> because the SCP remote processor enables auto boot.
>>>
>>> The redundant usage of rproc_boot increases the number of rproc.power
>>> over 1 and prevents successfully shutting down SCP by rproc_shutdown.
>>>
>>> Signed-off-by: Tinghan Shen <[email protected]>
>>
>> You should Cc stable on this commit, as it's a quite important fix.
>>
>> Regards,
>> Angelo
>>
> Hi Angelo,
>
> Ok, I'll add it at next version.
If this patch is not relevant with the SCP 1 support in this patchset,
better separate this patch out as a standalone fix.
Regards,
Peng.
>
> Thanks,
> TingHan
On Wed, 2022-09-28 at 17:40 +0800, Peng Fan wrote:
>
> On 9/28/2022 4:27 PM, TingHan Shen (沈廷翰) wrote:
> > On Tue, 2022-09-27 at 13:03 +0200, AngeloGioacchino Del Regno wrote:
> > > Il 27/09/22 04:55, Tinghan Shen ha scritto:
> > > > The video codec doesn't need to explicitly boot SCP in its flow
> > > > because the SCP remote processor enables auto boot.
> > > >
> > > > The redundant usage of rproc_boot increases the number of rproc.power
> > > > over 1 and prevents successfully shutting down SCP by rproc_shutdown.
> > > >
> > > > Signed-off-by: Tinghan Shen <[email protected]>
> > >
> > > You should Cc stable on this commit, as it's a quite important fix.
> > >
> > > Regards,
> > > Angelo
> > >
> >
> > Hi Angelo,
> >
> > Ok, I'll add it at next version.
>
> If this patch is not relevant with the SCP 1 support in this patchset,
> better separate this patch out as a standalone fix.
Ok, I'll send it separately.
Regards,
TingHan
>
> Regards,
> Peng.
>
> >
> > Thanks,
> > TingHan
On 27/09/2022 04:55, Tinghan Shen wrote:
> The node name doesn't matter to add the subnode as a cros-ec rpmsg device.
> Give it a clear persistent node name to simplify scp yaml.
>
> Signed-off-by: Tinghan Shen <[email protected]>
> ---
> .../bindings/remoteproc/mtk,scp.yaml | 35 ++++++++++---------
> .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 2 +-
> 2 files changed, 20 insertions(+), 17 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> index 7e091eaffc18..786bed897916 100644
> --- a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> @@ -58,6 +58,23 @@ properties:
> memory-region:
> maxItems: 1
>
> + cros-ec-rpmsg:
> + type: object
additionalProperties: false on this level
> + description:
> + This subnode represents the rpmsg device. The names of the devices
What are the devices? You wrote that it is one device, not devices.
> + are not important. The properties of this node are defined by the
> + individual bindings for the rpmsg devices.
??? No, you need to define the properties of the node, e.g. by a ref.
> +
> + properties:
> + mediatek,rpmsg-name:
> + $ref: /schemas/types.yaml#/definitions/string-array
> + description:
> + Contains the name for the rpmsg device. Used to match
> + the subnode to rpmsg device announced by SCP.
maxItems... but is it really a string-array?
> +
> + required:
> + - mediatek,rpmsg-name
> +
> required:
> - compatible
> - reg
> @@ -89,21 +106,7 @@ allOf:
> reg-names:
> maxItems: 2
>
Best regards,
Krzysztof
Good day,
A lot of comments related to the handling of SCP 0 and 1 have already been made
on this patchset, along with my own advice from the previous patchset on how to
move forward. As such I will wait for a new revision.
Thanks,
Mathieu
On Tue, Sep 27, 2022 at 10:55:55AM +0800, Tinghan Shen wrote:
> The mediatek remoteproc driver currently only allows bringing up a
> single core SCP, e.g. MT8183. It also only bringing up the 1st
> core in SoCs with a dual-core SCP, e.g. MT8195. This series support
> to bring-up the 2nd core of the dual-core SCP.
>
> v2 -> v3:
> 1. change the representation of dual-core SCP in dts file and update SCP yaml
> 2. rewrite SCP driver to reflect the change of dts node
> 3. add SCP core 1 node to mt8195.dtsi
> 4. remove redundant call of rproc_boot for SCP
> 5. refine IPI error message
>
> v1 -> v2:
> 1. update dt-binding property description
> 2. remove kconfig for scp dual driver
> 3. merge mtk_scp_dual.c and mtk_scp_subdev.c to mtk_scp.c
>
> Tinghan Shen (11):
> dt-bindings: remoteproc: mediatek: Give the subnode a persistent name
> dt-bindings: remoteproc: mediatek: Support MT8195 dual-core SCP
> arm64: dts: mt8195: Add SCP core 1 node
> remoteproc: mediatek: Remove redundant rproc_boot
> remoteproc: mediatek: Add SCP core 1 register definitions
> remoteproc: mediatek: Add MT8195 SCP core 1 operations
> remoteproc: mediatek: Probe MT8195 SCP core 1
> remoteproc: mediatek: Control SCP core 1 boot by rproc subdevice
> remoteproc: mediatek: Setup MT8195 SCP core 1 SRAM offset
> remoteproc: mediatek: Handle MT8195 SCP core 1 watchdog timeout
> remoteproc: mediatek: Refine ipi handler error message
>
> .../bindings/remoteproc/mtk,scp.yaml | 132 ++++++++--
> .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 2 +-
> arch/arm64/boot/dts/mediatek/mt8195.dtsi | 14 +-
> .../mediatek/vcodec/mtk_vcodec_fw_scp.c | 2 +-
> drivers/remoteproc/mtk_common.h | 35 +++
> drivers/remoteproc/mtk_scp.c | 241 +++++++++++++++++-
> include/linux/remoteproc/mtk_scp.h | 1 +
> 7 files changed, 397 insertions(+), 30 deletions(-)
>
> --
> 2.18.0
>
On Tue, 2022-11-01 at 14:40 -0600, Mathieu Poirier wrote:
> Good day,
>
> A lot of comments related to the handling of SCP 0 and 1 have already been made
> on this patchset, along with my own advice from the previous patchset on how to
> move forward. As such I will wait for a new revision.
>
> Thanks,
> Mathieu
Sorry for late response.
I'll update the series based on all of your comments.
Thank you!
>
> On Tue, Sep 27, 2022 at 10:55:55AM +0800, Tinghan Shen wrote:
> > The mediatek remoteproc driver currently only allows bringing up a
> > single core SCP, e.g. MT8183. It also only bringing up the 1st
> > core in SoCs with a dual-core SCP, e.g. MT8195. This series support
> > to bring-up the 2nd core of the dual-core SCP.
> >
> > v2 -> v3:
> > 1. change the representation of dual-core SCP in dts file and update SCP yaml
> > 2. rewrite SCP driver to reflect the change of dts node
> > 3. add SCP core 1 node to mt8195.dtsi
> > 4. remove redundant call of rproc_boot for SCP
> > 5. refine IPI error message
> >
> > v1 -> v2:
> > 1. update dt-binding property description
> > 2. remove kconfig for scp dual driver
> > 3. merge mtk_scp_dual.c and mtk_scp_subdev.c to mtk_scp.c
> >
> > Tinghan Shen (11):
> > dt-bindings: remoteproc: mediatek: Give the subnode a persistent name
> > dt-bindings: remoteproc: mediatek: Support MT8195 dual-core SCP
> > arm64: dts: mt8195: Add SCP core 1 node
> > remoteproc: mediatek: Remove redundant rproc_boot
> > remoteproc: mediatek: Add SCP core 1 register definitions
> > remoteproc: mediatek: Add MT8195 SCP core 1 operations
> > remoteproc: mediatek: Probe MT8195 SCP core 1
> > remoteproc: mediatek: Control SCP core 1 boot by rproc subdevice
> > remoteproc: mediatek: Setup MT8195 SCP core 1 SRAM offset
> > remoteproc: mediatek: Handle MT8195 SCP core 1 watchdog timeout
> > remoteproc: mediatek: Refine ipi handler error message
> >
> > .../bindings/remoteproc/mtk,scp.yaml | 132 ++++++++--
> > .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 2 +-
> > arch/arm64/boot/dts/mediatek/mt8195.dtsi | 14 +-
> > .../mediatek/vcodec/mtk_vcodec_fw_scp.c | 2 +-
> > drivers/remoteproc/mtk_common.h | 35 +++
> > drivers/remoteproc/mtk_scp.c | 241 +++++++++++++++++-
> > include/linux/remoteproc/mtk_scp.h | 1 +
> > 7 files changed, 397 insertions(+), 30 deletions(-)
> >
> > --
> > 2.18.0
> >
--
Best regards,
TingHan