| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CA+-6iNzZXF+yTmjMZ0SU0jO4L0ZzETZ-VvQpe4txv=SNTyo_bA@mail.gmail.com>
Date: Fri, 12 Jul 2024 15:54:11 -0400
From: Jim Quinlan <james.quinlan@...adcom.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: linux-pci@...r.kernel.org, Nicolas Saenz Julienne <nsaenz@...nel.org>,
Bjorn Helgaas <bhelgaas@...gle.com>, Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
Cyril Brulebois <kibi@...ian.org>, Stanimir Varbanov <svarbanov@...e.de>,
bcm-kernel-feedback-list@...adcom.com, jim2101024@...il.com,
Florian Fainelli <florian.fainelli@...adcom.com>, Lorenzo Pieralisi <lpieralisi@...nel.org>,
Krzysztof Wilczyński <kw@...ux.com>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
"moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" <linux-rpi-kernel@...ts.infradead.org>,
"moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" <linux-arm-kernel@...ts.infradead.org>,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" <devicetree@...r.kernel.org>, open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 01/12] dt-bindings: PCI: Add Broadcom STB 7712 SOC,
update maintainer
On Thu, Jul 4, 2024 at 2:40 AM Krzysztof Kozlowski <krzk@...nel.org> wrote:
>
> On 03/07/2024 20:02, Jim Quinlan wrote:
> > - Update maintainer; Nicolas hasn't been active and it
> > makes more sense to have a Broadcom maintainer
> > - Add a driver compatible string for the new STB SOC 7712
>
> You meant device? Bindings are for hardware.
Hello Krzysztof,
I should have replied to this before sending out V3. Since your form
letter says I did not address previous comments, I will address them
here and now (your v2 review of the bindings commit).
>
> > - Add two new resets for the 7712: "bridge", for the
> > the bridge between the PCIe core and the memory bus;
> > "swinit", the PCIe core reset.
> > - Order the compatible strings alphabetically
> > - Restructure the reset controllers so that the definitions
> > appear first before any rules that govern them.
>
> Please split cleanups from new device support.
Okay.
>
> >
> > Signed-off-by: Jim Quinlan <james.quinlan@...adcom.com>
> > ---
> > .../bindings/pci/brcm,stb-pcie.yaml | 44 +++++++++++++++----
> > 1 file changed, 36 insertions(+), 8 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> > index 11f8ea33240c..a070f35d28d7 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@...e.de>
> > + - Jim Quinlan <james.quinlan@...adcom.com>
> >
> > properties:
> > compatible:
> > @@ -16,11 +16,12 @@ 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
> > + - brcm,bcm7712-pcie # STB sibling SOC of Raspberry Pi 5
> >
> > reg:
> > maxItems: 1
> > @@ -95,6 +96,20 @@ properties:
> > minItems: 1
> > maxItems: 3
> >
> > + resets:
> > + items:
> > + - description: reset for phy calibration
> > + - description: reset for PCIe/CPU bus bridge
> > + - description: reset for soft PCIe core reset
> > + - description: reset for PERST# PCIe signal
>
> This won't work and I doubt you tested your code. You miss minItems.
>
> > +
> > + reset-names:
> > + items:
> > + - const: rescal
> > + - const: bridge
> > + - const: swinit
> > + - const: perst
>
> This does not match what you have in conditional, so just keep min and
> max Items here.
I do not understand. There are four possible resets, but any one chip
uses only 0, 1, or 3 of them:
CHIP NUM_RESETS NAMES
==== ========== =====
4908 1 perst
7216 1 rescal
7712 3 rescal, bridge, swinit
Other_Chips 0 -
Although I list four "reset-names", I have, in the rule for 7712,
maxItems=3 because it only uses rescal, bridge, and swinit. So I
don't know what you mean when you say "this does not match what you
have in your conditional". AFAICT, they are not supposed to match.
>
>
> > +
> > required:
> > - compatible
> > - reg
> > @@ -118,13 +133,10 @@ allOf:
> > then:
> > properties:
> > resets:
> > - items:
> > - - description: reset controller handling the PERST# signal
> > -
> > + minItems: 1
>
> maxItems instead. Why three resets should be valid?
For the "4908" conditional, minItems==maxItems==1. I do not
understand your question "Why three resets should be valid" -- can you
please elaborate?
>
>
> > reset-names:
> > items:
> > - const: perst
> > -
> > required:
> > - resets
> > - reset-names
> > @@ -136,12 +148,28 @@ allOf:
> > then:
> > properties:
> > resets:
> > + minItems: 1
> > + reset-names:
> > items:
> > - - description: phandle pointing to the RESCAL reset controller
> > + - const: rescal
> > + required:
> > + - resets
> > + - reset-names
>
> Why?
I do not know what you are questioning. The 7216 device uses one
reset: the "rescal". Again, maxItems==minItems==1. Please see the
summary note below.
>
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: brcm,bcm7712-pcie
> > + then:
> > + properties:
> > + resets:
> > + minItems: 3
>
> Again, you do not have 4 items here.
I do not want to have 4 items here; I want to have 3 for "rescal",
"bridge," and "swinit". In this case, maxItems==minItems==3.
Now , for V1 you requested that I define all resets at the top; I've
done that and there are 4 of them. But no chip uses all 4; each
individual chip only uses 0, 1, or 3 resets.
So there is no way that each chip's conditional rule can define
minItems and maxItems to match the description list of 4 resets,
unless you want me to undo your V1 request of describing the resets at
the top level instead of describing them in the rules.
Please advise,
Regards,
Jim Quinlan
Broadcom STB/CM
>
> >
> > reset-names:
> > items:
> > - const: rescal
> > + - const: bridge
> > + - const: swinit
> >
> > required:
> > - resets
>
> Best regards,
> Krzysztof
>
Download attachment "smime.p7s" of type "application/pkcs7-signature" (4210 bytes)
Powered by blists - more mailing lists