Dear Bo Shen, On 22.08.2012 03:33, Bo Shen wrote: > Hi Andreas, > > On 8/21/2012 19:26, Andreas Bießmann wrote: >> Dear Bo Shen, >> >> On 20.08.2012 08:32, Bo Shen wrote: >>> Using common spi flash operation function to replace private operation >>> funtion >>> >>> This patch is based on http://patchwork.ozlabs.org/patch/177896/ >>> which has been merged by Mike frysinger >> >> Mike, do you think this is a fix? Should it go into 2012.10? Will you >> take it? >> >>> Signed-off-by: Bo Shen <[email protected]> >>> --- >>> drivers/mtd/spi/atmel.c | 11 ++++++++--- >>> 1 file changed, 8 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/mtd/spi/atmel.c b/drivers/mtd/spi/atmel.c >>> index 89ebe9d..006f6d5 100644 >>> --- a/drivers/mtd/spi/atmel.c >>> +++ b/drivers/mtd/spi/atmel.c >>> @@ -518,13 +518,19 @@ struct spi_flash *spi_flash_probe_atmel(struct >>> spi_slave *spi, u8 *idcode) >>> asf->flash.erase = dataflash_erase_p2; >>> } >>> >>> + asf->flash.page_size = page_size; >>> + asf->flash.sector_size = page_size; >>> break; >>> >>> case DF_FAMILY_AT26F: >>> case DF_FAMILY_AT26DF: >>> asf->flash.read = spi_flash_cmd_read_fast; >>> - asf->flash.write = dataflash_write_p2; >>> - asf->flash.erase = dataflash_erase_p2; >>> + asf->flash.write = spi_flash_cmd_write_multi; >>> + asf->flash.erase = spi_flash_cmd_erase; >>> + asf->flash.page_size = page_size; >>> + asf->flash.sector_size = 4096; >> >> why do you take fixed value for sector size here? > > The fixed number fit for both AT25 and AT26 serials. So, I take it. > > Actually, we can calculate it as: q> asf->flash.sector_size = page_size * params->pages_per_block * > params->blocks_per_sector > So, may I need to change it?
I got it. The current driver for AT45 is written to only support page erase, therefore the page_size == sector_size. The AT26 on the other hand will now use the generic API which uses sector erase instead. Therefore we have different values at all, am I right? The only supported DF of at26 type is currently [1] which in fact has 4KiB erase size. Unfortunately others (at least [2]) have another layout. But this could be adopted if such a chip is supported in the future. >> >>> + /* clear SPRL# bit for locked flash */ >>> + spi_flash_cmd_write_status(&asf->flash, 0); >>> break; >>> >>> default: >>> @@ -532,7 +538,6 @@ struct spi_flash *spi_flash_probe_atmel(struct >>> spi_slave *spi, u8 *idcode) >>> goto err; >>> } >>> >>> - asf->flash.sector_size = page_size; >>> asf->flash.size = page_size * params->pages_per_block >>> * params->blocks_per_sector >>> * params->nr_sectors; >> >> And here you correlate number of sectors with page size ... but we may >> have page_size != sector_size for some at26 devices, aren't we? > > This is from old driver and this line is moved away. > For at26 devices, it is used the fixed number 4096 for sector size. > Please take following as a reference. > --------------------------------------------- > case DF_FAMILY_AT26F: > case DF_FAMILY_AT26DF: > asf->flash.read = spi_flash_cmd_read_fast; > - asf->flash.write = dataflash_write_p2; > - asf->flash.erase = dataflash_erase_p2; > + asf->flash.write = spi_flash_cmd_write_multi; > + asf->flash.erase = spi_flash_cmd_erase; > + asf->flash.page_size = page_size; > + asf->flash.sector_size = 4096; > --------------------------------------------- Yea, I got that already. The only thing I asked why you correlate these sector related parameters (blocks_per_sector and nr_sectors) with the page_size. I think this change is Ok. Acked-by: Andreas Bießmann <[email protected]> @Mike: As I understand you I should take patch #2 to #4 while you will take this one, am I right? [1] http://www.atmel.com/Images/doc3633.pdf [2] http://www.atmel.com/Images/doc3495.pdf _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
