[PATCH] usb: renesas-xhci: Fix External ROM access timeouts

[PATCH] usb: renesas-xhci: Fix External ROM access timeouts

[Marek Vasut]

Increase the External ROM access timeouts to prevent failures during
programming of External SPI EEPROM chips. The current timeouts are
too short for some SPI EEPROMs used with uPD720201 controllers.

The current timeout for Chip Erase in renesas_rom_erase() is 100 ms ,
the current timeout for Sector Erase issued by the controller before
Page Program in renesas_fw_download_image() is also 100 ms. Neither
timeout is sufficient for e.g. the Macronix MX25L5121E or MX25V5126F.

MX25L5121E reference manual [1] page 35 section "ERASE AND PROGRAMMING
PERFORMANCE" and page 23 section "Table 8. AC CHARACTERISTICS (Temperature
= 0°C to 70°C for Commercial grade, VCC = 2.7V ~ 3.6V)" row "tCE" indicate
that the maximum time required for Chip Erase opcode to complete is 2 s,
and for Sector Erase it is 300 ms .

MX25V5126F reference manual [2] page 47 section "13. ERASE AND PROGRAMMING
PERFORMANCE (2.3V - 3.6V)" and page 42 section "Table 8. AC CHARACTERISTICS
(Temperature = -40°C to 85°C for Industrial grade, VCC = 2.3V - 3.6V)" row
"tCE" indicate that the maximum time required for Chip Erase opcode to
complete is 3.2 s, and for Sector Erase it is 400 ms .

Update the timeouts such, that Chip Erase timeout is set to 5 seconds,
and Sector Erase timeout is set to 500 ms. Such lengthy timeouts ought
to be sufficient for majority of SPI EEPROM chips.

[1] https://www.macronix.com/Lists/Datasheet/Attachments/8634/MX25L5121E,%203V,%20512Kb,%20v1.3.pdf
[2] https://www.macronix.com/Lists/Datasheet/Attachments/8750/MX25V5126F,%202.5V,%20512Kb,%20v1.1.pdf

Fixes: 2478be82de44 ("usb: renesas-xhci: Add ROM loader for uPD720201")
Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Mathias Nyman <mathias.nyman@intel.com>
Cc: Vinod Koul <vkoul@kernel.org>
Cc: <stable@vger.kernel.org>
Cc: linux-renesas-soc@vger.kernel.org
Cc: linux-usb@vger.kernel.org
---
 drivers/usb/host/xhci-pci-renesas.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci-pci-renesas.c b/drivers/usb/host/xhci-pci-renesas.c
index 620f8f0febb8..86df80399c9f 100644
--- a/drivers/usb/host/xhci-pci-renesas.c
+++ b/drivers/usb/host/xhci-pci-renesas.c
@@ -47,8 +47,9 @@
 #define RENESAS_ROM_ERASE_MAGIC				0x5A65726F
 #define RENESAS_ROM_WRITE_MAGIC				0x53524F4D
 
-#define RENESAS_RETRY	10000
-#define RENESAS_DELAY	10
+#define RENESAS_RETRY			50000	/* 50000 * RENESAS_DELAY ~= 500ms */
+#define RENESAS_CHIP_ERASE_RETRY	500000	/* 500000 * RENESAS_DELAY ~= 5s */
+#define RENESAS_DELAY			10
 
 #define RENESAS_FW_NAME	"renesas_usb_fw.mem"
 
@@ -407,7 +408,7 @@ static void renesas_rom_erase(struct pci_dev *pdev)
 	/* sleep a bit while ROM is erased */
 	msleep(20);
 
-	for (i = 0; i < RENESAS_RETRY; i++) {
+	for (i = 0; i < RENESAS_CHIP_ERASE_RETRY; i++) {
 		retval = pci_read_config_byte(pdev, RENESAS_ROM_STATUS,
 					      &status);
 		status &= RENESAS_ROM_STATUS_ERASE;
-- 
2.47.2

Re: [PATCH] usb: renesas-xhci: Fix External ROM access timeouts

[Greg Kroah-Hartman]

On Sun, Jul 27, 2025 at 05:44:16PM +0200, Marek Vasut wrote:
> Increase the External ROM access timeouts to prevent failures during
> programming of External SPI EEPROM chips. The current timeouts are
> too short for some SPI EEPROMs used with uPD720201 controllers.
> 
> The current timeout for Chip Erase in renesas_rom_erase() is 100 ms ,
> the current timeout for Sector Erase issued by the controller before
> Page Program in renesas_fw_download_image() is also 100 ms. Neither
> timeout is sufficient for e.g. the Macronix MX25L5121E or MX25V5126F.
> 
> MX25L5121E reference manual [1] page 35 section "ERASE AND PROGRAMMING
> PERFORMANCE" and page 23 section "Table 8. AC CHARACTERISTICS (Temperature
> = 0°C to 70°C for Commercial grade, VCC = 2.7V ~ 3.6V)" row "tCE" indicate
> that the maximum time required for Chip Erase opcode to complete is 2 s,
> and for Sector Erase it is 300 ms .
> 
> MX25V5126F reference manual [2] page 47 section "13. ERASE AND PROGRAMMING
> PERFORMANCE (2.3V - 3.6V)" and page 42 section "Table 8. AC CHARACTERISTICS
> (Temperature = -40°C to 85°C for Industrial grade, VCC = 2.3V - 3.6V)" row
> "tCE" indicate that the maximum time required for Chip Erase opcode to
> complete is 3.2 s, and for Sector Erase it is 400 ms .
> 
> Update the timeouts such, that Chip Erase timeout is set to 5 seconds,
> and Sector Erase timeout is set to 500 ms. Such lengthy timeouts ought
> to be sufficient for majority of SPI EEPROM chips.
> 
> [1] https://www.macronix.com/Lists/Datasheet/Attachments/8634/MX25L5121E,%203V,%20512Kb,%20v1.3.pdf
> [2] https://www.macronix.com/Lists/Datasheet/Attachments/8750/MX25V5126F,%202.5V,%20512Kb,%20v1.1.pdf
> 
> Fixes: 2478be82de44 ("usb: renesas-xhci: Add ROM loader for uPD720201")
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> ---
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Mathias Nyman <mathias.nyman@intel.com>
> Cc: Vinod Koul <vkoul@kernel.org>
> Cc: <stable@vger.kernel.org>
> Cc: linux-renesas-soc@vger.kernel.org
> Cc: linux-usb@vger.kernel.org
> ---
>  drivers/usb/host/xhci-pci-renesas.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-pci-renesas.c b/drivers/usb/host/xhci-pci-renesas.c
> index 620f8f0febb8..86df80399c9f 100644
> --- a/drivers/usb/host/xhci-pci-renesas.c
> +++ b/drivers/usb/host/xhci-pci-renesas.c
> @@ -47,8 +47,9 @@
>  #define RENESAS_ROM_ERASE_MAGIC				0x5A65726F
>  #define RENESAS_ROM_WRITE_MAGIC				0x53524F4D
>  
> -#define RENESAS_RETRY	10000
> -#define RENESAS_DELAY	10
> +#define RENESAS_RETRY			50000	/* 50000 * RENESAS_DELAY ~= 500ms */
> +#define RENESAS_CHIP_ERASE_RETRY	500000	/* 500000 * RENESAS_DELAY ~= 5s */
> +#define RENESAS_DELAY			10
>  
>  #define RENESAS_FW_NAME	"renesas_usb_fw.mem"
>  
> @@ -407,7 +408,7 @@ static void renesas_rom_erase(struct pci_dev *pdev)
>  	/* sleep a bit while ROM is erased */
>  	msleep(20);
>  
> -	for (i = 0; i < RENESAS_RETRY; i++) {
> +	for (i = 0; i < RENESAS_CHIP_ERASE_RETRY; i++) {
>  		retval = pci_read_config_byte(pdev, RENESAS_ROM_STATUS,
>  					      &status);
>  		status &= RENESAS_ROM_STATUS_ERASE;
> -- 
> 2.47.2
> 
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- You have marked a patch with a "Fixes:" tag for a commit that is in an
  older released kernel, yet you do not have a cc: stable line in the
  signed-off-by area at all, which means that the patch will not be
  applied to any older kernel releases.  To properly fix this, please
  follow the documented rules in the
  Documentation/process/stable-kernel-rules.rst file for how to resolve
  this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

Re: [PATCH] usb: renesas-xhci: Fix External ROM access timeouts

[Marek Vasut]

On 7/28/25 6:18 AM, Greg Kroah-Hartman wrote:

[...]

>> Fixes: 2478be82de44 ("usb: renesas-xhci: Add ROM loader for uPD720201")
>> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
>> ---
>> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: Mathias Nyman <mathias.nyman@intel.com>
>> Cc: Vinod Koul <vkoul@kernel.org>
>> Cc: <stable@vger.kernel.org>

[...]

> - You have marked a patch with a "Fixes:" tag for a commit that is in an
>    older released kernel, yet you do not have a cc: stable line in the
>    signed-off-by area at all, which means that the patch will not be
>    applied to any older kernel releases.  To properly fix this, please
>    follow the documented rules in the
>    Documentation/process/stable-kernel-rules.rst file for how to resolve
>    this.

Maybe the bot should take into consideration Cc: stable below the --- 
too ? Or is that considered invalid ?

-- 
Best regards,
Marek Vasut

Re: [PATCH] usb: renesas-xhci: Fix External ROM access timeouts

[Greg Kroah-Hartman]

On Tue, Jul 29, 2025 at 05:09:55AM +0200, Marek Vasut wrote:
> On 7/28/25 6:18 AM, Greg Kroah-Hartman wrote:
> 
> [...]
> 
> > > Fixes: 2478be82de44 ("usb: renesas-xhci: Add ROM loader for uPD720201")
> > > Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> > > ---
> > > Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > Cc: Mathias Nyman <mathias.nyman@intel.com>
> > > Cc: Vinod Koul <vkoul@kernel.org>
> > > Cc: <stable@vger.kernel.org>
> 
> [...]
> 
> > - You have marked a patch with a "Fixes:" tag for a commit that is in an
> >    older released kernel, yet you do not have a cc: stable line in the
> >    signed-off-by area at all, which means that the patch will not be
> >    applied to any older kernel releases.  To properly fix this, please
> >    follow the documented rules in the
> >    Documentation/process/stable-kernel-rules.rst file for how to resolve
> >    this.
> 
> Maybe the bot should take into consideration Cc: stable below the --- too ?
> Or is that considered invalid ?

That is totally invalid, it gets cut off when the patch is applied and
then is lost :(

Re: [PATCH] usb: renesas-xhci: Fix External ROM access timeouts

[Geert Uytterhoeven]

Hi Greg,

On Tue, 29 Jul 2025 at 07:03, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Tue, Jul 29, 2025 at 05:09:55AM +0200, Marek Vasut wrote:
> > On 7/28/25 6:18 AM, Greg Kroah-Hartman wrote:
> >
> > [...]
> >
> > > > Fixes: 2478be82de44 ("usb: renesas-xhci: Add ROM loader for uPD720201")
> > > > Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> > > > ---
> > > > Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> > > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > Cc: Mathias Nyman <mathias.nyman@intel.com>
> > > > Cc: Vinod Koul <vkoul@kernel.org>
> > > > Cc: <stable@vger.kernel.org>
> >
> > [...]
> >
> > > - You have marked a patch with a "Fixes:" tag for a commit that is in an
> > >    older released kernel, yet you do not have a cc: stable line in the
> > >    signed-off-by area at all, which means that the patch will not be
> > >    applied to any older kernel releases.  To properly fix this, please
> > >    follow the documented rules in the
> > >    Documentation/process/stable-kernel-rules.rst file for how to resolve
> > >    this.
> >
> > Maybe the bot should take into consideration Cc: stable below the --- too ?
> > Or is that considered invalid ?
>
> That is totally invalid, it gets cut off when the patch is applied and
> then is lost :(

But the "Fix" keyword in the oneline-summary and the "Fixes" tag are
not, so your stable minions will still take it ;-)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] usb: renesas-xhci: Fix External ROM access timeouts

[Greg Kroah-Hartman]

On Tue, Jul 29, 2025 at 09:11:46AM +0200, Geert Uytterhoeven wrote:
> Hi Greg,
> 
> On Tue, 29 Jul 2025 at 07:03, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Tue, Jul 29, 2025 at 05:09:55AM +0200, Marek Vasut wrote:
> > > On 7/28/25 6:18 AM, Greg Kroah-Hartman wrote:
> > >
> > > [...]
> > >
> > > > > Fixes: 2478be82de44 ("usb: renesas-xhci: Add ROM loader for uPD720201")
> > > > > Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> > > > > ---
> > > > > Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> > > > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > > Cc: Mathias Nyman <mathias.nyman@intel.com>
> > > > > Cc: Vinod Koul <vkoul@kernel.org>
> > > > > Cc: <stable@vger.kernel.org>
> > >
> > > [...]
> > >
> > > > - You have marked a patch with a "Fixes:" tag for a commit that is in an
> > > >    older released kernel, yet you do not have a cc: stable line in the
> > > >    signed-off-by area at all, which means that the patch will not be
> > > >    applied to any older kernel releases.  To properly fix this, please
> > > >    follow the documented rules in the
> > > >    Documentation/process/stable-kernel-rules.rst file for how to resolve
> > > >    this.
> > >
> > > Maybe the bot should take into consideration Cc: stable below the --- too ?
> > > Or is that considered invalid ?
> >
> > That is totally invalid, it gets cut off when the patch is applied and
> > then is lost :(
> 
> But the "Fix" keyword in the oneline-summary and the "Fixes" tag are
> not, so your stable minions will still take it ;-)

{sigh}

No, please NEVER rely on that.  Grabbing "Fixes:" only patches are a
"best effort" thing that we do that is not reliable at all, AND you
never get a FAILED notification if something does not actually apply
properly.

The documentation clearly states how to mark something for the stable
tree, and that is NOT by only haveing a "Fixes:" tag in the commit.

thanks,

greg k-h

Re: [PATCH] usb: renesas-xhci: Fix External ROM access timeouts

[Michał Pecio]

Hi,

On Sun, 27 Jul 2025 17:44:16 +0200, Marek Vasut wrote:
> Increase the External ROM access timeouts to prevent failures during
> programming of External SPI EEPROM chips. The current timeouts are
> too short for some SPI EEPROMs used with uPD720201 controllers.
> 
> The current timeout for Chip Erase in renesas_rom_erase() is 100 ms ,
> the current timeout for Sector Erase issued by the controller before
> Page Program in renesas_fw_download_image() is also 100 ms. Neither
> timeout is sufficient for e.g. the Macronix MX25L5121E or MX25V5126F.

Out of curiosity, who uses this ROM update functionality and why?

It seems weird to write nonvolatile memories in a PCI probe routine.
Boards or PCIe cards fitted with ROMs are programmed with working
firmware at the factory and there ought to be no need to touch that.

And if you want to update this FW, dropping a file in /lib/firmware/
and loading a kernel module is not the usual (or convenient) UI...

Regards,
Michal

Re: [PATCH] usb: renesas-xhci: Fix External ROM access timeouts

[Mathias Nyman]

On 27.7.2025 18.44, Marek Vasut wrote:
> Increase the External ROM access timeouts to prevent failures during
> programming of External SPI EEPROM chips. The current timeouts are
> too short for some SPI EEPROMs used with uPD720201 controllers.
> 
> The current timeout for Chip Erase in renesas_rom_erase() is 100 ms ,
> the current timeout for Sector Erase issued by the controller before
> Page Program in renesas_fw_download_image() is also 100 ms. Neither
> timeout is sufficient for e.g. the Macronix MX25L5121E or MX25V5126F.
> 
> MX25L5121E reference manual [1] page 35 section "ERASE AND PROGRAMMING
> PERFORMANCE" and page 23 section "Table 8. AC CHARACTERISTICS (Temperature
> = 0°C to 70°C for Commercial grade, VCC = 2.7V ~ 3.6V)" row "tCE" indicate
> that the maximum time required for Chip Erase opcode to complete is 2 s,
> and for Sector Erase it is 300 ms .
> 
> MX25V5126F reference manual [2] page 47 section "13. ERASE AND PROGRAMMING
> PERFORMANCE (2.3V - 3.6V)" and page 42 section "Table 8. AC CHARACTERISTICS
> (Temperature = -40°C to 85°C for Industrial grade, VCC = 2.3V - 3.6V)" row
> "tCE" indicate that the maximum time required for Chip Erase opcode to
> complete is 3.2 s, and for Sector Erase it is 400 ms .
> 
> Update the timeouts such, that Chip Erase timeout is set to 5 seconds,
> and Sector Erase timeout is set to 500 ms. Such lengthy timeouts ought
> to be sufficient for majority of SPI EEPROM chips.
> 
> [1] https://www.macronix.com/Lists/Datasheet/Attachments/8634/MX25L5121E,%203V,%20512Kb,%20v1.3.pdf
> [2] https://www.macronix.com/Lists/Datasheet/Attachments/8750/MX25V5126F,%202.5V,%20512Kb,%20v1.1.pdf
> 
> Fixes: 2478be82de44 ("usb: renesas-xhci: Add ROM loader for uPD720201")
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> ---
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Mathias Nyman <mathias.nyman@intel.com>
> Cc: Vinod Koul <vkoul@kernel.org>
> Cc: <stable@vger.kernel.org>
> Cc: linux-renesas-soc@vger.kernel.org
> Cc: linux-usb@vger.kernel.org
> ---
>   drivers/usb/host/xhci-pci-renesas.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-pci-renesas.c b/drivers/usb/host/xhci-pci-renesas.c
> index 620f8f0febb8..86df80399c9f 100644
> --- a/drivers/usb/host/xhci-pci-renesas.c
> +++ b/drivers/usb/host/xhci-pci-renesas.c
> @@ -47,8 +47,9 @@
>   #define RENESAS_ROM_ERASE_MAGIC				0x5A65726F
>   #define RENESAS_ROM_WRITE_MAGIC				0x53524F4D
>   
> -#define RENESAS_RETRY	10000
> -#define RENESAS_DELAY	10
> +#define RENESAS_RETRY			50000	/* 50000 * RENESAS_DELAY ~= 500ms */
> +#define RENESAS_CHIP_ERASE_RETRY	500000	/* 500000 * RENESAS_DELAY ~= 5s */
> +#define RENESAS_DELAY			10

No objection, just making sure author is aware that RENESAS_RETRY is used in
_seven_ for loops, and this change will increase the timeout five-fold for
all of them.

Thanks
Mathias

Re: [PATCH] usb: renesas-xhci: Fix External ROM access timeouts

[Marek Vasut]

On 7/29/25 12:17 PM, Michał Pecio wrote:
> Hi,

Hi,

> On Sun, 27 Jul 2025 17:44:16 +0200, Marek Vasut wrote:
>> Increase the External ROM access timeouts to prevent failures during
>> programming of External SPI EEPROM chips. The current timeouts are
>> too short for some SPI EEPROMs used with uPD720201 controllers.
>>
>> The current timeout for Chip Erase in renesas_rom_erase() is 100 ms ,
>> the current timeout for Sector Erase issued by the controller before
>> Page Program in renesas_fw_download_image() is also 100 ms. Neither
>> timeout is sufficient for e.g. the Macronix MX25L5121E or MX25V5126F.
> 
> Out of curiosity, who uses this ROM update functionality and why?

arch/arm64/boot/dts/renesas/r8a779g3-sparrow-hawk.dts

The factory will likely use this code path to preload the SPI EEPROM 
during board production and testing.

> It seems weird to write nonvolatile memories in a PCI probe routine.
> Boards or PCIe cards fitted with ROMs are programmed with working
> firmware at the factory and there ought to be no need to touch that.

See above.

> And if you want to update this FW, dropping a file in /lib/firmware/
> and loading a kernel module is not the usual (or convenient) UI...

See above.

-- 
Best regards,
Marek Vasut

Re: [PATCH] usb: renesas-xhci: Fix External ROM access timeouts

[Marek Vasut]

On 7/29/25 5:20 PM, Mathias Nyman wrote:

Hi,

>> diff --git a/drivers/usb/host/xhci-pci-renesas.c b/drivers/usb/host/ 
>> xhci-pci-renesas.c
>> index 620f8f0febb8..86df80399c9f 100644
>> --- a/drivers/usb/host/xhci-pci-renesas.c
>> +++ b/drivers/usb/host/xhci-pci-renesas.c
>> @@ -47,8 +47,9 @@
>>   #define RENESAS_ROM_ERASE_MAGIC                0x5A65726F
>>   #define RENESAS_ROM_WRITE_MAGIC                0x53524F4D
>> -#define RENESAS_RETRY    10000
>> -#define RENESAS_DELAY    10
>> +#define RENESAS_RETRY            50000    /* 50000 * RENESAS_DELAY ~= 
>> 500ms */
>> +#define RENESAS_CHIP_ERASE_RETRY    500000    /* 500000 * 
>> RENESAS_DELAY ~= 5s */
>> +#define RENESAS_DELAY            10
> 
> No objection, just making sure author is aware that RENESAS_RETRY is 
> used in
> _seven_ for loops, and this change will increase the timeout five-fold for
> all of them.

Yes, I am aware this increases the timeout for all SPI EEPROM status 
polling loops in this driver.

The longer retry count would only have an impact in case something bad 
happened during SPI EEPROM programming. On user system, that should 
happen never -- user hardware should ship with already programmed SPI 
EEPROM, so this programming code is skipped. In factory, this might 
happen, but then it is likely a hardware defect and that hardware never 
reaches users.

-- 
Best regards,
Marek Vasut