On 14-Dec-17 12:04 AM, Lorenzo Pieralisi wrote: > On Wed, Dec 13, 2017 at 10:02:02PM +0530, Manikanta Maddireddy wrote: >> >> >> On 13-Dec-17 7:38 PM, Lorenzo Pieralisi wrote: >>> On Wed, Dec 13, 2017 at 05:20:39PM +0530, Manikanta Maddireddy wrote: >>>> >>>> >>>> On 12-Dec-17 5:02 PM, Lorenzo Pieralisi wrote: >>>>> On Mon, Oct 30, 2017 at 07:27:12PM +0530, Manikanta Maddireddy wrote: >>>>>> This patch ensures that LTSSM is started (by deasserting pcie_xrst) only >>>>>> after all the required root port register programming is completed. >>>>>> >>>>>> Signed-off-by: Manikanta Maddireddy <mmaddireddy@xxxxxxxxxx> >>>>>> --- >>>>>> V3: >>>>>> * no change in this patch >>>>>> V2: >>>>>> * no change in this patch >>>>>> >>>>>> drivers/pci/host/pci-tegra.c | 9 +++++---- >>>>>> 1 file changed, 5 insertions(+), 4 deletions(-) >>>>>> >>>>>> diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c >>>>>> index 96e8038c3019..b41c60c7414c 100644 >>>>>> --- a/drivers/pci/host/pci-tegra.c >>>>>> +++ b/drivers/pci/host/pci-tegra.c >>>>>> @@ -1024,9 +1024,6 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie) >>>>>> } >>>>>> } >>>>>> >>>>>> - /* take the PCIe interface module out of reset */ >>>>>> - reset_control_deassert(pcie->pcie_xrst); >>>>>> - >>>>>> /* finally enable PCIe */ >>>>>> value = afi_readl(pcie, AFI_CONFIGURATION); >>>>>> value |= AFI_CONFIGURATION_EN_FPCI; >>>>>> @@ -1065,7 +1062,6 @@ static void tegra_pcie_power_off(struct tegra_pcie *pcie) >>>>>> dev_err(dev, "failed to power off PHY(s): %d\n", err); >>>>>> } >>>>>> >>>>>> - reset_control_assert(pcie->pcie_xrst); >>>>> >>>>> This does not look like it is part of the reset de-assertion code move. >>>>> >>>>> tegra_pcie_enable_controller() -> tegra_pcie_enable_ports() >>>>> >>>>> in other words, why are you removing it ? >>>>> >>>>> Lorenzo >>>> >>>> Hi Lorenzo, >>>> >>>> Host driver should start LTSSM after programming all controller registers. >>>> In tegra_pcie_enable_controller() bunch of AFI module programming is done and >>>> I am adding PCIe register programming in this series. >>>> So I moved deasserting of pcie_xrst(which starts LTSSM) to tegra_pcie_enable_ports(), >>>> which is right after sending RST pulse(tegra_pcie_port_reset()) to endpoint. >>> >>> I asked why you removed the reset assertion in tegra_pcie_power_off(), >>> it is not clear to me. You still call tegra_pcie_power_off() in >>> the tegra_pcie_probe() error path and I see no reason why the reset >>> assertion - called through: >>> >>> tegra_pcie_put_resources() >>> -> tegra_pcie_power_of() >>> >>> is removed, if it was needed previously. >>> >>> Lorenzo >>> >> >> New sequence with this patch will be >> tegra_pcie_enable_controller() -> tegra_pcie_request_resources() -> tegra_pcie_enable_ports() >> ->goto put_resources on fail -> reset_control_deassert(pcie->pcie_xrst); >> >> Since pcie_xrst deassert happens after tegra_pcie_request_resources(), there is no need to assert pcie_xrst on put_resource failure. > > I do not understand you, sorry for being blunt. > > What has tegra_pcie_request_resources() to do with the reset > assertion/deassertion ? > > This patch moves: > > reset_control_deassert(pcie->pcie_xrst); > > from: > > tegra_pcie_enable_controller() > > to > > tegra_pcie_enable_ports() > > if: > > reset_control_assert(pcie->pcie_xrst); > > was needed before this patch in tegra_pcie_power_off() > > why it is not needed there after this patch is applied ? > > Lorenzo > Opposite of tegra_pcie_enable_ports() is missing on disable_msi failure in current driver. So I took care of assert pcie_xrst in https://patchwork.ozlabs.org/patch/846043/ along with other resources. >>>>>> reset_control_assert(pcie->afi_rst); >>>>>> reset_control_assert(pcie->pex_rst); >>>>>> >>>>>> @@ -2116,7 +2112,12 @@ static void tegra_pcie_enable_ports(struct tegra_pcie *pcie) >>>>>> port->index, port->lanes); >>>>>> >>>>>> tegra_pcie_port_enable(port); >>>>>> + } >>>>>> >>>>>> + /* take the PCIe interface module out of reset */ >>>>>> + reset_control_deassert(pcie->pcie_xrst); >>>>>> + >>>>>> + list_for_each_entry_safe(port, tmp, &pcie->ports, list) { >>>>>> if (tegra_pcie_port_check_link(port)) >>>>>> continue; >>>>>> >>>>>> -- >>>>>> 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
