diff mbox series

[v3,2/4] arm: x86: qemu: add UCLASS_QFW, split driver into _pio and _mmio

Message ID 20210223114329.16729-3-ashe@kivikakk.ee
State Superseded
Delegated to: Tom Rini
Headers show
Series Move qfw to DM, add Arm support | expand

Commit Message

Asherah Connor Feb. 23, 2021, 11:43 a.m. UTC
Split the qfw driver into qfw_pio and qfw_mmio, under their own uclass.
Each driver does arch/platform-specific I/O.

Signed-off-by: Asherah Connor <ashe@kivikakk.ee>
---

Changes in v3:
- Add new UCLASS_QFW, split qfw driver into PIO and MMIO variants.
- QFW no longer depends on or selects MISC.

 arch/x86/cpu/qemu/cpu.c  |   2 +-
 arch/x86/cpu/qemu/qemu.c |  18 ++-
 arch/x86/cpu/qfw_cpu.c   |   2 +-
 cmd/qfw.c                |  26 ++--
 common/Makefile          |   2 +
 common/qfw.c             | 111 ++++++++++++++++
 drivers/misc/Kconfig     |   1 -
 drivers/misc/Makefile    |   6 +
 drivers/misc/qfw.c       | 270 ++++++---------------------------------
 drivers/misc/qfw_mmio.c  | 101 +++++++++++++++
 drivers/misc/qfw_pio.c   |  66 ++++++++++
 include/dm/uclass-id.h   |   1 +
 include/qfw.h            |  45 +++++--
 13 files changed, 382 insertions(+), 269 deletions(-)
 create mode 100644 common/qfw.c
 create mode 100644 drivers/misc/qfw_mmio.c
 create mode 100644 drivers/misc/qfw_pio.c

Comments

Simon Glass Feb. 23, 2021, 3:58 p.m. UTC | #1
Hi Asherah,

On Tue, 23 Feb 2021 at 06:44, Asherah Connor <ashe@kivikakk.ee> wrote:
>
> Split the qfw driver into qfw_pio and qfw_mmio, under their own uclass.
> Each driver does arch/platform-specific I/O.
>
> Signed-off-by: Asherah Connor <ashe@kivikakk.ee>
> ---
>
> Changes in v3:
> - Add new UCLASS_QFW, split qfw driver into PIO and MMIO variants.
> - QFW no longer depends on or selects MISC.
>
>  arch/x86/cpu/qemu/cpu.c  |   2 +-
>  arch/x86/cpu/qemu/qemu.c |  18 ++-
>  arch/x86/cpu/qfw_cpu.c   |   2 +-
>  cmd/qfw.c                |  26 ++--
>  common/Makefile          |   2 +
>  common/qfw.c             | 111 ++++++++++++++++
>  drivers/misc/Kconfig     |   1 -
>  drivers/misc/Makefile    |   6 +
>  drivers/misc/qfw.c       | 270 ++++++---------------------------------
>  drivers/misc/qfw_mmio.c  | 101 +++++++++++++++
>  drivers/misc/qfw_pio.c   |  66 ++++++++++
>  include/dm/uclass-id.h   |   1 +
>  include/qfw.h            |  45 +++++--
>  13 files changed, 382 insertions(+), 269 deletions(-)
>  create mode 100644 common/qfw.c
>  create mode 100644 drivers/misc/qfw_mmio.c
>  create mode 100644 drivers/misc/qfw_pio.c
>


[..]

> diff --git a/include/qfw.h b/include/qfw.h
> index f9c6828841..d7e16651a2 100644
> --- a/include/qfw.h
> +++ b/include/qfw.h
> @@ -149,28 +149,49 @@ struct qfw_mmio {
>         u64 dma;
>  };
>
> -struct qfw_plat {
> -       /* MMIO used on Arm. */
> +struct qfw_pio_plat {
> +       u16 control_port;
> +       u16 data_port;
> +       u16 dma_port_low;
> +       u16 dma_port_high;
> +};
> +
> +struct qfw_mmio_plat {
>         volatile struct qfw_mmio *mmio;
> -       /* IO used on x86. */
> -       struct {
> -               u16 control_port;
> -               u16 data_port;
> -               u16 dma_port_low;
> -               u16 dma_port_high;
> -       } io;
>  };
>
> +struct qfw_dma {
> +       __be32 control;
> +       __be32 length;
> +       __be64 address;
> +};
> +
> +struct qfw_dev {
> +       struct udevice *dev;
> +       bool dma_present;
> +       struct list_head fw_list;
> +};
> +
> +struct dm_qfw_ops {

comment struct and methods. See other uclasses for how this is done.

> +       void (*read_entry_io)(struct udevice *dev, u16 entry, u32 size,
> +                             void *address);
> +       void (*read_entry_dma)(struct udevice *dev, struct qfw_dma *dma);
> +};
> +
> +#define dm_qfw_get_ops(dev) \
> +               ((struct dm_qfw_ops *)(dev)->driver->ops)
> +
> +int qfw_register(struct udevice *dev);
> +
>  struct udevice;
>  /**
>   * Get QEMU firmware config device
>   *
>   * @return struct udevice * if present, NULL otherwise
>   */
> -struct udevice *qemu_fwcfg_dev(void);
> +struct udevice *qfw_get_dev(void);

The problem with this function is that you drop the error number. Can
you instead do something like:

int qfw_get_dev(struct udevice **devp)

>
> -void qemu_fwcfg_read_entry(struct udevice *dev, u16 entry, u32 length,
> -                          void *address);
> +void qfw_read_entry(struct udevice *dev, u16 entry, u32 length, void *address);

Could you please add full function/struct comments to the things you
modify in the header file? That way people can read the API in one
place.

I'm not sure if it is easy to split up your patches a bit more. It is
often tricky with a DM conversion.

Regards,
Simon
diff mbox series

Patch

diff --git a/arch/x86/cpu/qemu/cpu.c b/arch/x86/cpu/qemu/cpu.c
index ab1b797f9a..09499aad78 100644
--- a/arch/x86/cpu/qemu/cpu.c
+++ b/arch/x86/cpu/qemu/cpu.c
@@ -22,7 +22,7 @@  int cpu_qemu_get_desc(const struct udevice *dev, char *buf, int size)
 
 static int cpu_qemu_get_count(const struct udevice *dev)
 {
-	struct udevice *qfw_dev = qemu_fwcfg_dev();
+	struct udevice *qfw_dev = qfw_get_dev();
 
 	if (!qfw_dev)
 		return -ENODEV;
diff --git a/arch/x86/cpu/qemu/qemu.c b/arch/x86/cpu/qemu/qemu.c
index e255af9a4a..5a61cc3bb4 100644
--- a/arch/x86/cpu/qemu/qemu.c
+++ b/arch/x86/cpu/qemu/qemu.c
@@ -20,18 +20,16 @@  static bool i440fx;
 #ifdef CONFIG_QFW
 
 /* on x86, the qfw registers are all IO ports */
-static const struct qfw_plat x86_qfw_plat = {
-	.io = {
-		.control_port	= 0x510,
-		.data_port	= 0x511,
-		.dma_port_low	= 0x514,
-		.dma_port_high	= 0x518,
-	},
+static const struct qfw_pio_plat x86_qfw_pio_plat = {
+	.control_port	= 0x510,
+	.data_port	= 0x511,
+	.dma_port_low	= 0x514,
+	.dma_port_high	= 0x518,
 };
 
-U_BOOT_DRVINFO(x86_qfw) = {
-	.name = "qfw",
-	.plat = &x86_qfw_plat,
+U_BOOT_DRVINFO(x86_qfw_pio) = {
+	.name = "qfw_pio",
+	.plat = &x86_qfw_pio_plat,
 };
 
 #endif
diff --git a/arch/x86/cpu/qfw_cpu.c b/arch/x86/cpu/qfw_cpu.c
index c8fb918494..e80cec0ca4 100644
--- a/arch/x86/cpu/qfw_cpu.c
+++ b/arch/x86/cpu/qfw_cpu.c
@@ -40,7 +40,7 @@  int qemu_cpu_fixup(void)
 	}
 
 	/* get qfw dev */
-	qfwdev = qemu_fwcfg_dev();
+	qfwdev = qfw_get_dev();
 	if (!qfwdev) {
 		printf("unable to find qfw device\n");
 		return -ENODEV;
diff --git a/cmd/qfw.c b/cmd/qfw.c
index ec80a9a3b5..a983a45380 100644
--- a/cmd/qfw.c
+++ b/cmd/qfw.c
@@ -22,8 +22,8 @@  static int qemu_fwcfg_cmd_setup_kernel(void *load_addr, void *initrd_addr)
 	char *data_addr;
 	uint32_t setup_size, kernel_size, cmdline_size, initrd_size;
 
-	qemu_fwcfg_read_entry(qfw_dev, FW_CFG_SETUP_SIZE, 4, &setup_size);
-	qemu_fwcfg_read_entry(qfw_dev, FW_CFG_KERNEL_SIZE, 4, &kernel_size);
+	qfw_read_entry(qfw_dev, FW_CFG_SETUP_SIZE, 4, &setup_size);
+	qfw_read_entry(qfw_dev, FW_CFG_KERNEL_SIZE, 4, &kernel_size);
 
 	if (setup_size == 0 || kernel_size == 0) {
 		printf("warning: no kernel available\n");
@@ -31,28 +31,28 @@  static int qemu_fwcfg_cmd_setup_kernel(void *load_addr, void *initrd_addr)
 	}
 
 	data_addr = load_addr;
-	qemu_fwcfg_read_entry(qfw_dev, FW_CFG_SETUP_DATA,
-			      le32_to_cpu(setup_size), data_addr);
+	qfw_read_entry(qfw_dev, FW_CFG_SETUP_DATA,
+		       le32_to_cpu(setup_size), data_addr);
 	data_addr += le32_to_cpu(setup_size);
 
-	qemu_fwcfg_read_entry(qfw_dev, FW_CFG_KERNEL_DATA,
-			      le32_to_cpu(kernel_size), data_addr);
+	qfw_read_entry(qfw_dev, FW_CFG_KERNEL_DATA,
+		       le32_to_cpu(kernel_size), data_addr);
 	data_addr += le32_to_cpu(kernel_size);
 
 	data_addr = initrd_addr;
-	qemu_fwcfg_read_entry(qfw_dev, FW_CFG_INITRD_SIZE, 4, &initrd_size);
+	qfw_read_entry(qfw_dev, FW_CFG_INITRD_SIZE, 4, &initrd_size);
 	if (initrd_size == 0) {
 		printf("warning: no initrd available\n");
 	} else {
-		qemu_fwcfg_read_entry(qfw_dev, FW_CFG_INITRD_DATA,
-				      le32_to_cpu(initrd_size), data_addr);
+		qfw_read_entry(qfw_dev, FW_CFG_INITRD_DATA,
+			       le32_to_cpu(initrd_size), data_addr);
 		data_addr += le32_to_cpu(initrd_size);
 	}
 
-	qemu_fwcfg_read_entry(qfw_dev, FW_CFG_CMDLINE_SIZE, 4, &cmdline_size);
+	qfw_read_entry(qfw_dev, FW_CFG_CMDLINE_SIZE, 4, &cmdline_size);
 	if (cmdline_size) {
-		qemu_fwcfg_read_entry(qfw_dev, FW_CFG_CMDLINE_DATA,
-				      le32_to_cpu(cmdline_size), data_addr);
+		qfw_read_entry(qfw_dev, FW_CFG_CMDLINE_DATA,
+			       le32_to_cpu(cmdline_size), data_addr);
 		/*
 		 * if kernel cmdline only contains '\0', (e.g. no -append
 		 * when invoking qemu), do not update bootargs
@@ -163,7 +163,7 @@  static int do_qemu_fw(struct cmd_tbl *cmdtp, int flag, int argc,
 	int ret;
 	struct cmd_tbl *fwcfg_cmd;
 
-	qfw_dev = qemu_fwcfg_dev();
+	qfw_dev = qfw_get_dev();
 	if (!qfw_dev) {
 		printf("QEMU fw_cfg interface not found\n");
 		return CMD_RET_USAGE;
diff --git a/common/Makefile b/common/Makefile
index daeea67cf2..f174a06c33 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -137,3 +137,5 @@  obj-$(CONFIG_CMD_LOADB) += xyzModem.o
 obj-$(CONFIG_$(SPL_TPL_)YMODEM_SUPPORT) += xyzModem.o
 
 obj-$(CONFIG_AVB_VERIFY) += avb_verify.o
+
+obj-$(CONFIG_QFW) += qfw.o
diff --git a/common/qfw.c b/common/qfw.c
new file mode 100644
index 0000000000..b4c9e4483c
--- /dev/null
+++ b/common/qfw.c
@@ -0,0 +1,111 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2015 Miao Yan <yanmiaobest@gmail.com>
+ * (C) Copyright 2021 Asherah Connor <ashe@kivikakk.ee>
+ */
+
+#include <dm.h>
+#include <dm/uclass.h>
+#include <qfw.h>
+#include <stdlib.h>
+
+struct udevice *qfw_get_dev(void)
+{
+	struct udevice *dev;
+	int ret;
+
+	ret = uclass_first_device(UCLASS_QFW, &dev);
+	if (ret)
+		return NULL;
+	return dev;
+}
+
+int qemu_fwcfg_online_cpus(struct udevice *dev)
+{
+	u16 nb_cpus;
+
+	qfw_read_entry(dev, FW_CFG_NB_CPUS, 2, &nb_cpus);
+
+	return le16_to_cpu(nb_cpus);
+}
+
+int qemu_fwcfg_read_firmware_list(struct udevice *dev)
+{
+	int i;
+	u32 count;
+	struct fw_file *file;
+	struct list_head *entry;
+
+	struct qfw_dev *qdev = dev_get_uclass_priv(dev);
+
+	/* don't read it twice */
+	if (!list_empty(&qdev->fw_list))
+		return 0;
+
+	qfw_read_entry(dev, FW_CFG_FILE_DIR, 4, &count);
+	if (!count)
+		return 0;
+
+	count = be32_to_cpu(count);
+	for (i = 0; i < count; i++) {
+		file = malloc(sizeof(*file));
+		if (!file) {
+			printf("error: allocating resource\n");
+			goto err;
+		}
+		qfw_read_entry(dev, FW_CFG_INVALID,
+			       sizeof(struct fw_cfg_file), &file->cfg);
+		file->addr = 0;
+		list_add_tail(&file->list, &qdev->fw_list);
+	}
+
+	return 0;
+
+err:
+	list_for_each(entry, &qdev->fw_list) {
+		file = list_entry(entry, struct fw_file, list);
+		free(file);
+	}
+
+	return -ENOMEM;
+}
+
+struct fw_file *qemu_fwcfg_find_file(struct udevice *dev, const char *name)
+{
+	struct list_head *entry;
+	struct fw_file *file;
+
+	struct qfw_dev *qdev = dev_get_uclass_priv(dev);
+
+	list_for_each(entry, &qdev->fw_list) {
+		file = list_entry(entry, struct fw_file, list);
+		if (!strcmp(file->cfg.name, name))
+			return file;
+	}
+
+	return NULL;
+}
+
+struct fw_file *qemu_fwcfg_file_iter_init(struct udevice *dev,
+					  struct fw_cfg_file_iter *iter)
+{
+	struct qfw_dev *qdev = dev_get_uclass_priv(dev);
+
+	iter->entry = qdev->fw_list.next;
+	iter->end = &qdev->fw_list;
+	return list_entry((struct list_head *)iter->entry,
+			  struct fw_file, list);
+}
+
+struct fw_file *qemu_fwcfg_file_iter_next(struct fw_cfg_file_iter *iter)
+{
+	iter->entry = ((struct list_head *)iter->entry)->next;
+	return list_entry((struct list_head *)iter->entry,
+			  struct fw_file, list);
+}
+
+bool qemu_fwcfg_file_iter_end(struct fw_cfg_file_iter *iter)
+{
+	return iter->entry == iter->end;
+}
+
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 0a65f29acd..7d2a299779 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -367,7 +367,6 @@  config WINBOND_W83627
 
 config QFW
 	bool
-	depends on MISC
 	help
 	  Hidden option to enable QEMU fw_cfg interface. This will be selected by
 	  either CONFIG_CMD_QFW or CONFIG_GENERATE_ACPI_TABLE.
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index d737203704..e6e1dfea95 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -56,6 +56,12 @@  obj-$(CONFIG_P2SB) += p2sb-uclass.o
 obj-$(CONFIG_PCA9551_LED) += pca9551_led.o
 obj-$(CONFIG_$(SPL_)PWRSEQ) += pwrseq-uclass.o
 obj-$(CONFIG_QFW) += qfw.o
+ifdef CONFIG_X86
+obj-$(CONFIG_QFW) += qfw_pio.o
+endif
+ifdef CONFIG_ARM
+obj-$(CONFIG_QFW) += qfw_mmio.o
+endif
 obj-$(CONFIG_ROCKCHIP_EFUSE) += rockchip-efuse.o
 obj-$(CONFIG_ROCKCHIP_OTP) += rockchip-otp.o
 obj-$(CONFIG_SANDBOX) += syscon_sandbox.o misc_sandbox.o
diff --git a/drivers/misc/qfw.c b/drivers/misc/qfw.c
index eae3afd23b..25b203375b 100644
--- a/drivers/misc/qfw.c
+++ b/drivers/misc/qfw.c
@@ -1,8 +1,11 @@ 
 // SPDX-License-Identifier: GPL-2.0+
 /*
  * (C) Copyright 2015 Miao Yan <yanmiaobest@gmail.com>
+ * (C) Copyright 2021 Asherah Connor <ashe@kivikakk.ee>
  */
 
+#define LOG_CATEGORY UCLASS_QFW
+
 #include <common.h>
 #include <command.h>
 #include <errno.h>
@@ -10,18 +13,11 @@ 
 #include <malloc.h>
 #include <qfw.h>
 #include <dm.h>
-#include <asm/io.h>
 #include <misc.h>
 #ifdef CONFIG_GENERATE_ACPI_TABLE
 #include <asm/tables.h>
 #endif
 
-/* Determined at runtime. */
-struct qfw_priv {
-	bool dma_present;
-	struct list_head fw_list;
-};
-
 #ifdef CONFIG_GENERATE_ACPI_TABLE
 /*
  * This function allocates memory for ACPI tables
@@ -77,8 +73,8 @@  static int bios_linker_allocate(struct udevice *dev,
 	debug("bios_linker_allocate: allocate file %s, size %u, zone %d, align %u, addr 0x%lx\n",
 	      file->cfg.name, size, entry->alloc.zone, align, aligned_addr);
 
-	qemu_fwcfg_read_entry(dev, be16_to_cpu(file->cfg.select),
-			      size, (void *)aligned_addr);
+	qfw_read_entry(dev, be16_to_cpu(file->cfg.select), size,
+		       (void *)aligned_addr);
 	file->addr = aligned_addr;
 
 	/* adjust address for low memory allocation */
@@ -160,7 +156,7 @@  ulong write_acpi_tables(ulong addr)
 	uint32_t size;
 	struct udevice *dev;
 
-	dev = qemu_fwcfg_dev();
+	dev = qfw_get_dev();
 	if (!dev) {
 		printf("error: no qfw\n");
 		return addr;
@@ -191,8 +187,7 @@  ulong write_acpi_tables(ulong addr)
 		return addr;
 	}
 
-	qemu_fwcfg_read_entry(dev, be16_to_cpu(file->cfg.select),
-			      size, table_loader);
+	qfw_read_entry(dev, be16_to_cpu(file->cfg.select), size, table_loader);
 
 	for (i = 0; i < (size / sizeof(*entry)); i++) {
 		entry = table_loader + i;
@@ -239,7 +234,7 @@  ulong acpi_get_rsdp_addr(void)
 	struct fw_file *file;
 	struct udevice *dev;
 
-	dev = qemu_fwcfg_dev();
+	dev = qfw_get_dev();
 	if (!dev) {
 		printf("error: no qfw\n");
 		return 0;
@@ -250,262 +245,75 @@  ulong acpi_get_rsdp_addr(void)
 }
 #endif
 
-/* Read configuration item using fw_cfg PIO/MMIO interface */
-static void qemu_fwcfg_read_entry_io(struct qfw_plat *plat, u16 entry,
-				     u32 size, void *address)
+static void qfw_read_entry_io(struct qfw_dev *qdev, u16 entry, u32 size,
+			      void *address)
 {
-	debug("qemu_fwcfg_read_entry_io: entry 0x%x, size %u address %p\n",
-	      entry, size, address);
+	struct dm_qfw_ops *ops = dm_qfw_get_ops(qdev->dev);
 
-	/*
-	 * writing FW_CFG_INVALID will cause read operation to resume at last
-	 * offset, otherwise read will start at offset 0
-	 *
-	 * Note: on platform where the control register is IO port, the
-	 * endianness is little endian.  Where it is on MMIO, the register is
-	 * big endian.
-	 */
-	if (entry != FW_CFG_INVALID) {
-		if (plat->mmio)
-			plat->mmio->selector = cpu_to_be16(entry);
-#ifdef CONFIG_X86
-		else
-			outw(cpu_to_le16(entry), plat->io.control_port);
-#endif
-	}
+	debug("%s: entry 0x%x, size %u address %p\n", __func__, entry, size,
+	      address);
 
-	/* the endianness of data register is string-preserving */
-
-	if (plat->mmio) {
-		while (size >= 8) {
-			*(u64 *)address = plat->mmio->data64;
-			address += 8;
-			size -= 8;
-		}
-		while (size >= 4) {
-			*(u32 *)address = plat->mmio->data32;
-			address += 4;
-			size -= 4;
-		}
-		while (size >= 2) {
-			*(u16 *)address = plat->mmio->data16;
-			address += 2;
-			size -= 2;
-		}
-		while (size >= 1) {
-			*(u8 *)address = plat->mmio->data8;
-			address += 1;
-			size -= 1;
-		}
-	}
-#ifdef CONFIG_X86
-	else {
-		u32 i = 0;
-		u8 *data = address;
-
-		while (size--)
-			data[i++] = inb(plat->io.data_port);
-	}
-#endif
+	ops->read_entry_io(qdev->dev, entry, size, address);
 }
 
-/* Read configuration item using fw_cfg DMA interface */
-static void qemu_fwcfg_read_entry_dma(struct qfw_plat *plat, u16 entry,
-				      u32 size, void *address)
+static void qfw_read_entry_dma(struct qfw_dev *qdev, u16 entry, u32 size,
+			       void *address)
 {
-	struct {
-		__be32 control;
-		__be32 length;
-		__be64 address;
-	} dma = {
+	struct dm_qfw_ops *ops = dm_qfw_get_ops(qdev->dev);
+
+	struct qfw_dma dma = {
 		.length = cpu_to_be32(size),
 		.address = cpu_to_be64((uintptr_t)address),
 		.control = cpu_to_be32(FW_CFG_DMA_READ),
 	};
 
 	/*
-	 * writting FW_CFG_INVALID will cause read operation to resume at
-	 * last offset, otherwise read will start at offset 0
+	 * writing FW_CFG_INVALID will cause read operation to resume at last
+	 * offset, otherwise read will start at offset 0
 	 */
 	if (entry != FW_CFG_INVALID)
 		dma.control |= cpu_to_be32(FW_CFG_DMA_SELECT | (entry << 16));
 
-	barrier();
-
-	debug("qemu_fwcfg_read_entry_dma: entry 0x%x, size %u address %p, control 0x%x\n",
+	debug("%s: entry 0x%x, size %u address %p, control 0x%x\n", __func__,
 	      entry, size, address, be32_to_cpu(dma.control));
 
-	/* the DMA address register is big-endian */
-	if (plat->mmio)
-		plat->mmio->dma = cpu_to_be64((uintptr_t)&dma);
-#ifdef CONFIG_X86
-	else
-		outl(cpu_to_be32((uintptr_t)&dma), plat->io.dma_port_high);
-#endif
-
+	barrier();
 
-	while (be32_to_cpu(dma.control) & ~FW_CFG_DMA_ERROR)
-#ifdef CONFIG_X86
-		__asm__ __volatile__ ("pause");
-#else
-		__asm__ __volatile__ ("yield");
-#endif
+	ops->read_entry_dma(qdev->dev, &dma);
 }
 
-void qemu_fwcfg_read_entry(struct udevice *dev, u16 entry, u32 length,
-			   void *address)
+void qfw_read_entry(struct udevice *dev, u16 entry, u32 length, void *address)
 {
-	struct qfw_plat *plat = dev_get_plat(dev);
-	struct qfw_priv *priv = dev_get_priv(dev);
+	struct qfw_dev *qdev = dev_get_uclass_priv(dev);
 
-	if (priv->dma_present)
-		qemu_fwcfg_read_entry_dma(plat, entry, length, address);
+	if (qdev->dma_present)
+		qfw_read_entry_dma(qdev, entry, length, address);
 	else
-		qemu_fwcfg_read_entry_io(plat, entry, length, address);
-}
-
-int qemu_fwcfg_online_cpus(struct udevice *dev)
-{
-	u16 nb_cpus;
-
-	qemu_fwcfg_read_entry(dev, FW_CFG_NB_CPUS, 2, &nb_cpus);
-
-	return le16_to_cpu(nb_cpus);
-}
-
-int qemu_fwcfg_read_firmware_list(struct udevice *dev)
-{
-	int i;
-	u32 count;
-	struct fw_file *file;
-	struct list_head *entry;
-
-	struct qfw_priv *priv = dev_get_priv(dev);
-
-	/* don't read it twice */
-	if (!list_empty(&priv->fw_list))
-		return 0;
-
-	qemu_fwcfg_read_entry(dev, FW_CFG_FILE_DIR, 4, &count);
-	if (!count)
-		return 0;
-
-	count = be32_to_cpu(count);
-	for (i = 0; i < count; i++) {
-		file = malloc(sizeof(*file));
-		if (!file) {
-			printf("error: allocating resource\n");
-			goto err;
-		}
-		qemu_fwcfg_read_entry(dev, FW_CFG_INVALID,
-				      sizeof(struct fw_cfg_file), &file->cfg);
-		file->addr = 0;
-		list_add_tail(&file->list, &priv->fw_list);
-	}
-
-	return 0;
-
-err:
-	list_for_each(entry, &priv->fw_list) {
-		file = list_entry(entry, struct fw_file, list);
-		free(file);
-	}
-
-	return -ENOMEM;
-}
-
-struct fw_file *qemu_fwcfg_find_file(struct udevice *dev, const char *name)
-{
-	struct list_head *entry;
-	struct fw_file *file;
-
-	struct qfw_priv *priv = dev_get_priv(dev);
-
-	list_for_each(entry, &priv->fw_list) {
-		file = list_entry(entry, struct fw_file, list);
-		if (!strcmp(file->cfg.name, name))
-			return file;
-	}
-
-	return NULL;
-}
-
-struct fw_file *qemu_fwcfg_file_iter_init(struct udevice *dev,
-					  struct fw_cfg_file_iter *iter)
-{
-	struct qfw_priv *priv = dev_get_priv(dev);
-
-	iter->entry = priv->fw_list.next;
-	iter->end = &priv->fw_list;
-	return list_entry((struct list_head *)iter->entry,
-			  struct fw_file, list);
-}
-
-struct fw_file *qemu_fwcfg_file_iter_next(struct fw_cfg_file_iter *iter)
-{
-	iter->entry = ((struct list_head *)iter->entry)->next;
-	return list_entry((struct list_head *)iter->entry,
-			  struct fw_file, list);
-}
-
-bool qemu_fwcfg_file_iter_end(struct fw_cfg_file_iter *iter)
-{
-	return iter->entry == iter->end;
-}
-
-static int qfw_of_to_plat(struct udevice *dev)
-{
-	struct qfw_plat *plat = dev_get_plat(dev);
-
-	plat->mmio = map_physmem(dev_read_addr(dev),
-				 sizeof(struct qfw_mmio),
-				 MAP_NOCACHE);
-
-	return 0;
+		qfw_read_entry_io(qdev, entry, length, address);
 }
 
-static int qfw_probe(struct udevice *dev)
+int qfw_register(struct udevice *dev)
 {
+	struct qfw_dev *qdev = dev_get_uclass_priv(dev);
 	u32 qemu, dma_enabled;
-	struct qfw_plat *plat = dev_get_plat(dev);
-	struct qfw_priv *priv = dev_get_priv(dev);
 
-	INIT_LIST_HEAD(&priv->fw_list);
+	qdev->dev = dev;
+	INIT_LIST_HEAD(&qdev->fw_list);
 
-	qemu_fwcfg_read_entry_io(plat, FW_CFG_SIGNATURE, 4, &qemu);
+	qfw_read_entry_io(qdev, FW_CFG_SIGNATURE, 4, &qemu);
 	if (be32_to_cpu(qemu) != QEMU_FW_CFG_SIGNATURE)
 		return -ENODEV;
 
-	qemu_fwcfg_read_entry_io(plat, FW_CFG_ID, 1, &dma_enabled);
+	qfw_read_entry_io(qdev, FW_CFG_ID, 1, &dma_enabled);
 	if (dma_enabled & FW_CFG_DMA_ENABLED)
-		priv->dma_present = true;
+		qdev->dma_present = true;
 
 	return 0;
 }
 
-static const struct udevice_id qfw_ids[] = {
-	{ .compatible = "qemu,fw-cfg-mmio" },
-	{}
-};
-
-U_BOOT_DRIVER(qfw) = {
+UCLASS_DRIVER(qfw) = {
+	.id		= UCLASS_QFW,
 	.name		= "qfw",
-	.id		= UCLASS_MISC,
-	.of_match	= qfw_ids,
-	.of_to_plat	= qfw_of_to_plat,
-	.plat_auto	= sizeof(struct qfw_plat),
-	.priv_auto	= sizeof(struct qfw_priv),
-	.probe		= qfw_probe,
+	.per_device_auto	= sizeof(struct qfw_dev),
 };
 
-struct udevice *qemu_fwcfg_dev(void)
-{
-	struct udevice *dev;
-	int ret;
-
-	/* XXX: decide what to do here */
-	ret = uclass_first_device(UCLASS_MISC, &dev);
-	if (ret)
-		return NULL;
-	return dev;
-}
diff --git a/drivers/misc/qfw_mmio.c b/drivers/misc/qfw_mmio.c
new file mode 100644
index 0000000000..aa903cb51c
--- /dev/null
+++ b/drivers/misc/qfw_mmio.c
@@ -0,0 +1,101 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * MMIO interface for QFW
+ *
+ * (C) Copyright 2015 Miao Yan <yanmiaobest@gmail.com>
+ * (C) Copyright 2021 Asherah Connor <ashe@kivikakk.ee>
+ */
+
+#define LOG_CATEGORY UCLASS_QFW
+
+#include <asm/types.h>
+#include <asm/io.h>
+#include <dm.h>
+#include <dm/device.h>
+#include <qfw.h>
+
+static void qfw_mmio_read_entry_io(struct udevice *dev, u16 entry, u32 size,
+				   void *address)
+{
+	struct qfw_mmio_plat *plat = dev_get_plat(dev);
+
+	/*
+	 * writing FW_CFG_INVALID will cause read operation to resume at last
+	 * offset, otherwise read will start at offset 0
+	 *
+	 * Note: on platform where the control register is MMIO, the register
+	 * is big endian.
+	 */
+	if (entry != FW_CFG_INVALID)
+		plat->mmio->selector = cpu_to_be16(entry);
+
+	/* the endianness of data register is string-preserving */
+	while (size >= 8) {
+		*(u64 *)address = plat->mmio->data64;
+		address += 8;
+		size -= 8;
+	}
+	while (size >= 4) {
+		*(u32 *)address = plat->mmio->data32;
+		address += 4;
+		size -= 4;
+	}
+	while (size >= 2) {
+		*(u16 *)address = plat->mmio->data16;
+		address += 2;
+		size -= 2;
+	}
+	while (size >= 1) {
+		*(u8 *)address = plat->mmio->data8;
+		address += 1;
+		size -= 1;
+	}
+}
+
+/* Read configuration item using fw_cfg DMA interface */
+static void qfw_mmio_read_entry_dma(struct udevice *dev, struct qfw_dma *dma)
+{
+	struct qfw_mmio_plat *plat = dev_get_plat(dev);
+
+	/* the DMA address register is big-endian */
+	plat->mmio->dma = cpu_to_be64((uintptr_t)dma);
+
+	while (be32_to_cpu(dma->control) & ~FW_CFG_DMA_ERROR)
+		__asm__ __volatile__ ("yield");
+}
+
+static int qfw_mmio_of_to_plat(struct udevice *dev)
+{
+	struct qfw_mmio_plat *plat = dev_get_plat(dev);
+
+	plat->mmio = map_physmem(dev_read_addr(dev),
+				 sizeof(struct qfw_mmio),
+				 MAP_NOCACHE);
+
+	return 0;
+}
+
+static int qfw_mmio_probe(struct udevice *dev)
+{
+	return qfw_register(dev);
+}
+
+static struct dm_qfw_ops qfw_mmio_ops = {
+	.read_entry_io = qfw_mmio_read_entry_io,
+	.read_entry_dma = qfw_mmio_read_entry_dma,
+};
+
+static const struct udevice_id qfw_mmio_ids[] = {
+	{ .compatible = "qemu,fw-cfg-mmio" },
+	{}
+};
+
+U_BOOT_DRIVER(qfw_mmio) = {
+	.name	= "qfw_mmio",
+	.id	= UCLASS_QFW,
+	.of_match	= qfw_mmio_ids,
+	.plat_auto	= sizeof(struct qfw_mmio_plat),
+	.of_to_plat	= qfw_mmio_of_to_plat,
+	.probe	= qfw_mmio_probe,
+	.ops	= &qfw_mmio_ops,
+};
diff --git a/drivers/misc/qfw_pio.c b/drivers/misc/qfw_pio.c
new file mode 100644
index 0000000000..beb722ad22
--- /dev/null
+++ b/drivers/misc/qfw_pio.c
@@ -0,0 +1,66 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * PIO interface for QFW
+ *
+ * (C) Copyright 2015 Miao Yan <yanmiaobest@gmail.com>
+ * (C) Copyright 2021 Asherah Connor <ashe@kivikakk.ee>
+ */
+
+#define LOG_CATEGORY UCLASS_QFW
+
+#include <asm/io.h>
+#include <dm/device.h>
+#include <qfw.h>
+
+static void qfw_pio_read_entry_io(struct udevice *dev, u16 entry, u32 size,
+				  void *address)
+{
+	struct qfw_pio_plat *plat = dev_get_plat(dev);
+
+	/*
+	 * writing FW_CFG_INVALID will cause read operation to resume at last
+	 * offset, otherwise read will start at offset 0
+	 *
+	 * Note: on platform where the control register is IO port, the
+	 * endianness is little endian.
+	 */
+	if (entry != FW_CFG_INVALID)
+		outw(cpu_to_le16(entry), plat->control_port);
+
+	/* the endianness of data register is string-preserving */
+	u32 i = 0;
+	u8 *data = address;
+
+	while (size--)
+		data[i++] = inb(plat->data_port);
+}
+
+/* Read configuration item using fw_cfg DMA interface */
+static void qfw_pio_read_entry_dma(struct udevice *dev, struct qfw_dma *dma)
+{
+	struct qfw_pio_plat *plat = dev_get_plat(dev);
+
+	/* the DMA address register is big-endian */
+	outl(cpu_to_be32((uintptr_t)dma), plat->dma_port_high);
+
+	while (be32_to_cpu(dma->control) & ~FW_CFG_DMA_ERROR)
+		__asm__ __volatile__ ("pause");
+}
+
+static int qfw_pio_probe(struct udevice *dev)
+{
+	return qfw_register(dev);
+}
+
+static struct dm_qfw_ops qfw_pio_ops = {
+	.read_entry_io = qfw_pio_read_entry_io,
+	.read_entry_dma = qfw_pio_read_entry_dma,
+};
+
+U_BOOT_DRIVER(qfw_pio) = {
+	.name	= "qfw_pio",
+	.id	= UCLASS_QFW,
+	.plat_auto	= sizeof(struct qfw_pio_plat),
+	.probe	= qfw_pio_probe,
+	.ops	= &qfw_pio_ops,
+};
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index d75de368c5..d800f679d5 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -90,6 +90,7 @@  enum uclass_id {
 	UCLASS_POWER_DOMAIN,	/* (SoC) Power domains */
 	UCLASS_PWM,		/* Pulse-width modulator */
 	UCLASS_PWRSEQ,		/* Power sequence device */
+	UCLASS_QFW,		/* QEMU firmware config device */
 	UCLASS_RAM,		/* RAM controller */
 	UCLASS_REGULATOR,	/* Regulator device */
 	UCLASS_REMOTEPROC,	/* Remote Processor device */
diff --git a/include/qfw.h b/include/qfw.h
index f9c6828841..d7e16651a2 100644
--- a/include/qfw.h
+++ b/include/qfw.h
@@ -149,28 +149,49 @@  struct qfw_mmio {
 	u64 dma;
 };
 
-struct qfw_plat {
-	/* MMIO used on Arm. */
+struct qfw_pio_plat {
+	u16 control_port;
+	u16 data_port;
+	u16 dma_port_low;
+	u16 dma_port_high;
+};
+
+struct qfw_mmio_plat {
 	volatile struct qfw_mmio *mmio;
-	/* IO used on x86. */
-	struct {
-		u16 control_port;
-		u16 data_port;
-		u16 dma_port_low;
-		u16 dma_port_high;
-	} io;
 };
 
+struct qfw_dma {
+	__be32 control;
+	__be32 length;
+	__be64 address;
+};
+
+struct qfw_dev {
+	struct udevice *dev;
+	bool dma_present;
+	struct list_head fw_list;
+};
+
+struct dm_qfw_ops {
+	void (*read_entry_io)(struct udevice *dev, u16 entry, u32 size,
+			      void *address);
+	void (*read_entry_dma)(struct udevice *dev, struct qfw_dma *dma);
+};
+
+#define dm_qfw_get_ops(dev) \
+		((struct dm_qfw_ops *)(dev)->driver->ops)
+
+int qfw_register(struct udevice *dev);
+
 struct udevice;
 /**
  * Get QEMU firmware config device
  *
  * @return struct udevice * if present, NULL otherwise
  */
-struct udevice *qemu_fwcfg_dev(void);
+struct udevice *qfw_get_dev(void);
 
-void qemu_fwcfg_read_entry(struct udevice *dev, u16 entry, u32 length,
-			   void *address);
+void qfw_read_entry(struct udevice *dev, u16 entry, u32 length, void *address);
 int qemu_fwcfg_read_firmware_list(struct udevice *dev);
 struct fw_file *qemu_fwcfg_find_file(struct udevice *dev, const char *name);