* [PATCH 0/2] remoteproc/mediatek: support L1TCM for MT8192 SCP
@ 2020-12-14 5:05 ` Tzung-Bi Shih
0 siblings, 0 replies; 18+ messages in thread
From: Tzung-Bi Shih @ 2020-12-14 5:05 UTC (permalink / raw)
To: ohad, bjorn.andersson, robh+dt
Cc: linux-remoteproc, matthias.bgg, linux-mediatek, devicetree, tzungbi
The series supports L1TCM which is a high performance memory region in
MT8192 SCP.
The 1st patch adds a new reg-name "l1tcm" for L1TCM.
The 2nd patch supports L1TCM in the firmware loader. Note that MT8192
SCP is still under development. The 2nd patch breaks early MT8192 SCP
firmware which should only break our development environment.
Tzung-Bi Shih (2):
dt-bindings: remoteproc: mediatek: add L1TCM memory region
remoteproc/mediatek: support L1TCM
.../bindings/remoteproc/mtk,scp.txt | 8 +--
drivers/remoteproc/mtk_common.h | 5 ++
drivers/remoteproc/mtk_scp.c | 54 ++++++++++++++++++-
3 files changed, 61 insertions(+), 6 deletions(-)
--
2.29.2.684.gfbc64c5ab5-goog
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 0/2] remoteproc/mediatek: support L1TCM for MT8192 SCP
@ 2020-12-14 5:05 ` Tzung-Bi Shih
0 siblings, 0 replies; 18+ messages in thread
From: Tzung-Bi Shih @ 2020-12-14 5:05 UTC (permalink / raw)
To: ohad, bjorn.andersson, robh+dt
Cc: matthias.bgg, linux-mediatek, linux-remoteproc, tzungbi, devicetree
The series supports L1TCM which is a high performance memory region in
MT8192 SCP.
The 1st patch adds a new reg-name "l1tcm" for L1TCM.
The 2nd patch supports L1TCM in the firmware loader. Note that MT8192
SCP is still under development. The 2nd patch breaks early MT8192 SCP
firmware which should only break our development environment.
Tzung-Bi Shih (2):
dt-bindings: remoteproc: mediatek: add L1TCM memory region
remoteproc/mediatek: support L1TCM
.../bindings/remoteproc/mtk,scp.txt | 8 +--
drivers/remoteproc/mtk_common.h | 5 ++
drivers/remoteproc/mtk_scp.c | 54 ++++++++++++++++++-
3 files changed, 61 insertions(+), 6 deletions(-)
--
2.29.2.684.gfbc64c5ab5-goog
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/2] dt-bindings: remoteproc: mediatek: add L1TCM memory region
2020-12-14 5:05 ` Tzung-Bi Shih
@ 2020-12-14 5:05 ` Tzung-Bi Shih
-1 siblings, 0 replies; 18+ messages in thread
From: Tzung-Bi Shih @ 2020-12-14 5:05 UTC (permalink / raw)
To: ohad, bjorn.andersson, robh+dt
Cc: linux-remoteproc, matthias.bgg, linux-mediatek, devicetree, tzungbi
Adds L1TCM memory region. The reg-name is "l1tcm".
Signed-off-by: Tzung-Bi Shih <tzungbi@google.com>
---
Documentation/devicetree/bindings/remoteproc/mtk,scp.txt | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/remoteproc/mtk,scp.txt b/Documentation/devicetree/bindings/remoteproc/mtk,scp.txt
index 3ba668bab14b..3f5f78764b60 100644
--- a/Documentation/devicetree/bindings/remoteproc/mtk,scp.txt
+++ b/Documentation/devicetree/bindings/remoteproc/mtk,scp.txt
@@ -6,10 +6,10 @@ Mediatek SoCs.
Required properties:
- compatible Should be "mediatek,mt8183-scp"
-- reg Should contain the address ranges for the two memory
- regions, SRAM and CFG.
-- reg-names Contains the corresponding names for the two memory
- regions. These should be named "sram" & "cfg".
+- reg Should contain the address ranges for memory regions:
+ SRAM, CFG, and L1TCM.
+- reg-names Contains the corresponding names for the memory regions:
+ "sram", "cfg", and "l1tcm".
- clocks Clock for co-processor (See: ../clock/clock-bindings.txt)
- clock-names Contains the corresponding name for the clock. This
should be named "main".
--
2.29.2.684.gfbc64c5ab5-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 1/2] dt-bindings: remoteproc: mediatek: add L1TCM memory region
@ 2020-12-14 5:05 ` Tzung-Bi Shih
0 siblings, 0 replies; 18+ messages in thread
From: Tzung-Bi Shih @ 2020-12-14 5:05 UTC (permalink / raw)
To: ohad, bjorn.andersson, robh+dt
Cc: matthias.bgg, linux-mediatek, linux-remoteproc, tzungbi, devicetree
Adds L1TCM memory region. The reg-name is "l1tcm".
Signed-off-by: Tzung-Bi Shih <tzungbi@google.com>
---
Documentation/devicetree/bindings/remoteproc/mtk,scp.txt | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/remoteproc/mtk,scp.txt b/Documentation/devicetree/bindings/remoteproc/mtk,scp.txt
index 3ba668bab14b..3f5f78764b60 100644
--- a/Documentation/devicetree/bindings/remoteproc/mtk,scp.txt
+++ b/Documentation/devicetree/bindings/remoteproc/mtk,scp.txt
@@ -6,10 +6,10 @@ Mediatek SoCs.
Required properties:
- compatible Should be "mediatek,mt8183-scp"
-- reg Should contain the address ranges for the two memory
- regions, SRAM and CFG.
-- reg-names Contains the corresponding names for the two memory
- regions. These should be named "sram" & "cfg".
+- reg Should contain the address ranges for memory regions:
+ SRAM, CFG, and L1TCM.
+- reg-names Contains the corresponding names for the memory regions:
+ "sram", "cfg", and "l1tcm".
- clocks Clock for co-processor (See: ../clock/clock-bindings.txt)
- clock-names Contains the corresponding name for the clock. This
should be named "main".
--
2.29.2.684.gfbc64c5ab5-goog
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/2] remoteproc/mediatek: support L1TCM
2020-12-14 5:05 ` Tzung-Bi Shih
@ 2020-12-14 5:05 ` Tzung-Bi Shih
-1 siblings, 0 replies; 18+ messages in thread
From: Tzung-Bi Shih @ 2020-12-14 5:05 UTC (permalink / raw)
To: ohad, bjorn.andersson, robh+dt
Cc: linux-remoteproc, matthias.bgg, linux-mediatek, devicetree, tzungbi
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 <tzungbi@google.com>
---
drivers/remoteproc/mtk_common.h | 5 +++
drivers/remoteproc/mtk_scp.c | 54 +++++++++++++++++++++++++++++++--
2 files changed, 57 insertions(+), 2 deletions(-)
diff --git a/drivers/remoteproc/mtk_common.h b/drivers/remoteproc/mtk_common.h
index 988edb4977c3..94bc54b224ee 100644
--- a/drivers/remoteproc/mtk_common.h
+++ b/drivers/remoteproc/mtk_common.h
@@ -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;
diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
index e0c235690361..f025aba67abc 100644
--- a/drivers/remoteproc/mtk_scp.c
+++ b/drivers/remoteproc/mtk_scp.c
@@ -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 */
@@ -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) {
+ 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,
};
--
2.29.2.684.gfbc64c5ab5-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/2] remoteproc/mediatek: support L1TCM
@ 2020-12-14 5:05 ` Tzung-Bi Shih
0 siblings, 0 replies; 18+ messages in thread
From: Tzung-Bi Shih @ 2020-12-14 5:05 UTC (permalink / raw)
To: ohad, bjorn.andersson, robh+dt
Cc: matthias.bgg, linux-mediatek, linux-remoteproc, tzungbi, devicetree
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 <tzungbi@google.com>
---
drivers/remoteproc/mtk_common.h | 5 +++
drivers/remoteproc/mtk_scp.c | 54 +++++++++++++++++++++++++++++++--
2 files changed, 57 insertions(+), 2 deletions(-)
diff --git a/drivers/remoteproc/mtk_common.h b/drivers/remoteproc/mtk_common.h
index 988edb4977c3..94bc54b224ee 100644
--- a/drivers/remoteproc/mtk_common.h
+++ b/drivers/remoteproc/mtk_common.h
@@ -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;
diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
index e0c235690361..f025aba67abc 100644
--- a/drivers/remoteproc/mtk_scp.c
+++ b/drivers/remoteproc/mtk_scp.c
@@ -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 */
@@ -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) {
+ 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,
};
--
2.29.2.684.gfbc64c5ab5-goog
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] dt-bindings: remoteproc: mediatek: add L1TCM memory region
2020-12-14 5:05 ` Tzung-Bi Shih
@ 2020-12-15 17:12 ` Rob Herring
-1 siblings, 0 replies; 18+ messages in thread
From: Rob Herring @ 2020-12-15 17:12 UTC (permalink / raw)
To: Tzung-Bi Shih
Cc: matthias.bgg, ohad, linux-mediatek, linux-remoteproc, robh+dt,
bjorn.andersson, devicetree
On Mon, 14 Dec 2020 13:05:20 +0800, Tzung-Bi Shih wrote:
> Adds L1TCM memory region. The reg-name is "l1tcm".
>
> Signed-off-by: Tzung-Bi Shih <tzungbi@google.com>
> ---
> Documentation/devicetree/bindings/remoteproc/mtk,scp.txt | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] dt-bindings: remoteproc: mediatek: add L1TCM memory region
@ 2020-12-15 17:12 ` Rob Herring
0 siblings, 0 replies; 18+ messages in thread
From: Rob Herring @ 2020-12-15 17:12 UTC (permalink / raw)
To: Tzung-Bi Shih
Cc: ohad, devicetree, linux-remoteproc, bjorn.andersson, robh+dt,
linux-mediatek, matthias.bgg
On Mon, 14 Dec 2020 13:05:20 +0800, Tzung-Bi Shih wrote:
> Adds L1TCM memory region. The reg-name is "l1tcm".
>
> Signed-off-by: Tzung-Bi Shih <tzungbi@google.com>
> ---
> Documentation/devicetree/bindings/remoteproc/mtk,scp.txt | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
Acked-by: Rob Herring <robh@kernel.org>
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] remoteproc/mediatek: support L1TCM
2020-12-14 5:05 ` Tzung-Bi Shih
@ 2021-01-06 23:15 ` Mathieu Poirier
-1 siblings, 0 replies; 18+ messages in thread
From: Mathieu Poirier @ 2021-01-06 23:15 UTC (permalink / raw)
To: Tzung-Bi Shih
Cc: ohad, bjorn.andersson, robh+dt, linux-remoteproc, matthias.bgg,
linux-mediatek, devicetree
Hi Shih,
On Mon, Dec 14, 2020 at 01:05:21PM +0800, Tzung-Bi Shih wrote:
> 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 <tzungbi@google.com>
> ---
> drivers/remoteproc/mtk_common.h | 5 +++
> drivers/remoteproc/mtk_scp.c | 54 +++++++++++++++++++++++++++++++--
> 2 files changed, 57 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/remoteproc/mtk_common.h b/drivers/remoteproc/mtk_common.h
> index 988edb4977c3..94bc54b224ee 100644
> --- a/drivers/remoteproc/mtk_common.h
> +++ b/drivers/remoteproc/mtk_common.h
> @@ -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;
>
> diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
> index e0c235690361..f025aba67abc 100644
> --- a/drivers/remoteproc/mtk_scp.c
> +++ b/drivers/remoteproc/mtk_scp.c
> @@ -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 */
> @@ -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 <mathieu.poirier@linaro.org>
> + 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,
> };
> --
> 2.29.2.684.gfbc64c5ab5-goog
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] remoteproc/mediatek: support L1TCM
@ 2021-01-06 23:15 ` Mathieu Poirier
0 siblings, 0 replies; 18+ messages in thread
From: Mathieu Poirier @ 2021-01-06 23:15 UTC (permalink / raw)
To: Tzung-Bi Shih
Cc: ohad, devicetree, linux-remoteproc, bjorn.andersson, robh+dt,
linux-mediatek, matthias.bgg
Hi Shih,
On Mon, Dec 14, 2020 at 01:05:21PM +0800, Tzung-Bi Shih wrote:
> 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 <tzungbi@google.com>
> ---
> drivers/remoteproc/mtk_common.h | 5 +++
> drivers/remoteproc/mtk_scp.c | 54 +++++++++++++++++++++++++++++++--
> 2 files changed, 57 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/remoteproc/mtk_common.h b/drivers/remoteproc/mtk_common.h
> index 988edb4977c3..94bc54b224ee 100644
> --- a/drivers/remoteproc/mtk_common.h
> +++ b/drivers/remoteproc/mtk_common.h
> @@ -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;
>
> diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
> index e0c235690361..f025aba67abc 100644
> --- a/drivers/remoteproc/mtk_scp.c
> +++ b/drivers/remoteproc/mtk_scp.c
> @@ -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 */
> @@ -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 <mathieu.poirier@linaro.org>
> + 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,
> };
> --
> 2.29.2.684.gfbc64c5ab5-goog
>
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] remoteproc/mediatek: support L1TCM
2021-01-06 23:15 ` Mathieu Poirier
@ 2021-01-07 1:50 ` Tzung-Bi Shih
-1 siblings, 0 replies; 18+ messages in thread
From: Tzung-Bi Shih @ 2021-01-07 1:50 UTC (permalink / raw)
To: Mathieu Poirier
Cc: Ohad Ben-Cohen, Bjorn Andersson, Rob Herring,
open list:REMOTE PROCESSOR (REMOTEPROC) SUBSYSTEM,
Matthias Brugger, moderated list:ARM/Mediatek SoC support,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
On Thu, Jan 7, 2021 at 7:15 AM Mathieu Poirier
<mathieu.poirier@linaro.org> wrote:
>
> > static void mt8183_scp_stop(struct mtk_scp *scp)
> > {
> > /* Disable SCP watchdog */
> > @@ -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.
>
> With the above:
>
> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>
> > + 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;
> > + }
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] remoteproc/mediatek: support L1TCM
@ 2021-01-07 1:50 ` Tzung-Bi Shih
0 siblings, 0 replies; 18+ messages in thread
From: Tzung-Bi Shih @ 2021-01-07 1:50 UTC (permalink / raw)
To: Mathieu Poirier
Cc: Ohad Ben-Cohen,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list:REMOTE PROCESSOR (REMOTEPROC) SUBSYSTEM,
Bjorn Andersson, Rob Herring,
moderated list:ARM/Mediatek SoC support, Matthias Brugger
On Thu, Jan 7, 2021 at 7:15 AM Mathieu Poirier
<mathieu.poirier@linaro.org> wrote:
>
> > static void mt8183_scp_stop(struct mtk_scp *scp)
> > {
> > /* Disable SCP watchdog */
> > @@ -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.
>
> With the above:
>
> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>
> > + 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;
> > + }
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] remoteproc/mediatek: support L1TCM
2021-01-07 1:50 ` Tzung-Bi Shih
@ 2021-01-07 17:54 ` Mathieu Poirier
-1 siblings, 0 replies; 18+ messages in thread
From: Mathieu Poirier @ 2021-01-07 17:54 UTC (permalink / raw)
To: Tzung-Bi Shih
Cc: Ohad Ben-Cohen, Bjorn Andersson, Rob Herring,
open list:REMOTE PROCESSOR (REMOTEPROC) SUBSYSTEM,
Matthias Brugger, moderated list:ARM/Mediatek SoC support,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
On Thu, Jan 07, 2021 at 09:50:21AM +0800, Tzung-Bi Shih wrote:
> On Thu, Jan 7, 2021 at 7:15 AM Mathieu Poirier
> <mathieu.poirier@linaro.org> wrote:
> >
> > > static void mt8183_scp_stop(struct mtk_scp *scp)
> > > {
> > > /* Disable SCP watchdog */
> > > @@ -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.
>
Very well - thanks for the clarification.
> >
> > With the above:
> >
> > Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> >
> > > + 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;
> > > + }
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] remoteproc/mediatek: support L1TCM
@ 2021-01-07 17:54 ` Mathieu Poirier
0 siblings, 0 replies; 18+ messages in thread
From: Mathieu Poirier @ 2021-01-07 17:54 UTC (permalink / raw)
To: Tzung-Bi Shih
Cc: Ohad Ben-Cohen,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list:REMOTE PROCESSOR (REMOTEPROC) SUBSYSTEM,
Bjorn Andersson, Rob Herring,
moderated list:ARM/Mediatek SoC support, Matthias Brugger
On Thu, Jan 07, 2021 at 09:50:21AM +0800, Tzung-Bi Shih wrote:
> On Thu, Jan 7, 2021 at 7:15 AM Mathieu Poirier
> <mathieu.poirier@linaro.org> wrote:
> >
> > > static void mt8183_scp_stop(struct mtk_scp *scp)
> > > {
> > > /* Disable SCP watchdog */
> > > @@ -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.
>
Very well - thanks for the clarification.
> >
> > With the above:
> >
> > Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> >
> > > + 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;
> > > + }
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] remoteproc/mediatek: support L1TCM
2021-01-07 1:50 ` Tzung-Bi Shih
@ 2021-01-07 18:02 ` Bjorn Andersson
-1 siblings, 0 replies; 18+ messages in thread
From: Bjorn Andersson @ 2021-01-07 18:02 UTC (permalink / raw)
To: Tzung-Bi Shih
Cc: Mathieu Poirier, Ohad Ben-Cohen, Rob Herring,
open list:REMOTE PROCESSOR (REMOTEPROC) SUBSYSTEM,
Matthias Brugger, moderated list:ARM/Mediatek SoC support,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
On Wed 06 Jan 19:50 CST 2021, Tzung-Bi Shih wrote:
> On Thu, Jan 7, 2021 at 7:15 AM Mathieu Poirier
> <mathieu.poirier@linaro.org> wrote:
> >
> > > static void mt8183_scp_stop(struct mtk_scp *scp)
> > > {
> > > /* Disable SCP watchdog */
> > > @@ -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.
>
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.
Thanks,
Bjorn
> >
> > With the above:
> >
> > Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> >
> > > + 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;
> > > + }
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] remoteproc/mediatek: support L1TCM
@ 2021-01-07 18:02 ` Bjorn Andersson
0 siblings, 0 replies; 18+ messages in thread
From: Bjorn Andersson @ 2021-01-07 18:02 UTC (permalink / raw)
To: Tzung-Bi Shih
Cc: Ohad Ben-Cohen,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Mathieu Poirier,
open list:REMOTE PROCESSOR (REMOTEPROC) SUBSYSTEM, Rob Herring,
moderated list:ARM/Mediatek SoC support, Matthias Brugger
On Wed 06 Jan 19:50 CST 2021, Tzung-Bi Shih wrote:
> On Thu, Jan 7, 2021 at 7:15 AM Mathieu Poirier
> <mathieu.poirier@linaro.org> wrote:
> >
> > > static void mt8183_scp_stop(struct mtk_scp *scp)
> > > {
> > > /* Disable SCP watchdog */
> > > @@ -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.
>
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.
Thanks,
Bjorn
> >
> > With the above:
> >
> > Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> >
> > > + 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;
> > > + }
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] remoteproc/mediatek: support L1TCM
2021-01-07 18:02 ` Bjorn Andersson
@ 2021-01-08 3:50 ` Tzung-Bi Shih
-1 siblings, 0 replies; 18+ messages in thread
From: Tzung-Bi Shih @ 2021-01-08 3:50 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Mathieu Poirier, Ohad Ben-Cohen, Rob Herring,
open list:REMOTE PROCESSOR (REMOTEPROC) SUBSYSTEM,
Matthias Brugger, moderated list:ARM/Mediatek SoC support,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
On Fri, Jan 8, 2021 at 2:02 AM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> On Wed 06 Jan 19:50 CST 2021, Tzung-Bi Shih wrote:
>
> > On Thu, Jan 7, 2021 at 7:15 AM Mathieu Poirier
> > <mathieu.poirier@linaro.org> wrote:
> > >
> > > > static void mt8183_scp_stop(struct mtk_scp *scp)
> > > > {
> > > > /* Disable SCP watchdog */
> > > > @@ -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.
> >
>
> 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.
Will do but what if we need to access the "res"?
For example:
scp->sram_size = resource_size(res);
By using devm_platform_ioremap_resource_byname() which will drop the
access to res. Shall we add a new variant of
devm_platform_ioremap_resource_byname()?
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] remoteproc/mediatek: support L1TCM
@ 2021-01-08 3:50 ` Tzung-Bi Shih
0 siblings, 0 replies; 18+ messages in thread
From: Tzung-Bi Shih @ 2021-01-08 3:50 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Ohad Ben-Cohen,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Mathieu Poirier,
open list:REMOTE PROCESSOR (REMOTEPROC) SUBSYSTEM, Rob Herring,
moderated list:ARM/Mediatek SoC support, Matthias Brugger
On Fri, Jan 8, 2021 at 2:02 AM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> On Wed 06 Jan 19:50 CST 2021, Tzung-Bi Shih wrote:
>
> > On Thu, Jan 7, 2021 at 7:15 AM Mathieu Poirier
> > <mathieu.poirier@linaro.org> wrote:
> > >
> > > > static void mt8183_scp_stop(struct mtk_scp *scp)
> > > > {
> > > > /* Disable SCP watchdog */
> > > > @@ -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.
> >
>
> 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.
Will do but what if we need to access the "res"?
For example:
scp->sram_size = resource_size(res);
By using devm_platform_ioremap_resource_byname() which will drop the
access to res. Shall we add a new variant of
devm_platform_ioremap_resource_byname()?
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2021-01-08 3:51 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-14 5:05 [PATCH 0/2] remoteproc/mediatek: support L1TCM for MT8192 SCP Tzung-Bi Shih
2020-12-14 5:05 ` Tzung-Bi Shih
2020-12-14 5:05 ` [PATCH 1/2] dt-bindings: remoteproc: mediatek: add L1TCM memory region Tzung-Bi Shih
2020-12-14 5:05 ` Tzung-Bi Shih
2020-12-15 17:12 ` Rob Herring
2020-12-15 17:12 ` Rob Herring
2020-12-14 5:05 ` [PATCH 2/2] remoteproc/mediatek: support L1TCM Tzung-Bi Shih
2020-12-14 5:05 ` Tzung-Bi Shih
2021-01-06 23:15 ` Mathieu Poirier
2021-01-06 23:15 ` Mathieu Poirier
2021-01-07 1:50 ` Tzung-Bi Shih
2021-01-07 1:50 ` Tzung-Bi Shih
2021-01-07 17:54 ` Mathieu Poirier
2021-01-07 17:54 ` Mathieu Poirier
2021-01-07 18:02 ` Bjorn Andersson
2021-01-07 18:02 ` Bjorn Andersson
2021-01-08 3:50 ` Tzung-Bi Shih
2021-01-08 3:50 ` Tzung-Bi Shih
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.