Re: [PATCH v3 04/17] hw/intc/loongson_ipi: Extract loongson_ipi_common_realize()

[Philippe Mathieu-Daudé]

On 18/7/24 04:11, maobibo wrote:
> On 2024/7/18 上午5:46, Philippe Mathieu-Daudé wrote:
> > From: Bibo Mao <maobibo@loongson.cn>
> >
> > In preparation to extract common IPI code in few commits,
> > extract loongson_ipi_common_realize().
> >
> > Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> > [PMD: Extracted from bigger commit, added commit description]
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> >   hw/intc/loongson_ipi.c | 25 ++++++++++++++++++-------
> >   1 file changed, 18 insertions(+), 7 deletions(-)
> >
> > diff --git a/hw/intc/loongson_ipi.c b/hw/intc/loongson_ipi.c
> > index 3b3481c43e..40ac769aad 100644
> > --- a/hw/intc/loongson_ipi.c
> > +++ b/hw/intc/loongson_ipi.c
> > @@ -275,7 +275,7 @@ static const MemoryRegionOps loongson_ipi64_ops = {
> >       .endianness = DEVICE_LITTLE_ENDIAN,
> >   };
> > -static void loongson_ipi_realize(DeviceState *dev, Error **errp)
> > +static void loongson_ipi_common_realize(DeviceState *dev, Error **errp)
> >   {
> >       LoongsonIPIState *s = LOONGSON_IPI(dev);
> >       SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> > @@ -301,20 +301,31 @@ static void loongson_ipi_realize(DeviceState 
> > *dev, Error **errp)
> >       sysbus_init_mmio(sbd, &s->ipi64_iocsr_mem);
> >       s->cpu = g_new0(IPICore, s->num_cpu);
> > -    if (s->cpu == NULL) {
> > -        error_setg(errp, "Memory allocation for IPICore faile");
> Philippe,
>
> Thanks for the whole series, it looks to me. It is split into small 
> patches and adds new option CONFIG_LOONGSON_IPI_COMMON, it is easier to 
> review and compile for multiple targets.
>
> One small nit, do we need keep checking sentence for if (s->cpu == NULL)?

No because g_new0() can not fail. Checking return value only
makes sense for g_try_new0() which returns.

> Overall, for the whole series it is ok for me and works well on 
> LoongArch machine.

Thanks!

> Reviewed-by: Bibo Mao <maobibo@loongson.cn>
> Tested-by: Bibo Mao <maobibo@loongson.cn>

Jiaxun, do you mind re-testing the series for your MIPS machine?

Regards,

Phil.