tests: gpio: Add check to validate initial values of gpio output

[albertofloyd]

Is your enhancement proposal related to a problem? Please describe.
Issue like this could be catch in sanity check for specific-boards.
#25101

Describe the solution you'd like
Current gpio test needs to add an additional test step where the following is done.

1. Configure gpio pins using GPIO flags introduced in v2.2 such as ACTIVE_HIGH or ACTIVE_LOW
2. Check via loopback that gpio input matches the expected value after gpio_configure.
3. Need to perform this check before start toggling gpio using set_raw()

Additional context
In some some boards, some GPIO are not explicitly set using set_raw_xx() and they rely in initial configuration from device tree values.
If gpio drivers have problems to set initial values, this can lead to increase power consumption and/or incorrect behavior.

[maksimmasalski]

@albertofloyd I understand that issue relates to an Input pin of the Microchip board.

1. It is possible to setup input pin in .overlay file using flag GPIO_ACTIVE_HIGH like this:

in-gpios  = <&gpio_140_176 14 0>; /* GPIO_156, JP31 Pin 13 */
out-gpios = <&gpio_140_176 15 GPIO_ACTIVE_HIGH>; /* GPIO_157, JP31 Pin 15 */

That setup will define that if pin receives HIGH signal, on logical level it will be 1 (as common).
2. Also possible to setup it in .c file like this. Some tests are using that configuration and others not:
rc = gpio_pin_configure(dev, PIN_OUT, GPIO_ACTIVE_HIGH | GPIO_OUTPUT_HIGH);

What do you mean "Check via loopback"? I can't understand that process. I see in the code there are already zassert to check gpio_pin_configure output.

[albertofloyd]

Test enhancement is not MCHP-specific, though the issue was found in MCHP board.

The enhancement is to update gpio test case so gpio initialization issues not taking effect can be caught.

Current oversimplified gpio test case sequence

1. Configure input gpio A
2. Register callback for gpio A and enable interrupt type T
3. Configure output gpio B i.e. HIGH per dts overlay
4. Toggle N times gpio B (since gpio B output is connected to gpio A, loopback )
   Each toggle results in a callback
5. Check N callbacks were received

Original issue in MCHP board.
At 3) gpio B wasn't HIGH due to driver issue, but test doesn't perform any checks, hence issue is found on app side testing

Suggested enhancement
3) Configure output gpio B i.e. HIGH per dts overlay
3a) Read gpio A and compare against expected value for B
Since A is connected to gpio B in most HW setups this will ensure gpio configuration flags do reflect.

[maksimmasalski]

Test enhancement is not MCHP-specific, though the issue was found in MCHP board.

The enhancement is to update gpio test case so gpio initialization issues not taking effect can be caught.

Current oversimplified gpio test case sequence

1. Configure input gpio A
2. Register callback for gpio A and enable interrupt type T
3. Configure output gpio B i.e. HIGH per dts overlay
4. Toggle N times gpio B (since gpio B output is connected to gpio A, loopback )
   Each toggle results in a callback
5. Check N callbacks were received

Original issue in MCHP board.
At 3) gpio B wasn't HIGH due to driver issue, but test doesn't perform any checks, hence issue is found on app side testing

Suggested enhancement
3) Configure output gpio B i.e. HIGH per dts overlay
3a) Read gpio A and compare against expected value for B
Since A is connected to gpio B in most HW setups this will ensure gpio configuration flags do reflect.

Thank you for a such detailed explanation, I'm working on it.

[maksimmasalski]

@albertofloyd Please take a look at my PR #26934

[albertofloyd]

@maksimmasalski
Looks like you are checking input after gpio_set() which I believe is already covered somewhere else in the test.

The idea was to perform get_pin() operation immediate after gpio_pin_configure() which seems to be part of latest test.
What remains is to understand why this step didn't catch the issue, or if latest code was added recently

Latest test code

rc = gpio_pin_configure(dev, PIN_OUT, GPIO_OUTPUT_HIGH);
zassert_equal(rc, 0,
	      "pin config output high failed");

   /* Check output is HIGH after configure */
rc = gpio_port_get_raw(dev, &v1);
zassert_equal(rc, 0,
	      "get raw high failed");
if (raw_in() != true) {
	TC_PRINT("FATAL output pin not wired to input pin? (out high => in low)\n");
	while (true) {
		k_sleep(K_FOREVER);
	}
}

[maksimmasalski]

rc = gpio_pin_configure(dev, PIN_OUT, GPIO_OUTPUT_HIGH);

I understand that check is already performed, but it is not working. Seems like deep underlying issue.

P.S. Next week I will have vacation, so can't continue work on it and find out the root cause.

[pabigot]

@albertofloyd agreed, see my summary at #26934 (comment). It's possible that 2f8167f fixed the problem.

[pabigot]

Unless the problem can be reproduced by reverting #25103 and re-testing I think this should just be closed.

[albertofloyd]

@pabigot Original issue was fixed with #25103
However, the problem was not caught back then with the gpio case but in our app development, hence this test request enhancement.

What could it be done is revert #25103 and see if latest test code catches the issue.

[pabigot]

@albertofloyd Exactly; please do that. I would, but I don't have the hardware.

[albertofloyd]

Reverted #25103 and test gpio basic test still passes.
Per my analysis this is because tests uses GPIO_OUTPUT_LOW only.
Modifying as below, tests fails without #25103 as needed.

However, I think a more robust case test approach would be to configure using both flags supported by API. GPIO_OUTPUT_LOW, GPIO_OUTPUT_HIGH
GPIO_OUTPUT_LOW may need to avoid false positive where pull-up jumpers do exist.

@@ -83,7 +83,7 @@ static int setup(void)

        TC_PRINT("Check %s output %d connected to input %d\n", DEV_NAME,
                 PIN_OUT, PIN_IN);
-       rc = gpio_pin_configure(dev, PIN_OUT, GPIO_OUTPUT_LOW);
+       rc = gpio_pin_configure(dev, PIN_OUT, GPIO_OUTPUT_HIGH);
        zassert_equal(rc, 0,
                      "pin config output low failed");

@@ -94,7 +94,7 @@ static int setup(void)
        rc = gpio_port_get_raw(dev, &v1);
        zassert_equal(rc, 0,
                      "get raw low failed");
-       if (raw_in() != false) {
+       if (raw_in() != true) {
                TC_PRINT("FATAL output pin not wired to input pin? (out low => in high)\n");
                while (true) {
                        k_sleep(K_FOREVER);

[pabigot]

@albertofloyd I'm still confused. Lines 84 through 105 test output low. Lines 107 through 121 test output high.

If I make the changes you suggest the test fails on nrf52840dk_nrf52840:

START - test_gpio_port
Validate device GPIO_1
Check GPIO_1 output 1 connected to input 2

    Assertion failed at ../src/test_gpio_port.c:104: setup: (v1 & BIT(PIN_IN) n)
out low does not read low
 FAIL - test_gpio_port

which is expected (the changes are incomplete). If I add:

-       zassert_equal(v1 & BIT(PIN_IN), 0,
+       zassert_equal(v1 & BIT(PIN_IN), BIT(PIN_IN),
                      "out low does not read low");

the tests pass, but they're redundant with the tests that follow immediately, and we're not checking whether setting output to low initially succeeds.

Either high or low needs to be tested first; swapping them might detect a problem on xec hardware but then miss a different problem that would only show up if low was tested first. Or are you suggesting we test one, then the other, then the first again? I can see an argument that might have some effect.

But also the test could reconfigure both test pins to disconnected (where supported) between checking high and low, in case there's something about explicitly setting the direction that changes the behavior. I've opened #27218 to see whether that has any effect. Please test it with and without #25103.

[albertofloyd]

@pabigot
My bad. Let me explain in detail why I think the test case doesn't catch the MCHP driver bug.

• XEC HW requires configuration to occur before setting output value, so any output value has no effect if pin is not configured.

Current test sequence

1. Configure OUTPUT_PIN as OUTPUT_LOW
   a. gpio driver setting for output to 0 but has no effect
   b. gpio driver perform pin configuration as output
2. Check INPUT_PIN, since default is LOW check succeeds (since default is LOW)
3. Configure pin OUTPUT_HIGH
   a. gpio driver setting for output has effect since pin is already during (1.a)
   b. gpio driver perform pin configuration as output
4. Check INPUT_PIN, since this time setting took effect, check succeeds again.

What I meant (but fail to conveyed) was to reverse 1 and 3 to catch the bug.
Since as long as there 2 calls to gpio_configure the second will always take effect.

#27218 looks good will try it tomorrow

[pabigot]

• XEC HW requires configuration to occur before setting output value, so any output value has no effect if pin is not configured.

Thanks; that's something I hypothesized but didn't know for sure.

What I meant (but fail to conveyed) was to reverse 1 and 3 to catch the bug.

Swapping the order wouldn't be sufficient: there are GPIO peripherals where the default output value is high (SX1509B at power-up, for example): if OUTPUT_HIGH went first the problem with XEC would be detected, but an equivalent problem in another driver might be missed.