public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Alessandro Rubini <rubini-list@gnudd.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 2/3] arm: omap: innovator: Prepare for mach-types.h changes
Date: Sun, 1 May 2011 22:28:17 +0200	[thread overview]
Message-ID: <20110501202817.GA17779@mail.gnudd.com> (raw)
In-Reply-To: <1304244632-8714-2-git-send-email-grinberg@compulab.co.il>

I'm sorry for sounding rude, it's not my intention.

I didn't follow closely the discussion about mach_types.h, but I think
we are heading in the wrong direction.

For example, this patch:

> -	if (machine_is_omap_h2())
> -		gd->bd->bi_arch_number = MACH_TYPE_OMAP_H2;
> -	else if (machine_is_omap_innovator())
> -		gd->bd->bi_arch_number = MACH_TYPE_OMAP_INNOVATOR;
> -	else
> -		gd->bd->bi_arch_number = MACH_TYPE_OMAP_GENERIC;
> +#if defined(CONFIG_MACH_OMAP_H2)
> +	gd->bd->bi_arch_number = MACH_TYPE_OMAP_H2;
> +#elif defined(CONFIG_MACH_OMAP_INNOVATOR)
> +	gd->bd->bi_arch_number = MACH_TYPE_OMAP_INNOVATOR;
> +#else
> +	gd->bd->bi_arch_number = MACH_TYPE_OMAP_GENERIC;
> +#endif

Since when turning if into ifdef has been a wise move for
maintainability?

The commis says:

> This board used machine_is_* macros for identifying the arch number.
> Use compile time defines instead.

But this already was compile-time: no code generated. But even if it
generated code, I prefer 3 run-time comparisons than 3 compile-time
ifdefs.

Note that mach_types.h, as designed by Russell King, already had
compile time selection, becuase if you selected one machine only (like
in u-boot), one of the "if" becomes compile-time-true and the other
ones become "0".

I see a lot of discussion about checkpatch compliance and cleanup-only
patches are being accepted; this goes in the opposite direction, for
no reason apparent to me.

thanks for your patience
/alessandro

  reply	other threads:[~2011-05-01 20:28 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-19 12:42 [U-Boot] Update and Cut down mach types Paulraj, Sandeep
2011-04-19 13:39 ` Matthias Weißer
2011-04-19 13:45   ` Paulraj, Sandeep
2011-04-20  8:44     ` Albert ARIBAUD
2011-04-19 14:21   ` Wolfgang Denk
2011-04-19 18:42     ` Matthias Weisser
2011-04-19 18:44     ` Michael Schwingen
2011-04-20  8:15       ` Detlev Zundel
2011-04-20  8:58 ` Igor Grinberg
2011-04-20 17:15   ` Michael Schwingen
2011-04-20 17:49     ` Albert ARIBAUD
2011-04-20 19:26       ` Michael Schwingen
2011-04-21 11:39         ` Albert ARIBAUD
2011-04-26 18:14           ` Michael Schwingen
2011-04-26 19:40             ` Wolfgang Denk
2011-04-26 20:38               ` Albert ARIBAUD
2011-04-26 21:32                 ` Wolfgang Denk
2011-04-26 21:38                   ` Reinhard Meyer
2011-04-27 10:19                     ` Michael Schwingen
2011-04-28  6:20                       ` Igor Grinberg
2011-04-29  8:58                         ` Detlev Zundel
2011-05-01 10:10                           ` [U-Boot] [PATCH 1/3] arm: omap: innovator: fix compilation error Igor Grinberg
2011-05-17 12:40                             ` Igor Grinberg
2011-05-21 21:40                               ` Paulraj, Sandeep
2011-05-01 10:10                           ` [U-Boot] [PATCH 2/3] arm: omap: innovator: Prepare for mach-types.h changes Igor Grinberg
2011-05-01 20:28                             ` Alessandro Rubini [this message]
2011-05-02  7:18                               ` Igor Grinberg
2011-05-03 10:08                                 ` [U-Boot] [PATCH v2 " Igor Grinberg
2011-05-03 12:29                                   ` Wolfgang Denk
2011-05-03 13:00                                     ` Igor Grinberg
2011-05-04  7:13                                       ` [U-Boot] [PATCH v3 " Igor Grinberg
2011-05-01 10:10                           ` [U-Boot] [PATCH 3/3] arm: at91: ether: " Igor Grinberg
2011-05-01 19:38                             ` Reinhard Meyer
2011-05-02  7:29                               ` Igor Grinberg
2011-05-02 10:09                                 ` Detlev Zundel
2011-05-02 12:49                                   ` [U-Boot] [PATCH v2 " Igor Grinberg
2011-05-16 13:31                                     ` Igor Grinberg
2011-04-27 11:44                     ` [U-Boot] Update and Cut down mach types Detlev Zundel

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=20110501202817.GA17779@mail.gnudd.com \
    --to=rubini-list@gnudd.com \
    --cc=u-boot@lists.denx.de \
    /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