[U-Boot] [PATCH 1/2 V2] SF: Add driver for Gigabyte device GD25LQ and GD25Q64B
Jagan Teki
jagannadh.teki at gmail.com
Tue May 28 08:29:54 CEST 2013
Any update on this.
Was this patch refereed for denx tree?
Thanks,
Jagan.
On Tue, May 21, 2013 at 6:40 PM, Jagan Teki <jagannadh.teki at gmail.com> wrote:
> Hi,
>
> I think this reviewed already, but have a very few comments.
>
> On Wed, Jan 23, 2013 at 12:00 PM, Rajeshwari Shinde
> <rajeshwari.s at samsung.com> wrote:
>> This patch adds driver for the gigabyte devices
>> GD25LQ and GD25Q64B required for Snow Board.
>>
>> Signed-off-by: Rajeshwari Shinde <rajeshwari.s at samsung.com>
>> ---
>> 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 <banajit.g at samsung.com>
>> + *
>> + * 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.
More information about the U-Boot
mailing list