* [Qemu-devel] [RFC PATCH] hw/sd/sdhci: Move PCI-related code into a separate file
@ 2019-02-20 12:06 Thomas Huth
2019-02-20 12:26 ` Philippe Mathieu-Daudé
2019-02-27 15:47 ` no-reply
0 siblings, 2 replies; 5+ messages in thread
From: Thomas Huth @ 2019-02-20 12:06 UTC (permalink / raw)
To: qemu-devel, Philippe Mathieu-Daudé; +Cc: yang.zhong, pbonzini
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
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);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [RFC PATCH] hw/sd/sdhci: Move PCI-related code into a separate file
2019-02-20 12:06 [Qemu-devel] [RFC PATCH] hw/sd/sdhci: Move PCI-related code into a separate file Thomas Huth
@ 2019-02-20 12:26 ` Philippe Mathieu-Daudé
2019-02-20 16:31 ` Paolo Bonzini
2019-02-27 15:47 ` no-reply
1 sibling, 1 reply; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-02-20 12:26 UTC (permalink / raw)
To: Thomas Huth, qemu-devel, Philippe Mathieu-Daudé; +Cc: yang.zhong, pbonzini
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);
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [RFC PATCH] hw/sd/sdhci: Move PCI-related code into a separate file
2019-02-20 12:26 ` Philippe Mathieu-Daudé
@ 2019-02-20 16:31 ` Paolo Bonzini
2019-02-20 17:34 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 5+ messages in thread
From: Paolo Bonzini @ 2019-02-20 16:31 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Thomas Huth, qemu-devel,
Philippe Mathieu-Daudé
Cc: yang.zhong
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);
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [RFC PATCH] hw/sd/sdhci: Move PCI-related code into a separate file
2019-02-20 16:31 ` Paolo Bonzini
@ 2019-02-20 17:34 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-02-20 17:34 UTC (permalink / raw)
To: Paolo Bonzini, Thomas Huth, qemu-devel,
Philippe Mathieu-Daudé
Cc: yang.zhong
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);
>>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [RFC PATCH] hw/sd/sdhci: Move PCI-related code into a separate file
2019-02-20 12:06 [Qemu-devel] [RFC PATCH] hw/sd/sdhci: Move PCI-related code into a separate file Thomas Huth
2019-02-20 12:26 ` Philippe Mathieu-Daudé
@ 2019-02-27 15:47 ` no-reply
1 sibling, 0 replies; 5+ messages in thread
From: no-reply @ 2019-02-27 15:47 UTC (permalink / raw)
To: thuth; +Cc: fam, qemu-devel, f4bug, yang.zhong, pbonzini
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-02-27 15:48 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-02-20 12:06 [Qemu-devel] [RFC PATCH] hw/sd/sdhci: Move PCI-related code into a separate file Thomas Huth
2019-02-20 12:26 ` Philippe Mathieu-Daudé
2019-02-20 16:31 ` Paolo Bonzini
2019-02-20 17:34 ` Philippe Mathieu-Daudé
2019-02-27 15:47 ` no-reply
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).