Re: [PATCH v1 1/3] dt-bindings: PCI: brcmstb: Add two optional props

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 4/6/23 11:35, Krzysztof Kozlowski wrote:
On 06/04/2023 18:58, Jim Quinlan wrote:
On Thu, Apr 6, 2023 at 11:39 AM Stefan Wahren <stefan.wahren@xxxxxxxx> 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@xxxxxxxxx>
---
   .../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.

If "completion" means Linux completion, then it is not suitable for DT
and entire property should be removed. If it is something else, then
explain here and commit msg. So far both refer to some completion...

This is a PCIe root complex binding so completion needs to be understood in the context of PCIe, that is the time needed for the root complex to complete/finish/proceed with a PCIe transaction layer packet (TLP).
--
Florian




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux