Ambiguity of Line.request()'s default arg polarity in light of ACTIVE_LOW outputs

[JonathonReinhart]

Background

Line.request() accepts LineRequestFlags including ACTIVE_LOW. This inverts the polarity of the value in the LineHandle::get_value() and LineHandle::set_value() APIs to mean:

• 0 -> inactive -> electrically high
• 1 -> active -> electrically low

Problem

Line.request() also accepts a default argument which is currently documented as:

For an output, the default parameter specifies the value the line should have when it is configured as an output.

The polarity of this argument is not clear.

If I do this:

line.request(LineRequestFlags.ACTIVE_LOW, 0, "me");

does 0 mean:

• inactive -> electrically high
• electrically low

Somewhat related: #49

[JonathonReinhart]

Initially, I would expect the polarity of default to match that of set_value(value): its polarity should respect ACTIVE_LOW.

---

cpio-cdev doesn't do anything with default other than stick it in the default_values array of the gpiohandle_request.

In the Linux uAPI, default_values is documented as:

* @default_values: if the %GPIOHANDLE_REQUEST_OUTPUT is set for a requested
* line, this specifies the default output value, should be 0 (low) or
* 1 (high), anything else than 0 or 1 will be interpreted as 1 (high)

This is still not abundantly clear, and by saying "0 (low)", implies the opposite of what I would have expected: That this value is unaffected by ACTIVE_LOW.

[JonathonReinhart]

Kernel Analysis

linehandle_create() -- the function which handles the GPIO_GET_LINEHANDLE_IOCTL invoked by request() -- calls gpiod_direction_output() to put the GPIO in output mode. This includes the !! boolean-ified default_value:

		if (lflags & GPIOHANDLE_REQUEST_OUTPUT) {
			int val = !!handlereq.default_values[i];

			ret = gpiod_direction_output(desc, val);

gpiod_direction_output() will then invert value if necessary (according to FLAG_ACTIVE_LOW) before passing it to gpiod_direction_output_raw_commit():

/**
 * gpiod_direction_output - set the GPIO direction to output
 * @desc:	GPIO to set to output
 * @value:	initial output value of the GPIO
 *
 * Set the direction of the passed GPIO to output, such as gpiod_set_value() can
 * be called safely on it. The initial value of the output must be specified
 * as the logical value of the GPIO, i.e. taking its ACTIVE_LOW status into
 * account.
 *
 * Return 0 in case of success, else an error code.
 */
int gpiod_direction_output(struct gpio_desc *desc, int value)
{
	int ret;

	VALIDATE_DESC(desc);
	if (test_bit(FLAG_ACTIVE_LOW, &desc->flags))
		value = !value;
	else
		value = !!value;

	// ...

	return gpiod_direction_output_raw_commit(desc, value);

Conclusion: default_values do respect GPIOHANDLE_REQUEST_ACTIVE_LOW.

[JonathonReinhart]

Empirical Analysis

I have a board with an active-low output that drives a reset line.

Example code 1 (good)

const COMMON_DELAY: Duration = Duration::from_millis(1);

reset_hand = line.request(
    gpio_cdev::LineRequestFlags::OUTPUT | gpio_cdev::LineRequestFlags::ACTIVE_LOW,
    0, // Inactive (high)
    GPIO_CONSUMER
)

thread::sleep(Duration::from_millis(3000));  // For testing

// Put chip in reset
reset_hand.set_value(1)?
thread::sleep(COMMON_DELAY);

// ... other stuff

// Bring chip out of reset
reset_hand.set_value(1)?

With that code, I observe no glitches. I see the line start high, stay there for the 3 second pause, go low for the expected amount of time (1-2 ms), then return high.

That tells me that there were no unexpected transitions due to an incorrect initial value.

Example code 2 (bad)

If I make the following change:

 reset_hand = line.request(
     gpio_cdev::LineRequestFlags::OUTPUT | gpio_cdev::LineRequestFlags::ACTIVE_LOW,
-    0, // Inactive (high)
+    1, // WRONG: Incorrectly assumes this means electrically high
     GPIO_CONSUMER
 )

Then I see my reset line drop immediately, and stay low for the entire 3 second pause, before returning high.

This tells me that the 1 initial value meant "active" and drove my line low.

---

Conclusion: default does respect ACTIVE_LOW.

[JonathonReinhart]

Summary:

• Both the kernel source analysis and empirical analysis come to the same conclusion: default does respect ACTIVE_LOW.
• The kernel uAPI docs could be improved in this regard.
• The gpio-cdev docs could be improved in this regard.

[warthog618]

* Both the [kernel source analysis](https://github.com/rust-embedded/gpio-cdev/issues/78#issuecomment-1861562611) and [empirical analysis](https://github.com/rust-embedded/gpio-cdev/issues/78#issuecomment-1861642126) come to the same conclusion: `default` _does_ respect `ACTIVE_LOW`.

* The kernel uAPI docs could be improved in this regard.

Thanks for the feedback. Just curious - if the kernel did NOT honor the active low flag then what purpose would the flag serve? I've always thought that the very existence of that flag implies that line values are logical, not physical.

Perhaps the documentation could be clearer, specifically that line values are logical not physical and that the active low flag controls the mapping between the two, but uAPI v1 is deprecated so it doesn't get much attention.

Hopefully the uAPI v2 documentation is a bit clearer - it uses active and inactive for the logical states and avoids mention of high and low (the physical states):

 * @GPIO_V2_LINE_FLAG_ACTIVE_LOW: line active state is physical low

...

 * struct gpio_v2_line_values - Values of GPIO lines
 * @bits: a bitmap containing the value of the lines, set to 1 for active
 * and 0 for inactive

Similarly, my gpiocdev library also uses the active/inactive terminology for line values, and documents the logical/physical mapping.

I'll see about a patch for the uAPI v1 docs to replace low/high with inactive/active.

Update: documentation patches submitted to the kernel and on track to make it into 6.9.