| Message ID | 20210208135048.2601693-2-f4bug@amsat.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/{lm32,sh4}: Kconfig cleanups | expand |
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
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.
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.)
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'))