public inbox for u-boot@lists.denx.de
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox