gpio: rework gpioaei

[vexeduxr]

Summary

Rework gpioaei to make it support more than one pin per GPIO resource.
Also allow one instance of gpioaei to handle multiple resources.

[vexeduxr]

I don't really like that we have to loop through the AEI resources twice, but I haven't been able to find a better way to do this.

[imp]

sys/dev/gpio/acpi_gpiobus.c

366

What's this brace doing here?

390

why BUS_ADD_CHILD and not bus_add_child? The usual pattern is to add the child with the ivars... So I'm a bit confused below that you get the ivards... Am I missing something here?

sys/dev/gpio/gpioaei.c

137

Doesn't this rely on the fact that type is initialized to 0 in the malloc? Maybe making that more explicit would help?

[vexeduxr]

sys/dev/gpio/acpi_gpiobus.c

366

It's so I don't pollute the rest of the function with the 4 aei variables.

390

gpiobus_add_child_common (which is called from acpi_gpiobus' bus_add_child) deals with allocating the ivars and calling device_set_ivars. So all we have to do after that is set the acpi specific bits and set the pins needed by _AEI.

why BUS_ADD_CHILD and not bus_add_child?

s/bus_add_child/device_add_child ?
Before we were doing device_add_child_ordered then initializing the resource list and calling device_set_ivars. But all of that is already done in gpiobus_add_child_common, which we can either call directly or call through BUS_ADD_CHILD, so I thought this would be better.

sys/dev/gpio/gpioaei.c

137

Yes and Yes :)

Looking through the code again, we could just remove the M_ZERO and initialize type and intr_rid explicitly.

[vexeduxr]

Allocate ctx without M_ZERO and explicitly initialize type and intr_rid.

[imp]

This is good to go in, but I'd feel happier if @jhb also reviewed it.

[vexeduxr]

Fix max pin check when enumerating _AEI.
GPIO_PIN_MAX returns the highest usable pin, so '>' should be used, not '>='.

[andrew]

sys/dev/gpio/acpi_gpiobus.c

366

If you don't want to move the variables to the top of the function moving this into a new function might be cleaner. It's uncommon to put these into the middle of a function.

sys/dev/gpio/gpioaei.c

124

Why M_NOWAIT when you don't handle it returning NULL?

244–245

It looks like you're implementing SLIST_FOREACH_SAFE

[vexeduxr]

sys/dev/gpio/gpioaei.c

124

This function gets called by ACPICA. While I tested M_WAITOK and it seemed to work fine, I'd rather not assume it's not doing anything else (e.g holding a mutex).

when you don't handle it returning NULL?

Not sure what else to do other than an assertion.

244–245

Ah, looks like I am.

[vexeduxr]

Move addition of gpioaei child to it's own function.
Use SLIST_FOREACH_SAFE.

[jhb]

I think this is mostly fine.

sys/dev/gpio/gpioaei.c

124

You should be able to use M_WAITOK here.

[vexeduxr]

sys/dev/gpio/gpioaei.c

124

Hmm, alright. Looking at the source, it doesn't seem to do anything weird.

[vexeduxr]

Use M_WAITOK.
Also rename acpi_gpiobus_add_aei to acpi_gpiobus_attach_aei.