On Wed, Jul 17, 2024 at 9:30 AM Krzysztof Kozlowski <krzk@xxxxxxxxxx> wrote: > > On 17/07/2024 15:20, Jim Quinlan wrote: > > On Wed, Jul 17, 2024 at 2:51 AM Krzysztof Kozlowski <krzk@xxxxxxxxxx> wrote: > >> > >> On 16/07/2024 23:31, Jim Quinlan wrote: > >>> o Change order of the compatible strings to be alphabetical > >>> > >>> o Describe resets/reset-names before using them in rules > >>> > >> > >> <form letter> > >> This is a friendly reminder during the review process. > >> > >> It seems my or other reviewer's previous comments were not fully > >> addressed. Maybe the feedback got lost between the quotes, maybe you > >> just forgot to apply it. Please go back to the previous discussion and > >> either implement all requested changes or keep discussing them. > >> > >> Thank you. > >> </form letter> > > > > I'm sorry Krzysztof, but AFAICT I paid attention to all of your comments and > > tried to answer them as best as I could. Please do not resort to a > > form letter; if > > you think I missed something(s) please oblige me and say what it is rather than > > having me search for something that you know and I do not. > > I do not see your response at all to my comments on patch #2. > > > > >> > >>> o Add minItems/maxItems where needed. > >>> > >>> o Change maintainer: Nicolas has not been active for a while. It also > >>> makes sense for a Broadcom employee to be the maintainer as many of the > >>> details are privy to Broadcom. > >>> > >>> Signed-off-by: Jim Quinlan <james.quinlan@xxxxxxxxxxxx> > >>> --- > >>> .../bindings/pci/brcm,stb-pcie.yaml | 26 ++++++++++++++----- > >>> 1 file changed, 19 insertions(+), 7 deletions(-) > >>> > >>> diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml > >>> index 11f8ea33240c..692f7ed7c98e 100644 > >>> --- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml > >>> +++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml > >>> @@ -7,7 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml# > >>> title: Brcmstb PCIe Host Controller > >>> > >>> maintainers: > >>> - - Nicolas Saenz Julienne <nsaenzjulienne@xxxxxxx> > >>> + - Jim Quinlan <james.quinlan@xxxxxxxxxxxx> > >>> > >>> properties: > >>> compatible: > >>> @@ -16,11 +16,11 @@ properties: > >>> - brcm,bcm2711-pcie # The Raspberry Pi 4 > >>> - brcm,bcm4908-pcie > >>> - brcm,bcm7211-pcie # Broadcom STB version of RPi4 > >>> - - brcm,bcm7278-pcie # Broadcom 7278 Arm > >>> - brcm,bcm7216-pcie # Broadcom 7216 Arm > >>> - - brcm,bcm7445-pcie # Broadcom 7445 Arm > >>> + - brcm,bcm7278-pcie # Broadcom 7278 Arm > >>> - brcm,bcm7425-pcie # Broadcom 7425 MIPs > >>> - brcm,bcm7435-pcie # Broadcom 7435 MIPs > >>> + - brcm,bcm7445-pcie # Broadcom 7445 Arm > >>> > >>> reg: > >>> maxItems: 1 > >>> @@ -95,6 +95,18 @@ properties: > >>> minItems: 1 > >>> maxItems: 3 > >>> > >>> + resets: > >>> + minItems: 1 > >>> + items: > >>> + - description: reset for external PCIe PERST# signal # perst > >>> + - description: reset for phy reset calibration # rescal > >>> + > >>> + reset-names: > >>> + minItems: 1 > >>> + items: > >>> + - const: perst > >>> + - const: rescal > >> > >> There are no devices with two resets. Anyway, this does not match one of > >> your variants which have first element as rescal. > > > > The 4908 chip exclusively uses the "perst" reset, and the 7216 chip > > exclusively uses the "rescal" reset. The rest of the chips use zero > > resets. All together, there are two resets. > > This is not enum, but a list. What you do mean overall two resets? You > have a chip which is both 4908 and 7216 at the same time? How is this > possible? > > > > > You are the one that wanted me to first list all resets at the top, > > and refer to them by the conditional rules. > > No, I wanted widest constraints at the top. Can you explain what you mean by "widest constraint"? I did not see the word "wide" in any of the Linux YAML documentation. > > My comment at v2 was already saying this: > > "This does not match what you have in conditional, so just keep min and > max Items here." > Okay, what I was referring to was your V1 response: "Fix the binding first - properties should be defined in top level "properties:" and then customized." I think this is correct but I have not been describing the reset/reset-names properties properly at the top level; i.e. I have been giving a full list instead of using "oneOf". Regards, Jim Quinlan Broadcom STB/CM > and you have numerous examples in the code for this. > > Best regards, > Krzysztof >
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature
