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
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <53BFBDFA.8040901@ti.com>
Date:	Fri, 11 Jul 2014 13:35:38 +0300
From:	Roger Quadros <rogerq@...com>
To:	"Gupta, Pekon" <pekon@...com>,
	"tony@...mide.com" <tony@...mide.com>,
	"computersforpeace@...il.com" <computersforpeace@...il.com>
CC:	"javier@...hile0.org" <javier@...hile0.org>,
	"ezequiel.garcia@...e-electrons.com" 
	<ezequiel.garcia@...e-electrons.com>,
	"dwmw2@...radead.org" <dwmw2@...radead.org>,
	"jg1.han@...sung.com" <jg1.han@...sung.com>,
	"Nori, Sekhar" <nsekhar@...com>,
	"linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
	"linux-omap@...r.kernel.org" <linux-omap@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH 02/10] mtd: nand: omap: Always use chip->ecc.steps
 for BCH sector count

On 07/11/2014 10:43 AM, Gupta, Pekon wrote:
>> From: Quadros, Roger
>>
>> Instead of hardcoding use the pre-calculated chip->ecc.steps for
>> configuring number of sectors to process with the BCH algorithm.
>>
>> This also avoids unnecessary access to the ECC_CONFIG register in
>> omap_calculate_ecc_bch().
>>
>> Signed-off-by: Roger Quadros <rogerq@...com>
>> ---
>> drivers/mtd/nand/omap2.c | 9 +++------
>> 1 file changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
>> index 5b8739c..6f3d7cd 100644
>> --- a/drivers/mtd/nand/omap2.c
>> +++ b/drivers/mtd/nand/omap2.c
>> @@ -1066,10 +1066,10 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info
>> *mtd, int mode)
>> 	unsigned int ecc_size1, ecc_size0;
>>
>> 	/* GPMC configurations for calculating ECC */
>> +	nsectors = chip->ecc.steps;
>> 	switch (ecc_opt) {
>> 	case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
>> 		bch_type = 0;
>> -		nsectors = 1;
>> 		if (mode == NAND_ECC_READ) {
>> 			wr_mode	  = BCH_WRAPMODE_6;
>> 			ecc_size0 = BCH_ECC_SIZE0;
>> @@ -1082,7 +1082,6 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info
>> *mtd, int mode)
>> 		break;
>> 	case OMAP_ECC_BCH4_CODE_HW:
>> 		bch_type = 0;
>> -		nsectors = chip->ecc.steps;
>> 		if (mode == NAND_ECC_READ) {
>> 			wr_mode	  = BCH_WRAPMODE_1;
>> 			ecc_size0 = BCH4R_ECC_SIZE0;
>> @@ -1095,7 +1094,6 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info
>> *mtd, int mode)
>> 		break;
>> 	case OMAP_ECC_BCH8_CODE_HW_DETECTION_SW:
>> 		bch_type = 1;
>> -		nsectors = 1;
>> 		if (mode == NAND_ECC_READ) {
>> 			wr_mode	  = BCH_WRAPMODE_6;
>> 			ecc_size0 = BCH_ECC_SIZE0;
>> @@ -1108,7 +1106,6 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info
>> *mtd, int mode)
>> 		break;
>> 	case OMAP_ECC_BCH8_CODE_HW:
>> 		bch_type = 1;
>> -		nsectors = chip->ecc.steps;
>> 		if (mode == NAND_ECC_READ) {
>> 			wr_mode	  = BCH_WRAPMODE_1;
>> 			ecc_size0 = BCH8R_ECC_SIZE0;
>> @@ -1121,7 +1118,6 @@ static void __maybe_unused omap_enable_hwecc_bch(struct mtd_info
>> *mtd, int mode)
>> 		break;
>> 	case OMAP_ECC_BCH16_CODE_HW:
>> 		bch_type = 0x2;
>> -		nsectors = chip->ecc.steps;
>> 		if (mode == NAND_ECC_READ) {
>> 			wr_mode	  = 0x01;
>> 			ecc_size0 = 52; /* ECC bits in nibbles per sector */
>> @@ -1176,6 +1172,7 @@ static int __maybe_unused omap_calculate_ecc_bch(struct mtd_info *mtd,
>> {
>> 	struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
>> 						   mtd);
>> +	struct nand_chip *chip = mtd->priv;
>> 	int eccbytes	= info->nand.ecc.bytes;
>> 	struct gpmc_nand_regs	*gpmc_regs = &info->reg;
>> 	u8 *ecc_code;
>> @@ -1183,7 +1180,7 @@ static int __maybe_unused omap_calculate_ecc_bch(struct mtd_info *mtd,
>> 	u32 val;
>> 	int i, j;
>>
>> -	nsectors = ((readl(info->reg.gpmc_ecc_config) >> 4) & 0x7) + 1;
>> +	nsectors = chip->ecc.steps;
> 
> Sorry NAK.. I'm sure you are breaking something here :-)
> 
> NAND driver supports multiple ECC schemes like;
> OMAP_ECC_CODE_HAM1_HW (support for legacy reasons)
> OMAP_ECC_CODE_BCH4_HW_DETECTION_SW (needed for OMAP3 and AM35xx)
> OMAP_ECC_CODE_BCH4_HW
> OMAP_ECC_CODE_BCH8_HW
> OMAP_ECC_CODE_BCH8_HW_DETECTION_SW  (needed for OMAP3 and AM35xx)
> OMAP_ECC_CODE_BCH16_HW
> 
> IIRC ..
> - software based ecc-schemes OMAP_ECC_CODE_BCHx_HW_DETECTION_SW
>   Reads/Write in per-sector granularity. (here nsector != chip->ecc.steps)

OK. I still don't have a full understanding about the ECC schemes so to ensure we
don't break anything I can just leave nsectors as it is and probably just store a
copy of it in omap_nand_info to avoid reading it back from gpmc_ecc_config.

I still have a few questions to clarify my understanding.

The only difference between OMAP_ECC_CODE_BCHx_HW_DETECTION_SW and
OMAP_ECC_CODE_BCHx_HW is that in the former the _correction_ is done by software
and in the latter the _correction_ is done by hardware (i.e. ELM module).
In both cases the _detection_ is done by the same hardware IP via ecc.calculate(),
i.e. omap_calculate_ecc_bch().

so why should nsector be different for both in the _detection_ stage?

An I right that ecc_steps is nothing but number of sub-blocks ECC calculation and correction
needs to be done for larger pages. This is a function of ECC hw capability (chip->ecc.size)
and NAND flash capability (mtd->writesize). i.e. ecc_steps = mtd->writesize / chip->ecc.size

We hardcode chip->ecc.size to 512 for all the ECC schemes in omap_nand_probe() so
calculate and correct will always be called for 512 byte sized blocks. So when does
the usecase for nsector > 1 come in?

cheers,
-roger
--
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

Powered by Openwall GNU/*/Linux Powered by OpenVZ