Re: [PATCHv2 2/3] x86: Add support for IDE on the legacy I/O ports

[Michel Stam <michel@reverze.net>]

[-- Attachment #1.1: Type: text/plain, Size: 2207 bytes --]

Point taken, I agree as far as the large number of ifdefs is concerned. 
This code could use some reworking, maybe split off into a separate file 
or even two.

I do not agree about letting the linker optimise, as that is 
compiler-dependent.

I may look into splitting this up another time.

On 04/04/2014 09:58 PM, Alexander Shiyan wrote:
> Fri, 04 Apr 2014 20:19:14 +0200 от Michel Stam <michel@reverze.net>:
>> I prefer to #ifdef out whatever code is not in use. That includes header
>> files, as some don't play nice.
>>
>> In this way, one can be certain that code that is not used will not be
>> compiled either, and as such will not cause needless errors either. It
>> also clearly indicates why this particular piece of code is there, in
>> this case the #include.
>> For platform_ide.h maybe not strictly necessary, but call it a habit.
>>
>> While working on this bit of code I got some errors if the BIOS IDE
>> support was not included, so I decided to fix it by ifdef-ing out
>> unnecessary code.
> Summing up, I want to say that a large number of #ifdef is not good.
> Unused code generally should be discarded by linker.
>
>> On 04/04/2014 09:12 PM, Alexander Shiyan wrote:
>>> Fri,  4 Apr 2014 20:09:46 +0200 от michel@reverze.net:
>>>> From: Michel Stam <m.stam@fugro.nl>
>>>>
>>>> ---
>>>>    arch/x86/boards/x86_generic/generic_pc.c |   73 +++++++++++++++++++++++
>>>>    drivers/ata/ide-sff.c                    |   94 ++++++++++++++++++++++++-----
>>>>    drivers/ata/intf_platform_ide.c          |   33 +++++++++-
>>>>    include/ata_drive.h                      |    1 +
>>>>    4 files changed, 180 insertions(+), 21 deletions(-)
>>>>
>>>> diff --git a/arch/x86/boards/x86_generic/generic_pc.c b/arch/x86/boards/x86_generic/generic_pc.c
>>>> index 74a7224..895b88d 100644
>>>> --- a/arch/x86/boards/x86_generic/generic_pc.c
>>>> +++ b/arch/x86/boards/x86_generic/generic_pc.c
>>>> @@ -27,6 +27,10 @@
>>>>    #include <ns16550.h>
>>>>    #include <linux/err.h>
>>>>    
>>>> +#ifdef CONFIG_DISK_INTF_PLATFORM_IDE
>>>> +#include <platform_ide.h>
>>>> +#endif
>>> Uhh, do you really need to #ifdef this?
>>>
>>> ---
> ---
>



[-- Attachment #1.2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4278 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox