Re: [PATCH] gpio: Do not accept gpio chip additions before gpiolib initialization

From: Guenter Roeck
Date: Thu Mar 31 2016 - 08:48:17 EST


On 03/30/2016 10:57 PM, Alexandre Courbot wrote:
On Wed, Mar 30, 2016 at 6:16 PM, Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
On 03/30/2016 01:37 AM, Alexandre Courbot wrote:

On Wed, Mar 30, 2016 at 3:20 AM, Guenter Roeck <linux@xxxxxxxxxxxx> wrote:

Since commit ff2b13592299 ("gpio: make the gpiochip a real device"),
attempts to add a gpio chip prior to gpiolib initialization cause the
system to crash. Dump a warning to the console and return an error
if the situation is encountered.


Mmm I see the problem but this could seriously delay the availability
of some GPIOs that are useful for early system boot.

I have not followed the GPIO device patches as closely as I should
have, but shouldn't you be able to register a GPIO chip without
immediately presenting it to user-space, for internal kernel needs? If
gpiolib is not initialized, then device-related operations would be
skipped, and gpiolib_dev_init() could then parse the list of
registered chips and fix them up when it gets called.

Again, I'm speaking without real knowledge here, but that pattern
seems more resilent to me.

You are absolutely right, but my knowledge of gpiolib is not good enough
to make that change. See this as a band-gap; it is better than just
crashing.

Actually, the following may be simpler:

Why not add a check in gpiochip_add_data() that will directly call
gpiolib_dev_init() if required? Then gpiolib_dev_init() could also
check whether it has already been called in that context and become a
no-op for when it is later called from core_initcall. Is there
anything that would prevents this from being a viable fix?

That was my first solution. Unfortunately, it doesn't work. It appears
that the calls made by gpiolib_dev_init() have dependencies themselves.
Though maybe I messed up - feel free to try yourself.

As mentioned in the other thread, I started looking into the solution
you suggested above. It should work, but it will take (me) a while
to implement it. Until then, guess we'll see more breakage.

Guenter