From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f45.google.com (mail-wr1-f45.google.com [209.85.221.45]) by mx.groups.io with SMTP id smtpd.web08.7180.1623242463658838412 for ; Wed, 09 Jun 2021 05:41:04 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=L0vCw7rs; spf=pass (domain: nuviainc.com, ip: 209.85.221.45, mailfrom: leif@nuviainc.com) Received: by mail-wr1-f45.google.com with SMTP id i94so20280818wri.4 for ; Wed, 09 Jun 2021 05:41:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuviainc-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to; bh=CTAwKq3l/YLGhzgJV4MrQpUZWihV7IPzz8D6TvVEX3o=; b=L0vCw7rs6i4kSs5qLo5Qi1zy1EGAVcddEGZ073rDQ9Bjqpu97EdJA9eGotmCIAhCOf Zxq4zbz40QmDte8ReRYodY3qZuYGIQcwhHMA4pOpMF/8ZlNNUfT3Lr0SsPOK8lk+csuH eSmT23cmXsHAnQHW5xLuktDkcCCfaSa7oSqhQXv5ncqD5RAgtzsONS5E/PR79aICt5Fw fd96nwbyjp4p4OSleZ67V7H++a2AsixAGzlSItE1R38lTktjip8nENwCzbp7xojDpK8S CGjnq0cm6uftIXnu0uHFgLdjNZyLbAhoeg3n0cDOeIHydR+rcdm9DXcc3EwYanTuXCBX dQxg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to; bh=CTAwKq3l/YLGhzgJV4MrQpUZWihV7IPzz8D6TvVEX3o=; b=h343xL6sOd0RxOVtUT2wqBqKH9LML3MrdFwpGcYBT0ytdGjAt6/FwwHKGzgvA28IsX bQT2k96m28aH+fAe1H1FCMUoHkXHIlPNo6ZDHlNapzDfNRZdqZ7YmX0LtoD2HCkVG+th nsN66UwK01ylz2gZ5yAlAjur6uS8WmyLC0pZRSq32FNlqcig5lvTv+or1wqP9DxD4xUU 1gMUBR0mjhjHOecDzPTnkUgYH3qjSxhpd5y/whZ8l6agUlQ6ztbmdQlkD5U69Cado9XD AhG+F5nBifJvOe0qNFZ63tfyrU85UsGsvxr8aJsXOSo/Bwg1ePnVGNtypv36qYLmHh/S 0EQw== X-Gm-Message-State: AOAM5330UU7XFL8d4Ea6v29FCZY0KpxJfJlaRfmNpSCFhnBK2r0BO6mY SNFMjFd0+gBGsFvLDizbEA+syw== X-Google-Smtp-Source: ABdhPJx6aZqWpf1hxKFsvU/mtbOZDHCx8DNdcS0P1FmJsvazxKDCryJeJ10SVPzWyfjtKp6d1+VOfA== X-Received: by 2002:adf:bc06:: with SMTP id s6mr29109356wrg.250.1623242462120; Wed, 09 Jun 2021 05:41:02 -0700 (PDT) Return-Path: Received: from leviathan (cpc1-cmbg19-2-0-cust915.5-4.cable.virginm.net. [82.27.183.148]) by smtp.gmail.com with ESMTPSA id y26sm14052370wma.33.2021.06.09.05.41.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 09 Jun 2021 05:41:01 -0700 (PDT) Date: Wed, 9 Jun 2021 13:40:59 +0100 From: "Leif Lindholm" To: Nhi Pham Cc: devel@edk2.groups.io, Vu Nguyen , Thang Nguyen , Chuong Tran , Phong Vo , Michael D Kinney , Ard Biesheuvel , Nate DeSimone Subject: Re: [edk2-platforms][PATCH v2 01/32] Ampere: Initial support for Ampere Altra processor and Mt. Jade platform Message-ID: <20210609124059.ltid25xfj6xcwyck@leviathan> References: <20210526100724.5359-1-nhi@os.amperecomputing.com> <20210526100724.5359-2-nhi@os.amperecomputing.com> <20210604230410.s7vuarxir2fqkqtw@leviathan> <8f7dde81-8337-9b9b-3b80-f575fad568fb@os.amperecomputing.com> MIME-Version: 1.0 In-Reply-To: <8f7dde81-8337-9b9b-3b80-f575fad568fb@os.amperecomputing.com> Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit On Wed, Jun 09, 2021 at 11:50:07 +0700, Nhi Pham wrote: > > > diff --git a/Silicon/Ampere/AmpereAltraPkg/Library/MailboxInterfaceLib/MailboxInterfaceLib.c b/Silicon/Ampere/AmpereAltraPkg/Library/MailboxInterfaceLib/MailboxInterfaceLib.c > > > new file mode 100644 > > > index 000000000000..0da1f90699f6 > > > --- /dev/null > > > +++ b/Silicon/Ampere/AmpereAltraPkg/Library/MailboxInterfaceLib/MailboxInterfaceLib.c > > > @@ -0,0 +1,282 @@ > > > +/** @file > > > + The library implements the hardware Mailbox (Doorbell) interface for communication > > > + between the Application Processor (ARMv8) and the System Control Processors (SMpro/PMpro). > > > + > > > + Copyright (c) 2021, Ampere Computing LLC. All rights reserved.
> > > + > > > + SPDX-License-Identifier: BSD-2-Clause-Patent > > > + > > > +**/ > > > + > > > +#include > > > + > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > + > > > +// > > > +// Hardware Doorbells > > > +// > > > +#define SMPRO_DB0_IRQ_OFST 40 > > > +#define SMPRO_DB0_BASE_ADDRESS (FixedPcdGet64 (PcdSmproDbBaseReg)) > > > + > > > +#define PMPRO_DB0_IRQ_OFST 56 > > > +#define PMPRO_DB0_BASE_ADDRESS (FixedPcdGet64 (PcdPmproDbBaseReg)) > > > + > > > +#define SLAVE_SOCKET_BASE_ADDRESS_OFFSET 0x400000000000 > > > + > > > +// > > > +// The base SPI interrupt number of the Slave socket > > > +// > > > +#define SLAVE_SOCKET_SPI_INTERRUPT 352 > > > + > > > +#define SLAVE_SOCKET_DOORBELL_INTERRUPT_BASE(Socket) ((Socket) * SLAVE_SOCKET_SPI_INTERRUPT - 32) > > > + > > > +// > > > +// Doorbell base register stride size > > > +// > > > +#define DB_BASE_REG_STRIDE 0x00001000 > > > + > > > +#define SMPRO_DBx_ADDRESS(socket, db) \ > > > + ((socket) * SLAVE_SOCKET_BASE_ADDRESS_OFFSET + SMPRO_DB0_BASE_ADDRESS + DB_BASE_REG_STRIDE * (db)) > > > + > > > +#define PMPRO_DBx_ADDRESS(socket, db) \ > > > + ((socket) * SLAVE_SOCKET_BASE_ADDRESS_OFFSET + PMPRO_DB0_BASE_ADDRESS + DB_BASE_REG_STRIDE * (db)) > > > + > > > +// > > > +// Doorbell Status Bits > > > +// > > > +#define DB_STATUS_AVAIL_BIT BIT16 > > > +#define DB_STATUS_ACK_BIT BIT0 > > > + > > > +/** > > > + Get the base address of a doorbell. > > > + > > > + @param[in] Socket Active socket index. > > > + @param[in] Doorbell Doorbell channel for communication with the SMpro/PMpro. > > > + > > > + @retval UINT32 The base address of the doorbell. > > > + The returned value is 0 indicate that the input parameters are invalid. > > > + > > > +**/ > > > +UINTN > > > +EFIAPI > > > +MailboxGetDoorbellAddress ( > > > + IN UINT8 Socket, > > > + IN DOORBELL_CHANNELS Doorbell > > > + ) > > > +{ > > > + UINTN DoorbellAddress; > > > + > > > + if (Socket >= GetNumberOfActiveSockets () > > > + || Doorbell >= NUMBER_OF_DOORBELLS_PER_SOCKET) > > > + { > > > + return 0; > > > + } > > Coding style is > > if () { > > } > > > > This file gets it consistently wrong. > > I agree that it should be consistent. But that coding style conforms to > "5.2.1.6 Each sub-expression of a complex predicate expression must be on a > separate line" in EDK II Coding Standards Spec (https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/5_source_files/52_spacing#5-2-1-6-each-sub-expression-of-a-complex-predicate-expression-must-be-on-a-separate-line). > > "Predicate expressions containing multiple operators with sub-expressions > joined by && or || must have each sub-expression on a separate line. The > opening brace, '|{|' of the body shall be on a line by itself and aligned in > the starting column of the associated keyword. > > while ( ( Code == MEETS_STANDARD) >   && ( Code == FUNCTIONAL)) > { >   ShipIt(); > } > > " > > However, I'm OK to change it as your suggestion. I stand corrected. I think it's confusing, but if it's not only explicitly permitted but actually required by the spec, then feel free to leave it in. Best Regards, Leif (pondering starting a separate thread on changing that rule in the coding style)