On Thu, Mar 14, 2019 at 11:05 PM Richard Zhu <hongxing.zhu@xxxxxxx> wrote: > > > > > + imx6_pcie->pcie_inbound_axi = > > > > devm_clk_get(&pdev->dev, > > > > > + "pcie_inbound_axi"); > > > > > + if (IS_ERR(imx6_pcie->pcie_inbound_axi)) { > > > > > + dev_err(&pdev->dev, > > > > > + "pcie clock source missing or > > > > invalid\n"); > > > > > + return > > > > PTR_ERR(imx6_pcie->pcie_inbound_axi); > > > > > + } > > > > > > > > On i.MX8MQ "pcie_bus" clock in vendor tree wasn't actually pointing > > > > to actual PCIE bus clock, so it might be worth checking if that's > > > > the case for i.MX8QM/X and you actually need one more clock. > > > [Richard Zhu] Regarding to my understanding, iMX PCIe module is > > connected to AXI bus. > > > Thus, the AXI related clock can be treated as bus clock. Correct me if my > > understand is wrong. > > > So, I use the pcie_bus clock for i.MX8QM/QXP PCIe in the dts binding. > > > Otherwise, I can use another new clock in codes to support i.MX8QM/QXP > > PCIes. > > > > > > > So, "pcie_bus" is supposed to be the clock driving PCIE bus itself. In this case > > the clock that is controlled by CLKREQ_B. On i.MX8MQ EVK that was an > > external 100 Mhz oscillator, so the final patch has "pcie_bus" pointing to a > > dedicated "fixed-clock": > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.ke > > rnel.org%2Flkml%2F20190220015857.7136-6-andrew.smirnov%40gmail.com > > %2FT%2F%23u&data=02%7C01%7Chongxing.zhu%40nxp.com%7Cb745 > > 5fe59e384723f44208d6a8ec835a%7C686ea1d3bc2b4c6fa92cd99c5c301635 > > %7C0%7C0%7C636882131105162138&sdata=s7438xBSNxnWwTMxZXkj > > LhgdPiS6puCRdrgr9suZpPQ%3D&reserved=0 > > > > Originally vendor tree was using "pcie_bus" to point at > > IMX8MQ_CLK_PCIE1_AUX. If the situation on i.MX8QM/QXP is similar, then, > > yeah, I think it should be moved out into a separate clock. > > > [Richard Zhu] The clocks of the i.MX8QM/QXP PCIe are different to the iMX8MQ PCIe's. > Five clocks MASTER_AXI, SLAVE_AXI, DBI_AXI, PIPE_CLK and PER_CLK are mandatory required. > Currently, They are named "pcie", "pcie_bus", "pcie_inbound_axi", "pcie_phy", "pcie_per" in the vendor tree. > PIPE_CLK is output to PHY, so "pcie_phy" clock name is used by it. > I'm not sure that the names of the xxx_AXI clocks are proper or not. > What're your suggests about the names of xxx_AXI clocks? > Did new clock names for all or part of these three xxx_AXI clocks shall be added in to the codes? > Thanks in advanced. > I am hardly an authority on how those clocks should be named, so don't put too much value in my suggestion. However if I had to do, I'd probably use "pcie" for MASTER_AXI and add "pcie_slave" or maybe "pcie2" to control SLAVE_AXI. Lucas, if you don't mind, could you please comment on clock naming situation here? Thanks, Andrey Smirnov