Re: [PATCH] dt-bindings: PCI: pci-ep: Add ref-clk-mode

[Rob Herring <robh@kernel.org>]

On Sat, May 10, 2025 at 01:04:16PM +0200, Niklas Cassel wrote:
> On Sat, May 10, 2025 at 01:01:51AM +0530, Manivannan Sadhasivam wrote:
> > On Fri, May 09, 2025 at 01:18:27PM -0500, Rob Herring wrote:
> > > > > > 
> > > > > > > +    description: Reference clocking architechture
> > > > > > > +    enum:
> > > > > > > +      - common-clk        # Common Reference Clock (provided by RC side)
> > > > > > 
> > > > > > Can we use 'common-clk-host' so that it is explicit that the clock is coming
> > > > > > from the host side?
> > > > > 
> > > > > Sure.
> > > > > 
> > > > > I take it that you prefer 'common-clk-host' over 'common-clk-rc' ?
> > > > > 
> > > > 
> > > > That's what I intended previously, but thinking more, I feel that we should
> > > > stick to '-rc'i, as that's what the PCIe spec uses.
> > > 
> > > Couldn't this apply to any link, not just a RC? Is there PCIe 
> > > terminology for upstream and downstream ends of a link?
> > > 
> > 
> > Usually, the refclk comes from the host machine to the endpoint, but doesn't
> > necessarily from the root complex. Since the refclk source could very well be
> > from the motherboard or the host system PCB, controlled by the host software.
> > 
> > > The 'common-clk' part seems redundant to me with '-rc' or whatever we 
> > > end up with added.
> > > 
> > 
> > No. It could be the other way around. We can drop the '-rc' suffix if it seem
> > redundant. Maybe that is a valid argument also since root complex doesn't
> > necessarily provide refclk and the common refclk usually comes from the host.
> 
> When the RC and EP uses a common clock (rather than separate clocks),
> the clock can either be provided by the host side or the EP side.
> 
> The most common by far (if using a common clock) is that it the common
> clock is provided by the host side. That is why my patch just named it
> 'common-clk' instead of 'common-clk-host' or 'common-clk-rc'.
> 
> I can use whatever name we agree on. I indend to send out V2 of this
> patch as part of a series that adds SRIS support to the dw-rockchip
> driver, in order to address Krzysztof's comment.
> 
> 
> > 
> > > Finally, this[1] seems related. Figure out a common solution.
> 
> I don't see the connection.
> 
> https://lore.kernel.org/all/20250406144822.21784-2-marek.vasut+renesas@mailbox.org/
> 
> does specify a reference clock, but that is in a host side DT binding.
> 
> 
> This patch adds a refclk-mode property to an endpoint side DT binding.

If we are dealing with the same property of the link, it doesn't matter 
which side. What we don't need is 2 different solutions.

> This property is needed such that the endpoint can configure the bits
> in its own PCIe Link Control Register before starting the link.
> 
> Perhaps the host side could also make use of a similar property, but I'm not
> sure, you don't know from the host side which endpoint will be plugged in.
> 
> >From the EP side, you do know if your SoC only supports common-clock or
> SRNS/SRIS, since that depends on if the board can source the clock from
> the PCIe slot or not (of all the DWC based drivers, only Qcom and Tegra
> can do so, rest uses SRNS/SRIS), so this property definitely makes sense
> in an EP side DT binding.

I don't understand why we need this in DT in the first place. Seems like 
needing to specify this breaks discoverability? Perhaps this information 
is only relevant after initial link is up and the host can read the EP 
registers?

Rob