Re: [PATCH v8 2/2] PCI: brcmstb: Configure HW CLKREQ# mode appropriate for downstream device

[Florian Fainelli <florian.fainelli@broadcom.com>]

[-- Attachment #1: Type: text/plain, Size: 2469 bytes --]

On 11/13/23 10:56, Jim Quinlan wrote:
> The Broadcom STB/CM PCIe HW core, which is also used in RPi SOCs, must be
> deliberately set by the PCIe RC HW into one of three mutually exclusive
> modes:
> 
> "safe" -- No CLKREQ# expected or required, refclk is always provided.  This
>      mode should work for all devices but is not be capable of any refclk
>      power savings.
> 
> "no-l1ss" -- CLKREQ# is expected to be driven by the downstream device for
>      CPM and ASPM L0s and L1.  Provides Clock Power Management, L0s, and L1,
>      but cannot provide L1 substate (L1SS) power savings. If the downstream
>      device connected to the RC is L1SS capable AND the OS enables L1SS, all
>      PCIe traffic may abruptly halt, potentially hanging the system.
> 
> "default" -- Bidirectional CLKREQ# between the RC and downstream device.
>      Provides ASPM L0s, L1, and L1SS, but not compliant to provide Clock
>      Power Management; specifically, may not be able to meet the T_CLRon max
>      timing of 400ns as specified in "Dynamic Clock Control", section
>      3.2.5.2.2 of the PCIe Express Mini CEM 2.1 specification.  This
>      situation is atypical and should happen only with older devices.
> 
> Previously, this driver always set the mode to "no-l1ss", as almost all
> STB/CM boards operate in this mode.  But now there is interest in
> activating L1SS power savings from STB/CM customers, which requires "aspm"
> mode.  In addition, a bug was filed for RPi4 CM platform because most
> devices did not work in "no-l1ss" mode.
> 
> Note that the mode is specified by the DT property "brcm,clkreq-mode".  If
> this property is omitted, then "default" mode is chosen.
> 
> Note: Since L1 substates are now possible, a modification was made
> regarding an internal bus timeout: During long periods of the PCIe RC HW
> being in an L1SS sleep state, there may be a timeout on an internal bus
> access, even though there may not be any PCIe access involved.  Such a
> timeout will cause a subsequent CPU abort.
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217276
> 
> Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
> Tested-by: Florian Fainelli <florian.fainelli@broadcom.com>

I don't think you could have carried that Tested-by tag given that I 
tested the previous version which is subtly different from this one, but 
since I now just did test this v8 and all is still well, I suppose that 
works just as well.

Thanks!
-- 
Florian


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]