On Wed, Jul 24, 2024 at 4:05 AM Krzysztof Kozlowski <krzk@xxxxxxxxxx> wrote: > > On 23/07/2024 20:44, 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> > >> > >>> 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. > > > > > > Hello Krzysztof, > > > > At this commit there are two resets; the 4908 requires "perst" and the > > 7216 requires "rescal". I now think what you are looking for is the > > top-level > > description of something like > > > > resets: > > maxItems: 1 > > oneOf: > > - description: reset controller handling the PERST# signal > > - description: phandle pointing to the RESCAL reset controller > > Now tell me, what sort of new information comes with this description? > "Phandle"? It cannot be anything else. Redundant. "Pointing to"? > Redundant. "reset-controller"? Well, resets always point to reset > controller. > > So what is the point of this description? Any point? > > > > > reset-names: > > maxItems: 1 > > oneOf: > > - const: perst > > - const: rescal > > > > I left out minItems because imItems==maxItems=1 > > > > Before I was giving both of them as the "potential candidates list" > > that will be used further on, but this is not how Yaml should be used. > > > > Is the above in the right direction? > > It's over complicated. First maxItems are redundant, because you define > number of items in items. Second, you have EXACTLY the same case as the > hardware for which I gave you bindings to use. I don't understand why > you insist on doing things differently, but you can. Take a look at many > other bindings how they achieve this - there are many, many examples. > But do not invent third or fourth method... ack, I will follow the qcom,ufs.yaml file you referenced. > > Best regards, > Krzysztof >
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature
