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

["Markus Schneider-Pargmann" <msp@baylibre.com>]

[-- Attachment #1: Type: text/plain, Size: 2995 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 289 bytes --]