Re: [PATCH v2 4/7] qfw: Add flag to allow probing before relocation
On Mon, Aug 28, 2023 at 06:22:31PM +0300, Alper Nebi Yasak wrote:
> On 2023-08-22 21:56 +03:00, Simon Glass wrote:
> > Hi Alper,
> >
> > On Tue, 22 Aug 2023 at 06:10, Alper Nebi Yasak
> > 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
> >> ---
> >> 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 @1010 instead on riscv64):
>
> fw-cfg@902 {
> dma-coherent;
> reg = <0x00 0x902 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,*?)
This seems like a whole lot of work to avoid using the flags mechanism
that we have, intentionally.
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH v2 4/7] qfw: Add flag to allow probing before relocation
On 2023-08-22 21:56 +03:00, Simon Glass wrote:
> Hi Alper,
>
> On Tue, 22 Aug 2023 at 06:10, Alper Nebi Yasak
> 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
>> ---
>> 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 @1010 instead on riscv64):
fw-cfg@902 {
dma-coherent;
reg = <0x00 0x902 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
Re: [PATCH v2 4/7] qfw: Add flag to allow probing before relocation
Hi Alper,
On Tue, 22 Aug 2023 at 06:10, Alper Nebi Yasak 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
> ---
> 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?
In the case where it isn't present it can return -EPERM.
Regards,
Simon
