* [U-Boot] [PATCH v2 01/26] sandbox: Add a way to skip time delays
2015-11-09 6:47 [U-Boot] [PATCH v2 00/26] usb: Drop requirement for USB unbinding, add tests Simon Glass
@ 2015-11-09 6:47 ` Simon Glass
2015-11-20 3:30 ` Simon Glass
2015-11-09 6:47 ` [U-Boot] [PATCH v2 02/26] dm: usb: Avoid time delays in sandbox tests Simon Glass
` (26 subsequent siblings)
27 siblings, 1 reply; 67+ messages in thread
From: Simon Glass @ 2015-11-09 6:47 UTC (permalink / raw)
To: u-boot
Some tests are slow due to delays which are unnecessary on sandbox. The
worst offender is USB where we lose two seconds. Add a way to disable time
delays.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None
arch/sandbox/cpu/cpu.c | 5 ++++-
arch/sandbox/cpu/state.c | 14 ++++++++++++++
arch/sandbox/include/asm/state.h | 19 +++++++++++++++++++
3 files changed, 37 insertions(+), 1 deletion(-)
diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c
index 3a7f5a0..196f3e1 100644
--- a/arch/sandbox/cpu/cpu.c
+++ b/arch/sandbox/cpu/cpu.c
@@ -37,7 +37,10 @@ void sandbox_exit(void)
/* delay x useconds */
void __udelay(unsigned long usec)
{
- os_usleep(usec);
+ struct sandbox_state *state = state_get_current();
+
+ if (!state->skip_delays)
+ os_usleep(usec);
}
int cleanup_before_linux(void)
diff --git a/arch/sandbox/cpu/state.c b/arch/sandbox/cpu/state.c
index 7e5d03e..d2a7dc9 100644
--- a/arch/sandbox/cpu/state.c
+++ b/arch/sandbox/cpu/state.c
@@ -337,6 +337,20 @@ struct sandbox_state *state_get_current(void)
return state;
}
+void state_set_skip_delays(bool skip_delays)
+{
+ struct sandbox_state *state = state_get_current();
+
+ state->skip_delays = skip_delays;
+}
+
+bool state_get_skip_delays(void)
+{
+ struct sandbox_state *state = state_get_current();
+
+ return state->skip_delays;
+}
+
int state_init(void)
{
state = &main_state;
diff --git a/arch/sandbox/include/asm/state.h b/arch/sandbox/include/asm/state.h
index 2bd28f6..e876ba2 100644
--- a/arch/sandbox/include/asm/state.h
+++ b/arch/sandbox/include/asm/state.h
@@ -63,6 +63,7 @@ struct sandbox_state {
enum reset_t last_reset; /* Last reset type */
bool reset_allowed[RESET_COUNT]; /* Allowed reset types */
enum state_terminal_raw term_raw; /* Terminal raw/cooked */
+ bool skip_delays; /* Ignore any time delays (for test) */
/* Pointer to information for each SPI bus/cs */
struct sandbox_spi_info spi[CONFIG_SANDBOX_SPI_MAX_BUS]
@@ -185,6 +186,24 @@ int sandbox_write_state(struct sandbox_state *state, const char *fname);
int state_setprop(int node, const char *prop_name, const void *data, int size);
/**
+ * Control skipping of time delays
+ *
+ * Some tests have unnecessay time delays (e.g. USB). Allow these to be
+ * skipped to speed up testing
+ *
+ * @param skip_delays true to skip delays from now on, false to honour delay
+ * requests
+ */
+void state_set_skip_delays(bool skip_delays);
+
+/**
+ * See if delays should be skipped
+ *
+ * @return true if delays should be skipped, false if they should be honoured
+ */
+bool state_get_skip_delays(void);
+
+/**
* Initialize the test system state
*/
int state_init(void);
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply related [flat|nested] 67+ messages in thread* [U-Boot] [PATCH v2 02/26] dm: usb: Avoid time delays in sandbox tests
2015-11-09 6:47 [U-Boot] [PATCH v2 00/26] usb: Drop requirement for USB unbinding, add tests Simon Glass
2015-11-09 6:47 ` [U-Boot] [PATCH v2 01/26] sandbox: Add a way to skip time delays Simon Glass
@ 2015-11-09 6:47 ` Simon Glass
2015-11-20 3:30 ` Simon Glass
2015-11-09 6:47 ` [U-Boot] [PATCH v2 03/26] Move console definitions into a new console.h file Simon Glass
` (25 subsequent siblings)
27 siblings, 1 reply; 67+ messages in thread
From: Simon Glass @ 2015-11-09 6:47 UTC (permalink / raw)
To: u-boot
Currently the USB tests take around two seconds to run. Remove these
unnecessary time delays so that the tests run quickly.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None
common/usb_hub.c | 10 +++++++++-
test/dm/test-main.c | 2 ++
test/dm/usb.c | 3 +++
3 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/common/usb_hub.c b/common/usb_hub.c
index a92c9fb..e1de813 100644
--- a/common/usb_hub.c
+++ b/common/usb_hub.c
@@ -31,6 +31,9 @@
#include <asm/unaligned.h>
#include <linux/ctype.h>
#include <asm/byteorder.h>
+#ifdef CONFIG_SANDBOX
+#include <asm/state.h>
+#endif
#include <asm/unaligned.h>
#include <dm/root.h>
@@ -466,7 +469,12 @@ static int usb_hub_configure(struct usb_device *dev)
unsigned short portstatus, portchange;
int ret;
ulong start = get_timer(0);
+ uint delay = CONFIG_SYS_HZ;
+#ifdef CONFIG_SANDBOX
+ if (state_get_skip_delays())
+ delay = 0;
+#endif
#ifdef CONFIG_DM_USB
debug("\n\nScanning '%s' port %d\n", dev->dev->name, i + 1);
#else
@@ -498,7 +506,7 @@ static int usb_hub_configure(struct usb_device *dev)
if (portstatus & USB_PORT_STAT_CONNECTION)
break;
- } while (get_timer(start) < CONFIG_SYS_HZ * 1);
+ } while (get_timer(start) < delay);
if (ret < 0)
continue;
diff --git a/test/dm/test-main.c b/test/dm/test-main.c
index 0e43ab9..a36a9c0 100644
--- a/test/dm/test-main.c
+++ b/test/dm/test-main.c
@@ -9,6 +9,7 @@
#include <dm.h>
#include <errno.h>
#include <malloc.h>
+#include <asm/state.h>
#include <dm/test.h>
#include <dm/root.h>
#include <dm/uclass-internal.h>
@@ -113,6 +114,7 @@ static int dm_test_main(const char *test_name)
ut_assertok(dm_scan_fdt(gd->fdt_blob, false));
test->func(uts);
+ state_set_skip_delays(false);
ut_assertok(dm_test_destroy(uts));
}
diff --git a/test/dm/usb.c b/test/dm/usb.c
index 9939d83..4300bbd 100644
--- a/test/dm/usb.c
+++ b/test/dm/usb.c
@@ -8,6 +8,8 @@
#include <dm.h>
#include <usb.h>
#include <asm/io.h>
+#include <asm/state.h>
+#include <dm/device-internal.h>
#include <dm/test.h>
#include <test/ut.h>
@@ -35,6 +37,7 @@ static int dm_test_usb_flash(struct unit_test_state *uts)
block_dev_desc_t *dev_desc;
char cmp[1024];
+ state_set_skip_delays(true);
ut_assertok(usb_init());
ut_assertok(uclass_get_device(UCLASS_MASS_STORAGE, 0, &dev));
ut_assertok(get_device("usb", "0", &dev_desc));
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply related [flat|nested] 67+ messages in thread* [U-Boot] [PATCH v2 03/26] Move console definitions into a new console.h file
2015-11-09 6:47 [U-Boot] [PATCH v2 00/26] usb: Drop requirement for USB unbinding, add tests Simon Glass
2015-11-09 6:47 ` [U-Boot] [PATCH v2 01/26] sandbox: Add a way to skip time delays Simon Glass
2015-11-09 6:47 ` [U-Boot] [PATCH v2 02/26] dm: usb: Avoid time delays in sandbox tests Simon Glass
@ 2015-11-09 6:47 ` Simon Glass
2015-11-20 3:31 ` Simon Glass
2015-11-09 6:47 ` [U-Boot] [PATCH v2 04/26] Drop config.h header from display_options.c Simon Glass
` (24 subsequent siblings)
27 siblings, 1 reply; 67+ messages in thread
From: Simon Glass @ 2015-11-09 6:47 UTC (permalink / raw)
To: u-boot
The console includes a global variable and several functions that are only
used by a small subset of U-Boot files. Before adding more functions, move
the definitions into their own header file.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None
arch/blackfin/lib/cmd_cache_dump.c | 1 +
arch/powerpc/cpu/mpc8260/ether_fcc.c | 1 +
arch/powerpc/cpu/mpc8260/i2c.c | 1 +
arch/powerpc/cpu/mpc8xx/i2c.c | 1 +
arch/sh/lib/board.c | 1 +
arch/sparc/lib/board.c | 1 +
board/Arcturus/ucp1020/spl.c | 1 +
board/astro/mcf5373l/fpga.c | 1 +
board/cobra5272/flash.c | 1 +
board/esd/common/cmd_loadpci.c | 1 +
board/esd/cpci405/cpci405.c | 1 +
board/esd/pmc405de/pmc405de.c | 1 +
board/esd/pmc440/cmd_pmc440.c | 1 +
board/esd/pmc440/pmc440.c | 1 +
board/esd/vme8349/caddy.c | 1 +
board/freescale/b4860qds/spl.c | 1 +
board/freescale/c29xpcie/spl.c | 1 +
board/freescale/mpc8569mds/mpc8569mds.c | 1 +
board/freescale/p1010rdb/spl.c | 1 +
board/freescale/p1022ds/spl.c | 1 +
board/freescale/p1_p2_rdb_pc/spl.c | 1 +
board/freescale/t102xqds/spl.c | 1 +
board/freescale/t102xrdb/spl.c | 1 +
board/freescale/t104xrdb/spl.c | 1 +
board/freescale/t208xqds/spl.c | 1 +
board/freescale/t208xrdb/spl.c | 1 +
board/freescale/t4qds/spl.c | 1 +
board/freescale/t4rdb/spl.c | 1 +
board/gdsys/common/cmd_ioloop.c | 1 +
board/inka4x0/inkadiag.c | 1 +
board/lwmon5/kbd.c | 1 +
board/mpl/common/kbd.c | 1 +
board/mpl/pati/pati.c | 1 +
board/renesas/sh7785lcr/selfcheck.c | 1 +
board/tqc/tqm5200/cmd_stk52xx.c | 1 +
board/tqc/tqm5200/tqm5200.c | 1 +
board/xes/xpedite1000/xpedite1000.c | 1 +
common/autoboot.c | 1 +
common/board_f.c | 1 +
common/board_r.c | 1 +
common/cli.c | 1 +
common/cli_hush.c | 1 +
common/cli_simple.c | 1 +
common/cmd_armflash.c | 1 +
common/cmd_dcr.c | 1 +
common/cmd_dfu.c | 1 +
common/cmd_fastboot.c | 1 +
common/cmd_fpgad.c | 1 +
common/cmd_fuse.c | 1 +
common/cmd_i2c.c | 1 +
common/cmd_load.c | 1 +
common/cmd_mem.c | 2 ++
common/cmd_misc.c | 1 +
common/cmd_mmc.c | 1 +
common/cmd_nand.c | 1 +
common/cmd_nvedit.c | 1 +
common/cmd_otp.c | 1 +
common/cmd_pci.c | 1 +
common/cmd_usb.c | 1 +
common/cmd_usb_mass_storage.c | 1 +
common/command.c | 1 +
common/console.c | 1 +
common/iomux.c | 1 +
common/main.c | 1 +
common/usb_kbd.c | 1 +
drivers/block/fsl_sata.c | 1 +
drivers/fpga/ACEX1K.c | 1 +
drivers/fpga/virtex2.c | 1 +
drivers/fpga/zynqpl.c | 1 +
drivers/i2c/adi_i2c.c | 1 +
drivers/input/input.c | 1 +
drivers/input/keyboard.c | 1 +
drivers/misc/cbmem_console.c | 2 +-
drivers/mtd/cfi_flash.c | 1 +
drivers/mtd/nand/bfin_nand.c | 1 +
drivers/net/e1000_spi.c | 1 +
drivers/net/keystone_net.c | 1 +
drivers/net/phy/phy.c | 1 +
drivers/net/vsc7385.c | 1 +
drivers/power/battery/bat_trats.c | 1 +
drivers/spi/bfin_spi.c | 1 +
drivers/spi/bfin_spi6xx.c | 1 +
drivers/spi/sh_qspi.c | 1 +
drivers/spi/sh_spi.c | 1 +
drivers/usb/gadget/ether.c | 1 +
drivers/usb/gadget/f_mass_storage.c | 1 +
drivers/usb/gadget/f_thor.c | 1 +
drivers/usb/host/r8a66597-hcd.c | 1 +
drivers/usb/musb-new/musb_uboot.c | 1 +
examples/standalone/mem_to_mem_idma2intr.c | 1 +
examples/standalone/smc911x_eeprom.c | 1 +
include/common.h | 17 -----------------
include/console.h | 30 ++++++++++++++++++++++++++++++
lib/display_options.c | 1 +
lib/gunzip.c | 1 +
net/net.c | 1 +
test/dm/test-main.c | 1 +
test/dm/usb.c | 1 +
98 files changed, 127 insertions(+), 18 deletions(-)
create mode 100644 include/console.h
diff --git a/arch/blackfin/lib/cmd_cache_dump.c b/arch/blackfin/lib/cmd_cache_dump.c
index 412e019..f9f9097 100644
--- a/arch/blackfin/lib/cmd_cache_dump.c
+++ b/arch/blackfin/lib/cmd_cache_dump.c
@@ -9,6 +9,7 @@
#include <config.h>
#include <common.h>
#include <command.h>
+#include <console.h>
#include <asm/blackfin.h>
#include <asm/mach-common/bits/mpu.h>
diff --git a/arch/powerpc/cpu/mpc8260/ether_fcc.c b/arch/powerpc/cpu/mpc8260/ether_fcc.c
index 30ea3de..9bb395e 100644
--- a/arch/powerpc/cpu/mpc8260/ether_fcc.c
+++ b/arch/powerpc/cpu/mpc8260/ether_fcc.c
@@ -24,6 +24,7 @@
*/
#include <common.h>
+#include <console.h>
#include <malloc.h>
#include <asm/cpm_8260.h>
#include <mpc8260.h>
diff --git a/arch/powerpc/cpu/mpc8260/i2c.c b/arch/powerpc/cpu/mpc8260/i2c.c
index 8658ebd..a0de101 100644
--- a/arch/powerpc/cpu/mpc8260/i2c.c
+++ b/arch/powerpc/cpu/mpc8260/i2c.c
@@ -9,6 +9,7 @@
*/
#include <common.h>
+#include <console.h>
#if defined(CONFIG_HARD_I2C)
diff --git a/arch/powerpc/cpu/mpc8xx/i2c.c b/arch/powerpc/cpu/mpc8xx/i2c.c
index 6146de3..3dff4ab 100644
--- a/arch/powerpc/cpu/mpc8xx/i2c.c
+++ b/arch/powerpc/cpu/mpc8xx/i2c.c
@@ -12,6 +12,7 @@
*/
#include <common.h>
+#include <console.h>
#ifdef CONFIG_HARD_I2C
diff --git a/arch/sh/lib/board.c b/arch/sh/lib/board.c
index 6dad3c7..69cdca3 100644
--- a/arch/sh/lib/board.c
+++ b/arch/sh/lib/board.c
@@ -7,6 +7,7 @@
#include <common.h>
#include <command.h>
+#include <console.h>
#include <malloc.h>
#include <stdio_dev.h>
#include <version.h>
diff --git a/arch/sparc/lib/board.c b/arch/sparc/lib/board.c
index d2ac6bc..10475d1 100644
--- a/arch/sparc/lib/board.c
+++ b/arch/sparc/lib/board.c
@@ -11,6 +11,7 @@
#include <common.h>
#include <command.h>
+#include <console.h>
#include <malloc.h>
#include <stdio_dev.h>
#include <config.h>
diff --git a/board/Arcturus/ucp1020/spl.c b/board/Arcturus/ucp1020/spl.c
index 236b0d0..9315bb7 100644
--- a/board/Arcturus/ucp1020/spl.c
+++ b/board/Arcturus/ucp1020/spl.c
@@ -9,6 +9,7 @@
*/
#include <common.h>
+#include <console.h>
#include <ns16550.h>
#include <malloc.h>
#include <mmc.h>
diff --git a/board/astro/mcf5373l/fpga.c b/board/astro/mcf5373l/fpga.c
index d1110df..53e0072 100644
--- a/board/astro/mcf5373l/fpga.c
+++ b/board/astro/mcf5373l/fpga.c
@@ -15,6 +15,7 @@
/* Altera/Xilinx FPGA configuration support for the ASTRO "URMEL" board */
#include <common.h>
+#include <console.h>
#include <watchdog.h>
#include <altera.h>
#include <ACEX1K.h>
diff --git a/board/cobra5272/flash.c b/board/cobra5272/flash.c
index c0b8337..4fac688 100644
--- a/board/cobra5272/flash.c
+++ b/board/cobra5272/flash.c
@@ -6,6 +6,7 @@
*/
#include <common.h>
+#include <console.h>
#define PHYS_FLASH_1 CONFIG_SYS_FLASH_BASE
#define FLASH_BANK_SIZE 0x200000
diff --git a/board/esd/common/cmd_loadpci.c b/board/esd/common/cmd_loadpci.c
index 95d1891..9541817 100644
--- a/board/esd/common/cmd_loadpci.c
+++ b/board/esd/common/cmd_loadpci.c
@@ -7,6 +7,7 @@
#include <common.h>
#include <command.h>
+#include <console.h>
#if !defined(CONFIG_440)
#include <asm/4xx_pci.h>
#endif
diff --git a/board/esd/cpci405/cpci405.c b/board/esd/cpci405/cpci405.c
index bf5a4cb..ca9a944 100644
--- a/board/esd/cpci405/cpci405.c
+++ b/board/esd/cpci405/cpci405.c
@@ -5,6 +5,7 @@
* SPDX-License-Identifier: GPL-2.0+
*/
#include <common.h>
+#include <console.h>
#include <libfdt.h>
#include <fdt_support.h>
#include <asm/processor.h>
diff --git a/board/esd/pmc405de/pmc405de.c b/board/esd/pmc405de/pmc405de.c
index 3e17132..24e4977 100644
--- a/board/esd/pmc405de/pmc405de.c
+++ b/board/esd/pmc405de/pmc405de.c
@@ -6,6 +6,7 @@
*/
#include <common.h>
+#include <console.h>
#include <libfdt.h>
#include <fdt_support.h>
#include <asm/processor.h>
diff --git a/board/esd/pmc440/cmd_pmc440.c b/board/esd/pmc440/cmd_pmc440.c
index 40b135f..b7cd595 100644
--- a/board/esd/pmc440/cmd_pmc440.c
+++ b/board/esd/pmc440/cmd_pmc440.c
@@ -6,6 +6,7 @@
*/
#include <common.h>
#include <command.h>
+#include <console.h>
#include <asm/io.h>
#include <asm/cache.h>
#include <asm/processor.h>
diff --git a/board/esd/pmc440/pmc440.c b/board/esd/pmc440/pmc440.c
index 15c3151..7e35c19 100644
--- a/board/esd/pmc440/pmc440.c
+++ b/board/esd/pmc440/pmc440.c
@@ -13,6 +13,7 @@
* SPDX-License-Identifier: GPL-2.0+
*/
#include <common.h>
+#include <console.h>
#include <libfdt.h>
#include <fdt_support.h>
#include <asm/ppc440.h>
diff --git a/board/esd/vme8349/caddy.c b/board/esd/vme8349/caddy.c
index cd56bed..0c1574c 100644
--- a/board/esd/vme8349/caddy.c
+++ b/board/esd/vme8349/caddy.c
@@ -8,6 +8,7 @@
*/
#include <common.h>
+#include <console.h>
#include <ioports.h>
#include <mpc83xx.h>
#include <asm/mpc8349_pci.h>
diff --git a/board/freescale/b4860qds/spl.c b/board/freescale/b4860qds/spl.c
index 3aa5a78..3f7cc03 100644
--- a/board/freescale/b4860qds/spl.c
+++ b/board/freescale/b4860qds/spl.c
@@ -4,6 +4,7 @@
*/
#include <common.h>
+#include <console.h>
#include <asm/spl.h>
#include <malloc.h>
#include <ns16550.h>
diff --git a/board/freescale/c29xpcie/spl.c b/board/freescale/c29xpcie/spl.c
index 2111711..3d31d41 100644
--- a/board/freescale/c29xpcie/spl.c
+++ b/board/freescale/c29xpcie/spl.c
@@ -4,6 +4,7 @@
*/
#include <common.h>
+#include <console.h>
#include <ns16550.h>
#include <malloc.h>
#include <mmc.h>
diff --git a/board/freescale/mpc8569mds/mpc8569mds.c b/board/freescale/mpc8569mds/mpc8569mds.c
index 836578f..122490c 100644
--- a/board/freescale/mpc8569mds/mpc8569mds.c
+++ b/board/freescale/mpc8569mds/mpc8569mds.c
@@ -7,6 +7,7 @@
*/
#include <common.h>
+#include <console.h>
#include <hwconfig.h>
#include <pci.h>
#include <asm/processor.h>
diff --git a/board/freescale/p1010rdb/spl.c b/board/freescale/p1010rdb/spl.c
index ee873b0..eb8e567 100644
--- a/board/freescale/p1010rdb/spl.c
+++ b/board/freescale/p1010rdb/spl.c
@@ -4,6 +4,7 @@
*/
#include <common.h>
+#include <console.h>
#include <ns16550.h>
#include <malloc.h>
#include <mmc.h>
diff --git a/board/freescale/p1022ds/spl.c b/board/freescale/p1022ds/spl.c
index 7bd9d29..89ef95a 100644
--- a/board/freescale/p1022ds/spl.c
+++ b/board/freescale/p1022ds/spl.c
@@ -5,6 +5,7 @@
*/
#include <common.h>
+#include <console.h>
#include <ns16550.h>
#include <malloc.h>
#include <mmc.h>
diff --git a/board/freescale/p1_p2_rdb_pc/spl.c b/board/freescale/p1_p2_rdb_pc/spl.c
index 8d0d850..0142746 100644
--- a/board/freescale/p1_p2_rdb_pc/spl.c
+++ b/board/freescale/p1_p2_rdb_pc/spl.c
@@ -5,6 +5,7 @@
*/
#include <common.h>
+#include <console.h>
#include <ns16550.h>
#include <malloc.h>
#include <mmc.h>
diff --git a/board/freescale/t102xqds/spl.c b/board/freescale/t102xqds/spl.c
index 08aef6e..073ff2d 100644
--- a/board/freescale/t102xqds/spl.c
+++ b/board/freescale/t102xqds/spl.c
@@ -4,6 +4,7 @@
*/
#include <common.h>
+#include <console.h>
#include <malloc.h>
#include <ns16550.h>
#include <nand.h>
diff --git a/board/freescale/t102xrdb/spl.c b/board/freescale/t102xrdb/spl.c
index 9c581ff..da97c44 100644
--- a/board/freescale/t102xrdb/spl.c
+++ b/board/freescale/t102xrdb/spl.c
@@ -4,6 +4,7 @@
*/
#include <common.h>
+#include <console.h>
#include <malloc.h>
#include <ns16550.h>
#include <nand.h>
diff --git a/board/freescale/t104xrdb/spl.c b/board/freescale/t104xrdb/spl.c
index 4e8735b..81f48c4 100644
--- a/board/freescale/t104xrdb/spl.c
+++ b/board/freescale/t104xrdb/spl.c
@@ -4,6 +4,7 @@
*/
#include <common.h>
+#include <console.h>
#include <malloc.h>
#include <ns16550.h>
#include <nand.h>
diff --git a/board/freescale/t208xqds/spl.c b/board/freescale/t208xqds/spl.c
index a71c617..55a0f8f 100644
--- a/board/freescale/t208xqds/spl.c
+++ b/board/freescale/t208xqds/spl.c
@@ -4,6 +4,7 @@
*/
#include <common.h>
+#include <console.h>
#include <malloc.h>
#include <ns16550.h>
#include <nand.h>
diff --git a/board/freescale/t208xrdb/spl.c b/board/freescale/t208xrdb/spl.c
index 9ae2b1e..f63366b 100644
--- a/board/freescale/t208xrdb/spl.c
+++ b/board/freescale/t208xrdb/spl.c
@@ -4,6 +4,7 @@
*/
#include <common.h>
+#include <console.h>
#include <malloc.h>
#include <ns16550.h>
#include <nand.h>
diff --git a/board/freescale/t4qds/spl.c b/board/freescale/t4qds/spl.c
index 0c6156e..d52059a 100644
--- a/board/freescale/t4qds/spl.c
+++ b/board/freescale/t4qds/spl.c
@@ -4,6 +4,7 @@
*/
#include <common.h>
+#include <console.h>
#include <asm/spl.h>
#include <malloc.h>
#include <ns16550.h>
diff --git a/board/freescale/t4rdb/spl.c b/board/freescale/t4rdb/spl.c
index 68ecde7..4c1e0cc 100644
--- a/board/freescale/t4rdb/spl.c
+++ b/board/freescale/t4rdb/spl.c
@@ -7,6 +7,7 @@
*/
#include <common.h>
+#include <console.h>
#include <asm/spl.h>
#include <malloc.h>
#include <ns16550.h>
diff --git a/board/gdsys/common/cmd_ioloop.c b/board/gdsys/common/cmd_ioloop.c
index e0c74fe..0e81c9d 100644
--- a/board/gdsys/common/cmd_ioloop.c
+++ b/board/gdsys/common/cmd_ioloop.c
@@ -7,6 +7,7 @@
#include <common.h>
#include <command.h>
+#include <console.h>
#include <gdsys_fpga.h>
diff --git a/board/inka4x0/inkadiag.c b/board/inka4x0/inkadiag.c
index 0bd12ec..4c43205 100644
--- a/board/inka4x0/inkadiag.c
+++ b/board/inka4x0/inkadiag.c
@@ -10,6 +10,7 @@
#include <asm/io.h>
#include <common.h>
#include <config.h>
+#include <console.h>
#include <mpc5xxx.h>
#include <pci.h>
diff --git a/board/lwmon5/kbd.c b/board/lwmon5/kbd.c
index 97962da..d6c0a20 100644
--- a/board/lwmon5/kbd.c
+++ b/board/lwmon5/kbd.c
@@ -17,6 +17,7 @@
#include <common.h>
#include <i2c.h>
#include <command.h>
+#include <console.h>
#include <post.h>
#include <serial.h>
#include <malloc.h>
diff --git a/board/mpl/common/kbd.c b/board/mpl/common/kbd.c
index 1da72c5..36b1694 100644
--- a/board/mpl/common/kbd.c
+++ b/board/mpl/common/kbd.c
@@ -8,6 +8,7 @@
* linux/drivers/char/pc_keyb.c
*/
#include <common.h>
+#include <console.h>
#include <asm/processor.h>
#include <stdio_dev.h>
#include "isa.h"
diff --git a/board/mpl/pati/pati.c b/board/mpl/pati/pati.c
index 958cdec..ddc2108 100644
--- a/board/mpl/pati/pati.c
+++ b/board/mpl/pati/pati.c
@@ -29,6 +29,7 @@
**********************************************************************************/
#include <common.h>
+#include <console.h>
#include <mpc5xx.h>
#include <stdio_dev.h>
#include <pci_ids.h>
diff --git a/board/renesas/sh7785lcr/selfcheck.c b/board/renesas/sh7785lcr/selfcheck.c
index d520750..9a6cdab 100644
--- a/board/renesas/sh7785lcr/selfcheck.c
+++ b/board/renesas/sh7785lcr/selfcheck.c
@@ -5,6 +5,7 @@
*/
#include <common.h>
+#include <console.h>
#include <asm/io.h>
#include <asm/processor.h>
#include <asm/pci.h>
diff --git a/board/tqc/tqm5200/cmd_stk52xx.c b/board/tqc/tqm5200/cmd_stk52xx.c
index 5f905d5..9d2d5a8 100644
--- a/board/tqc/tqm5200/cmd_stk52xx.c
+++ b/board/tqc/tqm5200/cmd_stk52xx.c
@@ -12,6 +12,7 @@
#include <common.h>
#include <command.h>
+#include <console.h>
#if defined(CONFIG_CMD_BSP)
diff --git a/board/tqc/tqm5200/tqm5200.c b/board/tqc/tqm5200/tqm5200.c
index 4d4f29d..8b82c34 100644
--- a/board/tqc/tqm5200/tqm5200.c
+++ b/board/tqc/tqm5200/tqm5200.c
@@ -12,6 +12,7 @@
*/
#include <common.h>
+#include <console.h>
#include <mpc5xxx.h>
#include <pci.h>
#include <asm/processor.h>
diff --git a/board/xes/xpedite1000/xpedite1000.c b/board/xes/xpedite1000/xpedite1000.c
index daab578..3b8a668 100644
--- a/board/xes/xpedite1000/xpedite1000.c
+++ b/board/xes/xpedite1000/xpedite1000.c
@@ -5,6 +5,7 @@
*/
#include <common.h>
+#include <console.h>
#include <asm/processor.h>
#include <spd_sdram.h>
#include <i2c.h>
diff --git a/common/autoboot.c b/common/autoboot.c
index c367076..c11fb31 100644
--- a/common/autoboot.c
+++ b/common/autoboot.c
@@ -9,6 +9,7 @@
#include <autoboot.h>
#include <bootretry.h>
#include <cli.h>
+#include <console.h>
#include <fdtdec.h>
#include <menu.h>
#include <post.h>
diff --git a/common/board_f.c b/common/board_f.c
index b7ed995..dbb987f 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -13,6 +13,7 @@
#include <common.h>
#include <linux/compiler.h>
#include <version.h>
+#include <console.h>
#include <environment.h>
#include <dm.h>
#include <fdtdec.h>
diff --git a/common/board_r.c b/common/board_r.c
index c4fd3ea..5a46b47 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -15,6 +15,7 @@
#if defined(CONFIG_CMD_BEDBUG)
#include <bedbug/type.h>
#endif
+#include <console.h>
#ifdef CONFIG_HAS_DATAFLASH
#include <dataflash.h>
#endif
diff --git a/common/cli.c b/common/cli.c
index b6ae80a..fbcd339 100644
--- a/common/cli.c
+++ b/common/cli.c
@@ -12,6 +12,7 @@
#include <common.h>
#include <cli.h>
#include <cli_hush.h>
+#include <console.h>
#include <fdtdec.h>
#include <malloc.h>
diff --git a/common/cli_hush.c b/common/cli_hush.c
index 296542f..a7cac4f 100644
--- a/common/cli_hush.c
+++ b/common/cli_hush.c
@@ -79,6 +79,7 @@
#include <malloc.h> /* malloc, free, realloc*/
#include <linux/ctype.h> /* isalpha, isdigit */
#include <common.h> /* readline */
+#include <console.h>
#include <bootretry.h>
#include <cli.h>
#include <cli_hush.h>
diff --git a/common/cli_simple.c b/common/cli_simple.c
index d8b40c9..9c3d073 100644
--- a/common/cli_simple.c
+++ b/common/cli_simple.c
@@ -12,6 +12,7 @@
#include <common.h>
#include <bootretry.h>
#include <cli.h>
+#include <console.h>
#include <linux/ctype.h>
#define DEBUG_PARSER 0 /* set to 1 to debug */
diff --git a/common/cmd_armflash.c b/common/cmd_armflash.c
index af453f7..b94d128 100644
--- a/common/cmd_armflash.c
+++ b/common/cmd_armflash.c
@@ -8,6 +8,7 @@
*/
#include <common.h>
#include <command.h>
+#include <console.h>
#include <asm/io.h>
#define MAX_REGIONS 4
diff --git a/common/cmd_dcr.c b/common/cmd_dcr.c
index 4fddd80..cc77250 100644
--- a/common/cmd_dcr.c
+++ b/common/cmd_dcr.c
@@ -13,6 +13,7 @@
#include <cli.h>
#include <config.h>
#include <command.h>
+#include <console.h>
unsigned long get_dcr (unsigned short);
unsigned long set_dcr (unsigned short, unsigned long);
diff --git a/common/cmd_dfu.c b/common/cmd_dfu.c
index f060db7..6d95ce9 100644
--- a/common/cmd_dfu.c
+++ b/common/cmd_dfu.c
@@ -14,6 +14,7 @@
#include <common.h>
#include <watchdog.h>
#include <dfu.h>
+#include <console.h>
#include <g_dnl.h>
#include <usb.h>
#include <net.h>
diff --git a/common/cmd_fastboot.c b/common/cmd_fastboot.c
index b9d1c8c..488822a 100644
--- a/common/cmd_fastboot.c
+++ b/common/cmd_fastboot.c
@@ -9,6 +9,7 @@
*/
#include <common.h>
#include <command.h>
+#include <console.h>
#include <g_dnl.h>
#include <usb.h>
diff --git a/common/cmd_fpgad.c b/common/cmd_fpgad.c
index 1f1d00f..5370c3e 100644
--- a/common/cmd_fpgad.c
+++ b/common/cmd_fpgad.c
@@ -11,6 +11,7 @@
#include <common.h>
#include <command.h>
+#include <console.h>
#include <gdsys_fpga.h>
diff --git a/common/cmd_fuse.c b/common/cmd_fuse.c
index d4bc0f6..5998f9b 100644
--- a/common/cmd_fuse.c
+++ b/common/cmd_fuse.c
@@ -11,6 +11,7 @@
#include <common.h>
#include <command.h>
+#include <console.h>
#include <fuse.h>
#include <asm/errno.h>
diff --git a/common/cmd_i2c.c b/common/cmd_i2c.c
index 864b259..3d0de81 100644
--- a/common/cmd_i2c.c
+++ b/common/cmd_i2c.c
@@ -69,6 +69,7 @@
#include <bootretry.h>
#include <cli.h>
#include <command.h>
+#include <console.h>
#include <dm.h>
#include <edid.h>
#include <environment.h>
diff --git a/common/cmd_load.c b/common/cmd_load.c
index d043e6d..0aa7937 100644
--- a/common/cmd_load.c
+++ b/common/cmd_load.c
@@ -10,6 +10,7 @@
*/
#include <common.h>
#include <command.h>
+#include <console.h>
#include <s_record.h>
#include <net.h>
#include <exports.h>
diff --git a/common/cmd_mem.c b/common/cmd_mem.c
index 43c3fb6..9fb2584 100644
--- a/common/cmd_mem.c
+++ b/common/cmd_mem.c
@@ -12,9 +12,11 @@
*/
#include <common.h>
+#include <console.h>
#include <bootretry.h>
#include <cli.h>
#include <command.h>
+#include <console.h>
#ifdef CONFIG_HAS_DATAFLASH
#include <dataflash.h>
#endif
diff --git a/common/cmd_misc.c b/common/cmd_misc.c
index 93f9eab..39d8683 100644
--- a/common/cmd_misc.c
+++ b/common/cmd_misc.c
@@ -10,6 +10,7 @@
*/
#include <common.h>
#include <command.h>
+#include <console.h>
static int do_sleep(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
{
diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c
index 1335e3d..dfc1ec8 100644
--- a/common/cmd_mmc.c
+++ b/common/cmd_mmc.c
@@ -7,6 +7,7 @@
#include <common.h>
#include <command.h>
+#include <console.h>
#include <mmc.h>
static int curr_device = -1;
diff --git a/common/cmd_nand.c b/common/cmd_nand.c
index 1482462..a6b67e2 100644
--- a/common/cmd_nand.c
+++ b/common/cmd_nand.c
@@ -22,6 +22,7 @@
#include <common.h>
#include <linux/mtd/mtd.h>
#include <command.h>
+#include <console.h>
#include <watchdog.h>
#include <malloc.h>
#include <asm/byteorder.h>
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index f4c2523..2f9cdd0 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -27,6 +27,7 @@
#include <common.h>
#include <cli.h>
#include <command.h>
+#include <console.h>
#include <environment.h>
#include <search.h>
#include <errno.h>
diff --git a/common/cmd_otp.c b/common/cmd_otp.c
index 593bb8c..10c1475 100644
--- a/common/cmd_otp.c
+++ b/common/cmd_otp.c
@@ -16,6 +16,7 @@
#include <config.h>
#include <common.h>
#include <command.h>
+#include <console.h>
#include <asm/blackfin.h>
#include <asm/clock.h>
diff --git a/common/cmd_pci.c b/common/cmd_pci.c
index dcecef8..802e433 100644
--- a/common/cmd_pci.c
+++ b/common/cmd_pci.c
@@ -17,6 +17,7 @@
#include <bootretry.h>
#include <cli.h>
#include <command.h>
+#include <console.h>
#include <asm/processor.h>
#include <asm/io.h>
#include <pci.h>
diff --git a/common/cmd_usb.c b/common/cmd_usb.c
index 4679134..c2d9803 100644
--- a/common/cmd_usb.c
+++ b/common/cmd_usb.c
@@ -13,6 +13,7 @@
#include <common.h>
#include <command.h>
+#include <console.h>
#include <dm.h>
#include <memalign.h>
#include <asm/byteorder.h>
diff --git a/common/cmd_usb_mass_storage.c b/common/cmd_usb_mass_storage.c
index 198dab1..0407389 100644
--- a/common/cmd_usb_mass_storage.c
+++ b/common/cmd_usb_mass_storage.c
@@ -8,6 +8,7 @@
#include <errno.h>
#include <common.h>
#include <command.h>
+#include <console.h>
#include <g_dnl.h>
#include <part.h>
#include <usb.h>
diff --git a/common/command.c b/common/command.c
index 972ae28..858e288 100644
--- a/common/command.c
+++ b/common/command.c
@@ -11,6 +11,7 @@
#include <common.h>
#include <command.h>
+#include <console.h>
#include <linux/ctype.h>
/*
diff --git a/common/console.c b/common/console.c
index ace206c..10972b0 100644
--- a/common/console.c
+++ b/common/console.c
@@ -6,6 +6,7 @@
*/
#include <common.h>
+#include <console.h>
#include <debug_uart.h>
#include <stdarg.h>
#include <iomux.h>
diff --git a/common/iomux.c b/common/iomux.c
index 62bdec6..3d8d00b 100644
--- a/common/iomux.c
+++ b/common/iomux.c
@@ -6,6 +6,7 @@
*/
#include <common.h>
+#include <console.h>
#include <serial.h>
#include <malloc.h>
diff --git a/common/main.c b/common/main.c
index ead0cd1..5a03181 100644
--- a/common/main.c
+++ b/common/main.c
@@ -10,6 +10,7 @@
#include <common.h>
#include <autoboot.h>
#include <cli.h>
+#include <console.h>
#include <version.h>
DECLARE_GLOBAL_DATA_PTR;
diff --git a/common/usb_kbd.c b/common/usb_kbd.c
index 5c6a372..5a90f84 100644
--- a/common/usb_kbd.c
+++ b/common/usb_kbd.c
@@ -8,6 +8,7 @@
* SPDX-License-Identifier: GPL-2.0+
*/
#include <common.h>
+#include <console.h>
#include <dm.h>
#include <errno.h>
#include <malloc.h>
diff --git a/drivers/block/fsl_sata.c b/drivers/block/fsl_sata.c
index 735708a..208a0ae 100644
--- a/drivers/block/fsl_sata.c
+++ b/drivers/block/fsl_sata.c
@@ -7,6 +7,7 @@
#include <common.h>
#include <command.h>
+#include <console.h>
#include <asm/io.h>
#include <asm/processor.h>
#include <asm/fsl_serdes.h>
diff --git a/drivers/fpga/ACEX1K.c b/drivers/fpga/ACEX1K.c
index 06b8837..1627f0e 100644
--- a/drivers/fpga/ACEX1K.c
+++ b/drivers/fpga/ACEX1K.c
@@ -9,6 +9,7 @@
*/
#include <common.h> /* core U-Boot definitions */
+#include <console.h>
#include <ACEX1K.h> /* ACEX device family */
/* Define FPGA_DEBUG to get debug printf's */
diff --git a/drivers/fpga/virtex2.c b/drivers/fpga/virtex2.c
index 0d2d9a4..f7cf02a 100644
--- a/drivers/fpga/virtex2.c
+++ b/drivers/fpga/virtex2.c
@@ -12,6 +12,7 @@
*/
#include <common.h>
+#include <console.h>
#include <virtex2.h>
#if 0
diff --git a/drivers/fpga/zynqpl.c b/drivers/fpga/zynqpl.c
index 6a74f89..ef889ea 100644
--- a/drivers/fpga/zynqpl.c
+++ b/drivers/fpga/zynqpl.c
@@ -8,6 +8,7 @@
*/
#include <common.h>
+#include <console.h>
#include <asm/io.h>
#include <fs.h>
#include <zynqpl.h>
diff --git a/drivers/i2c/adi_i2c.c b/drivers/i2c/adi_i2c.c
index c58f14a..f0c0841 100644
--- a/drivers/i2c/adi_i2c.c
+++ b/drivers/i2c/adi_i2c.c
@@ -7,6 +7,7 @@
*/
#include <common.h>
+#include <console.h>
#include <i2c.h>
#include <asm/clock.h>
diff --git a/drivers/input/input.c b/drivers/input/input.c
index fc1c45c..9e552f3 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -8,6 +8,7 @@
*/
#include <common.h>
+#include <console.h>
#include <errno.h>
#include <stdio_dev.h>
#include <input.h>
diff --git a/drivers/input/keyboard.c b/drivers/input/keyboard.c
index ca3886a..eec8c27 100644
--- a/drivers/input/keyboard.c
+++ b/drivers/input/keyboard.c
@@ -9,6 +9,7 @@
***********************************************************************/
#include <common.h>
+#include <console.h>
#include <stdio_dev.h>
#include <keyboard.h>
diff --git a/drivers/misc/cbmem_console.c b/drivers/misc/cbmem_console.c
index 5f85ccf..127121e 100644
--- a/drivers/misc/cbmem_console.c
+++ b/drivers/misc/cbmem_console.c
@@ -16,7 +16,7 @@
*/
#include <common.h>
-
+#include <console.h>
#ifndef CONFIG_SYS_COREBOOT
#error This driver requires coreboot
#endif
diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
index fc7a878..d7b317c 100644
--- a/drivers/mtd/cfi_flash.c
+++ b/drivers/mtd/cfi_flash.c
@@ -18,6 +18,7 @@
/* #define DEBUG */
#include <common.h>
+#include <console.h>
#include <asm/processor.h>
#include <asm/io.h>
#include <asm/byteorder.h>
diff --git a/drivers/mtd/nand/bfin_nand.c b/drivers/mtd/nand/bfin_nand.c
index 7e755e8..7c11868 100644
--- a/drivers/mtd/nand/bfin_nand.c
+++ b/drivers/mtd/nand/bfin_nand.c
@@ -15,6 +15,7 @@
*/
#include <common.h>
+#include <console.h>
#include <asm/io.h>
#ifdef DEBUG
diff --git a/drivers/net/e1000_spi.c b/drivers/net/e1000_spi.c
index e7f6826..df72375 100644
--- a/drivers/net/e1000_spi.c
+++ b/drivers/net/e1000_spi.c
@@ -1,4 +1,5 @@
#include <common.h>
+#include <console.h>
#include "e1000.h"
#include <linux/compiler.h>
diff --git a/drivers/net/keystone_net.c b/drivers/net/keystone_net.c
index 5ed29ae..24ca52e 100644
--- a/drivers/net/keystone_net.c
+++ b/drivers/net/keystone_net.c
@@ -8,6 +8,7 @@
*/
#include <common.h>
#include <command.h>
+#include <console.h>
#include <net.h>
#include <phy.h>
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index d7364ff..6deec1a 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -11,6 +11,7 @@
#include <config.h>
#include <common.h>
+#include <console.h>
#include <dm.h>
#include <malloc.h>
#include <net.h>
diff --git a/drivers/net/vsc7385.c b/drivers/net/vsc7385.c
index a5110e5..c6d6dce 100644
--- a/drivers/net/vsc7385.c
+++ b/drivers/net/vsc7385.c
@@ -14,6 +14,7 @@
#include <config.h>
#include <common.h>
+#include <console.h>
#include <asm/io.h>
#include <asm/errno.h>
#include "vsc7385.h"
diff --git a/drivers/power/battery/bat_trats.c b/drivers/power/battery/bat_trats.c
index bfde692..089e789 100644
--- a/drivers/power/battery/bat_trats.c
+++ b/drivers/power/battery/bat_trats.c
@@ -6,6 +6,7 @@
*/
#include <common.h>
+#include <console.h>
#include <power/pmic.h>
#include <power/battery.h>
#include <power/max8997_pmic.h>
diff --git a/drivers/spi/bfin_spi.c b/drivers/spi/bfin_spi.c
index 71a31d0..9a6fc78 100644
--- a/drivers/spi/bfin_spi.c
+++ b/drivers/spi/bfin_spi.c
@@ -9,6 +9,7 @@
/*#define DEBUG*/
#include <common.h>
+#include <console.h>
#include <malloc.h>
#include <spi.h>
diff --git a/drivers/spi/bfin_spi6xx.c b/drivers/spi/bfin_spi6xx.c
index 8359d76..9a27b78 100644
--- a/drivers/spi/bfin_spi6xx.c
+++ b/drivers/spi/bfin_spi6xx.c
@@ -18,6 +18,7 @@
*/
#include <common.h>
+#include <console.h>
#include <malloc.h>
#include <spi.h>
diff --git a/drivers/spi/sh_qspi.c b/drivers/spi/sh_qspi.c
index 7209e1d..75999c8 100644
--- a/drivers/spi/sh_qspi.c
+++ b/drivers/spi/sh_qspi.c
@@ -8,6 +8,7 @@
*/
#include <common.h>
+#include <console.h>
#include <malloc.h>
#include <spi.h>
#include <asm/arch/rmobile.h>
diff --git a/drivers/spi/sh_spi.c b/drivers/spi/sh_spi.c
index 7ca5e36..1384385 100644
--- a/drivers/spi/sh_spi.c
+++ b/drivers/spi/sh_spi.c
@@ -19,6 +19,7 @@
*/
#include <common.h>
+#include <console.h>
#include <malloc.h>
#include <spi.h>
#include <asm/io.h>
diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
index c5e35ee..cfe9a24 100644
--- a/drivers/usb/gadget/ether.c
+++ b/drivers/usb/gadget/ether.c
@@ -9,6 +9,7 @@
*/
#include <common.h>
+#include <console.h>
#include <asm/errno.h>
#include <linux/netdevice.h>
#include <linux/usb/ch9.h>
diff --git a/drivers/usb/gadget/f_mass_storage.c b/drivers/usb/gadget/f_mass_storage.c
index abe9391..ec1f23a 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -243,6 +243,7 @@
#include <config.h>
#include <malloc.h>
#include <common.h>
+#include <console.h>
#include <g_dnl.h>
#include <linux/err.h>
diff --git a/drivers/usb/gadget/f_thor.c b/drivers/usb/gadget/f_thor.c
index 9ed0ce3..a60e948 100644
--- a/drivers/usb/gadget/f_thor.c
+++ b/drivers/usb/gadget/f_thor.c
@@ -17,6 +17,7 @@
#include <errno.h>
#include <common.h>
+#include <console.h>
#include <malloc.h>
#include <memalign.h>
#include <version.h>
diff --git a/drivers/usb/host/r8a66597-hcd.c b/drivers/usb/host/r8a66597-hcd.c
index 373e04c..6ef5190 100644
--- a/drivers/usb/host/r8a66597-hcd.c
+++ b/drivers/usb/host/r8a66597-hcd.c
@@ -7,6 +7,7 @@
*/
#include <common.h>
+#include <console.h>
#include <usb.h>
#include <asm/io.h>
diff --git a/drivers/usb/musb-new/musb_uboot.c b/drivers/usb/musb-new/musb_uboot.c
index 10f6b5d..233a0e4 100644
--- a/drivers/usb/musb-new/musb_uboot.c
+++ b/drivers/usb/musb-new/musb_uboot.c
@@ -1,4 +1,5 @@
#include <common.h>
+#include <console.h>
#include <watchdog.h>
#ifdef CONFIG_ARCH_SUNXI
#include <asm/arch/usb_phy.h>
diff --git a/examples/standalone/mem_to_mem_idma2intr.c b/examples/standalone/mem_to_mem_idma2intr.c
index 215dc22..17da8db 100644
--- a/examples/standalone/mem_to_mem_idma2intr.c
+++ b/examples/standalone/mem_to_mem_idma2intr.c
@@ -28,6 +28,7 @@
#include <common.h>
+#include <console.h>
#include <exports.h>
DECLARE_GLOBAL_DATA_PTR;
diff --git a/examples/standalone/smc911x_eeprom.c b/examples/standalone/smc911x_eeprom.c
index 6c79c5f..364ad2d 100644
--- a/examples/standalone/smc911x_eeprom.c
+++ b/examples/standalone/smc911x_eeprom.c
@@ -15,6 +15,7 @@
*/
#include <common.h>
+#include <console.h>
#include <exports.h>
#include <linux/ctype.h>
#include "../drivers/net/smc911x.h"
diff --git a/include/common.h b/include/common.h
index 09a131d..e910730 100644
--- a/include/common.h
+++ b/include/common.h
@@ -218,7 +218,6 @@ int run_command_repeatable(const char *cmd, int flag);
* @return 0 on success, or != 0 on error.
*/
int run_command_list(const char *cmd, int len, int flag);
-extern char console_buffer[];
/* arch/$(ARCH)/lib/board.c */
void board_init_f(ulong);
@@ -862,15 +861,6 @@ void srand(unsigned int seed);
unsigned int rand(void);
unsigned int rand_r(unsigned int *seedp);
-/* common/console.c */
-int console_init_f(void); /* Before relocation; uses the serial stuff */
-int console_init_r(void); /* After relocation; uses the console stuff */
-int console_assign(int file, const char *devname); /* Assign the console */
-int ctrlc (void);
-int had_ctrlc (void); /* have we had a Control-C since last clear? */
-void clear_ctrlc (void); /* clear the Control-C condition */
-int disable_ctrlc (int); /* 1 to disable, 0 to enable Control-C detect */
-int confirm_yesno(void); /* 1 if input is "y", "Y", "yes" or "YES" */
/*
* STDIO based functions (can always be used)
*/
@@ -929,13 +919,6 @@ static inline struct in_addr getenv_ip(char *var)
return string_to_ip(getenv(var));
}
-/*
- * CONSOLE multiplexing.
- */
-#ifdef CONFIG_CONSOLE_MUX
-#include <iomux.h>
-#endif
-
int pcmcia_init (void);
#ifdef CONFIG_STATUS_LED
diff --git a/include/console.h b/include/console.h
new file mode 100644
index 0000000..097518d
--- /dev/null
+++ b/include/console.h
@@ -0,0 +1,30 @@
+/*
+ * (C) Copyright 2000-2009
+ * Wolfgang Denk, DENX Software Engineering, wd at denx.de.
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#ifndef __CONSOLE_H
+#define __CONSOLE_H
+
+extern char console_buffer[];
+
+/* common/console.c */
+int console_init_f(void); /* Before relocation; uses the serial stuff */
+int console_init_r(void); /* After relocation; uses the console stuff */
+int console_assign(int file, const char *devname); /* Assign the console */
+int ctrlc(void);
+int had_ctrlc(void); /* have we had a Control-C since last clear? */
+void clear_ctrlc(void); /* clear the Control-C condition */
+int disable_ctrlc(int); /* 1 to disable, 0 to enable Control-C detect */
+int confirm_yesno(void); /* 1 if input is "y", "Y", "yes" or "YES" */
+
+/*
+ * CONSOLE multiplexing.
+ */
+#ifdef CONFIG_CONSOLE_MUX
+#include <iomux.h>
+#endif
+
+#endif
diff --git a/lib/display_options.c b/lib/display_options.c
index 83ea4de..4f2961f 100644
--- a/lib/display_options.c
+++ b/lib/display_options.c
@@ -7,6 +7,7 @@
#include <config.h>
#include <common.h>
+#include <console.h>
#include <div64.h>
#include <inttypes.h>
#include <version.h>
diff --git a/lib/gunzip.c b/lib/gunzip.c
index 4128a18..bdd85c4 100644
--- a/lib/gunzip.c
+++ b/lib/gunzip.c
@@ -8,6 +8,7 @@
#include <common.h>
#include <watchdog.h>
#include <command.h>
+#include <console.h>
#include <image.h>
#include <malloc.h>
#include <u-boot/zlib.h>
diff --git a/net/net.c b/net/net.c
index 2926bce..4d5746a 100644
--- a/net/net.c
+++ b/net/net.c
@@ -83,6 +83,7 @@
#include <common.h>
#include <command.h>
+#include <console.h>
#include <environment.h>
#include <errno.h>
#include <net.h>
diff --git a/test/dm/test-main.c b/test/dm/test-main.c
index a36a9c0..867b517 100644
--- a/test/dm/test-main.c
+++ b/test/dm/test-main.c
@@ -6,6 +6,7 @@
#include <common.h>
#include <command.h>
+#include <console.h>
#include <dm.h>
#include <errno.h>
#include <malloc.h>
diff --git a/test/dm/usb.c b/test/dm/usb.c
index 4300bbd..ccc07d5 100644
--- a/test/dm/usb.c
+++ b/test/dm/usb.c
@@ -5,6 +5,7 @@
*/
#include <common.h>
+#include <console.h>
#include <dm.h>
#include <usb.h>
#include <asm/io.h>
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply related [flat|nested] 67+ messages in thread* [U-Boot] [PATCH v2 04/26] Drop config.h header from display_options.c
2015-11-09 6:47 [U-Boot] [PATCH v2 00/26] usb: Drop requirement for USB unbinding, add tests Simon Glass
` (2 preceding siblings ...)
2015-11-09 6:47 ` [U-Boot] [PATCH v2 03/26] Move console definitions into a new console.h file Simon Glass
@ 2015-11-09 6:47 ` Simon Glass
2015-11-20 3:31 ` Simon Glass
2015-11-09 6:47 ` [U-Boot] [PATCH v2 05/26] Add a circular memory buffer implementation Simon Glass
` (23 subsequent siblings)
27 siblings, 1 reply; 67+ messages in thread
From: Simon Glass @ 2015-11-09 6:47 UTC (permalink / raw)
To: u-boot
Since common.h will always include this automatically, it is not needed.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None
lib/display_options.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/lib/display_options.c b/lib/display_options.c
index 4f2961f..29343fc 100644
--- a/lib/display_options.c
+++ b/lib/display_options.c
@@ -5,7 +5,6 @@
* SPDX-License-Identifier: GPL-2.0+
*/
-#include <config.h>
#include <common.h>
#include <console.h>
#include <div64.h>
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply related [flat|nested] 67+ messages in thread* [U-Boot] [PATCH v2 05/26] Add a circular memory buffer implementation
2015-11-09 6:47 [U-Boot] [PATCH v2 00/26] usb: Drop requirement for USB unbinding, add tests Simon Glass
` (3 preceding siblings ...)
2015-11-09 6:47 ` [U-Boot] [PATCH v2 04/26] Drop config.h header from display_options.c Simon Glass
@ 2015-11-09 6:47 ` Simon Glass
2015-11-20 3:31 ` Simon Glass
2015-11-09 6:47 ` [U-Boot] [PATCH v2 06/26] console: Add a console buffer Simon Glass
` (22 subsequent siblings)
27 siblings, 1 reply; 67+ messages in thread
From: Simon Glass @ 2015-11-09 6:47 UTC (permalink / raw)
To: u-boot
This will be used to support console recording. It provides for a circular
buffer which can be written at the head and read from the tail. It supports
avoiding data copying by providing raw access to the data.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None
include/membuff.h | 246 ++++++++++++++++++++++++++++++++++
lib/Makefile | 1 +
lib/membuff.c | 390 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 637 insertions(+)
create mode 100644 include/membuff.h
create mode 100644 lib/membuff.c
diff --git a/include/membuff.h b/include/membuff.h
new file mode 100644
index 0000000..78918e7
--- /dev/null
+++ b/include/membuff.h
@@ -0,0 +1,246 @@
+/*
+ * Copyright (c) 2015 Google, Inc
+ * Written by Simon Glass <sjg@chromium.org>
+ *
+ * Copyright (c) 1992 Simon Glass
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#ifndef _MEMBUFF_H
+#define _MEMBUFF_H
+
+/**
+ * @struct membuff: holds the state of a membuff - it is used for input and
+ * output buffers. The buffer extends from @start to (@start + @size - 1).
+ * Data in the buffer extends from @tail to @head: it is written in at
+ * @head and read out from @tail. The membuff is empty when @head == @tail
+ * and full when adding another character would make @head == @tail. We
+ * therefore waste one character in the membuff to avoid having an extra flag
+ * to determine whether (when @head == @tail) the membuff is empty or full.
+ *
+ * xxxxxx data
+ * ...... empty
+ *
+ * .............xxxxxxxxxxxxxxxx.........................
+ * ^ ^
+ * tail head
+ *
+ * xxxxxxxxxxxxx................xxxxxxxxxxxxxxxxxxxxxxxxx
+ * ^ ^
+ * head tail
+ */
+struct membuff {
+ char *start; /** the start of the buffer */
+ char *end; /** the end of the buffer (start + length) */
+ char *head; /** current buffer head */
+ char *tail; /** current buffer tail */
+};
+
+/**
+ * membuff_purge() - reset a membuff to the empty state
+ *
+ * Initialise head and tail pointers so that the membuff becomes empty.
+ *
+ * @mb: membuff to purge
+ */
+void membuff_purge(struct membuff *mb);
+
+/**
+ * membuff_putraw() - find out where bytes can be written
+ *
+ * Work out where in the membuff some data could be written. Return a pointer
+ * to the address and the number of bytes which can be written there. If
+ * @update is true, the caller must then write the data immediately, since
+ * the membuff is updated as if the write has been done,
+ *
+ * Note that because the spare space in a membuff may not be contiguous, this
+ * function may not return @maxlen even if there is enough space in the
+ * membuff. However, by calling this function twice (with @update == true),
+ * you will get access to all the spare space.
+ *
+ * @mb: membuff to adjust
+ * @maxlen: the number of bytes we want to write
+ * @update: true to update the membuff as if the write happened, false to not
+ * @data: the address data can be written to
+ * @return number of bytes which can be written
+ */
+int membuff_putraw(struct membuff *mb, int maxlen, bool update, char **data);
+
+/**
+ * membuff_getraw() - find and return a pointer to available bytes
+ *
+ * Returns a pointer to any valid input data in the given membuff and
+ * optionally marks it as read. Note that not all input data may not be
+ * returned, since data is not necessarily contiguous in the membuff. However,
+ * if you call this function twice (with @update == true) you are guaranteed
+ * to get all available data, in at most two installments.
+ *
+ * @mb: membuff to adjust
+ * @maxlen: maximum number of bytes to get
+ * @update: true to update the membuff as if the bytes have been read (use
+ * false to check bytes without reading them)
+ * @data: returns address of data in input membuff
+ * @return the number of bytes available at *@data
+ */
+int membuff_getraw(struct membuff *mb, int maxlen, bool update, char **data);
+
+/**
+ * membuff_putbyte() - Writes a byte to a membuff
+ *
+ * @mb: membuff to adjust
+ * @ch: byte to write
+ * @return true on success, false if membuff is full
+ */
+bool membuff_putbyte(struct membuff *mb, int ch);
+
+/**
+ * @mb: membuff to adjust
+ * membuff_getbyte() - Read a byte from the membuff
+ * @return the byte read, or -1 if the membuff is empty
+ */
+int membuff_getbyte(struct membuff *mb);
+
+/**
+ * membuff_peekbyte() - check the next available byte
+ *
+ * Return the next byte which membuff_getbyte() would return, without
+ * removing it from the membuff.
+ *
+ * @mb: membuff to adjust
+ * @return the byte peeked, or -1 if the membuff is empty
+ */
+int membuff_peekbyte(struct membuff *mb);
+
+/**
+ * membuff_get() - get data from a membuff
+ *
+ * Copies any available data (up to @maxlen bytes) to @buff and removes it
+ * from the membuff.
+ *
+ * @mb: membuff to adjust
+ * @Buff: address of membuff to transfer bytes to
+ * @maxlen: maximum number of bytes to read
+ * @return the number of bytes read
+ */
+int membuff_get(struct membuff *mb, char *buff, int maxlen);
+
+/**
+ * membuff_put() - write data to a membuff
+ *
+ * Writes some data to a membuff. Returns the number of bytes added. If this
+ * is less than @lnehgt, then the membuff got full
+ *
+ * @mb: membuff to adjust
+ * @data: the data to write
+ * @length: number of bytes to write from 'data'
+ * @return the number of bytes added
+ */
+int membuff_put(struct membuff *mb, const char *buff, int length);
+
+/**
+ * membuff_isempty() - check if a membuff is empty
+ *
+ * @mb: membuff to check
+ * @return true if empty, else false
+ */
+bool membuff_isempty(struct membuff *mb);
+
+/**
+ * membuff_avail() - check available data in a membuff
+ *
+ * @mb: membuff to check
+ * @return number of bytes of data available
+ */
+int membuff_avail(struct membuff *mb);
+
+/**
+ * membuff_size() - get the size of a membuff
+ *
+ * Note that a membuff can only old data up to one byte less than its size.
+ *
+ * @mb: membuff to check
+ * @return total size
+ */
+int membuff_size(struct membuff *mb);
+
+/**
+ * membuff_makecontig() - adjust all membuff data to be contiguous
+ *
+ * This places all data in a membuff into a single contiguous lump, if
+ * possible
+ *
+ * @mb: membuff to adjust
+ * @return true on success
+ */
+bool membuff_makecontig(struct membuff *mb);
+
+/**
+ * membuff_free() - find the number of bytes that can be written to a membuff
+ *
+ * @mb: membuff to check
+ * @return returns the number of bytes free in a membuff
+ */
+int membuff_free(struct membuff *mb);
+
+/**
+ * membuff_readline() - read a line of text from a membuff
+ *
+ * Reads a line of text of up to 'maxlen' characters from a membuff and puts
+ * it in @str. Any character less than @minch is assumed to be the end of
+ * line character
+ *
+ * @mb: membuff to adjust
+ * @str: Place to put the line
+ * @maxlen: Maximum line length (excluding terminator)
+ * @return number of bytes read (including terminator) if a line has been
+ * read, 0 if nothing was there
+ */
+int membuff_readline(struct membuff *mb, char *str, int maxlen, int minch);
+
+/**
+ * membuff_extend_by() - expand a membuff
+ *
+ * Extends a membuff by the given number of bytes
+ *
+ * @mb: membuff to adjust
+ * @by: Number of bytes to increase the size by
+ * @max: Maximum size to allow
+ * @return 0 if the expand succeeded, -ENOMEM if not enough memory, -E2BIG
+ * if the the size would exceed @max
+ */
+int membuff_extend_by(struct membuff *mb, int by, int max);
+
+/**
+ * membuff_init() - set up a new membuff using an existing membuff
+ *
+ * @mb: membuff to set up
+ * @buff: Address of buffer
+ * @size: Size of buffer
+ */
+void membuff_init(struct membuff *mb, char *buff, int size);
+
+/**
+ * membuff_uninit() - clear a membuff so it can no longer be used
+ *
+ * @mb: membuff to uninit
+ */
+void membuff_uninit(struct membuff *mb);
+
+/**
+ * membuff_new() - create a new membuff
+ *
+ * @mb: membuff to init
+ * @size: size of membuff to create
+ * @return 0 if OK, -ENOMEM if out of memory
+ */
+int membuff_new(struct membuff *mb, int size);
+
+/**
+ * membuff_dispose() - free memory allocated to a membuff and uninit it
+ *
+ * @mb: membuff to dispose
+ */
+void membuff_dispose(struct membuff *mb);
+
+#endif
diff --git a/lib/Makefile b/lib/Makefile
index 3eecefa..6c36278 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -74,6 +74,7 @@ obj-y += div64.o
obj-y += hang.o
obj-y += linux_compat.o
obj-y += linux_string.o
+obj-y += membuff.o
obj-$(CONFIG_REGEX) += slre.o
obj-y += string.o
obj-y += time.o
diff --git a/lib/membuff.c b/lib/membuff.c
new file mode 100644
index 0000000..fc37757
--- /dev/null
+++ b/lib/membuff.c
@@ -0,0 +1,390 @@
+/*
+ * Copyright (c) 2015 Google, Inc
+ * Written by Simon Glass <sjg@chromium.org>
+ *
+ * Copyright (c) 1992 Simon Glass
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#include <common.h>
+#include <errno.h>
+#include <malloc.h>
+#include "membuff.h"
+
+void membuff_purge(struct membuff *mb)
+{
+ /* set mb->head and mb->tail so the buffers look empty */
+ mb->head = mb->start;
+ mb->tail = mb->start;
+}
+
+static int membuff_putrawflex(struct membuff *mb, int maxlen, bool update,
+ char ***data, int *offsetp)
+{
+ int len;
+
+ /* always write to 'mb->head' */
+ assert(data && offsetp);
+ *data = &mb->start;
+ *offsetp = mb->head - mb->start;
+
+ /* if there is no buffer, we can do nothing */
+ if (!mb->start)
+ return 0;
+
+ /*
+ * if head is ahead of tail, we can write from head until the end of
+ * the buffer
+ */
+ if (mb->head >= mb->tail) {
+ /* work out how many bytes can fit here */
+ len = mb->end - mb->head - 1;
+ if (maxlen >= 0 && len > maxlen)
+ len = maxlen;
+
+ /* update the head pointer to mark these bytes as written */
+ if (update)
+ mb->head += len;
+
+ /*
+ * if the tail isn't at start of the buffer, then we can
+ * write one more byte right at the end
+ */
+ if ((maxlen < 0 || len < maxlen) && mb->tail != mb->start) {
+ len++;
+ if (update)
+ mb->head = mb->start;
+ }
+
+ /* otherwise now we can write until head almost reaches tail */
+ } else {
+ /* work out how many bytes can fit here */
+ len = mb->tail - mb->head - 1;
+ if (maxlen >= 0 && len > maxlen)
+ len = maxlen;
+
+ /* update the head pointer to mark these bytes as written */
+ if (update)
+ mb->head += len;
+ }
+
+ /* return the number of bytes which can be/must be written */
+ return len;
+}
+
+int membuff_putraw(struct membuff *mb, int maxlen, bool update, char **data)
+{
+ char **datap;
+ int offset;
+ int size;
+
+ size = membuff_putrawflex(mb, maxlen, update, &datap, &offset);
+ *data = *datap + offset;
+
+ return size;
+}
+
+bool membuff_putbyte(struct membuff *mb, int ch)
+{
+ char *data;
+
+ if (membuff_putraw(mb, 1, true, &data) != 1)
+ return false;
+ *data = ch;
+
+ return true;
+}
+
+int membuff_getraw(struct membuff *mb, int maxlen, bool update, char **data)
+{
+ int len;
+
+ /* assume for now there is no data to get */
+ len = 0;
+
+ /*
+ * in this case head is ahead of tail, so we must return data between
+ *'tail' and 'head'
+ */
+ if (mb->head > mb->tail) {
+ /* work out the amount of data */
+ *data = mb->tail;
+ len = mb->head - mb->tail;
+
+ /* check it isn't too much */
+ if (maxlen >= 0 && len > maxlen)
+ len = maxlen;
+
+ /* & mark it as read from the buffer */
+ if (update)
+ mb->tail += len;
+ }
+
+ /*
+ * if head is before tail, then we have data between 'tail' and 'end'
+ * and some more data between 'start' and 'head'(which we can't
+ * return this time
+ */
+ else if (mb->head < mb->tail) {
+ /* work out the amount of data */
+ *data = mb->tail;
+ len = mb->end - mb->tail;
+ if (maxlen >= 0 && len > maxlen)
+ len = maxlen;
+ if (update) {
+ mb->tail += len;
+ if (mb->tail == mb->end)
+ mb->tail = mb->start;
+ }
+ }
+
+ debug("getraw: maxlen=%d, update=%d, head=%d, tail=%d, data=%d, len=%d",
+ maxlen, update, (int)(mb->head - mb->start),
+ (int)(mb->tail - mb->start), (int)(*data - mb->start), len);
+
+ /* return the number of bytes we found */
+ return len;
+}
+
+int membuff_getbyte(struct membuff *mb)
+{
+ char *data = 0;
+
+ return membuff_getraw(mb, 1, true, &data) != 1 ? -1 : *(uint8_t *)data;
+}
+
+int membuff_peekbyte(struct membuff *mb)
+{
+ char *data = 0;
+
+ return membuff_getraw(mb, 1, false, &data) != 1 ? -1 : *(uint8_t *)data;
+}
+
+int membuff_get(struct membuff *mb, char *buff, int maxlen)
+{
+ char *data = 0, *buffptr = buff;
+ int len = 1, i;
+
+ /*
+ * do this in up to two lots(see GetRaw for why) stopping when there
+ * is no more data
+ */
+ for (i = 0; len && i < 2; i++) {
+ /* get a pointer to the data available */
+ len = membuff_getraw(mb, maxlen, true, &data);
+
+ /* copy it into the buffer */
+ memcpy(buffptr, data, len);
+ buffptr += len;
+ maxlen -= len;
+ }
+
+ /* return the number of bytes read */
+ return buffptr - buff;
+}
+
+int membuff_put(struct membuff *mb, const char *buff, int length)
+{
+ char *data;
+ int towrite, i, written;
+
+ for (i = written = 0; i < 2; i++) {
+ /* ask where some data can be written */
+ towrite = membuff_putraw(mb, length, true, &data);
+
+ /* and write it, updating the bytes length */
+ memcpy(data, buff, towrite);
+ written += towrite;
+ buff += towrite;
+ length -= towrite;
+ }
+
+ /* return the number of bytes written */
+ return written;
+}
+
+bool membuff_isempty(struct membuff *mb)
+{
+ return mb->head == mb->tail;
+}
+
+int membuff_avail(struct membuff *mb)
+{
+ struct membuff copy;
+ int i, avail;
+ char *data = 0;
+
+ /* make a copy of this buffer's control data */
+ copy = *mb;
+
+ /* now read everything out of the copied buffer */
+ for (i = avail = 0; i < 2; i++)
+ avail += membuff_getraw(©, -1, true, &data);
+
+ /* and return how much we read */
+ return avail;
+}
+
+int membuff_size(struct membuff *mb)
+{
+ return mb->end - mb->start;
+}
+
+bool membuff_makecontig(struct membuff *mb)
+{
+ int topsize, botsize;
+
+ debug("makecontig: head=%d, tail=%d, size=%d",
+ (int)(mb->head - mb->start), (int)(mb->tail - mb->start),
+ (int)(mb->end - mb->start));
+
+ /*
+ * first we move anything at the start of the buffer into the correct
+ * place some way along
+ */
+ if (mb->tail > mb->head) {
+ /*
+ * the data is split into two parts, from 0 to ->head and
+ * from ->tail to ->end. We move the stuff from 0 to ->head
+ * up to make space for the other data before it
+ */
+ topsize = mb->end - mb->tail;
+ botsize = mb->head - mb->start;
+
+ /*
+ * must move data at bottom up by 'topsize' bytes - check if
+ * there's room
+ */
+ if (mb->head + topsize >= mb->tail)
+ return false;
+ memmove(mb->start + topsize, mb->start, botsize);
+ debug(" - memmove(%d, %d, %d)", topsize, 0, botsize);
+
+ /* nothing at the start, so skip that step */
+ } else {
+ topsize = mb->head - mb->tail;
+ botsize = 0;
+ }
+
+ /* now move data at top down to the bottom */
+ memcpy(mb->start, mb->tail, topsize);
+ debug(" - memcpy(%d, %d, %d)", 0, (int)(mb->tail - mb->start), topsize);
+
+ /* adjust pointers */
+ mb->tail = mb->start;
+ mb->head = mb->start + topsize + botsize;
+
+ debug(" - head=%d, tail=%d", (int)(mb->head - mb->start),
+ (int)(mb->tail - mb->start));
+
+ /* all ok */
+ return true;
+}
+
+int membuff_free(struct membuff *mb)
+{
+ return mb->end == mb->start ? 0 :
+ (mb->end - mb->start) - 1 - membuff_avail(mb);
+}
+
+int membuff_readline(struct membuff *mb, char *str, int maxlen, int minch)
+{
+ int len; /* number of bytes read (!= string length) */
+ char *s, *end;
+ bool ok = false;
+ char *orig = str;
+
+ end = mb->head >= mb->tail ? mb->head : mb->end;
+ for (len = 0, s = mb->tail; s < end && len < maxlen - 1; str++) {
+ *str = *s++;
+ len++;
+ if (*str == '\n' || *str < minch) {
+ ok = true;
+ break;
+ }
+ if (s == end && mb->tail > mb->head) {
+ s = mb->start;
+ end = mb->head;
+ }
+ }
+
+ /* couldn't get the whole string */
+ if (!ok) {
+ if (maxlen)
+ *orig = '\0';
+ return 0;
+ }
+
+ /* terminate the string, update the membuff and return success */
+ *str = '\0';
+ mb->tail = s == mb->end ? mb->start : s;
+
+ return len;
+}
+
+int membuff_extend_by(struct membuff *mb, int by, int max)
+{
+ int oldhead, oldtail;
+ int size, orig;
+ char *ptr;
+
+ /* double the buffer size until it is big enough */
+ assert(by >= 0);
+ for (orig = mb->end - mb->start, size = orig; size < orig + by;)
+ size *= 2;
+ if (max != -1)
+ size = min(size, max);
+ by = size - orig;
+
+ /* if we're already@maximum, give up */
+ if (by <= 0)
+ return -E2BIG;
+
+ oldhead = mb->head - mb->start;
+ oldtail = mb->tail - mb->start;
+ ptr = realloc(mb->start, size);
+ if (!ptr)
+ return -ENOMEM;
+ mb->start = ptr;
+ mb->head = mb->start + oldhead;
+ mb->tail = mb->start + oldtail;
+
+ if (mb->head < mb->tail) {
+ memmove(mb->tail + by, mb->tail, orig - oldtail);
+ mb->tail += by;
+ }
+ mb->end = mb->start + size;
+
+ return 0;
+}
+
+void membuff_init(struct membuff *mb, char *buff, int size)
+{
+ mb->start = buff;
+ mb->end = mb->start + size;
+ membuff_purge(mb);
+}
+
+int membuff_new(struct membuff *mb, int size)
+{
+ mb->start = malloc(size);
+ if (!mb->start)
+ return -ENOMEM;
+
+ membuff_init(mb, mb->start, size);
+ return 0;
+}
+
+void membuff_uninit(struct membuff *mb)
+{
+ mb->end = NULL;
+ mb->start = NULL;
+ membuff_purge(mb);
+}
+
+void membuff_dispose(struct membuff *mb)
+{
+ free(&mb->start);
+ membuff_uninit(mb);
+}
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply related [flat|nested] 67+ messages in thread* [U-Boot] [PATCH v2 06/26] console: Add a console buffer
2015-11-09 6:47 [U-Boot] [PATCH v2 00/26] usb: Drop requirement for USB unbinding, add tests Simon Glass
` (4 preceding siblings ...)
2015-11-09 6:47 ` [U-Boot] [PATCH v2 05/26] Add a circular memory buffer implementation Simon Glass
@ 2015-11-09 6:47 ` Simon Glass
2015-11-20 3:31 ` Simon Glass
2015-11-09 6:47 ` [U-Boot] [PATCH v2 07/26] sandbox: Enable console recording and silent console Simon Glass
` (21 subsequent siblings)
27 siblings, 1 reply; 67+ messages in thread
From: Simon Glass @ 2015-11-09 6:47 UTC (permalink / raw)
To: u-boot
It is useful to be able to record console output and provide console input
via a buffer. This provides sandbox with the ability to run a command and
check its output. If the console is set to silent then no visible output
is generated.
This also provides a means to fix the problem where tests produce unwanted
output, such as errors or warnings. This can be confusing. We can instead
set the console to silent and record this output. It can be checked later
in the test if required.
It is possible that this may prove useful for non-test situations. For
example the console output may be suppressed for normal operations, but
recorded and stored for access by the OS. That feature is not implemented
at present.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None
common/Kconfig | 28 ++++++++++++++++++++++
common/board_f.c | 10 ++++++++
common/board_r.c | 10 ++++++++
common/console.c | 50 ++++++++++++++++++++++++++++++++++++++-
include/asm-generic/global_data.h | 6 +++++
include/console.h | 22 +++++++++++++++++
6 files changed, 125 insertions(+), 1 deletion(-)
diff --git a/common/Kconfig b/common/Kconfig
index 0388a6c..d56d950 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -659,3 +659,31 @@ config CMD_TPM_TEST
endmenu
endmenu
+
+config CONSOLE_RECORD
+ bool "Console recording"
+ help
+ This provides a way to record console output (and provide console
+ input) through cirular buffers. This is mostly useful for testing.
+ Console output is recorded even when the console is silent.
+ To enable console recording, call console_record_reset_enable()
+ from your code.
+
+config CONSOLE_RECORD_OUT_SIZE
+ hex "Output buffer size"
+ depends on CONSOLE_RECORD
+ default 0x400 if CONSOLE_RECORD
+ help
+ Set the size of the console output buffer. When this fills up, no
+ more data will be recorded until some is removed. The buffer is
+ allocated immediately after the malloc() region is ready.
+
+config CONSOLE_RECORD_IN_SIZE
+ hex "Input buffer size"
+ depends on CONSOLE_RECORD
+ default 0x100 if CONSOLE_RECORD
+ help
+ Set the size of the console input buffer. When this contains data,
+ tstc() and getc() will use this in preference to real device input.
+ The buffer is allocated immediately after the malloc() region is
+ ready.
diff --git a/common/board_f.c b/common/board_f.c
index dbb987f..62d86dd 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -737,6 +737,15 @@ static int mark_bootstage(void)
return 0;
}
+static int initf_console_record(void)
+{
+#if defined(CONFIG_CONSOLE_RECORD) && defined(CONFIG_SYS_MALLOC_F_LEN)
+ return console_record_init();
+#else
+ return 0;
+#endif
+}
+
static int initf_dm(void)
{
#if defined(CONFIG_DM) && defined(CONFIG_SYS_MALLOC_F_LEN)
@@ -773,6 +782,7 @@ static init_fnc_t init_sequence_f[] = {
trace_early_init,
#endif
initf_malloc,
+ initf_console_record,
#if defined(CONFIG_MPC85xx) || defined(CONFIG_MPC86xx)
/* TODO: can this go into arch_cpu_init()? */
probecpu,
diff --git a/common/board_r.c b/common/board_r.c
index 5a46b47..f236ffd 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -277,6 +277,15 @@ static int initr_malloc(void)
return 0;
}
+static int initr_console_record(void)
+{
+#if defined(CONFIG_CONSOLE_RECORD)
+ return console_record_init();
+#else
+ return 0;
+#endif
+}
+
#ifdef CONFIG_SYS_NONCACHED_MEMORY
static int initr_noncached(void)
{
@@ -716,6 +725,7 @@ init_fnc_t init_sequence_r[] = {
#endif
initr_barrier,
initr_malloc,
+ initr_console_record,
#ifdef CONFIG_SYS_NONCACHED_MEMORY
initr_noncached,
#endif
diff --git a/common/console.c b/common/console.c
index 10972b0..b3f126c 100644
--- a/common/console.c
+++ b/common/console.c
@@ -378,6 +378,15 @@ int getc(void)
if (!gd->have_console)
return 0;
+#ifdef CONFIG_CONSOLE_RECORD
+ if (gd->console_in.start) {
+ int ch;
+
+ ch = membuff_getbyte(&gd->console_in);
+ if (ch != -1)
+ return 1;
+ }
+#endif
if (gd->flags & GD_FLG_DEVINIT) {
/* Get from the standard input */
return fgetc(stdin);
@@ -396,7 +405,12 @@ int tstc(void)
if (!gd->have_console)
return 0;
-
+#ifdef CONFIG_CONSOLE_RECORD
+ if (gd->console_in.start) {
+ if (membuff_peekbyte(&gd->console_in) != -1)
+ return 1;
+ }
+#endif
if (gd->flags & GD_FLG_DEVINIT) {
/* Test the standard input */
return ftstc(stdin);
@@ -470,6 +484,10 @@ void putc(const char c)
return;
}
#endif
+#ifdef CONFIG_CONSOLE_RECORD
+ if (gd && (gd->flags & GD_FLG_RECORD) && gd->console_out.start)
+ membuff_putbyte(&gd->console_out, c);
+#endif
#ifdef CONFIG_SILENT_CONSOLE
if (gd->flags & GD_FLG_SILENT)
return;
@@ -513,6 +531,10 @@ void puts(const char *s)
return;
}
#endif
+#ifdef CONFIG_CONSOLE_RECORD
+ if (gd && (gd->flags & GD_FLG_RECORD) && gd->console_out.start)
+ membuff_put(&gd->console_out, s, strlen(s));
+#endif
#ifdef CONFIG_SILENT_CONSOLE
if (gd->flags & GD_FLG_SILENT)
return;
@@ -575,6 +597,32 @@ int vprintf(const char *fmt, va_list args)
return i;
}
+#ifdef CONFIG_CONSOLE_RECORD
+int console_record_init(void)
+{
+ int ret;
+
+ ret = membuff_new(&gd->console_out, CONFIG_CONSOLE_RECORD_OUT_SIZE);
+ if (ret)
+ return ret;
+ ret = membuff_new(&gd->console_in, CONFIG_CONSOLE_RECORD_IN_SIZE);
+
+ return ret;
+}
+
+void console_record_reset(void)
+{
+ membuff_purge(&gd->console_out);
+ membuff_purge(&gd->console_in);
+}
+
+void console_record_reset_enable(void)
+{
+ console_record_reset();
+ gd->flags |= GD_FLG_RECORD;
+}
+#endif
+
/* test if ctrl-c was pressed */
static int ctrlc_disabled = 0; /* see disable_ctrl() */
static int ctrlc_was_pressed = 0;
diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
index d0383f3..1abdcaa 100644
--- a/include/asm-generic/global_data.h
+++ b/include/asm-generic/global_data.h
@@ -21,6 +21,7 @@
*/
#ifndef __ASSEMBLY__
+#include <membuff.h>
#include <linux/list.h>
typedef struct global_data {
@@ -103,6 +104,10 @@ typedef struct global_data {
#endif
struct udevice *cur_serial_dev; /* current serial device */
struct arch_global_data arch; /* architecture-specific data */
+#ifdef CONFIG_CONSOLE_RECORD
+ struct membuff console_out; /* console output */
+ struct membuff console_in; /* console input */
+#endif
} gd_t;
#endif
@@ -121,5 +126,6 @@ typedef struct global_data {
#define GD_FLG_FULL_MALLOC_INIT 0x00200 /* Full malloc() is ready */
#define GD_FLG_SPL_INIT 0x00400 /* spl_init() has been called */
#define GD_FLG_SKIP_RELOC 0x00800 /* Don't relocate */
+#define GD_FLG_RECORD 0x01000 /* Record console */
#endif /* __ASM_GENERIC_GBL_DATA_H */
diff --git a/include/console.h b/include/console.h
index 097518d..3d37f6a 100644
--- a/include/console.h
+++ b/include/console.h
@@ -20,6 +20,28 @@ void clear_ctrlc(void); /* clear the Control-C condition */
int disable_ctrlc(int); /* 1 to disable, 0 to enable Control-C detect */
int confirm_yesno(void); /* 1 if input is "y", "Y", "yes" or "YES" */
+/**
+ * console_record_init() - set up the console recording buffers
+ *
+ * This should be called as soon as malloc() is available so that the maximum
+ * amount of console output can be recorded.
+ */
+int console_record_init(void);
+
+/**
+ * console_record_reset() - reset the console recording buffers
+ *
+ * Removes any data in the buffers
+ */
+void console_record_reset(void);
+
+/**
+ * console_record_reset_enable() - reset and enable the console buffers
+ *
+ * This should be called to enable the console buffer.
+ */
+void console_record_reset_enable(void);
+
/*
* CONSOLE multiplexing.
*/
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply related [flat|nested] 67+ messages in thread* [U-Boot] [PATCH v2 07/26] sandbox: Enable console recording and silent console
2015-11-09 6:47 [U-Boot] [PATCH v2 00/26] usb: Drop requirement for USB unbinding, add tests Simon Glass
` (5 preceding siblings ...)
2015-11-09 6:47 ` [U-Boot] [PATCH v2 06/26] console: Add a console buffer Simon Glass
@ 2015-11-09 6:47 ` Simon Glass
2015-11-20 3:31 ` Simon Glass
2015-11-09 6:47 ` [U-Boot] [PATCH v2 08/26] test: Record and silence console in tests Simon Glass
` (20 subsequent siblings)
27 siblings, 1 reply; 67+ messages in thread
From: Simon Glass @ 2015-11-09 6:47 UTC (permalink / raw)
To: u-boot
Allow console recording so that tests can use it. Also allow the console
output to be suppressed, to reduce test output 'noise'.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None
configs/sandbox_defconfig | 3 +++
include/configs/sandbox.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 7fc8792..ac3cce2 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -69,3 +69,6 @@ CONFIG_REMOTEPROC_SANDBOX=y
CONFIG_CMD_REMOTEPROC=y
CONFIG_ADC=y
CONFIG_ADC_SANDBOX=y
+CONFIG_CONSOLE_RECORD=y
+CONFIG_CONSOLE_RECORD_OUT_SIZE=0x1000
+CONFIG_SYS_MALLOC_F_LEN=0x2000
diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h
index 9e66da2..f52488b 100644
--- a/include/configs/sandbox.h
+++ b/include/configs/sandbox.h
@@ -66,6 +66,7 @@
#define CONFIG_SYS_HUSH_PARSER
#define CONFIG_SYS_LONGHELP /* #undef to save memory */
#define CONFIG_SYS_CBSIZE 1024 /* Console I/O Buffer Size */
+#define CONFIG_SILENT_CONSOLE
/* Print Buffer Size */
#define CONFIG_SYS_PBSIZE (CONFIG_SYS_CBSIZE + sizeof(CONFIG_SYS_PROMPT) + 16)
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply related [flat|nested] 67+ messages in thread* [U-Boot] [PATCH v2 08/26] test: Record and silence console in tests
2015-11-09 6:47 [U-Boot] [PATCH v2 00/26] usb: Drop requirement for USB unbinding, add tests Simon Glass
` (6 preceding siblings ...)
2015-11-09 6:47 ` [U-Boot] [PATCH v2 07/26] sandbox: Enable console recording and silent console Simon Glass
@ 2015-11-09 6:47 ` Simon Glass
2015-11-20 3:31 ` Simon Glass
2015-11-09 6:47 ` [U-Boot] [PATCH v2 09/26] usb: Refactor USB tree output code for testing Simon Glass
` (19 subsequent siblings)
27 siblings, 1 reply; 67+ messages in thread
From: Simon Glass @ 2015-11-09 6:47 UTC (permalink / raw)
To: u-boot
When running sandbox tests, silence the console to avoid unwanted output.
Also, record the console in case tests want to check it.
The -v option can be used to enable stdout during tests.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None
arch/sandbox/cpu/start.c | 8 ++++++++
arch/sandbox/include/asm/state.h | 1 +
test/dm/test-main.c | 9 +++++++++
test/ut.c | 4 ++++
4 files changed, 22 insertions(+)
diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c
index 4c38fab..0dda4fc 100644
--- a/arch/sandbox/cpu/start.c
+++ b/arch/sandbox/cpu/start.c
@@ -257,6 +257,14 @@ static int sandbox_cmdline_cb_terminal(struct sandbox_state *state,
SANDBOX_CMDLINE_OPT_SHORT(terminal, 't', 1,
"Set terminal to raw/cooked mode");
+static int sandbox_cmdline_cb_verbose(struct sandbox_state *state,
+ const char *arg)
+{
+ state->show_test_output = true;
+ return 0;
+}
+SANDBOX_CMDLINE_OPT_SHORT(verbose, 'v', 0, "Show test output");
+
int main(int argc, char *argv[])
{
struct sandbox_state *state;
diff --git a/arch/sandbox/include/asm/state.h b/arch/sandbox/include/asm/state.h
index e876ba2..11856c2 100644
--- a/arch/sandbox/include/asm/state.h
+++ b/arch/sandbox/include/asm/state.h
@@ -64,6 +64,7 @@ struct sandbox_state {
bool reset_allowed[RESET_COUNT]; /* Allowed reset types */
enum state_terminal_raw term_raw; /* Terminal raw/cooked */
bool skip_delays; /* Ignore any time delays (for test) */
+ bool show_test_output; /* Don't suppress stdout in tests */
/* Pointer to information for each SPI bus/cs */
struct sandbox_spi_info spi[CONFIG_SANDBOX_SPI_MAX_BUS]
diff --git a/test/dm/test-main.c b/test/dm/test-main.c
index 867b517..91bdda8 100644
--- a/test/dm/test-main.c
+++ b/test/dm/test-main.c
@@ -76,6 +76,7 @@ static int dm_test_main(const char *test_name)
struct unit_test *tests = ll_entry_start(struct unit_test, dm_test);
const int n_ents = ll_entry_count(struct unit_test, dm_test);
struct unit_test_state *uts = &global_dm_test_state;
+ struct sandbox_state *state = state_get_current();
uts->priv = &_global_priv_dm_test_state;
struct unit_test *test;
int run_count;
@@ -114,7 +115,15 @@ static int dm_test_main(const char *test_name)
if (test->flags & DM_TESTF_SCAN_FDT)
ut_assertok(dm_scan_fdt(gd->fdt_blob, false));
+ /*
+ * Silence the console and rely on console reocrding to get
+ * our output.
+ */
+ console_record_reset();
+ if (!state->show_test_output)
+ gd->flags |= GD_FLG_SILENT;
test->func(uts);
+ gd->flags &= ~GD_FLG_SILENT;
state_set_skip_delays(false);
ut_assertok(dm_test_destroy(uts));
diff --git a/test/ut.c b/test/ut.c
index 0282de5..fa0f02d 100644
--- a/test/ut.c
+++ b/test/ut.c
@@ -10,9 +10,12 @@
#include <test/test.h>
#include <test/ut.h>
+DECLARE_GLOBAL_DATA_PTR;
+
void ut_fail(struct unit_test_state *uts, const char *fname, int line,
const char *func, const char *cond)
{
+ gd->flags &= ~(GD_FLG_SILENT | GD_FLG_RECORD);
printf("%s:%d, %s(): %s\n", fname, line, func, cond);
uts->fail_count++;
}
@@ -22,6 +25,7 @@ void ut_failf(struct unit_test_state *uts, const char *fname, int line,
{
va_list args;
+ gd->flags &= ~(GD_FLG_SILENT | GD_FLG_RECORD);
printf("%s:%d, %s(): %s: ", fname, line, func, cond);
va_start(args, fmt);
vprintf(fmt, args);
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply related [flat|nested] 67+ messages in thread* [U-Boot] [PATCH v2 09/26] usb: Refactor USB tree output code for testing
2015-11-09 6:47 [U-Boot] [PATCH v2 00/26] usb: Drop requirement for USB unbinding, add tests Simon Glass
` (7 preceding siblings ...)
2015-11-09 6:47 ` [U-Boot] [PATCH v2 08/26] test: Record and silence console in tests Simon Glass
@ 2015-11-09 6:47 ` Simon Glass
2015-11-20 3:31 ` Simon Glass
2015-11-09 6:47 ` [U-Boot] [PATCH v2 10/26] dm: core: Add safe device iteration macros Simon Glass
` (18 subsequent siblings)
27 siblings, 1 reply; 67+ messages in thread
From: Simon Glass @ 2015-11-09 6:47 UTC (permalink / raw)
To: u-boot
Allow the 'usb tree' command to be used from test code, so that we can
verify that it works correctly.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None
common/cmd_usb.c | 58 ++++++++++++++++++++++++++++++++------------------------
include/usb.h | 8 ++++++++
2 files changed, 41 insertions(+), 25 deletions(-)
diff --git a/common/cmd_usb.c b/common/cmd_usb.c
index c2d9803..a540b42 100644
--- a/common/cmd_usb.c
+++ b/common/cmd_usb.c
@@ -429,7 +429,7 @@ static void usb_show_tree_graph(struct usb_device *dev, char *pre)
}
/* main routine for the tree command */
-static void usb_show_tree(struct usb_device *dev)
+static void usb_show_subtree(struct usb_device *dev)
{
char preamble[32];
@@ -437,6 +437,37 @@ static void usb_show_tree(struct usb_device *dev)
usb_show_tree_graph(dev, &preamble[0]);
}
+void usb_show_tree(void)
+{
+#ifdef CONFIG_DM_USB
+ struct udevice *bus;
+
+ for (uclass_first_device(UCLASS_USB, &bus);
+ bus;
+ uclass_next_device(&bus)) {
+ struct usb_device *udev;
+ struct udevice *dev;
+
+ device_find_first_child(bus, &dev);
+ if (dev && device_active(dev)) {
+ udev = dev_get_parent_priv(dev);
+ usb_show_subtree(udev);
+ }
+ }
+#else
+ struct usb_device *udev;
+ int i;
+
+ for (i = 0; i < USB_MAX_DEVICE; i++) {
+ udev = usb_get_dev_index(i);
+ if (udev == NULL)
+ break;
+ if (udev->parent == NULL)
+ usb_show_subtree(udev);
+ }
+#endif
+}
+
static int usb_test(struct usb_device *dev, int port, char* arg)
{
int mode;
@@ -631,30 +662,7 @@ static int do_usb(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
}
if (strncmp(argv[1], "tree", 4) == 0) {
puts("USB device tree:\n");
-#ifdef CONFIG_DM_USB
- struct udevice *bus;
-
- for (uclass_first_device(UCLASS_USB, &bus);
- bus;
- uclass_next_device(&bus)) {
- struct usb_device *udev;
- struct udevice *dev;
-
- device_find_first_child(bus, &dev);
- if (dev && device_active(dev)) {
- udev = dev_get_parent_priv(dev);
- usb_show_tree(udev);
- }
- }
-#else
- for (i = 0; i < USB_MAX_DEVICE; i++) {
- udev = usb_get_dev_index(i);
- if (udev == NULL)
- break;
- if (udev->parent == NULL)
- usb_show_tree(udev);
- }
-#endif
+ usb_show_tree();
return 0;
}
if (strncmp(argv[1], "inf", 3) == 0) {
diff --git a/include/usb.h b/include/usb.h
index 3d0facb..a09c7f1 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -957,4 +957,12 @@ int usb_emul_find(struct udevice *bus, ulong pipe, struct udevice **emulp);
*/
void usb_emul_reset(struct udevice *dev);
+/**
+ * usb_show_tree() - show the USB device tree
+ *
+ * This shows a list of active USB devices along with basic information about
+ * each.
+ */
+void usb_show_tree(void);
+
#endif /*_USB_H_ */
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply related [flat|nested] 67+ messages in thread* [U-Boot] [PATCH v2 10/26] dm: core: Add safe device iteration macros
2015-11-09 6:47 [U-Boot] [PATCH v2 00/26] usb: Drop requirement for USB unbinding, add tests Simon Glass
` (8 preceding siblings ...)
2015-11-09 6:47 ` [U-Boot] [PATCH v2 09/26] usb: Refactor USB tree output code for testing Simon Glass
@ 2015-11-09 6:47 ` Simon Glass
2015-11-20 3:31 ` Simon Glass
2015-11-09 6:47 ` [U-Boot] [PATCH v2 11/26] sandbox: usb: Allow dynamic emulated USB device descriptors Simon Glass
` (17 subsequent siblings)
27 siblings, 1 reply; 67+ messages in thread
From: Simon Glass @ 2015-11-09 6:47 UTC (permalink / raw)
To: u-boot
Add iteration macros which support unbinding a device within the loop.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None
include/dm/device.h | 12 ++++++++++++
include/dm/uclass.h | 15 +++++++++++++++
2 files changed, 27 insertions(+)
diff --git a/include/dm/device.h b/include/dm/device.h
index 28ba4ca..7fb9935 100644
--- a/include/dm/device.h
+++ b/include/dm/device.h
@@ -509,6 +509,18 @@ static inline bool device_is_on_pci_bus(struct udevice *dev)
return device_get_uclass_id(dev->parent) == UCLASS_PCI;
}
+/**
+ * device_foreach_child_safe() - iterate through child devices safely
+ *
+ * This allows the @pos child to be removed in the loop if required.
+ *
+ * @pos: struct udevice * for the current device
+ * @next: struct udevice * for the next device
+ * @parent: parent device to scan
+ */
+#define device_foreach_child_safe(pos, next, parent) \
+ list_for_each_entry_safe(pos, next, &parent->child_head, sibling_node)
+
/* device resource management */
typedef void (*dr_release_t)(struct udevice *dev, void *res);
typedef int (*dr_match_t)(struct udevice *dev, void *res, void *match_data);
diff --git a/include/dm/uclass.h b/include/dm/uclass.h
index d214b88..bfbd27a 100644
--- a/include/dm/uclass.h
+++ b/include/dm/uclass.h
@@ -243,4 +243,19 @@ int uclass_resolve_seq(struct udevice *dev);
#define uclass_foreach_dev(pos, uc) \
list_for_each_entry(pos, &uc->dev_head, uclass_node)
+/**
+ * uclass_foreach_dev_safe() - Helper function to safely iteration through devs
+ *
+ * This creates a for() loop which works through the available devices in
+ * a uclass in order from start to end. Inside the loop, it is safe to remove
+ * @pos if required.
+ *
+ * @pos: struct udevice * to hold the current device. Set to NULL when there
+ * are no more devices.
+ * @next: struct udevice * to hold the next next
+ * @uc: uclass to scan
+ */
+#define uclass_foreach_dev_safe(pos, next, uc) \
+ list_for_each_entry_safe(pos, next, &uc->dev_head, uclass_node)
+
#endif
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply related [flat|nested] 67+ messages in thread* [U-Boot] [PATCH v2 11/26] sandbox: usb: Allow dynamic emulated USB device descriptors
2015-11-09 6:47 [U-Boot] [PATCH v2 00/26] usb: Drop requirement for USB unbinding, add tests Simon Glass
` (9 preceding siblings ...)
2015-11-09 6:47 ` [U-Boot] [PATCH v2 10/26] dm: core: Add safe device iteration macros Simon Glass
@ 2015-11-09 6:47 ` Simon Glass
2015-11-20 3:31 ` Simon Glass
2015-11-09 6:47 ` [U-Boot] [PATCH v2 12/26] sandbox: usb: Allow up to 4 emulated devices on a hub Simon Glass
` (16 subsequent siblings)
27 siblings, 1 reply; 67+ messages in thread
From: Simon Glass @ 2015-11-09 6:47 UTC (permalink / raw)
To: u-boot
We would like the serial number to come from the device tree node name of
the emulated device. This avoids them all having the same name. Adjust the
code to support this.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None
drivers/usb/emul/sandbox_flash.c | 48 +++++++++++++++++++++++-----------------
1 file changed, 28 insertions(+), 20 deletions(-)
diff --git a/drivers/usb/emul/sandbox_flash.c b/drivers/usb/emul/sandbox_flash.c
index 6e0808d..0965ad0 100644
--- a/drivers/usb/emul/sandbox_flash.c
+++ b/drivers/usb/emul/sandbox_flash.c
@@ -31,6 +31,14 @@ enum cmd_phase {
PHASE_STATUS,
};
+enum {
+ STRINGID_MANUFACTURER = 1,
+ STRINGID_PRODUCT,
+ STRINGID_SERIAL,
+
+ STRINGID_COUNT,
+};
+
/**
* struct sandbox_flash_priv - private state for this driver
*
@@ -61,6 +69,7 @@ struct sandbox_flash_priv {
struct sandbox_flash_plat {
const char *pathname;
+ struct usb_string flash_strings[STRINGID_COUNT];
};
struct scsi_inquiry_resp {
@@ -89,21 +98,6 @@ struct __packed scsi_read10_req {
u8 spare2[3];
};
-enum {
- STRINGID_MANUFACTURER = 1,
- STRINGID_PRODUCT,
- STRINGID_SERIAL,
-
- STRINGID_COUNT,
-};
-
-static struct usb_string flash_strings[] = {
- {STRINGID_MANUFACTURER, "sandbox"},
- {STRINGID_PRODUCT, "flash"},
- {STRINGID_SERIAL, "2345"},
- {},
-};
-
static struct usb_device_descriptor flash_device_desc = {
.bLength = sizeof(flash_device_desc),
.bDescriptorType = USB_DT_DEVICE,
@@ -246,7 +240,8 @@ static void handle_read(struct sandbox_flash_priv *priv, ulong lba,
}
}
-static int handle_ufi_command(struct sandbox_flash_priv *priv, const void *buff,
+static int handle_ufi_command(struct sandbox_flash_plat *plat,
+ struct sandbox_flash_priv *priv, const void *buff,
int len)
{
const struct SCSI_cmd_block *req = buff;
@@ -260,9 +255,10 @@ static int handle_ufi_command(struct sandbox_flash_priv *priv, const void *buff,
resp->data_format = 1;
resp->additional_len = 0x1f;
strncpy(resp->vendor,
- flash_strings[STRINGID_MANUFACTURER - 1].s,
+ plat->flash_strings[STRINGID_MANUFACTURER - 1].s,
sizeof(resp->vendor));
- strncpy(resp->product, flash_strings[STRINGID_PRODUCT - 1].s,
+ strncpy(resp->product,
+ plat->flash_strings[STRINGID_PRODUCT - 1].s,
sizeof(resp->product));
strncpy(resp->revision, "1.0", sizeof(resp->revision));
setup_response(priv, resp, sizeof(*resp));
@@ -303,6 +299,7 @@ static int handle_ufi_command(struct sandbox_flash_priv *priv, const void *buff,
static int sandbox_flash_bulk(struct udevice *dev, struct usb_device *udev,
unsigned long pipe, void *buff, int len)
{
+ struct sandbox_flash_plat *plat = dev_get_platdata(dev);
struct sandbox_flash_priv *priv = dev_get_priv(dev);
int ep = usb_pipeendpoint(pipe);
struct umass_bbb_cbw *cbw = buff;
@@ -325,7 +322,7 @@ static int sandbox_flash_bulk(struct udevice *dev, struct usb_device *udev,
goto err;
priv->transfer_len = cbw->dCBWDataTransferLength;
priv->tag = cbw->dCBWTag;
- return handle_ufi_command(priv, cbw->CBWCDB,
+ return handle_ufi_command(plat, priv, cbw->CBWCDB,
cbw->bCDBLength);
case PHASE_DATA:
debug("data out\n");
@@ -384,7 +381,18 @@ static int sandbox_flash_ofdata_to_platdata(struct udevice *dev)
static int sandbox_flash_bind(struct udevice *dev)
{
- return usb_emul_setup_device(dev, PACKET_SIZE_64, flash_strings,
+ struct sandbox_flash_plat *plat = dev_get_platdata(dev);
+ struct usb_string *fs;
+
+ fs = plat->flash_strings;
+ fs[0].id = STRINGID_MANUFACTURER;
+ fs[0].s = "sandbox";
+ fs[1].id = STRINGID_PRODUCT;
+ fs[1].s = "flash";
+ fs[2].id = STRINGID_SERIAL;
+ fs[2].s = dev->name;
+
+ return usb_emul_setup_device(dev, PACKET_SIZE_64, plat->flash_strings,
flash_desc_list);
}
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply related [flat|nested] 67+ messages in thread* [U-Boot] [PATCH v2 12/26] sandbox: usb: Allow up to 4 emulated devices on a hub
2015-11-09 6:47 [U-Boot] [PATCH v2 00/26] usb: Drop requirement for USB unbinding, add tests Simon Glass
` (10 preceding siblings ...)
2015-11-09 6:47 ` [U-Boot] [PATCH v2 11/26] sandbox: usb: Allow dynamic emulated USB device descriptors Simon Glass
@ 2015-11-09 6:47 ` Simon Glass
2015-11-20 3:31 ` Simon Glass
2015-11-09 6:47 ` [U-Boot] [PATCH v2 13/26] sandbox: usb: Allow finding a USB emulator for a device Simon Glass
` (15 subsequent siblings)
27 siblings, 1 reply; 67+ messages in thread
From: Simon Glass @ 2015-11-09 6:47 UTC (permalink / raw)
To: u-boot
To support more advanced testing, support 4 devices instead of 2.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None
drivers/usb/emul/sandbox_hub.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/emul/sandbox_hub.c b/drivers/usb/emul/sandbox_hub.c
index baf8bdc..624fbde 100644
--- a/drivers/usb/emul/sandbox_hub.c
+++ b/drivers/usb/emul/sandbox_hub.c
@@ -13,7 +13,7 @@
DECLARE_GLOBAL_DATA_PTR;
/* We only support up to 8 */
-#define SANDBOX_NUM_PORTS 2
+#define SANDBOX_NUM_PORTS 4
struct sandbox_hub_platdata {
struct usb_dev_platdata plat;
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply related [flat|nested] 67+ messages in thread* [U-Boot] [PATCH v2 13/26] sandbox: usb: Allow finding a USB emulator for a device
2015-11-09 6:47 [U-Boot] [PATCH v2 00/26] usb: Drop requirement for USB unbinding, add tests Simon Glass
` (11 preceding siblings ...)
2015-11-09 6:47 ` [U-Boot] [PATCH v2 12/26] sandbox: usb: Allow up to 4 emulated devices on a hub Simon Glass
@ 2015-11-09 6:47 ` Simon Glass
2015-11-20 3:31 ` Simon Glass
2015-11-09 6:47 ` [U-Boot] [PATCH v2 14/26] Revert "dm: usb: Rename usb_find_child to usb_find_emul_child" Simon Glass
` (14 subsequent siblings)
27 siblings, 1 reply; 67+ messages in thread
From: Simon Glass @ 2015-11-09 6:47 UTC (permalink / raw)
To: u-boot
Each USB device has an emulator. Currently this can only be found by
supplying the 'pipe' value, which contains the device number. Add a way
to find it directly from the emulated device.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None
drivers/usb/emul/usb-emul-uclass.c | 17 +++++++++++++++--
include/usb.h | 10 ++++++++++
2 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/emul/usb-emul-uclass.c b/drivers/usb/emul/usb-emul-uclass.c
index 205f2c5..6b5f3c0 100644
--- a/drivers/usb/emul/usb-emul-uclass.c
+++ b/drivers/usb/emul/usb-emul-uclass.c
@@ -108,9 +108,8 @@ static int usb_emul_get_descriptor(struct usb_dev_platdata *plat, int value,
return upto ? upto : length ? -EIO : 0;
}
-int usb_emul_find(struct udevice *bus, ulong pipe, struct udevice **emulp)
+static int usb_emul_find_devnum(int devnum, struct udevice **emulp)
{
- int devnum = usb_pipedevice(pipe);
struct udevice *dev;
struct uclass *uc;
int ret;
@@ -134,6 +133,20 @@ int usb_emul_find(struct udevice *bus, ulong pipe, struct udevice **emulp)
return -ENOENT;
}
+int usb_emul_find(struct udevice *bus, ulong pipe, struct udevice **emulp)
+{
+ int devnum = usb_pipedevice(pipe);
+
+ return usb_emul_find_devnum(devnum, emulp);
+}
+
+int usb_emul_find_for_dev(struct udevice *dev, struct udevice **emulp)
+{
+ struct usb_dev_platdata *udev = dev_get_parent_platdata(dev);
+
+ return usb_emul_find_devnum(udev->devnum, emulp);
+}
+
int usb_emul_control(struct udevice *emul, struct usb_device *udev,
unsigned long pipe, void *buffer, int length,
struct devrequest *setup)
diff --git a/include/usb.h b/include/usb.h
index a09c7f1..d684531 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -950,6 +950,16 @@ int usb_emul_bulk(struct udevice *emul, struct usb_device *udev,
int usb_emul_find(struct udevice *bus, ulong pipe, struct udevice **emulp);
/**
+ * usb_emul_find_for_dev() - Find an emulator for a particular device
+ *
+ * @bus: USB bus (controller)
+ * @dev: USB device to check
+ * @emulp: Returns pointer to emulator, or NULL if not found
+ * @return 0 if found, -ve on error
+ */
+int usb_emul_find_for_dev(struct udevice *dev, struct udevice **emulp);
+
+/**
* usb_emul_reset() - Reset all emulators ready for use
*
* Clear out any address information in the emulators and make then ready for
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply related [flat|nested] 67+ messages in thread* [U-Boot] [PATCH v2 14/26] Revert "dm: usb: Rename usb_find_child to usb_find_emul_child"
2015-11-09 6:47 [U-Boot] [PATCH v2 00/26] usb: Drop requirement for USB unbinding, add tests Simon Glass
` (12 preceding siblings ...)
2015-11-09 6:47 ` [U-Boot] [PATCH v2 13/26] sandbox: usb: Allow finding a USB emulator for a device Simon Glass
@ 2015-11-09 6:47 ` Simon Glass
2015-11-20 3:31 ` Simon Glass
2015-11-09 6:47 ` [U-Boot] [PATCH v2 15/26] Revert "dm: usb: Use device_unbind_children to clean up usb devs on stop" Simon Glass
` (13 subsequent siblings)
27 siblings, 1 reply; 67+ messages in thread
From: Simon Glass @ 2015-11-09 6:47 UTC (permalink / raw)
To: u-boot
This reverts commit 9b510df703d282effba4f56ac567aa8011d56e6b.
We want to avoid having the USB stack rely on unbind.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None
drivers/usb/host/usb-uclass.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index 7f6a9a6..838d05a 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -494,14 +494,15 @@ error:
}
/**
- * usb_find_emul_child() - Find an existing device for emulated devices
+ * usb_find_child() - Find an existing device which matches our needs
+ *
+ *
*/
-static int usb_find_emul_child(struct udevice *parent,
- struct usb_device_descriptor *desc,
- struct usb_interface_descriptor *iface,
- struct udevice **devp)
+static int usb_find_child(struct udevice *parent,
+ struct usb_device_descriptor *desc,
+ struct usb_interface_descriptor *iface,
+ struct udevice **devp)
{
-#ifdef CONFIG_SANDBOX
struct udevice *dev;
*devp = NULL;
@@ -520,7 +521,7 @@ static int usb_find_emul_child(struct udevice *parent,
return 0;
}
}
-#endif
+
return -ENOENT;
}
@@ -580,8 +581,8 @@ int usb_scan_device(struct udevice *parent, int port,
debug("read_descriptor for '%s': ret=%d\n", parent->name, ret);
if (ret)
return ret;
- ret = usb_find_emul_child(parent, &udev->descriptor, iface, &dev);
- debug("** usb_find_emul_child returns %d\n", ret);
+ ret = usb_find_child(parent, &udev->descriptor, iface, &dev);
+ debug("** usb_find_child returns %d\n", ret);
if (ret) {
if (ret != -ENOENT)
return ret;
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply related [flat|nested] 67+ messages in thread* [U-Boot] [PATCH v2 15/26] Revert "dm: usb: Use device_unbind_children to clean up usb devs on stop"
2015-11-09 6:47 [U-Boot] [PATCH v2 00/26] usb: Drop requirement for USB unbinding, add tests Simon Glass
` (13 preceding siblings ...)
2015-11-09 6:47 ` [U-Boot] [PATCH v2 14/26] Revert "dm: usb: Rename usb_find_child to usb_find_emul_child" Simon Glass
@ 2015-11-09 6:47 ` Simon Glass
2015-11-20 3:31 ` Simon Glass
2015-11-09 6:47 ` [U-Boot] [PATCH v2 16/26] Revert "dm: Export device_remove_children / device_unbind_children" Simon Glass
` (12 subsequent siblings)
27 siblings, 1 reply; 67+ messages in thread
From: Simon Glass @ 2015-11-09 6:47 UTC (permalink / raw)
To: u-boot
This reverts commit 6cda369509e0d3fa5f9e33c9d71589c4523799fa.
We want to avoid having the USB stack rely on unbind.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None
drivers/usb/host/usb-uclass.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index 838d05a..89611f1 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -158,9 +158,6 @@ int usb_stop(void)
ret = device_remove(bus);
if (ret && !err)
err = ret;
- ret = device_unbind_children(bus);
- if (ret && !err)
- err = ret;
}
#ifdef CONFIG_SANDBOX
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply related [flat|nested] 67+ messages in thread* [U-Boot] [PATCH v2 16/26] Revert "dm: Export device_remove_children / device_unbind_children"
2015-11-09 6:47 [U-Boot] [PATCH v2 00/26] usb: Drop requirement for USB unbinding, add tests Simon Glass
` (14 preceding siblings ...)
2015-11-09 6:47 ` [U-Boot] [PATCH v2 15/26] Revert "dm: usb: Use device_unbind_children to clean up usb devs on stop" Simon Glass
@ 2015-11-09 6:47 ` Simon Glass
2015-11-20 3:31 ` Simon Glass
2015-11-09 6:47 ` [U-Boot] [PATCH v2 17/26] dm: usb: Deprecate usb_get_dev_index() Simon Glass
` (11 subsequent siblings)
27 siblings, 1 reply; 67+ messages in thread
From: Simon Glass @ 2015-11-09 6:47 UTC (permalink / raw)
To: u-boot
This reverts commit bb52b367f6ca4a3a918e77737f4ff6a1089912d9.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None
drivers/core/device-remove.c | 22 ++++++++++++++++++----
include/dm/device-internal.h | 26 --------------------------
2 files changed, 18 insertions(+), 30 deletions(-)
diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c
index bd6d406..e1714b2 100644
--- a/drivers/core/device-remove.c
+++ b/drivers/core/device-remove.c
@@ -18,7 +18,16 @@
#include <dm/uclass-internal.h>
#include <dm/util.h>
-int device_unbind_children(struct udevice *dev)
+/**
+ * device_chld_unbind() - Unbind all device's children from the device
+ *
+ * On error, the function continues to unbind all children, and reports the
+ * first error.
+ *
+ * @dev: The device that is to be stripped of its children
+ * @return 0 on success, -ve on error
+ */
+static int device_chld_unbind(struct udevice *dev)
{
struct udevice *pos, *n;
int ret, saved_ret = 0;
@@ -34,7 +43,12 @@ int device_unbind_children(struct udevice *dev)
return saved_ret;
}
-int device_remove_children(struct udevice *dev)
+/**
+ * device_chld_remove() - Stop all device's children
+ * @dev: The device whose children are to be removed
+ * @return 0 on success, -ve on error
+ */
+static int device_chld_remove(struct udevice *dev)
{
struct udevice *pos, *n;
int ret;
@@ -73,7 +87,7 @@ int device_unbind(struct udevice *dev)
return ret;
}
- ret = device_unbind_children(dev);
+ ret = device_chld_unbind(dev);
if (ret)
return ret;
@@ -153,7 +167,7 @@ int device_remove(struct udevice *dev)
if (ret)
return ret;
- ret = device_remove_children(dev);
+ ret = device_chld_remove(dev);
if (ret)
goto err;
diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h
index 322d35a..9388870 100644
--- a/include/dm/device-internal.h
+++ b/include/dm/device-internal.h
@@ -107,32 +107,6 @@ int device_unbind(struct udevice *dev);
static inline int device_unbind(struct udevice *dev) { return 0; }
#endif
-/**
- * device_remove_children() - Stop all device's children
- * @dev: The device whose children are to be removed
- * @return 0 on success, -ve on error
- */
-#if CONFIG_IS_ENABLED(DM_DEVICE_REMOVE)
-int device_remove_children(struct udevice *dev);
-#else
-static inline int device_remove_children(struct udevice *dev) { return 0; }
-#endif
-
-/**
- * device_unbind_children() - Unbind all device's children from the device
- *
- * On error, the function continues to unbind all children, and reports the
- * first error.
- *
- * @dev: The device that is to be stripped of its children
- * @return 0 on success, -ve on error
- */
-#if CONFIG_IS_ENABLED(DM_DEVICE_REMOVE)
-int device_unbind_children(struct udevice *dev);
-#else
-static inline int device_unbind_children(struct udevice *dev) { return 0; }
-#endif
-
#if CONFIG_IS_ENABLED(DM_DEVICE_REMOVE)
void device_free(struct udevice *dev);
#else
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply related [flat|nested] 67+ messages in thread* [U-Boot] [PATCH v2 17/26] dm: usb: Deprecate usb_get_dev_index()
2015-11-09 6:47 [U-Boot] [PATCH v2 00/26] usb: Drop requirement for USB unbinding, add tests Simon Glass
` (15 preceding siblings ...)
2015-11-09 6:47 ` [U-Boot] [PATCH v2 16/26] Revert "dm: Export device_remove_children / device_unbind_children" Simon Glass
@ 2015-11-09 6:47 ` Simon Glass
2015-11-20 3:31 ` Simon Glass
2015-11-09 6:48 ` [U-Boot] [PATCH v2 18/26] dm: usb: Remove inactive children after a bus scan Simon Glass
` (10 subsequent siblings)
27 siblings, 1 reply; 67+ messages in thread
From: Simon Glass @ 2015-11-09 6:47 UTC (permalink / raw)
To: u-boot
This function should not be used with driver model. While there are users
of USB Ethernet that use driver model for USB but not Ethernet, we have
to keep it around. Add a comment to that effect.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None
drivers/usb/host/usb-uclass.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index 89611f1..4aa92f8 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -279,6 +279,14 @@ int usb_init(void)
return usb_started ? 0 : -1;
}
+/*
+ * TODO(sjg at chromium.org): Remove this legacy function. At present it is needed
+ * to support boards which use driver model for USB but not Ethernet, and want
+ * to use USB Ethernet.
+ *
+ * The #if clause is here to ensure that remains the only case.
+ */
+#if !defined(CONFIG_DM_ETH) && defined(CONFIG_USB_HOST_ETHER)
static struct usb_device *find_child_devnum(struct udevice *parent, int devnum)
{
struct usb_device *udev;
@@ -312,6 +320,7 @@ struct usb_device *usb_get_dev_index(struct udevice *bus, int index)
return find_child_devnum(dev, devnum);
}
+#endif
int usb_post_bind(struct udevice *dev)
{
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply related [flat|nested] 67+ messages in thread* [U-Boot] [PATCH v2 18/26] dm: usb: Remove inactive children after a bus scan
2015-11-09 6:47 [U-Boot] [PATCH v2 00/26] usb: Drop requirement for USB unbinding, add tests Simon Glass
` (16 preceding siblings ...)
2015-11-09 6:47 ` [U-Boot] [PATCH v2 17/26] dm: usb: Deprecate usb_get_dev_index() Simon Glass
@ 2015-11-09 6:48 ` Simon Glass
2015-11-09 8:22 ` Hans de Goede
2015-11-09 6:48 ` [U-Boot] [PATCH v2 19/26] dm: test: usb: Add tests for the 'usb tree' command Simon Glass
` (9 subsequent siblings)
27 siblings, 1 reply; 67+ messages in thread
From: Simon Glass @ 2015-11-09 6:48 UTC (permalink / raw)
To: u-boot
Each scan of the USB bus may return different results. Existing driver-model
devices are reused when found, but if a device no longer exists it will stay
around, de-activated, but bound.
Detect these devices and remove them after the scan completes.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None
drivers/usb/host/usb-uclass.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index 4aa92f8..50538e0 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -202,6 +202,20 @@ static void usb_scan_bus(struct udevice *bus, bool recurse)
printf("%d USB Device(s) found\n", priv->next_addr);
}
+static void remove_inactive_children(struct uclass *uc, struct udevice *bus)
+{
+ uclass_foreach_dev(bus, uc) {
+ struct udevice *dev, *next;
+
+ if (!device_active(bus))
+ continue;
+ device_foreach_child_safe(dev, next, bus) {
+ if (!device_active(dev))
+ device_unbind(dev);
+ }
+ }
+}
+
int usb_init(void)
{
int controllers_initialized = 0;
@@ -270,6 +284,15 @@ int usb_init(void)
}
debug("scan end\n");
+
+ /* Remove any devices that were not found on this scan */
+ remove_inactive_children(uc, bus);
+
+ ret = uclass_get(UCLASS_USB_HUB, &uc);
+ if (ret)
+ return ret;
+ remove_inactive_children(uc, bus);
+
/* if we were not able to find at least one working bus, bail out */
if (!count)
printf("No controllers found\n");
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply related [flat|nested] 67+ messages in thread* [U-Boot] [PATCH v2 18/26] dm: usb: Remove inactive children after a bus scan
2015-11-09 6:48 ` [U-Boot] [PATCH v2 18/26] dm: usb: Remove inactive children after a bus scan Simon Glass
@ 2015-11-09 8:22 ` Hans de Goede
2015-11-09 20:25 ` Simon Glass
0 siblings, 1 reply; 67+ messages in thread
From: Hans de Goede @ 2015-11-09 8:22 UTC (permalink / raw)
To: u-boot
Hi,
On 09-11-15 07:48, Simon Glass wrote:
> Each scan of the USB bus may return different results. Existing driver-model
> devices are reused when found, but if a device no longer exists it will stay
> around, de-activated, but bound.
>
> Detect these devices and remove them after the scan completes.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
I wonder how this is better then my original:
"dm: usb: Use device_unbind_children to clean up usb devs on stop"
Patch, the end result of both patches is the same and both are
a NOP when DM_DEVICE_REMOVE is not set. Where as my code seems
to be a much more KISS approach to the problem (my approach is
just 3 lines vs 23 lines for yours).
I know we will need usb_find_child in the DM_DEVICE_REMOVE not
set case, but why not only revert the:
"dm: usb: Rename usb_find_child to usb_find_emul_child"
commit, keep the other 2 you revert and drop this patch ?
This drops 3 patches from your patch-set and the end result is
more clean IMHO.
> Changes in v2: None
>
> drivers/usb/host/usb-uclass.c | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
> index 4aa92f8..50538e0 100644
> --- a/drivers/usb/host/usb-uclass.c
> +++ b/drivers/usb/host/usb-uclass.c
> @@ -202,6 +202,20 @@ static void usb_scan_bus(struct udevice *bus, bool recurse)
> printf("%d USB Device(s) found\n", priv->next_addr);
> }
>
> +static void remove_inactive_children(struct uclass *uc, struct udevice *bus)
> +{
> + uclass_foreach_dev(bus, uc) {
> + struct udevice *dev, *next;
> +
> + if (!device_active(bus))
> + continue;
> + device_foreach_child_safe(dev, next, bus) {
> + if (!device_active(dev))
> + device_unbind(dev);
> + }
> + }
> +}
> +
> int usb_init(void)
> {
> int controllers_initialized = 0;
> @@ -270,6 +284,15 @@ int usb_init(void)
> }
>
> debug("scan end\n");
> +
> + /* Remove any devices that were not found on this scan */
> + remove_inactive_children(uc, bus);
> +
> + ret = uclass_get(UCLASS_USB_HUB, &uc);
> + if (ret)
> + return ret;
> + remove_inactive_children(uc, bus);
> +
Why do you need to call remove_inactive_children twice here? This seems
worthy of a comment explaining why this is necessary.
> /* if we were not able to find at least one working bus, bail out */
> if (!count)
> printf("No controllers found\n");
>
Regards,
Hans
^ permalink raw reply [flat|nested] 67+ messages in thread* [U-Boot] [PATCH v2 18/26] dm: usb: Remove inactive children after a bus scan
2015-11-09 8:22 ` Hans de Goede
@ 2015-11-09 20:25 ` Simon Glass
2015-11-10 23:30 ` Simon Glass
0 siblings, 1 reply; 67+ messages in thread
From: Simon Glass @ 2015-11-09 20:25 UTC (permalink / raw)
To: u-boot
Hi Hans,
On 9 November 2015 at 00:22, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
> On 09-11-15 07:48, Simon Glass wrote:
>>
>> Each scan of the USB bus may return different results. Existing
>> driver-model
>> devices are reused when found, but if a device no longer exists it will
>> stay
>> around, de-activated, but bound.
>>
>> Detect these devices and remove them after the scan completes.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>
>
> I wonder how this is better then my original:
> "dm: usb: Use device_unbind_children to clean up usb devs on stop"
>
> Patch, the end result of both patches is the same and both are
> a NOP when DM_DEVICE_REMOVE is not set. Where as my code seems
> to be a much more KISS approach to the problem (my approach is
> just 3 lines vs 23 lines for yours).
>
> I know we will need usb_find_child in the DM_DEVICE_REMOVE not
> set case, but why not only revert the:
>
> "dm: usb: Rename usb_find_child to usb_find_emul_child"
>
> commit, keep the other 2 you revert and drop this patch ?
>
> This drops 3 patches from your patch-set and the end result is
> more clean IMHO.
I would like to avoid binding/unbinding things when nothing changes if
possible. Also I'd like to support attaching device tree
nodes/properties to USB devices as necessary, as we do with PCI, and
removing things breaks that.
I still have to figure out one more test case, so I'll do that before
commenting further.
>
>
>> Changes in v2: None
>>
>> drivers/usb/host/usb-uclass.c | 23 +++++++++++++++++++++++
>> 1 file changed, 23 insertions(+)
>>
>> diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
>> index 4aa92f8..50538e0 100644
>> --- a/drivers/usb/host/usb-uclass.c
>> +++ b/drivers/usb/host/usb-uclass.c
>> @@ -202,6 +202,20 @@ static void usb_scan_bus(struct udevice *bus, bool
>> recurse)
>> printf("%d USB Device(s) found\n", priv->next_addr);
>> }
>>
>> +static void remove_inactive_children(struct uclass *uc, struct udevice
>> *bus)
>> +{
>> + uclass_foreach_dev(bus, uc) {
>> + struct udevice *dev, *next;
>> +
>> + if (!device_active(bus))
>> + continue;
>> + device_foreach_child_safe(dev, next, bus) {
>> + if (!device_active(dev))
>> + device_unbind(dev);
>> + }
>> + }
>> +}
>> +
>> int usb_init(void)
>> {
>> int controllers_initialized = 0;
>> @@ -270,6 +284,15 @@ int usb_init(void)
>> }
>>
>> debug("scan end\n");
>> +
>> + /* Remove any devices that were not found on this scan */
>> + remove_inactive_children(uc, bus);
>> +
>> + ret = uclass_get(UCLASS_USB_HUB, &uc);
>> + if (ret)
>> + return ret;
>> + remove_inactive_children(uc, bus);
>> +
>
>
> Why do you need to call remove_inactive_children twice here? This seems
> worthy of a comment explaining why this is necessary.
One is removing the children of USB controllers, one is removing the
children of USB hubs. I'll add a comment.
>
>> /* if we were not able to find at least one working bus, bail out
>> */
>> if (!count)
>> printf("No controllers found\n");
Regards,
Simon
^ permalink raw reply [flat|nested] 67+ messages in thread* [U-Boot] [PATCH v2 18/26] dm: usb: Remove inactive children after a bus scan
2015-11-09 20:25 ` Simon Glass
@ 2015-11-10 23:30 ` Simon Glass
2015-11-11 17:02 ` Hans de Goede
2015-11-11 17:03 ` Hans de Goede
0 siblings, 2 replies; 67+ messages in thread
From: Simon Glass @ 2015-11-10 23:30 UTC (permalink / raw)
To: u-boot
Hi Hans,
On 9 November 2015 at 12:25, Simon Glass <sjg@chromium.org> wrote:
> Hi Hans,
>
> On 9 November 2015 at 00:22, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>> On 09-11-15 07:48, Simon Glass wrote:
>>>
>>> Each scan of the USB bus may return different results. Existing
>>> driver-model
>>> devices are reused when found, but if a device no longer exists it will
>>> stay
>>> around, de-activated, but bound.
>>>
>>> Detect these devices and remove them after the scan completes.
>>>
>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>
>>
>> I wonder how this is better then my original:
>> "dm: usb: Use device_unbind_children to clean up usb devs on stop"
>>
>> Patch, the end result of both patches is the same and both are
>> a NOP when DM_DEVICE_REMOVE is not set. Where as my code seems
>> to be a much more KISS approach to the problem (my approach is
>> just 3 lines vs 23 lines for yours).
>>
>> I know we will need usb_find_child in the DM_DEVICE_REMOVE not
>> set case, but why not only revert the:
>>
>> "dm: usb: Rename usb_find_child to usb_find_emul_child"
>>
>> commit, keep the other 2 you revert and drop this patch ?
>>
>> This drops 3 patches from your patch-set and the end result is
>> more clean IMHO.
>
> I would like to avoid binding/unbinding things when nothing changes if
> possible. Also I'd like to support attaching device tree
> nodes/properties to USB devices as necessary, as we do with PCI, and
> removing things breaks that.
>
> I still have to figure out one more test case, so I'll do that before
> commenting further.
I've added the test case and pushed a new tree. However it turns out
that this doesn't behave differently.
So please can you go ahead and run your original (manual) test case?
I'd like to make sure that my automated tests are correct and catch
the bug you reported and fixed.
>
>>
>>
>>> Changes in v2: None
>>>
>>> drivers/usb/host/usb-uclass.c | 23 +++++++++++++++++++++++
>>> 1 file changed, 23 insertions(+)
>>>
>>> diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
>>> index 4aa92f8..50538e0 100644
>>> --- a/drivers/usb/host/usb-uclass.c
>>> +++ b/drivers/usb/host/usb-uclass.c
>>> @@ -202,6 +202,20 @@ static void usb_scan_bus(struct udevice *bus, bool
>>> recurse)
>>> printf("%d USB Device(s) found\n", priv->next_addr);
>>> }
>>>
>>> +static void remove_inactive_children(struct uclass *uc, struct udevice
>>> *bus)
>>> +{
>>> + uclass_foreach_dev(bus, uc) {
>>> + struct udevice *dev, *next;
>>> +
>>> + if (!device_active(bus))
>>> + continue;
>>> + device_foreach_child_safe(dev, next, bus) {
>>> + if (!device_active(dev))
>>> + device_unbind(dev);
>>> + }
>>> + }
>>> +}
>>> +
>>> int usb_init(void)
>>> {
>>> int controllers_initialized = 0;
>>> @@ -270,6 +284,15 @@ int usb_init(void)
>>> }
>>>
>>> debug("scan end\n");
>>> +
>>> + /* Remove any devices that were not found on this scan */
>>> + remove_inactive_children(uc, bus);
>>> +
>>> + ret = uclass_get(UCLASS_USB_HUB, &uc);
>>> + if (ret)
>>> + return ret;
>>> + remove_inactive_children(uc, bus);
>>> +
>>
>>
>> Why do you need to call remove_inactive_children twice here? This seems
>> worthy of a comment explaining why this is necessary.
>
> One is removing the children of USB controllers, one is removing the
> children of USB hubs. I'll add a comment.
>
>>
>>> /* if we were not able to find at least one working bus, bail out
>>> */
>>> if (!count)
>>> printf("No controllers found\n");
>
> Regards,
> Simon
^ permalink raw reply [flat|nested] 67+ messages in thread* [U-Boot] [PATCH v2 18/26] dm: usb: Remove inactive children after a bus scan
2015-11-10 23:30 ` Simon Glass
@ 2015-11-11 17:02 ` Hans de Goede
2015-11-11 18:15 ` Simon Glass
2015-11-11 17:03 ` Hans de Goede
1 sibling, 1 reply; 67+ messages in thread
From: Hans de Goede @ 2015-11-11 17:02 UTC (permalink / raw)
To: u-boot
Hi,
On 11-11-15 00:30, Simon Glass wrote:
> Hi Hans,
>
> On 9 November 2015 at 12:25, Simon Glass <sjg@chromium.org> wrote:
>> Hi Hans,
>>
>> On 9 November 2015 at 00:22, Hans de Goede <hdegoede@redhat.com> wrote:
>>> Hi,
>>>
>>> On 09-11-15 07:48, Simon Glass wrote:
>>>>
>>>> Each scan of the USB bus may return different results. Existing
>>>> driver-model
>>>> devices are reused when found, but if a device no longer exists it will
>>>> stay
>>>> around, de-activated, but bound.
>>>>
>>>> Detect these devices and remove them after the scan completes.
>>>>
>>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>>
>>>
>>> I wonder how this is better then my original:
>>> "dm: usb: Use device_unbind_children to clean up usb devs on stop"
>>>
>>> Patch, the end result of both patches is the same and both are
>>> a NOP when DM_DEVICE_REMOVE is not set. Where as my code seems
>>> to be a much more KISS approach to the problem (my approach is
>>> just 3 lines vs 23 lines for yours).
>>>
>>> I know we will need usb_find_child in the DM_DEVICE_REMOVE not
>>> set case, but why not only revert the:
>>>
>>> "dm: usb: Rename usb_find_child to usb_find_emul_child"
>>>
>>> commit, keep the other 2 you revert and drop this patch ?
>>>
>>> This drops 3 patches from your patch-set and the end result is
>>> more clean IMHO.
>>
>> I would like to avoid binding/unbinding things when nothing changes if
>> possible. Also I'd like to support attaching device tree
>> nodes/properties to USB devices as necessary, as we do with PCI, and
>> removing things breaks that.
>>
>> I still have to figure out one more test case, so I'll do that before
>> commenting further.
>
> I've added the test case and pushed a new tree. However it turns out
> that this doesn't behave differently.
>
> So please can you go ahead and run your original (manual) test case?
> I'd like to make sure that my automated tests are correct and catch
> the bug you reported and fixed.
Ok, I've ran a whole battery of tests on your u-boot-dm/usb-working branch.
There are 2 issues:
1) You need to add these change to the commit introducing usb-keyb dm
support (or one of the related commits), otherwise usb-keyb support
breaks:
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -513,6 +513,7 @@ config ARCH_SUNXI
select DM
select DM_GPIO
select DM_ETH
+ select DM_KEYBOARD
select DM_SERIAL
select DM_USB
select OF_CONTROL
The breakage is that without this usb_scan_device() returns -96
(EPFNOSUPPORT) for usb keyboards.
2) With that branch there still is the purely cosmetical issue,
that if one plugs a usb-stick + keyb into the same hub, then swaps
their place and do "usb reset" and then "usb tree" looks like this:
USB device tree:
1 Hub (480 Mb/s, 100mA)
| USB2.0 Hub
|
+-3 Human Interface (1.5 Mb/s, 100mA)
| SINO WEALTH USB Composite Device
|
+-2 Mass Storage (480 Mb/s, 100mA)
USB Flash Disk 4C0E960F
Notice the order of devices listed: 1 - 3 - 2, which is a bit weird,
as said this is purely cosmetical.
Note you can easily fix this by only reverting the:
"dm: usb: Rename usb_find_child to usb_find_emul_child"
commit, and keep the other 2 you revert and dropping this patch ?
As I already suggested before, this is both more KISS and as you
can see it solves some actually (admittedly very minor) issues.
I'm not really buying your arguments for your more complex solution,
as shown above re-using the devices actually causes issues.
Your other argument of wanting to attach device-tree properties
I also do not find a strong argument, for one there has never been
a need to do so sofar, and if we ever need this we need a way
to specify which usb-device the properties belong to based on
the topology under the host / root-hub anyway, and match things
up when first scanning the bus. And if we can match things up
on the first scan we can also match them up on subsequent scans
and attach the same of-node again.
Anyways I'm fine with doing things your way, but I still have
a preference for the more KISS and IMHO robust solution of
simple unbinding all devices on usb-stop.
###
Talking about usb-stop, there is still one (BIG!) problem after
this patch set when building usb-support with DM_DEVICE_REMOVE
not set. This means that the controllers dma engines will not
be stopped when booting the actual OS and they will still be
accessing parts of the main memory while the actual OS is
booting, which is BAD. So I suggest adding something like
this to all host drivers which use dma in this way:
#if defined CONFIG_DM_USB && !defined CONFIG_DM_DEVICE_REMOVE
#error The EHCI driver cannot be used without CONFIG_DM_DEVICE_REMOVE otherwise DMA stays active while booting the OS
#endif
Regards,
Hans
>
>>
>>>
>>>
>>>> Changes in v2: None
>>>>
>>>> drivers/usb/host/usb-uclass.c | 23 +++++++++++++++++++++++
>>>> 1 file changed, 23 insertions(+)
>>>>
>>>> diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
>>>> index 4aa92f8..50538e0 100644
>>>> --- a/drivers/usb/host/usb-uclass.c
>>>> +++ b/drivers/usb/host/usb-uclass.c
>>>> @@ -202,6 +202,20 @@ static void usb_scan_bus(struct udevice *bus, bool
>>>> recurse)
>>>> printf("%d USB Device(s) found\n", priv->next_addr);
>>>> }
>>>>
>>>> +static void remove_inactive_children(struct uclass *uc, struct udevice
>>>> *bus)
>>>> +{
>>>> + uclass_foreach_dev(bus, uc) {
>>>> + struct udevice *dev, *next;
>>>> +
>>>> + if (!device_active(bus))
>>>> + continue;
>>>> + device_foreach_child_safe(dev, next, bus) {
>>>> + if (!device_active(dev))
>>>> + device_unbind(dev);
>>>> + }
>>>> + }
>>>> +}
>>>> +
>>>> int usb_init(void)
>>>> {
>>>> int controllers_initialized = 0;
>>>> @@ -270,6 +284,15 @@ int usb_init(void)
>>>> }
>>>>
>>>> debug("scan end\n");
>>>> +
>>>> + /* Remove any devices that were not found on this scan */
>>>> + remove_inactive_children(uc, bus);
>>>> +
>>>> + ret = uclass_get(UCLASS_USB_HUB, &uc);
>>>> + if (ret)
>>>> + return ret;
>>>> + remove_inactive_children(uc, bus);
>>>> +
>>>
>>>
>>> Why do you need to call remove_inactive_children twice here? This seems
>>> worthy of a comment explaining why this is necessary.
>>
>> One is removing the children of USB controllers, one is removing the
>> children of USB hubs. I'll add a comment.
>>
>>>
>>>> /* if we were not able to find at least one working bus, bail out
>>>> */
>>>> if (!count)
>>>> printf("No controllers found\n");
>>
>> Regards,
>> Simon
^ permalink raw reply [flat|nested] 67+ messages in thread* [U-Boot] [PATCH v2 18/26] dm: usb: Remove inactive children after a bus scan
2015-11-11 17:02 ` Hans de Goede
@ 2015-11-11 18:15 ` Simon Glass
2015-11-12 14:08 ` Hans de Goede
0 siblings, 1 reply; 67+ messages in thread
From: Simon Glass @ 2015-11-11 18:15 UTC (permalink / raw)
To: u-boot
Hi Hans,
On 11 November 2015 at 10:02, Hans de Goede <j.w.r.degoede@gmail.com> wrote:
>
> Hi,
>
>
> On 11-11-15 00:30, Simon Glass wrote:
>>
>> Hi Hans,
>>
>> On 9 November 2015 at 12:25, Simon Glass <sjg@chromium.org> wrote:
>>>
>>> Hi Hans,
>>>
>>> On 9 November 2015 at 00:22, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>> On 09-11-15 07:48, Simon Glass wrote:
>>>>>
>>>>>
>>>>> Each scan of the USB bus may return different results. Existing
>>>>> driver-model
>>>>> devices are reused when found, but if a device no longer exists it will
>>>>> stay
>>>>> around, de-activated, but bound.
>>>>>
>>>>> Detect these devices and remove them after the scan completes.
>>>>>
>>>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>>>
>>>>
>>>>
>>>> I wonder how this is better then my original:
>>>> "dm: usb: Use device_unbind_children to clean up usb devs on stop"
>>>>
>>>> Patch, the end result of both patches is the same and both are
>>>> a NOP when DM_DEVICE_REMOVE is not set. Where as my code seems
>>>> to be a much more KISS approach to the problem (my approach is
>>>> just 3 lines vs 23 lines for yours).
>>>>
>>>> I know we will need usb_find_child in the DM_DEVICE_REMOVE not
>>>> set case, but why not only revert the:
>>>>
>>>> "dm: usb: Rename usb_find_child to usb_find_emul_child"
>>>>
>>>> commit, keep the other 2 you revert and drop this patch ?
>>>>
>>>> This drops 3 patches from your patch-set and the end result is
>>>> more clean IMHO.
>>>
>>>
>>> I would like to avoid binding/unbinding things when nothing changes if
>>> possible. Also I'd like to support attaching device tree
>>> nodes/properties to USB devices as necessary, as we do with PCI, and
>>> removing things breaks that.
>>>
>>> I still have to figure out one more test case, so I'll do that before
>>> commenting further.
>>
>>
>> I've added the test case and pushed a new tree. However it turns out
>> that this doesn't behave differently.
>>
>> So please can you go ahead and run your original (manual) test case?
>> I'd like to make sure that my automated tests are correct and catch
>> the bug you reported and fixed.
>
>
> Ok, I've ran a whole battery of tests on your u-boot-dm/usb-working branch.
Thanks!
>
> There are 2 issues:
>
> 1) You need to add these change to the commit introducing usb-keyb dm
> support (or one of the related commits), otherwise usb-keyb support
> breaks:
>
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -513,6 +513,7 @@ config ARCH_SUNXI
> select DM
> select DM_GPIO
> select DM_ETH
> + select DM_KEYBOARD
> select DM_SERIAL
> select DM_USB
> select OF_CONTROL
>
> The breakage is that without this usb_scan_device() returns -96
> (EPFNOSUPPORT) for usb keyboards.
OK. I am a bit concerned this might affect other boards too. Still, if
we get this series applied soon there is plenty of time for testing.
I'll look a bit closer.
>
> 2) With that branch there still is the purely cosmetical issue,
> that if one plugs a usb-stick + keyb into the same hub, then swaps
> their place and do "usb reset" and then "usb tree" looks like this:
>
> USB device tree:
> 1 Hub (480 Mb/s, 100mA)
> | USB2.0 Hub
> |
> +-3 Human Interface (1.5 Mb/s, 100mA)
> | SINO WEALTH USB Composite Device
> |
> +-2 Mass Storage (480 Mb/s, 100mA)
> USB Flash Disk 4C0E960F
>
> Notice the order of devices listed: 1 - 3 - 2, which is a bit weird,
> as said this is purely cosmetical.
>
> Note you can easily fix this by only reverting the:
>
> "dm: usb: Rename usb_find_child to usb_find_emul_child"
>
> commit, and keep the other 2 you revert and dropping this patch ?
>
> As I already suggested before, this is both more KISS and as you
> can see it solves some actually (admittedly very minor) issues.
>
> I'm not really buying your arguments for your more complex solution,
> as shown above re-using the devices actually causes issues.
>
> Your other argument of wanting to attach device-tree properties
> I also do not find a strong argument, for one there has never been
> a need to do so sofar, and if we ever need this we need a way
> to specify which usb-device the properties belong to based on
> the topology under the host / root-hub anyway, and match things
> up when first scanning the bus. And if we can match things up
> on the first scan we can also match them up on subsequent scans
> and attach the same of-node again.
>
> Anyways I'm fine with doing things your way, but I still have
> a preference for the more KISS and IMHO robust solution of
> simple unbinding all devices on usb-stop.
I wonder if it really is more complex. My preferred algorithm is to
remove any devices that have not been probed after a scan. Yours is to
remove and unbind any USB devices before a scan. What is the
difference?
I much prefer not unbinding and rebinding devices. To my mind, devices
should be bound once if possible, and most of the time it is possible.
This is different from the Linux approach of combining 'bind' and
'probe'. At present sandbox cannot support 'usb reset'. I would like
sandbox to provide full support so that we can use tests to verify
functionality rather than manual testing.
Re the ordering above, I suppose I could fix that command easily
enough. This is not a common problem though. Does it really matter?
>
> ###
>
> Talking about usb-stop, there is still one (BIG!) problem after
> this patch set when building usb-support with DM_DEVICE_REMOVE
> not set. This means that the controllers dma engines will not
> be stopped when booting the actual OS and they will still be
> accessing parts of the main memory while the actual OS is
> booting, which is BAD. So I suggest adding something like
> this to all host drivers which use dma in this way:
>
> #if defined CONFIG_DM_USB && !defined CONFIG_DM_DEVICE_REMOVE
> #error The EHCI driver cannot be used without CONFIG_DM_DEVICE_REMOVE otherwise DMA stays active while booting the OS
> #endif
Sounds good. I am not suggesting that we encourage people to build USB
with out DM_DEVICE_REMOVE. I just want to make sure that we don't
create unnecessary dependencies such that DM_DEVICE_REMOVE becomes
impossible to use.
We have a note in the Kconfig for that, but I agree we need to make it
more explicit. With the #if approach it makes the pitfall much more
obvious.
>
> Regards,
>
> Hans
>
>
>
>
>
>>
>>>
>>>>
>>>>
>>>>> Changes in v2: None
>>>>>
>>>>> drivers/usb/host/usb-uclass.c | 23 +++++++++++++++++++++++
>>>>> 1 file changed, 23 insertions(+)
>>>>>
>>>>> diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
>>>>> index 4aa92f8..50538e0 100644
>>>>> --- a/drivers/usb/host/usb-uclass.c
>>>>> +++ b/drivers/usb/host/usb-uclass.c
>>>>> @@ -202,6 +202,20 @@ static void usb_scan_bus(struct udevice *bus, bool
>>>>> recurse)
>>>>> printf("%d USB Device(s) found\n", priv->next_addr);
>>>>> }
>>>>>
>>>>> +static void remove_inactive_children(struct uclass *uc, struct udevice
>>>>> *bus)
>>>>> +{
>>>>> + uclass_foreach_dev(bus, uc) {
>>>>> + struct udevice *dev, *next;
>>>>> +
>>>>> + if (!device_active(bus))
>>>>> + continue;
>>>>> + device_foreach_child_safe(dev, next, bus) {
>>>>> + if (!device_active(dev))
>>>>> + device_unbind(dev);
>>>>> + }
>>>>> + }
>>>>> +}
>>>>> +
>>>>> int usb_init(void)
>>>>> {
>>>>> int controllers_initialized = 0;
>>>>> @@ -270,6 +284,15 @@ int usb_init(void)
>>>>> }
>>>>>
>>>>> debug("scan end\n");
>>>>> +
>>>>> + /* Remove any devices that were not found on this scan */
>>>>> + remove_inactive_children(uc, bus);
>>>>> +
>>>>> + ret = uclass_get(UCLASS_USB_HUB, &uc);
>>>>> + if (ret)
>>>>> + return ret;
>>>>> + remove_inactive_children(uc, bus);
>>>>> +
>>>>
>>>>
>>>>
>>>> Why do you need to call remove_inactive_children twice here? This seems
>>>> worthy of a comment explaining why this is necessary.
>>>
>>>
>>> One is removing the children of USB controllers, one is removing the
>>> children of USB hubs. I'll add a comment.
>>>
>>>>
>>>>> /* if we were not able to find at least one working bus, bail out
>>>>> */
>>>>> if (!count)
>>>>> printf("No controllers found\n");
>>>
>>>
>>> Regards,
>>> Simon
Regards,
Simon
^ permalink raw reply [flat|nested] 67+ messages in thread* [U-Boot] [PATCH v2 18/26] dm: usb: Remove inactive children after a bus scan
2015-11-11 18:15 ` Simon Glass
@ 2015-11-12 14:08 ` Hans de Goede
2015-11-13 21:58 ` Simon Glass
0 siblings, 1 reply; 67+ messages in thread
From: Hans de Goede @ 2015-11-12 14:08 UTC (permalink / raw)
To: u-boot
Hi,
On 11-11-15 19:15, Simon Glass wrote:
> Hi Hans,
>
> On 11 November 2015 at 10:02, Hans de Goede <j.w.r.degoede@gmail.com> wrote:
<snip>
>> Ok, I've ran a whole battery of tests on your u-boot-dm/usb-working branch.
>
> Thanks!
>
>>
>> There are 2 issues:
>>
>> 1) You need to add these change to the commit introducing usb-keyb dm
>> support (or one of the related commits), otherwise usb-keyb support
>> breaks:
>>
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -513,6 +513,7 @@ config ARCH_SUNXI
>> select DM
>> select DM_GPIO
>> select DM_ETH
>> + select DM_KEYBOARD
>> select DM_SERIAL
>> select DM_USB
>> select OF_CONTROL
>>
>> The breakage is that without this usb_scan_device() returns -96
>> (EPFNOSUPPORT) for usb keyboards.
>
> OK. I am a bit concerned this might affect other boards too. Still, if
> we get this series applied soon there is plenty of time for testing.
> I'll look a bit closer.
Ok.
>> 2) With that branch there still is the purely cosmetical issue,
>> that if one plugs a usb-stick + keyb into the same hub, then swaps
>> their place and do "usb reset" and then "usb tree" looks like this:
>>
>> USB device tree:
>> 1 Hub (480 Mb/s, 100mA)
>> | USB2.0 Hub
>> |
>> +-3 Human Interface (1.5 Mb/s, 100mA)
>> | SINO WEALTH USB Composite Device
>> |
>> +-2 Mass Storage (480 Mb/s, 100mA)
>> USB Flash Disk 4C0E960F
>>
>> Notice the order of devices listed: 1 - 3 - 2, which is a bit weird,
>> as said this is purely cosmetical.
>>
>> Note you can easily fix this by only reverting the:
>>
>> "dm: usb: Rename usb_find_child to usb_find_emul_child"
>>
>> commit, and keep the other 2 you revert and dropping this patch ?
>>
>> As I already suggested before, this is both more KISS and as you
>> can see it solves some actually (admittedly very minor) issues.
>>
>> I'm not really buying your arguments for your more complex solution,
>> as shown above re-using the devices actually causes issues.
>>
>> Your other argument of wanting to attach device-tree properties
>> I also do not find a strong argument, for one there has never been
>> a need to do so sofar, and if we ever need this we need a way
>> to specify which usb-device the properties belong to based on
>> the topology under the host / root-hub anyway, and match things
>> up when first scanning the bus. And if we can match things up
>> on the first scan we can also match them up on subsequent scans
>> and attach the same of-node again.
>>
>> Anyways I'm fine with doing things your way, but I still have
>> a preference for the more KISS and IMHO robust solution of
>> simple unbinding all devices on usb-stop.
>
> I wonder if it really is more complex. My preferred algorithm is to
> remove any devices that have not been probed after a scan. Yours is to
> remove and unbind any USB devices before a scan. What is the
> difference?
If you unbind everything before (re)scanning you always start with
a clean slate, making things more reproducible and simpler.
As for it being more complex, your approach needs a whole new
helper function and needs to walk the list of devices twice, where
as mine is just a single extra function call + error checking.
As an added bonus my approach allows adding an ifdef to usb_find_child
turning it into a nop like this
#if defined CONFIG_SANDBOX || !defined CONFIG_DM_DEVICE_REMOVE
...
... complex code to find child
...
#else
return -ENODEV;
#endif
So besides not needing the extra helper function your solution
needs, my version also removes a bunch of extra code (from the
resulting binary) in almost all cases.
So your solution is definitely more complex, needlessly so IMHO.
> I much prefer not unbinding and rebinding devices. To my mind, devices
> should be bound once if possible, and most of the time it is possible.
Well in case of a re-scan we are discovering all devices from scratch,
not just looking for changed devices, simply starting with a clean
device list reflects this.
> This is different from the Linux approach of combining 'bind' and
> 'probe'. At present sandbox cannot support 'usb reset'. I would like
> sandbox to provide full support so that we can use tests to verify
> functionality rather than manual testing.
>
> Re the ordering above, I suppose I could fix that command easily
> enough. This is not a common problem though. Does it really matter?
Nope, as said this is a cosmetic issue, and for the record
I'm fine with your approach of always re-using existing devices
on a re-scan. I prefer the start with a clean slate approach but
either is fine with me.
>> ###
>>
>> Talking about usb-stop, there is still one (BIG!) problem after
>> this patch set when building usb-support with DM_DEVICE_REMOVE
>> not set. This means that the controllers dma engines will not
>> be stopped when booting the actual OS and they will still be
>> accessing parts of the main memory while the actual OS is
>> booting, which is BAD. So I suggest adding something like
>> this to all host drivers which use dma in this way:
>>
>> #if defined CONFIG_DM_USB && !defined CONFIG_DM_DEVICE_REMOVE
>> #error The EHCI driver cannot be used without CONFIG_DM_DEVICE_REMOVE otherwise DMA stays active while booting the OS
>> #endif
>
> Sounds good. I am not suggesting that we encourage people to build USB
> with out DM_DEVICE_REMOVE. I just want to make sure that we don't
> create unnecessary dependencies such that DM_DEVICE_REMOVE becomes
> impossible to use.
>
> We have a note in the Kconfig for that, but I agree we need to make it
> more explicit. With the #if approach it makes the pitfall much more
> obvious.
Ack, so we need to have patches for this for at least ohci, ehci, xhci
and I guess also uhci ? Who is going to write these patches ?
Regards,
Hans
^ permalink raw reply [flat|nested] 67+ messages in thread* [U-Boot] [PATCH v2 18/26] dm: usb: Remove inactive children after a bus scan
2015-11-12 14:08 ` Hans de Goede
@ 2015-11-13 21:58 ` Simon Glass
2015-11-15 19:35 ` Hans de Goede
0 siblings, 1 reply; 67+ messages in thread
From: Simon Glass @ 2015-11-13 21:58 UTC (permalink / raw)
To: u-boot
Hi Hans,
On 12 November 2015 at 07:08, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
> On 11-11-15 19:15, Simon Glass wrote:
>>
>> Hi Hans,
>>
>> On 11 November 2015 at 10:02, Hans de Goede <j.w.r.degoede@gmail.com>
>> wrote:
>
>
> <snip>
>
>>> Ok, I've ran a whole battery of tests on your u-boot-dm/usb-working
>>> branch.
>>
>>
>> Thanks!
>>
>>>
>>> There are 2 issues:
>>>
>>> 1) You need to add these change to the commit introducing usb-keyb dm
>>> support (or one of the related commits), otherwise usb-keyb support
>>> breaks:
>>>
>>> --- a/arch/arm/Kconfig
>>> +++ b/arch/arm/Kconfig
>>> @@ -513,6 +513,7 @@ config ARCH_SUNXI
>>> select DM
>>> select DM_GPIO
>>> select DM_ETH
>>> + select DM_KEYBOARD
>>> select DM_SERIAL
>>> select DM_USB
>>> select OF_CONTROL
>>>
>>> The breakage is that without this usb_scan_device() returns -96
>>> (EPFNOSUPPORT) for usb keyboards.
>>
>>
>> OK. I am a bit concerned this might affect other boards too. Still, if
>> we get this series applied soon there is plenty of time for testing.
>> I'll look a bit closer.
>
>
> Ok.
>
>
>>> 2) With that branch there still is the purely cosmetical issue,
>>> that if one plugs a usb-stick + keyb into the same hub, then swaps
>>> their place and do "usb reset" and then "usb tree" looks like this:
>>>
>>> USB device tree:
>>> 1 Hub (480 Mb/s, 100mA)
>>> | USB2.0 Hub
>>> |
>>> +-3 Human Interface (1.5 Mb/s, 100mA)
>>> | SINO WEALTH USB Composite Device
>>> |
>>> +-2 Mass Storage (480 Mb/s, 100mA)
>>> USB Flash Disk 4C0E960F
>>>
>>> Notice the order of devices listed: 1 - 3 - 2, which is a bit weird,
>>> as said this is purely cosmetical.
>>>
>>> Note you can easily fix this by only reverting the:
>>>
>>> "dm: usb: Rename usb_find_child to usb_find_emul_child"
>>>
>>> commit, and keep the other 2 you revert and dropping this patch ?
>>>
>>> As I already suggested before, this is both more KISS and as you
>>> can see it solves some actually (admittedly very minor) issues.
>>>
>>> I'm not really buying your arguments for your more complex solution,
>>> as shown above re-using the devices actually causes issues.
>>>
>>> Your other argument of wanting to attach device-tree properties
>>> I also do not find a strong argument, for one there has never been
>>> a need to do so sofar, and if we ever need this we need a way
>>> to specify which usb-device the properties belong to based on
>>> the topology under the host / root-hub anyway, and match things
>>> up when first scanning the bus. And if we can match things up
>>> on the first scan we can also match them up on subsequent scans
>>> and attach the same of-node again.
>>>
>>> Anyways I'm fine with doing things your way, but I still have
>>> a preference for the more KISS and IMHO robust solution of
>>> simple unbinding all devices on usb-stop.
>>
>>
>> I wonder if it really is more complex. My preferred algorithm is to
>> remove any devices that have not been probed after a scan. Yours is to
>> remove and unbind any USB devices before a scan. What is the
>> difference?
>
>
> If you unbind everything before (re)scanning you always start with
> a clean slate, making things more reproducible and simpler.
>
> As for it being more complex, your approach needs a whole new
> helper function and needs to walk the list of devices twice, where
> as mine is just a single extra function call + error checking.
The helper function is basically the same as the one you have - it
removes devices that are not activated, rather than all devices. I
don't think the difference is large.
It doesn't actually walk the list of devices twice. It walks the list
of USB controllers once, then the list of USB hubs once. We need to
check both, as hubs may be need to be unbound.
>
> As an added bonus my approach allows adding an ifdef to usb_find_child
> turning it into a nop like this
>
> #if defined CONFIG_SANDBOX || !defined CONFIG_DM_DEVICE_REMOVE
> ...
> ... complex code to find child
> ...
> #else
> return -ENODEV;
> #endif
I'm not sure that is a bonus. It means that sandbox is no-longer using
the same code path as other boards. That's actually one of the
objectives of sandbox.
>
> So besides not needing the extra helper function your solution
> needs, my version also removes a bunch of extra code (from the
> resulting binary) in almost all cases.
>
> So your solution is definitely more complex, needlessly so IMHO.
It does have the virtue of allowing sandbox USB to work properly. At
present 'usb reset' is broken, as I mentioned. Willy-nilly unbinding
of devices is a bad idea IMO.
Yes I understand that the code to find the child is needed, but it's not large:
struct udevice *dev;
*devp = NULL;
for (device_find_first_child(parent, &dev);
dev;
device_find_next_child(&dev)) {
struct usb_dev_platdata *plat = dev_get_parent_platdata(dev);
/* If this device is already in use, skip it */
if (device_active(dev))
continue;
debug(" %s: name='%s', plat=%d, desc=%d\n", __func__,
dev->name, plat->id.bDeviceClass, desc->bDeviceClass);
if (usb_match_one_id(desc, iface, &plat->id)) {
*devp = dev;
return 0;
}
}
and usb_match_one_id() is needed anyway to support drivers. I suppose
we could avoid this and always create a new device, which would
degenerate to your solution of removing all the old devices. So if we
want to provide that option, it would be easy to do.
>
>> I much prefer not unbinding and rebinding devices. To my mind, devices
>> should be bound once if possible, and most of the time it is possible.
>
>
> Well in case of a re-scan we are discovering all devices from scratch,
> not just looking for changed devices, simply starting with a clean
> device list reflects this.
Yes, although in most cases the list of devices will not change. We
are really talking about an unusual case. Most of the time when U-Boot
starts it runs 'usb start' once. The case where it runs multiple times
is IMO not an important case to optimise, unless we are trying to use
knowledge of previous devices to speed up detection of the new bus
state.
>
>> This is different from the Linux approach of combining 'bind' and
>> 'probe'. At present sandbox cannot support 'usb reset'. I would like
>> sandbox to provide full support so that we can use tests to verify
>> functionality rather than manual testing.
>>
>> Re the ordering above, I suppose I could fix that command easily
>> enough. This is not a common problem though. Does it really matter?
>
>
> Nope, as said this is a cosmetic issue, and for the record
> I'm fine with your approach of always re-using existing devices
> on a re-scan. I prefer the start with a clean slate approach but
> either is fine with me.
OK
>
>>> ###
>>>
>>> Talking about usb-stop, there is still one (BIG!) problem after
>>> this patch set when building usb-support with DM_DEVICE_REMOVE
>>> not set. This means that the controllers dma engines will not
>>> be stopped when booting the actual OS and they will still be
>>> accessing parts of the main memory while the actual OS is
>>> booting, which is BAD. So I suggest adding something like
>>> this to all host drivers which use dma in this way:
>>>
>>> #if defined CONFIG_DM_USB && !defined CONFIG_DM_DEVICE_REMOVE
>>> #error The EHCI driver cannot be used without CONFIG_DM_DEVICE_REMOVE
>>> otherwise DMA stays active while booting the OS
>>> #endif
>>
>>
>> Sounds good. I am not suggesting that we encourage people to build USB
>> with out DM_DEVICE_REMOVE. I just want to make sure that we don't
>> create unnecessary dependencies such that DM_DEVICE_REMOVE becomes
>> impossible to use.
>>
>> We have a note in the Kconfig for that, but I agree we need to make it
>> more explicit. With the #if approach it makes the pitfall much more
>> obvious.
>
>
> Ack, so we need to have patches for this for at least ohci, ehci, xhci
> and I guess also uhci ? Who is going to write these patches ?
I suppose I could do that. I would like to make sure that any USB fish
hooks are removed, and that is part of the solution.
I'd really like to resolve this and move things forward. If you still
have have strong objections please let me know. We did discuss this at
the time and I was clear that I did not want to keep the solution as
it was - I merged it because it fixed a bug, as you pointed out at the
time.
Regards,
Simon
^ permalink raw reply [flat|nested] 67+ messages in thread* [U-Boot] [PATCH v2 18/26] dm: usb: Remove inactive children after a bus scan
2015-11-13 21:58 ` Simon Glass
@ 2015-11-15 19:35 ` Hans de Goede
2015-11-16 21:08 ` Simon Glass
0 siblings, 1 reply; 67+ messages in thread
From: Hans de Goede @ 2015-11-15 19:35 UTC (permalink / raw)
To: u-boot
Hi,
On 11/13/2015 10:58 PM, Simon Glass wrote:
> Hi Hans,
<snip>
>>>> Talking about usb-stop, there is still one (BIG!) problem after
>>>> this patch set when building usb-support with DM_DEVICE_REMOVE
>>>> not set. This means that the controllers dma engines will not
>>>> be stopped when booting the actual OS and they will still be
>>>> accessing parts of the main memory while the actual OS is
>>>> booting, which is BAD. So I suggest adding something like
>>>> this to all host drivers which use dma in this way:
>>>>
>>>> #if defined CONFIG_DM_USB && !defined CONFIG_DM_DEVICE_REMOVE
>>>> #error The EHCI driver cannot be used without CONFIG_DM_DEVICE_REMOVE
>>>> otherwise DMA stays active while booting the OS
>>>> #endif
>>>
>>>
>>> Sounds good. I am not suggesting that we encourage people to build USB
>>> with out DM_DEVICE_REMOVE. I just want to make sure that we don't
>>> create unnecessary dependencies such that DM_DEVICE_REMOVE becomes
>>> impossible to use.
>>>
>>> We have a note in the Kconfig for that, but I agree we need to make it
>>> more explicit. With the #if approach it makes the pitfall much more
>>> obvious.
>>
>>
>> Ack, so we need to have patches for this for at least ohci, ehci, xhci
>> and I guess also uhci ? Who is going to write these patches ?
>
> I suppose I could do that. I would like to make sure that any USB fish
> hooks are removed, and that is part of the solution.
>
> I'd really like to resolve this and move things forward. If you still
> have have strong objections please let me know.
There are no objections from my side to move forward with your current
solution.
> We did discuss this at
> the time and I was clear that I did not want to keep the solution as
> it was - I merged it because it fixed a bug, as you pointed out at the
> time.
Ack.
Regards,
Hans
^ permalink raw reply [flat|nested] 67+ messages in thread
* [U-Boot] [PATCH v2 18/26] dm: usb: Remove inactive children after a bus scan
2015-11-15 19:35 ` Hans de Goede
@ 2015-11-16 21:08 ` Simon Glass
2015-11-20 3:32 ` Simon Glass
0 siblings, 1 reply; 67+ messages in thread
From: Simon Glass @ 2015-11-16 21:08 UTC (permalink / raw)
To: u-boot
Hi Hans,
On 15 November 2015 at 12:35, Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 11/13/2015 10:58 PM, Simon Glass wrote:
>>
>> Hi Hans,
>
>
> <snip>
>
>>>>> Talking about usb-stop, there is still one (BIG!) problem after
>>>>> this patch set when building usb-support with DM_DEVICE_REMOVE
>>>>> not set. This means that the controllers dma engines will not
>>>>> be stopped when booting the actual OS and they will still be
>>>>> accessing parts of the main memory while the actual OS is
>>>>> booting, which is BAD. So I suggest adding something like
>>>>> this to all host drivers which use dma in this way:
>>>>>
>>>>> #if defined CONFIG_DM_USB && !defined CONFIG_DM_DEVICE_REMOVE
>>>>> #error The EHCI driver cannot be used without CONFIG_DM_DEVICE_REMOVE
>>>>> otherwise DMA stays active while booting the OS
>>>>> #endif
>>>>
>>>>
>>>>
>>>> Sounds good. I am not suggesting that we encourage people to build USB
>>>> with out DM_DEVICE_REMOVE. I just want to make sure that we don't
>>>> create unnecessary dependencies such that DM_DEVICE_REMOVE becomes
>>>> impossible to use.
>>>>
>>>> We have a note in the Kconfig for that, but I agree we need to make it
>>>> more explicit. With the #if approach it makes the pitfall much more
>>>> obvious.
>>>
>>>
>>>
>>> Ack, so we need to have patches for this for at least ohci, ehci, xhci
>>> and I guess also uhci ? Who is going to write these patches ?
>>
>>
>> I suppose I could do that. I would like to make sure that any USB fish
>> hooks are removed, and that is part of the solution.
>>
>> I'd really like to resolve this and move things forward. If you still
>> have have strong objections please let me know.
>
>
> There are no objections from my side to move forward with your current
> solution.
>
>> We did discuss this at
>> the time and I was clear that I did not want to keep the solution as
>> it was - I merged it because it fixed a bug, as you pointed out at the
>> time.
>
>
> Ack.
OK thanks. I'll see if I can get some more comments on the other patches.
Regards,
Simon
^ permalink raw reply [flat|nested] 67+ messages in thread
* [U-Boot] [PATCH v2 18/26] dm: usb: Remove inactive children after a bus scan
2015-11-10 23:30 ` Simon Glass
2015-11-11 17:02 ` Hans de Goede
@ 2015-11-11 17:03 ` Hans de Goede
1 sibling, 0 replies; 67+ messages in thread
From: Hans de Goede @ 2015-11-11 17:03 UTC (permalink / raw)
To: u-boot
Hi,
On 11-11-15 00:30, Simon Glass wrote:
> Hi Hans,
>
> On 9 November 2015 at 12:25, Simon Glass <sjg@chromium.org> wrote:
>> Hi Hans,
>>
>> On 9 November 2015 at 00:22, Hans de Goede <hdegoede@redhat.com> wrote:
>>> Hi,
>>>
>>> On 09-11-15 07:48, Simon Glass wrote:
>>>>
>>>> Each scan of the USB bus may return different results. Existing
>>>> driver-model
>>>> devices are reused when found, but if a device no longer exists it will
>>>> stay
>>>> around, de-activated, but bound.
>>>>
>>>> Detect these devices and remove them after the scan completes.
>>>>
>>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>>
>>>
>>> I wonder how this is better then my original:
>>> "dm: usb: Use device_unbind_children to clean up usb devs on stop"
>>>
>>> Patch, the end result of both patches is the same and both are
>>> a NOP when DM_DEVICE_REMOVE is not set. Where as my code seems
>>> to be a much more KISS approach to the problem (my approach is
>>> just 3 lines vs 23 lines for yours).
>>>
>>> I know we will need usb_find_child in the DM_DEVICE_REMOVE not
>>> set case, but why not only revert the:
>>>
>>> "dm: usb: Rename usb_find_child to usb_find_emul_child"
>>>
>>> commit, keep the other 2 you revert and drop this patch ?
>>>
>>> This drops 3 patches from your patch-set and the end result is
>>> more clean IMHO.
>>
>> I would like to avoid binding/unbinding things when nothing changes if
>> possible. Also I'd like to support attaching device tree
>> nodes/properties to USB devices as necessary, as we do with PCI, and
>> removing things breaks that.
>>
>> I still have to figure out one more test case, so I'll do that before
>> commenting further.
>
> I've added the test case and pushed a new tree. However it turns out
> that this doesn't behave differently.
>
> So please can you go ahead and run your original (manual) test case?
> I'd like to make sure that my automated tests are correct and catch
> the bug you reported and fixed.
Ok, I've ran a whole battery of tests on your u-boot-dm/usb-working branch.
There are 2 issues:
1) You need to add these change to the commit introducing usb-keyb dm
support (or one of the related commits), otherwise usb-keyb support
breaks:
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -513,6 +513,7 @@ config ARCH_SUNXI
select DM
select DM_GPIO
select DM_ETH
+ select DM_KEYBOARD
select DM_SERIAL
select DM_USB
select OF_CONTROL
The breakage is that without this usb_scan_device() returns -96
(EPFNOSUPPORT) for usb keyboards.
2) With that branch there still is the purely cosmetical issue,
that if one plugs a usb-stick + keyb into the same hub, then swaps
their place and do "usb reset" and then "usb tree" looks like this:
USB device tree:
1 Hub (480 Mb/s, 100mA)
| USB2.0 Hub
|
+-3 Human Interface (1.5 Mb/s, 100mA)
| SINO WEALTH USB Composite Device
|
+-2 Mass Storage (480 Mb/s, 100mA)
USB Flash Disk 4C0E960F
Notice the order of devices listed: 1 - 3 - 2, which is a bit weird,
as said this is purely cosmetical.
Note you can easily fix this by only reverting the:
"dm: usb: Rename usb_find_child to usb_find_emul_child"
commit, and keep the other 2 you revert and dropping this patch ?
As I already suggested before, this is both more KISS and as you
can see it solves some actually (admittedly very minor) issues.
I'm not really buying your arguments for your more complex solution,
as shown above re-using the devices actually causes issues.
Your other argument of wanting to attach device-tree properties
I also do not find a strong argument, for one there has never been
a need to do so sofar, and if we ever need this we need a way
to specify which usb-device the properties belong to based on
the topology under the host / root-hub anyway, and match things
up when first scanning the bus. And if we can match things up
on the first scan we can also match them up on subsequent scans
and attach the same of-node again.
Anyways I'm fine with doing things your way, but I still have
a preference for the more KISS and IMHO robust solution of
simple unbinding all devices on usb-stop.
###
Talking about usb-stop, there is still one (BIG!) problem after
this patch set when building usb-support with DM_DEVICE_REMOVE
not set. This means that the controllers dma engines will not
be stopped when booting the actual OS and they will still be
accessing parts of the main memory while the actual OS is
booting, which is BAD. So I suggest adding something like
this to all host drivers which use dma in this way:
#if defined CONFIG_DM_USB && !defined CONFIG_DM_DEVICE_REMOVE
#error The EHCI driver cannot be used without CONFIG_DM_DEVICE_REMOVE otherwise DMA stays active while booting the OS
#endif
Regards,
Hans
>
>>
>>>
>>>
>>>> Changes in v2: None
>>>>
>>>> drivers/usb/host/usb-uclass.c | 23 +++++++++++++++++++++++
>>>> 1 file changed, 23 insertions(+)
>>>>
>>>> diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
>>>> index 4aa92f8..50538e0 100644
>>>> --- a/drivers/usb/host/usb-uclass.c
>>>> +++ b/drivers/usb/host/usb-uclass.c
>>>> @@ -202,6 +202,20 @@ static void usb_scan_bus(struct udevice *bus, bool
>>>> recurse)
>>>> printf("%d USB Device(s) found\n", priv->next_addr);
>>>> }
>>>>
>>>> +static void remove_inactive_children(struct uclass *uc, struct udevice
>>>> *bus)
>>>> +{
>>>> + uclass_foreach_dev(bus, uc) {
>>>> + struct udevice *dev, *next;
>>>> +
>>>> + if (!device_active(bus))
>>>> + continue;
>>>> + device_foreach_child_safe(dev, next, bus) {
>>>> + if (!device_active(dev))
>>>> + device_unbind(dev);
>>>> + }
>>>> + }
>>>> +}
>>>> +
>>>> int usb_init(void)
>>>> {
>>>> int controllers_initialized = 0;
>>>> @@ -270,6 +284,15 @@ int usb_init(void)
>>>> }
>>>>
>>>> debug("scan end\n");
>>>> +
>>>> + /* Remove any devices that were not found on this scan */
>>>> + remove_inactive_children(uc, bus);
>>>> +
>>>> + ret = uclass_get(UCLASS_USB_HUB, &uc);
>>>> + if (ret)
>>>> + return ret;
>>>> + remove_inactive_children(uc, bus);
>>>> +
>>>
>>>
>>> Why do you need to call remove_inactive_children twice here? This seems
>>> worthy of a comment explaining why this is necessary.
>>
>> One is removing the children of USB controllers, one is removing the
>> children of USB hubs. I'll add a comment.
>>
>>>
>>>> /* if we were not able to find at least one working bus, bail out
>>>> */
>>>> if (!count)
>>>> printf("No controllers found\n");
>>
>> Regards,
>> Simon
^ permalink raw reply [flat|nested] 67+ messages in thread
* [U-Boot] [PATCH v2 19/26] dm: test: usb: Add tests for the 'usb tree' command
2015-11-09 6:47 [U-Boot] [PATCH v2 00/26] usb: Drop requirement for USB unbinding, add tests Simon Glass
` (17 preceding siblings ...)
2015-11-09 6:48 ` [U-Boot] [PATCH v2 18/26] dm: usb: Remove inactive children after a bus scan Simon Glass
@ 2015-11-09 6:48 ` Simon Glass
2015-11-20 3:32 ` Simon Glass
2015-11-09 6:48 ` [U-Boot] [PATCH v2 20/26] dm: test: usb: Add a test for device reordering Simon Glass
` (8 subsequent siblings)
27 siblings, 1 reply; 67+ messages in thread
From: Simon Glass @ 2015-11-09 6:48 UTC (permalink / raw)
To: u-boot
Add tests that this command produces the right output, even when a rescan
results in a device disappearing from the bus.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None
arch/sandbox/dts/test.dts | 14 ++++-
test/dm/usb.c | 156 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 169 insertions(+), 1 deletion(-)
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index e2c4971..52749c0 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -307,12 +307,24 @@
compatible = "sandbox,usb-hub";
#address-cells = <1>;
#size-cells = <0>;
- flash-stick {
+ flash-stick at 0 {
reg = <0>;
compatible = "sandbox,usb-flash";
sandbox,filepath = "testflash.bin";
};
+ flash-stick at 1 {
+ reg = <1>;
+ compatible = "sandbox,usb-flash";
+ sandbox,filepath = "testflash1.bin";
+ };
+
+ flash-stick at 2 {
+ reg = <2>;
+ compatible = "sandbox,usb-flash";
+ sandbox,filepath = "testflash2.bin";
+ };
+
};
};
};
diff --git a/test/dm/usb.c b/test/dm/usb.c
index ccc07d5..721d3ad 100644
--- a/test/dm/usb.c
+++ b/test/dm/usb.c
@@ -12,8 +12,11 @@
#include <asm/state.h>
#include <dm/device-internal.h>
#include <dm/test.h>
+#include <dm/uclass-internal.h>
#include <test/ut.h>
+DECLARE_GLOBAL_DATA_PTR;
+
/* Test that sandbox USB works correctly */
static int dm_test_usb_base(struct unit_test_state *uts)
{
@@ -52,3 +55,156 @@ static int dm_test_usb_flash(struct unit_test_state *uts)
return 0;
}
DM_TEST(dm_test_usb_flash, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
+
+/* test that we can handle multiple storage devices */
+static int dm_test_usb_multi(struct unit_test_state *uts)
+{
+ struct udevice *dev;
+
+ state_set_skip_delays(true);
+ ut_assertok(usb_init());
+ ut_assertok(uclass_get_device(UCLASS_MASS_STORAGE, 0, &dev));
+ ut_assertok(uclass_get_device(UCLASS_MASS_STORAGE, 1, &dev));
+ ut_assertok(uclass_get_device(UCLASS_MASS_STORAGE, 2, &dev));
+
+ return 0;
+}
+DM_TEST(dm_test_usb_multi, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
+
+static int count_usb_devices(void)
+{
+ struct udevice *hub;
+ struct uclass *uc;
+ int count = 0;
+ int ret;
+
+ ret = uclass_get(UCLASS_USB_HUB, &uc);
+ if (ret)
+ return ret;
+
+ uclass_foreach_dev(hub, uc) {
+ struct udevice *dev;
+
+ count++;
+ for (device_find_first_child(hub, &dev);
+ dev;
+ device_find_next_child(&dev)) {
+ count++;
+ }
+ }
+
+ return count;
+}
+
+/* test that we can remove an emulated device and it is then not found */
+static int dm_test_usb_remove(struct unit_test_state *uts)
+{
+ struct udevice *dev, *emul;
+
+ /* Scan and check that all devices are present */
+ state_set_skip_delays(true);
+ ut_assertok(usb_init());
+ ut_assertok(uclass_get_device(UCLASS_MASS_STORAGE, 0, &dev));
+ ut_assertok(uclass_get_device(UCLASS_MASS_STORAGE, 1, &dev));
+ ut_assertok(uclass_get_device(UCLASS_MASS_STORAGE, 2, &dev));
+ ut_asserteq(5, count_usb_devices());
+ ut_assertok(usb_stop());
+ ut_asserteq(5, count_usb_devices());
+
+ /* Remove the second emulation device */
+ ut_assertok(uclass_find_device_by_name(UCLASS_USB_EMUL, "flash-stick at 1",
+ &dev));
+ ut_assertok(device_unbind(dev));
+
+ /* Rescan - only the first and third should be present */
+ ut_assertok(usb_init());
+ ut_assertok(uclass_get_device(UCLASS_MASS_STORAGE, 0, &dev));
+ ut_assertok(usb_emul_find_for_dev(dev, &emul));
+ ut_asserteq_str("flash-stick at 0", emul->name);
+ ut_assertok(uclass_get_device(UCLASS_MASS_STORAGE, 1, &dev));
+ ut_assertok(usb_emul_find_for_dev(dev, &emul));
+ ut_asserteq_str("flash-stick at 2", emul->name);
+
+ ut_asserteq(-ENODEV, uclass_get_device(UCLASS_MASS_STORAGE, 2, &dev));
+
+ ut_asserteq(4, count_usb_devices());
+ ut_assertok(usb_stop());
+ ut_asserteq(4, count_usb_devices());
+
+ return 0;
+}
+DM_TEST(dm_test_usb_remove, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
+
+const char usb_tree_base[] =
+" 1 Hub (12 Mb/s, 100mA)\n"
+" | sandbox hub 2345\n"
+" |\n"
+" |\b+-2 Mass Storage (12 Mb/s, 100mA)\n"
+" | sandbox flash flash-stick at 0\n"
+" | \n"
+" |\b+-3 Mass Storage (12 Mb/s, 100mA)\n"
+" | sandbox flash flash-stick at 1\n"
+" | \n"
+" |\b+-4 Mass Storage (12 Mb/s, 100mA)\n"
+" sandbox flash flash-stick at 2\n"
+" \n";
+
+/* test that the 'usb tree' command output looks correct */
+static int dm_test_usb_tree(struct unit_test_state *uts)
+{
+ char *data;
+ int len;
+
+ state_set_skip_delays(true);
+ ut_assertok(usb_init());
+ console_record_reset_enable();
+ usb_show_tree();
+ len = membuff_getraw(&gd->console_out, -1, true, &data);
+ if (len)
+ data[len] = '\0';
+ ut_asserteq_str(usb_tree_base, data);
+ ut_assertok(usb_stop());
+
+ return 0;
+}
+DM_TEST(dm_test_usb_tree, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
+
+const char usb_tree_remove[] =
+" 1 Hub (12 Mb/s, 100mA)\n"
+" | sandbox hub 2345\n"
+" |\n"
+" |\b+-2 Mass Storage (12 Mb/s, 100mA)\n"
+" | sandbox flash flash-stick at 0\n"
+" | \n"
+" |\b+-3 Mass Storage (12 Mb/s, 100mA)\n"
+" sandbox flash flash-stick at 2\n"
+" \n";
+
+/*
+ * test that the 'usb tree' command output looks correct when we remove a
+ * device
+ */
+static int dm_test_usb_tree_remove(struct unit_test_state *uts)
+{
+ struct udevice *dev;
+ char *data;
+ int len;
+
+ /* Remove the second emulation device */
+ ut_assertok(uclass_find_device_by_name(UCLASS_USB_EMUL, "flash-stick at 1",
+ &dev));
+ ut_assertok(device_unbind(dev));
+
+ state_set_skip_delays(true);
+ ut_assertok(usb_init());
+ console_record_reset_enable();
+ usb_show_tree();
+ len = membuff_getraw(&gd->console_out, -1, true, &data);
+ if (len)
+ data[len] = '\0';
+ ut_asserteq_str(usb_tree_remove, data);
+ ut_assertok(usb_stop());
+
+ return 0;
+}
+DM_TEST(dm_test_usb_tree_remove, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply related [flat|nested] 67+ messages in thread* [U-Boot] [PATCH v2 20/26] dm: test: usb: Add a test for device reordering
2015-11-09 6:47 [U-Boot] [PATCH v2 00/26] usb: Drop requirement for USB unbinding, add tests Simon Glass
` (18 preceding siblings ...)
2015-11-09 6:48 ` [U-Boot] [PATCH v2 19/26] dm: test: usb: Add tests for the 'usb tree' command Simon Glass
@ 2015-11-09 6:48 ` Simon Glass
2015-11-20 3:32 ` Simon Glass
2015-11-09 6:48 ` [U-Boot] [PATCH v2 21/26] usb: Drop unused code in usb_kbd.c Simon Glass
` (7 subsequent siblings)
27 siblings, 1 reply; 67+ messages in thread
From: Simon Glass @ 2015-11-09 6:48 UTC (permalink / raw)
To: u-boot
Add tests that 'usb tree' produces the right output when a device changes
order on the bus.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None
test/dm/usb.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/test/dm/usb.c b/test/dm/usb.c
index 721d3ad..fb193e8 100644
--- a/test/dm/usb.c
+++ b/test/dm/usb.c
@@ -208,3 +208,53 @@ static int dm_test_usb_tree_remove(struct unit_test_state *uts)
return 0;
}
DM_TEST(dm_test_usb_tree_remove, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
+
+const char usb_tree_reorder[] =
+" 1 Hub (12 Mb/s, 100mA)\n"
+" | sandbox hub 2345\n"
+" |\n"
+" |\b+-2 Mass Storage (12 Mb/s, 100mA)\n"
+" | sandbox flash flash-stick at 0\n"
+" | \n"
+" |\b+-3 Mass Storage (12 Mb/s, 100mA)\n"
+" | sandbox flash flash-stick at 2\n"
+" | \n"
+" |\b+-4 Mass Storage (12 Mb/s, 100mA)\n"
+" sandbox flash flash-stick at 1\n"
+" \n";
+
+/*
+ * test that the 'usb tree' command output looks correct when we reorder two
+ * devices.
+ */
+static int dm_test_usb_tree_reorder(struct unit_test_state *uts)
+{
+ struct udevice *dev, *parent;
+ char *data;
+ int len;
+
+ /* Remove the second emulation device */
+ ut_assertok(uclass_find_device_by_name(UCLASS_USB_EMUL, "flash-stick at 1",
+ &dev));
+ parent = dev->parent;
+
+ /* Reorder the devices in the parent list and uclass list */
+ list_del(&dev->sibling_node);
+ list_add_tail(&dev->sibling_node, &parent->child_head);
+
+ list_del(&dev->uclass_node);
+ list_add_tail(&dev->uclass_node, &dev->uclass->dev_head);
+
+ state_set_skip_delays(true);
+ ut_assertok(usb_init());
+ console_record_reset_enable();
+ usb_show_tree();
+ len = membuff_getraw(&gd->console_out, -1, true, &data);
+ if (len)
+ data[len] = '\0';
+ ut_asserteq_str(usb_tree_reorder, data);
+ ut_assertok(usb_stop());
+
+ return 0;
+}
+DM_TEST(dm_test_usb_tree_reorder, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply related [flat|nested] 67+ messages in thread* [U-Boot] [PATCH v2 21/26] usb: Drop unused code in usb_kbd.c
2015-11-09 6:47 [U-Boot] [PATCH v2 00/26] usb: Drop requirement for USB unbinding, add tests Simon Glass
` (19 preceding siblings ...)
2015-11-09 6:48 ` [U-Boot] [PATCH v2 20/26] dm: test: usb: Add a test for device reordering Simon Glass
@ 2015-11-09 6:48 ` Simon Glass
2015-11-20 3:32 ` Simon Glass
2015-11-09 6:48 ` [U-Boot] [PATCH v2 22/26] usb: Avoid open-coded USB constants " Simon Glass
` (6 subsequent siblings)
27 siblings, 1 reply; 67+ messages in thread
From: Simon Glass @ 2015-11-09 6:48 UTC (permalink / raw)
To: u-boot
This was missed in the conversion to driver model.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None
common/usb_kbd.c | 30 ------------------------------
1 file changed, 30 deletions(-)
diff --git a/common/usb_kbd.c b/common/usb_kbd.c
index 5a90f84..1d85212 100644
--- a/common/usb_kbd.c
+++ b/common/usb_kbd.c
@@ -541,35 +541,6 @@ int drv_usb_kbd_init(void)
int error, i;
debug("%s: Probing for keyboard\n", __func__);
-#ifdef CONFIG_DM_USB
- /*
- * TODO: We should add U_BOOT_USB_DEVICE() declarations to each USB
- * keyboard driver and then most of this file can be removed.
- */
- struct udevice *bus;
- struct uclass *uc;
- int ret;
-
- ret = uclass_get(UCLASS_USB, &uc);
- if (ret)
- return ret;
- uclass_foreach_dev(bus, uc) {
- for (i = 0; i < USB_MAX_DEVICE; i++) {
- struct usb_device *dev;
-
- dev = usb_get_dev_index(bus, i); /* get device */
- debug("i=%d, %p\n", i, dev);
- if (!dev)
- break; /* no more devices available */
-
- error = probe_usb_keyboard(dev);
- if (!error)
- return 1;
- if (error && error != -ENOENT)
- return error;
- } /* for */
- }
-#else
/* Scan all USB Devices */
for (i = 0; i < USB_MAX_DEVICE; i++) {
struct usb_device *dev;
@@ -588,7 +559,6 @@ int drv_usb_kbd_init(void)
if (error && error != -ENOENT)
return error;
}
-#endif
/* No USB Keyboard found */
return -1;
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply related [flat|nested] 67+ messages in thread* [U-Boot] [PATCH v2 22/26] usb: Avoid open-coded USB constants in usb_kbd.c
2015-11-09 6:47 [U-Boot] [PATCH v2 00/26] usb: Drop requirement for USB unbinding, add tests Simon Glass
` (20 preceding siblings ...)
2015-11-09 6:48 ` [U-Boot] [PATCH v2 21/26] usb: Drop unused code in usb_kbd.c Simon Glass
@ 2015-11-09 6:48 ` Simon Glass
2015-11-20 3:32 ` Simon Glass
2015-11-09 6:48 ` [U-Boot] [PATCH v2 23/26] usb: sandbox: Add support for interrupt operations Simon Glass
` (5 subsequent siblings)
27 siblings, 1 reply; 67+ messages in thread
From: Simon Glass @ 2015-11-09 6:48 UTC (permalink / raw)
To: u-boot
Replace the open-coded values with constants to make it clearer what they
mean.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None
common/usb_kbd.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/common/usb_kbd.c b/common/usb_kbd.c
index 1d85212..069fbd2 100644
--- a/common/usb_kbd.c
+++ b/common/usb_kbd.c
@@ -411,13 +411,13 @@ static int usb_kbd_probe_dev(struct usb_device *dev, unsigned int ifnum)
iface = &dev->config.if_desc[ifnum];
- if (iface->desc.bInterfaceClass != 3)
+ if (iface->desc.bInterfaceClass != USB_CLASS_HID)
return 0;
- if (iface->desc.bInterfaceSubClass != 1)
+ if (iface->desc.bInterfaceSubClass != USB_SUB_HID_BOOT)
return 0;
- if (iface->desc.bInterfaceProtocol != 1)
+ if (iface->desc.bInterfaceProtocol != USB_PROT_HID_KEYBOARD)
return 0;
if (iface->desc.bNumEndpoints != 1)
@@ -626,8 +626,8 @@ static const struct usb_device_id kbd_id_table[] = {
USB_DEVICE_ID_MATCH_INT_SUBCLASS |
USB_DEVICE_ID_MATCH_INT_PROTOCOL,
.bInterfaceClass = USB_CLASS_HID,
- .bInterfaceSubClass = 1,
- .bInterfaceProtocol = 1,
+ .bInterfaceSubClass = USB_SUB_HID_BOOT,
+ .bInterfaceProtocol = USB_PROT_HID_KEYBOARD,
},
{ } /* Terminating entry */
};
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply related [flat|nested] 67+ messages in thread* [U-Boot] [PATCH v2 23/26] usb: sandbox: Add support for interrupt operations
2015-11-09 6:47 [U-Boot] [PATCH v2 00/26] usb: Drop requirement for USB unbinding, add tests Simon Glass
` (21 preceding siblings ...)
2015-11-09 6:48 ` [U-Boot] [PATCH v2 22/26] usb: Avoid open-coded USB constants " Simon Glass
@ 2015-11-09 6:48 ` Simon Glass
2015-11-20 3:32 ` Simon Glass
2015-11-09 6:48 ` [U-Boot] [PATCH v2 24/26] usb: sandbox: Add a USB emulation driver Simon Glass
` (4 subsequent siblings)
27 siblings, 1 reply; 67+ messages in thread
From: Simon Glass @ 2015-11-09 6:48 UTC (permalink / raw)
To: u-boot
Allow USB device emulation to support interrupt URBs so that we can use USB
keyboards with sandbox.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None
drivers/usb/emul/usb-emul-uclass.c | 12 ++++++++++++
drivers/usb/host/usb-sandbox.c | 19 +++++++++++++++++++
include/usb.h | 11 +++++++++++
3 files changed, 42 insertions(+)
diff --git a/drivers/usb/emul/usb-emul-uclass.c b/drivers/usb/emul/usb-emul-uclass.c
index 6b5f3c0..ee7ea5a 100644
--- a/drivers/usb/emul/usb-emul-uclass.c
+++ b/drivers/usb/emul/usb-emul-uclass.c
@@ -218,6 +218,18 @@ int usb_emul_bulk(struct udevice *emul, struct usb_device *udev,
return ops->bulk(emul, udev, pipe, buffer, length);
}
+int usb_emul_int(struct udevice *emul, struct usb_device *udev,
+ unsigned long pipe, void *buffer, int length, int interval)
+{
+ struct dm_usb_ops *ops = usb_get_emul_ops(emul);
+
+ if (!ops->interrupt)
+ return -ENOSYS;
+ debug("%s: dev=%s\n", __func__, emul->name);
+
+ return ops->interrupt(emul, udev, pipe, buffer, length, interval);
+}
+
int usb_emul_setup_device(struct udevice *dev, int maxpacketsize,
struct usb_string *strings, void **desc_list)
{
diff --git a/drivers/usb/host/usb-sandbox.c b/drivers/usb/host/usb-sandbox.c
index c5f9822..5e3d96c 100644
--- a/drivers/usb/host/usb-sandbox.c
+++ b/drivers/usb/host/usb-sandbox.c
@@ -87,6 +87,24 @@ static int sandbox_submit_bulk(struct udevice *bus, struct usb_device *udev,
return ret;
}
+static int sandbox_submit_int(struct udevice *bus, struct usb_device *udev,
+ unsigned long pipe, void *buffer, int length,
+ int interval)
+{
+ struct udevice *emul;
+ int ret;
+
+ /* Just use child of dev as emulator? */
+ debug("%s: bus=%s\n", __func__, bus->name);
+ ret = usb_emul_find(bus, pipe, &emul);
+ usbmon_trace(bus, pipe, NULL, emul);
+ if (ret)
+ return ret;
+ ret = usb_emul_int(emul, udev, pipe, buffer, length, interval);
+
+ return ret;
+}
+
static int sandbox_alloc_device(struct udevice *dev, struct usb_device *udev)
{
return 0;
@@ -100,6 +118,7 @@ static int sandbox_usb_probe(struct udevice *dev)
static const struct dm_usb_ops sandbox_usb_ops = {
.control = sandbox_submit_control,
.bulk = sandbox_submit_bulk,
+ .interrupt = sandbox_submit_int,
.alloc_device = sandbox_alloc_device,
};
diff --git a/include/usb.h b/include/usb.h
index d684531..55b9268 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -938,6 +938,17 @@ int usb_emul_bulk(struct udevice *emul, struct usb_device *udev,
unsigned long pipe, void *buffer, int length);
/**
+ * usb_emul_int() - Send an interrupt packet to an emulator
+ *
+ * @emul: Emulator device
+ * @udev: USB device (which the emulator is causing to appear)
+ * See struct dm_usb_ops for details on other parameters
+ * @return 0 if OK, -ve on error
+ */
+int usb_emul_int(struct udevice *emul, struct usb_device *udev,
+ unsigned long pipe, void *buffer, int length, int interval);
+
+/**
* usb_emul_find() - Find an emulator for a particular device
*
* Check @pipe to find a device number on bus @bus and return it.
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply related [flat|nested] 67+ messages in thread* [U-Boot] [PATCH v2 24/26] usb: sandbox: Add a USB emulation driver
2015-11-09 6:47 [U-Boot] [PATCH v2 00/26] usb: Drop requirement for USB unbinding, add tests Simon Glass
` (22 preceding siblings ...)
2015-11-09 6:48 ` [U-Boot] [PATCH v2 23/26] usb: sandbox: Add support for interrupt operations Simon Glass
@ 2015-11-09 6:48 ` Simon Glass
2015-11-20 3:32 ` Simon Glass
2015-11-09 6:48 ` [U-Boot] [PATCH v2 25/26] sandbox: Enable USB keyboard Simon Glass
` (3 subsequent siblings)
27 siblings, 1 reply; 67+ messages in thread
From: Simon Glass @ 2015-11-09 6:48 UTC (permalink / raw)
To: u-boot
Add a simple USB keyboard driver for sandbox. It provides a function to
'load' it with input data, which it will then stream through to the normal
U-Boot input subsystem. When the input data is exhausted, the keyboard stops
providing data.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None
arch/sandbox/include/asm/test.h | 2 +
drivers/usb/emul/Makefile | 1 +
drivers/usb/emul/sandbox_keyb.c | 241 ++++++++++++++++++++++++++++++++++++++++
include/linux/usb/ch9.h | 20 ++++
4 files changed, 264 insertions(+)
create mode 100644 drivers/usb/emul/sandbox_keyb.c
diff --git a/arch/sandbox/include/asm/test.h b/arch/sandbox/include/asm/test.h
index d3c7851..224b0eb 100644
--- a/arch/sandbox/include/asm/test.h
+++ b/arch/sandbox/include/asm/test.h
@@ -86,4 +86,6 @@ long sandbox_i2c_rtc_set_offset(struct udevice *dev, bool use_system_time,
*/
long sandbox_i2c_rtc_get_set_base_time(struct udevice *dev, long base_time);
+int sandbox_usb_keyb_add_string(struct udevice *dev, const char *str);
+
#endif
diff --git a/drivers/usb/emul/Makefile b/drivers/usb/emul/Makefile
index 8fd83d5..b64ac6d 100644
--- a/drivers/usb/emul/Makefile
+++ b/drivers/usb/emul/Makefile
@@ -7,4 +7,5 @@
obj-$(CONFIG_USB_EMUL) += sandbox_flash.o
obj-$(CONFIG_USB_EMUL) += sandbox_hub.o
+obj-$(CONFIG_USB_EMUL) += sandbox_keyb.o
obj-$(CONFIG_USB_EMUL) += usb-emul-uclass.o
diff --git a/drivers/usb/emul/sandbox_keyb.c b/drivers/usb/emul/sandbox_keyb.c
new file mode 100644
index 0000000..2735985
--- /dev/null
+++ b/drivers/usb/emul/sandbox_keyb.c
@@ -0,0 +1,241 @@
+/*
+ * (C) Copyright 2015 Google, Inc
+ * Written by Simon Glass <sjg@chromium.org>
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <os.h>
+#include <scsi.h>
+#include <usb.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/*
+ * This driver emulates a USB keyboard using the USB HID specification (boot
+ * protocol)
+ */
+
+enum {
+ SANDBOX_KEYB_EP_IN = 1, /* endpoints */
+};
+
+enum cmd_phase {
+ PHASE_START,
+ PHASE_DATA,
+ PHASE_STATUS,
+};
+
+enum {
+ STRINGID_MANUFACTURER = 1,
+ STRINGID_PRODUCT,
+ STRINGID_SERIAL,
+
+ STRINGID_COUNT,
+};
+
+/**
+ * struct sandbox_keyb_priv - private state for this driver
+ *
+ */
+struct sandbox_keyb_priv {
+ struct membuff in;
+};
+
+struct sandbox_keyb_plat {
+ struct usb_string keyb_strings[STRINGID_COUNT];
+};
+
+static struct usb_device_descriptor keyb_device_desc = {
+ .bLength = sizeof(keyb_device_desc),
+ .bDescriptorType = USB_DT_DEVICE,
+
+ .bcdUSB = __constant_cpu_to_le16(0x0100),
+
+ .bDeviceClass = 0,
+ .bDeviceSubClass = 0,
+ .bDeviceProtocol = 0,
+
+ .idVendor = __constant_cpu_to_le16(0x1234),
+ .idProduct = __constant_cpu_to_le16(0x5679),
+ .iManufacturer = STRINGID_MANUFACTURER,
+ .iProduct = STRINGID_PRODUCT,
+ .iSerialNumber = STRINGID_SERIAL,
+ .bNumConfigurations = 1,
+};
+
+static struct usb_config_descriptor keyb_config0 = {
+ .bLength = sizeof(keyb_config0),
+ .bDescriptorType = USB_DT_CONFIG,
+
+ /* wTotalLength is set up by usb-emul-uclass */
+ .bNumInterfaces = 2,
+ .bConfigurationValue = 0,
+ .iConfiguration = 0,
+ .bmAttributes = 1 << 7 | 1 << 5,
+ .bMaxPower = 50,
+};
+
+static struct usb_interface_descriptor keyb_interface0 = {
+ .bLength = sizeof(keyb_interface0),
+ .bDescriptorType = USB_DT_INTERFACE,
+
+ .bInterfaceNumber = 0,
+ .bAlternateSetting = 0,
+ .bNumEndpoints = 1,
+ .bInterfaceClass = USB_CLASS_HID,
+ .bInterfaceSubClass = USB_SUB_HID_BOOT,
+ .bInterfaceProtocol = USB_PROT_HID_KEYBOARD,
+ .iInterface = 0,
+};
+
+static struct usb_class_hid_descriptor keyb_report0 = {
+ .bLength = sizeof(keyb_report0),
+ .bDescriptorType = USB_DT_HID,
+ .bcdCDC = 0x101,
+ .bCountryCode = 0,
+ .bNumDescriptors = 1,
+ .bDescriptorType0 = USB_DT_HID_REPORT,
+ .wDescriptorLength0 = 0x3f,
+};
+
+static struct usb_endpoint_descriptor keyb_endpoint0_in = {
+ .bLength = USB_DT_ENDPOINT_SIZE,
+ .bDescriptorType = USB_DT_ENDPOINT,
+
+ .bEndpointAddress = SANDBOX_KEYB_EP_IN | USB_ENDPOINT_DIR_MASK,
+ .bmAttributes = USB_ENDPOINT_XFER_BULK |
+ USB_ENDPOINT_XFER_ISOC,
+ .wMaxPacketSize = __constant_cpu_to_le16(8),
+ .bInterval = 0xa,
+};
+
+static struct usb_interface_descriptor keyb_interface1 = {
+ .bLength = sizeof(keyb_interface1),
+ .bDescriptorType = USB_DT_INTERFACE,
+
+ .bInterfaceNumber = 1,
+ .bAlternateSetting = 0,
+ .bNumEndpoints = 1,
+ .bInterfaceClass = USB_CLASS_HID,
+ .bInterfaceSubClass = USB_SUB_HID_BOOT,
+ .bInterfaceProtocol = USB_PROT_HID_MOUSE,
+ .iInterface = 0,
+};
+
+static struct usb_class_hid_descriptor keyb_report1 = {
+ .bLength = sizeof(struct usb_class_hid_descriptor),
+ .bDescriptorType = USB_DT_HID,
+ .bcdCDC = 0x101,
+ .bCountryCode = 0,
+ .bNumDescriptors = 1,
+ .bDescriptorType0 = USB_DT_HID_REPORT,
+ .wDescriptorLength0 = 0x32,
+};
+
+static struct usb_endpoint_descriptor keyb_endpoint1_in = {
+ .bLength = USB_DT_ENDPOINT_SIZE,
+ .bDescriptorType = USB_DT_ENDPOINT,
+
+ .bEndpointAddress = SANDBOX_KEYB_EP_IN | USB_ENDPOINT_DIR_MASK,
+ .bmAttributes = USB_ENDPOINT_XFER_BULK |
+ USB_ENDPOINT_XFER_ISOC,
+ .wMaxPacketSize = __constant_cpu_to_le16(8),
+ .bInterval = 0xa,
+};
+
+static void *keyb_desc_list[] = {
+ &keyb_device_desc,
+ &keyb_config0,
+ &keyb_interface0,
+ &keyb_report0,
+ &keyb_endpoint0_in,
+ &keyb_interface1,
+ &keyb_report1,
+ &keyb_endpoint1_in,
+ NULL,
+};
+
+int sandbox_usb_keyb_add_string(struct udevice *dev, const char *str)
+{
+ struct sandbox_keyb_priv *priv = dev_get_priv(dev);
+ int len, ret;
+
+ len = strlen(str);
+ ret = membuff_put(&priv->in, str, len);
+ if (ret != len)
+ return -ENOSPC;
+
+ return 0;
+}
+
+static int sandbox_keyb_control(struct udevice *dev, struct usb_device *udev,
+ unsigned long pipe, void *buff, int len,
+ struct devrequest *setup)
+{
+ debug("pipe=%lx\n", pipe);
+
+ return -EIO;
+}
+
+static int sandbox_keyb_interrupt(struct udevice *dev, struct usb_device *udev,
+ unsigned long pipe, void *buffer, int length, int interval)
+{
+ struct sandbox_keyb_priv *priv = dev_get_priv(dev);
+ uint8_t *data = buffer;
+ int ch;
+
+ memset(data, '\0', length);
+ ch = membuff_getbyte(&priv->in);
+ if (ch != -1)
+ data[2] = 4 + ch - 'a';
+
+ return 0;
+}
+
+static int sandbox_keyb_bind(struct udevice *dev)
+{
+ struct sandbox_keyb_plat *plat = dev_get_platdata(dev);
+ struct usb_string *fs;
+
+ fs = plat->keyb_strings;
+ fs[0].id = STRINGID_MANUFACTURER;
+ fs[0].s = "sandbox";
+ fs[1].id = STRINGID_PRODUCT;
+ fs[1].s = "keyboard";
+ fs[2].id = STRINGID_SERIAL;
+ fs[2].s = dev->name;
+
+ return usb_emul_setup_device(dev, PACKET_SIZE_8, plat->keyb_strings,
+ keyb_desc_list);
+}
+
+static int sandbox_keyb_probe(struct udevice *dev)
+{
+ struct sandbox_keyb_priv *priv = dev_get_priv(dev);
+
+ return membuff_new(&priv->in, 256);
+}
+
+static const struct dm_usb_ops sandbox_usb_keyb_ops = {
+ .control = sandbox_keyb_control,
+ .interrupt = sandbox_keyb_interrupt,
+};
+
+static const struct udevice_id sandbox_usb_keyb_ids[] = {
+ { .compatible = "sandbox,usb-keyb" },
+ { }
+};
+
+U_BOOT_DRIVER(usb_sandbox_keyb) = {
+ .name = "usb_sandbox_keyb",
+ .id = UCLASS_USB_EMUL,
+ .of_match = sandbox_usb_keyb_ids,
+ .bind = sandbox_keyb_bind,
+ .probe = sandbox_keyb_probe,
+ .ops = &sandbox_usb_keyb_ops,
+ .priv_auto_alloc_size = sizeof(struct sandbox_keyb_priv),
+ .platdata_auto_alloc_size = sizeof(struct sandbox_keyb_plat),
+};
diff --git a/include/linux/usb/ch9.h b/include/linux/usb/ch9.h
index 822fca0..0ad4782 100644
--- a/include/linux/usb/ch9.h
+++ b/include/linux/usb/ch9.h
@@ -229,6 +229,8 @@ struct usb_ctrlrequest {
#define USB_DT_PIPE_USAGE 0x24
/* From the USB 3.0 spec */
#define USB_DT_SS_ENDPOINT_COMP 0x30
+/* From HID 1.11 spec */
+#define USB_DT_HID_REPORT 0x22
/* Conventional codes for class-specific descriptors. The convention is
* defined in the USB "Common Class" Spec (3.11). Individual class specs
@@ -385,6 +387,24 @@ struct usb_generic_descriptor {
__u8 bDescriptorType;
};
+struct __packed usb_class_hid_descriptor {
+ u8 bLength;
+ u8 bDescriptorType;
+ u16 bcdCDC;
+ u8 bCountryCode;
+ u8 bNumDescriptors; /* 0x01 */
+ u8 bDescriptorType0;
+ u16 wDescriptorLength0;
+ /* optional descriptors are not supported. */
+};
+
+struct __packed usb_class_report_descriptor {
+ u8 bLength; /* dummy */
+ u8 bDescriptorType;
+ u16 wLength;
+ u8 bData[0];
+};
+
/*
* Endpoints
*/
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply related [flat|nested] 67+ messages in thread* [U-Boot] [PATCH v2 25/26] sandbox: Enable USB keyboard
2015-11-09 6:47 [U-Boot] [PATCH v2 00/26] usb: Drop requirement for USB unbinding, add tests Simon Glass
` (23 preceding siblings ...)
2015-11-09 6:48 ` [U-Boot] [PATCH v2 24/26] usb: sandbox: Add a USB emulation driver Simon Glass
@ 2015-11-09 6:48 ` Simon Glass
2015-11-20 3:32 ` Simon Glass
2015-11-09 6:48 ` [U-Boot] [PATCH v2 26/26] dm: test: usb: sandbox: Add keyboard tests for sandbox Simon Glass
` (2 subsequent siblings)
27 siblings, 1 reply; 67+ messages in thread
From: Simon Glass @ 2015-11-09 6:48 UTC (permalink / raw)
To: u-boot
Enable the USB keyboard on sandbox, now that we have a suitable emulation
driver.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None
configs/sandbox_defconfig | 2 ++
include/configs/sandbox.h | 2 +-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index ac3cce2..d1f10d5 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -72,3 +72,5 @@ CONFIG_ADC_SANDBOX=y
CONFIG_CONSOLE_RECORD=y
CONFIG_CONSOLE_RECORD_OUT_SIZE=0x1000
CONFIG_SYS_MALLOC_F_LEN=0x2000
+CONFIG_USB_KEYBOARD=y
+CONFIG_SYS_USB_EVENT_POLL=y
diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h
index f52488b..c89bc5a 100644
--- a/include/configs/sandbox.h
+++ b/include/configs/sandbox.h
@@ -178,7 +178,7 @@
#define CONFIG_KEYBOARD
-#define SANDBOX_SERIAL_SETTINGS "stdin=serial,cros-ec-keyb\0" \
+#define SANDBOX_SERIAL_SETTINGS "stdin=serial,cros-ec-keyb,usbkbd\0" \
"stdout=serial,lcd\0" \
"stderr=serial,lcd\0"
#else
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply related [flat|nested] 67+ messages in thread* [U-Boot] [PATCH v2 26/26] dm: test: usb: sandbox: Add keyboard tests for sandbox
2015-11-09 6:47 [U-Boot] [PATCH v2 00/26] usb: Drop requirement for USB unbinding, add tests Simon Glass
` (24 preceding siblings ...)
2015-11-09 6:48 ` [U-Boot] [PATCH v2 25/26] sandbox: Enable USB keyboard Simon Glass
@ 2015-11-09 6:48 ` Simon Glass
2015-11-20 3:32 ` Simon Glass
2015-11-09 8:14 ` [U-Boot] [PATCH v2 00/26] usb: Drop requirement for USB unbinding, add tests Hans de Goede
2015-11-09 14:17 ` Marek Vasut
27 siblings, 1 reply; 67+ messages in thread
From: Simon Glass @ 2015-11-09 6:48 UTC (permalink / raw)
To: u-boot
Add a test that verifies that USB keyboards work correctly on sandbox.
This verifies some additional parts of the USB stack.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Add various patches to support USB keyboards and additional tests
arch/sandbox/dts/test.dts | 5 +++++
test/dm/usb.c | 31 +++++++++++++++++++++++++++++++
2 files changed, 36 insertions(+)
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index 52749c0..b6d9a15 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -325,6 +325,11 @@
sandbox,filepath = "testflash2.bin";
};
+ keyb at 3 {
+ reg = <3>;
+ compatible = "sandbox,usb-keyb";
+ };
+
};
};
};
diff --git a/test/dm/usb.c b/test/dm/usb.c
index fb193e8..7d6b644 100644
--- a/test/dm/usb.c
+++ b/test/dm/usb.c
@@ -10,6 +10,7 @@
#include <usb.h>
#include <asm/io.h>
#include <asm/state.h>
+#include <asm/test.h>
#include <dm/device-internal.h>
#include <dm/test.h>
#include <dm/uclass-internal.h>
@@ -258,3 +259,33 @@ static int dm_test_usb_tree_reorder(struct unit_test_state *uts)
return 0;
}
DM_TEST(dm_test_usb_tree_reorder, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
+
+static int dm_test_usb_keyb(struct unit_test_state *uts)
+{
+ struct udevice *dev;
+
+ state_set_skip_delays(true);
+ ut_assertok(usb_init());
+
+ /* Initially there should be no characters */
+ ut_asserteq(0, tstc());
+
+ ut_assertok(uclass_get_device_by_name(UCLASS_USB_EMUL, "keyb",
+ &dev));
+
+ /*
+ * Add a string to the USB keyboard buffer - it should appear in
+ * stdin
+ */
+ ut_assertok(sandbox_usb_keyb_add_string(dev, "ab"));
+ ut_asserteq(1, tstc());
+ ut_asserteq('a', getc());
+ ut_asserteq(1, tstc());
+ ut_asserteq('b', getc());
+ ut_asserteq(0, tstc());
+
+ ut_assertok(usb_stop());
+
+ return 0;
+}
+DM_TEST(dm_test_usb_keyb, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply related [flat|nested] 67+ messages in thread* [U-Boot] [PATCH v2 00/26] usb: Drop requirement for USB unbinding, add tests
2015-11-09 6:47 [U-Boot] [PATCH v2 00/26] usb: Drop requirement for USB unbinding, add tests Simon Glass
` (25 preceding siblings ...)
2015-11-09 6:48 ` [U-Boot] [PATCH v2 26/26] dm: test: usb: sandbox: Add keyboard tests for sandbox Simon Glass
@ 2015-11-09 8:14 ` Hans de Goede
2015-11-09 16:54 ` Simon Glass
2015-11-09 14:17 ` Marek Vasut
27 siblings, 1 reply; 67+ messages in thread
From: Hans de Goede @ 2015-11-09 8:14 UTC (permalink / raw)
To: u-boot
Hi Simon,
On 09-11-15 07:47, Simon Glass wrote:
> There was quite a bit of discussion about the change that required the
> unbinding of USB devices for the subsystem to function correctly. E.g.
>
> https://patchwork.ozlabs.org/patch/485637/
>
> The key issue is the usb_get_dev_index() function which is not a good API
> for driver model. We can drop use of this function once everything is
> converted to driver model. Then I believe the problems raised by Hans go
> away. For now we can add a deprecation warning on the function.
>
> It is easy to convert USB keyboards to driver model. This series includes
> a patch for that.
>
> This series also includes reverts for the three commits which as discussed
> I would like to drop. U-Boot should be able to run normally and exit without
> unbinding anything.
>
> Also included are some tests for the 'usb tree' command. To make this work,
> console recording is implemented.
>
> Finally, a USB keyboard driver is provided for sandbox, so that this part
> of the stack can be tested automatically.
>
> Changes in v2:
> - Add various patches to support USB keyboards and additional tests
Do you have a git branch with this patch-set somewhere for me to test ?
Regards,
Hans
^ permalink raw reply [flat|nested] 67+ messages in thread* [U-Boot] [PATCH v2 00/26] usb: Drop requirement for USB unbinding, add tests
2015-11-09 8:14 ` [U-Boot] [PATCH v2 00/26] usb: Drop requirement for USB unbinding, add tests Hans de Goede
@ 2015-11-09 16:54 ` Simon Glass
2015-11-18 15:53 ` Simon Glass
0 siblings, 1 reply; 67+ messages in thread
From: Simon Glass @ 2015-11-09 16:54 UTC (permalink / raw)
To: u-boot
Hi Hans,
On 9 November 2015 at 00:14, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi Simon,
>
> On 09-11-15 07:47, Simon Glass wrote:
>>
>> There was quite a bit of discussion about the change that required the
>> unbinding of USB devices for the subsystem to function correctly. E.g.
>>
>> https://patchwork.ozlabs.org/patch/485637/
>>
>> The key issue is the usb_get_dev_index() function which is not a good API
>> for driver model. We can drop use of this function once everything is
>> converted to driver model. Then I believe the problems raised by Hans go
>> away. For now we can add a deprecation warning on the function.
>>
>> It is easy to convert USB keyboards to driver model. This series includes
>> a patch for that.
>>
>> This series also includes reverts for the three commits which as discussed
>> I would like to drop. U-Boot should be able to run normally and exit
>> without
>> unbinding anything.
>>
>> Also included are some tests for the 'usb tree' command. To make this
>> work,
>> console recording is implemented.
>>
>> Finally, a USB keyboard driver is provided for sandbox, so that this part
>> of the stack can be tested automatically.
>>
>> Changes in v2:
>> - Add various patches to support USB keyboards and additional tests
>
>
> Do you have a git branch with this patch-set somewhere for me to test ?
Yes I always upload branches before sending them. This one is
u-boot-dm/usb-working. I should get better at including the branch
name in the cover letter.
I still need to write a test for having two device types which switch
order. Now that I have a USB keyboard emulation I should be able to do
that. I'll then see the impact on 'usb tree'. So you may want to
review the test code only for now.
Regards,
Simon
^ permalink raw reply [flat|nested] 67+ messages in thread
* [U-Boot] [PATCH v2 00/26] usb: Drop requirement for USB unbinding, add tests
2015-11-09 16:54 ` Simon Glass
@ 2015-11-18 15:53 ` Simon Glass
0 siblings, 0 replies; 67+ messages in thread
From: Simon Glass @ 2015-11-18 15:53 UTC (permalink / raw)
To: u-boot
Hi,
On 9 November 2015 at 09:54, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Hans,
>
> On 9 November 2015 at 00:14, Hans de Goede <hdegoede@redhat.com> wrote:
> > Hi Simon,
> >
> > On 09-11-15 07:47, Simon Glass wrote:
> >>
> >> There was quite a bit of discussion about the change that required the
> >> unbinding of USB devices for the subsystem to function correctly. E.g.
> >>
> >> https://patchwork.ozlabs.org/patch/485637/
> >>
> >> The key issue is the usb_get_dev_index() function which is not a good API
> >> for driver model. We can drop use of this function once everything is
> >> converted to driver model. Then I believe the problems raised by Hans go
> >> away. For now we can add a deprecation warning on the function.
> >>
> >> It is easy to convert USB keyboards to driver model. This series includes
> >> a patch for that.
> >>
> >> This series also includes reverts for the three commits which as discussed
> >> I would like to drop. U-Boot should be able to run normally and exit
> >> without
> >> unbinding anything.
> >>
> >> Also included are some tests for the 'usb tree' command. To make this
> >> work,
> >> console recording is implemented.
> >>
> >> Finally, a USB keyboard driver is provided for sandbox, so that this part
> >> of the stack can be tested automatically.
> >>
> >> Changes in v2:
> >> - Add various patches to support USB keyboards and additional tests
> >
> >
> > Do you have a git branch with this patch-set somewhere for me to test ?
>
> Yes I always upload branches before sending them. This one is
> u-boot-dm/usb-working. I should get better at including the branch
> name in the cover letter.
>
> I still need to write a test for having two device types which switch
> order. Now that I have a USB keyboard emulation I should be able to do
> that. I'll then see the impact on 'usb tree'. So you may want to
> review the test code only for now.
>
> Regards,
> Simon
Are there any more comments on this series please? I plan to bring it
in soon since we are already at RC1.
Regards,
Simon
^ permalink raw reply [flat|nested] 67+ messages in thread
* [U-Boot] [PATCH v2 00/26] usb: Drop requirement for USB unbinding, add tests
2015-11-09 6:47 [U-Boot] [PATCH v2 00/26] usb: Drop requirement for USB unbinding, add tests Simon Glass
` (26 preceding siblings ...)
2015-11-09 8:14 ` [U-Boot] [PATCH v2 00/26] usb: Drop requirement for USB unbinding, add tests Hans de Goede
@ 2015-11-09 14:17 ` Marek Vasut
27 siblings, 0 replies; 67+ messages in thread
From: Marek Vasut @ 2015-11-09 14:17 UTC (permalink / raw)
To: u-boot
On Monday, November 09, 2015 at 07:47:42 AM, Simon Glass wrote:
> There was quite a bit of discussion about the change that required the
> unbinding of USB devices for the subsystem to function correctly. E.g.
>
> https://patchwork.ozlabs.org/patch/485637/
>
> The key issue is the usb_get_dev_index() function which is not a good API
> for driver model. We can drop use of this function once everything is
> converted to driver model. Then I believe the problems raised by Hans go
> away. For now we can add a deprecation warning on the function.
>
> It is easy to convert USB keyboards to driver model. This series includes
> a patch for that.
>
> This series also includes reverts for the three commits which as discussed
> I would like to drop. U-Boot should be able to run normally and exit
> without unbinding anything.
>
> Also included are some tests for the 'usb tree' command. To make this work,
> console recording is implemented.
>
> Finally, a USB keyboard driver is provided for sandbox, so that this part
> of the stack can be tested automatically.
>
> Changes in v2:
> - Add various patches to support USB keyboards and additional tests
Briefly
Reviewed-by: Marek Vasut <marex@denx.de>
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 67+ messages in thread