2023-05-17 04:37:51

by Tinghan Shen

[permalink] [raw]
Subject: [PATCH v12 00/11] Add support for MT8195 SCP 2nd core

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.

v11 -> v12:
1. add scp_add_single/multi_core() to patchset 6
2. remove unused comment in patchset 6
3. rename list name from mtk_scp_cluster to mtk_scp_list
4. rewrite the multi-core probe flow
5. disable rproc->autoboot and boot rproc by request_firmware_nowait at patchset 7
6. remove patchset 7 review tag

v10 -> v11:
1. rewrite patchset 5 to probe single-core SCP with the cluster list
2. Also in patchset 5, move the pointer of mtk_scp object from the
platform data property to the driver data property
3. move the appearance of mtk_scp cluster property to patcheset 7

v9 -> v10:
1. move the global mtk_scp list into the platform device driver data structure
2. remove an unnecessary if() condition

v8 -> v9:
1. initialize l1tcm_size/l1tcm_phys at patchset 05/11
2. rewrite patchset 06/11 to unify the flow and remove hacks

v7 -> v8:
1. update the node name of mt8192 asurada SCP rpmsg subnode
2. squash register definitions into driver patches
3. initialize local variables on the declaration at patch v8 06/11

v6 -> v7:
1. merge the mtk_scp_cluster struct into the mtk_scp structure
at the "Probe multi-core SCP" patch

v5 -> v6:
1. move the mtk_scp_of_regs structure from mtk_common.h to mtk_scp.c
2. rename the SCP core 0 label from 'scp' to 'scp_c0'

v4 -> v5:
1. move resource release actions to the platform driver remove operation
2. fix dual-core watchdog handling

v3 -> v4:
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. drop 'remove redundant call of rproc_boot for SCP' in v3 for further investigation

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: Improve the rpmsg subnode
definition
arm64: dts: mediatek: Update the node name of SCP rpmsg subnode
dt-bindings: remoteproc: mediatek: Support MT8195 dual-core SCP
remoteproc: mediatek: Add MT8195 SCP core 1 operations
remoteproc: mediatek: Introduce cluster on single-core SCP
remoteproc: mediatek: Probe multi-core SCP
remoteproc: mediatek: Control SCP core 1 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
arm64: dts: mediatek: mt8195: Add SCP 2nd core

.../bindings/remoteproc/mtk,scp.yaml | 176 ++++++-
.../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 2 +-
.../boot/dts/mediatek/mt8192-asurada.dtsi | 2 +-
.../boot/dts/mediatek/mt8195-cherry.dtsi | 6 +-
arch/arm64/boot/dts/mediatek/mt8195.dtsi | 32 +-
drivers/remoteproc/mtk_common.h | 32 ++
drivers/remoteproc/mtk_scp.c | 456 ++++++++++++++++--
7 files changed, 631 insertions(+), 75 deletions(-)

--
2.18.0



2023-05-17 04:39:35

by Tinghan Shen

[permalink] [raw]
Subject: [PATCH v12 02/11] arm64: dts: mediatek: Update the node name of SCP rpmsg subnode

Align the node name with the definition in SCP bindings.

Signed-off-by: Tinghan Shen <[email protected]>
Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
Acked-by: Matthias Brugger <[email protected]>
---
arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 2 +-
arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
index 63952c1251df..ac4aeeded37c 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
@@ -825,7 +825,7 @@
pinctrl-names = "default";
pinctrl-0 = <&scp_pins>;

- cros_ec {
+ cros-ec-rpmsg {
compatible = "google,cros-ec-rpmsg";
mediatek,rpmsg-name = "cros-ec-rpmsg";
};
diff --git a/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi b/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
index 5a440504d4f9..cd4d68f1d93b 100644
--- a/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
@@ -1280,7 +1280,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


2023-05-17 04:39:44

by Tinghan Shen

[permalink] [raw]
Subject: [PATCH v12 05/11] remoteproc: mediatek: Introduce cluster on single-core SCP

This is the preliminary step for probing multi-core SCP.
The initialization procedure for remoteproc is similar for both
single-core and multi-core architectures and is reusing to avoid
redundant code.

Rewrite the probing flow of single-core SCP to adapt with the 'cluster'
concept needed by probing the multi-core SCP. The main differences
are,
- the SCP core object(s) is maintained at the cluster list instead of at
the platofmr device driver data property.
- save the cluster information at the platofmr device driver data property.
- In order to keep the compatibility of exported SCP APIs which getting
the SCP core object by SCP node phandle, move the SCP core object
pointers to the platform device platform data property.

The registers of config and l1tcm are shared for multi-core
SCP. Reuse the mapped addresses for all cores.

Signed-off-by: Tinghan Shen <[email protected]>
---
drivers/remoteproc/mtk_common.h | 2 +
drivers/remoteproc/mtk_scp.c | 151 +++++++++++++++++++++++---------
2 files changed, 112 insertions(+), 41 deletions(-)

diff --git a/drivers/remoteproc/mtk_common.h b/drivers/remoteproc/mtk_common.h
index c0905aec3b4b..56395e8664cb 100644
--- a/drivers/remoteproc/mtk_common.h
+++ b/drivers/remoteproc/mtk_common.h
@@ -128,6 +128,8 @@ struct mtk_scp {
size_t dram_size;

struct rproc_subdev *rpmsg_subdev;
+
+ struct list_head elem;
};

/**
diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
index d66822dad943..c8fc6b46f82b 100644
--- a/drivers/remoteproc/mtk_scp.c
+++ b/drivers/remoteproc/mtk_scp.c
@@ -23,6 +23,14 @@
#define MAX_CODE_SIZE 0x500000
#define SECTION_NAME_IPI_BUFFER ".ipi_buffer"

+struct mtk_scp_of_cluster {
+ void __iomem *reg_base;
+ void __iomem *l1tcm_base;
+ size_t l1tcm_size;
+ phys_addr_t l1tcm_phys;
+ struct list_head mtk_scp_list;
+};
+
/**
* scp_get() - get a reference to SCP.
*
@@ -51,7 +59,7 @@ struct mtk_scp *scp_get(struct platform_device *pdev)
return NULL;
}

- return platform_get_drvdata(scp_pdev);
+ return *(struct mtk_scp **)dev_get_platdata(&scp_pdev->dev);
}
EXPORT_SYMBOL_GPL(scp_get);

@@ -810,14 +818,14 @@ static void scp_unmap_memory_region(struct mtk_scp *scp)
static int scp_register_ipi(struct platform_device *pdev, u32 id,
ipi_handler_t handler, void *priv)
{
- struct mtk_scp *scp = platform_get_drvdata(pdev);
+ struct mtk_scp *scp = *(struct mtk_scp **)dev_get_platdata(&pdev->dev);

return scp_ipi_register(scp, id, handler, priv);
}

static void scp_unregister_ipi(struct platform_device *pdev, u32 id)
{
- struct mtk_scp *scp = platform_get_drvdata(pdev);
+ struct mtk_scp *scp = *(struct mtk_scp **)dev_get_platdata(&pdev->dev);

scp_ipi_unregister(scp, id);
}
@@ -825,7 +833,7 @@ static void scp_unregister_ipi(struct platform_device *pdev, u32 id)
static int scp_send_ipi(struct platform_device *pdev, u32 id, void *buf,
unsigned int len, unsigned int wait)
{
- struct mtk_scp *scp = platform_get_drvdata(pdev);
+ struct mtk_scp *scp = *(struct mtk_scp **)dev_get_platdata(&pdev->dev);

return scp_ipi_send(scp, id, buf, len, wait);
}
@@ -855,7 +863,8 @@ static void scp_remove_rpmsg_subdev(struct mtk_scp *scp)
}
}

-static int scp_probe(struct platform_device *pdev)
+static struct mtk_scp *scp_rproc_init(struct platform_device *pdev,
+ struct mtk_scp_of_cluster *scp_cluster)
{
struct device *dev = &pdev->dev;
struct device_node *np = dev->of_node;
@@ -867,52 +876,42 @@ static int scp_probe(struct platform_device *pdev)

ret = rproc_of_parse_firmware(dev, 0, &fw_name);
if (ret < 0 && ret != -EINVAL)
- return ret;
+ return ERR_PTR(ret);

rproc = devm_rproc_alloc(dev, np->name, &scp_ops, fw_name, sizeof(*scp));
- if (!rproc)
- return dev_err_probe(dev, -ENOMEM, "unable to allocate remoteproc\n");
+ if (!rproc) {
+ dev_err(dev, "unable to allocate remoteproc\n");
+ return ERR_PTR(-ENOMEM);
+ }

scp = rproc->priv;
scp->rproc = rproc;
scp->dev = dev;
scp->data = of_device_get_match_data(dev);
- platform_set_drvdata(pdev, scp);
+ platform_device_add_data(pdev, &scp, sizeof(scp));
+
+ scp->reg_base = scp_cluster->reg_base;
+ scp->l1tcm_base = scp_cluster->l1tcm_base;
+ scp->l1tcm_size = scp_cluster->l1tcm_size;
+ scp->l1tcm_phys = scp_cluster->l1tcm_phys;

res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "sram");
scp->sram_base = devm_ioremap_resource(dev, res);
- if (IS_ERR(scp->sram_base))
- return dev_err_probe(dev, PTR_ERR(scp->sram_base),
- "Failed to parse and map sram memory\n");
+ if (IS_ERR(scp->sram_base)) {
+ dev_err(dev, "Failed to parse and map sram memory\n");
+ return ERR_PTR(PTR_ERR(scp->sram_base));
+ }

scp->sram_size = resource_size(res);
scp->sram_phys = res->start;

- /* l1tcm is an optional memory region */
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "l1tcm");
- scp->l1tcm_base = devm_ioremap_resource(dev, res);
- if (IS_ERR(scp->l1tcm_base)) {
- ret = PTR_ERR(scp->l1tcm_base);
- if (ret != -EINVAL) {
- return dev_err_probe(dev, ret, "Failed to map l1tcm memory\n");
- }
- } else {
- scp->l1tcm_size = resource_size(res);
- 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");
-
ret = scp->data->scp_clk_get(scp);
if (ret)
- return ret;
+ return ERR_PTR(ret);

ret = scp_map_memory_region(scp);
if (ret)
- return ret;
+ return ERR_PTR(ret);

mutex_init(&scp->send_lock);
for (i = 0; i < SCP_IPI_MAX; i++)
@@ -939,11 +938,7 @@ static int scp_probe(struct platform_device *pdev)
goto remove_subdev;
}

- ret = rproc_add(rproc);
- if (ret)
- goto remove_subdev;
-
- return 0;
+ return scp;

remove_subdev:
scp_remove_rpmsg_subdev(scp);
@@ -954,15 +949,13 @@ static int scp_probe(struct platform_device *pdev)
mutex_destroy(&scp->ipi_desc[i].lock);
mutex_destroy(&scp->send_lock);

- return ret;
+ return ERR_PTR(ret);
}

-static void scp_remove(struct platform_device *pdev)
+static void scp_free(struct mtk_scp *scp)
{
- struct mtk_scp *scp = platform_get_drvdata(pdev);
int i;

- rproc_del(scp->rproc);
scp_remove_rpmsg_subdev(scp);
scp_ipi_unregister(scp, SCP_IPI_INIT);
scp_unmap_memory_region(scp);
@@ -971,6 +964,82 @@ static void scp_remove(struct platform_device *pdev)
mutex_destroy(&scp->send_lock);
}

+static int scp_cluster_init(struct platform_device *pdev)
+{
+ struct mtk_scp_of_cluster *scp_cluster = platform_get_drvdata(pdev);
+ struct list_head *cluster = &scp_cluster->mtk_scp_list;
+ struct mtk_scp *scp;
+ int ret;
+
+ scp = scp_rproc_init(pdev, scp_cluster);
+ if (IS_ERR(scp))
+ return PTR_ERR(scp);
+
+ ret = rproc_add(scp->rproc);
+ if (ret) {
+ dev_err(dev, "Failed to add rproc\n");
+ scp_free(scp);
+ return ret;
+ }
+
+ list_add_tail(&scp->elem, cluster);
+
+ return 0;
+}
+
+static int scp_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct mtk_scp_of_cluster *scp_cluster;
+ struct resource *res;
+ int ret;
+
+ scp_cluster = devm_kzalloc(dev, sizeof(*scp_cluster), GFP_KERNEL);
+ if (!scp_cluster)
+ return -ENOMEM;
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cfg");
+ scp_cluster->reg_base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(scp_cluster->reg_base))
+ return dev_err_probe(dev, PTR_ERR(scp_cluster->reg_base),
+ "Failed to parse and map cfg memory\n");
+
+ /* l1tcm is an optional memory region */
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "l1tcm");
+ scp_cluster->l1tcm_base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(scp_cluster->l1tcm_base)) {
+ ret = PTR_ERR(scp_cluster->l1tcm_base);
+ if (ret != -EINVAL)
+ return dev_err_probe(dev, ret, "Failed to map l1tcm memory\n");
+
+ scp_cluster->l1tcm_base = NULL;
+ } else {
+ scp_cluster->l1tcm_size = resource_size(res);
+ scp_cluster->l1tcm_phys = res->start;
+ }
+
+ INIT_LIST_HEAD(&scp_cluster->mtk_scp_list);
+ platform_set_drvdata(pdev, scp_cluster);
+
+ ret = scp_cluster_init(pdev);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static void scp_remove(struct platform_device *pdev)
+{
+ struct mtk_scp_of_cluster *scp_cluster = platform_get_drvdata(pdev);
+ struct mtk_scp *scp, *temp;
+
+ list_for_each_entry_safe_reverse(scp, temp, &scp_cluster->mtk_scp_list, elem) {
+ list_del(&scp->elem);
+ rproc_del(scp->rproc);
+ scp_free(scp);
+ }
+}
+
static const struct mtk_scp_of_data mt8183_of_data = {
.scp_clk_get = mt8183_scp_clk_get,
.scp_before_load = mt8183_scp_before_load,
--
2.18.0


2023-05-17 04:40:01

by Tinghan Shen

[permalink] [raw]
Subject: [PATCH v12 03/11] dt-bindings: remoteproc: mediatek: Support MT8195 dual-core SCP

Extend the SCP binding to describe the MT8195 dual-core SCP.

Under different applications, the MT8195 SCP can be used as single-core
or dual-core. This change keeps the single-core definitions and
adds new definitions for the dual-core use case.

Signed-off-by: Tinghan Shen <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
---
.../bindings/remoteproc/mtk,scp.yaml | 145 +++++++++++++++++-
1 file changed, 141 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
index 271081df0e46..09102dda4942 100644
--- a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
@@ -21,6 +21,7 @@ properties:
- mediatek,mt8188-scp
- mediatek,mt8192-scp
- mediatek,mt8195-scp
+ - mediatek,mt8195-scp-dual

reg:
description:
@@ -31,10 +32,7 @@ properties:

reg-names:
minItems: 2
- items:
- - const: sram
- - const: cfg
- - const: l1tcm
+ maxItems: 3

clocks:
description:
@@ -70,6 +68,81 @@ properties:

unevaluatedProperties: false

+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 1
+
+ ranges:
+ description:
+ Standard ranges definition providing address translations for
+ local SCP SRAM address spaces to bus addresses.
+
+patternProperties:
+ "^scp@[a-f0-9]+$":
+ type: object
+ description:
+ The MediaTek SCP integrated to SoC might be a multi-core version.
+ The other cores are represented as child nodes of the boot core.
+ There are some integration differences for the IP like the usage of
+ address translator for translating SoC bus addresses into address space
+ for the processor.
+
+ Each SCP core has own cache memory. The SRAM and L1TCM are shared by
+ cores. The power of cache, SRAM and L1TCM power should be enabled
+ before booting SCP cores. The size of cache, SRAM, and L1TCM are varied
+ on differnt SoCs.
+
+ The SCP cores do not use an MMU, but has a set of registers to
+ control the translations between 32-bit CPU addresses into system bus
+ addresses. Cache and memory access settings are provided through a
+ Memory Protection Unit (MPU), programmable only from the SCP.
+
+ properties:
+ compatible:
+ enum:
+ - mediatek,scp-core
+
+ reg:
+ description: The base address and size of SRAM.
+ maxItems: 1
+
+ reg-names:
+ const: sram
+
+ interrupts:
+ maxItems: 1
+
+ firmware-name:
+ $ref: /schemas/types.yaml#/definitions/string
+ description:
+ If present, name (or relative path) of the file within the
+ firmware search path containing the firmware image used when
+ initializing sub cores of multi-core SCP.
+
+ memory-region:
+ maxItems: 1
+
+ cros-ec-rpmsg:
+ $ref: /schemas/mfd/google,cros-ec.yaml
+ description:
+ This subnode represents the rpmsg device. The properties
+ of this node are defined by the individual bindings for
+ the rpmsg devices.
+
+ required:
+ - mediatek,rpmsg-name
+
+ unevaluatedProperties: false
+
+ required:
+ - compatible
+ - reg
+ - reg-names
+
+ additionalProperties: false
+
required:
- compatible
- reg
@@ -99,7 +172,37 @@ allOf:
reg:
maxItems: 2
reg-names:
+ items:
+ - const: sram
+ - const: cfg
+ - if:
+ properties:
+ compatible:
+ enum:
+ - mediatek,mt8192-scp
+ - mediatek,mt8195-scp
+ then:
+ properties:
+ reg:
+ maxItems: 3
+ reg-names:
+ items:
+ - const: sram
+ - const: cfg
+ - const: l1tcm
+ - if:
+ properties:
+ compatible:
+ enum:
+ - mediatek,mt8195-scp-dual
+ then:
+ properties:
+ reg:
maxItems: 2
+ reg-names:
+ items:
+ - const: cfg
+ - const: l1tcm

additionalProperties: false

@@ -121,3 +224,37 @@ examples:
mediatek,rpmsg-name = "cros-ec-rpmsg";
};
};
+
+ - |
+ scp@10500000 {
+ compatible = "mediatek,mt8195-scp-dual";
+ reg = <0x10720000 0xe0000>,
+ <0x10700000 0x8000>;
+ reg-names = "cfg", "l1tcm";
+
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0x10500000 0x100000>;
+
+ scp@0 {
+ compatible = "mediatek,scp-core";
+ reg = <0x0 0xa0000>;
+ reg-names = "sram";
+
+ cros-ec-rpmsg {
+ compatible = "google,cros-ec-rpmsg";
+ mediatek,rpmsg-name = "cros-ec-rpmsg";
+ };
+ };
+
+ scp@a0000 {
+ compatible = "mediatek,scp-core";
+ reg = <0xa0000 0x20000>;
+ reg-names = "sram";
+
+ cros-ec-rpmsg {
+ compatible = "google,cros-ec-rpmsg";
+ mediatek,rpmsg-name = "cros-ec-rpmsg";
+ };
+ };
+ };
--
2.18.0


2023-05-17 04:40:23

by Tinghan Shen

[permalink] [raw]
Subject: [PATCH v12 06/11] remoteproc: mediatek: Probe multi-core SCP

The difference of single-core SCP and multi-core SCP device tree is
the presence of child device nodes described SCP cores. The SCP
driver populates the platform device and checks the child nodes
to identify whether it's a single-core SCP or a multi-core SCP.

Add the remoteproc instances of multi-core SCP to the SCP cluster list.
When the SCP driver is removed, it cleanup resources by walking
through the cluster list.

Signed-off-by: Tinghan Shen <[email protected]>
---
drivers/remoteproc/mtk_scp.c | 115 +++++++++++++++++++++++++++++++++--
1 file changed, 111 insertions(+), 4 deletions(-)

diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
index c8fc6b46f82b..d644e232dfec 100644
--- a/drivers/remoteproc/mtk_scp.c
+++ b/drivers/remoteproc/mtk_scp.c
@@ -864,7 +864,8 @@ static void scp_remove_rpmsg_subdev(struct mtk_scp *scp)
}

static struct mtk_scp *scp_rproc_init(struct platform_device *pdev,
- struct mtk_scp_of_cluster *scp_cluster)
+ struct mtk_scp_of_cluster *scp_cluster,
+ const struct mtk_scp_of_data *of_data)
{
struct device *dev = &pdev->dev;
struct device_node *np = dev->of_node;
@@ -887,7 +888,7 @@ static struct mtk_scp *scp_rproc_init(struct platform_device *pdev,
scp = rproc->priv;
scp->rproc = rproc;
scp->dev = dev;
- scp->data = of_device_get_match_data(dev);
+ scp->data = of_data;
platform_device_add_data(pdev, &scp, sizeof(scp));

scp->reg_base = scp_cluster->reg_base;
@@ -964,14 +965,15 @@ static void scp_free(struct mtk_scp *scp)
mutex_destroy(&scp->send_lock);
}

-static int scp_cluster_init(struct platform_device *pdev)
+static int scp_add_single_core(struct platform_device *pdev)
{
+ struct device *dev = &pdev->dev;
struct mtk_scp_of_cluster *scp_cluster = platform_get_drvdata(pdev);
struct list_head *cluster = &scp_cluster->mtk_scp_list;
struct mtk_scp *scp;
int ret;

- scp = scp_rproc_init(pdev, scp_cluster);
+ scp = scp_rproc_init(pdev, scp_cluster, of_device_get_match_data(dev));
if (IS_ERR(scp))
return PTR_ERR(scp);

@@ -987,6 +989,100 @@ static int scp_cluster_init(struct platform_device *pdev)
return 0;
}

+static int scp_add_multi_core(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev_of_node(dev);
+ struct platform_device *cpdev;
+ struct device_node *child;
+ struct mtk_scp_of_cluster *scp_cluster = platform_get_drvdata(pdev);
+ struct list_head *cluster = &scp_cluster->mtk_scp_list;
+ const struct mtk_scp_of_data **cluster_of_data;
+ struct mtk_scp *scp, *temp;
+ int core_id = 0;
+ int ret;
+
+ cluster_of_data = (const struct mtk_scp_of_data **)of_device_get_match_data(dev);
+
+ for_each_available_child_of_node(np, child) {
+ if (!cluster_of_data[core_id]) {
+ ret = -EINVAL;
+ dev_err(dev, "Not support core %d\n", core_id);
+ of_node_put(child);
+ goto init_fail;
+ }
+
+ cpdev = of_find_device_by_node(child);
+ if (!cpdev) {
+ ret = -ENODEV;
+ dev_err(dev, "Not found platform device for core %d\n", core_id);
+ of_node_put(child);
+ goto init_fail;
+ }
+
+ scp = scp_rproc_init(cpdev, scp_cluster, cluster_of_data[core_id]);
+ put_device(&cpdev->dev);
+ if (IS_ERR(scp)) {
+ ret = PTR_ERR(scp);
+ dev_err(dev, "Failed to initialize core %d rproc\n", core_id);
+ of_node_put(child);
+ goto init_fail;
+ }
+
+ ret = rproc_add(scp->rproc);
+ if (ret) {
+ dev_err(dev, "Failed to add rproc of core %d\n", core_id);
+ of_node_put(child);
+ scp_free(scp);
+ goto init_fail;
+ }
+
+ list_add_tail(&scp->elem, cluster);
+ core_id++;
+ }
+
+ return 0;
+
+init_fail:
+ list_for_each_entry_safe_reverse(scp, temp, cluster, elem) {
+ list_del(&scp->elem);
+ rproc_del(scp->rproc);
+ scp_free(scp);
+ }
+
+ return ret;
+}
+
+static int scp_is_single_core(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev_of_node(dev);
+ struct device_node *child;
+
+ child = of_get_next_available_child(np, NULL);
+ if (!child)
+ return dev_err_probe(dev, -ENODEV, "No child node\n");
+
+ of_node_put(child);
+ return of_node_name_eq(child, "cros-ec-rpmsg");
+}
+
+static int scp_cluster_init(struct platform_device *pdev)
+{
+ int ret;
+
+ ret = scp_is_single_core(pdev);
+ if (ret < 0)
+ return ret;
+
+ if (ret)
+ ret = scp_add_single_core(pdev);
+ else
+ ret = scp_add_multi_core(pdev);
+
+ return ret;
+}
+
static int scp_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -1021,6 +1117,10 @@ static int scp_probe(struct platform_device *pdev)
INIT_LIST_HEAD(&scp_cluster->mtk_scp_list);
platform_set_drvdata(pdev, scp_cluster);

+ ret = devm_of_platform_populate(dev);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to populate platform devices\n");
+
ret = scp_cluster_init(pdev);
if (ret)
return ret;
@@ -1114,12 +1214,19 @@ static const struct mtk_scp_of_data mt8195_of_data_c1 = {
.host_to_scp_int_bit = MT8195_CORE1_HOST_IPC_INT_BIT,
};

+static const struct mtk_scp_of_data *mt8195_of_data_cores[] = {
+ &mt8195_of_data,
+ &mt8195_of_data_c1,
+ NULL
+};
+
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-dual", .data = &mt8195_of_data_cores },
{},
};
MODULE_DEVICE_TABLE(of, mtk_scp_of_match);
--
2.18.0


2023-05-17 04:40:50

by Tinghan Shen

[permalink] [raw]
Subject: [PATCH v12 10/11] remoteproc: mediatek: Refine ipi handler error message

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]>
---
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 6319ba21bcad..706190d2db75 100644
--- a/drivers/remoteproc/mtk_scp.c
+++ b/drivers/remoteproc/mtk_scp.c
@@ -114,7 +114,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


2023-05-17 04:42:05

by Tinghan Shen

[permalink] [raw]
Subject: [PATCH v12 07/11] remoteproc: mediatek: Control SCP core 1 by rproc subdevice

Register SCP core 1 as a subdevice of core 0 for the boot sequence
and watchdog timeout handling. The core 1 has to boot after core 0
because the SCP clock and SRAM power is controlled by SCP core 0.
As for watchdog timeout handling, the remoteproc framework helps to
stop/start subdevices automatically when SCP driver receives watchdog
timeout event.

Signed-off-by: Tinghan Shen <[email protected]>
Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/remoteproc/mtk_common.h | 9 ++++
drivers/remoteproc/mtk_scp.c | 88 +++++++++++++++++++++++++++++++++
2 files changed, 97 insertions(+)

diff --git a/drivers/remoteproc/mtk_common.h b/drivers/remoteproc/mtk_common.h
index 56395e8664cb..85afed2e6928 100644
--- a/drivers/remoteproc/mtk_common.h
+++ b/drivers/remoteproc/mtk_common.h
@@ -100,6 +100,13 @@ struct mtk_scp_of_data {
size_t ipi_buf_offset;
};

+struct mtk_scp_core_subdev {
+ struct rproc_subdev subdev;
+ struct mtk_scp *scp;
+};
+
+#define to_core_subdev(d) container_of(d, struct mtk_scp_core_subdev, subdev)
+
struct mtk_scp {
struct device *dev;
struct rproc *rproc;
@@ -130,6 +137,8 @@ struct mtk_scp {
struct rproc_subdev *rpmsg_subdev;

struct list_head elem;
+ struct list_head *cluster;
+ struct mtk_scp_core_subdev *core_subdev;
};

/**
diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
index d644e232dfec..18534b1179b5 100644
--- a/drivers/remoteproc/mtk_scp.c
+++ b/drivers/remoteproc/mtk_scp.c
@@ -863,6 +863,60 @@ static void scp_remove_rpmsg_subdev(struct mtk_scp *scp)
}
}

+static int scp_core_subdev_start(struct rproc_subdev *subdev)
+{
+ struct mtk_scp_core_subdev *core_subdev = to_core_subdev(subdev);
+ struct mtk_scp *scp = core_subdev->scp;
+
+ rproc_boot(scp->rproc);
+
+ return 0;
+}
+
+static void scp_core_subdev_stop(struct rproc_subdev *subdev, bool crashed)
+{
+ struct mtk_scp_core_subdev *core_subdev = to_core_subdev(subdev);
+ struct mtk_scp *scp = core_subdev->scp;
+
+ rproc_shutdown(scp->rproc);
+}
+
+static int scp_core_subdev_register(struct mtk_scp *scp)
+{
+ struct device *dev = scp->dev;
+ struct mtk_scp_core_subdev *core_subdev;
+ struct mtk_scp *scp_c0;
+
+ scp_c0 = list_first_entry(scp->cluster, struct mtk_scp, elem);
+ if (!scp_c0)
+ return -ENODATA;
+
+ core_subdev = devm_kzalloc(dev, sizeof(*core_subdev), GFP_KERNEL);
+ if (!core_subdev)
+ return -ENOMEM;
+
+ core_subdev->scp = scp;
+ core_subdev->subdev.start = scp_core_subdev_start;
+ core_subdev->subdev.stop = scp_core_subdev_stop;
+
+ scp->core_subdev = core_subdev;
+ rproc_add_subdev(scp_c0->rproc, &scp->core_subdev->subdev);
+
+ return 0;
+}
+
+static void scp_core_subdev_unregister(struct mtk_scp *scp)
+{
+ struct mtk_scp *scp_c0;
+
+ if (scp->core_subdev) {
+ scp_c0 = list_first_entry(scp->cluster, struct mtk_scp, elem);
+ rproc_remove_subdev(scp_c0->rproc, &scp->core_subdev->subdev);
+ devm_kfree(scp->dev, scp->core_subdev);
+ scp->core_subdev = NULL;
+ }
+}
+
static struct mtk_scp *scp_rproc_init(struct platform_device *pdev,
struct mtk_scp_of_cluster *scp_cluster,
const struct mtk_scp_of_data *of_data)
@@ -957,6 +1011,7 @@ static void scp_free(struct mtk_scp *scp)
{
int i;

+ scp_core_subdev_unregister(scp);
scp_remove_rpmsg_subdev(scp);
scp_ipi_unregister(scp, SCP_IPI_INIT);
scp_unmap_memory_region(scp);
@@ -989,6 +1044,15 @@ static int scp_add_single_core(struct platform_device *pdev)
return 0;
}

+static void scp_rproc_boot_callback(const struct firmware *fw, void *context)
+{
+ struct rproc *rproc = context;
+
+ rproc_boot(rproc);
+
+ release_firmware(fw);
+}
+
static int scp_add_multi_core(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -1029,6 +1093,20 @@ static int scp_add_multi_core(struct platform_device *pdev)
goto init_fail;
}

+ scp->cluster = cluster;
+ if (!list_empty(cluster)) {
+ ret = scp_core_subdev_register(scp);
+ if (ret) {
+ dev_err(dev, "Failed to register core %d as subdev\n", core_id);
+ of_node_put(child);
+ scp_free(scp);
+ goto init_fail;
+ }
+ }
+
+ /* boot after all cores finished rproc_add() */
+ scp->rproc->auto_boot = false;
+
ret = rproc_add(scp->rproc);
if (ret) {
dev_err(dev, "Failed to add rproc of core %d\n", core_id);
@@ -1041,6 +1119,16 @@ static int scp_add_multi_core(struct platform_device *pdev)
core_id++;
}

+ /* boot core 0, and other cores are booted following core 0 as subdevices */
+ scp = list_first_entry(cluster, struct mtk_scp, elem);
+ ret = request_firmware_nowait(THIS_MODULE, FW_ACTION_UEVENT,
+ scp->rproc->firmware, &scp->rproc->dev, GFP_KERNEL,
+ scp->rproc, scp_rproc_boot_callback);
+ if (ret < 0) {
+ dev_err(dev, "request_firmware_nowait err: %d\n", ret);
+ goto init_fail;
+ }
+
return 0;

init_fail:
--
2.18.0


2023-05-17 04:42:35

by Tinghan Shen

[permalink] [raw]
Subject: [PATCH v12 04/11] remoteproc: mediatek: Add MT8195 SCP core 1 operations

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]>
Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
Reviewed-by: Matthias Brugger <[email protected]>
---
drivers/remoteproc/mtk_common.h | 9 ++++++
drivers/remoteproc/mtk_scp.c | 56 +++++++++++++++++++++++++++++++++
2 files changed, 65 insertions(+)

diff --git a/drivers/remoteproc/mtk_common.h b/drivers/remoteproc/mtk_common.h
index ea6fa1100a00..c0905aec3b4b 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,14 @@

#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_CFG 0x20034
+
#define SCP_FW_VER_LEN 32
#define SCP_SHARE_BUFFER_SIZE 288

diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
index dcc94ee2458d..d66822dad943 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 = rproc->priv;
@@ -989,6 +1033,18 @@ 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_of_data_c1 = {
+ .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 },
--
2.18.0


2023-05-17 04:43:23

by Tinghan Shen

[permalink] [raw]
Subject: [PATCH v12 09/11] remoteproc: mediatek: Handle MT8195 SCP core 1 watchdog timeout

The MT8195 SCP core 1 watchdog timeout needs to be handled in the
SCP core 0 IRQ handler because the MT8195 SCP core 1 watchdog timeout
IRQ is wired on the same IRQ entry for core 0 watchdog timeout.
MT8195 SCP has a watchdog status register to identify the watchdog
timeout source when IRQ triggered.

Signed-off-by: Tinghan Shen <[email protected]>
Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/remoteproc/mtk_common.h | 5 +++++
drivers/remoteproc/mtk_scp.c | 25 ++++++++++++++++++++++++-
2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/remoteproc/mtk_common.h b/drivers/remoteproc/mtk_common.h
index e1cec3109fd9..5ec83ac18503 100644
--- a/drivers/remoteproc/mtk_common.h
+++ b/drivers/remoteproc/mtk_common.h
@@ -55,6 +55,10 @@
#define MT8192_CORE0_WDT_IRQ 0x10030
#define MT8192_CORE0_WDT_CFG 0x10034

+#define MT8195_SYS_STATUS 0x4004
+#define MT8195_CORE0_WDT BIT(16)
+#define MT8195_CORE1_WDT BIT(17)
+
#define MT8195_L1TCM_SRAM_PDN_RESERVED_RSI_BITS GENMASK(7, 4)

#define MT8195_CPU1_SRAM_PD 0x1084
@@ -63,6 +67,7 @@
#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
diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
index 2b6c1725fc13..6319ba21bcad 100644
--- a/drivers/remoteproc/mtk_scp.c
+++ b/drivers/remoteproc/mtk_scp.c
@@ -230,6 +230,29 @@ static void mt8192_scp_irq_handler(struct mtk_scp *scp)
}
}

+static void mt8195_scp_irq_handler(struct mtk_scp *scp)
+{
+ u32 scp_to_host;
+
+ scp_to_host = readl(scp->reg_base + MT8192_SCP2APMCU_IPC_SET);
+
+ if (scp_to_host & MT8192_SCP_IPC_INT_BIT) {
+ scp_ipi_handler(scp);
+ } else {
+ u32 reason = readl(scp->reg_base + MT8195_SYS_STATUS);
+
+ if (reason & MT8195_CORE0_WDT)
+ writel(1, scp->reg_base + MT8192_CORE0_WDT_IRQ);
+
+ if (reason & MT8195_CORE1_WDT)
+ writel(1, scp->reg_base + MT8195_CORE1_WDT_IRQ);
+
+ scp_wdt_handler(scp, reason);
+ }
+
+ writel(scp_to_host, scp->reg_base + MT8192_SCP2APMCU_IPC_CLR);
+}
+
static void mt8195_scp_c1_irq_handler(struct mtk_scp *scp)
{
u32 scp_to_host;
@@ -1306,7 +1329,7 @@ static const struct mtk_scp_of_data mt8192_of_data = {
static const struct mtk_scp_of_data mt8195_of_data = {
.scp_clk_get = mt8195_scp_clk_get,
.scp_before_load = mt8195_scp_before_load,
- .scp_irq_handler = mt8192_scp_irq_handler,
+ .scp_irq_handler = mt8195_scp_irq_handler,
.scp_reset_assert = mt8192_scp_reset_assert,
.scp_reset_deassert = mt8192_scp_reset_deassert,
.scp_stop = mt8195_scp_stop,
--
2.18.0


2023-05-17 17:54:09

by Mathieu Poirier

[permalink] [raw]
Subject: Re: [PATCH v12 06/11] remoteproc: mediatek: Probe multi-core SCP

On Wed, May 17, 2023 at 12:34:44PM +0800, Tinghan Shen wrote:
> The difference of single-core SCP and multi-core SCP device tree is
> the presence of child device nodes described SCP cores. The SCP
> driver populates the platform device and checks the child nodes
> to identify whether it's a single-core SCP or a multi-core SCP.
>
> Add the remoteproc instances of multi-core SCP to the SCP cluster list.
> When the SCP driver is removed, it cleanup resources by walking
> through the cluster list.
>
> Signed-off-by: Tinghan Shen <[email protected]>
> ---
> drivers/remoteproc/mtk_scp.c | 115 +++++++++++++++++++++++++++++++++--
> 1 file changed, 111 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
> index c8fc6b46f82b..d644e232dfec 100644
> --- a/drivers/remoteproc/mtk_scp.c
> +++ b/drivers/remoteproc/mtk_scp.c
> @@ -864,7 +864,8 @@ static void scp_remove_rpmsg_subdev(struct mtk_scp *scp)
> }
>
> static struct mtk_scp *scp_rproc_init(struct platform_device *pdev,
> - struct mtk_scp_of_cluster *scp_cluster)
> + struct mtk_scp_of_cluster *scp_cluster,
> + const struct mtk_scp_of_data *of_data)
> {
> struct device *dev = &pdev->dev;
> struct device_node *np = dev->of_node;
> @@ -887,7 +888,7 @@ static struct mtk_scp *scp_rproc_init(struct platform_device *pdev,
> scp = rproc->priv;
> scp->rproc = rproc;
> scp->dev = dev;
> - scp->data = of_device_get_match_data(dev);
> + scp->data = of_data;
> platform_device_add_data(pdev, &scp, sizeof(scp));
>
> scp->reg_base = scp_cluster->reg_base;
> @@ -964,14 +965,15 @@ static void scp_free(struct mtk_scp *scp)
> mutex_destroy(&scp->send_lock);
> }
>
> -static int scp_cluster_init(struct platform_device *pdev)
> +static int scp_add_single_core(struct platform_device *pdev)
> {
> + struct device *dev = &pdev->dev;
> struct mtk_scp_of_cluster *scp_cluster = platform_get_drvdata(pdev);
> struct list_head *cluster = &scp_cluster->mtk_scp_list;
> struct mtk_scp *scp;
> int ret;
>
> - scp = scp_rproc_init(pdev, scp_cluster);
> + scp = scp_rproc_init(pdev, scp_cluster, of_device_get_match_data(dev));
> if (IS_ERR(scp))
> return PTR_ERR(scp);
>
> @@ -987,6 +989,100 @@ static int scp_cluster_init(struct platform_device *pdev)
> return 0;
> }
>
> +static int scp_add_multi_core(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct device_node *np = dev_of_node(dev);
> + struct platform_device *cpdev;
> + struct device_node *child;
> + struct mtk_scp_of_cluster *scp_cluster = platform_get_drvdata(pdev);
> + struct list_head *cluster = &scp_cluster->mtk_scp_list;
> + const struct mtk_scp_of_data **cluster_of_data;
> + struct mtk_scp *scp, *temp;
> + int core_id = 0;
> + int ret;
> +
> + cluster_of_data = (const struct mtk_scp_of_data **)of_device_get_match_data(dev);
> +
> + for_each_available_child_of_node(np, child) {
> + if (!cluster_of_data[core_id]) {
> + ret = -EINVAL;
> + dev_err(dev, "Not support core %d\n", core_id);
> + of_node_put(child);
> + goto init_fail;
> + }
> +
> + cpdev = of_find_device_by_node(child);
> + if (!cpdev) {
> + ret = -ENODEV;
> + dev_err(dev, "Not found platform device for core %d\n", core_id);
> + of_node_put(child);
> + goto init_fail;
> + }
> +
> + scp = scp_rproc_init(cpdev, scp_cluster, cluster_of_data[core_id]);
> + put_device(&cpdev->dev);
> + if (IS_ERR(scp)) {
> + ret = PTR_ERR(scp);
> + dev_err(dev, "Failed to initialize core %d rproc\n", core_id);
> + of_node_put(child);
> + goto init_fail;
> + }
> +
> + ret = rproc_add(scp->rproc);
> + if (ret) {
> + dev_err(dev, "Failed to add rproc of core %d\n", core_id);
> + of_node_put(child);
> + scp_free(scp);
> + goto init_fail;
> + }
> +
> + list_add_tail(&scp->elem, cluster);
> + core_id++;
> + }
> +
> + return 0;
> +
> +init_fail:
> + list_for_each_entry_safe_reverse(scp, temp, cluster, elem) {
> + list_del(&scp->elem);
> + rproc_del(scp->rproc);
> + scp_free(scp);
> + }
> +
> + return ret;
> +}
> +
> +static int scp_is_single_core(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct device_node *np = dev_of_node(dev);
> + struct device_node *child;
> +
> + child = of_get_next_available_child(np, NULL);
> + if (!child)
> + return dev_err_probe(dev, -ENODEV, "No child node\n");
> +
> + of_node_put(child);
> + return of_node_name_eq(child, "cros-ec-rpmsg");
> +}
> +
> +static int scp_cluster_init(struct platform_device *pdev)
> +{
> + int ret;
> +
> + ret = scp_is_single_core(pdev);
> + if (ret < 0)
> + return ret;
> +
> + if (ret)
> + ret = scp_add_single_core(pdev);
> + else
> + ret = scp_add_multi_core(pdev);
> +
> + return ret;
> +}
> +
> static int scp_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -1021,6 +1117,10 @@ static int scp_probe(struct platform_device *pdev)
> INIT_LIST_HEAD(&scp_cluster->mtk_scp_list);
> platform_set_drvdata(pdev, scp_cluster);
>
> + ret = devm_of_platform_populate(dev);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to populate platform devices\n");
> +
> ret = scp_cluster_init(pdev);
> if (ret)
> return ret;
> @@ -1114,12 +1214,19 @@ static const struct mtk_scp_of_data mt8195_of_data_c1 = {
> .host_to_scp_int_bit = MT8195_CORE1_HOST_IPC_INT_BIT,
> };
>
> +static const struct mtk_scp_of_data *mt8195_of_data_cores[] = {
> + &mt8195_of_data,
> + &mt8195_of_data_c1,
> + NULL
> +};
> +
> 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-dual", .data = &mt8195_of_data_cores },

I am happy with the refactoring work. Please do not change anything in patches
05 and 06 unless I ask for it.

> {},
> };
> MODULE_DEVICE_TABLE(of, mtk_scp_of_match);
> --
> 2.18.0
>

2023-05-17 18:22:15

by Mathieu Poirier

[permalink] [raw]
Subject: Re: [PATCH v12 07/11] remoteproc: mediatek: Control SCP core 1 by rproc subdevice

On Wed, May 17, 2023 at 12:34:45PM +0800, Tinghan Shen wrote:
> Register SCP core 1 as a subdevice of core 0 for the boot sequence
> and watchdog timeout handling. The core 1 has to boot after core 0
> because the SCP clock and SRAM power is controlled by SCP core 0.
> As for watchdog timeout handling, the remoteproc framework helps to
> stop/start subdevices automatically when SCP driver receives watchdog
> timeout event.
>

Subdevices should be reserved for virtio devices and not used for remote
processors. Mixing both will get us into trouble at some point in the future.
The way to address power and clock dependencies is to use a different rproc_ops
for SCP1. That rproc_ops would implemented the prepare() and unprepare()
functions to power up SCP1's power domain and deal with its clock.

I will stop here for this revision.

Thanks,
Mathieu


> Signed-off-by: Tinghan Shen <[email protected]>
> Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
> ---
> drivers/remoteproc/mtk_common.h | 9 ++++
> drivers/remoteproc/mtk_scp.c | 88 +++++++++++++++++++++++++++++++++
> 2 files changed, 97 insertions(+)
>
> diff --git a/drivers/remoteproc/mtk_common.h b/drivers/remoteproc/mtk_common.h
> index 56395e8664cb..85afed2e6928 100644
> --- a/drivers/remoteproc/mtk_common.h
> +++ b/drivers/remoteproc/mtk_common.h
> @@ -100,6 +100,13 @@ struct mtk_scp_of_data {
> size_t ipi_buf_offset;
> };
>
> +struct mtk_scp_core_subdev {
> + struct rproc_subdev subdev;
> + struct mtk_scp *scp;
> +};
> +
> +#define to_core_subdev(d) container_of(d, struct mtk_scp_core_subdev, subdev)
> +
> struct mtk_scp {
> struct device *dev;
> struct rproc *rproc;
> @@ -130,6 +137,8 @@ struct mtk_scp {
> struct rproc_subdev *rpmsg_subdev;
>
> struct list_head elem;
> + struct list_head *cluster;
> + struct mtk_scp_core_subdev *core_subdev;
> };
>
> /**
> diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
> index d644e232dfec..18534b1179b5 100644
> --- a/drivers/remoteproc/mtk_scp.c
> +++ b/drivers/remoteproc/mtk_scp.c
> @@ -863,6 +863,60 @@ static void scp_remove_rpmsg_subdev(struct mtk_scp *scp)
> }
> }
>
> +static int scp_core_subdev_start(struct rproc_subdev *subdev)
> +{
> + struct mtk_scp_core_subdev *core_subdev = to_core_subdev(subdev);
> + struct mtk_scp *scp = core_subdev->scp;
> +
> + rproc_boot(scp->rproc);
> +
> + return 0;
> +}
> +
> +static void scp_core_subdev_stop(struct rproc_subdev *subdev, bool crashed)
> +{
> + struct mtk_scp_core_subdev *core_subdev = to_core_subdev(subdev);
> + struct mtk_scp *scp = core_subdev->scp;
> +
> + rproc_shutdown(scp->rproc);
> +}
> +
> +static int scp_core_subdev_register(struct mtk_scp *scp)
> +{
> + struct device *dev = scp->dev;
> + struct mtk_scp_core_subdev *core_subdev;
> + struct mtk_scp *scp_c0;
> +
> + scp_c0 = list_first_entry(scp->cluster, struct mtk_scp, elem);
> + if (!scp_c0)
> + return -ENODATA;
> +
> + core_subdev = devm_kzalloc(dev, sizeof(*core_subdev), GFP_KERNEL);
> + if (!core_subdev)
> + return -ENOMEM;
> +
> + core_subdev->scp = scp;
> + core_subdev->subdev.start = scp_core_subdev_start;
> + core_subdev->subdev.stop = scp_core_subdev_stop;
> +
> + scp->core_subdev = core_subdev;
> + rproc_add_subdev(scp_c0->rproc, &scp->core_subdev->subdev);
> +
> + return 0;
> +}
> +
> +static void scp_core_subdev_unregister(struct mtk_scp *scp)
> +{
> + struct mtk_scp *scp_c0;
> +
> + if (scp->core_subdev) {
> + scp_c0 = list_first_entry(scp->cluster, struct mtk_scp, elem);
> + rproc_remove_subdev(scp_c0->rproc, &scp->core_subdev->subdev);
> + devm_kfree(scp->dev, scp->core_subdev);
> + scp->core_subdev = NULL;
> + }
> +}
> +
> static struct mtk_scp *scp_rproc_init(struct platform_device *pdev,
> struct mtk_scp_of_cluster *scp_cluster,
> const struct mtk_scp_of_data *of_data)
> @@ -957,6 +1011,7 @@ static void scp_free(struct mtk_scp *scp)
> {
> int i;
>
> + scp_core_subdev_unregister(scp);
> scp_remove_rpmsg_subdev(scp);
> scp_ipi_unregister(scp, SCP_IPI_INIT);
> scp_unmap_memory_region(scp);
> @@ -989,6 +1044,15 @@ static int scp_add_single_core(struct platform_device *pdev)
> return 0;
> }
>
> +static void scp_rproc_boot_callback(const struct firmware *fw, void *context)
> +{
> + struct rproc *rproc = context;
> +
> + rproc_boot(rproc);
> +
> + release_firmware(fw);
> +}
> +
> static int scp_add_multi_core(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -1029,6 +1093,20 @@ static int scp_add_multi_core(struct platform_device *pdev)
> goto init_fail;
> }
>
> + scp->cluster = cluster;
> + if (!list_empty(cluster)) {
> + ret = scp_core_subdev_register(scp);
> + if (ret) {
> + dev_err(dev, "Failed to register core %d as subdev\n", core_id);
> + of_node_put(child);
> + scp_free(scp);
> + goto init_fail;
> + }
> + }
> +
> + /* boot after all cores finished rproc_add() */
> + scp->rproc->auto_boot = false;
> +
> ret = rproc_add(scp->rproc);
> if (ret) {
> dev_err(dev, "Failed to add rproc of core %d\n", core_id);
> @@ -1041,6 +1119,16 @@ static int scp_add_multi_core(struct platform_device *pdev)
> core_id++;
> }
>
> + /* boot core 0, and other cores are booted following core 0 as subdevices */
> + scp = list_first_entry(cluster, struct mtk_scp, elem);
> + ret = request_firmware_nowait(THIS_MODULE, FW_ACTION_UEVENT,
> + scp->rproc->firmware, &scp->rproc->dev, GFP_KERNEL,
> + scp->rproc, scp_rproc_boot_callback);
> + if (ret < 0) {
> + dev_err(dev, "request_firmware_nowait err: %d\n", ret);
> + goto init_fail;
> + }
> +
> return 0;
>
> init_fail:
> --
> 2.18.0
>

2023-05-18 12:14:19

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH v12 07/11] remoteproc: mediatek: Control SCP core 1 by rproc subdevice

Hi Tinghan,

kernel test robot noticed the following build warnings:

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Tinghan-Shen/dt-bindings-remoteproc-mediatek-Improve-the-rpmsg-subnode-definition/20230517-123659
base: git://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux.git rproc-next
patch link: https://lore.kernel.org/r/20230517043449.26352-8-tinghan.shen%40mediatek.com
patch subject: [PATCH v12 07/11] remoteproc: mediatek: Control SCP core 1 by rproc subdevice
config: csky-randconfig-m041-20230517
compiler: csky-linux-gcc (GCC) 12.1.0

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Reported-by: Dan Carpenter <[email protected]>
| Closes: https://lore.kernel.org/r/[email protected]/

smatch warnings:
drivers/remoteproc/mtk_scp.c:891 scp_core_subdev_register() warn: can 'scp_c0' even be NULL?

vim +/scp_c0 +891 drivers/remoteproc/mtk_scp.c

69e1791d92cfa0 Tinghan Shen 2023-05-17 884 static int scp_core_subdev_register(struct mtk_scp *scp)
69e1791d92cfa0 Tinghan Shen 2023-05-17 885 {
69e1791d92cfa0 Tinghan Shen 2023-05-17 886 struct device *dev = scp->dev;
69e1791d92cfa0 Tinghan Shen 2023-05-17 887 struct mtk_scp_core_subdev *core_subdev;
69e1791d92cfa0 Tinghan Shen 2023-05-17 888 struct mtk_scp *scp_c0;
69e1791d92cfa0 Tinghan Shen 2023-05-17 889
69e1791d92cfa0 Tinghan Shen 2023-05-17 890 scp_c0 = list_first_entry(scp->cluster, struct mtk_scp, elem);
69e1791d92cfa0 Tinghan Shen 2023-05-17 @891 if (!scp_c0)
69e1791d92cfa0 Tinghan Shen 2023-05-17 892 return -ENODATA;

This NULL check isn't right. Use list_first_entry_or_null() if the list
can be empty.

69e1791d92cfa0 Tinghan Shen 2023-05-17 893
69e1791d92cfa0 Tinghan Shen 2023-05-17 894 core_subdev = devm_kzalloc(dev, sizeof(*core_subdev), GFP_KERNEL);
69e1791d92cfa0 Tinghan Shen 2023-05-17 895 if (!core_subdev)
69e1791d92cfa0 Tinghan Shen 2023-05-17 896 return -ENOMEM;
69e1791d92cfa0 Tinghan Shen 2023-05-17 897
69e1791d92cfa0 Tinghan Shen 2023-05-17 898 core_subdev->scp = scp;
69e1791d92cfa0 Tinghan Shen 2023-05-17 899 core_subdev->subdev.start = scp_core_subdev_start;
69e1791d92cfa0 Tinghan Shen 2023-05-17 900 core_subdev->subdev.stop = scp_core_subdev_stop;
69e1791d92cfa0 Tinghan Shen 2023-05-17 901
69e1791d92cfa0 Tinghan Shen 2023-05-17 902 scp->core_subdev = core_subdev;
69e1791d92cfa0 Tinghan Shen 2023-05-17 903 rproc_add_subdev(scp_c0->rproc, &scp->core_subdev->subdev);
69e1791d92cfa0 Tinghan Shen 2023-05-17 904
69e1791d92cfa0 Tinghan Shen 2023-05-17 905 return 0;
69e1791d92cfa0 Tinghan Shen 2023-05-17 906 }

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


2023-05-29 14:25:52

by Matthias Brugger

[permalink] [raw]
Subject: Re: [PATCH v12 03/11] dt-bindings: remoteproc: mediatek: Support MT8195 dual-core SCP



On 17/05/2023 06:34, Tinghan Shen wrote:
> Extend the SCP binding to describe the MT8195 dual-core SCP.
>
> Under different applications, the MT8195 SCP can be used as single-core
> or dual-core. This change keeps the single-core definitions and
> adds new definitions for the dual-core use case.
>
> Signed-off-by: Tinghan Shen <[email protected]>
> Reviewed-by: Rob Herring <[email protected]>
> Reviewed-by: AngeloGioacchino Del Regno <[email protected]>

Reviewed-by: Matthias Brugger <[email protected]>

> ---
> .../bindings/remoteproc/mtk,scp.yaml | 145 +++++++++++++++++-
> 1 file changed, 141 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> index 271081df0e46..09102dda4942 100644
> --- a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> @@ -21,6 +21,7 @@ properties:
> - mediatek,mt8188-scp
> - mediatek,mt8192-scp
> - mediatek,mt8195-scp
> + - mediatek,mt8195-scp-dual
>
> reg:
> description:
> @@ -31,10 +32,7 @@ properties:
>
> reg-names:
> minItems: 2
> - items:
> - - const: sram
> - - const: cfg
> - - const: l1tcm
> + maxItems: 3
>
> clocks:
> description:
> @@ -70,6 +68,81 @@ properties:
>
> unevaluatedProperties: false
>
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 1
> +
> + ranges:
> + description:
> + Standard ranges definition providing address translations for
> + local SCP SRAM address spaces to bus addresses.
> +
> +patternProperties:
> + "^scp@[a-f0-9]+$":
> + type: object
> + description:
> + The MediaTek SCP integrated to SoC might be a multi-core version.
> + The other cores are represented as child nodes of the boot core.
> + There are some integration differences for the IP like the usage of
> + address translator for translating SoC bus addresses into address space
> + for the processor.
> +
> + Each SCP core has own cache memory. The SRAM and L1TCM are shared by
> + cores. The power of cache, SRAM and L1TCM power should be enabled
> + before booting SCP cores. The size of cache, SRAM, and L1TCM are varied
> + on differnt SoCs.
> +
> + The SCP cores do not use an MMU, but has a set of registers to
> + control the translations between 32-bit CPU addresses into system bus
> + addresses. Cache and memory access settings are provided through a
> + Memory Protection Unit (MPU), programmable only from the SCP.
> +
> + properties:
> + compatible:
> + enum:
> + - mediatek,scp-core
> +
> + reg:
> + description: The base address and size of SRAM.
> + maxItems: 1
> +
> + reg-names:
> + const: sram
> +
> + interrupts:
> + maxItems: 1
> +
> + firmware-name:
> + $ref: /schemas/types.yaml#/definitions/string
> + description:
> + If present, name (or relative path) of the file within the
> + firmware search path containing the firmware image used when
> + initializing sub cores of multi-core SCP.
> +
> + memory-region:
> + maxItems: 1
> +
> + cros-ec-rpmsg:
> + $ref: /schemas/mfd/google,cros-ec.yaml
> + description:
> + This subnode represents the rpmsg device. The properties
> + of this node are defined by the individual bindings for
> + the rpmsg devices.
> +
> + required:
> + - mediatek,rpmsg-name
> +
> + unevaluatedProperties: false
> +
> + required:
> + - compatible
> + - reg
> + - reg-names
> +
> + additionalProperties: false
> +
> required:
> - compatible
> - reg
> @@ -99,7 +172,37 @@ allOf:
> reg:
> maxItems: 2
> reg-names:
> + items:
> + - const: sram
> + - const: cfg
> + - if:
> + properties:
> + compatible:
> + enum:
> + - mediatek,mt8192-scp
> + - mediatek,mt8195-scp
> + then:
> + properties:
> + reg:
> + maxItems: 3
> + reg-names:
> + items:
> + - const: sram
> + - const: cfg
> + - const: l1tcm
> + - if:
> + properties:
> + compatible:
> + enum:
> + - mediatek,mt8195-scp-dual
> + then:
> + properties:
> + reg:
> maxItems: 2
> + reg-names:
> + items:
> + - const: cfg
> + - const: l1tcm
>
> additionalProperties: false
>
> @@ -121,3 +224,37 @@ examples:
> mediatek,rpmsg-name = "cros-ec-rpmsg";
> };
> };
> +
> + - |
> + scp@10500000 {
> + compatible = "mediatek,mt8195-scp-dual";
> + reg = <0x10720000 0xe0000>,
> + <0x10700000 0x8000>;
> + reg-names = "cfg", "l1tcm";
> +
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0 0x10500000 0x100000>;
> +
> + scp@0 {
> + compatible = "mediatek,scp-core";
> + reg = <0x0 0xa0000>;
> + reg-names = "sram";
> +
> + cros-ec-rpmsg {
> + compatible = "google,cros-ec-rpmsg";
> + mediatek,rpmsg-name = "cros-ec-rpmsg";
> + };
> + };
> +
> + scp@a0000 {
> + compatible = "mediatek,scp-core";
> + reg = <0xa0000 0x20000>;
> + reg-names = "sram";
> +
> + cros-ec-rpmsg {
> + compatible = "google,cros-ec-rpmsg";
> + mediatek,rpmsg-name = "cros-ec-rpmsg";
> + };
> + };
> + };