Re: [Qemu-arm] [PATCH v3 08/10] aspeed: add a AST2500 SoC and support to the SCU and SDMC controllers controllers

[Peter Maydell]

On 2 August 2016 at 18:15, Cédric Le Goater <address@hidden> wrote:
> Based on previous work done by Andrew Jeffery <address@hidden>.
>
> Signed-off-by: Cédric Le Goater <address@hidden>
> ---
>
>  Changes since v2:
>
>  - more precise definitions of the hw-strap1 register
>
>  hw/arm/aspeed_soc.c          |  2 ++
>  hw/misc/aspeed_scu.c         | 45 +++++++++++++++++++++++++-
>  hw/misc/aspeed_sdmc.c        |  1 +
>  include/hw/misc/aspeed_scu.h | 77 
> +++++++++++++++++++++++++++++++++++++++++++-
>  4 files changed, 123 insertions(+), 2 deletions(-)
>
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index ec6ec3546908..2408dfe70c51 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -38,10 +38,12 @@ static const int uart_irqs[] = { 9, 32, 33, 34, 10 };
>  static const int timer_irqs[] = { 16, 17, 18, 35, 36, 37, 38, 39, };
>
>  #define AST2400_SDRAM_BASE       0x40000000
> +#define AST2500_SDRAM_BASE       0x80000000
>
>  static const AspeedSoCInfo aspeed_socs[] = {
>      { "ast2400-a0", "arm926", AST2400_A0_SILICON_REV, AST2400_SDRAM_BASE },
>      { "ast2400",    "arm926", AST2400_A0_SILICON_REV, AST2400_SDRAM_BASE },
> +    { "ast2500-a1", "arm1176", AST2500_A1_SILICON_REV, AST2500_SDRAM_BASE },
>  };
>
>  /*
> diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
> index c7e2c8263f55..6dd7e1085420 100644
> --- a/hw/misc/aspeed_scu.c
> +++ b/hw/misc/aspeed_scu.c
> @@ -120,6 +120,41 @@ static const uint32_t 
> ast2400_a0_resets[ASPEED_SCU_NR_REGS] = {
>       [BMC_DEV_ID]      = 0x00002402U
>  };
>
> +/* SCU70 bit 23: 0 24Mhz. bit 11:9: 0b001 AXI:ABH ratio 2:1 */
> +/* AST2500 revision A1 */
> +
> +static const uint32_t ast2500_a1_resets[ASPEED_SCU_NR_REGS] = {
> +     [SYS_RST_CTRL]    = 0xFFCFFEDCU,
> +     [CLK_SEL]         = 0xF3F40000U,
> +     [CLK_STOP_CTRL]   = 0x19FC3E8BU,
> +     [D2PLL_PARAM]     = 0x00026108U,
> +     [MPLL_PARAM]      = 0x00030291U,
> +     [HPLL_PARAM]      = 0x93000400U,
> +     [MISC_CTRL1]      = 0x00000010U,
> +     [PCI_CTRL1]       = 0x20001A03U,
> +     [PCI_CTRL2]       = 0x20001A03U,
> +     [PCI_CTRL3]       = 0x04000030U,
> +     [SYS_RST_STATUS]  = 0x00000001U,
> +     [SOC_SCRATCH1]    = 0x000000C0U, /* SoC completed DRAM init */
> +     [MISC_CTRL2]      = 0x00000023U,
> +     [RNG_CTRL]        = 0x0000000EU,
> +     [PINMUX_CTRL2]    = 0x0000F000U,
> +     [PINMUX_CTRL3]    = 0x03000000U,
> +     [PINMUX_CTRL4]    = 0x00000000U,
> +     [PINMUX_CTRL5]    = 0x0000A000U,
> +     [WDT_RST_CTRL]    = 0x023FFFF3U,
> +     [PINMUX_CTRL8]    = 0xFFFF0000U,
> +     [PINMUX_CTRL9]    = 0x000FFFFFU,
> +     [FREE_CNTR4]      = 0x000000FFU,
> +     [FREE_CNTR4_EXT]  = 0x000000FFU,
> +     [CPU2_BASE_SEG1]  = 0x80000000U,
> +     [CPU2_BASE_SEG4]  = 0x1E600000U,
> +     [CPU2_BASE_SEG5]  = 0xC0000000U,
> +     [UART_HPLL_CLK]   = 0x00001903U,
> +     [PCIE_CTRL]       = 0x0000007BU,
> +     [BMC_DEV_ID]      = 0x00002402U
> +};
> +
>  static uint64_t aspeed_scu_read(void *opaque, hwaddr offset, unsigned size)
>  {
>      AspeedSCUState *s = ASPEED_SCU(opaque);
> @@ -198,6 +233,10 @@ static void aspeed_scu_reset(DeviceState *dev)
>      case AST2400_A0_SILICON_REV:
>          reset = ast2400_a0_resets;
>          break;
> +    case AST2500_A0_SILICON_REV:
> +    case AST2500_A1_SILICON_REV:
> +        reset = ast2500_a1_resets;
> +        break;
>      default:
>          g_assert_not_reached();
>      }
> @@ -208,7 +247,11 @@ static void aspeed_scu_reset(DeviceState *dev)
>      s->regs[HW_STRAP2] = s->hw_strap2;
>  }
>
> -static uint32_t aspeed_silicon_revs[] = { AST2400_A0_SILICON_REV, };
> +static uint32_t aspeed_silicon_revs[] = {
> +    AST2400_A0_SILICON_REV,
> +    AST2500_A0_SILICON_REV,
> +    AST2500_A1_SILICON_REV

Prefer trailing comma.

> +};
>
>  bool is_supported_silicon_rev(uint32_t silicon_rev)
>  {
> diff --git a/hw/misc/aspeed_sdmc.c b/hw/misc/aspeed_sdmc.c
> index 6cc0301a6331..621d166890fa 100644
> --- a/hw/misc/aspeed_sdmc.c
> +++ b/hw/misc/aspeed_sdmc.c
> @@ -196,6 +196,7 @@ static void aspeed_sdmc_reset(DeviceState *dev)
>          break;
>
>      case AST2500_A0_SILICON_REV:
> +    case AST2500_A1_SILICON_REV:
>          s->regs[R_CONF] |=
>              ASPEED_SDMC_HW_VERSION(1) |
>              ASPEED_SDMC_VGA_APERTURE(ASPEED_SDMC_VGA_64MB) |
> diff --git a/include/hw/misc/aspeed_scu.h b/include/hw/misc/aspeed_scu.h
> index 0761f0880c69..20d7559d3395 100644
> --- a/include/hw/misc/aspeed_scu.h
> +++ b/include/hw/misc/aspeed_scu.h
> @@ -33,6 +33,7 @@ typedef struct AspeedSCUState {
>
>  #define AST2400_A0_SILICON_REV   0x02000303U
>  #define AST2500_A0_SILICON_REV   0x04000303U
> +#define AST2500_A1_SILICON_REV   0x04010303U
>
>  extern bool is_supported_silicon_rev(uint32_t silicon_rev);
>
> @@ -53,7 +54,7 @@ extern bool is_supported_silicon_rev(uint32_t silicon_rev);
>   *       1. 2012/12/29 Ryan Chen Create
>   */
>
> -/* Hardware Strapping Register definition (for Aspeed AST2400 SOC)
> +/* Hardware Strapping Register definition (for Aspeed AST2400 SoC)

This change should be squashed into the patch which added the line.

>   *
>   * 31:29  Software defined strapping registers
>   * 28:27  DRAM size setting (for VGA driver use)
> @@ -154,4 +155,78 @@ extern bool is_supported_silicon_rev(uint32_t 
> silicon_rev);
>  #define     AST2400_SPI_BOOT                           2
>  #define     AST2400_DIS_BOOT                           3
>
> +/*
> + * Hardware strapping register definition (for Aspeed AST2500 SoC and
> + * higher)
> + *
> + * 31     Enable SPI Flash Strap Auto Fetch Mode
> + * 30     Enable GPIO Strap Mode
> + * 29     Select UART Debug Port
> + * 28     Reserved (1)
> + * 27     Enable fast reset mode for ARM ICE debugger
> + * 26     Enable eSPI flash mode
> + * 25     Enable eSPI mode
> + * 24     Select DDR4 SDRAM
> + * 23     Select 25 MHz reference clock input mode
> + * 22     Enable GPIOE pass-through mode
> + * 21     Enable GPIOD pass-through mode
> + * 20     Disable LPC to decode SuperIO 0x2E/0x4E address
> + * 19     Enable ACPI function
> + * 18     Select USBCKI input frequency
> + * 17     Enable BMC 2nd boot watchdog timer
> + * 16     SuperIO configuration address selection
> + * 15     VGA Class Code selection
> + * 14     Select dedicated LPC reset input
> + * 13:12  SPI mode selection
> + * 11:9   AXI/AHB clock frequency ratio selection
> + * 8      Reserved (0)
> + * 7      Define MAC#2 interface
> + * 6      Define MAC#1 interface
> + * 5      Enable dedicated VGA BIOS ROM
> + * 4      Reserved (0)
> + * 3:2    VGA memory size selection
> + * 1      Reserved (1)
> + * 0      Disable CPU boot
> + */
> +#define SCU_AST2500_HW_STRAP_SPI_AUTOFETCH_ENABLE  (0x1 << 31)
> +#define SCU_AST2500_HW_STRAP_GPIO_STRAP_ENABLE     (0x1 << 30)
> +#define SCU_AST2500_HW_STRAP_UART_DEBUG            (0x1 << 29)
> +#define     UART_DEBUG_UART1                           0
> +#define     UART_DEBUG_UART5                           1
> +#define SCU_AST2500_HW_STRAP_RESERVED28            (0x1 << 28)
> +
> +#define SCU_AST2500_HW_STRAP_FAST_RESET_DBG        (0x1 << 27)
> +#define SCU_AST2500_HW_STRAP_ESPI_FLASH_ENABLE     (0x1 << 26)
> +#define SCU_AST2500_HW_STRAP_ESPI_ENABLE           (0x1 << 25)
> +#define SCU_AST2500_HW_STRAP_DDR4_ENABLE           (0x1 << 24)
> +
> +#define SCU_AST2500_HW_STRAP_ACPI_ENABLE           (0x1 << 19)
> +#define SCU_AST2500_HW_STRAP_USBCKI_FREQ           (0x1 << 18)
> +#define     USBCKI_FREQ_24MHZ                          0
> +#define     USBCKI_FREQ_28MHZ                          1
> +
> +#define SCU_AST2500_HW_STRAP_SET_AXI_AHB_RATIO(x)  (x << 9)

Brackets around uses of macro parameters (here and below).

> +#define SCU_AST2500_HW_STRAP_GET_AXI_AHB_RATIO(x)  ((x >> 9) & 7)
> +#define SCU_AST2500_HW_STRAP_CPU_AXI_RATIO_MASK    (0x7 << 9)
> +#define     AXI_AHB_RATIO_UNDEFINED                    0
> +#define     AXI_AHB_RATIO_2_1                          1
> +#define     AXI_AHB_RATIO_3_1                          2
> +#define     AXI_AHB_RATIO_4_1                          3
> +#define     AXI_AHB_RATIO_5_1                          4
> +#define     AXI_AHB_RATIO_6_1                          5
> +#define     AXI_AHB_RATIO_7_1                          6
> +#define     AXI_AHB_RATIO_8_1                          7
> +
> +#define SCU_AST2500_HW_STRAP_RESERVED1             (0x1 << 1)
> +#define SCU_AST2500_HW_STRAP_DIS_BOOT              (0x1 << 0)
> +
> +#define AST2500_HW_STRAP1_DEFAULTS (                                    \
> +        SCU_AST2500_HW_STRAP_RESERVED28 |                               \
> +        SCU_HW_STRAP_2ND_BOOT_WDT |                                     \
> +        SCU_HW_STRAP_VGA_CLASS_CODE |                                   \
> +        SCU_HW_STRAP_LPC_RESET_PIN |                                    \
> +        SCU_AST2500_HW_STRAP_SET_AXI_AHB_RATIO(AXI_AHB_RATIO_2_1) |     \
> +        SCU_HW_STRAP_VGA_SIZE_SET(VGA_16M_DRAM) |                       \
> +        SCU_AST2500_HW_STRAP_RESERVED1)
> +
>  #endif /* ASPEED_SCU_H */

Otherwise
Reviewed-by: Peter Maydell <address@hidden>

thanks
-- PMM