Re: [PATCH 4/5] ppc405_boards: use qdev properties instead of legacy m48t59_init() function

[Artyom Tarasenko]

On Fri, Oct 16, 2020 at 10:38 PM BALATON Zoltan <balaton@eik.bme.hu> wrote:
>
> On Fri, 16 Oct 2020, Mark Cave-Ayland wrote:
> > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> > ---
> > hw/ppc/ppc405_boards.c | 10 +++++++++-
> > 1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
> > index 6198ec1035..4687715b15 100644
> > --- a/hw/ppc/ppc405_boards.c
> > +++ b/hw/ppc/ppc405_boards.c
> > @@ -28,6 +28,8 @@
> > #include "qemu-common.h"
> > #include "cpu.h"
> > #include "hw/ppc/ppc.h"
> > +#include "hw/qdev-properties.h"
> > +#include "hw/sysbus.h"
> > #include "ppc405.h"
> > #include "hw/rtc/m48t59.h"
> > #include "hw/block/flash.h"
> > @@ -145,6 +147,8 @@ static void ref405ep_init(MachineState *machine)
> >     char *filename;
> >     ppc4xx_bd_info_t bd;
> >     CPUPPCState *env;
> > +    DeviceState *dev;
> > +    SysBusDevice *s;
> >     qemu_irq *pic;
> >     MemoryRegion *bios;
> >     MemoryRegion *sram = g_new(MemoryRegion, 1);
> > @@ -227,7 +231,11 @@ static void ref405ep_init(MachineState *machine)
> >     /* Register FPGA */
> >     ref405ep_fpga_init(sysmem, 0xF0300000);
> >     /* Register NVRAM */
> > -    m48t59_init(NULL, 0xF0000000, 0, 8192, 1968, 8);
> > +    dev = qdev_new("sysbus-m48t08");
> > +    qdev_prop_set_int32(dev, "base-year", 1968);
>
> Is there anything that uses other than 1968 as base year?

I think 40p uses 1900 as the base year.

> If not this
> could be the default in the device so you don't need these set prop calls
> here and in sun machines.
>
> The only other place this device is used seems to be ppc/prep machine that
> uses the isa version but does not set a base year. Is that a bug? The
> original prep machine removed in b2ce76a0730 used 2000 but that's unlikely
> as well as these machines predate that. Anyway, the sysbus and isa
> versions are different so their default base-year could be set
> independently and then boards won't need to set this propery and may be
> could use qdev_create_simple instead or whatever that was renamed to.
>
> Regards,
> BALATON Zoltan
>
> > +    s = SYS_BUS_DEVICE(dev);
> > +    sysbus_realize_and_unref(s, &error_fatal);
> > +    sysbus_mmio_map(s, 0, 0xF0000000);
> >     /* Load kernel */
> >     linux_boot = (kernel_filename != NULL);
> >     if (linux_boot) {
> >