On 11/6/20 5:21 AM, Huacai Chen wrote:
> As suggested by Philippe Mathieu-Daudé, rework Loongson's liointc:
> 1, Move macro definitions to loongson_liointc.h;
> 2, Remove magic values and use macros instead;
> 3, Replace dead D() code by trace events.
>
> Suggested-by: Philippe Mathieu-Daudé <f4...@amsat.org>
> Signed-off-by: Huacai Chen <che...@lemote.com>
> ---
> hw/intc/loongson_liointc.c | 49
> +++++++++++---------------------------
> include/hw/intc/loongson_liointc.h | 39 ++++++++++++++++++++++++++++++
> 2 files changed, 53 insertions(+), 35 deletions(-)
> create mode 100644 include/hw/intc/loongson_liointc.h
>
> diff --git a/hw/intc/loongson_liointc.c b/hw/intc/loongson_liointc.c
> index fbbfb57..be29e2f 100644
> --- a/hw/intc/loongson_liointc.c
> +++ b/hw/intc/loongson_liointc.c
> @@ -1,6 +1,7 @@
> /*
> * QEMU Loongson Local I/O interrupt controler.
> *
> + * Copyright (c) 2020 Huacai Chen <che...@lemote.com>
> * Copyright (c) 2020 Jiaxun Yang <jiaxun.y...@flygoat.com>
> *
> * This program is free software: you can redistribute it and/or modify
> @@ -19,33 +20,11 @@
> */
>
> #include "qemu/osdep.h"
> -#include "hw/sysbus.h"
> #include "qemu/module.h"
> +#include "qemu/log.h"
> #include "hw/irq.h"
> #include "hw/qdev-properties.h"
> -#include "qom/object.h"
> -
> -#define D(x)
> -
> -#define NUM_IRQS 32
> -
> -#define NUM_CORES 4
> -#define NUM_IPS 4
> -#define NUM_PARENTS (NUM_CORES * NUM_IPS)
> -#define PARENT_COREx_IPy(x, y) (NUM_IPS * x + y)
> -
> -#define R_MAPPER_START 0x0
> -#define R_MAPPER_END 0x20
> -#define R_ISR R_MAPPER_END
> -#define R_IEN 0x24
> -#define R_IEN_SET 0x28
> -#define R_IEN_CLR 0x2c
> -#define R_PERCORE_ISR(x) (0x40 + 0x8 * x)
> -#define R_END 0x64
> -
> -#define TYPE_LOONGSON_LIOINTC "loongson.liointc"
> -DECLARE_INSTANCE_CHECKER(struct loongson_liointc, LOONGSON_LIOINTC,
> - TYPE_LOONGSON_LIOINTC)
> +#include "hw/intc/loongson_liointc.h"
>
> struct loongson_liointc {
> SysBusDevice parent_obj;
> @@ -123,14 +102,13 @@ liointc_read(void *opaque, hwaddr addr, unsigned int
> size)
> goto out;
> }
>
> - /* Rest is 4 byte */
> + /* Rest are 4 bytes */
> if (size != 4 || (addr % 4)) {
> goto out;
> }
>
> - if (addr >= R_PERCORE_ISR(0) &&
> - addr < R_PERCORE_ISR(NUM_CORES)) {
> - int core = (addr - R_PERCORE_ISR(0)) / 8;
> + if (addr >= R_START && addr < R_END) {
> + int core = (addr - R_START) / R_ISR_SIZE;
> r = p->per_core_isr[core];
> goto out;
> }
> @@ -147,7 +125,8 @@ liointc_read(void *opaque, hwaddr addr, unsigned int size)
> }
>
> out:
> - D(qemu_log("%s: size=%d addr=%lx val=%x\n", __func__, size, addr, r));
> + qemu_log_mask(CPU_LOG_INT, "%s: size=%d addr=%lx val=%x\n",
> + __func__, size, addr, r);
> return r;
> }
>
> @@ -158,7 +137,8 @@ liointc_write(void *opaque, hwaddr addr,
> struct loongson_liointc *p = opaque;
> uint32_t value = val64;
>
> - D(qemu_log("%s: size=%d, addr=%lx val=%x\n", __func__, size, addr,
> value));
> + qemu_log_mask(CPU_LOG_INT, "%s: size=%d, addr=%lx val=%x\n",
> + __func__, size, addr, value);
>
> /* Mapper is 1 byte */
> if (size == 1 && addr < R_MAPPER_END) {
> @@ -166,14 +146,13 @@ liointc_write(void *opaque, hwaddr addr,
> goto out;
> }
>
> - /* Rest is 4 byte */
> + /* Rest are 4 bytes */
> if (size != 4 || (addr % 4)) {
> goto out;
> }
>
> - if (addr >= R_PERCORE_ISR(0) &&
> - addr < R_PERCORE_ISR(NUM_CORES)) {
> - int core = (addr - R_PERCORE_ISR(0)) / 8;
> + if (addr >= R_START && addr < R_END) {
> + int core = (addr - R_START) / R_ISR_SIZE;
> p->per_core_isr[core] = value;
> goto out;
> }
> @@ -224,7 +203,7 @@ static void loongson_liointc_init(Object *obj)
> }
>
> memory_region_init_io(&p->mmio, obj, &pic_ops, p,
> - "loongson.liointc", R_END);
> + TYPE_LOONGSON_LIOINTC, R_END);
> sysbus_init_mmio(SYS_BUS_DEVICE(obj), &p->mmio);
> }
>
> diff --git a/include/hw/intc/loongson_liointc.h
> b/include/hw/intc/loongson_liointc.h
> new file mode 100644
> index 0000000..e11f482
> --- /dev/null
> +++ b/include/hw/intc/loongson_liointc.h
> @@ -0,0 +1,39 @@
> +/*
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License. See the file "COPYING" in the main directory of this archive
> + * for more details.
> + *
> + * Copyright (c) 2020 Huacai Chen <che...@lemote.com>
> + * Copyright (c) 2020 Jiaxun Yang <jiaxun.y...@flygoat.com>
> + *
> + */
> +
> +#ifndef LOONSGON_LIOINTC_H
> +#define LOONGSON_LIOINTC_H
> +
> +#include "qemu/units.h"
> +#include "hw/sysbus.h"
> +#include "qom/object.h"
> +
> +#define NUM_IRQS 32
> +
> +#define NUM_CORES 4
> +#define NUM_IPS 4
> +#define NUM_PARENTS (NUM_CORES * NUM_IPS)
> +#define PARENT_COREx_IPy(x, y) (NUM_IPS * x + y)
> +
> +#define R_MAPPER_START 0x0
> +#define R_MAPPER_END 0x20
> +#define R_ISR R_MAPPER_END
> +#define R_IEN 0x24
> +#define R_IEN_SET 0x28
> +#define R_IEN_CLR 0x2c
> +#define R_ISR_SIZE 0x8
> +#define R_START 0x40
> +#define R_END 0x64
Can we keep the R_* definitions local in the .c?
(if you agree I can amend that when applying).
Thanks for cleaning!
Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org>
> +
> +#define TYPE_LOONGSON_LIOINTC "loongson.liointc"
> +DECLARE_INSTANCE_CHECKER(struct loongson_liointc, LOONGSON_LIOINTC,
> + TYPE_LOONGSON_LIOINTC)
> +
> +#endif /* LOONGSON_LIOINTC_H */
>
