All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wolfgang Denk <wd@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 5/9] apf9328: add default board configuration file
Date: Tue, 23 Aug 2011 13:26:19 +0200	[thread overview]
Message-ID: <20110823112619.93D24201520@gemini.denx.de> (raw)
In-Reply-To: <1FAD4112-2A57-415D-9CDF-8FC7A9EDB626@armadeus.org>

Dear Eric Jarrige,

can you please mind your line length?  It is strongly recommended that
text lines should not exceed 70 characters or so.  Thanks.

In message <1FAD4112-2A57-415D-9CDF-8FC7A9EDB626@armadeus.org> you wrote:

> >> +#define CONFIG_EXTRA_ENV_SETTINGS \
> >> +	"env_version="		CONFIG_ENV_VERSION		"\0"	\
> >> +	"fileaddr="		MK_STR(CONFIG_SYS_LOAD_ADDR)	"\0"	\
> >> +	"filesize="		MK_STR(CONFIG_SYS_MONITOR_LEN)	"\0"	\
> > 
> > filesize is dynamically computed, you should not add it
> 
> We need this variable already initialized at boot time to support the specific imx boot mode from serial port when the user lost the full content of the flash.

Stefano is right.  "filesize" and "fileaddr" are dynamic variables,
thet get created and updated on the fly. It makes no sense to
pre-define them to any specific value. [Actually I have some changes
in mind that avoid to save such variables with the environment at
all.]

> In such case (that is a stressing situation for the user) he can push U-Boot through the serial port and use directly the script "flash_uboot" to recover the original content of the flash. In such a use case this variable is not dynamically computed.

What do you mean by "push U-Boot through the serial port"?  Any of the
respective commands in U-Boot ("loads", "loadb", "loady") will
automatically update the respective variables.

> >> +#define CONFIG_NETMASK		255.255.255.0
> >> +#define CONFIG_IPADDR		192.168.000.10
> > 
> > Please drop fix ip address. They should not be part of mainline,
> 
> Here, I have a problem as our documentation on the wiki armadeus.org is based on this default IP addresses.

Then fix the documentation?

It is a somewhat bad idea to publish documentation to end users before
the code has passed even the inital code review...

> We really need a set of default IP addresses for private network to simplify as much as possible the life of the armadeus project developers. 

And what makes you think that 192.168.0.10 might be a free IP address
in my network?  Or assume that 255.255.255.0 is the right netmask in
this network?

These addresses may work for you, but they will not work for others,
and thus it makes no sense to use them as defaults.

If you need a reasonable default, then configure the board to use
DHCP.

> I never seen such a restriction in U-Boot doc and moreover there is a set CONFIG_XXIP documented in U-BOOT that confusing me,

There are situations where this cannot be avoided - like when U-Boot
is installed in a ROM, where the environment cannot be changed at all,
and where for some reason DHCP or similar is not possible.  But that
does not mean that this makes sense for the generic case.

> Is there another solution for a complete usable default configuration?

Use DHCP?

> > Why do we need such a stuff when your configuration is fixed at compile
> > time with CONFIG_SYS_SDRAM_MBYTE_SYZE 16 ?
> 
> 16MiB is the regular configuration but there are many configurations of boards with different size of memory.
> This set of parameters enables to support every boards at compilation by just changing the value of CONFIG_SYS_SDRAM_MBYTE_SYZE.
> So that the binary generated is fully optimized for each board.

That's a maintenance nightmage for everybody and not needed at all.

Please use get_ram_size() to have U-Boot auto-adjust to the actual RAM
size of your board and forget about all such static "optimizations".

> >> +#define CONFIG_SYS_INIT_SP_ADDR	(CONFIG_SYS_SDRAM_BASE	\
> >> +		+ CONFIG_SYS_SDRAM_1_SIZE - 0x0100000)
> > 
> > You do not use U_Boot macro here (GENERATED_GBL_DATA_SIZE), as it is
> > usual. Stack pointer is already in RAM before relocation. Is it correct ?
> 
> Correct. I do have to optimize the location of the initial Stack pointer  for my board so I put the init SP at any safe place in RAM.

I find your answers not really helpful and constructive.  You write
you "have to optimize the location", but you fail to explain why you
think so.  I consider it likely that you are wrong about the optimize
part.  And you are definitely wrong about the rest, as you obviously
did not understand Stefano's comment at all - he was referring to the
magic constant 0x0100000 in above macro.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
: 1.  What is the possibility of this being added in the future?
In the near future, the probability is close to zero. In the  distant
future, I'll be dead, and posterity can do whatever they like... :-)
                                                              - lwall

  parent reply	other threads:[~2011-08-23 11:26 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-08-10 20:33 [U-Boot] [PATCH 0/9] Series short description Eric Jarrige
2011-08-10 20:33 ` [U-Boot] [PATCH 1/9] mx1: export imx_gpio_mode() function Eric Jarrige
2011-08-10 20:33 ` [U-Boot] [PATCH 2/9] mx1: add i2c registers Eric Jarrige
2011-08-11  8:52   ` Stefano Babic
2011-08-11 23:49     ` Eric Jarrige
2011-10-06 22:04   ` Wolfgang Denk
2011-08-10 20:33 ` [U-Boot] [PATCH 3/9] apf9328: Add Armadeus Project board APF9328 Eric Jarrige
2011-08-11  8:50   ` Stefano Babic
2011-08-11 23:41     ` Eric Jarrige
2011-08-12  6:49       ` Stefano Babic
2011-08-17  7:31         ` Igor Grinberg
2011-08-17 21:58           ` Eric Jarrige
2011-08-18  6:20             ` Igor Grinberg
2011-08-18  8:51             ` Stefano Babic
2011-08-10 20:33 ` [U-Boot] [PATCH 4/9] apf9328: add apf9328 board in Makefile Eric Jarrige
2011-08-10 20:33 ` [U-Boot] [PATCH 5/9] apf9328: add default board configuration file Eric Jarrige
2011-08-11  9:21   ` Stefano Babic
2011-08-15 20:25     ` Eric Jarrige
2011-08-23  9:46       ` Stefano Babic
2011-08-24  4:50         ` Eric Jarrige
2011-08-23 11:26       ` Wolfgang Denk [this message]
2011-08-24  4:56         ` Eric Jarrige
2011-08-24  5:49           ` Wolfgang Denk
2011-08-24  6:34             ` Wolfgang Denk
2011-08-24 23:01               ` Eric Jarrige
2011-08-24 22:26             ` Eric Jarrige
2011-08-24 22:56               ` Wolfgang Denk
2011-08-24  6:22           ` Stefano Babic
2011-08-24 23:08             ` Eric Jarrige
2011-10-06 22:03   ` Wolfgang Denk
2011-08-10 20:33 ` [U-Boot] [PATCH 6/9] mx1: improve PLL freq computation Eric Jarrige
2011-08-11  9:22   ` Stefano Babic
2011-08-12  0:03     ` Eric Jarrige
2011-08-12  0:28       ` Eric Jarrige
2011-08-12  6:51       ` Stefano Babic
2011-08-10 20:33 ` [U-Boot] [PATCH 7/9] mx1: change a printf in speed.c to use debug instead Eric Jarrige
2011-08-10 20:33 ` [U-Boot] [PATCH 8/9] DM9000: change some printf " Eric Jarrige
2011-08-11  7:26   ` Simon Schwarz
2011-08-11  8:01     ` Detlev Zundel
2011-08-11 10:51       ` Eric Jarrige
2011-08-24 20:20         ` Wolfgang Denk
2011-08-24 23:04           ` Eric Jarrige
2011-08-25  3:19             ` Marek Vasut
2011-08-25  5:49             ` Wolfgang Denk
2011-08-11  9:27   ` Stefano Babic
2011-08-10 20:33 ` [U-Boot] [PATCH 9/9] arm920t: Fix jump to the relocated board_init_r Eric Jarrige

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=20110823112619.93D24201520@gemini.denx.de \
    --to=wd@denx.de \
    --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.