Message-ID: <CAL_JsqLZFTszxs=t1=R0FQ3vP0scvpUBa6=Fb2S9LrTEvx4d_g@mail.gmail.com>
Date:   Thu, 24 May 2018 18:47:21 -0500
From:   Rob Herring <robh@...nel.org>
To:     Bjorn Andersson <bjorn.andersson@...aro.org>
Cc:     Arnd Bergmann <arnd@...db.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        Alexander Graf <agraf@...e.de>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Kevin Hilman <khilman@...nel.org>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        Joerg Roedel <joro@...tes.org>,
        Robin Murphy <robin.murphy@....com>,
        Mark Brown <broonie@...nel.org>,
        Frank Rowand <frowand.list@...il.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        devicetree@...r.kernel.org,
        Architecture Mailman List <boot-architecture@...ts.linaro.org>,
        "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v2 1/8] driver core: make deferring probe after init optional

On Thu, May 24, 2018 at 5:28 PM, Bjorn Andersson
<bjorn.andersson@...aro.org> wrote:
> On Thu 24 May 10:50 PDT 2018, Rob Herring wrote:
>
>> Deferred probe will currently wait forever on dependent devices to probe,
>> but sometimes a driver will never exist. It's also not always critical for
>> a driver to exist. Platforms can rely on default configuration from the
>> bootloader or reset defaults for things such as pinctrl and power domains.
>> This is often the case with initial platform support until various drivers
>> get enabled. There's at least 2 scenarios where deferred probe can render
>> a platform broken. Both involve using a DT which has more devices and
>> dependencies than the kernel supports. The 1st case is a driver may be
>> disabled in the kernel config. The 2nd case is the kernel version may
>> simply not have the dependent driver. This can happen if using a newer DT
>> (provided by firmware perhaps) with a stable kernel version.
>>
>> Subsystems or drivers may opt-in to this behavior by calling
>> driver_deferred_probe_check_init_done() instead of just returning
>> -EPROBE_DEFER. They may use additional information from DT or kernel's
>> config to decide whether to continue to defer probe or not.
>>
>
> For builtin drivers this still looks reasonable.
>
> But I would like to have an additional clarification here stating that
> drivers that might be targeted by this query must not be compiled as
> modules.
>
> And I would prefer to see an ack from e.g. Arnd that arm-soc is okay
> that we drop "tristate" on drivers affected by this; e.g. if we put this
> in the pinctrl core then all pinctrl drivers should be "bool" and so
> should any i2c, ssbi and spmi buses and drivers be.

For pinctrl, you are not affected now unless you change your DT. I
made pinctrl opt-in since as you said it could lead to some
intermittent problems. If iommu or power domains are required, then it
should fail pretty hard.

However, good luck getting your serial console to work if it has a
pinctrl dependency and that's a module. It won't work because the
console has to be up before userspace. Of course, you can just rely on
earlycon, but that just proves that pinctrl is not required (at least
for 1 serial pin). :)

Rob

>
> Regards,
> Bjorn
>
>> Cc: Alexander Graf <agraf@...e.de>
>> Signed-off-by: Rob Herring <robh@...nel.org>
>> ---
>>  drivers/base/dd.c      | 17 +++++++++++++++++
>>  include/linux/device.h |  2 ++
>>  2 files changed, 19 insertions(+)
>>
>> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
>> index c9f54089429b..d6034718da6f 100644
>> --- a/drivers/base/dd.c
>> +++ b/drivers/base/dd.c
>> @@ -226,6 +226,16 @@ void device_unblock_probing(void)
>>       driver_deferred_probe_trigger();
>>  }
>>
>> +int driver_deferred_probe_check_init_done(struct device *dev, bool optional)
>> +{
>> +     if (optional && initcalls_done) {
>> +             dev_WARN(dev, "ignoring dependency for device, assuming no driver");
>> +             return -ENODEV;
>> +     }
>> +
>> +     return -EPROBE_DEFER;
>> +}
>> +
>>  /**
>>   * deferred_probe_initcall() - Enable probing of deferred devices
>>   *
>> @@ -240,6 +250,13 @@ static int deferred_probe_initcall(void)
>>       /* Sort as many dependencies as possible before exiting initcalls */
>>       flush_work(&deferred_probe_work);
>>       initcalls_done = true;
>> +
>> +     /*
>> +      * Trigger deferred probe again, this time we won't defer anything
>> +      * that is optional
>> +      */
>> +     driver_deferred_probe_trigger();
>> +     flush_work(&deferred_probe_work);
>>       return 0;
>>  }
>>  late_initcall(deferred_probe_initcall);
>> diff --git a/include/linux/device.h b/include/linux/device.h
>> index 477956990f5e..f3dafd44c285 100644
>> --- a/include/linux/device.h
>> +++ b/include/linux/device.h
>> @@ -334,6 +334,8 @@ struct device *driver_find_device(struct device_driver *drv,
>>                                 struct device *start, void *data,
>>                                 int (*match)(struct device *dev, void *data));
>>
>> +int driver_deferred_probe_check_init_done(struct device *dev, bool optional);
>> +
>>  /**
>>   * struct subsys_interface - interfaces to device functions
>>   * @name:       name of the device function
>> --
>> 2.17.0
>>

Powered by blists - more mailing lists