[U-Boot] [RFC PATCH 0/22] Introduce driver model serial uclass
This series adds support for a serial uclass, enabling serial drivers to be converted to use driver model.
Unfortunately this is quite a complicated process for a number of reasons:
- serial is used before relocation, but driver model does not support this - stdio member functions are not passed a device pointer, but driver model requires this (so does serial, but it uses an ugly work-around) - driver model requires malloc() but this is not available before relocation - for sandbox, if something goes wrong with the console, we still need to get an error message out through the fallback console
So this series includes quite a few patches to address the above, as well as the serial uclass and an implementation for sandbox.
The early malloc() implementation uses a very simple region of memory which it doles out as needed. This means that driver model can start up with very little memory usage, and no overhead for malloc() itself except for space wasted due to alignment. With a 32-bit machine and just a serial port set as a pre-reloc device (plus the mandatory root device), the usage is 176 bytes.
If you have limited time, please take a look at least at the uclass patch which is 'dm: Add a uclass for serial devices' (see include/serial.h).
The series is based on the Tegra GPIO series v3. You can get this tree in a rough unfinished form from u-boot-x86.git branch us-gpioe.
Note: This series is for comments only as there are some build errors for certain boards.
Simon Glass (22): stdio: Remove redundant code around stdio_register() calls stdio: Pass device pointer to stdio methods dm: Make sure that the root device is probed dm: Provide a way to shut down driver model sandbox: Remove all drivers before exit dm: Tidy up four minor code nits dm: Allow drivers to be marked 'before relocation' dm: Use '*' to indicate a device is activated Remove form-feeds from dlmalloc.c Add a simple malloc() implementation for pre-relocation sandbox: config: Enable pre-relocation malloc() dm: Support driver model prior to relocation stdio: Provide functions to add/remove devices using stdio_dev console: Remove vprintf() optimisation for sandbox Add a flag indicating when the serial console is ready dm: Add a way to indicate a preferred device within a uclass dm: Expand and improve the device lifecycle docs dm: Add a uclass for serial devices Set up stdio earlier when using driver model sandbox: Convert serial driver to use driver model sandbox: serial: Support a coloured console sandbox: dts: Add a serial console node
README | 16 +++ arch/blackfin/cpu/jtag-console.c | 10 +- arch/powerpc/cpu/mpc512x/serial.c | 10 +- arch/powerpc/cpu/mpc8xx/video.c | 6 +- arch/sandbox/cpu/cpu.c | 4 + arch/sandbox/dts/sandbox.dts | 11 ++ arch/x86/lib/video.c | 8 +- board/bf527-ezkit/video.c | 10 -- board/bf548-ezkit/video.c | 10 -- board/cm-bf548/video.c | 10 -- board/mpl/common/kbd.c | 6 +- board/mpl/common/kbd.h | 6 +- board/mpl/pati/pati.c | 8 +- board/netphone/phone_console.c | 16 +-- board/nokia/rx51/rx51.c | 6 +- board/rbc823/kbd.c | 8 +- common/board_f.c | 42 ++++++++ common/board_r.c | 28 +++-- common/cmd_log.c | 11 +- common/console.c | 24 ++--- common/dlmalloc.c | 81 ++++++++++---- common/lcd.c | 14 ++- common/stdio.c | 63 ++++++++--- common/usb_kbd.c | 6 +- doc/driver-model/README.txt | 220 +++++++++++++++++++++++++++++++++++--- drivers/core/device.c | 11 +- drivers/core/lists.c | 6 +- drivers/core/root.c | 22 +++- drivers/core/uclass.c | 15 ++- drivers/input/cros_ec_keyb.c | 6 +- drivers/input/i8042.c | 4 +- drivers/input/keyboard.c | 6 +- drivers/input/tegra-kbc.c | 6 +- drivers/misc/cbmem_console.c | 6 +- drivers/net/netconsole.c | 10 +- drivers/serial/Makefile | 4 + drivers/serial/sandbox.c | 140 +++++++++++++++++++----- drivers/serial/serial-uclass.c | 161 ++++++++++++++++++++++++++++ drivers/serial/serial.c | 55 ++++++++-- drivers/serial/usbtty.c | 8 +- drivers/video/cfb_console.c | 8 +- include/asm-generic/global_data.h | 7 ++ include/common.h | 5 + include/configs/ELPPC.h | 4 +- include/configs/MHPC.h | 4 +- include/configs/jadecpu.h | 4 +- include/configs/nokia_rx51.h | 5 +- include/configs/sandbox.h | 7 ++ include/dm/device-internal.h | 6 +- include/dm/device.h | 10 +- include/dm/lists.h | 22 +++- include/dm/root.h | 18 +++- include/dm/uclass-id.h | 1 + include/dm/uclass-internal.h | 3 +- include/dm/uclass.h | 21 +++- include/i8042.h | 6 +- include/serial.h | 90 ++++++++++++++++ include/stdio_dev.h | 17 +-- include/video.h | 8 +- test/dm/cmd_dm.c | 11 +- test/dm/core.c | 49 +++++++-- test/dm/test-driver.c | 11 ++ test/dm/test-fdt.c | 34 +++++- test/dm/test-main.c | 4 +- test/dm/test.dts | 12 +++ 65 files changed, 1186 insertions(+), 275 deletions(-) create mode 100644 drivers/serial/serial-uclass.c
There is no point in setting a structure memory to NULL when it has already been zeroed with memset().
Also, there is no need to create a stub function for stdio to call - if the function is NULL it will not be called.
This is a clean-up, with no change in functionality.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/x86/lib/video.c | 4 ---- board/bf527-ezkit/video.c | 10 ---------- board/bf548-ezkit/video.c | 10 ---------- board/cm-bf548/video.c | 10 ---------- board/mpl/common/kbd.c | 2 -- board/rbc823/kbd.c | 2 -- common/usb_kbd.c | 2 -- drivers/input/keyboard.c | 2 -- drivers/video/cfb_console.c | 2 -- 9 files changed, 44 deletions(-)
diff --git a/arch/x86/lib/video.c b/arch/x86/lib/video.c index dfd2a84..eb9c595 100644 --- a/arch/x86/lib/video.c +++ b/arch/x86/lib/video.c @@ -178,8 +178,6 @@ int video_init(void) vga_dev.flags = DEV_FLAGS_OUTPUT | DEV_FLAGS_SYSTEM; vga_dev.putc = video_putc; /* 'putc' function */ vga_dev.puts = video_puts; /* 'puts' function */ - vga_dev.tstc = NULL; /* 'tstc' function */ - vga_dev.getc = NULL; /* 'getc' function */
if (stdio_register(&vga_dev) == 0) return 1; @@ -191,8 +189,6 @@ int video_init(void) strcpy(kbd_dev.name, "kbd"); kbd_dev.ext = 0; kbd_dev.flags = DEV_FLAGS_INPUT | DEV_FLAGS_SYSTEM; - kbd_dev.putc = NULL; /* 'putc' function */ - kbd_dev.puts = NULL; /* 'puts' function */ kbd_dev.tstc = i8042_tstc; /* 'tstc' function */ kbd_dev.getc = i8042_getc; /* 'getc' function */
diff --git a/board/bf527-ezkit/video.c b/board/bf527-ezkit/video.c index 5d8a091..c2bf145 100644 --- a/board/bf527-ezkit/video.c +++ b/board/bf527-ezkit/video.c @@ -391,14 +391,6 @@ void video_stop(void) #endif }
-void video_putc(const char c) -{ -} - -void video_puts(const char *s) -{ -} - int drv_video_init(void) { int error, devices = 1; @@ -448,8 +440,6 @@ int drv_video_init(void) strcpy(videodev.name, "video"); videodev.ext = DEV_EXT_VIDEO; /* Video extensions */ videodev.flags = DEV_FLAGS_SYSTEM; /* No Output */ - videodev.putc = video_putc; /* 'putc' function */ - videodev.puts = video_puts; /* 'puts' function */
error = stdio_register(&videodev);
diff --git a/board/bf548-ezkit/video.c b/board/bf548-ezkit/video.c index 6737ac1..47e68c6 100644 --- a/board/bf548-ezkit/video.c +++ b/board/bf548-ezkit/video.c @@ -281,14 +281,6 @@ static void dma_bitblit(void *dst, fastimage_t *logo, int x, int y)
}
-void video_putc(const char c) -{ -} - -void video_puts(const char *s) -{ -} - int drv_video_init(void) { int error, devices = 1; @@ -338,8 +330,6 @@ int drv_video_init(void) strcpy(videodev.name, "video"); videodev.ext = DEV_EXT_VIDEO; /* Video extensions */ videodev.flags = DEV_FLAGS_SYSTEM; /* No Output */ - videodev.putc = video_putc; /* 'putc' function */ - videodev.puts = video_puts; /* 'puts' function */
error = stdio_register(&videodev);
diff --git a/board/cm-bf548/video.c b/board/cm-bf548/video.c index c35d285..b098615 100644 --- a/board/cm-bf548/video.c +++ b/board/cm-bf548/video.c @@ -283,14 +283,6 @@ static void dma_bitblit(void *dst, fastimage_t *logo, int x, int y)
}
-void video_putc(const char c) -{ -} - -void video_puts(const char *s) -{ -} - int drv_video_init(void) { int error, devices = 1; @@ -342,8 +334,6 @@ int drv_video_init(void) strcpy(videodev.name, "video"); videodev.ext = DEV_EXT_VIDEO; /* Video extensions */ videodev.flags = DEV_FLAGS_SYSTEM; /* No Output */ - videodev.putc = video_putc; /* 'putc' function */ - videodev.puts = video_puts; /* 'puts' function */
error = stdio_register(&videodev);
diff --git a/board/mpl/common/kbd.c b/board/mpl/common/kbd.c index 1b5487b..f56545e 100644 --- a/board/mpl/common/kbd.c +++ b/board/mpl/common/kbd.c @@ -204,8 +204,6 @@ int drv_isa_kbd_init (void) memset (&kbddev, 0, sizeof(kbddev)); strcpy(kbddev.name, DEVNAME); kbddev.flags = DEV_FLAGS_INPUT | DEV_FLAGS_SYSTEM; - kbddev.putc = NULL ; - kbddev.puts = NULL ; kbddev.getc = kbd_getc ; kbddev.tstc = kbd_testc ;
diff --git a/board/rbc823/kbd.c b/board/rbc823/kbd.c index b35509a..1f2c1b9 100644 --- a/board/rbc823/kbd.c +++ b/board/rbc823/kbd.c @@ -240,8 +240,6 @@ int drv_keyboard_init(void) memset (&kbd_dev, 0, sizeof(struct stdio_dev)); strcpy(kbd_dev.name, "kbd"); kbd_dev.flags = DEV_FLAGS_INPUT | DEV_FLAGS_SYSTEM; - kbd_dev.putc = NULL; - kbd_dev.puts = NULL; kbd_dev.getc = smc1_getc; kbd_dev.tstc = smc1_tstc; error = stdio_register (&kbd_dev); diff --git a/common/usb_kbd.c b/common/usb_kbd.c index 0b77c16..371e5bc 100644 --- a/common/usb_kbd.c +++ b/common/usb_kbd.c @@ -522,8 +522,6 @@ int drv_usb_kbd_init(void) memset(&usb_kbd_dev, 0, sizeof(struct stdio_dev)); strcpy(usb_kbd_dev.name, DEVNAME); usb_kbd_dev.flags = DEV_FLAGS_INPUT | DEV_FLAGS_SYSTEM; - usb_kbd_dev.putc = NULL; - usb_kbd_dev.puts = NULL; usb_kbd_dev.getc = usb_kbd_getc; usb_kbd_dev.tstc = usb_kbd_testc; usb_kbd_dev.priv = (void *)dev; diff --git a/drivers/input/keyboard.c b/drivers/input/keyboard.c index 614592e..5ef1cc0 100644 --- a/drivers/input/keyboard.c +++ b/drivers/input/keyboard.c @@ -275,8 +275,6 @@ int kbd_init (void) memset (&kbddev, 0, sizeof(kbddev)); strcpy(kbddev.name, DEVNAME); kbddev.flags = DEV_FLAGS_INPUT | DEV_FLAGS_SYSTEM; - kbddev.putc = NULL ; - kbddev.puts = NULL ; kbddev.getc = kbd_getc ; kbddev.tstc = kbd_testc ;
diff --git a/drivers/video/cfb_console.c b/drivers/video/cfb_console.c index b52e9ed..1cf8660 100644 --- a/drivers/video/cfb_console.c +++ b/drivers/video/cfb_console.c @@ -2279,8 +2279,6 @@ int drv_video_init(void) console_dev.flags = DEV_FLAGS_OUTPUT | DEV_FLAGS_SYSTEM; console_dev.putc = video_putc; /* 'putc' function */ console_dev.puts = video_puts; /* 'puts' function */ - console_dev.tstc = NULL; /* 'tstc' function */ - console_dev.getc = NULL; /* 'getc' function */
#if !defined(CONFIG_VGA_AS_SINGLE_DEVICE) /* Also init console device */
On Saturday, May 24, 2014 at 11:21:00 PM, Simon Glass wrote:
There is no point in setting a structure memory to NULL when it has already been zeroed with memset().
Also, there is no need to create a stub function for stdio to call - if the function is NULL it will not be called.
This is a clean-up, with no change in functionality.
Signed-off-by: Simon Glass sjg@chromium.org
Acked-by: Marek Vasut marex@denx.de
Best regards, Marek Vasut
At present stdio device functions do not get any clue as to which stdio device is being acted on. Some implementations go to great lengths to work around this, such as defining a whole separate set of functions for each possible device.
For driver model we need to associate a stdio_dev with a device. It doesn't seem possible to continue with this work-around approach.
Instead, add a stdio_dev pointer to each of the stdio member functions.
Note: The serial drivers have the same problem, but it is not strictly necessary to fix that to get driver model running. Also, if we convert serial over to driver model the problem will go away.
Code size increases by 244 bytes for Thumb2 and 428 for PowerPC.
22: stdio: Pass device pointer to stdio methods arm: (for 2/2 boards) all +244.0 bss -4.0 text +248.0 powerpc: (for 1/1 boards) all +428.0 text +428.0
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/blackfin/cpu/jtag-console.c | 10 ++++---- arch/powerpc/cpu/mpc512x/serial.c | 10 ++++---- arch/powerpc/cpu/mpc8xx/video.c | 6 ++--- arch/x86/lib/video.c | 4 +-- board/mpl/common/kbd.c | 4 +-- board/mpl/common/kbd.h | 6 +++-- board/mpl/pati/pati.c | 8 +++--- board/netphone/phone_console.c | 16 ++++++------ board/nokia/rx51/rx51.c | 6 ++--- board/rbc823/kbd.c | 6 ++--- common/cmd_log.c | 11 ++++---- common/console.c | 18 ++++++------- common/lcd.c | 14 ++++++++-- common/stdio.c | 34 +++++++++++++++++++----- common/usb_kbd.c | 4 +-- drivers/input/cros_ec_keyb.c | 6 ++--- drivers/input/i8042.c | 4 +-- drivers/input/keyboard.c | 4 +-- drivers/input/tegra-kbc.c | 6 ++--- drivers/misc/cbmem_console.c | 6 ++--- drivers/net/netconsole.c | 10 ++++---- drivers/serial/serial.c | 54 ++++++++++++++++++++++++++++++++++----- drivers/serial/usbtty.c | 8 +++--- drivers/video/cfb_console.c | 6 ++--- include/common.h | 5 ++++ include/configs/ELPPC.h | 4 +-- include/configs/MHPC.h | 4 +-- include/configs/jadecpu.h | 4 +-- include/configs/nokia_rx51.h | 5 ++-- include/i8042.h | 6 +++-- include/stdio_dev.h | 15 ++++++----- include/video.h | 8 +++--- 32 files changed, 200 insertions(+), 112 deletions(-)
diff --git a/arch/blackfin/cpu/jtag-console.c b/arch/blackfin/cpu/jtag-console.c index 7cddb85..b8be318 100644 --- a/arch/blackfin/cpu/jtag-console.c +++ b/arch/blackfin/cpu/jtag-console.c @@ -112,11 +112,11 @@ static void jtag_send(const char *raw_str, uint32_t len) if (cooked_str != raw_str) free((char *)cooked_str); } -static void jtag_putc(const char c) +static void jtag_putc(struct stdio_dev *dev, const char c) { jtag_send(&c, 1); } -static void jtag_puts(const char *s) +static void jtag_puts(struct stdio_dev *dev, const char *s) { jtag_send(s, strlen(s)); } @@ -133,7 +133,7 @@ static int jtag_tstc_dbg(void) }
/* Higher layers want to know when any data is available */ -static int jtag_tstc(void) +static int jtag_tstc(struct stdio_dev *dev) { return jtag_tstc_dbg() || leftovers_len; } @@ -142,7 +142,7 @@ static int jtag_tstc(void) * [32bit length][actual data] */ static uint32_t leftovers; -static int jtag_getc(void) +static int jtag_getc(struct stdio_dev *dev) { int ret; uint32_t emudat; @@ -173,7 +173,7 @@ static int jtag_getc(void) leftovers = emudat; }
- return jtag_getc(); + return jtag_getc(dev); }
int drv_jtag_console_init(void) diff --git a/arch/powerpc/cpu/mpc512x/serial.c b/arch/powerpc/cpu/mpc512x/serial.c index 42e0dc9..4105a28 100644 --- a/arch/powerpc/cpu/mpc512x/serial.c +++ b/arch/powerpc/cpu/mpc512x/serial.c @@ -384,7 +384,7 @@ struct stdio_dev *open_port(int num, int baudrate) sprintf(env_val, "%d", baudrate); setenv(env_var, env_val);
- if (port->start()) + if (port->start(port)) return NULL;
set_bit(num, &initialized); @@ -407,7 +407,7 @@ int close_port(int num) if (!port) return -1;
- ret = port->stop(); + ret = port->stop(port); clear_bit(num, &initialized);
return ret; @@ -418,7 +418,7 @@ int write_port(struct stdio_dev *port, char *buf) if (!port || !buf) return -1;
- port->puts(buf); + port->puts(port, buf);
return 0; } @@ -433,8 +433,8 @@ int read_port(struct stdio_dev *port, char *buf, int size) if (!size) return 0;
- while (port->tstc()) { - buf[cnt++] = port->getc(); + while (port->tstc(port)) { + buf[cnt++] = port->getc(port); if (cnt > size) break; } diff --git a/arch/powerpc/cpu/mpc8xx/video.c b/arch/powerpc/cpu/mpc8xx/video.c index fc35158..7a43352 100644 --- a/arch/powerpc/cpu/mpc8xx/video.c +++ b/arch/powerpc/cpu/mpc8xx/video.c @@ -983,7 +983,7 @@ static inline void console_newline (void) } }
-void video_putc (const char c) +void video_putc(struct stdio_dev *dev, const char c) { if (!video_enable) { serial_putc (c); @@ -1020,7 +1020,7 @@ void video_putc (const char c) } }
-void video_puts (const char *s) +void video_puts(struct stdio_dev *dev, const char *s) { int count = strlen (s);
@@ -1029,7 +1029,7 @@ void video_puts (const char *s) serial_putc (*s++); else while (count--) - video_putc (*s++); + video_putc(dev, *s++); }
/************************************************************************/ diff --git a/arch/x86/lib/video.c b/arch/x86/lib/video.c index eb9c595..975949d 100644 --- a/arch/x86/lib/video.c +++ b/arch/x86/lib/video.c @@ -104,7 +104,7 @@ static void __video_putc(const char c, int *x, int *y) } }
-static void video_putc(const char c) +static void video_putc(struct stdio_dev *dev, const char c) { int x, y, pos;
@@ -123,7 +123,7 @@ static void video_putc(const char c) outb_p(0xff & (pos >> 1), vidport+1); }
-static void video_puts(const char *s) +static void video_puts(struct stdio_dev *dev, const char *s) { int x, y, pos; char c; diff --git a/board/mpl/common/kbd.c b/board/mpl/common/kbd.c index f56545e..99de2ca 100644 --- a/board/mpl/common/kbd.c +++ b/board/mpl/common/kbd.c @@ -248,7 +248,7 @@ void kbd_put_queue(char data) }
/* test if a character is in the queue */ -int kbd_testc(void) +int kbd_testc(struct stdio_dev *dev) { if(in_pointer==out_pointer) return(0); /* no data */ @@ -256,7 +256,7 @@ int kbd_testc(void) return(1); } /* gets the character from the queue */ -int kbd_getc(void) +int kbd_getc(struct stdio_dev *dev) { char c; while(in_pointer==out_pointer); diff --git a/board/mpl/common/kbd.h b/board/mpl/common/kbd.h index 7b19b37..b549e20 100644 --- a/board/mpl/common/kbd.h +++ b/board/mpl/common/kbd.h @@ -8,8 +8,10 @@ #ifndef _KBD_H_ #define _KBD_H_
-extern int kbd_testc(void); -extern int kbd_getc(void); +struct stdio_dev; + +int kbd_testc(struct stdio_dev *sdev); +int kbd_getc(struct stdio_dev *sdev); extern void kbd_interrupt(void); extern char *kbd_initialize(void);
diff --git a/board/mpl/pati/pati.c b/board/mpl/pati/pati.c index 8ca9bb3..5d701a7 100644 --- a/board/mpl/pati/pati.c +++ b/board/mpl/pati/pati.c @@ -445,7 +445,7 @@ void pci_con_put_it(const char c) PCICON_SET_REG(PCICON_DBELL_REG,PCIMSG_CON_DATA); }
-void pci_con_putc(const char c) +void pci_con_putc(struct stdio_dev *dev, const char c) { pci_con_put_it(c); if(c == '\n') @@ -453,7 +453,7 @@ void pci_con_putc(const char c) }
-int pci_con_getc(void) +int pci_con_getc(struct stdio_dev *dev) { int res; int diff; @@ -473,14 +473,14 @@ int pci_con_getc(void) return res; }
-int pci_con_tstc(void) +int pci_con_tstc(struct stdio_dev *dev) { if(r_ptr==(volatile int)w_ptr) return 0; return 1; }
-void pci_con_puts (const char *s) +void pci_con_puts(struct stdio_dev *dev, const char *s) { while (*s) { pci_con_putc(*s); diff --git a/board/netphone/phone_console.c b/board/netphone/phone_console.c index d195a39..7c14ee6 100644 --- a/board/netphone/phone_console.c +++ b/board/netphone/phone_console.c @@ -240,7 +240,7 @@ static void console_init(void)
/*****************************************************************************/
-void phone_putc(const char c); +static void phone_putc(struct stdio_dev *dev, const char c);
/*****************************************************************************/
@@ -250,7 +250,7 @@ static int enabled = 0; /*****************************************************************************/
/* flush buffers */ -int phone_start(void) +int phone_start(struct stdio_dev *dev) { console_init();
@@ -266,7 +266,7 @@ int phone_start(void) return 0; }
-int phone_stop(void) +int phone_stop(struct stdio_dev *dev) { enabled = 0;
@@ -279,20 +279,20 @@ int phone_stop(void) return 0; }
-void phone_puts(const char *s) +void phone_puts(struct stdio_dev *dev, const char *s) { int count = strlen(s);
while (count--) - phone_putc(*s++); + phone_putc(dev, *s++); }
-int phone_tstc(void) +int phone_tstc(struct stdio_dev *dev) { return queued_char >= 0 ? 1 : 0; }
-int phone_getc(void) +int phone_getc(struct stdio_dev *dev) { int r;
@@ -853,7 +853,7 @@ static void newline(void) } }
-void phone_putc(const char c) +static void phone_putc(struct stdio_dev *dev, const char c) { int i;
diff --git a/board/nokia/rx51/rx51.c b/board/nokia/rx51/rx51.c index 3e419ef..c2e07db 100644 --- a/board/nokia/rx51/rx51.c +++ b/board/nokia/rx51/rx51.c @@ -585,7 +585,7 @@ static void rx51_kp_fill(u8 k, u8 mods) * Routine: rx51_kp_tstc * Description: Test if key was pressed (from buffer). */ -int rx51_kp_tstc(void) +int rx51_kp_tstc(struct stdio_dev *sdev) { u8 c, r, dk, i; u8 intr; @@ -641,10 +641,10 @@ int rx51_kp_tstc(void) * Routine: rx51_kp_getc * Description: Get last pressed key (from buffer). */ -int rx51_kp_getc(void) +int rx51_kp_getc(struct stdio_dev *sdev) { keybuf_head %= KEYBUF_SIZE; - while (!rx51_kp_tstc()) + while (!rx51_kp_tstc(sdev)) WATCHDOG_RESET(); return keybuf[keybuf_head++]; } diff --git a/board/rbc823/kbd.c b/board/rbc823/kbd.c index 1f2c1b9..1aafdb7 100644 --- a/board/rbc823/kbd.c +++ b/board/rbc823/kbd.c @@ -161,7 +161,7 @@ int smc1_init (void) return (0); }
-void smc1_putc(const char c) +void smc1_putc(struct stdio_dev *dev, const char c) { volatile cbd_t *tbdf; volatile char *buf; @@ -189,7 +189,7 @@ void smc1_putc(const char c) } }
-int smc1_getc(void) +int smc1_getc(struct stdio_dev *dev) { volatile cbd_t *rbdf; volatile unsigned char *buf; @@ -215,7 +215,7 @@ int smc1_getc(void) return(c); }
-int smc1_tstc(void) +int smc1_tstc(struct stdio_dev *dev) { volatile cbd_t *rbdf; volatile smc_uart_t *up; diff --git a/common/cmd_log.c b/common/cmd_log.c index 38d0f5e..873ee40 100644 --- a/common/cmd_log.c +++ b/common/cmd_log.c @@ -33,8 +33,8 @@ DECLARE_GLOBAL_DATA_PTR;
/* Local prototypes */ -static void logbuff_putc(const char c); -static void logbuff_puts(const char *s); +static void logbuff_putc(struct stdio_dev *dev, const char c); +static void logbuff_puts(struct stdio_dev *dev, const char *s); static int logbuff_printk(const char *line);
static char buf[1024]; @@ -143,7 +143,7 @@ int drv_logbuff_init(void) return (rc == 0) ? 1 : rc; }
-static void logbuff_putc(const char c) +static void logbuff_putc(struct stdio_dev *dev, const char c) { char buf[2]; buf[0] = c; @@ -151,7 +151,7 @@ static void logbuff_putc(const char c) logbuff_printk(buf); }
-static void logbuff_puts(const char *s) +static void logbuff_puts(struct stdio_dev *dev, const char *s) { logbuff_printk (s); } @@ -181,6 +181,7 @@ void logbuff_log(char *msg) */ int do_log(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { + struct stdio_dev *sdev = NULL; char *s; unsigned long i, start, size;
@@ -188,7 +189,7 @@ int do_log(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) /* Log concatenation of all arguments separated by spaces */ for (i = 2; i < argc; i++) { logbuff_printk(argv[i]); - logbuff_putc((i < argc - 1) ? ' ' : '\n'); + logbuff_putc(sdev, (i < argc - 1) ? ' ' : '\n'); } return 0; } diff --git a/common/console.c b/common/console.c index 5453726..11c102a 100644 --- a/common/console.c +++ b/common/console.c @@ -109,7 +109,7 @@ static int console_setfile(int file, struct stdio_dev * dev) case stderr: /* Start new device */ if (dev->start) { - error = dev->start(); + error = dev->start(dev); /* If it's not started dont use it */ if (error < 0) break; @@ -159,7 +159,7 @@ static int console_getc(int file) unsigned char ret;
/* This is never called with testcdev == NULL */ - ret = tstcdev->getc(); + ret = tstcdev->getc(tstcdev); tstcdev = NULL; return ret; } @@ -173,7 +173,7 @@ static int console_tstc(int file) for (i = 0; i < cd_count[file]; i++) { dev = console_devices[file][i]; if (dev->tstc != NULL) { - ret = dev->tstc(); + ret = dev->tstc(dev); if (ret > 0) { tstcdev = dev; disable_ctrlc(0); @@ -194,7 +194,7 @@ static void console_putc(int file, const char c) for (i = 0; i < cd_count[file]; i++) { dev = console_devices[file][i]; if (dev->putc != NULL) - dev->putc(c); + dev->putc(dev, c); } }
@@ -206,7 +206,7 @@ static void console_puts(int file, const char *s) for (i = 0; i < cd_count[file]; i++) { dev = console_devices[file][i]; if (dev->puts != NULL) - dev->puts(s); + dev->puts(dev, s); } }
@@ -222,22 +222,22 @@ static inline void console_doenv(int file, struct stdio_dev *dev) #else static inline int console_getc(int file) { - return stdio_devices[file]->getc(); + return stdio_devices[file]->getc(stdio_devices[file]); }
static inline int console_tstc(int file) { - return stdio_devices[file]->tstc(); + return stdio_devices[file]->tstc(stdio_devices[file]); }
static inline void console_putc(int file, const char c) { - stdio_devices[file]->putc(c); + stdio_devices[file]->putc(stdio_devices[file], c); }
static inline void console_puts(int file, const char *s) { - stdio_devices[file]->puts(s); + stdio_devices[file]->puts(stdio_devices[file], s); }
static inline void console_printdevs(int file) diff --git a/common/lcd.c b/common/lcd.c index 19b86b7..feb913a 100644 --- a/common/lcd.c +++ b/common/lcd.c @@ -214,6 +214,11 @@ static inline void console_newline(void)
/*----------------------------------------------------------------------*/
+static void lcd_stub_putc(struct stdio_dev *dev, const char c) +{ + lcd_putc(c); +} + void lcd_putc(const char c) { if (!lcd_is_enabled) { @@ -253,6 +258,11 @@ void lcd_putc(const char c)
/*----------------------------------------------------------------------*/
+static void lcd_stub_puts(struct stdio_dev *dev, const char *s) +{ + lcd_puts(s); +} + void lcd_puts(const char *s) { if (!lcd_is_enabled) { @@ -426,8 +436,8 @@ int drv_lcd_init(void) strcpy(lcddev.name, "lcd"); lcddev.ext = 0; /* No extensions */ lcddev.flags = DEV_FLAGS_OUTPUT; /* Output only */ - lcddev.putc = lcd_putc; /* 'putc' function */ - lcddev.puts = lcd_puts; /* 'puts' function */ + lcddev.putc = lcd_stub_putc; /* 'putc' function */ + lcddev.puts = lcd_stub_puts; /* 'puts' function */
rc = stdio_register(&lcddev);
diff --git a/common/stdio.c b/common/stdio.c index 844f98c..dd402cc 100644 --- a/common/stdio.c +++ b/common/stdio.c @@ -35,23 +35,43 @@ char *stdio_names[MAX_FILES] = { "stdin", "stdout", "stderr" };
#ifdef CONFIG_SYS_DEVICE_NULLDEV -void nulldev_putc(const char c) +void nulldev_putc(struct stdio_dev *dev, const char c) { /* nulldev is empty! */ }
-void nulldev_puts(const char *s) +void nulldev_puts(struct stdio_dev *dev, const char *s) { /* nulldev is empty! */ }
-int nulldev_input(void) +int nulldev_input(struct stdio_dev *dev) { /* nulldev is empty! */ return 0; } #endif
+void stdio_serial_putc(struct stdio_dev *dev, const char c) +{ + serial_putc(c); +} + +void stdio_serial_puts(struct stdio_dev *dev, const char *s) +{ + serial_puts(s); +} + +int stdio_serial_getc(struct stdio_dev *dev) +{ + return serial_getc(); +} + +int stdio_serial_tstc(struct stdio_dev *dev) +{ + return serial_tstc(); +} + /************************************************************************** * SYSTEM DRIVERS ************************************************************************** @@ -65,10 +85,10 @@ static void drv_system_init (void)
strcpy (dev.name, "serial"); dev.flags = DEV_FLAGS_OUTPUT | DEV_FLAGS_INPUT | DEV_FLAGS_SYSTEM; - dev.putc = serial_putc; - dev.puts = serial_puts; - dev.getc = serial_getc; - dev.tstc = serial_tstc; + dev.putc = stdio_serial_putc; + dev.puts = stdio_serial_puts; + dev.getc = stdio_serial_getc; + dev.tstc = stdio_serial_tstc; stdio_register (&dev);
#ifdef CONFIG_SYS_DEVICE_NULLDEV diff --git a/common/usb_kbd.c b/common/usb_kbd.c index 371e5bc..c34fd5c 100644 --- a/common/usb_kbd.c +++ b/common/usb_kbd.c @@ -360,7 +360,7 @@ static inline void usb_kbd_poll_for_event(struct usb_device *dev) }
/* test if a character is in the queue */ -static int usb_kbd_testc(void) +static int usb_kbd_testc(struct stdio_dev *sdev) { struct stdio_dev *dev; struct usb_device *usb_kbd_dev; @@ -386,7 +386,7 @@ static int usb_kbd_testc(void) }
/* gets the character from the queue */ -static int usb_kbd_getc(void) +static int usb_kbd_getc(struct stdio_dev *sdev) { struct stdio_dev *dev; struct usb_device *usb_kbd_dev; diff --git a/drivers/input/cros_ec_keyb.c b/drivers/input/cros_ec_keyb.c index a2501e0..47502b1 100644 --- a/drivers/input/cros_ec_keyb.c +++ b/drivers/input/cros_ec_keyb.c @@ -93,7 +93,7 @@ static int check_for_keys(struct keyb *config, * * @return 0 if no keys available, 1 if keys are available */ -static int kbd_tstc(void) +static int kbd_tstc(struct stdio_dev *dev) { /* Just get input to do this for us */ return config.inited ? input_tstc(&config.input) : 0; @@ -104,7 +104,7 @@ static int kbd_tstc(void) * * @return ASCII key code, or 0 if no key, or -1 if error */ -static int kbd_getc(void) +static int kbd_getc(struct stdio_dev *dev) { /* Just get input to do this for us */ return config.inited ? input_getc(&config.input) : 0; @@ -214,7 +214,7 @@ static int cros_ec_keyb_decode_fdt(const void *blob, int node, * * @return 0 if ok, -1 on error */ -static int cros_ec_init_keyboard(void) +static int cros_ec_init_keyboard(struct stdio_dev *dev) { const void *blob = gd->fdt_blob; int node; diff --git a/drivers/input/i8042.c b/drivers/input/i8042.c index 35fa0bb..ca1604c 100644 --- a/drivers/input/i8042.c +++ b/drivers/input/i8042.c @@ -398,7 +398,7 @@ int i8042_kbd_init(void) * i8042_tstc - test if keyboard input is available * option: cursor blinking if called in a loop */ -int i8042_tstc(void) +int i8042_tstc(struct stdio_dev *dev) { unsigned char scan_code = 0;
@@ -432,7 +432,7 @@ int i8042_tstc(void) * i8042_getc - wait till keyboard input is available * option: turn on/off cursor while waiting */ -int i8042_getc(void) +int i8042_getc(struct stdio_dev *dev) { int ret_chr; unsigned char scan_code; diff --git a/drivers/input/keyboard.c b/drivers/input/keyboard.c index 5ef1cc0..be0f333 100644 --- a/drivers/input/keyboard.c +++ b/drivers/input/keyboard.c @@ -70,7 +70,7 @@ static void kbd_put_queue(char data) }
/* test if a character is in the queue */ -static int kbd_testc(void) +static int kbd_testc(struct stdio_dev *dev) { #if defined(CONFIG_MPC5xxx) || defined(CONFIG_MPC8540) || defined(CONFIG_MPC8541) || defined(CONFIG_MPC8555) /* no ISR is used, so received chars must be polled */ @@ -83,7 +83,7 @@ static int kbd_testc(void) }
/* gets the character from the queue */ -static int kbd_getc(void) +static int kbd_getc(struct stdio_dev *dev) { char c; while(in_pointer==out_pointer) { diff --git a/drivers/input/tegra-kbc.c b/drivers/input/tegra-kbc.c index f137f93..7e36db0 100644 --- a/drivers/input/tegra-kbc.c +++ b/drivers/input/tegra-kbc.c @@ -194,7 +194,7 @@ int tegra_kbc_check(struct input_config *input) * * @return 0 if no keys available, 1 if keys are available */ -static int kbd_tstc(void) +static int kbd_tstc(struct stdio_dev *dev) { /* Just get input to do this for us */ return input_tstc(&config.input); @@ -207,7 +207,7 @@ static int kbd_tstc(void) * * @return ASCII key code, or 0 if no key, or -1 if error */ -static int kbd_getc(void) +static int kbd_getc(struct stdio_dev *dev) { /* Just get input to do this for us */ return input_getc(&config.input); @@ -289,7 +289,7 @@ static void tegra_kbc_open(void) * * @return 0 if ok, -ve on error */ -static int init_tegra_keyboard(void) +static int init_tegra_keyboard(struct stdio_dev *dev) { /* check if already created */ if (config.created) diff --git a/drivers/misc/cbmem_console.c b/drivers/misc/cbmem_console.c index 80a84fd..5f85ccf 100644 --- a/drivers/misc/cbmem_console.c +++ b/drivers/misc/cbmem_console.c @@ -31,7 +31,7 @@ struct cbmem_console {
static struct cbmem_console *cbmem_console_p;
-void cbmemc_putc(char data) +void cbmemc_putc(struct stdio_dev *dev, char data) { int cursor;
@@ -40,12 +40,12 @@ void cbmemc_putc(char data) cbmem_console_p->buffer_body[cursor] = data; }
-void cbmemc_puts(const char *str) +void cbmemc_puts(struct stdio_dev *dev, const char *str) { char c;
while ((c = *str++) != 0) - cbmemc_putc(c); + cbmemc_putc(dev, c); }
int cbmemc_init(void) diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c index 65c747e..623f749 100644 --- a/drivers/net/netconsole.c +++ b/drivers/net/netconsole.c @@ -215,7 +215,7 @@ static void nc_send_packet(const char *buf, int len) } }
-static int nc_start(void) +static int nc_start(struct stdio_dev *dev) { int retval;
@@ -235,7 +235,7 @@ static int nc_start(void) return 0; }
-static void nc_putc(char c) +static void nc_putc(struct stdio_dev *dev, char c) { if (output_recursion) return; @@ -246,7 +246,7 @@ static void nc_putc(char c) output_recursion = 0; }
-static void nc_puts(const char *s) +static void nc_puts(struct stdio_dev *dev, const char *s) { int len;
@@ -265,7 +265,7 @@ static void nc_puts(const char *s) output_recursion = 0; }
-static int nc_getc(void) +static int nc_getc(struct stdio_dev *dev) { uchar c;
@@ -286,7 +286,7 @@ static int nc_getc(void) return c; }
-static int nc_tstc(void) +static int nc_tstc(struct stdio_dev *dev) { struct eth_device *eth;
diff --git a/drivers/serial/serial.c b/drivers/serial/serial.c index fd61a5e..803d850 100644 --- a/drivers/serial/serial.c +++ b/drivers/serial/serial.c @@ -254,6 +254,48 @@ void serial_initialize(void) serial_assign(default_serial_console()->name); }
+int serial_stub_start(struct stdio_dev *sdev) +{ + struct serial_device *dev = sdev->priv; + + return dev->start(); +} + +int serial_stub_stop(struct stdio_dev *sdev) +{ + struct serial_device *dev = sdev->priv; + + return dev->stop(); +} + +void serial_stub_putc(struct stdio_dev *sdev, const char ch) +{ + struct serial_device *dev = sdev->priv; + + dev->putc(ch); +} + +void serial_stub_puts(struct stdio_dev *sdev, const char *str) +{ + struct serial_device *dev = sdev->priv; + + dev->puts(str); +} + +int serial_stub_getc(struct stdio_dev *sdev) +{ + struct serial_device *dev = sdev->priv; + + return dev->getc(); +} + +int serial_stub_tstc(struct stdio_dev *sdev) +{ + struct serial_device *dev = sdev->priv; + + return dev->tstc(); +} + /** * serial_stdio_init() - Register serial ports with STDIO core * @@ -272,12 +314,12 @@ void serial_stdio_init(void) strcpy(dev.name, s->name); dev.flags = DEV_FLAGS_OUTPUT | DEV_FLAGS_INPUT;
- dev.start = s->start; - dev.stop = s->stop; - dev.putc = s->putc; - dev.puts = s->puts; - dev.getc = s->getc; - dev.tstc = s->tstc; + dev.start = serial_stub_start; + dev.stop = serial_stub_stop; + dev.putc = serial_stub_putc; + dev.puts = serial_stub_puts; + dev.getc = serial_stub_getc; + dev.tstc = serial_stub_tstc;
stdio_register(&dev);
diff --git a/drivers/serial/usbtty.c b/drivers/serial/usbtty.c index 6b912ef..b030526 100644 --- a/drivers/serial/usbtty.c +++ b/drivers/serial/usbtty.c @@ -389,7 +389,7 @@ static void str2wide (char *str, u16 * wide) * Test whether a character is in the RX buffer */
-int usbtty_tstc (void) +int usbtty_tstc(struct stdio_dev *dev) { struct usb_endpoint_instance *endpoint = &endpoint_instance[rx_endpoint]; @@ -409,7 +409,7 @@ int usbtty_tstc (void) * written into its argument c. */
-int usbtty_getc (void) +int usbtty_getc(struct stdio_dev *dev) { char c; struct usb_endpoint_instance *endpoint = @@ -429,7 +429,7 @@ int usbtty_getc (void) /* * Output a single byte to the usb client port. */ -void usbtty_putc (const char c) +void usbtty_putc(struct stdio_dev *dev, const char c) { if (!usbtty_configured ()) return; @@ -484,7 +484,7 @@ static void __usbtty_puts (const char *str, int len) } }
-void usbtty_puts (const char *str) +void usbtty_puts(struct stdio_dev *dev, const char *str) { int n; int len; diff --git a/drivers/video/cfb_console.c b/drivers/video/cfb_console.c index 1cf8660..9231927 100644 --- a/drivers/video/cfb_console.c +++ b/drivers/video/cfb_console.c @@ -944,7 +944,7 @@ static void parse_putc(const char c) CURSOR_SET; }
-void video_putc(const char c) +void video_putc(struct stdio_dev *dev, const char c) { #ifdef CONFIG_CFB_CONSOLE_ANSI int i; @@ -1158,12 +1158,12 @@ void video_putc(const char c) flush_cache(VIDEO_FB_ADRS, VIDEO_SIZE); }
-void video_puts(const char *s) +void video_puts(struct stdio_dev *dev, const char *s) { int count = strlen(s);
while (count--) - video_putc(*s++); + video_putc(dev, *s++); }
/* diff --git a/include/common.h b/include/common.h index 232136c..0fd57f1 100644 --- a/include/common.h +++ b/include/common.h @@ -649,6 +649,11 @@ void serial_puts (const char *); int serial_getc (void); int serial_tstc (void);
+/* These versions take a stdio_dev pointer */ +struct stdio_dev; +int serial_stub_getc(struct stdio_dev *sdev); +int serial_stub_tstc(struct stdio_dev *sdev); + void _serial_setbrg (const int); void _serial_putc (const char, const int); void _serial_putc_raw(const char, const int); diff --git a/include/configs/ELPPC.h b/include/configs/ELPPC.h index 0ffbd41..debfc36 100644 --- a/include/configs/ELPPC.h +++ b/include/configs/ELPPC.h @@ -234,8 +234,8 @@ #define CONFIG_VIDEO #define CONFIG_CFB_CONSOLE #define VIDEO_KBD_INIT_FCT (simple_strtol (getenv("console"), NULL, 10)) -#define VIDEO_TSTC_FCT serial_tstc -#define VIDEO_GETC_FCT serial_getc +#define VIDEO_TSTC_FCT serial_stub_tstc +#define VIDEO_GETC_FCT serial_stub_getc
#define CONFIG_VIDEO_SMI_LYNXEM #define CONFIG_VIDEO_LOGO diff --git a/include/configs/MHPC.h b/include/configs/MHPC.h index 6314b53..d45be0f 100644 --- a/include/configs/MHPC.h +++ b/include/configs/MHPC.h @@ -96,8 +96,8 @@ #define CONFIG_VIDEO_LOGO
#define VIDEO_KBD_INIT_FCT 0 /* no KBD dev on MHPC - use serial */ -#define VIDEO_TSTC_FCT serial_tstc -#define VIDEO_GETC_FCT serial_getc +#define VIDEO_TSTC_FCT serial_stub_tstc +#define VIDEO_GETC_FCT serial_stub_getc
#define CONFIG_BR0_WORKAROUND 1
diff --git a/include/configs/jadecpu.h b/include/configs/jadecpu.h index b34e342..759e112 100644 --- a/include/configs/jadecpu.h +++ b/include/configs/jadecpu.h @@ -87,8 +87,8 @@ #define CONFIG_SYS_VIDEO_LOGO_MAX_SIZE (800*480 + 256*4 + 10*1024) #define VIDEO_FB_16BPP_WORD_SWAP #define VIDEO_KBD_INIT_FCT 0 -#define VIDEO_TSTC_FCT serial_tstc -#define VIDEO_GETC_FCT serial_getc +#define VIDEO_TSTC_FCT serial_stub_tstc +#define VIDEO_GETC_FCT serial_stub_getc
/* * BOOTP options diff --git a/include/configs/nokia_rx51.h b/include/configs/nokia_rx51.h index e0c0fac..53cb390 100644 --- a/include/configs/nokia_rx51.h +++ b/include/configs/nokia_rx51.h @@ -263,9 +263,10 @@ #define VIDEO_TSTC_FCT rx51_kp_tstc #define VIDEO_GETC_FCT rx51_kp_getc #ifndef __ASSEMBLY__ +struct stdio_dev; int rx51_kp_init(void); -int rx51_kp_tstc(void); -int rx51_kp_getc(void); +int rx51_kp_tstc(struct stdio_dev *sdev); +int rx51_kp_getc(struct stdio_dev *sdev); #endif
#ifndef MTDPARTS_DEFAULT diff --git a/include/i8042.h b/include/i8042.h index 96306192..58c85ec 100644 --- a/include/i8042.h +++ b/include/i8042.h @@ -72,8 +72,10 @@ void i8042_flush(void); */ int i8042_disable(void);
+struct stdio_dev; + int i8042_kbd_init(void); -int i8042_tstc(void); -int i8042_getc(void); +int i8042_tstc(struct stdio_dev *dev); +int i8042_getc(struct stdio_dev *dev);
#endif /* _I8042_H_ */ diff --git a/include/stdio_dev.h b/include/stdio_dev.h index e6dc12a..4587005 100644 --- a/include/stdio_dev.h +++ b/include/stdio_dev.h @@ -27,18 +27,21 @@ struct stdio_dev {
/* GENERAL functions */
- int (*start) (void); /* To start the device */ - int (*stop) (void); /* To stop the device */ + int (*start)(struct stdio_dev *dev); /* To start the device */ + int (*stop)(struct stdio_dev *dev); /* To stop the device */
/* OUTPUT functions */
- void (*putc) (const char c); /* To put a char */ - void (*puts) (const char *s); /* To put a string (accelerator) */ + /* To put a char */ + void (*putc)(struct stdio_dev *dev, const char c); + /* To put a string (accelerator) */ + void (*puts)(struct stdio_dev *dev, const char *s);
/* INPUT functions */
- int (*tstc) (void); /* To test if a char is ready... */ - int (*getc) (void); /* To get that char */ + /* To test if a char is ready... */ + int (*tstc)(struct stdio_dev *dev); + int (*getc)(struct stdio_dev *dev); /* To get that char */
/* Other functions */
diff --git a/include/video.h b/include/video.h index 0ff857b..673aa2e 100644 --- a/include/video.h +++ b/include/video.h @@ -11,9 +11,11 @@
/* Video functions */
-int video_init (void *videobase); -void video_putc (const char c); -void video_puts (const char *s); +struct stdio_dev; + +int video_init(void *videobase); +void video_putc(struct stdio_dev *dev, const char c); +void video_puts(struct stdio_dev *dev, const char *s);
/** * Display a BMP format bitmap on the screen
On Saturday, May 24, 2014 at 11:21:01 PM, Simon Glass wrote:
At present stdio device functions do not get any clue as to which stdio device is being acted on. Some implementations go to great lengths to work around this, such as defining a whole separate set of functions for each possible device.
For driver model we need to associate a stdio_dev with a device. It doesn't seem possible to continue with this work-around approach.
Instead, add a stdio_dev pointer to each of the stdio member functions.
Note: The serial drivers have the same problem, but it is not strictly necessary to fix that to get driver model running. Also, if we convert serial over to driver model the problem will go away.
Code size increases by 244 bytes for Thumb2 and 428 for PowerPC.
22: stdio: Pass device pointer to stdio methods arm: (for 2/2 boards) all +244.0 bss -4.0 text +248.0 powerpc: (for 1/1 boards) all +428.0 text +428.0
Signed-off-by: Simon Glass sjg@chromium.org
Thanks for doing this !
Reviewed-by: Marek Vasut marex@denx.de
Best regards, Marek Vasut
The root device should be probed just like any other device. The effect of this is to mark the device as activated, so that it can be removed (along with its children) if required.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/root.c | 3 +++ test/dm/core.c | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/core/root.c b/drivers/core/root.c index 4427b81..9fff164 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -48,6 +48,9 @@ int dm_init(void) ret = device_bind_by_name(NULL, &root_info, &DM_ROOT()); if (ret) return ret; + ret = device_probe(DM_ROOT()); + if (ret) + return ret;
return 0; } diff --git a/test/dm/core.c b/test/dm/core.c index 14a57c3..a889fad 100644 --- a/test/dm/core.c +++ b/test/dm/core.c @@ -106,7 +106,7 @@ static int dm_test_autoprobe(struct dm_test_state *dms) ut_asserteq(0, dm_testdrv_op_count[DM_TEST_OP_POST_PROBE]);
/* The root device should not be activated until needed */ - ut_assert(!(dms->root->flags & DM_FLAG_ACTIVATED)); + ut_assert(dms->root->flags & DM_FLAG_ACTIVATED);
/* * We should be able to find the three test devices, and they should
On Saturday, May 24, 2014 at 11:21:02 PM, Simon Glass wrote:
The root device should be probed just like any other device. The effect of this is to mark the device as activated, so that it can be removed (along with its children) if required.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/core/root.c | 3 +++ test/dm/core.c | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/core/root.c b/drivers/core/root.c index 4427b81..9fff164 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -48,6 +48,9 @@ int dm_init(void) ret = device_bind_by_name(NULL, &root_info, &DM_ROOT());
Off-topic: This &DM_ROOT() above looks a little suspicious, don't you think? [...]
Acked-by: Marek Vasut marex@denx.de
Best regards, Marek Vasut
Hi Marek,
On 1 June 2014 11:33, Marek Vasut marex@denx.de wrote:
On Saturday, May 24, 2014 at 11:21:02 PM, Simon Glass wrote:
The root device should be probed just like any other device. The effect of this is to mark the device as activated, so that it can be removed (along with its children) if required.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/core/root.c | 3 +++ test/dm/core.c | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/core/root.c b/drivers/core/root.c index 4427b81..9fff164 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -48,6 +48,9 @@ int dm_init(void) ret = device_bind_by_name(NULL, &root_info, &DM_ROOT());
Off-topic: This &DM_ROOT() above looks a little suspicious, don't you think?
Yes. I wonder if it should not be written as a function?
Regards, Simon
On Tuesday, June 03, 2014 at 03:59:49 AM, Simon Glass wrote:
Hi Marek,
On 1 June 2014 11:33, Marek Vasut marex@denx.de wrote:
On Saturday, May 24, 2014 at 11:21:02 PM, Simon Glass wrote:
The root device should be probed just like any other device. The effect of this is to mark the device as activated, so that it can be removed (along with its children) if required.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/core/root.c | 3 +++ test/dm/core.c | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/core/root.c b/drivers/core/root.c index 4427b81..9fff164 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -48,6 +48,9 @@ int dm_init(void)
ret = device_bind_by_name(NULL, &root_info, &DM_ROOT());Off-topic: This &DM_ROOT() above looks a little suspicious, don't you think?
Yes. I wonder if it should not be written as a function?
That'd be a good start. I cannot even find it's definition anywhere :-/
Best regards, Marek Vasut
Add a new method which removes and unbinds all drivers.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/root.c | 8 ++++++++ include/dm/root.h | 8 ++++++++ 2 files changed, 16 insertions(+)
diff --git a/drivers/core/root.c b/drivers/core/root.c index 9fff164..4873e7b 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -55,6 +55,14 @@ int dm_init(void) return 0; }
+int dm_uninit(void) +{ + device_remove(dm_root()); + device_unbind(dm_root()); + + return 0; +} + int dm_scan_platdata(void) { int ret; diff --git a/include/dm/root.h b/include/dm/root.h index 0ebccda..069d058 100644 --- a/include/dm/root.h +++ b/include/dm/root.h @@ -50,4 +50,12 @@ int dm_scan_fdt(const void *blob); */ int dm_init(void);
+/** + * dm_uninit - Uninitialise Driver Model structures + * + * All devices will be removed and unbound + * @return 0 if OK, -ve on error + */ +int dm_uninit(void); + #endif
On Saturday, May 24, 2014 at 11:21:03 PM, Simon Glass wrote:
Add a new method which removes and unbinds all drivers.
Signed-off-by: Simon Glass sjg@chromium.org
Acked-by: Marek Vasut marex@denx.de
Best regards, Marek Vasut
Drivers are supposed to be able to close down cleanly. To set a good example, make sandbox shut down its driver model drivers and remove them before exit.
It may be desirable to do the same more generally once driver model is more widely-used. This could be done during bootm, before U-Boot jumps to the OS. It seems far too early to make this change.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/sandbox/cpu/cpu.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c index 3f4005b..1aa397c 100644 --- a/arch/sandbox/cpu/cpu.c +++ b/arch/sandbox/cpu/cpu.c @@ -4,6 +4,7 @@ */
#include <common.h> +#include <dm/root.h> #include <os.h> #include <asm/state.h>
@@ -14,6 +15,9 @@ void reset_cpu(ulong ignored) if (state_uninit()) os_exit(2);
+ if (dm_uninit()) + os_exit(2); + /* This is considered normal termination for now */ os_exit(0); }
On Sat, May 24, 2014 at 4:21 PM, Simon Glass sjg@chromium.org wrote:
Drivers are supposed to be able to close down cleanly. To set a good example, make sandbox shut down its driver model drivers and remove them before exit.
Right. But we should be careful here...
It may be desirable to do the same more generally once driver model is more widely-used. This could be done during bootm, before U-Boot jumps to the OS. It seems far too early to make this change.
Some drivers should probably NOT be shut down prior to bootm handing off to some other OS. It could be that their sole purpose for starting and running was to enable some continued driver state for the OS.
I'm thinking of some clock, or PHY or maybe some aspect of the ARM SCU that should be "left running".
So maybe if there is a general "shutdown" pass, there might be a need for a flag to indicate that it is/isn't OK to do so at OS-hand-off time.
jdl
Hi Jon,
On 27 May 2014 09:28, Jon Loeliger loeliger@gmail.com wrote:
On Sat, May 24, 2014 at 4:21 PM, Simon Glass sjg@chromium.org wrote:
Drivers are supposed to be able to close down cleanly. To set a good example, make sandbox shut down its driver model drivers and remove them before exit.
Right. But we should be careful here...
It may be desirable to do the same more generally once driver model is more widely-used. This could be done during bootm, before U-Boot jumps to the OS. It seems far too early to make this change.
Some drivers should probably NOT be shut down prior to bootm handing off to some other OS. It could be that their sole purpose for starting and running was to enable some continued driver state for the OS.
I'm thinking of some clock, or PHY or maybe some aspect of the ARM SCU that should be "left running".
So maybe if there is a general "shutdown" pass, there might be a need for a flag to indicate that it is/isn't OK to do so at OS-hand-off time.
Yes agreed. I think we can cross that bridge when we come to it though. So long as the drivers have a way of shutting down then we have at least encouraged driver authors to think about this issue. My concern was that in U-Boot people might think that the remove() method is a waste of time.
Regards, Simon
There is a spelling mistake and two functions are missing comments altogether. Also the flags declaration is correct, but doesn't follow style. Finally, the uclass_get_device() function has some errors in its documentation.
Fix these problems.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/dm/device.h | 2 +- include/dm/lists.h | 20 ++++++++++++++++++++ include/dm/root.h | 2 +- include/dm/uclass.h | 6 ++++-- 4 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/include/dm/device.h b/include/dm/device.h index 4cd38ed..b048b66 100644 --- a/include/dm/device.h +++ b/include/dm/device.h @@ -21,7 +21,7 @@ struct driver_info; #define DM_FLAG_ACTIVATED (1 << 0)
/* DM is responsible for allocating and freeing platdata */ -#define DM_FLAG_ALLOC_PDATA (2 << 0) +#define DM_FLAG_ALLOC_PDATA (1 << 1)
/** * struct device - An instance of a driver diff --git a/include/dm/lists.h b/include/dm/lists.h index 0d09f9a..d0b720b 100644 --- a/include/dm/lists.h +++ b/include/dm/lists.h @@ -32,8 +32,28 @@ struct driver *lists_driver_lookup_name(const char *name); */ struct uclass_driver *lists_uclass_lookup(enum uclass_id id);
+/** + * lists_bind_drivers() - search for and bind all drivers to parent + * + * This searches the U_BOOT_DEVICE() structures and creates new devices for + * each one. The devices will have @parent as their parent. + * + * @parent: parent driver (root) + * @early_only: If true, bind only drivers with the DM_INIT_F flag. If false + * bind all drivers. + */ int lists_bind_drivers(struct device *parent);
+/** + * lists_bind_fdt() - bind a device tree node + * + * This creates a new device bound to the given device tree node, with + * @parent as its parent. + * + * @parent: parent driver (root) + * @blob: device tree blob + * @offset: offset of this device tree node + */ int lists_bind_fdt(struct device *parent, const void *blob, int offset);
#endif diff --git a/include/dm/root.h b/include/dm/root.h index 069d058..1631d5d 100644 --- a/include/dm/root.h +++ b/include/dm/root.h @@ -41,7 +41,7 @@ int dm_scan_platdata(void); int dm_scan_fdt(const void *blob);
/** - * dm_init() - Initialize Driver Model structures + * dm_init() - Initialise Driver Model structures * * This function will initialize roots of driver tree and class tree. * This needs to be called before anything uses the DM diff --git a/include/dm/uclass.h b/include/dm/uclass.h index cd23cfe..ac5c147 100644 --- a/include/dm/uclass.h +++ b/include/dm/uclass.h @@ -96,12 +96,14 @@ int uclass_get(enum uclass_id key, struct uclass **ucp); /** * uclass_get_device() - Get a uclass device based on an ID and index * + * The device is probed to activate it ready for use. + * * id: ID to look up * @index: Device number within that uclass (0=first) - * @ucp: Returns pointer to uclass (there is only one per for each ID) + * devp: Returns pointer to device (there is only one per for each ID) * @return 0 if OK, -ve on error */ -int uclass_get_device(enum uclass_id id, int index, struct device **ucp); +int uclass_get_device(enum uclass_id id, int index, struct device **devp);
/** * uclass_first_device() - Get the first device in a uclass
On Saturday, May 24, 2014 at 11:21:05 PM, Simon Glass wrote:
There is a spelling mistake and two functions are missing comments altogether. Also the flags declaration is correct, but doesn't follow style. Finally, the uclass_get_device() function has some errors in its documentation.
Fix these problems.
Signed-off-by: Simon Glass sjg@chromium.org
Acked-by: Marek Vasut marex@denx.de
Best regards, Marek Vasut
Driver model currently only operates after relocation is complete. In this state U-Boot typically has a small amount of memory available. In adding support for driver model prior to relocation we must try to use as little memory as possible.
In addition, on some machines the memory has not be inited and/or the CPU is not running at full speed or the data cache is off. These can reduce execution performance, so the less initialisation that is done before relocation the better.
An immediately-obvious improvement is to only initialise drivers which are actually going to be used before relocation. On many boards the only such driver is a serial UART, so this provides a very large potential benefit.
Allow drivers to mark themselves as 'pre-reloc' which means that they will be initialised prior to relocation. This can be done either with a driver flag or with a 'dm,pre-reloc' device tree property.
To support this, the various dm scanning function now take a 'pre_reloc_only' parameter which indicates that only drivers marked pre-reloc should be bound.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/board_r.c | 4 ++-- doc/driver-model/README.txt | 23 +++++++++++++++------- drivers/core/device.c | 6 ++++-- drivers/core/lists.c | 6 +++--- drivers/core/root.c | 11 +++++++---- include/dm/device-internal.h | 6 ++++-- include/dm/device.h | 5 +++++ include/dm/lists.h | 2 +- include/dm/root.h | 8 ++++++-- test/dm/core.c | 47 ++++++++++++++++++++++++++++++++++---------- test/dm/test-driver.c | 11 +++++++++++ test/dm/test-fdt.c | 20 ++++++++++++++++++- test/dm/test-main.c | 4 ++-- test/dm/test.dts | 11 +++++++++++ 14 files changed, 128 insertions(+), 36 deletions(-)
diff --git a/common/board_r.c b/common/board_r.c index d1f0aa9..d2a59ee 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -276,13 +276,13 @@ static int initr_dm(void) debug("dm_init() failed: %d\n", ret); return ret; } - ret = dm_scan_platdata(); + ret = dm_scan_platdata(false); if (ret) { debug("dm_scan_platdata() failed: %d\n", ret); return ret; } #ifdef CONFIG_OF_CONTROL - ret = dm_scan_fdt(gd->fdt_blob); + ret = dm_scan_fdt(gd->fdt_blob, false); if (ret) { debug("dm_scan_fdt() failed: %d\n", ret); return ret; diff --git a/doc/driver-model/README.txt b/doc/driver-model/README.txt index e0b395a..deacfe9 100644 --- a/doc/driver-model/README.txt +++ b/doc/driver-model/README.txt @@ -332,26 +332,35 @@ dealing with this might not be worth it. - Implemented a GPIO system, trying to keep it simple
+Pre-Relocation Support +---------------------- + +For pre-relocation we simply call the driver model init function. Only +drivers marked with DM_FLAG_PRE_RELOC or the device tree 'dm,pre-reloc' +flag are initialised prior to relocation. This helps to reduce the driver +model overhead. + +Then post relocation we throw that away and re-init driver model again. +For drivers which require some sort of continuity between pre- and +post-relocation devices, we can provide access to the pre-relocation +device pointers, but this is not currently implemented (the root device +pointer is saved but not made available). + + Things to punt for later ------------------------
- SPL support - this will have to be present before many drivers can be converted, but it seems like we can add it once we are happy with the core implementation. -- Pre-relocation support - similar story
-That is not to say that no thinking has gone into these - in fact there +That is not to say that no thinking has gone into this - in fact there is quite a lot there. However, getting these right is non-trivial and there is a high cost associated with going down the wrong path.
For SPL, it may be possible to fit in a simplified driver model with only bind and probe methods, to reduce size.
-For pre-relocation we can simply call the driver model init function. Then -post relocation we throw that away and re-init driver model again. For drivers -which require some sort of continuity between pre- and post-relocation -devices, we can provide access to the pre-relocation device pointers. - Uclasses are statically numbered at compile time. It would be possible to change this to dynamic numbering, but then we would require some sort of lookup service, perhaps searching by name. This is slightly less efficient diff --git a/drivers/core/device.c b/drivers/core/device.c index 55ba281..2c2634e 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -129,14 +129,16 @@ fail_bind: return ret; }
-int device_bind_by_name(struct device *parent, const struct driver_info *info, - struct device **devp) +int device_bind_by_name(struct device *parent, bool pre_reloc_only, + const struct driver_info *info, struct device **devp) { struct driver *drv;
drv = lists_driver_lookup_name(info->name); if (!drv) return -ENOENT; + if (pre_reloc_only && !(drv->flags & DM_FLAG_PRE_RELOC)) + return -EPERM;
return device_bind(parent, drv, info->name, (void *)info->platdata, -1, devp); diff --git a/drivers/core/lists.c b/drivers/core/lists.c index 6a53362..9e38993 100644 --- a/drivers/core/lists.c +++ b/drivers/core/lists.c @@ -61,7 +61,7 @@ struct uclass_driver *lists_uclass_lookup(enum uclass_id id) return NULL; }
-int lists_bind_drivers(struct device *parent) +int lists_bind_drivers(struct device *parent, bool pre_reloc_only) { struct driver_info *info = ll_entry_start(struct driver_info, driver_info); @@ -72,8 +72,8 @@ int lists_bind_drivers(struct device *parent) int ret;
for (entry = info; entry != info + n_ents; entry++) { - ret = device_bind_by_name(parent, entry, &dev); - if (ret) { + ret = device_bind_by_name(parent, pre_reloc_only, entry, &dev); + if (ret && ret != -EPERM) { dm_warn("No match for driver '%s'\n", entry->name); if (!result || ret != -ENOENT) result = ret; diff --git a/drivers/core/root.c b/drivers/core/root.c index 4873e7b..83299f7 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -45,7 +45,7 @@ int dm_init(void) } INIT_LIST_HEAD(&DM_UCLASS_ROOT());
- ret = device_bind_by_name(NULL, &root_info, &DM_ROOT()); + ret = device_bind_by_name(NULL, false, &root_info, &DM_ROOT()); if (ret) return ret; ret = device_probe(DM_ROOT()); @@ -63,11 +63,11 @@ int dm_uninit(void) return 0; }
-int dm_scan_platdata(void) +int dm_scan_platdata(bool pre_reloc_only) { int ret;
- ret = lists_bind_drivers(DM_ROOT()); + ret = lists_bind_drivers(DM_ROOT(), pre_reloc_only); if (ret == -ENOENT) { dm_warn("Some drivers were not found\n"); ret = 0; @@ -79,7 +79,7 @@ int dm_scan_platdata(void) }
#ifdef CONFIG_OF_CONTROL -int dm_scan_fdt(const void *blob) +int dm_scan_fdt(const void *blob, bool pre_reloc_only) { int offset = 0; int ret = 0, err; @@ -88,6 +88,9 @@ int dm_scan_fdt(const void *blob) do { offset = fdt_next_node(blob, offset, &depth); if (offset > 0 && depth == 1) { + if (pre_reloc_only && + !fdt_getprop(blob, offset, "dm,pre-reloc", NULL)) + continue; err = lists_bind_fdt(gd->dm_root, blob, offset); if (err && !ret) ret = err; diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h index e95b7a9..fde67bb 100644 --- a/include/dm/device-internal.h +++ b/include/dm/device-internal.h @@ -45,12 +45,14 @@ int device_bind(struct device *parent, struct driver *drv, * tree. * * @parent: Pointer to device's parent + * @pre_reloc_only: If true, bind the driver only if its DM_INIT_F flag is set. + * If false bind the driver always. * @info: Name and platdata for this device * @devp: Returns a pointer to the bound device * @return 0 if OK, -ve on error */ -int device_bind_by_name(struct device *parent, const struct driver_info *info, - struct device **devp); +int device_bind_by_name(struct device *parent, bool pre_reloc_only, + const struct driver_info *info, struct device **devp);
/** * device_probe() - Probe a device, activating it diff --git a/include/dm/device.h b/include/dm/device.h index b048b66..32650fd 100644 --- a/include/dm/device.h +++ b/include/dm/device.h @@ -23,6 +23,9 @@ struct driver_info; /* DM is responsible for allocating and freeing platdata */ #define DM_FLAG_ALLOC_PDATA (1 << 1)
+/* DM should init this device prior to relocation */ +#define DM_FLAG_PRE_RELOC (1 << 2) + /** * struct device - An instance of a driver * @@ -117,6 +120,7 @@ struct device_id { * ops: Driver-specific operations. This is typically a list of function * pointers defined by the driver, to implement driver functions required by * the uclass. + * @flags: driver flags - see DM_FLAGS_... */ struct driver { char *name; @@ -130,6 +134,7 @@ struct driver { int priv_auto_alloc_size; int platdata_auto_alloc_size; const void *ops; /* driver-specific operations */ + uint32_t flags; };
/* Declare a new U-Boot driver */ diff --git a/include/dm/lists.h b/include/dm/lists.h index d0b720b..13e025e 100644 --- a/include/dm/lists.h +++ b/include/dm/lists.h @@ -42,7 +42,7 @@ struct uclass_driver *lists_uclass_lookup(enum uclass_id id); * @early_only: If true, bind only drivers with the DM_INIT_F flag. If false * bind all drivers. */ -int lists_bind_drivers(struct device *parent); +int lists_bind_drivers(struct device *parent, bool pre_reloc_only);
/** * lists_bind_fdt() - bind a device tree node diff --git a/include/dm/root.h b/include/dm/root.h index 1631d5d..30ebe6c 100644 --- a/include/dm/root.h +++ b/include/dm/root.h @@ -26,9 +26,11 @@ struct device *dm_root(void); * * This scans all available platdata and creates drivers for each * + * @pre_reloc_only: If true, bind only drivers with the DM_FLAG_PRE_RELOC + * flag. If false bind all drivers. * @return 0 if OK, -ve on error */ -int dm_scan_platdata(void); +int dm_scan_platdata(bool pre_reloc_only);
/** * dm_scan_fdt() - Scan the device tree and bind drivers @@ -36,9 +38,11 @@ int dm_scan_platdata(void); * This scans the device tree and creates a driver for each node * * @blob: Pointer to device tree blob + * @pre_reloc_only: If true, bind only drivers with the DM_FLAG_PRE_RELOC + * flag. If false bind all drivers. * @return 0 if OK, -ve on error */ -int dm_scan_fdt(const void *blob); +int dm_scan_fdt(const void *blob, bool pre_reloc_only);
/** * dm_init() - Initialise Driver Model structures diff --git a/test/dm/core.c b/test/dm/core.c index a889fad..63bb561 100644 --- a/test/dm/core.c +++ b/test/dm/core.c @@ -25,6 +25,7 @@ enum { TEST_INTVAL2 = 3, TEST_INTVAL3 = 6, TEST_INTVAL_MANUAL = 101112, + TEST_INTVAL_PRE_RELOC = 7, };
static const struct dm_test_pdata test_pdata[] = { @@ -37,6 +38,10 @@ static const struct dm_test_pdata test_pdata_manual = { .ping_add = TEST_INTVAL_MANUAL, };
+static const struct dm_test_pdata test_pdata_pre_reloc = { + .ping_add = TEST_INTVAL_PRE_RELOC, +}; + U_BOOT_DEVICE(dm_test_info1) = { .name = "test_drv", .platdata = &test_pdata[0], @@ -57,6 +62,11 @@ static struct driver_info driver_info_manual = { .platdata = &test_pdata_manual, };
+static struct driver_info driver_info_pre_reloc = { + .name = "test_pre_reloc_drv", + .platdata = &test_pdata_manual, +}; + /* Test that binding with platdata occurs correctly */ static int dm_test_autobind(struct dm_test_state *dms) { @@ -71,7 +81,7 @@ static int dm_test_autobind(struct dm_test_state *dms) ut_asserteq(0, list_count_items(&gd->dm_root->child_head)); ut_asserteq(0, dm_testdrv_op_count[DM_TEST_OP_POST_BIND]);
- ut_assertok(dm_scan_platdata()); + ut_assertok(dm_scan_platdata(false));
/* We should have our test class now at least, plus more children */ ut_assert(1 < list_count_items(&gd->uclass_root)); @@ -181,7 +191,7 @@ static int dm_test_lifecycle(struct dm_test_state *dms)
memcpy(op_count, dm_testdrv_op_count, sizeof(op_count));
- ut_assertok(device_bind_by_name(dms->root, &driver_info_manual, + ut_assertok(device_bind_by_name(dms->root, false, &driver_info_manual, &dev)); ut_assert(dev); ut_assert(dm_testdrv_op_count[DM_TEST_OP_BIND] @@ -232,15 +242,15 @@ static int dm_test_ordering(struct dm_test_state *dms) struct device *dev, *dev_penultimate, *dev_last, *test_dev; int pingret;
- ut_assertok(device_bind_by_name(dms->root, &driver_info_manual, + ut_assertok(device_bind_by_name(dms->root, false, &driver_info_manual, &dev)); ut_assert(dev);
/* Bind two new devices (numbers 4 and 5) */ - ut_assertok(device_bind_by_name(dms->root, &driver_info_manual, + ut_assertok(device_bind_by_name(dms->root, false, &driver_info_manual, &dev_penultimate)); ut_assert(dev_penultimate); - ut_assertok(device_bind_by_name(dms->root, &driver_info_manual, + ut_assertok(device_bind_by_name(dms->root, false, &driver_info_manual, &dev_last)); ut_assert(dev_last);
@@ -255,7 +265,8 @@ static int dm_test_ordering(struct dm_test_state *dms) ut_assert(dev_last == test_dev);
/* Add back the original device 3, now in position 5 */ - ut_assertok(device_bind_by_name(dms->root, &driver_info_manual, &dev)); + ut_assertok(device_bind_by_name(dms->root, false, &driver_info_manual, + &dev)); ut_assert(dev);
/* Try ping */ @@ -375,8 +386,8 @@ static int dm_test_leak(struct dm_test_state *dms) if (!start.uordblks) puts("Warning: Please add '#define DEBUG' to the top of common/dlmalloc.c\n");
- ut_assertok(dm_scan_platdata()); - ut_assertok(dm_scan_fdt(gd->fdt_blob)); + ut_assertok(dm_scan_platdata(false)); + ut_assertok(dm_scan_fdt(gd->fdt_blob, false));
/* Scanning the uclass is enough to probe all the devices */ for (id = UCLASS_ROOT; id < UCLASS_COUNT; id++) { @@ -444,8 +455,8 @@ static int create_children(struct dm_test_state *dms, struct device *parent, for (i = 0; i < count; i++) { struct dm_test_pdata *pdata;
- ut_assertok(device_bind_by_name(parent, &driver_info_manual, - &dev)); + ut_assertok(device_bind_by_name(parent, false, + &driver_info_manual, &dev)); pdata = calloc(1, sizeof(*pdata)); pdata->ping_add = key + i; dev->platdata = pdata; @@ -542,3 +553,19 @@ static int dm_test_children(struct dm_test_state *dms) return 0; } DM_TEST(dm_test_children, 0); + +static int dm_test_pre_reloc(struct dm_test_state *dms) +{ + struct device *dev; + + /* The normal driver should refuse to bind before relocation */ + ut_asserteq(-EPERM, device_bind_by_name(dms->root, true, + &driver_info_manual, &dev)); + + /* But this one is marked pre-reloc */ + ut_assertok(device_bind_by_name(dms->root, true, + &driver_info_pre_reloc, &dev)); + + return 0; +} +DM_TEST(dm_test_pre_reloc, 0); diff --git a/test/dm/test-driver.c b/test/dm/test-driver.c index c4be8a1..e7fb199 100644 --- a/test/dm/test-driver.c +++ b/test/dm/test-driver.c @@ -144,3 +144,14 @@ U_BOOT_DRIVER(test_manual_drv) = { .remove = test_manual_remove, .unbind = test_manual_unbind, }; + +U_BOOT_DRIVER(test_pre_reloc_drv) = { + .name = "test_pre_reloc_drv", + .id = UCLASS_TEST, + .ops = &test_manual_ops, + .bind = test_manual_bind, + .probe = test_manual_probe, + .remove = test_manual_remove, + .unbind = test_manual_unbind, + .flags = DM_FLAG_PRE_RELOC, +}; diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c index e1d982f..d6f5bb8 100644 --- a/test/dm/test-fdt.c +++ b/test/dm/test-fdt.c @@ -99,7 +99,7 @@ static int dm_test_fdt(struct dm_test_state *dms) int ret; int i;
- ret = dm_scan_fdt(gd->fdt_blob); + ret = dm_scan_fdt(gd->fdt_blob, false); ut_assert(!ret);
ret = uclass_get(UCLASS_TEST_FDT, &uc); @@ -142,3 +142,21 @@ static int dm_test_fdt(struct dm_test_state *dms) return 0; } DM_TEST(dm_test_fdt, 0); + +static int dm_test_fdt_pre_reloc(struct dm_test_state *dms) +{ + struct uclass *uc; + int ret; + + ret = dm_scan_fdt(gd->fdt_blob, true); + ut_assert(!ret); + + ret = uclass_get(UCLASS_TEST_FDT, &uc); + ut_assert(!ret); + + /* These is only one pre-reloc device */ + ut_asserteq(1, list_count_items(&uc->dev_head)); + + return 0; +} +DM_TEST(dm_test_fdt_pre_reloc, 0); diff --git a/test/dm/test-main.c b/test/dm/test-main.c index 828ed46..49805eb 100644 --- a/test/dm/test-main.c +++ b/test/dm/test-main.c @@ -89,11 +89,11 @@ int dm_test_main(void) ut_assertok(dm_test_init(dms));
if (test->flags & DM_TESTF_SCAN_PDATA) - ut_assertok(dm_scan_platdata()); + ut_assertok(dm_scan_platdata(false)); if (test->flags & DM_TESTF_PROBE_TEST) ut_assertok(do_autoprobe(dms)); if (test->flags & DM_TESTF_SCAN_FDT) - ut_assertok(dm_scan_fdt(gd->fdt_blob)); + ut_assertok(dm_scan_fdt(gd->fdt_blob, false));
if (test->func(dms)) break; diff --git a/test/dm/test.dts b/test/dm/test.dts index ec5364f..b2eaddd 100644 --- a/test/dm/test.dts +++ b/test/dm/test.dts @@ -6,10 +6,21 @@ #address-cells = <1>; #size-cells = <0>;
+ alias { + console = <&uart0>; + }; + + uart0: serial { + compatible = "sandbox,serial"; + dm,pre-reloc; + //text-colour = "cyan"; + }; + a-test { reg = <0>; compatible = "denx,u-boot-fdt-test"; ping-add = <0>; + dm,pre-reloc; };
junk {
Allow drivers to mark themselves as 'pre-reloc' which means that they will be initialised prior to relocation. This can be done either with a driver flag or with a 'dm,pre-reloc' device tree property.
Hmmm, "dm,pre-reloc" isn't really describing the hardware any more. That's really a flag for the purpose of telling SW how it should work. We really should be careful here to maintain the DTS content as a description of the hardware.
HTH, jdl
On Sat, May 24, 2014 at 4:21 PM, Simon Glass sjg@chromium.org wrote:
Driver model currently only operates after relocation is complete. In this state U-Boot typically has a small amount of memory available. In adding support for driver model prior to relocation we must try to use as little memory as possible.
In addition, on some machines the memory has not be inited and/or the CPU is not running at full speed or the data cache is off. These can reduce execution performance, so the less initialisation that is done before relocation the better.
An immediately-obvious improvement is to only initialise drivers which are actually going to be used before relocation. On many boards the only such driver is a serial UART, so this provides a very large potential benefit.
Allow drivers to mark themselves as 'pre-reloc' which means that they will be initialised prior to relocation. This can be done either with a driver flag or with a 'dm,pre-reloc' device tree property.
To support this, the various dm scanning function now take a 'pre_reloc_only' parameter which indicates that only drivers marked pre-reloc should be bound.
Signed-off-by: Simon Glass sjg@chromium.org
common/board_r.c | 4 ++-- doc/driver-model/README.txt | 23 +++++++++++++++------- drivers/core/device.c | 6 ++++-- drivers/core/lists.c | 6 +++--- drivers/core/root.c | 11 +++++++---- include/dm/device-internal.h | 6 ++++-- include/dm/device.h | 5 +++++ include/dm/lists.h | 2 +- include/dm/root.h | 8 ++++++-- test/dm/core.c | 47 ++++++++++++++++++++++++++++++++++---------- test/dm/test-driver.c | 11 +++++++++++ test/dm/test-fdt.c | 20 ++++++++++++++++++- test/dm/test-main.c | 4 ++-- test/dm/test.dts | 11 +++++++++++ 14 files changed, 128 insertions(+), 36 deletions(-)
diff --git a/common/board_r.c b/common/board_r.c index d1f0aa9..d2a59ee 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -276,13 +276,13 @@ static int initr_dm(void) debug("dm_init() failed: %d\n", ret); return ret; }
ret = dm_scan_platdata();
ret = dm_scan_platdata(false); if (ret) { debug("dm_scan_platdata() failed: %d\n", ret); return ret; }#ifdef CONFIG_OF_CONTROL
ret = dm_scan_fdt(gd->fdt_blob);
ret = dm_scan_fdt(gd->fdt_blob, false); if (ret) { debug("dm_scan_fdt() failed: %d\n", ret); return ret;diff --git a/doc/driver-model/README.txt b/doc/driver-model/README.txt index e0b395a..deacfe9 100644 --- a/doc/driver-model/README.txt +++ b/doc/driver-model/README.txt @@ -332,26 +332,35 @@ dealing with this might not be worth it.
- Implemented a GPIO system, trying to keep it simple
+Pre-Relocation Support +----------------------
+For pre-relocation we simply call the driver model init function. Only +drivers marked with DM_FLAG_PRE_RELOC or the device tree 'dm,pre-reloc' +flag are initialised prior to relocation. This helps to reduce the driver +model overhead.
+Then post relocation we throw that away and re-init driver model again. +For drivers which require some sort of continuity between pre- and +post-relocation devices, we can provide access to the pre-relocation +device pointers, but this is not currently implemented (the root device +pointer is saved but not made available).
Things to punt for later
- SPL support - this will have to be present before many drivers can be
converted, but it seems like we can add it once we are happy with the core implementation. -- Pre-relocation support - similar story
-That is not to say that no thinking has gone into these - in fact there +That is not to say that no thinking has gone into this - in fact there is quite a lot there. However, getting these right is non-trivial and there is a high cost associated with going down the wrong path.
For SPL, it may be possible to fit in a simplified driver model with only bind and probe methods, to reduce size.
-For pre-relocation we can simply call the driver model init function. Then -post relocation we throw that away and re-init driver model again. For drivers -which require some sort of continuity between pre- and post-relocation -devices, we can provide access to the pre-relocation device pointers.
Uclasses are statically numbered at compile time. It would be possible to change this to dynamic numbering, but then we would require some sort of lookup service, perhaps searching by name. This is slightly less efficient diff --git a/drivers/core/device.c b/drivers/core/device.c index 55ba281..2c2634e 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -129,14 +129,16 @@ fail_bind: return ret; }
-int device_bind_by_name(struct device *parent, const struct driver_info *info,
struct device **devp)+int device_bind_by_name(struct device *parent, bool pre_reloc_only,
const struct driver_info *info, struct device **devp){ struct driver *drv;
drv = lists_driver_lookup_name(info->name); if (!drv) return -ENOENT;
if (pre_reloc_only && !(drv->flags & DM_FLAG_PRE_RELOC))return -EPERM; return device_bind(parent, drv, info->name, (void *)info->platdata, -1, devp);diff --git a/drivers/core/lists.c b/drivers/core/lists.c index 6a53362..9e38993 100644 --- a/drivers/core/lists.c +++ b/drivers/core/lists.c @@ -61,7 +61,7 @@ struct uclass_driver *lists_uclass_lookup(enum uclass_id id) return NULL; }
-int lists_bind_drivers(struct device *parent) +int lists_bind_drivers(struct device *parent, bool pre_reloc_only) { struct driver_info *info = ll_entry_start(struct driver_info, driver_info); @@ -72,8 +72,8 @@ int lists_bind_drivers(struct device *parent) int ret;
for (entry = info; entry != info + n_ents; entry++) {
ret = device_bind_by_name(parent, entry, &dev);if (ret) {
ret = device_bind_by_name(parent, pre_reloc_only, entry, &dev);if (ret && ret != -EPERM) { dm_warn("No match for driver '%s'\n", entry->name); if (!result || ret != -ENOENT) result = ret;diff --git a/drivers/core/root.c b/drivers/core/root.c index 4873e7b..83299f7 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -45,7 +45,7 @@ int dm_init(void) } INIT_LIST_HEAD(&DM_UCLASS_ROOT());
ret = device_bind_by_name(NULL, &root_info, &DM_ROOT());
ret = device_bind_by_name(NULL, false, &root_info, &DM_ROOT()); if (ret) return ret; ret = device_probe(DM_ROOT());@@ -63,11 +63,11 @@ int dm_uninit(void) return 0; }
-int dm_scan_platdata(void) +int dm_scan_platdata(bool pre_reloc_only) { int ret;
ret = lists_bind_drivers(DM_ROOT());
ret = lists_bind_drivers(DM_ROOT(), pre_reloc_only); if (ret == -ENOENT) { dm_warn("Some drivers were not found\n"); ret = 0;@@ -79,7 +79,7 @@ int dm_scan_platdata(void) }
#ifdef CONFIG_OF_CONTROL -int dm_scan_fdt(const void *blob) +int dm_scan_fdt(const void *blob, bool pre_reloc_only) { int offset = 0; int ret = 0, err; @@ -88,6 +88,9 @@ int dm_scan_fdt(const void *blob) do { offset = fdt_next_node(blob, offset, &depth); if (offset > 0 && depth == 1) {
if (pre_reloc_only &&!fdt_getprop(blob, offset, "dm,pre-reloc", NULL))continue; err = lists_bind_fdt(gd->dm_root, blob, offset); if (err && !ret) ret = err;diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h index e95b7a9..fde67bb 100644 --- a/include/dm/device-internal.h +++ b/include/dm/device-internal.h @@ -45,12 +45,14 @@ int device_bind(struct device *parent, struct driver *drv,
- tree.
- @parent: Pointer to device's parent
- @pre_reloc_only: If true, bind the driver only if its DM_INIT_F flag is set.
*/
- If false bind the driver always.
- @info: Name and platdata for this device
- @devp: Returns a pointer to the bound device
- @return 0 if OK, -ve on error
-int device_bind_by_name(struct device *parent, const struct driver_info *info,
struct device **devp);+int device_bind_by_name(struct device *parent, bool pre_reloc_only,
const struct driver_info *info, struct device **devp);/**
- device_probe() - Probe a device, activating it
diff --git a/include/dm/device.h b/include/dm/device.h index b048b66..32650fd 100644 --- a/include/dm/device.h +++ b/include/dm/device.h @@ -23,6 +23,9 @@ struct driver_info; /* DM is responsible for allocating and freeing platdata */ #define DM_FLAG_ALLOC_PDATA (1 << 1)
+/* DM should init this device prior to relocation */ +#define DM_FLAG_PRE_RELOC (1 << 2)
/**
- struct device - An instance of a driver
@@ -117,6 +120,7 @@ struct device_id {
- ops: Driver-specific operations. This is typically a list of function
- pointers defined by the driver, to implement driver functions required by
- the uclass.
*/
- @flags: driver flags - see DM_FLAGS_...
struct driver { char *name; @@ -130,6 +134,7 @@ struct driver { int priv_auto_alloc_size; int platdata_auto_alloc_size; const void *ops; /* driver-specific operations */
uint32_t flags;};
/* Declare a new U-Boot driver */ diff --git a/include/dm/lists.h b/include/dm/lists.h index d0b720b..13e025e 100644 --- a/include/dm/lists.h +++ b/include/dm/lists.h @@ -42,7 +42,7 @@ struct uclass_driver *lists_uclass_lookup(enum uclass_id id);
- @early_only: If true, bind only drivers with the DM_INIT_F flag. If false
- bind all drivers.
*/ -int lists_bind_drivers(struct device *parent); +int lists_bind_drivers(struct device *parent, bool pre_reloc_only);
/**
- lists_bind_fdt() - bind a device tree node
diff --git a/include/dm/root.h b/include/dm/root.h index 1631d5d..30ebe6c 100644 --- a/include/dm/root.h +++ b/include/dm/root.h @@ -26,9 +26,11 @@ struct device *dm_root(void);
- This scans all available platdata and creates drivers for each
- @pre_reloc_only: If true, bind only drivers with the DM_FLAG_PRE_RELOC
*/
- flag. If false bind all drivers.
- @return 0 if OK, -ve on error
-int dm_scan_platdata(void); +int dm_scan_platdata(bool pre_reloc_only);
/**
- dm_scan_fdt() - Scan the device tree and bind drivers
@@ -36,9 +38,11 @@ int dm_scan_platdata(void);
- This scans the device tree and creates a driver for each node
- @blob: Pointer to device tree blob
- @pre_reloc_only: If true, bind only drivers with the DM_FLAG_PRE_RELOC
*/
- flag. If false bind all drivers.
- @return 0 if OK, -ve on error
-int dm_scan_fdt(const void *blob); +int dm_scan_fdt(const void *blob, bool pre_reloc_only);
/**
- dm_init() - Initialise Driver Model structures
diff --git a/test/dm/core.c b/test/dm/core.c index a889fad..63bb561 100644 --- a/test/dm/core.c +++ b/test/dm/core.c @@ -25,6 +25,7 @@ enum { TEST_INTVAL2 = 3, TEST_INTVAL3 = 6, TEST_INTVAL_MANUAL = 101112,
TEST_INTVAL_PRE_RELOC = 7,};
static const struct dm_test_pdata test_pdata[] = { @@ -37,6 +38,10 @@ static const struct dm_test_pdata test_pdata_manual = { .ping_add = TEST_INTVAL_MANUAL, };
+static const struct dm_test_pdata test_pdata_pre_reloc = {
.ping_add = TEST_INTVAL_PRE_RELOC,+};
U_BOOT_DEVICE(dm_test_info1) = { .name = "test_drv", .platdata = &test_pdata[0], @@ -57,6 +62,11 @@ static struct driver_info driver_info_manual = { .platdata = &test_pdata_manual, };
+static struct driver_info driver_info_pre_reloc = {
.name = "test_pre_reloc_drv",.platdata = &test_pdata_manual,+};
/* Test that binding with platdata occurs correctly */ static int dm_test_autobind(struct dm_test_state *dms) { @@ -71,7 +81,7 @@ static int dm_test_autobind(struct dm_test_state *dms) ut_asserteq(0, list_count_items(&gd->dm_root->child_head)); ut_asserteq(0, dm_testdrv_op_count[DM_TEST_OP_POST_BIND]);
ut_assertok(dm_scan_platdata());
ut_assertok(dm_scan_platdata(false)); /* We should have our test class now at least, plus more children */ ut_assert(1 < list_count_items(&gd->uclass_root));@@ -181,7 +191,7 @@ static int dm_test_lifecycle(struct dm_test_state *dms)
memcpy(op_count, dm_testdrv_op_count, sizeof(op_count));
ut_assertok(device_bind_by_name(dms->root, &driver_info_manual,
ut_assertok(device_bind_by_name(dms->root, false, &driver_info_manual, &dev)); ut_assert(dev); ut_assert(dm_testdrv_op_count[DM_TEST_OP_BIND]@@ -232,15 +242,15 @@ static int dm_test_ordering(struct dm_test_state *dms) struct device *dev, *dev_penultimate, *dev_last, *test_dev; int pingret;
ut_assertok(device_bind_by_name(dms->root, &driver_info_manual,
ut_assertok(device_bind_by_name(dms->root, false, &driver_info_manual, &dev)); ut_assert(dev); /* Bind two new devices (numbers 4 and 5) */
ut_assertok(device_bind_by_name(dms->root, &driver_info_manual,
ut_assertok(device_bind_by_name(dms->root, false, &driver_info_manual, &dev_penultimate)); ut_assert(dev_penultimate);
ut_assertok(device_bind_by_name(dms->root, &driver_info_manual,
ut_assertok(device_bind_by_name(dms->root, false, &driver_info_manual, &dev_last)); ut_assert(dev_last);@@ -255,7 +265,8 @@ static int dm_test_ordering(struct dm_test_state *dms) ut_assert(dev_last == test_dev);
/* Add back the original device 3, now in position 5 */
ut_assertok(device_bind_by_name(dms->root, &driver_info_manual, &dev));
ut_assertok(device_bind_by_name(dms->root, false, &driver_info_manual,&dev)); ut_assert(dev); /* Try ping */@@ -375,8 +386,8 @@ static int dm_test_leak(struct dm_test_state *dms) if (!start.uordblks) puts("Warning: Please add '#define DEBUG' to the top of common/dlmalloc.c\n");
ut_assertok(dm_scan_platdata());ut_assertok(dm_scan_fdt(gd->fdt_blob));
ut_assertok(dm_scan_platdata(false));ut_assertok(dm_scan_fdt(gd->fdt_blob, false)); /* Scanning the uclass is enough to probe all the devices */ for (id = UCLASS_ROOT; id < UCLASS_COUNT; id++) {@@ -444,8 +455,8 @@ static int create_children(struct dm_test_state *dms, struct device *parent, for (i = 0; i < count; i++) { struct dm_test_pdata *pdata;
ut_assertok(device_bind_by_name(parent, &driver_info_manual,&dev));
ut_assertok(device_bind_by_name(parent, false,&driver_info_manual, &dev)); pdata = calloc(1, sizeof(*pdata)); pdata->ping_add = key + i; dev->platdata = pdata;@@ -542,3 +553,19 @@ static int dm_test_children(struct dm_test_state *dms) return 0; } DM_TEST(dm_test_children, 0);
+static int dm_test_pre_reloc(struct dm_test_state *dms) +{
struct device *dev;/* The normal driver should refuse to bind before relocation */ut_asserteq(-EPERM, device_bind_by_name(dms->root, true,&driver_info_manual, &dev));/* But this one is marked pre-reloc */ut_assertok(device_bind_by_name(dms->root, true,&driver_info_pre_reloc, &dev));return 0;+} +DM_TEST(dm_test_pre_reloc, 0); diff --git a/test/dm/test-driver.c b/test/dm/test-driver.c index c4be8a1..e7fb199 100644 --- a/test/dm/test-driver.c +++ b/test/dm/test-driver.c @@ -144,3 +144,14 @@ U_BOOT_DRIVER(test_manual_drv) = { .remove = test_manual_remove, .unbind = test_manual_unbind, };
+U_BOOT_DRIVER(test_pre_reloc_drv) = {
.name = "test_pre_reloc_drv",.id = UCLASS_TEST,.ops = &test_manual_ops,.bind = test_manual_bind,.probe = test_manual_probe,.remove = test_manual_remove,.unbind = test_manual_unbind,.flags = DM_FLAG_PRE_RELOC,+}; diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c index e1d982f..d6f5bb8 100644 --- a/test/dm/test-fdt.c +++ b/test/dm/test-fdt.c @@ -99,7 +99,7 @@ static int dm_test_fdt(struct dm_test_state *dms) int ret; int i;
ret = dm_scan_fdt(gd->fdt_blob);
ret = dm_scan_fdt(gd->fdt_blob, false); ut_assert(!ret); ret = uclass_get(UCLASS_TEST_FDT, &uc);@@ -142,3 +142,21 @@ static int dm_test_fdt(struct dm_test_state *dms) return 0; } DM_TEST(dm_test_fdt, 0);
+static int dm_test_fdt_pre_reloc(struct dm_test_state *dms) +{
struct uclass *uc;int ret;ret = dm_scan_fdt(gd->fdt_blob, true);ut_assert(!ret);ret = uclass_get(UCLASS_TEST_FDT, &uc);ut_assert(!ret);/* These is only one pre-reloc device */ut_asserteq(1, list_count_items(&uc->dev_head));return 0;+} +DM_TEST(dm_test_fdt_pre_reloc, 0); diff --git a/test/dm/test-main.c b/test/dm/test-main.c index 828ed46..49805eb 100644 --- a/test/dm/test-main.c +++ b/test/dm/test-main.c @@ -89,11 +89,11 @@ int dm_test_main(void) ut_assertok(dm_test_init(dms));
if (test->flags & DM_TESTF_SCAN_PDATA)
ut_assertok(dm_scan_platdata());
ut_assertok(dm_scan_platdata(false)); if (test->flags & DM_TESTF_PROBE_TEST) ut_assertok(do_autoprobe(dms)); if (test->flags & DM_TESTF_SCAN_FDT)
ut_assertok(dm_scan_fdt(gd->fdt_blob));
ut_assertok(dm_scan_fdt(gd->fdt_blob, false)); if (test->func(dms)) break;diff --git a/test/dm/test.dts b/test/dm/test.dts index ec5364f..b2eaddd 100644 --- a/test/dm/test.dts +++ b/test/dm/test.dts @@ -6,10 +6,21 @@ #address-cells = <1>; #size-cells = <0>;
alias {console = <&uart0>;};uart0: serial {compatible = "sandbox,serial";dm,pre-reloc;//text-colour = "cyan";};a-test { reg = <0>; compatible = "denx,u-boot-fdt-test"; ping-add = <0>;dm,pre-reloc; }; junk {-- 1.9.1.423.g4596e3a
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hi Jon,
On 27 May 2014 09:36, Jon Loeliger loeliger@gmail.com wrote:
Allow drivers to mark themselves as 'pre-reloc' which means that they will be initialised prior to relocation. This can be done either with a driver flag or with a 'dm,pre-reloc' device tree property.
Hmmm, "dm,pre-reloc" isn't really describing the hardware any more. That's really a flag for the purpose of telling SW how it should work. We really should be careful here to maintain the DTS content as a description of the hardware.
Yes this always comes up. Another approach would be to declare that a serial driver must be available and just use the console alias and assume that the device must be pre-reloc. But I worry that with I2C, SPI , etc. potentially wanting pre-reloc init this might be a losing battle.
This way, board authors have complete flexibility, and the file is in after all the U-Boot source tree.
Regards, Simon
HTH, jdl
On Sat, May 24, 2014 at 4:21 PM, Simon Glass sjg@chromium.org wrote:
Driver model currently only operates after relocation is complete. In this state U-Boot typically has a small amount of memory available. In adding support for driver model prior to relocation we must try to use as little memory as possible.
In addition, on some machines the memory has not be inited and/or the CPU is not running at full speed or the data cache is off. These can reduce execution performance, so the less initialisation that is done before relocation the better.
An immediately-obvious improvement is to only initialise drivers which are actually going to be used before relocation. On many boards the only such driver is a serial UART, so this provides a very large potential benefit.
Allow drivers to mark themselves as 'pre-reloc' which means that they will be initialised prior to relocation. This can be done either with a driver flag or with a 'dm,pre-reloc' device tree property.
To support this, the various dm scanning function now take a 'pre_reloc_only' parameter which indicates that only drivers marked pre-reloc should be bound.
Signed-off-by: Simon Glass sjg@chromium.org
common/board_r.c | 4 ++-- doc/driver-model/README.txt | 23 +++++++++++++++------- drivers/core/device.c | 6 ++++-- drivers/core/lists.c | 6 +++--- drivers/core/root.c | 11 +++++++---- include/dm/device-internal.h | 6 ++++-- include/dm/device.h | 5 +++++ include/dm/lists.h | 2 +- include/dm/root.h | 8 ++++++-- test/dm/core.c | 47 ++++++++++++++++++++++++++++++++++---------- test/dm/test-driver.c | 11 +++++++++++ test/dm/test-fdt.c | 20 ++++++++++++++++++- test/dm/test-main.c | 4 ++-- test/dm/test.dts | 11 +++++++++++ 14 files changed, 128 insertions(+), 36 deletions(-)
diff --git a/common/board_r.c b/common/board_r.c index d1f0aa9..d2a59ee 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -276,13 +276,13 @@ static int initr_dm(void) debug("dm_init() failed: %d\n", ret); return ret; }
ret = dm_scan_platdata();
ret = dm_scan_platdata(false); if (ret) { debug("dm_scan_platdata() failed: %d\n", ret); return ret; }#ifdef CONFIG_OF_CONTROL
ret = dm_scan_fdt(gd->fdt_blob);
ret = dm_scan_fdt(gd->fdt_blob, false); if (ret) { debug("dm_scan_fdt() failed: %d\n", ret); return ret;diff --git a/doc/driver-model/README.txt b/doc/driver-model/README.txt index e0b395a..deacfe9 100644 --- a/doc/driver-model/README.txt +++ b/doc/driver-model/README.txt @@ -332,26 +332,35 @@ dealing with this might not be worth it.
- Implemented a GPIO system, trying to keep it simple
+Pre-Relocation Support +----------------------
+For pre-relocation we simply call the driver model init function. Only +drivers marked with DM_FLAG_PRE_RELOC or the device tree 'dm,pre-reloc' +flag are initialised prior to relocation. This helps to reduce the driver +model overhead.
+Then post relocation we throw that away and re-init driver model again. +For drivers which require some sort of continuity between pre- and +post-relocation devices, we can provide access to the pre-relocation +device pointers, but this is not currently implemented (the root device +pointer is saved but not made available).
Things to punt for later
- SPL support - this will have to be present before many drivers can be
converted, but it seems like we can add it once we are happy with the core implementation. -- Pre-relocation support - similar story
-That is not to say that no thinking has gone into these - in fact there +That is not to say that no thinking has gone into this - in fact there is quite a lot there. However, getting these right is non-trivial and there is a high cost associated with going down the wrong path.
For SPL, it may be possible to fit in a simplified driver model with only bind and probe methods, to reduce size.
-For pre-relocation we can simply call the driver model init function. Then -post relocation we throw that away and re-init driver model again. For drivers -which require some sort of continuity between pre- and post-relocation -devices, we can provide access to the pre-relocation device pointers.
Uclasses are statically numbered at compile time. It would be possible to change this to dynamic numbering, but then we would require some sort of lookup service, perhaps searching by name. This is slightly less efficient diff --git a/drivers/core/device.c b/drivers/core/device.c index 55ba281..2c2634e 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -129,14 +129,16 @@ fail_bind: return ret; }
-int device_bind_by_name(struct device *parent, const struct driver_info *info,
struct device **devp)+int device_bind_by_name(struct device *parent, bool pre_reloc_only,
const struct driver_info *info, struct device **devp){ struct driver *drv;
drv = lists_driver_lookup_name(info->name); if (!drv) return -ENOENT;
if (pre_reloc_only && !(drv->flags & DM_FLAG_PRE_RELOC))return -EPERM; return device_bind(parent, drv, info->name, (void *)info->platdata, -1, devp);diff --git a/drivers/core/lists.c b/drivers/core/lists.c index 6a53362..9e38993 100644 --- a/drivers/core/lists.c +++ b/drivers/core/lists.c @@ -61,7 +61,7 @@ struct uclass_driver *lists_uclass_lookup(enum uclass_id id) return NULL; }
-int lists_bind_drivers(struct device *parent) +int lists_bind_drivers(struct device *parent, bool pre_reloc_only) { struct driver_info *info = ll_entry_start(struct driver_info, driver_info); @@ -72,8 +72,8 @@ int lists_bind_drivers(struct device *parent) int ret;
for (entry = info; entry != info + n_ents; entry++) {
ret = device_bind_by_name(parent, entry, &dev);if (ret) {
ret = device_bind_by_name(parent, pre_reloc_only, entry, &dev);if (ret && ret != -EPERM) { dm_warn("No match for driver '%s'\n", entry->name); if (!result || ret != -ENOENT) result = ret;diff --git a/drivers/core/root.c b/drivers/core/root.c index 4873e7b..83299f7 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -45,7 +45,7 @@ int dm_init(void) } INIT_LIST_HEAD(&DM_UCLASS_ROOT());
ret = device_bind_by_name(NULL, &root_info, &DM_ROOT());
ret = device_bind_by_name(NULL, false, &root_info, &DM_ROOT()); if (ret) return ret; ret = device_probe(DM_ROOT());@@ -63,11 +63,11 @@ int dm_uninit(void) return 0; }
-int dm_scan_platdata(void) +int dm_scan_platdata(bool pre_reloc_only) { int ret;
ret = lists_bind_drivers(DM_ROOT());
ret = lists_bind_drivers(DM_ROOT(), pre_reloc_only); if (ret == -ENOENT) { dm_warn("Some drivers were not found\n"); ret = 0;@@ -79,7 +79,7 @@ int dm_scan_platdata(void) }
#ifdef CONFIG_OF_CONTROL -int dm_scan_fdt(const void *blob) +int dm_scan_fdt(const void *blob, bool pre_reloc_only) { int offset = 0; int ret = 0, err; @@ -88,6 +88,9 @@ int dm_scan_fdt(const void *blob) do { offset = fdt_next_node(blob, offset, &depth); if (offset > 0 && depth == 1) {
if (pre_reloc_only &&!fdt_getprop(blob, offset, "dm,pre-reloc", NULL))continue; err = lists_bind_fdt(gd->dm_root, blob, offset); if (err && !ret) ret = err;diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h index e95b7a9..fde67bb 100644 --- a/include/dm/device-internal.h +++ b/include/dm/device-internal.h @@ -45,12 +45,14 @@ int device_bind(struct device *parent, struct driver *drv,
- tree.
- @parent: Pointer to device's parent
- @pre_reloc_only: If true, bind the driver only if its DM_INIT_F flag is set.
*/
- If false bind the driver always.
- @info: Name and platdata for this device
- @devp: Returns a pointer to the bound device
- @return 0 if OK, -ve on error
-int device_bind_by_name(struct device *parent, const struct driver_info *info,
struct device **devp);+int device_bind_by_name(struct device *parent, bool pre_reloc_only,
const struct driver_info *info, struct device **devp);/**
- device_probe() - Probe a device, activating it
diff --git a/include/dm/device.h b/include/dm/device.h index b048b66..32650fd 100644 --- a/include/dm/device.h +++ b/include/dm/device.h @@ -23,6 +23,9 @@ struct driver_info; /* DM is responsible for allocating and freeing platdata */ #define DM_FLAG_ALLOC_PDATA (1 << 1)
+/* DM should init this device prior to relocation */ +#define DM_FLAG_PRE_RELOC (1 << 2)
/**
- struct device - An instance of a driver
@@ -117,6 +120,7 @@ struct device_id {
- ops: Driver-specific operations. This is typically a list of function
- pointers defined by the driver, to implement driver functions required by
- the uclass.
*/
- @flags: driver flags - see DM_FLAGS_...
struct driver { char *name; @@ -130,6 +134,7 @@ struct driver { int priv_auto_alloc_size; int platdata_auto_alloc_size; const void *ops; /* driver-specific operations */
uint32_t flags;};
/* Declare a new U-Boot driver */ diff --git a/include/dm/lists.h b/include/dm/lists.h index d0b720b..13e025e 100644 --- a/include/dm/lists.h +++ b/include/dm/lists.h @@ -42,7 +42,7 @@ struct uclass_driver *lists_uclass_lookup(enum uclass_id id);
- @early_only: If true, bind only drivers with the DM_INIT_F flag. If false
- bind all drivers.
*/ -int lists_bind_drivers(struct device *parent); +int lists_bind_drivers(struct device *parent, bool pre_reloc_only);
/**
- lists_bind_fdt() - bind a device tree node
diff --git a/include/dm/root.h b/include/dm/root.h index 1631d5d..30ebe6c 100644 --- a/include/dm/root.h +++ b/include/dm/root.h @@ -26,9 +26,11 @@ struct device *dm_root(void);
- This scans all available platdata and creates drivers for each
- @pre_reloc_only: If true, bind only drivers with the DM_FLAG_PRE_RELOC
*/
- flag. If false bind all drivers.
- @return 0 if OK, -ve on error
-int dm_scan_platdata(void); +int dm_scan_platdata(bool pre_reloc_only);
/**
- dm_scan_fdt() - Scan the device tree and bind drivers
@@ -36,9 +38,11 @@ int dm_scan_platdata(void);
- This scans the device tree and creates a driver for each node
- @blob: Pointer to device tree blob
- @pre_reloc_only: If true, bind only drivers with the DM_FLAG_PRE_RELOC
*/
- flag. If false bind all drivers.
- @return 0 if OK, -ve on error
-int dm_scan_fdt(const void *blob); +int dm_scan_fdt(const void *blob, bool pre_reloc_only);
/**
- dm_init() - Initialise Driver Model structures
diff --git a/test/dm/core.c b/test/dm/core.c index a889fad..63bb561 100644 --- a/test/dm/core.c +++ b/test/dm/core.c @@ -25,6 +25,7 @@ enum { TEST_INTVAL2 = 3, TEST_INTVAL3 = 6, TEST_INTVAL_MANUAL = 101112,
TEST_INTVAL_PRE_RELOC = 7,};
static const struct dm_test_pdata test_pdata[] = { @@ -37,6 +38,10 @@ static const struct dm_test_pdata test_pdata_manual = { .ping_add = TEST_INTVAL_MANUAL, };
+static const struct dm_test_pdata test_pdata_pre_reloc = {
.ping_add = TEST_INTVAL_PRE_RELOC,+};
U_BOOT_DEVICE(dm_test_info1) = { .name = "test_drv", .platdata = &test_pdata[0], @@ -57,6 +62,11 @@ static struct driver_info driver_info_manual = { .platdata = &test_pdata_manual, };
+static struct driver_info driver_info_pre_reloc = {
.name = "test_pre_reloc_drv",.platdata = &test_pdata_manual,+};
/* Test that binding with platdata occurs correctly */ static int dm_test_autobind(struct dm_test_state *dms) { @@ -71,7 +81,7 @@ static int dm_test_autobind(struct dm_test_state *dms) ut_asserteq(0, list_count_items(&gd->dm_root->child_head)); ut_asserteq(0, dm_testdrv_op_count[DM_TEST_OP_POST_BIND]);
ut_assertok(dm_scan_platdata());
ut_assertok(dm_scan_platdata(false)); /* We should have our test class now at least, plus more children */ ut_assert(1 < list_count_items(&gd->uclass_root));@@ -181,7 +191,7 @@ static int dm_test_lifecycle(struct dm_test_state *dms)
memcpy(op_count, dm_testdrv_op_count, sizeof(op_count));
ut_assertok(device_bind_by_name(dms->root, &driver_info_manual,
ut_assertok(device_bind_by_name(dms->root, false, &driver_info_manual, &dev)); ut_assert(dev); ut_assert(dm_testdrv_op_count[DM_TEST_OP_BIND]@@ -232,15 +242,15 @@ static int dm_test_ordering(struct dm_test_state *dms) struct device *dev, *dev_penultimate, *dev_last, *test_dev; int pingret;
ut_assertok(device_bind_by_name(dms->root, &driver_info_manual,
ut_assertok(device_bind_by_name(dms->root, false, &driver_info_manual, &dev)); ut_assert(dev); /* Bind two new devices (numbers 4 and 5) */
ut_assertok(device_bind_by_name(dms->root, &driver_info_manual,
ut_assertok(device_bind_by_name(dms->root, false, &driver_info_manual, &dev_penultimate)); ut_assert(dev_penultimate);
ut_assertok(device_bind_by_name(dms->root, &driver_info_manual,
ut_assertok(device_bind_by_name(dms->root, false, &driver_info_manual, &dev_last)); ut_assert(dev_last);@@ -255,7 +265,8 @@ static int dm_test_ordering(struct dm_test_state *dms) ut_assert(dev_last == test_dev);
/* Add back the original device 3, now in position 5 */
ut_assertok(device_bind_by_name(dms->root, &driver_info_manual, &dev));
ut_assertok(device_bind_by_name(dms->root, false, &driver_info_manual,&dev)); ut_assert(dev); /* Try ping */@@ -375,8 +386,8 @@ static int dm_test_leak(struct dm_test_state *dms) if (!start.uordblks) puts("Warning: Please add '#define DEBUG' to the top of common/dlmalloc.c\n");
ut_assertok(dm_scan_platdata());ut_assertok(dm_scan_fdt(gd->fdt_blob));
ut_assertok(dm_scan_platdata(false));ut_assertok(dm_scan_fdt(gd->fdt_blob, false)); /* Scanning the uclass is enough to probe all the devices */ for (id = UCLASS_ROOT; id < UCLASS_COUNT; id++) {@@ -444,8 +455,8 @@ static int create_children(struct dm_test_state *dms, struct device *parent, for (i = 0; i < count; i++) { struct dm_test_pdata *pdata;
ut_assertok(device_bind_by_name(parent, &driver_info_manual,&dev));
ut_assertok(device_bind_by_name(parent, false,&driver_info_manual, &dev)); pdata = calloc(1, sizeof(*pdata)); pdata->ping_add = key + i; dev->platdata = pdata;@@ -542,3 +553,19 @@ static int dm_test_children(struct dm_test_state *dms) return 0; } DM_TEST(dm_test_children, 0);
+static int dm_test_pre_reloc(struct dm_test_state *dms) +{
struct device *dev;/* The normal driver should refuse to bind before relocation */ut_asserteq(-EPERM, device_bind_by_name(dms->root, true,&driver_info_manual, &dev));/* But this one is marked pre-reloc */ut_assertok(device_bind_by_name(dms->root, true,&driver_info_pre_reloc, &dev));return 0;+} +DM_TEST(dm_test_pre_reloc, 0); diff --git a/test/dm/test-driver.c b/test/dm/test-driver.c index c4be8a1..e7fb199 100644 --- a/test/dm/test-driver.c +++ b/test/dm/test-driver.c @@ -144,3 +144,14 @@ U_BOOT_DRIVER(test_manual_drv) = { .remove = test_manual_remove, .unbind = test_manual_unbind, };
+U_BOOT_DRIVER(test_pre_reloc_drv) = {
.name = "test_pre_reloc_drv",.id = UCLASS_TEST,.ops = &test_manual_ops,.bind = test_manual_bind,.probe = test_manual_probe,.remove = test_manual_remove,.unbind = test_manual_unbind,.flags = DM_FLAG_PRE_RELOC,+}; diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c index e1d982f..d6f5bb8 100644 --- a/test/dm/test-fdt.c +++ b/test/dm/test-fdt.c @@ -99,7 +99,7 @@ static int dm_test_fdt(struct dm_test_state *dms) int ret; int i;
ret = dm_scan_fdt(gd->fdt_blob);
ret = dm_scan_fdt(gd->fdt_blob, false); ut_assert(!ret); ret = uclass_get(UCLASS_TEST_FDT, &uc);@@ -142,3 +142,21 @@ static int dm_test_fdt(struct dm_test_state *dms) return 0; } DM_TEST(dm_test_fdt, 0);
+static int dm_test_fdt_pre_reloc(struct dm_test_state *dms) +{
struct uclass *uc;int ret;ret = dm_scan_fdt(gd->fdt_blob, true);ut_assert(!ret);ret = uclass_get(UCLASS_TEST_FDT, &uc);ut_assert(!ret);/* These is only one pre-reloc device */ut_asserteq(1, list_count_items(&uc->dev_head));return 0;+} +DM_TEST(dm_test_fdt_pre_reloc, 0); diff --git a/test/dm/test-main.c b/test/dm/test-main.c index 828ed46..49805eb 100644 --- a/test/dm/test-main.c +++ b/test/dm/test-main.c @@ -89,11 +89,11 @@ int dm_test_main(void) ut_assertok(dm_test_init(dms));
if (test->flags & DM_TESTF_SCAN_PDATA)
ut_assertok(dm_scan_platdata());
ut_assertok(dm_scan_platdata(false)); if (test->flags & DM_TESTF_PROBE_TEST) ut_assertok(do_autoprobe(dms)); if (test->flags & DM_TESTF_SCAN_FDT)
ut_assertok(dm_scan_fdt(gd->fdt_blob));
ut_assertok(dm_scan_fdt(gd->fdt_blob, false)); if (test->func(dms)) break;diff --git a/test/dm/test.dts b/test/dm/test.dts index ec5364f..b2eaddd 100644 --- a/test/dm/test.dts +++ b/test/dm/test.dts @@ -6,10 +6,21 @@ #address-cells = <1>; #size-cells = <0>;
alias {console = <&uart0>;};uart0: serial {compatible = "sandbox,serial";dm,pre-reloc;//text-colour = "cyan";};a-test { reg = <0>; compatible = "denx,u-boot-fdt-test"; ping-add = <0>;dm,pre-reloc; }; junk {-- 1.9.1.423.g4596e3a
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Make both dm enumeration commands support showing whether a driver is active or not, and use a consistent indicator (an asterisk).
Signed-off-by: Simon Glass sjg@chromium.org ---
test/dm/cmd_dm.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/test/dm/cmd_dm.c b/test/dm/cmd_dm.c index c6b2eb8..ee02314 100644 --- a/test/dm/cmd_dm.c +++ b/test/dm/cmd_dm.c @@ -23,9 +23,9 @@ static int display_succ(struct device *in, char *buf) char local[16]; struct device *pos, *n, *prev = NULL;
- printf("%s- %s @ %08x", buf, in->name, (uint)map_to_sysmem(in)); - if (in->flags & DM_FLAG_ACTIVATED) - puts(" - activated"); + printf("%s- %c %s @ %08x", buf, + in->flags & DM_FLAG_ACTIVATED ? '*' : ' ', + in->name, (uint)map_to_sysmem(in)); puts("\n");
if (list_empty(&in->child_head)) @@ -84,8 +84,9 @@ static int do_dm_dump_uclass(cmd_tbl_t *cmdtp, int flag, int argc, for (ret = uclass_first_device(id, &dev); dev; ret = uclass_next_device(&dev)) { - printf(" %s @ %08x:\n", dev->name, - (uint)map_to_sysmem(dev)); + printf(" %c %s @ %08x:\n", + dev->flags & DM_FLAG_ACTIVATED ? '*' : ' ', + dev->name, (uint)map_to_sysmem(dev)); } puts("\n"); }
Why is '*' better than 'active' here?
At the very least, you need to convince me in the log message.
Thanks, jdl
On Sat, May 24, 2014 at 4:21 PM, Simon Glass sjg@chromium.org wrote:
Make both dm enumeration commands support showing whether a driver is active or not, and use a consistent indicator (an asterisk).
Signed-off-by: Simon Glass sjg@chromium.org
test/dm/cmd_dm.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/test/dm/cmd_dm.c b/test/dm/cmd_dm.c index c6b2eb8..ee02314 100644 --- a/test/dm/cmd_dm.c +++ b/test/dm/cmd_dm.c @@ -23,9 +23,9 @@ static int display_succ(struct device *in, char *buf) char local[16]; struct device *pos, *n, *prev = NULL;
printf("%s- %s @ %08x", buf, in->name, (uint)map_to_sysmem(in));if (in->flags & DM_FLAG_ACTIVATED)puts(" - activated");
printf("%s- %c %s @ %08x", buf,in->flags & DM_FLAG_ACTIVATED ? '*' : ' ',in->name, (uint)map_to_sysmem(in)); puts("\n"); if (list_empty(&in->child_head))@@ -84,8 +84,9 @@ static int do_dm_dump_uclass(cmd_tbl_t *cmdtp, int flag, int argc, for (ret = uclass_first_device(id, &dev); dev; ret = uclass_next_device(&dev)) {
printf(" %s @ %08x:\n", dev->name,(uint)map_to_sysmem(dev));
printf(" %c %s @ %08x:\n",dev->flags & DM_FLAG_ACTIVATED ? '*' : ' ',dev->name, (uint)map_to_sysmem(dev)); } puts("\n"); }-- 1.9.1.423.g4596e3a
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hi Jon,
On 27 May 2014 08:25, Jon Loeliger loeliger@gmail.com wrote:
Why is '*' better than 'active' here?
At the very least, you need to convince me in the log message.
I'll add something like this:
This is more consise that using the word 'activated' and easily fits in the left side of the display, thus appears in the same column on each line. It is therefore easier to see which devices are activated.
Regards, Simon
On Tuesday, May 27, 2014 at 04:25:31 PM, Jon Loeliger wrote:
Why is '*' better than 'active' here?
At the very least, you need to convince me in the log message.
Please stop top-posting ;-)
The asterisk is wasting less space and it's pretty clear anyway.
Please add my:
Acked-by: Marek Vasut marex@denx.de
Best regards, Marek Vasut
On 24 May 2014 15:21, Simon Glass sjg@chromium.org wrote:
Make both dm enumeration commands support showing whether a driver is active or not, and use a consistent indicator (an asterisk).
Signed-off-by: Simon Glass sjg@chromium.org
test/dm/cmd_dm.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
Rebased to master and applied to u-boot-dm/master.
These don't really server any purpose in the modern age, IMO. On the other hand they show up as annoying control characters in my editor, which then happily removes them.
I believe we can drop these characters from the file.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/dlmalloc.c | 46 +++++++++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 23 deletions(-)
diff --git a/common/dlmalloc.c b/common/dlmalloc.c index 3c70d5d..d1cd561 100644 --- a/common/dlmalloc.c +++ b/common/dlmalloc.c @@ -220,7 +220,7 @@
*/
- +
/* Preliminaries */
@@ -1132,7 +1132,7 @@ gAllocatedSize))
#endif
- +
/* Type declarations @@ -1272,7 +1272,7 @@ nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ serviced via calls to mmap, and then later released via munmap.
*/ - + /* sizes, alignments */
#define SIZE_SZ (sizeof(INTERNAL_SIZE_T)) @@ -1297,7 +1297,7 @@ nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ #define aligned_OK(m) (((unsigned long)((m)) & (MALLOC_ALIGN_MASK)) == 0)
- +
/* Physical chunk operations @@ -1332,7 +1332,7 @@ nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ #define chunk_at_offset(p, s) ((mchunkptr)(((char*)(p)) + (s)))
- +
/* Dealing with use bits @@ -1371,7 +1371,7 @@ nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ (((mchunkptr)(((char*)(p)) + (s)))->size &= ~(PREV_INUSE))
- +
/* Dealing with size fields @@ -1394,7 +1394,7 @@ nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ #define set_foot(p, s) (((mchunkptr)((char*)(p) + (s)))->prev_size = (s))
- +
/* @@ -1566,7 +1566,7 @@ void mem_malloc_init(ulong start, ulong size)
#define is_small_request(nb) (nb < MAX_SMALLBIN_SIZE - SMALLBIN_WIDTH)
- +
/* To help compensate for the large number of bins, a one-level index @@ -1590,7 +1590,7 @@ void mem_malloc_init(ulong start, ulong size) #define clear_binblock(ii) (binblocks_w = (mbinptr)(binblocks_r & ~(idx2binblock(ii))))
- +
/* Other static bookkeeping data */ @@ -1628,7 +1628,7 @@ static unsigned int max_n_mmaps = 0; static unsigned long max_mmapped_mem = 0; #endif
- +
/* Debugging support @@ -1769,7 +1769,7 @@ static void do_check_malloced_chunk(p, s) mchunkptr p; INTERNAL_SIZE_T s; #define check_malloced_chunk(P,N) #endif
- +
/* Macro-based internal utilities @@ -1841,7 +1841,7 @@ static void do_check_malloced_chunk(p, s) mchunkptr p; INTERNAL_SIZE_T s; (last_remainder->fd = last_remainder->bk = last_remainder)
- +
/* Routines dealing with mmap(). */ @@ -1972,7 +1972,7 @@ static mchunkptr mremap_chunk(p, new_size) mchunkptr p; size_t new_size; #endif /* HAVE_MMAP */
- +
/* Extend the top-most chunk by obtaining memory from system. @@ -2089,7 +2089,7 @@ static void malloc_extend_top(nb) INTERNAL_SIZE_T nb; }
- +
/* Main public routines */
@@ -2396,7 +2396,7 @@ Void_t* mALLOc(bytes) size_t bytes; }
- +
/*
@@ -2513,7 +2513,7 @@ void fREe(mem) Void_t* mem; }
- +
/* @@ -2750,7 +2750,7 @@ Void_t* rEALLOc(oldmem, bytes) Void_t* oldmem; size_t bytes; }
- +
/*
@@ -2868,7 +2868,7 @@ Void_t* mEMALIGn(alignment, bytes) size_t alignment; size_t bytes;
}
- +
/* @@ -2975,7 +2975,7 @@ void cfree(mem) Void_t *mem; } #endif
- +
/*
@@ -3056,7 +3056,7 @@ int malloc_trim(pad) size_t pad; } }
- +
/* malloc_usable_size: @@ -3092,7 +3092,7 @@ size_t malloc_usable_size(mem) Void_t* mem; }
- +
/* Utility to update current_mallinfo for malloc_stats and mallinfo() */
@@ -3136,7 +3136,7 @@ static void malloc_update_mallinfo() } #endif /* DEBUG */
- +
/*
@@ -3183,7 +3183,7 @@ struct mallinfo mALLINFo() #endif /* DEBUG */
- +
/* mallopt:
All in favor of the patch, but there is a typo here:
These don't really server any purpose in the modern age, IMO. On the
s/server/serve/
Note trailing 'r'. IMO, I'd toss out the ', IMO' too if it were my patch.
Thanks, jdl
On Sat, May 24, 2014 at 4:21 PM, Simon Glass sjg@chromium.org wrote:
These don't really server any purpose in the modern age, IMO. On the other hand they show up as annoying control characters in my editor, which then happily removes them.
I believe we can drop these characters from the file.
Signed-off-by: Simon Glass sjg@chromium.org
common/dlmalloc.c | 46 +++++++++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 23 deletions(-)
diff --git a/common/dlmalloc.c b/common/dlmalloc.c index 3c70d5d..d1cd561 100644 --- a/common/dlmalloc.c +++ b/common/dlmalloc.c @@ -220,7 +220,7 @@
*/
/* Preliminaries */
@@ -1132,7 +1132,7 @@ gAllocatedSize))
#endif
/* Type declarations @@ -1272,7 +1272,7 @@ nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ serviced via calls to mmap, and then later released via munmap.
*/
/* sizes, alignments */
#define SIZE_SZ (sizeof(INTERNAL_SIZE_T)) @@ -1297,7 +1297,7 @@ nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ #define aligned_OK(m) (((unsigned long)((m)) & (MALLOC_ALIGN_MASK)) == 0)
/* Physical chunk operations @@ -1332,7 +1332,7 @@ nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ #define chunk_at_offset(p, s) ((mchunkptr)(((char*)(p)) + (s)))
/* Dealing with use bits @@ -1371,7 +1371,7 @@ nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ (((mchunkptr)(((char*)(p)) + (s)))->size &= ~(PREV_INUSE))
/* Dealing with size fields @@ -1394,7 +1394,7 @@ nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ #define set_foot(p, s) (((mchunkptr)((char*)(p) + (s)))->prev_size = (s))
/* @@ -1566,7 +1566,7 @@ void mem_malloc_init(ulong start, ulong size)
#define is_small_request(nb) (nb < MAX_SMALLBIN_SIZE - SMALLBIN_WIDTH)
/* To help compensate for the large number of bins, a one-level index @@ -1590,7 +1590,7 @@ void mem_malloc_init(ulong start, ulong size) #define clear_binblock(ii) (binblocks_w = (mbinptr)(binblocks_r & ~(idx2binblock(ii))))
/* Other static bookkeeping data */ @@ -1628,7 +1628,7 @@ static unsigned int max_n_mmaps = 0; static unsigned long max_mmapped_mem = 0; #endif
/* Debugging support @@ -1769,7 +1769,7 @@ static void do_check_malloced_chunk(p, s) mchunkptr p; INTERNAL_SIZE_T s; #define check_malloced_chunk(P,N) #endif
/* Macro-based internal utilities @@ -1841,7 +1841,7 @@ static void do_check_malloced_chunk(p, s) mchunkptr p; INTERNAL_SIZE_T s; (last_remainder->fd = last_remainder->bk = last_remainder)
/* Routines dealing with mmap(). */ @@ -1972,7 +1972,7 @@ static mchunkptr mremap_chunk(p, new_size) mchunkptr p; size_t new_size; #endif /* HAVE_MMAP */
/* Extend the top-most chunk by obtaining memory from system. @@ -2089,7 +2089,7 @@ static void malloc_extend_top(nb) INTERNAL_SIZE_T nb; }
/* Main public routines */
@@ -2396,7 +2396,7 @@ Void_t* mALLOc(bytes) size_t bytes; }
/*
@@ -2513,7 +2513,7 @@ void fREe(mem) Void_t* mem; }
/* @@ -2750,7 +2750,7 @@ Void_t* rEALLOc(oldmem, bytes) Void_t* oldmem; size_t bytes; }
/*
@@ -2868,7 +2868,7 @@ Void_t* mEMALIGn(alignment, bytes) size_t alignment; size_t bytes;
}
/* @@ -2975,7 +2975,7 @@ void cfree(mem) Void_t *mem; } #endif
/*
@@ -3056,7 +3056,7 @@ int malloc_trim(pad) size_t pad; } }
/* malloc_usable_size: @@ -3092,7 +3092,7 @@ size_t malloc_usable_size(mem) Void_t* mem; }
/* Utility to update current_mallinfo for malloc_stats and mallinfo() */
@@ -3136,7 +3136,7 @@ static void malloc_update_mallinfo() } #endif /* DEBUG */
/*
@@ -3183,7 +3183,7 @@ struct mallinfo mALLINFo() #endif /* DEBUG */
/* mallopt: -- 1.9.1.423.g4596e3a
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
If we are to have driver model before relocation we need to support some way of calling memory allocation routines.
The standard malloc() is pretty complicated:
1. It uses some BSS memory for its state, and BSS is not available before relocation
2. It supports algorithms for reducing memory fragmentation and improving performace of free(). Before relocation we could happily just not support free().
3. It includes about 4KB of code (Thumb 2) and 1KB of data. However since this has been loaded anyway this is not really a problem.
The simplest way to support pre-relocation malloc() is to reserve an area of memory and allocate it in increasing blocks as needed. This implementation does this.
To enable it, you need to define the address and size of the malloc() pool as described in the README.
Note that this implementation is only useful on machines which have some memory available before dram_init() is called - this includes those that do no DRAM init (like tegra) and those that do it in SPL (quite a few boards). Enabling driver model preior to relocation for the rest of the boards is left for a later exercise.
Signed-off-by: Simon Glass sjg@chromium.org ---
README | 16 ++++++++++++++++ common/board_f.c | 12 ++++++++++++ common/board_r.c | 4 ++++ common/dlmalloc.c | 35 +++++++++++++++++++++++++++++++++++ include/asm-generic/global_data.h | 5 +++++ 5 files changed, 72 insertions(+)
diff --git a/README b/README index 3c93d05..23ec935 100644 --- a/README +++ b/README @@ -3703,6 +3703,22 @@ Configuration Settings: - CONFIG_SYS_MALLOC_LEN: Size of DRAM reserved for malloc() use.
+- CONFIG_SYS_MALLOC_F_BASE +- CONFIG_SYS_MALLOC_F_LEN + Address and size of the malloc() pool for use before + relocation. If these are defined, then a very simple + malloc() implementation will become available before + relocation. It allocates regions with increasing addresses + within the region. calloc() is supported, but realloc() + is not available. free() is supported but does nothing. + The memory will be free (or in fact just forgotton) when + U-Boot relocates itself. + + You should select an address which is available before + relocation. This might be on-chip SRAM. Be careful to avoid + collision between this region and the stack, which + typically grows downwards. + - CONFIG_SYS_BOOTM_LEN: Normally compressed uImages are limited to an uncompressed size of 8 MBytes. If this is not enough, diff --git a/common/board_f.c b/common/board_f.c index 4ea4cb2..cc040c7 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -776,6 +776,17 @@ static int mark_bootstage(void) return 0; }
+static int initf_malloc(void) +{ +#ifdef CONFIG_SYS_MALLOC_F_BASE + gd->malloc_base = CONFIG_SYS_MALLOC_F_BASE; + gd->malloc_limit = CONFIG_SYS_MALLOC_F_BASE + CONFIG_SYS_MALLOC_F_LEN; + gd->malloc_ptr = 0; +#endif + + return 0; +} + static init_fnc_t init_sequence_f[] = { #ifdef CONFIG_SANDBOX setup_ram_buf, @@ -833,6 +844,7 @@ static init_fnc_t init_sequence_f[] = { sdram_adjust_866, init_timebase, #endif + initf_malloc, init_baud_rate, /* initialze baudrate settings */ serial_init, /* serial communications setup */ console_init_f, /* stage 1 init of console */ diff --git a/common/board_r.c b/common/board_r.c index d2a59ee..e16f3ad 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -259,6 +259,10 @@ static int initr_malloc(void) { ulong malloc_start;
+#ifdef CONFIG_SYS_MALLOC_F_BASE + debug("Pre-reloc malloc() used %#lx bytes (%ld KB)\n", gd->malloc_ptr, + gd->malloc_ptr / 1024); +#endif /* The malloc area is immediately below the monitor copy in DRAM */ malloc_start = gd->relocaddr - TOTAL_MALLOC_LEN; mem_malloc_init((ulong)map_sysmem(malloc_start, TOTAL_MALLOC_LEN), diff --git a/common/dlmalloc.c b/common/dlmalloc.c index d1cd561..16d85c2 100644 --- a/common/dlmalloc.c +++ b/common/dlmalloc.c @@ -930,6 +930,8 @@ struct mallinfo mALLINFo(); #endif /* 0 */ /* Moved to malloc.h */
#include <malloc.h> +#include <asm/io.h> + #ifdef DEBUG #if __STD_C static void malloc_update_mallinfo (void); @@ -2174,6 +2176,20 @@ Void_t* mALLOc(bytes) size_t bytes;
INTERNAL_SIZE_T nb;
+#ifdef CONFIG_SYS_MALLOC_F_BASE + if (!(gd->flags & GD_FLG_RELOC)) { + ulong new_ptr; + void *ptr; + + new_ptr = gd->malloc_ptr + bytes; + if (new_ptr > gd->malloc_limit) + panic("Out of pre-reloc memory"); + ptr = map_sysmem(gd->malloc_base + gd->malloc_ptr, bytes); + gd->malloc_ptr = ALIGN(new_ptr, sizeof(new_ptr)); + return ptr; + } +#endif + /* check if mem_malloc_init() was run */ if ((mem_malloc_start == 0) && (mem_malloc_end == 0)) { /* not initialized yet */ @@ -2437,6 +2453,12 @@ void fREe(mem) Void_t* mem; mchunkptr fwd; /* misc temp for linking */ int islr; /* track whether merging with last_remainder */
+#ifdef CONFIG_SYS_MALLOC_F_BASE + /* free() is a no-op - all the memory will be freed on relocation */ + if (!(gd->flags & GD_FLG_RELOC)) + return; +#endif + if (mem == NULL) /* free(0) has no effect */ return;
@@ -2588,6 +2610,13 @@ Void_t* rEALLOc(oldmem, bytes) Void_t* oldmem; size_t bytes; /* realloc of null is supposed to be same as malloc */ if (oldmem == NULL) return mALLOc(bytes);
+#ifdef CONFIG_SYS_MALLOC_F_BASE + if (!(gd->flags & GD_FLG_RELOC)) { + /* This is harder to support and should not be needed */ + panic("pre-reloc realloc() is not supported"); + } +#endif + newp = oldp = mem2chunk(oldmem); newsize = oldsize = chunksize(oldp);
@@ -2933,6 +2962,12 @@ Void_t* cALLOc(n, elem_size) size_t n; size_t elem_size; return NULL; else { +#ifdef CONFIG_SYS_MALLOC_F_BASE + if (!(gd->flags & GD_FLG_RELOC)) { + MALLOC_ZERO(mem, sz); + return mem; + } +#endif p = mem2chunk(mem);
/* Two optional cases in which clearing not necessary */ diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h index e98b661..1070a75 100644 --- a/include/asm-generic/global_data.h +++ b/include/asm-generic/global_data.h @@ -85,6 +85,11 @@ typedef struct global_data { #endif unsigned long timebase_h; unsigned long timebase_l; +#ifdef CONFIG_SYS_MALLOC_F_BASE + unsigned long malloc_base; /* base address of early malloc() */ + unsigned long malloc_limit; /* limit address */ + unsigned long malloc_ptr; /* current address */ +#endif struct arch_global_data arch; /* architecture-specific data */ } gd_t; #endif
Enable this for sandbox so that we will be able to use driver model before relocation.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/configs/sandbox.h | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h index fdc8b75..86ee70f 100644 --- a/include/configs/sandbox.h +++ b/include/configs/sandbox.h @@ -115,6 +115,10 @@ #define CONFIG_SYS_MONITOR_BASE 0 #define CONFIG_NR_DRAM_BANKS 1
+#define CONFIG_SYS_MALLOC_F_LEN 1024 +#define CONFIG_SYS_MALLOC_F_BASE (CONFIG_SYS_SDRAM_SIZE - \ + CONFIG_SYS_MALLOC_F_LEN) + #define CONFIG_BAUDRATE 115200 #define CONFIG_SYS_BAUDRATE_TABLE {4800, 9600, 19200, 38400, 57600,\ 115200}
Initialise devices marked 'pre-reloc' and make them available prior to relocation. Note that this requires pre-reloc malloc() to be available.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/board_f.c | 30 ++++++++++++++++++++++++++++++ common/board_r.c | 3 +++ include/asm-generic/global_data.h | 1 + 3 files changed, 34 insertions(+)
diff --git a/common/board_f.c b/common/board_f.c index cc040c7..0dc745a 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -14,6 +14,7 @@ #include <linux/compiler.h> #include <version.h> #include <environment.h> +#include <dm.h> #include <fdtdec.h> #include <fs.h> #if defined(CONFIG_CMD_IDE) @@ -52,6 +53,7 @@ #ifdef CONFIG_SANDBOX #include <asm/state.h> #endif +#include <dm/root.h> #include <linux/compiler.h>
/* @@ -787,6 +789,33 @@ static int initf_malloc(void) return 0; }
+static int initf_dm(void) +{ +#if defined(CONFIG_DM) && defined(CONFIG_SYS_MALLOC_F_BASE) + int ret; + + ret = dm_init(); + if (ret) { + debug("dm_init() failed: %d\n", ret); + return ret; + } + ret = dm_scan_platdata(true); + if (ret) { + debug("dm_scan_platdata() failed: %d\n", ret); + return ret; + } +#ifdef CONFIG_OF_CONTROL + ret = dm_scan_fdt(gd->fdt_blob, true); + if (ret) { + debug("dm_scan_fdt() failed: %d\n", ret); + return ret; + } +#endif +#endif + + return 0; +} + static init_fnc_t init_sequence_f[] = { #ifdef CONFIG_SANDBOX setup_ram_buf, @@ -845,6 +874,7 @@ static init_fnc_t init_sequence_f[] = { init_timebase, #endif initf_malloc, + initf_dm, init_baud_rate, /* initialze baudrate settings */ serial_init, /* serial communications setup */ console_init_f, /* stage 1 init of console */ diff --git a/common/board_r.c b/common/board_r.c index e16f3ad..13c1bc1 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -275,6 +275,9 @@ static int initr_dm(void) { int ret;
+ /* Save the pre-reloc driver model and start a new one */ + gd->dm_root_f = gd->dm_root; + gd->dm_root = NULL; ret = dm_init(); if (ret) { debug("dm_init() failed: %d\n", ret); diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h index 1070a75..b3b91d3 100644 --- a/include/asm-generic/global_data.h +++ b/include/asm-generic/global_data.h @@ -66,6 +66,7 @@ typedef struct global_data {
#ifdef CONFIG_DM struct device *dm_root; /* Root instance for Driver Model */ + struct device *dm_root_f; /* Pre-relocation root instance */ struct list_head uclass_root; /* Head of core tree */ #endif
The current functions for adding and removing devices require a device name. This is not convenient for driver model, which wants to store a pointer to the relevant device. Add new functions which provide this feature and adjust the old ones to call these.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/stdio.c | 29 ++++++++++++++++++++++------- include/stdio_dev.h | 2 ++ 2 files changed, 24 insertions(+), 7 deletions(-)
diff --git a/common/stdio.c b/common/stdio.c index dd402cc..f490d6b 100644 --- a/common/stdio.c +++ b/common/stdio.c @@ -148,7 +148,7 @@ struct stdio_dev* stdio_clone(struct stdio_dev *dev) return _dev; }
-int stdio_register (struct stdio_dev * dev) +int stdio_register_dev(struct stdio_dev *dev, struct stdio_dev **devp) { struct stdio_dev *_dev;
@@ -156,24 +156,27 @@ int stdio_register (struct stdio_dev * dev) if(!_dev) return -1; list_add_tail(&(_dev->list), &(devs.list)); + if (devp) + *devp = _dev; + return 0; }
+int stdio_register(struct stdio_dev *dev) +{ + return stdio_register_dev(dev, NULL); +} + /* deregister the device "devname". * returns 0 if success, -1 if device is assigned and 1 if devname not found */ #ifdef CONFIG_SYS_STDIO_DEREGISTER -int stdio_deregister(const char *devname) +int stdio_deregister_dev(struct stdio_dev *dev) { int l; struct list_head *pos; - struct stdio_dev *dev; char temp_names[3][16];
- dev = stdio_get_by_name(devname); - - if(!dev) /* device not found */ - return -1; /* get stdio devices (ListRemoveItem changes the dev list) */ for (l=0 ; l< MAX_FILES; l++) { if (stdio_devices[l] == dev) { @@ -197,6 +200,18 @@ int stdio_deregister(const char *devname) } return 0; } + +int stdio_deregister(const char *devname) +{ + struct stdio_dev *dev; + + dev = stdio_get_by_name(devname); + + if (!dev) /* device not found */ + return -1; + + return stdio_deregister_dev(dev); +} #endif /* CONFIG_SYS_STDIO_DEREGISTER */
int stdio_init (void) diff --git a/include/stdio_dev.h b/include/stdio_dev.h index 4587005..a7d0825 100644 --- a/include/stdio_dev.h +++ b/include/stdio_dev.h @@ -77,10 +77,12 @@ extern char *stdio_names[MAX_FILES]; * PROTOTYPES */ int stdio_register (struct stdio_dev * dev); +int stdio_register_dev(struct stdio_dev *dev, struct stdio_dev **devp); int stdio_init (void); void stdio_print_current_devices(void); #ifdef CONFIG_SYS_STDIO_DEREGISTER int stdio_deregister(const char *devname); +int stdio_deregister_dev(struct stdio_dev *dev); #endif struct list_head* stdio_get_list(void); struct stdio_dev* stdio_get_by_name(const char* name);
On Saturday, May 24, 2014 at 11:21:12 PM, Simon Glass wrote:
The current functions for adding and removing devices require a device name. This is not convenient for driver model, which wants to store a pointer to the relevant device. Add new functions which provide this feature and adjust the old ones to call these.
Signed-off-by: Simon Glass sjg@chromium.org
I think we should use errno.h and not return -1 all around.
Other than that: Acked-by: Marek Vasut marex@denx.de
Best regards, Marek Vasut
If the console is not present, we try to reduce overhead by stopping any output in vprintf(), before it gets to putc(). This is dubious merit in general, but in the case of sandbox it is incorrect, since we have a fallback console which reports errors very early in U-Boot. If this is defeated U-Boot can hang or exit with no indication of what is wrong.
Remove the optimisation for sandbox.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/console.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/console.c b/common/console.c index 11c102a..5576dfd 100644 --- a/common/console.c +++ b/common/console.c @@ -504,7 +504,7 @@ int vprintf(const char *fmt, va_list args) uint i; char printbuffer[CONFIG_SYS_PBSIZE];
-#ifndef CONFIG_PRE_CONSOLE_BUFFER +#if defined(CONFIG_PRE_CONSOLE_BUFFER) && !defined(CONFIG_SANDBOX) if (!gd->have_console) return 0; #endif
For sandbox we have a fallback console which is used very early during U-Boot, before serial drivers are available. Rather than try to guess when to switch to the real one, add a flag so we can be sure. This makes sure that sandbox can always output a panic() message, for example, and avoids silent failure (which is very annoying in sandbox).
Signed-off-by: Simon Glass sjg@chromium.org ---
common/console.c | 4 ++-- drivers/serial/serial.c | 1 + include/asm-generic/global_data.h | 1 + 3 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/common/console.c b/common/console.c index 5576dfd..898da39 100644 --- a/common/console.c +++ b/common/console.c @@ -417,7 +417,7 @@ static inline void print_pre_console_buffer(void) {} void putc(const char c) { #ifdef CONFIG_SANDBOX - if (!gd) { + if (!gd || !(gd->flags & GD_FLG_SERIAL_READY)) { os_putc(c); return; } @@ -447,7 +447,7 @@ void putc(const char c) void puts(const char *s) { #ifdef CONFIG_SANDBOX - if (!gd) { + if (!gd || !(gd->flags & GD_FLG_SERIAL_READY)) { os_puts(s); return; } diff --git a/drivers/serial/serial.c b/drivers/serial/serial.c index 803d850..d2eb752 100644 --- a/drivers/serial/serial.c +++ b/drivers/serial/serial.c @@ -418,6 +418,7 @@ static struct serial_device *get_current(void) */ int serial_init(void) { + gd->flags |= GD_FLG_SERIAL_READY; return get_current()->start(); }
diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h index b3b91d3..f0ea655 100644 --- a/include/asm-generic/global_data.h +++ b/include/asm-generic/global_data.h @@ -106,5 +106,6 @@ typedef struct global_data { #define GD_FLG_LOGINIT 0x00020 /* Log Buffer has been initialized */ #define GD_FLG_DISABLE_CONSOLE 0x00040 /* Disable console (in & out) */ #define GD_FLG_ENV_READY 0x00080 /* Env. imported into hash table */ +#define GD_FLG_SERIAL_READY 0x00100 /* Pre-reloc serial console ready */
#endif /* __ASM_GENERIC_GBL_DATA_H */
Where there are serveral device options that can be chosen, often one is preferred. This can normally be handled by aliases in the device tree.
However, when a device can be specified either with platform data or with a device tree node, which one should dm use? This situation happens with sandbox, where we want to use the device tree version if we have a device tree, and fall back to the platform data version if not. We need this to work because without a console U-Boot will not function.
The original approach was just to take the first device in the uclass and use that, but this does not work because the ordering is unknown.
The preferred device can be specified with a DM_FLAG_PREFER flag or a 'dm,prefer' property in the device tree node.
It is possible that a better approach will come to light in the future, but this gets around the problem as it currently stands.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/device.c | 5 +++++ drivers/core/uclass.c | 15 ++++++++++++++- include/dm/device.h | 3 +++ include/dm/uclass-internal.h | 3 ++- include/dm/uclass.h | 15 +++++++++++++++ test/dm/test-fdt.c | 14 ++++++++++++++ test/dm/test.dts | 1 + 7 files changed, 54 insertions(+), 2 deletions(-)
diff --git a/drivers/core/device.c b/drivers/core/device.c index 2c2634e..6b2c8f9 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -10,6 +10,7 @@ */
#include <common.h> +#include <fdtdec.h> #include <malloc.h> #include <dm/device.h> #include <dm/device-internal.h> @@ -21,6 +22,8 @@ #include <linux/err.h> #include <linux/list.h>
+DECLARE_GLOBAL_DATA_PTR; + /** * device_chld_unbind() - Unbind all device's children from the device * @@ -95,6 +98,8 @@ int device_bind(struct device *parent, struct driver *drv, const char *name, dev->parent = parent; dev->driver = drv; dev->uclass = uc; + if (fdtdec_get_bool(gd->fdt_blob, of_offset, "dm,prefer")) + dev->flags |= DM_FLAG_PREFER; if (!dev->platdata && drv->platdata_auto_alloc_size) dev->flags |= DM_FLAG_ALLOC_PDATA;
diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c index afb5fdc..d02ea5e 100644 --- a/drivers/core/uclass.c +++ b/drivers/core/uclass.c @@ -149,7 +149,8 @@ int uclass_find_device(enum uclass_id id, int index, struct device **devp) return ret;
list_for_each_entry(dev, &uc->dev_head, uclass_node) { - if (!index--) { + if (index == -1 ? (dev->flags & DM_FLAG_PREFER) : + !index--) { *devp = dev; return 0; } @@ -177,6 +178,18 @@ int uclass_get_device(enum uclass_id id, int index, struct device **devp) return 0; }
+int uclass_get_preferred_device(enum uclass_id id, struct device **devp) +{ + int err; + + err = uclass_get_device(id, -1, devp); + if (!err || err != -ENODEV) + return err; + + /* Nothing found, just pick the first device */ + return uclass_get_device(id, 0, devp); +} + int uclass_first_device(enum uclass_id id, struct device **devp) { struct uclass *uc; diff --git a/include/dm/device.h b/include/dm/device.h index 32650fd..d3c04ed 100644 --- a/include/dm/device.h +++ b/include/dm/device.h @@ -26,6 +26,9 @@ struct driver_info; /* DM should init this device prior to relocation */ #define DM_FLAG_PRE_RELOC (1 << 2)
+/* DM should prefer this driver when searching a uclass */ +#define DM_FLAG_PREFER (1 << 3) + /** * struct device - An instance of a driver * diff --git a/include/dm/uclass-internal.h b/include/dm/uclass-internal.h index cc65d52..79b4d9e 100644 --- a/include/dm/uclass-internal.h +++ b/include/dm/uclass-internal.h @@ -13,7 +13,8 @@ /** * uclass_find_device() - Return n-th child of uclass * @id: Id number of the uclass - * @index: Position of the child in uclass's list + * @index: Position of the child in uclass's list. If -1 then the + * device marked with DM_FLAG_PREFER is returned, if any * #devp: Returns pointer to device, or NULL on error * * The device is not prepared for use - this is an internal function diff --git a/include/dm/uclass.h b/include/dm/uclass.h index ac5c147..2ca87fc 100644 --- a/include/dm/uclass.h +++ b/include/dm/uclass.h @@ -106,6 +106,21 @@ int uclass_get(enum uclass_id key, struct uclass **ucp); int uclass_get_device(enum uclass_id id, int index, struct device **devp);
/** + * uclass_get_preferred_device() - Get the preferred uclass device + * + * id: ID to look up + * @ucp: Returns pointer to device (there is only one per for each ID) + * + * This returns the device with the DM_FLAG_PREFER flag set, if any. + * Otherwise it returns the first device. + * + * The device is probed to activate it ready for use. + * + * @return 0 if OK, -ve on error + */ +int uclass_get_preferred_device(enum uclass_id id, struct device **devp); + +/** * uclass_first_device() - Get the first device in a uclass * * @id: Uclass ID to look up diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c index d6f5bb8..5dce48e 100644 --- a/test/dm/test-fdt.c +++ b/test/dm/test-fdt.c @@ -160,3 +160,17 @@ static int dm_test_fdt_pre_reloc(struct dm_test_state *dms) return 0; } DM_TEST(dm_test_fdt_pre_reloc, 0); + +/* Test that autoprobe finds all the expected devices */ +static int dm_test_prefer(struct dm_test_state *dms) +{ + struct device *dev; + + ut_assertok(uclass_get_preferred_device(UCLASS_TEST_FDT, &dev)); + ut_assert(dev); + + ut_asserteq_str("b-test", dev->name); + + return 0; +} +DM_TEST(dm_test_prefer, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT); diff --git a/test/dm/test.dts b/test/dm/test.dts index b2eaddd..c42d4e7 100644 --- a/test/dm/test.dts +++ b/test/dm/test.dts @@ -36,6 +36,7 @@ reg = <3>; compatible = "denx,u-boot-fdt-test"; ping-add = <3>; + prefer; };
some-bus {
The preferred device can be specified with a DM_FLAG_PREFER flag or a 'dm,prefer' property in the device tree node.
It is possible that a better approach will come to light in the future, but this gets around the problem as it currently stands.
Here's your clue that something isn't quite right here. Again, this looks like the us of a DTS property to describe a SW functionality rather than describe the HW itself.
I'm not sure what needs to be done here either, but maybe it centers on a better understanding of *why* something is being preferred? And what if it is "preferred", then what does that really mean? And what if two devices are labelled as "preferred" -- Is that OK? Who checks and enforces it?
Sure, this may be a hack for now, but it needs more thought.
HTH, jdl
On Sat, May 24, 2014 at 4:21 PM, Simon Glass sjg@chromium.org wrote:
Where there are serveral device options that can be chosen, often one is preferred. This can normally be handled by aliases in the device tree.
However, when a device can be specified either with platform data or with a device tree node, which one should dm use? This situation happens with sandbox, where we want to use the device tree version if we have a device tree, and fall back to the platform data version if not. We need this to work because without a console U-Boot will not function.
The original approach was just to take the first device in the uclass and use that, but this does not work because the ordering is unknown.
The preferred device can be specified with a DM_FLAG_PREFER flag or a 'dm,prefer' property in the device tree node.
It is possible that a better approach will come to light in the future, but this gets around the problem as it currently stands.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/core/device.c | 5 +++++ drivers/core/uclass.c | 15 ++++++++++++++- include/dm/device.h | 3 +++ include/dm/uclass-internal.h | 3 ++- include/dm/uclass.h | 15 +++++++++++++++ test/dm/test-fdt.c | 14 ++++++++++++++ test/dm/test.dts | 1 + 7 files changed, 54 insertions(+), 2 deletions(-)
diff --git a/drivers/core/device.c b/drivers/core/device.c index 2c2634e..6b2c8f9 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -10,6 +10,7 @@ */
#include <common.h> +#include <fdtdec.h> #include <malloc.h> #include <dm/device.h> #include <dm/device-internal.h> @@ -21,6 +22,8 @@ #include <linux/err.h> #include <linux/list.h>
+DECLARE_GLOBAL_DATA_PTR;
/**
- device_chld_unbind() - Unbind all device's children from the device
@@ -95,6 +98,8 @@ int device_bind(struct device *parent, struct driver *drv, const char *name, dev->parent = parent; dev->driver = drv; dev->uclass = uc;
if (fdtdec_get_bool(gd->fdt_blob, of_offset, "dm,prefer"))dev->flags |= DM_FLAG_PREFER; if (!dev->platdata && drv->platdata_auto_alloc_size) dev->flags |= DM_FLAG_ALLOC_PDATA;diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c index afb5fdc..d02ea5e 100644 --- a/drivers/core/uclass.c +++ b/drivers/core/uclass.c @@ -149,7 +149,8 @@ int uclass_find_device(enum uclass_id id, int index, struct device **devp) return ret;
list_for_each_entry(dev, &uc->dev_head, uclass_node) {
if (!index--) {
if (index == -1 ? (dev->flags & DM_FLAG_PREFER) :!index--) { *devp = dev; return 0; }@@ -177,6 +178,18 @@ int uclass_get_device(enum uclass_id id, int index, struct device **devp) return 0; }
+int uclass_get_preferred_device(enum uclass_id id, struct device **devp) +{
int err;err = uclass_get_device(id, -1, devp);if (!err || err != -ENODEV)return err;/* Nothing found, just pick the first device */return uclass_get_device(id, 0, devp);+}
int uclass_first_device(enum uclass_id id, struct device **devp) { struct uclass *uc; diff --git a/include/dm/device.h b/include/dm/device.h index 32650fd..d3c04ed 100644 --- a/include/dm/device.h +++ b/include/dm/device.h @@ -26,6 +26,9 @@ struct driver_info; /* DM should init this device prior to relocation */ #define DM_FLAG_PRE_RELOC (1 << 2)
+/* DM should prefer this driver when searching a uclass */ +#define DM_FLAG_PREFER (1 << 3)
/**
- struct device - An instance of a driver
diff --git a/include/dm/uclass-internal.h b/include/dm/uclass-internal.h index cc65d52..79b4d9e 100644 --- a/include/dm/uclass-internal.h +++ b/include/dm/uclass-internal.h @@ -13,7 +13,8 @@ /**
- uclass_find_device() - Return n-th child of uclass
- @id: Id number of the uclass
- @index: Position of the child in uclass's list
- @index: Position of the child in uclass's list. If -1 then the
device marked with DM_FLAG_PREFER is returned, if any- #devp: Returns pointer to device, or NULL on error
- The device is not prepared for use - this is an internal function
diff --git a/include/dm/uclass.h b/include/dm/uclass.h index ac5c147..2ca87fc 100644 --- a/include/dm/uclass.h +++ b/include/dm/uclass.h @@ -106,6 +106,21 @@ int uclass_get(enum uclass_id key, struct uclass **ucp); int uclass_get_device(enum uclass_id id, int index, struct device **devp);
/**
- uclass_get_preferred_device() - Get the preferred uclass device
- id: ID to look up
- @ucp: Returns pointer to device (there is only one per for each ID)
- This returns the device with the DM_FLAG_PREFER flag set, if any.
- Otherwise it returns the first device.
- The device is probed to activate it ready for use.
- @return 0 if OK, -ve on error
- */
+int uclass_get_preferred_device(enum uclass_id id, struct device **devp);
+/**
- uclass_first_device() - Get the first device in a uclass
- @id: Uclass ID to look up
diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c index d6f5bb8..5dce48e 100644 --- a/test/dm/test-fdt.c +++ b/test/dm/test-fdt.c @@ -160,3 +160,17 @@ static int dm_test_fdt_pre_reloc(struct dm_test_state *dms) return 0; } DM_TEST(dm_test_fdt_pre_reloc, 0);
+/* Test that autoprobe finds all the expected devices */ +static int dm_test_prefer(struct dm_test_state *dms) +{
struct device *dev;ut_assertok(uclass_get_preferred_device(UCLASS_TEST_FDT, &dev));ut_assert(dev);ut_asserteq_str("b-test", dev->name);return 0;+} +DM_TEST(dm_test_prefer, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT); diff --git a/test/dm/test.dts b/test/dm/test.dts index b2eaddd..c42d4e7 100644 --- a/test/dm/test.dts +++ b/test/dm/test.dts @@ -36,6 +36,7 @@ reg = <3>; compatible = "denx,u-boot-fdt-test"; ping-add = <3>;
prefer; }; some-bus {-- 1.9.1.423.g4596e3a
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hi Jon,
On 27 May 2014 09:42, Jon Loeliger loeliger@gmail.com wrote:
The preferred device can be specified with a DM_FLAG_PREFER flag or a 'dm,prefer' property in the device tree node.
It is possible that a better approach will come to light in the future, but this gets around the problem as it currently stands.
Here's your clue that something isn't quite right here. Again, this looks like the us of a DTS property to describe a SW functionality rather than describe the HW itself.
I'm not sure what needs to be done here either, but maybe it centers on a better understanding of *why* something is being preferred? And what if it is "preferred", then what does that really mean? And what if two devices are labelled as "preferred" -- Is that OK? Who checks and enforces it?
Sure, this may be a hack for now, but it needs more thought.
Agreed, but I've done all the thinking I can so far, and this is the result.
Another way of doing this would be to designate the platform data device as a 'backup' device, only to be used if we don't get a similar device from the device tree. But I'm not sure that it would be better that way.
This is not intended to be a widely-used mechanism - it just deals with the issue of device tree vs. not.
My preference would be to require that all devices be represented by a device tree with driver model. However, I think that would discourage adoption in the short term. Also for sandbox we really do want to test all the code, and so always having a device tree would reduce test coverage.
Regards, Simon
The lifecycle of a device is an important part of driver model. Add to the existing documentation and clarify it.
Thanks for Jon Loeliger jdl@jdl.com for helping with the text and suggesting improvements.
(Jon please comment/adjust to help clarify things further)
Reported-by: Jon Loeliger jdl@jdl.com
Signed-off-by: Simon Glass sjg@chromium.org ---
doc/driver-model/README.txt | 197 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 191 insertions(+), 6 deletions(-)
diff --git a/doc/driver-model/README.txt b/doc/driver-model/README.txt index deacfe9..90e0516 100644 --- a/doc/driver-model/README.txt +++ b/doc/driver-model/README.txt @@ -95,11 +95,12 @@ are provided in test/dm. To run them, try: You should see something like this:
<...U-Boot banner...> - Running 12 driver model tests + Running 15 driver model tests Test: dm_test_autobind Test: dm_test_autoprobe Test: dm_test_children Test: dm_test_fdt + Test: dm_test_fdt_pre_reloc Test: dm_test_gpio sandbox_gpio: sb_gpio_get_value: error: offset 4 not reserved Test: dm_test_leak @@ -109,6 +110,8 @@ You should see something like this: Test: dm_test_operations Test: dm_test_ordering Test: dm_test_platdata + Test: dm_test_pre_reloc + Test: dm_test_prefer Test: dm_test_remove Test: dm_test_uclass Failures: 0 @@ -222,6 +225,40 @@ device tree) and probe. Platform Data -------------
+Platform data is like Linux platform data, if you are familiar with that. +It provides the board-specific information to start up a device. + +Why is this information not just stored in the device driver itself? The +idea is that the device driver is generic, and can in principle operate on +any board that has that type of device. For example, with modern +highly-complex SoCs it is common for the IP to come from an IP vendor, and +therefore (for example) the MMC controller may be the same on chips from +different vendors. It makes no sense to write independent drivers for the +MMC controller on each vendor's SoC, when they are all almost the same. +Similarly, we may have 6 UARTs in an SoC, all of which are mostly the same, +but lie at different addresses in the address space. + +Using the UART example, we have a single driver and it is instantiated 6 +times by supplying 6 lots of platform data. Each lot of platform data +gives the driver name and a pointer to a structure containing information +about this instance - e.g. the address of the register space. It may be that +one of the UARTS supports RS-485 operation - this can be added as a flag in +the platform data, which is set for this one port and clear for the rest. + +Think of your driver as a generic piece of code which knows how to talk to +a device, but needs to know where it is, any variant/option information and +so on. Platform data provides this link between the generic piece of code +and the specific way it is bound on a particular board. + +Examples of platform data include: + + - The base address of the IP block's register space + - Configuration options, like: + - the SPI polarity and maximum speed for a SPI controller + - the I2C speed to use for an I2C device + - the number of GPIOs available in a GPIO device + - Note this can be parsed from the Device Tree (see below) + Where does the platform data come from? See demo-pdata.c which sets up a table of driver names and their associated platform data. The data can be interpreted by the drivers however they like - it is @@ -259,21 +296,30 @@ following device tree fragment: sides = <4>; };
+This means that instead of having lots of U_BOOT_DEVICE() declarations in +the board file, we put these in the device tree. The allows a lot more +generality, since the same board file can support many types of boards (e,g. +with the same SoC) just by using different device trees. An added benefit +is that the Linux device tree can be used, thus further simplifying the +task of board-bring up either for U-Boot or Linux devs (whoever gets to the +baord first!).
The easiest way to make this work it to add a few members to the driver:
.platdata_auto_alloc_size = sizeof(struct dm_test_pdata), .ofdata_to_platdata = testfdt_ofdata_to_platdata, - .probe = testfdt_drv_probe,
The 'auto_alloc' feature allowed space for the platdata to be allocated and zeroed before the driver's ofdata_to_platdata method is called. This -method reads the information out of the device tree and puts it in -dev->platdata. Then the probe method is called to set up the device. +method (which the driver writer supplies) should read the information out +of the device tree and puts it in dev->platdata. Thus when the probe method +is called later (to set up the device ready for use) the platform data will +be present.
Note that both methods are optional. If you provide an ofdata_to_platdata -method then it wlil be called first (after bind). If you provide a probe -method it will be called next. +method then it wlil be called first (during activation). If you provide a +probe method it will be called next. See Driver Lifecycle below for more +details.
If you don't want to have the platdata automatically allocated then you can leave out platdata_auto_alloc_size. In this case you can use malloc @@ -295,6 +341,145 @@ numbering comes from include/dm/uclass.h. To add a new uclass, add to the end of the enum there, then declare your uclass as above.
+Driver Lifecycle +---------------- + +Here are the stages that a device goes through in driver model. Note that all +methods mentioned here are optional - e.g. if there is no probe() method for +a device then it will not be called. A simple device may have very few +methods actually defined. + +1. U-Boot scans the U_BOOT_DEVICE() declarations. It looks up the name +specified by each, to find the appropriate driver. It then calls +device_bind() to create a new device and bind' it to its driver. This will +call the device's bind() method. + +2. U-Boot scans through top-level nodes in the the device tree. It looks +at the compatible string in each node and uses the of_match part of the +U_BOOT_DRIVER() structure to find the right driver for each node. It then +calls device_bind() to bind the newly-created device to its driver (thereby +creating a device structure). This will also call the device's bind() +method. + +3. At this point all the devices are known, and bound to their drivers. +There is a 'struct device' allocated for all devices. However, nothing +has been activated (except for the root device). Each bound device that +was created from a U_BOOT_DEVICE() declaration will hold the platdata +pointer specified in that declaration. For a bound device created from +the device tree, platdata will be NULL, but of_offset will be the offset +of the device tree node that caused the device to be created. The uclass +is set, and the DM_FLAG_PREFER flag is set if the device node has the +'dm,prefer' property. + +Note: The device's bind() method is permitted to perform simple actions, +but should not scan the device tree node, not initialise hardware, nor set +up structures or allocate memory. All of these tasks should be left for the +probe() method. Note that compared to Linux, U-Boot's driver model has a +separate step of probe/remove which is independent of bind/unbind. This is +partly because in U-Boot it may be expensive to prove devices and we don't +want to do it until they are needed, or perhaps until after relocation. + +4. When a device needs to be used, U-Boot activates it, by following these +steps (see device_probe()): + + a. If priv_auto_alloc_size is non-zero, then the device-private space + is allocated for the device and zeroed. It will be accessible as + dev->priv. The driver can put anything it likes in there, but should use + it for run-time information, not platform data (which should be static + and known before the device is probed). + + b. If platdata_auto_alloc_size is non-zero, then the platform data space + is allocated. This is only useful for device tree operation, since + otherwise you would have to specific the platform data in the + U_BOOT_DEVICE() declaration. The space is allocated for the device and + zeroed. It will be accessible as dev->platdata. + + c. If the device's uclass specifies a non-zero per_device_auto_alloc_size, + then this space is allocated and zeroed also. It is allocated for and + stored in the device, but it is uclass data. owned by the uclass driver. + It is possible for the device to access it. + + d. All parent devices are probed. It is not possible to activate a device + unless its parents (all the way up to the root device) are activated. + This means (for example) that an I2C driver will require that its bus + be activated. + + e. If the driver provides a ofdata_to_platdata() method, then this is + called to convert the device tree data into platform data. This should + do various calls like fdtdec_get_int(gd->fdt_blob, dev->of_offset, ...) + to access the node and store the resulting information into dev->platdata. + After this point, the device works the same way whether it was bound + using a device tree node or U_BOOT_DEVICE() structure. In either case, + the platform data is now stored in the platdata structure. Typically you + will use the platdata_auto_alloc_size feature to specify the size of the + platform data structure, and U-Boot will automatically allocate and zero + it for you before entry to ofdata_to_platdata(). But if not, you can + allocate it yourself in ofdata_to_platdata(). Note that it is preferable + to do all the device tree decoding in ofdata_to_platdata() rather than + in probe(). (Apart from the ugliness of mixing configuration and run-time + data, one day it is possible that U-Boot will cache platformat data for + devices which are regularly de/activated). + + f. The device's probe() method is called. This should do anything that + is required by the device to get it going. This could include checking + that the hardware is actually present, setting up clocks for the + hardware and setting up hardware registers to initial values. The code + in probe() can access: + + - platform data in dev->platdata (for configuration) + - private data in dev->priv (for run-time state) + - uclass data in dev->uclass_priv (for things the uclass stores + about this device) + + Note: If you don't use priv_auto_alloc_size then you will need to + allocate the priv space here yourself. The same applies also to + platdata_auto_alloc_size. Remember to free them in the remove() method. + + g. The device is marked 'activated' + + h. The uclass's post_probe() method is called, if one exists. This may + cause the uclass to do some housekeeping to record the device as + activated and 'known' by the uclass. + +5. The device is now activated and can be used. From now until it is removed +all of the above structures are accessible. The device appears in the +uclass's list of devices (so if the device is in UCLASS_GPIO it will appear +as a device in the GPIO uclass). This is the 'running' state of the device. + +6. When the device is no-longer required, you can call device_remove() to +remove it. This performs the probe steps in reverse: + + a. The uclass's pre_remove() method is called, if one exists. This may + cause the uclass to do some housekeeping to record the device as + deactivated and no-longer 'known' by the uclass. + + b. All the device's children are removed. It is not permitted to have + an active child device with a non-active parent. + + c. The device's remove() method is called. At this stage nothing has been + deallocated so platform data, private data and the uclass data will all + still be present. This is where the hardware can be shut down. It is + intended that the device be completely inactive at this point, For U-Boot + to be sure that no hardware is running, it should be enough to remove + all devices. + + d. The device memory is freed (platform data, private data, uclass data). + + Note: for a U_BOOT_DEVICE() declaration, the platform data is supplied as + a static pointer and is not allocated. For device tree, the platform + data is allocated during activation and freed during dectivation, + typically automatically using platdata_auto_alloc_size. But if that value + is 0 then U-Boot will not do the allocation/freeing and you will need to + do this yourself in your ofdata_to_platdata() and remove() methods. This + difference is tracked by the device's DM_FLAG_ALLOC_PDATA flag. + + e. The device is marked inactive. Note that it is still bound, so the + device structure itself is not freed at this point. Should the device be + activated again, then the cycle starts again at step 4 above. + +7. The device is unbound. This is the step that actually destroys the + + Data Structures ---------------
On Sat, May 24, 2014 at 4:21 PM, Simon Glass sjg@chromium.org wrote:
The lifecycle of a device is an important part of driver model. Add to the existing documentation and clarify it.
Thanks for Jon Loeliger jdl@jdl.com for helping with the text and suggesting improvements.
(Jon please comment/adjust to help clarify things further)
Clearly that line should be below the '---'. :-)
Reported-by: Jon Loeliger jdl@jdl.com
Signed-off-by: Simon Glass sjg@chromium.org
A few nits, but otherwise feel free to add my ACK as well.
Platform Data
+Platform data is like Linux platform data, if you are familiar with that. +It provides the board-specific information to start up a device.
+Why is this information not just stored in the device driver itself? The +idea is that the device driver is generic, and can in principle operate on +any board that has that type of device. For example, with modern +highly-complex SoCs it is common for the IP to come from an IP vendor, and +therefore (for example) the MMC controller may be the same on chips from +different vendors. It makes no sense to write independent drivers for the +MMC controller on each vendor's SoC, when they are all almost the same. +Similarly, we may have 6 UARTs in an SoC, all of which are mostly the same, +but lie at different addresses in the address space.
+Using the UART example, we have a single driver and it is instantiated 6 +times by supplying 6 lots of platform data. Each lot of platform data +gives the driver name and a pointer to a structure containing information +about this instance - e.g. the address of the register space. It may be that +one of the UARTS supports RS-485 operation - this can be added as a flag in +the platform data, which is set for this one port and clear for the rest.
+Think of your driver as a generic piece of code which knows how to talk to +a device, but needs to know where it is, any variant/option information and +so on. Platform data provides this link between the generic piece of code +and the specific way it is bound on a particular board.
+Examples of platform data include:
- The base address of the IP block's register space
- Configuration options, like:
- the SPI polarity and maximum speed for a SPI controller- the I2C speed to use for an I2C device- the number of GPIOs available in a GPIO device
- Note this can be parsed from the Device Tree (see below)
Not sure to what 'this' refers in that last bullet. Should that whole last line read more like:
- Data that can be parsed from the Device Tree (see below)
Where does the platform data come from? See demo-pdata.c which sets up a table of driver names and their associated platform data.
This is a weak explanation of platform data origin. At the very least we need to say it is allocated per-device if needed, and then point to the demo-pdata.c as an *example*.
The data can be interpreted by the drivers however they like - it is @@ -259,21 +296,30 @@ following device tree fragment: sides = <4>; };
+This means that instead of having lots of U_BOOT_DEVICE() declarations in +the board file, we put these in the device tree. The allows a lot more
s/The/This approach/
+generality, since the same board file can support many types of boards (e,g. +with the same SoC) just by using different device trees. An added benefit +is that the Linux device tree can be used, thus further simplifying the +task of board-bring up either for U-Boot or Linux devs (whoever gets to the +baord first!).
I'd also s/devs/developers/. But that may be just me. :-)
The easiest way to make this work it to add a few members to the driver:
.platdata_auto_alloc_size = sizeof(struct dm_test_pdata), .ofdata_to_platdata = testfdt_ofdata_to_platdata,
.probe = testfdt_drv_probe,The 'auto_alloc' feature allowed space for the platdata to be allocated and zeroed before the driver's ofdata_to_platdata method is called. This -method reads the information out of the device tree and puts it in -dev->platdata. Then the probe method is called to set up the device. +method (which the driver writer supplies) should read the information out +of the device tree and puts it in dev->platdata. Thus when the probe method
s/puts/put/..; But *which* method here? The ofdata_to_platdata()? I think that needs to be explicitly referenced in the text here:
The ofdata_to_platdata() method, which the driver write supplies, should parse the device tree node for this device and place it in the dev->platdata.
+is called later (to set up the device ready for use) the platform data will +be present.
Note that both methods are optional. If you provide an ofdata_to_platdata -method then it wlil be called first (after bind). If you provide a probe -method it will be called next. +method then it wlil be called first (during activation). If you provide a +probe method it will be called next. See Driver Lifecycle below for more +details.
If you don't want to have the platdata automatically allocated then you can leave out platdata_auto_alloc_size. In this case you can use malloc @@ -295,6 +341,145 @@ numbering comes from include/dm/uclass.h. To add a new uclass, add to the end of the enum there, then declare your uclass as above.
+Driver Lifecycle +----------------
+Here are the stages that a device goes through in driver model. Note that all +methods mentioned here are optional - e.g. if there is no probe() method for +a device then it will not be called. A simple device may have very few +methods actually defined.
+1. U-Boot scans the U_BOOT_DEVICE() declarations. It looks up the name +specified by each, to find the appropriate driver. It then calls +device_bind() to create a new device and bind' it to its driver. This will +call the device's bind() method.
+2. U-Boot scans through top-level nodes in the the device tree. It looks +at the compatible string in each node and uses the of_match part of the +U_BOOT_DRIVER() structure to find the right driver for each node. It then +calls device_bind() to bind the newly-created device to its driver (thereby +creating a device structure). This will also call the device's bind() +method.
OK. The combination of paragraph 1. and 2. confused me. It reads like a device will have bind() called on it twice. But I don't think that is true. I think a device can have bind() called for it in one of two ways: either from a direct definition of a device using U_BOOT_DEVICE(), or as a result of inspecting the driver list and pawing through the DTS for appropriate and matching nodes. If I understand that correctly, then I think we should re-word these two paragraphs (1. and 2.) as parts of a first Bind Stage:
1. Bind Step A device and its driver are bound using one of these two methods:
A) Scan the U_BOOT_DEVICE() definitions, blah blah blah.
B) Scan the DTS and patch driver definitions found in U_BOOT_DRIVER() definitions, blah blah blah.
This following paragraph describes what the effect of "Step 1. Bind Stage" does. It isn't actually a separate step in the process. So delete the "3." here:
+3. At this point all the devices are known, and bound to their drivers. +There is a 'struct device' allocated for all devices. However, nothing +has been activated (except for the root device). Each bound device that +was created from a U_BOOT_DEVICE() declaration will hold the platdata +pointer specified in that declaration. For a bound device created from +the device tree, platdata will be NULL, but of_offset will be the offset +of the device tree node that caused the device to be created. The uclass +is set, and the DM_FLAG_PREFER flag is set if the device node has the +'dm,prefer' property.
No idea what the prefer property means or causes yet....
+Note: The device's bind() method is permitted to perform simple actions, +but should not scan the device tree node, not initialise hardware, nor set +up structures or allocate memory. All of these tasks should be left for the +probe() method.
Excellent. This is a crucial aspect of the Bind operation. It is so important that it should not be a "Note:"!
And this should be a new paragraph:
Note that compared to Linux, U-Boot's driver model has a +separate step of probe/remove which is independent of bind/unbind. This is +partly because in U-Boot it may be expensive to prove devices and we don't +want to do it until they are needed, or perhaps until after relocation.
OK, good. And here really is "2. Probe Stage":
+4. When a device needs to be used, U-Boot activates it, by following these +steps (see device_probe()):
- a. If priv_auto_alloc_size is non-zero, then the device-private space
- is allocated for the device and zeroed. It will be accessible as
- dev->priv. The driver can put anything it likes in there, but should use
- it for run-time information, not platform data (which should be static
- and known before the device is probed).
- b. If platdata_auto_alloc_size is non-zero, then the platform data space
- is allocated. This is only useful for device tree operation, since
- otherwise you would have to specific the platform data in the
- U_BOOT_DEVICE() declaration. The space is allocated for the device and
- zeroed. It will be accessible as dev->platdata.
- c. If the device's uclass specifies a non-zero per_device_auto_alloc_size,
- then this space is allocated and zeroed also. It is allocated for and
- stored in the device, but it is uclass data. owned by the uclass driver.
- It is possible for the device to access it.
- d. All parent devices are probed. It is not possible to activate a device
- unless its parents (all the way up to the root device) are activated.
- This means (for example) that an I2C driver will require that its bus
- be activated.
- e. If the driver provides a ofdata_to_platdata() method, then this is
- called to convert the device tree data into platform data. This should
- do various calls like fdtdec_get_int(gd->fdt_blob, dev->of_offset, ...)
- to access the node and store the resulting information into dev->platdata.
- After this point, the device works the same way whether it was bound
- using a device tree node or U_BOOT_DEVICE() structure. In either case,
- the platform data is now stored in the platdata structure. Typically you
- will use the platdata_auto_alloc_size feature to specify the size of the
- platform data structure, and U-Boot will automatically allocate and zero
- it for you before entry to ofdata_to_platdata(). But if not, you can
- allocate it yourself in ofdata_to_platdata(). Note that it is preferable
- to do all the device tree decoding in ofdata_to_platdata() rather than
- in probe(). (Apart from the ugliness of mixing configuration and run-time
- data, one day it is possible that U-Boot will cache platformat data for
- devices which are regularly de/activated).
- f. The device's probe() method is called. This should do anything that
- is required by the device to get it going. This could include checking
- that the hardware is actually present, setting up clocks for the
- hardware and setting up hardware registers to initial values. The code
- in probe() can access:
- platform data in dev->platdata (for configuration)- private data in dev->priv (for run-time state)- uclass data in dev->uclass_priv (for things the uclass storesabout this device)- Note: If you don't use priv_auto_alloc_size then you will need to
- allocate the priv space here yourself. The same applies also to
- platdata_auto_alloc_size. Remember to free them in the remove() method.
- g. The device is marked 'activated'
- h. The uclass's post_probe() method is called, if one exists. This may
- cause the uclass to do some housekeeping to record the device as
- activated and 'known' by the uclass.
This is an excellent run of documentation. Thanks!
+5. The device is now activated and can be used. From now until it is removed +all of the above structures are accessible. The device appears in the +uclass's list of devices (so if the device is in UCLASS_GPIO it will appear +as a device in the GPIO uclass). This is the 'running' state of the device.
Good.
+6. When the device is no-longer required, you can call device_remove() to +remove it. This performs the probe steps in reverse:
- a. The uclass's pre_remove() method is called, if one exists. This may
- cause the uclass to do some housekeeping to record the device as
- deactivated and no-longer 'known' by the uclass.
- b. All the device's children are removed. It is not permitted to have
- an active child device with a non-active parent.
Is that "are removed" meant to mean device_remove() is called recursively on all the children first?
- c. The device's remove() method is called. At this stage nothing has been
- deallocated so platform data, private data and the uclass data will all
- still be present. This is where the hardware can be shut down. It is
- intended that the device be completely inactive at this point, For U-Boot
- to be sure that no hardware is running, it should be enough to remove
- all devices.
- d. The device memory is freed (platform data, private data, uclass data).
- Note: for a U_BOOT_DEVICE() declaration, the platform data is supplied as
- a static pointer and is not allocated. For device tree, the platform
- data is allocated during activation and freed during dectivation,
- typically automatically using platdata_auto_alloc_size. But if that value
- is 0 then U-Boot will not do the allocation/freeing and you will need to
- do this yourself in your ofdata_to_platdata() and remove() methods. This
- difference is tracked by the device's DM_FLAG_ALLOC_PDATA flag.
"PDATA" is slightly ambiguous: "platform data" vs "priv data". This is meant to be PLATDATA, right?
- e. The device is marked inactive. Note that it is still bound, so the
- device structure itself is not freed at this point. Should the device be
- activated again, then the cycle starts again at step 4 above.
+7. The device is unbound. This is the step that actually destroys the
Destroys the... the... the something! Dammit getting old is hell! :-)
Overall, yes! Thank you. This documentation supplies a lot of the missing knowledge about the inner workings of the Device Model.
I think there are a few lingering issues around the UCLASS structure that will need some clarification still, though.
Thanks and HTH, jdl
Hi Jon,
On 27 May 2014 09:24, Jon Loeliger loeliger@gmail.com wrote:
On Sat, May 24, 2014 at 4:21 PM, Simon Glass sjg@chromium.org wrote:
The lifecycle of a device is an important part of driver model. Add to the existing documentation and clarify it.
Thanks for Jon Loeliger jdl@jdl.com for helping with the text and suggesting improvements.
(Jon please comment/adjust to help clarify things further)
Clearly that line should be below the '---'. :-)
Reported-by: Jon Loeliger jdl@jdl.com
Signed-off-by: Simon Glass sjg@chromium.org
A few nits, but otherwise feel free to add my ACK as well.
Thanks for the comments - I'll incorporate them in a new version, but leave your Ack off for now since there are changes.
Regards, Simon
+2. U-Boot scans through top-level nodes in the the device tree. It looks +at the compatible string in each node and uses the of_match part of the +U_BOOT_DRIVER() structure to find the right driver for each node. It then
Why is the scan only on the "top level"? My GPIO nodes are, for example, under an SOC node.
Thanks, jdl
On Sat, May 24, 2014 at 4:21 PM, Simon Glass sjg@chromium.org wrote:
The lifecycle of a device is an important part of driver model. Add to the existing documentation and clarify it.
Thanks for Jon Loeliger jdl@jdl.com for helping with the text and suggesting improvements.
(Jon please comment/adjust to help clarify things further)
Reported-by: Jon Loeliger jdl@jdl.com
Signed-off-by: Simon Glass sjg@chromium.org
doc/driver-model/README.txt | 197 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 191 insertions(+), 6 deletions(-)
diff --git a/doc/driver-model/README.txt b/doc/driver-model/README.txt index deacfe9..90e0516 100644 --- a/doc/driver-model/README.txt +++ b/doc/driver-model/README.txt @@ -95,11 +95,12 @@ are provided in test/dm. To run them, try: You should see something like this:
<...U-Boot banner...>
- Running 12 driver model tests
- Running 15 driver model tests Test: dm_test_autobind Test: dm_test_autoprobe Test: dm_test_children Test: dm_test_fdt
- Test: dm_test_fdt_pre_reloc Test: dm_test_gpio sandbox_gpio: sb_gpio_get_value: error: offset 4 not reserved Test: dm_test_leak
@@ -109,6 +110,8 @@ You should see something like this: Test: dm_test_operations Test: dm_test_ordering Test: dm_test_platdata
- Test: dm_test_pre_reloc
- Test: dm_test_prefer Test: dm_test_remove Test: dm_test_uclass Failures: 0
@@ -222,6 +225,40 @@ device tree) and probe. Platform Data
+Platform data is like Linux platform data, if you are familiar with that. +It provides the board-specific information to start up a device.
+Why is this information not just stored in the device driver itself? The +idea is that the device driver is generic, and can in principle operate on +any board that has that type of device. For example, with modern +highly-complex SoCs it is common for the IP to come from an IP vendor, and +therefore (for example) the MMC controller may be the same on chips from +different vendors. It makes no sense to write independent drivers for the +MMC controller on each vendor's SoC, when they are all almost the same. +Similarly, we may have 6 UARTs in an SoC, all of which are mostly the same, +but lie at different addresses in the address space.
+Using the UART example, we have a single driver and it is instantiated 6 +times by supplying 6 lots of platform data. Each lot of platform data +gives the driver name and a pointer to a structure containing information +about this instance - e.g. the address of the register space. It may be that +one of the UARTS supports RS-485 operation - this can be added as a flag in +the platform data, which is set for this one port and clear for the rest.
+Think of your driver as a generic piece of code which knows how to talk to +a device, but needs to know where it is, any variant/option information and +so on. Platform data provides this link between the generic piece of code +and the specific way it is bound on a particular board.
+Examples of platform data include:
- The base address of the IP block's register space
- Configuration options, like:
- the SPI polarity and maximum speed for a SPI controller- the I2C speed to use for an I2C device- the number of GPIOs available in a GPIO device
- Note this can be parsed from the Device Tree (see below)
Where does the platform data come from? See demo-pdata.c which sets up a table of driver names and their associated platform data. The data can be interpreted by the drivers however they like - it is @@ -259,21 +296,30 @@ following device tree fragment: sides = <4>; };
+This means that instead of having lots of U_BOOT_DEVICE() declarations in +the board file, we put these in the device tree. The allows a lot more +generality, since the same board file can support many types of boards (e,g. +with the same SoC) just by using different device trees. An added benefit +is that the Linux device tree can be used, thus further simplifying the +task of board-bring up either for U-Boot or Linux devs (whoever gets to the +baord first!).
The easiest way to make this work it to add a few members to the driver:
.platdata_auto_alloc_size = sizeof(struct dm_test_pdata), .ofdata_to_platdata = testfdt_ofdata_to_platdata,
.probe = testfdt_drv_probe,The 'auto_alloc' feature allowed space for the platdata to be allocated and zeroed before the driver's ofdata_to_platdata method is called. This -method reads the information out of the device tree and puts it in -dev->platdata. Then the probe method is called to set up the device. +method (which the driver writer supplies) should read the information out +of the device tree and puts it in dev->platdata. Thus when the probe method +is called later (to set up the device ready for use) the platform data will +be present.
Note that both methods are optional. If you provide an ofdata_to_platdata -method then it wlil be called first (after bind). If you provide a probe -method it will be called next. +method then it wlil be called first (during activation). If you provide a +probe method it will be called next. See Driver Lifecycle below for more +details.
If you don't want to have the platdata automatically allocated then you can leave out platdata_auto_alloc_size. In this case you can use malloc @@ -295,6 +341,145 @@ numbering comes from include/dm/uclass.h. To add a new uclass, add to the end of the enum there, then declare your uclass as above.
+Driver Lifecycle +----------------
+Here are the stages that a device goes through in driver model. Note that all +methods mentioned here are optional - e.g. if there is no probe() method for +a device then it will not be called. A simple device may have very few +methods actually defined.
+1. U-Boot scans the U_BOOT_DEVICE() declarations. It looks up the name +specified by each, to find the appropriate driver. It then calls +device_bind() to create a new device and bind' it to its driver. This will +call the device's bind() method.
+2. U-Boot scans through top-level nodes in the the device tree. It looks +at the compatible string in each node and uses the of_match part of the +U_BOOT_DRIVER() structure to find the right driver for each node. It then +calls device_bind() to bind the newly-created device to its driver (thereby +creating a device structure). This will also call the device's bind() +method.
+3. At this point all the devices are known, and bound to their drivers. +There is a 'struct device' allocated for all devices. However, nothing +has been activated (except for the root device). Each bound device that +was created from a U_BOOT_DEVICE() declaration will hold the platdata +pointer specified in that declaration. For a bound device created from +the device tree, platdata will be NULL, but of_offset will be the offset +of the device tree node that caused the device to be created. The uclass +is set, and the DM_FLAG_PREFER flag is set if the device node has the +'dm,prefer' property.
+Note: The device's bind() method is permitted to perform simple actions, +but should not scan the device tree node, not initialise hardware, nor set +up structures or allocate memory. All of these tasks should be left for the +probe() method. Note that compared to Linux, U-Boot's driver model has a +separate step of probe/remove which is independent of bind/unbind. This is +partly because in U-Boot it may be expensive to prove devices and we don't +want to do it until they are needed, or perhaps until after relocation.
+4. When a device needs to be used, U-Boot activates it, by following these +steps (see device_probe()):
- a. If priv_auto_alloc_size is non-zero, then the device-private space
- is allocated for the device and zeroed. It will be accessible as
- dev->priv. The driver can put anything it likes in there, but should use
- it for run-time information, not platform data (which should be static
- and known before the device is probed).
- b. If platdata_auto_alloc_size is non-zero, then the platform data space
- is allocated. This is only useful for device tree operation, since
- otherwise you would have to specific the platform data in the
- U_BOOT_DEVICE() declaration. The space is allocated for the device and
- zeroed. It will be accessible as dev->platdata.
- c. If the device's uclass specifies a non-zero per_device_auto_alloc_size,
- then this space is allocated and zeroed also. It is allocated for and
- stored in the device, but it is uclass data. owned by the uclass driver.
- It is possible for the device to access it.
- d. All parent devices are probed. It is not possible to activate a device
- unless its parents (all the way up to the root device) are activated.
- This means (for example) that an I2C driver will require that its bus
- be activated.
- e. If the driver provides a ofdata_to_platdata() method, then this is
- called to convert the device tree data into platform data. This should
- do various calls like fdtdec_get_int(gd->fdt_blob, dev->of_offset, ...)
- to access the node and store the resulting information into dev->platdata.
- After this point, the device works the same way whether it was bound
- using a device tree node or U_BOOT_DEVICE() structure. In either case,
- the platform data is now stored in the platdata structure. Typically you
- will use the platdata_auto_alloc_size feature to specify the size of the
- platform data structure, and U-Boot will automatically allocate and zero
- it for you before entry to ofdata_to_platdata(). But if not, you can
- allocate it yourself in ofdata_to_platdata(). Note that it is preferable
- to do all the device tree decoding in ofdata_to_platdata() rather than
- in probe(). (Apart from the ugliness of mixing configuration and run-time
- data, one day it is possible that U-Boot will cache platformat data for
- devices which are regularly de/activated).
- f. The device's probe() method is called. This should do anything that
- is required by the device to get it going. This could include checking
- that the hardware is actually present, setting up clocks for the
- hardware and setting up hardware registers to initial values. The code
- in probe() can access:
- platform data in dev->platdata (for configuration)- private data in dev->priv (for run-time state)- uclass data in dev->uclass_priv (for things the uclass storesabout this device)- Note: If you don't use priv_auto_alloc_size then you will need to
- allocate the priv space here yourself. The same applies also to
- platdata_auto_alloc_size. Remember to free them in the remove() method.
- g. The device is marked 'activated'
- h. The uclass's post_probe() method is called, if one exists. This may
- cause the uclass to do some housekeeping to record the device as
- activated and 'known' by the uclass.
+5. The device is now activated and can be used. From now until it is removed +all of the above structures are accessible. The device appears in the +uclass's list of devices (so if the device is in UCLASS_GPIO it will appear +as a device in the GPIO uclass). This is the 'running' state of the device.
+6. When the device is no-longer required, you can call device_remove() to +remove it. This performs the probe steps in reverse:
- a. The uclass's pre_remove() method is called, if one exists. This may
- cause the uclass to do some housekeeping to record the device as
- deactivated and no-longer 'known' by the uclass.
- b. All the device's children are removed. It is not permitted to have
- an active child device with a non-active parent.
- c. The device's remove() method is called. At this stage nothing has been
- deallocated so platform data, private data and the uclass data will all
- still be present. This is where the hardware can be shut down. It is
- intended that the device be completely inactive at this point, For U-Boot
- to be sure that no hardware is running, it should be enough to remove
- all devices.
- d. The device memory is freed (platform data, private data, uclass data).
- Note: for a U_BOOT_DEVICE() declaration, the platform data is supplied as
- a static pointer and is not allocated. For device tree, the platform
- data is allocated during activation and freed during dectivation,
- typically automatically using platdata_auto_alloc_size. But if that value
- is 0 then U-Boot will not do the allocation/freeing and you will need to
- do this yourself in your ofdata_to_platdata() and remove() methods. This
- difference is tracked by the device's DM_FLAG_ALLOC_PDATA flag.
- e. The device is marked inactive. Note that it is still bound, so the
- device structure itself is not freed at this point. Should the device be
- activated again, then the cycle starts again at step 4 above.
+7. The device is unbound. This is the step that actually destroys the
Data Structures
-- 1.9.1.423.g4596e3a
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hi Jon,
On 27 May 2014 11:37, Jon Loeliger loeliger@gmail.com wrote:
+2. U-Boot scans through top-level nodes in the the device tree. It looks +at the compatible string in each node and uses the of_match part of the +U_BOOT_DRIVER() structure to find the right driver for each node. It then
Why is the scan only on the "top level"? My GPIO nodes are, for example, under an SOC node.
Why? Do you have an SOC driver?
Thanks, jdl
Serial devices support simple byte input/output and a few operations to find out whether data is available. Add a basic uclass for serial devices to be used by drivers that are converted to driver model.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/serial/Makefile | 4 + drivers/serial/serial-uclass.c | 161 +++++++++++++++++++++++++++++++++++++++++ include/dm/uclass-id.h | 1 + include/serial.h | 90 +++++++++++++++++++++++ 4 files changed, 256 insertions(+) create mode 100644 drivers/serial/serial-uclass.c
diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile index 571c18f..4720e1d 100644 --- a/drivers/serial/Makefile +++ b/drivers/serial/Makefile @@ -5,7 +5,11 @@ # SPDX-License-Identifier: GPL-2.0+ #
+ifdef CONFIG_DM_SERIAL +obj-y += serial-uclass.o +else obj-y += serial.o +endif
obj-$(CONFIG_ALTERA_UART) += altera_uart.o obj-$(CONFIG_ALTERA_JTAG_UART) += altera_jtag_uart.o diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c new file mode 100644 index 0000000..22bb1ef --- /dev/null +++ b/drivers/serial/serial-uclass.c @@ -0,0 +1,161 @@ +/* + * Copyright (c) 2014 The Chromium OS Authors. + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <dm.h> +#include <errno.h> +#include <os.h> +#include <serial.h> +#include <stdio_dev.h> + +DECLARE_GLOBAL_DATA_PTR; + +/* The currently selected console serial device */ +struct device *cur_dev __attribute__ ((section(".data"))); + +/* Called prior to relocation */ +int serial_init(void) +{ + if (uclass_get_preferred_device(UCLASS_SERIAL, &cur_dev)) + panic("No serial driver found"); + gd->flags |= GD_FLG_SERIAL_READY; + + return 0; +} + +/* Called after relocation */ +void serial_initialize(void) +{ + if (uclass_get_preferred_device(UCLASS_SERIAL, &cur_dev)) + panic("No serial driver found"); +} + +void serial_putc(char ch) +{ + struct dm_serial_ops *ops = serial_get_ops(cur_dev); + + ops->putc(cur_dev, ch); +} + +void serial_setbrg(void) +{ + struct dm_serial_ops *ops = serial_get_ops(cur_dev); + + if (ops->setbrg) + ops->setbrg(cur_dev, gd->baudrate); +} + +void serial_puts(const char *str) +{ + while (*str) + serial_putc(*str++); +} + +int serial_tstc(void) +{ + struct dm_serial_ops *ops = serial_get_ops(cur_dev); + + if (ops->pending) + return ops->pending(cur_dev, true); + + return 1; +} + +int serial_getc(void) +{ + struct dm_serial_ops *ops = serial_get_ops(cur_dev); + int err; + + do { + err = ops->getc(cur_dev); + } while (err == -EAGAIN); + + return err >= 0 ? err : 0; +} + +void serial_stdio_init(void) +{ +} + +void serial_stub_putc(struct stdio_dev *sdev, const char ch) +{ + struct device *dev = sdev->priv; + struct dm_serial_ops *ops = serial_get_ops(dev); + + ops->putc(dev, ch); +} + +void serial_stub_puts(struct stdio_dev *sdev, const char *str) +{ + while (*str) + serial_stub_putc(sdev, *str++); +} + +int serial_stub_getc(struct stdio_dev *sdev) +{ + struct device *dev = sdev->priv; + struct dm_serial_ops *ops = serial_get_ops(dev); + + int err; + + do { + err = ops->getc(dev); + } while (err == -EAGAIN); + + return err >= 0 ? err : 0; +} + +int serial_stub_tstc(struct stdio_dev *sdev) +{ + struct device *dev = sdev->priv; + struct dm_serial_ops *ops = serial_get_ops(dev); + + if (ops->pending) + return ops->pending(dev, true); + + return 1; +} + +static int serial_post_probe(struct device *dev) +{ + struct stdio_dev sdev, *new_sdev; + + if (!(gd->flags & GD_FLG_RELOC)) + return 0; + + memset(&sdev, 0, sizeof(dev)); + + strncpy(sdev.name, dev->name, sizeof(sdev.name)); + sdev.flags = DEV_FLAGS_OUTPUT | DEV_FLAGS_INPUT; + sdev.priv = dev; + sdev.putc = serial_stub_putc; + sdev.puts = serial_stub_puts; + sdev.getc = serial_stub_getc; + sdev.tstc = serial_stub_tstc; + stdio_register_dev(&sdev, &new_sdev); + dev->uclass_priv = new_sdev; + + return 0; +} + +static int serial_pre_remove(struct device *dev) +{ +#ifdef CONFIG_SYS_STDIO_DEREGISTER + if (stdio_deregister_dev(dev->uclass_priv)) + return -EPERM; +#endif + dev->uclass_priv = NULL; + + return 0; +} + +UCLASS_DRIVER(serial) = { + .id = UCLASS_SERIAL, + .name = "serial", + .post_probe = serial_post_probe, + .pre_remove = serial_pre_remove, + .per_device_auto_alloc_size = sizeof(struct serial_dev_priv), +}; diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index f0e691c..e0fb30a 100644 --- a/include/dm/uclass-id.h +++ b/include/dm/uclass-id.h @@ -20,6 +20,7 @@ enum uclass_id {
/* U-Boot uclasses start here */ UCLASS_GPIO, + UCLASS_SERIAL,
UCLASS_COUNT, UCLASS_INVALID = -1, diff --git a/include/serial.h b/include/serial.h index d232d47..31d1127 100644 --- a/include/serial.h +++ b/include/serial.h @@ -72,4 +72,94 @@ extern int write_port(struct stdio_dev *port, char *buf); extern int read_port(struct stdio_dev *port, char *buf, int size); #endif
+struct device; + +/** + * struct struct dm_serial_ops - Driver model serial operations + * + * The uclass interface is implemented by all serial devices which use + * driver model. + */ +struct dm_serial_ops { + /** + * setbrg() - Set up the baud rate generator + * + * Adjust baud rate divisors to set up a new baud rate for this + * device. Not all devices will support all rates. If the rate + * cannot be supported, the driver is free to select the nearest + * available rate. or return -EINVAL if this is not possible. + * + * @dev: Device pointer + * @baudrate: New baud rate to use + * @return 0 if OK, -ve on error + */ + int (*setbrg)(struct device *dev, int baudrate); + /** + * getc() - Read a character and return it + * + * If no character is available, this should return -EAGAIN without + * waiting. + * + * @dev: Device pointer + * @return character (0..255), -ve on error + */ + int (*getc)(struct device *dev); + /** + * putc() - Write a character + * + * @dev: Device pointer + * @ch: character to write + * @return 0 if OK, -ve on error + */ + int (*putc)(struct device *dev, const char ch); + /** + * pending() - Check if input/output characters are waiting + * + * This can be used to return an indication of the number of waiting + * characters if the driver knows this (e.g. by looking at the FIFO + * level). It is acceptable to return 1 if an indeterminant number + * of characters is waiting. + * + * This method is optional. + * + * @dev: Device pointer + * @input: true to check input characters, false for output + * @return number of waiting characters, 0 for none, -ve on error + */ + int (*pending)(struct device *dev, bool input); + /** + * clear() - Clear the serial FIFOs/holding registers + * + * This method is optional. + * + * This quickly clears any input/output characters from the UART. + * If this is not possible, but characters still exist, then it + * is acceptable to return -EAGAIN (try again) or -EINVAL (not + * supported). + * + * @dev: Device pointer + * @return 0 if OK, -ve on error + */ + int (*clear)(struct device *dev); +#if CONFIG_POST & CONFIG_SYS_POST_UART + /** + * loop() - Control serial device loopback mode + * + * @dev: Device pointer + * @on: 1 to turn loopback on, 0 to turn if off + */ + int (*loop)(struct device *dev, int on); +#endif +}; + +/** + * struct serial_dev_priv - information about a device used by the uclass + */ +struct serial_dev_priv { + int dummy; +}; + +/* Access the serial operations for a device */ +#define serial_get_ops(dev) ((struct dm_serial_ops *)(dev)->driver->ops) + #endif
Since driver model registers itself with the stdio subsystem, and we want to avoid delayed registration and other complexity associated with the current serial console, move the stdio subsystem init earlier when driver model is used for serial.
This simplifies the implementation. Should there be any problems with this approach they can be dealt with as boards are converted over to use driver model for serial.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/board_r.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/common/board_r.c b/common/board_r.c index 13c1bc1..b883d36 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -760,6 +760,15 @@ init_fnc_t init_sequence_r[] = { set_cpu_clk_info, /* Setup clock information */ #endif initr_reloc_global_data, + initr_barrier, + initr_malloc, + bootstage_relocate, +#ifdef CONFIG_DM_SERIAL + stdio_init, +#endif +#ifdef CONFIG_DM + initr_dm, +#endif initr_serial, initr_announce, INIT_FUNC_WATCHDOG_RESET @@ -796,12 +805,6 @@ init_fnc_t init_sequence_r[] = { #ifdef CONFIG_WINBOND_83C553 initr_w83c553f, #endif - initr_barrier, - initr_malloc, - bootstage_relocate, -#ifdef CONFIG_DM - initr_dm, -#endif #ifdef CONFIG_ARCH_EARLY_INIT_R arch_early_init_r, #endif @@ -851,7 +854,9 @@ init_fnc_t init_sequence_r[] = { */ initr_pci, #endif +#ifndef CONFIG_DM_SERIAL stdio_init, +#endif initr_jumptable, #ifdef CONFIG_API initr_api,
Adjust the sandbox serial driver to use the new driver model uclass. The driver works much as before, but within the new framework.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/serial/sandbox.c | 67 +++++++++++++++++++++++++---------------------- include/configs/sandbox.h | 3 +++ 2 files changed, 39 insertions(+), 31 deletions(-)
diff --git a/drivers/serial/sandbox.c b/drivers/serial/sandbox.c index 51fd871..3401752 100644 --- a/drivers/serial/sandbox.c +++ b/drivers/serial/sandbox.c @@ -11,12 +11,16 @@ */
#include <common.h> +#include <dm.h> +#include <fdtdec.h> #include <lcd.h> #include <os.h> #include <serial.h> #include <linux/compiler.h> #include <asm/state.h>
+DECLARE_GLOBAL_DATA_PTR; + /* * * serial_buf: A buffer that holds keyboard characters for the @@ -30,27 +34,21 @@ static char serial_buf[16]; static unsigned int serial_buf_write; static unsigned int serial_buf_read;
-static int sandbox_serial_init(void) +static int sandbox_serial_probe(struct device *dev) { struct sandbox_state *state = state_get_current();
if (state->term_raw != STATE_TERM_COOKED) os_tty_raw(0, state->term_raw == STATE_TERM_RAW_WITH_SIGS); - return 0; -}
-static void sandbox_serial_setbrg(void) -{ + return 0; }
-static void sandbox_serial_putc(const char ch) +static int sandbox_serial_putc(struct device *dev, const char ch) { os_write(1, &ch, 1); -}
-static void sandbox_serial_puts(const char *str) -{ - os_write(1, str, strlen(str)); + return 0; }
static unsigned int increment_buffer_index(unsigned int index) @@ -58,12 +56,15 @@ static unsigned int increment_buffer_index(unsigned int index) return (index + 1) % ARRAY_SIZE(serial_buf); }
-static int sandbox_serial_tstc(void) +static int sandbox_serial_pending(struct device *dev, bool input) { const unsigned int next_index = increment_buffer_index(serial_buf_write); ssize_t count;
+ if (!input) + return 0; + os_usleep(100); #ifdef CONFIG_LCD lcd_sync(); @@ -74,38 +75,42 @@ static int sandbox_serial_tstc(void) count = os_read_no_block(0, &serial_buf[serial_buf_write], 1); if (count == 1) serial_buf_write = next_index; + return serial_buf_write != serial_buf_read; }
-static int sandbox_serial_getc(void) +static int sandbox_serial_getc(struct device *dev) { int result;
- while (!sandbox_serial_tstc()) - ; /* buffer empty */ + if (!sandbox_serial_pending(dev, true)) + return -EAGAIN; /* buffer empty */
result = serial_buf[serial_buf_read]; serial_buf_read = increment_buffer_index(serial_buf_read); return result; }
-static struct serial_device sandbox_serial_drv = { - .name = "sandbox_serial", - .start = sandbox_serial_init, - .stop = NULL, - .setbrg = sandbox_serial_setbrg, - .putc = sandbox_serial_putc, - .puts = sandbox_serial_puts, - .getc = sandbox_serial_getc, - .tstc = sandbox_serial_tstc, +static const struct dm_serial_ops sandbox_serial_ops = { + .putc = sandbox_serial_putc, + .pending = sandbox_serial_pending, + .getc = sandbox_serial_getc, };
-void sandbox_serial_initialize(void) -{ - serial_register(&sandbox_serial_drv); -} +static const struct device_id sandbox_serial_ids[] = { + { .compatible = "sandbox,serial" }, + { } +};
-__weak struct serial_device *default_serial_console(void) -{ - return &sandbox_serial_drv; -} +U_BOOT_DRIVER(serial_sandbox) = { + .name = "serial_sandbox", + .id = UCLASS_SERIAL, + .of_match = sandbox_serial_ids, + .probe = sandbox_serial_probe, + .ops = &sandbox_serial_ops, + .flags = DM_FLAG_PRE_RELOC, +}; + +U_BOOT_DEVICE(serial_sandbox_non_fdt) = { + .name = "serial_sandbox", +}; diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h index 86ee70f..67521a3 100644 --- a/include/configs/sandbox.h +++ b/include/configs/sandbox.h @@ -31,6 +31,9 @@ #define CONFIG_DM_DEMO_SHAPE #define CONFIG_DM_GPIO #define CONFIG_DM_TEST +#define CONFIG_DM_SERIAL + +#define CONFIG_SYS_STDIO_DEREGISTER
/* Number of bits in a C 'long' on this architecture */ #define CONFIG_SANDBOX_BITS_PER_LONG 64
The current sandbox serial driver is a pretty trivial example and does not have the featues that might be needed for other board serial drivers. To help provide a better example, add a text colour property to the device tree for sandbox. This uses platform data, a device tree node, driver private data and a remove() method.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/serial/sandbox.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+)
diff --git a/drivers/serial/sandbox.c b/drivers/serial/sandbox.c index 3401752..d47f4f4 100644 --- a/drivers/serial/sandbox.c +++ b/drivers/serial/sandbox.c @@ -34,19 +34,67 @@ static char serial_buf[16]; static unsigned int serial_buf_write; static unsigned int serial_buf_read;
+struct sandbox_serial_platdata { + int colour; /* Text colour to use for output, -1 for none */ +}; + +struct sandbox_serial_priv { + bool start_of_line; +}; + +/** + * output_ansi_colour() - Output an ANSI colour code + * + * @colour: Colour to output (0-7) + */ +static void output_ansi_colour(int colour) +{ + char ansi_code[] = "\x1b[1;3Xm"; + + ansi_code[5] = '0' + colour; + os_write(1, ansi_code, sizeof(ansi_code) - 1); +} + +static void output_ansi_reset(void) +{ + os_write(1, "\x1b[0m", 4); +} + static int sandbox_serial_probe(struct device *dev) { struct sandbox_state *state = state_get_current(); + struct sandbox_serial_priv *priv = dev_get_priv(dev);
if (state->term_raw != STATE_TERM_COOKED) os_tty_raw(0, state->term_raw == STATE_TERM_RAW_WITH_SIGS); + priv->start_of_line = 0; + + return 0; +} + +static int sandbox_serial_remove(struct device *dev) +{ + struct sandbox_serial_platdata *plat = dev->platdata; + + if (plat->colour != -1) + output_ansi_reset();
return 0; }
static int sandbox_serial_putc(struct device *dev, const char ch) { + struct sandbox_serial_priv *priv = dev_get_priv(dev); + struct sandbox_serial_platdata *plat = dev->platdata; + + if (priv->start_of_line && plat->colour != -1) { + priv->start_of_line = false; + output_ansi_colour(plat->colour); + } + os_write(1, &ch, 1); + if (ch == '\n') + priv->start_of_line = true;
return 0; } @@ -91,6 +139,32 @@ static int sandbox_serial_getc(struct device *dev) return result; }
+static const char * const ansi_colour[] = { + "black", "red", "green", "yellow", "blue", "megenta", "cyan", + "white", +}; + +static int sandbox_serial_ofdata_to_platdata(struct device *dev) +{ + struct sandbox_serial_platdata *plat = dev->platdata; + const char *colour; + int i; + + plat->colour = -1; + colour = fdt_getprop(gd->fdt_blob, dev->of_offset, "text-colour", + NULL); + if (colour) { + for (i = 0; i < ARRAY_SIZE(ansi_colour); i++) { + if (!strcmp(colour, ansi_colour[i])) { + plat->colour = i; + break; + } + } + } + + return 0; +} + static const struct dm_serial_ops sandbox_serial_ops = { .putc = sandbox_serial_putc, .pending = sandbox_serial_pending, @@ -106,11 +180,20 @@ U_BOOT_DRIVER(serial_sandbox) = { .name = "serial_sandbox", .id = UCLASS_SERIAL, .of_match = sandbox_serial_ids, + .ofdata_to_platdata = sandbox_serial_ofdata_to_platdata, + .platdata_auto_alloc_size = sizeof(struct sandbox_serial_platdata), + .priv_auto_alloc_size = sizeof(struct sandbox_serial_priv), .probe = sandbox_serial_probe, + .remove = sandbox_serial_remove, .ops = &sandbox_serial_ops, .flags = DM_FLAG_PRE_RELOC, };
+static const struct sandbox_serial_platdata platdata_non_fdt = { + .colour = -1, +}; + U_BOOT_DEVICE(serial_sandbox_non_fdt) = { .name = "serial_sandbox", + .platdata = &platdata_non_fdt, };
If the sandbox device tree is provided to U-Boot (with the -d flag) then it will use the device tree version in preference to the built-in device. The only difference is the colour.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/sandbox/dts/sandbox.dts | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/arch/sandbox/dts/sandbox.dts b/arch/sandbox/dts/sandbox.dts index 62d8037..98008de 100644 --- a/arch/sandbox/dts/sandbox.dts +++ b/arch/sandbox/dts/sandbox.dts @@ -1,6 +1,17 @@ /dts-v1/;
/ { + alias { + console = <&uart0>; + }; + + uart0: serial { + compatible = "sandbox,serial"; + dm,pre-reloc; + dm,prefer; + text-colour = "cyan"; + }; + triangle { compatible = "demo-shape"; colour = "cyan";
participants (3)
-
Jon Loeliger -
Marek Vasut -
Simon Glass