Re: [PATCH v4 2/4] firmware: encapsulate firmware loading status

[Daniel Wagner]

On 09/08/2016 05:49 PM, Ming Lei wrote:
> On Thu, Sep 8, 2016 at 8:26 PM, Daniel Wagner <wagi@xxxxxxxxx> wrote:
> >         if (ret != 0 && test_bit(...))
> >                 return -ENOENT;
>
> Another question, why do you want to return -ENOENT when
> userspace aborts the load? And looks it will always be override as
> -EAGAIN.

I took the -ENOENT from sync_cached_firmware_buf():

 /* wait until the shared firmware_buf becomes ready (or error) */
 static int sync_cached_firmware_buf(struct firmware_buf *buf)
 {
 	int ret = 0;

 	mutex_lock(&fw_lock);
-	while (!test_bit(FW_STATUS_DONE, &buf->status)) {
-		if (is_fw_load_aborted(buf)) {
+	while (!fw_status_is_done(&buf->fw_st)) {
+		if (fw_status_is_aborted(&buf->fw_st)) {
 			ret = -ENOENT;
 			break;
 		}
 		mutex_unlock(&fw_lock);
-		ret = wait_for_completion_interruptible(&buf->completion);
+		ret = fw_status_wait_timeout(&buf->fw_st, 0);
 		mutex_lock(&fw_lock);


There is a path where it not overwritten by -EAGAIN:

  request_firmware()
    _request_firmware()
      _request_firmware_prepare
        sync_cached_firmware_buf()
  	  fw_status_wait_timeout()
	    return -ENOENT

If you want me to change it to EAGAIN, I'll do it. I just tried hard not 
to have any changes in behavior.

cheer,
daniel