All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.