Page MenuHomeFreeBSD

acpi_gpiobus: Use non-rman resource activation
AbandonedPublic

Authored by cperciva on Oct 22 2024, 6:59 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Dec 3, 10:58 PM
Unknown Object (File)
Tue, Dec 3, 10:58 PM
Unknown Object (File)
Tue, Dec 3, 10:36 PM
Unknown Object (File)
Mon, Dec 2, 11:40 AM
Unknown Object (File)
Sun, Nov 24, 11:16 PM
Unknown Object (File)
Thu, Nov 14, 9:51 PM
Unknown Object (File)
Thu, Nov 14, 6:12 PM
Unknown Object (File)
Nov 6 2024, 10:21 AM
Subscribers

Details

Reviewers
wulf
jhb
Summary

bus_generic_activate_resource passes requests up to the parent device
and ultimately ACPI will do what we need for us.

This overrides gpiobus, which uses bus_generic_rman_activate_resource.

Sponsored by: Amazon

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 60163
Build 57047: arc lint + arc unit

Event Timeline

cperciva added a subscriber: jhb.

@jhb You switched gpiobus from bus_generic_activate_resource to bus_generic_rman_activate_resource in https://reviews.freebsd.org/D43887 and that wasn't working for my GPIO _AEI driver. I *think* this commit is correct (that ACPI takes care of things for us with an ACPI GPIO device) but I'm not 100% sure.

Eh, this doesn't seem correct on the surface. How are the resources being allocated? If they are being allocated via gpiobus_alloc_resource(), then it is wrong to pass those resources up to some random other device driver in the tree and ask it to activate a resource it didn't create and doesn't know how to manage. Do you have more details on "that wasn't working"? Do you get a panic?

In D47252#1077458, @jhb wrote:

Eh, this doesn't seem correct on the surface. How are the resources being allocated? If they are being allocated via gpiobus_alloc_resource(), then it is wrong to pass those resources up to some random other device driver in the tree and ask it to activate a resource it didn't create and doesn't know how to manage. Do you have more details on "that wasn't working"? Do you get a panic?

No panic, but bus_setup_intr fails. The IRQ is being initially assigned in gpio_alloc_intr_resource:

res = bus_alloc_resource(consumer_dev, SYS_RES_IRQ, rid, irq, irq, 1,
    alloc_flags);

The issue is we need to call into subr_intr.c to activate the interrupt. Currently this is done in nexus on arm, arm64, and riscv but this requires calling the parent bus_activate_resource in each bus driver.

Presumably the gpiobus driver needs to call into subr_intr.c for those platforms then? (And that needs to happen in gpiobus.c, this is not ACPI specific)

Is there a reason to not do it in bus_generic_rman_activate_resource on those archs? We could then replace nexus_activate_resource with bus_generic_rman_activate_resource on arm and riscv (it's missing non-posted memory support needed on arm64)

Hmmm, if the logic is the same for all IRQ resources ever allocated on intrng architectures, then doing it in the wrapper might indeed be simplest.

I honestly don't know enough about the resource activation and intrng code to understand the alternative you two are discussing -- can I leave it with you to figure out what needs to be done here in the upcoming week? I'd like to make sure the _AEI support lands in time for 14.2 since it's a major pain point for AWS.

See D47282 for teaching bus_generic_rman_activate_resource about INTRNG

I have confirmed that D47282 does what is needed here.