From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR01-VE1-obe.outbound.protection.outlook.com (mail-ve1eur01on0072.outbound.protection.outlook.com [104.47.1.72]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 4C00D21E1B779 for ; Thu, 21 Sep 2017 08:31:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector1-arm-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=xHPulq11IK8gkfJH3VEwAV5sqjcjmYnY50EaMa86nCg=; b=ZWodtk6sij1eU1frQFstNPSPLbPRP72/tKMb4eBdRpaQS2hgZT0LpuzcEMXmpxuXyHrPwzcAcqLQYHDDQZKroxZHbV+FXn/cU5KV5bUnGw5hWg2bpcAmD5y5wvFgQjngXgzFURpC4XJVU/OhVK5RC05Q8Oy5W+Mw3zladXF7Sd8= Received: from AM4PR0801MB1444.eurprd08.prod.outlook.com (10.168.5.24) by AM4PR0801MB1524.eurprd08.prod.outlook.com (10.168.5.150) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.56.11; Thu, 21 Sep 2017 15:34:07 +0000 Received: from AM4PR0801MB1444.eurprd08.prod.outlook.com ([fe80::4103:93c:2261:c26c]) by AM4PR0801MB1444.eurprd08.prod.outlook.com ([fe80::4103:93c:2261:c26c%13]) with mapi id 15.20.0056.018; Thu, 21 Sep 2017 15:34:07 +0000 From: Evan Lloyd To: Leif Lindholm CC: "edk2-devel@lists.01.org" , Ard Biesheuvel , Matteo Carlini , nd Thread-Topic: [PATCH 1/5] ArmPkg: Tidy GIC code before changes. Thread-Index: AQHTLXhIhy2LrkWV+UWzTCZkZ6rrN6K/gCbQ Date: Thu, 21 Sep 2017 15:34:07 +0000 Message-ID: References: <20170911152335.72672-1-evan.lloyd@arm.com> <20170911152335.72672-2-evan.lloyd@arm.com> <20170914164109.foiwrmrdbm7aftd6@bivouac.eciton.net> In-Reply-To: <20170914164109.foiwrmrdbm7aftd6@bivouac.eciton.net> Accept-Language: en-GB, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=Evan.Lloyd@arm.com; x-originating-ip: [217.140.96.140] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; AM4PR0801MB1524; 6:PQsMNhgERbkPDmzZVapX8tkW4RuCH+3AnxaXpQCtjWysympADFugXEmC1ei3hzcSIDgK9GTwuL38Ccae3hLNurAUgOt4lvJnDFlD3pNFywH0rtIX9rBE/0GQp6ahDLmeyPqKe5O9zMPYb+9RObM7j3WSB+KH3hvSyLUr1HSsOJaD45oftTXcVwbrdWD62ZSjUGvdpEzzpF2qkZvkz4MdC9vC0406JSX8MJmjGL1H6KOPIHU0/HfswGzjoaV1GmPqWTgXzT6B1Ttj3onBkARDJpT96hzViY+yOQqt83v5KU1Hh81IIj1oaRZrD38VOlAHJA3EcediDq+kLoSOUL/qlg==; 5:+LCuem22qo3tpZN4FUJ4ffuprlohV9bTZYjqb9r/adt7h96FAMpp1KTQ6EHI4eOEfN7ExDlITLCybhziKN8vs3jROvGha/MlgvyXYK0sXn5lIHJ69FgPLPtvaI6RMMuHvu+/wvhsWZ0Q3ILctsQoFw==; 24:vLmIzgKymnELQrAH9FsKCgsM5YbpkT6lk+E7iNu2zOeoeXKy18IhZo09R40U+I17G5ydYVetpPOG8Fm/5fzGQ5/3eSVirUlZTwYnl+gu7w4=; 7:bWwSB5ADJG+lUZbz6a2YvfNM6UJ8WDj56KWYTxYTDwEnhnWdrYR/JGnj0S9YgSfxxIodf1rxKi/s4gy7PH0WSWs4E1Bia3SNEWTDMzQECnWW9DaYKKeF7tGOb2tuGLEVO0FohznpIVVwXCUIAQsUvvxnn0/1Lv82JvuOT2+JU8tMMTpOOflGgFyTWfeUrbr471/77/TZymN2Lgyhg9ejoofHYVf9E+ioTbfy5TUoULE= x-ms-exchange-antispam-srfa-diagnostics: SSOS; x-ms-office365-filtering-correlation-id: 168c592c-ca96-4ea6-a2fc-08d5010632c6 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(300000500095)(300135000095)(300000501095)(300135300095)(22001)(300000502095)(300135100095)(2017030254152)(48565401081)(300000503095)(300135400095)(2017052603199)(201703131423075)(201703031133081)(201702281549075)(300000504095)(300135200095)(300000505095)(300135600095)(300000506095)(300135500095); SRVR:AM4PR0801MB1524; x-ms-traffictypediagnostic: AM4PR0801MB1524: nodisclaimer: True x-exchange-antispam-report-test: UriScan:(180628864354917)(162533806227266); x-microsoft-antispam-prvs: x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(2401047)(5005006)(8121501046)(10201501046)(100000703101)(100105400095)(3002001)(93006095)(93001095)(6055026)(6041248)(20161123558100)(20161123560025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123555025)(20161123562025)(20161123564025)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095); SRVR:AM4PR0801MB1524; BCL:0; PCL:0; RULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095); SRVR:AM4PR0801MB1524; x-forefront-prvs: 04371797A5 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(6009001)(346002)(376002)(39860400002)(189002)(24454002)(199003)(13464003)(5660300001)(97736004)(5250100002)(8676002)(81156014)(81166006)(86362001)(101416001)(66066001)(50986999)(68736007)(7696004)(25786009)(2950100002)(6916009)(54356999)(305945005)(76176999)(2906002)(14454004)(6116002)(7736002)(72206003)(6436002)(478600001)(3660700001)(229853002)(4326008)(3846002)(316002)(102836003)(6506006)(3280700002)(6246003)(54906003)(33656002)(106356001)(74316002)(8936002)(2900100001)(189998001)(105586002)(99286003)(9686003)(55016002)(53936002)(53546010); DIR:OUT; SFP:1101; SCL:1; SRVR:AM4PR0801MB1524; H:AM4PR0801MB1444.eurprd08.prod.outlook.com; FPR:; SPF:None; PTR:InfoNoRecords; MX:1; A:1; LANG:en; received-spf: None (protection.outlook.com: arm.com does not designate permitted sender hosts) spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-originalarrivaltime: 21 Sep 2017 15:34:07.4228 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM4PR0801MB1524 Subject: Re: [PATCH 1/5] ArmPkg: Tidy GIC code before changes. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 21 Sep 2017 15:31:03 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Leif. I agree/accept all the comments, except: > -----Original Message----- > From: Leif Lindholm [mailto:leif.lindholm@linaro.org] > Sent: 14 September 2017 17:41 > To: Evan Lloyd > Cc: edk2-devel@lists.01.org; Ard Biesheuvel ; > Matteo Carlini ; nd > Subject: Re: [PATCH 1/5] ArmPkg: Tidy GIC code before changes. >=20 > On Mon, Sep 11, 2017 at 04:23:31PM +0100, evan.lloyd@arm.com wrote: > > From: Evan Lloyd > > ... >=20 > > // whereas Affinity3 is defined at [32:39] in MPIDR > > - CpuAffinity =3D (MpId & (ARM_CORE_AFF0 | ARM_CORE_AFF1 | > > ARM_CORE_AFF2)) | ((MpId & ARM_CORE_AFF3) >> 8); > > + CpuAffinity =3D (MpId & (ARM_CORE_AFF0 | ARM_CORE_AFF1 | > ARM_CORE_AFF2)) > > + | ((MpId & ARM_CORE_AFF3) >> 8); >=20 > I can't find an explicit rule on this, but my preference aligns with what > examples I can see in the Coding Style: moving that lone '|' to the end o= f the > preceding line: >=20 > CpuAffinity =3D (MpId & (ARM_CORE_AFF0 | ARM_CORE_AFF1 | > ARM_CORE_AFF2)) | > ((MpId & ARM_CORE_AFF3) >> 8); [[Evan Lloyd]] 5.2.1.6 and 5.7.2.4 have multiple examples of prefix style,= =20 with 5.2.2.11 and 5.7.2.3 providing only 2 instances of a line suffix ope= rator. I can change it if you insist, but it will be a clear instance of a=20 maintainer's personal prejudice overriding the coding standard. I strongl= y believe prefix format is much clearer, especially for compound conditions with nesting. >=20 > > if (Revision =3D=3D ARM_GIC_ARCH_REVISION_3) { ... > > // Write set-enable register > > - MmioWrite32 (GicCpuRedistributorBase + > ARM_GICR_CTLR_FRAME_SIZE + ARM_GICR_ISENABLER + (4 * RegOffset), 1 > << RegShift); > > + MmioWrite32 ( > > + (GicCpuRedistributorBase > > + + ARM_GICR_CTLR_FRAME_SIZE > > + + ARM_GICR_ISENABLER > > + + (4 * RegOffset)), > > + 1 << RegShift > > + ); >=20 > Maybe rewrite as >=20 > #define SET_ENABLE_ADDRESS(base,offset) ((base) + > ARM_GICR_CTLR_FRAME_SIZE + \ > ARM_GICR_ISENABLER + (4 * offset= )) >=20 > (further up) >=20 > then >=20 > MmioWrite32 ( > SET_ENABLE_ADDRESS (GicCpuRedistributorBase, RegOffset), > 1 << RegShift > ); >=20 > ? [[Evan Lloyd]] Agree, but I called the macros ISENABLER_ADDRESS and ISENABL= ER_ADDRESS (using the register names) because SET_ seemed to imply writing something i= n this context. ...