Hi Fam, On 01/03/2018 04:52 AM, Fam Zheng wrote: > On Fri, 12/29 14:48, Philippe Mathieu-Daudé wrote: >> Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org> >> Reviewed-by: Alistair Francis <alistair.fran...@xilinx.com> >> --- >> hw/sd/sdhci.c | 22 ++++++++++++++-------- >> 1 file changed, 14 insertions(+), 8 deletions(-) >> >> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c >> index 38d82b4c61..ad5853d527 100644 >> --- a/hw/sd/sdhci.c >> +++ b/hw/sd/sdhci.c >> @@ -1194,6 +1194,15 @@ static void sdhci_initfn(SDHCIState *s) >> s->transfer_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, >> sdhci_data_transfer, s); >> } >> >> +static void sdhci_realizefn(SDHCIState *s, Error **errp) > > errp is not used here. It doesn't hurt to have it but if the contract is "the > function MAY return error", the callers should check and handle it correctly > (like return early instead of continuing).
Oh I see, good point, thanks! > Fam > >> +{ >> + s->buf_maxsz = sdhci_get_fifolen(s); >> + s->fifo_buffer = g_malloc0(s->buf_maxsz); >> + >> + memory_region_init_io(&s->iomem, OBJECT(s), &sdhci_mmio_ops, s, "sdhci", >> + SDHC_REGISTERS_MAP_SIZE); >> +} >> + >> static void sdhci_uninitfn(SDHCIState *s) >> { >> timer_del(s->insert_timer); >> @@ -1292,12 +1301,11 @@ static void sdhci_pci_realize(PCIDevice *dev, Error >> **errp) >> SDHCIState *s = PCI_SDHCI(dev); >> dev->config[PCI_CLASS_PROG] = 0x01; /* Standard Host supported DMA */ >> dev->config[PCI_INTERRUPT_PIN] = 0x01; /* interrupt pin A */ >> + >> sdhci_initfn(s); >> - s->buf_maxsz = sdhci_get_fifolen(s); >> - s->fifo_buffer = g_malloc0(s->buf_maxsz); >> + sdhci_realizefn(s, errp); >> + >> s->irq = pci_allocate_irq(dev); >> - memory_region_init_io(&s->iomem, OBJECT(s), &sdhci_mmio_ops, s, "sdhci", >> - SDHC_REGISTERS_MAP_SIZE); >> pci_register_bar(dev, 0, 0, &s->iomem); >> } >> >> @@ -1351,11 +1359,9 @@ static void sdhci_sysbus_realize(DeviceState *dev, >> Error ** errp) >> SDHCIState *s = SYSBUS_SDHCI(dev); >> SysBusDevice *sbd = SYS_BUS_DEVICE(dev); >> >> - s->buf_maxsz = sdhci_get_fifolen(s); >> - s->fifo_buffer = g_malloc0(s->buf_maxsz); >> + sdhci_realizefn(s, errp); >> + >> sysbus_init_irq(sbd, &s->irq); >> - memory_region_init_io(&s->iomem, OBJECT(s), &sdhci_mmio_ops, s, "sdhci", >> - SDHC_REGISTERS_MAP_SIZE); >> sysbus_init_mmio(sbd, &s->iomem); >> } >> >> -- >> 2.15.1 >> >>