| Message ID | 20220202132301.v3.9.I5f367dcce8107f2186b2aad4aef0dfcfafa034b9@changeid |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,01/14] arm64: dts: qcom: sc7180-trogdor: Add "-regulator" suffix to pp3300_hub | expand |
Hi, On Thu, Feb 3, 2022 at 1:59 PM Stephen Boyd <swboyd@chromium.org> wrote: > > Quoting Doug Anderson (2022-02-03 13:53:09) > > Hi, > > > > On Thu, Feb 3, 2022 at 1:42 PM Stephen Boyd <swboyd@chromium.org> wrote: > > > > > > Quoting Douglas Anderson (2022-02-02 13:23:43) > > > > I believe that the PCIe clkreq pin is an output. That means we > > > > shouldn't have a pull enabled for it. Turn it off. > > > > > > It sounds like it's a request from the PCI device to the PCI phy that > > > the clk should be on. I googled pcie clkreq open drain and this pdf[1] > > > says > > > > > > "The CLKREQ# signal is an open drain, active low signal that is driven > > > low by the PCI Express M.2 add-I Card function to request that the PCI > > > Express reference clock be available (active clock state) in order to > > > allow the PCI Express interface to send/receive data" > > > > > > so presumably if there isn't an external pull on the signal the open > > > drain feature will not work and the PCIe device won't be able to drive > > > it low. > > > > > > [1] https://advdownload.advantech.com/productfile/PIS/96FD80-P512-LIS/Product%20-%20Datasheet/96FD80-P512-LIS_datasheet20180110154919.pdf > > > > Yeah, I had some trouble figuring this out too, so if someone knows > > better than me then I'm more than happy to take advice here. I thought > > I had found something claiming that "clkreq" was an output and on the > > schematic I have from Qualcomm it shows an arrow going out from the > > SoC for this signal indicating that it's an output from the SoC. Of > > course, those arrows are notoriously wrong but at least it's one piece > > of evidence that someone thought this was an output from the SoC. > > > > Hrm, but I just checked the sc7280 "datasheet" which claims that this > > is an input. Sigh. > > > > I guess the options are: > > * If we're sure this is an input to the SoC then I think we should > > remove the drive-strength, right? > > * If we don't know then I guess we can leave both? > > I'll wait for qcom folks to confirm. Maybe it's bidirectional because it > is an open drain signal. I'm showing my cards that I'm no PCIe expert :) Ah ha! I searched some more and found a Qualcomm PCIe user guide on this! CLKREQ# signal properties – Bi-directional clock request signals whether the RC or AP requires control So it sounds as if leaving it as pull-up and having drive-strength as 2 is right. tl;dr: drop this patch from the series... > > In any case, for now we can just drop this patch? > > > > Sounds good to me. It needs to be resolved through for herobrine-r1? Yup. I responded to that patch and for now I'll wait for Bjorn to give me direction on how to handle it. -Doug
diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts index 82c3c8f0342b..3c5aab225748 100644 --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts @@ -827,7 +827,7 @@ &usb_2_hsphy { /* PINCTRL - additions to nodes defined in sc7280.dtsi */ &pcie1_clkreq_n { - bias-pull-up; + bias-disable; drive-strength = <2>; }; diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi index 6e20e8c07402..9140dca3b72a 100644 --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi @@ -367,7 +367,7 @@ key_vol_up_default: key-vol-up-default { }; &pcie1_clkreq_n { - bias-pull-up; + bias-disable; drive-strength = <2>; };
I believe that the PCIe clkreq pin is an output. That means we shouldn't have a pull enabled for it. Turn it off. Signed-off-by: Douglas Anderson <dianders@chromium.org> --- Changes in v3: - ("sc7280-idp: Disable pull from pcie1_clkreq") new for v3. arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r0.dts | 2 +- arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)