Re: [PATCH 1/2] hw/misc: Add PWRON STRAP bit fields in GCR module

[Peter Maydell]

On Wed, 6 Apr 2022 at 23:34, Hao Wu <wuhaotsh@google.com> wrote:
>
> Similar to the Aspeed code in include/misc/aspeed_scu.h, we define
> the PWRON STRAP fields in their corresponding module for NPCM7XX.
>
> Signed-off-by: Hao Wu <wuhaotsh@google.com>
> Reviewed-by: Patrick Venture <venture@google.com>
> ---
>  include/hw/misc/npcm7xx_gcr.h | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>
> diff --git a/include/hw/misc/npcm7xx_gcr.h b/include/hw/misc/npcm7xx_gcr.h
> index 13109d9d32..98da5d171f 100644
> --- a/include/hw/misc/npcm7xx_gcr.h
> +++ b/include/hw/misc/npcm7xx_gcr.h
> @@ -19,6 +19,36 @@
>  #include "exec/memory.h"
>  #include "hw/sysbus.h"
>
> +/*
> + * NPCM7XX PWRON STRAP bit fields
> + * 12: SPI0 powered by VSBV3 at 1.8V
> + * 11: System flash attached to BMC
> + * 10: BSP alternative pins.
> + * 9:8: Flash UART command route enabled.
> + * 7: Security enabled.
> + * 6: HI-Z state control.
> + * 5: ECC disabled.
> + * 4: Reserved
> + * 3: JTAG2 enabled.
> + * 2:0: CPU and DRAM clock frequency.
> + */
> +#define NPCM7XX_PWRON_STRAP_SPI0F18                 BIT(12)
> +#define NPCM7XX_PWRON_STRAP_SFAB                    BIT(11)
> +#define NPCM7XX_PWRON_STRAP_BSPA                    BIT(10)
> +#define NPCM7XX_PWRON_STRAP_FUP(x)                  ((x) << 8)
> +#define     FUP_NORM_UART2      3
> +#define     FUP_PROG_UART3      2
> +#define     FUP_PROG_UART2      1
> +#define     FUP_NORM_UART3      0
> +#define NPCM7XX_PWRON_STRAP_SECEN                   BIT(7)
> +#define NPCM7XX_PWRON_STRAP_HIZ                     BIT(6)
> +#define NPCM7XX_PWRON_STRAP_ECC                     BIT(5)
> +#define NPCM7XX_PWRON_STRAP_RESERVE1                BIT(4)
> +#define NPCM7XX_PWRON_STRAP_J2EN                    BIT(3)
> +#define NPCM7XX_PWRON_STRAP_CKFRQ(x)                ((x) << 8)

Comment says clock frequency is bits [2:0] but macro definition
puts them in bits [9:8]...

> +#define     CKFRQ_SKIPINIT      (0x000)
> +#define     CKFRQ_DEFAULT       (0x111)

These don't need parentheses around them.

thanks
-- PMM