Messages in this thread | | | Date | Mon, 11 Nov 2013 15:05:36 -0500 | From | Prarit Bhargava <> | Subject | Re: [PATCH v2 1/3] firmware: Introduce request_firmware_direct() |
| |
On 11/11/2013 12:30 PM, Takashi Iwai wrote: > At Mon, 11 Nov 2013 16:34:26 +0100, > Borislav Petkov wrote: >> >> On Mon, Nov 11, 2013 at 04:21:16PM +0100, Takashi Iwai wrote: >>> When CONFIG_FW_LOADER_USER_HELPER is set, request_firmware() falls >>> back to the usermode helper for loading via udev when the direct >>> loading fails. But the recent udev takes way too long timeout (60 >>> seconds) for non-existing firmware. This is unacceptable for the >>> drivers like microcode loader where they load firmwares optionally, >>> i.e. it's no error even if no requested file exists. >>> >>> This patch provides a new helper function, request_firmware_direct(). >>> It behaves as same as request_firmware() except for that it doesn't >>> fall back to usermode helper but returns an error immediately if the >>> f/w can't be loaded directly in kernel. >>> >>> Without CONFIG_FW_LOADER_USER_HELPER=y, request_firmware_direct() is >>> just an alias of request_firmware(), due to obvious reason. >>> >>> Tested-by: Prarit Bhargava <prarit@redhat.com> >>> Acked-by: Ming Lei <ming.lei@canonical.com> >>> Signed-off-by: Takashi Iwai <tiwai@suse.de> >>> --- >>> drivers/base/firmware_class.c | 36 +++++++++++++++++++++++++++++++----- >>> include/linux/firmware.h | 7 +++++++ >>> 2 files changed, 38 insertions(+), 5 deletions(-) >> >> I like it, the 60 seconds thing has been a senseless PITA for no good >> reason. I have always wondered what might change in 60 seconds wrt to us >> being able to load the firmware... >> >>> diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c >>> index eb8fb94ae2c5..7f48a6ffb0df 100644 >>> --- a/drivers/base/firmware_class.c >>> +++ b/drivers/base/firmware_class.c >>> @@ -1061,7 +1061,7 @@ static int assign_firmware_buf(struct firmware *fw, struct device *device, >>> /* called from request_firmware() and request_firmware_work_func() */ >>> static int >>> _request_firmware(const struct firmware **firmware_p, const char *name, >>> - struct device *device, bool uevent, bool nowait) >>> + struct device *device, bool uevent, bool nowait, bool fallback) >> >> Just a nitpick: three boolean args in a row starts to slowly look like a >> function from the windoze API. Can we do: >> >> _request_firmware(..., unsigned long flags) >> >> instead and have nice bit flags for that? > > Sounds like a good idea. How about the patch below? > (I used unsigned int since there shouldn't be so many different > behaviors.) > > > thanks, > > Takashi > > === > > From: Takashi Iwai <tiwai@suse.de> > Subject: [PATCH] firmware: Use bit flags instead of boolean combos > > More than two boolean arguments to a function are rather confusing and > error-prone for callers. Let's make the behavior bit flags instead of > triple combos. > > A nice suggestion by Borislav Petkov. > > Signed-off-by: Takashi Iwai <tiwai@suse.de>
Sure -- looks good.
Acked-by: Prarit Bhargava <prarit@redhat.com>
P.
| |