From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753907AbdJaWvc (ORCPT ); Tue, 31 Oct 2017 18:51:32 -0400 Received: from bastet.se.axis.com ([195.60.68.11]:52972 "EHLO bastet.se.axis.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750764AbdJaWva (ORCPT ); Tue, 31 Oct 2017 18:51:30 -0400 Subject: Re: [PATCH v2 09/17] PCI: dwc: dra7xx: Add ifdefs for host/ep specific code From: Niklas Cassel To: Kishon Vijay Abraham I , Bjorn Helgaas CC: , , References: <20171030124221.20690-1-niklas.cassel@axis.com> <20171030124221.20690-10-niklas.cassel@axis.com> <32dd0d05-4ed0-1c8b-6ea4-ea4d560c5570@ti.com> <669377ab-b859-24ed-5a2b-36bf826d426b@axis.com> Message-ID: <4bd547f0-8da7-cc90-6fa1-56a12b9fd72d@axis.com> Date: Tue, 31 Oct 2017 23:51:21 +0100 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: 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/31/2017 10:38 PM, Niklas Cassel wrote: >>>> >>>> static int __init dra7xx_pcie_probe(struct platform_device *pdev) >>>> { >>>> @@ -681,6 +697,7 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev) >>>> dra7xx->link_gen = 2; >>>> >>>> switch (mode) { >>>> +#ifdef CONFIG_PCI_DRA7XX_HOST >>>> case DW_PCIE_RC_TYPE: >>>> dra7xx_pcie_writel(dra7xx, PCIECTRL_TI_CONF_DEVICE_TYPE, >>>> DEVICE_TYPE_RC); >>>> @@ -688,6 +705,8 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev) >>>> if (ret < 0) >>>> goto err_gpio; >>>> break; >>>> +#endif >>>> +#ifdef CONFIG_PCI_DRA7XX_EP >>>> case DW_PCIE_EP_TYPE: >>>> dra7xx_pcie_writel(dra7xx, PCIECTRL_TI_CONF_DEVICE_TYPE, >>>> DEVICE_TYPE_EP); >>>> @@ -700,6 +719,7 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev) >>>> if (ret < 0) >>>> goto err_gpio; >>>> break; >>>> +#endif >> >> Actually, these ifdefs has to stay, otherwise we get build warnings, since we >> are calling functions that aren't defined (dra7xx_pcie_ep_unaligned_memaccess, >> dra7xx_add_pcie_ep, dra7xx_add_pcie_port). >> We could add dummy implementations for these inside an #else block following >> the ifdef blocks. However, I think that adding dummy implementations in the >> #else block is uglier and more verbose than keeping the ifdefs around the >> two cases. >> > > ..however, if you prefer dummy implementations inside the #else blocks, > I will of course do that. > And after thinking about it for a while a decided to go with dummy implementations in the #else blocks anyway :) This is how we usually do it in kernel code, we have one implementation if a certain Kconfig is set, and a dummy one if the Kconfig is not set. This way we avoid seeing ifdefs sprinkled all over the code, which makes it easier when trying to follow the flow of the code. Regards, Niklas From mboxrd@z Thu Jan 1 00:00:00 1970 From: Niklas Cassel Subject: Re: [PATCH v2 09/17] PCI: dwc: dra7xx: Add ifdefs for host/ep specific code Date: Tue, 31 Oct 2017 23:51:21 +0100 Message-ID: <4bd547f0-8da7-cc90-6fa1-56a12b9fd72d@axis.com> References: <20171030124221.20690-1-niklas.cassel@axis.com> <20171030124221.20690-10-niklas.cassel@axis.com> <32dd0d05-4ed0-1c8b-6ea4-ea4d560c5570@ti.com> <669377ab-b859-24ed-5a2b-36bf826d426b@axis.com> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Content-Language: en-US Sender: linux-pci-owner@vger.kernel.org To: Kishon Vijay Abraham I , Bjorn Helgaas Cc: linux-omap@vger.kernel.org, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org List-Id: linux-omap@vger.kernel.org On 10/31/2017 10:38 PM, Niklas Cassel wrote: >>>> >>>> static int __init dra7xx_pcie_probe(struct platform_device *pdev) >>>> { >>>> @@ -681,6 +697,7 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev) >>>> dra7xx->link_gen = 2; >>>> >>>> switch (mode) { >>>> +#ifdef CONFIG_PCI_DRA7XX_HOST >>>> case DW_PCIE_RC_TYPE: >>>> dra7xx_pcie_writel(dra7xx, PCIECTRL_TI_CONF_DEVICE_TYPE, >>>> DEVICE_TYPE_RC); >>>> @@ -688,6 +705,8 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev) >>>> if (ret < 0) >>>> goto err_gpio; >>>> break; >>>> +#endif >>>> +#ifdef CONFIG_PCI_DRA7XX_EP >>>> case DW_PCIE_EP_TYPE: >>>> dra7xx_pcie_writel(dra7xx, PCIECTRL_TI_CONF_DEVICE_TYPE, >>>> DEVICE_TYPE_EP); >>>> @@ -700,6 +719,7 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev) >>>> if (ret < 0) >>>> goto err_gpio; >>>> break; >>>> +#endif >> >> Actually, these ifdefs has to stay, otherwise we get build warnings, since we >> are calling functions that aren't defined (dra7xx_pcie_ep_unaligned_memaccess, >> dra7xx_add_pcie_ep, dra7xx_add_pcie_port). >> We could add dummy implementations for these inside an #else block following >> the ifdef blocks. However, I think that adding dummy implementations in the >> #else block is uglier and more verbose than keeping the ifdefs around the >> two cases. >> > > ..however, if you prefer dummy implementations inside the #else blocks, > I will of course do that. > And after thinking about it for a while a decided to go with dummy implementations in the #else blocks anyway :) This is how we usually do it in kernel code, we have one implementation if a certain Kconfig is set, and a dummy one if the Kconfig is not set. This way we avoid seeing ifdefs sprinkled all over the code, which makes it easier when trying to follow the flow of the code. Regards, Niklas