From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f67.google.com (mail-wr1-f67.google.com [209.85.221.67]) by mx.groups.io with SMTP id smtpd.web11.13346.1576689006557280205 for ; Wed, 18 Dec 2019 09:10:07 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@akeo-ie.20150623.gappssmtp.com header.s=20150623 header.b=ne+5gFEv; spf=none, err=permanent DNS error (domain: akeo.ie, ip: 209.85.221.67, mailfrom: pete@akeo.ie) Received: by mail-wr1-f67.google.com with SMTP id c9so3107818wrw.8 for ; Wed, 18 Dec 2019 09:10:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=akeo-ie.20150623.gappssmtp.com; s=20150623; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=dTf4GSwCYk8nxF1fwZOCeIuQcj0kOSk5qdHOETVadK8=; b=ne+5gFEvS1BhZIKR1lrHVR9w3YdTtiujCaz7dzAgY3XvP38Cww6g0F3w1Cvr0JqDTb euE0dRoYz3Piksxnh8KE7UKIC1tXdvkOIj3GuXtMUhQePBV6O3eQ++FhPvJ6hqNURtO0 IgSFkuIcLEZ1X72m7qPkLcywfUUoMxRHq1npdZChpYWpFZ9RMxmTLLvTC5uTCfxoSk+j KNU/tveW6MIPZgsiL9vz4g7sUm7UIbEV1TN/pWShiXnWtJB+kvggxogbyIwu1CYyGn3D RqgeNA0Z64vpwL2i8OS4+G18NmCcwujGhZmt3Bf+4kwtThePGCPg336QkItHx/cagtD0 ozyA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=dTf4GSwCYk8nxF1fwZOCeIuQcj0kOSk5qdHOETVadK8=; b=Rxg1Qmmr8gCU/0StnYdiCfbRN5+H6TvO05+Z29NjiXzjKN8+n2fDltRLTs9FR1nkVf yhqNBGgCrqPnO2BWw4FDya9tgKOTE1Dev7tS5BQT9PQpO+I0TzmDxmtGi9ZBb3zN1lAq oYcnE5I6wzUKgOVzBIzehQcYzj6/Bh4+NxRiyTr2deB+KtYo+HdI9Is5sMrlBYVKKyCl WZIvGDaOjElux8pwvycgly1M7BxTQcMCFM0vpk7gF7Ux5pb02zkxAFbSIjPVdkRrRZM9 tr9ifSyC4ibMJ9MRMuMSlSs32kahsY0DjEXbFVaekFaDXyIbHJAndg7lTd//x99YlXtN U6Iw== X-Gm-Message-State: APjAAAVWdSQEGdnfY755aBsYnBUvs47iR7U20Cm1hz5yj2gTDMCDExhg xtcBpf9nB8565sJUxYBbmf+ANA== X-Google-Smtp-Source: APXvYqxUoPEeEeHbpDAnpN7dlfd+P+BeGtvhXMyIX0fUP6MlHKyI7t7YvorubykMYtDI1gIJ0HegrQ== X-Received: by 2002:a5d:6ac5:: with SMTP id u5mr3990134wrw.271.1576689004953; Wed, 18 Dec 2019 09:10:04 -0800 (PST) Return-Path: Received: from [10.0.0.122] ([84.203.77.210]) by smtp.googlemail.com with ESMTPSA id s16sm3226406wrn.78.2019.12.18.09.10.03 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 18 Dec 2019 09:10:03 -0800 (PST) Subject: Re: [edk2-platforms][PATCH 3/6] Platform/RPi4: Improve SPCR and DBG2 ACPI table generation To: =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= , devel@edk2.groups.io Cc: ard.biesheuvel@linaro.org, leif.lindholm@linaro.org References: <20191218114156.9036-1-pete@akeo.ie> <20191218114156.9036-4-pete@akeo.ie> <1771aae3-dee9-ed27-5134-962434db5f57@redhat.com> From: "Pete Batard" Message-ID: Date: Wed, 18 Dec 2019 17:10:02 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.9.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 8bit On 2019.12.18 17:00, Philippe Mathieu-Daudé wrote: > On 12/18/19 5:36 PM, Pete Batard wrote: >> Hi Philippe, >> >> On 2019.12.18 15:57, Philippe Mathieu-Daudé wrote: >>> On 12/18/19 12:41 PM, Pete Batard wrote: >>>> Use code derived from JunoPkg to generate our serial tables and >>>> also use PCDs where possible. >>>> >>>> Signed-off-by: Pete Batard >>>> --- >>>>   Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf |   4 +- >>>>   Platform/RaspberryPi/RPi4/AcpiTables/Dbg2.aslc      | 100 >>>> +++++++++++++++++--- >>>>   Platform/RaspberryPi/RPi4/AcpiTables/Spcr.asl       |  48 ---------- >>>>   Platform/RaspberryPi/RPi4/AcpiTables/Spcr.aslc      |  94 >>>> ++++++++++++++++++ >>>>   4 files changed, 183 insertions(+), 63 deletions(-) >>>> >>>> diff --git a/Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf >>>> b/Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf >>>> index 50c9f7694d84..6b1155d66444 100644 >>>> --- a/Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf >>>> +++ b/Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf >>>> @@ -31,7 +31,7 @@ [Sources] >>>>     Gtdt.aslc >>>>     Dsdt.asl >>>>     Csrt.aslc >>>> -  Spcr.asl >>>> +  Spcr.aslc >>>>   [Packages] >>>>     ArmPkg/ArmPkg.dec >>>> @@ -47,4 +47,6 @@ [FixedPcd] >>>>     gArmTokenSpaceGuid.PcdArmArchTimerVirtIntrNum >>>>     gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase >>>>     gArmTokenSpaceGuid.PcdGicDistributorBase >>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase >>>> +  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate >>>>     gEmbeddedTokenSpaceGuid.PcdInterruptBaseAddress >>>> diff --git a/Platform/RaspberryPi/RPi4/AcpiTables/Dbg2.aslc >>>> b/Platform/RaspberryPi/RPi4/AcpiTables/Dbg2.aslc >>>> index 849cf5134793..589a5c2cbd71 100644 >>>> --- a/Platform/RaspberryPi/RPi4/AcpiTables/Dbg2.aslc >>>> +++ b/Platform/RaspberryPi/RPi4/AcpiTables/Dbg2.aslc >>>> @@ -2,27 +2,99 @@ >>>>    * >>>>    *  Debug Port Table (DBG2) >>>>    * >>>> - *  Copyright (c) Microsoft Corporation. All rights reserved. >>>> + *  Copyright (c) 2019, Pete Batard >>>> + *  Copyright (c) 2012-2016, ARM Limited. All rights reserved. >>>>    * >>>>    *  SPDX-License-Identifier: BSD-2-Clause-Patent >>>>    * >>>>    **/ >>>> -UINT8 Dbg2[92] = { >>>> -  0x44, 0x42, 0x47, 0x32, 0x5C, 0x00, 0x00, 0x00, 0x00, 0x00, >>>> -  0x4D, 0x53, 0x46, 0x54, 0x20, 0x20, 0x45, 0x44, 0x4B, 0x32, >>>> -  0x20, 0x20, 0x20, 0x20, 0x01, 0x00, 0x00, 0x00, 0x4D, 0x53, >>>> -  0x46, 0x54, 0x01, 0x00, 0x00, 0x00, 0x2C, 0x00, 0x00, 0x00, >>>> -  0x01, 0x00, 0x00, 0x00, 0x00, 0x30, 0x00, 0x01, 0x0A, 0x00, >>>> -  0x26, 0x00, 0x00, 0x00, 0x30, 0x00, 0x00, 0x80, 0x10, 0x00, >>>> -  0x00, 0x00, 0x16, 0x00, 0x22, 0x00, 0x00, 0x20, 0x00, 0x10, >>>> -  0x00, 0x50, 0x21, 0xFE, 0x00, 0x00, 0x00, 0x00, 0x6C, 0x00, >>>> -  0x00, 0x00, '\\',  '_',  'S',  'B',  '.',  'U',  'R',  'T', >>>> -  'M', 0x00, >>>> +#include >>>> +#include >>>> +#include >>>> +#include >>>> + >>>> +#include "AcpiTables.h" >>>> + >>>> +#pragma pack(1) >>>> + >>>> +#define RPI_DBG2_NUM_DEBUG_PORTS                        1 >>>> +#define RPI_DBG2_NUMBER_OF_GENERIC_ADDRESS_REGISTERS    1 >>>> +#define RPI_DBG2_NAMESPACESTRING_FIELD_SIZE             10 >>>> + >>>> +// >>>> +// When using the miniUART, PcdSerialRegisterBase points to the >>>> 8250 base address, >>>> +// which is offset by 0x40 from the actual Bcm2835 base address >>> >>> Actually this is the other way around, the 8250 is at AUX + 0x40. >>> >>> Can we clean that up or is it too late? >> >> Yeah, re-reading this I admit the comment is not too clear. >> >> Let me try to rephrase this better in v2. > > Actually my comment is about the code... Do you think we can clean the > RPi3/RPi4 codebase by changing > gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase? > > If you don't have time for this, maybe you can add a /* TODO FIXME */? Well, I'm planning to go with FixedPcdGet64 (PcdBcm283xRegistersAddress) + BCM2836_MINI_UART_OFFSET below in v2. So unless I'm missing something, I think this solves the point you raised and will also remove the need for the comment. /Pete > >>> >>>> +// >>>> +#define RPI_UART_BASE_ADDRESS (FixedPcdGet64 >>>> (PcdSerialRegisterBase) - 0x40) >>>> +#define RPI_UART_LENGTH                                 0x70 >>>> +// >>>> +// RPI_UART_STR should match the value used Uart.asl >>>> +// >>>> +#define RPI_UART_STR                                    { '\\', >>>> '_', 'S', 'B', '.', 'U', 'R', 'T', 'M', 0x00 } >>>> + >>>> +typedef struct { >>>> +  EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT         Dbg2Device; >>>> +  EFI_ACPI_5_1_GENERIC_ADDRESS_STRUCTURE BaseAddressRegister; >>>> +  UINT32                                                AddressSize; >>>> +  UINT8 NameSpaceString[RPI_DBG2_NAMESPACESTRING_FIELD_SIZE]; >>>> +} DBG2_DEBUG_DEVICE_INFORMATION; >>>> + >>>> +typedef struct { >>>> +  EFI_ACPI_DEBUG_PORT_2_DESCRIPTION_TABLE               Description; >>>> +  DBG2_DEBUG_DEVICE_INFORMATION >>>> Dbg2DeviceInfo[RPI_DBG2_NUM_DEBUG_PORTS]; >>>> +} DBG2_TABLE; >>>> + >>>> + >>>> +#define DBG2_DEBUG_PORT_DDI(NumReg, SubType, UartBase, UartAddrLen, >>>> UartNameStr) {                                    \ >>>> + { \ >>>> +      EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION, /* >>>> UINT8     Revision */                        \ >>>> +      sizeof (DBG2_DEBUG_DEVICE_INFORMATION), /* UINT16    Length >>>> */                          \ >>>> +      NumReg, /* UINT8     NumberofGenericAddressRegisters */ \ >>>> +      RPI_DBG2_NAMESPACESTRING_FIELD_SIZE, /* UINT16 >>>> NameSpaceStringLength */           \ >>>> +      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, NameSpaceString), >>>> /* UINT16    NameSpaceStringOffset */           \ >>>> +      0, /* UINT16    OemDataLength */                   \ >>>> +      0, /* UINT16    OemDataOffset */                   \ >>>> +      EFI_ACPI_DBG2_PORT_TYPE_SERIAL, /* UINT16    Port Type >>>> */                       \ >>>> +      SubType, /* UINT16    Port Subtype */                    \ >>>> +      {EFI_ACPI_RESERVED_BYTE, EFI_ACPI_RESERVED_BYTE}, /* UINT8 >>>> Reserved[2] */                     \ >>>> +      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, >>>> BaseAddressRegister), /* UINT16    BaseAddressRegister Offset */      \ >>>> +      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, AddressSize) /* >>>> UINT16    AddressSize Offset */              \ >>>> + }, \ >>>> +    ARM_GAS32 (UartBase),                            /* >>>> EFI_ACPI_5_1_GENERIC_ADDRESS_STRUCTURE BaseAddressRegister */ \ >>>> +    UartAddrLen,                                     /* UINT32 >>>> AddressSize */                                        \ >>>> +    UartNameStr                                      /* UINT8 >>>> NameSpaceString[MAX_DBG2_NAME_LEN] */                 \ >>>> +  } >>>> + >>>> + >>>> +STATIC DBG2_TABLE Dbg2 = { >>>> +  { >>>> +    ACPI_HEADER ( >>>> +      EFI_ACPI_5_1_DEBUG_PORT_2_TABLE_SIGNATURE, >>>> +      DBG2_TABLE, >>>> +      EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION >>>> +    ), >>>> +    OFFSET_OF (DBG2_TABLE, Dbg2DeviceInfo), >>>> +    RPI_DBG2_NUM_DEBUG_PORTS /* UINT32  NumberDbgDeviceInfo */ >>>> +  }, >>>> +  { >>>> +    /* >>>> +     * Kernel Debug Port >>>> +     */ >>>> +    DBG2_DEBUG_PORT_DDI ( >>>> +      RPI_DBG2_NUMBER_OF_GENERIC_ADDRESS_REGISTERS, >>>> +      EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_BCM2835_UART, >>>> +      RPI_UART_BASE_ADDRESS, >>>> +      RPI_UART_LENGTH, >>>> +      RPI_UART_STR >>>> +    ), >>>> +  } >>>>   }; >>> [...] >>> >> >