On 1/24/19 12:37 PM, Andrew Jones wrote:
> On Thu, Jan 24, 2019 at 11:59:43AM +0000, Andre Przywara wrote:
>> On Thu, 24 Jan 2019 11:16:29 +0000
>> Alexandru Elisei <alexandru.elisei@xxxxxxx> wrote:
>>
>>> A warning is displayed if uart0_base is different from what the code
>>> expects qemu to generate for the pl011 UART in the device tree.
>>> However, now we support the ns16550a UART emulated by kvmtool, which
>>> has a different address. This leads to the warning being displayed
>>> even though the UART is configured and working as expected.
>>>
>>> Now that we support multiple UARTs, the warning serves no purpose, so
>>> remove it.
>> Mmh, but we use that address before, right? So for anything not
>> emulating an UART at this QEMU specific address we write to some random
>> (device) memory?
>>
>> Drew, how important is this early print feature for kvm-unit-tests?
> The setup code passes through quite a few asserts before getting through
> io_init() (including in uart0_init), so I think there's still value in
> having a guessed UART address. Maybe we can provide guesses for both
> QEMU and kvmtool, and some selection method, that would be used until
> we've properly assigned uart0_base from DT?
>
>> Cheers,
>> Andre.
>>
>>> Signed-off-by: Alexandru Elisei <alexandru.elisei@xxxxxxx>
>>> ---
>>> lib/arm/io.c | 6 ------
>>> 1 file changed, 6 deletions(-)
>>>
>>> diff --git a/lib/arm/io.c b/lib/arm/io.c
>>> index 35fc05aeb4db..87435150f73e 100644
>>> --- a/lib/arm/io.c
>>> +++ b/lib/arm/io.c
>>> @@ -61,12 +61,6 @@ static void uart0_init(void)
>>> }
>>>
>>> uart0_base = ioremap(base.addr, base.size);
>>> -
>>> - if (uart0_base != (u8 *)UART_EARLY_BASE) {
>>> - printf("WARNING: early print support may not work. "
>>> - "Found uart at %p, but early base is %p.\n",
>>> - uart0_base, (u8 *)UART_EARLY_BASE);
>>> - }
>>> }
> This warning is doing what it should, which is pointing out that the
> UART_EARLY_BASE guess appears to be wrong. If we can provide a way
> to support more than one guess, then we should keep this warning but
> adjust it to match one of any of the guesses.
>
> Thanks,
> drew
I'm not really sure how to implement a selection method. I've looked at
splitting io_init() into uart0_init() and chr_testdev_init() and calling
uart0_init() very early in the setup process, but uart0_init() itself uses
printf() and assert().
I've also thought about adding another function, something like
uart0_early_init(), that is called very early in setup() and gets the base
address from the dtb bootargs. But that means calling dt_init() and
dt_get_bootargs(), which can fail.
One other option that could work is to make it a compile-time configuration.
What do you think?
