Re: [PATCH v2 4/5] PCI: stm32: Add PCIe endpoint support for STM32MP25

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

 





On 12/16/24 17:17, Manivannan Sadhasivam wrote:
On Mon, Dec 16, 2024 at 11:02:07AM +0100, Christian Bruel wrote:
Hi Manivanna,

On 12/3/24 16:22, Manivannan Sadhasivam wrote:
On Tue, Nov 26, 2024 at 04:51:18PM +0100, Christian Bruel wrote:

[...]

+static int stm32_pcie_start_link(struct dw_pcie *pci)
+{
+	struct stm32_pcie *stm32_pcie = to_stm32_pcie(pci);
+	int ret;
+
+	if (stm32_pcie->link_status == STM32_PCIE_EP_LINK_ENABLED) {
+		dev_dbg(pci->dev, "Link is already enabled\n");
+		return 0;
+	}
+
+	ret = stm32_pcie_enable_link(pci);
+	if (ret) {
+		dev_err(pci->dev, "PCIe cannot establish link: %d\n", ret);
+		return ret;
+	}

How the REFCLK is supplied to the endpoint? From host or generated locally?

 From Host only, we don't support the separated clock model.


OK. So even without refclk you are still able to access the controller
registers? So the controller CSRs should be accessible by separate local clock I
believe.

Anyhow, please add this limitation (refclk dependency from host) in commit
message.

[...]

+	ret = phy_set_mode(stm32_pcie->phy, PHY_MODE_PCIE);

Hmm, so PHY mode is common for both endpoint and host?

Yes it is. We need to init the phy here because it is a clock source for the
PCIe core clk


Clock source? Is it coming directly to PCIe or through RCC? There is no direct
clock representation from PHY to PCIe in DT binding.

We have the following simplified clock dependencies (details in the RM)

                                _____________
RCC  ck_icn_pcie--- ------------|-> dbi_clk  |
                    _________   |            |
     ck_icn_phy ---|->       |  |            |
                   |  pipe0--|--|->core_clk  |
     ck_ker ----- -|->       |  |            |
                   |         |  |            |
     100mhz pad ---|-> pll   |  |            |
                   |_________|  |____________|
                     COMBOPHY      PCIE


I considered adding the COMBOPHY pipe0 as the clock provider for the PCIe core_clk, but this did not provide any advantage since the PLL needs to be locked first and all settings need to be completed. Therefore, using clock_prepare_enable(pipe0) would be redundant with what phy_init already accomplishes. The phy_init function is necessary because it is used by the USB3 driver.

Since the core_clk is operational when all three other clocks are enabled and the PLL is locked, modeling pipe0 had minimal value, especially considering the dependencies of the USB3 driver.

I will add a comment in the code to explain this.


- Mani





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux