Re: [edk2-devel] [edk2-platforms][PATCH v2 1/5] Platform/RPi3/RpiFirmwareDxe: Add more query functions
On 2019.10.10 17:43, Leif Lindholm wrote:
On Thu, Oct 10, 2019 at 01:41:06PM +0100, Pete Batard wrote:
Hi Leif,
On 2019.10.10 09:39, Leif Lindholm wrote:
On Tue, Oct 08, 2019 at 01:38:37PM +0100, Pete Batard wrote:
This patch introduces the capability to also query the Model Name/
Manufacturer Name/CPU Name/Firmware Revision using the RpiFirmware
protocol. This is aims at making the driver more suitable to cater
for platforms other than the Raspberry Pi 3 as well as simplifying
the population of entries in PlatformSmbiosDxe.
Signed-off-by: Pete Batard
---
Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c | 155
+++-
Platform/RaspberryPi/RPi3/Include/Protocol/RpiFirmware.h | 58
++--
2 files changed, 196 insertions(+), 17 deletions(-)
diff --git a/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
b/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
index 9b5ee1946279..378c99bcba05 100644
--- a/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
+++ b/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
@@ -461,7 +461,7 @@ typedef struct {
RPI_FW_TAG_HEAD TagHead;
RPI_FW_MODEL_REVISION_TAG TagBody;
UINT32EndTag;
-} RPI_FW_GET_MODEL_REVISION_CMD;
+} RPI_FW_GET_REVISION_CMD;
#pragma pack()
STATIC
@@ -471,7 +471,7 @@ RpiFirmwareGetModelRevision (
OUT UINT32 *Revision
)
{
- RPI_FW_GET_MODEL_REVISION_CMD *Cmd;
+ RPI_FW_GET_REVISION_CMD *Cmd;
EFI_STATUSStatus;
UINT32Result;
@@ -506,6 +506,153 @@ RpiFirmwareGetModelRevision (
return EFI_SUCCESS;
}
+STATIC
+EFI_STATUS
+EFIAPI
+RpiFirmwareGetFirmwareRevision (
+ OUT UINT32 *Revision
+ )
+{
+ RPI_FW_GET_REVISION_CMD *Cmd;
+ EFI_STATUSStatus;
+ UINT32Result;
+
+ if (!AcquireSpinLockOrFail ()) {
+DEBUG ((DEBUG_ERROR, "%a: failed to acquire spinlock\n", __FUNCTION__));
+return EFI_DEVICE_ERROR;
+ }
+
+ Cmd = mDmaBuffer;
+ ZeroMem (Cmd, sizeof (*Cmd));
+
+ Cmd->BufferHead.BufferSize = sizeof (*Cmd);
+ Cmd->BufferHead.Response= 0;
+ Cmd->TagHead.TagId = RPI_MBOX_GET_REVISION;
+ Cmd->TagHead.TagSize= sizeof (Cmd->TagBody);
+ Cmd->TagHead.TagValueSize = 0;
+ Cmd->EndTag = 0;
+
+ Status = MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC_CHANNEL,
);
+
+ ReleaseSpinLock ();
+
+ if (EFI_ERROR (Status) ||
+ Cmd->BufferHead.Response != RPI_MBOX_RESP_SUCCESS) {
+DEBUG ((DEBUG_ERROR,
+ "%a: mailbox transaction error: Status == %r, Response == 0x%x\n",
+ __FUNCTION__, Status, Cmd->BufferHead.Response));
+return EFI_DEVICE_ERROR;
+ }
+
+ *Revision = Cmd->TagBody.Revision;
+ return EFI_SUCCESS;
+}
+
+STATIC
+CHAR8*
+EFIAPI
+RpiFirmwareGetModelName (
+ IN INTN ModelId
+ )
+{
+ UINT32 Revision;
+
+ // If a negative ModelId is passed, detect it.
+ if ((ModelId < 0) && (RpiFirmwareGetModelRevision () ==
EFI_SUCCESS))
Style-wise, please always use {} with if-statements.
Will do.
Beyond that, this pattern repeats identcally in three functions in
this file. Meanwhile, there is never any error handling of
RpiFirmwareGetModelRevision other than if not successful, we leave it
as negative.
Yes, because then we'll get the default case (that returns "Unknown ..."),
which is precisely what we want if we can't access the model revision for
any reason.
Are there other intended uses of that function, or could
we move this logic there?
I'm not sure how that would work, since ModelId, ManufacturerId, and CpuId
are for different parts of the bit fields, so, if I understand what you're
suggesting correctly (but I'm not really sure I do) trying to move the check
for negative value into RpiFirmwareGetModelRevision () wouldn't work that
great. Plus then the function name would become a misnommer.
Are you really seeing a functional issue with the current code, or something
that would make a possible contributor wanting to modify this section
puzzled as to what we're trying to accomplish here (which I think is pretty
clear, but then again, I wrote the code so maybe I'm not the most impartial?
Because if not, I'd rather save my limited supply of time, and just go with
a v3 that only adds the {} suggested.
I see no functional issues with the code, but with my own limited
supply of time I try to ensure the code stays as simple as possible so
I don't need to stop and think where there isn't actually anything
complicated going on (like here). I am after all a bear of very little
brain.
I also will *always* trigger on seeing a near-identical pattern
repeated many times in the same file.
In this instance, I find myself spending way too much time untangling
what is actually going on. I see
- SysInfoUpdateSmbiosType1 () calling mFwProtocol->GetModelRevision (),
and then using the
Re: [edk2-devel] [edk2-platforms][PATCH v2 1/5] Platform/RPi3/RpiFirmwareDxe: Add more query functions
On Thu, Oct 10, 2019 at 01:41:06PM +0100, Pete Batard wrote:
> Hi Leif,
>
> On 2019.10.10 09:39, Leif Lindholm wrote:
> > On Tue, Oct 08, 2019 at 01:38:37PM +0100, Pete Batard wrote:
> > > This patch introduces the capability to also query the Model Name/
> > > Manufacturer Name/CPU Name/Firmware Revision using the RpiFirmware
> > > protocol. This is aims at making the driver more suitable to cater
> > > for platforms other than the Raspberry Pi 3 as well as simplifying
> > > the population of entries in PlatformSmbiosDxe.
> > >
> > > Signed-off-by: Pete Batard
> > > ---
> > > Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c | 155
> > > +++-
> > > Platform/RaspberryPi/RPi3/Include/Protocol/RpiFirmware.h | 58
> > > ++--
> > > 2 files changed, 196 insertions(+), 17 deletions(-)
> > >
> > > diff --git
> > > a/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
> > > b/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
> > > index 9b5ee1946279..378c99bcba05 100644
> > > --- a/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
> > > +++ b/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
> > > @@ -461,7 +461,7 @@ typedef struct {
> > > RPI_FW_TAG_HEAD TagHead;
> > > RPI_FW_MODEL_REVISION_TAG TagBody;
> > > UINT32EndTag;
> > > -} RPI_FW_GET_MODEL_REVISION_CMD;
> > > +} RPI_FW_GET_REVISION_CMD;
> > > #pragma pack()
> > > STATIC
> > > @@ -471,7 +471,7 @@ RpiFirmwareGetModelRevision (
> > > OUT UINT32 *Revision
> > > )
> > > {
> > > - RPI_FW_GET_MODEL_REVISION_CMD *Cmd;
> > > + RPI_FW_GET_REVISION_CMD *Cmd;
> > > EFI_STATUSStatus;
> > > UINT32Result;
> > > @@ -506,6 +506,153 @@ RpiFirmwareGetModelRevision (
> > > return EFI_SUCCESS;
> > > }
> > > +STATIC
> > > +EFI_STATUS
> > > +EFIAPI
> > > +RpiFirmwareGetFirmwareRevision (
> > > + OUT UINT32 *Revision
> > > + )
> > > +{
> > > + RPI_FW_GET_REVISION_CMD *Cmd;
> > > + EFI_STATUSStatus;
> > > + UINT32Result;
> > > +
> > > + if (!AcquireSpinLockOrFail ()) {
> > > +DEBUG ((DEBUG_ERROR, "%a: failed to acquire spinlock\n",
> > > __FUNCTION__));
> > > +return EFI_DEVICE_ERROR;
> > > + }
> > > +
> > > + Cmd = mDmaBuffer;
> > > + ZeroMem (Cmd, sizeof (*Cmd));
> > > +
> > > + Cmd->BufferHead.BufferSize = sizeof (*Cmd);
> > > + Cmd->BufferHead.Response= 0;
> > > + Cmd->TagHead.TagId = RPI_MBOX_GET_REVISION;
> > > + Cmd->TagHead.TagSize= sizeof (Cmd->TagBody);
> > > + Cmd->TagHead.TagValueSize = 0;
> > > + Cmd->EndTag = 0;
> > > +
> > > + Status = MailboxTransaction (Cmd->BufferHead.BufferSize,
> > > RPI_MBOX_VC_CHANNEL, );
> > > +
> > > + ReleaseSpinLock ();
> > > +
> > > + if (EFI_ERROR (Status) ||
> > > + Cmd->BufferHead.Response != RPI_MBOX_RESP_SUCCESS) {
> > > +DEBUG ((DEBUG_ERROR,
> > > + "%a: mailbox transaction error: Status == %r, Response == 0x%x\n",
> > > + __FUNCTION__, Status, Cmd->BufferHead.Response));
> > > +return EFI_DEVICE_ERROR;
> > > + }
> > > +
> > > + *Revision = Cmd->TagBody.Revision;
> > > + return EFI_SUCCESS;
> > > +}
> > > +
> > > +STATIC
> > > +CHAR8*
> > > +EFIAPI
> > > +RpiFirmwareGetModelName (
> > > + IN INTN ModelId
> > > + )
> > > +{
> > > + UINT32 Revision;
> > > +
> > > + // If a negative ModelId is passed, detect it.
> > > + if ((ModelId < 0) && (RpiFirmwareGetModelRevision () ==
> > > EFI_SUCCESS))
> >
> > Style-wise, please always use {} with if-statements.
>
> Will do.
>
> > Beyond that, this pattern repeats identcally in three functions in
> > this file. Meanwhile, there is never any error handling of
> > RpiFirmwareGetModelRevision other than if not successful, we leave it
> > as negative.
>
> Yes, because then we'll get the default case (that returns "Unknown ..."),
> which is precisely what we want if we can't access the model revision for
> any reason.
>
> > Are there other intended uses of that function, or could
> > we move this logic there?
>
> I'm not sure how that would work, since ModelId, ManufacturerId, and CpuId
> are for different parts of the bit fields, so, if I understand what you're
> suggesting correctly (but I'm not really sure I do) trying to move the check
> for negative value into RpiFirmwareGetModelRevision () wouldn't work that
> great. Plus then the function name would become a misnommer.
>
> Are you really seeing a functional issue with the current code, or something
> that would make a possible contributor wanting to modify this section
> puzzled as to what we're trying to accomplish here (which I think is pretty
> clear, but then again, I wrote the code so maybe I'm not the most impartial?
> Because if not, I'd rather save my limited supply of time, and just go with
> a v3 that only adds the {} suggested.
I
Re: [edk2-devel] [edk2-platforms][PATCH v2 1/5] Platform/RPi3/RpiFirmwareDxe: Add more query functions
Hi Leif,
On 2019.10.10 09:39, Leif Lindholm wrote:
On Tue, Oct 08, 2019 at 01:38:37PM +0100, Pete Batard wrote:
This patch introduces the capability to also query the Model Name/
Manufacturer Name/CPU Name/Firmware Revision using the RpiFirmware
protocol. This is aims at making the driver more suitable to cater
for platforms other than the Raspberry Pi 3 as well as simplifying
the population of entries in PlatformSmbiosDxe.
Signed-off-by: Pete Batard
---
Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c | 155
+++-
Platform/RaspberryPi/RPi3/Include/Protocol/RpiFirmware.h | 58
++--
2 files changed, 196 insertions(+), 17 deletions(-)
diff --git a/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
b/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
index 9b5ee1946279..378c99bcba05 100644
--- a/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
+++ b/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
@@ -461,7 +461,7 @@ typedef struct {
RPI_FW_TAG_HEAD TagHead;
RPI_FW_MODEL_REVISION_TAG TagBody;
UINT32EndTag;
-} RPI_FW_GET_MODEL_REVISION_CMD;
+} RPI_FW_GET_REVISION_CMD;
#pragma pack()
STATIC
@@ -471,7 +471,7 @@ RpiFirmwareGetModelRevision (
OUT UINT32 *Revision
)
{
- RPI_FW_GET_MODEL_REVISION_CMD *Cmd;
+ RPI_FW_GET_REVISION_CMD *Cmd;
EFI_STATUSStatus;
UINT32Result;
@@ -506,6 +506,153 @@ RpiFirmwareGetModelRevision (
return EFI_SUCCESS;
}
+STATIC
+EFI_STATUS
+EFIAPI
+RpiFirmwareGetFirmwareRevision (
+ OUT UINT32 *Revision
+ )
+{
+ RPI_FW_GET_REVISION_CMD *Cmd;
+ EFI_STATUSStatus;
+ UINT32Result;
+
+ if (!AcquireSpinLockOrFail ()) {
+DEBUG ((DEBUG_ERROR, "%a: failed to acquire spinlock\n", __FUNCTION__));
+return EFI_DEVICE_ERROR;
+ }
+
+ Cmd = mDmaBuffer;
+ ZeroMem (Cmd, sizeof (*Cmd));
+
+ Cmd->BufferHead.BufferSize = sizeof (*Cmd);
+ Cmd->BufferHead.Response= 0;
+ Cmd->TagHead.TagId = RPI_MBOX_GET_REVISION;
+ Cmd->TagHead.TagSize= sizeof (Cmd->TagBody);
+ Cmd->TagHead.TagValueSize = 0;
+ Cmd->EndTag = 0;
+
+ Status = MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC_CHANNEL,
);
+
+ ReleaseSpinLock ();
+
+ if (EFI_ERROR (Status) ||
+ Cmd->BufferHead.Response != RPI_MBOX_RESP_SUCCESS) {
+DEBUG ((DEBUG_ERROR,
+ "%a: mailbox transaction error: Status == %r, Response == 0x%x\n",
+ __FUNCTION__, Status, Cmd->BufferHead.Response));
+return EFI_DEVICE_ERROR;
+ }
+
+ *Revision = Cmd->TagBody.Revision;
+ return EFI_SUCCESS;
+}
+
+STATIC
+CHAR8*
+EFIAPI
+RpiFirmwareGetModelName (
+ IN INTN ModelId
+ )
+{
+ UINT32 Revision;
+
+ // If a negative ModelId is passed, detect it.
+ if ((ModelId < 0) && (RpiFirmwareGetModelRevision () ==
EFI_SUCCESS))
Style-wise, please always use {} with if-statements.
Will do.
Beyond that, this pattern repeats identcally in three functions in
this file. Meanwhile, there is never any error handling of
RpiFirmwareGetModelRevision other than if not successful, we leave it
as negative.
Yes, because then we'll get the default case (that returns "Unknown
..."), which is precisely what we want if we can't access the model
revision for any reason.
Are there other intended uses of that function, or could
we move this logic there?
I'm not sure how that would work, since ModelId, ManufacturerId, and
CpuId are for different parts of the bit fields, so, if I understand
what you're suggesting correctly (but I'm not really sure I do) trying
to move the check for negative value into RpiFirmwareGetModelRevision ()
wouldn't work that great. Plus then the function name would become a
misnommer.
Are you really seeing a functional issue with the current code, or
something that would make a possible contributor wanting to modify this
section puzzled as to what we're trying to accomplish here (which I
think is pretty clear, but then again, I wrote the code so maybe I'm not
the most impartial? Because if not, I'd rather save my limited supply of
time, and just go with a v3 that only adds the {} suggested.
Regards,
/Pete
/
Leif
+ModelId = (Revision >> 4) & 0xFF;
+
+ switch (ModelId) {
+ //
www.raspberrypi.org/documentation/hardware/raspberrypi/revision-codes/README.md
+ case 0x00:
+return "Raspberry Pi Model A";
+ case 0x01:
+return "Raspberry Pi Model B";
+ case 0x02:
+return "Raspberry Pi Model A+";
+ case 0x03:
+return "Raspberry Pi Model B+";
+ case 0x04:
+return "Raspberry Pi 2 Model B";
+ case 0x06:
+return "Raspberry Pi Compute Module 1";
+ case 0x08:
+return "Raspberry Pi 3 Model B";
+ case 0x09:
+return "Raspberry Pi Zero";
+ case 0x0A:
+return "Raspberry Pi Compute Module 3";
+ case 0x0C:
+return
Re: [edk2-devel] [edk2-platforms][PATCH v2 1/5] Platform/RPi3/RpiFirmwareDxe: Add more query functions
On Tue, Oct 08, 2019 at 01:38:37PM +0100, Pete Batard wrote:
> This patch introduces the capability to also query the Model Name/
> Manufacturer Name/CPU Name/Firmware Revision using the RpiFirmware
> protocol. This is aims at making the driver more suitable to cater
> for platforms other than the Raspberry Pi 3 as well as simplifying
> the population of entries in PlatformSmbiosDxe.
>
> Signed-off-by: Pete Batard
> ---
> Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c | 155
> +++-
> Platform/RaspberryPi/RPi3/Include/Protocol/RpiFirmware.h | 58
> ++--
> 2 files changed, 196 insertions(+), 17 deletions(-)
>
> diff --git
> a/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
> b/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
> index 9b5ee1946279..378c99bcba05 100644
> --- a/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
> +++ b/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
> @@ -461,7 +461,7 @@ typedef struct {
>RPI_FW_TAG_HEAD TagHead;
>RPI_FW_MODEL_REVISION_TAG TagBody;
>UINT32EndTag;
> -} RPI_FW_GET_MODEL_REVISION_CMD;
> +} RPI_FW_GET_REVISION_CMD;
> #pragma pack()
>
> STATIC
> @@ -471,7 +471,7 @@ RpiFirmwareGetModelRevision (
>OUT UINT32 *Revision
>)
> {
> - RPI_FW_GET_MODEL_REVISION_CMD *Cmd;
> + RPI_FW_GET_REVISION_CMD *Cmd;
>EFI_STATUSStatus;
>UINT32Result;
>
> @@ -506,6 +506,153 @@ RpiFirmwareGetModelRevision (
>return EFI_SUCCESS;
> }
>
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +RpiFirmwareGetFirmwareRevision (
> + OUT UINT32 *Revision
> + )
> +{
> + RPI_FW_GET_REVISION_CMD *Cmd;
> + EFI_STATUSStatus;
> + UINT32Result;
> +
> + if (!AcquireSpinLockOrFail ()) {
> +DEBUG ((DEBUG_ERROR, "%a: failed to acquire spinlock\n", __FUNCTION__));
> +return EFI_DEVICE_ERROR;
> + }
> +
> + Cmd = mDmaBuffer;
> + ZeroMem (Cmd, sizeof (*Cmd));
> +
> + Cmd->BufferHead.BufferSize = sizeof (*Cmd);
> + Cmd->BufferHead.Response= 0;
> + Cmd->TagHead.TagId = RPI_MBOX_GET_REVISION;
> + Cmd->TagHead.TagSize= sizeof (Cmd->TagBody);
> + Cmd->TagHead.TagValueSize = 0;
> + Cmd->EndTag = 0;
> +
> + Status = MailboxTransaction (Cmd->BufferHead.BufferSize,
> RPI_MBOX_VC_CHANNEL, );
> +
> + ReleaseSpinLock ();
> +
> + if (EFI_ERROR (Status) ||
> + Cmd->BufferHead.Response != RPI_MBOX_RESP_SUCCESS) {
> +DEBUG ((DEBUG_ERROR,
> + "%a: mailbox transaction error: Status == %r, Response == 0x%x\n",
> + __FUNCTION__, Status, Cmd->BufferHead.Response));
> +return EFI_DEVICE_ERROR;
> + }
> +
> + *Revision = Cmd->TagBody.Revision;
> + return EFI_SUCCESS;
> +}
> +
> +STATIC
> +CHAR8*
> +EFIAPI
> +RpiFirmwareGetModelName (
> + IN INTN ModelId
> + )
> +{
> + UINT32 Revision;
> +
> + // If a negative ModelId is passed, detect it.
> + if ((ModelId < 0) && (RpiFirmwareGetModelRevision () ==
> EFI_SUCCESS))
Style-wise, please always use {} with if-statements.
Beyond that, this pattern repeats identcally in three functions in
this file. Meanwhile, there is never any error handling of
RpiFirmwareGetModelRevision other than if not successful, we leave it
as negative. Are there other intended uses of that function, or could
we move this logic there?
/
Leif
> +ModelId = (Revision >> 4) & 0xFF;
> +
> + switch (ModelId) {
> + //
> www.raspberrypi.org/documentation/hardware/raspberrypi/revision-codes/README.md
> + case 0x00:
> +return "Raspberry Pi Model A";
> + case 0x01:
> +return "Raspberry Pi Model B";
> + case 0x02:
> +return "Raspberry Pi Model A+";
> + case 0x03:
> +return "Raspberry Pi Model B+";
> + case 0x04:
> +return "Raspberry Pi 2 Model B";
> + case 0x06:
> +return "Raspberry Pi Compute Module 1";
> + case 0x08:
> +return "Raspberry Pi 3 Model B";
> + case 0x09:
> +return "Raspberry Pi Zero";
> + case 0x0A:
> +return "Raspberry Pi Compute Module 3";
> + case 0x0C:
> +return "Raspberry Pi Zero W";
> + case 0x0D:
> +return "Raspberry Pi 3 Model B+";
> + case 0x0E:
> +return "Raspberry Pi 3 Model A+";
> + case 0x11:
> +return "Raspberry Pi 4 Model B";
> + default:
> +return "Unknown Raspberry Pi Model";
> + }
> +}
> +
> +STATIC
> +CHAR8*
> +EFIAPI
> +RpiFirmwareGetManufacturerName (
> + IN INTN ManufacturerId
> + )
> +{
> + UINT32 Revision;
> +
> + // If a negative ModelId is passed, detect it.
> + if ((ManufacturerId < 0) && (RpiFirmwareGetModelRevision () ==
> EFI_SUCCESS))
> +ManufacturerId = (Revision >> 16) & 0x0F;
> +
> + switch (ManufacturerId) {
> + //
> www.raspberrypi.org/documentation/hardware/raspberrypi/revision-codes/README.md
> + case 0x00:
> +return "Sony UK";
> + case 0x01:
> +return "Egoman";
> + case 0x02:
> + case 0x04:
>
[edk2-devel] [edk2-platforms][PATCH v2 1/5] Platform/RPi3/RpiFirmwareDxe: Add more query functions
This patch introduces the capability to also query the Model Name/
Manufacturer Name/CPU Name/Firmware Revision using the RpiFirmware
protocol. This is aims at making the driver more suitable to cater
for platforms other than the Raspberry Pi 3 as well as simplifying
the population of entries in PlatformSmbiosDxe.
Signed-off-by: Pete Batard
---
Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c | 155
+++-
Platform/RaspberryPi/RPi3/Include/Protocol/RpiFirmware.h | 58
++--
2 files changed, 196 insertions(+), 17 deletions(-)
diff --git a/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
b/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
index 9b5ee1946279..378c99bcba05 100644
--- a/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
+++ b/Platform/RaspberryPi/RPi3/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
@@ -461,7 +461,7 @@ typedef struct {
RPI_FW_TAG_HEAD TagHead;
RPI_FW_MODEL_REVISION_TAG TagBody;
UINT32EndTag;
-} RPI_FW_GET_MODEL_REVISION_CMD;
+} RPI_FW_GET_REVISION_CMD;
#pragma pack()
STATIC
@@ -471,7 +471,7 @@ RpiFirmwareGetModelRevision (
OUT UINT32 *Revision
)
{
- RPI_FW_GET_MODEL_REVISION_CMD *Cmd;
+ RPI_FW_GET_REVISION_CMD *Cmd;
EFI_STATUSStatus;
UINT32Result;
@@ -506,6 +506,153 @@ RpiFirmwareGetModelRevision (
return EFI_SUCCESS;
}
+STATIC
+EFI_STATUS
+EFIAPI
+RpiFirmwareGetFirmwareRevision (
+ OUT UINT32 *Revision
+ )
+{
+ RPI_FW_GET_REVISION_CMD *Cmd;
+ EFI_STATUSStatus;
+ UINT32Result;
+
+ if (!AcquireSpinLockOrFail ()) {
+DEBUG ((DEBUG_ERROR, "%a: failed to acquire spinlock\n", __FUNCTION__));
+return EFI_DEVICE_ERROR;
+ }
+
+ Cmd = mDmaBuffer;
+ ZeroMem (Cmd, sizeof (*Cmd));
+
+ Cmd->BufferHead.BufferSize = sizeof (*Cmd);
+ Cmd->BufferHead.Response= 0;
+ Cmd->TagHead.TagId = RPI_MBOX_GET_REVISION;
+ Cmd->TagHead.TagSize= sizeof (Cmd->TagBody);
+ Cmd->TagHead.TagValueSize = 0;
+ Cmd->EndTag = 0;
+
+ Status = MailboxTransaction (Cmd->BufferHead.BufferSize,
RPI_MBOX_VC_CHANNEL, );
+
+ ReleaseSpinLock ();
+
+ if (EFI_ERROR (Status) ||
+ Cmd->BufferHead.Response != RPI_MBOX_RESP_SUCCESS) {
+DEBUG ((DEBUG_ERROR,
+ "%a: mailbox transaction error: Status == %r, Response == 0x%x\n",
+ __FUNCTION__, Status, Cmd->BufferHead.Response));
+return EFI_DEVICE_ERROR;
+ }
+
+ *Revision = Cmd->TagBody.Revision;
+ return EFI_SUCCESS;
+}
+
+STATIC
+CHAR8*
+EFIAPI
+RpiFirmwareGetModelName (
+ IN INTN ModelId
+ )
+{
+ UINT32 Revision;
+
+ // If a negative ModelId is passed, detect it.
+ if ((ModelId < 0) && (RpiFirmwareGetModelRevision () ==
EFI_SUCCESS))
+ModelId = (Revision >> 4) & 0xFF;
+
+ switch (ModelId) {
+ //
www.raspberrypi.org/documentation/hardware/raspberrypi/revision-codes/README.md
+ case 0x00:
+return "Raspberry Pi Model A";
+ case 0x01:
+return "Raspberry Pi Model B";
+ case 0x02:
+return "Raspberry Pi Model A+";
+ case 0x03:
+return "Raspberry Pi Model B+";
+ case 0x04:
+return "Raspberry Pi 2 Model B";
+ case 0x06:
+return "Raspberry Pi Compute Module 1";
+ case 0x08:
+return "Raspberry Pi 3 Model B";
+ case 0x09:
+return "Raspberry Pi Zero";
+ case 0x0A:
+return "Raspberry Pi Compute Module 3";
+ case 0x0C:
+return "Raspberry Pi Zero W";
+ case 0x0D:
+return "Raspberry Pi 3 Model B+";
+ case 0x0E:
+return "Raspberry Pi 3 Model A+";
+ case 0x11:
+return "Raspberry Pi 4 Model B";
+ default:
+return "Unknown Raspberry Pi Model";
+ }
+}
+
+STATIC
+CHAR8*
+EFIAPI
+RpiFirmwareGetManufacturerName (
+ IN INTN ManufacturerId
+ )
+{
+ UINT32 Revision;
+
+ // If a negative ModelId is passed, detect it.
+ if ((ManufacturerId < 0) && (RpiFirmwareGetModelRevision () ==
EFI_SUCCESS))
+ManufacturerId = (Revision >> 16) & 0x0F;
+
+ switch (ManufacturerId) {
+ //
www.raspberrypi.org/documentation/hardware/raspberrypi/revision-codes/README.md
+ case 0x00:
+return "Sony UK";
+ case 0x01:
+return "Egoman";
+ case 0x02:
+ case 0x04:
+return "Embest";
+ case 0x03:
+return "Sony Japan";
+ case 0x05:
+return "Stadium";
+ default:
+return "Unknown Manufacturer";
+ }
+}
+
+STATIC
+CHAR8*
+EFIAPI
+RpiFirmwareGetCpuName (
+ IN INTN CpuId
+ )
+{
+ UINT32 Revision;
+
+ // If a negative CpuId is passed, detect it.
+ if ((CpuId < 0) && (RpiFirmwareGetModelRevision () == EFI_SUCCESS))
+CpuId = (Revision >> 12) & 0x0F;
+
+ switch (CpuId) {
+ //
www.raspberrypi.org/documentation/hardware/raspberrypi/revision-codes/README.md
+ case 0x00:
+return "BCM2835 (ARM11)";
+ case 0x01:
+return "BCM2836 (ARM Cortex-A7)";
+ case 0x02:
+return "BCM2837 (ARM Cortex-A53)";
+ case 0x03:
+return "BCM2711 (ARM Cortex-A72)";
+
