On 15-Apr-19 4:36 PM, Thierry Reding wrote:
> On Thu, Apr 11, 2019 at 10:33:28PM +0530, Manikanta Maddireddy wrote:
>> In Tegra186 PHY programming is done by BPMP-FW, so PHY calls are skipped
>> in driver. REFCLK pad settings are independent of PHY and should be
>> programmed by driver. So move REFCLK pad settings out of phy_power_on().
>> These pad settings tune REFCLK peak to peak amplitude.
>>
>> Fixes: cf5d31801278 ("PCI: tegra: Program PADS_REFCLK_CFG* always, not
>> just on legacy SoCs")
>>
>> Signed-off-by: Manikanta Maddireddy <mmaddireddy@xxxxxxxxxx>
>> ---
>> drivers/pci/controller/pci-tegra.c | 20 +++++++++++++-------
>> 1 file changed, 13 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
>> index 0bf270bcea34..a61ce9d475b4 100644
>> --- a/drivers/pci/controller/pci-tegra.c
>> +++ b/drivers/pci/controller/pci-tegra.c
>> @@ -852,7 +852,6 @@ static int tegra_pcie_port_phy_power_off(struct tegra_pcie_port *port)
>> static int tegra_pcie_phy_power_on(struct tegra_pcie *pcie)
>> {
>> struct device *dev = pcie->dev;
>> - const struct tegra_pcie_soc *soc = pcie->soc;
>> struct tegra_pcie_port *port;
>> int err;
>>
>> @@ -878,12 +877,6 @@ static int tegra_pcie_phy_power_on(struct tegra_pcie *pcie)
>> }
>> }
>>
>> - /* Configure the reference clock driver */
>> - pads_writel(pcie, soc->pads_refclk_cfg0, PADS_REFCLK_CFG0);
>> -
>> - if (soc->num_ports > 2)
>> - pads_writel(pcie, soc->pads_refclk_cfg1, PADS_REFCLK_CFG1);
>> -
>> return 0;
>> }
>>
>> @@ -2092,11 +2085,24 @@ static bool tegra_pcie_port_check_link(struct tegra_pcie_port *port)
>> return false;
>> }
>>
>> +static void tegra_pcie_apply_pad_settings(struct tegra_pcie *pcie)
>> +{
>> + const struct tegra_pcie_soc *soc = pcie->soc;
>> +
>> + /* Configure the reference clock driver */
>> + pads_writel(pcie, soc->pads_refclk_cfg0, PADS_REFCLK_CFG0);
>> +
>> + if (soc->num_ports > 2)
>> + pads_writel(pcie, soc->pads_refclk_cfg1, PADS_REFCLK_CFG1);
>> +}
>> +
>> static void tegra_pcie_enable_ports(struct tegra_pcie *pcie)
>> {
>> struct device *dev = pcie->dev;
>> struct tegra_pcie_port *port, *tmp;
>>
>> + tegra_pcie_apply_pad_settings(pcie);
>> +
>> list_for_each_entry_safe(port, tmp, &pcie->ports, list) {
>> dev_info(dev, "probing port %u, using %u lanes\n",
>> port->index, port->lanes);
> This also seems to move the programming of these registers to a
> different point in time. Was that intentional? If so, please mention it
> in the commit message and describe why that's necessary.
>
> If that was not intentional, it seems like the right place to call this
> would be right after the call to tegra_pcie_enable_controller() in
> tegra_pcie_pm_resume().
>
> Thierry
PCIe pad registers access needs PEX clk and reset enabled, so I moved to
tegra_pcie_enable_ports(). But looking at this carefully I see a pattern
that only per port PCIe register programming is done, however PCIe
pad register spec is for all controller. So the right place would be
tegra_pcie_pm_resume() after enable PEX clk and reset. I will update
in V2
