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 0/57] RFC: Move arch-specific global data into its own structure
Date: Tue, 04 Dec 2012 20:17:35 +0100	[thread overview]
Message-ID: <20121204191735.02BC9201071@gemini.denx.de> (raw)
In-Reply-To: <CAPnjgZ3QMB+0vcamWAkxF1vvBwzgijAXpDUtBbb7YegEz0pssw@mail.gmail.com>

Dear Simon,

In message <CAPnjgZ3QMB+0vcamWAkxF1vvBwzgijAXpDUtBbb7YegEz0pssw@mail.gmail.com> you wrote:
> 
> The discussion at the time was here:
> 
> http://patchwork.ozlabs.org/patch/146798/
> 
> My previous effort to create a generic board init basically fell over
> on this point. Do you agree with the analysis and proposal on that
> thread?

Mostly; but some questions have never been really answered, for
example the effect on the memory footprint (i.e. code size).

> > General comments / questions:
> >
> > - We always attempted to keep global data as small as possible.  What
> >   happens here appears to be a move in a totally wrong direction.
> >   Instead of simplyfiyng it (and moving stuff out of global data), we
> >   add more and more complexity to it.  That's wrong.  We should not
> >   do that.
> 
> It creates a new generic global data which is very simple. For many
> archs this is empty or very short so they will be happy.
> 
> The global data is no larger in this series, nor is it any smaller.

See Graemes other comment...

> I hope that my moving arch-specific things into their own file it will
> help people to simplify things, but if it doesn't then at least it
> doesn't pollute everything else.
> 
> I think the complexity you refer to is the introduction of an
> architecture-specific structure within global data, where all the
> arch-specific stuff lives.
> 
> This is the solution arrived at on that thread. If this doesn't suit,
> please can you suggest an alternative.

I already did above, and basicly this is what Graeme asks for, too:
instead of adding stuff to GD, we should reduce it to the really
needed bare minimum.  I'm not sure hoch much architecture specific
data would survive such a cleanup.

> > - The change makes the code less readable.  Reading "gd->arch."
> >   instead of plain "gd->" is no improvements, but rather vice versa.
> >   If we really go this way, this should be improved.
> 
> Yes it would be nice. Are you suggesting some sort of macro, or something else?

I don;t like the additional level of nesting, nomatter if I have to
write it outt or if it's hidden in some macro (actually I fear tyhe
macro version would be even worse to understand).

> > - What exactly is the impact of this code changes on the memory
> >   footprint?
> 
> We are just moving structure members around a bit, not actually
> changing the function of the code. The series is basically a nop from
> that point of view.

You add a level of indirecting to the code.  I doubt that goues
without code to load some registers (which in turn will add other code
to push and pop needed registers to/from the stack).

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
"To IBM, 'open' means there is a modicum  of  interoperability  among
some of their equipment."                            - Harv Masterson

      parent reply	other threads:[~2012-12-04 19:17 UTC|newest]

Thread overview: 78+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-16 21:19 [U-Boot] [PATCH 0/57] RFC: Move arch-specific global data into its own structure Simon Glass
2012-11-16 21:19 ` [U-Boot] [PATCH 01/57] Add architecture-specific global data Simon Glass
2012-11-16 21:19 ` [U-Boot] [PATCH 02/57] at91: Move at91 global data into arch_global_data Simon Glass
2012-11-16 21:19 ` [U-Boot] [PATCH 03/57] arm: Move timer_rate_hz " Simon Glass
2012-11-16 21:19 ` [U-Boot] [PATCH 04/57] arm: Move tbu to arch_global_data Simon Glass
2012-11-16 21:19 ` [U-Boot] [PATCH 05/57] arm: Move tbl " Simon Glass
2012-11-16 21:19 ` [U-Boot] [PATCH 06/57] arm: Move lastinc " Simon Glass
2012-11-16 21:19 ` [U-Boot] [PATCH 07/57] arm: Move timer_reset_value " Simon Glass
2012-11-16 21:19 ` [U-Boot] [PATCH 08/57] ixp: Move timestamp " Simon Glass
2012-11-16 22:22   ` Marek Vasut
2012-11-16 21:19 ` [U-Boot] [PATCH 09/57] nds32: Drop tlb_addr from global data Simon Glass
2012-11-16 21:19 ` [U-Boot] [PATCH 10/57] arm: Move tlb_addr to arch_global_data Simon Glass
2012-11-16 21:19 ` [U-Boot] [PATCH 11/57] x86: Move gdt_addr, new_gd_addr " Simon Glass
2012-11-16 21:19 ` [U-Boot] [PATCH 12/57] x86: Remove reset_status, relocoff from global_data Simon Glass
2012-11-16 21:19 ` [U-Boot] [PATCH 13/57] x86: Move new_gd_addr to arch_global_data Simon Glass
2012-11-18  1:07   ` Graeme Russ
2012-12-14  6:34     ` Simon Glass
2012-11-16 21:19 ` [U-Boot] [PATCH 14/57] ppc: Move brg_clk " Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 15/57] ppc: Remove extra pci_clk fields from global_data Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 16/57] ppc: Move clock fields to arch_global_data Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 17/57] ppc: Move mpc83xx " Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 18/57] ppc: Move lbc_clk and cpu " Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 19/57] ppc: m68k: Move i2c1_clk, i2c2_clk " Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 20/57] ppc: Move CONFIG_QE " Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 21/57] ppc: Move used_laws " Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 22/57] ppc: Move used_tlb_cams " Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 23/57] ppc: Move mpc5xxx clocks " Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 24/57] ppc: Move mpc512x " Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 25/57] ppc: Move mpc8220 " Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 26/57] ppc: Move reset_status " Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 27/57] ppc: Move arbiter fields " Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 28/57] ppc: Move dp_alloc_base, dp_alloc_top " Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 29/57] arm: Move uart_clk " Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 30/57] ppc: Move mirror_hack " Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 31/57] ppc: Remove console_addr from global data Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 32/57] ppc: Move fpga_state to arch_global_data Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 33/57] ppc: Move wdt_last " Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 34/57] ppc: Move kbd_status " Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 35/57] ppc: arm: Move sdhc_clk into arch_global_data Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 36/57] sparc: Drop kbd_status and reset_status from global_data Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 37/57] m68k: Move CONFIG_EXTRA_CLOCK to arch_global_data Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 38/57] mips: Move per_clk and dev_clk " Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 39/57] avr32: Move stack_end " Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 40/57] avr32: Move cpu_hz " Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 41/57] sandbox: Move ram_buf " Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 42/57] Add generic global_data Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 43/57] Only use fb_base if we have a display Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 44/57] arm: Use generic global_data Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 45/57] avr32: " Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 46/57] blackfin: " Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 47/57] m68k: " Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 48/57] microblaze: " Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 49/57] mips: " Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 50/57] nds32: " Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 51/57] nios2: " Simon Glass
2012-11-20  4:01   ` Thomas Chou
2012-11-16 21:20 ` [U-Boot] [PATCH 52/57] openrisc: " Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 53/57] powerpc: " Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 54/57] sandbox: " Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 55/57] sh: " Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 56/57] sparc: " Simon Glass
2012-11-16 21:20 ` [U-Boot] [PATCH 57/57] x86: " Simon Glass
2012-11-20  7:25 ` [U-Boot] [PATCH 0/57] RFC: Move arch-specific global data into its own structure Wolfgang Denk
2012-11-20 14:06   ` Simon Glass
2012-11-28 23:57     ` Simon Glass
2012-12-03 14:54     ` Tom Rini
2012-12-03 22:02       ` Graeme Russ
2012-12-03 22:19         ` Simon Glass
2012-12-03 23:39           ` Tom Rini
2012-12-03 23:45             ` Graeme Russ
2012-12-04 19:27               ` Wolfgang Denk
2012-12-05  0:02                 ` Simon Glass
2012-12-04 19:25           ` Wolfgang Denk
2012-12-05  1:14             ` Graeme Russ
2012-12-06  0:33               ` Simon Glass
2012-12-04 19:05         ` Wolfgang Denk
2012-12-04 19:20       ` Wolfgang Denk
2012-12-04 19:17     ` Wolfgang Denk [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=20121204191735.02BC9201071@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.