Re: [PATCH v2 4/7] qfw: Add flag to allow probing before relocation

[Alper Nebi Yasak <alpernebiyasak@gmail.com>]

On 2023-08-22 21:56 +03:00, Simon Glass wrote:
> Hi Alper,
> 
> On Tue, 22 Aug 2023 at 06:10, Alper Nebi Yasak <alpernebiyasak@gmail.com> wrote:
>>
>> QEMU firmware config drivers need to be probed to bind the ramfb device.
>> The ramfb driver needs to be bound before relocation to properly reserve
>> video memory for it, otherwise it cannot be probed after relocation. Add
>> the flag to probe QEMU firmware config drivers before relocation so that
>> ramfb can work as an initial vidconsole.
>>
>> Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
>> ---
>> Alternatively, I guess I could default VIDEO_PCI_DEFAULT_FB_SIZE to a
>> higher size with "if VIDEO_RAMFB". But it exists because "PCI drivers
>> cannot be bound before relocation unless they are mentioned in the
>> devicetree" and qfw is in the QEMU-generated devicetree unlike those, so
>> I assumed this would be the preferred way.
>>
>> Changes in v2:
>> - Add patch "qfw: Add flag to allow probing before relocation"
>>
>>  drivers/misc/qfw.c         | 1 +
>>  drivers/misc/qfw_mmio.c    | 1 +
>>  drivers/misc/qfw_pio.c     | 1 +
>>  drivers/misc/qfw_sandbox.c | 1 +
>>  4 files changed, 4 insertions(+)
>>
>> diff --git a/drivers/misc/qfw.c b/drivers/misc/qfw.c
>> index 4e4260982cce..265f45290011 100644
>> --- a/drivers/misc/qfw.c
>> +++ b/drivers/misc/qfw.c
>> @@ -414,6 +414,7 @@ UCLASS_DRIVER(qfw) = {
>>         .name           = "qfw",
>>         .post_bind      = qfw_post_bind,
>>         .per_device_auto        = sizeof(struct qfw_dev),
>> +       .flags = DM_FLAG_PRE_RELOC,
>>  };
> 
> Should we add this to the DT instead?

I've experimented a bit, and got something that works nice on x86
because we have the device-trees in-tree. So I can add something to
arch/x86/dts/qemu-*.dts like:

    / {
        fw-cfg {
            compatible = "qemu,fw-cfg-pio";
            bootph-some-ram;

            qfw-ramfb {
                compatible = "qemu,qfw-ramfb";
                bootph-some-ram;
            };
        };
    };

Then add the compatibles as .of_match to qfw_pio and ramfb drivers, call
dm_scan_fdt_dev(dev) in qfw_post_bind(), merge qfw_bind_ramfb() into the
ramfb driver probe, and it will work without the board_early_init_[fr]
hooks or these flags. That all seems to be fine.

The problem is other arches where QEMU generates a device-tree at
runtime and passes it to U-Boot with OF_BOARD. It has, on arm64 (and
with @10100000 instead on riscv64):

    fw-cfg@9020000 {
        dma-coherent;
        reg = <0x00 0x9020000 0x00 0x18>;
        compatible = "qemu,fw-cfg-mmio";
    };

As a proof of concept, I used `qemu-system-* -M dumpdtb=file.dtb` and
`dtc -I dtb -O dts` to replace the empty qemu dts files, then set
OF_BOARD=n and OF_OMIT_DTB=n, and added similar modifications to
qemu-*-u-boot.dtsi. And everything seems fine like that as well.

I think it can be automated in Makefile, but I don't think disabling
OF_BOARD is right here. I see there's OF_BOARD_FIXUP, so I guess I'll
try finding the fw-cfg node there and adding the bootph props and ramfb
nodes... Is this going the right way?

(And the pio/ramfb compatibles don't exist, do we have to use u-boot,*?)

> In the case where it isn't present it can return -EPERM.
> 
> Regards,
> Simon