[U-Boot] [PATCH] Add SPI Flash STMicro's N25Q512A & add flag status check during SPI Flash write

[Insop Song]

> -----Original Message-----
> From: Jagan Teki [mailto:jagannadh.teki at gmail.com]
> Sent: Wednesday, June 05, 2013 2:00 AM
> To: Insop Song
> Cc: u-boot at lists.denx.de; yorksun at freescale.com
> Subject: Re: [U-Boot] [PATCH] Add SPI Flash STMicro's N25Q512A & add flag
> status check during SPI Flash write
> > And how about this?  can we add "flag_status_check" flag somewhere, and
> if the device is needed then we set the flag during device discovery.
> > Then call the flag check function if the flag is set.
> > I think this way might be more generic then what you did in your patch.
> >
> > What do you think?
> >
> > Thank you,
> 
> Can u just send pseudo code about your logic, i couldn't get u exactly sorry.
> 

Here is what I think, it is similar with your patch in the sense that "do the different status" check for different device.
However, the difference is that your patch checks based on device type at the status checking function.
Mine is proposing to add the information to the devices' struct (spi_flash) instead, so each device can have a flexibility to call flag status check. In this way if new device use this flag_status check, you can just hide the setting in devices' probe function, and not worrying about updating generic function call (spi_flash_cmd_poll_bit() as in your patch)

I am not so happy to add "flag_status" in struct spi_flash; so I am open to your idea.
Please see below and let me know what you think?




// 1. adding flag status
struct spi_flash {
	struct spi_slave *spi;

	const char	*name;

	/* Total flash size */
	u32		size;
	/* Write (page) size */
	u32		page_size;
	/* Erase (sector) size */
	u32		sector_size;

	/* if flag_status is use or not */
	u8		flag_status;

	int		(*read)(struct spi_flash *flash, u32 offset,
				size_t len, void *buf);
	int		(*write)(struct spi_flash *flash, u32 offset,
				size_t len, const void *buf);
	int		(*erase)(struct spi_flash *flash, u32 offset,
				size_t len);
};

// 2. in probe function, set the flag_status per vendor


struct spi_flash *spi_flash_probe_stmicro(struct spi_slave *spi, u8 * idcode)
{

	flash->spi = spi;
	flash->name = params->name;

	flash->write = spi_flash_cmd_write_multi;
	flash->erase = spi_flash_cmd_erase;
	flash->read = spi_flash_cmd_read_fast;
	flash->page_size = 256;
	flash->sector_size = 256 * params->pages_per_sector;
	flash->size = flash->sector_size * params->nr_sectors;

	// we do this for Micron, and will do the same if any other device needs this
	flash->flag_status = 1;

	return flash;
}


// 3. call flag status check if flash->flag_status is set


int spi_flash_cmd_wait_ready(struct spi_flash *flash, unsigned long timeout)
{
	if (flash->flag_status)
		return spi_flash_cmd_poll_bit(flash, timeout,
			CMD_READ_FLAG_STATUS, STATUS_PEC, STATUS_PEC);
	else
		return spi_flash_cmd_poll_bit(flash, timeout,
			CMD_READ_STATUS, STATUS_WIP, 0);
}


Thank you,

IS