On Mon, 2023-01-30 at 13:14 +0100, ~lexbaileylowrisc wrote:
> From: Emmanuel Blot <[email protected]>

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 <[email protected]>

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'))

Reply via email to