Hi Asherah, On Wed, Feb 24, 2021 at 11:23 AM Asherah Connor <[email protected]> wrote: > > Updates the QFW driver to use the driver model, splitting the driver > into qfw_pio and qfw_mmio (for non-x86) in their own uclass. > > Signed-off-by: Asherah Connor <[email protected]> > --- > > Changes in v4: > - PIO definitions are now #defines > - qfw_*_plat structs are no longer in header files > - Remove yield/pause in DMA wait loop > - Change struct udevice *qfw_get_dev(void) to int qfw_get_dev(struct > udevice **) > > arch/arm/Kconfig | 1 + > arch/x86/cpu/qemu/cpu.c | 9 +- > arch/x86/cpu/qemu/qemu.c | 47 +------- > arch/x86/cpu/qfw_cpu.c | 11 +- > cmd/qfw.c | 52 ++++----- > common/Makefile | 2 + > common/qfw.c | 105 +++++++++++++++++ > drivers/misc/Makefile | 6 +- > drivers/misc/qfw.c | 238 ++++++++++++++------------------------- > drivers/misc/qfw_mmio.c | 119 ++++++++++++++++++++ > drivers/misc/qfw_pio.c | 69 ++++++++++++ > include/dm/uclass-id.h | 1 + > include/qfw.h | 61 ++++++---- > 13 files changed, 466 insertions(+), 255 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/arch/arm/Kconfig b/arch/arm/Kconfig > index d51abbeaf0..cd01dc458a 100644 > --- a/arch/arm/Kconfig > +++ b/arch/arm/Kconfig > @@ -937,6 +937,7 @@ config ARCH_QEMU > imply DM_RNG > imply DM_RTC > imply RTC_PL031 > + imply CMD_QFW > > config ARCH_RMOBILE > bool "Renesas ARM SoCs" > diff --git a/arch/x86/cpu/qemu/cpu.c b/arch/x86/cpu/qemu/cpu.c > index 9ce86b379c..c78e374644 100644 > --- a/arch/x86/cpu/qemu/cpu.c > +++ b/arch/x86/cpu/qemu/cpu.c > @@ -22,7 +22,14 @@ int cpu_qemu_get_desc(const struct udevice *dev, char > *buf, int size) > > static int cpu_qemu_get_count(const struct udevice *dev) > { > - return qemu_fwcfg_online_cpus(); > + int ret; > + struct udevice *qfw_dev; > + > + ret = qfw_get_dev(&qfw_dev); > + if (ret) > + return ret; > + > + return qemu_fwcfg_online_cpus(qfw_dev); > } > > static const struct cpu_ops cpu_qemu_ops = { > diff --git a/arch/x86/cpu/qemu/qemu.c b/arch/x86/cpu/qemu/qemu.c > index 044a429c13..605f51e1b8 100644 > --- a/arch/x86/cpu/qemu/qemu.c > +++ b/arch/x86/cpu/qemu/qemu.c > @@ -8,6 +8,7 @@ > #include <init.h> > #include <pci.h> > #include <qfw.h> > +#include <dm/platdata.h> > #include <asm/irq.h> > #include <asm/post.h> > #include <asm/processor.h> > @@ -18,46 +19,10 @@ static bool i440fx; > > #ifdef CONFIG_QFW > > -/* on x86, the qfw registers are all IO ports */ > -#define FW_CONTROL_PORT 0x510 > -#define FW_DATA_PORT 0x511 > -#define FW_DMA_PORT_LOW 0x514 > -#define FW_DMA_PORT_HIGH 0x518 > - > -static void qemu_x86_fwcfg_read_entry_pio(uint16_t entry, > - uint32_t size, void *address) > -{ > - uint32_t i = 0; > - uint8_t *data = address; > - > - /* > - * writting 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), FW_CONTROL_PORT); > - > - /* the endianness of data register is string-preserving */ > - while (size--) > - data[i++] = inb(FW_DATA_PORT); > -} > - > -static void qemu_x86_fwcfg_read_entry_dma(struct fw_cfg_dma_access *dma) > -{ > - /* the DMA address register is big endian */ > - outl(cpu_to_be32((uintptr_t)dma), FW_DMA_PORT_HIGH); > - > - while (be32_to_cpu(dma->control) & ~FW_CFG_DMA_ERROR) > - __asm__ __volatile__ ("pause"); > -} > - > -static struct fw_cfg_arch_ops fwcfg_x86_ops = { > - .arch_read_pio = qemu_x86_fwcfg_read_entry_pio, > - .arch_read_dma = qemu_x86_fwcfg_read_entry_dma > +U_BOOT_DRVINFO(x86_qfw_pio) = { > + .name = "qfw_pio", > }; > + > #endif > > static void enable_pm_piix(void) > @@ -132,10 +97,6 @@ static void qemu_chipset_init(void) > > enable_pm_ich9(); > } > - > -#ifdef CONFIG_QFW > - qemu_fwcfg_init(&fwcfg_x86_ops); > -#endif > } > > #if !CONFIG_IS_ENABLED(SPL_X86_32BIT_INIT) > diff --git a/arch/x86/cpu/qfw_cpu.c b/arch/x86/cpu/qfw_cpu.c > index b959eaddde..9700908064 100644 > --- a/arch/x86/cpu/qfw_cpu.c > +++ b/arch/x86/cpu/qfw_cpu.c > @@ -18,7 +18,7 @@ int qemu_cpu_fixup(void) > int cpu_num; > int cpu_online; > struct uclass *uc; > - struct udevice *dev, *pdev; > + struct udevice *dev, *pdev, *qfwdev; > struct cpu_plat *plat; > char *cpu; > > @@ -39,6 +39,13 @@ int qemu_cpu_fixup(void) > return -ENODEV; > } > > + /* get qfw dev */ > + ret = qfw_get_dev(&qfwdev); > + if (ret) { > + printf("unable to find qfw device\n"); > + return ret; > + } > + > /* calculate cpus that are already bound */ > cpu_num = 0; > for (uclass_find_first_device(UCLASS_CPU, &dev); > @@ -48,7 +55,7 @@ int qemu_cpu_fixup(void) > } > > /* get actual cpu number */ > - cpu_online = qemu_fwcfg_online_cpus(); > + cpu_online = qemu_fwcfg_online_cpus(qfwdev); > if (cpu_online < 0) { > printf("unable to get online cpu number: %d\n", cpu_online); > return cpu_online; > diff --git a/cmd/qfw.c b/cmd/qfw.c > index bb571487f0..87af408a8d 100644 > --- a/cmd/qfw.c > +++ b/cmd/qfw.c > @@ -8,19 +8,22 @@ > #include <env.h> > #include <errno.h> > #include <qfw.h> > +#include <dm.h> > + > +static struct udevice *qfw_dev; > > /* > * This function prepares kernel for zboot. It loads kernel data > * to 'load_addr', initrd to 'initrd_addr' and kernel command > * line using qemu fw_cfg interface. > */ > -static int qemu_fwcfg_setup_kernel(void *load_addr, void *initrd_addr) > +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(FW_CFG_SETUP_SIZE, 4, &setup_size); > - qemu_fwcfg_read_entry(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"); > @@ -28,28 +31,28 @@ static int qemu_fwcfg_setup_kernel(void *load_addr, void > *initrd_addr) > } > > data_addr = load_addr; > - qemu_fwcfg_read_entry(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(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(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(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(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(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 > @@ -72,19 +75,18 @@ static int qemu_fwcfg_setup_kernel(void *load_addr, void > *initrd_addr) > return 0; > } > > -static int qemu_fwcfg_list_firmware(void) > +static int qemu_fwcfg_cmd_list_firmware(void) > { > int ret; > struct fw_cfg_file_iter iter; > struct fw_file *file; > > /* make sure fw_list is loaded */ > - ret = qemu_fwcfg_read_firmware_list(); > + ret = qemu_fwcfg_read_firmware_list(qfw_dev); > if (ret) > return ret; > > - > - for (file = qemu_fwcfg_file_iter_init(&iter); > + for (file = qemu_fwcfg_file_iter_init(qfw_dev, &iter); > !qemu_fwcfg_file_iter_end(&iter); > file = qemu_fwcfg_file_iter_next(&iter)) { > printf("%-56s\n", file->cfg.name); > @@ -96,7 +98,7 @@ static int qemu_fwcfg_list_firmware(void) > static int qemu_fwcfg_do_list(struct cmd_tbl *cmdtp, int flag, > int argc, char *const argv[]) > { > - if (qemu_fwcfg_list_firmware() < 0) > + if (qemu_fwcfg_cmd_list_firmware() < 0) > return CMD_RET_FAILURE; > > return 0; > @@ -105,14 +107,7 @@ static int qemu_fwcfg_do_list(struct cmd_tbl *cmdtp, int > flag, > static int qemu_fwcfg_do_cpus(struct cmd_tbl *cmdtp, int flag, > int argc, char *const argv[]) > { > - int ret = qemu_fwcfg_online_cpus(); > - if (ret < 0) { > - printf("QEMU fw_cfg interface not found\n"); > - return CMD_RET_FAILURE; > - } > - > - printf("%d cpu(s) online\n", qemu_fwcfg_online_cpus()); > - > + printf("%d cpu(s) online\n", qemu_fwcfg_online_cpus(qfw_dev)); > return 0; > } > > @@ -153,7 +148,7 @@ static int qemu_fwcfg_do_load(struct cmd_tbl *cmdtp, int > flag, > return CMD_RET_FAILURE; > } > > - return qemu_fwcfg_setup_kernel(load_addr, initrd_addr); > + return qemu_fwcfg_cmd_setup_kernel(load_addr, initrd_addr); > } > > static struct cmd_tbl fwcfg_commands[] = { > @@ -168,7 +163,8 @@ static int do_qemu_fw(struct cmd_tbl *cmdtp, int flag, > int argc, > int ret; > struct cmd_tbl *fwcfg_cmd; > > - if (!qemu_fwcfg_present()) { > + ret = qfw_get_dev(&qfw_dev); > + if (ret) { > 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..c0ffa20b74 > --- /dev/null > +++ b/common/qfw.c > @@ -0,0 +1,105 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * (C) Copyright 2015 Miao Yan <[email protected]> > + * (C) Copyright 2021 Asherah Connor <[email protected]> > + */ > + > +#include <dm.h> > +#include <dm/uclass.h> > +#include <qfw.h> > +#include <stdlib.h> > + > +int qfw_get_dev(struct udevice **devp) > +{ > + return uclass_first_device(UCLASS_QFW, devp); > +} > + > +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/Makefile b/drivers/misc/Makefile > index d737203704..2988289ea3 100644 > --- a/drivers/misc/Makefile > +++ b/drivers/misc/Makefile > @@ -55,7 +55,11 @@ obj-$(CONFIG_NUVOTON_NCT6102D) += nuvoton_nct6102d.o > 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_QFW > +obj-y += qfw.o > +obj-$(CONFIG_X86) += qfw_pio.o > +obj-$(CONFIG_ARM) += qfw_mmio.o
This does not scale when we reuse this driver on RISC-V, as I saw your QEMU patch. So we can do like this: CONFIG_QFW_PIO and CONFIG_QFW_MMIO Let QEMU x86 select CONFIG_QFW_PIO, and ARM (RISC-V in the future) select CONFIG_QFW_MMIO > +endif > obj-$(CONFIG_ROCKCHIP_EFUSE) += rockchip-efuse.o > obj-$(CONFIG_ROCKCHIP_OTP) += rockchip-otp.o > obj-$(CONFIG_SANDBOX) += syscon_sandbox.o misc_sandbox.o [snip] Regards, Bin

