On 12-Dec-17 5:15 PM, Lorenzo Pieralisi wrote:
> On Mon, Oct 30, 2017 at 07:27:13PM +0530, Manikanta Maddireddy wrote:
>> In Tegra186 PHY programming is done by BPMP-FW, so PHY calls are skipped
>
> It is time you defined what FW does and what the kernel does and stick
> to that SW initialization sequence for all Tegra PCI host bridges from
> now onwards. Those tegra_pcie_soc structures in the driver will end up
> with a gazillion hardcoded parameters if you do not take a stance from
> that perspective and I do not like that - at all.
>
> You may want to use DT bindings to describe the FW<->OS handover - or
> just enforce firmware guidelines for Tegra systems - something has to be
> done.
>
> This patch makes sense stand-alone but I wanted to get the message
> across.
>
> Thanks,
> Lorenzo
>
In Tegra186 BPMP-FW will take care of programming the UPHY, this is
updated in commit log of "9cea513d8cbc ("PCI: tegra: Add Tegra186 PCIe support")".
I didn't put it in DT bindings because there is no DT property for this purpose.
Do you want me to add a comment in PCIe tegra driver, perhaps near program_uphy variable?
In current host driver, REFCLK pads settings are done in tegra_pcie_port_phy_power_off()
which is not apt. REFCLK pad settings tunes the PCIe sideband signal REFCLK, it has
nothing to do with UPHY. So I am moving it out of tegra_pcie_port_phy_power_off().
Thanks,
Manikanta
>> 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 improve REFCLK peak to peak amplitude.
>>
>> Signed-off-by: Manikanta Maddireddy <mmaddireddy@xxxxxxxxxx>
>> ---
>> V3:
>> * Corrected commit log
>> V2:
>> * no change in this patch
>>
>> drivers/pci/host/pci-tegra.c | 20 +++++++++++++-------
>> 1 file changed, 13 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
>> index b41c60c7414c..068510b40c1a 100644
>> --- a/drivers/pci/host/pci-tegra.c
>> +++ b/drivers/pci/host/pci-tegra.c
>> @@ -910,7 +910,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;
>>
>> @@ -936,12 +935,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;
>> }
>>
>> @@ -2049,6 +2042,17 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
>> return 0;
>> }
>>
>> +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);
>> +}
>> +
>> /*
>> * FIXME: If there are no PCIe cards attached, then calling this function
>> * can result in the increase of the bootup time as there are big timeout
>> @@ -2107,6 +2111,8 @@ 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);
>> --
>> 2.1.4
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
