On Mon, May 12, 2025 at 08:59:09AM -0500, Rob Herring wrote: > > > > 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. It is not really a property of the link though. The RC could be running with a Separate Reference clock without spread spectrum clocking (SSC), while the EP could be running with a Separate Reference clock with SSC. The link would still be classified as SRIS, even though only one end of the link is using SSC. So AFAICT there cannot be property "per link". > > > 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? If we take the RK3588 SoC as an example, per the TRM, the SoC supports both SRNS and Common Clock. However, on the Rock 5b board (which uses the RK3588 SoC), the refclock when running is EP mode can only be sourced from the clock generator on the board itself (Separate Reference clock), it is not possible to source the refclock from the PCIe slot itself (Common Clock). However, this is a design limitation of the board, not of the SoC. E.g. Rockchip might have a development board that uses the RK3588 SoC, which allows you select where to source the clock from using a mux, either from the PCIe slot, or from the on board clock generator. Some development boards I have seen have a DIP switch on the board that allows you to select if you want to source the clock from the PCIe slot or not. However, not all boards have this nice feature. And even if you do have a DIP switch for that, and a GPIO which you can read the DIP switch value from, when running in Separate Reference clock mode, you can either run with or without SSC (i.e. SRNS mode or SRIS mode). When running in SRIS mode, to enable SSC, we need to write registers both in the PHY and in the controller, before even starting link training. I do realize that, for boards supporting more than a single mode (Common Clock/SRNS/SRIS), this device tree property is basically a configuration option. For boards only supporting a single mode, it is actually describing the hardware. E.g. Rock 5b can run in both SRNS and SRIS mode (Common Clock is not supported), and since this has to be configured before starting the link, I cannot think of a better way to control this than a device tree property. In my specific case, I will also need to add a SSC property to the PCIe PHY DT binding, to control if SSC should be enabled or not (needed when running in SRIS mode). Sure, perhaps it could be possible to use phy_set_mode() from the PCIe controller driver or similar, that conveys this information to the PHY driver... But with all the possible PCI bifurcation DT properties that already exist in the PCIe PHY DT binding, I'm not sure if making use of phy_set_mode() is feasible, or if I will be forced to add a property to the PCIe PHY DT binding. Kind regards, Niklas
