[RFC] hw/sd/sdhci: Move PCI-related code into a separate file

Some machines have an SDHCI device, but no PCI. To be able to
compile hw/sd/sdhci.c without CONFIG_PCI, we must not call functions
like pci_get_address_space() and pci_allocate_irq() there. Thus
move the PCI-related code into a separate file.

This is required for the upcoming Kconfig-like build system, e.g. it
is needed if a user wants to compile a QEMU binary with just one machine
that has SDHCI, but no PCI, like the ARM "raspi" machines for example.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 Note: Once we've got Kconfig in place, the "land" in the Makefile
 should be replaced with a proper new CONFIG_SDHCI_PCI switch instead

 hw/sd/Makefile.objs    |  1 +
 hw/sd/sdhci-internal.h | 34 ++++++++++++++++++
 hw/sd/sdhci-pci.c      | 87 ++++++++++++++++++++++++++++++++++++++++++++
 hw/sd/sdhci.c          | 98 +++-----------------------------------------------
 4 files changed, 127 insertions(+), 93 deletions(-)
 create mode 100644 hw/sd/sdhci-pci.c

On 2/20/19 1:06 PM, Thomas Huth wrote:
> Some machines have an SDHCI device, but no PCI. To be able to
> compile hw/sd/sdhci.c without CONFIG_PCI, we must not call functions
> like pci_get_address_space() and pci_allocate_irq() there. Thus
> move the PCI-related code into a separate file.
> 
> This is required for the upcoming Kconfig-like build system, e.g. it
> is needed if a user wants to compile a QEMU binary with just one machine
> that has SDHCI, but no PCI, like the ARM "raspi" machines for example.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>

I wonder if we should add a F: entry into the "PC Chipset" section of
MAINTAINERS.

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  Note: Once we've got Kconfig in place, the "land" in the Makefile
>  should be replaced with a proper new CONFIG_SDHCI_PCI switch instead
> 
>  hw/sd/Makefile.objs    |  1 +
>  hw/sd/sdhci-internal.h | 34 ++++++++++++++++++
>  hw/sd/sdhci-pci.c      | 87 ++++++++++++++++++++++++++++++++++++++++++++
>  hw/sd/sdhci.c          | 98 +++-----------------------------------------------
>  4 files changed, 127 insertions(+), 93 deletions(-)
>  create mode 100644 hw/sd/sdhci-pci.c
> 
> diff --git a/hw/sd/Makefile.objs b/hw/sd/Makefile.objs
> index a99d9fb..58be7b3 100644
> --- a/hw/sd/Makefile.objs
> +++ b/hw/sd/Makefile.objs
> @@ -2,6 +2,7 @@ common-obj-$(CONFIG_PL181) += pl181.o
>  common-obj-$(CONFIG_SSI_SD) += ssi-sd.o
>  common-obj-$(CONFIG_SD) += sd.o core.o sdmmc-internal.o
>  common-obj-$(CONFIG_SDHCI) += sdhci.o
> +common-obj-$(call land,$(CONFIG_SDHCI),$(CONFIG_PCI)) += sdhci-pci.o
>  
>  obj-$(CONFIG_MILKYMIST) += milkymist-memcard.o
>  obj-$(CONFIG_OMAP) += omap_mmc.o
> diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h
> index 19665fd..3414140 100644
> --- a/hw/sd/sdhci-internal.h
> +++ b/hw/sd/sdhci-internal.h
> @@ -304,4 +304,38 @@ extern const VMStateDescription sdhci_vmstate;
>  
>  #define ESDHC_PRNSTS_SDSTB              (1 << 3)
>  
> +/*
> + * Default SD/MMC host controller features information, which will be
> + * presented in CAPABILITIES register of generic SD host controller at reset.
> + *
> + * support:
> + * - 3.3v and 1.8v voltages
> + * - SDMA/ADMA1/ADMA2
> + * - high-speed
> + * max host controller R/W buffers size: 512B
> + * max clock frequency for SDclock: 52 MHz
> + * timeout clock frequency: 52 MHz
> + *
> + * does not support:
> + * - 3.0v voltage
> + * - 64-bit system bus
> + * - suspend/resume
> + */
> +#define SDHC_CAPAB_REG_DEFAULT 0x057834b4
> +
> +#define DEFINE_SDHCI_COMMON_PROPERTIES(_state) \
> +    DEFINE_PROP_UINT8("sd-spec-version", _state, sd_spec_version, 2), \
> +    DEFINE_PROP_UINT8("uhs", _state, uhs_mode, UHS_NOT_SUPPORTED), \
> +    \
> +    /* Capabilities registers provide information on supported
> +     * features of this specific host controller implementation */ \
> +    DEFINE_PROP_UINT64("capareg", _state, capareg, SDHC_CAPAB_REG_DEFAULT), \
> +    DEFINE_PROP_UINT64("maxcurr", _state, maxcurr, 0)
> +
> +void sdhci_initfn(SDHCIState *s);
> +void sdhci_uninitfn(SDHCIState *s);
> +void sdhci_common_realize(SDHCIState *s, Error **errp);
> +void sdhci_common_unrealize(SDHCIState *s, Error **errp);
> +void sdhci_common_class_init(ObjectClass *klass, void *data);
> +
>  #endif
> diff --git a/hw/sd/sdhci-pci.c b/hw/sd/sdhci-pci.c
> new file mode 100644
> index 0000000..f884661
> --- /dev/null
> +++ b/hw/sd/sdhci-pci.c
> @@ -0,0 +1,87 @@
> +/*
> + * SDHCI device on PCI
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> + * See the GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "hw/hw.h"
> +#include "hw/sd/sdhci.h"
> +#include "sdhci-internal.h"
> +
> +static Property sdhci_pci_properties[] = {
> +    DEFINE_SDHCI_COMMON_PROPERTIES(SDHCIState),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void sdhci_pci_realize(PCIDevice *dev, Error **errp)
> +{
> +    SDHCIState *s = PCI_SDHCI(dev);
> +    Error *local_err = NULL;
> +
> +    sdhci_initfn(s);
> +    sdhci_common_realize(s, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
> +
> +    dev->config[PCI_CLASS_PROG] = 0x01; /* Standard Host supported DMA */
> +    dev->config[PCI_INTERRUPT_PIN] = 0x01; /* interrupt pin A */
> +    s->irq = pci_allocate_irq(dev);
> +    s->dma_as = pci_get_address_space(dev);
> +    pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->iomem);
> +}
> +
> +static void sdhci_pci_exit(PCIDevice *dev)
> +{
> +    SDHCIState *s = PCI_SDHCI(dev);
> +
> +    sdhci_common_unrealize(s, &error_abort);
> +    sdhci_uninitfn(s);
> +}
> +
> +static void sdhci_pci_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> +
> +    k->realize = sdhci_pci_realize;
> +    k->exit = sdhci_pci_exit;
> +    k->vendor_id = PCI_VENDOR_ID_REDHAT;
> +    k->device_id = PCI_DEVICE_ID_REDHAT_SDHCI;
> +    k->class_id = PCI_CLASS_SYSTEM_SDHCI;
> +    dc->props = sdhci_pci_properties;
> +
> +    sdhci_common_class_init(klass, data);
> +}
> +
> +static const TypeInfo sdhci_pci_info = {
> +    .name = TYPE_PCI_SDHCI,
> +    .parent = TYPE_PCI_DEVICE,
> +    .instance_size = sizeof(SDHCIState),
> +    .class_init = sdhci_pci_class_init,
> +    .interfaces = (InterfaceInfo[]) {
> +        { INTERFACE_CONVENTIONAL_PCI_DEVICE },
> +        { },
> +    },
> +};
> +
> +static void sdhci_pci_register_type(void)
> +{
> +    type_register_static(&sdhci_pci_info);
> +}
> +
> +type_init(sdhci_pci_register_type)
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index 83f1574..17ad546 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -40,24 +40,6 @@
>  
>  #define MASKED_WRITE(reg, mask, val)  (reg = (reg & (mask)) | (val))
>  
> -/* Default SD/MMC host controller features information, which will be
> - * presented in CAPABILITIES register of generic SD host controller at reset.
> - *
> - * support:
> - * - 3.3v and 1.8v voltages
> - * - SDMA/ADMA1/ADMA2
> - * - high-speed
> - * max host controller R/W buffers size: 512B
> - * max clock frequency for SDclock: 52 MHz
> - * timeout clock frequency: 52 MHz
> - *
> - * does not support:
> - * - 3.0v voltage
> - * - 64-bit system bus
> - * - suspend/resume
> - */
> -#define SDHC_CAPAB_REG_DEFAULT 0x057834b4
> -
>  static inline unsigned int sdhci_get_fifolen(SDHCIState *s)
>  {
>      return 1 << (9 + FIELD_EX32(s->capareg, SDHC_CAPAB, MAXBLOCKLENGTH));
> @@ -1328,16 +1310,7 @@ static void sdhci_init_readonly_registers(SDHCIState *s, Error **errp)
>  
>  /* --- qdev common --- */
>  
> -#define DEFINE_SDHCI_COMMON_PROPERTIES(_state) \
> -    DEFINE_PROP_UINT8("sd-spec-version", _state, sd_spec_version, 2), \
> -    DEFINE_PROP_UINT8("uhs", _state, uhs_mode, UHS_NOT_SUPPORTED), \
> -    \
> -    /* Capabilities registers provide information on supported
> -     * features of this specific host controller implementation */ \
> -    DEFINE_PROP_UINT64("capareg", _state, capareg, SDHC_CAPAB_REG_DEFAULT), \
> -    DEFINE_PROP_UINT64("maxcurr", _state, maxcurr, 0)
> -
> -static void sdhci_initfn(SDHCIState *s)
> +void sdhci_initfn(SDHCIState *s)
>  {
>      qbus_create_inplace(&s->sdbus, sizeof(s->sdbus),
>                          TYPE_SDHCI_BUS, DEVICE(s), "sd-bus");
> @@ -1348,7 +1321,7 @@ static void sdhci_initfn(SDHCIState *s)
>      s->io_ops = &sdhci_mmio_ops;
>  }
>  
> -static void sdhci_uninitfn(SDHCIState *s)
> +void sdhci_uninitfn(SDHCIState *s)
>  {
>      timer_del(s->insert_timer);
>      timer_free(s->insert_timer);
> @@ -1359,7 +1332,7 @@ static void sdhci_uninitfn(SDHCIState *s)
>      s->fifo_buffer = NULL;
>  }
>  
> -static void sdhci_common_realize(SDHCIState *s, Error **errp)
> +void sdhci_common_realize(SDHCIState *s, Error **errp)
>  {
>      Error *local_err = NULL;
>  
> @@ -1375,7 +1348,7 @@ static void sdhci_common_realize(SDHCIState *s, Error **errp)
>                            SDHC_REGISTERS_MAP_SIZE);
>  }
>  
> -static void sdhci_common_unrealize(SDHCIState *s, Error **errp)
> +void sdhci_common_unrealize(SDHCIState *s, Error **errp)
>  {
>      /* This function is expected to be called only once for each class:
>       * - SysBus:    via DeviceClass->unrealize(),
> @@ -1445,7 +1418,7 @@ const VMStateDescription sdhci_vmstate = {
>      },
>  };
>  
> -static void sdhci_common_class_init(ObjectClass *klass, void *data)
> +void sdhci_common_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
>  
> @@ -1454,66 +1427,6 @@ static void sdhci_common_class_init(ObjectClass *klass, void *data)
>      dc->reset = sdhci_poweron_reset;
>  }
>  
> -/* --- qdev PCI --- */
> -
> -static Property sdhci_pci_properties[] = {
> -    DEFINE_SDHCI_COMMON_PROPERTIES(SDHCIState),
> -    DEFINE_PROP_END_OF_LIST(),
> -};
> -
> -static void sdhci_pci_realize(PCIDevice *dev, Error **errp)
> -{
> -    SDHCIState *s = PCI_SDHCI(dev);
> -    Error *local_err = NULL;
> -
> -    sdhci_initfn(s);
> -    sdhci_common_realize(s, &local_err);
> -    if (local_err) {
> -        error_propagate(errp, local_err);
> -        return;
> -    }
> -
> -    dev->config[PCI_CLASS_PROG] = 0x01; /* Standard Host supported DMA */
> -    dev->config[PCI_INTERRUPT_PIN] = 0x01; /* interrupt pin A */
> -    s->irq = pci_allocate_irq(dev);
> -    s->dma_as = pci_get_address_space(dev);
> -    pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->iomem);
> -}
> -
> -static void sdhci_pci_exit(PCIDevice *dev)
> -{
> -    SDHCIState *s = PCI_SDHCI(dev);
> -
> -    sdhci_common_unrealize(s, &error_abort);
> -    sdhci_uninitfn(s);
> -}
> -
> -static void sdhci_pci_class_init(ObjectClass *klass, void *data)
> -{
> -    DeviceClass *dc = DEVICE_CLASS(klass);
> -    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> -
> -    k->realize = sdhci_pci_realize;
> -    k->exit = sdhci_pci_exit;
> -    k->vendor_id = PCI_VENDOR_ID_REDHAT;
> -    k->device_id = PCI_DEVICE_ID_REDHAT_SDHCI;
> -    k->class_id = PCI_CLASS_SYSTEM_SDHCI;
> -    dc->props = sdhci_pci_properties;
> -
> -    sdhci_common_class_init(klass, data);
> -}
> -
> -static const TypeInfo sdhci_pci_info = {
> -    .name = TYPE_PCI_SDHCI,
> -    .parent = TYPE_PCI_DEVICE,
> -    .instance_size = sizeof(SDHCIState),
> -    .class_init = sdhci_pci_class_init,
> -    .interfaces = (InterfaceInfo[]) {
> -        { INTERFACE_CONVENTIONAL_PCI_DEVICE },
> -        { },
> -    },
> -};
> -
>  /* --- qdev SysBus --- */
>  
>  static Property sdhci_sysbus_properties[] = {
> @@ -1846,7 +1759,6 @@ static const TypeInfo imx_usdhc_info = {
>  
>  static void sdhci_register_types(void)
>  {
> -    type_register_static(&sdhci_pci_info);
>      type_register_static(&sdhci_sysbus_info);
>      type_register_static(&sdhci_bus_info);
>      type_register_static(&imx_usdhc_info);
>

On 20/02/19 13:26, Philippe Mathieu-Daudé wrote:
> On 2/20/19 1:06 PM, Thomas Huth wrote:
>> Some machines have an SDHCI device, but no PCI. To be able to
>> compile hw/sd/sdhci.c without CONFIG_PCI, we must not call functions
>> like pci_get_address_space() and pci_allocate_irq() there. Thus
>> move the PCI-related code into a separate file.
>>
>> This is required for the upcoming Kconfig-like build system, e.g. it
>> is needed if a user wants to compile a QEMU binary with just one machine
>> that has SDHCI, but no PCI, like the ARM "raspi" machines for example.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
> 
> I wonder if we should add a F: entry into the "PC Chipset" section of
> MAINTAINERS.

No, I don't think so.  It's not part of PC.

Paolo

> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
>> ---
>>  Note: Once we've got Kconfig in place, the "land" in the Makefile
>>  should be replaced with a proper new CONFIG_SDHCI_PCI switch instead
>>
>>  hw/sd/Makefile.objs    |  1 +
>>  hw/sd/sdhci-internal.h | 34 ++++++++++++++++++
>>  hw/sd/sdhci-pci.c      | 87 ++++++++++++++++++++++++++++++++++++++++++++
>>  hw/sd/sdhci.c          | 98 +++-----------------------------------------------
>>  4 files changed, 127 insertions(+), 93 deletions(-)
>>  create mode 100644 hw/sd/sdhci-pci.c
>>
>> diff --git a/hw/sd/Makefile.objs b/hw/sd/Makefile.objs
>> index a99d9fb..58be7b3 100644
>> --- a/hw/sd/Makefile.objs
>> +++ b/hw/sd/Makefile.objs
>> @@ -2,6 +2,7 @@ common-obj-$(CONFIG_PL181) += pl181.o
>>  common-obj-$(CONFIG_SSI_SD) += ssi-sd.o
>>  common-obj-$(CONFIG_SD) += sd.o core.o sdmmc-internal.o
>>  common-obj-$(CONFIG_SDHCI) += sdhci.o
>> +common-obj-$(call land,$(CONFIG_SDHCI),$(CONFIG_PCI)) += sdhci-pci.o
>>  
>>  obj-$(CONFIG_MILKYMIST) += milkymist-memcard.o
>>  obj-$(CONFIG_OMAP) += omap_mmc.o
>> diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h
>> index 19665fd..3414140 100644
>> --- a/hw/sd/sdhci-internal.h
>> +++ b/hw/sd/sdhci-internal.h
>> @@ -304,4 +304,38 @@ extern const VMStateDescription sdhci_vmstate;
>>  
>>  #define ESDHC_PRNSTS_SDSTB              (1 << 3)
>>  
>> +/*
>> + * Default SD/MMC host controller features information, which will be
>> + * presented in CAPABILITIES register of generic SD host controller at reset.
>> + *
>> + * support:
>> + * - 3.3v and 1.8v voltages
>> + * - SDMA/ADMA1/ADMA2
>> + * - high-speed
>> + * max host controller R/W buffers size: 512B
>> + * max clock frequency for SDclock: 52 MHz
>> + * timeout clock frequency: 52 MHz
>> + *
>> + * does not support:
>> + * - 3.0v voltage
>> + * - 64-bit system bus
>> + * - suspend/resume
>> + */
>> +#define SDHC_CAPAB_REG_DEFAULT 0x057834b4
>> +
>> +#define DEFINE_SDHCI_COMMON_PROPERTIES(_state) \
>> +    DEFINE_PROP_UINT8("sd-spec-version", _state, sd_spec_version, 2), \
>> +    DEFINE_PROP_UINT8("uhs", _state, uhs_mode, UHS_NOT_SUPPORTED), \
>> +    \
>> +    /* Capabilities registers provide information on supported
>> +     * features of this specific host controller implementation */ \
>> +    DEFINE_PROP_UINT64("capareg", _state, capareg, SDHC_CAPAB_REG_DEFAULT), \
>> +    DEFINE_PROP_UINT64("maxcurr", _state, maxcurr, 0)
>> +
>> +void sdhci_initfn(SDHCIState *s);
>> +void sdhci_uninitfn(SDHCIState *s);
>> +void sdhci_common_realize(SDHCIState *s, Error **errp);
>> +void sdhci_common_unrealize(SDHCIState *s, Error **errp);
>> +void sdhci_common_class_init(ObjectClass *klass, void *data);
>> +
>>  #endif
>> diff --git a/hw/sd/sdhci-pci.c b/hw/sd/sdhci-pci.c
>> new file mode 100644
>> index 0000000..f884661
>> --- /dev/null
>> +++ b/hw/sd/sdhci-pci.c
>> @@ -0,0 +1,87 @@
>> +/*
>> + * SDHCI device on PCI
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License as published by the
>> + * Free Software Foundation; either version 2 of the License, or (at your
>> + * option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>> + * See the GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License along
>> + * with this program; if not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "qapi/error.h"
>> +#include "hw/hw.h"
>> +#include "hw/sd/sdhci.h"
>> +#include "sdhci-internal.h"
>> +
>> +static Property sdhci_pci_properties[] = {
>> +    DEFINE_SDHCI_COMMON_PROPERTIES(SDHCIState),
>> +    DEFINE_PROP_END_OF_LIST(),
>> +};
>> +
>> +static void sdhci_pci_realize(PCIDevice *dev, Error **errp)
>> +{
>> +    SDHCIState *s = PCI_SDHCI(dev);
>> +    Error *local_err = NULL;
>> +
>> +    sdhci_initfn(s);
>> +    sdhci_common_realize(s, &local_err);
>> +    if (local_err) {
>> +        error_propagate(errp, local_err);
>> +        return;
>> +    }
>> +
>> +    dev->config[PCI_CLASS_PROG] = 0x01; /* Standard Host supported DMA */
>> +    dev->config[PCI_INTERRUPT_PIN] = 0x01; /* interrupt pin A */
>> +    s->irq = pci_allocate_irq(dev);
>> +    s->dma_as = pci_get_address_space(dev);
>> +    pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->iomem);
>> +}
>> +
>> +static void sdhci_pci_exit(PCIDevice *dev)
>> +{
>> +    SDHCIState *s = PCI_SDHCI(dev);
>> +
>> +    sdhci_common_unrealize(s, &error_abort);
>> +    sdhci_uninitfn(s);
>> +}
>> +
>> +static void sdhci_pci_class_init(ObjectClass *klass, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>> +
>> +    k->realize = sdhci_pci_realize;
>> +    k->exit = sdhci_pci_exit;
>> +    k->vendor_id = PCI_VENDOR_ID_REDHAT;
>> +    k->device_id = PCI_DEVICE_ID_REDHAT_SDHCI;
>> +    k->class_id = PCI_CLASS_SYSTEM_SDHCI;
>> +    dc->props = sdhci_pci_properties;
>> +
>> +    sdhci_common_class_init(klass, data);
>> +}
>> +
>> +static const TypeInfo sdhci_pci_info = {
>> +    .name = TYPE_PCI_SDHCI,
>> +    .parent = TYPE_PCI_DEVICE,
>> +    .instance_size = sizeof(SDHCIState),
>> +    .class_init = sdhci_pci_class_init,
>> +    .interfaces = (InterfaceInfo[]) {
>> +        { INTERFACE_CONVENTIONAL_PCI_DEVICE },
>> +        { },
>> +    },
>> +};
>> +
>> +static void sdhci_pci_register_type(void)
>> +{
>> +    type_register_static(&sdhci_pci_info);
>> +}
>> +
>> +type_init(sdhci_pci_register_type)
>> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
>> index 83f1574..17ad546 100644
>> --- a/hw/sd/sdhci.c
>> +++ b/hw/sd/sdhci.c
>> @@ -40,24 +40,6 @@
>>  
>>  #define MASKED_WRITE(reg, mask, val)  (reg = (reg & (mask)) | (val))
>>  
>> -/* Default SD/MMC host controller features information, which will be
>> - * presented in CAPABILITIES register of generic SD host controller at reset.
>> - *
>> - * support:
>> - * - 3.3v and 1.8v voltages
>> - * - SDMA/ADMA1/ADMA2
>> - * - high-speed
>> - * max host controller R/W buffers size: 512B
>> - * max clock frequency for SDclock: 52 MHz
>> - * timeout clock frequency: 52 MHz
>> - *
>> - * does not support:
>> - * - 3.0v voltage
>> - * - 64-bit system bus
>> - * - suspend/resume
>> - */
>> -#define SDHC_CAPAB_REG_DEFAULT 0x057834b4
>> -
>>  static inline unsigned int sdhci_get_fifolen(SDHCIState *s)
>>  {
>>      return 1 << (9 + FIELD_EX32(s->capareg, SDHC_CAPAB, MAXBLOCKLENGTH));
>> @@ -1328,16 +1310,7 @@ static void sdhci_init_readonly_registers(SDHCIState *s, Error **errp)
>>  
>>  /* --- qdev common --- */
>>  
>> -#define DEFINE_SDHCI_COMMON_PROPERTIES(_state) \
>> -    DEFINE_PROP_UINT8("sd-spec-version", _state, sd_spec_version, 2), \
>> -    DEFINE_PROP_UINT8("uhs", _state, uhs_mode, UHS_NOT_SUPPORTED), \
>> -    \
>> -    /* Capabilities registers provide information on supported
>> -     * features of this specific host controller implementation */ \
>> -    DEFINE_PROP_UINT64("capareg", _state, capareg, SDHC_CAPAB_REG_DEFAULT), \
>> -    DEFINE_PROP_UINT64("maxcurr", _state, maxcurr, 0)
>> -
>> -static void sdhci_initfn(SDHCIState *s)
>> +void sdhci_initfn(SDHCIState *s)
>>  {
>>      qbus_create_inplace(&s->sdbus, sizeof(s->sdbus),
>>                          TYPE_SDHCI_BUS, DEVICE(s), "sd-bus");
>> @@ -1348,7 +1321,7 @@ static void sdhci_initfn(SDHCIState *s)
>>      s->io_ops = &sdhci_mmio_ops;
>>  }
>>  
>> -static void sdhci_uninitfn(SDHCIState *s)
>> +void sdhci_uninitfn(SDHCIState *s)
>>  {
>>      timer_del(s->insert_timer);
>>      timer_free(s->insert_timer);
>> @@ -1359,7 +1332,7 @@ static void sdhci_uninitfn(SDHCIState *s)
>>      s->fifo_buffer = NULL;
>>  }
>>  
>> -static void sdhci_common_realize(SDHCIState *s, Error **errp)
>> +void sdhci_common_realize(SDHCIState *s, Error **errp)
>>  {
>>      Error *local_err = NULL;
>>  
>> @@ -1375,7 +1348,7 @@ static void sdhci_common_realize(SDHCIState *s, Error **errp)
>>                            SDHC_REGISTERS_MAP_SIZE);
>>  }
>>  
>> -static void sdhci_common_unrealize(SDHCIState *s, Error **errp)
>> +void sdhci_common_unrealize(SDHCIState *s, Error **errp)
>>  {
>>      /* This function is expected to be called only once for each class:
>>       * - SysBus:    via DeviceClass->unrealize(),
>> @@ -1445,7 +1418,7 @@ const VMStateDescription sdhci_vmstate = {
>>      },
>>  };
>>  
>> -static void sdhci_common_class_init(ObjectClass *klass, void *data)
>> +void sdhci_common_class_init(ObjectClass *klass, void *data)
>>  {
>>      DeviceClass *dc = DEVICE_CLASS(klass);
>>  
>> @@ -1454,66 +1427,6 @@ static void sdhci_common_class_init(ObjectClass *klass, void *data)
>>      dc->reset = sdhci_poweron_reset;
>>  }
>>  
>> -/* --- qdev PCI --- */
>> -
>> -static Property sdhci_pci_properties[] = {
>> -    DEFINE_SDHCI_COMMON_PROPERTIES(SDHCIState),
>> -    DEFINE_PROP_END_OF_LIST(),
>> -};
>> -
>> -static void sdhci_pci_realize(PCIDevice *dev, Error **errp)
>> -{
>> -    SDHCIState *s = PCI_SDHCI(dev);
>> -    Error *local_err = NULL;
>> -
>> -    sdhci_initfn(s);
>> -    sdhci_common_realize(s, &local_err);
>> -    if (local_err) {
>> -        error_propagate(errp, local_err);
>> -        return;
>> -    }
>> -
>> -    dev->config[PCI_CLASS_PROG] = 0x01; /* Standard Host supported DMA */
>> -    dev->config[PCI_INTERRUPT_PIN] = 0x01; /* interrupt pin A */
>> -    s->irq = pci_allocate_irq(dev);
>> -    s->dma_as = pci_get_address_space(dev);
>> -    pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->iomem);
>> -}
>> -
>> -static void sdhci_pci_exit(PCIDevice *dev)
>> -{
>> -    SDHCIState *s = PCI_SDHCI(dev);
>> -
>> -    sdhci_common_unrealize(s, &error_abort);
>> -    sdhci_uninitfn(s);
>> -}
>> -
>> -static void sdhci_pci_class_init(ObjectClass *klass, void *data)
>> -{
>> -    DeviceClass *dc = DEVICE_CLASS(klass);
>> -    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>> -
>> -    k->realize = sdhci_pci_realize;
>> -    k->exit = sdhci_pci_exit;
>> -    k->vendor_id = PCI_VENDOR_ID_REDHAT;
>> -    k->device_id = PCI_DEVICE_ID_REDHAT_SDHCI;
>> -    k->class_id = PCI_CLASS_SYSTEM_SDHCI;
>> -    dc->props = sdhci_pci_properties;
>> -
>> -    sdhci_common_class_init(klass, data);
>> -}
>> -
>> -static const TypeInfo sdhci_pci_info = {
>> -    .name = TYPE_PCI_SDHCI,
>> -    .parent = TYPE_PCI_DEVICE,
>> -    .instance_size = sizeof(SDHCIState),
>> -    .class_init = sdhci_pci_class_init,
>> -    .interfaces = (InterfaceInfo[]) {
>> -        { INTERFACE_CONVENTIONAL_PCI_DEVICE },
>> -        { },
>> -    },
>> -};
>> -
>>  /* --- qdev SysBus --- */
>>  
>>  static Property sdhci_sysbus_properties[] = {
>> @@ -1846,7 +1759,6 @@ static const TypeInfo imx_usdhc_info = {
>>  
>>  static void sdhci_register_types(void)
>>  {
>> -    type_register_static(&sdhci_pci_info);
>>      type_register_static(&sdhci_sysbus_info);
>>      type_register_static(&sdhci_bus_info);
>>      type_register_static(&imx_usdhc_info);
>>

On 2/20/19 5:31 PM, Paolo Bonzini wrote:
> On 20/02/19 13:26, Philippe Mathieu-Daudé wrote:
>> On 2/20/19 1:06 PM, Thomas Huth wrote:
>>> Some machines have an SDHCI device, but no PCI. To be able to
>>> compile hw/sd/sdhci.c without CONFIG_PCI, we must not call functions
>>> like pci_get_address_space() and pci_allocate_irq() there. Thus
>>> move the PCI-related code into a separate file.
>>>
>>> This is required for the upcoming Kconfig-like build system, e.g. it
>>> is needed if a user wants to compile a QEMU binary with just one machine
>>> that has SDHCI, but no PCI, like the ARM "raspi" machines for example.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>
>> I wonder if we should add a F: entry into the "PC Chipset" section of
>> MAINTAINERS.
> 
> No, I don't think so.  It's not part of PC.

You are correct, it might be used by any board provinding a PCI bus.

What I'm looking for is a list of potential users or maintainers as in
"is this device supported/maintainted?".

Currently the only explicit user of the codebase qtest.

> Paolo
> 
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>
>>> ---
>>>  Note: Once we've got Kconfig in place, the "land" in the Makefile
>>>  should be replaced with a proper new CONFIG_SDHCI_PCI switch instead
>>>
>>>  hw/sd/Makefile.objs    |  1 +
>>>  hw/sd/sdhci-internal.h | 34 ++++++++++++++++++
>>>  hw/sd/sdhci-pci.c      | 87 ++++++++++++++++++++++++++++++++++++++++++++
>>>  hw/sd/sdhci.c          | 98 +++-----------------------------------------------
>>>  4 files changed, 127 insertions(+), 93 deletions(-)
>>>  create mode 100644 hw/sd/sdhci-pci.c
>>>
>>> diff --git a/hw/sd/Makefile.objs b/hw/sd/Makefile.objs
>>> index a99d9fb..58be7b3 100644
>>> --- a/hw/sd/Makefile.objs
>>> +++ b/hw/sd/Makefile.objs
>>> @@ -2,6 +2,7 @@ common-obj-$(CONFIG_PL181) += pl181.o
>>>  common-obj-$(CONFIG_SSI_SD) += ssi-sd.o
>>>  common-obj-$(CONFIG_SD) += sd.o core.o sdmmc-internal.o
>>>  common-obj-$(CONFIG_SDHCI) += sdhci.o
>>> +common-obj-$(call land,$(CONFIG_SDHCI),$(CONFIG_PCI)) += sdhci-pci.o
>>>  
>>>  obj-$(CONFIG_MILKYMIST) += milkymist-memcard.o
>>>  obj-$(CONFIG_OMAP) += omap_mmc.o
>>> diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h
>>> index 19665fd..3414140 100644
>>> --- a/hw/sd/sdhci-internal.h
>>> +++ b/hw/sd/sdhci-internal.h
>>> @@ -304,4 +304,38 @@ extern const VMStateDescription sdhci_vmstate;
>>>  
>>>  #define ESDHC_PRNSTS_SDSTB              (1 << 3)
>>>  
>>> +/*
>>> + * Default SD/MMC host controller features information, which will be
>>> + * presented in CAPABILITIES register of generic SD host controller at reset.
>>> + *
>>> + * support:
>>> + * - 3.3v and 1.8v voltages
>>> + * - SDMA/ADMA1/ADMA2
>>> + * - high-speed
>>> + * max host controller R/W buffers size: 512B
>>> + * max clock frequency for SDclock: 52 MHz
>>> + * timeout clock frequency: 52 MHz
>>> + *
>>> + * does not support:
>>> + * - 3.0v voltage
>>> + * - 64-bit system bus
>>> + * - suspend/resume
>>> + */
>>> +#define SDHC_CAPAB_REG_DEFAULT 0x057834b4
>>> +
>>> +#define DEFINE_SDHCI_COMMON_PROPERTIES(_state) \
>>> +    DEFINE_PROP_UINT8("sd-spec-version", _state, sd_spec_version, 2), \
>>> +    DEFINE_PROP_UINT8("uhs", _state, uhs_mode, UHS_NOT_SUPPORTED), \
>>> +    \
>>> +    /* Capabilities registers provide information on supported
>>> +     * features of this specific host controller implementation */ \
>>> +    DEFINE_PROP_UINT64("capareg", _state, capareg, SDHC_CAPAB_REG_DEFAULT), \
>>> +    DEFINE_PROP_UINT64("maxcurr", _state, maxcurr, 0)
>>> +
>>> +void sdhci_initfn(SDHCIState *s);
>>> +void sdhci_uninitfn(SDHCIState *s);
>>> +void sdhci_common_realize(SDHCIState *s, Error **errp);
>>> +void sdhci_common_unrealize(SDHCIState *s, Error **errp);
>>> +void sdhci_common_class_init(ObjectClass *klass, void *data);
>>> +
>>>  #endif
>>> diff --git a/hw/sd/sdhci-pci.c b/hw/sd/sdhci-pci.c
>>> new file mode 100644
>>> index 0000000..f884661
>>> --- /dev/null
>>> +++ b/hw/sd/sdhci-pci.c
>>> @@ -0,0 +1,87 @@
>>> +/*
>>> + * SDHCI device on PCI
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify it
>>> + * under the terms of the GNU General Public License as published by the
>>> + * Free Software Foundation; either version 2 of the License, or (at your
>>> + * option) any later version.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>>> + * See the GNU General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU General Public License along
>>> + * with this program; if not, see <http://www.gnu.org/licenses/>.
>>> + */
>>> +
>>> +#include "qemu/osdep.h"
>>> +#include "qapi/error.h"
>>> +#include "hw/hw.h"
>>> +#include "hw/sd/sdhci.h"
>>> +#include "sdhci-internal.h"
>>> +
>>> +static Property sdhci_pci_properties[] = {
>>> +    DEFINE_SDHCI_COMMON_PROPERTIES(SDHCIState),
>>> +    DEFINE_PROP_END_OF_LIST(),
>>> +};
>>> +
>>> +static void sdhci_pci_realize(PCIDevice *dev, Error **errp)
>>> +{
>>> +    SDHCIState *s = PCI_SDHCI(dev);
>>> +    Error *local_err = NULL;
>>> +
>>> +    sdhci_initfn(s);
>>> +    sdhci_common_realize(s, &local_err);
>>> +    if (local_err) {
>>> +        error_propagate(errp, local_err);
>>> +        return;
>>> +    }
>>> +
>>> +    dev->config[PCI_CLASS_PROG] = 0x01; /* Standard Host supported DMA */
>>> +    dev->config[PCI_INTERRUPT_PIN] = 0x01; /* interrupt pin A */
>>> +    s->irq = pci_allocate_irq(dev);
>>> +    s->dma_as = pci_get_address_space(dev);
>>> +    pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->iomem);
>>> +}
>>> +
>>> +static void sdhci_pci_exit(PCIDevice *dev)
>>> +{
>>> +    SDHCIState *s = PCI_SDHCI(dev);
>>> +
>>> +    sdhci_common_unrealize(s, &error_abort);
>>> +    sdhci_uninitfn(s);
>>> +}
>>> +
>>> +static void sdhci_pci_class_init(ObjectClass *klass, void *data)
>>> +{
>>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>>> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>>> +
>>> +    k->realize = sdhci_pci_realize;
>>> +    k->exit = sdhci_pci_exit;
>>> +    k->vendor_id = PCI_VENDOR_ID_REDHAT;
>>> +    k->device_id = PCI_DEVICE_ID_REDHAT_SDHCI;
>>> +    k->class_id = PCI_CLASS_SYSTEM_SDHCI;
>>> +    dc->props = sdhci_pci_properties;
>>> +
>>> +    sdhci_common_class_init(klass, data);
>>> +}
>>> +
>>> +static const TypeInfo sdhci_pci_info = {
>>> +    .name = TYPE_PCI_SDHCI,
>>> +    .parent = TYPE_PCI_DEVICE,
>>> +    .instance_size = sizeof(SDHCIState),
>>> +    .class_init = sdhci_pci_class_init,
>>> +    .interfaces = (InterfaceInfo[]) {
>>> +        { INTERFACE_CONVENTIONAL_PCI_DEVICE },
>>> +        { },
>>> +    },
>>> +};
>>> +
>>> +static void sdhci_pci_register_type(void)
>>> +{
>>> +    type_register_static(&sdhci_pci_info);
>>> +}
>>> +
>>> +type_init(sdhci_pci_register_type)
>>> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
>>> index 83f1574..17ad546 100644
>>> --- a/hw/sd/sdhci.c
>>> +++ b/hw/sd/sdhci.c
>>> @@ -40,24 +40,6 @@
>>>  
>>>  #define MASKED_WRITE(reg, mask, val)  (reg = (reg & (mask)) | (val))
>>>  
>>> -/* Default SD/MMC host controller features information, which will be
>>> - * presented in CAPABILITIES register of generic SD host controller at reset.
>>> - *
>>> - * support:
>>> - * - 3.3v and 1.8v voltages
>>> - * - SDMA/ADMA1/ADMA2
>>> - * - high-speed
>>> - * max host controller R/W buffers size: 512B
>>> - * max clock frequency for SDclock: 52 MHz
>>> - * timeout clock frequency: 52 MHz
>>> - *
>>> - * does not support:
>>> - * - 3.0v voltage
>>> - * - 64-bit system bus
>>> - * - suspend/resume
>>> - */
>>> -#define SDHC_CAPAB_REG_DEFAULT 0x057834b4
>>> -
>>>  static inline unsigned int sdhci_get_fifolen(SDHCIState *s)
>>>  {
>>>      return 1 << (9 + FIELD_EX32(s->capareg, SDHC_CAPAB, MAXBLOCKLENGTH));
>>> @@ -1328,16 +1310,7 @@ static void sdhci_init_readonly_registers(SDHCIState *s, Error **errp)
>>>  
>>>  /* --- qdev common --- */
>>>  
>>> -#define DEFINE_SDHCI_COMMON_PROPERTIES(_state) \
>>> -    DEFINE_PROP_UINT8("sd-spec-version", _state, sd_spec_version, 2), \
>>> -    DEFINE_PROP_UINT8("uhs", _state, uhs_mode, UHS_NOT_SUPPORTED), \
>>> -    \
>>> -    /* Capabilities registers provide information on supported
>>> -     * features of this specific host controller implementation */ \
>>> -    DEFINE_PROP_UINT64("capareg", _state, capareg, SDHC_CAPAB_REG_DEFAULT), \
>>> -    DEFINE_PROP_UINT64("maxcurr", _state, maxcurr, 0)
>>> -
>>> -static void sdhci_initfn(SDHCIState *s)
>>> +void sdhci_initfn(SDHCIState *s)
>>>  {
>>>      qbus_create_inplace(&s->sdbus, sizeof(s->sdbus),
>>>                          TYPE_SDHCI_BUS, DEVICE(s), "sd-bus");
>>> @@ -1348,7 +1321,7 @@ static void sdhci_initfn(SDHCIState *s)
>>>      s->io_ops = &sdhci_mmio_ops;
>>>  }
>>>  
>>> -static void sdhci_uninitfn(SDHCIState *s)
>>> +void sdhci_uninitfn(SDHCIState *s)
>>>  {
>>>      timer_del(s->insert_timer);
>>>      timer_free(s->insert_timer);
>>> @@ -1359,7 +1332,7 @@ static void sdhci_uninitfn(SDHCIState *s)
>>>      s->fifo_buffer = NULL;
>>>  }
>>>  
>>> -static void sdhci_common_realize(SDHCIState *s, Error **errp)
>>> +void sdhci_common_realize(SDHCIState *s, Error **errp)
>>>  {
>>>      Error *local_err = NULL;
>>>  
>>> @@ -1375,7 +1348,7 @@ static void sdhci_common_realize(SDHCIState *s, Error **errp)
>>>                            SDHC_REGISTERS_MAP_SIZE);
>>>  }
>>>  
>>> -static void sdhci_common_unrealize(SDHCIState *s, Error **errp)
>>> +void sdhci_common_unrealize(SDHCIState *s, Error **errp)
>>>  {
>>>      /* This function is expected to be called only once for each class:
>>>       * - SysBus:    via DeviceClass->unrealize(),
>>> @@ -1445,7 +1418,7 @@ const VMStateDescription sdhci_vmstate = {
>>>      },
>>>  };
>>>  
>>> -static void sdhci_common_class_init(ObjectClass *klass, void *data)
>>> +void sdhci_common_class_init(ObjectClass *klass, void *data)
>>>  {
>>>      DeviceClass *dc = DEVICE_CLASS(klass);
>>>  
>>> @@ -1454,66 +1427,6 @@ static void sdhci_common_class_init(ObjectClass *klass, void *data)
>>>      dc->reset = sdhci_poweron_reset;
>>>  }
>>>  
>>> -/* --- qdev PCI --- */
>>> -
>>> -static Property sdhci_pci_properties[] = {
>>> -    DEFINE_SDHCI_COMMON_PROPERTIES(SDHCIState),
>>> -    DEFINE_PROP_END_OF_LIST(),
>>> -};
>>> -
>>> -static void sdhci_pci_realize(PCIDevice *dev, Error **errp)
>>> -{
>>> -    SDHCIState *s = PCI_SDHCI(dev);
>>> -    Error *local_err = NULL;
>>> -
>>> -    sdhci_initfn(s);
>>> -    sdhci_common_realize(s, &local_err);
>>> -    if (local_err) {
>>> -        error_propagate(errp, local_err);
>>> -        return;
>>> -    }
>>> -
>>> -    dev->config[PCI_CLASS_PROG] = 0x01; /* Standard Host supported DMA */
>>> -    dev->config[PCI_INTERRUPT_PIN] = 0x01; /* interrupt pin A */
>>> -    s->irq = pci_allocate_irq(dev);
>>> -    s->dma_as = pci_get_address_space(dev);
>>> -    pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->iomem);
>>> -}
>>> -
>>> -static void sdhci_pci_exit(PCIDevice *dev)
>>> -{
>>> -    SDHCIState *s = PCI_SDHCI(dev);
>>> -
>>> -    sdhci_common_unrealize(s, &error_abort);
>>> -    sdhci_uninitfn(s);
>>> -}
>>> -
>>> -static void sdhci_pci_class_init(ObjectClass *klass, void *data)
>>> -{
>>> -    DeviceClass *dc = DEVICE_CLASS(klass);
>>> -    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>>> -
>>> -    k->realize = sdhci_pci_realize;
>>> -    k->exit = sdhci_pci_exit;
>>> -    k->vendor_id = PCI_VENDOR_ID_REDHAT;
>>> -    k->device_id = PCI_DEVICE_ID_REDHAT_SDHCI;
>>> -    k->class_id = PCI_CLASS_SYSTEM_SDHCI;
>>> -    dc->props = sdhci_pci_properties;
>>> -
>>> -    sdhci_common_class_init(klass, data);
>>> -}
>>> -
>>> -static const TypeInfo sdhci_pci_info = {
>>> -    .name = TYPE_PCI_SDHCI,
>>> -    .parent = TYPE_PCI_DEVICE,
>>> -    .instance_size = sizeof(SDHCIState),
>>> -    .class_init = sdhci_pci_class_init,
>>> -    .interfaces = (InterfaceInfo[]) {
>>> -        { INTERFACE_CONVENTIONAL_PCI_DEVICE },
>>> -        { },
>>> -    },
>>> -};
>>> -
>>>  /* --- qdev SysBus --- */
>>>  
>>>  static Property sdhci_sysbus_properties[] = {
>>> @@ -1846,7 +1759,6 @@ static const TypeInfo imx_usdhc_info = {
>>>  
>>>  static void sdhci_register_types(void)
>>>  {
>>> -    type_register_static(&sdhci_pci_info);
>>>      type_register_static(&sdhci_sysbus_info);
>>>      type_register_static(&sdhci_bus_info);
>>>      type_register_static(&imx_usdhc_info);
>>>

Patchew URL: https://patchew.org/QEMU/1550664389-2865-1-git-send-email-thuth@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 1550664389-2865-1-git-send-email-thuth@redhat.com
Subject: [Qemu-devel] [RFC PATCH] hw/sd/sdhci: Move PCI-related code into a separate file
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]               patchew/1550664389-2865-1-git-send-email-thuth@redhat.com -> patchew/1550664389-2865-1-git-send-email-thuth@redhat.com
 * [new tag]               patchew/20190220145819.30969-1-berrange@redhat.com -> patchew/20190220145819.30969-1-berrange@redhat.com
 * [new tag]               patchew/20190221183829.31019-1-philmd@redhat.com -> patchew/20190221183829.31019-1-philmd@redhat.com
 * [new tag]               patchew/20190223105421.1290-1-davidkiarie4@gmail.com -> patchew/20190223105421.1290-1-davidkiarie4@gmail.com
 * [new tag]               patchew/20190227012132.16271-1-david@gibson.dropbear.id.au -> patchew/20190227012132.16271-1-david@gibson.dropbear.id.au
 * [new tag]               patchew/cover.1550836631.git.berto@igalia.com -> patchew/cover.1550836631.git.berto@igalia.com
Switched to a new branch 'test'
fd512d4ee7 hw/sd/sdhci: Move PCI-related code into a separate file

=== OUTPUT BEGIN ===
ERROR: Macros with complex values should be enclosed in parenthesis
#60: FILE: hw/sd/sdhci-internal.h:326:
+#define DEFINE_SDHCI_COMMON_PROPERTIES(_state) \
+    DEFINE_PROP_UINT8("sd-spec-version", _state, sd_spec_version, 2), \
+    DEFINE_PROP_UINT8("uhs", _state, uhs_mode, UHS_NOT_SUPPORTED), \
+    \
+    /* Capabilities registers provide information on supported

WARNING: Block comments use a leading /* on a separate line
#64: FILE: hw/sd/sdhci-internal.h:330:
+    /* Capabilities registers provide information on supported

WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#77: 
new file mode 100644

total: 1 errors, 2 warnings, 278 lines checked

Commit fd512d4ee7ec (hw/sd/sdhci: Move PCI-related code into a separate file) has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/1550664389-2865-1-git-send-email-thuth@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com