From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 3178F941CB3 for ; Wed, 11 Oct 2023 21:16:08 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=nJPawBDoQUdB3JrOJpdNcMPjOltnt3i2G+0X1SoeeDY=; c=relaxed/simple; d=groups.io; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject:To:Cc:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type:Content-Transfer-Encoding; s=20140610; t=1697058966; v=1; b=VaMsx0dfrpY1UHnOAc6e/5l1tUZIjwdKM+ju58SZl7B3JD3ZHBdNJBufxIpUWqgXsDi0YlPx UK1120CP+inPBJrirzbcwR2hYltIfN5wdFjll5pIkwKBRUKdnfsLeRapEEAX/xrN3clG1w68mca AEh70/+SqF1JBymyxWifQzLI= X-Received: by 127.0.0.2 with SMTP id YIikYY7687511xW4ZCufbBFw; Wed, 11 Oct 2023 14:16:06 -0700 X-Received: from mail-vs1-f51.google.com (mail-vs1-f51.google.com [209.85.217.51]) by mx.groups.io with SMTP id smtpd.web10.28396.1697058966123327459 for ; Wed, 11 Oct 2023 14:16:06 -0700 X-Received: by mail-vs1-f51.google.com with SMTP id ada2fe7eead31-45766b79b14so141864137.1 for ; Wed, 11 Oct 2023 14:16:05 -0700 (PDT) X-Gm-Message-State: jeHqjkurGff8T65xuTiySB4Ex7686176AA= X-Google-Smtp-Source: AGHT+IEiz63gmG/QlI6f637nvg+npJhAPsOGAJdXo8yJExQ0fP6fd+y3uAO4NZDgLXepNiSPGdVP1cZnB5zOdwVywbk= X-Received: by 2002:a67:fa96:0:b0:452:d87d:81e7 with SMTP id f22-20020a67fa96000000b00452d87d81e7mr20459272vsq.9.1697058965098; Wed, 11 Oct 2023 14:16:05 -0700 (PDT) MIME-Version: 1.0 References: <9143994e6a5f37650930abfc4141c6061e5bef98.1696448945.git.matt.devillier@gmail.com> In-Reply-To: From: "Pedro Falcato" Date: Wed, 11 Oct 2023 22:15:53 +0100 Message-ID: Subject: Re: [edk2-devel] [PATCH 1/3] MdeModulePkg: Add ClockRate field to SerialPortInfo To: Matt DeVillier Cc: devel@edk2.groups.io, Zhiguang Liu , "Ni, Ray" , gua.guo@intel.com Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,pedro.falcato@gmail.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=VaMsx0df; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=gmail.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io On Mon, Oct 9, 2023 at 4:54=E2=80=AFPM Matt DeVillier wrote: > > On Thu, Oct 5, 2023 at 12:45=E2=80=AFPM Pedro Falcato wrote: >> >> On Wed, Oct 4, 2023 at 9:01=E2=80=AFPM MrChromebox wrote: >> > >> > Add the ClockRate field to the UNIVERSAL_PAYLOAD_SERIAL_PORT_INFO >> > struct, so that the field can be used by UefiPayloadPkg to properly >> > set up the serial port on boards using a non-standard clock rate. >> > >> > Signed-off-by: Matt DeVillier >> > Change-Id: I9bcaf03ab63f6a45d2cf25a580f7a2eba388cbbd >> >> Series-generic-feedback: Remove Change-Id lines and CC the proper >> maintainers for each patch > > > ack > >> >> > --- >> > MdeModulePkg/Include/UniversalPayload/SerialPortInfo.h | 1 + >> > 1 file changed, 1 insertion(+) >> > >> > diff --git a/MdeModulePkg/Include/UniversalPayload/SerialPortInfo.h b/= MdeModulePkg/Include/UniversalPayload/SerialPortInfo.h >> > index 3c4459e2c0..e3c9f93654 100644 >> > --- a/MdeModulePkg/Include/UniversalPayload/SerialPortInfo.h >> > +++ b/MdeModulePkg/Include/UniversalPayload/SerialPortInfo.h >> > @@ -19,6 +19,7 @@ typedef struct { >> > BOOLEAN UseMmio; >> > UINT8 RegisterStride; >> > UINT32 BaudRate; >> > + UINT32 ClockRate; >> >> I don't think you can do this? UNIVERSAL_PAYLOAD_SERIAL_PORT_INFO is >> part of a spec (https://universalscalablefirmware.github.io/documentatio= n/2_universal_payload.html) >> and it doesn't even seem to be versioned, so you'd just break ABI. Am >> I missing something? > > > The USF spec says that it is currently at version 0.7 and under developme= nt / not final. I don't see why adding a field to a hob is problematic prov= ided it's properly documented. The alternatives are some really hacky worka= rounds to pass the necessary data, or the serial port not working. My 2 cents, as someone that neither maintains nor works on this code: 1) This sounds like something that needs to be discussed with the spec people (good luck! lol) 2) Changing the structure abruptly like this seems like it would break many current users. HOWEVER: The spec (https://universalscalablefirmware.github.io/documentation/2_unive= rsal_payload.html#common-payload-header) says that the common header's Revision (indeed, it's versioned and I missed that) should be incremented if current members are renamed or reinterpreted, but not if you just append members. So it seems that if you add ClockRate like this: --- a/MdeModulePkg/Include/UniversalPayload/SerialPortInfo.h +++ b/MdeModulePkg/Include/UniversalPayload/SerialPortInfo.h @@ -19,6 +19,7 @@ typedef struct { BOOLEAN UseMmio; UINT8 RegisterStride; UINT32 BaudRate; EFI_PHYSICAL_ADDRESS RegisterBase; + UINT32 ClockRate; } UNIVERSAL_PAYLOAD_SERIAL_PORT_INFO; -- this should be acceptable for the spec people and EDK2/coreboot/SlimBootloader/etc people, given all the current consuming code takes the generic header's Length into account (as specified by the spec). --=20 Pedro -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#109534): https://edk2.groups.io/g/devel/message/109534 Mute This Topic: https://groups.io/mt/101763374/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-