[PATCH v2 0/2] Support multiple reserved memory regions

[PATCH v2 0/2] Support multiple reserved memory regions

[Shun-yi Wang]

From: "shun-yi.wang" <shun-yi.wang@mediatek.com>

Besides the reserved memory region for SCP, there are additional 
reserved memory regions for specific hardware use.
Currently, only a single memory region is supported.
Modifications are made to support multiple memory regions.

Changes in v2:
 - Modify description of memory region in dt-bindings document
 - Fix comments in v1, initial value and from '!i' 'i == 0'
 - Link to v1: https://lore.kernel.org/all/20240703115308.17436-1-shun-yi.wang@mediatek.com

shun-yi.wang (2):
  dt-bindings: remoteproc: Support multiple reserved memory regions
  remoteproc: mediatek: Support multiple reserved memory regions

 .../bindings/remoteproc/mtk,scp.yaml          |  8 ++++--
 drivers/remoteproc/mtk_scp.c                  | 27 ++++++++++++-------
 2 files changed, 24 insertions(+), 11 deletions(-)

-- 
2.18.0

[PATCH v2 1/2] dt-bindings: remoteproc: Support multiple reserved memory regions

[Shun-yi Wang]

From: "shun-yi.wang" <shun-yi.wang@mediatek.com>

Remove the maximum number of 1 for memory regions.
Instead, add some descriptions to ensure the integrity
of the documentation.

Signed-off-by: shun-yi.wang <shun-yi.wang@mediatek.com>
---
 Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
index d05d1563ec19..3362c8ffdccc 100644
--- a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
@@ -55,7 +55,9 @@ properties:
       initializing SCP.
 
   memory-region:
-    maxItems: 1
+    description:
+      List of phandles to the reserved memory nodes used by
+      remoteproc devices.
 
   cros-ec-rpmsg:
     $ref: /schemas/mfd/google,cros-ec.yaml
@@ -123,7 +125,9 @@ patternProperties:
           initializing sub cores of multi-core SCP.
 
       memory-region:
-        maxItems: 1
+        description:
+          List of phandles to the reserved memory nodes used by
+          remoteproc devices.
 
       cros-ec-rpmsg:
         $ref: /schemas/mfd/google,cros-ec.yaml
-- 
2.18.0

Re: [PATCH v2 1/2] dt-bindings: remoteproc: Support multiple reserved memory regions

[Krzysztof Kozlowski]

On 31/07/2024 14:17, Shun-yi Wang wrote:
> From: "shun-yi.wang" <shun-yi.wang@mediatek.com>
> 
> Remove the maximum number of 1 for memory regions.

Why?

> Instead, add some descriptions to ensure the integrity
> of the documentation.

What? How is this related?

> 
> Signed-off-by: shun-yi.wang <shun-yi.wang@mediatek.com>
> ---
>  Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> index d05d1563ec19..3362c8ffdccc 100644
> --- a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> @@ -55,7 +55,9 @@ properties:
>        initializing SCP.
>  
>    memory-region:
> -    maxItems: 1

No, no, no. Bindings must be specific/constrainted.

> +    description:
> +      List of phandles to the reserved memory nodes used by
> +      remoteproc devices.

No, drop, it's entirely redundant and pointless. You did not add any new
information. This is always a list, always phandles and always reserved
memory regions. So what does it bring?

Please do not upstream random junk from your downstream kernel. :(

Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-bindings: remoteproc: Support multiple reserved memory regions

[Shun-Yi Wang (王順億)]

Hi Krzysztof,

Thanks for the reviews.

On Wed, 2024-07-31 at 14:40 +0200, Krzysztof Kozlowski wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  On 31/07/2024 14:17, Shun-yi Wang wrote:
> > From: "shun-yi.wang" <shun-yi.wang@mediatek.com>
> > 
> > Remove the maximum number of 1 for memory regions.
> 
> Why?
> 

For future applications, MTK SCP will reserve multiple regions for
specific hardware use.

> > Instead, add some descriptions to ensure the integrity
> > of the documentation.
> 
> What? How is this related?
> 

My original thinking was to keep the memory-region option.
But currently, there is no maximum value limitation, so I
add some description. Should I just drop the description directly?

Best regards,
Shun-yi

> > 
> > Signed-off-by: shun-yi.wang <shun-yi.wang@mediatek.com>
> > ---
> >  Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml | 8
> ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git
> a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> > index d05d1563ec19..3362c8ffdccc 100644
> > --- a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> > +++ b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml
> > @@ -55,7 +55,9 @@ properties:
> >        initializing SCP.
> >  
> >    memory-region:
> > -    maxItems: 1
> 
> No, no, no. Bindings must be specific/constrainted.
> 
> > +    description:
> > +      List of phandles to the reserved memory nodes used by
> > +      remoteproc devices.
> 
> No, drop, it's entirely redundant and pointless. You did not add any
> new
> information. This is always a list, always phandles and always
> reserved
> memory regions. So what does it bring?
> 
> Please do not upstream random junk from your downstream kernel. :(
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v2 1/2] dt-bindings: remoteproc: Support multiple reserved memory regions

[Krzysztof Kozlowski]

On 31/07/2024 15:41, Shun-Yi Wang (王順億) wrote:
> Hi Krzysztof,
> 
> Thanks for the reviews.
> 
> On Wed, 2024-07-31 at 14:40 +0200, Krzysztof Kozlowski wrote:
>>  	 
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>>  On 31/07/2024 14:17, Shun-yi Wang wrote:
>>> From: "shun-yi.wang" <shun-yi.wang@mediatek.com>
>>>
>>> Remove the maximum number of 1 for memory regions.
>>
>> Why?
>>
> 
> For future applications, MTK SCP will reserve multiple regions for
> specific hardware use.

That's not a reason to drop constrain on an entry.

> 
>>> Instead, add some descriptions to ensure the integrity
>>> of the documentation.
>>
>> What? How is this related?
>>
> 
> My original thinking was to keep the memory-region option.
> But currently, there is no maximum value limitation, so I
> add some description. Should I just drop the description directly?

Read all comments.

Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-bindings: remoteproc: Support multiple reserved memory regions

[Krzysztof Kozlowski]

On 31/07/2024 15:54, Krzysztof Kozlowski wrote:
> On 31/07/2024 15:41, Shun-Yi Wang (王順億) wrote:
>> Hi Krzysztof,
>>
>> Thanks for the reviews.
>>
>> On Wed, 2024-07-31 at 14:40 +0200, Krzysztof Kozlowski wrote:
>>>  	 
>>> External email : Please do not click links or open attachments until
>>> you have verified the sender or the content.
>>>  On 31/07/2024 14:17, Shun-yi Wang wrote:
>>>> From: "shun-yi.wang" <shun-yi.wang@mediatek.com>
>>>>
>>>> Remove the maximum number of 1 for memory regions.
>>>
>>> Why?
>>>
>>
>> For future applications, MTK SCP will reserve multiple regions for
>> specific hardware use.
> 
> That's not a reason to drop constrain on an entry.

Maybe I was not that clear, so to explain more - entries must be
constrained, so provide widest constraints in top-level properties place
and narrow the constrains per variant in allOf:if:then: like:

https://elixir.bootlin.com/linux/v6.8/source/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml#L132

Best regards,
Krzysztof

[PATCH v2 2/2] remoteproc: mediatek: Support multiple reserved memory regions

[Shun-yi Wang]

From: "shun-yi.wang" <shun-yi.wang@mediatek.com>

SCP supports multiple reserved memory regions, intended for
specific hardwards.

Signed-off-by: shun-yi.wang <shun-yi.wang@mediatek.com>
---
 drivers/remoteproc/mtk_scp.c | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
index 857a61760b27..0e799d0bf73d 100644
--- a/drivers/remoteproc/mtk_scp.c
+++ b/drivers/remoteproc/mtk_scp.c
@@ -1006,20 +1006,29 @@ EXPORT_SYMBOL_GPL(scp_mapping_dm_addr);
 
 static int scp_map_memory_region(struct mtk_scp *scp)
 {
-	int ret;
 	const struct mtk_scp_sizes_data *scp_sizes;
+	struct device_node *node = scp->dev->of_node;
+	struct of_phandle_iterator it;
+	int ret, err;
+	int i = 0;
 
-	ret = of_reserved_mem_device_init(scp->dev);
+	of_for_each_phandle(&it, err, node, "memory-region", NULL, 0) {
+		ret = of_reserved_mem_device_init_by_idx(scp->dev, node, i);
 
-	/* reserved memory is optional. */
-	if (ret == -ENODEV) {
-		dev_info(scp->dev, "skipping reserved memory initialization.");
-		return 0;
+		if (ret) {
+			dev_err(scp->dev, "failed to assign memory-region: %s\n",
+				it.node->name);
+			of_node_put(it.node);
+			return -ENOMEM;
+		}
+
+		i++;
 	}
 
-	if (ret) {
-		dev_err(scp->dev, "failed to assign memory-region: %d\n", ret);
-		return -ENOMEM;
+	/* reserved memory is optional. */
+	if (i == 0) {
+		dev_dbg(scp->dev, "skipping reserved memory initialization.");
+		return 0;
 	}
 
 	/* Reserved SCP code size */
-- 
2.18.0