Re: [PATCH v5 16/21] ppc4xx_sdram: Move ppc4xx DDR and DDR2 SDRAM controller models together

[BALATON Zoltan <balaton@eik.bme.hu>]

[-- Attachment #1: Type: text/plain, Size: 2054 bytes --]

On Sun, 18 Sep 2022, Philippe Mathieu-Daudé wrote:
> Hi Zoltan,
>
> On 18/9/22 22:24, BALATON Zoltan wrote:
>> Move the PPC4xx DDR and DDR2 SDRAM contrller models into a new file
>> called ppc4xx_sdram to separate from other device models and put them
>> in one place allowing sharing some code between them.
>> 
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>>   hw/ppc/meson.build      |   3 +-
>>   hw/ppc/ppc440_uc.c      | 332 -----------------
>>   hw/ppc/ppc4xx_devs.c    | 414 ---------------------
>>   hw/ppc/ppc4xx_sdram.c   | 771 ++++++++++++++++++++++++++++++++++++++++
>>   include/hw/ppc/ppc4xx.h |  24 +-
>>   5 files changed, 785 insertions(+), 759 deletions(-)
>
> This seems a proper cleanup, but even using `git-diff 
> --color-moved=dimmed-zebra` I'm having hard time reviewing this single
> patch.
> Looking at the changes in the ppc4xx_sdram_types[] array, it
> seems we can be move one model at a time, right?

I could try to break this patch up some more if absolutely necessary but 
I've already spent a lot of time rebasing this series as every little 
change in earlier patch leads to conflicts later then patches get squashed 
during rebase and I have to redo it again to separate them. So if you 
can't review it looking at it some more I could try a v6 but don't want if 
can be avoided. I'll wait for Cédric's comments too in any case to only do 
as many respins as needed.

Regards,
BALATON Zoltan

> [...]
>> +static const TypeInfo ppc4xx_sdram_types[] = {
>> +    {
>> +        .name           = TYPE_PPC4xx_SDRAM_DDR,
>> +        .parent         = TYPE_PPC4xx_DCR_DEVICE,
>> +        .instance_size  = sizeof(Ppc4xxSdramDdrState),
>> +        .class_init     = ppc4xx_sdram_ddr_class_init,
>> +    }, {
>> +        .name           = TYPE_PPC4xx_SDRAM_DDR2,
>> +        .parent         = TYPE_PPC4xx_DCR_DEVICE,
>> +        .instance_size  = sizeof(Ppc4xxSdramDdr2State),
>> +        .class_init     = ppc4xx_sdram_ddr2_class_init,
>> +    }
>> +};
>> +
>> +DEFINE_TYPES(ppc4xx_sdram_types)
> [...]
>
>