Messages in this thread | | | Subject | Re: [PATCH v3 1/3] firmware_class: encapsulate firmware loading status | From | Daniel Wagner <> | Date | Mon, 29 Aug 2016 16:18:33 +0200 |
| |
On 08/29/2016 11:50 AM, Daniel Wagner wrote: > On 08/25/2016 07:50 PM, Luis R. Rodriguez wrote: >>> +#else /* CONFIG_FW_LOADER_USER_HELPER */ >>> + >>> +static int loading_timeout = 60; >>> +#define firmware_loading_timeout() (loading_timeout * HZ) >>> + >>> +#define fw_status_wait_timeout(fw_st, long) 0 >> >> The timeout makes 0 sense for when !CONFIG_FW_LOADER_USER_HELPER so can >> we do away with adding a silly 60 value to an int here and >> the silly value of (loading_timeout * HZ) ? Its not used so its not >> clear to me why this is here. > > So the main reason that silly timeout is needed is the usage of > it in device_cache_fw_images(). I suggest we add a timeout > argument to _request_firmware() and use the right timeout value > at that level. > > That allows to move the loading_timeout into the > CONFIG_FW_LOADER_USER_HELPER section.
I forgot to answer your question. So we have the dependency to loading_timeout/firmware_loading_timeout from the firmware caching path. The patch added in the previous email removes that dependency.
We still need the 60 second even in the !CONFIG_FW_LOADER_USER_HELPER case. I think it would be a regression if we change that value, no?
cheers, daniel
| |