linux-kernel - Re: [PATCH v2 09/17] PCI: dwc: dra7xx: Add ifdefs for host/ep specific code

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 31 Oct 2017 23:51:21 +0100
From:   Niklas Cassel <niklas.cassel@...s.com>
To:     Kishon Vijay Abraham I <kishon@...com>,
        Bjorn Helgaas <bhelgaas@...gle.com>
CC:     <linux-omap@...r.kernel.org>, <linux-pci@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 09/17] PCI: dwc: dra7xx: Add ifdefs for host/ep
 specific code



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