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

[Mathias Nyman <mathias.nyman@linux.intel.com>]

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