[Qemu-devel] [PATCH v3 06/42] sdhci: refactor common sysbus/pci realize() into sdhci_realizefn()

Philippe Mathieu-Daudé posted 42 patches 8 years, 1 month ago
Only 39 patches received!
[Qemu-devel] [PATCH v3 06/42] sdhci: refactor common sysbus/pci realize() into sdhci_realizefn()
Posted by Philippe Mathieu-Daudé 8 years, 1 month ago
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@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)
+{
+    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


Re: [Qemu-devel] [PATCH v3 06/42] sdhci: refactor common sysbus/pci realize() into sdhci_realizefn()
Posted by Fam Zheng 8 years, 1 month ago
On Fri, 12/29 14:48, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Reviewed-by: Alistair Francis <alistair.francis@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).

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
> 
> 

Re: [Qemu-devel] [PATCH v3 06/42] sdhci: refactor common sysbus/pci realize() into sdhci_realizefn()
Posted by Philippe Mathieu-Daudé 8 years, 1 month ago
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é <f4bug@amsat.org>
>> Reviewed-by: Alistair Francis <alistair.francis@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
>>
>>