On Wed, Jun 5, 2013 at 10:10 AM, Insop Song <insop.s...@cohere.net> wrote: > Hi Jagan, > > Thank you for your feedback. > >> -----Original Message----- >> From: Jagan Teki [mailto:jagannadh.t...@gmail.com] >> Sent: Monday, June 03, 2013 10:31 PM >> To: Insop Song >> Cc: u-boot@lists.denx.de; york...@freescale.com >> Subject: Re: [U-Boot] [PATCH] Add SPI Flash STMicro's N25Q512A & add flag >> status check during SPI Flash write >> > >> > I've made a change and add spi_flash_cmd_wait_program to align with >> existing code structure. >> > Please see the patch below. >> > Erase is okay without checking, so I didn't add the check. >> >> No i see without the check in erase when i erase and read back i couldn't see >> 0xff Please check. > > I've tested on the N25Q512A erase without the flag check went okay. > However, adding the check to the erase would be safe, and I've tested erase > with flag status check. > Here is the diff of this change. I will use the format-patch when we finalize > the change. > > diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c > index 4240e9d..9cf5aaf 100644 > --- a/drivers/mtd/spi/spi_flash.c > +++ b/drivers/mtd/spi/spi_flash.c > @@ -247,6 +247,10 @@ int spi_flash_cmd_erase(struct spi_flash *flash, u32 > offset, size_t len) > if (ret) > goto out; > > + ret = spi_flash_cmd_wait_program(flash, SPI_FLASH_PROG_TIMEOUT); > + if (ret) > + break; > + > ret = spi_flash_cmd_wait_ready(flash, > SPI_FLASH_PAGE_ERASE_TIMEOUT); > if (ret) > goto out; > > >> > >> > I am not sure I'd agree with you on that you are putting the device check >> > in >> spi_flash_cmd_poll_bit(). >> > I am more inclined to make the change at the level where we call >> spi_flash_cmd_poll_bit() if we have to distinguish per devices. >> >> spi_flash_cmd_poll_bit() is common call for poll for read_status, as write >> call >> is common to all making changes to >> spi_flash_cmd_poll_bit() is valid i guess. >> Write call is a global which is used by >> so many user, i don't like to add the flag status there and make the >> confusion >> user. >> > > To your comment on "confusion to users", I agree on that. > With that argument, your patch is better since it hides the flag status check. > > 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. -- Thanks, Jagan. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
