* [U-Boot] [RFC PATCH 01/22] stdio: Remove redundant code around stdio_register() calls
2014-05-24 21:20 [U-Boot] [RFC PATCH 0/22] Introduce driver model serial uclass Simon Glass
@ 2014-05-24 21:21 ` Simon Glass
2014-06-01 17:30 ` Marek Vasut
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 02/22] stdio: Pass device pointer to stdio methods Simon Glass
` (20 subsequent siblings)
21 siblings, 1 reply; 47+ messages in thread
From: Simon Glass @ 2014-05-24 21:21 UTC (permalink / raw)
To: u-boot
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 */
--
1.9.1.423.g4596e3a
^ permalink raw reply related [flat|nested] 47+ messages in thread* [U-Boot] [RFC PATCH 02/22] stdio: Pass device pointer to stdio methods
2014-05-24 21:20 [U-Boot] [RFC PATCH 0/22] Introduce driver model serial uclass Simon Glass
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 01/22] stdio: Remove redundant code around stdio_register() calls Simon Glass
@ 2014-05-24 21:21 ` Simon Glass
2014-06-01 17:32 ` Marek Vasut
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 03/22] dm: Make sure that the root device is probed Simon Glass
` (19 subsequent siblings)
21 siblings, 1 reply; 47+ messages in thread
From: Simon Glass @ 2014-05-24 21:21 UTC (permalink / raw)
To: u-boot
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
--
1.9.1.423.g4596e3a
^ permalink raw reply related [flat|nested] 47+ messages in thread* [U-Boot] [RFC PATCH 02/22] stdio: Pass device pointer to stdio methods
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 02/22] stdio: Pass device pointer to stdio methods Simon Glass
@ 2014-06-01 17:32 ` Marek Vasut
0 siblings, 0 replies; 47+ messages in thread
From: Marek Vasut @ 2014-06-01 17:32 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 47+ messages in thread
* [U-Boot] [RFC PATCH 03/22] dm: Make sure that the root device is probed
2014-05-24 21:20 [U-Boot] [RFC PATCH 0/22] Introduce driver model serial uclass Simon Glass
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 01/22] stdio: Remove redundant code around stdio_register() calls Simon Glass
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 02/22] stdio: Pass device pointer to stdio methods Simon Glass
@ 2014-05-24 21:21 ` Simon Glass
2014-06-01 17:33 ` Marek Vasut
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 04/22] dm: Provide a way to shut down driver model Simon Glass
` (18 subsequent siblings)
21 siblings, 1 reply; 47+ messages in thread
From: Simon Glass @ 2014-05-24 21:21 UTC (permalink / raw)
To: u-boot
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
--
1.9.1.423.g4596e3a
^ permalink raw reply related [flat|nested] 47+ messages in thread* [U-Boot] [RFC PATCH 03/22] dm: Make sure that the root device is probed
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 03/22] dm: Make sure that the root device is probed Simon Glass
@ 2014-06-01 17:33 ` Marek Vasut
2014-06-03 1:59 ` Simon Glass
0 siblings, 1 reply; 47+ messages in thread
From: Marek Vasut @ 2014-06-01 17:33 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 47+ messages in thread
* [U-Boot] [RFC PATCH 03/22] dm: Make sure that the root device is probed
2014-06-01 17:33 ` Marek Vasut
@ 2014-06-03 1:59 ` Simon Glass
2014-06-03 11:25 ` Marek Vasut
0 siblings, 1 reply; 47+ messages in thread
From: Simon Glass @ 2014-06-03 1:59 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 47+ messages in thread
* [U-Boot] [RFC PATCH 03/22] dm: Make sure that the root device is probed
2014-06-03 1:59 ` Simon Glass
@ 2014-06-03 11:25 ` Marek Vasut
0 siblings, 0 replies; 47+ messages in thread
From: Marek Vasut @ 2014-06-03 11:25 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 47+ messages in thread
* [U-Boot] [RFC PATCH 04/22] dm: Provide a way to shut down driver model
2014-05-24 21:20 [U-Boot] [RFC PATCH 0/22] Introduce driver model serial uclass Simon Glass
` (2 preceding siblings ...)
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 03/22] dm: Make sure that the root device is probed Simon Glass
@ 2014-05-24 21:21 ` Simon Glass
2014-06-01 17:34 ` Marek Vasut
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 05/22] sandbox: Remove all drivers before exit Simon Glass
` (17 subsequent siblings)
21 siblings, 1 reply; 47+ messages in thread
From: Simon Glass @ 2014-05-24 21:21 UTC (permalink / raw)
To: u-boot
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
--
1.9.1.423.g4596e3a
^ permalink raw reply related [flat|nested] 47+ messages in thread* [U-Boot] [RFC PATCH 05/22] sandbox: Remove all drivers before exit
2014-05-24 21:20 [U-Boot] [RFC PATCH 0/22] Introduce driver model serial uclass Simon Glass
` (3 preceding siblings ...)
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 04/22] dm: Provide a way to shut down driver model Simon Glass
@ 2014-05-24 21:21 ` Simon Glass
2014-05-27 15:28 ` Jon Loeliger
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 06/22] dm: Tidy up four minor code nits Simon Glass
` (16 subsequent siblings)
21 siblings, 1 reply; 47+ messages in thread
From: Simon Glass @ 2014-05-24 21:21 UTC (permalink / raw)
To: u-boot
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);
}
--
1.9.1.423.g4596e3a
^ permalink raw reply related [flat|nested] 47+ messages in thread* [U-Boot] [RFC PATCH 05/22] sandbox: Remove all drivers before exit
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 05/22] sandbox: Remove all drivers before exit Simon Glass
@ 2014-05-27 15:28 ` Jon Loeliger
2014-05-27 21:05 ` Simon Glass
0 siblings, 1 reply; 47+ messages in thread
From: Jon Loeliger @ 2014-05-27 15:28 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 47+ messages in thread
* [U-Boot] [RFC PATCH 05/22] sandbox: Remove all drivers before exit
2014-05-27 15:28 ` Jon Loeliger
@ 2014-05-27 21:05 ` Simon Glass
0 siblings, 0 replies; 47+ messages in thread
From: Simon Glass @ 2014-05-27 21:05 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 47+ messages in thread
* [U-Boot] [RFC PATCH 06/22] dm: Tidy up four minor code nits
2014-05-24 21:20 [U-Boot] [RFC PATCH 0/22] Introduce driver model serial uclass Simon Glass
` (4 preceding siblings ...)
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 05/22] sandbox: Remove all drivers before exit Simon Glass
@ 2014-05-24 21:21 ` Simon Glass
2014-06-01 17:34 ` Marek Vasut
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 07/22] dm: Allow drivers to be marked 'before relocation' Simon Glass
` (15 subsequent siblings)
21 siblings, 1 reply; 47+ messages in thread
From: Simon Glass @ 2014-05-24 21:21 UTC (permalink / raw)
To: u-boot
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
--
1.9.1.423.g4596e3a
^ permalink raw reply related [flat|nested] 47+ messages in thread* [U-Boot] [RFC PATCH 07/22] dm: Allow drivers to be marked 'before relocation'
2014-05-24 21:20 [U-Boot] [RFC PATCH 0/22] Introduce driver model serial uclass Simon Glass
` (5 preceding siblings ...)
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 06/22] dm: Tidy up four minor code nits Simon Glass
@ 2014-05-24 21:21 ` Simon Glass
2014-05-27 15:36 ` Jon Loeliger
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 08/22] dm: Use '*' to indicate a device is activated Simon Glass
` (14 subsequent siblings)
21 siblings, 1 reply; 47+ messages in thread
From: Simon Glass @ 2014-05-24 21:21 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply related [flat|nested] 47+ messages in thread* [U-Boot] [RFC PATCH 07/22] dm: Allow drivers to be marked 'before relocation'
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 07/22] dm: Allow drivers to be marked 'before relocation' Simon Glass
@ 2014-05-27 15:36 ` Jon Loeliger
2014-05-27 21:09 ` Simon Glass
0 siblings, 1 reply; 47+ messages in thread
From: Jon Loeliger @ 2014-05-27 15:36 UTC (permalink / raw)
To: u-boot
> 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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
^ permalink raw reply [flat|nested] 47+ messages in thread* [U-Boot] [RFC PATCH 07/22] dm: Allow drivers to be marked 'before relocation'
2014-05-27 15:36 ` Jon Loeliger
@ 2014-05-27 21:09 ` Simon Glass
0 siblings, 0 replies; 47+ messages in thread
From: Simon Glass @ 2014-05-27 21:09 UTC (permalink / raw)
To: 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 at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
^ permalink raw reply [flat|nested] 47+ messages in thread
* [U-Boot] [RFC PATCH 08/22] dm: Use '*' to indicate a device is activated
2014-05-24 21:20 [U-Boot] [RFC PATCH 0/22] Introduce driver model serial uclass Simon Glass
` (6 preceding siblings ...)
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 07/22] dm: Allow drivers to be marked 'before relocation' Simon Glass
@ 2014-05-24 21:21 ` Simon Glass
2014-05-27 14:25 ` Jon Loeliger
2014-06-21 16:14 ` Simon Glass
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 09/22] Remove form-feeds from dlmalloc.c Simon Glass
` (13 subsequent siblings)
21 siblings, 2 replies; 47+ messages in thread
From: Simon Glass @ 2014-05-24 21:21 UTC (permalink / raw)
To: 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");
}
--
1.9.1.423.g4596e3a
^ permalink raw reply related [flat|nested] 47+ messages in thread* [U-Boot] [RFC PATCH 08/22] dm: Use '*' to indicate a device is activated
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 08/22] dm: Use '*' to indicate a device is activated Simon Glass
@ 2014-05-27 14:25 ` Jon Loeliger
2014-05-30 14:37 ` Simon Glass
2014-06-01 17:35 ` Marek Vasut
2014-06-21 16:14 ` Simon Glass
1 sibling, 2 replies; 47+ messages in thread
From: Jon Loeliger @ 2014-05-27 14:25 UTC (permalink / raw)
To: u-boot
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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
^ permalink raw reply [flat|nested] 47+ messages in thread* [U-Boot] [RFC PATCH 08/22] dm: Use '*' to indicate a device is activated
2014-05-27 14:25 ` Jon Loeliger
@ 2014-05-30 14:37 ` Simon Glass
2014-06-01 17:35 ` Marek Vasut
1 sibling, 0 replies; 47+ messages in thread
From: Simon Glass @ 2014-05-30 14:37 UTC (permalink / raw)
To: 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
^ permalink raw reply [flat|nested] 47+ messages in thread* [U-Boot] [RFC PATCH 08/22] dm: Use '*' to indicate a device is activated
2014-05-27 14:25 ` Jon Loeliger
2014-05-30 14:37 ` Simon Glass
@ 2014-06-01 17:35 ` Marek Vasut
1 sibling, 0 replies; 47+ messages in thread
From: Marek Vasut @ 2014-06-01 17:35 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 47+ messages in thread
* [U-Boot] [RFC PATCH 08/22] dm: Use '*' to indicate a device is activated
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 08/22] dm: Use '*' to indicate a device is activated Simon Glass
2014-05-27 14:25 ` Jon Loeliger
@ 2014-06-21 16:14 ` Simon Glass
1 sibling, 0 replies; 47+ messages in thread
From: Simon Glass @ 2014-06-21 16:14 UTC (permalink / raw)
To: u-boot
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.
^ permalink raw reply [flat|nested] 47+ messages in thread
* [U-Boot] [RFC PATCH 09/22] Remove form-feeds from dlmalloc.c
2014-05-24 21:20 [U-Boot] [RFC PATCH 0/22] Introduce driver model serial uclass Simon Glass
` (7 preceding siblings ...)
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 08/22] dm: Use '*' to indicate a device is activated Simon Glass
@ 2014-05-24 21:21 ` Simon Glass
2014-05-27 14:22 ` Jon Loeliger
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 10/22] Add a simple malloc() implementation for pre-relocation Simon Glass
` (12 subsequent siblings)
21 siblings, 1 reply; 47+ messages in thread
From: Simon Glass @ 2014-05-24 21:21 UTC (permalink / raw)
To: u-boot
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 @@
*/
-\f
+
/* Preliminaries */
@@ -1132,7 +1132,7 @@ gAllocatedSize))
#endif
-\f
+
/*
Type declarations
@@ -1272,7 +1272,7 @@ nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
serviced via calls to mmap, and then later released via munmap.
*/
-\f
+
/* 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)
-\f
+
/*
Physical chunk operations
@@ -1332,7 +1332,7 @@ nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
#define chunk_at_offset(p, s) ((mchunkptr)(((char*)(p)) + (s)))
-\f
+
/*
Dealing with use bits
@@ -1371,7 +1371,7 @@ nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
(((mchunkptr)(((char*)(p)) + (s)))->size &= ~(PREV_INUSE))
-\f
+
/*
Dealing with size fields
@@ -1394,7 +1394,7 @@ nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
#define set_foot(p, s) (((mchunkptr)((char*)(p) + (s)))->prev_size = (s))
-\f
+
/*
@@ -1566,7 +1566,7 @@ void mem_malloc_init(ulong start, ulong size)
#define is_small_request(nb) (nb < MAX_SMALLBIN_SIZE - SMALLBIN_WIDTH)
-\f
+
/*
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))))
-\f
+
/* Other static bookkeeping data */
@@ -1628,7 +1628,7 @@ static unsigned int max_n_mmaps = 0;
static unsigned long max_mmapped_mem = 0;
#endif
-\f
+
/*
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
-\f
+
/*
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)
-\f
+
/* Routines dealing with mmap(). */
@@ -1972,7 +1972,7 @@ static mchunkptr mremap_chunk(p, new_size) mchunkptr p; size_t new_size;
#endif /* HAVE_MMAP */
-\f
+
/*
Extend the top-most chunk by obtaining memory from system.
@@ -2089,7 +2089,7 @@ static void malloc_extend_top(nb) INTERNAL_SIZE_T nb;
}
-\f
+
/* Main public routines */
@@ -2396,7 +2396,7 @@ Void_t* mALLOc(bytes) size_t bytes;
}
-\f
+
/*
@@ -2513,7 +2513,7 @@ void fREe(mem) Void_t* mem;
}
-\f
+
/*
@@ -2750,7 +2750,7 @@ Void_t* rEALLOc(oldmem, bytes) Void_t* oldmem; size_t bytes;
}
-\f
+
/*
@@ -2868,7 +2868,7 @@ Void_t* mEMALIGn(alignment, bytes) size_t alignment; size_t bytes;
}
-\f
+
/*
@@ -2975,7 +2975,7 @@ void cfree(mem) Void_t *mem;
}
#endif
-\f
+
/*
@@ -3056,7 +3056,7 @@ int malloc_trim(pad) size_t pad;
}
}
-\f
+
/*
malloc_usable_size:
@@ -3092,7 +3092,7 @@ size_t malloc_usable_size(mem) Void_t* mem;
}
-\f
+
/* Utility to update current_mallinfo for malloc_stats and mallinfo() */
@@ -3136,7 +3136,7 @@ static void malloc_update_mallinfo()
}
#endif /* DEBUG */
-\f
+
/*
@@ -3183,7 +3183,7 @@ struct mallinfo mALLINFo()
#endif /* DEBUG */
-\f
+
/*
mallopt:
--
1.9.1.423.g4596e3a
^ permalink raw reply related [flat|nested] 47+ messages in thread* [U-Boot] [RFC PATCH 09/22] Remove form-feeds from dlmalloc.c
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 09/22] Remove form-feeds from dlmalloc.c Simon Glass
@ 2014-05-27 14:22 ` Jon Loeliger
0 siblings, 0 replies; 47+ messages in thread
From: Jon Loeliger @ 2014-05-27 14:22 UTC (permalink / raw)
To: u-boot
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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
^ permalink raw reply [flat|nested] 47+ messages in thread
* [U-Boot] [RFC PATCH 10/22] Add a simple malloc() implementation for pre-relocation
2014-05-24 21:20 [U-Boot] [RFC PATCH 0/22] Introduce driver model serial uclass Simon Glass
` (8 preceding siblings ...)
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 09/22] Remove form-feeds from dlmalloc.c Simon Glass
@ 2014-05-24 21:21 ` Simon Glass
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 11/22] sandbox: config: Enable pre-relocation malloc() Simon Glass
` (11 subsequent siblings)
21 siblings, 0 replies; 47+ messages in thread
From: Simon Glass @ 2014-05-24 21:21 UTC (permalink / raw)
To: 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
--
1.9.1.423.g4596e3a
^ permalink raw reply related [flat|nested] 47+ messages in thread* [U-Boot] [RFC PATCH 11/22] sandbox: config: Enable pre-relocation malloc()
2014-05-24 21:20 [U-Boot] [RFC PATCH 0/22] Introduce driver model serial uclass Simon Glass
` (9 preceding siblings ...)
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 10/22] Add a simple malloc() implementation for pre-relocation Simon Glass
@ 2014-05-24 21:21 ` Simon Glass
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 12/22] dm: Support driver model prior to relocation Simon Glass
` (10 subsequent siblings)
21 siblings, 0 replies; 47+ messages in thread
From: Simon Glass @ 2014-05-24 21:21 UTC (permalink / raw)
To: u-boot
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}
--
1.9.1.423.g4596e3a
^ permalink raw reply related [flat|nested] 47+ messages in thread* [U-Boot] [RFC PATCH 12/22] dm: Support driver model prior to relocation
2014-05-24 21:20 [U-Boot] [RFC PATCH 0/22] Introduce driver model serial uclass Simon Glass
` (10 preceding siblings ...)
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 11/22] sandbox: config: Enable pre-relocation malloc() Simon Glass
@ 2014-05-24 21:21 ` Simon Glass
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 13/22] stdio: Provide functions to add/remove devices using stdio_dev Simon Glass
` (9 subsequent siblings)
21 siblings, 0 replies; 47+ messages in thread
From: Simon Glass @ 2014-05-24 21:21 UTC (permalink / raw)
To: u-boot
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
--
1.9.1.423.g4596e3a
^ permalink raw reply related [flat|nested] 47+ messages in thread* [U-Boot] [RFC PATCH 13/22] stdio: Provide functions to add/remove devices using stdio_dev
2014-05-24 21:20 [U-Boot] [RFC PATCH 0/22] Introduce driver model serial uclass Simon Glass
` (11 preceding siblings ...)
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 12/22] dm: Support driver model prior to relocation Simon Glass
@ 2014-05-24 21:21 ` Simon Glass
2014-06-01 17:37 ` Marek Vasut
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 14/22] console: Remove vprintf() optimisation for sandbox Simon Glass
` (8 subsequent siblings)
21 siblings, 1 reply; 47+ messages in thread
From: Simon Glass @ 2014-05-24 21:21 UTC (permalink / raw)
To: u-boot
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);
--
1.9.1.423.g4596e3a
^ permalink raw reply related [flat|nested] 47+ messages in thread* [U-Boot] [RFC PATCH 14/22] console: Remove vprintf() optimisation for sandbox
2014-05-24 21:20 [U-Boot] [RFC PATCH 0/22] Introduce driver model serial uclass Simon Glass
` (12 preceding siblings ...)
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 13/22] stdio: Provide functions to add/remove devices using stdio_dev Simon Glass
@ 2014-05-24 21:21 ` Simon Glass
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 15/22] Add a flag indicating when the serial console is ready Simon Glass
` (7 subsequent siblings)
21 siblings, 0 replies; 47+ messages in thread
From: Simon Glass @ 2014-05-24 21:21 UTC (permalink / raw)
To: u-boot
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
--
1.9.1.423.g4596e3a
^ permalink raw reply related [flat|nested] 47+ messages in thread* [U-Boot] [RFC PATCH 15/22] Add a flag indicating when the serial console is ready
2014-05-24 21:20 [U-Boot] [RFC PATCH 0/22] Introduce driver model serial uclass Simon Glass
` (13 preceding siblings ...)
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 14/22] console: Remove vprintf() optimisation for sandbox Simon Glass
@ 2014-05-24 21:21 ` Simon Glass
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 16/22] dm: Add a way to indicate a preferred device within a uclass Simon Glass
` (6 subsequent siblings)
21 siblings, 0 replies; 47+ messages in thread
From: Simon Glass @ 2014-05-24 21:21 UTC (permalink / raw)
To: u-boot
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 */
--
1.9.1.423.g4596e3a
^ permalink raw reply related [flat|nested] 47+ messages in thread* [U-Boot] [RFC PATCH 16/22] dm: Add a way to indicate a preferred device within a uclass
2014-05-24 21:20 [U-Boot] [RFC PATCH 0/22] Introduce driver model serial uclass Simon Glass
` (14 preceding siblings ...)
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 15/22] Add a flag indicating when the serial console is ready Simon Glass
@ 2014-05-24 21:21 ` Simon Glass
2014-05-27 15:42 ` Jon Loeliger
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 17/22] dm: Expand and improve the device lifecycle docs Simon Glass
` (5 subsequent siblings)
21 siblings, 1 reply; 47+ messages in thread
From: Simon Glass @ 2014-05-24 21:21 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply related [flat|nested] 47+ messages in thread* [U-Boot] [RFC PATCH 16/22] dm: Add a way to indicate a preferred device within a uclass
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 16/22] dm: Add a way to indicate a preferred device within a uclass Simon Glass
@ 2014-05-27 15:42 ` Jon Loeliger
2014-05-30 14:43 ` Simon Glass
0 siblings, 1 reply; 47+ messages in thread
From: Jon Loeliger @ 2014-05-27 15:42 UTC (permalink / raw)
To: u-boot
> 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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
^ permalink raw reply [flat|nested] 47+ messages in thread* [U-Boot] [RFC PATCH 16/22] dm: Add a way to indicate a preferred device within a uclass
2014-05-27 15:42 ` Jon Loeliger
@ 2014-05-30 14:43 ` Simon Glass
0 siblings, 0 replies; 47+ messages in thread
From: Simon Glass @ 2014-05-30 14:43 UTC (permalink / raw)
To: 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
^ permalink raw reply [flat|nested] 47+ messages in thread
* [U-Boot] [RFC PATCH 17/22] dm: Expand and improve the device lifecycle docs
2014-05-24 21:20 [U-Boot] [RFC PATCH 0/22] Introduce driver model serial uclass Simon Glass
` (15 preceding siblings ...)
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 16/22] dm: Add a way to indicate a preferred device within a uclass Simon Glass
@ 2014-05-24 21:21 ` Simon Glass
2014-05-27 15:24 ` Jon Loeliger
2014-05-27 17:37 ` Jon Loeliger
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 18/22] dm: Add a uclass for serial devices Simon Glass
` (4 subsequent siblings)
21 siblings, 2 replies; 47+ messages in thread
From: Simon Glass @ 2014-05-24 21:21 UTC (permalink / raw)
To: u-boot
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
---------------
--
1.9.1.423.g4596e3a
^ permalink raw reply related [flat|nested] 47+ messages in thread* [U-Boot] [RFC PATCH 17/22] dm: Expand and improve the device lifecycle docs
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 17/22] dm: Expand and improve the device lifecycle docs Simon Glass
@ 2014-05-27 15:24 ` Jon Loeliger
2014-05-30 15:11 ` Simon Glass
2014-05-27 17:37 ` Jon Loeliger
1 sibling, 1 reply; 47+ messages in thread
From: Jon Loeliger @ 2014-05-27 15:24 UTC (permalink / raw)
To: u-boot
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 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.
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
^ permalink raw reply [flat|nested] 47+ messages in thread* [U-Boot] [RFC PATCH 17/22] dm: Expand and improve the device lifecycle docs
2014-05-27 15:24 ` Jon Loeliger
@ 2014-05-30 15:11 ` Simon Glass
0 siblings, 0 replies; 47+ messages in thread
From: Simon Glass @ 2014-05-30 15:11 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 47+ messages in thread
* [U-Boot] [RFC PATCH 17/22] dm: Expand and improve the device lifecycle docs
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 17/22] dm: Expand and improve the device lifecycle docs Simon Glass
2014-05-27 15:24 ` Jon Loeliger
@ 2014-05-27 17:37 ` Jon Loeliger
2014-05-30 15:12 ` Simon Glass
1 sibling, 1 reply; 47+ messages in thread
From: Jon Loeliger @ 2014-05-27 17:37 UTC (permalink / raw)
To: u-boot
> +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 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
> ---------------
>
> --
> 1.9.1.423.g4596e3a
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
^ permalink raw reply [flat|nested] 47+ messages in thread
* [U-Boot] [RFC PATCH 18/22] dm: Add a uclass for serial devices
2014-05-24 21:20 [U-Boot] [RFC PATCH 0/22] Introduce driver model serial uclass Simon Glass
` (16 preceding siblings ...)
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 17/22] dm: Expand and improve the device lifecycle docs Simon Glass
@ 2014-05-24 21:21 ` Simon Glass
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 19/22] Set up stdio earlier when using driver model Simon Glass
` (3 subsequent siblings)
21 siblings, 0 replies; 47+ messages in thread
From: Simon Glass @ 2014-05-24 21:21 UTC (permalink / raw)
To: u-boot
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
--
1.9.1.423.g4596e3a
^ permalink raw reply related [flat|nested] 47+ messages in thread* [U-Boot] [RFC PATCH 19/22] Set up stdio earlier when using driver model
2014-05-24 21:20 [U-Boot] [RFC PATCH 0/22] Introduce driver model serial uclass Simon Glass
` (17 preceding siblings ...)
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 18/22] dm: Add a uclass for serial devices Simon Glass
@ 2014-05-24 21:21 ` Simon Glass
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 20/22] sandbox: Convert serial driver to use " Simon Glass
` (2 subsequent siblings)
21 siblings, 0 replies; 47+ messages in thread
From: Simon Glass @ 2014-05-24 21:21 UTC (permalink / raw)
To: u-boot
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,
--
1.9.1.423.g4596e3a
^ permalink raw reply related [flat|nested] 47+ messages in thread* [U-Boot] [RFC PATCH 20/22] sandbox: Convert serial driver to use driver model
2014-05-24 21:20 [U-Boot] [RFC PATCH 0/22] Introduce driver model serial uclass Simon Glass
` (18 preceding siblings ...)
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 19/22] Set up stdio earlier when using driver model Simon Glass
@ 2014-05-24 21:21 ` Simon Glass
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 21/22] sandbox: serial: Support a coloured console Simon Glass
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 22/22] sandbox: dts: Add a serial console node Simon Glass
21 siblings, 0 replies; 47+ messages in thread
From: Simon Glass @ 2014-05-24 21:21 UTC (permalink / raw)
To: u-boot
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
--
1.9.1.423.g4596e3a
^ permalink raw reply related [flat|nested] 47+ messages in thread* [U-Boot] [RFC PATCH 21/22] sandbox: serial: Support a coloured console
2014-05-24 21:20 [U-Boot] [RFC PATCH 0/22] Introduce driver model serial uclass Simon Glass
` (19 preceding siblings ...)
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 20/22] sandbox: Convert serial driver to use " Simon Glass
@ 2014-05-24 21:21 ` Simon Glass
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 22/22] sandbox: dts: Add a serial console node Simon Glass
21 siblings, 0 replies; 47+ messages in thread
From: Simon Glass @ 2014-05-24 21:21 UTC (permalink / raw)
To: u-boot
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,
};
--
1.9.1.423.g4596e3a
^ permalink raw reply related [flat|nested] 47+ messages in thread* [U-Boot] [RFC PATCH 22/22] sandbox: dts: Add a serial console node
2014-05-24 21:20 [U-Boot] [RFC PATCH 0/22] Introduce driver model serial uclass Simon Glass
` (20 preceding siblings ...)
2014-05-24 21:21 ` [U-Boot] [RFC PATCH 21/22] sandbox: serial: Support a coloured console Simon Glass
@ 2014-05-24 21:21 ` Simon Glass
21 siblings, 0 replies; 47+ messages in thread
From: Simon Glass @ 2014-05-24 21:21 UTC (permalink / raw)
To: u-boot
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";
--
1.9.1.423.g4596e3a
^ permalink raw reply related [flat|nested] 47+ messages in thread