From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753969AbcANK6P (ORCPT ); Thu, 14 Jan 2016 05:58:15 -0500 Received: from us01smtprelay-2.synopsys.com ([198.182.60.111]:47732 "EHLO smtprelay.synopsys.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753960AbcANK5r (ORCPT ); Thu, 14 Jan 2016 05:57:47 -0500 Subject: Re: [PATCH v5 1/2] PCI support added to ARC To: Vineet Gupta , Arnd Bergmann References: <24c9b782dfade515bbed20ce75d85200a00a2e66.1452611234.git.jpinto@synopsys.com> <4314331.TCtUIaZPTC@wuerfel> <56977B80.2040600@synopsys.com> CC: Joao Pinto , "helgaas@kernel.org" , "linux-pci@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-snps-arc@lists.infradead.org" , "CARLOS.PALMINHA@synopsys.com" , "Alexey.Brodkin@synopsys.com" , "robh+dt@kernel.org" , "pawel.moll@arm.com" , "mark.rutland@arm.com" , "ijc+devicetree@hellion.org.uk" , "galak@codeaurora.org" From: Joao Pinto Message-ID: <56977E5B.7050309@synopsys.com> Date: Thu, 14 Jan 2016 10:54:19 +0000 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.13.184.19] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On 1/14/2016 10:51 AM, Vineet Gupta wrote: > On Thursday 14 January 2016 04:12 PM, Joao Pinto wrote: >> Hi, >> >> On 1/14/2016 10:22 AM, Arnd Bergmann wrote: >>> On Thursday 14 January 2016 05:26:58 Vineet Gupta wrote: >>>>> +/* >>>>> + * We don't have to worry about legacy ISA devices, so nothing to do here >>>>> + */ >>>>> +resource_size_t pcibios_align_resource(void *data, const struct resource *res, >>>>> + resource_size_t size, resource_size_t align) >>>>> +{ >>>>> + return res->start; >>>>> +} >>>> Doesn't this have to be EXPORT_SYMBOL_xxx as well given that the call >>>> (setup-res.c) can build as module ? >>> I only see a caller in drivers/pci/setup-res.c, and that is never part of a >>> loadable module. >>> >>>>> + >>>>> +void pcibios_fixup_bus(struct pci_bus *bus) >>>>> +{ >>>>> +} >>>>> +EXPORT_SYMBOL(pcibios_fixup_bus); >>>> EXPORT_SYMBOL_GPL ? >>>> >>>> As a seperate enhancement, it would be nicer if these 2 functions are defined weak >>>> in common code. That would make basic PCI support almost arch independent ! >>> I agree, that would be ideal. An easy way to do this would be to add >>> them as __weak functions in drivers/pci/, similar to how we handle >>> a lot of the other pcibios_* functions. >>> >>> A somewhat nicer method would be to have callback pointers in struct >>> pci_host_bridge, and call those when they are non-NULL so we can >>> remove the global pcibios_* functions from the API. That would also >>> bring us a big step closer to having PCI support itself as a loadable >>> module, and it would better reflect that those functions are really >>> host bridge specific rather than architecture specific. When you use >>> the same host bridge on multiple architectures, you typically have >>> the same requirements for hacks in there, but each architectures may >>> need to support multiple host bridges with different requirements. >> Since we will be constantly improving the driver and the core itself, I suggest >> that this functions be made __weak and in an update we can turn it struct >> pointers just like Arnd suggested. Is this good for you? > > There is no point in making it weak, w/o a fallback version in generic code. For > this series, I suggest you just remove the straggler EXPORT_SYMBOL and respin. > > And then as a follow up to make them weak (and hence eliminate the scattered > definitions all over). And then add as callbacks as suggested by Arnd. > Ok, I'll removed the EXPORT_SYMBOL and submit a new patch version. Thanks for your comments! > Thx, > -Vineet > >> >>> Arnd >>> >> Thanks >> Joao >> >> >> > > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Subject: Re: [PATCH v5 1/2] PCI support added to ARC To: Vineet Gupta , Arnd Bergmann References: <24c9b782dfade515bbed20ce75d85200a00a2e66.1452611234.git.jpinto@synopsys.com> <4314331.TCtUIaZPTC@wuerfel> <56977B80.2040600@synopsys.com> CC: Joao Pinto , "helgaas@kernel.org" , "linux-pci@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-snps-arc@lists.infradead.org" , "CARLOS.PALMINHA@synopsys.com" , "Alexey.Brodkin@synopsys.com" , "robh+dt@kernel.org" , "pawel.moll@arm.com" , "mark.rutland@arm.com" , "ijc+devicetree@hellion.org.uk" , "galak@codeaurora.org" From: Joao Pinto Message-ID: <56977E5B.7050309@synopsys.com> Date: Thu, 14 Jan 2016 10:54:19 +0000 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="windows-1252" Sender: linux-kernel-owner@vger.kernel.org List-ID: Hi, On 1/14/2016 10:51 AM, Vineet Gupta wrote: > On Thursday 14 January 2016 04:12 PM, Joao Pinto wrote: >> Hi, >> >> On 1/14/2016 10:22 AM, Arnd Bergmann wrote: >>> On Thursday 14 January 2016 05:26:58 Vineet Gupta wrote: >>>>> +/* >>>>> + * We don't have to worry about legacy ISA devices, so nothing to do here >>>>> + */ >>>>> +resource_size_t pcibios_align_resource(void *data, const struct resource *res, >>>>> + resource_size_t size, resource_size_t align) >>>>> +{ >>>>> + return res->start; >>>>> +} >>>> Doesn't this have to be EXPORT_SYMBOL_xxx as well given that the call >>>> (setup-res.c) can build as module ? >>> I only see a caller in drivers/pci/setup-res.c, and that is never part of a >>> loadable module. >>> >>>>> + >>>>> +void pcibios_fixup_bus(struct pci_bus *bus) >>>>> +{ >>>>> +} >>>>> +EXPORT_SYMBOL(pcibios_fixup_bus); >>>> EXPORT_SYMBOL_GPL ? >>>> >>>> As a seperate enhancement, it would be nicer if these 2 functions are defined weak >>>> in common code. That would make basic PCI support almost arch independent ! >>> I agree, that would be ideal. An easy way to do this would be to add >>> them as __weak functions in drivers/pci/, similar to how we handle >>> a lot of the other pcibios_* functions. >>> >>> A somewhat nicer method would be to have callback pointers in struct >>> pci_host_bridge, and call those when they are non-NULL so we can >>> remove the global pcibios_* functions from the API. That would also >>> bring us a big step closer to having PCI support itself as a loadable >>> module, and it would better reflect that those functions are really >>> host bridge specific rather than architecture specific. When you use >>> the same host bridge on multiple architectures, you typically have >>> the same requirements for hacks in there, but each architectures may >>> need to support multiple host bridges with different requirements. >> Since we will be constantly improving the driver and the core itself, I suggest >> that this functions be made __weak and in an update we can turn it struct >> pointers just like Arnd suggested. Is this good for you? > > There is no point in making it weak, w/o a fallback version in generic code. For > this series, I suggest you just remove the straggler EXPORT_SYMBOL and respin. > > And then as a follow up to make them weak (and hence eliminate the scattered > definitions all over). And then add as callbacks as suggested by Arnd. > Ok, I'll removed the EXPORT_SYMBOL and submit a new patch version. Thanks for your comments! > Thx, > -Vineet > >> >>> Arnd >>> >> Thanks >> Joao >> >> >> > > From mboxrd@z Thu Jan 1 00:00:00 1970 From: Joao.Pinto@synopsys.com (Joao Pinto) Date: Thu, 14 Jan 2016 10:54:19 +0000 Subject: [PATCH v5 1/2] PCI support added to ARC In-Reply-To: References: <24c9b782dfade515bbed20ce75d85200a00a2e66.1452611234.git.jpinto@synopsys.com> <4314331.TCtUIaZPTC@wuerfel> <56977B80.2040600@synopsys.com> List-ID: Message-ID: <56977E5B.7050309@synopsys.com> To: linux-snps-arc@lists.infradead.org Hi, On 1/14/2016 10:51 AM, Vineet Gupta wrote: > On Thursday 14 January 2016 04:12 PM, Joao Pinto wrote: >> Hi, >> >> On 1/14/2016 10:22 AM, Arnd Bergmann wrote: >>> On Thursday 14 January 2016 05:26:58 Vineet Gupta wrote: >>>>> +/* >>>>> + * We don't have to worry about legacy ISA devices, so nothing to do here >>>>> + */ >>>>> +resource_size_t pcibios_align_resource(void *data, const struct resource *res, >>>>> + resource_size_t size, resource_size_t align) >>>>> +{ >>>>> + return res->start; >>>>> +} >>>> Doesn't this have to be EXPORT_SYMBOL_xxx as well given that the call >>>> (setup-res.c) can build as module ? >>> I only see a caller in drivers/pci/setup-res.c, and that is never part of a >>> loadable module. >>> >>>>> + >>>>> +void pcibios_fixup_bus(struct pci_bus *bus) >>>>> +{ >>>>> +} >>>>> +EXPORT_SYMBOL(pcibios_fixup_bus); >>>> EXPORT_SYMBOL_GPL ? >>>> >>>> As a seperate enhancement, it would be nicer if these 2 functions are defined weak >>>> in common code. That would make basic PCI support almost arch independent ! >>> I agree, that would be ideal. An easy way to do this would be to add >>> them as __weak functions in drivers/pci/, similar to how we handle >>> a lot of the other pcibios_* functions. >>> >>> A somewhat nicer method would be to have callback pointers in struct >>> pci_host_bridge, and call those when they are non-NULL so we can >>> remove the global pcibios_* functions from the API. That would also >>> bring us a big step closer to having PCI support itself as a loadable >>> module, and it would better reflect that those functions are really >>> host bridge specific rather than architecture specific. When you use >>> the same host bridge on multiple architectures, you typically have >>> the same requirements for hacks in there, but each architectures may >>> need to support multiple host bridges with different requirements. >> Since we will be constantly improving the driver and the core itself, I suggest >> that this functions be made __weak and in an update we can turn it struct >> pointers just like Arnd suggested. Is this good for you? > > There is no point in making it weak, w/o a fallback version in generic code. For > this series, I suggest you just remove the straggler EXPORT_SYMBOL and respin. > > And then as a follow up to make them weak (and hence eliminate the scattered > definitions all over). And then add as callbacks as suggested by Arnd. > Ok, I'll removed the EXPORT_SYMBOL and submit a new patch version. Thanks for your comments! > Thx, > -Vineet > >> >>> Arnd >>> >> Thanks >> Joao >> >> >> > >