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
> >>>> 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
> >
