[PATCH v2 8/8] dt-bindings: remoteproc: k3-r5f: Require memory-region-names

Markus Schneider-Pargmann (TI) posted 8 patches 3 weeks, 4 days ago
There is a newer version of this series
[PATCH v2 8/8] dt-bindings: remoteproc: k3-r5f: Require memory-region-names
Posted by Markus Schneider-Pargmann (TI) 3 weeks, 4 days ago
If memory-region is used, require memory-region-names.

Signed-off-by: Markus Schneider-Pargmann (TI) <msp@baylibre.com>
---
 Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml
index 3f2425e0880f9a516ac10700a218ed035ff07d5a..775e9b3a193878349590c5036aa884617ebbcc9f 100644
--- a/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml
@@ -245,6 +245,13 @@ patternProperties:
       - resets
       - firmware-name
 
+    if:
+      required:
+        - memory-region
+    then:
+      required:
+        - memory-region-names
+
     unevaluatedProperties: false
 
 allOf:

-- 
2.53.0
Re: [PATCH v2 8/8] dt-bindings: remoteproc: k3-r5f: Require memory-region-names
Posted by Krzysztof Kozlowski 3 weeks, 4 days ago
On Thu, Mar 12, 2026 at 04:49:02PM +0100, Markus Schneider-Pargmann (TI) wrote:
> If memory-region is used, require memory-region-names.

Why?

I don't understand also why this is a separate change, but maybe answer
to "Why are you doing it" would cover it as well.

> 
> Signed-off-by: Markus Schneider-Pargmann (TI) <msp@baylibre.com>
> ---
>  Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
> 

Best regards,
Krzysztof
Re: [PATCH v2 8/8] dt-bindings: remoteproc: k3-r5f: Require memory-region-names
Posted by Markus Schneider-Pargmann 3 weeks, 4 days ago
Hi Krzysztof,

On Fri Mar 13, 2026 at 2:13 PM CET, Krzysztof Kozlowski wrote:
> On Thu, Mar 12, 2026 at 04:49:02PM +0100, Markus Schneider-Pargmann (TI) wrote:
>> If memory-region is used, require memory-region-names.
>
> Why?

This was a suggestion/comment from Conor in the last version:

    Is this really optional? Shouldn't it be made mandatory so that it is
    easy to tell the difference between the two configurations?

https://lore.kernel.org/all/20260303-hesitate-preoccupy-5e311cbd3e58@spud/

>
> I don't understand also why this is a separate change, but maybe answer
> to "Why are you doing it" would cover it as well.

I made this a separate patch so the git tree never has any
binding/devicectree warnings for memory-region-names even in-between
patches. That's why I created these patches in this order:

1. Add the memory-region-names as an optional property.
2. Add memory-region-names to all users of memory-region.
3. Make the property required if memory-region exists.

Best
Markus
Re: [PATCH v2 8/8] dt-bindings: remoteproc: k3-r5f: Require memory-region-names
Posted by Krzysztof Kozlowski 3 weeks, 3 days ago
On 13/03/2026 14:38, Markus Schneider-Pargmann wrote:
> Hi Krzysztof,
> 
> On Fri Mar 13, 2026 at 2:13 PM CET, Krzysztof Kozlowski wrote:
>> On Thu, Mar 12, 2026 at 04:49:02PM +0100, Markus Schneider-Pargmann (TI) wrote:
>>> If memory-region is used, require memory-region-names.
>>
>> Why?
> 
> This was a suggestion/comment from Conor in the last version:
> 
>     Is this really optional? Shouldn't it be made mandatory so that it is
>     easy to tell the difference between the two configurations?

Then write it in commit msg. You have entire commit msg to explain why
you are doing things, instead of obvious what. We can read the diff.

> 
> https://lore.kernel.org/all/20260303-hesitate-preoccupy-5e311cbd3e58@spud/
> 
>>
>> I don't understand also why this is a separate change, but maybe answer
>> to "Why are you doing it" would cover it as well.
> 
> I made this a separate patch so the git tree never has any
> binding/devicectree warnings for memory-region-names even in-between
> patches. That's why I created these patches in this order:
> 
> 1. Add the memory-region-names as an optional property.
> 2. Add memory-region-names to all users of memory-region.

So what is the point of this if it is optional? IOW, what does this
commit achieve? Almost nothing.

> 3. Make the property required if memory-region exists.

but only required here? You need to organize your work in logical hunks.

> Markus


Best regards,
Krzysztof
Re: [PATCH v2 8/8] dt-bindings: remoteproc: k3-r5f: Require memory-region-names
Posted by Conor Dooley 3 weeks, 3 days ago
On Fri, Mar 13, 2026 at 04:49:14PM +0100, Krzysztof Kozlowski wrote:
> On 13/03/2026 14:38, Markus Schneider-Pargmann wrote:
> > Hi Krzysztof,
> > 
> > On Fri Mar 13, 2026 at 2:13 PM CET, Krzysztof Kozlowski wrote:
> >> On Thu, Mar 12, 2026 at 04:49:02PM +0100, Markus Schneider-Pargmann (TI) wrote:
> >>> If memory-region is used, require memory-region-names.
> >>
> >> Why?
> > 
> > This was a suggestion/comment from Conor in the last version:
> > 
> >     Is this really optional? Shouldn't it be made mandatory so that it is
> >     easy to tell the difference between the two configurations?
> 
> Then write it in commit msg. You have entire commit msg to explain why
> you are doing things, instead of obvious what. We can read the diff.
> 
> > 
> > https://lore.kernel.org/all/20260303-hesitate-preoccupy-5e311cbd3e58@spud/
> > 
> >>
> >> I don't understand also why this is a separate change, but maybe answer
> >> to "Why are you doing it" would cover it as well.
> > 
> > I made this a separate patch so the git tree never has any
> > binding/devicectree warnings for memory-region-names even in-between
> > patches. That's why I created these patches in this order:
> > 
> > 1. Add the memory-region-names as an optional property.
> > 2. Add memory-region-names to all users of memory-region.
> 
> So what is the point of this if it is optional? IOW, what does this
> commit achieve? Almost nothing.
> 
> > 3. Make the property required if memory-region exists.
> 
> but only required here? You need to organize your work in logical hunks.

My rationale for my original request was that the meaning of the second
memory region is modified by this series. Previously it was always
"firmware image sections", but now it can also be "IPC resources".
Nothing changed in terms of the number of memory regions (it was 2-8
before and 2-8 after), so without making memory-region-names mandatory,
there'd be no way to tell which of the two configurations are being
used.

This patch should likely be squashed with the patch adding
memory-region-names, so that it is easily to provide an explanation for
what's going on.
Re: [PATCH v2 8/8] dt-bindings: remoteproc: k3-r5f: Require memory-region-names
Posted by Markus Schneider-Pargmann 3 weeks, 3 days ago
Hi,

On Fri Mar 13, 2026 at 5:18 PM CET, Conor Dooley wrote:
> On Fri, Mar 13, 2026 at 04:49:14PM +0100, Krzysztof Kozlowski wrote:
>> On 13/03/2026 14:38, Markus Schneider-Pargmann wrote:
>> > Hi Krzysztof,
>> > 
>> > On Fri Mar 13, 2026 at 2:13 PM CET, Krzysztof Kozlowski wrote:
>> >> On Thu, Mar 12, 2026 at 04:49:02PM +0100, Markus Schneider-Pargmann (TI) wrote:
>> >>> If memory-region is used, require memory-region-names.
>> >>
>> >> Why?
>> > 
>> > This was a suggestion/comment from Conor in the last version:
>> > 
>> >     Is this really optional? Shouldn't it be made mandatory so that it is
>> >     easy to tell the difference between the two configurations?
>> 
>> Then write it in commit msg. You have entire commit msg to explain why
>> you are doing things, instead of obvious what. We can read the diff.
>> 
>> > 
>> > https://lore.kernel.org/all/20260303-hesitate-preoccupy-5e311cbd3e58@spud/
>> > 
>> >>
>> >> I don't understand also why this is a separate change, but maybe answer
>> >> to "Why are you doing it" would cover it as well.
>> > 
>> > I made this a separate patch so the git tree never has any
>> > binding/devicectree warnings for memory-region-names even in-between
>> > patches. That's why I created these patches in this order:
>> > 
>> > 1. Add the memory-region-names as an optional property.
>> > 2. Add memory-region-names to all users of memory-region.
>> 
>> So what is the point of this if it is optional? IOW, what does this
>> commit achieve? Almost nothing.
>> 
>> > 3. Make the property required if memory-region exists.
>> 
>> but only required here? You need to organize your work in logical hunks.
>
> My rationale for my original request was that the meaning of the second
> memory region is modified by this series. Previously it was always
> "firmware image sections", but now it can also be "IPC resources".
> Nothing changed in terms of the number of memory regions (it was 2-8
> before and 2-8 after), so without making memory-region-names mandatory,
> there'd be no way to tell which of the two configurations are being
> used.
>
> This patch should likely be squashed with the patch adding
> memory-region-names, so that it is easily to provide an explanation for
> what's going on.

My goal was to not introduce any warnings in any of the patches.

That is the reason why I only added the requirement for
memory-region-names at the end, after adding memory-region-names to all
users.

The alternative patch order as you suggest is:
1. Introduce required memory-region-names
2. Add memory-region-names to all users

After patch 1 there will be new warnings about memory-region-names
missing for every user of r5f memory-region until patch 2 is applied. I
can happily squash this patch into the patch introducing
memory-region-names. I can also update the commit message to describe
why I split the patches this way.

Let me know what you prefer.

Best
Markus
Re: [PATCH v2 8/8] dt-bindings: remoteproc: k3-r5f: Require memory-region-names
Posted by Conor Dooley 3 weeks, 2 days ago
On Sat, Mar 14, 2026 at 03:28:25PM +0100, Markus Schneider-Pargmann wrote:
> Hi,
> 
> On Fri Mar 13, 2026 at 5:18 PM CET, Conor Dooley wrote:
> > On Fri, Mar 13, 2026 at 04:49:14PM +0100, Krzysztof Kozlowski wrote:
> >> On 13/03/2026 14:38, Markus Schneider-Pargmann wrote:
> >> > Hi Krzysztof,
> >> > 
> >> > On Fri Mar 13, 2026 at 2:13 PM CET, Krzysztof Kozlowski wrote:
> >> >> On Thu, Mar 12, 2026 at 04:49:02PM +0100, Markus Schneider-Pargmann (TI) wrote:
> >> >>> If memory-region is used, require memory-region-names.
> >> >>
> >> >> Why?
> >> > 
> >> > This was a suggestion/comment from Conor in the last version:
> >> > 
> >> >     Is this really optional? Shouldn't it be made mandatory so that it is
> >> >     easy to tell the difference between the two configurations?
> >> 
> >> Then write it in commit msg. You have entire commit msg to explain why
> >> you are doing things, instead of obvious what. We can read the diff.
> >> 
> >> > 
> >> > https://lore.kernel.org/all/20260303-hesitate-preoccupy-5e311cbd3e58@spud/
> >> > 
> >> >>
> >> >> I don't understand also why this is a separate change, but maybe answer
> >> >> to "Why are you doing it" would cover it as well.
> >> > 
> >> > I made this a separate patch so the git tree never has any
> >> > binding/devicectree warnings for memory-region-names even in-between
> >> > patches. That's why I created these patches in this order:
> >> > 
> >> > 1. Add the memory-region-names as an optional property.
> >> > 2. Add memory-region-names to all users of memory-region.
> >> 
> >> So what is the point of this if it is optional? IOW, what does this
> >> commit achieve? Almost nothing.
> >> 
> >> > 3. Make the property required if memory-region exists.
> >> 
> >> but only required here? You need to organize your work in logical hunks.
> >
> > My rationale for my original request was that the meaning of the second
> > memory region is modified by this series. Previously it was always
> > "firmware image sections", but now it can also be "IPC resources".
> > Nothing changed in terms of the number of memory regions (it was 2-8
> > before and 2-8 after), so without making memory-region-names mandatory,
> > there'd be no way to tell which of the two configurations are being
> > used.
> >
> > This patch should likely be squashed with the patch adding
> > memory-region-names, so that it is easily to provide an explanation for
> > what's going on.
> 
> My goal was to not introduce any warnings in any of the patches.
> 
> That is the reason why I only added the requirement for
> memory-region-names at the end, after adding memory-region-names to all
> users.
> 
> The alternative patch order as you suggest is:
> 1. Introduce required memory-region-names
> 2. Add memory-region-names to all users
> 
> After patch 1 there will be new warnings about memory-region-names
> missing for every user of r5f memory-region until patch 2 is applied. I
> can happily squash this patch into the patch introducing
> memory-region-names. I can also update the commit message to describe
> why I split the patches this way.
> 
> Let me know what you prefer.

Personally, I don't think that transient warnings that won't appear in
linux-next (just in the individual trees) are worth splitting for, when
the split is artificial and goes counter to explaining the motivation.