On Thu, Dec 14, 2017 at 09:57:24AM +0000, Lorenzo Pieralisi wrote: > On Thu, Dec 14, 2017 at 12:57:28AM +0530, Manikanta Maddireddy wrote: > > > > > > 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. > > Which is in another series. This patch has to be self-contained with a > single logical change that is applicable on its own. Even if we end up > merging the series above every patch has to make sense on its own. This > does not. > > So I would start with re-posting this series as a self contained one, > addressing all review comments; we will get to other series later when > this is sorted. Hi Manikanta, I wanted to check with you where are we with this series. I would mark it with "Changes requested", waiting for you to rebase it and update it according to review comments. Please let me know if that's OK with you. Thanks, Lorenzo -- 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
