From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752272AbdJPLn6 (ORCPT ); Mon, 16 Oct 2017 07:43:58 -0400 Received: from bastet.se.axis.com ([195.60.68.11]:52096 "EHLO bastet.se.axis.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752088AbdJPLnt (ORCPT ); Mon, 16 Oct 2017 07:43:49 -0400 Subject: Re: [PATCH 01/10] PCI: dwc: use DMA-API for allocating MSI data To: David Laight , Jingoo Han , Joao Pinto , Bjorn Helgaas CC: "linux-pci@vger.kernel.org" , "linux-kernel@vger.kernel.org" References: <20171013160914.3220-1-niklas.cassel@axis.com> <20171013160914.3220-2-niklas.cassel@axis.com> <063D6719AE5E284EB5DD2968C1650D6DD009511B@AcuExch.aculab.com> From: Niklas Cassel Message-ID: <2057f883-0c02-3e56-f3c2-ecc90e778e8e@axis.com> Date: Mon, 16 Oct 2017 13:43:46 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DD009511B@AcuExch.aculab.com> Content-Type: text/plain; charset="windows-1252" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.0.5.60] X-ClientProxiedBy: XBOX01.axis.com (10.0.5.15) To XBOX02.axis.com (10.0.5.16) X-TM-AS-GCONF: 00 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/13/2017 06:42 PM, David Laight wrote: > From: Behalf Of Niklas Cassel >> Sent: 13 October 2017 17:09 >> Since it is a PCIe endpoint device, rather than the CPU, that is supposed >> to write to this location, the proper way to get the address to this this >> location is really to use the DMA API, rather than virt_to_phys. >> >> Using virt_to_phys might work on some systems, but by using the DMA API, >> we know that it will work on all systems. >> >> This is essentially the same thing as allocating a buffer in a driver, >> to which the endpoint will write to. To do this, we use the DMA API. >> >> Signed-off-by: Niklas Cassel >> --- >> drivers/pci/dwc/pcie-designware-host.c | 23 ++++++++++++++++------- >> drivers/pci/dwc/pcie-designware.h | 3 ++- >> 2 files changed, 18 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c >> index 81e2157a7cfb..f6d152ea2a03 100644 >> --- a/drivers/pci/dwc/pcie-designware-host.c >> +++ b/drivers/pci/dwc/pcie-designware-host.c >> @@ -83,16 +83,25 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp) >> >> void dw_pcie_msi_init(struct pcie_port *pp) >> { >> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp); >> + struct device *dev = pci->dev; >> + struct page *page; >> u64 msi_target; >> >> - pp->msi_data = __get_free_pages(GFP_KERNEL, 0); >> - msi_target = virt_to_phys((void *)pp->msi_data); >> + page = alloc_page(GFP_KERNEL | GFP_DMA32); >> + pp->msi_data = dma_map_page(dev, page, 0, PAGE_SIZE, DMA_FROM_DEVICE); > > Where are the matching frees? Hello David. I couldn't find any matching free_pages call to the __get_free_pages() allocation, so following the same design, I did not add a dma_unmap_page. The current design of most (all?) DWC PCI drivers is that they can only be built-in (i.e. not built as a module), and that they cannot be unbinded (suppress_bind_attrs = true in the platform_driver struct). I will be happy to add a dma_unmap_page (and free_pages), if the maintainer suggests somewhere where I should put it. Regards, Niklas > > David > >> + if (dma_mapping_error(dev, pp->msi_data)) { >> + dev_err(dev, "failed to map msi data\n"); >> + __free_page(page); >> + return; >> + } >> + msi_target = (u64)pp->msi_data; >> >> /* program the msi_data */ >> dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_LO, 4, >> - (u32)(msi_target & 0xffffffff)); >> + lower_32_bits(msi_target)); >> dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_HI, 4, >> - (u32)(msi_target >> 32 & 0xffffffff)); >> + upper_32_bits(msi_target)); >> } >> >> static void dw_pcie_msi_clear_irq(struct pcie_port *pp, int irq) >> @@ -187,10 +196,10 @@ static void dw_msi_setup_msg(struct pcie_port *pp, unsigned int irq, u32 pos) >> if (pp->ops->get_msi_addr) >> msi_target = pp->ops->get_msi_addr(pp); >> else >> - msi_target = virt_to_phys((void *)pp->msi_data); >> + msi_target = (u64)pp->msi_data; >> >> - msg.address_lo = (u32)(msi_target & 0xffffffff); >> - msg.address_hi = (u32)(msi_target >> 32 & 0xffffffff); >> + msg.address_lo = lower_32_bits(msi_target); >> + msg.address_hi = upper_32_bits(msi_target); >> >> if (pp->ops->get_msi_data) >> msg.data = pp->ops->get_msi_data(pp, pos); >> diff --git a/drivers/pci/dwc/pcie-designware.h b/drivers/pci/dwc/pcie-designware.h >> index e5d9d77b778e..547352a317f8 100644 >> --- a/drivers/pci/dwc/pcie-designware.h >> +++ b/drivers/pci/dwc/pcie-designware.h >> @@ -17,6 +17,7 @@ >> #include >> #include >> #include >> +#include >> >> #include >> #include >> @@ -168,7 +169,7 @@ struct pcie_port { >> const struct dw_pcie_host_ops *ops; >> int msi_irq; >> struct irq_domain *irq_domain; >> - unsigned long msi_data; >> + dma_addr_t msi_data; >> DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS); >> }; >> >> -- >> 2.11.0 >