mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Sascha Hauer <s.hauer@pengutronix.de>
To: Christoph Fritz <chf.fritz@googlemail.com>
Cc: barebox@lists.infradead.org
Subject: Re: [PATCH 2/2] ARM OMAP: add support for loading Environment from UBI
Date: Tue, 25 Jun 2013 07:26:44 +0200	[thread overview]
Message-ID: <20130625052644.GG14308@pengutronix.de> (raw)
In-Reply-To: <1371808556.3949.15.camel@mars>

On Fri, Jun 21, 2013 at 11:55:56AM +0200, Christoph Fritz wrote:
> This patch adds support to load environment from a static
> UBI volume.
> 
>  	if (IS_ENABLED(CONFIG_ENV_HANDLING)) {
>  		int ret;
>  
> +		if (IS_ENABLED(CONFIG_ENVIRONMENT_IN_UBI)) {
> +#ifdef CONFIG_ENVIRONMENT_IN_UBI
> +			char s[PATH_MAX + 32];
> +			sprintf(s, "%s%s ", "ubiattach ",
> +				CONFIG_ENV_IN_UBI_PARTITION);
> +			ret = run_command(s, 0);
> +			if (ret) {
> +				sprintf(s, "%s%s ", "unable to ubiattach ",
> +					CONFIG_ENV_IN_UBI_PARTITION);
> +				pr_err(s);
> +			} else {
> +				default_environment_path =
> +					CONFIG_ENV_IN_UBI_VOLNAME;
> +			}
> +#endif

You shouldn't need the ifdef here aswell. Also this should be a helper
function called from board code rather than something in generic code.
You can also hardcode the volume name in board code, I don't think this
needs to be configurable.

Configurability of stuff like environment position always has a problem.
If it wasn't you who compiled a particular binary (or it even was you
and you can't remember), you don't know how the binary behaves. For
everything you hardcode the version printout in barebox will exactly
tell you the behaviour.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

      reply	other threads:[~2013-06-25  5:27 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-21  9:53 [PATCH 0/2] OMAP: Boot " Christoph Fritz
2013-06-21  9:55 ` [PATCH 1/2] ARM OMAP: MLO: add support for loading Barebox " Christoph Fritz
2013-06-25  4:22   ` Sascha Hauer
2013-06-21  9:55 ` [PATCH 2/2] ARM OMAP: add support for loading Environment " Christoph Fritz
2013-06-25  5:26   ` Sascha Hauer [this message]

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=20130625052644.GG14308@pengutronix.de \
    --to=s.hauer@pengutronix.de \
    --cc=barebox@lists.infradead.org \
    --cc=chf.fritz@googlemail.com \
    /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