Message-ID: <CAGt4E5taqv2kgK4ZF6R+otKwrVYiaNNkeCu5Qv7xW8cyBvv3Fw@mail.gmail.com>
Date: Thu, 1 Feb 2024 14:36:13 -0800
From: Markus Mayer <mmayer@...adcom.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc: Florian Fainelli <florian.fainelli@...adcom.com>, Rob Herring <robh+dt@...nel.org>, 
	Conor Dooley <conor+dt@...nel.org>, 
	Linux ARM Kernel List <linux-arm-kernel@...ts.infradead.org>, 
	Device Tree Mailing List <devicetree@...r.kernel.org>, Linux Kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 3/5] dt-bindings: memory: additional compatible strings
 for Broadcom DPFE

On Tue, 23 Jan 2024 at 13:27, Krzysztof Kozlowski
<krzysztof.kozlowski@...aro.org> 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@...adcom.com>
> > ---
> >  .../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@...nel.org>
> >    - Markus Mayer <mmayer@...adcom.com>
> >
> > +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?

Which concerns, specifically, are you referencing? I promise I am not
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.

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@linaro.org/

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.
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?

> so with ignored comments: NAK

Like I said before, it is not clear to me what you are looking for.
I'll gladly address any concerns as much as possible.

Regards,
-Markus

Powered by blists - more mailing lists