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@xxxxxxx>
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
>
