From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=pass header.i=@semihalf-com.20150623.gappssmtp.com header.s=20150623 header.b=taF+pGqq; spf=none, err=SPF record not found (domain: semihalf.com, ip: 209.85.222.196, mailfrom: mw@semihalf.com) Received: from mail-qk1-f196.google.com (mail-qk1-f196.google.com [209.85.222.196]) by groups.io with SMTP; Mon, 08 Apr 2019 23:16:12 -0700 Received: by mail-qk1-f196.google.com with SMTP id g1so9553164qki.5 for ; Mon, 08 Apr 2019 23:16:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=zmQiZU8E5rHoyGvt2CVkK36flgRgqA2fFKXZ0KzErXg=; b=taF+pGqqqndo12Tt165+DR58G3SxazraQ/I85ckoliRTaYW3uPicR4gy2iQ3u5KKgY +po3zPaaBy8O3alhXjYZ5Z2BEZy5c19VPsn3XJsR85e2Df/2unJsyC89wXoibTbxuFz5 nNob8p2lA7+T6WNJtFfudpiLmvksO8Jkul5w2l2n9JwW9TmvkUpt4JjoZpfwHlL23pZv nna6FkSrNqj97NCPWeQ8RU2kYZrjKD6e6xMPeXnaLEGoWadsiZ3n9nTwLW7/x3KpNPKq NeNRLcwgwFjfiaqizLCGJP1+pQ6OYm/oBFopNDPhBcu6y2kFxXnUauIHfe9fZwqJGHiE aDfg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=zmQiZU8E5rHoyGvt2CVkK36flgRgqA2fFKXZ0KzErXg=; b=biayVWOozjATtK5bSoi8reerSRvuIrNEIs1sooD2k4ZinIywcIyFyQ6dH5yamKgkdf rY1H/AUURVb9jdCFfInQZZxcTtAp9zfbCmSaLIMgXlexoBUa47A9HV6iCIgmLZGE9orf XInu8hZ3O2rCZoq7eaTV2kdMGoTAuGUcwrRXoAmRB4wGLggiaG8KMaVVGgXR5WAnX7RJ OjfJxbkAFkr5iWX0hfHxIxLaHYw29Jn6oOJUKYrDlzSZyBj6BdpbaZwWBsX701xQZ51P hzcyuEoujcucV99Ubbg50yKQCK4jdOAjmRqIXi8gEYXM9qETp3K6fdyePc5nGtv7PWyI OFog== X-Gm-Message-State: APjAAAUrL6qKg0abaV7OEZ8xv1zCZtcjEt8UnLjXGOC03EpSg3MhxTIv hKNNhOJf3G0lkUqMhyMGw3Aluzec21cUtuPiUtjKoQ== X-Google-Smtp-Source: APXvYqyL3FrNiwjbwcwH2w7QQY9qaL8Rb4EHMMQ5NvhejgyX5ciOoQSDKCNuUimkwTKKi9WrX+RQs4w/6rKknHD1QYY= X-Received: by 2002:a37:a40d:: with SMTP id n13mr25338852qke.325.1554790571508; Mon, 08 Apr 2019 23:16:11 -0700 (PDT) MIME-Version: 1.0 References: <20190409003327.3797-1-jeremy.linton@arm.com> In-Reply-To: From: Marcin Wojtas Date: Tue, 9 Apr 2019 08:15:59 +0200 Message-ID: Subject: Re: [PATCH] Marvell/Armada7k8k: Remove SPCR baud rate setting To: Jeremy Linton Cc: devel@edk2.groups.io, Ard Biesheuvel , Leif Lindholm , "Kinney, Michael D" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Jeremy, wt., 9 kwi 2019 o 06:03 Jeremy Linton napisa=C5=82(= a): > > Hi Marcin, > > On 4/8/19 8:31 PM, Marcin Wojtas wrote: > > Hi Jeremy, > > > > 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 > boot. You shouldn't need any parameters at all with recent kernels (i've > been testing with mainline/5.1rc3/4) to get the console and boot log. > AKA no 'console=3D', earlyprintk, earlycon, etc line at all. > I took the v5.1-rc3 and indeed it now boots without any parameter. That's g= reat. > > To fix earlycon by itself (so you can see the boot log with just > 'earlycon') only the baud rate change in the subject line is needed. But > once I fixed that, I also wanted the console to work by default. That > requires both the MMIO access type and the address to match between SPCR > and the DSDT entry. Hence the long explanation about why I "broke" > earlycon by changing the access type from GAS32 to GAS8. There is an > existing "quirk" in the 8250_dw module for the armada parts for DT, and > I suspect a similar thing is needed for ACPI (this might be a longer > discussion elsewhere) because without it, I couldn't get a > GAS32/MMIO32/reg_offset=3D2 combination to work. Hence my comment about > changing the ACPI id for the console from the HISI identifier. > I always used earlycon with full parameter list, so I don't consider the change as very problematic :) You can add my: Tested-by: Marcin Wojtas Best regards, Marcin > > > > > > > Thanks, > > Marcin > > > > 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 > >> >