remoteproc/mediatek: support L1TCM for MT8192 SCP

Adds L1TCM memory region. The reg-name is "l1tcm".

Signed-off-by Tzung-Bi Shih <[email protected]>

---
 Documentation/devicetree/bindings/remoteproc/mtk,scp.txt | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

L1TCM is a high performance memory region in MT8192 SCP.

Reads L1TCM memory region from DTS to determine if the machine supports. Loads L1TCM memory region to SCP sys if the firmware provides.

Starts from MT8192 SCP, the firmware contains physical addresses for each memory region, for instance:

Program Headers:

  Type   Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
  LOAD   0xXXXXXX 0xXXXXXXXX 0x10500000 0xXXXXX 0xXXXXX XXX 0xXXXX
  LOAD   0xXXXXXX 0xXXXXXXXX 0x10700000 0xXXXXX 0xXXXXX XXX 0xXXXX
  LOAD   0xXXXXXX 0xXXXXXXXX 0x50000000 0xXXXXX 0xXXXXX XXX 0xXXXX

Kernel driver can use the "PhysAddr" (i.e. da in the da_to_va callbacks) to know the ELF segment belongs to which region.

To backward compatible to MT8183 SCP, separates the da_to_va callbacks for new and legacy version.

Signed-off-by Tzung-Bi Shih <[email protected]>

---
 drivers/remoteproc/mtk_common.h |  5 +++
 drivers/remoteproc/mtk_scp.c    | 54 +++++++++++++++++++++++++++++++--
 2 files changed, 57 insertions(+), 2 deletions(-)

drivers/remoteproc/mtk_common.h +5 -0

index 988edb4977c3..94bc54b224ee 100644
@@ -75,6 +75,7 @@ struct mtk_scp_of_data {
 	void (*scp_reset_assert)(struct mtk_scp *scp);
 	void (*scp_reset_deassert)(struct mtk_scp *scp);
 	void (*scp_stop)(struct mtk_scp *scp);
+	void *(*scp_da_to_va)(struct mtk_scp *scp, u64 da, size_t len);
 
 	u32 host_to_scp_reg;
 	u32 host_to_scp_int_bit;

@@ -89,6 +90,10 @@ struct mtk_scp {
 	void __iomem *reg_base;
 	void __iomem *sram_base;
 	size_t sram_size;
+	phys_addr_t sram_phys;
+	void __iomem *l1tcm_base;
+	size_t l1tcm_size;
+	phys_addr_t l1tcm_phys;
 
 	const struct mtk_scp_of_data *data;
 

drivers/remoteproc/mtk_scp.c +37 -2

index e0c235690361..f025aba67abc 100644
@@ -458,9 +458,8 @@ static int scp_start(struct rproc *rproc)
 	return ret;
 }
 
-static void *scp_da_to_va(struct rproc *rproc, u64 da, size_t len)
+static void *mt8183_scp_da_to_va(struct mtk_scp *scp, u64 da, size_t len)
 {
-	struct mtk_scp *scp = (struct mtk_scp *)rproc->priv;
 	int offset;
 
 	if (da < scp->sram_size) {

@@ -476,6 +475,42 @@ static void *scp_da_to_va(struct rproc *rproc, u64 da, size_t len)
 	return NULL;
 }
 
+static void *mt8192_scp_da_to_va(struct mtk_scp *scp, u64 da, size_t len)
+{
+	int offset;
+
+	if (da >= scp->sram_phys &&
+	    (da + len) <= scp->sram_phys + scp->sram_size) {
+		offset = da - scp->sram_phys;
+		return (void __force *)scp->sram_base + offset;
+	}
+
+	/* optional memory region */
+	if (scp->l1tcm_size &&
+	    da >= scp->l1tcm_phys &&
+	    (da + len) <= scp->l1tcm_phys + scp->l1tcm_size) {
+		offset = da - scp->l1tcm_phys;
+		return (void __force *)scp->l1tcm_base + offset;
+	}
+
+	/* optional memory region */
+	if (scp->dram_size &&
+	    da >= scp->dma_addr &&
+	    (da + len) <= scp->dma_addr + scp->dram_size) {
+		offset = da - scp->dma_addr;
+		return scp->cpu_addr + offset;
+	}
+
+	return NULL;
+}
+
+static void *scp_da_to_va(struct rproc *rproc, u64 da, size_t len)
+{
+	struct mtk_scp *scp = (struct mtk_scp *)rproc->priv;
+
+	return scp->data->scp_da_to_va(scp, da, len);
+}
+
 static void mt8183_scp_stop(struct mtk_scp *scp)
 {
 	/* Disable SCP watchdog */

drivers/remoteproc/mtk_scp.c +11 -0

+		scp->l1tcm_base = devm_ioremap_resource(dev, res);
+		if (IS_ERR((__force void *)scp->l1tcm_base)) {
+			dev_err(dev, "Failed to parse and map l1tcm memory\n");
+			ret = PTR_ERR((__force void *)scp->l1tcm_base);
+			goto free_rproc;
+		}
+		scp->l1tcm_size = resource_size(res);
+		scp->l1tcm_phys = res->start;
+	}
 
 	mutex_init(&scp->send_lock);
 	for (i = 0; i < SCP_IPI_MAX; i++)

@@ -803,6 +851,7 @@ static const struct mtk_scp_of_data mt8183_of_data = {
 	.scp_reset_assert = mt8183_scp_reset_assert,
 	.scp_reset_deassert = mt8183_scp_reset_deassert,
 	.scp_stop = mt8183_scp_stop,
+	.scp_da_to_va = mt8183_scp_da_to_va,
 	.host_to_scp_reg = MT8183_HOST_TO_SCP,
 	.host_to_scp_int_bit = MT8183_HOST_IPC_INT_BIT,
 	.ipi_buf_offset = 0x7bdb0,

@@ -814,6 +863,7 @@ static const struct mtk_scp_of_data mt8192_of_data = {
 	.scp_reset_assert = mt8192_scp_reset_assert,
 	.scp_reset_deassert = mt8192_scp_reset_deassert,
 	.scp_stop = mt8192_scp_stop,
+	.scp_da_to_va = mt8192_scp_da_to_va,
 	.host_to_scp_reg = MT8192_GIPC_IN_SET,
 	.host_to_scp_int_bit = MT8192_HOST_IPC_INT_BIT,
 };

Signature · 1 line

2.29.2.684.gfbc64c5ab5-goog

drivers/remoteproc/mtk_scp.c static int scp_probe(struct platform_device *pdev) · line 755

Quoted 12 lines

static void mt8183_scp_stop(struct mtk_scp *scp)

 {
      /* Disable SCP watchdog */

drivers/remoteproc/mtk_scp.c +4 -0

@@ -714,6 +749,19 @@ static int scp_probe(struct platform_device *pdev)
              goto free_rproc;
      }
      scp->sram_size = resource_size(res);
+     scp->sram_phys = res->start;
+
+     res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "l1tcm");
+     if (res) {

As far as I can tell the if() condition isn't needed since platform_get_resource_byname() returns NULL on error and devm_ioremap_resource() is capable of handling that condition. As such the code to parse "l1tcm" can be the same as what is done for "sram".

With the above:

Reviewed-by Mathieu Poirier <[email protected]>

Quoted 9 lines

drivers/remoteproc/mtk_scp.c +9 -0

+             scp->l1tcm_base = devm_ioremap_resource(dev, res);
+             if (IS_ERR((__force void *)scp->l1tcm_base)) {
+                     dev_err(dev, "Failed to parse and map l1tcm memory\n");
+                     ret = PTR_ERR((__force void *)scp->l1tcm_base);
+                     goto free_rproc;
+             }
+             scp->l1tcm_size = resource_size(res);
+             scp->l1tcm_phys = res->start;
+     }

Quoted 17 lines · Mathieu Poirier

drivers/remoteproc/mtk_scp.c static int scp_probe(struct platform_device *pdev) · line 755

Quoted 12 lines

static void mt8183_scp_stop(struct mtk_scp *scp)

 {
      /* Disable SCP watchdog */

drivers/remoteproc/mtk_scp.c +4 -0

@@ -714,6 +749,19 @@ static int scp_probe(struct platform_device *pdev)
              goto free_rproc;
      }
      scp->sram_size = resource_size(res);
+     scp->sram_phys = res->start;
+
+     res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "l1tcm");
+     if (res) {

As far as I can tell the if() condition isn't needed since platform_get_resource_byname() returns NULL on error and devm_ioremap_resource() is capable of handling that condition. As such the code to parse "l1tcm" can be the same as what is done for "sram".

The "l1tcm" memory region is optional. The if() condition is for: if DTS doesn't provide the memory region, kernel can skip the code block.

Quoted 5 lines

With the above:

Reviewed-by Mathieu Poirier <[email protected]>

Quoted 9 lines

drivers/remoteproc/mtk_scp.c static int scp_probe(struct platform_device *pdev) · line 764

Quoted 9 lines

drivers/remoteproc/mtk_scp.c +9 -0

+             scp->l1tcm_base = devm_ioremap_resource(dev, res);
+             if (IS_ERR((__force void *)scp->l1tcm_base)) {
+                     dev_err(dev, "Failed to parse and map l1tcm memory\n");
+                     ret = PTR_ERR((__force void *)scp->l1tcm_base);
+                     goto free_rproc;
+             }
+             scp->l1tcm_size = resource_size(res);
+             scp->l1tcm_phys = res->start;
+     }

Quoted 17 lines · Mathieu Poirier

drivers/remoteproc/mtk_scp.c static int scp_probe(struct platform_device *pdev) · line 755

Quoted 12 lines

static void mt8183_scp_stop(struct mtk_scp *scp)

 {
      /* Disable SCP watchdog */

drivers/remoteproc/mtk_scp.c +4 -0

@@ -714,6 +749,19 @@ static int scp_probe(struct platform_device *pdev)
              goto free_rproc;
      }
      scp->sram_size = resource_size(res);
+     scp->sram_phys = res->start;
+
+     res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "l1tcm");
+     if (res) {

As far as I can tell the if() condition isn't needed since platform_get_resource_byname() returns NULL on error and devm_ioremap_resource() is capable of handling that condition. As such the code to parse "l1tcm" can be the same as what is done for "sram".

The "l1tcm" memory region is optional. The if() condition is for: if DTS doesn't provide the memory region, kernel can skip the code block.

Quoted 5 lines

With the above:

Reviewed-by Mathieu Poirier <[email protected]>

Quoted 9 lines

drivers/remoteproc/mtk_scp.c static int scp_probe(struct platform_device *pdev) · line 764

Quoted 9 lines

drivers/remoteproc/mtk_scp.c +9 -0

+             scp->l1tcm_base = devm_ioremap_resource(dev, res);
+             if (IS_ERR((__force void *)scp->l1tcm_base)) {
+                     dev_err(dev, "Failed to parse and map l1tcm memory\n");
+                     ret = PTR_ERR((__force void *)scp->l1tcm_base);
+                     goto free_rproc;
+             }
+             scp->l1tcm_size = resource_size(res);
+             scp->l1tcm_phys = res->start;
+     }

Quoted 20 lines · Tzung-Bi Shih

Quoted 17 lines · Mathieu Poirier

drivers/remoteproc/mtk_scp.c static int scp_probe(struct platform_device *pdev) · line 755

Quoted 12 lines

static void mt8183_scp_stop(struct mtk_scp *scp)

 {
      /* Disable SCP watchdog */

drivers/remoteproc/mtk_scp.c +4 -0

@@ -714,6 +749,19 @@ static int scp_probe(struct platform_device *pdev)
              goto free_rproc;
      }
      scp->sram_size = resource_size(res);
+     scp->sram_phys = res->start;
+
+     res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "l1tcm");
+     if (res) {

As far as I can tell the if() condition isn't needed since platform_get_resource_byname() returns NULL on error and devm_ioremap_resource() is capable of handling that condition. As such the code to parse "l1tcm" can be the same as what is done for "sram".

Quoted 3 lines

The "l1tcm" memory region is optional. The if() condition is for: if DTS doesn't provide the memory region, kernel can skip the code block.

People are actively looking for platform_get_resource_byname + devm_ioremap_resource() pairs to replace with devm_platform_ioremap_resource_byname(), so we're probably going to have someone try to patch this soon...

So please change the pair to devm_platform_ioremap_resource_byname() and treat a returned -EINVAL as the memory isn't specified and other IS_ERR() as errors.