From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S965708AbcIHJqA (ORCPT <rfc822;w@1wt.eu>);
        Thu, 8 Sep 2016 05:46:00 -0400
Received: from atlantic540.startdedicated.de ([188.138.9.77]:54072 "EHLO
        atlantic540.startdedicated.de" rhost-flags-OK-OK-OK-OK)
        by vger.kernel.org with ESMTP id S938577AbcIHJp7 (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Thu, 8 Sep 2016 05:45:59 -0400
Subject: Re: [PATCH v4 2/4] firmware: encapsulate firmware loading status
To: Daniel Wagner <daniel.wagner@bmw-carit.de>,
        "Luis R. Rodriguez" <mcgrof@kernel.org>, Takashi Iwai <tiwai@suse.de>
References: <1473237908-20989-1-git-send-email-wagi@monom.org>
 <1473237908-20989-3-git-send-email-wagi@monom.org>
 <20160908013940.GA3296@wotan.suse.de>
 <1aae9991-0fc2-3da7-b60b-001548f36daa@bmw-carit.de>
Cc: linux-kernel@vger.kernel.org, Ming Lei <ming.lei@canonical.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>
From: Daniel Wagner <wagi@monom.org>
Message-ID: <8dfb5256-8ac8-98f0-ad16-6a891065d204@monom.org>
Date: Thu, 8 Sep 2016 11:45:56 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Thunderbird/45.2.0
MIME-Version: 1.0
In-Reply-To: <1aae9991-0fc2-3da7-b60b-001548f36daa@bmw-carit.de>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 09/08/2016 10:05 AM, Daniel Wagner wrote:
> This while loop was originally a goto loop:
>
> 1f2b79599ee8 ("firmware loader: always let firmware_buf own the pages
> buffer")
>
> I don't think the code doesn't do what it was indented to do. The reason
> is that calling complete_all() the wait_for_completion() will not block
> again until it has 'eaten up' the done counter. That is around UMAX/2
> loops. So there is an reinit_completion() missing in this case.

Ah, I think I got it. It looks like kind of optimization. Instead doing

	wait_for_completion()
	if (done)
		do_this()
	if (abort)
		do_that()

we have

check_again:
	if (done) {
		do_this()
		goto out
	}
	if (abort) {
		do_that()
		goto out
	}
	wait_for_completion()
	goto check_again
out:

> Before the above commit, the completion was used to wait inside
> _request_firmware_load() till the UML had done its job. The answer for
> your question is probably in 1f2b79599ee8 ("firmware loader: always let
> firmware_buf own the pages buffer").

So it still does the same job after this change. Check if we have the 
firmware loaded already in buf if not kick off the umh loader and wait 
for the result.