| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <51D4FD7F.3020104@ti.com>
Date: Thu, 4 Jul 2013 10:13:43 +0530
From: Sourav Poddar <sourav.poddar@...com>
To: Florian Fainelli <florian@...nwrt.org>
CC: <artem.bityutskiy@...ux.intel.com>,
<linux-mtd@...ts.infradead.org>,
David Woodhouse <dwmw2@...radead.org>, <manonuevo@...ron.com>,
<tqnguyen@...ron.com>, <balbi@...com>, <rnayak@...com>,
<linux-omap@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCHv2] drivers: mtd: spinand: Add generic spinand frameowrk.
Hi,
On Wednesday 03 July 2013 10:47 PM, Florian Fainelli wrote:
> Hello,
>
> 2013/7/3 Sourav Poddar<sourav.poddar@...com>:
>> From: Mona Anonuevo<manonuevo@...ron.com>
>>
>> This patch adds support for a generic spinand framework(spinand_mtd.c).
>> This frameowrk can be used for other spi based flash devices. The idea
>> is to have a common model under drivers/mtd, as also present for other non spi
>> devices(there is a generic framework and device part simply attaches itself to it.)
> Resending my comments since your previous submissino
>
>> Signed-off-by: Mona Anonuevo<manonuevo@...ron.com>
>> Signed-off-by: Tuan Nguyen<tqnguyen@...ron.com>
>> Signed-off-by: Sourav Poddar<sourav.poddar@...com>
>> ----
>>
> [snip]
>
>> +if MTD_SPINAND
>> +
>> +config MTD_SPINAND_ONDIEECC
>> + bool "Use SPINAND internal ECC"
>> + help
>> + Internel ECC
>> +
>> +config MTD_SPINAND_SWECC
>> + bool "Use software ECC"
>> + depends on MTD_NAND
>> + help
>> + software ECC
> Can this somehow be made a runtime thing?
>
Ahh..I think we might opt for a device tree entry and based on that
check for ECC.
> [snip]
>
>> + if (count< oob_num&& ops->oobbuf&& chip->oobbuf) {
>> + int size;
>> + int offset, len, temp;
>> +
>> + /* repack spare to oob */
>> + memset(chip->oobbuf, 0, info->ecclayout->oobavail);
>> +
>> + temp = 0;
>> + offset = info->ecclayout->oobfree[0].offset;
>> + len = info->ecclayout->oobfree[0].length;
>> + memcpy(chip->oobbuf + temp,
>> + chip->buf + info->page_main_size + offset, len);
> Sounds like a for look might be useful here
>
I dont think so, there is a while loop above under which it happens.
We are increasing count at the bottom of the while loop. So, I think
this should work fine.
>> +
>> + temp += len;
>> + offset = info->ecclayout->oobfree[1].offset;
>> + len = info->ecclayout->oobfree[1].length;
>> + memcpy(chip->oobbuf + temp,
>> + chip->buf + info->page_main_size + offset, len);
>> +
>> + temp += len;
>> + offset = info->ecclayout->oobfree[2].offset;
>> + len = info->ecclayout->oobfree[2].length;
>> + memcpy(chip->oobbuf + temp,
>> + chip->buf + info->page_main_size + offset, len);
>> +
>> + temp += len;
>> + offset = info->ecclayout->oobfree[3].offset;
>> + len = info->ecclayout->oobfree[3].length;
>> + memcpy(chip->oobbuf + temp,
>> + chip->buf + info->page_main_size + offset, len);
>> +
> [snip]
>
>> + /* repack oob to spare */
>> + temp = 0;
>> + offset = info->ecclayout->oobfree[0].offset;
>> + len = info->ecclayout->oobfree[0].length;
>> + memcpy(chip->buf + info->page_main_size + offset,
>> + chip->oobbuf + temp, len);
> And here too.
>
>
Same as above.
>> +
>> + temp += len;
>> + offset = info->ecclayout->oobfree[1].offset;
>> + len = info->ecclayout->oobfree[1].length;
>> + memcpy(chip->buf + info->page_main_size + offset,
>> + chip->oobbuf + temp, len);
>> +
>> + temp += len;
>> + offset = info->ecclayout->oobfree[2].offset;
>> + len = info->ecclayout->oobfree[2].length;
>> + memcpy(chip->buf + info->page_main_size + offset,
>> + chip->oobbuf + temp, len);
>> +
>> + temp += len;
>> + offset = info->ecclayout->oobfree[3].offset;
>> + len = info->ecclayout->oobfree[3].length;
>> + memcpy(chip->buf + info->page_main_size + offset,
>> + chip->oobbuf + temp, len);
>> + }
> [snip]
>
>> +++ b/include/linux/mtd/spinand.h
>> @@ -0,0 +1,155 @@
>> +/*
>> + * linux/include/linux/mtd/spinand.h
>> + * Copyright (c) 2009-2010 Micron Technology, Inc.
>> + * This software is licensed under the terms of the GNU General Public
>> + * License version 2, as published by the Free Software Foundation, and
>> + * may be copied, distributed, and modified under those terms.
>> +
>> + * 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.
>> +/bin/bash: 4: command not found
>> + *
>> + * based on nand.h
>> + */
>> +#ifndef __LINUX_MTD_SPI_NAND_H
>> +#define __LINUX_MTD_SPI_NAND_H
>> +
>> +#include<linux/wait.h>
>> +#include<linux/spinlock.h>
>> +#include<linux/mtd/mtd.h>
>> +
>> +/* cmd */
>> +#define CMD_READ 0x13
>> +#define CMD_READ_RDM 0x03
>> +#define CMD_PROG_PAGE_CLRCACHE 0x02
>> +#define CMD_PROG_PAGE 0x84
>> +#define CMD_PROG_PAGE_EXC 0x10
>> +#define CMD_ERASE_BLK 0xd8
>> +#define CMD_WR_ENABLE 0x06
>> +#define CMD_WR_DISABLE 0x04
>> +#define CMD_READ_ID 0x9f
>> +#define CMD_RESET 0xff
>> +#define CMD_READ_REG 0x0f
>> +#define CMD_WRITE_REG 0x1f
> Can we somehow namespace these defines so they are not so generic?
> Something like SPI_NAND_CMD_READ would be just fine I guess.
>
I think SPI_CMD_READ will make more sense then, since these
commands will be applicable for other type of flash devices also.
>> +
>> +/* feature/ status reg */
>> +#define REG_BLOCK_LOCK 0xa0
>> +#define REG_OTP 0xb0
>> +#define REG_STATUS 0xc0/* timing */
>> +
>> +/* status */
>> +#define STATUS_OIP_MASK 0x01
>> +#define STATUS_READY (0<< 0)
>> +#define STATUS_BUSY (1<< 0)
>> +
>> +#define STATUS_E_FAIL_MASK 0x04
>> +#define STATUS_E_FAIL (1<< 2)
>> +
>> +#define STATUS_P_FAIL_MASK 0x08
>> +#define STATUS_P_FAIL (1<< 3)
>> +
>> +#define STATUS_ECC_MASK 0x30
>> +#define STATUS_ECC_1BIT_CORRECTED (1<< 4)
>> +#define STATUS_ECC_ERROR (2<< 4)
>> +#define STATUS_ECC_RESERVED (3<< 4)
>> +
>> +
>> +/*ECC enable defines*/
>> +#define OTP_ECC_MASK 0x10
>> +#define OTP_ECC_OFF 0
>> +#define OTP_ECC_ON 1
>> +
>> +#define ECC_DISABLED
>> +#define ECC_IN_NAND
>> +#define ECC_SOFT
>> +
>> +/* block lock */
>> +#define BL_ALL_LOCKED 0x38
>> +#define BL_1_2_LOCKED 0x30
>> +#define BL_1_4_LOCKED 0x28
>> +#define BL_1_8_LOCKED 0x20
>> +#define BL_1_16_LOCKED 0x18
>> +#define BL_1_32_LOCKED 0x10
>> +#define BL_1_64_LOCKED 0x08
>> +#define BL_ALL_UNLOCKED 0
> --
> Florian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists