linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: Mathias Nyman <mathias.nyman@intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Moritz Fischer <mdf@kernel.org>,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] usb: renesas-xhci: Prefer firmware loading on unknown ROM state
Date: Thu, 26 Aug 2021 12:28:21 +0200	[thread overview]
Message-ID: <s5hilzswldm.wl-tiwai@suse.de> (raw)
In-Reply-To: <20210819113427.1166-1-tiwai@suse.de>

On Thu, 19 Aug 2021 13:34:27 +0200,
Takashi Iwai wrote:
> 
> The recent attempt to handle an unknown ROM state in the commit
> d143825baf15 ("usb: renesas-xhci: Fix handling of unknown ROM state")
> resulted in a regression and reverted later by the commit 44cf53602f5a
> ("Revert "usb: renesas-xhci: Fix handling of unknown ROM state"").
> The problem of the former fix was that it treated the failure of
> firmware loading as a fatal error.  Since the firmware files aren't
> included in the standard linux-firmware tree, most users don't have
> them, hence they got the non-working system after that.  The revert
> fixed the regression, but also it didn't make the firmware loading
> triggered even on the devices that do need it.  So we need still a fix
> for them.
> 
> This is another attempt to handle the unknown ROM state.  Like the
> previous fix, this also tries to load the firmware when ROM shows
> unknown state.  In this patch, however, the failure of a firmware
> loading (such as a missing firmware file) isn't handled as a fatal
> error any longer when ROM has been already detected, but it falls back
> to the ROM mode like before.  The error is returned only when no ROM
> is detected and the firmware loading failed.
> 
> Along with it, for simplifying the code flow, the detection and the
> check of ROM is factored out from renesas_fw_check_running() and done
> in the caller side, renesas_xhci_check_request_fw().  It avoids the
> redundant ROM checks.
> 
> The patch was tested on Lenovo Thinkpad T14 gen (BIOS 1.34).  Also it
> was confirmed that no regression is seen on another Thinkpad T14
> machine that has worked without the patch, too.
> 
> Fixes: 44cf53602f5a ("Revert "usb: renesas-xhci: Fix handling of unknown ROM state"")
> BugLink: https://bugzilla.opensuse.org/show_bug.cgi?id=1189207
> Signed-off-by: Takashi Iwai <tiwai@suse.de>

A gentle ping to confirm whether this gets a review or not.


thanks,

Takashi


> ---
>  drivers/usb/host/xhci-pci-renesas.c | 35 +++++++++++++++++++----------
>  1 file changed, 23 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-pci-renesas.c b/drivers/usb/host/xhci-pci-renesas.c
> index aa88e57649a9..52599d96634f 100644
> --- a/drivers/usb/host/xhci-pci-renesas.c
> +++ b/drivers/usb/host/xhci-pci-renesas.c
> @@ -207,7 +207,8 @@ static int renesas_check_rom_state(struct pci_dev *pdev)
>  			return 0;
>  
>  		case RENESAS_ROM_STATUS_NO_RESULT: /* No result yet */
> -			return 0;
> +			dev_dbg(&pdev->dev, "Unknown ROM status ...\n");
> +			return -ENOENT;
>  
>  		case RENESAS_ROM_STATUS_ERROR: /* Error State */
>  		default: /* All other states are marked as "Reserved states" */
> @@ -224,14 +225,6 @@ static int renesas_fw_check_running(struct pci_dev *pdev)
>  	u8 fw_state;
>  	int err;
>  
> -	/* Check if device has ROM and loaded, if so skip everything */
> -	err = renesas_check_rom(pdev);
> -	if (err) { /* we have rom */
> -		err = renesas_check_rom_state(pdev);
> -		if (!err)
> -			return err;
> -	}
> -
>  	/*
>  	 * Test if the device is actually needing the firmware. As most
>  	 * BIOSes will initialize the device for us. If the device is
> @@ -591,21 +584,39 @@ int renesas_xhci_check_request_fw(struct pci_dev *pdev,
>  			(struct xhci_driver_data *)id->driver_data;
>  	const char *fw_name = driver_data->firmware;
>  	const struct firmware *fw;
> +	bool has_rom;
>  	int err;
>  
> +	/* Check if device has ROM and loaded, if so skip everything */
> +	has_rom = renesas_check_rom(pdev);
> +	if (has_rom) {
> +		err = renesas_check_rom_state(pdev);
> +		if (!err)
> +			return 0;
> +		else if (err != -ENOENT)
> +			has_rom = false;
> +	}
> +
>  	err = renesas_fw_check_running(pdev);
>  	/* Continue ahead, if the firmware is already running. */
>  	if (!err)
>  		return 0;
>  
> +	/* no firmware interface available */
>  	if (err != 1)
> -		return err;
> +		return has_rom ? 0 : err;
>  
>  	pci_dev_get(pdev);
> -	err = request_firmware(&fw, fw_name, &pdev->dev);
> +	err = firmware_request_nowarn(&fw, fw_name, &pdev->dev);
>  	pci_dev_put(pdev);
>  	if (err) {
> -		dev_err(&pdev->dev, "request_firmware failed: %d\n", err);
> +		if (has_rom) {
> +			dev_info(&pdev->dev, "failed to load firmware %s, fallback to ROM\n",
> +				 fw_name);
> +			return 0;
> +		}
> +		dev_err(&pdev->dev, "failed to load firmware %s: %d\n",
> +			fw_name, err);
>  		return err;
>  	}
>  
> -- 
> 2.26.2
> 

  reply	other threads:[~2021-08-26 10:28 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-19 11:34 Takashi Iwai
2021-08-26 10:28 ` Takashi Iwai [this message]
2021-08-26 11:08   ` Greg Kroah-Hartman
2021-08-26 11:50 ` Greg Kroah-Hartman
2021-08-26 11:55   ` Takashi Iwai
2021-08-26 12:21     ` Greg Kroah-Hartman
2021-08-26 12:37       ` Takashi Iwai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=s5hilzswldm.wl-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mathias.nyman@intel.com \
    --cc=mdf@kernel.org \
    --subject='Re: [PATCH] usb: renesas-xhci: Prefer firmware loading on unknown ROM state' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).