Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38522 )
Change subject: x86/acpi_device: Allow acpi_device_add_power_res params as optional ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38522/1/src/arch/x86/acpi_device.c File src/arch/x86/acpi_device.c:
https://review.coreboot.org/c/coreboot/+/38522/1/src/arch/x86/acpi_device.c@... PS1, Line 544: if (!reset_gpio && !enable_gpio && !stop_gpio) : return; :
We only need to define a `stop_gpio` for the _ON, _OFF methods. […]
What the check did was: It returned early if reset gpio, enable gpio and stop gpio are all 0s since in that case there is no real power control pin. For your case, it is totally fine to just have a stop gpio without having the rest. The check would fail for !stop_gpio and continue to add power resource.
With this check removed, if a call is made to acpi_device_add_power_res() with all gpios set to 0, then empty power resource would be added to the device. It is not bad, but unnecessary.
Yes, the NULL deref above is bad, but removing the check in lines 544-545 isn't really making it more general.