2019-11-07 09:41:54

by Jiasen Lin

[permalink] [raw]
Subject: [PATCH] NTB: Fix an error in get link status

The offset of PCIe Capability Header for AMD and HYGON NTB is 0x64,
but the macro which named "AMD_LINK_STATUS_OFFSET" is defined as 0x68.
It is offset of Device Capabilities Reg rather than Link Control Reg.

This code trigger an error in get link statsus:

cat /sys/kernel/debug/ntb_hw_amd/0000:43:00.1/info
LNK STA - 0x8fa1
Link Status - Up
Link Speed - PCI-E Gen 0
Link Width - x0

This patch use pcie_capability_read_dword to get link status.
After fix this issue, we can get link status accurately:

cat /sys/kernel/debug/ntb_hw_amd/0000:43:00.1/info
LNK STA - 0x11030042
Link Status - Up
Link Speed - PCI-E Gen 3
Link Width - x16

Signed-off-by: Jiasen Lin <[email protected]>
---
drivers/ntb/hw/amd/ntb_hw_amd.c | 5 +++--
drivers/ntb/hw/amd/ntb_hw_amd.h | 1 -
2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c
index 156c2a1..ae91105 100644
--- a/drivers/ntb/hw/amd/ntb_hw_amd.c
+++ b/drivers/ntb/hw/amd/ntb_hw_amd.c
@@ -855,8 +855,8 @@ 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);
+ rc = pcie_capability_read_dword(ndev->ntb.pdev,
+ PCI_EXP_LNKCTL, &stat);
if (rc)
return 0;
ndev->lnk_sta = stat;
@@ -1139,6 +1139,7 @@ static const struct ntb_dev_data dev_data[] = {
static const struct pci_device_id amd_ntb_pci_tbl[] = {
{ PCI_VDEVICE(AMD, 0x145b), (kernel_ulong_t)&dev_data[0] },
{ PCI_VDEVICE(AMD, 0x148b), (kernel_ulong_t)&dev_data[1] },
+ { PCI_VDEVICE(HYGON, 0x145b), (kernel_ulong_t)&dev_data[0] },
{ 0, }
};
MODULE_DEVICE_TABLE(pci, amd_ntb_pci_tbl);
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
--
2.7.4


2019-11-17 23:03:35

by Jon Mason

[permalink] [raw]
Subject: Re: [PATCH] NTB: Fix an error in get link status

On Thu, Nov 7, 2019 at 4:37 AM Jiasen Lin <[email protected]> wrote:
>
> The offset of PCIe Capability Header for AMD and HYGON NTB is 0x64,
> but the macro which named "AMD_LINK_STATUS_OFFSET" is defined as 0x68.
> It is offset of Device Capabilities Reg rather than Link Control Reg.
>
> This code trigger an error in get link statsus:
>
> cat /sys/kernel/debug/ntb_hw_amd/0000:43:00.1/info
> LNK STA - 0x8fa1
> Link Status - Up
> Link Speed - PCI-E Gen 0
> Link Width - x0
>
> This patch use pcie_capability_read_dword to get link status.
> After fix this issue, we can get link status accurately:
>
> cat /sys/kernel/debug/ntb_hw_amd/0000:43:00.1/info
> LNK STA - 0x11030042
> Link Status - Up
> Link Speed - PCI-E Gen 3
> Link Width - x16

No response from AMD maintainers, but it looks like you are correct.

This needs a "Fixes:" line here. I took the liberty of adding one to
this patch.

> Signed-off-by: Jiasen Lin <[email protected]>
> ---
> drivers/ntb/hw/amd/ntb_hw_amd.c | 5 +++--
> drivers/ntb/hw/amd/ntb_hw_amd.h | 1 -
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c
> index 156c2a1..ae91105 100644
> --- a/drivers/ntb/hw/amd/ntb_hw_amd.c
> +++ b/drivers/ntb/hw/amd/ntb_hw_amd.c
> @@ -855,8 +855,8 @@ 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);
> + rc = pcie_capability_read_dword(ndev->ntb.pdev,
> + PCI_EXP_LNKCTL, &stat);
> if (rc)
> return 0;
> ndev->lnk_sta = stat;
> @@ -1139,6 +1139,7 @@ static const struct ntb_dev_data dev_data[] = {
> static const struct pci_device_id amd_ntb_pci_tbl[] = {
> { PCI_VDEVICE(AMD, 0x145b), (kernel_ulong_t)&dev_data[0] },
> { PCI_VDEVICE(AMD, 0x148b), (kernel_ulong_t)&dev_data[1] },
> + { PCI_VDEVICE(HYGON, 0x145b), (kernel_ulong_t)&dev_data[0] },

This should be a separate patch. I took the liberty of splitting it
off into a unique patch and attributing it to you. I've pushed them
to the ntb-next branch on
https://github.com/jonmason/ntb

Please verify everything looks acceptable to you (given the changes I
did above that are attributed to you). Also, testing of the latest
code is always appreciated.

Thanks,
Jon


> { 0, }
> };
> MODULE_DEVICE_TABLE(pci, amd_ntb_pci_tbl);
> 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
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups "linux-ntb" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
> To view this discussion on the web visit https://groups.google.com/d/msgid/linux-ntb/1573119336-107732-1-git-send-email-linjiasen%40hygon.cn.

2019-11-18 10:25:37

by Jiasen Lin

[permalink] [raw]
Subject: Re: [PATCH] NTB: Fix an error in get link status



On 2019/11/18 7:00, Jon Mason wrote:
> On Thu, Nov 7, 2019 at 4:37 AM Jiasen Lin <[email protected]> wrote:
>>
>> The offset of PCIe Capability Header for AMD and HYGON NTB is 0x64,
>> but the macro which named "AMD_LINK_STATUS_OFFSET" is defined as 0x68.
>> It is offset of Device Capabilities Reg rather than Link Control Reg.
>>
>> This code trigger an error in get link statsus:
>>
>> cat /sys/kernel/debug/ntb_hw_amd/0000:43:00.1/info
>> LNK STA - 0x8fa1
>> Link Status - Up
>> Link Speed - PCI-E Gen 0
>> Link Width - x0
>>
>> This patch use pcie_capability_read_dword to get link status.
>> After fix this issue, we can get link status accurately:
>>
>> cat /sys/kernel/debug/ntb_hw_amd/0000:43:00.1/info
>> LNK STA - 0x11030042
>> Link Status - Up
>> Link Speed - PCI-E Gen 3
>> Link Width - x16
>
> No response from AMD maintainers, but it looks like you are correct.
>
> This needs a "Fixes:" line here. I took the liberty of adding one to
> this patch.
>

Thank you for your suggestions. Yes, this patch fix the commit id:
a1b3695 ("NTB: Add support for AMD PCI-Express Non-Transparent Bridge").

>> Signed-off-by: Jiasen Lin <[email protected]>
>> ---
>> drivers/ntb/hw/amd/ntb_hw_amd.c | 5 +++--
>> drivers/ntb/hw/amd/ntb_hw_amd.h | 1 -
>> 2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c b/drivers/ntb/hw/amd/ntb_hw_amd.c
>> index 156c2a1..ae91105 100644
>> --- a/drivers/ntb/hw/amd/ntb_hw_amd.c
>> +++ b/drivers/ntb/hw/amd/ntb_hw_amd.c
>> @@ -855,8 +855,8 @@ 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);
>> + rc = pcie_capability_read_dword(ndev->ntb.pdev,
>> + PCI_EXP_LNKCTL, &stat);
>> if (rc)
>> return 0;
>> ndev->lnk_sta = stat;
>> @@ -1139,6 +1139,7 @@ static const struct ntb_dev_data dev_data[] = {
>> static const struct pci_device_id amd_ntb_pci_tbl[] = {
>> { PCI_VDEVICE(AMD, 0x145b), (kernel_ulong_t)&dev_data[0] },
>> { PCI_VDEVICE(AMD, 0x148b), (kernel_ulong_t)&dev_data[1] },
>> + { PCI_VDEVICE(HYGON, 0x145b), (kernel_ulong_t)&dev_data[0] },
>
> This should be a separate patch. I took the liberty of splitting it
> off into a unique patch and attributing it to you. I've pushed them
> to the ntb-next branch on
> https://github.com/jonmason/ntb
>
Thank you for your comment. We appreciate the time and effort you have
spent to split it off, I will test it ASAP.

> Please verify everything looks acceptable to you (given the changes I
> did above that are attributed to you). Also, testing of the latest
> code is always appreciated.
>
> Thanks,
> Jon
>
>
>> { 0, }
>> };
>> MODULE_DEVICE_TABLE(pci, amd_ntb_pci_tbl);
>> 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
>> --
>> 2.7.4
>>
>> --
>> You received this message because you are subscribed to the Google Groups "linux-ntb" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
>> To view this discussion on the web visit https://groups.google.com/d/msgid/linux-ntb/1573119336-107732-1-git-send-email-linjiasen%40hygon.cn.

Thanks,

Jiasen Lin

2019-11-20 11:36:00

by Jiasen Lin

[permalink] [raw]
Subject: Re: [PATCH] NTB: Fix an error in get link status



On 2019/11/18 18:17, Jiasen Lin wrote:
>
>
> On 2019/11/18 7:00, Jon Mason wrote:
>> On Thu, Nov 7, 2019 at 4:37 AM Jiasen Lin <[email protected]> wrote:
>>>
>>> The offset of PCIe Capability Header for AMD and HYGON NTB is 0x64,
>>> but the macro which named "AMD_LINK_STATUS_OFFSET" is defined as 0x68.
>>> It is offset of Device Capabilities Reg rather than Link Control Reg.
>>>
>>> This code trigger an error in get link statsus:
>>>
>>>          cat /sys/kernel/debug/ntb_hw_amd/0000:43:00.1/info
>>>                  LNK STA -               0x8fa1
>>>                  Link Status -           Up
>>>                  Link Speed -            PCI-E Gen 0
>>>                  Link Width -            x0
>>>
>>> This patch use pcie_capability_read_dword to get link status.
>>> After fix this issue, we can get link status accurately:
>>>
>>>          cat /sys/kernel/debug/ntb_hw_amd/0000:43:00.1/info
>>>                  LNK STA -               0x11030042
>>>                  Link Status -           Up
>>>                  Link Speed -            PCI-E Gen 3
>>>                  Link Width -            x16
>>
>> No response from AMD maintainers, but it looks like you are correct.
>>
>> This needs a "Fixes:" line here.  I took the liberty of adding one to
>> this patch.
>>
>
> Thank you for your suggestions. Yes, this patch fix the commit id:
> a1b3695 ("NTB: Add support for AMD PCI-Express Non-Transparent Bridge").
>
>>> Signed-off-by: Jiasen Lin <[email protected]>
>>> ---
>>>   drivers/ntb/hw/amd/ntb_hw_amd.c | 5 +++--
>>>   drivers/ntb/hw/amd/ntb_hw_amd.h | 1 -
>>>   2 files changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/ntb/hw/amd/ntb_hw_amd.c
>>> b/drivers/ntb/hw/amd/ntb_hw_amd.c
>>> index 156c2a1..ae91105 100644
>>> --- a/drivers/ntb/hw/amd/ntb_hw_amd.c
>>> +++ b/drivers/ntb/hw/amd/ntb_hw_amd.c
>>> @@ -855,8 +855,8 @@ 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);
>>> +       rc = pcie_capability_read_dword(ndev->ntb.pdev,
>>> +                                  PCI_EXP_LNKCTL, &stat);
>>>          if (rc)
>>>                  return 0;
>>>          ndev->lnk_sta = stat;
>>> @@ -1139,6 +1139,7 @@ static const struct ntb_dev_data dev_data[] = {
>>>   static const struct pci_device_id amd_ntb_pci_tbl[] = {
>>>          { PCI_VDEVICE(AMD, 0x145b), (kernel_ulong_t)&dev_data[0] },
>>>          { PCI_VDEVICE(AMD, 0x148b), (kernel_ulong_t)&dev_data[1] },
>>> +       { PCI_VDEVICE(HYGON, 0x145b), (kernel_ulong_t)&dev_data[0] },
>>
>> This should be a separate patch.  I took the liberty of splitting it
>> off into a unique patch and attributing it to you.  I've pushed them
>> to the ntb-next branch on
>> https://github.com/jonmason/ntb
>>
> Thank you for your comment. We appreciate the time and effort you have
> spent to split it off, I will test it ASAP.
>
>> Please verify everything looks acceptable to you (given the changes I
>> did above that are attributed to you).  Also, testing of the latest
>> code is always appreciated.
>>
>> Thanks,
>> Jon
>>

I have tested these patches that are pushed to ntb-next branch, they
work well on HYGON platforms.

Thanks,
Jiasen Lin

>>
>>>          { 0, }
>>>   };
>>>   MODULE_DEVICE_TABLE(pci, amd_ntb_pci_tbl);
>>> 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
>>> --
>>> 2.7.4
>>>
>>> --
>>> You received this message because you are subscribed to the Google
>>> Groups "linux-ntb" group.
>>> To unsubscribe from this group and stop receiving emails from it,
>>> send an email to [email protected].
>>> To view this discussion on the web visit
>>> https://groups.google.com/d/msgid/linux-ntb/1573119336-107732-1-git-send-email-linjiasen%40hygon.cn.
>>>
>
> Thanks,
>
> Jiasen Lin