Tim, Tim Harvey <tharvey@xxxxxxxxxxxxx> writes: > On Tue, Apr 30, 2019 at 3:14 AM Lokesh Vutla <lokeshvutla@xxxxxx> wrote: >> - if (parent_data && parent_data->chip->irq_request_resources) { >> - r = parent_data->chip->irq_request_resources(parent_data); >> - if (r) >> - goto error; >> - } >> + r = irq_chip_request_resources_parent(data); >> + if (r) >> + gpiochip_unlock_as_irq(&txgpio->chip, txline->line); > > This patch breaks irq resources for thunderx-gpio as > parent_data->chip->irq_request_resources is undefined thus your new > irq_chip_request_resources_parent() returns -ENOSYS causing this > function to return an error where as before it would happily return 0. > > Is the following the correct fix or should we qualify > data->parent_data->chip->irq_request_resources before calling > irq_chip_request_resources_parent() in thunderx-gpio? You are not supposed to fiddle with parent data at all. Just because C allows you is no excuse to violate abstractions in the first place. irq_chip_request_resources_parent() rightfully returns -ENOSYS when it can't request a resource from the parent chip because that chip does not have anything to offer. It's up to the caller to do something sensible with the return code. If your chip is happy with the parent not providing it then handle -ENOSYS. None of the chip callbacks should return -ENOSYS. If one does then that wants to be fixed. Thanks, tglx