Any update on this. Was this patch refereed for denx tree? Thanks, Jagan.
On Tue, May 21, 2013 at 6:40 PM, Jagan Teki <[email protected]> wrote: > Hi, > > I think this reviewed already, but have a very few comments. > > On Wed, Jan 23, 2013 at 12:00 PM, Rajeshwari Shinde > <[email protected]> wrote: >> This patch adds driver for the gigabyte devices >> GD25LQ and GD25Q64B required for Snow Board. >> >> Signed-off-by: Rajeshwari Shinde <[email protected]> >> --- >> Changes in V2: >> - Added U-Boot copyright header to gigadevice.c >> - Removed unnecessary blank lines. >> drivers/mtd/spi/Makefile | 1 + >> drivers/mtd/spi/gigadevice.c | 81 >> ++++++++++++++++++++++++++++++++++ >> drivers/mtd/spi/spi_flash.c | 3 + >> drivers/mtd/spi/spi_flash_internal.h | 1 + >> 4 files changed, 86 insertions(+), 0 deletions(-) >> create mode 100644 drivers/mtd/spi/gigadevice.c >> >> diff --git a/drivers/mtd/spi/Makefile b/drivers/mtd/spi/Makefile >> index 90f8392..ecbb210 100644 >> --- a/drivers/mtd/spi/Makefile >> +++ b/drivers/mtd/spi/Makefile >> @@ -32,6 +32,7 @@ endif >> COBJS-$(CONFIG_SPI_FLASH) += spi_flash.o >> COBJS-$(CONFIG_SPI_FLASH_ATMEL) += atmel.o >> COBJS-$(CONFIG_SPI_FLASH_EON) += eon.o >> +COBJS-$(CONFIG_SPI_FLASH_GIGADEVICE) += gigadevice.o >> COBJS-$(CONFIG_SPI_FLASH_MACRONIX) += macronix.o >> COBJS-$(CONFIG_SPI_FLASH_SPANSION) += spansion.o >> COBJS-$(CONFIG_SPI_FLASH_SST) += sst.o >> diff --git a/drivers/mtd/spi/gigadevice.c b/drivers/mtd/spi/gigadevice.c >> new file mode 100644 >> index 0000000..b5e1ebe >> --- /dev/null >> +++ b/drivers/mtd/spi/gigadevice.c >> @@ -0,0 +1,81 @@ >> +/* >> + * Gigadevice SPI flash driver >> + * Copyright 2013, Samsung Electronics Co., Ltd. >> + * Author: Banajit Goswami <[email protected]> >> + * >> + * See file CREDITS for list of people who contributed to this >> + * project. >> + * >> + * This program is free software; you can redistribute it and/or >> + * modify it under the terms of the GNU General Public License as >> + * published by the Free Software Foundation; either version 2 of >> + * the License, or (at your option) any later version. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program; if not, write to the Free Software >> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, >> + * MA 02111-1307 USA >> + */ >> + >> +#include <common.h> >> +#include <malloc.h> >> +#include <spi_flash.h> >> + >> +#include "spi_flash_internal.h" >> + >> +struct gigadevice_spi_flash_params { >> + uint16_t id; >> + uint16_t nr_blocks; > > I think it's better to use u16 instead of uint16_t, uin16_t will get > back to arch include from include/linux which does u16 for directly > for first time. > >> + const char *name; >> +}; >> + >> +static const struct gigadevice_spi_flash_params >> gigadevice_spi_flash_table[] = { >> + { >> + .id = 0x6016, >> + .nr_blocks = 64, >> + .name = "GD25LQ", >> + }, >> + { >> + .id = 0x4017, >> + .nr_blocks = 128, >> + .name = "GD25Q64B", >> + }, > > Better to use clean code shape like.. > { > .id = 0x60, > .nr_blocks = 64, > .name = "GD25LQ", > } > >> +}; >> + >> +struct spi_flash *spi_flash_probe_gigadevice(struct spi_slave *spi, u8 >> *idcode) >> +{ >> + const struct gigadevice_spi_flash_params *params; >> + struct spi_flash *flash; >> + unsigned int i; >> + >> + for (i = 0; i < ARRAY_SIZE(gigadevice_spi_flash_table); i++) { >> + params = &gigadevice_spi_flash_table[i]; >> + if (params->id == ((idcode[1] << 8) | idcode[2])) >> + break; >> + } >> + >> + if (i == ARRAY_SIZE(gigadevice_spi_flash_table)) { >> + debug("SF: Unsupported Gigadevice ID %02x%02x\n", >> + idcode[1], idcode[2]); >> + return NULL; >> + } >> + >> + flash = spi_flash_alloc_base(spi, params->name); >> + if (!flash) { >> + debug("SF: Failed to allocate memory\n"); >> + return NULL; >> + } > > better to add a space here > >> + /* page_size */ >> + flash->page_size = 256; >> + /* sector_size = page_size * pages_per_sector */ >> + flash->sector_size = flash->page_size * 16; >> + /* size = sector_size * sector_per_block * number of blocks */ >> + flash->size = flash->sector_size * 16 * params->nr_blocks; > > comments on above size calculations are good, but not that much > important i guess. > > And also please provide a stand' link for flash part data sheet on > commit message, if possible. > I thought it's a good to refers don't no this sounds valid.. > > Thanks, > Jagan. -- -- Thanks, Jagan. _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
