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 <p...@akeo.ie>
---
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
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#48729): https://edk2.groups.io/g/devel/message/48729
Mute This Topic: https://groups.io/mt/34441817/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
