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

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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.