From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on archive.lwn.net X-Spam-Level: X-Spam-Status: No, score=-5.6 required=5.0 tests=DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI, T_DKIM_INVALID autolearn=ham autolearn_force=no version=3.4.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by archive.lwn.net (Postfix) with ESMTP id EDA557DE78 for ; Thu, 26 Apr 2018 15:31:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756723AbeDZPbX (ORCPT ); Thu, 26 Apr 2018 11:31:23 -0400 Received: from smtprelay6.synopsys.com ([198.182.37.59]:60006 "EHLO smtprelay.synopsys.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756414AbeDZPbW (ORCPT ); Thu, 26 Apr 2018 11:31:22 -0400 Received: from mailhost.synopsys.com (mailhost1.synopsys.com [10.12.238.239]) by smtprelay.synopsys.com (Postfix) with ESMTP id 91BCD1E05F3; Thu, 26 Apr 2018 17:31:20 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=synopsys.com; s=mail; t=1524756680; bh=eSfWkbZh6v6jR3zwGlgM3B04au0pMxNWmsfwlkzsXNg=; h=Subject:To:Cc:References:From:Date:In-Reply-To:From; b=MkXA81yaChRyBB1pmnAypjnHD01Agg6FqpvbDSlvBcJN0y9ONd5ZRVIiNurXzV7IC ZnC4oHyb4dSOiXuXvt5wCmXRO7VwOT9N595OOGGJoy0oWKO0jlQb4DoFnw95waXS9q NWecgQxS9ubCeZfl5Z42TbS7md4/DpVRT2moNVu7fiXH6rSkhiF235foQEvFSyE3M5 eWZJ6XSs0Nu/bcXJOo2e+5eW9bvuRqnW/xziGbLtyPj5KDv1JTwBvAoNytUp4mDPs7 rw3GeSv6Tl7Z7z+Mcgu0eHLxIdrsz1SiTGA3A2xBqgampuuuTNwM1a8t99p59GCnXF lrasRvL6phvWw== Received: from pt02.synopsys.com (pt02.synopsys.com [10.107.23.240]) by mailhost.synopsys.com (Postfix) with ESMTP id 2CE2755D7; Thu, 26 Apr 2018 08:31:19 -0700 (PDT) Received: from [127.0.0.1] (gustavo-e7480.internal.synopsys.com [10.107.25.102]) by pt02.synopsys.com (Postfix) with ESMTP id 8417B3DA2A; Thu, 26 Apr 2018 16:31:18 +0100 (WEST) Subject: Re: [RFC 01/10] PCI: dwc: Add MSI-X callbacks handler To: Kishon Vijay Abraham I , "bhelgaas@google.com" , "lorenzo.pieralisi@arm.com" , "Joao.Pinto@synopsys.com" , "jingoohan1@gmail.com" , "adouglas@cadence.com" , "niklas.cassel@axis.com" , "jesper.nilsson@axis.com" Cc: "linux-pci@vger.kernel.org" , "linux-doc@vger.kernel.org" , "linux-kernel@vger.kernel.org" References: <77b7b2687e9618d3f7d1f11c3fc6ecec9a9442ef.1523379766.git.gustavo.pimentel@synopsys.com> <0b7023d9-29c6-0993-07d3-b046d25b67ff@ti.com> <0e8b8ce9-db12-1e11-3eb5-62f3fa686d59@ti.com> <16d237b2-0052-447e-13cb-bc2f15848be4@synopsys.com> From: Gustavo Pimentel Message-ID: <45ad7d7e-9986-cebe-c2c0-3f0693b56a9b@synopsys.com> Date: Thu, 26 Apr 2018 16:30:09 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-doc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-doc@vger.kernel.org Hi Kishon, On 24/04/2018 12:24, Kishon Vijay Abraham I wrote: > Hi, > > On Tuesday 24 April 2018 03:06 PM, Gustavo Pimentel wrote: >> Hi Kishon, >> >> On 24/04/2018 08:07, Kishon Vijay Abraham I wrote: >>> Hi, >>> >>> On Monday 23 April 2018 03:06 PM, Gustavo Pimentel wrote: >>>> Hi Kishon, >>>> >>>> On 16/04/2018 10:29, Kishon Vijay Abraham I wrote: >>>>> Hi Gustavo, >>>>> >>>>> On Tuesday 10 April 2018 10:44 PM, Gustavo Pimentel wrote: >>>>>> Changes the pcie_raise_irq function signature, namely the interrupt_num >>>>>> variable type from u8 to u16 to accommodate the MSI-X maximum interrupts >>>>>> of 2048. >>>>>> >>>>>> Implements a PCIe config space capability iterator function to search and >>>>>> save the MSI and MSI-X pointers. With this method the code becomes more >>>>>> generic and flexible. >>>>>> >>>>>> Implements MSI-X set/get functions for sysfs interface in order to change >>>>>> the EP entries number. >>>>>> >>>>>> Implements EP MSI-X interface for triggering interruptions. >>>>>> >>>>>> Signed-off-by: Gustavo Pimentel >>>>>> --- >>>>>> drivers/pci/dwc/pci-dra7xx.c | 2 +- >>>>>> drivers/pci/dwc/pcie-artpec6.c | 2 +- >>>>>> drivers/pci/dwc/pcie-designware-ep.c | 145 ++++++++++++++++++++++++++++++++- >>>>>> drivers/pci/dwc/pcie-designware-plat.c | 6 +- >>>>>> drivers/pci/dwc/pcie-designware.h | 23 +++++- >>>>>> 5 files changed, 173 insertions(+), 5 deletions(-) >>>>>> >>>>>> diff --git a/drivers/pci/dwc/pci-dra7xx.c b/drivers/pci/dwc/pci-dra7xx.c >>>>>> index ed8558d..5265725 100644 >>>>>> --- a/drivers/pci/dwc/pci-dra7xx.c >>>>>> +++ b/drivers/pci/dwc/pci-dra7xx.c >>>>>> @@ -369,7 +369,7 @@ static void dra7xx_pcie_raise_msi_irq(struct dra7xx_pcie *dra7xx, >>>>>> } >>>>>> >>>>>> static int dra7xx_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no, >>>>>> - enum pci_epc_irq_type type, u8 interrupt_num) >>>>>> + enum pci_epc_irq_type type, u16 interrupt_num) >>>>>> { >>>>>> struct dw_pcie *pci = to_dw_pcie_from_ep(ep); >>>>>> struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pci); >>>>>> diff --git a/drivers/pci/dwc/pcie-artpec6.c b/drivers/pci/dwc/pcie-artpec6.c >>>>>> index e66cede..96dc259 100644 >>>>>> --- a/drivers/pci/dwc/pcie-artpec6.c >>>>>> +++ b/drivers/pci/dwc/pcie-artpec6.c >>>>>> @@ -428,7 +428,7 @@ static void artpec6_pcie_ep_init(struct dw_pcie_ep *ep) >>>>>> } >>>>>> >>>>>> static int artpec6_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no, >>>>>> - enum pci_epc_irq_type type, u8 interrupt_num) >>>>>> + enum pci_epc_irq_type type, u16 interrupt_num) >>>>>> { >>>>>> struct dw_pcie *pci = to_dw_pcie_from_ep(ep); >>>>>> >>>>>> diff --git a/drivers/pci/dwc/pcie-designware-ep.c b/drivers/pci/dwc/pcie-designware-ep.c >>>>>> index 15b22a6..874d4c2 100644 >>>>>> --- a/drivers/pci/dwc/pcie-designware-ep.c >>>>>> +++ b/drivers/pci/dwc/pcie-designware-ep.c >>>>>> @@ -40,6 +40,44 @@ void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar) >>>>>> __dw_pcie_ep_reset_bar(pci, bar, 0); >>>>>> } >>>>>> >>>>>> +void dw_pcie_ep_find_cap_addr(struct dw_pcie_ep *ep) >>>>>> +{ >>>>> >>>>> This should be implemented in a generic way similar to pci_find_capability(). >>>>> It'll be useful when we try to implement other capabilities as well. >>>> >>>> Hum, what you suggest? Something implemented on the pci-epf-core? >>> >>> yeah, Initially thought it could be implemented as a helper function in >>> pci-epc-core so that both designware and cadence can use it. >> >> That would be nice, however I couldn't find out how to access the config space, >> through the pci_epf or pci_epc structs. > > It's just a helper function so it can directly take the base address of the > configuration space as argument (in our case, it should be dbi_base). I don't think it will bring much benefit to this particular scope at this time being. In any case this could be improved later. Regards, Gustavo > > Thanks > Kishon > >> >> So, I reworked the functions like this: >> >> (on pcie-designware-ep.c) >> >> u8 __dw_pcie_ep_find_next_cap(struct dw_pcie *pci, u8 cap_ptr, >> u8 cap) >> { >> u8 cap_id, next_cap_ptr; >> u16 reg; >> >> reg = dw_pcie_readw_dbi(pci, cap_ptr); >> next_cap_ptr = (reg & 0xff00) >> 8; >> cap_id = (reg & 0x00ff); >> >> if (!next_cap_ptr || cap_id > PCI_CAP_ID_MAX) >> return 0; >> >> if (cap_id == cap) >> return cap_ptr; >> >> return __dw_pcie_ep_find_next_cap(pci, next_cap_ptr, cap); >> } >> >> u8 dw_pcie_ep_find_capability(struct dw_pcie *pci, u8 cap) >> { >> u8 next_cap_ptr; >> u16 reg; >> >> reg = dw_pcie_readw_dbi(pci, PCI_CAPABILITY_LIST); >> next_cap_ptr = (reg & 0x00ff); >> >> if (!next_cap_ptr) >> return 0; >> >> return __dw_pcie_ep_find_next_cap(pci, next_cap_ptr, cap); >> } >> >> int dw_pcie_ep_init(struct dw_pcie_ep *ep) >> { >> [...] >> ep->msi_cap = dw_pcie_ep_find_capability(pci, PCI_CAP_ID_MSI); >> ep->msix_cap = dw_pcie_ep_find_capability(pci, PCI_CAP_ID_MSIX); >> [...] >> } >> >>> >>> But do we really have to find the address like this? since all designware IP's >>> will have a particular capability at a fixed address offset, why not follow use >>> existing mechanism in dw_pcie_ep_get_msi? >> >> The capabilities are not fixed to a specific address offset by default they >> assume those values, but they can be easily change at design stage. >> >>> >>> Or is it possible for a particular capability to have address offsets for >>> different vendors? How is it for cadence? >> >> Yes, it's possible to have different address offset for different vendors. >> >>> >>> Thanks >>> Kishon >>> >> >> Thanks, >> Gustavo >> -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html