PCI/DPC: Add eDPC support

Commit Message

Dongdong Liu July 13, 2017, 12:20 p.m. UTC

Hi Sinan

Many thanks for your reveiw.

在 2017/7/13 4:52, Sinan Kaya 写道:
> On 6/24/2017 5:04 AM, Dongdong Liu wrote:
>> This code is to add eDPC support. Get and print the RP PIO error
>> information when the trigger condition is RP PIO error.
>>
>> For more information on eDPC, view the PCI-SIG eDPC ECN here:
>> https://pcisig.com/sites/default/files/specification_documents/ECN_Enhanced_DPC_2012-11-19_final.pdf
>>
>> Signed-off-by: Dongdong Liu <[e-mail]>
>> ---
>>  drivers/pci/pcie/pcie-dpc.c   | 161 ++++++++++++++++++++++++++++++++++++++++++
>>  include/uapi/linux/pci_regs.h |  10 +++
>>  2 files changed, 171 insertions(+)
>>
>> diff --git a/drivers/pci/pcie/pcie-dpc.c b/drivers/pci/pcie/pcie-dpc.c
>> index 77d2ca9..00fdf87 100644
>> --- a/drivers/pci/pcie/pcie-dpc.c
>> +++ b/drivers/pci/pcie/pcie-dpc.c
>> @@ -16,11 +16,55 @@
>>  #include <linux/pcieport_if.h>
>>  #include "../pci.h">
>
> <snip>
>
>> +static void dpc_rp_pio_get_info(struct dpc_dev *dpc)
>> +{
>> +	struct pci_dev *pdev = dpc->dev->port;
>> +	struct dpc_rp_pio_regs *rp_pio = dpc->rp_pio;
>> +	int i;
>> +	u16 cap;
>> +	u16 status;
>> +
>> +	pci_read_config_dword(pdev, dpc->cap_pos + PCI_EXP_DPC_RP_PIO_STATUS,
>> +			      &rp_pio->status);
>> +	pci_read_config_dword(pdev, dpc->cap_pos + PCI_EXP_DPC_RP_PIO_MASK,
>> +			      &rp_pio->mask);
>> +
>> +	pci_read_config_dword(pdev, dpc->cap_pos + PCI_EXP_DPC_RP_PIO_SEVERITY,
>> +			      &rp_pio->severity);
>> +	pci_read_config_dword(pdev, dpc->cap_pos + PCI_EXP_DPC_RP_PIO_SYSERROR,
>> +			      &rp_pio->syserror);
>> +	pci_read_config_dword(pdev, dpc->cap_pos + PCI_EXP_DPC_RP_PIO_EXCEPTION,
>> +			      &rp_pio->exception);
>> +
>> +	/* Get First Error Pointer */
>> +	pci_read_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_STATUS, &status);
>> +	rp_pio->first_error = (status & 0x1f00) >> 8;
>> +
>> +	pci_read_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_CAP, &cap);
>> +	rp_pio->log_size = (cap & PCI_EXP_DPC_RP_PIO_LOG_SIZE) >> 8;
>> +	if (rp_pio->log_size < 4 || rp_pio->log_size > 9) {
>> +		dev_err(&pdev->dev, "RP PIO log size %u is invaild\n",
>> +			rp_pio->log_size);
>> +		return;
>> +	}
>> +
>> +	pci_read_config_dword(pdev,
>> +			      dpc->cap_pos + PCI_EXP_DPC_RP_PIO_HEADER_LOG,
>> +			      &rp_pio->header_log.dw0);
>> +	pci_read_config_dword(pdev,
>> +			      dpc->cap_pos + PCI_EXP_DPC_RP_PIO_HEADER_LOG + 4,
>> +			      &rp_pio->header_log.dw1);
>> +	pci_read_config_dword(pdev,
>> +			      dpc->cap_pos + PCI_EXP_DPC_RP_PIO_HEADER_LOG + 8,
>> +			      &rp_pio->header_log.dw2);
>> +	pci_read_config_dword(pdev,
>> +			      dpc->cap_pos + PCI_EXP_DPC_RP_PIO_HEADER_LOG + 12,
>> +			      &rp_pio->header_log.dw3);
>> +	if (rp_pio->log_size == 4)
>> +		return;
>> +
>> +	pci_read_config_dword(pdev,
>> +			      dpc->cap_pos + PCI_EXP_DPC_RP_PIO_IMPSPEC_LOG,
>> +			      &rp_pio->impspec_log);
>> +	for (i = 0; i < rp_pio->log_size - 5; i++)
>> +		pci_read_config_dword(pdev,
>> +			dpc->cap_pos + PCI_EXP_DPC_RP_PIO_TLPPREFIX_LOG,
>> +			&rp_pio->tlp_prefix_log[i]);
>> +}
>> +
>> +static void dpc_precess_rp_pio_error(struct dpc_dev *dpc)
>> +{
>
> Why not have a stack local variable where you collect the information
> on the first function and print it on the second function.
>
> What benefit the allocated memory brings?

will be fixed as below.

>
>> +	dpc_rp_pio_get_info(dpc);
>> +	dpc_rp_pio_print_error(dpc);
>> +}
>> +
>
>> @@ -144,6 +299,12 @@ static int dpc_probe(struct pcie_device *dev)
>>
>>  	dpc->rp = (cap & PCI_EXP_DPC_CAP_RP_EXT);
>>
>> +	if (dpc->rp) {
>> +		dpc->rp_pio = devm_kzalloc(&pdev->dev, sizeof(*dpc->rp_pio),
>> +					   GFP_KERNEL);
>> +		if (!dpc->rp_pio)
>> +			return -ENOMEM;
>> +	}
>>  	ctl |= PCI_EXP_DPC_CTL_EN_NONFATAL | PCI_EXP_DPC_CTL_INT_EN;
>>  	pci_write_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_CTL, ctl);
>>
>

Comments

Sinan Kaya July 13, 2017, 7:25 p.m. UTC | #1

Hi Dongdong,

On 7/13/2017 8:20 AM, Dongdong Liu wrote:
> will be fixed as below.

<snip>

This one looked good to me.

Sinan

Patch

diff --git a/drivers/pci/pcie/pcie-dpc.c b/drivers/pci/pcie/pcie-dpc.c
index 9addc76..77fe097 100644
--- a/drivers/pci/pcie/pcie-dpc.c
+++ b/drivers/pci/pcie/pcie-dpc.c
@@ -42,7 +42,7 @@  struct dpc_dev {
         struct work_struct      work;
         int                     cap_pos;
         bool                    rp;
-   struct dpc_rp_pio_regs  *rp_pio;
+ u32                         rp_pio_status;
  };

  static const char * const rp_pio_error_string[] = {
@@ -123,10 +123,10 @@  static void interrupt_event_handler(struct work_struct *work)
         dpc_wait_link_inactive(pdev);
         if (dpc->rp && dpc_wait_rp_inactive(dpc))
                 return;
-   if (dpc->rp && dpc->rp_pio->status)
+ if (dpc->rp && dpc->rp_pio_status)
                 pci_write_config_dword(pdev,
                                       dpc->cap_pos + PCI_EXP_DPC_RP_PIO_STATUS,
-                                 dpc->rp_pio->status);
+                               dpc->rp_pio_status);

         pci_write_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_STATUS,
                 PCI_EXP_DPC_STATUS_TRIGGER | PCI_EXP_DPC_STATUS_INTERRUPT);
@@ -139,10 +139,9 @@  static void dpc_rp_pio_print_tlp_header(struct pci_dev *dev,
                 t->dw0, t->dw1, t->dw2, t->dw3);
  }

-static void dpc_rp_pio_print_error(struct dpc_dev *dpc)
+static void dpc_rp_pio_print_error(struct dpc_dev *dpc, struct dpc_rp_pio_regs *rp_pio)
  {
         struct pci_dev *pdev = dpc->dev->port;
-   struct dpc_rp_pio_regs *rp_pio = dpc->rp_pio;
         int i;
         u32 status;

@@ -172,10 +171,9 @@  static void dpc_rp_pio_print_error(struct dpc_dev *dpc)
                         rp_pio->tlp_prefix_log[i]);
  }

-static void dpc_rp_pio_get_info(struct dpc_dev *dpc)
+static void dpc_rp_pio_get_info(struct dpc_dev *dpc, struct dpc_rp_pio_regs *rp_pio)
  {
         struct pci_dev *pdev = dpc->dev->port;
-   struct dpc_rp_pio_regs *rp_pio = dpc->rp_pio;
         int i;
         u16 cap;
         u16 status;
@@ -230,8 +228,12 @@  static void dpc_rp_pio_get_info(struct dpc_dev *dpc)

  static void dpc_precess_rp_pio_error(struct dpc_dev *dpc)
  {
-   dpc_rp_pio_get_info(dpc);
-   dpc_rp_pio_print_error(dpc);
+ struct dpc_rp_pio_regs rp_pio_regs;
+
+ dpc_rp_pio_get_info(dpc, &rp_pio_regs);
+ dpc_rp_pio_print_error(dpc, &rp_pio_regs);
+
+ dpc->rp_pio_status = rp_pio_regs.status;
  }

  static irqreturn_t dpc_irq(int irq, void *context)
@@ -299,12 +301,6 @@  static int dpc_probe(struct pcie_device *dev)

         dpc->rp = (cap & PCI_EXP_DPC_CAP_RP_EXT);

-   if (dpc->rp) {
-           dpc->rp_pio = devm_kzalloc(&pdev->dev, sizeof(*dpc->rp_pio),
-                                      GFP_KERNEL);
-           if (!dpc->rp_pio)
-                   return -ENOMEM;
-   }

Thanks
Dongdong