Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753212AbbH3I0B (ORCPT ); Sun, 30 Aug 2015 04:26:01 -0400 Received: from mail-gw1-out.broadcom.com ([216.31.210.62]:42222 "EHLO mail-gw1-out.broadcom.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753061AbbH3IZz (ORCPT ); Sun, 30 Aug 2015 04:25:55 -0400 X-IronPort-AV: E=Sophos;i="5.17,433,1437462000"; d="scan'208";a="73842964" Message-ID: <55E2BE0E.1070907@broadcom.com> Date: Sun, 30 Aug 2015 10:25:50 +0200 From: Arend van Spriel User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0 MIME-Version: 1.0 To: Ming Lei 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 , Subject: Re: Problems loading firmware using built-in drivers with kernels that use initramfs. References: <20150825193408.GR8051@wotan.suse.de> <1440576394.2443.17.camel@loki> <20150829011130.GK8051@wotan.suse.de> <55E1724E.6040303@broadcom.com> <20150829183802.480b1425@tom-T450> In-Reply-To: <20150829183802.480b1425@tom-T450> Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5714 Lines: 154 On 08/29/2015 12:38 PM, Ming Lei wrote: > 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; > + 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/