2021-07-12 08:09:49

by Greg Kroah-Hartman

[permalink] [raw]
Subject: [PATCH 5.13 024/800] usb: renesas-xhci: Fix handling of unknown ROM state

From: Moritz Fischer <[email protected]>

commit d143825baf15f204dac60acdf95e428182aa3374 upstream.

The ROM load sometimes seems to return an unknown status
(RENESAS_ROM_STATUS_NO_RESULT) instead of success / fail.

If the ROM load indeed failed this leads to failures when trying to
communicate with the controller later on.

Attempt to load firmware using RAM load in those cases.

Fixes: 2478be82de44 ("usb: renesas-xhci: Add ROM loader for uPD720201")
Cc: [email protected]
Cc: Mathias Nyman <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Vinod Koul <[email protected]>
Tested-by: Vinod Koul <[email protected]>
Reviewed-by: Vinod Koul <[email protected]>
Signed-off-by: Moritz Fischer <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

---
drivers/usb/host/xhci-pci-renesas.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)

--- 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(struc
return 0;

case RENESAS_ROM_STATUS_NO_RESULT: /* No result yet */
- return 0;
+ dev_dbg(&pdev->dev, "Unknown ROM status ...\n");
+ break;

case RENESAS_ROM_STATUS_ERROR: /* Error State */
default: /* All other states are marked as "Reserved states" */
@@ -224,13 +225,12 @@ static int renesas_fw_check_running(stru
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;
- }
+ /*
+ * Only if device has ROM and loaded FW we can skip loading and
+ * return success. Otherwise (even unknown state), attempt to load FW.
+ */
+ if (renesas_check_rom(pdev) && !renesas_check_rom_state(pdev))
+ return 0;

/*
* Test if the device is actually needing the firmware. As most



2021-07-17 13:40:28

by Justin Forbes

[permalink] [raw]
Subject: Re: [PATCH 5.13 024/800] usb: renesas-xhci: Fix handling of unknown ROM state

On Mon, Jul 12, 2021 at 2:31 AM Greg Kroah-Hartman
<[email protected]> wrote:
>
> From: Moritz Fischer <[email protected]>
>
> commit d143825baf15f204dac60acdf95e428182aa3374 upstream.
>
> The ROM load sometimes seems to return an unknown status
> (RENESAS_ROM_STATUS_NO_RESULT) instead of success / fail.
>
> If the ROM load indeed failed this leads to failures when trying to
> communicate with the controller later on.
>
> Attempt to load firmware using RAM load in those cases.
>
> Fixes: 2478be82de44 ("usb: renesas-xhci: Add ROM loader for uPD720201")
> Cc: [email protected]
> Cc: Mathias Nyman <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Vinod Koul <[email protected]>
> Tested-by: Vinod Koul <[email protected]>
> Reviewed-by: Vinod Koul <[email protected]>
> Signed-off-by: Moritz Fischer <[email protected]>
> Link: https://lore.kernel.org/r/[email protected]
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
>

After sending out 5.12.17 for testing, we had a user complain that all
of their USB devices disappeared with the error:

Jul 15 23:18:53 kernel: xhci_hcd 0000:04:00.0: Direct firmware load
for renesas_usb_fw.mem failed with error -2
Jul 15 23:18:53 kernel: xhci_hcd 0000:04:00.0: request_firmware failed: -2
Jul 15 23:18:53 kernel: xhci_hcd: probe of 0000:04:00.0 failed with error -2

After first assuming that something was missing in the backport to
5.12, I had the user try 5.13.2, and then 5.14-rc1. Both of those
failed in the same way, so it is not working in the current Linus tree
either. Reverting this patch fixed the issue.

Justin

> ---
> drivers/usb/host/xhci-pci-renesas.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> --- 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(struc
> return 0;
>
> case RENESAS_ROM_STATUS_NO_RESULT: /* No result yet */
> - return 0;
> + dev_dbg(&pdev->dev, "Unknown ROM status ...\n");
> + break;
>
> case RENESAS_ROM_STATUS_ERROR: /* Error State */
> default: /* All other states are marked as "Reserved states" */
> @@ -224,13 +225,12 @@ static int renesas_fw_check_running(stru
> 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;
> - }
> + /*
> + * Only if device has ROM and loaded FW we can skip loading and
> + * return success. Otherwise (even unknown state), attempt to load FW.
> + */
> + if (renesas_check_rom(pdev) && !renesas_check_rom_state(pdev))
> + return 0;
>
> /*
> * Test if the device is actually needing the firmware. As most
>
>

2021-07-17 17:36:12

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 5.13 024/800] usb: renesas-xhci: Fix handling of unknown ROM state

On Sat, Jul 17, 2021 at 08:39:19AM -0500, Justin Forbes wrote:
> On Mon, Jul 12, 2021 at 2:31 AM Greg Kroah-Hartman
> <[email protected]> wrote:
> >
> > From: Moritz Fischer <[email protected]>
> >
> > commit d143825baf15f204dac60acdf95e428182aa3374 upstream.
> >
> > The ROM load sometimes seems to return an unknown status
> > (RENESAS_ROM_STATUS_NO_RESULT) instead of success / fail.
> >
> > If the ROM load indeed failed this leads to failures when trying to
> > communicate with the controller later on.
> >
> > Attempt to load firmware using RAM load in those cases.
> >
> > Fixes: 2478be82de44 ("usb: renesas-xhci: Add ROM loader for uPD720201")
> > Cc: [email protected]
> > Cc: Mathias Nyman <[email protected]>
> > Cc: Greg Kroah-Hartman <[email protected]>
> > Cc: Vinod Koul <[email protected]>
> > Tested-by: Vinod Koul <[email protected]>
> > Reviewed-by: Vinod Koul <[email protected]>
> > Signed-off-by: Moritz Fischer <[email protected]>
> > Link: https://lore.kernel.org/r/[email protected]
> > Signed-off-by: Greg Kroah-Hartman <[email protected]>
> >
>
> After sending out 5.12.17 for testing, we had a user complain that all
> of their USB devices disappeared with the error:
>
> Jul 15 23:18:53 kernel: xhci_hcd 0000:04:00.0: Direct firmware load
> for renesas_usb_fw.mem failed with error -2
> Jul 15 23:18:53 kernel: xhci_hcd 0000:04:00.0: request_firmware failed: -2
> Jul 15 23:18:53 kernel: xhci_hcd: probe of 0000:04:00.0 failed with error -2
>
> After first assuming that something was missing in the backport to
> 5.12, I had the user try 5.13.2, and then 5.14-rc1. Both of those
> failed in the same way, so it is not working in the current Linus tree
> either. Reverting this patch fixed the issue.

Can you send a revert for this so I can get that into Linus's tree and
then all stable releases as well?

thanks,

greg k-h

2021-07-17 19:49:40

by Moritz Fischer

[permalink] [raw]
Subject: Re: [PATCH 5.13 024/800] usb: renesas-xhci: Fix handling of unknown ROM state

On Sat, Jul 17, 2021 at 07:34:51PM +0200, Greg Kroah-Hartman wrote:
> On Sat, Jul 17, 2021 at 08:39:19AM -0500, Justin Forbes wrote:
> > On Mon, Jul 12, 2021 at 2:31 AM Greg Kroah-Hartman
> > <[email protected]> wrote:
> > >
> > > From: Moritz Fischer <[email protected]>
> > >
> > > commit d143825baf15f204dac60acdf95e428182aa3374 upstream.
> > >
> > > The ROM load sometimes seems to return an unknown status
> > > (RENESAS_ROM_STATUS_NO_RESULT) instead of success / fail.
> > >
> > > If the ROM load indeed failed this leads to failures when trying to
> > > communicate with the controller later on.
> > >
> > > Attempt to load firmware using RAM load in those cases.
> > >
> > > Fixes: 2478be82de44 ("usb: renesas-xhci: Add ROM loader for uPD720201")
> > > Cc: [email protected]
> > > Cc: Mathias Nyman <[email protected]>
> > > Cc: Greg Kroah-Hartman <[email protected]>
> > > Cc: Vinod Koul <[email protected]>
> > > Tested-by: Vinod Koul <[email protected]>
> > > Reviewed-by: Vinod Koul <[email protected]>
> > > Signed-off-by: Moritz Fischer <[email protected]>
> > > Link: https://lore.kernel.org/r/[email protected]
> > > Signed-off-by: Greg Kroah-Hartman <[email protected]>
> > >
> >
> > After sending out 5.12.17 for testing, we had a user complain that all
> > of their USB devices disappeared with the error:
> >
> > Jul 15 23:18:53 kernel: xhci_hcd 0000:04:00.0: Direct firmware load
> > for renesas_usb_fw.mem failed with error -2
> > Jul 15 23:18:53 kernel: xhci_hcd 0000:04:00.0: request_firmware failed: -2
> > Jul 15 23:18:53 kernel: xhci_hcd: probe of 0000:04:00.0 failed with error -2
> >
> > After first assuming that something was missing in the backport to
> > 5.12, I had the user try 5.13.2, and then 5.14-rc1. Both of those
> > failed in the same way, so it is not working in the current Linus tree
> > either. Reverting this patch fixed the issue.
>
> Can you send a revert for this so I can get that into Linus's tree and
> then all stable releases as well?
>
> thanks,
>
> greg k-h

Me or Justin? I can do it. This is annoying my system doesn't work
without this :-(

Back to the drawing board I guess ...

Justin, any idea if your customer had the eeprom populated and
programmed or not, just as additional datapoint.

- Moritz

2021-07-17 22:36:23

by Moritz Fischer

[permalink] [raw]
Subject: Re: [PATCH 5.13 024/800] usb: renesas-xhci: Fix handling of unknown ROM state

Justin,

On Sat, Jul 17, 2021 at 08:39:19AM -0500, Justin Forbes wrote:
> On Mon, Jul 12, 2021 at 2:31 AM Greg Kroah-Hartman
> <[email protected]> wrote:
> >
> > From: Moritz Fischer <[email protected]>
> >
> > commit d143825baf15f204dac60acdf95e428182aa3374 upstream.
> >
> > The ROM load sometimes seems to return an unknown status
> > (RENESAS_ROM_STATUS_NO_RESULT) instead of success / fail.
> >
> > If the ROM load indeed failed this leads to failures when trying to
> > communicate with the controller later on.
> >
> > Attempt to load firmware using RAM load in those cases.
> >
> > Fixes: 2478be82de44 ("usb: renesas-xhci: Add ROM loader for uPD720201")
> > Cc: [email protected]
> > Cc: Mathias Nyman <[email protected]>
> > Cc: Greg Kroah-Hartman <[email protected]>
> > Cc: Vinod Koul <[email protected]>
> > Tested-by: Vinod Koul <[email protected]>
> > Reviewed-by: Vinod Koul <[email protected]>
> > Signed-off-by: Moritz Fischer <[email protected]>
> > Link: https://lore.kernel.org/r/[email protected]
> > Signed-off-by: Greg Kroah-Hartman <[email protected]>
> >
>
> After sending out 5.12.17 for testing, we had a user complain that all
> of their USB devices disappeared with the error:
>
> Jul 15 23:18:53 kernel: xhci_hcd 0000:04:00.0: Direct firmware load
> for renesas_usb_fw.mem failed with error -2
> Jul 15 23:18:53 kernel: xhci_hcd 0000:04:00.0: request_firmware failed: -2
> Jul 15 23:18:53 kernel: xhci_hcd: probe of 0000:04:00.0 failed with error -2

This looks like it fails finding the actual firmware file (ENOENT). Any
chance you could give this a whirl on top of the original patch?

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 18c2bbddf080..cde8f6f1ec5d 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -379,7 +379,11 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
driver_data = (struct xhci_driver_data *)id->driver_data;
if (driver_data && driver_data->quirks & XHCI_RENESAS_FW_QUIRK) {
retval = renesas_xhci_check_request_fw(dev, id);
- if (retval)
+ /*
+ * If firmware wasn't found there's still a chance this might work without
+ * loading firmware on some systems, so let's try at least.
+ */
+ if (retval && retval != -ENOENT)
return retval;
}


Thanks,
Moritz

2021-07-19 16:48:05

by Justin Forbes

[permalink] [raw]
Subject: Re: [PATCH 5.13 024/800] usb: renesas-xhci: Fix handling of unknown ROM state

On Sat, Jul 17, 2021 at 2:48 PM Moritz Fischer <[email protected]> wrote:
>
> On Sat, Jul 17, 2021 at 07:34:51PM +0200, Greg Kroah-Hartman wrote:
> > On Sat, Jul 17, 2021 at 08:39:19AM -0500, Justin Forbes wrote:
> > > On Mon, Jul 12, 2021 at 2:31 AM Greg Kroah-Hartman
> > > <[email protected]> wrote:
> > > >
> > > > From: Moritz Fischer <[email protected]>
> > > >
> > > > commit d143825baf15f204dac60acdf95e428182aa3374 upstream.
> > > >
> > > > The ROM load sometimes seems to return an unknown status
> > > > (RENESAS_ROM_STATUS_NO_RESULT) instead of success / fail.
> > > >
> > > > If the ROM load indeed failed this leads to failures when trying to
> > > > communicate with the controller later on.
> > > >
> > > > Attempt to load firmware using RAM load in those cases.
> > > >
> > > > Fixes: 2478be82de44 ("usb: renesas-xhci: Add ROM loader for uPD720201")
> > > > Cc: [email protected]
> > > > Cc: Mathias Nyman <[email protected]>
> > > > Cc: Greg Kroah-Hartman <[email protected]>
> > > > Cc: Vinod Koul <[email protected]>
> > > > Tested-by: Vinod Koul <[email protected]>
> > > > Reviewed-by: Vinod Koul <[email protected]>
> > > > Signed-off-by: Moritz Fischer <[email protected]>
> > > > Link: https://lore.kernel.org/r/[email protected]
> > > > Signed-off-by: Greg Kroah-Hartman <[email protected]>
> > > >
> > >
> > > After sending out 5.12.17 for testing, we had a user complain that all
> > > of their USB devices disappeared with the error:
> > >
> > > Jul 15 23:18:53 kernel: xhci_hcd 0000:04:00.0: Direct firmware load
> > > for renesas_usb_fw.mem failed with error -2
> > > Jul 15 23:18:53 kernel: xhci_hcd 0000:04:00.0: request_firmware failed: -2
> > > Jul 15 23:18:53 kernel: xhci_hcd: probe of 0000:04:00.0 failed with error -2
> > >
> > > After first assuming that something was missing in the backport to
> > > 5.12, I had the user try 5.13.2, and then 5.14-rc1. Both of those
> > > failed in the same way, so it is not working in the current Linus tree
> > > either. Reverting this patch fixed the issue.
> >
> > Can you send a revert for this so I can get that into Linus's tree and
> > then all stable releases as well?
> >
> > thanks,
> >
> > greg k-h
>
> Me or Justin? I can do it. This is annoying my system doesn't work
> without this :-(
>
> Back to the drawing board I guess ...
>
> Justin, any idea if your customer had the eeprom populated and
> programmed or not, just as additional datapoint.

I am not sure, but I did have another user chime in on the bug saying
the test kernel with the revert fixed their system as well. It was a
T14s AMD
Generation_1. The original reporter had a ASUS System Product
Name/PRIME H370M-PLUS, BIOS 2801 04/13/2021.

Justin

> - Moritz

2021-07-19 16:59:09

by Justin Forbes

[permalink] [raw]
Subject: Re: [PATCH 5.13 024/800] usb: renesas-xhci: Fix handling of unknown ROM state

On Sat, Jul 17, 2021 at 5:33 PM Moritz Fischer <[email protected]> wrote:
>
> Justin,
>
> On Sat, Jul 17, 2021 at 08:39:19AM -0500, Justin Forbes wrote:
> > On Mon, Jul 12, 2021 at 2:31 AM Greg Kroah-Hartman
> > <[email protected]> wrote:
> > >
> > > From: Moritz Fischer <[email protected]>
> > >
> > > commit d143825baf15f204dac60acdf95e428182aa3374 upstream.
> > >
> > > The ROM load sometimes seems to return an unknown status
> > > (RENESAS_ROM_STATUS_NO_RESULT) instead of success / fail.
> > >
> > > If the ROM load indeed failed this leads to failures when trying to
> > > communicate with the controller later on.
> > >
> > > Attempt to load firmware using RAM load in those cases.
> > >
> > > Fixes: 2478be82de44 ("usb: renesas-xhci: Add ROM loader for uPD720201")
> > > Cc: [email protected]
> > > Cc: Mathias Nyman <[email protected]>
> > > Cc: Greg Kroah-Hartman <[email protected]>
> > > Cc: Vinod Koul <[email protected]>
> > > Tested-by: Vinod Koul <[email protected]>
> > > Reviewed-by: Vinod Koul <[email protected]>
> > > Signed-off-by: Moritz Fischer <[email protected]>
> > > Link: https://lore.kernel.org/r/[email protected]
> > > Signed-off-by: Greg Kroah-Hartman <[email protected]>
> > >
> >
> > After sending out 5.12.17 for testing, we had a user complain that all
> > of their USB devices disappeared with the error:
> >
> > Jul 15 23:18:53 kernel: xhci_hcd 0000:04:00.0: Direct firmware load
> > for renesas_usb_fw.mem failed with error -2
> > Jul 15 23:18:53 kernel: xhci_hcd 0000:04:00.0: request_firmware failed: -2
> > Jul 15 23:18:53 kernel: xhci_hcd: probe of 0000:04:00.0 failed with error -2
>
> This looks like it fails finding the actual firmware file (ENOENT). Any
> chance you could give this a whirl on top of the original patch?
>

Sure. test kernel building now, will let you know when the user reports back.

Justin

> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index 18c2bbddf080..cde8f6f1ec5d 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -379,7 +379,11 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
> driver_data = (struct xhci_driver_data *)id->driver_data;
> if (driver_data && driver_data->quirks & XHCI_RENESAS_FW_QUIRK) {
> retval = renesas_xhci_check_request_fw(dev, id);
> - if (retval)
> + /*
> + * If firmware wasn't found there's still a chance this might work without
> + * loading firmware on some systems, so let's try at least.
> + */
> + if (retval && retval != -ENOENT)
> return retval;
> }
>
>
> Thanks,
> Moritz

2021-07-20 03:03:56

by Justin Forbes

[permalink] [raw]
Subject: Re: [PATCH 5.13 024/800] usb: renesas-xhci: Fix handling of unknown ROM state

On Mon, Jul 19, 2021 at 10:33 AM Justin Forbes <[email protected]> wrote:
>
> On Sat, Jul 17, 2021 at 5:33 PM Moritz Fischer <[email protected]> wrote:
> >
> > Justin,
> >
> > On Sat, Jul 17, 2021 at 08:39:19AM -0500, Justin Forbes wrote:
> > > On Mon, Jul 12, 2021 at 2:31 AM Greg Kroah-Hartman
> > > <[email protected]> wrote:
> > > >
> > > > From: Moritz Fischer <[email protected]>
> > > >
> > > > commit d143825baf15f204dac60acdf95e428182aa3374 upstream.
> > > >
> > > > The ROM load sometimes seems to return an unknown status
> > > > (RENESAS_ROM_STATUS_NO_RESULT) instead of success / fail.
> > > >
> > > > If the ROM load indeed failed this leads to failures when trying to
> > > > communicate with the controller later on.
> > > >
> > > > Attempt to load firmware using RAM load in those cases.
> > > >
> > > > Fixes: 2478be82de44 ("usb: renesas-xhci: Add ROM loader for uPD720201")
> > > > Cc: [email protected]
> > > > Cc: Mathias Nyman <[email protected]>
> > > > Cc: Greg Kroah-Hartman <[email protected]>
> > > > Cc: Vinod Koul <[email protected]>
> > > > Tested-by: Vinod Koul <[email protected]>
> > > > Reviewed-by: Vinod Koul <[email protected]>
> > > > Signed-off-by: Moritz Fischer <[email protected]>
> > > > Link: https://lore.kernel.org/r/[email protected]
> > > > Signed-off-by: Greg Kroah-Hartman <[email protected]>
> > > >
> > >
> > > After sending out 5.12.17 for testing, we had a user complain that all
> > > of their USB devices disappeared with the error:
> > >
> > > Jul 15 23:18:53 kernel: xhci_hcd 0000:04:00.0: Direct firmware load
> > > for renesas_usb_fw.mem failed with error -2
> > > Jul 15 23:18:53 kernel: xhci_hcd 0000:04:00.0: request_firmware failed: -2
> > > Jul 15 23:18:53 kernel: xhci_hcd: probe of 0000:04:00.0 failed with error -2
> >
> > This looks like it fails finding the actual firmware file (ENOENT). Any
> > chance you could give this a whirl on top of the original patch?
> >
>
> Sure. test kernel building now, will let you know when the user reports back.

The original user reports success with this patch on top of the original patch.

Justin

>
> Justin
>
> > diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> > index 18c2bbddf080..cde8f6f1ec5d 100644
> > --- a/drivers/usb/host/xhci-pci.c
> > +++ b/drivers/usb/host/xhci-pci.c
> > @@ -379,7 +379,11 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
> > driver_data = (struct xhci_driver_data *)id->driver_data;
> > if (driver_data && driver_data->quirks & XHCI_RENESAS_FW_QUIRK) {
> > retval = renesas_xhci_check_request_fw(dev, id);
> > - if (retval)
> > + /*
> > + * If firmware wasn't found there's still a chance this might work without
> > + * loading firmware on some systems, so let's try at least.
> > + */
> > + if (retval && retval != -ENOENT)
> > return retval;
> > }
> >
> >
> > Thanks,
> > Moritz

2021-07-20 05:40:01

by Moritz Fischer

[permalink] [raw]
Subject: Re: [PATCH 5.13 024/800] usb: renesas-xhci: Fix handling of unknown ROM state

On Mon, Jul 19, 2021 at 09:57:00PM -0500, Justin Forbes wrote:
> On Mon, Jul 19, 2021 at 10:33 AM Justin Forbes <[email protected]> wrote:
> >
> > On Sat, Jul 17, 2021 at 5:33 PM Moritz Fischer <[email protected]> wrote:
> > >
> > > Justin,
> > >
> > > On Sat, Jul 17, 2021 at 08:39:19AM -0500, Justin Forbes wrote:
> > > > On Mon, Jul 12, 2021 at 2:31 AM Greg Kroah-Hartman
> > > > <[email protected]> wrote:
> > > > >
> > > > > From: Moritz Fischer <[email protected]>
> > > > >
> > > > > commit d143825baf15f204dac60acdf95e428182aa3374 upstream.
> > > > >
> > > > > The ROM load sometimes seems to return an unknown status
> > > > > (RENESAS_ROM_STATUS_NO_RESULT) instead of success / fail.
> > > > >
> > > > > If the ROM load indeed failed this leads to failures when trying to
> > > > > communicate with the controller later on.
> > > > >
> > > > > Attempt to load firmware using RAM load in those cases.
> > > > >
> > > > > Fixes: 2478be82de44 ("usb: renesas-xhci: Add ROM loader for uPD720201")
> > > > > Cc: [email protected]
> > > > > Cc: Mathias Nyman <[email protected]>
> > > > > Cc: Greg Kroah-Hartman <[email protected]>
> > > > > Cc: Vinod Koul <[email protected]>
> > > > > Tested-by: Vinod Koul <[email protected]>
> > > > > Reviewed-by: Vinod Koul <[email protected]>
> > > > > Signed-off-by: Moritz Fischer <[email protected]>
> > > > > Link: https://lore.kernel.org/r/[email protected]
> > > > > Signed-off-by: Greg Kroah-Hartman <[email protected]>
> > > > >
> > > >
> > > > After sending out 5.12.17 for testing, we had a user complain that all
> > > > of their USB devices disappeared with the error:
> > > >
> > > > Jul 15 23:18:53 kernel: xhci_hcd 0000:04:00.0: Direct firmware load
> > > > for renesas_usb_fw.mem failed with error -2
> > > > Jul 15 23:18:53 kernel: xhci_hcd 0000:04:00.0: request_firmware failed: -2
> > > > Jul 15 23:18:53 kernel: xhci_hcd: probe of 0000:04:00.0 failed with error -2
> > >
> > > This looks like it fails finding the actual firmware file (ENOENT). Any
> > > chance you could give this a whirl on top of the original patch?
> > >
> >
> > Sure. test kernel building now, will let you know when the user reports back.
>
> The original user reports success with this patch on top of the original patch.

That's good news I guess.

After reading through the datasheet once more I'm even more convinced
that the original code with the early return in
renesas_check_fw_running() is *very* shady.

There are three statuses to be investigated
- FW load status (fw_state)
- ROM download status (rom_status)
- Firmware version as reported by chip

Currently there the code takes an early return if the latter says the
external ROM is there and the 'write firmware to external ROM'
worked out, which I think shouldn't be happening, since it doesn't tell
us anything about the firmware state at all. In fact I think the early
return should not exist at all (a path that the original patch made more
likely to happen).

The FW load status indicates whether firmware has been runtime loaded
and returns 'No result yet' in your case, too I suspect, which *might*
happen if the chip configured itself from external ROM?

So the part that is unclear to me somewhat is should we use either of
them at all in trying to determine whether we should load firmware?

Maybe what we should do is:
- Attempt to request_firmware()
- If fail -> proceed and hope for the best
- If success
- Compare the firmware file version with the version reported by the
controller
- If they don't match, load firmware, otherwise leave it alone?

- Moritz


>
> Justin
>
> >
> > Justin
> >
> > > diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> > > index 18c2bbddf080..cde8f6f1ec5d 100644
> > > --- a/drivers/usb/host/xhci-pci.c
> > > +++ b/drivers/usb/host/xhci-pci.c
> > > @@ -379,7 +379,11 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
> > > driver_data = (struct xhci_driver_data *)id->driver_data;
> > > if (driver_data && driver_data->quirks & XHCI_RENESAS_FW_QUIRK) {
> > > retval = renesas_xhci_check_request_fw(dev, id);
> > > - if (retval)
> > > + /*
> > > + * If firmware wasn't found there's still a chance this might work without
> > > + * loading firmware on some systems, so let's try at least.
> > > + */
> > > + if (retval && retval != -ENOENT)
> > > return retval;
> > > }
> > >
> > >
> > > Thanks,
> > > Moritz

2021-07-28 17:54:01

by Niklaus vimja Hofer

[permalink] [raw]
Subject: Re: [PATCH 5.13 024/800] usb: renesas-xhci: Fix handling of unknown ROM state

Hi,

On Mon, 2021-07-19 10:28, Justin Forbes wrote:
> On Sat, Jul 17, 2021 at 2:48 PM Moritz Fischer <[email protected]> wrote:
> >
> > On Sat, Jul 17, 2021 at 07:34:51PM +0200, Greg Kroah-Hartman wrote:
> > > On Sat, Jul 17, 2021 at 08:39:19AM -0500, Justin Forbes wrote:
> > > > On Mon, Jul 12, 2021 at 2:31 AM Greg Kroah-Hartman
> > > > <[email protected]> wrote:
> > > > >
> > > > > From: Moritz Fischer <[email protected]>
> > > > >
> > > > > commit d143825baf15f204dac60acdf95e428182aa3374 upstream.
> > > > >
> > > > > The ROM load sometimes seems to return an unknown status
> > > > > (RENESAS_ROM_STATUS_NO_RESULT) instead of success / fail.
> > > > >
> > > > > If the ROM load indeed failed this leads to failures when trying to
> > > > > communicate with the controller later on.
> > > > >
> > > > > Attempt to load firmware using RAM load in those cases.
> > > > >
> > > > > Fixes: 2478be82de44 ("usb: renesas-xhci: Add ROM loader for uPD720201")
> > > > > Cc: [email protected]
> > > > > Cc: Mathias Nyman <[email protected]>
> > > > > Cc: Greg Kroah-Hartman <[email protected]>
> > > > > Cc: Vinod Koul <[email protected]>
> > > > > Tested-by: Vinod Koul <[email protected]>
> > > > > Reviewed-by: Vinod Koul <[email protected]>
> > > > > Signed-off-by: Moritz Fischer <[email protected]>
> > > > > Link: https://lore.kernel.org/r/[email protected]
> > > > > Signed-off-by: Greg Kroah-Hartman <[email protected]>
> > > > >
> > > >
> > > > After sending out 5.12.17 for testing, we had a user complain that all
> > > > of their USB devices disappeared with the error:
> > > >
> > > > Jul 15 23:18:53 kernel: xhci_hcd 0000:04:00.0: Direct firmware load
> > > > for renesas_usb_fw.mem failed with error -2
> > > > Jul 15 23:18:53 kernel: xhci_hcd 0000:04:00.0: request_firmware failed: -2
> > > > Jul 15 23:18:53 kernel: xhci_hcd: probe of 0000:04:00.0 failed with error -2
> > > >
> > > > After first assuming that something was missing in the backport to
> > > > 5.12, I had the user try 5.13.2, and then 5.14-rc1. Both of those
> > > > failed in the same way, so it is not working in the current Linus tree
> > > > either. Reverting this patch fixed the issue.
> > >
> > > Can you send a revert for this so I can get that into Linus's tree and
> > > then all stable releases as well?
> > >
> > > thanks,
> > >
> > > greg k-h
> >
> > Me or Justin? I can do it. This is annoying my system doesn't work
> > without this :-(
> >
> > Back to the drawing board I guess ...
> >
> > Justin, any idea if your customer had the eeprom populated and
> > programmed or not, just as additional datapoint.
>
> I am not sure, but I did have another user chime in on the bug saying
> the test kernel with the revert fixed their system as well. It was a
> T14s AMD
> Generation_1. The original reporter had a ASUS System Product
> Name/PRIME H370M-PLUS, BIOS 2801 04/13/2021.

This solved the same issue on my T14 AMD Gen 1 (non-s variant). I was
using 5.13.5 where it was not working and I was able to observe the same
error message in `dmesg`. Reverted the patch, recompiled, rebooted and
now it's working just fine.

Sincerely
--
Niklaus 'vimja' Hofer
[email protected]
xmpp: [email protected]