Re: [Qemu-devel] [PATCH v4 03/29] hw/sd/sdhci: Move PCI-related code into a separate file

2019-03-10 Thread Philippe Mathieu-Daudé 
On 3/7/19 3:18 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 new 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 
> ---
>  hw/sd/Kconfig  |  6 +++-
>  hw/sd/Makefile.objs|  1 +
>  hw/sd/sdhci-internal.h | 34 ++
>  hw/sd/sdhci-pci.c  | 87 
>  hw/sd/sdhci.c  | 98 
> +++---
>  5 files changed, 132 insertions(+), 94 deletions(-)
>  create mode 100644 hw/sd/sdhci-pci.c
> 
> diff --git a/hw/sd/Kconfig b/hw/sd/Kconfig
> index 864f535..c5e1e55 100644
> --- a/hw/sd/Kconfig
> +++ b/hw/sd/Kconfig
> @@ -12,6 +12,10 @@ config SD
>  
>  config SDHCI
>  bool
> +select SD

Hmm this should be SDBUS. I'll clean that up later.

Reviewed-by: Philippe Mathieu-Daudé 

> +
> +config SDHCI_PCI
> +bool
>  default y if PCI_DEVICES
>  depends on PCI
> -select SD
> +select SDHCI
> diff --git a/hw/sd/Makefile.objs b/hw/sd/Makefile.objs
> index a99d9fb..0665727 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-$(CONFIG_SDHCI_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 000..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 .
> + */
> +
> +#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

[Qemu-devel] [PATCH v4 03/29] hw/sd/sdhci: Move PCI-related code into a separate file

2019-03-07 Thread Thomas Huth 
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 new 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 
---
 hw/sd/Kconfig  |  6 +++-
 hw/sd/Makefile.objs|  1 +
 hw/sd/sdhci-internal.h | 34 ++
 hw/sd/sdhci-pci.c  | 87 
 hw/sd/sdhci.c  | 98 +++---
 5 files changed, 132 insertions(+), 94 deletions(-)
 create mode 100644 hw/sd/sdhci-pci.c

diff --git a/hw/sd/Kconfig b/hw/sd/Kconfig
index 864f535..c5e1e55 100644
--- a/hw/sd/Kconfig
+++ b/hw/sd/Kconfig
@@ -12,6 +12,10 @@ config SD
 
 config SDHCI
 bool
+select SD
+
+config SDHCI_PCI
+bool
 default y if PCI_DEVICES
 depends on PCI
-select SD
+select SDHCI
diff --git a/hw/sd/Makefile.objs b/hw/sd/Makefile.objs
index a99d9fb..0665727 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-$(CONFIG_SDHCI_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 000..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 .
+ */
+
+#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(Obje