Re: Fwd: [PATCH] NTB: Fix an error in get link status

[Sanjay R Mehta]

> Hi Sanjay R Mehta
>
> In more complex topology, read the Link Status and Control register of
> RP is also wrong. Suppose that a PCIe switch bridge to the Secondary RP,
> and Secondary internal SW.ds is a child device for switch's downstream
> port as illustrated in the following topology.
>
> In secondary PCI domain:
> Secondary RP--Switch UP--Switch DP--Secondary internal SW.us--Secondary
> internal SW.ds--Secondary NTB
>
> pci_rp = pci_find_pcie_root_port(ndev->ntb.pdev) will return the
> Secondary RP, and pcie_capability_read_dword(pci_rp,
> PCI_EXP_LNKCTL,&stat) will get the link status between Secondary RP and
> Switch UP. Maybe, read the Link Status and control register of Secondary
> internal SW.us is appropriate.
>
Hi Jiansen Lin,

I modified the code as per your suggestion and is working fine.
Adding Arindam for comments who was the co-author of the patch I was about to send for upstream review.

Thanks,
Sanjay Mehta
> struct pci_dev *pci_up = NULL;
> struct pci_dev *pci_dp = NULL;
>
> if (ndev->ntb.topo == NTB_TOPO_SEC) {
> ÂÂÂ /* Locate the pointer to Secondary up for this device */
> ÂÂÂ pci_dp = pci_upstream_bridge(ndev->ntb.pdev);
> ÂÂÂ /* Read the PCIe Link Control and Status register */
> ÂÂÂ if (pci_dp) {
> ÂÂÂÂÂÂ pci_up = pci_upstream_bridge(pci_dp);
> ÂÂÂÂÂÂ if (pci_up) {
> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ rc = pcie_capability_read_dword(pci_up, PCI_EXP_LNKCTL,
> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ &stat);
> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ if (rc)
> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ return 0;
> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ }
> ÂÂÂÂÂÂ }
> }
>
> Thanks,
> Jiansen Lin
>
>> I have modified the code according to your suggestion and tested it
>> on Dhyana platform, it works well, expect to receice your patch.
>>
>> Before modify the code, read the Link Status and control register of the
>> secondary NTB device to get link status.
>>
>> cat /sys/kernel/debug/ntb_hw_amd/0000\:43\:00.1/info
>> NTB Device Information:
>> Connection Topology -ÂÂ NTB_TOPO_SEC
>> LNK STA -ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ 0x11030042
>> Link Status -ÂÂÂÂÂÂÂÂÂÂ Up
>> Link Speed -ÂÂÂÂÂÂÂÂÂÂÂ PCI-E Gen 3
>> Link Width -ÂÂÂÂÂÂÂÂÂÂÂ x16
>>
>> After modify the code, read the Link Status and control register of the
>> secondary RP to get link status.
>>
>> cat /sys/kernel/debug/ntb_hw_amd/0000\:43\:00.1/info
>> NTB Device Information:
>> Connection Topology -ÂÂ NTB_TOPO_SEC
>> LNK STA -ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ 0x70830042
>> Link Status -ÂÂÂÂÂÂÂÂÂÂ Up
>> Link Speed -ÂÂÂÂÂÂÂÂÂÂÂ PCI-E Gen 3
>> Link Width -ÂÂÂÂÂÂÂÂÂÂÂ x8
>>
>> Thanks,
>> Jiasen Lin
>>
>>> ---
>>> Â drivers/ntb/hw/amd/ntb_hw_amd.c | 27 +++++++++++++++++++++++----
>>> Â drivers/ntb/hw/amd/ntb_hw_amd.h |Â 1 -
>>> Â 2 files changed, 23 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c
>>> b/drivers/ntb/hw/amd/ntb_hw_amd.c
>>> index 14ad69c..91e1966 100644
>>> --- a/drivers/ntb/hw/amd/ntb_hw_amd.c
>>> +++ b/drivers/ntb/hw/amd/ntb_hw_amd.c
>>> @@ -842,6 +842,8 @@ static inline void ndev_init_struct(struct
>>> amd_ntb_dev *ndev,
>>> Â static int amd_poll_link(struct amd_ntb_dev *ndev)
>>> Â {
>>> ÂÂÂÂÂ void __iomem *mmio = ndev->peer_mmio;
>>> +ÂÂÂ struct pci_dev *pci_rp = NULL;
>>> +ÂÂÂ struct pci_dev *pdev = NULL;
>>> ÂÂÂÂÂ u32 reg, stat;
>>> ÂÂÂÂÂ int rc;
>>> @@ -855,10 +857,27 @@ static int amd_poll_link(struct amd_ntb_dev *ndev)
>>> ÂÂÂÂÂ ndev->cntl_sta = reg;
>>> -ÂÂÂ rc = pci_read_config_dword(ndev->ntb.pdev,
>>> -ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ AMD_LINK_STATUS_OFFSET, &stat);
>>> -ÂÂÂ if (rc)
>>> -ÂÂÂÂÂÂÂ return 0;
>>> +ÂÂÂ if (ndev->ntb.topo == NTB_TOPO_SEC) {
>>> +ÂÂÂÂÂÂÂ /* Locate the pointer to PCIe Root Port for this device */
>>> +ÂÂÂÂÂÂÂ pci_rp = pci_find_pcie_root_port(ndev->ntb.pdev);
>>> +ÂÂÂÂÂÂÂ /* Read the PCIe Link Control and Status register */
>>> +ÂÂÂÂÂÂÂ if (pci_rp) {
>>> +ÂÂÂÂÂÂÂÂÂÂÂ rc = pcie_capability_read_dword(pci_rp, PCI_EXP_LNKCTL,
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ &stat);
>>> +ÂÂÂÂÂÂÂÂÂÂÂ if (rc)
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ return 0;
>>> +ÂÂÂÂÂÂÂ }
>>> +ÂÂÂ } else if (ndev->ntb.topo == NTB_TOPO_PRI) {
>>> +ÂÂÂÂÂÂÂ /*
>>> +ÂÂÂÂÂÂÂÂ * For NTB primary, we simply read the Link Status and control
>>> +ÂÂÂÂÂÂÂÂ * register of the NTB device itself.
>>> +ÂÂÂÂÂÂÂÂ */
>>> +ÂÂÂÂÂÂÂ pdev = ndev->ntb.pdev;
>>> +ÂÂÂÂÂÂÂ rc = pcie_capability_read_dword(pdev, PCI_EXP_LNKCTL, &stat);
>>> +ÂÂÂÂÂÂÂ if (rc)
>>> +ÂÂÂÂÂÂÂÂÂÂÂ return 0;
>>> +ÂÂÂ }
>>> +
>>> ÂÂÂÂÂ ndev->lnk_sta = stat;
>>> ÂÂÂÂÂ return 1;
>>> diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.h
>>> b/drivers/ntb/hw/amd/ntb_hw_amd.h
>>> index 139a307..39e5d18 100644
>>> --- a/drivers/ntb/hw/amd/ntb_hw_amd.h
>>> +++ b/drivers/ntb/hw/amd/ntb_hw_amd.h
>>> @@ -53,7 +53,6 @@
>>> Â #include <linux/pci.h>
>>> Â #define AMD_LINK_HB_TIMEOUTÂÂÂ msecs_to_jiffies(1000)
>>> -#define AMD_LINK_STATUS_OFFSETÂÂÂ 0x68
>>> Â #define NTB_LIN_STA_ACTIVE_BITÂÂÂ 0x00000002
>>> Â #define NTB_LNK_STA_SPEED_MASKÂÂÂ 0x000F0000
>>> Â #define NTB_LNK_STA_WIDTH_MASKÂÂÂ 0x03F00000
>>>