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

[Daniel Wagner <daniel.wagner@bmw-carit.de>]

On 09/08/2016 05:49 PM, Ming Lei wrote:
> On Thu, Sep 8, 2016 at 8:26 PM, Daniel Wagner <wagi@monom.org> 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