From mboxrd@z Thu Jan  1 00:00:00 1970
From: Asherah Connor <ashe@kivikakk.ee>
Date: Wed, 24 Feb 2021 00:22:54 +0000
Subject: [PATCH v3 1/4] arm: x86: qemu: move qfw to DM, include Arm support
In-Reply-To: <d26821d4-dc30-2e17-a2dc-67dbaab6beee@gmx.de>
References: <20210223114329.16729-1-ashe@kivikakk.ee>
 <20210223114329.16729-2-ashe@kivikakk.ee>
 <d26821d4-dc30-2e17-a2dc-67dbaab6beee@gmx.de>
Message-ID: <20210224002254.xckn6g4tcue6cqkd@talia.n4wrvuuuhszuhem3na2pm5saea.px.internal.cloudapp.net>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On 21/02/23 01:02:p, Heinrich Schuchardt wrote:
> On 23.02.21 12:43, Asherah Connor wrote:
> For which architectures does the fw_cfg device exist?
> 
> It it is only ARM and X86, than I am missing such a dependency on
> CONFIG_CMD_QFW.

Right now we have:

arch/arm/Kconfig:
	...
	config ARCH_QEMU
		...
		imply CMD_QFW

I think we also want this:

arch/x86/cpu/qemu/Kconfig
	config QEMU
		...
		imply CMD_QFW

This is my first time using Kconfig and I'll admit I'm not too certain
where things go.


> If these numbers are constants, why should they be copied to platform
> data? This only increases code size.
> 
> I think there is nothing wrong with using constants here.

Okay, excellent.  I'll fold those into drivers/misc/qfw_pio.c and get
rid of the qfw_pio_plat struct entirely.  


> ARM yield is meant to be used on multi-threaded systems to indicate that
> the thread can be swapped. Why would we need it in U-Boot which is
> single-threaded?
> 
> Can't we simply use
> 
> while (be32_to_cpu(dma.control) & ~FW_CFG_DMA_ERROR);
> 
> with no command in the loop for all architectures?

Good question.  This code originated here, where the original (x86-only)
driver used pause:

https://gitlab.denx.de/u-boot/u-boot/-/commit/f60df20aa966c3de850afafe3cce70a51d0b261c#41c93c056084377352da52f1d88fc49288a4846f_0_59

When porting to Arm I used the equivalent.

While U-Boot is single-threaded, the architecture that executes this
instruction is always QEMU, and -- at a guess -- it might be that
pause/yield here lets QEMU finish its part of the DMA faster.

I've run the QEMU tests on arm(64)/x86(_64) without the yield or pause
and they still pass.  It might be simply unnecessary, so I'll remove for
now in favour of simplicity and less arch-specific code.

Best,

Asherah