From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.101.70, mailfrom: jeremy.linton@arm.com) Received: from foss.arm.com (foss.arm.com [217.140.101.70]) by groups.io with SMTP; Mon, 08 Apr 2019 21:03:28 -0700 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id A281415AB; Mon, 8 Apr 2019 21:03:28 -0700 (PDT) Received: from [192.168.100.242] (usa-sjc-mx-foss1.foss.arm.com [217.140.101.70]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 5FEC63F59C; Mon, 8 Apr 2019 21:03:27 -0700 (PDT) Subject: Re: [PATCH] Marvell/Armada7k8k: Remove SPCR baud rate setting To: Marcin Wojtas Cc: devel@edk2.groups.io, Ard Biesheuvel , Leif Lindholm , "Kinney, Michael D" References: <20190409003327.3797-1-jeremy.linton@arm.com> From: "Jeremy Linton" Message-ID: Date: Sat, 6 Apr 2019 08:20:57 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: quoted-printable Hi Marcin, On 4/8/19 8:31 PM, Marcin Wojtas wrote: > Hi Jeremy, >=20 > Thanks for the patch. However I played with booting with ACPI with and > without your patch - I may be missing something, but I don't see a > difference, when using earlycon (both > 'earlycon=3Duart,mmio32,0xf0512000' and > 'earlycon=3Duart8250,mmio32,0xf0512000'). Can you please show what > booting scenario is fixed with the patch? Sorry, should have been more clear, this patch fixes normal ACPI console=20 boot. You shouldn't need any parameters at all with recent kernels (i've=20 been testing with mainline/5.1rc3/4) to get the console and boot log.=20 AKA no 'console=3D', earlyprintk, earlycon, etc line at all. To fix earlycon by itself (so you can see the boot log with just=20 'earlycon') only the baud rate change in the subject line is needed. But=20 once I fixed that, I also wanted the console to work by default. That=20 requires both the MMIO access type and the address to match between SPCR=20 and the DSDT entry. Hence the long explanation about why I "broke"=20 earlycon by changing the access type from GAS32 to GAS8. There is an=20 existing "quirk" in the 8250_dw module for the armada parts for DT, and=20 I suspect a similar thing is needed for ACPI (this might be a longer=20 discussion elsewhere) because without it, I couldn't get a=20 GAS32/MMIO32/reg_offset=3D2 combination to work. Hence my comment about=20 changing the ACPI id for the console from the HISI identifier. >=20 > Thanks, > Marcin >=20 > wt., 9 kwi 2019 o 02:33 Jeremy Linton napisa=C5= =82(a): >> >> The mcbin (and likely others) have a nonstandard uart clock. >> This means that the earlycon programming will incorrectly set >> the baud rate if it is specified. The way around this is to tell >> the kernel to continue using the preprogrammed baud rate. This >> is done by setting the baud to 0. >> >> Further, the SPCR and DSDT serial port need to match the port >> address and port access type for the kernel to conclude they >> are the same. >> >> So while ARM_GAS32 is correct for earlycon (it can be used alone >> on the kernel command line) by providing the reg-shift=3D2 value, >> it also sets the io type to MMIO32, which doesn't match the DSDT >> defined MMIO. This means that the actual console will never appear. >> The obvious fix is to set reg-width=3D4 in DSDT, but that also changes >> the accesssors to 32-bits (similarly to earlycon) and results in >> console failure. >> >> So the less obvious fix, is to use the GAS8 specifier. This means >> that earlycon needs to be fully specified as >> earlycon=3Duart,mmio32,0xf0512000, but has the extremely useful featur= e >> that the console default works without any user interaction. >> >> If in the future marvell decides to define their own ACPI id for the >> console and upstream a quirk, the ARM_GAS8 portion of this should >> be reverted. >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Jeremy Linton >> --- >> Silicon/Marvell/Armada7k8k/AcpiTables/Spcr.aslc | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/Silicon/Marvell/Armada7k8k/AcpiTables/Spcr.aslc b/Silicon= /Marvell/Armada7k8k/AcpiTables/Spcr.aslc >> index e78bb9036f..06c7af069c 100644 >> --- a/Silicon/Marvell/Armada7k8k/AcpiTables/Spcr.aslc >> +++ b/Silicon/Marvell/Armada7k8k/AcpiTables/Spcr.aslc >> @@ -30,11 +30,11 @@ EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spc= r =3D { >> { EFI_ACPI_RESERVED_BYTE, >> EFI_ACPI_RESERVED_BYTE, >> EFI_ACPI_RESERVED_BYTE }, = // Reserved1[3] >> - ARM_GAS32 (FixedPcdGet64(PcdSerialRegisterBase)), = // BaseAddress >> + ARM_GAS8 (FixedPcdGet64(PcdSerialRegisterBase)), = // BaseAddress >> EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERRUPT_TYPE_GIC,= // InterruptType >> 0, = // Irq >> 51, = // GlobalSystemInterrupt >> - EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_115200, = // BaudRate >> + 0, = // Keep Firmware Baud >> EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_PARITY_NO_PARITY, = // Parity >> EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_STOP_BITS_1, = // StopBits >> 0, = // FlowControl >> -- >> 2.20.1 >>