On Wed, Sep 9, 2015 at 7:52 AM, Simon Glass sjg@chromium.org wrote:

Now that PCI bridges are probed before relocation we need additional memory. Each PCI bridge takes 240 bytes at present since it uses the same uclass as the PCI controller. Probably we should split this out so that bridges have their own uclass.

Expand the memory on link so that it works correctly.

Signed-off-by: Simon Glass sjg@chromium.org

configs/chromebook_link_defconfig | 1 + 1 file changed, 1 insertion(+)

diff --git a/configs/chromebook_link_defconfig b/configs/chromebook_link_defconfig index 9855736..b3ae925 100644 --- a/configs/chromebook_link_defconfig +++ b/configs/chromebook_link_defconfig @@ -1,4 +1,5 @@ CONFIG_X86=y +CONFIG_SYS_MALLOC_F_LEN=0x1800 CONFIG_VENDOR_GOOGLE=y CONFIG_DEFAULT_DEVICE_TREE="chromebook_link" CONFIG_TARGET_CHROMEBOOK_LINK=y --

Reviewed-by: Bin Meng bmeng.cn@gmail.com

On 14 September 2015 at 06:15, Bin Meng bmeng.cn@gmail.com wrote:

On Wed, Sep 9, 2015 at 7:52 AM, Simon Glass sjg@chromium.org wrote:

...

It's useful to get a a trace of memory allocations in early init. Add a debug() call to provide that. It can be enabled by adding '#define DEBUG' to the top of the file.

Signed-off-by: Simon Glass sjg@chromium.org

common/malloc_simple.c | 4 ++++ 1 file changed, 4 insertions(+)

diff --git a/common/malloc_simple.c b/common/malloc_simple.c index 134e059..e357827 100644 --- a/common/malloc_simple.c +++ b/common/malloc_simple.c @@ -19,10 +19,13 @@ void *malloc_simple(size_t bytes) void *ptr;

    new_ptr = gd->malloc_ptr + bytes;

•   debug("%s: size=%zx, ptr=%lx, limit=%lx\n", __func__, bytes, new_ptr,
  

•         gd->malloc_limit);
    if (new_ptr > gd->malloc_limit)
            return NULL;
    ptr = map_sysmem(gd->malloc_base + gd->malloc_ptr, bytes);
    gd->malloc_ptr = ALIGN(new_ptr, sizeof(new_ptr));
  

•   return ptr;
  

}

@@ -37,6 +40,7 @@ void *memalign_simple(size_t align, size_t bytes) return NULL; ptr = map_sysmem(addr, bytes); gd->malloc_ptr = ALIGN(new_ptr, sizeof(new_ptr));

•   return ptr;
  

}

--

Reviewed-by: Bin Meng bmeng.cn@gmail.com

Applied to u-boot-x86.

On Wed, Sep 9, 2015 at 7:52 AM, Simon Glass sjg@chromium.org wrote:

When the auto-configuration process fails for a device (generally due to lack of memory) we should return the error correctly so that we don't continue to try memory allocations which will fail.

Adjust the code to check for errors and abort if something goes wrong.

Signed-off-by: Simon Glass sjg@chromium.org

drivers/pci/pci-uclass.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index b25298f..437c81a 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -381,9 +381,13 @@ int pci_auto_config_devices(struct udevice *bus) !ret && dev; ret = device_find_next_child(&dev)) { unsigned int max_bus;

•           int ret;
  
            debug("%s: device %s\n", __func__, dev->name);
  

•           max_bus = pciauto_config_device(hose, pci_get_bdf(dev));
  

•           ret = pciauto_config_device(hose, pci_get_bdf(dev));
  

•           if (ret < 0)
  

•                   return ret;
  

•           max_bus = ret;
            sub_bus = max(sub_bus, max_bus);
    }
    debug("%s: done\n", __func__);
  

@@ -757,6 +761,8 @@ static int pci_uclass_post_probe(struct udevice *bus)

#ifdef CONFIG_PCI_PNP ret = pci_auto_config_devices(bus);

•   if (ret < 0)
  

•           return ret;
  

#endif

#if defined(CONFIG_X86) && defined(CONFIG_HAVE_FSP) @@ -773,11 +779,14 @@ static int pci_uclass_post_probe(struct udevice *bus) * Note we only call this 1) after U-Boot is relocated, and 2) * root bus has finished probing. */

•   if ((gd->flags & GD_FLG_RELOC) && (bus->seq == 0))
  

•   if ((gd->flags & GD_FLG_RELOC) && (bus->seq == 0)) {
            ret = fsp_init_phase_pci();
  

•           if (ret)
  

•                   return ret;
  

•   }
  

#endif

•   return ret < 0 ? ret : 0;
  

•   return 0;
  

}

static int pci_uclass_child_post_bind(struct udevice *dev)

Reviewed-by: Bin Meng bmeng.cn@gmail.com

On 14 September 2015 at 06:15, Bin Meng bmeng.cn@gmail.com wrote:

On Wed, Sep 9, 2015 at 7:52 AM, Simon Glass sjg@chromium.org wrote:

...

One debug() statement is missing a newline. The other has a repeated word. Fix these.

Signed-off-by: Simon Glass sjg@chromium.org

drivers/pci/pci-uclass.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 437c81a..7347160 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -418,7 +418,7 @@ int dm_pci_hose_probe_bus(struct pci_controller *hose, pci_dev_t bdf)

    ret = device_probe(bus);
    if (ret) {

•           debug("%s: Cannot probe bus bus %s: %d\n", __func__, bus->name,
  

•           debug("%s: Cannot probe bus %s: %d\n", __func__, bus->name,
                  ret);
            return ret;
    }
  

@@ -537,7 +537,7 @@ static int pci_find_and_bind_driver(struct udevice *parent,

    ret = device_bind_driver(parent, drv, str, devp);
    if (ret) {

•           debug("%s: Failed to bind generic driver: %d", __func__, ret);
  

•           debug("%s: Failed to bind generic driver: %d\n", __func__, ret);
            return ret;
    }
    debug("%s: No match found: bound generic driver instead\n", __func__);
  

--

Reviewed-by: Bin Meng bmeng.cn@gmail.com

Applied to u-boot-x86.

On Wed, Sep 9, 2015 at 7:52 AM, Simon Glass sjg@chromium.org wrote:

The current code returns 0 even if it failed to find or bind a driver. The caller then has to check the returned device to see if it is NULL. It is better to return an error code in this case so that it is clear what happened.

Adjust the code to return -EPERM, indicating that the device was not bound because it is not needed for pre-relocation use. Add comments so that the return value is clear.

Signed-off-by: Simon Glass sjg@chromium.org

drivers/pci/pci-uclass.c | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 7347160..54285ee 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -458,10 +458,17 @@ static bool pci_match_one_id(const struct pci_device_id *id,

• pci_find_and_bind_driver() - Find and bind the right PCI driver
• This only looks at certain fields in the descriptor.

• • @parent: Parent bus
• • @find_id: Specification of the driver to find
• • @bdf: Bus/device/function addreess - see PCI_BDF()
• • @devp: Returns a pointer to the device created
• • @return 0 if OK, -EPERM if the device is not needed before relocation and

• •    therefore was not created, other -ve value on error
    

  */

static int pci_find_and_bind_driver(struct udevice *parent,

•                               struct pci_device_id *find_id, pci_dev_t bdf,
  

•                               struct udevice **devp)
  

•                               struct pci_device_id *find_id,
  

•                               pci_dev_t bdf, struct udevice **devp)
  

{ struct pci_driver_entry *start, *entry; const char *drv; @@ -497,7 +504,7 @@ static int pci_find_and_bind_driver(struct udevice *parent, */ if (!(gd->flags & GD_FLG_RELOC) && !(drv->flags & DM_FLAG_PRE_RELOC))

•                           return 0;
  

•                           return -EPERM;
  
                    /*
                     * We could pass the descriptor to the driver as
  

@@ -525,7 +532,7 @@ static int pci_find_and_bind_driver(struct udevice *parent, * limited (ie: using Cache As RAM). */ if (!(gd->flags & GD_FLG_RELOC) && !bridge)

•           return 0;
  

•           return -EPERM;
  
    /* Bind a generic driver so that the device can be used */
    sprintf(name, "pci_%x:%x.%x", parent->seq, PCI_DEV(bdf),
  

@@ -613,17 +620,17 @@ int pci_bind_bus_devices(struct udevice *bus) ret = pci_find_and_bind_driver(bus, &find_id, bdf, &dev); }

•           if (ret)
  

•           if (ret == -EPERM)
  

•                   continue;
  

•           else if (ret)
                    return ret;
  
            /* Update the platform data */
  

•           if (dev) {
  

•                   pplat = dev_get_parent_platdata(dev);
  

•                   pplat->devfn = PCI_MASK_BUS(bdf);
  

•                   pplat->vendor = vendor;
  

•                   pplat->device = device;
  

•                   pplat->class = class;
  

•           }
  

•           pplat = dev_get_parent_platdata(dev);
  

•           pplat->devfn = PCI_MASK_BUS(bdf);
  

•           pplat->vendor = vendor;
  

•           pplat->device = device;
  

•           pplat->class = class;
    }
  
    return 0;
  

--

Reviewed-by: Bin Meng bmeng.cn@gmail.com

Hi Bin,

On 14 September 2015 at 06:16, Bin Meng bmeng.cn@gmail.com wrote:

Hi Simon,

On Wed, Sep 9, 2015 at 7:52 AM, Simon Glass sjg@chromium.org wrote:

...

We want to be able to add other common code to this function. So change the driver's version to have an underscore before it, just like _debug_uart_putc(). Define debug_uart_init() to call this version.

Update all drivers to this new method.

Signed-off-by: Simon Glass sjg@chromium.org

drivers/serial/ns16550.c | 2 +- drivers/serial/serial_efi.c | 2 +- drivers/serial/serial_s5p.c | 2 +- include/debug_uart.h | 9 +++++++-- lib/efi/efi_stub.c | 2 +- 5 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index 6275a11..6433844 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -257,7 +257,7 @@ int NS16550_tstc(NS16550_t com_port) (1 << CONFIG_DEBUG_UART_SHIFT), \ CONFIG_DEBUG_UART_SHIFT)

-void debug_uart_init(void) +static inline void _debug_uart_init(void)

Is this 'static inline' change needed? As I don't see the same changes applied to serial_efi.c and serial_s5p.c

Yes I've added it to those two - it is not essential but I think it makes sense since there is only one declaration and we want to avoid unnecessary function calls.

...

{ struct NS16550 *com_port = (struct NS16550 *)CONFIG_DEBUG_UART_BASE; int baud_divisor; diff --git a/drivers/serial/serial_efi.c b/drivers/serial/serial_efi.c index cf57d89..ee3029b 100644 --- a/drivers/serial/serial_efi.c +++ b/drivers/serial/serial_efi.c @@ -107,7 +107,7 @@ static int serial_efi_pending(struct udevice *dev, bool input)

• There is nothing to init here since the EFI console is already running by
• the time we enter U-Boot.

*/ -void debug_uart_init(void) +void _debug_uart_init(void) { }

diff --git a/drivers/serial/serial_s5p.c b/drivers/serial/serial_s5p.c index 3f0b588..e7adf0d 100644 --- a/drivers/serial/serial_s5p.c +++ b/drivers/serial/serial_s5p.c @@ -207,7 +207,7 @@ U_BOOT_DRIVER(serial_s5p) = {

#include <debug_uart.h>

-void debug_uart_init(void) +void _debug_uart_init(void) { struct s5p_uart *uart = (struct s5p_uart *)CONFIG_DEBUG_UART_BASE;

diff --git a/include/debug_uart.h b/include/debug_uart.h index a75e377..257ba00 100644 --- a/include/debug_uart.h +++ b/include/debug_uart.h @@ -38,7 +38,7 @@

• To enable the debug UART in your serial driver:
• • #include <debug_uart.h>

• • • Define debug_uart_init(), trying to avoid using the stack

• • • Define _debug_uart_init(), trying to avoid using the stack
  • • Define _debug_uart_putc() as static inline (avoiding stack usage)
  • • Immediately afterwards, add DEBUG_UART_FUNCS to define the rest of the

  • functionality (printch(), etc.)
    

@@ -132,6 +132,11 @@ void printhex8(uint value); void printhex8(uint value) \ { \ printhex(value, 8); \

•   }
  

•   } \
  

+\

•   void debug_uart_init(void) \
  

•   { \
  

•           _debug_uart_init(); \
  

•   } \
  
  

#endif diff --git a/lib/efi/efi_stub.c b/lib/efi/efi_stub.c index d4d3e49..e138709 100644 --- a/lib/efi/efi_stub.c +++ b/lib/efi/efi_stub.c @@ -59,7 +59,7 @@ struct __packed desctab_info {

• considering if we start needing more U-Boot functionality. Note that we
• could then move get_codeseg32() to arch/x86/cpu/cpu.c.

*/ -void debug_uart_init(void) +void _debug_uart_init(void) { }

--

Regards, Bin

Regards, Simon

On Wed, Sep 9, 2015 at 7:52 AM, Simon Glass sjg@chromium.org wrote:

Some boards need to set things up before the debug UART can be used. On these boards a call to debug_uart_init() is insufficient. When this option is enabled, the function board_debug_uart_init() will be called when debug_uart_init() is called. You can put any code here that is needed to set up the UART ready for use, such as set pin multiplexing or enable clocks.

Signed-off-by: Simon Glass sjg@chromium.org

drivers/serial/Kconfig | 11 +++++++++++ include/debug_uart.h | 14 ++++++++++++++ 2 files changed, 25 insertions(+)

diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig index ccb80d2..4418a57 100644 --- a/drivers/serial/Kconfig +++ b/drivers/serial/Kconfig @@ -109,6 +109,17 @@ config DEBUG_UART_SHIFT value. Use this value to specify the shift to use, where 0=byte registers, 2=32-bit word registers, etc.

+config DEBUG_UART_BOARD_INIT

•   bool "Enable board-specific debug UART init"
  

•   depends on DEBUG_UART
  

•   help
  

•     Some boards need to set things up before the debug UART can be used.
  

•     On these boards a call to debug_uart_init() is insufficient. When
  

•     this option is enabled, the function board_debug_uart_init() will
  

•     be called when debug_uart_init() is called. You can put any code
  

•     here that is needed to set up the UART ready for use, such as set
  

•     pin multiplexing or enable clocks.
  

config ROCKCHIP_SERIAL bool "Rockchip on-chip UART support" depends on ARCH_UNIPHIER && DM_SERIAL diff --git a/include/debug_uart.h b/include/debug_uart.h index 257ba00..a6b7ce8 100644 --- a/include/debug_uart.h +++ b/include/debug_uart.h @@ -42,6 +42,11 @@

• • Define _debug_uart_putc() as static inline (avoiding stack usage)
• • Immediately afterwards, add DEBUG_UART_FUNCS to define the rest of the

• functionality (printch(), etc.)
  

• • If your board needs additional init for the UART to work, enable
• • CONFIG_DEBUG_UART_BOARD_INIT and write a function called
• • board_debug_uart_init() to perform that init. When debug_uart_init() is
• • called, the init will happen automatically.
  */

/** @@ -57,6 +62,14 @@ */ void debug_uart_init(void);

+#ifdef CONFIG_DEBUG_UART_BOARD_INIT +void board_debug_uart_init(void); +#else +static inline void board_debug_uart_init(void) +{ +} +#endif

/**

• printch() - Output a character to the debug UART

@@ -136,6 +149,7 @@ void printhex8(uint value); \ void debug_uart_init(void) \ { \

•           board_debug_uart_init(); \
            _debug_uart_init(); \
    } \
  
  

--

Reviewed-by: Bin Meng bmeng.cn@gmail.com

On Wed, Sep 9, 2015 at 7:52 AM, Simon Glass sjg@chromium.org wrote:

It is useful to see a message from the debug UART early during boot so that you know things are working. Add an option to enable this. The message will be displayed as soon as debug_uart_init() is called.

Signed-off-by: Simon Glass sjg@chromium.org

drivers/serial/Kconfig | 10 ++++++++++ include/debug_uart.h | 7 +++++++ 2 files changed, 17 insertions(+)

diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig index 4418a57..fa03d17 100644 --- a/drivers/serial/Kconfig +++ b/drivers/serial/Kconfig @@ -120,6 +120,16 @@ config DEBUG_UART_BOARD_INIT here that is needed to set up the UART ready for use, such as set pin multiplexing or enable clocks.

+config DEBUG_UART_ANNOUNCE

•   bool "Show a message when the debug UART starts up"
  

•   depends on DEBUG_UART
  

•   help
  

•     Enable this option to show a message when the debug UART is ready
  

•     for use. You will see a message like "<debug_uart> " as soon as
  

•     U-Boot has the UART ready for use (i.e. your code calls
  

•     debug_uart_init()). This can be useful just as a check that
  

•     everything is working.
  

config ROCKCHIP_SERIAL bool "Rockchip on-chip UART support" depends on ARCH_UNIPHIER && DM_SERIAL diff --git a/include/debug_uart.h b/include/debug_uart.h index a6b7ce8..5d5349b 100644 --- a/include/debug_uart.h +++ b/include/debug_uart.h @@ -105,6 +105,12 @@ void printhex4(uint value); */ void printhex8(uint value);

+#ifdef CONFIG_DEBUG_UART_ANNOUNCE +#define _DEBUG_UART_ANNOUNCE printascii("<debug_uart> "); +#else +#define _DEBUG_UART_ANNOUNCE +#endif

/*

• Now define some functions - this should be inserted into the serial driver

*/ @@ -151,6 +157,7 @@ void printhex8(uint value); { \ board_debug_uart_init(); \ _debug_uart_init(); \

•           _DEBUG_UART_ANNOUNCE \
    } \
  
  

#endif

Reviewed-by: Bin Meng bmeng.cn@gmail.com

Hi Simon,

On Wed, Sep 9, 2015 at 7:52 AM, Simon Glass sjg@chromium.org wrote:

If the debug UART is enabled, get it ready for use at the earliest possible opportunity. This is not actually very early, but until we have a stack it is difficult to make it work.

Signed-off-by: Simon Glass sjg@chromium.org

arch/x86/cpu/start.S | 3 +++ 1 file changed, 3 insertions(+)

diff --git a/arch/x86/cpu/start.S b/arch/x86/cpu/start.S index d072825..e6e9cb5 100644 --- a/arch/x86/cpu/start.S +++ b/arch/x86/cpu/start.S @@ -134,6 +134,9 @@ car_init_ret: * mov $'a', %eax * call printch */

Can we remove the comment block above since it is just duplicate to the codes below?

+#ifdef CONFIG_DEBUG_UART

•   call  debug_uart_init
  

nits: please use a 'tab' after 'call'.

+#endif

    /* Get address of global_data */
    mov     %fs:0, %edx

--

Regards, Bin

On Wed, Sep 9, 2015 at 7:52 AM, Simon Glass sjg@chromium.org wrote:

Add support for the debug UART on link. This is useful for early debugging.

Signed-off-by: Simon Glass sjg@chromium.org

arch/x86/cpu/ivybridge/cpu.c | 7 +++++++ configs/chromebook_link_defconfig | 10 +++++++--- 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/arch/x86/cpu/ivybridge/cpu.c b/arch/x86/cpu/ivybridge/cpu.c index cce5923..0e6512c 100644 --- a/arch/x86/cpu/ivybridge/cpu.c +++ b/arch/x86/cpu/ivybridge/cpu.c @@ -340,3 +340,10 @@ int print_cpuinfo(void)

    return 0;

}

+void board_debug_uart_init(void) +{

•   /* This enables the debug UART */
  

•   pci_x86_write_config(NULL, PCH_LPC_DEV, LPC_EN, COMA_LPC_EN,
  

•                        PCI_SIZE_16);
  

+} diff --git a/configs/chromebook_link_defconfig b/configs/chromebook_link_defconfig index b3ae925..7408cb1 100644 --- a/configs/chromebook_link_defconfig +++ b/configs/chromebook_link_defconfig @@ -15,17 +15,21 @@ CONFIG_CMD_BOOTSTAGE=y CONFIG_CMD_TPM=y CONFIG_CMD_TPM_TEST=y CONFIG_OF_CONTROL=y -CONFIG_DM_PCI=y -CONFIG_SPI_FLASH=y CONFIG_CMD_CROS_EC=y CONFIG_CROS_EC=y CONFIG_CROS_EC_LPC=y +CONFIG_SPI_FLASH=y +CONFIG_DM_PCI=y +CONFIG_DM_RTC=y +CONFIG_DEBUG_UART=y +CONFIG_DEBUG_UART_BASE=0x3f8 +CONFIG_DEBUG_UART_CLOCK=1843200 +CONFIG_DEBUG_UART_BOARD_INIT=y CONFIG_DM_TPM=y CONFIG_TPM_TIS_LPC=y CONFIG_VIDEO_VESA=y CONFIG_FRAMEBUFFER_SET_VESA_MODE=y CONFIG_FRAMEBUFFER_VESA_MODE_11A=y -CONFIG_DM_RTC=y CONFIG_USE_PRIVATE_LIBGCC=y CONFIG_SYS_VSNPRINTF=y CONFIG_TPM=y --

Reviewed-by: Bin Meng bmeng.cn@gmail.com