Paths

Table of Contentst

Differential D47331

acpi_gpiobus: IoRestriction is only for type IO
AcceptedPublic
Actions

Authored by cperciva on Oct 29 2024, 5:07 PM.

Details

Reviewers

jrtc27

Summary

The IoRestriction field of ACPI_RESOURCE_GPIO is only meaningful
for resources of type ACPI_RESOURCE_GPIO_TYPE_IO.

Reported by: jrtc27

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 60249
Build 57133: arc lint + arc unit

Event Timeline

cperciva created this revision.Oct 29 2024, 5:07 PM

Herald added subscribers: loos, imp. · View Herald TranscriptOct 29 2024, 5:07 PM

cperciva requested review of this revision.Oct 29 2024, 5:07 PM

Harbormaster completed remote builds in B60249: Diff 145610.Oct 29 2024, 5:07 PM

Don't we need to set GPIO_PIN_INPUT for interrupts?

Technically should have a blank line between the ifs, though that was true before.

c.f. https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/06_Device_Configuration/Device_Configuration.html#gpio-connection-descriptor

This revision is now accepted and ready to land.Oct 29 2024, 5:18 PM

In D47331#1079365, @andrew wrote:

Don't we need to set GPIO_PIN_INPUT for interrupts?

Depends what our framework is expecting I guess, but that would seem sensible to me.

Hmm, I don't actually see anything in the ACPI spec saying that IoRestriction only applies for ACPI_RESOURCE_GPIO_TYPE_IO. So I think we have three options here:

Go ahead with this patch, on the basis of "this is what Linux does",
Force GPIO_PIN_INPUT for ACPI_RESOURCE_GPIO_TYPE_INT on the basis of "if we're generating interrupts obviously it's an input", or
Just parse the values we're given and hope they make sense.

@andrew, @jrtc27, any preference?

In D47331#1079374, @cperciva wrote:

Hmm, I don't actually see anything in the ACPI spec saying that IoRestriction only applies for ACPI_RESOURCE_GPIO_TYPE_IO. So I think we have three options here:

Go ahead with this patch, on the basis of "this is what Linux does",

Force GPIO_PIN_INPUT for ACPI_RESOURCE_GPIO_TYPE_INT on the basis of "if we're generating interrupts obviously it's an input", or

Just parse the values we're given and hope they make sense.

@andrew, @jrtc27, any preference?

This table in the link I posted above:

In D47331#1079376, @jrtc27 wrote:

In D47331#1079374, @cperciva wrote:

Hmm, I don't actually see anything in the ACPI spec saying that IoRestriction only applies for ACPI_RESOURCE_GPIO_TYPE_IO. So I think we have three options here:

Go ahead with this patch, on the basis of "this is what Linux does",

Force GPIO_PIN_INPUT for ACPI_RESOURCE_GPIO_TYPE_INT on the basis of "if we're generating interrupts obviously it's an input", or

Just parse the values we're given and hope they make sense.

@andrew, @jrtc27, any preference?

This table in the link I posted above:

(Note that interrupts have a separate definition for byte 7, with some common fields, but the I/O Restriction bits are replaced by the Interrupt Mode and half the Interrupt Polarity)

In D47331#1079376, @jrtc27 wrote:

This table in the link I posted above:

Aha, I didn't notice the text "for IO Connections" .

Revision Contents
Changeset List

Path

Size

sys/

dev/

gpio/

acpi_gpiobus.c

16 lines

Diff 145610

View Options

sys/dev/gpio/acpi_gpiobus.c

acpi_gpiobus: IoRestriction is only for type IOAcceptedPublicActions