From mboxrd@z Thu Jan 1 00:00:00 1970 Message-ID: <4BFCF06E.5010809@compulab.co.il> Date: Wed, 26 May 2010 12:57:02 +0300 From: Mike Rapoport MIME-Version: 1.0 To: Lei Wen Subject: Re: [PATCH 01/20] mtd: pxa3xx_nand: refuse the flash definition get from platform References: <4BFA2B5B.4080105@compulab.co.il> <4BFA5C58.7050109@compulab.co.il> <4BFA7D60.6020703@compulab.co.il> <4BFBA4BA.9020502@compulab.co.il> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: Eric Miao , David Woodhouse , Haojian Zhuang , Marek Vasut , linux-mtd@lists.infradead.org, Marc Kleine-Budde , David Woodhouse , linux-arm-kernel List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Lei Wen wrote: > On Tue, May 25, 2010 at 6:21 PM, Mike Rapoport wrote: >> Can you please elaborate what are the attributes that cannot be detected? > > The attributes comes from two new features that would be included into > pxa3xx_nand: BCH support and 8bit per 512bytes ECC. > Originally, our pxa3xx_nand only support Hamming ECC way. So no doubt > just read the timing register, and just set use ecc bit in NDCR is > enough. > But for now there is two type of ECC type. For distinguish, we > introduce a new register called NDECCCTRL. NDECCCTRL_BCH_EN in the bit > 0 of that register control whether use the BCH ECC. > You may ask me why not just read from NDECCCTRL to know the ECC type. > It is right, but not always to be true. It the last command executed > for nand before kernel take control is not read or write operation, > the bootloader has no reason to set the NDECCCTRL register. So our > kernel also unable to detect whether to use BCH or HAMMING ECC. > > This two type of ecc has different character, if use hamming ecc for > reading while that page is written by BCH, it would instant report of > DB error... > This is not the worst case., for we may ask the bootloader to always > set the NDECCCTRL. (There is no reason to apply ECC when do command > except the read and write although) > > The worst would comes at the supporting of 8bit per 512 bytes ECC. Our > BCH engine only support 16bit corroction per 2k data, aka 4bit per > 512bytes. > If we need to support 8bit per 512bytes feature, which is needed by a > lot of newest flash which require such high ecc strength. > Then we have to apply the BCH to 1k data instead of 2k. The layout of > content on flash would change. > Original BCH apply to 2k nand, the layout may like: | 2k data | + | 30 > bytes ECC | > Now after apply BCH to 1k range, the layout would be like: | 1k data | > + | 30 bytes ECC| + | 1k data| + |30 bytes ECC|. > Even as I said let the bootloader to always set the NDECCCTRL. The > kernel would still cannot distinguish the two use case of 4bit per > 512bytes and 8bit per 512 bytes ECC strength requirement. > For they both use BCH, the only difference could be told on the nand > layout, not by register reading. > > Those two new feature would lead different nand content layout. So if > not distinguish properly, nand would not be able to work functional. > And obviously, the two features cannot get from register reading, and > that is the reason why I suggest drop the feature. Why not make the ECC type and strength part of the pxa3xx_nand_platform_data? This way you can ensure NAND layout consistency for particular platform without reduction in the driver robustness. > Thanks, > Lei -- Sincerely yours, Mike. From mboxrd@z Thu Jan 1 00:00:00 1970 From: mike@compulab.co.il (Mike Rapoport) Date: Wed, 26 May 2010 12:57:02 +0300 Subject: [PATCH 01/20] mtd: pxa3xx_nand: refuse the flash definition get from platform In-Reply-To: References: <4BFA2B5B.4080105@compulab.co.il> <4BFA5C58.7050109@compulab.co.il> <4BFA7D60.6020703@compulab.co.il> <4BFBA4BA.9020502@compulab.co.il> Message-ID: <4BFCF06E.5010809@compulab.co.il> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Lei Wen wrote: > On Tue, May 25, 2010 at 6:21 PM, Mike Rapoport wrote: >> Can you please elaborate what are the attributes that cannot be detected? > > The attributes comes from two new features that would be included into > pxa3xx_nand: BCH support and 8bit per 512bytes ECC. > Originally, our pxa3xx_nand only support Hamming ECC way. So no doubt > just read the timing register, and just set use ecc bit in NDCR is > enough. > But for now there is two type of ECC type. For distinguish, we > introduce a new register called NDECCCTRL. NDECCCTRL_BCH_EN in the bit > 0 of that register control whether use the BCH ECC. > You may ask me why not just read from NDECCCTRL to know the ECC type. > It is right, but not always to be true. It the last command executed > for nand before kernel take control is not read or write operation, > the bootloader has no reason to set the NDECCCTRL register. So our > kernel also unable to detect whether to use BCH or HAMMING ECC. > > This two type of ecc has different character, if use hamming ecc for > reading while that page is written by BCH, it would instant report of > DB error... > This is not the worst case., for we may ask the bootloader to always > set the NDECCCTRL. (There is no reason to apply ECC when do command > except the read and write although) > > The worst would comes at the supporting of 8bit per 512 bytes ECC. Our > BCH engine only support 16bit corroction per 2k data, aka 4bit per > 512bytes. > If we need to support 8bit per 512bytes feature, which is needed by a > lot of newest flash which require such high ecc strength. > Then we have to apply the BCH to 1k data instead of 2k. The layout of > content on flash would change. > Original BCH apply to 2k nand, the layout may like: | 2k data | + | 30 > bytes ECC | > Now after apply BCH to 1k range, the layout would be like: | 1k data | > + | 30 bytes ECC| + | 1k data| + |30 bytes ECC|. > Even as I said let the bootloader to always set the NDECCCTRL. The > kernel would still cannot distinguish the two use case of 4bit per > 512bytes and 8bit per 512 bytes ECC strength requirement. > For they both use BCH, the only difference could be told on the nand > layout, not by register reading. > > Those two new feature would lead different nand content layout. So if > not distinguish properly, nand would not be able to work functional. > And obviously, the two features cannot get from register reading, and > that is the reason why I suggest drop the feature. Why not make the ECC type and strength part of the pxa3xx_nand_platform_data? This way you can ensure NAND layout consistency for particular platform without reduction in the driver robustness. > Thanks, > Lei -- Sincerely yours, Mike.