Re: [PATCH] hw/char/mchp_pfsoc_mmuart: QOM'ify PolarFire MMUART

[Philippe Mathieu-Daudé]

Hi Bin,

On 9/23/21 12:29, Philippe Mathieu-Daudé wrote:
> On 9/23/21 07:16, Bin Meng wrote:> On Sun, Sep 19, 2021 at 2:07 AM 
> Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> > > - Embed SerialMM in MchpPfSoCMMUartState and QOM-initialize it
> > > - Alias SERIAL_MM 'chardev' property on MCHP_PFSOC_UART
> > > - Forward SerialMM sysbus IRQ in mchp_pfsoc_mmuart_realize()
> > > - Keep mchp_pfsoc_mmuart_create() behavior

> > > +    if (!sysbus_realize(SYS_BUS_DEVICE(&s->serial_mm), errp)) {
> > > +        return;
> > > +    }
> > > +    sysbus_pass_irq(SYS_BUS_DEVICE(dev), 
> > > SYS_BUS_DEVICE(&s->serial_mm));
> > > +    memory_region_add_subregion(&s->iomem, 0x20,

So the bug is here, the serial is at 0x00 and the other register
stubs at 0x20. I can see u-boot console doing s/0x20/0x00/.

> > > +                
> > > sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->serial_mm), 0));
> > > +}
> > > +
> > > +static void mchp_pfsoc_mmuart_class_init(ObjectClass *oc, void *data)
> > > +{
> > > +    DeviceClass *dc = DEVICE_CLASS(oc);
> > > +
> > > +    dc->realize = mchp_pfsoc_mmuart_realize;
> > > +}
> > > +
> > > +static const TypeInfo mchp_pfsoc_mmuart_info = {
> > > +    .name          = TYPE_MCHP_PFSOC_UART,
> > > +    .parent        = TYPE_SYS_BUS_DEVICE,
> > > +    .instance_size = sizeof(MchpPfSoCMMUartState),
> > > +    .instance_init = mchp_pfsoc_mmuart_init,
> > > +    .class_init    = mchp_pfsoc_mmuart_class_init,
> > > +};
> > > +
> > > +static void mchp_pfsoc_mmuart_register_types(void)
> > > +{
> > > +    type_register_static(&mchp_pfsoc_mmuart_info);
> > > +}
> > > +
> > > +type_init(mchp_pfsoc_mmuart_register_types)
> > > +
> > > +MchpPfSoCMMUartState *mchp_pfsoc_mmuart_create(MemoryRegion *sysmem,
> > > +                                               hwaddr base,
> > > +                                               qemu_irq irq, Chardev 
> > > *chr)
> > > +{
> > > +    DeviceState *dev = qdev_new(TYPE_MCHP_PFSOC_UART);
> > > +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> > > +
> > > +    qdev_prop_set_chr(dev, "chardev", chr);
> > > +    sysbus_realize(sbd, &error_fatal);
> > > +
> > > +    memory_region_add_subregion(sysmem, base, 
> > > sysbus_mmio_get_region(sbd, 0));
> > > +    sysbus_connect_irq(sbd, 0, irq);
> > > +
> > > +    return MCHP_PFSOC_UART(dev);
> > >   }
> >
> > This patch unfortunately breaks the polarfire machine that no serial
> > output is seen. I did not take a further look yet.
>
> Doh, it passed the CI... Ah, now I see, this machine is not covered
> by CI, only manual testing per 
> docs/system/riscv/microchip-icicle-kit.rst... I'll have a look during 
> the week-end.
>
> Regards,
>
> Phil.