All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marek Vasut <marek.vasut@gmail.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] fw_env: use vars from the board config
Date: Mon, 2 Jan 2012 17:38:55 +0100	[thread overview]
Message-ID: <201201021738.55448.marek.vasut@gmail.com> (raw)
In-Reply-To: <CACW_hTZRTGpvpLccqTxG+hZ2v=n4UD4vkbdqEVatf4uxNXg8zw@mail.gmail.com>

Cc u-boot ML please.

> 2012/1/2 Marek Vasut <marek.vasut@gmail.com>
> 
> > > it is quite odd that fw_printenv/fw_setenv does not
> > > use the settings from include/configs but instead
> > > redefines things.
> > > 
> > > This patch uses the variables from the config file
> > > The edit in fw_env.c is only needed to resolve a name clash
> > > 
> > > Signed-off-by: Frans Meulenbroeks <fransmeulenbroeks@gmail.com>
> > > 
> > > ---
> > > 
> > > Note: this is more intended to get some feedback.
> > > (also to see if I am on the right track)
> > > I did test the changes locally.
> > > 
> > > (and yes, I know there are some more things that could be cleaned up).
> > > ---
> > > 
> > >  tools/env/fw_env.c |   20 ++++++++++----------
> > >  tools/env/fw_env.h |   36 ++++++++++++++++--------------------
> > >  2 files changed, 26 insertions(+), 30 deletions(-)
> > > 
> > > diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
> > > index 996682e..6597fbf 100644
> > > --- a/tools/env/fw_env.c
> > > +++ b/tools/env/fw_env.c
> > > @@ -79,7 +79,7 @@ static int dev_current;
> > > 
> > >  #define ENVSECTORS(i) envdevices[(i)].env_sectors
> > >  #define DEVTYPE(i)    envdevices[(i)].mtd_type
> > > 
> > > -#define CONFIG_ENV_SIZE ENVSIZE(dev_current)
> > > +#define CFG_ENV_SIZE ENVSIZE(dev_current)
> > 
> > NAK, don't change it to CFG_... for no reason! Why did you change it ?
> > Just use
> > ENVSIZE(dev_current) instead.
> 
> I agree with that.
> As I wrote in the comment of the patch, this was mainly to get some
> feedback that I am on the right track.
> I've seen a number of places where the code of fw_env.c could be improved,
> but opted for minimal change for now, as for now I am mostly solicitating
> feedback on the changes in fw_env.h
> (and the actual reason for the change is that CONFIG_ENV_SIZE is defined
> here, but also in config.h, resulting in a naming config, a quick rename
> was the simplest way forward for now)

Let's see what the others think
> 
> And actually I feel that lines like:
> #define ENV1_SIZE         CONFIG_ENV_SIZE
> in fw_env.h are somewhat pointless and it would be better to eliminate
> ENV1_SIZE completely.
> 
> There are more cases like that. I'm happy to spent time on this, but only
> if it is felt to be useful and has any chance on being accepted.
> 
> Best regards, Frans

M

      parent reply	other threads:[~2012-01-02 16:38 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-02 13:59 [U-Boot] [PATCH] fw_env: use vars from the board config Frans Meulenbroeks
2012-01-02 15:11 ` Marek Vasut
     [not found]   ` <CACW_hTZRTGpvpLccqTxG+hZ2v=n4UD4vkbdqEVatf4uxNXg8zw@mail.gmail.com>
2012-01-02 16:38     ` Marek Vasut [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=201201021738.55448.marek.vasut@gmail.com \
    --to=marek.vasut@gmail.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.