[PATCH v2 1/7] arm: a37xx: pci: Don't put link into LTSSM Recovery state during probe

[Stefan Roese]

Hi Pali,

On 01.06.21 14:57, Pali Rohár wrote:
> On Thursday 27 May 2021 08:19:32 Stefan Roese wrote:
>> On 26.05.21 17:59, Pali Rohár wrote:
>>> During our debugging of the Aardvark driver in Linux we have discovered
>>> that the PCIE_CORE_LINK_CTRL_STAT_REG register in fact controls standard
>>> PCIe Link Control Register for PCIe Root Bridge. This led us to discover
>>> that the name of the PCIE_CORE_LINK_TRAINING macro and the corresponding
>>> comment by this macro's usage is misleading; this bit in fact controls
>>> Retrain Link, which, according to PCIe base spec is defined as:
>>>
>>>     A write of 1b to this bit initiates Link retraining by directing the
>>>     Physical Layer LTSSM to the Recovery state. If the LTSSM is already in
>>>     Recovery or Configuration, re-entering Recovery is permitted but not
>>>     required.
>>>
>>> Entering Recovery state is normally done from LTSSM L0, L0s and L1 states.
>>> But since the pci-aardvark.c driver enables Link Training just a few lines
>>> above, the controller is not in L0 ready state yet. So setting aardvark bit
>>> PCIE_CORE_LINK_TRAINING does not actually enter Recovery state at this
>>> place.
>>>
>>> Moreover, trying to enter LTSSM Recovery state without other configuration
>>> is causing issues for some cards (e.g. Atheros AR9xxx and QCA9xxx). Since
>>> Recovery state is not entered, these issues are not triggered.
>>>
>>> Remove code which tries to enter LTSSM Recovery state completely.
>>>
>>> Signed-off-by: Pali Rohár <pali at kernel.org>
>>> Reviewed-by: Marek Behún <marek.behun at nic.cz>
>>
>> Reviewed-by: Stefan Roese <sr at denx.de>
>>
>> Thanks,
>> Stefan
> 
> Hello Stefan! Thank you for review. Would you be sending these A3720
> patches to 2021.07 version?

My plan was to postpone these patches to the next release, as they seem
quite intrusive. But please let me know if you think this is not the
case and they should be added as fixes into this release still.

Thanks,
Stefan

>>> ---
>>>    drivers/pci/pci-aardvark.c | 5 -----
>>>    1 file changed, 5 deletions(-)
>>>
>>> diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
>>> index c43d4f309b19..06c567e236f9 100644
>>> --- a/drivers/pci/pci-aardvark.c
>>> +++ b/drivers/pci/pci-aardvark.c
>>> @@ -613,11 +613,6 @@ static int pcie_advk_setup_hw(struct pcie_advk *pcie)
>>>    	reg |= PIO_CTRL_ADDR_WIN_DISABLE;
>>>    	advk_writel(pcie, reg, PIO_CTRL);
>>> -	/* Start link training */
>>> -	reg = advk_readl(pcie, PCIE_CORE_LINK_CTRL_STAT_REG);
>>> -	reg |= PCIE_CORE_LINK_TRAINING;
>>> -	advk_writel(pcie, reg, PCIE_CORE_LINK_CTRL_STAT_REG);
>>> -
>>>    	/* Wait for PCIe link up */
>>>    	if (pcie_advk_wait_for_link(pcie))
>>>    		return -ENXIO;
>>>
>>
>>
>> Viele Grüße,
>> Stefan
>>
>> -- 
>> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
>> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>> Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de


Viele Grüße,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de