Re: [U-Boot] Antw: Re: U-Boot doesn't silent the output
Hi Wolfgang, On 18 March 2014 09:03, Wolfgang Denk w...@denx.de wrote: What exactly did you pass in the kernel command line - just console=, i. e. without a value? Did you try passing a valid device name instead, like console=null? Apologies for hijacking this thread. If I remember, fixup_silent_linux ensures that 'console=' is present in the kernel arguments (i.e. it will replace console=/dev/ttyS0 (or similar) with console=). I think preferred behavior for this may be to instead leave any 'console' arguments as they are and instead ensure that 'quiet' or 'loglevel=1' is present instead. There are two motivations for doing this - the first is that when using a lower loglevel you still get suppressed kernel output - but you also get any errors. Thus if something goes wrong you'll see why rather than wonder if U-Boot even started the kernel. The second is that I've seen a few times in the past where setting console to nothing (console=) results in strange behavior (it once increased boot time). After all we want a console we just don't want to use it as much. I can provide a patch for this if you think you may take it? Thanks, Andrew Murray ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Antw: Re: U-Boot doesn't silent the output
On 18 March 2014 11:30, Wolfgang Denk w...@denx.de wrote: Dear Andrew, In message capcvp5fnjo9y3mkhudgar1jx0tfhkexu06bbtg5nccsq3wb...@mail.gmail.com you wrote: If I remember, fixup_silent_linux ensures that 'console=' is present in the kernel arguments (i.e. it will replace console=/dev/ttyS0 (or similar) with console=). I think preferred behavior for this may be to instead leave any 'console' arguments as they are and instead ensure that 'quiet' or 'loglevel=1' is present instead. There are two motivations for doing this - the first is that when using a lower loglevel you still get suppressed kernel output - but you also get any errors. Thus if something goes wrong you'll see why rather than wonder if U-Boot even started the kernel. The second is that I've seen a few times in the past where setting console to nothing (console=) results in strange behavior (it once increased boot time). After all we want a console we just don't want to use it as much. I can provide a patch for this if you think you may take it? I have to admit that I don't know if this is a good idea. I do know that some users use this feature to make sure the console port is completely free, and no characters ever are sent to it, for example because they use it for application specific purposes. Of course one might ask if this is a good idea (IMO a separate console port is a very useful feature), but you know how some companies design their hardware... Yes I can understand why they would want to do that. However I would argue that using the 'silent' feature isn't the correct way to achieve it. If a user depends on having a console completely free then they probably shouldn't add a 'console=xyz' to their boot args in the first place. Thanks, Andrew Murray ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Antw: Re: U-Boot doesn't silent the output
On 18 March 2014 14:17, Wolfgang Denk w...@denx.de wrote: Dear Andrew, In message capcvp5ekxdb_y16sym7x2cln67hbg4jt8jkos7n8+fsd8dr...@mail.gmail.com you wrote: However I would argue that using the 'silent' feature isn't the correct way to achieve it. If a user depends on having a console completely free then they probably shouldn't add a 'console=xyz' to their boot args in the first place. Even if it is console=null ? If it is 'console=null' (which as you point out would be sensible), then the existing 'silent' feature will change that to 'console=' (which shouldn't ought to make a difference anyway). Where a user has 'console=null' in their bootargs, then my console would turn that into 'console=null loglevel=1' rather that the present which would be 'console='. Thanks, Andrew Murray ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 5/7] zynq: Add support for auto nandboot
On 17 February 2014 12:26, Siva Durga Prasad Paladugu siva.durga.palad...@xilinx.com wrote: From: Jagannadha Sutradharudu Teki jaga...@xilinx.com Added support to find the nand bootmode and also updated the default env. with nandboot. Signed-off-by: Siva Durga Prasad Paladugu siva...@xilinx.com Signed-off-by: Jagannadha Sutradharudu Teki jaga...@xilinx.com --- Changes for v3: -Separated out the nand patch series as per Michal comment. Changes for v2: -None --- board/xilinx/zynq/board.c |4 doc/README.zynq |3 ++- include/configs/zynq-common.h |3 +++ 3 files changed, 9 insertions(+), 1 deletions(-) diff --git a/board/xilinx/zynq/board.c b/board/xilinx/zynq/board.c index 5a47149..9dd158c 100644 --- a/board/xilinx/zynq/board.c +++ b/board/xilinx/zynq/board.c @@ -15,6 +15,7 @@ DECLARE_GLOBAL_DATA_PTR; /* Bootmode setting values */ #define ZYNQ_BM_MASK 0x0F #define ZYNQ_BM_NOR0x02 +#define ZYNQ_BM_NAND 0x04 #define ZYNQ_BM_SD 0x05 #define ZYNQ_BM_JTAG 0x0 @@ -75,6 +76,9 @@ int board_late_init(void) case ZYNQ_BM_NOR: setenv(modeboot, norboot); break; + case ZYNQ_BM_NAND: + setenv(modeboot, nandboot); + break; case ZYNQ_BM_SD: setenv(modeboot, sdboot); break; diff --git a/doc/README.zynq b/doc/README.zynq index 043c970..e0b38cf 100644 --- a/doc/README.zynq +++ b/doc/README.zynq @@ -54,10 +54,11 @@ is intern used in autoboot. SLCR bootmode register Bit[3:0] values #define ZYNQ_BM_NOR0x02 +#define ZYNQ_BM_NOR0x04 #define ZYNQ_BM_SD 0x05 #define ZYNQ_BM_JTAG 0x0 -modeboot variable can assign any of norboot, sdboot or jtagboot +modeboot variable can assign any of norboot, nandboot, sdboot or jtagboot bootmode strings at runtime. 5. Mainline status diff --git a/include/configs/zynq-common.h b/include/configs/zynq-common.h index 08adaa2..068e59a 100644 --- a/include/configs/zynq-common.h +++ b/include/configs/zynq-common.h @@ -170,6 +170,9 @@ norboot=echo Copying FIT from NOR flash to RAM... \ cp.b ${nor_flash_off} ${load_addr} ${fit_size} \ bootm ${load_addr}\0 \ + nandboot=echo Copying FIT from NAND flash to RAM... \ + nand read ${load_addr} ${flash_off} ${fit_size} \ + bootm ${load_addr}\0 \ Is there an opportunity to use 'nboot' here? i.e. nboot ${load_addr} 0 ${flash_off} (instead of nand read and bootm) This would mean you don't waste time reading NAND you don't use and you don't have to worry about maintaining ${fit_size} whenever the kernel changes size. Andrew Murray sdboot=echo Copying FIT from SD to RAM... \ fatload mmc 0 ${load_addr} ${fit_image} \ bootm ${load_addr}\0 \ -- 1.7.4 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] usb: Prevent using reserved registers on DM36x usb
On 30 September 2013 00:26, Marek Vasut ma...@denx.de wrote: Dear Andrew Murray, The musb driver defines and uses MUSB_CSR0_H_DIS_PING, however this bit is reserved on the DM36x. Thus this patch ensures that the reserved bit is not accesssed. It has been observed that some USB devices will fail to enumerate with errors such as 'error in inquiry' without this patch. See http://www.ti.com/litv/pdf/sprufh9a for details. Cc: Marek Vasut ma...@denx.de Cc: Tom Rini tr...@ti.com Signed-off-by: Andrew Murray amur...@embedded-bits.co.uk Tom, can you check this and if it's OK with you, pick this by hand? For my part, I'm fine here, Acked-by: Marek Vasut ma...@denx.de Tom, was this version OK? Andrew Murray ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] usb: Fix error handling in musb_hcd.c
The wait_until_[rx|tx]ep_ready functions return a u8 to indicate success containing the value 0, 1 or -1. This patch changes the return type to an int to accommodate the negative return values. These functions are used in the file using calls such as if (!wait_until... Where a -1 is returned it is mishandled and treated as success instead of a CRC error. This patch addresses this. Cc: Marek Vasut ma...@denx.de Cc: Tom Rini tr...@ti.com Signed-off-by: Andrew Murray amur...@embedded-bits.co.uk --- drivers/usb/musb/musb_hcd.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/usb/musb/musb_hcd.c b/drivers/usb/musb/musb_hcd.c index ae39c4a..708fa12 100644 --- a/drivers/usb/musb/musb_hcd.c +++ b/drivers/usb/musb/musb_hcd.c @@ -267,7 +267,7 @@ static int wait_until_ep0_ready(struct usb_device *dev, u32 bit_mask) /* * waits until tx ep is ready. Returns 1 when ep is ready and 0 on error. */ -static u8 wait_until_txep_ready(struct usb_device *dev, u8 ep) +static int wait_until_txep_ready(struct usb_device *dev, u8 ep) { u16 csr; int timeout = CONFIG_MUSB_TIMEOUT; @@ -299,7 +299,7 @@ static u8 wait_until_txep_ready(struct usb_device *dev, u8 ep) /* * waits until rx ep is ready. Returns 1 when ep is ready and 0 on error. */ -static u8 wait_until_rxep_ready(struct usb_device *dev, u8 ep) +static int wait_until_rxep_ready(struct usb_device *dev, u8 ep) { u16 csr; int timeout = CONFIG_MUSB_TIMEOUT; @@ -1018,7 +1018,7 @@ int submit_bulk_msg(struct usb_device *dev, unsigned long pipe, writew(csr | MUSB_TXCSR_TXPKTRDY, musbr-txcsr); /* Wait until the TxPktRdy bit is cleared */ - if (!wait_until_txep_ready(dev, MUSB_BULK_EP)) { + if (wait_until_txep_ready(dev, MUSB_BULK_EP) != 1) { readw(musbr-txcsr); usb_settoggle(dev, ep, dir_out, (csr MUSB_TXCSR_H_DATATOGGLE_SHIFT) 1); @@ -1053,7 +1053,7 @@ int submit_bulk_msg(struct usb_device *dev, unsigned long pipe, writew(csr | MUSB_RXCSR_H_REQPKT, musbr-rxcsr); /* Wait until the RxPktRdy bit is set */ - if (!wait_until_rxep_ready(dev, MUSB_BULK_EP)) { + if (wait_until_rxep_ready(dev, MUSB_BULK_EP) != 1) { csr = readw(musbr-rxcsr); usb_settoggle(dev, ep, dir_out, (csr MUSB_S_RXCSR_H_DATATOGGLE) 1); @@ -1226,7 +1226,7 @@ int submit_int_msg(struct usb_device *dev, unsigned long pipe, writew(csr | MUSB_RXCSR_H_REQPKT, musbr-rxcsr); /* Wait until the RxPktRdy bit is set */ - if (!wait_until_rxep_ready(dev, MUSB_INTR_EP)) { + if (wait_until_rxep_ready(dev, MUSB_INTR_EP) != 1) { csr = readw(musbr-rxcsr); usb_settoggle(dev, ep, dir_out, (csr MUSB_S_RXCSR_H_DATATOGGLE) 1); -- 1.7.9.5 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2] usb: Prevent using reserved registers on DM36x usb
The musb driver defines and uses MUSB_CSR0_H_DIS_PING, however this bit is reserved on the DM36x. Thus this patch ensures that the reserved bit is not accesssed. It has been observed that some USB devices will fail to enumerate with errors such as 'error in inquiry' without this patch. See http://www.ti.com/litv/pdf/sprufh9a for details. Cc: Marek Vasut ma...@denx.de Cc: Tom Rini tr...@ti.com Signed-off-by: Andrew Murray amur...@embedded-bits.co.uk --- drivers/usb/musb/musb_hcd.c | 19 ++- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/drivers/usb/musb/musb_hcd.c b/drivers/usb/musb/musb_hcd.c index 3dc5d6a..ae39c4a 100644 --- a/drivers/usb/musb/musb_hcd.c +++ b/drivers/usb/musb/musb_hcd.c @@ -417,8 +417,12 @@ static int ctrlreq_out_data_phase(struct usb_device *dev, u32 len, void *buffer) /* Set TXPKTRDY bit */ csr = readw(musbr-txcsr); - writew(csr | MUSB_CSR0_H_DIS_PING | MUSB_CSR0_TXPKTRDY, - musbr-txcsr); + + csr |= MUSB_CSR0_TXPKTRDY; +#if !defined(CONFIG_SOC_DM365) + csr |= MUSB_CSR0_H_DIS_PING; +#endif + writew(csr, musbr-txcsr); result = wait_until_ep0_ready(dev, MUSB_CSR0_TXPKTRDY); if (result 0) break; @@ -439,8 +443,10 @@ static int ctrlreq_out_status_phase(struct usb_device *dev) /* Set the StatusPkt bit */ csr = readw(musbr-txcsr); - csr |= (MUSB_CSR0_H_DIS_PING | MUSB_CSR0_TXPKTRDY | - MUSB_CSR0_H_STATUSPKT); + csr |= (MUSB_CSR0_TXPKTRDY | MUSB_CSR0_H_STATUSPKT); +#if !defined(CONFIG_SOC_DM365) + csr |= MUSB_CSR0_H_DIS_PING; +#endif writew(csr, musbr-txcsr); /* Wait until TXPKTRDY bit is cleared */ @@ -457,7 +463,10 @@ static int ctrlreq_in_status_phase(struct usb_device *dev) int result; /* Set the StatusPkt bit and ReqPkt bit */ - csr = MUSB_CSR0_H_DIS_PING | MUSB_CSR0_H_REQPKT | MUSB_CSR0_H_STATUSPKT; + csr = MUSB_CSR0_H_REQPKT | MUSB_CSR0_H_STATUSPKT; +#if !defined(CONFIG_SOC_DM365) + csr |= MUSB_CSR0_H_DIS_PING; +#endif writew(csr, musbr-txcsr); result = wait_until_ep0_ready(dev, MUSB_CSR0_H_REQPKT); -- 1.7.9.5 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH RFC] usb: Prevent using reserved registers on DM36x usb
On 27 September 2013 19:10, Marek Vasut ma...@denx.de wrote: It would be much nicer if you avoided using this bit in musb_hcd.c instead of hacking it like this. Bump? Will we get a V2 here ? I've posted a v2 on the list, apologies for the slow response. Andrew Murray ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH RFC] usb: Prevent using reserved registers on DM36x usb
The musb driver defines and uses MUSB_CSR0_H_DIS_PING, however this bit is reserved on the DM36x. Thus this patch ensures that the reserved bit is not accesssed. It has been observed that some USB devices will fail to enumerate with errors such as 'error in inquiry' without this patch. See http://www.ti.com/litv/pdf/sprufh9a for details. Cc: Marek Vasut ma...@denx.de Cc: Tom Rini tr...@ti.com Signed-off-by: Andrew Murray amur...@embedded-bits.co.uk --- drivers/usb/musb/musb_core.h |4 1 file changed, 4 insertions(+) diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h index ec8a038..c9a9d66 100644 --- a/drivers/usb/musb/musb_core.h +++ b/drivers/usb/musb/musb_core.h @@ -216,7 +216,11 @@ struct musb_regs { #define MUSB_CSR0_P_SENTSTALL 0x0004 /* CSR0 in Host mode */ +#if defined(CONFIG_SOC_DM365) +#define MUSB_CSR0_H_DIS_PING 0x +#else #define MUSB_CSR0_H_DIS_PING 0x0800 +#endif #define MUSB_CSR0_H_WR_DATATOGGLE 0x0400 /* Set to allow setting: */ #define MUSB_CSR0_H_DATATOGGLE 0x0200 /* Data toggle control */ #define MUSB_CSR0_H_NAKTIMEOUT 0x0080 -- 1.7.9.5 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Failing USB devices
On 30 August 2013 18:05, Andrew Murray amur...@embedded-bits.co.uk wrote: On 23 August 2013 04:23, Marek Vasut ma...@denx.de wrote: Dear Andrew Murray, Hello, I'm unable to use some mass storage devices with the current git version (6612ab33956ae09c5ba2fde9c1540b519625ba37) of UBoot on a TI Davinci custom board. I have 7 pen drives, and 2 of them fail. I-O DATA USB Flash Disk (0x04bb/0x0c43) ... 2 USB Device(s) found scan end scanning usb for storage devices... i=0 i=1 USB Mass Storage device detected Transport: Bulk/Bulk/Bulk Endpoints In 1 Out 2 Int 0 usb_control_msg: request: 0xFE, requesttype: 0xA1, value 0x0 index 0x0 length 0x1 Get Max LUN - len = 1, result = 0 address 2 COMMAND phase DATA phase STATUS phase !CSWSIGNATURE BBB_reset usb_control_msg: request: 0xFF, requesttype: 0x21, value 0x0 index 0x0 length 0x0 RESET:stall inquiry returns -1 COMMAND phase DATA phase STATUS phase !CSWSIGNATURE BBB_reset usb_control_msg: request: 0xFF, requesttype: 0x21, value 0x0 index 0x0 length 0x0 RESET:stall inquiry returns -1 COMMAND phase DATA phase STATUS phase !CSWSIGNATURE BBB_reset usb_control_msg: request: 0xFF, requesttype: 0x21, value 0x0 index 0x0 length 0x0 RESET:stall inquiry returns -1 COMMAND phase DATA phase STATUS phase !CSWSIGNATURE BBB_reset usb_control_msg: request: 0xFF, requesttype: 0x21, value 0x0 index 0x0 length 0x0 RESET:stall inquiry returns -1 COMMAND phase DATA phase STATUS phase !CSWSIGNATURE BBB_reset usb_control_msg: request: 0xFF, requesttype: 0x21, value 0x0 index 0x0 length 0x0 RESET:stall inquiry returns -1 error in inquiry i=2 0 Storage Device(s) found In this case putting a delay of 1000ms in usb_stor_BBB_transport between the COMMAND and DATA phase, (in place of the conditional 5ms USB_READY delay), overcomes this issue. It seems this 1000ms delay is only required for the first COMMAND, subsequent COMMAND phases seem happy with the 5ms delay. Lexar JumpDrive 32GB (0x0424/0x2507): So the device takes too long to init itself after powering up the port. It would be nice if you could check the spec and see if there is a way to query the device for ready condition. Although I remember we already do that. After further investigation I've learnt that removing references to MUSB_CSR0_H_DIS_PING in musb_hcd.c removes the need for a large delay between the COMMAND and STATUS phases, this allows more USB mass storage devices to work. MUSB_CSR0_H_DIS_PING sets bit 12 of txcsr, however on the DM36x (http://www.ti.com/litv/pdf/sprufh9a) this appears to be a reserved bit. In any case it appears to have an undesirable effect. The linux driver does appear to use this bit, yet these devices do work under Linux. I'll submit a patch if you feel this is incorrect, any suggestions for the best way to fix this? Hi Marek, As there has been no feedback on this, are you happy to accept a patch that conditionally sets MUSB_CSR0_H_DIS_PING in the header file to 0 when CONFIG_SOC_DM365 is defined? Thanks, Andrew Murray ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Failing USB devices
On 23 August 2013 04:23, Marek Vasut ma...@denx.de wrote: Dear Andrew Murray, Hello, I'm unable to use some mass storage devices with the current git version (6612ab33956ae09c5ba2fde9c1540b519625ba37) of UBoot on a TI Davinci custom board. I have 7 pen drives, and 2 of them fail. I-O DATA USB Flash Disk (0x04bb/0x0c43) ... 2 USB Device(s) found scan end scanning usb for storage devices... i=0 i=1 USB Mass Storage device detected Transport: Bulk/Bulk/Bulk Endpoints In 1 Out 2 Int 0 usb_control_msg: request: 0xFE, requesttype: 0xA1, value 0x0 index 0x0 length 0x1 Get Max LUN - len = 1, result = 0 address 2 COMMAND phase DATA phase STATUS phase !CSWSIGNATURE BBB_reset usb_control_msg: request: 0xFF, requesttype: 0x21, value 0x0 index 0x0 length 0x0 RESET:stall inquiry returns -1 COMMAND phase DATA phase STATUS phase !CSWSIGNATURE BBB_reset usb_control_msg: request: 0xFF, requesttype: 0x21, value 0x0 index 0x0 length 0x0 RESET:stall inquiry returns -1 COMMAND phase DATA phase STATUS phase !CSWSIGNATURE BBB_reset usb_control_msg: request: 0xFF, requesttype: 0x21, value 0x0 index 0x0 length 0x0 RESET:stall inquiry returns -1 COMMAND phase DATA phase STATUS phase !CSWSIGNATURE BBB_reset usb_control_msg: request: 0xFF, requesttype: 0x21, value 0x0 index 0x0 length 0x0 RESET:stall inquiry returns -1 COMMAND phase DATA phase STATUS phase !CSWSIGNATURE BBB_reset usb_control_msg: request: 0xFF, requesttype: 0x21, value 0x0 index 0x0 length 0x0 RESET:stall inquiry returns -1 error in inquiry i=2 0 Storage Device(s) found In this case putting a delay of 1000ms in usb_stor_BBB_transport between the COMMAND and DATA phase, (in place of the conditional 5ms USB_READY delay), overcomes this issue. It seems this 1000ms delay is only required for the first COMMAND, subsequent COMMAND phases seem happy with the 5ms delay. Lexar JumpDrive 32GB (0x0424/0x2507): So the device takes too long to init itself after powering up the port. It would be nice if you could check the spec and see if there is a way to query the device for ready condition. Although I remember we already do that. After further investigation I've learnt that removing references to MUSB_CSR0_H_DIS_PING in musb_hcd.c removes the need for a large delay between the COMMAND and STATUS phases, this allows more USB mass storage devices to work. MUSB_CSR0_H_DIS_PING sets bit 12 of txcsr, however on the DM36x (http://www.ti.com/litv/pdf/sprufh9a) this appears to be a reserved bit. In any case it appears to have an undesirable effect. The linux driver does appear to use this bit, yet these devices do work under Linux. I'll submit a patch if you feel this is incorrect, any suggestions for the best way to fix this? Andrew Murray ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Failing USB devices
On 23 August 2013 04:23, Marek Vasut ma...@denx.de wrote: Dear Andrew Murray, Hello, I'm unable to use some mass storage devices with the current git version (6612ab33956ae09c5ba2fde9c1540b519625ba37) of UBoot on a TI Davinci custom board. I have 7 pen drives, and 2 of them fail. I-O DATA USB Flash Disk (0x04bb/0x0c43) ... 2 USB Device(s) found scan end scanning usb for storage devices... i=0 i=1 USB Mass Storage device detected Transport: Bulk/Bulk/Bulk Endpoints In 1 Out 2 Int 0 usb_control_msg: request: 0xFE, requesttype: 0xA1, value 0x0 index 0x0 length 0x1 Get Max LUN - len = 1, result = 0 address 2 COMMAND phase DATA phase STATUS phase !CSWSIGNATURE BBB_reset usb_control_msg: request: 0xFF, requesttype: 0x21, value 0x0 index 0x0 length 0x0 RESET:stall inquiry returns -1 COMMAND phase DATA phase STATUS phase !CSWSIGNATURE BBB_reset usb_control_msg: request: 0xFF, requesttype: 0x21, value 0x0 index 0x0 length 0x0 RESET:stall inquiry returns -1 COMMAND phase DATA phase STATUS phase !CSWSIGNATURE BBB_reset usb_control_msg: request: 0xFF, requesttype: 0x21, value 0x0 index 0x0 length 0x0 RESET:stall inquiry returns -1 COMMAND phase DATA phase STATUS phase !CSWSIGNATURE BBB_reset usb_control_msg: request: 0xFF, requesttype: 0x21, value 0x0 index 0x0 length 0x0 RESET:stall inquiry returns -1 COMMAND phase DATA phase STATUS phase !CSWSIGNATURE BBB_reset usb_control_msg: request: 0xFF, requesttype: 0x21, value 0x0 index 0x0 length 0x0 RESET:stall inquiry returns -1 error in inquiry i=2 0 Storage Device(s) found In this case putting a delay of 1000ms in usb_stor_BBB_transport between the COMMAND and DATA phase, (in place of the conditional 5ms USB_READY delay), overcomes this issue. It seems this 1000ms delay is only required for the first COMMAND, subsequent COMMAND phases seem happy with the 5ms delay. Lexar JumpDrive 32GB (0x0424/0x2507): So the device takes too long to init itself after powering up the port. The port is powered after usb_hub_power_on, I've used CONFIG_USB_HUB_MIN_POWER_ON_DELAY to set this to a large value (10 seconds), however this has no effect on the above failure. In fact with some further testing it seems that adding the 1000ms delay anywhere inside the loop inside the usb_inquiry function overcomes the above issues. The inquiry still fails, but the subsequent BBB_reset's stop stalling - successfully reset and then the next inquiry succeeds and the storage device detected. E.g... . COMMAND phase DATA phase STATUS phase !CSWSIGNATURE BBB_reset RESET:stall inquiry returns -1 . COMMAND phase DATA phase STATUS phase !CSWSIGNATURE BBB_reset BBB_reset result 0: status 0 reset BBB_reset result 0: status 0 clearing IN endpoint BBB_reset result 0: status 0 clearing OUT endpoint BBB_reset done inquiry returns -1 . COMMAND phase DATA phase STATUS phase inquiry returns 0 ISO Vers 2, Response Data 2 COMMAND phase STATUS phase COMMAND phase DATA phase STATUS phase Read Capacity returns: 0xff3f3d00, 0x2 Capacity = 0x3d4000, blocksz = 0x200 address 2 partype: 0 Does this suggest there needs to be a larger initial delay in between the COMMAND and STATUS phase, something wrong with the BBB_reset or something else? It would be nice if you could check the spec and see if there is a way to query the device for ready condition. Although I remember we already do that. Unfortunately I do not have the spec (the USB device is an off-the-shelf pen drive). The above failure happens in the usb_inquiry function, this seems to be the first user of BBB transport and this happens before the first call to usb_test_unit_ready. Andrew Murray ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Failing USB devices
On 23 August 2013 11:54, Benoît Thébaudeau benoit.thebaud...@advansee.com wrote: Dear Marek, Andrew, ... This is the kind of issue that made me create this (discarded) patch at some point: http://patchwork.ozlabs.org/patch/176527/ Can you try it? I'm not sure that it will be helpful for your specific case. Unfortunately it has no effect. But then the STATUS phase isn't stalling in my case, it's just giving back an unexpected value for CSWSIGNATURE - I have no idea why this is. I also have this issue with some devices and mainline U-Boot. I can confirm that this is a device initialization delay issue because subsequent USB commands on the device succeed (only the single automatic U-Boot USB command that I use at boot time fails). You may be able to work around these by setting CONFIG_USB_HUB_MIN_POWER_ON_DELAY in your config file (assuming the device is behind a hub). Best regards, Benoît Andrew Murray ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] Failing USB devices
, requesttype: 0x23, value 0x8 index 0x5 length 0x0 port 5 returns 0 usb_control_msg: request: 0x3, requesttype: 0x23, value 0x8 index 0x6 length 0x0 port 6 returns 0 usb_control_msg: request: 0x3, requesttype: 0x23, value 0x8 index 0x7 length 0x0 port 7 returns 0 usb_control_msg: request: 0x0, requesttype: 0xA3, value 0x0 index 0x1 length 0x4 Port 1 Status 100 Change 0 usb_control_msg: request: 0x0, requesttype: 0xA3, value 0x0 index 0x2 length 0x4 Port 2 Status 100 Change 0 usb_control_msg: request: 0x0, requesttype: 0xA3, value 0x0 index 0x3 length 0x4 Port 3 Status 100 Change 0 usb_control_msg: request: 0x0, requesttype: 0xA3, value 0x0 index 0x4 length 0x4 Port 4 Status 100 Change 0 usb_control_msg: request: 0x0, requesttype: 0xA3, value 0x0 index 0x5 length 0x4 Port 5 Status 100 Change 0 usb_control_msg: request: 0x0, requesttype: 0xA3, value 0x0 index 0x6 length 0x4 Port 6 Status 100 Change 0 usb_control_msg: request: 0x0, requesttype: 0xA3, value 0x0 index 0x7 length 0x4 Port 7 Status 100 Change 0 1 USB Device(s) found scan end scanning usb for storage devices... i=0 i=1 0 Storage Device(s) found In this case setting CONFIG_USB_HUB_MIN_POWER_ON_DELAY to 3000 overcomes this issue. I have other USB drives with similar issues. It seems there is no correct value for CONFIG_USB_HUB_MIN_POWER_ON_DELAY - setting this to a large value supports more drives, but at the expense of boot time. Is this a suggested way to determine when the endpoint is ready rather than waiting an arbitrary amount of time? Is this possible? Likewise for the delay between the COMMAND and DATA phase. Has this issue been seen before? Is there any reason why a large delay would be required between the very first COMMAND and DATA phase vs subsequent phases - and if so is there value in changing UBoot to support this? Andrew Murray ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Improve output of bootm command
Hi Mike, On 11 September 2011 21:17, Mike Frysinger vap...@gentoo.org wrote: your change to the if statement is pointless ? You are right - I thought I was going crazy then realized I made this patch on an earlier version of the code which didn't have the load == image_start check in the first line (see http://git.denx.de/?p=u-boot.git;a=commitdiff;h=02cf345973a7fe9986626448a089ed54f1a26d13 from January). I must have blindly moved this patch forward. I will get my tools in order. Many thanks for you patience. Andrew Murray ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Improve output of bootm command
On 11 September 2011 20:22, Mike Frysinger vap...@gentoo.org wrote: On Saturday, September 10, 2011 10:57:47 Andrew Murray wrote: --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c if (load == blob_start || load == image_start) { .. - } else { + } else if (load != image_start) { sorry, but why does this new if() make any sense ? the only way this else branch could execute is if load != image_start since load == image_start was explicitly handled in the first if check. -mike Yes that's correct. The move is executed and a print statement displayed - only when the load address differs from the image start address. In other words the patch prevents unnecessary/confusing output and a call to a function that doesn't do anything when load == image_start. Andrew Murray ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2 2/7] Add macros for recording init calls during UBoot execution
From: Andrew Murray amur...@mpcdata.com This patch adds macros which allow for the instrumentation of UBoot boot time. The macros can be used to call existing initialisation functions during start up. Each macro adds printf statements before and after the initialisation call. --- Changes for v2: - Use dedicated printf with timestamp function - Allow DO_INITCALL_RET macro to provide return value Signed-off-by: Andrew Murray amur...@theiet.org --- include/common.h | 25 + 1 files changed, 25 insertions(+), 0 deletions(-) diff --git a/include/common.h b/include/common.h index 1e21b7a..a926430 100644 --- a/include/common.h +++ b/include/common.h @@ -176,6 +176,31 @@ typedef void (interrupt_handler_t)(void *); #endif /* CONFIG_SERIAL_MULTI */ +#if defined(CONFIG_BOOT_TRACE) +#define DO_INITCALL(x, ...) \ + do { \ + printf_boot_trace(calling 0x%pF\n, x); \ + (x)(__VA_ARGS__); \ + printf_boot_trace(initcall 0x%pF returned\n, x); \ + } while (0) +#define DO_INITCALL_RET(x, ...) \ + ({ \ + int __ret; \ + printf_boot_trace(calling 0x%pF\n, x); \ + __ret = (x)(__VA_ARGS__); \ + printf_boot_trace(initcall 0x%pF returned\n, x); \ + __ret; \ + }) +#define DO_INITCALL_END(x) \ + printf_boot_trace(initcall 0x%pF returned\n, x) +#else +#define DO_INITCALL(x, ...) \ + (x)(__VA_ARGS__) +#define DO_INITCALL_RET(x, ret, ...) \ + ret = (x)(__VA_ARGS__) +#define DO_INITCALL_END(x) +#endif + /* * General Purpose Utilities */ -- 1.7.4.1 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2 1/7] Add bootgraph.pl script for generating a boot graph SVG file
From: Andrew Murray amur...@mpcdata.com This is a port of the kernel's script/bootgraph.pl script which generates an SVG image illustrating boot time in UBoot. The script relies on additional output generated by UBoot during boot when the CONFIG_BOOT_TRACE option is enabled. Signed-off-by: Andrew Murray amur...@theiet.org --- tools/bootgraph.pl | 189 1 files changed, 189 insertions(+), 0 deletions(-) create mode 100755 tools/bootgraph.pl diff --git a/tools/bootgraph.pl b/tools/bootgraph.pl new file mode 100755 index 000..8b206b5 --- /dev/null +++ b/tools/bootgraph.pl @@ -0,0 +1,189 @@ +#!/usr/bin/perl + +# Copyright (C) 2011, Andrew Murray amur...@theiet.org) +# Copyright (C) 2008, Intel Corporation (Arjan van de Ven ar...@linux.intel.com) +# +# This program file is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by the +# Free Software Foundation; version 2 of the License. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License +# for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program in a file named COPYING; if not, write to the +# Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, +# Boston, MA 02110-1301 USA +# +# This script was originally written by Arjan for use with the Linux kernel and +# subsequently ported across to UBoot by Andrew. +# +# This script turns a output trace from UBoot into a SVG graphic that shows +# which functions take how much time. You can view SVG graphics with various +# programs, including Inkscape, The Gimp and Firefox. +# +# For this script to work, the UBoot config file needs to be compiled with the +# CONFIG_BOOT_TRACE configuration option enabled. +# +# usage: +# cat uboot.trace | perl ./tools/bootgraph.pl output.svg +# + +use strict; + +my %start; +my %end; +my %type; +my $done = 0; +my $maxtime = 0; +my $firsttime = 9; +my $count = 0; +my $textoffset = hex('0x80008000'); +my $reloffset = $textoffset; +my %sym; + +if (!open FILE, System.map) { + print STDERR END; +No map file found ./System.map. Make sure that a file named +System.map exists in the current working directory. +Usage: + cat uboot.trace | perl ./tools/bootgraph.pl output.svg +END + exit 1; +} + +# Read in a map file to support in the printing of function names +while (FILE) { + my $line = $_; + if ($line =~ /([a-zA-Z0-9]+)\ [a-zA-Z] ([a-zA-Z0-0\_\.]+)/) { + $sym{$1} = $2; + } +} +close FILE; + +# Parse lines of UBoot's trace to generate a timeline +while () { + my $line = $_; + + # Find start of init calls as instrumented in UBoot with DO_INITCALLXXX macros + if ($line =~ /([0-9\.]+)\] calling (0x[a-zA-Z0-9\_\.]+)/) { + my $func = sprintf(%x, (hex($2) - $reloffset + $textoffset)); + $func = $sym{$func}; + if ($done == 0) { + $start{$func} = $1; + $type{$func} = 0; + if ($1 $firsttime) { + $firsttime = $1; + } + } + $count = $count + 1; + } + + # Find end of init calls as instrumented in UBoot with DO_INITCALLXXX macros + if ($line =~ /([0-9\.]+)\] initcall (0x[a-zA-Z0-9\_\.]+).*returned/) { + if ($done == 0) { + my $func = sprintf(%x, (hex($2) - $reloffset + $textoffset)); + $func = $sym{$func}; + $end{$func} = $1; + $maxtime = $1; + } + } + + # Determine where UBoot relocates itself in RAM + if ($line =~ /performing relocate to ram to (0x[a-zA-Z0-9\_\.]+)/) { + $reloffset = hex($1); + } + + # Stop scanning once we've left UBoot + if ($line =~ /Starting kernel /) { + $done = 1; + } +} + +# No data was captured +if ($count == 0) { +print STDERR END; +No data found in the dmesg. Make sure that CONFIG_BOOT_TRACE is +defined. +Usage: + cat uboot.trace | perl ./tools/bootgraph.pl output.svg +END +exit 1; +} + +print ?xml version=\1.0\ standalone=\no\? \n; +print svg width=\2000\ height=\100%\ version=\1.1\ xmlns=\http://www.w3.org/2000/svg\;\n; + +my @styles; + +$styles[0] = fill:rgb(0,0,255);fill-opacity:0.5;stroke-width:1;stroke:rgb(0,0,0); +$styles[1] = fill:rgb(0,255,0);fill-opacity:0.5;stroke-width:1;stroke:rgb(0,0,0); +$styles[2] = fill:rgb(255,0,20);fill-opacity:0.5;stroke-width:1;stroke:rgb(0,0,0); +$styles[3] = fill:rgb(255,255,20);fill-opacity:0.5;stroke-width:1;stroke:rgb(0,0,0); +$styles[4] = fill:rgb(255,0,255);fill
[U-Boot] [PATCH v2 3/7] Add timing information to printf's for use with bootgraph.pl
From: Andrew Murray amur...@mpcdata.com This patch adds timings information to printfs. --- Changes for v2: - Add dedicated function to printf with timestamps - Fix compiler warnings - Remove code duplication within printf Signed-off-by: Andrew Murray amur...@theiet.org --- common/console.c | 27 --- include/common.h |2 ++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/common/console.c b/common/console.c index 8c650e0..63b84ba 100644 --- a/common/console.c +++ b/common/console.c @@ -365,24 +365,45 @@ void puts(const char *s) } } -int printf(const char *fmt, ...) +#if defined(CONFIG_BOOT_TRACE) +int printf_boot_trace(const char *fmt, ...) { va_list args; - uint i; + uint i = 0; char printbuffer[CONFIG_SYS_PBSIZE]; + char *buf = printbuffer; + unsigned long long ticks = get_ticks(); + uint secs = ticks / get_tbclk(); + uint msec = ((ticks * 100) / get_tbclk()) - (secs * 100); + + i += sprintf(buf, [%5u.%06u] , secs, msec); + buf += i; va_start(args, fmt); /* For this to work, printbuffer must be larger than * anything we ever want to print. */ - i = vsprintf(printbuffer, fmt, args); + i += vsprintf(buf, fmt, args); va_end(args); /* Print the string */ puts(printbuffer); return i; } +#endif + +int printf(const char *fmt, ...) +{ + va_list args; + uint i = 0; + + va_start(args, fmt); + i = vprintf(fmt, args); + va_end(args); + + return i; +} int vprintf(const char *fmt, va_list args) { diff --git a/include/common.h b/include/common.h index a926430..16175b4 100644 --- a/include/common.h +++ b/include/common.h @@ -712,6 +712,8 @@ voidputs(const char *s); intprintf(const char *fmt, ...) __attribute__ ((format (__printf__, 1, 2))); intvprintf(const char *fmt, va_list args); +intprintf_boot_trace(const char *fmt, ...) + __attribute__ ((format (__printf__, 1, 2))); /* stderr */ #define eputc(c) fputc(stderr, c) -- 1.7.4.1 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2 5/7] Add bootgraph instrumentation for bootm command
Under normal operation the bootm command will never return - this patch adds additional instrumentation to signal the 'end' of the bootm command such that this point can be reflected in any bootgraph SVG. Signed-off-by: Andrew Murray amur...@theiet.org --- common/cmd_bootm.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index 1966da4..147e8de 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -697,6 +697,7 @@ int do_bootm (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) arch_preboot_os(); + DO_INITCALL_END(do_bootm); boot_fn(0, argc, argv, images); show_boot_progress (-9); -- 1.7.4.1 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2 7/7] Add documentation for bootgraph.pl
From: Andrew Murray amur...@mpcdata.com Documentation for the CONFIG_BOOT_TRACE option. --- Changes for v2: - Update documentation for v2 changes Signed-off-by: Andrew Murray amur...@theiet.org --- doc/README.bootgraph | 54 ++ 1 files changed, 54 insertions(+), 0 deletions(-) create mode 100644 doc/README.bootgraph diff --git a/doc/README.bootgraph b/doc/README.bootgraph new file mode 100644 index 000..cdd8211 --- /dev/null +++ b/doc/README.bootgraph @@ -0,0 +1,54 @@ +Bootgraph Instrumentation +- + +The CONFIG_BOOT_TRACE configuration option can be defined to output extensive +instrumentation to assist in boot time reduction. The tools/bootgraph.pl script +can utilise this instrumentation to generate an SVG graph showing where UBoot +spends its time. + +When enabled all printf_boot_trace output is prefixed with timing information +similar to the Linux kernel's CONFIG_PRINTK_TIME option. This allows you to +measure the interval between operations which is useful for identifying long +delays during UBoot operation. + +When enabled additional console output will be generated - this output includes +the addresses of executed commands and instrumented functions. For example: + +[2.456000] initcall 0x9ff86814 returned +[2.46] calling 0x9ff7c338 +[2.59] initcall 0x9ff7c338 returned +[2.594000] calling 0x9ff864ac + +At present executed commands are only displayed when the HUSH parser +(CONFIG_SYS_HUSH_PARSER) is not being used. + +Functions such as initialisation code can be instrumented by using the +DO_INITCALLXXX macros found in include/common.h (see arch/arm/lib/board.c) for +examples). For example: + +@@ -508,7 +518,7 @@ void board_init_r (gd_t *id, ulong dest_addr) + + #ifdef CONFIG_GENERIC_MMC +puts(MMC: ); +- mmc_initialize(bd); ++ DO_INITCALL(mmc_initialize, bd); + #endif + +For functions that do not return such as do_bootm function, the DO_INITCALL_END +macro can be used to mark the latest point of the function. + +When CONFIG_BOOT_TIME is not defined the DO_INITCALLXXX macros continue to call +the intended function and should not add overhead. + +An SVG graph can be generated which allows for the visualisation of UBoot boot +time by using the tools/bootgraph.pl script. This script has been ported from +the Linux kernel (scripts/bootgraph.pl) and is used in a similar way. For +example: + +cat console_output | perl ./tools/bootgraph.pl graph.svg + +Assuming console_output is a file containing the console output of UBoot with +CONFIG_BOOT_TRACE enabled - the graph.svg file should provide a graphical +representation of boot time with resolved function addresses. + +Andrew Murray amur...@theiet.org -- 1.7.4.1 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2 6/7] Add bootgraph instrumentation for UBoot commands
From: Andrew Murray amur...@mpcdata.com This patch adds bootgraph instrumentation for all U_BOOT_CMDs where the HUSH parser is not used. The patch also adds instrumentation for the common boot delay. --- Changes for v2: - Use improved DO_INITCALL_RET macro Signed-off-by: Andrew Murray amur...@theiet.org --- common/main.c |8 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/common/main.c b/common/main.c index 2730c6f..bf95463 100644 --- a/common/main.c +++ b/common/main.c @@ -376,7 +376,7 @@ void main_loop (void) debug (### main_loop: bootcmd=\%s\\n, s ? s : UNDEFINED); - if (bootdelay = 0 s !abortboot (bootdelay)) { + if (bootdelay = 0 s !DO_INITCALL_RET(abortboot, bootdelay)) { # ifdef CONFIG_AUTOBOOT_KEYED int prev = disable_ctrlc(1);/* disable Control C checking */ # endif @@ -385,11 +385,11 @@ void main_loop (void) run_command (s, 0); # else parse_string_outer(s, FLAG_PARSE_SEMICOLON | - FLAG_EXIT_FROM_LOOP); + FLAG_EXIT_FROM_LOOP); # endif # ifdef CONFIG_AUTOBOOT_KEYED - disable_ctrlc(prev);/* restore Control C checking */ + disable_ctrlc(prev);/* restore Control C checking */ # endif } @@ -1371,7 +1371,7 @@ int run_command (const char *cmd, int flag) #endif /* OK - call function to do the command */ - if ((cmdtp-cmd) (cmdtp, flag, argc, argv) != 0) { + if (DO_INITCALL_RET(cmdtp-cmd, cmdtp, flag, argc, argv) != 0) { rc = -1; } -- 1.7.4.1 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2 4/7] Add bootgraph instrumentation for ARM boards
From: Andrew Murray amur...@mpcdata.com This patch adds bootgraph instrumentation for ARM boards. --- Changes for v2: - Use improvde DO_INITCALL_RET macro - Fix compiler warnings Signed-off-by: Andrew Murray amur...@theiet.org --- arch/arm/lib/board.c | 26 +- 1 files changed, 17 insertions(+), 9 deletions(-) diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c index 169dfeb..7b09bf1 100644 --- a/arch/arm/lib/board.c +++ b/arch/arm/lib/board.c @@ -282,7 +282,7 @@ void board_init_f (ulong bootflag) gd-mon_len = _bss_end_ofs; for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) { - if ((*init_fnc_ptr)() != 0) { + if (DO_INITCALL_RET(*init_fnc_ptr) != 0) { hang (); } } @@ -416,6 +416,9 @@ void board_init_f (ulong bootflag) debug (relocation Offset is: %08lx\n, gd-reloc_off); memcpy (id, (void *)gd, sizeof (gd_t)); +#if defined(CONFIG_BOOT_TRACE) + printf(performing relocate to ram to 0x%08lx\n, addr); +#endif relocate_code (addr_sp, id, addr); /* NOTREACHED - relocate_code() does not return */ @@ -444,6 +447,10 @@ void board_init_r (gd_t *id, ulong dest_addr) ulong flash_size; #endif +#if defined(CONFIG_BOOT_TRACE) + printf(completed relocate to ram\n); +#endif + gd = id; bd = gd-bd; @@ -451,7 +458,8 @@ void board_init_r (gd_t *id, ulong dest_addr) monitor_flash_len = _end_ofs; debug (monitor flash len: %08lX\n, monitor_flash_len); - board_init(); /* Setup chipselects */ + + DO_INITCALL(board_init);/* Setup chipselects */ #ifdef CONFIG_SERIAL_MULTI serial_initialize(); @@ -499,7 +507,7 @@ void board_init_r (gd_t *id, ulong dest_addr) #if defined(CONFIG_CMD_NAND) puts (NAND: ); - nand_init();/* go init the NAND */ + DO_INITCALL(nand_init); /* go init the NAND */ #endif #if defined(CONFIG_CMD_ONENAND) @@ -508,7 +516,7 @@ void board_init_r (gd_t *id, ulong dest_addr) #ifdef CONFIG_GENERIC_MMC puts(MMC: ); - mmc_initialize(bd); + DO_INITCALL(mmc_initialize, bd); #endif #ifdef CONFIG_HAS_DATAFLASH @@ -517,7 +525,7 @@ void board_init_r (gd_t *id, ulong dest_addr) #endif /* initialize environment */ - env_relocate (); + DO_INITCALL(env_relocate); #if defined(CONFIG_CMD_PCI) || defined(CONFIG_PCI) arm_pci_init(); @@ -526,16 +534,16 @@ void board_init_r (gd_t *id, ulong dest_addr) /* IP Address */ gd-bd-bi_ip_addr = getenv_IPaddr (ipaddr); - stdio_init (); /* get the devices list going. */ + DO_INITCALL(stdio_init);/* get the devices list going. */ - jumptable_init (); + DO_INITCALL(jumptable_init); #if defined(CONFIG_API) /* Initialize API */ api_init (); #endif - console_init_r (); /* fully init console as a device */ + DO_INITCALL(console_init_r);/* fully init console as a device */ #if defined(CONFIG_ARCH_MISC_INIT) /* miscellaneous arch dependent initialisations */ @@ -543,7 +551,7 @@ void board_init_r (gd_t *id, ulong dest_addr) #endif #if defined(CONFIG_MISC_INIT_R) /* miscellaneous platform dependent initialisations */ - misc_init_r (); + DO_INITCALL(misc_init_r); #endif /* set up exceptions */ -- 1.7.4.1 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 0/7] Bootgraph.pl instrumentation support for UBoot
On 1 September 2011 00:53, Andrew Murray amur...@theiet.org wrote: I will assume that we have a microsecond timer, update my patch and resubmit so you can take a look and see what you think. Hopefully we can unify this, your patch and the boot_progress stuff. Excellent! OK, well I will await the patch, read up on the original intentions and we can go from there. I'll look forward to making UBoot more boot time friendly. Good night. I've updated my patches based on your feedback. I guess the next step is to see how to integrate with the bootstage work. Andrew Murray ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2 2/7] Add macros for recording init calls during UBoot execution
From: Andrew Murray amur...@mpcdata.com The previous patch included a compile error when the CONFIG_BOOT_TRACE macro is not set, this is an update to that patch --- This patch adds macros which allow for the instrumentation of UBoot boot time. The macros can be used to call existing initialisation functions during start up. Each macro adds printf statements before and after the initialisation call. --- Changes for v2: - Use dedicated printf with timestamp function - Allow DO_INITCALL_RET macro to provide return value Signed-off-by: Andrew Murray amur...@theiet.org --- include/common.h | 29 + 1 files changed, 29 insertions(+), 0 deletions(-) diff --git a/include/common.h b/include/common.h index 1e21b7a..196c306 100644 --- a/include/common.h +++ b/include/common.h @@ -176,6 +176,33 @@ typedef void (interrupt_handler_t)(void *); #endif /* CONFIG_SERIAL_MULTI */ +#if defined(CONFIG_BOOT_TRACE) +#define DO_INITCALL(x, ...) \ + do { \ + printf_boot_trace(calling 0x%pF\n, x); \ + (x)(__VA_ARGS__); \ + printf_boot_trace(initcall 0x%pF returned\n, x); \ + } while (0) +#define DO_INITCALL_RET(x, ...) \ + ({ \ + int __ret; \ + printf_boot_trace(calling 0x%pF\n, x); \ + __ret = (x)(__VA_ARGS__); \ + printf_boot_trace(initcall 0x%pF returned\n, x); \ + __ret; \ + }) +#define DO_INITCALL_END(x) \ + do { \ + printf_boot_trace(initcall 0x%pF returned\n, x); \ + } while (0) +#else +#define DO_INITCALL(x, ...) \ + ({ (x)(__VA_ARGS__); }) +#define DO_INITCALL_RET(x, ...) \ + ({ (x)(__VA_ARGS__); }) +#define DO_INITCALL_END(x) +#endif + /* * General Purpose Utilities */ @@ -687,6 +714,8 @@ voidputs(const char *s); intprintf(const char *fmt, ...) __attribute__ ((format (__printf__, 1, 2))); intvprintf(const char *fmt, va_list args); +intprintf_boot_trace(const char *fmt, ...) + __attribute__ ((format (__printf__, 1, 2))); /* stderr */ #define eputc(c) fputc(stderr, c) -- 1.7.4.1 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] Improve output of bootm command
It's common for the bootm command to move a provided image in memory prior to it's execution - this move can contribute to increased boot times. This move is often seen in poorly configured devices which boot from NAND. This patch improves the output of the bootm command such that when a move occurs it is made clear to the user. Signed-off-by: Andrew Murray amur...@theiet.org --- common/cmd_bootm.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index 147e8de..b5f28b0 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -346,8 +346,8 @@ static int bootm_load_os(image_info_t os, ulong *load_end, int boot_progress) case IH_COMP_NONE: if (load == blob_start || load == image_start) { printf ( XIP %s ... , type_name); - } else { - printf ( Loading %s ... , type_name); + } else if (load != image_start) { + printf ( Moving %s ... , type_name); memmove_wd ((void *)load, (void *)image_start, image_len, CHUNKSZ); } -- 1.7.4.1 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Improve output of bootm command
Hi, On 10 September 2011 16:06, Wolfgang Denk w...@denx.de wrote: Sorry, but I don't want to have this output. It would be always be printed for all systems booting from NOR flash, where the copy operation is absolutely normal. Also, on all PowerPC systems it is absolutely normal that you must load the image to a different address in RAM, because otherwise you would overwrite the exception vectors and thus crash U-Boot. What you are trying to warn about is an absolutely normal use case, and there is no reason for additional output. Yes, there may be systems that could work more efficiently, but there are many areas where you can optimize (starting with the Linux image building). Yes I understand your point but perhaps my description was not very useful. The patch doesn't add any additional messages or output or anything which indicates something is not ideal. Instead it renames the exiting 'Loading' text to a arguably more descriptive 'Moving' text - and removes the message all together when the move operation doesn't occur. Thanks, Andrew Murray ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] U-Boot with gcov / gprof?
Hi, I've not used that - but have used -finstrument-functions which works fairly well. The difficulty is figuring out what to do during the extra calls it generates. Thanks, Andrew Murray On 2 September 2011 05:06, Simon Glass s...@chromium.org wrote: Hi, Has anyone got a profiling tool running with U-Boot? I can't see a mention of it in the lists but thought I should ask. I would like to get something running. Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 1/7] Add bootgraph.pl script for generating a boot graph SVG file
From: Andrew Murray amur...@mpcdata.com This is a port of the kernel's script/bootgraph.pl script which generates an SVG image illustrating boot time in UBoot. The script relies on additional output generated by UBoot during boot when the CONFIG_BOOT_TRACE option is enabled. Signed-off-by: Andrew Murray amur...@theiet.org --- tools/bootgraph.pl | 189 1 files changed, 189 insertions(+), 0 deletions(-) create mode 100755 tools/bootgraph.pl diff --git a/tools/bootgraph.pl b/tools/bootgraph.pl new file mode 100755 index 000..8b206b5 --- /dev/null +++ b/tools/bootgraph.pl @@ -0,0 +1,189 @@ +#!/usr/bin/perl + +# Copyright (C) 2011, Andrew Murray amur...@theiet.org) +# Copyright (C) 2008, Intel Corporation (Arjan van de Ven ar...@linux.intel.com) +# +# This program file is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by the +# Free Software Foundation; version 2 of the License. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License +# for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program in a file named COPYING; if not, write to the +# Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, +# Boston, MA 02110-1301 USA +# +# This script was originally written by Arjan for use with the Linux kernel and +# subsequently ported across to UBoot by Andrew. +# +# This script turns a output trace from UBoot into a SVG graphic that shows +# which functions take how much time. You can view SVG graphics with various +# programs, including Inkscape, The Gimp and Firefox. +# +# For this script to work, the UBoot config file needs to be compiled with the +# CONFIG_BOOT_TRACE configuration option enabled. +# +# usage: +# cat uboot.trace | perl ./tools/bootgraph.pl output.svg +# + +use strict; + +my %start; +my %end; +my %type; +my $done = 0; +my $maxtime = 0; +my $firsttime = 9; +my $count = 0; +my $textoffset = hex('0x80008000'); +my $reloffset = $textoffset; +my %sym; + +if (!open FILE, System.map) { + print STDERR END; +No map file found ./System.map. Make sure that a file named +System.map exists in the current working directory. +Usage: + cat uboot.trace | perl ./tools/bootgraph.pl output.svg +END + exit 1; +} + +# Read in a map file to support in the printing of function names +while (FILE) { + my $line = $_; + if ($line =~ /([a-zA-Z0-9]+)\ [a-zA-Z] ([a-zA-Z0-0\_\.]+)/) { + $sym{$1} = $2; + } +} +close FILE; + +# Parse lines of UBoot's trace to generate a timeline +while () { + my $line = $_; + + # Find start of init calls as instrumented in UBoot with DO_INITCALLXXX macros + if ($line =~ /([0-9\.]+)\] calling (0x[a-zA-Z0-9\_\.]+)/) { + my $func = sprintf(%x, (hex($2) - $reloffset + $textoffset)); + $func = $sym{$func}; + if ($done == 0) { + $start{$func} = $1; + $type{$func} = 0; + if ($1 $firsttime) { + $firsttime = $1; + } + } + $count = $count + 1; + } + + # Find end of init calls as instrumented in UBoot with DO_INITCALLXXX macros + if ($line =~ /([0-9\.]+)\] initcall (0x[a-zA-Z0-9\_\.]+).*returned/) { + if ($done == 0) { + my $func = sprintf(%x, (hex($2) - $reloffset + $textoffset)); + $func = $sym{$func}; + $end{$func} = $1; + $maxtime = $1; + } + } + + # Determine where UBoot relocates itself in RAM + if ($line =~ /performing relocate to ram to (0x[a-zA-Z0-9\_\.]+)/) { + $reloffset = hex($1); + } + + # Stop scanning once we've left UBoot + if ($line =~ /Starting kernel /) { + $done = 1; + } +} + +# No data was captured +if ($count == 0) { +print STDERR END; +No data found in the dmesg. Make sure that CONFIG_BOOT_TRACE is +defined. +Usage: + cat uboot.trace | perl ./tools/bootgraph.pl output.svg +END +exit 1; +} + +print ?xml version=\1.0\ standalone=\no\? \n; +print svg width=\2000\ height=\100%\ version=\1.1\ xmlns=\http://www.w3.org/2000/svg\;\n; + +my @styles; + +$styles[0] = fill:rgb(0,0,255);fill-opacity:0.5;stroke-width:1;stroke:rgb(0,0,0); +$styles[1] = fill:rgb(0,255,0);fill-opacity:0.5;stroke-width:1;stroke:rgb(0,0,0); +$styles[2] = fill:rgb(255,0,20);fill-opacity:0.5;stroke-width:1;stroke:rgb(0,0,0); +$styles[3] = fill:rgb(255,255,20);fill-opacity:0.5;stroke-width:1;stroke:rgb(0,0,0); +$styles[4] = fill:rgb(255,0,255);fill
[U-Boot] [PATCH 0/7] Bootgraph.pl instrumentation support for UBoot
This patchset introduces the CONFIG_BOOT_TRACE option which provides support for boot time instrumentation. When enabled printf output is prefixed with timing information (similar to the kernel's CONFIG_PRINTK_TIME option) and additional output is generated which instruments functions and commands called (much like the kernel's initcall_debug functionality). The kernel's bootgraph.pl script has been ported to render UBoots instrumented ouptut into a pretty SVG graph. An example of this can be found here: http://goo.gl/dX8aR - which shows the boot time of a Beagle board. The patch currently provides support for instrumentation of UBoot commands (e.g. U_BOOT_CMD) for all platforms but only when the HUSH shell is not in use. Initialisation instrumentation is only limited to the arch/arm/lib/board.c file at present but can very easily be extended to other relevant files. The patch also includes documentation. Andrew Murray (7): Add bootgraph.pl script for generating a boot graph SVG file Add macros for recording init calls during UBoot execution Add timing information to printf's for use with bootgraph.pl Add bootgraph instrumentation for ARM boards Add bootgraph instrumentation for bootm command Add bootgraph instrumentation for UBoot commands Add documentation for bootgraph.pl arch/arm/lib/board.c | 28 +--- common/cmd_bootm.c |1 + common/console.c | 12 +++- common/main.c| 19 +++-- doc/README.bootgraph | 57 +++ include/common.h | 23 ++ tools/bootgraph.pl | 189 ++ 7 files changed, 312 insertions(+), 17 deletions(-) create mode 100644 doc/README.bootgraph create mode 100755 tools/bootgraph.pl -- 1.7.4.1 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 3/7] Add timing information to printf's for use with bootgraph.pl
From: Andrew Murray amur...@mpcdata.com This patch adds timings information to printfs. Signed-off-by: Andrew Murray amur...@theiet.org --- common/console.c | 12 +++- 1 files changed, 11 insertions(+), 1 deletions(-) diff --git a/common/console.c b/common/console.c index 8c650e0..33de3fa 100644 --- a/common/console.c +++ b/common/console.c @@ -370,13 +370,23 @@ int printf(const char *fmt, ...) va_list args; uint i; char printbuffer[CONFIG_SYS_PBSIZE]; + char *buf = printbuffer; va_start(args, fmt); +#if defined(CONFIG_BOOT_TRACE) + unsigned long long ticks = get_ticks(); + int secs = ticks / get_tbclk(); + int msec = ((ticks * 100) / get_tbclk()) - (secs * 100); + + i += sprintf(buf, [%5lu.%06lu] , secs, msec); + buf += i; +#endif + /* For this to work, printbuffer must be larger than * anything we ever want to print. */ - i = vsprintf(printbuffer, fmt, args); + i += vsprintf(buf, fmt, args); va_end(args); /* Print the string */ -- 1.7.4.1 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 2/7] Add macros for recording init calls during UBoot execution
From: Andrew Murray amur...@mpcdata.com This patch adds macros which allow for the instrumentation of UBoot boot time. The macros can be used to call existing initialisation functions during start up. Each macro adds printf statements before and after the initialisation call. Signed-off-by: Andrew Murray amur...@theiet.org --- include/common.h | 23 +++ 1 files changed, 23 insertions(+), 0 deletions(-) diff --git a/include/common.h b/include/common.h index 1e21b7a..aa21b10 100644 --- a/include/common.h +++ b/include/common.h @@ -176,6 +176,29 @@ typedef void (interrupt_handler_t)(void *); #endif /* CONFIG_SERIAL_MULTI */ +#if defined(CONFIG_BOOT_TRACE) +#define DO_INITCALL(x, ...) \ + do { \ + printf(calling 0x%pF\n, x); \ + (x)(__VA_ARGS__); \ + printf(initcall 0x%pF returned\n, x); \ + } while (0) +#define DO_INITCALL_RET(x, ret, ...) \ + do { \ + printf(calling 0x%pF\n, x); \ + ret = (x)(__VA_ARGS__); \ + printf(initcall 0x%pF returned\n, x); \ + } while (0) +#define DO_INITCALL_END(x) \ + printf(initcall 0x%pF returned\n, x) +#else +#define DO_INITCALL(x, ...) \ + (x)(__VA_ARGS__) +#define DO_INITCALL_RET(x, ret, ...) \ + ret = (x)(__VA_ARGS__) +#define DO_INITCALL_END(x) +#endif + /* * General Purpose Utilities */ -- 1.7.4.1 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 5/7] Add bootgraph instrumentation for bootm command
Under normal operation the bootm command will never return - this patch adds additional instrumentation to signal the 'end' of the bootm command such that this point can be reflected in any bootgraph SVG. Signed-off-by: Andrew Murray amur...@theiet.org --- common/cmd_bootm.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index 1966da4..147e8de 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -697,6 +697,7 @@ int do_bootm (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) arch_preboot_os(); + DO_INITCALL_END(do_bootm); boot_fn(0, argc, argv, images); show_boot_progress (-9); -- 1.7.4.1 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 4/7] Add bootgraph instrumentation for ARM boards
From: Andrew Murray amur...@mpcdata.com This patch adds bootgraph instrumentation for ARM boards. Signed-off-by: Andrew Murray amur...@theiet.org --- arch/arm/lib/board.c | 28 +++- 1 files changed, 19 insertions(+), 9 deletions(-) diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c index 169dfeb..e6bb8e2 100644 --- a/arch/arm/lib/board.c +++ b/arch/arm/lib/board.c @@ -271,6 +271,7 @@ void board_init_f (ulong bootflag) init_fnc_t **init_fnc_ptr; gd_t *id; ulong addr, addr_sp; + int ret; /* Pointer is writable since we allocated a register for it */ gd = (gd_t *) ((CONFIG_SYS_INIT_SP_ADDR) ~0x07); @@ -282,7 +283,8 @@ void board_init_f (ulong bootflag) gd-mon_len = _bss_end_ofs; for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) { - if ((*init_fnc_ptr)() != 0) { + DO_INITCALL_RET(*init_fnc_ptr, ret); + if (ret != 0) { hang (); } } @@ -416,6 +418,9 @@ void board_init_f (ulong bootflag) debug (relocation Offset is: %08lx\n, gd-reloc_off); memcpy (id, (void *)gd, sizeof (gd_t)); +#if defined(CONFIG_BOOT_TRACE) + printf(performing relocate to ram to 0x%pF\n, addr); +#endif relocate_code (addr_sp, id, addr); /* NOTREACHED - relocate_code() does not return */ @@ -444,6 +449,10 @@ void board_init_r (gd_t *id, ulong dest_addr) ulong flash_size; #endif +#if defined(CONFIG_BOOT_TRACE) + printf(completed relocate to ram\n); +#endif + gd = id; bd = gd-bd; @@ -451,7 +460,8 @@ void board_init_r (gd_t *id, ulong dest_addr) monitor_flash_len = _end_ofs; debug (monitor flash len: %08lX\n, monitor_flash_len); - board_init(); /* Setup chipselects */ + + DO_INITCALL(board_init);/* Setup chipselects */ #ifdef CONFIG_SERIAL_MULTI serial_initialize(); @@ -499,7 +509,7 @@ void board_init_r (gd_t *id, ulong dest_addr) #if defined(CONFIG_CMD_NAND) puts (NAND: ); - nand_init();/* go init the NAND */ + DO_INITCALL(nand_init); /* go init the NAND */ #endif #if defined(CONFIG_CMD_ONENAND) @@ -508,7 +518,7 @@ void board_init_r (gd_t *id, ulong dest_addr) #ifdef CONFIG_GENERIC_MMC puts(MMC: ); - mmc_initialize(bd); + DO_INITCALL(mmc_initialize, bd); #endif #ifdef CONFIG_HAS_DATAFLASH @@ -517,7 +527,7 @@ void board_init_r (gd_t *id, ulong dest_addr) #endif /* initialize environment */ - env_relocate (); + DO_INITCALL(env_relocate); #if defined(CONFIG_CMD_PCI) || defined(CONFIG_PCI) arm_pci_init(); @@ -526,16 +536,16 @@ void board_init_r (gd_t *id, ulong dest_addr) /* IP Address */ gd-bd-bi_ip_addr = getenv_IPaddr (ipaddr); - stdio_init (); /* get the devices list going. */ + DO_INITCALL(stdio_init);/* get the devices list going. */ - jumptable_init (); + DO_INITCALL(jumptable_init); #if defined(CONFIG_API) /* Initialize API */ api_init (); #endif - console_init_r (); /* fully init console as a device */ + DO_INITCALL(console_init_r);/* fully init console as a device */ #if defined(CONFIG_ARCH_MISC_INIT) /* miscellaneous arch dependent initialisations */ @@ -543,7 +553,7 @@ void board_init_r (gd_t *id, ulong dest_addr) #endif #if defined(CONFIG_MISC_INIT_R) /* miscellaneous platform dependent initialisations */ - misc_init_r (); + DO_INITCALL(misc_init_r); #endif /* set up exceptions */ -- 1.7.4.1 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 6/7] Add bootgraph instrumentation for UBoot commands
From: Andrew Murray amur...@mpcdata.com This patch adds bootgraph instrumentation for all U_BOOT_CMDs where the HUSH parser is not used. The patch also adds instrumentation for the common boot delay. Signed-off-by: Andrew Murray amur...@theiet.org --- common/main.c | 19 --- 1 files changed, 12 insertions(+), 7 deletions(-) diff --git a/common/main.c b/common/main.c index 2730c6f..0c78a94 100644 --- a/common/main.c +++ b/common/main.c @@ -273,6 +273,7 @@ void main_loop (void) int rc = 1; int flag; #endif + int ret; #if defined(CONFIG_BOOTDELAY) (CONFIG_BOOTDELAY = 0) char *s; @@ -376,21 +377,24 @@ void main_loop (void) debug (### main_loop: bootcmd=\%s\\n, s ? s : UNDEFINED); - if (bootdelay = 0 s !abortboot (bootdelay)) { + if (bootdelay = 0 s) { + DO_INITCALL_RET(abortboot, ret, bootdelay); + if (!ret) { # ifdef CONFIG_AUTOBOOT_KEYED - int prev = disable_ctrlc(1);/* disable Control C checking */ + int prev = disable_ctrlc(1);/* disable Control C checking */ # endif # ifndef CONFIG_SYS_HUSH_PARSER - run_command (s, 0); + run_command (s, 0); # else - parse_string_outer(s, FLAG_PARSE_SEMICOLON | + parse_string_outer(s, FLAG_PARSE_SEMICOLON | FLAG_EXIT_FROM_LOOP); # endif # ifdef CONFIG_AUTOBOOT_KEYED - disable_ctrlc(prev);/* restore Control C checking */ + disable_ctrlc(prev);/* restore Control C checking */ # endif + } } # ifdef CONFIG_MENUKEY @@ -1271,7 +1275,7 @@ int run_command (const char *cmd, int flag) char *argv[CONFIG_SYS_MAXARGS + 1]; /* NULL terminated */ int argc, inquotes; int repeatable = 1; - int rc = 0; + int rc = 0, ret; #ifdef DEBUG_PARSER printf ([RUN_COMMAND] cmd[%p]=\, cmd); @@ -1371,7 +1375,8 @@ int run_command (const char *cmd, int flag) #endif /* OK - call function to do the command */ - if ((cmdtp-cmd) (cmdtp, flag, argc, argv) != 0) { + DO_INITCALL_RET(cmdtp-cmd, ret, cmdtp, flag, argc, argv); + if (ret != 0) { rc = -1; } -- 1.7.4.1 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 7/7] Add documentation for bootgraph.pl
From: Andrew Murray amur...@mpcdata.com Documentation for the CONFIG_BOOT_TRACE option. Signed-off-by: Andrew Murray amur...@theiet.org --- doc/README.bootgraph | 57 ++ 1 files changed, 57 insertions(+), 0 deletions(-) create mode 100644 doc/README.bootgraph diff --git a/doc/README.bootgraph b/doc/README.bootgraph new file mode 100644 index 000..af8d9e4 --- /dev/null +++ b/doc/README.bootgraph @@ -0,0 +1,57 @@ +Bootgraph Instrumentation +- + +The CONFIG_BOOT_TRACE configuration option can be defined to output extensive +instrumentation to assist in boot time reduction. The tools/bootgraph.pl script +can utilise this instrumentation to generate an SVG graph showing where UBoot +spends its time. + +When enabled all printf output is prefixed with timing information similar +to the Linux kernel's CONFIG_PRINTK_TIME option. This allows you to measure the +interval between operations which is useful for identifying long delays during +UBoot operation. For example: + +[3.133000] ## Booting kernel from Legacy Image at 8200 ... +[3.139000]Image Name: Angstrom/2.6.32/beagleboard + +When enabled additional console output will be generated - this output includes +the addresses of executed commands and instrumented functions. For example: + +[2.456000] initcall 0x9ff86814 returned +[2.46] calling 0x9ff7c338 +[2.59] initcall 0x9ff7c338 returned +[2.594000] calling 0x9ff864ac + +At present executed commands are only displayed when the HUSH parser +(CONFIG_SYS_HUSH_PARSER) is not being used. + +Functions such as initialisation code can be instrumented by using the +DO_INITCALLXXX macros found in include/common.h (see arch/arm/lib/board.c) for +examples). For example: + +@@ -508,7 +518,7 @@ void board_init_r (gd_t *id, ulong dest_addr) + + #ifdef CONFIG_GENERIC_MMC +puts(MMC: ); +- mmc_initialize(bd); ++ DO_INITCALL(mmc_initialize, bd); + #endif + +For functions that do not return such as do_bootm function, the DO_INITCALL_END +macro can be used to mark the latest point of the function. + +When CONFIG_BOOT_TIME is not defined the DO_INITCALLXXX macros continue to call +the intended function and should not add overhead. + +An SVG graph can be generated which allows for the visualisation of UBoot boot +time by using the tools/bootgraph.pl script. This script has been ported from +the Linux kernel (scripts/bootgraph.pl) and is used in a similar way. For +example: + +cat console_output | perl ./tools/bootgraph.pl graph.svg + +Assuming console_output is a file containing the console output of UBoot with +CONFIG_BOOT_TRACE enabled - the graph.svg file should provide a graphical +representation of boot time with resolved function addresses. + +Andrew Murray amur...@theiet.org -- 1.7.4.1 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 0/7] Bootgraph.pl instrumentation support for UBoot
On 1 September 2011 00:12, Simon Glass s...@chromium.org wrote: Hi Mike, On Wed, Aug 31, 2011 at 3:47 PM, Mike Frysinger vap...@gentoo.org wrote: On Wednesday, August 31, 2011 18:20:54 Andrew Murray wrote: This patchset introduces the CONFIG_BOOT_TRACE option which provides support for boot time instrumentation. When enabled printf output is prefixed with timing information (similar to the kernel's CONFIG_PRINTK_TIME option) and additional output is generated which instruments functions and commands called (much like the kernel's initcall_debug functionality). The kernel's bootgraph.pl script has been ported to render UBoots instrumented ouptut into a pretty SVG graph. An example of this can be found here: http://goo.gl/dX8aR - which shows the boot time of a Beagle board. The patch currently provides support for instrumentation of UBoot commands (e.g. U_BOOT_CMD) for all platforms but only when the HUSH shell is not in use. Initialisation instrumentation is only limited to the arch/arm/lib/board.c file at present but can very easily be extended to other relevant files. i feel like we've had similar ideas tossed around semi-recently. am i just misremembering ? -mike Yes, for example: http://patchwork.ozlabs.org/patch/95513/ It got caught up with a big discussion about whether we want a microsecond timer. There is now one in Tegra, but not in the generic timer API. There was also a request to unify this with the boot_progress stuff (i.e. it turned into a big cleanup). I haven't got back to it yet, but could probably do something next week. I also have patches to pass the timings to the kernel and have it report them to user space through a device. Planning to send an RFC to the LKML about that probably next week as well. Could be fun. Regards, Simon Ah - my bad. I only subscribed to the mailing list today (my first UBoot patch) and didn't notice this previous work. Is there any cross over between my approach and what is planned/already been done? Andrew Murray ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/7] Add timing information to printf's for use with bootgraph.pl
On 31 August 2011 23:47, Mike Frysinger vap...@gentoo.org wrote: On Wednesday, August 31, 2011 18:20:57 Andrew Murray wrote: va_list args; uint i; char printbuffer[CONFIG_SYS_PBSIZE]; + char *buf = printbuffer; va_start(args, fmt); +#if defined(CONFIG_BOOT_TRACE) + unsigned long long ticks = get_ticks(); + int secs = ticks / get_tbclk(); + int msec = ((ticks * 100) / get_tbclk()) - (secs * 100); + + i += sprintf(buf, [%5lu.%06lu] , secs, msec); + buf += i; +#endif + /* For this to work, printbuffer must be larger than * anything we ever want to print. */ - i = vsprintf(printbuffer, fmt, args); + i += vsprintf(buf, fmt, args); va_end(args); NAK for a few reasons: - i dont see how this could possibly compile warning free - you never initialize i, only added to it - you call va_start() inbetween variable decls -mike Yes this does look dreadful - I'll update the patch pending outcome of thread with Simon Glass and existing work in this area. Andrew Murray ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/7] Add macros for recording init calls during UBoot execution
On 31 August 2011 23:50, Mike Frysinger vap...@gentoo.org wrote: On Wednesday, August 31, 2011 18:20:56 Andrew Murray wrote: +#if defined(CONFIG_BOOT_TRACE) +#define DO_INITCALL(x, ...) \ + do { \ + printf(calling 0x%pF\n, x); \ + (x)(__VA_ARGS__); \ + printf(initcall 0x%pF returned\n, x); \ + } while (0) are there any void initcalls ? or just ones where you ignore the value ? otherwise we can simply rename DO_INITCALL_RET() to DO_INITCALL(). I guess it depends what you define as an initcall. A lot of functions called during startup (e.g. arch/arm/lib/board.c) return int and in most cases that value is ignored - but there are occasions where void is returned - e.g. env_relocate. For simplicity you could limit the use cases for this macro to those which just return int and take your proposed approach? Andrew Murray ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/7] Add timing information to printf's for use with bootgraph.pl
On 1 September 2011 00:38, Graeme Russ graeme.r...@gmail.com wrote: Hi Mike, Andrew, On Thu, Sep 1, 2011 at 8:47 AM, Mike Frysinger vap...@gentoo.org wrote: On Wednesday, August 31, 2011 18:20:57 Andrew Murray wrote: va_list args; uint i; char printbuffer[CONFIG_SYS_PBSIZE]; + char *buf = printbuffer; va_start(args, fmt); +#if defined(CONFIG_BOOT_TRACE) + unsigned long long ticks = get_ticks(); + int secs = ticks / get_tbclk(); + int msec = ((ticks * 100) / get_tbclk()) - (secs * 100); + + i += sprintf(buf, [%5lu.%06lu] , secs, msec); + buf += i; +#endif + /* For this to work, printbuffer must be larger than * anything we ever want to print. */ - i = vsprintf(printbuffer, fmt, args); + i += vsprintf(buf, fmt, args); va_end(args); NAK for a few reasons: - i dont see how this could possibly compile warning free - you never initialize i, only added to it - you call va_start() inbetween variable decls And correct me if I'm wrong, but EVERY printf() will get the timing info tacked on - Even the ones without \n which are intermediate prints in larger messages which is going to lead to very ugly outputs I think instead we should look at another 'printf() with timestamp' function which can be used on an as-needed basis Yes that's exactly the case ... e.g. [3.122000] initcall 0x9ff864cc returned [3.126000] calling 0x9ff78174 [3.129000] ## Booting kernel from Legacy Image at 8200 ... [3.135000]Image Name: Angstrom/2.6.32/beagleboard [3.141000]Image Type: [3.143000] ARM Linux Kernel Image (uncompressed) [3.148000]Data Size:[3.151000] 3194192 Bytes = [ 3.154000] 3[3.155000] MiB It's certainty ugly. A dedicated printf seems very sensible. Andrew Murray ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 0/7] Bootgraph.pl instrumentation support for UBoot
On 1 September 2011 00:39, Simon Glass s...@chromium.org wrote: Is there any cross over between my approach and what is planned/already been done? Don't worry - your contribution is very welcome! Yes I think there is cross-over, and perhaps the right approach is to try to merge them somehow. It is great to get graphs out of the code and it really helps with analysis. My interest was mainly in monitoring boot time in the field rather than in the lab. But we should have one framework for both. [SNIP] I will assume that we have a microsecond timer, update my patch and resubmit so you can take a look and see what you think. Hopefully we can unify this, your patch and the boot_progress stuff. Excellent! OK, well I will await the patch, read up on the original intentions and we can go from there. I'll look forward to making UBoot more boot time friendly. Good night. Andrew Murray ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot