From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f65.google.com (mail-wr1-f65.google.com [209.85.221.65]) by mx.groups.io with SMTP id smtpd.web10.5169.1570711284154604808 for ; Thu, 10 Oct 2019 05:41:24 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@akeo-ie.20150623.gappssmtp.com header.s=20150623 header.b=o3Xbb1WR; spf=none, err=permanent DNS error (domain: akeo.ie, ip: 209.85.221.65, mailfrom: pete@akeo.ie) Received: by mail-wr1-f65.google.com with SMTP id y19so7723150wrd.3 for ; Thu, 10 Oct 2019 05:41:23 -0700 (PDT) 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=PXP0jb4aB0+UXk61wvj99EuIASgFqs12919V1Enab9Q=; b=o3Xbb1WROKmY5hdf+pecxGbmxNjimId0nOBwq4FCJtbH0jpw7vvPBBMdwEifQttz2D uHe23voDZPOIp5ZnwpD7lxgQmMIrHqNJWdLbWvV3arhBCuhzfSFqpAb8IdkxgaYCQx3k rJi79WqiWRJEzEvLQsvSdXIYyRnuWZptHFanooOkfGXxqKB3jKw7Fn5oGs2a0GkmMEMi l8ld1aFvM5IT/1nCF4uVDyozfPV7dbEVZaMrYpNUhz2hQteojkWTa1zDxX11vfArN14E e3aYc4PHGFuGNx8m71c67q2Z30jvt+y/4gaAOk5fqBWa9G0+rxqex3khLKZiLr9FSJFp RZ1w== 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=PXP0jb4aB0+UXk61wvj99EuIASgFqs12919V1Enab9Q=; b=pFhX6bM0FIURD4QdYGDecQblTaaBBNUCiuWf1JJW2N8zkgWBw+1vcC/9iD1+GtEfDv VXoeU6XYa6qunfT2ctG92kEmDZMIh3lfa6vUCvT7EaPFRJCnTmpi7eABLtTT9vxz/dER pAFNAX/69226YEjqCPCZw6rUXgcouI2dLArN3bdbRSlu7Ev0znXLsv3Us2ldeIwI3ojX z6kxM9v/0XBVvqvZdyYylQGaAKw/JBw72/thZVIcMBp8zGC9KNDLE3QU3NOe9F91TbDn +Kxn8y/xyNNXA+lR6HOOwEJhIdoWm0XJn+BcbJWGuBP5vKBEYNPiOa1ks0QIofv7MtaB PqWQ== X-Gm-Message-State: APjAAAWllK0e7qOdS3tUQvKex/gDJbjTjfbwVY1bRI7CCNueBqvyegUo DrozaBml8A1i/CaDwnD7iabPuQ== X-Google-Smtp-Source: APXvYqx3wRUJhyWAW12wVDw0WoNndLkuDWSYgVNgMB8dy2k/8i863ROhvynZsvA0W6mdZo2rhpYGCQ== X-Received: by 2002:a5d:5271:: with SMTP id l17mr8600708wrc.19.1570711282719; Thu, 10 Oct 2019 05:41:22 -0700 (PDT) Return-Path: Received: from [10.0.0.122] ([84.203.40.122]) by smtp.googlemail.com with ESMTPSA id q22sm5112193wmj.5.2019.10.10.05.41.21 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 10 Oct 2019 05:41:21 -0700 (PDT) Subject: Re: [edk2-platforms][PATCH v2 2/5] Platform/RPi3/RpiFirmwareDxe: Improve serial number population To: Leif Lindholm Cc: devel@edk2.groups.io, ard.biesheuvel@linaro.org References: <20191008123841.12952-1-pete@akeo.ie> <20191008123841.12952-3-pete@akeo.ie> <20191010084344.GT25504@bivouac.eciton.net> From: "Pete Batard" Message-ID: <766e6b6f-d788-e46e-e58f-84a2f851645c@akeo.ie> Date: Thu, 10 Oct 2019 13:41:20 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: <20191010084344.GT25504@bivouac.eciton.net> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit Hi Leif, It's a disagreement. And the same goes for 3/5 & 4/5. Please see the note I wrote in 0/5 for the v2, because the cover letter this is usually the place I try to clarify elements that may throw off a maintainer, and that don't belong in a commit message. Not so sound flippant here, but as long as there isn't an MTV award for "Most atomic codebase ever", I just don't have the time to split what I consider to be frivolous commits. The reasoning behind that is that I realistically don't consider that people are actually going to be thrown off by a "while I was here I also fixed an obvious typo" that got added into an existing commit or, most important, that even if they do, the amount of time that is going to be collectively wasted by people who might be thrown of by not having uber atomicity is not going to exceed the amount of time it will cost *me* to split it. Therefore, while I do understand the desire to have an atomic commit history, I'm afraid that if we can't strike a balance between how much extra time contributors are expected to waste vs how atomic a *real-life* codebase is enforced to be, if I have to split every little typo and stylistic fix into yet another commit, I'm simply not going to bother fixing typos or low hanging fruits I see any more. Regards, /Pete On 2019.10.10 09:43, Leif Lindholm wrote: > On Tue, Oct 08, 2019 at 01:38:38PM +0100, Pete Batard wrote: >> Improve RpiFirmwareGetSerial() to derive a serial number from the >> MAC address, in case the platform returns 0 for the serial number. >> >> Also fix a typo where "%s" was used instead of "%a". > > I did not see a reply to my feedback on the previous round, so I'm > unsure if this is a mistake or a disagreement? > >> Signed-off-by: Pete Batard >> --- >> Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c | 11 +++++++++-- >> 1 file changed, 9 insertions(+), 2 deletions(-) >> >> diff --git a/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c b/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c >> index 378c99bcba05..c2344252d2c0 100644 >> --- a/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c >> +++ b/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c >> @@ -219,7 +219,7 @@ RpiFirmwareSetPowerState ( >> >> if (!EFI_ERROR (Status) && >> PowerState ^ (Cmd->TagBody.PowerState & RPI_MBOX_POWER_STATE_ENABLE)) { >> - DEBUG ((DEBUG_ERROR, "%a: failed to %sable power for device %d\n", >> + DEBUG ((DEBUG_ERROR, "%a: failed to %aable power for device %d\n", > > This is clearly a bugfix, but I don't see it as being part of "Improve > serial number population", which is what it claims to be if I view it > wih git blame. > > / > Leif > >> __FUNCTION__, PowerState ? "en" : "dis", DeviceId)); >> Status = EFI_DEVICE_ERROR; >> } >> @@ -393,7 +393,14 @@ RpiFirmwareGetSerial ( >> } >> >> *Serial = Cmd->TagBody.Serial; >> - return EFI_SUCCESS; >> + // Some platforms return 0 for serial. For those, try to use the MAC address. >> + if (*Serial == 0) { >> + Status = RpiFirmwareGetMacAddress ((UINT8*) Serial); >> + // Convert to a more user-friendly value >> + *Serial = SwapBytes64 (*Serial << 16); >> + } >> + >> + return Status; >> } >> >> #pragma pack() >> -- >> 2.21.0.windows.1 >>