Re: [RFC 2/2] PCI: imx6: Add support for i.MX8QM/QXP PCIe

[Andrey Smirnov]

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&amp;data=02%7C01%7Chongxing.zhu%40nxp.com%7Cb745
> > 5fe59e384723f44208d6a8ec835a%7C686ea1d3bc2b4c6fa92cd99c5c301635
> > %7C0%7C0%7C636882131105162138&amp;sdata=s7438xBSNxnWwTMxZXkj
> > LhgdPiS6puCRdrgr9suZpPQ%3D&amp;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