Re: [PATCH V3 01/12] PCI: tegra: Start LTSSM after programming root port

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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



[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux