Changelog: v4: * Use 64-bit value on PL061 register base address. * Use fallback to be compatible with current PcdPL061GpioBase value when platform gpio driver isn't present. * Remove the dependancy on PL061. Move the dependancy to platform gpio driver instead. v3: * Remove GPIO_PIN_MASK_HIGH_8BIT() and GPIO_PIN_MASK_LOW_8BIT(). * Avoid to use MmioAnd8() on updating GPIO DATA register, since PL061 could access each bit by specified register offset. * Add PLATFORM_GPIO_CONTROLLER structure in embedded gpio. * Support multiple PL061 gpio controllers in one platform. v2: * Append the patch to fix gpio pin mask macro.
Haojian Zhuang (3): ArmPlatformPkg: PL061: fix accessing GPIO DATA EmbeddedPkg: enhance for multiple gpio controllers ArmPlatformPkg: PL061: support multiple controller
ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c | 137 +++++++++++++++------ .../Drivers/PL061GpioDxe/PL061GpioDxe.inf | 1 + ArmPlatformPkg/Include/Drivers/PL061Gpio.h | 51 ++++---- EmbeddedPkg/EmbeddedPkg.dec | 1 + EmbeddedPkg/Include/Protocol/EmbeddedGpio.h | 17 +++ 5 files changed, 143 insertions(+), 64 deletions(-)
// All bits low except one bit high, restricted to 8 bits // (i.e. ensures zeros above 8bits)
But '&&' is wrong at here. It'll only return 1 or 0 as the result of GPIO_PIN_MASK_HIGH_8BIT(Pin).
Since PL061 spec said in below.
In order to write to GPIODATA, the corresponding bits in the mask, resulting from the address bus, PADDR[9:2], must be HIGH. Otherwise the bit values remain unchanged by the write. Similarly, the values read from this register are determined for each bit, by the mask bit derived from the address used to access the data register, PADDR[9:2]. Bits that are 1 in the address mask cause the corresponding bits in GPIODATA to be read, and bits that are 0 in the address mask cause the corresponding bits in GPIODATA to be read as 0, regardless of their value.
Now simplify the way to read/write bit of GPIO_DATA register.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Haojian Zhuang haojian.zhuang@linaro.org --- ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c | 16 ++++++++-------- ArmPlatformPkg/Include/Drivers/PL061Gpio.h | 4 ---- 2 files changed, 8 insertions(+), 12 deletions(-)
diff --git a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c index ff05662..042fc76 100644 --- a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c +++ b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c @@ -125,7 +125,7 @@ Get ( } }
- if (MmioRead8 (PL061_GPIO_DATA_REG) & GPIO_PIN_MASK_HIGH_8BIT(Gpio)) { + if (MmioRead8 (PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Gpio) << 2))) { *Value = 1; } else { *Value = 0; @@ -181,21 +181,21 @@ Set ( { case GPIO_MODE_INPUT: // Set the corresponding direction bit to LOW for input - MmioAnd8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK_LOW_8BIT(Gpio)); + MmioAnd8 (PL061_GPIO_DIR_REG, ~GPIO_PIN_MASK(Gpio)); break;
case GPIO_MODE_OUTPUT_0: // Set the corresponding data bit to LOW for 0 - MmioAnd8 (PL061_GPIO_DATA_REG, GPIO_PIN_MASK_LOW_8BIT(Gpio)); + MmioWrite8 (PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Gpio) << 2), 0); // Set the corresponding direction bit to HIGH for output - MmioOr8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK_HIGH_8BIT(Gpio)); + MmioOr8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK(Gpio)); break;
case GPIO_MODE_OUTPUT_1: // Set the corresponding data bit to HIGH for 1 - MmioOr8 (PL061_GPIO_DATA_REG, GPIO_PIN_MASK_HIGH_8BIT(Gpio)); + MmioWrite8 (PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Gpio) << 2), 0xff); // Set the corresponding direction bit to HIGH for output - MmioOr8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK_HIGH_8BIT(Gpio)); + MmioOr8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK(Gpio)); break;
default: @@ -250,9 +250,9 @@ GetMode ( }
// Check if it is input or output - if (MmioRead8 (PL061_GPIO_DIR_REG) & GPIO_PIN_MASK_HIGH_8BIT(Gpio)) { + if (MmioRead8 (PL061_GPIO_DIR_REG) & GPIO_PIN_MASK(Gpio)) { // Pin set to output - if (MmioRead8 (PL061_GPIO_DATA_REG) & GPIO_PIN_MASK_HIGH_8BIT(Gpio)) { + if (MmioRead8 (PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Gpio) << 2))) { *Mode = GPIO_MODE_OUTPUT_1; } else { *Mode = GPIO_MODE_OUTPUT_0; diff --git a/ArmPlatformPkg/Include/Drivers/PL061Gpio.h b/ArmPlatformPkg/Include/Drivers/PL061Gpio.h index 38458f4..d436fd4 100644 --- a/ArmPlatformPkg/Include/Drivers/PL061Gpio.h +++ b/ArmPlatformPkg/Include/Drivers/PL061Gpio.h @@ -46,9 +46,5 @@
// All bits low except one bit high, native bit length #define GPIO_PIN_MASK(Pin) (1UL << ((UINTN)(Pin))) -// All bits low except one bit high, restricted to 8 bits (i.e. ensures zeros above 8bits) -#define GPIO_PIN_MASK_HIGH_8BIT(Pin) (GPIO_PIN_MASK(Pin) && 0xFF) -// All bits high except one bit low, restricted to 8 bits (i.e. ensures zeros above 8bits) -#define GPIO_PIN_MASK_LOW_8BIT(Pin) ((~GPIO_PIN_MASK(Pin)) && 0xFF)
#endif // __PL061_GPIO_H__
EmbeddedGpio only supports one gpio controller in one platform. Now create PLATFORM_GPIO_CONTROLLER to support multiple gpio controllers in one platform.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Haojian Zhuang haojian.zhuang@linaro.org --- EmbeddedPkg/EmbeddedPkg.dec | 1 + EmbeddedPkg/Include/Protocol/EmbeddedGpio.h | 17 +++++++++++++++++ 2 files changed, 18 insertions(+)
diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec index 4bac580..bd3e301 100644 --- a/EmbeddedPkg/EmbeddedPkg.dec +++ b/EmbeddedPkg/EmbeddedPkg.dec @@ -65,6 +65,7 @@ gAndroidFastbootTransportProtocolGuid = { 0x74bd9fe0, 0x8902, 0x11e3, {0xb9, 0xd3, 0xf7, 0x22, 0x38, 0xfc, 0x9a, 0x31}} gAndroidFastbootPlatformProtocolGuid = { 0x524685a0, 0x89a0, 0x11e3, {0x9d, 0x4d, 0xbf, 0xa9, 0xf6, 0xa4, 0x03, 0x08}} gUsbDeviceProtocolGuid = { 0x021bd2ca, 0x51d2, 0x11e3, {0x8e, 0x56, 0xb7, 0x54, 0x17, 0xc7, 0x0b, 0x44 }} + gPlatformGpioProtocolGuid = { 0x52ce9845, 0x5af4, 0x43e2, {0xba, 0xfd, 0x23, 0x08, 0x12, 0x54, 0x7a, 0xc2 }}
[PcdsFeatureFlag.common] gEmbeddedTokenSpaceGuid.PcdEmbeddedMacBoot|FALSE|BOOLEAN|0x00000001 diff --git a/EmbeddedPkg/Include/Protocol/EmbeddedGpio.h b/EmbeddedPkg/Include/Protocol/EmbeddedGpio.h index 4e7c8db..b8bc929 100644 --- a/EmbeddedPkg/Include/Protocol/EmbeddedGpio.h +++ b/EmbeddedPkg/Include/Protocol/EmbeddedGpio.h @@ -164,4 +164,21 @@ struct _EMBEDDED_GPIO {
extern EFI_GUID gEmbeddedGpioProtocolGuid;
+typedef struct _GPIO_CONTROLLER GPIO_CONTROLLER; +typedef struct _PLATFORM_GPIO_CONTROLLER PLATFORM_GPIO_CONTROLLER; + +struct _GPIO_CONTROLLER { + UINTN RegisterBase; + UINTN GpioIndex; + UINTN InternalGpioCount; +}; + +struct _PLATFORM_GPIO_CONTROLLER { + UINTN GpioCount; + UINTN GpioControllerCount; + GPIO_CONTROLLER *GpioController; +}; + +extern EFI_GUID gPlatformGpioProtocolGuid; + #endif
Support multiple PL061 controllers. If platform gpio driver couldn't be found, PL061 gpio driver will continue to load PcdPL061GpioBase as the register base. It could be compatible with the use case of current PL061 gpio driver.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Haojian Zhuang haojian.zhuang@linaro.org --- ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c | 137 +++++++++++++++------ .../Drivers/PL061GpioDxe/PL061GpioDxe.inf | 1 + ArmPlatformPkg/Include/Drivers/PL061Gpio.h | 47 ++++--- 3 files changed, 125 insertions(+), 60 deletions(-)
diff --git a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c index 042fc76..02da8e1 100644 --- a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c +++ b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c @@ -19,6 +19,7 @@ #include <Library/BaseMemoryLib.h> #include <Library/DebugLib.h> #include <Library/IoLib.h> +#include <Library/MemoryAllocationLib.h> #include <Library/PcdLib.h> #include <Library/UefiBootServicesTableLib.h> #include <Library/UefiLib.h> @@ -28,6 +29,7 @@ #include <Drivers/PL061Gpio.h>
BOOLEAN mPL061Initialized = FALSE; +PLATFORM_GPIO_CONTROLLER *mPL061PlatformGpio;
/** Function implementations @@ -38,20 +40,36 @@ PL061Identify ( VOID ) { - // Check if this is a PrimeCell Peripheral - if ( (MmioRead8 (PL061_GPIO_PCELL_ID0) != 0x0D) - || (MmioRead8 (PL061_GPIO_PCELL_ID1) != 0xF0) - || (MmioRead8 (PL061_GPIO_PCELL_ID2) != 0x05) - || (MmioRead8 (PL061_GPIO_PCELL_ID3) != 0xB1)) { - return EFI_NOT_FOUND; + UINTN Index; + UINTN RegisterBase; + + if ( (mPL061PlatformGpio->GpioCount == 0) + || (mPL061PlatformGpio->GpioControllerCount == 0)) { + return EFI_NOT_FOUND; }
- // Check if this PrimeCell Peripheral is the PL061 GPIO - if ( (MmioRead8 (PL061_GPIO_PERIPH_ID0) != 0x61) - || (MmioRead8 (PL061_GPIO_PERIPH_ID1) != 0x10) - || ((MmioRead8 (PL061_GPIO_PERIPH_ID2) & 0xF) != 0x04) - || (MmioRead8 (PL061_GPIO_PERIPH_ID3) != 0x00)) { - return EFI_NOT_FOUND; + for (Index = 0; Index < mPL061PlatformGpio->GpioControllerCount; Index++) { + if (mPL061PlatformGpio->GpioController[Index].InternalGpioCount != PL061_GPIO_PINS) { + return EFI_INVALID_PARAMETER; + } + + RegisterBase = mPL061PlatformGpio->GpioController[Index].RegisterBase; + + // Check if this is a PrimeCell Peripheral + if ( (MmioRead8 (RegisterBase + PL061_GPIO_PCELL_ID0) != 0x0D) + || (MmioRead8 (RegisterBase + PL061_GPIO_PCELL_ID1) != 0xF0) + || (MmioRead8 (RegisterBase + PL061_GPIO_PCELL_ID2) != 0x05) + || (MmioRead8 (RegisterBase + PL061_GPIO_PCELL_ID3) != 0xB1)) { + return EFI_NOT_FOUND; + } + + // Check if this PrimeCell Peripheral is the PL061 GPIO + if ( (MmioRead8 (RegisterBase + PL061_GPIO_PERIPH_ID0) != 0x61) + || (MmioRead8 (RegisterBase + PL061_GPIO_PERIPH_ID1) != 0x10) + || ((MmioRead8 (RegisterBase + PL061_GPIO_PERIPH_ID2) & 0xF) != 0x04) + || (MmioRead8 (RegisterBase + PL061_GPIO_PERIPH_ID3) != 0x00)) { + return EFI_NOT_FOUND; + } }
return EFI_SUCCESS; @@ -84,6 +102,31 @@ PL061Initialize ( return Status; }
+EFI_STATUS +EFIAPI +PL061Locate ( + IN EMBEDDED_GPIO_PIN Gpio, + OUT UINTN *ControllerIndex, + OUT UINTN *ControllerOffset, + OUT UINTN *RegisterBase + ) +{ + UINT32 Index; + + for (Index = 0; Index < mPL061PlatformGpio->GpioControllerCount; Index++) { + if ( (Gpio >= mPL061PlatformGpio->GpioController[Index].GpioIndex) + && (Gpio < mPL061PlatformGpio->GpioController[Index].GpioIndex + + mPL061PlatformGpio->GpioController[Index].InternalGpioCount)) { + *ControllerIndex = Index; + *ControllerOffset = Gpio % mPL061PlatformGpio->GpioController[Index].InternalGpioCount; + *RegisterBase = mPL061PlatformGpio->GpioController[Index].RegisterBase; + return EFI_SUCCESS; + } + } + DEBUG ((EFI_D_ERROR, "%a, failed to locate gpio %d\n", __func__, Gpio)); + return EFI_INVALID_PARAMETER; +} + /**
Routine Description: @@ -110,11 +153,15 @@ Get ( ) { EFI_STATUS Status = EFI_SUCCESS; + UINTN Index, Offset, RegisterBase;
- if ( (Value == NULL) - || (Gpio > LAST_GPIO_PIN)) - { - return EFI_INVALID_PARAMETER; + Status = PL061Locate (Gpio, &Index, &Offset, &RegisterBase); + if (EFI_ERROR (Status)) + goto EXIT; + + if (Value == NULL) { + Status = EFI_INVALID_PARAMETER; + goto EXIT; }
// Initialize the hardware if not already done @@ -125,7 +172,7 @@ Get ( } }
- if (MmioRead8 (PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Gpio) << 2))) { + if (MmioRead8 (RegisterBase + PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Offset) << 2))) { *Value = 1; } else { *Value = 0; @@ -162,12 +209,11 @@ Set ( ) { EFI_STATUS Status = EFI_SUCCESS; + UINTN Index, Offset, RegisterBase;
- // Check for errors - if (Gpio > LAST_GPIO_PIN) { - Status = EFI_INVALID_PARAMETER; + Status = PL061Locate (Gpio, &Index, &Offset, &RegisterBase); + if (EFI_ERROR (Status)) goto EXIT; - }
// Initialize the hardware if not already done if (!mPL061Initialized) { @@ -181,21 +227,21 @@ Set ( { case GPIO_MODE_INPUT: // Set the corresponding direction bit to LOW for input - MmioAnd8 (PL061_GPIO_DIR_REG, ~GPIO_PIN_MASK(Gpio)); + MmioAnd8 (RegisterBase + PL061_GPIO_DIR_REG, ~GPIO_PIN_MASK(Gpio)); break;
case GPIO_MODE_OUTPUT_0: // Set the corresponding data bit to LOW for 0 - MmioWrite8 (PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Gpio) << 2), 0); + MmioWrite8 (RegisterBase + PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Offset) << 2), 0); // Set the corresponding direction bit to HIGH for output - MmioOr8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK(Gpio)); + MmioOr8 (RegisterBase + PL061_GPIO_DIR_REG, GPIO_PIN_MASK(Offset)); break;
case GPIO_MODE_OUTPUT_1: // Set the corresponding data bit to HIGH for 1 - MmioWrite8 (PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Gpio) << 2), 0xff); + MmioWrite8 (RegisterBase + PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Offset) << 2), 0xff); // Set the corresponding direction bit to HIGH for output - MmioOr8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK(Gpio)); + MmioOr8 (RegisterBase + PL061_GPIO_DIR_REG, GPIO_PIN_MASK(Offset)); break;
default: @@ -233,13 +279,12 @@ GetMode ( OUT EMBEDDED_GPIO_MODE *Mode ) { - EFI_STATUS Status; + EFI_STATUS Status; + UINTN Index, Offset, RegisterBase;
- // Check for errors - if ( (Mode == NULL) - || (Gpio > LAST_GPIO_PIN)) { - return EFI_INVALID_PARAMETER; - } + Status = PL061Locate (Gpio, &Index, &Offset, &RegisterBase); + if (EFI_ERROR (Status)) + return Status;
// Initialize the hardware if not already done if (!mPL061Initialized) { @@ -250,9 +295,9 @@ GetMode ( }
// Check if it is input or output - if (MmioRead8 (PL061_GPIO_DIR_REG) & GPIO_PIN_MASK(Gpio)) { + if (MmioRead8 (RegisterBase + PL061_GPIO_DIR_REG) & GPIO_PIN_MASK(Offset)) { // Pin set to output - if (MmioRead8 (PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Gpio) << 2))) { + if (MmioRead8 (RegisterBase + PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Offset) << 2))) { *Mode = GPIO_MODE_OUTPUT_1; } else { *Mode = GPIO_MODE_OUTPUT_0; @@ -321,14 +366,34 @@ PL061InstallProtocol ( IN EFI_SYSTEM_TABLE *SystemTable ) { - EFI_STATUS Status; - EFI_HANDLE Handle; + EFI_STATUS Status; + EFI_HANDLE Handle; + GPIO_CONTROLLER *GpioController;
// // Make sure the Gpio protocol has not been installed in the system yet. // ASSERT_PROTOCOL_ALREADY_INSTALLED (NULL, &gEmbeddedGpioProtocolGuid);
+ Status = gBS->LocateProtocol (&gPlatformGpioProtocolGuid, NULL, (VOID **)&mPL061PlatformGpio); + if (EFI_ERROR (Status) && (Status == EFI_NOT_FOUND)) { + // Create the mPL061PlatformGpio + mPL061PlatformGpio = (PLATFORM_GPIO_CONTROLLER *)AllocateZeroPool (sizeof (PLATFORM_GPIO_CONTROLLER) + sizeof (GPIO_CONTROLLER)); + if (mPL061PlatformGpio == NULL) { + DEBUG ((EFI_D_ERROR, "%a: failed to allocate PLATFORM_GPIO_CONTROLLER\n", __func__)); + return EFI_BAD_BUFFER_SIZE; + } + + mPL061PlatformGpio->GpioCount = PL061_GPIO_PINS; + mPL061PlatformGpio->GpioControllerCount = 1; + mPL061PlatformGpio->GpioController = (GPIO_CONTROLLER *)((UINTN) mPL061PlatformGpio + sizeof (PLATFORM_GPIO_CONTROLLER)); + + GpioController = mPL061PlatformGpio->GpioController; + GpioController->RegisterBase = (UINTN) PcdGet32 (PcdPL061GpioBase); + GpioController->GpioIndex = 0; + GpioController->InternalGpioCount = PL061_GPIO_PINS; + } + // Install the Embedded GPIO Protocol onto a new handle Handle = NULL; Status = gBS->InstallMultipleProtocolInterfaces( diff --git a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061GpioDxe.inf b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061GpioDxe.inf index 9d9e4cd..405a3a9 100644 --- a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061GpioDxe.inf +++ b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061GpioDxe.inf @@ -45,6 +45,7 @@
[Protocols] gEmbeddedGpioProtocolGuid + gPlatformGpioProtocolGuid
[Depex] TRUE diff --git a/ArmPlatformPkg/Include/Drivers/PL061Gpio.h b/ArmPlatformPkg/Include/Drivers/PL061Gpio.h index d436fd4..356428a 100644 --- a/ArmPlatformPkg/Include/Drivers/PL061Gpio.h +++ b/ArmPlatformPkg/Include/Drivers/PL061Gpio.h @@ -19,30 +19,29 @@ #include <Protocol/EmbeddedGpio.h>
// PL061 GPIO Registers -#define PL061_GPIO_DATA_REG ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0x000) -#define PL061_GPIO_DIR_REG ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0x400) -#define PL061_GPIO_IS_REG ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0x404) -#define PL061_GPIO_IBE_REG ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0x408) -#define PL061_GPIO_IEV_REG ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0x40C) -#define PL061_GPIO_IE_REG ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0x410) -#define PL061_GPIO_RIS_REG ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0x414) -#define PL061_GPIO_MIS_REG ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0x410) -#define PL061_GPIO_IC_REG ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0x41C) -#define PL061_GPIO_AFSEL_REG ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0x420) - -#define PL061_GPIO_PERIPH_ID0 ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0xFE0) -#define PL061_GPIO_PERIPH_ID1 ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0xFE4) -#define PL061_GPIO_PERIPH_ID2 ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0xFE8) -#define PL061_GPIO_PERIPH_ID3 ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0xFEC) - -#define PL061_GPIO_PCELL_ID0 ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0xFF0) -#define PL061_GPIO_PCELL_ID1 ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0xFF4) -#define PL061_GPIO_PCELL_ID2 ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0xFF8) -#define PL061_GPIO_PCELL_ID3 ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0xFFC) - - -// GPIO pins are numbered 0..7 -#define LAST_GPIO_PIN 7 +#define PL061_GPIO_DATA_REG 0x000 +#define PL061_GPIO_DIR_REG 0x400 +#define PL061_GPIO_IS_REG 0x404 +#define PL061_GPIO_IBE_REG 0x408 +#define PL061_GPIO_IEV_REG 0x40C +#define PL061_GPIO_IE_REG 0x410 +#define PL061_GPIO_RIS_REG 0x414 +#define PL061_GPIO_MIS_REG 0x410 +#define PL061_GPIO_IC_REG 0x41C +#define PL061_GPIO_AFSEL_REG 0x420 + +#define PL061_GPIO_PERIPH_ID0 0xFE0 +#define PL061_GPIO_PERIPH_ID1 0xFE4 +#define PL061_GPIO_PERIPH_ID2 0xFE8 +#define PL061_GPIO_PERIPH_ID3 0xFEC + +#define PL061_GPIO_PCELL_ID0 0xFF0 +#define PL061_GPIO_PCELL_ID1 0xFF4 +#define PL061_GPIO_PCELL_ID2 0xFF8 +#define PL061_GPIO_PCELL_ID3 0xFFC + + +#define PL061_GPIO_PINS 8
// All bits low except one bit high, native bit length #define GPIO_PIN_MASK(Pin) (1UL << ((UINTN)(Pin)))
Ping. This disappeared down a black hole.
On 19 August 2015 at 08:56, Haojian Zhuang haojian.zhuang@linaro.org wrote:
Changelog: v4: * Use 64-bit value on PL061 register base address. * Use fallback to be compatible with current PcdPL061GpioBase value when platform gpio driver isn't present. * Remove the dependancy on PL061. Move the dependancy to platform gpio driver instead. v3: * Remove GPIO_PIN_MASK_HIGH_8BIT() and GPIO_PIN_MASK_LOW_8BIT(). * Avoid to use MmioAnd8() on updating GPIO DATA register, since PL061 could access each bit by specified register offset. * Add PLATFORM_GPIO_CONTROLLER structure in embedded gpio. * Support multiple PL061 gpio controllers in one platform. v2: * Append the patch to fix gpio pin mask macro.
Haojian Zhuang (3): ArmPlatformPkg: PL061: fix accessing GPIO DATA EmbeddedPkg: enhance for multiple gpio controllers ArmPlatformPkg: PL061: support multiple controller
ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c | 137 +++++++++++++++------ .../Drivers/PL061GpioDxe/PL061GpioDxe.inf | 1 + ArmPlatformPkg/Include/Drivers/PL061Gpio.h | 51 ++++---- EmbeddedPkg/EmbeddedPkg.dec | 1 + EmbeddedPkg/Include/Protocol/EmbeddedGpio.h | 17 +++ 5 files changed, 143 insertions(+), 64 deletions(-)
-- 2.1.4
I found this patch series sitting in my "review" folder. I have some really minor comments, but I'll reply to the individual patches.
In the meantime, I tested this on Juno which also has PL061 and it works fine.
So for the whole series:
Tested-by: Ryan Harkin ryan.harkin@linaro.org
On 19 August 2015 at 08:56, Haojian Zhuang haojian.zhuang@linaro.org wrote:
// All bits low except one bit high, restricted to 8 bits // (i.e. ensures zeros above 8bits)
But '&&' is wrong at here. It'll only return 1 or 0 as the result of GPIO_PIN_MASK_HIGH_8BIT(Pin).
Since PL061 spec said in below.
In order to write to GPIODATA, the corresponding bits in the mask, resulting from the address bus, PADDR[9:2], must be HIGH. Otherwise the bit values remain unchanged by the write. Similarly, the values read from this register are determined for each bit, by the mask bit derived from the address used to access the data register, PADDR[9:2]. Bits that are 1 in the address mask cause the corresponding bits in GPIODATA to be read, and bits that are 0 in the address mask cause the corresponding bits in GPIODATA to be read as 0, regardless of their value.
Now simplify the way to read/write bit of GPIO_DATA register.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Haojian Zhuang haojian.zhuang@linaro.org
ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c | 16 ++++++++-------- ArmPlatformPkg/Include/Drivers/PL061Gpio.h | 4 ---- 2 files changed, 8 insertions(+), 12 deletions(-)
diff --git a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c index ff05662..042fc76 100644 --- a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c +++ b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c @@ -125,7 +125,7 @@ Get ( } }
- if (MmioRead8 (PL061_GPIO_DATA_REG) & GPIO_PIN_MASK_HIGH_8BIT(Gpio)) {
- if (MmioRead8 (PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Gpio) << 2))) {
Shouldn't this be and OR "|", not a plus "+"?
I realise that they probably achieve the same thing here, but using a "+" for masking is unusual.
The same comment applied to the other changes below:
*Value = 1;} else { *Value = 0; @@ -181,21 +181,21 @@ Set ( { case GPIO_MODE_INPUT: // Set the corresponding direction bit to LOW for input
MmioAnd8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK_LOW_8BIT(Gpio));
MmioAnd8 (PL061_GPIO_DIR_REG, ~GPIO_PIN_MASK(Gpio)); break;case GPIO_MODE_OUTPUT_0: // Set the corresponding data bit to LOW for 0
MmioAnd8 (PL061_GPIO_DATA_REG, GPIO_PIN_MASK_LOW_8BIT(Gpio));
MmioWrite8 (PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Gpio) << 2), 0); // Set the corresponding direction bit to HIGH for output
MmioOr8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK_HIGH_8BIT(Gpio));
MmioOr8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK(Gpio)); break;case GPIO_MODE_OUTPUT_1: // Set the corresponding data bit to HIGH for 1
MmioOr8 (PL061_GPIO_DATA_REG, GPIO_PIN_MASK_HIGH_8BIT(Gpio));
MmioWrite8 (PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Gpio) << 2), 0xff); // Set the corresponding direction bit to HIGH for output
MmioOr8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK_HIGH_8BIT(Gpio));
MmioOr8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK(Gpio)); break;default:
@@ -250,9 +250,9 @@ GetMode ( }
// Check if it is input or output
- if (MmioRead8 (PL061_GPIO_DIR_REG) & GPIO_PIN_MASK_HIGH_8BIT(Gpio)) {
- if (MmioRead8 (PL061_GPIO_DIR_REG) & GPIO_PIN_MASK(Gpio)) { // Pin set to output
- if (MmioRead8 (PL061_GPIO_DATA_REG) & GPIO_PIN_MASK_HIGH_8BIT(Gpio)) {
- if (MmioRead8 (PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Gpio) << 2))) { *Mode = GPIO_MODE_OUTPUT_1; } else { *Mode = GPIO_MODE_OUTPUT_0;
diff --git a/ArmPlatformPkg/Include/Drivers/PL061Gpio.h b/ArmPlatformPkg/Include/Drivers/PL061Gpio.h index 38458f4..d436fd4 100644 --- a/ArmPlatformPkg/Include/Drivers/PL061Gpio.h +++ b/ArmPlatformPkg/Include/Drivers/PL061Gpio.h @@ -46,9 +46,5 @@
// All bits low except one bit high, native bit length #define GPIO_PIN_MASK(Pin) (1UL << ((UINTN)(Pin))) -// All bits low except one bit high, restricted to 8 bits (i.e. ensures zeros above 8bits) -#define GPIO_PIN_MASK_HIGH_8BIT(Pin) (GPIO_PIN_MASK(Pin) && 0xFF) -// All bits high except one bit low, restricted to 8 bits (i.e. ensures zeros above 8bits) -#define GPIO_PIN_MASK_LOW_8BIT(Pin) ((~GPIO_PIN_MASK(Pin)) && 0xFF)
#endif // __PL061_GPIO_H__
2.1.4
edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 02/10/16 16:00, Ryan Harkin wrote:
On 19 August 2015 at 08:56, Haojian Zhuang haojian.zhuang@linaro.org wrote:
// All bits low except one bit high, restricted to 8 bits // (i.e. ensures zeros above 8bits)
But '&&' is wrong at here. It'll only return 1 or 0 as the result of GPIO_PIN_MASK_HIGH_8BIT(Pin).
Since PL061 spec said in below.
In order to write to GPIODATA, the corresponding bits in the mask, resulting from the address bus, PADDR[9:2], must be HIGH. Otherwise the bit values remain unchanged by the write. Similarly, the values read from this register are determined for each bit, by the mask bit derived from the address used to access the data register, PADDR[9:2]. Bits that are 1 in the address mask cause the corresponding bits in GPIODATA to be read, and bits that are 0 in the address mask cause the corresponding bits in GPIODATA to be read as 0, regardless of their value.
Now simplify the way to read/write bit of GPIO_DATA register.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Haojian Zhuang haojian.zhuang@linaro.org
ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c | 16 ++++++++-------- ArmPlatformPkg/Include/Drivers/PL061Gpio.h | 4 ---- 2 files changed, 8 insertions(+), 12 deletions(-)
diff --git a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c index ff05662..042fc76 100644 --- a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c +++ b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c @@ -125,7 +125,7 @@ Get ( } }
- if (MmioRead8 (PL061_GPIO_DATA_REG) & GPIO_PIN_MASK_HIGH_8BIT(Gpio)) {
- if (MmioRead8 (PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Gpio) << 2))) {
Shouldn't this be and OR "|", not a plus "+"?
Not knowing anything about what's going on: I don't think so. The above does not manipulate a register value, it calculates a register offset.
I realise that they probably achieve the same thing here, but using a "+" for masking is unusual.
It's not (value) masking, it's offset calculation.
(Whether the offset calculation is *correct* is of course over my head.)
The same comment applied to the other changes below:
Those changes are different:
*Value = 1;} else { *Value = 0; @@ -181,21 +181,21 @@ Set ( { case GPIO_MODE_INPUT: // Set the corresponding direction bit to LOW for input
MmioAnd8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK_LOW_8BIT(Gpio));
MmioAnd8 (PL061_GPIO_DIR_REG, ~GPIO_PIN_MASK(Gpio));
MmioAnd8() reads the register at the given offset, masks the value read, and then writes back the result.
break; case GPIO_MODE_OUTPUT_0: // Set the corresponding data bit to LOW for 0
MmioAnd8 (PL061_GPIO_DATA_REG, GPIO_PIN_MASK_LOW_8BIT(Gpio));
MmioWrite8 (PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Gpio) << 2), 0);
This replaces the read-mask-write triplet (using a constant offset) with a simple write (to a dynamically calculated offset).
// Set the corresponding direction bit to HIGH for output
MmioOr8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK_HIGH_8BIT(Gpio));
MmioOr8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK(Gpio)); break;
read-set-write, only the or-mask changes
case GPIO_MODE_OUTPUT_1: // Set the corresponding data bit to HIGH for 1
MmioOr8 (PL061_GPIO_DATA_REG, GPIO_PIN_MASK_HIGH_8BIT(Gpio));
MmioWrite8 (PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Gpio) << 2), 0xff);
replaces read-set-write to a constant register offset with a write to a dynamically calculated offset
etc
Thanks Laszlo
// Set the corresponding direction bit to HIGH for output
MmioOr8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK_HIGH_8BIT(Gpio));
MmioOr8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK(Gpio)); break;default:
@@ -250,9 +250,9 @@ GetMode ( }
// Check if it is input or output
- if (MmioRead8 (PL061_GPIO_DIR_REG) & GPIO_PIN_MASK_HIGH_8BIT(Gpio)) {
- if (MmioRead8 (PL061_GPIO_DIR_REG) & GPIO_PIN_MASK(Gpio)) { // Pin set to output
- if (MmioRead8 (PL061_GPIO_DATA_REG) & GPIO_PIN_MASK_HIGH_8BIT(Gpio)) {
- if (MmioRead8 (PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Gpio) << 2))) { *Mode = GPIO_MODE_OUTPUT_1; } else { *Mode = GPIO_MODE_OUTPUT_0;
diff --git a/ArmPlatformPkg/Include/Drivers/PL061Gpio.h b/ArmPlatformPkg/Include/Drivers/PL061Gpio.h index 38458f4..d436fd4 100644 --- a/ArmPlatformPkg/Include/Drivers/PL061Gpio.h +++ b/ArmPlatformPkg/Include/Drivers/PL061Gpio.h @@ -46,9 +46,5 @@
// All bits low except one bit high, native bit length #define GPIO_PIN_MASK(Pin) (1UL << ((UINTN)(Pin))) -// All bits low except one bit high, restricted to 8 bits (i.e. ensures zeros above 8bits) -#define GPIO_PIN_MASK_HIGH_8BIT(Pin) (GPIO_PIN_MASK(Pin) && 0xFF) -// All bits high except one bit low, restricted to 8 bits (i.e. ensures zeros above 8bits) -#define GPIO_PIN_MASK_LOW_8BIT(Pin) ((~GPIO_PIN_MASK(Pin)) && 0xFF)
#endif // __PL061_GPIO_H__
2.1.4
edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 19 August 2015 at 08:56, Haojian Zhuang haojian.zhuang@linaro.org wrote:
Support multiple PL061 controllers. If platform gpio driver couldn't be found, PL061 gpio driver will continue to load PcdPL061GpioBase as the register base. It could be compatible with the use case of current PL061 gpio driver.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Haojian Zhuang haojian.zhuang@linaro.org
ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c | 137 +++++++++++++++------ .../Drivers/PL061GpioDxe/PL061GpioDxe.inf | 1 + ArmPlatformPkg/Include/Drivers/PL061Gpio.h | 47 ++++--- 3 files changed, 125 insertions(+), 60 deletions(-)
diff --git a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c index 042fc76..02da8e1 100644 --- a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c +++ b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c @@ -19,6 +19,7 @@ #include <Library/BaseMemoryLib.h> #include <Library/DebugLib.h> #include <Library/IoLib.h> +#include <Library/MemoryAllocationLib.h> #include <Library/PcdLib.h> #include <Library/UefiBootServicesTableLib.h> #include <Library/UefiLib.h> @@ -28,6 +29,7 @@ #include <Drivers/PL061Gpio.h>
BOOLEAN mPL061Initialized = FALSE; +PLATFORM_GPIO_CONTROLLER *mPL061PlatformGpio;
/** Function implementations @@ -38,20 +40,36 @@ PL061Identify ( VOID ) {
- // Check if this is a PrimeCell Peripheral
- if ( (MmioRead8 (PL061_GPIO_PCELL_ID0) != 0x0D)
|| (MmioRead8 (PL061_GPIO_PCELL_ID1) != 0xF0)|| (MmioRead8 (PL061_GPIO_PCELL_ID2) != 0x05)|| (MmioRead8 (PL061_GPIO_PCELL_ID3) != 0xB1)) {- return EFI_NOT_FOUND;
- UINTN Index;
- UINTN RegisterBase;
- if ( (mPL061PlatformGpio->GpioCount == 0)
|| (mPL061PlatformGpio->GpioControllerCount == 0)) {return EFI_NOT_FOUND;
- // Check if this PrimeCell Peripheral is the PL061 GPIO
- if ( (MmioRead8 (PL061_GPIO_PERIPH_ID0) != 0x61)
|| (MmioRead8 (PL061_GPIO_PERIPH_ID1) != 0x10)|| ((MmioRead8 (PL061_GPIO_PERIPH_ID2) & 0xF) != 0x04)|| (MmioRead8 (PL061_GPIO_PERIPH_ID3) != 0x00)) {- return EFI_NOT_FOUND;
for (Index = 0; Index < mPL061PlatformGpio->GpioControllerCount; Index++) {
if (mPL061PlatformGpio->GpioController[Index].InternalGpioCount != PL061_GPIO_PINS) {
return EFI_INVALID_PARAMETER;}
RegisterBase = mPL061PlatformGpio->GpioController[Index].RegisterBase;
// Check if this is a PrimeCell Peripheral
if ( (MmioRead8 (RegisterBase + PL061_GPIO_PCELL_ID0) != 0x0D)
|| (MmioRead8 (RegisterBase + PL061_GPIO_PCELL_ID1) != 0xF0)|| (MmioRead8 (RegisterBase + PL061_GPIO_PCELL_ID2) != 0x05)|| (MmioRead8 (RegisterBase + PL061_GPIO_PCELL_ID3) != 0xB1)) {return EFI_NOT_FOUND;}
// Check if this PrimeCell Peripheral is the PL061 GPIO
if ( (MmioRead8 (RegisterBase + PL061_GPIO_PERIPH_ID0) != 0x61)
|| (MmioRead8 (RegisterBase + PL061_GPIO_PERIPH_ID1) != 0x10)|| ((MmioRead8 (RegisterBase + PL061_GPIO_PERIPH_ID2) & 0xF) != 0x04)|| (MmioRead8 (RegisterBase + PL061_GPIO_PERIPH_ID3) != 0x00)) {return EFI_NOT_FOUND;} }
return EFI_SUCCESS;
@@ -84,6 +102,31 @@ PL061Initialize ( return Status; }
+EFI_STATUS +EFIAPI +PL061Locate (
- IN EMBEDDED_GPIO_PIN Gpio,
- OUT UINTN *ControllerIndex,
- OUT UINTN *ControllerOffset,
- OUT UINTN *RegisterBase
- )
+{
- UINT32 Index;
- for (Index = 0; Index < mPL061PlatformGpio->GpioControllerCount; Index++) {
- if ( (Gpio >= mPL061PlatformGpio->GpioController[Index].GpioIndex)
&& (Gpio < mPL061PlatformGpio->GpioController[Index].GpioIndex+ mPL061PlatformGpio->GpioController[Index].InternalGpioCount)) {*ControllerIndex = Index;*ControllerOffset = Gpio % mPL061PlatformGpio->GpioController[Index].InternalGpioCount;*RegisterBase = mPL061PlatformGpio->GpioController[Index].RegisterBase;return EFI_SUCCESS;- }
- }
- DEBUG ((EFI_D_ERROR, "%a, failed to locate gpio %d\n", __func__, Gpio));
- return EFI_INVALID_PARAMETER;
+}
/**
Routine Description: @@ -110,11 +153,15 @@ Get ( ) { EFI_STATUS Status = EFI_SUCCESS;
- UINTN Index, Offset, RegisterBase;
- if ( (Value == NULL)
|| (Gpio > LAST_GPIO_PIN))- {
- return EFI_INVALID_PARAMETER;
Status = PL061Locate (Gpio, &Index, &Offset, &RegisterBase);
if (EFI_ERROR (Status))
goto EXIT;
if (Value == NULL) {
Status = EFI_INVALID_PARAMETER;
goto EXIT; }
// Initialize the hardware if not already done
@@ -125,7 +172,7 @@ Get ( } }
- if (MmioRead8 (PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Gpio) << 2))) {
- if (MmioRead8 (RegisterBase + PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Offset) << 2))) {
As per my comment on [PATCH 1/3], I think this should be a "|" for the mask, not a "+". So I think exra brackets would make sense here too:
+ if (MmioRead8 ((RegisterBase + PL061_GPIO_DATA_REG) | (GPIO_PIN_MASK(Offset) << 2))) {
I appreciate that the original code was using a "+", but I don't think it should have.
There are a few more later in this patch that should be changed too.
*Value = 1;} else { *Value = 0; @@ -162,12 +209,11 @@ Set ( ) { EFI_STATUS Status = EFI_SUCCESS;
- UINTN Index, Offset, RegisterBase;
- // Check for errors
- if (Gpio > LAST_GPIO_PIN) {
- Status = EFI_INVALID_PARAMETER;
- Status = PL061Locate (Gpio, &Index, &Offset, &RegisterBase);
- if (EFI_ERROR (Status)) goto EXIT;
}
// Initialize the hardware if not already done if (!mPL061Initialized) {
@@ -181,21 +227,21 @@ Set ( { case GPIO_MODE_INPUT: // Set the corresponding direction bit to LOW for input
MmioAnd8 (PL061_GPIO_DIR_REG, ~GPIO_PIN_MASK(Gpio));
MmioAnd8 (RegisterBase + PL061_GPIO_DIR_REG, ~GPIO_PIN_MASK(Gpio)); break;case GPIO_MODE_OUTPUT_0: // Set the corresponding data bit to LOW for 0
MmioWrite8 (PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Gpio) << 2), 0);
MmioWrite8 (RegisterBase + PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Offset) << 2), 0); // Set the corresponding direction bit to HIGH for output
MmioOr8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK(Gpio));
MmioOr8 (RegisterBase + PL061_GPIO_DIR_REG, GPIO_PIN_MASK(Offset)); break;case GPIO_MODE_OUTPUT_1: // Set the corresponding data bit to HIGH for 1
MmioWrite8 (PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Gpio) << 2), 0xff);
MmioWrite8 (RegisterBase + PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Offset) << 2), 0xff); // Set the corresponding direction bit to HIGH for output
MmioOr8 (PL061_GPIO_DIR_REG, GPIO_PIN_MASK(Gpio));
MmioOr8 (RegisterBase + PL061_GPIO_DIR_REG, GPIO_PIN_MASK(Offset)); break;default:
@@ -233,13 +279,12 @@ GetMode ( OUT EMBEDDED_GPIO_MODE *Mode ) {
- EFI_STATUS Status;
- EFI_STATUS Status;
- UINTN Index, Offset, RegisterBase;
- // Check for errors
- if ( (Mode == NULL)
|| (Gpio > LAST_GPIO_PIN)) {- return EFI_INVALID_PARAMETER;
- }
Status = PL061Locate (Gpio, &Index, &Offset, &RegisterBase);
if (EFI_ERROR (Status))
return Status;
// Initialize the hardware if not already done if (!mPL061Initialized) {
@@ -250,9 +295,9 @@ GetMode ( }
// Check if it is input or output
- if (MmioRead8 (PL061_GPIO_DIR_REG) & GPIO_PIN_MASK(Gpio)) {
- if (MmioRead8 (RegisterBase + PL061_GPIO_DIR_REG) & GPIO_PIN_MASK(Offset)) { // Pin set to output
- if (MmioRead8 (PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Gpio) << 2))) {
- if (MmioRead8 (RegisterBase + PL061_GPIO_DATA_REG + (GPIO_PIN_MASK(Offset) << 2))) { *Mode = GPIO_MODE_OUTPUT_1; } else { *Mode = GPIO_MODE_OUTPUT_0;
@@ -321,14 +366,34 @@ PL061InstallProtocol ( IN EFI_SYSTEM_TABLE *SystemTable ) {
- EFI_STATUS Status;
- EFI_HANDLE Handle;
EFI_STATUS Status;
EFI_HANDLE Handle;
GPIO_CONTROLLER *GpioController;
// // Make sure the Gpio protocol has not been installed in the system yet. // ASSERT_PROTOCOL_ALREADY_INSTALLED (NULL, &gEmbeddedGpioProtocolGuid);
Status = gBS->LocateProtocol (&gPlatformGpioProtocolGuid, NULL, (VOID **)&mPL061PlatformGpio);
if (EFI_ERROR (Status) && (Status == EFI_NOT_FOUND)) {
// Create the mPL061PlatformGpio
mPL061PlatformGpio = (PLATFORM_GPIO_CONTROLLER *)AllocateZeroPool (sizeof (PLATFORM_GPIO_CONTROLLER) + sizeof (GPIO_CONTROLLER));
if (mPL061PlatformGpio == NULL) {
DEBUG ((EFI_D_ERROR, "%a: failed to allocate PLATFORM_GPIO_CONTROLLER\n", __func__));return EFI_BAD_BUFFER_SIZE;}
mPL061PlatformGpio->GpioCount = PL061_GPIO_PINS;
mPL061PlatformGpio->GpioControllerCount = 1;
mPL061PlatformGpio->GpioController = (GPIO_CONTROLLER *)((UINTN) mPL061PlatformGpio + sizeof (PLATFORM_GPIO_CONTROLLER));
GpioController = mPL061PlatformGpio->GpioController;
GpioController->RegisterBase = (UINTN) PcdGet32 (PcdPL061GpioBase);
GpioController->GpioIndex = 0;
GpioController->InternalGpioCount = PL061_GPIO_PINS;
}
// Install the Embedded GPIO Protocol onto a new handle Handle = NULL; Status = gBS->InstallMultipleProtocolInterfaces(
diff --git a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061GpioDxe.inf b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061GpioDxe.inf index 9d9e4cd..405a3a9 100644 --- a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061GpioDxe.inf +++ b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061GpioDxe.inf @@ -45,6 +45,7 @@
[Protocols] gEmbeddedGpioProtocolGuid
- gPlatformGpioProtocolGuid
[Depex] TRUE diff --git a/ArmPlatformPkg/Include/Drivers/PL061Gpio.h b/ArmPlatformPkg/Include/Drivers/PL061Gpio.h index d436fd4..356428a 100644 --- a/ArmPlatformPkg/Include/Drivers/PL061Gpio.h +++ b/ArmPlatformPkg/Include/Drivers/PL061Gpio.h @@ -19,30 +19,29 @@ #include <Protocol/EmbeddedGpio.h>
// PL061 GPIO Registers -#define PL061_GPIO_DATA_REG ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0x000) -#define PL061_GPIO_DIR_REG ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0x400) -#define PL061_GPIO_IS_REG ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0x404) -#define PL061_GPIO_IBE_REG ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0x408) -#define PL061_GPIO_IEV_REG ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0x40C) -#define PL061_GPIO_IE_REG ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0x410) -#define PL061_GPIO_RIS_REG ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0x414) -#define PL061_GPIO_MIS_REG ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0x410) -#define PL061_GPIO_IC_REG ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0x41C) -#define PL061_GPIO_AFSEL_REG ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0x420)
-#define PL061_GPIO_PERIPH_ID0 ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0xFE0) -#define PL061_GPIO_PERIPH_ID1 ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0xFE4) -#define PL061_GPIO_PERIPH_ID2 ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0xFE8) -#define PL061_GPIO_PERIPH_ID3 ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0xFEC)
-#define PL061_GPIO_PCELL_ID0 ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0xFF0) -#define PL061_GPIO_PCELL_ID1 ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0xFF4) -#define PL061_GPIO_PCELL_ID2 ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0xFF8) -#define PL061_GPIO_PCELL_ID3 ((UINT32)PcdGet32 (PcdPL061GpioBase) + 0xFFC)
-// GPIO pins are numbered 0..7 -#define LAST_GPIO_PIN 7 +#define PL061_GPIO_DATA_REG 0x000 +#define PL061_GPIO_DIR_REG 0x400 +#define PL061_GPIO_IS_REG 0x404 +#define PL061_GPIO_IBE_REG 0x408 +#define PL061_GPIO_IEV_REG 0x40C +#define PL061_GPIO_IE_REG 0x410 +#define PL061_GPIO_RIS_REG 0x414 +#define PL061_GPIO_MIS_REG 0x410 +#define PL061_GPIO_IC_REG 0x41C +#define PL061_GPIO_AFSEL_REG 0x420
+#define PL061_GPIO_PERIPH_ID0 0xFE0 +#define PL061_GPIO_PERIPH_ID1 0xFE4 +#define PL061_GPIO_PERIPH_ID2 0xFE8 +#define PL061_GPIO_PERIPH_ID3 0xFEC
+#define PL061_GPIO_PCELL_ID0 0xFF0 +#define PL061_GPIO_PCELL_ID1 0xFF4 +#define PL061_GPIO_PCELL_ID2 0xFF8 +#define PL061_GPIO_PCELL_ID3 0xFFC
+#define PL061_GPIO_PINS 8
// All bits low except one bit high, native bit length
#define GPIO_PIN_MASK(Pin) (1UL << ((UINTN)(Pin)))
2.1.4
edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
-
Haojian Zhuang -
Laszlo Ersek -
Ryan Harkin