USB keyboard: don't crash on "usb reset"
cover USB keyboard: don't crash on "usb reset"
Hi Marek,
Here is the second set of USB patches / fixes I've been working on, currently u-boot does "really bad things" (tm) when doing "usb reset" while using an usb keyboard. This set fixes this.
Given that these fix "really bad things" (tm) I not only believe that these are suitable to go into v2014.10, but I also believe that they actually should :)
Regards,
Hans
PATCH 1/5 usb: kbd: Do not treat -ENODEV as an error for usb_kbd_deregister
ENODEV menas no usb keyboard was registered, threat this as a successful usb_kbd_deregister.
--- common/usb_kbd.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
common/usb_kbd.c +6 -1
@@ -8,6 +8,7 @@* SPDX-License-Identifier: GPL-2.0+ */ #include <common.h> +#include <errno.h> #include <malloc.h> #include <stdio_dev.h> #include <asm/byteorder.h>
@@ -559,7 +560,11 @@ int drv_usb_kbd_init(void)int usb_kbd_deregister (void ) {#ifdef CONFIG_SYS_STDIO_DEREGISTER -return stdio_deregister (DEVNAME ); +int ret =stdio_deregister (DEVNAME ); +if (ret && ret != -ENODEV ) +return ret; + +return 0 ;#else return 1 ;#endif
Signature · 1 line
2.1.0
PATCH 2/5 usb: kbd: On a "usb reset" call usb_kbd_deregister() before calling usb_stop()
In reply to Hans de GoedeWe need to call usb_kbd_deregister() before calling usb_stop().
usbkbd's stdio_dev->priv points to the usb_device, and usb_kbd_testc dereferences usb_device->privptr.
usb_stop zeros usb_device, leaving usb_device->privptr NULL, causing bad things (tm) to happen once control returns to the main loop and usb_kbd_testc gets called.
Calling usb_kbd_deregister() avoids this. Note that we do not allow the "usb reset" to continue when the deregister fails. This will be fixed in a later patch.
For the same reasons always fail "usb stop" if the usb_kbd_deregister() fails, even in the force path. This can happen when CONFIG_SYS_STDIO_DEREGISTER is not set.
--- common/cmd_usb.c | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-)
common/cmd_usb.c +15 -12
@@ -430,6 +430,16 @@ static int do_usbboot(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) }#endif /* CONFIG_USB_STORAGE */ +static int do_usb_stop_keyboard (void ) +{ +#ifdef CONFIG_USB_KEYBOARD +if (usb_kbd_deregister () !=0 ) { +printf ("USB not stopped: usbkbd still using USB\n" ); +return 1 ; + } +#endif +return 0 ; +}/****************************************************************************** * usb command intepreter
@@ -450,6 +460,8 @@ static int do_usb(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])if ((strncmp (argv[1 ],"reset" ,5 ) ==0 ) || (strncmp (argv[1 ],"start" ,5 ) ==0 )) {bootstage_mark_name (BOOTSTAGE_ID_USB_START ,"usb_start" ); +if (do_usb_stop_keyboard () !=0 ) +return 1 ;usb_stop ();printf ("(Re)start USB...\n" );if (usb_init () >=0 ) {
@@ -468,19 +480,10 @@ static int do_usb(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])return 0 ; }if (strncmp (argv[1 ],"stop" ,4 ) ==0 ) { -#ifdef CONFIG_USB_KEYBOARD -if (argc ==2 ) { -if (usb_kbd_deregister () !=0 ) { -printf ("USB not stopped: usbkbd still" -" using USB\n" ); -return 1 ; - } - }else { -/* forced stop, switch console in to serial */ +if (argc !=2 )console_assign (stdin,"serial" ); -usb_kbd_deregister (); - } -#endif +if (do_usb_stop_keyboard () !=0 ) +return 1 ;printf ("stopping USB..\n" );usb_stop ();return 0 ;
Signature · 1 line
2.1.0
PATCH 2/5 usb: kbd: On a "usb reset" call usb_kbd_deregister() before calling usb_stop()
Quoted 27 lines · Hans de Goede
We need to call
usb_kbd_deregister()before callingusb_stop().usbkbd's
stdio_dev->privpoints to theusb_device, andusb_kbd_testcdereferencesusb_device->privptr.
usb_stopzerosusb_device, leavingusb_device->privptrNULL, causing bad things (tm) to happen once control returns to the main loop andusb_kbd_testcgets called.Calling
usb_kbd_deregister()avoids this. Note that we do not allow the "usb reset" to continue when the deregister fails. This will be fixed in a later patch.For the same reasons always fail "usb stop" if the
usb_kbd_deregister()fails, even in the force path. This can happen whenCONFIG_SYS_STDIO_DEREGISTERis not set.Signed-off-by Hans de Goede <[email protected]>--- common/cmd_usb.c | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-)common/cmd_usb.c +0
-0
common/cmd_usb.c static int do_usbboot(cmd_tbl_t *cmdtp, int flag, int · line 437
Quoted 8 lines
common/cmd_usb.c +5
-0@@ -430,6 +430,16 @@ static int do_usbboot(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) }#endif /* CONFIG_USB_STORAGE */ +static int do_usb_stop_keyboard (void ) +{ +#ifdef CONFIG_USB_KEYBOARD +if (usb_kbd_deregister () !=0 ) { +printf ("USB not stopped: usbkbd still using USB\n" );
I tried this set of patches with the DWC2 driver on RPI just now and i get this message and 'usb start' fails and doesn't scan bus. I suspect we have a problem here, can you check and post a fix ?
Best regards,
Marek Vasut
PATCH 2/5 usb: kbd: On a "usb reset" call usb_kbd_deregister() before calling usb_stop()
Hi,
Quoted 31 lines · Marek Vasut
Quoted 30 lines · Hans de Goede
We need to call
usb_kbd_deregister()before callingusb_stop().usbkbd's
stdio_dev->privpoints to theusb_device, andusb_kbd_testcdereferencesusb_device->privptr.
usb_stopzerosusb_device, leavingusb_device->privptrNULL, causing bad things (tm) to happen once control returns to the main loop andusb_kbd_testcgets called.Calling
usb_kbd_deregister()avoids this. Note that we do not allow the "usb reset" to continue when the deregister fails. This will be fixed in a later patch.For the same reasons always fail "usb stop" if the
usb_kbd_deregister()fails, even in the force path. This can happen whenCONFIG_SYS_STDIO_DEREGISTERis not set.Signed-off-by Hans de Goede <[email protected]>--- common/cmd_usb.c | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-)common/cmd_usb.c +0
-0@@ -430,6 +430,16 @@ static int do_usbboot(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) } #endif /* CONFIG_USB_STORAGE */
Quoted 9 lines
common/cmd_usb.c static int do_usbboot(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) · line 437
Quoted 5 lines
common/cmd_usb.c +5
-0+static int do_usb_stop_keyboard (void ) +{ +#ifdef CONFIG_USB_KEYBOARD +if (usb_kbd_deregister () !=0 ) { +printf ("USB not stopped: usbkbd still using USB\n" );I tried this set of patches with the DWC2 driver on RPI just now and i get this message and 'usb start' fails and doesn't scan bus. I suspect we have a problem here, can you check and post a fix ?
Do you have the entire set applied ? This is a known regression as mentioned in the commit msg. Once the force parameter is in place this should go away for a start/reset.
Also is usb start being called twice ? The first deregister should never fail, since then no device is registered.
Regards,
Hans
PATCH 2/5 usb: kbd: On a "usb reset" call usb_kbd_deregister() before calling usb_stop()
Quoted 46 lines · Hans de Goede
Hi,
Quoted 33 lines · Marek Vasut
Quoted 32 lines · Hans de Goede
We need to call
usb_kbd_deregister()before callingusb_stop().usbkbd's
stdio_dev->privpoints to theusb_device, andusb_kbd_testcdereferencesusb_device->privptr.
usb_stopzerosusb_device, leavingusb_device->privptrNULL, causing bad things (tm) to happen once control returns to the main loop andusb_kbd_testcgets called.Calling
usb_kbd_deregister()avoids this. Note that we do not allow the "usb reset" to continue when the deregister fails. This will be fixed in a later patch.For the same reasons always fail "usb stop" if the
usb_kbd_deregister()fails, even in the force path. This can happen whenCONFIG_SYS_STDIO_DEREGISTERis not set.Signed-off-by Hans de Goede <[email protected]>---
common/cmd_usb.c | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-)common/cmd_usb.c +0
-0@@ -430,6 +430,16 @@ static int do_usbboot(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) } #endif /* CONFIG_USB_STORAGE */Quoted 9 lines
common/cmd_usb.c static int do_usbboot(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) · line 437
Quoted 5 lines
common/cmd_usb.c +5
-0+static int do_usb_stop_keyboard (void ) +{ +#ifdef CONFIG_USB_KEYBOARD +if (usb_kbd_deregister () !=0 ) { +printf ("USB not stopped: usbkbd still using USB\n" );I tried this set of patches with the DWC2 driver on RPI just now and i get this message and 'usb start' fails and doesn't scan bus. I suspect we have a problem here, can you check and post a fix ?
Quoted 1 line
Do you have the entire set applied ?
Yes, the current u-boot-usb/master
Quoted 3 lines
This is a known regression as
mentioned in the commit msg. Once the force parameter is in place this should go away for a start/reset.
You mean the 'force' param ?
Quoted 2 lines
Also is usb start being called twice ? The first deregister should never fail, since then no device is registered.
Ugh, I must be dumb or somesuch, for whatever reason the issue went away and I can no longer replicate it. I will keep on a lookout.
Best regards,
Marek Vasut
PATCH 2/5 usb: kbd: On a "usb reset" call usb_kbd_deregister() before calling usb_stop()
Hi,
Quoted 49 lines · Marek Vasut
Quoted 46 lines · Hans de Goede
Hi,
Quoted 33 lines · Marek Vasut
Quoted 32 lines · Hans de Goede
We need to call
usb_kbd_deregister()before callingusb_stop().usbkbd's
stdio_dev->privpoints to theusb_device, andusb_kbd_testcdereferencesusb_device->privptr.
usb_stopzerosusb_device, leavingusb_device->privptrNULL, causing bad things (tm) to happen once control returns to the main loop andusb_kbd_testcgets called.Calling
usb_kbd_deregister()avoids this. Note that we do not allow the "usb reset" to continue when the deregister fails. This will be fixed in a later patch.For the same reasons always fail "usb stop" if the
usb_kbd_deregister()fails, even in the force path. This can happen whenCONFIG_SYS_STDIO_DEREGISTERis not set.Signed-off-by Hans de Goede <[email protected]>---
common/cmd_usb.c | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-)common/cmd_usb.c +0
-0@@ -430,6 +430,16 @@ static int do_usbboot(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) } #endif /* CONFIG_USB_STORAGE */Quoted 9 lines
common/cmd_usb.c static int do_usbboot(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) · line 437
Quoted 5 lines
common/cmd_usb.c +5
-0+static int do_usb_stop_keyboard (void ) +{ +#ifdef CONFIG_USB_KEYBOARD +if (usb_kbd_deregister () !=0 ) { +printf ("USB not stopped: usbkbd still using USB\n" );I tried this set of patches with the DWC2 driver on RPI just now and i get this message and 'usb start' fails and doesn't scan bus. I suspect we have a problem here, can you check and post a fix ?
Quoted 1 line
Do you have the entire set applied ?
Quoted 7 lines
Yes, the current u-boot-usb/master
Quoted 3 lines
This is a known regression as
mentioned in the commit msg. Once the force parameter is in place this should go away for a start/reset.You mean the 'force' param ?
Yes, when that is 1, which it is on a start / reset, the deregister should never fail.
Quoted 5 lines
Quoted 2 lines
Also is usb start being called twice ? The first deregister should never fail, since then no device is registered.
Ugh, I must be dumb or somesuch, for whatever reason the issue went away and I can no longer replicate it. I will keep on a lookout.
Ok.
Regards,
Hans
PATCH 3/5 usb: kbd: Remove check for already being registered
In reply to Hans de GoedeWe now always properly deregister the keyboard before calling drv_usb_kbd_init(), so we can drop the check for already being registered.
--- common/usb_kbd.c | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-)
common/usb_kbd.c +1 -11
@@ -490,7 +490,7 @@ static int usb_kbd_probe(struct usb_device *dev, unsigned int ifnum)/* Search for keyboard and register it if found. */ int drv_usb_kbd_init (void ) { -struct stdio_dev usb_kbd_dev, *old_dev; +struct stdio_dev usb_kbd_dev;struct usb_device *dev;char *stdinname =getenv ("stdin" );int error, i;
@@ -509,16 +509,6 @@ int drv_usb_kbd_init(void)if (usb_kbd_probe (dev,0 ) !=1 )continue ; -/* We found a keyboard, check if it is already registered. */ -debug ("USB KBD: found set up device.\n" ); - old_dev =stdio_get_by_name (DEVNAME ); -if (old_dev) { -/* Already registered, just return ok. */ -debug ("USB KBD: is already registered.\n" ); -usb_kbd_deregister (); -return 1 ; - } -/* Register the keyboard */ debug ("USB KBD: register.\n" );memset (&usb_kbd_dev,0 ,sizeof (struct stdio_dev ));
Signature · 1 line
2.1.0
PATCH 4/5 stdio: Add force parameter to stdio_deregister
In reply to Hans de GoedeIn some cases we really want to move forward with a deregister, add a force parameter to allow this, and replace the dev with a nulldev in this case.
--- common/stdio.c | 13 ++++++++++--- common/usb_kbd.c | 2 +- drivers/serial/serial-uclass.c | 2 +- include/stdio_dev.h | 4 ++-- 4 files changed, 14 insertions(+), 7 deletions(-)
common/stdio.c +10 -3
@@ -34,6 +34,9 @@ char *stdio_names[MAX_FILES] = { "stdin", "stdout", "stderr" };#define CONFIG_SYS_DEVICE_NULLDEV 1#endif +#ifdef CONFIG_SYS_STDIO_DEREGISTER +#define CONFIG_SYS_DEVICE_NULLDEV 1 +#endif #ifdef CONFIG_SYS_DEVICE_NULLDEV void nulldev_putc (struct stdio_dev *dev,const char c)
@@ -172,7 +175,7 @@ int stdio_register(struct stdio_dev *dev)* returns 0 if success, -1 if device is assigned and 1 if devname not found */ #ifdef CONFIG_SYS_STDIO_DEREGISTER -int stdio_deregister_dev (struct stdio_dev *dev) +int stdio_deregister_dev (struct stdio_dev *dev,int force) {int l;struct list_head *pos;
@@ -181,6 +184,10 @@ int stdio_deregister_dev(struct stdio_dev *dev)/* get stdio devices (ListRemoveItem changes the dev list) */ for (l=0 ; l<MAX_FILES ; l++) {if (stdio_devices[l] == dev) { +if (force) { +strcpy (temp_names[l],"nulldev" ); +continue ; + }/* Device is assigned -> report error */ return -1 ; }
@@ -202,7 +209,7 @@ int stdio_deregister_dev(struct stdio_dev *dev)return 0 ; } -int stdio_deregister (const char *devname) +int stdio_deregister (const char *devname,int force) {struct stdio_dev *dev;
@@ -211,7 +218,7 @@ int stdio_deregister(const char *devname)if (!dev)/* device not found */ return -ENODEV ; - return stdio_deregister_dev(dev); + return stdio_deregister_dev(dev, force); }#endif /* CONFIG_SYS_STDIO_DEREGISTER */
common/usb_kbd.c +1 -1
@@ -550,7 +550,7 @@ int drv_usb_kbd_init(void)int usb_kbd_deregister (void ) {#ifdef CONFIG_SYS_STDIO_DEREGISTER -int ret =stdio_deregister (DEVNAME ); +int ret =stdio_deregister (DEVNAME ,0 );if (ret && ret != -ENODEV )return ret;
drivers/serial/serial-uclass.c +1 -1
@@ -197,7 +197,7 @@ static int serial_pre_remove(struct udevice *dev)#ifdef CONFIG_SYS_STDIO_DEREGISTER struct serial_dev_priv *upriv = dev->uclass_priv; -if (stdio_deregister_dev (upriv->sdev)) +if (stdio_deregister_dev (upriv->sdev),0 )return -EPERM ;#endif
include/stdio_dev.h +2 -2
@@ -103,8 +103,8 @@ int stdio_init(void);void stdio_print_current_devices (void );#ifdef CONFIG_SYS_STDIO_DEREGISTER -int stdio_deregister (const char *devname); -int stdio_deregister_dev (struct stdio_dev *dev); +int stdio_deregister (const char *devname,int force); +int stdio_deregister_dev (struct stdio_dev *dev,int force);#endif struct list_head *stdio_get_list (void );struct stdio_dev *stdio_get_by_name (const char * name);
Signature · 1 line
2.1.0
PATCH 4/5 stdio: Add force parameter to stdio_deregister
Dear Hans de Goede,
Typographical error here:
drivers/serial/serial-uclass.c
Quoted 3 lines
drivers/serial/serial-uclass.c +0
-0<at> <at> -197,7 +197,7 <at> <at> static int serial_pre_remove(struct
udevice *dev)
drivers/serial/serial-uclass.c static int serial_pre_remove(struct udevice *dev) · line 200
Quoted 5 lines
#ifdef
CONFIG_SYS_STDIO_DEREGISTERdrivers/serial/serial-uclass.c +1
-1struct serial_dev_priv *upriv = dev->uclass_priv; -if (stdio_deregister_dev (upriv->sdev)) +if (stdio_deregister_dev (upriv->sdev),0 )
Breaks sandbox build, and probably others.
All the best,
RgC
Quoted 5 lines · Hans de Goede <hdegoede <at> redhat.com> writes:
In some cases we really want to move forward with a deregister, add a force parameter to allow this, and replace the dev with a nulldev in this case.
Signed-off-by Hans de Goede <hdegoede <at> redhat.com>
PATCH 4/5 stdio: Add force parameter to stdio_deregister
In reply to Hans de GoedeHi,
Quoted 81 lines · Hans de Goede
In some cases we really want to move forward with a deregister, add a force parameter to allow this, and replace the dev with a nulldev in this case.
Signed-off-by Hans de Goede <[email protected]>--- common/stdio.c | 13 ++++++++++--- common/usb_kbd.c | 2 +- drivers/serial/serial-uclass.c | 2 +- include/stdio_dev.h | 4 ++-- 4 files changed, 14 insertions(+), 7 deletions(-)common/stdio.c +10
-3@@ -34,6 +34,9 @@ char *stdio_names[MAX_FILES] = { "stdin", "stdout", "stderr" };#define CONFIG_SYS_DEVICE_NULLDEV 1#endif +#ifdef CONFIG_SYS_STDIO_DEREGISTER +#define CONFIG_SYS_DEVICE_NULLDEV 1 +#endif #ifdef CONFIG_SYS_DEVICE_NULLDEV void nulldev_putc (struct stdio_dev *dev,const char c)@@ -172,7 +175,7 @@ int stdio_register(struct stdio_dev *dev)* returns 0 if success, -1 if device is assigned and 1 if devname not found */ #ifdef CONFIG_SYS_STDIO_DEREGISTER -int stdio_deregister_dev (struct stdio_dev *dev) +int stdio_deregister_dev (struct stdio_dev *dev,int force) {int l;struct list_head *pos;@@ -181,6 +184,10 @@ int stdio_deregister_dev(struct stdio_dev *dev)/* get stdio devices (ListRemoveItem changes the dev list) */ for (l=0 ; l<MAX_FILES ; l++) {if (stdio_devices[l] == dev) { +if (force) { +strcpy (temp_names[l],"nulldev" ); +continue ; + }/* Device is assigned -> report error */ return -1 ; }@@ -202,7 +209,7 @@ int stdio_deregister_dev(struct stdio_dev *dev)return 0 ; } -int stdio_deregister (const char *devname) +int stdio_deregister (const char *devname,int force) {struct stdio_dev *dev;@@ -211,7 +218,7 @@ int stdio_deregister(const char *devname)if (!dev)/* device not found */ return -ENODEV ; - return stdio_deregister_dev(dev); + return stdio_deregister_dev(dev, force); }#endif /* CONFIG_SYS_STDIO_DEREGISTER */ common/usb_kbd.c +1
-1@@ -550,7 +550,7 @@ int drv_usb_kbd_init(void)int usb_kbd_deregister (void ) {#ifdef CONFIG_SYS_STDIO_DEREGISTER -int ret =stdio_deregister (DEVNAME ); +int ret =stdio_deregister (DEVNAME ,0 );if (ret && ret != -ENODEV )return ret;drivers/serial/serial-uclass.c +0
-0
drivers/serial/serial-uclass.c static int serial_pre_remove(struct udevice *dev) · line 200
Quoted 5 lines
drivers/serial/serial-uclass.c +1
-1@@ -197,7 +197,7 @@ static int serial_pre_remove(struct udevice *dev)#ifdef CONFIG_SYS_STDIO_DEREGISTER struct serial_dev_priv *upriv = dev->uclass_priv; -if (stdio_deregister_dev (upriv->sdev)) +if (stdio_deregister_dev (upriv->sdev),0 )
That bracket seems to be in a strange place.
Quoted 21 lines
drivers/serial/serial-uclass.c +0
-0return -EPERM ;#endif include/stdio_dev.h +2
-3@@ -103,8 +103,8 @@ int stdio_init(void);void stdio_print_current_devices (void );#ifdef CONFIG_SYS_STDIO_DEREGISTER -int stdio_deregister (const char *devname); -int stdio_deregister_dev (struct stdio_dev *dev); +int stdio_deregister (const char *devname,int force); +int stdio_deregister_dev (struct stdio_dev *dev,int force);#endif struct list_head *stdio_get_list (void );struct stdio_dev *stdio_get_by_name (const char * name); --2.1.0
Quoted 4 lines
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Regards,
Simon
PATCH 4/5 stdio: Add force parameter to stdio_deregister
Quoted 8 lines · Simon Glass
Hi,
Quoted 5 lines · Hans de Goede
In some cases we really want to move forward with a deregister, add a force parameter to allow this, and replace the dev with a nulldev in this case.
Signed-off-by Hans de Goede <[email protected]>
[...]
Quoted 6 lines
Quoted 6 lines
(quoted diff) +0
-0
b/drivers/serial/serial-uclass.cindex d04104e..61cbdc6 100644drivers/serial/serial-uclass.c +0
-0@@ -197,7 +197,7 @@ static int serial_pre_remove(struct udevice *dev)
Quoted 8 lines
drivers/serial/serial-uclass.c static int serial_pre_remove(struct udevice *dev) · line 200
Quoted 6 lines
drivers/serial/serial-uclass.c +1
-1#ifdef CONFIG_SYS_STDIO_DEREGISTER struct serial_dev_priv *upriv = dev->uclass_priv; -if (stdio_deregister_dev (upriv->sdev)) +if (stdio_deregister_dev (upriv->sdev),0 )That bracket seems to be in a strange place.
Good find, thanks! I have two questions:
1) How come I did not notice this and my build didn't spit?
2) Can either of you guys please prepare a patch?
Best regards,
Marek Vasut
PATCH 4/5 stdio: Add force parameter to stdio_deregister
Hi Marek,
Quoted 23 lines · Marek Vasut
Quoted 8 lines · Simon Glass
Hi,
Quoted 5 lines · Hans de Goede
In some cases we really want to move forward with a deregister, add a force parameter to allow this, and replace the dev with a nulldev in this case.
Signed-off-by Hans de Goede <[email protected]>[...]
Quoted 10 lines
Quoted 10 lines
(quoted diff) +0
-0
b/drivers/serial/serial-uclass.cindex d04104e..61cbdc6 100644drivers/serial/serial-uclass.c +0
-0@@ -197,7 +197,7 @@ static int serial_pre_remove(struct udevice *dev)#ifdef CONFIG_SYS_STDIO_DEREGISTER struct serial_dev_priv *upriv = dev->uclass_priv;
Quoted 7 lines
Quoted 4 lines
drivers/serial/serial-uclass.c static int serial_pre_remove(struct udevice *dev) · line 200
Quoted 2 lines
drivers/serial/serial-uclass.c +1
-1-if (stdio_deregister_dev (upriv->sdev)) +if (stdio_deregister_dev (upriv->sdev),0 )That bracket seems to be in a strange place.
Good find, thanks! I have two questions:
1) How come I did not notice this and my build didn't spit?
If you have CONFIG_SYS_STDIO_DEREGISTER, CONFIG_DM and CONFIG_DM_SERIAL set then I'm not sure. I made sure that sandbox has all of these but it might be the only board.
Quoted 5 lines
2) Can either of you guys please prepare a patch?
Best regards,
Marek Vasut
Regards,
Simon
PATCH 4/5 stdio: Add force parameter to stdio_deregister
Quoted 30 lines · Simon Glass
Hi Marek,
Quoted 27 lines · Marek Vasut
Quoted 8 lines · Simon Glass
Hi,
Quoted 5 lines · Hans de Goede
In some cases we really want to move forward with a deregister, add a force parameter to allow this, and replace the dev with a nulldev in this case.
Signed-off-by Hans de Goede <[email protected]>[...]
Quoted 14 lines
drivers/serial/serial-uclass.c static int serial_pre_remove(struct udevice *dev) · line 202
Quoted 12 lines
(quoted diff) +0
-0
b/drivers/serial/serial-uclass.cindex d04104e..61cbdc6 100644drivers/serial/serial-uclass.c +1
-1@@ -197,7 +197,7 @@ static int serial_pre_remove(struct udevice *dev)#ifdef CONFIG_SYS_STDIO_DEREGISTER struct serial_dev_priv *upriv = dev->uclass_priv; -if (stdio_deregister_dev (upriv->sdev)) +if (stdio_deregister_dev (upriv->sdev),0 )That bracket seems to be in a strange place.
Quoted 6 lines
Quoted 2 lines
Good find, thanks! I have two questions:
1) How come I did not notice this and my build didn't spit?If you have
CONFIG_SYS_STDIO_DEREGISTER,CONFIG_DMandCONFIG_DM_SERIALset then I'm not sure. I made sure that sandbox has all of these but it might be the only board.
I see, error on my end then. I will start building sandbox for the USB tree. Thank you for pointing this out! This also stresses my point that U-Boot project does need a proper CI (which we could have had thanks to Vadim, but we didn't persudate that, dang again).
I think this CI stuff should be added to the agenda of the U-Boot minisummit discussion.
Another point to the CI discussion could be that we could prepare a docker image with all the toolchains preinstalled, so one can run buildman easily in a well defined environment on his/her own.
Best regards,
Marek Vasut
PATCH 4/5 stdio: Add force parameter to stdio_deregister
Hi Marek,
Quoted 34 lines · Marek Vasut
Quoted 33 lines · Simon Glass
Hi Marek,
Quoted 29 lines · Marek Vasut
Quoted 8 lines · Simon Glass
Hi,
Quoted 5 lines · Hans de Goede
In some cases we really want to move forward with a deregister, add a force parameter to allow this, and replace the dev with a nulldev in this case.
Signed-off-by Hans de Goede <[email protected]>[...]
Quoted 6 lines
Quoted 6 lines
(quoted diff) +0
-0
b/drivers/serial/serial-uclass.cindex d04104e..61cbdc6 100644drivers/serial/serial-uclass.c +0
-0@@ -197,7 +197,7 @@ static int serial_pre_remove(struct udevice *dev)Quoted 8 lines
drivers/serial/serial-uclass.c static int serial_pre_remove(struct udevice *dev) · line 200
Quoted 6 lines
drivers/serial/serial-uclass.c +1
-1#ifdef CONFIG_SYS_STDIO_DEREGISTER struct serial_dev_priv *upriv = dev->uclass_priv; -if (stdio_deregister_dev (upriv->sdev)) +if (stdio_deregister_dev (upriv->sdev),0 )That bracket seems to be in a strange place.
Good find, thanks! I have two questions:
1) How come I did not notice this and my build didn't spit?
Quoted 8 lines
Quoted 3 lines
If you have
CONFIG_SYS_STDIO_DEREGISTER,CONFIG_DMandCONFIG_DM_SERIALset then I'm not sure. I made sure that sandbox has all of these but it might be the only board.I see, error on my end then. I will start building sandbox for the USB tree. Thank you for pointing this out! This also stresses my point that U-Boot project does need a proper CI (which we could have had thanks to Vadim, but we didn't persudate that, dang again).
What is a Cl? Do you mean his gerrit code review stuff?
Quoted 10 lines
I think this CI stuff should be added to the agenda of the U-Boot minisummit discussion.
Another point to the CI discussion could be that we could prepare a docker image with all the toolchains preinstalled, so one can run buildman easily in a well defined environment on his/her own.
Best regards,
Marek Vasut
Regards,
Simon
PATCH 4/5 stdio: Add force parameter to stdio_deregister
Quoted 42 lines · Simon Glass
Hi Marek,
Quoted 39 lines · Marek Vasut
Quoted 37 lines · Simon Glass
Hi Marek,
Quoted 23 lines · Marek Vasut
Quoted 8 lines · Simon Glass
Hi,
Quoted 5 lines · Hans de Goede
In some cases we really want to move forward with a deregister, add a force parameter to allow this, and replace the dev with a nulldev in this case.
Signed-off-by Hans de Goede <[email protected]>[...]
Quoted 11 lines
Quoted 11 lines
(quoted diff) +0
-0
b/drivers/serial/serial-uclass.cindex d04104e..61cbdc6 100644drivers/serial/serial-uclass.c +0
-0@@ -197,7 +197,7 @@ static int serial_pre_remove(struct udevice *dev)#ifdef CONFIG_SYS_STDIO_DEREGISTER struct serial_dev_priv *upriv = dev->uclass_priv;Quoted 7 lines
Quoted 4 lines
drivers/serial/serial-uclass.c static int serial_pre_remove(struct udevice *dev) · line 200
Quoted 2 lines
drivers/serial/serial-uclass.c +1
-1-if (stdio_deregister_dev (upriv->sdev)) +if (stdio_deregister_dev (upriv->sdev),0 )That bracket seems to be in a strange place.
Good find, thanks! I have two questions:
1) How come I did not notice this and my build didn't spit?If you have
CONFIG_SYS_STDIO_DEREGISTER,CONFIG_DMandCONFIG_DM_SERIALset then I'm not sure. I made sure that sandbox has all of these but it might be the only board.
Quoted 6 lines
Quoted 4 lines
I see, error on my end then. I will start building sandbox for the USB tree. Thank you for pointing this out! This also stresses my point that U-Boot project does need a proper CI (which we could have had thanks to Vadim, but we didn't persudate that, dang again).
What is a Cl? Do you mean his gerrit code review stuff?
I mean more continuous integration (build testing) of the code before a PR is submitted to the ML. Right now, we all do our own thing when it comes to testing before PR, but it would be nice to have one easy way of doing the build testing before submitting the PR, don't you think ? This might apply to Linux too.
Best regards,
Marek Vasut
PATCH 4/5 stdio: Add force parameter to stdio_deregister
Hi Marek,
Quoted 49 lines · Marek Vasut
Quoted 11 lines · Simon Glass
Hi Marek,
Quoted 8 lines · Marek Vasut
Quoted 7 lines · Simon Glass
Hi Marek,
Quoted 4 lines · Marek Vasut
Quoted 3 lines · Simon Glass
Hi,
On 20 September 2014 08:54, Hans de Goede <[email protected]>
Quoted 36 lines · wrote:
Quoted 35 lines
Quoted 24 lines
Quoted 24 lines
Quoted 5 lines
Quoted 5 lines
In some cases we really want to move forward with a deregister, add a force parameter to allow this, and replace the dev with a nulldev in this case.
Signed-off-by Hans de Goede <[email protected]>[...]
Quoted 15 lines
drivers/serial/serial-uclass.c static int serial_pre_remove(struct udevice · line 203
Quoted 13 lines
(quoted diff) +0
-0
b/drivers/serial/serial-uclass.cindex d04104e..61cbdc6 100644drivers/serial/serial-uclass.c +1
-1@@ -197,7 +197,7 @@ static int serial_pre_remove(struct udevice *dev)#ifdef CONFIG_SYS_STDIO_DEREGISTER struct serial_dev_priv *upriv = dev->uclass_priv; -if (stdio_deregister_dev (upriv->sdev)) +if (stdio_deregister_dev (upriv->sdev),0 )That bracket seems to be in a strange place.
Quoted 6 lines
Quoted 2 lines
Good find, thanks! I have two questions:
1) How come I did not notice this and my build didn't spit?If you have
CONFIG_SYS_STDIO_DEREGISTER,CONFIG_DMandCONFIG_DM_SERIALset then I'm not sure. I made sure that sandbox has all of these but it might be the only board.I see, error on my end then. I will start building sandbox for the USB tree. Thank you for pointing this out! This also stresses my point that U-Boot project does need a proper CI (which we could have had thanks to Vadim, but we didn't persudate that, dang again).
Quoted 8 lines
Quoted 1 line
What is a Cl? Do you mean his gerrit code review stuff?
I mean more continuous integration (build testing) of the code before a PR is submitted to the ML. Right now, we all do our own thing when it comes to testing before PR, but it would be nice to have one easy way of doing the build testing before submitting the PR, don't you think ? This might apply to Linux too.
Sure it would be useful. Before submitting my pull request I get all the patches in a branch and run:
./tools/buildman/buildman -b x86-push
This checks every commit for every board that I build, and gives me good confidence that no patch introduces new breakages.
Regards,
Simon
PATCH 4/5 stdio: Add force parameter to stdio_deregister
Quoted 1 line · Simon Glass
Hi Marek,
Hi Simon,
[..]
Quoted 6 lines
Quoted 5 lines
I mean more continuous integration (build testing) of the code before a PR is submitted to the ML. Right now, we all do our own thing when it comes to testing before PR, but it would be nice to have one easy way of doing the build testing before submitting the PR, don't you think ? This might apply to Linux too.
Quoted 7 lines
Sure it would be useful. Before submitting my pull request I get all the patches in a branch and run:
./tools/buildman/buildman-b x86-pushThis checks every commit for every board that I build, and gives me good confidence that no patch introduces new breakages.
I agree that buildman solves the CI part nicely, but we also have the part where one has to install the myriad of toolchains for all the architectures to be able to do the compile testing. I wonder if this cannot be made easy in some way -- maybe through a re-usable docker image with all the parts in it already.
Best regards,
Marek Vasut
PATCH 4/5 stdio: Add force parameter to stdio_deregister
Hi Marek,
Quoted 18 lines · Marek Vasut
Quoted 1 line · Simon Glass
Hi Marek,
Hi Simon,
[..]
Quoted 11 lines
Quoted 5 lines
I mean more continuous integration (build testing) of the code before a PR is submitted to the ML. Right now, we all do our own thing when it comes to testing before PR, but it would be nice to have one easy way of doing the build testing before submitting the PR, don't you think ? This might apply to Linux too.
Sure it would be useful. Before submitting my pull request I get all the patches in a branch and run:
./tools/buildman/buildman-b x86-push
Quoted 8 lines
Quoted 2 lines
This checks every commit for every board that I build, and gives me good confidence that no patch introduces new breakages.
I agree that buildman solves the CI part nicely, but we also have the part where one has to install the myriad of toolchains for all the architectures to be able to do the compile testing. I wonder if this cannot be made easy in some way -- maybe through a re-usable docker image with all the parts in it already.
It would be create if we could download all the toolchains from one place. Maybe we need a toolchain maintainer?
Regards,
Simon
PATCH 4/5 stdio: Add force parameter to stdio_deregister
Quoted 24 lines · Simon Glass
Hi Marek,
Quoted 21 lines · Marek Vasut
Quoted 1 line · Simon Glass
Hi Marek,
Hi Simon,
[..]
Quoted 13 lines
Quoted 5 lines
I mean more continuous integration (build testing) of the code before a PR is submitted to the ML. Right now, we all do our own thing when it comes to testing before PR, but it would be nice to have one easy way of doing the build testing before submitting the PR, don't you think ? This might apply to Linux too.
Sure it would be useful. Before submitting my pull request I get all the patches in a branch and run:
./tools/buildman/buildman-b x86-pushThis checks every commit for every board that I build, and gives me good confidence that no patch introduces new breakages.
Quoted 8 lines
Quoted 5 lines
I agree that buildman solves the CI part nicely, but we also have the part where one has to install the myriad of toolchains for all the architectures to be able to do the compile testing. I wonder if this cannot be made easy in some way -- maybe through a re-usable docker image with all the parts in it already.
It would be create if we could download all the toolchains from one place. Maybe we need a toolchain maintainer?
What about [1], this is where we can source the more exotic toolchains from, can we not? I think it was even you who pointed me to this site and it really is a nice one ;-)
[1] https://www.kernel.org/pub/tools/crosstool/
Best regards,
Marek Vasut
PATCH 4/5 stdio: Add force parameter to stdio_deregister
Quoted 5 lines · Marek Vasut
What about [1], this is where we can source the more exotic toolchains from, can we not? I think it was even you who pointed me to this site and it really is a nice one ;-)
Exactly, the https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
script pulls the toolchains from this same location ;-)
PATCH 4/5 stdio: Add force parameter to stdio_deregister
Hi,
Quoted 10 lines · Fabio Estevam
Quoted 5 lines · Marek Vasut
What about [1], this is where we can source the more exotic toolchains from, can we not? I think it was even you who pointed me to this site and it really is a nice one ;-)
Exactly, the https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
script pulls the toolchains from this same location ;-)
OK I see. So all we need is someone to add an option to buildman to pull them down and put them somewhere?
Regards,
Simon
PATCH 4/5 stdio: Add force parameter to stdio_deregister
Quoted 1 line · Simon Glass
Hi,
Hi,
Quoted 6 lines · Fabio Estevam
Quoted 5 lines · Marek Vasut
Quoted 4 lines
What about [1], this is where we can source the more exotic toolchains from, can we not? I think it was even you who pointed me to this site and it really is a nice one ;-)
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
Exactly, the
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbi
n/make.cross script pulls the toolchains from this same location ;-)OK I see. So all we need is someone to add an option to buildman to pull them down and put them somewhere?
That might just work.
Best regards,
Marek Vasut
PATCH 4/5 stdio: Add force parameter to stdio_deregister
In reply to Marek VasutHi Marek,
Quoted 5 lines · Marek Vasut
I agree that buildman solves the CI part nicely, but we also have the part where one has to install the myriad of toolchains for all the architectures to be able to do the compile testing. I wonder if this cannot be made easy in some way -- maybe through a re-usable docker image with all the parts in it already.
Maybe you could take a look at the make.cross script kbuild robot uses for cross compiling for lots of different architectures:
https://lists.01.org/pipermail/kbuild-all/2014-October/006965.html
PATCH 4/5 stdio: Add force parameter to stdio_deregister
Hi,
Quoted 10 lines · Fabio Estevam
Hi Marek,
Quoted 5 lines · Marek Vasut
I agree that buildman solves the CI part nicely, but we also have the part where one has to install the myriad of toolchains for all the architectures to be able to do the compile testing. I wonder if this cannot be made easy in some way -- maybe through a re-usable docker image with all the parts in it already.
Quoted 3 lines
Maybe you could take a look at the make.cross script kbuild robot uses for cross compiling for lots of different architectures:
https://lists.01.org/pipermail/kbuild-all/2014-October/006965.html
It's not really the builder that is the problem - we have one. It's all the different toolchains that no one knows about. I still can't build all the boards successfully.
Regards,
Simon
PATCH 4/5 stdio: Add force parameter to stdio_deregister
Quoted 3 lines · Simon Glass
It's not really the builder that is the problem - we have one. It's all the different toolchains that no one knows about. I still can't build all the boards successfully.
Aren't all the toolchains U-boot need included in the make.cross script?
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
It allows to build for all the archs that the kernel support.
Regards,
Fabio Estevam
PATCH 4/5 stdio: Add force parameter to stdio_deregister
In reply to Marek VasutQuoted 22 lines · Marek Vasut
Quoted 8 lines · Simon Glass
Hi,
Quoted 5 lines · Hans de Goede
In some cases we really want to move forward with a deregister, add a force parameter to allow this, and replace the dev with a nulldev in this case.
Signed-off-by Hans de Goede <[email protected]>[...]
Quoted 10 lines
Quoted 10 lines
(quoted diff) +0
-0
b/drivers/serial/serial-uclass.cindex d04104e..61cbdc6 100644drivers/serial/serial-uclass.c +0
-0@@ -197,7 +197,7 @@ static int serial_pre_remove(struct udevice *dev)#ifdef CONFIG_SYS_STDIO_DEREGISTER struct serial_dev_priv *upriv = dev->uclass_priv;
Quoted 7 lines
Quoted 4 lines
drivers/serial/serial-uclass.c static int serial_pre_remove(struct udevice *dev) · line 200
Quoted 2 lines
drivers/serial/serial-uclass.c +1
-1-if (stdio_deregister_dev (upriv->sdev)) +if (stdio_deregister_dev (upriv->sdev),0 )That bracket seems to be in a strange place.
Good find, thanks! I have two questions:
1) How come I did not notice this and my build didn't spit?
Because only sandbox uses this right now I think. But I'm unhappy that I can't seem to make repeated runs of:
$ ./tools/buildman/buildman -b master -c 1 -ve -T 1 -j 9 \ 'arc|blackfin|microblaze|m68k|nds32|sparc|x86|aarch64|sandbox|mips|avr32|arm|powerpc' $ ./tools/buildman/buildman -b master -c 1 -ve -T 1 -j 9 \ 'arc|blackfin|microblaze|m68k|nds32|sparc|x86|aarch64|sandbox|mips|avr32|arm|powerpc' \ -svel
Show me just new problems from the last time I did it. I must be doing something wrong, Simon?
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141009/0c76d601/attachment.pgp>
PATCH 4/5 stdio: Add force parameter to stdio_deregister
Hi Tom,
Quoted 33 lines · Tom Rini
Quoted 22 lines · Marek Vasut
Quoted 8 lines · Simon Glass
Hi,
Quoted 5 lines · Hans de Goede
In some cases we really want to move forward with a deregister, add a force parameter to allow this, and replace the dev with a nulldev in this case.
Signed-off-by Hans de Goede <[email protected]>[...]
Quoted 10 lines
Quoted 10 lines
(quoted diff) +0
-0
b/drivers/serial/serial-uclass.cindex d04104e..61cbdc6 100644drivers/serial/serial-uclass.c +0
-0@@ -197,7 +197,7 @@ static int serial_pre_remove(struct udevice *dev)#ifdef CONFIG_SYS_STDIO_DEREGISTER struct serial_dev_priv *upriv = dev->uclass_priv;Quoted 7 lines
Quoted 4 lines
drivers/serial/serial-uclass.c static int serial_pre_remove(struct udevice *dev) · line 200
Quoted 2 lines
drivers/serial/serial-uclass.c +1
-1-if (stdio_deregister_dev (upriv->sdev)) +if (stdio_deregister_dev (upriv->sdev),0 )That bracket seems to be in a strange place.
Good find, thanks! I have two questions:
1) How come I did not notice this and my build didn't spit?Because only sandbox uses this right now I think. But I'm unhappy that I can't seem to make repeated runs of:
Quoted 8 lines
$ ./tools/buildman/buildman -b master -c 1 -ve -T 1 -j 9 \ 'arc|blackfin|microblaze|m68k|nds32|sparc|x86|aarch64|sandbox|mips|avr32|arm|powerpc' $ ./tools/buildman/buildman -b master -c 1 -ve -T 1 -j 9 \ 'arc|blackfin|microblaze|m68k|nds32|sparc|x86|aarch64|sandbox|mips|avr32|arm|powerpc' \ -svelShow me just new problems from the last time I did it. I must be doing something wrong, Simon?
I don't really see anything obviously wrong. But I'm not sure what you are expecting. This is going to just build the top commit in branch master, and if the commit hash has changed it will remove any previous results for that commit before starting the build. So you will always get a full list of errors, not a delta from last time. If you want that you could add a second commit with your fixes and not adjust the first commit in the branch (and use -c2).
If you leave off '-b master -c1' it would have about the same effect, assuming that you have 'master' checked out.
In the second line you are specifying -ve twice but that doesn't matter. Why are you changing the thread/jobs defaults? Does that speed it up? Also you don't need the quotes and the | between archs.
Also note there is --step to avoid building every commit. For example, this will build the upstream commit and your branch's top commit (only) assuming that master tracks upstream/master.
buildman -b master --step 0
Regards,
Simon
PATCH 4/5 stdio: Add force parameter to stdio_deregister
Quoted 45 lines · Simon Glass
Hi Tom,
Quoted 33 lines · Tom Rini
Quoted 22 lines · Marek Vasut
Quoted 8 lines · Simon Glass
Hi,
Quoted 5 lines · Hans de Goede
In some cases we really want to move forward with a deregister, add a force parameter to allow this, and replace the dev with a nulldev in this case.
Signed-off-by Hans de Goede <[email protected]>[...]
Quoted 10 lines
Quoted 10 lines
(quoted diff) +0
-0
b/drivers/serial/serial-uclass.cindex d04104e..61cbdc6 100644drivers/serial/serial-uclass.c +0
-0@@ -197,7 +197,7 @@ static int serial_pre_remove(struct udevice *dev)#ifdef CONFIG_SYS_STDIO_DEREGISTER struct serial_dev_priv *upriv = dev->uclass_priv;Quoted 7 lines
Quoted 4 lines
drivers/serial/serial-uclass.c static int serial_pre_remove(struct udevice *dev) · line 200
Quoted 2 lines
drivers/serial/serial-uclass.c +1
-1-if (stdio_deregister_dev (upriv->sdev)) +if (stdio_deregister_dev (upriv->sdev),0 )That bracket seems to be in a strange place.
Good find, thanks! I have two questions:
1) How come I did not notice this and my build didn't spit?Because only sandbox uses this right now I think. But I'm unhappy that I can't seem to make repeated runs of:
Quoted 8 lines
$ ./tools/buildman/buildman -b master -c 1 -ve -T 1 -j 9 \ 'arc|blackfin|microblaze|m68k|nds32|sparc|x86|aarch64|sandbox|mips|avr32|arm|powerpc' $ ./tools/buildman/buildman -b master -c 1 -ve -T 1 -j 9 \ 'arc|blackfin|microblaze|m68k|nds32|sparc|x86|aarch64|sandbox|mips|avr32|arm|powerpc' \ -svelShow me just new problems from the last time I did it. I must be doing something wrong, Simon?
Quoted 7 lines
I don't really see anything obviously wrong. But I'm not sure what you are expecting. This is going to just build the top commit in branch master, and if the commit hash has changed it will remove any previous results for that commit before starting the build. So you will always get a full list of errors, not a delta from last time. If you want that you could add a second commit with your fixes and not adjust the first commit in the branch (and use -c2).
Ah, OK, this got me going towards what I wanted. I was assuming for some incorrect reason that with -c1 it would just implicitly compare vs the last build it had available. I really want -c2 as I should have a full build from the last go-round (which is to say really, last merge). This is what I wanted (a simplified build):
trini at bill-the-cat:~/work/u-boot/u-boot (32d0192...)$ ./tools/buildman/buildman -b HEAD -c 2 -ve -T 1 -j 9 'sandbox|sparc' -svel
boards.cfg is up to date. Nothing to do.
Summary of 2 commits for 6 boards (1 thread, 9 jobs per thread) 01: usb: kbd: Remove check for already being registered
sparc: + grsim_leon2 gr_cpci_ax2000 gr_xc3s_1500 grsim gr_ep2s60
+(grsim_leon2,gr_cpci_ax2000,gr_xc3s_1500,grsim,gr_ep2s60) disk/part.c: In function `get_device':
+(grsim_leon2,gr_cpci_ax2000,gr_xc3s_1500,grsim,gr_ep2s60) disk/part.c:454: warning: 'hwpart' might be used uninitialized in this function
02: stdio: Add force parameter to stdio_deregister
sandbox: + sandbox
+(sandbox) drivers/serial/serial-uclass.c: In function ?serial_pre_remove?:
+(sandbox) drivers/serial/serial-uclass.c:201:2: error: too few arguments to function ?stdio_deregister_dev?
+(sandbox) include/stdio_dev.h:107:5: note: declared here +(sandbox) make[2]: *** [drivers/serial/serial-uclass.o] Error 1 +(sandbox) make[1]: *** [drivers/serial] Error 2
+(sandbox) make: *** [sub-make] Error 2
w+(sandbox) drivers/serial/serial-uclass.c:201:39: warning:
left-hand operand of comma expression has no effect [-Wunused-value]
And I don't care about the stuff around 01 (it was like that before), I care about 02 which is new problems.
Quoted 6 lines
If you leave off '-b master -c1' it would have about the same effect, assuming that you have 'master' checked out.
In the second line you are specifying -ve twice but that doesn't matter. Why are you changing the thread/jobs defaults? Does that speed it up?
We have some race condition still on very large machines that I haven't been able to track down. Doing it that way was less problematic I think.
Quoted 1 line
Also you don't need the quotes and the | between archs.
Oh, interesting.
Quoted 5 lines
Also note there is --step to avoid building every commit. For example, this will build the upstream commit and your branch's top commit (only) assuming that master tracks upstream/master.
buildman -b master --step 0
I think this is what I really wanted, yes.
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141009/bfd496c8/attachment.pgp>
PATCH 5/5 usb: kbd: Allow "usb reset" to continue when an usb kbd is used
In reply to Hans de GoedeUse the new force parameter to make the stdio_deregister succeed, replacing stdin with a nulldev, and assume that the usb keyboard will come back after the reset.
--- common/cmd_usb.c | 8 ++++---- common/usb_kbd.c | 4 ++-- include/usb.h | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-)
common/cmd_usb.c +4 -4
@@ -430,10 +430,10 @@ static int do_usbboot(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) }#endif /* CONFIG_USB_STORAGE */ -static int do_usb_stop_keyboard (void ) +static int do_usb_stop_keyboard (int force) {#ifdef CONFIG_USB_KEYBOARD -if (usb_kbd_deregister () !=0 ) { +if (usb_kbd_deregister (force) !=0 ) {printf ("USB not stopped: usbkbd still using USB\n" );return 1 ; }
@@ -460,7 +460,7 @@ static int do_usb(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])if ((strncmp (argv[1 ],"reset" ,5 ) ==0 ) || (strncmp (argv[1 ],"start" ,5 ) ==0 )) {bootstage_mark_name (BOOTSTAGE_ID_USB_START ,"usb_start" ); -if (do_usb_stop_keyboard () !=0 ) +if (do_usb_stop_keyboard (1 ) !=0 )return 1 ;usb_stop ();printf ("(Re)start USB...\n" );
@@ -482,7 +482,7 @@ static int do_usb(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])if (strncmp (argv[1 ],"stop" ,4 ) ==0 ) {if (argc !=2 )console_assign (stdin,"serial" ); -if (do_usb_stop_keyboard () !=0 ) +if (do_usb_stop_keyboard (0 ) !=0 )return 1 ;printf ("stopping USB..\n" );usb_stop ();
common/usb_kbd.c +2 -2
@@ -547,10 +547,10 @@ int drv_usb_kbd_init(void) }/* Deregister the keyboard. */ -int usb_kbd_deregister (void ) +int usb_kbd_deregister (int force) {#ifdef CONFIG_SYS_STDIO_DEREGISTER -int ret =stdio_deregister (DEVNAME ,0 ); +int ret =stdio_deregister (DEVNAME , force);if (ret && ret != -ENODEV )return ret;
include/usb.h +1 -1
@@ -216,7 +216,7 @@ int usb_host_eth_scan(int mode);#ifdef CONFIG_USB_KEYBOARD int drv_usb_kbd_init (void ); -int usb_kbd_deregister (void ); +int usb_kbd_deregister (int force);#endif /* routines */
Signature · 1 line
2.1.0
Marek Vasut
Appliedcover USB keyboard: don't crash on "usb reset"
In reply to Hans de GoedeQuoted 9 lines · Hans de Goede
Hi Marek,
Here is the second set of USB patches / fixes I've been working on, currently u-boot does "really bad things" (tm) when doing "usb reset" while using an usb keyboard. This set fixes this.
Given that these fix "really bad things" (tm) I not only believe that these are suitable to go into v2014.10, but I also believe that they actually should :)
Applied all, thanks a lot!
Best regards,
Marek Vasut