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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, May 09, 2025 at 01:18:27PM -0500, Rob Herring wrote:
> On Wed, Apr 30, 2025 at 01:23:03PM +0530, Manivannan Sadhasivam wrote:
> > On Wed, Apr 30, 2025 at 09:16:56AM +0200, Niklas Cassel wrote:
> > > Hello Mani,
> > > 
> > > On Wed, Apr 30, 2025 at 12:35:18PM +0530, Manivannan Sadhasivam wrote:
> > > > On Fri, Apr 25, 2025 at 11:20:12AM +0200, Niklas Cassel wrote:
> > > > > While some boards designs support multiple reference clocking schemes
> > > > > (e.g. Common Clock and SRNS), and can choose the clocking scheme using
> > > > > e.g. a DIP switch, most boards designs only support a single clocking
> > > > > scheme (even if the SoC might support multiple clocking schemes).
> > > > > 
> > > > > This property is needed such that the PCI controller driver, in endpoint
> > > > > mode, can set the proper bits, e.g. the Common Clock Configuration bit and
> > > > > the SRIS Clocking bit, in the PCIe Link Control Register (Offset 10h).
> > > > > (Sometimes, there are also specific bits that needs to be set in the PHY.)
> > > > > 
> > > > 
> > > > Thanks for adding the property. I did plan to submit something similar to allow
> > > > Qcom PCIe EP controllers to run in SRIS mode.
> > > > 
> > > > > Some device tree bindings have already implemented vendor specific
> > > > > properties to handle this, e.g. "nvidia,enable-ext-refclk" (Common Clock)
> > > > > and "nvidia,enable-srns" (SRNS). However, since this property is common
> > > > > for all PCI controllers running in endpoint mode, this really ought to be
> > > > > a property in the common pcie-ep.yaml device tree binding.
> > > > > 
> > > > 
> > > > We should also mark the nvidia specific properties deprecated and use this one.
> > > > But that's for another follow up series.
> > > > 
> > > > > Add a new ref-clk-mode property that describes the reference clocking
> > > > > scheme used by the endpoint. (We do not add a common-clk-ssc option, since
> > > > > we cannot know/control if the common clock provided by the host uses SSC.)
> > > > > 
> > > > > Signed-off-by: Niklas Cassel <cassel@xxxxxxxxxx>
> > > > > ---
> > > > >  Documentation/devicetree/bindings/pci/pci-ep.yaml | 9 +++++++++
> > > > >  1 file changed, 9 insertions(+)
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/pci/pci-ep.yaml b/Documentation/devicetree/bindings/pci/pci-ep.yaml
> > > > > index f75000e3093d..206c1dc2ab82 100644
> > > > > --- a/Documentation/devicetree/bindings/pci/pci-ep.yaml
> > > > > +++ b/Documentation/devicetree/bindings/pci/pci-ep.yaml
> > > > > @@ -42,6 +42,15 @@ properties:
> > > > >      default: 1
> > > > >      maximum: 16
> > > > >  
> > > > > +  ref-clk-mode:
> > > > 
> > > > How about 'refclk-mode' instead of 'ref-clk-mode'? 'refclk' is the most widely
> > > > used terminology in the bindings.
> > > 
> > > I does seem that way.
> > > Will use your suggestion in V2.
> > > 
> > > 
> > > > 
> > > > > +    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.

> Finally, this[1] seems related. Figure out a common solution.
> 

Thanks for pointing it out. I think the patch also refers to the 'refclk' though
there was never a mention of 'reference clock'. I will respond there.

- Mani

-- 
மணிவண்ணன் சதாசிவம்




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux