From patchwork Thu Jul 13 12:20:39 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Dongdong Liu X-Patchwork-Id: 9838407 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 896BA602A0 for ; Thu, 13 Jul 2017 12:21:06 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 78CC0286B5 for ; Thu, 13 Jul 2017 12:21:06 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6B39C286CF; Thu, 13 Jul 2017 12:21:06 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id AA22F286B5 for ; Thu, 13 Jul 2017 12:21:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751193AbdGMMVF (ORCPT ); Thu, 13 Jul 2017 08:21:05 -0400 Received: from szxga02-in.huawei.com ([45.249.212.188]:9851 "EHLO szxga02-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751165AbdGMMVE (ORCPT ); Thu, 13 Jul 2017 08:21:04 -0400 Received: from 172.30.72.57 (EHLO dggeml406-hub.china.huawei.com) ([172.30.72.57]) by dggrg02-dlp.huawei.com (MOS 4.4.6-GA FastPath queued) with ESMTP id AQZ19591; Thu, 13 Jul 2017 20:20:49 +0800 (CST) Received: from [127.0.0.1] (10.63.141.25) by dggeml406-hub.china.huawei.com (10.3.17.50) with Microsoft SMTP Server id 14.3.301.0; Thu, 13 Jul 2017 20:20:39 +0800 Subject: Re: PCI/DPC: Add eDPC support To: Sinan Kaya , References: <1498295066-87895-1-git-send-email-liudongdong3@huawei.com> CC: , , , From: Dongdong Liu Message-ID: <75f291f2-f820-82d5-2249-440009939a4f@huawei.com> Date: Thu, 13 Jul 2017 20:20:39 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: X-Originating-IP: [10.63.141.25] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A090205.596765A3.008B, ss=1, re=0.000, recu=0.000, reip=0.000, cl=1, cld=1, fgs=0, ip=0.0.0.0, so=2014-11-16 11:51:01, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: dd4ac7d66f3f013771e8a4382c1be6ae Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 >> --- >> 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 >> #include "../pci.h"> > > > >> +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); >> > 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