Groups > linux.kernel > #1213275 > unrolled thread

Re: Problems loading firmware using built-in drivers with kernels that use initramfs.

Started by	"Luis R. Rodriguez" <mcgrof@suse.com>
First post	2015-08-25 21:40 +0200
Last post	2015-09-02 02:40 +0200
Articles	16 on this page of 36 — 10 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Page 2 of 2 — ← Prev page 1 [2]

#1215876

From	Arend van Spriel <arend@broadcom.com>
Date	2015-08-30 10:30 +0200
Message-ID	<q369Y-3wN-9@gated-at.bofh.it>
In reply to	#1215752

On 08/29/2015 12:38 PM, Ming Lei wrote:
> On Sat, 29 Aug 2015 10:50:22 +0200
> Arend van Spriel <arend@broadcom.com> wrote:
>
>> On 08/29/2015 09:11 AM, Takashi Iwai wrote:
>>> On Sat, 29 Aug 2015 06:09:01 +0200,
>>> Ming Lei wrote:
>>>>
>>>> On Sat, Aug 29, 2015 at 9:11 AM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
>>>>> On Thu, Aug 27, 2015 at 08:55:13AM +0800, Ming Lei wrote:
>>>>>> On Thu, Aug 27, 2015 at 2:07 AM, Linus Torvalds
>>>>>> <torvalds@linux-foundation.org> wrote:
>>>>>>> On Wed, Aug 26, 2015 at 1:06 AM, Liam Girdwood
>>>>>>> <liam.r.girdwood@linux.intel.com> wrote:
>>>>>>>>
>>>>>>>> I think the options are to either :-
>>>>>>>>
>>>>>>>> 1) Don not support audio DSP drivers using topology data as built-in
>>>>>>>> drivers. Audio is not really a critical system required for booting
>>>>>>>> anyway.
>>>>>>>
>>>>>>> Yes, forcing it to be a module and not letting people compile it in by
>>>>>>> mistake (and then not have it work) is an option.
>>>>>>>
>>>>>>> That said, there are situations where people don't want to use
>>>>>>> modules. I used to eschew them for security reasons, for example - now
>>>>>>> I instead just do a one-time temporary key. But others may have other
>>>>>>> reasons to try to avoid modules.
>>>>>>>
>>>>>>>> 2) Create a default PCM for every driver that has topology data on the
>>>>>>>> assumption that every sound card will at least 1 PCM. This PCM can then
>>>>>>>> be re-configured when the FW is loaded.
>>>>>>>
>>>>>>> That would seem to be the better option if it is reasonably implementable.
>>>>>>>
>>>>>>> Of course, some kind of timer-based retry (limited *somehow*) of the
>>>>>>> fw loading could work too, but smells really really hacky.
>>>>>>
>>>>>> Yeah, years ago, we discussed to use -EPROBE_DEFER for the situation,
>>>>>> which should be one kind of fix, but looks there were objections at that time.
>>>>>
>>>>> That would still be a hack. I'll note there is also asynchronous probe support
>>>>> now but to use that would also be a hack for this issue. We don't want to
>>>>
>>>> If we think firmware as one kind of resources like regulators, gpio and others,
>>>> PROBE_DEFER is one good match for firmware loading case, and
>>>> it has been used by lots of drivers, so why can't it be used for
>>>> firmware loading?
>>>>
>>>> One problem is that we need to convert drivers into returning -EPROBE_DEFER
>>>> in case of request failure, and that may involve some work, but which
>>>> should be mechanical.
>>>
>>> I find such a delaying mechanism not so bad, too.  It's very
>>> straightforward, at least, no big pain in the transition in the driver
>>> side.
>>
>> Not sure how this is going to work with request_firmware_nowait(). We
>> use that in our drivers to get rid of ~60 sec. delay in probe and
>> consequently boot time when built-in. So basically we return 0 on probe
>> lacking better knowledge. Guess we can always move back to
>> request_firmware calls when defer_probe support is available.
>
> How about the following untested draft patch?
>
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index be0eb46..f66912f 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -171,6 +171,12 @@ static void driver_deferred_probe_trigger(void)
>   	queue_work(deferred_wq, &deferred_probe_work);
>   }
>
> +void driver_trigger_fw_load()
> +{
> +	driver_deferred_probe_trigger();
> +}
> +EXPORT_SYMBOL_GPL(driver_trigger_fw_load);
> +
>   /**
>    * deferred_probe_initcall() - Enable probing of deferred devices
>    *
> diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
> index 8524450..f879a07 100644
> --- a/drivers/base/firmware_class.c
> +++ b/drivers/base/firmware_class.c
> @@ -1132,6 +1132,11 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
>   	if (ret <= 0) /* error or already assigned */
>   		goto out;
>
> +	if (system_state == SYSTEM_BOOTING) {
> +		ret = -EPROBE_DEFER;
> +		goto out;
> +	}
> +
>   	ret = 0;
>   	timeout = firmware_loading_timeout();
>   	if (opt_flags & FW_OPT_NOWAIT) {
> @@ -1311,6 +1316,9 @@ request_firmware_nowait(
>   {
>   	struct firmware_work *fw_work;
>
> +	if (system_state == SYSTEM_BOOTING)
> +		return -EPROBE_DEFER;
> +

Does this mean a built-in driver can not get firmware from initramfs or 
built in the kernel early. Seems a bit too aggressive. The problem 
stated in this thread is when the firmware is not on initramfs but only 
on the rootfs.

Regards,
Arend

>   	fw_work = kzalloc(sizeof(struct firmware_work), gfp);
>   	if (!fw_work)
>   		return -ENOMEM;
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 5d7bc63..1c189fe 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -289,6 +289,7 @@ extern struct device_driver *driver_find(const char *name,
>   					 struct bus_type *bus);
>   extern int driver_probe_done(void);
>   extern void wait_for_device_probe(void);
> +extern void driver_trigger_fw_load(void);
>
>
>   /* sysfs interface for exporting driver attributes */
> diff --git a/init/main.c b/init/main.c
> index 9e64d70..be8411b 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -943,6 +943,9 @@ static int __ref kernel_init(void *unused)
>
>   	flush_delayed_fput();
>
> +	/* trigger probe for request_firmware and its no_wait pair */
> +	driver_trigger_fw_load();
> +
>   	if (ramdisk_execute_command) {
>   		ret = run_init_process(ramdisk_execute_command);
>   		if (!ret)
>
>
>
> Thanks,
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1215951

From	Linus Torvalds <torvalds@linux-foundation.org>
Date	2015-08-30 20:20 +0200
Message-ID	<q3fmV-8oI-19@gated-at.bofh.it>
In reply to	#1215876

On Sun, Aug 30, 2015 at 1:25 AM, Arend van Spriel <arend@broadcom.com> wrote:
> On 08/29/2015 12:38 PM, Ming Lei wrote:
>
> Does this mean a built-in driver can not get firmware from initramfs or
> built in the kernel early. Seems a bit too aggressive.

Yeah, that seems wrong. Loading firmware from initramfs is required
for some things, like disk drivers. Of course, depending on how it's
done, it's all after the SYSTEM_BOOTING phase, but ..

What we *might* do is to not allow it for the user-mode helper
fallback, but I think it's more likely that we'll just deprecate the
usermode helper fw loader entirely, so adding new error cases for it
seems pointless.

    Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1216220

From	Ming Lei <ming.lei@canonical.com>
Date	2015-08-31 16:30 +0200
Message-ID	<q3yfV-1QK-27@gated-at.bofh.it>
In reply to	#1215876

On Sun, Aug 30, 2015 at 4:25 PM, Arend van Spriel <arend@broadcom.com> wrote:
> On 08/29/2015 12:38 PM, Ming Lei wrote:
>>
>> On Sat, 29 Aug 2015 10:50:22 +0200
>> Arend van Spriel <arend@broadcom.com> wrote:
>>
>>> On 08/29/2015 09:11 AM, Takashi Iwai wrote:
>>>>
>>>> On Sat, 29 Aug 2015 06:09:01 +0200,
>>>> Ming Lei wrote:
>>>>>
>>>>>
>>>>> On Sat, Aug 29, 2015 at 9:11 AM, Luis R. Rodriguez <mcgrof@suse.com>
>>>>> wrote:
>>>>>>
>>>>>> On Thu, Aug 27, 2015 at 08:55:13AM +0800, Ming Lei wrote:
>>>>>>>
>>>>>>> On Thu, Aug 27, 2015 at 2:07 AM, Linus Torvalds
>>>>>>> <torvalds@linux-foundation.org> wrote:
>>>>>>>>
>>>>>>>> On Wed, Aug 26, 2015 at 1:06 AM, Liam Girdwood
>>>>>>>> <liam.r.girdwood@linux.intel.com> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I think the options are to either :-
>>>>>>>>>
>>>>>>>>> 1) Don not support audio DSP drivers using topology data as
>>>>>>>>> built-in
>>>>>>>>> drivers. Audio is not really a critical system required for booting
>>>>>>>>> anyway.
>>>>>>>>
>>>>>>>>
>>>>>>>> Yes, forcing it to be a module and not letting people compile it in
>>>>>>>> by
>>>>>>>> mistake (and then not have it work) is an option.
>>>>>>>>
>>>>>>>> That said, there are situations where people don't want to use
>>>>>>>> modules. I used to eschew them for security reasons, for example -
>>>>>>>> now
>>>>>>>> I instead just do a one-time temporary key. But others may have
>>>>>>>> other
>>>>>>>> reasons to try to avoid modules.
>>>>>>>>
>>>>>>>>> 2) Create a default PCM for every driver that has topology data on
>>>>>>>>> the
>>>>>>>>> assumption that every sound card will at least 1 PCM. This PCM can
>>>>>>>>> then
>>>>>>>>> be re-configured when the FW is loaded.
>>>>>>>>
>>>>>>>>
>>>>>>>> That would seem to be the better option if it is reasonably
>>>>>>>> implementable.
>>>>>>>>
>>>>>>>> Of course, some kind of timer-based retry (limited *somehow*) of the
>>>>>>>> fw loading could work too, but smells really really hacky.
>>>>>>>
>>>>>>>
>>>>>>> Yeah, years ago, we discussed to use -EPROBE_DEFER for the situation,
>>>>>>> which should be one kind of fix, but looks there were objections at
>>>>>>> that time.
>>>>>>
>>>>>>
>>>>>> That would still be a hack. I'll note there is also asynchronous probe
>>>>>> support
>>>>>> now but to use that would also be a hack for this issue. We don't want
>>>>>> to
>>>>>
>>>>>
>>>>> If we think firmware as one kind of resources like regulators, gpio and
>>>>> others,
>>>>> PROBE_DEFER is one good match for firmware loading case, and
>>>>> it has been used by lots of drivers, so why can't it be used for
>>>>> firmware loading?
>>>>>
>>>>> One problem is that we need to convert drivers into returning
>>>>> -EPROBE_DEFER
>>>>> in case of request failure, and that may involve some work, but which
>>>>> should be mechanical.
>>>>
>>>>
>>>> I find such a delaying mechanism not so bad, too.  It's very
>>>> straightforward, at least, no big pain in the transition in the driver
>>>> side.
>>>
>>>
>>> Not sure how this is going to work with request_firmware_nowait(). We
>>> use that in our drivers to get rid of ~60 sec. delay in probe and
>>> consequently boot time when built-in. So basically we return 0 on probe
>>> lacking better knowledge. Guess we can always move back to
>>> request_firmware calls when defer_probe support is available.
>>
>>
>> How about the following untested draft patch?
>>
>> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
>> index be0eb46..f66912f 100644
>> --- a/drivers/base/dd.c
>> +++ b/drivers/base/dd.c
>> @@ -171,6 +171,12 @@ static void driver_deferred_probe_trigger(void)
>>         queue_work(deferred_wq, &deferred_probe_work);
>>   }
>>
>> +void driver_trigger_fw_load()
>> +{
>> +       driver_deferred_probe_trigger();
>> +}
>> +EXPORT_SYMBOL_GPL(driver_trigger_fw_load);
>> +
>>   /**
>>    * deferred_probe_initcall() - Enable probing of deferred devices
>>    *
>> diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
>> index 8524450..f879a07 100644
>> --- a/drivers/base/firmware_class.c
>> +++ b/drivers/base/firmware_class.c
>> @@ -1132,6 +1132,11 @@ _request_firmware(const struct firmware
>> **firmware_p, const char *name,
>>         if (ret <= 0) /* error or already assigned */
>>                 goto out;
>>
>> +       if (system_state == SYSTEM_BOOTING) {
>> +               ret = -EPROBE_DEFER;
>> +               goto out;
>> +       }
>> +
>>         ret = 0;
>>         timeout = firmware_loading_timeout();
>>         if (opt_flags & FW_OPT_NOWAIT) {
>> @@ -1311,6 +1316,9 @@ request_firmware_nowait(
>>   {
>>         struct firmware_work *fw_work;
>>
>> +       if (system_state == SYSTEM_BOOTING)
>> +               return -EPROBE_DEFER;
>> +
>
>
> Does this mean a built-in driver can not get firmware from initramfs or
> built in the kernel early. Seems a bit too aggressive. The problem stated in
> this thread is when the firmware is not on initramfs but only on the rootfs.

Yes, strictly speaking, user mode request can't be handled with defer probe
during booting because we don't know how the user helper handles the
request, that said even checking if the firmware exists in current path doesn't
make sense for user mode request.

So the patch should have used defer proble for direct load only
during booting.

>
> Regards,
> Arend
>
>
>>         fw_work = kzalloc(sizeof(struct firmware_work), gfp);
>>         if (!fw_work)
>>                 return -ENOMEM;
>> diff --git a/include/linux/device.h b/include/linux/device.h
>> index 5d7bc63..1c189fe 100644
>> --- a/include/linux/device.h
>> +++ b/include/linux/device.h
>> @@ -289,6 +289,7 @@ extern struct device_driver *driver_find(const char
>> *name,
>>                                          struct bus_type *bus);
>>   extern int driver_probe_done(void);
>>   extern void wait_for_device_probe(void);
>> +extern void driver_trigger_fw_load(void);
>>
>>
>>   /* sysfs interface for exporting driver attributes */
>> diff --git a/init/main.c b/init/main.c
>> index 9e64d70..be8411b 100644
>> --- a/init/main.c
>> +++ b/init/main.c
>> @@ -943,6 +943,9 @@ static int __ref kernel_init(void *unused)
>>
>>         flush_delayed_fput();
>>
>> +       /* trigger probe for request_firmware and its no_wait pair */
>> +       driver_trigger_fw_load();
>> +
>>         if (ramdisk_execute_command) {
>>                 ret = run_init_process(ramdisk_execute_command);
>>                 if (!ret)
>>
>>
>>
>> Thanks,
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1217253

From	"Luis R. Rodriguez" <mcgrof@suse.com>
Date	2015-09-02 03:20 +0200
Message-ID	<q44Sv-6P9-31@gated-at.bofh.it>
In reply to	#1216220

On Mon, Aug 31, 2015 at 10:21:34PM +0800, Ming Lei wrote:
> On Sun, Aug 30, 2015 at 4:25 PM, Arend van Spriel <arend@broadcom.com> wrote:
> > Does this mean a built-in driver can not get firmware from initramfs or
> > built in the kernel early. Seems a bit too aggressive. The problem stated in
> > this thread is when the firmware is not on initramfs but only on the rootfs.
> 
> Yes, strictly speaking, user mode request can't be handled with defer probe
> during booting because we don't know how the user helper handles the
> request,

FWIW I have a strategy in mind to help us compartamentalize the user mode
helper only to the dell-rbu driver, and as such phase out that code eventually
completely. Its part of the goals I have with the extensible firmware API I've
been proposing.

> that said even checking if the firmware exists in current path doesn't
> make sense for user mode request.
> 
> So the patch should have used defer proble for direct load only
> during booting.

What exact guarantees would we be giving to callers if they follow up on probe
with -EDEFER_PROBE ? I'd much prefer to try to avoid such uses in init / probe
(note that unless you're using async probe since we batch both so it doesn't really
matter where you place your code) all together and then for the few remaining
stragglers understand the requirements and provide an interface that lets them
claim their requirements and try to meets them.

A grammatical hunt for drivers who call fw API on init / probe can be
completed, although I know the hunt needs a bit more fine tuning it surely can
be completed. If we don't have many callers the compexity added for only a
few callers with rather loose criteria seems rather unnecessary, specially if
we can change the drivers and make these driver sthe exception rather than
a norm.

Then as for drivers *needing* the fw at probe why not have a proper interface
that does guarantee they get the requirements they ask for first ? For instance
a new probe type specified by the driver could enable the core to wait for say
an event and then tirgger a probe, kind of how we ended up defining the async
probe type preference:

static struct some_bus_driver some_driver = {                                  
        .probe = some_probe,                                              
        .id_table = some_id,                                              
        .driver = {                                                             
                .name = DEVICE_NAME,                                            
                .pm = &some_pm_ops,                                       
                .probe_type = PROBE_PREFER_POST_FOO,
        },                                                                      
}; 

Then we just don't try just hoping for completion but rather can do something
about the criteria passed.

  Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1217558

From	Arend van Spriel <arend@broadcom.com>
Date	2015-09-02 14:20 +0200
Message-ID	<q4fbd-4Me-27@gated-at.bofh.it>
In reply to	#1217253

On 09/02/2015 03:19 AM, Luis R. Rodriguez wrote:
> On Mon, Aug 31, 2015 at 10:21:34PM +0800, Ming Lei wrote:
>> On Sun, Aug 30, 2015 at 4:25 PM, Arend van Spriel <arend@broadcom.com> wrote:
>>> Does this mean a built-in driver can not get firmware from initramfs or
>>> built in the kernel early. Seems a bit too aggressive. The problem stated in
>>> this thread is when the firmware is not on initramfs but only on the rootfs.
>>
>> Yes, strictly speaking, user mode request can't be handled with defer probe
>> during booting because we don't know how the user helper handles the
>> request,
>
> FWIW I have a strategy in mind to help us compartamentalize the user mode
> helper only to the dell-rbu driver, and as such phase out that code eventually
> completely. Its part of the goals I have with the extensible firmware API I've
> been proposing.
>
>> that said even checking if the firmware exists in current path doesn't
>> make sense for user mode request.
>>
>> So the patch should have used defer proble for direct load only
>> during booting.
>
> What exact guarantees would we be giving to callers if they follow up on probe
> with -EDEFER_PROBE ? I'd much prefer to try to avoid such uses in init / probe
> (note that unless you're using async probe since we batch both so it doesn't really
> matter where you place your code) all together and then for the few remaining
> stragglers understand the requirements and provide an interface that lets them
> claim their requirements and try to meets them.
>
> A grammatical hunt for drivers who call fw API on init / probe can be
> completed, although I know the hunt needs a bit more fine tuning it surely can
> be completed. If we don't have many callers the compexity added for only a
> few callers with rather loose criteria seems rather unnecessary, specially if
> we can change the drivers and make these driver sthe exception rather than
> a norm.
>
> Then as for drivers *needing* the fw at probe why not have a proper interface
> that does guarantee they get the requirements they ask for first ? For instance
> a new probe type specified by the driver could enable the core to wait for say
> an event and then tirgger a probe, kind of how we ended up defining the async
> probe type preference:
>
> static struct some_bus_driver some_driver = {
>          .probe = some_probe,
>          .id_table = some_id,
>          .driver = {
>                  .name = DEVICE_NAME,
>                  .pm = &some_pm_ops,
>                  .probe_type = PROBE_PREFER_POST_FOO,
>          },
> };
>
> Then we just don't try just hoping for completion but rather can do something
> about the criteria passed.

That sounds good to me and learning about the async probe type. We do a 
schedule work in our module_init to avoid the probe being done in init 
context. Guess we can change that using the async probe type :-p

Regards,
Arend
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1217559

From	Arend van Spriel <arend@broadcom.com>
Date	2015-09-02 14:20 +0200
Message-ID	<q4fbd-4Me-25@gated-at.bofh.it>
In reply to	#1217558

On 09/02/2015 02:09 PM, Arend van Spriel wrote:
> On 09/02/2015 03:19 AM, Luis R. Rodriguez wrote:
>> On Mon, Aug 31, 2015 at 10:21:34PM +0800, Ming Lei wrote:
>>> On Sun, Aug 30, 2015 at 4:25 PM, Arend van Spriel
>>> <arend@broadcom.com> wrote:
>>>> Does this mean a built-in driver can not get firmware from initramfs or
>>>> built in the kernel early. Seems a bit too aggressive. The problem
>>>> stated in
>>>> this thread is when the firmware is not on initramfs but only on the
>>>> rootfs.
>>>
>>> Yes, strictly speaking, user mode request can't be handled with defer
>>> probe
>>> during booting because we don't know how the user helper handles the
>>> request,
>>
>> FWIW I have a strategy in mind to help us compartamentalize the user mode
>> helper only to the dell-rbu driver, and as such phase out that code
>> eventually
>> completely. Its part of the goals I have with the extensible firmware
>> API I've
>> been proposing.
>>
>>> that said even checking if the firmware exists in current path doesn't
>>> make sense for user mode request.
>>>
>>> So the patch should have used defer proble for direct load only
>>> during booting.
>>
>> What exact guarantees would we be giving to callers if they follow up
>> on probe
>> with -EDEFER_PROBE ? I'd much prefer to try to avoid such uses in init
>> / probe
>> (note that unless you're using async probe since we batch both so it
>> doesn't really
>> matter where you place your code) all together and then for the few
>> remaining
>> stragglers understand the requirements and provide an interface that
>> lets them
>> claim their requirements and try to meets them.
>>
>> A grammatical hunt for drivers who call fw API on init / probe can be
>> completed, although I know the hunt needs a bit more fine tuning it
>> surely can
>> be completed. If we don't have many callers the compexity added for
>> only a
>> few callers with rather loose criteria seems rather unnecessary,
>> specially if
>> we can change the drivers and make these driver sthe exception rather
>> than
>> a norm.
>>
>> Then as for drivers *needing* the fw at probe why not have a proper
>> interface
>> that does guarantee they get the requirements they ask for first ? For
>> instance
>> a new probe type specified by the driver could enable the core to wait
>> for say
>> an event and then tirgger a probe, kind of how we ended up defining
>> the async
>> probe type preference:
>>
>> static struct some_bus_driver some_driver = {
>>          .probe = some_probe,
>>          .id_table = some_id,
>>          .driver = {
>>                  .name = DEVICE_NAME,
>>                  .pm = &some_pm_ops,
>>                  .probe_type = PROBE_PREFER_POST_FOO,
>>          },
>> };
>>
>> Then we just don't try just hoping for completion but rather can do
>> something
>> about the criteria passed.

So should the probe type indicate some event or should it just indicate 
what the driver needs, ie. .probe_type = PROBE_TYPE_NEED_FW.

Regards,
Arend

> That sounds good to me and learning about the async probe type. We do a
> schedule work in our module_init to avoid the probe being done in init
> context. Guess we can change that using the async probe type :-p
>
> Regards,
> Arend
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1217808

From	"Luis R. Rodriguez" <mcgrof@suse.com>
Date	2015-09-02 21:00 +0200
Message-ID	<q4lqj-5as-25@gated-at.bofh.it>
In reply to	#1217559

On Wed, Sep 02, 2015 at 02:13:49PM +0200, Arend van Spriel wrote:
> On 09/02/2015 02:09 PM, Arend van Spriel wrote:
> >On 09/02/2015 03:19 AM, Luis R. Rodriguez wrote:
> >>On Mon, Aug 31, 2015 at 10:21:34PM +0800, Ming Lei wrote:
> >>>On Sun, Aug 30, 2015 at 4:25 PM, Arend van Spriel
> >>><arend@broadcom.com> wrote:
> >>>>Does this mean a built-in driver can not get firmware from initramfs or
> >>>>built in the kernel early. Seems a bit too aggressive. The problem
> >>>>stated in
> >>>>this thread is when the firmware is not on initramfs but only on the
> >>>>rootfs.
> >>>
> >>>Yes, strictly speaking, user mode request can't be handled with defer
> >>>probe
> >>>during booting because we don't know how the user helper handles the
> >>>request,
> >>
> >>FWIW I have a strategy in mind to help us compartamentalize the user mode
> >>helper only to the dell-rbu driver, and as such phase out that code
> >>eventually
> >>completely. Its part of the goals I have with the extensible firmware
> >>API I've
> >>been proposing.
> >>
> >>>that said even checking if the firmware exists in current path doesn't
> >>>make sense for user mode request.
> >>>
> >>>So the patch should have used defer proble for direct load only
> >>>during booting.
> >>
> >>What exact guarantees would we be giving to callers if they follow up
> >>on probe
> >>with -EDEFER_PROBE ? I'd much prefer to try to avoid such uses in init
> >>/ probe
> >>(note that unless you're using async probe since we batch both so it
> >>doesn't really
> >>matter where you place your code) all together and then for the few
> >>remaining
> >>stragglers understand the requirements and provide an interface that
> >>lets them
> >>claim their requirements and try to meets them.
> >>
> >>A grammatical hunt for drivers who call fw API on init / probe can be
> >>completed, although I know the hunt needs a bit more fine tuning it
> >>surely can
> >>be completed. If we don't have many callers the compexity added for
> >>only a
> >>few callers with rather loose criteria seems rather unnecessary,
> >>specially if
> >>we can change the drivers and make these driver sthe exception rather
> >>than
> >>a norm.
> >>
> >>Then as for drivers *needing* the fw at probe why not have a proper
> >>interface
> >>that does guarantee they get the requirements they ask for first ? For
> >>instance
> >>a new probe type specified by the driver could enable the core to wait
> >>for say
> >>an event and then tirgger a probe, kind of how we ended up defining
> >>the async
> >>probe type preference:
> >>
> >>static struct some_bus_driver some_driver = {
> >>         .probe = some_probe,
> >>         .id_table = some_id,
> >>         .driver = {
> >>                 .name = DEVICE_NAME,
> >>                 .pm = &some_pm_ops,
> >>                 .probe_type = PROBE_PREFER_POST_FOO,
> >>         },
> >>};
> >>
> >>Then we just don't try just hoping for completion but rather can do
> >>something
> >>about the criteria passed.
> 
> So should the probe type indicate some event or should it just
> indicate what the driver needs, ie. .probe_type =
> PROBE_TYPE_NEED_FW.

Right so this is an open question. I suggested something like the above
since the deferred probe documentation on drivers/base/dd.c states:

 * Sometimes driver probe order matters, but the kernel doesn't always have     
 * dependency information 

I'm alluding that we consider *avoiding* -EPROBE_DEFER for areas of the
kernel where some work can be done to not only list the dependency
the information from the driver but also we know we can get it from
the kernel. In this case I do believe we could not only express the
requirement but also wait for it in the kernel. Before we do that
though I think it'd be good to do a grammar hunt to determine exactly
how popular all this fw on probe needed really is.

Greg, provided we find sufficient drivers that really do need firmware
on probe, any thoughts on this line of approach to address this?

  Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1217861

From	Arend van Spriel <arend@broadcom.com>
Date	2015-09-02 23:10 +0200
Message-ID	<q4ns6-8gJ-19@gated-at.bofh.it>
In reply to	#1217808

On 09/02/2015 08:58 PM, Luis R. Rodriguez wrote:
> On Wed, Sep 02, 2015 at 02:13:49PM +0200, Arend van Spriel wrote:
>> On 09/02/2015 02:09 PM, Arend van Spriel wrote:
>>> On 09/02/2015 03:19 AM, Luis R. Rodriguez wrote:
>>>> On Mon, Aug 31, 2015 at 10:21:34PM +0800, Ming Lei wrote:
>>>>> On Sun, Aug 30, 2015 at 4:25 PM, Arend van Spriel
>>>>> <arend@broadcom.com> wrote:
>>>>>> Does this mean a built-in driver can not get firmware from initramfs or
>>>>>> built in the kernel early. Seems a bit too aggressive. The problem
>>>>>> stated in
>>>>>> this thread is when the firmware is not on initramfs but only on the
>>>>>> rootfs.
>>>>>
>>>>> Yes, strictly speaking, user mode request can't be handled with defer
>>>>> probe
>>>>> during booting because we don't know how the user helper handles the
>>>>> request,
>>>>
>>>> FWIW I have a strategy in mind to help us compartamentalize the user mode
>>>> helper only to the dell-rbu driver, and as such phase out that code
>>>> eventually
>>>> completely. Its part of the goals I have with the extensible firmware
>>>> API I've
>>>> been proposing.
>>>>
>>>>> that said even checking if the firmware exists in current path doesn't
>>>>> make sense for user mode request.
>>>>>
>>>>> So the patch should have used defer proble for direct load only
>>>>> during booting.
>>>>
>>>> What exact guarantees would we be giving to callers if they follow up
>>>> on probe
>>>> with -EDEFER_PROBE ? I'd much prefer to try to avoid such uses in init
>>>> / probe
>>>> (note that unless you're using async probe since we batch both so it
>>>> doesn't really
>>>> matter where you place your code) all together and then for the few
>>>> remaining
>>>> stragglers understand the requirements and provide an interface that
>>>> lets them
>>>> claim their requirements and try to meets them.
>>>>
>>>> A grammatical hunt for drivers who call fw API on init / probe can be
>>>> completed, although I know the hunt needs a bit more fine tuning it
>>>> surely can
>>>> be completed. If we don't have many callers the compexity added for
>>>> only a
>>>> few callers with rather loose criteria seems rather unnecessary,
>>>> specially if
>>>> we can change the drivers and make these driver sthe exception rather
>>>> than
>>>> a norm.
>>>>
>>>> Then as for drivers *needing* the fw at probe why not have a proper
>>>> interface
>>>> that does guarantee they get the requirements they ask for first ? For
>>>> instance
>>>> a new probe type specified by the driver could enable the core to wait
>>>> for say
>>>> an event and then tirgger a probe, kind of how we ended up defining
>>>> the async
>>>> probe type preference:
>>>>
>>>> static struct some_bus_driver some_driver = {
>>>>          .probe = some_probe,
>>>>          .id_table = some_id,
>>>>          .driver = {
>>>>                  .name = DEVICE_NAME,
>>>>                  .pm = &some_pm_ops,
>>>>                  .probe_type = PROBE_PREFER_POST_FOO,
>>>>          },
>>>> };
>>>>
>>>> Then we just don't try just hoping for completion but rather can do
>>>> something
>>>> about the criteria passed.
>>
>> So should the probe type indicate some event or should it just
>> indicate what the driver needs, ie. .probe_type =
>> PROBE_TYPE_NEED_FW.
>
> Right so this is an open question. I suggested something like the above
> since the deferred probe documentation on drivers/base/dd.c states:
>
>   * Sometimes driver probe order matters, but the kernel doesn't always have
>   * dependency information
>
> I'm alluding that we consider *avoiding* -EPROBE_DEFER for areas of the
> kernel where some work can be done to not only list the dependency
> the information from the driver but also we know we can get it from
> the kernel. In this case I do believe we could not only express the
> requirement but also wait for it in the kernel. Before we do that
> though I think it'd be good to do a grammar hunt to determine exactly
> how popular all this fw on probe needed really is.

Ok. So some background why we need it in brcm80211 drivers. So as a 
wireless network device driver the answer we got when asking for an 
event to load firware is upon IF_UP for a registered net device. Because 
we try to do things smart we query the firmware running on the device 
for capabilities before we can register the net device hence we request 
the firmware during probe. This may be specific to wireless drivers 
(Intel has same approach if not mistaken) but I suspect there may be more.

Regards,
Arend


> Greg, provided we find sufficient drivers that really do need firmware
> on probe, any thoughts on this line of approach to address this?
>
>    Luis
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1217907

From	Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date	2015-09-03 01:20 +0200
Message-ID	<q4ptT-2FO-5@gated-at.bofh.it>
In reply to	#1217861

On Wed, Sep 2, 2015 at 2:03 PM, Arend van Spriel <arend@broadcom.com> wrote:
> On 09/02/2015 08:58 PM, Luis R. Rodriguez wrote:
>>
>> On Wed, Sep 02, 2015 at 02:13:49PM +0200, Arend van Spriel wrote:
>>>
>>> On 09/02/2015 02:09 PM, Arend van Spriel wrote:
>>>>
>>>> On 09/02/2015 03:19 AM, Luis R. Rodriguez wrote:
>>>>>
>>>>> On Mon, Aug 31, 2015 at 10:21:34PM +0800, Ming Lei wrote:
>>>>>>
>>>>>> On Sun, Aug 30, 2015 at 4:25 PM, Arend van Spriel
>>>>>> <arend@broadcom.com> wrote:
>>>>>>>
>>>>>>> Does this mean a built-in driver can not get firmware from initramfs
>>>>>>> or
>>>>>>> built in the kernel early. Seems a bit too aggressive. The problem
>>>>>>> stated in
>>>>>>> this thread is when the firmware is not on initramfs but only on the
>>>>>>> rootfs.
>>>>>>
>>>>>>
>>>>>> Yes, strictly speaking, user mode request can't be handled with defer
>>>>>> probe
>>>>>> during booting because we don't know how the user helper handles the
>>>>>> request,
>>>>>
>>>>>
>>>>> FWIW I have a strategy in mind to help us compartamentalize the user
>>>>> mode
>>>>> helper only to the dell-rbu driver, and as such phase out that code
>>>>> eventually
>>>>> completely. Its part of the goals I have with the extensible firmware
>>>>> API I've
>>>>> been proposing.
>>>>>
>>>>>> that said even checking if the firmware exists in current path doesn't
>>>>>> make sense for user mode request.
>>>>>>
>>>>>> So the patch should have used defer proble for direct load only
>>>>>> during booting.
>>>>>
>>>>>
>>>>> What exact guarantees would we be giving to callers if they follow up
>>>>> on probe
>>>>> with -EDEFER_PROBE ? I'd much prefer to try to avoid such uses in init
>>>>> / probe
>>>>> (note that unless you're using async probe since we batch both so it
>>>>> doesn't really
>>>>> matter where you place your code) all together and then for the few
>>>>> remaining
>>>>> stragglers understand the requirements and provide an interface that
>>>>> lets them
>>>>> claim their requirements and try to meets them.
>>>>>
>>>>> A grammatical hunt for drivers who call fw API on init / probe can be
>>>>> completed, although I know the hunt needs a bit more fine tuning it
>>>>> surely can
>>>>> be completed. If we don't have many callers the compexity added for
>>>>> only a
>>>>> few callers with rather loose criteria seems rather unnecessary,
>>>>> specially if
>>>>> we can change the drivers and make these driver sthe exception rather
>>>>> than
>>>>> a norm.
>>>>>
>>>>> Then as for drivers *needing* the fw at probe why not have a proper
>>>>> interface
>>>>> that does guarantee they get the requirements they ask for first ? For
>>>>> instance
>>>>> a new probe type specified by the driver could enable the core to wait
>>>>> for say
>>>>> an event and then tirgger a probe, kind of how we ended up defining
>>>>> the async
>>>>> probe type preference:
>>>>>
>>>>> static struct some_bus_driver some_driver = {
>>>>>          .probe = some_probe,
>>>>>          .id_table = some_id,
>>>>>          .driver = {
>>>>>                  .name = DEVICE_NAME,
>>>>>                  .pm = &some_pm_ops,
>>>>>                  .probe_type = PROBE_PREFER_POST_FOO,
>>>>>          },
>>>>> };
>>>>>
>>>>> Then we just don't try just hoping for completion but rather can do
>>>>> something
>>>>> about the criteria passed.
>>>
>>>
>>> So should the probe type indicate some event or should it just
>>> indicate what the driver needs, ie. .probe_type =
>>> PROBE_TYPE_NEED_FW.
>>
>>
>> Right so this is an open question. I suggested something like the above
>> since the deferred probe documentation on drivers/base/dd.c states:
>>
>>   * Sometimes driver probe order matters, but the kernel doesn't always
>> have
>>   * dependency information
>>
>> I'm alluding that we consider *avoiding* -EPROBE_DEFER for areas of the
>> kernel where some work can be done to not only list the dependency
>> the information from the driver but also we know we can get it from
>> the kernel. In this case I do believe we could not only express the
>> requirement but also wait for it in the kernel. Before we do that
>> though I think it'd be good to do a grammar hunt to determine exactly
>> how popular all this fw on probe needed really is.
>
>
> Ok. So some background why we need it in brcm80211 drivers. So as a wireless
> network device driver the answer we got when asking for an event to load
> firware is upon IF_UP for a registered net device. Because we try to do
> things smart we query the firmware running on the device for capabilities
> before we can register the net device hence we request the firmware during
> probe. This may be specific to wireless drivers (Intel has same approach if
> not mistaken) but I suspect there may be more.

We have the same issue with input devices: before we can register one
we need to set their capabilities and to know their capabilities we
quite often need to load their firmware/config and query the device.

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1217912

From	"Luis R. Rodriguez" <mcgrof@suse.com>
Date	2015-09-03 01:30 +0200
Message-ID	<q4pDz-2R2-1@gated-at.bofh.it>
In reply to	#1217907

On Wed, Sep 02, 2015 at 04:13:51PM -0700, Dmitry Torokhov wrote:
> On Wed, Sep 2, 2015 at 2:03 PM, Arend van Spriel <arend@broadcom.com> wrote:
> > On 09/02/2015 08:58 PM, Luis R. Rodriguez wrote:
> >>
> >> On Wed, Sep 02, 2015 at 02:13:49PM +0200, Arend van Spriel wrote:
> >>>
> >>> On 09/02/2015 02:09 PM, Arend van Spriel wrote:
> >>>>
> >>>> On 09/02/2015 03:19 AM, Luis R. Rodriguez wrote:
> >>>>>
> >>>>> On Mon, Aug 31, 2015 at 10:21:34PM +0800, Ming Lei wrote:
> >>>>>>
> >>>>>> On Sun, Aug 30, 2015 at 4:25 PM, Arend van Spriel
> >>>>>> <arend@broadcom.com> wrote:
> >>>>>>>
> >>>>>>> Does this mean a built-in driver can not get firmware from initramfs
> >>>>>>> or
> >>>>>>> built in the kernel early. Seems a bit too aggressive. The problem
> >>>>>>> stated in
> >>>>>>> this thread is when the firmware is not on initramfs but only on the
> >>>>>>> rootfs.
> >>>>>>
> >>>>>>
> >>>>>> Yes, strictly speaking, user mode request can't be handled with defer
> >>>>>> probe
> >>>>>> during booting because we don't know how the user helper handles the
> >>>>>> request,
> >>>>>
> >>>>>
> >>>>> FWIW I have a strategy in mind to help us compartamentalize the user
> >>>>> mode
> >>>>> helper only to the dell-rbu driver, and as such phase out that code
> >>>>> eventually
> >>>>> completely. Its part of the goals I have with the extensible firmware
> >>>>> API I've
> >>>>> been proposing.
> >>>>>
> >>>>>> that said even checking if the firmware exists in current path doesn't
> >>>>>> make sense for user mode request.
> >>>>>>
> >>>>>> So the patch should have used defer proble for direct load only
> >>>>>> during booting.
> >>>>>
> >>>>>
> >>>>> What exact guarantees would we be giving to callers if they follow up
> >>>>> on probe
> >>>>> with -EDEFER_PROBE ? I'd much prefer to try to avoid such uses in init
> >>>>> / probe
> >>>>> (note that unless you're using async probe since we batch both so it
> >>>>> doesn't really
> >>>>> matter where you place your code) all together and then for the few
> >>>>> remaining
> >>>>> stragglers understand the requirements and provide an interface that
> >>>>> lets them
> >>>>> claim their requirements and try to meets them.
> >>>>>
> >>>>> A grammatical hunt for drivers who call fw API on init / probe can be
> >>>>> completed, although I know the hunt needs a bit more fine tuning it
> >>>>> surely can
> >>>>> be completed. If we don't have many callers the compexity added for
> >>>>> only a
> >>>>> few callers with rather loose criteria seems rather unnecessary,
> >>>>> specially if
> >>>>> we can change the drivers and make these driver sthe exception rather
> >>>>> than
> >>>>> a norm.
> >>>>>
> >>>>> Then as for drivers *needing* the fw at probe why not have a proper
> >>>>> interface
> >>>>> that does guarantee they get the requirements they ask for first ? For
> >>>>> instance
> >>>>> a new probe type specified by the driver could enable the core to wait
> >>>>> for say
> >>>>> an event and then tirgger a probe, kind of how we ended up defining
> >>>>> the async
> >>>>> probe type preference:
> >>>>>
> >>>>> static struct some_bus_driver some_driver = {
> >>>>>          .probe = some_probe,
> >>>>>          .id_table = some_id,
> >>>>>          .driver = {
> >>>>>                  .name = DEVICE_NAME,
> >>>>>                  .pm = &some_pm_ops,
> >>>>>                  .probe_type = PROBE_PREFER_POST_FOO,
> >>>>>          },
> >>>>> };
> >>>>>
> >>>>> Then we just don't try just hoping for completion but rather can do
> >>>>> something
> >>>>> about the criteria passed.
> >>>
> >>>
> >>> So should the probe type indicate some event or should it just
> >>> indicate what the driver needs, ie. .probe_type =
> >>> PROBE_TYPE_NEED_FW.
> >>
> >>
> >> Right so this is an open question. I suggested something like the above
> >> since the deferred probe documentation on drivers/base/dd.c states:
> >>
> >>   * Sometimes driver probe order matters, but the kernel doesn't always
> >> have
> >>   * dependency information
> >>
> >> I'm alluding that we consider *avoiding* -EPROBE_DEFER for areas of the
> >> kernel where some work can be done to not only list the dependency
> >> the information from the driver but also we know we can get it from
> >> the kernel. In this case I do believe we could not only express the
> >> requirement but also wait for it in the kernel. Before we do that
> >> though I think it'd be good to do a grammar hunt to determine exactly
> >> how popular all this fw on probe needed really is.
> >
> >
> > Ok. So some background why we need it in brcm80211 drivers. So as a wireless
> > network device driver the answer we got when asking for an event to load
> > firware is upon IF_UP for a registered net device. Because we try to do
> > things smart we query the firmware running on the device for capabilities
> > before we can register the net device hence we request the firmware during
> > probe. This may be specific to wireless drivers (Intel has same approach if
> > not mistaken) but I suspect there may be more.
> 
> We have the same issue with input devices: before we can register one
> we need to set their capabilities and to know their capabilities we
> quite often need to load their firmware/config and query the device.

Should Arend's driver use async probe then?

IMHO its just as hacky as using -EPROBE_DEFER too, but its at least
preemptively hacky. Sadly I can't think of clear and clever way for the kernel
to know when firmware will be ready either...  Would userspace know? Should the
kernel learn this from userspace ?

For instance, if init is going to use initramfs it can pivot_root() and later
send us a smoke signal when done, and it it doesn't it can also send us a smoke
signal. In the absence of such a hint being implemented I suppose -EPROBE_DEFER,
async probe or a notifier on pivot_root() is best effort we can do, but again
*eh*.

  Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1217917

From	Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date	2015-09-03 01:30 +0200
Message-ID	<q4pDA-2R2-11@gated-at.bofh.it>
In reply to	#1217912

On Wed, Sep 2, 2015 at 4:22 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> On Wed, Sep 02, 2015 at 04:13:51PM -0700, Dmitry Torokhov wrote:
>> On Wed, Sep 2, 2015 at 2:03 PM, Arend van Spriel <arend@broadcom.com> wrote:
>> > On 09/02/2015 08:58 PM, Luis R. Rodriguez wrote:
>> >>
>> >> On Wed, Sep 02, 2015 at 02:13:49PM +0200, Arend van Spriel wrote:
>> >>>
>> >>> On 09/02/2015 02:09 PM, Arend van Spriel wrote:
>> >>>>
>> >>>> On 09/02/2015 03:19 AM, Luis R. Rodriguez wrote:
>> >>>>>
>> >>>>> On Mon, Aug 31, 2015 at 10:21:34PM +0800, Ming Lei wrote:
>> >>>>>>
>> >>>>>> On Sun, Aug 30, 2015 at 4:25 PM, Arend van Spriel
>> >>>>>> <arend@broadcom.com> wrote:
>> >>>>>>>
>> >>>>>>> Does this mean a built-in driver can not get firmware from initramfs
>> >>>>>>> or
>> >>>>>>> built in the kernel early. Seems a bit too aggressive. The problem
>> >>>>>>> stated in
>> >>>>>>> this thread is when the firmware is not on initramfs but only on the
>> >>>>>>> rootfs.
>> >>>>>>
>> >>>>>>
>> >>>>>> Yes, strictly speaking, user mode request can't be handled with defer
>> >>>>>> probe
>> >>>>>> during booting because we don't know how the user helper handles the
>> >>>>>> request,
>> >>>>>
>> >>>>>
>> >>>>> FWIW I have a strategy in mind to help us compartamentalize the user
>> >>>>> mode
>> >>>>> helper only to the dell-rbu driver, and as such phase out that code
>> >>>>> eventually
>> >>>>> completely. Its part of the goals I have with the extensible firmware
>> >>>>> API I've
>> >>>>> been proposing.
>> >>>>>
>> >>>>>> that said even checking if the firmware exists in current path doesn't
>> >>>>>> make sense for user mode request.
>> >>>>>>
>> >>>>>> So the patch should have used defer proble for direct load only
>> >>>>>> during booting.
>> >>>>>
>> >>>>>
>> >>>>> What exact guarantees would we be giving to callers if they follow up
>> >>>>> on probe
>> >>>>> with -EDEFER_PROBE ? I'd much prefer to try to avoid such uses in init
>> >>>>> / probe
>> >>>>> (note that unless you're using async probe since we batch both so it
>> >>>>> doesn't really
>> >>>>> matter where you place your code) all together and then for the few
>> >>>>> remaining
>> >>>>> stragglers understand the requirements and provide an interface that
>> >>>>> lets them
>> >>>>> claim their requirements and try to meets them.
>> >>>>>
>> >>>>> A grammatical hunt for drivers who call fw API on init / probe can be
>> >>>>> completed, although I know the hunt needs a bit more fine tuning it
>> >>>>> surely can
>> >>>>> be completed. If we don't have many callers the compexity added for
>> >>>>> only a
>> >>>>> few callers with rather loose criteria seems rather unnecessary,
>> >>>>> specially if
>> >>>>> we can change the drivers and make these driver sthe exception rather
>> >>>>> than
>> >>>>> a norm.
>> >>>>>
>> >>>>> Then as for drivers *needing* the fw at probe why not have a proper
>> >>>>> interface
>> >>>>> that does guarantee they get the requirements they ask for first ? For
>> >>>>> instance
>> >>>>> a new probe type specified by the driver could enable the core to wait
>> >>>>> for say
>> >>>>> an event and then tirgger a probe, kind of how we ended up defining
>> >>>>> the async
>> >>>>> probe type preference:
>> >>>>>
>> >>>>> static struct some_bus_driver some_driver = {
>> >>>>>          .probe = some_probe,
>> >>>>>          .id_table = some_id,
>> >>>>>          .driver = {
>> >>>>>                  .name = DEVICE_NAME,
>> >>>>>                  .pm = &some_pm_ops,
>> >>>>>                  .probe_type = PROBE_PREFER_POST_FOO,
>> >>>>>          },
>> >>>>> };
>> >>>>>
>> >>>>> Then we just don't try just hoping for completion but rather can do
>> >>>>> something
>> >>>>> about the criteria passed.
>> >>>
>> >>>
>> >>> So should the probe type indicate some event or should it just
>> >>> indicate what the driver needs, ie. .probe_type =
>> >>> PROBE_TYPE_NEED_FW.
>> >>
>> >>
>> >> Right so this is an open question. I suggested something like the above
>> >> since the deferred probe documentation on drivers/base/dd.c states:
>> >>
>> >>   * Sometimes driver probe order matters, but the kernel doesn't always
>> >> have
>> >>   * dependency information
>> >>
>> >> I'm alluding that we consider *avoiding* -EPROBE_DEFER for areas of the
>> >> kernel where some work can be done to not only list the dependency
>> >> the information from the driver but also we know we can get it from
>> >> the kernel. In this case I do believe we could not only express the
>> >> requirement but also wait for it in the kernel. Before we do that
>> >> though I think it'd be good to do a grammar hunt to determine exactly
>> >> how popular all this fw on probe needed really is.
>> >
>> >
>> > Ok. So some background why we need it in brcm80211 drivers. So as a wireless
>> > network device driver the answer we got when asking for an event to load
>> > firware is upon IF_UP for a registered net device. Because we try to do
>> > things smart we query the firmware running on the device for capabilities
>> > before we can register the net device hence we request the firmware during
>> > probe. This may be specific to wireless drivers (Intel has same approach if
>> > not mistaken) but I suspect there may be more.
>>
>> We have the same issue with input devices: before we can register one
>> we need to set their capabilities and to know their capabilities we
>> quite often need to load their firmware/config and query the device.
>
> Should Arend's driver use async probe then?

What has async probe have to do with anything? We will still be
waiting for async probes to finish before we mount rootfs so it will
not change absolutely anything.

>
> IMHO its just as hacky as using -EPROBE_DEFER too, but its at least
> preemptively hacky. Sadly I can't think of clear and clever way for the kernel
> to know when firmware will be ready either...  Would userspace know? Should the
> kernel learn this from userspace ?

Yes. Given only userspace knows when firmware is available (I could
have it on a separate device and mount it at some time). So maybe
userpsace should simply try and scan busses for unbound devices and
tell them to re-probe when it decides that firmware is finally
available.

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1217947

From	"Luis R. Rodriguez" <mcgrof@suse.com>
Date	2015-09-03 01:50 +0200
Message-ID	<q4pWV-3dk-7@gated-at.bofh.it>
In reply to	#1217917

On Wed, Sep 2, 2015 at 4:29 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Wed, Sep 2, 2015 at 4:22 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
>> On Wed, Sep 02, 2015 at 04:13:51PM -0700, Dmitry Torokhov wrote:
>>> On Wed, Sep 2, 2015 at 2:03 PM, Arend van Spriel <arend@broadcom.com> wrote:
>>> > Ok. So some background why we need it in brcm80211 drivers. So as a wireless
>>> > network device driver the answer we got when asking for an event to load
>>> > firware is upon IF_UP for a registered net device. Because we try to do
>>> > things smart we query the firmware running on the device for capabilities
>>> > before we can register the net device hence we request the firmware during
>>> > probe. This may be specific to wireless drivers (Intel has same approach if
>>> > not mistaken) but I suspect there may be more.
>>>
>>> We have the same issue with input devices: before we can register one
>>> we need to set their capabilities and to know their capabilities we
>>> quite often need to load their firmware/config and query the device.
>>
>> Should Arend's driver use async probe then?
>
> What has async probe have to do with anything? We will still be
> waiting for async probes to finish before we mount rootfs so it will
> not change absolutely anything.

:) Right, its what I was alluding to as well.

>> IMHO its just as hacky as using -EPROBE_DEFER too, but its at least
>> preemptively hacky. Sadly I can't think of clear and clever way for the kernel
>> to know when firmware will be ready either...  Would userspace know? Should the
>> kernel learn this from userspace ?
>
> Yes. Given only userspace knows when firmware is available (I could
> have it on a separate device and mount it at some time). So maybe
> userpsace should simply try and scan busses for unbound devices and
> tell them to re-probe when it decides that firmware is finally
> available.

OK, the folks wanting this mechanism can implement it then. Short of
that we only have hacks.

 Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1218459

From	Arend van Spriel <arend@broadcom.com>
Date	2015-09-03 19:30 +0200
Message-ID	<q4GuN-1LD-65@gated-at.bofh.it>
In reply to	#1217947

On 09/03/2015 01:46 AM, Luis R. Rodriguez wrote:
> On Wed, Sep 2, 2015 at 4:29 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
>> On Wed, Sep 2, 2015 at 4:22 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
>>> On Wed, Sep 02, 2015 at 04:13:51PM -0700, Dmitry Torokhov wrote:
>>>> On Wed, Sep 2, 2015 at 2:03 PM, Arend van Spriel <arend@broadcom.com> wrote:
>>>>> Ok. So some background why we need it in brcm80211 drivers. So as a wireless
>>>>> network device driver the answer we got when asking for an event to load
>>>>> firware is upon IF_UP for a registered net device. Because we try to do
>>>>> things smart we query the firmware running on the device for capabilities
>>>>> before we can register the net device hence we request the firmware during
>>>>> probe. This may be specific to wireless drivers (Intel has same approach if
>>>>> not mistaken) but I suspect there may be more.
>>>>
>>>> We have the same issue with input devices: before we can register one
>>>> we need to set their capabilities and to know their capabilities we
>>>> quite often need to load their firmware/config and query the device.
>>>
>>> Should Arend's driver use async probe then?
>>
>> What has async probe have to do with anything? We will still be
>> waiting for async probes to finish before we mount rootfs so it will
>> not change absolutely anything.
>
> :) Right, its what I was alluding to as well.

Indeed. However, upon module_init we schedule a worker in which the 
driver are registered. We do that to make sure the probe is not done 
within module_init context. That could now be done with async probe. 
This is not the problem discussed here so let's not spend more time on this.

>>> IMHO its just as hacky as using -EPROBE_DEFER too, but its at least
>>> preemptively hacky. Sadly I can't think of clear and clever way for the kernel
>>> to know when firmware will be ready either...  Would userspace know? Should the
>>> kernel learn this from userspace ?
>>
>> Yes. Given only userspace knows when firmware is available (I could
>> have it on a separate device and mount it at some time). So maybe
>> userpsace should simply try and scan busses for unbound devices and
>> tell them to re-probe when it decides that firmware is finally
>> available.
>
> OK, the folks wanting this mechanism can implement it then. Short of
> that we only have hacks.

So what does "userspace knows when firmware is available" mean here. The 
specific firmware file the driver wants or the collection of firmware 
files which may or may not have the specific firmware file the driver 
wants. I assume the latter and re-probe will fail as expected.

Regards,
Arend
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1218461

From	Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date	2015-09-03 19:40 +0200
Message-ID	<q4GEq-1WX-11@gated-at.bofh.it>
In reply to	#1218459

On Thu, Sep 3, 2015 at 10:23 AM, Arend van Spriel <arend@broadcom.com> wrote:
> On 09/03/2015 01:46 AM, Luis R. Rodriguez wrote:
>>
>> On Wed, Sep 2, 2015 at 4:29 PM, Dmitry Torokhov
>> <dmitry.torokhov@gmail.com> wrote:
>>>
>>> On Wed, Sep 2, 2015 at 4:22 PM, Luis R. Rodriguez <mcgrof@suse.com>
>>> wrote:
>>>> IMHO its just as hacky as using -EPROBE_DEFER too, but its at least
>>>> preemptively hacky. Sadly I can't think of clear and clever way for the
>>>> kernel
>>>> to know when firmware will be ready either...  Would userspace know?
>>>> Should the
>>>> kernel learn this from userspace ?
>>>
>>>
>>> Yes. Given only userspace knows when firmware is available (I could
>>> have it on a separate device and mount it at some time). So maybe
>>> userpsace should simply try and scan busses for unbound devices and
>>> tell them to re-probe when it decides that firmware is finally
>>> available.
>>
>>
>> OK, the folks wanting this mechanism can implement it then. Short of
>> that we only have hacks.
>
>
> So what does "userspace knows when firmware is available" mean here. The
> specific firmware file the driver wants or the collection of firmware files
> which may or may not have the specific firmware file the driver wants. I
> assume the latter and re-probe will fail as expected.

Right, the latter.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1217849

From	Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date	2015-09-02 22:50 +0200
Message-ID	<q4n8J-7EF-1@gated-at.bofh.it>
In reply to	#1217559

On Wed, Sep 2, 2015 at 5:13 AM, Arend van Spriel <arend@broadcom.com> wrote:
> On 09/02/2015 02:09 PM, Arend van Spriel wrote:
>>
>> On 09/02/2015 03:19 AM, Luis R. Rodriguez wrote:
>>>
>>> On Mon, Aug 31, 2015 at 10:21:34PM +0800, Ming Lei wrote:
>>>>
>>>> On Sun, Aug 30, 2015 at 4:25 PM, Arend van Spriel
>>>> <arend@broadcom.com> wrote:
>>>>>
>>>>> Does this mean a built-in driver can not get firmware from initramfs or
>>>>> built in the kernel early. Seems a bit too aggressive. The problem
>>>>> stated in
>>>>> this thread is when the firmware is not on initramfs but only on the
>>>>> rootfs.
>>>>
>>>>
>>>> Yes, strictly speaking, user mode request can't be handled with defer
>>>> probe
>>>> during booting because we don't know how the user helper handles the
>>>> request,
>>>
>>>
>>> FWIW I have a strategy in mind to help us compartamentalize the user mode
>>> helper only to the dell-rbu driver, and as such phase out that code
>>> eventually
>>> completely. Its part of the goals I have with the extensible firmware
>>> API I've
>>> been proposing.
>>>
>>>> that said even checking if the firmware exists in current path doesn't
>>>> make sense for user mode request.
>>>>
>>>> So the patch should have used defer proble for direct load only
>>>> during booting.
>>>
>>>
>>> What exact guarantees would we be giving to callers if they follow up
>>> on probe
>>> with -EDEFER_PROBE ? I'd much prefer to try to avoid such uses in init
>>> / probe
>>> (note that unless you're using async probe since we batch both so it
>>> doesn't really
>>> matter where you place your code) all together and then for the few
>>> remaining
>>> stragglers understand the requirements and provide an interface that
>>> lets them
>>> claim their requirements and try to meets them.
>>>
>>> A grammatical hunt for drivers who call fw API on init / probe can be
>>> completed, although I know the hunt needs a bit more fine tuning it
>>> surely can
>>> be completed. If we don't have many callers the compexity added for
>>> only a
>>> few callers with rather loose criteria seems rather unnecessary,
>>> specially if
>>> we can change the drivers and make these driver sthe exception rather
>>> than
>>> a norm.
>>>
>>> Then as for drivers *needing* the fw at probe why not have a proper
>>> interface
>>> that does guarantee they get the requirements they ask for first ? For
>>> instance
>>> a new probe type specified by the driver could enable the core to wait
>>> for say
>>> an event and then tirgger a probe, kind of how we ended up defining
>>> the async
>>> probe type preference:
>>>
>>> static struct some_bus_driver some_driver = {
>>>          .probe = some_probe,
>>>          .id_table = some_id,
>>>          .driver = {
>>>                  .name = DEVICE_NAME,
>>>                  .pm = &some_pm_ops,
>>>                  .probe_type = PROBE_PREFER_POST_FOO,
>>>          },
>>> };
>>>
>>> Then we just don't try just hoping for completion but rather can do
>>> something
>>> about the criteria passed.
>
>
> So should the probe type indicate some event or should it just indicate what
> the driver needs, ie. .probe_type = PROBE_TYPE_NEED_FW.

What will kernel do when it sees such an option? It does not know when
firmware will become available (if ever). We can only try and fail and
then userspace may try re-probing devices once firmware is available.
I guess the question is how to let userspace know what devices failed
to probe because of the lack of firmware.

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1217186

From	"Luis R. Rodriguez" <mcgrof@suse.com>
Date	2015-09-02 02:40 +0200
Message-ID	<q44fM-5Qc-5@gated-at.bofh.it>
In reply to	#1215683

On Sat, Aug 29, 2015 at 12:09:01PM +0800, Ming Lei wrote:
> On Sat, Aug 29, 2015 at 9:11 AM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> > On Thu, Aug 27, 2015 at 08:55:13AM +0800, Ming Lei wrote:
> >> On Thu, Aug 27, 2015 at 2:07 AM, Linus Torvalds
> >> <torvalds@linux-foundation.org> wrote:
> >> > On Wed, Aug 26, 2015 at 1:06 AM, Liam Girdwood
> >> > <liam.r.girdwood@linux.intel.com> wrote:
> >> >>
> >> >> I think the options are to either :-
> >> >>
> >> >> 1) Don not support audio DSP drivers using topology data as built-in
> >> >> drivers. Audio is not really a critical system required for booting
> >> >> anyway.
> >> >
> >> > Yes, forcing it to be a module and not letting people compile it in by
> >> > mistake (and then not have it work) is an option.
> >> >
> >> > That said, there are situations where people don't want to use
> >> > modules. I used to eschew them for security reasons, for example - now
> >> > I instead just do a one-time temporary key. But others may have other
> >> > reasons to try to avoid modules.
> >> >
> >> >> 2) Create a default PCM for every driver that has topology data on the
> >> >> assumption that every sound card will at least 1 PCM. This PCM can then
> >> >> be re-configured when the FW is loaded.
> >> >
> >> > That would seem to be the better option if it is reasonably implementable.
> >> >
> >> > Of course, some kind of timer-based retry (limited *somehow*) of the
> >> > fw loading could work too, but smells really really hacky.
> >>
> >> Yeah, years ago, we discussed to use -EPROBE_DEFER for the situation,
> >> which should be one kind of fix, but looks there were objections at that time.
> >
> > That would still be a hack. I'll note there is also asynchronous probe support
> > now but to use that would also be a hack for this issue. We don't want to
> 
> If we think firmware as one kind of resources like regulators, gpio and others,
> PROBE_DEFER is one good match for firmware loading case, and
> it has been used by lots of drivers, so why can't it be used for
> firmware loading?

I'm glad you asked, it begs the question if we could have done something better
for these other components. In short its a matter of if we have an interface
that would let devices coming up ask: are my requirements available yet? Reason
we kick -EPROBE_DEFER is we can't answer this as we have no way to map some
of these requirements pricely so -EPROBE_DEFER is the best we can do at times.

It doesn't mean we shouldn't think harder, and for firmware I think we can and
should try harder to answer these questions.

I'm arguing that its a viable solution to use -EPROBE_DEFER but I don't think
its the best we can do but also I worry about the lack of semantics that would
be implied by user if they start doing this all over. In terms of semantics
I'd want at least some undestanding by the caller over certain guarantees of
what we are going to try to do for them by using -EPROBE_DEFER.

> One problem is that we need to convert drivers into returning -EPROBE_DEFER
> in case of request failure, and that may involve some work, but which
> should be mechanical.

And there may be cases where the fs might already be available, so it would
be pointless to retry if the error was true. To me that's a bit sloppy, and
part of the sloppiness comes from the lack of clear semantics. Its why we are
having this discussion. Its also not the first of its case and its why I'm kind
of trying to be a bit pedantic. I would prefer to avoid just a bandaid.

> > encourage folks to go down that road.  They'd be hacks for this issue as you
> > are simply delaying the driver probe for a later time and there is no guarantee
> > that any pivot_root() might have already been completed later to ensure your
> > driver's fw file is present. So it may work or it may not.
> 
> We can trigger defer probe explicitly once root fs is setup or other condition
> is met.

Now we're talking, that's the sort of line of solution I'd much prefer, but again
that's building on top of a use case that I think we should try to avoid. I think
the strategy is sound but not the way we're deferring probe. I think that's prone
to error and the solution lacks clarity.

  Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

csiph-web

Re: Problems loading firmware using built-in drivers with kernels that use initramfs.

Contents

#1215876

#1215951

#1216220

#1217253

#1217558

#1217559

#1217808

#1217861

#1217907

#1217912

#1217917

#1217947

#1218459

#1218461

#1217849

#1217186