The PL061 GPIO controller is a bit of an anachronism, and the existing driver does nothing to hide this - leading to it being very tricky to read.
Rewrite it to document (in comments and code) what is actually happening, and fix some bugs in the process. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Leif Lindholm <[email protected]> --- ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c | 70 +++++++++++++++++++++---- ArmPlatformPkg/Include/Drivers/PL061Gpio.h | 6 +-- 2 files changed, 61 insertions(+), 15 deletions(-) diff --git a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c index 7fa9ab6..4bfb8de 100644 --- a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c +++ b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c @@ -28,6 +28,56 @@ #include <Protocol/EmbeddedGpio.h> #include <Drivers/PL061Gpio.h> +// +// The PL061 is a strange beast. The 8-bit data register is aliased across a +// region 0x400 bytes in size, with bits [9:2] of the address operating as a +// mask for both read and write operations: +// For reads: +// - All bits where their corresponding mask bit is 1 return the current +// value of that bit in the GPIO_DATA register. +// - All bits where their corresponding mask bit is 0 return 0. +// For writes: +// - All bits where their corresponding mask bit is 1 set the bit in the +// GPIO_DATA register to the written value. +// - All bits where their corresponding mask bit is 0 are left untouched +// in the GPIO_DATA register. +// +// To keep this driver intelligible, PL061EffectiveAddress, PL061GetPins and +// Pl061SetPins provide an internal abstraction from this interface. + +STATIC +UINTN +EFIAPI +PL061EffectiveAddress ( + IN UINTN Address, + IN UINTN Mask + ) +{ + return ((Address + PL061_GPIO_DATA_REG_OFFSET) + (Mask << 2)); +} + +STATIC +UINTN +EFIAPI +PL061GetPins ( + IN UINTN Address, + IN UINTN Mask + ) +{ + return MmioRead8 (PL061EffectiveAddress (Address, Mask)); +} + +STATIC +VOID +EFIAPI +PL061SetPins ( + IN UINTN Address, + IN UINTN Mask, + IN UINTN Value + ) +{ + MmioWrite8 (PL061EffectiveAddress (Address, Mask), Value); +} /** Function implementations @@ -113,7 +163,7 @@ Get ( return EFI_INVALID_PARAMETER; } - if (MmioRead8 (PL061_GPIO_DATA_REG) & GPIO_PIN_MASK_HIGH_8BIT(Gpio)) { + if (PL061GetPins (PL061_GPIO_DATA_REG, Gpio)) { *Value = 1; } else { *Value = 0; @@ -160,21 +210,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) & 0xFF); 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)); // 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)); + // Set the corresponding data bit to LOW for 0 + PL061SetPins (PL061_GPIO_DATA_REG, GPIO_PIN_MASK(Gpio), 0); 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)); // 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)); + // Set the corresponding data bit to HIGH for 1 + PL061SetPins (PL061_GPIO_DATA_REG, GPIO_PIN_MASK(Gpio), 1); break; default: @@ -219,9 +269,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 (PL061GetPins (PL061_GPIO_DATA_REG, GPIO_PIN_MASK(Gpio))) { *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..9da68bd 100644 --- a/ArmPlatformPkg/Include/Drivers/PL061Gpio.h +++ b/ArmPlatformPkg/Include/Drivers/PL061Gpio.h @@ -19,6 +19,7 @@ #include <Protocol/EmbeddedGpio.h> // PL061 GPIO Registers +#define PL061_GPIO_DATA_REG_OFFSET ((UINTN) 0x000) #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) @@ -40,15 +41,10 @@ #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 // 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 [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
