| Message ID | 20231107-refclk_always_on-v2-0-de23962fc4b3@quicinc.com |
|---|---|
| Headers | show |
| Series | phy: qcom-qmp-pcie: Add support to keep refclk always on | expand |
On Tue, 7 Nov 2023 at 14:26, Krishna chaitanya chundru <quic_krichai@quicinc.com> wrote: > > Document qcom,refclk-always-on property which is needed in some platforms > to supply refclk even in PCIe low power states. > > Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com> > --- > .../devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml > index 2c3d6553a7ba..263291447a5b 100644 > --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml > +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml > @@ -93,6 +93,13 @@ properties: > "#phy-cells": > const: 0 > > + qcom,refclk-always-on: > + type: boolean > + description: If there is some issues in platform with clkreq signal nit: there are some However this still doesn't describe what kind of issues with clkreq you observe. I mean, clkreq is just a GPIO pin. > + propagation to the host and due to that host will not send refclk, which > + results in linkdown in L1.2 or L1.1 exit initiated by EP. This property > + if set keeps refclk always on even in Low power states (optional). > + > required: > - compatible > - reg > > -- > 2.42.0 > >
On 11/7/23 13:26, Krishna chaitanya chundru wrote: > This series adds support to provide refclk to endpoint even in low > power states. > > Due to some platform specific issues with CLKREQ signal, it is not being > propagated to the host and as host doesn't know the clkreq signal host is > not sending refclk. Due to this endpoint is seeing linkdown and going > to bad state. > To avoid those ref clk should be provided always to the endpoint. The > issue is coming only when ep intiates the L1.1 or L1.2 exit and clkreq > is not being propagated properly to the host. I'm gonna sound like a broken record, but: How much power does this consume? Would it matter if we kept this clock always-on for all platforms with this version of the phy? Konrad
On Tue, Nov 07, 2023 at 03:01:47PM +0200, Dmitry Baryshkov wrote: > On Tue, 7 Nov 2023 at 14:26, Krishna chaitanya chundru > <quic_krichai@quicinc.com> wrote: > > > > Document qcom,refclk-always-on property which is needed in some platforms > > to supply refclk even in PCIe low power states. > > > > Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com> > > --- > > .../devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml > > index 2c3d6553a7ba..263291447a5b 100644 > > --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml > > +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml > > @@ -93,6 +93,13 @@ properties: > > "#phy-cells": > > const: 0 > > > > + qcom,refclk-always-on: > > + type: boolean > > + description: If there is some issues in platform with clkreq signal > > nit: there are some > > However this still doesn't describe what kind of issues with clkreq > you observe. I mean, clkreq is just a GPIO pin. > > > + propagation to the host and due to that host will not send refclk, which > > + results in linkdown in L1.2 or L1.1 exit initiated by EP. This property > > + if set keeps refclk always on even in Low power states (optional). Dimitry's issues with the property aside, putting "(optional)" in the description is meaningless - qcom,refclk-always-on isn't listed in the required properties section, so therefore has to be optional. Cheers, Conor. > > + > > required: > > - compatible > > - reg > > > > -- > > 2.42.0 > > > > > > > -- > With best wishes > Dmitry
On 11/8/2023 3:27 AM, Konrad Dybcio wrote: > > > On 11/7/23 13:26, Krishna chaitanya chundru wrote: >> This series adds support to provide refclk to endpoint even in low >> power states. >> >> Due to some platform specific issues with CLKREQ signal, it is not being >> propagated to the host and as host doesn't know the clkreq signal >> host is >> not sending refclk. Due to this endpoint is seeing linkdown and going >> to bad state. >> To avoid those ref clk should be provided always to the endpoint. The >> issue is coming only when ep intiates the L1.1 or L1.2 exit and clkreq >> is not being propagated properly to the host. > I'm gonna sound like a broken record, but: > > How much power does this consume? Would it matter if we kept this > clock always-on for all platforms with this version of the phy? > > Konrad We see about 22mw extra power consumption with refclk always on. We can't keep this property always on as there is impact on power consumption. - Krishna Chaitanya
On 11/9/23 10:32, Krishna Chaitanya Chundru wrote: > > On 11/8/2023 3:27 AM, Konrad Dybcio wrote: >> >> >> On 11/7/23 13:26, Krishna chaitanya chundru wrote: >>> This series adds support to provide refclk to endpoint even in low >>> power states. >>> >>> Due to some platform specific issues with CLKREQ signal, it is not being >>> propagated to the host and as host doesn't know the clkreq signal host is >>> not sending refclk. Due to this endpoint is seeing linkdown and going >>> to bad state. >>> To avoid those ref clk should be provided always to the endpoint. The >>> issue is coming only when ep intiates the L1.1 or L1.2 exit and clkreq >>> is not being propagated properly to the host. >> I'm gonna sound like a broken record, but: >> >> How much power does this consume? Would it matter if we kept this >> clock always-on for all platforms with this version of the phy? >> >> Konrad > > We see about 22mw extra power consumption with refclk always on. > > We can't keep this property always on as there is impact on power consumption. Ooeheh, that's not far away from a low-clocked efficiency CPU core.. Thanks for checking! Konrad
On 11/8/2023 9:22 PM, Conor Dooley wrote: > On Tue, Nov 07, 2023 at 03:01:47PM +0200, Dmitry Baryshkov wrote: >> On Tue, 7 Nov 2023 at 14:26, Krishna chaitanya chundru >> <quic_krichai@quicinc.com> wrote: >>> Document qcom,refclk-always-on property which is needed in some platforms >>> to supply refclk even in PCIe low power states. >>> >>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com> >>> --- >>> .../devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml | 7 +++++++ >>> 1 file changed, 7 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml >>> index 2c3d6553a7ba..263291447a5b 100644 >>> --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml >>> +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml >>> @@ -93,6 +93,13 @@ properties: >>> "#phy-cells": >>> const: 0 >>> >>> + qcom,refclk-always-on: >>> + type: boolean >>> + description: If there is some issues in platform with clkreq signal >> nit: there are some >> >> However this still doesn't describe what kind of issues with clkreq >> you observe. I mean, clkreq is just a GPIO pin. >> >>> + propagation to the host and due to that host will not send refclk, which >>> + results in linkdown in L1.2 or L1.1 exit initiated by EP. This property >>> + if set keeps refclk always on even in Low power states (optional). > Dimitry's issues with the property aside, putting "(optional)" in the > description is meaningless - qcom,refclk-always-on isn't listed in the > required properties section, so therefore has to be optional. > > Cheers, > Conor. I removed the optional flag and updated the description. - Krishna chaitanya. >>> + >>> required: >>> - compatible >>> - reg >>> >>> -- >>> 2.42.0 >>> >>> >> >> -- >> With best wishes >> Dmitry
This series adds support to provide refclk to endpoint even in low power states. Due to some platform specific issues with CLKREQ signal, it is not being propagated to the host and as host doesn't know the clkreq signal host is not sending refclk. Due to this endpoint is seeing linkdown and going to bad state. To avoid those ref clk should be provided always to the endpoint. The issue is coming only when ep intiates the L1.1 or L1.2 exit and clkreq is not being propagated properly to the host. Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com> --- Changes in v2: - Added refclk cntrl registers to the applicable phy versions & added reg layout where - refclk cntrl offset needs to be updated (Dmitry) - Error out if refclk_always_on is set and there is no refclk control register to enable it (Dmitry) - updated the dt-binding description & some nit's as suggested by (Bjorn) - Link to v1: https://lore.kernel.org/r/20231106-refclk_always_on-v1-0-17a7fd8b532b@quicinc.com --- Krishna chaitanya chundru (3): dt-bindings: phy: qcom,qmp: Add PCIe qcom,refclk-always-on property phy: qcom-qmp-pcie: Add endpoint refclk control register offset phy: qcom-qmp-pcie: Add support for keeping refclk always on .../bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml | 7 ++++ drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 40 ++++++++++++++++++++-- drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h | 1 + drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5_20.h | 1 + drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_20.h | 1 + 5 files changed, 47 insertions(+), 3 deletions(-) --- base-commit: 71e68e182e382e951d6248bccc3c960dcec5a718 change-id: 20231106-refclk_always_on-9beae8297cb8 Best regards,