[U-Boot] [PATCH] integrator: switch to DM serial port

[Simon Glass]

Hi Linus,

On 27 July 2015 at 03:19, Linus Walleij <linus.walleij at linaro.org> wrote:
> This switches the Integrator boards over to using the device model
> for its serial ports.
>
> Cc: Masahiro Yamada <yamada.masahiro at socionext.com>
> Signed-off-by: Linus Walleij <linus.walleij at linaro.org>
> ---
> ChangeLog v1->v2:
> - Moved CONFIG_DM and CONFIG_DM_SERIAL to Kconfig selects
> - Moved CONFIG_SYS_MALLOC_F_LEN to Kconfig option
> ---

Reviewed-by: Simon Glass <sjg at chromium.org>

But a few questions below.

>  arch/arm/Kconfig                     |  2 ++
>  arch/arm/mach-integrator/Kconfig     |  3 +++
>  board/armltd/integrator/integrator.c | 18 ++++++++++++++++++
>  include/configs/integrator-common.h  |  5 +++++
>  include/configs/integratorap.h       | 11 -----------
>  include/configs/integratorcp.h       |  9 ---------
>  6 files changed, 28 insertions(+), 20 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 3355b3bcaa58..04006176553c 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -457,6 +457,8 @@ config ARCH_HIGHBANK
>
>  config ARCH_INTEGRATOR
>         bool "ARM Ltd. Integrator family"
> +       select DM
> +       select DM_SERIAL
>
>  config ARCH_KEYSTONE
>         bool "TI Keystone"
> diff --git a/arch/arm/mach-integrator/Kconfig b/arch/arm/mach-integrator/Kconfig
> index c54d69db0e42..d506ee5b39cd 100644
> --- a/arch/arm/mach-integrator/Kconfig
> +++ b/arch/arm/mach-integrator/Kconfig
> @@ -53,4 +53,7 @@ config SYS_CONFIG_NAME
>         default "integratorap" if ARCH_INTEGRATOR_AP
>         default "integratorcp" if ARCH_INTEGRATOR_CP
>
> +config SYS_MALLOC_F_LEN
> +       default 0x2000

Why is this needed? Are you allocating a lot of stuff?

> +
>  endmenu
> diff --git a/board/armltd/integrator/integrator.c b/board/armltd/integrator/integrator.c
> index e94ac850c751..cbe706170d0f 100644
> --- a/board/armltd/integrator/integrator.c
> +++ b/board/armltd/integrator/integrator.c
> @@ -20,11 +20,29 @@
>  #include <common.h>
>  #include <netdev.h>
>  #include <asm/io.h>
> +#include <dm/platdata.h>
> +#include <dm/platform_data/serial_pl01x.h>
>  #include "arm-ebi.h"
>  #include "integrator-sc.h"
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> +static const struct pl01x_serial_platdata serial_platdata = {
> +       .base = 0x16000000,
> +#ifdef CONFIG_ARCH_CINTEGRATOR
> +       .type = TYPE_PL011,
> +       .clock = 14745600,
> +#else
> +       .type = TYPE_PL010,
> +       .clock = 0, /* Not used for PL010 */

You can leave out the zero, I'd suggest:

+       /* Clock not used for PL010 */


> +#endif
> +};
> +
> +U_BOOT_DEVICE(integrator_serials) = {
> +       .name = "serial_pl01x",
> +       .platdata = &serial_platdata,
> +};
> +
>  void peripheral_power_enable (void);
>
>  #if defined(CONFIG_SHOW_BOOT_PROGRESS)
> diff --git a/include/configs/integrator-common.h b/include/configs/integrator-common.h
> index 1d307ca1e6a8..a22efd699c17 100644
> --- a/include/configs/integrator-common.h
> +++ b/include/configs/integrator-common.h
> @@ -20,6 +20,11 @@
>  #define CONFIG_SYS_BARGSIZE            CONFIG_SYS_CBSIZE /* Boot Argument Buffer Size*/
>  #define CONFIG_SYS_MALLOC_LEN          (CONFIG_ENV_SIZE + 128*1024) /* Size of malloc() pool */
>
> +/* Serial port PL010/PL011 through the device model */

driver model

> +#define CONFIG_PL01X_SERIAL
> +#define CONFIG_BAUDRATE                        38400
> +#define CONFIG_CONS_INDEX              0

Do you need that because there is no device tree?

> +
>  #define CONFIG_CMDLINE_TAG             /* enable passing of ATAGs  */
>  #define CONFIG_SETUP_MEMORY_TAGS
>  #define CONFIG_OF_LIBFDT               /* enable passing a Device Tree */
> diff --git a/include/configs/integratorap.h b/include/configs/integratorap.h
> index c76ebcbb60a9..26aa8cadc01d 100644
> --- a/include/configs/integratorap.h
> +++ b/include/configs/integratorap.h
> @@ -21,17 +21,6 @@
>  #define CONFIG_SYS_HZ_CLOCK            24000000        /* Timer 1 is clocked at 24Mhz */
>
>  /*
> - * PL010 Configuration
> - */
> -#define CONFIG_PL010_SERIAL
> -#define CONFIG_CONS_INDEX      0
> -#define CONFIG_BAUDRATE                38400
> -#define CONFIG_PL01x_PORTS     { (void *) (CONFIG_SYS_SERIAL0), (void *) (CONFIG_SYS_SERIAL1) }
> -#define CONFIG_SYS_SERIAL0             0x16000000
> -#define CONFIG_SYS_SERIAL1             0x17000000
> -
> -
> -/*
>   * BOOTP options
>   */
>  #define CONFIG_BOOTP_BOOTFILESIZE
> diff --git a/include/configs/integratorcp.h b/include/configs/integratorcp.h
> index d6f260287534..4d04f544c815 100644
> --- a/include/configs/integratorcp.h
> +++ b/include/configs/integratorcp.h
> @@ -28,15 +28,6 @@
>  #define CONFIG_SMC91111_BASE    0xC8000000
>  #undef CONFIG_SMC91111_EXT_PHY
>
> -/* PL011 configuration */
> -#define CONFIG_PL011_SERIAL
> -#define CONFIG_PL011_CLOCK     14745600
> -#define CONFIG_PL01x_PORTS     { (void *)CONFIG_SYS_SERIAL0, (void *)CONFIG_SYS_SERIAL1 }
> -#define CONFIG_CONS_INDEX      0
> -#define CONFIG_BAUDRATE                38400
> -#define CONFIG_SYS_SERIAL0             0x16000000
> -#define CONFIG_SYS_SERIAL1             0x17000000
> -
>  /*
>   * Command line configuration.
>   */
> --
> 2.4.3
>

Regards,
Simon