Re: [U-Boot] Antw: Re: U-Boot doesn't silent the output

2014-03-18 Thread Andrew Murray
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

2014-03-18 Thread Andrew Murray
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

2014-03-18 Thread Andrew Murray
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

2014-02-17 Thread Andrew Murray
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

2013-10-09 Thread Andrew Murray
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

2013-10-01 Thread Andrew Murray
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

2013-09-29 Thread 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
---
 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

2013-09-29 Thread Andrew Murray
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

2013-09-13 Thread 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
---
 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

2013-09-05 Thread Andrew Murray
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

2013-08-30 Thread Andrew Murray
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

2013-08-24 Thread Andrew Murray
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

2013-08-24 Thread Andrew Murray
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

2013-08-23 Thread Andrew Murray
, 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

2011-09-11 Thread Andrew Murray
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

2011-09-11 Thread Andrew Murray
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

2011-09-10 Thread Andrew Murray
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

2011-09-10 Thread Andrew Murray
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

2011-09-10 Thread Andrew Murray
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

2011-09-10 Thread Andrew Murray
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

2011-09-10 Thread Andrew Murray
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

2011-09-10 Thread Andrew Murray
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

2011-09-10 Thread Andrew Murray
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

2011-09-10 Thread Andrew Murray
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

2011-09-10 Thread Andrew Murray
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

2011-09-10 Thread Andrew Murray
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

2011-09-10 Thread Andrew Murray
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?

2011-09-02 Thread Andrew Murray
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

2011-08-31 Thread Andrew Murray
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

2011-08-31 Thread Andrew Murray
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

2011-08-31 Thread Andrew Murray
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

2011-08-31 Thread Andrew Murray
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

2011-08-31 Thread Andrew Murray
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

2011-08-31 Thread Andrew Murray
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

2011-08-31 Thread Andrew Murray
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

2011-08-31 Thread Andrew Murray
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

2011-08-31 Thread Andrew Murray
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

2011-08-31 Thread Andrew Murray
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

2011-08-31 Thread Andrew Murray
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

2011-08-31 Thread Andrew Murray
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

2011-08-31 Thread Andrew Murray
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