Re: [PATCH v3 09/14] arm64: dts: qcom: sc7280: Disable pull from pcie1_clkreq

[Doug Anderson]

Hi,

On Thu, Feb 3, 2022 at 1:59 PM Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote:
>
> Quoting Doug Anderson (2022-02-03 13:53:09)
> > Hi,
> >
> > On Thu, Feb 3, 2022 at 1:42 PM Stephen Boyd <swboyd@xxxxxxxxxxxx> 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