[U-Boot] [PATCH 3/3] Powerpc: pcie: Make pcie link state judgement more specific

[Xiaowei Bao]

> -----Original Message-----
> From: York Sun
> Sent: Thursday, November 09, 2017 5:46 AM
> To: Joakim Tjernlund <Joakim.Tjernlund at infinera.com>; Xiaowei Bao
> <xiaowei.bao at nxp.com>
> Cc: wd at denx.de; Mingkai Hu <mingkai.hu at nxp.com>;
> tony.obrien at alliedtelesis.co.nz; u-boot at lists.denx.de; Z.q. Hou
> <zhiqiang.hou at nxp.com>; hamish.martin at alliedtelesis.co.nz; M.h. Lian
> <minghuan.lian at nxp.com>
> Subject: Re: [U-Boot] [PATCH 3/3] Powerpc: pcie: Make pcie link state
> judgement more specific
> 
> On 11/08/2017 01:30 PM, Joakim Tjernlund wrote:
> > On Wed, 2017-11-08 at 21:05 +0000, York Sun wrote:
> >> CAUTION: This email originated from outside of the organization. Do not
> click links or open attachments unless you recognize the sender and know the
> content is safe.
> >>
> >>
> >> On 10/22/2017 07:39 PM, Xiaowei Bao wrote:
> >>>
> >>> -----Original Message-----
> >>> From: Joakim Tjernlund [mailto:Joakim.Tjernlund at infinera.com]
> >>> Sent: Friday, October 20, 2017 9:13 PM
> >>> To: wd at denx.de; Mingkai Hu <mingkai.hu at nxp.com>;
> >>> tony.obrien at alliedtelesis.co.nz; u-boot at lists.denx.de; Z.q. Hou
> >>> <zhiqiang.hou at nxp.com>; York Sun <york.sun at nxp.com>; Xiaowei Bao
> >>> <xiaowei.bao at nxp.com>; hamish.martin at alliedtelesis.co.nz; M.h. Lian
> >>> <minghuan.lian at nxp.com>
> >>> Subject: Re: [U-Boot] [PATCH 3/3] Powerpc: pcie: Make pcie link
> >>> state judgement more specific
> >>>
> >>> On Fri, 2017-10-20 at 18:16 +0800, Bao Xiaowei wrote:
> >>>> CAUTION: This email originated from outside of the organization. Do not
> click links or open attachments unless you recognize the sender and know the
> content is safe.
> >>>>
> >>>>
> >>>> For some special reset times for longer pcie devices, the pcie
> >>>> device may on polling compliance state, the RC considers the pcie
> >>>> device is link up, but the pcie device is not link up, only the L0
> >>>> state is link up state.
> >>>>
> >>>> Signed-off-by: Bao Xiaowei <xiaowei.bao at nxp.com>
> >>>> ---
> >>>>  arch/powerpc/include/asm/fsl_pci.h |  2 ++
> >>>>  drivers/pci/fsl_pci_init.c         | 10 ++++++----
> >>>>  2 files changed, 8 insertions(+), 4 deletions(-)
> >>>>
> >>>> diff --git a/arch/powerpc/include/asm/fsl_pci.h
> >>>> b/arch/powerpc/include/asm/fsl_pci.h
> >>>> index 70a5461..323b182 100644
> >>>> --- a/arch/powerpc/include/asm/fsl_pci.h
> >>>> +++ b/arch/powerpc/include/asm/fsl_pci.h
> >>>> @@ -25,6 +25,8 @@
> >>>>  #define PCI_LTSSM      0x404   /* PCIe Link Training, Status State Machine
> */
> >>>>  #define PCI_LTSSM_L0   0x16    /* L0 state */
> >>>>  #define PCI_LTSSM_L0_PEX_REV3  0x11    /* L0 state for pex rev3*/
> >>>> +#define LTSSM_PCIE_DETECT_QUIET                0x00    /* Detect state */
> >>>> +#define LTSSM_PCIE_DETECT_ACTIVE       0x01    /* Detect state */
> >>>>
> >>>>  int fsl_setup_hose(struct pci_controller *hose, unsigned long
> >>>> addr); int fsl_is_pci_agent(struct pci_controller *hose); diff
> >>>> --git a/drivers/pci/fsl_pci_init.c b/drivers/pci/fsl_pci_init.c
> >>>> index
> >>>> be57e53..9b5f386 100644
> >>>> --- a/drivers/pci/fsl_pci_init.c
> >>>> +++ b/drivers/pci/fsl_pci_init.c
> >>>> @@ -335,15 +335,17 @@ static int fsl_pci_link_up(struct pci_controller
> *hose,
> >>>>                 pci_ltssm_l0 = PCI_LTSSM_L0;
> >>>>
> >>>>         ltssm = fsl_get_ltssm(hose, pci_info);
> >>>> -
> >>>> -       if (ltssm == pci_ltssm_l0) {
> >>>> +       if (ltssm == LTSSM_PCIE_DETECT_QUIET ||
> >>>> +               ltssm == LTSSM_PCIE_DETECT_ACTIVE) {
> >>>> +               enabled = 0;
> >>>> +       } else if (ltssm == pci_ltssm_l0) {
> >>>>                 enabled = 1;
> >>>>         } else {
> >>>> -               for (i = 0; i < 100 && ltssm < pci_ltssm_l0; i++) {
> >>>> +               for (i = 0; i < 100 && ltssm != pci_ltssm_l0; i++)
> >>>> + {
> >>>>                         ltssm = fsl_get_ltssm(hose, pci_info);
> >>>>                         udelay(1000);
> >>>
> >>> Do you really need this long loop here ? It causes a long delay in
> >>> case the PCIe device is in permanent polling state. Our device is in
> >>> polling state until clocks is configured and that will be done from
> >>> user space in Linux
> >>>
> >>> Yes, if the pcie device is in permanent polling state, it will take probably
> 100ms delay, but this case is occur very few special devices, if we want to use
> the pcie device in uboot, we have to wait the device link up state is ok, so need
> some time to wait the pcie device ready. if the pcie slot have no device, the
> function will return at once, will not bring delay.
> >>
> >> Joakim,
> >>
> >> Are we OK with this change? Can you test it to make sure no negative
> >> impact on your boards?
> >
> > Not really happy, what device needs 100 ms extra to exit polling state?
> >
> 
> Joakim,
>  
> I understand your concern. I don't like the delay either.
> 
> Xiaowei,
> 
> You said this only occur for very few special devices. Can you explain more?
> How can you avoid this delay for general cases?
> 
> York

Hi York,

For the general pcie devices, it will not bring delay, because the RC access these
devices can get the link up state correctly, usually, if the slot have the device, 
return the L0 state, if the slot have not device, return DETECT state, but for 
a few device, the initial phase it may be polling state(this issue is feedback from
customer, we have not this device), After a period of time, the device resume the
L0 state, so for this  type of device, we need wait a moment, and about the 1 ms delay,
It is  estimate value, in other words, if this type device resume the L0 state within 
the 1ms, we consider that it is link up.