From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-11.2 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7A2CEC2D0A8 for ; Wed, 23 Sep 2020 20:17:35 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id F10F8206D9 for ; Wed, 23 Sep 2020 20:17:34 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="CzWuSkWl"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="S87XfJac" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org F10F8206D9 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Message-ID:Subject:To:From: Date:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:References:List-Owner; bh=urxWcblLlnwA6LsfzWX1C1jsQtxpbX4oby/G9LyWZB0=; b=CzWuSkWl3aHykhwVGKygqKz5u /fZ7OcrTvJNeDLO/FUrWNdNUgvF1XdV5IsgoUixzIYKnqmHPNpXRrHhDv4Bglcasw7H3fEWI9Kzfz hk0ddMKnp6nP1eF0X1+k7MbyQnzNiAq1ARhVibRdHoP5p1ZyiAb04sT7h3QsGeH4sdR7EvzOKcuiS aR09iRealkqXRZtLRQY9C36EVdeJDmWWI5Hq9vKNuXI42gDlbr2OifBPMEBmsd8aKiqFqM6ClGTt5 87SnxhDLLo7mIQ0XewINRNGP07Pe0teNfYqRE4Uy9EctE2nJX5cHqetGc33IXGm3L+XSrJKD2v4uV chk5h3tiQ==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kLBBc-00034h-Js; Wed, 23 Sep 2020 20:16:20 +0000 Received: from mail.kernel.org ([198.145.29.99]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kLBBY-00033R-G8 for linux-arm-kernel@lists.infradead.org; Wed, 23 Sep 2020 20:16:18 +0000 Received: from localhost (52.sub-72-107-123.myvzw.com [72.107.123.52]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 1FA17206D9; Wed, 23 Sep 2020 20:16:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1600892175; bh=kBHE5F+4sQBEEv3YvIGz5yx2kVnlkeU6BE+dWdGgrq4=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=S87XfJac/gD+PaUViJsXXG0/Cvc29LXJXaQ7+NpltSUREgXOQhTRO28AuP1k5MHDg Z1D40tf9Y8Rua7Vq3cDYVUtJ8k2XICRbE6AOMrw5T+RsoVwOhDA0QxzViS+HPCFA4S 4HfmzYberjKoX8B0KzUm3I8lWupEVDNn9a0gdpJQ= Date: Wed, 23 Sep 2020 15:16:13 -0500 From: Bjorn Helgaas To: Xiaowei Bao Subject: Re: [PATCH v6 04/11] PCI: designware-ep: Modify MSI and MSIX CAP way of finding Message-ID: <20200923201613.GA2291357@bjorn-Precision-5520> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200314033038.24844-5-xiaowei.bao@nxp.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200923_161616_702031_A42E4661 X-CRM114-Status: GOOD ( 28.97 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: roy.zang@nxp.com, lorenzo.pieralisi@arm.com, devicetree@vger.kernel.org, jingoohan1@gmail.com, Zhiqiang.Hou@nxp.com, linuxppc-dev@lists.ozlabs.org, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, leoyang.li@nxp.com, Minghuan.Lian@nxp.com, robh+dt@kernel.org, linux-arm-kernel@lists.infradead.org, gustavo.pimentel@synopsys.com, bhelgaas@google.com, andrew.murray@arm.com, kishon@ti.com, shawnguo@kernel.org, mingkai.hu@nxp.com, amurray@thegoodpenguin.co.uk Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org s/MSIX/MSI-X/ (subject and below) On Sat, Mar 14, 2020 at 11:30:31AM +0800, Xiaowei Bao wrote: > Each PF of EP device should have it's own MSI or MSIX capabitily > struct, so create a dw_pcie_ep_func struct and remove the msi_cap > and msix_cap to this struct from dw_pcie_ep, and manage the PFs > with a list. s/capabitily/capability/ I know Lorenzo has already applied this, but for the future, or in case there are other reasons to update this patch. There are a bunch of unnecessary initializations below for future cleanup. > Signed-off-by: Xiaowei Bao > --- > v3: > - This is a new patch, to fix the issue of MSI and MSIX CAP way of > finding. > v4: > - Correct some word of commit message. > v5: > - No change. > v6: > - Fix up the compile error. > > drivers/pci/controller/dwc/pcie-designware-ep.c | 135 +++++++++++++++++++++--- > drivers/pci/controller/dwc/pcie-designware.h | 18 +++- > 2 files changed, 134 insertions(+), 19 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > index 933bb89..fb915f2 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > @@ -19,6 +19,19 @@ void dw_pcie_ep_linkup(struct dw_pcie_ep *ep) > pci_epc_linkup(epc); > } > > +struct dw_pcie_ep_func * > +dw_pcie_ep_get_func_from_ep(struct dw_pcie_ep *ep, u8 func_no) > +{ > + struct dw_pcie_ep_func *ep_func; > + > + list_for_each_entry(ep_func, &ep->func_list, list) { > + if (ep_func->func_no == func_no) > + return ep_func; > + } > + > + return NULL; > +} > + > static unsigned int dw_pcie_ep_func_select(struct dw_pcie_ep *ep, u8 func_no) > { > unsigned int func_offset = 0; > @@ -59,6 +72,47 @@ void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar) > __dw_pcie_ep_reset_bar(pci, func_no, bar, 0); > } > > +static u8 __dw_pcie_ep_find_next_cap(struct dw_pcie_ep *ep, u8 func_no, > + u8 cap_ptr, u8 cap) > +{ > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep); > + unsigned int func_offset = 0; Unnecessary initialization. > + u8 cap_id, next_cap_ptr; > + u16 reg; > + > + if (!cap_ptr) > + return 0; > + > + func_offset = dw_pcie_ep_func_select(ep, func_no); > + > + reg = dw_pcie_readw_dbi(pci, func_offset + cap_ptr); > + cap_id = (reg & 0x00ff); > + > + if (cap_id > PCI_CAP_ID_MAX) > + return 0; > + > + if (cap_id == cap) > + return cap_ptr; > + > + next_cap_ptr = (reg & 0xff00) >> 8; > + return __dw_pcie_ep_find_next_cap(ep, func_no, next_cap_ptr, cap); > +} > + > +static u8 dw_pcie_ep_find_capability(struct dw_pcie_ep *ep, u8 func_no, u8 cap) > +{ > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep); > + unsigned int func_offset = 0; Unnecessary initialization. > + u8 next_cap_ptr; > + u16 reg; > + > + func_offset = dw_pcie_ep_func_select(ep, func_no); > + > + reg = dw_pcie_readw_dbi(pci, func_offset + PCI_CAPABILITY_LIST); > + next_cap_ptr = (reg & 0x00ff); > + > + return __dw_pcie_ep_find_next_cap(ep, func_no, next_cap_ptr, cap); > +} > + > static int dw_pcie_ep_write_header(struct pci_epc *epc, u8 func_no, > struct pci_epf_header *hdr) > { > @@ -246,13 +300,18 @@ static int dw_pcie_ep_get_msi(struct pci_epc *epc, u8 func_no) > struct dw_pcie *pci = to_dw_pcie_from_ep(ep); > u32 val, reg; > unsigned int func_offset = 0; Unnecessary initialization (not from your patch). > + struct dw_pcie_ep_func *ep_func; > > - if (!ep->msi_cap) > + ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no); > + if (!ep_func) > + return -EINVAL; > + > + if (!ep_func->msi_cap) > return -EINVAL; > > func_offset = dw_pcie_ep_func_select(ep, func_no); > > - reg = ep->msi_cap + func_offset + PCI_MSI_FLAGS; > + reg = ep_func->msi_cap + func_offset + PCI_MSI_FLAGS; > val = dw_pcie_readw_dbi(pci, reg); > if (!(val & PCI_MSI_FLAGS_ENABLE)) > return -EINVAL; > @@ -268,13 +327,18 @@ static int dw_pcie_ep_set_msi(struct pci_epc *epc, u8 func_no, u8 interrupts) > struct dw_pcie *pci = to_dw_pcie_from_ep(ep); > u32 val, reg; > unsigned int func_offset = 0; Unnecessary initialization (not from your patch). > + struct dw_pcie_ep_func *ep_func; > + > + ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no); > + if (!ep_func) > + return -EINVAL; > > - if (!ep->msi_cap) > + if (!ep_func->msi_cap) > return -EINVAL; > > func_offset = dw_pcie_ep_func_select(ep, func_no); > > - reg = ep->msi_cap + func_offset + PCI_MSI_FLAGS; > + reg = ep_func->msi_cap + func_offset + PCI_MSI_FLAGS; > val = dw_pcie_readw_dbi(pci, reg); > val &= ~PCI_MSI_FLAGS_QMASK; > val |= (interrupts << 1) & PCI_MSI_FLAGS_QMASK; > @@ -291,13 +355,18 @@ static int dw_pcie_ep_get_msix(struct pci_epc *epc, u8 func_no) > struct dw_pcie *pci = to_dw_pcie_from_ep(ep); > u32 val, reg; > unsigned int func_offset = 0; Unnecessary initialization (not from your patch). > + struct dw_pcie_ep_func *ep_func; > + > + ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no); > + if (!ep_func) > + return -EINVAL; > > - if (!ep->msix_cap) > + if (!ep_func->msix_cap) > return -EINVAL; > > func_offset = dw_pcie_ep_func_select(ep, func_no); > > - reg = ep->msix_cap + func_offset + PCI_MSIX_FLAGS; > + reg = ep_func->msix_cap + func_offset + PCI_MSIX_FLAGS; > val = dw_pcie_readw_dbi(pci, reg); > if (!(val & PCI_MSIX_FLAGS_ENABLE)) > return -EINVAL; > @@ -313,13 +382,18 @@ static int dw_pcie_ep_set_msix(struct pci_epc *epc, u8 func_no, u16 interrupts) > struct dw_pcie *pci = to_dw_pcie_from_ep(ep); > u32 val, reg; > unsigned int func_offset = 0; Unnecessary initialization (not from your patch). > + struct dw_pcie_ep_func *ep_func; > > - if (!ep->msix_cap) > + ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no); > + if (!ep_func) > + return -EINVAL; > + > + if (!ep_func->msix_cap) > return -EINVAL; > > func_offset = dw_pcie_ep_func_select(ep, func_no); > > - reg = ep->msix_cap + func_offset + PCI_MSIX_FLAGS; > + reg = ep_func->msix_cap + func_offset + PCI_MSIX_FLAGS; > val = dw_pcie_readw_dbi(pci, reg); > val &= ~PCI_MSIX_FLAGS_QSIZE; > val |= interrupts; > @@ -404,6 +478,7 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no, > u8 interrupt_num) > { > struct dw_pcie *pci = to_dw_pcie_from_ep(ep); > + struct dw_pcie_ep_func *ep_func; > struct pci_epc *epc = ep->epc; > unsigned int aligned_offset; > unsigned int func_offset = 0; Unnecessary initialization (not from your patch). > @@ -413,25 +488,29 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no, > bool has_upper; > int ret; > > - if (!ep->msi_cap) > + ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no); > + if (!ep_func) > + return -EINVAL; > + > + if (!ep_func->msi_cap) > return -EINVAL; > > func_offset = dw_pcie_ep_func_select(ep, func_no); > > /* Raise MSI per the PCI Local Bus Specification Revision 3.0, 6.8.1. */ > - reg = ep->msi_cap + func_offset + PCI_MSI_FLAGS; > + reg = ep_func->msi_cap + func_offset + PCI_MSI_FLAGS; > msg_ctrl = dw_pcie_readw_dbi(pci, reg); > has_upper = !!(msg_ctrl & PCI_MSI_FLAGS_64BIT); > - reg = ep->msi_cap + func_offset + PCI_MSI_ADDRESS_LO; > + reg = ep_func->msi_cap + func_offset + PCI_MSI_ADDRESS_LO; > msg_addr_lower = dw_pcie_readl_dbi(pci, reg); > if (has_upper) { > - reg = ep->msi_cap + func_offset + PCI_MSI_ADDRESS_HI; > + reg = ep_func->msi_cap + func_offset + PCI_MSI_ADDRESS_HI; > msg_addr_upper = dw_pcie_readl_dbi(pci, reg); > - reg = ep->msi_cap + func_offset + PCI_MSI_DATA_64; > + reg = ep_func->msi_cap + func_offset + PCI_MSI_DATA_64; > msg_data = dw_pcie_readw_dbi(pci, reg); > } else { > msg_addr_upper = 0; > - reg = ep->msi_cap + func_offset + PCI_MSI_DATA_32; > + reg = ep_func->msi_cap + func_offset + PCI_MSI_DATA_32; > msg_data = dw_pcie_readw_dbi(pci, reg); > } > aligned_offset = msg_addr_lower & (epc->mem->page_size - 1); > @@ -467,6 +546,7 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no, > u16 interrupt_num) > { > struct dw_pcie *pci = to_dw_pcie_from_ep(ep); > + struct dw_pcie_ep_func *ep_func; > struct pci_epc *epc = ep->epc; > u16 tbl_offset, bir; > unsigned int func_offset = 0; Unnecessary initialization (not from your patch). > @@ -477,9 +557,16 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no, > void __iomem *msix_tbl; > int ret; > > + ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no); > + if (!ep_func) > + return -EINVAL; > + > + if (!ep_func->msix_cap) > + return -EINVAL; > + > func_offset = dw_pcie_ep_func_select(ep, func_no); > > - reg = ep->msix_cap + func_offset + PCI_MSIX_TABLE; > + reg = ep_func->msix_cap + func_offset + PCI_MSIX_TABLE; > tbl_offset = dw_pcie_readl_dbi(pci, reg); > bir = (tbl_offset & PCI_MSIX_TABLE_BIR); > tbl_offset &= PCI_MSIX_TABLE_OFFSET; > @@ -558,6 +645,7 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) > int i; > int ret; > u32 reg; > + u8 func_no; > void *addr; > u8 hdr_type; > unsigned int nbars; > @@ -566,6 +654,9 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) > struct dw_pcie *pci = to_dw_pcie_from_ep(ep); > struct device *dev = pci->dev; > struct device_node *np = dev->of_node; > + struct dw_pcie_ep_func *ep_func; > + > + INIT_LIST_HEAD(&ep->func_list); > > if (!pci->dbi_base || !pci->dbi_base2) { > dev_err(dev, "dbi_base/dbi_base2 is not populated\n"); > @@ -632,9 +723,19 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) > if (ret < 0) > epc->max_functions = 1; > > - ep->msi_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI); > + for (func_no = 0; func_no < epc->max_functions; func_no++) { > + ep_func = devm_kzalloc(dev, sizeof(*ep_func), GFP_KERNEL); > + if (!ep_func) > + return -ENOMEM; > > - ep->msix_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSIX); > + ep_func->func_no = func_no; > + ep_func->msi_cap = dw_pcie_ep_find_capability(ep, func_no, > + PCI_CAP_ID_MSI); > + ep_func->msix_cap = dw_pcie_ep_find_capability(ep, func_no, > + PCI_CAP_ID_MSIX); > + > + list_add_tail(&ep_func->list, &ep->func_list); > + } > > if (ep->ops->ep_init) > ep->ops->ep_init(ep); > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h > index cb32afa..dd9b7b4 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.h > +++ b/drivers/pci/controller/dwc/pcie-designware.h > @@ -230,8 +230,16 @@ struct dw_pcie_ep_ops { > unsigned int (*func_conf_select)(struct dw_pcie_ep *ep, u8 func_no); > }; > > +struct dw_pcie_ep_func { > + struct list_head list; > + u8 func_no; > + u8 msi_cap; /* MSI capability offset */ > + u8 msix_cap; /* MSI-X capability offset */ > +}; > + > struct dw_pcie_ep { > struct pci_epc *epc; > + struct list_head func_list; > const struct dw_pcie_ep_ops *ops; > phys_addr_t phys_base; > size_t addr_size; > @@ -244,8 +252,6 @@ struct dw_pcie_ep { > u32 num_ob_windows; > void __iomem *msi_mem; > phys_addr_t msi_mem_phys; > - u8 msi_cap; /* MSI capability offset */ > - u8 msix_cap; /* MSI-X capability offset */ > }; > > struct dw_pcie_ops { > @@ -437,6 +443,8 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no, > int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8 func_no, > u16 interrupt_num); > void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar); > +struct dw_pcie_ep_func * > +dw_pcie_ep_get_func_from_ep(struct dw_pcie_ep *ep, u8 func_no); > #else > static inline void dw_pcie_ep_linkup(struct dw_pcie_ep *ep) > { > @@ -478,5 +486,11 @@ static inline int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, > static inline void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar) > { > } > + > +static inline struct dw_pcie_ep_func * > +dw_pcie_ep_get_func_from_ep(struct dw_pcie_ep *ep, u8 func_no) > +{ > + return NULL; > +} > #endif > #endif /* _PCIE_DESIGNWARE_H */ > -- > 2.9.5 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel