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

[Conor Dooley <conor@kernel.org>]

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

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.

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