Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752120AbbH2Ki4 (ORCPT ); Sat, 29 Aug 2015 06:38:56 -0400 Received: from youngberry.canonical.com ([91.189.89.112]:38818 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751577AbbH2Kiy (ORCPT ); Sat, 29 Aug 2015 06:38:54 -0400 Date: Sat, 29 Aug 2015 18:38:02 +0800 From: Ming Lei To: Arend van Spriel Cc: Takashi Iwai , "Luis R. Rodriguez" , Linus Torvalds , Liam Girdwood , "Jie, Yang" , "Dmitry Torokhov" , "joonas.lahtinen@linux.intel.com" , Tom Gundersen , Al Viro , Greg Kroah-Hartman , Kay Sievers , David Woodhouse , "Luis Rodriguez" , lkml , yalin wang , tom.leiming@gmail.com Subject: Re: Problems loading firmware using built-in drivers with kernels that use initramfs. Message-ID: <20150829183802.480b1425@tom-T450> In-Reply-To: <55E1724E.6040303@broadcom.com> References: <20150825193408.GR8051@wotan.suse.de> <1440576394.2443.17.camel@loki> <20150829011130.GK8051@wotan.suse.de> <55E1724E.6040303@broadcom.com> X-Mailer: Claws Mail 3.9.3 (GTK+ 2.24.23; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5261 Lines: 142 On Sat, 29 Aug 2015 10:50:22 +0200 Arend van Spriel 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 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 > >>>> wrote: > >>>>> On Wed, Aug 26, 2015 at 1:06 AM, Liam Girdwood > >>>>> 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; + 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/