| Message ID | 1604636510-8347-3-git-send-email-chenhc@lemote.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mips: Add Loongson-3 machine support | expand |
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é <f4bug@amsat.org> > Signed-off-by: Huacai Chen <chenhc@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 <chenhc@lemote.com> > * Copyright (c) 2020 Jiaxun Yang <jiaxun.yang@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 <chenhc@lemote.com> > + * Copyright (c) 2020 Jiaxun Yang <jiaxun.yang@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é <f4bug@amsat.org> > + > +#define TYPE_LOONGSON_LIOINTC "loongson.liointc" > +DECLARE_INSTANCE_CHECKER(struct loongson_liointc, LOONGSON_LIOINTC, > + TYPE_LOONGSON_LIOINTC) > + > +#endif /* LOONGSON_LIOINTC_H */ >
On 11/23/20 9:52 PM, Philippe Mathieu-Daudé wrote: > 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é <f4bug@amsat.org> >> Signed-off-by: Huacai Chen <chenhc@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 <chenhc@lemote.com> >> * Copyright (c) 2020 Jiaxun Yang <jiaxun.yang@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 <chenhc@lemote.com> >> + * Copyright (c) 2020 Jiaxun Yang <jiaxun.yang@flygoat.com> >> + * >> + */ >> + >> +#ifndef LOONSGON_LIOINTC_H >> +#define LOONGSON_LIOINTC_H Clang is smart: hw/intc/loongson_liointc.h:11:9: error: 'LOONSGON_LIOINTC_H' is used as a header guard here, followed by #define of a different macro [-Werror,-Wheader-guard] #ifndef LOONSGON_LIOINTC_H ^~~~~~~~~~~~~~~~~~ include/hw/intc/loongson_liointc.h:12:9: note: 'LOONGSON_LIOINTC_H' is defined here; did you mean 'LOONSGON_LIOINTC_H'? #define LOONGSON_LIOINTC_H ^~~~~~~~~~~~~~~~~~ LOONSGON_LIOINTC_H 1 error generated. Once fixed: Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >> + >> +#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é <f4bug@amsat.org> > >> + >> +#define TYPE_LOONGSON_LIOINTC "loongson.liointc" >> +DECLARE_INSTANCE_CHECKER(struct loongson_liointc, LOONGSON_LIOINTC, >> + TYPE_LOONGSON_LIOINTC) >> + >> +#endif /* LOONGSON_LIOINTC_H */ >> >
Hi, Philippe, On Tue, Nov 24, 2020 at 4:52 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > > 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é <f4bug@amsat.org> > > Signed-off-by: Huacai Chen <chenhc@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 <chenhc@lemote.com> > > * Copyright (c) 2020 Jiaxun Yang <jiaxun.yang@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 <chenhc@lemote.com> > > + * Copyright (c) 2020 Jiaxun Yang <jiaxun.yang@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). If put them in .c, then .h is to small.., but TYPE_LOONGSON_LIOINTC should be defined in .h since it will be used in other place. Huacai > > Thanks for cleaning! > > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > > > + > > +#define TYPE_LOONGSON_LIOINTC "loongson.liointc" > > +DECLARE_INSTANCE_CHECKER(struct loongson_liointc, LOONGSON_LIOINTC, > > + TYPE_LOONGSON_LIOINTC) > > + > > +#endif /* LOONGSON_LIOINTC_H */ > >
Hi, Philippe, On Tue, Nov 24, 2020 at 6:24 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > > On 11/23/20 9:52 PM, Philippe Mathieu-Daudé wrote: > > 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é <f4bug@amsat.org> > >> Signed-off-by: Huacai Chen <chenhc@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 <chenhc@lemote.com> > >> * Copyright (c) 2020 Jiaxun Yang <jiaxun.yang@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 <chenhc@lemote.com> > >> + * Copyright (c) 2020 Jiaxun Yang <jiaxun.yang@flygoat.com> > >> + * > >> + */ > >> + > >> +#ifndef LOONSGON_LIOINTC_H > >> +#define LOONGSON_LIOINTC_H > > Clang is smart: > > hw/intc/loongson_liointc.h:11:9: error: 'LOONSGON_LIOINTC_H' is used as > a header guard here, followed by #define of a different macro > [-Werror,-Wheader-guard] > #ifndef LOONSGON_LIOINTC_H > ^~~~~~~~~~~~~~~~~~ > include/hw/intc/loongson_liointc.h:12:9: note: 'LOONGSON_LIOINTC_H' is > defined here; did you mean 'LOONSGON_LIOINTC_H'? > #define LOONGSON_LIOINTC_H > ^~~~~~~~~~~~~~~~~~ > LOONSGON_LIOINTC_H > 1 error generated. > > Once fixed: > Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> OK, will be fixed in the next version. > > >> + > >> +#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é <f4bug@amsat.org> > > > >> + > >> +#define TYPE_LOONGSON_LIOINTC "loongson.liointc" > >> +DECLARE_INSTANCE_CHECKER(struct loongson_liointc, LOONGSON_LIOINTC, > >> + TYPE_LOONGSON_LIOINTC) > >> + > >> +#endif /* LOONGSON_LIOINTC_H */ > >> > >
On 11/28/20 7:19 AM, Huacai Chen wrote: > On Tue, Nov 24, 2020 at 4:52 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: >> 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é <f4bug@amsat.org> >>> Signed-off-by: Huacai Chen <chenhc@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 <chenhc@lemote.com> >>> * Copyright (c) 2020 Jiaxun Yang <jiaxun.yang@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 <chenhc@lemote.com> >>> + * Copyright (c) 2020 Jiaxun Yang <jiaxun.yang@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). > If put them in .c, then .h is to small.., Not a problem: include/hw/ppc/openpic_kvm.h include/hw/display/ramfb.h include/hw/input/lasips2.h include/hw/usb/chipidea.h include/hw/s390x/ap-bridge.h include/hw/char/lm32_juart.h include/hw/isa/vt82c686.h include/hw/net/lan9118.h include/hw/intc/imx_gpcv2.h include/hw/usb/xhci.h > but TYPE_LOONGSON_LIOINTC > should be defined in .h since it will be used in other place. > > Huacai >> >> Thanks for cleaning! >> >> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >> >>> + >>> +#define TYPE_LOONGSON_LIOINTC "loongson.liointc" >>> +DECLARE_INSTANCE_CHECKER(struct loongson_liointc, LOONGSON_LIOINTC, >>> + TYPE_LOONGSON_LIOINTC) >>> + >>> +#endif /* LOONGSON_LIOINTC_H */ >>> >
Hi, Phillippe, On Mon, Nov 30, 2020 at 6:08 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > > On 11/28/20 7:19 AM, Huacai Chen wrote: > > On Tue, Nov 24, 2020 at 4:52 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > >> 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é <f4bug@amsat.org> > >>> Signed-off-by: Huacai Chen <chenhc@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 <chenhc@lemote.com> > >>> * Copyright (c) 2020 Jiaxun Yang <jiaxun.yang@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 <chenhc@lemote.com> > >>> + * Copyright (c) 2020 Jiaxun Yang <jiaxun.yang@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). > > If put them in .c, then .h is to small.., > > Not a problem: > > include/hw/ppc/openpic_kvm.h > include/hw/display/ramfb.h > include/hw/input/lasips2.h > include/hw/usb/chipidea.h > include/hw/s390x/ap-bridge.h > include/hw/char/lm32_juart.h > include/hw/isa/vt82c686.h > include/hw/net/lan9118.h > include/hw/intc/imx_gpcv2.h > include/hw/usb/xhci.h OK, I will put all these macros in .c file. Huacai > > > but TYPE_LOONGSON_LIOINTC > > should be defined in .h since it will be used in other place. > > > > Huacai > >> > >> Thanks for cleaning! > >> > >> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > >> > >>> + > >>> +#define TYPE_LOONGSON_LIOINTC "loongson.liointc" > >>> +DECLARE_INSTANCE_CHECKER(struct loongson_liointc, LOONGSON_LIOINTC, > >>> + TYPE_LOONGSON_LIOINTC) > >>> + > >>> +#endif /* 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 <chenhc@lemote.com> * Copyright (c) 2020 Jiaxun Yang <jiaxun.yang@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 <chenhc@lemote.com> + * Copyright (c) 2020 Jiaxun Yang <jiaxun.yang@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 + +#define TYPE_LOONGSON_LIOINTC "loongson.liointc" +DECLARE_INSTANCE_CHECKER(struct loongson_liointc, LOONGSON_LIOINTC, + TYPE_LOONGSON_LIOINTC) + +#endif /* LOONGSON_LIOINTC_H */
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é <f4bug@amsat.org> Signed-off-by: Huacai Chen <chenhc@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