All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] Selectable / delayed initialization
@ 2011-08-17  3:25 Simon Glass
  2011-08-17  6:42 ` Graeme Russ
  2011-08-18 15:33 ` Mike Frysinger
  0 siblings, 2 replies; 9+ messages in thread
From: Simon Glass @ 2011-08-17  3:25 UTC (permalink / raw)
  To: u-boot

Hi,

When U-Boot starts up it initializes all the peripherals which are
enabled  This might include USB, I2C, SD/MMC, LCD, Ethernet, UARTs,
etc.

To save time, reduce the amount of code executed and thereby improve
security, we are wanting to initialize only some peripherals at the
start. For example we might start up I2C and SC/MMC.

Later when it becomes clear that USB and Ethernet are needed we want
to init those also. I suppose the easiest way of describing this is
that we want to have some 'level 0' init, then perhaps move to level 1
and init peripherals at that level, then perhaps to level 2, etc.
Quite often we will only get to level 1 before jumping to the kernel.

At the moment the U-Boot init code is spread around a bit - there are
files called board.c within arch/xx/cpu/, board/xx/ and arch/xx/lib/.
So it is hard to make this change just within a board file. Some other
files have big lists of init calls like stdio.c and serial.c.

I am thinking of creating a way of specifying a list of peripherals
which should be inited in each level, then moving all init code into a
function which knows how to init each peripheral. Then we can call
init_level(2) for example, to start up peripherals in that level.
Perhaps level 0 would be reserved for pre-relocation init.

I am interested in any thoughts that people have about this and
whether it might be implemented in a generally useful way. I am
assuming that a driver registration / initcall approach would be a
bridge too far for U-Boot at the moment, so I am thinking of a
function with a big 'switch' statement.

The other related issue is that I notice that the arch/xxx/lib/board.c
files are different but have a lot of common code. Is there some scope
for starting a common/board.c file which slowly builds up common
functionality so that over time we end up with only the
architecture-specific code in the arch/lib/xxx/board.c? If so these
these two goals could perhaps be progressed together.

Regards,
Simon

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] Selectable / delayed initialization
  2011-08-17  3:25 [U-Boot] Selectable / delayed initialization Simon Glass
@ 2011-08-17  6:42 ` Graeme Russ
  2011-08-18 15:46   ` Simon Glass
  2011-08-18 15:33 ` Mike Frysinger
  1 sibling, 1 reply; 9+ messages in thread
From: Graeme Russ @ 2011-08-17  6:42 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On Wed, Aug 17, 2011 at 1:25 PM, Simon Glass <sjg@chromium.org> wrote:
> Hi,
>
> When U-Boot starts up it initializes all the peripherals which are
> enabled  This might include USB, I2C, SD/MMC, LCD, Ethernet, UARTs,
> etc.
>
> To save time, reduce the amount of code executed and thereby improve
> security, we are wanting to initialize only some peripherals at the
> start. For example we might start up I2C and SC/MMC.
>
> Later when it becomes clear that USB and Ethernet are needed we want
> to init those also. I suppose the easiest way of describing this is
> that we want to have some 'level 0' init, then perhaps move to level 1
> and init peripherals at that level, then perhaps to level 2, etc.
> Quite often we will only get to level 1 before jumping to the kernel.

Well technically, the device init should only be done either

a) U-Boot core code needs to access the device (to read the environment
   for example)
b) When a U-Boot command first uses the device
b) When doing final init ready for the OS kernel

> At the moment the U-Boot init code is spread around a bit - there are
> files called board.c within arch/xx/cpu/, board/xx/ and arch/xx/lib/.
> So it is hard to make this change just within a board file. Some other
> files have big lists of init calls like stdio.c and serial.c.

Yes, it is a mess

> I am thinking of creating a way of specifying a list of peripherals
> which should be inited in each level, then moving all init code into a
> function which knows how to init each peripheral. Then we can call
> init_level(2) for example, to start up peripherals in that level.
> Perhaps level 0 would be reserved for pre-relocation init.
>
> I am interested in any thoughts that people have about this and
> whether it might be implemented in a generally useful way. I am
> assuming that a driver registration / initcall approach would be a
> bridge too far for U-Boot at the moment, so I am thinking of a
> function with a big 'switch' statement.

I would prefer a device level 'start' and 'stop' hooks so when a command
or the U-Boot core needs to use a particular device it calls
device->start(), uses the device then calls device->stop(). Devices that
are in use permanently (timers and console ports are prime examples),
device->stop() is never called (or maybe called proir to entering the
OS kernel). This would be a massive architectural change for U-Boot and
leads into a 'driver framework'

> The other related issue is that I notice that the arch/xxx/lib/board.c
> files are different but have a lot of common code. Is there some scope
> for starting a common/board.c file which slowly builds up common
> functionality so that over time we end up with only the
> architecture-specific code in the arch/lib/xxx/board.c? If so these
> these two goals could perhaps be progressed together.

Now that relocation has settled down and looks to be working in a much
more platform independent manner, board_init_f() and board_init_r() should
be able to be moved into /lib/ with any arch or board specific
functionality carved out into init functions. Have a look at x86 and
you'll see that board_init_f() and board_init_r() work in a very simlar
manner to each other. I don't think this is going to be particularly
difficult to implement. I am, however, still scratching my head trying to
figure out how to create a simple mechanism for dynamic init sequence
building (even if it is done at compile time). By this I mean a standard
arch level init sequence and a way of allowing boards to insert init
functions at arbitrary points in the init sequence without a mass of
#ifdef's in /lib/board.c and without have each and every board having to
define the whole init sequence in /board/init.c. At the moment it's done
by reserving some specific points in the init sequence where U-Boot calls
into the board-specific init functions. If the board has no need for such
a call, it still needs to implement a do nothing stub function. I'm sure
someone can come up with some clever pre-processor macro-foo to do
something like this:

In board/foo.c:

int init_board_gpio()
{
	/* Do Stuff */
}
BOARD_INIT(init_board_leds, 214);

int init_board_leds()
{
	/* Do Stuff */
}
/* Init board LEDs after the boards GPIO has been initialised */
BOARD_INIT(init_board_leds, 215);

In arch/xxx/sdram.c

int init_sdram()
{
	/* Do Stuff */
}
/* SDRAM gets intialised really early (but not before timers) */
BOARD_INIT(init_sdram, 10);

In arch/xxx/<soc>/timer.c

int init_timers()
{
	/* Do Stuff */
}
/* Timers are needed early */
BOARD_INITinit_timers, 9);


You could even #define some of the steps with nice gaps between:

#define INIT_STEP_CPU		1	
#define INIT_STEP_TIMERS	INIT_STEP_CPU + 10
#define INIT_STEP_SDARM		INIT_STEP_TIMERS + 10

But maybe it's all wishful thinking

Regards,

Graeme

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] Selectable / delayed initialization
  2011-08-17  3:25 [U-Boot] Selectable / delayed initialization Simon Glass
  2011-08-17  6:42 ` Graeme Russ
@ 2011-08-18 15:33 ` Mike Frysinger
  2011-08-18 15:59   ` Simon Glass
  1 sibling, 1 reply; 9+ messages in thread
From: Mike Frysinger @ 2011-08-18 15:33 UTC (permalink / raw)
  To: u-boot

On Tuesday, August 16, 2011 23:25:35 Simon Glass wrote:
> When U-Boot starts up it initializes all the peripherals which are
> enabled  This might include USB, I2C, SD/MMC, LCD, Ethernet, UARTs,
> etc.

i'm not sure about this statement.  many frameworks decouple the "register" 
step from the "initialize the hardware" step.  i believe USB, SD/MMC, and 
Ethernet (ignoring the MAC address issue) do already.

arches calling i2c_init() in their arch/*/board.c are broken imo.  only a few 
do it: arm, ppc, m68k.  the common "i2c" command already takes care of 
exposing "i2c reset" for people to init this as necessary, and other arches 
havent had any troubles that i can see (certainly i know people have used i2c 
on Blackfin systems from u-boot).

a UART gets initialized all the time as it is the serial console ... not much 
to do here i dont think, and the overhead here tends to be fairly low as you 
program a few regs without polling for status updates.

LCDs should only get initialized if it's the splash screen, otherwise i cant 
think of any reason that hardware would ever get touched.  so if you dont want 
the overhead of programming that hardware, then dont enable splash screen 
support ?  i cant see a situation where you'd enable splash screen support but 
not actually show the splash screen ...

> To save time, reduce the amount of code executed and thereby improve
> security, we are wanting to initialize only some peripherals at the
> start. For example we might start up I2C and SC/MMC.

this is generally the policy now.  init code should only be *registering* the 
availability of devices.  it shouldnt be initializing the actual hardware 
until requested.

i know NAND flashes misbehave, and i posted a few patches to fix that, but 
there was one or two edge cases that needed addressing before it could be 
merged.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110818/386a7c04/attachment.pgp 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] Selectable / delayed initialization
  2011-08-17  6:42 ` Graeme Russ
@ 2011-08-18 15:46   ` Simon Glass
  0 siblings, 0 replies; 9+ messages in thread
From: Simon Glass @ 2011-08-18 15:46 UTC (permalink / raw)
  To: u-boot

Hi Graeme,

On Tue, Aug 16, 2011 at 11:42 PM, Graeme Russ <graeme.russ@gmail.com> wrote:
> Hi Simon,
>
> On Wed, Aug 17, 2011 at 1:25 PM, Simon Glass <sjg@chromium.org> wrote:
>> Hi,
>>
>> When U-Boot starts up it initializes all the peripherals which are
>> enabled ?This might include USB, I2C, SD/MMC, LCD, Ethernet, UARTs,
>> etc.
>>
>> To save time, reduce the amount of code executed and thereby improve
>> security, we are wanting to initialize only some peripherals at the
>> start. For example we might start up I2C and SC/MMC.
>>
>> Later when it becomes clear that USB and Ethernet are needed we want
>> to init those also. I suppose the easiest way of describing this is
>> that we want to have some 'level 0' init, then perhaps move to level 1
>> and init peripherals at that level, then perhaps to level 2, etc.
>> Quite often we will only get to level 1 before jumping to the kernel.
>
> Well technically, the device init should only be done either
>
> a) U-Boot core code needs to access the device (to read the environment
> ? for example)
> b) When a U-Boot command first uses the device
> b) When doing final init ready for the OS kernel
>
>> At the moment the U-Boot init code is spread around a bit - there are
>> files called board.c within arch/xx/cpu/, board/xx/ and arch/xx/lib/.
>> So it is hard to make this change just within a board file. Some other
>> files have big lists of init calls like stdio.c and serial.c.
>
> Yes, it is a mess
>
>> I am thinking of creating a way of specifying a list of peripherals
>> which should be inited in each level, then moving all init code into a
>> function which knows how to init each peripheral. Then we can call
>> init_level(2) for example, to start up peripherals in that level.
>> Perhaps level 0 would be reserved for pre-relocation init.
>>
>> I am interested in any thoughts that people have about this and
>> whether it might be implemented in a generally useful way. I am
>> assuming that a driver registration / initcall approach would be a
>> bridge too far for U-Boot at the moment, so I am thinking of a
>> function with a big 'switch' statement.

Well given your recent patch it seems this isn't a bridge too far :-)
I would very much like init to be done via a registration macro as
your patch does (and U-Boot commands already do). If we are going down
that path I would even argue for a way to configure it at start-up
(add/remove things).

>
> I would prefer a device level 'start' and 'stop' hooks so when a command
> or the U-Boot core needs to use a particular device it calls
> device->start(), uses the device then calls device->stop(). Devices that
> are in use permanently (timers and console ports are prime examples),
> device->stop() is never called (or maybe called proir to entering the
> OS kernel). This would be a massive architectural change for U-Boot and
> leads into a 'driver framework'

That's great as far as it goes. But many devices have one-time
initialization also. So perhaps

device->init()
device->start()
device->stop()
device->start()
device->stop()
device->uninit()

although some might argue for open/close instead of start/stop.

As an example, USB typically requires some clock/pinmux setup after
which it is ready for use. This happens at the beginning, often
regardless of whether 'usb start' is ever used. While this might be
against the grain, forcing all the init into the start() function then
means that this code needs to check if the init was done in a previous
start() call, and skip it if so. That seems wrong to me.

>
>> The other related issue is that I notice that the arch/xxx/lib/board.c
>> files are different but have a lot of common code. Is there some scope
>> for starting a common/board.c file which slowly builds up common
>> functionality so that over time we end up with only the
>> architecture-specific code in the arch/lib/xxx/board.c? If so these
>> these two goals could perhaps be progressed together.
>
> Now that relocation has settled down and looks to be working in a much
> more platform independent manner, board_init_f() and board_init_r() should
> be able to be moved into /lib/ with any arch or board specific
> functionality carved out into init functions. Have a look at x86 and
> you'll see that board_init_f() and board_init_r() work in a very simlar
> manner to each other. I don't think this is going to be particularly
> difficult to implement. I am, however, still scratching my head trying to
> figure out how to create a simple mechanism for dynamic init sequence
> building (even if it is done at compile time). By this I mean a standard
> arch level init sequence and a way of allowing boards to insert init
> functions at arbitrary points in the init sequence without a mass of
> #ifdef's in /lib/board.c and without have each and every board having to
> define the whole init sequence in /board/init.c. At the moment it's done
> by reserving some specific points in the init sequence where U-Boot calls
> into the board-specific init functions. If the board has no need for such
> a call, it still needs to implement a do nothing stub function. I'm sure
> someone can come up with some clever pre-processor macro-foo to do
> something like this:
>
> In board/foo.c:
>
> int init_board_gpio()
> {
> ? ? ? ?/* Do Stuff */
> }
> BOARD_INIT(init_board_leds, 214);
>
> int init_board_leds()
> {
> ? ? ? ?/* Do Stuff */
> }
> /* Init board LEDs after the boards GPIO has been initialised */
> BOARD_INIT(init_board_leds, 215);
>
> In arch/xxx/sdram.c
>
> int init_sdram()
> {
> ? ? ? ?/* Do Stuff */
> }
> /* SDRAM gets intialised really early (but not before timers) */
> BOARD_INIT(init_sdram, 10);
>
> In arch/xxx/<soc>/timer.c
>
> int init_timers()
> {
> ? ? ? ?/* Do Stuff */
> }
> /* Timers are needed early */
> BOARD_INITinit_timers, 9);
>
>
> You could even #define some of the steps with nice gaps between:
>
> #define INIT_STEP_CPU ? ? ? ? ? 1
> #define INIT_STEP_TIMERS ? ? ? ?INIT_STEP_CPU + 10
> #define INIT_STEP_SDARM ? ? ? ? INIT_STEP_TIMERS + 10
>
> But maybe it's all wishful thinking

Well let's hope not. Going back to my original request, I can see how
your patch can be fairly easily modified to do what I require - simply
by adding a tag to each init call indicating what it refers to (usb,
mmc, uart, etc.) and then providing a list of tags we want to init
for.

Regards
Simon

>
> Regards,
>
> Graeme
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] Selectable / delayed initialization
  2011-08-18 15:33 ` Mike Frysinger
@ 2011-08-18 15:59   ` Simon Glass
  2011-08-18 17:54     ` Mike Frysinger
  0 siblings, 1 reply; 9+ messages in thread
From: Simon Glass @ 2011-08-18 15:59 UTC (permalink / raw)
  To: u-boot

Hi Mike,

On Thu, Aug 18, 2011 at 8:33 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Tuesday, August 16, 2011 23:25:35 Simon Glass wrote:
>> When U-Boot starts up it initializes all the peripherals which are
>> enabled ?This might include USB, I2C, SD/MMC, LCD, Ethernet, UARTs,
>> etc.
>
> i'm not sure about this statement. ?many frameworks decouple the "register"
> step from the "initialize the hardware" step. ?i believe USB, SD/MMC, and
> Ethernet (ignoring the MAC address issue) do already.

Yes that is true in many cases.

As an example, I feel it is best that most/all the code to do with USB
sits in the USB driver. Let's say we have usb_lowlevel_init() which
sets up the SOC and then usb_start() which starts up a particular
port. Then I don't want to call either of these until I decide that
USB is required. The way board files tend to work today is to just
have a list of calls to init routines, then rely on #ifdef to disable
what is not required.

My desire is to make the decision on what to init at run-time rather
than compile-time. The boot moves through specific stages where we
need to bring in progressively more smarts to complete sucessfully.
Initially we can just use eMMC, then we find that doesn't work so must
use USB or later maybe network. But we don't want to execute the init
code for all of that stuff up front - and in fact if you saw my
postload email, the code may not even be loaded at that point.

As you suggest there is the option of doing the bare minimum init and
then later when we do usb_start() we notice that we haven't called
usb_lowlevel_init() yet. If we have a proper driver framework then we
can keep track of which drivers have been activated, and activate USB
before it is used.

>
> arches calling i2c_init() in their arch/*/board.c are broken imo. ?only a few
> do it: arm, ppc, m68k. ?the common "i2c" command already takes care of
> exposing "i2c reset" for people to init this as necessary, and other arches
> havent had any troubles that i can see (certainly i know people have used i2c
> on Blackfin systems from u-boot).

I feel the idea of calling i2c_init() multiple times with a speed
parameter is not great. Perhaps the function is just misnamed.

>
> a UART gets initialized all the time as it is the serial console ... not much
> to do here i dont think, and the overhead here tends to be fairly low as you
> program a few regs without polling for status updates.

Yes.

>
> LCDs should only get initialized if it's the splash screen, otherwise i cant
> think of any reason that hardware would ever get touched. ?so if you dont want
> the overhead of programming that hardware, then dont enable splash screen
> support ? ?i cant see a situation where you'd enable splash screen support but
> not actually show the splash screen ...

Well often U-Boot is asked to set up the LCD so that it will work for
the kernel. There may be a specific power-on init sequence to follow
even if nothing is actually written to the screen. For example in
Chrome OS we don't have splash screen support but still init the LCD.

>
>> To save time, reduce the amount of code executed and thereby improve
>> security, we are wanting to initialize only some peripherals at the
>> start. For example we might start up I2C and SC/MMC.
>
> this is generally the policy now. ?init code should only be *registering* the
> availability of devices. ?it shouldnt be initializing the actual hardware
> until requested.

OK then I vote for a framework which understands this and calls driver
init once (in a lazy fashion when needed if you like).

>
> i know NAND flashes misbehave, and i posted a few patches to fix that, but
> there was one or two edge cases that needed addressing before it could be
> merged.
> -mike
>

Regards,
Simon

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] Selectable / delayed initialization
  2011-08-18 15:59   ` Simon Glass
@ 2011-08-18 17:54     ` Mike Frysinger
  2011-08-18 19:01       ` Simon Glass
  0 siblings, 1 reply; 9+ messages in thread
From: Mike Frysinger @ 2011-08-18 17:54 UTC (permalink / raw)
  To: u-boot

On Thursday, August 18, 2011 11:59:35 Simon Glass wrote:
> On Thu, Aug 18, 2011 at 8:33 AM, Mike Frysinger wrote:
> > On Tuesday, August 16, 2011 23:25:35 Simon Glass wrote:
> >> When U-Boot starts up it initializes all the peripherals which are
> >> enabled  This might include USB, I2C, SD/MMC, LCD, Ethernet, UARTs,
> >> etc.
> > 
> > i'm not sure about this statement.  many frameworks decouple the
> > "register" step from the "initialize the hardware" step.  i believe USB,
> > SD/MMC, and Ethernet (ignoring the MAC address issue) do already.
> 
> Yes that is true in many cases.
> 
> As an example, I feel it is best that most/all the code to do with USB
> sits in the USB driver. Let's say we have usb_lowlevel_init() which
> sets up the SOC and then usb_start() which starts up a particular
> port. Then I don't want to call either of these until I decide that
> USB is required. The way board files tend to work today is to just
> have a list of calls to init routines, then rely on #ifdef to disable
> what is not required.
> 
> My desire is to make the decision on what to init at run-time rather
> than compile-time. The boot moves through specific stages where we
> need to bring in progressively more smarts to complete sucessfully.
> Initially we can just use eMMC, then we find that doesn't work so must
> use USB or later maybe network. But we don't want to execute the init
> code for all of that stuff up front - and in fact if you saw my
> postload email, the code may not even be loaded at that point.

i dont follow.  before u-boot gets to command line processing (either the 
"bootcommand" env var or user input), little to no hardware should be 
initialized, only registered as available.  after that, what hardware actually 
gets poked is completely user decided at run-time.

dont want to use usb hardware ?  then dont call "usb start".  dont want to use 
network hardware ?  then dont do network commands (i.e. tftp).

what am i missing ?

> > arches calling i2c_init() in their arch/*/board.c are broken imo.  only a
> > few do it: arm, ppc, m68k.  the common "i2c" command already takes care
> > of exposing "i2c reset" for people to init this as necessary, and other
> > arches havent had any troubles that i can see (certainly i know people
> > have used i2c on Blackfin systems from u-boot).
> 
> I feel the idea of calling i2c_init() multiple times with a speed
> parameter is not great. Perhaps the function is just misnamed.

perhaps it is more historical, but that is the current API, and it is already 
called multiple times, so it's required to work.  i dont think this has been a 
big deal so far for driver writers, but that doesnt mean it has to stay this 
way.  but wrt your requirements, i dont think we need to dive down that rabbit 
hole ... just kill off the explicit call to i2c_init() in the arm board.c.

> > LCDs should only get initialized if it's the splash screen, otherwise i
> > cant think of any reason that hardware would ever get touched.  so if
> > you dont want the overhead of programming that hardware, then dont
> > enable splash screen support ?  i cant see a situation where you'd
> > enable splash screen support but not actually show the splash screen ...
> 
> Well often U-Boot is asked to set up the LCD so that it will work for
> the kernel. There may be a specific power-on init sequence to follow
> even if nothing is actually written to the screen. For example in
> Chrome OS we don't have splash screen support but still init the LCD.

ruh-roh.  i think this is generally considered the wrong way to do things.  
the kernel drivers should be able to pick up the hardware and have it work 
regardless of what the (possibly brain dead) firmware that came before it did.  
relying on the hardware to be in a certain state.  it's even better done in 
the kernel as there you can do the init asynchronously whereas in u-boot, 
everything is single threaded.

> >> To save time, reduce the amount of code executed and thereby improve
> >> security, we are wanting to initialize only some peripherals at the
> >> start. For example we might start up I2C and SC/MMC.
> > 
> > this is generally the policy now.  init code should only be *registering*
> > the availability of devices.  it shouldnt be initializing the actual
> > hardware until requested.
> 
> OK then I vote for a framework which understands this and calls driver
> init once (in a lazy fashion when needed if you like).

each framework should already do that.  if there's a specific one you think is 
missing, then you should highlight it.

if you're talking about overhauling/unifying the driver model rather than each 
one being open coded, that's a different thing.  but i dont think your current 
requirements (as i understand them) necessitates such effort.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110818/410220c1/attachment.pgp 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] Selectable / delayed initialization
  2011-08-18 17:54     ` Mike Frysinger
@ 2011-08-18 19:01       ` Simon Glass
  2011-08-18 19:53         ` Mike Frysinger
  0 siblings, 1 reply; 9+ messages in thread
From: Simon Glass @ 2011-08-18 19:01 UTC (permalink / raw)
  To: u-boot

Hi Mike,

On Thu, Aug 18, 2011 at 11:54 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Thursday, August 18, 2011 11:59:35 Simon Glass wrote:
[snip snip]

> i dont follow. ?before u-boot gets to command line processing (either the
> "bootcommand" env var or user input), little to no hardware should be
> initialized, only registered as available. ?after that, what hardware actually
> gets poked is completely user decided at run-time.
>
> dont want to use usb hardware ? ?then dont call "usb start". ?dont want to use
> network hardware ? ?then dont do network commands (i.e. tftp).
>
> what am i missing ?

OK that's not how it is at the moment in some cases, but fair enough.
We have been pushing Tegra in that directly as it happens, but
certainly there is still a fair amount of mandatory init which should
be moved into / called from drivers.

>> > arches calling i2c_init() in their arch/*/board.c are broken imo. ?only a
>> > few do it: arm, ppc, m68k. ?the common "i2c" command already takes care
>> > of exposing "i2c reset" for people to init this as necessary, and other
>> > arches havent had any troubles that i can see (certainly i know people
>> > have used i2c on Blackfin systems from u-boot).
>>
>> I feel the idea of calling i2c_init() multiple times with a speed
>> parameter is not great. Perhaps the function is just misnamed.
>
> perhaps it is more historical, but that is the current API, and it is already
> called multiple times, so it's required to work. ?i dont think this has been a
> big deal so far for driver writers, but that doesnt mean it has to stay this
> way. ?but wrt your requirements, i dont think we need to dive down that rabbit
> hole ... just kill off the explicit call to i2c_init() in the arm board.c.

Er ok, but I found one in stdio.c!

>> > LCDs should only get initialized if it's the splash screen, otherwise i
>> > cant think of any reason that hardware would ever get touched. ?so if
>> > you dont want the overhead of programming that hardware, then dont
>> > enable splash screen support ? ?i cant see a situation where you'd
>> > enable splash screen support but not actually show the splash screen ...
>>
>> Well often U-Boot is asked to set up the LCD so that it will work for
>> the kernel. There may be a specific power-on init sequence to follow
>> even if nothing is actually written to the screen. For example in
>> Chrome OS we don't have splash screen support but still init the LCD.
>
> ruh-roh. ?i think this is generally considered the wrong way to do things.
> the kernel drivers should be able to pick up the hardware and have it work
> regardless of what the (possibly brain dead) firmware that came before it did.
> relying on the hardware to be in a certain state. ?it's even better done in
> the kernel as there you can do the init asynchronously whereas in u-boot,
> everything is single threaded.

We can't have a sensible console in U-Boot without keyboard and LCD -
for me the splash screen is a separate issue.

Yes the single-threaded problem does make it tricky. Am thinking about
that problem now.

>
>> >> To save time, reduce the amount of code executed and thereby improve
>> >> security, we are wanting to initialize only some peripherals at the
>> >> start. For example we might start up I2C and SC/MMC.
>> >
>> > this is generally the policy now. ?init code should only be *registering*
>> > the availability of devices. ?it shouldnt be initializing the actual
>> > hardware until requested.
>>
>> OK then I vote for a framework which understands this and calls driver
>> init once (in a lazy fashion when needed if you like).
>
> each framework should already do that. ?if there's a specific one you think is
> missing, then you should highlight it.
>
> if you're talking about overhauling/unifying the driver model rather than each
> one being open coded, that's a different thing. ?but i dont think your current
> requirements (as i understand them) necessitates such effort.

I suppose the ad-hoc nature of drivers means that people miss the
'generally accepted way' that things are supposed to be done.

Regards,
Simon

> -mike
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] Selectable / delayed initialization
  2011-08-18 19:01       ` Simon Glass
@ 2011-08-18 19:53         ` Mike Frysinger
  2011-08-18 20:12           ` Simon Glass
  0 siblings, 1 reply; 9+ messages in thread
From: Mike Frysinger @ 2011-08-18 19:53 UTC (permalink / raw)
  To: u-boot

On Thursday, August 18, 2011 15:01:28 Simon Glass wrote:
> On Thu, Aug 18, 2011 at 11:54 AM, Mike Frysinger wrote:
> > On Thursday, August 18, 2011 11:59:35 Simon Glass wrote:
> > i dont follow.  before u-boot gets to command line processing (either the
> > "bootcommand" env var or user input), little to no hardware should be
> > initialized, only registered as available.  after that, what hardware
> > actually gets poked is completely user decided at run-time.
> > 
> > dont want to use usb hardware ?  then dont call "usb start".  dont want
> > to use network hardware ?  then dont do network commands (i.e. tftp).
> > 
> > what am i missing ?
> 
> OK that's not how it is at the moment in some cases, but fair enough.
> We have been pushing Tegra in that directly as it happens, but
> certainly there is still a fair amount of mandatory init which should
> be moved into / called from drivers.

if there's specific frameworks/arches that always init hardware, then in 
general that's a bug and we should treat it as such (the u-boot policy doc 
explicitly bans this).  each framework (since they're all open coded in their 
own way) will need to be reviewed.  but hopefully there shouldn't be many.

> >> > arches calling i2c_init() in their arch/*/board.c are broken imo.
> >> >  only a few do it: arm, ppc, m68k.  the common "i2c" command already
> >> > takes care of exposing "i2c reset" for people to init this as
> >> > necessary, and other arches havent had any troubles that i can see
> >> > (certainly i know people have used i2c on Blackfin systems from
> >> > u-boot).
> >> 
> >> I feel the idea of calling i2c_init() multiple times with a speed
> >> parameter is not great. Perhaps the function is just misnamed.
> > 
> > perhaps it is more historical, but that is the current API, and it is
> > already called multiple times, so it's required to work.  i dont think
> > this has been a big deal so far for driver writers, but that doesnt mean
> > it has to stay this way.  but wrt your requirements, i dont think we
> > need to dive down that rabbit hole ... just kill off the explicit call
> > to i2c_init() in the arm board.c.
> 
> Er ok, but I found one in stdio.c!

i'd also vote that as a bug :).  in looking at the history, i think this is 
just a really old wart that we should burn off (it predates the git history 
which goes back to 2000-2002).  if there are specific devices that need i2c, 
they themselves should be calling i2c_init().

i know Wolfgang is sometimes hesitant to change these long seated things, so 
we might have to do what i was doing with the nand change ... the default is 
kept for a while, but people get a build warning if they are implicitly opted 
into the old behavior.  and if a board porter hasnt picked up their pieces by 
the time we drop the old behavior, well that's now their problem.  most people 
(the active) ones will continue to work.

check out the commit here:
http://git.denx.de/?p=u-boot/u-boot-
blackfin.git;a=commit;h=df57f7de71fd1540991a0099be307a65679c53bb

> >> > LCDs should only get initialized if it's the splash screen, otherwise
> >> > i cant think of any reason that hardware would ever get touched.  so
> >> > if you dont want the overhead of programming that hardware, then dont
> >> > enable splash screen support ?  i cant see a situation where you'd
> >> > enable splash screen support but not actually show the splash screen
> >> > ...
> >> 
> >> Well often U-Boot is asked to set up the LCD so that it will work for
> >> the kernel. There may be a specific power-on init sequence to follow
> >> even if nothing is actually written to the screen. For example in
> >> Chrome OS we don't have splash screen support but still init the LCD.
> > 
> > ruh-roh.  i think this is generally considered the wrong way to do
> > things. the kernel drivers should be able to pick up the hardware and
> > have it work regardless of what the (possibly brain dead) firmware that
> > came before it did. relying on the hardware to be in a certain state.
> >  it's even better done in the kernel as there you can do the init
> > asynchronously whereas in u-boot, everything is single threaded.
> 
> We can't have a sensible console in U-Boot without keyboard and LCD -
> for me the splash screen is a separate issue.
> 
> Yes the single-threaded problem does make it tricky. Am thinking about
> that problem now.

if the hardware can be split up into the "prime the settings, wait for things 
to prime, and then do something else", you could (ab)use the current board 
callback system (where the board gets access to a few hooks along the way) to 
split up the LCD init so that the polling-some-registers part doesnt really 
happen.

if you wanted to talk about not touching the LCD at all *unless* someone 
wanted to interact with the keyboard, that might be an interesting discussion.  
we could add a hook to the shell interpreter where it would call a board func 
upon first user input.  then you could skip the LCD part in the default (since 
you just want to boot linux automatically), but if someone wanted to interrupt 
that and talk to u-boot to debug, you'd get a slight pause.  optimize for the 
common and make the uncommon [debug] step take a little longer.

> >> >> To save time, reduce the amount of code executed and thereby improve
> >> >> security, we are wanting to initialize only some peripherals at the
> >> >> start. For example we might start up I2C and SC/MMC.
> >> > 
> >> > this is generally the policy now.  init code should only be
> >> > *registering* the availability of devices.  it shouldnt be
> >> > initializing the actual hardware until requested.
> >> 
> >> OK then I vote for a framework which understands this and calls driver
> >> init once (in a lazy fashion when needed if you like).
> > 
> > each framework should already do that.  if there's a specific one you
> > think is missing, then you should highlight it.
> > 
> > if you're talking about overhauling/unifying the driver model rather than
> > each one being open coded, that's a different thing.  but i dont think
> > your current requirements (as i understand them) necessitates such
> > effort.
> 
> I suppose the ad-hoc nature of drivers means that people miss the
> 'generally accepted way' that things are supposed to be done.

yes, but over the years i think (hope?) we've gotten better at catching this 
when someone submits something new.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110818/4d66b2e6/attachment.pgp 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] Selectable / delayed initialization
  2011-08-18 19:53         ` Mike Frysinger
@ 2011-08-18 20:12           ` Simon Glass
  0 siblings, 0 replies; 9+ messages in thread
From: Simon Glass @ 2011-08-18 20:12 UTC (permalink / raw)
  To: u-boot

Hi Mike,

On Thu, Aug 18, 2011 at 1:53 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Thursday, August 18, 2011 15:01:28 Simon Glass wrote:
...
>> OK that's not how it is at the moment in some cases, but fair enough.
>> We have been pushing Tegra in that directly as it happens, but
>> certainly there is still a fair amount of mandatory init which should
>> be moved into / called from drivers.
>
> if there's specific frameworks/arches that always init hardware, then in
> general that's a bug and we should treat it as such (the u-boot policy doc
> explicitly bans this). ?each framework (since they're all open coded in their
> own way) will need to be reviewed. ?but hopefully there shouldn't be many.

OK

...
>> Er ok, but I found one in stdio.c!
>
> i'd also vote that as a bug :). ?in looking at the history, i think this is
> just a really old wart that we should burn off (it predates the git history
> which goes back to 2000-2002). ?if there are specific devices that need i2c,
> they themselves should be calling i2c_init().
>
> i know Wolfgang is sometimes hesitant to change these long seated things, so
> we might have to do what i was doing with the nand change ... the default is
> kept for a while, but people get a build warning if they are implicitly opted
> into the old behavior. ?and if a board porter hasnt picked up their pieces by
> the time we drop the old behavior, well that's now their problem. ?most people
> (the active) ones will continue to work.
>
> check out the commit here:
> http://git.denx.de/?p=u-boot/u-boot-
> blackfin.git;a=commit;h=df57f7de71fd1540991a0099be307a65679c53bb

OK I see.

...
>> We can't have a sensible console in U-Boot without keyboard and LCD -
>> for me the splash screen is a separate issue.
>>
>> Yes the single-threaded problem does make it tricky. Am thinking about
>> that problem now.
>
> if the hardware can be split up into the "prime the settings, wait for things
> to prime, and then do something else", you could (ab)use the current board
> callback system (where the board gets access to a few hooks along the way) to
> split up the LCD init so that the polling-some-registers part doesnt really
> happen.

Yes - would like to hide all delays behind other running code, without
firing up the second CPU. But LCD is the worst.

>
> if you wanted to talk about not touching the LCD at all *unless* someone
> wanted to interact with the keyboard, that might be an interesting discussion.
> we could add a hook to the shell interpreter where it would call a board func
> upon first user input. ?then you could skip the LCD part in the default (since
> you just want to boot linux automatically), but if someone wanted to interrupt
> that and talk to u-boot to debug, you'd get a slight pause. ?optimize for the
> common and make the uncommon [debug] step take a little longer.

That's exactly the sort of thing that I think a driver framework would
handle nicely, and generically.

...
>> I suppose the ad-hoc nature of drivers means that people miss the
>> 'generally accepted way' that things are supposed to be done.
>
> yes, but over the years i think (hope?) we've gotten better at catching this
> when someone submits something new.
> -mike
>

Yes, the code is definitely improving so I think you are right.

Regards,
Simon

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2011-08-18 20:12 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-17  3:25 [U-Boot] Selectable / delayed initialization Simon Glass
2011-08-17  6:42 ` Graeme Russ
2011-08-18 15:46   ` Simon Glass
2011-08-18 15:33 ` Mike Frysinger
2011-08-18 15:59   ` Simon Glass
2011-08-18 17:54     ` Mike Frysinger
2011-08-18 19:01       ` Simon Glass
2011-08-18 19:53         ` Mike Frysinger
2011-08-18 20:12           ` Simon Glass

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.