On Mon, 2023-01-30 at 13:14 +0100, ~lexbaileylowrisc wrote:
> From: Emmanuel Blot <eblot@rivosinc.com>
Your patch titles are a little long. You don't need to include `qemu`
or `[ot]` in the patch and commit titles.
>
> Use a separate Kconfig symbols for Ibex UART, Timer, and SPI devices:
> having an Ibex CPU does not imply usage of these specific
> implementations.
Why is this required though?
>
> Signed-off-by: Emmanuel Blot <eblot@rivosinc.com>
You are missing your SoB here (and in other patches)
> ---
> hw/char/Kconfig | 3 +++
> hw/char/meson.build | 2 +-
> hw/riscv/Kconfig | 3 +++
> hw/ssi/Kconfig | 4 ++++
> hw/ssi/meson.build | 2 +-
> hw/timer/Kconfig | 3 +++
> hw/timer/ibex_timer.c | 2 +-
> hw/timer/meson.build | 2 +-
> 8 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/hw/char/Kconfig b/hw/char/Kconfig
> index 020c0a84bb..0cbbaedb2f 100644
> --- a/hw/char/Kconfig
> +++ b/hw/char/Kconfig
> @@ -95,3 +95,6 @@ config IP_OCTAL_232
> bool
> default y
> depends on IPACK
> +
> +config IBEX_UART
> + bool
> diff --git a/hw/char/meson.build b/hw/char/meson.build
> index a9e1dc26c0..f47fa11bbe 100644
> --- a/hw/char/meson.build
> +++ b/hw/char/meson.build
> @@ -2,7 +2,7 @@ system_ss.add(when: 'CONFIG_CADENCE', if_true:
> files('cadence_uart.c'))
> system_ss.add(when: 'CONFIG_CMSDK_APB_UART', if_true: files('cmsdk-
> apb-uart.c'))
> system_ss.add(when: 'CONFIG_ESCC', if_true: files('escc.c'))
> system_ss.add(when: 'CONFIG_GRLIB', if_true:
> files('grlib_apbuart.c'))
> -system_ss.add(when: 'CONFIG_IBEX', if_true: files('ibex_uart.c'))
> +system_ss.add(when: 'CONFIG_IBEX_UART', if_true:
> files('ibex_uart.c'))
> system_ss.add(when: 'CONFIG_IMX', if_true: files('imx_serial.c'))
> system_ss.add(when: 'CONFIG_IP_OCTAL_232', if_true:
> files('ipoctal232.c'))
> system_ss.add(when: 'CONFIG_ISA_BUS', if_true: files('parallel-
> isa.c'))
> diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
> index 0222c93f87..e6aa32ee8f 100644
> --- a/hw/riscv/Kconfig
> +++ b/hw/riscv/Kconfig
> @@ -38,6 +38,9 @@ config OPENTITAN
> default y
> depends on RISCV32
> select IBEX
> + select IBEX_SPI_HOST
> + select IBEX_TIMER
> + select IBEX_UART
> select SIFIVE_PLIC
> select UNIMP
>
> diff --git a/hw/ssi/Kconfig b/hw/ssi/Kconfig
> index 1bd56463c1..c8d573eeb9 100644
> --- a/hw/ssi/Kconfig
> +++ b/hw/ssi/Kconfig
> @@ -32,3 +32,7 @@ config PNV_SPI
> config ALLWINNER_A10_SPI
> bool
> select SSI
> +
> +config IBEX_SPI_HOST
> + bool
> + select SSI
> diff --git a/hw/ssi/meson.build b/hw/ssi/meson.build
> index 6afb1ea200..a3ba319280 100644
> --- a/hw/ssi/meson.build
> +++ b/hw/ssi/meson.build
> @@ -10,6 +10,6 @@ system_ss.add(when: 'CONFIG_XILINX_SPI', if_true:
> files('xilinx_spi.c'))
> system_ss.add(when: 'CONFIG_XILINX_SPIPS', if_true:
> files('xilinx_spips.c'))
> system_ss.add(when: 'CONFIG_XLNX_VERSAL', if_true: files('xlnx-
> versal-ospi.c'))
> system_ss.add(when: 'CONFIG_IMX', if_true: files('imx_spi.c'))
> -system_ss.add(when: 'CONFIG_IBEX', if_true:
> files('ibex_spi_host.c'))
> +system_ss.add(when: 'CONFIG_IBEX_SPI_HOST', if_true:
> files('ibex_spi_host.c'))
> system_ss.add(when: 'CONFIG_BCM2835_SPI', if_true:
> files('bcm2835_spi.c'))
> system_ss.add(when: 'CONFIG_PNV_SPI', if_true: files('pnv_spi.c'))
> diff --git a/hw/timer/Kconfig b/hw/timer/Kconfig
> index b3d823ce2c..2fdefd0d1f 100644
> --- a/hw/timer/Kconfig
> +++ b/hw/timer/Kconfig
> @@ -65,3 +65,6 @@ config STELLARIS_GPTM
>
> config AVR_TIMER16
> bool
> +
> +config IBEX_TIMER
> + bool
> diff --git a/hw/timer/ibex_timer.c b/hw/timer/ibex_timer.c
> index ee18652189..858e1a67de 100644
> --- a/hw/timer/ibex_timer.c
> +++ b/hw/timer/ibex_timer.c
> @@ -31,7 +31,7 @@
> #include "hw/timer/ibex_timer.h"
> #include "hw/core/irq.h"
> #include "hw/core/qdev-properties.h"
> -#include "target/riscv/cpu.h"
> +#include "hw/core/registerfields.h"
Why does this need to be changed?
Alistair
> #include "migration/vmstate.h"
>
> REG32(ALERT_TEST, 0x00)
> diff --git a/hw/timer/meson.build b/hw/timer/meson.build
> index 178321c029..cc02aa0dda 100644
> --- a/hw/timer/meson.build
> +++ b/hw/timer/meson.build
> @@ -30,7 +30,7 @@ system_ss.add(when: 'CONFIG_SSE_TIMER', if_true:
> files('sse-timer.c'))
> system_ss.add(when: 'CONFIG_STELLARIS_GPTM', if_true:
> files('stellaris-gptm.c'))
> system_ss.add(when: 'CONFIG_STM32F2XX_TIMER', if_true:
> files('stm32f2xx_timer.c'))
> system_ss.add(when: 'CONFIG_XILINX', if_true:
> files('xilinx_timer.c'))
> -specific_ss.add(when: 'CONFIG_IBEX', if_true: files('ibex_timer.c'))
> +system_ss.add(when: 'CONFIG_IBEX_TIMER', if_true:
> files('ibex_timer.c'))
> system_ss.add(when: 'CONFIG_SIFIVE_PWM', if_true:
> files('sifive_pwm.c'))
>
> specific_ss.add(when: 'CONFIG_AVR_TIMER16', if_true:
> files('avr_timer16.c'))