On Sat, Jul 30, 2016 at 02:42:41PM +0200, Arend van Spriel wrote:

+ Luis (again) ;-)

On 29-07-16 08:13, Daniel Wagner wrote:

quoted

On 07/28/2016 09:01 PM, Bjorn Andersson wrote:

quoted

On Thu 28 Jul 11:33 PDT 2016, Dmitry Torokhov wrote:

quoted

On Thu, Jul 28, 2016 at 09:55:11AM +0200, Daniel Wagner wrote:

quoted

From: Daniel Wagner <redacted>

[..]

quoted

Do not quite like it... I'd rather asynchronous request give out a
firmware status pointer that could be used later on.

Excellent. Why not get rid of the callback function as well and have
fw_loading_wait() return result (0 = firmware available, < 0 = fail).
Just to confirm, you are proposing a new API function next to
request_firmware_nowait(), right?

If proposing new firmware_class patches please bounce / Cc me, I've
recently asked for me to be added to MAINTAINERS so I get these
e-mails as I'm working on a new flexible API which would allow us
to extend the firmware API without having to care about the old
stupid usermode helper at all.

quoted

quoted

quoted

    pcu->fw_st = request_firmware_async(IMS_PCU_FIRMWARE_NAME,
-                       pcu,
-                       ims_pcu_process_async_firmware);

    +			    pcu);

quoted

quoted

quoted

    if (IS_ERR(pcu->fw_st))
        return PTR_ERR(pcu->fw_st);

    ....

    err = fw_loading_wait(pcu->fw_st);

	if (err)
		return err;

	fw = fwstat_get_firmware(pcu->fw_st);

Or whatever consistent prefix it is going to be.

quoted

quoted

quoted

In the remoteproc case (patch 6) this would clean up the code, rather
than replacing the completion API 1 to 1. I like it!

IIRC most drivers do it the same way. So request_firmware_async() indeed
would be good thing to have. Let me try that.

While the idea behind this series is a good one I am wondering about the
need for these drivers to use the asynchronous API. The historic reason
might be to avoid timeout caused by user-mode helper, but that may no
longer apply and these drivers could be better off using
request_firmware_direct().

BTW I have in my queue for the sysdata API something like firmware_request_direct()
but with async support. The only thing left to do I think is just add the devm
helpers so drivers no longer need to worry about the release of the firmware.

There have been numerous discussions about the firmware API. Here most
recent one:

http://www.spinics.net/lists/linux-wireless/index.html#152755

And more importantly, the sysdata API queue:

https://git.kernel.org/cgit/linux/kernel/git/mcgrof/linux-next.git/log/?h=20160616-sysdata-v2

  Luis