eeprom: at25: fix read_poll_timeout argument order

[PATCH] eeprom: at25: fix read_poll_timeout argument order

Posted by Markus Breitenberger 1 month ago

From: Markus Breitenberger <bre@keba.com>

Commit 8ad6249c51d0 ("eeprom: at25: convert to spi-mem API") passes
the arguments to read_poll_timeout() in the wrong order. It supplies false
as sleep_us, USEC_PER_MSEC as timeout_us, and USEC_PER_MSEC * EE_TIMEOUT
as sleep_before_read.

Consequently, at25_wait_ready() busy-polls with a timeout of only 1 ms
instead of sleeping between polls and timing out after 25 ms. EEPROMs which
keep the write-in-progress bit set for longer than 1 ms are reported as
timed out, although the page may still be programmed after the driver
returns. A multi-page write is then aborted after the first page, leaving a
partial write.

This was reproduced with a 25LC080: a 32-byte write spanning two 16-byte
pages returned -ETIMEDOUT after programming only the first page.

Pass the polling interval, timeout, and sleep_before_read argument in their
proper positions.

Fixes: 8ad6249c51d0 ("eeprom: at25: convert to spi-mem API")
Cc: stable@vger.kernel.org
Signed-off-by: Markus Breitenberger <bre@keba.com>
---
 drivers/nvmem/at25.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/nvmem/at25.c b/drivers/nvmem/at25.c
index bc2cfb75d9bb4..9e71c6a859471 100644
--- a/drivers/nvmem/at25.c
+++ b/drivers/nvmem/at25.c
@@ -224,8 +224,8 @@ static int at25_wait_ready(struct at25_data *at25)
 					   SPI_MEM_OP_DATA_IN(1, bounce, 1));
 
 	read_poll_timeout(spi_mem_exec_op, status,
-			  status || !(bounce[0] & AT25_SR_nRDY), false,
-			  USEC_PER_MSEC, USEC_PER_MSEC * EE_TIMEOUT,
+			  status || !(bounce[0] & AT25_SR_nRDY),
+			  USEC_PER_MSEC, USEC_PER_MSEC * EE_TIMEOUT, false,
 			  at25->spimem, &op);
 	if (status < 0)
 		return status;
-- 
2.47.3

Re: [PATCH] eeprom: at25: fix read_poll_timeout argument order

Posted by Francesco Valla 2 weeks, 2 days ago

Hi Markus,

On Wed, Aug 26, 2026 at 02:13:34PM +0200, Markus Breitenberger wrote:
> From: Markus Breitenberger <bre@keba.com>
> 
> Commit 8ad6249c51d0 ("eeprom: at25: convert to spi-mem API") passes
> the arguments to read_poll_timeout() in the wrong order. It supplies false
> as sleep_us, USEC_PER_MSEC as timeout_us, and USEC_PER_MSEC * EE_TIMEOUT
> as sleep_before_read.
> 
> Consequently, at25_wait_ready() busy-polls with a timeout of only 1 ms
> instead of sleeping between polls and timing out after 25 ms. EEPROMs which
> keep the write-in-progress bit set for longer than 1 ms are reported as
> timed out, although the page may still be programmed after the driver
> returns. A multi-page write is then aborted after the first page, leaving a
> partial write.
> 
> This was reproduced with a 25LC080: a 32-byte write spanning two 16-byte
> pages returned -ETIMEDOUT after programming only the first page.
> 

This impacted also a device equipped with a M95512W EEPROM from ST, for
which I saw consistent write timeouts (even if on a second read the
memory contains the intended value). I can confirm your fix solved the
issue.

Tested-by: Francesco Valla <francesco@valla.it>

> Pass the polling interval, timeout, and sleep_before_read argument in their
> proper positions.
> 
> Fixes: 8ad6249c51d0 ("eeprom: at25: convert to spi-mem API")
> Cc: stable@vger.kernel.org
> Signed-off-by: Markus Breitenberger <bre@keba.com>
> ---
>  drivers/nvmem/at25.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/nvmem/at25.c b/drivers/nvmem/at25.c
> index bc2cfb75d9bb4..9e71c6a859471 100644
> --- a/drivers/nvmem/at25.c
> +++ b/drivers/nvmem/at25.c
> @@ -224,8 +224,8 @@ static int at25_wait_ready(struct at25_data *at25)
>  					   SPI_MEM_OP_DATA_IN(1, bounce, 1));
>  
>  	read_poll_timeout(spi_mem_exec_op, status,
> -			  status || !(bounce[0] & AT25_SR_nRDY), false,
> -			  USEC_PER_MSEC, USEC_PER_MSEC * EE_TIMEOUT,
> +			  status || !(bounce[0] & AT25_SR_nRDY),
> +			  USEC_PER_MSEC, USEC_PER_MSEC * EE_TIMEOUT, false,
>  			  at25->spimem, &op);
>  	if (status < 0)
>  		return status;
> -- 
> 2.47.3
> 

Thank you!

Regards,

Francesco

Re: [PATCH] eeprom: at25: fix read_poll_timeout argument order

Posted by Sverdlin, Alexander 1 month ago

Hi Markus!

On Wed, 2026-08-26 at 14:13 +0200, Markus Breitenberger wrote:
> From: Markus Breitenberger <bre@keba.com>
> 
> Commit 8ad6249c51d0 ("eeprom: at25: convert to spi-mem API") passes
> the arguments to read_poll_timeout() in the wrong order. It supplies false
> as sleep_us, USEC_PER_MSEC as timeout_us, and USEC_PER_MSEC * EE_TIMEOUT
> as sleep_before_read.
> 
> Consequently, at25_wait_ready() busy-polls with a timeout of only 1 ms
> instead of sleeping between polls and timing out after 25 ms. EEPROMs which
> keep the write-in-progress bit set for longer than 1 ms are reported as
> timed out, although the page may still be programmed after the driver
> returns. A multi-page write is then aborted after the first page, leaving a
> partial write.
> 
> This was reproduced with a 25LC080: a 32-byte write spanning two 16-byte
> pages returned -ETIMEDOUT after programming only the first page.
> 
> Pass the polling interval, timeout, and sleep_before_read argument in their
> proper positions.
> 
> Fixes: 8ad6249c51d0 ("eeprom: at25: convert to spi-mem API")
> Cc: stable@vger.kernel.org
> Signed-off-by: Markus Breitenberger <bre@keba.com>

Thank you for the patch!
I'm sorry, my patch caused a regression for you!

Your change both look correct to me and doesn't cause a regression for
the FRAM chips I've performed the conversion for:

Reviewed-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>
Tested-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>

> ---
>  drivers/nvmem/at25.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/nvmem/at25.c b/drivers/nvmem/at25.c
> index bc2cfb75d9bb4..9e71c6a859471 100644
> --- a/drivers/nvmem/at25.c
> +++ b/drivers/nvmem/at25.c
> @@ -224,8 +224,8 @@ static int at25_wait_ready(struct at25_data *at25)
>  					   SPI_MEM_OP_DATA_IN(1, bounce, 1));
>  
>  	read_poll_timeout(spi_mem_exec_op, status,
> -			  status || !(bounce[0] & AT25_SR_nRDY), false,
> -			  USEC_PER_MSEC, USEC_PER_MSEC * EE_TIMEOUT,
> +			  status || !(bounce[0] & AT25_SR_nRDY),
> +			  USEC_PER_MSEC, USEC_PER_MSEC * EE_TIMEOUT, false,
>  			  at25->spimem, &op);
>  	if (status < 0)
>  		return status;

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com