From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from eu-smtp-delivery-143.mimecast.com (eu-smtp-delivery-143.mimecast.com [207.82.80.143]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 9B5D31A1E82 for ; Tue, 11 Oct 2016 03:23:17 -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=mA5ssXhGGXSnwUMJNiNHAx1G0zoNqgV35YVyr5n1DQQ=; b=alJ4j3k2P2TbFLGRHiU0TJYhGc30jmk0+gx7SzSPDlNEWEFNxKsscnPMlfK7QjMO+fc7mcT+M9HN87V43xSREO6TSD03JvawlZ4mMNJnt8qbT8rNjXlow5HdKc9eP7Sz2Hou8iOwcsdWbIjto6Er5fey60X6GlmwkivRDCycnkk= Received: from EUR01-DB5-obe.outbound.protection.outlook.com (mail-db5eur01lp0176.outbound.protection.outlook.com [213.199.154.176]) (Using TLS) by eu-smtp-1.mimecast.com with ESMTP id uk-mta-34-lCANTcltOp-sdmNQgO_N2Q-1; Tue, 11 Oct 2016 11:23:14 +0100 Received: from AM5PR0801MB1762.eurprd08.prod.outlook.com (10.169.247.16) by AM5PR0801MB1764.eurprd08.prod.outlook.com (10.169.247.18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.1.649.16; Tue, 11 Oct 2016 10:23:13 +0000 Received: from AM5PR0801MB1762.eurprd08.prod.outlook.com ([10.169.247.16]) by AM5PR0801MB1762.eurprd08.prod.outlook.com ([10.169.247.16]) with mapi id 15.01.0649.024; Tue, 11 Oct 2016 10:23:13 +0000 From: Evan Lloyd To: Leif Lindholm CC: "edk2-devel@ml01.01.org" , "ard.biesheuvel@linaro.org" , "ryan.harkin@linaro.org" Thread-Topic: [PATCH 3/3] ArmPlatformPkg: Remove UINTN cast when setting BaudRate. Thread-Index: AQHSIykqFiPwnANMQUOBBx5dsqFG56CjAh2g Date: Tue, 11 Oct 2016 10:23:12 +0000 Message-ID: References: <20160921203315.11204-1-evan.lloyd@arm.com> <20160921203315.11204-4-evan.lloyd@arm.com> <20161010190438.GN3471@bivouac.eciton.net> In-Reply-To: <20161010190438.GN3471@bivouac.eciton.net> Accept-Language: en-GB, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [217.140.96.140] x-ms-office365-filtering-correlation-id: 154e3822-28b9-4c12-8c86-08d3f1c09b4c x-microsoft-exchange-diagnostics: 1; AM5PR0801MB1764; 7:t8gYiifNIP8akijYNLIcKke9L/kAtH9eA3kgv0w7p7LOeDR+QpuahxgIaQWJsUzTRAsXPWCrTBacFjo+8u5OkfV8qkb3Zti+oaeJlPe4Xk+JA7Dwz7n1aKzt1jFYvO7EwCeqC14VYuoTUPtU5UX3Bvbykd8JZ2qSkdOu6ekfDj2TL36K3FKOchmVulowz6tdKUezjUiBYSPtXYq47Qez9So++z0gclhDo7dQWyH6V4UP6XSXPkeiyqk2tNjy4rPwJ3Nmz/5pA0SwUOaCnzJAihd5R1q3Q3y4/ZBqv8yvZ+59gU68z2VomrIfcKYXUFOi0Ej9YYlZBq8s5z65PsdN2xy9pvwE33JiG7Ds0TKHdVc=; 20:wjtd12HtX1RP0clXN/EN1F2GjsiLwJ7zdWyVIFn2GjsSYUsjaP1uG1RfFs9bqkq1zEq2FA27jwm1q74bJRXDZIXdkyAid6baMkLOF665AfBZpE1hp+SWjSUwiQWRgfNjQ1yhHw4uyoClqgq+Gu5xF7t9Ri7Pp8MhCGquanYYRzo= x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:AM5PR0801MB1764; x-ld-processed: f34e5979-57d9-4aaa-ad4d-b122a662184d,ExtAddr x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(180628864354917); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(6040176)(601004)(2401047)(5005006)(8121501046)(10201501046)(3002001)(6055026); SRVR:AM5PR0801MB1764; BCL:0; PCL:0; RULEID:; SRVR:AM5PR0801MB1764; x-forefront-prvs: 00922518D8 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(6009001)(7916002)(199003)(189002)(40434004)(24454002)(13464003)(8676002)(97736004)(2950100002)(189998001)(305945005)(5890100001)(11100500001)(7846002)(101416001)(66066001)(102836003)(68736007)(76576001)(9686002)(81166006)(575784001)(74316002)(586003)(110136003)(3846002)(7736002)(81156014)(92566002)(10400500002)(6916009)(4326007)(7696004)(87936001)(6116002)(5002640100001)(77096005)(105586002)(106116001)(106356001)(19580395003)(19580405001)(5660300001)(122556002)(2906002)(86362001)(3280700002)(54356999)(76176999)(8936002)(2900100001)(50986999)(3660700001)(33656002)(19627235001); DIR:OUT; SFP:1101; SCL:1; SRVR:AM5PR0801MB1764; H:AM5PR0801MB1762.eurprd08.prod.outlook.com; FPR:; SPF:None; PTR:InfoNoRecords; A:1; MX:1; LANG:en; spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-originalarrivaltime: 11 Oct 2016 10:23:12.9010 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM5PR0801MB1764 X-MC-Unique: lCANTcltOp-sdmNQgO_N2Q-1 Subject: Re: [PATCH 3/3] ArmPlatformPkg: Remove UINTN cast when setting BaudRate. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 11 Oct 2016 10:23:18 -0000 Content-Language: en-US Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable Hi Leif. Please feel free to fix the space change as you see fit and proper, as it w= as just incidental tidying up. It would be good to have a discussion about the general position, though. There are, I am sure, sound reasons for not rolling these things together (= and I knew that, and shouldn't have, but...). I understand some of those reasons, but I see some unfortunate consequences= too, so I'd like to play devil's advocate here. One unfortunate effect is to discourage the submission of trivial changes t= hat would not in themselves justify the rigmarole of a patch. I know we would hope to do it all properly, but I don't think that is how h= uman nature works. The cost/benefit comparison of adding or removing a spa= ce (or other cosmetic change) as a separate patch is not really worthwhile,= so it is much easier to not "see" the improvement. Please note: I am not = thinking solely of myself here - I know others find the same thing. Of course, if the intent is " to discourage the submission of trivial chang= es" that would make a sort of sense from the maintainer's perspective. My position is that by making minor incidental improvement relatively expen= sive the actual effect is to discourage it. Does the benefit derived from discrete patches really override this disadva= ntage? One could rephrase that as "does a tidy git log outweigh good code quality?= " Regards, Evan >-----Original Message----- >From: Leif Lindholm [mailto:leif.lindholm@linaro.org] >Sent: 10 October 2016 20:05 >To: Evan Lloyd >Cc: edk2-devel@ml01.01.org; ard.biesheuvel@linaro.org; >ryan.harkin@linaro.org >Subject: Re: [PATCH 3/3] ArmPlatformPkg: Remove UINTN cast when >setting BaudRate. > > >On Wed, Sep 21, 2016 at 09:33:15PM +0100, evan.lloyd@arm.com wrote: >> From: Alexei >> >> SerialPortInitialize() set the BaudRate variable (type UINT64) as: >> BaudRate =3D (UINTN)FixedPcdGet64 (PcdUartDefaultBaudRate); >> >> This commit fixes a potential problem on ARM 32-bit builds, where the >> UINTN type is defined as UINT32, by removing the cast: >> >> BaudRate =3D FixedPcdGet64 (PcdUartDefaultBaudRate); >> >> Note - a minor whitespace correction is rolled into this commit. > >I can unroll it for you before committing, but I'm not going to leave >the history in a state where it looks like a FixedPcdGet8 was modified >by a commit with the title "Remove UINTN cast when setting BaudRate.". > >For the fix: >Reviewed-by: Leif Lindholm > >Let me know how you want to deal with the whitespace change. > >/ > Leif > >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Alexei Fedorov >> Signed-off-by: Evan Lloyd >> --- >> ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git >a/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c >b/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c >> index >5dce852d90f9cafb828d81dae39d03451ea608e2..4a24eded0e7d0f91270bf778 >cf1d89b6c809d0b2 100644 >> --- a/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c >> +++ b/ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.c >> @@ -41,11 +41,11 @@ SerialPortInitialize ( >> UINT8 DataBits; >> EFI_STOP_BITS_TYPE StopBits; >> >> - BaudRate =3D (UINTN)FixedPcdGet64 (PcdUartDefaultBaudRate); >> + BaudRate =3D FixedPcdGet64 (PcdUartDefaultBaudRate); >> ReceiveFifoDepth =3D 0; // Use default FIFO depth >> Parity =3D (EFI_PARITY_TYPE)FixedPcdGet8 (PcdUartDefaultParity); >> DataBits =3D FixedPcdGet8 (PcdUartDefaultDataBits); >> - StopBits =3D (EFI_STOP_BITS_TYPE) FixedPcdGet8 >(PcdUartDefaultStopBits); >> + StopBits =3D (EFI_STOP_BITS_TYPE)FixedPcdGet8 >(PcdUartDefaultStopBits); >> >> return PL011UartInitializePort ( >> (UINTN)FixedPcdGet64 (PcdSerialRegisterBase), >> -- >> Guid("CE165669-3EF3-493F-B85D-6190EE5B9759") >> IMPORTANT NOTICE: The contents of this email and any attachments are confid= ential and may also be privileged. If you are not the intended recipient, p= lease notify the sender immediately and do not disclose the contents to any= other person, use it for any purpose, or store or copy the information in = any medium. Thank you.