From 328d8cfa6278a5558ce510662df73f4c17086567 Mon Sep 17 00:00:00 2001 From: Leif Lindholm Date: Wed, 28 Oct 2015 18:11:49 +0000 Subject: ArmPlatformPkg: PL061 - rewrite the hardware interaction 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 Reviewed-by: Ard Biesheuvel Reviewed-by: Ryan Harkin --- ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c | 72 +++++++++++++++++++++---- ArmPlatformPkg/Include/Drivers/PL061Gpio.h | 5 +- 2 files changed, 62 insertions(+), 15 deletions(-) diff --git a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c index 45897ca753..38341a3d05 100644 --- a/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c +++ b/ArmPlatformPkg/Drivers/PL061GpioDxe/PL061Gpio.c @@ -28,6 +28,56 @@ #include #include +// +// 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 @@ -88,7 +138,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; @@ -135,22 +185,22 @@ 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)); - break; + 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), 0xff); + break; default: // Other modes are not supported @@ -194,9 +244,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 38458f4844..8fde2bb5ef 100644 --- a/ArmPlatformPkg/Include/Drivers/PL061Gpio.h +++ b/ArmPlatformPkg/Include/Drivers/PL061Gpio.h @@ -19,6 +19,7 @@ #include // 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) @@ -46,9 +47,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__ -- cgit v1.2.3