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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxx>:
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@xxxxxxxxxxx:
From: Michel Stam <m.stam@xxxxxxxx>

---
   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: smime.p7s
Description: S/MIME Cryptographic Signature

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

[Index of Archives]     [Linux Embedded]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux