On Tue, 2015-08-18 at 10:45 +0200, Ard Biesheuvel wrote:
On 18 August 2015 at 10:39, 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..e8a2094 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));This should be ~GPIO_PIN_MASK(Gpio)
OK. I'll fix it.
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), GPIO_PIN_MASK(Gpio));You should write the value 0x0 here: this will only affect the GPIO that you selected, so it will clear only a single bit
All these are already fixed in v3 patches. But I miss to merge them into the 3rd patch. I'll reformat the patches.
And these other 2 patches of v3 are sent separately since I met the issue of sending email. I have to resend them again.
Regards Haojian