Re: [flashrom] [PATCH] Return code for spi_read_status_register

[Stefan Tauner]

On Sun, 03 Jun 2012 16:51:18 +0200
Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> wrote:

  spi_read_status_register now returns success/failure
and stores the
 status register value via call-by-reference. That way, status register
 reading failure (only possible if the programmer couldn't run RDSR) can
 be detected and acted upon in the future.
with this patch this failure is largely ignored and i am not sure that
this does not change behavior due to the 0xff status value that is
returned on errors.

  Signed-off-by: Carl-Daniel Hailfinger
<c-d.hailfinger.devel.2006(a)gmx.net>
 
 Index: flashrom-spi_rdsr_errorcheck/it87spi.c
 ===================================================================
 --- flashrom-spi_rdsr_errorcheck/it87spi.c	(Revision 1539)
 +++ flashrom-spi_rdsr_errorcheck/it87spi.c	(Arbeitskopie)
 @@ -321,6 +321,7 @@
  {
  	unsigned int i;
  	int result;
 +	uint8_t status;
  	chipaddr bios = flash->virtual_memory;
  
  	result = spi_write_enable(flash);
 @@ -335,7 +336,7 @@
  	/* Wait until the Write-In-Progress bit is cleared.
  	 * This usually takes 1-10 ms, so wait in 1 ms steps.
  	 */
 -	while (spi_read_status_register(flash) & SPI_SR_WIP)
 +	while (spi_read_status_register(flash, &status), status & SPI_SR_WIP)
  		programmer_delay(1000);
  	return 0;
for example here it would be easy to store the return value of
spi_read_status_register and return it instead of the fixed 0

   }
 Index: flashrom-spi_rdsr_errorcheck/a25.c
 ===================================================================
 --- flashrom-spi_rdsr_errorcheck/a25.c	(Revision 1539)
 +++ flashrom-spi_rdsr_errorcheck/a25.c	(Arbeitskopie)
 @@ -33,7 +33,7 @@
  {
  	uint8_t status;
  
 -	status = spi_read_status_register(flash);
 +	spi_read_status_register(flash, &status);
  	msg_cdbg("Chip status register is %02x\n", status);
here the output
might change depending on what the programmers'
send_spi_command routine "return" in case of errors etc.

in general i like the idea very much, but i would rather see a
"complete" patch, that does acts appropriately on errors... try to
convince me if you disagree :)
-- 
Kind regards/Mit freundlichen Grüßen, Stefan Tauner