lkml.org 
[lkml]   [2020]   [Jun]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    SubjectRe: [PATCH] pinctrl: initialise nsp-mux earlier.
    From
    Date


    On 6/30/2020 7:23 PM, Mark Tomlinson wrote:
    > On Tue, 2020-06-30 at 15:08 -0700, Ray Jui wrote:
    >> May I know which GPIO driver you are referring to on NSP? Both the iProc
    >> GPIO driver and the NSP GPIO driver are initialized at the level of
    >> 'arch_initcall_sync', which is supposed to be after 'arch_initcall' used
    >> here in the pinmux driver
    >
    > Sorry, it looks like I made a mistake in my testing (or I was lucky),
    > and this patch doesn't fix the issue. What is happening is:
    > 1) nsp-pinmux driver is registered (arch_initcall).
    > 2) nsp-gpio-a driver is registered (arch_initcall_sync).
    > 3) of_platform_default_populate_init() is called (also at level
    > arch_initcall_sync), which scans the device tree, adds the nsp-gpio-a
    > device, runs its probe, and this returns -EPROBE_DEFER with the error
    > message.
    > 4) Only now nsp-pinmux device is probed.
    >
    > Changing the 'arch_initcall_sync' to 'device_initcall' in nsp-gpio-a
    > ensures that the pinmux is probed first since
    > of_platform_default_populate_init() will be called between the two
    > register calls, and the error goes away. Is this change acceptable as a
    > solution?

    If probe deferral did not work, certainly but it sounds like this is
    being done just for the sake of eliminating a round of probe deferral,
    is there a functional problem this is fixing?

    >
    >>> though the probe will succeed when the driver is re-initialised, the
    >>> error can be scary to end users. To fix this, change the time the
    >>
    >> Scary to end users? I don't know about that. -EPROBE_DEFER was
    >> introduced exactly for this purpose. Perhaps users need to learn what
    >> -EPROBE_DEFER errno means?
    >
    > The actual error message in syslog is:
    >
    > kern.err kernel: gpiochip_add_data_with_key: GPIOs 480..511
    > (18000020.gpio) failed to register, -517
    >
    > So an end user sees "err" and "failed", and doesn't know what "-517"
    > means.

    How about this instead:

    diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
    index 4fa075d49fbc..10d9d0c17c9e 100644
    --- a/drivers/gpio/gpiolib.c
    +++ b/drivers/gpio/gpiolib.c
    @@ -1818,9 +1818,10 @@ int gpiochip_add_data_with_key(struct gpio_chip
    *gc, void *data,
    ida_simple_remove(&gpio_ida, gdev->id);
    err_free_gdev:
    /* failures here can mean systems won't boot... */
    - pr_err("%s: GPIOs %d..%d (%s) failed to register, %d\n", __func__,
    - gdev->base, gdev->base + gdev->ngpio - 1,
    - gc->label ? : "generic", ret);
    + if (ret != -EPROBE_DEFER)
    + pr_err("%s: GPIOs %d..%d (%s) failed to register, %d\n",
    + __func__, gdev->base, gdev->base + gdev->ngpio - 1,
    + gc->label ? : "generic", ret);
    kfree(gdev);
    return ret;
    }
    --
    Florian

    \
     
     \ /
      Last update: 2020-07-01 05:15    [W:5.635 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site