Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <1552467452-538-1-git-send-email-hongxing.zhu@nxp.com>
 <1552467452-538-2-git-send-email-hongxing.zhu@nxp.com> <CAHQ1cqE3fcCQ2mHk-SuLdBn3XDUy9U2h=qsTCS6aO+3za0Aptw@mail.gmail.com>
 <AM0PR0402MB357070EA4DF0331D0F29EDCE8C4B0@AM0PR0402MB3570.eurprd04.prod.outlook.com>
 <CAHQ1cqHjiLUFRUMO40fcpO83ATrkAa5NcZWnjjZMe+ZskSCQiQ@mail.gmail.com> <AM0PR0402MB35709D0CB62204A2AE49BDBD8C440@AM0PR0402MB3570.eurprd04.prod.outlook.com>
In-Reply-To: <AM0PR0402MB35709D0CB62204A2AE49BDBD8C440@AM0PR0402MB3570.eurprd04.prod.outlook.com>
From:   Andrey Smirnov <andrew.smirnov@gmail.com>
Date:   Mon, 18 Mar 2019 20:35:33 -0700
Message-ID: <CAHQ1cqEQUw+nE9novFC2oh8OBAGQ_KW2Q=d8EsQnFmD+BN4EAg@mail.gmail.com>
Subject: Re: [RFC 2/2] PCI: imx6: Add support for i.MX8QM/QXP PCIe
To:     Richard Zhu <hongxing.zhu@nxp.com>,
        "l.stach@pengutronix.de" <l.stach@pengutronix.de>
Cc:     "bhelgaas@google.com" <bhelgaas@google.com>,
        "lorenzo.pieralisi@arm.com" <lorenzo.pieralisi@arm.com>,
        "linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Mar 14, 2019 at 11:05 PM Richard Zhu <hongxing.zhu@nxp.com> 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