From: "Alexander Shiyan" <shc_work@mail.ru>
To: "Michel Stam" <michel@reverze.net>
Cc: barebox@lists.infradead.org
Subject: Re: [PATCHv2 2/3] x86: Add support for IDE on the legacy I/O ports
Date: Fri, 04 Apr 2014 23:58:44 +0400 [thread overview]
Message-ID: <1396641524.291157734@f305.i.mail.ru> (raw)
In-Reply-To: <533EF7A2.90007@reverze.net>
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?
> >
> > ---
---
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
next prev parent reply other threads:[~2014-04-04 19:59 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-04-04 18:09 [PATCHv2 1/3] common: Allow for I/O mapped I/O michel
2014-04-04 18:09 ` [PATCHv2 2/3] x86: Add support for IDE on the legacy I/O ports michel
2014-04-04 19:12 ` Alexander Shiyan
2014-04-04 18:19 ` Michel Stam
2014-04-04 19:58 ` Alexander Shiyan [this message]
2014-04-04 19:29 ` Michel Stam
2014-04-04 18:09 ` [PATCHv2 3/3] x86: ns16550: Rework driver to allow for x86 I/O space michel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1396641524.291157734@f305.i.mail.ru \
--to=shc_work@mail.ru \
--cc=barebox@lists.infradead.org \
--cc=michel@reverze.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox