[v3,06/10] palmetto-bmc: add board specific configuration

aspeed_board_init() now uses a board identifier to customize some values
specific to the board.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---

 Changes since v2:

 - removed silicon-rev and cpu-model. This is now in the SoC.

 Changes since v1:

 - changed aspeed_init() prototype to use a 'const AspeedBoardConfig *'
 - fixed white space issues

 hw/arm/aspeed.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

On Tue, 2016-08-02 at 19:15 +0200, Cédric Le Goater wrote:
> aspeed_board_init() now uses a board identifier to customize some values
> specific to the board.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: Andrew Jeffery <andrew@aj.id.au>

> ---
> 
>  Changes since v2:
> 
>  - removed silicon-rev and cpu-model. This is now in the SoC.
> 
>  Changes since v1:
> 
>  - changed aspeed_init() prototype to use a 'const AspeedBoardConfig *'
>  - fixed white space issues
> 
>  hw/arm/aspeed.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index ad0a062b5624..4226b8dcd95c 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -30,6 +30,19 @@ typedef struct AspeedBoardState {
>      MemoryRegion ram;
>  } AspeedBoardState;
>  
> +typedef struct AspeedBoardConfig {
> +    const char *soc_name;
> +    uint32_t hw_strap1;
> +} AspeedBoardConfig;
> +
> +enum {
> +    PALMETTO_BMC
> +};
> +
> +static const AspeedBoardConfig aspeed_boards[] = {
> +    [PALMETTO_BMC] = { "ast2400-a0", 0x120CE416 },
> +};
> +
>  static void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
>                                        Error **errp)
>  {
> @@ -56,13 +69,14 @@ static void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
>      }
>  }
>  
> -static void aspeed_board_init(MachineState *machine)
> +static void aspeed_board_init(MachineState *machine,
> +                              const AspeedBoardConfig *cfg)
>  {
>      AspeedBoardState *bmc;
>      AspeedSoCClass *sc;
>  
>      bmc = g_new0(AspeedBoardState, 1);
> -    object_initialize(&bmc->soc, (sizeof(bmc->soc)), "ast2400-a0");
> +    object_initialize(&bmc->soc, (sizeof(bmc->soc)), cfg->soc_name);
>      object_property_add_child(OBJECT(machine), "soc", OBJECT(&bmc->soc),
>                                &error_abort);
>  
> @@ -73,7 +87,7 @@ static void aspeed_board_init(MachineState *machine)
>                                  &bmc->ram);
>      object_property_add_const_link(OBJECT(&bmc->soc), "ram", OBJECT(&bmc->ram),
>                                     &error_abort);
> -    object_property_set_int(OBJECT(&bmc->soc), 0x120CE416, "hw-strap1",
> +    object_property_set_int(OBJECT(&bmc->soc), cfg->hw_strap1, "hw-strap1",
>                              &error_abort);
>      object_property_set_bool(OBJECT(&bmc->soc), true, "realized",
>                               &error_abort);
> @@ -93,7 +107,7 @@ static void aspeed_board_init(MachineState *machine)
>  
>  static void palmetto_bmc_init(MachineState *machine)
>  {
> -    aspeed_board_init(machine);
> +    aspeed_board_init(machine, &aspeed_boards[PALMETTO_BMC]);
>  }
>  
>  static void palmetto_bmc_class_init(ObjectClass *oc, void *data)

On 2 August 2016 at 18:15, Cédric Le Goater <clg@kaod.org> wrote:
> aspeed_board_init() now uses a board identifier to customize some values
> specific to the board.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>
>  Changes since v2:
>
>  - removed silicon-rev and cpu-model. This is now in the SoC.
>
>  Changes since v1:
>
>  - changed aspeed_init() prototype to use a 'const AspeedBoardConfig *'
>  - fixed white space issues
>
>  hw/arm/aspeed.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index ad0a062b5624..4226b8dcd95c 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -30,6 +30,19 @@ typedef struct AspeedBoardState {
>      MemoryRegion ram;
>  } AspeedBoardState;
>
> +typedef struct AspeedBoardConfig {
> +    const char *soc_name;
> +    uint32_t hw_strap1;
> +} AspeedBoardConfig;
> +
> +enum {
> +    PALMETTO_BMC

Since you need to respin this set anyway, I shall pick a nit:
a trailing comma here would be preferable because then you
don't need to change the line when you add the next entry
to the enum.

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

On 08/11/2016 12:27 PM, Peter Maydell wrote:
> On 2 August 2016 at 18:15, Cédric Le Goater <clg@kaod.org> wrote:
>> aspeed_board_init() now uses a board identifier to customize some values
>> specific to the board.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>
>>  Changes since v2:
>>
>>  - removed silicon-rev and cpu-model. This is now in the SoC.
>>
>>  Changes since v1:
>>
>>  - changed aspeed_init() prototype to use a 'const AspeedBoardConfig *'
>>  - fixed white space issues
>>
>>  hw/arm/aspeed.c | 22 ++++++++++++++++++----
>>  1 file changed, 18 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
>> index ad0a062b5624..4226b8dcd95c 100644
>> --- a/hw/arm/aspeed.c
>> +++ b/hw/arm/aspeed.c
>> @@ -30,6 +30,19 @@ typedef struct AspeedBoardState {
>>      MemoryRegion ram;
>>  } AspeedBoardState;
>>
>> +typedef struct AspeedBoardConfig {
>> +    const char *soc_name;
>> +    uint32_t hw_strap1;
>> +} AspeedBoardConfig;
>> +
>> +enum {
>> +    PALMETTO_BMC
> 
> Since you need to respin this set anyway, I shall pick a nit:
> a trailing comma here would be preferable because then you
> don't need to change the line when you add the next entry
> to the enum.

yes. This is a good pratice. I will fix that in the next version.

Thanks for the review,

C.

> Otherwise
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> 
> thanks
> -- PMM
>