Expander GPIO: Initial state not set correctly when configuring as output and low active

[agners]

Describe the bug

It seems that external GPIOs (controlled through firmware) do not apply the initial state correctly.

I am using the following device tree snippet

	pwr_led: led-pwr {
		label = "PWR";
		linux,default-trigger = "none";
		default-state = "off";
		gpios = <&expgpio 2 GPIO_ACTIVE_LOW>;
	};

The expectation is that this makes the LED to be off by default.

The leds-gpio driver uses the gpiod API to initialize the GPIO correctly:
https://elixir.bootlin.com/linux/v5.15/source/drivers/leds/leds-gpio.c#L109

The gpiod_direction_output API takes care of the polarity flag, and requests a high output from the GPIO driver:
https://elixir.bootlin.com/linux/v5.15/source/drivers/gpio/gpiolib.c#L2324

Currently, the value (1) is directly passed to the firmware in gpio-raspberrypi-exp.c. The state of the GPIO however is not changed.

It seems that the firmware is aware of the polarity configuration on its own, and does not wrongly applies the new value?

This change fixes the problem for that particular use case, but I am not sure if that is correct:

@@ -118,6 +118,9 @@ static int rpi_exp_gpio_dir_out(struct gpio_chip *gc, unsigned int off, int val)
        if (ret < 0)
                return ret;
        set_out.polarity = ret;         /* Retain existing setting */
+       if (set_out.polarity)
+               set_out.state = !val;
 
        ret = rpi_firmware_property(gpio->fw, RPI_FIRMWARE_SET_GPIO_CONFIG,
                                    &set_out, sizeof(set_out));

What is interesting to me is that reading back the GPIO seems to return the level "unaltered":

[    1.643491] rpi_exp_gpio_get, off 2, state 1

Steps to reproduce the behaviour

Apply the following device tree change:

	pwr_led: led-pwr {
		label = "PWR";
		linux,default-trigger = "none";
		default-state = "off";
		gpios = <&expgpio 2 GPIO_ACTIVE_LOW>;
	};

Observe that the red LED remains on.

Device (s)

Raspberry Pi CM4

System

This is on Home Assistant OS. We are using the latest 5.15 downstream kernel from this repository. As this is a pure kernel issue, this should be the very same on a regular Raspberry Pi.

# uname -a
Linux homeassistant 5.15.32-v8 #1 SMP PREEMPT Thu Jul 28 14:06:36 UTC 2022 aarch64 HAOS

# dmesg | grep firmware
[    0.108313] raspberrypi-firmware soc:firmware: Attached to firmware from 2022-03-24T13:19:26, variant start
[    0.112338] raspberrypi-firmware soc:firmware: Firmware hash is e5a963efa66a1974127860b42e913d2374139ff5

Logs

No response

Additional context

No response

[pelwell]

If you declare the GPIO as ACTIVE_LOW then "off" gives a high logic level. Change one or the other to get an initial low logic level.

[agners]

If you declare the GPIO as ACTIVE_LOW then "off" gives a high logic level.

Yes, that is the theory. Unfortunately, it doesn't work that way. When ACTIVE_LOW and "off" is given, the logic level is low (which is wrong). This leads to the LED to be on (since the GPIO is connected on the cathode side of the LED).

[agners]

FWIW, the polarity is ACTIVE_LOW by default for the power LED. So using the following should turn off both LEDs.

dtparam=act_led_trigger=none
dtparam=pwr_led_trigger=none

But that is not the case: It does turn off the activity LED, but the power LED stays on. I first thought that default-state = "keep" is the reason, using default-state = "off" does not help (which essentially is the problematic configuration I'd like to report here).

Side note: Most guides suggest to also set pwr_led_activelow=off, e.g. to turn off both LEDs, the suggestion is:

# Disable the ACT LED.
dtparam=act_led_trigger=none
dtparam=act_led_activelow=off

# Disable the PWR LED.
dtparam=pwr_led_trigger=none
dtparam=pwr_led_activelow=off

For the activity LED, this has no effect as it is high active by default.

However, for the power LED this changes the GPIO setting to be GPIO_ACTIVE_HIGH (since off = 0 = GPIO_ACTIVE_HIGH).

/* Bit 0 express polarity */
#define GPIO_ACTIVE_HIGH 0
#define GPIO_ACTIVE_LOW 1

Currently, this makes the power LED to be off. But it is wrong, the PWR LED is not a high active GPIO. But setting power LED low active should cause the LED to be on!

[pelwell]

The expectation is that this makes the LED to be off by default.

If you declare the GPIO as ACTIVE_LOW then "off" gives a high logic level.
Yes, that is the theory.

Those two statements are contradictory. Explain.

[agners]

The expectation is that this makes the LED to be off by default.

		default-state = "off";
		gpios = <&expgpio 2 GPIO_ACTIVE_LOW>;

I expect it to be off, but when I do this, the LED is on (means logic level high).

I think we are in agreement what we expect:

If you declare the GPIO as ACTIVE_LOW then "off" gives a high logic level.

"gives a high logic level." -> LED off.

I don't see the contradiction?

We agree that

• GPIO logic level low equals LED on
• GPIO logic level high equals LED off

Correct?

You said:

If you declare the GPIO as ACTIVE_LOW then "off" gives a high logic level.

I assume with "then off" you meant default-state = "off", correct?

So, GPIO_ACTIVE_LOW + default-state = "off", should lead to a GPIO level high, and a LED which is off.

That is not the case.

[6by9]

The firmware does have a notion of polarity for itself.

https://github.com/raspberrypi/firmware/blob/master/extra/dt-blob.dts#L1768

            pin@p130 { function = "output"; termination = "no_pulling"; polarity = "active_low"; startup_state = "active"; }; // PWR_LED (RED)

So &expgpio 2 is defined as being inverted within the firmware.

Memory says that the set/get state calls were meant to pass through directly though, and the polarity was only to retain the state (as was required by the older gpio API).

Testing for myself:
Using https://github.com/6by9/rpi3-gpiovirtbuf as my quick and dirty hacking tool, rpi3-gpiovirtbuf s 130 0 does indeed turn a CM4 power LED on, and rpi3-gpiovirtbuf s 130 1 turns it off.

Checking with a multimeter, s 130 1 sets pin 2 of the 74LVC1G07 driver to 3.3V, and s 130 0 sets pin 2 to 0 volts, so there is no inversion there.

The datasheet of the 74LVC1G07 fig 3 shows an inverter before the open-drain output, therefore 1 / 3V3 / (inactive with ACTIVE_LOW) will be inverted to drive the FETs gate at 0V, and hence the LED is off.
0 / 0V / (active with ACTIVE_LOW) will be inverted to "high" to drive the FET, and the LED is on.

As you say, the kernel for CM4 is configured with

		led-pwr {
			label = "PWR";
			gpios = <&expgpio 2 GPIO_ACTIVE_LOW>;
			default-state = "keep";
			linux,default-trigger = "default-on";
		};

and using /sys/class/leds/led1/brightness, 0 turns the LED off, and 1 turns it on, so it is doing the appropriate inversion based on that ACTIVE_LOW flag.

So that largely confirms to me that whilst the firmware does have a notion of polarity, it is bypassed when driven by Linux, and that is behaving as it should.

The only difference I can see with your DT snippet is that you're setting it to default-state = "off"; instead of keep. So is it only that initial state that you're worrying about?

[6by9]

Ah, it looks to be the firmware on that initial set. It passes through all the parameters via a different path, and that does apply a polarity correction which we don't want.
I'll look at it in more detail tomorrow.

[agners]

Ah, it looks to be the firmware on that initial set. It passes through all the parameters via a different path, and that does apply a polarity correction which we don't want.

That very much looks like what I am observing indeed.

It is confusing as subsequent GPIO sets/clears seem to work.

I'll look at it in more detail tomorrow.

Very much appreciated, thank you!

[6by9]

Test firmware for 2711 based boards (Pi4/CM4/Pi400) that should solve this at https://drive.google.com/file/d/1CI2wzuYDSWa4ewSm8Spw4ztI2xp5HL0r/view?usp=sharing

The same fix will apply to the other boards as well, but that will come with the full release.

[agners]

@6by9 thanks, just copied the firmware on my board and I can confirm the power LED is now behaving as expected!

I am a bit hesitant to deploy this firmware in production. I'll wait until it makes it into stable and gets a bit wider testing. I use a kernel level work around meanwhile.

From my side, this issue can be closed. Thanks!

[6by9]

Firmware change has been merged internally, so should be in the next rpi-update release.