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
signature.asc
Description: PGP signature
