[v2] Support multiple reserved memory regions

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

Posted by Shun-yi Wang 1 month, 2 weeks ago

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

Posted by Krzysztof Kozlowski 1 month, 2 weeks ago

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

Posted by Shun-Yi Wang (王順億) 1 month, 2 weeks ago

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

Posted by Krzysztof Kozlowski 1 month, 2 weeks ago

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

Posted by Krzysztof Kozlowski 1 month, 2 weeks ago

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