Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 13 Nov 2019 00:56:28 +0000
From:   Luis Chamberlain <mcgrof@kernel.org>
To:     Drew DeVault <sir@cmpwn.com>
Cc:     linux-kernel@vger.kernel.org,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        ~sircmpwn/public-inbox@lists.sr.ht, robbat2@gentoo.org
Subject: Re: [PATCH v2] firmware loader: log path to loaded firmwares
Message-ID: <20191113005628.GT11244@42.do-not-panic.com>
References: <20191103180646.34880-1-sir@cmpwn.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20191103180646.34880-1-sir@cmpwn.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Sun, Nov 03, 2019 at 01:06:46PM -0500, Drew DeVault wrote:
> 
> This is useful for users who are trying to identify the firmwares in use
> on their system.
> 
> Signed-off-by: Drew DeVault <sir@cmpwn.com>
> ---
> v2 uses dev_dbg instead of printk(KERN_INFO)
> 
>  drivers/base/firmware_loader/main.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
> index bf44c79beae9..2537da43a572 100644
> --- a/drivers/base/firmware_loader/main.c
> +++ b/drivers/base/firmware_loader/main.c
> @@ -504,6 +504,7 @@ fw_get_filesystem_firmware(struct device *device, struct fw_priv *fw_priv,
>  					 path);
>  			continue;
>  		}
> +		dev_dbg(device, "Loading firmware from %s\n", path);

Because this is dev_dbg() I'm willing to consider it, so that its not
always enabled. However its not in the right place, the code path you
are addressing is only for direct filesystem lookups. If that fails
some systems do a fallback call out to userspace. To cover both cases,
you want it at the end of _request_firmware() on the success path. Can
you send a new patch?

  Luis

>  		if (decompress) {
>  			dev_dbg(device, "f/w decompressing %s\n",
>  				fw_priv->fw_name);
> -- 
> 2.23.0
>