From: Jim Quinlan <jim2101024@gmail.com> To: Stefan Wahren <stefan.wahren@i2se.com> Cc: linux-pci@vger.kernel.org, "Nicolas Saenz Julienne" <nsaenz@kernel.org>, "Bjorn Helgaas" <bhelgaas@google.com>, "Lorenzo Pieralisi" <lorenzo.pieralisi@arm.com>, "Cyril Brulebois" <kibi@debian.org>, "Phil Elwell" <phil@raspberrypi.com>, bcm-kernel-feedback-list@broadcom.com, james.quinlan@broadcom.com, "Florian Fainelli" <f.fainelli@gmail.com>, "Lorenzo Pieralisi" <lpieralisi@kernel.org>, "Krzysztof Wilczyński" <kw@linux.com>, "Rob Herring" <robh@kernel.org>, "Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>, "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" <linux-rpi-kernel@lists.infradead.org>, "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" <linux-arm-kernel@lists.infradead.org>, "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" <devicetree@vger.kernel.org>, "open list" <linux-kernel@vger.kernel.org> Subject: Re: [PATCH v1 1/3] dt-bindings: PCI: brcmstb: Add two optional props Date: Thu, 6 Apr 2023 12:58:59 -0400 [thread overview] Message-ID: <CANCKTBsLxkPb1ajACkyhJk6J1aB2iwX0oKifHkADG0fFPUqMhQ@mail.gmail.com> (raw) In-Reply-To: <d0bf241b-ead4-94b7-3f03-a26227f9eb58@i2se.com> On Thu, Apr 6, 2023 at 11:39 AM Stefan Wahren <stefan.wahren@i2se.com> wrote: > > Hi Jim, > > Am 06.04.23 um 14:46 schrieb Jim Quinlan: > > Regarding "brcm,enable-l1ss": > > > > The Broadcom STB/CM PCIe HW -- which is also used by RPi SOCs -- requires > > the driver probe to configure one of three clkreq# modes: > > > > (a) clkreq# driven by the RC > > (b) clkreq# driven by the EP for ASPM L0s, L1 > > (c) bidirectional clkreq#, as used for L1 Substates (L1SS). > > > > The HW can tell the difference between (a) and (b), but does not know > > when to configure (c). Further, the HW will cause a CPU abort on boot if > > guesses wrong regarding the need for (c). So we introduce the boolean > > "brcm,enable-l1ss" property to indicate that (c) is desired. This > > property is already present in the Raspian version of Linux, but the > > driver implementaion that will follow adds more details and discerns > > between (a) and (b). > > > > Regarding "brcm,completion-timeout-msecs" > > > > Our HW will cause a CPU abort if the L1SS exit time is longer than the > > completion abort timeout. We've been asked to make this configurable, so > > we are introducing "brcm,completion-abort-msecs". > > > > Signed-off-by: Jim Quinlan <jim2101024@gmail.com> > > --- > > .../devicetree/bindings/pci/brcm,stb-pcie.yaml | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml > > index 7e15aae7d69e..ef4ccc05b258 100644 > > --- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml > > +++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml > > @@ -64,6 +64,18 @@ properties: > > > > aspm-no-l0s: true > > > > + brcm,enable-l1ss: > > + description: Indicates that the downstream device is L1SS > > + capable and L1SS is desired, e.g. by setting > > + CONFIG_PCIEASPM_POWER_SUPERSAVE=y. Note that CLKREQ# > > not sure about this, but maybe we should avoid references to Linux > kernel config parameter in a DT binding. Since the driver already gaves > warning in case the DT parameter is present, but kernel config doesn't > fit, this should be enough. Hello Stefan, I will remove this reference. > > > + assertion to clock active must be within 400ns. > > + type: boolean > > + > > + brcm,completion-timeout-msecs: > > + description: Number of msecs before completion timeout > > + abort occurs. > > + $ref: /schemas/types.yaml#/definitions/uint32 > > According to the driver at least 0 is not allowed, maybe we should > define minimum and maximum here and let dtbs_check take care of invalid > values? I'm not sure I follow what you mean about a zero value; the property may have any value but the driver will clamp it to a minimum of ~30msec. Regardless, I can add a "minimum: 30" line to the YAML. Thanks, Jim Quinlan Broadcom STB > > Best regards > > > + > > brcm,scb-sizes: > > description: u64 giving the 64bit PCIe memory > > viewport size of a memory controller. There may be up to
WARNING: multiple messages have this Message-ID (diff)
From: Jim Quinlan <jim2101024@gmail.com> To: Stefan Wahren <stefan.wahren@i2se.com> Cc: linux-pci@vger.kernel.org, "Nicolas Saenz Julienne" <nsaenz@kernel.org>, "Bjorn Helgaas" <bhelgaas@google.com>, "Lorenzo Pieralisi" <lorenzo.pieralisi@arm.com>, "Cyril Brulebois" <kibi@debian.org>, "Phil Elwell" <phil@raspberrypi.com>, bcm-kernel-feedback-list@broadcom.com, james.quinlan@broadcom.com, "Florian Fainelli" <f.fainelli@gmail.com>, "Lorenzo Pieralisi" <lpieralisi@kernel.org>, "Krzysztof Wilczyński" <kw@linux.com>, "Rob Herring" <robh@kernel.org>, "Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>, "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" <linux-rpi-kernel@lists.infradead.org>, "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" <linux-arm-kernel@lists.infradead.org>, "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" <devicetree@vger.kernel.org>, "open list" <linux-kernel@vger.kernel.org> Subject: Re: [PATCH v1 1/3] dt-bindings: PCI: brcmstb: Add two optional props Date: Thu, 6 Apr 2023 12:58:59 -0400 [thread overview] Message-ID: <CANCKTBsLxkPb1ajACkyhJk6J1aB2iwX0oKifHkADG0fFPUqMhQ@mail.gmail.com> (raw) In-Reply-To: <d0bf241b-ead4-94b7-3f03-a26227f9eb58@i2se.com> On Thu, Apr 6, 2023 at 11:39 AM Stefan Wahren <stefan.wahren@i2se.com> wrote: > > Hi Jim, > > Am 06.04.23 um 14:46 schrieb Jim Quinlan: > > Regarding "brcm,enable-l1ss": > > > > The Broadcom STB/CM PCIe HW -- which is also used by RPi SOCs -- requires > > the driver probe to configure one of three clkreq# modes: > > > > (a) clkreq# driven by the RC > > (b) clkreq# driven by the EP for ASPM L0s, L1 > > (c) bidirectional clkreq#, as used for L1 Substates (L1SS). > > > > The HW can tell the difference between (a) and (b), but does not know > > when to configure (c). Further, the HW will cause a CPU abort on boot if > > guesses wrong regarding the need for (c). So we introduce the boolean > > "brcm,enable-l1ss" property to indicate that (c) is desired. This > > property is already present in the Raspian version of Linux, but the > > driver implementaion that will follow adds more details and discerns > > between (a) and (b). > > > > Regarding "brcm,completion-timeout-msecs" > > > > Our HW will cause a CPU abort if the L1SS exit time is longer than the > > completion abort timeout. We've been asked to make this configurable, so > > we are introducing "brcm,completion-abort-msecs". > > > > Signed-off-by: Jim Quinlan <jim2101024@gmail.com> > > --- > > .../devicetree/bindings/pci/brcm,stb-pcie.yaml | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml > > index 7e15aae7d69e..ef4ccc05b258 100644 > > --- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml > > +++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml > > @@ -64,6 +64,18 @@ properties: > > > > aspm-no-l0s: true > > > > + brcm,enable-l1ss: > > + description: Indicates that the downstream device is L1SS > > + capable and L1SS is desired, e.g. by setting > > + CONFIG_PCIEASPM_POWER_SUPERSAVE=y. Note that CLKREQ# > > not sure about this, but maybe we should avoid references to Linux > kernel config parameter in a DT binding. Since the driver already gaves > warning in case the DT parameter is present, but kernel config doesn't > fit, this should be enough. Hello Stefan, I will remove this reference. > > > + assertion to clock active must be within 400ns. > > + type: boolean > > + > > + brcm,completion-timeout-msecs: > > + description: Number of msecs before completion timeout > > + abort occurs. > > + $ref: /schemas/types.yaml#/definitions/uint32 > > According to the driver at least 0 is not allowed, maybe we should > define minimum and maximum here and let dtbs_check take care of invalid > values? I'm not sure I follow what you mean about a zero value; the property may have any value but the driver will clamp it to a minimum of ~30msec. Regardless, I can add a "minimum: 30" line to the YAML. Thanks, Jim Quinlan Broadcom STB > > Best regards > > > + > > brcm,scb-sizes: > > description: u64 giving the 64bit PCIe memory > > viewport size of a memory controller. There may be up to _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2023-04-06 16:59 UTC|newest] Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-04-06 12:46 [PATCH v1 0/3] PCI: brcmstb: Clkreq# accomodations of downstream device Jim Quinlan 2023-04-06 12:46 ` Jim Quinlan 2023-04-06 12:46 ` [PATCH v1 1/3] dt-bindings: PCI: brcmstb: Add two optional props Jim Quinlan 2023-04-06 12:46 ` Jim Quinlan 2023-04-06 15:39 ` Stefan Wahren 2023-04-06 15:39 ` Stefan Wahren 2023-04-06 16:58 ` Jim Quinlan [this message] 2023-04-06 16:58 ` Jim Quinlan 2023-04-06 18:35 ` Krzysztof Kozlowski 2023-04-06 18:35 ` Krzysztof Kozlowski 2023-04-06 18:47 ` Florian Fainelli 2023-04-06 18:47 ` Florian Fainelli 2023-04-06 18:53 ` Krzysztof Kozlowski 2023-04-06 18:53 ` Krzysztof Kozlowski 2023-04-06 18:34 ` Krzysztof Kozlowski 2023-04-06 18:34 ` Krzysztof Kozlowski 2023-04-06 18:53 ` Florian Fainelli 2023-04-06 18:53 ` Florian Fainelli 2023-04-06 18:54 ` Krzysztof Kozlowski 2023-04-06 18:54 ` Krzysztof Kozlowski 2023-04-06 12:46 ` [PATCH v1 2/3] PCI: brcmstb: Clkreq# accomodations of downstream device Jim Quinlan 2023-04-06 12:46 ` Jim Quinlan 2023-04-06 19:09 ` Bjorn Helgaas 2023-04-06 19:09 ` Bjorn Helgaas 2023-04-06 20:03 ` Jim Quinlan 2023-04-06 20:03 ` Jim Quinlan 2023-04-06 12:46 ` [PATCH v1 3/3] PCI: brcmstb: Allow setting the completion timeout Jim Quinlan 2023-04-06 12:46 ` Jim Quinlan 2023-04-06 15:59 ` Stefan Wahren 2023-04-06 15:59 ` Stefan Wahren 2023-04-06 17:15 ` Jim Quinlan 2023-04-06 17:15 ` Jim Quinlan 2023-04-06 21:31 ` [PATCH v1 0/3] PCI: brcmstb: Clkreq# accomodations of downstream device Cyril Brulebois 2023-04-06 21:31 ` Cyril Brulebois 2023-04-07 15:06 ` Hank Barta 2023-04-07 15:06 ` Hank Barta
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CANCKTBsLxkPb1ajACkyhJk6J1aB2iwX0oKifHkADG0fFPUqMhQ@mail.gmail.com \ --to=jim2101024@gmail.com \ --cc=bcm-kernel-feedback-list@broadcom.com \ --cc=bhelgaas@google.com \ --cc=devicetree@vger.kernel.org \ --cc=f.fainelli@gmail.com \ --cc=james.quinlan@broadcom.com \ --cc=kibi@debian.org \ --cc=krzysztof.kozlowski+dt@linaro.org \ --cc=kw@linux.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pci@vger.kernel.org \ --cc=linux-rpi-kernel@lists.infradead.org \ --cc=lorenzo.pieralisi@arm.com \ --cc=lpieralisi@kernel.org \ --cc=nsaenz@kernel.org \ --cc=phil@raspberrypi.com \ --cc=robh@kernel.org \ --cc=stefan.wahren@i2se.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.