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 <[email protected]> >> >> 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é <[email protected]> >> Tested-by: Philippe Mathieu-Daudé <[email protected]> >> >>> --- >>> 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); >>>
