Re: [PATCH 2/2] hw/arm: Use bit fields for NPCM7XX PWRON STRAPs

[Peter Maydell]

On Wed, 6 Apr 2022 at 23:34, Hao Wu <wuhaotsh@google.com> wrote:
>
> This patch uses the defined fields to describe PWRON STRAPs for
> better readability.
>
> Signed-off-by: Hao Wu <wuhaotsh@google.com>
> Reviewed-by: Patrick Venture <venture@google.com>
> ---
>  hw/arm/npcm7xx_boards.c | 24 +++++++++++++++++++-----
>  1 file changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/hw/arm/npcm7xx_boards.c b/hw/arm/npcm7xx_boards.c
> index 0678a56156..62d4092657 100644
> --- a/hw/arm/npcm7xx_boards.c
> +++ b/hw/arm/npcm7xx_boards.c
> @@ -30,11 +30,25 @@
>  #include "sysemu/sysemu.h"
>  #include "sysemu/block-backend.h"
>
> -#define NPCM750_EVB_POWER_ON_STRAPS 0x00001ff7
> -#define QUANTA_GSJ_POWER_ON_STRAPS 0x00001fff
> -#define QUANTA_GBS_POWER_ON_STRAPS 0x000017ff
> -#define KUDO_BMC_POWER_ON_STRAPS 0x00001fff
> -#define MORI_BMC_POWER_ON_STRAPS 0x00001fff
> +#define NPCM7XX_POWER_ON_STRAPS_DEFAULT (           \
> +        NPCM7XX_PWRON_STRAP_SPI0F18 |               \
> +        NPCM7XX_PWRON_STRAP_SFAB |                  \
> +        NPCM7XX_PWRON_STRAP_BSPA |                  \
> +        NPCM7XX_PWRON_STRAP_FUP(FUP_NORM_UART2) |   \
> +        NPCM7XX_PWRON_STRAP_SECEN |                 \
> +        NPCM7XX_PWRON_STRAP_HIZ |                   \
> +        NPCM7XX_PWRON_STRAP_ECC |                   \
> +        NPCM7XX_PWRON_STRAP_RESERVE1 |              \
> +        NPCM7XX_PWRON_STRAP_J2EN |                  \
> +        NPCM7XX_PWRON_STRAP_CKFRQ(CKFRQ_DEFAULT))   \

You don't want the trailing '\' on this last line.

> +
> +#define NPCM750_EVB_POWER_ON_STRAPS ( \
> +        NPCM7XX_POWER_ON_STRAPS_DEFAULT & ~NPCM7XX_PWRON_STRAP_RESERVE1)

This was 0x00001ff7, but the new macro definition makes it
0x1fef (since NPCM7XX_POWER_ON_STRAPS_DEFAULT sets all bits [12:0]
and we then clear bit 4).

> +#define QUANTA_GSJ_POWER_ON_STRAPS NPCM7XX_POWER_ON_STRAPS_DEFAULT
> +#define QUANTA_GBS_POWER_ON_STRAPS ( \
> +        NPCM7XX_POWER_ON_STRAPS_DEFAULT & ~NPCM7XX_PWRON_STRAP_SFAB)
> +#define KUDO_BMC_POWER_ON_STRAPS NPCM7XX_POWER_ON_STRAPS_DEFAULT
> +#define MORI_BMC_POWER_ON_STRAPS NPCM7XX_POWER_ON_STRAPS_DEFAULT
>
>  static const char npcm7xx_default_bootrom[] = "npcm7xx_bootrom.bin";

thanks
-- PMM