From: "Pete Batard" <pete@akeo.ie>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: edk2-devel-groups-io <devel@edk2.groups.io>,
"Leif Lindholm" <leif.lindholm@linaro.org>,
"Philippe Mathieu-Daudé" <philmd@redhat.com>
Subject: Re: [edk2-platforms][PATCH 2/2] Platform/RPi/RPiFirmwareDxe: Fix serial number population for RPi4
Date: Thu, 9 Jan 2020 15:02:07 +0000 [thread overview]
Message-ID: <787fa8df-69a2-9aba-8296-8a6e14a8af4a@akeo.ie> (raw)
In-Reply-To: <CAKv+Gu-UUFGxfdiqL3dtB6OhZTFiTq2pHt=QZCrftS3hcRYpEQ@mail.gmail.com>
Hi Ard,
On 2020.01.09 14:44, Ard Biesheuvel wrote:
> On Wed, 8 Jan 2020 at 18:00, Pete Batard <pete@akeo.ie> wrote:
>>
>> Some (all?) Raspbery Pi 4 platforms report 0x0000000010000000 as their
>> board serial when queried through the VideoCore mailbox.
>>
>> Fix this by using the MAC address then.
>>
>> Signed-off-by: Pete Batard <pete@akeo.ie>
>> ---
>> Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
>> index dd61ef089ca7..75826fdc0e53 100644
>> --- a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
>> +++ b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
>> @@ -394,8 +394,9 @@ RpiFirmwareGetSerial (
>> }
>>
>> *Serial = Cmd->TagBody.Serial;
>> - // Some platforms return 0 for serial. For those, try to use the MAC address.
>> - if (*Serial == 0) {
>> + // Some platforms return 0 or 0x0000000010000000 for serial.
>> + // For those, try to use the MAC address.
>> + if ((*Serial == 0) || ((*Serial & 0xFFFFFFFF0FFFFFFFULL) == 0)) {
>
> What is the point of using a mask here? Is it deliberately matching
> 0x0000000020000000 or 0x00000000F0000000 as well?
My expectation is that if we're seeing 0x0000000010000000 as a board
serial, there's not so insignificant chance are future boards may use
0x0000000010000000, 0x0000000030000000 and so on.
In other words, when someone starts counting from 0 to 1 somewhere, it's
not unreasonable to also expect them to reach 2 at some stage...
Now, if anything, the one change we could apply here is remove the
(*Serial == 0) check, which of course is redundant. But I fail to see
why we'd want to restrict ourselves to checking only for
0x0000000010000000 here, when we have no clear idea how the value we
read may evolve in the future, as anything that the mask condition
matches is clearly not a serial we want to use anyway...
Regards,
/Pete
>
>> Status = RpiFirmwareGetMacAddress ((UINT8*) Serial);
>> // Convert to a more user-friendly value
>> *Serial = SwapBytes64 (*Serial << 16);
>> --
>> 2.21.0.windows.1
>>
next prev parent reply other threads:[~2020-01-09 15:02 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-08 17:00 [edk2-platforms][PATCH 0/2] Platform/RPi: Smbios reporting improvements Pete Batard
2020-01-08 17:00 ` [edk2-platforms][PATCH 1/2] Platform/RPi/SmbiosDxe: Report a more human readable firmware revision Pete Batard
2020-01-20 14:08 ` Philippe Mathieu-Daudé
2020-01-08 17:00 ` [edk2-platforms][PATCH 2/2] Platform/RPi/RPiFirmwareDxe: Fix serial number population for RPi4 Pete Batard
2020-01-09 14:44 ` Ard Biesheuvel
2020-01-09 15:02 ` Pete Batard [this message]
2020-01-20 13:50 ` Pete Batard
2020-02-14 9:02 ` [edk2-platforms][PATCH 0/2] Platform/RPi: Smbios reporting improvements Ard Biesheuvel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=787fa8df-69a2-9aba-8296-8a6e14a8af4a@akeo.ie \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox