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. 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." and you have numerous examples in the code for this. Best regards, Krzysztof
