On 01/02/2024 23:36, Markus Mayer wrote: > On Tue, 23 Jan 2024 at 13:27, Krzysztof Kozlowski > <krzysztof.kozlowski@xxxxxxxxxx> wrote: >> >> On 19/01/2024 22:52, Markus Mayer wrote: >>> Add versioned compatible strings for Broadcom DPFE. These take the form >>> brcm,dpfe-cpu-v<N> where <N> is a number from 1 to 4. >>> >>> The chip-specific strings have been kept for compatibility purposes >>> (hardware is in the field). For new chips, the properly versioned >>> compatible string should be used. >>> >>> Signed-off-by: Markus Mayer <mmayer@xxxxxxxxxxxx> >>> --- >>> .../memory-controllers/brcm,dpfe-cpu.yaml | 21 ++++++++++++++++++- >>> 1 file changed, 20 insertions(+), 1 deletion(-) >>> >>> diff --git a/Documentation/devicetree/bindings/memory-controllers/brcm,dpfe-cpu.yaml b/Documentation/devicetree/bindings/memory-controllers/brcm,dpfe-cpu.yaml >>> index 3f00bc2fd3ec..42c8160d95d1 100644 >>> --- a/Documentation/devicetree/bindings/memory-controllers/brcm,dpfe-cpu.yaml >>> +++ b/Documentation/devicetree/bindings/memory-controllers/brcm,dpfe-cpu.yaml >>> @@ -10,9 +10,28 @@ maintainers: >>> - Krzysztof Kozlowski <krzk@xxxxxxxxxx> >>> - Markus Mayer <mmayer@xxxxxxxxxxxx> >>> >>> +description: | >>> + >> >> Drop blank line. > > Will do. > >>> + The DCPU (DDR PHY Front End CPU) interfaces directly with the DDR PHY >>> + chip on Broadcom STB SoCs. An API exists for other agents to retrieve >>> + or set certain DDR PHY chip parameters by JEDEC. >>> + >>> + Different, incompatible versions of this API have been created over >>> + time. The API has changed for the some chips as development progressed >>> + and features were added or changed. >>> + >>> + We rely on the boot firmware (which already knows the API version >>> + required) to populate Device Tree with the corresponding compatible >>> + string. >>> + >>> properties: >>> compatible: >>> items: >>> + - enum: >>> + - brcm,dpfe-cpu-v1 >>> + - brcm,dpfe-cpu-v2 >>> + - brcm,dpfe-cpu-v3 >>> + - brcm,dpfe-cpu-v4 >> >> I don't see my comment resolved - except more unusual order of >> compatibles - and nothing in commit msg explains my previous concerns. > > I am confused. What about ordering them in alphabetically ascending > order is unusual? Order of entire list - you just added everything to the beginning of the list. This does not make sense. > > Which concerns, specifically, are you referencing? I promise I am not That you claim here that bcm7271 is both v1, v2, v3 and v4. Nothing in the commit msg explains this. Nothing from my message here: https://lore.kernel.org/all/3fff866f-fbe8-4d23-87f3-275380adf3d4@xxxxxxxxxx/ is resolved or addressed. > deliberately ignoring concerns as there would be no point in doing so. > I have nothing to gain from it. I am trying to get code accepted into > the kernel. I am not trying to "win any battles" or "prove that I can > be more stubborn" or anything of that nature. If it is possible to > integrate concerns raised by the maintainer, I will gladly do so. And > if not, I'd like to find a way that works for both sides. > > BTW, I once used to be on the Linaro Landing Team for Broadcom. You'll > find some commits from me in the kernel that carry a linaro.org e-mail > address. Most are from over a decade ago. Yikes, time flies! > > The reason I am mentioning this is to point out that > > * I am not new to this. > * I am respecting the Open Source community and so is the rest of the team. > * We are trying to do the right thing and be "good citizens". > * We upstream whatever we can to the relevant project, not just the kernel. > * We aren't on some kind of power-trip or unwilling to listen to feedback. OK, I see you sent the almost same patch with no improvements in the code and in commit msg, so that was the assumption. I made quite clear concerns last time and asked several questions. > > I am not saying this because I think any of the above makes me special > or particularly accomplished. However, I do think that some things may > need to be clarified as there does seem to be a certain attitude at > play here, with certain assumptions, that is far from constructive. > Hopefully, this has now been cleared up, and we can move forward with > addressing the outstanding concerns regarding our DPFE compatible > strings. > >> I think my final comment was pretty clear yet ignored completely. There >> was no follow up: >> https://lore.kernel.org/all/3fff866f-fbe8-4d23-87f3-275380adf3d4@xxxxxxxxxx/ > > Unfortunately, it may not be as clear as you think. I did my best to > incorporate what I thought you meant. Clearly that didn't work out so > well. > > So, please clarify in more detail, and maybe showing some examples, > what it is you would like to see. For instance, I have no idea what > "(e.g. bcm7271 + v1 + generic fallback)" is actually supposed to mean. This means: List of three compatibles, SoC specific, your versioned one, generic compatible used as fallback. > Is that shorthand for 3 compatible strings (brcm,bcm7271-dpfe-cpu, > brcm,dpfe-cpu-v1, brcm,dpfe-cpu). If so, how is that different from > what we already had? If something is unclear in that message, please continue that message. There was no further explanation, no further comments on my email, so I assume you agree or understand it. > >> so with ignored comments: NAK Also - missing device prefix in subject: Please use subject prefixes matching the subsystem. You can get them for example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory your patch is touching. Best regards, Krzysztof
