[U-Boot] [PATCH v2 6/7] sf: Update status reg check in spi_flash_cmd_wait_ready

[Marek Vasut]

On Monday, August 17, 2015 at 12:32:53 PM, Jagan Teki wrote:
> Current flash wait_ready logic is not modular to add new
> register status check, hence few of the logic is used from
> Linux spi-nor framework.
> 
> Below are the sf speed runs with 'sf update' on whole flash, 16MiB.
> 
> => sf update 0x100 0x0 0x1000000
> device 0 whole chip
> 16777216 bytes written, 0 bytes skipped in 59.564s, speed 289262 B/s
> 
> => sf update 0x100 0x0 0x1000000
> device 0 whole chip
> 16777216 bytes written, 0 bytes skipped in 62.549s, speed 275036 B/s
> 
> => sf update 0x100 0x0 0x1000000
> device 0 whole chip
> 16777216 bytes written, 0 bytes skipped in 61.276s, speed 284359 B/s
> 
> Signed-off-by: Jagan Teki <jteki at openedev.com>
> Cc: Simon Glass <sjg at chromium.org>
> Cc: Marek Vasut <marex at denx.de>
> Cc: Michal Simek <michal.simek at xilinx.com>
> Cc: Siva Durga Prasad Paladugu <sivadur at xilinx.com>
> Cc: Stefan Roese <sr at denx.de>
> Cc: Tom Warren <twarren at nvidia.com>
> Cc: Bin Meng <bmeng.cn at gmail.com>
> Cc: Tom Rini <trini at konsulko.com>
> Tested-by: Jagan Teki <jteki at openedev.com>
> ---
>  drivers/mtd/spi/sf_ops.c | 76
> ++++++++++++------------------------------------ 1 file changed, 18
> insertions(+), 58 deletions(-)
> 
> diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
> index 0f34450..7d7c264 100644
> --- a/drivers/mtd/spi/sf_ops.c
> +++ b/drivers/mtd/spi/sf_ops.c
> @@ -138,72 +138,32 @@ static void spi_flash_dual_flash(struct spi_flash
> *flash, u32 *addr) }
>  #endif
> 
> -static int spi_flash_poll_status(struct spi_slave *spi, unsigned long
> timeout, -				 u8 cmd, u8 poll_bit)
> +/*
> + * Service routine to read status register until ready, or timeout occurs.
> + * Returns non-zero if error.

If you're adding this sort of description, please make it a kerneldoc.

> + */
> +int spi_flash_cmd_wait_ready(struct spi_flash *flash, unsigned long
> deadline) {

Please rename $deadline to $timeout and note in the kerneldoc that the
timeout is in mSec.

> -	unsigned long timebase;
> -	unsigned long flags = SPI_XFER_BEGIN;
> -	int ret;
> -	u8 status;
> -	u8 check_status = 0x0;
> +	int timeout, ret, sr;
> 
> -	if (cmd == CMD_FLAG_STATUS)
> -		check_status = poll_bit;
> +	timeout = get_timer(0);

This should be $start or something.

> -#ifdef CONFIG_SF_DUAL_FLASH
> -	if (spi->flags & SPI_XFER_U_PAGE)
> -		flags |= SPI_XFER_U_PAGE;
> -#endif
> -	ret = spi_xfer(spi, 8, &cmd, NULL, flags);
> -	if (ret) {
> -		debug("SF: fail to read %s status register\n",
> -		      cmd == CMD_READ_STATUS ? "read" : "flag");
> -		return ret;
> -	}
> -
> -	timebase = get_timer(0);
> -	do {
> -		WATCHDOG_RESET();
> -
> -		ret = spi_xfer(spi, 8, NULL, &status, 0);
> -		if (ret)
> -			return -1;
> +	while (get_timer(timeout) < deadline) {
> +		ret = spi_flash_cmd_read_status(flash, &sr);
> +		if (ret < 0)
> +			return ret;
> 
> -		if ((status & poll_bit) == check_status)
> +		if (sr < 0)
>  			break;
> +		else if (!(sr & STATUS_WIP))
> +			return 0;
> 
> -	} while (get_timer(timebase) < timeout);
> -
> -	spi_xfer(spi, 0, NULL, NULL, SPI_XFER_END);
> -
> -	if ((status & poll_bit) == check_status)
> -		return 0;
> -
> -	/* Timed out */
> -	debug("SF: time out!\n");
> -	return -1;
> -}
> -
> -int spi_flash_cmd_wait_ready(struct spi_flash *flash, unsigned long
> timeout) -{
> -	struct spi_slave *spi = flash->spi;
> -	int ret;
> -	u8 poll_bit = STATUS_WIP;
> -	u8 cmd = CMD_READ_STATUS;
> -
> -	ret = spi_flash_poll_status(spi, timeout, cmd, poll_bit);
> -	if (ret < 0)
> -		return ret;
> -
> -	if (flash->poll_cmd == CMD_FLAG_STATUS) {
> -		poll_bit = STATUS_PEC;
> -		cmd = CMD_FLAG_STATUS;
> -		ret = spi_flash_poll_status(spi, timeout, cmd, poll_bit);
> -		if (ret < 0)
> -			return ret;
> +		cond_resched();

Huh ? Why do you need cond_resched() here at all ? Drop it please.

>  	}
> 
> -	return 0;
> +	printf("SF: Timeout!\n");
> +
> +	return -ETIMEDOUT;
>  }
> 
>  int spi_flash_write_common(struct spi_flash *flash, const u8 *cmd,