[PATCH 0/4] hw/{lm32,sh4}: Kconfig cleanups

[PATCH 0/4] hw/{lm32,sh4}: Kconfig cleanups

[Philippe Mathieu-Daudé]

Patches extracted from "target: Provide target-specific Kconfig":
https://www.mail-archive.com/qemu-block@nongnu.org/msg79924.html

Single change, renamed PERIPHERALS as DEVICES as suggested by
Zoltan.

Since LM32/SH4 are not very active, can it go via qemu-trivial@?

Thanks,

Phil.

Philippe Mathieu-Daudé (4):
  hw/sh4/Kconfig: Rename CONFIG_SH4 -> CONFIG_SH4_DEVICES
  hw/lm32/Kconfig: Introduce CONFIG_LM32_EVR for lm32-evr/uclinux boards
  hw/lm32/Kconfig: Rename CONFIG_LM32 -> CONFIG_LM32_DEVICES
  hw/lm32/Kconfig: Have MILKYMIST select LM32_DEVICES

 default-configs/devices/lm32-softmmu.mak |  2 +-
 hw/block/meson.build                     |  2 +-
 hw/char/meson.build                      |  6 +++---
 hw/intc/meson.build                      |  4 ++--
 hw/lm32/Kconfig                          | 10 +++++++---
 hw/lm32/meson.build                      |  2 +-
 hw/sh4/Kconfig                           |  6 +++---
 hw/timer/meson.build                     |  4 ++--
 8 files changed, 20 insertions(+), 16 deletions(-)

-- 
2.26.2

[PATCH 1/4] hw/sh4/Kconfig: Rename CONFIG_SH4 -> CONFIG_SH4_DEVICES

[Philippe Mathieu-Daudé]

We want to be able to use the 'SH4' config for architecture
specific features. As CONFIG_SH4 is only used to select
peripherals, rename it CONFIG_SH4_DEVICES.

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/block/meson.build | 2 +-
 hw/char/meson.build  | 2 +-
 hw/intc/meson.build  | 2 +-
 hw/sh4/Kconfig       | 6 +++---
 hw/timer/meson.build | 2 +-
 5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/block/meson.build b/hw/block/meson.build
index 602ca6c8541..f1deee2d648 100644
--- a/hw/block/meson.build
+++ b/hw/block/meson.build
@@ -12,7 +12,7 @@
 softmmu_ss.add(when: 'CONFIG_SSI_M25P80', if_true: files('m25p80.c'))
 softmmu_ss.add(when: 'CONFIG_SWIM', if_true: files('swim.c'))
 softmmu_ss.add(when: 'CONFIG_XEN', if_true: files('xen-block.c'))
-softmmu_ss.add(when: 'CONFIG_SH4', if_true: files('tc58128.c'))
+softmmu_ss.add(when: 'CONFIG_SH4_DEVICES', if_true: files('tc58128.c'))
 softmmu_ss.add(when: 'CONFIG_NVME_PCI', if_true: files('nvme.c', 'nvme-ns.c'))
 
 specific_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true: files('virtio-blk.c'))
diff --git a/hw/char/meson.build b/hw/char/meson.build
index 196ac91fa29..370f5d5ad17 100644
--- a/hw/char/meson.build
+++ b/hw/char/meson.build
@@ -31,7 +31,7 @@
 softmmu_ss.add(when: 'CONFIG_RASPI', if_true: files('bcm2835_aux.c'))
 softmmu_ss.add(when: 'CONFIG_RENESAS_SCI', if_true: files('renesas_sci.c'))
 softmmu_ss.add(when: 'CONFIG_SIFIVE_UART', if_true: files('sifive_uart.c'))
-softmmu_ss.add(when: 'CONFIG_SH4', if_true: files('sh_serial.c'))
+softmmu_ss.add(when: 'CONFIG_SH4_DEVICES', if_true: files('sh_serial.c'))
 softmmu_ss.add(when: 'CONFIG_STM32F2XX_USART', if_true: files('stm32f2xx_usart.c'))
 softmmu_ss.add(when: 'CONFIG_MCHP_PFSOC_MMUART', if_true: files('mchp_pfsoc_mmuart.c'))
 
diff --git a/hw/intc/meson.build b/hw/intc/meson.build
index 53cba115690..7f2a0fed2e3 100644
--- a/hw/intc/meson.build
+++ b/hw/intc/meson.build
@@ -47,7 +47,7 @@
 specific_ss.add(when: 'CONFIG_RX_ICU', if_true: files('rx_icu.c'))
 specific_ss.add(when: 'CONFIG_S390_FLIC', if_true: files('s390_flic.c'))
 specific_ss.add(when: 'CONFIG_S390_FLIC_KVM', if_true: files('s390_flic_kvm.c'))
-specific_ss.add(when: 'CONFIG_SH4', if_true: files('sh_intc.c'))
+specific_ss.add(when: 'CONFIG_SH4_DEVICES', if_true: files('sh_intc.c'))
 specific_ss.add(when: 'CONFIG_SIFIVE_CLINT', if_true: files('sifive_clint.c'))
 specific_ss.add(when: 'CONFIG_SIFIVE_PLIC', if_true: files('sifive_plic.c'))
 specific_ss.add(when: 'CONFIG_XICS', if_true: files('xics.c'))
diff --git a/hw/sh4/Kconfig b/hw/sh4/Kconfig
index 4cbce3a0ed5..c20b86e3fde 100644
--- a/hw/sh4/Kconfig
+++ b/hw/sh4/Kconfig
@@ -9,16 +9,16 @@ config R2D
     select USB_OHCI_PCI
     select PCI
     select SM501
-    select SH4
+    select SH4_DEVICES
 
 config SHIX
     bool
     select SH7750
-    select SH4
+    select SH4_DEVICES
 
 config SH7750
     bool
 
-config SH4
+config SH4_DEVICES
     bool
     select PTIMER
diff --git a/hw/timer/meson.build b/hw/timer/meson.build
index be343f68fed..a0fa3845d80 100644
--- a/hw/timer/meson.build
+++ b/hw/timer/meson.build
@@ -30,7 +30,7 @@
 softmmu_ss.add(when: 'CONFIG_PUV3', if_true: files('puv3_ost.c'))
 softmmu_ss.add(when: 'CONFIG_PXA2XX', if_true: files('pxa2xx_timer.c'))
 softmmu_ss.add(when: 'CONFIG_RASPI', if_true: files('bcm2835_systmr.c'))
-softmmu_ss.add(when: 'CONFIG_SH4', if_true: files('sh_timer.c'))
+softmmu_ss.add(when: 'CONFIG_SH4_DEVICES', if_true: files('sh_timer.c'))
 softmmu_ss.add(when: 'CONFIG_SLAVIO', if_true: files('slavio_timer.c'))
 softmmu_ss.add(when: 'CONFIG_STM32F2XX_TIMER', if_true: files('stm32f2xx_timer.c'))
 softmmu_ss.add(when: 'CONFIG_XILINX', if_true: files('xilinx_timer.c'))
-- 
2.26.2

Re: [PATCH 1/4] hw/sh4/Kconfig: Rename CONFIG_SH4 -> CONFIG_SH4_DEVICES

[Peter Maydell]

On Mon, 8 Feb 2021 at 20:04, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> We want to be able to use the 'SH4' config for architecture
> specific features. As CONFIG_SH4 is only used to select
> peripherals, rename it CONFIG_SH4_DEVICES.
>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/block/meson.build | 2 +-
>  hw/char/meson.build  | 2 +-
>  hw/intc/meson.build  | 2 +-
>  hw/sh4/Kconfig       | 6 +++---
>  hw/timer/meson.build | 2 +-
>  5 files changed, 7 insertions(+), 7 deletions(-)

We could if we wished be more fine-grained about this, eg by
adding new CONFIG options for each device:
 CONFIG_TC58128
 CONFIG_SH_SERIAL
 CONFIG_SH_INTC
 CONFIG_SH_TIMER
 CONFIG_SH_PCI

and then in hw/sh4/Kconfig
 * config SH7750:
   add 'select SH_SERIAL', 'select SH_INTC', 'select SH_TIMER'
 * config R2D:
   add 'select SH7750' (it's a pre-existing bug that it doesn't, since
   r2d.c has a call to sh7750_init(). Harmless at the moment because
   nothing actually uses CONFIG_SH7750 -- hw/sh4/meson.build always
   compiles sh7750.c and sh7750_regnames.c unconditionally...)
   add 'select SH_PCI' (and make hw/sh4/meson.build build sh_pci.c
   only if it is set...)
 * config SHIX
   add 'select TC58128'

Do we have a general preference for how fine-grained we go with the
Kconfig switches ? Looking at the arm code, in some cases we have
a CONFIG_ for the SoC that uses 'select' to turn on a separate
CONFIG_ for each device (the STM32 SoCs are done this way), and
in some cases we just have the meson.build use the SoC's CONFIG_*
to control whether we compile its devices (the Xilinx and Exynos4
SoCs are done this way). When reviewing new code it would be helpful
to know whether to suggest doing it one way or the other :-)

thanks
-- PMM

Re: [PATCH 1/4] hw/sh4/Kconfig: Rename CONFIG_SH4 -> CONFIG_SH4_DEVICES

[Philippe Mathieu-Daudé]

On 2/8/21 9:22 PM, Peter Maydell wrote:
> On Mon, 8 Feb 2021 at 20:04, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>
>> We want to be able to use the 'SH4' config for architecture
>> specific features. As CONFIG_SH4 is only used to select
>> peripherals, rename it CONFIG_SH4_DEVICES.
>>
>> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  hw/block/meson.build | 2 +-
>>  hw/char/meson.build  | 2 +-
>>  hw/intc/meson.build  | 2 +-
>>  hw/sh4/Kconfig       | 6 +++---
>>  hw/timer/meson.build | 2 +-
>>  5 files changed, 7 insertions(+), 7 deletions(-)
> 
> We could if we wished be more fine-grained about this, eg by
> adding new CONFIG options for each device:
>  CONFIG_TC58128
>  CONFIG_SH_SERIAL
>  CONFIG_SH_INTC
>  CONFIG_SH_TIMER
>  CONFIG_SH_PCI
> 
> and then in hw/sh4/Kconfig
>  * config SH7750:
>    add 'select SH_SERIAL', 'select SH_INTC', 'select SH_TIMER'
>  * config R2D:
>    add 'select SH7750' (it's a pre-existing bug that it doesn't, since
>    r2d.c has a call to sh7750_init(). Harmless at the moment because
>    nothing actually uses CONFIG_SH7750 -- hw/sh4/meson.build always
>    compiles sh7750.c and sh7750_regnames.c unconditionally...)
>    add 'select SH_PCI' (and make hw/sh4/meson.build build sh_pci.c
>    only if it is set...)
>  * config SHIX
>    add 'select TC58128'

OK.

> Do we have a general preference for how fine-grained we go with the
> Kconfig switches ? Looking at the arm code, in some cases we have
> a CONFIG_ for the SoC that uses 'select' to turn on a separate
> CONFIG_ for each device (the STM32 SoCs are done this way), and
> in some cases we just have the meson.build use the SoC's CONFIG_*
> to control whether we compile its devices (the Xilinx and Exynos4
> SoCs are done this way). When reviewing new code it would be helpful
> to know whether to suggest doing it one way or the other :-)

My view is:

- Use fine granularity with shared/reusable models, so if someone
  wants to build a QEMU for a single machine, it is possible (using
  --without-default-devices and a specific target config.mak).
  Example: STM32F205_SOC and STM32F405_SOC

- For some (family of) SoC, a single switch is acceptable. In
  particular when all models are implemented in the same source file.
  Example: ASPEED_SOC

- Do not look at hw/i386/Kconfig

I have an old branch where I was generating a .dot of the Kconfig tree
for easier visualization, fine granularity was helpful. I'll try to
update it.

Regards,

Phil.

Re: [PATCH 1/4] hw/sh4/Kconfig: Rename CONFIG_SH4 -> CONFIG_SH4_DEVICES

[Philippe Mathieu-Daudé]

On 2/21/21 7:07 PM, Philippe Mathieu-Daudé wrote:
> On 2/8/21 9:22 PM, Peter Maydell wrote:
>> On Mon, 8 Feb 2021 at 20:04, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>>
>>> We want to be able to use the 'SH4' config for architecture
>>> specific features. As CONFIG_SH4 is only used to select
>>> peripherals, rename it CONFIG_SH4_DEVICES.
>>>
>>> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>>  hw/block/meson.build | 2 +-
>>>  hw/char/meson.build  | 2 +-
>>>  hw/intc/meson.build  | 2 +-
>>>  hw/sh4/Kconfig       | 6 +++---
>>>  hw/timer/meson.build | 2 +-
>>>  5 files changed, 7 insertions(+), 7 deletions(-)
>>
>> We could if we wished be more fine-grained about this, eg by
>> adding new CONFIG options for each device:
>>  CONFIG_TC58128
>>  CONFIG_SH_SERIAL
>>  CONFIG_SH_INTC
>>  CONFIG_SH_TIMER
>>  CONFIG_SH_PCI
>>
>> and then in hw/sh4/Kconfig
>>  * config SH7750:
>>    add 'select SH_SERIAL', 'select SH_INTC', 'select SH_TIMER'
>>  * config R2D:
>>    add 'select SH7750' (it's a pre-existing bug that it doesn't, since
>>    r2d.c has a call to sh7750_init(). Harmless at the moment because
>>    nothing actually uses CONFIG_SH7750 -- hw/sh4/meson.build always
>>    compiles sh7750.c and sh7750_regnames.c unconditionally...)
>>    add 'select SH_PCI' (and make hw/sh4/meson.build build sh_pci.c
>>    only if it is set...)
>>  * config SHIX
>>    add 'select TC58128'
> 
> OK.

(Forgot to say in this case it makes sense because SH4 and RX targets
share peripherals IP cores, so some models could be reused.)

[PATCH 2/4] hw/lm32/Kconfig: Introduce CONFIG_LM32_EVR for lm32-evr/uclinux boards

[Philippe Mathieu-Daudé]

We want to be able to use the 'LM32' config for architecture
specific features. Introduce CONFIG_LM32_EVR to select the
lm32-evr / lm32-uclinux boards.

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 default-configs/devices/lm32-softmmu.mak | 2 +-
 hw/lm32/Kconfig                          | 6 +++++-
 hw/lm32/meson.build                      | 2 +-
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/default-configs/devices/lm32-softmmu.mak b/default-configs/devices/lm32-softmmu.mak
index 115b3e34c98..1bce3f6e8b6 100644
--- a/default-configs/devices/lm32-softmmu.mak
+++ b/default-configs/devices/lm32-softmmu.mak
@@ -8,5 +8,5 @@ CONFIG_SEMIHOSTING=y
 
 # Boards:
 #
-CONFIG_LM32=y
+CONFIG_LM32_EVR=y
 CONFIG_MILKYMIST=y
diff --git a/hw/lm32/Kconfig b/hw/lm32/Kconfig
index ed2e3060b04..20c36edc402 100644
--- a/hw/lm32/Kconfig
+++ b/hw/lm32/Kconfig
@@ -1,7 +1,6 @@
 config LM32
     bool
     select PTIMER
-    select PFLASH_CFI02
 
 config MILKYMIST
     bool
@@ -12,3 +11,8 @@ config MILKYMIST
     select FRAMEBUFFER
     select SD
     select USB_OHCI
+
+config LM32_EVR
+    bool
+    select LM32
+    select PFLASH_CFI02
diff --git a/hw/lm32/meson.build b/hw/lm32/meson.build
index 8caf0a727ff..42d6f8db3d9 100644
--- a/hw/lm32/meson.build
+++ b/hw/lm32/meson.build
@@ -1,6 +1,6 @@
 lm32_ss = ss.source_set()
 # LM32 boards
-lm32_ss.add(when: 'CONFIG_LM32', if_true: files('lm32_boards.c'))
+lm32_ss.add(when: 'CONFIG_LM32_EVR', if_true: files('lm32_boards.c'))
 lm32_ss.add(when: 'CONFIG_MILKYMIST', if_true: files('milkymist.c'))
 
 hw_arch += {'lm32': lm32_ss}
-- 
2.26.2

[PATCH 3/4] hw/lm32/Kconfig: Rename CONFIG_LM32 -> CONFIG_LM32_DEVICES

[Philippe Mathieu-Daudé]

We want to be able to use the 'LM32' config for architecture
specific features. As CONFIG_LM32 is only used to select
peripherals, rename it CONFIG_LM32_DEVICES.

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/char/meson.build  | 4 ++--
 hw/intc/meson.build  | 2 +-
 hw/lm32/Kconfig      | 4 ++--
 hw/timer/meson.build | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/char/meson.build b/hw/char/meson.build
index 370f5d5ad17..6ab90e68568 100644
--- a/hw/char/meson.build
+++ b/hw/char/meson.build
@@ -8,8 +8,8 @@
 softmmu_ss.add(when: 'CONFIG_IPACK', if_true: files('ipoctal232.c'))
 softmmu_ss.add(when: 'CONFIG_ISA_BUS', if_true: files('parallel-isa.c'))
 softmmu_ss.add(when: 'CONFIG_ISA_DEBUG', if_true: files('debugcon.c'))
-softmmu_ss.add(when: 'CONFIG_LM32', if_true: files('lm32_juart.c'))
-softmmu_ss.add(when: 'CONFIG_LM32', if_true: files('lm32_uart.c'))
+softmmu_ss.add(when: 'CONFIG_LM32_DEVICES', if_true: files('lm32_juart.c'))
+softmmu_ss.add(when: 'CONFIG_LM32_DEVICES', if_true: files('lm32_uart.c'))
 softmmu_ss.add(when: 'CONFIG_MILKYMIST', if_true: files('milkymist-uart.c'))
 softmmu_ss.add(when: 'CONFIG_NRF51_SOC', if_true: files('nrf51_uart.c'))
 softmmu_ss.add(when: 'CONFIG_PARALLEL', if_true: files('parallel.c'))
diff --git a/hw/intc/meson.build b/hw/intc/meson.build
index 7f2a0fed2e3..2b975b235f2 100644
--- a/hw/intc/meson.build
+++ b/hw/intc/meson.build
@@ -14,7 +14,7 @@
 softmmu_ss.add(when: 'CONFIG_I8259', if_true: files('i8259_common.c', 'i8259.c'))
 softmmu_ss.add(when: 'CONFIG_IMX', if_true: files('imx_avic.c', 'imx_gpcv2.c'))
 softmmu_ss.add(when: 'CONFIG_IOAPIC', if_true: files('ioapic_common.c'))
-softmmu_ss.add(when: 'CONFIG_LM32', if_true: files('lm32_pic.c'))
+softmmu_ss.add(when: 'CONFIG_LM32_DEVICES', if_true: files('lm32_pic.c'))
 softmmu_ss.add(when: 'CONFIG_OPENPIC', if_true: files('openpic.c'))
 softmmu_ss.add(when: 'CONFIG_PL190', if_true: files('pl190.c'))
 softmmu_ss.add(when: 'CONFIG_PUV3', if_true: files('puv3_intc.c'))
diff --git a/hw/lm32/Kconfig b/hw/lm32/Kconfig
index 20c36edc402..518c84ed508 100644
--- a/hw/lm32/Kconfig
+++ b/hw/lm32/Kconfig
@@ -1,4 +1,4 @@
-config LM32
+config LM32_DEVICES
     bool
     select PTIMER
 
@@ -14,5 +14,5 @@ config MILKYMIST
 
 config LM32_EVR
     bool
-    select LM32
+    select LM32_DEVICES
     select PFLASH_CFI02
diff --git a/hw/timer/meson.build b/hw/timer/meson.build
index a0fa3845d80..809a8988b9f 100644
--- a/hw/timer/meson.build
+++ b/hw/timer/meson.build
@@ -19,7 +19,7 @@
 softmmu_ss.add(when: 'CONFIG_I8254', if_true: files('i8254_common.c', 'i8254.c'))
 softmmu_ss.add(when: 'CONFIG_IMX', if_true: files('imx_epit.c'))
 softmmu_ss.add(when: 'CONFIG_IMX', if_true: files('imx_gpt.c'))
-softmmu_ss.add(when: 'CONFIG_LM32', if_true: files('lm32_timer.c'))
+softmmu_ss.add(when: 'CONFIG_LM32_DEVICES', if_true: files('lm32_timer.c'))
 softmmu_ss.add(when: 'CONFIG_MILKYMIST', if_true: files('milkymist-sysctl.c'))
 softmmu_ss.add(when: 'CONFIG_MIPS_CPS', if_true: files('mips_gictimer.c'))
 softmmu_ss.add(when: 'CONFIG_MSF2', if_true: files('mss-timer.c'))
-- 
2.26.2

[PATCH 4/4] hw/lm32/Kconfig: Have MILKYMIST select LM32_DEVICES

[Philippe Mathieu-Daudé]

The Milkymist board requires more than the PTIMER. Directly
select the LM32_DEVICES. This fixes:

  /usr/bin/ld:
  libqemu-lm32-softmmu.fa.p/target_lm32_gdbstub.c.o: in function `lm32_cpu_gdb_read_register':
  target/lm32/gdbstub.c:46: undefined reference to `lm32_pic_get_im'
  target/lm32/gdbstub.c:48: undefined reference to `lm32_pic_get_ip'
  libqemu-lm32-softmmu.fa.p/target_lm32_op_helper.c.o: in function `helper_wcsr_im':
  target/lm32/op_helper.c:107: undefined reference to `lm32_pic_set_im'
  libqemu-lm32-softmmu.fa.p/target_lm32_op_helper.c.o: in function `helper_wcsr_ip':
  target/lm32/op_helper.c:114: undefined reference to `lm32_pic_set_ip'
  libqemu-lm32-softmmu.fa.p/target_lm32_op_helper.c.o: in function `helper_wcsr_jtx':
  target/lm32/op_helper.c:120: undefined reference to `lm32_juart_set_jtx'
  libqemu-lm32-softmmu.fa.p/target_lm32_op_helper.c.o: in function `helper_wcsr_jrx':
  target/lm32/op_helper.c:125: undefined reference to `lm32_juart_set_jrx'
  libqemu-lm32-softmmu.fa.p/target_lm32_translate.c.o: in function `lm32_cpu_dump_state':
  target/lm32/translate.c:1161: undefined reference to `lm32_pic_get_ip'
  target/lm32/translate.c:1161: undefined reference to `lm32_pic_get_im'

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/lm32/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/lm32/Kconfig b/hw/lm32/Kconfig
index 518c84ed508..8ac94205d71 100644
--- a/hw/lm32/Kconfig
+++ b/hw/lm32/Kconfig
@@ -6,11 +6,11 @@ config MILKYMIST
     bool
     # FIXME: disabling it results in compile-time errors
     select MILKYMIST_TMU2 if OPENGL && X11
-    select PTIMER
     select PFLASH_CFI01
     select FRAMEBUFFER
     select SD
     select USB_OHCI
+    select LM32_DEVICES
 
 config LM32_EVR
     bool
-- 
2.26.2