From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753185AbcB2Jaj (ORCPT ); Mon, 29 Feb 2016 04:30:39 -0500 Received: from arroyo.ext.ti.com ([192.94.94.40]:36816 "EHLO arroyo.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752611AbcB2Jaf (ORCPT ); Mon, 29 Feb 2016 04:30:35 -0500 Subject: Re: [PATCH 0/5] Modularize PCI_DW related drivers. To: Arnd Bergmann References: <1454889644-27830-1-git-send-email-paul.gortmaker@windriver.com> <6350001.oOdGGcH81x@wuerfel> <56CEB7BC.7010803@ti.com> <2837386.VTD89bjPJz@wuerfel> CC: Paul Gortmaker , , Bjorn Helgaas , , Frank Rowand , Geert Uytterhoeven , Grant Likely , Ley Foon Tan , Murali Karicheri , Rob Herring , Russell King , Stanimir Varbanov , Thierry Reding , , , From: Kishon Vijay Abraham I Message-ID: <56D40F7F.5060204@ti.com> Date: Mon, 29 Feb 2016 14:59:35 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <2837386.VTD89bjPJz@wuerfel> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Arnd, On Thursday 25 February 2016 02:05 PM, Arnd Bergmann wrote: > On Thursday 25 February 2016 13:43:48 Kishon Vijay Abraham I wrote: >> Hi Arnd, >> >> On Wednesday 24 February 2016 02:34 PM, Arnd Bergmann wrote: >>> On Wednesday 24 February 2016 11:39:26 Kishon Vijay Abraham I wrote: >>>> Hi, >>>> >>>> On Monday 08 February 2016 05:30 AM, Paul Gortmaker wrote: >>>>> In a recent patch series that aimed to remove code related to module >>>>> unload for PCI support that was simply non modular, the discussion >>>>> led to people wanting to keep the code and push towards taking the >>>>> steps needed to support moving it towards tristate instead[1]. >>>>> >>>>> Here, we take step one, which is simply making the Kconfig change >>>>> and then dealing with any build fallout or modpost fallout. What >>>>> amounts to essentially a sanity build test. To be clear, these >>>>> have not been runtime validated; that will need to be done by those >>>>> with access to real hardware. However, the changes are not anything >>>>> that should disrupt any existing built-in validation, so real world >>>>> users should not be impacted by this change. >>>>> >>>>> We start with a smaller family of drivers; those that actively select >>>>> PCI_DW, as a nice self contained group to test the waters and see if >>>>> everyone is still good with this approach before investing more time >>>>> on a wider scale to other pci/host/ code blocks. >>>>> >>>>> As such the drivers here share a dependency on having the same group >>>>> of functions exported in order to successfully complete modpost. >>>>> >>>>> In addition, we have to stray outside drivers/pci to add exports >>>>> in two places; once for an ARM fault handler, and once for an OF >>>>> variable. >>>>> >>>>> The pci-keystone-dw.c instance was handled separately because it >>>>> consists of two source files that need their own group of driver >>>>> specific exports above and beyond the "shared" ones. >>>>> >>>>> Then we convert the Kconfig for all remaining at once; we could have >>>>> done it on a per driver basis for ease of revert if anyone really >>>>> objects, but since it would be a one line change, that seemed like >>>>> not a real concern. >>>>> >>>>> Build testing was done on the linux-next tree for arm allmodconfig. >>>> >>>> I took these patches and gave a test with DRA7xx board. As expected there was >>>> no issues when the driver was built-in. However when I tried to rmmod/modprobe >>>> I got this error [2]. >>> >>> Thanks for testing this! >>> >>>> [2] -> http://pastebin.ubuntu.com/15185894/ >>> >>> It looks like you are hitting the BUG_ON() in ioremap_pte_range() >>> that checks if a virtual address already has a page table entry, >>> which in turn is probably a result of dw_pcie_host_init() >>> calling pci_remap_iospace() again for the same memory area >>> it has called the last time, and no cleanup done inbetween. >>> >>> Could you try adding a pci_unmap_iospace() and calling that >>> in the device remove function? Let me know if you need help >>> implementing it. >> >> That didn't look straight forward to me :-( I'll try to see this next week. Any >> help from you will make it simpler for me. > > I tried writing the function now, and I think it's actually quite easy: > > void pci_unmap_iospace(const struct resource *res) > { > #if defined(PCI_IOBASE) && defined(CONFIG_MMU) > return iounmap(PCI_IOBASE + res->start); > #endif > } > > You just need to pass the same resource in here htat you pass into > pci_remap_iospace(). I still seem to get the abort in ioremap_page_range(). Here's the patch I used [3] and here's the kernel log [4]. [3] -> http://pastebin.ubuntu.com/15241614/ [4] -> http://pastebin.ubuntu.com/15241637/ Thanks Kishon